From: Matt Corallo Date: Wed, 28 Aug 2024 14:35:54 +0000 (+0000) Subject: Correct manual shutdown detection on channel closure X-Git-Tag: v0.0.124-rc1~4^2~1 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=683aa8350ed545add81bbcbca6a9281ddeba3609;p=rust-lightning 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 fe01a2629..26e385082 100644 --- 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, pub(crate) channel_funding_txo: Option, } @@ -3540,6 +3541,7 @@ impl ChannelContext 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 Channel 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 Channel 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 258bc0661..0e377b348 100644 --- 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"),