Skip to content

Commit 885d4c9

Browse files
committed
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.
1 parent cf9baf7 commit 885d4c9

File tree

1 file changed

+14
-1
lines changed

1 file changed

+14
-1
lines changed

lightning/src/ln/channelmanager.rs

+14-1
Original file line numberDiff line numberDiff line change
@@ -2753,6 +2753,12 @@ macro_rules! handle_error {
27532753
} };
27542754
}
27552755

2756+
/// When a channel is removed, two things need to happen:
2757+
/// (a) This must be called in the same `per_peer_state` lock as the channel-closing action,
2758+
/// (b) [`ChannelManager::finish_close_channel`] needs to be called unlocked.
2759+
///
2760+
/// Note that this step can be skipped if the channel was never opened (through the creation of a
2761+
/// [`ChannelMonitor`]/channel funding transaction) to begin with.
27562762
macro_rules! update_maps_on_chan_removal {
27572763
($self: expr, $peer_state: expr, $channel_context: expr) => {{
27582764
if let Some(outpoint) = $channel_context.get_funding_txo() {
@@ -3627,6 +3633,10 @@ where
36273633
self.close_channel_internal(channel_id, counterparty_node_id, target_feerate_sats_per_1000_weight, shutdown_script)
36283634
}
36293635

3636+
/// When a channel is removed, two things need to happen:
3637+
/// (a) [`update_maps_on_chan_removal`] must be called in the same `per_peer_state` lock as
3638+
/// the channel-closing action,
3639+
/// (b) this needs to be called unlocked.
36303640
fn finish_close_channel(&self, mut shutdown_res: ShutdownResult) {
36313641
debug_assert_ne!(self.per_peer_state.held_by_thread(), LockHeldState::HeldByThread);
36323642
#[cfg(debug_assertions)]
@@ -8972,7 +8982,10 @@ where
89728982
_ => unblock_chan(chan, &mut peer_state.pending_msg_events),
89738983
};
89748984
if let Some(shutdown_result) = shutdown_result {
8975-
log_trace!(self.logger, "Removing channel after unblocking signer");
8985+
let context = &chan.context();
8986+
let logger = WithChannelContext::from(&self.logger, context, None);
8987+
log_trace!(logger, "Removing channel {} now that signer is unblocked", context.channel_id());
8988+
update_maps_on_chan_removal!(self, peer_state, context);
89768989
shutdown_results.push(shutdown_result);
89778990
false
89788991
} else {

0 commit comments

Comments
 (0)