From ac090a4936fa0116483b7c52ef4c16ab5143884f Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Jun 2025 14:46:25 -0500 Subject: [PATCH 1/5] Don't set sent_funding_txid in maybe_promote_splice_funding When sending splice_locked, set PendingSplice::sent_funding_txid prior to calling FundedChannel::maybe_promote_splice_funding. This makes the latter idempotent when the splice is not promoted. Otherwise, successive calls could override the previously set sent_funding_txid. --- lightning/src/ln/channel.rs | 128 +++++++++++++++++++++--------------- 1 file changed, 74 insertions(+), 54 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index fb58b51d4dc..f8d6b8c3451 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2156,6 +2156,41 @@ struct PendingSplice { received_funding_txid: Option, } +#[cfg(splicing)] +impl PendingSplice { + fn check_get_splice_locked( + &mut self, context: &ChannelContext, funding: &FundingScope, height: u32, + ) -> Option + where + SP::Target: SignerProvider, + { + if !context.check_funding_meets_minimum_depth(funding, height) { + return None; + } + + let confirmed_funding_txid = match funding.get_funding_txid() { + Some(funding_txid) => funding_txid, + None => { + debug_assert!(false); + return None; + }, + }; + + match self.sent_funding_txid { + Some(sent_funding_txid) if confirmed_funding_txid == sent_funding_txid => None, + _ => { + let splice_locked = msgs::SpliceLocked { + channel_id: context.channel_id(), + splice_txid: confirmed_funding_txid, + }; + self.sent_funding_txid = Some(splice_locked.splice_txid); + Some(splice_locked) + }, + } + } + +} + /// Wrapper around a [`Transaction`] useful for caching the result of [`Transaction::compute_txid`]. struct ConfirmedTransaction<'a> { tx: &'a Transaction, @@ -5525,6 +5560,29 @@ where self.get_initial_counterparty_commitment_signature(funding, logger) } + fn check_funding_meets_minimum_depth(&self, funding: &FundingScope, height: u32) -> bool { + let minimum_depth = self + .minimum_depth(funding) + .expect("ChannelContext::minimum_depth should be set for FundedChannel"); + + // Zero-conf channels always meet the minimum depth. + if minimum_depth == 0 { + return true; + } + + if funding.funding_tx_confirmation_height == 0 { + return false; + } + + let funding_tx_confirmations = + height as i64 - funding.funding_tx_confirmation_height as i64 + 1; + if funding_tx_confirmations < minimum_depth as i64 { + return false; + } + + return true; + } + #[rustfmt::skip] fn check_for_funding_tx_confirmed( &mut self, funding: &mut FundingScope, block_hash: &BlockHash, height: u32, @@ -9074,58 +9132,13 @@ where } } - #[cfg(splicing)] - fn check_get_splice_locked( - &self, pending_splice: &PendingSplice, funding: &FundingScope, height: u32, - ) -> Option { - if !self.check_funding_meets_minimum_depth(funding, height) { - return None; - } - - let confirmed_funding_txid = match funding.get_funding_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 => None, - _ => Some(msgs::SpliceLocked { - channel_id: self.context.channel_id(), - splice_txid: confirmed_funding_txid, - }), - } - } - fn check_funding_meets_minimum_depth(&self, funding: &FundingScope, height: u32) -> bool { - let minimum_depth = self - .context - .minimum_depth(funding) - .expect("ChannelContext::minimum_depth should be set for FundedChannel"); - - // Zero-conf channels always meet the minimum depth. - if minimum_depth == 0 { - return true; - } - - if funding.funding_tx_confirmation_height == 0 { - return false; - } - - let funding_tx_confirmations = - height as i64 - funding.funding_tx_confirmation_height as i64 + 1; - if funding_tx_confirmations < minimum_depth as i64 { - return false; - } - - return true; + self.context.check_funding_meets_minimum_depth(funding, height) } #[cfg(splicing)] fn maybe_promote_splice_funding( - &mut self, splice_txid: Txid, confirmed_funding_index: usize, logger: &L, + &mut self, confirmed_funding_index: usize, logger: &L, ) -> bool where L::Target: Logger, @@ -9134,7 +9147,13 @@ where debug_assert!(confirmed_funding_index < self.pending_funding.len()); let pending_splice = self.pending_splice.as_mut().unwrap(); - pending_splice.sent_funding_txid = Some(splice_txid); + let splice_txid = match pending_splice.sent_funding_txid { + Some(sent_funding_txid) => sent_funding_txid, + None => { + debug_assert!(false); + return false; + }, + }; if pending_splice.sent_funding_txid == pending_splice.received_funding_txid { log_info!( @@ -9145,6 +9164,7 @@ where ); let funding = self.pending_funding.get_mut(confirmed_funding_index).unwrap(); + debug_assert_eq!(Some(splice_txid), funding.get_funding_txid()); promote_splice_funding!(self, funding); return true; @@ -9234,7 +9254,7 @@ where #[cfg(splicing)] if let Some(confirmed_funding_index) = confirmed_funding_index { - let pending_splice = match self.pending_splice.as_ref() { + let pending_splice = match self.pending_splice.as_mut() { Some(pending_splice) => pending_splice, None => { // TODO: Move pending_funding into pending_splice @@ -9245,7 +9265,7 @@ where }; let funding = self.pending_funding.get(confirmed_funding_index).unwrap(); - if let Some(splice_locked) = self.check_get_splice_locked(pending_splice, funding, height) { + if let Some(splice_locked) = pending_splice.check_get_splice_locked(&self.context, funding, height) { for &(idx, tx) in txdata.iter() { if idx > index_in_block { self.context.check_for_funding_tx_spent(funding, tx, logger)?; @@ -9260,7 +9280,7 @@ where ); let announcement_sigs = self - .maybe_promote_splice_funding(splice_locked.splice_txid, confirmed_funding_index, logger) + .maybe_promote_splice_funding(confirmed_funding_index, logger) .then(|| self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger)) .flatten(); @@ -9417,13 +9437,13 @@ where } } - let pending_splice = self.pending_splice.as_ref().unwrap(); + let pending_splice = self.pending_splice.as_mut().unwrap(); let funding = self.pending_funding.get(confirmed_funding_index).unwrap(); - if let Some(splice_locked) = self.check_get_splice_locked(pending_splice, funding, height) { + if let Some(splice_locked) = pending_splice.check_get_splice_locked(&self.context, funding, height) { log_info!(logger, "Sending a splice_locked to our peer for channel {}", &self.context.channel_id); let announcement_sigs = self - .maybe_promote_splice_funding(splice_locked.splice_txid, confirmed_funding_index, logger) + .maybe_promote_splice_funding(confirmed_funding_index, logger) .then(|| chain_node_signer .and_then(|(chain_hash, node_signer, user_config)| self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger) From eeee50edf412d4b3666d034fe8611bc47461c3d8 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Jun 2025 16:22:58 -0500 Subject: [PATCH 2/5] Don't inline call to FundedChannel::splice_locked This will help make the code more compact when using rustfmt. --- lightning/src/ln/channelmanager.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 656135daf84..2e4908641da 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -10155,12 +10155,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ 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 + let result = chan.splice_locked( + msg, &self.node_signer, self.chain_hash, &self.default_configuration, + &self.best_block.read().unwrap(), &&logger, ); + let announcement_sigs_opt = try_channel_entry!(self, peer_state, result, chan_entry); if !chan.has_pending_splice() { let mut short_to_chan_info = self.short_to_chan_info.write().unwrap(); From 5170373a6ad4f79a8905c535ff5d48d4355fb93c Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Jun 2025 16:29:27 -0500 Subject: [PATCH 3/5] rustfmt ChannelManager::internal_splice_locked Use of a macro as a function parameter prevented this method from being formatted by rustfmt. Extract out a variable such is now can be. --- lightning/src/ln/channelmanager.rs | 61 ++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 19 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2e4908641da..a8908c6cedf 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -10148,42 +10148,65 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ // 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!( + hash_map::Entry::Vacant(_) => { + let err = 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)), + counterparty_node_id, + ); + return Err(MsgHandleErrInternal::send_err_msg_no_close(err, 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 result = chan.splice_locked( - msg, &self.node_signer, self.chain_hash, &self.default_configuration, - &self.best_block.read().unwrap(), &&logger, + msg, + &self.node_signer, + self.chain_hash, + &self.default_configuration, + &self.best_block.read().unwrap(), + &&logger, ); - let announcement_sigs_opt = try_channel_entry!(self, peer_state, result, chan_entry); + let announcement_sigs_opt = + try_channel_entry!(self, peer_state, result, chan_entry); if !chan.has_pending_splice() { let mut short_to_chan_info = self.short_to_chan_info.write().unwrap(); insert_short_channel_id!(short_to_chan_info, chan); let mut pending_events = self.pending_events.lock().unwrap(); - pending_events.push_back((events::Event::ChannelReady { - channel_id: chan.context.channel_id(), - user_channel_id: chan.context.get_user_id(), - counterparty_node_id: chan.context.get_counterparty_node_id(), - funding_txo: chan.funding.get_funding_txo().map(|outpoint| outpoint.into_bitcoin_outpoint()), - channel_type: chan.funding.get_channel_type().clone(), - }, None)); + pending_events.push_back(( + events::Event::ChannelReady { + channel_id: chan.context.channel_id(), + user_channel_id: chan.context.get_user_id(), + counterparty_node_id: chan.context.get_counterparty_node_id(), + funding_txo: chan + .funding + .get_funding_txo() + .map(|outpoint| outpoint.into_bitcoin_outpoint()), + channel_type: chan.funding.get_channel_type().clone(), + }, + None, + )); } 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, - }); + 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)); + return Err(MsgHandleErrInternal::send_err_msg_no_close( + "Channel is not funded, cannot splice".to_owned(), + msg.channel_id, + )); } }, }; From 22b578b0ffecd9c6deae7e4d1fc59dd3c615a1ae Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Jun 2025 17:32:48 -0500 Subject: [PATCH 4/5] Return new funding_txo with splice_locked When sending or receiving splice_locked results in promoting a FundingScope, return the new funding_txo to ChannelManager. This is used to determine if Event::ChannelReady should be emitted. This is deemed safer than checking the channel if there are any pending splices after it handles splice_locked or after checking funding confirmations. --- lightning/src/ln/channel.rs | 43 +++++++++++++++++++----------- lightning/src/ln/channelmanager.rs | 16 +++++------ 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f8d6b8c3451..06155b65386 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2188,7 +2188,6 @@ impl PendingSplice { }, } } - } /// Wrapper around a [`Transaction`] useful for caching the result of [`Transaction::compute_txid`]. @@ -9279,12 +9278,18 @@ where &self.context.channel_id, ); - let announcement_sigs = self - .maybe_promote_splice_funding(confirmed_funding_index, logger) + let funding_promoted = + self.maybe_promote_splice_funding(confirmed_funding_index, logger); + let funding_txo = funding_promoted.then(|| { + self.funding + .get_funding_txo() + .expect("Splice FundingScope should always have a funding_txo") + }); + let announcement_sigs = funding_promoted .then(|| self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger)) .flatten(); - return Ok((Some(FundingConfirmedMessage::Splice(splice_locked)), announcement_sigs)); + return Ok((Some(FundingConfirmedMessage::Splice(splice_locked, funding_txo)), announcement_sigs)); } } @@ -9442,8 +9447,14 @@ where if let Some(splice_locked) = pending_splice.check_get_splice_locked(&self.context, funding, height) { log_info!(logger, "Sending a splice_locked to our peer for channel {}", &self.context.channel_id); - let announcement_sigs = self - .maybe_promote_splice_funding(confirmed_funding_index, logger) + let funding_promoted = + self.maybe_promote_splice_funding(confirmed_funding_index, logger); + let funding_txo = funding_promoted.then(|| { + self.funding + .get_funding_txo() + .expect("Splice FundingScope should always have a funding_txo") + }); + let announcement_sigs = funding_promoted .then(|| chain_node_signer .and_then(|(chain_hash, node_signer, user_config)| self.get_announcement_sigs(node_signer, chain_hash, user_config, height, logger) @@ -9451,7 +9462,7 @@ where ) .flatten(); - return Ok((Some(FundingConfirmedMessage::Splice(splice_locked)), timed_out_htlcs, announcement_sigs)); + return Ok((Some(FundingConfirmedMessage::Splice(splice_locked, funding_txo)), timed_out_htlcs, announcement_sigs)); } } @@ -9945,7 +9956,7 @@ where 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> + ) -> Result<(Option, Option), ChannelError> where NS::Target: NodeSigner, L::Target: Logger, @@ -9978,13 +9989,18 @@ where &self.context.channel_id, ); promote_splice_funding!(self, funding); - return Ok(self.get_announcement_sigs( + let funding_txo = self + .funding + .get_funding_txo() + .expect("Splice FundingScope should always have a funding_txo"); + let announcement_sigs = self.get_announcement_sigs( node_signer, chain_hash, user_config, best_block.height, logger, - )); + ); + return Ok((Some(funding_txo), announcement_sigs)); } let err = "unknown splice funding txid"; @@ -10008,7 +10024,7 @@ where } pending_splice.received_funding_txid = Some(msg.splice_txid); - Ok(None) + Ok((None, None)) } // Send stuff to our remote peers: @@ -10735,11 +10751,6 @@ where } } - #[cfg(splicing)] - pub fn has_pending_splice(&self) -> bool { - self.pending_splice.is_some() - } - pub fn remove_legacy_scids_before_block(&mut self, height: u32) -> alloc::vec::Drain { let end = self .funding diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a8908c6cedf..861cb2af80d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -10166,10 +10166,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self.best_block.read().unwrap(), &&logger, ); - let announcement_sigs_opt = + let (funding_txo, announcement_sigs_opt) = try_channel_entry!(self, peer_state, result, chan_entry); - if !chan.has_pending_splice() { + if funding_txo.is_some() { let mut short_to_chan_info = self.short_to_chan_info.write().unwrap(); insert_short_channel_id!(short_to_chan_info, chan); @@ -10179,9 +10179,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ channel_id: chan.context.channel_id(), user_channel_id: chan.context.get_user_id(), counterparty_node_id: chan.context.get_counterparty_node_id(), - funding_txo: chan - .funding - .get_funding_txo() + funding_txo: funding_txo .map(|outpoint| outpoint.into_bitcoin_outpoint()), channel_type: chan.funding.get_channel_type().clone(), }, @@ -12224,7 +12222,7 @@ where pub(super) enum FundingConfirmedMessage { Establishment(msgs::ChannelReady), #[cfg(splicing)] - Splice(msgs::SpliceLocked), + Splice(msgs::SpliceLocked, Option), } impl< @@ -12298,8 +12296,8 @@ where } }, #[cfg(splicing)] - Some(FundingConfirmedMessage::Splice(splice_locked)) => { - if !funded_channel.has_pending_splice() { + Some(FundingConfirmedMessage::Splice(splice_locked, funding_txo)) => { + if funding_txo.is_some() { let mut short_to_chan_info = self.short_to_chan_info.write().unwrap(); insert_short_channel_id!(short_to_chan_info, funded_channel); @@ -12308,7 +12306,7 @@ where channel_id: funded_channel.context.channel_id(), user_channel_id: funded_channel.context.get_user_id(), counterparty_node_id: funded_channel.context.get_counterparty_node_id(), - funding_txo: funded_channel.funding.get_funding_txo().map(|outpoint| outpoint.into_bitcoin_outpoint()), + funding_txo: funding_txo.map(|outpoint| outpoint.into_bitcoin_outpoint()), channel_type: funded_channel.funding.get_channel_type().clone(), }, None)); } From db69ec7d4924d5684df7e50dae595f42b0bc8dcf Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Jun 2025 17:35:55 -0500 Subject: [PATCH 5/5] Increase CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY The spec is being changed to keep around a channel_announcement when the funding_txo is spent. This allows spliced channels more time to exchange splice_locked messages before the channel is dropped from the network graph. While LDK does not drop such channels, it uses this constant to allow forwarding HTLCs over the channel using SCIDs from previous funding transactions. Here, the increase from 12 to 144 reflects double the spec change of 72 blocks. --- lightning/src/ln/channel.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 06155b65386..01fa73a6db5 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1424,7 +1424,7 @@ pub(crate) const COINBASE_MATURITY: u32 = 100; /// The number of blocks to wait for a channel_announcement to propagate such that payments using an /// older SCID can still be relayed. Once the spend of the previous funding transaction has reached /// this number of confirmations, the corresponding SCID will be forgotten. -const CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY: u32 = 12; +const CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY: u32 = 144; struct PendingChannelMonitorUpdate { update: ChannelMonitorUpdate,