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..1f1cf425c92 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, @@ -1890,8 +1886,8 @@ fn splice_seed() -> Vec { // CommitmentSigned message with proper signature (r=f7, s=01...) and funding_txid TLV // signature r encodes sighash first byte f7, s follows the pattern from funding_created // TLV type 1 (odd/optional) for funding_txid as per impl_writeable_msg!(CommitmentSigned, ...) - // Note: txid is encoded in reverse byte order (Bitcoin standard), so to get display 0000...0033, encode 3300...0000 - ext_from_hex("0084 c000000000000000000000000000000000000000000000000000000000000000 00000000000000000000000000000000000000000000000000000000000000f7 0100000000000000000000000000000000000000000000000000000000000000 0000 01 20 3300000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test); + // Note: txid is encoded in reverse byte order (Bitcoin standard), so to get display 0000...0031, encode 3100...0000 + ext_from_hex("0084 c000000000000000000000000000000000000000000000000000000000000000 00000000000000000000000000000000000000000000000000000000000000f7 0100000000000000000000000000000000000000000000000000000000000000 0000 01 20 3100000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test); // After commitment_signed exchange, we need to exchange tx_signatures. // Message type IDs: TxSignatures = 71 (0x0047) @@ -1904,19 +1900,19 @@ fn splice_seed() -> Vec { // inbound read from peer id 0 of len 150 (134 message + 16 MAC) ext_from_hex("030096", &mut test); // TxSignatures message with shared_input_signature TLV (type 0) - // txid must match the splice funding txid (0x33 in reverse byte order) + // txid must match the splice funding txid (0x31 in reverse byte order) // shared_input_signature: 64-byte fuzz signature for the shared input - ext_from_hex("0047 c000000000000000000000000000000000000000000000000000000000000000 3300000000000000000000000000000000000000000000000000000000000000 0000 00 40 00000000000000000000000000000000000000000000000000000000000000dc 0100000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test); + ext_from_hex("0047 c000000000000000000000000000000000000000000000000000000000000000 3100000000000000000000000000000000000000000000000000000000000000 0000 00 40 00000000000000000000000000000000000000000000000000000000000000dc 0100000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test); // Connect a block with the splice funding transaction to confirm it // The splice funding tx: version(4) + input_count(1) + txid(32) + vout(4) + script_len(1) + sequence(4) // + output_count(1) + value(8) + script_len(1) + script(34) + locktime(4) = 94 bytes = 0x5e // Transaction structure from FundingTransactionReadyForSigning: // - Input: spending c000...00:0 with sequence 0xfffffffd - // - Output: 115536 sats to OP_0 PUSH32 6e00...00 + // - Output: 115538 sats to OP_0 PUSH32 6e00...00 // - Locktime: 13 ext_from_hex("0c005e", &mut test); - ext_from_hex("02000000 01 c000000000000000000000000000000000000000000000000000000000000000 00000000 00 fdffffff 01 50c3010000000000 22 00206e00000000000000000000000000000000000000000000000000000000000000 0d000000", &mut test); + ext_from_hex("02000000 01 c000000000000000000000000000000000000000000000000000000000000000 00000000 00 fdffffff 01 52c3010000000000 22 00206e00000000000000000000000000000000000000000000000000000000000000 0d000000", &mut test); // Connect additional blocks to reach minimum_depth confirmations for _ in 0..5 { @@ -1933,8 +1929,8 @@ fn splice_seed() -> Vec { // inbound read from peer id 0 of len 82 (66 message + 16 MAC) ext_from_hex("030052", &mut test); // SpliceLocked message (type 77 = 0x004d): channel_id + splice_txid + mac - // splice_txid must match the splice funding txid (0x33 in reverse byte order) - ext_from_hex("004d c000000000000000000000000000000000000000000000000000000000000000 3300000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test); + // splice_txid must match the splice funding txid (0x31 in reverse byte order) + ext_from_hex("004d c000000000000000000000000000000000000000000000000000000000000000 3100000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test); test } @@ -2064,6 +2060,6 @@ mod tests { // Splice locked assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Handling SendSpliceLocked event in peer_handler for node 030000000000000000000000000000000000000000000000000000000000000002 for channel c000000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&1)); - assert_eq!(log_entries.get(&("lightning::ln::channel".to_string(), "Promoting splice funding txid 0000000000000000000000000000000000000000000000000000000000000033".to_string())), Some(&1)); + assert_eq!(log_entries.get(&("lightning::ln::channel".to_string(), "Promoting splice funding txid 0000000000000000000000000000000000000000000000000000000000000031".to_string())), Some(&1)); } } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 32c0e94bdc8..c9301ac67f5 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12324,7 +12324,25 @@ where ); let min_rbf_feerate = prev_feerate.map(min_rbf_feerate); let prior = if pending_splice.last_funding_feerate_sat_per_1000_weight.is_some() { - self.build_prior_contribution() + if let Some(prior) = self + .pending_splice + .as_ref() + .and_then(|pending_splice| pending_splice.contributions.last()) + { + let holder_balance = self + .get_holder_counterparty_balances_floor_incl_fee(&self.funding) + .map(|(h, _)| h) + .map_err(|e| APIError::ChannelUnavailable { + err: format!( + "Channel {} cannot be spliced at this time: {}", + self.context.channel_id(), + e + ), + })?; + Some(PriorContribution::new(prior.clone(), holder_balance)) + } else { + None + } } else { None }; @@ -12346,21 +12364,6 @@ where Ok(FundingTemplate::new(Some(shared_input), min_rbf_feerate, prior_contribution)) } - /// Clones the prior contribution and fetches the holder balance for deferred feerate - /// adjustment. - fn build_prior_contribution(&self) -> Option { - debug_assert!( - self.pending_splice.is_some(), - "build_prior_contribution requires pending_splice" - ); - let prior = self.pending_splice.as_ref()?.contributions.last()?; - let holder_balance = self - .get_holder_counterparty_balances_floor_incl_fee(&self.funding) - .map(|(h, _)| h) - .ok(); - Some(PriorContribution::new(prior.clone(), holder_balance)) - } - /// Returns whether this channel can ever RBF, independent of splice state. fn is_rbf_compatible(&self) -> Result<(), String> { if self.context.minimum_depth(&self.funding) == Some(0) { @@ -12532,14 +12535,12 @@ where }; } - if let Err(e) = contribution.validate().and_then(|()| { - // For splice-out, our_funding_contribution is adjusted to cover fees if there - // aren't any inputs. - let our_funding_contribution = contribution.net_value(); + let our_funding_contribution = contribution.net_value(); + + if let Err(e) = self.validate_splice_contributions(our_funding_contribution, SignedAmount::ZERO) - }) { + { log_error!(logger, "Channel {} cannot be funded: {}", self.context.channel_id(), e); - return Err(QuiescentError::FailSplice(self.splice_funding_failed_for(contribution))); } @@ -14101,13 +14102,11 @@ where // funding_contributed and quiescence, reducing the holder's // balance. If invalid, disconnect and return the contribution so // the user can reclaim their inputs. - if let Err(e) = contribution.validate().and_then(|()| { - let our_funding_contribution = contribution.net_value(); - self.validate_splice_contributions( - our_funding_contribution, - SignedAmount::ZERO, - ) - }) { + let our_funding_contribution = contribution.net_value(); + if let Err(e) = self.validate_splice_contributions( + our_funding_contribution, + SignedAmount::ZERO, + ) { let failed = self.splice_funding_failed_for(contribution); return Err(( ChannelError::WarnAndDisconnect(format!( diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 0a5fb647de1..904e1f30406 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -133,9 +133,13 @@ pub enum FundingContributionError { }, /// The splice value is invalid (zero, empty outputs, or exceeds the maximum money supply). InvalidSpliceValue, + /// An input's `prevtx` is too large to fit in a `tx_add_input` message. + PrevTxTooLarge, /// Coin selection failed to find suitable inputs. CoinSelectionFailed, - /// This is not an RBF scenario (no minimum RBF feerate available). + /// Coin selection is required but no coin selection source was provided. + MissingCoinSelectionSource, + /// This template cannot build an RBF contribution. NotRbfScenario, } @@ -151,11 +155,17 @@ impl core::fmt::Display for FundingContributionError { FundingContributionError::InvalidSpliceValue => { write!(f, "Invalid splice value (zero, empty, or exceeds limit)") }, + FundingContributionError::PrevTxTooLarge => { + write!(f, "Input prevtx is too large to fit in a tx_add_input message") + }, 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)") + write!(f, "This template cannot build an RBF contribution") }, } } @@ -164,18 +174,17 @@ impl core::fmt::Display for FundingContributionError { /// The user's prior contribution from a previous splice negotiation on this channel. /// /// When a pending splice exists with negotiated candidates, the prior contribution is -/// available for reuse (e.g., to bump the feerate via RBF). Contains the raw contribution and -/// the holder's balance for deferred feerate adjustment in [`FundingTemplate::rbf_sync`] or -/// [`FundingTemplate::rbf`]. +/// available for reuse. It stores the raw contribution together with the holder's balance for +/// deferred feerate adjustment when the contribution is later reused via +/// [`FundingTemplate::with_prior_contribution`] 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. +/// deciding whether to reuse it or replace it with +/// [`FundingTemplate::without_prior_contribution`]. #[derive(Debug, Clone, PartialEq, Eq)] pub(super) struct PriorContribution { contribution: FundingContribution, - /// The holder's balance, used for feerate adjustment. `None` when the balance computation - /// fails, in which case adjustment is skipped and coin selection is re-run. + /// The holder's balance, used for feerate adjustment. /// /// This value is captured at [`ChannelManager::splice_channel`] time and may become stale /// if balances change before the contribution is used. Staleness is acceptable here because @@ -186,11 +195,11 @@ pub(super) struct PriorContribution { /// /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel /// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed - holder_balance: Option, + holder_balance: Amount, } impl PriorContribution { - pub(super) fn new(contribution: FundingContribution, holder_balance: Option) -> Self { + pub(super) fn new(contribution: FundingContribution, holder_balance: Amount) -> Self { Self { contribution, holder_balance } } } @@ -203,27 +212,27 @@ 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_sync`] to remove funds from the channel -/// - [`FundingTemplate::splice_in_and_out_sync`] to do both +/// For a fresh splice (no pending splice to replace), either use the convenience methods +/// [`FundingTemplate::splice_in_sync`] and [`FundingTemplate::splice_out`] or start with +/// [`FundingTemplate::without_prior_contribution`] to compose a request manually. /// -/// These perform coin selection and require `min_feerate` and `max_feerate` parameters. +/// The builder API supports adding value, adding withdrawal outputs, or both. Attach a wallet +/// when the request may need new wallet inputs; pure splice-out requests can be built without one +/// and pay fees from the channel balance. /// /// # 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, use +/// [`FundingTemplate::rbf_prior_contribution_sync`] (or +/// [`FundingTemplate::rbf_prior_contribution`] for async) to retry the stored prior contribution +/// at an RBF-compatible feerate. To amend that prior request before building, start from +/// [`FundingTemplate::with_prior_contribution`] instead. /// /// Check [`FundingTemplate::min_rbf_feerate`] for the minimum feerate required (the greater of /// the previous feerate + 25 sat/kwu and the spec's 25/24 rule). 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. +/// [`FundingTemplate::prior_contribution`] to inspect the stored contribution before deciding +/// whether to reuse it or replace it with a fresh request via +/// [`FundingTemplate::without_prior_contribution`]. /// /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel /// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed @@ -253,8 +262,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 } @@ -262,406 +271,167 @@ impl FundingTemplate { /// Returns a reference to the prior contribution from a previous splice negotiation, if /// available. /// - /// Use this to inspect the prior contribution's parameters (e.g., - /// [`FundingContribution::value_added`], [`FundingContribution::outputs`]) before deciding - /// whether to reuse it via [`FundingTemplate::rbf_sync`] or build a fresh contribution - /// with different parameters using the splice methods. + /// Use this to inspect the prior contribution's current parameters (for example, + /// [`FundingContribution::outputs`], [`FundingContribution::change_output`], and + /// [`FundingContribution::net_value`]) before deciding + /// 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 - /// the acceptor. This can change other parameters too (e.g., - /// [`FundingContribution::value_added`] may be higher if the change output was removed to - /// cover a higher fee). + /// the acceptor. This can change other parameters too; for example, the amount added to the + /// channel may increase if the change output was removed to cover a higher fee. pub fn prior_contribution(&self) -> Option<&FundingContribution> { self.prior_contribution.as_ref().map(|p| &p.contribution) } -} - -macro_rules! build_funding_contribution { - ($value_added:expr, $outputs:expr, $shared_input:expr, $min_rbf_feerate:expr, $feerate:expr, $max_feerate:expr, $force_coin_selection:expr, $wallet:ident, $($await:tt)*) => {{ - let value_added: Amount = $value_added; - let outputs: Vec = $outputs; - let shared_input: Option = $shared_input; - let min_rbf_feerate: Option = $min_rbf_feerate; - let feerate: FeeRate = $feerate; - let max_feerate: FeeRate = $max_feerate; - let force_coin_selection: bool = $force_coin_selection; - - if feerate > max_feerate { - return Err(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 is_splice = shared_input.is_some(); - - 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(), - }; - - 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)? - } - }; - - // NOTE: Must NOT fail after UTXO selection - - 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 { - value_added, - estimated_fee, - inputs, - outputs, - change_output, - feerate, - max_feerate, - is_splice, - }; + /// 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) + } - Ok(contribution) - }}; -} + /// 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) + } -impl FundingTemplate { - /// Creates a [`FundingContribution`] for adding funds to a channel using `wallet` to perform - /// coin selection. + /// Creates a [`FundingContribution`] for adding funds to a channel. /// - /// `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`. + /// 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 + /// amount that prior request was already adding to the channel instead of replacing it. Use + /// [`FundingTemplate::without_prior_contribution`] if you want to replace the prior request + /// instead. + /// + /// `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 value_added == Amount::ZERO { - return Err(FundingContributionError::InvalidSpliceValue); - } - 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 - ) + self.with_prior_contribution(min_feerate, max_feerate) + .with_coin_selection_source(wallet) + .add_value(value_added) + .build() + .await } - /// Creates a [`FundingContribution`] for adding funds to a channel using `wallet` to perform - /// coin selection. + /// Creates a [`FundingContribution`] for adding funds to a channel. /// - /// See [`FundingTemplate::splice_in`] for details. + /// 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 { - return Err(FundingContributionError::InvalidSpliceValue); - } - 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, - ) - } - - /// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to - /// perform coin selection. - /// - /// `outputs` are the complete set of withdrawal outputs for this contribution. When - /// replacing a prior contribution via RBF, use [`FundingTemplate::prior_contribution`] to - /// inspect the prior parameters. To keep existing withdrawals and add new ones, include the - /// prior's outputs: combine [`FundingContribution::outputs`] with the new outputs. - pub async fn splice_out( - self, outputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, - ) -> Result { - if outputs.is_empty() { - return Err(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 - ) + self.with_prior_contribution(min_feerate, max_feerate) + .with_coin_selection_source_sync(wallet) + .add_value(value_added) + .build() } - /// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to - /// perform coin selection. + /// Creates a [`FundingContribution`] for removing funds from a channel. /// - /// See [`FundingTemplate::splice_out`] for details. - pub fn splice_out_sync( - self, outputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, - ) -> Result { - if outputs.is_empty() { - return Err(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, - ) - } - - /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using - /// `wallet` to perform coin selection. + /// 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. /// - /// `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, - ) -> 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 - ) - } - - /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using - /// `wallet` to perform coin selection. + /// `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 FundingTemplate { shared_input, min_rbf_feerate, .. } = self; - build_funding_contribution!( - value_added, - outputs, - shared_input, - min_rbf_feerate, - min_feerate, - max_feerate, - false, - wallet, - ) + self.with_prior_contribution(min_feerate, max_feerate).add_outputs(outputs).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. + /// This requires [`FundingTemplate::prior_contribution`] to be available. `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 + /// - If the prior contribution's feerate can be adjusted to the effective target feerate, the /// adjusted contribution is returned directly. For splice-in, the change output absorbs /// the fee difference. For splice-out (no wallet inputs), the holder's channel balance /// covers the higher fees. - /// - If adjustment fails, coin selection is re-run using the prior contribution's - /// parameters and the caller's `max_feerate`. For splice-out contributions, this changes + /// - If adjustment fails, coin selection is re-run using the prior contribution's current + /// request 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, + /// Returns a [`FundingContributionError`] if there is no reusable prior contribution, if no + /// effective RBF feerate is available, if the effective feerate violates the template's fee + /// constraints, or if coin selection fails. + 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, - }); - } - - 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 - ) - }, + if self.prior_contribution().is_none() { + return Err(FundingContributionError::NotRbfScenario); } + let feerate = feerate + .or_else(|| self.min_rbf_feerate()) + .ok_or(FundingContributionError::NotRbfScenario)?; + 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() } } @@ -715,15 +485,47 @@ fn estimate_transaction_fee( Weight::from_wu(weight) * feerate } +fn validate_inputs(inputs: &[FundingTxInput]) -> Result<(), FundingContributionError> { + let mut total_value = Amount::ZERO; + for input in inputs { + use crate::util::ser::Writeable; + const MESSAGE_TEMPLATE: msgs::TxAddInput = msgs::TxAddInput { + channel_id: ChannelId([0; 32]), + serial_id: 0, + prevtx: None, + prevtx_out: 0, + sequence: 0, + // Mutually exclusive with prevtx, which is accounted for below. + shared_input_txid: None, + }; + let message_len = MESSAGE_TEMPLATE.serialized_length() + input.prevtx.serialized_length(); + (message_len <= LN_MAX_MSG_LEN) + .then(|| ()) + .ok_or(FundingContributionError::PrevTxTooLarge)?; + + total_value = match total_value.checked_add(input.utxo.output.value) { + Some(sum) if sum <= Amount::MAX_MONEY => sum, + _ => return Err(FundingContributionError::InvalidSpliceValue), + }; + } + + Ok(()) +} + +/// Describes how an amended contribution should source its wallet-backed inputs. +enum FundingInputs { + None, + /// Reuses the contribution's existing inputs while targeting at least `value_added` added to + /// the channel after fees. If dropping the change output leaves surplus value, it remains in + /// the channel contribution. + CoinSelected { + value_added: Amount, + }, +} + /// The components of a funding transaction contributed by one party. #[derive(Debug, Clone, PartialEq, Eq)] pub struct FundingContribution { - /// The amount to contribute to the channel. - /// - /// If `value_added` is [`Amount::ZERO`], then any fees will be deducted from the channel - /// balance instead of paid by `inputs`. - value_added: Amount, - /// The estimate fees responsible to be paid for the contribution. estimated_fee: Amount, @@ -749,14 +551,13 @@ pub struct FundingContribution { } impl_writeable_tlv_based!(FundingContribution, { - (1, value_added, required), - (3, estimated_fee, required), - (5, inputs, optional_vec), - (7, outputs, optional_vec), - (9, change_output, option), - (11, feerate, required), - (13, max_feerate, required), - (15, is_splice, required), + (1, estimated_fee, required), + (3, inputs, optional_vec), + (5, outputs, optional_vec), + (7, change_output, option), + (9, feerate, required), + (11, max_feerate, required), + (13, is_splice, required), }); impl FundingContribution { @@ -776,9 +577,18 @@ impl FundingContribution { self.outputs.iter().chain(self.change_output.iter()) } - /// Returns the amount added to the channel by this contribution. + /// The value that will be added to the channel after fees. See [`Self::net_value`] for the net + /// value contribution to the channel. pub fn value_added(&self) -> Amount { - self.value_added + let total_input_value = self.inputs.iter().map(|i| i.utxo.output.value).sum::(); + total_input_value + .checked_sub(self.estimated_fee) + .and_then(|v| { + v.checked_sub( + self.change_output.as_ref().map_or(Amount::ZERO, |output| output.value), + ) + }) + .unwrap_or(Amount::ZERO) } /// Returns the outputs (e.g., withdrawal destinations) included in this contribution. @@ -796,6 +606,104 @@ impl FundingContribution { self.change_output.as_ref() } + fn with_inputs_and_outputs( + self, funding_inputs: FundingInputs, outputs: &[TxOut], + ) -> Option { + let (target_value_added, inputs) = match funding_inputs { + FundingInputs::None => (None, Vec::new()), + FundingInputs::CoinSelected { value_added } => (Some(value_added), self.inputs), + }; + + if inputs.is_empty() && target_value_added.unwrap_or(Amount::ZERO) != Amount::ZERO { + // Prior contribution didn't have any inputs, but now we need some. + return None; + } + + let estimated_fee_no_change = + estimate_transaction_fee(&inputs, &outputs, None, true, self.is_splice, self.feerate); + + // When inputs are coin-selected, adjust the existing change output, if any, to account for + // the new value added. + if let Some(value_added) = target_value_added { + let estimated_fee = estimate_transaction_fee( + &inputs, + &outputs, + self.change_output.as_ref(), + true, + self.is_splice, + self.feerate, + ); + let required_value = value_added.checked_add(estimated_fee)?; + + let total_input_value: Amount = + inputs.iter().map(|input| input.utxo.output.value).sum(); + + if self.change_output.is_none() && total_input_value < required_value { + // The prior input selection is not enough, we may need to re-attempt coin + // selection. + return None; + } + + if let Some(change_output) = self.change_output.as_ref() { + let dust_limit = change_output.script_pubkey.minimal_non_dust(); + match total_input_value.checked_sub(required_value) { + Some(new_change_value) if new_change_value >= dust_limit => { + let new_change_output = TxOut { + value: new_change_value, + script_pubkey: change_output.script_pubkey.clone(), + }; + return Some(FundingContribution { + estimated_fee, + inputs, + outputs: outputs.to_vec(), + change_output: Some(new_change_output), + ..self + }); + }, + _ => { + let required_value = value_added.checked_add(estimated_fee_no_change)?; + if total_input_value < required_value { + return None; + } + }, + } + } + } + + Some(FundingContribution { + estimated_fee: estimated_fee_no_change, + outputs: outputs.to_vec(), + inputs, + change_output: None, + ..self + }) + } + + /// Tries to satisfy a new request using only this contribution's existing inputs. + /// + /// For input-backed contributions, this reuses the current inputs, adjusts the explicit + /// outputs, and shrinks or drops the change output as needed before applying + /// `target_feerate`. If dropping change leaves surplus value, that surplus remains in the + /// channel contribution. + /// + /// For input-less contributions, `holder_balance` must be provided to cover the outputs and + /// fees from the channel balance. + /// + /// Returns `None` if the request would require new wallet inputs or cannot accommodate the + /// requested feerate. + fn amend_without_coin_selection( + self, funding_inputs: FundingInputs, outputs: &[TxOut], target_feerate: FeeRate, + max_feerate: FeeRate, holder_balance: Amount, + ) -> Option { + let new_contribution_at_current_feerate = + self.with_inputs_and_outputs(funding_inputs, outputs)?; + let mut new_contribution_at_target_feerate = new_contribution_at_current_feerate + .at_feerate(target_feerate, holder_balance, true) + .ok()?; + new_contribution_at_target_feerate.max_feerate = max_feerate; + Some(new_contribution_at_target_feerate) + } + pub(super) fn into_tx_parts(self) -> (Vec, Vec) { let FundingContribution { inputs, mut outputs, change_output, .. } = self; @@ -830,64 +738,6 @@ impl FundingContribution { } } - /// Validates that the funding inputs are suitable for use in the interactive transaction - /// protocol, checking prevtx sizes and input sufficiency. - pub fn validate(&self) -> Result<(), String> { - for FundingTxInput { utxo, prevtx, .. } in self.inputs.iter() { - use crate::util::ser::Writeable; - const MESSAGE_TEMPLATE: msgs::TxAddInput = msgs::TxAddInput { - channel_id: ChannelId([0; 32]), - serial_id: 0, - prevtx: None, - prevtx_out: 0, - sequence: 0, - // Mutually exclusive with prevtx, which is accounted for below. - shared_input_txid: None, - }; - let message_len = MESSAGE_TEMPLATE.serialized_length() + prevtx.serialized_length(); - if message_len > LN_MAX_MSG_LEN { - return Err(format!( - "Funding input references a prevtx that is too large for tx_add_input: {}", - utxo.outpoint - )); - } - } - - // Fees for splice-out are paid from the channel balance whereas fees for splice-in - // are paid by the funding inputs. Therefore, in the case of splice-out, we add the - // fees on top of the user-specified contribution. We leave the user-specified - // contribution as-is for splice-ins. - if !self.inputs.is_empty() { - let mut total_input_value = Amount::ZERO; - for FundingTxInput { utxo, .. } in self.inputs.iter() { - total_input_value = total_input_value - .checked_add(utxo.output.value) - .ok_or("Sum of input values is greater than the total bitcoin supply")?; - } - - // If the inputs are enough to cover intended contribution amount plus fees (which - // include the change output weight when present), we are fine. - // If the inputs are less, but enough to cover intended contribution amount with - // (lower) fees without change, we are also fine (change will not be generated). - // Since estimated_fee includes change weight, this check is conservative. - // - // Note: dust limit is not relevant in this check. - - let contributed_input_value = self.value_added; - let estimated_fee = self.estimated_fee; - let minimal_input_amount_needed = contributed_input_value - .checked_add(estimated_fee) - .ok_or(format!("{contributed_input_value} contribution plus {estimated_fee} fee estimate exceeds the total bitcoin supply"))?; - if total_input_value < minimal_input_amount_needed { - return Err(format!( - "Total input amount {total_input_value} is lower than needed for splice-in contribution {contributed_input_value}, considering fees of {estimated_fee}. Need more inputs.", - )); - } - } - - Ok(()) - } - /// Computes the adjusted fee and change output value at the given target feerate, which may /// differ from the feerate used during coin selection. /// @@ -933,54 +783,36 @@ impl FundingContribution { } } - if !self.inputs.is_empty() { - if let Some(ref change_output) = self.change_output { - let old_change_value = change_output.value; - let dust_limit = change_output.script_pubkey.minimal_non_dust(); + let target_fee = estimate_transaction_fee( + &self.inputs, + &self.outputs, + self.change_output.as_ref(), + is_initiator, + self.is_splice, + target_feerate, + ); - // Target fee including the change output's weight. - let target_fee = estimate_transaction_fee( - &self.inputs, - &self.outputs, - self.change_output.as_ref(), - is_initiator, - self.is_splice, - target_feerate, - ); + let net_value_without_fee = self.net_value_without_fee(); + if net_value_without_fee.is_positive() { + let fee_buffer = self + .estimated_fee + .checked_add( + self.change_output.as_ref().map_or(Amount::ZERO, |output| output.value), + ) + .ok_or(FeeRateAdjustmentError::FeeBufferOverflow)?; - let fee_buffer = self - .estimated_fee - .checked_add(old_change_value) - .ok_or(FeeRateAdjustmentError::FeeBufferOverflow)?; + if let Some(change_output) = self.change_output.as_ref() { + let dust_limit = change_output.script_pubkey.minimal_non_dust(); + if let Some(new_change_value) = fee_buffer.checked_sub(target_fee) { + if new_change_value >= dust_limit { + return Ok((target_fee, Some(new_change_value))); + } - match fee_buffer.checked_sub(target_fee) { - Some(new_change_value) if new_change_value >= dust_limit => { - Ok((target_fee, Some(new_change_value))) - }, - _ => { - // Change would be below dust or negative. Try without change. - let target_fee_no_change = estimate_transaction_fee( - &self.inputs, - &self.outputs, - None, - is_initiator, - self.is_splice, - target_feerate, - ); - if target_fee_no_change > fee_buffer { - Err(FeeRateAdjustmentError::FeeBufferInsufficient { - source: "estimated fee + change value", - available: fee_buffer, - required: target_fee_no_change, - }) - } else { - Ok((target_fee_no_change, None)) - } - }, + // Our remaining change was not enough to be a valid output, fallthrough to the + // no remaining change case. } - } else { - // No change output. - let target_fee = estimate_transaction_fee( + + let target_fee_no_change = estimate_transaction_fee( &self.inputs, &self.outputs, None, @@ -988,50 +820,39 @@ impl FundingContribution { self.is_splice, target_feerate, ); - // The fee buffer is total input value minus value_added and output values. - // This is estimated_fee plus the coin selection surplus (dust burned to - // fees), ensuring we never silently reduce value_added beyond the small - // surplus from coin selection. - let total_input_value: Amount = - self.inputs.iter().map(|i| i.utxo.output.value).sum(); - let output_values: Amount = self.outputs.iter().map(|o| o.value).sum(); - let fee_buffer = total_input_value - .checked_sub(self.value_added) - .and_then(|v| v.checked_sub(output_values)) - .ok_or(FeeRateAdjustmentError::FeeBufferOverflow)?; - if target_fee > fee_buffer { - return Err(FeeRateAdjustmentError::FeeBufferInsufficient { - source: "estimated fee + coin selection surplus", + if target_fee_no_change > fee_buffer { + Err(FeeRateAdjustmentError::FeeBufferInsufficient { + source: "estimated fee + change value", available: fee_buffer, - required: target_fee, - }); + required: target_fee_no_change, + }) + } else { + Ok((target_fee_no_change, None)) } + } else if let Some(_surplus) = fee_buffer.checked_sub(target_fee) { Ok((target_fee, None)) + } else { + Err(FeeRateAdjustmentError::FeeBufferInsufficient { + source: "estimated fee", + available: fee_buffer, + required: target_fee, + }) } } else { - // No inputs (splice-out): fees paid from channel balance. - let target_fee = estimate_transaction_fee( - &[], - &self.outputs, - None, - is_initiator, - self.is_splice, - target_feerate, - ); - - // Check that the channel balance can cover the withdrawal outputs plus fees. - let value_removed: Amount = self.outputs.iter().map(|o| o.value).sum(); + // Check that the channel balance can cover the negative contribution. If there is a + // surplus, it implicitly goes back to the channel balance. let total_cost = target_fee - .checked_add(value_removed) + .checked_add(net_value_without_fee.unsigned_abs()) .ok_or(FeeRateAdjustmentError::FeeBufferOverflow)?; if total_cost > holder_balance { return Err(FeeRateAdjustmentError::FeeBufferInsufficient { - source: "channel balance - withdrawal outputs", - available: holder_balance.checked_sub(value_removed).unwrap_or(Amount::ZERO), + source: "channel balance", + available: holder_balance + .checked_sub(net_value_without_fee.unsigned_abs()) + .unwrap_or(Amount::ZERO), required: target_fee, }); } - // Surplus goes back to the channel balance. Ok((target_fee, None)) } } @@ -1044,12 +865,10 @@ impl FundingContribution { ) -> Result { let (new_estimated_fee, new_change) = self.compute_feerate_adjustment(feerate, holder_balance, is_initiator)?; - let surplus = self.fee_buffer_surplus(new_estimated_fee, &new_change); match new_change { Some(value) => self.change_output.as_mut().unwrap().value = value, None => self.change_output = None, } - self.value_added += surplus; self.estimated_fee = new_estimated_fee; self.feerate = feerate; Ok(self) @@ -1088,15 +907,28 @@ impl FundingContribution { ) -> Result { let (new_estimated_fee, new_change) = self.compute_feerate_adjustment(target_feerate, holder_balance, is_initiator)?; - let surplus = self - .fee_buffer_surplus(new_estimated_fee, &new_change) + + let prev_fee = self + .estimated_fee + .to_signed() + .expect("total input amount cannot exceed Amount::MAX_MONEY"); + let prev_change = self + .change_output + .as_ref() + .map_or(Amount::ZERO, |output| output.value) .to_signed() - .expect("surplus does not exceed Amount::MAX_MONEY"); - let net_value = self - .net_value_with_fee(new_estimated_fee) - .checked_add(surplus) - .expect("net_value + surplus does not overflow"); - Ok(net_value) + .expect("total input amount cannot exceed Amount::MAX_MONEY"); + + let new_fee = new_estimated_fee + .to_signed() + .expect("total input amount cannot exceed Amount::MAX_MONEY"); + let new_change = new_change + .unwrap_or(Amount::ZERO) + .to_signed() + .expect("total input amount cannot exceed Amount::MAX_MONEY"); + + let prev_net_value = self.net_value(); + Ok(prev_net_value + prev_fee + prev_change - new_fee - new_change) } /// Returns the net value at the given target feerate without mutating `self`, @@ -1115,55 +947,35 @@ impl FundingContribution { self.net_value_at_feerate(target_feerate, holder_balance, true) } - /// Returns the fee buffer surplus when a change output is removed. - /// - /// The fee buffer is the actual amount available for fees from inputs: total input value - /// minus value_added and output values. This includes both the weight-based estimated_fee - /// and any coin selection surplus (dust burned to fees). When the change output is removed, - /// the fee buffer may exceed the new fee; the surplus is returned so it can be redirected - /// to value_added rather than being burned as excess fees. - /// - /// Returns [`Amount::ZERO`] when there are no inputs or the change output is kept. - fn fee_buffer_surplus(&self, new_estimated_fee: Amount, new_change: &Option) -> Amount { - if !self.inputs.is_empty() && new_change.is_none() { - let total_input_value: Amount = self.inputs.iter().map(|i| i.utxo.output.value).sum(); - let output_values: Amount = self.outputs.iter().map(|o| o.value).sum(); - let fee_buffer = total_input_value - self.value_added - output_values; - debug_assert!(fee_buffer >= new_estimated_fee); - fee_buffer - new_estimated_fee - } else { - Amount::ZERO - } - } - - /// 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. + /// The net value contributed to a channel by the splice. pub fn net_value(&self) -> SignedAmount { - self.net_value_with_fee(self.estimated_fee) + let estimated_fee = self + .estimated_fee + .to_signed() + .expect("total_input_value is validated to not exceed Amount::MAX_MONEY"); + self.net_value_without_fee() + .checked_sub(estimated_fee) + .expect("all amounts are validated to not exceed Amount::MAX_MONEY") } - /// Computes the net value using the given `estimated_fee` for the splice-out (no inputs) - /// case. For splice-in, fees are paid by inputs so `estimated_fee` is not deducted. - fn net_value_with_fee(&self, estimated_fee: Amount) -> SignedAmount { - let unpaid_fees = if self.inputs.is_empty() { estimated_fee } else { Amount::ZERO } - .to_signed() - .expect("estimated_fee is validated to not exceed Amount::MAX_MONEY"); - let value_added = self - .value_added + fn net_value_without_fee(&self) -> SignedAmount { + let total_input_value = self + .inputs + .iter() + .map(|input| input.utxo.output.value) + .sum::() .to_signed() - .expect("value_added is validated to not exceed Amount::MAX_MONEY"); - let value_removed = self + .expect("total_input_value is validated to not exceed Amount::MAX_MONEY"); + let total_output_value = self .outputs .iter() + .chain(self.change_output.iter()) .map(|txout| txout.value) .sum::() .to_signed() - .expect("value_removed is validated to not exceed Amount::MAX_MONEY"); - - let contribution_amount = value_added - value_removed; - contribution_amount - .checked_sub(unpaid_fees) + .expect("total_output_value is validated to not exceed Amount::MAX_MONEY"); + total_input_value + .checked_sub(total_output_value) .expect("all amounts are validated to not exceed Amount::MAX_MONEY") } } @@ -1172,29 +984,559 @@ impl FundingContribution { /// establishment protocol or when splicing. pub type FundingTxInput = crate::util::wallet_utils::ConfirmedUtxo; -#[cfg(test)] -mod tests { - use super::{ - estimate_transaction_fee, FeeRateAdjustmentError, FundingContribution, - FundingContributionError, FundingTemplate, FundingTxInput, PriorContribution, - }; - use crate::chain::ClaimId; - use crate::util::wallet_utils::{CoinSelection, CoinSelectionSourceSync, Input}; - use bitcoin::hashes::Hash; - use bitcoin::transaction::{Transaction, TxOut, Version}; - use bitcoin::{Amount, FeeRate, Psbt, ScriptBuf, SignedAmount, WPubkeyHash}; - - #[test] - #[rustfmt::skip] - fn test_estimate_transaction_fee() { - let one_input = [funding_input_sats(1_000)]; - let two_inputs = [funding_input_sats(1_000), funding_input_sats(1_000)]; - - // 2 inputs, initiator, 2000 sat/kw feerate - assert_eq!( - estimate_transaction_fee(&two_inputs, &[], None, true, false, FeeRate::from_sat_per_kwu(2000)), - Amount::from_sat(if cfg!(feature = "grind_signatures") { 1512 } else { 1516 }), - ); +#[derive(Debug, Clone, PartialEq, Eq)] +struct NoCoinSelectionSource; +#[derive(Debug, Clone, PartialEq, Eq)] +struct AsyncCoinSelectionSource(W); +#[derive(Debug, Clone, PartialEq, Eq)] +struct SyncCoinSelectionSource(W); + +#[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, +} + +/// A builder for composing or amending a [`FundingContribution`]. +/// +/// The builder tracks a requested amount to add to the channel together with any explicit +/// withdrawal outputs. Building without an attached wallet only succeeds when the request can be +/// satisfied by reusing or amending a prior contribution, or by constructing a pure splice-out +/// that pays fees from the channel balance. +/// +/// Attach a wallet via [`FundingBuilder::with_coin_selection_source`] or +/// [`FundingBuilder::with_coin_selection_source_sync`] when the request may need new wallet +/// inputs. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FundingBuilder(FundingBuilderInner); + +/// A [`FundingBuilder`] with an attached asynchronous [`CoinSelectionSource`]. +/// +/// Created by [`FundingBuilder::with_coin_selection_source`]. The attached wallet is only used +/// if the request cannot be satisfied by reusing a prior contribution or by building a pure +/// splice-out directly. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AsyncFundingBuilder(FundingBuilderInner>); + +/// A [`FundingBuilder`] with an attached synchronous [`CoinSelectionSourceSync`]. +/// +/// Created by [`FundingBuilder::with_coin_selection_source_sync`]. The attached wallet is only +/// used if the request cannot be satisfied by reusing a prior contribution or by building a pure +/// splice-out directly. +#[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 + } + + fn build_from_prior_contribution( + &mut self, contribution: PriorContribution, + ) -> Result { + let PriorContribution { contribution, holder_balance } = contribution; + + if self.request_matches_prior(&contribution) { + // Same request, but the feerate may have changed. Adjust the prior contribution + // to the new feerate if possible. + return contribution + .for_initiator_at_feerate(self.feerate, holder_balance) + .map(|mut adjusted| { + adjusted.max_feerate = self.max_feerate; + adjusted + }) + .map_err(|_| FundingContributionError::MissingCoinSelectionSource); + } + + let funding_inputs = if self.value_added != Amount::ZERO { + FundingInputs::CoinSelected { value_added: self.value_added } + } else { + FundingInputs::None + }; + return contribution + .amend_without_coin_selection( + funding_inputs, + &self.outputs, + self.feerate, + self.max_feerate, + holder_balance, + ) + .ok_or_else(|| FundingContributionError::MissingCoinSelectionSource); + } + + /// 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 by building a contribution that pays + /// fees from the channel balance. + /// + /// Returns [`FundingContributionError::MissingCoinSelectionSource`] if the request is + /// otherwise valid but needs wallet inputs. + fn try_build_without_coin_selection( + &mut self, + ) -> Result { + if let Some(contribution) = self.prior_contribution.take() { + return self.build_from_prior_contribution(contribution); + } + + if self.value_added == Amount::ZERO { + let estimated_fee = estimate_transaction_fee( + &[], + &self.outputs, + None, + true, + self.shared_input.is_some(), + self.feerate, + ); + return Ok(FundingContribution { + estimated_fee, + inputs: vec![], + outputs: core::mem::take(&mut self.outputs), + change_output: None, + feerate: self.feerate, + max_feerate: self.max_feerate, + is_splice: self.shared_input.is_some(), + }); + } + + Err(FundingContributionError::MissingCoinSelectionSource) + } + + 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() + }; + + 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), + }; + } + + 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) => { + let outputs = prior.contribution.outputs.clone(); + (prior.contribution.value_added(), outputs) + }, + None => (Amount::ZERO, Vec::new()), + }; + + FundingBuilder(FundingBuilderInner { + shared_input, + min_rbf_feerate, + prior_contribution, + value_added, + outputs, + feerate, + max_feerate, + state: NoCoinSelectionSource, + }) + } + + /// Attaches an asynchronous [`CoinSelectionSource`] for later use. + /// + /// The wallet is only consulted if [`AsyncFundingBuilder::build`] cannot satisfy the request by + /// reusing a prior contribution or by constructing a pure splice-out directly. + pub fn with_coin_selection_source( + self, wallet: W, + ) -> AsyncFundingBuilder { + AsyncFundingBuilder(self.0.with_state(AsyncCoinSelectionSource(wallet))) + } + + /// Attaches a synchronous [`CoinSelectionSourceSync`] for later use. + /// + /// The wallet is only consulted if [`SyncFundingBuilder::build`] cannot satisfy the request by + /// reusing a prior contribution or by constructing a pure splice-out directly. + 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 any change output derived when the contribution is built. + pub fn add_output(self, output: TxOut) -> Self { + FundingBuilder(self.0.add_output_inner(output)) + } + + /// Adds withdrawal outputs to the request. + /// + /// `outputs` are appended to the current set of explicit outputs. If the builder was seeded + /// from a prior contribution, this adds additional withdrawals on top of the prior outputs. + /// This does not affect any change output derived when the contribution is built. + pub fn add_outputs(self, outputs: Vec) -> Self { + FundingBuilder(self.0.add_outputs_inner(outputs)) + } + + /// Removes all explicit withdrawal outputs whose script pubkey matches `script_pubkey`. + /// + /// This only affects 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 or 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(mut self) -> Result { + self.0.build_without_coin_selection() + } +} + +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, + } + } + + 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 add_outputs_inner(mut self, outputs: Vec) -> Self { + self.outputs.extend(outputs); + 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 then tries to build it without selecting new wallet + /// inputs. + /// + /// Returns [`FundingContributionError::MissingCoinSelectionSource`] if the request is valid but + /// cannot be satisfied without wallet inputs. + fn build_without_coin_selection( + &mut self, + ) -> Result { + self.validate_contribution_parameters()?; + self.try_build_without_coin_selection() + } +} + +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 any change output derived when the contribution is built. + pub fn add_output(self, output: TxOut) -> Self { + AsyncFundingBuilder(self.0.add_output_inner(output)) + } + + /// Adds withdrawal outputs to the request. + /// + /// `outputs` are appended to the current set of explicit outputs. If the builder was seeded + /// from a prior contribution, this adds additional withdrawals on top of the prior outputs. + /// This does not affect any change output derived when the contribution is built. + pub fn add_outputs(self, outputs: Vec) -> Self { + AsyncFundingBuilder(self.0.add_outputs_inner(outputs)) + } + + /// Removes all explicit withdrawal outputs whose script pubkey matches `script_pubkey`. + /// + /// This only affects 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)) + } + + /// Increases the requested amount to add 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 contribution's current amount added to the + /// channel. If the updated request cannot be satisfied in-place, [`AsyncFundingBuilder::build`] + /// may re-run coin selection and return a contribution with a different input set. + pub fn add_value(self, value: Amount) -> Self { + AsyncFundingBuilder(self.0.add_value_inner(value)) + } + + /// Decreases the requested amount to add 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 contribution's current + /// amount added to the channel. If the updated request cannot be satisfied in-place, + /// [`AsyncFundingBuilder::build`] may re-run coin selection and return a contribution with a + /// different input set. + pub fn remove_value(self, value: Amount) -> Self { + AsyncFundingBuilder(self.0.remove_value_inner(value)) + } +} + +impl AsyncFundingBuilder { + /// Builds a [`FundingContribution`], using the attached asynchronous wallet only when needed. + /// + /// If the request can be satisfied by reusing or amending a prior contribution, or by building + /// a pure splice-out directly, 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 (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)?; + + let CoinSelection { confirmed_utxos: inputs, change_output } = coin_selection; + validate_inputs(&inputs)?; + + let outputs = inner.outputs; + let is_splice = inner.shared_input.is_some(); + let estimated_fee = estimate_transaction_fee( + &inputs, + &outputs, + change_output.as_ref(), + true, + is_splice, + inner.feerate, + ); + + return Ok(FundingContribution { + estimated_fee, + inputs, + outputs, + change_output, + feerate: inner.feerate, + max_feerate: inner.max_feerate, + is_splice, + }); + } +} + +impl SyncFundingBuilder { + /// 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 any change output derived when the contribution is built. + pub fn add_output(self, output: TxOut) -> Self { + SyncFundingBuilder(self.0.add_output_inner(output)) + } + + /// Adds withdrawal outputs to the request. + /// + /// `outputs` are appended to the current set of explicit outputs. If the builder was seeded + /// from a prior contribution, this adds additional withdrawals on top of the prior outputs. + /// This does not affect any change output derived when the contribution is built. + pub fn add_outputs(self, outputs: Vec) -> Self { + SyncFundingBuilder(self.0.add_outputs_inner(outputs)) + } + + /// Removes all explicit withdrawal outputs whose script pubkey matches `script_pubkey`. + /// + /// This only affects 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 to add 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 contribution's current amount added to the + /// channel. If the updated request cannot be satisfied in-place, [`SyncFundingBuilder::build`] + /// may re-run coin selection and return a contribution with a different input set. + pub fn add_value(self, value: Amount) -> Self { + SyncFundingBuilder(self.0.add_value_inner(value)) + } + + /// Decreases the requested amount to add 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 contribution's current + /// amount added to the channel. If the updated request cannot be satisfied in-place, + /// [`SyncFundingBuilder::build`] may re-run coin selection and return a contribution with a + /// different input set. + pub fn remove_value(self, value: Amount) -> Self { + SyncFundingBuilder(self.0.remove_value_inner(value)) + } +} + +impl SyncFundingBuilder { + /// Builds a [`FundingContribution`], using the attached synchronous wallet only when needed. + /// + /// If the request can be satisfied by reusing or amending a prior contribution, or by building + /// a pure splice-out directly, 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 (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)?; + + let CoinSelection { confirmed_utxos: inputs, change_output } = coin_selection; + validate_inputs(&inputs)?; + + let outputs = inner.outputs; + let is_splice = inner.shared_input.is_some(); + let estimated_fee = estimate_transaction_fee( + &inputs, + &outputs, + change_output.as_ref(), + true, + is_splice, + inner.feerate, + ); + + return Ok(FundingContribution { + estimated_fee, + inputs, + outputs, + change_output, + feerate: inner.feerate, + max_feerate: inner.max_feerate, + is_splice, + }); + } +} + +#[cfg(test)] +mod tests { + use super::{ + estimate_transaction_fee, FeeRateAdjustmentError, FundingBuilder, FundingContribution, + FundingContributionError, FundingTemplate, FundingTxInput, PriorContribution, + }; + use crate::chain::ClaimId; + use crate::util::wallet_utils::{CoinSelection, CoinSelectionSourceSync, Input}; + use bitcoin::hashes::Hash; + use bitcoin::transaction::{Transaction, TxOut, Version}; + use bitcoin::{Amount, FeeRate, Psbt, ScriptBuf, SignedAmount, WPubkeyHash, WScriptHash}; + + #[test] + #[rustfmt::skip] + fn test_estimate_transaction_fee() { + let one_input = [funding_input_sats(1_000)]; + let two_inputs = [funding_input_sats(1_000), funding_input_sats(1_000)]; + + // 2 inputs, initiator, 2000 sat/kw feerate + assert_eq!( + estimate_transaction_fee(&two_inputs, &[], None, true, false, FeeRate::from_sat_per_kwu(2000)), + Amount::from_sat(if cfg!(feature = "grind_signatures") { 1512 } else { 1516 }), + ); // higher feerate assert_eq!( @@ -1280,202 +1622,166 @@ mod tests { } } - #[test] - #[rustfmt::skip] - fn test_check_v2_funding_inputs_sufficient() { - // positive case, inputs well over intended contribution - { - let expected_fee = if cfg!(feature = "grind_signatures") { 2278 } else { 2284 }; - let contribution = FundingContribution { - value_added: Amount::from_sat(220_000), - estimated_fee: Amount::from_sat(expected_fee), - inputs: vec![ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - outputs: vec![], - change_output: None, - is_splice: true, - feerate: FeeRate::from_sat_per_kwu(2000), - max_feerate: FeeRate::MAX, - }; - assert!(contribution.validate().is_ok()); - assert_eq!(contribution.net_value(), contribution.value_added.to_signed().unwrap()); - } + struct UnreachableWallet; - // Net splice-in - { - let expected_fee = if cfg!(feature = "grind_signatures") { 2526 } else { 2532 }; - let contribution = FundingContribution { - value_added: Amount::from_sat(220_000), - estimated_fee: Amount::from_sat(expected_fee), - inputs: vec![ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - outputs: vec![ - funding_output_sats(200_000), - ], - change_output: None, - is_splice: true, - feerate: FeeRate::from_sat_per_kwu(2000), - max_feerate: FeeRate::MAX, - }; - assert!(contribution.validate().is_ok()); - assert_eq!(contribution.net_value(), SignedAmount::from_sat(220_000 - 200_000)); + impl CoinSelectionSourceSync for UnreachableWallet { + 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 { + unreachable!("should not reach coin selection") } - - // Net splice-out - { - let expected_fee = if cfg!(feature = "grind_signatures") { 2526 } else { 2532 }; - let contribution = FundingContribution { - value_added: Amount::from_sat(220_000), - estimated_fee: Amount::from_sat(expected_fee), - inputs: vec![ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - outputs: vec![ - funding_output_sats(400_000), - ], - change_output: None, - is_splice: true, - feerate: FeeRate::from_sat_per_kwu(2000), - max_feerate: FeeRate::MAX, - }; - assert!(contribution.validate().is_ok()); - assert_eq!(contribution.net_value(), SignedAmount::from_sat(220_000 - 400_000)); + fn sign_psbt(&self, _psbt: Psbt) -> Result { + unreachable!("should not reach signing") } + } - // Net splice-out, inputs insufficient to cover fees - { - let expected_fee = if cfg!(feature = "grind_signatures") { 113670 } else { 113940 }; - let contribution = FundingContribution { - value_added: Amount::from_sat(220_000), - estimated_fee: Amount::from_sat(expected_fee), - inputs: vec![ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - outputs: vec![ - funding_output_sats(400_000), - ], - change_output: None, - is_splice: true, - feerate: FeeRate::from_sat_per_kwu(90000), - max_feerate: FeeRate::MAX, - }; - assert_eq!( - contribution.validate(), - Err(format!( - "Total input amount 0.00300000 BTC is lower than needed for splice-in contribution 0.00220000 BTC, considering fees of {}. Need more inputs.", - Amount::from_sat(expected_fee), - )), - ); - } + #[test] + fn test_funding_builder_builds_splice_out_without_wallet() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let output = funding_output_sats(25_000); - // negative case, inputs clearly insufficient - { - let expected_fee = if cfg!(feature = "grind_signatures") { 1736 } else { 1740 }; - let contribution = FundingContribution { - value_added: Amount::from_sat(220_000), - estimated_fee: Amount::from_sat(expected_fee), - inputs: vec![ - funding_input_sats(100_000), - ], - outputs: vec![], - change_output: None, - is_splice: true, - feerate: FeeRate::from_sat_per_kwu(2000), - max_feerate: FeeRate::MAX, - }; - assert_eq!( - contribution.validate(), - Err(format!( - "Total input amount 0.00100000 BTC is lower than needed for splice-in contribution 0.00220000 BTC, considering fees of {}. Need more inputs.", - Amount::from_sat(expected_fee), - )), - ); - } + let contribution = + FundingBuilder::new(FundingTemplate::new(None, None, None), feerate, FeeRate::MAX) + .add_output(output.clone()) + .build() + .unwrap(); + + let expected_fee = estimate_transaction_fee( + &[], + std::slice::from_ref(&output), + None, + true, + false, + feerate, + ); + assert!(contribution.inputs.is_empty()); + assert_eq!(contribution.outputs, vec![output.clone()]); + assert!(contribution.change_output.is_none()); + assert_eq!(contribution.estimated_fee, expected_fee); + assert_eq!( + contribution.net_value(), + -output.value.to_signed().unwrap() - expected_fee.to_signed().unwrap(), + ); + } + + #[test] + fn test_funding_builder_requires_wallet_for_splice_in() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let builder = + FundingBuilder::new(FundingTemplate::new(None, None, None), feerate, FeeRate::MAX); + let builder = FundingBuilder(builder.0.add_value_inner(Amount::from_sat(25_000))); + + assert!(matches!( + builder.build(), + Err(FundingContributionError::MissingCoinSelectionSource), + )); + } + + #[test] + fn test_funding_builder_amends_prior_by_dropping_subdust_change() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let inputs = vec![funding_input_sats(100_000)]; + let change = funding_output_sats(500); + let dust_limit = change.script_pubkey.minimal_non_dust(); + assert!(change.value >= dust_limit); + + let estimated_fee_with_change = + estimate_transaction_fee(&inputs, &[], Some(&change), true, true, feerate); + let estimated_fee_no_change = + estimate_transaction_fee(&inputs, &[], None, true, true, feerate); + let prior = FundingContribution { + estimated_fee: estimated_fee_with_change, + inputs: inputs.clone(), + outputs: vec![], + change_output: Some(change.clone()), + feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let delta = Amount::from_sat(change.value.to_sat() - dust_limit.to_sat() + 1); + let target_value_added = prior.value_added().checked_add(delta).unwrap(); + let total_input_value: Amount = inputs.iter().map(|input| input.utxo.output.value).sum(); + let remaining_change = total_input_value + .checked_sub(target_value_added.checked_add(estimated_fee_with_change).unwrap()) + .unwrap(); + assert_eq!(remaining_change.to_sat(), dust_limit.to_sat() - 1); + assert!( + total_input_value >= target_value_added.checked_add(estimated_fee_no_change).unwrap() + ); + + let builder = + FundingTemplate::new(None, None, Some(PriorContribution::new(prior, Amount::MAX))) + .with_prior_contribution(feerate, FeeRate::MAX); + let contribution = FundingBuilder(builder.0.add_value_inner(delta)).build().unwrap(); + + assert!(contribution.change_output.is_none()); + assert_eq!(contribution.inputs, inputs); + assert!(contribution.outputs.is_empty()); + assert_eq!(contribution.estimated_fee, estimated_fee_no_change); + assert_eq!( + contribution.value_added(), + total_input_value.checked_sub(estimated_fee_no_change).unwrap() + ); + assert!(contribution.value_added() > target_value_added); + } + + #[test] + fn test_funding_builder_remove_outputs_removes_all_matching_scripts() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let removed_script = ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()); + let kept_script = ScriptBuf::new_p2wsh(&WScriptHash::all_zeros()); + let removed_output_1 = + TxOut { value: Amount::from_sat(10_000), script_pubkey: removed_script.clone() }; + let removed_output_2 = + TxOut { value: Amount::from_sat(12_000), script_pubkey: removed_script.clone() }; + let kept_output = TxOut { value: Amount::from_sat(15_000), script_pubkey: kept_script }; - // barely covers - { - let expected_fee = if cfg!(feature = "grind_signatures") { 2278 } else { 2284 }; - let contribution = FundingContribution { - value_added: Amount::from_sat(300_000 - expected_fee - 20), - estimated_fee: Amount::from_sat(expected_fee), - inputs: vec![ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - outputs: vec![], - change_output: None, - is_splice: true, - feerate: FeeRate::from_sat_per_kwu(2000), - max_feerate: FeeRate::MAX, - }; - assert!(contribution.validate().is_ok()); - assert_eq!(contribution.net_value(), contribution.value_added.to_signed().unwrap()); - } + let contribution = + FundingBuilder::new(FundingTemplate::new(None, None, None), feerate, FeeRate::MAX) + .add_output(removed_output_1) + .add_output(kept_output.clone()) + .add_output(removed_output_2) + .remove_outputs(&removed_script) + .build() + .unwrap(); - // higher fee rate, does not cover - { - let expected_fee = if cfg!(feature = "grind_signatures") { 2506 } else { 2513 }; - let contribution = FundingContribution { - value_added: Amount::from_sat(298032), - estimated_fee: Amount::from_sat(expected_fee), - inputs: vec![ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - outputs: vec![], - change_output: None, - is_splice: true, - feerate: FeeRate::from_sat_per_kwu(2200), - max_feerate: FeeRate::MAX, - }; - assert_eq!( - contribution.validate(), - Err(format!( - "Total input amount 0.00300000 BTC is lower than needed for splice-in contribution 0.00298032 BTC, considering fees of {}. Need more inputs.", - Amount::from_sat(expected_fee), - )), - ); - } + assert_eq!(contribution.outputs, vec![kept_output]); + } - // barely covers, less fees (not a splice) - { - let expected_fee = if cfg!(feature = "grind_signatures") { 1512 } else { 1516 }; - let contribution = FundingContribution { - value_added: Amount::from_sat(300_000 - expected_fee - 20), - estimated_fee: Amount::from_sat(expected_fee), - inputs: vec![ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - outputs: vec![], - change_output: None, - is_splice: false, - feerate: FeeRate::from_sat_per_kwu(2000), - max_feerate: FeeRate::MAX, - }; - assert!(contribution.validate().is_ok()); - assert_eq!(contribution.net_value(), contribution.value_added.to_signed().unwrap()); - } + #[test] + fn test_sync_funding_builder_add_and_remove_value_update_request() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let builder = + FundingBuilder::new(FundingTemplate::new(None, None, None), feerate, FeeRate::MAX) + .with_coin_selection_source_sync(UnreachableWallet) + .add_value(Amount::from_sat(20_000)) + .add_value(Amount::from_sat(5_000)) + .remove_value(Amount::from_sat(10_000)); + + let (_, must_pay_to) = builder.0.prepare_coin_selection_request().unwrap(); + assert_eq!(must_pay_to.len(), 1); + assert_eq!(must_pay_to[0].value, Amount::from_sat(15_000)); } - struct UnreachableWallet; + #[test] + fn test_sync_funding_builder_remove_value_saturates_at_zero() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let output = funding_output_sats(8_000); + let contribution = + FundingBuilder::new(FundingTemplate::new(None, None, None), feerate, FeeRate::MAX) + .with_coin_selection_source_sync(UnreachableWallet) + .add_value(Amount::from_sat(10_000)) + .remove_value(Amount::from_sat(15_000)) + .add_output(output.clone()) + .build() + .unwrap(); - impl CoinSelectionSourceSync for UnreachableWallet { - 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 { - unreachable!("should not reach coin selection") - } - fn sign_psbt(&self, _psbt: Psbt) -> Result { - unreachable!("should not reach signing") - } + assert!(contribution.inputs.is_empty()); + assert_eq!(contribution.outputs, vec![output]); + assert!(contribution.change_output.is_none()); + assert_eq!(contribution.value_added(), Amount::ZERO); } #[test] @@ -1492,17 +1798,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); @@ -1511,42 +1817,42 @@ 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), )); } + } - // 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_funding_builder_validates_mixed_request_max_money() { + let over_max = Amount::MAX_MONEY + Amount::from_sat(1); + let feerate = FeeRate::from_sat_per_kwu(2000); - // 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), - )); - } + // Mixed add/remove request with value_added > MAX_MONEY. + assert!(matches!( + FundingTemplate::new(None, None, None) + .without_prior_contribution(feerate, feerate) + .with_coin_selection_source_sync(UnreachableWallet) + .add_value(over_max) + .add_outputs(vec![funding_output_sats(1_000)]) + .build(), + Err(FundingContributionError::InvalidSpliceValue), + )); + + // Mixed add/remove request with outputs summing > MAX_MONEY. + let half_over = Amount::MAX_MONEY / 2 + Amount::from_sat(1); + assert!(matches!( + FundingTemplate::new(None, None, None) + .without_prior_contribution(feerate, feerate) + .with_coin_selection_source_sync(UnreachableWallet) + .add_value(Amount::from_sat(1_000)) + .add_outputs(vec![ + funding_output_sats(half_over.to_sat()), + funding_output_sats(half_over.to_sat()), + ]) + .build(), + Err(FundingContributionError::InvalidSpliceValue), + )); } #[test] @@ -1578,6 +1884,34 @@ mod tests { } } + #[test] + fn test_build_funding_contribution_rejects_oversized_prevtx() { + use crate::util::ser::Writeable; + + let feerate = FeeRate::from_sat_per_kwu(2000); + let prevtx = Transaction { + input: vec![], + output: vec![funding_output_sats(50_000); 2_200], + version: Version::TWO, + lock_time: bitcoin::absolute::LockTime::ZERO, + }; + assert!(prevtx.serialized_length() > crate::ln::LN_MAX_MSG_LEN); + + let wallet = SingleUtxoWallet { + utxo: FundingTxInput::new_p2wpkh(prevtx, 0).unwrap(), + change_output: None, + }; + assert!(matches!( + FundingTemplate::new(None, None, None).splice_in_sync( + Amount::from_sat(10_000), + feerate, + feerate, + wallet, + ), + Err(FundingContributionError::PrevTxTooLarge), + )); + } + #[test] fn test_for_acceptor_at_feerate_higher_change_adjusted() { // Splice-in: higher target feerate reduces the change output. @@ -1594,7 +1928,6 @@ mod tests { estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::from_sat(50_000), estimated_fee, inputs: inputs.clone(), outputs: vec![], @@ -1632,7 +1965,6 @@ mod tests { estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::from_sat(50_000), estimated_fee, inputs, outputs: vec![], @@ -1673,7 +2005,6 @@ mod tests { let change = funding_output_sats(change_value.to_sat()); let contribution = FundingContribution { - value_added, estimated_fee, inputs: inputs.clone(), outputs: vec![], @@ -1709,7 +2040,6 @@ mod tests { estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::from_sat(50_000), estimated_fee, inputs, outputs: vec![], @@ -1735,7 +2065,6 @@ mod tests { estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::ZERO, estimated_fee, inputs: vec![], outputs: outputs.clone(), @@ -1765,7 +2094,6 @@ mod tests { estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::ZERO, estimated_fee, inputs: vec![], outputs, @@ -1789,12 +2117,12 @@ mod tests { let target_feerate = FeeRate::from_sat_per_kwu(3000); let inputs = vec![funding_input_sats(100_000)]; let change = funding_output_sats(10_000); + let change_value = change.value; let estimated_fee = estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::from_sat(50_000), estimated_fee, inputs, outputs: vec![], @@ -1809,7 +2137,10 @@ mod tests { let net_at_feerate = contribution.net_value_for_acceptor_at_feerate(target_feerate, Amount::MAX).unwrap(); assert_eq!(net_at_feerate, contribution.net_value()); - assert_eq!(net_at_feerate, Amount::from_sat(50_000).to_signed().unwrap()); + assert_eq!( + net_at_feerate, + (Amount::from_sat(100_000) - estimated_fee - change_value).to_signed().unwrap(), + ); } #[test] @@ -1824,7 +2155,6 @@ mod tests { estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::ZERO, estimated_fee, inputs: vec![], outputs: outputs.clone(), @@ -1848,6 +2178,81 @@ mod tests { assert!(net_at_feerate > contribution.net_value()); } + #[test] + fn test_for_acceptor_at_feerate_mixed_splice_out_balance_insufficient() { + // Inputs are present, but the contribution is net-negative overall, so the remaining value + // must still come from the channel balance. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = original_feerate; + let inputs = vec![funding_input_sats(100_000)]; + let outputs = vec![funding_output_sats(120_000)]; + + let estimated_fee = + estimate_transaction_fee(&inputs, &outputs, None, true, true, original_feerate); + let target_fee = + estimate_transaction_fee(&inputs, &outputs, None, false, true, target_feerate); + + let contribution = FundingContribution { + estimated_fee, + inputs, + outputs, + change_output: None, + feerate: original_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let holder_balance = target_fee + Amount::from_sat(19_999); + match contribution.for_acceptor_at_feerate(target_feerate, holder_balance) { + Err(FeeRateAdjustmentError::FeeBufferInsufficient { source, available, required }) => { + assert_eq!(source, "channel balance"); + assert_eq!(available, target_fee - Amount::from_sat(1)); + assert_eq!(required, target_fee); + }, + other => panic!("Expected channel-balance shortfall, got {other:?}"), + } + } + + #[test] + fn test_for_acceptor_at_feerate_mixed_splice_out_balance_sufficient() { + // Inputs are present, but the contribution is net-negative overall, so success still depends + // on the channel balance covering the remaining amount plus fees. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = original_feerate; + let inputs = vec![funding_input_sats(100_000)]; + let outputs = vec![funding_output_sats(120_000)]; + + let estimated_fee = + estimate_transaction_fee(&inputs, &outputs, None, true, true, original_feerate); + let target_fee = + estimate_transaction_fee(&inputs, &outputs, None, false, true, target_feerate); + + let contribution = FundingContribution { + estimated_fee, + inputs: inputs.clone(), + outputs: outputs.clone(), + change_output: None, + feerate: original_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let holder_balance = target_fee + Amount::from_sat(20_000); + let adjusted = + contribution.for_acceptor_at_feerate(target_feerate, holder_balance).unwrap(); + + assert_eq!(adjusted.inputs, inputs); + assert_eq!(adjusted.outputs, outputs); + assert!(adjusted.change_output.is_none()); + assert_eq!(adjusted.estimated_fee, target_fee); + assert_eq!( + adjusted.net_value(), + SignedAmount::ZERO + - Amount::from_sat(20_000).to_signed().unwrap() + - target_fee.to_signed().unwrap(), + ); + } + #[test] fn test_net_value_for_acceptor_at_feerate_does_not_mutate() { // Verify net_value_for_acceptor_at_feerate does not modify the contribution. @@ -1860,7 +2265,6 @@ mod tests { estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::from_sat(50_000), estimated_fee, inputs, outputs: vec![], @@ -1882,6 +2286,54 @@ mod tests { assert_eq!(contribution.change_output.as_ref().unwrap().value, change_before); } + #[test] + fn test_net_value_for_initiator_at_feerate_change_removed() { + // The non-mutating initiator path should match the adjusted contribution when increasing the + // feerate drops the change output. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(3000); + let value_added = Amount::from_sat(50_000); + let change_value = Amount::from_sat(1_000); + + let dummy_inputs = vec![funding_input_sats(1)]; + let change = funding_output_sats(change_value.to_sat()); + let estimated_fee = estimate_transaction_fee( + &dummy_inputs, + &[], + Some(&change), + true, + true, + original_feerate, + ); + let inputs = + vec![funding_input_sats((value_added + estimated_fee + change_value).to_sat())]; + + let contribution = FundingContribution { + estimated_fee, + inputs: inputs.clone(), + outputs: vec![], + change_output: Some(change), + feerate: original_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let adjusted = + contribution.clone().for_initiator_at_feerate(target_feerate, Amount::MAX).unwrap(); + let net_at_feerate = + contribution.net_value_for_initiator_at_feerate(target_feerate, Amount::MAX).unwrap(); + + let expected_fee_no_change = + estimate_transaction_fee(&inputs, &[], None, true, true, target_feerate); + assert!(adjusted.change_output.is_none()); + assert_eq!(adjusted.estimated_fee, expected_fee_no_change); + assert_eq!(net_at_feerate, adjusted.net_value()); + + // The non-mutating helper must leave the original contribution untouched. + assert_eq!(contribution.estimated_fee, estimated_fee); + assert_eq!(contribution.change_output.as_ref().unwrap().value, change_value); + } + #[test] fn test_net_value_for_acceptor_at_feerate_too_high() { // net_value_for_acceptor_at_feerate returns Err when feerate can't be accommodated. @@ -1894,7 +2346,6 @@ mod tests { estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::from_sat(50_000), estimated_fee, inputs, outputs: vec![], @@ -1922,7 +2373,6 @@ mod tests { estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::from_sat(50_000), estimated_fee, inputs, outputs: vec![], @@ -1954,7 +2404,6 @@ mod tests { estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::from_sat(50_000), estimated_fee, inputs, outputs: vec![], @@ -1989,7 +2438,6 @@ mod tests { estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::from_sat(50_000), estimated_fee, inputs, outputs: vec![], @@ -2032,7 +2480,6 @@ mod tests { assert!(target_fee > estimated_fee); let contribution = FundingContribution { - value_added, estimated_fee, inputs, outputs: vec![], @@ -2065,7 +2512,6 @@ mod tests { assert!(target_fee > estimated_fee); let contribution = FundingContribution { - value_added, estimated_fee, inputs, outputs: vec![], @@ -2104,7 +2550,6 @@ mod tests { assert!(estimated_fee - target_fee < dust_limit); let contribution = FundingContribution { - value_added: Amount::from_sat(50_000), estimated_fee, inputs, outputs: vec![], @@ -2141,7 +2586,6 @@ mod tests { let target_fee = estimate_transaction_fee(&inputs, &[], None, false, true, feerate); let contribution = FundingContribution { - value_added, estimated_fee, inputs, outputs: vec![], @@ -2160,20 +2604,17 @@ mod tests { assert!(adjusted.change_output.is_none()); assert_eq!(adjusted.estimated_fee, target_fee); let surplus = estimated_fee - target_fee; - assert_eq!(adjusted.value_added, value_added + surplus); + assert_eq!(adjusted.value_added(), value_added + surplus); assert_eq!(adjusted.net_value(), net_value_before + surplus.to_signed().unwrap()); } #[test] - fn test_for_acceptor_at_feerate_fee_buffer_overflow() { - // Construct a contribution with estimated_fee and change values that overflow Amount. + fn test_for_acceptor_at_feerate_fee_buffer_overflow_with_change() { + // Overflow in estimated_fee + change value should surface as FeeBufferOverflow. let feerate = FeeRate::from_sat_per_kwu(2000); - let inputs = vec![funding_input_sats(100_000)]; - let contribution = FundingContribution { - value_added: Amount::from_sat(50_000), estimated_fee: Amount::MAX, - inputs, + inputs: vec![funding_input_sats(100_000)], outputs: vec![], change_output: Some(funding_output_sats(1)), feerate, @@ -2196,7 +2637,6 @@ mod tests { estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::ZERO, estimated_fee, inputs: vec![], outputs: outputs.clone(), @@ -2223,7 +2663,6 @@ mod tests { estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::ZERO, estimated_fee, inputs: vec![], outputs: outputs.clone(), @@ -2254,7 +2693,6 @@ mod tests { estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::ZERO, estimated_fee, inputs: vec![], outputs, @@ -2283,7 +2721,6 @@ mod tests { estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate); let contribution = FundingContribution { - value_added: Amount::from_sat(50_000), estimated_fee, inputs, outputs: vec![], @@ -2311,14 +2748,13 @@ mod tests { #[test] fn test_rbf_sync_rejects_max_feerate_below_min_rbf_feerate() { - // When the caller's max_feerate is below the minimum RBF feerate, rbf_sync should - // return Err(()). + // When the caller's max_feerate is below the minimum RBF feerate, + // rbf_prior_contribution_sync should return an error. let prior_feerate = FeeRate::from_sat_per_kwu(2000); let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); let max_feerate = FeeRate::from_sat_per_kwu(2020); let prior = FundingContribution { - value_added: Amount::from_sat(50_000), estimated_fee: Amount::from_sat(1_000), inputs: vec![funding_input_sats(100_000)], outputs: vec![], @@ -2332,10 +2768,10 @@ mod tests { let template = FundingTemplate::new( None, Some(min_rbf_feerate), - Some(PriorContribution::new(prior, None)), + Some(PriorContribution::new(prior, Amount::MAX)), ); assert!(matches!( - template.rbf_sync(max_feerate, UnreachableWallet), + template.rbf_prior_contribution_sync(None, max_feerate, UnreachableWallet), Err(FundingContributionError::FeeRateExceedsMaximum { .. }), )); } @@ -2343,7 +2779,8 @@ mod tests { #[test] fn test_rbf_sync_adjusts_prior_to_rbf_feerate() { // When the prior contribution's feerate is below the minimum RBF feerate and holder - // balance is available, rbf_sync should adjust the prior to the RBF feerate. + // balance is available, rbf_prior_contribution_sync should adjust the prior to the + // RBF feerate. let prior_feerate = FeeRate::from_sat_per_kwu(2000); let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); let max_feerate = FeeRate::from_sat_per_kwu(5000); @@ -2354,7 +2791,6 @@ mod tests { 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![], @@ -2367,13 +2803,111 @@ mod tests { let template = FundingTemplate::new( None, Some(min_rbf_feerate), - Some(PriorContribution::new(prior, Some(Amount::MAX))), + Some(PriorContribution::new(prior, 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_override_feerate() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); + let override_feerate = FeeRate::from_sat_per_kwu(2100); + let max_feerate = FeeRate::from_sat_per_kwu(5000); + + let inputs = vec![funding_input_sats(100_000)]; + let change = funding_output_sats(10_000); + let estimated_fee = + estimate_transaction_fee(&inputs, &[], Some(&change), true, true, prior_feerate); + + let prior = FundingContribution { + estimated_fee, + inputs, + outputs: vec![], + change_output: Some(change), + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let template = FundingTemplate::new( + None, + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, Amount::MAX)), + ); + let contribution = template + .rbf_prior_contribution_sync(Some(override_feerate), max_feerate, UnreachableWallet) + .unwrap(); + assert_eq!(contribution.feerate, override_feerate); + assert_eq!(contribution.max_feerate, max_feerate); + } + + #[test] + fn test_rbf_sync_rejects_explicit_override_below_min_rbf_feerate() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); + let override_feerate = FeeRate::from_sat_per_kwu(2024); + + let prior = FundingContribution { + estimated_fee: Amount::from_sat(1_000), + inputs: vec![funding_input_sats(100_000)], + outputs: vec![], + change_output: None, + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let template = FundingTemplate::new( + None, + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, Amount::MAX)), + ); + assert!(matches!( + template.rbf_prior_contribution_sync( + Some(override_feerate), + FeeRate::MAX, + UnreachableWallet, + ), + Err(FundingContributionError::FeeRateBelowRbfMinimum { .. }), + )); + } + + #[test] + fn test_rbf_sync_rejects_explicit_override_above_max_feerate() { + let prior_feerate = FeeRate::from_sat_per_kwu(2000); + let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); + let override_feerate = FeeRate::from_sat_per_kwu(2100); + let max_feerate = FeeRate::from_sat_per_kwu(2099); + + let prior = FundingContribution { + estimated_fee: Amount::from_sat(1_000), + inputs: vec![funding_input_sats(100_000)], + outputs: vec![], + change_output: None, + feerate: prior_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + }; + + let template = FundingTemplate::new( + None, + Some(min_rbf_feerate), + Some(PriorContribution::new(prior, Amount::MAX)), + ); + assert!(matches!( + template.rbf_prior_contribution_sync( + Some(override_feerate), + max_feerate, + UnreachableWallet, + ), + Err(FundingContributionError::FeeRateExceedsMaximum { .. }), + )); + } + /// A mock wallet that returns a single UTXO for coin selection. struct SingleUtxoWallet { utxo: FundingTxInput, @@ -2409,14 +2943,13 @@ mod tests { #[test] fn test_rbf_sync_unadjusted_splice_out_runs_coin_selection() { // When the prior contribution's feerate is below the minimum RBF feerate and no - // holder balance is available, rbf_sync should run coin selection to add inputs that - // cover the higher RBF fee. + // holder balance is available, rbf_prior_contribution_sync should run coin selection to + // add inputs that cover the higher RBF fee. let prior_feerate = FeeRate::from_sat_per_kwu(2000); let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); let withdrawal = funding_output_sats(20_000); let prior = FundingContribution { - value_added: Amount::ZERO, estimated_fee: Amount::from_sat(500), inputs: vec![], outputs: vec![withdrawal.clone()], @@ -2429,7 +2962,7 @@ mod tests { let template = FundingTemplate::new( Some(shared_input(100_000)), Some(min_rbf_feerate), - Some(PriorContribution::new(prior, None)), + Some(PriorContribution::new(prior, Amount::ZERO)), ); let wallet = SingleUtxoWallet { @@ -2437,47 +2970,27 @@ mod tests { change_output: Some(funding_output_sats(40_000)), }; - // rbf_sync should succeed and the contribution should have inputs from coin selection. - let contribution = template.rbf_sync(FeeRate::MAX, &wallet).unwrap(); - assert_eq!(contribution.value_added, Amount::ZERO); + // rbf_prior_contribution_sync should succeed and the contribution should have inputs from + // coin selection. + let contribution = + template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet).unwrap(); assert!(!contribution.inputs.is_empty(), "coin selection should have added inputs"); + assert!(contribution.value_added() > Amount::ZERO); assert_eq!(contribution.outputs, vec![withdrawal]); assert_eq!(contribution.feerate, min_rbf_feerate); } - #[test] - fn test_rbf_sync_no_prior_fee_bump_only_runs_coin_selection() { - // When there is no prior contribution (e.g., acceptor), rbf_sync should run coin - // selection to add inputs for a fee-bump-only contribution. - let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); - - let template = - FundingTemplate::new(Some(shared_input(100_000)), Some(min_rbf_feerate), None); - - let wallet = SingleUtxoWallet { - utxo: funding_input_sats(50_000), - change_output: Some(funding_output_sats(45_000)), - }; - - let contribution = template.rbf_sync(FeeRate::MAX, &wallet).unwrap(); - assert_eq!(contribution.value_added, Amount::ZERO); - assert!(!contribution.inputs.is_empty(), "coin selection should have added inputs"); - assert!(contribution.outputs.is_empty()); - assert_eq!(contribution.feerate, min_rbf_feerate); - } - #[test] fn test_rbf_sync_unadjusted_uses_callers_max_feerate() { // When the prior contribution's feerate is below the minimum RBF feerate and no - // holder balance is available, rbf_sync should use the caller's max_feerate (not the - // prior's) for the resulting contribution. + // holder balance is available, rbf_prior_contribution_sync should use the caller's + // max_feerate (not the prior's) for the resulting contribution. let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); let prior_max_feerate = FeeRate::from_sat_per_kwu(50_000); let callers_max_feerate = FeeRate::from_sat_per_kwu(10_000); let withdrawal = funding_output_sats(20_000); let prior = FundingContribution { - value_added: Amount::ZERO, estimated_fee: Amount::from_sat(500), inputs: vec![], outputs: vec![withdrawal.clone()], @@ -2490,7 +3003,7 @@ mod tests { let template = FundingTemplate::new( Some(shared_input(100_000)), Some(min_rbf_feerate), - Some(PriorContribution::new(prior, None)), + Some(PriorContribution::new(prior, Amount::MAX)), ); let wallet = SingleUtxoWallet { @@ -2498,7 +3011,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" @@ -2506,9 +3020,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. + 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_prior_contribution_sync), coin selection should NOT + // run. // Fees come from the channel balance. let min_rbf_feerate = FeeRate::from_sat_per_kwu(2025); let feerate = FeeRate::from_sat_per_kwu(2025); @@ -2517,12 +3032,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(); - assert_eq!(contribution.value_added, Amount::ZERO); + 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 9adccd17627..3885db54e8b 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -20,7 +20,7 @@ use crate::ln::channel::{ }; use crate::ln::channelmanager::{provided_init_features, PaymentId, BREAKDOWN_TIMEOUT}; use crate::ln::functional_test_utils::*; -use crate::ln::funding::FundingContribution; +use crate::ln::funding::{FundingContribution, FundingContributionError}; use crate::ln::msgs::{self, BaseMessageHandler, ChannelMessageHandler, MessageSendEvent}; use crate::ln::outbound_payment::RecipientOnionFields; use crate::ln::types::ChannelId; @@ -41,7 +41,7 @@ use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; use bitcoin::transaction::Version; use bitcoin::{ Amount, FeeRate, OutPoint as BitcoinOutPoint, Psbt, ScriptBuf, Transaction, TxOut, Txid, - WPubkeyHash, + WPubkeyHash, WScriptHash, }; #[test] @@ -158,40 +158,6 @@ impl CoinSelectionSourceSync for TightBudgetWallet { } } -#[test] -fn test_validate_accounts_for_change_output_weight() { - // Demonstrates that estimated_fee includes the change output's weight when building a - // FundingContribution. A mock wallet returns a single input whose value is between - // estimated_fee_without_change (1736/1740 sats) and estimated_fee_with_change (1984/1988 - // sats) above value_added. The validate() check correctly catches that the inputs are - // insufficient when the change output weight is included. Without accounting for the change - // output weight, the check would incorrectly pass. - 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 (_, _, channel_id, _) = - create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0); - - let feerate = FeeRate::from_sat_per_kwu(2000); - let funding_template = - nodes[0].node.splice_channel(&channel_id, &nodes[1].node.get_our_node_id()).unwrap(); - - // Input value = value_added + 1800: above 1736/1740 (fee without change), below 1984/1988 - // (fee with change). - let value_added = Amount::from_sat(20_000); - let wallet = TightBudgetWallet { - utxo_value: value_added + Amount::from_sat(1800), - change_value: Amount::from_sat(1000), - }; - let contribution = - funding_template.splice_in_sync(value_added, feerate, FeeRate::MAX, &wallet).unwrap(); - - assert!(contribution.change_output().is_some()); - assert!(contribution.validate().is_err()); -} - pub fn negotiate_splice_tx<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, funding_contribution: FundingContribution, @@ -255,7 +221,11 @@ pub fn do_initiate_rbf_splice_in_and_out<'a, 'b, 'c, 'd>( 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) + .without_prior_contribution(feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(value_added) + .add_outputs(outputs) + .build() .unwrap(); node.node .funding_contributed(&channel_id, &node_id_counterparty, funding_contribution.clone(), None) @@ -271,9 +241,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, @@ -305,7 +273,11 @@ pub fn do_initiate_splice_in_and_out<'a, 'b, 'c, 'd>( 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) + .without_prior_contribution(feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .add_value(value_added) + .add_outputs(outputs) + .build() .unwrap(); initiator .node @@ -1370,9 +1342,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) @@ -1865,7 +1836,8 @@ fn do_test_splice_commitment_broadcast(splice_status: SpliceStatus, claim_htlcs: let splice_in_amount = initial_channel_capacity / 2; let initiator_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, Amount::from_sat(splice_in_amount)); - let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, initiator_contribution); + let (splice_tx, _) = + splice_channel(&nodes[0], &nodes[1], channel_id, initiator_contribution.clone()); let (preimage2, payment_hash2, ..) = route_payment(&nodes[0], &[&nodes[1]], payment_amount); let htlc_expiry = nodes[0].best_block_info().1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS; @@ -1916,7 +1888,7 @@ fn do_test_splice_commitment_broadcast(splice_status: SpliceStatus, claim_htlcs: message: "test".to_owned(), }; let closed_channel_capacity = if splice_status == SpliceStatus::Locked { - initial_channel_capacity + splice_in_amount + initial_channel_capacity + initiator_contribution.net_value().to_sat() as u64 } else { initial_channel_capacity }; @@ -3690,7 +3662,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())), @@ -3699,13 +3671,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 @@ -3727,13 +3697,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 @@ -5543,7 +5511,8 @@ 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); @@ -5557,7 +5526,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()); } @@ -5758,6 +5728,128 @@ fn test_splice_rbf_sequential() { ); } +#[test] +fn test_splice_rbf_amends_prior_contribution_request() { + 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 (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0); + + let initial_added_value = Amount::from_sat(100_000); + let half_added_value = Amount::from_sat(initial_added_value.to_sat() / 2); + provide_utxo_reserves(&nodes, 1, Amount::from_sat(250_000)); + + let initial_contribution = + do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, initial_added_value); + let (initial_inputs, _) = initial_contribution.clone().into_contributed_inputs_and_outputs(); + let (splice_tx_0, new_funding_script) = + splice_channel(&nodes[0], &nodes[1], channel_id, initial_contribution.clone()); + + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let first_output = TxOut { + value: Amount::from_sat(10_000), + script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_raw_hash(Hash::all_zeros())), + }; + let second_output = TxOut { + value: Amount::from_sat(15_000), + script_pubkey: ScriptBuf::new_p2wsh(&WScriptHash::all_zeros()), + }; + + let run_rbf_round = |contribution: FundingContribution| { + 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 (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); + tx + }; + + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + assert!(funding_template.prior_contribution().unwrap().outputs().is_empty()); + let rbf_feerate = funding_template.min_rbf_feerate().unwrap(); + let contribution_1 = funding_template + .splice_out(vec![first_output.clone(), second_output.clone()], rbf_feerate, FeeRate::MAX) + .unwrap(); + let (inputs_1, _) = contribution_1.clone().into_contributed_inputs_and_outputs(); + assert_eq!(inputs_1, initial_inputs); + assert_eq!(contribution_1.outputs(), &[first_output.clone(), second_output.clone()]); + assert!(contribution_1.net_value() < initial_contribution.net_value()); + let splice_tx_1 = run_rbf_round(contribution_1.clone()); + + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + assert_eq!(funding_template.prior_contribution().unwrap().outputs(), contribution_1.outputs()); + let rbf_feerate = funding_template.min_rbf_feerate().unwrap(); + let contribution_2 = funding_template + .with_prior_contribution(rbf_feerate, FeeRate::MAX) + .with_coin_selection_source_sync(&wallet) + .remove_value(half_added_value) + .build() + .unwrap(); + let (inputs_2, _) = contribution_2.clone().into_contributed_inputs_and_outputs(); + assert_eq!(inputs_2, initial_inputs); + assert_eq!(contribution_2.outputs(), contribution_1.outputs()); + assert!(contribution_2.net_value() < contribution_1.net_value()); + let splice_tx_2 = run_rbf_round(contribution_2.clone()); + + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + assert_eq!(funding_template.prior_contribution().unwrap().outputs(), contribution_2.outputs()); + let rbf_feerate = funding_template.min_rbf_feerate().unwrap(); + let contribution_3 = funding_template + .with_prior_contribution(rbf_feerate, FeeRate::MAX) + .remove_outputs(&first_output.script_pubkey) + .build() + .unwrap(); + let (inputs_3, _) = contribution_3.clone().into_contributed_inputs_and_outputs(); + assert_eq!(inputs_3, initial_inputs); + assert_eq!(contribution_3.outputs(), std::slice::from_ref(&second_output)); + assert!(contribution_3.net_value() > contribution_2.net_value()); + let splice_tx_3 = run_rbf_round(contribution_3.clone()); + + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + assert_eq!(funding_template.prior_contribution().unwrap().outputs(), contribution_3.outputs()); + let contribution_4 = + funding_template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet).unwrap(); + let (inputs_4, _) = contribution_4.clone().into_contributed_inputs_and_outputs(); + assert_eq!(inputs_4, initial_inputs); + assert_eq!(contribution_4.outputs(), contribution_3.outputs()); + assert_eq!(contribution_4.net_value(), contribution_3.net_value()); + assert!( + contribution_4.change_output().unwrap().value + < contribution_3.change_output().unwrap().value + ); + let rbf_tx_final = run_rbf_round(contribution_4); + + lock_rbf_splice_after_blocks( + &nodes[0], + &nodes[1], + &rbf_tx_final, + ANTI_REORG_DELAY - 1, + &[ + splice_tx_0.compute_txid(), + splice_tx_1.compute_txid(), + splice_tx_2.compute_txid(), + splice_tx_3.compute_txid(), + ], + ); +} + #[test] fn test_splice_rbf_acceptor_contributes_then_disconnects() { // When both nodes contribute to a splice and the initiator RBFs (with the acceptor @@ -6010,9 +6102,9 @@ fn test_splice_channel_with_pending_splice_includes_rbf_floor() { assert_eq!(funding_template.min_rbf_feerate(), Some(expected_floor)); assert!(funding_template.prior_contribution().is_some()); - // rbf_sync returns the Adjusted prior contribution directly. + // rbf_prior_contribution_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] @@ -6213,8 +6305,8 @@ fn test_funding_contributed_rbf_adjustment_insufficient_budget() { #[test] fn test_prior_contribution_unadjusted_when_max_feerate_too_low() { - // Test that rbf_sync re-runs coin selection when the prior contribution's max_feerate is - // too low to accommodate the minimum RBF feerate. + // Test that rbf_prior_contribution_sync re-runs coin selection when the prior + // contribution's max_feerate is too low to accommodate the minimum RBF feerate. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); @@ -6244,13 +6336,13 @@ fn test_prior_contribution_unadjusted_when_max_feerate_too_low() { let (_splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); // Call splice_channel again — the minimum RBF feerate (floor + 25 sat/kwu) exceeds the prior - // contribution's max_feerate (floor), so adjustment fails. rbf_sync re-runs coin selection - // with the caller's max_feerate. + // contribution's max_feerate (floor), so adjustment fails. + // rbf_prior_contribution_sync re-runs coin selection with the caller's max_feerate. let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); assert!(funding_template.min_rbf_feerate().is_some()); assert!(funding_template.prior_contribution().is_some()); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - assert!(funding_template.rbf_sync(FeeRate::MAX, &wallet).is_ok()); + assert!(funding_template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet).is_ok()); } #[test] @@ -6291,17 +6383,19 @@ fn test_splice_channel_during_negotiation_includes_rbf_feerate() { let expected_floor = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64 + 25); assert_eq!(template.min_rbf_feerate(), Some(expected_floor)); - // No prior contribution since there are no negotiated candidates yet. rbf_sync runs - // 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!(matches!( + template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet), + Err(FundingContributionError::NotRbfScenario) + )); } #[test] fn test_rbf_sync_returns_err_when_no_min_rbf_feerate() { - // Test that rbf_sync returns Err(()) when there is no pending splice (min_rbf_feerate is - // None), indicating this is not an RBF scenario. + // Test that rbf_prior_contribution_sync returns `NotRbfScenario` when there is no pending + // splice (min_rbf_feerate is None). 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]); @@ -6323,15 +6417,15 @@ 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), )); } #[test] fn test_rbf_sync_returns_err_when_max_feerate_below_min_rbf() { - // Test that rbf_sync returns Err when the caller's max_feerate is below the minimum - // RBF feerate. + // Test that rbf_prior_contribution_sync returns an error when the caller's max_feerate is + // below the minimum RBF feerate. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); @@ -6359,7 +6453,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 { .. }), )); } @@ -6420,9 +6514,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"); @@ -6632,9 +6724,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);