Automatically fail intercepts back on timeout

Author: valentinewallace

lightning/src/ln/channelmanager.rs

@@ -3171,6 +3171,9 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 	///
 	/// Note that LDK does not enforce fee requirements in `amt_to_forward_msat`.
 	///
+	/// Errors if the event was not handled in time, in which case the HTLC was automatically failed
+	/// backwards.
+	///
 	/// [`HTLCIntercepted`]: events::Event::HTLCIntercepted
 	// TODO: when we move to deciding the best outbound channel at forward time, only take
 	// `next_node_id` and not `next_hop_scid`
@@ -3210,6 +3213,9 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 	/// Fails the intercepted HTLC indicated by intercept_id. Should only be called in response to
 	/// a [`HTLCIntercepted`] event. See [`ChannelManager::forward_intercepted_htlc`].
 	///
+	/// Errors if the event was not handled in time, in which case the HTLC was automatically failed
+	/// backwards.
+	///
 	/// [`HTLCIntercepted`]: events::Event::HTLCIntercepted
 	pub fn fail_intercepted_htlc(&self, intercept_id: InterceptId) -> Result<(), APIError> {
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
@@ -6208,25 +6214,46 @@ where
 			});
 
 			if let Some(height) = height_opt {
+				macro_rules! time_out_htlc {
+					($amt_msat: expr, $payment_hash: expr, $prev_hop_data: expr) => {
+						let mut htlc_msat_height_data = byte_utils::be64_to_array($amt_msat).to_vec();
+						htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(height));
+
+						timed_out_htlcs.push((HTLCSource::PreviousHopData($prev_hop_data), $payment_hash, HTLCFailReason::Reason {
+							failure_code: 0x4000 | 15,
+							data: htlc_msat_height_data
+						}, HTLCDestination::FailedPayment { payment_hash: $payment_hash }));
+					}
+				}
 				channel_state.claimable_htlcs.retain(|payment_hash, (_, htlcs)| {
 					htlcs.retain(|htlc| {
 						// If height is approaching the number of blocks we think it takes us to get
 						// our commitment transaction confirmed before the HTLC expires, plus the
 						// number of blocks we generally consider it to take to do a commitment update,
 						// just give up on it and fail the HTLC.
 						if height >= htlc.cltv_expiry - HTLC_FAIL_BACK_BUFFER {
-							let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
-							htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(height));
-
-							timed_out_htlcs.push((HTLCSource::PreviousHopData(htlc.prev_hop.clone()), payment_hash.clone(), HTLCFailReason::Reason {
-								failure_code: 0x4000 | 15,
-								data: htlc_msat_height_data
-							}, HTLCDestination::FailedPayment { payment_hash: payment_hash.clone() }));
+							time_out_htlc!(htlc.value, *payment_hash, htlc.prev_hop.clone());
 							false
 						} else { true }
 					});
 					!htlcs.is_empty() // Only retain this entry if htlcs has at least one entry.
 				});
+
+				let mut intercepted_htlcs = self.pending_intercepted_htlcs.lock().unwrap();
+				intercepted_htlcs.retain(|_, htlc| {
+					if height >= htlc.forward_info.outgoing_cltv_value - HTLC_FAIL_BACK_BUFFER {
+						let prev_hop_data = HTLCPreviousHopData {
+							short_channel_id: htlc.prev_short_channel_id,
+							htlc_id: htlc.prev_htlc_id,
+							incoming_packet_shared_secret: htlc.forward_info.incoming_shared_secret,
+							phantom_shared_secret: None,
+							outpoint: htlc.prev_funding_outpoint,
+						};
+
+						time_out_htlc!(htlc.forward_info.outgoing_amt_msat, htlc.forward_info.payment_hash, prev_hop_data);
+						false
+					} else { true }
+				});
 			}
 		}
 

lightning/src/ln/payment_tests.rs

@@ -16,13 +16,13 @@ use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ChannelMonitor, LATENCY_GRA
 use crate::chain::transaction::OutPoint;
 use crate::chain::keysinterface::KeysInterface;
 use crate::ln::channel::EXPIRE_PREV_CONFIG_TICKS;
-use crate::ln::channelmanager::{self, BREAKDOWN_TIMEOUT, ChannelManager, ChannelManagerReadArgs, InterceptId, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS};
+use crate::ln::channelmanager::{self, BREAKDOWN_TIMEOUT, ChannelManager, ChannelManagerReadArgs, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS};
 use crate::ln::msgs;
 use crate::ln::msgs::ChannelMessageHandler;
 use crate::routing::gossip::RoutingFees;
 use crate::routing::router::{find_route, get_route, PaymentParameters, RouteHint, RouteHintHop, RouteParameters};
 use crate::util::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider};
-use crate::util::test_utils;
+use crate::util::{byte_utils, test_utils};
 use crate::util::errors::APIError;
 use crate::util::enforcing_trait_impls::EnforcingSigner;
 use crate::util::ser::{ReadableArgs, Writeable};
@@ -1387,16 +1387,25 @@ fn abandoned_send_payment_idempotent() {
 	claim_payment(&nodes[0], &[&nodes[1]], second_payment_preimage);
 }
 
+#[derive(PartialEq)]
+enum InterceptTest {
+	Forward,
+	Fail,
+	Timeout,
+}
+
 #[test]
 fn intercepted_payment() {
 	// Test that detecting an intercept scid on payment forward will signal LDK to generate an
 	// intercept event, which the LSP can then use to either (a) open a JIT channel to forward the
 	// payment or (b) fail the payment.
-	do_test_intercepted_payment(false);
-	do_test_intercepted_payment(true);
+	do_test_intercepted_payment(InterceptTest::Forward);
+	do_test_intercepted_payment(InterceptTest::Fail);
+	// Make sure that intercepted payments will be automatically failed back if too many blocks pass.
+	do_test_intercepted_payment(InterceptTest::Timeout);
 }
 
-fn do_test_intercepted_payment(fail_intercept: bool) {
+fn do_test_intercepted_payment(test: InterceptTest) {
 	let chanmon_cfgs = create_chanmon_cfgs(3);
 	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
 
@@ -1471,7 +1480,7 @@ fn do_test_intercepted_payment(fail_intercept: bool) {
 	let unknown_scid_err = nodes[1].node.forward_intercepted_htlc(intercept_id, 4242, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err();
 	assert_eq!(unknown_scid_err, APIError::APIMisuseError { err: "Channel with short channel id 4242 not found".to_string() });
 
-	if fail_intercept {
+	if test == InterceptTest::Fail {
 		// Ensure we can fail the intercepted payment back.
 		nodes[1].node.fail_intercepted_htlc(intercept_id).unwrap();
 		expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[1], vec![HTLCDestination::UnknownNextHop { requested_forward_scid: intercept_scid }]);
@@ -1489,15 +1498,10 @@ fn do_test_intercepted_payment(fail_intercept: bool) {
 			.blamed_chan_closed(true)
 			.expected_htlc_error_data(0x4000 | 10, &[]);
 			expect_payment_failed_conditions(&nodes[0], payment_hash, false, fail_conditions);
-	} else {
+	} else if test == InterceptTest::Forward {
 		// Open the just-in-time channel so the payment can then be forwarded.
 		let scid = create_announced_chan_between_nodes(&nodes, 1, 2, channelmanager::provided_init_features(), channelmanager::provided_init_features()).0.contents.short_channel_id;
 
-		// Check for unknown intercept id error.
-		let unknown_intercept_id = InterceptId([42; 32]);
-		let unknown_intercept_id_err = nodes[1].node.forward_intercepted_htlc(unknown_intercept_id, scid, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err();
-		assert_eq!(unknown_intercept_id_err , APIError::APIMisuseError { err: format!("Payment with intercept id {:?} not found", unknown_intercept_id.0) });
-
 		// Finally, forward the intercepted payment through and claim it.
 		nodes[1].node.forward_intercepted_htlc(intercept_id, scid, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap();
 		expect_pending_htlcs_forwardable!(nodes[1]);
@@ -1535,5 +1539,38 @@ fn do_test_intercepted_payment(fail_intercept: bool) {
 			},
 			_ => panic!("Unexpected event")
 		}
+	} else if test == InterceptTest::Timeout {
+		let mut block = Block {
+			header: BlockHeader { version: 0x20000000, prev_blockhash: nodes[0].best_block_hash(), merkle_root: TxMerkleNode::all_zeros(), time: 42, bits: 42, nonce: 42 },
+			txdata: vec![],
+		};
+		connect_block(&nodes[0], &block);
+		connect_block(&nodes[1], &block);
+		let block_count = 183; // find_route adds a random CLTV offset, so hardcode rather than summing consts
+		for _ in 0..block_count {
+			block.header.prev_blockhash = block.block_hash();
+			connect_block(&nodes[0], &block);
+			connect_block(&nodes[1], &block);
+		}
+		expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[1], vec![HTLCDestination::FailedPayment { payment_hash }]);
+		check_added_monitors!(nodes[1], 1);
+		let htlc_timeout_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+		assert!(htlc_timeout_updates.update_add_htlcs.is_empty());
+		assert_eq!(htlc_timeout_updates.update_fail_htlcs.len(), 1);
+		assert!(htlc_timeout_updates.update_fail_malformed_htlcs.is_empty());
+		assert!(htlc_timeout_updates.update_fee.is_none());
+
+		nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &htlc_timeout_updates.update_fail_htlcs[0]);
+		commitment_signed_dance!(nodes[0], nodes[1], htlc_timeout_updates.commitment_signed, false);
+		// amt_msat as u64, followed by the height at which we failed back above
+		let mut expected_failure_data = byte_utils::be64_to_array(amt_msat).to_vec();
+		expected_failure_data.extend_from_slice(&byte_utils::be32_to_array(block_count - 1));
+		expect_payment_failed!(nodes[0], payment_hash, false, 0x4000 | 15, &expected_failure_data[..]);
+
+		let scid = create_announced_chan_between_nodes(&nodes, 1, 2, channelmanager::provided_init_features(), channelmanager::provided_init_features()).0.contents.short_channel_id;
+
+		// Check for unknown intercept id error.
+		let unknown_intercept_id_err = nodes[1].node.forward_intercepted_htlc(intercept_id, scid, nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err();
+		assert_eq!(unknown_intercept_id_err , APIError::APIMisuseError { err: format!("Payment with intercept id {:?} not found", intercept_id.0) });
 	}
 }