From 73b20e23f561dcd0904d89954d55665299db99c1 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 29 Nov 2023 05:59:42 +0000 Subject: [PATCH] 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. --- lightning/src/ln/channelmanager.rs | 25 +++++++++---------------- lightning/src/ln/shutdown_tests.rs | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c4efd895796..4ed35f33155 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2603,7 +2603,8 @@ where let mut failed_htlcs: Vec<(HTLCSource, PaymentHash)>; let mut shutdown_result = None; - loop { + + { let per_peer_state = self.per_peer_state.read().unwrap(); let peer_state_mutex = per_peer_state.get(counterparty_node_id) @@ -2614,10 +2615,11 @@ where match peer_state.channel_by_id.entry(channel_id.clone()) { hash_map::Entry::Occupied(mut chan_phase_entry) => { + let unbroadcasted_batch_funding_txid = + chan_phase_entry.get().context().unbroadcasted_batch_funding_txid(); if let ChannelPhase::Funded(chan) = chan_phase_entry.get_mut() { let funding_txo_opt = chan.context.get_funding_txo(); let their_features = &peer_state.latest_features; - let unbroadcasted_batch_funding_txid = chan.context.unbroadcasted_batch_funding_txid(); 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; @@ -2637,21 +2639,12 @@ 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); - shutdown_result = Some((None, Vec::new(), unbroadcasted_batch_funding_txid)); - } } - break; + } else { + self.issue_channel_close_events(chan_phase_entry.get().context(), ClosureReason::HolderForceClosed); + failed_htlcs = Vec::new(); + remove_channel_phase!(self, chan_phase_entry); + shutdown_result = Some((None, Vec::new(), unbroadcasted_batch_funding_txid)); } }, hash_map::Entry::Vacant(_) => { diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index 0d02e89b8bb..71e7c806017 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -275,6 +275,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).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