Merge pull request #3458 from shaavan/i3455

Include counterparty node ids in PaymentForwarded

Author: TheBlueMatt

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<u128>,
+		/// 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<PublicKey>,
+		/// 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<PublicKey>,
 		/// 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()

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,

lightning/src/ln/functional_test_utils.rs

@@ -2360,7 +2360,8 @@ pub fn expect_payment_forwarded<CM: AChannelManager, H: NodeHolder<CM=CM>>(
 	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<CM: AChannelManager, H: NodeHolder<CM=CM>>(
 				// 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<CM: AChannelManager, H: NodeHolder<CM=CM>>(
 				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()
 					));

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.