Add missing `update_maps_on_chan_removal` call in signer restore

When a channel is closed, we have to call
`update_maps_on_chan_removal` in the same per-peer-state lock as
the removal of the `ChannelPhase` object. We forgot to do so in
`ChannelManager::signer_unblocked` leaving dangling references to
the channel.

We also take this opportunity to include more context in the
channel-closure log in `ChannelManager::signer_unblocked` and add
documentation to `update_maps_on_chan_removal` and
`finish_close_channel` to hopefully avoid this issue in the future.

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 3464a0077e8d7d2b12138b7eda833b6976445d21..dd5c201263ad266f22e45b5bb2265776aa38de8b 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -2753,6 +2753,12 @@ macro_rules! handle_error {
        } };
 }
 
+/// When a channel is removed, two things need to happen:
+/// (a) This must be called in the same `per_peer_state` lock as the channel-closing action,
+/// (b) [`ChannelManager::finish_close_channel`] needs to be called unlocked.
+///
+/// Note that this step can be skipped if the channel was never opened (through the creation of a
+/// [`ChannelMonitor`]/channel funding transaction) to begin with.
 macro_rules! update_maps_on_chan_removal {
        ($self: expr, $peer_state: expr, $channel_context: expr) => {{
                if let Some(outpoint) = $channel_context.get_funding_txo() {
@@ -3627,6 +3633,10 @@ where
                self.close_channel_internal(channel_id, counterparty_node_id, target_feerate_sats_per_1000_weight, shutdown_script)
        }
 
+       /// When a channel is removed, two things need to happen:
+       /// (a) [`update_maps_on_chan_removal`] must be called in the same `per_peer_state` lock as
+       ///     the channel-closing action,
+       /// (b) this needs to be called unlocked.
        fn finish_close_channel(&self, mut shutdown_res: ShutdownResult) {
                debug_assert_ne!(self.per_peer_state.held_by_thread(), LockHeldState::HeldByThread);
                #[cfg(debug_assertions)]
@@ -8972,7 +8982,10 @@ where
                                        _ => unblock_chan(chan, &mut peer_state.pending_msg_events),
                                };
                                if let Some(shutdown_result) = shutdown_result {
-                                       log_trace!(self.logger, "Removing channel after unblocking signer");
+                                       let context = &chan.context();
+                                       let logger = WithChannelContext::from(&self.logger, context, None);
+                                       log_trace!(logger, "Removing channel {} now that signer is unblocked", context.channel_id());
+                                       update_maps_on_chan_removal!(self, peer_state, context);
                                        shutdown_results.push(shutdown_result);
                                        false
                                } else {