Merge PR 'Fail held HTLC failures when force-closing' (#5050)

### lightning/src/chain/channelmonitor.rs
@@ -696,6 +696,10 @@ pub(crate) enum ChannelMonitorUpdateStep {
 		/// If set to false, we shouldn't broadcast the latest holder commitment transaction as we
 		/// think we've fallen behind!
 		should_broadcast: bool,
+		/// HTLCs which the counterparty failed but whose failures had not been handled when the
+		/// channel was closed. Those which are no longer in any commitment transaction we track
+		/// are failed once this update is applied.
+		counterparty_failed_htlcs: Vec<(HTLCSource, PaymentHash)>,
 	},
 	ShutdownScript {
 		scriptpubkey: ScriptBuf,
@@ -770,6 +774,7 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
 	},
 	(4, ChannelForceClosed) => {
 		(0, should_broadcast, required),
+		(1, counterparty_failed_htlcs, optional_vec),
 	},
 	(5, ShutdownScript) => {
 		(0, scriptpubkey, required),
@@ -4461,9 +4466,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 						ret = Err(());
 					}
 				},
-				ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } => {
+				ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast, counterparty_failed_htlcs } => {
 					log_trace!(logger, "Updating ChannelMonitor: channel force closed, should broadcast: {}", should_broadcast);
 					self.lockdown_from_offchain = true;
+					self.fail_counterparty_failed_htlcs(counterparty_failed_htlcs, logger, entropy_source);
 					if *should_broadcast {
 						// There's no need to broadcast our commitment transaction if we've seen one
 						// confirmed (even with 1 confirmation) as it'll be rejected as
@@ -4601,6 +4607,77 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		self.funding_spend_seen || self.lockdown_from_offchain || self.holder_tx_signed
 	}
 
+	/// Fails HTLCs which the counterparty failed off-chain but whose failures were not handled
+	/// before the channel was closed.
+	///
+	/// HTLCs still in a commitment transaction we track are skipped: we may not have been given
+	/// the revocation of the counterparty commitment transaction containing them, in which case
+	/// the counterparty can still claim them on-chain. They are instead resolved on-chain or, for
+	/// forwarded HTLCs, failed backwards as the inbound HTLCs approach expiry, like any other HTLC
+	/// we track.
+	fn fail_counterparty_failed_htlcs<L: Logger, ES: EntropySource>(
+		&mut self, htlcs: &[(HTLCSource, PaymentHash)], logger: &WithContext<L>,
+		entropy_source: &ES,
+	) {
+		let in_counterparty_commitment = |source: &HTLCSource| {
+			[
+				self.funding.current_counterparty_commitment_txid,
+				self.funding.prev_counterparty_commitment_txid,
+			]
+			.into_iter()
+			.flatten()
+			.filter_map(|txid| self.funding.counterparty_claimable_outpoints.get(&txid))
+			.flatten()
+			.any(|(_, source_opt)| source_opt.as_deref() == Some(source))
+		};
+		for (source, payment_hash) in htlcs {
+			let logger = WithContext::from(logger, None, None, Some(*payment_hash));
+			if in_counterparty_commitment(source) {
+				log_trace!(
+					logger,
+					"Not failing HTLC as it is still in a counterparty commitment transaction"
+				);
+				continue;
+			}
+			// An HTLC the counterparty failed is removed from our commitment transaction before
+			// it is removed from theirs, so it can't be in ours if it is in neither of their
+			// unrevoked ones.
+			debug_assert!(!holder_commitment_htlcs!(self, CURRENT_WITH_SOURCES)
+				.any(|(_, s)| s == Some(source)));
+			let duplicate_event =
+				self.pending_monitor_events.iter().chain(self.provided_monitor_events.iter()).any(
+					|(_, event)| match event {
+						MonitorEvent::HTLCEvent(upd) => upd.source == *source,
+						_ => false,
+					},
+				);
+			if duplicate_event {
+				continue;
+			}
+			// The HTLC can't have been failed on-chain, as the counterparty revoked the commitment
+			// transaction containing it and we'd have swept it ourselves had it been broadcast.
+			let newly_failed = self.failed_back_htlc_ids.insert(SentHTLCId::from_source(source));
+			debug_assert!(newly_failed);
+			log_trace!(logger, "Failing HTLC the counterparty failed before the channel closed");
+			// The value isn't used when failing HTLCs.
+			let htlc_value_msat = match source {
+				HTLCSource::OutboundRoute { first_hop_htlc_msat, .. } => Some(*first_hop_htlc_msat),
+				_ => source.inbound_htlc_amount_msat(),
+			};
+			let htlc_value_satoshis = htlc_value_msat.unwrap_or(0) / 1000;
+			push_monitor_event(
+				&mut self.pending_monitor_events,
+				MonitorEvent::HTLCEvent(HTLCUpdate {
+					source: source.clone(),
+					payment_preimage: None,
+					payment_hash: *payment_hash,
+					htlc_value_satoshis,
+				}),
+				entropy_source,
+			);
+		}
+	}
+
 	/// Given outbound HTLCs from a counterparty commitment update, checks if the funding output
 	/// has been spent on-chain. If so, creates `OnchainEvent::HTLCUpdate` entries to fail back
 	/// HTLCs that weren't already known to the monitor.

### lightning/src/ln/chanmon_update_fail_tests.rs
@@ -14,20 +14,26 @@
 
 use crate::chain::chaininterface::LowerBoundedFeeEstimator;
 use crate::chain::chainmonitor::ChainMonitor;
-use crate::chain::channelmonitor::{ChannelMonitor, MonitorEvent, ANTI_REORG_DELAY};
+use crate::chain::channelmonitor::{
+	ChannelMonitor, MonitorEvent, ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS,
+};
 use crate::chain::transaction::OutPoint;
 use crate::chain::{BlockLocator, ChannelMonitorUpdateStatus, Confirm, Listen, Watch};
-use crate::events::{ClosureReason, Event, HTLCHandlingFailureType, PaymentPurpose};
+use crate::events::{
+	ClosureReason, Event, HTLCHandlingFailureReason, HTLCHandlingFailureType, PaymentPurpose,
+};
 use crate::ln::channel::AnnouncementSigsState;
 use crate::ln::channelmanager::{PaymentId, RAACommitmentOrder, TrustedChannelFeatures};
 use crate::ln::msgs;
 use crate::ln::msgs::{
 	BaseMessageHandler, ChannelMessageHandler, MessageSendEvent, RoutingMessageHandler,
 };
+use crate::ln::onion_utils::LocalHTLCFailureReason;
 use crate::ln::outbound_payment::{RecipientOnionFields, Retry};
 use crate::ln::types::ChannelId;
 use crate::routing::router::{PaymentParameters, RouteParameters};
 use crate::sign::NodeSigner;
+use crate::types::string::UntrustedString;
 use crate::util::native_async::FutureQueue;
 use crate::util::persist::{
 	MonitorName, MonitorUpdatingPersisterAsync, CHANNEL_MONITOR_PERSISTENCE_PRIMARY_NAMESPACE,
@@ -5734,3 +5740,337 @@ fn test_stale_manager_with_since_applied_blocked_mon_update() {
 	expect_payment_sent(&nodes[0], preimage_2, None, true, true);
 	expect_payment_forwarded!(nodes[1], nodes[0], nodes[2], Some(1000), false, true);
 }
+
+#[derive(Clone, Copy, PartialEq)]
+enum ForceCloseTrigger {
+	Holder,
+	CounterpartyError,
+	CounterpartyCommitmentConfirmed,
+}
+
+fn do_test_force_close_with_monitor_pending_htlc_fail(
+	trigger: ForceCloseTrigger, reload: bool, stale_manager: bool,
+) {
+	// When our counterparty fails an HTLC we forwarded, we hold the failure until the
+	// `ChannelMonitorUpdate` for their `revoke_and_ack` completes. If the channel is force-closed
+	// while that update is in-flight, the held failure can no longer be released, and as the HTLC
+	// is no longer in any commitment transaction the `ChannelMonitor` tracks, it wouldn't be
+	// resolved on-chain either. Instead, the closing `ChannelMonitorUpdate` hands such HTLCs to the
+	// `ChannelMonitor`, which fails them backwards once both updates have been persisted. Tested
+	// with the closure triggered by us and by the counterparty, and across a restart, including
+	// with a `ChannelManager` from before the force-close, which still holds the failure and
+	// force-closes the channel on startup as the `ChannelMonitor` is ahead of it.
+	assert!(reload || !stale_manager);
+	assert!(trigger == ForceCloseTrigger::Holder || !reload);
+	let chanmon_cfgs = create_chanmon_cfgs(3);
+	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+	let legacy_cfg = test_legacy_channel_config();
+	let persister;
+	let new_chain_monitor;
+	let nodes_1_deserialized;
+	let node_chanmgrs = create_node_chanmgrs(
+		3,
+		&node_cfgs,
+		&[Some(legacy_cfg.clone()), Some(legacy_cfg.clone()), Some(legacy_cfg)],
+	);
+	let mut nodes = create_network(3, &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 node_c_id = nodes[2].node.get_our_node_id();
+
+	let chan_id_ab = create_announced_chan_between_nodes(&nodes, 0, 1).2;
+	let chan_id_bc = create_announced_chan_between_nodes(&nodes, 1, 2).2;
+
+	let (_, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
+
+	nodes[2].node.fail_htlc_backwards(&payment_hash);
+	expect_and_process_pending_htlcs_and_htlc_handling_failed(
+		&nodes[2],
+		&[HTLCHandlingFailureType::Receive { payment_hash }],
+	);
+	check_added_monitors(&nodes[2], 1);
+
+	// Run the commitment dance removing the HTLC from the B <-> C channel, but leave B's
+	// `ChannelMonitorUpdate` for C's final `revoke_and_ack` in-flight.
+	let cs_updates = get_htlc_update_msgs(&nodes[2], &node_b_id);
+	nodes[1].node.handle_update_fail_htlc(node_c_id, &cs_updates.update_fail_htlcs[0]);
+	let cs_raa = commitment_signed_dance_return_raa(
+		&nodes[1],
+		&nodes[2],
+		&cs_updates.commitment_signed,
+		true,
+	);
+
+	chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
+	nodes[1].node.handle_revoke_and_ack(node_c_id, &cs_raa);
+	check_added_monitors(&nodes[1], 1);
+	let (_, raa_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_bc);
+	assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+	assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
+	assert!(get_monitor!(nodes[1], chan_id_bc).get_all_current_outbound_htlcs().is_empty());
+	let stale_node_b_ser = nodes[1].node.encode();
+
+	chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
+	match trigger {
+		ForceCloseTrigger::Holder => {
+			let message = "Channel force-closed".to_owned();
+			nodes[1]
+				.node
+				.force_close_broadcasting_latest_txn(&chan_id_bc, &node_c_id, message.clone())
+				.unwrap();
+			check_added_monitors(&nodes[1], 1);
+			check_closed_broadcast(&nodes[1], 1, true);
+			let reason =
+				ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true), message };
+			check_closed_event(&nodes[1], 1, reason, &[node_c_id], 100_000);
+		},
+		ForceCloseTrigger::CounterpartyError => {
+			let data = "Bogus error".to_owned();
+			let msg = msgs::ErrorMessage { channel_id: chan_id_bc, data: data.clone() };
+			nodes[1].node.handle_error(node_c_id, &msg);
+			check_added_monitors(&nodes[1], 1);
+			check_closed_broadcast(&nodes[1], 1, false);
+			let reason = ClosureReason::CounterpartyForceClosed { peer_msg: UntrustedString(data) };
+			check_closed_event(&nodes[1], 1, reason, &[node_c_id], 100_000);
+		},
+		ForceCloseTrigger::CounterpartyCommitmentConfirmed => {
+			let cs_commitment_tx = get_local_commitment_txn!(nodes[2], chan_id_bc);
+			mine_transaction(&nodes[1], &cs_commitment_tx[0]);
+			let reason = ClosureReason::CommitmentTxConfirmed;
+			check_closed_event(&nodes[1], 1, reason, &[node_c_id], 100_000);
+			check_closed_broadcast(&nodes[1], 1, true);
+			check_added_monitors(&nodes[1], 1);
+		},
+	}
+	let (_, close_update_id) = nodes[1].chain_monitor.get_latest_mon_update_id(chan_id_bc);
+
+	if reload {
+		let node_b_ser = if stale_manager { stale_node_b_ser } else { nodes[1].node.encode() };
+		let mon_ab_ser = get_monitor!(nodes[1], chan_id_ab).encode();
+		let mon_bc_ser = get_monitor!(nodes[1], chan_id_bc).encode();
+		// Reconstructing the pending HTLC set from the `Channel`s would fail the HTLC on startup
+		// already, so use the persisted set to test the failure by the `ChannelMonitor`.
+		reload_node!(
+			nodes[1],
+			&node_b_ser,
+			&[&mon_ab_ser, &mon_bc_ser],
+			persister,
+			new_chain_monitor,
+			nodes_1_deserialized,
+			Some(false)
+		);
+		if stale_manager {
+			// The stale `ChannelManager` still has the channel open, so it force-closes it on
+			// startup.
+			nodes[1].node.test_process_background_events();
+			check_added_monitors(&nodes[1], 1);
+		}
+		nodes[0].node.peer_disconnected(node_b_id);
+		nodes[2].node.peer_disconnected(node_b_id);
+		reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
+	} else {
+		// The failure isn't released while either update is in-flight.
+		let chain_monitor = &nodes[1].chain_monitor.chain_monitor;
+		chain_monitor.channel_monitor_updated(chan_id_bc, close_update_id).unwrap();
+		assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
+		assert!(!nodes[1].node.needs_pending_htlc_processing());
+		chain_monitor.channel_monitor_updated(chan_id_bc, raa_update_id).unwrap();
+	}
+
+	let mut events = nodes[1].node.get_and_clear_pending_events();
+	if stale_manager {
+		match events.remove(0) {
+			Event::ChannelClosed { reason: ClosureReason::OutdatedChannelManager, .. } => {},
+			ev => panic!("Unexpected event: {ev:?}"),
+		}
+	}
+	assert_eq!(events.len(), 1, "{events:?}");
+	match &events[0] {
+		Event::HTLCHandlingFailed { failure_type, failure_reason, .. } => {
+			let expected_type = HTLCHandlingFailureType::Forward {
+				node_id: Some(node_c_id),
+				channel_id: chan_id_bc,
+			};
+			assert_eq!(*failure_type, expected_type);
+			let reason = LocalHTLCFailureReason::OnChainTimeout;
+			assert_eq!(*failure_reason, Some(HTLCHandlingFailureReason::Local { reason }));
+		},
+		ev => panic!("Unexpected event: {ev:?}"),
+	}
+	expect_and_process_pending_htlcs(&nodes[1], false);
+	check_added_monitors(&nodes[1], 1);
+	let bs_updates = get_htlc_update_msgs(&nodes[1], &node_a_id);
+	nodes[0].node.handle_update_fail_htlc(node_b_id, &bs_updates.update_fail_htlcs[0]);
+	do_commitment_signed_dance(&nodes[0], &nodes[1], &bs_updates.commitment_signed, false, false);
+	let conditions = PaymentFailedConditions::new().blamed_chan_closed(true);
+	expect_payment_failed_conditions(&nodes[0], payment_hash, false, conditions);
+}
+
+#[test]
+fn test_force_close_with_monitor_pending_htlc_fail() {
+	do_test_force_close_with_monitor_pending_htlc_fail(ForceCloseTrigger::Holder, false, false);
+	do_test_force_close_with_monitor_pending_htlc_fail(ForceCloseTrigger::Holder, true, false);
+	do_test_force_close_with_monitor_pending_htlc_fail(ForceCloseTrigger::Holder, true, true);
+	let trigger = ForceCloseTrigger::CounterpartyError;
+	do_test_force_close_with_monitor_pending_htlc_fail(trigger, false, false);
+	let trigger = ForceCloseTrigger::CounterpartyCommitmentConfirmed;
+	do_test_force_close_with_monitor_pending_htlc_fail(trigger, false, false);
+}
+
+#[test]
+fn test_force_close_with_blocked_raa_htlc_fail() {
+	// If the `ChannelMonitorUpdate` for the counterparty's `revoke_and_ack` is blocked rather than
+	// in-flight when the channel is force-closed, the `ChannelMonitor` never learns the
+	// revocation. It then still tracks the failed HTLC in the counterparty's previous commitment
+	// transaction, which the counterparty could still broadcast to claim it. Thus, the HTLC must
+	// not be failed backwards when the channel closes, but only as the inbound HTLC approaches
+	// expiry, like any other HTLC the `ChannelMonitor` tracks.
+	let chanmon_cfgs = create_chanmon_cfgs(3);
+	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+	let legacy_cfg = test_legacy_channel_config();
+	let node_chanmgrs = create_node_chanmgrs(
+		3,
+		&node_cfgs,
+		&[Some(legacy_cfg.clone()), Some(legacy_cfg.clone()), Some(legacy_cfg)],
+	);
+	let nodes = create_network(3, &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 node_c_id = nodes[2].node.get_our_node_id();
+
+	let chan_id_ab = create_announced_chan_between_nodes(&nodes, 0, 1).2;
+	let chan_id_bc = create_announced_chan_between_nodes(&nodes, 1, 2).2;
+
+	let (preimage_1, payment_hash_1, ..) =
+		route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
+	let (_, payment_hash_2, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
+	let (_, payment_hash_3, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
+
+	// C fails payment 3, and B responds with its `revoke_and_ack` and `commitment_signed`.
+	nodes[2].node.fail_htlc_backwards(&payment_hash_3);
+	expect_and_process_pending_htlcs_and_htlc_handling_failed(
+		&nodes[2],
+		&[HTLCHandlingFailureType::Receive { payment_hash: payment_hash_3 }],
+	);
+	check_added_monitors(&nodes[2], 1);
+	let cs_updates = get_htlc_update_msgs(&nodes[2], &node_b_id);
+	nodes[1].node.handle_update_fail_htlc(node_c_id, &cs_updates.update_fail_htlcs[0]);
+	nodes[1].node.handle_commitment_signed_batch_test(node_c_id, &cs_updates.commitment_signed);
+	check_added_monitors(&nodes[1], 1);
+	let (bs_raa, bs_commitment_signed) = get_revoke_commit_msgs(&nodes[1], &node_c_id);
+
+	// While C awaits B's `revoke_and_ack`, it claims payment 1 and fails payment 2, both of which
+	// go into its holding cell and are sent together once B's `revoke_and_ack` arrives.
+	nodes[2].node.claim_funds(preimage_1, Default::default());
+	check_added_monitors(&nodes[2], 1);
+	expect_payment_claimed!(nodes[2], payment_hash_1, 1_000_000);
+	nodes[2].node.fail_htlc_backwards(&payment_hash_2);
+	expect_and_process_pending_htlcs_and_htlc_handling_failed(
+		&nodes[2],
+		&[HTLCHandlingFailureType::Receive { payment_hash: payment_hash_2 }],
+	);
+	assert!(nodes[2].node.get_and_clear_pending_msg_events().is_empty());
+
+	nodes[2].node.handle_revoke_and_ack(node_b_id, &bs_raa);
+	check_added_monitors(&nodes[2], 1);
+	let mut cs_updates = get_htlc_update_msgs(&nodes[2], &node_b_id);
+	assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
+	assert_eq!(cs_updates.update_fail_htlcs.len(), 1);
+	nodes[2].node.handle_commitment_signed_batch_test(node_b_id, &bs_commitment_signed);
+	check_added_monitors(&nodes[2], 1);
+	let cs_raa = get_event_msg!(nodes[2], MessageSendEvent::SendRevokeAndACK, node_b_id);
+
+	// B claims payment 1 upstream, leaving the A <-> B `ChannelMonitorUpdate` in-flight, which
+	// blocks the `ChannelMonitorUpdate` for C's `revoke_and_ack` that removes payment 3.
+	chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
+	nodes[1].node.handle_update_fulfill_htlc(node_c_id, cs_updates.update_fulfill_htlcs.remove(0));
+	check_added_monitors(&nodes[1], 1);
+	nodes[1].node.handle_update_fail_htlc(node_c_id, &cs_updates.update_fail_htlcs[0]);
+	nodes[1].node.handle_commitment_signed_batch_test(node_c_id, &cs_updates.commitment_signed);
+	check_added_monitors(&nodes[1], 1);
+	nodes[1].node.handle_revoke_and_ack(node_c_id, &cs_raa);
+	check_added_monitors(&nodes[1], 0);
+	get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, node_c_id);
+	assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
+
+	let message = "Channel force-closed".to_owned();
+	nodes[1]
+		.node
+		.force_close_broadcasting_latest_txn(&chan_id_bc, &node_c_id, message.clone())
+		.unwrap();
+	check_added_monitors(&nodes[1], 1);
+	check_closed_broadcast(&nodes[1], 1, true);
+	let reason = ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true), message };
+	check_closed_event(&nodes[1], 1, reason, &[node_c_id], 100_000);
+
+	// Once all updates complete, B forwards the claim of payment 1 but doesn't fail payment 3.
+	let chain_monitor = &nodes[1].chain_monitor.chain_monitor;
+	for (channel_id, update_ids) in chain_monitor.list_pending_monitor_updates() {
+		for update_id in update_ids {
+			chain_monitor.channel_monitor_updated(channel_id, update_id).unwrap();
+		}
+	}
+	let events = nodes[1].node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 1, "{events:?}");
+	assert!(matches!(events[0], Event::PaymentForwarded { .. }), "{events:?}");
+	assert!(!nodes[1].node.needs_pending_htlc_processing());
+	check_added_monitors(&nodes[1], 0);
+	let mut bs_updates = get_htlc_update_msgs(&nodes[1], &node_a_id);
+	nodes[0].node.handle_update_fulfill_htlc(node_b_id, bs_updates.update_fulfill_htlcs.remove(0));
+	do_commitment_signed_dance(&nodes[0], &nodes[1], &bs_updates.commitment_signed, false, false);
+	expect_payment_sent(&nodes[0], preimage_1, None, true, true);
+
+	// Payments 2 and 3 are failed backwards once the inbound HTLCs approach expiry.
+	let htlc_expiry = nodes[1]
+		.node
+		.list_channels()
+		.iter()
+		.find(|chan| chan.channel_id == chan_id_ab)
+		.unwrap()
+		.pending_inbound_htlcs
+		.iter()
+		.map(|htlc| htlc.cltv_expiry)
+		.min()
+		.unwrap();
+	let fail_height = htlc_expiry - LATENCY_GRACE_PERIOD_BLOCKS;
+	connect_blocks(&nodes[1], fail_height - 1 - nodes[1].best_block_info().1);
+	assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
+	connect_blocks(&nodes[1], 1);
+	// The already-claimed payment 1 is also still in the counterparty's commitment transaction and
+	// is failed as well, which is ignored as the inbound HTLC has already been removed.
+	let events = nodes[1].node.get_and_clear_pending_events();
+	assert_eq!(events.len(), 3, "{events:?}");
+	for event in events {
+		match event {
+			Event::HTLCHandlingFailed { failure_reason, .. } => {
+				let reason = LocalHTLCFailureReason::OnChainTimeout;
+				assert_eq!(failure_reason, Some(HTLCHandlingFailureReason::Local { reason }));
+			},
+			ev => panic!("Unexpected event: {ev:?}"),
+		}
+	}
+	expect_and_process_pending_htlcs(&nodes[1], false);
+	check_added_monitors(&nodes[1], 1);
+	let bs_updates = get_htlc_update_msgs(&nodes[1], &node_a_id);
+	assert_eq!(bs_updates.update_fail_htlcs.len(), 2);
+	for update_fail in bs_updates.update_fail_htlcs.iter() {
+		nodes[0].node.handle_update_fail_htlc(node_b_id, update_fail);
+	}
+	do_commitment_signed_dance(&nodes[0], &nodes[1], &bs_updates.commitment_signed, false, false);
+	let mut failed_payments = Vec::new();
+	for event in nodes[0].node.get_and_clear_pending_events() {
+		match event {
+			Event::PaymentPathFailed { .. } => {},
+			Event::PaymentFailed { payment_hash, .. } => {
+				failed_payments.push(payment_hash.unwrap())
+			},
+			ev => panic!("Unexpected event: {ev:?}"),
+		}
+	}
+	failed_payments.sort();
+	let mut expected_payments = vec![payment_hash_2, payment_hash_3];
+	expected_payments.sort();
+	assert_eq!(failed_payments, expected_payments);
+}

### lightning/src/ln/channel.rs
@@ -3659,8 +3659,9 @@ pub(super) struct ChannelContext<SP: SignerProvider> {
 
 	// TODO: If a channel is drop'd, we don't know whether the `ChannelMonitor` is ultimately
 	// responsible for some of the HTLCs here or not - we don't know whether the update in question
-	// completed or not. We currently ignore these fields entirely when force-closing a channel,
-	// but need to handle this somehow or we run the risk of losing HTLCs!
+	// completed or not. Other than `monitor_pending_failures`, which are handed to the
+	// `ChannelMonitor` when force-closing, we currently ignore these fields entirely when
+	// force-closing a channel, but need to handle this somehow or we run the risk of losing HTLCs!
 	monitor_pending_forwards: Vec<(PendingHTLCInfo, u64)>,
 	monitor_pending_failures: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
 	monitor_pending_finalized_fulfills: Vec<(HTLCSource, Option<AttributionData>)>,
@@ -6715,10 +6716,20 @@ impl<SP: SignerProvider> ChannelContext<SP> {
 			if self.counterparty_next_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
 				self.latest_monitor_update_id = self.get_latest_unblocked_monitor_update_id() + 1;
 
+				// HTLC failures are held until the `ChannelMonitorUpdate` for the counterparty's
+				// `revoke_and_ack` completes. As they can no longer be released, hand them to the
+				// `ChannelMonitor` to fail. It may already have been given the revocation, in
+				// which case it no longer tracks the HTLCs and wouldn't resolve them otherwise.
+				let counterparty_failed_htlcs = self
+					.monitor_pending_failures
+					.drain(..)
+					.map(|(source, payment_hash, _)| (source, payment_hash))
+					.collect();
 				let update = ChannelMonitorUpdate {
 					update_id: self.latest_monitor_update_id,
 					updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed {
 						should_broadcast: broadcast,
+						counterparty_failed_htlcs,
 					}],
 					channel_id: Some(self.channel_id()),
 				};

### lightning/src/ln/channelmanager.rs
@@ -1124,6 +1124,16 @@ impl HTLCSource {
 		}
 	}
 
+	/// Returns the total amount of the inbound HTLC(s) (i.e. the source(s) referred to by this
+	/// object), if the source was a forwarded HTLC and all inbound HTLCs were first forwarded on
+	/// LDK 0.0.117 or later.
+	pub(crate) fn inbound_htlc_amount_msat(&self) -> Option<u64> {
+		match self {
+			Self::OutboundRoute { .. } => None,
+			_ => self.previous_hop_data().iter().map(|hop| hop.amount_msat).sum(),
+		}
+	}
+
 	pub(crate) fn static_invoice(&self) -> Option<StaticInvoice> {
 		match self {
 			Self::OutboundRoute {
@@ -20249,6 +20259,7 @@ impl<
 					update_id: monitor.get_latest_update_id().saturating_add(1),
 					updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed {
 						should_broadcast: true,
+						counterparty_failed_htlcs: Vec::new(),
 					}],
 					channel_id: Some(monitor.channel_id()),
 				};

### lightning/src/util/persist.rs
@@ -2628,7 +2628,10 @@ mod tests {
 
 		let legacy_update = ChannelMonitorUpdate {
 			update_id: u64::MAX,
-			updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast: true }],
+			updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed {
+				should_broadcast: true,
+				counterparty_failed_htlcs: Vec::new(),
+			}],
 			channel_id: Some(monitor.channel_id()),
 		};
 

### lightning/src/util/test_utils.rs
@@ -699,7 +699,7 @@ impl<'a> chain::Watch<TestChannelSigner> for TestChainMonitor<'a> {
 			assert_eq!(channel_id, exp.0);
 			assert_eq!(update.updates.len(), 1);
 			let update = &update.updates[0];
-			if let ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } = update {
+			if let ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast, .. } = update {
 				assert_eq!(*should_broadcast, exp.1);
 			} else {
 				panic!();