Decode hold times as a sender

Hold times are surfaced via the PaymentPathSuccessful event.

Author: joostjager

fuzz/src/process_onion_failure.rs

@@ -115,7 +115,7 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	let path = Path { hops, blinded_tail };
 
 	let htlc_source = HTLCSource::OutboundRoute {
-		path,
+		path: path.clone(),
 		session_priv,
 		first_hop_htlc_msat: 0,
 		payment_id,
@@ -133,8 +133,19 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 	} else {
 		None
 	};
-	let encrypted_packet = OnionErrorPacket { data: failure_data.into(), attribution_data };
+	let encrypted_packet =
+		OnionErrorPacket { data: failure_data.into(), attribution_data: attribution_data.clone() };
 	lightning::ln::process_onion_failure(&secp_ctx, &logger, &htlc_source, encrypted_packet);
+
+	if let Some(attribution_data) = attribution_data {
+		lightning::ln::process_onion_success(
+			&secp_ctx,
+			&logger,
+			&path,
+			&session_priv,
+			attribution_data,
+		);
+	}
 }
 
 /// Method that needs to be added manually, {name}_test

lightning-background-processor/src/lib.rs

@@ -2732,6 +2732,7 @@ mod tests {
 				payment_id: PaymentId([42; 32]),
 				payment_hash: None,
 				path: path.clone(),
+				hold_times: None,
 			});
 			let event = $receive.expect("PaymentPathSuccessful not handled within deadline");
 			match event {

lightning/src/events/mod.rs

@@ -1080,6 +1080,8 @@ pub enum Event {
 		///
 		/// May contain a closed channel if the HTLC sent along the path was fulfilled on chain.
 		path: Path,
+		/// The hold times as reported by each hop.
+		hold_times: Option<Vec<u32>>,
 	},
 	/// Indicates an outbound HTLC we sent failed, likely due to an intermediary node being unable to
 	/// handle the HTLC.
@@ -1816,13 +1818,19 @@ impl Writeable for Event {
 					(4, funding_info, required),
 				})
 			},
-			&Event::PaymentPathSuccessful { ref payment_id, ref payment_hash, ref path } => {
+			&Event::PaymentPathSuccessful {
+				ref payment_id,
+				ref payment_hash,
+				ref path,
+				ref hold_times,
+			} => {
 				13u8.write(writer)?;
 				write_tlv_fields!(writer, {
 					(0, payment_id, required),
 					(2, payment_hash, option),
 					(4, path.hops, required_vec),
 					(6, path.blinded_tail, option),
+					(8, hold_times, option),
 				})
 			},
 			&Event::PaymentFailed { ref payment_id, ref payment_hash, ref reason } => {
@@ -2311,11 +2319,13 @@ impl MaybeReadable for Event {
 						(2, payment_hash, option),
 						(4, path, required_vec),
 						(6, blinded_tail, option),
+						(8, hold_times, option),
 					});
 					Ok(Some(Event::PaymentPathSuccessful {
 						payment_id: payment_id.0.unwrap(),
 						payment_hash,
 						path: Path { hops: path, blinded_tail },
+						hold_times,
 					}))
 				};
 				f()

lightning/src/ln/async_payments_tests.rs

@@ -445,7 +445,7 @@ fn async_receive_flow_success() {
 	let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev)
 		.with_payment_preimage(keysend_preimage);
 	do_pass_along_path(args);
-	let res =
+	let (res, _) =
 		claim_payment_along_route(ClaimAlongRouteArgs::new(&nodes[0], route, keysend_preimage));
 	assert!(res.is_some());
 	assert_eq!(res, Some(PaidBolt12Invoice::StaticInvoice(static_invoice)));

lightning/src/ln/channel.rs

@@ -402,7 +402,7 @@ impl OutboundHTLCState {
 enum OutboundHTLCOutcome {
 	/// We started always filling in the preimages here in 0.0.105, and the requirement
 	/// that the preimages always be filled in was added in 0.2.
-	Success(PaymentPreimage, #[allow(dead_code)] Option<AttributionData>),
+	Success(PaymentPreimage, Option<AttributionData>),
 	Failure(HTLCFailReason),
 }
 
@@ -1181,7 +1181,7 @@ pub(super) struct MonitorRestoreUpdates {
 	pub order: RAACommitmentOrder,
 	pub accepted_htlcs: Vec<(PendingHTLCInfo, u64)>,
 	pub failed_htlcs: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
-	pub finalized_claimed_htlcs: Vec<HTLCSource>,
+	pub finalized_claimed_htlcs: Vec<(HTLCSource, Option<AttributionData>)>,
 	pub pending_update_adds: Vec<msgs::UpdateAddHTLC>,
 	pub funding_broadcastable: Option<Transaction>,
 	pub channel_ready: Option<msgs::ChannelReady>,
@@ -2313,7 +2313,7 @@ where
 	// but need to handle this somehow or we run the risk of losing HTLCs!
 	monitor_pending_forwards: Vec<(PendingHTLCInfo, u64)>,
 	monitor_pending_failures: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
-	monitor_pending_finalized_fulfills: Vec<HTLCSource>,
+	monitor_pending_finalized_fulfills: Vec<(HTLCSource, Option<AttributionData>)>,
 	monitor_pending_update_adds: Vec<msgs::UpdateAddHTLC>,
 	monitor_pending_tx_signatures: Option<msgs::TxSignatures>,
 
@@ -6749,7 +6749,7 @@ where
 
 		self.mark_outbound_htlc_removed(
 			msg.htlc_id,
-			OutboundHTLCOutcome::Success(msg.payment_preimage, None),
+			OutboundHTLCOutcome::Success(msg.payment_preimage, msg.attribution_data.clone()),
 		)
 		.map(|htlc| {
 			(htlc.source.clone(), htlc.amount_msat, htlc.skimmed_fee_msat, htlc.send_timestamp)
@@ -7554,8 +7554,11 @@ where
 							});
 							revoked_htlcs.push((htlc.source.clone(), htlc.payment_hash, reason));
 						},
-						OutboundHTLCOutcome::Success(..) => {
-							finalized_claimed_htlcs.push(htlc.source.clone());
+						OutboundHTLCOutcome::Success(_, attribution_data) => {
+							// Even though a fast track was taken for fulfilled HTLCs to the incoming side, we still
+							// pass along attribution data here so that we can include hold time information in the
+							// final PaymentPathSuccessful events.
+							finalized_claimed_htlcs.push((htlc.source.clone(), attribution_data));
 							// They fulfilled, so we sent them money
 							value_to_self_msat_diff -= htlc.amount_msat as i64;
 						},
@@ -8048,7 +8051,7 @@ where
 		&mut self, resend_raa: bool, resend_commitment: bool, resend_channel_ready: bool,
 		mut pending_forwards: Vec<(PendingHTLCInfo, u64)>,
 		mut pending_fails: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
-		mut pending_finalized_claimed_htlcs: Vec<HTLCSource>,
+		mut pending_finalized_claimed_htlcs: Vec<(HTLCSource, Option<AttributionData>)>,
 	) {
 		self.context.monitor_pending_revoke_and_ack |= resend_raa;
 		self.context.monitor_pending_commitment_signed |= resend_commitment;

lightning/src/ln/channelmanager.rs

@@ -78,7 +78,7 @@ use crate::ln::onion_payment::{
 };
 use crate::ln::onion_utils::{self};
 use crate::ln::onion_utils::{process_fulfill_attribution_data, AttributionData};
-use crate::ln::onion_utils::{HTLCFailReason, LocalHTLCFailureReason};
+use crate::ln::onion_utils::{process_onion_success, HTLCFailReason, LocalHTLCFailureReason};
 use crate::ln::our_peer_storage::EncryptedOurPeerStorage;
 #[cfg(test)]
 use crate::ln::outbound_payment;
@@ -8002,8 +8002,30 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 		);
 	}
 
-	fn finalize_claims(&self, sources: Vec<HTLCSource>) {
-		self.pending_outbound_payments.finalize_claims(sources, &self.pending_events);
+	fn finalize_claims(&self, sources: Vec<(HTLCSource, Option<AttributionData>)>) {
+		// Decode attribution data to hold times.
+		let hold_times = sources
+			.into_iter()
+			.filter_map(|(source, attribution_data)| {
+				if let HTLCSource::OutboundRoute { ref session_priv, ref path, .. } = source {
+					let hold_times = attribution_data.map(|attribution_data| {
+						process_onion_success(
+							&self.secp_ctx,
+							&self.logger,
+							path,
+							session_priv,
+							attribution_data,
+						)
+					});
+
+					Some((source, hold_times))
+				} else {
+					None
+				}
+			})
+			.collect();
+
+		self.pending_outbound_payments.finalize_claims(hold_times, &self.pending_events);
 	}
 
 	fn claim_funds_internal(
@@ -16723,15 +16745,15 @@ mod tests {
 		let events = nodes[0].node.get_and_clear_pending_events();
 		assert_eq!(events.len(), 2);
 		match events[0] {
-			Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path } => {
+			Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path, .. } => {
 				assert_eq!(payment_id, *actual_payment_id);
 				assert_eq!(our_payment_hash, *payment_hash.as_ref().unwrap());
 				assert_eq!(route.paths[0], *path);
 			},
 			_ => panic!("Unexpected event"),
 		}
 		match events[1] {
-			Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path } => {
+			Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path, ..} => {
 				assert_eq!(payment_id, *actual_payment_id);
 				assert_eq!(our_payment_hash, *payment_hash.as_ref().unwrap());
 				assert_eq!(route.paths[0], *path);

lightning/src/ln/functional_test_utils.rs

@@ -2839,11 +2839,13 @@ macro_rules! expect_payment_claimed {
 	};
 }
 
+/// Inspect events to assert that a payment was sent. If this was a BOLT 12 payment, the BOLT 12 invoice is returned. If
+/// per-path claims are expected, the events for each path are returned as well.
 pub fn expect_payment_sent<CM: AChannelManager, H: NodeHolder<CM = CM>>(
 	node: &H, expected_payment_preimage: PaymentPreimage,
 	expected_fee_msat_opt: Option<Option<u64>>, expect_per_path_claims: bool,
 	expect_post_ev_mon_update: bool,
-) -> Option<PaidBolt12Invoice> {
+) -> (Option<PaidBolt12Invoice>, Vec<Event>) {
 	let events = node.node().get_and_clear_pending_events();
 	let expected_payment_hash = PaymentHash(
 		bitcoin::hashes::sha256::Hash::hash(&expected_payment_preimage.0).to_byte_array(),
@@ -2858,6 +2860,7 @@ pub fn expect_payment_sent<CM: AChannelManager, H: NodeHolder<CM = CM>>(
 	}
 	// We return the invoice because some test may want to check the invoice details.
 	let invoice;
+	let mut path_events = Vec::new();
 	let expected_payment_id = match events[0] {
 		Event::PaymentSent {
 			ref payment_id,
@@ -2886,12 +2889,14 @@ pub fn expect_payment_sent<CM: AChannelManager, H: NodeHolder<CM = CM>>(
 				Event::PaymentPathSuccessful { payment_id, payment_hash, .. } => {
 					assert_eq!(payment_id, expected_payment_id);
 					assert_eq!(payment_hash, Some(expected_payment_hash));
+
+					path_events.push(events[i].clone());
 				},
 				_ => panic!("Unexpected event"),
 			}
 		}
 	}
-	invoice
+	(invoice, path_events)
 }
 
 #[macro_export]
@@ -3903,15 +3908,17 @@ pub fn pass_claimed_payment_along_route(args: ClaimAlongRouteArgs) -> u64 {
 
 	expected_total_fee_msat
 }
-pub fn claim_payment_along_route(args: ClaimAlongRouteArgs) -> Option<PaidBolt12Invoice> {
+pub fn claim_payment_along_route(
+	args: ClaimAlongRouteArgs,
+) -> (Option<PaidBolt12Invoice>, Vec<Event>) {
 	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, payment_preimage, Some(expected_total_fee_msat))
 	} else {
-		None
+		(None, Vec::new())
 	}
 }
 
@@ -3924,6 +3931,7 @@ pub fn claim_payment<'a, 'b, 'c>(
 		&[expected_route],
 		our_payment_preimage,
 	))
+	.0
 }
 
 pub const TEST_FINAL_CLTV: u32 = 70;

lightning/src/ln/mod.rs

@@ -54,6 +54,8 @@ pub use onion_utils::{create_payment_onion, LocalHTLCFailureReason};
 
 #[cfg(fuzzing)]
 pub use onion_utils::process_onion_failure;
+#[cfg(fuzzing)]
+pub use onion_utils::process_onion_success;
 
 #[cfg(fuzzing)]
 pub use onion_utils::AttributionData;