Add forwards-compatibility logic in `LocalHTLCFailureReason` ser
What the developer wrote
Authored by Matt Corallo
Add forwards-compatibility logic in `LocalHTLCFailureReason` ser
We recently created a `LocalHTLCFailureReason` enum to cover
detailed reasons why an HTLC may have failed, in excess of the
on-the-wire failure encoding.
Sadly, when we did so we introduced a serialization format which
isn't particularly conducive to introducing new failure reasons in
the future. Luckily, `LocalHTLCFailureReason` already has logic
that works for forwards-compatibility - it can be converted to, and
from, the on-the-wire error codes.
Thus, here, we take advantage of that, writing both the on-the-wire
error code as well as a code for the specific case. If we don't
recognize the specific case on deserialization, we'll fall back to
the default case for the on-the-wire code we've written.
We also, of course, include a length-prefixed TLV stream to allow
for additional fields in the future.
Fixes #4087
What changed, and why it matters
This commit changes how a Lightning node stores and reloads its own internal reasons for payment failures. The old format used a single fixed list of numeric codes, which would have made future upgrades fragile. The new format writes both the standard on-the-wire failure code and a separate internal reason code, plus a placeholder for future extra fields. If an older or newer version reads a reason it does not know, it now falls back to the standard failure code instead of potentially failing to load the data. This is a forward-compatibility improvement, not a fix for an active exploit.
Treat as a maintainability and compatibility improvement. Include in normal review and testing; verify that round-trip serialization of all enum variants and fallback behavior for unknown reason codes work as intended. No urgent security response is indicated by the commit content.
Security signals we found
Forward-compatibility hardening for persisted enum data
Graceful fallback to known on-the-wire failure code on unknown internal reason
Addition of length-prefixed TLV stream for future field extensions
No memory safety, cryptographic, or authorization changes visible
No explicit vulnerability or CVE referenced in commit message
Evidence from the diff
The patch replaces the impl_writeable_tlv_based_enum! macro usage for LocalHTLCFailureReason with a custom ser_failure_reasons! macro that implements Readable/Writeable directly. Serialization now emits a u16 on-the-wire failure code (self.failure_code()), a u8 internal reason discriminator, and an empty TLV stream. Deserialization reads the u16 code, the u8 reason, and any TLV fields; if the u8 reason is unrecognized it falls back to code.into(). The internal reason indices are also renumbered from odd TLV-style values (1,3,5…) to a dense 1-based sequence. This makes the persisted representation robust against adding new failure reasons and avoids deserialization errors when an unknown internal reason is encountered.
Changed components
lightning/src/ln/onion_utils.rsLocalHTLCFailureReason serialization/deserializationInspect captured patch +71 / −46
diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs
index 210910e..962bb9a 100644
--- a/lightning/src/ln/onion_utils.rs
+++ b/lightning/src/ln/onion_utils.rs
@@ -14,7 +14,7 @@ use crate::crypto::streams::ChaChaReader;
use crate::events::HTLCHandlingFailureReason;
use crate::ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS;
use crate::ln::channelmanager::{HTLCSource, RecipientOnionFields};
-use crate::ln::msgs;
+use crate::ln::msgs::{self, DecodeError};
use crate::offers::invoice_request::InvoiceRequest;
use crate::routing::gossip::NetworkUpdate;
use crate::routing::router::{BlindedTail, Path, RouteHop, RouteParameters, TrampolineHop};
@@ -1807,51 +1807,76 @@ impl_from_u16_for_htlc_reason!(
]
);
-impl_writeable_tlv_based_enum!(LocalHTLCFailureReason,
- (1, TemporaryNodeFailure) => {},
- (3, PermanentNodeFailure) => {},
- (5, RequiredNodeFeature) => {},
- (7, InvalidOnionVersion) => {},
- (9, InvalidOnionHMAC) => {},
- (11, InvalidOnionKey) => {},
- (13, TemporaryChannelFailure) => {},
- (15, PermanentChannelFailure) => {},
- (17, RequiredChannelFeature) => {},
- (19, UnknownNextPeer) => {},
- (21, AmountBelowMinimum) => {},
- (23, FeeInsufficient) => {},
- (25, IncorrectCLTVExpiry) => {},
- (27, CLTVExpiryTooSoon) => {},
- (29, IncorrectPaymentDetails) => {},
- (31, FinalIncorrectCLTVExpiry) => {},
- (33, FinalIncorrectHTLCAmount) => {},
- (35, ChannelDisabled) => {},
- (37, CLTVExpiryTooFar) => {},
- (39, InvalidOnionPayload) => {},
- (41, MPPTimeout) => {},
- (43, InvalidOnionBlinding) => {},
- (45, UnknownFailureCode) => {
- (0, code, required),
- },
- (47, ForwardExpiryBuffer) => {},
- (49, InvalidTrampolineForward) => {},
- (51, PaymentClaimBuffer) => {},
- (53, DustLimitHolder) => {},
- (55, DustLimitCounterparty) => {},
- (57, FeeSpikeBuffer) => {},
- (59, PrivateChannelForward) => {},
- (61, RealSCIDForward) => {},
- (63, ChannelNotReady) => {},
- (65, InvalidKeysendPreimage) => {},
- (67, InvalidTrampolinePayload) => {},
- (69, PaymentSecretRequired) => {},
- (71, OutgoingCLTVTooSoon) => {},
- (73, ChannelClosed) => {},
- (75, OnChainTimeout) => {},
- (77, ZeroAmount) => {},
- (79, HTLCMinimum) => {},
- (81, HTLCMaximum) => {},
- (83, PeerOffline) => {},
+macro_rules! ser_failure_reasons {
+ ($(($idx: expr, $name: ident)),*) => {
+ impl Readable for LocalHTLCFailureReason {
+ fn read<R: Read>(r: &mut R) -> Result<LocalHTLCFailureReason, DecodeError> {
+ let code: u16 = Readable::read(r)?;
+ let reason: u8 = Readable::read(r)?;
+ read_tlv_fields!(r, {});
+ match reason {
+ $($idx => Ok(LocalHTLCFailureReason::$name),)*
+ _ => Ok(code.into()),
+ }
+ }
+ }
+ impl Writeable for LocalHTLCFailureReason {
+ fn write<W: Writer>(&self, writer: &mut W) -> Result<(), bitcoin::io::Error> {
+ self.failure_code().write(writer)?;
+ let reason: u8 = match self {
+ $(LocalHTLCFailureReason::$name => $idx,)*
+ LocalHTLCFailureReason::UnknownFailureCode { .. } => 0xff,
+ };
+ reason.write(writer)?;
+ write_tlv_fields!(writer, {});
+ Ok(())
+ }
+ }
+ }
+}
+
+ser_failure_reasons!(
+ (1, TemporaryNodeFailure),
+ (2, PermanentNodeFailure),
+ (3, RequiredNodeFeature),
+ (4, InvalidOnionVersion),
+ (5, InvalidOnionHMAC),
+ (6, InvalidOnionKey),
+ (7, TemporaryChannelFailure),
+ (8, PermanentChannelFailure),
+ (9, RequiredChannelFeature),
+ (10, UnknownNextPeer),
+ (11, AmountBelowMinimum),
+ (12, FeeInsufficient),
+ (13, IncorrectCLTVExpiry),
+ (14, CLTVExpiryTooSoon),
+ (15, IncorrectPaymentDetails),
+ (16, FinalIncorrectCLTVExpiry),
+ (17, FinalIncorrectHTLCAmount),
+ (18, ChannelDisabled),
+ (19, CLTVExpiryTooFar),
+ (20, InvalidOnionPayload),
+ (21, MPPTimeout),
+ (22, InvalidOnionBlinding),
+ (23, ForwardExpiryBuffer),
+ (24, InvalidTrampolineForward),
+ (25, PaymentClaimBuffer),
+ (26, DustLimitHolder),
+ (27, DustLimitCounterparty),
+ (28, FeeSpikeBuffer),
+ (29, PrivateChannelForward),
+ (30, RealSCIDForward),
+ (31, ChannelNotReady),
+ (32, InvalidKeysendPreimage),
+ (33, InvalidTrampolinePayload),
+ (34, PaymentSecretRequired),
+ (35, OutgoingCLTVTooSoon),
+ (36, ChannelClosed),
+ (37, OnChainTimeout),
+ (38, ZeroAmount),
+ (39, HTLCMinimum),
+ (40, HTLCMaximum),
+ (41, PeerOffline)
);
impl From<&HTLCFailReason> for HTLCHandlingFailureReason {
Why this scored 31/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.