Merge pull request #2256 from joostjager/attr-errs

Attributable failures

Author: joostjager

fuzz/src/process_onion_failure.rs

@@ -118,8 +118,17 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 		HTLCSource::OutboundRoute { path, session_priv, first_hop_htlc_msat: 0, payment_id };
 
 	let failure_len = get_u16!();
-	let encrypted_packet = OnionErrorPacket { data: get_slice!(failure_len).into() };
+	let failure_data = get_slice!(failure_len);
 
+	let attribution_data = if get_bool!() {
+		Some(lightning::ln::AttributionData {
+			hold_times: get_slice!(80).try_into().unwrap(),
+			hmacs: get_slice!(840).try_into().unwrap(),
+		})
+	} else {
+		None
+	};
+	let encrypted_packet = OnionErrorPacket { data: failure_data.into(), attribution_data };
 	lightning::ln::process_onion_failure(&secp_ctx, &logger, &htlc_source, encrypted_packet);
 }
 

lightning/src/ln/channel.rs

@@ -50,7 +50,7 @@ use crate::ln::chan_utils::{
 #[cfg(splicing)]
 use crate::ln::chan_utils::FUNDING_TRANSACTION_WITNESS_WEIGHT;
 use crate::ln::chan_utils;
-use crate::ln::onion_utils::{HTLCFailReason};
+use crate::ln::onion_utils::{HTLCFailReason, AttributionData};
 use crate::chain::BestBlock;
 use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator, fee_for_weight};
 use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS};
@@ -68,6 +68,7 @@ use crate::util::scid_utils::scid_from_parts;
 
 use crate::io;
 use crate::prelude::*;
+use core::time::Duration;
 use core::{cmp,mem,fmt};
 use core::ops::Deref;
 #[cfg(any(test, fuzzing, debug_assertions))]
@@ -323,6 +324,7 @@ struct OutboundHTLCOutput {
 	source: HTLCSource,
 	blinding_point: Option<PublicKey>,
 	skimmed_fee_msat: Option<u64>,
+	send_timestamp: Option<Duration>,
 }
 
 /// See AwaitingRemoteRevoke ChannelState for more info
@@ -4982,7 +4984,7 @@ trait FailHTLCContents {
 impl FailHTLCContents for msgs::OnionErrorPacket {
 	type Message = msgs::UpdateFailHTLC;
 	fn to_message(self, htlc_id: u64, channel_id: ChannelId) -> Self::Message {
-		msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self.data }
+		msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self.data, attribution_data: self.attribution_data }
 	}
 	fn to_inbound_htlc_state(self) -> InboundHTLCState {
 		InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(self))
@@ -6148,10 +6150,16 @@ impl<SP: Deref> FundedChannel<SP> where
 					false
 				} else { true }
 			});
+			let now = duration_since_epoch();
 			pending_outbound_htlcs.retain(|htlc| {
 				if let &OutboundHTLCState::AwaitingRemovedRemoteRevoke(ref outcome) = &htlc.state {
 					log_trace!(logger, " ...removing outbound AwaitingRemovedRemoteRevoke {}", &htlc.payment_hash);
-					if let OutboundHTLCOutcome::Failure(reason) = outcome.clone() { // We really want take() here, but, again, non-mut ref :(
+					if let OutboundHTLCOutcome::Failure(mut reason) = outcome.clone() { // We really want take() here, but, again, non-mut ref :(
+						if let (Some(timestamp), Some(now)) = (htlc.send_timestamp, now) {
+							let hold_time = u32::try_from(now.saturating_sub(timestamp).as_millis()).unwrap_or(u32::MAX);
+							reason.set_hold_time(hold_time);
+						}
+
 						revoked_htlcs.push((htlc.source.clone(), htlc.payment_hash, reason));
 					} else {
 						finalized_claimed_htlcs.push(htlc.source.clone());
@@ -6897,6 +6905,7 @@ impl<SP: Deref> FundedChannel<SP> where
 							channel_id: self.context.channel_id(),
 							htlc_id: htlc.htlc_id,
 							reason: err_packet.data.clone(),
+							attribution_data: err_packet.attribution_data.clone(),
 						});
 					},
 					&InboundHTLCRemovalReason::FailMalformed((ref sha256_of_onion, ref failure_code)) => {
@@ -8723,6 +8732,13 @@ impl<SP: Deref> FundedChannel<SP> where
 			return Ok(None);
 		}
 
+		// Record the approximate time when the HTLC is sent to the peer. This timestamp is later used to calculate the
+		// htlc hold time for reporting back to the sender. There is some freedom to report a time including or
+		// excluding our own processing time. What we choose here doesn't matter all that much, because it will probably
+		// just shift sender-applied penalties between our incoming and outgoing side. So we choose measuring points
+		// that are simple to implement, and we do it on the outgoing side because then the failure message that encodes
+		// the hold time still needs to be built in channel manager.
+		let send_timestamp = duration_since_epoch();
 		self.context.pending_outbound_htlcs.push(OutboundHTLCOutput {
 			htlc_id: self.context.next_holder_htlc_id,
 			amount_msat,
@@ -8732,6 +8748,7 @@ impl<SP: Deref> FundedChannel<SP> where
 			source,
 			blinding_point,
 			skimmed_fee_msat,
+			send_timestamp,
 		});
 
 		let res = msgs::UpdateAddHTLC {
@@ -10305,6 +10322,7 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
 				dropped_inbound_htlcs += 1;
 			}
 		}
+		let mut removed_htlc_failure_attribution_data: Vec<&Option<AttributionData>> = Vec::new();
 		(self.context.pending_inbound_htlcs.len() as u64 - dropped_inbound_htlcs).write(writer)?;
 		for htlc in self.context.pending_inbound_htlcs.iter() {
 			if let &InboundHTLCState::RemoteAnnounced(_) = &htlc.state {
@@ -10330,9 +10348,10 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
 				&InboundHTLCState::LocalRemoved(ref removal_reason) => {
 					4u8.write(writer)?;
 					match removal_reason {
-						InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket { data }) => {
+						InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket { data, attribution_data }) => {
 							0u8.write(writer)?;
 							data.write(writer)?;
+							removed_htlc_failure_attribution_data.push(&attribution_data);
 						},
 						InboundHTLCRemovalReason::FailMalformed((hash, code)) => {
 							1u8.write(writer)?;
@@ -10392,11 +10411,13 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
 			pending_outbound_blinding_points.push(htlc.blinding_point);
 		}
 
-		let mut holding_cell_skimmed_fees: Vec<Option<u64>> = Vec::new();
-		let mut holding_cell_blinding_points: Vec<Option<PublicKey>> = Vec::new();
+		let holding_cell_htlc_update_count = self.context.holding_cell_htlc_updates.len();
+		let mut holding_cell_skimmed_fees: Vec<Option<u64>> = Vec::with_capacity(holding_cell_htlc_update_count);
+		let mut holding_cell_blinding_points: Vec<Option<PublicKey>> = Vec::with_capacity(holding_cell_htlc_update_count);
+		let mut holding_cell_failure_attribution_data: Vec<Option<&AttributionData>> = Vec::with_capacity(holding_cell_htlc_update_count);
 		// Vec of (htlc_id, failure_code, sha256_of_onion)
 		let mut malformed_htlcs: Vec<(u64, u16, [u8; 32])> = Vec::new();
-		(self.context.holding_cell_htlc_updates.len() as u64).write(writer)?;
+		(holding_cell_htlc_update_count as u64).write(writer)?;
 		for update in self.context.holding_cell_htlc_updates.iter() {
 			match update {
 				&HTLCUpdateAwaitingACK::AddHTLC {
@@ -10422,6 +10443,9 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
 					2u8.write(writer)?;
 					htlc_id.write(writer)?;
 					err_packet.data.write(writer)?;
+
+					// Store the attribution data for later writing.
+					holding_cell_failure_attribution_data.push(err_packet.attribution_data.as_ref());
 				}
 				&HTLCUpdateAwaitingACK::FailMalformedHTLC {
 					htlc_id, failure_code, sha256_of_onion
@@ -10433,6 +10457,10 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
 					2u8.write(writer)?;
 					htlc_id.write(writer)?;
 					Vec::<u8>::new().write(writer)?;
+
+					// Push 'None' attribution data for FailMalformedHTLC, because FailMalformedHTLC uses the same
+					// type 2 and is deserialized as a FailHTLC.
+					holding_cell_failure_attribution_data.push(None);
 				}
 			}
 		}
@@ -10605,6 +10633,8 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
 			(49, self.context.local_initiated_shutdown, option), // Added in 0.0.122
 			(51, is_manual_broadcast, option), // Added in 0.0.124
 			(53, funding_tx_broadcast_safe_event_emitted, option), // Added in 0.0.124
+			(55, removed_htlc_failure_attribution_data, optional_vec), // Added in 0.2
+			(57, holding_cell_failure_attribution_data, optional_vec), // Added in 0.2
 		});
 
 		Ok(())
@@ -10682,6 +10712,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 						let reason = match <u8 as Readable>::read(reader)? {
 							0 => InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket {
 								data: Readable::read(reader)?,
+								attribution_data: None,
 							}),
 							1 => InboundHTLCRemovalReason::FailMalformed(Readable::read(reader)?),
 							2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?),
@@ -10722,6 +10753,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 				},
 				skimmed_fee_msat: None,
 				blinding_point: None,
+				send_timestamp: None,
 			});
 		}
 
@@ -10746,6 +10778,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 					htlc_id: Readable::read(reader)?,
 					err_packet: OnionErrorPacket {
 						data: Readable::read(reader)?,
+						attribution_data: None,
 					},
 				},
 				_ => return Err(DecodeError::InvalidValue),
@@ -10889,6 +10922,9 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 		let mut pending_outbound_blinding_points_opt: Option<Vec<Option<PublicKey>>> = None;
 		let mut holding_cell_blinding_points_opt: Option<Vec<Option<PublicKey>>> = None;
 
+		let mut removed_htlc_failure_attribution_data: Option<Vec<Option<AttributionData>>> = None;
+		let mut holding_cell_failure_attribution_data: Option<Vec<Option<AttributionData>>> = None;
+
 		let mut malformed_htlcs: Option<Vec<(u64, u16, [u8; 32])>> = None;
 		let mut monitor_pending_update_adds: Option<Vec<msgs::UpdateAddHTLC>> = None;
 
@@ -10931,6 +10967,8 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 			(49, local_initiated_shutdown, option),
 			(51, is_manual_broadcast, option),
 			(53, funding_tx_broadcast_safe_event_emitted, option),
+			(55, removed_htlc_failure_attribution_data, optional_vec),
+			(57, holding_cell_failure_attribution_data, optional_vec),
 		});
 
 		let holder_signer = signer_provider.derive_channel_signer(channel_keys_id);
@@ -11012,6 +11050,38 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 			if iter.next().is_some() { return Err(DecodeError::InvalidValue) }
 		}
 
+		if let Some(attribution_data_list) = removed_htlc_failure_attribution_data {
+			let mut removed_htlc_relay_failures =
+				pending_inbound_htlcs.iter_mut().filter_map(|status|
+					if let InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(ref mut packet)) = &mut status.state {
+						Some(&mut packet.attribution_data)
+					} else {
+						None
+					}
+				);
+
+			for attribution_data in attribution_data_list {
+				*removed_htlc_relay_failures.next().ok_or(DecodeError::InvalidValue)? = attribution_data;
+			}
+			if removed_htlc_relay_failures.next().is_some() { return Err(DecodeError::InvalidValue); }
+		}
+
+		if let Some(attribution_data_list) = holding_cell_failure_attribution_data {
+			let mut holding_cell_failures =
+				holding_cell_htlc_updates.iter_mut().filter_map(|upd|
+					if let HTLCUpdateAwaitingACK::FailHTLC { err_packet: OnionErrorPacket { ref mut attribution_data, .. }, .. } = upd {
+						Some(attribution_data)
+					} else {
+						None
+					}
+				);
+
+			for attribution_data in attribution_data_list {
+				*holding_cell_failures.next().ok_or(DecodeError::InvalidValue)? = attribution_data;
+			}
+			if holding_cell_failures.next().is_some() { return Err(DecodeError::InvalidValue); }
+		}
+
 		if let Some(malformed_htlcs) = malformed_htlcs {
 			for (malformed_htlc_id, failure_code, sha256_of_onion) in malformed_htlcs {
 				let htlc_idx = holding_cell_htlc_updates.iter().position(|htlc| {
@@ -11201,6 +11271,18 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 	}
 }
 
+fn duration_since_epoch() -> Option<Duration> {
+	#[cfg(not(feature = "std"))]
+	let now = None;
+
+	#[cfg(feature = "std")]
+	let now = Some(std::time::SystemTime::now()
+		.duration_since(std::time::SystemTime::UNIX_EPOCH)
+		.expect("SystemTime::now() should come after SystemTime::UNIX_EPOCH"));
+
+	now
+}
+
 #[cfg(test)]
 mod tests {
 	use std::cmp;
@@ -11214,7 +11296,7 @@ mod tests {
 	use bitcoin::network::Network;
 	#[cfg(splicing)]
 	use bitcoin::Weight;
-	use crate::ln::onion_utils::INVALID_ONION_BLINDING;
+	use crate::ln::onion_utils::{AttributionData, INVALID_ONION_BLINDING};
 	use crate::types::payment::{PaymentHash, PaymentPreimage};
 	use crate::ln::channel_keys::{RevocationKey, RevocationBasepoint};
 	use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
@@ -11436,6 +11518,7 @@ mod tests {
 			},
 			skimmed_fee_msat: None,
 			blinding_point: None,
+			send_timestamp: None,
 		});
 
 		// Make sure when Node A calculates their local commitment transaction, none of the HTLCs pass
@@ -11820,6 +11903,7 @@ mod tests {
 			source: dummy_htlc_source.clone(),
 			skimmed_fee_msat: None,
 			blinding_point: None,
+			send_timestamp: None,
 		};
 		let mut pending_outbound_htlcs = vec![dummy_outbound_output.clone(); 10];
 		for (idx, htlc) in pending_outbound_htlcs.iter_mut().enumerate() {
@@ -11851,7 +11935,7 @@ mod tests {
 			htlc_id: 0,
 		};
 		let dummy_holding_cell_failed_htlc = |htlc_id| HTLCUpdateAwaitingACK::FailHTLC {
-			htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] }
+			htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42], attribution_data: Some(AttributionData::new()) }
 		};
 		let dummy_holding_cell_malformed_htlc = |htlc_id| HTLCUpdateAwaitingACK::FailMalformedHTLC {
 			htlc_id, failure_code: INVALID_ONION_BLINDING, sha256_of_onion: [0; 32],
@@ -12133,6 +12217,7 @@ mod tests {
 				source: HTLCSource::dummy(),
 				skimmed_fee_msat: None,
 				blinding_point: None,
+				send_timestamp: None,
 			};
 			out.payment_hash.0 = Sha256::hash(&<Vec<u8>>::from_hex("0202020202020202020202020202020202020202020202020202020202020202").unwrap()).to_byte_array();
 			out
@@ -12147,6 +12232,7 @@ mod tests {
 				source: HTLCSource::dummy(),
 				skimmed_fee_msat: None,
 				blinding_point: None,
+				send_timestamp: None,
 			};
 			out.payment_hash.0 = Sha256::hash(&<Vec<u8>>::from_hex("0303030303030303030303030303030303030303030303030303030303030303").unwrap()).to_byte_array();
 			out
@@ -12559,6 +12645,7 @@ mod tests {
 				source: HTLCSource::dummy(),
 				skimmed_fee_msat: None,
 				blinding_point: None,
+				send_timestamp: None,
 			};
 			out.payment_hash.0 = Sha256::hash(&<Vec<u8>>::from_hex("0505050505050505050505050505050505050505050505050505050505050505").unwrap()).to_byte_array();
 			out
@@ -12573,6 +12660,7 @@ mod tests {
 				source: HTLCSource::dummy(),
 				skimmed_fee_msat: None,
 				blinding_point: None,
+				send_timestamp: None,
 			};
 			out.payment_hash.0 = Sha256::hash(&<Vec<u8>>::from_hex("0505050505050505050505050505050505050505050505050505050505050505").unwrap()).to_byte_array();
 			out