Move pre-funded-channel immediate shutdown logic to the right place

Because a `Funded` `Channel` cannot possibly be pre-funding, the
logic in `ChannelManager::close_channel_internal` to handle
pre-funding channels is in the wrong place.

Rather than being handled inside the `Funded` branch, it should be
in an `else` following it, handling either of the two
`ChannelPhases` outside of `Funded`.

Sadly, because of a previous control flow management `loop {}`, the
existing code will infinite loop, which is fixed here.

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index b1afeb1e9fcea42f3c9b08b1d1ec5f293ee96a5a..a74dbbfe4ceee417cd3d1555a0634b5e2f9bccdb 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -2714,9 +2714,10 @@ where
        fn close_channel_internal(&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, target_feerate_sats_per_1000_weight: Option<u32>, override_shutdown_script: Option<ShutdownScript>) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 
-               let mut failed_htlcs: Vec<(HTLCSource, PaymentHash)>;
-               let shutdown_result;
-               loop {
+               let mut failed_htlcs: Vec<(HTLCSource, PaymentHash)> = Vec::new();
+               let mut shutdown_result = None;
+
+               {
                        let per_peer_state = self.per_peer_state.read().unwrap();
 
                        let peer_state_mutex = per_peer_state.get(counterparty_node_id)
@@ -2733,8 +2734,6 @@ where
                                                let (shutdown_msg, mut monitor_update_opt, htlcs) =
                                                        chan.get_shutdown(&self.signer_provider, their_features, target_feerate_sats_per_1000_weight, override_shutdown_script)?;
                                                failed_htlcs = htlcs;
-                                               shutdown_result = None;
-                                               debug_assert_eq!(shutdown_result.is_some(), chan.is_shutdown());
 
                                                // We can send the `shutdown` message before updating the `ChannelMonitor`
                                                // here as we don't need the monitor update to complete until we send a
@@ -2751,20 +2750,11 @@ where
                                                if let Some(monitor_update) = monitor_update_opt.take() {
                                                        handle_new_monitor_update!(self, funding_txo_opt.unwrap(), monitor_update,
                                                                peer_state_lock, peer_state, per_peer_state, chan);
-                                                       break;
                                                }
-
-                                               if chan.is_shutdown() {
-                                                       if let ChannelPhase::Funded(chan) = remove_channel_phase!(self, chan_phase_entry) {
-                                                               if let Ok(channel_update) = self.get_channel_update_for_broadcast(&chan) {
-                                                                       peer_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
-                                                                               msg: channel_update
-                                                                       });
-                                                               }
-                                                               self.issue_channel_close_events(&chan.context, ClosureReason::HolderForceClosed);
-                                                       }
-                                               }
-                                               break;
+                                       } else {
+                                               self.issue_channel_close_events(chan_phase_entry.get().context(), ClosureReason::HolderForceClosed);
+                                               let mut chan_phase = remove_channel_phase!(self, chan_phase_entry);
+                                               shutdown_result = Some(chan_phase.context_mut().force_shutdown(false));
                                        }
                                },
                                hash_map::Entry::Vacant(_) => {

diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs
index 308211d0b370285e509c6885a60de084ba214d06..bdd32e90d0ace1cca8b1fd46c4eac99ecb486b61 100644 (file)
--- a/lightning/src/ln/shutdown_tests.rs
+++ b/lightning/src/ln/shutdown_tests.rs
@@ -277,6 +277,21 @@ fn shutdown_on_unfunded_channel() {
        check_closed_event!(nodes[0], 1, ClosureReason::CounterpartyCoopClosedUnfundedChannel, [nodes[1].node.get_our_node_id()], 1_000_000);
 }
 
+#[test]
+fn close_on_unfunded_channel() {
+       // Test the user asking us to close prior to funding generation
+       let chanmon_cfgs = create_chanmon_cfgs(2);
+       let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+       let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+       let chan_id = nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 1_000_000, 100_000, 0, None, None).unwrap();
+       let _open_chan = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
+
+       nodes[0].node.close_channel(&chan_id, &nodes[1].node.get_our_node_id()).unwrap();
+       check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 1_000_000);
+}
+
 #[test]
 fn expect_channel_shutdown_state_with_force_closure() {
        // Test sending a shutdown prior to channel_ready after funding generation