Merge pull request #3700 from carlaKC/3541-api-changes

(3/3) Add Failure Reason to HTLCHandlingFailed

Author: valentinewallace

lightning-liquidity/src/lsps2/service.rs

@@ -31,7 +31,7 @@ use crate::prelude::hash_map::Entry;
 use crate::prelude::{new_hash_map, HashMap};
 use crate::sync::{Arc, Mutex, MutexGuard, RwLock};
 
-use lightning::events::HTLCDestination;
+use lightning::events::HTLCHandlingFailureType;
 use lightning::ln::channelmanager::{AChannelManager, InterceptId};
 use lightning::ln::msgs::{ErrorAction, LightningError};
 use lightning::ln::types::ChannelId;
@@ -863,9 +863,9 @@ where
 	///
 	/// [`Event::HTLCHandlingFailed`]: lightning::events::Event::HTLCHandlingFailed
 	pub fn htlc_handling_failed(
-		&self, failed_next_destination: HTLCDestination,
+		&self, failure_type: HTLCHandlingFailureType,
 	) -> Result<(), APIError> {
-		if let HTLCDestination::NextHopChannel { channel_id, .. } = failed_next_destination {
+		if let HTLCHandlingFailureType::Forward { channel_id, .. } = failure_type {
 			let peer_by_channel_id = self.peer_by_channel_id.read().unwrap();
 			if let Some(counterparty_node_id) = peer_by_channel_id.get(&channel_id) {
 				let outer_state_lock = self.per_peer_state.read().unwrap();

lightning/src/events/mod.rs

@@ -26,7 +26,7 @@ use crate::ln::channel::FUNDING_CONF_DEADLINE_BLOCKS;
 use crate::offers::invoice::Bolt12Invoice;
 use crate::offers::static_invoice::StaticInvoice;
 use crate::types::features::ChannelTypeFeatures;
-use crate::ln::msgs;
+use crate::ln::{msgs, LocalHTLCFailureReason};
 use crate::ln::types::ChannelId;
 use crate::types::payment::{PaymentPreimage, PaymentHash, PaymentSecret};
 use crate::onion_message::messenger::Responder;
@@ -466,12 +466,12 @@ impl_writeable_tlv_based_enum_upgradable!(ClosureReason,
 	},
 );
 
-/// Intended destination of a failed HTLC as indicated in [`Event::HTLCHandlingFailed`].
+/// The type of HTLC handling performed in [`Event::HTLCHandlingFailed`].
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub enum HTLCDestination {
+pub enum HTLCHandlingFailureType {
 	/// We tried forwarding to a channel but failed to do so. An example of such an instance is when
 	/// there is insufficient capacity in our outbound channel.
-	NextHopChannel {
+	Forward {
 		/// The `node_id` of the next node. For backwards compatibility, this field is
 		/// marked as optional, versions prior to 0.0.110 may not always be able to provide
 		/// counterparty node information.
@@ -480,12 +480,17 @@ pub enum HTLCDestination {
 		channel_id: ChannelId,
 	},
 	/// Scenario where we are unsure of the next node to forward the HTLC to.
+	///
+	/// Deprecated: will only be used in versions before LDK v0.2.0. Downgrades will result in
+	/// this type being represented as [`Self::InvalidForward`].
 	UnknownNextHop {
 		/// Short channel id we are requesting to forward an HTLC to.
 		requested_forward_scid: u64,
 	},
 	/// We couldn't forward to the outgoing scid. An example would be attempting to send a duplicate
 	/// intercept HTLC.
+	///
+	/// In LDK v0.2.0 and greater, this variant replaces [`Self::UnknownNextHop`].
 	InvalidForward {
 		/// Short channel id we are requesting to forward an HTLC to.
 		requested_forward_scid: u64
@@ -502,14 +507,14 @@ pub enum HTLCDestination {
 	/// * The counterparty node modified the HTLC in transit,
 	/// * A probing attack where an intermediary node is trying to detect if we are the ultimate
 	///   recipient for a payment.
-	FailedPayment {
+	Receive {
 		/// The payment hash of the payment we attempted to process.
 		payment_hash: PaymentHash
 	},
 }
 
-impl_writeable_tlv_based_enum_upgradable!(HTLCDestination,
-	(0, NextHopChannel) => {
+impl_writeable_tlv_based_enum_upgradable!(HTLCHandlingFailureType,
+	(0, Forward) => {
 		(0, node_id, required),
 		(2, channel_id, required),
 	},
@@ -520,11 +525,36 @@ impl_writeable_tlv_based_enum_upgradable!(HTLCDestination,
 		(0, requested_forward_scid, required),
 	},
 	(3, InvalidOnion) => {},
-	(4, FailedPayment) => {
+	(4, Receive) => {
 		(0, payment_hash, required),
 	},
 );
 
+/// The reason for HTLC failures in [`Event::HTLCHandlingFailed`].
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum HTLCHandlingFailureReason {
+	/// The forwarded HTLC was failed back by the downstream node with an encrypted error reason.
+	Downstream,
+	/// The HTLC was failed locally by our node.
+	Local {
+		/// The reason that our node chose to fail the HTLC.
+		reason: LocalHTLCFailureReason,
+	},
+}
+
+impl_writeable_tlv_based_enum!(HTLCHandlingFailureReason,
+	(1, Downstream) => {},
+	(3, Local) => {
+		(0, reason, required),
+	},
+);
+
+impl From<LocalHTLCFailureReason> for HTLCHandlingFailureReason {
+	fn from(value: LocalHTLCFailureReason) -> Self {
+		HTLCHandlingFailureReason::Local { reason: value }
+	}
+}
+
 /// Will be used in [`Event::HTLCIntercepted`] to identify the next hop in the HTLC's path.
 /// Currently only used in serialization for the sake of maintaining compatibility. More variants
 /// will be added for general-purpose HTLC forward intercepts as well as trampoline forward
@@ -1460,8 +1490,12 @@ pub enum Event {
 	HTLCHandlingFailed {
 		/// The channel over which the HTLC was received.
 		prev_channel_id: ChannelId,
-		/// Destination of the HTLC that failed to be processed.
-		failed_next_destination: HTLCDestination,
+		/// The type of HTLC handling that failed.
+		failure_type: HTLCHandlingFailureType,
+		/// The reason that the HTLC failed.
+		///
+		/// This field will be `None` only for objects serialized prior to LDK 0.2.0.
+		failure_reason: Option<HTLCHandlingFailureReason>
 	},
 	/// Indicates that a transaction originating from LDK needs to have its fee bumped. This event
 	/// requires confirmed external funds to be readily available to spend.
@@ -1766,11 +1800,12 @@ impl Writeable for Event {
 					(8, path.blinded_tail, option),
 				})
 			},
-			&Event::HTLCHandlingFailed { ref prev_channel_id, ref failed_next_destination } => {
+			&Event::HTLCHandlingFailed { ref prev_channel_id, ref failure_type, ref failure_reason } => {
 				25u8.write(writer)?;
 				write_tlv_fields!(writer, {
 					(0, prev_channel_id, required),
-					(2, failed_next_destination, required),
+					(1, failure_reason, option),
+					(2, failure_type, required),
 				})
 			},
 			&Event::BumpTransaction(ref event)=> {
@@ -2218,14 +2253,24 @@ impl MaybeReadable for Event {
 			25u8 => {
 				let mut f = || {
 					let mut prev_channel_id = ChannelId::new_zero();
-					let mut failed_next_destination_opt = UpgradableRequired(None);
+					let mut failure_reason = None;
+					let mut failure_type_opt = UpgradableRequired(None);
 					read_tlv_fields!(reader, {
 						(0, prev_channel_id, required),
-						(2, failed_next_destination_opt, upgradable_required),
+						(1, failure_reason, option),
+						(2, failure_type_opt, upgradable_required),
 					});
+
+					// If a legacy HTLCHandlingFailureType::UnknownNextHop was written, upgrade
+					// it to its new representation, otherwise leave unchanged.
+					if let Some(HTLCHandlingFailureType::UnknownNextHop { requested_forward_scid }) = failure_type_opt.0 {
+						failure_type_opt.0 = Some(HTLCHandlingFailureType::InvalidForward { requested_forward_scid });
+						failure_reason = Some(LocalHTLCFailureReason::UnknownNextPeer.into());
+					}
 					Ok(Some(Event::HTLCHandlingFailed {
 						prev_channel_id,
-						failed_next_destination: _init_tlv_based_struct_field!(failed_next_destination_opt, upgradable_required),
+						failure_type: _init_tlv_based_struct_field!(failure_type_opt, upgradable_required),
+						failure_reason
 					}))
 				};
 				f()

lightning/src/ln/async_payments_tests.rs

@@ -11,7 +11,7 @@ use crate::blinded_path::message::{MessageContext, OffersContext};
 use crate::blinded_path::payment::PaymentContext;
 use crate::blinded_path::payment::{AsyncBolt12OfferContext, BlindedPaymentTlvs};
 use crate::chain::channelmonitor::{HTLC_FAIL_BACK_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS};
-use crate::events::{Event, HTLCDestination, PaidBolt12Invoice, PaymentFailureReason};
+use crate::events::{Event, HTLCHandlingFailureType, PaidBolt12Invoice, PaymentFailureReason};
 use crate::ln::blinded_payment_tests::{fail_blinded_htlc_backwards, get_blinded_route_parameters};
 use crate::ln::channelmanager::{PaymentId, RecipientOnionFields};
 use crate::ln::functional_test_utils::*;
@@ -172,7 +172,7 @@ fn invalid_keysend_payment_secret() {
 		PassAlongPathArgs::new(&nodes[0], &expected_route[0], amt_msat, payment_hash, ev.clone())
 			.with_payment_secret(invalid_payment_secret)
 			.with_payment_preimage(keysend_preimage)
-			.expect_failure(HTLCDestination::FailedPayment { payment_hash });
+			.expect_failure(HTLCHandlingFailureType::Receive { payment_hash });
 	do_pass_along_path(args);
 
 	let updates_2_1 = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
@@ -701,7 +701,7 @@ fn amount_doesnt_match_invreq() {
 	let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev)
 		.with_payment_preimage(keysend_preimage)
 		.without_claimable_event()
-		.expect_failure(HTLCDestination::FailedPayment { payment_hash });
+		.expect_failure(HTLCHandlingFailureType::Receive { payment_hash });
 	do_pass_along_path(args);
 
 	// Modify the invoice request stored in our outbounds to be the correct one, to make sure the
@@ -917,7 +917,7 @@ fn invalid_async_receive_with_retry<F1, F2>(
 	nodes[2].node.fail_htlc_backwards(&payment_hash);
 	expect_pending_htlcs_forwardable_conditions(
 		nodes[2].node.get_and_clear_pending_events(),
-		&[HTLCDestination::FailedPayment { payment_hash }],
+		&[HTLCHandlingFailureType::Receive { payment_hash }],
 	);
 	nodes[2].node.process_pending_htlc_forwards();
 	check_added_monitors!(nodes[2], 1);
@@ -937,7 +937,7 @@ fn invalid_async_receive_with_retry<F1, F2>(
 	let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev)
 		.with_payment_preimage(keysend_preimage)
 		.without_claimable_event()
-		.expect_failure(HTLCDestination::FailedPayment { payment_hash });
+		.expect_failure(HTLCHandlingFailureType::Receive { payment_hash });
 	do_pass_along_path(args);
 	fail_blinded_htlc_backwards(payment_hash, 1, &[&nodes[0], &nodes[1], &nodes[2]], true);
 
@@ -1103,7 +1103,7 @@ fn expired_static_invoice_payment_path() {
 	let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev)
 		.with_payment_preimage(keysend_preimage)
 		.without_claimable_event()
-		.expect_failure(HTLCDestination::FailedPayment { payment_hash });
+		.expect_failure(HTLCHandlingFailureType::Receive { payment_hash });
 	do_pass_along_path(args);
 	fail_blinded_htlc_backwards(payment_hash, 1, &[&nodes[0], &nodes[1], &nodes[2]], false);
 	nodes[2].logger.assert_log_contains(