Check in-flight monitor updates in force_shutdown for LocalAnnounced HTLCs

When building dropped_outbound_htlcs during force_shutdown,
LocalAnnounced HTLCs were only checked against blocked_monitor_updates.
If the monitor update had already been dispatched to the ChainMonitor
(returned InProgress) and was sitting in PeerState::in_flight_monitor_updates,
the HTLC was silently dropped with no PaymentPathFailed or PaymentFailed
event generated.

Pass the in-flight monitor updates for the channel into force_shutdown
so both blocked and in-flight updates are checked. This ensures
LocalAnnounced HTLCs whose monitor updates have been dispatched but not
yet persisted are properly included in dropped_outbound_htlcs and failed
back to the sender.

Closes #4431

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: joostjager

lightning/src/chain/channelmonitor.rs

@@ -6760,7 +6760,7 @@ mod tests {
 	use bitcoin::{Sequence, Witness};
 
 	use crate::chain::chaininterface::LowerBoundedFeeEstimator;
-	use crate::events::ClosureReason;
+	use crate::events::{ClosureReason, Event};
 
 	use super::ChannelMonitorUpdateStep;
 	use crate::chain::channelmonitor::{ChannelMonitor, WithChannelMonitor};
@@ -6890,7 +6890,15 @@ mod tests {
 		check_spends!(htlc_txn[1], broadcast_tx);
 
 		check_closed_broadcast(&nodes[1], 1, true);
-		check_closed_event(&nodes[1], 1, ClosureReason::CommitmentTxConfirmed, &[nodes[0].node.get_our_node_id()], 100000);
+		let mut events = nodes[1].node.get_and_clear_pending_events();
+		assert_eq!(events.len(), 3);
+		assert!(matches!(
+			events.pop().unwrap(),
+			Event::ChannelClosed { reason: ClosureReason::CommitmentTxConfirmed, .. }
+		));
+		expect_payment_failed_conditions_event(
+			events, payment_hash, false, PaymentFailedConditions::new(),
+		);
 		check_added_monitors(&nodes[1], 1);
 	}
 

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -279,11 +279,20 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
 	check_added_monitors(&nodes[0], 1);
 	check_closed_broadcast(&nodes[0], 1, true);
 
-	// TODO: Once we hit the chain with the failure transaction we should check that we get a
-	// PaymentPathFailed event
-
 	assert_eq!(nodes[0].node.list_channels().len(), 0);
-	check_closed_event(&nodes[0], 1, reason, &[node_b_id], 100000);
+
+	let mut events = nodes[0].node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 3);
+	assert!(matches!(
+		events.pop().unwrap(),
+		Event::ChannelClosed { reason: ClosureReason::HolderForceClosed { .. }, .. }
+	));
+	expect_payment_failed_conditions_event(
+		events,
+		payment_hash_2,
+		false,
+		PaymentFailedConditions::new(),
+	);
 }
 
 #[test]
@@ -5167,3 +5176,66 @@ fn test_mpp_claim_to_holding_cell() {
 	expect_payment_claimable!(nodes[3], paymnt_hash_2, payment_secret_2, 400_000);
 	claim_payment(&nodes[2], &[&nodes[3]], preimage_2);
 }
+
+#[test]
+fn test_force_close_with_in_progress_monitor_update_drops_htlc() {
+	// When a channel is force-closed while a monitor update is InProgress, any HTLC in
+	// LocalAnnounced state (committed to the channel but monitor update not yet persisted) may
+	// not be included in the ChannelMonitor. Verify that the payment is properly failed back
+	// via PaymentPathFailed/PaymentFailed events rather than being silently dropped.
+	let chanmon_cfgs = create_chanmon_cfgs(2);
+	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+	let node_b_id = nodes[1].node.get_our_node_id();
+
+	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
+
+	let (route, payment_hash, _, payment_secret) =
+		get_route_and_payment_hash!(&nodes[0], nodes[1], 1_000_000);
+
+	// Set node A's monitor persistence to InProgress so the HTLC monitor update won't complete.
+	chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
+
+	let onion = RecipientOnionFields::secret_only(payment_secret);
+	let payment_id = PaymentId(payment_hash.0);
+	nodes[0].node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap();
+	check_added_monitors(&nodes[0], 1);
+
+	// The HTLC is now LocalAnnounced but the monitor update hasn't been persisted.
+	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+
+	// Force-close the channel while the monitor update is still InProgress.
+	nodes[0]
+		.node
+		.force_close_broadcasting_latest_txn(&channel_id, &node_b_id, "force close".to_owned())
+		.unwrap();
+	check_added_monitors(&nodes[0], 1);
+	check_closed_broadcast(&nodes[0], 1, true);
+
+	// The payment should be failed back since the channel is closed and the HTLC was never
+	// committed by the counterparty.
+	let mut events = nodes[0].node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 3);
+	assert!(matches!(
+		events.pop().unwrap(),
+		Event::ChannelClosed { reason: ClosureReason::HolderForceClosed { .. }, .. }
+	));
+	expect_payment_failed_conditions_event(
+		events,
+		payment_hash,
+		false,
+		PaymentFailedConditions::new(),
+	);
+
+	// Now complete the pending monitor update. The ChannelMonitor will learn about the HTLC,
+	// but the broadcast commitment transaction does not include it (it was never committed by
+	// the counterparty). Completing the update should not produce duplicate payment failure
+	// events or panics.
+	chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
+	let (latest_update, _) = nodes[0].chain_monitor.get_latest_mon_update_id(channel_id);
+	nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(channel_id, latest_update);
+
+	assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
+}

lightning/src/ln/channel.rs

@@ -2372,9 +2372,12 @@ where
 		})
 	}
 
-	pub fn force_shutdown(&mut self, closure_reason: ClosureReason) -> ShutdownResult {
+	pub fn force_shutdown(
+		&mut self, closure_reason: ClosureReason,
+		in_flight_monitor_updates: &[&ChannelMonitorUpdate],
+	) -> ShutdownResult {
 		let (funding, context) = self.funding_and_context_mut();
-		context.force_shutdown(funding, closure_reason)
+		context.force_shutdown(funding, closure_reason, in_flight_monitor_updates)
 	}
 
 	#[rustfmt::skip]
@@ -6239,6 +6242,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 	/// those explicitly stated to be alowed after shutdown, e.g. some simple getters).
 	fn force_shutdown(
 		&mut self, funding: &FundingScope, mut closure_reason: ClosureReason,
+		in_flight_monitor_updates: &[&ChannelMonitorUpdate],
 	) -> ShutdownResult {
 		// Note that we MUST only generate a monitor update that indicates force-closure - we're
 		// called during initialization prior to the chain_monitor in the encompassing ChannelManager
@@ -6267,14 +6271,16 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 		}
 
 		// Once we're closed, the `ChannelMonitor` is responsible for resolving any remaining
-		// HTLCs. However, in the specific case of us pushing new HTLC(s) to the counterparty in
-		// the latest commitment transaction that we haven't actually sent due to a block
-		// `ChannelMonitorUpdate`, we may have some HTLCs that the `ChannelMonitor` won't know
-		// about and thus really need to be included in `dropped_outbound_htlcs`.
+		// HTLCs. However, if we have HTLCs in `LocalAnnounced` state whose corresponding
+		// `ChannelMonitorUpdate` is either blocked (in `blocked_monitor_updates`) or dispatched
+		// but not yet persisted (in `in_flight_monitor_updates`), the `ChannelMonitor` may not
+		// know about them and they need to be included in `dropped_outbound_htlcs`.
 		'htlc_iter: for htlc in self.pending_outbound_htlcs.iter() {
 			if let OutboundHTLCState::LocalAnnounced(_) = htlc.state {
-				for update in self.blocked_monitor_updates.iter() {
-					for update in update.update.updates.iter() {
+				let blocked_iter = self.blocked_monitor_updates.iter().map(|u| &u.update);
+				let in_flight_iter = in_flight_monitor_updates.iter().copied();
+				for update in blocked_iter.chain(in_flight_iter) {
+					for update in update.updates.iter() {
 						let have_htlc = match update {
 							ChannelMonitorUpdateStep::LatestCounterpartyCommitment {
 								htlc_data,
@@ -7073,10 +7079,14 @@ where
 		&self.context
 	}
 
-	pub fn force_shutdown(&mut self, closure_reason: ClosureReason) -> ShutdownResult {
+	pub fn force_shutdown(
+		&mut self, closure_reason: ClosureReason,
+		in_flight_monitor_updates: &[&ChannelMonitorUpdate],
+	) -> ShutdownResult {
 		let splice_funding_failed = self.maybe_fail_splice_negotiation();
 
-		let mut shutdown_result = self.context.force_shutdown(&self.funding, closure_reason);
+		let mut shutdown_result =
+			self.context.force_shutdown(&self.funding, closure_reason, in_flight_monitor_updates);
 		shutdown_result.splice_funding_failed = splice_funding_failed;
 		shutdown_result
 	}
@@ -9869,7 +9879,7 @@ where
 						(closing_signed, signed_tx, shutdown_result)
 					}
 					Err(err) => {
-						let shutdown = self.context.force_shutdown(&self.funding, ClosureReason::ProcessingError {err: err.to_string()});
+						let shutdown = self.context.force_shutdown(&self.funding, ClosureReason::ProcessingError {err: err.to_string()}, &[]);
 						(None, None, Some(shutdown))
 					}
 				}
@@ -13706,7 +13716,7 @@ pub(super) struct OutboundV1Channel<SP: SignerProvider> {
 
 impl<SP: SignerProvider> OutboundV1Channel<SP> {
 	pub fn abandon_unfunded_chan(&mut self, closure_reason: ClosureReason) -> ShutdownResult {
-		self.context.force_shutdown(&self.funding, closure_reason)
+		self.context.force_shutdown(&self.funding, closure_reason, &[])
 	}
 
 	#[allow(dead_code)] // TODO(dual_funding): Remove once opending V2 channels is enabled.

lightning/src/ln/channelmanager.rs

@@ -4431,7 +4431,11 @@ impl<
 			let mut shutdown_res = if let Some(res) = coop_close_shutdown_res {
 				res
 			} else {
-				chan.force_shutdown(reason)
+				let in_flight = in_flight_monitor_updates
+					.get(&chan_id)
+					.map(|(_, updates)| updates.iter().collect::<Vec<_>>())
+					.unwrap_or_default();
+				chan.force_shutdown(reason, &in_flight)
 			};
 			let chan_update = self.get_channel_update_for_broadcast(chan).ok();
 
@@ -4493,7 +4497,7 @@ impl<
 		convert_channel_err_internal(err, chan_id, |reason, msg| {
 			let logger = WithChannelContext::from(&self.logger, chan.context(), None);
 
-			let shutdown_res = chan.force_shutdown(reason);
+			let shutdown_res = chan.force_shutdown(reason, &[]);
 			log_error!(logger, "Closed channel due to close-required error: {}", msg);
 			self.short_to_chan_info.write().unwrap().remove(&chan.context().outbound_scid_alias());
 			// If the channel was never confirmed on-chain prior to its closure, remove the
@@ -18300,7 +18304,7 @@ impl<
 							monitor.get_cur_counterparty_commitment_number(), channel.get_cur_counterparty_commitment_transaction_number());
 					}
 					let shutdown_result =
-						channel.force_shutdown(ClosureReason::OutdatedChannelManager);
+						channel.force_shutdown(ClosureReason::OutdatedChannelManager, &[]);
 					if shutdown_result.unbroadcasted_batch_funding_txid.is_some() {
 						return Err(DecodeError::InvalidValue);
 					}