Payment silently stuck when channel force-closed during in-progress monitor update

[joostjager]

Discovered via #4381

When a payment's HTLC is in LocalAnnounced state and the corresponding ChannelMonitorUpdate has been dispatched to the ChainMonitor (which returned InProgress) but not yet persisted, force-closing the channel causes the payment to be silently dropped. No PaymentPathFailed or PaymentFailed event is ever generated, leaving the payment permanently stuck in Pending state.

The root cause is in the channel closure path in channel.rs. When building dropped_outbound_htlcs, the code iterates LocalAnnounced HTLCs and checks blocked_monitor_updates to find HTLCs that the ChannelMonitor doesn't know about yet. However, if the monitor update was already dispatched to the ChainMonitor (returned InProgress) and is no longer in blocked_monitor_updates, the HTLC is assumed to be known by the monitor. This is incorrect: the monitor update may not have been applied yet, so the ChannelMonitor doesn't know about the HTLC either. The HTLC falls through the cracks, neither the channel nor the monitor fails it back, and no payment failure event is generated.

Completing the monitor update after the force-close does not help either. Once the ChannelMonitor learns about the HTLC, it has no on-chain event to trigger resolution since the broadcast commitment transaction does not include the uncommitted HTLC. The payment is permanently stuck with no recovery path.

Reproduction sequence:

1. Send a payment while the monitor persister returns InProgress
2. The HTLC enters LocalAnnounced state; the monitor update is dispatched but not yet persisted
3. Force-close the channel
4. The closure reports "0 HTLCs to fail" and no PaymentFailed event is generated
5. Completing the pending monitor update afterwards still does not resolve the payment
6. The payment remains in Pending state indefinitely

#[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 a PaymentFailed event 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_a_id = nodes[0].node.get_our_node_id();
	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.
	// No messages should have been sent yet.
	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());

	// Force-close the channel while the monitor update is still InProgress.
	let message = "Channel force-closed".to_owned();
	let reason = ClosureReason::HolderForceClosed {
		broadcasted_latest_txn: Some(true),
		message: message.clone(),
	};
	nodes[0].node.force_close_broadcasting_latest_txn(&channel_id, &node_b_id, message).unwrap();
	check_added_monitors(&nodes[0], 1);
	check_closed_broadcast!(nodes[0], true);
	check_closed_event(&nodes[0], 1, reason, &[node_b_id], 100000);

	// The payment should be failed back since the channel is now closed and the HTLC was never
	// committed to the counterparty.
	let events = nodes[0].node.get_and_clear_pending_events();
	let found_failed = events.iter().any(
		|ev| matches!(ev, Event::PaymentFailed { payment_id: ev_id, .. } if *ev_id == payment_id),
	);

	if !found_failed {
		// Even completing the pending monitor update does not resolve the payment: the
		// ChannelMonitor learns about the HTLC but the on-chain commitment tx does not
		// include it, so no failure event is generated.
		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);

		let events_after = nodes[0].node.get_and_clear_pending_events();
		let found_after = events_after.iter().any(
			|ev| matches!(ev, Event::PaymentFailed { payment_id: ev_id, .. } if *ev_id == payment_id),
		);
		assert!(
			found_after,
			"Expected PaymentFailed event for the stuck payment. \
			 Not found on force-close (events: {:?}), and not found after completing \
			 monitor update (events: {:?})",
			events, events_after
		);
	}
}

[Claude]

[wpaulino]

Looks like we just need to also consider the set of updates included in PeerState::in_flight_monitor_updates?

[joostjager]

Looks like we just need to also consider the set of updates included in PeerState::in_flight_monitor_updates?

Applied that fix indeed

[joostjager]

This doesn't appear to be a real issue. The payment resolves to failure once the commitment confirms. See also #4434 (comment)