From 445fb74095b0990e8919dc0ab3a4bad19ee9d317 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Tue, 3 Jun 2025 11:21:15 +0200 Subject: [PATCH 01/22] Rename DualFundingContext This is a simple rename, DualFundingContext to FundingNegotiationContext, to suggest that this is use not only in dual-funded channel open. Also rename the field dual_funding_context to funding_negotiation_context. --- lightning/src/ln/channel.rs | 38 +++++++++++++++--------------- lightning/src/ln/channelmanager.rs | 2 +- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index fc161c5a7c6..b349d4b10c8 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2777,7 +2777,7 @@ where debug_assert!(self.interactive_tx_constructor.is_none()); let mut funding_inputs = Vec::new(); - mem::swap(&mut self.dual_funding_context.our_funding_inputs, &mut funding_inputs); + mem::swap(&mut self.funding_negotiation_context.our_funding_inputs, &mut funding_inputs); // TODO(splicing): Add prev funding tx as input, must be provided as a parameter @@ -2799,10 +2799,10 @@ where .map_err(|_err| AbortReason::InternalError("Error getting destination script"))? }; let change_value_opt = calculate_change_output_value( - self.funding.is_outbound(), self.dual_funding_context.our_funding_satoshis, + self.funding.is_outbound(), self.funding_negotiation_context.our_funding_satoshis, &funding_inputs, None, &shared_funding_output.script_pubkey, &funding_outputs, - self.dual_funding_context.funding_feerate_sat_per_1000_weight, + self.funding_negotiation_context.funding_feerate_sat_per_1000_weight, change_script.minimal_non_dust().to_sat(), )?; if let Some(change_value) = change_value_opt { @@ -2811,7 +2811,7 @@ where 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.dual_funding_context.funding_feerate_sat_per_1000_weight, change_output_weight); + let change_output_fee = fee_for_weight(self.funding_negotiation_context.funding_feerate_sat_per_1000_weight, change_output_weight); let change_value_decreased_with_fee = change_value.saturating_sub(change_output_fee); // Check dust limit again if change_value_decreased_with_fee > self.context.holder_dust_limit_satoshis { @@ -2825,12 +2825,12 @@ where holder_node_id, counterparty_node_id: self.context.counterparty_node_id, channel_id: self.context.channel_id(), - feerate_sat_per_kw: self.dual_funding_context.funding_feerate_sat_per_1000_weight, + feerate_sat_per_kw: self.funding_negotiation_context.funding_feerate_sat_per_1000_weight, is_initiator: self.funding.is_outbound(), - funding_tx_locktime: self.dual_funding_context.funding_tx_locktime, + funding_tx_locktime: self.funding_negotiation_context.funding_tx_locktime, inputs_to_contribute: funding_inputs, shared_funding_input: None, - shared_funding_output: SharedOwnedOutput::new(shared_funding_output, self.dual_funding_context.our_funding_satoshis), + shared_funding_output: SharedOwnedOutput::new(shared_funding_output, self.funding_negotiation_context.our_funding_satoshis), outputs_to_contribute: funding_outputs, }; let mut tx_constructor = InteractiveTxConstructor::new(constructor_args)?; @@ -2920,7 +2920,7 @@ where where L::Target: Logger { - let our_funding_satoshis = self.dual_funding_context.our_funding_satoshis; + let our_funding_satoshis = self.funding_negotiation_context.our_funding_satoshis; let transaction_number = self.unfunded_context.transaction_number(); let mut output_index = None; @@ -5841,8 +5841,8 @@ fn check_v2_funding_inputs_sufficient( } } -/// Context for dual-funded channels. -pub(super) struct DualFundingChannelContext { +/// Context for negotiating channels (dual-funded V2 open, splicing) +pub(super) struct FundingNegotiationContext { /// The amount in satoshis we will be contributing to the channel. pub our_funding_satoshis: u64, /// The amount in satoshis our counterparty will be contributing to the channel. @@ -11796,7 +11796,7 @@ where pub funding: FundingScope, pub context: ChannelContext, pub unfunded_context: UnfundedChannelContext, - pub dual_funding_context: DualFundingChannelContext, + pub funding_negotiation_context: FundingNegotiationContext, /// The current interactive transaction construction session under negotiation. pub interactive_tx_constructor: Option, /// The signing session created after `tx_complete` handling @@ -11859,7 +11859,7 @@ where unfunded_channel_age_ticks: 0, holder_commitment_point: HolderCommitmentPoint::new(&context.holder_signer, &context.secp_ctx), }; - let dual_funding_context = DualFundingChannelContext { + let funding_negotiation_context = FundingNegotiationContext { our_funding_satoshis: funding_satoshis, // TODO(dual_funding) TODO(splicing) Include counterparty contribution, once that's enabled their_funding_satoshis: None, @@ -11871,7 +11871,7 @@ where funding, context, unfunded_context, - dual_funding_context, + funding_negotiation_context, interactive_tx_constructor: None, interactive_tx_signing_session: None, }; @@ -11947,7 +11947,7 @@ where }, funding_feerate_sat_per_1000_weight: self.context.feerate_per_kw, second_per_commitment_point, - locktime: self.dual_funding_context.funding_tx_locktime.to_consensus_u32(), + locktime: self.funding_negotiation_context.funding_tx_locktime.to_consensus_u32(), require_confirmed_inputs: None, } } @@ -12019,7 +12019,7 @@ where &funding.get_counterparty_pubkeys().revocation_basepoint); context.channel_id = channel_id; - let dual_funding_context = DualFundingChannelContext { + let funding_negotiation_context = FundingNegotiationContext { our_funding_satoshis: our_funding_satoshis, their_funding_satoshis: Some(msg.common_fields.funding_satoshis), funding_tx_locktime: LockTime::from_consensus(msg.locktime), @@ -12037,8 +12037,8 @@ where holder_node_id, counterparty_node_id, channel_id: context.channel_id, - feerate_sat_per_kw: dual_funding_context.funding_feerate_sat_per_1000_weight, - funding_tx_locktime: dual_funding_context.funding_tx_locktime, + 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, @@ -12057,7 +12057,7 @@ where Ok(Self { funding, context, - dual_funding_context, + funding_negotiation_context, interactive_tx_constructor, interactive_tx_signing_session: None, unfunded_context, @@ -12123,7 +12123,7 @@ where }), channel_type: Some(self.funding.get_channel_type().clone()), }, - funding_satoshis: self.dual_funding_context.our_funding_satoshis, + funding_satoshis: self.funding_negotiation_context.our_funding_satoshis, second_per_commitment_point, require_confirmed_inputs: None, } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index d0d429a0abc..e4016e79535 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8481,7 +8481,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ // Inbound V2 channels with contributed inputs are not considered unfunded. if let Some(unfunded_chan) = chan.as_unfunded_v2() { - if unfunded_chan.dual_funding_context.our_funding_satoshis != 0 { + if unfunded_chan.funding_negotiation_context.our_funding_satoshis != 0 { continue; } } From 08a303f991046942b2f493a1cde0fafc6c790c9d Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Tue, 17 Jun 2025 05:30:42 +0200 Subject: [PATCH 02/22] Extend begin_interactive_...() with splicing-specific parameters The begin_interactive_funding_tx_construction() method is extended with `is_initiator` parameter (splice initiator), and `prev_funding_input` optional parameter, containing the previous funding transaction, which will be added to the negotiation as an input by the initiator. --- lightning/src/ln/channel.rs | 42 ++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index b349d4b10c8..448e258b4bf 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2766,12 +2766,13 @@ where /// default destination address is used. /// If error occurs, it is caused by our side, not the counterparty. #[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled - #[rustfmt::skip] fn begin_interactive_funding_tx_construction( &mut self, signer_provider: &SP, entropy_source: &ES, holder_node_id: PublicKey, - change_destination_opt: Option, + is_initiator: bool, change_destination_opt: Option, + prev_funding_input: Option<(TxIn, TransactionU16LenLimited)>, ) -> Result, AbortReason> - where ES::Target: EntropySource + where + ES::Target: EntropySource, { debug_assert!(matches!(self.context.channel_state, ChannelState::NegotiatingFunding(_))); debug_assert!(self.interactive_tx_constructor.is_none()); @@ -2779,7 +2780,11 @@ where let mut funding_inputs = Vec::new(); mem::swap(&mut self.funding_negotiation_context.our_funding_inputs, &mut funding_inputs); - // TODO(splicing): Add prev funding tx as input, must be provided as a parameter + if is_initiator { + if let Some(prev_funding_input) = prev_funding_input { + funding_inputs.push(prev_funding_input); + } + } // Add output for funding tx // Note: For the error case when the inputs are insufficient, it will be handled after @@ -2795,23 +2800,28 @@ where let change_script = if let Some(script) = change_destination_opt { script } else { - signer_provider.get_destination_script(self.context.channel_keys_id) + signer_provider + .get_destination_script(self.context.channel_keys_id) .map_err(|_err| AbortReason::InternalError("Error getting destination script"))? }; let change_value_opt = calculate_change_output_value( - self.funding.is_outbound(), self.funding_negotiation_context.our_funding_satoshis, - &funding_inputs, None, - &shared_funding_output.script_pubkey, &funding_outputs, + is_initiator, + self.funding_negotiation_context.our_funding_satoshis, + &funding_inputs, + None, + &shared_funding_output.script_pubkey, + &funding_outputs, self.funding_negotiation_context.funding_feerate_sat_per_1000_weight, change_script.minimal_non_dust().to_sat(), )?; if let Some(change_value) = change_value_opt { - let mut change_output = TxOut { - value: Amount::from_sat(change_value), - script_pubkey: change_script, - }; + let mut change_output = + TxOut { value: Amount::from_sat(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_negotiation_context.funding_feerate_sat_per_1000_weight, change_output_weight); + let change_output_fee = fee_for_weight( + self.funding_negotiation_context.funding_feerate_sat_per_1000_weight, + change_output_weight, + ); let change_value_decreased_with_fee = change_value.saturating_sub(change_output_fee); // Check dust limit again if change_value_decreased_with_fee > self.context.holder_dust_limit_satoshis { @@ -2825,8 +2835,10 @@ where holder_node_id, counterparty_node_id: self.context.counterparty_node_id, channel_id: self.context.channel_id(), - feerate_sat_per_kw: self.funding_negotiation_context.funding_feerate_sat_per_1000_weight, - is_initiator: self.funding.is_outbound(), + feerate_sat_per_kw: self + .funding_negotiation_context + .funding_feerate_sat_per_1000_weight, + is_initiator, funding_tx_locktime: self.funding_negotiation_context.funding_tx_locktime, inputs_to_contribute: funding_inputs, shared_funding_input: None, From 1c65e1885767500f92d6c01eda626acde523b0d0 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 16 Jul 2025 10:45:42 -0500 Subject: [PATCH 03/22] f - rustfmt --- lightning/src/ln/channel.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 448e258b4bf..8ab2f570d1e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2842,7 +2842,10 @@ where funding_tx_locktime: self.funding_negotiation_context.funding_tx_locktime, inputs_to_contribute: funding_inputs, shared_funding_input: None, - shared_funding_output: SharedOwnedOutput::new(shared_funding_output, self.funding_negotiation_context.our_funding_satoshis), + shared_funding_output: SharedOwnedOutput::new( + shared_funding_output, + self.funding_negotiation_context.our_funding_satoshis, + ), outputs_to_contribute: funding_outputs, }; let mut tx_constructor = InteractiveTxConstructor::new(constructor_args)?; From 755511fd2e45226ea965ab5094b171c1e9c83de6 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 16 Jul 2025 10:43:41 -0500 Subject: [PATCH 04/22] f - include shared_funding_input --- lightning/src/ln/channel.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 8ab2f570d1e..207d57a0f52 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -58,8 +58,8 @@ use crate::ln::channelmanager::{ use crate::ln::interactivetxs::{ calculate_change_output_value, get_output_weight, AbortReason, HandleTxCompleteResult, InteractiveTxConstructor, InteractiveTxConstructorArgs, InteractiveTxMessageSend, - InteractiveTxMessageSendResult, InteractiveTxSigningSession, SharedOwnedOutput, - TX_COMMON_FIELDS_WEIGHT, + InteractiveTxMessageSendResult, InteractiveTxSigningSession, SharedOwnedInput, + SharedOwnedOutput, TX_COMMON_FIELDS_WEIGHT, }; use crate::ln::msgs; use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError, OnionErrorPacket}; @@ -2769,7 +2769,7 @@ where fn begin_interactive_funding_tx_construction( &mut self, signer_provider: &SP, entropy_source: &ES, holder_node_id: PublicKey, is_initiator: bool, change_destination_opt: Option, - prev_funding_input: Option<(TxIn, TransactionU16LenLimited)>, + shared_funding_input: Option, ) -> Result, AbortReason> where ES::Target: EntropySource, @@ -2780,12 +2780,6 @@ where let mut funding_inputs = Vec::new(); mem::swap(&mut self.funding_negotiation_context.our_funding_inputs, &mut funding_inputs); - if is_initiator { - if let Some(prev_funding_input) = prev_funding_input { - funding_inputs.push(prev_funding_input); - } - } - // Add output for funding tx // Note: For the error case when the inputs are insufficient, it will be handled after // the `calculate_change_output_value` call below @@ -2841,7 +2835,7 @@ where is_initiator, funding_tx_locktime: self.funding_negotiation_context.funding_tx_locktime, inputs_to_contribute: funding_inputs, - shared_funding_input: None, + shared_funding_input, shared_funding_output: SharedOwnedOutput::new( shared_funding_output, self.funding_negotiation_context.our_funding_satoshis, From ec4adec559af449182ce3e4c9ce5f5c2a816b5b5 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 16 Jul 2025 11:52:45 -0500 Subject: [PATCH 05/22] f - pass FundingNegotiationContext to calculate_change_output_value --- lightning/src/ln/channel.rs | 20 ++-- lightning/src/ln/interactivetxs.rs | 178 +++++++++++++---------------- 2 files changed, 89 insertions(+), 109 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 207d57a0f52..e1eeedd2b21 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2768,8 +2768,7 @@ where #[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled fn begin_interactive_funding_tx_construction( &mut self, signer_provider: &SP, entropy_source: &ES, holder_node_id: PublicKey, - is_initiator: bool, change_destination_opt: Option, - shared_funding_input: Option, + change_destination_opt: Option, shared_funding_input: Option, ) -> Result, AbortReason> where ES::Target: EntropySource, @@ -2777,9 +2776,6 @@ where debug_assert!(matches!(self.context.channel_state, ChannelState::NegotiatingFunding(_))); debug_assert!(self.interactive_tx_constructor.is_none()); - let mut funding_inputs = Vec::new(); - mem::swap(&mut self.funding_negotiation_context.our_funding_inputs, &mut funding_inputs); - // Add output for funding tx // Note: For the error case when the inputs are insufficient, it will be handled after // the `calculate_change_output_value` call below @@ -2799,13 +2795,10 @@ where .map_err(|_err| AbortReason::InternalError("Error getting destination script"))? }; let change_value_opt = calculate_change_output_value( - is_initiator, - self.funding_negotiation_context.our_funding_satoshis, - &funding_inputs, + &self.funding_negotiation_context, None, &shared_funding_output.script_pubkey, &funding_outputs, - self.funding_negotiation_context.funding_feerate_sat_per_1000_weight, change_script.minimal_non_dust().to_sat(), )?; if let Some(change_value) = change_value_opt { @@ -2824,6 +2817,9 @@ where } } + let mut funding_inputs = Vec::new(); + mem::swap(&mut self.funding_negotiation_context.our_funding_inputs, &mut funding_inputs); + let constructor_args = InteractiveTxConstructorArgs { entropy_source, holder_node_id, @@ -2832,7 +2828,7 @@ where feerate_sat_per_kw: self .funding_negotiation_context .funding_feerate_sat_per_1000_weight, - is_initiator, + is_initiator: self.funding_negotiation_context.is_initiator, funding_tx_locktime: self.funding_negotiation_context.funding_tx_locktime, inputs_to_contribute: funding_inputs, shared_funding_input, @@ -5852,6 +5848,8 @@ fn check_v2_funding_inputs_sufficient( /// Context for negotiating channels (dual-funded V2 open, splicing) pub(super) struct FundingNegotiationContext { + /// Whether we initiated the funding negotiation. + pub is_initiator: bool, /// The amount in satoshis we will be contributing to the channel. pub our_funding_satoshis: u64, /// The amount in satoshis our counterparty will be contributing to the channel. @@ -11869,6 +11867,7 @@ where holder_commitment_point: HolderCommitmentPoint::new(&context.holder_signer, &context.secp_ctx), }; let funding_negotiation_context = FundingNegotiationContext { + is_initiator: true, our_funding_satoshis: funding_satoshis, // TODO(dual_funding) TODO(splicing) Include counterparty contribution, once that's enabled their_funding_satoshis: None, @@ -12029,6 +12028,7 @@ where context.channel_id = channel_id; let funding_negotiation_context = FundingNegotiationContext { + is_initiator: false, our_funding_satoshis: our_funding_satoshis, their_funding_satoshis: Some(msg.common_fields.funding_satoshis), funding_tx_locktime: LockTime::from_consensus(msg.locktime), diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index acf28cf8fd3..28bed0e4b09 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -22,7 +22,7 @@ use bitcoin::{OutPoint, ScriptBuf, Sequence, Transaction, TxIn, TxOut, Txid, Wei use crate::chain::chaininterface::fee_for_weight; use crate::events::bump_transaction::{BASE_INPUT_WEIGHT, EMPTY_SCRIPT_SIG_WEIGHT}; use crate::ln::chan_utils::FUNDING_TRANSACTION_WITNESS_WEIGHT; -use crate::ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS; +use crate::ln::channel::{FundingNegotiationContext, TOTAL_BITCOIN_SUPPLY_SATOSHIS}; use crate::ln::msgs; use crate::ln::msgs::{MessageSendEvent, SerialId, TxSignatures}; use crate::ln::types::ChannelId; @@ -1874,26 +1874,21 @@ impl InteractiveTxConstructor { /// `Err(AbortReason::InsufficientFees)` /// /// Parameters: -/// - `is_initiator` - Whether we are the negotiation initiator or not (acceptor). -/// - `our_contribution` - The sats amount we intend to contribute to the funding -/// transaction being negotiated. -/// - `funding_inputs` - List of our inputs. It does not include the shared input, if there is one. +/// - `context` - Context of the funding negotiation, including non-shared inputs and feerate. /// - `shared_input` - The locally owned amount of the shared input (in sats), if there is one. /// - `shared_output_funding_script` - The script of the shared output. /// - `funding_outputs` - Our funding outputs. -/// - `funding_feerate_sat_per_1000_weight` - Fee rate to be used. /// - `change_output_dust_limit` - The dust limit (in sats) to consider. pub(super) fn calculate_change_output_value( - is_initiator: bool, our_contribution: u64, - funding_inputs: &Vec<(TxIn, TransactionU16LenLimited)>, shared_input: Option, + context: &FundingNegotiationContext, shared_input: Option, shared_output_funding_script: &ScriptBuf, funding_outputs: &Vec, - funding_feerate_sat_per_1000_weight: u32, change_output_dust_limit: u64, + change_output_dust_limit: u64, ) -> Result, AbortReason> { // Process inputs and their prev txs: // calculate value sum and weight sum of inputs, also perform checks let mut total_input_satoshis = 0u64; let mut our_funding_inputs_weight = 0u64; - for (txin, tx) in funding_inputs.iter() { + for (txin, tx) in context.our_funding_inputs.iter() { let txid = tx.as_transaction().compute_txid(); if txin.previous_output.txid != txid { return Err(AbortReason::PrevTxOutInvalid); @@ -1922,7 +1917,7 @@ pub(super) fn calculate_change_output_value( // If we are the initiator, we must pay for the weight of the funding output and // all common fields in the funding transaction. - if is_initiator { + 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); @@ -1931,15 +1926,15 @@ pub(super) fn calculate_change_output_value( } } - let fees_sats = fee_for_weight(funding_feerate_sat_per_1000_weight, weight); + let fees_sats = fee_for_weight(context.funding_feerate_sat_per_1000_weight, weight); let net_total_less_fees = total_input_satoshis.saturating_sub(total_output_satoshis).saturating_sub(fees_sats); - if net_total_less_fees < our_contribution { + if net_total_less_fees < context.our_funding_satoshis { // Not enough to cover contribution plus fees return Err(AbortReason::InsufficientFees); } - let remaining_value = net_total_less_fees.saturating_sub(our_contribution); + let remaining_value = net_total_less_fees.saturating_sub(context.our_funding_satoshis); if remaining_value < change_output_dust_limit { // Enough to cover contribution plus fees, but leftover is below dust limit; no change Ok(None) @@ -1952,7 +1947,7 @@ pub(super) fn calculate_change_output_value( #[cfg(test)] mod tests { use crate::chain::chaininterface::{fee_for_weight, FEERATE_FLOOR_SATS_PER_KW}; - use crate::ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS; + use crate::ln::channel::{FundingNegotiationContext, TOTAL_BITCOIN_SUPPLY_SATOSHIS}; use crate::ln::interactivetxs::{ calculate_change_output_value, generate_holder_serial_id, AbortReason, HandleTxCompleteValue, InteractiveTxConstructor, InteractiveTxConstructorArgs, @@ -2980,89 +2975,74 @@ mod tests { let gross_change = total_inputs - total_outputs - our_contributed; let fees = 1746; let common_fees = 234; - { - // There is leftover for change - let res = calculate_change_output_value( - true, - our_contributed, - &inputs, - None, - &ScriptBuf::new(), - &outputs, - funding_feerate_sat_per_1000_weight, - 300, - ); - assert_eq!(res, Ok(Some(gross_change - fees - common_fees))); - } - { - // There is leftover for change, without common fees - let res = calculate_change_output_value( - false, - our_contributed, - &inputs, - None, - &ScriptBuf::new(), - &outputs, - funding_feerate_sat_per_1000_weight, - 300, - ); - assert_eq!(res, Ok(Some(gross_change - fees))); - } - { - // Larger fee, smaller change - let res = calculate_change_output_value( - true, - our_contributed, - &inputs, - None, - &ScriptBuf::new(), - &outputs, - funding_feerate_sat_per_1000_weight * 3, - 300, - ); - assert_eq!(res, Ok(Some(4060))); - } - { - // Insufficient inputs, no leftover - let res = calculate_change_output_value( - false, - 130_000, - &inputs, - None, - &ScriptBuf::new(), - &outputs, - funding_feerate_sat_per_1000_weight, - 300, - ); - assert_eq!(res, Err(AbortReason::InsufficientFees)); - } - { - // Very small leftover - let res = calculate_change_output_value( - false, - 118_000, - &inputs, - None, - &ScriptBuf::new(), - &outputs, - funding_feerate_sat_per_1000_weight, - 300, - ); - assert_eq!(res, Ok(None)); - } - { - // Small leftover, but not dust - let res = calculate_change_output_value( - false, - 117_992, - &inputs, - None, - &ScriptBuf::new(), - &outputs, - funding_feerate_sat_per_1000_weight, - 100, - ); - assert_eq!(res, Ok(Some(262))); - } + + // There is leftover for change + let context = FundingNegotiationContext { + is_initiator: true, + our_funding_satoshis: our_contributed, + their_funding_satoshis: None, + funding_tx_locktime: AbsoluteLockTime::ZERO, + funding_feerate_sat_per_1000_weight, + our_funding_inputs: inputs, + }; + assert_eq!( + calculate_change_output_value(&context, None, &ScriptBuf::new(), &outputs, 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, None, &ScriptBuf::new(), &outputs, 300), + Ok(Some(gross_change - fees)), + ); + + // Larger fee, smaller change + let context = FundingNegotiationContext { + is_initiator: true, + funding_feerate_sat_per_1000_weight: funding_feerate_sat_per_1000_weight * 3, + ..context + }; + assert_eq!( + calculate_change_output_value(&context, None, &ScriptBuf::new(), &outputs, 300), + Ok(Some(4060)), + ); + + // Insufficient inputs, no leftover + let context = FundingNegotiationContext { + is_initiator: false, + our_funding_satoshis: 130_000, + funding_feerate_sat_per_1000_weight: funding_feerate_sat_per_1000_weight, + ..context + }; + assert_eq!( + calculate_change_output_value(&context, None, &ScriptBuf::new(), &outputs, 300), + Err(AbortReason::InsufficientFees), + ); + + // Very small leftover + let context = FundingNegotiationContext { + is_initiator: false, + our_funding_satoshis: 118_000, + ..context + }; + assert_eq!( + calculate_change_output_value(&context, None, &ScriptBuf::new(), &outputs, 300), + Ok(None), + ); + + // Small leftover, but not dust + let context = FundingNegotiationContext { + is_initiator: false, + our_funding_satoshis: 117_992, + ..context + }; + assert_eq!( + calculate_change_output_value(&context, None, &ScriptBuf::new(), &outputs, 100), + Ok(Some(262)), + ); } } From 1279a86d5e0c8f8ebe63ab8d7548993e41ed5b82 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 16 Jul 2025 11:56:40 -0500 Subject: [PATCH 06/22] f - reorder test --- lightning/src/ln/interactivetxs.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 28bed0e4b09..3633c329bf1 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -3000,22 +3000,10 @@ mod tests { Ok(Some(gross_change - fees)), ); - // Larger fee, smaller change - let context = FundingNegotiationContext { - is_initiator: true, - funding_feerate_sat_per_1000_weight: funding_feerate_sat_per_1000_weight * 3, - ..context - }; - assert_eq!( - calculate_change_output_value(&context, None, &ScriptBuf::new(), &outputs, 300), - Ok(Some(4060)), - ); - // Insufficient inputs, no leftover let context = FundingNegotiationContext { is_initiator: false, our_funding_satoshis: 130_000, - funding_feerate_sat_per_1000_weight: funding_feerate_sat_per_1000_weight, ..context }; assert_eq!( @@ -3044,5 +3032,17 @@ mod tests { calculate_change_output_value(&context, None, &ScriptBuf::new(), &outputs, 100), Ok(Some(262)), ); + + // Larger fee, smaller change + let context = FundingNegotiationContext { + is_initiator: true, + our_funding_satoshis: our_contributed, + funding_feerate_sat_per_1000_weight: funding_feerate_sat_per_1000_weight * 3, + ..context + }; + assert_eq!( + calculate_change_output_value(&context, None, &ScriptBuf::new(), &outputs, 300), + Ok(Some(4060)), + ); } } From a1433feafb0d894373a8c62bec29d490e410862a Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Tue, 17 Jun 2025 05:43:37 +0200 Subject: [PATCH 07/22] Introduce NegotiatingChannelView to bridge Funded and PendingV2 Introduce struct NegotiatingChannelView to perform transaction negotiation logic, on top of either PendingV2Channel (dual-funded channel open) or FundedChannel (splicing). --- lightning/src/ln/channel.rs | 159 ++++++++++++++++++++++++----- lightning/src/ln/channelmanager.rs | 65 ++++-------- 2 files changed, 156 insertions(+), 68 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e1eeedd2b21..8baa35ec75b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1732,6 +1732,19 @@ where } } + pub fn as_negotiating_channel(&mut self) -> Result, ChannelError> { + match &mut self.phase { + ChannelPhase::UnfundedV2(chan) => Ok(chan.as_negotiating_channel()), + #[cfg(splicing)] + ChannelPhase::Funded(chan) => { + Ok(chan.as_renegotiating_channel().map_err(|err| ChannelError::Warn(err.into()))?) + }, + _ => Err(ChannelError::Warn( + "Got a transaction negotiation message in an invalid phase".to_owned(), + )), + } + } + #[rustfmt::skip] pub fn funding_signed( &mut self, msg: &msgs::FundingSigned, best_block: BestBlock, signer_provider: &SP, logger: &L @@ -1770,9 +1783,11 @@ where where L::Target: Logger, { - if let ChannelPhase::UnfundedV2(chan) = &mut self.phase { - let logger = WithChannelContext::from(logger, &chan.context, None); - chan.funding_tx_constructed(signing_session, &&logger) + let logger = WithChannelContext::from(logger, self.context(), None); + if let Ok(mut negotiating_channel) = self.as_negotiating_channel() { + let (commitment_signed, event) = + negotiating_channel.funding_tx_constructed(signing_session, &&logger)?; + Ok((commitment_signed, event)) } else { Err(ChannelError::Warn("Got a tx_complete message with no interactive transaction construction expected or in-progress".to_owned())) } @@ -2175,8 +2190,13 @@ impl FundingScope { /// Info about a pending splice, used in the pre-splice channel #[cfg(splicing)] struct PendingSplice { + /// Intended contributions to the splice from our end pub our_funding_contribution: i64, funding: Option, + funding_negotiation_context: FundingNegotiationContext, + /// The current interactive transaction construction session under negotiation. + interactive_tx_constructor: Option, + interactive_tx_signing_session: Option, /// The funding txid used in the `splice_locked` sent to the counterparty. sent_funding_txid: Option, @@ -2757,7 +2777,23 @@ where } } -impl PendingV2Channel +/// A short-lived subset view of a channel, used for V2 funding negotiation or re-negotiation. +/// Can be produced by: +/// - [`PendingV2Channel`], at V2 channel open, and +/// - [`FundedChannel`], when splicing. +pub struct NegotiatingChannelView<'a, SP: Deref> +where + SP::Target: SignerProvider, +{ + context: &'a mut ChannelContext, + funding: &'a mut FundingScope, + funding_negotiation_context: &'a mut FundingNegotiationContext, + interactive_tx_constructor: &'a mut Option, + interactive_tx_signing_session: &'a mut Option, + holder_commitment_transaction_number: u64, +} + +impl<'a, SP: Deref> NegotiatingChannelView<'a, SP> where SP::Target: SignerProvider, { @@ -2841,13 +2877,15 @@ where let mut tx_constructor = InteractiveTxConstructor::new(constructor_args)?; let msg = tx_constructor.take_initiator_first_message(); - self.interactive_tx_constructor = Some(tx_constructor); + *self.interactive_tx_constructor = Some(tx_constructor); Ok(msg) } - pub fn tx_add_input(&mut self, msg: &msgs::TxAddInput) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult(match &mut self.interactive_tx_constructor { + pub(super) fn tx_add_input( + &mut self, msg: &msgs::TxAddInput, + ) -> InteractiveTxMessageSendResult { + InteractiveTxMessageSendResult(match self.interactive_tx_constructor { Some(ref mut tx_constructor) => tx_constructor .handle_tx_add_input(msg) .map_err(|reason| reason.into_tx_abort_msg(self.context.channel_id())), @@ -2858,8 +2896,10 @@ where }) } - pub fn tx_add_output(&mut self, msg: &msgs::TxAddOutput) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult(match &mut self.interactive_tx_constructor { + pub(super) fn tx_add_output( + &mut self, msg: &msgs::TxAddOutput, + ) -> InteractiveTxMessageSendResult { + InteractiveTxMessageSendResult(match self.interactive_tx_constructor { Some(ref mut tx_constructor) => tx_constructor .handle_tx_add_output(msg) .map_err(|reason| reason.into_tx_abort_msg(self.context.channel_id())), @@ -2870,8 +2910,10 @@ where }) } - pub fn tx_remove_input(&mut self, msg: &msgs::TxRemoveInput) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult(match &mut self.interactive_tx_constructor { + pub(super) fn tx_remove_input( + &mut self, msg: &msgs::TxRemoveInput, + ) -> InteractiveTxMessageSendResult { + InteractiveTxMessageSendResult(match self.interactive_tx_constructor { Some(ref mut tx_constructor) => tx_constructor .handle_tx_remove_input(msg) .map_err(|reason| reason.into_tx_abort_msg(self.context.channel_id())), @@ -2882,10 +2924,10 @@ where }) } - pub fn tx_remove_output( + pub(super) fn tx_remove_output( &mut self, msg: &msgs::TxRemoveOutput, ) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult(match &mut self.interactive_tx_constructor { + InteractiveTxMessageSendResult(match self.interactive_tx_constructor { Some(ref mut tx_constructor) => tx_constructor .handle_tx_remove_output(msg) .map_err(|reason| reason.into_tx_abort_msg(self.context.channel_id())), @@ -2896,9 +2938,9 @@ where }) } - pub fn tx_complete(&mut self, msg: &msgs::TxComplete) -> HandleTxCompleteResult { - let tx_constructor = match &mut self.interactive_tx_constructor { - Some(ref mut tx_constructor) => tx_constructor, + pub(super) fn tx_complete(&mut self, msg: &msgs::TxComplete) -> HandleTxCompleteResult { + let tx_constructor = match self.interactive_tx_constructor { + Some(tx_constructor) => tx_constructor, None => { let tx_abort = msgs::TxAbort { channel_id: msg.channel_id, @@ -2919,14 +2961,14 @@ where } #[rustfmt::skip] - pub fn funding_tx_constructed( + fn funding_tx_constructed( &mut self, mut signing_session: InteractiveTxSigningSession, logger: &L ) -> Result<(msgs::CommitmentSigned, Option), ChannelError> where L::Target: Logger { - let our_funding_satoshis = self.funding_negotiation_context.our_funding_satoshis; - let transaction_number = self.unfunded_context.transaction_number(); + let our_funding_satoshis = self.funding_negotiation_context + .our_funding_satoshis; let mut output_index = None; let expected_spk = self.funding.get_funding_redeemscript().to_p2wsh(); @@ -2947,9 +2989,10 @@ where let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; return Err(ChannelError::Close((msg.to_owned(), reason))); }; - self.funding.channel_transaction_parameters.funding_outpoint = Some(outpoint); + self.funding + .channel_transaction_parameters.funding_outpoint = Some(outpoint); - self.context.assert_no_commitment_advancement(transaction_number, "initial commitment_signed"); + self.context.assert_no_commitment_advancement(self.holder_commitment_transaction_number, "initial commitment_signed"); let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger); let commitment_signed = match commitment_signed { Ok(commitment_signed) => commitment_signed, @@ -3000,8 +3043,8 @@ where self.context.channel_state = channel_state; // Clear the interactive transaction constructor - self.interactive_tx_constructor.take(); - self.interactive_tx_signing_session = Some(signing_session); + *self.interactive_tx_constructor = None; + *self.interactive_tx_signing_session = Some(signing_session); Ok((commitment_signed, funding_ready_for_sig_event)) } @@ -5997,6 +6040,42 @@ where self.context.force_shutdown(&self.funding, closure_reason) } + /// If we are in splicing/refunding, return a short-lived [`NegotiatingChannelView`]. + #[cfg(splicing)] + fn as_renegotiating_channel(&mut self) -> Result, &'static str> { + if let Some(ref mut pending_splice) = &mut self.pending_splice { + if let Some(ref mut funding) = &mut pending_splice.funding { + if pending_splice.funding_negotiation_context.our_funding_satoshis != 0 + || pending_splice + .funding_negotiation_context + .their_funding_satoshis + .unwrap_or_default() != 0 + { + Ok(NegotiatingChannelView { + context: &mut self.context, + funding, + funding_negotiation_context: &mut pending_splice + .funding_negotiation_context, + interactive_tx_constructor: &mut pending_splice.interactive_tx_constructor, + interactive_tx_signing_session: &mut pending_splice + .interactive_tx_signing_session, + holder_commitment_transaction_number: self + .holder_commitment_point + .transaction_number(), + }) + } else { + Err("Received unexpected interactive transaction negotiation message: \ + the channel is splicing, but splice_init/splice_ack has not been exchanged yet") + } + } else { + Err("Received unexpected interactive transaction negotiation message: \ + the channel is splicing, but splice_init/splice_ack has not been exchanged yet") + } + } else { + Err("Received unexpected interactive transaction negotiation message: the channel is funded and not splicing") + } + } + #[rustfmt::skip] fn check_remote_fee( channel_type: &ChannelTypeFeatures, fee_estimator: &LowerBoundedFeeEstimator, @@ -10181,10 +10260,11 @@ where ) -> Result { // Check if a splice has been initiated already. // Note: only a single outstanding splice is supported (per spec) - if let Some(splice_info) = &self.pending_splice { + if let Some(pending_splice) = &self.pending_splice { return Err(APIError::APIMisuseError { err: format!( "Channel {} cannot be spliced, as it has already a splice pending (contribution {})", - self.context.channel_id(), splice_info.our_funding_contribution + self.context.channel_id(), + pending_splice.our_funding_contribution, )}); } @@ -10216,10 +10296,27 @@ where "Insufficient inputs for splicing; channel ID {}, err {}", self.context.channel_id(), err, )})?; + // Convert inputs + let mut funding_inputs = Vec::new(); + for (tx_in, tx, _w) in our_funding_inputs.into_iter() { + let tx16 = TransactionU16LenLimited::new(tx.clone()).map_err(|_e| APIError::APIMisuseError { err: format!("Too large transaction")})?; + funding_inputs.push((tx_in.clone(), tx16)); + } + let funding_negotiation_context = FundingNegotiationContext { + is_initiator: true, + our_funding_satoshis: 0, // set at later phase + their_funding_satoshis: None, // set at later phase + funding_tx_locktime: LockTime::from_consensus(locktime), + funding_feerate_sat_per_1000_weight: funding_feerate_per_kw, + our_funding_inputs: funding_inputs, + }; self.pending_splice = Some(PendingSplice { our_funding_contribution: our_funding_contribution_satoshis, funding: None, + funding_negotiation_context, + interactive_tx_constructor: None, + interactive_tx_signing_session: None, sent_funding_txid: None, received_funding_txid: None, }); @@ -12147,6 +12244,18 @@ where pub fn get_accept_channel_v2_message(&self) -> msgs::AcceptChannelV2 { self.generate_accept_channel_v2_message() } + + /// Return a short-lived [`NegotiatingChannelView`]. + fn as_negotiating_channel(&mut self) -> NegotiatingChannelView { + NegotiatingChannelView { + context: &mut self.context, + funding: &mut self.funding, + funding_negotiation_context: &mut self.funding_negotiation_context, + interactive_tx_constructor: &mut self.interactive_tx_constructor, + interactive_tx_signing_session: &mut self.interactive_tx_signing_session, + holder_commitment_transaction_number: self.unfunded_context.transaction_number(), + } + } } // Unfunded channel utilities diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index e4016e79535..1af7970bf3d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8885,7 +8885,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } #[rustfmt::skip] - fn internal_tx_msg) -> Result>( + fn internal_tx_msg) -> Result>( &self, counterparty_node_id: &PublicKey, channel_id: ChannelId, tx_msg_handler: HandleTxMsgFn ) -> Result<(), MsgHandleErrInternal> { let per_peer_state = self.per_peer_state.read().unwrap(); @@ -8903,9 +8903,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let channel = chan_entry.get_mut(); let msg_send_event = match tx_msg_handler(channel) { Ok(msg_send_event) => msg_send_event, - Err(tx_msg_str) => return Err(MsgHandleErrInternal::from_chan_no_close(ChannelError::Warn( - format!("Got a {tx_msg_str} message with no interactive transaction construction expected or in-progress") - ), channel_id)), + Err(err) => return Err(MsgHandleErrInternal::from_chan_no_close(err, channel_id)), }; peer_state.pending_msg_events.push(msg_send_event); Ok(()) @@ -8923,12 +8921,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self, counterparty_node_id: PublicKey, msg: &msgs::TxAddInput, ) -> Result<(), MsgHandleErrInternal> { self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel| { - match channel.as_unfunded_v2_mut() { - Some(unfunded_channel) => { - Ok(unfunded_channel.tx_add_input(msg).into_msg_send_event(counterparty_node_id)) - }, - None => Err("tx_add_input"), - } + Ok(channel + .as_negotiating_channel()? + .tx_add_input(msg) + .into_msg_send_event(counterparty_node_id)) }) } @@ -8936,15 +8932,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self, counterparty_node_id: PublicKey, msg: &msgs::TxAddOutput, ) -> Result<(), MsgHandleErrInternal> { self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel| { - match channel.as_unfunded_v2_mut() { - Some(unfunded_channel) => { - let msg_send_event = unfunded_channel - .tx_add_output(msg) - .into_msg_send_event(counterparty_node_id); - Ok(msg_send_event) - }, - None => Err("tx_add_output"), - } + Ok(channel + .as_negotiating_channel()? + .tx_add_output(msg) + .into_msg_send_event(counterparty_node_id)) }) } @@ -8952,15 +8943,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveInput, ) -> Result<(), MsgHandleErrInternal> { self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel| { - match channel.as_unfunded_v2_mut() { - Some(unfunded_channel) => { - let msg_send_event = unfunded_channel - .tx_remove_input(msg) - .into_msg_send_event(counterparty_node_id); - Ok(msg_send_event) - }, - None => Err("tx_remove_input"), - } + Ok(channel + .as_negotiating_channel()? + .tx_remove_input(msg) + .into_msg_send_event(counterparty_node_id)) }) } @@ -8968,15 +8954,10 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveOutput, ) -> Result<(), MsgHandleErrInternal> { self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel| { - match channel.as_unfunded_v2_mut() { - Some(unfunded_channel) => { - let msg_send_event = unfunded_channel - .tx_remove_output(msg) - .into_msg_send_event(counterparty_node_id); - Ok(msg_send_event) - }, - None => Err("tx_remove_output"), - } + Ok(channel + .as_negotiating_channel()? + .tx_remove_output(msg) + .into_msg_send_event(counterparty_node_id)) }) } @@ -8994,13 +8975,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let peer_state = &mut *peer_state_lock; match peer_state.channel_by_id.entry(msg.channel_id) { hash_map::Entry::Occupied(mut chan_entry) => { - let (msg_send_event_opt, signing_session_opt) = match chan_entry.get_mut().as_unfunded_v2_mut() { - Some(chan) => chan.tx_complete(msg) + let (msg_send_event_opt, signing_session_opt) = match chan_entry.get_mut().as_negotiating_channel() { + Ok(mut negotiating_channel) => negotiating_channel + .tx_complete(msg) .into_msg_send_event_or_signing_session(counterparty_node_id), - None => { - let msg = "Got a tx_complete message with no interactive transaction construction expected or in-progress"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - let err = ChannelError::Close((msg.to_owned(), reason)); + Err(err) => { try_channel_entry!(self, peer_state, Err(err), chan_entry) }, }; From 1572f3be5b46027de3f8285e4f632b4e45579bcd Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Tue, 17 Jun 2025 10:58:42 +0200 Subject: [PATCH 08/22] Implement transaction negotiation during splicing Fill the logic for including transaction negotiation during splicing, implement the functions: splice_channel, splice_init, splice_ack, funding_tx_constructed. Also extend the test case test_v1_splice_in with the steps for funding negotiation during splicing. --- lightning/src/ln/channel.rs | 538 ++++++++++++++++++++++++++--- lightning/src/ln/channelmanager.rs | 53 +-- lightning/src/ln/splicing_tests.rs | 131 ++++++- 3 files changed, 638 insertions(+), 84 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 8baa35ec75b..b89322632d1 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -25,6 +25,8 @@ use bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE; use bitcoin::secp256k1::{ecdsa::Signature, Secp256k1}; use bitcoin::secp256k1::{PublicKey, SecretKey}; use bitcoin::{secp256k1, sighash}; +#[cfg(splicing)] +use bitcoin::{Sequence, Witness}; use crate::chain::chaininterface::{ fee_for_weight, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator, @@ -1784,13 +1786,10 @@ where L::Target: Logger, { let logger = WithChannelContext::from(logger, self.context(), None); - if let Ok(mut negotiating_channel) = self.as_negotiating_channel() { - let (commitment_signed, event) = - negotiating_channel.funding_tx_constructed(signing_session, &&logger)?; - Ok((commitment_signed, event)) - } else { - Err(ChannelError::Warn("Got a tx_complete message with no interactive transaction construction expected or in-progress".to_owned())) - } + let mut negotiating_channel = self.as_negotiating_channel()?; + let (commitment_signed, event) = + negotiating_channel.funding_tx_constructed(signing_session, &&logger)?; + Ok((commitment_signed, event)) } pub fn force_shutdown(&mut self, closure_reason: ClosureReason) -> ShutdownResult { @@ -2185,9 +2184,83 @@ impl FundingScope { pub fn get_short_channel_id(&self) -> Option { self.short_channel_id } + + /// Construct FundingScope for a splicing channel + #[cfg(splicing)] + pub fn for_splice( + prev_funding: &Self, context: &ChannelContext, our_funding_contribution_sats: i64, + post_channel_value: u64, counterparty_funding_pubkey: PublicKey, + ) -> Result + where + SP::Target: SignerProvider, + { + let post_value_to_self_msat_signed = (prev_funding.value_to_self_msat as i64) + .saturating_add(our_funding_contribution_sats * 1000); + debug_assert!(post_value_to_self_msat_signed >= 0); + let post_value_to_self_msat = post_value_to_self_msat_signed as u64; + + let prev_funding_txid = prev_funding.get_funding_txid(); + // Update the splicing 'tweak', this will rotate the keys in the signer + let holder_pubkeys = match &context.holder_signer { + ChannelSignerType::Ecdsa(ecdsa) => ecdsa.pubkeys(prev_funding_txid, &context.secp_ctx), + // TODO (taproot|arik) + #[cfg(taproot)] + _ => todo!(), + }; + let channel_parameters = &prev_funding.channel_transaction_parameters; + let mut post_channel_transaction_parameters = ChannelTransactionParameters { + holder_pubkeys, + holder_selected_contest_delay: channel_parameters.holder_selected_contest_delay, + // The 'outbound' attribute doesn't change, even if the splice initiator is the other node + is_outbound_from_holder: channel_parameters.is_outbound_from_holder, + counterparty_parameters: channel_parameters.counterparty_parameters.clone(), + funding_outpoint: None, // filled later + splice_parent_funding_txid: prev_funding_txid, + channel_type_features: channel_parameters.channel_type_features.clone(), + channel_value_satoshis: post_channel_value, + }; + if let Some(ref mut counterparty_parameters) = + post_channel_transaction_parameters.counterparty_parameters + { + counterparty_parameters.pubkeys.funding_pubkey = counterparty_funding_pubkey; + } + + // New reserve values are based on the new channel value, and v2-specific + let counterparty_selected_channel_reserve_satoshis = Some(get_v2_channel_reserve_satoshis( + post_channel_value, + context.counterparty_dust_limit_satoshis, + )); + let holder_selected_channel_reserve_satoshis = + get_v2_channel_reserve_satoshis(post_channel_value, MIN_CHAN_DUST_LIMIT_SATOSHIS); + Ok(Self { + channel_transaction_parameters: post_channel_transaction_parameters, + value_to_self_msat: post_value_to_self_msat, + funding_transaction: None, + counterparty_selected_channel_reserve_satoshis, + holder_selected_channel_reserve_satoshis, + #[cfg(debug_assertions)] + holder_max_commitment_tx_output: Mutex::new(( + post_value_to_self_msat, + (post_channel_value * 1000).saturating_sub(post_value_to_self_msat), + )), + #[cfg(debug_assertions)] + counterparty_max_commitment_tx_output: Mutex::new(( + post_value_to_self_msat, + (post_channel_value * 1000).saturating_sub(post_value_to_self_msat), + )), + #[cfg(any(test, fuzzing))] + next_local_commitment_tx_fee_info_cached: Mutex::new(None), + #[cfg(any(test, fuzzing))] + next_remote_commitment_tx_fee_info_cached: Mutex::new(None), + funding_tx_confirmation_height: 0, + funding_tx_confirmed_in: None, + minimum_depth_override: None, + short_channel_id: None, + }) + } } -/// Info about a pending splice, used in the pre-splice channel +/// Info about a pending splice #[cfg(splicing)] struct PendingSplice { /// Intended contributions to the splice from our end @@ -2196,7 +2269,6 @@ struct PendingSplice { funding_negotiation_context: FundingNegotiationContext, /// The current interactive transaction construction session under negotiation. interactive_tx_constructor: Option, - interactive_tx_signing_session: Option, /// The funding txid used in the `splice_locked` sent to the counterparty. sent_funding_txid: Option, @@ -2263,6 +2335,28 @@ impl<'a> From<&'a Transaction> for ConfirmedTransaction<'a> { } } +#[cfg(splicing)] +impl PendingSplice { + #[inline] + fn add_checked(base: u64, delta: i64) -> u64 { + if delta >= 0 { + base.saturating_add(delta as u64) + } else { + base.saturating_sub(delta.abs() as u64) + } + } + + /// Compute the post-splice channel value from the pre-splice values and the peer contributions + pub fn compute_post_value( + pre_channel_value: u64, our_funding_contribution: i64, their_funding_contribution: i64, + ) -> u64 { + Self::add_checked( + pre_channel_value, + our_funding_contribution.saturating_add(their_funding_contribution), + ) + } +} + /// Contains everything about the channel including state, and various flags. pub(super) struct ChannelContext where @@ -2781,11 +2875,13 @@ where /// Can be produced by: /// - [`PendingV2Channel`], at V2 channel open, and /// - [`FundedChannel`], when splicing. -pub struct NegotiatingChannelView<'a, SP: Deref> +pub(super) struct NegotiatingChannelView<'a, SP: Deref> where SP::Target: SignerProvider, { context: &'a mut ChannelContext, + /// The funding scope being (re)negotiated. + /// In case of splicing it is the new spliced scope. funding: &'a mut FundingScope, funding_negotiation_context: &'a mut FundingNegotiationContext, interactive_tx_constructor: &'a mut Option, @@ -2797,6 +2893,10 @@ impl<'a, SP: Deref> NegotiatingChannelView<'a, SP> where SP::Target: SignerProvider, { + fn is_splice(&self) -> bool { + self.funding.channel_transaction_parameters.splice_parent_funding_txid.is_some() + } + /// Prepare and start interactive transaction negotiation. /// `change_destination_opt` - Optional destination for optional change; if None, /// default destination address is used. @@ -2809,7 +2909,15 @@ where where ES::Target: EntropySource, { - debug_assert!(matches!(self.context.channel_state, ChannelState::NegotiatingFunding(_))); + if self.is_splice() { + debug_assert!(matches!(self.context.channel_state, ChannelState::ChannelReady(_))); + } else { + debug_assert!(matches!( + self.context.channel_state, + ChannelState::NegotiatingFunding(_) + )); + } + debug_assert!(self.interactive_tx_constructor.is_none()); // Add output for funding tx @@ -2992,6 +3100,15 @@ where self.funding .channel_transaction_parameters.funding_outpoint = Some(outpoint); + if self.is_splice() { + let message = "TODO Forced error, incomplete implementation".to_owned(); + // TODO(splicing) Forced error, as the use case is not complete + return Err(ChannelError::Close(( + message.clone(), + ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false), message } + ))); + } + self.context.assert_no_commitment_advancement(self.holder_commitment_transaction_number, "initial commitment_signed"); let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger); let commitment_signed = match commitment_signed { @@ -6045,28 +6162,16 @@ where fn as_renegotiating_channel(&mut self) -> Result, &'static str> { if let Some(ref mut pending_splice) = &mut self.pending_splice { if let Some(ref mut funding) = &mut pending_splice.funding { - if pending_splice.funding_negotiation_context.our_funding_satoshis != 0 - || pending_splice - .funding_negotiation_context - .their_funding_satoshis - .unwrap_or_default() != 0 - { - Ok(NegotiatingChannelView { - context: &mut self.context, - funding, - funding_negotiation_context: &mut pending_splice - .funding_negotiation_context, - interactive_tx_constructor: &mut pending_splice.interactive_tx_constructor, - interactive_tx_signing_session: &mut pending_splice - .interactive_tx_signing_session, - holder_commitment_transaction_number: self - .holder_commitment_point - .transaction_number(), - }) - } else { - Err("Received unexpected interactive transaction negotiation message: \ - the channel is splicing, but splice_init/splice_ack has not been exchanged yet") - } + Ok(NegotiatingChannelView { + context: &mut self.context, + funding, + funding_negotiation_context: &mut pending_splice.funding_negotiation_context, + interactive_tx_constructor: &mut pending_splice.interactive_tx_constructor, + interactive_tx_signing_session: &mut self.interactive_tx_signing_session, + holder_commitment_transaction_number: self + .holder_commitment_point + .transaction_number(), + }) } else { Err("Received unexpected interactive transaction negotiation message: \ the channel is splicing, but splice_init/splice_ack has not been exchanged yet") @@ -10255,7 +10360,7 @@ where #[cfg(splicing)] #[rustfmt::skip] pub fn splice_channel(&mut self, our_funding_contribution_satoshis: i64, - our_funding_inputs: &Vec<(TxIn, Transaction, Weight)>, + our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, funding_feerate_per_kw: u32, locktime: u32, ) -> Result { // Check if a splice has been initiated already. @@ -10299,8 +10404,9 @@ where // Convert inputs let mut funding_inputs = Vec::new(); for (tx_in, tx, _w) in our_funding_inputs.into_iter() { - let tx16 = TransactionU16LenLimited::new(tx.clone()).map_err(|_e| APIError::APIMisuseError { err: format!("Too large transaction")})?; - funding_inputs.push((tx_in.clone(), tx16)); + let tx16 = TransactionU16LenLimited::new(tx) + .map_err(|_e| APIError::APIMisuseError { err: format!("Too large transaction") })?; + funding_inputs.push((tx_in, tx16)); } let funding_negotiation_context = FundingNegotiationContext { @@ -10316,7 +10422,6 @@ where funding: None, funding_negotiation_context, interactive_tx_constructor: None, - interactive_tx_signing_session: None, sent_funding_txid: None, received_funding_txid: None, }); @@ -10343,18 +10448,21 @@ where } } - /// Handle splice_init + /// Checks during handling splice_init #[cfg(splicing)] - #[rustfmt::skip] - pub fn splice_init(&mut self, msg: &msgs::SpliceInit) -> Result { + pub fn validate_splice_init(&self, msg: &msgs::SpliceInit) -> Result<(), ChannelError> { let their_funding_contribution_satoshis = msg.funding_contribution_satoshis; // TODO(splicing): Currently not possible to contribute on the splicing-acceptor side let our_funding_contribution_satoshis = 0i64; + // TODO(splicing): Add check that we are the quiescence acceptor + // Check if a splice has been initiated already. - if let Some(splice_info) = &self.pending_splice { + if let Some(pending_splice) = &self.pending_splice { return Err(ChannelError::Warn(format!( - "Channel has already a splice pending, contribution {}", splice_info.our_funding_contribution, + "Channel {} has already a splice pending, contribution {}", + self.context.channel_id(), + pending_splice.our_funding_contribution, ))); } @@ -10362,14 +10470,18 @@ where // MUST send a warning and close the connection or send an error // and fail the channel. if !self.context.is_live() { - return Err(ChannelError::Warn(format!("Splicing requested on a channel that is not live"))); + return Err(ChannelError::Warn(format!( + "Splicing requested on a channel that is not live" + ))); } - if their_funding_contribution_satoshis.saturating_add(our_funding_contribution_satoshis) < 0 { + if their_funding_contribution_satoshis.saturating_add(our_funding_contribution_satoshis) < 0 + { return Err(ChannelError::Warn(format!( "Splice-out not supported, only splice in, contribution is {} ({} + {})", their_funding_contribution_satoshis + our_funding_contribution_satoshis, - their_funding_contribution_satoshis, our_funding_contribution_satoshis, + their_funding_contribution_satoshis, + our_funding_contribution_satoshis, ))); } @@ -10378,11 +10490,114 @@ where // Note on channel reserve requirement pre-check: as the splice acceptor does not contribute, // it can't go below reserve, therefore no pre-check is done here. - // TODO(splicing): Once splice acceptor can contribute, add reserve pre-check, similar to the one in `splice_ack`. - // TODO(splicing): Store msg.funding_pubkey - // TODO(splicing): Apply start of splice (splice_start) + let pre_channel_value = self.funding.value_to_self_msat; + let _post_channel_value = PendingSplice::compute_post_value( + pre_channel_value, + their_funding_contribution_satoshis, + our_funding_contribution_satoshis, + ); + let _post_balance = PendingSplice::add_checked( + self.funding.value_to_self_msat, + our_funding_contribution_satoshis, + ); + // TODO: Early check for reserve requirement + + Ok(()) + } + + /// See also [`validate_splice_init`] + #[cfg(splicing)] + pub(crate) fn splice_init( + &mut self, msg: &msgs::SpliceInit, our_funding_contribution: i64, signer_provider: &SP, + entropy_source: &ES, holder_node_id: &PublicKey, logger: &L, + ) -> Result + where + ES::Target: EntropySource, + L::Target: Logger, + { + let _res = self.validate_splice_init(msg)?; + + let pre_channel_value = self.funding.get_value_satoshis(); + let their_funding_contribution = msg.funding_contribution_satoshis; + + let post_channel_value = PendingSplice::compute_post_value( + pre_channel_value, + our_funding_contribution, + their_funding_contribution, + ); + + let funding_scope = FundingScope::for_splice( + &self.funding, + &self.context, + our_funding_contribution, + post_channel_value, + msg.funding_pubkey, + )?; + + let (our_funding_satoshis, their_funding_satoshis) = calculate_total_funding_contribution( + pre_channel_value, + our_funding_contribution, + msg.funding_contribution_satoshis, + false, // is_outbound + )?; + + let funding_negotiation_context = FundingNegotiationContext { + is_initiator: false, + our_funding_satoshis, + their_funding_satoshis: Some(their_funding_satoshis), + funding_tx_locktime: LockTime::from_consensus(msg.locktime), + funding_feerate_sat_per_1000_weight: msg.funding_feerate_per_kw, + our_funding_inputs: Vec::new(), + }; + + self.pending_splice = Some(PendingSplice { + our_funding_contribution, + funding: Some(funding_scope), + funding_negotiation_context, + interactive_tx_constructor: None, + received_funding_txid: None, + sent_funding_txid: None, + }); + + log_info!(logger, "Splicing process started after splice_init, new channel value {}, old {}, outgoing {}, channel_id {}", + post_channel_value, pre_channel_value, false, self.context.channel_id); + + let splice_ack_msg = self.get_splice_ack(our_funding_contribution); + + // Build NegotiatingChannelView locally, similar to Channel::as_renegotiating_channel() + let pending_splice_mut = &mut self.pending_splice.as_mut().unwrap(); // set above + let mut negotiating_view = NegotiatingChannelView { + context: &mut self.context, + funding: &mut pending_splice_mut.funding.as_mut().unwrap(), // set above + funding_negotiation_context: &mut pending_splice_mut.funding_negotiation_context, + interactive_tx_constructor: &mut pending_splice_mut.interactive_tx_constructor, + interactive_tx_signing_session: &mut self.interactive_tx_signing_session, + holder_commitment_transaction_number: self.holder_commitment_point.transaction_number(), + }; + + // Start interactive funding negotiation. TODO(splicing): Add current funding as extra input, once shared inputs are supported, see #3842. + let _msg = negotiating_view + .begin_interactive_funding_tx_construction( + signer_provider, + entropy_source, + holder_node_id.clone(), + None, + None, + ) + .map_err(|err| { + ChannelError::Warn(format!( + "Failed to start interactive transaction construction, {:?}", + err + )) + })?; + + Ok(splice_ack_msg) + } + /// Get the splice_ack message that can be sent in response to splice initiation. + #[cfg(splicing)] + pub fn get_splice_ack(&self, our_funding_contribution_satoshis: i64) -> msgs::SpliceAck { // TODO(splicing): The exisiting pubkey is reused, but a new one should be generated. See #3542. // Note that channel_keys_id is supposed NOT to change let splice_ack_msg = msgs::SpliceAck { @@ -10392,20 +10607,134 @@ where require_confirmed_inputs: None, }; // TODO(splicing): start interactive funding negotiation - Ok(splice_ack_msg) + splice_ack_msg } /// Handle splice_ack #[cfg(splicing)] - pub fn splice_ack(&mut self, _msg: &msgs::SpliceAck) -> Result<(), ChannelError> { + pub(crate) fn splice_ack( + &mut self, msg: &msgs::SpliceAck, signer_provider: &SP, entropy_source: &ES, + holder_node_id: &PublicKey, logger: &L, + ) -> Result, ChannelError> + where + ES::Target: EntropySource, + L::Target: Logger, + { // check if splice is pending - if self.pending_splice.is_none() { + let pending_splice = if let Some(ref mut pending_splice) = &mut self.pending_splice { + pending_splice + } else { return Err(ChannelError::Warn(format!("Channel is not in pending splice"))); }; + // TODO(splicing): Add check that we are the splice (quiescence) initiator + + if pending_splice.funding.is_some() || pending_splice.interactive_tx_constructor.is_some() { + return Err(ChannelError::Warn(format!( + "Got unexpected splice_ack, splice already negotiating" + ))); + } + + let our_funding_contribution = pending_splice.our_funding_contribution; + let their_funding_contribution_satoshis = msg.funding_contribution_satoshis; + // TODO(splicing): Pre-check for reserve requirement // (Note: It should also be checked later at tx_complete) - Ok(()) + let pre_channel_value = self.funding.get_value_satoshis(); + let post_channel_value = PendingSplice::compute_post_value( + pre_channel_value, + our_funding_contribution, + their_funding_contribution_satoshis, + ); + let _post_balance = + PendingSplice::add_checked(self.funding.value_to_self_msat, our_funding_contribution); + + // TODO(splicing): Pre-check for reserve requirement + // (Note: It should also be checked later at tx_complete) + + let funding_scope = FundingScope::for_splice( + &self.funding, + &self.context, + our_funding_contribution, + post_channel_value, + msg.funding_pubkey, + )?; + + let (our_funding_satoshis, their_funding_satoshis) = calculate_total_funding_contribution( + pre_channel_value, + our_funding_contribution, + their_funding_contribution_satoshis, + true, // is_outbound + )?; + + let pre_funding_transaction = &self.funding.funding_transaction; + let pre_funding_txo = &self.funding.get_funding_txo(); + // We need the current funding tx as an extra input + let prev_funding_input = + Self::get_input_of_previous_funding(pre_funding_transaction, pre_funding_txo)?; + debug_assert!(pending_splice.funding.is_none()); + pending_splice.funding = Some(funding_scope); + // update funding values + pending_splice.funding_negotiation_context.our_funding_satoshis = our_funding_satoshis; + pending_splice.funding_negotiation_context.their_funding_satoshis = + Some(their_funding_satoshis); + debug_assert!(pending_splice.interactive_tx_constructor.is_none()); + debug_assert!(self.interactive_tx_signing_session.is_none()); + + log_info!(logger, "Splicing process started after splice_ack, new channel value {}, old {}, outgoing {}, channel_id {}", + post_channel_value, pre_channel_value, true, self.context.channel_id); + + // Build NegotiatingChannelView locally, similar to Channel::as_renegotiating_channel() + let mut negotiating_view = NegotiatingChannelView { + context: &mut self.context, + funding: &mut pending_splice.funding.as_mut().unwrap(), // set above + funding_negotiation_context: &mut pending_splice.funding_negotiation_context, + interactive_tx_constructor: &mut pending_splice.interactive_tx_constructor, + interactive_tx_signing_session: &mut self.interactive_tx_signing_session, + holder_commitment_transaction_number: self.holder_commitment_point.transaction_number(), + }; + + // Start interactive funding negotiation, with the previous funding transaction as an extra shared input + let tx_msg_opt = negotiating_view + .begin_interactive_funding_tx_construction( + signer_provider, + entropy_source, + holder_node_id.clone(), + None, + Some(prev_funding_input), + ) + .map_err(|err| { + ChannelError::Warn(format!("V2 channel rejected due to sender error, {:?}", err)) + })?; + Ok(tx_msg_opt) + } + + /// Get a transaction input that is the previous funding transaction + #[cfg(splicing)] + fn get_input_of_previous_funding( + pre_funding_transaction: &Option, pre_funding_txo: &Option, + ) -> Result<(TxIn, TransactionU16LenLimited), ChannelError> { + if let Some(pre_funding_transaction) = pre_funding_transaction { + if let Some(pre_funding_txo) = pre_funding_txo { + Ok(( + TxIn { + previous_output: pre_funding_txo.into_bitcoin_outpoint(), + script_sig: ScriptBuf::new(), + sequence: Sequence::ZERO, + witness: Witness::new(), + }, + TransactionU16LenLimited::new(pre_funding_transaction.clone()).unwrap(), // TODO err? + )) + } else { + Err(ChannelError::Warn( + "Internal error: Missing previous funding transaction outpoint".to_string(), + )) + } + } else { + Err(ChannelError::Warn( + "Internal error: Missing previous funding transaction".to_string(), + )) + } } #[cfg(splicing)] @@ -11892,6 +12221,39 @@ where } } +/// Calculate total funding contributions, needed for interactive tx for splicing, +/// based on the current channel value and the splice contributions. +/// The inputs are the explicit contributions of the peers into the splice transaction, +/// this method adds the previous funding transaction as well, as it will be added +/// automatically by the splice initiator. +/// Example: old channel value is 100k, peer A wants to add +50k, B nothing, +/// and A is the splice initiator: A will have to add inputs for 150k, B nothing. +/// The results cannot be negative. +#[cfg(splicing)] +fn calculate_total_funding_contribution( + pre_channel_value: u64, our_splice_contribution: i64, their_splice_contribution: i64, + is_initiator: bool, +) -> Result<(u64, u64), ChannelError> { + // Initiator also adds the previous funding as input + let mut our_contribution_with_prev = our_splice_contribution; + let mut their_contribution_with_prev = their_splice_contribution; + if is_initiator { + our_contribution_with_prev = + our_contribution_with_prev.saturating_add(pre_channel_value as i64); + } else { + their_contribution_with_prev = + their_contribution_with_prev.saturating_add(pre_channel_value as i64); + } + if our_contribution_with_prev < 0 || their_contribution_with_prev < 0 { + return Err(ChannelError::Warn(format!( + "Funding contribution cannot be negative! ours {} theirs {} pre {} initiator {} acceptor {}", + our_contribution_with_prev, their_contribution_with_prev, pre_channel_value, + our_splice_contribution, their_splice_contribution + ))); + } + Ok((our_contribution_with_prev.abs() as u64, their_contribution_with_prev.abs() as u64)) +} + // A not-yet-funded channel using V2 channel establishment. pub(super) struct PendingV2Channel where @@ -15215,4 +15577,76 @@ mod tests { ); } } + + #[cfg(splicing)] + fn get_pre_and_post( + pre_channel_value: u64, our_funding_contribution: i64, their_funding_contribution: i64, + ) -> (u64, u64) { + use crate::ln::channel::PendingSplice; + + let post_channel_value = PendingSplice::compute_post_value( + pre_channel_value, + our_funding_contribution, + their_funding_contribution, + ); + (pre_channel_value, post_channel_value) + } + + #[cfg(splicing)] + #[test] + fn test_splice_compute_post_value() { + { + // increase, small amounts + let (pre_channel_value, post_channel_value) = get_pre_and_post(9_000, 6_000, 0); + assert_eq!(pre_channel_value, 9_000); + assert_eq!(post_channel_value, 15_000); + } + { + // increase, small amounts + let (pre_channel_value, post_channel_value) = get_pre_and_post(9_000, 4_000, 2_000); + assert_eq!(pre_channel_value, 9_000); + assert_eq!(post_channel_value, 15_000); + } + { + // increase, small amounts + let (pre_channel_value, post_channel_value) = get_pre_and_post(9_000, 0, 6_000); + assert_eq!(pre_channel_value, 9_000); + assert_eq!(post_channel_value, 15_000); + } + { + // decrease, small amounts + let (pre_channel_value, post_channel_value) = get_pre_and_post(15_000, -6_000, 0); + assert_eq!(pre_channel_value, 15_000); + assert_eq!(post_channel_value, 9_000); + } + { + // decrease, small amounts + let (pre_channel_value, post_channel_value) = get_pre_and_post(15_000, -4_000, -2_000); + assert_eq!(pre_channel_value, 15_000); + assert_eq!(post_channel_value, 9_000); + } + { + // increase and decrease + let (pre_channel_value, post_channel_value) = get_pre_and_post(15_000, 4_000, -2_000); + assert_eq!(pre_channel_value, 15_000); + assert_eq!(post_channel_value, 17_000); + } + let base2: u64 = 2; + let huge63i3 = (base2.pow(63) - 3) as i64; + assert_eq!(huge63i3, 9223372036854775805); + assert_eq!(-huge63i3, -9223372036854775805); + { + // increase, large amount + let (pre_channel_value, post_channel_value) = get_pre_and_post(9_000, huge63i3, 3); + assert_eq!(pre_channel_value, 9_000); + assert_eq!(post_channel_value, 9223372036854784807); + } + { + // increase, large amounts + let (pre_channel_value, post_channel_value) = + get_pre_and_post(9_000, huge63i3, huge63i3); + assert_eq!(pre_channel_value, 9_000); + assert_eq!(post_channel_value, 9223372036854784807); + } + } } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 1af7970bf3d..035cd849d1d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4494,7 +4494,7 @@ where let mut res = Ok(()); PersistenceNotifierGuard::optionally_notify(self, || { let result = self.internal_splice_channel( - channel_id, counterparty_node_id, our_funding_contribution_satoshis, &our_funding_inputs, funding_feerate_per_kw, locktime + channel_id, counterparty_node_id, our_funding_contribution_satoshis, our_funding_inputs, funding_feerate_per_kw, locktime ); res = result; match res { @@ -4510,7 +4510,7 @@ where #[rustfmt::skip] fn internal_splice_channel( &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, our_funding_contribution_satoshis: i64, - our_funding_inputs: &Vec<(TxIn, Transaction, Weight)>, + our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, funding_feerate_per_kw: u32, locktime: Option, ) -> Result<(), APIError> { let per_peer_state = self.per_peer_state.read().unwrap(); @@ -10095,6 +10095,9 @@ 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(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!( @@ -10102,24 +10105,22 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ counterparty_node_id, msg.channel_id, ), msg.channel_id)), hash_map::Entry::Occupied(mut chan_entry) => { - if let Some(chan) = chan_entry.get_mut().as_funded_mut() { - let splice_ack_msg = try_channel_entry!(self, peer_state, chan.splice_init(msg), chan_entry); + 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.signer_provider, &self.entropy_source, + &self.get_our_node_id(), &self.logger + ); + let splice_ack_msg = try_channel_entry!(self, peer_state, init_res, chan_entry); peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceAck { node_id: *counterparty_node_id, msg: splice_ack_msg, }); + Ok(()) } else { - return Err(MsgHandleErrInternal::send_err_msg_no_close("Channel is not funded, cannot be spliced".to_owned(), msg.channel_id)); + try_channel_entry!(self, peer_state, Err(ChannelError::close("Channel is not funded, cannot be spliced".into())), chan_entry) } }, - }; - - // TODO(splicing): - // Change channel, change phase (remove and add) - // Create new post-splice channel - // etc. - - Ok(()) + } } /// Handle incoming splice request ack, transition channel to splice-pending (unless some check fails). @@ -10137,26 +10138,26 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ // Look for the channel match peer_state.channel_by_id.entry(msg.channel_id) { - hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!( + hash_map::Entry::Vacant(_) => Err(MsgHandleErrInternal::send_err_msg_no_close(format!( "Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id ), msg.channel_id)), hash_map::Entry::Occupied(mut chan_entry) => { - if let Some(chan) = chan_entry.get_mut().as_funded_mut() { - try_channel_entry!(self, peer_state, chan.splice_ack(msg), chan_entry); + 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 + ); + let tx_msg_opt = try_channel_entry!(self, peer_state, splice_ack_res, chan_entry); + if let Some(tx_msg) = tx_msg_opt { + peer_state.pending_msg_events.push(tx_msg.into_msg_send_event(counterparty_node_id.clone())); + } + Ok(()) } else { - return Err(MsgHandleErrInternal::send_err_msg_no_close("Channel is not funded, cannot splice".to_owned(), msg.channel_id)); + try_channel_entry!(self, peer_state, Err(ChannelError::close("Channel is not funded, cannot be spliced".into())), chan_entry) } }, - }; - - // TODO(splicing): - // Change channel, change phase (remove and add) - // Create new post-splice channel - // Start splice funding transaction negotiation - // etc. - - Err(MsgHandleErrInternal::send_err_msg_no_close("TODO(splicing): Splicing is not implemented (splice_ack)".to_owned(), msg.channel_id)) + } } #[cfg(splicing)] diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 33f5a500789..602f24471f5 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -48,10 +48,15 @@ fn test_v1_splice_in() { // ==== Channel is now ready for normal operation + // Expected balances + let mut exp_balance1 = 1000 * channel_value_sat; + let mut _exp_balance2 = 0; + // === Start of Splicing // Amount being added to the channel through the splice-in let splice_in_sats = 20_000; + let post_splice_channel_value = channel_value_sat + splice_in_sats; let funding_feerate_per_kw = 1024; // Create additional inputs @@ -121,17 +126,131 @@ fn test_v1_splice_in() { assert!(channel.is_usable); assert!(channel.is_channel_ready); assert_eq!(channel.channel_value_satoshis, channel_value_sat); - assert_eq!( - channel.outbound_capacity_msat, - 1000 * (channel_value_sat - channel_reserve_amnt_sat) - ); + assert_eq!(channel.outbound_capacity_msat, exp_balance1 - 1000 * channel_reserve_amnt_sat); assert!(channel.funding_txo.is_some()); assert!(channel.confirmations.unwrap() > 0); } - let _error_msg = get_err_msg(initiator_node, &acceptor_node.node.get_our_node_id()); + // exp_balance1 += 1000 * splice_in_sats; // increase in balance + + // Negotiate transaction inputs and outputs + + // First input + let tx_add_input_msg = get_event_msg!( + &initiator_node, + MessageSendEvent::SendTxAddInput, + acceptor_node.node.get_our_node_id() + ); + dbg!(&tx_add_input_msg); + // check which input is this + let inputs_seen_in_reverse = if let Some(prevtx) = tx_add_input_msg.prevtx.as_ref() { + let value = prevtx + .as_transaction() + .output[tx_add_input_msg.prevtx_out as usize] + .value + .to_sat(); + value == extra_splice_funding_input_sats + } else { + false + }; + + let _res = acceptor_node + .node + .handle_tx_add_input(initiator_node.node.get_our_node_id(), &tx_add_input_msg); + let tx_complete_msg = get_event_msg!( + acceptor_node, + MessageSendEvent::SendTxComplete, + initiator_node.node.get_our_node_id() + ); + + let _res = initiator_node + .node + .handle_tx_complete(acceptor_node.node.get_our_node_id(), &tx_complete_msg); + // Second input + let exp_value = + if inputs_seen_in_reverse { channel_value_sat } else { extra_splice_funding_input_sats }; + let tx_add_input2_msg = get_event_msg!( + &initiator_node, + MessageSendEvent::SendTxAddInput, + acceptor_node.node.get_our_node_id() + ); + dbg!(&tx_add_input2_msg); + // FIXME: Determine why these fail + //assert_eq!(tx_add_input2_msg.prevtx, None); + //assert_eq!(tx_add_input2_msg.shared_input_txid.unwrap().to_string(), "4f128bedf1a15baf465ab1bfd6e97c8f82628f4156bf86eb1cbc132cda6733ae"); + + let _res = acceptor_node + .node + .handle_tx_add_input(initiator_node.node.get_our_node_id(), &tx_add_input2_msg); + let tx_complete_msg = get_event_msg!( + acceptor_node, + MessageSendEvent::SendTxComplete, + initiator_node.node.get_our_node_id() + ); + + let _res = initiator_node + .node + .handle_tx_complete(acceptor_node.node.get_our_node_id(), &tx_complete_msg); + + // TxAddOutput for the change output + let tx_add_output_msg = get_event_msg!( + &initiator_node, + MessageSendEvent::SendTxAddOutput, + acceptor_node.node.get_our_node_id() + ); + dbg!(&tx_add_output_msg); + // FIXME: Determine why these fail + //assert!(tx_add_output_msg.script.is_p2wsh()); + //assert_eq!(tx_add_output_msg.sats, post_splice_channel_value); + + let _res = acceptor_node + .node + .handle_tx_add_output(initiator_node.node.get_our_node_id(), &tx_add_output_msg); + let tx_complete_msg = get_event_msg!( + &acceptor_node, + MessageSendEvent::SendTxComplete, + initiator_node.node.get_our_node_id() + ); + + let _res = initiator_node + .node + .handle_tx_complete(acceptor_node.node.get_our_node_id(), &tx_complete_msg); + // TxAddOutput for the splice funding + let tx_add_output2_msg = get_event_msg!( + &initiator_node, + MessageSendEvent::SendTxAddOutput, + acceptor_node.node.get_our_node_id() + ); + dbg!(&tx_add_output2_msg); + // FIXME: Determine why these fail + //assert!(tx_add_output2_msg.script.is_p2wpkh()); + //assert_eq!(tx_add_output2_msg.sats, 14094); // extra_splice_input_sats - splice_in_sats + + let _res = acceptor_node + .node + .handle_tx_add_output(initiator_node.node.get_our_node_id(), &tx_add_output2_msg); + let _tx_complete_msg = get_event_msg!( + acceptor_node, + MessageSendEvent::SendTxComplete, + initiator_node.node.get_our_node_id() + ); + + // TODO(splicing) This is the last tx_complete, which triggers the commitment flow, which is not yet fully implemented + let _res = initiator_node + .node + .handle_tx_complete(acceptor_node.node.get_our_node_id(), &tx_complete_msg); + let events = initiator_node.node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 2); + match events[0] { + MessageSendEvent::SendTxComplete { .. } => {}, + _ => panic!("Unexpected event {:?}", events[0]), + } + match events[1] { + MessageSendEvent::HandleError { .. } => {}, + _ => panic!("Unexpected event {:?}", events[1]), + } - // TODO(splicing): continue with splice transaction negotiation + // TODO(splicing): Continue with commitment flow, new tx confirmation // === Close channel, cooperatively initiator_node.node.close_channel(&channel_id, &acceptor_node.node.get_our_node_id()).unwrap(); From 271c5a3bb49ef9e3f48934fe778c8f8c77195e65 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 16 Jul 2025 10:43:41 -0500 Subject: [PATCH 09/22] f - include shared_funding_input --- lightning/src/ln/channel.rs | 61 ++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 35 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index b89322632d1..be7011d1982 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2258,6 +2258,27 @@ impl FundingScope { short_channel_id: None, }) } + + /// Returns a `SharedOwnedInput` for using this `FundingScope` as the input to a new splice. + #[cfg(splicing)] + fn to_splice_funding_input(&self) -> SharedOwnedInput { + let funding_txo = self.get_funding_txo().expect("funding_txo should be set"); + let input = TxIn { + previous_output: funding_txo.into_bitcoin_outpoint(), + script_sig: ScriptBuf::new(), + sequence: Sequence::ZERO, + witness: Witness::new(), + }; + + let prev_output = TxOut { + value: Amount::from_sat(self.get_value_satoshis()), + script_pubkey: self.get_funding_redeemscript().to_p2wsh(), + }; + + let local_owned = self.value_to_self_msat / 1000; + + SharedOwnedInput::new(input, prev_output, local_owned) + } } /// Info about a pending splice @@ -10542,6 +10563,8 @@ where false, // is_outbound )?; + let prev_funding_input = self.funding.to_splice_funding_input(); + let funding_negotiation_context = FundingNegotiationContext { is_initiator: false, our_funding_satoshis, @@ -10576,14 +10599,13 @@ where holder_commitment_transaction_number: self.holder_commitment_point.transaction_number(), }; - // Start interactive funding negotiation. TODO(splicing): Add current funding as extra input, once shared inputs are supported, see #3842. let _msg = negotiating_view .begin_interactive_funding_tx_construction( signer_provider, entropy_source, holder_node_id.clone(), None, - None, + Some(prev_funding_input), ) .map_err(|err| { ChannelError::Warn(format!( @@ -10667,11 +10689,8 @@ where true, // is_outbound )?; - let pre_funding_transaction = &self.funding.funding_transaction; - let pre_funding_txo = &self.funding.get_funding_txo(); - // We need the current funding tx as an extra input - let prev_funding_input = - Self::get_input_of_previous_funding(pre_funding_transaction, pre_funding_txo)?; + let prev_funding_input = self.funding.to_splice_funding_input(); + debug_assert!(pending_splice.funding.is_none()); pending_splice.funding = Some(funding_scope); // update funding values @@ -10709,34 +10728,6 @@ where Ok(tx_msg_opt) } - /// Get a transaction input that is the previous funding transaction - #[cfg(splicing)] - fn get_input_of_previous_funding( - pre_funding_transaction: &Option, pre_funding_txo: &Option, - ) -> Result<(TxIn, TransactionU16LenLimited), ChannelError> { - if let Some(pre_funding_transaction) = pre_funding_transaction { - if let Some(pre_funding_txo) = pre_funding_txo { - Ok(( - TxIn { - previous_output: pre_funding_txo.into_bitcoin_outpoint(), - script_sig: ScriptBuf::new(), - sequence: Sequence::ZERO, - witness: Witness::new(), - }, - TransactionU16LenLimited::new(pre_funding_transaction.clone()).unwrap(), // TODO err? - )) - } else { - Err(ChannelError::Warn( - "Internal error: Missing previous funding transaction outpoint".to_string(), - )) - } - } else { - Err(ChannelError::Warn( - "Internal error: Missing previous funding transaction".to_string(), - )) - } - } - #[cfg(splicing)] pub fn splice_locked( &mut self, msg: &msgs::SpliceLocked, node_signer: &NS, chain_hash: ChainHash, From a21b1a6b11d279189d032595c835c72c0d605d4d Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 16 Jul 2025 13:51:30 -0500 Subject: [PATCH 10/22] f - simplify some code --- lightning/src/ln/channel.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index be7011d1982..f92a2fb7b81 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1739,7 +1739,7 @@ where ChannelPhase::UnfundedV2(chan) => Ok(chan.as_negotiating_channel()), #[cfg(splicing)] ChannelPhase::Funded(chan) => { - Ok(chan.as_renegotiating_channel().map_err(|err| ChannelError::Warn(err.into()))?) + chan.as_renegotiating_channel().map_err(|err| ChannelError::Warn(err.into())) }, _ => Err(ChannelError::Warn( "Got a transaction negotiation message in an invalid phase".to_owned(), @@ -1787,9 +1787,7 @@ where { let logger = WithChannelContext::from(logger, self.context(), None); let mut negotiating_channel = self.as_negotiating_channel()?; - let (commitment_signed, event) = - negotiating_channel.funding_tx_constructed(signing_session, &&logger)?; - Ok((commitment_signed, event)) + negotiating_channel.funding_tx_constructed(signing_session, &&logger) } pub fn force_shutdown(&mut self, closure_reason: ClosureReason) -> ShutdownResult { @@ -2219,11 +2217,12 @@ impl FundingScope { channel_type_features: channel_parameters.channel_type_features.clone(), channel_value_satoshis: post_channel_value, }; - if let Some(ref mut counterparty_parameters) = - post_channel_transaction_parameters.counterparty_parameters - { - counterparty_parameters.pubkeys.funding_pubkey = counterparty_funding_pubkey; - } + post_channel_transaction_parameters + .counterparty_parameters + .as_mut() + .expect("counterparty_parameters should be set") + .pubkeys + .funding_pubkey = counterparty_funding_pubkey; // New reserve values are based on the new channel value, and v2-specific let counterparty_selected_channel_reserve_satoshis = Some(get_v2_channel_reserve_satoshis( From 91b5843b9b1dfa0b31f18bbbb38468a2565e1cfa Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 18 Jul 2025 11:11:45 -0500 Subject: [PATCH 11/22] f - account for shared input when calculating change --- lightning/src/ln/channel.rs | 2 +- lightning/src/ln/interactivetxs.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f92a2fb7b81..0aba67a362d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2960,7 +2960,7 @@ where }; let change_value_opt = calculate_change_output_value( &self.funding_negotiation_context, - None, + shared_funding_input.as_ref().map(|input| input.local_owned()), &shared_funding_output.script_pubkey, &funding_outputs, change_script.minimal_non_dust().to_sat(), diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 3633c329bf1..82bb122736e 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -1270,6 +1270,10 @@ impl SharedOwnedInput { Self { input, prev_output, local_owned } } + pub fn local_owned(&self) -> u64 { + self.local_owned + } + fn remote_owned(&self) -> u64 { self.prev_output.value.to_sat().saturating_sub(self.local_owned) } From 06e7a94d51b0afebea3074dd0ed93b41a95ba615 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Mon, 7 Jul 2025 11:08:46 +0200 Subject: [PATCH 12/22] Formatting, remove rustfmt skips --- lightning/src/ln/channel.rs | 55 +++++++++++++++++-------- lightning/src/ln/channelmanager.rs | 66 ++++++++++++++++++------------ 2 files changed, 77 insertions(+), 44 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 0aba67a362d..4cb620e9087 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -10378,35 +10378,41 @@ where /// - `our_funding_inputs`: the inputs we contribute to the new funding transaction. /// Includes the witness weight for this input (e.g. P2WPKH_WITNESS_WEIGHT=109 for typical P2WPKH inputs). #[cfg(splicing)] - #[rustfmt::skip] - pub fn splice_channel(&mut self, our_funding_contribution_satoshis: i64, - our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, - funding_feerate_per_kw: u32, locktime: u32, + pub fn splice_channel( + &mut self, our_funding_contribution_satoshis: i64, + our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, funding_feerate_per_kw: u32, + locktime: u32, ) -> Result { // Check if a splice has been initiated already. // Note: only a single outstanding splice is supported (per spec) if let Some(pending_splice) = &self.pending_splice { - return Err(APIError::APIMisuseError { err: format!( + return Err(APIError::APIMisuseError { + err: format!( "Channel {} cannot be spliced, as it has already a splice pending (contribution {})", self.context.channel_id(), pending_splice.our_funding_contribution, - )}); + ), + }); } if !self.context.is_live() { - return Err(APIError::APIMisuseError { err: format!( - "Channel {} cannot be spliced, as channel is not live", - self.context.channel_id() - )}); + return Err(APIError::APIMisuseError { + err: format!( + "Channel {} cannot be spliced, as channel is not live", + self.context.channel_id() + ), + }); } // TODO(splicing): check for quiescence if our_funding_contribution_satoshis < 0 { - return Err(APIError::APIMisuseError { err: format!( + return Err(APIError::APIMisuseError { + err: format!( "TODO(splicing): Splice-out not supported, only splice in; channel ID {}, contribution {}", self.context.channel_id(), our_funding_contribution_satoshis, - )}); + ), + }); } // TODO(splicing): Once splice-out is supported, check that channel balance does not go below 0 @@ -10416,11 +10422,20 @@ where // (Cannot test for miminum required post-splice channel value) // Check that inputs are sufficient to cover our contribution. - let _fee = check_v2_funding_inputs_sufficient(our_funding_contribution_satoshis, &our_funding_inputs, true, true, funding_feerate_per_kw) - .map_err(|err| APIError::APIMisuseError { err: format!( + let _fee = check_v2_funding_inputs_sufficient( + our_funding_contribution_satoshis, + &our_funding_inputs, + true, + true, + funding_feerate_per_kw, + ) + .map_err(|err| APIError::APIMisuseError { + err: format!( "Insufficient inputs for splicing; channel ID {}, err {}", - self.context.channel_id(), err, - )})?; + self.context.channel_id(), + err, + ), + })?; // Convert inputs let mut funding_inputs = Vec::new(); for (tx_in, tx, _w) in our_funding_inputs.into_iter() { @@ -10431,7 +10446,7 @@ where let funding_negotiation_context = FundingNegotiationContext { is_initiator: true, - our_funding_satoshis: 0, // set at later phase + our_funding_satoshis: 0, // set at later phase their_funding_satoshis: None, // set at later phase funding_tx_locktime: LockTime::from_consensus(locktime), funding_feerate_sat_per_1000_weight: funding_feerate_per_kw, @@ -10446,7 +10461,11 @@ where received_funding_txid: None, }); - let msg = self.get_splice_init(our_funding_contribution_satoshis, funding_feerate_per_kw, locktime); + let msg = self.get_splice_init( + our_funding_contribution_satoshis, + funding_feerate_per_kw, + locktime, + ); Ok(msg) } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 035cd849d1d..6359a78b107 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4507,16 +4507,22 @@ where /// See [`splice_channel`] #[cfg(splicing)] - #[rustfmt::skip] fn internal_splice_channel( - &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, our_funding_contribution_satoshis: i64, - our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, - funding_feerate_per_kw: u32, locktime: Option, + &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, + our_funding_contribution_satoshis: i64, + our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, funding_feerate_per_kw: u32, + locktime: Option, ) -> Result<(), APIError> { let per_peer_state = self.per_peer_state.read().unwrap(); - let peer_state_mutex = match per_peer_state.get(counterparty_node_id) - .ok_or_else(|| APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) }) { + let peer_state_mutex = match per_peer_state.get(counterparty_node_id).ok_or_else(|| { + APIError::ChannelUnavailable { + err: format!( + "Can't find a peer matching the passed counterparty node_id {}", + counterparty_node_id + ), + } + }) { Ok(p) => p, Err(e) => return Err(e), }; @@ -4529,7 +4535,12 @@ where hash_map::Entry::Occupied(mut chan_phase_entry) => { let locktime = locktime.unwrap_or_else(|| self.current_best_block().height); if let Some(chan) = chan_phase_entry.get_mut().as_funded_mut() { - let msg = chan.splice_channel(our_funding_contribution_satoshis, our_funding_inputs, funding_feerate_per_kw, locktime)?; + let msg = chan.splice_channel( + our_funding_contribution_satoshis, + our_funding_inputs, + funding_feerate_per_kw, + locktime, + )?; peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceInit { node_id: *counterparty_node_id, msg, @@ -4540,18 +4551,16 @@ where err: format!( "Channel with id {} is not funded, cannot splice it", channel_id - ) + ), }) } }, - hash_map::Entry::Vacant(_) => { - Err(APIError::ChannelUnavailable { - err: format!( - "Channel with id {} not found for the passed counterparty node_id {}", - channel_id, counterparty_node_id, - ) - }) - }, + hash_map::Entry::Vacant(_) => Err(APIError::ChannelUnavailable { + err: format!( + "Channel with id {} not found for the passed counterparty node_id {}", + channel_id, counterparty_node_id, + ), + }), } } @@ -8884,18 +8893,23 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } - #[rustfmt::skip] - fn internal_tx_msg) -> Result>( - &self, counterparty_node_id: &PublicKey, channel_id: ChannelId, tx_msg_handler: HandleTxMsgFn + fn internal_tx_msg< + HandleTxMsgFn: Fn(&mut Channel) -> Result, + >( + &self, counterparty_node_id: &PublicKey, channel_id: ChannelId, + tx_msg_handler: HandleTxMsgFn, ) -> Result<(), MsgHandleErrInternal> { let per_peer_state = self.per_peer_state.read().unwrap(); - let peer_state_mutex = per_peer_state.get(counterparty_node_id) - .ok_or_else(|| { - debug_assert!(false); - MsgHandleErrInternal::send_err_msg_no_close( - format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), - channel_id) - })?; + let peer_state_mutex = per_peer_state.get(counterparty_node_id).ok_or_else(|| { + debug_assert!(false); + MsgHandleErrInternal::send_err_msg_no_close( + format!( + "Can't find a peer matching the passed counterparty node_id {}", + counterparty_node_id + ), + channel_id, + ) + })?; let mut peer_state_lock = peer_state_mutex.lock().unwrap(); let peer_state = &mut *peer_state_lock; match peer_state.channel_by_id.entry(channel_id) { From ab1d4b5e80156d070b49a4c66f085c5b0e0b1b8f Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 16 Jul 2025 16:12:18 -0700 Subject: [PATCH 13/22] NegotiatingChannelView cleanup --- lightning/src/ln/channel.rs | 631 ++++++++++++++--------------- lightning/src/ln/channelmanager.rs | 69 ++-- lightning/src/ln/interactivetxs.rs | 98 +++-- 3 files changed, 390 insertions(+), 408 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 4cb620e9087..d805f22f503 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1734,16 +1734,12 @@ where } } - pub fn as_negotiating_channel(&mut self) -> Result, ChannelError> { + pub fn as_negotiating_channel(&mut self) -> Option { match &mut self.phase { - ChannelPhase::UnfundedV2(chan) => Ok(chan.as_negotiating_channel()), + ChannelPhase::UnfundedV2(chan) => Some(chan.as_negotiating_channel()), #[cfg(splicing)] - ChannelPhase::Funded(chan) => { - chan.as_renegotiating_channel().map_err(|err| ChannelError::Warn(err.into())) - }, - _ => Err(ChannelError::Warn( - "Got a transaction negotiation message in an invalid phase".to_owned(), - )), + ChannelPhase::Funded(chan) => chan.as_renegotiating_channel(), + _ => None, } } @@ -1780,14 +1776,63 @@ where } pub fn funding_tx_constructed( - &mut self, signing_session: InteractiveTxSigningSession, logger: &L, + &mut self, logger: &L, ) -> Result<(msgs::CommitmentSigned, Option), ChannelError> where L::Target: Logger, { let logger = WithChannelContext::from(logger, self.context(), None); - let mut negotiating_channel = self.as_negotiating_channel()?; - negotiating_channel.funding_tx_constructed(signing_session, &&logger) + let (funding, context, signing_session, is_splice, holder_commitment_transaction_number) = + // TODO: Could use a wrapper or trait to DRY this + match &mut self.phase { + ChannelPhase::UnfundedV2(chan) => { + chan.interactive_tx_signing_session = Some( + chan.interactive_tx_constructor + .take() + .expect("TODO") + .into_signing_session(), + ); + ( + &mut chan.funding, + &mut chan.context, + chan.interactive_tx_signing_session.as_mut().expect("TODO"), + false, + chan.unfunded_context.transaction_number(), + ) + }, + #[cfg(splicing)] + ChannelPhase::Funded(chan) => { + chan.interactive_tx_signing_session = Some( + chan.pending_splice + .as_mut() + .expect("TODO") + .interactive_tx_constructor + .take() + .expect("TODO") + .into_signing_session(), + ); + ( + &mut chan.funding, + &mut chan.context, + chan.interactive_tx_signing_session.as_mut().expect("TODO"), + true, + chan.holder_commitment_point.transaction_number(), + ) + }, + _ => { + return Err(ChannelError::Warn( + "Got a transaction negotiation message in an invalid phase".to_owned(), + )) + }, + }; + funding_tx_constructed( + funding, + context, + signing_session, + is_splice, + holder_commitment_transaction_number, + &&logger, + ) } pub fn force_shutdown(&mut self, closure_reason: ClosureReason) -> ShutdownResult { @@ -2286,7 +2331,7 @@ struct PendingSplice { /// Intended contributions to the splice from our end pub our_funding_contribution: i64, funding: Option, - funding_negotiation_context: FundingNegotiationContext, + funding_negotiation_context: Option, /// The current interactive transaction construction session under negotiation. interactive_tx_constructor: Option, @@ -2895,295 +2940,232 @@ where /// Can be produced by: /// - [`PendingV2Channel`], at V2 channel open, and /// - [`FundedChannel`], when splicing. -pub(super) struct NegotiatingChannelView<'a, SP: Deref> +pub(super) struct NegotiatingChannelView<'a>(&'a mut InteractiveTxConstructor); + +/// Prepare and start interactive transaction negotiation. +/// `change_destination_opt` - Optional destination for optional change; if None, +/// default destination address is used. +/// If error occurs, it is caused by our side, not the counterparty. +#[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled +fn begin_interactive_funding_tx_construction( + funding: &FundingScope, context: &ChannelContext, + mut funding_negotiation_context: FundingNegotiationContext, is_splice: bool, + signer_provider: &SP, entropy_source: &ES, holder_node_id: PublicKey, + change_destination_opt: Option, shared_funding_input: Option, +) -> Result where SP::Target: SignerProvider, + ES::Target: EntropySource, { - context: &'a mut ChannelContext, - /// The funding scope being (re)negotiated. - /// In case of splicing it is the new spliced scope. - funding: &'a mut FundingScope, - funding_negotiation_context: &'a mut FundingNegotiationContext, - interactive_tx_constructor: &'a mut Option, - interactive_tx_signing_session: &'a mut Option, - holder_commitment_transaction_number: u64, + if is_splice { + debug_assert!(matches!(context.channel_state, ChannelState::ChannelReady(_))); + } else { + debug_assert!(matches!(context.channel_state, ChannelState::NegotiatingFunding(_))); + } + + // Add output for funding tx + // Note: For the error case when the inputs are insufficient, it will be handled after + // the `calculate_change_output_value` call below + let mut funding_outputs = Vec::new(); + + let shared_funding_output = TxOut { + value: Amount::from_sat(funding.get_value_satoshis()), + script_pubkey: funding.get_funding_redeemscript().to_p2wsh(), + }; + + // Optionally add change output + let change_script = if let Some(script) = change_destination_opt { + script + } else { + signer_provider + .get_destination_script(context.channel_keys_id) + .map_err(|_err| AbortReason::InternalError("Error getting destination script"))? + }; + let change_value_opt = calculate_change_output_value( + &funding_negotiation_context, + shared_funding_input.as_ref().map(|input| input.local_owned()), + &shared_funding_output.script_pubkey, + &funding_outputs, + change_script.minimal_non_dust().to_sat(), + )?; + if let Some(change_value) = change_value_opt { + let mut change_output = + TxOut { value: Amount::from_sat(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( + funding_negotiation_context.funding_feerate_sat_per_1000_weight, + change_output_weight, + ); + let change_value_decreased_with_fee = change_value.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); + funding_outputs.push(change_output); + } + } + + let mut funding_inputs = Vec::new(); + mem::swap(&mut funding_negotiation_context.our_funding_inputs, &mut funding_inputs); + + let constructor_args = InteractiveTxConstructorArgs { + entropy_source, + holder_node_id, + counterparty_node_id: context.counterparty_node_id, + channel_id: context.channel_id(), + feerate_sat_per_kw: funding_negotiation_context.funding_feerate_sat_per_1000_weight, + is_initiator: funding_negotiation_context.is_initiator, + funding_tx_locktime: funding_negotiation_context.funding_tx_locktime, + inputs_to_contribute: funding_inputs, + shared_funding_input, + shared_funding_output: SharedOwnedOutput::new( + shared_funding_output, + funding_negotiation_context.our_funding_satoshis, + ), + outputs_to_contribute: funding_outputs, + }; + InteractiveTxConstructor::new(constructor_args) } -impl<'a, SP: Deref> NegotiatingChannelView<'a, SP> +#[rustfmt::skip] +fn funding_tx_constructed( + funding: &mut FundingScope, context: &mut ChannelContext, + signing_session: &mut InteractiveTxSigningSession, is_splice: bool, + holder_commitment_transaction_number: u64, logger: &L +) -> Result<(msgs::CommitmentSigned, Option), ChannelError> where SP::Target: SignerProvider, + L::Target: Logger { - fn is_splice(&self) -> bool { - self.funding.channel_transaction_parameters.splice_parent_funding_txid.is_some() - } - - /// Prepare and start interactive transaction negotiation. - /// `change_destination_opt` - Optional destination for optional change; if None, - /// default destination address is used. - /// If error occurs, it is caused by our side, not the counterparty. - #[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled - fn begin_interactive_funding_tx_construction( - &mut self, signer_provider: &SP, entropy_source: &ES, holder_node_id: PublicKey, - change_destination_opt: Option, shared_funding_input: Option, - ) -> Result, AbortReason> - where - ES::Target: EntropySource, - { - if self.is_splice() { - debug_assert!(matches!(self.context.channel_state, ChannelState::ChannelReady(_))); - } else { - debug_assert!(matches!( - self.context.channel_state, - ChannelState::NegotiatingFunding(_) - )); + let mut output_index = None; + let expected_spk = funding.get_funding_redeemscript().to_p2wsh(); + for (idx, outp) in signing_session.unsigned_tx().outputs().enumerate() { + if outp.script_pubkey() == &expected_spk && outp.value() == funding.get_value_satoshis() { + if output_index.is_some() { + let msg = "Multiple outputs matched the expected script and value"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); + } + output_index = Some(idx as u16); } + } + let outpoint = if let Some(output_index) = output_index { + OutPoint { txid: signing_session.unsigned_tx().compute_txid(), index: output_index } + } else { + let msg = "No output matched the funding script_pubkey"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); + }; + funding + .channel_transaction_parameters.funding_outpoint = Some(outpoint); + + if is_splice { + let message = "TODO Forced error, incomplete implementation".to_owned(); + // TODO(splicing) Forced error, as the use case is not complete + return Err(ChannelError::Close(( + message.clone(), + ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false), message } + ))); + } - debug_assert!(self.interactive_tx_constructor.is_none()); - - // Add output for funding tx - // Note: For the error case when the inputs are insufficient, it will be handled after - // the `calculate_change_output_value` call below - let mut funding_outputs = Vec::new(); - - let shared_funding_output = TxOut { - value: Amount::from_sat(self.funding.get_value_satoshis()), - script_pubkey: self.funding.get_funding_redeemscript().to_p2wsh(), - }; + context.assert_no_commitment_advancement(holder_commitment_transaction_number, "initial commitment_signed"); + let commitment_signed = context.get_initial_commitment_signed(&funding, logger); + let commitment_signed = match commitment_signed { + Ok(commitment_signed) => commitment_signed, + Err(e) => { + funding.channel_transaction_parameters.funding_outpoint = None; + return Err(e) + }, + }; - // Optionally add change output - let change_script = if let Some(script) = change_destination_opt { - script - } else { - signer_provider - .get_destination_script(self.context.channel_keys_id) - .map_err(|_err| AbortReason::InternalError("Error getting destination script"))? - }; - let change_value_opt = calculate_change_output_value( - &self.funding_negotiation_context, - shared_funding_input.as_ref().map(|input| input.local_owned()), - &shared_funding_output.script_pubkey, - &funding_outputs, - change_script.minimal_non_dust().to_sat(), - )?; - if let Some(change_value) = change_value_opt { - let mut change_output = - TxOut { value: Amount::from_sat(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_negotiation_context.funding_feerate_sat_per_1000_weight, - change_output_weight, + let funding_ready_for_sig_event = if signing_session.local_inputs_count() == 0 { + if signing_session.provide_holder_witnesses(context.channel_id, Vec::new()).is_err() { + debug_assert!( + false, + "Zero inputs were provided & zero witnesses were provided, but a count mismatch was somehow found", ); - let change_value_decreased_with_fee = change_value.saturating_sub(change_output_fee); - // Check dust limit again - if change_value_decreased_with_fee > self.context.holder_dust_limit_satoshis { - change_output.value = Amount::from_sat(change_value_decreased_with_fee); - funding_outputs.push(change_output); - } + let msg = "V2 channel rejected due to sender error"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); } + None + } else { + // TODO(dual_funding): Send event for signing if we've contributed funds. + // Inform the user that SIGHASH_ALL must be used for all signatures when contributing + // inputs/signatures. + // Also warn the user that we don't do anything to prevent the counterparty from + // providing non-standard witnesses which will prevent the funding transaction from + // confirming. This warning must appear in doc comments wherever the user is contributing + // funds, whether they are initiator or acceptor. + // + // The following warning can be used when the APIs allowing contributing inputs become available: + //
+ // WARNING: LDK makes no attempt to prevent the counterparty from using non-standard inputs which + // will prevent the funding transaction from being relayed on the bitcoin network and hence being + // confirmed. + //
+ debug_assert!( + false, + "We don't support users providing inputs but somehow we had more than zero inputs", + ); + let msg = "V2 channel rejected due to sender error"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); + }; - let mut funding_inputs = Vec::new(); - mem::swap(&mut self.funding_negotiation_context.our_funding_inputs, &mut funding_inputs); - - let constructor_args = InteractiveTxConstructorArgs { - entropy_source, - holder_node_id, - counterparty_node_id: self.context.counterparty_node_id, - channel_id: self.context.channel_id(), - feerate_sat_per_kw: self - .funding_negotiation_context - .funding_feerate_sat_per_1000_weight, - is_initiator: self.funding_negotiation_context.is_initiator, - funding_tx_locktime: self.funding_negotiation_context.funding_tx_locktime, - inputs_to_contribute: funding_inputs, - shared_funding_input, - shared_funding_output: SharedOwnedOutput::new( - shared_funding_output, - self.funding_negotiation_context.our_funding_satoshis, - ), - outputs_to_contribute: funding_outputs, - }; - let mut tx_constructor = InteractiveTxConstructor::new(constructor_args)?; - let msg = tx_constructor.take_initiator_first_message(); - - *self.interactive_tx_constructor = Some(tx_constructor); + let mut channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); + channel_state.set_interactive_signing(); + context.channel_state = channel_state; - Ok(msg) - } + Ok((commitment_signed, funding_ready_for_sig_event)) +} +impl<'a> NegotiatingChannelView<'a> { pub(super) fn tx_add_input( &mut self, msg: &msgs::TxAddInput, ) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult(match self.interactive_tx_constructor { - Some(ref mut tx_constructor) => tx_constructor + InteractiveTxMessageSendResult( + self.0 .handle_tx_add_input(msg) - .map_err(|reason| reason.into_tx_abort_msg(self.context.channel_id())), - None => Err(msgs::TxAbort { - channel_id: self.context.channel_id(), - data: b"No interactive transaction negotiation in progress".to_vec(), - }), - }) + .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), + ) } pub(super) fn tx_add_output( &mut self, msg: &msgs::TxAddOutput, ) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult(match self.interactive_tx_constructor { - Some(ref mut tx_constructor) => tx_constructor + InteractiveTxMessageSendResult( + self.0 .handle_tx_add_output(msg) - .map_err(|reason| reason.into_tx_abort_msg(self.context.channel_id())), - None => Err(msgs::TxAbort { - channel_id: self.context.channel_id(), - data: b"No interactive transaction negotiation in progress".to_vec(), - }), - }) + .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), + ) } pub(super) fn tx_remove_input( &mut self, msg: &msgs::TxRemoveInput, ) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult(match self.interactive_tx_constructor { - Some(ref mut tx_constructor) => tx_constructor + InteractiveTxMessageSendResult( + self.0 .handle_tx_remove_input(msg) - .map_err(|reason| reason.into_tx_abort_msg(self.context.channel_id())), - None => Err(msgs::TxAbort { - channel_id: self.context.channel_id(), - data: b"No interactive transaction negotiation in progress".to_vec(), - }), - }) + .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), + ) } pub(super) fn tx_remove_output( &mut self, msg: &msgs::TxRemoveOutput, ) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult(match self.interactive_tx_constructor { - Some(ref mut tx_constructor) => tx_constructor + InteractiveTxMessageSendResult( + self.0 .handle_tx_remove_output(msg) - .map_err(|reason| reason.into_tx_abort_msg(self.context.channel_id())), - None => Err(msgs::TxAbort { - channel_id: self.context.channel_id(), - data: b"No interactive transaction negotiation in progress".to_vec(), - }), - }) + .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), + ) } pub(super) fn tx_complete(&mut self, msg: &msgs::TxComplete) -> HandleTxCompleteResult { - let tx_constructor = match self.interactive_tx_constructor { - Some(tx_constructor) => tx_constructor, - None => { - let tx_abort = msgs::TxAbort { - channel_id: msg.channel_id, - data: b"No interactive transaction negotiation in progress".to_vec(), - }; - return HandleTxCompleteResult(Err(tx_abort)); - }, - }; - - let tx_complete = match tx_constructor.handle_tx_complete(msg) { - Ok(tx_complete) => tx_complete, - Err(reason) => { - return HandleTxCompleteResult(Err(reason.into_tx_abort_msg(msg.channel_id))) - }, - }; - - HandleTxCompleteResult(Ok(tx_complete)) - } - - #[rustfmt::skip] - fn funding_tx_constructed( - &mut self, mut signing_session: InteractiveTxSigningSession, logger: &L - ) -> Result<(msgs::CommitmentSigned, Option), ChannelError> - where - L::Target: Logger - { - let our_funding_satoshis = self.funding_negotiation_context - .our_funding_satoshis; - - let mut output_index = None; - let expected_spk = self.funding.get_funding_redeemscript().to_p2wsh(); - for (idx, outp) in signing_session.unsigned_tx().outputs().enumerate() { - if outp.script_pubkey() == &expected_spk && outp.value() == self.funding.get_value_satoshis() { - if output_index.is_some() { - let msg = "Multiple outputs matched the expected script and value"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); - } - output_index = Some(idx as u16); - } - } - let outpoint = if let Some(output_index) = output_index { - OutPoint { txid: signing_session.unsigned_tx().compute_txid(), index: output_index } - } else { - let msg = "No output matched the funding script_pubkey"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); - }; - self.funding - .channel_transaction_parameters.funding_outpoint = Some(outpoint); - - if self.is_splice() { - let message = "TODO Forced error, incomplete implementation".to_owned(); - // TODO(splicing) Forced error, as the use case is not complete - return Err(ChannelError::Close(( - message.clone(), - ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false), message } - ))); - } - - self.context.assert_no_commitment_advancement(self.holder_commitment_transaction_number, "initial commitment_signed"); - let commitment_signed = self.context.get_initial_commitment_signed(&self.funding, logger); - let commitment_signed = match commitment_signed { - Ok(commitment_signed) => commitment_signed, - Err(e) => { - self.funding.channel_transaction_parameters.funding_outpoint = None; - return Err(e) - }, - }; - - let funding_ready_for_sig_event = if signing_session.local_inputs_count() == 0 { - debug_assert_eq!(our_funding_satoshis, 0); - if signing_session.provide_holder_witnesses(self.context.channel_id, Vec::new()).is_err() { - debug_assert!( - false, - "Zero inputs were provided & zero witnesses were provided, but a count mismatch was somehow found", - ); - let msg = "V2 channel rejected due to sender error"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); - } - None - } else { - // TODO(dual_funding): Send event for signing if we've contributed funds. - // Inform the user that SIGHASH_ALL must be used for all signatures when contributing - // inputs/signatures. - // Also warn the user that we don't do anything to prevent the counterparty from - // providing non-standard witnesses which will prevent the funding transaction from - // confirming. This warning must appear in doc comments wherever the user is contributing - // funds, whether they are initiator or acceptor. - // - // The following warning can be used when the APIs allowing contributing inputs become available: - //
- // WARNING: LDK makes no attempt to prevent the counterparty from using non-standard inputs which - // will prevent the funding transaction from being relayed on the bitcoin network and hence being - // confirmed. - //
- debug_assert!( - false, - "We don't support users providing inputs but somehow we had more than zero inputs", - ); - let msg = "V2 channel rejected due to sender error"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); - }; - - let mut channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); - channel_state.set_interactive_signing(); - self.context.channel_state = channel_state; - - // Clear the interactive transaction constructor - *self.interactive_tx_constructor = None; - *self.interactive_tx_signing_session = Some(signing_session); - - Ok((commitment_signed, funding_ready_for_sig_event)) + HandleTxCompleteResult( + self.0 + .handle_tx_complete(msg) + .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), + ) } } @@ -6179,26 +6161,11 @@ where /// If we are in splicing/refunding, return a short-lived [`NegotiatingChannelView`]. #[cfg(splicing)] - fn as_renegotiating_channel(&mut self) -> Result, &'static str> { - if let Some(ref mut pending_splice) = &mut self.pending_splice { - if let Some(ref mut funding) = &mut pending_splice.funding { - Ok(NegotiatingChannelView { - context: &mut self.context, - funding, - funding_negotiation_context: &mut pending_splice.funding_negotiation_context, - interactive_tx_constructor: &mut pending_splice.interactive_tx_constructor, - interactive_tx_signing_session: &mut self.interactive_tx_signing_session, - holder_commitment_transaction_number: self - .holder_commitment_point - .transaction_number(), - }) - } else { - Err("Received unexpected interactive transaction negotiation message: \ - the channel is splicing, but splice_init/splice_ack has not been exchanged yet") - } - } else { - Err("Received unexpected interactive transaction negotiation message: the channel is funded and not splicing") - } + fn as_renegotiating_channel(&mut self) -> Option { + self.pending_splice + .as_mut() + .and_then(|pending_splice| pending_splice.interactive_tx_constructor.as_mut()) + .map(|interactive_tx_constructor| NegotiatingChannelView(interactive_tx_constructor)) } #[rustfmt::skip] @@ -10455,7 +10422,7 @@ where self.pending_splice = Some(PendingSplice { our_funding_contribution: our_funding_contribution_satoshis, funding: None, - funding_negotiation_context, + funding_negotiation_context: Some(funding_negotiation_context), interactive_tx_constructor: None, sent_funding_txid: None, received_funding_txid: None, @@ -10595,7 +10562,7 @@ where self.pending_splice = Some(PendingSplice { our_funding_contribution, funding: Some(funding_scope), - funding_negotiation_context, + funding_negotiation_context: Some(funding_negotiation_context), interactive_tx_constructor: None, received_funding_txid: None, sent_funding_txid: None, @@ -10606,31 +10573,29 @@ where let splice_ack_msg = self.get_splice_ack(our_funding_contribution); - // Build NegotiatingChannelView locally, similar to Channel::as_renegotiating_channel() - let pending_splice_mut = &mut self.pending_splice.as_mut().unwrap(); // set above - let mut negotiating_view = NegotiatingChannelView { - context: &mut self.context, - funding: &mut pending_splice_mut.funding.as_mut().unwrap(), // set above - funding_negotiation_context: &mut pending_splice_mut.funding_negotiation_context, - interactive_tx_constructor: &mut pending_splice_mut.interactive_tx_constructor, - interactive_tx_signing_session: &mut self.interactive_tx_signing_session, - holder_commitment_transaction_number: self.holder_commitment_point.transaction_number(), - }; + let pending_splice = self.pending_splice.as_mut().unwrap(); + let mut interactive_tx_constructor = begin_interactive_funding_tx_construction( + pending_splice.funding.as_ref().unwrap(), + &self.context, + pending_splice.funding_negotiation_context.take().unwrap(), + true, + signer_provider, + entropy_source, + holder_node_id.clone(), + None, + Some(prev_funding_input), + ) + .map_err(|err| { + ChannelError::Warn(format!( + "Failed to start interactive transaction construction, {:?}", + err + )) + })?; - let _msg = negotiating_view - .begin_interactive_funding_tx_construction( - signer_provider, - entropy_source, - holder_node_id.clone(), - None, - Some(prev_funding_input), - ) - .map_err(|err| { - ChannelError::Warn(format!( - "Failed to start interactive transaction construction, {:?}", - err - )) - })?; + // TODO: Propagate message + let _msg = interactive_tx_constructor.take_initiator_first_message(); + self.pending_splice.as_mut().unwrap().interactive_tx_constructor = + Some(interactive_tx_constructor); Ok(splice_ack_msg) } @@ -10712,8 +10677,9 @@ where debug_assert!(pending_splice.funding.is_none()); pending_splice.funding = Some(funding_scope); // update funding values - pending_splice.funding_negotiation_context.our_funding_satoshis = our_funding_satoshis; - pending_splice.funding_negotiation_context.their_funding_satoshis = + pending_splice.funding_negotiation_context.as_mut().unwrap().our_funding_satoshis = + our_funding_satoshis; + pending_splice.funding_negotiation_context.as_mut().unwrap().their_funding_satoshis = Some(their_funding_satoshis); debug_assert!(pending_splice.interactive_tx_constructor.is_none()); debug_assert!(self.interactive_tx_signing_session.is_none()); @@ -10721,28 +10687,24 @@ where log_info!(logger, "Splicing process started after splice_ack, new channel value {}, old {}, outgoing {}, channel_id {}", post_channel_value, pre_channel_value, true, self.context.channel_id); - // Build NegotiatingChannelView locally, similar to Channel::as_renegotiating_channel() - let mut negotiating_view = NegotiatingChannelView { - context: &mut self.context, - funding: &mut pending_splice.funding.as_mut().unwrap(), // set above - funding_negotiation_context: &mut pending_splice.funding_negotiation_context, - interactive_tx_constructor: &mut pending_splice.interactive_tx_constructor, - interactive_tx_signing_session: &mut self.interactive_tx_signing_session, - holder_commitment_transaction_number: self.holder_commitment_point.transaction_number(), - }; - // Start interactive funding negotiation, with the previous funding transaction as an extra shared input - let tx_msg_opt = negotiating_view - .begin_interactive_funding_tx_construction( - signer_provider, - entropy_source, - holder_node_id.clone(), - None, - Some(prev_funding_input), - ) - .map_err(|err| { - ChannelError::Warn(format!("V2 channel rejected due to sender error, {:?}", err)) - })?; + let mut interactive_tx_constructor = begin_interactive_funding_tx_construction( + pending_splice.funding.as_ref().unwrap(), + &self.context, + pending_splice.funding_negotiation_context.take().unwrap(), + true, + signer_provider, + entropy_source, + holder_node_id.clone(), + None, + Some(prev_funding_input), + ) + .map_err(|err| { + ChannelError::Warn(format!("V2 channel rejected due to sender error, {:?}", err)) + })?; + let tx_msg_opt = interactive_tx_constructor.take_initiator_first_message(); + self.pending_splice.as_mut().unwrap().interactive_tx_constructor = + Some(interactive_tx_constructor); Ok(tx_msg_opt) } @@ -12617,15 +12579,8 @@ where } /// Return a short-lived [`NegotiatingChannelView`]. - fn as_negotiating_channel(&mut self) -> NegotiatingChannelView { - NegotiatingChannelView { - context: &mut self.context, - funding: &mut self.funding, - funding_negotiation_context: &mut self.funding_negotiation_context, - interactive_tx_constructor: &mut self.interactive_tx_constructor, - interactive_tx_signing_session: &mut self.interactive_tx_signing_session, - holder_commitment_transaction_number: self.unfunded_context.transaction_number(), - } + fn as_negotiating_channel(&mut self) -> NegotiatingChannelView { + NegotiatingChannelView(self.interactive_tx_constructor.as_mut().expect("TODO")) } } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6359a78b107..07159aacd9a 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7661,7 +7661,8 @@ where ComplFunc: FnOnce( Option, bool, - ) -> (Option, Option), + ) + -> (Option, Option), >( &self, prev_hop: HTLCPreviousHopData, payment_preimage: PaymentPreimage, payment_info: Option, completion_action: ComplFunc, @@ -8893,9 +8894,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } - fn internal_tx_msg< - HandleTxMsgFn: Fn(&mut Channel) -> Result, - >( + fn internal_tx_msg) -> Option>( &self, counterparty_node_id: &PublicKey, channel_id: ChannelId, tx_msg_handler: HandleTxMsgFn, ) -> Result<(), MsgHandleErrInternal> { @@ -8916,8 +8915,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ hash_map::Entry::Occupied(mut chan_entry) => { let channel = chan_entry.get_mut(); let msg_send_event = match tx_msg_handler(channel) { - Ok(msg_send_event) => msg_send_event, - Err(err) => return Err(MsgHandleErrInternal::from_chan_no_close(err, channel_id)), + Some(msg_send_event) => msg_send_event, + None => { + let err = ChannelError::Warn("Received unexpected interactive transaction negotiation message".to_owned()); + return Err(MsgHandleErrInternal::from_chan_no_close(err, channel_id)) + }, }; peer_state.pending_msg_events.push(msg_send_event); Ok(()) @@ -8935,10 +8937,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self, counterparty_node_id: PublicKey, msg: &msgs::TxAddInput, ) -> Result<(), MsgHandleErrInternal> { self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel| { - Ok(channel - .as_negotiating_channel()? - .tx_add_input(msg) - .into_msg_send_event(counterparty_node_id)) + Some( + channel + .as_negotiating_channel()? + .tx_add_input(msg) + .into_msg_send_event(counterparty_node_id), + ) }) } @@ -8946,10 +8950,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self, counterparty_node_id: PublicKey, msg: &msgs::TxAddOutput, ) -> Result<(), MsgHandleErrInternal> { self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel| { - Ok(channel - .as_negotiating_channel()? - .tx_add_output(msg) - .into_msg_send_event(counterparty_node_id)) + Some( + channel + .as_negotiating_channel()? + .tx_add_output(msg) + .into_msg_send_event(counterparty_node_id), + ) }) } @@ -8957,10 +8963,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveInput, ) -> Result<(), MsgHandleErrInternal> { self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel| { - Ok(channel - .as_negotiating_channel()? - .tx_remove_input(msg) - .into_msg_send_event(counterparty_node_id)) + Some( + channel + .as_negotiating_channel()? + .tx_remove_input(msg) + .into_msg_send_event(counterparty_node_id), + ) }) } @@ -8968,10 +8976,12 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self, counterparty_node_id: PublicKey, msg: &msgs::TxRemoveOutput, ) -> Result<(), MsgHandleErrInternal> { self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel| { - Ok(channel - .as_negotiating_channel()? - .tx_remove_output(msg) - .into_msg_send_event(counterparty_node_id)) + Some( + channel + .as_negotiating_channel()? + .tx_remove_output(msg) + .into_msg_send_event(counterparty_node_id), + ) }) } @@ -8989,21 +8999,22 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let peer_state = &mut *peer_state_lock; match peer_state.channel_by_id.entry(msg.channel_id) { hash_map::Entry::Occupied(mut chan_entry) => { - let (msg_send_event_opt, signing_session_opt) = match chan_entry.get_mut().as_negotiating_channel() { - Ok(mut negotiating_channel) => negotiating_channel + let (msg_send_event_opt, ready_to_sign) = match chan_entry.get_mut().as_negotiating_channel() { + Some(mut negotiating_channel) => negotiating_channel .tx_complete(msg) - .into_msg_send_event_or_signing_session(counterparty_node_id), - Err(err) => { - try_channel_entry!(self, peer_state, Err(err), chan_entry) + .into_msg_send_event(counterparty_node_id), + None => { + let err = ChannelError::Warn("Received unexpected tx_complete message".to_owned()); + return Err(MsgHandleErrInternal::from_chan_no_close(err, msg.channel_id)) }, }; if let Some(msg_send_event) = msg_send_event_opt { peer_state.pending_msg_events.push(msg_send_event); }; - if let Some(signing_session) = signing_session_opt { + if ready_to_sign { let (commitment_signed, funding_ready_for_sig_event_opt) = chan_entry .get_mut() - .funding_tx_constructed(signing_session, &self.logger) + .funding_tx_constructed(&self.logger) .map_err(|err| MsgHandleErrInternal::send_err_msg_no_close(format!("{}", err), msg.channel_id))?; if let Some(funding_ready_for_sig_event) = funding_ready_for_sig_event_opt { let mut pending_events = self.pending_events.lock().unwrap(); diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 82bb122736e..7a6c4516a38 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -1604,24 +1604,22 @@ where pub(super) enum HandleTxCompleteValue { SendTxMessage(InteractiveTxMessageSend), - SendTxComplete(InteractiveTxMessageSend, InteractiveTxSigningSession), - NegotiationComplete(InteractiveTxSigningSession), + SendTxComplete(InteractiveTxMessageSend, bool), + NegotiationComplete, } impl HandleTxCompleteValue { - pub fn into_msg_send_event_or_signing_session( + pub fn into_msg_send_event( self, counterparty_node_id: PublicKey, - ) -> (Option, Option) { + ) -> (Option, bool) { match self { HandleTxCompleteValue::SendTxMessage(msg) => { - (Some(msg.into_msg_send_event(counterparty_node_id)), None) + (Some(msg.into_msg_send_event(counterparty_node_id)), false) }, - HandleTxCompleteValue::SendTxComplete(msg, signing_session) => { - (Some(msg.into_msg_send_event(counterparty_node_id)), Some(signing_session)) - }, - HandleTxCompleteValue::NegotiationComplete(signing_session) => { - (None, Some(signing_session)) + HandleTxCompleteValue::SendTxComplete(msg, ready_to_sign) => { + (Some(msg.into_msg_send_event(counterparty_node_id)), ready_to_sign) }, + HandleTxCompleteValue::NegotiationComplete => (None, true), } } } @@ -1629,19 +1627,19 @@ impl HandleTxCompleteValue { pub(super) struct HandleTxCompleteResult(pub Result); impl HandleTxCompleteResult { - pub fn into_msg_send_event_or_signing_session( + pub fn into_msg_send_event( self, counterparty_node_id: PublicKey, - ) -> (Option, Option) { + ) -> (Option, bool) { match self.0 { Ok(interactive_tx_msg_send) => { - interactive_tx_msg_send.into_msg_send_event_or_signing_session(counterparty_node_id) + interactive_tx_msg_send.into_msg_send_event(counterparty_node_id) }, Err(tx_abort_msg) => ( Some(MessageSendEvent::SendTxAbort { node_id: counterparty_node_id, msg: tx_abort_msg, }), - None, + false, ), } } @@ -1839,8 +1837,8 @@ impl InteractiveTxConstructor { StateMachine::ReceivedTxComplete(_) => { let msg_send = self.maybe_send_message()?; match &self.state_machine { - StateMachine::NegotiationComplete(s) => { - Ok(HandleTxCompleteValue::SendTxComplete(msg_send, s.0.clone())) + StateMachine::NegotiationComplete(_) => { + Ok(HandleTxCompleteValue::SendTxComplete(msg_send, true)) }, StateMachine::SentChangeMsg(_) => { Ok(HandleTxCompleteValue::SendTxMessage(msg_send)) @@ -1851,9 +1849,7 @@ impl InteractiveTxConstructor { }, } }, - StateMachine::NegotiationComplete(s) => { - Ok(HandleTxCompleteValue::NegotiationComplete(s.0.clone())) - }, + StateMachine::NegotiationComplete(_) => Ok(HandleTxCompleteValue::NegotiationComplete), _ => { debug_assert!( false, @@ -1863,6 +1859,13 @@ impl InteractiveTxConstructor { }, } } + + pub fn into_signing_session(self) -> InteractiveTxSigningSession { + match self.state_machine { + StateMachine::NegotiationComplete(s) => s.0, + _ => panic!("Signing session is not ready yet"), + } + } } /// Determine whether a change output should be added, and if yes, of what size, considering our @@ -2092,7 +2095,7 @@ mod tests { ), outputs_to_contribute: session.outputs_a, }) { - Ok(r) => r, + Ok(r) => Some(r), Err(abort_reason) => { assert_eq!( Some((abort_reason, ErrorCulprit::NodeA)), @@ -2129,7 +2132,7 @@ mod tests { ), outputs_to_contribute: session.outputs_b, }) { - Ok(r) => r, + Ok(r) => Some(r), Err(abort_reason) => { assert_eq!( Some((abort_reason, ErrorCulprit::NodeB)), @@ -2146,35 +2149,43 @@ mod tests { match msg { InteractiveTxMessageSend::TxAddInput(msg) => for_constructor .handle_tx_add_input(&msg) - .map(|msg_send| (Some(msg_send), None)), + .map(|msg_send| (Some(msg_send), false)), InteractiveTxMessageSend::TxAddOutput(msg) => for_constructor .handle_tx_add_output(&msg) - .map(|msg_send| (Some(msg_send), None)), + .map(|msg_send| (Some(msg_send), false)), InteractiveTxMessageSend::TxComplete(msg) => { for_constructor.handle_tx_complete(&msg).map(|value| match value { HandleTxCompleteValue::SendTxMessage(msg_send) => { - (Some(msg_send), None) + (Some(msg_send), false) }, - HandleTxCompleteValue::SendTxComplete(msg_send, tx) => { - (Some(msg_send), Some(tx)) + HandleTxCompleteValue::SendTxComplete(msg_send, ready_to_sign) => { + (Some(msg_send), ready_to_sign) }, - HandleTxCompleteValue::NegotiationComplete(tx) => (None, Some(tx)), + HandleTxCompleteValue::NegotiationComplete => (None, true), }) }, } }; - let mut message_send_a = constructor_a.take_initiator_first_message(); + 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 final_tx_a.is_none() || final_tx_b.is_none() { + while constructor_a.is_some() || constructor_b.is_some() { if let Some(message_send_a) = message_send_a.take() { - match handle_message_send(message_send_a, &mut constructor_b) { - Ok((msg_send, interactive_signing_session)) => { + match handle_message_send(message_send_a, constructor_b.as_mut().unwrap()) { + Ok((msg_send, ready_to_sign)) => { message_send_b = msg_send; - final_tx_b = interactive_signing_session - .map(|session| session.unsigned_tx.compute_txid()); + if ready_to_sign { + final_tx_b = Some( + constructor_b + .take() + .unwrap() + .into_signing_session() + .unsigned_tx + .compute_txid(), + ); + } }, Err(abort_reason) => { let error_culprit = match abort_reason { @@ -2195,11 +2206,19 @@ mod tests { } } if let Some(message_send_b) = message_send_b.take() { - match handle_message_send(message_send_b, &mut constructor_a) { - Ok((msg_send, interactive_signing_session)) => { + match handle_message_send(message_send_b, constructor_a.as_mut().unwrap()) { + Ok((msg_send, ready_to_sign)) => { message_send_a = msg_send; - final_tx_a = interactive_signing_session - .map(|session| session.unsigned_tx.compute_txid()); + if ready_to_sign { + final_tx_a = Some( + constructor_a + .take() + .unwrap() + .into_signing_session() + .unsigned_tx + .compute_txid(), + ); + } }, Err(abort_reason) => { let error_culprit = match abort_reason { @@ -2995,10 +3014,7 @@ mod tests { ); // There is leftover for change, without common fees - let context = FundingNegotiationContext { - is_initiator: false, - ..context - }; + let context = FundingNegotiationContext { is_initiator: false, ..context }; assert_eq!( calculate_change_output_value(&context, None, &ScriptBuf::new(), &outputs, 300), Ok(Some(gross_change - fees)), From f75c489d6725ce06846270056c36e6a8f88d4ef4 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 17 Jul 2025 15:47:06 -0500 Subject: [PATCH 14/22] f - move instead of swap --- lightning/src/ln/channel.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index d805f22f503..f918c404ef3 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2949,7 +2949,7 @@ pub(super) struct NegotiatingChannelView<'a>(&'a mut InteractiveTxConstructor); #[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled fn begin_interactive_funding_tx_construction( funding: &FundingScope, context: &ChannelContext, - mut funding_negotiation_context: FundingNegotiationContext, is_splice: bool, + funding_negotiation_context: FundingNegotiationContext, is_splice: bool, signer_provider: &SP, entropy_source: &ES, holder_node_id: PublicKey, change_destination_opt: Option, shared_funding_input: Option, ) -> Result @@ -3004,9 +3004,6 @@ where } } - let mut funding_inputs = Vec::new(); - mem::swap(&mut funding_negotiation_context.our_funding_inputs, &mut funding_inputs); - let constructor_args = InteractiveTxConstructorArgs { entropy_source, holder_node_id, @@ -3015,7 +3012,7 @@ where feerate_sat_per_kw: funding_negotiation_context.funding_feerate_sat_per_1000_weight, is_initiator: funding_negotiation_context.is_initiator, funding_tx_locktime: funding_negotiation_context.funding_tx_locktime, - inputs_to_contribute: funding_inputs, + inputs_to_contribute: funding_negotiation_context.our_funding_inputs, shared_funding_input, shared_funding_output: SharedOwnedOutput::new( shared_funding_output, From b64c45fd1786bc745c6f7a0e382d7e229e16c8ac Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 17 Jul 2025 16:47:13 -0500 Subject: [PATCH 15/22] f - Move begin_interactive_funding_tx_construction --- lightning/src/ln/channel.rs | 262 ++++++++++++++++++------------------ 1 file changed, 129 insertions(+), 133 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f918c404ef3..73118be5a3d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2942,87 +2942,6 @@ where /// - [`FundedChannel`], when splicing. pub(super) struct NegotiatingChannelView<'a>(&'a mut InteractiveTxConstructor); -/// Prepare and start interactive transaction negotiation. -/// `change_destination_opt` - Optional destination for optional change; if None, -/// default destination address is used. -/// If error occurs, it is caused by our side, not the counterparty. -#[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled -fn begin_interactive_funding_tx_construction( - funding: &FundingScope, context: &ChannelContext, - funding_negotiation_context: FundingNegotiationContext, is_splice: bool, - signer_provider: &SP, entropy_source: &ES, holder_node_id: PublicKey, - change_destination_opt: Option, shared_funding_input: Option, -) -> Result -where - SP::Target: SignerProvider, - ES::Target: EntropySource, -{ - if is_splice { - debug_assert!(matches!(context.channel_state, ChannelState::ChannelReady(_))); - } else { - debug_assert!(matches!(context.channel_state, ChannelState::NegotiatingFunding(_))); - } - - // Add output for funding tx - // Note: For the error case when the inputs are insufficient, it will be handled after - // the `calculate_change_output_value` call below - let mut funding_outputs = Vec::new(); - - let shared_funding_output = TxOut { - value: Amount::from_sat(funding.get_value_satoshis()), - script_pubkey: funding.get_funding_redeemscript().to_p2wsh(), - }; - - // Optionally add change output - let change_script = if let Some(script) = change_destination_opt { - script - } else { - signer_provider - .get_destination_script(context.channel_keys_id) - .map_err(|_err| AbortReason::InternalError("Error getting destination script"))? - }; - let change_value_opt = calculate_change_output_value( - &funding_negotiation_context, - shared_funding_input.as_ref().map(|input| input.local_owned()), - &shared_funding_output.script_pubkey, - &funding_outputs, - change_script.minimal_non_dust().to_sat(), - )?; - if let Some(change_value) = change_value_opt { - let mut change_output = - TxOut { value: Amount::from_sat(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( - funding_negotiation_context.funding_feerate_sat_per_1000_weight, - change_output_weight, - ); - let change_value_decreased_with_fee = change_value.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); - funding_outputs.push(change_output); - } - } - - let constructor_args = InteractiveTxConstructorArgs { - entropy_source, - holder_node_id, - counterparty_node_id: context.counterparty_node_id, - channel_id: context.channel_id(), - feerate_sat_per_kw: funding_negotiation_context.funding_feerate_sat_per_1000_weight, - is_initiator: funding_negotiation_context.is_initiator, - funding_tx_locktime: funding_negotiation_context.funding_tx_locktime, - inputs_to_contribute: funding_negotiation_context.our_funding_inputs, - shared_funding_input, - shared_funding_output: SharedOwnedOutput::new( - shared_funding_output, - funding_negotiation_context.our_funding_satoshis, - ), - outputs_to_contribute: funding_outputs, - }; - InteractiveTxConstructor::new(constructor_args) -} - #[rustfmt::skip] fn funding_tx_constructed( funding: &mut FundingScope, context: &mut ChannelContext, @@ -6031,6 +5950,85 @@ pub(super) struct FundingNegotiationContext { pub our_funding_inputs: Vec<(TxIn, TransactionU16LenLimited)>, } +impl FundingNegotiationContext { + /// Prepare and start interactive transaction negotiation. + /// `change_destination_opt` - Optional destination for optional change; if None, + /// default destination address is used. + /// If error occurs, it is caused by our side, not the counterparty. + fn into_interactive_tx_constructor( + self, context: &ChannelContext, funding: &FundingScope, is_splice: bool, + signer_provider: &SP, entropy_source: &ES, holder_node_id: PublicKey, + change_destination_opt: Option, shared_funding_input: Option, + ) -> Result + where + SP::Target: SignerProvider, + ES::Target: EntropySource, + { + if is_splice { + debug_assert!(matches!(context.channel_state, ChannelState::ChannelReady(_))); + } else { + debug_assert!(matches!(context.channel_state, ChannelState::NegotiatingFunding(_))); + } + + // Add output for funding tx + // Note: For the error case when the inputs are insufficient, it will be handled after + // the `calculate_change_output_value` call below + let mut funding_outputs = Vec::new(); + + let shared_funding_output = TxOut { + value: Amount::from_sat(funding.get_value_satoshis()), + script_pubkey: funding.get_funding_redeemscript().to_p2wsh(), + }; + + // Optionally add change output + let change_script = if let Some(script) = change_destination_opt { + script + } else { + signer_provider + .get_destination_script(context.channel_keys_id) + .map_err(|_err| AbortReason::InternalError("Error getting destination script"))? + }; + let change_value_opt = calculate_change_output_value( + &self, + shared_funding_input.as_ref().map(|input| input.local_owned()), + &shared_funding_output.script_pubkey, + &funding_outputs, + change_script.minimal_non_dust().to_sat(), + )?; + if let Some(change_value) = change_value_opt { + let mut change_output = + TxOut { value: Amount::from_sat(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.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); + funding_outputs.push(change_output); + } + } + + let constructor_args = InteractiveTxConstructorArgs { + entropy_source, + holder_node_id, + 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, + shared_funding_output: SharedOwnedOutput::new( + shared_funding_output, + self.our_funding_satoshis, + ), + outputs_to_contribute: funding_outputs, + }; + InteractiveTxConstructor::new(constructor_args) + } +} + // Holder designates channel data owned for the benefit of the user client. // Counterparty designates channel data owned by the another channel participant entity. pub(super) struct FundedChannel @@ -10530,7 +10528,7 @@ where their_funding_contribution, ); - let funding_scope = FundingScope::for_splice( + let splice_funding = FundingScope::for_splice( &self.funding, &self.context, our_funding_contribution, @@ -10556,43 +10554,39 @@ where our_funding_inputs: Vec::new(), }; - self.pending_splice = Some(PendingSplice { - our_funding_contribution, - funding: Some(funding_scope), - funding_negotiation_context: Some(funding_negotiation_context), - interactive_tx_constructor: None, - received_funding_txid: None, - sent_funding_txid: None, - }); - log_info!(logger, "Splicing process started after splice_init, new channel value {}, old {}, outgoing {}, channel_id {}", post_channel_value, pre_channel_value, false, self.context.channel_id); let splice_ack_msg = self.get_splice_ack(our_funding_contribution); - let pending_splice = self.pending_splice.as_mut().unwrap(); - let mut interactive_tx_constructor = begin_interactive_funding_tx_construction( - pending_splice.funding.as_ref().unwrap(), - &self.context, - pending_splice.funding_negotiation_context.take().unwrap(), - true, - signer_provider, - entropy_source, - holder_node_id.clone(), - None, - Some(prev_funding_input), - ) - .map_err(|err| { - ChannelError::Warn(format!( - "Failed to start interactive transaction construction, {:?}", - err - )) - })?; - + let mut interactive_tx_constructor = funding_negotiation_context + .into_interactive_tx_constructor( + &self.context, + &splice_funding, + true, + signer_provider, + entropy_source, + holder_node_id.clone(), + None, + Some(prev_funding_input), + ) + .map_err(|err| { + ChannelError::Warn(format!( + "Failed to start interactive transaction construction, {:?}", + err + )) + })?; // TODO: Propagate message let _msg = interactive_tx_constructor.take_initiator_first_message(); - self.pending_splice.as_mut().unwrap().interactive_tx_constructor = - Some(interactive_tx_constructor); + + self.pending_splice = Some(PendingSplice { + our_funding_contribution, + funding: Some(splice_funding), + funding_negotiation_context: None, + interactive_tx_constructor: Some(interactive_tx_constructor), + received_funding_txid: None, + sent_funding_txid: None, + }); Ok(splice_ack_msg) } @@ -10654,7 +10648,7 @@ where // TODO(splicing): Pre-check for reserve requirement // (Note: It should also be checked later at tx_complete) - let funding_scope = FundingScope::for_splice( + let splice_funding = FundingScope::for_splice( &self.funding, &self.context, our_funding_contribution, @@ -10671,37 +10665,39 @@ where let prev_funding_input = self.funding.to_splice_funding_input(); - debug_assert!(pending_splice.funding.is_none()); - pending_splice.funding = Some(funding_scope); // update funding values pending_splice.funding_negotiation_context.as_mut().unwrap().our_funding_satoshis = our_funding_satoshis; pending_splice.funding_negotiation_context.as_mut().unwrap().their_funding_satoshis = Some(their_funding_satoshis); - debug_assert!(pending_splice.interactive_tx_constructor.is_none()); - debug_assert!(self.interactive_tx_signing_session.is_none()); log_info!(logger, "Splicing process started after splice_ack, new channel value {}, old {}, outgoing {}, channel_id {}", post_channel_value, pre_channel_value, true, self.context.channel_id); // Start interactive funding negotiation, with the previous funding transaction as an extra shared input - let mut interactive_tx_constructor = begin_interactive_funding_tx_construction( - pending_splice.funding.as_ref().unwrap(), - &self.context, - pending_splice.funding_negotiation_context.take().unwrap(), - true, - signer_provider, - entropy_source, - holder_node_id.clone(), - None, - Some(prev_funding_input), - ) - .map_err(|err| { - ChannelError::Warn(format!("V2 channel rejected due to sender error, {:?}", err)) - })?; + let funding_negotiation_context = pending_splice.funding_negotiation_context.take().unwrap(); + let mut interactive_tx_constructor = funding_negotiation_context + .into_interactive_tx_constructor( + &self.context, + &splice_funding, + true, + signer_provider, + entropy_source, + holder_node_id.clone(), + None, + Some(prev_funding_input), + ) + .map_err(|err| { + ChannelError::Warn(format!("V2 channel rejected due to sender error, {:?}", err)) + })?; let tx_msg_opt = interactive_tx_constructor.take_initiator_first_message(); - self.pending_splice.as_mut().unwrap().interactive_tx_constructor = - Some(interactive_tx_constructor); + + debug_assert!(pending_splice.funding.is_none()); + debug_assert!(pending_splice.interactive_tx_constructor.is_none()); + debug_assert!(self.interactive_tx_signing_session.is_none()); + pending_splice.funding = Some(splice_funding); + pending_splice.interactive_tx_constructor = Some(interactive_tx_constructor); + Ok(tx_msg_opt) } From 44020e76a1d7a444db7cfe2c6fa93b5d572c55e9 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 17 Jul 2025 17:40:24 -0500 Subject: [PATCH 16/22] f - Add FundingNegotiation enum --- lightning/src/ln/channel.rs | 58 ++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 73118be5a3d..74f6ffe81ae 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1883,7 +1883,8 @@ where ChannelPhase::Funded(mut funded_channel) => { #[cfg(splicing)] let has_negotiated_pending_splice = funded_channel.pending_splice.as_ref() - .map(|pending_splice| pending_splice.funding.is_some()) + .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()) + .map(|funding_negotiation| funding_negotiation.as_funding().is_some()) .unwrap_or(false); #[cfg(splicing)] let session_received_commitment_signed = funded_channel @@ -2330,8 +2331,7 @@ impl FundingScope { struct PendingSplice { /// Intended contributions to the splice from our end pub our_funding_contribution: i64, - funding: Option, - funding_negotiation_context: Option, + funding_negotiation: Option, /// The current interactive transaction construction session under negotiation. interactive_tx_constructor: Option, @@ -2342,6 +2342,20 @@ struct PendingSplice { received_funding_txid: Option, } +enum FundingNegotiation { + AwaitingAck(FundingNegotiationContext), + Pending(FundingScope), +} + +impl FundingNegotiation { + fn as_funding(&self) -> Option<&FundingScope> { + match self { + FundingNegotiation::AwaitingAck(_) => None, + FundingNegotiation::Pending(funding) => Some(funding), + } + } +} + #[cfg(splicing)] impl PendingSplice { fn check_get_splice_locked( @@ -6884,7 +6898,8 @@ where let pending_splice_funding = self .pending_splice .as_ref() - .and_then(|pending_splice| pending_splice.funding.as_ref()) + .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()) + .and_then(|funding_negotiation| funding_negotiation.as_funding()) .expect("Funding must exist for negotiated pending splice"); let (holder_commitment_tx, _) = self.context.validate_commitment_signed( pending_splice_funding, @@ -10416,8 +10431,7 @@ where }; self.pending_splice = Some(PendingSplice { our_funding_contribution: our_funding_contribution_satoshis, - funding: None, - funding_negotiation_context: Some(funding_negotiation_context), + funding_negotiation: Some(FundingNegotiation::AwaitingAck(funding_negotiation_context)), interactive_tx_constructor: None, sent_funding_txid: None, received_funding_txid: None, @@ -10581,8 +10595,7 @@ where self.pending_splice = Some(PendingSplice { our_funding_contribution, - funding: Some(splice_funding), - funding_negotiation_context: None, + funding_negotiation: Some(FundingNegotiation::Pending(splice_funding)), interactive_tx_constructor: Some(interactive_tx_constructor), received_funding_txid: None, sent_funding_txid: None, @@ -10625,11 +10638,20 @@ where // TODO(splicing): Add check that we are the splice (quiescence) initiator - if pending_splice.funding.is_some() || pending_splice.interactive_tx_constructor.is_some() { - return Err(ChannelError::Warn(format!( - "Got unexpected splice_ack, splice already negotiating" - ))); - } + let mut funding_negotiation_context = match pending_splice.funding_negotiation.take() { + Some(FundingNegotiation::AwaitingAck(context)) => context, + Some(FundingNegotiation::Pending(funding)) => { + pending_splice.funding_negotiation = Some(FundingNegotiation::Pending(funding)); + return Err(ChannelError::Warn(format!( + "Got unexpected splice_ack; splice negotiation already in progress" + ))); + }, + None => { + return Err(ChannelError::Warn(format!( + "Got unexpected splice_ack; no splice negotiation in progress" + ))); + }, + }; let our_funding_contribution = pending_splice.our_funding_contribution; let their_funding_contribution_satoshis = msg.funding_contribution_satoshis; @@ -10666,16 +10688,13 @@ where let prev_funding_input = self.funding.to_splice_funding_input(); // update funding values - pending_splice.funding_negotiation_context.as_mut().unwrap().our_funding_satoshis = - our_funding_satoshis; - pending_splice.funding_negotiation_context.as_mut().unwrap().their_funding_satoshis = - Some(their_funding_satoshis); + funding_negotiation_context.our_funding_satoshis = our_funding_satoshis; + funding_negotiation_context.their_funding_satoshis = Some(their_funding_satoshis); log_info!(logger, "Splicing process started after splice_ack, new channel value {}, old {}, outgoing {}, channel_id {}", post_channel_value, pre_channel_value, true, self.context.channel_id); // Start interactive funding negotiation, with the previous funding transaction as an extra shared input - let funding_negotiation_context = pending_splice.funding_negotiation_context.take().unwrap(); let mut interactive_tx_constructor = funding_negotiation_context .into_interactive_tx_constructor( &self.context, @@ -10692,10 +10711,9 @@ where })?; let tx_msg_opt = interactive_tx_constructor.take_initiator_first_message(); - debug_assert!(pending_splice.funding.is_none()); debug_assert!(pending_splice.interactive_tx_constructor.is_none()); debug_assert!(self.interactive_tx_signing_session.is_none()); - pending_splice.funding = Some(splice_funding); + pending_splice.funding_negotiation = Some(FundingNegotiation::Pending(splice_funding)); pending_splice.interactive_tx_constructor = Some(interactive_tx_constructor); Ok(tx_msg_opt) From a64dd9a5e0bb936ebbe79d92a0d562706a51e0de Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 17 Jul 2025 21:29:32 -0500 Subject: [PATCH 17/22] f - add FundingNegotiation::AwaitingSignatures --- lightning/src/ln/channel.rs | 161 ++++++++++++++++++++++-------------- 1 file changed, 97 insertions(+), 64 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 74f6ffe81ae..c92798fd74d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1782,57 +1782,74 @@ where L::Target: Logger, { let logger = WithChannelContext::from(logger, self.context(), None); - let (funding, context, signing_session, is_splice, holder_commitment_transaction_number) = - // TODO: Could use a wrapper or trait to DRY this - match &mut self.phase { - ChannelPhase::UnfundedV2(chan) => { - chan.interactive_tx_signing_session = Some( - chan.interactive_tx_constructor - .take() - .expect("TODO") - .into_signing_session(), - ); - ( - &mut chan.funding, - &mut chan.context, - chan.interactive_tx_signing_session.as_mut().expect("TODO"), - false, - chan.unfunded_context.transaction_number(), - ) - }, - #[cfg(splicing)] - ChannelPhase::Funded(chan) => { - chan.interactive_tx_signing_session = Some( - chan.pending_splice - .as_mut() - .expect("TODO") - .interactive_tx_constructor - .take() - .expect("TODO") - .into_signing_session(), - ); - ( - &mut chan.funding, - &mut chan.context, - chan.interactive_tx_signing_session.as_mut().expect("TODO"), - true, - chan.holder_commitment_point.transaction_number(), - ) - }, - _ => { + match &mut self.phase { + ChannelPhase::UnfundedV2(chan) => { + let mut signing_session = + chan.interactive_tx_constructor.take().expect("TODO").into_signing_session(); + let result = funding_tx_constructed( + &mut chan.funding, + &mut chan.context, + &mut signing_session, + false, + chan.unfunded_context.transaction_number(), + &&logger, + ); + + // FIXME: Should this remain None if result is an Err? + chan.interactive_tx_signing_session = Some(signing_session); + + return result; + }, + #[cfg(splicing)] + ChannelPhase::Funded(chan) => { + if let Some(pending_splice) = chan.pending_splice.as_mut() { + if let Some(funding_negotiation) = pending_splice.funding_negotiation.take() { + if let FundingNegotiation::Pending( + mut funding, + interactive_tx_constructor, + ) = funding_negotiation + { + let mut signing_session = + interactive_tx_constructor.into_signing_session(); + let result = funding_tx_constructed( + &mut funding, + &mut chan.context, + &mut signing_session, + true, + chan.holder_commitment_point.transaction_number(), + &&logger, + ); + + // FIXME: Should these remain None if result is an Err? + chan.interactive_tx_signing_session = Some(signing_session); + pending_splice.funding_negotiation = + Some(FundingNegotiation::AwaitingSignatures(funding)); + + return result; + } else { + pending_splice.funding_negotiation = Some(funding_negotiation); + return Err(ChannelError::Warn( + "Got a transaction negotiation message in an invalid state" + .to_owned(), + )); + } + } else { + return Err(ChannelError::Warn( + "Got a transaction negotiation message in an invalid state".to_owned(), + )); + } + } else { return Err(ChannelError::Warn( - "Got a transaction negotiation message in an invalid phase".to_owned(), - )) - }, - }; - funding_tx_constructed( - funding, - context, - signing_session, - is_splice, - holder_commitment_transaction_number, - &&logger, - ) + "Got a transaction negotiation message in an invalid state".to_owned(), + )); + } + }, + _ => { + return Err(ChannelError::Warn( + "Got a transaction negotiation message in an invalid phase".to_owned(), + )) + }, + } } pub fn force_shutdown(&mut self, closure_reason: ClosureReason) -> ShutdownResult { @@ -2332,8 +2349,6 @@ struct PendingSplice { /// Intended contributions to the splice from our end pub our_funding_contribution: i64, funding_negotiation: Option, - /// The current interactive transaction construction session under negotiation. - interactive_tx_constructor: Option, /// The funding txid used in the `splice_locked` sent to the counterparty. sent_funding_txid: Option, @@ -2344,14 +2359,16 @@ struct PendingSplice { enum FundingNegotiation { AwaitingAck(FundingNegotiationContext), - Pending(FundingScope), + Pending(FundingScope, InteractiveTxConstructor), + AwaitingSignatures(FundingScope), } impl FundingNegotiation { fn as_funding(&self) -> Option<&FundingScope> { match self { FundingNegotiation::AwaitingAck(_) => None, - FundingNegotiation::Pending(funding) => Some(funding), + FundingNegotiation::Pending(funding, _) => Some(funding), + FundingNegotiation::AwaitingSignatures(funding) => Some(funding), } } } @@ -6173,8 +6190,16 @@ where fn as_renegotiating_channel(&mut self) -> Option { self.pending_splice .as_mut() - .and_then(|pending_splice| pending_splice.interactive_tx_constructor.as_mut()) - .map(|interactive_tx_constructor| NegotiatingChannelView(interactive_tx_constructor)) + .and_then(|pending_splice| pending_splice.funding_negotiation.as_mut()) + .and_then(|funding_negotiation| { + if let FundingNegotiation::Pending(_, interactive_tx_constructor) = + funding_negotiation + { + Some(NegotiatingChannelView(interactive_tx_constructor)) + } else { + None + } + }) } #[rustfmt::skip] @@ -10432,7 +10457,6 @@ where self.pending_splice = Some(PendingSplice { our_funding_contribution: our_funding_contribution_satoshis, funding_negotiation: Some(FundingNegotiation::AwaitingAck(funding_negotiation_context)), - interactive_tx_constructor: None, sent_funding_txid: None, received_funding_txid: None, }); @@ -10595,8 +10619,10 @@ where self.pending_splice = Some(PendingSplice { our_funding_contribution, - funding_negotiation: Some(FundingNegotiation::Pending(splice_funding)), - interactive_tx_constructor: Some(interactive_tx_constructor), + funding_negotiation: Some(FundingNegotiation::Pending( + splice_funding, + interactive_tx_constructor, + )), received_funding_txid: None, sent_funding_txid: None, }); @@ -10640,8 +10666,16 @@ where let mut funding_negotiation_context = match pending_splice.funding_negotiation.take() { Some(FundingNegotiation::AwaitingAck(context)) => context, - Some(FundingNegotiation::Pending(funding)) => { - pending_splice.funding_negotiation = Some(FundingNegotiation::Pending(funding)); + Some(FundingNegotiation::Pending(funding, constructor)) => { + pending_splice.funding_negotiation = + Some(FundingNegotiation::Pending(funding, constructor)); + return Err(ChannelError::Warn(format!( + "Got unexpected splice_ack; splice negotiation already in progress" + ))); + }, + Some(FundingNegotiation::AwaitingSignatures(funding)) => { + pending_splice.funding_negotiation = + Some(FundingNegotiation::AwaitingSignatures(funding)); return Err(ChannelError::Warn(format!( "Got unexpected splice_ack; splice negotiation already in progress" ))); @@ -10711,10 +10745,9 @@ where })?; let tx_msg_opt = interactive_tx_constructor.take_initiator_first_message(); - debug_assert!(pending_splice.interactive_tx_constructor.is_none()); debug_assert!(self.interactive_tx_signing_session.is_none()); - pending_splice.funding_negotiation = Some(FundingNegotiation::Pending(splice_funding)); - pending_splice.interactive_tx_constructor = Some(interactive_tx_constructor); + pending_splice.funding_negotiation = + Some(FundingNegotiation::Pending(splice_funding, interactive_tx_constructor)); Ok(tx_msg_opt) } From dca1c3c3be42da291b0bd25dd47cbfc98719cc26 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 18 Jul 2025 09:15:06 -0500 Subject: [PATCH 18/22] f - simplify error handling --- lightning/src/ln/channel.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index c92798fd74d..0abc6a753e8 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1827,22 +1827,15 @@ where return result; } else { + // Replace the taken state pending_splice.funding_negotiation = Some(funding_negotiation); - return Err(ChannelError::Warn( - "Got a transaction negotiation message in an invalid state" - .to_owned(), - )); } - } else { - return Err(ChannelError::Warn( - "Got a transaction negotiation message in an invalid state".to_owned(), - )); } - } else { - return Err(ChannelError::Warn( - "Got a transaction negotiation message in an invalid state".to_owned(), - )); } + + return Err(ChannelError::Warn( + "Got a transaction negotiation message in an invalid state".to_owned(), + )); }, _ => { return Err(ChannelError::Warn( From d1dfa291e00dd7812a779bbd32e922de41dc5bfe Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 18 Jul 2025 09:25:28 -0500 Subject: [PATCH 19/22] f - move funding_tx_constructed to ChannelContext --- lightning/src/ln/channel.rs | 190 ++++++++++++++++++------------------ 1 file changed, 93 insertions(+), 97 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 0abc6a753e8..824bfa2da51 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1786,9 +1786,8 @@ where ChannelPhase::UnfundedV2(chan) => { let mut signing_session = chan.interactive_tx_constructor.take().expect("TODO").into_signing_session(); - let result = funding_tx_constructed( + let result = chan.context.funding_tx_constructed( &mut chan.funding, - &mut chan.context, &mut signing_session, false, chan.unfunded_context.transaction_number(), @@ -1811,9 +1810,8 @@ where { let mut signing_session = interactive_tx_constructor.into_signing_session(); - let result = funding_tx_constructed( + let result = chan.context.funding_tx_constructed( &mut funding, - &mut chan.context, &mut signing_session, true, chan.holder_commitment_point.transaction_number(), @@ -2966,99 +2964,6 @@ where /// - [`FundedChannel`], when splicing. pub(super) struct NegotiatingChannelView<'a>(&'a mut InteractiveTxConstructor); -#[rustfmt::skip] -fn funding_tx_constructed( - funding: &mut FundingScope, context: &mut ChannelContext, - signing_session: &mut InteractiveTxSigningSession, is_splice: bool, - holder_commitment_transaction_number: u64, logger: &L -) -> Result<(msgs::CommitmentSigned, Option), ChannelError> -where - SP::Target: SignerProvider, - L::Target: Logger -{ - let mut output_index = None; - let expected_spk = funding.get_funding_redeemscript().to_p2wsh(); - for (idx, outp) in signing_session.unsigned_tx().outputs().enumerate() { - if outp.script_pubkey() == &expected_spk && outp.value() == funding.get_value_satoshis() { - if output_index.is_some() { - let msg = "Multiple outputs matched the expected script and value"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); - } - output_index = Some(idx as u16); - } - } - let outpoint = if let Some(output_index) = output_index { - OutPoint { txid: signing_session.unsigned_tx().compute_txid(), index: output_index } - } else { - let msg = "No output matched the funding script_pubkey"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); - }; - funding - .channel_transaction_parameters.funding_outpoint = Some(outpoint); - - if is_splice { - let message = "TODO Forced error, incomplete implementation".to_owned(); - // TODO(splicing) Forced error, as the use case is not complete - return Err(ChannelError::Close(( - message.clone(), - ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false), message } - ))); - } - - context.assert_no_commitment_advancement(holder_commitment_transaction_number, "initial commitment_signed"); - let commitment_signed = context.get_initial_commitment_signed(&funding, logger); - let commitment_signed = match commitment_signed { - Ok(commitment_signed) => commitment_signed, - Err(e) => { - funding.channel_transaction_parameters.funding_outpoint = None; - return Err(e) - }, - }; - - let funding_ready_for_sig_event = if signing_session.local_inputs_count() == 0 { - if signing_session.provide_holder_witnesses(context.channel_id, Vec::new()).is_err() { - debug_assert!( - false, - "Zero inputs were provided & zero witnesses were provided, but a count mismatch was somehow found", - ); - let msg = "V2 channel rejected due to sender error"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); - } - None - } else { - // TODO(dual_funding): Send event for signing if we've contributed funds. - // Inform the user that SIGHASH_ALL must be used for all signatures when contributing - // inputs/signatures. - // Also warn the user that we don't do anything to prevent the counterparty from - // providing non-standard witnesses which will prevent the funding transaction from - // confirming. This warning must appear in doc comments wherever the user is contributing - // funds, whether they are initiator or acceptor. - // - // The following warning can be used when the APIs allowing contributing inputs become available: - //
- // WARNING: LDK makes no attempt to prevent the counterparty from using non-standard inputs which - // will prevent the funding transaction from being relayed on the bitcoin network and hence being - // confirmed. - //
- debug_assert!( - false, - "We don't support users providing inputs but somehow we had more than zero inputs", - ); - let msg = "V2 channel rejected due to sender error"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); - }; - - let mut channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); - channel_state.set_interactive_signing(); - context.channel_state = channel_state; - - Ok((commitment_signed, funding_ready_for_sig_event)) -} - impl<'a> NegotiatingChannelView<'a> { pub(super) fn tx_add_input( &mut self, msg: &msgs::TxAddInput, @@ -5575,6 +5480,97 @@ where Ok(()) } + #[rustfmt::skip] + fn funding_tx_constructed( + &mut self, funding: &mut FundingScope, signing_session: &mut InteractiveTxSigningSession, + is_splice: bool, holder_commitment_transaction_number: u64, logger: &L + ) -> Result<(msgs::CommitmentSigned, Option), ChannelError> + where + L::Target: Logger + { + let mut output_index = None; + let expected_spk = funding.get_funding_redeemscript().to_p2wsh(); + for (idx, outp) in signing_session.unsigned_tx().outputs().enumerate() { + if outp.script_pubkey() == &expected_spk && outp.value() == funding.get_value_satoshis() { + if output_index.is_some() { + let msg = "Multiple outputs matched the expected script and value"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); + } + output_index = Some(idx as u16); + } + } + let outpoint = if let Some(output_index) = output_index { + OutPoint { txid: signing_session.unsigned_tx().compute_txid(), index: output_index } + } else { + let msg = "No output matched the funding script_pubkey"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); + }; + funding + .channel_transaction_parameters.funding_outpoint = Some(outpoint); + + if is_splice { + let message = "TODO Forced error, incomplete implementation".to_owned(); + // TODO(splicing) Forced error, as the use case is not complete + return Err(ChannelError::Close(( + message.clone(), + ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false), message } + ))); + } + + self.assert_no_commitment_advancement(holder_commitment_transaction_number, "initial commitment_signed"); + let commitment_signed = self.get_initial_commitment_signed(&funding, logger); + let commitment_signed = match commitment_signed { + Ok(commitment_signed) => commitment_signed, + Err(e) => { + funding.channel_transaction_parameters.funding_outpoint = None; + return Err(e) + }, + }; + + let funding_ready_for_sig_event = if signing_session.local_inputs_count() == 0 { + if signing_session.provide_holder_witnesses(self.channel_id, Vec::new()).is_err() { + debug_assert!( + false, + "Zero inputs were provided & zero witnesses were provided, but a count mismatch was somehow found", + ); + let msg = "V2 channel rejected due to sender error"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); + } + None + } else { + // TODO(dual_funding): Send event for signing if we've contributed funds. + // Inform the user that SIGHASH_ALL must be used for all signatures when contributing + // inputs/signatures. + // Also warn the user that we don't do anything to prevent the counterparty from + // providing non-standard witnesses which will prevent the funding transaction from + // confirming. This warning must appear in doc comments wherever the user is contributing + // funds, whether they are initiator or acceptor. + // + // The following warning can be used when the APIs allowing contributing inputs become available: + //
+ // WARNING: LDK makes no attempt to prevent the counterparty from using non-standard inputs which + // will prevent the funding transaction from being relayed on the bitcoin network and hence being + // confirmed. + //
+ debug_assert!( + false, + "We don't support users providing inputs but somehow we had more than zero inputs", + ); + let msg = "V2 channel rejected due to sender error"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); + }; + + let mut channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); + channel_state.set_interactive_signing(); + self.channel_state = channel_state; + + Ok((commitment_signed, funding_ready_for_sig_event)) + } + /// Asserts that the commitment tx numbers have not advanced from their initial number. #[rustfmt::skip] fn assert_no_commitment_advancement(&self, holder_commitment_transaction_number: u64, msg_name: &str) { From 2eeb7956845f9aff38e4dd7431315d419d26c225 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 18 Jul 2025 10:27:04 -0500 Subject: [PATCH 20/22] f - remove NegotiatingChannelView --- lightning/src/ln/channel.rs | 79 +++--------------------------- lightning/src/ln/channelmanager.rs | 58 ++++++++++++++-------- 2 files changed, 46 insertions(+), 91 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 824bfa2da51..31b72f5ce10 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -58,10 +58,9 @@ use crate::ln::channelmanager::{ BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, }; use crate::ln::interactivetxs::{ - calculate_change_output_value, get_output_weight, AbortReason, HandleTxCompleteResult, - InteractiveTxConstructor, InteractiveTxConstructorArgs, InteractiveTxMessageSend, - InteractiveTxMessageSendResult, InteractiveTxSigningSession, SharedOwnedInput, - SharedOwnedOutput, TX_COMMON_FIELDS_WEIGHT, + calculate_change_output_value, get_output_weight, AbortReason, InteractiveTxConstructor, + InteractiveTxConstructorArgs, InteractiveTxMessageSend, InteractiveTxSigningSession, + SharedOwnedInput, SharedOwnedOutput, TX_COMMON_FIELDS_WEIGHT, }; use crate::ln::msgs; use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError, OnionErrorPacket}; @@ -1734,11 +1733,11 @@ where } } - pub fn as_negotiating_channel(&mut self) -> Option { + pub fn interactive_tx_constructor_mut(&mut self) -> Option<&mut InteractiveTxConstructor> { match &mut self.phase { - ChannelPhase::UnfundedV2(chan) => Some(chan.as_negotiating_channel()), + ChannelPhase::UnfundedV2(chan) => chan.interactive_tx_constructor.as_mut(), #[cfg(splicing)] - ChannelPhase::Funded(chan) => chan.as_renegotiating_channel(), + ChannelPhase::Funded(chan) => chan.interactive_tx_constructor_mut(), _ => None, } } @@ -2958,62 +2957,6 @@ where } } -/// A short-lived subset view of a channel, used for V2 funding negotiation or re-negotiation. -/// Can be produced by: -/// - [`PendingV2Channel`], at V2 channel open, and -/// - [`FundedChannel`], when splicing. -pub(super) struct NegotiatingChannelView<'a>(&'a mut InteractiveTxConstructor); - -impl<'a> NegotiatingChannelView<'a> { - pub(super) fn tx_add_input( - &mut self, msg: &msgs::TxAddInput, - ) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult( - self.0 - .handle_tx_add_input(msg) - .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), - ) - } - - pub(super) fn tx_add_output( - &mut self, msg: &msgs::TxAddOutput, - ) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult( - self.0 - .handle_tx_add_output(msg) - .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), - ) - } - - pub(super) fn tx_remove_input( - &mut self, msg: &msgs::TxRemoveInput, - ) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult( - self.0 - .handle_tx_remove_input(msg) - .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), - ) - } - - pub(super) fn tx_remove_output( - &mut self, msg: &msgs::TxRemoveOutput, - ) -> InteractiveTxMessageSendResult { - InteractiveTxMessageSendResult( - self.0 - .handle_tx_remove_output(msg) - .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), - ) - } - - pub(super) fn tx_complete(&mut self, msg: &msgs::TxComplete) -> HandleTxCompleteResult { - HandleTxCompleteResult( - self.0 - .handle_tx_complete(msg) - .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), - ) - } -} - impl ChannelContext where SP::Target: SignerProvider, @@ -6174,9 +6117,8 @@ where self.context.force_shutdown(&self.funding, closure_reason) } - /// If we are in splicing/refunding, return a short-lived [`NegotiatingChannelView`]. #[cfg(splicing)] - fn as_renegotiating_channel(&mut self) -> Option { + fn interactive_tx_constructor_mut(&mut self) -> Option<&mut InteractiveTxConstructor> { self.pending_splice .as_mut() .and_then(|pending_splice| pending_splice.funding_negotiation.as_mut()) @@ -6184,7 +6126,7 @@ where if let FundingNegotiation::Pending(_, interactive_tx_constructor) = funding_negotiation { - Some(NegotiatingChannelView(interactive_tx_constructor)) + Some(interactive_tx_constructor) } else { None } @@ -12610,11 +12552,6 @@ where pub fn get_accept_channel_v2_message(&self) -> msgs::AcceptChannelV2 { self.generate_accept_channel_v2_message() } - - /// Return a short-lived [`NegotiatingChannelView`]. - fn as_negotiating_channel(&mut self) -> NegotiatingChannelView { - NegotiatingChannelView(self.interactive_tx_constructor.as_mut().expect("TODO")) - } } // Unfunded channel utilities diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 07159aacd9a..bffceb79b67 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -66,6 +66,7 @@ use crate::ln::channel::{ }; use crate::ln::channel_state::ChannelDetails; use crate::ln::inbound_payment; +use crate::ln::interactivetxs::{HandleTxCompleteResult, InteractiveTxMessageSendResult}; use crate::ln::msgs; use crate::ln::msgs::{ BaseMessageHandler, ChannelMessageHandler, CommitmentUpdate, DecodeError, LightningError, @@ -8938,10 +8939,13 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ ) -> Result<(), MsgHandleErrInternal> { self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel| { Some( - channel - .as_negotiating_channel()? - .tx_add_input(msg) - .into_msg_send_event(counterparty_node_id), + InteractiveTxMessageSendResult( + channel + .interactive_tx_constructor_mut()? + .handle_tx_add_input(msg) + .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), + ) + .into_msg_send_event(counterparty_node_id), ) }) } @@ -8951,10 +8955,13 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ ) -> Result<(), MsgHandleErrInternal> { self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel| { Some( - channel - .as_negotiating_channel()? - .tx_add_output(msg) - .into_msg_send_event(counterparty_node_id), + InteractiveTxMessageSendResult( + channel + .interactive_tx_constructor_mut()? + .handle_tx_add_output(msg) + .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), + ) + .into_msg_send_event(counterparty_node_id), ) }) } @@ -8964,10 +8971,13 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ ) -> Result<(), MsgHandleErrInternal> { self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel| { Some( - channel - .as_negotiating_channel()? - .tx_remove_input(msg) - .into_msg_send_event(counterparty_node_id), + InteractiveTxMessageSendResult( + channel + .interactive_tx_constructor_mut()? + .handle_tx_remove_input(msg) + .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), + ) + .into_msg_send_event(counterparty_node_id), ) }) } @@ -8977,10 +8987,13 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ ) -> Result<(), MsgHandleErrInternal> { self.internal_tx_msg(&counterparty_node_id, msg.channel_id, |channel: &mut Channel| { Some( - channel - .as_negotiating_channel()? - .tx_remove_output(msg) - .into_msg_send_event(counterparty_node_id), + InteractiveTxMessageSendResult( + channel + .interactive_tx_constructor_mut()? + .handle_tx_remove_output(msg) + .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), + ) + .into_msg_send_event(counterparty_node_id), ) }) } @@ -8999,10 +9012,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ let peer_state = &mut *peer_state_lock; match peer_state.channel_by_id.entry(msg.channel_id) { hash_map::Entry::Occupied(mut chan_entry) => { - let (msg_send_event_opt, ready_to_sign) = match chan_entry.get_mut().as_negotiating_channel() { - Some(mut negotiating_channel) => negotiating_channel - .tx_complete(msg) - .into_msg_send_event(counterparty_node_id), + let (msg_send_event_opt, ready_to_sign) = match chan_entry.get_mut().interactive_tx_constructor_mut() { + Some(interactive_tx_constructor) => { + HandleTxCompleteResult( + interactive_tx_constructor + .handle_tx_complete(msg) + .map_err(|reason| reason.into_tx_abort_msg(msg.channel_id)), + ) + .into_msg_send_event(counterparty_node_id) + }, None => { let err = ChannelError::Warn("Received unexpected tx_complete message".to_owned()); return Err(MsgHandleErrInternal::from_chan_no_close(err, msg.channel_id)) From 05a6c853f12083bfc235419b5858d10898db01b9 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 18 Jul 2025 13:08:20 -0500 Subject: [PATCH 21/22] f - drop is_splice parameter --- lightning/src/ln/channel.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 31b72f5ce10..12fec2c14a9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5919,15 +5919,15 @@ impl FundingNegotiationContext { /// default destination address is used. /// If error occurs, it is caused by our side, not the counterparty. fn into_interactive_tx_constructor( - self, context: &ChannelContext, funding: &FundingScope, is_splice: bool, - signer_provider: &SP, entropy_source: &ES, holder_node_id: PublicKey, - change_destination_opt: Option, shared_funding_input: Option, + self, context: &ChannelContext, funding: &FundingScope, signer_provider: &SP, + entropy_source: &ES, holder_node_id: PublicKey, change_destination_opt: Option, + shared_funding_input: Option, ) -> Result where SP::Target: SignerProvider, ES::Target: EntropySource, { - if is_splice { + if shared_funding_input.is_some() { debug_assert!(matches!(context.channel_state, ChannelState::ChannelReady(_))); } else { debug_assert!(matches!(context.channel_state, ChannelState::NegotiatingFunding(_))); @@ -10532,7 +10532,6 @@ where .into_interactive_tx_constructor( &self.context, &splice_funding, - true, signer_provider, entropy_source, holder_node_id.clone(), @@ -10664,7 +10663,6 @@ where .into_interactive_tx_constructor( &self.context, &splice_funding, - true, signer_provider, entropy_source, holder_node_id.clone(), From 3b3124002ab945e70a3acdc77bf6ff432b3bff61 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Sat, 19 Jul 2025 07:03:49 +0200 Subject: [PATCH 22/22] f Fix splice test by handling any input&output order --- lightning/src/ln/splicing_tests.rs | 67 +++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 20 deletions(-) diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 602f24471f5..a2739f42391 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -28,6 +28,7 @@ fn test_v1_splice_in() { let channel_value_sat = 100_000; let channel_reserve_amnt_sat = 1_000; + let expect_outputs_in_reverse = true; let (_, _, channel_id, _) = create_announced_chan_between_nodes_with_value( &nodes, @@ -142,17 +143,24 @@ fn test_v1_splice_in() { acceptor_node.node.get_our_node_id() ); dbg!(&tx_add_input_msg); - // check which input is this - let inputs_seen_in_reverse = if let Some(prevtx) = tx_add_input_msg.prevtx.as_ref() { - let value = prevtx - .as_transaction() - .output[tx_add_input_msg.prevtx_out as usize] + // check which input is this (order is non-deterministic), based on the presense of prevtx + let inputs_seen_in_reverse = tx_add_input_msg.prevtx.is_some(); + if !inputs_seen_in_reverse { + // Input is the revious funding input + assert_eq!(tx_add_input_msg.prevtx, None); + assert_eq!( + tx_add_input_msg.shared_input_txid.unwrap().to_string(), + "4f128bedf1a15baf465ab1bfd6e97c8f82628f4156bf86eb1cbc132cda6733ae" + ); + } else { + // Input is the extra input + let prevtx_value = tx_add_input_msg.prevtx.as_ref().unwrap().as_transaction().output + [tx_add_input_msg.prevtx_out as usize] .value .to_sat(); - value == extra_splice_funding_input_sats - } else { - false - }; + assert_eq!(prevtx_value, extra_splice_funding_input_sats); + assert_eq!(tx_add_input_msg.shared_input_txid, None); + } let _res = acceptor_node .node @@ -167,17 +175,28 @@ fn test_v1_splice_in() { .node .handle_tx_complete(acceptor_node.node.get_our_node_id(), &tx_complete_msg); // Second input - let exp_value = - if inputs_seen_in_reverse { channel_value_sat } else { extra_splice_funding_input_sats }; let tx_add_input2_msg = get_event_msg!( &initiator_node, MessageSendEvent::SendTxAddInput, acceptor_node.node.get_our_node_id() ); dbg!(&tx_add_input2_msg); - // FIXME: Determine why these fail - //assert_eq!(tx_add_input2_msg.prevtx, None); - //assert_eq!(tx_add_input2_msg.shared_input_txid.unwrap().to_string(), "4f128bedf1a15baf465ab1bfd6e97c8f82628f4156bf86eb1cbc132cda6733ae"); + if !inputs_seen_in_reverse { + // Input is the extra input + let prevtx_value = tx_add_input2_msg.prevtx.as_ref().unwrap().as_transaction().output + [tx_add_input2_msg.prevtx_out as usize] + .value + .to_sat(); + assert_eq!(prevtx_value, extra_splice_funding_input_sats); + assert_eq!(tx_add_input2_msg.shared_input_txid, None); + } else { + // Input is the revious funding input + assert_eq!(tx_add_input2_msg.prevtx, None); + assert_eq!( + tx_add_input2_msg.shared_input_txid.unwrap().to_string(), + "4f128bedf1a15baf465ab1bfd6e97c8f82628f4156bf86eb1cbc132cda6733ae" + ); + } let _res = acceptor_node .node @@ -199,9 +218,13 @@ fn test_v1_splice_in() { acceptor_node.node.get_our_node_id() ); dbg!(&tx_add_output_msg); - // FIXME: Determine why these fail - //assert!(tx_add_output_msg.script.is_p2wsh()); - //assert_eq!(tx_add_output_msg.sats, post_splice_channel_value); + if !expect_outputs_in_reverse { + assert!(tx_add_output_msg.script.is_p2wsh()); + assert_eq!(tx_add_output_msg.sats, post_splice_channel_value); + } else { + assert!(tx_add_output_msg.script.is_p2wpkh()); + assert_eq!(tx_add_output_msg.sats, 14146); // extra_splice_funding_input_sats - splice_in_sats + } let _res = acceptor_node .node @@ -222,9 +245,13 @@ fn test_v1_splice_in() { acceptor_node.node.get_our_node_id() ); dbg!(&tx_add_output2_msg); - // FIXME: Determine why these fail - //assert!(tx_add_output2_msg.script.is_p2wpkh()); - //assert_eq!(tx_add_output2_msg.sats, 14094); // extra_splice_input_sats - splice_in_sats + if !expect_outputs_in_reverse { + assert!(tx_add_output2_msg.script.is_p2wpkh()); + assert_eq!(tx_add_output2_msg.sats, 14146); // extra_splice_funding_input_sats - splice_in_sats + } else { + assert!(tx_add_output2_msg.script.is_p2wsh()); + assert_eq!(tx_add_output2_msg.sats, post_splice_channel_value); + } let _res = acceptor_node .node