Check in-flight monitor updates in force_shutdown for LocalAnnounced HTLCs

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

@@ -271,19 +271,24 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
 
 	// ...and make sure we can force-close a frozen channel
 	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], 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]

lightning/src/ln/channel.rs

@@ -2326,9 +2326,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]
@@ -5891,6 +5894,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
@@ -5919,14 +5923,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();
+				for update in blocked_iter.chain(in_flight_iter) {
+					for update in update.updates.iter() {
 						let have_htlc = match update {
 							ChannelMonitorUpdateStep::LatestCounterpartyCommitment {
 								htlc_data,
@@ -6730,10 +6736,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
 	}
@@ -9528,7 +9538,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))
 					}
 				}
@@ -13365,7 +13375,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

@@ -4478,7 +4478,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.as_slice())
+					.unwrap_or(&[]);
+				chan.force_shutdown(reason, in_flight)
 			};
 			let chan_update = self.get_channel_update_for_broadcast(chan).ok();
 
@@ -4540,7 +4544,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
@@ -18449,7 +18453,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);
 					}