Avoid startup `PeerState` entries for peers with unfunded channels

If a peer creates a channel with us which never reaches the funding
stage (or never gets any commitment updates after creation), we'll
avoid inserting the `update_id` into
`closed_channel_monitor_update_ids` at runtime to avoid keeping a
`PeerState` entry around for no reason. However, on startup we
still create a `ChannelMonitorUpdate` with a `ChannelForceClosed`
update step to ensure the `ChannelMonitor` is locked and shut down.

This is pretty redundant, and results in a bunch of on-startup
`ChannelMonitorUpdate`s for any old but non-archived
`ChannelMonitor`s. Instead, here, we check if a `ChannelMonitor`
already saw a `ChannelForceClosed` update step before we generate
the on-startup `ChannelMonitorUpdate`.

This also allows us to skip the `closed_channel_monitor_update_ids`
insertion as we can be confident we'll never have a
`ChannelMonitorUpdate` for this channel at all.

diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index 28b75219849a88385a54ca0162dd480fc34ebdac..c596d276408befc18d75a129c4028f362d3e5162 100644 (file)
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -1682,6 +1682,12 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
                self.inner.lock().unwrap().get_cur_holder_commitment_number()
        }
 
+       /// Gets whether we've been notified that this channel is closed by the `ChannelManager` (i.e.
+       /// via a [`ChannelMonitorUpdateStep::ChannelForceClosed`]).
+       pub(crate) fn offchain_closed(&self) -> bool {
+               self.inner.lock().unwrap().lockdown_from_offchain
+       }
+
        /// Gets the `node_id` of the counterparty for this channel.
        ///
        /// Will be `None` for channels constructed on LDK versions prior to 0.0.110 and always `Some`

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index f9391cc9101180a08632759c46e86d411a15c412..0fd27525fed8b3a98eee480aaa8f4563003a044d 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -7123,8 +7123,6 @@ where
                                let prev_channel_id = hop_data.channel_id;
                                let prev_user_channel_id = hop_data.user_channel_id;
                                let completed_blocker = RAAMonitorUpdateBlockingAction::from_prev_hop_data(&hop_data);
-                               #[cfg(debug_assertions)]
-                               let claiming_chan_funding_outpoint = hop_data.outpoint;
                                self.claim_funds_from_hop(hop_data, payment_preimage,
                                        |htlc_claim_value_msat, definitely_duplicate| {
                                                let chan_to_release =
@@ -7149,61 +7147,6 @@ where
                                                        // monitor updates still in flight. In that case, we shouldn't
                                                        // immediately free, but instead let that monitor update complete
                                                        // in the background.
-                                                       #[cfg(debug_assertions)] {
-                                                               let background_events = self.pending_background_events.lock().unwrap();
-                                                               // There should be a `BackgroundEvent` pending...
-                                                               assert!(background_events.iter().any(|ev| {
-                                                                       match ev {
-                                                                               BackgroundEvent::MonitorUpdateRegeneratedOnStartup {
-                                                                                       funding_txo, update, ..
-                                                                               } => {
-                                                                                       if *funding_txo == claiming_chan_funding_outpoint {
-                                                                                               // to apply a monitor update that blocked the claiming channel,
-                                                                                               assert!(update.updates.iter().any(|upd|
-                                                                                                       if let ChannelMonitorUpdateStep::PaymentPreimage {
-                                                                                                               payment_preimage: update_preimage
-                                                                                                       } = upd {
-                                                                                                               payment_preimage == *update_preimage
-                                                                                                       } else {
-                                                                                                               false
-                                                                                                       }
-                                                                                               ), "{:?}", update);
-                                                                                               true
-                                                                                       } else if *funding_txo == next_channel_outpoint {
-                                                                                               // or the channel we'd unblock is already closed,
-                                                                                               assert!(update.updates.iter().any(|upd|
-                                                                                                       if let ChannelMonitorUpdateStep::ChannelForceClosed { .. } = upd {
-                                                                                                               true
-                                                                                                       } else {
-                                                                                                               false
-                                                                                                       }
-                                                                                               ), "{:?}", update);
-                                                                                               true
-                                                                                       } else { false }
-                                                                               },
-                                                                               // or the channel we'd unblock is already closed (for an
-                                                                               // old channel),
-                                                                               BackgroundEvent::ClosedMonitorUpdateRegeneratedOnStartup(
-                                                                                       (funding_txo, _channel_id, monitor_update)
-                                                                               ) => {
-                                                                                       if *funding_txo == next_channel_outpoint {
-                                                                                               assert_eq!(monitor_update.updates.len(), 1);
-                                                                                               assert!(matches!(
-                                                                                                       monitor_update.updates[0],
-                                                                                                       ChannelMonitorUpdateStep::ChannelForceClosed { .. }
-                                                                                               ));
-                                                                                               true
-                                                                                       } else { false }
-                                                                               },
-                                                                               // or the monitor update has completed and will unblock
-                                                                               // immediately once we get going.
-                                                                               BackgroundEvent::MonitorUpdatesComplete {
-                                                                                       channel_id, ..
-                                                                               } =>
-                                                                                       *channel_id == prev_channel_id,
-                                                                       }
-                                                               }), "{:?}", *background_events);
-                                                       }
                                                        (None, None)
                                                } else if definitely_duplicate {
                                                        if let Some(other_chan) = chan_to_release {
@@ -12483,6 +12426,10 @@ where
                }
 
                for (funding_txo, monitor) in args.channel_monitors.iter() {
+                       if monitor.offchain_closed() {
+                               // We already appled a ChannelForceClosed update.
+                               continue;
+                       }
                        if !funding_txo_set.contains(funding_txo) {
                                let logger = WithChannelMonitor::from(&args.logger, monitor, None);
                                let channel_id = monitor.channel_id();

diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
index 947ec4a1304294c1a816930171e6273a6dbadbf4..11bfe7867ee25b52fe63e7037813d9681faacb88 100644 (file)
--- a/lightning/src/ln/monitor_tests.rs
+++ b/lightning/src/ln/monitor_tests.rs
@@ -2300,9 +2300,6 @@ fn do_test_restored_packages_retry(check_old_monitor_retries_after_upgrade: bool
 
        // Connecting more blocks should result in the HTLC transactions being rebroadcast.
        connect_blocks(&nodes[0], 6);
-       if check_old_monitor_retries_after_upgrade {
-               check_added_monitors(&nodes[0], 1);
-       }
        {
                let txn = nodes[0].tx_broadcaster.txn_broadcast();
                if !nodes[0].connect_style.borrow().skips_blocks() {
@@ -3023,7 +3020,6 @@ fn do_test_anchors_monitor_fixes_counterparty_payment_script_on_reload(confirm_c
                // If we saw the commitment before our `counterparty_payment_script` was fixed, we'll never
                // get the spendable output event for the `to_remote` output, so we'll need to get it
                // manually via `get_spendable_outputs`.
-               check_added_monitors(&nodes[1], 1);
                let outputs = get_monitor!(nodes[1], chan_id).get_spendable_outputs(&commitment_tx, commitment_tx_conf_height);
                assert_eq!(outputs.len(), 1);
                let spend_tx = nodes[1].keys_manager.backing.spend_spendable_outputs(

diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index eac2a35c6a1bc166bd37d0f7bf99ffd311edec92..6c5193587509d80ef6459f1b7702ccd682ae1869 100644 (file)
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -992,7 +992,6 @@ fn do_test_completed_payment_not_retryable_on_reload(use_dust: bool) {
        nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id());
 
        nodes[0].node.test_process_background_events();
-       check_added_monitors(&nodes[0], 1);
 
        let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
        reconnect_args.send_channel_ready = (true, true);
@@ -1022,7 +1021,6 @@ fn do_test_completed_payment_not_retryable_on_reload(use_dust: bool) {
        nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id());
 
        nodes[0].node.test_process_background_events();
-       check_added_monitors(&nodes[0], 1);
 
        reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
 
@@ -1161,7 +1159,6 @@ fn do_test_dup_htlc_onchain_doesnt_fail_on_reload(persist_manager_post_event: bo
        let height = nodes[0].blocks.lock().unwrap().len() as u32 - 1;
        nodes[0].chain_monitor.chain_monitor.block_connected(&claim_block, height);
        assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
-       check_added_monitors(&nodes[0], 1);
 }
 
 #[test]
@@ -3521,7 +3518,6 @@ fn do_no_missing_sent_on_reload(persist_manager_with_payment: bool, at_midpoint:
        reload_node!(nodes[0], test_default_channel_config(), &nodes[0].node.encode(), &[&chan_0_monitor_serialized], persister_c, chain_monitor_c, nodes_0_deserialized_c);
        let events = nodes[0].node.get_and_clear_pending_events();
        assert!(events.is_empty());
-       check_added_monitors(&nodes[0], 1);
 }
 
 #[test]