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);

lightning/src/ln/channel.rs

@@ -78,6 +78,7 @@ use crate::sync::Mutex;
 use crate::sign::type_resolver::ChannelSignerType;
 
 use super::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint, RevocationBasepoint};
+use super::onion_utils::{HMAC_COUNT, HOLD_TIME_LEN, MAX_HOPS, HMAC_LEN};
 
 #[cfg(any(test, feature = "_test_utils"))]
 #[allow(unused)]
@@ -121,7 +122,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 +221,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 +252,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 +5348,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, None, 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 +5357,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: Option<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 +5381,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 +5459,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(), attribution_data));
 		}
 
 		UpdateFulfillFetch::NewClaim {
@@ -5470,10 +5471,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: 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, 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
@@ -6201,7 +6202,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, None, 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 +6367,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 +7140,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 +10644,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 +11005,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)

lightning/src/ln/channelmanager.rs

@@ -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, build_failure_packet, process_onion_success, HMAC_COUNT, HMAC_LEN, HOLD_TIME_LEN, MAX_HOPS};
 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,19 @@ where
 						pending_claim: PendingMPPClaimPointer(Arc::clone(pending_claim)),
 					}
 				});
+
+				log_info!(self.logger, "ONLY ONCE");
+
+				let mut attribution_data = AttributionData::new();
+				let hold_time_bytes: [u8; 4] = (100 as u32).to_be_bytes();
+				attribution_data.hold_times[..HOLD_TIME_LEN].copy_from_slice(&hold_time_bytes);
+				attribution_data.add_hmacs(&htlc.prev_hop.incoming_packet_shared_secret, &[]);
+				attribution_data.crypt(&htlc.prev_hop.incoming_packet_shared_secret);
+
 				self.claim_funds_from_hop(
 					htlc.prev_hop, payment_preimage, payment_info.clone(),
+					Some(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 +7282,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: 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();
@@ -7282,15 +7295,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: Option<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 +7337,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 +7489,17 @@ 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>,
 	) {
+		log_info!(self.logger, "claim_funds_internal - ONLY NON FINAL");
 		match source {
 			HTLCSource::OutboundRoute { session_priv, payment_id, path, bolt12_invoice, .. } => {
+				// Log attribution data.
+				log_info!(self.logger, "SENDER: Attribution data: {:?}", attribution_data);
+				process_onion_success(&self.secp_ctx, &self.logger, &path,
+					&session_priv, None,
+					attribution_data.unwrap().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 +7516,25 @@ 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 attribution_data = attribution_data
+					.map(|attribution_data| {
+						let mut attribution_data = attribution_data.clone();
+
+						attribution_data.shift_right();
+
+						// Fake hold time here.
+						let hold_time = hop_data.incoming_packet_shared_secret[0];
+
+						let hold_time_bytes: [u8; 4] = (10 + hold_time as u32).to_be_bytes();
+						attribution_data.hold_times[..HOLD_TIME_LEN].copy_from_slice(&hold_time_bytes);
+						attribution_data.add_hmacs(&hop_data.incoming_packet_shared_secret, &[]);
+						attribution_data.crypt(&hop_data.incoming_packet_shared_secret);
+
+						attribution_data
+					});
+
+				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,
@@ -8937,7 +8978,7 @@ 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(),
 		);
 
 		Ok(())
@@ -9638,6 +9679,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,
 							);
 						} else {
 							log_trace!(logger, "Failing HTLC with hash {} from our monitor", &htlc_update.payment_hash);
@@ -12154,6 +12196,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 +14948,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, None,
 								|_, _|
 									(Some(MonitorUpdateCompletionAction::PaymentClaimed { payment_hash, pending_mpp_claim }), pending_claim_ptr)
 							);
@@ -15011,7 +15054,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,
 			);
 		}
 

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);

lightning/src/ln/msgs.rs

@@ -735,6 +735,9 @@ pub struct UpdateFulfillHTLC {
 	pub htlc_id: u64,
 	/// The pre-image of the payment hash, allowing HTLC redemption
 	pub payment_preimage: PaymentPreimage,
+
+	/// Optional field for the attribution data that allows the sender to pinpoint the failing node under all conditions
+	pub attribution_data: Option<AttributionData>,
 }
 
 /// A [`peer_storage`] message that can be sent to or received from a peer.
@@ -3076,7 +3079,10 @@ impl_writeable_msg!(UpdateFulfillHTLC, {
 	channel_id,
 	htlc_id,
 	payment_preimage
-}, {});
+}, {
+	// Specified TLV key 1 plus 100 during testing phase.
+	(101, attribution_data, option)
+});
 
 impl_writeable_msg!(PeerStorage, { data }, {});
 
@@ -5552,6 +5558,7 @@ mod tests {
 			channel_id: ChannelId::from_bytes([2; 32]),
 			htlc_id: 2316138423780173,
 			payment_preimage: PaymentPreimage([1; 32]),
+			attribution_data: None,
 		};
 		let encoded_value = update_fulfill_htlc.encode();
 		let target_value = <Vec<u8>>::from_hex("020202020202020202020202020202020202020202020202020202020202020200083a840000034d0101010101010101010101010101010101010101010101010101010101010101").unwrap();