From ce9ebccb73ae7b252ba4c3e8cc87ddfdbedd7804 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 10 Apr 2025 13:28:04 -0500 Subject: [PATCH 1/8] Generalize do_chain_event signature for splicing When processing confirmed transactions and updates to the best block, ChannelManager may be instructed to send a channel_ready message when a channel's funding transaction has confirmed and has met the required number of confirmations. A similar action is needed for sending splice_locked once splice transaction has confirmed with required number of confirmations. Generalize do_chain_event signature to allow for either scenario. --- lightning/src/ln/channel.rs | 12 +++++----- lightning/src/ln/channelmanager.rs | 35 ++++++++++++++++++------------ 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index d5cccc0bc1f..2531878df1f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -38,7 +38,7 @@ use crate::ln::msgs; use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError, OnionErrorPacket}; use crate::ln::script::{self, ShutdownScript}; use crate::ln::channel_state::{ChannelShutdownState, CounterpartyForwardingInfo, InboundHTLCDetails, InboundHTLCStateDetails, OutboundHTLCDetails, OutboundHTLCStateDetails}; -use crate::ln::channelmanager::{self, OpenChannelMessage, PendingHTLCStatus, HTLCSource, SentHTLCId, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentClaimDetails, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT}; +use crate::ln::channelmanager::{self, FundingConfirmedMessage, OpenChannelMessage, PendingHTLCStatus, HTLCSource, SentHTLCId, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentClaimDetails, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT}; use crate::ln::chan_utils::{ CounterpartyCommitmentSecrets, HTLCOutputInCommitment, htlc_success_tx_weight, htlc_timeout_tx_weight, ChannelPublicKeys, CommitmentTransaction, @@ -8156,7 +8156,7 @@ impl FundedChannel where pub fn transactions_confirmed( &mut self, block_hash: &BlockHash, height: u32, txdata: &TransactionData, chain_hash: ChainHash, node_signer: &NS, user_config: &UserConfig, logger: &L - ) -> Result<(Option, Option), ClosureReason> + ) -> Result<(Option, Option), ClosureReason> where NS::Target: NodeSigner, L::Target: Logger @@ -8217,7 +8217,7 @@ impl FundedChannel where if let Some(channel_ready) = self.check_get_channel_ready(height, logger) { log_info!(logger, "Sending a channel_ready to our peer for channel {}", &self.context.channel_id); let announcement_sigs = self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger); - msgs = (Some(channel_ready), announcement_sigs); + msgs = (Some(FundingConfirmedMessage::Establishment(channel_ready)), announcement_sigs); } } for inp in tx.input.iter() { @@ -8245,7 +8245,7 @@ impl FundedChannel where pub fn best_block_updated( &mut self, height: u32, highest_header_time: u32, chain_hash: ChainHash, node_signer: &NS, user_config: &UserConfig, logger: &L - ) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>, Option), ClosureReason> + ) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>, Option), ClosureReason> where NS::Target: NodeSigner, L::Target: Logger @@ -8256,7 +8256,7 @@ impl FundedChannel where fn do_best_block_updated( &mut self, height: u32, highest_header_time: u32, chain_node_signer: Option<(ChainHash, &NS, &UserConfig)>, logger: &L - ) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>, Option), ClosureReason> + ) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>, Option), ClosureReason> where NS::Target: NodeSigner, L::Target: Logger @@ -8285,7 +8285,7 @@ impl FundedChannel where self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger) } else { None }; log_info!(logger, "Sending a channel_ready to our peer for channel {}", &self.context.channel_id); - return Ok((Some(channel_ready), timed_out_htlcs, announcement_sigs)); + return Ok((Some(FundingConfirmedMessage::Establishment(channel_ready)), timed_out_htlcs, announcement_sigs)); } if matches!(self.context.channel_state, ChannelState::ChannelReady(_)) || diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d0a73e89992..40e9537dee4 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -11662,6 +11662,10 @@ where } } +pub(super) enum FundingConfirmedMessage { + Establishment(msgs::ChannelReady), +} + impl ChannelManager where M::Target: chain::Watch<::EcdsaSigner>, @@ -11677,7 +11681,7 @@ where /// Calls a function which handles an on-chain event (blocks dis/connected, transactions /// un/confirmed, etc) on each channel, handling any resulting errors or messages generated by /// the function. - fn do_chain_event) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>, Option), ClosureReason>> + fn do_chain_event) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>, Option), ClosureReason>> (&self, height_opt: Option, f: FN) { // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called // during initialization prior to the chain_monitor being fully configured in some cases. @@ -11698,7 +11702,7 @@ where None => true, Some(funded_channel) => { let res = f(funded_channel); - if let Ok((channel_ready_opt, mut timed_out_pending_htlcs, announcement_sigs)) = res { + if let Ok((funding_confirmed_opt, mut timed_out_pending_htlcs, announcement_sigs)) = res { for (source, payment_hash) in timed_out_pending_htlcs.drain(..) { let failure_code = 0x1000|14; /* expiry_too_soon */ let data = self.get_htlc_inbound_temp_fail_data(failure_code); @@ -11706,19 +11710,22 @@ where HTLCDestination::NextHopChannel { node_id: Some(funded_channel.context.get_counterparty_node_id()), channel_id: funded_channel.context.channel_id() })); } let logger = WithChannelContext::from(&self.logger, &funded_channel.context, None); - if let Some(channel_ready) = channel_ready_opt { - send_channel_ready!(self, pending_msg_events, funded_channel, channel_ready); - if funded_channel.context.is_usable() { - log_trace!(logger, "Sending channel_ready with private initial channel_update for our counterparty on channel {}", funded_channel.context.channel_id()); - if let Ok(msg) = self.get_channel_update_for_unicast(funded_channel) { - pending_msg_events.push(MessageSendEvent::SendChannelUpdate { - node_id: funded_channel.context.get_counterparty_node_id(), - msg, - }); + match funding_confirmed_opt { + Some(FundingConfirmedMessage::Establishment(channel_ready)) => { + send_channel_ready!(self, pending_msg_events, funded_channel, channel_ready); + if funded_channel.context.is_usable() { + log_trace!(logger, "Sending channel_ready with private initial channel_update for our counterparty on channel {}", funded_channel.context.channel_id()); + if let Ok(msg) = self.get_channel_update_for_unicast(funded_channel) { + pending_msg_events.push(MessageSendEvent::SendChannelUpdate { + node_id: funded_channel.context.get_counterparty_node_id(), + msg, + }); + } + } else { + log_trace!(logger, "Sending channel_ready WITHOUT channel_update for {}", funded_channel.context.channel_id()); } - } else { - log_trace!(logger, "Sending channel_ready WITHOUT channel_update for {}", funded_channel.context.channel_id()); - } + }, + None => {}, } { From 2e85dde2a71b47e0d59b379e5f1d062752ee1662 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 15 Apr 2025 15:26:37 -0500 Subject: [PATCH 2/8] Move ChannelContext::funding_tx_confirmed_in to FundingScope When processing confirmed transactions, if the funding transaction is found then information about it in the ChannelContext is updated. In preparation for splicing, move this data to FundingScope. --- lightning/src/ln/channel.rs | 32 +++++++++++++++++------------- lightning/src/ln/channelmanager.rs | 2 +- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 2531878df1f..95e36d73df9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1685,6 +1685,8 @@ pub(super) struct FundingScope { /// The transaction which funds this channel. Note that for manually-funded channels (i.e., /// [`ChannelContext::is_manual_broadcast`] is true) this will be a dummy empty transaction. funding_transaction: Option, + /// The hash of the block in which the funding transaction was included. + funding_tx_confirmed_in: Option, } impl Writeable for FundingScope { @@ -1695,6 +1697,7 @@ impl Writeable for FundingScope { (5, self.holder_selected_channel_reserve_satoshis, required), (7, self.channel_transaction_parameters, (required: ReadableArgs, None)), (9, self.funding_transaction, option), + (11, self.funding_tx_confirmed_in, option), }); Ok(()) } @@ -1707,6 +1710,7 @@ impl Readable for FundingScope { let mut holder_selected_channel_reserve_satoshis = RequiredWrapper(None); let mut channel_transaction_parameters = RequiredWrapper(None); let mut funding_transaction = None; + let mut funding_tx_confirmed_in = None; read_tlv_fields!(reader, { (1, value_to_self_msat, required), @@ -1714,6 +1718,7 @@ impl Readable for FundingScope { (5, holder_selected_channel_reserve_satoshis, required), (7, channel_transaction_parameters, (required: ReadableArgs, None)), (9, funding_transaction, option), + (11, funding_tx_confirmed_in, option), }); Ok(Self { @@ -1726,6 +1731,7 @@ impl Readable for FundingScope { counterparty_max_commitment_tx_output: Mutex::new((0, 0)), channel_transaction_parameters: channel_transaction_parameters.0.unwrap(), funding_transaction, + funding_tx_confirmed_in, #[cfg(any(test, fuzzing))] next_local_commitment_tx_fee_info_cached: Mutex::new(None), #[cfg(any(test, fuzzing))] @@ -1799,6 +1805,11 @@ impl FundingScope { pub fn get_channel_type(&self) -> &ChannelTypeFeatures { &self.channel_transaction_parameters.channel_type_features } + + /// Returns the block hash in which our funding transaction was confirmed. + pub fn get_funding_tx_confirmed_in(&self) -> Option { + self.funding_tx_confirmed_in + } } /// Info about a pending splice, used in the pre-splice channel @@ -1958,8 +1969,6 @@ pub(super) struct ChannelContext where SP::Target: SignerProvider { /// milliseconds, so any accidental force-closes here should be exceedingly rare. expecting_peer_commitment_signed: bool, - /// The hash of the block in which the funding transaction was included. - funding_tx_confirmed_in: Option, funding_tx_confirmation_height: u32, short_channel_id: Option, /// Either the height at which this channel was created or the height at which it was last @@ -2770,6 +2779,7 @@ impl ChannelContext where SP::Target: SignerProvider { channel_value_satoshis, }, funding_transaction: None, + funding_tx_confirmed_in: None, }; let channel_context = ChannelContext { user_id, @@ -2833,7 +2843,6 @@ impl ChannelContext where SP::Target: SignerProvider { closing_fee_limits: None, target_closing_feerate_sats_per_kw: None, - funding_tx_confirmed_in: None, funding_tx_confirmation_height: 0, short_channel_id: None, channel_creation_height: current_chain_height, @@ -3006,6 +3015,7 @@ impl ChannelContext where SP::Target: SignerProvider { channel_value_satoshis, }, funding_transaction: None, + funding_tx_confirmed_in: None, }; let channel_context = Self { user_id, @@ -3067,7 +3077,6 @@ impl ChannelContext where SP::Target: SignerProvider { closing_fee_limits: None, target_closing_feerate_sats_per_kw: None, - funding_tx_confirmed_in: None, funding_tx_confirmation_height: 0, short_channel_id: None, channel_creation_height: current_chain_height, @@ -3460,11 +3469,6 @@ impl ChannelContext where SP::Target: SignerProvider { Ok(()) } - /// Returns the block hash in which our funding transaction was confirmed. - pub fn get_funding_tx_confirmed_in(&self) -> Option { - self.funding_tx_confirmed_in - } - /// Returns the current number of confirmations on the funding transaction. pub fn get_funding_tx_confirmations(&self, height: u32) -> u32 { if self.funding_tx_confirmation_height == 0 { @@ -8197,7 +8201,7 @@ impl FundedChannel where } } self.context.funding_tx_confirmation_height = height; - self.context.funding_tx_confirmed_in = Some(*block_hash); + self.funding.funding_tx_confirmed_in = Some(*block_hash); self.context.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) { Ok(scid) => Some(scid), Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"), @@ -8307,12 +8311,12 @@ impl FundedChannel where // 0-conf channel, but not doing so may lead to the // `ChannelManager::short_to_chan_info` map being inconsistent, so we currently have // to. - if funding_tx_confirmations == 0 && self.context.funding_tx_confirmed_in.is_some() { + if funding_tx_confirmations == 0 && self.funding.funding_tx_confirmed_in.is_some() { let err_reason = format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.", self.context.minimum_depth.unwrap(), funding_tx_confirmations); return Err(ClosureReason::ProcessingError { err: err_reason }); } - } else if !self.funding.is_outbound() && self.context.funding_tx_confirmed_in.is_none() && + } else if !self.funding.is_outbound() && self.funding.funding_tx_confirmed_in.is_none() && height >= self.context.channel_creation_height + FUNDING_CONF_DEADLINE_BLOCKS { log_info!(logger, "Closing channel {} due to funding timeout", &self.context.channel_id); // If funding_tx_confirmed_in is unset, the channel must not be active @@ -10690,7 +10694,7 @@ impl Writeable for FundedChannel where SP::Target: SignerProvider // consider the stale state on reload. 0u8.write(writer)?; - self.context.funding_tx_confirmed_in.write(writer)?; + self.funding.funding_tx_confirmed_in.write(writer)?; self.context.funding_tx_confirmation_height.write(writer)?; self.context.short_channel_id.write(writer)?; @@ -11326,6 +11330,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel channel_transaction_parameters: channel_parameters, funding_transaction, + funding_tx_confirmed_in, }, pending_funding: pending_funding.unwrap(), context: ChannelContext { @@ -11389,7 +11394,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel closing_fee_limits: None, target_closing_feerate_sats_per_kw, - funding_tx_confirmed_in, funding_tx_confirmation_height, short_channel_id, channel_creation_height, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 40e9537dee4..5c2f3cb0e6b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -11639,7 +11639,7 @@ where for chan in peer_state.channel_by_id.values().filter_map(Channel::as_funded) { let txid_opt = chan.funding.get_funding_txo(); let height_opt = chan.context.get_funding_tx_confirmation_height(); - let hash_opt = chan.context.get_funding_tx_confirmed_in(); + let hash_opt = chan.funding.get_funding_tx_confirmed_in(); if let (Some(funding_txo), Some(conf_height), Some(block_hash)) = (txid_opt, height_opt, hash_opt) { res.push((funding_txo.txid, conf_height, Some(block_hash))); } From 8d605b0a597ffc1e545373ca11834a98374db399 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 15 Apr 2025 16:07:25 -0500 Subject: [PATCH 3/8] Move ChannelContext::funding_tx_confirmation_height to FundingScope When processing confirmed transactions, if the funding transaction is found then information about it in the ChannelContext is updated. In preparation for splicing, move this data to FundingScope. --- lightning/src/ln/channel.rs | 83 ++++++++++++++++-------------- lightning/src/ln/channel_state.rs | 2 +- lightning/src/ln/channelmanager.rs | 32 ++++++------ 3 files changed, 61 insertions(+), 56 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 95e36d73df9..685ded1b346 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1687,6 +1687,7 @@ pub(super) struct FundingScope { funding_transaction: Option, /// The hash of the block in which the funding transaction was included. funding_tx_confirmed_in: Option, + funding_tx_confirmation_height: u32, } impl Writeable for FundingScope { @@ -1698,6 +1699,7 @@ impl Writeable for FundingScope { (7, self.channel_transaction_parameters, (required: ReadableArgs, None)), (9, self.funding_transaction, option), (11, self.funding_tx_confirmed_in, option), + (13, self.funding_tx_confirmation_height, required), }); Ok(()) } @@ -1711,6 +1713,7 @@ impl Readable for FundingScope { let mut channel_transaction_parameters = RequiredWrapper(None); let mut funding_transaction = None; let mut funding_tx_confirmed_in = None; + let mut funding_tx_confirmation_height = RequiredWrapper(None); read_tlv_fields!(reader, { (1, value_to_self_msat, required), @@ -1719,6 +1722,7 @@ impl Readable for FundingScope { (7, channel_transaction_parameters, (required: ReadableArgs, None)), (9, funding_transaction, option), (11, funding_tx_confirmed_in, option), + (13, funding_tx_confirmation_height, required), }); Ok(Self { @@ -1732,6 +1736,7 @@ impl Readable for FundingScope { channel_transaction_parameters: channel_transaction_parameters.0.unwrap(), funding_transaction, funding_tx_confirmed_in, + funding_tx_confirmation_height: funding_tx_confirmation_height.0.unwrap(), #[cfg(any(test, fuzzing))] next_local_commitment_tx_fee_info_cached: Mutex::new(None), #[cfg(any(test, fuzzing))] @@ -1810,6 +1815,26 @@ impl FundingScope { pub fn get_funding_tx_confirmed_in(&self) -> Option { self.funding_tx_confirmed_in } + + /// Returns the height in which our funding transaction was confirmed. + pub fn get_funding_tx_confirmation_height(&self) -> Option { + let conf_height = self.funding_tx_confirmation_height; + if conf_height > 0 { + Some(conf_height) + } else { + None + } + } + + /// Returns the current number of confirmations on the funding transaction. + pub fn get_funding_tx_confirmations(&self, height: u32) -> u32 { + if self.funding_tx_confirmation_height == 0 { + // We either haven't seen any confirmation yet, or observed a reorg. + return 0; + } + + height.checked_sub(self.funding_tx_confirmation_height).map_or(0, |c| c + 1) + } } /// Info about a pending splice, used in the pre-splice channel @@ -1969,7 +1994,6 @@ pub(super) struct ChannelContext where SP::Target: SignerProvider { /// milliseconds, so any accidental force-closes here should be exceedingly rare. expecting_peer_commitment_signed: bool, - funding_tx_confirmation_height: u32, short_channel_id: Option, /// Either the height at which this channel was created or the height at which it was last /// serialized if it was serialized by versions prior to 0.0.103. @@ -2780,6 +2804,7 @@ impl ChannelContext where SP::Target: SignerProvider { }, funding_transaction: None, funding_tx_confirmed_in: None, + funding_tx_confirmation_height: 0, }; let channel_context = ChannelContext { user_id, @@ -2843,7 +2868,6 @@ impl ChannelContext where SP::Target: SignerProvider { closing_fee_limits: None, target_closing_feerate_sats_per_kw: None, - funding_tx_confirmation_height: 0, short_channel_id: None, channel_creation_height: current_chain_height, @@ -3016,6 +3040,7 @@ impl ChannelContext where SP::Target: SignerProvider { }, funding_transaction: None, funding_tx_confirmed_in: None, + funding_tx_confirmation_height: 0, }; let channel_context = Self { user_id, @@ -3077,7 +3102,6 @@ impl ChannelContext where SP::Target: SignerProvider { closing_fee_limits: None, target_closing_feerate_sats_per_kw: None, - funding_tx_confirmation_height: 0, short_channel_id: None, channel_creation_height: current_chain_height, @@ -3319,16 +3343,6 @@ impl ChannelContext where SP::Target: SignerProvider { self.outbound_scid_alias = outbound_scid_alias; } - /// Returns the height in which our funding transaction was confirmed. - pub fn get_funding_tx_confirmation_height(&self) -> Option { - let conf_height = self.funding_tx_confirmation_height; - if conf_height > 0 { - Some(conf_height) - } else { - None - } - } - /// Performs checks against necessary constraints after receiving either an `accept_channel` or /// `accept_channel2` message. pub fn do_accept_channel_checks( @@ -3469,16 +3483,6 @@ impl ChannelContext where SP::Target: SignerProvider { Ok(()) } - /// Returns the current number of confirmations on the funding transaction. - pub fn get_funding_tx_confirmations(&self, height: u32) -> u32 { - if self.funding_tx_confirmation_height == 0 { - // We either haven't seen any confirmation yet, or observed a reorg. - return 0; - } - - height.checked_sub(self.funding_tx_confirmation_height).map_or(0, |c| c + 1) - } - /// Allowed in any state (including after shutdown) pub fn get_counterparty_node_id(&self) -> PublicKey { self.counterparty_node_id @@ -6740,7 +6744,7 @@ impl FundedChannel where matches!(self.context.channel_state, ChannelState::ChannelReady(_))) { // Broadcast only if not yet confirmed - if self.context.get_funding_tx_confirmation_height().is_none() { + if self.funding.get_funding_tx_confirmation_height().is_none() { funding_broadcastable = Some(funding_transaction.clone()) } } @@ -8076,13 +8080,13 @@ impl FundedChannel where // Called: // * always when a new block/transactions are confirmed with the new height // * when funding is signed with a height of 0 - if self.context.funding_tx_confirmation_height == 0 && self.context.minimum_depth != Some(0) { + if self.funding.funding_tx_confirmation_height == 0 && self.context.minimum_depth != Some(0) { return None; } - let funding_tx_confirmations = height as i64 - self.context.funding_tx_confirmation_height as i64 + 1; + let funding_tx_confirmations = height as i64 - self.funding.funding_tx_confirmation_height as i64 + 1; if funding_tx_confirmations <= 0 { - self.context.funding_tx_confirmation_height = 0; + self.funding.funding_tx_confirmation_height = 0; } if funding_tx_confirmations < self.context.minimum_depth.unwrap_or(0) as i64 { @@ -8102,7 +8106,7 @@ impl FundedChannel where // We got a reorg but not enough to trigger a force close, just ignore. false } else { - if self.context.funding_tx_confirmation_height != 0 && + if self.funding.funding_tx_confirmation_height != 0 && self.context.channel_state < ChannelState::ChannelReady(ChannelReadyFlags::new()) { // We should never see a funding transaction on-chain until we've received @@ -8170,7 +8174,7 @@ impl FundedChannel where for &(index_in_block, tx) in txdata.iter() { // Check if the transaction is the expected funding transaction, and if it is, // check that it pays the right amount to the right script. - if self.context.funding_tx_confirmation_height == 0 { + if self.funding.funding_tx_confirmation_height == 0 { if tx.compute_txid() == funding_txo.txid { let txo_idx = funding_txo.index as usize; if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != self.funding.get_funding_redeemscript().to_p2wsh() || @@ -8200,7 +8204,8 @@ impl FundedChannel where } } } - self.context.funding_tx_confirmation_height = height; + + self.funding.funding_tx_confirmation_height = height; self.funding.funding_tx_confirmed_in = Some(*block_hash); self.context.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) { Ok(scid) => Some(scid), @@ -8294,8 +8299,8 @@ impl FundedChannel where if matches!(self.context.channel_state, ChannelState::ChannelReady(_)) || self.context.channel_state.is_our_channel_ready() { - let mut funding_tx_confirmations = height as i64 - self.context.funding_tx_confirmation_height as i64 + 1; - if self.context.funding_tx_confirmation_height == 0 { + let mut funding_tx_confirmations = height as i64 - self.funding.funding_tx_confirmation_height as i64 + 1; + if self.funding.funding_tx_confirmation_height == 0 { // Note that check_get_channel_ready may reset funding_tx_confirmation_height to // zero if it has been reorged out, however in either case, our state flags // indicate we've already sent a channel_ready @@ -8335,10 +8340,10 @@ impl FundedChannel where /// force-close the channel, but may also indicate a harmless reorganization of a block or two /// before the channel has reached channel_ready and we can just wait for more blocks. pub fn funding_transaction_unconfirmed(&mut self, logger: &L) -> Result<(), ClosureReason> where L::Target: Logger { - if self.context.funding_tx_confirmation_height != 0 { + if self.funding.funding_tx_confirmation_height != 0 { // We handle the funding disconnection by calling best_block_updated with a height one // below where our funding was connected, implying a reorg back to conf_height - 1. - let reorg_height = self.context.funding_tx_confirmation_height - 1; + let reorg_height = self.funding.funding_tx_confirmation_height - 1; // We use the time field to bump the current time we set on channel updates if its // larger. If we don't know that time has moved forward, we can just set it to the last // time we saw and it will be ignored. @@ -8411,7 +8416,7 @@ impl FundedChannel where NS::Target: NodeSigner, L::Target: Logger { - if self.context.funding_tx_confirmation_height == 0 || self.context.funding_tx_confirmation_height + 5 > best_block_height { + if self.funding.funding_tx_confirmation_height == 0 || self.funding.funding_tx_confirmation_height + 5 > best_block_height { return None; } @@ -8532,7 +8537,7 @@ impl FundedChannel where } self.context.announcement_sigs = Some((msg.node_signature, msg.bitcoin_signature)); - if self.context.funding_tx_confirmation_height == 0 || self.context.funding_tx_confirmation_height + 5 > best_block_height { + if self.funding.funding_tx_confirmation_height == 0 || self.funding.funding_tx_confirmation_height + 5 > best_block_height { return Err(ChannelError::Ignore( "Got announcement_signatures prior to the required six confirmations - we may not have received a block yet that our peer has".to_owned())); } @@ -8545,7 +8550,7 @@ impl FundedChannel where pub fn get_signed_channel_announcement( &self, node_signer: &NS, chain_hash: ChainHash, best_block_height: u32, user_config: &UserConfig ) -> Option where NS::Target: NodeSigner { - if self.context.funding_tx_confirmation_height == 0 || self.context.funding_tx_confirmation_height + 5 > best_block_height { + if self.funding.funding_tx_confirmation_height == 0 || self.funding.funding_tx_confirmation_height + 5 > best_block_height { return None; } let announcement = match self.get_channel_announcement(node_signer, chain_hash, user_config) { @@ -10695,7 +10700,7 @@ impl Writeable for FundedChannel where SP::Target: SignerProvider 0u8.write(writer)?; self.funding.funding_tx_confirmed_in.write(writer)?; - self.context.funding_tx_confirmation_height.write(writer)?; + self.funding.funding_tx_confirmation_height.write(writer)?; self.context.short_channel_id.write(writer)?; self.context.counterparty_dust_limit_satoshis.write(writer)?; @@ -11331,6 +11336,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel channel_transaction_parameters: channel_parameters, funding_transaction, funding_tx_confirmed_in, + funding_tx_confirmation_height, }, pending_funding: pending_funding.unwrap(), context: ChannelContext { @@ -11394,7 +11400,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel closing_fee_limits: None, target_closing_feerate_sats_per_kw, - funding_tx_confirmation_height, short_channel_id, channel_creation_height, diff --git a/lightning/src/ln/channel_state.rs b/lightning/src/ln/channel_state.rs index c941e0eb9d0..f822b664166 100644 --- a/lightning/src/ln/channel_state.rs +++ b/lightning/src/ln/channel_state.rs @@ -532,7 +532,7 @@ impl ChannelDetails { next_outbound_htlc_minimum_msat: balance.next_outbound_htlc_minimum_msat, user_channel_id: context.get_user_id(), confirmations_required: context.minimum_depth(), - confirmations: Some(context.get_funding_tx_confirmations(best_block_height)), + confirmations: Some(funding.get_funding_tx_confirmations(best_block_height)), force_close_spend_delay: funding.get_counterparty_selected_contest_delay(), is_outbound: funding.is_outbound(), is_channel_ready: context.is_usable(), diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5c2f3cb0e6b..023769ca075 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3016,7 +3016,7 @@ macro_rules! handle_error { /// Note that this step can be skipped if the channel was never opened (through the creation of a /// [`ChannelMonitor`]/channel funding transaction) to begin with. macro_rules! locked_close_channel { - ($self: ident, $peer_state: expr, $channel_context: expr, $shutdown_res_mut: expr) => {{ + ($self: ident, $peer_state: expr, $channel_context: expr, $channel_funding: expr, $shutdown_res_mut: expr) => {{ if let Some((_, funding_txo, _, update)) = $shutdown_res_mut.monitor_update.take() { handle_new_monitor_update!($self, funding_txo, update, $peer_state, $channel_context, REMAIN_LOCKED_UPDATE_ACTIONS_PROCESSED_LATER); @@ -3026,7 +3026,7 @@ macro_rules! locked_close_channel { // into the map (which prevents the `PeerState` from being cleaned up) for channels that // never even got confirmations (which would open us up to DoS attacks). let update_id = $channel_context.get_latest_monitor_update_id(); - if $channel_context.get_funding_tx_confirmation_height().is_some() || $channel_context.minimum_depth() == Some(0) || update_id > 1 { + if $channel_funding.get_funding_tx_confirmation_height().is_some() || $channel_context.minimum_depth() == Some(0) || update_id > 1 { let chan_id = $channel_context.channel_id(); $peer_state.closed_channel_monitor_update_ids.insert(chan_id, update_id); } @@ -3064,7 +3064,7 @@ macro_rules! convert_channel_err { let logger = WithChannelContext::from(&$self.logger, &$context, None); log_error!(logger, "Closing channel {} due to close-required error: {}", $channel_id, msg); let mut shutdown_res = $context.force_shutdown($funding, true, reason); - locked_close_channel!($self, $peer_state, $context, &mut shutdown_res); + locked_close_channel!($self, $peer_state, $context, $funding, &mut shutdown_res); let err = MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, shutdown_res, $channel_update); (true, err) @@ -3129,7 +3129,7 @@ macro_rules! remove_channel_entry { ($self: ident, $peer_state: expr, $entry: expr, $shutdown_res_mut: expr) => { { let channel = $entry.remove_entry().1; - locked_close_channel!($self, $peer_state, &channel.context(), $shutdown_res_mut); + locked_close_channel!($self, $peer_state, &channel.context(), channel.funding(), $shutdown_res_mut); channel } } @@ -4084,7 +4084,7 @@ where let mut peer_state = peer_state_mutex.lock().unwrap(); if let Some(mut chan) = peer_state.channel_by_id.remove(&channel_id) { let mut close_res = chan.force_shutdown(false, ClosureReason::FundingBatchClosure); - locked_close_channel!(self, &mut *peer_state, chan.context(), close_res); + locked_close_channel!(self, &mut *peer_state, chan.context(), chan.funding(), close_res); shutdown_results.push(close_res); } } @@ -5489,7 +5489,7 @@ where .map(|(mut chan, mut peer_state)| { let closure_reason = ClosureReason::ProcessingError { err: e.clone() }; let mut close_res = chan.force_shutdown(false, closure_reason); - locked_close_channel!(self, peer_state, chan.context(), close_res); + locked_close_channel!(self, peer_state, chan.context(), chan.funding(), close_res); shutdown_results.push(close_res); peer_state.pending_msg_events.push(MessageSendEvent::HandleError { node_id: counterparty_node_id, @@ -6737,8 +6737,8 @@ where "Force-closing pending channel with ID {} for not establishing in a timely manner", context.channel_id()); let mut close_res = chan.force_shutdown(false, ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) }); - let context = chan.context_mut(); - locked_close_channel!(self, peer_state, context, close_res); + let (funding, context) = chan.funding_and_context_mut(); + locked_close_channel!(self, peer_state, context, funding, close_res); shutdown_channels.push(close_res); pending_msg_events.push(MessageSendEvent::HandleError { node_id: context.get_counterparty_node_id(), @@ -8024,7 +8024,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ // This covers non-zero-conf inbound `Channel`s that we are currently monitoring, but those // which have not yet had any confirmations on-chain. if !funded_chan.funding.is_outbound() && funded_chan.context.minimum_depth().unwrap_or(1) != 0 && - funded_chan.context.get_funding_tx_confirmations(best_block_height) == 0 + funded_chan.funding.get_funding_tx_confirmations(best_block_height) == 0 { num_unfunded_channels += 1; } @@ -9849,7 +9849,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let context = &chan.context(); let logger = WithChannelContext::from(&self.logger, context, None); log_trace!(logger, "Removing channel {} now that the signer is unblocked", context.channel_id()); - locked_close_channel!(self, peer_state, context, shutdown_result); + locked_close_channel!(self, peer_state, context, chan.funding(), shutdown_result); shutdown_results.push(shutdown_result); false } else { @@ -9891,7 +9891,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } debug_assert_eq!(shutdown_result_opt.is_some(), funded_chan.is_shutdown()); if let Some(mut shutdown_result) = shutdown_result_opt { - locked_close_channel!(self, peer_state, &funded_chan.context, shutdown_result); + locked_close_channel!(self, peer_state, &funded_chan.context, &funded_chan.funding, shutdown_result); shutdown_results.push(shutdown_result); } if let Some(tx) = tx_opt { @@ -11217,8 +11217,8 @@ where } // Clean up for removal. let mut close_res = chan.force_shutdown(false, ClosureReason::DisconnectedPeer); - let context = chan.context_mut(); - locked_close_channel!(self, peer_state, &context, close_res); + let (funding, context) = chan.funding_and_context_mut(); + locked_close_channel!(self, peer_state, &context, funding, close_res); failed_channels.push(close_res); false }); @@ -11638,7 +11638,7 @@ where let peer_state = &mut *peer_state_lock; for chan in peer_state.channel_by_id.values().filter_map(Channel::as_funded) { let txid_opt = chan.funding.get_funding_txo(); - let height_opt = chan.context.get_funding_tx_confirmation_height(); + let height_opt = chan.funding.get_funding_tx_confirmation_height(); let hash_opt = chan.funding.get_funding_tx_confirmed_in(); if let (Some(funding_txo), Some(conf_height), Some(block_hash)) = (txid_opt, height_opt, hash_opt) { res.push((funding_txo.txid, conf_height, Some(block_hash))); @@ -11737,7 +11737,7 @@ where // (re-)broadcast signed `channel_announcement`s and // `channel_update`s for any channels less than a week old. let funding_conf_height = - funded_channel.context.get_funding_tx_confirmation_height().unwrap_or(height); + funded_channel.funding.get_funding_tx_confirmation_height().unwrap_or(height); // To avoid broadcast storms after each block, only // re-broadcast every hour (6 blocks) after the initial // broadcast, or if this is the first time we're ready to @@ -11795,7 +11795,7 @@ where // reorged out of the main chain. Close the channel. let reason_message = format!("{}", reason); let mut close_res = funded_channel.context.force_shutdown(&funded_channel.funding, true, reason); - locked_close_channel!(self, peer_state, &funded_channel.context, close_res); + locked_close_channel!(self, peer_state, &funded_channel.context, &funded_channel.funding, close_res); failed_channels.push(close_res); if let Ok(update) = self.get_channel_update_for_broadcast(&funded_channel) { let mut pending_broadcast_messages = self.pending_broadcast_messages.lock().unwrap(); From d830342b5654f0de76bc2a33cf9b5d2b4cd93177 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 15 Apr 2025 16:20:57 -0500 Subject: [PATCH 4/8] Move ChannelContext::short_channel_id to FundingScope When processing confirmed transactions, if the funding transaction is found then information about it in the ChannelContext is updated. In preparation for splicing, move this data to FundingScope. --- lightning/src/ln/channel.rs | 36 +++++++++++++++++------------- lightning/src/ln/channel_state.rs | 2 +- lightning/src/ln/channelmanager.rs | 20 ++++++++--------- lightning/src/ln/payment_tests.rs | 14 ++++++------ 4 files changed, 38 insertions(+), 34 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 685ded1b346..cb4a7a11179 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1688,6 +1688,7 @@ pub(super) struct FundingScope { /// The hash of the block in which the funding transaction was included. funding_tx_confirmed_in: Option, funding_tx_confirmation_height: u32, + short_channel_id: Option, } impl Writeable for FundingScope { @@ -1700,6 +1701,7 @@ impl Writeable for FundingScope { (9, self.funding_transaction, option), (11, self.funding_tx_confirmed_in, option), (13, self.funding_tx_confirmation_height, required), + (15, self.short_channel_id, option), }); Ok(()) } @@ -1714,6 +1716,7 @@ impl Readable for FundingScope { let mut funding_transaction = None; let mut funding_tx_confirmed_in = None; let mut funding_tx_confirmation_height = RequiredWrapper(None); + let mut short_channel_id = None; read_tlv_fields!(reader, { (1, value_to_self_msat, required), @@ -1723,6 +1726,7 @@ impl Readable for FundingScope { (9, funding_transaction, option), (11, funding_tx_confirmed_in, option), (13, funding_tx_confirmation_height, required), + (15, short_channel_id, option), }); Ok(Self { @@ -1737,6 +1741,7 @@ impl Readable for FundingScope { funding_transaction, funding_tx_confirmed_in, funding_tx_confirmation_height: funding_tx_confirmation_height.0.unwrap(), + short_channel_id, #[cfg(any(test, fuzzing))] next_local_commitment_tx_fee_info_cached: Mutex::new(None), #[cfg(any(test, fuzzing))] @@ -1835,6 +1840,13 @@ impl FundingScope { height.checked_sub(self.funding_tx_confirmation_height).map_or(0, |c| c + 1) } + + /// Gets the channel's `short_channel_id`. + /// + /// Will return `None` if the funding hasn't been confirmed yet. + pub fn get_short_channel_id(&self) -> Option { + self.short_channel_id + } } /// Info about a pending splice, used in the pre-splice channel @@ -1994,7 +2006,6 @@ pub(super) struct ChannelContext where SP::Target: SignerProvider { /// milliseconds, so any accidental force-closes here should be exceedingly rare. expecting_peer_commitment_signed: bool, - short_channel_id: Option, /// Either the height at which this channel was created or the height at which it was last /// serialized if it was serialized by versions prior to 0.0.103. /// We use this to close if funding is never broadcasted. @@ -2805,6 +2816,7 @@ impl ChannelContext where SP::Target: SignerProvider { funding_transaction: None, funding_tx_confirmed_in: None, funding_tx_confirmation_height: 0, + short_channel_id: None, }; let channel_context = ChannelContext { user_id, @@ -2868,7 +2880,6 @@ impl ChannelContext where SP::Target: SignerProvider { closing_fee_limits: None, target_closing_feerate_sats_per_kw: None, - short_channel_id: None, channel_creation_height: current_chain_height, feerate_per_kw: open_channel_fields.commitment_feerate_sat_per_1000_weight, @@ -3041,6 +3052,7 @@ impl ChannelContext where SP::Target: SignerProvider { funding_transaction: None, funding_tx_confirmed_in: None, funding_tx_confirmation_height: 0, + short_channel_id: None, }; let channel_context = Self { user_id, @@ -3102,7 +3114,6 @@ impl ChannelContext where SP::Target: SignerProvider { closing_fee_limits: None, target_closing_feerate_sats_per_kw: None, - short_channel_id: None, channel_creation_height: current_chain_height, feerate_per_kw: commitment_feerate, @@ -3312,13 +3323,6 @@ impl ChannelContext where SP::Target: SignerProvider { self.user_id } - /// Gets the channel's `short_channel_id`. - /// - /// Will return `None` if the channel hasn't been confirmed yet. - pub fn get_short_channel_id(&self) -> Option { - self.short_channel_id - } - /// Allowed in any state (including after shutdown) pub fn latest_inbound_scid_alias(&self) -> Option { self.latest_inbound_scid_alias @@ -5531,7 +5535,7 @@ impl FundedChannel where } if let Some(scid_alias) = msg.short_channel_id_alias { - if Some(scid_alias) != self.context.short_channel_id { + if Some(scid_alias) != self.funding.short_channel_id { // The scid alias provided can be used to route payments *from* our counterparty, // i.e. can be used for inbound payments and provided in invoices, but is not used // when routing outbound payments. @@ -8207,7 +8211,7 @@ impl FundedChannel where self.funding.funding_tx_confirmation_height = height; self.funding.funding_tx_confirmed_in = Some(*block_hash); - self.context.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) { + self.funding.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) { Ok(scid) => Some(scid), Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"), } @@ -8387,7 +8391,7 @@ impl FundedChannel where return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel is not currently usable".to_owned())); } - let short_channel_id = self.context.get_short_channel_id() + let short_channel_id = self.funding.get_short_channel_id() .ok_or(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel has not been confirmed yet".to_owned()))?; let node_id = NodeId::from_pubkey(&node_signer.get_node_id(Recipient::Node) .map_err(|_| ChannelError::Ignore("Failed to retrieve own public key".to_owned()))?); @@ -8459,7 +8463,7 @@ impl FundedChannel where }, Ok(v) => v }; - let short_channel_id = match self.context.get_short_channel_id() { + let short_channel_id = match self.funding.get_short_channel_id() { Some(scid) => scid, None => return None, }; @@ -10701,7 +10705,7 @@ impl Writeable for FundedChannel where SP::Target: SignerProvider self.funding.funding_tx_confirmed_in.write(writer)?; self.funding.funding_tx_confirmation_height.write(writer)?; - self.context.short_channel_id.write(writer)?; + self.funding.short_channel_id.write(writer)?; self.context.counterparty_dust_limit_satoshis.write(writer)?; self.context.holder_dust_limit_satoshis.write(writer)?; @@ -11337,6 +11341,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel funding_transaction, funding_tx_confirmed_in, funding_tx_confirmation_height, + short_channel_id, }, pending_funding: pending_funding.unwrap(), context: ChannelContext { @@ -11400,7 +11405,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel closing_fee_limits: None, target_closing_feerate_sats_per_kw, - short_channel_id, channel_creation_height, counterparty_dust_limit_satoshis, diff --git a/lightning/src/ln/channel_state.rs b/lightning/src/ln/channel_state.rs index f822b664166..99e2833f644 100644 --- a/lightning/src/ln/channel_state.rs +++ b/lightning/src/ln/channel_state.rs @@ -516,7 +516,7 @@ impl ChannelDetails { } else { None }, - short_channel_id: context.get_short_channel_id(), + short_channel_id: funding.get_short_channel_id(), outbound_scid_alias: if context.is_usable() { Some(context.outbound_scid_alias()) } else { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 023769ca075..b2529993fed 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3031,7 +3031,7 @@ macro_rules! locked_close_channel { $peer_state.closed_channel_monitor_update_ids.insert(chan_id, update_id); } let mut short_to_chan_info = $self.short_to_chan_info.write().unwrap(); - if let Some(short_id) = $channel_context.get_short_channel_id() { + if let Some(short_id) = $channel_funding.get_short_channel_id() { short_to_chan_info.remove(&short_id); } else { // If the channel was never confirmed on-chain prior to its closure, remove the @@ -3147,7 +3147,7 @@ macro_rules! send_channel_ready { let outbound_alias_insert = short_to_chan_info.insert($channel.context.outbound_scid_alias(), ($channel.context.get_counterparty_node_id(), $channel.context.channel_id())); assert!(outbound_alias_insert.is_none() || outbound_alias_insert.unwrap() == ($channel.context.get_counterparty_node_id(), $channel.context.channel_id()), "SCIDs should never collide - ensure you weren't behind the chain tip by a full month when creating channels"); - if let Some(real_scid) = $channel.context.get_short_channel_id() { + if let Some(real_scid) = $channel.funding.get_short_channel_id() { let scid_insert = short_to_chan_info.insert(real_scid, ($channel.context.get_counterparty_node_id(), $channel.context.channel_id())); assert!(scid_insert.is_none() || scid_insert.unwrap() == ($channel.context.get_counterparty_node_id(), $channel.context.channel_id()), "SCIDs should never collide - ensure you weren't behind the chain tip by a full month when creating channels"); @@ -4572,7 +4572,7 @@ where action: msgs::ErrorAction::IgnoreError }); } - if chan.context.get_short_channel_id().is_none() { + if chan.funding.get_short_channel_id().is_none() { return Err(LightningError{err: "Channel not yet established".to_owned(), action: msgs::ErrorAction::IgnoreError}); } let logger = WithChannelContext::from(&self.logger, &chan.context, None); @@ -4594,7 +4594,7 @@ where fn get_channel_update_for_unicast(&self, chan: &FundedChannel) -> Result { let logger = WithChannelContext::from(&self.logger, &chan.context, None); log_trace!(logger, "Attempting to generate channel update for channel {}", chan.context.channel_id()); - let short_channel_id = match chan.context.get_short_channel_id().or(chan.context.latest_inbound_scid_alias()) { + let short_channel_id = match chan.funding.get_short_channel_id().or(chan.context.latest_inbound_scid_alias()) { None => return Err(LightningError{err: "Channel not yet established".to_owned(), action: msgs::ErrorAction::IgnoreError}), Some(id) => id, }; @@ -5657,7 +5657,7 @@ where err: format!("Channel with id {} not fully established", next_hop_channel_id) }) } - funded_chan.context.get_short_channel_id().unwrap_or(funded_chan.context.outbound_scid_alias()) + funded_chan.funding.get_short_channel_id().unwrap_or(funded_chan.context.outbound_scid_alias()) } else { return Err(APIError::ChannelUnavailable { err: format!("Channel with id {} for the passed counterparty node_id {} is still opening.", @@ -6099,7 +6099,7 @@ where }; let logger = WithChannelContext::from(&self.logger, &optimal_channel.context, Some(payment_hash)); - let channel_description = if optimal_channel.context.get_short_channel_id() == Some(short_chan_id) { + let channel_description = if optimal_channel.funding.get_short_channel_id() == Some(short_chan_id) { "specified" } else { "alternate" @@ -7663,7 +7663,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ ); let counterparty_node_id = channel.context.get_counterparty_node_id(); - let short_channel_id = channel.context.get_short_channel_id().unwrap_or(channel.context.outbound_scid_alias()); + let short_channel_id = channel.funding.get_short_channel_id().unwrap_or(channel.context.outbound_scid_alias()); let mut htlc_forwards = None; if !pending_forwards.is_empty() { @@ -10913,7 +10913,7 @@ where .iter() .filter(|(_, channel)| channel.context().is_usable()) .min_by_key(|(_, channel)| channel.context().channel_creation_height) - .and_then(|(_, channel)| channel.context().get_short_channel_id()), + .and_then(|(_, channel)| channel.funding().get_short_channel_id()), }) .collect::>(); @@ -11776,7 +11776,7 @@ where }); } if funded_channel.is_our_channel_ready() { - if let Some(real_scid) = funded_channel.context.get_short_channel_id() { + if let Some(real_scid) = funded_channel.funding.get_short_channel_id() { // If we sent a 0conf channel_ready, and now have an SCID, we add it // to the short_to_chan_info map here. Note that we check whether we // can relay using the real SCID at relay-time (i.e. @@ -13870,7 +13870,7 @@ where log_info!(logger, "Successfully loaded channel {} at update_id {} against monitor at update id {} with {} blocked updates", &channel.context.channel_id(), channel.context.get_latest_monitor_update_id(), monitor.get_latest_update_id(), channel.blocked_monitor_updates_pending()); - if let Some(short_channel_id) = channel.context.get_short_channel_id() { + if let Some(short_channel_id) = channel.funding.get_short_channel_id() { short_to_chan_info.insert(short_channel_id, (channel.context.get_counterparty_node_id(), channel.context.channel_id())); } per_peer_state.entry(channel.context.get_counterparty_node_id()) diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 13570393288..da81674ae9b 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -1709,7 +1709,7 @@ fn test_trivial_inflight_htlc_tracking(){ let chan_1_used_liquidity = inflight_htlcs.used_liquidity_msat( &NodeId::from_pubkey(&nodes[0].node.get_our_node_id()) , &NodeId::from_pubkey(&nodes[1].node.get_our_node_id()), - channel_1.context().get_short_channel_id().unwrap() + channel_1.funding().get_short_channel_id().unwrap() ); assert_eq!(chan_1_used_liquidity, None); } @@ -1721,7 +1721,7 @@ fn test_trivial_inflight_htlc_tracking(){ let chan_2_used_liquidity = inflight_htlcs.used_liquidity_msat( &NodeId::from_pubkey(&nodes[1].node.get_our_node_id()) , &NodeId::from_pubkey(&nodes[2].node.get_our_node_id()), - channel_2.context().get_short_channel_id().unwrap() + channel_2.funding().get_short_channel_id().unwrap() ); assert_eq!(chan_2_used_liquidity, None); @@ -1746,7 +1746,7 @@ fn test_trivial_inflight_htlc_tracking(){ let chan_1_used_liquidity = inflight_htlcs.used_liquidity_msat( &NodeId::from_pubkey(&nodes[0].node.get_our_node_id()) , &NodeId::from_pubkey(&nodes[1].node.get_our_node_id()), - channel_1.context().get_short_channel_id().unwrap() + channel_1.funding().get_short_channel_id().unwrap() ); // First hop accounts for expected 1000 msat fee assert_eq!(chan_1_used_liquidity, Some(501000)); @@ -1759,7 +1759,7 @@ fn test_trivial_inflight_htlc_tracking(){ let chan_2_used_liquidity = inflight_htlcs.used_liquidity_msat( &NodeId::from_pubkey(&nodes[1].node.get_our_node_id()) , &NodeId::from_pubkey(&nodes[2].node.get_our_node_id()), - channel_2.context().get_short_channel_id().unwrap() + channel_2.funding().get_short_channel_id().unwrap() ); assert_eq!(chan_2_used_liquidity, Some(500000)); @@ -1785,7 +1785,7 @@ fn test_trivial_inflight_htlc_tracking(){ let chan_1_used_liquidity = inflight_htlcs.used_liquidity_msat( &NodeId::from_pubkey(&nodes[0].node.get_our_node_id()) , &NodeId::from_pubkey(&nodes[1].node.get_our_node_id()), - channel_1.context().get_short_channel_id().unwrap() + channel_1.funding().get_short_channel_id().unwrap() ); assert_eq!(chan_1_used_liquidity, None); } @@ -1797,7 +1797,7 @@ fn test_trivial_inflight_htlc_tracking(){ let chan_2_used_liquidity = inflight_htlcs.used_liquidity_msat( &NodeId::from_pubkey(&nodes[1].node.get_our_node_id()) , &NodeId::from_pubkey(&nodes[2].node.get_our_node_id()), - channel_2.context().get_short_channel_id().unwrap() + channel_2.funding().get_short_channel_id().unwrap() ); assert_eq!(chan_2_used_liquidity, None); } @@ -1838,7 +1838,7 @@ fn test_holding_cell_inflight_htlcs() { let used_liquidity = inflight_htlcs.used_liquidity_msat( &NodeId::from_pubkey(&nodes[0].node.get_our_node_id()) , &NodeId::from_pubkey(&nodes[1].node.get_our_node_id()), - channel.context().get_short_channel_id().unwrap() + channel.funding().get_short_channel_id().unwrap() ); assert_eq!(used_liquidity, Some(2000000)); From 4b4ce817741d31280187cc99aa892a721a868f1d Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 11 Apr 2025 14:11:41 -0500 Subject: [PATCH 5/8] Move ChannelContext::minimum_depth to FundingScope When processing confirmed transactions, if the funding transaction is found then information about it in the ChannelContext is updated. In preparation for splicing, move this data to FundingScope. --- lightning/src/ln/channel.rs | 65 ++++++++++++++++++------------ lightning/src/ln/channel_state.rs | 2 +- lightning/src/ln/channelmanager.rs | 8 ++-- 3 files changed, 45 insertions(+), 30 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index cb4a7a11179..1b8724c3ecc 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1689,6 +1689,9 @@ pub(super) struct FundingScope { funding_tx_confirmed_in: Option, funding_tx_confirmation_height: u32, short_channel_id: Option, + + /// The number of confirmation required before sending `channel_ready` or `splice_locked`. + minimum_depth: Option, } impl Writeable for FundingScope { @@ -1702,6 +1705,7 @@ impl Writeable for FundingScope { (11, self.funding_tx_confirmed_in, option), (13, self.funding_tx_confirmation_height, required), (15, self.short_channel_id, option), + (17, self.minimum_depth, option), }); Ok(()) } @@ -1717,6 +1721,7 @@ impl Readable for FundingScope { let mut funding_tx_confirmed_in = None; let mut funding_tx_confirmation_height = RequiredWrapper(None); let mut short_channel_id = None; + let mut minimum_depth = None; read_tlv_fields!(reader, { (1, value_to_self_msat, required), @@ -1727,6 +1732,7 @@ impl Readable for FundingScope { (11, funding_tx_confirmed_in, option), (13, funding_tx_confirmation_height, required), (15, short_channel_id, option), + (17, minimum_depth, option), }); Ok(Self { @@ -1742,6 +1748,7 @@ impl Readable for FundingScope { funding_tx_confirmed_in, funding_tx_confirmation_height: funding_tx_confirmation_height.0.unwrap(), short_channel_id, + minimum_depth, #[cfg(any(test, fuzzing))] next_local_commitment_tx_fee_info_cached: Mutex::new(None), #[cfg(any(test, fuzzing))] @@ -1847,6 +1854,10 @@ impl FundingScope { pub fn get_short_channel_id(&self) -> Option { self.short_channel_id } + + pub fn minimum_depth(&self) -> Option { + self.minimum_depth + } } /// Info about a pending splice, used in the pre-splice channel @@ -2035,7 +2046,7 @@ pub(super) struct ChannelContext where SP::Target: SignerProvider { #[cfg(not(any(test, feature="_test_utils")))] counterparty_max_accepted_htlcs: u16, holder_max_accepted_htlcs: u16, - minimum_depth: Option, + negotiated_minimum_depth: Option, counterparty_forwarding_info: Option, @@ -2817,6 +2828,7 @@ impl ChannelContext where SP::Target: SignerProvider { funding_tx_confirmed_in: None, funding_tx_confirmation_height: 0, short_channel_id: None, + minimum_depth, }; let channel_context = ChannelContext { user_id, @@ -2891,7 +2903,7 @@ impl ChannelContext where SP::Target: SignerProvider { holder_htlc_minimum_msat: if config.channel_handshake_config.our_htlc_minimum_msat == 0 { 1 } else { config.channel_handshake_config.our_htlc_minimum_msat }, counterparty_max_accepted_htlcs: open_channel_fields.max_accepted_htlcs, holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, max_htlcs(&channel_type)), - minimum_depth, + negotiated_minimum_depth: minimum_depth, counterparty_forwarding_info: None, @@ -3053,6 +3065,7 @@ impl ChannelContext where SP::Target: SignerProvider { funding_tx_confirmed_in: None, funding_tx_confirmation_height: 0, short_channel_id: None, + minimum_depth: None, // Filled in in accept_channel }; let channel_context = Self { user_id, @@ -3127,7 +3140,7 @@ impl ChannelContext where SP::Target: SignerProvider { holder_htlc_minimum_msat: if config.channel_handshake_config.our_htlc_minimum_msat == 0 { 1 } else { config.channel_handshake_config.our_htlc_minimum_msat }, counterparty_max_accepted_htlcs: 0, holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, max_htlcs(&channel_type)), - minimum_depth: None, // Filled in in accept_channel + negotiated_minimum_depth: None, // Filled in in accept_channel counterparty_forwarding_info: None, @@ -3313,10 +3326,6 @@ impl ChannelContext where SP::Target: SignerProvider { self.temporary_channel_id } - pub fn minimum_depth(&self) -> Option { - self.minimum_depth - } - /// Gets the "user_id" value passed into the construction of this channel. It has no special /// meaning and exists only to allow users to have a persistent identifier of a channel. pub fn get_user_id(&self) -> u128 { @@ -3458,10 +3467,11 @@ impl ChannelContext where SP::Target: SignerProvider { self.counterparty_max_accepted_htlcs = common_fields.max_accepted_htlcs; if peer_limits.trust_own_funding_0conf { - self.minimum_depth = Some(common_fields.minimum_depth); + funding.minimum_depth = Some(common_fields.minimum_depth); } else { - self.minimum_depth = Some(cmp::max(1, common_fields.minimum_depth)); + funding.minimum_depth = Some(cmp::max(1, common_fields.minimum_depth)); } + self.negotiated_minimum_depth = funding.minimum_depth; let counterparty_pubkeys = ChannelPublicKeys { funding_pubkey: common_fields.funding_pubkey, @@ -6761,7 +6771,7 @@ impl FundedChannel where // the funding transaction confirmed before the monitor was persisted, or // * a 0-conf channel and intended to send the channel_ready before any broadcast at all. let channel_ready = if self.context.monitor_pending_channel_ready { - assert!(!self.funding.is_outbound() || self.context.minimum_depth == Some(0), + assert!(!self.funding.is_outbound() || self.funding.minimum_depth == Some(0), "Funding transaction broadcast by the local client before it should have - LDK didn't do it!"); self.context.monitor_pending_channel_ready = false; self.get_channel_ready(logger) @@ -8010,7 +8020,7 @@ impl FundedChannel where ) { // If we're not a 0conf channel, we'll be waiting on a monitor update with only // AwaitingChannelReady set, though our peer could have sent their channel_ready. - debug_assert!(self.context.minimum_depth.unwrap_or(1) > 0); + debug_assert!(self.funding.minimum_depth.unwrap_or(1) > 0); return true; } if self.holder_commitment_point.transaction_number() == INITIAL_COMMITMENT_NUMBER - 1 && @@ -8084,7 +8094,7 @@ impl FundedChannel where // Called: // * always when a new block/transactions are confirmed with the new height // * when funding is signed with a height of 0 - if self.funding.funding_tx_confirmation_height == 0 && self.context.minimum_depth != Some(0) { + if self.funding.funding_tx_confirmation_height == 0 && self.funding.minimum_depth != Some(0) { return None; } @@ -8093,7 +8103,7 @@ impl FundedChannel where self.funding.funding_tx_confirmation_height = 0; } - if funding_tx_confirmations < self.context.minimum_depth.unwrap_or(0) as i64 { + if funding_tx_confirmations < self.funding.minimum_depth.unwrap_or(0) as i64 { return None; } @@ -8219,9 +8229,9 @@ impl FundedChannel where // If this is a coinbase transaction and not a 0-conf channel // we should update our min_depth to 100 to handle coinbase maturity if tx.is_coinbase() && - self.context.minimum_depth.unwrap_or(0) > 0 && - self.context.minimum_depth.unwrap_or(0) < COINBASE_MATURITY { - self.context.minimum_depth = Some(COINBASE_MATURITY); + self.funding.minimum_depth.unwrap_or(0) > 0 && + self.funding.minimum_depth.unwrap_or(0) < COINBASE_MATURITY { + self.funding.minimum_depth = Some(COINBASE_MATURITY); } } // If we allow 1-conf funding, we may need to check for channel_ready here and @@ -8322,7 +8332,7 @@ impl FundedChannel where // to. if funding_tx_confirmations == 0 && self.funding.funding_tx_confirmed_in.is_some() { let err_reason = format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.", - self.context.minimum_depth.unwrap(), funding_tx_confirmations); + self.funding.minimum_depth.unwrap(), funding_tx_confirmations); return Err(ClosureReason::ProcessingError { err: err_reason }); } } else if !self.funding.is_outbound() && self.funding.funding_tx_confirmed_in.is_none() && @@ -9608,9 +9618,9 @@ impl OutboundV1Channel where SP::Target: SignerProvider { // If the funding transaction is a coinbase transaction, we need to set the minimum depth to 100. // We can skip this if it is a zero-conf channel. if funding_transaction.is_coinbase() && - self.context.minimum_depth.unwrap_or(0) > 0 && - self.context.minimum_depth.unwrap_or(0) < COINBASE_MATURITY { - self.context.minimum_depth = Some(COINBASE_MATURITY); + self.funding.minimum_depth.unwrap_or(0) > 0 && + self.funding.minimum_depth.unwrap_or(0) < COINBASE_MATURITY { + self.funding.minimum_depth = Some(COINBASE_MATURITY); } debug_assert!(self.funding.funding_transaction.is_none()); @@ -9939,7 +9949,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { dust_limit_satoshis: self.context.holder_dust_limit_satoshis, max_htlc_value_in_flight_msat: self.context.holder_max_htlc_value_in_flight_msat, htlc_minimum_msat: self.context.holder_htlc_minimum_msat, - minimum_depth: self.context.minimum_depth.unwrap(), + minimum_depth: self.funding.minimum_depth.unwrap(), to_self_delay: self.funding.get_holder_selected_contest_delay(), max_accepted_htlcs: self.context.holder_max_accepted_htlcs, funding_pubkey: keys.funding_pubkey, @@ -10351,7 +10361,7 @@ impl PendingV2Channel where SP::Target: SignerProvider { dust_limit_satoshis: self.context.holder_dust_limit_satoshis, max_htlc_value_in_flight_msat: self.context.holder_max_htlc_value_in_flight_msat, htlc_minimum_msat: self.context.holder_htlc_minimum_msat, - minimum_depth: self.context.minimum_depth.unwrap(), + minimum_depth: self.funding.minimum_depth.unwrap(), to_self_delay: self.funding.get_holder_selected_contest_delay(), max_accepted_htlcs: self.context.holder_max_accepted_htlcs, funding_pubkey: keys.funding_pubkey, @@ -10719,7 +10729,7 @@ impl Writeable for FundedChannel where SP::Target: SignerProvider self.context.counterparty_max_accepted_htlcs.write(writer)?; // Note that this field is ignored by 0.0.99+ as the TLV Optional variant is used instead. - self.context.minimum_depth.unwrap_or(0).write(writer)?; + self.context.negotiated_minimum_depth.unwrap_or(0).write(writer)?; match &self.context.counterparty_forwarding_info { Some(info) => { @@ -10794,7 +10804,7 @@ impl Writeable for FundedChannel where SP::Target: SignerProvider // here. On the read side, old versions will simply ignore the odd-type entries here, // and new versions map the default values to None and allow the TLV entries here to // override that. - (1, self.context.minimum_depth, option), + (1, self.funding.minimum_depth, option), (2, chan_type, option), (3, self.funding.counterparty_selected_channel_reserve_satoshis, option), (4, serialized_holder_selected_reserve, option), @@ -10830,6 +10840,7 @@ impl Writeable for FundedChannel where SP::Target: SignerProvider (54, self.pending_funding, optional_vec), // Added in 0.2 (55, removed_htlc_failure_attribution_data, optional_vec), // Added in 0.2 (57, holding_cell_failure_attribution_data, optional_vec), // Added in 0.2 + (59, self.context.negotiated_minimum_depth, option), // Added in 0.2 }); Ok(()) @@ -11128,6 +11139,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel let mut is_manual_broadcast = None; let mut pending_funding = Some(Vec::new()); + let mut negotiated_minimum_depth: Option = None; read_tlv_fields!(reader, { (0, announcement_sigs, option), @@ -11167,6 +11179,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel (54, pending_funding, optional_vec), // Added in 0.2 (55, removed_htlc_failure_attribution_data, optional_vec), (57, holding_cell_failure_attribution_data, optional_vec), + (59, negotiated_minimum_depth, option), // Added in 0.2 }); let holder_signer = signer_provider.derive_channel_signer(channel_keys_id); @@ -11342,6 +11355,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel funding_tx_confirmed_in, funding_tx_confirmation_height, short_channel_id, + minimum_depth, }, pending_funding: pending_funding.unwrap(), context: ChannelContext { @@ -11414,7 +11428,8 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel counterparty_htlc_minimum_msat, holder_htlc_minimum_msat, counterparty_max_accepted_htlcs, - minimum_depth, + // TODO: But what if minimum_depth (TLV 1) had been overridden with COINBASE_MATURITY? + negotiated_minimum_depth: negotiated_minimum_depth.or(minimum_depth), counterparty_forwarding_info, diff --git a/lightning/src/ln/channel_state.rs b/lightning/src/ln/channel_state.rs index 99e2833f644..38c08211d0d 100644 --- a/lightning/src/ln/channel_state.rs +++ b/lightning/src/ln/channel_state.rs @@ -531,7 +531,7 @@ impl ChannelDetails { next_outbound_htlc_limit_msat: balance.next_outbound_htlc_limit_msat, next_outbound_htlc_minimum_msat: balance.next_outbound_htlc_minimum_msat, user_channel_id: context.get_user_id(), - confirmations_required: context.minimum_depth(), + confirmations_required: funding.minimum_depth(), confirmations: Some(funding.get_funding_tx_confirmations(best_block_height)), force_close_spend_delay: funding.get_counterparty_selected_contest_delay(), is_outbound: funding.is_outbound(), diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b2529993fed..053ee5fbac3 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3026,7 +3026,7 @@ macro_rules! locked_close_channel { // into the map (which prevents the `PeerState` from being cleaned up) for channels that // never even got confirmations (which would open us up to DoS attacks). let update_id = $channel_context.get_latest_monitor_update_id(); - if $channel_funding.get_funding_tx_confirmation_height().is_some() || $channel_context.minimum_depth() == Some(0) || update_id > 1 { + if $channel_funding.get_funding_tx_confirmation_height().is_some() || $channel_funding.minimum_depth() == Some(0) || update_id > 1 { let chan_id = $channel_context.channel_id(); $peer_state.closed_channel_monitor_update_ids.insert(chan_id, update_id); } @@ -7947,7 +7947,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if accept_0conf { // This should have been correctly configured by the call to Inbound(V1/V2)Channel::new. - debug_assert!(channel.context().minimum_depth().unwrap() == 0); + debug_assert!(channel.funding().minimum_depth().unwrap() == 0); } else if channel.funding().get_channel_type().requires_zero_conf() { let send_msg_err_event = MessageSendEvent::HandleError { node_id: channel.context().get_counterparty_node_id(), @@ -8023,7 +8023,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ Some(funded_chan) => { // This covers non-zero-conf inbound `Channel`s that we are currently monitoring, but those // which have not yet had any confirmations on-chain. - if !funded_chan.funding.is_outbound() && funded_chan.context.minimum_depth().unwrap_or(1) != 0 && + if !funded_chan.funding.is_outbound() && funded_chan.funding.minimum_depth().unwrap_or(1) != 0 && funded_chan.funding.get_funding_tx_confirmations(best_block_height) == 0 { num_unfunded_channels += 1; @@ -8036,7 +8036,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } // 0conf channels are not considered unfunded. - if chan.context().minimum_depth().unwrap_or(1) == 0 { + if chan.funding().minimum_depth().unwrap_or(1) == 0 { continue; } From 6bbbce8750f66837ebdfc1a2909d06bee9f87883 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 17 Apr 2025 10:51:56 -0500 Subject: [PATCH 6/8] Refactor funding tx confirmation check into helper When checking if channel_ready should be sent, the funding transaction must reach minimum_depth confirmations. The same logic is needed for splicing a channel, so refactor it into a helper method. --- lightning/src/ln/channel.rs | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1b8724c3ecc..936467ef99d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4864,6 +4864,23 @@ impl ChannelContext where SP::Target: SignerProvider { self.counterparty_cur_commitment_point = Some(counterparty_cur_commitment_point_override); self.get_initial_counterparty_commitment_signature(funding, logger) } + + fn check_funding_confirmations(&self, funding: &mut FundingScope, height: u32) -> bool { + if funding.funding_tx_confirmation_height == 0 && funding.minimum_depth != Some(0) { + return false; + } + + let funding_tx_confirmations = height as i64 - funding.funding_tx_confirmation_height as i64 + 1; + if funding_tx_confirmations <= 0 { + funding.funding_tx_confirmation_height = 0; + } + + if funding_tx_confirmations < funding.minimum_depth.unwrap_or(0) as i64 { + return false; + } + + return true; + } } // Internal utility functions for channels @@ -8094,16 +8111,7 @@ impl FundedChannel where // Called: // * always when a new block/transactions are confirmed with the new height // * when funding is signed with a height of 0 - if self.funding.funding_tx_confirmation_height == 0 && self.funding.minimum_depth != Some(0) { - return None; - } - - let funding_tx_confirmations = height as i64 - self.funding.funding_tx_confirmation_height as i64 + 1; - if funding_tx_confirmations <= 0 { - self.funding.funding_tx_confirmation_height = 0; - } - - if funding_tx_confirmations < self.funding.minimum_depth.unwrap_or(0) as i64 { + if !self.context.check_funding_confirmations(&mut self.funding, height) { return None; } From 71e27a443ccd4bd50186f871a918a2385cb53fda Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 9 Apr 2025 18:24:48 -0500 Subject: [PATCH 7/8] WIP: Check all funding transactions --- lightning/src/ln/channel.rs | 279 ++++++++++++++++++++++------- lightning/src/ln/channelmanager.rs | 9 + 2 files changed, 224 insertions(+), 64 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 936467ef99d..46ef70c40ca 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1864,6 +1864,8 @@ impl FundingScope { #[cfg(splicing)] struct PendingSplice { pub our_funding_contribution: i64, + sent_funding_txid: Option, + received_funding_txid: Option, } /// Contains everything about the channel including state, and various flags. @@ -4865,6 +4867,40 @@ impl ChannelContext where SP::Target: SignerProvider { self.get_initial_counterparty_commitment_signature(funding, logger) } + #[cfg(splicing)] + fn check_get_splice_locked( + &mut self, pending_splice: &PendingSplice, funding: &mut FundingScope, height: u32, + logger: &L, + ) -> Option + where + L::Target: Logger, + { + if !self.check_funding_confirmations(funding, height) { + return None; + } + + let confirmed_funding_txid = match funding.get_funding_txo().map(|txo| txo.txid) { + Some(funding_txid) => funding_txid, + None => { + debug_assert!(false); + return None; + }, + }; + + match pending_splice.sent_funding_txid { + Some(sent_funding_txid) if confirmed_funding_txid == sent_funding_txid => { + debug_assert!(false); + None + }, + _ => { + Some(msgs::SpliceLocked { + channel_id: self.channel_id(), + splice_txid: confirmed_funding_txid, + }) + }, + } + } + fn check_funding_confirmations(&self, funding: &mut FundingScope, height: u32) -> bool { if funding.funding_tx_confirmation_height == 0 && funding.minimum_depth != Some(0) { return false; @@ -4881,6 +4917,78 @@ impl ChannelContext where SP::Target: SignerProvider { return true; } + + fn check_for_funding_tx<'a, L: Deref>( + &mut self, funding: &mut FundingScope, block_hash: &BlockHash, height: u32, + txdata: &'a TransactionData, logger: &L, + ) -> Result, ClosureReason> + where + L::Target: Logger + { + let funding_txo = match funding.get_funding_txo() { + Some(funding_txo) => funding_txo, + None => { + debug_assert!(false); + return Ok(None); + }, + }; + + let mut confirmed_funding_tx = None; + for &(index_in_block, tx) in txdata.iter() { + // Check if the transaction is the expected funding transaction, and if it is, + // check that it pays the right amount to the right script. + if funding.funding_tx_confirmation_height == 0 { + if tx.compute_txid() == funding_txo.txid { + let txo_idx = funding_txo.index as usize; + if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != funding.get_funding_redeemscript().to_p2wsh() || + tx.output[txo_idx].value.to_sat() != funding.get_value_satoshis() { + if funding.is_outbound() { + // If we generated the funding transaction and it doesn't match what it + // should, the client is really broken and we should just panic and + // tell them off. That said, because hash collisions happen with high + // probability in fuzzing mode, if we're fuzzing we just close the + // channel and move on. + #[cfg(not(fuzzing))] + panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!"); + } + self.update_time_counter += 1; + let err_reason = "funding tx had wrong script/value or output index"; + return Err(ClosureReason::ProcessingError { err: err_reason.to_owned() }); + } else { + if funding.is_outbound() { + if !tx.is_coinbase() { + for input in tx.input.iter() { + if input.witness.is_empty() { + // We generated a malleable funding transaction, implying we've + // just exposed ourselves to funds loss to our counterparty. + #[cfg(not(fuzzing))] + panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!"); + } + } + } + } + + funding.funding_tx_confirmation_height = height; + funding.funding_tx_confirmed_in = Some(*block_hash); + funding.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) { + Ok(scid) => Some(scid), + Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"), + }; + } + + confirmed_funding_tx = Some(tx); + } + } + for inp in tx.input.iter() { + if inp.previous_output == funding_txo.into_bitcoin_outpoint() { + log_info!(logger, "Detected channel-closing tx {} spending {}:{}, closing channel {}", tx.compute_txid(), inp.previous_output.txid, inp.previous_output.vout, &self.channel_id()); + return Err(ClosureReason::CommitmentTxConfirmed); + } + } + } + + Ok(confirmed_funding_tx) + } } // Internal utility functions for channels @@ -5061,6 +5169,16 @@ pub(super) struct FundedChannel where SP::Target: SignerProvider { pending_splice: Option, } +#[cfg(splicing)] +macro_rules! promote_splice_funding { + ($self: expr, $funding: expr) => { + core::mem::swap(&mut $self.funding, $funding); + $self.pending_splice = None; + $self.pending_funding.clear(); + $self.context.announcement_sigs_state = AnnouncementSigsState::NotSent; + } +} + #[cfg(any(test, fuzzing))] struct CommitmentTxInfoCached { fee: u64, @@ -8191,75 +8309,69 @@ impl FundedChannel where NS::Target: NodeSigner, L::Target: Logger { - let mut msgs = (None, None); - if let Some(funding_txo) = self.funding.get_funding_txo() { - for &(index_in_block, tx) in txdata.iter() { - // Check if the transaction is the expected funding transaction, and if it is, - // check that it pays the right amount to the right script. - if self.funding.funding_tx_confirmation_height == 0 { - if tx.compute_txid() == funding_txo.txid { - let txo_idx = funding_txo.index as usize; - if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != self.funding.get_funding_redeemscript().to_p2wsh() || - tx.output[txo_idx].value.to_sat() != self.funding.get_value_satoshis() { - if self.funding.is_outbound() { - // If we generated the funding transaction and it doesn't match what it - // should, the client is really broken and we should just panic and - // tell them off. That said, because hash collisions happen with high - // probability in fuzzing mode, if we're fuzzing we just close the - // channel and move on. - #[cfg(not(fuzzing))] - panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!"); - } - self.context.update_time_counter += 1; - let err_reason = "funding tx had wrong script/value or output index"; - return Err(ClosureReason::ProcessingError { err: err_reason.to_owned() }); - } else { - if self.funding.is_outbound() { - if !tx.is_coinbase() { - for input in tx.input.iter() { - if input.witness.is_empty() { - // We generated a malleable funding transaction, implying we've - // just exposed ourselves to funds loss to our counterparty. - #[cfg(not(fuzzing))] - panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!"); - } - } - } - } + // If we allow 1-conf funding, we may need to check for channel_ready or splice_locked here + // and send it immediately instead of waiting for a best_block_updated call (which may have + // already happened for this block). + let confirmed_funding_tx = self.context.check_for_funding_tx( + &mut self.funding, block_hash, height, txdata, logger, + )?; - self.funding.funding_tx_confirmation_height = height; - self.funding.funding_tx_confirmed_in = Some(*block_hash); - self.funding.short_channel_id = match scid_from_parts(height as u64, index_in_block as u64, txo_idx as u64) { - Ok(scid) => Some(scid), - Err(_) => panic!("Block was bogus - either height was > 16 million, had > 16 million transactions, or had > 65k outputs"), - } - } - // If this is a coinbase transaction and not a 0-conf channel - // we should update our min_depth to 100 to handle coinbase maturity - if tx.is_coinbase() && - self.funding.minimum_depth.unwrap_or(0) > 0 && - self.funding.minimum_depth.unwrap_or(0) < COINBASE_MATURITY { - self.funding.minimum_depth = Some(COINBASE_MATURITY); - } - } - // If we allow 1-conf funding, we may need to check for channel_ready here and - // send it immediately instead of waiting for a best_block_updated call (which - // may have already happened for this block). - if let Some(channel_ready) = self.check_get_channel_ready(height, logger) { - log_info!(logger, "Sending a channel_ready to our peer for channel {}", &self.context.channel_id); - let announcement_sigs = self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger); - msgs = (Some(FundingConfirmedMessage::Establishment(channel_ready)), announcement_sigs); - } + if let Some(funding_tx) = confirmed_funding_tx { + // If this is a coinbase transaction and not a 0-conf channel + // we should update our min_depth to 100 to handle coinbase maturity + if funding_tx.is_coinbase() && + self.funding.minimum_depth.unwrap_or(0) > 0 && + self.funding.minimum_depth.unwrap_or(0) < COINBASE_MATURITY { + self.funding.minimum_depth = Some(COINBASE_MATURITY); + } + + if let Some(channel_ready) = self.check_get_channel_ready(height, logger) { + log_info!(logger, "Sending a channel_ready to our peer for channel {}", &self.context.channel_id); + let announcement_sigs = self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger); + return Ok((Some(FundingConfirmedMessage::Establishment(channel_ready)), announcement_sigs)); + } + } + + #[cfg(splicing)] + let mut confirmed_funding = None; + #[cfg(splicing)] + for funding in self.pending_funding.iter_mut() { + if self.context.check_for_funding_tx(funding, block_hash, height, txdata, logger)?.is_some() { + if confirmed_funding.is_some() { + let err_reason = "splice tx of another pending funding already confirmed"; + return Err(ClosureReason::ProcessingError { err: err_reason.to_owned() }); } - for inp in tx.input.iter() { - if inp.previous_output == funding_txo.into_bitcoin_outpoint() { - log_info!(logger, "Detected channel-closing tx {} spending {}:{}, closing channel {}", tx.compute_txid(), inp.previous_output.txid, inp.previous_output.vout, &self.context.channel_id()); - return Err(ClosureReason::CommitmentTxConfirmed); - } + + confirmed_funding = Some(funding); + } + } + + #[cfg(splicing)] + if let Some(funding) = confirmed_funding { + let pending_splice = match self.pending_splice.as_mut() { + Some(pending_splice) => pending_splice, + None => { + // TODO: Move pending_funding into pending_splice? + debug_assert!(false); + // TODO: Error instead? + return Ok((None, None)); + }, + }; + + if let Some(splice_locked) = self.context.check_get_splice_locked(pending_splice, funding, height, logger) { + log_info!(logger, "Sending a splice_locked to our peer for channel {}", &self.context.channel_id); + + let mut announcement_sigs = None; + if Some(splice_locked.splice_txid) == pending_splice.received_funding_txid { + promote_splice_funding!(self, funding); + announcement_sigs = self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger); } + + return Ok((Some(FundingConfirmedMessage::Splice(splice_locked)), announcement_sigs)); } } - Ok(msgs) + + Ok((None, None)) } /// When a new block is connected, we check the height of the block against outbound holding @@ -8352,6 +8464,43 @@ impl FundedChannel where return Err(ClosureReason::FundingTimedOut); } + #[cfg(splicing)] + let mut confirmed_funding = None; + #[cfg(splicing)] + for funding in self.pending_funding.iter_mut() { + if confirmed_funding.is_some() { + let err_reason = "splice tx of another pending funding already confirmed"; + return Err(ClosureReason::ProcessingError { err: err_reason.to_owned() }); + } + + confirmed_funding = Some(funding); + } + + #[cfg(splicing)] + if let Some(funding) = confirmed_funding { + let pending_splice = match self.pending_splice.as_mut() { + Some(pending_splice) => pending_splice, + None => { + // TODO: Move pending_funding into pending_splice? + debug_assert!(false); + // TODO: Error instead? + return Ok((None, timed_out_htlcs, None)); + }, + }; + + if let Some(splice_locked) = self.context.check_get_splice_locked(pending_splice, funding, height, logger) { + let mut announcement_sigs = None; + if Some(splice_locked.splice_txid) == pending_splice.received_funding_txid { + promote_splice_funding!(self, funding); + if let Some((chain_hash, node_signer, user_config)) = chain_node_signer { + announcement_sigs = self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger); + } + } + log_info!(logger, "Sending a splice_locked to our peer for channel {}", &self.context.channel_id); + return Ok((Some(FundingConfirmedMessage::Splice(splice_locked)), timed_out_htlcs, announcement_sigs)); + } + } + let announcement_sigs = if let Some((chain_hash, node_signer, user_config)) = chain_node_signer { self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger) } else { None }; @@ -8683,6 +8832,8 @@ impl FundedChannel where self.pending_splice = Some(PendingSplice { our_funding_contribution: our_funding_contribution_satoshis, + sent_funding_txid: None, + received_funding_txid: None, }); let msg = self.get_splice_init(our_funding_contribution_satoshis, funding_feerate_per_kw, locktime); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 053ee5fbac3..46a68e3f533 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -11664,6 +11664,8 @@ where pub(super) enum FundingConfirmedMessage { Establishment(msgs::ChannelReady), + #[cfg(splicing)] + Splice(msgs::SpliceLocked), } impl ChannelManager @@ -11725,6 +11727,13 @@ where log_trace!(logger, "Sending channel_ready WITHOUT channel_update for {}", funded_channel.context.channel_id()); } }, + #[cfg(splicing)] + Some(FundingConfirmedMessage::Splice(splice_locked)) => { + pending_msg_events.push(MessageSendEvent::SendSpliceLocked { + node_id: funded_channel.context.get_counterparty_node_id(), + msg: splice_locked, + }); + }, None => {}, } From e4c056652b28c2151c4e4aabaf5390953c1aaa2e Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 18 Apr 2025 17:27:13 -0500 Subject: [PATCH 8/8] Handle splice_locked message --- lightning/src/ln/channel.rs | 35 +++++++++++++++++++ lightning/src/ln/channelmanager.rs | 55 ++++++++++++++++++++++++++++-- 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 46ef70c40ca..e2c4aeeb477 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -8922,6 +8922,41 @@ impl FundedChannel where Ok(()) } + #[cfg(splicing)] + pub fn splice_locked( + &mut self, msg: &msgs::SpliceLocked, node_signer: &NS, chain_hash: ChainHash, + user_config: &UserConfig, best_block: &BestBlock, logger: &L, + ) -> Result, ChannelError> + where + NS::Target: NodeSigner, + L::Target: Logger + { + let pending_splice = match self.pending_splice.as_mut() { + Some(pending_splice) => pending_splice, + None => { + return Err(ChannelError::Warn(format!("Channel is not in pending splice"))); + }, + }; + + if let Some(sent_funding_txid) = pending_splice.sent_funding_txid { + if sent_funding_txid == msg.splice_txid { + if let Some(funding) = self.pending_funding + .iter_mut() + .find(|funding| funding.get_funding_txo().map(|txo| txo.txid) == Some(sent_funding_txid)) + { + promote_splice_funding!(self, funding); + return Ok(self.get_announcement_sigs(node_signer, chain_hash, user_config, best_block.height, logger)); + } + + // TODO: Close channel? + return Ok(None); + } + } + + pending_splice.received_funding_txid = Some(msg.splice_txid); + Ok(None) + } + // Send stuff to our remote peers: /// Queues up an outbound HTLC to send by placing it in the holding cell. You should call diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 46a68e3f533..3c57508ca25 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9603,6 +9603,48 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ Err(MsgHandleErrInternal::send_err_msg_no_close("TODO(splicing): Splicing is not implemented (splice_ack)".to_owned(), msg.channel_id)) } + #[cfg(splicing)] + fn internal_splice_locked(&self, counterparty_node_id: &PublicKey, msg: &msgs::SpliceLocked) -> Result<(), MsgHandleErrInternal> { + let per_peer_state = self.per_peer_state.read().unwrap(); + let peer_state_mutex = per_peer_state.get(counterparty_node_id) + .ok_or_else(|| { + debug_assert!(false); + MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id) + })?; + let mut peer_state_lock = peer_state_mutex.lock().unwrap(); + let peer_state = &mut *peer_state_lock; + + // Look for the channel + match peer_state.channel_by_id.entry(msg.channel_id) { + hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!( + "Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", + counterparty_node_id + ), msg.channel_id)), + hash_map::Entry::Occupied(mut chan_entry) => { + if let Some(chan) = chan_entry.get_mut().as_funded_mut() { + let logger = WithChannelContext::from(&self.logger, &chan.context, None); + let announcement_sigs_opt = try_channel_entry!( + self, peer_state, chan.splice_locked( + msg, &self.node_signer, self.chain_hash, &self.default_configuration, + &self.best_block.read().unwrap(), &&logger, + ), chan_entry + ); + if let Some(announcement_sigs) = announcement_sigs_opt { + log_trace!(logger, "Sending announcement_signatures for channel {}", chan.context.channel_id()); + peer_state.pending_msg_events.push(MessageSendEvent::SendAnnouncementSignatures { + node_id: counterparty_node_id.clone(), + msg: announcement_sigs, + }); + } + } else { + return Err(MsgHandleErrInternal::send_err_msg_no_close("Channel is not funded, cannot splice".to_owned(), msg.channel_id)); + } + }, + }; + + Err(MsgHandleErrInternal::send_err_msg_no_close("TODO(splicing): Splicing is not implemented (splice_locked)".to_owned(), msg.channel_id)) + } + /// Process pending events from the [`chain::Watch`], returning whether any events were processed. fn process_pending_monitor_events(&self) -> bool { debug_assert!(self.total_consistency_lock.try_write().is_err()); // Caller holds read lock @@ -12111,9 +12153,16 @@ where #[cfg(splicing)] fn handle_splice_locked(&self, counterparty_node_id: PublicKey, msg: &msgs::SpliceLocked) { - let _: Result<(), _> = handle_error!(self, Err(MsgHandleErrInternal::send_err_msg_no_close( - "Splicing not supported (splice_locked)".to_owned(), - msg.channel_id)), counterparty_node_id); + let _persistence_guard = PersistenceNotifierGuard::optionally_notify(self, || { + let res = self.internal_splice_locked(&counterparty_node_id, msg); + let persist = match &res { + Err(e) if e.closes_channel() => NotifyOption::DoPersist, + Err(_) => NotifyOption::SkipPersistHandleEvents, + Ok(()) => NotifyOption::SkipPersistHandleEvents, + }; + let _ = handle_error!(self, res, counterparty_node_id); + persist + }); } fn handle_shutdown(&self, counterparty_node_id: PublicKey, msg: &msgs::Shutdown) {