fix duplicate HTLC fail-back on stale force-close

Author: Anyitechs

lightning/src/chain/channelmonitor.rs

@@ -5502,6 +5502,67 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			}
 		}
 
+		// Immediate fail-back on stale force-close, regardless of expiry or whether we're allowed to send further updates.
+		let current_counterparty_htlcs = if let Some(txid) = self.funding.current_counterparty_commitment_txid {
+			if let Some(htlc_outputs) = self.funding.counterparty_claimable_outpoints.get(&txid) {
+				Some(htlc_outputs.iter().map(|&(ref a, ref b)| (a, b.as_ref().map(|boxed| &**boxed))))
+			} else { None }
+		} else { None }.into_iter().flatten();
+
+		let prev_counterparty_htlcs = if let Some(txid) = self.funding.prev_counterparty_commitment_txid {
+			if let Some(htlc_outputs) = self.funding.counterparty_claimable_outpoints.get(&txid) {
+				Some(htlc_outputs.iter().map(|&(ref a, ref b)| (a, b.as_ref().map(|boxed| &**boxed))))
+			} else { None }
+		} else { None }.into_iter().flatten();
+
+		let htlcs = holder_commitment_htlcs!(self, CURRENT_WITH_SOURCES)
+			.chain(current_counterparty_htlcs)
+			.chain(prev_counterparty_htlcs);
+
+		// Group by payment hash so we fail back all identical HTLCs together
+		let mut htlcs_by_hash: HashMap<PaymentHash, Vec<(&HTLCOutputInCommitment, &HTLCSource)>> = new_hash_map();
+		for (htlc, source_opt) in htlcs {
+			if let Some(source) = source_opt {
+				htlcs_by_hash.entry(htlc.payment_hash).or_default().push((htlc, source));
+			}
+		}
+
+		let monitor_htlc_sources: HashSet<&HTLCSource> = self
+			.onchain_events_awaiting_threshold_conf
+			.iter()
+			.filter_map(|event_entry| match event_entry.event {
+				OnchainEvent::HTLCUpdate { ref source, .. } => Some(source),
+				_ => None,
+			})
+			.collect();
+
+		for (payment_hash, htlc_group) in htlcs_by_hash {
+			let is_any_htlc_missing = htlc_group
+				.iter()
+				.any(|(_, source)| !monitor_htlc_sources.contains(source));
+			if is_any_htlc_missing {
+				log_info!(logger,
+					"Detected stale force-close. Failing back related HTLCs for hash {}.",
+					&payment_hash);
+				for (htlc, source) in htlc_group {
+					if self
+						.failed_back_htlc_ids
+						.insert(SentHTLCId::from_source(source))
+					{
+						log_error!(logger,
+							"Failing back HTLC for payment {} due to stale close.",
+							log_bytes!(payment_hash.0));
+						self.pending_monitor_events.push(MonitorEvent::HTLCEvent(HTLCUpdate {
+							source: source.clone(),
+							payment_preimage: None,
+							payment_hash,
+							htlc_value_satoshis: Some(htlc.amount_msat / 1000),
+						}));
+					}
+				}
+			}
+		}
+
 		if self.no_further_updates_allowed() {
 			// Fail back HTLCs on backwards channels if they expire within
 			// `LATENCY_GRACE_PERIOD_BLOCKS` blocks and the channel is closed (i.e. we're at a

lightning/src/ln/functional_tests.rs

@@ -32,8 +32,8 @@ use crate::ln::channel::{
 	MIN_CHAN_DUST_LIMIT_SATOSHIS, UNFUNDED_CHANNEL_AGE_LIMIT_TICKS,
 };
 use crate::ln::channelmanager::{
-	PaymentId, RAACommitmentOrder, RecipientOnionFields, BREAKDOWN_TIMEOUT, DISABLE_GOSSIP_TICKS,
-	ENABLE_GOSSIP_TICKS, MIN_CLTV_EXPIRY_DELTA,
+	PaymentId, RAACommitmentOrder, RecipientOnionFields, Retry, BREAKDOWN_TIMEOUT,
+	DISABLE_GOSSIP_TICKS, ENABLE_GOSSIP_TICKS, MIN_CLTV_EXPIRY_DELTA,
 };
 use crate::ln::msgs;
 use crate::ln::msgs::{
@@ -9680,3 +9680,82 @@ pub fn test_multi_post_event_actions() {
 	do_test_multi_post_event_actions(true);
 	do_test_multi_post_event_actions(false);
 }
+
+#[xtest(feature = "_externalize_tests")]
+fn test_stale_force_close_with_identical_htlcs() {
+	let chanmon_cfgs = create_chanmon_cfgs(3);
+	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+	let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+	let chan_a_b = create_announced_chan_between_nodes(&nodes, 0, 1);
+	let _chan_b_c = create_announced_chan_between_nodes(&nodes, 1, 2);
+
+	let stale_tx = get_local_commitment_txn!(nodes[0], chan_a_b.2)[0].clone();
+
+	let (_payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash!(nodes[2]);
+
+	let make_event = |pid: PaymentId| {
+		let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), 42);
+		let route_params = RouteParameters::from_payment_params_and_value(payment_params, 10_000);
+		nodes[0]
+			.node
+			.send_payment(
+				payment_hash,
+				RecipientOnionFields::secret_only(payment_secret),
+				pid,
+				route_params,
+				Retry::Attempts(0),
+			)
+			.unwrap();
+		check_added_monitors!(&nodes[0], 1);
+		let mut events = nodes[0].node.get_and_clear_pending_msg_events();
+		remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events)
+	};
+
+	let first_event = make_event(PaymentId([1; 32]));
+	do_pass_along_path(
+		PassAlongPathArgs::new(
+			&nodes[0],
+			&[&nodes[1], &nodes[2]],
+			10_000,
+			payment_hash,
+			first_event,
+		)
+		.with_payment_secret(payment_secret)
+		.without_clearing_recipient_events()
+		.without_claimable_event(),
+	);
+
+	let second_event = make_event(PaymentId([2; 32]));
+	do_pass_along_path(
+		PassAlongPathArgs::new(
+			&nodes[0],
+			&[&nodes[1], &nodes[2]],
+			10_000,
+			payment_hash,
+			second_event,
+		)
+		.with_payment_secret(payment_secret)
+		.without_clearing_recipient_events()
+		.without_claimable_event(),
+	);
+
+	mine_transaction(&nodes[1], &stale_tx);
+	check_added_monitors(&nodes[1], 1);
+	nodes[1].node.process_pending_htlc_forwards();
+
+	let events = nodes[1].node.get_and_clear_pending_events();
+	let failed = events
+		.iter()
+		.filter(|e| matches!(e, crate::events::Event::HTLCHandlingFailed { .. }))
+		.count();
+	assert_eq!(
+		failed, 2,
+		"ChannelMonitor should immediately surface two HTLC failures after stale close"
+	);
+
+	nodes[1].node.get_and_clear_pending_msg_events();
+	nodes[2].node.get_and_clear_pending_events();
+	nodes[2].node.get_and_clear_pending_msg_events();
+}