From c1e0cf45fd3e78a8d31ea07a538ebcfc4ef1c574 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Tue, 16 Dec 2025 13:37:23 +0200 Subject: [PATCH 01/32] ln/events: multiple htlcs in/out for trampoline PaymentForwarded --- .../tests/lsps2_integration_tests.rs | 8 +- lightning/src/events/mod.rs | 141 ++++++++++-------- lightning/src/ln/chanmon_update_fail_tests.rs | 4 +- lightning/src/ln/channelmanager.rs | 16 +- lightning/src/ln/functional_test_utils.rs | 42 +++--- lightning/src/ln/functional_tests.rs | 26 ++-- lightning/src/util/ser.rs | 1 + 7 files changed, 134 insertions(+), 104 deletions(-) diff --git a/lightning-liquidity/tests/lsps2_integration_tests.rs b/lightning-liquidity/tests/lsps2_integration_tests.rs index 33a6dd697cf..77be3cb5aa1 100644 --- a/lightning-liquidity/tests/lsps2_integration_tests.rs +++ b/lightning-liquidity/tests/lsps2_integration_tests.rs @@ -1331,14 +1331,14 @@ fn client_trusts_lsp_end_to_end_test() { let total_fee_msat = match service_events[0].clone() { Event::PaymentForwarded { - prev_node_id, - next_node_id, + ref prev_htlcs, + ref next_htlcs, skimmed_fee_msat, total_fee_earned_msat, .. } => { - assert_eq!(prev_node_id, Some(payer_node_id)); - assert_eq!(next_node_id, Some(client_node_id)); + assert_eq!(prev_htlcs[0].node_id, Some(payer_node_id)); + assert_eq!(next_htlcs[0].node_id, Some(client_node_id)); service_handler.payment_forwarded(channel_id, skimmed_fee_msat.unwrap_or(0)).unwrap(); Some(total_fee_earned_msat.unwrap() - skimmed_fee_msat.unwrap()) }, diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 3f6bb0efb01..fb669178fa7 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -45,7 +45,7 @@ use crate::util::ser::{ UpgradableRequired, WithoutLength, Writeable, Writer, }; -use crate::io; +use crate::io::{self, ErrorKind::InvalidData as IOInvalidData}; use crate::sync::Arc; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::Hash; @@ -738,6 +738,25 @@ pub enum InboundChannelFunds { DualFunded, } +/// Identifies the channel and peer committed to a HTLC, used for both incoming and outgoing HTLCs. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct HTLCLocator { + /// The channel that the HTLC was sent or received on. + pub channel_id: ChannelId, + + /// The `user_channel_id` for `channel_id`. + pub user_channel_id: Option, + + /// The public key identity of the node that the HTLC was sent to or received from. + pub node_id: Option, +} + +impl_writeable_tlv_based!(HTLCLocator, { + (1, channel_id, required), + (3, user_channel_id, option), + (5, node_id, option), +}); + /// An Event which you should probably take some action in response to. /// /// Note that while Writeable and Readable are implemented for Event, you probably shouldn't use @@ -1331,38 +1350,22 @@ pub enum Event { /// This event is generated when a payment has been successfully forwarded through us and a /// forwarding fee earned. /// + /// Note that downgrading from 0.3 and above with pending trampoline forwards that use multipart + /// payments will produce an event that only provides information about the first htlc that was + /// received/dispatched. + /// /// # Failure Behavior and Persistence /// This event will eventually be replayed after failures-to-handle (i.e., the event handler /// returning `Err(ReplayEvent ())`) and will be persisted across restarts. PaymentForwarded { - /// The channel id of the incoming channel between the previous node and us. - /// - /// This is only `None` for events generated or serialized by versions prior to 0.0.107. - prev_channel_id: Option, - /// The channel id of the outgoing channel between the next node and us. - /// - /// This is only `None` for events generated or serialized by versions prior to 0.0.107. - next_channel_id: Option, - /// The `user_channel_id` of the incoming channel between the previous node and us. - /// - /// This is only `None` for events generated or serialized by versions prior to 0.0.122. - prev_user_channel_id: Option, - /// The `user_channel_id` of the outgoing channel between the next node and us. - /// - /// This will be `None` if the payment was settled via an on-chain transaction. See the - /// caveat described for the `total_fee_earned_msat` field. Moreover it will be `None` for - /// events generated or serialized by versions prior to 0.0.122. - next_user_channel_id: Option, - /// The node id of the previous node. - /// - /// This is only `None` for HTLCs received prior to 0.1 or for events serialized by - /// versions prior to 0.1 - prev_node_id: Option, - /// The node id of the next node. - /// - /// This is only `None` for HTLCs received prior to 0.1 or for events serialized by - /// versions prior to 0.1 - next_node_id: Option, + /// The set of HTLCs forwarded to our node that will be claimed by this forward. Contains a + /// single HTLC for source-routed payments, and may contain multiple HTLCs when we acted as + /// a trampoline router, responsible for pathfinding within the route. + prev_htlcs: Vec, + /// The set of HTLCs forwarded by our node that have been claimed by this forward. Contains + /// a single HTLC for regular source-routed payments, and may contain multiple HTLCs when + /// we acted as a trampoline router, responsible for pathfinding within the route. + next_htlcs: Vec, /// The total fee, in milli-satoshis, which was earned as a result of the payment. /// /// Note that if we force-closed the channel over which we forwarded an HTLC while the HTLC @@ -2026,29 +2029,33 @@ impl Writeable for Event { }); }, &Event::PaymentForwarded { - prev_channel_id, - next_channel_id, - prev_user_channel_id, - next_user_channel_id, - prev_node_id, - next_node_id, + ref prev_htlcs, + ref next_htlcs, total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx, outbound_amount_forwarded_msat, } => { 7u8.write(writer)?; + // Fields 1, 3, 9, 11, 13 and 15 are written for backwards compatibility. + let legacy_prev = prev_htlcs.first().ok_or(io::Error::from(IOInvalidData))?; + let legacy_next = next_htlcs.first().ok_or(io::Error::from(IOInvalidData))?; write_tlv_fields!(writer, { (0, total_fee_earned_msat, option), - (1, prev_channel_id, option), + (1, Some(legacy_prev.channel_id), option), (2, claim_from_onchain_tx, required), - (3, next_channel_id, option), + (3, Some(legacy_next.channel_id), option), (5, outbound_amount_forwarded_msat, option), (7, skimmed_fee_msat, option), - (9, prev_user_channel_id, option), - (11, next_user_channel_id, option), - (13, prev_node_id, option), - (15, next_node_id, option), + (9, legacy_prev.user_channel_id, option), + (11, legacy_next.user_channel_id, option), + (13, legacy_prev.node_id, option), + (15, legacy_next.node_id, option), + // HTLCs are written as required, rather than required_vec, so that they can be + // deserialized using default_value to fill in legacy fields which expects + // LengthReadable (required_vec is WithoutLength). + (17, *prev_htlcs, required), + (19, *next_htlcs, required), }); }, &Event::ChannelClosed { @@ -2548,35 +2555,51 @@ impl MaybeReadable for Event { }, 7u8 => { let mut f = || { - let mut prev_channel_id = None; - let mut next_channel_id = None; - let mut prev_user_channel_id = None; - let mut next_user_channel_id = None; - let mut prev_node_id = None; - let mut next_node_id = None; + // Legacy values that have been replaced by prev_htlcs and next_htlcs. + let mut prev_channel_id_legacy = None; + let mut next_channel_id_legacy = None; + let mut prev_user_channel_id_legacy = None; + let mut next_user_channel_id_legacy = None; + let mut prev_node_id_legacy = None; + let mut next_node_id_legacy = None; + let mut total_fee_earned_msat = None; let mut skimmed_fee_msat = None; let mut claim_from_onchain_tx = false; let mut outbound_amount_forwarded_msat = None; + let mut prev_htlcs = vec![]; + let mut next_htlcs = vec![]; read_tlv_fields!(reader, { (0, total_fee_earned_msat, option), - (1, prev_channel_id, option), + (1, prev_channel_id_legacy, option), (2, claim_from_onchain_tx, required), - (3, next_channel_id, option), + (3, next_channel_id_legacy, option), (5, outbound_amount_forwarded_msat, option), (7, skimmed_fee_msat, option), - (9, prev_user_channel_id, option), - (11, next_user_channel_id, option), - (13, prev_node_id, option), - (15, next_node_id, option), + (9, prev_user_channel_id_legacy, option), + (11, next_user_channel_id_legacy, option), + (13, prev_node_id_legacy, option), + (15, next_node_id_legacy, option), + // We can unwrap in the eagerly-evaluated default_value code because we + // always write legacy fields to be backwards compatible, and expect + // this field to be set because the legacy field was only None for versions + // before 0.0.107 and we do not allow upgrades with pending forwards to 0.1 + // for any version 0.0.123 or earlier. + (17, prev_htlcs, (default_value, vec![HTLCLocator{ + channel_id: prev_channel_id_legacy.unwrap(), + user_channel_id: prev_user_channel_id_legacy, + node_id: prev_node_id_legacy, + }])), + (19, next_htlcs, (default_value, vec![HTLCLocator{ + channel_id: next_channel_id_legacy.unwrap(), + user_channel_id: next_user_channel_id_legacy, + node_id: next_node_id_legacy, + }])), }); + Ok(Some(Event::PaymentForwarded { - prev_channel_id, - next_channel_id, - prev_user_channel_id, - next_user_channel_id, - prev_node_id, - next_node_id, + prev_htlcs, + next_htlcs, total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx, diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index cd32d219b93..36428256d67 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -3940,11 +3940,11 @@ fn do_test_durable_preimages_on_closed_channel( let evs = nodes[1].node.get_and_clear_pending_events(); assert_eq!(evs.len(), if close_chans_before_reload { 2 } else { 1 }); for ev in evs { - if let Event::PaymentForwarded { claim_from_onchain_tx, next_user_channel_id, .. } = ev { + if let Event::PaymentForwarded { claim_from_onchain_tx, next_htlcs, .. } = ev { if !claim_from_onchain_tx { // If the outbound channel is still open, the `next_user_channel_id` should be available. // This was previously broken. - assert!(next_user_channel_id.is_some()) + assert!(next_htlcs[0].user_channel_id.is_some()) } } else { panic!(); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6bf04cd62a4..a5725a70fbd 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9756,12 +9756,16 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ ( Some(MonitorUpdateCompletionAction::EmitEventAndFreeOtherChannel { event: events::Event::PaymentForwarded { - prev_channel_id: Some(prev_channel_id), - next_channel_id: Some(next_channel_id), - prev_user_channel_id, - next_user_channel_id, - prev_node_id, - next_node_id: Some(next_channel_counterparty_node_id), + prev_htlcs: vec![events::HTLCLocator { + channel_id: prev_channel_id, + user_channel_id: prev_user_channel_id, + node_id: prev_node_id, + }], + next_htlcs: vec![events::HTLCLocator { + channel_id: next_channel_id, + user_channel_id: next_user_channel_id, + node_id: Some(next_channel_counterparty_node_id), + }], total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx: from_onchain, diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 2d971c3a100..641842ddaff 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -3095,17 +3095,16 @@ pub fn expect_payment_forwarded>( ) -> Option { match event { Event::PaymentForwarded { - prev_channel_id, - next_channel_id, - prev_user_channel_id, - next_user_channel_id, - prev_node_id, - next_node_id, + prev_htlcs, + next_htlcs, total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx, .. } => { + assert_eq!(prev_htlcs.len(), 1); + assert_eq!(next_htlcs.len(), 1); + if allow_1_msat_fee_overpay { // Aggregating fees for blinded paths may result in a rounding error, causing slight // overpayment in fees. @@ -3120,33 +3119,36 @@ pub fn expect_payment_forwarded>( // overpaid amount. assert!(skimmed_fee_msat == expected_extra_fees_msat); if !upstream_force_closed { - assert_eq!(prev_node.node().get_our_node_id(), prev_node_id.unwrap()); + let prev_node_id = prev_htlcs[0].node_id.unwrap(); + let prev_channel_id = prev_htlcs[0].channel_id; + let prev_user_channel_id = prev_htlcs[0].user_channel_id.unwrap(); + + assert_eq!(prev_node.node().get_our_node_id(), prev_node_id); // Is the event prev_channel_id in one of the channels between the two nodes? let node_chans = node.node().list_channels(); - assert!(node_chans.iter().any(|x| x.counterparty.node_id == prev_node_id.unwrap() - && x.channel_id == prev_channel_id.unwrap() - && x.user_channel_id == prev_user_channel_id.unwrap())); + assert!(node_chans.iter().any(|x| x.counterparty.node_id == prev_node_id + && x.channel_id == prev_channel_id + && x.user_channel_id == prev_user_channel_id)); } // We check for force closures since a force closed channel is removed from the // node's channel list if !downstream_force_closed { + let next_node_id = next_htlcs[0].node_id.unwrap(); + let next_channel_id = next_htlcs[0].channel_id; + let next_user_channel_id = next_htlcs[0].user_channel_id.unwrap(); // As documented, `next_user_channel_id` will only be `Some` if we didn't settle via an // onchain transaction, just as the `total_fee_earned_msat` field. Rather than // introducing yet another variable, we use the latter's state as a flag to detect // this and only check if it's `Some`. - assert_eq!(next_node.node().get_our_node_id(), next_node_id.unwrap()); + assert_eq!(next_node.node().get_our_node_id(), next_node_id); let node_chans = node.node().list_channels(); if total_fee_earned_msat.is_none() { - assert!(node_chans - .iter() - .any(|x| x.counterparty.node_id == next_node_id.unwrap() - && x.channel_id == next_channel_id.unwrap())); + assert!(node_chans.iter().any(|x| x.counterparty.node_id == next_node_id + && x.channel_id == next_channel_id)); } else { - assert!(node_chans - .iter() - .any(|x| x.counterparty.node_id == next_node_id.unwrap() - && x.channel_id == next_channel_id.unwrap() - && x.user_channel_id == next_user_channel_id.unwrap())); + assert!(node_chans.iter().any(|x| x.counterparty.node_id == next_node_id + && x.channel_id == next_channel_id + && x.user_channel_id == next_user_channel_id)); } } assert_eq!(claim_from_onchain_tx, downstream_force_closed); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 09a87d93156..17fbc1fce28 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -1490,37 +1490,37 @@ pub fn test_htlc_on_chain_success() { connect_blocks(&nodes[1], TEST_FINAL_CLTV); // Confirm blocks until the HTLC expires let forwarded_events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(forwarded_events.len(), 3); - let chan_id = Some(chan_1.2); + let chan_id = chan_1.2; match forwarded_events[0] { Event::PaymentForwarded { + ref prev_htlcs, + ref next_htlcs, total_fee_earned_msat, - prev_channel_id, claim_from_onchain_tx, - next_channel_id, outbound_amount_forwarded_msat, .. } => { assert_eq!(total_fee_earned_msat, Some(1000)); - assert_eq!(prev_channel_id, chan_id); + assert_eq!(prev_htlcs[0].channel_id, chan_id); assert_eq!(claim_from_onchain_tx, true); - assert_eq!(next_channel_id, Some(chan_2.2)); + assert_eq!(next_htlcs[0].channel_id, chan_2.2); assert_eq!(outbound_amount_forwarded_msat, Some(3000000)); }, _ => panic!(), } match forwarded_events[1] { Event::PaymentForwarded { + ref prev_htlcs, + ref next_htlcs, total_fee_earned_msat, - prev_channel_id, claim_from_onchain_tx, - next_channel_id, outbound_amount_forwarded_msat, .. } => { assert_eq!(total_fee_earned_msat, Some(1000)); - assert_eq!(prev_channel_id, chan_id); + assert_eq!(prev_htlcs[0].channel_id, chan_id); assert_eq!(claim_from_onchain_tx, true); - assert_eq!(next_channel_id, Some(chan_2.2)); + assert_eq!(next_htlcs[0].channel_id, chan_2.2); assert_eq!(outbound_amount_forwarded_msat, Some(3000000)); }, _ => panic!(), @@ -4031,17 +4031,17 @@ pub fn test_onchain_to_onchain_claim() { assert_eq!(events.len(), 2); match events[0] { Event::PaymentForwarded { + ref prev_htlcs, + ref next_htlcs, total_fee_earned_msat, - prev_channel_id, claim_from_onchain_tx, - next_channel_id, outbound_amount_forwarded_msat, .. } => { assert_eq!(total_fee_earned_msat, Some(1000)); - assert_eq!(prev_channel_id, Some(chan_1.2)); + assert_eq!(prev_htlcs[0].channel_id, chan_1.2); assert_eq!(claim_from_onchain_tx, true); - assert_eq!(next_channel_id, Some(chan_2.2)); + assert_eq!(next_htlcs[0].channel_id, chan_2.2); assert_eq!(outbound_amount_forwarded_msat, Some(3000000)); }, _ => panic!("Unexpected event"), diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 2eace55a4bf..45ca98b6fd0 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -1109,6 +1109,7 @@ impl_for_vec!(crate::routing::router::TrampolineHop); impl_for_vec_with_element_length_prefix!(crate::ln::msgs::UpdateAddHTLC); impl_writeable_for_vec_with_element_length_prefix!(&crate::ln::msgs::UpdateAddHTLC); impl_for_vec!(u32); +impl_for_vec!(crate::events::HTLCLocator); impl Writeable for Vec { #[inline] From 2e0126a24ea715c228b864f90517054c08f0a7d6 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 2 Mar 2026 09:23:14 +0200 Subject: [PATCH 02/32] f add note on when Option fields are None --- lightning/src/events/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index fb669178fa7..121cea660a5 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -745,9 +745,15 @@ pub struct HTLCLocator { pub channel_id: ChannelId, /// The `user_channel_id` for `channel_id`. + /// + /// This will be `None` if the payment was settled via an on-chain transaction. It will also + /// be `None` for events serialized by versions prior to 0.0.122. pub user_channel_id: Option, /// The public key identity of the node that the HTLC was sent to or received from. + /// + /// This is only `None` for HTLCs received prior to 0.1 or for events serialized by versions + /// prior to 0.1. pub node_id: Option, } From 90a778c85d26510c78fcbc4346b4cb1c3ee256ac Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 2 Mar 2026 09:28:48 +0200 Subject: [PATCH 03/32] f Write garbage data rather than failing to write PaymentForwarded --- lightning/src/events/mod.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 121cea660a5..ac481c3fa63 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -45,7 +45,7 @@ use crate::util::ser::{ UpgradableRequired, WithoutLength, Writeable, Writer, }; -use crate::io::{self, ErrorKind::InvalidData as IOInvalidData}; +use crate::io; use crate::sync::Arc; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::Hash; @@ -2043,9 +2043,23 @@ impl Writeable for Event { outbound_amount_forwarded_msat, } => { 7u8.write(writer)?; - // Fields 1, 3, 9, 11, 13 and 15 are written for backwards compatibility. - let legacy_prev = prev_htlcs.first().ok_or(io::Error::from(IOInvalidData))?; - let legacy_next = next_htlcs.first().ok_or(io::Error::from(IOInvalidData))?; + // Fields 1, 3, 9, 11, 13 and 15 are written for backwards compatibility. We don't + // want to fail writes, so we write garbage data if we don't have at least on htlc. + debug_assert!( + !prev_htlcs.is_empty(), + "at least one prev_htlc required for PaymentForwarded", + ); + debug_assert!( + !next_htlcs.is_empty(), + "at least one next_htlc required for PaymentForwarded", + ); + let empty_locator = HTLCLocator { + channel_id: ChannelId::new_zero(), + user_channel_id: None, + node_id: None, + }; + let legacy_prev = prev_htlcs.first().unwrap_or(&empty_locator); + let legacy_next = next_htlcs.first().unwrap_or(&empty_locator); write_tlv_fields!(writer, { (0, total_fee_earned_msat, option), (1, Some(legacy_prev.channel_id), option), From 35d6c5a2dfbaacdfa28822b4b67ef5484823d393 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 2 Mar 2026 09:40:25 +0200 Subject: [PATCH 04/32] f Use zero channel ID instead of unwrap for unexpected None channel_id This should never actually happen, but we don't want to panic. --- lightning/src/events/mod.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index ac481c3fa63..0c645b62043 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -2600,18 +2600,19 @@ impl MaybeReadable for Event { (11, next_user_channel_id_legacy, option), (13, prev_node_id_legacy, option), (15, next_node_id_legacy, option), - // We can unwrap in the eagerly-evaluated default_value code because we - // always write legacy fields to be backwards compatible, and expect - // this field to be set because the legacy field was only None for versions - // before 0.0.107 and we do not allow upgrades with pending forwards to 0.1 - // for any version 0.0.123 or earlier. + // We never expect prev/next_channel_id_legacy to be None because this field + // was only None for versions before 0.0.107 and we do not allow upgrades + // with pending forwards to 0.1 for any version 0.0.123 or earlier. We + // currently write the legacy fields for backwards compatibility, but we + // use a zero channel ID in the eagerly evaluated default_value block to + // allow the legacy field to be deprecated in future. (17, prev_htlcs, (default_value, vec![HTLCLocator{ - channel_id: prev_channel_id_legacy.unwrap(), + channel_id: prev_channel_id_legacy.unwrap_or(ChannelId::new_zero()), user_channel_id: prev_user_channel_id_legacy, node_id: prev_node_id_legacy, }])), (19, next_htlcs, (default_value, vec![HTLCLocator{ - channel_id: next_channel_id_legacy.unwrap(), + channel_id: next_channel_id_legacy.unwrap_or(ChannelId::new_zero()), user_channel_id: next_user_channel_id_legacy, node_id: next_node_id_legacy, }])), From 56fc0700ee77e938469b9cfa7816d023eafebee8 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Tue, 3 Mar 2026 11:37:31 +0200 Subject: [PATCH 05/32] f Fail to read if new and legacy fields are not populated --- lightning/src/events/mod.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 0c645b62043..8973f7c59de 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -2618,6 +2618,13 @@ impl MaybeReadable for Event { }])), }); + // Legacy fields must be present if prev/next_htlcs are not. + if prev_htlcs.is_empty() && prev_node_id_legacy.is_none() + || next_htlcs.is_empty() && next_node_id_legacy.is_none() + { + return Err(msgs::DecodeError::InvalidValue); + } + Ok(Some(Event::PaymentForwarded { prev_htlcs, next_htlcs, From 6f9a4cd74b8028dbbb511334e7fa10bcadf43b3d Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Tue, 16 Dec 2025 15:14:55 +0200 Subject: [PATCH 06/32] ln: make event optional in EmitEventAndFreeOtherChannel In the commits that follow, we want to be able to free the other channel without emitting an event so that we can emit a single event for trampoline payments with multiple incoming HTLCs. We still want to go through the full claim flow for each incoming HTLC (and persist the EmitEventAndFreeOtherChannel event to be picked up on restart), but do not want multiple events for the same trampoline forward. Changing from upgradable_required to upgradable_option is forwards compatible - old versions of the software will always have written this field, newer versions don't require it to be there but will be able to read it as-is. This change is not backwards compatible, because older versions of the software will expect the field to be present but newer versions may not write it. An alternative would be to add a new event type, but that would need to have an even TLV (because the event must be understood and processed on restart to claim the incoming HTLC), so that option isn't backwards compatible either. --- lightning/src/ln/channelmanager.rs | 15 ++++++++++----- pending_changelog/4304.txt | 3 +++ 2 files changed, 13 insertions(+), 5 deletions(-) create mode 100644 pending_changelog/4304.txt diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a5725a70fbd..d9aa933494e 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1393,7 +1393,7 @@ pub(crate) enum MonitorUpdateCompletionAction { /// edge completes, we will surface an [`Event::PaymentForwarded`] as well as unblock the /// outbound edge. EmitEventAndFreeOtherChannel { - event: events::Event, + event: Option, downstream_counterparty_and_funding_outpoint: Option, }, /// Indicates we should immediately resume the operation of another channel, unless there is @@ -1428,7 +1428,10 @@ impl_writeable_tlv_based_enum_upgradable!(MonitorUpdateCompletionAction, (5, downstream_channel_id, required), }, (2, EmitEventAndFreeOtherChannel) => { - (0, event, upgradable_required), + // LDK prior to 0.3 required this field. It will not be present for trampoline payments + // with multiple incoming HTLCS, so nodes cannot downgrade while trampoline payments + // are in the process of being resolved. + (0, event, upgradable_option), // LDK prior to 0.0.116 did not have this field as the monitor update application order was // required by clients. If we downgrade to something prior to 0.0.116 this may result in // monitor updates which aren't properly blocked or resumed, however that's fine - we don't @@ -9755,7 +9758,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ ); ( Some(MonitorUpdateCompletionAction::EmitEventAndFreeOtherChannel { - event: events::Event::PaymentForwarded { + event: Some(events::Event::PaymentForwarded { prev_htlcs: vec![events::HTLCLocator { channel_id: prev_channel_id, user_channel_id: prev_user_channel_id, @@ -9770,7 +9773,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ skimmed_fee_msat, claim_from_onchain_tx: from_onchain, outbound_amount_forwarded_msat: forwarded_htlc_value_msat, - }, + }), downstream_counterparty_and_funding_outpoint: chan_to_release, }), None, @@ -10000,7 +10003,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ event, downstream_counterparty_and_funding_outpoint, } => { - self.pending_events.lock().unwrap().push_back((event, None)); + if let Some(event) = event { + self.pending_events.lock().unwrap().push_back((event, None)); + } if let Some(unblocked) = downstream_counterparty_and_funding_outpoint { self.handle_monitor_update_release( unblocked.counterparty_node_id, diff --git a/pending_changelog/4304.txt b/pending_changelog/4304.txt new file mode 100644 index 00000000000..8c1580a2f4c --- /dev/null +++ b/pending_changelog/4304.txt @@ -0,0 +1,3 @@ +## Backwards Compatibility + +* Downgrade is not possible while the node has in-flight trampoline forwards. From f4b24a7fd0d55dd3aed01969de87fe54356478a1 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 7 Jan 2026 15:36:30 -0500 Subject: [PATCH 07/32] ln/refactor: rename EmitEventAndFreeOtherChannel to note optional event --- lightning/src/ln/channelmanager.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d9aa933494e..ef3d59e9048 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1392,7 +1392,7 @@ pub(crate) enum MonitorUpdateCompletionAction { /// completes a monitor update containing the payment preimage. In that case, after the inbound /// edge completes, we will surface an [`Event::PaymentForwarded`] as well as unblock the /// outbound edge. - EmitEventAndFreeOtherChannel { + EmitEventOptionAndFreeOtherChannel { event: Option, downstream_counterparty_and_funding_outpoint: Option, }, @@ -1403,8 +1403,8 @@ pub(crate) enum MonitorUpdateCompletionAction { /// This is usually generated when we've forwarded an HTLC and want to block the outbound edge /// from completing a monitor update which removes the payment preimage until the inbound edge /// completes a monitor update containing the payment preimage. However, we use this variant - /// instead of [`Self::EmitEventAndFreeOtherChannel`] when we discover that the claim was in - /// fact duplicative and we simply want to resume the outbound edge channel immediately. + /// instead of [`Self::EmitEventOptionAndFreeOtherChannel`] when we discover that the claim was + /// in fact duplicative and we simply want to resume the outbound edge channel immediately. /// /// This variant should thus never be written to disk, as it is processed inline rather than /// stored for later processing. @@ -1427,7 +1427,7 @@ impl_writeable_tlv_based_enum_upgradable!(MonitorUpdateCompletionAction, (4, blocking_action, upgradable_required), (5, downstream_channel_id, required), }, - (2, EmitEventAndFreeOtherChannel) => { + (2, EmitEventOptionAndFreeOtherChannel) => { // LDK prior to 0.3 required this field. It will not be present for trampoline payments // with multiple incoming HTLCS, so nodes cannot downgrade while trampoline payments // are in the process of being resolved. @@ -9757,7 +9757,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ "skimmed_fee_msat must always be included in total_fee_earned_msat" ); ( - Some(MonitorUpdateCompletionAction::EmitEventAndFreeOtherChannel { + Some(MonitorUpdateCompletionAction::EmitEventOptionAndFreeOtherChannel { event: Some(events::Event::PaymentForwarded { prev_htlcs: vec![events::HTLCLocator { channel_id: prev_channel_id, @@ -9999,7 +9999,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } }, - MonitorUpdateCompletionAction::EmitEventAndFreeOtherChannel { + MonitorUpdateCompletionAction::EmitEventOptionAndFreeOtherChannel { event, downstream_counterparty_and_funding_outpoint, } => { @@ -19511,7 +19511,7 @@ impl< let logger = WithContext::from(&args.logger, Some(node_id), Some(*channel_id), None); for action in actions.iter() { - if let MonitorUpdateCompletionAction::EmitEventAndFreeOtherChannel { + if let MonitorUpdateCompletionAction::EmitEventOptionAndFreeOtherChannel { downstream_counterparty_and_funding_outpoint: Some(EventUnblockedChannel { counterparty_node_id: blocked_node_id, From 76b010529f2bf12c2a65f9ec406e3d7d60b13ee6 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 25 Feb 2026 15:35:42 +0200 Subject: [PATCH 08/32] ln: make channel required in `MonitorUpdateCompletionAction` `downstream_counterparty_and_funding_outpoint` was added to LDK in 0.0.116. We do not allow direct upgrades with pending forwards to 0.1 from 0.0.123 and below, so we can now assume that this field will always be present. This change also makes it impossible to create a `EmitEventOptionAndFreeOtherChannel` action with nothing in it (no event or channel), which could have been possible now that we've made the event optional). --- lightning/src/ln/channelmanager.rs | 47 +++++++++++++----------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index ef3d59e9048..d60c21c8587 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1394,7 +1394,7 @@ pub(crate) enum MonitorUpdateCompletionAction { /// outbound edge. EmitEventOptionAndFreeOtherChannel { event: Option, - downstream_counterparty_and_funding_outpoint: Option, + downstream_counterparty_and_funding_outpoint: EventUnblockedChannel, }, /// Indicates we should immediately resume the operation of another channel, unless there is /// some other reason why the channel is blocked. In practice this simply means immediately @@ -1432,12 +1432,7 @@ impl_writeable_tlv_based_enum_upgradable!(MonitorUpdateCompletionAction, // with multiple incoming HTLCS, so nodes cannot downgrade while trampoline payments // are in the process of being resolved. (0, event, upgradable_option), - // LDK prior to 0.0.116 did not have this field as the monitor update application order was - // required by clients. If we downgrade to something prior to 0.0.116 this may result in - // monitor updates which aren't properly blocked or resumed, however that's fine - we don't - // support async monitor updates even in LDK 0.0.116 and once we do we'll require no - // downgrades to prior versions. - (1, downstream_counterparty_and_funding_outpoint, upgradable_option), + (1, downstream_counterparty_and_funding_outpoint, upgradable_required), }, ); @@ -9674,12 +9669,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ None, Some(attribution_data), |htlc_claim_value_msat, definitely_duplicate| { - let chan_to_release = Some(EventUnblockedChannel { + let chan_to_release = EventUnblockedChannel { counterparty_node_id: next_channel_counterparty_node_id, funding_txo: next_channel_outpoint, channel_id: next_channel_id, blocking_action: completed_blocker, - }); + }; if definitely_duplicate && startup_replay { // On startup we may get redundant claims which are related to @@ -9732,15 +9727,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } (None, None) } else if definitely_duplicate { - if let Some(other_chan) = chan_to_release { - (Some(MonitorUpdateCompletionAction::FreeOtherChannelImmediately { - downstream_counterparty_node_id: other_chan.counterparty_node_id, - downstream_channel_id: other_chan.channel_id, - blocking_action: other_chan.blocking_action, - }), None) - } else { - (None, None) - } + ( + Some(MonitorUpdateCompletionAction::FreeOtherChannelImmediately { + downstream_counterparty_node_id: chan_to_release + .counterparty_node_id, + downstream_channel_id: chan_to_release.channel_id, + blocking_action: chan_to_release.blocking_action, + }), + None, + ) } else { let total_fee_earned_msat = if let Some(forwarded_htlc_value) = forwarded_htlc_value_msat { @@ -10006,13 +10001,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(event) = event { self.pending_events.lock().unwrap().push_back((event, None)); } - if let Some(unblocked) = downstream_counterparty_and_funding_outpoint { - self.handle_monitor_update_release( - unblocked.counterparty_node_id, - unblocked.channel_id, - Some(unblocked.blocking_action), - ); - } + self.handle_monitor_update_release( + downstream_counterparty_and_funding_outpoint.counterparty_node_id, + downstream_counterparty_and_funding_outpoint.channel_id, + Some(downstream_counterparty_and_funding_outpoint.blocking_action), + ); }, MonitorUpdateCompletionAction::FreeOtherChannelImmediately { downstream_counterparty_node_id, @@ -19513,12 +19506,12 @@ impl< for action in actions.iter() { if let MonitorUpdateCompletionAction::EmitEventOptionAndFreeOtherChannel { downstream_counterparty_and_funding_outpoint: - Some(EventUnblockedChannel { + EventUnblockedChannel { counterparty_node_id: blocked_node_id, funding_txo: _, channel_id: blocked_channel_id, blocking_action, - }), + }, .. } = action { From bfacf133d765abbe2d77204d68993948d63e628e Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Thu, 26 Feb 2026 09:51:50 +0200 Subject: [PATCH 09/32] ln/refactor: rename FreeOtherChannelImmediately to FreeDuplicateClaimImmediately --- lightning/src/ln/channelmanager.rs | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d60c21c8587..acf18720de3 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1400,15 +1400,15 @@ pub(crate) enum MonitorUpdateCompletionAction { /// some other reason why the channel is blocked. In practice this simply means immediately /// removing the [`RAAMonitorUpdateBlockingAction`] provided from the blocking set. /// - /// This is usually generated when we've forwarded an HTLC and want to block the outbound edge - /// from completing a monitor update which removes the payment preimage until the inbound edge + /// This is generated when we've forwarded an HTLC and want to block the outbound edge from + /// completing a monitor update which removes the payment preimage until the inbound edge /// completes a monitor update containing the payment preimage. However, we use this variant /// instead of [`Self::EmitEventOptionAndFreeOtherChannel`] when we discover that the claim was /// in fact duplicative and we simply want to resume the outbound edge channel immediately. /// /// This variant should thus never be written to disk, as it is processed inline rather than /// stored for later processing. - FreeOtherChannelImmediately { + FreeDuplicateClaimImmediately { downstream_counterparty_node_id: PublicKey, blocking_action: RAAMonitorUpdateBlockingAction, downstream_channel_id: ChannelId, @@ -1420,9 +1420,9 @@ impl_writeable_tlv_based_enum_upgradable!(MonitorUpdateCompletionAction, (0, payment_hash, required), (9999999999, pending_mpp_claim, (static_value, None)), }, - // Note that FreeOtherChannelImmediately should never be written - we were supposed to free + // Note that FreeDuplicateClaimImmediately should never be written - we were supposed to free // *immediately*. However, for simplicity we implement read/write here. - (1, FreeOtherChannelImmediately) => { + (1, FreeDuplicateClaimImmediately) => { (0, downstream_counterparty_node_id, required), (4, blocking_action, upgradable_required), (5, downstream_channel_id, required), @@ -9420,7 +9420,7 @@ impl< log_trace!(logger, "Completing monitor update completion action as claim was redundant: {:?}", action); - if let MonitorUpdateCompletionAction::FreeOtherChannelImmediately { + if let MonitorUpdateCompletionAction::FreeDuplicateClaimImmediately { downstream_counterparty_node_id: node_id, blocking_action: blocker, downstream_channel_id: channel_id, @@ -9728,12 +9728,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ (None, None) } else if definitely_duplicate { ( - Some(MonitorUpdateCompletionAction::FreeOtherChannelImmediately { - downstream_counterparty_node_id: chan_to_release - .counterparty_node_id, - downstream_channel_id: chan_to_release.channel_id, - blocking_action: chan_to_release.blocking_action, - }), + Some( + MonitorUpdateCompletionAction::FreeDuplicateClaimImmediately { + downstream_counterparty_node_id: chan_to_release + .counterparty_node_id, + downstream_channel_id: chan_to_release.channel_id, + blocking_action: chan_to_release.blocking_action, + }, + ), None, ) } else { @@ -10007,7 +10009,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ Some(downstream_counterparty_and_funding_outpoint.blocking_action), ); }, - MonitorUpdateCompletionAction::FreeOtherChannelImmediately { + MonitorUpdateCompletionAction::FreeDuplicateClaimImmediately { downstream_counterparty_node_id, downstream_channel_id, blocking_action, @@ -19534,7 +19536,7 @@ impl< // anymore. } } - if let MonitorUpdateCompletionAction::FreeOtherChannelImmediately { + if let MonitorUpdateCompletionAction::FreeDuplicateClaimImmediately { .. } = action { From 2805d6a8480575fa2669cce17d656493c3503c5b Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 7 Jan 2026 15:05:44 -0500 Subject: [PATCH 10/32] ln+events: allow multiple prev_channel_id in HTLCHandlingFailed In preparation for trampoline failures, allow multiple previous channel ids. We'll only emit a single HTLCHandlingFailed for all of our failed back HTLCs, so we want to be able to express all of them in one event. --- lightning/src/events/mod.rs | 27 ++++++++++++++++++++------- lightning/src/ln/channelmanager.rs | 4 ++-- lightning/src/ln/monitor_tests.rs | 4 ++-- lightning/src/util/ser.rs | 1 + 4 files changed, 25 insertions(+), 11 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 8973f7c59de..551552c446f 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1665,12 +1665,17 @@ pub enum Event { /// Indicates that the HTLC was accepted, but could not be processed when or after attempting to /// forward it. /// + /// Note that downgrading from 0.3 with pending trampoline forwards that have incoming multipart + /// payments will produce an event that only provides information about the first htlc that was + /// received/dispatched. + /// /// # Failure Behavior and Persistence /// This event will eventually be replayed after failures-to-handle (i.e., the event handler /// returning `Err(ReplayEvent ())`) and will be persisted across restarts. HTLCHandlingFailed { - /// The channel over which the HTLC was received. - prev_channel_id: ChannelId, + /// The channel(s) over which the HTLC(s) was received. May contain multiple entries for + /// trampoline forwards. + prev_channel_ids: Vec, /// The type of HTLC handling that failed. failure_type: HTLCHandlingFailureType, /// The reason that the HTLC failed. @@ -2223,15 +2228,19 @@ impl Writeable for Event { }) }, &Event::HTLCHandlingFailed { - ref prev_channel_id, + ref prev_channel_ids, ref failure_type, ref failure_reason, } => { 25u8.write(writer)?; + let legacy_chan_id = + prev_channel_ids.first().ok_or(io::Error::from(IOInvalidData))?; write_tlv_fields!(writer, { - (0, prev_channel_id, required), + // Write legacy field to remain backwards compatible. + (0, legacy_chan_id, required), (1, failure_reason, option), (2, failure_type, required), + (3, *prev_channel_ids, required), }) }, &Event::BumpTransaction(ref event) => { @@ -2817,13 +2826,17 @@ impl MaybeReadable for Event { }, 25u8 => { let mut f = || { - let mut prev_channel_id = ChannelId::new_zero(); + let mut prev_channel_id_legacy = ChannelId::new_zero(); let mut failure_reason = None; let mut failure_type_opt = UpgradableRequired(None); + let mut prev_channel_ids = vec![]; read_tlv_fields!(reader, { - (0, prev_channel_id, required), + (0, prev_channel_id_legacy, required), (1, failure_reason, option), (2, failure_type_opt, upgradable_required), + (3, prev_channel_ids, (default_value, vec![ + prev_channel_id_legacy, + ])), }); // If a legacy HTLCHandlingFailureType::UnknownNextHop was written, upgrade @@ -2838,7 +2851,7 @@ impl MaybeReadable for Event { failure_reason = Some(LocalHTLCFailureReason::UnknownNextPeer.into()); } Ok(Some(Event::HTLCHandlingFailed { - prev_channel_id, + prev_channel_ids, failure_type: _init_tlv_based_struct_field!( failure_type_opt, upgradable_required diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index acf18720de3..2520d684d08 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7423,7 +7423,7 @@ impl< .push(failure); self.pending_events.lock().unwrap().push_back(( events::Event::HTLCHandlingFailed { - prev_channel_id: incoming_channel_id, + prev_channel_ids: vec![incoming_channel_id], failure_type, failure_reason: Some(failure_reason), }, @@ -9018,7 +9018,7 @@ impl< let mut pending_events = self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::HTLCHandlingFailed { - prev_channel_id: *channel_id, + prev_channel_ids: vec![*channel_id], failure_type, failure_reason: Some(onion_error.into()), }, diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 18a976871a6..2368776dd3f 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -3780,8 +3780,8 @@ fn do_test_lost_timeout_monitor_events(confirm_tx: CommitmentType, dust_htlcs: b Event::PaymentFailed { payment_hash, .. } => { assert_eq!(payment_hash, Some(hash_b)); }, - Event::HTLCHandlingFailed { prev_channel_id, .. } => { - assert_eq!(prev_channel_id, chan_a); + Event::HTLCHandlingFailed { prev_channel_ids, .. } => { + assert_eq!(prev_channel_ids[0], chan_a); }, _ => panic!("Wrong event {ev:?}"), } diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 45ca98b6fd0..b226332ae93 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -1110,6 +1110,7 @@ impl_for_vec_with_element_length_prefix!(crate::ln::msgs::UpdateAddHTLC); impl_writeable_for_vec_with_element_length_prefix!(&crate::ln::msgs::UpdateAddHTLC); impl_for_vec!(u32); impl_for_vec!(crate::events::HTLCLocator); +impl_for_vec!(crate::ln::types::ChannelId); impl Writeable for Vec { #[inline] From 897b7618b6b63b028c04bb15355f24aaa73c8898 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 2 Mar 2026 10:25:41 +0200 Subject: [PATCH 11/32] f Write garbage data rather than failing to write HTLCHandlingFailed --- lightning/src/events/mod.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 551552c446f..b5dee3e7ec1 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -2233,10 +2233,15 @@ impl Writeable for Event { ref failure_reason, } => { 25u8.write(writer)?; - let legacy_chan_id = - prev_channel_ids.first().ok_or(io::Error::from(IOInvalidData))?; + // Legacy field is written for backwards compatibility. We don't want to fail writes + // so we write garbage data if we don't have the data we expect. + debug_assert!( + !prev_channel_ids.is_empty(), + "at least one prev_channel_id required for HTLCHandlingFailed" + ); + let zero_id = ChannelId::new_zero(); + let legacy_chan_id = prev_channel_ids.first().unwrap_or(&zero_id); write_tlv_fields!(writer, { - // Write legacy field to remain backwards compatible. (0, legacy_chan_id, required), (1, failure_reason, option), (2, failure_type, required), From 7f88d37ede35911e75216ed41c6c574f20c4c73b Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Tue, 6 Jan 2026 15:28:42 -0500 Subject: [PATCH 12/32] events: add TrampolineForward variant to HTLCHandlingFailureType --- lightning/src/events/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index b5dee3e7ec1..0321a55f561 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -584,6 +584,10 @@ pub enum HTLCHandlingFailureType { /// The payment hash of the payment we attempted to process. payment_hash: PaymentHash, }, + /// We were responsible for pathfinding and forwarding of a trampoline payment, but failed to + /// do so. An example of such an instance is when we can't find a route to the specified + /// trampoline destination. + TrampolineForward {}, } impl_writeable_tlv_based_enum_upgradable!(HTLCHandlingFailureType, @@ -601,6 +605,7 @@ impl_writeable_tlv_based_enum_upgradable!(HTLCHandlingFailureType, (4, Receive) => { (0, payment_hash, required), }, + (5, TrampolineForward) => {}, ); /// The reason for HTLC failures in [`Event::HTLCHandlingFailed`]. From 4d58be695d52a46593d38823c6d6059370ac46fd Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Tue, 2 Dec 2025 10:06:41 -0500 Subject: [PATCH 13/32] ln: add TrampolineForward SendHTLCId variant This commit adds a SendHTLCId for trampoline forwards, identified by their session_priv. As with an OutboundRoute, we can expect our HTLC to be uniquely identified by a randomly generated session_priv. TrampolineForward could also be identified by the set of all previous outbound scid/htlc id pairs that represent its incoming HTLC(s). We choose the 32 byte session_priv to fix the size of this identifier rather than 16 byte scid/id pairs that will grow with the number of incoming htlcs. --- lightning/src/ln/channelmanager.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2520d684d08..19322baa320 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -756,6 +756,7 @@ impl Default for OptionalOfferPaymentParams { pub(crate) enum SentHTLCId { PreviousHopData { prev_outbound_scid_alias: u64, htlc_id: u64 }, OutboundRoute { session_priv: [u8; SECRET_KEY_SIZE] }, + TrampolineForward { session_priv: [u8; SECRET_KEY_SIZE] }, } impl SentHTLCId { pub(crate) fn from_source(source: &HTLCSource) -> Self { @@ -778,6 +779,9 @@ impl_writeable_tlv_based_enum!(SentHTLCId, (2, OutboundRoute) => { (0, session_priv, required), }, + (4, TrampolineForward) => { + (0, session_priv, required), + }, ); type FailedHTLCForward = (HTLCSource, PaymentHash, HTLCFailReason, HTLCHandlingFailureType); From 073198010553a0fff0961fc8d66b036cbd531a6a Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 22 Aug 2025 10:37:21 -0400 Subject: [PATCH 14/32] ln: add TrampolineForward variant to HTLCSource enum We only have payment details for HTLCSource::TrampolineForward available once we've dispatched the payment. If we get to the stage where we need a HTLCId for the outbound payment, we expect dispatch details to be present. Co-authored-by: Arik Sosman Co-authored-by: Maurice Poirrier --- lightning/src/chain/channelmonitor.rs | 2 + lightning/src/ln/channelmanager.rs | 70 ++++++++++++++++++++++++++- lightning/src/routing/router.rs | 5 ++ 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index a8d055a9c5b..f4d57142531 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -2795,6 +2795,7 @@ impl ChannelMonitorImpl { let outbound_payment = match source { None => panic!("Outbound HTLCs should have a source"), Some(&HTLCSource::PreviousHopData(_)) => false, + Some(&HTLCSource::TrampolineForward { .. }) => false, Some(&HTLCSource::OutboundRoute { .. }) => true, }; return Some(Balance::MaybeTimeoutClaimableHTLC { @@ -3007,6 +3008,7 @@ impl ChannelMonitor { let outbound_payment = match source { None => panic!("Outbound HTLCs should have a source"), Some(HTLCSource::PreviousHopData(_)) => false, + Some(HTLCSource::TrampolineForward { .. }) => false, Some(HTLCSource::OutboundRoute { .. }) => true, }; if outbound_payment { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 19322baa320..053f8feddfe 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -759,12 +759,23 @@ pub(crate) enum SentHTLCId { TrampolineForward { session_priv: [u8; SECRET_KEY_SIZE] }, } impl SentHTLCId { + /// Creates an identifier for the [`HTLCSource`] provided. Note that for MPP trampoline payments + /// each outgoing HTLC will have a distinct identifier. pub(crate) fn from_source(source: &HTLCSource) -> Self { match source { HTLCSource::PreviousHopData(hop_data) => Self::PreviousHopData { prev_outbound_scid_alias: hop_data.prev_outbound_scid_alias, htlc_id: hop_data.htlc_id, }, + HTLCSource::TrampolineForward { + ref outbound_payment, + .. + } => Self::TrampolineForward { + session_priv: outbound_payment + .as_ref() + .map(|o| o.session_priv.secret_bytes()) + .expect("trying to identify a trampoline payment that we have no outbound_payment tracked for"), + }, HTLCSource::OutboundRoute { session_priv, .. } => { Self::OutboundRoute { session_priv: session_priv.secret_bytes() } }, @@ -789,11 +800,31 @@ type FailedHTLCForward = (HTLCSource, PaymentHash, HTLCFailReason, HTLCHandlingF mod fuzzy_channelmanager { use super::*; + /// Information about a HTLC sent as part of a (possibly MPP) payment to the next trampoline. + #[derive(Clone, Debug, PartialEq, Eq)] + pub struct TrampolineDispatch { + /// The payment ID used for the outbound payment. + pub payment_id: PaymentId, + /// The path used for the outbound payment. + pub path: Path, + /// The session private key used for inter-trampoline outer onions. + pub session_priv: SecretKey, + } + /// Tracks the inbound corresponding to an outbound HTLC - #[allow(clippy::derive_hash_xor_eq)] // Our Hash is faithful to the data, we just don't have SecretKey::hash + #[allow(clippy::derive_hash_xor_eq, dead_code)] // Our Hash is faithful to the data, we just don't have SecretKey::hash #[derive(Clone, Debug, PartialEq, Eq)] pub enum HTLCSource { PreviousHopData(HTLCPreviousHopData), + TrampolineForward { + /// We might be forwarding an incoming payment that was received over MPP, and therefore + /// need to store the vector of corresponding `HTLCPreviousHopData` values. + previous_hop_data: Vec, + incoming_trampoline_shared_secret: [u8; 32], + /// Track outbound payment details once the payment has been dispatched, will be `None` + /// when waiting for incoming MPP to accumulate. + outbound_payment: Option, + }, OutboundRoute { path: Path, session_priv: SecretKey, @@ -856,6 +887,20 @@ impl core::hash::Hash for HTLCSource { first_hop_htlc_msat.hash(hasher); bolt12_invoice.hash(hasher); }, + HTLCSource::TrampolineForward { + previous_hop_data, + incoming_trampoline_shared_secret, + outbound_payment, + } => { + 2u8.hash(hasher); + previous_hop_data.hash(hasher); + incoming_trampoline_shared_secret.hash(hasher); + if let Some(payment) = outbound_payment { + payment.payment_id.hash(hasher); + payment.path.hash(hasher); + payment.session_priv[..].hash(hasher); + } + }, } } } @@ -9029,6 +9074,7 @@ impl< None, )); }, + HTLCSource::TrampolineForward { .. } => todo!(), } } @@ -9783,6 +9829,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, ); }, + HTLCSource::TrampolineForward { .. } => todo!(), } } @@ -17271,6 +17318,8 @@ impl Readable for HTLCSource { }) } 1 => Ok(HTLCSource::PreviousHopData(Readable::read(reader)?)), + // Note: we intentionally do not read HTLCSource::TrampolineForward because we do not + // want to allow downgrades with in-flight trampoline forwards. _ => Err(DecodeError::UnknownRequiredFeature), } } @@ -17303,6 +17352,18 @@ impl Writeable for HTLCSource { 1u8.write(writer)?; field.write(writer)?; }, + HTLCSource::TrampolineForward { + ref previous_hop_data, + incoming_trampoline_shared_secret, + ref outbound_payment, + } => { + 2u8.write(writer)?; + write_tlv_fields!(writer, { + (1, *previous_hop_data, required_vec), + (3, incoming_trampoline_shared_secret, required), + (5, outbound_payment, option), + }); + }, } Ok(()) } @@ -17320,6 +17381,12 @@ impl_writeable_tlv_based!(PendingAddHTLCInfo, { (9, prev_counterparty_node_id, required), }); +impl_writeable_tlv_based!(TrampolineDispatch, { + (1, payment_id, required), + (3, path, required), + (5, session_priv, required), +}); + impl Writeable for HTLCForwardInfo { fn write(&self, w: &mut W) -> Result<(), io::Error> { const FAIL_HTLC_VARIANT_ID: u8 = 1; @@ -19176,6 +19243,7 @@ impl< } else { true } }); }, + HTLCSource::TrampolineForward { .. } => todo!(), HTLCSource::OutboundRoute { payment_id, session_priv, diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs index 90697ad246e..874ea12ed9c 100644 --- a/lightning/src/routing/router.rs +++ b/lightning/src/routing/router.rs @@ -656,6 +656,11 @@ impl Path { } } +impl_writeable_tlv_based!(Path,{ + (1, hops, required_vec), + (3, blinded_tail, option), +}); + /// A route directs a payment from the sender (us) to the recipient. If the recipient supports MPP, /// it can take multiple paths. Each path is composed of one or more hops through the network. #[derive(Clone, Debug, Hash, PartialEq, Eq)] From 38e4ee278eea2ea3a3f539211c5757dc8ead0ab9 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 11 Feb 2026 09:48:24 +0200 Subject: [PATCH 15/32] ln: add failure_type helper to HTLCSource for HTLCHandlingFailureType To create the right handling type based on source, add a helper. This is mainly useful for PreviousHopData/TrampolineForward. This helper maps an OutboundRoute to a HTLCHandlingFailureType::Forward. This value isn't actually used once we reach `forward_htlc_backwards_internal`, because we don't emit `HTLCHandlingFailed` events for our own payments. This issue is pre-existing, and could be addressed with an API change to the failure function, which is left out of scope of this work. --- lightning/src/ln/channelmanager.rs | 91 ++++++++++++++++++------------ 1 file changed, 56 insertions(+), 35 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 053f8feddfe..984dffca007 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -839,6 +839,26 @@ mod fuzzy_channelmanager { }, } + impl HTLCSource { + pub fn failure_type( + &self, counterparty_node: PublicKey, channel_id: ChannelId, + ) -> HTLCHandlingFailureType { + match self { + // We won't actually emit an event with HTLCHandlingFailure if our source is an + // OutboundRoute, but `fail_htlc_backwards_internal` requires that we provide it. + HTLCSource::PreviousHopData(_) | HTLCSource::OutboundRoute { .. } => { + HTLCHandlingFailureType::Forward { + node_id: Some(counterparty_node), + channel_id, + } + }, + HTLCSource::TrampolineForward { .. } => { + HTLCHandlingFailureType::TrampolineForward {} + }, + } + } + } + /// Tracks the inbound corresponding to an outbound HTLC #[derive(Clone, Debug, Hash, PartialEq, Eq)] pub struct HTLCPreviousHopData { @@ -4043,11 +4063,8 @@ impl< for htlc_source in failed_htlcs.drain(..) { let failure_reason = LocalHTLCFailureReason::ChannelClosed; let reason = HTLCFailReason::from_failure_code(failure_reason); - let receiver = HTLCHandlingFailureType::Forward { - node_id: Some(*counterparty_node_id), - channel_id: *chan_id, - }; let (source, hash) = htlc_source; + let receiver = source.failure_type(*counterparty_node_id, *chan_id); self.fail_htlc_backwards_internal(&source, &hash, &reason, receiver, None); } @@ -4210,10 +4227,7 @@ impl< let (source, payment_hash, counterparty_node_id, channel_id) = htlc_source; let failure_reason = LocalHTLCFailureReason::ChannelClosed; let reason = HTLCFailReason::from_failure_code(failure_reason); - let receiver = HTLCHandlingFailureType::Forward { - node_id: Some(counterparty_node_id), - channel_id, - }; + let receiver = source.failure_type(counterparty_node_id, channel_id); self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver, None); } if let Some((_, funding_txo, _channel_id, monitor_update)) = shutdown_res.monitor_update { @@ -7657,6 +7671,8 @@ impl< }; failed_forwards.push(( + // This can't be a trampoline payment because we don't process them + // as forwards (we're the last/"receiving" onion node). HTLCSource::PreviousHopData(prev_hop), payment_hash, HTLCFailReason::reason(reason, err_data), @@ -8935,11 +8951,14 @@ impl< for (htlc_src, payment_hash) in htlcs_to_fail.drain(..) { let reason = HTLCFailReason::reason(failure_reason, onion_failure_data.clone()); - let receiver = HTLCHandlingFailureType::Forward { - node_id: Some(counterparty_node_id.clone()), - channel_id, - }; - self.fail_htlc_backwards_internal(&htlc_src, &payment_hash, &reason, receiver, None); + let failure_type = htlc_src.failure_type(*counterparty_node_id, channel_id); + self.fail_htlc_backwards_internal( + &htlc_src, + &payment_hash, + &reason, + failure_type, + None, + ); } } @@ -9910,11 +9929,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } self.finalize_claims(finalized_claimed_htlcs); for failure in failed_htlcs { - let receiver = HTLCHandlingFailureType::Forward { - node_id: Some(counterparty_node_id), - channel_id, - }; - self.fail_htlc_backwards_internal(&failure.0, &failure.1, &failure.2, receiver, None); + let failure_type = failure.0.failure_type(counterparty_node_id, channel_id); + self.fail_htlc_backwards_internal( + &failure.0, + &failure.1, + &failure.2, + failure_type, + None, + ); } self.prune_persisted_inbound_htlc_onions( channel_id, @@ -12062,13 +12084,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } for htlc_source in dropped_htlcs.drain(..) { - let receiver = HTLCHandlingFailureType::Forward { - node_id: Some(counterparty_node_id.clone()), - channel_id: msg.channel_id, - }; - let reason = HTLCFailReason::from_failure_code(LocalHTLCFailureReason::ChannelClosed); let (source, hash) = htlc_source; - self.fail_htlc_backwards_internal(&source, &hash, &reason, receiver, None); + let failure_type = source.failure_type(*counterparty_node_id, msg.channel_id); + let reason = HTLCFailReason::from_failure_code(LocalHTLCFailureReason::ChannelClosed); + self.fail_htlc_backwards_internal(&source, &hash, &reason, failure_type, None); } Ok(()) @@ -13111,10 +13130,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } else { log_trace!(logger, "Failing HTLC from our monitor"); let failure_reason = LocalHTLCFailureReason::OnChainTimeout; - let receiver = HTLCHandlingFailureType::Forward { - node_id: Some(counterparty_node_id), - channel_id, - }; + let failure_type = + htlc_update.source.failure_type(counterparty_node_id, channel_id); let reason = HTLCFailReason::from_failure_code(failure_reason); let completion_update = Some(PaymentCompleteUpdate { counterparty_node_id, @@ -13126,7 +13143,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &htlc_update.source, &htlc_update.payment_hash, &reason, - receiver, + failure_type, completion_update, ); } @@ -15579,8 +15596,8 @@ impl< for (source, payment_hash) in timed_out_pending_htlcs.drain(..) { let reason = LocalHTLCFailureReason::CLTVExpiryTooSoon; let data = self.get_htlc_inbound_temp_fail_data(reason); - timed_out_htlcs.push((source, payment_hash, HTLCFailReason::reason(reason, data), - HTLCHandlingFailureType::Forward { node_id: Some(funded_channel.context.get_counterparty_node_id()), channel_id: *channel_id })); + let failure_type = source.failure_type(funded_channel.context.get_counterparty_node_id(), *channel_id); + timed_out_htlcs.push((source, payment_hash, HTLCFailReason::reason(reason, data), failure_type)); } let logger = WithChannelContext::from(&self.logger, &funded_channel.context, None); match funding_confirmed_opt { @@ -20080,11 +20097,15 @@ impl< for htlc_source in failed_htlcs { let (source, hash, counterparty_id, channel_id, failure_reason, ev_action) = htlc_source; - let receiver = - HTLCHandlingFailureType::Forward { node_id: Some(counterparty_id), channel_id }; + let failure_type = source.failure_type(counterparty_id, channel_id); let reason = HTLCFailReason::from_failure_code(failure_reason); - channel_manager - .fail_htlc_backwards_internal(&source, &hash, &reason, receiver, ev_action); + channel_manager.fail_htlc_backwards_internal( + &source, + &hash, + &reason, + failure_type, + ev_action, + ); } for ((_, hash), htlcs) in already_forwarded_htlcs.into_iter() { for (htlc, _) in htlcs { From 3b62b3482d49abfe463dcdfd6ed7ffae8cb2f772 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 2 Mar 2026 10:01:02 +0200 Subject: [PATCH 16/32] f Use failure_type var name throughout for HTLCHandlingFailureType --- lightning/src/ln/channelmanager.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 984dffca007..9d71df017b1 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4064,8 +4064,8 @@ impl< let failure_reason = LocalHTLCFailureReason::ChannelClosed; let reason = HTLCFailReason::from_failure_code(failure_reason); let (source, hash) = htlc_source; - let receiver = source.failure_type(*counterparty_node_id, *chan_id); - self.fail_htlc_backwards_internal(&source, &hash, &reason, receiver, None); + let failure_type = source.failure_type(*counterparty_node_id, *chan_id); + self.fail_htlc_backwards_internal(&source, &hash, &reason, failure_type, None); } let _ = self.handle_error(shutdown_result, *counterparty_node_id); @@ -4227,8 +4227,8 @@ impl< let (source, payment_hash, counterparty_node_id, channel_id) = htlc_source; let failure_reason = LocalHTLCFailureReason::ChannelClosed; let reason = HTLCFailReason::from_failure_code(failure_reason); - let receiver = source.failure_type(counterparty_node_id, channel_id); - self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver, None); + let failure_type = source.failure_type(counterparty_node_id, channel_id); + self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, failure_type, None); } if let Some((_, funding_txo, _channel_id, monitor_update)) = shutdown_res.monitor_update { debug_assert!(false, "This should have been handled in `convert_channel_err`"); From 335b43823f355ff0a09bdd17a88d1ed3e88cf1f0 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 2 Mar 2026 10:07:18 +0200 Subject: [PATCH 17/32] f add assert that we only handle regular forwards in channel not found --- lightning/src/ln/channelmanager.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9d71df017b1..69cf85a1443 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7783,6 +7783,10 @@ impl< continue; } } else { + debug_assert!( + false, + "We only expect to handle regular forwards in forwarding_channel_not_found" + ); let msg = format!("Unknown short channel id {} for forward HTLC", short_chan_id); failure_handler( From 2b4bca7bd2fd83d9393181ec9d9ee79508d2e466 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Tue, 16 Dec 2025 15:21:57 +0200 Subject: [PATCH 18/32] ln/refactor: add claim funds for htlc forward helper Will need to share this code when we add trampoline forwarding. This commit exactly moves the logic as-is, in preparation for the next commit that will update to suit trampoline. Co-authored-by: Arik Sosman Co-authored-by: Maurice Poirrier --- lightning/src/ln/channelmanager.rs | 292 ++++++++++++++++------------- 1 file changed, 159 insertions(+), 133 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 69cf85a1443..90393736daf 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9321,6 +9321,153 @@ impl< } } + /// Claims funds for a forwarded HTLC where we are an intermediate hop. + /// + /// Processes attribution data, calculates fees earned, and emits a [`Event::PaymentForwarded`] + /// event upon successful claim. + fn claim_funds_from_htlc_forward_hop( + &self, payment_preimage: PaymentPreimage, forwarded_htlc_value_msat: Option, + skimmed_fee_msat: Option, from_onchain: bool, startup_replay: bool, + next_channel_counterparty_node_id: PublicKey, next_channel_outpoint: OutPoint, + next_channel_id: ChannelId, next_user_channel_id: Option, + hop_data: HTLCPreviousHopData, attribution_data: Option, + send_timestamp: Option, + ) { + let prev_channel_id = hop_data.channel_id; + let prev_user_channel_id = hop_data.user_channel_id; + let prev_node_id = hop_data.counterparty_node_id; + let completed_blocker = RAAMonitorUpdateBlockingAction::from_prev_hop_data(&hop_data); + + // Obtain hold time, if available. + let hold_time = hold_time_since(send_timestamp).unwrap_or(0); + + // If attribution data was received from downstream, we shift it and get it ready for adding our hold + // time. Note that fulfilled HTLCs take a fast path to the incoming side. We don't need to wait for RAA + // to record the hold time like we do for failed HTLCs. + let attribution_data = process_fulfill_attribution_data( + attribution_data, + &hop_data.incoming_packet_shared_secret, + hold_time, + ); + + #[cfg(test)] + let claiming_chan_funding_outpoint = hop_data.outpoint; + self.claim_funds_from_hop( + hop_data, + payment_preimage, + None, + Some(attribution_data), + |htlc_claim_value_msat, definitely_duplicate| { + let chan_to_release = EventUnblockedChannel { + counterparty_node_id: next_channel_counterparty_node_id, + funding_txo: next_channel_outpoint, + channel_id: next_channel_id, + blocking_action: completed_blocker, + }; + + if definitely_duplicate && startup_replay { + // On startup we may get redundant claims which are related to + // monitor updates still in flight. In that case, we shouldn't + // immediately free, but instead let that monitor update complete + // in the background. + #[cfg(test)] + { + let per_peer_state = self.per_peer_state.deadlocking_read(); + // The channel we'd unblock should already be closed, or... + let channel_closed = per_peer_state + .get(&next_channel_counterparty_node_id) + .map(|lck| lck.deadlocking_lock()) + .map(|peer| !peer.channel_by_id.contains_key(&next_channel_id)) + .unwrap_or(true); + let background_events = self.pending_background_events.lock().unwrap(); + // there should be a `BackgroundEvent` pending... + let matching_bg_event = + background_events.iter().any(|ev| { + match ev { + // to apply a monitor update that blocked the claiming channel, + BackgroundEvent::MonitorUpdateRegeneratedOnStartup { + funding_txo, + update, + .. + } => { + if *funding_txo == claiming_chan_funding_outpoint { + assert!( + update.updates.iter().any(|upd| { + if let ChannelMonitorUpdateStep::PaymentPreimage { + payment_preimage: update_preimage, .. + } = upd { + payment_preimage == *update_preimage + } else { false } + }), + "{:?}", + update + ); + true + } else { + false + } + }, + // or the monitor update has completed and will unblock + // immediately once we get going. + BackgroundEvent::MonitorUpdatesComplete { + channel_id, .. + } => *channel_id == prev_channel_id, + } + }); + assert!(channel_closed || matching_bg_event, "{:?}", *background_events); + } + (None, None) + } else if definitely_duplicate { + ( + Some(MonitorUpdateCompletionAction::FreeDuplicateClaimImmediately { + downstream_counterparty_node_id: chan_to_release.counterparty_node_id, + downstream_channel_id: chan_to_release.channel_id, + blocking_action: chan_to_release.blocking_action, + }), + None, + ) + } else { + let total_fee_earned_msat = + if let Some(forwarded_htlc_value) = forwarded_htlc_value_msat { + if let Some(claimed_htlc_value) = htlc_claim_value_msat { + Some(claimed_htlc_value - forwarded_htlc_value) + } else { + None + } + } else { + None + }; + debug_assert!( + skimmed_fee_msat <= total_fee_earned_msat, + "skimmed_fee_msat must always be included in total_fee_earned_msat" + ); + ( + Some(MonitorUpdateCompletionAction::EmitEventOptionAndFreeOtherChannel { + event: Some(events::Event::PaymentForwarded { + prev_htlcs: vec![events::HTLCLocator { + channel_id: prev_channel_id, + user_channel_id: prev_user_channel_id, + node_id: prev_node_id, + }], + next_htlcs: vec![events::HTLCLocator { + channel_id: next_channel_id, + user_channel_id: next_user_channel_id, + node_id: Some(next_channel_counterparty_node_id), + }], + total_fee_earned_msat, + skimmed_fee_msat, + claim_from_onchain_tx: from_onchain, + outbound_amount_forwarded_msat: forwarded_htlc_value_msat, + }), + downstream_counterparty_and_funding_outpoint: chan_to_release, + }), + None, + ) + } + }, + ); + } + fn claim_funds_from_hop< ComplFunc: FnOnce( Option, @@ -9716,140 +9863,19 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } }, HTLCSource::PreviousHopData(hop_data) => { - let prev_channel_id = hop_data.channel_id; - let prev_user_channel_id = hop_data.user_channel_id; - let prev_node_id = hop_data.counterparty_node_id; - let completed_blocker = - RAAMonitorUpdateBlockingAction::from_prev_hop_data(&hop_data); - - // Obtain hold time, if available. - let hold_time = hold_time_since(send_timestamp).unwrap_or(0); - - // If attribution data was received from downstream, we shift it and get it ready for adding our hold - // time. Note that fulfilled HTLCs take a fast path to the incoming side. We don't need to wait for RAA - // to record the hold time like we do for failed HTLCs. - let attribution_data = process_fulfill_attribution_data( - attribution_data, - &hop_data.incoming_packet_shared_secret, - hold_time, - ); - - #[cfg(test)] - let claiming_chan_funding_outpoint = hop_data.outpoint; - self.claim_funds_from_hop( - hop_data, + self.claim_funds_from_htlc_forward_hop( payment_preimage, - None, - Some(attribution_data), - |htlc_claim_value_msat, definitely_duplicate| { - let chan_to_release = EventUnblockedChannel { - counterparty_node_id: next_channel_counterparty_node_id, - funding_txo: next_channel_outpoint, - channel_id: next_channel_id, - blocking_action: completed_blocker, - }; - - if definitely_duplicate && startup_replay { - // On startup we may get redundant claims which are related to - // monitor updates still in flight. In that case, we shouldn't - // immediately free, but instead let that monitor update complete - // in the background. - #[cfg(test)] - { - let per_peer_state = self.per_peer_state.deadlocking_read(); - // The channel we'd unblock should already be closed, or... - let channel_closed = per_peer_state - .get(&next_channel_counterparty_node_id) - .map(|lck| lck.deadlocking_lock()) - .map(|peer| !peer.channel_by_id.contains_key(&next_channel_id)) - .unwrap_or(true); - let background_events = - self.pending_background_events.lock().unwrap(); - // there should be a `BackgroundEvent` pending... - let matching_bg_event = - background_events.iter().any(|ev| { - match ev { - // to apply a monitor update that blocked the claiming channel, - BackgroundEvent::MonitorUpdateRegeneratedOnStartup { - funding_txo, update, .. - } => { - if *funding_txo == claiming_chan_funding_outpoint { - assert!(update.updates.iter().any(|upd| - if let ChannelMonitorUpdateStep::PaymentPreimage { - payment_preimage: update_preimage, .. - } = upd { - payment_preimage == *update_preimage - } else { false } - ), "{:?}", update); - true - } else { false } - }, - // or the monitor update has completed and will unblock - // immediately once we get going. - BackgroundEvent::MonitorUpdatesComplete { - channel_id, .. - } => - *channel_id == prev_channel_id, - } - }); - assert!( - channel_closed || matching_bg_event, - "{:?}", - *background_events - ); - } - (None, None) - } else if definitely_duplicate { - ( - Some( - MonitorUpdateCompletionAction::FreeDuplicateClaimImmediately { - downstream_counterparty_node_id: chan_to_release - .counterparty_node_id, - downstream_channel_id: chan_to_release.channel_id, - blocking_action: chan_to_release.blocking_action, - }, - ), - None, - ) - } else { - let total_fee_earned_msat = - if let Some(forwarded_htlc_value) = forwarded_htlc_value_msat { - if let Some(claimed_htlc_value) = htlc_claim_value_msat { - Some(claimed_htlc_value - forwarded_htlc_value) - } else { - None - } - } else { - None - }; - debug_assert!( - skimmed_fee_msat <= total_fee_earned_msat, - "skimmed_fee_msat must always be included in total_fee_earned_msat" - ); - ( - Some(MonitorUpdateCompletionAction::EmitEventOptionAndFreeOtherChannel { - event: Some(events::Event::PaymentForwarded { - prev_htlcs: vec![events::HTLCLocator { - channel_id: prev_channel_id, - user_channel_id: prev_user_channel_id, - node_id: prev_node_id, - }], - next_htlcs: vec![events::HTLCLocator { - channel_id: next_channel_id, - user_channel_id: next_user_channel_id, - node_id: Some(next_channel_counterparty_node_id), - }], - total_fee_earned_msat, - skimmed_fee_msat, - claim_from_onchain_tx: from_onchain, - outbound_amount_forwarded_msat: forwarded_htlc_value_msat, - }), - downstream_counterparty_and_funding_outpoint: chan_to_release, - }), - None, - ) - } - }, + forwarded_htlc_value_msat, + skimmed_fee_msat, + from_onchain, + startup_replay, + next_channel_counterparty_node_id, + next_channel_outpoint, + next_channel_id, + next_user_channel_id, + hop_data, + attribution_data, + send_timestamp, ); }, HTLCSource::TrampolineForward { .. } => todo!(), From 89ed28a756882530682f3460893bfd1e67dea0c8 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Tue, 6 Jan 2026 09:09:06 -0500 Subject: [PATCH 19/32] ln/refactor: pass closure to create PaymentForwarded event When we introduce trampoline forwards, we're going to want to provide two external pieces of information to create events: - When to emit an event: we only want to emit one trampoline event, even when we have multiple incoming htlcs. We need to make multiple calls to claim_funds_from_htlc_forward_hop to claim each individual htlc, which are not aware of each other, so we rely on the caller's closure to decide when to emit Some or None. - Forwarding fees: we will not be able to calculate the total fee for a trampoline forward when an individual outgoing htlcs is fulfilled, because there may be other outgoing htlcs that are not accounted for (we only get the htlc_claim_value_msat for the single htlc that was just fulfilled). In future, we'll be able to provide the total fee from the channelmanager's top level view. --- lightning/src/ln/channelmanager.rs | 102 ++++++++++++++++------------- 1 file changed, 57 insertions(+), 45 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 90393736daf..56b85345bb1 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -879,6 +879,16 @@ mod fuzzy_channelmanager { /// channel remains unconfirmed for too long. pub cltv_expiry: Option, } + + impl From<&HTLCPreviousHopData> for events::HTLCLocator { + fn from(value: &HTLCPreviousHopData) -> Self { + events::HTLCLocator { + channel_id: value.channel_id, + user_channel_id: value.user_channel_id, + node_id: value.counterparty_node_id, + } + } + } } #[cfg(fuzzing)] pub use self::fuzzy_channelmanager::*; @@ -9324,18 +9334,16 @@ impl< /// Claims funds for a forwarded HTLC where we are an intermediate hop. /// /// Processes attribution data, calculates fees earned, and emits a [`Event::PaymentForwarded`] - /// event upon successful claim. + /// event upon successful claim. `make_payment_forwarded_event` is responsible for creating a + /// single [`Event::PaymentForwarded`] event that represents the forward. fn claim_funds_from_htlc_forward_hop( - &self, payment_preimage: PaymentPreimage, forwarded_htlc_value_msat: Option, - skimmed_fee_msat: Option, from_onchain: bool, startup_replay: bool, - next_channel_counterparty_node_id: PublicKey, next_channel_outpoint: OutPoint, - next_channel_id: ChannelId, next_user_channel_id: Option, - hop_data: HTLCPreviousHopData, attribution_data: Option, - send_timestamp: Option, + &self, payment_preimage: PaymentPreimage, + make_payment_forwarded_event: impl Fn(Option) -> Option, + startup_replay: bool, next_channel_counterparty_node_id: PublicKey, + next_channel_outpoint: OutPoint, next_channel_id: ChannelId, hop_data: HTLCPreviousHopData, + attribution_data: Option, send_timestamp: Option, ) { - let prev_channel_id = hop_data.channel_id; - let prev_user_channel_id = hop_data.user_channel_id; - let prev_node_id = hop_data.counterparty_node_id; + let _prev_channel_id = hop_data.channel_id; let completed_blocker = RAAMonitorUpdateBlockingAction::from_prev_hop_data(&hop_data); // Obtain hold time, if available. @@ -9411,7 +9419,7 @@ impl< // immediately once we get going. BackgroundEvent::MonitorUpdatesComplete { channel_id, .. - } => *channel_id == prev_channel_id, + } => *channel_id == _prev_channel_id, } }); assert!(channel_closed || matching_bg_event, "{:?}", *background_events); @@ -9427,38 +9435,16 @@ impl< None, ) } else { - let total_fee_earned_msat = - if let Some(forwarded_htlc_value) = forwarded_htlc_value_msat { - if let Some(claimed_htlc_value) = htlc_claim_value_msat { - Some(claimed_htlc_value - forwarded_htlc_value) - } else { - None - } - } else { - None - }; - debug_assert!( - skimmed_fee_msat <= total_fee_earned_msat, - "skimmed_fee_msat must always be included in total_fee_earned_msat" - ); + let event = make_payment_forwarded_event(htlc_claim_value_msat); + if let Some(ref payment_forwarded) = event { + debug_assert!(matches!( + payment_forwarded, + &events::Event::PaymentForwarded { .. } + )); + } ( Some(MonitorUpdateCompletionAction::EmitEventOptionAndFreeOtherChannel { - event: Some(events::Event::PaymentForwarded { - prev_htlcs: vec![events::HTLCLocator { - channel_id: prev_channel_id, - user_channel_id: prev_user_channel_id, - node_id: prev_node_id, - }], - next_htlcs: vec![events::HTLCLocator { - channel_id: next_channel_id, - user_channel_id: next_user_channel_id, - node_id: Some(next_channel_counterparty_node_id), - }], - total_fee_earned_msat, - skimmed_fee_msat, - claim_from_onchain_tx: from_onchain, - outbound_amount_forwarded_msat: forwarded_htlc_value_msat, - }), + event, downstream_counterparty_and_funding_outpoint: chan_to_release, }), None, @@ -9863,16 +9849,42 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } }, HTLCSource::PreviousHopData(hop_data) => { + let prev_htlcs = vec![events::HTLCLocator::from(&hop_data)]; self.claim_funds_from_htlc_forward_hop( payment_preimage, - forwarded_htlc_value_msat, - skimmed_fee_msat, - from_onchain, + |htlc_claim_value_msat: Option| -> Option { + let total_fee_earned_msat = + if let Some(forwarded_htlc_value) = forwarded_htlc_value_msat { + if let Some(claimed_htlc_value) = htlc_claim_value_msat { + Some(claimed_htlc_value - forwarded_htlc_value) + } else { + None + } + } else { + None + }; + debug_assert!( + skimmed_fee_msat <= total_fee_earned_msat, + "skimmed_fee_msat must always be included in total_fee_earned_msat" + ); + + Some(events::Event::PaymentForwarded { + prev_htlcs: prev_htlcs.clone(), + next_htlcs: vec![events::HTLCLocator { + channel_id: next_channel_id, + user_channel_id: next_user_channel_id, + node_id: Some(next_channel_counterparty_node_id), + }], + total_fee_earned_msat, + skimmed_fee_msat, + claim_from_onchain_tx: from_onchain, + outbound_amount_forwarded_msat: forwarded_htlc_value_msat, + }) + }, startup_replay, next_channel_counterparty_node_id, next_channel_outpoint, next_channel_id, - next_user_channel_id, hop_data, attribution_data, send_timestamp, From e4edd8699c78d06021a87d8ea637626bd1f999f2 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 2 Mar 2026 10:30:10 +0200 Subject: [PATCH 20/32] f Use FnOnce to avoid clone --- lightning/src/ln/channelmanager.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 56b85345bb1..d8661f4298d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9338,7 +9338,7 @@ impl< /// single [`Event::PaymentForwarded`] event that represents the forward. fn claim_funds_from_htlc_forward_hop( &self, payment_preimage: PaymentPreimage, - make_payment_forwarded_event: impl Fn(Option) -> Option, + make_payment_forwarded_event: impl FnOnce(Option) -> Option, startup_replay: bool, next_channel_counterparty_node_id: PublicKey, next_channel_outpoint: OutPoint, next_channel_id: ChannelId, hop_data: HTLCPreviousHopData, attribution_data: Option, send_timestamp: Option, @@ -9869,7 +9869,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ ); Some(events::Event::PaymentForwarded { - prev_htlcs: prev_htlcs.clone(), + prev_htlcs, next_htlcs: vec![events::HTLCLocator { channel_id: next_channel_id, user_channel_id: next_user_channel_id, From f2e28ef12e08ff046ba982311ea93d11d370a4d1 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Tue, 6 Jan 2026 09:42:37 -0500 Subject: [PATCH 21/32] ln: add trampoline routing payment claiming Implement payment claiming for `HTLCSource::TrampolineForward` by iterating through previous hop data and claiming funds for each HTLC. Co-authored-by: Arik Sosman Co-authored-by: Maurice Poirrier --- lightning/src/ln/channelmanager.rs | 42 +++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d8661f4298d..da5c3331d15 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9890,7 +9890,47 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ send_timestamp, ); }, - HTLCSource::TrampolineForward { .. } => todo!(), + HTLCSource::TrampolineForward { previous_hop_data, .. } => { + // Only emit a single event for trampoline claims. + let prev_htlcs: Vec = + previous_hop_data.iter().map(Into::into).collect(); + for (i, current_previous_hop_data) in previous_hop_data.into_iter().enumerate() { + self.claim_funds_from_htlc_forward_hop( + payment_preimage, + |_: Option| -> Option { + if i == 0 { + Some(events::Event::PaymentForwarded { + prev_htlcs: prev_htlcs.clone(), + // TODO: When trampoline payments are tracked in our + // pending_outbound_payments, we'll be able to provide all the + // outgoing htlcs for this forward. + next_htlcs: vec![events::HTLCLocator { + channel_id: next_channel_id, + user_channel_id: next_user_channel_id, + node_id: Some(next_channel_counterparty_node_id), + }], + // TODO: When trampoline payments are tracked in our + // pending_outbound_payments, we'll be able to lookup our total + // fee earnings. + total_fee_earned_msat: None, + skimmed_fee_msat, + claim_from_onchain_tx: from_onchain, + outbound_amount_forwarded_msat: forwarded_htlc_value_msat, + }) + } else { + None + } + }, + startup_replay, + next_channel_counterparty_node_id, + next_channel_outpoint, + next_channel_id, + current_previous_hop_data, + attribution_data.clone(), + send_timestamp, + ); + } + }, } } From ed92dac9d09ae3aab597de1fe963f346acfbd522 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 25 Feb 2026 11:59:45 +0200 Subject: [PATCH 22/32] f block on inbound claim for trampoline update_fulfill_htlc Similar to forwards, we need to block on the incoming channel storing our preimage to safely revoke on the outbound channel. --- lightning/src/ln/channelmanager.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index da5c3331d15..207488411de 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -12322,20 +12322,25 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ chan.update_fulfill_htlc(&msg), chan_entry ); - if let HTLCSource::PreviousHopData(prev_hop) = &res.0 { - let logger = - WithChannelContext::from(&self.logger, &chan.context, None); + let prev_hops = match &res.0 { + HTLCSource::PreviousHopData(prev_hop) => vec![prev_hop], + HTLCSource::TrampolineForward { previous_hop_data, .. } => { + previous_hop_data.iter().collect() + }, + _ => vec![], + }; + let logger = WithChannelContext::from(&self.logger, &chan.context, None); + for prev_hop in prev_hops { log_trace!(logger, "Holding the next revoke_and_ack until the preimage is durably persisted in the inbound edge's ChannelMonitor", - ); + ); peer_state .actions_blocking_raa_monitor_updates .entry(msg.channel_id) .or_insert_with(Vec::new) - .push(RAAMonitorUpdateBlockingAction::from_prev_hop_data( - &prev_hop, - )); + .push(RAAMonitorUpdateBlockingAction::from_prev_hop_data(prev_hop)); } + // Note that we do not need to push an `actions_blocking_raa_monitor_updates` // entry here, even though we *do* need to block the next RAA monitor update. // We do this instead in the `claim_funds_internal` by attaching a From 8d0590f79ee15c203038a726fb9568bc7f72e8af Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Tue, 3 Mar 2026 11:33:03 +0200 Subject: [PATCH 23/32] f Add note on forwarded_htlc_value_msat --- lightning/src/ln/channelmanager.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 207488411de..1bc4dd0cbab 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9915,6 +9915,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ total_fee_earned_msat: None, skimmed_fee_msat, claim_from_onchain_tx: from_onchain, + // TODO: When trampoline payments are tracked in our + // pending_outbound_payments, set to the total amount sent (not + // just the amount of the outgoing htlc that was first settled). outbound_amount_forwarded_msat: forwarded_htlc_value_msat, }) } else { From 9e30218ee98792c0e2bf0ba5d5f570c7ba9f8aa1 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Thu, 20 Nov 2025 11:04:59 -0500 Subject: [PATCH 24/32] ln/refactor: add blinded forwarding failure helper function We'll want this extracted when we need to handle trampoline and regular forwards. Co-authored-by: Arik Sosman Co-authored-by: Maurice Poirrier --- lightning/src/ln/channelmanager.rs | 100 ++++++++++++++++++----------- 1 file changed, 62 insertions(+), 38 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 1bc4dd0cbab..81b5bd12bc1 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8992,6 +8992,19 @@ impl< debug_assert_ne!(peer.held_by_thread(), LockHeldState::HeldByThread); } + let push_forward_htlcs_failure = + |prev_outbound_scid_alias: u64, failure: HTLCForwardInfo| { + let mut forward_htlcs = self.forward_htlcs.lock().unwrap(); + match forward_htlcs.entry(prev_outbound_scid_alias) { + hash_map::Entry::Occupied(mut entry) => { + entry.get_mut().push(failure); + }, + hash_map::Entry::Vacant(entry) => { + entry.insert(vec![failure]); + }, + } + }; + //TODO: There is a timing attack here where if a node fails an HTLC back to us they can //identify whether we sent it or not based on the (I presume) very different runtime //between the branches here. We should make this async and move it into the forward HTLCs @@ -9058,45 +9071,19 @@ impl< if blinded_failure.is_some() { "blinded " } else { "" }, onion_error ); - // In case of trampoline + phantom we prioritize the trampoline failure over the phantom failure. - // TODO: Correctly wrap the error packet twice if failing back a trampoline + phantom HTLC. - let secondary_shared_secret = trampoline_shared_secret.or(*phantom_shared_secret); - let failure = match blinded_failure { - Some(BlindedFailure::FromIntroductionNode) => { - let blinded_onion_error = HTLCFailReason::reason( - LocalHTLCFailureReason::InvalidOnionBlinding, - vec![0; 32], - ); - let err_packet = blinded_onion_error.get_encrypted_failure_packet( - incoming_packet_shared_secret, - &secondary_shared_secret, - ); - HTLCForwardInfo::FailHTLC { htlc_id: *htlc_id, err_packet } - }, - Some(BlindedFailure::FromBlindedNode) => HTLCForwardInfo::FailMalformedHTLC { - htlc_id: *htlc_id, - failure_code: LocalHTLCFailureReason::InvalidOnionBlinding.failure_code(), - sha256_of_onion: [0; 32], - }, - None => { - let err_packet = onion_error.get_encrypted_failure_packet( - incoming_packet_shared_secret, - &secondary_shared_secret, - ); - HTLCForwardInfo::FailHTLC { htlc_id: *htlc_id, err_packet } - }, - }; - let mut forward_htlcs = self.forward_htlcs.lock().unwrap(); - match forward_htlcs.entry(*prev_outbound_scid_alias) { - hash_map::Entry::Occupied(mut entry) => { - entry.get_mut().push(failure); - }, - hash_map::Entry::Vacant(entry) => { - entry.insert(vec![failure]); - }, - } - mem::drop(forward_htlcs); + push_forward_htlcs_failure( + *prev_outbound_scid_alias, + get_htlc_forward_failure( + blinded_failure, + onion_error, + incoming_packet_shared_secret, + trampoline_shared_secret, + phantom_shared_secret, + *htlc_id, + ), + ); + let mut pending_events = self.pending_events.lock().unwrap(); pending_events.push_back(( events::Event::HTLCHandlingFailed { @@ -13873,6 +13860,43 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } +/// Constructs an HTLC forward failure for sending back to the previous hop, converting to a blinded +/// failure where appropriate. +/// +/// When both trampoline and phantom secrets are present, the trampoline secret takes priority +/// for error encryption. +fn get_htlc_forward_failure( + blinded_failure: &Option, onion_error: &HTLCFailReason, + incoming_packet_shared_secret: &[u8; 32], trampoline_shared_secret: &Option<[u8; 32]>, + phantom_shared_secret: &Option<[u8; 32]>, htlc_id: u64, +) -> HTLCForwardInfo { + // TODO: Correctly wrap the error packet twice if failing back a trampoline + phantom HTLC. + let secondary_shared_secret = trampoline_shared_secret.or(*phantom_shared_secret); + match blinded_failure { + Some(BlindedFailure::FromIntroductionNode) => { + let blinded_onion_error = + HTLCFailReason::reason(LocalHTLCFailureReason::InvalidOnionBlinding, vec![0; 32]); + let err_packet = blinded_onion_error.get_encrypted_failure_packet( + incoming_packet_shared_secret, + &secondary_shared_secret, + ); + HTLCForwardInfo::FailHTLC { htlc_id, err_packet } + }, + Some(BlindedFailure::FromBlindedNode) => HTLCForwardInfo::FailMalformedHTLC { + htlc_id, + failure_code: LocalHTLCFailureReason::InvalidOnionBlinding.failure_code(), + sha256_of_onion: [0; 32], + }, + None => { + let err_packet = onion_error.get_encrypted_failure_packet( + incoming_packet_shared_secret, + &secondary_shared_secret, + ); + HTLCForwardInfo::FailHTLC { htlc_id, err_packet } + }, + } +} + /// Parameters used with [`create_bolt11_invoice`]. /// /// [`create_bolt11_invoice`]: ChannelManager::create_bolt11_invoice From ebe48e4acff6b1742891d7f39b61f49533b8fe6a Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 1 Dec 2025 15:50:46 -0500 Subject: [PATCH 25/32] ln: add trampoline routing failure handling Implement failure propagation for `HTLCSource::TrampolineForward` by iterating through previous hop data and failing each HTLC with `TemporaryTrampolineFailure`. Note that testing should be implemented when trampoline forward is completed. Co-authored-by: Arik Sosman Co-authored-by: Maurice Poirrier --- lightning/src/ln/channelmanager.rs | 69 +++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 81b5bd12bc1..5ac5c0d61ee 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9094,7 +9094,74 @@ impl< None, )); }, - HTLCSource::TrampolineForward { .. } => todo!(), + HTLCSource::TrampolineForward { + previous_hop_data, + incoming_trampoline_shared_secret, + .. + } => { + let decoded_onion_failure = + onion_error.decode_onion_failure(&self.secp_ctx, &self.logger, &source); + log_trace!( + WithContext::from(&self.logger, None, None, Some(*payment_hash)), + "Trampoline forward failed downstream on {}", + if let Some(scid) = decoded_onion_failure.short_channel_id { + scid.to_string() + } else { + "unknown channel".to_string() + }, + ); + let incoming_trampoline_shared_secret = Some(*incoming_trampoline_shared_secret); + + // TODO: when we receive a failure from a single outgoing trampoline HTLC, we don't + // necessarily want to fail all of our incoming HTLCs back yet. We may have other + // outgoing HTLCs that need to resolve first. This will be tracked in our + // pending_outbound_payments in a followup. + for current_hop_data in previous_hop_data { + let HTLCPreviousHopData { + prev_outbound_scid_alias, + htlc_id, + incoming_packet_shared_secret, + blinded_failure, + channel_id, + .. + } = current_hop_data; + log_trace!( + WithContext::from(&self.logger, None, Some(*channel_id), Some(*payment_hash)), + "Failing {}HTLC with payment_hash {} backwards from us following Trampoline forwarding failure: {:?}", + if blinded_failure.is_some() { "blinded " } else { "" }, &payment_hash, onion_error + ); + let onion_error = HTLCFailReason::reason( + LocalHTLCFailureReason::TemporaryTrampolineFailure, + Vec::new(), + ); + push_forward_htlcs_failure( + *prev_outbound_scid_alias, + get_htlc_forward_failure( + blinded_failure, + &onion_error, + incoming_packet_shared_secret, + &incoming_trampoline_shared_secret, + &None, + *htlc_id, + ), + ); + } + + // We only want to emit a single event for trampoline failures, so we do it once + // we've failed back all of our incoming HTLCs. + let mut pending_events = self.pending_events.lock().unwrap(); + pending_events.push_back(( + events::Event::HTLCHandlingFailed { + prev_channel_ids: previous_hop_data + .iter() + .map(|prev| prev.channel_id) + .collect(), + failure_type, + failure_reason: Some(onion_error.into()), + }, + None, + )); + }, } } From 3422b42fc106e092a597e4bbfe076c520edf1144 Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 25 Aug 2025 15:33:44 -0400 Subject: [PATCH 26/32] ln/refactor: extract channelmonitor recovery to external helper Move recovery logic for `HTLCSource::PreviousHopData` into `channel_monitor_recovery_internal` to prepare for trampoline forward reuse. Co-authored-by: Arik Sosman Co-authored-by: Maurice Poirrier --- lightning/src/ln/channelmanager.rs | 156 ++++++++++++++++++----------- 1 file changed, 99 insertions(+), 57 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5ac5c0d61ee..9dd985021e6 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -19381,65 +19381,19 @@ impl< let htlc_id = SentHTLCId::from_source(&htlc_source); match htlc_source { HTLCSource::PreviousHopData(prev_hop_data) => { - let pending_forward_matches_htlc = |info: &PendingAddHTLCInfo| { - info.prev_funding_outpoint == prev_hop_data.outpoint - && info.prev_htlc_id == prev_hop_data.htlc_id - }; - - // If `reconstruct_manager_from_monitors` is set, we always add all inbound committed - // HTLCs to `decode_update_add_htlcs` in the above loop, but we need to prune from - // those added HTLCs if they were already forwarded to the outbound edge. Otherwise, - // we'll double-forward. - if reconstruct_manager_from_monitors { - dedup_decode_update_add_htlcs( - &mut decode_update_add_htlcs, - &prev_hop_data, - "HTLC already forwarded to the outbound edge", - &&logger, - ); - prune_forwarded_htlc( - &mut already_forwarded_htlcs, - &prev_hop_data, - &htlc.payment_hash, - ); - } - - // The ChannelMonitor is now responsible for this HTLC's - // failure/success and will let us know what its outcome is. If we - // still have an entry for this HTLC in `forward_htlcs_legacy`, - // `pending_intercepted_htlcs_legacy`, or - // `decode_update_add_htlcs_legacy`, we were apparently not persisted - // after the monitor was when forwarding the payment. - dedup_decode_update_add_htlcs( + reconcile_pending_htlcs_with_monitor( + reconstruct_manager_from_monitors, + &mut already_forwarded_htlcs, + &mut forward_htlcs_legacy, + &mut pending_events_read, + &mut pending_intercepted_htlcs_legacy, + &mut decode_update_add_htlcs, &mut decode_update_add_htlcs_legacy, - &prev_hop_data, - "HTLC was forwarded to the closed channel", - &&logger, + prev_hop_data, + &logger, + htlc.payment_hash, + monitor.channel_id(), ); - forward_htlcs_legacy.retain(|_, forwards| { - forwards.retain(|forward| { - if let HTLCForwardInfo::AddHTLC(htlc_info) = forward { - if pending_forward_matches_htlc(&htlc_info) { - log_info!(logger, "Removing pending to-forward HTLC with hash {} as it was forwarded to the closed channel {}", - &htlc.payment_hash, &monitor.channel_id()); - false - } else { true } - } else { true } - }); - !forwards.is_empty() - }); - pending_intercepted_htlcs_legacy.retain(|intercepted_id, htlc_info| { - if pending_forward_matches_htlc(&htlc_info) { - log_info!(logger, "Removing pending intercepted HTLC with hash {} as it was forwarded to the closed channel {}", - &htlc.payment_hash, &monitor.channel_id()); - pending_events_read.retain(|(event, _)| { - if let Event::HTLCIntercepted { intercept_id: ev_id, .. } = event { - intercepted_id != ev_id - } else { true } - }); - false - } else { true } - }); }, HTLCSource::TrampolineForward { .. } => todo!(), HTLCSource::OutboundRoute { @@ -20341,6 +20295,94 @@ impl< } } +fn prune_forwarded_htlc( + already_forwarded_htlcs: &mut HashMap< + (ChannelId, PaymentHash), + Vec<(HTLCPreviousHopData, OutboundHop)>, + >, + prev_hop: &HTLCPreviousHopData, payment_hash: &PaymentHash, +) { + if let hash_map::Entry::Occupied(mut entry) = + already_forwarded_htlcs.entry((prev_hop.channel_id, *payment_hash)) + { + entry.get_mut().retain(|(htlc, _)| prev_hop.htlc_id != htlc.htlc_id); + if entry.get().is_empty() { + entry.remove(); + } + } +} + +/// Removes pending HTLC entries that the ChannelMonitor has already taken responsibility for, +/// cleaning up state mismatches that can occur during restart. +fn reconcile_pending_htlcs_with_monitor( + reconstruct_manager_from_monitors: bool, + already_forwarded_htlcs: &mut HashMap< + (ChannelId, PaymentHash), + Vec<(HTLCPreviousHopData, OutboundHop)>, + >, + forward_htlcs_legacy: &mut HashMap>, + pending_events_read: &mut VecDeque<(Event, Option)>, + pending_intercepted_htlcs_legacy: &mut HashMap, + decode_update_add_htlcs: &mut HashMap>, + decode_update_add_htlcs_legacy: &mut HashMap>, + prev_hop_data: HTLCPreviousHopData, logger: &impl Logger, payment_hash: PaymentHash, + channel_id: ChannelId, +) { + let pending_forward_matches_htlc = |info: &PendingAddHTLCInfo| { + info.prev_funding_outpoint == prev_hop_data.outpoint + && info.prev_htlc_id == prev_hop_data.htlc_id + }; + + // If `reconstruct_manager_from_monitors` is set, we always add all inbound committed + // HTLCs to `decode_update_add_htlcs` in the above loop, but we need to prune from + // those added HTLCs if they were already forwarded to the outbound edge. Otherwise, + // we'll double-forward. + if reconstruct_manager_from_monitors { + dedup_decode_update_add_htlcs( + decode_update_add_htlcs, + &prev_hop_data, + "HTLC already forwarded to the outbound edge", + &&logger, + ); + prune_forwarded_htlc(already_forwarded_htlcs, &prev_hop_data, &payment_hash); + } + + // The ChannelMonitor is now responsible for this HTLC's failure/success and will let us know + // what its outcome is. If we still have an entry for this HTLC in `forward_htlcs_legacy`, + // `pending_intercepted_htlcs_legacy`, or `decode_update_add_htlcs_legacy`, we were apparently + // not persisted after the monitor was when forwarding the payment. + dedup_decode_update_add_htlcs( + decode_update_add_htlcs_legacy, + &prev_hop_data, + "HTLC was forwarded to the closed channel", + &&logger, + ); + forward_htlcs_legacy.retain(|_, forwards| { + forwards.retain(|forward| { + if let HTLCForwardInfo::AddHTLC(htlc_info) = forward { + if pending_forward_matches_htlc(&htlc_info) { + log_info!(logger, "Removing pending to-forward HTLC with hash {} as it was forwarded to the closed channel {}", + &payment_hash, channel_id); + false + } else { true } + } else { true } + }); + !forwards.is_empty() + }); + pending_intercepted_htlcs_legacy.retain(|intercepted_id, htlc_info| { + if pending_forward_matches_htlc(&htlc_info) { + log_info!(logger, "Removing pending intercepted HTLC with hash {} as it was forwarded to the closed channel {}", + payment_hash, channel_id); + pending_events_read.retain(|(event, _)| { + if let Event::HTLCIntercepted { intercept_id: ev_id, .. } = event { + intercepted_id != ev_id + } else { true } + }); + false + } else { true } + }); +} + #[cfg(test)] mod tests { use crate::events::{ClosureReason, Event, HTLCHandlingFailureType}; From dcc2031506453e76db84acd78986344930957700 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 2 Mar 2026 10:34:27 +0200 Subject: [PATCH 27/32] f Remove duplicate prune_forwarded_htlc --- lightning/src/ln/channelmanager.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9dd985021e6..f0aaac0bade 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -19244,21 +19244,6 @@ impl< (ChannelId, PaymentHash), Vec<(HTLCPreviousHopData, OutboundHop)>, > = new_hash_map(); - let prune_forwarded_htlc = |already_forwarded_htlcs: &mut HashMap< - (ChannelId, PaymentHash), - Vec<(HTLCPreviousHopData, OutboundHop)>, - >, - prev_hop: &HTLCPreviousHopData, - payment_hash: &PaymentHash| { - if let hash_map::Entry::Occupied(mut entry) = - already_forwarded_htlcs.entry((prev_hop.channel_id, *payment_hash)) - { - entry.get_mut().retain(|(htlc, _)| prev_hop.htlc_id != htlc.htlc_id); - if entry.get().is_empty() { - entry.remove(); - } - } - }; { // If we're tracking pending payments, ensure we haven't lost any by looking at the // ChannelMonitor data for any channels for which we do not have authorative state From 0ea9cf5f450520f66428a970ddc618262c2bebc7 Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 25 Aug 2025 15:38:34 -0400 Subject: [PATCH 28/32] ln: add channel monitor recovery for trampoline forwards Implement channel monitor recovery for trampoline forwards iterating over all hop data and updating pending forwards. --- lightning/src/ln/channelmanager.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f0aaac0bade..260a9d0797d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -19380,7 +19380,23 @@ impl< monitor.channel_id(), ); }, - HTLCSource::TrampolineForward { .. } => todo!(), + HTLCSource::TrampolineForward { previous_hop_data, .. } => { + for prev_hop_data in previous_hop_data { + reconcile_pending_htlcs_with_monitor( + reconstruct_manager_from_monitors, + &mut already_forwarded_htlcs, + &mut forward_htlcs_legacy, + &mut pending_events_read, + &mut pending_intercepted_htlcs_legacy, + &mut decode_update_add_htlcs, + &mut decode_update_add_htlcs_legacy, + prev_hop_data, + &logger, + htlc.payment_hash, + monitor.channel_id(), + ); + } + }, HTLCSource::OutboundRoute { payment_id, session_priv, From bb5a827541968e6086bcf6b65c929cd453210737 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Fri, 27 Feb 2026 10:50:00 +0200 Subject: [PATCH 29/32] ln/refactor: move outgoing payment replay code into helper function --- lightning/src/ln/channelmanager.rs | 236 +++++++++++++++++------------ 1 file changed, 135 insertions(+), 101 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 260a9d0797d..2e9cd9bdda2 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -18546,6 +18546,81 @@ fn dedup_decode_update_add_htlcs( } } +/// Checks if a forwarded HTLC claim needs to be replayed on startup, returning None if it doesn't +/// need to be replayed. When the HTLC needs to be claimed, it returns a bool indicating whether +/// deserialization of should be failed due to missing information. +fn prev_hop_needs_claim_replay( + prev_hop: &HTLCPreviousHopData, payment_preimage: PaymentPreimage, + inbound_edge_monitor: &ChannelMonitor, + short_to_chan_info: &HashMap, logger: &L, +) -> Option { + // If the inbound edge of the payment's monitor has been fully claimed we've had at least + // `ANTI_REORG_DELAY` blocks to get any PaymentForwarded event(s) to the user and assume that + // there's no need to try to replay the claim just for that. + let inbound_edge_balances = inbound_edge_monitor.get_claimable_balances(); + if inbound_edge_balances.is_empty() { + return None; + } + + let mut fail_read = false; + if prev_hop.counterparty_node_id.is_none() { + // We no longer support claiming an HTLC where we don't have the counterparty_node_id + // available if the claim has to go to a closed channel. Its possible we can get away with + // it if the channel is not yet closed, but its by no means a guarantee. + + // Thus, in this case we are a bit more aggressive with our pruning - if we have no use for + // the claim (because the inbound edge of the payment's monitor has already claimed the + // HTLC) we skip trying to replay the claim. + let htlc_payment_hash: PaymentHash = payment_preimage.into(); + let logger = + WithChannelMonitor::from(logger, inbound_edge_monitor, Some(htlc_payment_hash)); + let balance_could_incl_htlc = |bal| match bal { + &Balance::ClaimableOnChannelClose { .. } => { + // The channel is still open, assume we can still + // claim against it + true + }, + &Balance::MaybePreimageClaimableHTLC { payment_hash, .. } => { + payment_hash == htlc_payment_hash + }, + _ => false, + }; + let htlc_may_be_in_balances = inbound_edge_balances.iter().any(balance_could_incl_htlc); + if !htlc_may_be_in_balances { + return None; + } + + // First check if we're absolutely going to fail - if we need to replay this claim to get + // the preimage into the inbound edge monitor but the channel is closed (and thus we'll + // immediately panic if we call claim_funds_from_hop). + if short_to_chan_info.get(&prev_hop.prev_outbound_scid_alias).is_none() { + log_error!(logger, + "We need to replay the HTLC claim for payment_hash {} (preimage {}) but cannot do so as the HTLC was forwarded prior to LDK 0.0.124.\ + All HTLCs that were forwarded by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1", + htlc_payment_hash, + payment_preimage, + ); + fail_read = true; + } + + // At this point we're confident we need the claim, but the + // inbound edge channel is still live. As long as this remains + // the case, we can conceivably proceed, but we run some risk + // of panicking at runtime. The user ideally should have read + // the release notes and we wouldn't be here, but we go ahead + // and let things run in the hope that it'll all just work out. + log_error!(logger, + "We need to replay the HTLC claim for payment_hash {} (preimage {}) but don't have all the required information to do so reliably.\ + As long as the channel for the inbound edge of the forward remains open, this may work okay, but we may panic at runtime!\ + All HTLCs that were forwarded by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1\ + Continuing anyway, though panics may occur!", + htlc_payment_hash, + payment_preimage, + ); + } + Some(fail_read) +} + // Implement ReadableArgs for an Arc'd ChannelManager to make it a bit easier to work with the // SipmleArcChannelManager type: impl< @@ -19507,112 +19582,71 @@ impl< // preimages from it which may be needed in upstream channels for forwarded // payments. let mut fail_read = false; - let outbound_claimed_htlcs_iter = monitor.get_all_current_outbound_htlcs() + let outbound_claimed_htlcs_iter = monitor + .get_all_current_outbound_htlcs() .into_iter() .filter_map(|(htlc_source, (htlc, preimage_opt))| { - if let HTLCSource::PreviousHopData(prev_hop) = &htlc_source { - if let Some(payment_preimage) = preimage_opt { - let inbound_edge_monitor = args.channel_monitors.get(&prev_hop.channel_id); - // Note that for channels which have gone to chain, - // `get_all_current_outbound_htlcs` is never pruned and always returns - // a constant set until the monitor is removed/archived. Thus, we - // want to skip replaying claims that have definitely been resolved - // on-chain. - - // If the inbound monitor is not present, we assume it was fully - // resolved and properly archived, implying this payment had plenty - // of time to get claimed and we can safely skip any further - // attempts to claim it (they wouldn't succeed anyway as we don't - // have a monitor against which to do so). - let inbound_edge_monitor = if let Some(monitor) = inbound_edge_monitor { - monitor - } else { - return None; - }; - // Second, if the inbound edge of the payment's monitor has been - // fully claimed we've had at least `ANTI_REORG_DELAY` blocks to - // get any PaymentForwarded event(s) to the user and assume that - // there's no need to try to replay the claim just for that. - let inbound_edge_balances = inbound_edge_monitor.get_claimable_balances(); - if inbound_edge_balances.is_empty() { - return None; - } - - if prev_hop.counterparty_node_id.is_none() { - // We no longer support claiming an HTLC where we don't have - // the counterparty_node_id available if the claim has to go to - // a closed channel. Its possible we can get away with it if - // the channel is not yet closed, but its by no means a - // guarantee. - - // Thus, in this case we are a bit more aggressive with our - // pruning - if we have no use for the claim (because the - // inbound edge of the payment's monitor has already claimed - // the HTLC) we skip trying to replay the claim. - let htlc_payment_hash: PaymentHash = payment_preimage.into(); - let logger = WithChannelMonitor::from( - &args.logger, - monitor, - Some(htlc_payment_hash), - ); - let balance_could_incl_htlc = |bal| match bal { - &Balance::ClaimableOnChannelClose { .. } => { - // The channel is still open, assume we can still - // claim against it - true - }, - &Balance::MaybePreimageClaimableHTLC { payment_hash, .. } => { - payment_hash == htlc_payment_hash - }, - _ => false, - }; - let htlc_may_be_in_balances = - inbound_edge_balances.iter().any(balance_could_incl_htlc); - if !htlc_may_be_in_balances { - return None; - } - - // First check if we're absolutely going to fail - if we need - // to replay this claim to get the preimage into the inbound - // edge monitor but the channel is closed (and thus we'll - // immediately panic if we call claim_funds_from_hop). - if short_to_chan_info.get(&prev_hop.prev_outbound_scid_alias).is_none() { - log_error!(logger, - "We need to replay the HTLC claim for payment_hash {} (preimage {}) but cannot do so as the HTLC was forwarded prior to LDK 0.0.124.\ - All HTLCs that were forwarded by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1", - htlc_payment_hash, - payment_preimage, - ); - fail_read = true; - } + let payment_preimage = preimage_opt?; + let prev_htlcs = match &htlc_source { + HTLCSource::PreviousHopData(prev_hop) => vec![prev_hop], + // If it was an outbound payment, we've handled it above - if a preimage + // came in and we persisted the `ChannelManager` we either handled it + // and are good to go or the channel force-closed - we don't have to + // handle the channel still live case here. + _ => vec![], + }; + let prev_htlcs_count = prev_htlcs.len(); + if prev_htlcs_count == 0 { + return None; + } - // At this point we're confident we need the claim, but the - // inbound edge channel is still live. As long as this remains - // the case, we can conceivably proceed, but we run some risk - // of panicking at runtime. The user ideally should have read - // the release notes and we wouldn't be here, but we go ahead - // and let things run in the hope that it'll all just work out. - log_error!(logger, - "We need to replay the HTLC claim for payment_hash {} (preimage {}) but don't have all the required information to do so reliably.\ - As long as the channel for the inbound edge of the forward remains open, this may work okay, but we may panic at runtime!\ - All HTLCs that were forwarded by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1\ - Continuing anyway, though panics may occur!", - htlc_payment_hash, - payment_preimage, - ); + for prev_hop in prev_htlcs { + // Note that for channels which have gone to chain, + // `get_all_current_outbound_htlcs` is never pruned and always returns + // a constant set until the monitor is removed/archived. Thus, we want + // to skip replaying claims that have definitely been resolved on-chain. + + // If the inbound monitor is not present, we assume it was fully + // resolved and properly archived, implying this payment had plenty of + // time to get claimed and we can safely skip any further attempts to + // claim it (they wouldn't succeed anyway as we don't have a monitor + // against which to do so). + let inbound_edge_monitor = + args.channel_monitors.get(&prev_hop.channel_id)?; + let logger = WithChannelMonitor::from( + &args.logger, + monitor, + Some(payment_preimage.into()), + ); + if let Some(fail_claim_read) = prev_hop_needs_claim_replay( + prev_hop, + payment_preimage, + inbound_edge_monitor, + &short_to_chan_info, + &logger, + ) { + // We can only fail to read from disk for legacy HTLCs that have + // a single prev_htlc. If we could fail_claim_read for multiple + // prev_htlcs, it wouldn't be correct to exit early on our first + // claimable prev_hop (because a subsequent one may + // fail_claim_read). + if fail_claim_read { + debug_assert!(prev_htlcs_count == 1); } - - Some((htlc_source, payment_preimage, htlc.amount_msat, - is_channel_closed, monitor.get_counterparty_node_id(), - monitor.get_funding_txo(), monitor.channel_id(), user_channel_id_opt)) - } else { None } - } else { - // If it was an outbound payment, we've handled it above - if a preimage - // came in and we persisted the `ChannelManager` we either handled it and - // are good to go or the channel force-closed - we don't have to handle the - // channel still live case here. - None + fail_read |= fail_claim_read; + return Some(( + htlc_source, + payment_preimage, + htlc.amount_msat, + is_channel_closed, + monitor.get_counterparty_node_id(), + monitor.get_funding_txo(), + monitor.channel_id(), + user_channel_id_opt, + )); + } } + None }); for tuple in outbound_claimed_htlcs_iter { pending_claims_to_replay.push(tuple); From 8a5b642a579b153f210e3121f637178a4d4669f4 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 2 Mar 2026 11:03:23 +0200 Subject: [PATCH 30/32] f No longer handle claims with missing counterparty_node_id --- lightning/src/ln/channelmanager.rs | 138 +++++++---------------------- 1 file changed, 30 insertions(+), 108 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2e9cd9bdda2..135e5c1b0d0 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -43,7 +43,7 @@ use crate::chain::chaininterface::{ TransactionType, }; use crate::chain::channelmonitor::{ - Balance, ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, MonitorEvent, + ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, MonitorEvent, WithChannelMonitor, ANTI_REORG_DELAY, CLTV_CLAIM_BUFFER, HTLC_FAIL_BACK_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, MAX_BLOCKS_FOR_CONF, }; @@ -18546,81 +18546,6 @@ fn dedup_decode_update_add_htlcs( } } -/// Checks if a forwarded HTLC claim needs to be replayed on startup, returning None if it doesn't -/// need to be replayed. When the HTLC needs to be claimed, it returns a bool indicating whether -/// deserialization of should be failed due to missing information. -fn prev_hop_needs_claim_replay( - prev_hop: &HTLCPreviousHopData, payment_preimage: PaymentPreimage, - inbound_edge_monitor: &ChannelMonitor, - short_to_chan_info: &HashMap, logger: &L, -) -> Option { - // If the inbound edge of the payment's monitor has been fully claimed we've had at least - // `ANTI_REORG_DELAY` blocks to get any PaymentForwarded event(s) to the user and assume that - // there's no need to try to replay the claim just for that. - let inbound_edge_balances = inbound_edge_monitor.get_claimable_balances(); - if inbound_edge_balances.is_empty() { - return None; - } - - let mut fail_read = false; - if prev_hop.counterparty_node_id.is_none() { - // We no longer support claiming an HTLC where we don't have the counterparty_node_id - // available if the claim has to go to a closed channel. Its possible we can get away with - // it if the channel is not yet closed, but its by no means a guarantee. - - // Thus, in this case we are a bit more aggressive with our pruning - if we have no use for - // the claim (because the inbound edge of the payment's monitor has already claimed the - // HTLC) we skip trying to replay the claim. - let htlc_payment_hash: PaymentHash = payment_preimage.into(); - let logger = - WithChannelMonitor::from(logger, inbound_edge_monitor, Some(htlc_payment_hash)); - let balance_could_incl_htlc = |bal| match bal { - &Balance::ClaimableOnChannelClose { .. } => { - // The channel is still open, assume we can still - // claim against it - true - }, - &Balance::MaybePreimageClaimableHTLC { payment_hash, .. } => { - payment_hash == htlc_payment_hash - }, - _ => false, - }; - let htlc_may_be_in_balances = inbound_edge_balances.iter().any(balance_could_incl_htlc); - if !htlc_may_be_in_balances { - return None; - } - - // First check if we're absolutely going to fail - if we need to replay this claim to get - // the preimage into the inbound edge monitor but the channel is closed (and thus we'll - // immediately panic if we call claim_funds_from_hop). - if short_to_chan_info.get(&prev_hop.prev_outbound_scid_alias).is_none() { - log_error!(logger, - "We need to replay the HTLC claim for payment_hash {} (preimage {}) but cannot do so as the HTLC was forwarded prior to LDK 0.0.124.\ - All HTLCs that were forwarded by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1", - htlc_payment_hash, - payment_preimage, - ); - fail_read = true; - } - - // At this point we're confident we need the claim, but the - // inbound edge channel is still live. As long as this remains - // the case, we can conceivably proceed, but we run some risk - // of panicking at runtime. The user ideally should have read - // the release notes and we wouldn't be here, but we go ahead - // and let things run in the hope that it'll all just work out. - log_error!(logger, - "We need to replay the HTLC claim for payment_hash {} (preimage {}) but don't have all the required information to do so reliably.\ - As long as the channel for the inbound edge of the forward remains open, this may work okay, but we may panic at runtime!\ - All HTLCs that were forwarded by LDK 0.0.123 and prior must be resolved prior to upgrading to LDK 0.1\ - Continuing anyway, though panics may occur!", - htlc_payment_hash, - payment_preimage, - ); - } - Some(fail_read) -} - // Implement ReadableArgs for an Arc'd ChannelManager to make it a bit easier to work with the // SipmleArcChannelManager type: impl< @@ -19612,39 +19537,36 @@ impl< // claim it (they wouldn't succeed anyway as we don't have a monitor // against which to do so). let inbound_edge_monitor = - args.channel_monitors.get(&prev_hop.channel_id)?; - let logger = WithChannelMonitor::from( - &args.logger, - monitor, - Some(payment_preimage.into()), - ); - if let Some(fail_claim_read) = prev_hop_needs_claim_replay( - prev_hop, - payment_preimage, - inbound_edge_monitor, - &short_to_chan_info, - &logger, - ) { - // We can only fail to read from disk for legacy HTLCs that have - // a single prev_htlc. If we could fail_claim_read for multiple - // prev_htlcs, it wouldn't be correct to exit early on our first - // claimable prev_hop (because a subsequent one may - // fail_claim_read). - if fail_claim_read { - debug_assert!(prev_htlcs_count == 1); - } - fail_read |= fail_claim_read; - return Some(( - htlc_source, - payment_preimage, - htlc.amount_msat, - is_channel_closed, - monitor.get_counterparty_node_id(), - monitor.get_funding_txo(), - monitor.channel_id(), - user_channel_id_opt, - )); + match args.channel_monitors.get(&prev_hop.channel_id) { + Some(monitor) => monitor, + None => continue, + }; + + if inbound_edge_monitor.get_claimable_balances().is_empty() { + continue; + } + + // We no longer support claiming an HTLC where we don't have the + // counterparty_node_id. This field has been populated since 0.0.124, + // so we expect it to be present for in flight claims in 0.3+. + if prev_hop.counterparty_node_id.is_none() { + fail_read = true; + return None; } + return Some(( + // When we have multiple prev_htlcs we assume that they all + // share the same htlc_source which contains all previous hops, + // so we can exit on the first claimable prev_hop because this + // will result in all prev_hops being claimed. + htlc_source, + payment_preimage, + htlc.amount_msat, + is_channel_closed, + monitor.get_counterparty_node_id(), + monitor.get_funding_txo(), + monitor.channel_id(), + user_channel_id_opt, + )); } None }); From c01088957f7a4dcdfaf2be986944f8a8ce695c30 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 2 Mar 2026 11:16:36 +0200 Subject: [PATCH 31/32] f Clarify comment about prev_htlcs from same HTLCSource --- lightning/src/ln/channelmanager.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 135e5c1b0d0..3600b97aaed 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -19554,9 +19554,9 @@ impl< return None; } return Some(( - // When we have multiple prev_htlcs we assume that they all - // share the same htlc_source which contains all previous hops, - // so we can exit on the first claimable prev_hop because this + // When we have multiple prev_htlcs we know that they are all from + // a single HTLCSource (see match above) which contains all previous + // hops, so we can exit on the first claimable prev_hop because this // will result in all prev_hops being claimed. htlc_source, payment_preimage, From 3f9ca25d07288b6b19a1c32272ae5f6e8f632e20 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Fri, 16 Jan 2026 13:03:55 -0500 Subject: [PATCH 32/32] ln: handle trampoline claims on restart This commit uses the existing outbound payment claims replay logic to restore trampoline claims. If any single previous hop in a htlc source with multiple previous hops requires claim, we represent this with a single outbound claimed htlc because we assume that *all* of the incoming htlcs are represented in the source, and will be appropriately claimed (rather than submitting multiple claims, which will end up being duplicates of each other). This is the case for trampoline payments, where the htlc_source stores all previous hops. --- lightning/src/ln/channelmanager.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 3600b97aaed..ad5d4d4d23a 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -19514,6 +19514,9 @@ impl< let payment_preimage = preimage_opt?; let prev_htlcs = match &htlc_source { HTLCSource::PreviousHopData(prev_hop) => vec![prev_hop], + HTLCSource::TrampolineForward { previous_hop_data, .. } => { + previous_hop_data.iter().collect() + }, // If it was an outbound payment, we've handled it above - if a preimage // came in and we persisted the `ChannelManager` we either handled it // and are good to go or the channel force-closed - we don't have to