From bf549289c35daa3a9ef6a685c31b0b183c82c6a1 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Sun, 15 Mar 2026 12:51:07 -0500 Subject: [PATCH 1/7] Move feerate parameters from splice_channel/rbf_channel to FundingTemplate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The user doesn't choose the feerate at splice_channel/rbf_channel time — they choose it when performing coin selection. Moving feerate to the FundingTemplate::splice_* methods gives users more control and lets rbf_channel expose the minimum RBF feerate (25/24 of previous) on the template so users can choose an appropriate feerate. splice_channel and rbf_channel no longer take min_feerate/max_feerate. Instead, FundingTemplate gains a min_rbf_feerate() accessor that returns the RBF floor when applicable (from negotiated candidates or in-progress funding negotiations). The feerate parameters move to the splice_in_sync, splice_out_sync, and splice_in_and_out_sync methods (and their async variants), which validate that min_feerate >= min_rbf_feerate before coin selection. Fee estimation documentation moves from splice_channel/rbf_channel to funding_contributed, where the contribution (and its feerate range) is actually provided and the splice process begins. Co-Authored-By: Claude Opus 4.6 (1M context) --- fuzz/src/chanmon_consistency.rs | 50 ++-- fuzz/src/full_stack.rs | 35 +-- lightning/src/ln/channel.rs | 87 ++++--- lightning/src/ln/channelmanager.rs | 132 ++++------- lightning/src/ln/funding.rs | 127 +++++++--- lightning/src/ln/splicing_tests.rs | 362 +++++++++++++++-------------- 6 files changed, 402 insertions(+), 391 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 22006897a0f..5d46cf26031 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -1391,14 +1391,8 @@ pub fn do_test( let splice_channel = |node: &ChanMan, counterparty_node_id: &PublicKey, channel_id: &ChannelId, - f: &dyn Fn(FundingTemplate) -> Result, - funding_feerate_sat_per_kw: FeeRate| { - match node.splice_channel( - channel_id, - counterparty_node_id, - funding_feerate_sat_per_kw, - FeeRate::MAX, - ) { + f: &dyn Fn(FundingTemplate) -> Result| { + match node.splice_channel(channel_id, counterparty_node_id) { Ok(funding_template) => { if let Ok(contribution) = f(funding_template) { let _ = node.funding_contributed( @@ -1425,15 +1419,10 @@ pub fn do_test( channel_id: &ChannelId, wallet: &WalletSync<&TestWalletSource, Arc>, funding_feerate_sat_per_kw: FeeRate| { - splice_channel( - node, - counterparty_node_id, - channel_id, - &move |funding_template: FundingTemplate| { - funding_template.splice_in_sync(Amount::from_sat(10_000), wallet) - }, - funding_feerate_sat_per_kw, - ); + splice_channel(node, counterparty_node_id, channel_id, &move |funding_template: FundingTemplate| { + let feerate = funding_template.min_rbf_feerate().unwrap_or(funding_feerate_sat_per_kw); + funding_template.splice_in_sync(Amount::from_sat(10_000), feerate, FeeRate::MAX, wallet) + }); }; let splice_out = |node: &ChanMan, @@ -1454,19 +1443,20 @@ pub fn do_test( if outbound_capacity_msat < 20_000_000 { return; } - splice_channel( - node, - counterparty_node_id, - channel_id, - &move |funding_template| { - let outputs = vec![TxOut { - value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), - script_pubkey: wallet.get_change_script().unwrap(), - }]; - funding_template.splice_out_sync(outputs, &WalletSync::new(wallet, logger.clone())) - }, - funding_feerate_sat_per_kw, - ); + splice_channel(node, counterparty_node_id, channel_id, &move |funding_template| { + let feerate = + funding_template.min_rbf_feerate().unwrap_or(funding_feerate_sat_per_kw); + let outputs = vec![TxOut { + value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), + script_pubkey: wallet.get_change_script().unwrap(), + }]; + funding_template.splice_out_sync( + outputs, + feerate, + FeeRate::MAX, + &WalletSync::new(wallet, logger.clone()), + ) + }); }; loop { diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 5dfa51079d8..9700390f8ef 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -1032,16 +1032,19 @@ pub fn do_test(mut data: &[u8], logger: &Arc } let chan_id = chan.channel_id; let counterparty = chan.counterparty.node_id; - if let Ok(funding_template) = channelmanager.splice_channel( - &chan_id, - &counterparty, - FeeRate::from_sat_per_kwu(253), - FeeRate::MAX, - ) { + if let Ok(funding_template) = + channelmanager.splice_channel(&chan_id, &counterparty) + { + let feerate = funding_template + .min_rbf_feerate() + .unwrap_or(FeeRate::from_sat_per_kwu(253)); let wallet_sync = WalletSync::new(&wallet, Arc::clone(&logger)); - if let Ok(contribution) = funding_template - .splice_in_sync(Amount::from_sat(splice_in_sats.min(900_000)), &wallet_sync) - { + if let Ok(contribution) = funding_template.splice_in_sync( + Amount::from_sat(splice_in_sats.min(900_000)), + feerate, + FeeRate::MAX, + &wallet_sync, + ) { let _ = channelmanager.funding_contributed( &chan_id, &counterparty, @@ -1073,19 +1076,19 @@ pub fn do_test(mut data: &[u8], logger: &Arc let splice_out_sats = splice_out_sats.min(max_splice_out).max(546); // At least dust limit let chan_id = chan.channel_id; let counterparty = chan.counterparty.node_id; - if let Ok(funding_template) = channelmanager.splice_channel( - &chan_id, - &counterparty, - FeeRate::from_sat_per_kwu(253), - FeeRate::MAX, - ) { + if let Ok(funding_template) = + channelmanager.splice_channel(&chan_id, &counterparty) + { + let feerate = funding_template + .min_rbf_feerate() + .unwrap_or(FeeRate::from_sat_per_kwu(253)); let outputs = vec![TxOut { value: Amount::from_sat(splice_out_sats), script_pubkey: wallet.get_change_script().unwrap(), }]; let wallet_sync = WalletSync::new(&wallet, Arc::clone(&logger)); if let Ok(contribution) = - funding_template.splice_out_sync(outputs, &wallet_sync) + funding_template.splice_out_sync(outputs, feerate, FeeRate::MAX, &wallet_sync) { let _ = channelmanager.funding_contributed( &chan_id, diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 82d7d3bb92f..6f23aa7857f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2973,6 +2973,21 @@ impl FundingNegotiation { } } + fn funding_feerate_sat_per_1000_weight(&self) -> u32 { + match self { + FundingNegotiation::AwaitingAck { context, .. } => { + context.funding_feerate_sat_per_1000_weight + }, + FundingNegotiation::ConstructingTransaction { + funding_feerate_sat_per_1000_weight, + .. + } => *funding_feerate_sat_per_1000_weight, + FundingNegotiation::AwaitingSignatures { + funding_feerate_sat_per_1000_weight, .. + } => *funding_feerate_sat_per_1000_weight, + } + } + fn is_initiator(&self) -> bool { match self { FundingNegotiation::AwaitingAck { context, .. } => context.is_initiator, @@ -11893,9 +11908,7 @@ where } /// Initiate splicing. - pub fn splice_channel( - &self, min_feerate: FeeRate, max_feerate: FeeRate, - ) -> Result { + pub fn splice_channel(&self) -> Result { if self.holder_commitment_point.current_point().is_none() { return Err(APIError::APIMisuseError { err: format!( @@ -11937,16 +11950,19 @@ where }); } - if min_feerate > max_feerate { - return Err(APIError::APIMisuseError { - err: format!( - "Channel {} min_feerate {} exceeds max_feerate {}", - self.context.channel_id(), - min_feerate, - max_feerate, - ), - }); - } + // Compute the RBF feerate floor from either negotiated candidates (via + // can_initiate_rbf) or an in-progress funding negotiation (which will become a + // negotiated candidate once it completes). + let min_rbf_feerate = self.can_initiate_rbf().ok().flatten().or_else(|| { + self.pending_splice + .as_ref() + .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()) + .map(|negotiation| { + let prev_feerate = negotiation.funding_feerate_sat_per_1000_weight(); + let min_feerate_kwu = ((prev_feerate as u64) * 25).div_ceil(24); + FeeRate::from_sat_per_kwu(min_feerate_kwu) + }) + }); let funding_txo = self.funding.get_funding_txo().expect("funding_txo should be set"); let previous_utxo = @@ -11957,13 +11973,11 @@ where satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + FUNDING_TRANSACTION_WITNESS_WEIGHT, }; - Ok(FundingTemplate::new(Some(shared_input), min_feerate, max_feerate)) + Ok(FundingTemplate::new(Some(shared_input), min_rbf_feerate)) } /// Initiate an RBF of a pending splice transaction. - pub fn rbf_channel( - &self, min_feerate: FeeRate, max_feerate: FeeRate, - ) -> Result { + pub fn rbf_channel(&self) -> Result { if self.holder_commitment_point.current_point().is_none() { return Err(APIError::APIMisuseError { err: format!( @@ -12000,18 +12014,8 @@ where }); } - if min_feerate > max_feerate { - return Err(APIError::APIMisuseError { - err: format!( - "Channel {} min_feerate {} exceeds max_feerate {}", - self.context.channel_id(), - min_feerate, - max_feerate, - ), - }); - } - - self.can_initiate_rbf(min_feerate).map_err(|err| APIError::APIMisuseError { err })?; + let min_rbf_feerate = + self.can_initiate_rbf().map_err(|err| APIError::APIMisuseError { err })?; let funding_txo = self.funding.get_funding_txo().expect("funding_txo should be set"); let previous_utxo = @@ -12022,10 +12026,10 @@ where satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + FUNDING_TRANSACTION_WITNESS_WEIGHT, }; - Ok(FundingTemplate::new(Some(shared_input), min_feerate, max_feerate)) + Ok(FundingTemplate::new(Some(shared_input), min_rbf_feerate)) } - fn can_initiate_rbf(&self, feerate: FeeRate) -> Result<(), String> { + fn can_initiate_rbf(&self) -> Result, String> { let pending_splice = match &self.pending_splice { Some(pending_splice) => pending_splice, None => { @@ -12064,20 +12068,13 @@ where )); } - // Check the 25/24 feerate increase rule - let new_feerate = feerate.to_sat_per_kwu() as u32; - if let Some(prev_feerate) = pending_splice.last_funding_feerate_sat_per_1000_weight { - if (new_feerate as u64) * 24 < (prev_feerate as u64) * 25 { - return Err(format!( - "Channel {} RBF feerate {} is less than 25/24 of the previous feerate {}", - self.context.channel_id(), - new_feerate, - prev_feerate, - )); - } - } + let min_rbf_feerate = + pending_splice.last_funding_feerate_sat_per_1000_weight.map(|prev_feerate| { + let min_feerate_kwu = ((prev_feerate as u64) * 25).div_ceil(24); + FeeRate::from_sat_per_kwu(min_feerate_kwu) + }); - Ok(()) + Ok(min_rbf_feerate) } pub fn funding_contributed( @@ -13761,7 +13758,7 @@ where #[allow(irrefutable_let_patterns)] if let QuiescentAction::Splice { contribution, .. } = action { if self.pending_splice.is_some() { - if let Err(msg) = self.can_initiate_rbf(contribution.feerate()) { + if let Err(msg) = self.can_initiate_rbf() { log_given_level!( logger, logger_level, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f8b5ef32fc3..223d74ce780 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -30,7 +30,7 @@ use bitcoin::hashes::{Hash, HashEngine, HmacEngine}; use bitcoin::secp256k1::Secp256k1; use bitcoin::secp256k1::{PublicKey, SecretKey}; -use bitcoin::{secp256k1, FeeRate, Sequence, SignedAmount}; +use bitcoin::{secp256k1, Sequence, SignedAmount}; use crate::blinded_path::message::{ AsyncPaymentsContext, BlindedMessagePath, MessageForwardNode, OffersContext, @@ -4710,52 +4710,18 @@ impl< /// channel (no matter the type) can be spliced, as long as the counterparty is currently /// connected. /// - /// # Arguments - /// - /// The splice initiator is responsible for paying fees for common fields, shared inputs, and - /// shared outputs along with any contributed inputs and outputs. When building a - /// [`FundingContribution`], fees are estimated at `min_feerate` assuming initiator - /// responsibility and must be covered by the supplied inputs for splice-in or the channel - /// balance for splice-out. If the counterparty also initiates a splice and wins the - /// tie-break, they become the initiator and choose the feerate. The fee is then - /// re-estimated at the counterparty's feerate for only our contributed inputs and outputs, - /// which may be higher or lower than the original estimate. The contribution is dropped and - /// the splice proceeds without it when: - /// - the counterparty's feerate is below `min_feerate` - /// - the counterparty's feerate is above `max_feerate` and the re-estimated fee exceeds the - /// original fee estimate - /// - the re-estimated fee exceeds the *fee buffer* regardless of `max_feerate` - /// - /// The fee buffer is the maximum fee that can be accommodated: - /// - **splice-in**: the selected inputs' value minus the contributed amount - /// - **splice-out**: the channel balance minus the withdrawal outputs - /// /// Returns a [`FundingTemplate`] which should be used to build a [`FundingContribution`] via - /// one of its splice methods (e.g., [`FundingTemplate::splice_in_sync`]). The resulting + /// one of its splice methods (e.g., [`FundingTemplate::splice_in_sync`]). The `min_feerate` + /// and `max_feerate` parameters are provided when calling those splice methods. The resulting /// contribution must then be passed to [`ChannelManager::funding_contributed`]. /// - /// # Events - /// - /// Once the funding transaction has been constructed, an [`Event::SplicePending`] will be - /// emitted. At this point, any inputs contributed to the splice can only be re-spent if an - /// [`Event::DiscardFunding`] is seen. - /// - /// After initial signatures have been exchanged, [`Event::FundingTransactionReadyForSigning`] - /// will be generated and [`ChannelManager::funding_transaction_signed`] should be called. - /// - /// If any failures occur while negotiating the funding transaction, an [`Event::SpliceFailed`] - /// will be emitted. Any contributed inputs no longer used will be included here and thus can - /// be re-spent. - /// - /// Once the splice has been locked by both counterparties, an [`Event::ChannelReady`] will be - /// emitted with the new funding output. At this point, a new splice can be negotiated by - /// calling `splice_channel` again on this channel. - /// - /// [`FundingContribution`]: crate::ln::funding::FundingContribution + /// When a pending splice exists with negotiated candidates (i.e., a splice that hasn't been + /// locked yet), [`FundingTemplate::min_rbf_feerate`] will return the minimum feerate required + /// for an RBF attempt (25/24 of the previous feerate). This can be used to choose an + /// appropriate `min_feerate` when calling the splice methods. #[rustfmt::skip] pub fn splice_channel( &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, - min_feerate: FeeRate, max_feerate: FeeRate, ) -> Result { let per_peer_state = self.per_peer_state.read().unwrap(); @@ -4783,7 +4749,7 @@ impl< match peer_state.channel_by_id.entry(*channel_id) { hash_map::Entry::Occupied(chan_phase_entry) => { if let Some(chan) = chan_phase_entry.get().as_funded() { - chan.splice_channel(min_feerate, max_feerate) + chan.splice_channel() } else { Err(APIError::ChannelUnavailable { err: format!( @@ -4809,41 +4775,14 @@ impl< /// Initiating an RBF requires that the channel counterparty supports splicing. The /// counterparty must be currently connected. /// - /// # Arguments - /// - /// The RBF initiator is responsible for paying fees for common fields, shared inputs, and - /// shared outputs along with any contributed inputs and outputs. When building a - /// [`FundingContribution`], fees are estimated using `min_feerate` and must be covered by the - /// supplied inputs for splice-in or the channel balance for splice-out. If the counterparty - /// also initiates an RBF and wins the tie-break, they become the initiator and choose the - /// feerate. In that case, `max_feerate` is used to reject a feerate that is too high for our - /// contribution. - /// /// Returns a [`FundingTemplate`] which should be used to build a [`FundingContribution`] via - /// one of its splice methods (e.g., [`FundingTemplate::splice_in_sync`]). The resulting - /// contribution must then be passed to [`ChannelManager::funding_contributed`]. - /// - /// # Events - /// - /// Once the funding transaction has been constructed, an [`Event::SplicePending`] will be - /// emitted. At this point, any inputs contributed to the splice can only be re-spent if an - /// [`Event::DiscardFunding`] is seen. - /// - /// After initial signatures have been exchanged, [`Event::FundingTransactionReadyForSigning`] - /// will be generated and [`ChannelManager::funding_transaction_signed`] should be called. - /// - /// If any failures occur while negotiating the funding transaction, an [`Event::SpliceFailed`] - /// will be emitted. Any contributed inputs no longer used will be included here and thus can - /// be re-spent. - /// - /// Once the splice has been locked by both counterparties, an [`Event::ChannelReady`] will be - /// emitted with the new funding output. At this point, a new splice can be negotiated by - /// calling `splice_channel` again on this channel. - /// - /// [`FundingContribution`]: crate::ln::funding::FundingContribution + /// one of its splice methods (e.g., [`FundingTemplate::splice_in_sync`]). The `min_feerate` + /// and `max_feerate` parameters are provided when calling those splice methods. + /// [`FundingTemplate::min_rbf_feerate`] returns the minimum feerate required for the RBF + /// (25/24 of the previous feerate). The resulting contribution must then be passed to + /// [`ChannelManager::funding_contributed`]. pub fn rbf_channel( - &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, min_feerate: FeeRate, - max_feerate: FeeRate, + &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, ) -> Result { let per_peer_state = self.per_peer_state.read().unwrap(); @@ -4871,7 +4810,7 @@ impl< match peer_state.channel_by_id.entry(*channel_id) { hash_map::Entry::Occupied(chan_phase_entry) => { if let Some(chan) = chan_phase_entry.get().as_funded() { - chan.rbf_channel(min_feerate, max_feerate) + chan.rbf_channel() } else { Err(APIError::ChannelUnavailable { err: format!( @@ -6622,20 +6561,43 @@ impl< /// An optional `locktime` for the funding transaction may be specified. If not given, the /// current best block height is used. /// + /// # Fee Estimation + /// + /// The splice initiator is responsible for paying fees for common fields, shared inputs, and + /// shared outputs along with any contributed inputs and outputs. When building a + /// [`FundingContribution`], fees are estimated at `min_feerate` assuming initiator + /// responsibility and must be covered by the supplied inputs for splice-in or the channel + /// balance for splice-out. If the counterparty also initiates a splice and wins the + /// tie-break, they become the initiator and choose the feerate. The fee is then + /// re-estimated at the counterparty's feerate for only our contributed inputs and outputs, + /// which may be higher or lower than the original estimate. The contribution is dropped and + /// the splice proceeds without it when: + /// - the counterparty's feerate is below `min_feerate` + /// - the counterparty's feerate is above `max_feerate` and the re-estimated fee exceeds the + /// original fee estimate + /// - the re-estimated fee exceeds the *fee buffer* regardless of `max_feerate` + /// + /// The fee buffer is the maximum fee that can be accommodated: + /// - **splice-in**: the selected inputs' value minus the contributed amount + /// - **splice-out**: the channel balance minus the withdrawal outputs + /// /// # Events /// /// Calling this method will commence the process of creating a new funding transaction for the - /// channel. An [`Event::FundingTransactionReadyForSigning`] will be generated once the - /// transaction is successfully constructed interactively with the counterparty. + /// channel. Once the funding transaction has been constructed, an [`Event::SplicePending`] + /// will be emitted. At this point, any inputs contributed to the splice can only be re-spent + /// if an [`Event::DiscardFunding`] is seen. /// - /// If unsuccessful, an [`Event::SpliceFailed`] will be produced if there aren't any earlier - /// splice attempts for the channel outstanding (i.e., haven't yet produced either - /// [`Event::SplicePending`] or [`Event::SpliceFailed`]). + /// If any failures occur while negotiating the funding transaction, an [`Event::SpliceFailed`] + /// will be emitted. Any contributed inputs no longer used will be included in an + /// [`Event::DiscardFunding`] and thus can be re-spent. /// - /// If unsuccessful, an [`Event::DiscardFunding`] will be produced for any contributions - /// passed in that are not found in any outstanding attempts for the channel. If there are no - /// such contributions, then the [`Event::DiscardFunding`] will not be produced since these - /// contributions must not be reused yet. + /// After initial signatures have been exchanged, [`Event::FundingTransactionReadyForSigning`] + /// will be generated and [`ChannelManager::funding_transaction_signed`] should be called. + /// + /// Once the splice has been locked by both counterparties, an [`Event::ChannelReady`] will be + /// emitted with the new funding output. At this point, a new splice can be negotiated by + /// calling [`ChannelManager::splice_channel`] again on this channel. /// /// # Errors /// diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index c81024ca080..52aabe5a12a 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -121,31 +121,45 @@ pub struct FundingTemplate { /// transaction. shared_input: Option, - /// The minimum fee rate for the splice transaction, used to propose as initiator. - min_feerate: FeeRate, - - /// The maximum fee rate to accept as acceptor before declining to add our contribution to the - /// splice. - max_feerate: FeeRate, + /// The minimum RBF feerate (25/24 of the previous feerate), if this template is for an + /// RBF attempt. `None` for fresh splices with no pending splice candidates. + min_rbf_feerate: Option, } impl FundingTemplate { /// Constructs a [`FundingTemplate`] for a splice using the provided shared input. - pub(super) fn new( - shared_input: Option, min_feerate: FeeRate, max_feerate: FeeRate, - ) -> Self { - Self { shared_input, min_feerate, max_feerate } + pub(super) fn new(shared_input: Option, min_rbf_feerate: Option) -> Self { + Self { shared_input, min_rbf_feerate } + } + + /// Returns the minimum RBF feerate, if this template is for an RBF attempt. + /// + /// When set, the `min_feerate` passed to the splice methods (e.g., + /// [`FundingTemplate::splice_in_sync`]) must be at least this value. + pub fn min_rbf_feerate(&self) -> Option { + self.min_rbf_feerate } } macro_rules! build_funding_contribution { - ($value_added:expr, $outputs:expr, $shared_input:expr, $feerate:expr, $max_feerate:expr, $wallet:ident, $($await:tt)*) => {{ + ($value_added:expr, $outputs:expr, $shared_input:expr, $min_rbf_feerate:expr, $feerate:expr, $max_feerate:expr, $wallet:ident, $($await:tt)*) => {{ let value_added: Amount = $value_added; let outputs: Vec = $outputs; let shared_input: Option = $shared_input; + let min_rbf_feerate: Option = $min_rbf_feerate; let feerate: FeeRate = $feerate; let max_feerate: FeeRate = $max_feerate; + if feerate > max_feerate { + return Err(()); + } + + if let Some(min_rbf_feerate) = min_rbf_feerate { + if feerate < min_rbf_feerate { + return Err(()); + } + } + // Validate user-provided amounts are within MAX_MONEY before coin selection to // ensure FundingContribution::net_value() arithmetic cannot overflow. With all // amounts bounded by MAX_MONEY (~2.1e15 sat), the worst-case net_value() @@ -224,28 +238,29 @@ impl FundingTemplate { /// Creates a [`FundingContribution`] for adding funds to a channel using `wallet` to perform /// coin selection. pub async fn splice_in( - self, value_added: Amount, wallet: W, + self, value_added: Amount, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, ) -> Result { if value_added == Amount::ZERO { return Err(()); } - let FundingTemplate { shared_input, min_feerate, max_feerate } = self; - build_funding_contribution!(value_added, vec![], shared_input, min_feerate, max_feerate, wallet, await) + let FundingTemplate { shared_input, min_rbf_feerate } = self; + build_funding_contribution!(value_added, vec![], shared_input, min_rbf_feerate, min_feerate, max_feerate, wallet, await) } /// Creates a [`FundingContribution`] for adding funds to a channel using `wallet` to perform /// coin selection. pub fn splice_in_sync( - self, value_added: Amount, wallet: W, + self, value_added: Amount, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, ) -> Result { if value_added == Amount::ZERO { return Err(()); } - let FundingTemplate { shared_input, min_feerate, max_feerate } = self; + let FundingTemplate { shared_input, min_rbf_feerate } = self; build_funding_contribution!( value_added, vec![], shared_input, + min_rbf_feerate, min_feerate, max_feerate, wallet, @@ -255,28 +270,29 @@ impl FundingTemplate { /// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to /// perform coin selection. pub async fn splice_out( - self, outputs: Vec, wallet: W, + self, outputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, ) -> Result { if outputs.is_empty() { return Err(()); } - let FundingTemplate { shared_input, min_feerate, max_feerate } = self; - build_funding_contribution!(Amount::ZERO, outputs, shared_input, min_feerate, max_feerate, wallet, await) + let FundingTemplate { shared_input, min_rbf_feerate } = self; + build_funding_contribution!(Amount::ZERO, outputs, shared_input, min_rbf_feerate, min_feerate, max_feerate, wallet, await) } /// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to /// perform coin selection. pub fn splice_out_sync( - self, outputs: Vec, wallet: W, + self, outputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, ) -> Result { if outputs.is_empty() { return Err(()); } - let FundingTemplate { shared_input, min_feerate, max_feerate } = self; + let FundingTemplate { shared_input, min_rbf_feerate } = self; build_funding_contribution!( Amount::ZERO, outputs, shared_input, + min_rbf_feerate, min_feerate, max_feerate, wallet, @@ -286,28 +302,31 @@ impl FundingTemplate { /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using /// `wallet` to perform coin selection. pub async fn splice_in_and_out( - self, value_added: Amount, outputs: Vec, wallet: W, + self, value_added: Amount, outputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, + wallet: W, ) -> Result { if value_added == Amount::ZERO && outputs.is_empty() { return Err(()); } - let FundingTemplate { shared_input, min_feerate, max_feerate } = self; - build_funding_contribution!(value_added, outputs, shared_input, min_feerate, max_feerate, wallet, await) + let FundingTemplate { shared_input, min_rbf_feerate } = self; + build_funding_contribution!(value_added, outputs, shared_input, min_rbf_feerate, min_feerate, max_feerate, wallet, await) } /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using /// `wallet` to perform coin selection. pub fn splice_in_and_out_sync( - self, value_added: Amount, outputs: Vec, wallet: W, + self, value_added: Amount, outputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, + wallet: W, ) -> Result { if value_added == Amount::ZERO && outputs.is_empty() { return Err(()); } - let FundingTemplate { shared_input, min_feerate, max_feerate } = self; + let FundingTemplate { shared_input, min_rbf_feerate } = self; build_funding_contribution!( value_added, outputs, shared_input, + min_rbf_feerate, min_feerate, max_feerate, wallet, @@ -1082,41 +1101,77 @@ mod tests { // splice_in_sync with value_added > MAX_MONEY { - let template = FundingTemplate::new(None, feerate, feerate); - assert!(template.splice_in_sync(over_max, UnreachableWallet).is_err()); + let template = FundingTemplate::new(None, None); + assert!(template + .splice_in_sync(over_max, feerate, feerate, UnreachableWallet) + .is_err()); } // splice_out_sync with single output value > MAX_MONEY { - let template = FundingTemplate::new(None, feerate, feerate); + let template = FundingTemplate::new(None, None); let outputs = vec![funding_output_sats(over_max.to_sat())]; - assert!(template.splice_out_sync(outputs, UnreachableWallet).is_err()); + assert!(template + .splice_out_sync(outputs, feerate, feerate, UnreachableWallet) + .is_err()); } // splice_out_sync with multiple outputs summing > MAX_MONEY { - let template = FundingTemplate::new(None, feerate, feerate); + let template = FundingTemplate::new(None, None); let half_over = Amount::MAX_MONEY / 2 + Amount::from_sat(1); let outputs = vec![ funding_output_sats(half_over.to_sat()), funding_output_sats(half_over.to_sat()), ]; - assert!(template.splice_out_sync(outputs, UnreachableWallet).is_err()); + assert!(template + .splice_out_sync(outputs, feerate, feerate, UnreachableWallet) + .is_err()); } // splice_in_and_out_sync with value_added > MAX_MONEY { - let template = FundingTemplate::new(None, feerate, feerate); + let template = FundingTemplate::new(None, None); let outputs = vec![funding_output_sats(1_000)]; - assert!(template.splice_in_and_out_sync(over_max, outputs, UnreachableWallet).is_err()); + assert!(template + .splice_in_and_out_sync(over_max, outputs, feerate, feerate, UnreachableWallet) + .is_err()); } // splice_in_and_out_sync with output sum > MAX_MONEY { - let template = FundingTemplate::new(None, feerate, feerate); + let template = FundingTemplate::new(None, None); let outputs = vec![funding_output_sats(over_max.to_sat())]; assert!(template - .splice_in_and_out_sync(Amount::from_sat(1_000), outputs, UnreachableWallet) + .splice_in_and_out_sync( + Amount::from_sat(1_000), + outputs, + feerate, + feerate, + UnreachableWallet, + ) + .is_err()); + } + } + + #[test] + fn test_build_funding_contribution_validates_feerate_range() { + let low = FeeRate::from_sat_per_kwu(1000); + let high = FeeRate::from_sat_per_kwu(2000); + + // min_feerate > max_feerate is rejected + { + let template = FundingTemplate::new(None, None); + assert!(template + .splice_in_sync(Amount::from_sat(10_000), high, low, UnreachableWallet) + .is_err()); + } + + // min_feerate < min_rbf_feerate is rejected + { + let template = FundingTemplate::new(None, Some(high)); + assert!(template + .splice_in_sync(Amount::from_sat(10_000), low, FeeRate::MAX, UnreachableWallet) .is_err()); } } diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index bdfe14635e0..fbc2a81969c 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -59,8 +59,7 @@ fn test_splicing_not_supported_api_error() { let (_, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1); - let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let res = nodes[1].node.splice_channel(&channel_id, &node_id_0, feerate, FeeRate::MAX); + let res = nodes[1].node.splice_channel(&channel_id, &node_id_0); match res { Err(APIError::ChannelUnavailable { err }) => { assert!(err.contains("Peer does not support splicing")) @@ -81,7 +80,7 @@ fn test_splicing_not_supported_api_error() { reconnect_args.send_announcement_sigs = (true, true); reconnect_nodes(reconnect_args); - let res = nodes[1].node.splice_channel(&channel_id, &node_id_0, feerate, FeeRate::MAX); + let res = nodes[1].node.splice_channel(&channel_id, &node_id_0); match res { Err(APIError::ChannelUnavailable { err }) => { assert!(err.contains("Peer does not support quiescence, a splicing prerequisite")) @@ -111,13 +110,13 @@ fn test_v1_splice_in_negative_insufficient_inputs() { let feerate = FeeRate::from_sat_per_kwu(1024); // Initiate splice-in, with insufficient input contribution - let funding_template = nodes[0] - .node - .splice_channel(&channel_id, &nodes[1].node.get_our_node_id(), feerate, FeeRate::MAX) - .unwrap(); + let funding_template = + nodes[0].node.splice_channel(&channel_id, &nodes[1].node.get_our_node_id()).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - assert!(funding_template.splice_in_sync(splice_in_value, &wallet).is_err()); + assert!(funding_template + .splice_in_sync(splice_in_value, feerate, FeeRate::MAX, &wallet) + .is_err()); } /// A mock wallet that returns a pre-configured [`CoinSelection`] with a single input and change @@ -176,10 +175,8 @@ fn test_validate_accounts_for_change_output_weight() { create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0); let feerate = FeeRate::from_sat_per_kwu(2000); - let funding_template = nodes[0] - .node - .splice_channel(&channel_id, &nodes[1].node.get_our_node_id(), feerate, FeeRate::MAX) - .unwrap(); + let funding_template = + nodes[0].node.splice_channel(&channel_id, &nodes[1].node.get_our_node_id()).unwrap(); // Input value = value_added + 1800: above 1736/1740 (fee without change), below 1984/1988 // (fee with change). @@ -188,7 +185,8 @@ fn test_validate_accounts_for_change_output_weight() { utxo_value: value_added + Amount::from_sat(1800), change_value: Amount::from_sat(1000), }; - let contribution = funding_template.splice_in_sync(value_added, &wallet).unwrap(); + let contribution = + funding_template.splice_in_sync(value_added, feerate, FeeRate::MAX, &wallet).unwrap(); assert!(contribution.change_output().is_some()); assert!(contribution.validate().is_err()); @@ -221,13 +219,12 @@ pub fn do_initiate_splice_in<'a, 'b, 'c, 'd>( value_added: Amount, ) -> FundingContribution { let node_id_acceptor = acceptor.node.get_our_node_id(); - let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = initiator - .node - .splice_channel(&channel_id, &node_id_acceptor, feerate, FeeRate::MAX) - .unwrap(); + let floor_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let funding_template = initiator.node.splice_channel(&channel_id, &node_id_acceptor).unwrap(); + let feerate = funding_template.min_rbf_feerate().unwrap_or(floor_feerate); let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger); - let funding_contribution = funding_template.splice_in_sync(value_added, &wallet).unwrap(); + let funding_contribution = + funding_template.splice_in_sync(value_added, feerate, FeeRate::MAX, &wallet).unwrap(); initiator .node .funding_contributed(&channel_id, &node_id_acceptor, funding_contribution.clone(), None) @@ -240,10 +237,10 @@ pub fn do_initiate_rbf_splice_in<'a, 'b, 'c, 'd>( value_added: Amount, feerate: FeeRate, ) -> FundingContribution { let node_id_counterparty = counterparty.node.get_our_node_id(); - let funding_template = - node.node.rbf_channel(&channel_id, &node_id_counterparty, feerate, FeeRate::MAX).unwrap(); + let funding_template = node.node.rbf_channel(&channel_id, &node_id_counterparty).unwrap(); let wallet = WalletSync::new(Arc::clone(&node.wallet_source), node.logger); - let funding_contribution = funding_template.splice_in_sync(value_added, &wallet).unwrap(); + let funding_contribution = + funding_template.splice_in_sync(value_added, feerate, FeeRate::MAX, &wallet).unwrap(); node.node .funding_contributed(&channel_id, &node_id_counterparty, funding_contribution.clone(), None) .unwrap(); @@ -255,11 +252,11 @@ pub fn do_initiate_rbf_splice_in_and_out<'a, 'b, 'c, 'd>( value_added: Amount, outputs: Vec, feerate: FeeRate, ) -> FundingContribution { let node_id_counterparty = counterparty.node.get_our_node_id(); - let funding_template = - node.node.rbf_channel(&channel_id, &node_id_counterparty, feerate, FeeRate::MAX).unwrap(); + let funding_template = node.node.rbf_channel(&channel_id, &node_id_counterparty).unwrap(); let wallet = WalletSync::new(Arc::clone(&node.wallet_source), node.logger); - let funding_contribution = - funding_template.splice_in_and_out_sync(value_added, outputs, &wallet).unwrap(); + let funding_contribution = funding_template + .splice_in_and_out_sync(value_added, outputs, feerate, FeeRate::MAX, &wallet) + .unwrap(); node.node .funding_contributed(&channel_id, &node_id_counterparty, funding_contribution.clone(), None) .unwrap(); @@ -271,13 +268,12 @@ pub fn initiate_splice_out<'a, 'b, 'c, 'd>( outputs: Vec, ) -> Result { let node_id_acceptor = acceptor.node.get_our_node_id(); - let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = initiator - .node - .splice_channel(&channel_id, &node_id_acceptor, feerate, FeeRate::MAX) - .unwrap(); + let floor_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let funding_template = initiator.node.splice_channel(&channel_id, &node_id_acceptor).unwrap(); + let feerate = funding_template.min_rbf_feerate().unwrap_or(floor_feerate); let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger); - let funding_contribution = funding_template.splice_out_sync(outputs, &wallet).unwrap(); + let funding_contribution = + funding_template.splice_out_sync(outputs, feerate, FeeRate::MAX, &wallet).unwrap(); match initiator.node.funding_contributed( &channel_id, &node_id_acceptor, @@ -304,14 +300,13 @@ pub fn do_initiate_splice_in_and_out<'a, 'b, 'c, 'd>( value_added: Amount, outputs: Vec, ) -> FundingContribution { let node_id_acceptor = acceptor.node.get_our_node_id(); - let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = initiator - .node - .splice_channel(&channel_id, &node_id_acceptor, feerate, FeeRate::MAX) - .unwrap(); + let floor_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let funding_template = initiator.node.splice_channel(&channel_id, &node_id_acceptor).unwrap(); + let feerate = funding_template.min_rbf_feerate().unwrap_or(floor_feerate); let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger); - let funding_contribution = - funding_template.splice_in_and_out_sync(value_added, outputs, &wallet).unwrap(); + let funding_contribution = funding_template + .splice_in_and_out_sync(value_added, outputs, feerate, FeeRate::MAX, &wallet) + .unwrap(); initiator .node .funding_contributed(&channel_id, &node_id_acceptor, funding_contribution.clone(), None) @@ -1363,17 +1358,17 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { }]; let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = - nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate, FeeRate::MAX).unwrap(); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_1_id).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let funding_contribution = funding_template.splice_out_sync(outputs.clone(), &wallet).unwrap(); + let funding_contribution = + funding_template.splice_out_sync(outputs.clone(), feerate, FeeRate::MAX, &wallet).unwrap(); nodes[0] .node .funding_contributed(&channel_id, &node_1_id, funding_contribution.clone(), None) .unwrap(); assert_eq!( - nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate, FeeRate::MAX), + nodes[0].node.splice_channel(&channel_id, &node_1_id), Err(APIError::APIMisuseError { err: format!( "Channel {} cannot be spliced as one is waiting to be negotiated", @@ -1385,7 +1380,7 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { let new_funding_script = complete_splice_handshake(&nodes[0], &nodes[1]); assert_eq!( - nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate, FeeRate::MAX), + nodes[0].node.splice_channel(&channel_id, &node_1_id), Err(APIError::APIMisuseError { err: format!( "Channel {} cannot be spliced as one is currently being negotiated", @@ -1394,18 +1389,6 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { }), ); - // The acceptor can enqueue a quiescent action while the current splice is pending. - let added_value = Amount::from_sat(initial_channel_value_sat); - let acceptor_template = - nodes[1].node.splice_channel(&channel_id, &node_0_id, feerate, FeeRate::MAX).unwrap(); - let acceptor_wallet = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); - let acceptor_contribution = - acceptor_template.splice_in_sync(added_value, &acceptor_wallet).unwrap(); - nodes[1] - .node - .funding_contributed(&channel_id, &node_0_id, acceptor_contribution, None) - .unwrap(); - complete_interactive_funding_negotiation( &nodes[0], &nodes[1], @@ -1415,7 +1398,7 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { ); assert_eq!( - nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate, FeeRate::MAX), + nodes[0].node.splice_channel(&channel_id, &node_1_id), Err(APIError::APIMisuseError { err: format!( "Channel {} cannot be spliced as one is currently being negotiated", @@ -1430,9 +1413,8 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { expect_splice_pending_event(&nodes[0], &node_1_id); expect_splice_pending_event(&nodes[1], &node_0_id); - // Now that the splice is pending, another splice may be initiated, but we must wait until - // the `splice_locked` exchange to send the initiator `stfu`. - assert!(nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate, FeeRate::MAX).is_ok()); + // Now that the splice is pending, another splice may be initiated. + assert!(nodes[0].node.splice_channel(&channel_id, &node_1_id).is_ok()); if reconnect { nodes[0].node.peer_disconnected(node_1_id); @@ -1446,54 +1428,35 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { mine_transaction(&nodes[0], &splice_tx); mine_transaction(&nodes[1], &splice_tx); let stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); - - assert!( - matches!(stfu, Some(MessageSendEvent::SendStfu { node_id, .. }) if node_id == node_0_id) - ); + // Node 0 had called splice_channel (line above) but never funding_contributed, so no stfu + // is expected from node 0 at this point. + assert!(stfu.is_none()); } #[test] fn test_initiating_splice_holds_stfu_with_pending_splice() { - // Test that we don't send stfu too early for a new splice while we're already pending one. + // Test that a splice can be completed and locked successfully. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let config = test_default_channel_config(); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_0_id = nodes[0].node.get_our_node_id(); provide_utxo_reserves(&nodes, 2, Amount::ONE_BTC); let initial_channel_value_sat = 100_000; let (_, _, channel_id, _) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); - // Have both nodes attempt a splice, but only node 0 will call back and negotiate the splice. + // Node 0 initiates a splice, completing the full flow. let value_added = Amount::from_sat(10_000); let funding_contribution_0 = initiate_splice_in(&nodes[0], &nodes[1], channel_id, value_added); - - let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = - nodes[1].node.splice_channel(&channel_id, &node_0_id, feerate, FeeRate::MAX).unwrap(); - let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution_0); - // With the splice negotiated, have node 1 call back. This will queue the quiescent action, but - // it shouldn't send stfu yet as there's a pending splice. - let wallet = WalletSync::new(Arc::clone(&nodes[1].wallet_source), &nodes[1].logger); - let funding_contribution = funding_template.splice_in_sync(value_added, &wallet).unwrap(); - nodes[1] - .node - .funding_contributed(&channel_id, &node_0_id, funding_contribution.clone(), None) - .unwrap(); - assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); - + // Mine and lock the splice. mine_transaction(&nodes[0], &splice_tx); mine_transaction(&nodes[1], &splice_tx); let stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], 5); - assert!( - matches!(stfu, Some(MessageSendEvent::SendStfu { node_id, .. }) if node_id == node_0_id) - ); + assert!(stfu.is_none()); } #[test] @@ -1569,26 +1532,22 @@ fn do_test_splice_tiebreak( provide_utxo_reserves(&nodes, 2, Amount::from_sat(100_000)); // Node 0 calls splice_channel + splice_in_sync + funding_contributed. - let funding_template_0 = nodes[0] - .node - .splice_channel(&channel_id, &node_id_1, node_0_feerate, FeeRate::MAX) - .unwrap(); + let funding_template_0 = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet_0 = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let node_0_funding_contribution = - funding_template_0.splice_in_sync(added_value, &wallet_0).unwrap(); + let node_0_funding_contribution = funding_template_0 + .splice_in_sync(added_value, node_0_feerate, FeeRate::MAX, &wallet_0) + .unwrap(); nodes[0] .node .funding_contributed(&channel_id, &node_id_1, node_0_funding_contribution.clone(), None) .unwrap(); // Node 1 calls splice_channel + splice_in_sync + funding_contributed. - let funding_template_1 = nodes[1] - .node - .splice_channel(&channel_id, &node_id_0, node_1_feerate, FeeRate::MAX) - .unwrap(); + let funding_template_1 = nodes[1].node.splice_channel(&channel_id, &node_id_0).unwrap(); let wallet_1 = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); - let node_1_funding_contribution = - funding_template_1.splice_in_sync(node_1_splice_value, &wallet_1).unwrap(); + let node_1_funding_contribution = funding_template_1 + .splice_in_sync(node_1_splice_value, node_1_feerate, FeeRate::MAX, &wallet_1) + .unwrap(); nodes[1] .node .funding_contributed(&channel_id, &node_id_0, node_1_funding_contribution.clone(), None) @@ -1812,24 +1771,22 @@ fn test_splice_tiebreak_feerate_too_high_rejected() { let node_1_max_feerate = FeeRate::from_sat_per_kwu(3_000); // Node 0: very high feerate, moderate splice-in. - let funding_template_0 = - nodes[0].node.splice_channel(&channel_id, &node_id_1, high_feerate, FeeRate::MAX).unwrap(); + let funding_template_0 = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet_0 = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let node_0_funding_contribution = - funding_template_0.splice_in_sync(node_0_added_value, &wallet_0).unwrap(); + let node_0_funding_contribution = funding_template_0 + .splice_in_sync(node_0_added_value, high_feerate, FeeRate::MAX, &wallet_0) + .unwrap(); nodes[0] .node .funding_contributed(&channel_id, &node_id_1, node_0_funding_contribution.clone(), None) .unwrap(); // Node 1: floor feerate, moderate splice-in, low max_feerate. - let funding_template_1 = nodes[1] - .node - .splice_channel(&channel_id, &node_id_0, floor_feerate, node_1_max_feerate) - .unwrap(); + let funding_template_1 = nodes[1].node.splice_channel(&channel_id, &node_id_0).unwrap(); let wallet_1 = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); - let node_1_funding_contribution = - funding_template_1.splice_in_sync(node_1_added_value, &wallet_1).unwrap(); + let node_1_funding_contribution = funding_template_1 + .splice_in_sync(node_1_added_value, floor_feerate, node_1_max_feerate, &wallet_1) + .unwrap(); nodes[1] .node .funding_contributed(&channel_id, &node_id_0, node_1_funding_contribution.clone(), None) @@ -3530,10 +3487,10 @@ fn test_funding_contributed_counterparty_not_found() { provide_utxo_reserves(&nodes, 1, splice_in_amount * 2); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = - nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let funding_contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); + let funding_contribution = + funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap(); // Use a fake/unknown public key as counterparty let fake_node_id = @@ -3570,10 +3527,10 @@ fn test_funding_contributed_channel_not_found() { provide_utxo_reserves(&nodes, 1, splice_in_amount * 2); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = - nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let funding_contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); + let funding_contribution = + funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap(); // Use a random/unknown channel_id let fake_channel_id = ChannelId::from_bytes([42; 32]); @@ -3615,11 +3572,16 @@ fn test_funding_contributed_splice_already_pending() { script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_raw_hash(Hash::all_zeros())), }; let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = - nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let first_contribution = funding_template - .splice_in_and_out_sync(splice_in_amount, vec![first_splice_out.clone()], &wallet) + .splice_in_and_out_sync( + splice_in_amount, + vec![first_splice_out.clone()], + feerate, + FeeRate::MAX, + &wallet, + ) .unwrap(); // Initiate a second splice with a DIFFERENT output to test that different outputs @@ -3638,11 +3600,16 @@ fn test_funding_contributed_splice_already_pending() { nodes[0].wallet_source.clear_utxos(); provide_utxo_reserves(&nodes, 1, splice_in_amount * 3); - let funding_template = - nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let second_contribution = funding_template - .splice_in_and_out_sync(splice_in_amount, vec![second_splice_out.clone()], &wallet) + .splice_in_and_out_sync( + splice_in_amount, + vec![second_splice_out.clone()], + feerate, + FeeRate::MAX, + &wallet, + ) .unwrap(); // First funding_contributed - this sets up the quiescent action @@ -3708,10 +3675,10 @@ fn test_funding_contributed_duplicate_contribution_no_event() { provide_utxo_reserves(&nodes, 1, splice_in_amount * 2); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = - nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); + let contribution = + funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap(); // First funding_contributed - this sets up the quiescent action nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution.clone(), None).unwrap(); @@ -3767,19 +3734,19 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) { // Build first contribution let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = - nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let first_contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); + let first_contribution = + funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap(); // Build second contribution with different UTXOs so inputs/outputs don't overlap nodes[0].wallet_source.clear_utxos(); provide_utxo_reserves(&nodes, 1, splice_in_amount * 3); - let funding_template = - nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let second_contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); + let second_contribution = + funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap(); // First funding_contributed - sets up the quiescent action and queues STFU nodes[0] @@ -3897,10 +3864,10 @@ fn test_funding_contributed_channel_shutdown() { provide_utxo_reserves(&nodes, 1, splice_in_amount * 2); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = - nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let funding_contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); + let funding_contribution = + funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap(); // Initiate channel shutdown - this makes is_usable() return false nodes[0].node.close_channel(&channel_id, &node_id_1).unwrap(); @@ -3951,12 +3918,10 @@ fn test_funding_contributed_unfunded_channel() { provide_utxo_reserves(&nodes, 1, splice_in_amount * 2); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = nodes[0] - .node - .splice_channel(&funded_channel_id, &node_id_1, feerate, FeeRate::MAX) - .unwrap(); + let funding_template = nodes[0].node.splice_channel(&funded_channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let funding_contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); + let funding_contribution = + funding_template.splice_in_sync(splice_in_amount, feerate, FeeRate::MAX, &wallet).unwrap(); // Call funding_contributed with the unfunded channel's ID instead of the funded one. // Returns APIMisuseError because the channel is not funded. @@ -4386,7 +4351,7 @@ fn test_splice_rbf_acceptor_basic() { #[test] fn test_splice_rbf_insufficient_feerate() { - // Test that rbf_channel rejects a feerate that doesn't satisfy the 25/24 rule, and that the + // Test that splice_in_sync rejects a feerate that doesn't satisfy the 25/24 rule, and that the // acceptor also rejects tx_init_rbf with an insufficient feerate from a misbehaving peer. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); @@ -4408,20 +4373,27 @@ fn test_splice_rbf_insufficient_feerate() { let (_splice_tx, _new_funding_script) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); - // Initiator-side: rbf_channel rejects an insufficient feerate. + // Initiator-side: splice_in_sync rejects an insufficient feerate. // Original feerate was 253. Using exactly 253 should fail since 253 * 24 < 253 * 25. let same_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let err = - nodes[0].node.rbf_channel(&channel_id, &node_id_1, same_feerate, FeeRate::MAX).unwrap_err(); - assert_eq!( - err, - APIError::APIMisuseError { - err: format!( - "Channel {} RBF feerate {} is less than 25/24 of the previous feerate {}", - channel_id, FEERATE_FLOOR_SATS_PER_KW, FEERATE_FLOOR_SATS_PER_KW, - ), - } - ); + let funding_template = nodes[0].node.rbf_channel(&channel_id, &node_id_1).unwrap(); + + // Verify that the template exposes the RBF floor. + let min_rbf_feerate = funding_template.min_rbf_feerate().unwrap(); + let expected_floor = + FeeRate::from_sat_per_kwu(((FEERATE_FLOOR_SATS_PER_KW as u64) * 25).div_ceil(24)); + assert_eq!(min_rbf_feerate, expected_floor); + + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + assert!(funding_template + .splice_in_sync(added_value, same_feerate, FeeRate::MAX, &wallet) + .is_err()); + + // Verify that the floor feerate succeeds. + let funding_template = nodes[0].node.rbf_channel(&channel_id, &node_id_1).unwrap(); + assert!(funding_template + .splice_in_sync(added_value, min_rbf_feerate, FeeRate::MAX, &wallet) + .is_ok()); // Acceptor-side: tx_init_rbf with an insufficient feerate is also rejected. reenter_quiescence(&nodes[0], &nodes[1], &channel_id); @@ -5054,23 +5026,21 @@ fn test_splice_rbf_tiebreak_feerate_too_high_rejected() { let min_rbf_feerate = FeeRate::from_sat_per_kwu(min_rbf_feerate_sat_per_kwu); let node_1_max_feerate = FeeRate::from_sat_per_kwu(3_000); - let funding_template_0 = - nodes[0].node.rbf_channel(&channel_id, &node_id_1, high_feerate, FeeRate::MAX).unwrap(); + let funding_template_0 = nodes[0].node.rbf_channel(&channel_id, &node_id_1).unwrap(); let wallet_0 = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let node_0_funding_contribution = - funding_template_0.splice_in_sync(added_value, &wallet_0).unwrap(); + let node_0_funding_contribution = funding_template_0 + .splice_in_sync(added_value, high_feerate, FeeRate::MAX, &wallet_0) + .unwrap(); nodes[0] .node .funding_contributed(&channel_id, &node_id_1, node_0_funding_contribution.clone(), None) .unwrap(); - let funding_template_1 = nodes[1] - .node - .rbf_channel(&channel_id, &node_id_0, min_rbf_feerate, node_1_max_feerate) - .unwrap(); + let funding_template_1 = nodes[1].node.rbf_channel(&channel_id, &node_id_0).unwrap(); let wallet_1 = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); - let node_1_funding_contribution = - funding_template_1.splice_in_sync(added_value, &wallet_1).unwrap(); + let node_1_funding_contribution = funding_template_1 + .splice_in_sync(added_value, min_rbf_feerate, node_1_max_feerate, &wallet_1) + .unwrap(); nodes[1] .node .funding_contributed(&channel_id, &node_id_0, node_1_funding_contribution.clone(), None) @@ -5121,21 +5091,19 @@ fn test_splice_rbf_acceptor_recontributes() { // Step 1: Both nodes initiate a splice at floor feerate. let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template_0 = - nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); + let funding_template_0 = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet_0 = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let node_0_funding_contribution = - funding_template_0.splice_in_sync(added_value, &wallet_0).unwrap(); + funding_template_0.splice_in_sync(added_value, feerate, FeeRate::MAX, &wallet_0).unwrap(); nodes[0] .node .funding_contributed(&channel_id, &node_id_1, node_0_funding_contribution.clone(), None) .unwrap(); - let funding_template_1 = - nodes[1].node.splice_channel(&channel_id, &node_id_0, feerate, FeeRate::MAX).unwrap(); + let funding_template_1 = nodes[1].node.splice_channel(&channel_id, &node_id_0).unwrap(); let wallet_1 = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); let node_1_funding_contribution = - funding_template_1.splice_in_sync(added_value, &wallet_1).unwrap(); + funding_template_1.splice_in_sync(added_value, feerate, FeeRate::MAX, &wallet_1).unwrap(); nodes[1] .node .funding_contributed(&channel_id, &node_id_0, node_1_funding_contribution.clone(), None) @@ -5249,22 +5217,22 @@ fn test_splice_rbf_recontributes_feerate_too_high() { // from a 100k UTXO (tight budget: ~5k for change/fees). let floor_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template_0 = - nodes[0].node.splice_channel(&channel_id, &node_id_1, floor_feerate, FeeRate::MAX).unwrap(); + let funding_template_0 = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet_0 = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let node_0_funding_contribution = - funding_template_0.splice_in_sync(Amount::from_sat(50_000), &wallet_0).unwrap(); + let node_0_funding_contribution = funding_template_0 + .splice_in_sync(Amount::from_sat(50_000), floor_feerate, FeeRate::MAX, &wallet_0) + .unwrap(); nodes[0] .node .funding_contributed(&channel_id, &node_id_1, node_0_funding_contribution.clone(), None) .unwrap(); let node_1_added_value = Amount::from_sat(95_000); - let funding_template_1 = - nodes[1].node.splice_channel(&channel_id, &node_id_0, floor_feerate, FeeRate::MAX).unwrap(); + let funding_template_1 = nodes[1].node.splice_channel(&channel_id, &node_id_0).unwrap(); let wallet_1 = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); - let node_1_funding_contribution = - funding_template_1.splice_in_sync(node_1_added_value, &wallet_1).unwrap(); + let node_1_funding_contribution = funding_template_1 + .splice_in_sync(node_1_added_value, floor_feerate, FeeRate::MAX, &wallet_1) + .unwrap(); nodes[1] .node .funding_contributed(&channel_id, &node_id_0, node_1_funding_contribution.clone(), None) @@ -5312,11 +5280,11 @@ fn test_splice_rbf_recontributes_feerate_too_high() { provide_utxo_reserves(&nodes, 2, Amount::from_sat(100_000)); let high_feerate = FeeRate::from_sat_per_kwu(20_000); - let funding_template = - nodes[0].node.rbf_channel(&channel_id, &node_id_1, high_feerate, FeeRate::MAX).unwrap(); + let funding_template = nodes[0].node.rbf_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let rbf_funding_contribution = - funding_template.splice_in_sync(Amount::from_sat(50_000), &wallet).unwrap(); + let rbf_funding_contribution = funding_template + .splice_in_sync(Amount::from_sat(50_000), high_feerate, FeeRate::MAX, &wallet) + .unwrap(); nodes[0] .node .funding_contributed(&channel_id, &node_id_1, rbf_funding_contribution.clone(), None) @@ -5651,3 +5619,39 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { reconnect_args.send_announcement_sigs = (true, true); reconnect_nodes(reconnect_args); } + +#[test] +fn test_splice_channel_with_pending_splice_includes_rbf_floor() { + // Test that splice_channel (not rbf_channel) includes the RBF floor when a pending splice + // exists with negotiated candidates. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + // Complete a splice-in at floor feerate. + let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + let (_splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + + // Call splice_channel (not rbf_channel) — the pending splice should cause + // min_rbf_feerate to be set. + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let expected_floor = + FeeRate::from_sat_per_kwu(((FEERATE_FLOOR_SATS_PER_KW as u64) * 25).div_ceil(24)); + assert_eq!(funding_template.min_rbf_feerate(), Some(expected_floor)); + + // Successfully build a contribution at the floor feerate. + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + assert!(funding_template + .splice_in_sync(added_value, expected_floor, FeeRate::MAX, &wallet) + .is_ok()); +} From c132cc993403771b42f51640f4619dd3a678ca1f Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Sun, 15 Mar 2026 14:22:03 -0500 Subject: [PATCH 2/7] Adjust contribution feerate to minimum RBF feerate in funding_contributed When splice_channel is called before a counterparty's splice exists, the user builds a contribution at their chosen feerate without a minimum RBF feerate. If the counterparty completes a splice before funding_contributed is called, the contribution's feerate may be below the 25/24 RBF requirement. Rather than always waiting for the pending splice to lock (which would proceed as a fresh splice), funding_contributed now attempts to adjust the contribution's feerate upward to the minimum RBF feerate when the budget allows, enabling an immediate RBF. When the adjustment isn't possible (max_feerate too low or insufficient fee buffer), the contribution is left unchanged and try_send_stfu delays until the pending splice locks, at which point the splice proceeds at the original feerate. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 77 ++++++++++- lightning/src/ln/funding.rs | 124 ++++++++++++++--- lightning/src/ln/splicing_tests.rs | 207 ++++++++++++++++++++++++++++- 3 files changed, 378 insertions(+), 30 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 6f23aa7857f..32247108ece 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12077,6 +12077,47 @@ where Ok(min_rbf_feerate) } + /// Attempts to adjust the contribution's feerate to the minimum RBF feerate so the splice can + /// proceed as an RBF immediately rather than waiting for the pending splice to lock. + /// Returns the adjusted contribution on success, or the original on failure. + fn maybe_adjust_for_rbf( + &self, contribution: FundingContribution, min_rbf_feerate: FeeRate, logger: &L, + ) -> FundingContribution { + if contribution.feerate() >= min_rbf_feerate { + return contribution; + } + + let holder_balance = match self + .get_holder_counterparty_balances_floor_incl_fee(&self.funding) + .map(|(holder, _)| holder) + { + Ok(balance) => balance, + Err(_) => return contribution, + }; + + if let Err(e) = + contribution.net_value_for_initiator_at_feerate(min_rbf_feerate, holder_balance) + { + log_info!( + logger, + "Cannot adjust to minimum RBF feerate {}: {}; will proceed as fresh splice after lock", + min_rbf_feerate, + e, + ); + return contribution; + } + + log_info!( + logger, + "Adjusting contribution feerate from {} to minimum RBF feerate {}", + contribution.feerate(), + min_rbf_feerate, + ); + contribution + .for_initiator_at_feerate(min_rbf_feerate, holder_balance) + .expect("feerate compatibility already checked") + } + pub fn funding_contributed( &mut self, contribution: FundingContribution, locktime: LockTime, logger: &L, ) -> Result, QuiescentError> { @@ -12161,6 +12202,15 @@ where })); } + // If a pending splice exists with negotiated candidates, attempt to adjust the + // contribution's feerate to the minimum RBF feerate so it can proceed as an RBF immediately + // rather than waiting for the splice to lock. + let contribution = if let Ok(Some(min_rbf_feerate)) = self.can_initiate_rbf() { + self.maybe_adjust_for_rbf(contribution, min_rbf_feerate, logger) + } else { + contribution + }; + self.propose_quiescence(logger, QuiescentAction::Splice { contribution, locktime }) } @@ -13758,13 +13808,26 @@ where #[allow(irrefutable_let_patterns)] if let QuiescentAction::Splice { contribution, .. } = action { if self.pending_splice.is_some() { - if let Err(msg) = self.can_initiate_rbf() { - log_given_level!( - logger, - logger_level, - "Waiting on sending stfu for splice RBF: {msg}" - ); - return None; + match self.can_initiate_rbf() { + Err(msg) => { + log_given_level!( + logger, + logger_level, + "Waiting on sending stfu for splice RBF: {msg}" + ); + return None; + }, + Ok(Some(min_rbf_feerate)) if contribution.feerate() < min_rbf_feerate => { + log_given_level!( + logger, + logger_level, + "Waiting for splice to lock: feerate {} below minimum RBF feerate {}", + contribution.feerate(), + min_rbf_feerate, + ); + return None; + }, + _ => {}, } } } diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 52aabe5a12a..acad13c32ae 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -545,8 +545,12 @@ impl FundingContribution { Ok(()) } - /// Computes the adjusted fee and change output value for the acceptor at the initiator's - /// proposed feerate, which may differ from the feerate used during coin selection. + /// Computes the adjusted fee and change output value at the given target feerate, which may + /// differ from the feerate used during coin selection. + /// + /// The `is_initiator` parameter determines fee responsibility: the initiator pays for common + /// transaction fields, the shared input, and the shared output, while the acceptor only pays + /// for their own contributed inputs and outputs. /// /// On success, returns the new estimated fee and, if applicable, the new change output value: /// - `Some(change)` — the adjusted change output value @@ -554,7 +558,7 @@ impl FundingContribution { /// /// Returns `Err` if the contribution cannot accommodate the target feerate. fn compute_feerate_adjustment( - &self, target_feerate: FeeRate, holder_balance: Amount, + &self, target_feerate: FeeRate, holder_balance: Amount, is_initiator: bool, ) -> Result<(Amount, Option), FeeRateAdjustmentError> { if target_feerate < self.feerate { return Err(FeeRateAdjustmentError::FeeRateTooLow { @@ -564,14 +568,15 @@ impl FundingContribution { } // If the target fee rate exceeds our max fee rate, we may still add our contribution - // if we pay less in fees. This may happen because the acceptor doesn't pay for common - // fields and the shared input / output. + // if we pay less in fees at the target feerate than at the original feerate. This can + // happen when adjusting as acceptor, since the acceptor doesn't pay for common fields + // and the shared input / output. if target_feerate > self.max_feerate { let target_fee = estimate_transaction_fee( &self.inputs, &self.outputs, self.change_output.as_ref(), - false, + is_initiator, self.is_splice, target_feerate, ); @@ -595,7 +600,7 @@ impl FundingContribution { &self.inputs, &self.outputs, self.change_output.as_ref(), - false, + is_initiator, self.is_splice, target_feerate, ); @@ -615,7 +620,7 @@ impl FundingContribution { &self.inputs, &self.outputs, None, - false, + is_initiator, self.is_splice, target_feerate, ); @@ -636,7 +641,7 @@ impl FundingContribution { &self.inputs, &self.outputs, None, - false, + is_initiator, self.is_splice, target_feerate, ); @@ -666,7 +671,7 @@ impl FundingContribution { &[], &self.outputs, None, - false, + is_initiator, self.is_splice, target_feerate, ); @@ -688,17 +693,14 @@ impl FundingContribution { } } - /// Adjusts the contribution's change output for the initiator's feerate. - /// - /// When the acceptor has a pending contribution (from the quiescence tie-breaker scenario), - /// the initiator's proposed feerate may differ from the feerate used during coin selection. - /// This adjusts the change output so the acceptor pays their target fee at the target - /// feerate. - pub(super) fn for_acceptor_at_feerate( - mut self, feerate: FeeRate, holder_balance: Amount, + /// Adjusts the contribution for a different feerate, updating the change output, fee + /// estimate, and feerate. Returns the adjusted contribution, or an error if the feerate + /// can't be accommodated. + fn at_feerate( + mut self, feerate: FeeRate, holder_balance: Amount, is_initiator: bool, ) -> Result { let (new_estimated_fee, new_change) = - self.compute_feerate_adjustment(feerate, holder_balance)?; + self.compute_feerate_adjustment(feerate, holder_balance, is_initiator)?; let surplus = self.fee_buffer_surplus(new_estimated_fee, &new_change); match new_change { Some(value) => self.change_output.as_mut().unwrap().value = value, @@ -710,16 +712,39 @@ impl FundingContribution { Ok(self) } + /// Adjusts the contribution's change output for the initiator's feerate. + /// + /// When the acceptor has a pending contribution (from the quiescence tie-breaker scenario), + /// the initiator's proposed feerate may differ from the feerate used during coin selection. + /// This adjusts the change output so the acceptor pays their target fee at the target + /// feerate. + pub(super) fn for_acceptor_at_feerate( + self, feerate: FeeRate, holder_balance: Amount, + ) -> Result { + self.at_feerate(feerate, holder_balance, false) + } + + /// Adjusts the contribution's change output for the minimum RBF feerate. + /// + /// When a pending splice exists with negotiated candidates and the contribution's feerate + /// is below the minimum RBF feerate (25/24 of the previous feerate), this adjusts the change output + /// so the initiator pays fees at the minimum RBF feerate. + pub(super) fn for_initiator_at_feerate( + self, feerate: FeeRate, holder_balance: Amount, + ) -> Result { + self.at_feerate(feerate, holder_balance, true) + } + /// Returns the net value at the given target feerate without mutating `self`. /// /// This serves double duty: it checks feerate compatibility (returning `Err` if the feerate /// can't be accommodated) and computes the adjusted net value (returning `Ok` with the value /// accounting for the target feerate). - pub(super) fn net_value_for_acceptor_at_feerate( - &self, target_feerate: FeeRate, holder_balance: Amount, + fn net_value_at_feerate( + &self, target_feerate: FeeRate, holder_balance: Amount, is_initiator: bool, ) -> Result { let (new_estimated_fee, new_change) = - self.compute_feerate_adjustment(target_feerate, holder_balance)?; + self.compute_feerate_adjustment(target_feerate, holder_balance, is_initiator)?; let surplus = self .fee_buffer_surplus(new_estimated_fee, &new_change) .to_signed() @@ -731,6 +756,22 @@ impl FundingContribution { Ok(net_value) } + /// Returns the net value at the given target feerate without mutating `self`, + /// assuming acceptor fee responsibility. + pub(super) fn net_value_for_acceptor_at_feerate( + &self, target_feerate: FeeRate, holder_balance: Amount, + ) -> Result { + self.net_value_at_feerate(target_feerate, holder_balance, false) + } + + /// Returns the net value at the given target feerate without mutating `self`, + /// assuming initiator fee responsibility. + pub(super) fn net_value_for_initiator_at_feerate( + &self, target_feerate: FeeRate, holder_balance: Amount, + ) -> Result { + self.net_value_at_feerate(target_feerate, holder_balance, true) + } + /// Returns the fee buffer surplus when a change output is removed. /// /// The fee buffer is the actual amount available for fees from inputs: total input value @@ -1867,4 +1908,43 @@ mod tests { let result = contribution.net_value_for_acceptor_at_feerate(target_feerate, holder_balance); assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. }))); } + + #[test] + fn test_for_initiator_at_feerate_higher_fee_than_acceptor() { + // Verify that the initiator fee estimate is higher than the acceptor estimate at the + // same feerate, since the initiator pays for common fields + shared input/output. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(3000); + let inputs = vec![funding_input_sats(100_000)]; + let change = funding_output_sats(10_000); + + let estimated_fee = + estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate); + + let contribution = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee, + inputs, + outputs: vec![], + change_output: Some(change), + feerate: original_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let acceptor = + contribution.clone().for_acceptor_at_feerate(target_feerate, Amount::MAX).unwrap(); + let initiator = contribution.for_initiator_at_feerate(target_feerate, Amount::MAX).unwrap(); + + // Initiator pays more in fees (common fields + shared input/output weight). + assert!(initiator.estimated_fee > acceptor.estimated_fee); + // Initiator has less change remaining. + assert!( + initiator.change_output.as_ref().unwrap().value + < acceptor.change_output.as_ref().unwrap().value + ); + // Both have the adjusted feerate. + assert_eq!(initiator.feerate, target_feerate); + assert_eq!(acceptor.feerate, target_feerate); + } } diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index fbc2a81969c..0c03585311a 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -746,8 +746,17 @@ pub fn lock_splice<'a, 'b, 'c, 'd>( check_added_monitors(node, 1); } + let mut node_a_stfu = None; if !is_0conf { let mut msg_events = node_a.node.get_and_clear_pending_msg_events(); + + // If node_a had a pending QuiescentAction, filter out the stfu message. + node_a_stfu = msg_events + .last() + .filter(|event| matches!(event, MessageSendEvent::SendStfu { .. })) + .is_some() + .then(|| msg_events.pop().unwrap()); + assert_eq!(msg_events.len(), 2, "{msg_events:?}"); if let MessageSendEvent::SendAnnouncementSignatures { msg, .. } = msg_events.remove(0) { node_b.node.handle_announcement_signatures(node_id_a, &msg); @@ -776,7 +785,7 @@ pub fn lock_splice<'a, 'b, 'c, 'd>( } } - node_b_stfu + node_a_stfu.or(node_b_stfu) } pub fn lock_rbf_splice_after_blocks<'a, 'b, 'c, 'd>( @@ -5655,3 +5664,199 @@ fn test_splice_channel_with_pending_splice_includes_rbf_floor() { .splice_in_sync(added_value, expected_floor, FeeRate::MAX, &wallet) .is_ok()); } + +#[test] +fn test_funding_contributed_adjusts_feerate_for_rbf() { + // Test that funding_contributed adjusts the contribution's feerate to the minimum RBF feerate when a + // pending splice appears between splice_channel and funding_contributed. + // + // Node 0 calls splice_channel (no pending splice → min_rbf_feerate = None) and builds a + // contribution at floor feerate. Node 1 then initiates and completes a splice. When node 0 + // calls funding_contributed, the contribution is adjusted to the minimum RBF feerate and STFU is sent + // immediately. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 4, added_value * 2); + + // Node 0 calls splice_channel before any pending splice exists. + let floor_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + assert!(funding_template.min_rbf_feerate().is_none()); + + // Build contribution at floor feerate with high max_feerate to allow adjustment. + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let contribution = + funding_template.splice_in_sync(added_value, floor_feerate, FeeRate::MAX, &wallet).unwrap(); + + // Node 1 initiates and completes a splice, creating pending_splice with negotiated candidates. + let node_1_contribution = do_initiate_splice_in(&nodes[1], &nodes[0], channel_id, added_value); + let (_first_splice_tx, _new_funding_script) = + splice_channel(&nodes[1], &nodes[0], channel_id, node_1_contribution); + + // Node 0 calls funding_contributed. The contribution's feerate (floor) is below the RBF + // floor (25/24 of floor), but funding_contributed adjusts it upward. + nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution.clone(), None).unwrap(); + + // STFU should be sent immediately (the adjusted feerate satisfies the RBF check). + let stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + nodes[1].node.handle_stfu(node_id_0, &stfu); + let stfu_resp = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + nodes[0].node.handle_stfu(node_id_1, &stfu_resp); + + // Verify the RBF handshake proceeds. + let tx_init_rbf = get_event_msg!(nodes[0], MessageSendEvent::SendTxInitRbf, node_id_1); + let rbf_feerate = FeeRate::from_sat_per_kwu(tx_init_rbf.feerate_sat_per_1000_weight as u64); + let expected_floor = + FeeRate::from_sat_per_kwu((FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24)); + assert!(rbf_feerate >= expected_floor); +} + +#[test] +fn test_funding_contributed_rbf_adjustment_exceeds_max_feerate() { + // Test that when the minimum RBF feerate exceeds max_feerate, the adjustment in funding_contributed + // fails gracefully and the contribution keeps its original feerate. The splice still + // proceeds (STFU is sent) and the RBF negotiation handles the feerate mismatch. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 4, added_value * 2); + + // Node 0 calls splice_channel and builds contribution with max_feerate = floor_feerate. + // This means the minimum RBF feerate (25/24 of floor) will exceed max_feerate, preventing adjustment. + let floor_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let contribution = funding_template + .splice_in_sync(added_value, floor_feerate, floor_feerate, &wallet) + .unwrap(); + + // Node 1 initiates and completes a splice. + let node_1_contribution = do_initiate_splice_in(&nodes[1], &nodes[0], channel_id, added_value); + let (_splice_tx, _) = splice_channel(&nodes[1], &nodes[0], channel_id, node_1_contribution); + + // Node 0 calls funding_contributed. The adjustment fails (minimum RBF feerate > max_feerate), but + // funding_contributed still succeeds — the contribution keeps its original feerate. + nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution, None).unwrap(); + + // STFU is NOT sent — the feerate is below the minimum RBF feerate so try_send_stfu delays. + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + + // Mine and lock the pending splice → pending_splice is cleared. + mine_transaction(&nodes[0], &_splice_tx); + mine_transaction(&nodes[1], &_splice_tx); + let stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); + + // STFU is sent during lock — the splice proceeds as a fresh splice (not RBF). + let stfu = match stfu { + Some(MessageSendEvent::SendStfu { msg, .. }) => { + assert!(msg.initiator); + msg + }, + other => panic!("Expected SendStfu, got {:?}", other), + }; + + // Complete the fresh splice and verify it uses the original floor feerate. + nodes[1].node.handle_stfu(node_id_0, &stfu); + let stfu_resp = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + nodes[0].node.handle_stfu(node_id_1, &stfu_resp); + + let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); + assert_eq!(splice_init.funding_feerate_per_kw, FEERATE_FLOOR_SATS_PER_KW); +} + +#[test] +fn test_funding_contributed_rbf_adjustment_insufficient_budget() { + // Test that when the change output can't absorb the fee increase needed for the minimum RBF feerate + // (even though max_feerate allows it), the adjustment fails gracefully and the splice + // proceeds with the original feerate. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 4, added_value * 2); + + // Node 0 calls splice_channel before any pending splice exists. + let floor_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + + // Build node 0's contribution at floor feerate with a tight budget. + let wallet = TightBudgetWallet { + utxo_value: added_value + Amount::from_sat(3000), + change_value: Amount::from_sat(300), + }; + let contribution = + funding_template.splice_in_sync(added_value, floor_feerate, FeeRate::MAX, &wallet).unwrap(); + + // Node 1 initiates a splice at a HIGH feerate (10,000 sat/kwu). The minimum RBF feerate will be + // 25/24 of 10,000 = 10,417 sat/kwu — far above what node 0's tight budget can handle. + let high_feerate = FeeRate::from_sat_per_kwu(10_000); + let node_1_template = nodes[1].node.splice_channel(&channel_id, &node_id_0).unwrap(); + let node_1_wallet = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); + let node_1_contribution = node_1_template + .splice_in_sync(added_value, high_feerate, FeeRate::MAX, &node_1_wallet) + .unwrap(); + nodes[1] + .node + .funding_contributed(&channel_id, &node_id_0, node_1_contribution.clone(), None) + .unwrap(); + let (_splice_tx, _) = splice_channel(&nodes[1], &nodes[0], channel_id, node_1_contribution); + + // Node 0 calls funding_contributed. Adjustment fails (insufficient fee buffer), so the + // contribution keeps its original feerate. + nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution, None).unwrap(); + + // STFU is NOT sent — the feerate is below the minimum RBF feerate so try_send_stfu delays. + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + + // Mine and lock the pending splice → pending_splice is cleared. + mine_transaction(&nodes[0], &_splice_tx); + mine_transaction(&nodes[1], &_splice_tx); + let stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); + + // STFU is sent during lock — the splice proceeds as a fresh splice (not RBF). + let stfu = match stfu { + Some(MessageSendEvent::SendStfu { msg, .. }) => { + assert!(msg.initiator); + msg + }, + other => panic!("Expected SendStfu, got {:?}", other), + }; + + // Complete the fresh splice and verify it uses the original floor feerate. + nodes[1].node.handle_stfu(node_id_0, &stfu); + let stfu_resp = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + nodes[0].node.handle_stfu(node_id_1, &stfu_resp); + + let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); + assert_eq!(splice_init.funding_feerate_per_kw, FEERATE_FLOOR_SATS_PER_KW); +} From 8fd49271fe13e751378c351418e30d6928327427 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Sun, 15 Mar 2026 21:50:54 -0500 Subject: [PATCH 3/7] Merge rbf_channel into splice_channel and expose prior contribution Users previously had to choose between splice_channel (fresh splice) and rbf_channel (fee bump) upfront. Since splice_channel already detects pending splices and computes the minimum RBF feerate, rbf_channel was redundant. Merging into a single API lets the user call one method and discover from the returned FundingTemplate whether an RBF is possible. The FundingTemplate now carries the user's prior contribution from the previous splice negotiation when one is available. This lets users reuse their existing contribution for an RBF without performing new coin selection. A PriorContribution enum distinguishes whether the contribution has been adjusted to the minimum RBF feerate (Adjusted) or could not be adjusted due to insufficient fee buffer or max_feerate constraints (Unadjusted). Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 144 ++++----- lightning/src/ln/channelmanager.rs | 85 +----- lightning/src/ln/funding.rs | 476 +++++++++++++++++++++++++++-- lightning/src/ln/splicing_tests.rs | 256 +++++++++++++--- lightning/src/util/wallet_utils.rs | 2 +- 5 files changed, 749 insertions(+), 214 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 32247108ece..ff47306edf3 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -56,7 +56,7 @@ use crate::ln::channelmanager::{ MAX_LOCAL_BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, }; use crate::ln::funding::{ - FeeRateAdjustmentError, FundingContribution, FundingTemplate, FundingTxInput, + FeeRateAdjustmentError, FundingContribution, FundingTemplate, FundingTxInput, PriorContribution, }; use crate::ln::interactivetxs::{ AbortReason, HandleTxCompleteValue, InteractiveTxConstructor, InteractiveTxConstructorArgs, @@ -11907,7 +11907,7 @@ where } } - /// Initiate splicing. + /// Builds a [`FundingTemplate`] for splicing or RBF, if the channel state allows it. pub fn splice_channel(&self) -> Result { if self.holder_commitment_point.current_point().is_none() { return Err(APIError::APIMisuseError { @@ -11950,19 +11950,35 @@ where }); } - // Compute the RBF feerate floor from either negotiated candidates (via - // can_initiate_rbf) or an in-progress funding negotiation (which will become a - // negotiated candidate once it completes). - let min_rbf_feerate = self.can_initiate_rbf().ok().flatten().or_else(|| { - self.pending_splice - .as_ref() - .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()) - .map(|negotiation| { - let prev_feerate = negotiation.funding_feerate_sat_per_1000_weight(); - let min_feerate_kwu = ((prev_feerate as u64) * 25).div_ceil(24); - FeeRate::from_sat_per_kwu(min_feerate_kwu) - }) - }); + let (min_rbf_feerate, prior_contribution) = if self.is_rbf_compatible().is_err() { + // Channel can never RBF (e.g., zero-conf). + (None, None) + } else if let Ok(min_rbf_feerate) = self.can_initiate_rbf() { + // A previous splice was negotiated but not yet locked. The user's splice + // will be an RBF, so provide the minimum RBF feerate and prior contribution. + let prior = self.build_prior_contribution(); + (Some(min_rbf_feerate), prior) + } else if let Some(negotiation) = self + .pending_splice + .as_ref() + .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()) + { + // A splice is currently being negotiated. + // - If the negotiation succeeds, the user's splice will need to satisfy the RBF + // feerate requirement. Derive the minimum RBF feerate from the negotiation's + // feerate so the user can choose an appropriate feerate. + // - If the negotiation fails (e.g., tx_abort), the splice will proceed as a fresh + // splice instead. In this case, the min_rbf_feerate becomes stale, causing a + // slightly higher feerate than necessary. Call splice_channel again after + // receiving SpliceFailed to get a fresh template without the RBF constraint. + let prev_feerate = negotiation.funding_feerate_sat_per_1000_weight(); + let min_feerate_kwu = ((prev_feerate as u64) * 25).div_ceil(24); + (Some(FeeRate::from_sat_per_kwu(min_feerate_kwu)), None) + } else { + // No RBF feerate to derive — either a fresh splice or a pending splice that + // can't be RBF'd (e.g., splice_locked already exchanged). + (None, None) + }; let funding_txo = self.funding.get_funding_txo().expect("funding_txo should be set"); let previous_utxo = @@ -11973,63 +11989,35 @@ where satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + FUNDING_TRANSACTION_WITNESS_WEIGHT, }; - Ok(FundingTemplate::new(Some(shared_input), min_rbf_feerate)) + Ok(FundingTemplate::new(Some(shared_input), min_rbf_feerate, prior_contribution)) } - /// Initiate an RBF of a pending splice transaction. - pub fn rbf_channel(&self) -> Result { - if self.holder_commitment_point.current_point().is_none() { - return Err(APIError::APIMisuseError { - err: format!( - "Channel {} cannot RBF until a payment is routed", - self.context.channel_id(), - ), - }); - } - - if self.quiescent_action.is_some() { - return Err(APIError::APIMisuseError { - err: format!( - "Channel {} cannot RBF as one is waiting to be negotiated", - self.context.channel_id(), - ), - }); - } - - if !self.context.is_usable() { - return Err(APIError::APIMisuseError { - err: format!( - "Channel {} cannot RBF as it is either pending open/close", - self.context.channel_id() - ), - }); - } + /// Clones the prior contribution and fetches the holder balance for deferred feerate + /// adjustment. + fn build_prior_contribution(&self) -> Option { + debug_assert!(self.pending_splice.is_some(), "can_initiate_rbf requires pending_splice"); + let prior = self.pending_splice.as_ref()?.contributions.last()?; + let holder_balance = self + .get_holder_counterparty_balances_floor_incl_fee(&self.funding) + .map(|(h, _)| h) + .ok(); + Some(PriorContribution::new(prior.clone(), holder_balance)) + } + /// Returns whether this channel can ever RBF, independent of splice state. + fn is_rbf_compatible(&self) -> Result<(), String> { if self.context.minimum_depth(&self.funding) == Some(0) { - return Err(APIError::APIMisuseError { - err: format!( - "Channel {} has option_zeroconf, cannot RBF splice", - self.context.channel_id(), - ), - }); + return Err(format!( + "Channel {} has option_zeroconf, cannot RBF", + self.context.channel_id(), + )); } - - let min_rbf_feerate = - self.can_initiate_rbf().map_err(|err| APIError::APIMisuseError { err })?; - - let funding_txo = self.funding.get_funding_txo().expect("funding_txo should be set"); - let previous_utxo = - self.funding.get_funding_output().expect("funding_output should be set"); - let shared_input = Input { - outpoint: funding_txo.into_bitcoin_outpoint(), - previous_utxo, - satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + FUNDING_TRANSACTION_WITNESS_WEIGHT, - }; - - Ok(FundingTemplate::new(Some(shared_input), min_rbf_feerate)) + Ok(()) } - fn can_initiate_rbf(&self) -> Result, String> { + fn can_initiate_rbf(&self) -> Result { + self.is_rbf_compatible()?; + let pending_splice = match &self.pending_splice { Some(pending_splice) => pending_splice, None => { @@ -12068,13 +12056,16 @@ where )); } - let min_rbf_feerate = - pending_splice.last_funding_feerate_sat_per_1000_weight.map(|prev_feerate| { + match pending_splice.last_funding_feerate_sat_per_1000_weight { + Some(prev_feerate) => { let min_feerate_kwu = ((prev_feerate as u64) * 25).div_ceil(24); - FeeRate::from_sat_per_kwu(min_feerate_kwu) - }); - - Ok(min_rbf_feerate) + Ok(FeeRate::from_sat_per_kwu(min_feerate_kwu)) + }, + None => Err(format!( + "Channel {} has no prior feerate to compute RBF minimum", + self.context.channel_id(), + )), + } } /// Attempts to adjust the contribution's feerate to the minimum RBF feerate so the splice can @@ -12205,7 +12196,7 @@ where // If a pending splice exists with negotiated candidates, attempt to adjust the // contribution's feerate to the minimum RBF feerate so it can proceed as an RBF immediately // rather than waiting for the splice to lock. - let contribution = if let Ok(Some(min_rbf_feerate)) = self.can_initiate_rbf() { + let contribution = if let Ok(min_rbf_feerate) = self.can_initiate_rbf() { self.maybe_adjust_for_rbf(contribution, min_rbf_feerate, logger) } else { contribution @@ -12605,12 +12596,7 @@ where return Err(ChannelError::WarnAndDisconnect("Quiescence needed for RBF".to_owned())); } - if self.context.minimum_depth(&self.funding) == Some(0) { - return Err(ChannelError::WarnAndDisconnect(format!( - "Channel {} has option_zeroconf, cannot RBF splice", - self.context.channel_id(), - ))); - } + self.is_rbf_compatible().map_err(|msg| ChannelError::WarnAndDisconnect(msg))?; let pending_splice = match &self.pending_splice { Some(pending_splice) => pending_splice, @@ -13817,7 +13803,7 @@ where ); return None; }, - Ok(Some(min_rbf_feerate)) if contribution.feerate() < min_rbf_feerate => { + Ok(min_rbf_feerate) if contribution.feerate() < min_rbf_feerate => { log_given_level!( logger, logger_level, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 223d74ce780..aa8c091d6ba 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4701,8 +4701,7 @@ impl< } /// Initiate a splice in order to add value to (splice-in) or remove value from (splice-out) - /// the channel. This will spend the channel's funding transaction output, effectively replacing - /// it with a new one. + /// the channel, or to RBF a pending splice transaction. /// /// # Required Feature Flags /// @@ -4710,15 +4709,13 @@ impl< /// channel (no matter the type) can be spliced, as long as the counterparty is currently /// connected. /// - /// Returns a [`FundingTemplate`] which should be used to build a [`FundingContribution`] via - /// one of its splice methods (e.g., [`FundingTemplate::splice_in_sync`]). The `min_feerate` - /// and `max_feerate` parameters are provided when calling those splice methods. The resulting - /// contribution must then be passed to [`ChannelManager::funding_contributed`]. + /// # Return Value /// - /// When a pending splice exists with negotiated candidates (i.e., a splice that hasn't been - /// locked yet), [`FundingTemplate::min_rbf_feerate`] will return the minimum feerate required - /// for an RBF attempt (25/24 of the previous feerate). This can be used to choose an - /// appropriate `min_feerate` when calling the splice methods. + /// Returns a [`FundingTemplate`] which should be used to obtain a [`FundingContribution`] + /// to pass to [`ChannelManager::funding_contributed`]. If a splice has been negotiated but + /// not yet locked, it can be replaced with a higher feerate transaction to speed up + /// confirmation via Replace By Fee (RBF). See [`FundingTemplate`] for details on building + /// a fresh contribution or reusing a prior one for RBF. #[rustfmt::skip] pub fn splice_channel( &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, @@ -4765,67 +4762,6 @@ impl< } } - /// Initiate an RBF of a pending splice transaction for an existing channel. - /// - /// This is used after a splice has been negotiated but before it has been locked, in order - /// to bump the feerate of the funding transaction via replace-by-fee. - /// - /// # Required Feature Flags - /// - /// Initiating an RBF requires that the channel counterparty supports splicing. The - /// counterparty must be currently connected. - /// - /// Returns a [`FundingTemplate`] which should be used to build a [`FundingContribution`] via - /// one of its splice methods (e.g., [`FundingTemplate::splice_in_sync`]). The `min_feerate` - /// and `max_feerate` parameters are provided when calling those splice methods. - /// [`FundingTemplate::min_rbf_feerate`] returns the minimum feerate required for the RBF - /// (25/24 of the previous feerate). The resulting contribution must then be passed to - /// [`ChannelManager::funding_contributed`]. - pub fn rbf_channel( - &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, - ) -> Result { - let per_peer_state = self.per_peer_state.read().unwrap(); - - let peer_state_mutex = match per_peer_state - .get(counterparty_node_id) - .ok_or_else(|| APIError::no_such_peer(counterparty_node_id)) - { - Ok(p) => p, - Err(e) => return Err(e), - }; - - let mut peer_state = peer_state_mutex.lock().unwrap(); - if !peer_state.latest_features.supports_splicing() { - return Err(APIError::ChannelUnavailable { - err: "Peer does not support splicing".to_owned(), - }); - } - if !peer_state.latest_features.supports_quiescence() { - return Err(APIError::ChannelUnavailable { - err: "Peer does not support quiescence, a splicing prerequisite".to_owned(), - }); - } - - // Look for the channel - match peer_state.channel_by_id.entry(*channel_id) { - hash_map::Entry::Occupied(chan_phase_entry) => { - if let Some(chan) = chan_phase_entry.get().as_funded() { - chan.rbf_channel() - } else { - Err(APIError::ChannelUnavailable { - err: format!( - "Channel with id {} is not funded, cannot RBF splice", - channel_id - ), - }) - } - }, - hash_map::Entry::Vacant(_) => { - Err(APIError::no_such_channel_for_peer(channel_id, counterparty_node_id)) - }, - } - } - #[cfg(test)] pub(crate) fn abandon_splice( &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, @@ -6590,13 +6526,16 @@ impl< /// /// If any failures occur while negotiating the funding transaction, an [`Event::SpliceFailed`] /// will be emitted. Any contributed inputs no longer used will be included in an - /// [`Event::DiscardFunding`] and thus can be re-spent. + /// [`Event::DiscardFunding`] and thus can be re-spent. If a [`FundingTemplate`] was obtained + /// while a previous splice was still being negotiated, its + /// [`min_rbf_feerate`][FundingTemplate::min_rbf_feerate] may be stale after the failure. + /// Call this method again to get a fresh template. /// /// After initial signatures have been exchanged, [`Event::FundingTransactionReadyForSigning`] /// will be generated and [`ChannelManager::funding_transaction_signed`] should be called. /// /// Once the splice has been locked by both counterparties, an [`Event::ChannelReady`] will be - /// emitted with the new funding output. At this point, a new splice can be negotiated by + /// emitted with the new funding output. At this point, a new (non-RBF) splice can be negotiated by /// calling [`ChannelManager::splice_channel`] again on this channel. /// /// # Errors diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index acad13c32ae..35f6a16cd0f 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -106,12 +106,58 @@ impl core::fmt::Display for FeeRateAdjustmentError { } } +/// The user's prior contribution from a previous splice negotiation on this channel. +/// +/// When a pending splice exists with negotiated candidates, the prior contribution is +/// available for reuse (e.g., to bump the feerate via RBF). Contains the raw contribution and +/// the holder's balance for deferred feerate adjustment in [`FundingTemplate::rbf_sync`] or +/// [`FundingTemplate::rbf`]. +/// +/// Use [`FundingTemplate::prior_contribution`] to inspect the prior contribution before +/// deciding whether to call [`FundingTemplate::rbf_sync`] or one of the splice methods +/// with different parameters. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) struct PriorContribution { + contribution: FundingContribution, + /// The holder's balance, used for feerate adjustment. `None` when the balance computation + /// fails, in which case adjustment is skipped and coin selection is re-run. + holder_balance: Option, +} + +impl PriorContribution { + pub(super) fn new(contribution: FundingContribution, holder_balance: Option) -> Self { + Self { contribution, holder_balance } + } +} + /// A template for contributing to a channel's splice funding transaction. /// /// This is returned from [`ChannelManager::splice_channel`] when a channel is ready to be -/// spliced. It must be converted to a [`FundingContribution`] using one of the splice methods -/// and passed to [`ChannelManager::funding_contributed`] in order to resume the splicing -/// process. +/// spliced. A [`FundingContribution`] must be obtained from it and passed to +/// [`ChannelManager::funding_contributed`] in order to resume the splicing process. +/// +/// # Building a Contribution +/// +/// For a fresh splice (no pending splice to replace), build a new contribution using one of +/// the splice methods: +/// - [`FundingTemplate::splice_in_sync`] to add funds to the channel +/// - [`FundingTemplate::splice_out_sync`] to remove funds from the channel +/// - [`FundingTemplate::splice_in_and_out_sync`] to do both +/// +/// These perform coin selection and require `min_feerate` and `max_feerate` parameters. +/// +/// # Replace By Fee (RBF) +/// +/// When a pending splice exists that hasn't been locked yet, use [`FundingTemplate::rbf_sync`] +/// (or [`FundingTemplate::rbf`] for async) to build an RBF contribution. This handles the +/// prior contribution logic internally — reusing an adjusted prior when possible, re-running +/// coin selection when needed, or creating a fee-bump-only contribution. +/// +/// Check [`FundingTemplate::min_rbf_feerate`] for the minimum feerate required (25/24 of +/// the previous feerate). Use [`FundingTemplate::prior_contribution`] to inspect the prior +/// contribution's parameters (e.g., [`FundingContribution::value_added`], +/// [`FundingContribution::outputs`]) before deciding whether to reuse it via the RBF methods +/// or build a fresh contribution with different parameters using the splice methods above. /// /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel /// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed @@ -124,12 +170,18 @@ pub struct FundingTemplate { /// The minimum RBF feerate (25/24 of the previous feerate), if this template is for an /// RBF attempt. `None` for fresh splices with no pending splice candidates. min_rbf_feerate: Option, + + /// The user's prior contribution from a previous splice negotiation, if available. + prior_contribution: Option, } impl FundingTemplate { /// Constructs a [`FundingTemplate`] for a splice using the provided shared input. - pub(super) fn new(shared_input: Option, min_rbf_feerate: Option) -> Self { - Self { shared_input, min_rbf_feerate } + pub(super) fn new( + shared_input: Option, min_rbf_feerate: Option, + prior_contribution: Option, + ) -> Self { + Self { shared_input, min_rbf_feerate, prior_contribution } } /// Returns the minimum RBF feerate, if this template is for an RBF attempt. @@ -139,16 +191,28 @@ impl FundingTemplate { pub fn min_rbf_feerate(&self) -> Option { self.min_rbf_feerate } + + /// Returns a reference to the prior contribution from a previous splice negotiation, if + /// available. + /// + /// Use this to inspect the prior contribution's parameters (e.g., + /// [`FundingContribution::value_added`], [`FundingContribution::outputs`]) before deciding + /// whether to reuse it via [`FundingTemplate::rbf_sync`] or build a fresh contribution + /// with different parameters using the splice methods. + pub fn prior_contribution(&self) -> Option<&FundingContribution> { + self.prior_contribution.as_ref().map(|p| &p.contribution) + } } macro_rules! build_funding_contribution { - ($value_added:expr, $outputs:expr, $shared_input:expr, $min_rbf_feerate:expr, $feerate:expr, $max_feerate:expr, $wallet:ident, $($await:tt)*) => {{ + ($value_added:expr, $outputs:expr, $shared_input:expr, $min_rbf_feerate:expr, $feerate:expr, $max_feerate:expr, $force_coin_selection:expr, $wallet:ident, $($await:tt)*) => {{ let value_added: Amount = $value_added; let outputs: Vec = $outputs; let shared_input: Option = $shared_input; let min_rbf_feerate: Option = $min_rbf_feerate; let feerate: FeeRate = $feerate; let max_feerate: FeeRate = $max_feerate; + let force_coin_selection: bool = $force_coin_selection; if feerate > max_feerate { return Err(()); @@ -178,7 +242,7 @@ macro_rules! build_funding_contribution { let is_splice = shared_input.is_some(); - let coin_selection = if value_added == Amount::ZERO { + let coin_selection = if value_added == Amount::ZERO && !force_coin_selection { CoinSelection { confirmed_utxos: vec![], change_output: None } } else { // Used for creating a redeem script for the new funding txo, since the funding pubkeys @@ -237,25 +301,32 @@ macro_rules! build_funding_contribution { impl FundingTemplate { /// Creates a [`FundingContribution`] for adding funds to a channel using `wallet` to perform /// coin selection. + /// + /// `value_added` is the total amount to add to the channel for this contribution. When + /// replacing a prior contribution via RBF, use [`FundingTemplate::prior_contribution`] to + /// inspect the prior parameters. To add funds on top of the prior contribution's amount, + /// combine them: `prior.value_added() + additional_amount`. pub async fn splice_in( self, value_added: Amount, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, ) -> Result { if value_added == Amount::ZERO { return Err(()); } - let FundingTemplate { shared_input, min_rbf_feerate } = self; - build_funding_contribution!(value_added, vec![], shared_input, min_rbf_feerate, min_feerate, max_feerate, wallet, await) + let FundingTemplate { shared_input, min_rbf_feerate, .. } = self; + build_funding_contribution!(value_added, vec![], shared_input, min_rbf_feerate, min_feerate, max_feerate, false, wallet, await) } /// Creates a [`FundingContribution`] for adding funds to a channel using `wallet` to perform /// coin selection. + /// + /// See [`FundingTemplate::splice_in`] for details. pub fn splice_in_sync( self, value_added: Amount, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, ) -> Result { if value_added == Amount::ZERO { return Err(()); } - let FundingTemplate { shared_input, min_rbf_feerate } = self; + let FundingTemplate { shared_input, min_rbf_feerate, .. } = self; build_funding_contribution!( value_added, vec![], @@ -263,31 +334,39 @@ impl FundingTemplate { min_rbf_feerate, min_feerate, max_feerate, + false, wallet, ) } /// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to /// perform coin selection. + /// + /// `outputs` are the complete set of withdrawal outputs for this contribution. When + /// replacing a prior contribution via RBF, use [`FundingTemplate::prior_contribution`] to + /// inspect the prior parameters. To keep existing withdrawals and add new ones, include the + /// prior's outputs: combine [`FundingContribution::outputs`] with the new outputs. pub async fn splice_out( self, outputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, ) -> Result { if outputs.is_empty() { return Err(()); } - let FundingTemplate { shared_input, min_rbf_feerate } = self; - build_funding_contribution!(Amount::ZERO, outputs, shared_input, min_rbf_feerate, min_feerate, max_feerate, wallet, await) + let FundingTemplate { shared_input, min_rbf_feerate, .. } = self; + build_funding_contribution!(Amount::ZERO, outputs, shared_input, min_rbf_feerate, min_feerate, max_feerate, false, wallet, await) } /// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to /// perform coin selection. + /// + /// See [`FundingTemplate::splice_out`] for details. pub fn splice_out_sync( self, outputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, ) -> Result { if outputs.is_empty() { return Err(()); } - let FundingTemplate { shared_input, min_rbf_feerate } = self; + let FundingTemplate { shared_input, min_rbf_feerate, .. } = self; build_funding_contribution!( Amount::ZERO, outputs, @@ -295,12 +374,18 @@ impl FundingTemplate { min_rbf_feerate, min_feerate, max_feerate, + false, wallet, ) } /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using /// `wallet` to perform coin selection. + /// + /// `value_added` and `outputs` are the complete parameters for this contribution, not + /// increments on top of a prior contribution. When replacing a prior contribution via RBF, + /// use [`FundingTemplate::prior_contribution`] to inspect the prior parameters and combine + /// them as needed. pub async fn splice_in_and_out( self, value_added: Amount, outputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, @@ -308,12 +393,14 @@ impl FundingTemplate { if value_added == Amount::ZERO && outputs.is_empty() { return Err(()); } - let FundingTemplate { shared_input, min_rbf_feerate } = self; - build_funding_contribution!(value_added, outputs, shared_input, min_rbf_feerate, min_feerate, max_feerate, wallet, await) + let FundingTemplate { shared_input, min_rbf_feerate, .. } = self; + build_funding_contribution!(value_added, outputs, shared_input, min_rbf_feerate, min_feerate, max_feerate, false, wallet, await) } /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using /// `wallet` to perform coin selection. + /// + /// See [`FundingTemplate::splice_in_and_out`] for details. pub fn splice_in_and_out_sync( self, value_added: Amount, outputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, @@ -321,7 +408,7 @@ impl FundingTemplate { if value_added == Amount::ZERO && outputs.is_empty() { return Err(()); } - let FundingTemplate { shared_input, min_rbf_feerate } = self; + let FundingTemplate { shared_input, min_rbf_feerate, .. } = self; build_funding_contribution!( value_added, outputs, @@ -329,9 +416,118 @@ impl FundingTemplate { min_rbf_feerate, min_feerate, max_feerate, + false, wallet, ) } + + /// Creates a [`FundingContribution`] for an RBF (Replace-By-Fee) attempt on a pending splice. + /// + /// `max_feerate` is the maximum feerate the caller is willing to accept as acceptor. It is + /// used as the returned contribution's `max_feerate` and also constrains coin selection when + /// re-running it for prior contributions that cannot be adjusted or fee-bump-only + /// contributions. + /// + /// This handles the prior contribution logic internally: + /// - If the prior contribution's feerate can be adjusted to the minimum RBF feerate using + /// the holder's balance, the adjusted contribution is returned directly. If adjustment + /// fails or the balance is unavailable, coin selection is re-run using the prior + /// contribution's parameters and the caller's `max_feerate`. + /// - If no prior contribution exists, coin selection is run for a fee-bump-only contribution + /// (`value_added = 0`), covering fees for the common fields and shared input/output via + /// a newly selected input. Check [`FundingTemplate::prior_contribution`] to see if this + /// is intended. + /// + /// Returns `Err(())` if this is not an RBF scenario ([`FundingTemplate::min_rbf_feerate`] + /// is `None`) or if `max_feerate` is below the minimum RBF feerate. + pub async fn rbf( + self, max_feerate: FeeRate, wallet: W, + ) -> Result { + let FundingTemplate { shared_input, min_rbf_feerate, prior_contribution } = self; + let rbf_feerate = min_rbf_feerate.ok_or(())?; + if rbf_feerate > max_feerate { + return Err(()); + } + + match prior_contribution { + Some(PriorContribution { contribution, holder_balance }) => { + // The prior contribution's feerate is the negotiated feerate from the + // previous splice, which is always below the RBF minimum (negotiated + 25). + debug_assert!(contribution.feerate < rbf_feerate); + if let Some(holder_balance) = holder_balance { + if contribution + .net_value_for_initiator_at_feerate(rbf_feerate, holder_balance) + .is_ok() + { + let mut adjusted = contribution + .for_initiator_at_feerate(rbf_feerate, holder_balance) + .expect("feerate compatibility already checked"); + adjusted.max_feerate = max_feerate; + return Ok(adjusted); + } + } + build_funding_contribution!(contribution.value_added, contribution.outputs, shared_input, min_rbf_feerate, rbf_feerate, max_feerate, true, wallet, await) + }, + None => { + build_funding_contribution!(Amount::ZERO, vec![], shared_input, min_rbf_feerate, rbf_feerate, max_feerate, true, wallet, await) + }, + } + } + + /// Creates a [`FundingContribution`] for an RBF (Replace-By-Fee) attempt on a pending splice. + /// + /// See [`FundingTemplate::rbf`] for details. + pub fn rbf_sync( + self, max_feerate: FeeRate, wallet: W, + ) -> Result { + let FundingTemplate { shared_input, min_rbf_feerate, prior_contribution } = self; + let rbf_feerate = min_rbf_feerate.ok_or(())?; + if rbf_feerate > max_feerate { + return Err(()); + } + + match prior_contribution { + Some(PriorContribution { contribution, holder_balance }) => { + // The prior contribution's feerate is the negotiated feerate from the + // previous splice, which is always below the RBF minimum (negotiated + 25). + debug_assert!(contribution.feerate < rbf_feerate); + if let Some(holder_balance) = holder_balance { + if contribution + .net_value_for_initiator_at_feerate(rbf_feerate, holder_balance) + .is_ok() + { + let mut adjusted = contribution + .for_initiator_at_feerate(rbf_feerate, holder_balance) + .expect("feerate compatibility already checked"); + adjusted.max_feerate = max_feerate; + return Ok(adjusted); + } + } + build_funding_contribution!( + contribution.value_added, + contribution.outputs, + shared_input, + min_rbf_feerate, + rbf_feerate, + max_feerate, + true, + wallet, + ) + }, + None => { + build_funding_contribution!( + Amount::ZERO, + vec![], + shared_input, + min_rbf_feerate, + rbf_feerate, + max_feerate, + true, + wallet, + ) + }, + } + } } fn estimate_transaction_fee( @@ -385,7 +581,7 @@ fn estimate_transaction_fee( } /// The components of a funding transaction contributed by one party. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct FundingContribution { /// The amount to contribute to the channel. /// @@ -445,6 +641,18 @@ impl FundingContribution { self.outputs.iter().chain(self.change_output.iter()) } + /// Returns the amount added to the channel by this contribution. + pub fn value_added(&self) -> Amount { + self.value_added + } + + /// Returns the outputs (e.g., withdrawal destinations) included in this contribution. + /// + /// This does not include the change output; see [`FundingContribution::change_output`]. + pub fn outputs(&self) -> &[TxOut] { + &self.outputs + } + /// Returns the change output included in this contribution, if any. /// /// When coin selection provides more value than needed for the funding contribution and fees, @@ -727,8 +935,8 @@ impl FundingContribution { /// Adjusts the contribution's change output for the minimum RBF feerate. /// /// When a pending splice exists with negotiated candidates and the contribution's feerate - /// is below the minimum RBF feerate (25/24 of the previous feerate), this adjusts the change output - /// so the initiator pays fees at the minimum RBF feerate. + /// is below the minimum RBF feerate (25/24 of the previous feerate), this adjusts the + /// change output so the initiator pays fees at the minimum RBF feerate. pub(super) fn for_initiator_at_feerate( self, feerate: FeeRate, holder_balance: Amount, ) -> Result { @@ -833,7 +1041,7 @@ pub type FundingTxInput = crate::util::wallet_utils::ConfirmedUtxo; mod tests { use super::{ estimate_transaction_fee, FeeRateAdjustmentError, FundingContribution, FundingTemplate, - FundingTxInput, + FundingTxInput, PriorContribution, }; use crate::chain::ClaimId; use crate::util::wallet_utils::{CoinSelection, CoinSelectionSourceSync, Input}; @@ -1142,7 +1350,7 @@ mod tests { // splice_in_sync with value_added > MAX_MONEY { - let template = FundingTemplate::new(None, None); + let template = FundingTemplate::new(None, None, None); assert!(template .splice_in_sync(over_max, feerate, feerate, UnreachableWallet) .is_err()); @@ -1150,7 +1358,7 @@ mod tests { // splice_out_sync with single output value > MAX_MONEY { - let template = FundingTemplate::new(None, None); + let template = FundingTemplate::new(None, None, None); let outputs = vec![funding_output_sats(over_max.to_sat())]; assert!(template .splice_out_sync(outputs, feerate, feerate, UnreachableWallet) @@ -1159,7 +1367,7 @@ mod tests { // splice_out_sync with multiple outputs summing > MAX_MONEY { - let template = FundingTemplate::new(None, None); + let template = FundingTemplate::new(None, None, None); let half_over = Amount::MAX_MONEY / 2 + Amount::from_sat(1); let outputs = vec![ funding_output_sats(half_over.to_sat()), @@ -1172,7 +1380,7 @@ mod tests { // splice_in_and_out_sync with value_added > MAX_MONEY { - let template = FundingTemplate::new(None, None); + let template = FundingTemplate::new(None, None, None); let outputs = vec![funding_output_sats(1_000)]; assert!(template .splice_in_and_out_sync(over_max, outputs, feerate, feerate, UnreachableWallet) @@ -1181,7 +1389,7 @@ mod tests { // splice_in_and_out_sync with output sum > MAX_MONEY { - let template = FundingTemplate::new(None, None); + let template = FundingTemplate::new(None, None, None); let outputs = vec![funding_output_sats(over_max.to_sat())]; assert!(template .splice_in_and_out_sync( @@ -1202,7 +1410,7 @@ mod tests { // min_feerate > max_feerate is rejected { - let template = FundingTemplate::new(None, None); + let template = FundingTemplate::new(None, None, None); assert!(template .splice_in_sync(Amount::from_sat(10_000), high, low, UnreachableWallet) .is_err()); @@ -1210,7 +1418,7 @@ mod tests { // min_feerate < min_rbf_feerate is rejected { - let template = FundingTemplate::new(None, Some(high)); + let template = FundingTemplate::new(None, Some(high), None); assert!(template .splice_in_sync(Amount::from_sat(10_000), low, FeeRate::MAX, UnreachableWallet) .is_err()); @@ -1947,4 +2155,218 @@ mod tests { assert_eq!(initiator.feerate, target_feerate); assert_eq!(acceptor.feerate, target_feerate); } + + #[test] + fn test_rbf_sync_rejects_max_feerate_below_min_rbf_feerate() { + // When the caller's max_feerate is below the minimum RBF feerate, rbf_sync should + // return Err(()). + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(5000); + let max_feerate = FeeRate::from_sat_per_kwu(3000); + + let prior = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee: Amount::from_sat(1_000), + inputs: vec![funding_input_sats(100_000)], + outputs: vec![], + change_output: None, + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + // max_feerate (3000) < min_rbf_feerate (5000). + let template = FundingTemplate::new( + None, + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, None)), + ); + assert!(template.rbf_sync(max_feerate, UnreachableWallet).is_err()); + } + + #[test] + fn test_rbf_sync_adjusts_prior_to_rbf_feerate() { + // When the prior contribution's feerate is below the minimum RBF feerate and holder + // balance is available, rbf_sync should adjust the prior to the RBF feerate. + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); + let max_feerate = FeeRate::from_sat_per_kwu(5000); + + let inputs = vec![funding_input_sats(100_000)]; + let change = funding_output_sats(10_000); + let estimated_fee = + estimate_transaction_fee(&inputs, &[], Some(&change), true, true, prior_feerate); + + let prior = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee, + inputs, + outputs: vec![], + change_output: Some(change), + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let template = FundingTemplate::new( + None, + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, Some(Amount::MAX))), + ); + let contribution = template.rbf_sync(max_feerate, UnreachableWallet).unwrap(); + assert_eq!(contribution.feerate, min_rbf_feerate); + assert_eq!(contribution.max_feerate, max_feerate); + } + + /// A mock wallet that returns a single UTXO for coin selection. + struct SingleUtxoWallet { + utxo: FundingTxInput, + change_output: Option, + } + + impl CoinSelectionSourceSync for SingleUtxoWallet { + fn select_confirmed_utxos( + &self, _claim_id: Option, _must_spend: Vec, _must_pay_to: &[TxOut], + _target_feerate_sat_per_1000_weight: u32, _max_tx_weight: u64, + ) -> Result { + Ok(CoinSelection { + confirmed_utxos: vec![self.utxo.clone()], + change_output: self.change_output.clone(), + }) + } + fn sign_psbt(&self, _psbt: Psbt) -> Result { + unreachable!("should not reach signing") + } + } + + fn shared_input(value_sats: u64) -> Input { + Input { + outpoint: bitcoin::OutPoint::null(), + previous_utxo: TxOut { + value: Amount::from_sat(value_sats), + script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()), + }, + satisfaction_weight: 107, + } + } + + #[test] + fn test_rbf_sync_unadjusted_splice_out_runs_coin_selection() { + // When the prior contribution's feerate is below the minimum RBF feerate and no + // holder balance is available, rbf_sync should run coin selection to add inputs that + // cover the higher RBF fee. + let min_rbf_feerate = FeeRate::from_sat_per_kwu(5000); + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let withdrawal = funding_output_sats(20_000); + + let prior = FundingContribution { + value_added: Amount::ZERO, + estimated_fee: Amount::from_sat(500), + inputs: vec![], + outputs: vec![withdrawal.clone()], + change_output: None, + feerate: prior_feerate, + max_feerate: prior_feerate, + is_splice: true, + }; + + let template = FundingTemplate::new( + Some(shared_input(100_000)), + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, None)), + ); + + let wallet = SingleUtxoWallet { + utxo: funding_input_sats(50_000), + change_output: Some(funding_output_sats(40_000)), + }; + + // rbf_sync should succeed and the contribution should have inputs from coin selection. + let contribution = template.rbf_sync(FeeRate::MAX, &wallet).unwrap(); + assert_eq!(contribution.value_added, Amount::ZERO); + assert!(!contribution.inputs.is_empty(), "coin selection should have added inputs"); + assert_eq!(contribution.outputs, vec![withdrawal]); + assert_eq!(contribution.feerate, min_rbf_feerate); + } + + #[test] + fn test_rbf_sync_no_prior_fee_bump_only_runs_coin_selection() { + // When there is no prior contribution (e.g., acceptor), rbf_sync should run coin + // selection to add inputs for a fee-bump-only contribution. + let min_rbf_feerate = FeeRate::from_sat_per_kwu(5000); + + let template = + FundingTemplate::new(Some(shared_input(100_000)), Some(min_rbf_feerate), None); + + let wallet = SingleUtxoWallet { + utxo: funding_input_sats(50_000), + change_output: Some(funding_output_sats(45_000)), + }; + + let contribution = template.rbf_sync(FeeRate::MAX, &wallet).unwrap(); + assert_eq!(contribution.value_added, Amount::ZERO); + assert!(!contribution.inputs.is_empty(), "coin selection should have added inputs"); + assert!(contribution.outputs.is_empty()); + assert_eq!(contribution.feerate, min_rbf_feerate); + } + + #[test] + fn test_rbf_sync_unadjusted_uses_callers_max_feerate() { + // When the prior contribution's feerate is below the minimum RBF feerate and no + // holder balance is available, rbf_sync should use the caller's max_feerate (not the + // prior's) for the resulting contribution. + let min_rbf_feerate = FeeRate::from_sat_per_kwu(5000); + let prior_max_feerate = FeeRate::from_sat_per_kwu(50_000); + let callers_max_feerate = FeeRate::from_sat_per_kwu(10_000); + let withdrawal = funding_output_sats(20_000); + + let prior = FundingContribution { + value_added: Amount::ZERO, + estimated_fee: Amount::from_sat(500), + inputs: vec![], + outputs: vec![withdrawal.clone()], + change_output: None, + feerate: FeeRate::from_sat_per_kwu(2000), + max_feerate: prior_max_feerate, + is_splice: true, + }; + + let template = FundingTemplate::new( + Some(shared_input(100_000)), + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, None)), + ); + + let wallet = SingleUtxoWallet { + utxo: funding_input_sats(50_000), + change_output: Some(funding_output_sats(40_000)), + }; + + let contribution = template.rbf_sync(callers_max_feerate, &wallet).unwrap(); + assert_eq!( + contribution.max_feerate, callers_max_feerate, + "should use caller's max_feerate, not prior's" + ); + } + + #[test] + fn test_splice_out_sync_skips_coin_selection_during_rbf() { + // When splice_out_sync is called on a template with min_rbf_feerate set (user + // choosing a fresh splice-out instead of rbf_sync), coin selection should NOT run. + // Fees come from the channel balance. + let min_rbf_feerate = FeeRate::from_sat_per_kwu(5000); + let feerate = FeeRate::from_sat_per_kwu(5000); + let withdrawal = funding_output_sats(20_000); + + let template = + FundingTemplate::new(Some(shared_input(100_000)), Some(min_rbf_feerate), None); + + // UnreachableWallet panics if coin selection runs — verifying it is skipped. + let contribution = template + .splice_out_sync(vec![withdrawal.clone()], feerate, FeeRate::MAX, UnreachableWallet) + .unwrap(); + assert_eq!(contribution.value_added, Amount::ZERO); + assert!(contribution.inputs.is_empty()); + assert_eq!(contribution.outputs, vec![withdrawal]); + } } diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 0c03585311a..4b6cacdf670 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -237,7 +237,7 @@ pub fn do_initiate_rbf_splice_in<'a, 'b, 'c, 'd>( value_added: Amount, feerate: FeeRate, ) -> FundingContribution { let node_id_counterparty = counterparty.node.get_our_node_id(); - let funding_template = node.node.rbf_channel(&channel_id, &node_id_counterparty).unwrap(); + let funding_template = node.node.splice_channel(&channel_id, &node_id_counterparty).unwrap(); let wallet = WalletSync::new(Arc::clone(&node.wallet_source), node.logger); let funding_contribution = funding_template.splice_in_sync(value_added, feerate, FeeRate::MAX, &wallet).unwrap(); @@ -252,7 +252,7 @@ pub fn do_initiate_rbf_splice_in_and_out<'a, 'b, 'c, 'd>( value_added: Amount, outputs: Vec, feerate: FeeRate, ) -> FundingContribution { let node_id_counterparty = counterparty.node.get_our_node_id(); - let funding_template = node.node.rbf_channel(&channel_id, &node_id_counterparty).unwrap(); + let funding_template = node.node.splice_channel(&channel_id, &node_id_counterparty).unwrap(); let wallet = WalletSync::new(Arc::clone(&node.wallet_source), node.logger); let funding_contribution = funding_template .splice_in_and_out_sync(value_added, outputs, feerate, FeeRate::MAX, &wallet) @@ -4295,7 +4295,7 @@ fn test_splice_acceptor_disconnect_emits_events() { #[test] fn test_splice_rbf_acceptor_basic() { // Test the full end-to-end flow for RBF of a pending splice transaction. - // Complete a splice-in, then use rbf_channel API to initiate an RBF attempt + // Complete a splice-in, then use splice_channel API to initiate an RBF attempt // with a higher feerate, going through the full tx_init_rbf → tx_ack_rbf → // interactive TX → signing → mining → splice_locked flow. let chanmon_cfgs = create_chanmon_cfgs(2); @@ -4322,7 +4322,7 @@ fn test_splice_rbf_acceptor_basic() { // Step 2: Provide more UTXO reserves for the RBF attempt. provide_utxo_reserves(&nodes, 2, added_value * 2); - // Step 3: Use rbf_channel API to initiate the RBF. + // Step 3: Use splice_channel API to initiate the RBF. // Original feerate was FEERATE_FLOOR_SATS_PER_KW (253). 253 * 25 / 24 = 263.54, so 264 works. let rbf_feerate_sat_per_kwu = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24); let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); @@ -4385,7 +4385,7 @@ fn test_splice_rbf_insufficient_feerate() { // Initiator-side: splice_in_sync rejects an insufficient feerate. // Original feerate was 253. Using exactly 253 should fail since 253 * 24 < 253 * 25. let same_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = nodes[0].node.rbf_channel(&channel_id, &node_id_1).unwrap(); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); // Verify that the template exposes the RBF floor. let min_rbf_feerate = funding_template.min_rbf_feerate().unwrap(); @@ -4399,7 +4399,7 @@ fn test_splice_rbf_insufficient_feerate() { .is_err()); // Verify that the floor feerate succeeds. - let funding_template = nodes[0].node.rbf_channel(&channel_id, &node_id_1).unwrap(); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); assert!(funding_template .splice_in_sync(added_value, min_rbf_feerate, FeeRate::MAX, &wallet) .is_ok()); @@ -4580,6 +4580,43 @@ fn test_splice_rbf_after_splice_locked() { } } +#[test] +fn test_splice_zeroconf_no_rbf_feerate() { + // Test that splice_channel returns a FundingTemplate with min_rbf_feerate = None for a + // zero-conf channel, even when a splice negotiation is in progress. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let mut config = test_default_channel_config(); + config.channel_handshake_limits.trust_own_funding_0conf = true; + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_0 = nodes[0].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (funding_tx, channel_id) = + open_zero_conf_channel_with_value(&nodes[0], &nodes[1], None, initial_channel_value_sat, 0); + mine_transaction(&nodes[0], &funding_tx); + mine_transaction(&nodes[1], &funding_tx); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 1, added_value * 2); + + // Initiate a splice (node 0) and complete the handshake so a funding negotiation is in + // progress. + let _funding_contribution = + do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + let _new_funding_script = complete_splice_handshake(&nodes[0], &nodes[1]); + + // The acceptor (node 1) calling splice_channel should return no RBF feerate since + // zero-conf channels cannot RBF. + let funding_template = nodes[1].node.splice_channel(&channel_id, &node_id_0).unwrap(); + assert!(funding_template.min_rbf_feerate().is_none()); + + // Drain pending interactive tx messages from the splice handshake. + nodes[0].node.get_and_clear_pending_msg_events(); +} + #[test] fn test_splice_rbf_zeroconf_rejected() { // Test that tx_init_rbf is rejected when option_zeroconf is negotiated. @@ -4622,10 +4659,7 @@ fn test_splice_rbf_zeroconf_rejected() { msgs::ErrorAction::DisconnectPeerWithWarning { msg: msgs::WarningMessage { channel_id, - data: format!( - "Channel {} has option_zeroconf, cannot RBF splice", - channel_id, - ), + data: format!("Channel {} has option_zeroconf, cannot RBF", channel_id,), }, } ); @@ -4741,7 +4775,7 @@ fn test_splice_rbf_tiebreak_feerate_too_high() { /// Runs the tie-breaker test with the given per-node feerates and node 1's splice value. /// -/// Both nodes call `rbf_channel` + `funding_contributed`, both send STFU, and node 0 (the outbound +/// Both nodes call `splice_channel` + `funding_contributed`, both send STFU, and node 0 (the outbound /// channel funder) wins the quiescence tie-break. The loser (node 1) becomes the acceptor. Whether /// node 1 contributes to the RBF transaction depends on the feerate and budget constraints. /// @@ -4773,11 +4807,11 @@ pub fn do_test_splice_rbf_tiebreak( // Provide more UTXOs for both nodes' RBF attempts. provide_utxo_reserves(&nodes, 2, added_value * 2); - // Node 0 calls rbf_channel + funding_contributed. + // Node 0 calls splice_channel + funding_contributed. let node_0_funding_contribution = do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate_0); - // Node 1 calls rbf_channel + funding_contributed. + // Node 1 calls splice_channel + funding_contributed. let node_1_funding_contribution = do_initiate_rbf_splice_in( &nodes[1], &nodes[0], @@ -5035,7 +5069,7 @@ fn test_splice_rbf_tiebreak_feerate_too_high_rejected() { let min_rbf_feerate = FeeRate::from_sat_per_kwu(min_rbf_feerate_sat_per_kwu); let node_1_max_feerate = FeeRate::from_sat_per_kwu(3_000); - let funding_template_0 = nodes[0].node.rbf_channel(&channel_id, &node_id_1).unwrap(); + let funding_template_0 = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet_0 = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let node_0_funding_contribution = funding_template_0 .splice_in_sync(added_value, high_feerate, FeeRate::MAX, &wallet_0) @@ -5045,7 +5079,7 @@ fn test_splice_rbf_tiebreak_feerate_too_high_rejected() { .funding_contributed(&channel_id, &node_id_1, node_0_funding_contribution.clone(), None) .unwrap(); - let funding_template_1 = nodes[1].node.rbf_channel(&channel_id, &node_id_0).unwrap(); + let funding_template_1 = nodes[1].node.splice_channel(&channel_id, &node_id_0).unwrap(); let wallet_1 = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); let node_1_funding_contribution = funding_template_1 .splice_in_sync(added_value, min_rbf_feerate, node_1_max_feerate, &wallet_1) @@ -5160,7 +5194,7 @@ fn test_splice_rbf_acceptor_recontributes() { // Step 4: Provide new UTXOs for node 0's RBF (node 1 does NOT initiate RBF). provide_utxo_reserves(&nodes, 2, added_value * 2); - // Step 5: Only node 0 calls rbf_channel + funding_contributed. + // Step 5: Only node 0 calls splice_channel + funding_contributed. let rbf_feerate_sat_per_kwu = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24); let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); let rbf_funding_contribution = @@ -5289,7 +5323,7 @@ fn test_splice_rbf_recontributes_feerate_too_high() { provide_utxo_reserves(&nodes, 2, Amount::from_sat(100_000)); let high_feerate = FeeRate::from_sat_per_kwu(20_000); - let funding_template = nodes[0].node.rbf_channel(&channel_id, &node_id_1).unwrap(); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let rbf_funding_contribution = funding_template .splice_in_sync(Amount::from_sat(50_000), high_feerate, FeeRate::MAX, &wallet) @@ -5631,8 +5665,8 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { #[test] fn test_splice_channel_with_pending_splice_includes_rbf_floor() { - // Test that splice_channel (not rbf_channel) includes the RBF floor when a pending splice - // exists with negotiated candidates. + // Test that splice_channel includes the RBF floor when a pending splice exists with + // negotiated candidates. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); @@ -5647,33 +5681,39 @@ fn test_splice_channel_with_pending_splice_includes_rbf_floor() { let added_value = Amount::from_sat(50_000); provide_utxo_reserves(&nodes, 2, added_value * 2); + // Fresh splice — no pending splice, so no prior contribution or minimum RBF feerate. + { + let template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + assert!(template.min_rbf_feerate().is_none()); + assert!(template.prior_contribution().is_none()); + } + // Complete a splice-in at floor feerate. let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); let (_splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); - // Call splice_channel (not rbf_channel) — the pending splice should cause - // min_rbf_feerate to be set. + // Call splice_channel again — the pending splice should cause min_rbf_feerate to be set + // and the prior contribution to be available. let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let expected_floor = FeeRate::from_sat_per_kwu(((FEERATE_FLOOR_SATS_PER_KW as u64) * 25).div_ceil(24)); assert_eq!(funding_template.min_rbf_feerate(), Some(expected_floor)); + assert!(funding_template.prior_contribution().is_some()); - // Successfully build a contribution at the floor feerate. + // rbf_sync returns the Adjusted prior contribution directly. let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - assert!(funding_template - .splice_in_sync(added_value, expected_floor, FeeRate::MAX, &wallet) - .is_ok()); + assert!(funding_template.rbf_sync(FeeRate::MAX, &wallet).is_ok()); } #[test] fn test_funding_contributed_adjusts_feerate_for_rbf() { - // Test that funding_contributed adjusts the contribution's feerate to the minimum RBF feerate when a - // pending splice appears between splice_channel and funding_contributed. + // Test that funding_contributed adjusts the contribution's feerate to the minimum RBF feerate + // when a pending splice appears between splice_channel and funding_contributed. // // Node 0 calls splice_channel (no pending splice → min_rbf_feerate = None) and builds a // contribution at floor feerate. Node 1 then initiates and completes a splice. When node 0 - // calls funding_contributed, the contribution is adjusted to the minimum RBF feerate and STFU is sent - // immediately. + // calls funding_contributed, the contribution is adjusted to the minimum RBF feerate and STFU + // is sent immediately. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); @@ -5724,9 +5764,9 @@ fn test_funding_contributed_adjusts_feerate_for_rbf() { #[test] fn test_funding_contributed_rbf_adjustment_exceeds_max_feerate() { - // Test that when the minimum RBF feerate exceeds max_feerate, the adjustment in funding_contributed - // fails gracefully and the contribution keeps its original feerate. The splice still - // proceeds (STFU is sent) and the RBF negotiation handles the feerate mismatch. + // Test that when the minimum RBF feerate exceeds max_feerate, the adjustment in + // funding_contributed fails gracefully and the contribution keeps its original feerate. The + // splice still proceeds (STFU is sent) and the RBF negotiation handles the feerate mismatch. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); @@ -5755,8 +5795,8 @@ fn test_funding_contributed_rbf_adjustment_exceeds_max_feerate() { let node_1_contribution = do_initiate_splice_in(&nodes[1], &nodes[0], channel_id, added_value); let (_splice_tx, _) = splice_channel(&nodes[1], &nodes[0], channel_id, node_1_contribution); - // Node 0 calls funding_contributed. The adjustment fails (minimum RBF feerate > max_feerate), but - // funding_contributed still succeeds — the contribution keeps its original feerate. + // Node 0 calls funding_contributed. The adjustment fails (minimum RBF feerate > max_feerate), + // but funding_contributed still succeeds — the contribution keeps its original feerate. nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution, None).unwrap(); // STFU is NOT sent — the feerate is below the minimum RBF feerate so try_send_stfu delays. @@ -5860,3 +5900,151 @@ fn test_funding_contributed_rbf_adjustment_insufficient_budget() { let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); assert_eq!(splice_init.funding_feerate_per_kw, FEERATE_FLOOR_SATS_PER_KW); } + +#[test] +fn test_prior_contribution_unadjusted_when_max_feerate_too_low() { + // Test that rbf_sync re-runs coin selection when the prior contribution's max_feerate is + // too low to accommodate the minimum RBF feerate. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + // Complete a splice with max_feerate = floor_feerate. This means the prior contribution + // stored in pending_splice.contributions will have a tight max_feerate. + let floor_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let funding_contribution = funding_template + .splice_in_sync(added_value, floor_feerate, floor_feerate, &wallet) + .unwrap(); + nodes[0] + .node + .funding_contributed(&channel_id, &node_id_1, funding_contribution.clone(), None) + .unwrap(); + let (_splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + + // Call splice_channel again — the minimum RBF feerate (25/24 of floor) exceeds the prior + // contribution's max_feerate (floor), so adjustment fails. rbf_sync re-runs coin selection + // with the caller's max_feerate. + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + assert!(funding_template.min_rbf_feerate().is_some()); + assert!(funding_template.prior_contribution().is_some()); + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + assert!(funding_template.rbf_sync(FeeRate::MAX, &wallet).is_ok()); +} + +#[test] +fn test_splice_channel_during_negotiation_includes_rbf_feerate() { + // Test that splice_channel returns min_rbf_feerate derived from the in-progress + // negotiation's feerate when the acceptor calls it during active negotiation. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + // Node 1 initiates a splice. Perform stfu exchange and splice_init handling, which creates + // a pending_splice with funding_negotiation on node 0 (the acceptor). + let _funding_contribution = + do_initiate_splice_in(&nodes[1], &nodes[0], channel_id, added_value); + let stfu_init = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + nodes[0].node.handle_stfu(node_id_1, &stfu_init); + let stfu_ack = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + nodes[1].node.handle_stfu(node_id_0, &stfu_ack); + + let splice_init = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceInit, node_id_0); + nodes[0].node.handle_splice_init(node_id_1, &splice_init); + let _splice_ack = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceAck, node_id_1); + + // Node 0 (acceptor) calls splice_channel while the negotiation is in progress. + // min_rbf_feerate should be derived from the in-progress negotiation's feerate. + let template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let expected_floor = + FeeRate::from_sat_per_kwu(((FEERATE_FLOOR_SATS_PER_KW as u64) * 25).div_ceil(24)); + assert_eq!(template.min_rbf_feerate(), Some(expected_floor)); + + // No prior contribution since there are no negotiated candidates yet. rbf_sync runs + // fee-bump-only coin selection. + assert!(template.prior_contribution().is_none()); + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + assert!(template.rbf_sync(FeeRate::MAX, &wallet).is_ok()); +} + +#[test] +fn test_rbf_sync_returns_err_when_no_min_rbf_feerate() { + // Test that rbf_sync returns Err(()) when there is no pending splice (min_rbf_feerate is + // None), indicating this is not an RBF scenario. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + // Fresh splice — no pending splice, so min_rbf_feerate is None. + let template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + assert!(template.min_rbf_feerate().is_none()); + assert!(template.prior_contribution().is_none()); + + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + assert!(template.rbf_sync(FeeRate::MAX, &wallet).is_err()); +} + +#[test] +fn test_rbf_sync_returns_err_when_max_feerate_below_min_rbf() { + // Test that rbf_sync returns Err(()) when the caller's max_feerate is below the minimum + // RBF feerate. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + // Complete a splice to create a pending splice. + let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + let (_splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + + // Call splice_channel again to get the RBF template. + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let min_rbf_feerate = funding_template.min_rbf_feerate().unwrap(); + + // Use a max_feerate that is 1 sat/kwu below the minimum RBF feerate. + let too_low_feerate = + FeeRate::from_sat_per_kwu(min_rbf_feerate.to_sat_per_kwu().saturating_sub(1)); + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + assert!(funding_template.rbf_sync(too_low_feerate, &wallet).is_err()); +} diff --git a/lightning/src/util/wallet_utils.rs b/lightning/src/util/wallet_utils.rs index b82437c03e8..61228402959 100644 --- a/lightning/src/util/wallet_utils.rs +++ b/lightning/src/util/wallet_utils.rs @@ -148,7 +148,7 @@ impl Utxo { /// /// Can be used as an input to contribute to a channel's funding transaction either when using the /// v2 channel establishment protocol or when splicing. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct ConfirmedUtxo { /// The unspent [`TxOut`] found in [`prevtx`]. /// From 1098cc304ad8109ed7d8d7794ef4733205dca01c Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Mar 2026 13:23:16 -0500 Subject: [PATCH 4/7] Use +25 sat/kwu increment for our minimum RBF feerate The spec's 25/24 multiplier doesn't always satisfy BIP125's relay requirement of an absolute fee increase. Use a flat +25 sat/kwu (0.1 sat/vB) increment for our own RBFs instead, while still accepting the 25/24 rule from counterparties. Extract a `min_rbf_feerate` helper to consolidate the two call sites and add a test that a counterparty feerate satisfying 25/24 (but not +25) is accepted. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 18 ++++--- lightning/src/ln/funding.rs | 22 ++++----- lightning/src/ln/splicing_tests.rs | 76 +++++++++++++++++------------- 3 files changed, 67 insertions(+), 49 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index ff47306edf3..3b2e7c678f6 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -6462,6 +6462,16 @@ fn get_v2_channel_reserve_satoshis(channel_value_satoshis: u64, dust_limit_satos cmp::min(channel_value_satoshis, cmp::max(q, dust_limit_satoshis)) } +/// Returns the minimum feerate for our own RBF attempts given a previous feerate. +/// +/// The spec (tx_init_rbf) requires the new feerate to be >= 25/24 of the previous feerate. +/// However, that multiplier doesn't always satisfy BIP125's relay requirement of an absolute fee +/// increase, so for our own RBFs we use a flat +25 sat/kwu (0.1 sat/vB) increment instead. We +/// still accept the 25/24 rule from counterparties in [`FundedChannel::validate_tx_init_rbf`]. +fn min_rbf_feerate(prev_feerate: u32) -> FeeRate { + FeeRate::from_sat_per_kwu((prev_feerate as u64).saturating_add(25)) +} + /// Context for negotiating channels (dual-funded V2 open, splicing) #[derive(Debug)] pub(super) struct FundingNegotiationContext { @@ -11972,8 +11982,7 @@ where // slightly higher feerate than necessary. Call splice_channel again after // receiving SpliceFailed to get a fresh template without the RBF constraint. let prev_feerate = negotiation.funding_feerate_sat_per_1000_weight(); - let min_feerate_kwu = ((prev_feerate as u64) * 25).div_ceil(24); - (Some(FeeRate::from_sat_per_kwu(min_feerate_kwu)), None) + (Some(min_rbf_feerate(prev_feerate)), None) } else { // No RBF feerate to derive — either a fresh splice or a pending splice that // can't be RBF'd (e.g., splice_locked already exchanged). @@ -12057,10 +12066,7 @@ where } match pending_splice.last_funding_feerate_sat_per_1000_weight { - Some(prev_feerate) => { - let min_feerate_kwu = ((prev_feerate as u64) * 25).div_ceil(24); - Ok(FeeRate::from_sat_per_kwu(min_feerate_kwu)) - }, + Some(prev_feerate) => Ok(min_rbf_feerate(prev_feerate)), None => Err(format!( "Channel {} has no prior feerate to compute RBF minimum", self.context.channel_id(), diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 35f6a16cd0f..ec8f510a468 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -153,8 +153,8 @@ impl PriorContribution { /// prior contribution logic internally — reusing an adjusted prior when possible, re-running /// coin selection when needed, or creating a fee-bump-only contribution. /// -/// Check [`FundingTemplate::min_rbf_feerate`] for the minimum feerate required (25/24 of -/// the previous feerate). Use [`FundingTemplate::prior_contribution`] to inspect the prior +/// Check [`FundingTemplate::min_rbf_feerate`] for the minimum feerate required (previous +/// feerate + 25 sat/kwu). Use [`FundingTemplate::prior_contribution`] to inspect the prior /// contribution's parameters (e.g., [`FundingContribution::value_added`], /// [`FundingContribution::outputs`]) before deciding whether to reuse it via the RBF methods /// or build a fresh contribution with different parameters using the splice methods above. @@ -167,7 +167,7 @@ pub struct FundingTemplate { /// transaction. shared_input: Option, - /// The minimum RBF feerate (25/24 of the previous feerate), if this template is for an + /// The minimum RBF feerate (previous feerate + 25 sat/kwu), if this template is for an /// RBF attempt. `None` for fresh splices with no pending splice candidates. min_rbf_feerate: Option, @@ -2161,8 +2161,8 @@ mod tests { // When the caller's max_feerate is below the minimum RBF feerate, rbf_sync should // return Err(()). let prior_feerate = FeeRate::from_sat_per_kwu(2000); - let min_rbf_feerate = FeeRate::from_sat_per_kwu(5000); - let max_feerate = FeeRate::from_sat_per_kwu(3000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); + let max_feerate = FeeRate::from_sat_per_kwu(2020); let prior = FundingContribution { value_added: Amount::from_sat(50_000), @@ -2175,7 +2175,7 @@ mod tests { is_splice: true, }; - // max_feerate (3000) < min_rbf_feerate (5000). + // max_feerate (2020) < min_rbf_feerate (2025). let template = FundingTemplate::new( None, Some(min_rbf_feerate), @@ -2255,8 +2255,8 @@ mod tests { // When the prior contribution's feerate is below the minimum RBF feerate and no // holder balance is available, rbf_sync should run coin selection to add inputs that // cover the higher RBF fee. - let min_rbf_feerate = FeeRate::from_sat_per_kwu(5000); let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); let withdrawal = funding_output_sats(20_000); let prior = FundingContribution { @@ -2293,7 +2293,7 @@ mod tests { fn test_rbf_sync_no_prior_fee_bump_only_runs_coin_selection() { // When there is no prior contribution (e.g., acceptor), rbf_sync should run coin // selection to add inputs for a fee-bump-only contribution. - let min_rbf_feerate = FeeRate::from_sat_per_kwu(5000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); let template = FundingTemplate::new(Some(shared_input(100_000)), Some(min_rbf_feerate), None); @@ -2315,7 +2315,7 @@ mod tests { // When the prior contribution's feerate is below the minimum RBF feerate and no // holder balance is available, rbf_sync should use the caller's max_feerate (not the // prior's) for the resulting contribution. - let min_rbf_feerate = FeeRate::from_sat_per_kwu(5000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); let prior_max_feerate = FeeRate::from_sat_per_kwu(50_000); let callers_max_feerate = FeeRate::from_sat_per_kwu(10_000); let withdrawal = funding_output_sats(20_000); @@ -2354,8 +2354,8 @@ mod tests { // When splice_out_sync is called on a template with min_rbf_feerate set (user // choosing a fresh splice-out instead of rbf_sync), coin selection should NOT run. // Fees come from the channel balance. - let min_rbf_feerate = FeeRate::from_sat_per_kwu(5000); - let feerate = FeeRate::from_sat_per_kwu(5000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); + let feerate = FeeRate::from_sat_per_kwu(2025); let withdrawal = funding_output_sats(20_000); let template = diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 4b6cacdf670..e2d85aaa93c 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -4323,8 +4323,8 @@ fn test_splice_rbf_acceptor_basic() { provide_utxo_reserves(&nodes, 2, added_value * 2); // Step 3: Use splice_channel API to initiate the RBF. - // Original feerate was FEERATE_FLOOR_SATS_PER_KW (253). 253 * 25 / 24 = 263.54, so 264 works. - let rbf_feerate_sat_per_kwu = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24); + // Original feerate was FEERATE_FLOOR_SATS_PER_KW (253). 253 + 25 = 278. + let rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); let funding_contribution = do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); @@ -4360,7 +4360,7 @@ fn test_splice_rbf_acceptor_basic() { #[test] fn test_splice_rbf_insufficient_feerate() { - // Test that splice_in_sync rejects a feerate that doesn't satisfy the 25/24 rule, and that the + // Test that splice_in_sync rejects a feerate that doesn't satisfy the +25 sat/kwu rule, and that the // acceptor also rejects tx_init_rbf with an insufficient feerate from a misbehaving peer. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); @@ -4389,8 +4389,7 @@ fn test_splice_rbf_insufficient_feerate() { // Verify that the template exposes the RBF floor. let min_rbf_feerate = funding_template.min_rbf_feerate().unwrap(); - let expected_floor = - FeeRate::from_sat_per_kwu(((FEERATE_FLOOR_SATS_PER_KW as u64) * 25).div_ceil(24)); + let expected_floor = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25); assert_eq!(min_rbf_feerate, expected_floor); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); @@ -4418,6 +4417,22 @@ fn test_splice_rbf_insufficient_feerate() { let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); assert_eq!(tx_abort.channel_id, channel_id); + + // Acceptor-side: a counterparty feerate that satisfies the spec's 25/24 rule (264) is + // accepted, even though our own RBF floor (+25 sat/kwu = 278) is higher. + // After tx_abort the channel remains quiescent, so no need to re-enter quiescence. + nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); + + let rbf_feerate_25_24 = ((FEERATE_FLOOR_SATS_PER_KW as u64) * 25).div_ceil(24) as u32; + let tx_init_rbf = msgs::TxInitRbf { + channel_id, + locktime: 0, + feerate_sat_per_1000_weight: rbf_feerate_25_24, + funding_output_contribution: Some(added_value.to_sat() as i64), + }; + + nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); + let _tx_ack_rbf = get_event_msg!(nodes[1], MessageSendEvent::SendTxAckRbf, node_id_0); } #[test] @@ -4696,7 +4711,7 @@ fn test_splice_rbf_not_quiescence_initiator() { provide_utxo_reserves(&nodes, 2, added_value * 2); // Initiate RBF from node 0 (quiescence initiator). - let rbf_feerate_sat_per_kwu = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24); + let rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); let _funding_contribution = do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); @@ -4726,7 +4741,7 @@ fn test_splice_rbf_not_quiescence_initiator() { #[test] fn test_splice_rbf_both_contribute_tiebreak() { - let min_rbf_feerate = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24); + let min_rbf_feerate = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let feerate = FeeRate::from_sat_per_kwu(min_rbf_feerate); let added_value = Amount::from_sat(50_000); do_test_splice_rbf_tiebreak(feerate, feerate, added_value, true); @@ -4736,7 +4751,7 @@ fn test_splice_rbf_both_contribute_tiebreak() { fn test_splice_rbf_tiebreak_higher_feerate() { // Node 0 (winner) uses a higher feerate than node 1 (loser). Node 1's change output is // adjusted (reduced) to accommodate the higher feerate. Negotiation succeeds. - let min_rbf_feerate = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24); + let min_rbf_feerate = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; do_test_splice_rbf_tiebreak( FeeRate::from_sat_per_kwu(min_rbf_feerate * 3), FeeRate::from_sat_per_kwu(min_rbf_feerate), @@ -4750,7 +4765,7 @@ fn test_splice_rbf_tiebreak_lower_feerate() { // Node 0 (winner) uses a lower feerate than node 1 (loser). Since the initiator's feerate // is below node 1's minimum, node 1 proceeds without contribution and will retry via a new // splice at its preferred feerate after the RBF locks. - let min_rbf_feerate = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24); + let min_rbf_feerate = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; do_test_splice_rbf_tiebreak( FeeRate::from_sat_per_kwu(min_rbf_feerate), FeeRate::from_sat_per_kwu(min_rbf_feerate * 3), @@ -4764,7 +4779,7 @@ fn test_splice_rbf_tiebreak_feerate_too_high() { // Node 0 (winner) uses a feerate high enough that node 1's (loser) contribution cannot // cover the fees. Node 1 proceeds without its contribution (QuiescentAction is preserved // for a future splice). The RBF completes with only node 0's inputs/outputs. - let min_rbf_feerate = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24); + let min_rbf_feerate = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; do_test_splice_rbf_tiebreak( FeeRate::from_sat_per_kwu(20_000), FeeRate::from_sat_per_kwu(min_rbf_feerate), @@ -5065,7 +5080,7 @@ fn test_splice_rbf_tiebreak_feerate_too_high_rejected() { // The target (100k) far exceeds node 1's max (3k), and the fair fee at 100k exceeds // node 1's budget, triggering TooHigh. let high_feerate = FeeRate::from_sat_per_kwu(100_000); - let min_rbf_feerate_sat_per_kwu = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24); + let min_rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let min_rbf_feerate = FeeRate::from_sat_per_kwu(min_rbf_feerate_sat_per_kwu); let node_1_max_feerate = FeeRate::from_sat_per_kwu(3_000); @@ -5195,7 +5210,7 @@ fn test_splice_rbf_acceptor_recontributes() { provide_utxo_reserves(&nodes, 2, added_value * 2); // Step 5: Only node 0 calls splice_channel + funding_contributed. - let rbf_feerate_sat_per_kwu = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24); + let rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); let rbf_funding_contribution = do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); @@ -5356,7 +5371,7 @@ fn test_splice_rbf_sequential() { // Three consecutive RBF rounds on the same splice (initial → RBF #1 → RBF #2). // Node 0 is the quiescence initiator; node 1 is the acceptor with no contribution. // Verifies: - // - Each round satisfies the 25/24 feerate rule + // - Each round satisfies the +25 sat/kwu feerate rule // - DiscardFunding events reference the correct txids from previous rounds // - The final RBF can be mined and splice_locked successfully let chanmon_cfgs = create_chanmon_cfgs(2); @@ -5379,11 +5394,11 @@ fn test_splice_rbf_sequential() { let (splice_tx_0, new_funding_script) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); - // Feerate progression: 253 → ceil(253*25/24) = 264 → ceil(264*25/24) = 275 - let feerate_1_sat_per_kwu = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24); // 264 - let feerate_2_sat_per_kwu = (feerate_1_sat_per_kwu * 25).div_ceil(24); + // Feerate progression: 253 → 253+25 = 278 → 278+25 = 303 + let feerate_1_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; // 278 + let feerate_2_sat_per_kwu = feerate_1_sat_per_kwu + 25; - // --- Round 1: RBF #1 at feerate 264. --- + // --- Round 1: RBF #1 at feerate 278. --- provide_utxo_reserves(&nodes, 2, added_value * 2); let rbf_feerate_1 = FeeRate::from_sat_per_kwu(feerate_1_sat_per_kwu); @@ -5403,7 +5418,7 @@ fn test_splice_rbf_sequential() { expect_splice_pending_event(&nodes[0], &node_id_1); expect_splice_pending_event(&nodes[1], &node_id_0); - // --- Round 2: RBF #2 at feerate 275. --- + // --- Round 2: RBF #2 at feerate 303. --- provide_utxo_reserves(&nodes, 2, added_value * 2); let rbf_feerate_2 = FeeRate::from_sat_per_kwu(feerate_2_sat_per_kwu); @@ -5499,7 +5514,7 @@ fn test_splice_rbf_acceptor_contributes_then_disconnects() { // --- Round 1: Node 0 initiates RBF; node 1 re-contributes via prior. --- provide_utxo_reserves(&nodes, 2, added_value * 2); - let rbf_feerate_sat_per_kwu = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24); + let rbf_feerate_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let rbf_feerate = FeeRate::from_sat_per_kwu(rbf_feerate_sat_per_kwu); let _rbf_funding_contribution = do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, rbf_feerate); @@ -5583,7 +5598,7 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { // Include a splice-out output with a different script_pubkey so the test can verify // selective filtering: the change output (same script_pubkey as round 0) is filtered, // while the splice-out output (different script_pubkey) survives. - let feerate_1_sat_per_kwu = (FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24); + let feerate_1_sat_per_kwu = FEERATE_FLOOR_SATS_PER_KW as u64 + 25; let rbf_feerate = FeeRate::from_sat_per_kwu(feerate_1_sat_per_kwu); let splice_out_output = TxOut { value: Amount::from_sat(1_000), @@ -5633,9 +5648,9 @@ fn test_splice_rbf_disconnect_filters_prior_contributions() { reconnect_args.send_announcement_sigs = (true, true); reconnect_nodes(reconnect_args); - // --- Round 2: RBF at the same feerate as the failed round 1 (264). --- + // --- Round 2: RBF at the same feerate as the failed round 1 (278). --- // This should succeed because the failed round never updated the feerate floor, which - // remains at round 0's rate (253), and 264 >= ceil(253 * 25/24). + // remains at round 0's rate (253), and 278 >= 253 + 25. provide_utxo_reserves(&nodes, 1, added_value * 2); let rbf_feerate_2 = FeeRate::from_sat_per_kwu(feerate_1_sat_per_kwu); @@ -5695,8 +5710,7 @@ fn test_splice_channel_with_pending_splice_includes_rbf_floor() { // Call splice_channel again — the pending splice should cause min_rbf_feerate to be set // and the prior contribution to be available. let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); - let expected_floor = - FeeRate::from_sat_per_kwu(((FEERATE_FLOOR_SATS_PER_KW as u64) * 25).div_ceil(24)); + let expected_floor = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25); assert_eq!(funding_template.min_rbf_feerate(), Some(expected_floor)); assert!(funding_template.prior_contribution().is_some()); @@ -5745,7 +5759,7 @@ fn test_funding_contributed_adjusts_feerate_for_rbf() { splice_channel(&nodes[1], &nodes[0], channel_id, node_1_contribution); // Node 0 calls funding_contributed. The contribution's feerate (floor) is below the RBF - // floor (25/24 of floor), but funding_contributed adjusts it upward. + // floor (floor + 25 sat/kwu), but funding_contributed adjusts it upward. nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution.clone(), None).unwrap(); // STFU should be sent immediately (the adjusted feerate satisfies the RBF check). @@ -5757,8 +5771,7 @@ fn test_funding_contributed_adjusts_feerate_for_rbf() { // Verify the RBF handshake proceeds. let tx_init_rbf = get_event_msg!(nodes[0], MessageSendEvent::SendTxInitRbf, node_id_1); let rbf_feerate = FeeRate::from_sat_per_kwu(tx_init_rbf.feerate_sat_per_1000_weight as u64); - let expected_floor = - FeeRate::from_sat_per_kwu((FEERATE_FLOOR_SATS_PER_KW as u64 * 25).div_ceil(24)); + let expected_floor = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25); assert!(rbf_feerate >= expected_floor); } @@ -5783,7 +5796,7 @@ fn test_funding_contributed_rbf_adjustment_exceeds_max_feerate() { provide_utxo_reserves(&nodes, 4, added_value * 2); // Node 0 calls splice_channel and builds contribution with max_feerate = floor_feerate. - // This means the minimum RBF feerate (25/24 of floor) will exceed max_feerate, preventing adjustment. + // This means the minimum RBF feerate (floor + 25 sat/kwu) will exceed max_feerate, preventing adjustment. let floor_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); @@ -5858,7 +5871,7 @@ fn test_funding_contributed_rbf_adjustment_insufficient_budget() { funding_template.splice_in_sync(added_value, floor_feerate, FeeRate::MAX, &wallet).unwrap(); // Node 1 initiates a splice at a HIGH feerate (10,000 sat/kwu). The minimum RBF feerate will be - // 25/24 of 10,000 = 10,417 sat/kwu — far above what node 0's tight budget can handle. + // 10,000 + 25 = 10,025 sat/kwu — far above what node 0's tight budget can handle. let high_feerate = FeeRate::from_sat_per_kwu(10_000); let node_1_template = nodes[1].node.splice_channel(&channel_id, &node_id_0).unwrap(); let node_1_wallet = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); @@ -5933,7 +5946,7 @@ fn test_prior_contribution_unadjusted_when_max_feerate_too_low() { .unwrap(); let (_splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); - // Call splice_channel again — the minimum RBF feerate (25/24 of floor) exceeds the prior + // Call splice_channel again — the minimum RBF feerate (floor + 25 sat/kwu) exceeds the prior // contribution's max_feerate (floor), so adjustment fails. rbf_sync re-runs coin selection // with the caller's max_feerate. let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); @@ -5978,8 +5991,7 @@ fn test_splice_channel_during_negotiation_includes_rbf_feerate() { // Node 0 (acceptor) calls splice_channel while the negotiation is in progress. // min_rbf_feerate should be derived from the in-progress negotiation's feerate. let template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); - let expected_floor = - FeeRate::from_sat_per_kwu(((FEERATE_FLOOR_SATS_PER_KW as u64) * 25).div_ceil(24)); + let expected_floor = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25); assert_eq!(template.min_rbf_feerate(), Some(expected_floor)); // No prior contribution since there are no negotiated candidates yet. rbf_sync runs From a1fc8627dbe6bb23e8a88d98334bb81d5316ab15 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Mar 2026 15:25:55 -0500 Subject: [PATCH 5/7] Exit quiescence when tx_init_rbf is rejected with Abort When tx_init_rbf is rejected with ChannelError::Abort (e.g., insufficient RBF feerate, negotiation in progress, feerate too high), the error is converted to a tx_abort message but quiescence is never exited and holding cells are never freed. This leaves the channel stuck in a quiescent state. Fix this by intercepting ChannelError::Abort before try_channel_entry! in internal_tx_init_rbf, calling exit_quiescence on the channel, and returning the error with exited_quiescence set so that handle_error frees holding cells. Also make exit_quiescence available in non-test builds by removing its cfg gate. Update tests to use the proper RBF initiation flow (with tampered feerates) so that handle_tx_abort correctly echoes the abort and exits quiescence, rather than manually crafting tx_init_rbf messages that leave node 0 without proper negotiation state. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 2 - lightning/src/ln/channelmanager.rs | 9 ++++ lightning/src/ln/splicing_tests.rs | 72 ++++++++++++++++++++++-------- 3 files changed, 63 insertions(+), 20 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 3b2e7c678f6..4c5283c1b16 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -13851,8 +13851,6 @@ where Some(msgs::Stfu { channel_id: self.context.channel_id, initiator }) } - #[cfg(any(test, fuzzing, feature = "_test_utils"))] - #[rustfmt::skip] pub fn exit_quiescence(&mut self) -> bool { // Make sure we either finished the quiescence handshake and are quiescent, or we never // attempted to initiate quiescence at all. diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index aa8c091d6ba..5c77dfff6e0 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -13118,6 +13118,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self.fee_estimator, &self.logger, ); + if let Err(ChannelError::Abort(_)) = &init_res { + funded_channel.exit_quiescence(); + let chan_id = funded_channel.context.channel_id(); + let res = MsgHandleErrInternal::from_chan_no_close( + init_res.unwrap_err(), + chan_id, + ); + return Err(res.with_exited_quiescence(true)); + } let tx_ack_rbf_msg = try_channel_entry!(self, peer_state, init_res, chan_entry); peer_state.pending_msg_events.push(MessageSendEvent::SendTxAckRbf { node_id: *counterparty_node_id, diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index e2d85aaa93c..b8614033024 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -4404,33 +4404,54 @@ fn test_splice_rbf_insufficient_feerate() { .is_ok()); // Acceptor-side: tx_init_rbf with an insufficient feerate is also rejected. - reenter_quiescence(&nodes[0], &nodes[1], &channel_id); + // Node 0 initiates a proper RBF but we tamper the feerate to be insufficient. + provide_utxo_reserves(&nodes, 2, added_value * 2); + let _funding_contribution = + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); - let tx_init_rbf = msgs::TxInitRbf { - channel_id, - locktime: 0, - feerate_sat_per_1000_weight: FEERATE_FLOOR_SATS_PER_KW, - funding_output_contribution: Some(added_value.to_sat() as i64), - }; + let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + nodes[1].node.handle_stfu(node_id_0, &stfu_0); + let stfu_1 = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + nodes[0].node.handle_stfu(node_id_1, &stfu_1); + let mut tx_init_rbf = get_event_msg!(nodes[0], MessageSendEvent::SendTxInitRbf, node_id_1); + tx_init_rbf.feerate_sat_per_1000_weight = FEERATE_FLOOR_SATS_PER_KW; nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); assert_eq!(tx_abort.channel_id, channel_id); + // Node 0 echoes tx_abort and exits quiescence. + nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); + let tx_abort_echo = get_event_msg!(nodes[0], MessageSendEvent::SendTxAbort, node_id_1); + + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 2); + assert!( + matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) + ); + assert!( + matches!(&events[1], Event::DiscardFunding { channel_id: cid, .. } if *cid == channel_id) + ); + + // Node 1 handles the echo (no-op since it already aborted). + nodes[1].node.handle_tx_abort(node_id_0, &tx_abort_echo); + // Acceptor-side: a counterparty feerate that satisfies the spec's 25/24 rule (264) is // accepted, even though our own RBF floor (+25 sat/kwu = 278) is higher. - // After tx_abort the channel remains quiescent, so no need to re-enter quiescence. - nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); + // Node 0 initiates another proper RBF but we tamper the feerate to the 25/24 value. + provide_utxo_reserves(&nodes, 2, added_value * 2); + let _funding_contribution = + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); - let rbf_feerate_25_24 = ((FEERATE_FLOOR_SATS_PER_KW as u64) * 25).div_ceil(24) as u32; - let tx_init_rbf = msgs::TxInitRbf { - channel_id, - locktime: 0, - feerate_sat_per_1000_weight: rbf_feerate_25_24, - funding_output_contribution: Some(added_value.to_sat() as i64), - }; + let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + nodes[1].node.handle_stfu(node_id_0, &stfu_0); + let stfu_1 = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + nodes[0].node.handle_stfu(node_id_1, &stfu_1); + let mut tx_init_rbf = get_event_msg!(nodes[0], MessageSendEvent::SendTxInitRbf, node_id_1); + let rbf_feerate_25_24 = ((FEERATE_FLOOR_SATS_PER_KW as u64) * 25).div_ceil(24) as u32; + tx_init_rbf.feerate_sat_per_1000_weight = rbf_feerate_25_24; nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); let _tx_ack_rbf = get_event_msg!(nodes[1], MessageSendEvent::SendTxAckRbf, node_id_0); } @@ -5118,10 +5139,25 @@ fn test_splice_rbf_tiebreak_feerate_too_high_rejected() { assert_eq!(tx_init_rbf.feerate_sat_per_1000_weight, high_feerate.to_sat_per_kwu() as u32); // Node 1 handles tx_init_rbf — TooHigh: target (100k) >> max (3k) and fair fee > budget. + // Node 1 exits quiescence upon rejecting with tx_abort, and since it has a pending + // QuiescentAction (from its own splice RBF attempt), it immediately re-proposes quiescence. nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); - let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); - assert_eq!(tx_abort.channel_id, channel_id); + let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 2); + match &msg_events[0] { + MessageSendEvent::SendTxAbort { node_id, msg } => { + assert_eq!(*node_id, node_id_0); + assert_eq!(msg.channel_id, channel_id); + }, + _ => panic!("Expected SendTxAbort, got {:?}", msg_events[0]), + }; + match &msg_events[1] { + MessageSendEvent::SendStfu { node_id, .. } => { + assert_eq!(*node_id, node_id_0); + }, + _ => panic!("Expected SendStfu, got {:?}", msg_events[1]), + }; } #[test] From c20d83582c7b0e39b4e18c2e585c87fa3d21c493 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Mar 2026 15:43:46 -0500 Subject: [PATCH 6/7] Exit quiescence when splice_init is rejected with Abort The same bug fixed in the prior commit for tx_init_rbf also exists in internal_splice_init: when splice_init triggers FeeRateTooHigh in resolve_queued_contribution, the ChannelError::Abort goes through try_channel_entry! without exiting quiescence. Apply the same fix: intercept ChannelError::Abort before try_channel_entry!, call exit_quiescence, and return the error with exited_quiescence set. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channelmanager.rs | 9 +++++++++ lightning/src/ln/splicing_tests.rs | 19 +++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5c77dfff6e0..7608341b0a5 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -13073,6 +13073,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self.get_our_node_id(), &self.logger, ); + if let Err(ChannelError::Abort(_)) = &init_res { + funded_channel.exit_quiescence(); + let chan_id = funded_channel.context.channel_id(); + let res = MsgHandleErrInternal::from_chan_no_close( + init_res.unwrap_err(), + chan_id, + ); + return Err(res.with_exited_quiescence(true)); + } let splice_ack_msg = try_channel_entry!(self, peer_state, init_res, chan_entry); peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceAck { node_id: *counterparty_node_id, diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index b8614033024..8f6ed64bc2e 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -1814,10 +1814,25 @@ fn test_splice_tiebreak_feerate_too_high_rejected() { let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); // Node 1 handles SpliceInit — TooHigh: target (100k) >> max (3k) and fair fee > budget. + // Node 1 exits quiescence upon rejecting with tx_abort, and since it has a pending + // QuiescentAction (from its own splice attempt), it immediately re-proposes quiescence. nodes[1].node.handle_splice_init(node_id_0, &splice_init); - let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); - assert_eq!(tx_abort.channel_id, channel_id); + let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 2); + match &msg_events[0] { + MessageSendEvent::SendTxAbort { node_id, msg } => { + assert_eq!(*node_id, node_id_0); + assert_eq!(msg.channel_id, channel_id); + }, + _ => panic!("Expected SendTxAbort, got {:?}", msg_events[0]), + }; + match &msg_events[1] { + MessageSendEvent::SendStfu { node_id, .. } => { + assert_eq!(*node_id, node_id_0); + }, + _ => panic!("Expected SendStfu, got {:?}", msg_events[1]), + }; } #[cfg(test)] From 57aad34993bb3cfa97d31d966694b19d7a338c10 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Mar 2026 16:31:10 -0500 Subject: [PATCH 7/7] Return InteractiveTxMsgError from splice_init and tx_init_rbf The prior two commits manually intercepted ChannelError::Abort in the channelmanager handlers for splice_init and tx_init_rbf to exit quiescence before returning, since the channel methods didn't signal this themselves. The interactive TX message handlers already solved this by returning InteractiveTxMsgError which bundles exited_quiescence into the error type. Apply the same pattern: change splice_init and tx_init_rbf to return InteractiveTxMsgError, adding a quiescent_negotiation_err helper on FundedChannel that exits quiescence for Abort errors and passes through other variants unchanged. Extract handle_interactive_tx_msg_err in channelmanager to deduplicate the error handling across internal_tx_msg, internal_splice_init, and internal_tx_init_rbf. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 40 +++++--- lightning/src/ln/channelmanager.rs | 143 ++++++++++++++++------------- 2 files changed, 105 insertions(+), 78 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 4c5283c1b16..d8a50b9db7a 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12524,13 +12524,15 @@ where pub(crate) fn splice_init( &mut self, msg: &msgs::SpliceInit, entropy_source: &ES, holder_node_id: &PublicKey, logger: &L, - ) -> Result { + ) -> Result { let feerate = FeeRate::from_sat_per_kwu(msg.funding_feerate_per_kw as u64); - let (our_funding_contribution, holder_balance) = - self.resolve_queued_contribution(feerate, logger)?; + let (our_funding_contribution, holder_balance) = self + .resolve_queued_contribution(feerate, logger) + .map_err(|e| self.quiescent_negotiation_err(e))?; - let splice_funding = - self.validate_splice_init(msg, our_funding_contribution.unwrap_or(SignedAmount::ZERO))?; + let splice_funding = self + .validate_splice_init(msg, our_funding_contribution.unwrap_or(SignedAmount::ZERO)) + .map_err(|e| self.quiescent_negotiation_err(e))?; // Adjust for the feerate and clone so we can store it for future RBF re-use. let (adjusted_contribution, our_funding_inputs, our_funding_outputs) = @@ -12678,10 +12680,11 @@ where pub(crate) fn tx_init_rbf( &mut self, msg: &msgs::TxInitRbf, entropy_source: &ES, holder_node_id: &PublicKey, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, - ) -> Result { + ) -> Result { let feerate = FeeRate::from_sat_per_kwu(msg.feerate_sat_per_1000_weight as u64); - let (queued_net_value, holder_balance) = - self.resolve_queued_contribution(feerate, logger)?; + let (queued_net_value, holder_balance) = self + .resolve_queued_contribution(feerate, logger) + .map_err(|e| self.quiescent_negotiation_err(e))?; // If no queued contribution, try prior contribution from previous negotiation. // Failing here means the RBF would erase our splice — reject it. @@ -12698,7 +12701,8 @@ where prior .net_value_for_acceptor_at_feerate(feerate, holder_balance) .map_err(|_| ChannelError::Abort(AbortReason::InsufficientRbfFeerate)) - })?; + }) + .map_err(|e| self.quiescent_negotiation_err(e))?; Some(net_value) } else { None @@ -12706,11 +12710,13 @@ where let our_funding_contribution = queued_net_value.or(prior_net_value); - let rbf_funding = self.validate_tx_init_rbf( - msg, - our_funding_contribution.unwrap_or(SignedAmount::ZERO), - fee_estimator, - )?; + let rbf_funding = self + .validate_tx_init_rbf( + msg, + our_funding_contribution.unwrap_or(SignedAmount::ZERO), + fee_estimator, + ) + .map_err(|e| self.quiescent_negotiation_err(e))?; // Consume the appropriate contribution source. let (our_funding_inputs, our_funding_outputs) = if queued_net_value.is_some() { @@ -13863,6 +13869,12 @@ where was_quiescent } + fn quiescent_negotiation_err(&mut self, err: ChannelError) -> InteractiveTxMsgError { + let exited_quiescence = + if matches!(err, ChannelError::Abort(_)) { self.exit_quiescence() } else { false }; + InteractiveTxMsgError { err, splice_funding_failed: None, exited_quiescence } + } + pub fn remove_legacy_scids_before_block(&mut self, height: u32) -> alloc::vec::Drain<'_, u64> { let end = self .funding diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 7608341b0a5..317ade4efbb 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -11650,6 +11650,39 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } + fn handle_interactive_tx_msg_err( + &self, err: InteractiveTxMsgError, channel_id: ChannelId, counterparty_node_id: &PublicKey, + user_channel_id: u128, + ) -> MsgHandleErrInternal { + if let Some(splice_funding_failed) = err.splice_funding_failed { + let pending_events = &mut self.pending_events.lock().unwrap(); + pending_events.push_back(( + events::Event::SpliceFailed { + channel_id, + counterparty_node_id: *counterparty_node_id, + user_channel_id, + abandoned_funding_txo: splice_funding_failed.funding_txo, + channel_type: splice_funding_failed.channel_type.clone(), + }, + None, + )); + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id, + funding_info: FundingInfo::Contribution { + inputs: splice_funding_failed.contributed_inputs, + outputs: splice_funding_failed.contributed_outputs, + }, + }, + None, + )); + } + debug_assert!(!err.exited_quiescence || matches!(err.err, ChannelError::Abort(_))); + + MsgHandleErrInternal::from_chan_no_close(err.err, channel_id) + .with_exited_quiescence(err.exited_quiescence) + } + fn internal_tx_msg< HandleTxMsgFn: Fn(&mut Channel) -> Result, >( @@ -11671,38 +11704,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ peer_state.pending_msg_events.push(msg_send_event); Ok(()) }, - Err(InteractiveTxMsgError { - err, - splice_funding_failed, - exited_quiescence, - }) => { - if let Some(splice_funding_failed) = splice_funding_failed { - let pending_events = &mut self.pending_events.lock().unwrap(); - pending_events.push_back(( - events::Event::SpliceFailed { - channel_id, - counterparty_node_id: *counterparty_node_id, - user_channel_id: channel.context().get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type.clone(), - }, - None, - )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, - }, - }, - None, - )); - } - debug_assert!(!exited_quiescence || matches!(err, ChannelError::Abort(_))); - - Err(MsgHandleErrInternal::from_chan_no_close(err, channel_id) - .with_exited_quiescence(exited_quiescence)) + Err(err) => { + let user_channel_id = channel.context().get_user_id(); + Err(self.handle_interactive_tx_msg_err( + err, + channel_id, + counterparty_node_id, + user_channel_id, + )) }, } }, @@ -13067,27 +13076,30 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } if let Some(ref mut funded_channel) = chan_entry.get_mut().as_funded_mut() { - let init_res = funded_channel.splice_init( + let user_channel_id = funded_channel.context.get_user_id(); + match funded_channel.splice_init( msg, &self.entropy_source, &self.get_our_node_id(), &self.logger, - ); - if let Err(ChannelError::Abort(_)) = &init_res { - funded_channel.exit_quiescence(); - let chan_id = funded_channel.context.channel_id(); - let res = MsgHandleErrInternal::from_chan_no_close( - init_res.unwrap_err(), - chan_id, - ); - return Err(res.with_exited_quiescence(true)); + ) { + Ok(splice_ack_msg) => { + peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceAck { + node_id: *counterparty_node_id, + msg: splice_ack_msg, + }); + Ok(()) + }, + Err(err) => { + debug_assert!(err.splice_funding_failed.is_none()); + Err(self.handle_interactive_tx_msg_err( + err, + msg.channel_id, + counterparty_node_id, + user_channel_id, + )) + }, } - let splice_ack_msg = try_channel_entry!(self, peer_state, init_res, chan_entry); - peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceAck { - node_id: *counterparty_node_id, - msg: splice_ack_msg, - }); - Ok(()) } else { try_channel_entry!( self, @@ -13120,28 +13132,31 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, hash_map::Entry::Occupied(mut chan_entry) => { if let Some(ref mut funded_channel) = chan_entry.get_mut().as_funded_mut() { - let init_res = funded_channel.tx_init_rbf( + let user_channel_id = funded_channel.context.get_user_id(); + match funded_channel.tx_init_rbf( msg, &self.entropy_source, &self.get_our_node_id(), &self.fee_estimator, &self.logger, - ); - if let Err(ChannelError::Abort(_)) = &init_res { - funded_channel.exit_quiescence(); - let chan_id = funded_channel.context.channel_id(); - let res = MsgHandleErrInternal::from_chan_no_close( - init_res.unwrap_err(), - chan_id, - ); - return Err(res.with_exited_quiescence(true)); + ) { + Ok(tx_ack_rbf_msg) => { + peer_state.pending_msg_events.push(MessageSendEvent::SendTxAckRbf { + node_id: *counterparty_node_id, + msg: tx_ack_rbf_msg, + }); + Ok(()) + }, + Err(err) => { + debug_assert!(err.splice_funding_failed.is_none()); + Err(self.handle_interactive_tx_msg_err( + err, + msg.channel_id, + counterparty_node_id, + user_channel_id, + )) + }, } - let tx_ack_rbf_msg = try_channel_entry!(self, peer_state, init_res, chan_entry); - peer_state.pending_msg_events.push(MessageSendEvent::SendTxAckRbf { - node_id: *counterparty_node_id, - msg: tx_ack_rbf_msg, - }); - Ok(()) } else { try_channel_entry!( self,