Add hold times to update_fulfill_htlc

Author: joostjager

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -2511,6 +2511,7 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
 		channel_id: chan_id_2,
 		htlc_id: 0,
 		payment_preimage,
+		attribution_data: None,
 	};
 	if second_fails {
 		nodes[2].node.fail_htlc_backwards(&payment_hash);
@@ -2524,8 +2525,11 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
 
 		let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
 		assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
-		// Check that the message we're about to deliver matches the one generated:
-		assert_eq!(fulfill_msg, cs_updates.update_fulfill_htlcs[0]);
+
+		// Check that the message we're about to deliver matches the one generated. Ignore attribution data.
+		assert_eq!(fulfill_msg.channel_id, cs_updates.update_fulfill_htlcs[0].channel_id);
+		assert_eq!(fulfill_msg.htlc_id, cs_updates.update_fulfill_htlcs[0].htlc_id);
+		assert_eq!(fulfill_msg.payment_preimage, cs_updates.update_fulfill_htlcs[0].payment_preimage);
 	}
 	nodes[1].node.handle_update_fulfill_htlc(nodes[2].node.get_our_node_id(), &fulfill_msg);
 	expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(1000), false, false);

lightning/src/ln/channel.rs

@@ -121,7 +121,7 @@ enum FeeUpdateState {
 enum InboundHTLCRemovalReason {
 	FailRelay(msgs::OnionErrorPacket),
 	FailMalformed(([u8; 32], u16)),
-	Fulfill(PaymentPreimage),
+	Fulfill(PaymentPreimage, Option<AttributionData>),
 }
 
 /// Represents the resolution status of an inbound HTLC.
@@ -220,7 +220,7 @@ impl From<&InboundHTLCState> for Option<InboundHTLCStateDetails> {
 				Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFail),
 			InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailMalformed(_)) =>
 				Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFail),
-			InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_)) =>
+			InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_, _)) =>
 				Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFulfill),
 		}
 	}
@@ -251,7 +251,7 @@ impl InboundHTLCState {
 
 	fn preimage(&self) -> Option<PaymentPreimage> {
 		match self {
-			InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(preimage)) => Some(*preimage),
+			InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(preimage, _)) => Some(*preimage),
 			_ => None,
 		}
 	}
@@ -5347,7 +5347,7 @@ impl<SP: Deref> FundedChannel<SP> where
 		// (see equivalent if condition there).
 		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);
+		let fulfill_resp = self.get_update_fulfill_htlc(htlc_id_arg, payment_preimage_arg, None, AttributionData::new(), logger);
 		self.context.latest_monitor_update_id = mon_update_id;
 		if let UpdateFulfillFetch::NewClaim { msg, .. } = fulfill_resp {
 			assert!(!msg); // The HTLC must have ended up in the holding cell.
@@ -5356,7 +5356,7 @@ impl<SP: Deref> FundedChannel<SP> 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: AttributionData, logger: &L,
 	) -> UpdateFulfillFetch where L::Target: Logger {
 		// Either ChannelReady got set (which means it won't be unset) or there is no way any
 		// caller thought we could have something claimed (cause we wouldn't have accepted in an
@@ -5380,7 +5380,7 @@ impl<SP: Deref> FundedChannel<SP> where
 				match htlc.state {
 					InboundHTLCState::Committed => {},
 					InboundHTLCState::LocalRemoved(ref reason) => {
-						if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
+						if let &InboundHTLCRemovalReason::Fulfill(_, _) = reason {
 						} else {
 							log_warn!(logger, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", &htlc.payment_hash, &self.context.channel_id());
 							debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
@@ -5458,7 +5458,7 @@ impl<SP: Deref> FundedChannel<SP> where
 				return UpdateFulfillFetch::NewClaim { monitor_update, htlc_value_msat, msg: false };
 			}
 			log_trace!(logger, "Upgrading HTLC {} to LocalRemoved with a Fulfill in channel {}!", &htlc.payment_hash, &self.context.channel_id);
-			htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(payment_preimage_arg.clone()));
+			htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(payment_preimage_arg.clone(), Some(attribution_data)));
 		}
 
 		UpdateFulfillFetch::NewClaim {
@@ -5470,10 +5470,10 @@ impl<SP: Deref> FundedChannel<SP> 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: 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, msg } => {
 				// Even if we aren't supposed to let new monitor updates with commitment state
 				// updates run, we still need to push the preimage ChannelMonitorUpdateStep no
@@ -5848,7 +5848,7 @@ impl<SP: Deref> FundedChannel<SP> where
 		Err(ChannelError::close("Remote tried to fulfill/fail an HTLC we couldn't find".to_owned()))
 	}
 
-	pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<(HTLCSource, u64, Option<u64>), ChannelError> {
+	pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<(HTLCSource, u64, Option<u64>, Option<Duration>), ChannelError> {
 		if self.context.channel_state.is_remote_stfu_sent() || self.context.channel_state.is_quiescent() {
 			return Err(ChannelError::WarnAndDisconnect("Got fulfill HTLC message while quiescent".to_owned()));
 		}
@@ -5859,7 +5859,7 @@ impl<SP: Deref> FundedChannel<SP> where
 			return Err(ChannelError::close("Peer sent update_fulfill_htlc when we needed a channel_reestablish".to_owned()));
 		}
 
-		self.mark_outbound_htlc_removed(msg.htlc_id, OutboundHTLCOutcome::Success(msg.payment_preimage)).map(|htlc| (htlc.source.clone(), htlc.amount_msat, htlc.skimmed_fee_msat))
+		self.mark_outbound_htlc_removed(msg.htlc_id, OutboundHTLCOutcome::Success(msg.payment_preimage)).map(|htlc| (htlc.source.clone(), htlc.amount_msat, htlc.skimmed_fee_msat, htlc.send_timestamp))
 	}
 
 	pub fn update_fail_htlc(&mut self, msg: &msgs::UpdateFailHTLC, fail_reason: HTLCFailReason) -> Result<(), ChannelError> {
@@ -6201,7 +6201,7 @@ impl<SP: Deref> FundedChannel<SP> where
 						// We do not bother to track and include `payment_info` here, however.
 						let mut additional_monitor_update =
 							if let UpdateFulfillFetch::NewClaim { monitor_update, .. } =
-								self.get_update_fulfill_htlc(htlc_id, *payment_preimage, None, logger)
+								self.get_update_fulfill_htlc(htlc_id, *payment_preimage, None, AttributionData::new(), logger) // TODO: Attr data from holding cell
 							{ monitor_update } else { unreachable!() };
 						update_fulfill_count += 1;
 						monitor_update.updates.append(&mut additional_monitor_update.updates);
@@ -6366,7 +6366,7 @@ impl<SP: Deref> FundedChannel<SP> where
 			pending_inbound_htlcs.retain(|htlc| {
 				if let &InboundHTLCState::LocalRemoved(ref reason) = &htlc.state {
 					log_trace!(logger, " ...removing inbound LocalRemoved {}", &htlc.payment_hash);
-					if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
+					if let &InboundHTLCRemovalReason::Fulfill(_, _) = reason {
 						value_to_self_msat_diff += htlc.amount_msat as i64;
 					}
 					*expecting_peer_commitment_signed = true;
@@ -7139,11 +7139,12 @@ impl<SP: Deref> FundedChannel<SP> where
 							failure_code: failure_code.clone(),
 						});
 					},
-					&InboundHTLCRemovalReason::Fulfill(ref payment_preimage) => {
+					&InboundHTLCRemovalReason::Fulfill(ref payment_preimage, ref attribution_data) => {
 						update_fulfill_htlcs.push(msgs::UpdateFulfillHTLC {
 							channel_id: self.context.channel_id(),
 							htlc_id: htlc.htlc_id,
 							payment_preimage: payment_preimage.clone(),
+							attribution_data: attribution_data.clone(),
 						});
 					},
 				}
@@ -10642,7 +10643,7 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
 							1u8.write(writer)?;
 							(hash, code).write(writer)?;
 						},
-						InboundHTLCRemovalReason::Fulfill(preimage) => {
+						InboundHTLCRemovalReason::Fulfill(preimage, _) => { // TODO: Persistence
 							2u8.write(writer)?;
 							preimage.write(writer)?;
 						},
@@ -11003,7 +11004,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 								attribution_data: None,
 							}),
 							1 => InboundHTLCRemovalReason::FailMalformed(Readable::read(reader)?),
-							2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?),
+							2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?, None), // TODO: Persistence
 							_ => return Err(DecodeError::InvalidValue),
 						};
 						InboundHTLCState::LocalRemoved(reason)
@@ -11584,7 +11585,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 	}
 }
 
-fn duration_since_epoch() -> Option<Duration> {
+pub(crate) fn duration_since_epoch() -> Option<Duration> {
 	#[cfg(not(feature = "std"))]
 	let now = None;
 

lightning/src/ln/channelmanager.rs

@@ -50,7 +50,7 @@ use crate::events::{self, Event, EventHandler, EventsProvider, InboundChannelFun
 use crate::ln::inbound_payment;
 use crate::ln::types::ChannelId;
 use crate::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret};
-use crate::ln::channel::{self, Channel, ChannelError, ChannelUpdateStatus, FundedChannel, ShutdownResult, UpdateFulfillCommitFetch, OutboundV1Channel, ReconnectionMsg, InboundV1Channel, WithChannelContext};
+use crate::ln::channel::{self, duration_since_epoch, Channel, ChannelError, ChannelUpdateStatus, FundedChannel, InboundV1Channel, OutboundV1Channel, ReconnectionMsg, ShutdownResult, UpdateFulfillCommitFetch, WithChannelContext};
 use crate::ln::channel::PendingV2Channel;
 use crate::ln::channel_state::ChannelDetails;
 use crate::types::features::{Bolt12InvoiceFeatures, ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
@@ -59,7 +59,7 @@ use crate::types::features::Bolt11InvoiceFeatures;
 use crate::routing::router::{BlindedTail, InFlightHtlcs, Path, Payee, PaymentParameters, RouteParameters, RouteParametersConfig, Router, FixedRouter, Route};
 use crate::ln::onion_payment::{check_incoming_htlc_cltv, create_recv_pending_htlc_info, create_fwd_pending_htlc_info, decode_incoming_update_add_htlc_onion, HopConnector, InboundHTLCErr, NextPacketDetails, invalid_payment_err_data};
 use crate::ln::msgs;
-use crate::ln::onion_utils::{self};
+use crate::ln::onion_utils::{self, process_onion_success, HOLD_TIME_LEN};
 use crate::ln::onion_utils::{HTLCFailReason, LocalHTLCFailureReason};
 use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, CommitmentUpdate, DecodeError, LightningError, MessageSendEvent};
 #[cfg(test)]
@@ -88,6 +88,8 @@ use crate::util::ser::{BigSize, FixedLengthReader, LengthReadable, Readable, Rea
 use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::errors::APIError;
 
+use crate::ln::onion_utils::AttributionData;
+
 #[cfg(async_payments)] use {
 	crate::offers::offer::Amount,
 	crate::offers::static_invoice::{DEFAULT_RELATIVE_EXPIRY as STATIC_INVOICE_DEFAULT_RELATIVE_EXPIRY, StaticInvoice, StaticInvoiceBuilder},
@@ -7239,8 +7241,14 @@ where
 						pending_claim: PendingMPPClaimPointer(Arc::clone(pending_claim)),
 					}
 				});
+
+				let mut attribution_data = AttributionData::new();
+				attribution_data.update(&[], &htlc.prev_hop.incoming_packet_shared_secret, 0);
+				attribution_data.crypt(&htlc.prev_hop.incoming_packet_shared_secret);
+
 				self.claim_funds_from_hop(
 					htlc.prev_hop, payment_preimage, payment_info.clone(),
+					attribution_data,
 					|_, definitely_duplicate| {
 						debug_assert!(!definitely_duplicate, "We shouldn't claim duplicatively from a payment");
 						(Some(MonitorUpdateCompletionAction::PaymentClaimed { payment_hash, pending_mpp_claim: this_mpp_claim }), raa_blocker)
@@ -7269,7 +7277,7 @@ where
 		ComplFunc: FnOnce(Option<u64>, bool) -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>)
 	>(
 		&self, prev_hop: HTLCPreviousHopData, payment_preimage: PaymentPreimage,
-		payment_info: Option<PaymentClaimDetails>, completion_action: ComplFunc,
+		payment_info: Option<PaymentClaimDetails>, attribution_data: 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();
@@ -7282,15 +7290,17 @@ 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<
 		ComplFunc: FnOnce(Option<u64>, bool) -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>)
 	>(
 		&self, prev_hop: HTLCClaimSource, payment_preimage: PaymentPreimage,
-		payment_info: Option<PaymentClaimDetails>, completion_action: ComplFunc,
+		payment_info: Option<PaymentClaimDetails>, attribution_data: AttributionData, completion_action: ComplFunc,
 	) {
+		log_info!(self.logger, "claim_mpp_part called");
+
 		//TODO: Delay the claimed_funds relaying just like we do outbound relay!
 
 		// If we haven't yet run background events assume we're still deserializing and shouldn't
@@ -7322,7 +7332,7 @@ where
 			if let hash_map::Entry::Occupied(mut chan_entry) = peer_state.channel_by_id.entry(chan_id) {
 				if let Some(chan) = chan_entry.get_mut().as_funded_mut() {
 					let logger = WithChannelContext::from(&self.logger, &chan.context, None);
-					let fulfill_res = chan.get_update_fulfill_htlc_and_commit(prev_hop.htlc_id, payment_preimage, payment_info, &&logger);
+					let fulfill_res = chan.get_update_fulfill_htlc_and_commit(prev_hop.htlc_id, payment_preimage, payment_info, attribution_data, &&logger);
 
 					match fulfill_res {
 						UpdateFulfillCommitFetch::NewClaim { htlc_value_msat, monitor_update } => {
@@ -7474,9 +7484,16 @@ 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>,
+		attribution_data: Option<&AttributionData>, send_timestamp: Option<Duration>,
 	) {
+		log_info!(self.logger, "claim_funds_internal - ONLY NON FINAL");
 		match source {
 			HTLCSource::OutboundRoute { session_priv, payment_id, path, bolt12_invoice, .. } => {
+				if let Some(attribution_data) = attribution_data {
+					process_onion_success(&self.secp_ctx, &self.logger, &path,
+						&session_priv, attribution_data.clone());
+				}
+
 				debug_assert!(self.background_events_processed_since_startup.load(Ordering::Acquire),
 					"We don't support claim_htlc claims during startup - monitors may not be available yet");
 				debug_assert_eq!(next_channel_counterparty_node_id, path.hops[0].pubkey);
@@ -7493,7 +7510,27 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 				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,
+
+				let mut attribution_data = attribution_data
+					.map_or(AttributionData::new(), |attribution_data| {
+						let mut attribution_data = attribution_data.clone();
+
+						attribution_data.shift_right();
+
+						attribution_data
+					});
+
+				let now = duration_since_epoch();
+				let hold_time = if let (Some(timestamp), Some(now)) = (send_timestamp, now) {
+					u32::try_from(now.saturating_sub(timestamp).as_millis()).unwrap_or(u32::MAX)
+				} else {
+					0
+				};
+
+				attribution_data.update(&[], &hop_data.incoming_packet_shared_secret, hold_time);
+				attribution_data.crypt(&hop_data.incoming_packet_shared_secret);
+
+				self.claim_funds_from_hop(hop_data, payment_preimage, None, attribution_data,
 					|htlc_claim_value_msat, definitely_duplicate| {
 						let chan_to_release = Some(EventUnblockedChannel {
 							counterparty_node_id: next_channel_counterparty_node_id,
@@ -8896,7 +8933,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 	fn internal_update_fulfill_htlc(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFulfillHTLC) -> 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(|| {
@@ -8937,7 +8974,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 		};
 		self.claim_funds_internal(htlc_source, msg.payment_preimage.clone(),
 			Some(forwarded_htlc_value), skimmed_fee_msat, false, false, *counterparty_node_id,
-			funding_txo, msg.channel_id, Some(next_user_channel_id),
+			funding_txo, msg.channel_id, Some(next_user_channel_id), msg.attribution_data.as_ref(),
+			send_timestamp,
 		);
 
 		Ok(())
@@ -9638,6 +9676,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
 								htlc_update.source, preimage,
 								htlc_update.htlc_value_satoshis.map(|v| v * 1000), None, true,
 								false, counterparty_node_id, funding_outpoint, channel_id, None,
+								None, None,
 							);
 						} else {
 							log_trace!(logger, "Failing HTLC with hash {} from our monitor", &htlc_update.payment_hash);
@@ -12154,6 +12193,7 @@ where
 	}
 
 	fn handle_update_fulfill_htlc(&self, counterparty_node_id: PublicKey, msg: &msgs::UpdateFulfillHTLC) {
+		log_info!(self.logger, "Received update_fulfill_htlc: {:?}", msg);
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 		let _ = handle_error!(self, self.internal_update_fulfill_htlc(&counterparty_node_id, msg), counterparty_node_id);
 	}
@@ -14905,7 +14945,7 @@ where
 							// already (clearly) durably on disk in the `ChannelMonitor` so there's
 							// no need to worry about getting it into others.
 							channel_manager.claim_mpp_part(
-								part.into(), payment_preimage, None,
+								part.into(), payment_preimage, None, AttributionData::new(),
 								|_, _|
 									(Some(MonitorUpdateCompletionAction::PaymentClaimed { payment_hash, pending_mpp_claim }), pending_claim_ptr)
 							);
@@ -15011,7 +15051,7 @@ where
 			// channel is closed we just assume that it probably came from an on-chain claim.
 			channel_manager.claim_funds_internal(source, preimage, Some(downstream_value), None,
 				downstream_closed, true, downstream_node_id, downstream_funding,
-				downstream_channel_id, None
+				downstream_channel_id, None, None, None,
 			);
 		}
 

lightning/src/ln/htlc_reserve_unit_tests.rs

@@ -1785,6 +1785,7 @@ pub fn test_update_fulfill_htlc_bolt2_update_fulfill_htlc_before_commitment() {
 		channel_id: chan.2,
 		htlc_id: 0,
 		payment_preimage: our_payment_preimage,
+		attribution_data: None,
 	};
 
 	nodes[0].node.handle_update_fulfill_htlc(node_b_id, &update_msg);