From b081696fbe4b31ef65cbcd6c9227f6a564e3926a Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 23 Apr 2026 10:07:44 -0700 Subject: [PATCH 1/3] Support manually selecting inputs consuming their entire value This commit introduces an alternative way of splicing in funds without coin selection by requiring the full UTXO to be provided. Each UTXO's entire value (minus fees) is allocated towards the channel, which provides unified balance wallets a more intuitive API when splicing funds into the channel, as they don't particularly care about maintaining a portion of their balance onchain. To simplify the implementation, we require that contributions are not allowed to mix coin-selected inputs with manually-selected ones. Users will need to start a fresh contribution if they want to change the funding input mode. --- lightning/src/ln/funding.rs | 1077 +++++++++++++++++++++++----- lightning/src/ln/splicing_tests.rs | 75 +- 2 files changed, 983 insertions(+), 169 deletions(-) diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 386aa3d92a3..dd9611e830c 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -24,7 +24,7 @@ use crate::ln::LN_MAX_MSG_LEN; use crate::prelude::*; use crate::util::native_async::MaybeSend; use crate::util::wallet_utils::{ - CoinSelection, CoinSelectionSource, CoinSelectionSourceSync, Input, + CoinSelection, CoinSelectionSource, CoinSelectionSourceSync, ConfirmedUtxo, Input, }; /// Error returned when a [`FundingContribution`] cannot be adjusted to a target feerate. @@ -147,6 +147,8 @@ pub enum FundingContributionError { /// the builder fall back to fresh coin selection, which may replace the prior input set instead /// of preserving it. MissingCoinSelectionSource, + /// The request cannot be satisfied using the manually selected inputs. + ManuallySelectedInputsInsufficient, /// This template cannot build an RBF contribution. NotRbfScenario, } @@ -172,6 +174,9 @@ impl core::fmt::Display for FundingContributionError { FundingContributionError::MissingCoinSelectionSource => { write!(f, "Coin selection source required to build this contribution") }, + FundingContributionError::ManuallySelectedInputsInsufficient => { + write!(f, "The request cannot be satisfied using the manually selected inputs") + }, FundingContributionError::NotRbfScenario => { write!(f, "This template cannot build an RBF contribution") }, @@ -336,13 +341,15 @@ impl FundingTemplate { /// 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. + /// the prior contribution used. This is not supported when the prior contribution used manually + /// selected inputs; use [`FundingTemplate::splice_in_inputs`] or + /// [`FundingTemplate::without_prior_contribution`] in that case. pub async fn splice_in( self, value_added: Amount, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, ) -> Result { self.with_prior_contribution(min_feerate, max_feerate) .with_coin_selection_source(wallet) - .add_value(value_added) + .add_value(value_added)? .build() .await } @@ -350,16 +357,40 @@ impl FundingTemplate { /// Creates a [`FundingContribution`] for adding funds to a channel. /// /// This is the synchronous variant of [`FundingTemplate::splice_in`]; `value_added`, - /// `min_feerate`, `max_feerate`, and `wallet` have the same meaning. + /// `min_feerate`, `max_feerate`, and `wallet` have the same meaning, including the restriction + /// on prior contributions with manually selected inputs. pub fn splice_in_sync( self, value_added: Amount, min_feerate: FeeRate, max_feerate: FeeRate, wallet: W, ) -> Result { self.with_prior_contribution(min_feerate, max_feerate) .with_coin_selection_source_sync(wallet) - .add_value(value_added) + .add_value(value_added)? .build() } + /// Creates a [`FundingContribution`] for adding funds to a channel using manually selected + /// inputs. + /// + /// This is a convenience wrapper around [`FundingTemplate::with_prior_contribution`] with no + /// wallet attached. Each input is fully consumed with no change output, so the amount added to + /// the channel is derived from the total input value minus the estimated fee. + /// + /// When a prior contribution with manually selected inputs is present, `inputs` are appended to + /// the prior [`FundingContribution::inputs`] instead of replacing them. Use + /// [`FundingTemplate::without_prior_contribution`] if you want to replace the prior request + /// instead. If the template carries a coin-selected prior contribution, manual inputs are + /// incompatible and this method returns [`FundingContributionError::InvalidSpliceValue`]. + /// + /// `inputs` are the additional manually selected inputs to fully consume. `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`. + pub fn splice_in_inputs( + self, inputs: Vec, min_feerate: FeeRate, max_feerate: FeeRate, + ) -> Result { + self.with_prior_contribution(min_feerate, max_feerate).add_inputs(inputs)?.build() + } + /// Creates a [`FundingContribution`] for removing funds from a channel. /// /// This is a convenience wrapper around [`FundingTemplate::with_prior_contribution`] with no @@ -527,25 +558,70 @@ fn validate_inputs(inputs: &[FundingTxInput]) -> Result<(), FundingContributionE Ok(()) } -/// Describes how an amended contribution should source its wallet-backed inputs. +/// Describes how a contribution request should source its wallet-backed inputs. +#[derive(Debug, Clone, PartialEq, Eq)] 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, - }, + CoinSelected { value_added: Amount }, + /// Replaces the contribution's inputs with the provided set and fully consumes them without a + /// change output. The amount added to the channel is recomputed from the input total minus fees, + /// while explicit withdrawal outputs still reduce the splice's net value. + ManuallySelected { inputs: Vec }, } +impl FundingInputs { + fn mode(&self) -> FundingInputMode { + match self { + FundingInputs::CoinSelected { .. } => FundingInputMode::CoinSelected, + FundingInputs::ManuallySelected { .. } => FundingInputMode::ManuallySelected, + } + } + + fn is_empty(&self) -> bool { + match self { + FundingInputs::CoinSelected { value_added } => *value_added == Amount::ZERO, + FundingInputs::ManuallySelected { inputs } => inputs.is_empty(), + } + } + + fn value_added(&self) -> Amount { + match self { + FundingInputs::CoinSelected { value_added } => *value_added, + FundingInputs::ManuallySelected { .. } => Amount::ZERO, + } + } + + fn manually_selected_inputs(&self) -> &[FundingTxInput] { + match self { + FundingInputs::ManuallySelected { inputs } => inputs, + FundingInputs::CoinSelected { .. } => &[], + } + } +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +enum FundingInputMode { + CoinSelected, + ManuallySelected, +} + +impl_writeable_tlv_based_enum!(FundingInputMode, + (1, CoinSelected) => {}, + (3, ManuallySelected) => {} +); + /// The components of a funding transaction contributed by one party. #[derive(Debug, Clone, PartialEq, Eq)] pub struct FundingContribution { /// The estimate fees responsible to be paid for the contribution. estimated_fee: Amount, - /// The inputs included in the funding transaction to meet the contributed amount plus fees. Any - /// excess amount will be sent to a change output. + /// The inputs included in the funding transaction. + /// + /// For coin-selected contributions, excess value is returned via [`Self::change_output`]. For + /// manually selected inputs, the full input value is consumed and no change output is created. inputs: Vec, /// The outputs to include in the funding transaction. @@ -565,6 +641,12 @@ pub struct FundingContribution { /// Whether the contribution is for funding a splice. is_splice: bool, + + /// Whether this contribution currently uses coin-selected or manual-input semantics. + /// + /// This is `None` when the contribution has no inputs and is set accordingly based on the first + /// `add_value` or `add_input` call on the builder. + input_mode: Option, } impl_writeable_tlv_based!(FundingContribution, { @@ -575,6 +657,7 @@ impl_writeable_tlv_based!(FundingContribution, { (9, feerate, required), (11, max_feerate, required), (13, is_splice, required), + (15, input_mode, option), }); impl FundingContribution { @@ -594,11 +677,13 @@ impl FundingContribution { self.outputs.iter().chain(self.change_output.iter()) } - /// The value that will be added to the channel after fees. See [`Self::net_value`] for the net - /// value contribution to the channel. + /// The positive value added to the channel after explicit outputs and fees. + /// + /// This saturates at zero for net-negative contributions. See [`Self::net_value`] for the full + /// signed contribution to the channel. pub fn value_added(&self) -> Amount { let total_input_value = self.inputs.iter().map(|i| i.utxo.output.value).sum::(); - let total_output_value = self.outputs.iter().map(|output| output.value).sum::(); + let total_output_value = self.outputs.iter().map(|output| output.value).sum(); total_input_value .checked_sub(total_output_value) .and_then(|v| v.checked_sub(self.estimated_fee)) @@ -610,6 +695,11 @@ impl FundingContribution { .unwrap_or(Amount::ZERO) } + /// Returns the inputs included in this contribution. + pub fn inputs(&self) -> &[ConfirmedUtxo] { + &self.inputs + } + /// Returns the outputs (e.g., withdrawal destinations) included in this contribution. /// /// This does not include the change output; see [`FundingContribution::change_output`]. @@ -638,84 +728,91 @@ impl FundingContribution { /// Returns `None` if the request would require new wallet inputs or cannot accommodate the /// requested feerate. fn amend_without_coin_selection( - self, inputs: FundingInputs, outputs: &[TxOut], target_feerate: FeeRate, + self, funding_inputs: Option, outputs: &[TxOut], target_feerate: FeeRate, max_feerate: FeeRate, spliceable_balance: Amount, ) -> Option { // NOTE: The contribution returned is not guaranteed to be valid. We defer doing so until // `compute_feerate_adjustment`. - let adjust_for_inputs_and_outputs = - |contribution: Self, inputs: FundingInputs, outputs: &[TxOut]| -> Option { - let (target_value_added, inputs) = match inputs { - FundingInputs::None => (None, Vec::new()), - FundingInputs::CoinSelected { value_added } => { - (Some(value_added), contribution.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 adjust_for_inputs_and_outputs = |contribution: Self, + inputs: Option, + outputs: &[TxOut]| + -> Option { + let input_mode = inputs.as_ref().map(FundingInputs::mode); + let (target_value_added, inputs) = match inputs { + None => (None, Vec::new()), + Some(FundingInputs::CoinSelected { value_added }) => { + // We track the prior contribution's inputs here to see if they can cover the + // new `value_added` without running coin selection. + (Some(value_added), contribution.inputs) + }, + Some(FundingInputs::ManuallySelected { inputs }) => (None, inputs), + }; - // When inputs are coin-selected, adjust the existing change output, if any, to account - // for the requested value added and any explicit outputs that must also be funded by - // the inputs. - if let Some(value_added) = target_value_added { - let estimated_fee = estimate_transaction_fee( - &inputs, - &outputs, - contribution.change_output.as_ref(), - true, - contribution.is_splice, - contribution.feerate, - ); - let total_output_value: Amount = - outputs.iter().map(|output| output.value).sum(); - let required_value = - value_added.checked_add(total_output_value)?.checked_add(estimated_fee)?; - - if let Some(change_output) = contribution.change_output.as_ref() { - let dust_limit = change_output.script_pubkey.minimal_non_dust(); - let total_input_value: Amount = - inputs.iter().map(|input| input.utxo.output.value).sum(); - 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), - ..contribution - }); - }, - _ => {}, - } - } - } + 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( + // When inputs are coin-selected, adjust the existing change output, if any, to account + // for the requested value added and any explicit outputs that must also be funded by + // the inputs. + if let Some(value_added) = target_value_added { + let estimated_fee = estimate_transaction_fee( &inputs, &outputs, - None, + contribution.change_output.as_ref(), true, contribution.is_splice, contribution.feerate, ); - Some(FundingContribution { - estimated_fee: estimated_fee_no_change, - outputs: outputs.to_vec(), - inputs, - change_output: None, - ..contribution - }) - }; + let total_output_value: Amount = outputs.iter().map(|output| output.value).sum(); + let required_value = + value_added.checked_add(total_output_value)?.checked_add(estimated_fee)?; + + if let Some(change_output) = contribution.change_output.as_ref() { + let dust_limit = change_output.script_pubkey.minimal_non_dust(); + let total_input_value: Amount = + inputs.iter().map(|input| input.utxo.output.value).sum(); + 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), + input_mode, + ..contribution + }); + }, + _ => {}, + } + } + } + + let estimated_fee_no_change = estimate_transaction_fee( + &inputs, + &outputs, + None, + true, + contribution.is_splice, + contribution.feerate, + ); + Some(FundingContribution { + estimated_fee: estimated_fee_no_change, + outputs: outputs.to_vec(), + inputs, + change_output: None, + input_mode, + ..contribution + }) + }; let new_contribution_at_current_feerate = - adjust_for_inputs_and_outputs(self, inputs, outputs)?; + adjust_for_inputs_and_outputs(self, funding_inputs, outputs)?; let mut new_contribution_at_target_feerate = new_contribution_at_current_feerate .at_feerate(target_feerate, spliceable_balance, true) .ok()?; @@ -812,7 +909,9 @@ impl FundingContribution { target_feerate, ); - if !self.inputs.is_empty() { + if !self.inputs.is_empty() && self.input_mode == Some(FundingInputMode::CoinSelected) { + // Any withdrawal outputs and fees always come from the coin-selected inputs, as we want + // to guarantee the net contribution adds the desired value. let fee_buffer = self .estimated_fee .checked_add( @@ -858,18 +957,22 @@ impl FundingContribution { }) } } else { - // Without coin-selected inputs, both the withdrawals and the fee come from the channel - // balance. - let value_removed: Amount = self.outputs.iter().map(|o| o.value).sum(); - let total_cost = target_fee - .checked_add(value_removed) - .ok_or(FeeRateAdjustmentError::FeeBufferOverflow)?; - if total_cost > spliceable_balance { + // Manually selected inputs may either add value to the channel or offset some of the + // withdrawal outputs. Any remaining fee cost must come from the channel balance. + let net_value_without_fee = self.net_value_without_fee(); + let fee_buffer = if net_value_without_fee.is_negative() { + spliceable_balance + .checked_sub(net_value_without_fee.unsigned_abs()) + .unwrap_or(Amount::ZERO) + } else { + spliceable_balance + .checked_add(net_value_without_fee.unsigned_abs()) + .ok_or(FeeRateAdjustmentError::FeeBufferOverflow)? + }; + if fee_buffer < target_fee { return Err(FeeRateAdjustmentError::FeeBufferInsufficient { - source: "channel balance - withdrawal outputs", - available: spliceable_balance - .checked_sub(value_removed) - .unwrap_or(Amount::ZERO), + source: "channel balance", + available: fee_buffer, required: target_fee, }); } @@ -1016,7 +1119,7 @@ struct FundingBuilderInner { shared_input: Option, min_rbf_feerate: Option, prior_contribution: Option, - value_added: Amount, + funding_inputs: Option, outputs: Vec, feerate: FeeRate, max_feerate: FeeRate, @@ -1025,43 +1128,62 @@ struct FundingBuilderInner { /// 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. +/// The builder tracks either a requested amount to add to the channel or a fixed set of manually +/// selected inputs, 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, by using only manually selected inputs, or by constructing a 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. +/// inputs. Manually selected inputs are not supplemented with coin selection. #[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. +/// if the request cannot be satisfied by reusing a prior contribution, by using only manually +/// selected inputs, 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. +/// used if the request cannot be satisfied by reusing a prior contribution, by using only +/// manually selected inputs, 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 + let request_matches_prior_inputs = + match (self.funding_inputs.as_ref(), prior_contribution.input_mode) { + ( + Some(FundingInputs::ManuallySelected { inputs }), + Some(FundingInputMode::ManuallySelected), + ) => { + let request_inputs = inputs.iter().map(|input| input.utxo.outpoint); + let prior_inputs = + prior_contribution.inputs.iter().map(|input| input.utxo.outpoint); + request_inputs.eq(prior_inputs) + }, + ( + Some(FundingInputs::CoinSelected { value_added }), + Some(FundingInputMode::CoinSelected), + ) => *value_added == prior_contribution.value_added(), + (None, None) => true, + _ => false, + }; + request_matches_prior_inputs && self.outputs == prior_contribution.outputs } fn build_from_prior_contribution( &mut self, contribution: PriorContribution, ) -> Result { let PriorContribution { contribution, spliceable_balance } = contribution; + let input_mode = self.funding_inputs.as_ref().map(FundingInputs::mode); if self.request_matches_prior(&contribution) { // Same request, but the feerate may have changed. Adjust the prior contribution @@ -1072,33 +1194,42 @@ impl FundingBuilderInner { adjusted.max_feerate = self.max_feerate; adjusted }) - .map_err(|_| FundingContributionError::MissingCoinSelectionSource); + .map_err(|_| { + if input_mode == Some(FundingInputMode::ManuallySelected) { + FundingContributionError::ManuallySelectedInputsInsufficient + } else { + 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.funding_inputs.take(), &self.outputs, self.feerate, self.max_feerate, spliceable_balance, ) - .ok_or_else(|| FundingContributionError::MissingCoinSelectionSource); + .ok_or_else(|| { + if input_mode == Some(FundingInputMode::ManuallySelected) { + FundingContributionError::ManuallySelectedInputsInsufficient + } 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. + /// contribution, it also supports manually selected inputs and pure splice-out requests by + /// building a contribution without coin selection. /// /// Returns [`FundingContributionError::MissingCoinSelectionSource`] if the request is - /// otherwise valid but needs wallet inputs. + /// otherwise valid but needs wallet inputs, or + /// [`FundingContributionError::ManuallySelectedInputsInsufficient`] if the manually selected + /// inputs cannot satisfy the request. fn try_build_without_coin_selection( &mut self, ) -> Result { @@ -1106,23 +1237,44 @@ impl FundingBuilderInner { return self.build_from_prior_contribution(contribution); } - if self.value_added == Amount::ZERO { + let value_added = + self.funding_inputs.as_ref().map_or(Amount::ZERO, FundingInputs::value_added); + if value_added == Amount::ZERO { + let inputs = self + .funding_inputs + .as_ref() + .map_or(&[][..], FundingInputs::manually_selected_inputs); + let input_mode = + if inputs.is_empty() { None } else { Some(FundingInputMode::ManuallySelected) }; + + let total_input_value: Amount = + inputs.iter().map(|input| input.utxo.output.value).sum(); let estimated_fee = estimate_transaction_fee( - &[], + inputs, &self.outputs, None, true, self.shared_input.is_some(), self.feerate, ); + if !inputs.is_empty() { + total_input_value + .checked_sub(estimated_fee) + .ok_or(FundingContributionError::ManuallySelectedInputsInsufficient)?; + } + return Ok(FundingContribution { estimated_fee, - inputs: vec![], + inputs: match &mut self.funding_inputs { + Some(FundingInputs::ManuallySelected { inputs }) => core::mem::take(inputs), + None | Some(FundingInputs::CoinSelected { .. }) => Vec::new(), + }, 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(), + input_mode, }); } @@ -1130,7 +1282,7 @@ impl FundingBuilderInner { } fn prepare_coin_selection_request( - &self, + &self, value_added: Amount, ) -> Result<(Vec, Vec), FundingContributionError> { let dummy_pubkey = PublicKey::from_slice(&[2; 33]).unwrap(); let shared_output = bitcoin::TxOut { @@ -1139,7 +1291,7 @@ impl FundingBuilderInner { .as_ref() .map(|shared_input| shared_input.previous_utxo.value) .unwrap_or(Amount::ZERO) - .checked_add(self.value_added) + .checked_add(value_added) .ok_or(FundingContributionError::InvalidSpliceValue)?, script_pubkey: make_funding_redeemscript(&dummy_pubkey, &dummy_pubkey).to_p2wsh(), }; @@ -1171,7 +1323,9 @@ impl FundingBuilderInner { } } - if self.value_added == Amount::ZERO && self.outputs.is_empty() { + if self.funding_inputs.as_ref().map_or(true, FundingInputs::is_empty) + && self.outputs.is_empty() + { return Err(FundingContributionError::InvalidSpliceValue); } @@ -1179,10 +1333,16 @@ impl FundingBuilderInner { // 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 { + if self.funding_inputs.as_ref().map_or(Amount::ZERO, FundingInputs::value_added) + > Amount::MAX_MONEY + { return Err(FundingContributionError::InvalidSpliceValue); } + validate_inputs( + self.funding_inputs.as_ref().map_or(&[][..], FundingInputs::manually_selected_inputs), + )?; + let mut value_removed = Amount::ZERO; for output in self.outputs.iter() { value_removed = match value_removed.checked_add(output.value) { @@ -1198,19 +1358,29 @@ impl FundingBuilderInner { 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() { + let (funding_inputs, outputs) = match prior_contribution.as_ref() { Some(prior) => { - let outputs = prior.contribution.outputs.clone(); - (prior.contribution.value_added(), outputs) + let funding_inputs = match prior.contribution.input_mode { + Some(FundingInputMode::ManuallySelected) => { + Some(FundingInputs::ManuallySelected { + inputs: prior.contribution.inputs.clone(), + }) + }, + Some(FundingInputMode::CoinSelected) => Some(FundingInputs::CoinSelected { + value_added: prior.contribution.value_added(), + }), + None => None, + }; + (funding_inputs, prior.contribution.outputs.clone()) }, - None => (Amount::ZERO, Vec::new()), + None => (None, Vec::new()), }; FundingBuilder(FundingBuilderInner { shared_input, min_rbf_feerate, prior_contribution, - value_added, + funding_inputs, outputs, feerate, max_feerate, @@ -1221,7 +1391,8 @@ impl FundingBuilder { /// 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. + /// reusing a prior contribution, by using only manually selected inputs, or by constructing a + /// pure splice-out directly. pub fn with_coin_selection_source( self, wallet: W, ) -> AsyncFundingBuilder { @@ -1231,13 +1402,58 @@ impl FundingBuilder { /// 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. + /// reusing a prior contribution, by using only manually selected inputs, 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 manually selected input to the request. + /// + /// Each input is fully consumed with no change output. When built without additional coin + /// selection, the inputs and explicit outputs are modeled by their net effect on the channel: + /// the contribution may be net-positive or net-negative before fees. + /// + /// Manually selected inputs are a separate request mode and cannot be combined with requesting + /// additional coin-selected value. If the manually selected inputs cannot satisfy the request, + /// [`FundingBuilder::build`] returns + /// [`FundingContributionError::ManuallySelectedInputsInsufficient`] instead of falling back to + /// coin selection. + /// + /// Returns [`FundingContributionError::InvalidSpliceValue`] if the builder already has a + /// coin-selected value request. + pub fn add_input(self, input: FundingTxInput) -> Result { + self.0.add_input_inner(input).map(FundingBuilder) + } + + /// Adds manually selected inputs to the request. + /// + /// Each input is fully consumed with no change output. When built without additional coin + /// selection, the inputs and explicit outputs are modeled by their net effect on the channel: + /// the contribution may be net-positive or net-negative before fees. + /// + /// Manually selected inputs are a separate request mode and cannot be combined with requesting + /// additional coin-selected value. If the manually selected inputs cannot satisfy the request, + /// [`FundingBuilder::build`] returns + /// [`FundingContributionError::ManuallySelectedInputsInsufficient`] instead of falling back to + /// coin selection. + /// + /// Returns [`FundingContributionError::InvalidSpliceValue`] if the builder already has a + /// coin-selected value request. + pub fn add_inputs(self, inputs: Vec) -> Result { + self.0.add_inputs_inner(inputs).map(FundingBuilder) + } + + /// Removes all manually selected inputs whose outpoint matches `outpoint`. + /// + /// Returns [`FundingContributionError::InvalidSpliceValue`] if the builder already has a + /// coin-selected value request. + pub fn remove_input(self, outpoint: &OutPoint) -> Result { + self.0.remove_input_inner(outpoint).map(FundingBuilder) + } + /// Adds a withdrawal output to the request. /// /// `output` is appended to the current set of explicit outputs. If the builder was seeded from @@ -1267,11 +1483,12 @@ impl FundingBuilder { /// 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. + /// contribution, by using only manually selected inputs, or by building a splice-out + /// contribution that pays fees from the channel balance. /// /// Returns [`FundingContributionError::MissingCoinSelectionSource`] if additional wallet - /// inputs are needed. + /// inputs are needed, or [`FundingContributionError::ManuallySelectedInputsInsufficient`] if + /// the manually selected inputs cannot satisfy the request. pub fn build(mut self) -> Result { self.0.build_without_coin_selection() } @@ -1283,7 +1500,7 @@ impl FundingBuilderInner { shared_input: self.shared_input, min_rbf_feerate: self.min_rbf_feerate, prior_contribution: self.prior_contribution, - value_added: self.value_added, + funding_inputs: self.funding_inputs, outputs: self.outputs, feerate: self.feerate, max_feerate: self.max_feerate, @@ -1291,16 +1508,73 @@ impl FundingBuilderInner { } } - 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 add_value_inner(mut self, value: Amount) -> Result { + match &mut self.funding_inputs { + None => self.funding_inputs = Some(FundingInputs::CoinSelected { value_added: value }), + Some(FundingInputs::CoinSelected { value_added }) => { + *value_added = + Amount::from_sat(value_added.to_sat().saturating_add(value.to_sat())); + }, + Some(FundingInputs::ManuallySelected { .. }) => { + return Err(FundingContributionError::InvalidSpliceValue); + }, + } + Ok(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 remove_value_inner(mut self, value: Amount) -> Result { + match &mut self.funding_inputs { + None => {}, + Some(FundingInputs::CoinSelected { value_added }) => { + *value_added = + Amount::from_sat(value_added.to_sat().saturating_sub(value.to_sat())); + }, + Some(FundingInputs::ManuallySelected { .. }) => { + return Err(FundingContributionError::InvalidSpliceValue); + }, + } + Ok(self) + } + + fn add_input_inner(mut self, input: FundingTxInput) -> Result { + match &mut self.funding_inputs { + None => { + self.funding_inputs = Some(FundingInputs::ManuallySelected { inputs: vec![input] }) + }, + Some(FundingInputs::ManuallySelected { inputs }) => inputs.push(input), + Some(FundingInputs::CoinSelected { .. }) => { + return Err(FundingContributionError::InvalidSpliceValue); + }, + } + Ok(self) + } + + fn add_inputs_inner( + mut self, inputs: Vec, + ) -> Result { + match &mut self.funding_inputs { + None => self.funding_inputs = Some(FundingInputs::ManuallySelected { inputs }), + Some(FundingInputs::ManuallySelected { inputs: existing_inputs }) => { + existing_inputs.extend(inputs) + }, + Some(FundingInputs::CoinSelected { .. }) => { + return Err(FundingContributionError::InvalidSpliceValue); + }, + } + Ok(self) + } + + fn remove_input_inner(mut self, outpoint: &OutPoint) -> Result { + match &mut self.funding_inputs { + None => {}, + Some(FundingInputs::ManuallySelected { inputs }) => { + inputs.retain(|input| input.utxo.outpoint != *outpoint); + }, + Some(FundingInputs::CoinSelected { .. }) => { + return Err(FundingContributionError::InvalidSpliceValue); + }, + } + Ok(self) } fn add_output_inner(mut self, output: TxOut) -> Self { @@ -1322,7 +1596,9 @@ impl FundingBuilderInner { /// inputs. /// /// Returns [`FundingContributionError::MissingCoinSelectionSource`] if the request is valid but - /// cannot be satisfied without wallet inputs. + /// cannot be satisfied without wallet inputs, or + /// [`FundingContributionError::ManuallySelectedInputsInsufficient`] if the manually selected + /// inputs cannot satisfy the request. fn build_without_coin_selection( &mut self, ) -> Result { @@ -1364,8 +1640,11 @@ impl AsyncFundingBuilder { /// 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)) + /// + /// Returns [`FundingContributionError::InvalidSpliceValue`] if the builder already has manually + /// selected inputs. + pub fn add_value(self, value: Amount) -> Result { + self.0.add_value_inner(value).map(AsyncFundingBuilder) } /// Decreases the requested amount to add to the channel. @@ -1375,8 +1654,11 @@ impl AsyncFundingBuilder { /// 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)) + /// + /// Returns [`FundingContributionError::InvalidSpliceValue`] if the builder already has manually + /// selected inputs. + pub fn remove_value(self, value: Amount) -> Result { + self.0.remove_value_inner(value).map(AsyncFundingBuilder) } } @@ -1384,7 +1666,8 @@ 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. + /// a pure splice-out directly, or by using only manually selected inputs, the attached wallet is + /// ignored. pub async fn build(self) -> Result { let mut inner = self.0; match inner.build_without_coin_selection() { @@ -1392,7 +1675,9 @@ impl AsyncFundingBuilder { other => return other, } - let (must_spend, must_pay_to) = inner.prepare_coin_selection_request()?; + let value_added = + inner.funding_inputs.as_ref().map_or(Amount::ZERO, FundingInputs::value_added); + let (must_spend, must_pay_to) = inner.prepare_coin_selection_request(value_added)?; let AsyncCoinSelectionSource(wallet) = inner.state; let coin_selection = wallet .select_confirmed_utxos( @@ -1427,6 +1712,7 @@ impl AsyncFundingBuilder { feerate: inner.feerate, max_feerate: inner.max_feerate, is_splice, + input_mode: Some(FundingInputMode::CoinSelected), }); } } @@ -1464,8 +1750,11 @@ impl SyncFundingBuilder { /// 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)) + /// + /// Returns [`FundingContributionError::InvalidSpliceValue`] if the builder already has manually + /// selected inputs. + pub fn add_value(self, value: Amount) -> Result { + self.0.add_value_inner(value).map(SyncFundingBuilder) } /// Decreases the requested amount to add to the channel. @@ -1475,8 +1764,11 @@ impl SyncFundingBuilder { /// 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)) + /// + /// Returns [`FundingContributionError::InvalidSpliceValue`] if the builder already has manually + /// selected inputs. + pub fn remove_value(self, value: Amount) -> Result { + self.0.remove_value_inner(value).map(SyncFundingBuilder) } } @@ -1484,7 +1776,8 @@ 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. + /// a pure splice-out directly, or by using only manually selected inputs, the attached wallet is + /// ignored. pub fn build(self) -> Result { let mut inner = self.0; match inner.build_without_coin_selection() { @@ -1492,7 +1785,9 @@ impl SyncFundingBuilder { other => return other, } - let (must_spend, must_pay_to) = inner.prepare_coin_selection_request()?; + let value_added = + inner.funding_inputs.as_ref().map_or(Amount::ZERO, FundingInputs::value_added); + let (must_spend, must_pay_to) = inner.prepare_coin_selection_request(value_added)?; let SyncCoinSelectionSource(wallet) = inner.state; let coin_selection = wallet .select_confirmed_utxos( @@ -1526,6 +1821,7 @@ impl SyncFundingBuilder { feerate: inner.feerate, max_feerate: inner.max_feerate, is_splice, + input_mode: Some(FundingInputMode::CoinSelected), }); } } @@ -1534,7 +1830,8 @@ impl SyncFundingBuilder { mod tests { use super::{ estimate_transaction_fee, FeeRateAdjustmentError, FundingBuilder, FundingContribution, - FundingContributionError, FundingTemplate, FundingTxInput, PriorContribution, + FundingContributionError, FundingInputMode, FundingTemplate, FundingTxInput, + PriorContribution, SyncCoinSelectionSource, SyncFundingBuilder, }; use crate::chain::ClaimId; use crate::util::wallet_utils::{CoinSelection, CoinSelectionSourceSync, Input}; @@ -1712,7 +2009,7 @@ mod tests { 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))); + let builder = FundingBuilder(builder.0.add_value_inner(Amount::from_sat(25_000)).unwrap()); assert!(matches!( builder.build(), @@ -1740,6 +2037,7 @@ mod tests { feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let delta = Amount::from_sat(change.value.to_sat() - dust_limit.to_sat() + 1); @@ -1754,9 +2052,10 @@ mod tests { ); let builder = - FundingTemplate::new(None, None, Some(PriorContribution::new(prior, Amount::MAX))) + FundingTemplate::new(None, None, Some(PriorContribution::new(prior, Amount::ZERO))) .with_prior_contribution(feerate, FeeRate::MAX); - let contribution = FundingBuilder(builder.0.add_value_inner(delta)).build().unwrap(); + let contribution = + FundingBuilder(builder.0.add_value_inner(delta).unwrap()).build().unwrap(); assert!(contribution.change_output.is_none()); assert_eq!(contribution.inputs, inputs); @@ -1795,16 +2094,40 @@ mod tests { #[test] fn test_funding_builder_add_and_remove_value_update_request() { let feerate = FeeRate::from_sat_per_kwu(2000); - let builder = + let value_added = Amount::from_sat(15_000); + let input_template = funding_input_sats(1); + let estimated_fee = estimate_transaction_fee( + std::slice::from_ref(&input_template), + &[], + None, + true, + false, + feerate, + ); + let selected_amount = value_added + estimated_fee; + let input = funding_input_sats(selected_amount.to_sat()); + let wallet = MustPayToWallet { + utxo: input.clone(), + change_output: None, + expected_must_pay_to_values: vec![value_added], + }; + + let contribution = FundingBuilder::new(FundingTemplate::new(None, None, None), feerate, FeeRate::MAX) - .with_coin_selection_source_sync(UnreachableWallet) + .with_coin_selection_source_sync(wallet) .add_value(Amount::from_sat(20_000)) + .unwrap() .add_value(Amount::from_sat(5_000)) - .remove_value(Amount::from_sat(10_000)); + .unwrap() + .remove_value(Amount::from_sat(10_000)) + .unwrap() + .build() + .unwrap(); - 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)); + assert_eq!(contribution.inputs, vec![input]); + assert!(contribution.outputs.is_empty()); + assert!(contribution.change_output.is_none()); + assert_eq!(contribution.value_added(), value_added); } #[test] @@ -1836,6 +2159,7 @@ mod tests { FundingBuilder::new(FundingTemplate::new(None, None, None), feerate, FeeRate::MAX) .with_coin_selection_source_sync(wallet) .add_value(value_added) + .unwrap() .add_output(output.clone()) .build() .unwrap(); @@ -1853,7 +2177,9 @@ mod tests { FundingBuilder::new(FundingTemplate::new(None, None, None), feerate, FeeRate::MAX) .with_coin_selection_source_sync(UnreachableWallet) .add_value(Amount::from_sat(10_000)) + .unwrap() .remove_value(Amount::from_sat(15_000)) + .unwrap() .add_output(output.clone()) .build() .unwrap(); @@ -1864,6 +2190,399 @@ mod tests { assert_eq!(contribution.value_added(), Amount::ZERO); } + #[test] + fn test_funding_builder_builds_manual_input_contribution_without_change() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let input = funding_input_sats(100_000); + let output = funding_output_sats(25_000); + + let contribution = FundingTemplate::new(None, None, None) + .without_prior_contribution(feerate, FeeRate::MAX) + .add_input(input.clone()) + .unwrap() + .add_output(output.clone()) + .build() + .unwrap(); + + let expected_fee = estimate_transaction_fee( + std::slice::from_ref(&input), + std::slice::from_ref(&output), + None, + true, + false, + feerate, + ); + assert_eq!(contribution.inputs, vec![input]); + assert_eq!(contribution.outputs, vec![output.clone()]); + assert!(contribution.change_output.is_none()); + assert_eq!(contribution.input_mode, Some(FundingInputMode::ManuallySelected)); + assert_eq!(contribution.estimated_fee, expected_fee); + assert_eq!( + contribution.value_added(), + Amount::from_sat(100_000) - output.value - expected_fee, + ); + assert_eq!( + contribution.net_value(), + Amount::from_sat(100_000).to_signed().unwrap() + - output.value.to_signed().unwrap() + - expected_fee.to_signed().unwrap(), + ); + } + + #[test] + fn test_funding_builder_add_inputs_builds_manual_input_contribution() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let first_input = funding_input_sats(40_000); + let second_input = funding_input_sats(60_000); + let output = funding_output_sats(25_000); + + let contribution = FundingTemplate::new(None, None, None) + .without_prior_contribution(feerate, FeeRate::MAX) + .add_inputs(vec![first_input.clone(), second_input.clone()]) + .unwrap() + .add_output(output.clone()) + .build() + .unwrap(); + + let expected_fee = estimate_transaction_fee( + &[first_input.clone(), second_input.clone()], + std::slice::from_ref(&output), + None, + true, + false, + feerate, + ); + assert_eq!(contribution.inputs, vec![first_input, second_input]); + assert_eq!(contribution.outputs, vec![output.clone()]); + assert!(contribution.change_output.is_none()); + assert_eq!(contribution.input_mode, Some(FundingInputMode::ManuallySelected)); + assert_eq!(contribution.estimated_fee, expected_fee); + assert_eq!( + contribution.value_added(), + Amount::from_sat(100_000) - output.value - expected_fee, + ); + } + + #[test] + fn test_funding_builder_remove_input_updates_manual_input_request() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let first_input = funding_input_sats(40_000); + let second_input = funding_input_sats(60_000); + let output = funding_output_sats(25_000); + + let contribution = FundingTemplate::new(None, None, None) + .without_prior_contribution(feerate, FeeRate::MAX) + .add_inputs(vec![first_input.clone(), second_input.clone()]) + .unwrap() + .remove_input(&first_input.utxo.outpoint) + .unwrap() + .add_output(output.clone()) + .build() + .unwrap(); + + let expected_fee = estimate_transaction_fee( + std::slice::from_ref(&second_input), + std::slice::from_ref(&output), + None, + true, + false, + feerate, + ); + assert_eq!(contribution.inputs, vec![second_input]); + assert_eq!(contribution.outputs, vec![output.clone()]); + assert_eq!(contribution.input_mode, Some(FundingInputMode::ManuallySelected)); + assert_eq!( + contribution.value_added(), + Amount::from_sat(60_000) - output.value - expected_fee, + ); + } + + #[test] + fn test_splice_in_inputs_builds_manual_input_contribution() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let first_input = funding_input_sats(40_000); + let second_input = funding_input_sats(60_000); + + let contribution = FundingTemplate::new(None, None, None) + .splice_in_inputs( + vec![first_input.clone(), second_input.clone()], + feerate, + FeeRate::MAX, + ) + .unwrap(); + + let expected_fee = estimate_transaction_fee( + &[first_input.clone(), second_input.clone()], + &[], + None, + true, + false, + feerate, + ); + assert_eq!(contribution.inputs, vec![first_input, second_input]); + assert!(contribution.outputs.is_empty()); + assert!(contribution.change_output.is_none()); + assert_eq!(contribution.input_mode, Some(FundingInputMode::ManuallySelected)); + assert_eq!(contribution.value_added(), Amount::from_sat(100_000) - expected_fee); + } + + #[test] + fn test_splice_in_inputs_appends_to_prior_manual_inputs() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let prior_input = funding_input_sats(40_000); + let additional_input = funding_input_sats(60_000); + let prior_fee = estimate_transaction_fee( + std::slice::from_ref(&prior_input), + &[], + None, + true, + false, + feerate, + ); + let prior = FundingContribution { + estimated_fee: prior_fee, + inputs: vec![prior_input.clone()], + outputs: vec![], + change_output: None, + feerate, + max_feerate: FeeRate::MAX, + is_splice: false, + input_mode: Some(FundingInputMode::ManuallySelected), + }; + + let contribution = FundingTemplate::new( + None, + None, + Some(PriorContribution::new(prior, Amount::MAX_MONEY)), + ) + .splice_in_inputs(vec![additional_input.clone()], feerate, FeeRate::MAX) + .unwrap(); + + assert_eq!(contribution.inputs, vec![prior_input, additional_input]); + assert!(contribution.outputs.is_empty()); + assert_eq!(contribution.input_mode, Some(FundingInputMode::ManuallySelected)); + } + + #[test] + fn test_sync_funding_builder_manual_inputs_insufficient_do_not_fallback_to_coin_selection() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let builder = FundingTemplate::new(None, None, None) + .without_prior_contribution(feerate, FeeRate::MAX) + .add_input(funding_input_sats(1)) + .unwrap(); + let builder = + SyncFundingBuilder(builder.0.with_state(SyncCoinSelectionSource(UnreachableWallet))); + + assert!(matches!( + builder.build(), + Err(FundingContributionError::ManuallySelectedInputsInsufficient), + )); + } + + #[test] + fn test_funding_builder_rejects_manual_inputs_with_value_request() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let builder = FundingTemplate::new(None, None, None) + .without_prior_contribution(feerate, FeeRate::MAX) + .add_input(funding_input_sats(100_000)) + .unwrap(); + let result = builder.clone().0.add_value_inner(Amount::from_sat(1_000)); + assert!(matches!(result, Err(FundingContributionError::InvalidSpliceValue),)); + + let builder = + SyncFundingBuilder(builder.0.with_state(SyncCoinSelectionSource(UnreachableWallet))); + let result = builder.remove_value(Amount::from_sat(1_000)); + assert!(matches!(result, Err(FundingContributionError::InvalidSpliceValue),)); + } + + #[test] + fn test_funding_builder_rejects_manual_inputs_on_coin_selected_prior() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let prior_input = funding_input_sats(100_000); + let prior_outpoint = prior_input.utxo.outpoint; + let prior = FundingContribution { + estimated_fee: Amount::from_sat(1_000), + inputs: vec![prior_input], + outputs: vec![], + change_output: Some(funding_output_sats(10_000)), + feerate, + max_feerate: FeeRate::MAX, + is_splice: false, + input_mode: Some(FundingInputMode::CoinSelected), + }; + + let builder = + FundingTemplate::new(None, None, Some(PriorContribution::new(prior, Amount::ZERO))) + .with_prior_contribution(feerate, FeeRate::MAX); + + assert!(matches!( + builder.clone().add_input(funding_input_sats(50_000)), + Err(FundingContributionError::InvalidSpliceValue), + )); + assert!(matches!( + builder.remove_input(&prior_outpoint), + Err(FundingContributionError::InvalidSpliceValue), + )); + } + + #[test] + fn test_funding_builder_validates_manual_input_max_money() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let inputs = vec![funding_input_sats(Amount::MAX_MONEY.to_sat()), funding_input_sats(1)]; + + let builder = FundingTemplate::new(None, None, None) + .without_prior_contribution(feerate, FeeRate::MAX) + .add_inputs(inputs) + .unwrap(); + + assert!(matches!(builder.build(), Err(FundingContributionError::InvalidSpliceValue),)); + } + + #[test] + fn test_build_from_prior_manual_inputs_exact_match_reuses_and_adjusts() { + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(3000); + let input = funding_input_sats(100_000); + let output = funding_output_sats(20_000); + let estimated_fee = estimate_transaction_fee( + std::slice::from_ref(&input), + std::slice::from_ref(&output), + None, + true, + false, + original_feerate, + ); + let prior = FundingContribution { + estimated_fee, + inputs: vec![input.clone()], + outputs: vec![output.clone()], + change_output: None, + feerate: original_feerate, + max_feerate: FeeRate::MAX, + is_splice: false, + input_mode: Some(FundingInputMode::ManuallySelected), + }; + + let contribution = FundingTemplate::new( + None, + None, + Some(PriorContribution::new(prior, Amount::MAX_MONEY)), + ) + .with_prior_contribution(target_feerate, FeeRate::MAX) + .build() + .unwrap(); + + assert_eq!(contribution.inputs, vec![input]); + assert_eq!(contribution.outputs, vec![output]); + assert_eq!(contribution.feerate, target_feerate); + assert_eq!(contribution.input_mode, Some(FundingInputMode::ManuallySelected)); + } + + #[test] + fn test_build_from_prior_manual_inputs_changed_request_insufficient_maps_error() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let input = funding_input_sats(50_000); + let estimated_fee = + estimate_transaction_fee(std::slice::from_ref(&input), &[], None, true, false, feerate); + let prior = FundingContribution { + estimated_fee, + inputs: vec![input], + outputs: vec![], + change_output: None, + feerate, + max_feerate: FeeRate::MAX, + is_splice: false, + input_mode: Some(FundingInputMode::ManuallySelected), + }; + + let result = + FundingTemplate::new(None, None, Some(PriorContribution::new(prior, Amount::ZERO))) + .with_prior_contribution(feerate, FeeRate::MAX) + .add_output(funding_output_sats(60_000)) + .build(); + + assert!(matches!( + result, + Err(FundingContributionError::ManuallySelectedInputsInsufficient), + )); + } + + #[test] + fn test_for_acceptor_at_feerate_manual_inputs_balance_insufficient() { + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(100_000); + let inputs = vec![funding_input_sats(100_000)]; + let outputs = vec![funding_output_sats(80_000)]; + let net_value_without_fee = Amount::from_sat(20_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); + assert!(target_fee > net_value_without_fee); + + let contribution = FundingContribution { + estimated_fee, + inputs, + outputs, + change_output: None, + feerate: original_feerate, + max_feerate: FeeRate::MAX, + is_splice: true, + input_mode: Some(FundingInputMode::ManuallySelected), + }; + + let holder_balance = target_fee + .checked_sub(net_value_without_fee) + .and_then(|shortfall| shortfall.checked_sub(Amount::from_sat(1))) + .unwrap(); + 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_manual_inputs_balance_sufficient() { + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(100_000); + let inputs = vec![funding_input_sats(100_000)]; + let outputs = vec![funding_output_sats(80_000)]; + let net_value_without_fee = Amount::from_sat(20_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, + input_mode: Some(FundingInputMode::ManuallySelected), + }; + + let holder_balance = target_fee.checked_sub(net_value_without_fee).unwrap(); + let adjusted = + contribution.for_acceptor_at_feerate(target_feerate, holder_balance).unwrap(); + + assert_eq!(adjusted.inputs, inputs); + assert_eq!(adjusted.outputs, outputs); + assert_eq!(adjusted.estimated_fee, target_fee); + assert_eq!( + adjusted.net_value(), + net_value_without_fee.to_signed().unwrap() - target_fee.to_signed().unwrap(), + ); + } + #[test] fn test_build_funding_contribution_validates_max_money() { let over_max = Amount::MAX_MONEY + Amount::from_sat(1); @@ -1914,6 +2633,7 @@ mod tests { .without_prior_contribution(feerate, feerate) .with_coin_selection_source_sync(UnreachableWallet) .add_value(over_max) + .unwrap() .add_outputs(vec![funding_output_sats(1_000)]) .build(), Err(FundingContributionError::InvalidSpliceValue), @@ -1926,6 +2646,7 @@ mod tests { .without_prior_contribution(feerate, feerate) .with_coin_selection_source_sync(UnreachableWallet) .add_value(Amount::from_sat(1_000)) + .unwrap() .add_outputs(vec![ funding_output_sats(half_over.to_sat()), funding_output_sats(half_over.to_sat()), @@ -1986,6 +2707,7 @@ mod tests { .with_prior_contribution(feerate, feerate) .with_coin_selection_source_sync(wallet) .add_value(Amount::from_sat(10_000)) + .unwrap() .build(), Err(FundingContributionError::PrevTxTooLarge), )); @@ -2014,6 +2736,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let net_value_before = contribution.net_value(); @@ -2051,6 +2774,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); @@ -2091,6 +2815,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let net_value_before = contribution.net_value(); @@ -2126,6 +2851,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); @@ -2151,6 +2877,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let contribution = @@ -2180,6 +2907,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; // Balance of 55,000 sats can't cover outputs (50,000) + target_fee at 50k sat/kwu. @@ -2209,6 +2937,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; // For splice-in with change that stays above dust, the surplus is absorbed by the change @@ -2241,6 +2970,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let net_at_feerate = @@ -2276,6 +3006,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let net_before = contribution.net_value(); @@ -2309,6 +3040,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let result = contribution.net_value_for_acceptor_at_feerate(target_feerate, Amount::MAX); @@ -2336,6 +3068,7 @@ mod tests { feerate: original_feerate, max_feerate, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); @@ -2367,6 +3100,7 @@ mod tests { feerate: original_feerate, max_feerate, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); @@ -2401,6 +3135,7 @@ mod tests { feerate: original_feerate, max_feerate, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); @@ -2443,6 +3178,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); @@ -2475,6 +3211,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); @@ -2513,6 +3250,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); @@ -2549,6 +3287,7 @@ mod tests { feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; // target == min feerate, so FeeRateTooLow check passes. @@ -2576,6 +3315,7 @@ mod tests { feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let result = contribution.for_acceptor_at_feerate(feerate, Amount::MAX); @@ -2600,6 +3340,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; // Balance of 40,000 sats is less than outputs (50,000) + target_fee. @@ -2626,6 +3367,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; // Balance of 100,000 sats is more than outputs (50,000) + target_fee. @@ -2656,6 +3398,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; // Balance of 40,000 sats is less than outputs (50,000) + target_fee. @@ -2685,6 +3428,7 @@ mod tests { feerate: original_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let acceptor = @@ -2719,6 +3463,7 @@ mod tests { feerate: prior_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; // max_feerate (2020) < min_rbf_feerate (2025). @@ -2755,6 +3500,7 @@ mod tests { feerate: prior_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let template = FundingTemplate::new( @@ -2788,6 +3534,7 @@ mod tests { feerate: prior_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let template = FundingTemplate::new( @@ -2816,6 +3563,7 @@ mod tests { feerate: prior_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let template = FundingTemplate::new( @@ -2848,6 +3596,7 @@ mod tests { feerate: prior_feerate, max_feerate: FeeRate::MAX, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let template = FundingTemplate::new( @@ -2914,6 +3663,7 @@ mod tests { feerate: prior_feerate, max_feerate: prior_feerate, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let template = FundingTemplate::new( @@ -2955,6 +3705,7 @@ mod tests { feerate: FeeRate::from_sat_per_kwu(2000), max_feerate: prior_max_feerate, is_splice: true, + input_mode: Some(FundingInputMode::CoinSelected), }; let template = FundingTemplate::new( diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index a5361358653..a50a216ad8b 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -41,6 +41,7 @@ use bitcoin::hashes::Hash; use bitcoin::secp256k1::ecdsa::Signature; use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; use bitcoin::transaction::Version; +use bitcoin::SignedAmount; use bitcoin::{ Amount, FeeRate, OutPoint as BitcoinOutPoint, Psbt, ScriptBuf, Transaction, TxOut, Txid, WPubkeyHash, WScriptHash, @@ -226,6 +227,7 @@ pub fn do_initiate_rbf_splice_in_and_out<'a, 'b, 'c, 'd>( .without_prior_contribution(feerate, FeeRate::MAX) .with_coin_selection_source_sync(&wallet) .add_value(value_added) + .unwrap() .add_outputs(outputs) .build() .unwrap(); @@ -278,6 +280,7 @@ pub fn do_initiate_splice_in_and_out<'a, 'b, 'c, 'd>( .without_prior_contribution(feerate, FeeRate::MAX) .with_coin_selection_source_sync(&wallet) .add_value(value_added) + .unwrap() .add_outputs(outputs) .build() .unwrap(); @@ -3816,6 +3819,7 @@ fn test_funding_contributed_splice_already_pending() { .with_prior_contribution(feerate, FeeRate::MAX) .with_coin_selection_source_sync(&wallet) .add_value(splice_in_amount) + .unwrap() .add_output(first_splice_out.clone()) .build() .unwrap(); @@ -3842,6 +3846,7 @@ fn test_funding_contributed_splice_already_pending() { .without_prior_contribution(feerate, FeeRate::MAX) .with_coin_selection_source_sync(&wallet) .add_value(splice_in_amount) + .unwrap() .add_output(second_splice_out.clone()) .build() .unwrap(); @@ -6152,6 +6157,7 @@ fn test_splice_rbf_amends_prior_net_positive_contribution_request() { .with_prior_contribution(rbf_feerate, FeeRate::MAX) .with_coin_selection_source_sync(&wallet) .remove_value(half_added_value) + .unwrap() .build() .unwrap(); let (inputs_2, _) = contribution_2.clone().into_contributed_inputs_and_outputs(); @@ -6235,6 +6241,11 @@ fn test_splice_rbf_amends_prior_net_negative_contribution_request() { assert!(initial_inputs.is_empty()); let (splice_tx_0, new_funding_script) = splice_channel(&nodes[0], &nodes[1], channel_id, initial_contribution.clone()); + let manual_input_pair_tx = provide_utxo_reserves(&nodes, 2, Amount::from_sat(20_000)); + let manual_input_single_tx = provide_utxo_reserves(&nodes, 1, Amount::from_sat(10_000)); + let manual_input_0 = ConfirmedUtxo::new_p2wpkh(manual_input_pair_tx.clone(), 0).unwrap(); + let manual_input_1 = ConfirmedUtxo::new_p2wpkh(manual_input_pair_tx, 1).unwrap(); + let manual_input_2 = ConfirmedUtxo::new_p2wpkh(manual_input_single_tx, 0).unwrap(); let run_rbf_round = |contribution: FundingContribution| { nodes[0] @@ -6287,21 +6298,72 @@ fn test_splice_rbf_amends_prior_net_negative_contribution_request() { 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 contribution_3 = - funding_template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet).unwrap(); + let rbf_feerate = funding_template.min_rbf_feerate().unwrap(); + let contribution_3 = funding_template + .with_prior_contribution(rbf_feerate, FeeRate::MAX) + .add_inputs(vec![manual_input_0.clone(), manual_input_1.clone()]) + .unwrap() + .build() + .unwrap(); let (inputs_3, _) = contribution_3.clone().into_contributed_inputs_and_outputs(); - assert!(inputs_3.is_empty()); + assert_eq!(inputs_3, vec![manual_input_0.utxo.outpoint, manual_input_1.utxo.outpoint],); assert_eq!(contribution_3.outputs(), contribution_2.outputs()); - assert!(contribution_3.net_value() < contribution_2.net_value()); + assert!(contribution_3.net_value() > SignedAmount::ZERO); assert!(contribution_3.change_output().is_none()); - let rbf_tx_final = run_rbf_round(contribution_3); + 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 prior_inputs = funding_template + .prior_contribution() + .unwrap() + .clone() + .into_contributed_inputs_and_outputs() + .0; + assert_eq!(prior_inputs, vec![manual_input_0.utxo.outpoint, manual_input_1.utxo.outpoint],); + let rbf_feerate = funding_template.min_rbf_feerate().unwrap(); + let contribution_4 = funding_template + .with_prior_contribution(rbf_feerate, FeeRate::MAX) + .add_input(manual_input_2.clone()) + .unwrap() + .remove_input(&manual_input_0.utxo.outpoint) + .unwrap() + .remove_input(&manual_input_1.utxo.outpoint) + .unwrap() + .build() + .unwrap(); + let (inputs_4, _) = contribution_4.clone().into_contributed_inputs_and_outputs(); + assert_eq!(inputs_4, vec![manual_input_2.utxo.outpoint]); + assert_eq!(contribution_4.outputs(), contribution_3.outputs()); + assert!(contribution_4.net_value() < SignedAmount::ZERO); + assert!(contribution_4.net_value() < contribution_3.net_value()); + assert!(contribution_4.change_output().is_none()); + let splice_tx_4 = run_rbf_round(contribution_4.clone()); + + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1).unwrap(); + assert_eq!(funding_template.prior_contribution().unwrap().outputs(), contribution_4.outputs()); + let contribution_5 = + funding_template.rbf_prior_contribution_sync(None, FeeRate::MAX, &wallet).unwrap(); + let (inputs_5, _) = contribution_5.clone().into_contributed_inputs_and_outputs(); + assert_eq!(inputs_5, vec![manual_input_2.utxo.outpoint]); + assert_eq!(contribution_5.outputs(), contribution_4.outputs()); + assert!(contribution_5.net_value() < SignedAmount::ZERO); + assert!(contribution_5.net_value() < contribution_4.net_value()); + assert!(contribution_5.change_output().is_none()); + let rbf_tx_final = run_rbf_round(contribution_5); 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_0.compute_txid(), + splice_tx_1.compute_txid(), + splice_tx_2.compute_txid(), + splice_tx_3.compute_txid(), + splice_tx_4.compute_txid(), + ], ); } @@ -7273,6 +7335,7 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { .without_prior_contribution(rbf_feerate, FeeRate::MAX) .with_coin_selection_source_sync(&wallet) .add_value(added_value) + .unwrap() .build() .unwrap(); let result = nodes[0].node.funding_contributed(&channel_id, &node_id_1, contribution, None); From cc0f81ba2ef952706282ac06af0e2639be5c4edc Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Tue, 28 Apr 2026 14:25:49 -0700 Subject: [PATCH 2/3] Include spliceable balance in every FundingTemplate There's no reason not to do so, and it allows us to fail earlier when the user's net contribution exceeds their spliceable balance. --- lightning/src/ln/channel.rs | 30 ++- lightning/src/ln/funding.rs | 415 +++++++++++++++-------------- lightning/src/ln/splicing_tests.rs | 44 ++- 3 files changed, 257 insertions(+), 232 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e07ee7fceab..96dbdfebdde 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -56,7 +56,7 @@ use crate::ln::channelmanager::{ MAX_LOCAL_BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, }; use crate::ln::funding::{ - FeeRateAdjustmentError, FundingContribution, FundingTemplate, FundingTxInput, PriorContribution, + FeeRateAdjustmentError, FundingContribution, FundingTemplate, FundingTxInput, }; use crate::ln::interactivetxs::{ AbortReason, HandleTxCompleteValue, InteractiveTxConstructor, InteractiveTxConstructorArgs, @@ -12383,6 +12383,16 @@ where }); } + let spliceable_balance = self.get_next_splice_out_maximum(&self.funding).map_err(|e| { + APIError::ChannelUnavailable { + err: format!( + "Channel {} cannot be spliced at this time: {}", + self.context.channel_id(), + e + ), + } + })?; + let (min_rbf_feerate, prior_contribution) = if self.is_rbf_compatible().is_err() { // Channel can never RBF (e.g., zero-conf). (None, None) @@ -12415,16 +12425,7 @@ where .as_ref() .and_then(|pending_splice| pending_splice.contributions.last()) { - let spliceable_balance = self - .get_next_splice_out_maximum(&self.funding) - .map_err(|e| APIError::ChannelUnavailable { - err: format!( - "Channel {} cannot be spliced at this time: {}", - self.context.channel_id(), - e - ), - })?; - Some(PriorContribution::new(prior.clone(), spliceable_balance)) + Some(prior.clone()) } else { None } @@ -12446,7 +12447,12 @@ where satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + FUNDING_TRANSACTION_WITNESS_WEIGHT, }; - Ok(FundingTemplate::new(Some(shared_input), min_rbf_feerate, prior_contribution)) + Ok(FundingTemplate::new( + Some(shared_input), + min_rbf_feerate, + prior_contribution, + spliceable_balance, + )) } /// Returns whether this channel can ever RBF, independent of splice state. diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index dd9611e830c..d707428ecb2 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -132,7 +132,8 @@ pub enum FundingContributionError { /// The minimum RBF feerate. min_rbf_feerate: FeeRate, }, - /// The splice value is invalid (zero, empty outputs, or exceeds the maximum money supply). + /// The splice value is invalid (zero, empty outputs, exceeds the maximum money supply, or + /// splices out more than the available channel balance). InvalidSpliceValue, /// An input's `prevtx` is too large to fit in a `tx_add_input` message. PrevTxTooLarge, @@ -163,7 +164,7 @@ impl core::fmt::Display for FundingContributionError { write!(f, "Feerate {} is below minimum RBF feerate {}", feerate, min_rbf_feerate) }, FundingContributionError::InvalidSpliceValue => { - write!(f, "Invalid splice value (zero, empty, or exceeds limit)") + write!(f, "Invalid splice value (zero, empty, exceeds limit, or overdraws balance)") }, FundingContributionError::PrevTxTooLarge => { write!(f, "Input prevtx is too large to fit in a tx_add_input message") @@ -184,39 +185,6 @@ 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. 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 reuse it or replace it with -/// [`FundingTemplate::without_prior_contribution`]. -#[derive(Debug, Clone, PartialEq, Eq)] -pub(super) struct PriorContribution { - contribution: FundingContribution, - /// The holder's spliceable 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 - /// this is only used as an optimization to determine if the prior contribution can be - /// reused with adjusted fees — the contribution is re-validated at - /// [`ChannelManager::funding_contributed`] time and again at quiescence time against the - /// current balances. - /// - /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel - /// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed - spliceable_balance: Amount, -} - -impl PriorContribution { - pub(super) fn new(contribution: FundingContribution, spliceable_balance: Amount) -> Self { - Self { contribution, spliceable_balance } - } -} - /// A template for contributing to a channel's splice funding transaction. /// /// This is returned from [`ChannelManager::splice_channel`] when a channel is ready to be @@ -260,17 +228,30 @@ pub struct FundingTemplate { /// pending splice candidates. min_rbf_feerate: Option, - /// The user's prior contribution from a previous splice negotiation, if available. - prior_contribution: Option, + /// The user's prior contribution from a previous splice negotiation on this channel. + prior_contribution: Option, + + /// The portion of the user's balance that can be spliced out. + /// + /// 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 + /// this is only used as an optimization to determine if the prior contribution can be + /// reused with adjusted fees — the contribution is re-validated at + /// [`ChannelManager::funding_contributed`] time and again at quiescence time against the + /// current balances. + /// + /// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel + /// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed + spliceable_balance: Amount, } impl FundingTemplate { /// Constructs a [`FundingTemplate`] for a splice using the provided shared input. pub(super) fn new( shared_input: Option, min_rbf_feerate: Option, - prior_contribution: Option, + prior_contribution: Option, spliceable_balance: Amount, ) -> Self { - Self { shared_input, min_rbf_feerate, prior_contribution } + Self { shared_input, min_rbf_feerate, prior_contribution, spliceable_balance } } /// Returns the minimum RBF feerate, if this template is for an RBF attempt. @@ -296,7 +277,7 @@ impl FundingTemplate { /// 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) + self.prior_contribution.as_ref() } /// Creates a [`FundingBuilder`] for constructing a contribution. @@ -1118,7 +1099,8 @@ struct SyncCoinSelectionSource(W); struct FundingBuilderInner { shared_input: Option, min_rbf_feerate: Option, - prior_contribution: Option, + prior_contribution: Option, + spliceable_balance: Amount, funding_inputs: Option, outputs: Vec, feerate: FeeRate, @@ -1180,16 +1162,15 @@ impl FundingBuilderInner { } fn build_from_prior_contribution( - &mut self, contribution: PriorContribution, + &mut self, contribution: FundingContribution, ) -> Result { - let PriorContribution { contribution, spliceable_balance } = contribution; let input_mode = self.funding_inputs.as_ref().map(FundingInputs::mode); 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, spliceable_balance) + .for_initiator_at_feerate(self.feerate, self.spliceable_balance) .map(|mut adjusted| { adjusted.max_feerate = self.max_feerate; adjusted @@ -1209,7 +1190,7 @@ impl FundingBuilderInner { &self.outputs, self.feerate, self.max_feerate, - spliceable_balance, + self.spliceable_balance, ) .ok_or_else(|| { if input_mode == Some(FundingInputMode::ManuallySelected) { @@ -1247,8 +1228,6 @@ impl FundingBuilderInner { let input_mode = if inputs.is_empty() { None } else { Some(FundingInputMode::ManuallySelected) }; - let total_input_value: Amount = - inputs.iter().map(|input| input.utxo.output.value).sum(); let estimated_fee = estimate_transaction_fee( inputs, &self.outputs, @@ -1257,13 +1236,8 @@ impl FundingBuilderInner { self.shared_input.is_some(), self.feerate, ); - if !inputs.is_empty() { - total_input_value - .checked_sub(estimated_fee) - .ok_or(FundingContributionError::ManuallySelectedInputsInsufficient)?; - } - return Ok(FundingContribution { + let contribution = FundingContribution { estimated_fee, inputs: match &mut self.funding_inputs { Some(FundingInputs::ManuallySelected { inputs }) => core::mem::take(inputs), @@ -1275,7 +1249,19 @@ impl FundingBuilderInner { max_feerate: self.max_feerate, is_splice: self.shared_input.is_some(), input_mode, - }); + }; + let net_value = contribution.net_value(); + if net_value.is_negative() { + self.spliceable_balance.checked_sub(net_value.unsigned_abs()).ok_or_else(|| { + if contribution.inputs.is_empty() { + FundingContributionError::InvalidSpliceValue + } else { + FundingContributionError::ManuallySelectedInputsInsufficient + } + })?; + } + + return Ok(contribution); } Err(FundingContributionError::MissingCoinSelectionSource) @@ -1357,21 +1343,26 @@ impl FundingBuilderInner { impl FundingBuilder { fn new(template: FundingTemplate, feerate: FeeRate, max_feerate: FeeRate) -> FundingBuilder { - let FundingTemplate { shared_input, min_rbf_feerate, prior_contribution } = template; + let FundingTemplate { + shared_input, + min_rbf_feerate, + prior_contribution, + spliceable_balance, + } = template; let (funding_inputs, outputs) = match prior_contribution.as_ref() { - Some(prior) => { - let funding_inputs = match prior.contribution.input_mode { + Some(prior_contribution) => { + let funding_inputs = match prior_contribution.input_mode { Some(FundingInputMode::ManuallySelected) => { Some(FundingInputs::ManuallySelected { - inputs: prior.contribution.inputs.clone(), + inputs: prior_contribution.inputs.clone(), }) }, Some(FundingInputMode::CoinSelected) => Some(FundingInputs::CoinSelected { - value_added: prior.contribution.value_added(), + value_added: prior_contribution.value_added(), }), None => None, }; - (funding_inputs, prior.contribution.outputs.clone()) + (funding_inputs, prior_contribution.outputs.clone()) }, None => (None, Vec::new()), }; @@ -1380,6 +1371,7 @@ impl FundingBuilder { shared_input, min_rbf_feerate, prior_contribution, + spliceable_balance, funding_inputs, outputs, feerate, @@ -1500,6 +1492,7 @@ impl FundingBuilderInner { shared_input: self.shared_input, min_rbf_feerate: self.min_rbf_feerate, prior_contribution: self.prior_contribution, + spliceable_balance: self.spliceable_balance, funding_inputs: self.funding_inputs, outputs: self.outputs, feerate: self.feerate, @@ -1831,7 +1824,7 @@ mod tests { use super::{ estimate_transaction_fee, FeeRateAdjustmentError, FundingBuilder, FundingContribution, FundingContributionError, FundingInputMode, FundingTemplate, FundingTxInput, - PriorContribution, SyncCoinSelectionSource, SyncFundingBuilder, + SyncCoinSelectionSource, SyncFundingBuilder, }; use crate::chain::ClaimId; use crate::util::wallet_utils::{CoinSelection, CoinSelectionSourceSync, Input}; @@ -1980,11 +1973,14 @@ mod tests { let feerate = FeeRate::from_sat_per_kwu(2000); let output = funding_output_sats(25_000); - let contribution = - FundingBuilder::new(FundingTemplate::new(None, None, None), feerate, FeeRate::MAX) - .add_output(output.clone()) - .build() - .unwrap(); + let contribution = FundingBuilder::new( + FundingTemplate::new(None, None, None, Amount::MAX_MONEY), + feerate, + FeeRate::MAX, + ) + .add_output(output.clone()) + .build() + .unwrap(); let expected_fee = estimate_transaction_fee( &[], @@ -2004,11 +2000,38 @@ mod tests { ); } + #[test] + fn test_funding_builder_rejects_splice_out_over_balance() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let output = funding_output_sats(25_000); + let expected_fee = estimate_transaction_fee( + &[], + std::slice::from_ref(&output), + None, + true, + false, + feerate, + ); + let exact_balance = output.value + expected_fee; + + let contribution = FundingTemplate::new(None, None, None, exact_balance) + .splice_out(vec![output.clone()], feerate, FeeRate::MAX) + .unwrap(); + assert_eq!(contribution.net_value(), -exact_balance.to_signed().unwrap()); + + let result = FundingTemplate::new(None, None, None, exact_balance - Amount::from_sat(1)) + .splice_out(vec![output], feerate, FeeRate::MAX); + assert!(matches!(result, Err(FundingContributionError::InvalidSpliceValue))); + } + #[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::new( + FundingTemplate::new(None, None, None, Amount::ZERO), + feerate, + FeeRate::MAX, + ); let builder = FundingBuilder(builder.0.add_value_inner(Amount::from_sat(25_000)).unwrap()); assert!(matches!( @@ -2051,9 +2074,8 @@ mod tests { total_input_value >= target_value_added.checked_add(estimated_fee_no_change).unwrap() ); - let builder = - FundingTemplate::new(None, None, Some(PriorContribution::new(prior, Amount::ZERO))) - .with_prior_contribution(feerate, FeeRate::MAX); + let builder = FundingTemplate::new(None, None, Some(prior), Amount::MAX_MONEY) + .with_prior_contribution(feerate, FeeRate::MAX); let contribution = FundingBuilder(builder.0.add_value_inner(delta).unwrap()).build().unwrap(); @@ -2079,14 +2101,17 @@ mod tests { 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 }; - 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(); + let contribution = FundingBuilder::new( + FundingTemplate::new(None, None, None, Amount::MAX_MONEY), + feerate, + FeeRate::MAX, + ) + .add_output(removed_output_1) + .add_output(kept_output.clone()) + .add_output(removed_output_2) + .remove_outputs(&removed_script) + .build() + .unwrap(); assert_eq!(contribution.outputs, vec![kept_output]); } @@ -2112,17 +2137,20 @@ mod tests { expected_must_pay_to_values: vec![value_added], }; - let contribution = - FundingBuilder::new(FundingTemplate::new(None, None, None), feerate, FeeRate::MAX) - .with_coin_selection_source_sync(wallet) - .add_value(Amount::from_sat(20_000)) - .unwrap() - .add_value(Amount::from_sat(5_000)) - .unwrap() - .remove_value(Amount::from_sat(10_000)) - .unwrap() - .build() - .unwrap(); + let contribution = FundingBuilder::new( + FundingTemplate::new(None, None, None, Amount::ZERO), + feerate, + FeeRate::MAX, + ) + .with_coin_selection_source_sync(wallet) + .add_value(Amount::from_sat(20_000)) + .unwrap() + .add_value(Amount::from_sat(5_000)) + .unwrap() + .remove_value(Amount::from_sat(10_000)) + .unwrap() + .build() + .unwrap(); assert_eq!(contribution.inputs, vec![input]); assert!(contribution.outputs.is_empty()); @@ -2155,14 +2183,17 @@ mod tests { expected_must_pay_to_values: vec![output.value, value_added], }; - let contribution = - FundingBuilder::new(FundingTemplate::new(None, None, None), feerate, FeeRate::MAX) - .with_coin_selection_source_sync(wallet) - .add_value(value_added) - .unwrap() - .add_output(output.clone()) - .build() - .unwrap(); + let contribution = FundingBuilder::new( + FundingTemplate::new(None, None, None, Amount::MAX_MONEY), + feerate, + FeeRate::MAX, + ) + .with_coin_selection_source_sync(wallet) + .add_value(value_added) + .unwrap() + .add_output(output.clone()) + .build() + .unwrap(); assert_eq!(contribution.value_added(), value_added); assert_eq!(contribution.outputs, vec![output]); @@ -2173,16 +2204,19 @@ mod tests { fn test_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)) - .unwrap() - .remove_value(Amount::from_sat(15_000)) - .unwrap() - .add_output(output.clone()) - .build() - .unwrap(); + let contribution = FundingBuilder::new( + FundingTemplate::new(None, None, None, Amount::MAX_MONEY), + feerate, + FeeRate::MAX, + ) + .with_coin_selection_source_sync(UnreachableWallet) + .add_value(Amount::from_sat(10_000)) + .unwrap() + .remove_value(Amount::from_sat(15_000)) + .unwrap() + .add_output(output.clone()) + .build() + .unwrap(); assert!(contribution.inputs.is_empty()); assert_eq!(contribution.outputs, vec![output]); @@ -2196,7 +2230,7 @@ mod tests { let input = funding_input_sats(100_000); let output = funding_output_sats(25_000); - let contribution = FundingTemplate::new(None, None, None) + let contribution = FundingTemplate::new(None, None, None, Amount::ZERO) .without_prior_contribution(feerate, FeeRate::MAX) .add_input(input.clone()) .unwrap() @@ -2236,7 +2270,7 @@ mod tests { let second_input = funding_input_sats(60_000); let output = funding_output_sats(25_000); - let contribution = FundingTemplate::new(None, None, None) + let contribution = FundingTemplate::new(None, None, None, Amount::ZERO) .without_prior_contribution(feerate, FeeRate::MAX) .add_inputs(vec![first_input.clone(), second_input.clone()]) .unwrap() @@ -2270,7 +2304,7 @@ mod tests { let second_input = funding_input_sats(60_000); let output = funding_output_sats(25_000); - let contribution = FundingTemplate::new(None, None, None) + let contribution = FundingTemplate::new(None, None, None, Amount::ZERO) .without_prior_contribution(feerate, FeeRate::MAX) .add_inputs(vec![first_input.clone(), second_input.clone()]) .unwrap() @@ -2303,7 +2337,7 @@ mod tests { let first_input = funding_input_sats(40_000); let second_input = funding_input_sats(60_000); - let contribution = FundingTemplate::new(None, None, None) + let contribution = FundingTemplate::new(None, None, None, Amount::ZERO) .splice_in_inputs( vec![first_input.clone(), second_input.clone()], feerate, @@ -2350,13 +2384,9 @@ mod tests { input_mode: Some(FundingInputMode::ManuallySelected), }; - let contribution = FundingTemplate::new( - None, - None, - Some(PriorContribution::new(prior, Amount::MAX_MONEY)), - ) - .splice_in_inputs(vec![additional_input.clone()], feerate, FeeRate::MAX) - .unwrap(); + let contribution = FundingTemplate::new(None, None, Some(prior), Amount::MAX_MONEY) + .splice_in_inputs(vec![additional_input.clone()], feerate, FeeRate::MAX) + .unwrap(); assert_eq!(contribution.inputs, vec![prior_input, additional_input]); assert!(contribution.outputs.is_empty()); @@ -2366,7 +2396,7 @@ mod tests { #[test] fn test_sync_funding_builder_manual_inputs_insufficient_do_not_fallback_to_coin_selection() { let feerate = FeeRate::from_sat_per_kwu(2000); - let builder = FundingTemplate::new(None, None, None) + let builder = FundingTemplate::new(None, None, None, Amount::ZERO) .without_prior_contribution(feerate, FeeRate::MAX) .add_input(funding_input_sats(1)) .unwrap(); @@ -2382,7 +2412,7 @@ mod tests { #[test] fn test_funding_builder_rejects_manual_inputs_with_value_request() { let feerate = FeeRate::from_sat_per_kwu(2000); - let builder = FundingTemplate::new(None, None, None) + let builder = FundingTemplate::new(None, None, None, Amount::ZERO) .without_prior_contribution(feerate, FeeRate::MAX) .add_input(funding_input_sats(100_000)) .unwrap(); @@ -2411,9 +2441,8 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let builder = - FundingTemplate::new(None, None, Some(PriorContribution::new(prior, Amount::ZERO))) - .with_prior_contribution(feerate, FeeRate::MAX); + let builder = FundingTemplate::new(None, None, Some(prior), Amount::MAX_MONEY) + .with_prior_contribution(feerate, FeeRate::MAX); assert!(matches!( builder.clone().add_input(funding_input_sats(50_000)), @@ -2430,7 +2459,7 @@ mod tests { let feerate = FeeRate::from_sat_per_kwu(2000); let inputs = vec![funding_input_sats(Amount::MAX_MONEY.to_sat()), funding_input_sats(1)]; - let builder = FundingTemplate::new(None, None, None) + let builder = FundingTemplate::new(None, None, None, Amount::ZERO) .without_prior_contribution(feerate, FeeRate::MAX) .add_inputs(inputs) .unwrap(); @@ -2463,14 +2492,10 @@ mod tests { input_mode: Some(FundingInputMode::ManuallySelected), }; - let contribution = FundingTemplate::new( - None, - None, - Some(PriorContribution::new(prior, Amount::MAX_MONEY)), - ) - .with_prior_contribution(target_feerate, FeeRate::MAX) - .build() - .unwrap(); + let contribution = FundingTemplate::new(None, None, Some(prior), Amount::MAX_MONEY) + .with_prior_contribution(target_feerate, FeeRate::MAX) + .build() + .unwrap(); assert_eq!(contribution.inputs, vec![input]); assert_eq!(contribution.outputs, vec![output]); @@ -2495,11 +2520,10 @@ mod tests { input_mode: Some(FundingInputMode::ManuallySelected), }; - let result = - FundingTemplate::new(None, None, Some(PriorContribution::new(prior, Amount::ZERO))) - .with_prior_contribution(feerate, FeeRate::MAX) - .add_output(funding_output_sats(60_000)) - .build(); + let result = FundingTemplate::new(None, None, Some(prior), Amount::ZERO) + .with_prior_contribution(feerate, FeeRate::MAX) + .add_output(funding_output_sats(60_000)) + .build(); assert!(matches!( result, @@ -2590,7 +2614,7 @@ mod tests { // splice_in_sync with value_added > MAX_MONEY { - let template = FundingTemplate::new(None, None, None); + let template = FundingTemplate::new(None, None, None, Amount::ZERO); assert!(matches!( template.splice_in_sync(over_max, feerate, feerate, UnreachableWallet), Err(FundingContributionError::InvalidSpliceValue), @@ -2599,7 +2623,7 @@ mod tests { // splice_out with single output value > MAX_MONEY { - let template = FundingTemplate::new(None, None, None); + let template = FundingTemplate::new(None, None, None, Amount::ZERO); let outputs = vec![funding_output_sats(over_max.to_sat())]; assert!(matches!( template.splice_out(outputs, feerate, feerate), @@ -2609,7 +2633,7 @@ mod tests { // splice_out with multiple outputs summing > MAX_MONEY { - let template = FundingTemplate::new(None, None, None); + let template = FundingTemplate::new(None, None, None, Amount::ZERO); let half_over = Amount::MAX_MONEY / 2 + Amount::from_sat(1); let outputs = vec![ funding_output_sats(half_over.to_sat()), @@ -2629,7 +2653,7 @@ mod tests { // Mixed add/remove request with value_added > MAX_MONEY. assert!(matches!( - FundingTemplate::new(None, None, None) + FundingTemplate::new(None, None, None, Amount::ZERO) .without_prior_contribution(feerate, feerate) .with_coin_selection_source_sync(UnreachableWallet) .add_value(over_max) @@ -2642,7 +2666,7 @@ mod tests { // 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) + FundingTemplate::new(None, None, None, Amount::ZERO) .without_prior_contribution(feerate, feerate) .with_coin_selection_source_sync(UnreachableWallet) .add_value(Amount::from_sat(1_000)) @@ -2663,7 +2687,7 @@ mod tests { // min_feerate > max_feerate is rejected { - let template = FundingTemplate::new(None, None, None); + let template = FundingTemplate::new(None, None, None, Amount::ZERO); assert!(matches!( template.splice_in_sync(Amount::from_sat(10_000), high, low, UnreachableWallet), Err(FundingContributionError::FeeRateExceedsMaximum { .. }), @@ -2672,7 +2696,7 @@ mod tests { // min_feerate < min_rbf_feerate is rejected { - let template = FundingTemplate::new(None, Some(high), None); + let template = FundingTemplate::new(None, Some(high), None, Amount::ZERO); assert!(matches!( template.splice_in_sync( Amount::from_sat(10_000), @@ -2703,7 +2727,7 @@ mod tests { change_output: None, }; assert!(matches!( - FundingTemplate::new(None, None, None) + FundingTemplate::new(None, None, None, Amount::ZERO) .with_prior_contribution(feerate, feerate) .with_coin_selection_source_sync(wallet) .add_value(Amount::from_sat(10_000)) @@ -2741,7 +2765,7 @@ mod tests { let net_value_before = contribution.net_value(); let contribution = - contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX).unwrap(); + contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY).unwrap(); // Target fee at target feerate for acceptor (is_initiator=false), including change weight. let expected_target_fee = @@ -2777,7 +2801,7 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); + let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY); assert!(matches!(result, Err(FeeRateAdjustmentError::FeeRateTooLow { .. }))); } @@ -2820,7 +2844,7 @@ mod tests { let net_value_before = contribution.net_value(); let contribution = - contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX).unwrap(); + contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY).unwrap(); // Change should be removed; estimated_fee updated to no-change target fee. assert!(contribution.change_output.is_none()); @@ -2854,7 +2878,7 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); + let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY); assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. }))); } @@ -2881,7 +2905,7 @@ mod tests { }; let contribution = - contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX).unwrap(); + contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY).unwrap(); // estimated_fee is updated to the target fee; surplus goes back to channel balance. let expected_target_fee = estimate_transaction_fee(&[], &outputs, None, false, true, target_feerate); @@ -2942,8 +2966,9 @@ mod tests { // For splice-in with change that stays above dust, the surplus is absorbed by the change // output so net_value_for_acceptor_at_feerate equals net_value. - let net_at_feerate = - contribution.net_value_for_acceptor_at_feerate(target_feerate, Amount::MAX).unwrap(); + let net_at_feerate = contribution + .net_value_for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY) + .unwrap(); assert_eq!(net_at_feerate, contribution.net_value()); assert_eq!( net_at_feerate, @@ -2973,8 +2998,9 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let net_at_feerate = - contribution.net_value_for_acceptor_at_feerate(target_feerate, Amount::MAX).unwrap(); + let net_at_feerate = contribution + .net_value_for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY) + .unwrap(); // The target fee at target feerate should be less than the initiator's fee estimate. let target_fee = estimate_transaction_fee(&[], &outputs, None, false, true, target_feerate); @@ -3013,7 +3039,7 @@ mod tests { let fee_before = contribution.estimated_fee; let change_before = contribution.change_output.as_ref().unwrap().value; - let _ = contribution.net_value_for_acceptor_at_feerate(target_feerate, Amount::MAX); + let _ = contribution.net_value_for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY); // Nothing should have changed. assert_eq!(contribution.net_value(), net_before); @@ -3043,7 +3069,8 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let result = contribution.net_value_for_acceptor_at_feerate(target_feerate, Amount::MAX); + let result = + contribution.net_value_for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY); assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. }))); } @@ -3071,7 +3098,7 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); + let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY); assert!(matches!(result, Err(FeeRateAdjustmentError::FeeRateTooHigh { .. }))); } @@ -3103,7 +3130,7 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); + let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY); assert!(result.is_ok()); let adjusted = result.unwrap(); @@ -3138,7 +3165,7 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); + let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY); assert!(result.is_ok()); let adjusted = result.unwrap(); @@ -3181,7 +3208,7 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); + let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY); assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. }))); } @@ -3214,7 +3241,7 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); + let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY); assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferInsufficient { .. }))); } @@ -3253,7 +3280,7 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX); + let result = contribution.for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY); assert!(result.is_ok()); let adjusted = result.unwrap(); assert!(adjusted.change_output.is_none()); @@ -3293,7 +3320,7 @@ mod tests { // target == min feerate, so FeeRateTooLow check passes. // The surplus (estimated_fee - target_fee) goes to value_added (shared output). let net_value_before = contribution.net_value(); - let result = contribution.for_acceptor_at_feerate(feerate, Amount::MAX); + let result = contribution.for_acceptor_at_feerate(feerate, Amount::MAX_MONEY); assert!(result.is_ok()); let adjusted = result.unwrap(); assert!(adjusted.change_output.is_none()); @@ -3318,7 +3345,7 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let result = contribution.for_acceptor_at_feerate(feerate, Amount::MAX); + let result = contribution.for_acceptor_at_feerate(feerate, Amount::MAX_MONEY); assert!(matches!(result, Err(FeeRateAdjustmentError::FeeBufferOverflow))); } @@ -3431,9 +3458,12 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let acceptor = - contribution.clone().for_acceptor_at_feerate(target_feerate, Amount::MAX).unwrap(); - let initiator = contribution.for_initiator_at_feerate(target_feerate, Amount::MAX).unwrap(); + let acceptor = contribution + .clone() + .for_acceptor_at_feerate(target_feerate, Amount::MAX_MONEY) + .unwrap(); + let initiator = + contribution.for_initiator_at_feerate(target_feerate, Amount::MAX_MONEY).unwrap(); // Initiator pays more in fees (common fields + shared input/output weight). assert!(initiator.estimated_fee > acceptor.estimated_fee); @@ -3467,11 +3497,8 @@ mod tests { }; // max_feerate (2020) < min_rbf_feerate (2025). - let template = FundingTemplate::new( - None, - Some(min_rbf_feerate), - Some(PriorContribution::new(prior, Amount::MAX)), - ); + let template = + FundingTemplate::new(None, Some(min_rbf_feerate), Some(prior), Amount::MAX_MONEY); assert!(matches!( template.rbf_prior_contribution_sync(None, max_feerate, UnreachableWallet), Err(FundingContributionError::FeeRateExceedsMaximum { .. }), @@ -3503,11 +3530,8 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let template = FundingTemplate::new( - None, - Some(min_rbf_feerate), - Some(PriorContribution::new(prior, Amount::MAX)), - ); + let template = + FundingTemplate::new(None, Some(min_rbf_feerate), Some(prior), Amount::MAX_MONEY); let contribution = template.rbf_prior_contribution_sync(None, max_feerate, UnreachableWallet).unwrap(); assert_eq!(contribution.feerate, min_rbf_feerate); @@ -3537,11 +3561,8 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let template = FundingTemplate::new( - None, - Some(min_rbf_feerate), - Some(PriorContribution::new(prior, Amount::MAX)), - ); + let template = + FundingTemplate::new(None, Some(min_rbf_feerate), Some(prior), Amount::MAX_MONEY); let contribution = template .rbf_prior_contribution_sync(Some(override_feerate), max_feerate, UnreachableWallet) .unwrap(); @@ -3566,11 +3587,8 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let template = FundingTemplate::new( - None, - Some(min_rbf_feerate), - Some(PriorContribution::new(prior, Amount::MAX)), - ); + let template = + FundingTemplate::new(None, Some(min_rbf_feerate), Some(prior), Amount::MAX_MONEY); assert!(matches!( template.rbf_prior_contribution_sync( Some(override_feerate), @@ -3599,11 +3617,8 @@ mod tests { input_mode: Some(FundingInputMode::CoinSelected), }; - let template = FundingTemplate::new( - None, - Some(min_rbf_feerate), - Some(PriorContribution::new(prior, Amount::MAX)), - ); + let template = + FundingTemplate::new(None, Some(min_rbf_feerate), Some(prior), Amount::MAX_MONEY); assert!(matches!( template.rbf_prior_contribution_sync( Some(override_feerate), @@ -3669,7 +3684,8 @@ mod tests { let template = FundingTemplate::new( Some(shared_input(100_000)), Some(min_rbf_feerate), - Some(PriorContribution::new(prior, Amount::ZERO)), + Some(prior), + Amount::ZERO, ); let wallet = SingleUtxoWallet { @@ -3711,7 +3727,8 @@ mod tests { let template = FundingTemplate::new( Some(shared_input(100_000)), Some(min_rbf_feerate), - Some(PriorContribution::new(prior, Amount::MAX)), + Some(prior), + Amount::MAX_MONEY, ); let wallet = SingleUtxoWallet { @@ -3737,8 +3754,12 @@ mod tests { let feerate = FeeRate::from_sat_per_kwu(2025); let withdrawal = funding_output_sats(20_000); - let template = - FundingTemplate::new(Some(shared_input(100_000)), Some(min_rbf_feerate), None); + let template = FundingTemplate::new( + Some(shared_input(100_000)), + Some(min_rbf_feerate), + None, + Amount::MAX_MONEY, + ); let contribution = template.splice_out(vec![withdrawal.clone()], feerate, FeeRate::MAX).unwrap(); diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index a50a216ad8b..de94afbf766 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -242,10 +242,8 @@ pub fn initiate_splice_out<'a, 'b, 'c, 'd>( outputs: Vec, ) -> Result { let node_id_acceptor = acceptor.node.get_our_node_id(); - 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 funding_contribution = funding_template.splice_out(outputs, feerate, FeeRate::MAX).unwrap(); + let funding_contribution = + build_splice_out_contribution(initiator, acceptor, channel_id, outputs).unwrap(); match initiator.node.funding_contributed( &channel_id, &node_id_acceptor, @@ -260,6 +258,17 @@ pub fn initiate_splice_out<'a, 'b, 'c, 'd>( } } +pub fn build_splice_out_contribution<'a, 'b, 'c, 'd>( + initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, + outputs: Vec, +) -> Result { + let node_id_acceptor = acceptor.node.get_our_node_id(); + 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); + funding_template.splice_out(outputs, feerate, FeeRate::MAX) +} + pub fn initiate_splice_in_and_out<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, value_added: Amount, outputs: Vec, @@ -4265,15 +4274,10 @@ fn do_test_splice_pending_htlcs(config: UserConfig) { let script_pubkey = initiator.wallet_source.get_change_script().unwrap(); let outputs = vec![TxOut { value: splice_out + Amount::ONE_SAT, script_pubkey }]; - let error = initiate_splice_out(initiator, acceptor, channel_id, outputs).unwrap_err(); - let cannot_accept_contribution = - format!("Channel {} cannot accept funding contribution", channel_id); - assert_eq!(error, APIError::APIMisuseError { err: cannot_accept_contribution }); - let cannot_be_funded = format!( - "Channel {} cannot be funded: Our splice-out value of {} is greater than the maximum {}", - channel_id, splice_out_incl_fees + Amount::ONE_SAT, splice_out_incl_fees, - ); - initiator.logger.assert_log("lightning::ln::channel", cannot_be_funded, 1); + assert!(matches!( + build_splice_out_contribution(initiator, acceptor, channel_id, outputs), + Err(FundingContributionError::InvalidSpliceValue), + )); // 2) Check that splicing out with the additional satoshi removed passes validation on the sender's side. @@ -7763,16 +7767,10 @@ fn do_test_0reserve_splice_holder_validation( mine_transaction(acceptor, &splice_tx); lock_splice_after_blocks(initiator, acceptor, ANTI_REORG_DELAY - 1); } else { - assert!(initiate_splice_out(initiator, acceptor, channel_id, outputs).is_err()); - let splice_out_value = - splice_out_max_value + Amount::from_sat(estimated_fees_sat) + Amount::ONE_SAT; - let splice_out_max_value = splice_out_max_value + Amount::from_sat(estimated_fees_sat); - let cannot_be_funded = format!( - "Channel {channel_id} cannot be funded: Our \ - splice-out value of {splice_out_value} is greater than the maximum \ - {splice_out_max_value}" - ); - initiator.logger.assert_log("lightning::ln::channel", cannot_be_funded, 1); + assert!(matches!( + build_splice_out_contribution(initiator, acceptor, channel_id, outputs), + Err(FundingContributionError::InvalidSpliceValue), + )); } channel_type From 6e16493b4b870c2b9c6f30eda93c972837d04d4e Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 6 May 2026 12:26:13 -0700 Subject: [PATCH 3/3] Detect duplicate inputs and outputs upon building FundingBuilder While this is already enforced when we get to the interactive negotiation phase, we choose to fail early anyway. --- lightning/src/ln/funding.rs | 57 +++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index d707428ecb2..6a68271dad9 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -132,8 +132,8 @@ pub enum FundingContributionError { /// The minimum RBF feerate. min_rbf_feerate: FeeRate, }, - /// The splice value is invalid (zero, empty outputs, exceeds the maximum money supply, or - /// splices out more than the available channel balance). + /// The splice value is invalid (zero, empty outputs, duplicate inputs or outputs, exceeds the + /// maximum money supply, or splices out more than the available channel balance). InvalidSpliceValue, /// An input's `prevtx` is too large to fit in a `tx_add_input` message. PrevTxTooLarge, @@ -164,7 +164,10 @@ impl core::fmt::Display for FundingContributionError { write!(f, "Feerate {} is below minimum RBF feerate {}", feerate, min_rbf_feerate) }, FundingContributionError::InvalidSpliceValue => { - write!(f, "Invalid splice value (zero, empty, exceeds limit, or overdraws balance)") + write!( + f, + "Invalid splice value (zero, empty, duplicate, exceeds limit, or overdraws balance)" + ) }, FundingContributionError::PrevTxTooLarge => { write!(f, "Input prevtx is too large to fit in a tx_add_input message") @@ -514,7 +517,14 @@ fn estimate_transaction_fee( fn validate_inputs(inputs: &[FundingTxInput]) -> Result<(), FundingContributionError> { let mut total_value = Amount::ZERO; - for input in inputs { + for (idx, input) in inputs.iter().enumerate() { + if inputs[..idx] + .iter() + .any(|existing_input| existing_input.utxo.outpoint == input.utxo.outpoint) + { + return Err(FundingContributionError::InvalidSpliceValue); + } + use crate::util::ser::Writeable; const MESSAGE_TEMPLATE: msgs::TxAddInput = msgs::TxAddInput { channel_id: ChannelId([0; 32]), @@ -1330,7 +1340,14 @@ impl FundingBuilderInner { )?; let mut value_removed = Amount::ZERO; - for output in self.outputs.iter() { + for (idx, output) in self.outputs.iter().enumerate() { + if self.outputs[..idx] + .iter() + .any(|existing_output| existing_output.script_pubkey == output.script_pubkey) + { + return Err(FundingContributionError::InvalidSpliceValue); + } + value_removed = match value_removed.checked_add(output.value) { Some(sum) if sum <= Amount::MAX_MONEY => sum, _ => return Err(FundingContributionError::InvalidSpliceValue), @@ -2297,6 +2314,36 @@ mod tests { ); } + #[test] + fn test_funding_builder_rejects_duplicate_inputs() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let input = funding_input_sats(100_000); + + let result = FundingTemplate::new(None, None, None, Amount::ZERO) + .without_prior_contribution(feerate, FeeRate::MAX) + .add_inputs(vec![input.clone(), input]) + .unwrap() + .build(); + + assert!(matches!(result, Err(FundingContributionError::InvalidSpliceValue),)); + } + + #[test] + fn test_funding_builder_rejects_duplicate_outputs() { + let feerate = FeeRate::from_sat_per_kwu(2000); + let first_output = funding_output_sats(25_000); + let second_output = funding_output_sats(30_000); + assert_ne!(first_output, second_output); + assert_eq!(first_output.script_pubkey, second_output.script_pubkey); + + let result = FundingTemplate::new(None, None, None, Amount::MAX_MONEY) + .without_prior_contribution(feerate, FeeRate::MAX) + .add_outputs(vec![first_output, second_output]) + .build(); + + assert!(matches!(result, Err(FundingContributionError::InvalidSpliceValue),)); + } + #[test] fn test_funding_builder_remove_input_updates_manual_input_request() { let feerate = FeeRate::from_sat_per_kwu(2000);