From 76bb833806297e9b392a5213e1481edde84535cb Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 19 Mar 2026 14:52:08 -0700 Subject: [PATCH 1/2] Remove wallet argument from FundingTemplate::splice_out It does not require coin selection, so the wallet argument is not necessary. --- fuzz/src/chanmon_consistency.rs | 20 +--- fuzz/src/full_stack.rs | 10 +- lightning/src/ln/funding.rs | 182 +++++++++++++++++------------ lightning/src/ln/splicing_tests.rs | 11 +- 4 files changed, 118 insertions(+), 105 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index f20f93c789c..07aae78f22f 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -1504,7 +1504,6 @@ pub fn do_test(data: &[u8], out: Out) { counterparty_node_id: &PublicKey, channel_id: &ChannelId, wallet: &TestWalletSource, - logger: Arc, funding_feerate_sat_per_kw: FeeRate| { // We conditionally splice out `MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS` only when the node // has double the balance required to send a payment upon a `0xff` byte. We do this to @@ -1524,12 +1523,7 @@ pub fn do_test(data: &[u8], out: Out) { 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()), - ) + funding_template.splice_out(outputs, feerate, FeeRate::MAX) }); }; @@ -2450,30 +2444,26 @@ pub fn do_test(data: &[u8], out: Out) { 0xa4 => { let cp_node_id = nodes[1].get_our_node_id(); let wallet = &wallets[0]; - let logger = Arc::clone(&loggers[0]); let feerate_sat_per_kw = fee_estimators[0].feerate_sat_per_kw(); - splice_out(&nodes[0], &cp_node_id, &chan_a_id, wallet, logger, feerate_sat_per_kw); + splice_out(&nodes[0], &cp_node_id, &chan_a_id, wallet, feerate_sat_per_kw); }, 0xa5 => { let cp_node_id = nodes[0].get_our_node_id(); let wallet = &wallets[1]; - let logger = Arc::clone(&loggers[1]); let feerate_sat_per_kw = fee_estimators[1].feerate_sat_per_kw(); - splice_out(&nodes[1], &cp_node_id, &chan_a_id, wallet, logger, feerate_sat_per_kw); + splice_out(&nodes[1], &cp_node_id, &chan_a_id, wallet, feerate_sat_per_kw); }, 0xa6 => { let cp_node_id = nodes[2].get_our_node_id(); let wallet = &wallets[1]; - let logger = Arc::clone(&loggers[1]); let feerate_sat_per_kw = fee_estimators[1].feerate_sat_per_kw(); - splice_out(&nodes[1], &cp_node_id, &chan_b_id, wallet, logger, feerate_sat_per_kw); + splice_out(&nodes[1], &cp_node_id, &chan_b_id, wallet, feerate_sat_per_kw); }, 0xa7 => { let cp_node_id = nodes[1].get_our_node_id(); let wallet = &wallets[2]; - let logger = Arc::clone(&loggers[2]); let feerate_sat_per_kw = fee_estimators[2].feerate_sat_per_kw(); - splice_out(&nodes[2], &cp_node_id, &chan_b_id, wallet, logger, feerate_sat_per_kw); + splice_out(&nodes[2], &cp_node_id, &chan_b_id, wallet, feerate_sat_per_kw); }, // Sync node by 1 block to cover confirmation of a transaction. diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index c1d7982e5e4..f300ded4fb7 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -1083,13 +1083,9 @@ pub fn do_test(mut data: &[u8], logger: &Arc 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, - feerate, - FeeRate::MAX, - &wallet_sync, - ) { + if let Ok(contribution) = + funding_template.splice_out(outputs, feerate, FeeRate::MAX) + { let _ = channelmanager.funding_contributed( &chan_id, &counterparty, diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 923b429c4f5..0e8805d3e31 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -206,10 +206,12 @@ impl PriorContribution { /// 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_out`] 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. +/// These require `min_feerate` and `max_feerate` parameters. The splice-in variants perform +/// coin selection when wallet inputs are needed, while splice-out spends only from the channel +/// balance. /// /// # Replace By Fee (RBF) /// @@ -285,31 +287,13 @@ macro_rules! build_funding_contribution { let max_feerate: FeeRate = $max_feerate; let force_coin_selection: bool = $force_coin_selection; - if feerate > max_feerate { - return Err(FundingContributionError::FeeRateExceedsMaximum { feerate, max_feerate }); - } - - if let Some(min_rbf_feerate) = min_rbf_feerate { - if feerate < min_rbf_feerate { - return Err(FundingContributionError::FeeRateBelowRbfMinimum { feerate, min_rbf_feerate }); - } - } - - // 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() - // computation is -2 * MAX_MONEY (~-4.2e15), well within i64::MIN (~-9.2e18). - if value_added > Amount::MAX_MONEY { - return Err(FundingContributionError::InvalidSpliceValue); - } - - let mut value_removed = Amount::ZERO; - for txout in outputs.iter() { - value_removed = match value_removed.checked_add(txout.value) { - Some(sum) if sum <= Amount::MAX_MONEY => sum, - _ => return Err(FundingContributionError::InvalidSpliceValue), - }; - } + let value_removed = validate_funding_contribution_params( + value_added, + &outputs, + min_rbf_feerate, + feerate, + max_feerate, + )?; let is_splice = shared_input.is_some(); @@ -348,25 +332,52 @@ macro_rules! build_funding_contribution { let CoinSelection { confirmed_utxos: inputs, change_output } = coin_selection; - // The caller creating a FundingContribution is always the initiator for fee estimation - // purposes — this is conservative, overestimating rather than underestimating fees if - // the node ends up as the acceptor. - let estimated_fee = estimate_transaction_fee(&inputs, &outputs, change_output.as_ref(), true, is_splice, feerate); - debug_assert!(estimated_fee <= Amount::MAX_MONEY); - - let contribution = FundingContribution { + Ok(FundingContribution::new( value_added, - estimated_fee, - inputs, outputs, + inputs, change_output, feerate, max_feerate, is_splice, + )) + }}; +} + +fn validate_funding_contribution_params( + value_added: Amount, outputs: &[TxOut], min_rbf_feerate: Option, feerate: FeeRate, + max_feerate: FeeRate, +) -> Result { + if feerate > max_feerate { + return Err(FundingContributionError::FeeRateExceedsMaximum { feerate, max_feerate }); + } + + if let Some(min_rbf_feerate) = min_rbf_feerate { + if feerate < min_rbf_feerate { + return Err(FundingContributionError::FeeRateBelowRbfMinimum { + feerate, + min_rbf_feerate, + }); + } + } + + // 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() + // computation is -2 * MAX_MONEY (~-4.2e15), well within i64::MIN (~-9.2e18). + if value_added > Amount::MAX_MONEY { + return Err(FundingContributionError::InvalidSpliceValue); + } + + let mut value_removed = Amount::ZERO; + for txout in outputs.iter() { + value_removed = match value_removed.checked_add(txout.value) { + Some(sum) if sum <= Amount::MAX_MONEY => sum, + _ => return Err(FundingContributionError::InvalidSpliceValue), }; + } - Ok(contribution) - }}; + Ok(value_removed) } impl FundingTemplate { @@ -410,44 +421,37 @@ impl FundingTemplate { ) } - /// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to - /// perform coin selection. + /// Creates a [`FundingContribution`] for removing funds from a channel. + /// + /// Fees are paid from the channel balance, so this does not perform coin selection or spend + /// wallet inputs. /// /// `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(FundingContributionError::InvalidSpliceValue); - } - 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, + pub fn splice_out( + self, outputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, ) -> Result { if outputs.is_empty() { return Err(FundingContributionError::InvalidSpliceValue); } - let FundingTemplate { shared_input, min_rbf_feerate, .. } = self; - build_funding_contribution!( + validate_funding_contribution_params( + Amount::ZERO, + &outputs, + self.min_rbf_feerate, + min_feerate, + max_feerate, + )?; + Ok(FundingContribution::new( Amount::ZERO, outputs, - shared_input, - min_rbf_feerate, + vec![], + None, min_feerate, max_feerate, - false, - wallet, - ) + self.shared_input.is_some(), + )) } /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using @@ -708,6 +712,35 @@ impl_writeable_tlv_based!(FundingContribution, { }); impl FundingContribution { + fn new( + value_added: Amount, outputs: Vec, inputs: Vec, + change_output: Option, feerate: FeeRate, max_feerate: FeeRate, is_splice: bool, + ) -> Self { + // The caller creating a FundingContribution is always the initiator for fee estimation + // purposes — this is conservative, overestimating rather than underestimating fees if the + // node ends up as the acceptor. + let estimated_fee = estimate_transaction_fee( + &inputs, + &outputs, + change_output.as_ref(), + true, + is_splice, + feerate, + ); + debug_assert!(estimated_fee <= Amount::MAX_MONEY); + + Self { + value_added, + estimated_fee, + inputs, + outputs, + change_output, + feerate, + max_feerate, + is_splice, + } + } + pub(super) fn feerate(&self) -> FeeRate { self.feerate } @@ -1440,17 +1473,17 @@ mod tests { )); } - // splice_out_sync with single output value > MAX_MONEY + // splice_out with single output value > MAX_MONEY { let template = FundingTemplate::new(None, None, None); let outputs = vec![funding_output_sats(over_max.to_sat())]; assert!(matches!( - template.splice_out_sync(outputs, feerate, feerate, UnreachableWallet), + template.splice_out(outputs, feerate, feerate), Err(FundingContributionError::InvalidSpliceValue), )); } - // splice_out_sync with multiple outputs summing > MAX_MONEY + // splice_out with multiple outputs summing > MAX_MONEY { let template = FundingTemplate::new(None, None, None); let half_over = Amount::MAX_MONEY / 2 + Amount::from_sat(1); @@ -1459,7 +1492,7 @@ mod tests { funding_output_sats(half_over.to_sat()), ]; assert!(matches!( - template.splice_out_sync(outputs, feerate, feerate, UnreachableWallet), + template.splice_out(outputs, feerate, feerate), Err(FundingContributionError::InvalidSpliceValue), )); } @@ -2454,10 +2487,10 @@ mod tests { } #[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. + fn test_splice_out_skips_coin_selection_during_rbf() { + // When splice_out is called on a template with min_rbf_feerate set (user + // choosing a fresh splice-out instead of rbf_sync), fees still come from the + // channel balance rather than wallet inputs. 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); @@ -2465,12 +2498,11 @@ mod tests { 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(); + let contribution = + template.splice_out(vec![withdrawal.clone()], feerate, FeeRate::MAX).unwrap(); assert_eq!(contribution.value_added, Amount::ZERO); assert!(contribution.inputs.is_empty()); + assert!(contribution.change_output.is_none()); assert_eq!(contribution.outputs, vec![withdrawal]); } } diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 5279f2dfcc0..90c276fb4dd 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -271,9 +271,7 @@ pub fn initiate_splice_out<'a, 'b, 'c, 'd>( 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, feerate, FeeRate::MAX, &wallet).unwrap(); + let funding_contribution = funding_template.splice_out(outputs, feerate, FeeRate::MAX).unwrap(); match initiator.node.funding_contributed( &channel_id, &node_id_acceptor, @@ -1367,9 +1365,8 @@ 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).unwrap(); - let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let funding_contribution = - funding_template.splice_out_sync(outputs.clone(), feerate, FeeRate::MAX, &wallet).unwrap(); + funding_template.splice_out(outputs.clone(), feerate, FeeRate::MAX).unwrap(); nodes[0] .node .funding_contributed(&channel_id, &node_1_id, funding_contribution.clone(), None) @@ -6336,9 +6333,7 @@ fn test_splice_revalidation_at_quiescence() { 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).unwrap(); - let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - let contribution = - funding_template.splice_out_sync(outputs, feerate, FeeRate::MAX, &wallet).unwrap(); + let contribution = funding_template.splice_out(outputs, feerate, FeeRate::MAX).unwrap(); nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution.clone(), None).unwrap(); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty(), "stfu should be delayed"); From 19b230bccee656e2d2b3ae5b72822af5ef804f4f Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 30 Mar 2026 11:24:59 -0700 Subject: [PATCH 2/2] Introduce funding contribution builder API This commit introduces a `FundingBuilder`, following the builder pattern, to iteratively construct a `FundingContribution`. It supports adding/removing value (which may trigger a coin selection attempt), adding/remove outputs, and amending prior contributions (which was previously not possible). A `FundingBuilder` may be obtained in two ways: via `FundingTemplate::contribution_builder` (which resumes from the prior contribution to amend if available) or `FundingTemplate::without_prior_contribution_builder` to start from scratch and fully replace the prior contribution. When `FundingBuilder::build` is called, we attempt to build the contribution iteratively such that coin selection is our last resort. The splice-related convenience methods on `FundingTemplate` now map to the `FundingBuilder` under the hood. --- lightning/src/ln/funding.rs | 1800 +++++++++++++++++++++------- lightning/src/ln/splicing_tests.rs | 279 ++++- 2 files changed, 1621 insertions(+), 458 deletions(-) diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 0e8805d3e31..8f3a980614b 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -23,9 +23,7 @@ use crate::ln::types::ChannelId; use crate::ln::LN_MAX_MSG_LEN; use crate::prelude::*; use crate::util::native_async::MaybeSend; -use crate::util::wallet_utils::{ - CoinSelection, CoinSelectionSource, CoinSelectionSourceSync, Input, -}; +use crate::util::wallet_utils::{CoinSelectionSource, CoinSelectionSourceSync, Input}; /// Error returned when the acceptor's contribution cannot accommodate the initiator's proposed /// feerate. @@ -135,6 +133,8 @@ pub enum FundingContributionError { InvalidSpliceValue, /// Coin selection failed to find suitable inputs. CoinSelectionFailed, + /// Coin selection is required but no coin selection source was provided. + MissingCoinSelectionSource, /// This is not an RBF scenario (no minimum RBF feerate available). NotRbfScenario, } @@ -154,6 +154,9 @@ impl core::fmt::Display for FundingContributionError { FundingContributionError::CoinSelectionFailed => { write!(f, "Coin selection failed to find suitable inputs") }, + FundingContributionError::MissingCoinSelectionSource => { + write!(f, "Coin selection source required to build this contribution") + }, FundingContributionError::NotRbfScenario => { write!(f, "Not an RBF scenario (no minimum RBF feerate)") }, @@ -165,12 +168,12 @@ impl core::fmt::Display for FundingContributionError { /// /// 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`]. +/// the holder's balance for deferred feerate adjustment in +/// [`FundingTemplate::rbf_prior_contribution_sync`] or [`FundingTemplate::rbf_prior_contribution`]. /// -/// 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. +/// Use [`FundingTemplate::prior_contribution`] to inspect the prior contribution before deciding +/// whether to call [`FundingTemplate::rbf_prior_contribution_sync`] or start a fresh +/// [`FundingBuilder`] with different parameters. #[derive(Debug, Clone, PartialEq, Eq)] pub(super) struct PriorContribution { contribution: FundingContribution, @@ -203,28 +206,38 @@ impl PriorContribution { /// /// # 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`] to remove funds from the channel -/// - [`FundingTemplate::splice_in_and_out_sync`] to do both +/// Build a contribution with [`FundingTemplate::with_prior_contribution`] for builder-style +/// construction, [`FundingTemplate::splice_in`] to add funds, or [`FundingTemplate::splice_out`] to +/// withdraw funds. /// -/// These require `min_feerate` and `max_feerate` parameters. The splice-in variants perform -/// coin selection when wallet inputs are needed, while splice-out spends only from the channel -/// balance. +/// All of these APIs use the same underlying builder behavior. For a fresh splice, they start +/// from an empty request. When a prior contribution is present, they start from that prior state: +/// builder mutations amend the prior request, [`FundingTemplate::splice_in`] adds on top of the +/// prior [`FundingContribution::value_added`], and [`FundingTemplate::splice_out`] appends to the +/// prior [`FundingContribution::outputs`]. Use [`FundingTemplate::without_prior_contribution`] to +/// ignore the prior contribution and start from scratch. +/// +/// `min_feerate` is the feerate used for fee estimation and, when needed, coin selection. +/// `max_feerate` is the highest feerate we are willing to tolerate if we end up as the acceptor. +/// A wallet may be attached via [`FundingBuilder::with_coin_selection_source`] or +/// [`FundingBuilder::with_coin_selection_source_sync`], but coin selection is only performed when +/// the request cannot be satisfied by reusing the prior contribution or by a pure splice-out. +/// In particular, changing the prior contribution's `value_added` may require coin selection to +/// run again, yielding a different input set than the prior contribution used. /// /// # 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. +/// When a pending splice exists that hasn't been locked yet, +/// [`FundingTemplate::with_prior_contribution`] starts from the prior contribution by default. Use +/// [`FundingTemplate::without_prior_contribution`] to ignore it, or +/// [`FundingTemplate::rbf_prior_contribution_sync`] (or [`FundingTemplate::rbf_prior_contribution`] +/// for async) to reuse the prior contribution as-is. /// /// 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. +/// or build a fresh contribution with different parameters using the builder above. /// /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel /// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed @@ -253,8 +266,8 @@ impl FundingTemplate { /// 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. + /// When set, the `min_feerate` passed to the splice/builder methods must be at least this + /// value. pub fn min_rbf_feerate(&self) -> Option { self.min_rbf_feerate } @@ -264,8 +277,9 @@ impl FundingTemplate { /// /// 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. + /// whether to reuse it via [`FundingTemplate::rbf_prior_contribution`] or build a fresh + /// contribution with different parameters using + /// [`FundingTemplate::without_prior_contribution`]. /// /// Note: the returned contribution may reflect a different feerate than originally provided, /// as it may have been adjusted for RBF or for the counterparty's feerate when acting as @@ -275,233 +289,605 @@ impl FundingTemplate { pub fn prior_contribution(&self) -> Option<&FundingContribution> { self.prior_contribution.as_ref().map(|p| &p.contribution) } + + /// Creates a [`FundingBuilder`] for constructing a contribution. + /// + /// If a prior contribution is available, the builder starts from it automatically and builder + /// mutations amend that prior request. Use [`FundingTemplate::without_prior_contribution`] to + /// start empty instead. + /// + /// `feerate` is the feerate used for fee estimation and, if wallet inputs are needed, coin + /// selection. When [`FundingTemplate::min_rbf_feerate`] is set, it must be at least that value. + /// `max_feerate` is the highest feerate we are willing to tolerate if we end up as the + /// acceptor, and must be at least `feerate`. + pub fn with_prior_contribution(self, feerate: FeeRate, max_feerate: FeeRate) -> FundingBuilder { + FundingBuilder::new(self, feerate, max_feerate) + } + + /// Creates a [`FundingBuilder`] for constructing a contribution without using any prior + /// contribution. + /// + /// `feerate` and `max_feerate` have the same meaning as in + /// [`FundingTemplate::with_prior_contribution`]. This is useful when an RBF template carries a + /// prior contribution but the caller wants to replace, rather than amend, that request. + pub fn without_prior_contribution( + mut self, feerate: FeeRate, max_feerate: FeeRate, + ) -> FundingBuilder { + self.prior_contribution.take(); + FundingBuilder::new(self, feerate, max_feerate) + } } -macro_rules! build_funding_contribution { - ($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; - - let value_removed = validate_funding_contribution_params( - value_added, - &outputs, - min_rbf_feerate, - feerate, - max_feerate, - )?; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct NoCoinSelectionSource; + +#[derive(Debug, Clone, PartialEq, Eq)] +struct AsyncCoinSelectionSource(W); - let is_splice = shared_input.is_some(); +#[derive(Debug, Clone, PartialEq, Eq)] +struct SyncCoinSelectionSource(W); - 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 - // are unknown at this point. Only needed when selecting which UTXOs to include in the - // funding tx that would be sufficient to pay for fees. Hence, the value doesn't matter. - let dummy_pubkey = PublicKey::from_slice(&[2; 33]).unwrap(); - - let shared_output = bitcoin::TxOut { - value: shared_input - .as_ref() - .map(|shared_input| shared_input.previous_utxo.value) - .unwrap_or(Amount::ZERO) - .checked_add(value_added) - .ok_or(FundingContributionError::InvalidSpliceValue)? - .checked_sub(value_removed) - .ok_or(FundingContributionError::InvalidSpliceValue)?, - script_pubkey: make_funding_redeemscript(&dummy_pubkey, &dummy_pubkey).to_p2wsh(), - }; +#[derive(Debug, Clone, PartialEq, Eq)] +struct FundingBuilderInner { + shared_input: Option, + min_rbf_feerate: Option, + prior_contribution: Option, + value_added: Amount, + outputs: Vec, + feerate: FeeRate, + max_feerate: FeeRate, + state: State, +} - let claim_id = None; - let must_spend = shared_input.map(|input| vec![input]).unwrap_or_default(); - if outputs.is_empty() { - let must_pay_to = &[shared_output]; - $wallet.select_confirmed_utxos(claim_id, must_spend, must_pay_to, feerate.to_sat_per_kwu() as u32, u64::MAX)$(.$await)*.map_err(|_| FundingContributionError::CoinSelectionFailed)? - } else { - let must_pay_to: Vec<_> = outputs.iter().cloned().chain(core::iter::once(shared_output)).collect(); - $wallet.select_confirmed_utxos(claim_id, must_spend, &must_pay_to, feerate.to_sat_per_kwu() as u32, u64::MAX)$(.$await)*.map_err(|_| FundingContributionError::CoinSelectionFailed)? +/// A builder for creating or amending a [`FundingContribution`]. +/// +/// Use [`FundingTemplate::with_prior_contribution`] to build a contribution, seeded from any prior +/// contribution by default. Use [`FundingTemplate::without_prior_contribution`] to start fresh. +/// If you are in an RBF flow and just want to reuse the pending splice's prior contribution at an +/// RBF-compatible feerate without changing its requested value or outputs, use +/// [`FundingTemplate::rbf_prior_contribution_sync`] / +/// [`FundingTemplate::rbf_prior_contribution`] instead of the builder. +/// +/// Attach a wallet via [`FundingBuilder::with_coin_selection_source`] or +/// [`FundingBuilder::with_coin_selection_source_sync`] to obtain an [`AsyncFundingBuilder`] or +/// [`SyncFundingBuilder`]. When the builder is seeded from a prior contribution, +/// [`AsyncFundingBuilder::add_value`], [`AsyncFundingBuilder::remove_value`], +/// [`SyncFundingBuilder::add_value`], and [`SyncFundingBuilder::remove_value`] adjust the prior +/// [`FundingContribution::value_added`], while [`FundingBuilder::add_output`] and +/// [`FundingBuilder::remove_outputs`] adjust the prior [`FundingContribution::outputs`]. Building +/// first tries to reuse or amend the prior contribution without coin selection; only if that fails +/// and additional wallet inputs are required does it use an attached coin selection source. Thus, +/// changing `value_added` on a prior contribution is not guaranteed to preserve its previous +/// inputs. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FundingBuilder(FundingBuilderInner); + +/// A builder for creating or amending a [`FundingContribution`] with an attached asynchronous +/// [`CoinSelectionSource`]. +/// +/// Created by [`FundingBuilder::with_coin_selection_source`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AsyncFundingBuilder(FundingBuilderInner>); + +/// A builder for creating or amending a [`FundingContribution`] with an attached synchronous +/// [`CoinSelectionSourceSync`]. +/// +/// Created by [`FundingBuilder::with_coin_selection_source_sync`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SyncFundingBuilder(FundingBuilderInner>); + +impl FundingBuilderInner { + fn request_matches_prior(&self, prior_contribution: &FundingContribution) -> bool { + self.value_added == prior_contribution.value_added + && self.outputs == prior_contribution.outputs + } + + /// Tries to satisfy the current builder request by reusing or amending the prior contribution. + /// + /// If the request matches the prior contribution's `value_added` and `outputs`, this reprices + /// the prior contribution to `self.feerate` when the cached `holder_balance` shows that the new + /// fee can be absorbed. Otherwise it delegates to + /// [`FundingContribution::amend_without_coin_selection`] to rewrite the contribution in-place + /// without selecting new wallet inputs. + /// + /// Returns `None` if there is no prior contribution or if satisfying the updated request would + /// require coin selection. + fn amend_prior_contribution(&mut self) -> Option { + let PriorContribution { contribution, holder_balance } = + self.prior_contribution.as_ref()?; + if self.request_matches_prior(contribution) { + // Same request, but the feerate may have changed. Adjust the prior contribution to + // the new feerate if possible. + if let Some(holder_balance) = *holder_balance { + if contribution + .net_value_for_initiator_at_feerate(self.feerate, holder_balance) + .is_ok() + { + let contribution = self + .prior_contribution + .take() + .expect("prior contribution already checked") + .contribution; + let mut adjusted = contribution + .for_initiator_at_feerate(self.feerate, holder_balance) + .expect("feerate compatibility already checked"); + adjusted.max_feerate = self.max_feerate; + return Some(adjusted); + } } + return None; + } + + contribution.amend_without_coin_selection( + self.value_added, + self.outputs.clone(), + self.feerate, + self.max_feerate, + *holder_balance, + ) + } + + /// Tries to build the current request without selecting any new wallet inputs. + /// + /// This first attempts to reuse or amend any prior contribution. If there is no prior + /// contribution, it + /// also supports pure splice-out requests (`value_added == 0` with explicit `outputs`) by + /// building a contribution that pays fees from the channel balance. + /// + /// Returns `None` when additional wallet inputs are required. + fn try_build_without_coin_selection(&mut self) -> Option { + if let Some(contribution) = self.amend_prior_contribution() { + return Some(contribution); + } + + if self.prior_contribution.is_none() + && self.value_added == Amount::ZERO + && !self.outputs.is_empty() + { + return Some(FundingContribution::new( + Amount::ZERO, + self.outputs.clone(), + vec![], + None, + self.feerate, + self.max_feerate, + self.shared_input.is_some(), + )); + } + + None + } + + fn prepare_coin_selection_request( + &self, + ) -> Result<(Vec, Vec), FundingContributionError> { + let value_removed = self.outputs.iter().map(|output| output.value).sum(); + + let dummy_pubkey = PublicKey::from_slice(&[2; 33]).unwrap(); + let shared_output = bitcoin::TxOut { + value: self + .shared_input + .as_ref() + .map(|shared_input| shared_input.previous_utxo.value) + .unwrap_or(Amount::ZERO) + .checked_add(self.value_added) + .ok_or(FundingContributionError::InvalidSpliceValue)? + .checked_sub(value_removed) + .ok_or(FundingContributionError::InvalidSpliceValue)?, + script_pubkey: make_funding_redeemscript(&dummy_pubkey, &dummy_pubkey).to_p2wsh(), + }; + + let must_spend = self.shared_input.clone().map(|input| vec![input]).unwrap_or_default(); + let must_pay_to = if self.outputs.is_empty() { + vec![shared_output] + } else { + self.outputs.iter().cloned().chain(core::iter::once(shared_output)).collect() }; - // NOTE: Must NOT fail after UTXO selection + Ok((must_spend, must_pay_to)) + } + + fn validate_contribution_parameters(&self) -> Result<(), FundingContributionError> { + if self.feerate > self.max_feerate { + return Err(FundingContributionError::FeeRateExceedsMaximum { + feerate: self.feerate, + max_feerate: self.max_feerate, + }); + } + + if let Some(min_rbf_feerate) = self.min_rbf_feerate.as_ref() { + if self.feerate < *min_rbf_feerate { + return Err(FundingContributionError::FeeRateBelowRbfMinimum { + feerate: self.feerate, + min_rbf_feerate: *min_rbf_feerate, + }); + } + } + + if self.value_added == Amount::ZERO && self.outputs.is_empty() { + return Err(FundingContributionError::InvalidSpliceValue); + } + + // 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() + // computation is -2 * MAX_MONEY (~-4.2e15), well within i64::MIN (~-9.2e18). + if self.value_added > Amount::MAX_MONEY { + return Err(FundingContributionError::InvalidSpliceValue); + } + + let mut value_removed = Amount::ZERO; + for output in self.outputs.iter() { + value_removed = match value_removed.checked_add(output.value) { + Some(sum) if sum <= Amount::MAX_MONEY => sum, + _ => return Err(FundingContributionError::InvalidSpliceValue), + }; + } - let CoinSelection { confirmed_utxos: inputs, change_output } = coin_selection; + Ok(()) + } +} + +impl FundingBuilder { + fn new(template: FundingTemplate, feerate: FeeRate, max_feerate: FeeRate) -> FundingBuilder { + let FundingTemplate { shared_input, min_rbf_feerate, prior_contribution } = template; + let (value_added, outputs) = match prior_contribution.as_ref() { + Some(prior_contribution) => ( + prior_contribution.contribution.value_added, + prior_contribution.contribution.outputs.clone(), + ), + None => (Amount::ZERO, Vec::new()), + }; - Ok(FundingContribution::new( + FundingBuilder(FundingBuilderInner { + shared_input, + min_rbf_feerate, + prior_contribution, value_added, outputs, - inputs, - change_output, feerate, max_feerate, - is_splice, - )) - }}; + state: NoCoinSelectionSource, + }) + } + + /// Attaches an asynchronous [`CoinSelectionSource`] for later use. + /// + /// `wallet` is only consulted if [`AsyncFundingBuilder::build`] cannot satisfy the request by + /// reusing/amending the prior contribution or by building a pure splice-out contribution. + pub fn with_coin_selection_source( + self, wallet: W, + ) -> AsyncFundingBuilder { + AsyncFundingBuilder(self.0.with_state(AsyncCoinSelectionSource(wallet))) + } + + /// Attaches a synchronous [`CoinSelectionSourceSync`] for later use. + /// + /// `wallet` is only consulted if [`SyncFundingBuilder::build`] cannot satisfy the request by + /// reusing/amending the prior contribution or by building a pure splice-out contribution. + pub fn with_coin_selection_source_sync( + self, wallet: W, + ) -> SyncFundingBuilder { + SyncFundingBuilder(self.0.with_state(SyncCoinSelectionSource(wallet))) + } + + /// Adds a withdrawal output to the request. + /// + /// `output` is appended to the current set of explicit outputs. If the builder was seeded from + /// a prior contribution, this adds an additional withdrawal on top of the prior outputs. This + /// does not affect the change output. + pub fn add_output(self, output: TxOut) -> Self { + FundingBuilder(self.0.add_output_inner(output)) + } + + /// Removes all withdrawal outputs whose script pubkey matches `script_pubkey`. + /// + /// This only affects explicit outputs returned by [`FundingContribution::outputs`]; it never + /// removes the change output returned by [`FundingContribution::change_output`]. + pub fn remove_outputs(self, script_pubkey: &ScriptBuf) -> Self { + FundingBuilder(self.0.remove_outputs_inner(script_pubkey)) + } + + /// Builds a [`FundingContribution`] without coin selection. + /// + /// This succeeds when the request can be satisfied by reusing/amending a prior contribution or + /// by building a splice-out contribution that pays fees from the channel balance. + /// + /// Returns [`FundingContributionError::MissingCoinSelectionSource`] if additional wallet inputs + /// are needed. + pub fn build(self) -> Result { + let mut inner = self.0; + inner.build_without_coin_selection() + } } -fn validate_funding_contribution_params( - value_added: Amount, outputs: &[TxOut], min_rbf_feerate: Option, feerate: FeeRate, - max_feerate: FeeRate, -) -> Result { - if feerate > max_feerate { - return Err(FundingContributionError::FeeRateExceedsMaximum { feerate, max_feerate }); +impl FundingBuilderInner { + fn with_state(self, state: NewState) -> FundingBuilderInner { + FundingBuilderInner { + shared_input: self.shared_input, + min_rbf_feerate: self.min_rbf_feerate, + prior_contribution: self.prior_contribution, + value_added: self.value_added, + outputs: self.outputs, + feerate: self.feerate, + max_feerate: self.max_feerate, + state, + } } - if let Some(min_rbf_feerate) = min_rbf_feerate { - if feerate < min_rbf_feerate { - return Err(FundingContributionError::FeeRateBelowRbfMinimum { - feerate, - min_rbf_feerate, - }); + fn add_value_inner(mut self, value: Amount) -> Self { + self.value_added = + Amount::from_sat(self.value_added.to_sat().saturating_add(value.to_sat())); + self + } + + fn remove_value_inner(mut self, value: Amount) -> Self { + self.value_added = + Amount::from_sat(self.value_added.to_sat().saturating_sub(value.to_sat())); + self + } + + fn add_output_inner(mut self, output: TxOut) -> Self { + self.outputs.push(output); + self + } + + fn remove_outputs_inner(mut self, script_pubkey: &ScriptBuf) -> Self { + self.outputs.retain(|output| output.script_pubkey != *script_pubkey); + self + } + + /// Validates the current request and tries to build it without selecting new wallet inputs. + /// + /// This enforces the builder's feerate and amount invariants first. It then delegates to the + /// internal no-coin-selection path, which may reuse/amend a prior contribution or build a pure + /// splice-out directly. + /// + /// Returns [`FundingContributionError::MissingCoinSelectionSource`] if the request is otherwise + /// valid but requires new wallet inputs. + fn build_without_coin_selection( + &mut self, + ) -> Result { + self.validate_contribution_parameters()?; + + if let Some(contribution) = self.try_build_without_coin_selection() { + return Ok(contribution); } + + Err(FundingContributionError::MissingCoinSelectionSource) } +} - // 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() - // computation is -2 * MAX_MONEY (~-4.2e15), well within i64::MIN (~-9.2e18). - if value_added > Amount::MAX_MONEY { - return Err(FundingContributionError::InvalidSpliceValue); +impl AsyncFundingBuilder { + /// Adds a withdrawal output to the request. + /// + /// `output` is appended to the current set of explicit outputs. If the builder was seeded from + /// a prior contribution, this adds an additional withdrawal on top of the prior outputs. This + /// does not affect the change output. + pub fn add_output(self, output: TxOut) -> Self { + AsyncFundingBuilder(self.0.add_output_inner(output)) } - let mut value_removed = Amount::ZERO; - for txout in outputs.iter() { - value_removed = match value_removed.checked_add(txout.value) { - Some(sum) if sum <= Amount::MAX_MONEY => sum, - _ => return Err(FundingContributionError::InvalidSpliceValue), - }; + /// Removes all withdrawal outputs whose script pubkey matches `script_pubkey`. + /// + /// This only affects explicit outputs returned by [`FundingContribution::outputs`]; it never + /// removes the change output returned by [`FundingContribution::change_output`]. + pub fn remove_outputs(self, script_pubkey: &ScriptBuf) -> Self { + AsyncFundingBuilder(self.0.remove_outputs_inner(script_pubkey)) } - Ok(value_removed) + /// Increases the requested amount added to the channel. + /// + /// `value` is added on top of the builder's current request. If the builder was seeded from a + /// prior contribution, this increases that prior [`FundingContribution::value_added`]. If the + /// amended request cannot be satisfied in-place, [`AsyncFundingBuilder::build`] re-runs coin + /// selection and may return a contribution with a different input set than the prior one. + pub fn add_value(self, value: Amount) -> Self { + AsyncFundingBuilder(self.0.add_value_inner(value)) + } + + /// Decreases the requested amount added to the channel. + /// + /// `value` is subtracted from the builder's current request, saturating at zero. If the builder + /// was seeded from a prior contribution, this decreases that prior + /// [`FundingContribution::value_added`]. If the amended request cannot be satisfied in-place, + /// [`AsyncFundingBuilder::build`] re-runs coin selection and may return a contribution with a + /// different input set than the prior one. + pub fn remove_value(self, value: Amount) -> Self { + AsyncFundingBuilder(self.0.remove_value_inner(value)) + } } -impl FundingTemplate { - /// Creates a [`FundingContribution`] for adding funds to a channel using `wallet` to perform - /// coin selection. +impl AsyncFundingBuilder { + /// Builds a [`FundingContribution`] using the attached asynchronous coin selection source when + /// coin selection is required. /// - /// `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(FundingContributionError::InvalidSpliceValue); + /// If the request can be satisfied by reusing/amending a prior contribution or by a pure + /// splice-out, the attached wallet is ignored. + pub async fn build(self) -> Result { + let mut inner = self.0; + match inner.build_without_coin_selection() { + Err(FundingContributionError::MissingCoinSelectionSource) => {}, + other => return other, } - 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) + + let (must_spend, must_pay_to) = inner.prepare_coin_selection_request()?; + let AsyncCoinSelectionSource(wallet) = inner.state; + let coin_selection = wallet + .select_confirmed_utxos( + None, + must_spend, + &must_pay_to, + inner.feerate.to_sat_per_kwu() as u32, + u64::MAX, + ) + .await + .map_err(|_| FundingContributionError::CoinSelectionFailed)?; + + return Ok(FundingContribution::new( + inner.value_added, + inner.outputs, + coin_selection.confirmed_utxos, + coin_selection.change_output, + inner.feerate, + inner.max_feerate, + inner.shared_input.is_some(), + )); } +} - /// Creates a [`FundingContribution`] for adding funds to a channel using `wallet` to perform - /// coin selection. +impl SyncFundingBuilder { + /// Adds a withdrawal output to the request. /// - /// 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(FundingContributionError::InvalidSpliceValue); + /// `output` is appended to the current set of explicit outputs. If the builder was seeded from + /// a prior contribution, this adds an additional withdrawal on top of the prior outputs. This + /// does not affect the change output. + pub fn add_output(self, output: TxOut) -> Self { + SyncFundingBuilder(self.0.add_output_inner(output)) + } + + /// Removes all withdrawal outputs whose script pubkey matches `script_pubkey`. + /// + /// This only affects explicit outputs returned by [`FundingContribution::outputs`]; it never + /// removes the change output returned by [`FundingContribution::change_output`]. + pub fn remove_outputs(self, script_pubkey: &ScriptBuf) -> Self { + SyncFundingBuilder(self.0.remove_outputs_inner(script_pubkey)) + } + + /// Increases the requested amount added to the channel. + /// + /// `value` is added on top of the builder's current request. If the builder was seeded from a + /// prior contribution, this increases that prior [`FundingContribution::value_added`]. If the + /// amended request cannot be satisfied in-place, [`SyncFundingBuilder::build`] re-runs coin + /// selection and may return a contribution with a different input set than the prior one. + pub fn add_value(self, value: Amount) -> Self { + SyncFundingBuilder(self.0.add_value_inner(value)) + } + + /// Decreases the requested amount added to the channel. + /// + /// `value` is subtracted from the builder's current request, saturating at zero. If the builder + /// was seeded from a prior contribution, this decreases that prior + /// [`FundingContribution::value_added`]. If the amended request cannot be satisfied in-place, + /// [`SyncFundingBuilder::build`] re-runs coin selection and may return a contribution with a + /// different input set than the prior one. + pub fn remove_value(self, value: Amount) -> Self { + SyncFundingBuilder(self.0.remove_value_inner(value)) + } +} + +impl SyncFundingBuilder { + /// Builds a [`FundingContribution`] using the attached synchronous coin selection source when + /// coin selection is required. + /// + /// If the request can be satisfied by reusing/amending a prior contribution or by a pure + /// splice-out, the attached wallet is ignored. + pub fn build(self) -> Result { + let mut inner = self.0; + match inner.build_without_coin_selection() { + Err(FundingContributionError::MissingCoinSelectionSource) => {}, + other => return other, } - 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, - ) + + let (must_spend, must_pay_to) = inner.prepare_coin_selection_request()?; + let SyncCoinSelectionSource(wallet) = inner.state; + let coin_selection = wallet + .select_confirmed_utxos( + None, + must_spend, + &must_pay_to, + inner.feerate.to_sat_per_kwu() as u32, + u64::MAX, + ) + .map_err(|_| FundingContributionError::CoinSelectionFailed)?; + + return Ok(FundingContribution::new( + inner.value_added, + inner.outputs, + coin_selection.confirmed_utxos, + coin_selection.change_output, + inner.feerate, + inner.max_feerate, + inner.shared_input.is_some(), + )); } +} - /// Creates a [`FundingContribution`] for removing funds from a channel. +impl FundingTemplate { + /// Creates a [`FundingContribution`] for adding funds to a channel. /// - /// Fees are paid from the channel balance, so this does not perform coin selection or spend - /// wallet inputs. + /// This is a convenience wrapper around [`FundingTemplate::with_prior_contribution`]. As a + /// result, if this template carries a prior contribution, `value_added` is added on top of the + /// prior [`FundingContribution::value_added`] instead of replacing it. Use + /// [`FundingTemplate::without_prior_contribution`] if you want to replace the prior request + /// instead. /// - /// `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 fn splice_out( - self, outputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, + /// `value_added` is the amount of additional value to add to the channel. `min_feerate` is the + /// feerate used for fee estimation and, if needed, coin selection; when + /// [`FundingTemplate::min_rbf_feerate`] is set, it must be at least that value. `max_feerate` is + /// the highest feerate we are willing to tolerate if we end up as the acceptor, and must be at + /// least `min_feerate`. `wallet` is only consulted if the request cannot be satisfied by + /// reusing/amending the prior contribution. When this template carries a prior contribution, + /// increasing its value may therefore re-run coin selection and yield a different input set than + /// the prior contribution used. + pub async fn splice_in( + self, value_added: Amount, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, ) -> Result { - if outputs.is_empty() { - return Err(FundingContributionError::InvalidSpliceValue); - } - validate_funding_contribution_params( - Amount::ZERO, - &outputs, - self.min_rbf_feerate, - min_feerate, - max_feerate, - )?; - Ok(FundingContribution::new( - Amount::ZERO, - outputs, - vec![], - None, - min_feerate, - max_feerate, - self.shared_input.is_some(), - )) + self.with_prior_contribution(min_feerate, max_feerate) + .with_coin_selection_source(wallet) + .add_value(value_added) + .build() + .await } - /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using - /// `wallet` to perform coin selection. + /// Creates a [`FundingContribution`] for adding funds to a channel. /// - /// `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, + /// This is the synchronous variant of [`FundingTemplate::splice_in`]; `value_added`, + /// `min_feerate`, `max_feerate`, and `wallet` have the same meaning. + pub fn splice_in_sync( + self, value_added: Amount, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, ) -> Result { - if value_added == Amount::ZERO && outputs.is_empty() { - return Err(FundingContributionError::InvalidSpliceValue); - } - 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) + self.with_prior_contribution(min_feerate, max_feerate) + .with_coin_selection_source_sync(wallet) + .add_value(value_added) + .build() } - /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using - /// `wallet` to perform coin selection. + /// Creates a [`FundingContribution`] for removing funds from a channel. + /// + /// This is a convenience wrapper around [`FundingTemplate::with_prior_contribution`] with no + /// wallet attached. For a fresh splice, fees are paid from the channel balance, so this does + /// not perform coin selection or spend wallet inputs. When a prior contribution is present, + /// `outputs` are appended to the prior [`FundingContribution::outputs`] instead of replacing + /// them. Use [`FundingTemplate::without_prior_contribution`] if you want to replace the prior + /// outputs instead. + /// + /// `outputs` are the additional withdrawal outputs to include. `min_feerate` is the feerate + /// used for fee estimation and must be at least [`FundingTemplate::min_rbf_feerate`] when that + /// is set. `max_feerate` is the highest feerate we are willing to tolerate if we end up as the + /// acceptor, and must be at least `min_feerate`. /// - /// 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, + /// If amending a prior contribution would require selecting new wallet inputs, this method + /// returns [`FundingContributionError::MissingCoinSelectionSource`]. In that case, use the + /// builder APIs with a coin selection source instead. + pub fn splice_out( + self, outputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, ) -> Result { - if value_added == Amount::ZERO && outputs.is_empty() { - return Err(FundingContributionError::InvalidSpliceValue); + let mut builder = self.with_prior_contribution(min_feerate, max_feerate); + for output in outputs { + builder = builder.add_output(output); } - 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, - ) + builder.build() } /// 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. + /// `feerate` overrides the template's minimum RBF feerate. Passing `None` uses + /// [`FundingTemplate::min_rbf_feerate`]. `max_feerate` is the highest feerate we are willing to + /// tolerate if we end up as the acceptor, and must be at least the effective feerate. `wallet` + /// is only consulted if the prior contribution cannot be reused or adjusted directly. + /// The chosen `max_feerate` is stored on the returned contribution and also constrains coin + /// selection when prior contributions cannot be adjusted in-place. /// /// This handles the prior contribution logic internally: /// - If the prior contribution's feerate can be adjusted to the minimum RBF feerate, the @@ -512,108 +898,44 @@ impl FundingTemplate { /// parameters and the caller's `max_feerate`. For splice-out contributions, this changes /// the fee source: wallet inputs are selected to cover fees instead of deducting them /// from the channel balance. - /// - 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. /// /// # Errors /// /// Returns a [`FundingContributionError`] if this is not an RBF scenario, if `max_feerate` /// is below the minimum RBF feerate, or if coin selection fails. - pub async fn rbf( - self, max_feerate: FeeRate, wallet: W, + pub async fn rbf_prior_contribution( + self, feerate: Option, max_feerate: FeeRate, wallet: W, ) -> Result { - let FundingTemplate { shared_input, min_rbf_feerate, prior_contribution } = self; - let rbf_feerate = min_rbf_feerate.ok_or(FundingContributionError::NotRbfScenario)?; - if rbf_feerate > max_feerate { - return Err(FundingContributionError::FeeRateExceedsMaximum { - feerate: rbf_feerate, - max_feerate, - }); + if self.prior_contribution().is_none() { + return Err(FundingContributionError::NotRbfScenario); } + let feerate = feerate + .or_else(|| self.min_rbf_feerate()) + .ok_or(FundingContributionError::NotRbfScenario)?; - match prior_contribution { - Some(PriorContribution { contribution, holder_balance }) => { - // Try to adjust the prior contribution to the RBF feerate. This fails if - // the holder balance can't cover the adjustment (splice-out) or the fee - // buffer is insufficient (splice-in), or if the prior's feerate is already - // above rbf_feerate (e.g., from a counterparty-initiated RBF that locked - // at a higher feerate). In all cases, fall through to re-run coin selection. - 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) - }, - } + self.with_prior_contribution(feerate, max_feerate) + .with_coin_selection_source(wallet) + .build() + .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, + /// This is the synchronous variant of [`FundingTemplate::rbf_prior_contribution`]; `feerate`, + /// `max_feerate`, and `wallet` have the same meaning. + pub fn rbf_prior_contribution_sync( + self, feerate: Option, max_feerate: FeeRate, wallet: W, ) -> Result { - let FundingTemplate { shared_input, min_rbf_feerate, prior_contribution } = self; - let rbf_feerate = min_rbf_feerate.ok_or(FundingContributionError::NotRbfScenario)?; - if rbf_feerate > max_feerate { - return Err(FundingContributionError::FeeRateExceedsMaximum { - feerate: rbf_feerate, - max_feerate, - }); + if self.prior_contribution().is_none() { + return Err(FundingContributionError::NotRbfScenario); } + let feerate = feerate + .or_else(|| self.min_rbf_feerate()) + .ok_or(FundingContributionError::NotRbfScenario)?; - match prior_contribution { - Some(PriorContribution { contribution, holder_balance }) => { - // See comment in `rbf` for details on when this adjustment fails. - 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, - ) - }, - } + self.with_prior_contribution(feerate, max_feerate) + .with_coin_selection_source_sync(wallet) + .build() } } @@ -668,6 +990,11 @@ fn estimate_transaction_fee( } /// The components of a funding transaction contributed by one party. +/// +/// A contribution records the requested channel delta together with any wallet inputs and change +/// output needed to fund it. When derived from a prior contribution, its feerate, change output, +/// and even its final [`FundingContribution::value_added`] may be adjusted to account for a new +/// feerate. #[derive(Debug, Clone, PartialEq, Eq)] pub struct FundingContribution { /// The amount to contribute to the channel. @@ -758,6 +1085,11 @@ impl FundingContribution { } /// Returns the amount added to the channel by this contribution. + /// + /// This is the final amount encoded in the contribution, not necessarily the exact amount that + /// was originally requested. For example, when a prior contribution is repriced and its change + /// output is dropped, any remaining fee buffer is folded into the channel balance by increasing + /// this value instead of burning it as excess fees. pub fn value_added(&self) -> Amount { self.value_added } @@ -772,11 +1104,113 @@ impl FundingContribution { /// Returns the change output included in this contribution, if any. /// /// When coin selection provides more value than needed for the funding contribution and fees, - /// the surplus is returned to the wallet via this change output. + /// the surplus is returned to the wallet via this change output. When a prior contribution is + /// adjusted to a higher feerate, this output may shrink or disappear. pub fn change_output(&self) -> Option<&TxOut> { self.change_output.as_ref() } + /// Rewrites this contribution for a new request without selecting any new wallet inputs. + /// + /// `value_added` and `outputs` are the full replacement request to encode in the returned + /// contribution. `feerate` is the new fee-estimation feerate, and `max_feerate` is copied onto + /// the returned contribution for later acceptor-side fee checks. `holder_balance` is required + /// when this contribution has no inputs, since splice-out fees must then be paid from the channel + /// balance. + /// + /// For input-backed contributions, this tries to reuse the existing inputs and either shrink the + /// current change output or drop it entirely, folding any leftover fee buffer into + /// `value_added` instead of burning it as excess fees. For input-less contributions, it only + /// succeeds when the channel balance can cover `outputs` plus the newly estimated fee. Returns + /// `None` if the amendment would require new wallet inputs or if arithmetic/balance checks fail. + fn amend_without_coin_selection( + &self, value_added: Amount, outputs: Vec, feerate: FeeRate, max_feerate: FeeRate, + holder_balance: Option, + ) -> Option { + if self.inputs.is_empty() { + let holder_balance = holder_balance?; + if value_added != Amount::ZERO { + // Prior contribution didn't have any inputs, but now we need some. + return None; + } + + let estimated_fee = + estimate_transaction_fee(&[], &outputs, None, true, self.is_splice, feerate); + let value_removed = outputs.iter().map(|output| output.value).sum(); + let total_cost = estimated_fee.checked_add(value_removed)?; + if total_cost > holder_balance { + return None; + } + + return Some(FundingContribution { + value_added, + estimated_fee, + inputs: vec![], + outputs, + change_output: None, + feerate, + max_feerate, + is_splice: self.is_splice, + }); + } + + let total_input_value: Amount = + self.inputs.iter().map(|input| input.utxo.output.value).sum(); + let value_removed = outputs.iter().map(|output| output.value).sum(); + + if let Some(change_output) = self.change_output.as_ref() { + let estimated_fee = estimate_transaction_fee( + &self.inputs, + &outputs, + Some(change_output), + true, + self.is_splice, + feerate, + ); + let required_value = + value_added.checked_add(value_removed)?.checked_add(estimated_fee)?; + if total_input_value >= required_value { + let new_change_value = total_input_value.checked_sub(required_value)?; + if new_change_value >= change_output.script_pubkey.minimal_non_dust() { + let new_change_output = TxOut { + value: new_change_value, + script_pubkey: change_output.script_pubkey.clone(), + }; + return Some(FundingContribution { + value_added, + estimated_fee, + inputs: self.inputs.clone(), + outputs, + change_output: Some(new_change_output), + feerate, + max_feerate, + is_splice: self.is_splice, + }); + } + } + } + + let estimated_fee = + estimate_transaction_fee(&self.inputs, &outputs, None, true, self.is_splice, feerate); + let required_value = value_added.checked_add(value_removed)?.checked_add(estimated_fee)?; + if total_input_value < required_value { + return None; + } + let surplus = total_input_value.checked_sub(required_value)?; + let value_added = value_added.checked_add(surplus)?; + + Some(FundingContribution { + value_added, + estimated_fee, + inputs: self.inputs.clone(), + outputs, + change_output: None, + feerate, + max_feerate, + is_splice: self.is_splice, + }) + } + pub(super) fn into_tx_parts(self) -> (Vec, Vec) { let FundingContribution { inputs, mut outputs, change_output, .. } = self; @@ -812,7 +1246,11 @@ impl FundingContribution { } /// Validates that the funding inputs are suitable for use in the interactive transaction - /// protocol, checking prevtx sizes and input sufficiency. + /// protocol. + /// + /// This checks that each input's previous transaction fits in a `tx_add_input` message and that + /// contributed inputs are sufficient to cover the requested splice-in amount plus the stored fee + /// estimate. pub fn validate(&self) -> Result<(), String> { for FundingTxInput { utxo, prevtx, .. } in self.inputs.iter() { use crate::util::ser::Writeable; @@ -1117,9 +1555,10 @@ impl FundingContribution { } } - /// The net value contributed to a channel by the splice. If negative, more value will be - /// spliced out than spliced in. Fees will be deducted from the expected splice-out amount - /// if no inputs were included. + /// Returns the net value contributed to the channel by this splice. + /// + /// This is `value_added - sum(outputs)`, minus fees only when no wallet inputs are present. + /// A negative value means more is being withdrawn than added. pub fn net_value(&self) -> SignedAmount { self.net_value_with_fee(self.estimated_fee) } @@ -1496,38 +1935,163 @@ mod tests { Err(FundingContributionError::InvalidSpliceValue), )); } + } - // splice_in_and_out_sync with value_added > MAX_MONEY - { - let template = FundingTemplate::new(None, None, None); - let outputs = vec![funding_output_sats(1_000)]; - assert!(matches!( - template.splice_in_and_out_sync( - over_max, - outputs, - feerate, - feerate, - UnreachableWallet - ), - Err(FundingContributionError::InvalidSpliceValue), - )); - } + #[test] + fn test_with_prior_contribution_rejects_empty_contribution() { + let builder = FundingTemplate::new(Some(shared_input(100_000)), None, None) + .with_prior_contribution(FeeRate::from_sat_per_kwu(2000), FeeRate::MAX); + assert!(matches!(builder.build(), Err(FundingContributionError::InvalidSpliceValue))); + } - // splice_in_and_out_sync with output sum > MAX_MONEY - { - let template = FundingTemplate::new(None, None, None); - let outputs = vec![funding_output_sats(over_max.to_sat())]; - assert!(matches!( - template.splice_in_and_out_sync( - Amount::from_sat(1_000), - outputs, - feerate, - feerate, - UnreachableWallet, - ), - Err(FundingContributionError::InvalidSpliceValue), - )); - } + #[test] + fn test_rbf_with_prior_contribution_requires_coin_selection_source_when_holder_balance_missing() + { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2500); + let value_added = Amount::from_sat(50_000); + let change = funding_output_sats(90_000); + + let dummy_inputs = vec![funding_input_sats(1)]; + let estimated_fee = + estimate_transaction_fee(&dummy_inputs, &[], Some(&change), true, true, prior_feerate); + let input_value = value_added + estimated_fee + change.value; + + let prior = FundingContribution { + value_added, + estimated_fee, + inputs: vec![funding_input_sats(input_value.to_sat())], + outputs: vec![], + change_output: Some(change), + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let template = FundingTemplate::new( + Some(shared_input(100_000)), + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, None)), + ); + + assert!(matches!( + template.with_prior_contribution(min_rbf_feerate, FeeRate::MAX).build(), + Err(FundingContributionError::MissingCoinSelectionSource), + )); + } + + #[test] + fn test_rbf_with_prior_contribution_rejects_explicit_feerate_below_min_rbf_feerate() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2500); + let low_feerate = FeeRate::from_sat_per_kwu(2400); + + let inputs = vec![funding_input_sats(100_000)]; + let estimated_fee = estimate_transaction_fee(&inputs, &[], None, true, true, prior_feerate); + let prior = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee, + inputs, + outputs: vec![], + change_output: None, + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let template = FundingTemplate::new( + Some(shared_input(100_000)), + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, Some(Amount::MAX))), + ); + + assert!(matches!( + template.with_prior_contribution(low_feerate, FeeRate::MAX).build(), + Err(FundingContributionError::FeeRateBelowRbfMinimum { + feerate, + min_rbf_feerate: min_feerate, + }) if feerate == low_feerate && min_feerate == min_rbf_feerate, + )); + } + + #[test] + fn test_with_prior_contribution_with_coin_selection_source_sync_builds() { + let wallet = SingleUtxoWallet { + utxo: funding_input_sats(60_000), + change_output: Some(funding_output_sats(5_000)), + }; + + let contribution = FundingTemplate::new(Some(shared_input(100_000)), None, None) + .with_prior_contribution(FeeRate::from_sat_per_kwu(2000), FeeRate::MAX) + .with_coin_selection_source_sync(wallet) + .add_value(Amount::from_sat(50_000)) + .build() + .unwrap(); + + assert_eq!(contribution.value_added, Amount::from_sat(50_000)); + assert!(!contribution.inputs.is_empty()); + } + + #[test] + fn test_with_prior_contribution_uses_prior_contribution_by_default() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2500); + let prior_outputs = vec![funding_output_sats(10_000)]; + let prior_estimated_fee = + estimate_transaction_fee(&[], &prior_outputs, None, true, true, prior_feerate); + let prior = FundingContribution { + value_added: Amount::ZERO, + estimated_fee: prior_estimated_fee, + inputs: vec![], + outputs: prior_outputs.clone(), + change_output: None, + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let contribution = FundingTemplate::new( + Some(shared_input(100_000)), + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, Some(Amount::from_sat(100_000)))), + ) + .with_prior_contribution(min_rbf_feerate, FeeRate::MAX) + .build() + .unwrap(); + + assert_eq!(contribution.value_added, Amount::ZERO); + assert_eq!(contribution.outputs, prior_outputs); + assert_eq!(contribution.feerate, min_rbf_feerate); + } + + #[test] + fn test_without_prior_contribution_starts_empty() { + let prior_outputs = vec![funding_output_sats(10_000)]; + let prior = FundingContribution { + value_added: Amount::ZERO, + estimated_fee: Amount::from_sat(500), + inputs: vec![], + outputs: prior_outputs, + change_output: None, + feerate: FeeRate::from_sat_per_kwu(2000), + max_feerate: FeeRate::MAX, + is_splice: true, + }; + let new_output = funding_output_sats(20_000); + + let contribution = FundingTemplate::new( + Some(shared_input(100_000)), + Some(FeeRate::from_sat_per_kwu(2500)), + Some(PriorContribution::new(prior, Some(Amount::from_sat(100_000)))), + ) + .without_prior_contribution(FeeRate::from_sat_per_kwu(2500), FeeRate::MAX) + .add_output(new_output.clone()) + .build() + .unwrap(); + + assert_eq!(contribution.value_added, Amount::ZERO); + assert_eq!(contribution.outputs, vec![new_output]); + assert!(contribution.inputs.is_empty()); } #[test] @@ -1704,64 +2268,6 @@ mod tests { assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. }))); } - #[test] - fn test_for_acceptor_at_feerate_splice_out_sufficient() { - // Splice-out (no inputs): the fee estimate from the is_initiator=true overestimate covers - // the acceptor's target fee at a moderately higher target feerate. - let original_feerate = FeeRate::from_sat_per_kwu(2000); - let target_feerate = FeeRate::from_sat_per_kwu(3000); - let outputs = vec![funding_output_sats(50_000)]; - - let estimated_fee = - estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate); - - let contribution = FundingContribution { - value_added: Amount::ZERO, - estimated_fee, - inputs: vec![], - outputs: outputs.clone(), - change_output: None, - feerate: original_feerate, - max_feerate: FeeRate::MAX, - is_splice: true, - }; - - let contribution = - contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX).unwrap(); - // estimated_fee is updated to the target fee; surplus goes back to channel balance. - let expected_target_fee = - estimate_transaction_fee(&[], &outputs, None, false, true, target_feerate); - assert_eq!(contribution.estimated_fee, expected_target_fee); - assert!(expected_target_fee <= estimated_fee); - } - - #[test] - fn test_for_acceptor_at_feerate_splice_out_insufficient() { - // Splice-out: channel balance too small for outputs + target fee at high target feerate. - let original_feerate = FeeRate::from_sat_per_kwu(2000); - let target_feerate = FeeRate::from_sat_per_kwu(50_000); - let outputs = vec![funding_output_sats(50_000)]; - - let estimated_fee = - estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate); - - let contribution = FundingContribution { - value_added: Amount::ZERO, - estimated_fee, - inputs: vec![], - outputs, - change_output: None, - feerate: original_feerate, - max_feerate: FeeRate::MAX, - is_splice: true, - }; - - // Balance of 55,000 sats can't cover outputs (50,000) + target_fee at 50k sat/kwu. - let holder_balance = Amount::from_sat(55_000); - let result = contribution.for_acceptor_at_feerate(target_feerate, holder_balance); - assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. }))); - } - #[test] fn test_net_value_for_acceptor_at_feerate_splice_in() { // Splice-in: net_value_for_acceptor_at_feerate returns the same value as net_value() since @@ -2027,39 +2533,6 @@ mod tests { assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. }))); } - #[test] - fn test_for_acceptor_at_feerate_no_change_insufficient() { - // Inputs present, no change output. The target feerate is so high that the fee buffer - // (total input value minus value_added) cannot cover the target fee. - let original_feerate = FeeRate::from_sat_per_kwu(2000); - let target_feerate = FeeRate::from_sat_per_kwu(20_000); - let value_added = Amount::from_sat(1); - - // Compute estimated_fee first (weight-based, independent of input value). - let dummy_inputs = vec![funding_input_sats(1)]; - let estimated_fee = - estimate_transaction_fee(&dummy_inputs, &[], None, true, true, original_feerate); - - // Realistic input: value_added + estimated_fee (no surplus). - let inputs = vec![funding_input_sats((value_added + estimated_fee).to_sat())]; - let target_fee = estimate_transaction_fee(&inputs, &[], None, false, true, target_feerate); - assert!(target_fee > estimated_fee); - - let contribution = FundingContribution { - value_added, - estimated_fee, - inputs, - outputs: vec![], - change_output: None, - feerate: original_feerate, - max_feerate: FeeRate::MAX, - is_splice: true, - }; - - let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); - assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. }))); - } - #[test] fn test_for_acceptor_at_feerate_no_change_surplus_below_dust() { // Inputs present, no change output. The acceptor built their contribution at a low @@ -2316,7 +2789,7 @@ mod tests { Some(PriorContribution::new(prior, None)), ); assert!(matches!( - template.rbf_sync(max_feerate, UnreachableWallet), + template.rbf_prior_contribution_sync(None, max_feerate, UnreachableWallet), Err(FundingContributionError::FeeRateExceedsMaximum { .. }), )); } @@ -2350,11 +2823,489 @@ mod tests { Some(min_rbf_feerate), Some(PriorContribution::new(prior, Some(Amount::MAX))), ); - let contribution = template.rbf_sync(max_feerate, UnreachableWallet).unwrap(); + let contribution = + template.rbf_prior_contribution_sync(None, max_feerate, UnreachableWallet).unwrap(); assert_eq!(contribution.feerate, min_rbf_feerate); assert_eq!(contribution.max_feerate, max_feerate); } + #[test] + fn test_rbf_sync_uses_explicit_feerate_override() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2500); + let explicit_feerate = FeeRate::from_sat_per_kwu(3500); + let max_feerate = FeeRate::from_sat_per_kwu(5000); + let change = funding_output_sats(10_000); + let inputs = vec![funding_input_sats(100_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( + Some(shared_input(100_000)), + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, Some(Amount::MAX))), + ); + + let contribution = template + .rbf_prior_contribution_sync(Some(explicit_feerate), max_feerate, UnreachableWallet) + .unwrap(); + assert_eq!(contribution.feerate, explicit_feerate); + assert_eq!(contribution.max_feerate, max_feerate); + assert_ne!(contribution.feerate, min_rbf_feerate); + } + + #[test] + fn test_rbf_sync_same_request_uses_coin_selection_when_holder_balance_missing() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2500); + let value_added = Amount::from_sat(50_000); + let change = funding_output_sats(10_000); + + let dummy_inputs = vec![funding_input_sats(1)]; + let estimated_fee = + estimate_transaction_fee(&dummy_inputs, &[], Some(&change), true, true, prior_feerate); + let input_value = value_added + estimated_fee + change.value; + let prior = FundingContribution { + value_added, + estimated_fee, + inputs: vec![funding_input_sats(input_value.to_sat())], + outputs: vec![], + change_output: Some(change), + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let template = FundingTemplate::new( + Some(shared_input(100_000)), + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, None)), + ); + + let wallet_utxo = funding_input_sats(80_000); + let wallet_change = funding_output_sats(5_000); + let wallet = SingleUtxoWallet { + utxo: wallet_utxo.clone(), + change_output: Some(wallet_change.clone()), + }; + + let contribution = + template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet).unwrap(); + assert_eq!(contribution.value_added, value_added); + assert_eq!(contribution.inputs, vec![wallet_utxo]); + assert_eq!(contribution.change_output, Some(wallet_change)); + assert_eq!(contribution.feerate, min_rbf_feerate); + } + + #[test] + fn test_rbf_sync_same_request_uses_coin_selection_when_prior_cannot_adjust() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(20_000); + let value_added = Amount::from_sat(50_000); + + let dummy_inputs = vec![funding_input_sats(1)]; + let estimated_fee = + estimate_transaction_fee(&dummy_inputs, &[], None, true, true, prior_feerate); + let inputs = vec![funding_input_sats((value_added + estimated_fee).to_sat())]; + let target_fee = estimate_transaction_fee(&inputs, &[], None, true, true, min_rbf_feerate); + assert!(target_fee > estimated_fee); + + let prior = FundingContribution { + value_added, + estimated_fee, + inputs, + outputs: vec![], + change_output: None, + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let template = FundingTemplate::new( + Some(shared_input(100_000)), + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, Some(Amount::MAX))), + ); + + let wallet_utxo = funding_input_sats(80_000); + let wallet_change = funding_output_sats(5_000); + let wallet = SingleUtxoWallet { + utxo: wallet_utxo.clone(), + change_output: Some(wallet_change.clone()), + }; + + let contribution = + template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet).unwrap(); + assert_eq!(contribution.value_added, value_added); + assert_eq!(contribution.inputs, vec![wallet_utxo]); + assert_eq!(contribution.change_output, Some(wallet_change)); + assert_eq!(contribution.feerate, min_rbf_feerate); + } + + #[test] + fn test_rbf_with_prior_contribution_amends_prior_without_coin_selection() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2500); + let original_value_added = Amount::from_sat(50_000); + let added_value = Amount::from_sat(5_000); + let withdrawal = funding_output_sats(20_000); + let change = funding_output_sats(10_000); + + let dummy_inputs = vec![funding_input_sats(1)]; + let estimated_fee = estimate_transaction_fee( + &dummy_inputs, + std::slice::from_ref(&withdrawal), + Some(&change), + true, + true, + prior_feerate, + ); + let input_value = original_value_added + withdrawal.value + estimated_fee + change.value; + let inputs = vec![funding_input_sats(input_value.to_sat())]; + + let prior = FundingContribution { + value_added: original_value_added, + estimated_fee, + inputs: inputs.clone(), + outputs: vec![withdrawal.clone()], + change_output: Some(change), + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let template = FundingTemplate::new( + Some(shared_input(100_000)), + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, Some(Amount::MAX))), + ); + let min_rbf_feerate = template.min_rbf_feerate().unwrap(); + + let builder = template + .with_prior_contribution(min_rbf_feerate, FeeRate::MAX) + .remove_outputs(&withdrawal.script_pubkey) + .with_coin_selection_source_sync(UnreachableWallet) + .add_value(added_value); + + let contribution = builder.build().unwrap(); + assert_eq!(contribution.value_added, original_value_added + added_value); + assert!(contribution.outputs.is_empty()); + assert_eq!(contribution.inputs, inputs); + assert!(contribution.change_output.is_some()); + assert_eq!(contribution.feerate, min_rbf_feerate); + } + + #[test] + fn test_with_prior_contribution_remove_outputs_does_not_remove_change_output() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let value_added = Amount::from_sat(50_000); + let withdrawal = funding_output_sats(20_000); + let change_script = ScriptBuf::new_p2wpkh(&WPubkeyHash::from_slice(&[1; 20]).unwrap()); + let change = + TxOut { value: Amount::from_sat(10_000), script_pubkey: change_script.clone() }; + + let dummy_inputs = vec![funding_input_sats(1)]; + let estimated_fee = estimate_transaction_fee( + &dummy_inputs, + std::slice::from_ref(&withdrawal), + Some(&change), + true, + true, + prior_feerate, + ); + let input_value = value_added + withdrawal.value + estimated_fee + change.value; + let inputs = vec![funding_input_sats(input_value.to_sat())]; + + let prior = FundingContribution { + value_added, + estimated_fee, + inputs, + outputs: vec![withdrawal.clone()], + change_output: Some(change.clone()), + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let contribution = FundingTemplate::new( + Some(shared_input(100_000)), + None, + Some(PriorContribution::new(prior, Some(Amount::MAX))), + ) + .with_prior_contribution(prior_feerate, FeeRate::MAX) + .remove_outputs(&change_script) + .build() + .unwrap(); + + assert_eq!(contribution.outputs, vec![withdrawal]); + assert_eq!(contribution.change_output, Some(change)); + } + + #[test] + fn test_amend_prior_splice_out_without_inputs_succeeds() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let prior_outputs = vec![funding_output_sats(10_000)]; + let prior_estimated_fee = + estimate_transaction_fee(&[], &prior_outputs, None, true, true, prior_feerate); + let prior = FundingContribution { + value_added: Amount::ZERO, + estimated_fee: prior_estimated_fee, + inputs: vec![], + outputs: prior_outputs, + change_output: None, + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let amended_outputs = vec![funding_output_sats(20_000)]; + let amended_feerate = FeeRate::from_sat_per_kwu(3000); + let max_feerate = FeeRate::from_sat_per_kwu(10_000); + let amended = prior + .amend_without_coin_selection( + Amount::ZERO, + amended_outputs.clone(), + amended_feerate, + max_feerate, + Some(Amount::from_sat(100_000)), + ) + .unwrap(); + + assert_eq!(amended.value_added, Amount::ZERO); + assert!(amended.inputs.is_empty()); + assert_eq!(amended.outputs, amended_outputs.clone()); + assert!(amended.change_output.is_none()); + assert_eq!( + amended.estimated_fee, + estimate_transaction_fee(&[], &amended_outputs, None, true, true, amended_feerate), + ); + assert_eq!(amended.feerate, amended_feerate); + assert_eq!(amended.max_feerate, max_feerate); + } + + #[test] + fn test_amend_prior_splice_out_without_inputs_rejects_value_added() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let prior_outputs = vec![funding_output_sats(10_000)]; + let prior_estimated_fee = + estimate_transaction_fee(&[], &prior_outputs, None, true, true, prior_feerate); + let prior = FundingContribution { + value_added: Amount::ZERO, + estimated_fee: prior_estimated_fee, + inputs: vec![], + outputs: prior_outputs, + change_output: None, + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + assert!(prior + .amend_without_coin_selection( + Amount::ONE_SAT, + vec![funding_output_sats(20_000)], + FeeRate::from_sat_per_kwu(3000), + FeeRate::MAX, + Some(Amount::MAX), + ) + .is_none()); + } + + #[test] + fn test_amend_prior_splice_out_without_inputs_fails_when_holder_balance_is_too_low() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let prior_outputs = vec![funding_output_sats(10_000)]; + let prior_estimated_fee = + estimate_transaction_fee(&[], &prior_outputs, None, true, true, prior_feerate); + let prior = FundingContribution { + value_added: Amount::ZERO, + estimated_fee: prior_estimated_fee, + inputs: vec![], + outputs: prior_outputs, + change_output: None, + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + assert!(prior + .amend_without_coin_selection( + Amount::ZERO, + vec![funding_output_sats(50_000)], + FeeRate::from_sat_per_kwu(3000), + FeeRate::MAX, + Some(Amount::from_sat(50_000)), + ) + .is_none()); + } + + #[test] + fn test_amend_prior_with_change_removes_dust_change_output() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let amended_feerate = FeeRate::from_sat_per_kwu(7000); + let value_added = Amount::from_sat(50_000); + let change = funding_output_sats(500); + + let dummy_inputs = vec![funding_input_sats(1)]; + let estimated_fee = + estimate_transaction_fee(&dummy_inputs, &[], Some(&change), true, true, prior_feerate); + let target_fee_no_change = + estimate_transaction_fee(&dummy_inputs, &[], None, true, true, amended_feerate); + let input_value = value_added + target_fee_no_change + Amount::from_sat(100); + let inputs = vec![funding_input_sats(input_value.to_sat())]; + let target_fee_with_change = + estimate_transaction_fee(&inputs, &[], Some(&change), true, true, amended_feerate); + let dust_limit = change.script_pubkey.minimal_non_dust(); + let amended_change_value = input_value + .checked_sub(value_added) + .and_then(|value| value.checked_sub(target_fee_with_change)); + assert!(amended_change_value.map_or(true, |value| value < dust_limit)); + + let prior = FundingContribution { + value_added, + estimated_fee, + inputs: inputs.clone(), + outputs: vec![], + change_output: Some(change.clone()), + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let amended = prior + .amend_without_coin_selection( + value_added, + vec![], + amended_feerate, + FeeRate::MAX, + Some(Amount::MAX), + ) + .unwrap(); + + assert_eq!(amended.inputs, inputs.clone()); + assert!(amended.change_output.is_none()); + assert_eq!(amended.estimated_fee, target_fee_no_change); + assert_eq!(amended.value_added, value_added + Amount::from_sat(100)); + } + + #[test] + fn test_amend_prior_without_change_uses_existing_fee_buffer() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let amended_feerate = FeeRate::from_sat_per_kwu(4000); + let value_added = Amount::from_sat(50_000); + let outputs = vec![funding_output_sats(20_000)]; + + let dummy_inputs = vec![funding_input_sats(1)]; + let prior_estimated_fee = + estimate_transaction_fee(&dummy_inputs, &outputs, None, true, true, prior_feerate); + let target_fee = + estimate_transaction_fee(&dummy_inputs, &outputs, None, true, true, amended_feerate); + let inputs = vec![funding_input_sats( + (value_added + outputs[0].value + target_fee + Amount::from_sat(1_000)).to_sat(), + )]; + + let prior = FundingContribution { + value_added, + estimated_fee: prior_estimated_fee, + inputs: inputs.clone(), + outputs: outputs.clone(), + change_output: None, + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let amended = prior + .amend_without_coin_selection( + value_added, + outputs.clone(), + amended_feerate, + FeeRate::MAX, + Some(Amount::MAX), + ) + .unwrap(); + + assert_eq!(amended.inputs, inputs.clone()); + assert_eq!(amended.outputs, outputs); + assert!(amended.change_output.is_none()); + assert_eq!( + amended.estimated_fee, + estimate_transaction_fee(&inputs, &amended.outputs, None, true, true, amended_feerate), + ); + assert_eq!(amended.value_added, value_added + Amount::from_sat(1_000)); + } + + #[test] + fn test_rbf_with_prior_contribution_uses_coin_selection_when_required() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2500); + let value_added = Amount::from_sat(50_000); + + let dummy_inputs = vec![funding_input_sats(1)]; + let estimated_fee = + estimate_transaction_fee(&dummy_inputs, &[], None, true, true, prior_feerate); + let inputs = vec![funding_input_sats((value_added + estimated_fee).to_sat())]; + + let prior = FundingContribution { + value_added, + estimated_fee, + inputs, + outputs: vec![], + change_output: None, + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let template = FundingTemplate::new( + Some(shared_input(100_000)), + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, Some(Amount::MAX))), + ); + + let wallet = SingleUtxoWallet { + utxo: funding_input_sats(60_000), + change_output: Some(funding_output_sats(5_000)), + }; + let min_rbf_feerate = template.min_rbf_feerate().unwrap(); + let contribution = template + .with_prior_contribution(min_rbf_feerate, FeeRate::MAX) + .with_coin_selection_source_sync(wallet) + .add_value(Amount::ONE_SAT) + .build() + .unwrap(); + assert_eq!(contribution.value_added, value_added + Amount::ONE_SAT); + assert_eq!(contribution.feerate, min_rbf_feerate); + assert!(!contribution.inputs.is_empty()); + } + + #[test] + fn test_with_prior_contribution_splice_out_build_skips_coin_selection() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let withdrawal = funding_output_sats(20_000); + + let builder = FundingTemplate::new(Some(shared_input(100_000)), None, None) + .with_prior_contribution(feerate, FeeRate::MAX) + .add_output(withdrawal.clone()); + + let contribution = builder.build().unwrap(); + assert_eq!(contribution.value_added, Amount::ZERO); + assert!(contribution.inputs.is_empty()); + assert!(contribution.change_output.is_none()); + assert_eq!(contribution.outputs, vec![withdrawal]); + } + /// A mock wallet that returns a single UTXO for coin selection. struct SingleUtxoWallet { utxo: FundingTxInput, @@ -2419,7 +3370,8 @@ mod tests { }; // rbf_sync should succeed and the contribution should have inputs from coin selection. - let contribution = template.rbf_sync(FeeRate::MAX, &wallet).unwrap(); + let contribution = + template.rbf_prior_contribution_sync(None, 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]); @@ -2427,9 +3379,8 @@ mod tests { } #[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. + fn test_rbf_sync_returns_err_without_prior_contribution() { + // When there is no prior contribution (e.g., acceptor), RBF should be rejected. let min_rbf_feerate = FeeRate::from_sat_per_kwu(5000); let template = @@ -2440,11 +3391,7 @@ mod tests { 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); + assert!(template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet).is_err()); } #[test] @@ -2479,7 +3426,8 @@ mod tests { change_output: Some(funding_output_sats(40_000)), }; - let contribution = template.rbf_sync(callers_max_feerate, &wallet).unwrap(); + let contribution = + template.rbf_prior_contribution_sync(None, callers_max_feerate, &wallet).unwrap(); assert_eq!( contribution.max_feerate, callers_max_feerate, "should use caller's max_feerate, not prior's" diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 90c276fb4dd..1fa4c715ef7 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -127,6 +127,9 @@ struct TightBudgetWallet { change_value: Amount, } +#[cfg(test)] +struct UnreachableCoinSelectionWallet; + #[cfg(test)] impl CoinSelectionSourceSync for TightBudgetWallet { fn select_confirmed_utxos( @@ -158,6 +161,20 @@ impl CoinSelectionSourceSync for TightBudgetWallet { } } +#[cfg(test)] +impl CoinSelectionSourceSync for UnreachableCoinSelectionWallet { + 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 { + panic!("coin selection should not run") + } + + fn sign_psbt(&self, _psbt: Psbt) -> Result { + unreachable!("should not reach signing") + } +} + #[test] fn test_validate_accounts_for_change_output_weight() { // Demonstrates that estimated_fee includes the change output's weight when building a @@ -254,9 +271,14 @@ pub fn do_initiate_rbf_splice_in_and_out<'a, 'b, 'c, 'd>( let node_id_counterparty = counterparty.node.get_our_node_id(); 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) - .unwrap(); + let mut builder = funding_template + .without_prior_contribution(feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(value_added); + for output in outputs { + builder = builder.add_output(output); + } + let funding_contribution = builder.build().unwrap(); node.node .funding_contributed(&channel_id, &node_id_counterparty, funding_contribution.clone(), None) .unwrap(); @@ -302,9 +324,14 @@ pub fn do_initiate_splice_in_and_out<'a, 'b, 'c, 'd>( 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, feerate, FeeRate::MAX, &wallet) - .unwrap(); + let mut builder = funding_template + .with_prior_contribution(feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(value_added); + for output in outputs { + builder = builder.add_output(output); + } + let funding_contribution = builder.build().unwrap(); initiator .node .funding_contributed(&channel_id, &node_id_acceptor, funding_contribution.clone(), None) @@ -3683,7 +3710,7 @@ fn test_funding_contributed_splice_already_pending() { let splice_in_amount = Amount::from_sat(20_000); provide_utxo_reserves(&nodes, 2, splice_in_amount * 2); - // Use splice_in_and_out with an output so we can test output filtering + // Use the contribution builder with an output so we can test output filtering let first_splice_out = TxOut { value: Amount::from_sat(5_000), script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_raw_hash(Hash::all_zeros())), @@ -3692,13 +3719,11 @@ fn test_funding_contributed_splice_already_pending() { 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()], - feerate, - FeeRate::MAX, - &wallet, - ) + .with_prior_contribution(feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(splice_in_amount) + .add_output(first_splice_out.clone()) + .build() .unwrap(); // Initiate a second splice with a DIFFERENT output to test that different outputs @@ -3720,13 +3745,11 @@ fn test_funding_contributed_splice_already_pending() { 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()], - feerate, - FeeRate::MAX, - &wallet, - ) + .without_prior_contribution(feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(splice_in_amount) + .add_output(second_splice_out.clone()) + .build() .unwrap(); // First funding_contributed - this sets up the quiescent action @@ -5455,7 +5478,7 @@ fn test_splice_rbf_after_counterparty_rbf_aborted() { nodes[0].node.get_and_clear_pending_events(); nodes[1].node.get_and_clear_pending_events(); - // Step 5: Node 1 initiates its own RBF via splice_channel → rbf_sync. + // Step 5: Node 1 initiates its own RBF via splice_channel → rbf_prior_contribution_sync. // The prior contribution's feerate is restored to the original floor feerate, not the // RBF-adjusted feerate. provide_utxo_reserves(&nodes, 2, added_value * 2); @@ -5469,7 +5492,8 @@ fn test_splice_rbf_after_counterparty_rbf_aborted() { ); let wallet = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); - let rbf_contribution = funding_template.rbf_sync(FeeRate::MAX, &wallet); + let rbf_contribution = + funding_template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet); assert!(rbf_contribution.is_ok()); } @@ -5670,6 +5694,194 @@ fn test_splice_rbf_sequential() { ); } +#[test] +fn test_splice_rbf_iterative_contribution_amendments() { + 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 splice_in_value = Amount::from_sat(20_000); + let splice_out = TxOut { + value: Amount::from_sat(10_000), + script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_raw_hash(Hash::all_zeros())), + }; + + // Round 0: start with a splice-out only. + let initial_contribution = + initiate_splice_out(&nodes[0], &nodes[1], channel_id, vec![splice_out.clone()]).unwrap(); + assert_eq!(initial_contribution.value_added(), Amount::ZERO); + assert_eq!(initial_contribution.outputs(), &[splice_out.clone()][..]); + + let (splice_tx_0, new_funding_script) = + splice_channel(&nodes[0], &nodes[1], channel_id, initial_contribution); + + let rbf_splice = |contribution: FundingContribution| -> Transaction { + nodes[0] + .node + .funding_contributed(&channel_id, &node_id_1, contribution.clone(), None) + .unwrap(); + complete_rbf_handshake(&nodes[0], &nodes[1]); + complete_interactive_funding_negotiation( + &nodes[0], + &nodes[1], + channel_id, + contribution, + new_funding_script.clone(), + ); + let (splice_tx, splice_locked) = sign_interactive_funding_tx(&nodes[0], &nodes[1], false); + assert!(splice_locked.is_none()); + expect_splice_pending_event(&nodes[0], &node_id_1); + expect_splice_pending_event(&nodes[1], &node_id_0); + splice_tx + }; + + // Round 1: RBF the splice-out to also splice funds in. + provide_utxo_reserves(&nodes, 2, splice_in_value * 2); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let prior_contribution = funding_template.prior_contribution().unwrap(); + assert_eq!(prior_contribution.value_added(), Amount::ZERO); + assert_eq!(prior_contribution.outputs(), &[splice_out.clone()][..]); + let min_rbf_feerate = funding_template.min_rbf_feerate().unwrap(); + + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let first_rbf_contribution = funding_template + .with_prior_contribution(min_rbf_feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(splice_in_value) + .build() + .unwrap(); + assert_eq!(first_rbf_contribution.value_added(), splice_in_value); + assert_eq!(first_rbf_contribution.outputs(), &[splice_out.clone()][..]); + let splice_tx_1 = rbf_splice(first_rbf_contribution); + + // Round 2: RBF again, removing the splice-out while keeping the splice-in. We intentionally + // don't use `with_coin_selection_source_sync` to ensure coin selection is only re-run when + // necessary. + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let prior_contribution = funding_template.prior_contribution().unwrap(); + assert_eq!(prior_contribution.value_added(), splice_in_value); + assert_eq!(prior_contribution.outputs(), &[splice_out.clone()][..]); + let min_rbf_feerate = funding_template.min_rbf_feerate().unwrap(); + + let second_rbf_contribution = funding_template + .with_prior_contribution(min_rbf_feerate, FeeRate::MAX) + .remove_outputs(&splice_out.script_pubkey) + .build() + .unwrap(); + assert_eq!(second_rbf_contribution.value_added(), splice_in_value); + assert!(second_rbf_contribution.outputs().is_empty()); + let splice_tx_2 = rbf_splice(second_rbf_contribution); + + // Round 3: first try to RBF away the remaining splice-in entirely. This leaves an empty + // contribution, so building it should fail locally. + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let prior_contribution = funding_template.prior_contribution().unwrap(); + assert_eq!(prior_contribution.value_added(), splice_in_value); + assert!(prior_contribution.outputs().is_empty()); + let min_rbf_feerate = funding_template.min_rbf_feerate().unwrap(); + + assert!(matches!( + funding_template + .with_prior_contribution(min_rbf_feerate, FeeRate::MAX) + .with_coin_selection_source_sync(UnreachableCoinSelectionWallet) + .remove_value(splice_in_value) + .build(), + Err(crate::ln::funding::FundingContributionError::InvalidSpliceValue), + )); + + // The failed local build should not disturb the prior contribution tracked for the pending + // splice. Retry with a valid amendment that removes only half the splice-in value. Attach a + // wallet that panics if coin selection runs so we prove this round amends the prior + // contribution directly. + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let prior_contribution = funding_template.prior_contribution().unwrap(); + assert_eq!(prior_contribution.value_added(), splice_in_value); + assert!(prior_contribution.outputs().is_empty()); + let min_rbf_feerate = funding_template.min_rbf_feerate().unwrap(); + + let half_splice_in_value = splice_in_value / 2; + let third_rbf_contribution = funding_template + .with_prior_contribution(min_rbf_feerate, FeeRate::MAX) + .with_coin_selection_source_sync(UnreachableCoinSelectionWallet) + .remove_value(half_splice_in_value) + .build() + .unwrap(); + assert_eq!(third_rbf_contribution.value_added(), splice_in_value - half_splice_in_value); + assert!(third_rbf_contribution.outputs().is_empty()); + let splice_tx_3 = rbf_splice(third_rbf_contribution); + + // Round 4: splice the removed value back in and prove we still amend the prior contribution + // without coin selection. + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let prior_contribution = funding_template.prior_contribution().unwrap(); + assert_eq!(prior_contribution.value_added(), splice_in_value - half_splice_in_value); + assert!(prior_contribution.outputs().is_empty()); + let min_rbf_feerate = funding_template.min_rbf_feerate().unwrap(); + + let fourth_rbf_contribution = funding_template + .with_prior_contribution(min_rbf_feerate, FeeRate::MAX) + .with_coin_selection_source_sync(UnreachableCoinSelectionWallet) + .add_value(half_splice_in_value) + .build() + .unwrap(); + assert_eq!(fourth_rbf_contribution.value_added(), splice_in_value); + assert!(fourth_rbf_contribution.outputs().is_empty()); + let splice_tx_4 = rbf_splice(fourth_rbf_contribution); + + // Round 5: add enough value that the prior inputs can no longer cover the amended + // contribution, forcing coin selection to run and add another wallet input. + provide_utxo_reserves(&nodes, 1, Amount::from_sat(40_000)); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + let prior_contribution = funding_template.prior_contribution().unwrap(); + assert_eq!(prior_contribution.value_added(), splice_in_value); + assert!(prior_contribution.outputs().is_empty()); + let prior_input_count = prior_contribution.contributed_inputs().count(); + let min_rbf_feerate = funding_template.min_rbf_feerate().unwrap(); + + let coin_selection_splice_in_value = Amount::from_sat(50_000); + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let fifth_rbf_contribution = funding_template + .with_prior_contribution(min_rbf_feerate, FeeRate::MAX) + .with_coin_selection_source_sync(wallet) + .add_value(coin_selection_splice_in_value) + .build() + .unwrap(); + assert_eq!( + fifth_rbf_contribution.value_added(), + splice_in_value + coin_selection_splice_in_value + ); + assert!(fifth_rbf_contribution.outputs().is_empty()); + assert!(fifth_rbf_contribution.contributed_inputs().count() > prior_input_count); + let splice_tx_5 = rbf_splice(fifth_rbf_contribution); + + // Lock the latest valid RBF and ensure the replaced splice candidates were discarded. + lock_rbf_splice_after_blocks( + &nodes[0], + &nodes[1], + &splice_tx_5, + ANTI_REORG_DELAY - 1, + &[ + splice_tx_0.compute_txid(), + splice_tx_1.compute_txid(), + splice_tx_2.compute_txid(), + splice_tx_3.compute_txid(), + splice_tx_4.compute_txid(), + ], + ); + + let fresh_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + assert!(fresh_template.min_rbf_feerate().is_none()); + assert!(fresh_template.prior_contribution().is_none()); +} + #[test] fn test_splice_rbf_acceptor_contributes_then_disconnects() { // When both nodes contribute to a splice and the initiator RBFs (with the acceptor @@ -5925,7 +6137,7 @@ fn test_splice_channel_with_pending_splice_includes_rbf_floor() { // rbf_sync returns the Adjusted prior contribution directly. let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - assert!(funding_template.rbf_sync(FeeRate::MAX, &wallet).is_ok()); + assert!(funding_template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet).is_ok()); } #[test] @@ -6163,7 +6375,7 @@ fn test_prior_contribution_unadjusted_when_max_feerate_too_low() { 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()); + assert!(funding_template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet).is_ok()); } #[test] @@ -6205,11 +6417,10 @@ fn test_splice_channel_during_negotiation_includes_rbf_feerate() { 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. + // No prior contribution since there are no negotiated candidates yet, so RBF is rejected. 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()); + assert!(template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet).is_err()); } #[test] @@ -6237,7 +6448,7 @@ fn test_rbf_sync_returns_err_when_no_min_rbf_feerate() { let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); assert!(matches!( - template.rbf_sync(FeeRate::MAX, &wallet), + template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet), Err(crate::ln::funding::FundingContributionError::NotRbfScenario), )); } @@ -6273,7 +6484,7 @@ fn test_rbf_sync_returns_err_when_max_feerate_below_min_rbf() { 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!(matches!( - funding_template.rbf_sync(too_low_feerate, &wallet), + funding_template.rbf_prior_contribution_sync(None, too_low_feerate, &wallet), Err(crate::ln::funding::FundingContributionError::FeeRateExceedsMaximum { .. }), )); } @@ -6543,8 +6754,12 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { let rbf_feerate = FeeRate::from_sat_per_kwu(next_feerate); 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, rbf_feerate, FeeRate::MAX, &wallet).unwrap(); + let contribution = funding_template + .without_prior_contribution(rbf_feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(added_value) + .build() + .unwrap(); let result = nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution, None); assert!(result.is_err(), "Expected rejection for low feerate: {:?}", result);