From c80afe9e8e07fd7dee7b1b157b9dd7f7104ddde9 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 13 Jan 2026 16:45:52 -0600 Subject: [PATCH 1/7] Move FundingTxInput::sequence to Utxo A forthcoming commit will change CoinSelection to include FundingTxInput instead of Utxo, though the former will probably be renamed. This is so CoinSelectionSource can be used when funding a splice. Further updating WalletSource to use FundingTxInput is not desirable, however, as it would result in looking up each confirmed UTXOs previous transaction even if it is not selected. See Wallet's implementation of CoinSelectionSource, which delegates to WalletSource for listing all confirmed UTXOs. This commit moves FundingTxInput::sequence to Utxo, and thus the responsibility for setting it to WalletSource implementations. Doing so will allow Wallet's CoinSelectionSource implementation to delegate looking up previous transactions to WalletSource without having to explicitly set the sequence on any FundingTxInput. --- lightning/src/events/bump_transaction/mod.rs | 10 +++++++- lightning/src/ln/funding.rs | 25 ++++++++++++------- lightning/src/ln/interactivetxs.rs | 8 ++++-- lightning/src/util/anchor_channel_reserves.rs | 3 ++- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/lightning/src/events/bump_transaction/mod.rs b/lightning/src/events/bump_transaction/mod.rs index ff034176385..a79e927e169 100644 --- a/lightning/src/events/bump_transaction/mod.rs +++ b/lightning/src/events/bump_transaction/mod.rs @@ -284,12 +284,15 @@ pub struct Utxo { /// with their lengths included, required to satisfy the output's script. The weight consumed by /// the input's `script_sig` must account for [`WITNESS_SCALE_FACTOR`]. pub satisfaction_weight: u64, + /// The sequence number to use in the [`TxIn`] when spending the UTXO. + pub sequence: Sequence, } impl_writeable_tlv_based!(Utxo, { (1, outpoint, required), (3, output, required), (5, satisfaction_weight, required), + (7, sequence, (default_value, Sequence::ENABLE_RBF_NO_LOCKTIME)), }); impl Utxo { @@ -304,6 +307,7 @@ impl Utxo { outpoint, output: TxOut { value, script_pubkey: ScriptBuf::new_p2pkh(pubkey_hash) }, satisfaction_weight: script_sig_size * WITNESS_SCALE_FACTOR as u64 + 1, /* empty witness */ + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, } } @@ -323,6 +327,7 @@ impl Utxo { }, satisfaction_weight: script_sig_size * WITNESS_SCALE_FACTOR as u64 + P2WPKH_WITNESS_WEIGHT, + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, } } @@ -332,6 +337,7 @@ impl Utxo { outpoint, output: TxOut { value, script_pubkey: ScriptBuf::new_p2wpkh(pubkey_hash) }, satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + P2WPKH_WITNESS_WEIGHT, + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, } } @@ -343,6 +349,7 @@ impl Utxo { outpoint, output: TxOut { value, script_pubkey: ScriptBuf::new_p2tr_tweaked(tweaked_public_key) }, satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + P2TR_KEY_PATH_WITNESS_WEIGHT, + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, } } } @@ -737,7 +744,7 @@ where tx.input.push(TxIn { previous_output: utxo.outpoint, script_sig: ScriptBuf::new(), - sequence: Sequence::ZERO, + sequence: utxo.sequence, witness: Witness::new(), }); } @@ -1392,6 +1399,7 @@ mod tests { script_pubkey: ScriptBuf::new(), }, satisfaction_weight: 5, // Just the script_sig and witness lengths + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, }], change_output: None, }, diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 8092a0e4451..50e0938fc8b 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -108,11 +108,6 @@ pub struct FundingTxInput { /// [`TxOut`]: bitcoin::TxOut pub(super) utxo: Utxo, - /// The sequence number to use in the [`TxIn`]. - /// - /// [`TxIn`]: bitcoin::TxIn - pub(super) sequence: Sequence, - /// The transaction containing the unspent [`TxOut`] referenced by [`utxo`]. /// /// [`TxOut`]: bitcoin::TxOut @@ -122,7 +117,19 @@ pub struct FundingTxInput { impl_writeable_tlv_based!(FundingTxInput, { (1, utxo, required), - (3, sequence, required), + (3, _sequence, (legacy, Sequence, + |read_val: Option<&Sequence>| { + if let Some(sequence) = read_val { + // Utxo contains sequence now, so update it if the value read here differs since + // this indicates Utxo::sequence was read with default_value + let utxo: &mut Utxo = utxo.0.as_mut().expect("utxo is required"); + if utxo.sequence != *sequence { + utxo.sequence = *sequence; + } + } + Ok(()) + }, + |input: &FundingTxInput| Some(input.utxo.sequence))), (5, prevtx, required), }); @@ -140,8 +147,8 @@ impl FundingTxInput { .ok_or(())? .clone(), satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + witness_weight.to_wu(), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, }, - sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, prevtx, }) } @@ -234,14 +241,14 @@ impl FundingTxInput { /// /// [`TxIn`]: bitcoin::TxIn pub fn sequence(&self) -> Sequence { - self.sequence + self.utxo.sequence } /// Sets the sequence number to use in the [`TxIn`]. /// /// [`TxIn`]: bitcoin::TxIn pub fn set_sequence(&mut self, sequence: Sequence) { - self.sequence = sequence; + self.utxo.sequence = sequence; } /// Converts the [`FundingTxInput`] into a [`Utxo`] for coin selection. diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index a004f6e9f14..3c47658e963 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -2054,9 +2054,13 @@ impl InteractiveTxConstructor { let mut inputs_to_contribute: Vec<(SerialId, InputOwned)> = inputs_to_contribute .into_iter() - .map(|FundingTxInput { utxo, sequence, prevtx: prev_tx }| { + .map(|FundingTxInput { utxo, prevtx: prev_tx }| { let serial_id = generate_holder_serial_id(entropy_source, is_initiator); - let txin = TxIn { previous_output: utxo.outpoint, sequence, ..Default::default() }; + let txin = TxIn { + previous_output: utxo.outpoint, + sequence: utxo.sequence, + ..Default::default() + }; let prev_output = utxo.output; let input = InputOwned::Single(SingleOwnedInput { input: txin, diff --git a/lightning/src/util/anchor_channel_reserves.rs b/lightning/src/util/anchor_channel_reserves.rs index 8026af03d58..25a0e7ca0ba 100644 --- a/lightning/src/util/anchor_channel_reserves.rs +++ b/lightning/src/util/anchor_channel_reserves.rs @@ -315,7 +315,7 @@ where #[cfg(test)] mod test { use super::*; - use bitcoin::{OutPoint, ScriptBuf, TxOut, Txid}; + use bitcoin::{OutPoint, ScriptBuf, Sequence, TxOut, Txid}; use std::str::FromStr; #[test] @@ -343,6 +343,7 @@ mod test { }, output: TxOut { value: amount, script_pubkey: ScriptBuf::new() }, satisfaction_weight: 1 * 4 + (1 + 1 + 72 + 1 + 33), + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, } } From edd0c83228454fc367088b2cd71974db029fcd52 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 13 Jan 2026 21:30:46 -0600 Subject: [PATCH 2/7] Use FundingTxInput instead of Utxo in CoinSelection In order to reuse CoinSelectionSource for splicing, the previous transaction of each UTXO is needed. Update CoinSelection to use FundingTxInput (renamed to ConfirmedUtxo) so that it is available. This requires adding a method to WalletSource to look up a previous transaction for a UTXO. Otherwise, Wallet's implementation of CoinSelectionSource would need WalletSource to include the previous transactions when listing confirmed UTXOs to select from. But this would be inefficient since only some UTXOs are selected. --- fuzz/src/full_stack.rs | 4 +- lightning/src/events/bump_transaction/mod.rs | 110 +++++++++++++----- lightning/src/events/bump_transaction/sync.rs | 14 ++- lightning/src/ln/functional_test_utils.rs | 3 +- lightning/src/ln/funding.rs | 19 ++- lightning/src/util/test_utils.rs | 42 ++++--- 6 files changed, 135 insertions(+), 57 deletions(-) diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index f7f912cfd48..11eca60151e 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -668,9 +668,7 @@ pub fn do_test(mut data: &[u8], logger: &Arc) { script_pubkey: wallet.get_change_script().unwrap(), }], }; - let coinbase_txid = coinbase_tx.compute_txid(); - wallet - .add_utxo(bitcoin::OutPoint { txid: coinbase_txid, vout: 0 }, Amount::from_sat(1_000_000)); + wallet.add_utxo(coinbase_tx.clone(), 0); loop { match get_slice!(1)[0] { diff --git a/lightning/src/events/bump_transaction/mod.rs b/lightning/src/events/bump_transaction/mod.rs index a79e927e169..8a04f622f7c 100644 --- a/lightning/src/events/bump_transaction/mod.rs +++ b/lightning/src/events/bump_transaction/mod.rs @@ -30,6 +30,7 @@ use crate::ln::chan_utils::{ HTLC_TIMEOUT_INPUT_KEYED_ANCHOR_WITNESS_WEIGHT, HTLC_TIMEOUT_INPUT_P2A_ANCHOR_WITNESS_WEIGHT, P2WSH_TXOUT_WEIGHT, SEGWIT_MARKER_FLAG_WEIGHT, TRUC_CHILD_MAX_WEIGHT, TRUC_MAX_WEIGHT, }; +use crate::ln::funding::FundingTxInput; use crate::ln::types::ChannelId; use crate::prelude::*; use crate::sign::ecdsa::EcdsaChannelSigner; @@ -354,13 +355,16 @@ impl Utxo { } } +/// An unspent transaction output with at least one confirmation. +pub type ConfirmedUtxo = FundingTxInput; + /// The result of a successful coin selection attempt for a transaction requiring additional UTXOs /// to cover its fees. #[derive(Clone, Debug)] pub struct CoinSelection { /// The set of UTXOs (with at least 1 confirmation) to spend and use within a transaction /// requiring additional fees. - pub confirmed_utxos: Vec, + pub confirmed_utxos: Vec, /// An additional output tracking whether any change remained after coin selection. This output /// should always have a value above dust for its given `script_pubkey`. It should not be /// spent until the transaction it belongs to confirms to ensure mempool descendant limits are @@ -368,6 +372,16 @@ pub struct CoinSelection { pub change_output: Option, } +impl CoinSelection { + fn satisfaction_weight(&self) -> u64 { + self.confirmed_utxos.iter().map(|ConfirmedUtxo { utxo, .. }| utxo.satisfaction_weight).sum() + } + + fn input_amount(&self) -> Amount { + self.confirmed_utxos.iter().map(|ConfirmedUtxo { utxo, .. }| utxo.output.value).sum() + } +} + /// An abstraction over a bitcoin wallet that can perform coin selection over a set of UTXOs and can /// sign for them. The coin selection method aims to mimic Bitcoin Core's `fundrawtransaction` RPC, /// which most wallets should be able to satisfy. Otherwise, consider implementing [`WalletSource`], @@ -438,11 +452,18 @@ pub trait WalletSource { fn list_confirmed_utxos<'a>( &'a self, ) -> impl Future, ()>> + MaybeSend + 'a; + + /// Returns the previous transaction containing the UTXO referenced by the outpoint. + fn get_prevtx<'a>( + &'a self, outpoint: OutPoint, + ) -> impl Future> + MaybeSend + 'a; + /// Returns a script to use for change above dust resulting from a successful coin selection /// attempt. fn get_change_script<'a>( &'a self, ) -> impl Future> + MaybeSend + 'a; + /// Signs and provides the full [`TxIn::script_sig`] and [`TxIn::witness`] for all inputs within /// the transaction known to the wallet (i.e., any provided via /// [`WalletSource::list_confirmed_utxos`]). @@ -628,10 +649,26 @@ where Some(TxOut { script_pubkey: change_script, value: change_output_amount }) }; - Ok(CoinSelection { - confirmed_utxos: selected_utxos.into_iter().map(|(utxo, _)| utxo).collect(), - change_output, - }) + let mut confirmed_utxos = Vec::with_capacity(selected_utxos.len()); + for (utxo, _) in selected_utxos { + let prevtx = self.source.get_prevtx(utxo.outpoint).await?; + let prevtx_id = prevtx.compute_txid(); + if prevtx_id != utxo.outpoint.txid + || prevtx.output.get(utxo.outpoint.vout as usize).is_none() + { + log_error!( + self.logger, + "Tx {} from wallet source doesn't contain output referenced by outpoint: {}", + prevtx_id, + utxo.outpoint, + ); + return Err(()); + } + + confirmed_utxos.push(ConfirmedUtxo { utxo, prevtx }); + } + + Ok(CoinSelection { confirmed_utxos, change_output }) } } @@ -740,7 +777,7 @@ where /// Updates a transaction with the result of a successful coin selection attempt. fn process_coin_selection(&self, tx: &mut Transaction, coin_selection: &CoinSelection) { - for utxo in coin_selection.confirmed_utxos.iter() { + for ConfirmedUtxo { utxo, .. } in coin_selection.confirmed_utxos.iter() { tx.input.push(TxIn { previous_output: utxo.outpoint, script_sig: ScriptBuf::new(), @@ -865,12 +902,10 @@ where output: vec![], }; - let input_satisfaction_weight: u64 = - coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum(); + let input_satisfaction_weight = coin_selection.satisfaction_weight(); let total_satisfaction_weight = anchor_input_witness_weight + EMPTY_SCRIPT_SIG_WEIGHT + input_satisfaction_weight; - let total_input_amount = must_spend_amount - + coin_selection.confirmed_utxos.iter().map(|utxo| utxo.output.value).sum(); + let total_input_amount = must_spend_amount + coin_selection.input_amount(); self.process_coin_selection(&mut anchor_tx, &coin_selection); let anchor_txid = anchor_tx.compute_txid(); @@ -885,10 +920,10 @@ where let index = idx + 1; debug_assert_eq!( anchor_psbt.unsigned_tx.input[index].previous_output, - utxo.outpoint + utxo.outpoint() ); - if utxo.output.script_pubkey.is_witness_program() { - anchor_psbt.inputs[index].witness_utxo = Some(utxo.output); + if utxo.output().script_pubkey.is_witness_program() { + anchor_psbt.inputs[index].witness_utxo = Some(utxo.into_output()); } } @@ -1127,13 +1162,11 @@ where utxo_id = claim_id.step_with_bytes(&broadcasted_htlcs.to_be_bytes()); #[cfg(debug_assertions)] - let input_satisfaction_weight: u64 = - coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum(); + let input_satisfaction_weight = coin_selection.satisfaction_weight(); #[cfg(debug_assertions)] let total_satisfaction_weight = must_spend_satisfaction_weight + input_satisfaction_weight; #[cfg(debug_assertions)] - let input_value: u64 = - coin_selection.confirmed_utxos.iter().map(|utxo| utxo.output.value.to_sat()).sum(); + let input_value = coin_selection.input_amount().to_sat(); #[cfg(debug_assertions)] let total_input_amount = must_spend_amount + input_value; @@ -1154,9 +1187,12 @@ where for (idx, utxo) in coin_selection.confirmed_utxos.into_iter().enumerate() { // offset to skip the htlc inputs let index = idx + selected_htlcs.len(); - debug_assert_eq!(htlc_psbt.unsigned_tx.input[index].previous_output, utxo.outpoint); - if utxo.output.script_pubkey.is_witness_program() { - htlc_psbt.inputs[index].witness_utxo = Some(utxo.output); + debug_assert_eq!( + htlc_psbt.unsigned_tx.input[index].previous_output, + utxo.outpoint() + ); + if utxo.output().script_pubkey.is_witness_program() { + htlc_psbt.inputs[index].witness_utxo = Some(utxo.into_output()); } } @@ -1311,10 +1347,9 @@ mod tests { use crate::util::ser::Readable; use crate::util::test_utils::{TestBroadcaster, TestLogger}; - use bitcoin::hashes::Hash; use bitcoin::hex::FromHex; use bitcoin::{ - Network, ScriptBuf, Transaction, Txid, WitnessProgram, WitnessVersion, XOnlyPublicKey, + Network, ScriptBuf, Transaction, WitnessProgram, WitnessVersion, XOnlyPublicKey, }; struct TestCoinSelectionSource { @@ -1335,9 +1370,17 @@ mod tests { Ok(res) } fn sign_psbt(&self, psbt: Psbt) -> Result { + let prevtx_ids: Vec<_> = self + .expected_selects + .lock() + .unwrap() + .iter() + .flat_map(|selection| selection.3.confirmed_utxos.iter()) + .map(|utxo| utxo.prevtx.compute_txid()) + .collect(); let mut tx = psbt.unsigned_tx; for input in tx.input.iter_mut() { - if input.previous_output.txid != Txid::from_byte_array([44; 32]) { + if prevtx_ids.contains(&input.previous_output.txid) { // Channel output, add a realistic size witness to make the assertions happy input.witness = Witness::from_slice(&[vec![42; 162]]); } @@ -1378,6 +1421,13 @@ mod tests { .weight() .to_wu(); + let prevtx = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![], + output: vec![TxOut { value: Amount::from_sat(200), script_pubkey: ScriptBuf::new() }], + }; + let broadcaster = TestBroadcaster::new(Network::Testnet); let source = TestCoinSelectionSource { expected_selects: Mutex::new(vec![ @@ -1392,14 +1442,14 @@ mod tests { commitment_and_anchor_fee, 868, CoinSelection { - confirmed_utxos: vec![Utxo { - outpoint: OutPoint { txid: Txid::from_byte_array([44; 32]), vout: 0 }, - output: TxOut { - value: Amount::from_sat(200), - script_pubkey: ScriptBuf::new(), + confirmed_utxos: vec![ConfirmedUtxo { + utxo: Utxo { + outpoint: OutPoint { txid: prevtx.compute_txid(), vout: 0 }, + output: prevtx.output[0].clone(), + satisfaction_weight: 5, // Just the script_sig and witness lengths + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, }, - satisfaction_weight: 5, // Just the script_sig and witness lengths - sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + prevtx, }], change_output: None, }, diff --git a/lightning/src/events/bump_transaction/sync.rs b/lightning/src/events/bump_transaction/sync.rs index f4245cd5194..a521fa9c48a 100644 --- a/lightning/src/events/bump_transaction/sync.rs +++ b/lightning/src/events/bump_transaction/sync.rs @@ -21,7 +21,7 @@ use crate::sign::SignerProvider; use crate::util::async_poll::{dummy_waker, MaybeSend, MaybeSync}; use crate::util::logger::Logger; -use bitcoin::{Psbt, ScriptBuf, Transaction, TxOut}; +use bitcoin::{OutPoint, Psbt, ScriptBuf, Transaction, TxOut}; use super::BumpTransactionEvent; use super::{ @@ -37,9 +37,14 @@ use super::{ pub trait WalletSourceSync { /// Returns all UTXOs, with at least 1 confirmation each, that are available to spend. fn list_confirmed_utxos(&self) -> Result, ()>; + + /// Returns the previous transaction containing the UTXO referenced by the outpoint. + fn get_prevtx(&self, outpoint: OutPoint) -> Result; + /// Returns a script to use for change above dust resulting from a successful coin selection /// attempt. fn get_change_script(&self) -> Result; + /// Signs and provides the full [`TxIn::script_sig`] and [`TxIn::witness`] for all inputs within /// the transaction known to the wallet (i.e., any provided via /// [`WalletSource::list_confirmed_utxos`]). @@ -79,6 +84,13 @@ where async move { utxos } } + fn get_prevtx<'a>( + &'a self, outpoint: OutPoint, + ) -> impl Future> + MaybeSend + 'a { + let prevtx = self.0.get_prevtx(outpoint); + Box::pin(async move { prevtx }) + } + fn get_change_script<'a>( &'a self, ) -> impl Future> + MaybeSend + 'a { diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index a0246a9d091..33f78b13553 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -397,8 +397,7 @@ fn do_connect_block_without_consistency_checks<'a, 'b, 'c, 'd>( let wallet_script = node.wallet_source.get_change_script().unwrap(); for (idx, output) in tx.output.iter().enumerate() { if output.script_pubkey == wallet_script { - let outpoint = bitcoin::OutPoint { txid: tx.compute_txid(), vout: idx as u32 }; - node.wallet_source.add_utxo(outpoint, output.value); + node.wallet_source.add_utxo(tx.clone(), idx as u32); } } } diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 50e0938fc8b..9981250b05e 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -103,16 +103,17 @@ impl SpliceContribution { /// establishment protocol or when splicing. #[derive(Debug, Clone)] pub struct FundingTxInput { - /// The unspent [`TxOut`] that the input spends. + /// The unspent [`TxOut`] found in [`prevtx`]. /// /// [`TxOut`]: bitcoin::TxOut - pub(super) utxo: Utxo, + /// [`prevtx`]: Self::prevtx + pub(crate) utxo: Utxo, /// The transaction containing the unspent [`TxOut`] referenced by [`utxo`]. /// /// [`TxOut`]: bitcoin::TxOut /// [`utxo`]: Self::utxo - pub(super) prevtx: Transaction, + pub(crate) prevtx: Transaction, } impl_writeable_tlv_based!(FundingTxInput, { @@ -237,6 +238,11 @@ impl FundingTxInput { self.utxo.outpoint } + /// The unspent output. + pub fn output(&self) -> &TxOut { + &self.utxo.output + } + /// The sequence number to use in the [`TxIn`]. /// /// [`TxIn`]: bitcoin::TxIn @@ -251,8 +257,13 @@ impl FundingTxInput { self.utxo.sequence = sequence; } - /// Converts the [`FundingTxInput`] into a [`Utxo`] for coin selection. + /// Converts the [`FundingTxInput`] into a [`Utxo`]. pub fn into_utxo(self) -> Utxo { self.utxo } + + /// Converts the [`FundingTxInput`] into a [`TxOut`]. + pub fn into_output(self) -> TxOut { + self.utxo.output + } } diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 1d3137a75aa..76fa0a5f3fd 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -22,7 +22,7 @@ use crate::chain::channelmonitor::{ use crate::chain::transaction::OutPoint; use crate::chain::WatchedOutput; use crate::events::bump_transaction::sync::WalletSourceSync; -use crate::events::bump_transaction::Utxo; +use crate::events::bump_transaction::{ConfirmedUtxo, Utxo}; #[cfg(any(test, feature = "_externalize_tests"))] use crate::ln::chan_utils::CommitmentTransaction; use crate::ln::channel_state::ChannelDetails; @@ -2256,7 +2256,7 @@ impl Drop for TestScorer { pub struct TestWalletSource { secret_key: SecretKey, - utxos: Mutex>, + utxos: Mutex>, secp: Secp256k1, } @@ -2265,21 +2265,13 @@ impl TestWalletSource { Self { secret_key, utxos: Mutex::new(Vec::new()), secp: Secp256k1::new() } } - pub fn add_utxo(&self, outpoint: bitcoin::OutPoint, value: Amount) -> TxOut { - let public_key = bitcoin::PublicKey::new(self.secret_key.public_key(&self.secp)); - let utxo = Utxo::new_v0_p2wpkh(outpoint, value, &public_key.wpubkey_hash().unwrap()); - self.utxos.lock().unwrap().push(utxo.clone()); - utxo.output - } - - pub fn add_custom_utxo(&self, utxo: Utxo) -> TxOut { - let output = utxo.output.clone(); + pub fn add_utxo(&self, prevtx: Transaction, vout: u32) { + let utxo = ConfirmedUtxo::new_p2wpkh(prevtx, vout).unwrap(); self.utxos.lock().unwrap().push(utxo); - output } pub fn remove_utxo(&self, outpoint: bitcoin::OutPoint) { - self.utxos.lock().unwrap().retain(|utxo| utxo.outpoint != outpoint); + self.utxos.lock().unwrap().retain(|utxo| utxo.outpoint() != outpoint); } pub fn clear_utxos(&self) { @@ -2292,12 +2284,12 @@ impl TestWalletSource { let utxos = self.utxos.lock().unwrap(); for i in 0..tx.input.len() { if let Some(utxo) = - utxos.iter().find(|utxo| utxo.outpoint == tx.input[i].previous_output) + utxos.iter().find(|utxo| utxo.outpoint() == tx.input[i].previous_output) { let sighash = SighashCache::new(&tx).p2wpkh_signature_hash( i, - &utxo.output.script_pubkey, - utxo.output.value, + &utxo.output().script_pubkey, + utxo.output().value, EcdsaSighashType::All, )?; #[cfg(not(feature = "grind_signatures"))] @@ -2322,7 +2314,23 @@ impl TestWalletSource { impl WalletSourceSync for TestWalletSource { fn list_confirmed_utxos(&self) -> Result, ()> { - Ok(self.utxos.lock().unwrap().clone()) + Ok(self + .utxos + .lock() + .unwrap() + .iter() + .map(|ConfirmedUtxo { utxo, .. }| utxo.clone()) + .collect()) + } + + fn get_prevtx(&self, outpoint: bitcoin::OutPoint) -> Result { + self.utxos + .lock() + .unwrap() + .iter() + .find(|confirmed_utxo| confirmed_utxo.utxo.outpoint == outpoint) + .map(|ConfirmedUtxo { prevtx, .. }| prevtx.clone()) + .ok_or(()) } fn get_change_script(&self) -> Result { From 39a1e62f6b77376a4df4622e2cea3dddc35141dc Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 28 Jan 2026 15:32:51 -0600 Subject: [PATCH 3/7] Make ClaimId optional in coin selection CoinSelectionSource is used for anchor bumping where a ClaimId is passed in to avoid double spending other claims. To re-use this trait for funding a splice, the ClaimId must be optional. And, if None, then any locked UTXOs may be considered ineligible by an implementation. --- lightning/src/events/bump_transaction/mod.rs | 29 ++++++++++++++----- lightning/src/events/bump_transaction/sync.rs | 6 ++-- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/lightning/src/events/bump_transaction/mod.rs b/lightning/src/events/bump_transaction/mod.rs index 8a04f622f7c..e042fb28a63 100644 --- a/lightning/src/events/bump_transaction/mod.rs +++ b/lightning/src/events/bump_transaction/mod.rs @@ -425,9 +425,12 @@ pub trait CoinSelectionSource { /// which UTXOs to double spend is left to the implementation, but it must strive to keep the /// set of other claims being double spent to a minimum. /// + /// If `claim_id` is not set, then the selection should be treated as if it were for a unique + /// claim and must NOT be double-spent rather than being kept to a minimum. + /// /// [`ChannelMonitor::rebroadcast_pending_claims`]: crate::chain::channelmonitor::ChannelMonitor::rebroadcast_pending_claims fn select_confirmed_utxos<'a>( - &'a self, claim_id: ClaimId, must_spend: Vec, must_pay_to: &'a [TxOut], + &'a self, claim_id: Option, must_spend: Vec, must_pay_to: &'a [TxOut], target_feerate_sat_per_1000_weight: u32, max_tx_weight: u64, ) -> impl Future> + MaybeSend + 'a; /// Signs and provides the full witness for all inputs within the transaction known to the @@ -492,7 +495,7 @@ where // TODO: Do we care about cleaning this up once the UTXOs have a confirmed spend? We can do so // by checking whether any UTXOs that exist in the map are no longer returned in // `list_confirmed_utxos`. - locked_utxos: Mutex>, + locked_utxos: Mutex>>, } impl Wallet @@ -514,11 +517,13 @@ where /// least 1 satoshi at the current feerate, otherwise, we'll only attempt to spend those which /// contribute at least twice their fee. async fn select_confirmed_utxos_internal( - &self, utxos: &[Utxo], claim_id: ClaimId, force_conflicting_utxo_spend: bool, + &self, utxos: &[Utxo], claim_id: Option, force_conflicting_utxo_spend: bool, tolerate_high_network_feerates: bool, target_feerate_sat_per_1000_weight: u32, preexisting_tx_weight: u64, input_amount_sat: Amount, target_amount_sat: Amount, max_tx_weight: u64, ) -> Result { + debug_assert!(!(claim_id.is_none() && force_conflicting_utxo_spend)); + // P2WSH and P2TR outputs are both the heaviest-weight standard outputs at 34 bytes let max_coin_selection_weight = max_tx_weight .checked_sub(preexisting_tx_weight + P2WSH_TXOUT_WEIGHT) @@ -538,7 +543,12 @@ where .iter() .filter_map(|utxo| { if let Some(utxo_claim_id) = locked_utxos.get(&utxo.outpoint) { - if *utxo_claim_id != claim_id && !force_conflicting_utxo_spend { + // TODO(splicing): For splicing (i.e., claim_id.is_none()), ideally we'd + // allow force_conflicting_utxo_spend for an RBF attempt. However, we'd need + // something similar to a ClaimId to identify a splice. + if (utxo_claim_id.is_none() || *utxo_claim_id != claim_id) + && !force_conflicting_utxo_spend + { log_trace!( self.logger, "Skipping UTXO {} to prevent conflicting spend", @@ -678,7 +688,7 @@ where W::Target: WalletSource + MaybeSend + MaybeSync, { fn select_confirmed_utxos<'a>( - &'a self, claim_id: ClaimId, must_spend: Vec, must_pay_to: &'a [TxOut], + &'a self, claim_id: Option, must_spend: Vec, must_pay_to: &'a [TxOut], target_feerate_sat_per_1000_weight: u32, max_tx_weight: u64, ) -> impl Future> + MaybeSend + 'a { async move { @@ -703,6 +713,9 @@ where let configs = [(false, false), (false, true), (true, false), (true, true)]; for (force_conflicting_utxo_spend, tolerate_high_network_feerates) in configs { + if claim_id.is_none() && force_conflicting_utxo_spend { + continue; + } log_debug!( self.logger, "Attempting coin selection targeting {} sat/kW (force_conflicting_utxo_spend = {}, tolerate_high_network_feerates = {})", @@ -874,7 +887,7 @@ where let coin_selection: CoinSelection = self .utxo_source .select_confirmed_utxos( - claim_id, + Some(claim_id), must_spend, &[], package_target_feerate_sat_per_1000_weight, @@ -1137,7 +1150,7 @@ where let coin_selection: CoinSelection = match self .utxo_source .select_confirmed_utxos( - utxo_id, + Some(utxo_id), must_spend, &htlc_tx.output, target_feerate_sat_per_1000_weight, @@ -1358,7 +1371,7 @@ mod tests { } impl CoinSelectionSourceSync for TestCoinSelectionSource { fn select_confirmed_utxos( - &self, _claim_id: ClaimId, must_spend: Vec, _must_pay_to: &[TxOut], + &self, _claim_id: Option, must_spend: Vec, _must_pay_to: &[TxOut], target_feerate_sat_per_1000_weight: u32, _max_tx_weight: u64, ) -> Result { let mut expected_selects = self.expected_selects.lock().unwrap(); diff --git a/lightning/src/events/bump_transaction/sync.rs b/lightning/src/events/bump_transaction/sync.rs index a521fa9c48a..39088bb0e97 100644 --- a/lightning/src/events/bump_transaction/sync.rs +++ b/lightning/src/events/bump_transaction/sync.rs @@ -135,7 +135,7 @@ where W::Target: WalletSourceSync + MaybeSend + MaybeSync, { fn select_confirmed_utxos( - &self, claim_id: ClaimId, must_spend: Vec, must_pay_to: &[TxOut], + &self, claim_id: Option, must_spend: Vec, must_pay_to: &[TxOut], target_feerate_sat_per_1000_weight: u32, max_tx_weight: u64, ) -> Result { let fut = self.wallet.select_confirmed_utxos( @@ -214,7 +214,7 @@ pub trait CoinSelectionSourceSync { /// /// [`ChannelMonitor::rebroadcast_pending_claims`]: crate::chain::channelmonitor::ChannelMonitor::rebroadcast_pending_claims fn select_confirmed_utxos( - &self, claim_id: ClaimId, must_spend: Vec, must_pay_to: &[TxOut], + &self, claim_id: Option, must_spend: Vec, must_pay_to: &[TxOut], target_feerate_sat_per_1000_weight: u32, max_tx_weight: u64, ) -> Result; @@ -247,7 +247,7 @@ where T::Target: CoinSelectionSourceSync, { fn select_confirmed_utxos<'a>( - &'a self, claim_id: ClaimId, must_spend: Vec, must_pay_to: &'a [TxOut], + &'a self, claim_id: Option, must_spend: Vec, must_pay_to: &'a [TxOut], target_feerate_sat_per_1000_weight: u32, max_tx_weight: u64, ) -> impl Future> + MaybeSend + 'a { let coins = self.0.select_confirmed_utxos( From 5e28127e9ce98a6125d2c2227f1a6b0647d3a33e Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 18 Dec 2025 09:54:00 -0600 Subject: [PATCH 4/7] Split splice initiation into two phases Previously, splice_channel required callers to manually construct funding inputs and pass them directly, making coin selection the caller's responsibility. This made the API difficult to use and prevented reuse of the existing CoinSelectionSource trait. Introduce a two-phase API: splice_channel now returns a FundingTemplate that callers use to build a FundingContribution via wallet-backed splice methods (e.g., splice_in_sync, splice_out_sync), which handle coin selection automatically. The completed contribution is then passed to a new funding_contributed method to begin quiescence and negotiation. This also renames SpliceContribution to FundingContribution and moves fee estimation and input validation into the funding module, co-located with the types they operate on. Co-Authored-By: Claude Opus 4.6 --- fuzz/src/chanmon_consistency.rs | 323 ++++---- fuzz/src/full_stack.rs | 104 ++- .../src/upgrade_downgrade_tests.rs | 9 +- lightning/src/ln/async_signer_tests.rs | 8 +- lightning/src/ln/channel.rs | 659 +++++----------- lightning/src/ln/channelmanager.rs | 158 +++- lightning/src/ln/functional_test_utils.rs | 8 +- lightning/src/ln/funding.rs | 729 ++++++++++++++++-- lightning/src/ln/splicing_tests.rs | 616 ++++++++------- lightning/src/ln/zero_fee_commitment_tests.rs | 4 +- lightning/src/util/ser.rs | 14 + 11 files changed, 1575 insertions(+), 1057 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 87d58da4832..1a763339c8d 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -26,6 +26,7 @@ use bitcoin::opcodes; use bitcoin::script::{Builder, ScriptBuf}; use bitcoin::transaction::Version; use bitcoin::transaction::{Transaction, TxOut}; +use bitcoin::FeeRate; use bitcoin::hash_types::BlockHash; use bitcoin::hashes::sha256::Hash as Sha256; @@ -45,6 +46,7 @@ use lightning::chain::{ chainmonitor, channelmonitor, BestBlock, ChannelMonitorUpdateStatus, Confirm, Watch, }; use lightning::events; +use lightning::events::bump_transaction::sync::{WalletSourceSync, WalletSync}; use lightning::ln::channel::{ FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS, }; @@ -53,7 +55,6 @@ use lightning::ln::channelmanager::{ ChainParameters, ChannelManager, ChannelManagerReadArgs, PaymentId, RecentPaymentDetails, }; use lightning::ln::functional_test_utils::*; -use lightning::ln::funding::{FundingTxInput, SpliceContribution}; use lightning::ln::inbound_payment::ExpandedKey; use lightning::ln::msgs::{ BaseMessageHandler, ChannelMessageHandler, CommitmentUpdate, Init, MessageSendEvent, @@ -72,12 +73,14 @@ use lightning::sign::{ SignerProvider, }; use lightning::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret}; +use lightning::util::async_poll::{MaybeSend, MaybeSync}; use lightning::util::config::UserConfig; use lightning::util::errors::APIError; use lightning::util::hash_tables::*; use lightning::util::logger::Logger; use lightning::util::ser::{LengthReadable, ReadableArgs, Writeable, Writer}; use lightning::util::test_channel_signer::{EnforcementState, TestChannelSigner}; +use lightning::util::test_utils::TestWalletSource; use lightning_invoice::RawBolt11Invoice; @@ -176,63 +179,6 @@ impl Writer for VecWriter { } } -pub struct TestWallet { - secret_key: SecretKey, - utxos: Mutex>, - secp: Secp256k1, -} - -impl TestWallet { - pub fn new(secret_key: SecretKey) -> Self { - Self { secret_key, utxos: Mutex::new(Vec::new()), secp: Secp256k1::new() } - } - - fn get_change_script(&self) -> Result { - let public_key = bitcoin::PublicKey::new(self.secret_key.public_key(&self.secp)); - Ok(ScriptBuf::new_p2wpkh(&public_key.wpubkey_hash().unwrap())) - } - - pub fn add_utxo(&self, outpoint: bitcoin::OutPoint, value: Amount) -> TxOut { - let public_key = bitcoin::PublicKey::new(self.secret_key.public_key(&self.secp)); - let utxo = lightning::events::bump_transaction::Utxo::new_v0_p2wpkh( - outpoint, - value, - &public_key.wpubkey_hash().unwrap(), - ); - self.utxos.lock().unwrap().push(utxo.clone()); - utxo.output - } - - pub fn sign_tx( - &self, mut tx: Transaction, - ) -> Result { - let utxos = self.utxos.lock().unwrap(); - for i in 0..tx.input.len() { - if let Some(utxo) = - utxos.iter().find(|utxo| utxo.outpoint == tx.input[i].previous_output) - { - let sighash = bitcoin::sighash::SighashCache::new(&tx).p2wpkh_signature_hash( - i, - &utxo.output.script_pubkey, - utxo.output.value, - bitcoin::EcdsaSighashType::All, - )?; - let signature = self.secp.sign_ecdsa( - &secp256k1::Message::from_digest(sighash.to_byte_array()), - &self.secret_key, - ); - let bitcoin_sig = bitcoin::ecdsa::Signature { - signature, - sighash_type: bitcoin::EcdsaSighashType::All, - }; - tx.input[i].witness = - bitcoin::Witness::p2wpkh(&bitcoin_sig, &self.secret_key.public_key(&self.secp)); - } - } - Ok(tx) - } -} - /// The LDK API requires that any time we tell it we're done persisting a `ChannelMonitor[Update]` /// we never pass it in as the "latest" `ChannelMonitor` on startup. However, we can pass /// out-of-date monitors as long as we never told LDK we finished persisting them, which we do by @@ -542,7 +488,7 @@ type ChanMan<'a> = ChannelManager< Arc, &'a FuzzRouter, &'a FuzzRouter, - Arc, + Arc, >; #[inline] @@ -778,7 +724,9 @@ fn send_mpp_hop_payment( } #[inline] -pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { +pub fn do_test( + data: &[u8], underlying_out: Out, anchors: bool, +) { let out = SearchingOutput::new(underlying_out); let broadcast = Arc::new(TestBroadcaster { txn_broadcasted: RefCell::new(Vec::new()) }); let router = FuzzRouter {}; @@ -805,7 +753,7 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { macro_rules! make_node { ($node_id: expr, $fee_estimator: expr) => {{ - let logger: Arc = + let logger: Arc = Arc::new(test_logger::TestLogger::new($node_id.to_string(), out.clone())); let node_secret = SecretKey::from_slice(&[ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, @@ -854,6 +802,7 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { ), monitor, keys_manager, + logger, ) }}; } @@ -865,7 +814,7 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { keys, fee_estimator| { let keys_manager = Arc::clone(keys); - let logger: Arc = + let logger: Arc = Arc::new(test_logger::TestLogger::new(node_id.to_string(), out.clone())); let chain_monitor = Arc::new(TestChainMonitor::new( broadcast.clone(), @@ -1159,9 +1108,9 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { }}; } - let wallet_a = TestWallet::new(SecretKey::from_slice(&[1; 32]).unwrap()); - let wallet_b = TestWallet::new(SecretKey::from_slice(&[2; 32]).unwrap()); - let wallet_c = TestWallet::new(SecretKey::from_slice(&[3; 32]).unwrap()); + let wallet_a = TestWalletSource::new(SecretKey::from_slice(&[1; 32]).unwrap()); + let wallet_b = TestWalletSource::new(SecretKey::from_slice(&[2; 32]).unwrap()); + let wallet_c = TestWalletSource::new(SecretKey::from_slice(&[3; 32]).unwrap()); let wallets = vec![wallet_a, wallet_b, wallet_c]; let coinbase_tx = bitcoin::Transaction { version: bitcoin::transaction::Version::TWO, @@ -1175,12 +1124,8 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { }) .collect(), }; - let coinbase_txid = coinbase_tx.compute_txid(); wallets.iter().enumerate().for_each(|(i, w)| { - w.add_utxo( - bitcoin::OutPoint { txid: coinbase_txid, vout: i as u32 }, - Amount::from_sat(100_000), - ); + w.add_utxo(coinbase_tx.clone(), i as u32); }); let fee_est_a = Arc::new(FuzzEstimator { ret_val: atomic::AtomicU32::new(253) }); @@ -1192,11 +1137,13 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { // 3 nodes is enough to hit all the possible cases, notably unknown-source-unknown-dest // forwarding. - let (node_a, mut monitor_a, keys_manager_a) = make_node!(0, fee_est_a); - let (node_b, mut monitor_b, keys_manager_b) = make_node!(1, fee_est_b); - let (node_c, mut monitor_c, keys_manager_c) = make_node!(2, fee_est_c); + let (node_a, mut monitor_a, keys_manager_a, logger_a) = make_node!(0, fee_est_a); + let (node_b, mut monitor_b, keys_manager_b, logger_b) = make_node!(1, fee_est_b); + let (node_c, mut monitor_c, keys_manager_c, logger_c) = make_node!(2, fee_est_c); let mut nodes = [node_a, node_b, node_c]; + let loggers = [logger_a, logger_b, logger_c]; + let fee_estimators = [Arc::clone(&fee_est_a), Arc::clone(&fee_est_b), Arc::clone(&fee_est_c)]; // Connect peers first, then create channels connect_peers!(nodes[0], nodes[1]); @@ -2130,79 +2077,83 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { }, 0xa0 => { - let input = FundingTxInput::new_p2wpkh(coinbase_tx.clone(), 0).unwrap(); - let contribution = - SpliceContribution::splice_in(Amount::from_sat(10_000), vec![input], None); - let funding_feerate_sat_per_kw = fee_est_a.ret_val.load(atomic::Ordering::Acquire); - if let Err(e) = nodes[0].splice_channel( + let feerate_sat_per_kw = fee_estimators[0].ret_val.load(atomic::Ordering::Acquire); + let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); + match nodes[0].splice_channel( &chan_a_id, &nodes[1].get_our_node_id(), - contribution, - funding_feerate_sat_per_kw, None, + feerate, ) { - assert!( - matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice pending")), - "{:?}", - e - ); + Ok(funding_template) => { + let wallet = WalletSync::new(&wallets[0], Arc::clone(&loggers[0])); + if let Ok(contribution) = funding_template.splice_in_sync(None, Amount::from_sat(10_000), &wallet) { + let _ = nodes[0].funding_contributed(&chan_a_id, &nodes[1].get_our_node_id(), contribution, None); + } + }, + Err(e) => { + assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + }, } }, 0xa1 => { - let input = FundingTxInput::new_p2wpkh(coinbase_tx.clone(), 1).unwrap(); - let contribution = - SpliceContribution::splice_in(Amount::from_sat(10_000), vec![input], None); - let funding_feerate_sat_per_kw = fee_est_b.ret_val.load(atomic::Ordering::Acquire); - if let Err(e) = nodes[1].splice_channel( + let feerate_sat_per_kw = fee_estimators[1].ret_val.load(atomic::Ordering::Acquire); + let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); + match nodes[1].splice_channel( &chan_a_id, &nodes[0].get_our_node_id(), - contribution, - funding_feerate_sat_per_kw, None, + feerate, ) { - assert!( - matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice pending")), - "{:?}", - e - ); + Ok(funding_template) => { + let wallet = WalletSync::new(&wallets[1], Arc::clone(&loggers[1])); + if let Ok(contribution) = funding_template.splice_in_sync(None, Amount::from_sat(10_000), &wallet) { + let _ = nodes[1].funding_contributed(&chan_a_id, &nodes[0].get_our_node_id(), contribution, None); + } + }, + Err(e) => { + assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + }, } }, 0xa2 => { - let input = FundingTxInput::new_p2wpkh(coinbase_tx.clone(), 0).unwrap(); - let contribution = - SpliceContribution::splice_in(Amount::from_sat(10_000), vec![input], None); - let funding_feerate_sat_per_kw = fee_est_b.ret_val.load(atomic::Ordering::Acquire); - if let Err(e) = nodes[1].splice_channel( + let feerate_sat_per_kw = fee_estimators[1].ret_val.load(atomic::Ordering::Acquire); + let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); + match nodes[1].splice_channel( &chan_b_id, &nodes[2].get_our_node_id(), - contribution, - funding_feerate_sat_per_kw, None, + feerate, ) { - assert!( - matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice pending")), - "{:?}", - e - ); + Ok(funding_template) => { + let wallet = WalletSync::new(&wallets[1], Arc::clone(&loggers[1])); + if let Ok(contribution) = funding_template.splice_in_sync(None, Amount::from_sat(10_000), &wallet) { + let _ = nodes[1].funding_contributed(&chan_b_id, &nodes[2].get_our_node_id(), contribution, None); + } + }, + Err(e) => { + assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + }, } }, 0xa3 => { - let input = FundingTxInput::new_p2wpkh(coinbase_tx.clone(), 1).unwrap(); - let contribution = - SpliceContribution::splice_in(Amount::from_sat(10_000), vec![input], None); - let funding_feerate_sat_per_kw = fee_est_c.ret_val.load(atomic::Ordering::Acquire); - if let Err(e) = nodes[2].splice_channel( + let feerate_sat_per_kw = fee_estimators[2].ret_val.load(atomic::Ordering::Acquire); + let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); + match nodes[2].splice_channel( &chan_b_id, &nodes[1].get_our_node_id(), - contribution, - funding_feerate_sat_per_kw, None, + feerate, ) { - assert!( - matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice pending")), - "{:?}", - e - ); + Ok(funding_template) => { + let wallet = WalletSync::new(&wallets[2], Arc::clone(&loggers[2])); + if let Ok(contribution) = funding_template.splice_in_sync(None, Amount::from_sat(10_000), &wallet) { + let _ = nodes[2].funding_contributed(&chan_b_id, &nodes[1].get_our_node_id(), contribution, None); + } + }, + Err(e) => { + assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + }, } }, @@ -2217,24 +2168,28 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { .map(|chan| chan.outbound_capacity_msat) .unwrap(); if outbound_capacity_msat >= 20_000_000 { - let contribution = SpliceContribution::splice_out(vec![TxOut { - value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), - script_pubkey: coinbase_tx.output[0].script_pubkey.clone(), - }]); - let funding_feerate_sat_per_kw = - fee_est_a.ret_val.load(atomic::Ordering::Acquire); - if let Err(e) = nodes[0].splice_channel( + let feerate_sat_per_kw = + fee_estimators[0].ret_val.load(atomic::Ordering::Acquire); + let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); + match nodes[0].splice_channel( &chan_a_id, &nodes[1].get_our_node_id(), - contribution, - funding_feerate_sat_per_kw, None, + feerate, ) { - assert!( - matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice pending")), - "{:?}", - e - ); + Ok(funding_template) => { + let outputs = vec![TxOut { + value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), + script_pubkey: coinbase_tx.output[0].script_pubkey.clone(), + }]; + let wallet = WalletSync::new(&wallets[0], Arc::clone(&loggers[0])); + if let Ok(contribution) = funding_template.splice_out_sync(outputs, &wallet) { + let _ = nodes[0].funding_contributed(&chan_a_id, &nodes[1].get_our_node_id(), contribution, None); + } + }, + Err(e) => { + assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + }, } } }, @@ -2246,24 +2201,28 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { .map(|chan| chan.outbound_capacity_msat) .unwrap(); if outbound_capacity_msat >= 20_000_000 { - let contribution = SpliceContribution::splice_out(vec![TxOut { - value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), - script_pubkey: coinbase_tx.output[1].script_pubkey.clone(), - }]); - let funding_feerate_sat_per_kw = - fee_est_b.ret_val.load(atomic::Ordering::Acquire); - if let Err(e) = nodes[1].splice_channel( + let feerate_sat_per_kw = + fee_estimators[1].ret_val.load(atomic::Ordering::Acquire); + let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); + match nodes[1].splice_channel( &chan_a_id, &nodes[0].get_our_node_id(), - contribution, - funding_feerate_sat_per_kw, None, + feerate, ) { - assert!( - matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice pending")), - "{:?}", - e - ); + Ok(funding_template) => { + let outputs = vec![TxOut { + value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), + script_pubkey: coinbase_tx.output[1].script_pubkey.clone(), + }]; + let wallet = WalletSync::new(&wallets[1], Arc::clone(&loggers[1])); + if let Ok(contribution) = funding_template.splice_out_sync(outputs, &wallet) { + let _ = nodes[1].funding_contributed(&chan_a_id, &nodes[0].get_our_node_id(), contribution, None); + } + }, + Err(e) => { + assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + }, } } }, @@ -2275,24 +2234,28 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { .map(|chan| chan.outbound_capacity_msat) .unwrap(); if outbound_capacity_msat >= 20_000_000 { - let contribution = SpliceContribution::splice_out(vec![TxOut { - value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), - script_pubkey: coinbase_tx.output[1].script_pubkey.clone(), - }]); - let funding_feerate_sat_per_kw = - fee_est_b.ret_val.load(atomic::Ordering::Acquire); - if let Err(e) = nodes[1].splice_channel( + let feerate_sat_per_kw = + fee_estimators[1].ret_val.load(atomic::Ordering::Acquire); + let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); + match nodes[1].splice_channel( &chan_b_id, &nodes[2].get_our_node_id(), - contribution, - funding_feerate_sat_per_kw, None, + feerate, ) { - assert!( - matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice pending")), - "{:?}", - e - ); + Ok(funding_template) => { + let outputs = vec![TxOut { + value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), + script_pubkey: coinbase_tx.output[1].script_pubkey.clone(), + }]; + let wallet = WalletSync::new(&wallets[1], Arc::clone(&loggers[1])); + if let Ok(contribution) = funding_template.splice_out_sync(outputs, &wallet) { + let _ = nodes[1].funding_contributed(&chan_b_id, &nodes[2].get_our_node_id(), contribution, None); + } + }, + Err(e) => { + assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + }, } } }, @@ -2304,24 +2267,28 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { .map(|chan| chan.outbound_capacity_msat) .unwrap(); if outbound_capacity_msat >= 20_000_000 { - let contribution = SpliceContribution::splice_out(vec![TxOut { - value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), - script_pubkey: coinbase_tx.output[2].script_pubkey.clone(), - }]); - let funding_feerate_sat_per_kw = - fee_est_c.ret_val.load(atomic::Ordering::Acquire); - if let Err(e) = nodes[2].splice_channel( + let feerate_sat_per_kw = + fee_estimators[2].ret_val.load(atomic::Ordering::Acquire); + let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); + match nodes[2].splice_channel( &chan_b_id, &nodes[1].get_our_node_id(), - contribution, - funding_feerate_sat_per_kw, None, + feerate, ) { - assert!( - matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice pending")), - "{:?}", - e - ); + Ok(funding_template) => { + let outputs = vec![TxOut { + value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), + script_pubkey: coinbase_tx.output[2].script_pubkey.clone(), + }]; + let wallet = WalletSync::new(&wallets[2], Arc::clone(&loggers[2])); + if let Ok(contribution) = funding_template.splice_out_sync(outputs, &wallet) { + let _ = nodes[2].funding_contributed(&chan_b_id, &nodes[1].get_our_node_id(), contribution, None); + } + }, + Err(e) => { + assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + }, } } }, @@ -2609,7 +2576,7 @@ impl SearchingOutput { } } -pub fn chanmon_consistency_test(data: &[u8], out: Out) { +pub fn chanmon_consistency_test(data: &[u8], out: Out) { do_test(data, out.clone(), false); do_test(data, out, true); } diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 11eca60151e..ebbd93f0052 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -22,6 +22,7 @@ use bitcoin::opcodes; use bitcoin::script::{Builder, ScriptBuf}; use bitcoin::transaction::Version; use bitcoin::transaction::{Transaction, TxIn, TxOut}; +use bitcoin::FeeRate; use bitcoin::hash_types::{BlockHash, Txid}; use bitcoin::hashes::sha256::Hash as Sha256; @@ -30,8 +31,6 @@ use bitcoin::hashes::Hash as _; use bitcoin::hex::FromHex; use bitcoin::WPubkeyHash; -use lightning::ln::funding::{FundingTxInput, SpliceContribution}; - use lightning::blinded_path::message::{BlindedMessagePath, MessageContext, MessageForwardNode}; use lightning::blinded_path::payment::{BlindedPaymentPath, ReceiveTlvs}; use lightning::chain; @@ -41,7 +40,7 @@ use lightning::chain::chaininterface::{ use lightning::chain::chainmonitor; use lightning::chain::transaction::OutPoint; use lightning::chain::{BestBlock, ChannelMonitorUpdateStatus, Confirm, Listen}; -use lightning::events::bump_transaction::sync::WalletSourceSync; +use lightning::events::bump_transaction::sync::{WalletSourceSync, WalletSync}; use lightning::events::Event; use lightning::ln::channel_state::ChannelDetails; use lightning::ln::channelmanager::{ChainParameters, ChannelManager, InterceptId, PaymentId}; @@ -65,6 +64,7 @@ use lightning::sign::{ SignerProvider, }; use lightning::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret}; +use lightning::util::async_poll::{MaybeSend, MaybeSync}; use lightning::util::config::{ChannelConfig, UserConfig}; use lightning::util::hash_tables::*; use lightning::util::logger::Logger; @@ -227,7 +227,7 @@ type ChannelMan<'a> = ChannelManager< Arc, Arc, Arc, - Arc, + Arc, Arc, Arc, >, @@ -239,14 +239,20 @@ type ChannelMan<'a> = ChannelManager< Arc, &'a FuzzRouter, &'a FuzzRouter, - Arc, + Arc, >; type PeerMan<'a> = PeerManager< Peer<'a>, Arc>, - Arc>>, Arc, Arc>>, + Arc< + P2PGossipSync< + Arc>>, + Arc, + Arc, + >, + >, IgnoringMessageHandler, - Arc, + Arc, IgnoringMessageHandler, Arc, IgnoringMessageHandler, @@ -260,7 +266,7 @@ struct MoneyLossDetector<'a> { Arc, Arc, Arc, - Arc, + Arc, Arc, Arc, >, @@ -285,7 +291,7 @@ impl<'a> MoneyLossDetector<'a> { Arc, Arc, Arc, - Arc, + Arc, Arc, Arc, >, @@ -520,7 +526,7 @@ impl SignerProvider for KeyProvider { } #[inline] -pub fn do_test(mut data: &[u8], logger: &Arc) { +pub fn do_test(mut data: &[u8], logger: &Arc) { if data.len() < 32 { return; } @@ -1024,20 +1030,27 @@ pub fn do_test(mut data: &[u8], logger: &Arc) { if splice_in_sats == 0 { continue; } - // Create a funding input from the coinbase transaction - if let Ok(input) = FundingTxInput::new_p2wpkh(coinbase_tx.clone(), 0) { - let contribution = SpliceContribution::splice_in( - Amount::from_sat(splice_in_sats.min(900_000)), // Cap at available funds minus fees - vec![input], - Some(wallet.get_change_script().unwrap()), - ); - let _ = channelmanager.splice_channel( - &chan.channel_id, - &chan.counterparty.node_id, - contribution, - 253, // funding_feerate_per_kw + let chan_id = chan.channel_id; + let counterparty = chan.counterparty.node_id; + if let Ok(funding_template) = channelmanager.splice_channel( + &chan_id, + &counterparty, + None, + FeeRate::from_sat_per_kwu(253), + ) { + let wallet_sync = WalletSync::new(&wallet, Arc::clone(&logger)); + if let Ok(contribution) = funding_template.splice_in_sync( None, - ); + Amount::from_sat(splice_in_sats.min(900_000)), + &wallet_sync, + ) { + let _ = channelmanager.funding_contributed( + &chan_id, + &counterparty, + contribution, + None, + ); + } } }, // Splice-out: remove funds from a channel @@ -1060,17 +1073,28 @@ pub fn do_test(mut data: &[u8], logger: &Arc) { // Cap splice-out at a reasonable portion of channel capacity let max_splice_out = chan.channel_value_satoshis / 4; let splice_out_sats = splice_out_sats.min(max_splice_out).max(546); // At least dust limit - let contribution = SpliceContribution::splice_out(vec![TxOut { - value: Amount::from_sat(splice_out_sats), - script_pubkey: wallet.get_change_script().unwrap(), - }]); - let _ = channelmanager.splice_channel( - &chan.channel_id, - &chan.counterparty.node_id, - contribution, - 253, // funding_feerate_per_kw + let chan_id = chan.channel_id; + let counterparty = chan.counterparty.node_id; + if let Ok(funding_template) = channelmanager.splice_channel( + &chan_id, + &counterparty, None, - ); + FeeRate::from_sat_per_kwu(253), + ) { + let outputs = vec![TxOut { + value: Amount::from_sat(splice_out_sats), + script_pubkey: wallet.get_change_script().unwrap(), + }]; + let wallet_sync = WalletSync::new(&wallet, Arc::clone(&logger)); + if let Ok(contribution) = funding_template.splice_out_sync(outputs, &wallet_sync) { + let _ = channelmanager.funding_contributed( + &chan_id, + &counterparty, + contribution, + None, + ); + } + } }, _ => return, } @@ -1137,14 +1161,15 @@ pub fn do_test(mut data: &[u8], logger: &Arc) { } } -pub fn full_stack_test(data: &[u8], out: Out) { - let logger: Arc = Arc::new(test_logger::TestLogger::new("".to_owned(), out)); +pub fn full_stack_test(data: &[u8], out: Out) { + let logger: Arc = + Arc::new(test_logger::TestLogger::new("".to_owned(), out)); do_test(data, &logger); } #[no_mangle] pub extern "C" fn full_stack_run(data: *const u8, datalen: usize) { - let logger: Arc = + let logger: Arc = Arc::new(test_logger::TestLogger::new("".to_owned(), test_logger::DevNull {})); do_test(unsafe { std::slice::from_raw_parts(data, datalen) }, &logger); } @@ -1930,6 +1955,7 @@ pub fn write_fst_seeds(path: &str) { #[cfg(test)] mod tests { + use lightning::util::async_poll::{MaybeSend, MaybeSync}; use lightning::util::logger::{Logger, Record}; use std::collections::HashMap; use std::sync::{Arc, Mutex}; @@ -1961,7 +1987,7 @@ mod tests { let test = super::two_peer_forwarding_seed(); let logger = Arc::new(TrackingLogger { lines: Mutex::new(HashMap::new()) }); - super::do_test(&test, &(Arc::clone(&logger) as Arc)); + super::do_test(&test, &(Arc::clone(&logger) as Arc)); let log_entries = logger.lines.lock().unwrap(); // 1 @@ -1996,7 +2022,7 @@ mod tests { let test = super::gossip_exchange_seed(); let logger = Arc::new(TrackingLogger { lines: Mutex::new(HashMap::new()) }); - super::do_test(&test, &(Arc::clone(&logger) as Arc)); + super::do_test(&test, &(Arc::clone(&logger) as Arc)); let log_entries = logger.lines.lock().unwrap(); assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Sending message to all peers except Some(PublicKey(0000000000000000000000000000000000000000000000000000000000000002ff00000000000000000000000000000000000000000000000000000000000002)) or the announced channel's counterparties: ChannelAnnouncement { node_signature_1: 3026020200b202200303030303030303030303030303030303030303030303030303030303030303, node_signature_2: 3026020200b202200202020202020202020202020202020202020202020202020202020202020202, bitcoin_signature_1: 3026020200b202200303030303030303030303030303030303030303030303030303030303030303, bitcoin_signature_2: 3026020200b202200202020202020202020202020202020202020202020202020202020202020202, contents: UnsignedChannelAnnouncement { features: [], chain_hash: 6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000, short_channel_id: 42, node_id_1: NodeId(030303030303030303030303030303030303030303030303030303030303030303), node_id_2: NodeId(020202020202020202020202020202020202020202020202020202020202020202), bitcoin_key_1: NodeId(030303030303030303030303030303030303030303030303030303030303030303), bitcoin_key_2: NodeId(020202020202020202020202020202020202020202020202020202020202020202), excess_data: [] } }".to_string())), Some(&1)); @@ -2009,7 +2035,7 @@ mod tests { let test = super::splice_seed(); let logger = Arc::new(TrackingLogger { lines: Mutex::new(HashMap::new()) }); - super::do_test(&test, &(Arc::clone(&logger) as Arc)); + super::do_test(&test, &(Arc::clone(&logger) as Arc)); let log_entries = logger.lines.lock().unwrap(); diff --git a/lightning-tests/src/upgrade_downgrade_tests.rs b/lightning-tests/src/upgrade_downgrade_tests.rs index 14b0a5c5822..dde194105c3 100644 --- a/lightning-tests/src/upgrade_downgrade_tests.rs +++ b/lightning-tests/src/upgrade_downgrade_tests.rs @@ -49,7 +49,6 @@ use lightning::chain::channelmonitor::{ANTI_REORG_DELAY, HTLC_FAIL_BACK_BUFFER}; use lightning::events::bump_transaction::sync::WalletSourceSync; use lightning::events::{ClosureReason, Event, HTLCHandlingFailureType}; use lightning::ln::functional_test_utils::*; -use lightning::ln::funding::SpliceContribution; use lightning::ln::msgs::BaseMessageHandler as _; use lightning::ln::msgs::ChannelMessageHandler as _; use lightning::ln::msgs::MessageSendEvent; @@ -453,11 +452,13 @@ fn do_test_0_1_htlc_forward_after_splice(fail_htlc: bool) { reconnect_b_c_args.send_announcement_sigs = (true, true); reconnect_nodes(reconnect_b_c_args); - let contribution = SpliceContribution::splice_out(vec![TxOut { + let outputs = vec![TxOut { value: Amount::from_sat(1_000), script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), - }]); - let splice_tx = splice_channel(&nodes[0], &nodes[1], ChannelId(chan_id_bytes_a), contribution); + }]; + let channel_id = ChannelId(chan_id_bytes_a); + let funding_contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs); + let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); for node in nodes.iter() { mine_transaction(node, &splice_tx); connect_blocks(node, ANTI_REORG_DELAY - 1); diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index b81279c10ac..f34a2b3275c 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -11,8 +11,7 @@ //! properly with a signer implementation that asynchronously derives signatures. use crate::events::bump_transaction::sync::WalletSourceSync; -use crate::ln::funding::SpliceContribution; -use crate::ln::splicing_tests::negotiate_splice_tx; +use crate::ln::splicing_tests::{initiate_splice_out, negotiate_splice_tx}; use crate::prelude::*; use crate::util::ser::Writeable; use bitcoin::secp256k1::Secp256k1; @@ -1573,10 +1572,11 @@ fn test_async_splice_initial_commit_sig() { ); // Negotiate a splice up until the signature exchange. - let contribution = SpliceContribution::splice_out(vec![TxOut { + let outputs = vec![TxOut { value: Amount::from_sat(1_000), script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), - }]); + }]; + let contribution = initiate_splice_out(initiator, acceptor, channel_id, outputs); negotiate_splice_tx(initiator, acceptor, channel_id, contribution); assert!(initiator.node.get_and_clear_pending_msg_events().is_empty()); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 48b52992953..e000ebc93eb 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -11,7 +11,7 @@ use bitcoin::absolute::LockTime; use bitcoin::amount::{Amount, SignedAmount}; use bitcoin::consensus::encode; use bitcoin::constants::ChainHash; -use bitcoin::script::{Builder, Script, ScriptBuf, WScriptHash}; +use bitcoin::script::{Builder, Script, ScriptBuf}; use bitcoin::sighash::EcdsaSighashType; use bitcoin::transaction::{Transaction, TxOut}; use bitcoin::Witness; @@ -36,6 +36,7 @@ use crate::chain::channelmonitor::{ }; use crate::chain::transaction::{OutPoint, TransactionData}; use crate::chain::BestBlock; +use crate::events::bump_transaction::Input; use crate::events::{ClosureReason, FundingInfo}; use crate::ln::chan_utils; use crate::ln::chan_utils::{ @@ -43,7 +44,7 @@ use crate::ln::chan_utils::{ selected_commitment_sat_per_1000_weight, ChannelPublicKeys, ChannelTransactionParameters, ClosingTransaction, CommitmentTransaction, CounterpartyChannelTransactionParameters, CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HolderCommitmentTransaction, - BASE_INPUT_WEIGHT, EMPTY_SCRIPT_SIG_WEIGHT, FUNDING_TRANSACTION_WITNESS_WEIGHT, + EMPTY_SCRIPT_SIG_WEIGHT, FUNDING_TRANSACTION_WITNESS_WEIGHT, }; use crate::ln::channel_state::{ ChannelShutdownState, CounterpartyForwardingInfo, InboundHTLCDetails, InboundHTLCStateDetails, @@ -55,12 +56,11 @@ use crate::ln::channelmanager::{ PendingHTLCStatus, RAACommitmentOrder, SentHTLCId, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, }; -use crate::ln::funding::{FundingTxInput, SpliceContribution}; +use crate::ln::funding::{FundingContribution, FundingTemplate, FundingTxInput}; use crate::ln::interactivetxs::{ calculate_change_output_value, get_output_weight, AbortReason, HandleTxCompleteValue, InteractiveTxConstructor, InteractiveTxConstructorArgs, InteractiveTxMessageSend, InteractiveTxSigningSession, NegotiationError, SharedOwnedInput, SharedOwnedOutput, - TX_COMMON_FIELDS_WEIGHT, }; use crate::ln::msgs; use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError, OnionErrorPacket}; @@ -69,7 +69,6 @@ use crate::ln::onion_utils::{ }; use crate::ln::script::{self, ShutdownScript}; use crate::ln::types::ChannelId; -use crate::ln::LN_MAX_MSG_LEN; use crate::offers::static_invoice::StaticInvoice; use crate::routing::gossip::NodeId; use crate::sign::ecdsa::EcdsaChannelSigner; @@ -2686,6 +2685,20 @@ impl FundingScope { self.channel_transaction_parameters.funding_outpoint } + /// Gets the funding output for this channel, if available. + /// + /// When a channel is spliced, this continues to refer to the original funding output (which + /// was spent by the splice transaction) until the splice transaction reaches sufficient + /// confirmations to be locked (and we exchange `splice_locked` messages with our peer). + pub fn get_funding_output(&self) -> Option { + self.channel_transaction_parameters.make_funding_redeemscript_opt().map(|redeem_script| { + TxOut { + value: Amount::from_sat(self.get_value_satoshis()), + script_pubkey: redeem_script.to_p2wsh(), + } + }) + } + fn get_funding_txid(&self) -> Option { self.channel_transaction_parameters.funding_outpoint.map(|txo| txo.txid) } @@ -3010,7 +3023,12 @@ impl_writeable_tlv_based!(SpliceInstructions, { #[derive(Debug)] pub(crate) enum QuiescentAction { - Splice(SpliceInstructions), + // Deprecated in favor of the Splice variant and no longer produced as of LDK 0.3. + LegacySplice(SpliceInstructions), + Splice { + contribution: FundingContribution, + locktime: LockTime, + }, #[cfg(any(test, fuzzing))] DoNothing, } @@ -3023,11 +3041,19 @@ pub(crate) enum StfuResponse { #[cfg(any(test, fuzzing))] impl_writeable_tlv_based_enum_upgradable!(QuiescentAction, (0, DoNothing) => {}, - {1, Splice} => (), + (2, Splice) => { + (0, contribution, required), + (1, locktime, required), + }, + {1, LegacySplice} => (), ); #[cfg(not(any(test, fuzzing)))] -impl_writeable_tlv_based_enum_upgradable!(QuiescentAction,, - {1, Splice} => (), +impl_writeable_tlv_based_enum_upgradable!(QuiescentAction, + (2, Splice) => { + (0, contribution, required), + (1, locktime, required), + }, + {1, LegacySplice} => (), ); /// Wrapper around a [`Transaction`] useful for caching the result of [`Transaction::compute_txid`]. @@ -6632,130 +6658,6 @@ fn get_v2_channel_reserve_satoshis(channel_value_satoshis: u64, dust_limit_satos cmp::min(channel_value_satoshis, cmp::max(q, dust_limit_satoshis)) } -fn check_splice_contribution_sufficient( - contribution: &SpliceContribution, is_initiator: bool, funding_feerate: FeeRate, -) -> Result { - if contribution.inputs().is_empty() { - let estimated_fee = Amount::from_sat(estimate_v2_funding_transaction_fee( - contribution.inputs(), - contribution.outputs(), - is_initiator, - true, // is_splice - funding_feerate.to_sat_per_kwu() as u32, - )); - - let contribution_amount = contribution.net_value(); - contribution_amount - .checked_sub( - estimated_fee.to_signed().expect("fees should never exceed Amount::MAX_MONEY"), - ) - .ok_or(format!( - "{estimated_fee} splice-out amount plus {} fee estimate exceeds the total bitcoin supply", - contribution_amount.unsigned_abs(), - )) - } else { - check_v2_funding_inputs_sufficient( - contribution.value_added(), - contribution.inputs(), - contribution.outputs(), - is_initiator, - true, - funding_feerate.to_sat_per_kwu() as u32, - ) - .map(|_| contribution.net_value()) - } -} - -/// Estimate our part of the fee of the new funding transaction. -#[allow(dead_code)] // TODO(dual_funding): TODO(splicing): Remove allow once used. -#[rustfmt::skip] -fn estimate_v2_funding_transaction_fee( - funding_inputs: &[FundingTxInput], outputs: &[TxOut], is_initiator: bool, is_splice: bool, - funding_feerate_sat_per_1000_weight: u32, -) -> u64 { - let input_weight: u64 = funding_inputs - .iter() - .map(|input| BASE_INPUT_WEIGHT.saturating_add(input.utxo.satisfaction_weight)) - .fold(0, |total_weight, input_weight| total_weight.saturating_add(input_weight)); - - let output_weight: u64 = outputs - .iter() - .map(|txout| txout.weight().to_wu()) - .fold(0, |total_weight, output_weight| total_weight.saturating_add(output_weight)); - - let mut weight = input_weight.saturating_add(output_weight); - - // The initiator pays for all common fields and the shared output in the funding transaction. - if is_initiator { - weight = weight - .saturating_add(TX_COMMON_FIELDS_WEIGHT) - // The weight of the funding output, a P2WSH output - // NOTE: The witness script hash given here is irrelevant as it's a fixed size and we just want - // to calculate the contributed weight, so we use an all-zero hash. - .saturating_add(get_output_weight(&ScriptBuf::new_p2wsh( - &WScriptHash::from_raw_hash(Hash::all_zeros()) - )).to_wu()); - - // The splice initiator pays for the input spending the previous funding output. - if is_splice { - weight = weight - .saturating_add(BASE_INPUT_WEIGHT) - .saturating_add(EMPTY_SCRIPT_SIG_WEIGHT) - .saturating_add(FUNDING_TRANSACTION_WITNESS_WEIGHT); - #[cfg(feature = "grind_signatures")] - { - // Guarantees a low R signature - weight -= 1; - } - } - } - - fee_for_weight(funding_feerate_sat_per_1000_weight, weight) -} - -/// Verify that the provided inputs to the funding transaction are enough -/// to cover the intended contribution amount *plus* the proportional fees. -/// Fees are computed using `estimate_v2_funding_transaction_fee`, and contain -/// the fees of the inputs, fees of the inputs weight, and for the initiator, -/// the fees of the common fields as well as the output and extra input weights. -/// Returns estimated (partial) fees as additional information -#[rustfmt::skip] -fn check_v2_funding_inputs_sufficient( - contributed_input_value: Amount, funding_inputs: &[FundingTxInput], outputs: &[TxOut], - is_initiator: bool, is_splice: bool, funding_feerate_sat_per_1000_weight: u32, -) -> Result { - let estimated_fee = Amount::from_sat(estimate_v2_funding_transaction_fee( - funding_inputs, outputs, is_initiator, is_splice, funding_feerate_sat_per_1000_weight, - )); - - let mut total_input_value = Amount::ZERO; - for FundingTxInput { utxo, .. } in funding_inputs.iter() { - total_input_value = total_input_value.checked_add(utxo.output.value) - .ok_or("Sum of input values is greater than the total bitcoin supply")?; - } - - // If the inputs are enough to cover intended contribution amount, with fees even when - // there is a change output, we are fine. - // If the inputs are less, but enough to cover intended contribution amount, with - // (lower) fees with no change, we are also fine (change will not be generated). - // So it's enough to check considering the lower, no-change fees. - // - // Note: dust limit is not relevant in this check. - // - // TODO(splicing): refine check including the fact wether a change will be added or not. - // Can be done once dual funding preparation is included. - - let minimal_input_amount_needed = contributed_input_value.checked_add(estimated_fee) - .ok_or(format!("{contributed_input_value} contribution plus {estimated_fee} fee estimate exceeds the total bitcoin supply"))?; - if total_input_value < minimal_input_amount_needed { - Err(format!( - "Total input amount {total_input_value} is lower than needed for splice-in contribution {contributed_input_value}, considering fees of {estimated_fee}. Need more inputs.", - )) - } else { - Ok(estimated_fee) - } -} - /// Context for negotiating channels (dual-funded V2 open, splicing) #[derive(Debug)] pub(super) struct FundingNegotiationContext { @@ -7121,7 +7023,7 @@ where self.reset_pending_splice_state() } else { match self.quiescent_action.take() { - Some(QuiescentAction::Splice(instructions)) => { + Some(QuiescentAction::LegacySplice(instructions)) => { self.context.channel_state.clear_awaiting_quiescence(); let (inputs, outputs) = instructions.into_contributed_inputs_and_outputs(); Some(SpliceFundingFailed { @@ -7131,6 +7033,16 @@ where contributed_outputs: outputs, }) }, + Some(QuiescentAction::Splice { contribution, .. }) => { + self.context.channel_state.clear_awaiting_quiescence(); + let (inputs, outputs) = contribution.into_contributed_inputs_and_outputs(); + Some(SpliceFundingFailed { + funding_txo: None, + channel_type: None, + contributed_inputs: inputs, + contributed_outputs: outputs, + }) + }, #[cfg(any(test, fuzzing))] Some(quiescent_action) => { self.quiescent_action = Some(quiescent_action); @@ -11551,7 +11463,12 @@ where self.get_announcement_sigs(node_signer, chain_hash, user_config, block_height, logger); if let Some(quiescent_action) = self.quiescent_action.as_ref() { - if matches!(quiescent_action, QuiescentAction::Splice(_)) { + // TODO(splicing): If we didn't win quiescence, then we can contribute as an acceptor + // instead of waiting for the splice to lock. + if matches!( + quiescent_action, + QuiescentAction::Splice { .. } | QuiescentAction::LegacySplice(_) + ) { self.context.channel_state.set_awaiting_quiescence(); } } @@ -12196,14 +12113,7 @@ where } /// Initiate splicing. - /// - `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). - /// - `change_script`: an option change output script. If `None` and needed, one will be - /// generated by `SignerProvider::get_destination_script`. - pub fn splice_channel( - &mut self, contribution: SpliceContribution, funding_feerate_per_kw: u32, locktime: u32, - logger: &L, - ) -> Result, APIError> { + pub fn splice_channel(&mut self, feerate: FeeRate) -> Result { if self.holder_commitment_point.current_point().is_none() { return Err(APIError::APIMisuseError { err: format!( @@ -12213,17 +12123,29 @@ where }); } - // Check if a splice has been initiated already. - // Note: only a single outstanding splice is supported (per spec) - if self.pending_splice.is_some() || self.quiescent_action.is_some() { + if self.quiescent_action.is_some() { return Err(APIError::APIMisuseError { err: format!( - "Channel {} cannot be spliced, as it has already a splice pending", + "Channel {} cannot be spliced as one is waiting to be negotiated", self.context.channel_id(), ), }); } + if let Some(pending_splice) = &self.pending_splice { + if let Some(funding_negotiation) = &pending_splice.funding_negotiation { + debug_assert!(self.context.channel_state.is_quiescent()); + if funding_negotiation.is_initiator() { + return Err(APIError::APIMisuseError { + err: format!( + "Channel {} cannot be spliced as one is currently being negotiated", + self.context.channel_id(), + ), + }); + } + } + } + if !self.context.is_usable() { return Err(APIError::APIMisuseError { err: format!( @@ -12233,81 +12155,68 @@ where }); } - let our_funding_contribution = contribution.net_value(); - if our_funding_contribution == SignedAmount::ZERO { - return Err(APIError::APIMisuseError { - err: format!( - "Channel {} cannot be spliced; contribution cannot be zero", - self.context.channel_id(), - ), - }); - } + let funding_txo = self.funding.get_funding_txo().expect("funding_txo should be set"); + let previous_utxo = + self.funding.get_funding_output().expect("funding_output should be set"); + let shared_input = Input { + outpoint: funding_txo.into_bitcoin_outpoint(), + previous_utxo, + satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + FUNDING_TRANSACTION_WITNESS_WEIGHT, + }; - // Fees for splice-out are paid from the channel balance whereas fees for splice-in - // are paid by the funding inputs. Therefore, in the case of splice-out, we add the - // fees on top of the user-specified contribution. We leave the user-specified - // contribution as-is for splice-ins. - let adjusted_funding_contribution = check_splice_contribution_sufficient( - &contribution, - true, - FeeRate::from_sat_per_kwu(u64::from(funding_feerate_per_kw)), - ) - .map_err(|e| APIError::APIMisuseError { - err: format!( - "Channel {} cannot be {}; {}", - self.context.channel_id(), - if our_funding_contribution.is_positive() { "spliced in" } else { "spliced out" }, - e - ), - })?; + Ok(FundingTemplate::new(Some(shared_input), feerate, true)) + } - // Note: post-splice channel value is not yet known at this point, counterparty contribution is not known - // (Cannot test for miminum required post-splice channel value) - let their_funding_contribution = SignedAmount::ZERO; - self.validate_splice_contributions( - adjusted_funding_contribution, - their_funding_contribution, - ) - .map_err(|err| APIError::APIMisuseError { err })?; - - for FundingTxInput { utxo, prevtx, .. } in contribution.inputs().iter() { - const MESSAGE_TEMPLATE: msgs::TxAddInput = msgs::TxAddInput { - channel_id: ChannelId([0; 32]), - serial_id: 0, - prevtx: None, - prevtx_out: 0, - sequence: 0, - // Mutually exclusive with prevtx, which is accounted for below. - shared_input_txid: None, - }; - let message_len = MESSAGE_TEMPLATE.serialized_length() + prevtx.serialized_length(); - if message_len > LN_MAX_MSG_LEN { - return Err(APIError::APIMisuseError { - err: format!( - "Funding input references a prevtx that is too large for tx_add_input: {}", - utxo.outpoint, - ), - }); - } - } + pub fn funding_contributed( + &mut self, contribution: FundingContribution, locktime: LockTime, logger: &L, + ) -> Result, SpliceFundingFailed> { + debug_assert!(contribution.is_splice()); - let (our_funding_inputs, our_funding_outputs, change_script) = contribution.into_tx_parts(); + if let Err(e) = contribution.net_value().and_then(|our_funding_contribution| { + // For splice-out, our_funding_contribution is adjusted to cover fees if there + // aren't any inputs. + self.validate_splice_contributions(our_funding_contribution, SignedAmount::ZERO) + }) { + log_error!(logger, "Channel {} cannot be funded: {}", self.context.channel_id(), e); - let action = QuiescentAction::Splice(SpliceInstructions { - adjusted_funding_contribution, - our_funding_inputs, - our_funding_outputs, - change_script, - funding_feerate_per_kw, - locktime, - }); - self.propose_quiescence(logger, action) - .map_err(|e| APIError::APIMisuseError { err: e.to_owned() }) + let (contributed_inputs, contributed_outputs) = + contribution.into_contributed_inputs_and_outputs(); + + return Err(SpliceFundingFailed { + funding_txo: None, + channel_type: None, + contributed_inputs, + contributed_outputs, + }); + } + + self.propose_quiescence(logger, QuiescentAction::Splice { contribution, locktime }).map_err( + |(e, action)| { + log_error!(logger, "{}", e); + // FIXME: Any better way to do this? + if let QuiescentAction::Splice { contribution, .. } = action { + let (contributed_inputs, contributed_outputs) = + contribution.into_contributed_inputs_and_outputs(); + SpliceFundingFailed { + funding_txo: None, + channel_type: None, + contributed_inputs, + contributed_outputs, + } + } else { + debug_assert!(false); + SpliceFundingFailed { + funding_txo: None, + channel_type: None, + contributed_inputs: vec![], + contributed_outputs: vec![], + } + } + }, + ) } fn send_splice_init(&mut self, instructions: SpliceInstructions) -> msgs::SpliceInit { - debug_assert!(self.pending_splice.is_none()); - let SpliceInstructions { adjusted_funding_contribution, our_funding_inputs, @@ -12329,6 +12238,13 @@ where change_script, }; + self.send_splice_init_internal(context) + } + + fn send_splice_init_internal( + &mut self, context: FundingNegotiationContext, + ) -> msgs::SpliceInit { + debug_assert!(self.pending_splice.is_none()); // Rotate the funding pubkey using the prev_funding_txid as a tweak let prev_funding_txid = self.funding.get_funding_txid(); let funding_pubkey = match (prev_funding_txid, &self.context.holder_signer) { @@ -12343,6 +12259,10 @@ where _ => todo!(), }; + let funding_feerate_per_kw = context.funding_feerate_sat_per_1000_weight; + let funding_contribution_satoshis = context.our_funding_contribution.to_sat(); + let locktime = context.funding_tx_locktime.to_consensus_u32(); + let funding_negotiation = FundingNegotiation::AwaitingAck { context, new_holder_funding_key: funding_pubkey }; self.pending_splice = Some(PendingFunding { @@ -12354,7 +12274,7 @@ where msgs::SpliceInit { channel_id: self.context.channel_id, - funding_contribution_satoshis: adjusted_funding_contribution.to_sat(), + funding_contribution_satoshis, funding_feerate_per_kw, locktime, funding_pubkey, @@ -12421,7 +12341,7 @@ where } // TODO(splicing): Once splice acceptor can contribute, check that inputs are sufficient, - // similarly to the check in `splice_channel`. + // similarly to the check in `funding_contributed`. debug_assert_eq!(our_funding_contribution, SignedAmount::ZERO); let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis); @@ -13408,14 +13328,14 @@ where #[rustfmt::skip] pub fn propose_quiescence( &mut self, logger: &L, action: QuiescentAction, - ) -> Result, &'static str> { + ) -> Result, (&'static str, QuiescentAction)> { log_debug!(logger, "Attempting to initiate quiescence"); if !self.context.is_usable() { - return Err("Channel is not in a usable state to propose quiescence"); + return Err(("Channel is not in a usable state to propose quiescence", action)); } if self.quiescent_action.is_some() { - return Err("Channel already has a pending quiescent action and cannot start another"); + return Err(("Channel already has a pending quiescent action and cannot start another", action)); } self.quiescent_action = Some(action); @@ -13556,9 +13476,10 @@ where "Internal Error: Didn't have anything to do after reaching quiescence".to_owned() )); }, - Some(QuiescentAction::Splice(instructions)) => { + Some(QuiescentAction::LegacySplice(instructions)) => { if self.pending_splice.is_some() { - self.quiescent_action = Some(QuiescentAction::Splice(instructions)); + debug_assert!(false); + self.quiescent_action = Some(QuiescentAction::LegacySplice(instructions)); return Err(ChannelError::WarnAndDisconnect( format!( @@ -13571,6 +13492,53 @@ where let splice_init = self.send_splice_init(instructions); return Ok(Some(StfuResponse::SpliceInit(splice_init))); }, + Some(QuiescentAction::Splice { contribution, locktime }) => { + // TODO(splicing): If the splice has been negotiated but has not been locked, we + // can RBF here to add the contribution. + if self.pending_splice.is_some() { + debug_assert!(false); + self.quiescent_action = + Some(QuiescentAction::Splice { contribution, locktime }); + + return Err(ChannelError::WarnAndDisconnect( + format!( + "Channel {} cannot be spliced as it already has a splice pending", + self.context.channel_id(), + ), + )); + } + + let prev_funding_input = self.funding.to_splice_funding_input(); + let is_initiator = contribution.is_initiator(); + let our_funding_contribution = match contribution.net_value() { + Ok(net_value) => net_value, + Err(e) => { + debug_assert!(false); + return Err(ChannelError::WarnAndDisconnect( + format!( + "Internal Error: Insufficient funding contribution: {}", + e, + ) + )); + }, + }; + let funding_feerate_per_kw = contribution.feerate().to_sat_per_kwu() as u32; + let (our_funding_inputs, our_funding_outputs, change_script) = contribution.into_tx_parts(); + + let context = FundingNegotiationContext { + is_initiator, + our_funding_contribution, + funding_tx_locktime: locktime, + funding_feerate_sat_per_1000_weight: funding_feerate_per_kw, + shared_funding_input: Some(prev_funding_input), + our_funding_inputs, + our_funding_outputs, + change_script, + }; + + let splice_init = self.send_splice_init_internal(context); + return Ok(Some(StfuResponse::SpliceInit(splice_init))); + }, #[cfg(any(test, fuzzing))] Some(QuiescentAction::DoNothing) => { // In quiescence test we want to just hang out here, letting the test manually @@ -16130,7 +16098,6 @@ mod tests { }; use crate::ln::channel_keys::{RevocationBasepoint, RevocationKey}; use crate::ln::channelmanager::{self, HTLCSource, PaymentId}; - use crate::ln::funding::FundingTxInput; use crate::ln::msgs; use crate::ln::msgs::{ChannelUpdate, UnsignedChannelUpdate, MAX_VALUE_MSAT}; use crate::ln::onion_utils::{AttributionData, LocalHTLCFailureReason}; @@ -16162,7 +16129,7 @@ mod tests { use bitcoin::secp256k1::{ecdsa::Signature, Secp256k1}; use bitcoin::secp256k1::{PublicKey, SecretKey}; use bitcoin::transaction::{Transaction, TxOut, Version}; - use bitcoin::{ScriptBuf, WPubkeyHash, WitnessProgram, WitnessVersion}; + use bitcoin::{WitnessProgram, WitnessVersion}; use std::cmp; fn dummy_inbound_update_add() -> InboundUpdateAdd { @@ -18510,250 +18477,6 @@ mod tests { assert!(node_a_chan.check_get_channel_ready(0, &&logger).is_some()); } - #[test] - #[rustfmt::skip] - fn test_estimate_v2_funding_transaction_fee() { - use crate::ln::channel::estimate_v2_funding_transaction_fee; - - let one_input = [funding_input_sats(1_000)]; - let two_inputs = [funding_input_sats(1_000), funding_input_sats(1_000)]; - - // 2 inputs, initiator, 2000 sat/kw feerate - assert_eq!( - estimate_v2_funding_transaction_fee(&two_inputs, &[], true, false, 2000), - if cfg!(feature = "grind_signatures") { 1512 } else { 1516 }, - ); - - // higher feerate - assert_eq!( - estimate_v2_funding_transaction_fee(&two_inputs, &[], true, false, 3000), - if cfg!(feature = "grind_signatures") { 2268 } else { 2274 }, - ); - - // only 1 input - assert_eq!( - estimate_v2_funding_transaction_fee(&one_input, &[], true, false, 2000), - if cfg!(feature = "grind_signatures") { 970 } else { 972 }, - ); - - // 0 inputs - assert_eq!( - estimate_v2_funding_transaction_fee(&[], &[], true, false, 2000), - 428, - ); - - // not initiator - assert_eq!( - estimate_v2_funding_transaction_fee(&[], &[], false, false, 2000), - 0, - ); - - // splice initiator - assert_eq!( - estimate_v2_funding_transaction_fee(&one_input, &[], true, true, 2000), - if cfg!(feature = "grind_signatures") { 1736 } else { 1740 }, - ); - - // splice acceptor - assert_eq!( - estimate_v2_funding_transaction_fee(&one_input, &[], false, true, 2000), - if cfg!(feature = "grind_signatures") { 542 } else { 544 }, - ); - } - - #[rustfmt::skip] - fn funding_input_sats(input_value_sats: u64) -> FundingTxInput { - let prevout = TxOut { - value: Amount::from_sat(input_value_sats), - script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()), - }; - let prevtx = Transaction { - input: vec![], output: vec![prevout], - version: Version::TWO, lock_time: bitcoin::absolute::LockTime::ZERO, - }; - - FundingTxInput::new_p2wpkh(prevtx, 0).unwrap() - } - - fn funding_output_sats(output_value_sats: u64) -> TxOut { - TxOut { - value: Amount::from_sat(output_value_sats), - script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()), - } - } - - #[test] - #[rustfmt::skip] - fn test_check_v2_funding_inputs_sufficient() { - use crate::ln::channel::check_v2_funding_inputs_sufficient; - - // positive case, inputs well over intended contribution - { - let expected_fee = if cfg!(feature = "grind_signatures") { 2278 } else { 2284 }; - assert_eq!( - check_v2_funding_inputs_sufficient( - Amount::from_sat(220_000), - &[ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - &[], - true, - true, - 2000, - ).unwrap(), - Amount::from_sat(expected_fee), - ); - } - - // Net splice-in - { - let expected_fee = if cfg!(feature = "grind_signatures") { 2526 } else { 2532 }; - assert_eq!( - check_v2_funding_inputs_sufficient( - Amount::from_sat(220_000), - &[ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - &[ - funding_output_sats(200_000), - ], - true, - true, - 2000, - ).unwrap(), - Amount::from_sat(expected_fee), - ); - } - - // Net splice-out - { - let expected_fee = if cfg!(feature = "grind_signatures") { 2526 } else { 2532 }; - assert_eq!( - check_v2_funding_inputs_sufficient( - Amount::from_sat(220_000), - &[ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - &[ - funding_output_sats(400_000), - ], - true, - true, - 2000, - ).unwrap(), - Amount::from_sat(expected_fee), - ); - } - - // Net splice-out, inputs insufficient to cover fees - { - let expected_fee = if cfg!(feature = "grind_signatures") { 113670 } else { 113940 }; - assert_eq!( - check_v2_funding_inputs_sufficient( - Amount::from_sat(220_000), - &[ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - &[ - funding_output_sats(400_000), - ], - true, - true, - 90000, - ), - Err(format!( - "Total input amount 0.00300000 BTC is lower than needed for splice-in contribution 0.00220000 BTC, considering fees of {}. Need more inputs.", - Amount::from_sat(expected_fee), - )), - ); - } - - // negative case, inputs clearly insufficient - { - let expected_fee = if cfg!(feature = "grind_signatures") { 1736 } else { 1740 }; - assert_eq!( - check_v2_funding_inputs_sufficient( - Amount::from_sat(220_000), - &[ - funding_input_sats(100_000), - ], - &[], - true, - true, - 2000, - ), - Err(format!( - "Total input amount 0.00100000 BTC is lower than needed for splice-in contribution 0.00220000 BTC, considering fees of {}. Need more inputs.", - Amount::from_sat(expected_fee), - )), - ); - } - - // barely covers - { - let expected_fee = if cfg!(feature = "grind_signatures") { 2278 } else { 2284 }; - assert_eq!( - check_v2_funding_inputs_sufficient( - Amount::from_sat(300_000 - expected_fee - 20), - &[ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - &[], - true, - true, - 2000, - ).unwrap(), - Amount::from_sat(expected_fee), - ); - } - - // higher fee rate, does not cover - { - let expected_fee = if cfg!(feature = "grind_signatures") { 2506 } else { 2513 }; - assert_eq!( - check_v2_funding_inputs_sufficient( - Amount::from_sat(298032), - &[ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - &[], - true, - true, - 2200, - ), - Err(format!( - "Total input amount 0.00300000 BTC is lower than needed for splice-in contribution 0.00298032 BTC, considering fees of {}. Need more inputs.", - Amount::from_sat(expected_fee), - )), - ); - } - - // barely covers, less fees (no extra weight, not initiator) - { - let expected_fee = if cfg!(feature = "grind_signatures") { 1084 } else { 1088 }; - assert_eq!( - check_v2_funding_inputs_sufficient( - Amount::from_sat(300_000 - expected_fee - 20), - &[ - funding_input_sats(200_000), - funding_input_sats(100_000), - ], - &[], - false, - false, - 2000, - ).unwrap(), - Amount::from_sat(expected_fee), - ); - } - } - fn get_pre_and_post( pre_channel_value: u64, our_funding_contribution: i64, their_funding_contribution: i64, ) -> (u64, u64) { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index eae26cc2d91..f70f4b133d0 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -30,7 +30,7 @@ use bitcoin::hashes::{Hash, HashEngine, HmacEngine}; use bitcoin::secp256k1::Secp256k1; use bitcoin::secp256k1::{PublicKey, SecretKey}; -use bitcoin::{secp256k1, Sequence, SignedAmount}; +use bitcoin::{secp256k1, FeeRate, Sequence, SignedAmount}; use crate::blinded_path::message::{ AsyncPaymentsContext, BlindedMessagePath, MessageForwardNode, OffersContext, @@ -64,7 +64,7 @@ use crate::ln::channel::{ UpdateFulfillCommitFetch, WithChannelContext, }; use crate::ln::channel_state::ChannelDetails; -use crate::ln::funding::SpliceContribution; +use crate::ln::funding::{FundingContribution, FundingTemplate}; use crate::ln::inbound_payment; use crate::ln::interactivetxs::InteractiveTxMessageSend; use crate::ln::msgs; @@ -4546,13 +4546,14 @@ impl< /// /// # Arguments /// - /// Provide a `contribution` to determine if value is spliced in or out. The splice initiator is - /// responsible for paying fees for common fields, shared inputs, and shared outputs along with - /// any contributed inputs and outputs. Fees are determined using `funding_feerate_per_kw` and - /// must be covered by the supplied inputs for splice-in or the channel balance for splice-out. + /// The splice initiator is responsible for paying fees for common fields, shared inputs, and + /// shared outputs along with any contributed inputs and outputs. Fees are determined using + /// `feerate` and must be covered by the supplied inputs for splice-in or the channel balance + /// for splice-out. /// - /// An optional `locktime` for the funding transaction may be specified. If not given, the - /// current best block height is used. + /// Returns a [`FundingTemplate`] which should be used to build a [`FundingContribution`] via + /// one of its splice methods (e.g., [`FundingTemplate::splice_in_sync`]). The resulting + /// contribution must then be passed to [`ChannelManager::funding_contributed`]. /// /// # Events /// @@ -4570,29 +4571,26 @@ impl< /// Once the splice has been locked by both counterparties, an [`Event::ChannelReady`] will be /// emitted with the new funding output. At this point, a new splice can be negotiated by /// calling `splice_channel` again on this channel. + /// + /// [`FundingContribution`]: crate::ln::funding::FundingContribution #[rustfmt::skip] pub fn splice_channel( - &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, - contribution: SpliceContribution, funding_feerate_per_kw: u32, locktime: Option, - ) -> Result<(), APIError> { - let mut res = Ok(()); + &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, feerate: FeeRate, + ) -> Result { + let mut res = Err(APIError::APIMisuseError { err: String::new() }); PersistenceNotifierGuard::optionally_notify(self, || { let result = self.internal_splice_channel( - channel_id, counterparty_node_id, contribution, funding_feerate_per_kw, locktime + channel_id, counterparty_node_id, feerate, ); res = result; - match res { - Ok(_) => NotifyOption::DoPersist, - Err(_) => NotifyOption::SkipPersistNoEvents, - } + NotifyOption::SkipPersistNoEvents }); res } fn internal_splice_channel( - &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, - contribution: SpliceContribution, funding_feerate_per_kw: u32, locktime: Option, - ) -> Result<(), APIError> { + &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, feerate: FeeRate, + ) -> Result { let per_peer_state = self.per_peer_state.read().unwrap(); let peer_state_mutex = match per_peer_state @@ -4618,22 +4616,8 @@ impl< // Look for the channel match peer_state.channel_by_id.entry(*channel_id) { 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 logger = WithChannelContext::from(&self.logger, &chan.context, None); - let msg_opt = chan.splice_channel( - contribution, - funding_feerate_per_kw, - locktime, - &&logger, - )?; - if let Some(msg) = msg_opt { - peer_state.pending_msg_events.push(MessageSendEvent::SendStfu { - node_id: *counterparty_node_id, - msg, - }); - } - Ok(()) + chan.splice_channel(feerate) } else { Err(APIError::ChannelUnavailable { err: format!( @@ -6342,6 +6326,108 @@ impl< result } + /// Adds or removes funds from the given channel as specified by a [`FundingContribution`]. + /// + /// Used after [`ChannelManager::splice_channel`] by constructing a [`FundingContribution`] + /// from the returned [`FundingTemplate`] and passing it here. + /// + /// Calling this method will commence the process of creating a new funding transaction for the + /// channel. An [`Event::FundingTransactionReadyForSigning`] will be generated once the + /// transaction is successfully constructed interactively with the counterparty. + /// If unsuccessful, an [`Event::SpliceFailed`] will be surfaced instead. + /// + /// An optional `locktime` for the funding transaction may be specified. If not given, the + /// current best block height is used. + /// + /// Returns [`ChannelUnavailable`] when a channel is not found or an incorrect + /// `counterparty_node_id` is provided. + /// + /// Returns [`APIMisuseError`] when a channel is not in a state where it is expecting funding + /// contribution. + /// + /// [`ChannelUnavailable`]: APIError::ChannelUnavailable + /// [`APIMisuseError`]: APIError::APIMisuseError + pub fn funding_contributed( + &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, + contribution: FundingContribution, locktime: Option, + ) -> Result<(), APIError> { + let mut result = Ok(()); + PersistenceNotifierGuard::optionally_notify(self, || { + let per_peer_state = self.per_peer_state.read().unwrap(); + let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id); + if peer_state_mutex_opt.is_none() { + result = Err(APIError::ChannelUnavailable { + err: format!("Can't find a peer matching the passed counterparty node_id {counterparty_node_id}") + }); + return NotifyOption::SkipPersistNoEvents; + } + + let mut peer_state = peer_state_mutex_opt.unwrap().lock().unwrap(); + + match peer_state.channel_by_id.get_mut(channel_id) { + Some(channel) => match channel.as_funded_mut() { + Some(chan) => { + let locktime = bitcoin::absolute::LockTime::from_consensus( + locktime.unwrap_or_else(|| self.current_best_block().height), + ); + let logger = WithChannelContext::from(&self.logger, chan.context(), None); + match chan.funding_contributed(contribution, locktime, &&logger) { + Ok(msg_opt) => { + if let Some(msg) = msg_opt { + peer_state.pending_msg_events.push( + MessageSendEvent::SendStfu { + node_id: *counterparty_node_id, + msg, + }, + ); + } + }, + Err(splice_funding_failed) => { + let pending_events = &mut self.pending_events.lock().unwrap(); + pending_events.push_back(( + events::Event::SpliceFailed { + channel_id: *channel_id, + counterparty_node_id: *counterparty_node_id, + user_channel_id: channel.context().get_user_id(), + abandoned_funding_txo: splice_funding_failed.funding_txo, + channel_type: splice_funding_failed.channel_type.clone(), + contributed_inputs: splice_funding_failed + .contributed_inputs, + contributed_outputs: splice_funding_failed + .contributed_outputs, + }, + None, + )); + }, + } + + return NotifyOption::DoPersist; + }, + None => { + result = Err(APIError::APIMisuseError { + err: format!( + "Channel with id {} not expecting funding contribution", + channel_id + ), + }); + return NotifyOption::SkipPersistNoEvents; + }, + }, + None => { + result = Err(APIError::ChannelUnavailable { + err: format!( + "Channel with id {} not found for the passed counterparty node_id {}", + channel_id, counterparty_node_id + ), + }); + return NotifyOption::SkipPersistNoEvents; + }, + } + }); + + result + } + /// Handles a signed funding transaction generated by interactive transaction construction and /// provided by the client. Should only be called in response to a [`FundingTransactionReadyForSigning`] /// event. @@ -13315,7 +13401,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }); notify = NotifyOption::SkipPersistHandleEvents; }, - Err(msg) => log_trace!(logger, "{}", msg), + Err((msg, _action)) => log_trace!(logger, "{}", msg), } } else { result = Err(APIError::APIMisuseError { diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 33f78b13553..66a0147e131 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -404,10 +404,10 @@ fn do_connect_block_without_consistency_checks<'a, 'b, 'c, 'd>( } pub fn provide_anchor_reserves<'a, 'b, 'c>(nodes: &[Node<'a, 'b, 'c>]) -> Transaction { - provide_anchor_utxo_reserves(nodes, 1, Amount::ONE_BTC) + provide_utxo_reserves(nodes, 1, Amount::ONE_BTC) } -pub fn provide_anchor_utxo_reserves<'a, 'b, 'c>( +pub fn provide_utxo_reserves<'a, 'b, 'c>( nodes: &[Node<'a, 'b, 'c>], utxos: usize, amount: Amount, ) -> Transaction { let mut output = Vec::with_capacity(nodes.len()); @@ -614,6 +614,10 @@ impl<'a, 'b, 'c> Node<'a, 'b, 'c> { self.blocks.lock().unwrap()[height as usize].0.header } + pub fn provide_funding_utxos(&self, utxos: usize, amount: Amount) -> Transaction { + provide_utxo_reserves(core::slice::from_ref(self), utxos, amount) + } + /// Executes `enable_channel_signer_op` for every single signer operation for this channel. #[cfg(test)] pub fn enable_all_channel_signer_ops(&self, peer_id: &PublicKey, chan_id: &ChannelId) { diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 9981250b05e..e369bd8a3ec 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -9,29 +9,329 @@ //! Types pertaining to funding channels. -use alloc::vec::Vec; +use bitcoin::hashes::Hash; +use bitcoin::secp256k1::PublicKey; +use bitcoin::{ + Amount, FeeRate, OutPoint, Script, ScriptBuf, Sequence, SignedAmount, Transaction, TxOut, + WScriptHash, Weight, +}; + +use core::ops::Deref; + +use crate::events::bump_transaction::sync::CoinSelectionSourceSync; +use crate::events::bump_transaction::{CoinSelectionSource, Input, Utxo}; +use crate::ln::chan_utils::{ + make_funding_redeemscript, BASE_INPUT_WEIGHT, EMPTY_SCRIPT_SIG_WEIGHT, + FUNDING_TRANSACTION_WITNESS_WEIGHT, +}; +use crate::ln::interactivetxs::{get_output_weight, TX_COMMON_FIELDS_WEIGHT}; +use crate::ln::msgs; +use crate::ln::types::ChannelId; +use crate::ln::LN_MAX_MSG_LEN; +use crate::prelude::*; +use crate::sign::{P2TR_KEY_PATH_WITNESS_WEIGHT, P2WPKH_WITNESS_WEIGHT}; +use crate::util::async_poll::MaybeSend; + +/// A template for contributing to a channel's splice funding transaction. +/// +/// This is returned from [`ChannelManager::splice_channel`] when a channel is ready to be +/// spliced. It must be converted to a [`FundingContribution`] using one of the splice methods +/// and passed to [`ChannelManager::funding_contributed`] in order to resume the splicing +/// process. +/// +/// [`ChannelManager::splice_channel`]: crate::ln::channelmanager::ChannelManager::splice_channel +/// [`ChannelManager::funding_contributed`]: crate::ln::channelmanager::ChannelManager::funding_contributed +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FundingTemplate { + /// The shared input, which, if present indicates the funding template is for a splice funding + /// transaction. + shared_input: Option, + + /// The fee rate to use for coin selection. + feerate: FeeRate, + + /// Whether the contributor initiated the funding, and thus is responsible for fees incurred for + /// common fields and shared inputs and outputs. + is_initiator: bool, +} -use bitcoin::{Amount, ScriptBuf, SignedAmount, TxOut}; -use bitcoin::{Script, Sequence, Transaction, Weight}; +impl FundingTemplate { + /// Constructs a [`FundingTemplate`] for a splice using the provided shared input. + pub(super) fn new( + shared_input: Option, feerate: FeeRate, is_initiator: bool, + ) -> Self { + Self { shared_input, feerate, is_initiator } + } +} -use crate::events::bump_transaction::Utxo; -use crate::ln::chan_utils::EMPTY_SCRIPT_SIG_WEIGHT; -use crate::sign::{P2TR_KEY_PATH_WITNESS_WEIGHT, P2WPKH_WITNESS_WEIGHT}; +macro_rules! build_funding_contribution { + ($value_added:expr, $outputs:expr, $change_script:expr, $shared_input:expr, $feerate:expr, $is_initiator:expr, $wallet:ident, $($await:tt)*) => {{ + let value_added: Amount = $value_added; + let outputs: Vec = $outputs; + let change_script: Option = $change_script; + let shared_input: Option = $shared_input; + let feerate: FeeRate = $feerate; + let is_initiator: bool = $is_initiator; + + let value_removed = outputs.iter().map(|txout| txout.value).sum(); + let is_splice = shared_input.is_some(); + + let inputs = if value_added == Amount::ZERO { + vec![] + } else { + // Used for creating a redeem script for the new funding txo, since the funding pubkeys + // are unknown at this point. Only needed when selecting which UTXOs to include in the + // funding tx that would be sufficient to pay for fees. Hence, the value doesn't matter. + let dummy_pubkey = PublicKey::from_slice(&[2; 33]).unwrap(); + + let shared_output = bitcoin::TxOut { + value: shared_input + .as_ref() + .map(|shared_input| shared_input.previous_utxo.value) + .unwrap_or(Amount::ZERO) + .checked_add(value_added) + .ok_or(())? + .checked_sub(value_removed) + .ok_or(())?, + script_pubkey: make_funding_redeemscript(&dummy_pubkey, &dummy_pubkey).to_p2wsh(), + }; + + let claim_id = None; + let must_spend = shared_input.map(|input| vec![input]).unwrap_or_default(); + let selection = if outputs.is_empty() { + let must_pay_to = &[shared_output]; + $wallet.select_confirmed_utxos(claim_id, must_spend, must_pay_to, feerate.to_sat_per_kwu() as u32, u64::MAX)$(.$await)*? + } else { + let must_pay_to: Vec<_> = outputs.iter().cloned().chain(core::iter::once(shared_output)).collect(); + $wallet.select_confirmed_utxos(claim_id, must_spend, &must_pay_to, feerate.to_sat_per_kwu() as u32, u64::MAX)$(.$await)*? + }; + selection.confirmed_utxos + }; + + // NOTE: Must NOT fail after UTXO selection + + let estimated_fee = estimate_transaction_fee(&inputs, &outputs, is_initiator, is_splice, feerate); + + let contribution = FundingContribution { + value_added, + estimated_fee, + inputs, + outputs, + change_script, + feerate, + is_initiator, + is_splice, + }; + + Ok(contribution) + }}; +} + +impl FundingTemplate { + /// Creates a [`FundingContribution`] for adding funds to a channel using `wallet` to perform + /// coin selection. + /// + /// An optional `change_script` may be given to use as a change output. If `None` and change is + /// needed, one will be generated using [`SignerProvider::get_destination_script`]. + /// + /// [`SignerProvider::get_destination_script`]: crate::sign::SignerProvider::get_destination_script + pub async fn splice_in( + self, change_script: Option, value_added: Amount, wallet: W, + ) -> Result + where + W::Target: CoinSelectionSource + MaybeSend, + { + if value_added == Amount::ZERO { + return Err(()); + } + let FundingTemplate { shared_input, feerate, is_initiator } = self; + build_funding_contribution!(value_added, vec![], change_script, shared_input, feerate, is_initiator, wallet, await) + } + + /// Creates a [`FundingContribution`] for adding funds to a channel using `wallet` to perform + /// coin selection. + /// + /// An optional `change_script` may be given to use as a change output. If `None` and change is + /// needed, one will be generated using [`SignerProvider::get_destination_script`]. + /// + /// [`SignerProvider::get_destination_script`]: crate::sign::SignerProvider::get_destination_script + pub fn splice_in_sync( + self, change_script: Option, value_added: Amount, wallet: W, + ) -> Result + where + W::Target: CoinSelectionSourceSync, + { + if value_added == Amount::ZERO { + return Err(()); + } + let FundingTemplate { shared_input, feerate, is_initiator } = self; + build_funding_contribution!( + value_added, + vec![], + change_script, + shared_input, + feerate, + is_initiator, + wallet, + ) + } + + /// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to + /// perform coin selection. + pub async fn splice_out( + self, outputs: Vec, wallet: W, + ) -> Result + where + W::Target: CoinSelectionSource + MaybeSend, + { + if outputs.is_empty() { + return Err(()); + } + let FundingTemplate { shared_input, feerate, is_initiator } = self; + build_funding_contribution!(Amount::ZERO, outputs, None, shared_input, feerate, is_initiator, wallet, await) + } + + /// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to + /// perform coin selection. + pub fn splice_out_sync( + self, outputs: Vec, wallet: W, + ) -> Result + where + W::Target: CoinSelectionSourceSync, + { + if outputs.is_empty() { + return Err(()); + } + let FundingTemplate { shared_input, feerate, is_initiator } = self; + build_funding_contribution!( + Amount::ZERO, + outputs, + None, + shared_input, + feerate, + is_initiator, + wallet, + ) + } + + /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using + /// `wallet` to perform coin selection. + /// + /// An optional `change_script` may be given to use as a change output. If `None` and change is + /// needed, one will be generated using [`SignerProvider::get_destination_script`]. + /// + /// [`SignerProvider::get_destination_script`]: crate::sign::SignerProvider::get_destination_script + pub async fn splice_in_and_out( + self, change_script: Option, value_added: Amount, outputs: Vec, + wallet: W, + ) -> Result + where + W::Target: CoinSelectionSource + MaybeSend, + { + if value_added == Amount::ZERO && outputs.is_empty() { + return Err(()); + } + let FundingTemplate { shared_input, feerate, is_initiator } = self; + build_funding_contribution!(value_added, outputs, change_script, shared_input, feerate, is_initiator, wallet, await) + } -/// The components of a splice's funding transaction that are contributed by one party. + /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using + /// `wallet` to perform coin selection. + /// + /// An optional `change_script` may be given to use as a change output. If `None` and change is + /// needed, one will be generated using [`SignerProvider::get_destination_script`]. + /// + /// [`SignerProvider::get_destination_script`]: crate::sign::SignerProvider::get_destination_script + pub fn splice_in_and_out_sync( + self, change_script: Option, value_added: Amount, outputs: Vec, + wallet: W, + ) -> Result + where + W::Target: CoinSelectionSourceSync, + { + if value_added == Amount::ZERO && outputs.is_empty() { + return Err(()); + } + let FundingTemplate { shared_input, feerate, is_initiator } = self; + build_funding_contribution!( + value_added, + outputs, + change_script, + shared_input, + feerate, + is_initiator, + wallet, + ) + } +} + +fn estimate_transaction_fee( + inputs: &[FundingTxInput], outputs: &[TxOut], is_initiator: bool, is_splice: bool, + feerate: FeeRate, +) -> Amount { + let input_weight: u64 = inputs + .iter() + .map(|input| BASE_INPUT_WEIGHT.saturating_add(input.utxo.satisfaction_weight)) + .fold(0, |total_weight, input_weight| total_weight.saturating_add(input_weight)); + + let output_weight: u64 = outputs + .iter() + .map(|txout| txout.weight().to_wu()) + .fold(0, |total_weight, output_weight| total_weight.saturating_add(output_weight)); + + let mut weight = input_weight.saturating_add(output_weight); + + // The initiator pays for all common fields and the shared output in the funding transaction. + if is_initiator { + weight = weight + .saturating_add(TX_COMMON_FIELDS_WEIGHT) + // The weight of the funding output, a P2WSH output + // NOTE: The witness script hash given here is irrelevant as it's a fixed size and we just want + // to calculate the contributed weight, so we use an all-zero hash. + // + // TODO(taproot): Needs to consider different weights based on channel type + .saturating_add( + get_output_weight(&ScriptBuf::new_p2wsh(&WScriptHash::from_raw_hash( + Hash::all_zeros(), + ))) + .to_wu(), + ); + + // The splice initiator pays for the input spending the previous funding output. + if is_splice { + weight = weight + .saturating_add(BASE_INPUT_WEIGHT) + .saturating_add(EMPTY_SCRIPT_SIG_WEIGHT) + .saturating_add(FUNDING_TRANSACTION_WITNESS_WEIGHT); + #[cfg(feature = "grind_signatures")] + { + // Guarantees a low R signature + weight -= 1; + } + } + } + + Weight::from_wu(weight) * feerate +} + +/// The components of a funding transaction contributed by one party. #[derive(Debug, Clone)] -pub struct SpliceContribution { - /// The amount from [`inputs`] to contribute to the splice. +pub struct FundingContribution { + /// The amount to contribute to the channel. /// - /// [`inputs`]: Self::inputs + /// If `value_added` is [`Amount::ZERO`], then any fees will be deducted from the channel + /// balance instead of paid by `inputs`. value_added: Amount, - /// The inputs included in the splice's funding transaction to meet the contributed amount - /// plus fees. Any excess amount will be sent to a change output. + /// The estimate fees responsible to be paid for the contribution. + estimated_fee: Amount, + + /// The inputs included in the funding transaction to meet the contributed amount plus fees. Any + /// excess amount will be sent to a change output. inputs: Vec, - /// The outputs to include in the splice's funding transaction. The total value of all - /// outputs plus fees will be the amount that is removed. + /// The outputs to include in the funding transaction. The total value of all outputs plus fees + /// will be the amount that is removed. outputs: Vec, /// An optional change output script. This will be used if needed or, when not set, @@ -39,63 +339,127 @@ pub struct SpliceContribution { /// /// [`SignerProvider::get_destination_script`]: crate::sign::SignerProvider::get_destination_script change_script: Option, + + /// The fee rate used to select `inputs`. + feerate: FeeRate, + + /// Whether the contributor initiated the funding, and thus is responsible for fees incurred for + /// common fields and shared inputs and outputs. + is_initiator: bool, + + /// Whether the contribution is for funding a splice. + is_splice: bool, } -impl SpliceContribution { - /// Creates a contribution for when funds are only added to a channel. - pub fn splice_in( - value_added: Amount, inputs: Vec, change_script: Option, - ) -> Self { - Self { value_added, inputs, outputs: vec![], change_script } +impl_writeable_tlv_based!(FundingContribution, { + (1, value_added, required), + (3, estimated_fee, required), + (5, inputs, optional_vec), + (7, outputs, optional_vec), + (9, change_script, option), + (11, feerate, required), + (13, is_initiator, required), + (15, is_splice, required), +}); + +impl FundingContribution { + pub(super) fn feerate(&self) -> FeeRate { + self.feerate } - /// Creates a contribution for when funds are only removed from a channel. - pub fn splice_out(outputs: Vec) -> Self { - Self { value_added: Amount::ZERO, inputs: vec![], outputs, change_script: None } + pub(super) fn is_initiator(&self) -> bool { + self.is_initiator } - /// Creates a contribution for when funds are both added to and removed from a channel. - /// - /// Note that `value_added` represents the value added by `inputs` but should not account for - /// value removed by `outputs`. The net value contributed can be obtained by calling - /// [`SpliceContribution::net_value`]. - pub fn splice_in_and_out( - value_added: Amount, inputs: Vec, outputs: Vec, - change_script: Option, - ) -> Self { - Self { value_added, inputs, outputs, change_script } + pub(super) fn is_splice(&self) -> bool { + self.is_splice + } + + pub(super) fn into_tx_parts(self) -> (Vec, Vec, Option) { + let FundingContribution { inputs, outputs, change_script, .. } = self; + (inputs, outputs, change_script) + } + + pub(super) fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { + (self.inputs.into_iter().map(|input| input.utxo.outpoint).collect(), self.outputs) } /// The net value contributed to a channel by the splice. If negative, more value will be - /// spliced out than spliced in. - pub fn net_value(&self) -> SignedAmount { - let value_added = self.value_added.to_signed().unwrap_or(SignedAmount::MAX); + /// spliced out than spliced in. Fees will be deducted from the expected splice-out amount + /// if no inputs were included. + pub fn net_value(&self) -> Result { + for FundingTxInput { utxo, prevtx, .. } in self.inputs.iter() { + use crate::util::ser::Writeable; + const MESSAGE_TEMPLATE: msgs::TxAddInput = msgs::TxAddInput { + channel_id: ChannelId([0; 32]), + serial_id: 0, + prevtx: None, + prevtx_out: 0, + sequence: 0, + // Mutually exclusive with prevtx, which is accounted for below. + shared_input_txid: None, + }; + let message_len = MESSAGE_TEMPLATE.serialized_length() + prevtx.serialized_length(); + if message_len > LN_MAX_MSG_LEN { + return Err(format!( + "Funding input references a prevtx that is too large for tx_add_input: {}", + utxo.outpoint + )); + } + } + + // Fees for splice-out are paid from the channel balance whereas fees for splice-in + // are paid by the funding inputs. Therefore, in the case of splice-out, we add the + // fees on top of the user-specified contribution. We leave the user-specified + // contribution as-is for splice-ins. + if !self.inputs.is_empty() { + let mut total_input_value = Amount::ZERO; + for FundingTxInput { utxo, .. } in self.inputs.iter() { + total_input_value = total_input_value + .checked_add(utxo.output.value) + .ok_or("Sum of input values is greater than the total bitcoin supply")?; + } + + // If the inputs are enough to cover intended contribution amount, with fees even when + // there is a change output, we are fine. + // If the inputs are less, but enough to cover intended contribution amount, with + // (lower) fees with no change, we are also fine (change will not be generated). + // So it's enough to check considering the lower, no-change fees. + // + // Note: dust limit is not relevant in this check. + + let contributed_input_value = self.value_added; + let estimated_fee = self.estimated_fee; + let minimal_input_amount_needed = contributed_input_value + .checked_add(estimated_fee) + .ok_or(format!("{contributed_input_value} contribution plus {estimated_fee} fee estimate exceeds the total bitcoin supply"))?; + if total_input_value < minimal_input_amount_needed { + return Err(format!( + "Total input amount {total_input_value} is lower than needed for splice-in contribution {contributed_input_value}, considering fees of {estimated_fee}. Need more inputs.", + )); + } + } + + let unpaid_fees = if self.inputs.is_empty() { self.estimated_fee } else { Amount::ZERO } + .to_signed() + .expect("fees should never exceed Amount::MAX_MONEY"); + let value_added = self.value_added.to_signed().map_err(|_| "Value added too large")?; let value_removed = self .outputs .iter() .map(|txout| txout.value) .sum::() .to_signed() - .unwrap_or(SignedAmount::MAX); + .map_err(|_| "Value removed too large")?; - value_added - value_removed - } + let contribution_amount = value_added - value_removed; + let adjusted_contribution = contribution_amount.checked_sub(unpaid_fees).ok_or(format!( + "{} splice-out amount plus {} fee estimate exceeds the total bitcoin supply", + contribution_amount.unsigned_abs(), + self.estimated_fee, + ))?; - pub(super) fn value_added(&self) -> Amount { - self.value_added - } - - pub(super) fn inputs(&self) -> &[FundingTxInput] { - &self.inputs[..] - } - - pub(super) fn outputs(&self) -> &[TxOut] { - &self.outputs[..] - } - - pub(super) fn into_tx_parts(self) -> (Vec, Vec, Option) { - let SpliceContribution { value_added: _, inputs, outputs, change_script } = self; - (inputs, outputs, change_script) + Ok(adjusted_contribution) } } @@ -267,3 +631,260 @@ impl FundingTxInput { self.utxo.output } } + +#[cfg(test)] +mod tests { + use super::{estimate_transaction_fee, FundingContribution, FundingTxInput}; + use bitcoin::hashes::Hash; + use bitcoin::transaction::{Transaction, TxOut, Version}; + use bitcoin::{Amount, FeeRate, ScriptBuf, SignedAmount, WPubkeyHash}; + + #[test] + #[rustfmt::skip] + fn test_estimate_transaction_fee() { + let one_input = [funding_input_sats(1_000)]; + let two_inputs = [funding_input_sats(1_000), funding_input_sats(1_000)]; + + // 2 inputs, initiator, 2000 sat/kw feerate + assert_eq!( + estimate_transaction_fee(&two_inputs, &[], true, false, FeeRate::from_sat_per_kwu(2000)), + Amount::from_sat(if cfg!(feature = "grind_signatures") { 1512 } else { 1516 }), + ); + + // higher feerate + assert_eq!( + estimate_transaction_fee(&two_inputs, &[], true, false, FeeRate::from_sat_per_kwu(3000)), + Amount::from_sat(if cfg!(feature = "grind_signatures") { 2268 } else { 2274 }), + ); + + // only 1 input + assert_eq!( + estimate_transaction_fee(&one_input, &[], true, false, FeeRate::from_sat_per_kwu(2000)), + Amount::from_sat(if cfg!(feature = "grind_signatures") { 970 } else { 972 }), + ); + + // 0 inputs + assert_eq!( + estimate_transaction_fee(&[], &[], true, false, FeeRate::from_sat_per_kwu(2000)), + Amount::from_sat(428), + ); + + // not initiator + assert_eq!( + estimate_transaction_fee(&[], &[], false, false, FeeRate::from_sat_per_kwu(2000)), + Amount::from_sat(0), + ); + + // splice initiator + assert_eq!( + estimate_transaction_fee(&one_input, &[], true, true, FeeRate::from_sat_per_kwu(2000)), + Amount::from_sat(if cfg!(feature = "grind_signatures") { 1736 } else { 1740 }), + ); + + // splice acceptor + assert_eq!( + estimate_transaction_fee(&one_input, &[], false, true, FeeRate::from_sat_per_kwu(2000)), + Amount::from_sat(if cfg!(feature = "grind_signatures") { 542 } else { 544 }), + ); + } + + #[rustfmt::skip] + fn funding_input_sats(input_value_sats: u64) -> FundingTxInput { + let prevout = TxOut { + value: Amount::from_sat(input_value_sats), + script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()), + }; + let prevtx = Transaction { + input: vec![], output: vec![prevout], + version: Version::TWO, lock_time: bitcoin::absolute::LockTime::ZERO, + }; + + FundingTxInput::new_p2wpkh(prevtx, 0).unwrap() + } + + fn funding_output_sats(output_value_sats: u64) -> TxOut { + TxOut { + value: Amount::from_sat(output_value_sats), + script_pubkey: ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()), + } + } + + #[test] + #[rustfmt::skip] + fn test_check_v2_funding_inputs_sufficient() { + // positive case, inputs well over intended contribution + { + let expected_fee = if cfg!(feature = "grind_signatures") { 2278 } else { 2284 }; + let contribution = FundingContribution { + value_added: Amount::from_sat(220_000), + estimated_fee: Amount::from_sat(expected_fee), + inputs: vec![ + funding_input_sats(200_000), + funding_input_sats(100_000), + ], + outputs: vec![], + change_script: None, + is_initiator: true, + is_splice: true, + feerate: FeeRate::from_sat_per_kwu(2000), + }; + assert_eq!(contribution.net_value(), Ok(contribution.value_added.to_signed().unwrap())); + } + + // Net splice-in + { + let expected_fee = if cfg!(feature = "grind_signatures") { 2526 } else { 2532 }; + let contribution = FundingContribution { + value_added: Amount::from_sat(220_000), + estimated_fee: Amount::from_sat(expected_fee), + inputs: vec![ + funding_input_sats(200_000), + funding_input_sats(100_000), + ], + outputs: vec![ + funding_output_sats(200_000), + ], + change_script: None, + is_initiator: true, + is_splice: true, + feerate: FeeRate::from_sat_per_kwu(2000), + }; + assert_eq!(contribution.net_value(), Ok(SignedAmount::from_sat(220_000 - 200_000))); + } + + // Net splice-out + { + let expected_fee = if cfg!(feature = "grind_signatures") { 2526 } else { 2532 }; + let contribution = FundingContribution { + value_added: Amount::from_sat(220_000), + estimated_fee: Amount::from_sat(expected_fee), + inputs: vec![ + funding_input_sats(200_000), + funding_input_sats(100_000), + ], + outputs: vec![ + funding_output_sats(400_000), + ], + change_script: None, + is_initiator: true, + is_splice: true, + feerate: FeeRate::from_sat_per_kwu(2000), + }; + assert_eq!(contribution.net_value(), Ok(SignedAmount::from_sat(220_000 - 400_000))); + } + + // Net splice-out, inputs insufficient to cover fees + { + let expected_fee = if cfg!(feature = "grind_signatures") { 113670 } else { 113940 }; + let contribution = FundingContribution { + value_added: Amount::from_sat(220_000), + estimated_fee: Amount::from_sat(expected_fee), + inputs: vec![ + funding_input_sats(200_000), + funding_input_sats(100_000), + ], + outputs: vec![ + funding_output_sats(400_000), + ], + change_script: None, + is_initiator: true, + is_splice: true, + feerate: FeeRate::from_sat_per_kwu(90000), + }; + assert_eq!( + contribution.net_value(), + Err(format!( + "Total input amount 0.00300000 BTC is lower than needed for splice-in contribution 0.00220000 BTC, considering fees of {}. Need more inputs.", + Amount::from_sat(expected_fee), + )), + ); + } + + // negative case, inputs clearly insufficient + { + let expected_fee = if cfg!(feature = "grind_signatures") { 1736 } else { 1740 }; + let contribution = FundingContribution { + value_added: Amount::from_sat(220_000), + estimated_fee: Amount::from_sat(expected_fee), + inputs: vec![ + funding_input_sats(100_000), + ], + outputs: vec![], + change_script: None, + is_initiator: true, + is_splice: true, + feerate: FeeRate::from_sat_per_kwu(2000), + }; + assert_eq!( + contribution.net_value(), + Err(format!( + "Total input amount 0.00100000 BTC is lower than needed for splice-in contribution 0.00220000 BTC, considering fees of {}. Need more inputs.", + Amount::from_sat(expected_fee), + )), + ); + } + + // barely covers + { + let expected_fee = if cfg!(feature = "grind_signatures") { 2278 } else { 2284 }; + let contribution = FundingContribution { + value_added: Amount::from_sat(300_000 - expected_fee - 20), + estimated_fee: Amount::from_sat(expected_fee), + inputs: vec![ + funding_input_sats(200_000), + funding_input_sats(100_000), + ], + outputs: vec![], + change_script: None, + is_initiator: true, + is_splice: true, + feerate: FeeRate::from_sat_per_kwu(2000), + }; + assert_eq!(contribution.net_value(), Ok(contribution.value_added.to_signed().unwrap())); + } + + // higher fee rate, does not cover + { + let expected_fee = if cfg!(feature = "grind_signatures") { 2506 } else { 2513 }; + let contribution = FundingContribution { + value_added: Amount::from_sat(298032), + estimated_fee: Amount::from_sat(expected_fee), + inputs: vec![ + funding_input_sats(200_000), + funding_input_sats(100_000), + ], + outputs: vec![], + change_script: None, + is_initiator: true, + is_splice: true, + feerate: FeeRate::from_sat_per_kwu(2200), + }; + assert_eq!( + contribution.net_value(), + Err(format!( + "Total input amount 0.00300000 BTC is lower than needed for splice-in contribution 0.00298032 BTC, considering fees of {}. Need more inputs.", + Amount::from_sat(expected_fee), + )), + ); + } + + // barely covers, less fees (no extra weight, not initiator) + { + let expected_fee = if cfg!(feature = "grind_signatures") { 1084 } else { 1088 }; + let contribution = FundingContribution { + value_added: Amount::from_sat(300_000 - expected_fee - 20), + estimated_fee: Amount::from_sat(expected_fee), + inputs: vec![ + funding_input_sats(200_000), + funding_input_sats(100_000), + ], + outputs: vec![], + change_script: None, + is_initiator: false, + is_splice: false, + feerate: FeeRate::from_sat_per_kwu(2000), + }; + assert_eq!(contribution.net_value(), Ok(contribution.value_added.to_signed().unwrap())); + } + } +} diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 4846f7137cc..31c13e124d4 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -13,13 +13,13 @@ use crate::chain::chaininterface::{TransactionType, FEERATE_FLOOR_SATS_PER_KW}; use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS}; use crate::chain::transaction::OutPoint; use crate::chain::ChannelMonitorUpdateStatus; -use crate::events::bump_transaction::sync::WalletSourceSync; +use crate::events::bump_transaction::sync::{WalletSourceSync, WalletSync}; use crate::events::{ClosureReason, Event, FundingInfo, HTLCHandlingFailureType}; use crate::ln::chan_utils; use crate::ln::channel::CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY; use crate::ln::channelmanager::{provided_init_features, PaymentId, BREAKDOWN_TIMEOUT}; use crate::ln::functional_test_utils::*; -use crate::ln::funding::{FundingTxInput, SpliceContribution}; +use crate::ln::funding::FundingContribution; use crate::ln::msgs::{self, BaseMessageHandler, ChannelMessageHandler, MessageSendEvent}; use crate::ln::outbound_payment::RecipientOnionFields; use crate::ln::types::ChannelId; @@ -27,10 +27,14 @@ use crate::routing::router::{PaymentParameters, RouteParameters}; use crate::util::errors::APIError; use crate::util::ser::Writeable; +use crate::sync::Arc; + use bitcoin::hashes::Hash; use bitcoin::secp256k1::ecdsa::Signature; use bitcoin::secp256k1::PublicKey; -use bitcoin::{Amount, OutPoint as BitcoinOutPoint, ScriptBuf, Transaction, TxOut, WPubkeyHash}; +use bitcoin::{ + Amount, FeeRate, OutPoint as BitcoinOutPoint, ScriptBuf, Transaction, TxOut, WPubkeyHash, +}; #[test] fn test_splicing_not_supported_api_error() { @@ -47,15 +51,8 @@ fn test_splicing_not_supported_api_error() { let (_, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1); - let bs_contribution = SpliceContribution::splice_in(Amount::ZERO, Vec::new(), None); - - let res = nodes[1].node.splice_channel( - &channel_id, - &node_id_0, - bs_contribution.clone(), - 0, // funding_feerate_per_kw, - None, // locktime - ); + let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let res = nodes[1].node.splice_channel(&channel_id, &node_id_0, feerate); match res { Err(APIError::ChannelUnavailable { err }) => { assert!(err.contains("Peer does not support splicing")) @@ -76,13 +73,7 @@ fn test_splicing_not_supported_api_error() { reconnect_args.send_announcement_sigs = (true, true); reconnect_nodes(reconnect_args); - let res = nodes[1].node.splice_channel( - &channel_id, - &node_id_0, - bs_contribution, - 0, // funding_feerate_per_kw, - None, // locktime - ); + let res = nodes[1].node.splice_channel(&channel_id, &node_id_0, feerate); match res { Err(APIError::ChannelUnavailable { err }) => { assert!(err.contains("Peer does not support quiescence, a splicing prerequisite")) @@ -102,64 +93,122 @@ fn test_v1_splice_in_negative_insufficient_inputs() { create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0); // Amount being added to the channel through the splice-in - let splice_in_sats = 20_000; + let splice_in_value = Amount::from_sat(20_000); // Create additional inputs, but insufficient - let extra_splice_funding_input_sats = splice_in_sats - 1; - let funding_inputs = - create_dual_funding_utxos_with_prev_txs(&nodes[0], &[extra_splice_funding_input_sats]); + let extra_splice_funding_input = splice_in_value - Amount::ONE_SAT; - let contribution = - SpliceContribution::splice_in(Amount::from_sat(splice_in_sats), funding_inputs, None); + provide_utxo_reserves(&nodes, 1, extra_splice_funding_input); + + let feerate = FeeRate::from_sat_per_kwu(1024); // Initiate splice-in, with insufficient input contribution - let res = nodes[0].node.splice_channel( - &channel_id, - &nodes[1].node.get_our_node_id(), - contribution, - 1024, // funding_feerate_per_kw, - None, // locktime - ); - match res { - Err(APIError::APIMisuseError { err }) => { - assert!(err.contains("Need more inputs")) - }, - _ => panic!("Wrong error {:?}", res.err().unwrap()), - } + let funding_template = nodes[0] + .node + .splice_channel(&channel_id, &nodes[1].node.get_our_node_id(), feerate) + .unwrap(); + + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + assert!(funding_template.splice_in_sync(None, splice_in_value, &wallet).is_err()); } pub fn negotiate_splice_tx<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, - initiator_contribution: SpliceContribution, + funding_contribution: FundingContribution, ) { - let new_funding_script = - complete_splice_handshake(initiator, acceptor, channel_id, initiator_contribution.clone()); + let new_funding_script = complete_splice_handshake(initiator, acceptor); + complete_interactive_funding_negotiation( initiator, acceptor, channel_id, - initiator_contribution, + funding_contribution, new_funding_script, ); } -pub fn complete_splice_handshake<'a, 'b, 'c, 'd>( +pub fn initiate_splice_in<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, - initiator_contribution: SpliceContribution, -) -> ScriptBuf { - let node_id_initiator = initiator.node.get_our_node_id(); + value_added: Amount, +) -> FundingContribution { + let change_script = Some(initiator.wallet_source.get_change_script().unwrap()); + do_initiate_splice_in(initiator, acceptor, channel_id, value_added, change_script) +} + +pub fn do_initiate_splice_in<'a, 'b, 'c, 'd>( + initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, + value_added: Amount, change_script: Option, +) -> FundingContribution { + let node_id_acceptor = acceptor.node.get_our_node_id(); + let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let funding_template = + initiator.node.splice_channel(&channel_id, &node_id_acceptor, feerate).unwrap(); + let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger); + let funding_contribution = + funding_template.splice_in_sync(change_script, value_added, &wallet).unwrap(); + initiator + .node + .funding_contributed(&channel_id, &node_id_acceptor, funding_contribution.clone(), None) + .unwrap(); + funding_contribution +} + +pub fn initiate_splice_out<'a, 'b, 'c, 'd>( + initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, + outputs: Vec, +) -> FundingContribution { let node_id_acceptor = acceptor.node.get_our_node_id(); + let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let funding_template = + initiator.node.splice_channel(&channel_id, &node_id_acceptor, feerate).unwrap(); + let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger); + let funding_contribution = funding_template.splice_out_sync(outputs, &wallet).unwrap(); + initiator + .node + .funding_contributed(&channel_id, &node_id_acceptor, funding_contribution.clone(), None) + .unwrap(); + funding_contribution +} + +pub fn initiate_splice_in_and_out<'a, 'b, 'c, 'd>( + initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, + value_added: Amount, outputs: Vec, +) -> FundingContribution { + let change_script = Some(initiator.wallet_source.get_change_script().unwrap()); + do_initiate_splice_in_and_out( + initiator, + acceptor, + channel_id, + value_added, + outputs, + change_script, + ) +} +pub fn do_initiate_splice_in_and_out<'a, 'b, 'c, 'd>( + initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, + value_added: Amount, outputs: Vec, change_script: Option, +) -> FundingContribution { + let node_id_acceptor = acceptor.node.get_our_node_id(); + let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let funding_template = + initiator.node.splice_channel(&channel_id, &node_id_acceptor, feerate).unwrap(); + let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger); + let funding_contribution = funding_template + .splice_in_and_out_sync(change_script, value_added, outputs, &wallet) + .unwrap(); initiator .node - .splice_channel( - &channel_id, - &node_id_acceptor, - initiator_contribution, - FEERATE_FLOOR_SATS_PER_KW, - None, - ) + .funding_contributed(&channel_id, &node_id_acceptor, funding_contribution.clone(), None) .unwrap(); + funding_contribution +} + +pub fn complete_splice_handshake<'a, 'b, 'c, 'd>( + initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, +) -> ScriptBuf { + let node_id_initiator = initiator.node.get_our_node_id(); + let node_id_acceptor = acceptor.node.get_our_node_id(); let stfu_init = get_event_msg!(initiator, MessageSendEvent::SendStfu, node_id_acceptor); acceptor.node.handle_stfu(node_id_initiator, &stfu_init); @@ -182,7 +231,7 @@ pub fn complete_splice_handshake<'a, 'b, 'c, 'd>( pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, - initiator_contribution: SpliceContribution, new_funding_script: ScriptBuf, + initiator_contribution: FundingContribution, new_funding_script: ScriptBuf, ) { let node_id_initiator = initiator.node.get_our_node_id(); let node_id_acceptor = acceptor.node.get_our_node_id(); @@ -358,19 +407,18 @@ pub fn sign_interactive_funding_tx<'a, 'b, 'c, 'd>( pub fn splice_channel<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, - initiator_contribution: SpliceContribution, + funding_contribution: FundingContribution, ) -> Transaction { let node_id_initiator = initiator.node.get_our_node_id(); let node_id_acceptor = acceptor.node.get_our_node_id(); - let new_funding_script = - complete_splice_handshake(initiator, acceptor, channel_id, initiator_contribution.clone()); + let new_funding_script = complete_splice_handshake(initiator, acceptor); complete_interactive_funding_negotiation( initiator, acceptor, channel_id, - initiator_contribution, + funding_contribution, new_funding_script, ); let (splice_tx, splice_locked) = sign_interactive_funding_tx(initiator, acceptor, false); @@ -384,20 +432,20 @@ pub fn splice_channel<'a, 'b, 'c, 'd>( pub fn lock_splice_after_blocks<'a, 'b, 'c, 'd>( node_a: &'a Node<'b, 'c, 'd>, node_b: &'a Node<'b, 'c, 'd>, num_blocks: u32, -) { +) -> Option { connect_blocks(node_a, num_blocks); connect_blocks(node_b, num_blocks); let node_id_b = node_b.node.get_our_node_id(); let splice_locked_for_node_b = get_event_msg!(node_a, MessageSendEvent::SendSpliceLocked, node_id_b); - lock_splice(node_a, node_b, &splice_locked_for_node_b, false); + lock_splice(node_a, node_b, &splice_locked_for_node_b, false) } pub fn lock_splice<'a, 'b, 'c, 'd>( node_a: &'a Node<'b, 'c, 'd>, node_b: &'a Node<'b, 'c, 'd>, splice_locked_for_node_b: &msgs::SpliceLocked, is_0conf: bool, -) { +) -> Option { let (prev_funding_outpoint, prev_funding_script) = node_a .chain_monitor .chain_monitor @@ -411,6 +459,15 @@ pub fn lock_splice<'a, 'b, 'c, 'd>( node_b.node.handle_splice_locked(node_id_a, splice_locked_for_node_b); let mut msg_events = node_b.node.get_and_clear_pending_msg_events(); + + // If the acceptor had a pending QuiescentAction, return the stfu message so that it can be used + // for the next splice attempt. + let node_b_stfu = msg_events + .last() + .filter(|event| matches!(event, MessageSendEvent::SendStfu { .. })) + .is_some() + .then(|| msg_events.pop().unwrap()); + assert_eq!(msg_events.len(), if is_0conf { 1 } else { 2 }, "{msg_events:?}"); if let MessageSendEvent::SendSpliceLocked { msg, .. } = msg_events.remove(0) { node_a.node.handle_splice_locked(node_id_b, &msg); @@ -457,6 +514,8 @@ pub fn lock_splice<'a, 'b, 'c, 'd>( .chain_source .remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script.clone()); node_b.chain_source.remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script); + + node_b_stfu } #[test] @@ -501,20 +560,11 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { let (_, _, channel_id, _) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 50_000_000); - let contribution = SpliceContribution::splice_out(vec![TxOut { + let outputs = vec![TxOut { value: Amount::from_sat(1_000), script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), - }]); - nodes[0] - .node - .splice_channel( - &channel_id, - &node_id_1, - contribution.clone(), - FEERATE_FLOOR_SATS_PER_KW, - None, - ) - .unwrap(); + }]; + let _ = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs.clone()); // Attempt a splice negotiation that only goes up to receiving `splice_init`. Reconnecting // should implicitly abort the negotiation and reset the splice state such that we're able to @@ -559,16 +609,7 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { reconnect_args.send_announcement_sigs = (true, true); reconnect_nodes(reconnect_args); - nodes[0] - .node - .splice_channel( - &channel_id, - &node_id_1, - contribution.clone(), - FEERATE_FLOOR_SATS_PER_KW, - None, - ) - .unwrap(); + let _ = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs.clone()); // Attempt a splice negotiation that ends mid-construction of the funding transaction. // Reconnecting should implicitly abort the negotiation and reset the splice state such that @@ -618,16 +659,7 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { reconnect_args.send_announcement_sigs = (true, true); reconnect_nodes(reconnect_args); - nodes[0] - .node - .splice_channel( - &channel_id, - &node_id_1, - contribution.clone(), - FEERATE_FLOOR_SATS_PER_KW, - None, - ) - .unwrap(); + let _ = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs.clone()); // Attempt a splice negotiation that ends before the initial `commitment_signed` messages are // exchanged. The node missing the other's `commitment_signed` upon reconnecting should @@ -705,7 +737,8 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { // Attempt a splice negotiation that completes, (i.e. `tx_signatures` are exchanged). Reconnecting // should not abort the negotiation or reset the splice state. - let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, contribution); + let funding_contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs); + let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); if reload { let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); @@ -757,20 +790,11 @@ fn test_config_reject_inbound_splices() { let (_, _, channel_id, _) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 50_000_000); - let contribution = SpliceContribution::splice_out(vec![TxOut { + let outputs = vec![TxOut { value: Amount::from_sat(1_000), script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), - }]); - nodes[0] - .node - .splice_channel( - &channel_id, - &node_id_1, - contribution.clone(), - FEERATE_FLOOR_SATS_PER_KW, - None, - ) - .unwrap(); + }]; + let _ = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs.clone()); let stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu); @@ -798,7 +822,8 @@ fn test_config_reject_inbound_splices() { reconnect_args.send_announcement_sigs = (true, true); reconnect_nodes(reconnect_args); - let _ = splice_channel(&nodes[1], &nodes[0], channel_id, contribution); + let funding_contribution = initiate_splice_out(&nodes[1], &nodes[0], channel_id, outputs); + let _ = splice_channel(&nodes[1], &nodes[0], channel_id, funding_contribution); } #[test] @@ -816,24 +841,23 @@ fn test_splice_in() { let _ = send_payment(&nodes[0], &[&nodes[1]], 100_000); - let coinbase_tx1 = provide_anchor_reserves(&nodes); - let coinbase_tx2 = provide_anchor_reserves(&nodes); - let added_value = Amount::from_sat(initial_channel_value_sat * 2); + let utxo_value = added_value * 3 / 4; let change_script = ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()); let fees = Amount::from_sat(321); - let initiator_contribution = SpliceContribution::splice_in( + provide_utxo_reserves(&nodes, 2, utxo_value); + + let funding_contribution = do_initiate_splice_in( + &nodes[0], + &nodes[1], + channel_id, added_value, - vec![ - FundingTxInput::new_p2wpkh(coinbase_tx1, 0).unwrap(), - FundingTxInput::new_p2wpkh(coinbase_tx2, 0).unwrap(), - ], Some(change_script.clone()), ); - let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, initiator_contribution); - let expected_change = Amount::ONE_BTC * 2 - added_value - fees; + let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + let expected_change = utxo_value * 2 - added_value - fees; assert_eq!( splice_tx.output.iter().find(|txout| txout.script_pubkey == change_script).unwrap().value, expected_change, @@ -868,7 +892,7 @@ fn test_splice_out() { let _ = send_payment(&nodes[0], &[&nodes[1]], 100_000); - let initiator_contribution = SpliceContribution::splice_out(vec![ + let outputs = vec![ TxOut { value: Amount::from_sat(initial_channel_value_sat / 4), script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), @@ -877,9 +901,10 @@ fn test_splice_out() { value: Amount::from_sat(initial_channel_value_sat / 4), script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), }, - ]); + ]; + let funding_contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs); - let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, initiator_contribution); + let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); mine_transaction(&nodes[0], &splice_tx); mine_transaction(&nodes[1], &splice_tx); @@ -909,14 +934,12 @@ fn test_splice_in_and_out() { let _ = send_payment(&nodes[0], &[&nodes[1]], 100_000); - let coinbase_tx1 = provide_anchor_reserves(&nodes); - let coinbase_tx2 = provide_anchor_reserves(&nodes); - // Contribute a net negative value, with fees taken from the contributed inputs and the // remaining value sent to change let htlc_limit_msat = nodes[0].node.list_channels()[0].next_outbound_htlc_limit_msat; let added_value = Amount::from_sat(htlc_limit_msat / 1000); let removed_value = added_value * 2; + let utxo_value = added_value * 3 / 4; let change_script = ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()); let fees = if cfg!(feature = "grind_signatures") { Amount::from_sat(383) @@ -926,27 +949,29 @@ fn test_splice_in_and_out() { assert!(htlc_limit_msat > initial_channel_value_sat / 2 * 1000); - let initiator_contribution = SpliceContribution::splice_in_and_out( + provide_utxo_reserves(&nodes, 2, utxo_value); + + let outputs = vec![ + TxOut { + value: removed_value / 2, + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }, + TxOut { + value: removed_value / 2, + script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), + }, + ]; + let funding_contribution = do_initiate_splice_in_and_out( + &nodes[0], + &nodes[1], + channel_id, added_value, - vec![ - FundingTxInput::new_p2wpkh(coinbase_tx1, 0).unwrap(), - FundingTxInput::new_p2wpkh(coinbase_tx2, 0).unwrap(), - ], - vec![ - TxOut { - value: removed_value / 2, - script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), - }, - TxOut { - value: removed_value / 2, - script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), - }, - ], + outputs, Some(change_script.clone()), ); - let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, initiator_contribution); - let expected_change = Amount::ONE_BTC * 2 - added_value - fees; + let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + let expected_change = utxo_value * 2 - added_value - fees; assert_eq!( splice_tx.output.iter().find(|txout| txout.script_pubkey == change_script).unwrap().value, expected_change, @@ -965,13 +990,11 @@ fn test_splice_in_and_out() { assert!(htlc_limit_msat < added_value.to_sat() * 1000); let _ = send_payment(&nodes[0], &[&nodes[1]], htlc_limit_msat); - let coinbase_tx1 = provide_anchor_reserves(&nodes); - let coinbase_tx2 = provide_anchor_reserves(&nodes); - // Contribute a net positive value, with fees taken from the contributed inputs and the // remaining value sent to change let added_value = Amount::from_sat(initial_channel_value_sat * 2); let removed_value = added_value / 2; + let utxo_value = added_value * 3 / 4; let change_script = ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()); let fees = if cfg!(feature = "grind_signatures") { Amount::from_sat(383) @@ -979,27 +1002,32 @@ fn test_splice_in_and_out() { Amount::from_sat(384) }; - let initiator_contribution = SpliceContribution::splice_in_and_out( + // Clear UTXOs so that the change output from the previous splice isn't considered + nodes[0].wallet_source.clear_utxos(); + + provide_utxo_reserves(&nodes, 2, utxo_value); + + let outputs = vec![ + TxOut { + value: removed_value / 2, + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }, + TxOut { + value: removed_value / 2, + script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), + }, + ]; + let funding_contribution = do_initiate_splice_in_and_out( + &nodes[0], + &nodes[1], + channel_id, added_value, - vec![ - FundingTxInput::new_p2wpkh(coinbase_tx1, 0).unwrap(), - FundingTxInput::new_p2wpkh(coinbase_tx2, 0).unwrap(), - ], - vec![ - TxOut { - value: removed_value / 2, - script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), - }, - TxOut { - value: removed_value / 2, - script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), - }, - ], + outputs, Some(change_script.clone()), ); - let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, initiator_contribution); - let expected_change = Amount::ONE_BTC * 2 - added_value - fees; + let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + let expected_change = utxo_value * 2 - added_value - fees; assert_eq!( splice_tx.output.iter().find(|txout| txout.script_pubkey == change_script).unwrap().value, expected_change, @@ -1016,46 +1044,108 @@ fn test_splice_in_and_out() { let htlc_limit_msat = nodes[0].node.list_channels()[0].next_outbound_htlc_limit_msat; assert!(htlc_limit_msat > initial_channel_value_sat / 2 * 1000); let _ = send_payment(&nodes[0], &[&nodes[1]], htlc_limit_msat); +} - let coinbase_tx1 = provide_anchor_reserves(&nodes); - let coinbase_tx2 = provide_anchor_reserves(&nodes); +#[test] +fn test_fails_initiating_concurrent_splices() { + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let config = test_default_channel_config(); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - // Fail adding a net contribution value of zero - let added_value = Amount::from_sat(initial_channel_value_sat * 2); - let removed_value = added_value; - let change_script = ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()); + let initial_channel_value_sat = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0); + let node_0_id = nodes[0].node.get_our_node_id(); + let node_1_id = nodes[1].node.get_our_node_id(); - let initiator_contribution = SpliceContribution::splice_in_and_out( - added_value, - vec![ - FundingTxInput::new_p2wpkh(coinbase_tx1, 0).unwrap(), - FundingTxInput::new_p2wpkh(coinbase_tx2, 0).unwrap(), - ], - vec![ - TxOut { - value: removed_value / 2, - script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), - }, - TxOut { - value: removed_value / 2, - script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), - }, - ], - Some(change_script), + provide_utxo_reserves(&nodes, 2, Amount::ONE_BTC); + + let outputs = vec![TxOut { + value: Amount::from_sat(initial_channel_value_sat / 4), + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }]; + let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate).unwrap(); + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let funding_contribution = funding_template.splice_out_sync(outputs.clone(), &wallet).unwrap(); + nodes[0] + .node + .funding_contributed(&channel_id, &node_1_id, funding_contribution.clone(), None) + .unwrap(); + + assert_eq!( + nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate), + Err(APIError::APIMisuseError { + err: format!( + "Channel {} cannot be spliced as one is waiting to be negotiated", + channel_id + ), + }), ); + let new_funding_script = complete_splice_handshake(&nodes[0], &nodes[1]); + assert_eq!( - nodes[0].node.splice_channel( - &channel_id, - &nodes[1].node.get_our_node_id(), - initiator_contribution, - FEERATE_FLOOR_SATS_PER_KW, - None, - ), + nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate), Err(APIError::APIMisuseError { - err: format!("Channel {} cannot be spliced; contribution cannot be zero", channel_id), + err: format!( + "Channel {} cannot be spliced as one is currently being negotiated", + channel_id + ), }), ); + + // The acceptor can enqueue a quiescent action while the current splice is pending. + let added_value = Amount::from_sat(initial_channel_value_sat); + let acceptor_template = nodes[1].node.splice_channel(&channel_id, &node_0_id, feerate).unwrap(); + let acceptor_wallet = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); + let change_script = Some(nodes[1].wallet_source.get_change_script().unwrap()); + let acceptor_contribution = + acceptor_template.splice_in_sync(change_script, added_value, &acceptor_wallet).unwrap(); + nodes[1] + .node + .funding_contributed(&channel_id, &node_0_id, acceptor_contribution, None) + .unwrap(); + + complete_interactive_funding_negotiation( + &nodes[0], + &nodes[1], + channel_id, + funding_contribution, + new_funding_script, + ); + + assert_eq!( + nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate), + Err(APIError::APIMisuseError { + err: format!( + "Channel {} cannot be spliced as one is currently being negotiated", + channel_id + ), + }), + ); + + let (splice_tx, splice_locked) = sign_interactive_funding_tx(&nodes[0], &nodes[1], false); + assert!(splice_locked.is_none()); + + expect_splice_pending_event(&nodes[0], &node_1_id); + expect_splice_pending_event(&nodes[1], &node_0_id); + + // Now that the splice is pending, another splice may be initiated. + assert!(nodes[0].node.splice_channel(&channel_id, &node_1_id, feerate).is_ok()); + + mine_transaction(&nodes[0], &splice_tx); + mine_transaction(&nodes[1], &splice_tx); + let stfu = lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); + + // However, the acceptor had enqueued a quiescent action while the splice was pending, so it + // will now attempt to initiate quiescence. + assert!( + matches!(stfu, Some(MessageSendEvent::SendStfu { node_id, .. }) if node_id == node_0_id) + ); } #[cfg(test)] @@ -1091,16 +1181,19 @@ fn do_test_splice_commitment_broadcast(splice_status: SpliceStatus, claim_htlcs: let (_, _, channel_id, initial_funding_tx) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); - let coinbase_tx = provide_anchor_reserves(&nodes); + let coinbase_tx = provide_utxo_reserves(&nodes, 1, Amount::ONE_BTC); // We want to have two HTLCs pending to make sure we can claim those sent before and after a // splice negotiation. let payment_amount = 1_000_000; let (preimage1, payment_hash1, ..) = route_payment(&nodes[0], &[&nodes[1]], payment_amount); + let splice_in_amount = initial_channel_capacity / 2; - let initiator_contribution = SpliceContribution::splice_in( + let initiator_contribution = do_initiate_splice_in( + &nodes[0], + &nodes[1], + channel_id, Amount::from_sat(splice_in_amount), - vec![FundingTxInput::new_p2wpkh(coinbase_tx.clone(), 0).unwrap()], Some(nodes[0].wallet_source.get_change_script().unwrap()), ); let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, initiator_contribution); @@ -1296,7 +1389,7 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { route_payment(&nodes[0], &[&nodes[1]], 1_000_000); // Negotiate the splice up until the nodes exchange `tx_complete`. - let initiator_contribution = SpliceContribution::splice_out(vec![ + let outputs = vec![ TxOut { value: Amount::from_sat(initial_channel_value_sat / 4), script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), @@ -1305,7 +1398,8 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { value: Amount::from_sat(initial_channel_value_sat / 4), script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), }, - ]); + ]; + let initiator_contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs); negotiate_splice_tx(&nodes[0], &nodes[1], channel_id, initiator_contribution); // Node 0 should have a signing event to handle since they had a contribution in the splice. @@ -1582,36 +1676,35 @@ fn do_test_propose_splice_while_disconnected(reload: bool, use_0conf: bool) { nodes[1].node.peer_disconnected(node_id_0); let splice_out_sat = initial_channel_value_sat / 4; - let node_0_contribution = SpliceContribution::splice_out(vec![TxOut { + let node_0_outputs = vec![TxOut { value: Amount::from_sat(splice_out_sat), script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), - }]); + }]; + let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); + let funding_template = nodes[0].node.splice_channel(&channel_id, &node_id_1, feerate).unwrap(); + let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); + let node_0_funding_contribution = + funding_template.splice_out_sync(node_0_outputs, &wallet).unwrap(); nodes[0] .node - .splice_channel( - &channel_id, - &node_id_1, - node_0_contribution.clone(), - FEERATE_FLOOR_SATS_PER_KW, - None, - ) + .funding_contributed(&channel_id, &node_id_1, node_0_funding_contribution.clone(), None) .unwrap(); + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - let node_1_contribution = SpliceContribution::splice_out(vec![TxOut { + let node_1_outputs = vec![TxOut { value: Amount::from_sat(splice_out_sat), script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), - }]); + }]; + let funding_template = nodes[1].node.splice_channel(&channel_id, &node_id_0, feerate).unwrap(); + let wallet = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); + let node_1_funding_contribution = + funding_template.splice_out_sync(node_1_outputs, &wallet).unwrap(); nodes[1] .node - .splice_channel( - &channel_id, - &node_id_0, - node_1_contribution.clone(), - FEERATE_FLOOR_SATS_PER_KW, - None, - ) + .funding_contributed(&channel_id, &node_id_0, node_1_funding_contribution.clone(), None) .unwrap(); + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); if reload { @@ -1644,6 +1737,7 @@ fn do_test_propose_splice_while_disconnected(reload: bool, use_0conf: bool) { } reconnect_args.send_stfu = (true, true); reconnect_nodes(reconnect_args); + let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); @@ -1667,7 +1761,7 @@ fn do_test_propose_splice_while_disconnected(reload: bool, use_0conf: bool) { &nodes[0], &nodes[1], channel_id, - node_0_contribution, + node_0_funding_contribution, new_funding_script, ); let (splice_tx, splice_locked) = sign_interactive_funding_tx(&nodes[0], &nodes[1], use_0conf); @@ -1806,7 +1900,7 @@ fn do_test_propose_splice_while_disconnected(reload: bool, use_0conf: bool) { &nodes[1], &nodes[0], channel_id, - node_1_contribution, + node_1_funding_contribution, new_funding_script, ); let (splice_tx, splice_locked) = sign_interactive_funding_tx(&nodes[1], &nodes[0], use_0conf); @@ -1845,17 +1939,15 @@ fn disconnect_on_unexpected_interactive_tx_message() { let (_, _, channel_id, _) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); - let coinbase_tx = provide_anchor_reserves(&nodes); + provide_utxo_reserves(&nodes, 1, Amount::ONE_BTC); + let splice_in_amount = initial_channel_capacity / 2; - let contribution = SpliceContribution::splice_in( - Amount::from_sat(splice_in_amount), - vec![FundingTxInput::new_p2wpkh(coinbase_tx, 0).unwrap()], - Some(nodes[0].wallet_source.get_change_script().unwrap()), - ); + let contribution = + initiate_splice_in(initiator, acceptor, channel_id, Amount::from_sat(splice_in_amount)); // Complete interactive-tx construction, but fail by having the acceptor send a duplicate // tx_complete instead of commitment_signed. - negotiate_splice_tx(initiator, acceptor, channel_id, contribution.clone()); + negotiate_splice_tx(initiator, acceptor, channel_id, contribution); let _ = get_event!(initiator, Event::FundingTransactionReadyForSigning); let _ = get_htlc_update_msgs(acceptor, &node_id_initiator); @@ -1883,17 +1975,15 @@ fn fail_splice_on_interactive_tx_error() { let (_, _, channel_id, _) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); - let coinbase_tx = provide_anchor_reserves(&nodes); + provide_utxo_reserves(&nodes, 1, Amount::ONE_BTC); + let splice_in_amount = initial_channel_capacity / 2; - let contribution = SpliceContribution::splice_in( - Amount::from_sat(splice_in_amount), - vec![FundingTxInput::new_p2wpkh(coinbase_tx, 0).unwrap()], - Some(nodes[0].wallet_source.get_change_script().unwrap()), - ); // Fail during interactive-tx construction by having the acceptor echo back tx_add_input instead // of sending tx_complete. The failure occurs because the serial id will have the wrong parity. - let _ = complete_splice_handshake(initiator, acceptor, channel_id, contribution.clone()); + let funding_contribution = + initiate_splice_in(initiator, acceptor, channel_id, Amount::from_sat(splice_in_amount)); + let _ = complete_splice_handshake(initiator, acceptor); let tx_add_input = get_event_msg!(initiator, MessageSendEvent::SendTxAddInput, node_id_acceptor); @@ -1907,7 +1997,7 @@ fn fail_splice_on_interactive_tx_error() { match event { Event::SpliceFailed { contributed_inputs, .. } => { assert_eq!(contributed_inputs.len(), 1); - assert_eq!(contributed_inputs[0], contribution.inputs()[0].outpoint()); + assert_eq!(contributed_inputs[0], funding_contribution.into_tx_parts().0[0].outpoint()); }, _ => panic!("Expected Event::SpliceFailed"), } @@ -1936,17 +2026,15 @@ fn fail_splice_on_tx_abort() { let (_, _, channel_id, _) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); - let coinbase_tx = provide_anchor_reserves(&nodes); + provide_utxo_reserves(&nodes, 1, Amount::ONE_BTC); + let splice_in_amount = initial_channel_capacity / 2; - let contribution = SpliceContribution::splice_in( - Amount::from_sat(splice_in_amount), - vec![FundingTxInput::new_p2wpkh(coinbase_tx, 0).unwrap()], - Some(nodes[0].wallet_source.get_change_script().unwrap()), - ); // Fail during interactive-tx construction by having the acceptor send tx_abort instead of // tx_complete. - let _ = complete_splice_handshake(initiator, acceptor, channel_id, contribution.clone()); + let funding_contribution = + initiate_splice_in(initiator, acceptor, channel_id, Amount::from_sat(splice_in_amount)); + let _ = complete_splice_handshake(initiator, acceptor); let tx_add_input = get_event_msg!(initiator, MessageSendEvent::SendTxAddInput, node_id_acceptor); @@ -1963,7 +2051,7 @@ fn fail_splice_on_tx_abort() { match event { Event::SpliceFailed { contributed_inputs, .. } => { assert_eq!(contributed_inputs.len(), 1); - assert_eq!(contributed_inputs[0], contribution.inputs()[0].outpoint()); + assert_eq!(contributed_inputs[0], funding_contribution.into_tx_parts().0[0].outpoint()); }, _ => panic!("Expected Event::SpliceFailed"), } @@ -1989,16 +2077,13 @@ fn fail_splice_on_channel_close() { let (_, _, channel_id, _) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); - let coinbase_tx = provide_anchor_reserves(&nodes); + provide_utxo_reserves(&nodes, 1, Amount::ONE_BTC); + let splice_in_amount = initial_channel_capacity / 2; - let contribution = SpliceContribution::splice_in( - Amount::from_sat(splice_in_amount), - vec![FundingTxInput::new_p2wpkh(coinbase_tx, 0).unwrap()], - Some(nodes[0].wallet_source.get_change_script().unwrap()), - ); // Close the channel before completion of interactive-tx construction. - let _ = complete_splice_handshake(initiator, acceptor, channel_id, contribution.clone()); + let _ = initiate_splice_in(initiator, acceptor, channel_id, Amount::from_sat(splice_in_amount)); + let _ = complete_splice_handshake(initiator, acceptor); let _tx_add_input = get_event_msg!(initiator, MessageSendEvent::SendTxAddInput, node_id_acceptor); @@ -2039,25 +2124,12 @@ fn fail_quiescent_action_on_channel_close() { let (_, _, channel_id, _) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); - let coinbase_tx = provide_anchor_reserves(&nodes); let splice_in_amount = initial_channel_capacity / 2; - let contribution = SpliceContribution::splice_in( - Amount::from_sat(splice_in_amount), - vec![FundingTxInput::new_p2wpkh(coinbase_tx, 0).unwrap()], - Some(nodes[0].wallet_source.get_change_script().unwrap()), - ); + + provide_utxo_reserves(&nodes, 1, Amount::ONE_BTC); // Close the channel before completion of STFU handshake. - initiator - .node - .splice_channel( - &channel_id, - &node_id_acceptor, - contribution, - FEERATE_FLOOR_SATS_PER_KW, - None, - ) - .unwrap(); + let _ = initiate_splice_in(initiator, acceptor, channel_id, Amount::from_sat(splice_in_amount)); let _stfu_init = get_event_msg!(initiator, MessageSendEvent::SendStfu, node_id_acceptor); @@ -2134,19 +2206,21 @@ fn do_test_splice_with_inflight_htlc_forward_and_resolution(expire_scid_pre_forw // Splice both channels, lock them, and connect enough blocks to trigger the legacy SCID pruning // logic while the HTLC is still pending. - let contribution = SpliceContribution::splice_out(vec![TxOut { + let outputs_0_1 = vec![TxOut { value: Amount::from_sat(1_000), script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), - }]); + }]; + let contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id_0_1, outputs_0_1); let splice_tx_0_1 = splice_channel(&nodes[0], &nodes[1], channel_id_0_1, contribution); for node in &nodes { mine_transaction(node, &splice_tx_0_1); } - let contribution = SpliceContribution::splice_out(vec![TxOut { + let outputs_1_2 = vec![TxOut { value: Amount::from_sat(1_000), script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), - }]); + }]; + let contribution = initiate_splice_out(&nodes[1], &nodes[2], channel_id_1_2, outputs_1_2); let splice_tx_1_2 = splice_channel(&nodes[1], &nodes[2], channel_id_1_2, contribution); for node in &nodes { mine_transaction(node, &splice_tx_1_2); @@ -2250,10 +2324,11 @@ fn test_splice_buffer_commitment_signed_until_funding_tx_signed() { // Negotiate a splice-out where only the initiator (node 0) has a contribution. // This means node 1 will send their commitment_signed immediately after tx_complete. - let initiator_contribution = SpliceContribution::splice_out(vec![TxOut { + let outputs = vec![TxOut { value: Amount::from_sat(1_000), script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), - }]); + }]; + let initiator_contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs); negotiate_splice_tx(&nodes[0], &nodes[1], channel_id, initiator_contribution); // Node 0 (initiator with contribution) should have a signing event to handle. @@ -2370,10 +2445,11 @@ fn test_splice_buffer_invalid_commitment_signed_closes_channel() { // Negotiate a splice-out where only the initiator (node 0) has a contribution. // This means node 1 will send their commitment_signed immediately after tx_complete. - let initiator_contribution = SpliceContribution::splice_out(vec![TxOut { + let outputs = vec![TxOut { value: Amount::from_sat(1_000), script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), - }]); + }]; + let initiator_contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs); negotiate_splice_tx(&nodes[0], &nodes[1], channel_id, initiator_contribution); // Node 0 (initiator with contribution) should have a signing event to handle. diff --git a/lightning/src/ln/zero_fee_commitment_tests.rs b/lightning/src/ln/zero_fee_commitment_tests.rs index d287b6e3de1..b7221552603 100644 --- a/lightning/src/ln/zero_fee_commitment_tests.rs +++ b/lightning/src/ln/zero_fee_commitment_tests.rs @@ -129,7 +129,7 @@ fn test_htlc_claim_chunking() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &configs); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let coinbase_tx = provide_anchor_utxo_reserves(&nodes, 50, Amount::from_sat(500)); + let coinbase_tx = provide_utxo_reserves(&nodes, 50, Amount::from_sat(500)); const CHAN_CAPACITY: u64 = 10_000_000; let (_, _, chan_id, _funding_tx) = create_announced_chan_between_nodes_with_value( @@ -319,7 +319,7 @@ fn test_anchor_tx_too_big() { let node_a_id = nodes[0].node.get_our_node_id(); - let _coinbase_tx_a = provide_anchor_utxo_reserves(&nodes, 50, Amount::from_sat(500)); + let _coinbase_tx_a = provide_utxo_reserves(&nodes, 50, Amount::from_sat(500)); const CHAN_CAPACITY: u64 = 10_000_000; let (_, _, chan_id, _funding_tx) = create_announced_chan_between_nodes_with_value( diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 6579c0353a3..2eace55a4bf 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -41,6 +41,7 @@ use bitcoin::secp256k1::ecdsa; use bitcoin::secp256k1::schnorr; use bitcoin::secp256k1::{PublicKey, SecretKey}; use bitcoin::transaction::{OutPoint, Transaction, TxOut}; +use bitcoin::FeeRate; use bitcoin::{consensus, Sequence, TxIn, Weight, Witness}; use dnssec_prover::rr::Name; @@ -1426,6 +1427,19 @@ impl Readable for Weight { } } +impl Writeable for FeeRate { + fn write(&self, w: &mut W) -> Result<(), io::Error> { + self.to_sat_per_kwu().write(w) + } +} + +impl Readable for FeeRate { + fn read(r: &mut R) -> Result { + let sat_kwu: u64 = Readable::read(r)?; + Ok(FeeRate::from_sat_per_kwu(sat_kwu)) + } +} + impl Writeable for Txid { fn write(&self, w: &mut W) -> Result<(), io::Error> { w.write_all(&self[..]) From b1c180295959256c0871ae92058e44d54d0f9ae8 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 20 Jan 2026 12:08:20 -0600 Subject: [PATCH 5/7] Use CoinSelection::change_output when splicing Now that CoinSelection is used to fund a splice funding transaction, use that for determining of a change output should be used. Previously, the initiator could either provide a change script upfront or let LDK generate one using SignerProvider::get_destination_script. Since older versions may have serialized a SpliceInstruction without a change script while waiting on quiescence, LDK must still generate a change output in this case. --- fuzz/src/chanmon_consistency.rs | 196 +++++++++++------- fuzz/src/full_stack.rs | 7 +- .../src/upgrade_downgrade_tests.rs | 2 +- lightning/src/ln/channel.rs | 140 +++++++------ lightning/src/ln/funding.rs | 101 ++++----- lightning/src/ln/interactivetxs.rs | 1 - lightning/src/ln/splicing_tests.rs | 137 +++++------- 7 files changed, 306 insertions(+), 278 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 1a763339c8d..e08dd109dab 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -2079,80 +2079,104 @@ pub fn do_test( 0xa0 => { let feerate_sat_per_kw = fee_estimators[0].ret_val.load(atomic::Ordering::Acquire); let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); - match nodes[0].splice_channel( - &chan_a_id, - &nodes[1].get_our_node_id(), - None, - feerate, - ) { + match nodes[0].splice_channel(&chan_a_id, &nodes[1].get_our_node_id(), feerate) { Ok(funding_template) => { let wallet = WalletSync::new(&wallets[0], Arc::clone(&loggers[0])); - if let Ok(contribution) = funding_template.splice_in_sync(None, Amount::from_sat(10_000), &wallet) { - let _ = nodes[0].funding_contributed(&chan_a_id, &nodes[1].get_our_node_id(), contribution, None); + if let Ok(contribution) = + funding_template.splice_in_sync(Amount::from_sat(10_000), &wallet) + { + let _ = nodes[0].funding_contributed( + &chan_a_id, + &nodes[1].get_our_node_id(), + contribution, + None, + ); } }, Err(e) => { - assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + assert!( + matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), + "{:?}", + e + ); }, } }, 0xa1 => { let feerate_sat_per_kw = fee_estimators[1].ret_val.load(atomic::Ordering::Acquire); let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); - match nodes[1].splice_channel( - &chan_a_id, - &nodes[0].get_our_node_id(), - None, - feerate, - ) { + match nodes[1].splice_channel(&chan_a_id, &nodes[0].get_our_node_id(), feerate) { Ok(funding_template) => { let wallet = WalletSync::new(&wallets[1], Arc::clone(&loggers[1])); - if let Ok(contribution) = funding_template.splice_in_sync(None, Amount::from_sat(10_000), &wallet) { - let _ = nodes[1].funding_contributed(&chan_a_id, &nodes[0].get_our_node_id(), contribution, None); + if let Ok(contribution) = + funding_template.splice_in_sync(Amount::from_sat(10_000), &wallet) + { + let _ = nodes[1].funding_contributed( + &chan_a_id, + &nodes[0].get_our_node_id(), + contribution, + None, + ); } }, Err(e) => { - assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + assert!( + matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), + "{:?}", + e + ); }, } }, 0xa2 => { let feerate_sat_per_kw = fee_estimators[1].ret_val.load(atomic::Ordering::Acquire); let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); - match nodes[1].splice_channel( - &chan_b_id, - &nodes[2].get_our_node_id(), - None, - feerate, - ) { + match nodes[1].splice_channel(&chan_b_id, &nodes[2].get_our_node_id(), feerate) { Ok(funding_template) => { let wallet = WalletSync::new(&wallets[1], Arc::clone(&loggers[1])); - if let Ok(contribution) = funding_template.splice_in_sync(None, Amount::from_sat(10_000), &wallet) { - let _ = nodes[1].funding_contributed(&chan_b_id, &nodes[2].get_our_node_id(), contribution, None); + if let Ok(contribution) = + funding_template.splice_in_sync(Amount::from_sat(10_000), &wallet) + { + let _ = nodes[1].funding_contributed( + &chan_b_id, + &nodes[2].get_our_node_id(), + contribution, + None, + ); } }, Err(e) => { - assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + assert!( + matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), + "{:?}", + e + ); }, } }, 0xa3 => { let feerate_sat_per_kw = fee_estimators[2].ret_val.load(atomic::Ordering::Acquire); let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); - match nodes[2].splice_channel( - &chan_b_id, - &nodes[1].get_our_node_id(), - None, - feerate, - ) { + match nodes[2].splice_channel(&chan_b_id, &nodes[1].get_our_node_id(), feerate) { Ok(funding_template) => { let wallet = WalletSync::new(&wallets[2], Arc::clone(&loggers[2])); - if let Ok(contribution) = funding_template.splice_in_sync(None, Amount::from_sat(10_000), &wallet) { - let _ = nodes[2].funding_contributed(&chan_b_id, &nodes[1].get_our_node_id(), contribution, None); + if let Ok(contribution) = + funding_template.splice_in_sync(Amount::from_sat(10_000), &wallet) + { + let _ = nodes[2].funding_contributed( + &chan_b_id, + &nodes[1].get_our_node_id(), + contribution, + None, + ); } }, Err(e) => { - assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + assert!( + matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), + "{:?}", + e + ); }, } }, @@ -2171,24 +2195,31 @@ pub fn do_test( let feerate_sat_per_kw = fee_estimators[0].ret_val.load(atomic::Ordering::Acquire); let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); - match nodes[0].splice_channel( - &chan_a_id, - &nodes[1].get_our_node_id(), - None, - feerate, - ) { + match nodes[0].splice_channel(&chan_a_id, &nodes[1].get_our_node_id(), feerate) + { Ok(funding_template) => { let outputs = vec![TxOut { value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), script_pubkey: coinbase_tx.output[0].script_pubkey.clone(), }]; let wallet = WalletSync::new(&wallets[0], Arc::clone(&loggers[0])); - if let Ok(contribution) = funding_template.splice_out_sync(outputs, &wallet) { - let _ = nodes[0].funding_contributed(&chan_a_id, &nodes[1].get_our_node_id(), contribution, None); + if let Ok(contribution) = + funding_template.splice_out_sync(outputs, &wallet) + { + let _ = nodes[0].funding_contributed( + &chan_a_id, + &nodes[1].get_our_node_id(), + contribution, + None, + ); } }, Err(e) => { - assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + assert!( + matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), + "{:?}", + e + ); }, } } @@ -2204,24 +2235,31 @@ pub fn do_test( let feerate_sat_per_kw = fee_estimators[1].ret_val.load(atomic::Ordering::Acquire); let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); - match nodes[1].splice_channel( - &chan_a_id, - &nodes[0].get_our_node_id(), - None, - feerate, - ) { + match nodes[1].splice_channel(&chan_a_id, &nodes[0].get_our_node_id(), feerate) + { Ok(funding_template) => { let outputs = vec![TxOut { value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), script_pubkey: coinbase_tx.output[1].script_pubkey.clone(), }]; let wallet = WalletSync::new(&wallets[1], Arc::clone(&loggers[1])); - if let Ok(contribution) = funding_template.splice_out_sync(outputs, &wallet) { - let _ = nodes[1].funding_contributed(&chan_a_id, &nodes[0].get_our_node_id(), contribution, None); + if let Ok(contribution) = + funding_template.splice_out_sync(outputs, &wallet) + { + let _ = nodes[1].funding_contributed( + &chan_a_id, + &nodes[0].get_our_node_id(), + contribution, + None, + ); } }, Err(e) => { - assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + assert!( + matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), + "{:?}", + e + ); }, } } @@ -2237,24 +2275,31 @@ pub fn do_test( let feerate_sat_per_kw = fee_estimators[1].ret_val.load(atomic::Ordering::Acquire); let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); - match nodes[1].splice_channel( - &chan_b_id, - &nodes[2].get_our_node_id(), - None, - feerate, - ) { + match nodes[1].splice_channel(&chan_b_id, &nodes[2].get_our_node_id(), feerate) + { Ok(funding_template) => { let outputs = vec![TxOut { value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), script_pubkey: coinbase_tx.output[1].script_pubkey.clone(), }]; let wallet = WalletSync::new(&wallets[1], Arc::clone(&loggers[1])); - if let Ok(contribution) = funding_template.splice_out_sync(outputs, &wallet) { - let _ = nodes[1].funding_contributed(&chan_b_id, &nodes[2].get_our_node_id(), contribution, None); + if let Ok(contribution) = + funding_template.splice_out_sync(outputs, &wallet) + { + let _ = nodes[1].funding_contributed( + &chan_b_id, + &nodes[2].get_our_node_id(), + contribution, + None, + ); } }, Err(e) => { - assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + assert!( + matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), + "{:?}", + e + ); }, } } @@ -2270,24 +2315,31 @@ pub fn do_test( let feerate_sat_per_kw = fee_estimators[2].ret_val.load(atomic::Ordering::Acquire); let feerate = FeeRate::from_sat_per_kwu(feerate_sat_per_kw as u64); - match nodes[2].splice_channel( - &chan_b_id, - &nodes[1].get_our_node_id(), - None, - feerate, - ) { + match nodes[2].splice_channel(&chan_b_id, &nodes[1].get_our_node_id(), feerate) + { Ok(funding_template) => { let outputs = vec![TxOut { value: Amount::from_sat(MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS), script_pubkey: coinbase_tx.output[2].script_pubkey.clone(), }]; let wallet = WalletSync::new(&wallets[2], Arc::clone(&loggers[2])); - if let Ok(contribution) = funding_template.splice_out_sync(outputs, &wallet) { - let _ = nodes[2].funding_contributed(&chan_b_id, &nodes[1].get_our_node_id(), contribution, None); + if let Ok(contribution) = + funding_template.splice_out_sync(outputs, &wallet) + { + let _ = nodes[2].funding_contributed( + &chan_b_id, + &nodes[1].get_our_node_id(), + contribution, + None, + ); } }, Err(e) => { - assert!(matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), "{:?}", e); + assert!( + matches!(e, APIError::APIMisuseError { ref err } if err.contains("splice")), + "{:?}", + e + ); }, } } diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index ebbd93f0052..de2c540cf92 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -1035,12 +1035,10 @@ pub fn do_test(mut data: &[u8], logger: &Arc if let Ok(funding_template) = channelmanager.splice_channel( &chan_id, &counterparty, - None, FeeRate::from_sat_per_kwu(253), ) { let wallet_sync = WalletSync::new(&wallet, Arc::clone(&logger)); if let Ok(contribution) = funding_template.splice_in_sync( - None, Amount::from_sat(splice_in_sats.min(900_000)), &wallet_sync, ) { @@ -1078,7 +1076,6 @@ pub fn do_test(mut data: &[u8], logger: &Arc if let Ok(funding_template) = channelmanager.splice_channel( &chan_id, &counterparty, - None, FeeRate::from_sat_per_kwu(253), ) { let outputs = vec![TxOut { @@ -1086,7 +1083,9 @@ pub fn do_test(mut data: &[u8], logger: &Arc script_pubkey: wallet.get_change_script().unwrap(), }]; let wallet_sync = WalletSync::new(&wallet, Arc::clone(&logger)); - if let Ok(contribution) = funding_template.splice_out_sync(outputs, &wallet_sync) { + if let Ok(contribution) = + funding_template.splice_out_sync(outputs, &wallet_sync) + { let _ = channelmanager.funding_contributed( &chan_id, &counterparty, diff --git a/lightning-tests/src/upgrade_downgrade_tests.rs b/lightning-tests/src/upgrade_downgrade_tests.rs index dde194105c3..f18e0e56800 100644 --- a/lightning-tests/src/upgrade_downgrade_tests.rs +++ b/lightning-tests/src/upgrade_downgrade_tests.rs @@ -458,7 +458,7 @@ fn do_test_0_1_htlc_forward_after_splice(fail_htlc: bool) { }]; let channel_id = ChannelId(chan_id_bytes_a); let funding_contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs); - let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); for node in nodes.iter() { mine_transaction(node, &splice_tx); connect_blocks(node, ANTI_REORG_DELAY - 1); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e000ebc93eb..db85a26ae91 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2908,6 +2908,7 @@ impl_writeable_tlv_based!(PendingFunding, { enum FundingNegotiation { AwaitingAck { context: FundingNegotiationContext, + change_strategy: ChangeStrategy, new_holder_funding_key: PublicKey, }, ConstructingTransaction { @@ -6680,10 +6681,17 @@ pub(super) struct FundingNegotiationContext { /// The funding outputs we will be contributing to the channel. #[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled. pub our_funding_outputs: Vec, +} + +/// How the funding transaction's change is determined. +#[derive(Debug)] +pub(super) enum ChangeStrategy { + /// The change output, if any, is included in the FundingContribution's outputs. + FromCoinSelection, + /// The change output script. This will be used if needed or -- if not set -- generated using /// `SignerProvider::get_destination_script`. - #[allow(dead_code)] // TODO(splicing): Remove once splicing is enabled. - pub change_script: Option, + LegacyUserProvided(Option), } impl FundingNegotiationContext { @@ -6691,7 +6699,7 @@ impl FundingNegotiationContext { /// If error occurs, it is caused by our side, not the counterparty. fn into_interactive_tx_constructor( mut self, context: &ChannelContext, funding: &FundingScope, signer_provider: &SP, - entropy_source: &ES, holder_node_id: PublicKey, + entropy_source: &ES, holder_node_id: PublicKey, change_strategy: ChangeStrategy, ) -> Result { debug_assert_eq!( self.shared_funding_input.is_some(), @@ -6712,46 +6720,15 @@ impl FundingNegotiationContext { script_pubkey: funding.get_funding_redeemscript().to_p2wsh(), }; - // Optionally add change output - let change_value_opt = if !self.our_funding_inputs.is_empty() { - match calculate_change_output_value( - &self, - self.shared_funding_input.is_some(), - &shared_funding_output.script_pubkey, - context.holder_dust_limit_satoshis, - ) { - Ok(change_value_opt) => change_value_opt, - Err(reason) => { - return Err(self.into_negotiation_error(reason)); - }, - } - } else { - None - }; - - if let Some(change_value) = change_value_opt { - let change_script = if let Some(script) = self.change_script { - script - } else { - match signer_provider.get_destination_script(context.channel_keys_id) { - Ok(script) => script, - Err(_) => { - let reason = AbortReason::InternalError("Error getting change script"); - return Err(self.into_negotiation_error(reason)); - }, - } - }; - let mut change_output = TxOut { value: change_value, script_pubkey: change_script }; - let change_output_weight = get_output_weight(&change_output.script_pubkey).to_wu(); - let change_output_fee = - fee_for_weight(self.funding_feerate_sat_per_1000_weight, change_output_weight); - let change_value_decreased_with_fee = - change_value.to_sat().saturating_sub(change_output_fee); - // Check dust limit again - if change_value_decreased_with_fee > context.holder_dust_limit_satoshis { - change_output.value = Amount::from_sat(change_value_decreased_with_fee); - self.our_funding_outputs.push(change_output); - } + match self.calculate_change_output( + context, + signer_provider, + &shared_funding_output, + change_strategy, + ) { + Ok(Some(change_output)) => self.our_funding_outputs.push(change_output), + Ok(None) => {}, + Err(reason) => return Err(self.into_negotiation_error(reason)), } let constructor_args = InteractiveTxConstructorArgs { @@ -6773,6 +6750,52 @@ impl FundingNegotiationContext { InteractiveTxConstructor::new(constructor_args) } + fn calculate_change_output( + &self, context: &ChannelContext, signer_provider: &SP, shared_funding_output: &TxOut, + change_strategy: ChangeStrategy, + ) -> Result, AbortReason> { + if self.our_funding_inputs.is_empty() { + return Ok(None); + } + + let change_script = match change_strategy { + ChangeStrategy::FromCoinSelection => return Ok(None), + ChangeStrategy::LegacyUserProvided(change_script) => change_script, + }; + + let change_value = calculate_change_output_value( + &self, + self.shared_funding_input.is_some(), + &shared_funding_output.script_pubkey, + context.holder_dust_limit_satoshis, + )?; + + if let Some(change_value) = change_value { + let change_script = match change_script { + Some(script) => script, + None => match signer_provider.get_destination_script(context.channel_keys_id) { + Ok(script) => script, + Err(_) => { + return Err(AbortReason::InternalError("Error getting change script")) + }, + }, + }; + let mut change_output = TxOut { value: change_value, script_pubkey: change_script }; + let change_output_weight = get_output_weight(&change_output.script_pubkey).to_wu(); + let change_output_fee = + fee_for_weight(self.funding_feerate_sat_per_1000_weight, change_output_weight); + let change_value_decreased_with_fee = + change_value.to_sat().saturating_sub(change_output_fee); + // Check dust limit again + if change_value_decreased_with_fee > context.holder_dust_limit_satoshis { + change_output.value = Amount::from_sat(change_value_decreased_with_fee); + return Ok(Some(change_output)); + } + } + + Ok(None) + } + fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { let (contributed_inputs, contributed_outputs) = self.into_contributed_inputs_and_outputs(); NegotiationError { reason, contributed_inputs, contributed_outputs } @@ -12235,14 +12258,13 @@ where shared_funding_input: Some(prev_funding_input), our_funding_inputs, our_funding_outputs, - change_script, }; - self.send_splice_init_internal(context) + self.send_splice_init_internal(context, ChangeStrategy::LegacyUserProvided(change_script)) } fn send_splice_init_internal( - &mut self, context: FundingNegotiationContext, + &mut self, context: FundingNegotiationContext, change_strategy: ChangeStrategy, ) -> msgs::SpliceInit { debug_assert!(self.pending_splice.is_none()); // Rotate the funding pubkey using the prev_funding_txid as a tweak @@ -12263,8 +12285,11 @@ where let funding_contribution_satoshis = context.our_funding_contribution.to_sat(); let locktime = context.funding_tx_locktime.to_consensus_u32(); - let funding_negotiation = - FundingNegotiation::AwaitingAck { context, new_holder_funding_key: funding_pubkey }; + let funding_negotiation = FundingNegotiation::AwaitingAck { + context, + change_strategy, + new_holder_funding_key: funding_pubkey, + }; self.pending_splice = Some(PendingFunding { funding_negotiation: Some(funding_negotiation), negotiated_candidates: vec![], @@ -12490,7 +12515,6 @@ where shared_funding_input: Some(prev_funding_input), our_funding_inputs: Vec::new(), our_funding_outputs: Vec::new(), - change_script: None, }; let mut interactive_tx_constructor = funding_negotiation_context @@ -12500,6 +12524,8 @@ where signer_provider, entropy_source, holder_node_id.clone(), + // ChangeStrategy doesn't matter when no inputs are contributed + ChangeStrategy::FromCoinSelection, ) .map_err(|err| { ChannelError::WarnAndDisconnect(format!( @@ -12550,11 +12576,11 @@ where let pending_splice = self.pending_splice.as_mut().expect("We should have returned an error earlier!"); // TODO: Good candidate for a let else statement once MSRV >= 1.65 - let funding_negotiation_context = - if let Some(FundingNegotiation::AwaitingAck { context, .. }) = + let (funding_negotiation_context, change_strategy) = + if let Some(FundingNegotiation::AwaitingAck { context, change_strategy, .. }) = pending_splice.funding_negotiation.take() { - context + (context, change_strategy) } else { panic!("We should have returned an error earlier!"); }; @@ -12566,6 +12592,7 @@ where signer_provider, entropy_source, holder_node_id.clone(), + change_strategy, ) .map_err(|err| { ChannelError::WarnAndDisconnect(format!( @@ -12596,7 +12623,7 @@ where let (funding_negotiation_context, new_holder_funding_key) = match &pending_splice .funding_negotiation { - Some(FundingNegotiation::AwaitingAck { context, new_holder_funding_key }) => { + Some(FundingNegotiation::AwaitingAck { context, new_holder_funding_key, .. }) => { (context, new_holder_funding_key) }, Some(FundingNegotiation::ConstructingTransaction { .. }) @@ -13523,7 +13550,7 @@ where }, }; let funding_feerate_per_kw = contribution.feerate().to_sat_per_kwu() as u32; - let (our_funding_inputs, our_funding_outputs, change_script) = contribution.into_tx_parts(); + let (our_funding_inputs, our_funding_outputs) = contribution.into_tx_parts(); let context = FundingNegotiationContext { is_initiator, @@ -13533,10 +13560,9 @@ where shared_funding_input: Some(prev_funding_input), our_funding_inputs, our_funding_outputs, - change_script, }; - let splice_init = self.send_splice_init_internal(context); + let splice_init = self.send_splice_init_internal(context, ChangeStrategy::FromCoinSelection); return Ok(Some(StfuResponse::SpliceInit(splice_init))); }, #[cfg(any(test, fuzzing))] @@ -14320,7 +14346,6 @@ impl PendingV2Channel { shared_funding_input: None, our_funding_inputs: funding_inputs, our_funding_outputs: Vec::new(), - change_script: None, }; let chan = Self { funding, @@ -14467,7 +14492,6 @@ impl PendingV2Channel { shared_funding_input: None, our_funding_inputs: our_funding_inputs.clone(), our_funding_outputs: Vec::new(), - change_script: None, }; let shared_funding_output = TxOut { value: Amount::from_sat(funding.get_value_satoshis()), diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index e369bd8a3ec..06b972d7126 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -19,7 +19,7 @@ use bitcoin::{ use core::ops::Deref; use crate::events::bump_transaction::sync::CoinSelectionSourceSync; -use crate::events::bump_transaction::{CoinSelectionSource, Input, Utxo}; +use crate::events::bump_transaction::{CoinSelection, CoinSelectionSource, Input, Utxo}; use crate::ln::chan_utils::{ make_funding_redeemscript, BASE_INPUT_WEIGHT, EMPTY_SCRIPT_SIG_WEIGHT, FUNDING_TRANSACTION_WITNESS_WEIGHT, @@ -57,18 +57,15 @@ pub struct FundingTemplate { impl FundingTemplate { /// Constructs a [`FundingTemplate`] for a splice using the provided shared input. - pub(super) fn new( - shared_input: Option, feerate: FeeRate, is_initiator: bool, - ) -> Self { + pub(super) fn new(shared_input: Option, feerate: FeeRate, is_initiator: bool) -> Self { Self { shared_input, feerate, is_initiator } } } macro_rules! build_funding_contribution { - ($value_added:expr, $outputs:expr, $change_script:expr, $shared_input:expr, $feerate:expr, $is_initiator:expr, $wallet:ident, $($await:tt)*) => {{ + ($value_added:expr, $outputs:expr, $shared_input:expr, $feerate:expr, $is_initiator:expr, $wallet:ident, $($await:tt)*) => {{ let value_added: Amount = $value_added; let outputs: Vec = $outputs; - let change_script: Option = $change_script; let shared_input: Option = $shared_input; let feerate: FeeRate = $feerate; let is_initiator: bool = $is_initiator; @@ -76,8 +73,8 @@ macro_rules! build_funding_contribution { let value_removed = outputs.iter().map(|txout| txout.value).sum(); let is_splice = shared_input.is_some(); - let inputs = if value_added == Amount::ZERO { - vec![] + let coin_selection = if value_added == Amount::ZERO { + CoinSelection { confirmed_utxos: vec![], change_output: None } } else { // Used for creating a redeem script for the new funding txo, since the funding pubkeys // are unknown at this point. Only needed when selecting which UTXOs to include in the @@ -98,18 +95,19 @@ macro_rules! build_funding_contribution { let claim_id = None; let must_spend = shared_input.map(|input| vec![input]).unwrap_or_default(); - let selection = if outputs.is_empty() { + if outputs.is_empty() { let must_pay_to = &[shared_output]; $wallet.select_confirmed_utxos(claim_id, must_spend, must_pay_to, feerate.to_sat_per_kwu() as u32, u64::MAX)$(.$await)*? } else { let must_pay_to: Vec<_> = outputs.iter().cloned().chain(core::iter::once(shared_output)).collect(); $wallet.select_confirmed_utxos(claim_id, must_spend, &must_pay_to, feerate.to_sat_per_kwu() as u32, u64::MAX)$(.$await)*? - }; - selection.confirmed_utxos + } }; // NOTE: Must NOT fail after UTXO selection + let CoinSelection { confirmed_utxos: inputs, change_output } = coin_selection; + let estimated_fee = estimate_transaction_fee(&inputs, &outputs, is_initiator, is_splice, feerate); let contribution = FundingContribution { @@ -117,7 +115,7 @@ macro_rules! build_funding_contribution { estimated_fee, inputs, outputs, - change_script, + change_output, feerate, is_initiator, is_splice, @@ -130,13 +128,8 @@ macro_rules! build_funding_contribution { impl FundingTemplate { /// Creates a [`FundingContribution`] for adding funds to a channel using `wallet` to perform /// coin selection. - /// - /// An optional `change_script` may be given to use as a change output. If `None` and change is - /// needed, one will be generated using [`SignerProvider::get_destination_script`]. - /// - /// [`SignerProvider::get_destination_script`]: crate::sign::SignerProvider::get_destination_script pub async fn splice_in( - self, change_script: Option, value_added: Amount, wallet: W, + self, value_added: Amount, wallet: W, ) -> Result where W::Target: CoinSelectionSource + MaybeSend, @@ -145,18 +138,13 @@ impl FundingTemplate { return Err(()); } let FundingTemplate { shared_input, feerate, is_initiator } = self; - build_funding_contribution!(value_added, vec![], change_script, shared_input, feerate, is_initiator, wallet, await) + build_funding_contribution!(value_added, vec![], shared_input, feerate, is_initiator, wallet, await) } /// Creates a [`FundingContribution`] for adding funds to a channel using `wallet` to perform /// coin selection. - /// - /// An optional `change_script` may be given to use as a change output. If `None` and change is - /// needed, one will be generated using [`SignerProvider::get_destination_script`]. - /// - /// [`SignerProvider::get_destination_script`]: crate::sign::SignerProvider::get_destination_script pub fn splice_in_sync( - self, change_script: Option, value_added: Amount, wallet: W, + self, value_added: Amount, wallet: W, ) -> Result where W::Target: CoinSelectionSourceSync, @@ -168,7 +156,6 @@ impl FundingTemplate { build_funding_contribution!( value_added, vec![], - change_script, shared_input, feerate, is_initiator, @@ -188,7 +175,7 @@ impl FundingTemplate { return Err(()); } let FundingTemplate { shared_input, feerate, is_initiator } = self; - build_funding_contribution!(Amount::ZERO, outputs, None, shared_input, feerate, is_initiator, wallet, await) + build_funding_contribution!(Amount::ZERO, outputs, shared_input, feerate, is_initiator, wallet, await) } /// Creates a [`FundingContribution`] for removing funds from a channel using `wallet` to @@ -206,7 +193,6 @@ impl FundingTemplate { build_funding_contribution!( Amount::ZERO, outputs, - None, shared_input, feerate, is_initiator, @@ -216,14 +202,8 @@ impl FundingTemplate { /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using /// `wallet` to perform coin selection. - /// - /// An optional `change_script` may be given to use as a change output. If `None` and change is - /// needed, one will be generated using [`SignerProvider::get_destination_script`]. - /// - /// [`SignerProvider::get_destination_script`]: crate::sign::SignerProvider::get_destination_script pub async fn splice_in_and_out( - self, change_script: Option, value_added: Amount, outputs: Vec, - wallet: W, + self, value_added: Amount, outputs: Vec, wallet: W, ) -> Result where W::Target: CoinSelectionSource + MaybeSend, @@ -232,19 +212,13 @@ impl FundingTemplate { return Err(()); } let FundingTemplate { shared_input, feerate, is_initiator } = self; - build_funding_contribution!(value_added, outputs, change_script, shared_input, feerate, is_initiator, wallet, await) + build_funding_contribution!(value_added, outputs, shared_input, feerate, is_initiator, wallet, await) } /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using /// `wallet` to perform coin selection. - /// - /// An optional `change_script` may be given to use as a change output. If `None` and change is - /// needed, one will be generated using [`SignerProvider::get_destination_script`]. - /// - /// [`SignerProvider::get_destination_script`]: crate::sign::SignerProvider::get_destination_script pub fn splice_in_and_out_sync( - self, change_script: Option, value_added: Amount, outputs: Vec, - wallet: W, + self, value_added: Amount, outputs: Vec, wallet: W, ) -> Result where W::Target: CoinSelectionSourceSync, @@ -256,7 +230,6 @@ impl FundingTemplate { build_funding_contribution!( value_added, outputs, - change_script, shared_input, feerate, is_initiator, @@ -334,11 +307,8 @@ pub struct FundingContribution { /// will be the amount that is removed. outputs: Vec, - /// An optional change output script. This will be used if needed or, when not set, - /// generated using [`SignerProvider::get_destination_script`]. - /// - /// [`SignerProvider::get_destination_script`]: crate::sign::SignerProvider::get_destination_script - change_script: Option, + /// The output where any change will be sent. + change_output: Option, /// The fee rate used to select `inputs`. feerate: FeeRate, @@ -356,7 +326,7 @@ impl_writeable_tlv_based!(FundingContribution, { (3, estimated_fee, required), (5, inputs, optional_vec), (7, outputs, optional_vec), - (9, change_script, option), + (9, change_output, option), (11, feerate, required), (13, is_initiator, required), (15, is_splice, required), @@ -375,13 +345,20 @@ impl FundingContribution { self.is_splice } - pub(super) fn into_tx_parts(self) -> (Vec, Vec, Option) { - let FundingContribution { inputs, outputs, change_script, .. } = self; - (inputs, outputs, change_script) + pub(super) fn into_tx_parts(self) -> (Vec, Vec) { + let FundingContribution { inputs, mut outputs, change_output, .. } = self; + + if let Some(change_output) = change_output { + outputs.push(change_output); + } + + (inputs, outputs) } pub(super) fn into_contributed_inputs_and_outputs(self) -> (Vec, Vec) { - (self.inputs.into_iter().map(|input| input.utxo.outpoint).collect(), self.outputs) + let (inputs, outputs) = self.into_tx_parts(); + + (inputs.into_iter().map(|input| input.utxo.outpoint).collect(), outputs) } /// The net value contributed to a channel by the splice. If negative, more value will be @@ -723,7 +700,7 @@ mod tests { funding_input_sats(100_000), ], outputs: vec![], - change_script: None, + change_output: None, is_initiator: true, is_splice: true, feerate: FeeRate::from_sat_per_kwu(2000), @@ -744,7 +721,7 @@ mod tests { outputs: vec![ funding_output_sats(200_000), ], - change_script: None, + change_output: None, is_initiator: true, is_splice: true, feerate: FeeRate::from_sat_per_kwu(2000), @@ -765,7 +742,7 @@ mod tests { outputs: vec![ funding_output_sats(400_000), ], - change_script: None, + change_output: None, is_initiator: true, is_splice: true, feerate: FeeRate::from_sat_per_kwu(2000), @@ -786,7 +763,7 @@ mod tests { outputs: vec![ funding_output_sats(400_000), ], - change_script: None, + change_output: None, is_initiator: true, is_splice: true, feerate: FeeRate::from_sat_per_kwu(90000), @@ -810,7 +787,7 @@ mod tests { funding_input_sats(100_000), ], outputs: vec![], - change_script: None, + change_output: None, is_initiator: true, is_splice: true, feerate: FeeRate::from_sat_per_kwu(2000), @@ -835,7 +812,7 @@ mod tests { funding_input_sats(100_000), ], outputs: vec![], - change_script: None, + change_output: None, is_initiator: true, is_splice: true, feerate: FeeRate::from_sat_per_kwu(2000), @@ -854,7 +831,7 @@ mod tests { funding_input_sats(100_000), ], outputs: vec![], - change_script: None, + change_output: None, is_initiator: true, is_splice: true, feerate: FeeRate::from_sat_per_kwu(2200), @@ -879,7 +856,7 @@ mod tests { funding_input_sats(100_000), ], outputs: vec![], - change_script: None, + change_output: None, is_initiator: false, is_splice: false, feerate: FeeRate::from_sat_per_kwu(2000), diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 3c47658e963..c5db1bcbe8a 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -3435,7 +3435,6 @@ mod tests { shared_funding_input: None, our_funding_inputs: inputs, our_funding_outputs: outputs, - change_script: None, }; let gross_change = total_inputs - total_outputs - context.our_funding_contribution.to_unsigned().unwrap(); diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 31c13e124d4..cc422d650a7 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -29,12 +29,9 @@ use crate::util::ser::Writeable; use crate::sync::Arc; -use bitcoin::hashes::Hash; use bitcoin::secp256k1::ecdsa::Signature; use bitcoin::secp256k1::PublicKey; -use bitcoin::{ - Amount, FeeRate, OutPoint as BitcoinOutPoint, ScriptBuf, Transaction, TxOut, WPubkeyHash, -}; +use bitcoin::{Amount, FeeRate, OutPoint as BitcoinOutPoint, ScriptBuf, Transaction, TxOut}; #[test] fn test_splicing_not_supported_api_error() { @@ -109,7 +106,7 @@ fn test_v1_splice_in_negative_insufficient_inputs() { .unwrap(); let wallet = WalletSync::new(Arc::clone(&nodes[0].wallet_source), nodes[0].logger); - assert!(funding_template.splice_in_sync(None, splice_in_value, &wallet).is_err()); + assert!(funding_template.splice_in_sync(splice_in_value, &wallet).is_err()); } pub fn negotiate_splice_tx<'a, 'b, 'c, 'd>( @@ -131,21 +128,19 @@ pub fn initiate_splice_in<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, value_added: Amount, ) -> FundingContribution { - let change_script = Some(initiator.wallet_source.get_change_script().unwrap()); - do_initiate_splice_in(initiator, acceptor, channel_id, value_added, change_script) + do_initiate_splice_in(initiator, acceptor, channel_id, value_added) } pub fn do_initiate_splice_in<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, - value_added: Amount, change_script: Option, + value_added: Amount, ) -> FundingContribution { let node_id_acceptor = acceptor.node.get_our_node_id(); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); let funding_template = initiator.node.splice_channel(&channel_id, &node_id_acceptor, feerate).unwrap(); let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger); - let funding_contribution = - funding_template.splice_in_sync(change_script, value_added, &wallet).unwrap(); + let funding_contribution = funding_template.splice_in_sync(value_added, &wallet).unwrap(); initiator .node .funding_contributed(&channel_id, &node_id_acceptor, funding_contribution.clone(), None) @@ -174,29 +169,20 @@ pub fn initiate_splice_in_and_out<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, value_added: Amount, outputs: Vec, ) -> FundingContribution { - let change_script = Some(initiator.wallet_source.get_change_script().unwrap()); - do_initiate_splice_in_and_out( - initiator, - acceptor, - channel_id, - value_added, - outputs, - change_script, - ) + do_initiate_splice_in_and_out(initiator, acceptor, channel_id, value_added, outputs) } pub fn do_initiate_splice_in_and_out<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, - value_added: Amount, outputs: Vec, change_script: Option, + value_added: Amount, outputs: Vec, ) -> FundingContribution { let node_id_acceptor = acceptor.node.get_our_node_id(); let feerate = FeeRate::from_sat_per_kwu(FEERATE_FLOOR_SATS_PER_KW as u64); let funding_template = initiator.node.splice_channel(&channel_id, &node_id_acceptor, feerate).unwrap(); let wallet = WalletSync::new(Arc::clone(&initiator.wallet_source), initiator.logger); - let funding_contribution = funding_template - .splice_in_and_out_sync(change_script, value_added, outputs, &wallet) - .unwrap(); + let funding_contribution = + funding_template.splice_in_and_out_sync(value_added, outputs, &wallet).unwrap(); initiator .node .funding_contributed(&channel_id, &node_id_acceptor, funding_contribution.clone(), None) @@ -245,8 +231,7 @@ pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( }) .map(|channel| channel.funding_txo.unwrap()) .unwrap(); - let (initiator_inputs, initiator_outputs, initiator_change_script) = - initiator_contribution.into_tx_parts(); + let (initiator_inputs, initiator_outputs) = initiator_contribution.into_tx_parts(); let mut expected_initiator_inputs = initiator_inputs .iter() .map(|input| input.utxo.outpoint) @@ -256,7 +241,6 @@ pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( .into_iter() .map(|output| output.script_pubkey) .chain(core::iter::once(new_funding_script)) - .chain(initiator_change_script.into_iter()) .collect::>(); let mut acceptor_sent_tx_complete = false; @@ -408,7 +392,7 @@ pub fn sign_interactive_funding_tx<'a, 'b, 'c, 'd>( pub fn splice_channel<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, funding_contribution: FundingContribution, -) -> Transaction { +) -> (Transaction, ScriptBuf) { let node_id_initiator = initiator.node.get_our_node_id(); let node_id_acceptor = acceptor.node.get_our_node_id(); @@ -419,7 +403,7 @@ pub fn splice_channel<'a, 'b, 'c, 'd>( acceptor, channel_id, funding_contribution, - new_funding_script, + new_funding_script.clone(), ); let (splice_tx, splice_locked) = sign_interactive_funding_tx(initiator, acceptor, false); assert!(splice_locked.is_none()); @@ -427,7 +411,7 @@ pub fn splice_channel<'a, 'b, 'c, 'd>( expect_splice_pending_event(initiator, &node_id_acceptor); expect_splice_pending_event(acceptor, &node_id_initiator); - splice_tx + (splice_tx, new_funding_script) } pub fn lock_splice_after_blocks<'a, 'b, 'c, 'd>( @@ -738,7 +722,7 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { // Attempt a splice negotiation that completes, (i.e. `tx_signatures` are exchanged). Reconnecting // should not abort the negotiation or reset the splice state. let funding_contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs); - let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); if reload { let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); @@ -843,23 +827,22 @@ fn test_splice_in() { let added_value = Amount::from_sat(initial_channel_value_sat * 2); let utxo_value = added_value * 3 / 4; - let change_script = ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()); - let fees = Amount::from_sat(321); + let fees = Amount::from_sat(322); provide_utxo_reserves(&nodes, 2, utxo_value); - let funding_contribution = do_initiate_splice_in( - &nodes[0], - &nodes[1], - channel_id, - added_value, - Some(change_script.clone()), - ); + let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); - let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + let (splice_tx, new_funding_script) = + splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); let expected_change = utxo_value * 2 - added_value - fees; assert_eq!( - splice_tx.output.iter().find(|txout| txout.script_pubkey == change_script).unwrap().value, + splice_tx + .output + .iter() + .find(|txout| txout.script_pubkey != new_funding_script) + .unwrap() + .value, expected_change, ); @@ -904,7 +887,7 @@ fn test_splice_out() { ]; let funding_contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs); - let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); mine_transaction(&nodes[0], &splice_tx); mine_transaction(&nodes[1], &splice_tx); @@ -940,11 +923,10 @@ fn test_splice_in_and_out() { let added_value = Amount::from_sat(htlc_limit_msat / 1000); let removed_value = added_value * 2; let utxo_value = added_value * 3 / 4; - let change_script = ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()); let fees = if cfg!(feature = "grind_signatures") { - Amount::from_sat(383) + Amount::from_sat(385) } else { - Amount::from_sat(384) + Amount::from_sat(385) }; assert!(htlc_limit_msat > initial_channel_value_sat / 2 * 1000); @@ -961,19 +943,20 @@ fn test_splice_in_and_out() { script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), }, ]; - let funding_contribution = do_initiate_splice_in_and_out( - &nodes[0], - &nodes[1], - channel_id, - added_value, - outputs, - Some(change_script.clone()), - ); + let funding_contribution = + do_initiate_splice_in_and_out(&nodes[0], &nodes[1], channel_id, added_value, outputs); - let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + let (splice_tx, new_funding_script) = + splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); let expected_change = utxo_value * 2 - added_value - fees; assert_eq!( - splice_tx.output.iter().find(|txout| txout.script_pubkey == change_script).unwrap().value, + splice_tx + .output + .iter() + .filter(|txout| txout.value != removed_value / 2) + .find(|txout| txout.script_pubkey != new_funding_script) + .unwrap() + .value, expected_change, ); @@ -995,11 +978,10 @@ fn test_splice_in_and_out() { let added_value = Amount::from_sat(initial_channel_value_sat * 2); let removed_value = added_value / 2; let utxo_value = added_value * 3 / 4; - let change_script = ScriptBuf::new_p2wpkh(&WPubkeyHash::all_zeros()); let fees = if cfg!(feature = "grind_signatures") { - Amount::from_sat(383) + Amount::from_sat(385) } else { - Amount::from_sat(384) + Amount::from_sat(385) }; // Clear UTXOs so that the change output from the previous splice isn't considered @@ -1017,19 +999,20 @@ fn test_splice_in_and_out() { script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), }, ]; - let funding_contribution = do_initiate_splice_in_and_out( - &nodes[0], - &nodes[1], - channel_id, - added_value, - outputs, - Some(change_script.clone()), - ); + let funding_contribution = + do_initiate_splice_in_and_out(&nodes[0], &nodes[1], channel_id, added_value, outputs); - let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + let (splice_tx, new_funding_script) = + splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); let expected_change = utxo_value * 2 - added_value - fees; assert_eq!( - splice_tx.output.iter().find(|txout| txout.script_pubkey == change_script).unwrap().value, + splice_tx + .output + .iter() + .filter(|txout| txout.value != removed_value / 2) + .find(|txout| txout.script_pubkey != new_funding_script) + .unwrap() + .value, expected_change, ); @@ -1102,9 +1085,8 @@ fn test_fails_initiating_concurrent_splices() { let added_value = Amount::from_sat(initial_channel_value_sat); let acceptor_template = nodes[1].node.splice_channel(&channel_id, &node_0_id, feerate).unwrap(); let acceptor_wallet = WalletSync::new(Arc::clone(&nodes[1].wallet_source), nodes[1].logger); - let change_script = Some(nodes[1].wallet_source.get_change_script().unwrap()); let acceptor_contribution = - acceptor_template.splice_in_sync(change_script, added_value, &acceptor_wallet).unwrap(); + acceptor_template.splice_in_sync(added_value, &acceptor_wallet).unwrap(); nodes[1] .node .funding_contributed(&channel_id, &node_0_id, acceptor_contribution, None) @@ -1189,14 +1171,9 @@ fn do_test_splice_commitment_broadcast(splice_status: SpliceStatus, claim_htlcs: let (preimage1, payment_hash1, ..) = route_payment(&nodes[0], &[&nodes[1]], payment_amount); let splice_in_amount = initial_channel_capacity / 2; - let initiator_contribution = do_initiate_splice_in( - &nodes[0], - &nodes[1], - channel_id, - Amount::from_sat(splice_in_amount), - Some(nodes[0].wallet_source.get_change_script().unwrap()), - ); - let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, initiator_contribution); + let initiator_contribution = + do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, Amount::from_sat(splice_in_amount)); + let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, initiator_contribution); let (preimage2, payment_hash2, ..) = route_payment(&nodes[0], &[&nodes[1]], payment_amount); let htlc_expiry = nodes[0].best_block_info().1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS; @@ -2211,7 +2188,7 @@ fn do_test_splice_with_inflight_htlc_forward_and_resolution(expire_scid_pre_forw script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), }]; let contribution = initiate_splice_out(&nodes[0], &nodes[1], channel_id_0_1, outputs_0_1); - let splice_tx_0_1 = splice_channel(&nodes[0], &nodes[1], channel_id_0_1, contribution); + let (splice_tx_0_1, _) = splice_channel(&nodes[0], &nodes[1], channel_id_0_1, contribution); for node in &nodes { mine_transaction(node, &splice_tx_0_1); } @@ -2221,7 +2198,7 @@ fn do_test_splice_with_inflight_htlc_forward_and_resolution(expire_scid_pre_forw script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), }]; let contribution = initiate_splice_out(&nodes[1], &nodes[2], channel_id_1_2, outputs_1_2); - let splice_tx_1_2 = splice_channel(&nodes[1], &nodes[2], channel_id_1_2, contribution); + let (splice_tx_1_2, _) = splice_channel(&nodes[1], &nodes[2], channel_id_1_2, contribution); for node in &nodes { mine_transaction(node, &splice_tx_1_2); } From 8330af03a10180ea96a64416858962c40a461a6d Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 2 Feb 2026 09:52:57 -0600 Subject: [PATCH 6/7] Consistently log in propose_quiescence Instead of logging both inside propose_quiescence and at the call site, only log inside it. This simplifies the return type. --- lightning/src/ln/channel.rs | 14 +++++++++----- lightning/src/ln/channelmanager.rs | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index db85a26ae91..0f1916ac59f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12214,8 +12214,7 @@ where } self.propose_quiescence(logger, QuiescentAction::Splice { contribution, locktime }).map_err( - |(e, action)| { - log_error!(logger, "{}", e); + |action| { // FIXME: Any better way to do this? if let QuiescentAction::Splice { contribution, .. } = action { let (contributed_inputs, contributed_outputs) = @@ -13355,14 +13354,19 @@ where #[rustfmt::skip] pub fn propose_quiescence( &mut self, logger: &L, action: QuiescentAction, - ) -> Result, (&'static str, QuiescentAction)> { + ) -> Result, QuiescentAction> { log_debug!(logger, "Attempting to initiate quiescence"); if !self.context.is_usable() { - return Err(("Channel is not in a usable state to propose quiescence", action)); + log_debug!(logger, "Channel is not in a usable state to propose quiescence"); + return Err(action); } if self.quiescent_action.is_some() { - return Err(("Channel already has a pending quiescent action and cannot start another", action)); + log_debug!( + logger, + "Channel already has a pending quiescent action and cannot start another", + ); + return Err(action); } self.quiescent_action = Some(action); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f70f4b133d0..75de6ab5d10 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -13401,7 +13401,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }); notify = NotifyOption::SkipPersistHandleEvents; }, - Err((msg, _action)) => log_trace!(logger, "{}", msg), + Err(action) => log_trace!(logger, "Failed to propose quiescence for: {:?}", action), } } else { result = Err(APIError::APIMisuseError { From 05067c55052a5ba2d71ca366a8785508591e8be9 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 2 Feb 2026 14:02:04 -0600 Subject: [PATCH 7/7] Run rustfmt on fuzz --- fuzz/src/lsps_message.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/fuzz/src/lsps_message.rs b/fuzz/src/lsps_message.rs index 547a27b70ee..8371d1c5fc7 100644 --- a/fuzz/src/lsps_message.rs +++ b/fuzz/src/lsps_message.rs @@ -77,17 +77,20 @@ pub fn do_test(data: &[u8]) { genesis_block.header.time, )); - let liquidity_manager = Arc::new(LiquidityManagerSync::new( - Arc::clone(&keys_manager), - Arc::clone(&keys_manager), - Arc::clone(&manager), - None::>, - None, - kv_store, - Arc::clone(&tx_broadcaster), - None, - None, - ).unwrap()); + let liquidity_manager = Arc::new( + LiquidityManagerSync::new( + Arc::clone(&keys_manager), + Arc::clone(&keys_manager), + Arc::clone(&manager), + None::>, + None, + kv_store, + Arc::clone(&tx_broadcaster), + None, + None, + ) + .unwrap(), + ); let mut reader = data; if let Ok(Some(msg)) = liquidity_manager.read(LSPS_MESSAGE_TYPE_ID, &mut reader) { let secp = Secp256k1::signing_only();