diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index a8d055a9c5b..52a0590c314 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/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); } diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index cd32d219b93..dc658584c38 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/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] diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 15aa1daecfe..bca44428bed 100644 --- a/lightning/src/ln/channel.rs +++ b/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 ChannelContext { /// 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 ChannelContext { } // 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 { impl OutboundV1Channel { 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. diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6bf04cd62a4..c98480a6022 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/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); }