Merge pull request #3650 from joostjager/attributable-failures-prefactor

Attributable failures prefactor

Author: joostjager

lightning/src/ln/channel.rs

@@ -35,7 +35,7 @@ use crate::ln::interactivetxs::{
 	TX_COMMON_FIELDS_WEIGHT,
 };
 use crate::ln::msgs;
-use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError};
+use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError, OnionErrorPacket};
 use crate::ln::script::{self, ShutdownScript};
 use crate::ln::channel_state::{ChannelShutdownState, CounterpartyForwardingInfo, InboundHTLCDetails, InboundHTLCStateDetails, OutboundHTLCDetails, OutboundHTLCStateDetails};
 use crate::ln::channelmanager::{self, OpenChannelMessage, PendingHTLCStatus, HTLCSource, SentHTLCId, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentClaimDetails, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
@@ -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};
 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};
@@ -4933,7 +4933,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 }
+		msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self.data }
 	}
 	fn to_inbound_htlc_state(self) -> InboundHTLCState {
 		InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(self))
@@ -6136,7 +6136,7 @@ impl<SP: Deref> FundedChannel<SP> where
 										require_commitment = true;
 										match fail_msg {
 											HTLCFailureMsg::Relay(msg) => {
-												htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(msg.reason.clone()));
+												htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(msg.clone().into()));
 												update_fail_htlcs.push(msg)
 											},
 											HTLCFailureMsg::Malformed(msg) => {
@@ -6844,7 +6844,7 @@ impl<SP: Deref> FundedChannel<SP> where
 						update_fail_htlcs.push(msgs::UpdateFailHTLC {
 							channel_id: self.context.channel_id(),
 							htlc_id: htlc.htlc_id,
-							reason: err_packet.clone()
+							reason: err_packet.data.clone(),
 						});
 					},
 					&InboundHTLCRemovalReason::FailMalformed((ref sha256_of_onion, ref failure_code)) => {
@@ -10142,11 +10142,6 @@ fn get_initial_channel_type(config: &UserConfig, their_features: &InitFeatures)
 const SERIALIZATION_VERSION: u8 = 4;
 const MIN_SERIALIZATION_VERSION: u8 = 4;
 
-impl_writeable_tlv_based_enum_legacy!(InboundHTLCRemovalReason,;
-	(0, FailRelay),
-	(1, FailMalformed),
-	(2, Fulfill),
-);
 
 impl Writeable for ChannelUpdateStatus {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
@@ -10276,7 +10271,20 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
 				},
 				&InboundHTLCState::LocalRemoved(ref removal_reason) => {
 					4u8.write(writer)?;
-					removal_reason.write(writer)?;
+					match removal_reason {
+						InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket { data }) => {
+							0u8.write(writer)?;
+							data.write(writer)?;
+						},
+						InboundHTLCRemovalReason::FailMalformed((hash, code)) => {
+							1u8.write(writer)?;
+							(hash, code).write(writer)?;
+						},
+						InboundHTLCRemovalReason::Fulfill(preimage) => {
+							2u8.write(writer)?;
+							preimage.write(writer)?;
+						},
+					}
 				},
 			}
 		}
@@ -10355,7 +10363,7 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
 				&HTLCUpdateAwaitingACK::FailHTLC { ref htlc_id, ref err_packet } => {
 					2u8.write(writer)?;
 					htlc_id.write(writer)?;
-					err_packet.write(writer)?;
+					err_packet.data.write(writer)?;
 				}
 				&HTLCUpdateAwaitingACK::FailMalformedHTLC {
 					htlc_id, failure_code, sha256_of_onion
@@ -10364,10 +10372,9 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
 					// `::FailHTLC` variant and write the real malformed error as an optional TLV.
 					malformed_htlcs.push((htlc_id, failure_code, sha256_of_onion));
 
-					let dummy_err_packet = msgs::OnionErrorPacket { data: Vec::new() };
 					2u8.write(writer)?;
 					htlc_id.write(writer)?;
-					dummy_err_packet.write(writer)?;
+					Vec::<u8>::new().write(writer)?;
 				}
 			}
 		}
@@ -10613,7 +10620,17 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 						InboundHTLCState::AwaitingAnnouncedRemoteRevoke(resolution)
 					},
 					3 => InboundHTLCState::Committed,
-					4 => InboundHTLCState::LocalRemoved(Readable::read(reader)?),
+					4 => {
+						let reason = match <u8 as Readable>::read(reader)? {
+							0 => InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket {
+								data: Readable::read(reader)?,
+							}),
+							1 => InboundHTLCRemovalReason::FailMalformed(Readable::read(reader)?),
+							2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?),
+							_ => return Err(DecodeError::InvalidValue),
+						};
+						InboundHTLCState::LocalRemoved(reason)
+					},
 					_ => return Err(DecodeError::InvalidValue),
 				},
 			});
@@ -10669,7 +10686,9 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
 				},
 				2 => HTLCUpdateAwaitingACK::FailHTLC {
 					htlc_id: Readable::read(reader)?,
-					err_packet: Readable::read(reader)?,
+					err_packet: OnionErrorPacket {
+						data: Readable::read(reader)?,
+					},
 				},
 				_ => return Err(DecodeError::InvalidValue),
 			});

lightning/src/ln/channelmanager.rs

@@ -4470,11 +4470,12 @@ where
 		} else {
 			(err_code, &res.0[..])
 		};
+		let failure = HTLCFailReason::reason(err_code, err_data.to_vec())
+		.get_encrypted_failure_packet(shared_secret, &None);
 		HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
 			channel_id: msg.channel_id,
 			htlc_id: msg.htlc_id,
-			reason: HTLCFailReason::reason(err_code, err_data.to_vec())
-				.get_encrypted_failure_packet(shared_secret, &None),
+			reason: failure.data,
 		})
 	}
 
@@ -4498,11 +4499,12 @@ where
 							}
 						))
 					}
+					let failure = HTLCFailReason::reason($err_code, $data.to_vec())
+						.get_encrypted_failure_packet(&shared_secret, &None);
 					return PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
 						channel_id: msg.channel_id,
 						htlc_id: msg.htlc_id,
-						reason: HTLCFailReason::reason($err_code, $data.to_vec())
-							.get_encrypted_failure_packet(&shared_secret, &None),
+						reason: failure.data,
 					}));
 				}
 			}
@@ -5881,7 +5883,7 @@ where
 				let failure = match htlc_fail {
 					HTLCFailureMsg::Relay(fail_htlc) => HTLCForwardInfo::FailHTLC {
 						htlc_id: fail_htlc.htlc_id,
-						err_packet: fail_htlc.reason,
+						err_packet: fail_htlc.into(),
 					},
 					HTLCFailureMsg::Malformed(fail_malformed_htlc) => HTLCForwardInfo::FailMalformedHTLC {
 						htlc_id: fail_malformed_htlc.htlc_id,
@@ -13224,19 +13226,18 @@ impl Writeable for HTLCForwardInfo {
 				FAIL_HTLC_VARIANT_ID.write(w)?;
 				write_tlv_fields!(w, {
 					(0, htlc_id, required),
-					(2, err_packet, required),
+					(2, err_packet.data, required),
 				});
 			},
 			Self::FailMalformedHTLC { htlc_id, failure_code, sha256_of_onion } => {
 				// Since this variant was added in 0.0.119, write this as `::FailHTLC` with an empty error
 				// packet so older versions have something to fail back with, but serialize the real data as
 				// optional TLVs for the benefit of newer versions.
 				FAIL_HTLC_VARIANT_ID.write(w)?;
-				let dummy_err_packet = msgs::OnionErrorPacket { data: Vec::new() };
 				write_tlv_fields!(w, {
 					(0, htlc_id, required),
 					(1, failure_code, required),
-					(2, dummy_err_packet, required),
+					(2, Vec::<u8>::new(), required),
 					(3, sha256_of_onion, required),
 				});
 			},
@@ -13266,7 +13267,9 @@ impl Readable for HTLCForwardInfo {
 				} else {
 					Self::FailHTLC {
 						htlc_id: _init_tlv_based_struct_field!(htlc_id, required),
-						err_packet: _init_tlv_based_struct_field!(err_packet, required),
+						err_packet: crate::ln::msgs::OnionErrorPacket {
+							data: _init_tlv_based_struct_field!(err_packet, required),
+						},
 					}
 				}
 			},
@@ -16273,7 +16276,7 @@ mod tests {
 		let mut nodes = create_network(1, &node_cfg, &chanmgrs);
 
 		let dummy_failed_htlc = |htlc_id| {
-			HTLCForwardInfo::FailHTLC { htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] }, }
+			HTLCForwardInfo::FailHTLC { htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] } }
 		};
 		let dummy_malformed_htlc = |htlc_id| {
 			HTLCForwardInfo::FailMalformedHTLC { htlc_id, failure_code: 0x4000, sha256_of_onion: [0; 32] }

lightning/src/ln/functional_tests.rs

@@ -7057,7 +7057,7 @@ pub fn test_update_fulfill_htlc_bolt2_update_fail_htlc_before_commitment() {
 	let update_msg = msgs::UpdateFailHTLC{
 		channel_id: chan.2,
 		htlc_id: 0,
-		reason: msgs::OnionErrorPacket { data: Vec::new()},
+		reason: Vec::new(),
 	};
 
 	nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &update_msg);

lightning/src/ln/msgs.rs

@@ -766,7 +766,7 @@ pub struct UpdateFailHTLC {
 	pub channel_id: ChannelId,
 	/// The HTLC ID
 	pub htlc_id: u64,
-	pub(crate) reason: OnionErrorPacket,
+	pub(crate) reason: Vec<u8>,
 }
 
 /// An [`update_fail_malformed_htlc`] message to be sent to or received from a peer.
@@ -2355,6 +2355,14 @@ pub(crate) struct OnionErrorPacket {
 	pub(crate) data: Vec<u8>,
 }
 
+impl From<UpdateFailHTLC> for OnionErrorPacket {
+	fn from(msg: UpdateFailHTLC) -> Self {
+		OnionErrorPacket {
+			data: msg.reason,
+		}
+	}
+}
+
 impl fmt::Display for DecodeError {
 	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
 		match *self {
@@ -3001,13 +3009,6 @@ impl_writeable_msg!(PeerStorageRetrieval, {
 	data
 }, {});
 
-// Note that this is written as a part of ChannelManager objects, and thus cannot change its
-// serialization format in a way which assumes we know the total serialized length/message end
-// position.
-impl_writeable!(OnionErrorPacket, {
-	data
-});
-
 // Note that this is written as a part of ChannelManager objects, and thus cannot change its
 // serialization format in a way which assumes we know the total serialized length/message end
 // position.
@@ -3933,7 +3934,7 @@ mod tests {
 	use crate::ln::types::ChannelId;
 	use crate::types::payment::{PaymentPreimage, PaymentHash, PaymentSecret};
 	use crate::types::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
-	use crate::ln::msgs::{self, FinalOnionHopData, OnionErrorPacket, CommonOpenChannelFields, CommonAcceptChannelFields, OutboundTrampolinePayload, TrampolineOnionPacket, InboundOnionForwardPayload, InboundOnionReceivePayload};
+	use crate::ln::msgs::{self, FinalOnionHopData, CommonOpenChannelFields, CommonAcceptChannelFields, OutboundTrampolinePayload, TrampolineOnionPacket, InboundOnionForwardPayload, InboundOnionReceivePayload};
 	use crate::ln::msgs::SocketAddress;
 	use crate::routing::gossip::{NodeAlias, NodeId};
 	use crate::util::ser::{BigSize, FixedLengthReader, Hostname, LengthReadable, Readable, ReadableArgs, TransactionU16LenLimited, Writeable};
@@ -4932,13 +4933,10 @@ mod tests {
 
 	#[test]
 	fn encoding_update_fail_htlc() {
-		let reason = OnionErrorPacket {
-			data: [1; 32].to_vec(),
-		};
 		let update_fail_htlc = msgs::UpdateFailHTLC {
 			channel_id: ChannelId::from_bytes([2; 32]),
 			htlc_id: 2316138423780173,
-			reason
+			reason: [1; 32].to_vec()
 		};
 		let encoded_value = update_fail_htlc.encode();
 		let target_value = <Vec<u8>>::from_hex("020202020202020202020202020202020202020202020202020202020202020200083a840000034d00200101010101010101010101010101010101010101010101010101010101010101").unwrap();

lightning/src/ln/onion_payment.rs

@@ -407,7 +407,7 @@ where
 	).map_err(|e| {
 		let (err_code, err_data) = match e {
 			HTLCFailureMsg::Malformed(m) => (m.failure_code, Vec::new()),
-			HTLCFailureMsg::Relay(r) => (0x4000 | 22, r.reason.data),
+			HTLCFailureMsg::Relay(r) => (0x4000 | 22, r.reason),
 		};
 		let msg = "Failed to decode update add htlc onion";
 		InboundHTLCErr { msg, err_code, err_data }
@@ -512,11 +512,12 @@ where
 		}
 
 		log_info!(logger, "Failed to accept/forward incoming HTLC: {}", message);
+		let failure = HTLCFailReason::reason(err_code, data.to_vec())
+			.get_encrypted_failure_packet(&shared_secret, &trampoline_shared_secret);
 		return Err(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
 			channel_id: msg.channel_id,
 			htlc_id: msg.htlc_id,
-			reason: HTLCFailReason::reason(err_code, data.to_vec())
-				.get_encrypted_failure_packet(&shared_secret, &trampoline_shared_secret),
+			reason: failure.data,
 		}));
 	};