From 6e7367826e6e286e5da23fa1157ebafbe53ffc1a Mon Sep 17 00:00:00 2001 From: shaavan Date: Wed, 11 Dec 2024 18:10:58 +0530 Subject: [PATCH 1/3] Include Counterparty Node IDs in `PaymentForwarded` This commit adds counterparty node IDs to `PaymentForwarded` to address the potential ambiguity of using `ChannelIds` alone, especially in cases like v1 0conf opens where `ChannelIds` may not be unique. Including the counterparty node IDs provides better clarity and makes the information more useful. --- lightning/src/events/mod.rs | 25 +++++++++++++++++++++---- lightning/src/ln/channelmanager.rs | 3 +++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 92991f4b228..0319e54c3d3 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1184,6 +1184,16 @@ pub enum Event { /// 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 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 @@ -1601,8 +1611,8 @@ impl Writeable for Event { } &Event::PaymentForwarded { prev_channel_id, next_channel_id, prev_user_channel_id, next_user_channel_id, - total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx, - outbound_amount_forwarded_msat, + prev_node_id, next_node_id, total_fee_earned_msat, skimmed_fee_msat, + claim_from_onchain_tx, outbound_amount_forwarded_msat, } => { 7u8.write(writer)?; write_tlv_fields!(writer, { @@ -1614,6 +1624,8 @@ impl Writeable for Event { (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), }); }, &Event::ChannelClosed { ref channel_id, ref user_channel_id, ref reason, @@ -1981,6 +1993,8 @@ impl MaybeReadable for Event { 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; let mut total_fee_earned_msat = None; let mut skimmed_fee_msat = None; let mut claim_from_onchain_tx = false; @@ -1994,11 +2008,14 @@ impl MaybeReadable for Event { (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), }); Ok(Some(Event::PaymentForwarded { prev_channel_id, next_channel_id, prev_user_channel_id, - next_user_channel_id, total_fee_earned_msat, skimmed_fee_msat, - claim_from_onchain_tx, outbound_amount_forwarded_msat, + next_user_channel_id, prev_node_id, next_node_id, + total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx, + outbound_amount_forwarded_msat, })) }; f() diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a58863ab1a2..ed7574fa286 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7309,6 +7309,7 @@ where 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); self.claim_funds_from_hop(hop_data, payment_preimage, None, |htlc_claim_value_msat, definitely_duplicate| { @@ -7358,6 +7359,8 @@ where next_channel_id: Some(next_channel_id), prev_user_channel_id, next_user_channel_id, + prev_node_id, + next_node_id: next_channel_counterparty_node_id, total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx: from_onchain, From 29eace91494361fe17903d64425d78ed93498220 Mon Sep 17 00:00:00 2001 From: shaavan Date: Wed, 11 Dec 2024 19:07:54 +0530 Subject: [PATCH 2/3] Update test utils --- lightning/src/ln/functional_test_utils.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 6c6b24bce7c..13de0f3988b 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -2360,7 +2360,8 @@ pub fn expect_payment_forwarded>( match event { Event::PaymentForwarded { prev_channel_id, next_channel_id, prev_user_channel_id, next_user_channel_id, - total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx, .. + prev_node_id, next_node_id, total_fee_earned_msat, + skimmed_fee_msat, claim_from_onchain_tx, .. } => { if allow_1_msat_fee_overpay { // Aggregating fees for blinded paths may result in a rounding error, causing slight @@ -2379,6 +2380,7 @@ pub fn expect_payment_forwarded>( // Is the event prev_channel_id in one of the channels between the two nodes? assert!(node.node().list_channels().iter().any(|x| x.counterparty.node_id == prev_node.node().get_our_node_id() && + prev_node.node().get_our_node_id() == prev_node_id.unwrap() && x.channel_id == prev_channel_id.unwrap() && x.user_channel_id == prev_user_channel_id.unwrap() )); @@ -2393,11 +2395,13 @@ pub fn expect_payment_forwarded>( if total_fee_earned_msat.is_none() { assert!(node.node().list_channels().iter().any(|x| x.counterparty.node_id == next_node.node().get_our_node_id() && + next_node.node().get_our_node_id() == next_node_id.unwrap() && x.channel_id == next_channel_id.unwrap() )); } else { assert!(node.node().list_channels().iter().any(|x| x.counterparty.node_id == next_node.node().get_our_node_id() && + next_node.node().get_our_node_id() == next_node_id.unwrap() && x.channel_id == next_channel_id.unwrap() && x.user_channel_id == next_user_channel_id.unwrap() )); From e4545257c9bcf8b82edca7a3fef8b2e62e7cb78d Mon Sep 17 00:00:00 2001 From: shaavan Date: Wed, 11 Dec 2024 18:50:23 +0530 Subject: [PATCH 3/3] Add ChangeLog --- .../3358-include-counterparty-id-in-payment-forwarded.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 pending_changelog/3358-include-counterparty-id-in-payment-forwarded.txt diff --git a/pending_changelog/3358-include-counterparty-id-in-payment-forwarded.txt b/pending_changelog/3358-include-counterparty-id-in-payment-forwarded.txt new file mode 100644 index 00000000000..8c2b1e338a6 --- /dev/null +++ b/pending_changelog/3358-include-counterparty-id-in-payment-forwarded.txt @@ -0,0 +1,7 @@ +API Updates +=========== + +To improve clarity and uniqueness in identifying forwarded payments, the `PaymentForwarded` +event now includes counterparty node IDs alongside `ChannelIds`. This change resolves +potential ambiguity in cases like v1 0conf channel opens, where `ChannelIds` alone may not +be unique. \ No newline at end of file