Merge PR 'Add amounts to HTLC locators' (#4767)

from amt-htlc-locators into main

Reviewed-on: https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/pulls/4767
Reviewed-by: Matt Corallo <matt@noreply.gitea.bitcoin.ninja>

Author: Matt Corallo

lightning/src/events/mod.rs

@@ -866,6 +866,9 @@ pub struct HTLCLocator {
 	/// The channel that the HTLC was sent or received on.
 	pub channel_id: ChannelId,
 
+	/// The amount, in milli-satoshis, of the HTLC that was sent or received, if known.
+	pub amount_msat: Option<u64>,
+
 	/// The `user_channel_id` for `channel_id`.
 	///
 	/// This will be `None` if the payment was settled via an on-chain transaction. It will also
@@ -883,6 +886,7 @@ impl_ser_tlv_based!(HTLCLocator, {
 	(1, channel_id, required),
 	(3, user_channel_id, option),
 	(5, node_id, option),
+	(7, amount_msat, option),
 });
 
 /// An Event which you should probably take some action in response to.
@@ -2215,6 +2219,7 @@ impl Writeable for Event {
 				);
 				let empty_locator = HTLCLocator {
 					channel_id: ChannelId::new_zero(),
+					amount_msat: None,
 					user_channel_id: None,
 					node_id: None,
 				};
@@ -2782,11 +2787,14 @@ impl MaybeReadable for Event {
 						// 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.ok_or(DecodeError::InvalidValue)?,
+							amount_msat: total_fee_earned_msat
+								.map(|fee| outbound_amount_forwarded_msat + fee),
 							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.ok_or(DecodeError::InvalidValue)?,
+							amount_msat: Some(outbound_amount_forwarded_msat),
 							user_channel_id: next_user_channel_id_legacy,
 							node_id: next_node_id_legacy,
 						}])),
@@ -3228,6 +3236,48 @@ impl MaybeReadable for Event {
 	}
 }
 
+#[cfg(test)]
+mod tests {
+	use super::*;
+
+	#[test]
+	fn legacy_payment_forwarded_preserves_unknown_inbound_htlc_amount() {
+		let prev_channel_id = ChannelId::from_bytes([1; 32]);
+		let next_channel_id = ChannelId::from_bytes([2; 32]);
+		let mut encoded_legacy_event = vec![
+			7,  // Event::PaymentForwarded
+			81, // TLV stream length
+			1, 32, // prev_channel_id
+		];
+		encoded_legacy_event.extend_from_slice(&[1; 32]);
+		encoded_legacy_event.extend_from_slice(&[2, 1, 0]); // claim_from_onchain_tx
+		encoded_legacy_event.extend_from_slice(&[3, 32]); // next_channel_id
+		encoded_legacy_event.extend_from_slice(&[2; 32]);
+		// outbound_amount_forwarded_msat
+		encoded_legacy_event.extend_from_slice(&[5, 8, 0, 0, 0, 0, 0, 45, 198, 192]);
+
+		match Event::read(&mut &encoded_legacy_event[..]).unwrap().unwrap() {
+			Event::PaymentForwarded {
+				prev_htlcs,
+				next_htlcs,
+				total_fee_earned_msat,
+				outbound_amount_forwarded_msat,
+				..
+			} => {
+				assert_eq!(total_fee_earned_msat, None);
+				assert_eq!(outbound_amount_forwarded_msat, 3_000_000);
+				assert_eq!(prev_htlcs.len(), 1);
+				assert_eq!(prev_htlcs[0].channel_id, prev_channel_id);
+				assert_eq!(prev_htlcs[0].amount_msat, None);
+				assert_eq!(next_htlcs.len(), 1);
+				assert_eq!(next_htlcs[0].channel_id, next_channel_id);
+				assert_eq!(next_htlcs[0].amount_msat, Some(3_000_000));
+			},
+			_ => panic!("expected PaymentForwarded event"),
+		}
+	}
+}
+
 /// A trait indicating an object may generate events.
 ///
 /// Events are processed by passing an [`EventHandler`] to [`process_pending_events`].

lightning/src/ln/channel.rs

@@ -8175,6 +8175,7 @@ where
 				let prev_hop_data = HTLCPreviousHopData {
 					prev_outbound_scid_alias,
 					user_channel_id: Some(user_channel_id),
+					amount_msat: Some(htlc.amount_msat),
 					htlc_id: htlc.htlc_id,
 					incoming_packet_shared_secret: *incoming_packet_shared_secret,
 					phantom_shared_secret: *phantom_shared_secret,

lightning/src/ln/channelmanager.rs

@@ -481,6 +481,7 @@ impl PendingAddHTLCInfo {
 		HTLCPreviousHopData {
 			prev_outbound_scid_alias: self.prev_outbound_scid_alias,
 			user_channel_id: Some(self.prev_user_channel_id),
+			amount_msat: self.forward_info.incoming_amt_msat,
 			outpoint: self.prev_funding_outpoint,
 			channel_id: self.prev_channel_id,
 			counterparty_node_id: Some(self.prev_counterparty_node_id),
@@ -944,6 +945,7 @@ mod fuzzy_channelmanager {
 	pub struct HTLCPreviousHopData {
 		pub prev_outbound_scid_alias: u64,
 		pub user_channel_id: Option<u128>,
+		pub amount_msat: Option<u64>,
 		pub htlc_id: u64,
 		pub incoming_packet_shared_secret: [u8; 32],
 		pub phantom_shared_secret: Option<[u8; 32]>,
@@ -960,12 +962,13 @@ mod fuzzy_channelmanager {
 		pub cltv_expiry: Option<u32>,
 	}
 
-	impl From<&HTLCPreviousHopData> for events::HTLCLocator {
-		fn from(value: &HTLCPreviousHopData) -> Self {
+	impl HTLCPreviousHopData {
+		pub(super) fn htlc_locator(&self, amount_msat: Option<u64>) -> events::HTLCLocator {
 			events::HTLCLocator {
-				channel_id: value.channel_id,
-				user_channel_id: value.user_channel_id,
-				node_id: value.counterparty_node_id,
+				channel_id: self.channel_id,
+				amount_msat,
+				user_channel_id: self.user_channel_id,
+				node_id: self.counterparty_node_id,
 			}
 		}
 	}
@@ -8860,6 +8863,7 @@ impl<
 					let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
 						prev_outbound_scid_alias: prev_hop.prev_outbound_scid_alias,
 						user_channel_id: prev_hop.user_channel_id,
+						amount_msat: Some(value),
 						counterparty_node_id: prev_hop.counterparty_node_id,
 						channel_id: prev_channel_id,
 						outpoint: prev_funding_outpoint,
@@ -10522,7 +10526,7 @@ 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)];
+				let event_prev_hop_data = hop_data.clone();
 				self.claim_funds_from_htlc_forward_hop(
 					payment_preimage,
 					|htlc_claim_value_msat: Option<u64>| -> Option<events::Event> {
@@ -10536,11 +10540,16 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 							skimmed_fee_msat <= total_fee_earned_msat,
 							"skimmed_fee_msat must always be included in total_fee_earned_msat"
 						);
+						let prev_htlc_amount_msat =
+							event_prev_hop_data.amount_msat.or(htlc_claim_value_msat);
 
 						Some(events::Event::PaymentForwarded {
-							prev_htlcs,
+							prev_htlcs: vec![
+								event_prev_hop_data.htlc_locator(prev_htlc_amount_msat)
+							],
 							next_htlcs: vec![events::HTLCLocator {
 								channel_id: next_channel_id,
+								amount_msat: Some(forwarded_htlc_value_msat),
 								user_channel_id: next_user_channel_id,
 								node_id: Some(next_channel_counterparty_node_id),
 							}],
@@ -10561,20 +10570,29 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 			},
 			HTLCSource::TrampolineForward { previous_hop_data, .. } => {
 				// Only emit a single event for trampoline claims.
-				let prev_htlcs: Vec<events::HTLCLocator> =
-					previous_hop_data.iter().map(Into::into).collect();
+				let mut event_prev_htlcs = Some(
+					previous_hop_data.iter().map(|hop| hop.htlc_locator(hop.amount_msat)).collect(),
+				);
 				for (i, current_previous_hop_data) in previous_hop_data.into_iter().enumerate() {
 					self.claim_funds_from_htlc_forward_hop(
 						payment_preimage,
 						|_: Option<u64>| -> Option<events::Event> {
 							if i == 0 {
+								let Some(prev_htlcs) = event_prev_htlcs.take() else {
+									debug_assert!(
+										false,
+										"trampoline forward event already emitted"
+									);
+									return None;
+								};
 								Some(events::Event::PaymentForwarded {
-									prev_htlcs: prev_htlcs.clone(),
+									prev_htlcs,
 									// 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,
+										amount_msat: Some(forwarded_htlc_value_msat),
 										user_channel_id: next_user_channel_id,
 										node_id: Some(next_channel_counterparty_node_id),
 									}],
@@ -18343,6 +18361,7 @@ impl_ser_tlv_based!(HTLCPreviousHopData, {
 	(9, channel_id, (default_value, ChannelId::v1_from_funding_outpoint(outpoint.0.unwrap()))),
 	(11, counterparty_node_id, option),
 	(13, trampoline_shared_secret, option),
+	(15, amount_msat, option),
 });
 
 fn write_claimable_htlc<W: Writer>(

lightning/src/ln/functional_test_utils.rs

@@ -3097,6 +3097,7 @@ pub fn expect_payment_forwarded<CM: AChannelManager, H: NodeHolder<CM = CM>>(
 			total_fee_earned_msat,
 			skimmed_fee_msat,
 			claim_from_onchain_tx,
+			outbound_amount_forwarded_msat,
 			..
 		} => {
 			assert_eq!(prev_htlcs.len(), 1);
@@ -3115,6 +3116,19 @@ pub fn expect_payment_forwarded<CM: AChannelManager, H: NodeHolder<CM = CM>>(
 			// Check that the (knowingly) withheld amount is always less or equal to the expected
 			// overpaid amount.
 			assert!(skimmed_fee_msat == expected_extra_fees_msat);
+			match expected_fee {
+				Some(_) => {
+					let actual_fee = total_fee_earned_msat.unwrap();
+					assert_eq!(next_htlcs[0].amount_msat, Some(outbound_amount_forwarded_msat));
+					assert_eq!(
+						prev_htlcs[0].amount_msat,
+						Some(next_htlcs[0].amount_msat.unwrap() + actual_fee)
+					);
+				},
+				None => {
+					assert_eq!(total_fee_earned_msat, None);
+				},
+			}
 			if !upstream_force_closed {
 				let prev_node_id = prev_htlcs[0].node_id.unwrap();
 				let prev_channel_id = prev_htlcs[0].channel_id;

lightning/src/ln/functional_tests.rs

@@ -1506,8 +1506,10 @@ pub fn test_htlc_on_chain_success() {
 		} => {
 			assert_eq!(total_fee_earned_msat, Some(1000));
 			assert_eq!(prev_htlcs[0].channel_id, chan_id);
+			assert_eq!(prev_htlcs[0].amount_msat, Some(3001000));
 			assert_eq!(claim_from_onchain_tx, true);
 			assert_eq!(next_htlcs[0].channel_id, chan_2.2);
+			assert_eq!(next_htlcs[0].amount_msat, Some(3000000));
 			assert_eq!(outbound_amount_forwarded_msat, 3000000);
 		},
 		_ => panic!(),
@@ -1523,8 +1525,10 @@ pub fn test_htlc_on_chain_success() {
 		} => {
 			assert_eq!(total_fee_earned_msat, Some(1000));
 			assert_eq!(prev_htlcs[0].channel_id, chan_id);
+			assert_eq!(prev_htlcs[0].amount_msat, Some(3001000));
 			assert_eq!(claim_from_onchain_tx, true);
 			assert_eq!(next_htlcs[0].channel_id, chan_2.2);
+			assert_eq!(next_htlcs[0].amount_msat, Some(3000000));
 			assert_eq!(outbound_amount_forwarded_msat, 3000000);
 		},
 		_ => panic!(),

lightning/src/ln/trampoline_forward_tests.rs

@@ -27,6 +27,7 @@ fn test_prev_hop_data(htlc_id: u64) -> HTLCPreviousHopData {
 	HTLCPreviousHopData {
 		prev_outbound_scid_alias: 0,
 		user_channel_id: None,
+		amount_msat: None,
 		htlc_id,
 		incoming_packet_shared_secret: [0; 32],
 		phantom_shared_secret: None,