Hold time reporting

Adds hold time reporting for the final and intermediate nodes.

Author: joostjager

lightning/src/ln/blinded_payment_tests.rs

@@ -2333,6 +2333,8 @@ fn test_trampoline_unblinded_receive() {
 			None,
 		).unwrap();
 
+		// Use a different session key to construct the replacement onion packet. Note that the sender isn't aware of
+		// this and won't be able to decode the fulfill hold times.
 		let outer_session_priv = secret_from_hex("e52c20461ed7acd46c4e7b591a37610519179482887bd73bf3b94617f8f03677");
 
 		let (outer_payloads, _, _) = onion_utils::build_onion_payloads(&route.paths[0], outer_total_msat, &recipient_onion_fields, outer_starting_htlc_offset, &None, None, Some(trampoline_packet)).unwrap();

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -2890,7 +2890,7 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
 		as_raa = Some(get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, node_b_id));
 	}
 
-	let fulfill_msg = msgs::UpdateFulfillHTLC {
+	let mut fulfill_msg = msgs::UpdateFulfillHTLC {
 		channel_id: chan_id_2,
 		htlc_id: 0,
 		payment_preimage,
@@ -2904,6 +2904,8 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
 		);
 		check_added_monitors!(nodes[2], 1);
 		get_htlc_update_msgs!(nodes[2], node_b_id);
+	// Note that we don't populate fulfill_msg.attribution_data here, which will lead to hold times being
+	// unavailable.
 	} else {
 		nodes[2].node.claim_funds(payment_preimage);
 		check_added_monitors!(nodes[2], 1);
@@ -2919,6 +2921,7 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
 			fulfill_msg.payment_preimage,
 			cs_updates.update_fulfill_htlcs[0].payment_preimage
 		);
+		fulfill_msg.attribution_data = cs_updates.update_fulfill_htlcs[0].attribution_data.clone();
 	}
 	nodes[1].node.handle_update_fulfill_htlc(node_c_id, &fulfill_msg);
 	expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(1000), false, false);

lightning/src/ln/channel.rs

@@ -6166,7 +6166,7 @@ where
 		assert!(!self.context.channel_state.can_generate_new_commitment());
 		let mon_update_id = self.context.latest_monitor_update_id; // Forget the ChannelMonitor update
 		let fulfill_resp =
-			self.get_update_fulfill_htlc(htlc_id_arg, payment_preimage_arg, None, logger);
+			self.get_update_fulfill_htlc(htlc_id_arg, payment_preimage_arg, None, None, logger);
 		self.context.latest_monitor_update_id = mon_update_id;
 		if let UpdateFulfillFetch::NewClaim { update_blocked, .. } = fulfill_resp {
 			assert!(update_blocked); // The HTLC must have ended up in the holding cell.
@@ -6175,7 +6175,8 @@ where
 
 	fn get_update_fulfill_htlc<L: Deref>(
 		&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage,
-		payment_info: Option<PaymentClaimDetails>, logger: &L,
+		payment_info: Option<PaymentClaimDetails>, attribution_data: Option<AttributionData>,
+		logger: &L,
 	) -> UpdateFulfillFetch
 	where
 		L::Target: Logger,
@@ -6290,7 +6291,7 @@ where
 			self.context.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::ClaimHTLC {
 				payment_preimage: payment_preimage_arg,
 				htlc_id: htlc_id_arg,
-				attribution_data: None,
+				attribution_data,
 			});
 			return UpdateFulfillFetch::NewClaim {
 				monitor_update,
@@ -6321,7 +6322,7 @@ where
 			);
 			htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(
 				payment_preimage_arg.clone(),
-				None,
+				attribution_data,
 			));
 		}
 
@@ -6330,13 +6331,20 @@ where
 
 	pub fn get_update_fulfill_htlc_and_commit<L: Deref>(
 		&mut self, htlc_id: u64, payment_preimage: PaymentPreimage,
-		payment_info: Option<PaymentClaimDetails>, logger: &L,
+		payment_info: Option<PaymentClaimDetails>, attribution_data: Option<AttributionData>,
+		logger: &L,
 	) -> UpdateFulfillCommitFetch
 	where
 		L::Target: Logger,
 	{
 		let release_cs_monitor = self.context.blocked_monitor_updates.is_empty();
-		match self.get_update_fulfill_htlc(htlc_id, payment_preimage, payment_info, logger) {
+		match self.get_update_fulfill_htlc(
+			htlc_id,
+			payment_preimage,
+			payment_info,
+			attribution_data,
+			logger,
+		) {
 			UpdateFulfillFetch::NewClaim {
 				mut monitor_update,
 				htlc_value_msat,
@@ -6668,7 +6676,7 @@ where
 
 	pub fn update_fulfill_htlc(
 		&mut self, msg: &msgs::UpdateFulfillHTLC,
-	) -> Result<(HTLCSource, u64, Option<u64>), ChannelError> {
+	) -> Result<(HTLCSource, u64, Option<u64>, Option<Duration>), ChannelError> {
 		if self.context.channel_state.is_remote_stfu_sent()
 			|| self.context.channel_state.is_quiescent()
 		{
@@ -6688,8 +6696,9 @@ where
 		}
 
 		let outcome = OutboundHTLCOutcome::Success(msg.payment_preimage);
-		self.mark_outbound_htlc_removed(msg.htlc_id, outcome)
-			.map(|htlc| (htlc.source.clone(), htlc.amount_msat, htlc.skimmed_fee_msat))
+		self.mark_outbound_htlc_removed(msg.htlc_id, outcome).map(|htlc| {
+			(htlc.source.clone(), htlc.amount_msat, htlc.skimmed_fee_msat, htlc.send_timestamp)
+		})
 	}
 
 	#[rustfmt::skip]
@@ -7249,7 +7258,11 @@ where
 						}
 						None
 					},
-					&HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, htlc_id, .. } => {
+					&HTLCUpdateAwaitingACK::ClaimHTLC {
+						ref payment_preimage,
+						htlc_id,
+						ref attribution_data,
+					} => {
 						// If an HTLC claim was previously added to the holding cell (via
 						// `get_update_fulfill_htlc`, then generating the claim message itself must
 						// not fail - any in between attempts to claim the HTLC will have resulted
@@ -7260,8 +7273,13 @@ where
 						// `ChannelMonitorUpdate` to the user, making this one redundant, however
 						// there's no harm in including the extra `ChannelMonitorUpdateStep` here.
 						// We do not bother to track and include `payment_info` here, however.
-						let fulfill =
-							self.get_update_fulfill_htlc(htlc_id, *payment_preimage, None, logger);
+						let fulfill = self.get_update_fulfill_htlc(
+							htlc_id,
+							*payment_preimage,
+							None,
+							attribution_data.clone(),
+							logger,
+						);
 						let mut additional_monitor_update =
 							if let UpdateFulfillFetch::NewClaim { monitor_update, .. } = fulfill {
 								monitor_update
@@ -13565,7 +13583,7 @@ where
 	}
 }
 
-fn duration_since_epoch() -> Option<Duration> {
+pub(crate) fn duration_since_epoch() -> Option<Duration> {
 	#[cfg(not(feature = "std"))]
 	let now = None;
 
@@ -13581,7 +13599,7 @@ fn duration_since_epoch() -> Option<Duration> {
 
 /// Returns the time expressed in hold time units (1 unit = 100 ms) that has elapsed between send_timestamp and now. If
 /// any of the arguments are `None`, returns `None`.
-fn hold_time(send_timestamp: Option<Duration>, now: Option<Duration>) -> Option<u32> {
+pub(crate) fn hold_time(send_timestamp: Option<Duration>, now: Option<Duration>) -> Option<u32> {
 	send_timestamp.and_then(|t| {
 		now.map(|now| {
 			let elapsed = now.saturating_sub(t).as_millis() / HOLD_TIME_UNIT_MILLIS;

lightning/src/ln/channelmanager.rs

@@ -58,12 +58,12 @@ use crate::events::{
 use crate::events::{FundingInfo, PaidBolt12Invoice};
 // Since this struct is returned in `list_channels` methods, expose it here in case users want to
 // construct one themselves.
-use crate::ln::channel::PendingV2Channel;
 use crate::ln::channel::{
-	self, Channel, ChannelError, ChannelUpdateStatus, FundedChannel, InboundV1Channel,
+	self, hold_time, Channel, ChannelError, ChannelUpdateStatus, FundedChannel, InboundV1Channel,
 	OutboundV1Channel, ReconnectionMsg, ShutdownResult, UpdateFulfillCommitFetch,
 	WithChannelContext,
 };
+use crate::ln::channel::{duration_since_epoch, PendingV2Channel};
 use crate::ln::channel_state::ChannelDetails;
 use crate::ln::inbound_payment;
 use crate::ln::msgs;
@@ -77,6 +77,7 @@ use crate::ln::onion_payment::{
 	NextPacketDetails,
 };
 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::our_peer_storage::EncryptedOurPeerStorage;
 #[cfg(test)]
@@ -7639,10 +7640,30 @@ where
 						pending_claim: PendingMPPClaimPointer(Arc::clone(pending_claim)),
 					}
 				});
+
+				// Create new attribution data as the final hop. Always report a zero hold time, because reporting a
+				// non-zero value will not make a difference in the penalty that may be applied by the sender. If there
+				// is a phantom hop, we need to double-process.
+				let attribution_data =
+					if let Some(phantom_secret) = htlc.prev_hop.phantom_shared_secret {
+						let attribution_data =
+							process_fulfill_attribution_data(None, &phantom_secret, 0);
+						Some(attribution_data)
+					} else {
+						None
+					};
+
+				let attribution_data = process_fulfill_attribution_data(
+					attribution_data.as_ref(),
+					&htlc.prev_hop.incoming_packet_shared_secret,
+					0,
+				);
+
 				self.claim_funds_from_hop(
 					htlc.prev_hop,
 					payment_preimage,
 					payment_info.clone(),
+					Some(attribution_data),
 					|_, definitely_duplicate| {
 						debug_assert!(
 							!definitely_duplicate,
@@ -7687,7 +7708,8 @@ where
 		) -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),
 	>(
 		&self, prev_hop: HTLCPreviousHopData, payment_preimage: PaymentPreimage,
-		payment_info: Option<PaymentClaimDetails>, completion_action: ComplFunc,
+		payment_info: Option<PaymentClaimDetails>, attribution_data: Option<AttributionData>,
+		completion_action: ComplFunc,
 	) {
 		let counterparty_node_id = prev_hop.counterparty_node_id.or_else(|| {
 			let short_to_chan_info = self.short_to_chan_info.read().unwrap();
@@ -7700,7 +7722,13 @@ where
 			channel_id: prev_hop.channel_id,
 			htlc_id: prev_hop.htlc_id,
 		};
-		self.claim_mpp_part(htlc_source, payment_preimage, payment_info, completion_action)
+		self.claim_mpp_part(
+			htlc_source,
+			payment_preimage,
+			payment_info,
+			attribution_data,
+			completion_action,
+		)
 	}
 
 	fn claim_mpp_part<
@@ -7710,7 +7738,8 @@ where
 		) -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),
 	>(
 		&self, prev_hop: HTLCClaimSource, payment_preimage: PaymentPreimage,
-		payment_info: Option<PaymentClaimDetails>, completion_action: ComplFunc,
+		payment_info: Option<PaymentClaimDetails>, attribution_data: Option<AttributionData>,
+		completion_action: ComplFunc,
 	) {
 		//TODO: Delay the claimed_funds relaying just like we do outbound relay!
 
@@ -7751,6 +7780,7 @@ where
 						prev_hop.htlc_id,
 						payment_preimage,
 						payment_info,
+						attribution_data,
 						&&logger,
 					);
 
@@ -7959,7 +7989,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 		forwarded_htlc_value_msat: Option<u64>, skimmed_fee_msat: Option<u64>, from_onchain: bool,
 		startup_replay: bool, next_channel_counterparty_node_id: PublicKey,
 		next_channel_outpoint: OutPoint, next_channel_id: ChannelId,
-		next_user_channel_id: Option<u128>,
+		next_user_channel_id: Option<u128>, attribution_data: Option<&AttributionData>,
+		send_timestamp: Option<Duration>,
 	) {
 		match source {
 			HTLCSource::OutboundRoute {
@@ -7991,10 +8022,25 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 				let prev_node_id = hop_data.counterparty_node_id;
 				let completed_blocker =
 					RAAMonitorUpdateBlockingAction::from_prev_hop_data(&hop_data);
+
+				// Obtain hold time, if available.
+				let now = duration_since_epoch();
+				let hold_time = hold_time(send_timestamp, now).unwrap_or(0);
+
+				// If attribution data was received from downstream, we shift it and get it ready for adding our hold
+				// time. Note that fulfilled HTLCs take a fast path to the incoming side. We don't need to wait for RAA
+				// to record the hold time like we do for failed HTLCs.
+				let attribution_data = process_fulfill_attribution_data(
+					attribution_data,
+					&hop_data.incoming_packet_shared_secret,
+					hold_time,
+				);
+
 				self.claim_funds_from_hop(
 					hop_data,
 					payment_preimage,
 					None,
+					Some(attribution_data),
 					|htlc_claim_value_msat, definitely_duplicate| {
 						let chan_to_release = Some(EventUnblockedChannel {
 							counterparty_node_id: next_channel_counterparty_node_id,
@@ -9553,7 +9599,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 	) -> Result<(), MsgHandleErrInternal> {
 		let funding_txo;
 		let next_user_channel_id;
-		let (htlc_source, forwarded_htlc_value, skimmed_fee_msat) = {
+		let (htlc_source, forwarded_htlc_value, skimmed_fee_msat, send_timestamp) = {
 			let per_peer_state = self.per_peer_state.read().unwrap();
 			let peer_state_mutex = per_peer_state.get(counterparty_node_id).ok_or_else(|| {
 				debug_assert!(false);
@@ -9608,6 +9654,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 			funding_txo,
 			msg.channel_id,
 			Some(next_user_channel_id),
+			msg.attribution_data.as_ref(),
+			send_timestamp,
 		);
 
 		Ok(())
@@ -10429,6 +10477,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 								"Claiming HTLC with preimage {} from our monitor",
 								preimage
 							);
+							// Claim the funds from the previous hop, if there is one. Because this is in response to a
+							// chain event, no attribution data is available.
 							self.claim_funds_internal(
 								htlc_update.source,
 								preimage,
@@ -10440,6 +10490,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 								funding_outpoint,
 								channel_id,
 								None,
+								None,
+								None,
 							);
 						} else {
 							log_trace!(
@@ -16293,10 +16345,14 @@ where
 							// Note that we don't need to pass the `payment_info` here - its
 							// already (clearly) durably on disk in the `ChannelMonitor` so there's
 							// no need to worry about getting it into others.
+							//
+							// We don't encode any attribution data, because the required onion shared secret isn't
+							// available here.
 							channel_manager.claim_mpp_part(
 								part.into(),
 								payment_preimage,
 								None,
+								None,
 								|_, _| {
 									(
 										Some(MonitorUpdateCompletionAction::PaymentClaimed {
@@ -16441,6 +16497,7 @@ where
 			// We use `downstream_closed` in place of `from_onchain` here just as a guess - we
 			// don't remember in the `ChannelMonitor` where we got a preimage from, but if the
 			// channel is closed we just assume that it probably came from an on-chain claim.
+			// The same holds for attribution data. We don't have any, so we pass an empty one.
 			channel_manager.claim_funds_internal(
 				source,
 				preimage,
@@ -16452,6 +16509,8 @@ where
 				downstream_funding,
 				downstream_channel_id,
 				None,
+				None,
+				None,
 			);
 		}
 

lightning/src/ln/functional_tests.rs

@@ -7748,7 +7748,8 @@ pub fn test_onion_value_mpp_set_calculation() {
 		assert_eq!(node.node.get_our_node_id(), payment_event.node_id);
 
 		if idx == 0 {
-			// routing node
+			// Manipulate the onion packet for the routing node. Note that we pick a dummy session_priv here. The sender
+			// won't be able to decode fulfill attribution data.
 			let session_priv = [3; 32];
 			let height = nodes[0].best_block_info().1;
 			let session_priv = SecretKey::from_slice(&session_priv).unwrap();