Merge pull request #3084 from jkczyz/2024-05-onion-fields

Include `RecipientOnionFields` in `Event::PaymentClaimed`

Author: TheBlueMatt

lightning-invoice/src/utils.rs

@@ -1397,7 +1397,9 @@ mod test {
 		let payment_preimage_opt = if user_generated_pmt_hash { None } else { Some(payment_preimage) };
 		assert_eq!(other_events.borrow().len(), 1);
 		check_payment_claimable(&other_events.borrow()[0], payment_hash, payment_secret, payment_amt, payment_preimage_opt, invoice.recover_payee_pub_key());
-		do_claim_payment_along_route(&nodes[0], &[&vec!(&nodes[fwd_idx])[..]], false, payment_preimage);
+		do_claim_payment_along_route(
+			ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[fwd_idx]]], payment_preimage)
+		);
 		expect_payment_sent(&nodes[0], payment_preimage, None, true, true);
 	}
 

lightning/src/events/mod.rs

@@ -653,6 +653,11 @@ pub enum Event {
 		/// The sender-intended sum total of all the MPP parts. This will be `None` for events
 		/// serialized prior to LDK version 0.0.117.
 		sender_intended_total_msat: Option<u64>,
+		/// The fields in the onion which were received with each HTLC. Only fields which were
+		/// identical in each HTLC involved in the payment will be included here.
+		///
+		/// Payments received on LDK versions prior to 0.0.124 will have this field unset.
+		onion_fields: Option<RecipientOnionFields>,
 	},
 	/// Indicates that a peer connection with a node is needed in order to send an [`OnionMessage`].
 	///
@@ -1348,7 +1353,7 @@ impl Writeable for Event {
 				// We never write the OpenChannelRequest events as, upon disconnection, peers
 				// drop any channels which have not yet exchanged funding_signed.
 			},
-			&Event::PaymentClaimed { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id, ref htlcs, ref sender_intended_total_msat } => {
+			&Event::PaymentClaimed { ref payment_hash, ref amount_msat, ref purpose, ref receiver_node_id, ref htlcs, ref sender_intended_total_msat, ref onion_fields } => {
 				19u8.write(writer)?;
 				write_tlv_fields!(writer, {
 					(0, payment_hash, required),
@@ -1357,6 +1362,7 @@ impl Writeable for Event {
 					(4, amount_msat, required),
 					(5, *htlcs, optional_vec),
 					(7, sender_intended_total_msat, option),
+					(9, onion_fields, option),
 				});
 			},
 			&Event::ProbeSuccessful { ref payment_id, ref payment_hash, ref path } => {
@@ -1719,13 +1725,15 @@ impl MaybeReadable for Event {
 					let mut receiver_node_id = None;
 					let mut htlcs: Option<Vec<ClaimedHTLC>> = Some(vec![]);
 					let mut sender_intended_total_msat: Option<u64> = None;
+					let mut onion_fields = None;
 					read_tlv_fields!(reader, {
 						(0, payment_hash, required),
 						(1, receiver_node_id, option),
 						(2, purpose, upgradable_required),
 						(4, amount_msat, required),
 						(5, htlcs, optional_vec),
 						(7, sender_intended_total_msat, option),
+						(9, onion_fields, option),
 					});
 					Ok(Some(Event::PaymentClaimed {
 						receiver_node_id,
@@ -1734,6 +1742,7 @@ impl MaybeReadable for Event {
 						amount_msat,
 						htlcs: htlcs.unwrap_or(vec![]),
 						sender_intended_total_msat,
+						onion_fields,
 					}))
 				};
 				f()

lightning/src/ln/blinded_payment_tests.rs

@@ -180,7 +180,9 @@ fn mpp_to_one_hop_blinded_path() {
 	let ev = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
 	pass_along_path(&nodes[0], expected_route[1], amt_msat, payment_hash.clone(),
 		Some(payment_secret), ev.clone(), true, None);
-	claim_payment_along_route(&nodes[0], expected_route, false, payment_preimage);
+	claim_payment_along_route(
+		ClaimAlongRouteArgs::new(&nodes[0], expected_route, payment_preimage)
+	);
 }
 
 #[test]
@@ -244,7 +246,9 @@ fn mpp_to_three_hop_blinded_paths() {
 	let ev = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
 	pass_along_path(&nodes[0], expected_route[1], amt_msat, payment_hash.clone(),
 		Some(payment_secret), ev.clone(), true, None);
-	claim_payment_along_route(&nodes[0], expected_route, false, payment_preimage);
+	claim_payment_along_route(
+		ClaimAlongRouteArgs::new(&nodes[0], expected_route, payment_preimage)
+	);
 }
 
 enum ForwardCheckFail {
@@ -643,7 +647,9 @@ fn do_blinded_intercept_payment(intercept_node_fails: bool) {
 	expect_pending_htlcs_forwardable!(nodes[2]);
 
 	expect_payment_claimable!(&nodes[2], payment_hash, payment_secret, amt_msat, None, nodes[2].node.get_our_node_id());
-	do_claim_payment_along_route(&nodes[0], &vec!(&vec!(&nodes[1], &nodes[2])[..]), false, payment_preimage);
+	do_claim_payment_along_route(
+		ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1], &nodes[2]]], payment_preimage)
+	);
 	expect_payment_sent(&nodes[0], payment_preimage, Some(Some(1000)), true, true);
 }
 
@@ -1217,7 +1223,9 @@ fn blinded_keysend() {
 
 	let ev = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events);
 	pass_along_path(&nodes[0], expected_route[0], amt_msat, payment_hash, Some(payment_secret), ev.clone(), true, Some(keysend_preimage));
-	claim_payment_along_route(&nodes[0], expected_route, false, keysend_preimage);
+	claim_payment_along_route(
+		ClaimAlongRouteArgs::new(&nodes[0], expected_route, keysend_preimage)
+	);
 }
 
 #[test]
@@ -1268,7 +1276,9 @@ fn blinded_mpp_keysend() {
 	let ev = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
 	pass_along_path(&nodes[0], expected_route[1], amt_msat, payment_hash.clone(),
 		Some(payment_secret), ev.clone(), true, Some(keysend_preimage));
-	claim_payment_along_route(&nodes[0], expected_route, false, keysend_preimage);
+	claim_payment_along_route(
+		ClaimAlongRouteArgs::new(&nodes[0], expected_route, keysend_preimage)
+	);
 }
 
 #[test]
@@ -1316,5 +1326,8 @@ fn custom_tlvs_to_blinded_path() {
 		.with_payment_secret(payment_secret)
 		.with_custom_tlvs(recipient_onion_fields.custom_tlvs.clone());
 	do_pass_along_path(args);
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage);
+	claim_payment_along_route(
+		ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1]]], payment_preimage)
+			.with_custom_tlvs(recipient_onion_fields.custom_tlvs.clone())
+	);
 }

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -2025,7 +2025,9 @@ fn test_path_paused_mpp() {
 	assert_eq!(events.len(), 1);
 	pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 200_000, payment_hash.clone(), Some(payment_secret), events.pop().unwrap(), true, None);
 
-	claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage);
+	claim_payment_along_route(
+		ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], payment_preimage)
+	);
 }
 
 #[test]

lightning/src/ln/channelmanager.rs

@@ -681,13 +681,15 @@ struct ClaimingPayment {
 	receiver_node_id: PublicKey,
 	htlcs: Vec<events::ClaimedHTLC>,
 	sender_intended_value: Option<u64>,
+	onion_fields: Option<RecipientOnionFields>,
 }
 impl_writeable_tlv_based!(ClaimingPayment, {
 	(0, amount_msat, required),
 	(2, payment_purpose, required),
 	(4, receiver_node_id, required),
 	(5, htlcs, optional_vec),
 	(7, sender_intended_value, option),
+	(9, onion_fields, option),
 });
 
 struct ClaimablePayment {
@@ -6325,19 +6327,27 @@ where
 					}
 				}
 
-				let htlcs = payment.htlcs.iter().map(events::ClaimedHTLC::from).collect();
-				let sender_intended_value = payment.htlcs.first().map(|htlc| htlc.total_msat);
-				let dup_purpose = claimable_payments.pending_claiming_payments.insert(payment_hash,
-					ClaimingPayment { amount_msat: payment.htlcs.iter().map(|source| source.value).sum(),
-					payment_purpose: payment.purpose, receiver_node_id, htlcs, sender_intended_value
-				});
-				if dup_purpose.is_some() {
-					debug_assert!(false, "Shouldn't get a duplicate pending claim event ever");
-					log_error!(self.logger, "Got a duplicate pending claimable event on payment hash {}! Please report this bug",
-						&payment_hash);
-				}
+				let claiming_payment = claimable_payments.pending_claiming_payments
+					.entry(payment_hash)
+					.and_modify(|_| {
+						debug_assert!(false, "Shouldn't get a duplicate pending claim event ever");
+						log_error!(self.logger, "Got a duplicate pending claimable event on payment hash {}! Please report this bug",
+							&payment_hash);
+					})
+					.or_insert_with(|| {
+						let htlcs = payment.htlcs.iter().map(events::ClaimedHTLC::from).collect();
+						let sender_intended_value = payment.htlcs.first().map(|htlc| htlc.total_msat);
+						ClaimingPayment {
+							amount_msat: payment.htlcs.iter().map(|source| source.value).sum(),
+							payment_purpose: payment.purpose,
+							receiver_node_id,
+							htlcs,
+							sender_intended_value,
+							onion_fields: payment.onion_fields,
+						}
+					});
 
-				if let Some(RecipientOnionFields { ref custom_tlvs, .. }) = payment.onion_fields {
+				if let Some(RecipientOnionFields { ref custom_tlvs, .. }) = claiming_payment.onion_fields {
 					if !custom_tlvs_known && custom_tlvs.iter().any(|(typ, _)| typ % 2 == 0) {
 						log_info!(self.logger, "Rejecting payment with payment hash {} as we cannot accept payment with unknown even TLVs: {}",
 							&payment_hash, log_iter!(custom_tlvs.iter().map(|(typ, _)| typ).filter(|typ| *typ % 2 == 0)));
@@ -6744,6 +6754,7 @@ where
 						receiver_node_id,
 						htlcs,
 						sender_intended_value: sender_intended_total_msat,
+						onion_fields,
 					}) = payment {
 						self.pending_events.lock().unwrap().push_back((events::Event::PaymentClaimed {
 							payment_hash,
@@ -6752,6 +6763,7 @@ where
 							receiver_node_id: Some(receiver_node_id),
 							htlcs,
 							sender_intended_total_msat,
+							onion_fields,
 						}, None));
 					}
 				},
@@ -12273,6 +12285,7 @@ where
 						amount_msat: claimable_amt_msat,
 						htlcs: payment.htlcs.iter().map(events::ClaimedHTLC::from).collect(),
 						sender_intended_total_msat: payment.htlcs.first().map(|htlc| htlc.total_msat),
+						onion_fields: payment.onion_fields,
 					}, None));
 				}
 			}

lightning/src/ln/functional_test_utils.rs

@@ -2716,18 +2716,12 @@ pub fn send_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route
 	(our_payment_preimage, our_payment_hash, our_payment_secret, payment_id)
 }
 
-pub fn do_claim_payment_along_route<'a, 'b, 'c>(
-	origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool,
-	our_payment_preimage: PaymentPreimage
-) -> u64 {
-	for path in expected_paths.iter() {
-		assert_eq!(path.last().unwrap().node.get_our_node_id(), expected_paths[0].last().unwrap().node.get_our_node_id());
+pub fn do_claim_payment_along_route(args: ClaimAlongRouteArgs) -> u64 {
+	for path in args.expected_paths.iter() {
+		assert_eq!(path.last().unwrap().node.get_our_node_id(), args.expected_paths[0].last().unwrap().node.get_our_node_id());
 	}
-	expected_paths[0].last().unwrap().node.claim_funds(our_payment_preimage);
-	pass_claimed_payment_along_route(
-		ClaimAlongRouteArgs::new(origin_node, expected_paths, our_payment_preimage)
-			.skip_last(skip_last)
-	)
+	args.expected_paths[0].last().unwrap().node.claim_funds(args.payment_preimage);
+	pass_claimed_payment_along_route(args)
 }
 
 pub struct ClaimAlongRouteArgs<'a, 'b, 'c, 'd> {
@@ -2737,6 +2731,7 @@ pub struct ClaimAlongRouteArgs<'a, 'b, 'c, 'd> {
 	pub expected_min_htlc_overpay: Vec<u32>,
 	pub skip_last: bool,
 	pub payment_preimage: PaymentPreimage,
+	pub custom_tlvs: Vec<(u64, Vec<u8>)>,
 	// Allow forwarding nodes to have taken 1 msat more fee than expected based on the downstream
 	// fulfill amount.
 	//
@@ -2755,7 +2750,7 @@ impl<'a, 'b, 'c, 'd> ClaimAlongRouteArgs<'a, 'b, 'c, 'd> {
 		Self {
 			origin_node, expected_paths, expected_extra_fees: vec![0; expected_paths.len()],
 			expected_min_htlc_overpay: vec![0; expected_paths.len()], skip_last: false, payment_preimage,
-			allow_1_msat_fee_overpay: false,
+			allow_1_msat_fee_overpay: false, custom_tlvs: vec![],
 		}
 	}
 	pub fn skip_last(mut self, skip_last: bool) -> Self {
@@ -2774,12 +2769,16 @@ impl<'a, 'b, 'c, 'd> ClaimAlongRouteArgs<'a, 'b, 'c, 'd> {
 		self.allow_1_msat_fee_overpay = true;
 		self
 	}
+	pub fn with_custom_tlvs(mut self, custom_tlvs: Vec<(u64, Vec<u8>)>) -> Self {
+		self.custom_tlvs = custom_tlvs;
+		self
+	}
 }
 
-pub fn pass_claimed_payment_along_route<'a, 'b, 'c, 'd>(args: ClaimAlongRouteArgs) -> u64 {
+pub fn pass_claimed_payment_along_route(args: ClaimAlongRouteArgs) -> u64 {
 	let ClaimAlongRouteArgs {
 		origin_node, expected_paths, expected_extra_fees, expected_min_htlc_overpay, skip_last,
-		payment_preimage: our_payment_preimage, allow_1_msat_fee_overpay,
+		payment_preimage: our_payment_preimage, allow_1_msat_fee_overpay, custom_tlvs,
 	} = args;
 	let claim_event = expected_paths[0].last().unwrap().node.get_and_clear_pending_events();
 	assert_eq!(claim_event.len(), 1);
@@ -2793,11 +2792,13 @@ pub fn pass_claimed_payment_along_route<'a, 'b, 'c, 'd>(args: ClaimAlongRouteArg
 				| PaymentPurpose::Bolt12RefundPayment { payment_preimage: Some(preimage), .. },
 			amount_msat,
 			ref htlcs,
+			ref onion_fields,
 			..
 		} => {
 			assert_eq!(preimage, our_payment_preimage);
 			assert_eq!(htlcs.len(), expected_paths.len());  // One per path.
 			assert_eq!(htlcs.iter().map(|h| h.value_msat).sum::<u64>(), amount_msat);
+			assert_eq!(onion_fields.as_ref().unwrap().custom_tlvs, custom_tlvs);
 			expected_paths.iter().zip(htlcs).for_each(|(path, htlc)| check_claimed_htlc_channel(origin_node, path, htlc));
 			fwd_amt_msat = amount_msat;
 		},
@@ -2808,11 +2809,13 @@ pub fn pass_claimed_payment_along_route<'a, 'b, 'c, 'd>(args: ClaimAlongRouteArg
 			payment_hash,
 			amount_msat,
 			ref htlcs,
+			ref onion_fields,
 			..
 		} => {
 			assert_eq!(&payment_hash.0, &Sha256::hash(&our_payment_preimage.0)[..]);
 			assert_eq!(htlcs.len(), expected_paths.len());  // One per path.
 			assert_eq!(htlcs.iter().map(|h| h.value_msat).sum::<u64>(), amount_msat);
+			assert_eq!(onion_fields.as_ref().unwrap().custom_tlvs, custom_tlvs);
 			expected_paths.iter().zip(htlcs).for_each(|(path, htlc)| check_claimed_htlc_channel(origin_node, path, htlc));
 			fwd_amt_msat = amount_msat;
 		}
@@ -2956,15 +2959,20 @@ pub fn pass_claimed_payment_along_route<'a, 'b, 'c, 'd>(args: ClaimAlongRouteArg
 
 	expected_total_fee_msat
 }
-pub fn claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_preimage: PaymentPreimage) {
-	let expected_total_fee_msat = do_claim_payment_along_route(origin_node, expected_paths, skip_last, our_payment_preimage);
+pub fn claim_payment_along_route(args: ClaimAlongRouteArgs) {
+	let origin_node = args.origin_node;
+	let payment_preimage = args.payment_preimage;
+	let skip_last = args.skip_last;
+	let expected_total_fee_msat = do_claim_payment_along_route(args);
 	if !skip_last {
-		expect_payment_sent!(origin_node, our_payment_preimage, Some(expected_total_fee_msat));
+		expect_payment_sent!(origin_node, payment_preimage, Some(expected_total_fee_msat));
 	}
 }
 
 pub fn claim_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], our_payment_preimage: PaymentPreimage) {
-	claim_payment_along_route(origin_node, &[expected_route], false, our_payment_preimage);
+	claim_payment_along_route(
+		ClaimAlongRouteArgs::new(origin_node, &[expected_route], our_payment_preimage)
+	);
 }
 
 pub const TEST_FINAL_CLTV: u32 = 70;

lightning/src/ln/functional_tests.rs

@@ -3812,7 +3812,10 @@ fn test_simple_peer_disconnect() {
 	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id());
 	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id());
 
-	claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], true, payment_preimage_3);
+	claim_payment_along_route(
+		ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1], &nodes[2]]], payment_preimage_3)
+			.skip_last(true)
+	);
 	fail_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[2]]], true, payment_hash_5);
 
 	let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
@@ -8296,7 +8299,9 @@ fn test_onion_value_mpp_set_calculation() {
 	let ev = remove_first_msg_event_to_node(&expected_paths[1][0].node.get_our_node_id(), &mut events);
 	pass_along_path(&nodes[0], expected_paths[1], 101_000, our_payment_hash.clone(), Some(our_payment_secret), ev, true, None);
 
-	claim_payment_along_route(&nodes[0], expected_paths, false, our_payment_preimage);
+	claim_payment_along_route(
+		ClaimAlongRouteArgs::new(&nodes[0], expected_paths, our_payment_preimage)
+	);
 }
 
 fn do_test_overshoot_mpp(msat_amounts: &[u64], total_msat: u64) {
@@ -8362,7 +8367,9 @@ fn do_test_overshoot_mpp(msat_amounts: &[u64], total_msat: u64) {
 		pass_along_path(&nodes[src_idx], expected_path, amount_received, our_payment_hash.clone(), Some(our_payment_secret), ev, became_claimable_now, None);
 	}
 
-	claim_payment_along_route(&nodes[src_idx], &expected_paths, false, our_payment_preimage);
+	claim_payment_along_route(
+		ClaimAlongRouteArgs::new(&nodes[src_idx], &expected_paths, our_payment_preimage)
+	);
 }
 
 #[test]
@@ -8394,7 +8401,9 @@ fn test_simple_mpp() {
 	route.paths[1].hops[0].short_channel_id = chan_2_id;
 	route.paths[1].hops[1].short_channel_id = chan_4_id;
 	send_along_route_with_secret(&nodes[0], route, &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], 200_000, payment_hash, payment_secret);
-	claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage);
+	claim_payment_along_route(
+		ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], payment_preimage)
+	);
 }
 
 #[test]
@@ -9838,7 +9847,9 @@ fn test_inconsistent_mpp_params() {
 	assert_eq!(events.len(), 1);
 	pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 15_000_000, our_payment_hash, Some(our_payment_secret), events.pop().unwrap(), true, None);
 
-	do_claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, our_payment_preimage);
+	do_claim_payment_along_route(
+		ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], our_payment_preimage)
+	);
 	expect_payment_sent(&nodes[0], our_payment_preimage, Some(None), true, true);
 }