Skip to content

Commit 9f9d448

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 1feb713 commit 9f9d448

File tree

1 file changed

+16
-1
lines changed

1 file changed

+16
-1
lines changed

lightning/src/ln/channelmanager.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2888,6 +2888,13 @@ macro_rules! handle_error {
28882888
} };
28892889
}
28902890

2891+
/// When a channel is removed, two things need to happen:
2892+
/// (a) This must be called in the same `per_peer_state` lock as the channel-closing action,
2893+
/// (b) [`ChannelManager::finish_close_channel`] needs to be called without holding any locks
2894+
/// (except [`ChannelManager::total_consistency_lock`].
2895+
///
2896+
/// Note that this step can be skipped if the channel was never opened (through the creation of a
2897+
/// [`ChannelMonitor`]/channel funding transaction) to begin with.
28912898
macro_rules! update_maps_on_chan_removal {
28922899
($self: expr, $peer_state: expr, $channel_context: expr) => {{
28932900
if let Some(outpoint) = $channel_context.get_funding_txo() {
@@ -3762,6 +3769,11 @@ where
37623769
self.close_channel_internal(channel_id, counterparty_node_id, target_feerate_sats_per_1000_weight, shutdown_script)
37633770
}
37643771

3772+
/// When a channel is removed, two things need to happen:
3773+
/// (a) [`update_maps_on_chan_removal`] must be called in the same `per_peer_state` lock as
3774+
/// the channel-closing action,
3775+
/// (b) this needs to be called without holding any locks (except
3776+
/// [`ChannelManager::total_consistency_lock`].
37653777
fn finish_close_channel(&self, mut shutdown_res: ShutdownResult) {
37663778
debug_assert_ne!(self.per_peer_state.held_by_thread(), LockHeldState::HeldByThread);
37673779
#[cfg(debug_assertions)]
@@ -9124,7 +9136,10 @@ where
91249136
_ => unblock_chan(chan, &mut peer_state.pending_msg_events),
91259137
};
91269138
if let Some(shutdown_result) = shutdown_result {
9127-
log_trace!(self.logger, "Removing channel after unblocking signer");
9139+
let context = &chan.context();
9140+
let logger = WithChannelContext::from(&self.logger, context, None);
9141+
log_trace!(logger, "Removing channel {} now that the signer is unblocked", context.channel_id());
9142+
update_maps_on_chan_removal!(self, peer_state, context);
91289143
shutdown_results.push(shutdown_result);
91299144
false
91309145
} else {

0 commit comments

Comments
 (0)