Handle `short_to_id` state updates on channel closure via macros
authorMatt Corallo <git@bluematt.me>
Tue, 8 Feb 2022 21:43:14 +0000 (21:43 +0000)
committerMatt Corallo <git@bluematt.me>
Wed, 9 Mar 2022 19:14:38 +0000 (19:14 +0000)
This avoids needing to update channel closure code in many places
as we add multiple SCIDs for each channel and have to track them.

lightning/src/ln/channelmanager.rs

index 46bad1f56566cb0eb802fc60fe88592d66b5a3d8..94d78e798ca9ecfe3e9df4945e84ad609eb75ebf 100644 (file)
@@ -1405,6 +1405,14 @@ macro_rules! handle_error {
        }
 }
 
+macro_rules! update_maps_on_chan_removal {
+       ($short_to_id: expr, $channel: expr) => {
+               if let Some(short_id) = $channel.get_short_channel_id() {
+                       $short_to_id.remove(&short_id);
+               }
+       }
+}
+
 /// Returns (boolean indicating if we should remove the Channel object from memory, a mapped error)
 macro_rules! convert_chan_err {
        ($self: ident, $err: expr, $short_to_id: expr, $channel: expr, $channel_id: expr) => {
@@ -1417,18 +1425,14 @@ macro_rules! convert_chan_err {
                        },
                        ChannelError::Close(msg) => {
                                log_error!($self.logger, "Closing channel {} due to close-required error: {}", log_bytes!($channel_id[..]), msg);
-                               if let Some(short_id) = $channel.get_short_channel_id() {
-                                       $short_to_id.remove(&short_id);
-                               }
+                               update_maps_on_chan_removal!($short_to_id, $channel);
                                let shutdown_res = $channel.force_shutdown(true);
                                (true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, $channel.get_user_id(),
                                        shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
                        },
                        ChannelError::CloseDelayBroadcast(msg) => {
                                log_error!($self.logger, "Channel {} need to be shutdown but closing transactions not broadcast due to {}", log_bytes!($channel_id[..]), msg);
-                               if let Some(short_id) = $channel.get_short_channel_id() {
-                                       $short_to_id.remove(&short_id);
-                               }
+                               update_maps_on_chan_removal!($short_to_id, $channel);
                                let shutdown_res = $channel.force_shutdown(false);
                                (true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, $channel.get_user_id(),
                                        shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok()))
@@ -1471,9 +1475,7 @@ macro_rules! remove_channel {
        ($channel_state: expr, $entry: expr) => {
                {
                        let channel = $entry.remove_entry().1;
-                       if let Some(short_id) = channel.get_short_channel_id() {
-                               $channel_state.short_to_id.remove(&short_id);
-                       }
+                       update_maps_on_chan_removal!($channel_state.short_to_id, channel);
                        channel
                }
        }
@@ -1484,9 +1486,7 @@ macro_rules! handle_monitor_err {
                match $err {
                        ChannelMonitorUpdateErr::PermanentFailure => {
                                log_error!($self.logger, "Closing channel {} due to monitor update ChannelMonitorUpdateErr::PermanentFailure", log_bytes!($chan_id[..]));
-                               if let Some(short_id) = $chan.get_short_channel_id() {
-                                       $short_to_id.remove(&short_id);
-                               }
+                               update_maps_on_chan_removal!($short_to_id, $chan);
                                // TODO: $failed_fails is dropped here, which will cause other channels to hit the
                                // chain in a confused state! We need to move them into the ChannelMonitor which
                                // will be responsible for failing backwards once things confirm on-chain.
@@ -2049,9 +2049,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                return Err(APIError::ChannelUnavailable{err: "No such channel".to_owned()});
                                        }
                                }
-                               if let Some(short_id) = chan.get().get_short_channel_id() {
-                                       channel_state.short_to_id.remove(&short_id);
-                               }
                                if peer_node_id.is_some() {
                                        if let Some(peer_msg) = peer_msg {
                                                self.issue_channel_close_events(chan.get(),ClosureReason::CounterpartyForceClosed { peer_msg: peer_msg.to_string() });
@@ -2059,7 +2056,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                } else {
                                        self.issue_channel_close_events(chan.get(),ClosureReason::HolderForceClosed);
                                }
-                               chan.remove_entry().1
+                               remove_channel!(channel_state, chan)
                        } else {
                                return Err(APIError::ChannelUnavailable{err: "No such channel".to_owned()});
                        }
@@ -3206,12 +3203,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                                                }
                                                                                ChannelError::Close(msg) => {
                                                                                        log_trace!(self.logger, "Closing channel {} due to Close-required error: {}", log_bytes!(chan.key()[..]), msg);
-                                                                                       let (channel_id, mut channel) = chan.remove_entry();
-                                                                                       if let Some(short_id) = channel.get_short_channel_id() {
-                                                                                               channel_state.short_to_id.remove(&short_id);
-                                                                                       }
+                                                                                       let mut channel = remove_channel!(channel_state, chan);
                                                                                        // ChannelClosed event is generated by handle_error for us.
-                                                                                       Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, channel.get_user_id(), channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
+                                                                                       Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel.channel_id(), channel.get_user_id(), channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok()))
                                                                                },
                                                                                ChannelError::CloseDelayBroadcast(_) => { panic!("Wait is only generated on receipt of channel_reestablish, which is handled by try_chan_entry, we don't bother to support it here"); }
                                                                        };
@@ -4479,10 +4473,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                // also implies there are no pending HTLCs left on the channel, so we can
                                                // fully delete it from tracking (the channel monitor is still around to
                                                // watch for old state broadcasts)!
-                                               if let Some(short_id) = chan_entry.get().get_short_channel_id() {
-                                                       channel_state.short_to_id.remove(&short_id);
-                                               }
-                                               (tx, Some(chan_entry.remove_entry().1))
+                                               (tx, Some(remove_channel!(channel_state, chan_entry)))
                                        } else { (tx, None) }
                                },
                                hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id))
@@ -4920,12 +4911,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        let mut channel_lock = self.channel_state.lock().unwrap();
                                        let channel_state = &mut *channel_lock;
                                        let by_id = &mut channel_state.by_id;
-                                       let short_to_id = &mut channel_state.short_to_id;
                                        let pending_msg_events = &mut channel_state.pending_msg_events;
-                                       if let Some(mut chan) = by_id.remove(&funding_outpoint.to_channel_id()) {
-                                               if let Some(short_id) = chan.get_short_channel_id() {
-                                                       short_to_id.remove(&short_id);
-                                               }
+                                       if let hash_map::Entry::Occupied(chan_entry) = by_id.entry(funding_outpoint.to_channel_id()) {
+                                               let mut chan = remove_channel!(channel_state, chan_entry);
                                                failed_channels.push(chan.force_shutdown(false));
                                                if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
                                                        pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
@@ -5054,10 +5042,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                if let Some(tx) = tx_opt {
                                                        // We're done with this channel. We got a closing_signed and sent back
                                                        // a closing_signed with a closing transaction to broadcast.
-                                                       if let Some(short_id) = chan.get_short_channel_id() {
-                                                               short_to_id.remove(&short_id);
-                                                       }
-
                                                        if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
                                                                pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
                                                                        msg: update
@@ -5068,6 +5052,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
                                                        log_info!(self.logger, "Broadcasting {}", log_tx!(tx));
                                                        self.tx_broadcaster.broadcast_transaction(&tx);
+                                                       update_maps_on_chan_removal!(short_to_id, chan);
                                                        false
                                                } else { true }
                                        },
@@ -5585,9 +5570,7 @@ where
                                                }
                                        }
                                } else if let Err(reason) = res {
-                                       if let Some(short_id) = channel.get_short_channel_id() {
-                                               short_to_id.remove(&short_id);
-                                       }
+                                       update_maps_on_chan_removal!(short_to_id, channel);
                                        // It looks like our counterparty went on-chain or funding transaction was
                                        // reorged out of the main chain. Close the channel.
                                        failed_channels.push(channel.force_shutdown(true));
@@ -5783,9 +5766,7 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
                                log_debug!(self.logger, "Failing all channels with {} due to no_connection_possible", log_pubkey!(counterparty_node_id));
                                channel_state.by_id.retain(|_, chan| {
                                        if chan.get_counterparty_node_id() == *counterparty_node_id {
-                                               if let Some(short_id) = chan.get_short_channel_id() {
-                                                       short_to_id.remove(&short_id);
-                                               }
+                                               update_maps_on_chan_removal!(short_to_id, chan);
                                                failed_channels.push(chan.force_shutdown(true));
                                                if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
                                                        pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
@@ -5804,9 +5785,7 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
                                        if chan.get_counterparty_node_id() == *counterparty_node_id {
                                                chan.remove_uncommitted_htlcs_and_mark_paused(&self.logger);
                                                if chan.is_shutdown() {
-                                                       if let Some(short_id) = chan.get_short_channel_id() {
-                                                               short_to_id.remove(&short_id);
-                                                       }
+                                                       update_maps_on_chan_removal!(short_to_id, chan);
                                                        self.issue_channel_close_events(chan, ClosureReason::DisconnectedPeer);
                                                        return false;
                                                } else {