Correct manual shutdown detection on channel closure

In 5e874c3dc9cf1606a3cbbccab3a0d25089a97280 we'd intended to not
reveal the dummy funding transaction in `Event::DiscardFunding`.
However, instead of looking at the channel that was just closed,
the logic only looks at any other channels which were funded as a
part of the same batch. Because manually-funded transactions
cannot currently be done for batch funding, this was actually dead
code, preventing the new changes from taking effect.

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index fe01a26290c331e28bd6186408ab14919ab5078b..26e385082f9e352491f519c8f6585aa5acc83c62 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -936,6 +936,7 @@ pub(crate) struct ShutdownResult {
        pub(crate) user_channel_id: u128,
        pub(crate) channel_capacity_satoshis: u64,
        pub(crate) counterparty_node_id: PublicKey,
+       pub(crate) is_manual_broadcast: bool,
        pub(crate) unbroadcasted_funding_tx: Option<Transaction>,
        pub(crate) channel_funding_txo: Option<OutPoint>,
 }
@@ -3540,6 +3541,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                        channel_capacity_satoshis: self.channel_value_satoshis,
                        counterparty_node_id: self.counterparty_node_id,
                        unbroadcasted_funding_tx,
+                       is_manual_broadcast: self.is_manual_broadcast,
                        channel_funding_txo: self.get_funding_txo(),
                }
        }
@@ -6243,6 +6245,7 @@ impl<SP: Deref> Channel<SP> where
                                        channel_capacity_satoshis: self.context.channel_value_satoshis,
                                        counterparty_node_id: self.context.counterparty_node_id,
                                        unbroadcasted_funding_tx: self.context.unbroadcasted_funding(),
+                                       is_manual_broadcast: self.context.is_manual_broadcast,
                                        channel_funding_txo: self.context.get_funding_txo(),
                                };
                                let tx = self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig);
@@ -6275,6 +6278,7 @@ impl<SP: Deref> Channel<SP> where
                                                channel_capacity_satoshis: self.context.channel_value_satoshis,
                                                counterparty_node_id: self.context.counterparty_node_id,
                                                unbroadcasted_funding_tx: self.context.unbroadcasted_funding(),
+                                               is_manual_broadcast: self.context.is_manual_broadcast,
                                                channel_funding_txo: self.context.get_funding_txo(),
                                        };
                                        if closing_signed.is_some() {

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 258bc066125fc15505423f78de164dacfd52e753..0e377b348db34117d9e58e7b34f1d4be641904c3 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -3510,7 +3510,6 @@ where
                        let _ = self.chain_monitor.update_channel(funding_txo, &monitor_update);
                }
                let mut shutdown_results = Vec::new();
-               let mut is_manual_broadcast = false;
                if let Some(txid) = shutdown_res.unbroadcasted_batch_funding_txid {
                        let mut funding_batch_states = self.funding_batch_states.lock().unwrap();
                        let affected_channels = funding_batch_states.remove(&txid).into_iter().flatten();
@@ -3520,10 +3519,6 @@ where
                                if let Some(peer_state_mutex) = per_peer_state.get(&counterparty_node_id) {
                                        let mut peer_state = peer_state_mutex.lock().unwrap();
                                        if let Some(mut chan) = peer_state.channel_by_id.remove(&channel_id) {
-                                               // We override the previous value, so we could change from true -> false,
-                                               // but this is fine because if a channel has manual_broadcast set to false
-                                               // we should choose the safier condition.
-                                               is_manual_broadcast = chan.context().is_manual_broadcast();
                                                update_maps_on_chan_removal!(self, &chan.context());
                                                shutdown_results.push(chan.context_mut().force_shutdown(false, ClosureReason::FundingBatchClosure));
                                        }
@@ -3548,7 +3543,7 @@ where
                        }, None));
 
                        if let Some(transaction) = shutdown_res.unbroadcasted_funding_tx {
-                               let funding_info = if is_manual_broadcast {
+                               let funding_info = if shutdown_res.is_manual_broadcast {
                                        FundingInfo::OutPoint {
                                                outpoint: shutdown_res.channel_funding_txo
                                                        .expect("We had an unbroadcasted funding tx, so should also have had a funding outpoint"),