From ee2467b83907203cb90da8967ab591a018d5e703 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 10 Feb 2026 17:57:13 -0600 Subject: [PATCH 01/13] Stop persisting QuiescentAction and remove legacy code Now that the Splice variant (containing non-serializable FundingContribution) is the only variant produced, and the previous commit consumes the acceptor's quiescent_action in splice_init(), there is no longer a need to persist it. This allows removing LegacySplice, SpliceInstructions, ChangeStrategy, and related code paths including calculate_change_output, calculate_change_output_value, and the legacy send_splice_init method. With ChangeStrategy removed, the only remaining path in calculate_change_output was FromCoinSelection which always returned Ok(None), making it dead code. The into_interactive_tx_constructor method is simplified accordingly, and the signer_provider parameter is removed from it and from splice_init/splice_ack since it was only needed for the removed change output calculation. On deserialization, quiescent_action (TLV 65) is still read for backwards compatibility but discarded, and the awaiting_quiescence channel state flag is cleared since it cannot be acted upon without a quiescent_action. Co-Authored-By: Claude Opus 4.6 --- lightning/src/ln/channel.rs | 213 +++------------------------- lightning/src/ln/channelmanager.rs | 2 - lightning/src/ln/interactivetxs.rs | 215 ++--------------------------- lightning/src/ln/splicing_tests.rs | 76 ++-------- 4 files changed, 37 insertions(+), 469 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index cd98ed70a43..13bc26768b8 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -28,7 +28,7 @@ use bitcoin::{secp256k1, sighash, FeeRate, Sequence, TxIn}; use crate::blinded_path::message::BlindedMessagePath; use crate::chain::chaininterface::{ - fee_for_weight, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator, TransactionType, + ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator, TransactionType, }; use crate::chain::channelmonitor::{ ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, CommitmentHTLCData, @@ -57,9 +57,9 @@ use crate::ln::channelmanager::{ }; use crate::ln::funding::{FundingContribution, FundingTemplate, FundingTxInput}; use crate::ln::interactivetxs::{ - calculate_change_output_value, get_output_weight, AbortReason, HandleTxCompleteValue, - InteractiveTxConstructor, InteractiveTxConstructorArgs, InteractiveTxMessageSend, - InteractiveTxSigningSession, NegotiationError, SharedOwnedInput, SharedOwnedOutput, + AbortReason, HandleTxCompleteValue, InteractiveTxConstructor, InteractiveTxConstructorArgs, + InteractiveTxMessageSend, InteractiveTxSigningSession, NegotiationError, SharedOwnedInput, + SharedOwnedOutput, }; use crate::ln::msgs; use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError, OnionErrorPacket}; @@ -2924,7 +2924,6 @@ impl_writeable_tlv_based!(PendingFunding, { enum FundingNegotiation { AwaitingAck { context: FundingNegotiationContext, - change_strategy: ChangeStrategy, new_holder_funding_key: PublicKey, }, ConstructingTransaction { @@ -3010,38 +3009,8 @@ impl PendingFunding { } } -#[derive(Debug)] -pub(crate) struct SpliceInstructions { - adjusted_funding_contribution: SignedAmount, - our_funding_inputs: Vec, - our_funding_outputs: Vec, - change_script: Option, - funding_feerate_per_kw: u32, - locktime: u32, -} - -impl SpliceInstructions { - fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - ( - self.our_funding_inputs.into_iter().map(|input| input.utxo.outpoint).collect(), - self.our_funding_outputs, - ) - } -} - -impl_writeable_tlv_based!(SpliceInstructions, { - (1, adjusted_funding_contribution, required), - (3, our_funding_inputs, required_vec), - (5, our_funding_outputs, required_vec), - (7, change_script, option), - (9, funding_feerate_per_kw, required), - (11, locktime, required), -}); - #[derive(Debug)] pub(crate) enum QuiescentAction { - // Deprecated in favor of the Splice variant and no longer produced as of LDK 0.3. - LegacySplice(SpliceInstructions), Splice { contribution: FundingContribution, locktime: LockTime, @@ -3059,10 +3028,6 @@ pub(super) enum QuiescentError { impl From for QuiescentError { fn from(action: QuiescentAction) -> Self { match action { - QuiescentAction::LegacySplice(_) => { - debug_assert!(false); - QuiescentError::DoNothing - }, QuiescentAction::Splice { contribution, .. } => { let (contributed_inputs, contributed_outputs) = contribution.into_contributed_inputs_and_outputs(); @@ -3091,7 +3056,6 @@ impl_writeable_tlv_based_enum_upgradable!(QuiescentAction, (0, contribution, required), (1, locktime, required), }, - {1, LegacySplice} => (), ); #[cfg(not(any(test, fuzzing, feature = "_test_utils")))] impl_writeable_tlv_based_enum_upgradable!(QuiescentAction, @@ -3099,7 +3063,6 @@ impl_writeable_tlv_based_enum_upgradable!(QuiescentAction, (0, contribution, required), (1, locktime, required), }, - {1, LegacySplice} => (), ); /// Wrapper around a [`Transaction`] useful for caching the result of [`Transaction::compute_txid`]. @@ -6737,23 +6700,12 @@ pub(super) struct FundingNegotiationContext { pub our_funding_outputs: Vec, } -/// How the funding transaction's change is determined. -#[derive(Debug)] -pub(super) enum ChangeStrategy { - /// The change output, if any, is included in the FundingContribution's outputs. - FromCoinSelection, - - /// The change output script. This will be used if needed or -- if not set -- generated using - /// `SignerProvider::get_destination_script`. - LegacyUserProvided(Option), -} - impl FundingNegotiationContext { /// Prepare and start interactive transaction negotiation. /// If error occurs, it is caused by our side, not the counterparty. fn into_interactive_tx_constructor( - mut self, context: &ChannelContext, funding: &FundingScope, signer_provider: &SP, - entropy_source: &ES, holder_node_id: PublicKey, change_strategy: ChangeStrategy, + self, context: &ChannelContext, funding: &FundingScope, entropy_source: &ES, + holder_node_id: PublicKey, ) -> Result { debug_assert_eq!( self.shared_funding_input.is_some(), @@ -6766,25 +6718,11 @@ impl FundingNegotiationContext { debug_assert!(matches!(context.channel_state, ChannelState::NegotiatingFunding(_))); } - // Note: For the error case when the inputs are insufficient, it will be handled after - // the `calculate_change_output_value` call below - let shared_funding_output = TxOut { value: Amount::from_sat(funding.get_value_satoshis()), script_pubkey: funding.get_funding_redeemscript().to_p2wsh(), }; - match self.calculate_change_output( - context, - signer_provider, - &shared_funding_output, - change_strategy, - ) { - Ok(Some(change_output)) => self.our_funding_outputs.push(change_output), - Ok(None) => {}, - Err(reason) => return Err(self.into_negotiation_error(reason)), - } - let constructor_args = InteractiveTxConstructorArgs { entropy_source, holder_node_id, @@ -6804,57 +6742,6 @@ impl FundingNegotiationContext { InteractiveTxConstructor::new(constructor_args) } - fn calculate_change_output( - &self, context: &ChannelContext, signer_provider: &SP, shared_funding_output: &TxOut, - change_strategy: ChangeStrategy, - ) -> Result, AbortReason> { - if self.our_funding_inputs.is_empty() { - return Ok(None); - } - - let change_script = match change_strategy { - ChangeStrategy::FromCoinSelection => return Ok(None), - ChangeStrategy::LegacyUserProvided(change_script) => change_script, - }; - - let change_value = calculate_change_output_value( - &self, - self.shared_funding_input.is_some(), - &shared_funding_output.script_pubkey, - context.holder_dust_limit_satoshis, - )?; - - if let Some(change_value) = change_value { - let change_script = match change_script { - Some(script) => script, - None => match signer_provider.get_destination_script(context.channel_keys_id) { - Ok(script) => script, - Err(_) => { - return Err(AbortReason::InternalError("Error getting change script")) - }, - }, - }; - let mut change_output = TxOut { value: change_value, script_pubkey: change_script }; - let change_output_weight = get_output_weight(&change_output.script_pubkey).to_wu(); - let change_output_fee = - fee_for_weight(self.funding_feerate_sat_per_1000_weight, change_output_weight); - let change_value_decreased_with_fee = - change_value.to_sat().saturating_sub(change_output_fee); - // Check dust limit again - if change_value_decreased_with_fee > context.holder_dust_limit_satoshis { - change_output.value = Amount::from_sat(change_value_decreased_with_fee); - return Ok(Some(change_output)); - } - } - - Ok(None) - } - - fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { - let (contributed_inputs, contributed_outputs) = self.into_contributed_inputs_and_outputs(); - NegotiationError { reason, contributed_inputs, contributed_outputs } - } - fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { let contributed_inputs = self.our_funding_inputs.into_iter().map(|input| input.utxo.outpoint).collect(); @@ -7101,15 +6988,6 @@ where fn abandon_quiescent_action(&mut self) -> Option { match self.quiescent_action.take() { - Some(QuiescentAction::LegacySplice(instructions)) => { - let (inputs, outputs) = instructions.into_contributed_inputs_and_outputs(); - Some(SpliceFundingFailed { - funding_txo: None, - channel_type: None, - contributed_inputs: inputs, - contributed_outputs: outputs, - }) - }, Some(QuiescentAction::Splice { contribution, .. }) => { let (inputs, outputs) = contribution.into_contributed_inputs_and_outputs(); Some(SpliceFundingFailed { @@ -12318,32 +12196,8 @@ where self.propose_quiescence(logger, QuiescentAction::Splice { contribution, locktime }) } - fn send_splice_init(&mut self, instructions: SpliceInstructions) -> msgs::SpliceInit { - let SpliceInstructions { - adjusted_funding_contribution, - our_funding_inputs, - our_funding_outputs, - change_script, - funding_feerate_per_kw, - locktime, - } = instructions; - - let prev_funding_input = self.funding.to_splice_funding_input(); - let context = FundingNegotiationContext { - is_initiator: true, - our_funding_contribution: adjusted_funding_contribution, - funding_tx_locktime: LockTime::from_consensus(locktime), - funding_feerate_sat_per_1000_weight: funding_feerate_per_kw, - shared_funding_input: Some(prev_funding_input), - our_funding_inputs, - our_funding_outputs, - }; - - self.send_splice_init_internal(context, ChangeStrategy::LegacyUserProvided(change_script)) - } - fn send_splice_init_internal( - &mut self, context: FundingNegotiationContext, change_strategy: ChangeStrategy, + &mut self, context: FundingNegotiationContext, ) -> msgs::SpliceInit { debug_assert!(self.pending_splice.is_none()); // Rotate the funding pubkey using the prev_funding_txid as a tweak @@ -12364,11 +12218,8 @@ where let funding_contribution_satoshis = context.our_funding_contribution.to_sat(); let locktime = context.funding_tx_locktime.to_consensus_u32(); - let funding_negotiation = FundingNegotiation::AwaitingAck { - context, - change_strategy, - new_holder_funding_key: funding_pubkey, - }; + let funding_negotiation = + FundingNegotiation::AwaitingAck { context, new_holder_funding_key: funding_pubkey }; self.pending_splice = Some(PendingFunding { funding_negotiation: Some(funding_negotiation), negotiated_candidates: vec![], @@ -12572,7 +12423,7 @@ where pub(crate) fn splice_init( &mut self, msg: &msgs::SpliceInit, our_funding_contribution_satoshis: i64, - signer_provider: &SP, entropy_source: &ES, holder_node_id: &PublicKey, logger: &L, + entropy_source: &ES, holder_node_id: &PublicKey, logger: &L, ) -> Result { let our_funding_contribution = SignedAmount::from_sat(our_funding_contribution_satoshis); let splice_funding = self.validate_splice_init(msg, our_funding_contribution)?; @@ -12600,11 +12451,8 @@ where .into_interactive_tx_constructor( &self.context, &splice_funding, - signer_provider, entropy_source, holder_node_id.clone(), - // ChangeStrategy doesn't matter when no inputs are contributed - ChangeStrategy::FromCoinSelection, ) .map_err(|err| { ChannelError::WarnAndDisconnect(format!( @@ -12639,8 +12487,8 @@ where } pub(crate) fn splice_ack( - &mut self, msg: &msgs::SpliceAck, signer_provider: &SP, entropy_source: &ES, - holder_node_id: &PublicKey, logger: &L, + &mut self, msg: &msgs::SpliceAck, entropy_source: &ES, holder_node_id: &PublicKey, + logger: &L, ) -> Result, ChannelError> { let splice_funding = self.validate_splice_ack(msg)?; @@ -12655,11 +12503,11 @@ where let pending_splice = self.pending_splice.as_mut().expect("We should have returned an error earlier!"); // TODO: Good candidate for a let else statement once MSRV >= 1.65 - let (funding_negotiation_context, change_strategy) = - if let Some(FundingNegotiation::AwaitingAck { context, change_strategy, .. }) = + let funding_negotiation_context = + if let Some(FundingNegotiation::AwaitingAck { context, .. }) = pending_splice.funding_negotiation.take() { - (context, change_strategy) + context } else { panic!("We should have returned an error earlier!"); }; @@ -12668,10 +12516,8 @@ where .into_interactive_tx_constructor( &self.context, &splice_funding, - signer_provider, entropy_source, holder_node_id.clone(), - change_strategy, ) .map_err(|err| { ChannelError::WarnAndDisconnect(format!( @@ -13540,22 +13386,6 @@ where "Internal Error: Didn't have anything to do after reaching quiescence".to_owned() )); }, - Some(QuiescentAction::LegacySplice(instructions)) => { - if self.pending_splice.is_some() { - debug_assert!(false); - self.quiescent_action = Some(QuiescentAction::LegacySplice(instructions)); - - return Err(ChannelError::WarnAndDisconnect( - format!( - "Channel {} cannot be spliced as it already has a splice pending", - self.context.channel_id(), - ), - )); - } - - let splice_init = self.send_splice_init(instructions); - return Ok(Some(StfuResponse::SpliceInit(splice_init))); - }, Some(QuiescentAction::Splice { contribution, locktime }) => { // TODO(splicing): If the splice has been negotiated but has not been locked, we // can RBF here to add the contribution. @@ -13587,7 +13417,7 @@ where our_funding_outputs, }; - let splice_init = self.send_splice_init_internal(context, ChangeStrategy::FromCoinSelection); + let splice_init = self.send_splice_init_internal(context); return Ok(Some(StfuResponse::SpliceInit(splice_init))); }, #[cfg(any(test, fuzzing, feature = "_test_utils"))] @@ -13629,8 +13459,7 @@ where // We can't initiate another splice while ours is pending, so don't bother becoming // quiescent yet. // TODO(splicing): Allow the splice as an RBF once supported. - let has_splice_action = matches!(action, QuiescentAction::Splice { .. }) - || matches!(action, QuiescentAction::LegacySplice(_)); + let has_splice_action = matches!(action, QuiescentAction::Splice { .. }); if has_splice_action && self.pending_splice.is_some() { log_given_level!( logger, @@ -15222,7 +15051,7 @@ impl Writeable for FundedChannel { (61, fulfill_attribution_data, optional_vec), // Added in 0.2 (63, holder_commitment_point_current, option), // Added in 0.2 (64, pending_splice, option), // Added in 0.2 - (65, self.quiescent_action, option), // Added in 0.2 + // 65 was previously used for quiescent_action (67, pending_outbound_held_htlc_flags, optional_vec), // Added in 0.2 (69, holding_cell_held_htlc_flags, optional_vec), // Added in 0.2 (71, holder_commitment_point_previous_revoked, option), // Added in 0.3 @@ -15612,7 +15441,7 @@ impl<'a, 'b, 'c, ES: EntropySource, SP: SignerProvider> let mut minimum_depth_override: Option = None; let mut pending_splice: Option = None; - let mut quiescent_action = None; + let mut _quiescent_action: Option = None; let mut pending_outbound_held_htlc_flags_opt: Option>> = None; let mut holding_cell_held_htlc_flags_opt: Option>> = None; @@ -15666,7 +15495,7 @@ impl<'a, 'b, 'c, ES: EntropySource, SP: SignerProvider> (61, fulfill_attribution_data, optional_vec), // Added in 0.2 (63, holder_commitment_point_current_opt, option), // Added in 0.2 (64, pending_splice, option), // Added in 0.2 - (65, quiescent_action, upgradable_option), // Added in 0.2 + (65, _quiescent_action, upgradable_option), // Added in 0.2 (67, pending_outbound_held_htlc_flags_opt, optional_vec), // Added in 0.2 (69, holding_cell_held_htlc_flags_opt, optional_vec), // Added in 0.2 (71, holder_commitment_point_previous_revoked_opt, option), // Added in 0.3 @@ -16131,7 +15960,7 @@ impl<'a, 'b, 'c, ES: EntropySource, SP: SignerProvider> }, holder_commitment_point, pending_splice, - quiescent_action, + quiescent_action: None, }) } } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d7c1b6000bf..525a764fd33 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -12836,7 +12836,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let init_res = funded_channel.splice_init( msg, our_funding_contribution, - &self.signer_provider, &self.entropy_source, &self.get_our_node_id(), &self.logger, @@ -12880,7 +12879,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(ref mut funded_channel) = chan_entry.get_mut().as_funded_mut() { let splice_ack_res = funded_channel.splice_ack( msg, - &self.signer_provider, &self.entropy_source, &self.get_our_node_id(), &self.logger, diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 7e7a9fb609c..17dab1918c0 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -12,7 +12,7 @@ use crate::io_extras::sink; use crate::prelude::*; use bitcoin::absolute::LockTime as AbsoluteLockTime; -use bitcoin::amount::{Amount, SignedAmount}; +use bitcoin::amount::Amount; use bitcoin::consensus::Encodable; use bitcoin::constants::WITNESS_SCALE_FACTOR; use bitcoin::ecdsa::Signature as BitcoinSignature; @@ -31,7 +31,7 @@ use crate::ln::chan_utils::{ BASE_INPUT_WEIGHT, EMPTY_SCRIPT_SIG_WEIGHT, FUNDING_TRANSACTION_WITNESS_WEIGHT, SEGWIT_MARKER_FLAG_WEIGHT, }; -use crate::ln::channel::{FundingNegotiationContext, TOTAL_BITCOIN_SUPPLY_SATOSHIS}; +use crate::ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS; use crate::ln::funding::FundingTxInput; use crate::ln::msgs; use crate::ln::msgs::{MessageSendEvent, SerialId, TxSignatures}; @@ -2323,102 +2323,16 @@ impl InteractiveTxConstructor { } } -/// Determine whether a change output should be added, and if yes, of what size, considering our -/// given inputs and outputs, and intended contribution. Takes into account the fees and the dust -/// limit. -/// -/// Three outcomes are possible: -/// - Inputs are sufficient for intended contribution, fees, and a larger-than-dust change: -/// `Ok(Some(change_amount))` -/// - Inputs are sufficient for intended contribution and fees, and a change output isn't needed: -/// `Ok(None)` -/// - Inputs are not sufficient to cover contribution and fees: -/// `Err(AbortReason::InsufficientFees)` -/// -/// Parameters: -/// - `context` - Context of the funding negotiation, including non-shared inputs and feerate. -/// - `is_splice` - Whether we splicing an existing channel or dual-funding a new one. -/// - `shared_output_funding_script` - The script of the shared output. -/// - `funding_outputs` - Our funding outputs. -/// - `change_output_dust_limit` - The dust limit (in sats) to consider. -pub(super) fn calculate_change_output_value( - context: &FundingNegotiationContext, is_splice: bool, shared_output_funding_script: &ScriptBuf, - change_output_dust_limit: u64, -) -> Result, AbortReason> { - let mut total_input_value = Amount::ZERO; - let mut our_funding_inputs_weight = 0u64; - for FundingTxInput { utxo, .. } in context.our_funding_inputs.iter() { - total_input_value = total_input_value.checked_add(utxo.output.value).unwrap_or(Amount::MAX); - - let weight = BASE_INPUT_WEIGHT + utxo.satisfaction_weight; - our_funding_inputs_weight = our_funding_inputs_weight.saturating_add(weight); - } - - let funding_outputs = &context.our_funding_outputs; - let total_output_value = funding_outputs - .iter() - .fold(Amount::ZERO, |total, out| total.checked_add(out.value).unwrap_or(Amount::MAX)); - - let our_funding_outputs_weight = funding_outputs.iter().fold(0u64, |weight, out| { - weight.saturating_add(get_output_weight(&out.script_pubkey).to_wu()) - }); - let mut weight = our_funding_outputs_weight.saturating_add(our_funding_inputs_weight); - - // If we are the initiator, we must pay for the weight of the funding output and - // all common fields in the funding transaction. - if context.is_initiator { - weight = weight.saturating_add(get_output_weight(shared_output_funding_script).to_wu()); - weight = weight.saturating_add(TX_COMMON_FIELDS_WEIGHT); - if is_splice { - // TODO(taproot): Needs to consider different weights based on channel type - weight = weight.saturating_add(BASE_INPUT_WEIGHT); - weight = weight.saturating_add(EMPTY_SCRIPT_SIG_WEIGHT); - weight = weight.saturating_add(FUNDING_TRANSACTION_WITNESS_WEIGHT); - #[cfg(feature = "grind_signatures")] - { - // Guarantees a low R signature - weight -= 1; - } - } - } - - let contributed_fees = - Amount::from_sat(fee_for_weight(context.funding_feerate_sat_per_1000_weight, weight)); - - let contributed_input_value = - context.our_funding_contribution + total_output_value.to_signed().unwrap(); - assert!(contributed_input_value > SignedAmount::ZERO); - let contributed_input_value = contributed_input_value.unsigned_abs(); - - let total_input_value_less_fees = - total_input_value.checked_sub(contributed_fees).unwrap_or(Amount::ZERO); - if total_input_value_less_fees < contributed_input_value { - // Not enough to cover contribution plus fees - return Err(AbortReason::InsufficientFees); - } - - let remaining_value = total_input_value_less_fees - .checked_sub(contributed_input_value) - .expect("remaining_value should not be negative"); - if remaining_value.to_sat() < change_output_dust_limit { - // Enough to cover contribution plus fees, but leftover is below dust limit; no change - Ok(None) - } else { - // Enough to have over-dust change - Ok(Some(remaining_value)) - } -} - #[cfg(test)] mod tests { use crate::chain::chaininterface::{fee_for_weight, FEERATE_FLOOR_SATS_PER_KW}; - use crate::ln::channel::{FundingNegotiationContext, TOTAL_BITCOIN_SUPPLY_SATOSHIS}; + use crate::ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS; use crate::ln::funding::FundingTxInput; use crate::ln::interactivetxs::{ - calculate_change_output_value, generate_holder_serial_id, AbortReason, - HandleTxCompleteValue, InteractiveTxConstructor, InteractiveTxConstructorArgs, - InteractiveTxMessageSend, SharedOwnedInput, SharedOwnedOutput, MAX_INPUTS_OUTPUTS_COUNT, - MAX_RECEIVED_TX_ADD_INPUT_COUNT, MAX_RECEIVED_TX_ADD_OUTPUT_COUNT, + generate_holder_serial_id, AbortReason, HandleTxCompleteValue, InteractiveTxConstructor, + InteractiveTxConstructorArgs, InteractiveTxMessageSend, SharedOwnedInput, + SharedOwnedOutput, MAX_INPUTS_OUTPUTS_COUNT, MAX_RECEIVED_TX_ADD_INPUT_COUNT, + MAX_RECEIVED_TX_ADD_OUTPUT_COUNT, }; use crate::ln::types::ChannelId; use crate::sign::EntropySource; @@ -2433,8 +2347,7 @@ mod tests { use bitcoin::transaction::Version; use bitcoin::{opcodes, WScriptHash, Weight, XOnlyPublicKey}; use bitcoin::{ - OutPoint, PubkeyHash, ScriptBuf, Sequence, SignedAmount, Transaction, TxIn, TxOut, - WPubkeyHash, + OutPoint, PubkeyHash, ScriptBuf, Sequence, Transaction, TxIn, TxOut, WPubkeyHash, }; use super::{ @@ -3398,118 +3311,6 @@ mod tests { assert_eq!(generate_holder_serial_id(&&entropy_source, false) % 2, 1) } - #[test] - fn test_calculate_change_output_value_open() { - let input_prevouts = [ - TxOut { - value: Amount::from_sat(70_000), - script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()), - }, - TxOut { - value: Amount::from_sat(60_000), - script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()), - }, - ]; - let inputs = input_prevouts - .iter() - .map(|txout| { - let prevtx = Transaction { - input: Vec::new(), - output: vec![(*txout).clone()], - lock_time: AbsoluteLockTime::ZERO, - version: Version::TWO, - }; - - FundingTxInput::new_p2wpkh(prevtx, 0).unwrap() - }) - .collect(); - let txout = TxOut { value: Amount::from_sat(10_000), script_pubkey: ScriptBuf::new() }; - let outputs = vec![txout]; - let funding_feerate_sat_per_1000_weight = 3000; - - let total_inputs: Amount = input_prevouts.iter().map(|o| o.value).sum(); - let total_outputs: Amount = outputs.iter().map(|o| o.value).sum(); - let fees = if cfg!(feature = "grind_signatures") { - Amount::from_sat(1734) - } else { - Amount::from_sat(1740) - }; - let common_fees = Amount::from_sat(234); - - // There is leftover for change - let context = FundingNegotiationContext { - is_initiator: true, - our_funding_contribution: SignedAmount::from_sat(110_000), - funding_tx_locktime: AbsoluteLockTime::ZERO, - funding_feerate_sat_per_1000_weight, - shared_funding_input: None, - our_funding_inputs: inputs, - our_funding_outputs: outputs, - }; - let gross_change = - total_inputs - total_outputs - context.our_funding_contribution.to_unsigned().unwrap(); - assert_eq!( - calculate_change_output_value(&context, false, &ScriptBuf::new(), 300), - Ok(Some(gross_change - fees - common_fees)), - ); - - // There is leftover for change, without common fees - let context = FundingNegotiationContext { is_initiator: false, ..context }; - assert_eq!( - calculate_change_output_value(&context, false, &ScriptBuf::new(), 300), - Ok(Some(gross_change - fees)), - ); - - // Insufficient inputs, no leftover - let context = FundingNegotiationContext { - is_initiator: false, - our_funding_contribution: SignedAmount::from_sat(130_000), - ..context - }; - assert_eq!( - calculate_change_output_value(&context, false, &ScriptBuf::new(), 300), - Err(AbortReason::InsufficientFees), - ); - - // Very small leftover - let context = FundingNegotiationContext { - is_initiator: false, - our_funding_contribution: SignedAmount::from_sat(118_000), - ..context - }; - assert_eq!( - calculate_change_output_value(&context, false, &ScriptBuf::new(), 300), - Ok(None), - ); - - // Small leftover, but not dust - let context = FundingNegotiationContext { - is_initiator: false, - our_funding_contribution: SignedAmount::from_sat(117_992), - ..context - }; - let gross_change = - total_inputs - total_outputs - context.our_funding_contribution.to_unsigned().unwrap(); - assert_eq!( - calculate_change_output_value(&context, false, &ScriptBuf::new(), 100), - Ok(Some(gross_change - fees)), - ); - - // Larger fee, smaller change - let context = FundingNegotiationContext { - is_initiator: true, - our_funding_contribution: SignedAmount::from_sat(110_000), - funding_feerate_sat_per_1000_weight: funding_feerate_sat_per_1000_weight * 3, - ..context - }; - let gross_change = - total_inputs - total_outputs - context.our_funding_contribution.to_unsigned().unwrap(); - assert_eq!( - calculate_change_output_value(&context, false, &ScriptBuf::new(), 300), - Ok(Some(gross_change - fees * 3 - common_fees * 3)), - ); - } - fn do_verify_tx_signatures( transaction: Transaction, prev_outputs: Vec, ) -> Result<(), String> { diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index ab890fdbab7..4e6aceb3b86 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -1663,28 +1663,23 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { #[test] fn test_propose_splice_while_disconnected() { - do_test_propose_splice_while_disconnected(false, false); - do_test_propose_splice_while_disconnected(false, true); - do_test_propose_splice_while_disconnected(true, false); - do_test_propose_splice_while_disconnected(true, true); + do_test_propose_splice_while_disconnected(false); + do_test_propose_splice_while_disconnected(true); } #[cfg(test)] -fn do_test_propose_splice_while_disconnected(reload: bool, use_0conf: bool) { +fn do_test_propose_splice_while_disconnected(use_0conf: bool) { // Test that both nodes are able to propose a splice while the counterparty is disconnected, and // whoever doesn't go first due to the quiescence tie-breaker, will retry their splice after the // first one becomes locked. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let (persister_0a, persister_0b, persister_1a, persister_1b); - let (chain_monitor_0a, chain_monitor_0b, chain_monitor_1a, chain_monitor_1b); let mut config = test_default_channel_config(); if use_0conf { config.channel_handshake_limits.trust_own_funding_0conf = true; } let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]); - let (node_0a, node_0b, node_1a, node_1b); - let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); @@ -1722,15 +1717,8 @@ fn do_test_propose_splice_while_disconnected(reload: bool, use_0conf: bool) { value: Amount::from_sat(splice_out_sat), script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), }]; - let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate).unwrap(); - let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let node_0_funding_contribution = - funding_template.splice_out_sync(node_0_outputs, &wallet).unwrap(); - nodes[0] - .node - .funding_contributed(&channel_id, &node_id_1, node_0_funding_contribution.clone(), None) - .unwrap(); + initiate_splice_out(&nodes[0], &nodes[1], channel_id, node_0_outputs); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); @@ -1738,38 +1726,11 @@ fn do_test_propose_splice_while_disconnected(reload: bool, use_0conf: bool) { value: Amount::from_sat(splice_out_sat), script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), }]; - let funding_template = nodes[1].node.splice_channel(&channel_id, &node_id_0, feerate).unwrap(); - let wallet = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); let node_1_funding_contribution = - funding_template.splice_out_sync(node_1_outputs, &wallet).unwrap(); - nodes[1] - .node - .funding_contributed(&channel_id, &node_id_0, node_1_funding_contribution.clone(), None) - .unwrap(); + initiate_splice_out(&nodes[1], &nodes[0], channel_id, node_1_outputs); assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); - if reload { - let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); - reload_node!( - nodes[0], - nodes[0].node.encode(), - &[&encoded_monitor_0], - persister_0a, - chain_monitor_0a, - node_0a - ); - let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode(); - reload_node!( - nodes[1], - nodes[1].node.encode(), - &[&encoded_monitor_1], - persister_1a, - chain_monitor_1a, - node_1a - ); - } - // Reconnect the nodes. Both nodes should attempt quiescence as the initiator, but only one will // be it via the tie-breaker. let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); @@ -1890,29 +1851,8 @@ fn do_test_propose_splice_while_disconnected(reload: bool, use_0conf: bool) { // Reconnect the nodes. This should trigger the node which lost the tie-breaker to resend `stfu` // for their splice attempt. - if reload { - let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); - reload_node!( - nodes[0], - nodes[0].node.encode(), - &[&encoded_monitor_0], - persister_0b, - chain_monitor_0b, - node_0b - ); - let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode(); - reload_node!( - nodes[1], - nodes[1].node.encode(), - &[&encoded_monitor_1], - persister_1b, - chain_monitor_1b, - node_1b - ); - } else { - nodes[0].node.peer_disconnected(node_id_1); - nodes[1].node.peer_disconnected(node_id_0); - } + nodes[0].node.peer_disconnected(node_id_1); + nodes[1].node.peer_disconnected(node_id_0); let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); if !use_0conf { reconnect_args.send_announcement_sigs = (true, true); From ea7efe05b452446cf9d1fef67ad0bdf9a9c40972 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Feb 2026 09:51:04 -0600 Subject: [PATCH 02/13] f - remove QuiescentAction reading --- lightning/src/ln/channel.rs | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 13bc26768b8..c38e2082c47 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3049,22 +3049,6 @@ pub(crate) enum StfuResponse { SpliceInit(msgs::SpliceInit), } -#[cfg(any(test, fuzzing, feature = "_test_utils"))] -impl_writeable_tlv_based_enum_upgradable!(QuiescentAction, - (0, DoNothing) => {}, - (2, Splice) => { - (0, contribution, required), - (1, locktime, required), - }, -); -#[cfg(not(any(test, fuzzing, feature = "_test_utils")))] -impl_writeable_tlv_based_enum_upgradable!(QuiescentAction, - (2, Splice) => { - (0, contribution, required), - (1, locktime, required), - }, -); - /// Wrapper around a [`Transaction`] useful for caching the result of [`Transaction::compute_txid`]. struct ConfirmedTransaction<'a> { tx: &'a Transaction, @@ -15441,7 +15425,6 @@ impl<'a, 'b, 'c, ES: EntropySource, SP: SignerProvider> let mut minimum_depth_override: Option = None; let mut pending_splice: Option = None; - let mut _quiescent_action: Option = None; let mut pending_outbound_held_htlc_flags_opt: Option>> = None; let mut holding_cell_held_htlc_flags_opt: Option>> = None; @@ -15495,7 +15478,7 @@ impl<'a, 'b, 'c, ES: EntropySource, SP: SignerProvider> (61, fulfill_attribution_data, optional_vec), // Added in 0.2 (63, holder_commitment_point_current_opt, option), // Added in 0.2 (64, pending_splice, option), // Added in 0.2 - (65, _quiescent_action, upgradable_option), // Added in 0.2 + // 65 quiescent_action: Added in 0.2; removed in 0.3 (67, pending_outbound_held_htlc_flags_opt, optional_vec), // Added in 0.2 (69, holding_cell_held_htlc_flags_opt, optional_vec), // Added in 0.2 (71, holder_commitment_point_previous_revoked_opt, option), // Added in 0.3 From 383bbc8320e2980321b3cff8eee2147931602f91 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Feb 2026 09:53:07 -0600 Subject: [PATCH 03/13] f - rename send_splice_init_internal --- lightning/src/ln/channel.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index c38e2082c47..4fe05c41280 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12180,9 +12180,7 @@ where self.propose_quiescence(logger, QuiescentAction::Splice { contribution, locktime }) } - fn send_splice_init_internal( - &mut self, context: FundingNegotiationContext, - ) -> msgs::SpliceInit { + fn send_splice_init(&mut self, context: FundingNegotiationContext) -> msgs::SpliceInit { debug_assert!(self.pending_splice.is_none()); // Rotate the funding pubkey using the prev_funding_txid as a tweak let prev_funding_txid = self.funding.get_funding_txid(); @@ -13401,7 +13399,7 @@ where our_funding_outputs, }; - let splice_init = self.send_splice_init_internal(context); + let splice_init = self.send_splice_init(context); return Ok(Some(StfuResponse::SpliceInit(splice_init))); }, #[cfg(any(test, fuzzing, feature = "_test_utils"))] From ce35601f51124895c601d1adb4b0896a322b8e87 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 11 Feb 2026 22:44:01 -0600 Subject: [PATCH 04/13] Split InteractiveTxConstructor::new into outbound/inbound variants Replace the single public InteractiveTxConstructor::new() with separate new_for_outbound() and new_for_inbound() constructors. This moves the initiator's first message preparation out of the core constructor, making it infallible and removing is_initiator from the args struct. Callers no longer need to handle constructor errors, which avoids having to generate SpliceFailed/DiscardFunding events after the QuiescentAction has already been consumed during splice_init/splice_ack handling. Co-Authored-By: Claude Opus 4.6 --- lightning/src/ln/channel.rs | 36 ++---- lightning/src/ln/interactivetxs.rs | 183 ++++++++++++++--------------- 2 files changed, 96 insertions(+), 123 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 4fe05c41280..eae860b9669 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -58,8 +58,7 @@ use crate::ln::channelmanager::{ use crate::ln::funding::{FundingContribution, FundingTemplate, FundingTxInput}; use crate::ln::interactivetxs::{ AbortReason, HandleTxCompleteValue, InteractiveTxConstructor, InteractiveTxConstructorArgs, - InteractiveTxMessageSend, InteractiveTxSigningSession, NegotiationError, SharedOwnedInput, - SharedOwnedOutput, + InteractiveTxMessageSend, InteractiveTxSigningSession, SharedOwnedInput, SharedOwnedOutput, }; use crate::ln::msgs; use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError, OnionErrorPacket}; @@ -6690,7 +6689,7 @@ impl FundingNegotiationContext { fn into_interactive_tx_constructor( self, context: &ChannelContext, funding: &FundingScope, entropy_source: &ES, holder_node_id: PublicKey, - ) -> Result { + ) -> InteractiveTxConstructor { debug_assert_eq!( self.shared_funding_input.is_some(), funding.channel_transaction_parameters.splice_parent_funding_txid.is_some(), @@ -6713,7 +6712,6 @@ impl FundingNegotiationContext { counterparty_node_id: context.counterparty_node_id, channel_id: context.channel_id(), feerate_sat_per_kw: self.funding_feerate_sat_per_1000_weight, - is_initiator: self.is_initiator, funding_tx_locktime: self.funding_tx_locktime, inputs_to_contribute: self.our_funding_inputs, shared_funding_input: self.shared_funding_input, @@ -6723,7 +6721,11 @@ impl FundingNegotiationContext { ), outputs_to_contribute: self.our_funding_outputs, }; - InteractiveTxConstructor::new(constructor_args) + if self.is_initiator { + InteractiveTxConstructor::new_for_outbound(constructor_args) + } else { + InteractiveTxConstructor::new_for_inbound(constructor_args) + } } fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { @@ -12435,13 +12437,7 @@ where &splice_funding, entropy_source, holder_node_id.clone(), - ) - .map_err(|err| { - ChannelError::WarnAndDisconnect(format!( - "Failed to start interactive transaction construction, {:?}", - err - )) - })?; + ); debug_assert!(interactive_tx_constructor.take_initiator_first_message().is_none()); // TODO(splicing): if quiescent_action is set, integrate what the user wants to do into the @@ -12500,13 +12496,7 @@ where &splice_funding, entropy_source, holder_node_id.clone(), - ) - .map_err(|err| { - ChannelError::WarnAndDisconnect(format!( - "Failed to start interactive transaction construction, {:?}", - err - )) - })?; + ); let tx_msg_opt = interactive_tx_constructor.take_initiator_first_message(); debug_assert!(self.context.interactive_tx_signing_session.is_none()); @@ -14367,7 +14357,7 @@ impl PendingV2Channel { script_pubkey: funding.get_funding_redeemscript().to_p2wsh(), }; - let interactive_tx_constructor = Some(InteractiveTxConstructor::new( + let interactive_tx_constructor = Some(InteractiveTxConstructor::new_for_inbound( InteractiveTxConstructorArgs { entropy_source, holder_node_id, @@ -14375,16 +14365,12 @@ impl PendingV2Channel { channel_id: context.channel_id, feerate_sat_per_kw: funding_negotiation_context.funding_feerate_sat_per_1000_weight, funding_tx_locktime: funding_negotiation_context.funding_tx_locktime, - is_initiator: false, inputs_to_contribute: our_funding_inputs, shared_funding_input: None, shared_funding_output: SharedOwnedOutput::new(shared_funding_output, our_funding_contribution_sats), outputs_to_contribute: funding_negotiation_context.our_funding_outputs.clone(), } - ).map_err(|err| { - let reason = ClosureReason::ProcessingError { err: err.reason.to_string() }; - ChannelError::Close((err.reason.to_string(), reason)) - })?); + )); let unfunded_context = UnfundedChannelContext { unfunded_channel_age_ticks: 0, diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 17dab1918c0..9f9a9ce487e 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -2020,7 +2020,6 @@ pub(super) struct InteractiveTxConstructorArgs<'a, ES: EntropySource> { pub counterparty_node_id: PublicKey, pub channel_id: ChannelId, pub feerate_sat_per_kw: u32, - pub is_initiator: bool, pub funding_tx_locktime: AbsoluteLockTime, pub inputs_to_contribute: Vec, pub shared_funding_input: Option, @@ -2031,18 +2030,15 @@ pub(super) struct InteractiveTxConstructorArgs<'a, ES: EntropySource> { impl InteractiveTxConstructor { /// Instantiates a new `InteractiveTxConstructor`. /// - /// If the holder is the initiator, they need to send the first message which is a `TxAddInput` - /// message. - pub fn new( - args: InteractiveTxConstructorArgs, - ) -> Result { + /// Use [`Self::new_for_outbound`] or [`Self::new_for_inbound`] instead to also prepare the + /// first message for the initiator. + fn new(args: InteractiveTxConstructorArgs, is_initiator: bool) -> Self { let InteractiveTxConstructorArgs { entropy_source, holder_node_id, counterparty_node_id, channel_id, feerate_sat_per_kw, - is_initiator, funding_tx_locktime, inputs_to_contribute, shared_funding_input, @@ -2112,7 +2108,7 @@ impl InteractiveTxConstructor { let next_input_index = (!inputs_to_contribute.is_empty()).then_some(0); let next_output_index = (!outputs_to_contribute.is_empty()).then_some(0); - let mut constructor = Self { + Self { state_machine, is_initiator, initiator_first_message: None, @@ -2121,19 +2117,32 @@ impl InteractiveTxConstructor { outputs_to_contribute, next_input_index, next_output_index, - }; - // We'll store the first message for the initiator. - if is_initiator { - match constructor.maybe_send_message() { - Ok(message) => { - constructor.initiator_first_message = Some(message); - }, - Err(reason) => { - return Err(constructor.into_negotiation_error(reason)); - }, - } } - Ok(constructor) + } + + /// Instantiates a new `InteractiveTxConstructor` for the initiator (outbound splice). + /// + /// The initiator always has the shared funding output added internally, so preparing the + /// first message should never fail. Debug asserts verify this invariant. + pub fn new_for_outbound(args: InteractiveTxConstructorArgs) -> Self { + let mut constructor = Self::new(args, true); + match constructor.maybe_send_message() { + Ok(message) => constructor.initiator_first_message = Some(message), + Err(reason) => { + debug_assert!( + false, + "Outbound constructor should always have inputs: {:?}", + reason + ); + }, + } + constructor + } + + /// Instantiates a new `InteractiveTxConstructor` for the non-initiator (inbound splice or + /// dual-funded channel acceptor). + pub fn new_for_inbound(args: InteractiveTxConstructorArgs) -> Self { + Self::new(args, false) } fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { @@ -2438,84 +2447,62 @@ mod tests { &SecretKey::from_slice(&[43; 32]).unwrap(), ); - let mut constructor_a = match InteractiveTxConstructor::new(InteractiveTxConstructorArgs { - entropy_source, - channel_id, - feerate_sat_per_kw: TEST_FEERATE_SATS_PER_KW, - holder_node_id, - counterparty_node_id, - is_initiator: true, - funding_tx_locktime, - inputs_to_contribute: session.inputs_a, - shared_funding_input: session.a_shared_input.map(|(op, prev_output, lo)| { - SharedOwnedInput::new( - TxIn { - previous_output: op, - sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, - ..Default::default() - }, - prev_output, - lo, - true, // holder_sig_first - generate_funding_script_pubkey(), // witness_script for test - ) - }), - shared_funding_output: SharedOwnedOutput::new( - session.shared_output_a.0, - session.shared_output_a.1, - ), - outputs_to_contribute: session.outputs_a, - }) { - Ok(r) => Some(r), - Err(e) => { - assert_eq!( - Some((e.reason, ErrorCulprit::NodeA)), - session.expect_error, - "Test: {}", - session.description - ); - return; - }, - }; - let mut constructor_b = match InteractiveTxConstructor::new(InteractiveTxConstructorArgs { - entropy_source, - holder_node_id, - counterparty_node_id, - channel_id, - feerate_sat_per_kw: TEST_FEERATE_SATS_PER_KW, - is_initiator: false, - funding_tx_locktime, - inputs_to_contribute: session.inputs_b, - shared_funding_input: session.b_shared_input.map(|(op, prev_output, lo)| { - SharedOwnedInput::new( - TxIn { - previous_output: op, - sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, - ..Default::default() - }, - prev_output, - lo, - false, // holder_sig_first - generate_funding_script_pubkey(), // witness_script for test - ) - }), - shared_funding_output: SharedOwnedOutput::new( - session.shared_output_b.0, - session.shared_output_b.1, - ), - outputs_to_contribute: session.outputs_b, - }) { - Ok(r) => Some(r), - Err(e) => { - assert_eq!( - Some((e.reason, ErrorCulprit::NodeB)), - session.expect_error, - "Test: {}", - session.description - ); - return; - }, - }; + let mut constructor_a = + Some(InteractiveTxConstructor::new_for_outbound(InteractiveTxConstructorArgs { + entropy_source, + channel_id, + feerate_sat_per_kw: TEST_FEERATE_SATS_PER_KW, + holder_node_id, + counterparty_node_id, + funding_tx_locktime, + inputs_to_contribute: session.inputs_a, + shared_funding_input: session.a_shared_input.map(|(op, prev_output, lo)| { + SharedOwnedInput::new( + TxIn { + previous_output: op, + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + ..Default::default() + }, + prev_output, + lo, + true, // holder_sig_first + generate_funding_script_pubkey(), // witness_script for test + ) + }), + shared_funding_output: SharedOwnedOutput::new( + session.shared_output_a.0, + session.shared_output_a.1, + ), + outputs_to_contribute: session.outputs_a, + })); + let mut constructor_b = + Some(InteractiveTxConstructor::new_for_inbound(InteractiveTxConstructorArgs { + entropy_source, + holder_node_id, + counterparty_node_id, + channel_id, + feerate_sat_per_kw: TEST_FEERATE_SATS_PER_KW, + funding_tx_locktime, + inputs_to_contribute: session.inputs_b, + shared_funding_input: session.b_shared_input.map(|(op, prev_output, lo)| { + SharedOwnedInput::new( + TxIn { + previous_output: op, + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + ..Default::default() + }, + prev_output, + lo, + false, // holder_sig_first + generate_funding_script_pubkey(), // witness_script for test + ) + }), + shared_funding_output: SharedOwnedOutput::new( + session.shared_output_b.0, + session.shared_output_b.1, + ), + outputs_to_contribute: session.outputs_b, + })); let handle_message_send = |msg: InteractiveTxMessageSend, for_constructor: &mut InteractiveTxConstructor| { From 079bd5dc809f26057413a142bf810474a24179c3 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Feb 2026 10:25:41 -0600 Subject: [PATCH 05/13] f - remove initiator_first_message --- lightning/src/ln/channel.rs | 12 ++++++------ lightning/src/ln/interactivetxs.rs | 29 +++++++++++++---------------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index eae860b9669..9ff33f0f5e8 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -6689,7 +6689,7 @@ impl FundingNegotiationContext { fn into_interactive_tx_constructor( self, context: &ChannelContext, funding: &FundingScope, entropy_source: &ES, holder_node_id: PublicKey, - ) -> InteractiveTxConstructor { + ) -> (InteractiveTxConstructor, Option) { debug_assert_eq!( self.shared_funding_input.is_some(), funding.channel_transaction_parameters.splice_parent_funding_txid.is_some(), @@ -6724,7 +6724,7 @@ impl FundingNegotiationContext { if self.is_initiator { InteractiveTxConstructor::new_for_outbound(constructor_args) } else { - InteractiveTxConstructor::new_for_inbound(constructor_args) + (InteractiveTxConstructor::new_for_inbound(constructor_args), None) } } @@ -12431,14 +12431,14 @@ where our_funding_outputs: Vec::new(), }; - let mut interactive_tx_constructor = funding_negotiation_context + let (interactive_tx_constructor, first_message) = funding_negotiation_context .into_interactive_tx_constructor( &self.context, &splice_funding, entropy_source, holder_node_id.clone(), ); - debug_assert!(interactive_tx_constructor.take_initiator_first_message().is_none()); + debug_assert!(first_message.is_none()); // TODO(splicing): if quiescent_action is set, integrate what the user wants to do into the // counterparty-initiated splice. For always-on nodes this probably isn't a useful @@ -12490,14 +12490,14 @@ where panic!("We should have returned an error earlier!"); }; - let mut interactive_tx_constructor = funding_negotiation_context + let (interactive_tx_constructor, tx_msg_opt) = funding_negotiation_context .into_interactive_tx_constructor( &self.context, &splice_funding, entropy_source, holder_node_id.clone(), ); - let tx_msg_opt = interactive_tx_constructor.take_initiator_first_message(); + debug_assert!(tx_msg_opt.is_some()); debug_assert!(self.context.interactive_tx_signing_session.is_none()); diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 9f9a9ce487e..f7e0ce34346 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -1951,7 +1951,6 @@ impl InteractiveTxInput { pub(super) struct InteractiveTxConstructor { state_machine: StateMachine, is_initiator: bool, - initiator_first_message: Option, channel_id: ChannelId, inputs_to_contribute: Vec<(SerialId, InputOwned)>, outputs_to_contribute: Vec<(SerialId, OutputOwned)>, @@ -2111,7 +2110,6 @@ impl InteractiveTxConstructor { Self { state_machine, is_initiator, - initiator_first_message: None, channel_id, inputs_to_contribute, outputs_to_contribute, @@ -2124,19 +2122,22 @@ impl InteractiveTxConstructor { /// /// The initiator always has the shared funding output added internally, so preparing the /// first message should never fail. Debug asserts verify this invariant. - pub fn new_for_outbound(args: InteractiveTxConstructorArgs) -> Self { + pub fn new_for_outbound( + args: InteractiveTxConstructorArgs, + ) -> (Self, Option) { let mut constructor = Self::new(args, true); - match constructor.maybe_send_message() { - Ok(message) => constructor.initiator_first_message = Some(message), + let message = match constructor.maybe_send_message() { + Ok(message) => Some(message), Err(reason) => { debug_assert!( false, "Outbound constructor should always have inputs: {:?}", reason ); + None }, - } - constructor + }; + (constructor, message) } /// Instantiates a new `InteractiveTxConstructor` for the non-initiator (inbound splice or @@ -2188,10 +2189,6 @@ impl InteractiveTxConstructor { self.is_initiator } - pub fn take_initiator_first_message(&mut self) -> Option { - self.initiator_first_message.take() - } - fn maybe_send_message(&mut self) -> Result { let channel_id = self.channel_id; @@ -2447,8 +2444,8 @@ mod tests { &SecretKey::from_slice(&[43; 32]).unwrap(), ); - let mut constructor_a = - Some(InteractiveTxConstructor::new_for_outbound(InteractiveTxConstructorArgs { + let (constructor_a, mut message_send_a) = + InteractiveTxConstructor::new_for_outbound(InteractiveTxConstructorArgs { entropy_source, channel_id, feerate_sat_per_kw: TEST_FEERATE_SATS_PER_KW, @@ -2474,7 +2471,8 @@ mod tests { session.shared_output_a.1, ), outputs_to_contribute: session.outputs_a, - })); + }); + let mut constructor_a = Some(constructor_a); let mut constructor_b = Some(InteractiveTxConstructor::new_for_inbound(InteractiveTxConstructorArgs { entropy_source, @@ -2503,6 +2501,7 @@ mod tests { ), outputs_to_contribute: session.outputs_b, })); + let mut message_send_b = None; let handle_message_send = |msg: InteractiveTxMessageSend, for_constructor: &mut InteractiveTxConstructor| { @@ -2526,8 +2525,6 @@ mod tests { } }; - let mut message_send_a = constructor_a.as_mut().unwrap().take_initiator_first_message(); - let mut message_send_b = None; let mut final_tx_a = None; let mut final_tx_b = None; while constructor_a.is_some() || constructor_b.is_some() { From f786b8168b73add3d115b98a7a1730dba6163474 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 19 Feb 2026 15:19:12 -0600 Subject: [PATCH 06/13] Adjust FundingContribution for acceptor When constructing a FundingContribution, it's always assumed the estimated_fee is for when used as the initiator, who pays for the common fields and shared inputs / outputs. However, when the contribution is used as the acceptor, we'd be overpaying fees. Provide a method on FundingContribution that adjusts the fees and the change output, if possible. --- lightning/src/ln/funding.rs | 449 +++++++++++++++++++++++++++++++++++- 1 file changed, 447 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index dc29b23b1e3..9cda06ced1d 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -11,7 +11,9 @@ use bitcoin::hashes::Hash; use bitcoin::secp256k1::PublicKey; -use bitcoin::{Amount, FeeRate, OutPoint, ScriptBuf, SignedAmount, TxOut, WScriptHash, Weight}; +use bitcoin::{ + Amount, FeeRate, OutPoint, ScriptBuf, SignedAmount, TxOut, WPubkeyHash, WScriptHash, Weight, +}; use crate::ln::chan_utils::{ make_funding_redeemscript, BASE_INPUT_WEIGHT, EMPTY_SCRIPT_SIG_WEIGHT, @@ -313,6 +315,14 @@ impl FundingContribution { self.outputs.iter().chain(self.change_output.iter()) } + /// Returns the change output included in this contribution, if any. + /// + /// When coin selection provides more value than needed for the funding contribution and fees, + /// the surplus is returned to the wallet via this change output. + pub fn change_output(&self) -> Option<&TxOut> { + self.change_output.as_ref() + } + pub(super) fn into_tx_parts(self) -> (Vec, Vec) { let FundingContribution { inputs, mut outputs, change_output, .. } = self; @@ -405,11 +415,144 @@ impl FundingContribution { Ok(()) } + /// Computes the feerate adjustment as a pure `&self` operation, returning the new estimated + /// fee and optionally the new change output value. + /// + /// Returns `Ok((new_estimated_fee, new_change_value))` or `Err`: + /// - `(fee, Some(change))` — inputs with change: both should be updated + /// - `(fee, None)` — inputs without change (or change removed), or splice-out: fee updated + /// only + fn compute_feerate_adjustment( + &self, target_feerate: FeeRate, + ) -> Result<(Amount, Option), String> { + let is_splice = self.is_splice; + + if !self.inputs.is_empty() { + let budget = self.estimated_fee; + + if let Some(ref change_output) = self.change_output { + let old_change_value = change_output.value; + let dust_limit = change_output.script_pubkey.minimal_non_dust(); + + // Fair fee including the change output's weight. + let all_outputs: Vec = + self.outputs.iter().chain(self.change_output.iter()).cloned().collect(); + let fair_fee = estimate_transaction_fee( + &self.inputs, + &all_outputs, + false, + is_splice, + target_feerate, + ); + + let available = budget + .checked_add(old_change_value) + .ok_or("Budget plus change value overflow".to_string())?; + + match available.checked_sub(fair_fee) { + Some(new_change_value) if new_change_value >= dust_limit => { + Ok((fair_fee, Some(new_change_value))) + }, + _ => { + // Change would be below dust or negative. Try without change. + let fair_fee_no_change = estimate_transaction_fee( + &self.inputs, + &self.outputs, + false, + is_splice, + target_feerate, + ); + if available >= fair_fee_no_change { + Ok((fair_fee_no_change, None)) + } else { + Err(format!( + "Feerate too high: available fee budget {} insufficient for required fee {}", + available, fair_fee_no_change, + )) + } + }, + } + } else { + // No change output. + let fair_fee = estimate_transaction_fee( + &self.inputs, + &self.outputs, + false, + is_splice, + target_feerate, + ); + if budget < fair_fee { + return Err(format!( + "Feerate too high: fee budget {} insufficient for required fee {}", + budget, fair_fee, + )); + } + let surplus = budget - fair_fee; + let dust_limit = + ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()).minimal_non_dust(); + if surplus >= dust_limit { + return Err(format!( + "Fee surplus {} exceeds dust limit {}; cannot burn without change output", + surplus, dust_limit, + )); + } + Ok((fair_fee, None)) + } + } else { + // No inputs (splice-out): fees paid from channel balance. + let fair_fee = + estimate_transaction_fee(&[], &self.outputs, false, is_splice, target_feerate); + if self.estimated_fee < fair_fee { + return Err(format!( + "Feerate too high: estimated fee {} insufficient for required fee {}", + self.estimated_fee, fair_fee, + )); + } + // Surplus goes back to the channel balance. + Ok((fair_fee, None)) + } + } + + /// Adjusts the contribution's change output for the initiator's feerate. + /// + /// When the acceptor has a pending contribution (from the quiescence tie-breaker scenario), + /// the initiator's proposed feerate may differ from the feerate used during coin selection. + /// This adjusts the change output so the acceptor pays their fair share at the target + /// feerate. + pub(super) fn for_acceptor_at_feerate(mut self, feerate: FeeRate) -> Result { + let (new_estimated_fee, new_change) = self.compute_feerate_adjustment(feerate)?; + match new_change { + Some(value) => self.change_output.as_mut().unwrap().value = value, + None => self.change_output = None, + } + self.estimated_fee = new_estimated_fee; + self.feerate = feerate; + Ok(self) + } + + /// Returns the net value at the given target feerate without mutating `self`. + /// + /// This serves double duty: it checks feerate compatibility (returning `Err` if the feerate + /// can't be accommodated) and computes the adjusted net value (returning `Ok` with the value + /// accounting for the target feerate). + pub(super) fn net_value_for_acceptor_at_feerate( + &self, target_feerate: FeeRate, + ) -> Result { + let (new_estimated_fee, _) = self.compute_feerate_adjustment(target_feerate)?; + Ok(self.net_value_with_fee(new_estimated_fee)) + } + /// The net value contributed to a channel by the splice. If negative, more value will be /// spliced out than spliced in. Fees will be deducted from the expected splice-out amount /// if no inputs were included. pub fn net_value(&self) -> SignedAmount { - let unpaid_fees = if self.inputs.is_empty() { self.estimated_fee } else { Amount::ZERO } + self.net_value_with_fee(self.estimated_fee) + } + + /// Computes the net value using the given `estimated_fee` for the splice-out (no inputs) + /// case. For splice-in, fees are paid by inputs so `estimated_fee` is not deducted. + fn net_value_with_fee(&self, estimated_fee: Amount) -> SignedAmount { + let unpaid_fees = if self.inputs.is_empty() { estimated_fee } else { Amount::ZERO } .to_signed() .expect("estimated_fee is validated to not exceed Amount::MAX_MONEY"); let value_added = self @@ -749,4 +892,306 @@ mod tests { .is_err()); } } + + #[test] + fn test_for_acceptor_at_feerate_higher_change_adjusted() { + // Splice-in: higher target feerate reduces the change output. + // The budget (is_initiator=true) overestimates by including common TX fields, + // shared output, and shared input weight. So we need a sufficiently high target + // feerate for the acceptor's fair fee to exceed the budget, causing the change + // to decrease. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(5000); + let input = funding_input_sats(100_000); + let change = funding_output_sats(10_000); + + // Budget computed as initiator (overestimate, without change output weight). + let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + + let contribution = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee: budget, + inputs: vec![input.clone()], + outputs: vec![], + change_output: Some(change.clone()), + feerate: original_feerate, + is_splice: true, + }; + + let net_value_before = contribution.net_value(); + let contribution = contribution.for_acceptor_at_feerate(target_feerate).unwrap(); + + // Fair fee at target feerate for acceptor (is_initiator=false), including change weight. + let expected_fair_fee = + estimate_transaction_fee(&[input], &[change], false, true, target_feerate); + let expected_change = budget + Amount::from_sat(10_000) - expected_fair_fee; + + assert_eq!(contribution.estimated_fee, expected_fair_fee); + assert!(contribution.change_output.is_some()); + assert_eq!(contribution.change_output.as_ref().unwrap().value, expected_change); + assert!(expected_change < Amount::from_sat(10_000)); // Change reduced + assert_eq!(contribution.net_value(), net_value_before); + } + + #[test] + fn test_for_acceptor_at_feerate_lower_change_increased() { + // Splice-in: lower target feerate increases the change output. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(1000); + let input = funding_input_sats(100_000); + let change = funding_output_sats(10_000); + + let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + + let contribution = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee: budget, + inputs: vec![input.clone()], + outputs: vec![], + change_output: Some(change.clone()), + feerate: original_feerate, + is_splice: true, + }; + + let net_value_before = contribution.net_value(); + let contribution = contribution.for_acceptor_at_feerate(target_feerate).unwrap(); + + let expected_fair_fee = + estimate_transaction_fee(&[input], &[change], false, true, target_feerate); + let expected_change = budget + Amount::from_sat(10_000) - expected_fair_fee; + + assert_eq!(contribution.estimated_fee, expected_fair_fee); + assert!(contribution.change_output.is_some()); + assert_eq!(contribution.change_output.as_ref().unwrap().value, expected_change); + assert!(expected_change > Amount::from_sat(10_000)); // Change increased + assert_eq!(contribution.net_value(), net_value_before); + } + + #[test] + fn test_for_acceptor_at_feerate_change_removed() { + // Splice-in: feerate high enough that change drops below dust and is removed, + // but budget + change still covers the fee without the change output. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(7000); + let input = funding_input_sats(100_000); + let change = funding_output_sats(500); + + let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + + let contribution = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee: budget, + inputs: vec![input.clone()], + outputs: vec![], + change_output: Some(change), + feerate: original_feerate, + is_splice: true, + }; + + let net_value_before = contribution.net_value(); + let contribution = contribution.for_acceptor_at_feerate(target_feerate).unwrap(); + + // Change should be removed; estimated_fee updated to no-change fair fee. + assert!(contribution.change_output.is_none()); + let expected_fee_no_change = + estimate_transaction_fee(&[input], &[], false, true, target_feerate); + assert_eq!(contribution.estimated_fee, expected_fee_no_change); + assert_eq!(contribution.net_value(), net_value_before); + } + + #[test] + fn test_for_acceptor_at_feerate_too_high_rejected() { + // Splice-in: feerate so high that even without change, the fee can't be covered. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(100_000); + let input = funding_input_sats(100_000); + let change = funding_output_sats(500); + + let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + + let contribution = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee: budget, + inputs: vec![input], + outputs: vec![], + change_output: Some(change), + feerate: original_feerate, + is_splice: true, + }; + + let result = contribution.for_acceptor_at_feerate(target_feerate); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("Feerate too high")); + } + + #[test] + fn test_for_acceptor_at_feerate_splice_out_sufficient() { + // Splice-out (no inputs): budget from is_initiator=true overestimate covers the + // acceptor's fair fee at a moderately higher target feerate. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(3000); + let output = funding_output_sats(50_000); + + let budget = estimate_transaction_fee(&[], &[output.clone()], true, true, original_feerate); + + let contribution = FundingContribution { + value_added: Amount::ZERO, + estimated_fee: budget, + inputs: vec![], + outputs: vec![output.clone()], + change_output: None, + feerate: original_feerate, + is_splice: true, + }; + + let contribution = contribution.for_acceptor_at_feerate(target_feerate).unwrap(); + // estimated_fee is updated to the fair fee; surplus goes back to channel balance. + let expected_fair_fee = + estimate_transaction_fee(&[], &[output], false, true, target_feerate); + assert_eq!(contribution.estimated_fee, expected_fair_fee); + assert!(expected_fair_fee <= budget); + } + + #[test] + fn test_for_acceptor_at_feerate_splice_out_insufficient() { + // Splice-out: target feerate too high for the is_initiator=true budget. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(50_000); + let output = funding_output_sats(50_000); + + let budget = estimate_transaction_fee(&[], &[output.clone()], true, true, original_feerate); + + let contribution = FundingContribution { + value_added: Amount::ZERO, + estimated_fee: budget, + inputs: vec![], + outputs: vec![output], + change_output: None, + feerate: original_feerate, + is_splice: true, + }; + + let result = contribution.for_acceptor_at_feerate(target_feerate); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("Feerate too high")); + } + + #[test] + fn test_net_value_for_acceptor_at_feerate_splice_in() { + // Splice-in: net_value_for_acceptor_at_feerate returns the same value as net_value() since + // splice-in fees are paid by inputs, not from channel balance. + 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 change = funding_output_sats(10_000); + + let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + + let contribution = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee: budget, + inputs: vec![input], + outputs: vec![], + change_output: Some(change), + feerate: original_feerate, + is_splice: true, + }; + + // For splice-in, unpaid_fees is zero so net_value_for_acceptor_at_feerate equals net_value. + let net_at_feerate = + contribution.net_value_for_acceptor_at_feerate(target_feerate).unwrap(); + assert_eq!(net_at_feerate, contribution.net_value()); + assert_eq!(net_at_feerate, Amount::from_sat(50_000).to_signed().unwrap()); + } + + #[test] + fn test_net_value_for_acceptor_at_feerate_splice_out() { + // Splice-out: net_value_for_acceptor_at_feerate returns the adjusted value using the fair fee + // at the target feerate. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(3000); + let output = funding_output_sats(50_000); + + let budget = estimate_transaction_fee(&[], &[output.clone()], true, true, original_feerate); + + let contribution = FundingContribution { + value_added: Amount::ZERO, + estimated_fee: budget, + inputs: vec![], + outputs: vec![output.clone()], + change_output: None, + feerate: original_feerate, + is_splice: true, + }; + + let net_at_feerate = + contribution.net_value_for_acceptor_at_feerate(target_feerate).unwrap(); + + // The fair fee at target feerate should be less than the initiator's budget. + let fair_fee = estimate_transaction_fee(&[], &[output], false, true, target_feerate); + let expected_net = SignedAmount::ZERO + - Amount::from_sat(50_000).to_signed().unwrap() + - fair_fee.to_signed().unwrap(); + assert_eq!(net_at_feerate, expected_net); + + // Should be less negative than net_value() which uses the higher budget. + assert!(net_at_feerate > contribution.net_value()); + } + + #[test] + fn test_net_value_for_acceptor_at_feerate_does_not_mutate() { + // Verify net_value_for_acceptor_at_feerate does not modify the contribution. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(5000); + let input = funding_input_sats(100_000); + let change = funding_output_sats(10_000); + + let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + + let contribution = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee: budget, + inputs: vec![input], + outputs: vec![], + change_output: Some(change), + feerate: original_feerate, + is_splice: true, + }; + + let net_before = contribution.net_value(); + 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); + + // Nothing should have changed. + assert_eq!(contribution.net_value(), net_before); + assert_eq!(contribution.estimated_fee, fee_before); + assert_eq!(contribution.change_output.as_ref().unwrap().value, change_before); + } + + #[test] + fn test_net_value_for_acceptor_at_feerate_too_high() { + // net_value_for_acceptor_at_feerate returns Err when feerate can't be accommodated. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let target_feerate = FeeRate::from_sat_per_kwu(100_000); + let input = funding_input_sats(100_000); + let change = funding_output_sats(500); + + let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + + let contribution = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee: budget, + inputs: vec![input], + outputs: vec![], + change_output: Some(change), + feerate: original_feerate, + is_splice: true, + }; + + let result = contribution.net_value_for_acceptor_at_feerate(target_feerate); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("Feerate too high")); + } } From 353183987eae893e3cbd161f1d4486355a545de3 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Feb 2026 11:21:27 -0600 Subject: [PATCH 07/13] f - rewrite docs --- lightning/src/ln/funding.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 9cda06ced1d..c57a6aeae90 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -415,13 +415,14 @@ impl FundingContribution { Ok(()) } - /// Computes the feerate adjustment as a pure `&self` operation, returning the new estimated - /// fee and optionally the new change output value. + /// Computes the adjusted fee and change output value for the acceptor at the initiator's + /// proposed feerate, which may differ from the feerate used during coin selection. /// - /// Returns `Ok((new_estimated_fee, new_change_value))` or `Err`: - /// - `(fee, Some(change))` — inputs with change: both should be updated - /// - `(fee, None)` — inputs without change (or change removed), or splice-out: fee updated - /// only + /// On success, returns the new estimated fee and, if applicable, the new change output value: + /// - `Some(change)` — the adjusted change output value + /// - `None` — no change output (no inputs or change fell below dust) + /// + /// Returns `Err` if the contribution cannot accommodate the target feerate. fn compute_feerate_adjustment( &self, target_feerate: FeeRate, ) -> Result<(Amount, Option), String> { From bfa869957ab759b3fd3ecce1eb9b2a62091b2012 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Feb 2026 15:52:37 -0600 Subject: [PATCH 08/13] f - Add min/max feerate parameters to splice funding APIs Add FeeRateAdjustmentError enum with TooLow, TooHigh, and BudgetInsufficient variants to distinguish different feerate incompatibility scenarios. Add max_feerate field to FundingTemplate and FundingContribution. Update compute_feerate_adjustment to check feerate bounds: - Target below min_feerate (coin selection rate) -> TooLow - Target above max_feerate with fair_fee > estimated_fee -> TooHigh - Target above max_feerate but fair_fee <= estimated_fee -> allowed (change output not consumed, acceptor pays less than as initiator) - Budget insufficient within acceptable range -> BudgetInsufficient Change splice_channel and rbf_channel signatures from feerate -> (min_feerate, max_feerate). Coin selection uses min_feerate; as initiator, min_feerate is proposed in splice_init. Co-Authored-By: Claude Opus 4.6 --- lightning/src/ln/channel.rs | 17 +- lightning/src/ln/channelmanager.rs | 5 +- lightning/src/ln/funding.rs | 338 +++++++++++++++++++++++------ lightning/src/ln/splicing_tests.rs | 71 +++--- 4 files changed, 338 insertions(+), 93 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9ff33f0f5e8..93af6ad6adf 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12052,7 +12052,9 @@ where } /// Initiate splicing. - pub fn splice_channel(&self, feerate: FeeRate) -> Result { + pub fn splice_channel( + &self, min_feerate: FeeRate, max_feerate: FeeRate, + ) -> Result { if self.holder_commitment_point.current_point().is_none() { return Err(APIError::APIMisuseError { err: format!( @@ -12094,6 +12096,17 @@ where }); } + if min_feerate > max_feerate { + return Err(APIError::APIMisuseError { + err: format!( + "Channel {} min_feerate {} exceeds max_feerate {}", + self.context.channel_id(), + min_feerate, + max_feerate, + ), + }); + } + let funding_txo = self.funding.get_funding_txo().expect("funding_txo should be set"); let previous_utxo = self.funding.get_funding_output().expect("funding_output should be set"); @@ -12103,7 +12116,7 @@ where satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + FUNDING_TRANSACTION_WITNESS_WEIGHT, }; - Ok(FundingTemplate::new(Some(shared_input), feerate)) + Ok(FundingTemplate::new(Some(shared_input), min_feerate, max_feerate)) } pub fn funding_contributed( diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 525a764fd33..ebec3768d15 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4670,7 +4670,8 @@ impl< /// [`FundingContribution`]: crate::ln::funding::FundingContribution #[rustfmt::skip] pub fn splice_channel( - &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, feerate: FeeRate, + &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, + min_feerate: FeeRate, max_feerate: FeeRate, ) -> Result { let per_peer_state = self.per_peer_state.read().unwrap(); @@ -4698,7 +4699,7 @@ impl< match peer_state.channel_by_id.entry(*channel_id) { hash_map::Entry::Occupied(chan_phase_entry) => { if let Some(chan) = chan_phase_entry.get().as_funded() { - chan.splice_channel(feerate) + chan.splice_channel(min_feerate, max_feerate) } else { Err(APIError::ChannelUnavailable { err: format!( diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index c57a6aeae90..1c53e5bf50a 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -29,6 +29,31 @@ use crate::util::wallet_utils::{ CoinSelection, CoinSelectionSource, CoinSelectionSourceSync, Input, }; +/// Error returned when the acceptor's contribution cannot accommodate the initiator's proposed +/// feerate. +#[derive(Debug)] +pub(super) enum FeeRateAdjustmentError { + /// Target feerate is below our minimum. The counterparty's splice can proceed without our + /// contribution; we'll retry via RBF at our preferred feerate. + TooLow(String), + /// Target feerate is above our maximum and would consume too much of the change output. The + /// splice should be rejected. + TooHigh(String), + /// Target feerate is within our acceptable range but our UTXO budget is insufficient to + /// cover the required fees. The counterparty's splice can proceed without our contribution. + BudgetInsufficient(String), +} + +impl core::fmt::Display for FeeRateAdjustmentError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + FeeRateAdjustmentError::TooLow(msg) => write!(f, "{}", msg), + FeeRateAdjustmentError::TooHigh(msg) => write!(f, "{}", msg), + FeeRateAdjustmentError::BudgetInsufficient(msg) => write!(f, "{}", msg), + } + } +} + /// 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 @@ -44,23 +69,29 @@ pub struct FundingTemplate { /// transaction. shared_input: Option, - /// The fee rate to use for coin selection. - feerate: FeeRate, + /// The minimum fee rate to use for coin selection and to propose as initiator. + min_feerate: FeeRate, + + /// The maximum fee rate to accept as acceptor before rejecting the splice. + max_feerate: FeeRate, } impl FundingTemplate { /// Constructs a [`FundingTemplate`] for a splice using the provided shared input. - pub(super) fn new(shared_input: Option, feerate: FeeRate) -> Self { - Self { shared_input, feerate } + pub(super) fn new( + shared_input: Option, min_feerate: FeeRate, max_feerate: FeeRate, + ) -> Self { + Self { shared_input, min_feerate, max_feerate } } } macro_rules! build_funding_contribution { - ($value_added:expr, $outputs:expr, $shared_input:expr, $feerate:expr, $wallet:ident, $($await:tt)*) => {{ + ($value_added:expr, $outputs:expr, $shared_input:expr, $feerate:expr, $max_feerate:expr, $wallet:ident, $($await:tt)*) => {{ let value_added: Amount = $value_added; let outputs: Vec = $outputs; let shared_input: Option = $shared_input; let feerate: FeeRate = $feerate; + let max_feerate: FeeRate = $max_feerate; // Validate user-provided amounts are within MAX_MONEY before coin selection to // ensure FundingContribution::net_value() arithmetic cannot overflow. With all @@ -128,6 +159,7 @@ macro_rules! build_funding_contribution { outputs, change_output, feerate, + max_feerate, is_splice, }; @@ -144,8 +176,8 @@ impl FundingTemplate { if value_added == Amount::ZERO { return Err(()); } - let FundingTemplate { shared_input, feerate } = self; - build_funding_contribution!(value_added, vec![], shared_input, feerate, wallet, await) + let FundingTemplate { shared_input, min_feerate, max_feerate } = self; + build_funding_contribution!(value_added, vec![], shared_input, min_feerate, max_feerate, wallet, await) } /// Creates a [`FundingContribution`] for adding funds to a channel using `wallet` to perform @@ -156,8 +188,15 @@ impl FundingTemplate { if value_added == Amount::ZERO { return Err(()); } - let FundingTemplate { shared_input, feerate } = self; - build_funding_contribution!(value_added, vec![], shared_input, feerate, wallet,) + let FundingTemplate { shared_input, min_feerate, max_feerate } = self; + build_funding_contribution!( + value_added, + vec![], + shared_input, + min_feerate, + max_feerate, + wallet, + ) } /// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to @@ -168,8 +207,8 @@ impl FundingTemplate { if outputs.is_empty() { return Err(()); } - let FundingTemplate { shared_input, feerate } = self; - build_funding_contribution!(Amount::ZERO, outputs, shared_input, feerate, wallet, await) + let FundingTemplate { shared_input, min_feerate, max_feerate } = self; + build_funding_contribution!(Amount::ZERO, outputs, shared_input, min_feerate, max_feerate, wallet, await) } /// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to @@ -180,8 +219,15 @@ impl FundingTemplate { if outputs.is_empty() { return Err(()); } - let FundingTemplate { shared_input, feerate } = self; - build_funding_contribution!(Amount::ZERO, outputs, shared_input, feerate, wallet,) + let FundingTemplate { shared_input, min_feerate, max_feerate } = self; + build_funding_contribution!( + Amount::ZERO, + outputs, + shared_input, + min_feerate, + max_feerate, + wallet, + ) } /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using @@ -192,8 +238,8 @@ impl FundingTemplate { if value_added == Amount::ZERO && outputs.is_empty() { return Err(()); } - let FundingTemplate { shared_input, feerate } = self; - build_funding_contribution!(value_added, outputs, shared_input, feerate, wallet, await) + let FundingTemplate { shared_input, min_feerate, max_feerate } = self; + build_funding_contribution!(value_added, outputs, shared_input, min_feerate, max_feerate, wallet, await) } /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using @@ -204,8 +250,15 @@ impl FundingTemplate { if value_added == Amount::ZERO && outputs.is_empty() { return Err(()); } - let FundingTemplate { shared_input, feerate } = self; - build_funding_contribution!(value_added, outputs, shared_input, feerate, wallet,) + let FundingTemplate { shared_input, min_feerate, max_feerate } = self; + build_funding_contribution!( + value_added, + outputs, + shared_input, + min_feerate, + max_feerate, + wallet, + ) } } @@ -281,9 +334,12 @@ pub struct FundingContribution { /// The output where any change will be sent. change_output: Option, - /// The fee rate used to select `inputs`. + /// The fee rate used to select `inputs` (the minimum feerate). feerate: FeeRate, + /// The maximum fee rate to accept as acceptor before rejecting the splice. + max_feerate: FeeRate, + /// Whether the contribution is for funding a splice. is_splice: bool, } @@ -296,6 +352,7 @@ impl_writeable_tlv_based!(FundingContribution, { (9, change_output, option), (11, feerate, required), (13, is_splice, required), + (15, max_feerate, required), }); impl FundingContribution { @@ -425,7 +482,34 @@ impl FundingContribution { /// Returns `Err` if the contribution cannot accommodate the target feerate. fn compute_feerate_adjustment( &self, target_feerate: FeeRate, - ) -> Result<(Amount, Option), String> { + ) -> Result<(Amount, Option), FeeRateAdjustmentError> { + if target_feerate < self.feerate { + return Err(FeeRateAdjustmentError::TooLow(format!( + "Target feerate {} is below our minimum {}", + target_feerate, self.feerate, + ))); + } + + if target_feerate > self.max_feerate { + // Check if the acceptor's fair fee exceeds their estimated fee (budget). + // If fair_fee <= estimated_fee, the change output isn't consumed (it may even + // grow), so we allow it despite exceeding max_feerate. + let fair_fee = estimate_transaction_fee( + &self.inputs, + &self.outputs, + false, + self.is_splice, + target_feerate, + ); + if fair_fee > self.estimated_fee { + return Err(FeeRateAdjustmentError::TooHigh(format!( + "Target feerate {} exceeds our maximum {} and fair fee {} exceeds budget {}", + target_feerate, self.max_feerate, fair_fee, self.estimated_fee, + ))); + } + // Fall through: fair_fee <= estimated_fee, change not consumed + } + let is_splice = self.is_splice; if !self.inputs.is_empty() { @@ -446,9 +530,11 @@ impl FundingContribution { target_feerate, ); - let available = budget - .checked_add(old_change_value) - .ok_or("Budget plus change value overflow".to_string())?; + let available = budget.checked_add(old_change_value).ok_or_else(|| { + FeeRateAdjustmentError::BudgetInsufficient( + "Budget plus change value overflow".to_string(), + ) + })?; match available.checked_sub(fair_fee) { Some(new_change_value) if new_change_value >= dust_limit => { @@ -466,10 +552,10 @@ impl FundingContribution { if available >= fair_fee_no_change { Ok((fair_fee_no_change, None)) } else { - Err(format!( - "Feerate too high: available fee budget {} insufficient for required fee {}", + Err(FeeRateAdjustmentError::BudgetInsufficient(format!( + "Available fee budget {} insufficient for required fee {}", available, fair_fee_no_change, - )) + ))) } }, } @@ -483,19 +569,19 @@ impl FundingContribution { target_feerate, ); if budget < fair_fee { - return Err(format!( - "Feerate too high: fee budget {} insufficient for required fee {}", + return Err(FeeRateAdjustmentError::BudgetInsufficient(format!( + "Fee budget {} insufficient for required fee {}", budget, fair_fee, - )); + ))); } let surplus = budget - fair_fee; let dust_limit = ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()).minimal_non_dust(); if surplus >= dust_limit { - return Err(format!( + return Err(FeeRateAdjustmentError::BudgetInsufficient(format!( "Fee surplus {} exceeds dust limit {}; cannot burn without change output", surplus, dust_limit, - )); + ))); } Ok((fair_fee, None)) } @@ -504,10 +590,10 @@ impl FundingContribution { let fair_fee = estimate_transaction_fee(&[], &self.outputs, false, is_splice, target_feerate); if self.estimated_fee < fair_fee { - return Err(format!( - "Feerate too high: estimated fee {} insufficient for required fee {}", + return Err(FeeRateAdjustmentError::BudgetInsufficient(format!( + "Estimated fee {} insufficient for required fee {}", self.estimated_fee, fair_fee, - )); + ))); } // Surplus goes back to the channel balance. Ok((fair_fee, None)) @@ -520,7 +606,9 @@ impl FundingContribution { /// the initiator's proposed feerate may differ from the feerate used during coin selection. /// This adjusts the change output so the acceptor pays their fair share at the target /// feerate. - pub(super) fn for_acceptor_at_feerate(mut self, feerate: FeeRate) -> Result { + pub(super) fn for_acceptor_at_feerate( + mut self, feerate: FeeRate, + ) -> Result { let (new_estimated_fee, new_change) = self.compute_feerate_adjustment(feerate)?; match new_change { Some(value) => self.change_output.as_mut().unwrap().value = value, @@ -538,7 +626,7 @@ impl FundingContribution { /// accounting for the target feerate). pub(super) fn net_value_for_acceptor_at_feerate( &self, target_feerate: FeeRate, - ) -> Result { + ) -> Result { let (new_estimated_fee, _) = self.compute_feerate_adjustment(target_feerate)?; Ok(self.net_value_with_fee(new_estimated_fee)) } @@ -581,7 +669,10 @@ pub type FundingTxInput = crate::util::wallet_utils::ConfirmedUtxo; #[cfg(test)] mod tests { - use super::{estimate_transaction_fee, FundingContribution, FundingTemplate, FundingTxInput}; + use super::{ + estimate_transaction_fee, FeeRateAdjustmentError, FundingContribution, FundingTemplate, + FundingTxInput, + }; use crate::chain::ClaimId; use crate::util::wallet_utils::{CoinSelection, CoinSelectionSourceSync, Input}; use bitcoin::hashes::Hash; @@ -675,6 +766,7 @@ mod tests { change_output: None, is_splice: true, feerate: FeeRate::from_sat_per_kwu(2000), + max_feerate: FeeRate::MAX, }; assert!(contribution.validate().is_ok()); assert_eq!(contribution.net_value(), contribution.value_added.to_signed().unwrap()); @@ -696,6 +788,7 @@ mod tests { change_output: None, is_splice: true, feerate: FeeRate::from_sat_per_kwu(2000), + max_feerate: FeeRate::MAX, }; assert!(contribution.validate().is_ok()); assert_eq!(contribution.net_value(), SignedAmount::from_sat(220_000 - 200_000)); @@ -717,6 +810,7 @@ mod tests { change_output: None, is_splice: true, feerate: FeeRate::from_sat_per_kwu(2000), + max_feerate: FeeRate::MAX, }; assert!(contribution.validate().is_ok()); assert_eq!(contribution.net_value(), SignedAmount::from_sat(220_000 - 400_000)); @@ -738,6 +832,7 @@ mod tests { change_output: None, is_splice: true, feerate: FeeRate::from_sat_per_kwu(90000), + max_feerate: FeeRate::MAX, }; assert_eq!( contribution.validate(), @@ -761,6 +856,7 @@ mod tests { change_output: None, is_splice: true, feerate: FeeRate::from_sat_per_kwu(2000), + max_feerate: FeeRate::MAX, }; assert_eq!( contribution.validate(), @@ -785,6 +881,7 @@ mod tests { change_output: None, is_splice: true, feerate: FeeRate::from_sat_per_kwu(2000), + max_feerate: FeeRate::MAX, }; assert!(contribution.validate().is_ok()); assert_eq!(contribution.net_value(), contribution.value_added.to_signed().unwrap()); @@ -804,6 +901,7 @@ mod tests { change_output: None, is_splice: true, feerate: FeeRate::from_sat_per_kwu(2200), + max_feerate: FeeRate::MAX, }; assert_eq!( contribution.validate(), @@ -828,6 +926,7 @@ mod tests { change_output: None, is_splice: false, feerate: FeeRate::from_sat_per_kwu(2000), + max_feerate: FeeRate::MAX, }; assert!(contribution.validate().is_ok()); assert_eq!(contribution.net_value(), contribution.value_added.to_signed().unwrap()); @@ -855,20 +954,20 @@ mod tests { // splice_in_sync with value_added > MAX_MONEY { - let template = FundingTemplate::new(None, feerate); + let template = FundingTemplate::new(None, feerate, feerate); assert!(template.splice_in_sync(over_max, UnreachableWallet).is_err()); } // splice_out_sync with single output value > MAX_MONEY { - let template = FundingTemplate::new(None, feerate); + let template = FundingTemplate::new(None, feerate, feerate); let outputs = vec![funding_output_sats(over_max.to_sat())]; assert!(template.splice_out_sync(outputs, UnreachableWallet).is_err()); } // splice_out_sync with multiple outputs summing > MAX_MONEY { - let template = FundingTemplate::new(None, feerate); + let template = FundingTemplate::new(None, feerate, feerate); let half_over = Amount::MAX_MONEY / 2 + Amount::from_sat(1); let outputs = vec![ funding_output_sats(half_over.to_sat()), @@ -879,14 +978,14 @@ mod tests { // splice_in_and_out_sync with value_added > MAX_MONEY { - let template = FundingTemplate::new(None, feerate); + let template = FundingTemplate::new(None, feerate, feerate); let outputs = vec![funding_output_sats(1_000)]; assert!(template.splice_in_and_out_sync(over_max, outputs, UnreachableWallet).is_err()); } // splice_in_and_out_sync with output sum > MAX_MONEY { - let template = FundingTemplate::new(None, feerate); + let template = FundingTemplate::new(None, feerate, feerate); let outputs = vec![funding_output_sats(over_max.to_sat())]; assert!(template .splice_in_and_out_sync(Amount::from_sat(1_000), outputs, UnreachableWallet) @@ -917,6 +1016,7 @@ mod tests { change_output: Some(change.clone()), feerate: original_feerate, is_splice: true, + max_feerate: FeeRate::MAX, }; let net_value_before = contribution.net_value(); @@ -935,8 +1035,8 @@ mod tests { } #[test] - fn test_for_acceptor_at_feerate_lower_change_increased() { - // Splice-in: lower target feerate increases the change output. + fn test_for_acceptor_at_feerate_lower_rejected_too_low() { + // Splice-in: target feerate below our minimum is rejected as TooLow. let original_feerate = FeeRate::from_sat_per_kwu(2000); let target_feerate = FeeRate::from_sat_per_kwu(1000); let input = funding_input_sats(100_000); @@ -947,25 +1047,16 @@ mod tests { let contribution = FundingContribution { value_added: Amount::from_sat(50_000), estimated_fee: budget, - inputs: vec![input.clone()], + inputs: vec![input], outputs: vec![], - change_output: Some(change.clone()), + change_output: Some(change), feerate: original_feerate, is_splice: true, + max_feerate: FeeRate::MAX, }; - let net_value_before = contribution.net_value(); - let contribution = contribution.for_acceptor_at_feerate(target_feerate).unwrap(); - - let expected_fair_fee = - estimate_transaction_fee(&[input], &[change], false, true, target_feerate); - let expected_change = budget + Amount::from_sat(10_000) - expected_fair_fee; - - assert_eq!(contribution.estimated_fee, expected_fair_fee); - assert!(contribution.change_output.is_some()); - assert_eq!(contribution.change_output.as_ref().unwrap().value, expected_change); - assert!(expected_change > Amount::from_sat(10_000)); // Change increased - assert_eq!(contribution.net_value(), net_value_before); + let result = contribution.for_acceptor_at_feerate(target_feerate); + assert!(matches!(result, Err(FeeRateAdjustmentError::TooLow(_)))); } #[test] @@ -987,6 +1078,7 @@ mod tests { change_output: Some(change), feerate: original_feerate, is_splice: true, + max_feerate: FeeRate::MAX, }; let net_value_before = contribution.net_value(); @@ -1018,11 +1110,11 @@ mod tests { change_output: Some(change), feerate: original_feerate, is_splice: true, + max_feerate: FeeRate::MAX, }; let result = contribution.for_acceptor_at_feerate(target_feerate); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("Feerate too high")); + assert!(matches!(result, Err(FeeRateAdjustmentError::BudgetInsufficient(_)))); } #[test] @@ -1043,6 +1135,7 @@ mod tests { change_output: None, feerate: original_feerate, is_splice: true, + max_feerate: FeeRate::MAX, }; let contribution = contribution.for_acceptor_at_feerate(target_feerate).unwrap(); @@ -1070,11 +1163,11 @@ mod tests { change_output: None, feerate: original_feerate, is_splice: true, + max_feerate: FeeRate::MAX, }; let result = contribution.for_acceptor_at_feerate(target_feerate); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("Feerate too high")); + assert!(matches!(result, Err(FeeRateAdjustmentError::BudgetInsufficient(_)))); } #[test] @@ -1096,6 +1189,7 @@ mod tests { change_output: Some(change), feerate: original_feerate, is_splice: true, + max_feerate: FeeRate::MAX, }; // For splice-in, unpaid_fees is zero so net_value_for_acceptor_at_feerate equals net_value. @@ -1123,6 +1217,7 @@ mod tests { change_output: None, feerate: original_feerate, is_splice: true, + max_feerate: FeeRate::MAX, }; let net_at_feerate = @@ -1157,6 +1252,7 @@ mod tests { change_output: Some(change), feerate: original_feerate, is_splice: true, + max_feerate: FeeRate::MAX, }; let net_before = contribution.net_value(); @@ -1189,10 +1285,126 @@ mod tests { change_output: Some(change), feerate: original_feerate, is_splice: true, + max_feerate: FeeRate::MAX, }; let result = contribution.net_value_for_acceptor_at_feerate(target_feerate); - assert!(result.is_err()); - assert!(result.unwrap_err().contains("Feerate too high")); + assert!(matches!(result, Err(FeeRateAdjustmentError::BudgetInsufficient(_)))); + } + + #[test] + fn test_for_acceptor_at_feerate_exceeds_max_rejected() { + // Splice-in: target feerate exceeds max_feerate and fair fee exceeds budget, + // so the adjustment is rejected as TooHigh. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let max_feerate = FeeRate::from_sat_per_kwu(3000); + let target_feerate = FeeRate::from_sat_per_kwu(100_000); + let input = funding_input_sats(100_000); + let change = funding_output_sats(10_000); + + let budget = estimate_transaction_fee( + &[input.clone()], + &[], + true, + true, + original_feerate, + ); + + let contribution = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee: budget, + inputs: vec![input], + outputs: vec![], + change_output: Some(change), + feerate: original_feerate, + is_splice: true, + max_feerate, + }; + + let result = contribution.for_acceptor_at_feerate(target_feerate); + assert!(matches!(result, Err(FeeRateAdjustmentError::TooHigh(_)))); + } + + #[test] + fn test_for_acceptor_at_feerate_exceeds_max_allowed() { + // Splice-in: target feerate exceeds max_feerate but the acceptor's fair fee + // (is_initiator=false at target) is less than the budget (is_initiator=true at + // original feerate). This works because the initiator budget includes ~598 WU of + // extra weight (common TX fields, funding output, shared input) that the acceptor + // doesn't pay for, so the budget is ~2.5x larger than the acceptor's fair fee at + // the same feerate. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let max_feerate = FeeRate::from_sat_per_kwu(3000); + let target_feerate = FeeRate::from_sat_per_kwu(4000); + let input = funding_input_sats(100_000); + let change = funding_output_sats(10_000); + + let budget = estimate_transaction_fee( + &[input.clone()], + &[], + true, + true, + original_feerate, + ); + + let contribution = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee: budget, + inputs: vec![input.clone()], + outputs: vec![], + change_output: Some(change.clone()), + feerate: original_feerate, + is_splice: true, + max_feerate, + }; + + let result = contribution.for_acceptor_at_feerate(target_feerate); + assert!(result.is_ok()); + let adjusted = result.unwrap(); + + // The acceptor's fair fee at target (4000, is_initiator=false) is less than the + // budget at original (2000, is_initiator=true) due to the ~2.5x weight ratio, + // so change increases despite the higher feerate. + assert!(adjusted.change_output.is_some()); + assert!(adjusted.change_output.as_ref().unwrap().value > Amount::from_sat(10_000)); + } + + #[test] + fn test_for_acceptor_at_feerate_within_range() { + // Splice-in: target feerate is between min and max, so the min/max checks + // don't interfere and the normal adjustment logic applies. + let original_feerate = FeeRate::from_sat_per_kwu(2000); + let max_feerate = FeeRate::from_sat_per_kwu(5000); + let target_feerate = FeeRate::from_sat_per_kwu(3000); + let input = funding_input_sats(100_000); + let change = funding_output_sats(10_000); + + let budget = estimate_transaction_fee( + &[input.clone()], + &[], + true, + true, + original_feerate, + ); + + let contribution = FundingContribution { + value_added: Amount::from_sat(50_000), + estimated_fee: budget, + inputs: vec![input], + outputs: vec![], + change_output: Some(change), + feerate: original_feerate, + is_splice: true, + max_feerate, + }; + + let result = contribution.for_acceptor_at_feerate(target_feerate); + assert!(result.is_ok()); + let adjusted = result.unwrap(); + + // At a higher target feerate, the fair fee increases so change should decrease + // (or stay the same if the budget overestimate absorbs the difference). + // The key assertion is that the adjustment succeeds with a valid change output. + assert!(adjusted.change_output.is_some()); } } diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 4e6aceb3b86..2a9fc488c92 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -52,7 +52,7 @@ fn test_splicing_not_supported_api_error() { let (_, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let res = nodes[1].node.splice_channel(&channel_id, &node_id_0, feerate); + let res = nodes[1].node.splice_channel(&channel_id, &node_id_0, feerate, FeeRate::MAX); match res { Err(APIError::ChannelUnavailable { err }) => { assert!(err.contains("Peer does not support splicing")) @@ -73,7 +73,7 @@ fn test_splicing_not_supported_api_error() { reconnect_args.send_announcement_sigs = (true, true); reconnect_nodes(reconnect_args); - let res = nodes[1].node.splice_channel(&channel_id, &node_id_0, feerate); + let res = nodes[1].node.splice_channel(&channel_id, &node_id_0, feerate, FeeRate::MAX); match res { Err(APIError::ChannelUnavailable { err }) => { assert!(err.contains("Peer does not support quiescence, a splicing prerequisite")) @@ -105,7 +105,7 @@ fn test_v1_splice_in_negative_insufficient_inputs() { // Initiate splice-in, with insufficient input contribution let funding_template = nodes[0] .node - .splice_channel(&channel_id, &nodes[1].node.get_our_node_id(), feerate) + .splice_channel(&channel_id, &nodes[1].node.get_our_node_id(), feerate, FeeRate::MAX) .unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); @@ -140,8 +140,10 @@ pub fn do_initiate_splice_in<'a, 'b, 'c, 'd>( ) -> FundingContribution { let node_id_acceptor = acceptor.node.get_our_node_id(); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = - initiator.node.splice_channel(&channel_id, &node_id_acceptor, feerate).unwrap(); + let funding_template = initiator + .node + .splice_channel(&channel_id, &node_id_acceptor, feerate, FeeRate::MAX) + .unwrap(); let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger); let funding_contribution = funding_template.splice_in_sync(value_added, &wallet).unwrap(); initiator @@ -157,8 +159,10 @@ pub fn initiate_splice_out<'a, 'b, 'c, 'd>( ) -> FundingContribution { let node_id_acceptor = acceptor.node.get_our_node_id(); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = - initiator.node.splice_channel(&channel_id, &node_id_acceptor, feerate).unwrap(); + let funding_template = initiator + .node + .splice_channel(&channel_id, &node_id_acceptor, feerate, FeeRate::MAX) + .unwrap(); let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger); let funding_contribution = funding_template.splice_out_sync(outputs, &wallet).unwrap(); initiator @@ -181,8 +185,10 @@ pub fn do_initiate_splice_in_and_out<'a, 'b, 'c, 'd>( ) -> FundingContribution { let node_id_acceptor = acceptor.node.get_our_node_id(); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = - initiator.node.splice_channel(&channel_id, &node_id_acceptor, feerate).unwrap(); + let funding_template = initiator + .node + .splice_channel(&channel_id, &node_id_acceptor, feerate, FeeRate::MAX) + .unwrap(); let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger); let funding_contribution = funding_template.splice_in_and_out_sync(value_added, outputs, &wallet).unwrap(); @@ -1065,7 +1071,8 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { }]; let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate).unwrap(); + let funding_template = + nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate, FeeRate::MAX).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let funding_contribution = funding_template.splice_out_sync(outputs.clone(), &wallet).unwrap(); nodes[0] @@ -1074,7 +1081,7 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { .unwrap(); assert_eq!( - nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate), + nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate, FeeRate::MAX), Err(APIError::APIMisuseError { err: format!( "Channel {} cannot be spliced as one is waiting to be negotiated", @@ -1086,7 +1093,7 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { let new_funding_script = complete_splice_handshake(&nodes[0], &nodes[1]); assert_eq!( - nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate), + nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate, FeeRate::MAX), Err(APIError::APIMisuseError { err: format!( "Channel {} cannot be spliced as one is currently being negotiated", @@ -1097,7 +1104,8 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { // The acceptor can enqueue a quiescent action while the current splice is pending. let added_value = Amount::from_sat(initial_channel_value_sat); - let acceptor_template = nodes[1].node.splice_channel(&channel_id, &node_0_id, feerate).unwrap(); + let acceptor_template = + nodes[1].node.splice_channel(&channel_id, &node_0_id, feerate, FeeRate::MAX).unwrap(); let acceptor_wallet = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); let acceptor_contribution = acceptor_template.splice_in_sync(added_value, &acceptor_wallet).unwrap(); @@ -1115,7 +1123,7 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { ); assert_eq!( - nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate), + nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate, FeeRate::MAX), Err(APIError::APIMisuseError { err: format!( "Channel {} cannot be spliced as one is currently being negotiated", @@ -1132,7 +1140,7 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { // Now that the splice is pending, another splice may be initiated, but we must wait until // the `splice_locked` exchange to send the initiator `stfu`. - assert!(nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate).is_ok()); + assert!(nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate, FeeRate::MAX).is_ok()); if reconnect { nodes[0].node.peer_disconnected(node_1_id); @@ -1173,7 +1181,8 @@ fn test_initiating_splice_holds_stfu_with_pending_splice() { let funding_contribution_0 = initiate_splice_in(&nodes[0], &nodes[1], channel_id, value_added); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = nodes[1].node.splice_channel(&channel_id, &node_0_id, feerate).unwrap(); + let funding_template = + nodes[1].node.splice_channel(&channel_id, &node_0_id, feerate, FeeRate::MAX).unwrap(); let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution_0); @@ -2853,7 +2862,8 @@ fn test_funding_contributed_counterparty_not_found() { provide_utxo_reserves(&nodes, 1, splice_in_amount * 2); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate).unwrap(); + let funding_template = + nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let funding_contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); @@ -2892,7 +2902,8 @@ fn test_funding_contributed_channel_not_found() { provide_utxo_reserves(&nodes, 1, splice_in_amount * 2); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate).unwrap(); + let funding_template = + nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let funding_contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); @@ -2936,7 +2947,8 @@ fn test_funding_contributed_splice_already_pending() { script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::from_raw_hash(Hash::all_zeros())), }; let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate).unwrap(); + let funding_template = + nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let first_contribution = funding_template .splice_in_and_out_sync(splice_in_amount, vec![first_splice_out.clone()], &wallet) @@ -2958,7 +2970,8 @@ fn test_funding_contributed_splice_already_pending() { nodes[0].wallet_source.clear_utxos(); provide_utxo_reserves(&nodes, 1, splice_in_amount * 3); - let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate).unwrap(); + let funding_template = + nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let second_contribution = funding_template .splice_in_and_out_sync(splice_in_amount, vec![second_splice_out.clone()], &wallet) @@ -3027,7 +3040,8 @@ fn test_funding_contributed_duplicate_contribution_no_event() { provide_utxo_reserves(&nodes, 1, splice_in_amount * 2); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate).unwrap(); + let funding_template = + nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); @@ -3085,7 +3099,8 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) { // Build first contribution let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate).unwrap(); + let funding_template = + nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let first_contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); @@ -3093,7 +3108,8 @@ fn do_test_funding_contributed_active_funding_negotiation(state: u8) { nodes[0].wallet_source.clear_utxos(); provide_utxo_reserves(&nodes, 1, splice_in_amount * 3); - let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate).unwrap(); + let funding_template = + nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let second_contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); @@ -3213,7 +3229,8 @@ fn test_funding_contributed_channel_shutdown() { provide_utxo_reserves(&nodes, 1, splice_in_amount * 2); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate).unwrap(); + let funding_template = + nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate, FeeRate::MAX).unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let funding_contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); @@ -3266,8 +3283,10 @@ fn test_funding_contributed_unfunded_channel() { provide_utxo_reserves(&nodes, 1, splice_in_amount * 2); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); - let funding_template = - nodes[0].node.splice_channel(&funded_channel_id, &node_id_1, feerate).unwrap(); + let funding_template = nodes[0] + .node + .splice_channel(&funded_channel_id, &node_id_1, feerate, FeeRate::MAX) + .unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); let funding_contribution = funding_template.splice_in_sync(splice_in_amount, &wallet).unwrap(); From f102b4a4bcef570a87d94fe5e22197a9dcf71128 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 23 Feb 2026 10:46:11 -0600 Subject: [PATCH 09/13] Include change output weight in estimate_transaction_fee Add a `change_output: Option<&TxOut>` parameter to `estimate_transaction_fee` so the initial fee estimate accounts for the change output's weight. Previously, the change output weight was omitted from `estimated_fee` in `FundingContribution`, causing the estimate to be slightly too low when a change output was present. This also eliminates an unnecessary `Vec` allocation in `compute_feerate_adjustment`, which previously cloned outputs into a temporary Vec just to include the change output for the fee estimate. A mock `TightBudgetWallet` is added to `splicing_tests` to demonstrate that `validate()` correctly rejects contributions where the input value is sufficient without the change output weight but insufficient with it. Co-Authored-By: Claude Opus 4.6 --- lightning/src/ln/funding.rs | 162 ++++++++++++++++++++++------- lightning/src/ln/splicing_tests.rs | 79 +++++++++++++- 2 files changed, 203 insertions(+), 38 deletions(-) diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 1c53e5bf50a..d1a40aeab51 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -149,7 +149,7 @@ macro_rules! build_funding_contribution { // The caller creating a FundingContribution is always the initiator for fee estimation // purposes — this is conservative, overestimating rather than underestimating fees if // the node ends up as the acceptor. - let estimated_fee = estimate_transaction_fee(&inputs, &outputs, true, is_splice, feerate); + let estimated_fee = estimate_transaction_fee(&inputs, &outputs, change_output.as_ref(), true, is_splice, feerate); debug_assert!(estimated_fee <= Amount::MAX_MONEY); let contribution = FundingContribution { @@ -263,8 +263,8 @@ impl FundingTemplate { } fn estimate_transaction_fee( - inputs: &[FundingTxInput], outputs: &[TxOut], is_initiator: bool, is_splice: bool, - feerate: FeeRate, + inputs: &[FundingTxInput], outputs: &[TxOut], change_output: Option<&TxOut>, + is_initiator: bool, is_splice: bool, feerate: FeeRate, ) -> Amount { let input_weight: u64 = inputs .iter() @@ -273,6 +273,7 @@ fn estimate_transaction_fee( let output_weight: u64 = outputs .iter() + .chain(change_output.into_iter()) .map(|txout| txout.weight().to_wu()) .fold(0, |total_weight, output_weight| total_weight.saturating_add(output_weight)); @@ -449,11 +450,11 @@ impl FundingContribution { .ok_or("Sum of input values is greater than the total bitcoin supply")?; } - // If the inputs are enough to cover intended contribution amount, with fees even when - // there is a change output, we are fine. - // If the inputs are less, but enough to cover intended contribution amount, with - // (lower) fees with no change, we are also fine (change will not be generated). - // So it's enough to check considering the lower, no-change fees. + // If the inputs are enough to cover intended contribution amount plus fees (which + // include the change output weight when present), we are fine. + // If the inputs are less, but enough to cover intended contribution amount with + // (lower) fees without change, we are also fine (change will not be generated). + // Since estimated_fee includes change weight, this check is conservative. // // Note: dust limit is not relevant in this check. @@ -497,6 +498,7 @@ impl FundingContribution { let fair_fee = estimate_transaction_fee( &self.inputs, &self.outputs, + self.change_output.as_ref(), false, self.is_splice, target_feerate, @@ -520,11 +522,10 @@ impl FundingContribution { let dust_limit = change_output.script_pubkey.minimal_non_dust(); // Fair fee including the change output's weight. - let all_outputs: Vec = - self.outputs.iter().chain(self.change_output.iter()).cloned().collect(); let fair_fee = estimate_transaction_fee( &self.inputs, - &all_outputs, + &self.outputs, + self.change_output.as_ref(), false, is_splice, target_feerate, @@ -545,6 +546,7 @@ impl FundingContribution { let fair_fee_no_change = estimate_transaction_fee( &self.inputs, &self.outputs, + None, false, is_splice, target_feerate, @@ -564,6 +566,7 @@ impl FundingContribution { let fair_fee = estimate_transaction_fee( &self.inputs, &self.outputs, + None, false, is_splice, target_feerate, @@ -587,8 +590,14 @@ impl FundingContribution { } } else { // No inputs (splice-out): fees paid from channel balance. - let fair_fee = - estimate_transaction_fee(&[], &self.outputs, false, is_splice, target_feerate); + let fair_fee = estimate_transaction_fee( + &[], + &self.outputs, + None, + false, + is_splice, + target_feerate, + ); if self.estimated_fee < fair_fee { return Err(FeeRateAdjustmentError::BudgetInsufficient(format!( "Estimated fee {} insufficient for required fee {}", @@ -687,45 +696,71 @@ mod tests { // 2 inputs, initiator, 2000 sat/kw feerate assert_eq!( - estimate_transaction_fee(&two_inputs, &[], true, false, FeeRate::from_sat_per_kwu(2000)), + estimate_transaction_fee(&two_inputs, &[], None, true, false, FeeRate::from_sat_per_kwu(2000)), Amount::from_sat(if cfg!(feature = "grind_signatures") { 1512 } else { 1516 }), ); // higher feerate assert_eq!( - estimate_transaction_fee(&two_inputs, &[], true, false, FeeRate::from_sat_per_kwu(3000)), + estimate_transaction_fee(&two_inputs, &[], None, true, false, FeeRate::from_sat_per_kwu(3000)), Amount::from_sat(if cfg!(feature = "grind_signatures") { 2268 } else { 2274 }), ); // only 1 input assert_eq!( - estimate_transaction_fee(&one_input, &[], true, false, FeeRate::from_sat_per_kwu(2000)), + estimate_transaction_fee(&one_input, &[], None, true, false, FeeRate::from_sat_per_kwu(2000)), Amount::from_sat(if cfg!(feature = "grind_signatures") { 970 } else { 972 }), ); // 0 inputs assert_eq!( - estimate_transaction_fee(&[], &[], true, false, FeeRate::from_sat_per_kwu(2000)), + estimate_transaction_fee(&[], &[], None, true, false, FeeRate::from_sat_per_kwu(2000)), Amount::from_sat(428), ); // not initiator assert_eq!( - estimate_transaction_fee(&[], &[], false, false, FeeRate::from_sat_per_kwu(2000)), + estimate_transaction_fee(&[], &[], None, false, false, FeeRate::from_sat_per_kwu(2000)), Amount::from_sat(0), ); // splice initiator assert_eq!( - estimate_transaction_fee(&one_input, &[], true, true, FeeRate::from_sat_per_kwu(2000)), + estimate_transaction_fee(&one_input, &[], None, true, true, FeeRate::from_sat_per_kwu(2000)), Amount::from_sat(if cfg!(feature = "grind_signatures") { 1736 } else { 1740 }), ); // splice acceptor assert_eq!( - estimate_transaction_fee(&one_input, &[], false, true, FeeRate::from_sat_per_kwu(2000)), + estimate_transaction_fee(&one_input, &[], None, false, true, FeeRate::from_sat_per_kwu(2000)), Amount::from_sat(if cfg!(feature = "grind_signatures") { 542 } else { 544 }), ); + + // splice initiator, 1 input, 1 output + let output = funding_output_sats(500); + assert_eq!( + estimate_transaction_fee(&one_input, &[output.clone()], None, true, true, FeeRate::from_sat_per_kwu(2000)), + Amount::from_sat(if cfg!(feature = "grind_signatures") { 1984 } else { 1988 }), + ); + + // splice acceptor, 1 input, 1 output + assert_eq!( + estimate_transaction_fee(&one_input, &[output.clone()], None, false, true, FeeRate::from_sat_per_kwu(2000)), + Amount::from_sat(if cfg!(feature = "grind_signatures") { 790 } else { 792 }), + ); + + // splice initiator, 1 input, 1 output, 1 change via change_output parameter + let change = funding_output_sats(1_000); + assert_eq!( + estimate_transaction_fee(&one_input, &[output.clone()], Some(&change), true, true, FeeRate::from_sat_per_kwu(2000)), + Amount::from_sat(if cfg!(feature = "grind_signatures") { 2232 } else { 2236 }), + ); + + // splice acceptor, 1 input, 1 output, 1 change via change_output parameter + assert_eq!( + estimate_transaction_fee(&one_input, &[output.clone()], Some(&change), false, true, FeeRate::from_sat_per_kwu(2000)), + Amount::from_sat(if cfg!(feature = "grind_signatures") { 1038 } else { 1040 }), + ); } #[rustfmt::skip] @@ -1001,12 +1036,19 @@ mod tests { // feerate for the acceptor's fair fee to exceed the budget, causing the change // to decrease. let original_feerate = FeeRate::from_sat_per_kwu(2000); - let target_feerate = FeeRate::from_sat_per_kwu(5000); + let target_feerate = FeeRate::from_sat_per_kwu(6000); let input = funding_input_sats(100_000); let change = funding_output_sats(10_000); - // Budget computed as initiator (overestimate, without change output weight). - let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + // Budget computed as initiator (overestimate), including change output weight. + let budget = estimate_transaction_fee( + &[input.clone()], + &[], + Some(&change), + true, + true, + original_feerate, + ); let contribution = FundingContribution { value_added: Amount::from_sat(50_000), @@ -1024,7 +1066,7 @@ mod tests { // Fair fee at target feerate for acceptor (is_initiator=false), including change weight. let expected_fair_fee = - estimate_transaction_fee(&[input], &[change], false, true, target_feerate); + estimate_transaction_fee(&[input], &[], Some(&change), false, true, target_feerate); let expected_change = budget + Amount::from_sat(10_000) - expected_fair_fee; assert_eq!(contribution.estimated_fee, expected_fair_fee); @@ -1042,7 +1084,14 @@ mod tests { let input = funding_input_sats(100_000); let change = funding_output_sats(10_000); - let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + let budget = estimate_transaction_fee( + &[input.clone()], + &[], + Some(&change), + true, + true, + original_feerate, + ); let contribution = FundingContribution { value_added: Amount::from_sat(50_000), @@ -1068,7 +1117,14 @@ mod tests { let input = funding_input_sats(100_000); let change = funding_output_sats(500); - let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + let budget = estimate_transaction_fee( + &[input.clone()], + &[], + Some(&change), + true, + true, + original_feerate, + ); let contribution = FundingContribution { value_added: Amount::from_sat(50_000), @@ -1087,7 +1143,7 @@ mod tests { // Change should be removed; estimated_fee updated to no-change fair fee. assert!(contribution.change_output.is_none()); let expected_fee_no_change = - estimate_transaction_fee(&[input], &[], false, true, target_feerate); + estimate_transaction_fee(&[input], &[], None, false, true, target_feerate); assert_eq!(contribution.estimated_fee, expected_fee_no_change); assert_eq!(contribution.net_value(), net_value_before); } @@ -1100,7 +1156,14 @@ mod tests { let input = funding_input_sats(100_000); let change = funding_output_sats(500); - let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + let budget = estimate_transaction_fee( + &[input.clone()], + &[], + Some(&change), + true, + true, + original_feerate, + ); let contribution = FundingContribution { value_added: Amount::from_sat(50_000), @@ -1125,7 +1188,8 @@ mod tests { let target_feerate = FeeRate::from_sat_per_kwu(3000); let output = funding_output_sats(50_000); - let budget = estimate_transaction_fee(&[], &[output.clone()], true, true, original_feerate); + let budget = + estimate_transaction_fee(&[], &[output.clone()], None, true, true, original_feerate); let contribution = FundingContribution { value_added: Amount::ZERO, @@ -1141,7 +1205,7 @@ mod tests { let contribution = contribution.for_acceptor_at_feerate(target_feerate).unwrap(); // estimated_fee is updated to the fair fee; surplus goes back to channel balance. let expected_fair_fee = - estimate_transaction_fee(&[], &[output], false, true, target_feerate); + estimate_transaction_fee(&[], &[output], None, false, true, target_feerate); assert_eq!(contribution.estimated_fee, expected_fair_fee); assert!(expected_fair_fee <= budget); } @@ -1153,7 +1217,8 @@ mod tests { let target_feerate = FeeRate::from_sat_per_kwu(50_000); let output = funding_output_sats(50_000); - let budget = estimate_transaction_fee(&[], &[output.clone()], true, true, original_feerate); + let budget = + estimate_transaction_fee(&[], &[output.clone()], None, true, true, original_feerate); let contribution = FundingContribution { value_added: Amount::ZERO, @@ -1179,7 +1244,14 @@ mod tests { let input = funding_input_sats(100_000); let change = funding_output_sats(10_000); - let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + let budget = estimate_transaction_fee( + &[input.clone()], + &[], + Some(&change), + true, + true, + original_feerate, + ); let contribution = FundingContribution { value_added: Amount::from_sat(50_000), @@ -1207,7 +1279,8 @@ mod tests { let target_feerate = FeeRate::from_sat_per_kwu(3000); let output = funding_output_sats(50_000); - let budget = estimate_transaction_fee(&[], &[output.clone()], true, true, original_feerate); + let budget = + estimate_transaction_fee(&[], &[output.clone()], None, true, true, original_feerate); let contribution = FundingContribution { value_added: Amount::ZERO, @@ -1224,7 +1297,7 @@ mod tests { contribution.net_value_for_acceptor_at_feerate(target_feerate).unwrap(); // The fair fee at target feerate should be less than the initiator's budget. - let fair_fee = estimate_transaction_fee(&[], &[output], false, true, target_feerate); + let fair_fee = estimate_transaction_fee(&[], &[output], None, false, true, target_feerate); let expected_net = SignedAmount::ZERO - Amount::from_sat(50_000).to_signed().unwrap() - fair_fee.to_signed().unwrap(); @@ -1242,7 +1315,14 @@ mod tests { let input = funding_input_sats(100_000); let change = funding_output_sats(10_000); - let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + let budget = estimate_transaction_fee( + &[input.clone()], + &[], + Some(&change), + true, + true, + original_feerate, + ); let contribution = FundingContribution { value_added: Amount::from_sat(50_000), @@ -1275,7 +1355,14 @@ mod tests { let input = funding_input_sats(100_000); let change = funding_output_sats(500); - let budget = estimate_transaction_fee(&[input.clone()], &[], true, true, original_feerate); + let budget = estimate_transaction_fee( + &[input.clone()], + &[], + Some(&change), + true, + true, + original_feerate, + ); let contribution = FundingContribution { value_added: Amount::from_sat(50_000), @@ -1305,6 +1392,7 @@ mod tests { let budget = estimate_transaction_fee( &[input.clone()], &[], + Some(&change), true, true, original_feerate, @@ -1342,6 +1430,7 @@ mod tests { let budget = estimate_transaction_fee( &[input.clone()], &[], + Some(&change), true, true, original_feerate, @@ -1382,6 +1471,7 @@ mod tests { let budget = estimate_transaction_fee( &[input.clone()], &[], + Some(&change), true, true, original_feerate, diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 2a9fc488c92..c75a39ebea3 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -25,15 +25,18 @@ use crate::ln::types::ChannelId; use crate::routing::router::{PaymentParameters, RouteParameters}; use crate::util::errors::APIError; use crate::util::ser::Writeable; -use crate::util::wallet_utils::{WalletSourceSync, WalletSync}; +use crate::util::wallet_utils::{ + CoinSelection, CoinSelectionSourceSync, ConfirmedUtxo, Input, WalletSourceSync, WalletSync, +}; use crate::sync::Arc; use bitcoin::hashes::Hash; use bitcoin::secp256k1::ecdsa::Signature; use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; +use bitcoin::transaction::Version; use bitcoin::{ - Amount, FeeRate, OutPoint as BitcoinOutPoint, ScriptBuf, Transaction, TxOut, WPubkeyHash, + Amount, FeeRate, OutPoint as BitcoinOutPoint, Psbt, ScriptBuf, Transaction, TxOut, WPubkeyHash, }; #[test] @@ -112,6 +115,78 @@ fn test_v1_splice_in_negative_insufficient_inputs() { assert!(funding_template.splice_in_sync(splice_in_value, &wallet).is_err()); } +/// A mock wallet that returns a pre-configured [`CoinSelection`] with a single input and change +/// output. Used to test edge cases where the input value is tight relative to the fee estimate. +struct TightBudgetWallet { + utxo_value: Amount, + change_value: Amount, +} + +impl CoinSelectionSourceSync for TightBudgetWallet { + fn select_confirmed_utxos( + &self, _claim_id: Option, _must_spend: Vec, + _must_pay_to: &[TxOut], _target_feerate_sat_per_1000_weight: u32, _max_tx_weight: u64, + ) -> Result { + let prevout = TxOut { + value: self.utxo_value, + script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()), + }; + let prevtx = Transaction { + input: vec![], + output: vec![prevout], + version: Version::TWO, + lock_time: bitcoin::absolute::LockTime::ZERO, + }; + let utxo = ConfirmedUtxo::new_p2wpkh(prevtx, 0).unwrap(); + + let change_output = TxOut { + value: self.change_value, + script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()), + }; + + Ok(CoinSelection { confirmed_utxos: vec![utxo], change_output: Some(change_output) }) + } + + fn sign_psbt(&self, _psbt: Psbt) -> Result { + unreachable!("should not reach signing") + } +} + +#[test] +fn test_validate_accounts_for_change_output_weight() { + // Demonstrates that estimated_fee includes the change output's weight when building a + // FundingContribution. A mock wallet returns a single input whose value is between + // estimated_fee_without_change (1736/1740 sats) and estimated_fee_with_change (1984/1988 + // sats) above value_added. The validate() check correctly catches that the inputs are + // insufficient when the change output weight is included. Without accounting for the change + // output weight, the check would incorrectly pass. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0); + + let feerate = FeeRate::from_sat_per_kwu(2000); + let funding_template = nodes[0] + .node + .splice_channel(&channel_id, &nodes[1].node.get_our_node_id(), feerate, FeeRate::MAX) + .unwrap(); + + // Input value = value_added + 1800: above 1736/1740 (fee without change), below 1984/1988 + // (fee with change). + let value_added = Amount::from_sat(20_000); + let wallet = TightBudgetWallet { + utxo_value: value_added + Amount::from_sat(1800), + change_value: Amount::from_sat(1000), + }; + let contribution = funding_template.splice_in_sync(value_added, &wallet).unwrap(); + + assert!(contribution.change_output().is_some()); + assert!(contribution.validate().is_err()); +} + pub fn negotiate_splice_tx<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, funding_contribution: FundingContribution, From 3b34d0ef2782639dab7077445c1786ce8a92520e Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 10 Feb 2026 16:02:31 -0600 Subject: [PATCH 10/13] Contribute to splice as acceptor When both nodes want to splice simultaneously, the quiescence tie-breaker designates one as the initiator. Previously, the losing node responded with zero contribution, requiring a second full splice session after the first splice locked. This is wasteful, especially for often-offline nodes that may connect and immediately want to splice. Instead, the losing node contributes to the winner's splice as the acceptor, merging both contributions into a single splice transaction. Since the FundingContribution was originally built with initiator fees (which include common fields and shared input/output weight), the fee is adjusted to the acceptor rate before contributing, with the surplus returned to the change output. Co-Authored-By: Claude Opus 4.6 --- lightning/src/ln/channel.rs | 67 +++- lightning/src/ln/channelmanager.rs | 4 - lightning/src/ln/splicing_tests.rs | 533 +++++++++++++++++++++++++---- 3 files changed, 515 insertions(+), 89 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 93af6ad6adf..40f1a16cfcd 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12195,6 +12195,26 @@ where self.propose_quiescence(logger, QuiescentAction::Splice { contribution, locktime }) } + /// Returns a reference to the funding contribution queued by a pending [`QuiescentAction`], + /// if any. + fn queued_funding_contribution(&self) -> Option<&FundingContribution> { + match &self.quiescent_action { + Some(QuiescentAction::Splice { contribution, .. }) => Some(contribution), + _ => None, + } + } + + /// Consumes and returns the funding contribution from the pending [`QuiescentAction`], if any. + fn take_queued_funding_contribution(&mut self) -> Option { + match &self.quiescent_action { + Some(QuiescentAction::Splice { .. }) => match self.quiescent_action.take() { + Some(QuiescentAction::Splice { contribution, .. }) => Some(contribution), + _ => unreachable!(), + }, + _ => None, + } + } + fn send_splice_init(&mut self, context: FundingNegotiationContext) -> msgs::SpliceInit { debug_assert!(self.pending_splice.is_none()); // Rotate the funding pubkey using the prev_funding_txid as a tweak @@ -12292,10 +12312,6 @@ where )); } - // TODO(splicing): Once splice acceptor can contribute, check that inputs are sufficient, - // similarly to the check in `funding_contributed`. - debug_assert_eq!(our_funding_contribution, SignedAmount::ZERO); - let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis); if their_funding_contribution == SignedAmount::ZERO { return Err(ChannelError::WarnAndDisconnect(format!( @@ -12419,11 +12435,37 @@ where } pub(crate) fn splice_init( - &mut self, msg: &msgs::SpliceInit, our_funding_contribution_satoshis: i64, - entropy_source: &ES, holder_node_id: &PublicKey, logger: &L, + &mut self, msg: &msgs::SpliceInit, entropy_source: &ES, holder_node_id: &PublicKey, + logger: &L, ) -> Result { - let our_funding_contribution = SignedAmount::from_sat(our_funding_contribution_satoshis); - let splice_funding = self.validate_splice_init(msg, our_funding_contribution)?; + let feerate = FeeRate::from_sat_per_kwu(msg.funding_feerate_per_kw as u64); + let our_funding_contribution = self.queued_funding_contribution().and_then(|c| { + c.net_value_for_acceptor_at_feerate(feerate) + .map_err(|e| { + log_info!( + logger, + "Cannot accommodate initiator's feerate for channel {}: {}; \ + proceeding without contribution", + self.context.channel_id(), + e, + ); + }) + .ok() + }); + + let splice_funding = + self.validate_splice_init(msg, our_funding_contribution.unwrap_or(SignedAmount::ZERO))?; + + let (our_funding_inputs, our_funding_outputs) = if our_funding_contribution.is_some() { + self.take_queued_funding_contribution() + .expect("queued_funding_contribution was Some") + .for_acceptor_at_feerate(feerate) + .expect("feerate compatibility already checked") + .into_tx_parts() + } else { + Default::default() + }; + let our_funding_contribution = our_funding_contribution.unwrap_or(SignedAmount::ZERO); log_info!( logger, @@ -12440,8 +12482,8 @@ where funding_tx_locktime: LockTime::from_consensus(msg.locktime), funding_feerate_sat_per_1000_weight: msg.funding_feerate_per_kw, shared_funding_input: Some(prev_funding_input), - our_funding_inputs: Vec::new(), - our_funding_outputs: Vec::new(), + our_funding_inputs, + our_funding_outputs, }; let (interactive_tx_constructor, first_message) = funding_negotiation_context @@ -12453,11 +12495,6 @@ where ); debug_assert!(first_message.is_none()); - // TODO(splicing): if quiescent_action is set, integrate what the user wants to do into the - // counterparty-initiated splice. For always-on nodes this probably isn't a useful - // optimization, but for often-offline nodes it may be, as we may connect and immediately - // go into splicing from both sides. - let new_funding_pubkey = splice_funding.get_holder_pubkeys().funding_pubkey; self.pending_splice = Some(PendingFunding { funding_negotiation: Some(FundingNegotiation::ConstructingTransaction { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index ebec3768d15..f62dbee0014 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -12814,9 +12814,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let mut peer_state_lock = peer_state_mutex.lock().unwrap(); let peer_state = &mut *peer_state_lock; - // TODO(splicing): Currently not possible to contribute on the splicing-acceptor side - let our_funding_contribution = 0i64; - // Look for the channel match peer_state.channel_by_id.entry(msg.channel_id) { hash_map::Entry::Vacant(_) => { @@ -12836,7 +12833,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ if let Some(ref mut funded_channel) = chan_entry.get_mut().as_funded_mut() { let init_res = funded_channel.splice_init( msg, - our_funding_contribution, &self.entropy_source, &self.get_our_node_id(), &self.logger, diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index c75a39ebea3..40be32d6950 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -302,6 +302,21 @@ pub fn complete_splice_handshake<'a, 'b, 'c, 'd>( pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, initiator_contribution: FundingContribution, new_funding_script: ScriptBuf, +) { + complete_interactive_funding_negotiation_for_both( + initiator, + acceptor, + channel_id, + initiator_contribution, + None, + new_funding_script, + ); +} + +pub fn complete_interactive_funding_negotiation_for_both<'a, 'b, 'c, 'd>( + initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, + initiator_contribution: FundingContribution, + acceptor_contribution: Option, new_funding_script: ScriptBuf, ) { let node_id_initiator = initiator.node.get_our_node_id(); let node_id_acceptor = acceptor.node.get_our_node_id(); @@ -327,8 +342,22 @@ pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( .chain(core::iter::once(new_funding_script)) .collect::>(); + let (mut expected_acceptor_inputs, mut expected_acceptor_scripts) = + if let Some(acceptor_contribution) = acceptor_contribution { + let (acceptor_inputs, acceptor_outputs) = acceptor_contribution.into_tx_parts(); + let expected_acceptor_inputs = + acceptor_inputs.iter().map(|input| input.utxo.outpoint).collect::>(); + let expected_acceptor_scripts = + acceptor_outputs.into_iter().map(|output| output.script_pubkey).collect::>(); + (expected_acceptor_inputs, expected_acceptor_scripts) + } else { + (Vec::new(), Vec::new()) + }; + let mut acceptor_sent_tx_complete = false; + let mut initiator_sent_tx_complete; loop { + // Initiator's turn: send TxAddInput, TxAddOutput, or TxComplete if !expected_initiator_inputs.is_empty() { let tx_add_input = get_event_msg!(initiator, MessageSendEvent::SendTxAddInput, node_id_acceptor); @@ -345,6 +374,7 @@ pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( expected_initiator_inputs.iter().position(|input| *input == input_prevout).unwrap(), ); acceptor.node.handle_tx_add_input(node_id_initiator, &tx_add_input); + initiator_sent_tx_complete = false; } else if !expected_initiator_scripts.is_empty() { let tx_add_output = get_event_msg!(initiator, MessageSendEvent::SendTxAddOutput, node_id_acceptor); @@ -355,6 +385,7 @@ pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( .unwrap(), ); acceptor.node.handle_tx_add_output(node_id_initiator, &tx_add_output); + initiator_sent_tx_complete = false; } else { let msg_events = initiator.node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), 1, "{msg_events:?}"); @@ -363,24 +394,69 @@ pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( } else { panic!(); } + initiator_sent_tx_complete = true; if acceptor_sent_tx_complete { break; } } - let mut msg_events = acceptor.node.get_and_clear_pending_msg_events(); + // Acceptor's turn: send TxAddInput, TxAddOutput, or TxComplete + let msg_events = acceptor.node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), 1, "{msg_events:?}"); - if let MessageSendEvent::SendTxComplete { ref msg, .. } = msg_events.remove(0) { - initiator.node.handle_tx_complete(node_id_acceptor, msg); - } else { - panic!(); + match &msg_events[0] { + MessageSendEvent::SendTxAddInput { msg, .. } => { + let input_prevout = BitcoinOutPoint { + txid: msg + .prevtx + .as_ref() + .map(|prevtx| prevtx.compute_txid()) + .or(msg.shared_input_txid) + .unwrap(), + vout: msg.prevtx_out, + }; + expected_acceptor_inputs.remove( + expected_acceptor_inputs + .iter() + .position(|input| *input == input_prevout) + .unwrap(), + ); + initiator.node.handle_tx_add_input(node_id_acceptor, msg); + acceptor_sent_tx_complete = false; + }, + MessageSendEvent::SendTxAddOutput { msg, .. } => { + expected_acceptor_scripts.remove( + expected_acceptor_scripts + .iter() + .position(|script| *script == msg.script) + .unwrap(), + ); + initiator.node.handle_tx_add_output(node_id_acceptor, msg); + acceptor_sent_tx_complete = false; + }, + MessageSendEvent::SendTxComplete { msg, .. } => { + initiator.node.handle_tx_complete(node_id_acceptor, msg); + acceptor_sent_tx_complete = true; + if initiator_sent_tx_complete { + break; + } + }, + _ => panic!("Unexpected message event: {:?}", msg_events[0]), } - acceptor_sent_tx_complete = true; } + + assert!(expected_acceptor_inputs.is_empty(), "Not all acceptor inputs were sent"); + assert!(expected_acceptor_scripts.is_empty(), "Not all acceptor outputs were sent"); } pub fn sign_interactive_funding_tx<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, is_0conf: bool, +) -> (Transaction, Option<(msgs::SpliceLocked, PublicKey)>) { + sign_interactive_funding_tx_with_acceptor_contribution(initiator, acceptor, is_0conf, false) +} + +pub fn sign_interactive_funding_tx_with_acceptor_contribution<'a, 'b, 'c, 'd>( + initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, is_0conf: bool, + acceptor_has_contribution: bool, ) -> (Transaction, Option<(msgs::SpliceLocked, PublicKey)>) { let node_id_initiator = initiator.node.get_our_node_id(); let node_id_acceptor = acceptor.node.get_our_node_id(); @@ -414,6 +490,29 @@ pub fn sign_interactive_funding_tx<'a, 'b, 'c, 'd>( }; acceptor.node.handle_commitment_signed(node_id_initiator, &initial_commit_sig_for_acceptor); + if acceptor_has_contribution { + // When the acceptor contributed inputs, it needs to sign as well. The counterparty's + // commitment_signed is buffered until the acceptor signs. + assert!(acceptor.node.get_and_clear_pending_msg_events().is_empty()); + + let event = get_event!(acceptor, Event::FundingTransactionReadyForSigning); + if let Event::FundingTransactionReadyForSigning { + channel_id, + counterparty_node_id, + unsigned_transaction, + .. + } = event + { + let partially_signed_tx = acceptor.wallet_source.sign_tx(unsigned_transaction).unwrap(); + acceptor + .node + .funding_transaction_signed(&channel_id, &counterparty_node_id, partially_signed_tx) + .unwrap(); + } else { + panic!(); + } + } + let msg_events = acceptor.node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), 2, "{msg_events:?}"); if let MessageSendEvent::UpdateHTLCs { ref updates, .. } = &msg_events[0] { @@ -1279,6 +1378,351 @@ fn test_initiating_splice_holds_stfu_with_pending_splice() { ); } +#[test] +fn test_splice_both_contribute_tiebreak() { + // Same feerate: the acceptor's change increases because is_initiator=false has lower weight. + do_test_splice_both_contribute_tiebreak(None, None); +} + +#[test] +fn test_splice_tiebreak_higher_feerate() { + // Node 0 (winner) uses a higher feerate than node 1 (loser). Node 1's change output is + // adjusted (reduced) to accommodate the higher feerate. Negotiation succeeds. + let floor = FEERATE_FLOOR_SATS_PER_KW as u64; + do_test_splice_both_contribute_tiebreak( + Some(FeeRate::from_sat_per_kwu(floor * 3)), + Some(FeeRate::from_sat_per_kwu(floor)), + ); +} + +#[test] +fn test_splice_tiebreak_lower_feerate() { + // Node 0 (winner) uses a lower feerate than node 1 (loser). Node 1's change output increases + // because the acceptor's fair fee decreases. Negotiation succeeds. + let floor = FEERATE_FLOOR_SATS_PER_KW as u64; + do_test_splice_both_contribute_tiebreak( + Some(FeeRate::from_sat_per_kwu(floor)), + Some(FeeRate::from_sat_per_kwu(floor * 3)), + ); +} + +/// Runs the splice tie-breaker test with optional per-node feerates. +/// If `node_0_feerate` or `node_1_feerate` is None, both use the same default feerate. +#[cfg(test)] +fn do_test_splice_both_contribute_tiebreak( + node_0_feerate: Option, node_1_feerate: Option, +) { + // Both nodes call splice_channel + splice_in_sync + funding_contributed, both send STFU, + // one wins the quiescence tie-break (node 0, the outbound channel funder). The loser + // (node 1) becomes the acceptor and its stored QuiescentAction is consumed by the + // splice_init handler, contributing its inputs/outputs to the splice transaction. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, Amount::from_sat(100_000)); + + let default_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let feerate_0 = node_0_feerate.unwrap_or(default_feerate); + let feerate_1 = node_1_feerate.unwrap_or(default_feerate); + + // Node 0 calls splice_channel + splice_in_sync + funding_contributed at feerate_0. + let funding_template_0 = + nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate_0, FeeRate::MAX).unwrap(); + let wallet_0 = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let node_0_funding_contribution = + funding_template_0.splice_in_sync(added_value, &wallet_0).unwrap(); + nodes[0] + .node + .funding_contributed(&channel_id, &node_id_1, node_0_funding_contribution.clone(), None) + .unwrap(); + + // Node 1 calls splice_channel + splice_in_sync + funding_contributed at feerate_1. + let funding_template_1 = + nodes[1].node.splice_channel(&channel_id, &node_id_0, feerate_1, FeeRate::MAX).unwrap(); + let wallet_1 = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); + let node_1_funding_contribution = + funding_template_1.splice_in_sync(added_value, &wallet_1).unwrap(); + nodes[1] + .node + .funding_contributed(&channel_id, &node_id_0, node_1_funding_contribution.clone(), None) + .unwrap(); + + // Capture change output values before the tiebreak. + let node_0_change = node_0_funding_contribution + .change_output() + .expect("splice-in should have a change output") + .clone(); + let node_1_change = node_1_funding_contribution + .change_output() + .expect("splice-in should have a change output") + .clone(); + + // Both nodes emit STFU. + let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + assert!(stfu_0.initiator); + let stfu_1 = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + assert!(stfu_1.initiator); + + // Tie-break: node 1 handles node 0's STFU first — node 1 loses (not the outbound funder). + nodes[1].node.handle_stfu(node_id_0, &stfu_0); + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); + + // Node 0 handles node 1's STFU — node 0 wins (outbound funder), sends SpliceInit. + nodes[0].node.handle_stfu(node_id_1, &stfu_1); + + let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); + + // Node 1 handles SpliceInit — its contribution is adjusted for node 0's feerate as acceptor, + // then sends SpliceAck with its contribution. + nodes[1].node.handle_splice_init(node_id_0, &splice_init); + let splice_ack = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); + assert_ne!( + splice_ack.funding_contribution_satoshis, 0, + "Acceptor should contribute to the splice" + ); + + // Node 0 handles SpliceAck — starts interactive tx construction. + nodes[0].node.handle_splice_ack(node_id_1, &splice_ack); + + // Compute the new funding script from the splice pubkeys. + let new_funding_script = chan_utils::make_funding_redeemscript( + &splice_init.funding_pubkey, + &splice_ack.funding_pubkey, + ) + .to_p2wsh(); + + // Complete interactive funding negotiation with both parties' inputs/outputs. + complete_interactive_funding_negotiation_for_both( + &nodes[0], + &nodes[1], + channel_id, + node_0_funding_contribution, + Some(node_1_funding_contribution), + new_funding_script, + ); + + // Sign (acceptor has contribution) and broadcast. + let (tx, splice_locked) = + sign_interactive_funding_tx_with_acceptor_contribution(&nodes[0], &nodes[1], false, true); + assert!(splice_locked.is_none()); + + // The initiator's change output should remain unchanged (no feerate adjustment). + let initiator_change_in_tx = tx + .output + .iter() + .find(|o| o.script_pubkey == node_0_change.script_pubkey) + .expect("Initiator's change output should be in the splice transaction"); + assert_eq!( + initiator_change_in_tx.value, node_0_change.value, + "Initiator's change output should remain unchanged", + ); + + // The acceptor's change output should be adjusted based on the feerate difference. + let acceptor_change_in_tx = tx + .output + .iter() + .find(|o| o.script_pubkey == node_1_change.script_pubkey) + .expect("Acceptor's change output should be in the splice transaction"); + if feerate_0 <= feerate_1 { + // Initiator's feerate <= acceptor's original: the acceptor's change increases because + // is_initiator=false has lower weight, and the feerate is the same or lower. + assert!( + acceptor_change_in_tx.value > node_1_change.value, + "Acceptor's change should increase when initiator feerate ({}) <= acceptor feerate \ + ({}): adjusted {} vs original {}", + feerate_0.to_sat_per_kwu(), + feerate_1.to_sat_per_kwu(), + acceptor_change_in_tx.value, + node_1_change.value, + ); + } else { + // Initiator's feerate > acceptor's original: the higher feerate more than compensates + // for the lower weight, so the acceptor's change decreases. + assert!( + acceptor_change_in_tx.value < node_1_change.value, + "Acceptor's change should decrease when initiator feerate ({}) > acceptor feerate \ + ({}): adjusted {} vs original {}", + feerate_0.to_sat_per_kwu(), + feerate_1.to_sat_per_kwu(), + acceptor_change_in_tx.value, + node_1_change.value, + ); + } + + expect_splice_pending_event(&nodes[0], &node_id_1); + expect_splice_pending_event(&nodes[1], &node_id_0); + + mine_transaction(&nodes[0], &tx); + mine_transaction(&nodes[1], &tx); + + lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); +} + +#[test] +fn test_splice_tiebreak_feerate_too_high() { + // Node 0 (winner) uses a feerate high enough that node 1's (loser) contribution cannot + // cover the fees. Node 1 proceeds without its contribution (QuiescentAction is preserved + // for a future splice). The splice completes with only node 0's inputs/outputs. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + provide_utxo_reserves(&nodes, 2, Amount::from_sat(100_000)); + + // Node 0 uses a high feerate (20,000 sat/kwu). Node 1 uses the floor feerate but + // splices in a large amount (95,000 sats from a 100,000 sat UTXO), leaving very little + // change/fee budget. Node 1's budget (~5,000 sats) can't cover the acceptor's fair fee + // at 20,000 sat/kwu, so adjust_for_feerate fails. + let high_feerate = FeeRate::from_sat_per_kwu(20_000); + let floor_feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let node_0_added_value = Amount::from_sat(50_000); + let node_1_added_value = Amount::from_sat(95_000); + + // Node 0: high feerate, moderate splice-in. + let funding_template_0 = + nodes[0].node.splice_channel(&channel_id, &node_id_1, high_feerate, FeeRate::MAX).unwrap(); + let wallet_0 = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let node_0_funding_contribution = + funding_template_0.splice_in_sync(node_0_added_value, &wallet_0).unwrap(); + nodes[0] + .node + .funding_contributed(&channel_id, &node_id_1, node_0_funding_contribution.clone(), None) + .unwrap(); + + // Node 1: floor feerate, tight budget (95,000 from 100,000 sat UTXO). + let funding_template_1 = + nodes[1].node.splice_channel(&channel_id, &node_id_0, floor_feerate, FeeRate::MAX).unwrap(); + let wallet_1 = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); + let node_1_funding_contribution = + funding_template_1.splice_in_sync(node_1_added_value, &wallet_1).unwrap(); + nodes[1] + .node + .funding_contributed(&channel_id, &node_id_0, node_1_funding_contribution.clone(), None) + .unwrap(); + + // Both emit STFU. + let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + let stfu_1 = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + + // Tie-break: node 0 wins. + nodes[1].node.handle_stfu(node_id_0, &stfu_0); + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); + nodes[0].node.handle_stfu(node_id_1, &stfu_1); + + // Node 0 sends SpliceInit at 20,000 sat/kwu. + let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); + + // Node 1 handles SpliceInit — adjust_for_feerate fails because node 1's contribution + // can't cover fees at 20,000 sat/kwu. Node 1 proceeds without its contribution. + nodes[1].node.handle_splice_init(node_id_0, &splice_init); + let splice_ack = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); + assert_eq!( + splice_ack.funding_contribution_satoshis, 0, + "Acceptor should not contribute when feerate adjustment fails" + ); + + // Node 0 handles SpliceAck — starts interactive tx construction. + nodes[0].node.handle_splice_ack(node_id_1, &splice_ack); + + let new_funding_script = chan_utils::make_funding_redeemscript( + &splice_init.funding_pubkey, + &splice_ack.funding_pubkey, + ) + .to_p2wsh(); + + // Complete with only node 0's contribution. + complete_interactive_funding_negotiation_for_both( + &nodes[0], + &nodes[1], + channel_id, + node_0_funding_contribution, + None, + new_funding_script, + ); + + // Sign (no acceptor contribution) and broadcast. + let (tx, splice_locked) = + sign_interactive_funding_tx_with_acceptor_contribution(&nodes[0], &nodes[1], false, false); + assert!(splice_locked.is_none()); + + expect_splice_pending_event(&nodes[0], &node_id_1); + expect_splice_pending_event(&nodes[1], &node_id_0); + + mine_transaction(&nodes[0], &tx); + mine_transaction(&nodes[1], &tx); + + // After splice_locked, node 1's preserved QuiescentAction triggers STFU. + let node_1_stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); + let stfu_1 = if let Some(MessageSendEvent::SendStfu { msg, .. }) = node_1_stfu { + assert!(msg.initiator); + msg + } else { + panic!("Expected SendStfu from node 1 after splice_locked"); + }; + + // === Part 2: Node 1 retries as initiator === + + // Node 0 receives node 1's STFU and responds with its own STFU. + nodes[0].node.handle_stfu(node_id_1, &stfu_1); + let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + + // Node 1 receives STFU → quiescence established → node 1 is the initiator → sends SpliceInit. + nodes[1].node.handle_stfu(node_id_0, &stfu_0); + let splice_init = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceInit, node_id_0); + + // Node 0 handles SpliceInit → sends SpliceAck. + nodes[0].node.handle_splice_init(node_id_1, &splice_init); + let splice_ack = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceAck, node_id_1); + + // Node 1 handles SpliceAck → starts interactive tx construction. + nodes[1].node.handle_splice_ack(node_id_0, &splice_ack); + + let new_funding_script_2 = chan_utils::make_funding_redeemscript( + &splice_init.funding_pubkey, + &splice_ack.funding_pubkey, + ) + .to_p2wsh(); + + // Complete interactive funding negotiation with node 1 as initiator (only node 1 contributes). + complete_interactive_funding_negotiation( + &nodes[1], + &nodes[0], + channel_id, + node_1_funding_contribution, + new_funding_script_2, + ); + + // Sign (no acceptor contribution) and broadcast. + let (new_splice_tx, splice_locked) = sign_interactive_funding_tx(&nodes[1], &nodes[0], false); + assert!(splice_locked.is_none()); + + expect_splice_pending_event(&nodes[1], &node_id_0); + expect_splice_pending_event(&nodes[0], &node_id_1); + + mine_transaction(&nodes[1], &new_splice_tx); + mine_transaction(&nodes[0], &new_splice_tx); + + lock_splice_after_blocks(&nodes[1], &nodes[0], ANTI_REORG_DELAY - 1); +} + #[cfg(test)] #[derive(PartialEq)] enum SpliceStatus { @@ -1754,8 +2198,8 @@ fn test_propose_splice_while_disconnected() { #[cfg(test)] fn do_test_propose_splice_while_disconnected(use_0conf: bool) { // Test that both nodes are able to propose a splice while the counterparty is disconnected, and - // whoever doesn't go first due to the quiescence tie-breaker, will retry their splice after the - // first one becomes locked. + // whoever doesn't go first due to the quiescence tie-breaker, will have their contribution + // merged into the counterparty-initiated splice. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let mut config = test_default_channel_config(); @@ -1835,23 +2279,28 @@ fn do_test_propose_splice_while_disconnected(use_0conf: bool) { .map(|monitor| (monitor.get_funding_txo(), monitor.get_funding_script())) .unwrap(); - // Negotiate the first splice to completion. + // Negotiate the splice to completion. Node 1's quiescent action should be consumed by + // splice_init, so both contributions are merged into a single splice. nodes[1].node.handle_splice_init(node_id_0, &splice_init); let splice_ack = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); + assert_ne!(splice_ack.funding_contribution_satoshis, 0); nodes[0].node.handle_splice_ack(node_id_1, &splice_ack); let new_funding_script = chan_utils::make_funding_redeemscript( &splice_init.funding_pubkey, &splice_ack.funding_pubkey, ) .to_p2wsh(); - complete_interactive_funding_negotiation( + complete_interactive_funding_negotiation_for_both( &nodes[0], &nodes[1], channel_id, node_0_funding_contribution, + Some(node_1_funding_contribution), new_funding_script, ); - let (splice_tx, splice_locked) = sign_interactive_funding_tx(&nodes[0], &nodes[1], use_0conf); + let (splice_tx, splice_locked) = sign_interactive_funding_tx_with_acceptor_contribution( + &nodes[0], &nodes[1], use_0conf, true, + ); expect_splice_pending_event(&nodes[0], &node_id_1); expect_splice_pending_event(&nodes[1], &node_id_0); @@ -1865,7 +2314,7 @@ fn do_test_propose_splice_while_disconnected(use_0conf: bool) { mine_transaction(&nodes[0], &splice_tx); mine_transaction(&nodes[1], &splice_tx); - // Mine enough blocks for the first splice to become locked. + // Mine enough blocks for the splice to become locked. connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1); connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1); @@ -1873,10 +2322,9 @@ fn do_test_propose_splice_while_disconnected(use_0conf: bool) { }; nodes[1].node.handle_splice_locked(node_id_0, &splice_locked); - // We should see the node which lost the tie-breaker attempt their splice now by first - // negotiating quiescence, but their `stfu` won't be sent until after another reconnection. + // Node 1's quiescent action was consumed, so it should NOT send stfu. let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), if use_0conf { 2 } else { 3 }, "{msg_events:?}"); + assert_eq!(msg_events.len(), if use_0conf { 1 } else { 2 }, "{msg_events:?}"); if let MessageSendEvent::SendSpliceLocked { ref msg, .. } = &msg_events[0] { nodes[0].node.handle_splice_locked(node_id_1, msg); if use_0conf { @@ -1897,10 +2345,6 @@ fn do_test_propose_splice_while_disconnected(use_0conf: bool) { panic!("Unexpected event {:?}", &msg_events[1]); } } - assert!(matches!( - &msg_events[if use_0conf { 1 } else { 2 }], - MessageSendEvent::SendStfu { .. } - )); let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), if use_0conf { 0 } else { 2 }, "{msg_events:?}"); @@ -1933,57 +2377,6 @@ fn do_test_propose_splice_while_disconnected(use_0conf: bool) { .chain_source .remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script); - // Reconnect the nodes. This should trigger the node which lost the tie-breaker to resend `stfu` - // for their splice attempt. - nodes[0].node.peer_disconnected(node_id_1); - nodes[1].node.peer_disconnected(node_id_0); - let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); - if !use_0conf { - reconnect_args.send_announcement_sigs = (true, true); - } - reconnect_args.send_stfu = (true, false); - reconnect_nodes(reconnect_args); - - // Drive the second splice to completion. - let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1, "{msg_events:?}"); - if let MessageSendEvent::SendStfu { ref msg, .. } = msg_events[0] { - nodes[1].node.handle_stfu(node_id_0, msg); - } else { - panic!("Unexpected event {:?}", &msg_events[0]); - } - - let splice_init = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceInit, node_id_0); - nodes[0].node.handle_splice_init(node_id_1, &splice_init); - let splice_ack = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceAck, node_id_1); - nodes[1].node.handle_splice_ack(node_id_0, &splice_ack); - let new_funding_script = chan_utils::make_funding_redeemscript( - &splice_init.funding_pubkey, - &splice_ack.funding_pubkey, - ) - .to_p2wsh(); - complete_interactive_funding_negotiation( - &nodes[1], - &nodes[0], - channel_id, - node_1_funding_contribution, - new_funding_script, - ); - let (splice_tx, splice_locked) = sign_interactive_funding_tx(&nodes[1], &nodes[0], use_0conf); - expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); - - if use_0conf { - let (splice_locked, for_node_id) = splice_locked.unwrap(); - assert_eq!(for_node_id, node_id_0); - lock_splice(&nodes[1], &nodes[0], &splice_locked, true); - } else { - assert!(splice_locked.is_none()); - mine_transaction(&nodes[0], &splice_tx); - mine_transaction(&nodes[1], &splice_tx); - lock_splice_after_blocks(&nodes[1], &nodes[0], ANTI_REORG_DELAY - 1); - } - // Sanity check that we can still make a test payment. send_payment(&nodes[0], &[&nodes[1]], 1_000_000); } From f94fbd72b90e66fb98ebff18ac3d3a55843f41c1 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Feb 2026 15:52:37 -0600 Subject: [PATCH 11/13] f - Add min/max feerate parameters to splice funding APIs Add FeeRateAdjustmentError enum with TooLow, TooHigh, and BudgetInsufficient variants to distinguish different feerate incompatibility scenarios. Add max_feerate field to FundingTemplate and FundingContribution. Update compute_feerate_adjustment to check feerate bounds: - Target below min_feerate (coin selection rate) -> TooLow - Target above max_feerate with fair_fee > estimated_fee -> TooHigh - Target above max_feerate but fair_fee <= estimated_fee -> allowed (change output not consumed, acceptor pays less than as initiator) - Budget insufficient within acceptable range -> BudgetInsufficient Change splice_channel and rbf_channel signatures from feerate -> (min_feerate, max_feerate). Coin selection uses min_feerate; as initiator, min_feerate is proposed in splice_init. Co-Authored-By: Claude Opus 4.6 --- lightning/src/ln/splicing_tests.rs | 148 ++++++++++++++++++++++++++++- 1 file changed, 143 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 40be32d6950..9ec3183b72c 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -1397,13 +1397,151 @@ fn test_splice_tiebreak_higher_feerate() { #[test] fn test_splice_tiebreak_lower_feerate() { - // Node 0 (winner) uses a lower feerate than node 1 (loser). Node 1's change output increases - // because the acceptor's fair fee decreases. Negotiation succeeds. + // Node 0 (winner) uses a lower feerate than node 1 (loser). Since the initiator's feerate + // is below node 1's minimum, node 1 proceeds without contribution let floor = FEERATE_FLOOR_SATS_PER_KW as u64; - do_test_splice_both_contribute_tiebreak( - Some(FeeRate::from_sat_per_kwu(floor)), - Some(FeeRate::from_sat_per_kwu(floor * 3)), + let node_0_feerate = FeeRate::from_sat_per_kwu(floor); + let node_1_feerate = FeeRate::from_sat_per_kwu(floor * 3); + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + + let added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, Amount::from_sat(100_000)); + + // Node 0: low feerate, moderate splice-in. + let funding_template_0 = nodes[0] + .node + .splice_channel(&channel_id, &node_id_1, node_0_feerate, FeeRate::MAX) + .unwrap(); + let wallet_0 = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let node_0_funding_contribution = + funding_template_0.splice_in_sync(added_value, &wallet_0).unwrap(); + nodes[0] + .node + .funding_contributed(&channel_id, &node_id_1, node_0_funding_contribution.clone(), None) + .unwrap(); + + // Node 1: higher feerate. + let funding_template_1 = nodes[1] + .node + .splice_channel(&channel_id, &node_id_0, node_1_feerate, FeeRate::MAX) + .unwrap(); + let wallet_1 = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); + let node_1_funding_contribution = + funding_template_1.splice_in_sync(added_value, &wallet_1).unwrap(); + nodes[1] + .node + .funding_contributed(&channel_id, &node_id_0, node_1_funding_contribution.clone(), None) + .unwrap(); + + // Both emit STFU. + let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + let stfu_1 = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + + // Tie-break: node 0 wins. + nodes[1].node.handle_stfu(node_id_0, &stfu_0); + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); + nodes[0].node.handle_stfu(node_id_1, &stfu_1); + + // Node 0 sends SpliceInit at its low feerate. + let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); + + // Node 1 handles SpliceInit — initiator's feerate is below node 1's minimum, + // so node 1 proceeds without contribution (QuiescentAction preserved for RBF). + nodes[1].node.handle_splice_init(node_id_0, &splice_init); + let splice_ack = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); + assert_eq!( + splice_ack.funding_contribution_satoshis, 0, + "Acceptor should not contribute when initiator's feerate is below minimum" ); + + // Node 0 handles SpliceAck — starts interactive tx construction. + nodes[0].node.handle_splice_ack(node_id_1, &splice_ack); + + let new_funding_script = chan_utils::make_funding_redeemscript( + &splice_init.funding_pubkey, + &splice_ack.funding_pubkey, + ) + .to_p2wsh(); + + // Complete with only node 0's contribution. + complete_interactive_funding_negotiation_for_both( + &nodes[0], + &nodes[1], + channel_id, + node_0_funding_contribution, + None, + new_funding_script, + ); + + // Sign (no acceptor contribution) and broadcast. + let (tx, splice_locked) = + sign_interactive_funding_tx_with_acceptor_contribution(&nodes[0], &nodes[1], false, false); + assert!(splice_locked.is_none()); + + expect_splice_pending_event(&nodes[0], &node_id_1); + expect_splice_pending_event(&nodes[1], &node_id_0); + + mine_transaction(&nodes[0], &tx); + mine_transaction(&nodes[1], &tx); + + // After splice_locked, node 1's preserved QuiescentAction triggers STFU for RBF retry. + let node_1_stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); + let stfu_1 = if let Some(MessageSendEvent::SendStfu { msg, .. }) = node_1_stfu { + assert!(msg.initiator); + msg + } else { + panic!("Expected SendStfu from node 1 after splice_locked"); + }; + + // === Part 2: Node 1 retries as initiator at its preferred feerate === + // TODO(splicing): Node 1 should retry contribution via RBF above instead + + nodes[0].node.handle_stfu(node_id_1, &stfu_1); + let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + + nodes[1].node.handle_stfu(node_id_0, &stfu_0); + let splice_init = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceInit, node_id_0); + + nodes[0].node.handle_splice_init(node_id_1, &splice_init); + let splice_ack = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceAck, node_id_1); + + nodes[1].node.handle_splice_ack(node_id_0, &splice_ack); + + let new_funding_script_2 = chan_utils::make_funding_redeemscript( + &splice_init.funding_pubkey, + &splice_ack.funding_pubkey, + ) + .to_p2wsh(); + + complete_interactive_funding_negotiation( + &nodes[1], + &nodes[0], + channel_id, + node_1_funding_contribution, + new_funding_script_2, + ); + + let (new_splice_tx, splice_locked) = sign_interactive_funding_tx(&nodes[1], &nodes[0], false); + assert!(splice_locked.is_none()); + + expect_splice_pending_event(&nodes[1], &node_id_0); + expect_splice_pending_event(&nodes[0], &node_id_1); + + mine_transaction(&nodes[1], &new_splice_tx); + mine_transaction(&nodes[0], &new_splice_tx); + + lock_splice_after_blocks(&nodes[1], &nodes[0], ANTI_REORG_DELAY - 1); } /// Runs the splice tie-breaker test with optional per-node feerates. From 6fd11bdbd5a4d28928d9a30bde9d53492deb0834 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Feb 2026 10:43:05 -0600 Subject: [PATCH 12/13] f - log feerate --- lightning/src/ln/channel.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 40f1a16cfcd..d4bfe1b6430 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12444,8 +12444,9 @@ where .map_err(|e| { log_info!( logger, - "Cannot accommodate initiator's feerate for channel {}: {}; \ + "Cannot accommodate initiator's feerate ({}) for channel {}: {}; \ proceeding without contribution", + feerate, self.context.channel_id(), e, ); From 6d75f9928ddb2f7890519e2088ec9d1d9fb9a0a4 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 25 Feb 2026 10:51:04 -0600 Subject: [PATCH 13/13] Refactor complete_interactive_funding_negotiation_for_both Use a single get_and_clear_pending_msg_events() + match pattern for the initiator's turn, matching the existing acceptor code path. Also add assertions that all expected initiator inputs and outputs were sent. Co-Authored-By: Claude Opus 4.6 --- lightning/src/ln/splicing_tests.rs | 82 +++++++++++++++--------------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 9ec3183b72c..284d6fbf764 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -354,50 +354,50 @@ pub fn complete_interactive_funding_negotiation_for_both<'a, 'b, 'c, 'd>( (Vec::new(), Vec::new()) }; - let mut acceptor_sent_tx_complete = false; let mut initiator_sent_tx_complete; + let mut acceptor_sent_tx_complete = false; loop { // Initiator's turn: send TxAddInput, TxAddOutput, or TxComplete - if !expected_initiator_inputs.is_empty() { - let tx_add_input = - get_event_msg!(initiator, MessageSendEvent::SendTxAddInput, node_id_acceptor); - let input_prevout = BitcoinOutPoint { - txid: tx_add_input - .prevtx - .as_ref() - .map(|prevtx| prevtx.compute_txid()) - .or(tx_add_input.shared_input_txid) - .unwrap(), - vout: tx_add_input.prevtx_out, - }; - expected_initiator_inputs.remove( - expected_initiator_inputs.iter().position(|input| *input == input_prevout).unwrap(), - ); - acceptor.node.handle_tx_add_input(node_id_initiator, &tx_add_input); - initiator_sent_tx_complete = false; - } else if !expected_initiator_scripts.is_empty() { - let tx_add_output = - get_event_msg!(initiator, MessageSendEvent::SendTxAddOutput, node_id_acceptor); - expected_initiator_scripts.remove( - expected_initiator_scripts - .iter() - .position(|script| *script == tx_add_output.script) - .unwrap(), - ); - acceptor.node.handle_tx_add_output(node_id_initiator, &tx_add_output); - initiator_sent_tx_complete = false; - } else { - let msg_events = initiator.node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1, "{msg_events:?}"); - if let MessageSendEvent::SendTxComplete { ref msg, .. } = &msg_events[0] { + let msg_events = initiator.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + match &msg_events[0] { + MessageSendEvent::SendTxAddInput { msg, .. } => { + let input_prevout = BitcoinOutPoint { + txid: msg + .prevtx + .as_ref() + .map(|prevtx| prevtx.compute_txid()) + .or(msg.shared_input_txid) + .unwrap(), + vout: msg.prevtx_out, + }; + expected_initiator_inputs.remove( + expected_initiator_inputs + .iter() + .position(|input| *input == input_prevout) + .unwrap(), + ); + acceptor.node.handle_tx_add_input(node_id_initiator, msg); + initiator_sent_tx_complete = false; + }, + MessageSendEvent::SendTxAddOutput { msg, .. } => { + expected_initiator_scripts.remove( + expected_initiator_scripts + .iter() + .position(|script| *script == msg.script) + .unwrap(), + ); + acceptor.node.handle_tx_add_output(node_id_initiator, msg); + initiator_sent_tx_complete = false; + }, + MessageSendEvent::SendTxComplete { msg, .. } => { acceptor.node.handle_tx_complete(node_id_initiator, msg); - } else { - panic!(); - } - initiator_sent_tx_complete = true; - if acceptor_sent_tx_complete { - break; - } + initiator_sent_tx_complete = true; + if acceptor_sent_tx_complete { + break; + } + }, + _ => panic!("Unexpected message event: {:?}", msg_events[0]), } // Acceptor's turn: send TxAddInput, TxAddOutput, or TxComplete @@ -444,6 +444,8 @@ pub fn complete_interactive_funding_negotiation_for_both<'a, 'b, 'c, 'd>( } } + assert!(expected_initiator_inputs.is_empty(), "Not all initiator inputs were sent"); + assert!(expected_initiator_scripts.is_empty(), "Not all initiator outputs were sent"); assert!(expected_acceptor_inputs.is_empty(), "Not all acceptor inputs were sent"); assert!(expected_acceptor_scripts.is_empty(), "Not all acceptor outputs were sent"); }