From 39bae0b96d7e3a4729ba4fdb4e5e89ded83b9395 Mon Sep 17 00:00:00 2001 From: Martin Saposnic Date: Tue, 25 Feb 2025 11:29:36 -0300 Subject: [PATCH 1/3] Introduce flag avoidpartialspends New flag called avoid_partial_spends is introduced It's a parameter used when building a tx and it's passed to coin_select. Later commits will introduce behavior for this flag --- crates/wallet/src/wallet/coin_selection.rs | 542 ++++++++++++--------- crates/wallet/src/wallet/mod.rs | 12 +- crates/wallet/src/wallet/tx_builder.rs | 14 + 3 files changed, 328 insertions(+), 240 deletions(-) diff --git a/crates/wallet/src/wallet/coin_selection.rs b/crates/wallet/src/wallet/coin_selection.rs index 4429fae73..cc716c6b3 100644 --- a/crates/wallet/src/wallet/coin_selection.rs +++ b/crates/wallet/src/wallet/coin_selection.rs @@ -38,13 +38,17 @@ //! impl CoinSelectionAlgorithm for AlwaysSpendEverything { //! fn coin_select( //! &self, -//! required_utxos: Vec, -//! optional_utxos: Vec, -//! fee_rate: FeeRate, -//! target_amount: Amount, -//! drain_script: &Script, -//! rand: &mut R, +//! params: CoinSelectionParams<'_, R>, //! ) -> Result { +//! let CoinSelectionParams { +//! required_utxos, +//! optional_utxos, +//! fee_rate, +//! target_amount, +//! drain_script, +//! rand: _, +//! avoid_partial_spends, +//! } = params; //! let mut selected_amount = Amount::ZERO; //! let mut additional_weight = Weight::ZERO; //! let all_utxos_selected = required_utxos @@ -196,6 +200,25 @@ impl CoinSelectionResult { } } +/// Params for coin selection +#[derive(Debug)] +pub struct CoinSelectionParams<'a, R: RngCore> { + /// - `required_utxos`: the utxos that must be spent regardless of `target_amount` with their weight cost + pub required_utxos: Vec, + /// - `optional_utxos`: the remaining available utxos to satisfy `target_amount` with their weight cost + pub optional_utxos: Vec, + /// - `fee_rate`: fee rate to use + pub fee_rate: FeeRate, + /// - `target_amount`: the outgoing amount and the fees already accumulated from adding outputs and transaction’s header. + pub target_amount: Amount, + /// - `drain_script`: the script to use in case of change + pub drain_script: &'a Script, + /// - `rand`: random number generated used by some coin selection algorithms such as [`SingleRandomDraw`] + pub rand: &'a mut R, + /// - `avoid_partial_spends`: if true, the algorithm should try to avoid partial spends + pub avoid_partial_spends: bool, +} + /// Trait for generalized coin selection algorithms /// /// This trait can be implemented to make the [`Wallet`](super::Wallet) use a customized coin @@ -204,24 +227,9 @@ impl CoinSelectionResult { /// For an example see [this module](crate::wallet::coin_selection)'s documentation. pub trait CoinSelectionAlgorithm: core::fmt::Debug { /// Perform the coin selection - /// - /// - `required_utxos`: the utxos that must be spent regardless of `target_amount` with their - /// weight cost - /// - `optional_utxos`: the remaining available utxos to satisfy `target_amount` with their - /// weight cost - /// - `fee_rate`: fee rate to use - /// - `target_amount`: the outgoing amount and the fees already accumulated from adding - /// outputs and transaction’s header. - /// - `drain_script`: the script to use in case of change - /// - `rand`: random number generated used by some coin selection algorithms such as [`SingleRandomDraw`] fn coin_select( &self, - required_utxos: Vec, - optional_utxos: Vec, - fee_rate: FeeRate, - target_amount: Amount, - drain_script: &Script, - rand: &mut R, + params: CoinSelectionParams<'_, R>, ) -> Result; } @@ -235,13 +243,17 @@ pub struct LargestFirstCoinSelection; impl CoinSelectionAlgorithm for LargestFirstCoinSelection { fn coin_select( &self, - required_utxos: Vec, - mut optional_utxos: Vec, - fee_rate: FeeRate, - target_amount: Amount, - drain_script: &Script, - _: &mut R, + params: CoinSelectionParams<'_, R>, ) -> Result { + let CoinSelectionParams { + required_utxos, + mut optional_utxos, + fee_rate, + target_amount, + drain_script, + rand: _, + avoid_partial_spends: _, + } = params; // We put the "required UTXOs" first and make sure the optional UTXOs are sorted, // initially smallest to largest, before being reversed with `.rev()`. let utxos = { @@ -266,13 +278,17 @@ pub struct OldestFirstCoinSelection; impl CoinSelectionAlgorithm for OldestFirstCoinSelection { fn coin_select( &self, - required_utxos: Vec, - mut optional_utxos: Vec, - fee_rate: FeeRate, - target_amount: Amount, - drain_script: &Script, - _: &mut R, + params: CoinSelectionParams<'_, R>, ) -> Result { + let CoinSelectionParams { + required_utxos, + mut optional_utxos, + fee_rate, + target_amount, + drain_script, + rand: _, + avoid_partial_spends: _, + } = params; // We put the "required UTXOs" first and make sure the optional UTXOs are sorted from // oldest to newest according to blocktime // For utxo that doesn't exist in DB, they will have lowest priority to be selected @@ -442,13 +458,17 @@ const BNB_TOTAL_TRIES: usize = 100_000; impl CoinSelectionAlgorithm for BranchAndBoundCoinSelection { fn coin_select( &self, - required_utxos: Vec, - optional_utxos: Vec, - fee_rate: FeeRate, - target_amount: Amount, - drain_script: &Script, - rand: &mut R, + params: CoinSelectionParams<'_, R>, ) -> Result { + let CoinSelectionParams { + required_utxos, + optional_utxos, + fee_rate, + target_amount, + drain_script, + rand: _, + avoid_partial_spends, + } = params; // Mapping every (UTXO, usize) to an output group let required_ogs: Vec = required_utxos .iter() @@ -538,14 +558,18 @@ impl CoinSelectionAlgorithm for BranchAndBoundCoinSe fee_rate, ) { Ok(r) => Ok(r), - Err(_) => self.fallback_algorithm.coin_select( - required_utxos, - optional_utxos, - fee_rate, - target_amount, - drain_script, - rand, - ), + Err(_) => { + let params = CoinSelectionParams { + required_utxos, + optional_utxos, + fee_rate, + target_amount, + drain_script, + rand: params.rand, + avoid_partial_spends, + }; + self.fallback_algorithm.coin_select(params) + } } } } @@ -679,13 +703,17 @@ pub struct SingleRandomDraw; impl CoinSelectionAlgorithm for SingleRandomDraw { fn coin_select( &self, - required_utxos: Vec, - mut optional_utxos: Vec, - fee_rate: FeeRate, - target_amount: Amount, - drain_script: &Script, - rand: &mut R, + params: CoinSelectionParams<'_, R>, ) -> Result { + let CoinSelectionParams { + required_utxos, + mut optional_utxos, + fee_rate, + target_amount, + drain_script, + rand, + avoid_partial_spends: _, + } = params; // We put the required UTXOs first and then the randomize optional UTXOs to take as needed let utxos = { shuffle_slice(&mut optional_utxos, rand); @@ -762,6 +790,8 @@ mod test { const FEE_AMOUNT: Amount = Amount::from_sat(50); + const DO_NOT_AVOID_PARTIAL_SPENDS: bool = false; + fn unconfirmed_utxo(value: Amount, index: u32, last_seen: u64) -> WeightedUtxo { utxo( value, @@ -922,16 +952,16 @@ mod test { let utxos = get_test_utxos(); let drain_script = ScriptBuf::default(); let target_amount = Amount::from_sat(250_000) + FEE_AMOUNT; - let result = LargestFirstCoinSelection - .coin_select( - utxos, - vec![], - FeeRate::from_sat_per_vb_unchecked(1), + .coin_select(CoinSelectionParams { + required_utxos: utxos, + optional_utxos: vec![], + fee_rate: FeeRate::from_sat_per_vb_unchecked(1), target_amount, - &drain_script, - &mut thread_rng(), - ) + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) .unwrap(); assert_eq!(result.selected.len(), 3); @@ -946,14 +976,15 @@ mod test { let target_amount = Amount::from_sat(20_000) + FEE_AMOUNT; let result = LargestFirstCoinSelection - .coin_select( - utxos, - vec![], - FeeRate::from_sat_per_vb_unchecked(1), + .coin_select(CoinSelectionParams { + required_utxos: utxos, + optional_utxos: vec![], + fee_rate: FeeRate::from_sat_per_vb_unchecked(1), target_amount, - &drain_script, - &mut thread_rng(), - ) + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) .unwrap(); assert_eq!(result.selected.len(), 3); @@ -968,14 +999,15 @@ mod test { let target_amount = Amount::from_sat(20_000) + FEE_AMOUNT; let result = LargestFirstCoinSelection - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb_unchecked(1), + .coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, + fee_rate: FeeRate::from_sat_per_vb_unchecked(1), target_amount, - &drain_script, - &mut thread_rng(), - ) + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) .unwrap(); assert_eq!(result.selected.len(), 1); @@ -989,14 +1021,15 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = Amount::from_sat(500_000) + FEE_AMOUNT; - let result = LargestFirstCoinSelection.coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb_unchecked(1), + let result = LargestFirstCoinSelection.coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, + fee_rate: FeeRate::from_sat_per_vb_unchecked(1), target_amount, - &drain_script, - &mut thread_rng(), - ); + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }); assert!(matches!(result, Err(InsufficientFunds { .. }))); } @@ -1006,14 +1039,15 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = Amount::from_sat(250_000) + FEE_AMOUNT; - let result = LargestFirstCoinSelection.coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb_unchecked(1000), + let result = LargestFirstCoinSelection.coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, + fee_rate: FeeRate::from_sat_per_vb_unchecked(1000), target_amount, - &drain_script, - &mut thread_rng(), - ); + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }); assert!(matches!(result, Err(InsufficientFunds { .. }))); } @@ -1024,14 +1058,15 @@ mod test { let target_amount = Amount::from_sat(180_000) + FEE_AMOUNT; let result = OldestFirstCoinSelection - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb_unchecked(1), + .coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, + fee_rate: FeeRate::from_sat_per_vb_unchecked(1), target_amount, - &drain_script, - &mut thread_rng(), - ) + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) .unwrap(); assert_eq!(result.selected.len(), 2); @@ -1046,14 +1081,15 @@ mod test { let target_amount = Amount::from_sat(20_000) + FEE_AMOUNT; let result = OldestFirstCoinSelection - .coin_select( - utxos, - vec![], - FeeRate::from_sat_per_vb_unchecked(1), + .coin_select(CoinSelectionParams { + required_utxos: utxos, + optional_utxos: vec![], + fee_rate: FeeRate::from_sat_per_vb_unchecked(1), target_amount, - &drain_script, - &mut thread_rng(), - ) + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) .unwrap(); assert_eq!(result.selected.len(), 3); @@ -1068,14 +1104,15 @@ mod test { let target_amount = Amount::from_sat(20_000) + FEE_AMOUNT; let result = OldestFirstCoinSelection - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb_unchecked(1), + .coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, + fee_rate: FeeRate::from_sat_per_vb_unchecked(1), target_amount, - &drain_script, - &mut thread_rng(), - ) + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) .unwrap(); assert_eq!(result.selected.len(), 1); @@ -1089,14 +1126,15 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = Amount::from_sat(600_000) + FEE_AMOUNT; - let result = OldestFirstCoinSelection.coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb_unchecked(1), + let result = OldestFirstCoinSelection.coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, + fee_rate: FeeRate::from_sat_per_vb_unchecked(1), target_amount, - &drain_script, - &mut thread_rng(), - ); + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }); assert!(matches!(result, Err(InsufficientFunds { .. }))); } @@ -1108,14 +1146,15 @@ mod test { utxos.iter().map(|wu| wu.utxo.txout().value).sum::() - Amount::from_sat(50); let drain_script = ScriptBuf::default(); - let result = OldestFirstCoinSelection.coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb_unchecked(1000), + let result = OldestFirstCoinSelection.coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, + fee_rate: FeeRate::from_sat_per_vb_unchecked(1000), target_amount, - &drain_script, - &mut thread_rng(), - ); + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }); assert!(matches!(result, Err(InsufficientFunds { .. }))); } @@ -1128,14 +1167,15 @@ mod test { let target_amount = Amount::from_sat(250_000) + FEE_AMOUNT; let result = BranchAndBoundCoinSelection::::default() - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb_unchecked(1), + .coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, + fee_rate: FeeRate::from_sat_per_vb_unchecked(1), target_amount, - &drain_script, - &mut thread_rng(), - ) + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) .unwrap(); assert_eq!(result.selected.len(), 3); @@ -1150,14 +1190,15 @@ mod test { let target_amount = Amount::from_sat(20_000) + FEE_AMOUNT; let result = BranchAndBoundCoinSelection::::default() - .coin_select( - utxos.clone(), - utxos, - FeeRate::from_sat_per_vb_unchecked(1), + .coin_select(CoinSelectionParams { + required_utxos: utxos.clone(), + optional_utxos: utxos, + fee_rate: FeeRate::from_sat_per_vb_unchecked(1), target_amount, - &drain_script, - &mut thread_rng(), - ) + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) .unwrap(); assert_eq!(result.selected.len(), 3); @@ -1174,14 +1215,15 @@ mod test { let target_amount = calc_target_amount(&[utxos[0].clone(), utxos[2].clone()], fee_rate); let result = BranchAndBoundCoinSelection::::default() - .coin_select( - vec![], - utxos, + .coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, fee_rate, target_amount, - &drain_script, - &mut thread_rng(), - ) + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) .unwrap(); assert_eq!(result.selected.len(), 2); @@ -1198,14 +1240,15 @@ mod test { let fee_rate = FeeRate::from_sat_per_vb_unchecked(1); let drain_script = ScriptBuf::default(); - let result = SingleRandomDraw.coin_select( - vec![], - utxos, + let result = SingleRandomDraw.coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, fee_rate, target_amount, - &drain_script, - &mut thread_rng(), - ); + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }); assert!( matches!(result, Ok(CoinSelectionResult {selected, fee_amount, ..}) @@ -1226,14 +1269,15 @@ mod test { let fee_rate = FeeRate::from_sat_per_vb_unchecked(1); let drain_script = ScriptBuf::default(); - let result = SingleRandomDraw.coin_select( - vec![], - utxos, + let result = SingleRandomDraw.coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, fee_rate, target_amount, - &drain_script, - &mut rng, - ); + drain_script: &drain_script, + rand: &mut rng, + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }); assert!(matches!(result, Err(InsufficientFunds {needed, available}) if needed == Amount::from_sat(300_254) && available == Amount::from_sat(300_010))); @@ -1269,14 +1313,15 @@ mod test { let target_amount = calc_target_amount(&[utxos[0].clone(), utxos[2].clone()], fee_rate); let result = BranchAndBoundCoinSelection::::default() - .coin_select( - required, - optional, + .coin_select(CoinSelectionParams { + required_utxos: required, + optional_utxos: optional, fee_rate, target_amount, - &drain_script, - &mut thread_rng(), - ) + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) .unwrap(); assert_eq!(result.selected.len(), 2); @@ -1291,12 +1336,15 @@ mod test { let target_amount = Amount::from_sat(500_000) + FEE_AMOUNT; let result = BranchAndBoundCoinSelection::::default().coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb_unchecked(1), - target_amount, - &drain_script, - &mut thread_rng(), + CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, + fee_rate: FeeRate::from_sat_per_vb_unchecked(1), + target_amount, + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }, ); assert!(matches!(result, Err(InsufficientFunds { .. }))); @@ -1309,12 +1357,15 @@ mod test { let target_amount = Amount::from_sat(250_000) + FEE_AMOUNT; let result = BranchAndBoundCoinSelection::::default().coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb_unchecked(1000), - target_amount, - &drain_script, - &mut thread_rng(), + CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, + fee_rate: FeeRate::from_sat_per_vb_unchecked(1000), + target_amount, + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }, ); assert!(matches!(result, Err(InsufficientFunds { .. }))); } @@ -1328,14 +1379,15 @@ mod test { let target_amount = calc_target_amount(&utxos[0..1], fee_rate); let result = BranchAndBoundCoinSelection::::default() - .coin_select( - vec![], - utxos, + .coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, fee_rate, target_amount, - &drain_script, - &mut thread_rng(), - ) + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) .unwrap(); assert_eq!(result.selected.len(), 1); @@ -1357,14 +1409,15 @@ mod test { let target_amount = sum_random_utxos(&mut rng, &mut optional_utxos); let drain_script = ScriptBuf::default(); let result = BranchAndBoundCoinSelection::::default() - .coin_select( - vec![], - optional_utxos, - FeeRate::ZERO, + .coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: optional_utxos, + fee_rate: FeeRate::ZERO, target_amount, - &drain_script, - &mut thread_rng(), - ) + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) .unwrap(); assert_eq!(result.selected_amount(), target_amount); } @@ -1527,12 +1580,15 @@ mod test { let drain_script = ScriptBuf::default(); let selection = BranchAndBoundCoinSelection::::default().coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb_unchecked(10), - Amount::from_sat(500_000), - &drain_script, - &mut thread_rng(), + CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, + fee_rate: FeeRate::from_sat_per_vb_unchecked(10), + target_amount: Amount::from_sat(500_000), + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }, ); assert_matches!( @@ -1554,12 +1610,15 @@ mod test { ); let selection = BranchAndBoundCoinSelection::::default().coin_select( - required, - optional, - FeeRate::from_sat_per_vb_unchecked(10), - Amount::from_sat(500_000), - &drain_script, - &mut thread_rng(), + CoinSelectionParams { + required_utxos: required, + optional_utxos: optional, + fee_rate: FeeRate::from_sat_per_vb_unchecked(10), + target_amount: Amount::from_sat(500_000), + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }, ); assert_matches!( @@ -1577,12 +1636,15 @@ mod test { let drain_script = ScriptBuf::default(); let selection = BranchAndBoundCoinSelection::::default().coin_select( - utxos, - vec![], - FeeRate::from_sat_per_vb_unchecked(10_000), - Amount::from_sat(500_000), - &drain_script, - &mut thread_rng(), + CoinSelectionParams { + required_utxos: utxos, + optional_utxos: vec![], + fee_rate: FeeRate::from_sat_per_vb_unchecked(10_000), + target_amount: Amount::from_sat(500_000), + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }, ); assert_matches!( @@ -1606,14 +1668,15 @@ mod test { let bnb_with_oldest_first = BranchAndBoundCoinSelection::new(8 + 1 + 22, OldestFirstCoinSelection); let res = bnb_with_oldest_first - .coin_select( - vec![], - optional_utxos, - feerate, - target_amount, - &drain_script, - &mut thread_rng(), - ) + .coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: optional_utxos, + fee_rate: feerate, + target_amount: target_amount, + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) .unwrap(); assert_eq!(res.selected_amount(), Amount::from_sat(200_000)); } @@ -1760,30 +1823,39 @@ mod test { let result = match tc.coin_selection_algo { CoinSelectionAlgo::BranchAndBound => { BranchAndBoundCoinSelection::::default().coin_select( - vec![], - optional, + CoinSelectionParams { + required_utxos: vec![], + optional_utxos: optional, + fee_rate, + target_amount, + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }, + ) + } + CoinSelectionAlgo::OldestFirst => { + OldestFirstCoinSelection.coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: optional, fee_rate, target_amount, - &drain_script, - &mut thread_rng(), - ) + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) + } + CoinSelectionAlgo::LargestFirst => { + LargestFirstCoinSelection.coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: optional, + fee_rate, + target_amount, + drain_script: &drain_script, + rand: &mut thread_rng(), + avoid_partial_spends: DO_NOT_AVOID_PARTIAL_SPENDS, + }) } - CoinSelectionAlgo::OldestFirst => OldestFirstCoinSelection.coin_select( - vec![], - optional, - fee_rate, - target_amount, - &drain_script, - &mut thread_rng(), - ), - CoinSelectionAlgo::LargestFirst => LargestFirstCoinSelection.coin_select( - vec![], - optional, - fee_rate, - target_amount, - &drain_script, - &mut thread_rng(), - ), }; assert!(result.is_ok(), "coin_select failed {}", tc.name); diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 6e00b1094..83d581928 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -19,6 +19,7 @@ use alloc::{ sync::Arc, vec::Vec, }; +use coin_selection::CoinSelectionParams; use core::{cmp::Ordering, fmt, mem, ops::Deref}; use bdk_chain::{ @@ -1451,14 +1452,15 @@ impl Wallet { coin_selection::filter_duplicates(required_utxos, optional_utxos); let coin_selection = coin_selection - .coin_select( + .coin_select(CoinSelectionParams { required_utxos, optional_utxos, fee_rate, - outgoing + fee_amount, - &drain_script, - rng, - ) + target_amount: outgoing + fee_amount, + drain_script: &drain_script, + rand: rng, + avoid_partial_spends: params.avoid_partial_spends, + }) .map_err(CreateTxError::CoinSelection)?; let excess = &coin_selection.excess; diff --git a/crates/wallet/src/wallet/tx_builder.rs b/crates/wallet/src/wallet/tx_builder.rs index da41c6b00..d6ae18a9c 100644 --- a/crates/wallet/src/wallet/tx_builder.rs +++ b/crates/wallet/src/wallet/tx_builder.rs @@ -140,6 +140,7 @@ pub(crate) struct TxParams { pub(crate) bumping_fee: Option, pub(crate) current_height: Option, pub(crate) allow_dust: bool, + pub(crate) avoid_partial_spends: bool, } #[derive(Clone, Copy, Debug)] @@ -575,6 +576,19 @@ impl<'a, Cs> TxBuilder<'a, Cs> { self } + /// Avoid partial spends. + /// + /// Group outputs by address, selecting many (possibly all) or none, + /// instead of selecting on a per-output basis. Privacy is improved as + /// addresses are mostly swept with fewer transactions and outputs are + /// aggregated in clean change addresses. It may result in higher fees + /// due to less optimal coin selection caused by this added limitation + /// and possibly a larger-than-necessary number of inputs being used + pub fn avoid_partial_spends(&mut self) -> &mut Self { + self.params.avoid_partial_spends = true; + self + } + /// Replace the recipients already added with a new list pub fn set_recipients(&mut self, recipients: Vec<(ScriptBuf, Amount)>) -> &mut Self { self.params.recipients = recipients; From 115439f79a019f9a539bc0fdc08ddb4de16d3a09 Mon Sep 17 00:00:00 2001 From: Martin Saposnic Date: Tue, 25 Feb 2025 11:32:25 -0300 Subject: [PATCH 2/3] Introduce changes to support avoid_partial_spends functionality This commit introduces many changes but they don't _really_ change anything. Tests will still pass as usual. The main change is how UTXOs are treated on coin_selection. Before, each UTXO was treated individually, but now each UTXO is its own **group**. For now, group_utxos_if_applies just creates groups with a single UTXO inside of it, but future commits will add logic to this function so it groups by pub_key --- crates/wallet/src/wallet/coin_selection.rs | 165 ++++++++++++++------- 1 file changed, 114 insertions(+), 51 deletions(-) diff --git a/crates/wallet/src/wallet/coin_selection.rs b/crates/wallet/src/wallet/coin_selection.rs index cc716c6b3..acd46d844 100644 --- a/crates/wallet/src/wallet/coin_selection.rs +++ b/crates/wallet/src/wallet/coin_selection.rs @@ -232,6 +232,10 @@ pub trait CoinSelectionAlgorithm: core::fmt::Debug { params: CoinSelectionParams<'_, R>, ) -> Result; } +fn group_utxos_if_applies(utxos: Vec, _: bool) -> Vec> { + // No grouping: every UTXO is its own group. + return utxos.into_iter().map(|u| vec![u]).collect(); +} /// Simple and dumb coin selection /// @@ -247,21 +251,31 @@ impl CoinSelectionAlgorithm for LargestFirstCoinSelection { ) -> Result { let CoinSelectionParams { required_utxos, - mut optional_utxos, + optional_utxos, fee_rate, target_amount, drain_script, rand: _, - avoid_partial_spends: _, + avoid_partial_spends, } = params; + let required_utxo_group = + group_utxos_if_applies(required_utxos.clone(), avoid_partial_spends); + let mut optional_utxos_group = group_utxos_if_applies(optional_utxos, avoid_partial_spends); // We put the "required UTXOs" first and make sure the optional UTXOs are sorted, // initially smallest to largest, before being reversed with `.rev()`. let utxos = { - optional_utxos.sort_unstable_by_key(|wu| wu.utxo.txout().value); - required_utxos + optional_utxos_group.sort_unstable_by_key(|group| { + group.iter().map(|wu| wu.utxo.txout().value).sum::() + }); + required_utxo_group .into_iter() .map(|utxo| (true, utxo)) - .chain(optional_utxos.into_iter().rev().map(|utxo| (false, utxo))) + .chain( + optional_utxos_group + .into_iter() + .rev() + .map(|utxo| (false, utxo)), + ) }; select_sorted_utxos(utxos, fee_rate, target_amount, drain_script) @@ -282,26 +296,30 @@ impl CoinSelectionAlgorithm for OldestFirstCoinSelection { ) -> Result { let CoinSelectionParams { required_utxos, - mut optional_utxos, + optional_utxos, fee_rate, target_amount, drain_script, rand: _, - avoid_partial_spends: _, + avoid_partial_spends, } = params; + let required_utxo_group = + group_utxos_if_applies(required_utxos.clone(), avoid_partial_spends); + let mut optional_utxos_group = + group_utxos_if_applies(optional_utxos.clone(), avoid_partial_spends); // We put the "required UTXOs" first and make sure the optional UTXOs are sorted from // oldest to newest according to blocktime // For utxo that doesn't exist in DB, they will have lowest priority to be selected let utxos = { - optional_utxos.sort_unstable_by_key(|wu| match &wu.utxo { - Utxo::Local(local) => Some(local.chain_position), + optional_utxos_group.sort_unstable_by_key(|group| match group[0].utxo { + Utxo::Local(ref local) => Some(local.chain_position), Utxo::Foreign { .. } => None, }); - required_utxos + required_utxo_group .into_iter() .map(|utxo| (true, utxo)) - .chain(optional_utxos.into_iter().map(|utxo| (false, utxo))) + .chain(optional_utxos_group.into_iter().map(|utxo| (false, utxo))) }; select_sorted_utxos(utxos, fee_rate, target_amount, drain_script) @@ -336,7 +354,7 @@ pub fn decide_change(remaining_amount: Amount, fee_rate: FeeRate, drain_script: } fn select_sorted_utxos( - utxos: impl Iterator, + utxos: impl Iterator)>, fee_rate: FeeRate, target_amount: Amount, drain_script: &Script, @@ -346,20 +364,23 @@ fn select_sorted_utxos( let selected = utxos .scan( (&mut selected_amount, &mut fee_amount), - |(selected_amount, fee_amount), (must_use, weighted_utxo)| { + |(selected_amount, fee_amount), (must_use, group)| { if must_use || **selected_amount < target_amount + **fee_amount { - **fee_amount += fee_rate - * TxIn::default() - .segwit_weight() - .checked_add(weighted_utxo.satisfaction_weight) - .expect("`Weight` addition should not cause an integer overflow"); - **selected_amount += weighted_utxo.utxo.txout().value; - Some(weighted_utxo.utxo) + for weighted_utxo in &group { + **fee_amount += fee_rate + * TxIn::default() + .segwit_weight() + .checked_add(weighted_utxo.satisfaction_weight) + .expect("`Weight` addition should not cause an integer overflow"); + **selected_amount += weighted_utxo.utxo.txout().value; + } + Some(group.into_iter().map(|wu| wu.utxo).collect::>()) } else { None } }, ) + .flatten() .collect::>(); let amount_needed_with_fees = target_amount + fee_amount; @@ -469,26 +490,42 @@ impl CoinSelectionAlgorithm for BranchAndBoundCoinSe rand: _, avoid_partial_spends, } = params; + let required_utxo_group = + group_utxos_if_applies(required_utxos.clone(), avoid_partial_spends); + let optional_utxos_group = + group_utxos_if_applies(optional_utxos.clone(), avoid_partial_spends); // Mapping every (UTXO, usize) to an output group - let required_ogs: Vec = required_utxos - .iter() - .map(|u| OutputGroup::new(u.clone(), fee_rate)) + let required_ogs: Vec> = required_utxo_group + .into_iter() + .map(|group| { + group + .into_iter() + .map(|weighted_utxo| OutputGroup::new(weighted_utxo, fee_rate)) + .collect() + }) .collect(); // Mapping every (UTXO, usize) to an output group, filtering UTXOs with a negative // effective value - let optional_ogs: Vec = optional_utxos - .iter() - .map(|u| OutputGroup::new(u.clone(), fee_rate)) - .filter(|u| u.effective_value.is_positive()) + let optional_ogs: Vec> = optional_utxos_group + .into_iter() + .map(|group| { + group + .into_iter() + .map(|weighted_utxo| OutputGroup::new(weighted_utxo, fee_rate)) + .filter(|og| og.effective_value.is_positive()) + .collect() + }) .collect(); let curr_value = required_ogs .iter() + .flat_map(|group| group.iter()) .fold(SignedAmount::ZERO, |acc, x| acc + x.effective_value); let curr_available_value = optional_ogs .iter() + .flat_map(|group| group.iter()) .fold(SignedAmount::ZERO, |acc, x| acc + x.effective_value); let cost_of_change = (Weight::from_vb(self.size_of_change).expect("overflow occurred") @@ -515,9 +552,11 @@ impl CoinSelectionAlgorithm for BranchAndBoundCoinSe // positive effective value), sum their value and their fee cost. let (utxo_fees, utxo_value) = required_ogs.iter().chain(optional_ogs.iter()).fold( (Amount::ZERO, Amount::ZERO), - |(mut fees, mut value), utxo| { - fees += utxo.fee; - value += utxo.weighted_utxo.utxo.txout().value; + |(mut fees, mut value), group| { + for utxo in group { + fees += utxo.fee; + value += utxo.weighted_utxo.utxo.txout().value; + } (fees, value) }, ); @@ -580,8 +619,8 @@ impl BranchAndBoundCoinSelection { #[allow(clippy::too_many_arguments)] fn bnb( &self, - required_utxos: Vec, - mut optional_utxos: Vec, + required_utxos: Vec>, + mut optional_utxos: Vec>, mut curr_value: SignedAmount, mut curr_available_value: SignedAmount, target_amount: SignedAmount, @@ -596,7 +635,12 @@ impl BranchAndBoundCoinSelection { let mut current_selection: Vec = Vec::with_capacity(optional_utxos.len()); // Sort the utxo_pool - optional_utxos.sort_unstable_by_key(|a| a.effective_value); + optional_utxos.sort_unstable_by_key(|group| { + group + .iter() + .map(|og| og.effective_value) + .sum::() + }); optional_utxos.reverse(); // Contains the best selection we found @@ -637,7 +681,10 @@ impl BranchAndBoundCoinSelection { // Walk backwards to find the last included UTXO that still needs to have its omission branch traversed. while let Some(false) = current_selection.last() { current_selection.pop(); - curr_available_value += optional_utxos[current_selection.len()].effective_value; + curr_available_value += optional_utxos[current_selection.len()] + .iter() + .map(|og| og.effective_value) + .sum::(); } if current_selection.last_mut().is_none() { @@ -655,17 +702,26 @@ impl BranchAndBoundCoinSelection { } let utxo = &optional_utxos[current_selection.len() - 1]; - curr_value -= utxo.effective_value; + curr_value -= utxo + .iter() + .map(|og| og.effective_value) + .sum::(); } else { // Moving forwards, continuing down this branch let utxo = &optional_utxos[current_selection.len()]; // Remove this utxo from the curr_available_value utxo amount - curr_available_value -= utxo.effective_value; + curr_available_value -= utxo + .iter() + .map(|og| og.effective_value) + .sum::(); // Inclusion branch first (Largest First Exploration) current_selection.push(true); - curr_value += utxo.effective_value; + curr_value += utxo + .iter() + .map(|og| og.effective_value) + .sum::(); } } @@ -679,7 +735,7 @@ impl BranchAndBoundCoinSelection { .into_iter() .zip(best_selection) .filter_map(|(optional, is_in_best)| if is_in_best { Some(optional) } else { None }) - .collect::>(); + .collect::>>(); let selected_amount = best_selection_value.unwrap(); @@ -707,21 +763,23 @@ impl CoinSelectionAlgorithm for SingleRandomDraw { ) -> Result { let CoinSelectionParams { required_utxos, - mut optional_utxos, + optional_utxos, fee_rate, target_amount, drain_script, rand, - avoid_partial_spends: _, + avoid_partial_spends, } = params; + let required_utxo_group = group_utxos_if_applies(required_utxos, avoid_partial_spends); + let mut optional_utxos_group = group_utxos_if_applies(optional_utxos, avoid_partial_spends); // We put the required UTXOs first and then the randomize optional UTXOs to take as needed let utxos = { - shuffle_slice(&mut optional_utxos, rand); + shuffle_slice(&mut optional_utxos_group, rand); - required_utxos + required_utxo_group .into_iter() .map(|utxo| (true, utxo)) - .chain(optional_utxos.into_iter().map(|utxo| (false, utxo))) + .chain(optional_utxos_group.into_iter().map(|utxo| (false, utxo))) }; // select required UTXOs and then random optional UTXOs. @@ -730,15 +788,20 @@ impl CoinSelectionAlgorithm for SingleRandomDraw { } fn calculate_cs_result( - mut selected_utxos: Vec, - mut required_utxos: Vec, + mut selected_utxos: Vec>, + mut required_utxos: Vec>, excess: Excess, ) -> CoinSelectionResult { selected_utxos.append(&mut required_utxos); - let fee_amount = selected_utxos.iter().map(|u| u.fee).sum(); + let fee_amount = selected_utxos + .iter() + .flat_map(|group| group.iter()) + .map(|u| u.fee) + .sum(); let selected = selected_utxos .into_iter() - .map(|u| u.weighted_utxo.utxo) + .flatten() + .map(|og| og.weighted_utxo.utxo) .collect::>(); CoinSelectionResult { @@ -1444,7 +1507,7 @@ mod test { let target_amount = SignedAmount::from_sat(20_000) + FEE_AMOUNT.to_signed().unwrap(); let result = BranchAndBoundCoinSelection::new(size_of_change, SingleRandomDraw).bnb( vec![], - utxos, + utxos.into_iter().map(|u| vec![u]).collect(), SignedAmount::ZERO, curr_available_value, target_amount, @@ -1477,7 +1540,7 @@ mod test { let result = BranchAndBoundCoinSelection::new(size_of_change, SingleRandomDraw).bnb( vec![], - utxos, + utxos.into_iter().map(|u| vec![u]).collect(), SignedAmount::ZERO, curr_available_value, target_amount, @@ -1518,7 +1581,7 @@ mod test { let result = BranchAndBoundCoinSelection::new(size_of_change, SingleRandomDraw) .bnb( vec![], - utxos, + utxos.into_iter().map(|u| vec![u]).collect(), curr_value, curr_available_value, target_amount, @@ -1558,7 +1621,7 @@ mod test { let result = BranchAndBoundCoinSelection::::default() .bnb( vec![], - optional_utxos, + optional_utxos.into_iter().map(|u| vec![u]).collect(), curr_value, curr_available_value, target_amount, From 5ac620bf775f72a856fb83315f0585dae4f94f7d Mon Sep 17 00:00:00 2001 From: Martin Saposnic Date: Tue, 25 Feb 2025 11:33:28 -0300 Subject: [PATCH 3/3] Make group_utxos_if_applies group by pubkey Now avoid_partial_spends will work as intended. If passed as true, it will group utxos by pubkey, and if selected, the grouped UTXOs will be used together. Tests are added that check that new functionality works --- crates/wallet/src/test_utils.rs | 53 +++ crates/wallet/src/wallet/coin_selection.rs | 500 ++++++++++++++++++++- crates/wallet/tests/wallet.rs | 38 ++ 3 files changed, 588 insertions(+), 3 deletions(-) diff --git a/crates/wallet/src/test_utils.rs b/crates/wallet/src/test_utils.rs index 7e1778fab..022d62406 100644 --- a/crates/wallet/src/test_utils.rs +++ b/crates/wallet/src/test_utils.rs @@ -119,6 +119,59 @@ fn new_funded_wallet(descriptor: &str, change_descriptor: Option<&str>) -> (Wall (wallet, tx1.compute_txid()) } +/// Return a fake wallet that has only reused outputs for testing. +/// +/// The wallet contains two transactions with one output each, both paying to the same common +/// script. The common script is derived from the first unused external address. +pub fn get_wallet_with_only_reused_outputs( + descriptor: &str, + change_descriptor: Option<&str>, +) -> (Wallet, (Txid, Txid)) { + // Create an empty wallet. + let params = if let Some(change_desc) = change_descriptor { + Wallet::create(descriptor.to_string(), change_desc.to_string()) + } else { + Wallet::create_single(descriptor.to_string()) + }; + + let mut wallet = params + .network(Network::Regtest) + .create_wallet_no_persist() + .expect("descriptors must be valid"); + + // Derive a common external address to be reused. + let common_addr = wallet.next_unused_address(KeychainKind::External).address; + let common_script = common_addr.script_pubkey(); + + // Fabricate transaction 1 with one output paying to the common script. + let tx1 = Transaction { + version: transaction::Version::ONE, + lock_time: absolute::LockTime::ZERO, + input: vec![], // No inputs since it's fabricated. + output: vec![TxOut { + value: Amount::from_sat(500_000), + script_pubkey: common_script.clone(), + }], + }; + + // Fabricate transaction 2 with one output paying to the common script. + let tx2 = Transaction { + version: transaction::Version::ONE, + lock_time: absolute::LockTime::ZERO, + input: vec![], + output: vec![TxOut { + value: Amount::from_sat(100_000), + script_pubkey: common_script, + }], + }; + + // Insert these fabricated transactions into the wallet. + insert_tx(&mut wallet, tx1.clone()); + insert_tx(&mut wallet, tx2.clone()); + + (wallet, (tx1.compute_txid(), tx2.compute_txid())) +} + /// Return a fake wallet that appears to be funded for testing. /// /// The funded wallet contains a tx with a 76_000 sats input and two outputs, one spending 25_000 diff --git a/crates/wallet/src/wallet/coin_selection.rs b/crates/wallet/src/wallet/coin_selection.rs index acd46d844..9299dc7e3 100644 --- a/crates/wallet/src/wallet/coin_selection.rs +++ b/crates/wallet/src/wallet/coin_selection.rs @@ -117,6 +117,7 @@ use bitcoin::OutPoint; use bitcoin::TxIn; use bitcoin::{Script, Weight}; +use chain::bdk_core::collections::HashMap; use core::convert::TryInto; use core::fmt::{self, Formatter}; use rand_core::RngCore; @@ -232,9 +233,43 @@ pub trait CoinSelectionAlgorithm: core::fmt::Debug { params: CoinSelectionParams<'_, R>, ) -> Result; } -fn group_utxos_if_applies(utxos: Vec, _: bool) -> Vec> { - // No grouping: every UTXO is its own group. - return utxos.into_iter().map(|u| vec![u]).collect(); + +// See https://github.com/bitcoin/bitcoin/pull/18418/files +// https://bitcoincore.reviews/17824.html#l-339 +const OUTPUT_GROUP_MAX_ENTRIES: usize = 100; + +/// Group weighted UTXOs based on their script_pubkey if partial spends should be avoided. +/// +/// If avoid_partial_spends is false each UTXO is kept in its own group. +/// If true, UTXOs sharing the same script_pubkey are grouped together, and if a group +/// would exceed OUTPUT_GROUP_MAX_ENTRIES the group is split into chunks. +fn group_utxos_if_applies( + utxos: Vec, + avoid_partial_spends: bool, +) -> Vec> { + if !avoid_partial_spends { + // No grouping: every UTXO is its own group. + return utxos.into_iter().map(|u| vec![u]).collect(); + } + + // Group UTXOs by their scriptPubKey bytes. + let mut groups_by_spk: HashMap, Vec> = HashMap::new(); + for weighted_utxo in utxos { + let spk = weighted_utxo.utxo.txout().script_pubkey.as_bytes().to_vec(); + groups_by_spk.entry(spk).or_default().push(weighted_utxo); + } + // For each group, split into multiple groups if needed. + let mut final_groups = Vec::new(); + for (_spk, group) in groups_by_spk { + if group.len() > OUTPUT_GROUP_MAX_ENTRIES { + for chunk in group.chunks(OUTPUT_GROUP_MAX_ENTRIES) { + final_groups.push(chunk.to_vec()); + } + } else { + final_groups.push(group); + } + } + final_groups } /// Simple and dumb coin selection @@ -991,6 +1026,87 @@ mod test { .collect() } + fn generate_utxos_with_same_address() -> Vec { + // Two distinct scripts to simulate two addresses: A and B. + let script_a = bitcoin::ScriptBuf::from(vec![b'A']); + let script_b = bitcoin::ScriptBuf::from(vec![b'B']); + + vec![ + // 1.0 btc to A + WeightedUtxo { + satisfaction_weight: Weight::from_wu_usize(P2WPKH_SATISFACTION_SIZE), + utxo: Utxo::Local(LocalOutput { + outpoint: OutPoint::from_str( + "ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:0", + ) + .unwrap(), + txout: TxOut { + value: Amount::from_sat(1_000_000_000), + script_pubkey: script_a.clone(), + }, + keychain: KeychainKind::External, + is_spent: false, + derivation_index: 42, + chain_position: ChainPosition::Unconfirmed { last_seen: Some(0) }, + }), + }, + // 0.5 btc to A + WeightedUtxo { + satisfaction_weight: Weight::from_wu_usize(P2WPKH_SATISFACTION_SIZE), + utxo: Utxo::Local(LocalOutput { + outpoint: OutPoint::from_str( + "ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:1", + ) + .unwrap(), + txout: TxOut { + value: Amount::from_sat(500_000_000), + script_pubkey: script_a, + }, + keychain: KeychainKind::External, + is_spent: false, + derivation_index: 42, + chain_position: ChainPosition::Unconfirmed { last_seen: Some(0) }, + }), + }, + // 1.0 btc to B + WeightedUtxo { + satisfaction_weight: Weight::from_wu_usize(P2WPKH_SATISFACTION_SIZE), + utxo: Utxo::Local(LocalOutput { + outpoint: OutPoint::from_str( + "ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:2", + ) + .unwrap(), + txout: TxOut { + value: Amount::from_sat(1_000_000_000), + script_pubkey: script_b.clone(), + }, + keychain: KeychainKind::External, + is_spent: false, + derivation_index: 42, + chain_position: ChainPosition::Unconfirmed { last_seen: Some(0) }, + }), + }, + // 0.5 btc to B + WeightedUtxo { + satisfaction_weight: Weight::from_wu_usize(P2WPKH_SATISFACTION_SIZE), + utxo: Utxo::Local(LocalOutput { + outpoint: OutPoint::from_str( + "ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:3", + ) + .unwrap(), + txout: TxOut { + value: Amount::from_sat(500_000_000), + script_pubkey: script_b, + }, + keychain: KeychainKind::External, + is_spent: false, + derivation_index: 42, + chain_position: ChainPosition::Unconfirmed { last_seen: Some(0) }, + }), + }, + ] + } + fn sum_random_utxos(mut rng: &mut StdRng, utxos: &mut [WeightedUtxo]) -> Amount { let utxos_picked_len = rng.gen_range(2..utxos.len() / 2); utxos.shuffle(&mut rng); @@ -1950,4 +2066,382 @@ mod test { assert_eq!(vouts, tc.exp_vouts, "wrong selected vouts for {}", tc.name); } } + + #[test] + fn test_group_utxos_if_applies_grouping() { + // generate 4 utxos: + // - Two for script A + // - Two for script B + let utxos = generate_utxos_with_same_address(); + + // Grouping should combine utxos with the same script when avoiding partial spends. + let groups = group_utxos_if_applies(utxos, true); + + // Since we have two distinct script_pubkeys we expect 2 groups. + assert_eq!( + groups.len(), + 2, + "Expected 2 groups for 2 distinct addresses" + ); + + // Each group must have exactly two UTXOs. + for group in groups { + assert_eq!(group.len(), 2, "Each group should contain exactly 2 UTXOs"); + // Check that all UTXOs in the group share the same script_pubkey. + let script = group[0].utxo.txout().script_pubkey.clone(); + for utxo in group.iter() { + assert_eq!(utxo.utxo.txout().script_pubkey, script); + } + } + } + + #[test] + fn test_group_utxos_if_applies_max_entries() { + // Create 101 UTXOs with the same script (address A) + let script_a = bitcoin::ScriptBuf::from(vec![b'A']); + let mut utxos = Vec::new(); + for i in 0..101 { + utxos.push(WeightedUtxo { + satisfaction_weight: Weight::from_wu_usize(P2WPKH_SATISFACTION_SIZE), + utxo: Utxo::Local(LocalOutput { + outpoint: OutPoint::from_str(&format!( + "ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:{}", + i + )) + .unwrap(), + txout: TxOut { + value: Amount::from_sat(1_000_000_000), + script_pubkey: script_a.clone(), + }, + keychain: KeychainKind::External, + is_spent: false, + derivation_index: 42, + chain_position: ChainPosition::Unconfirmed { last_seen: Some(0) }, + }), + }); + } + + // Group UTXOs with avoid_partial_spends enabled. + let groups = group_utxos_if_applies(utxos, true); + + // Since all UTXOs share the same script_pubkey and OUTPUT_GROUP_MAX_ENTRIES is 100, + // they must be split into 2 groups: one with 100 utxos and one with 1. + assert_eq!( + groups.len(), + 2, + "Expected 2 groups after splitting 101 UTXOs" + ); + let sizes: Vec = groups.iter().map(|g| g.len()).collect(); + assert!( + sizes.contains(&100), + "One group should contain exactly 100 UTXOs" + ); + assert!( + sizes.contains(&1), + "One group should contain exactly 1 UTXO" + ); + } + + #[test] + fn test_coin_selection_grouping_address_behavior() { + // Scenario: our node has four outputs: + // • 1.0 btc to A + // • 0.5 btc to A + // • 1.0 btc to B + // • 0.5 btc to B + // + // The node sends 0.2 btc to C. + // + // Without avoid_partial_spends: + // • The algorithm considers each UTXO separately. + // • In our LargestFirstCoinSelection (which orders optional groups descending by total value) + // the highest‐value individual coin is chosen. + // • Here that is the 1.0 btc output. + // + // With avoid_partial_spends: + // • UTXOs sharing the same address are grouped. + // • One group (either all A’s or all B’s) is used, so both UTXOs from that address are selected. + // + // To eliminate fee effects we use a zero fee rate. + let fee_rate = FeeRate::ZERO; + // Set target low enough so that a single UTXO would suffice. + let target = Amount::from_sat(200_000_000); + // A dummy drain script (change output script) + let drain_script = ScriptBuf::new(); + + // Generate the four test UTXOs. + let utxos = generate_utxos_with_same_address(); + + // --- Case 1: Without avoid_partial_spends (grouping disabled) + let mut rng = thread_rng(); + let res_no_group = LargestFirstCoinSelection + .coin_select(CoinSelectionParams { + required_utxos: vec![], // no required UTXOs + optional_utxos: utxos.clone(), // all UTXOs as optional + fee_rate, + target_amount: target, + drain_script: &drain_script, + rand: &mut rng, + avoid_partial_spends: false, // grouping disabled + }) + .expect("coin selection should succeed without grouping"); + // Without grouping, the algorithm picks one UTXO—the one with the highest value. + // In our ordering, the 1.0 btc output is chosen. + assert_eq!( + res_no_group.selected.len(), + 1, + "expected 1 UTXO selected when not grouping" + ); + let selected_no_group = res_no_group.selected_amount(); + // We expect the selected UTXO to have a value of 1.0 btc (1_000_000_000 sat). + assert_eq!( + selected_no_group, + Amount::from_sat(1_000_000_000), + "expected non-grouped selection to pick the 1.0 btc output" + ); + + // --- Case 2: With avoid_partial_spends enabled (grouping enabled) + let res_group = LargestFirstCoinSelection + .coin_select(CoinSelectionParams { + required_utxos: vec![], // no required UTXOs + optional_utxos: utxos, // all UTXOs as optional + fee_rate, + target_amount: target, + drain_script: &drain_script, + rand: &mut rng, + avoid_partial_spends: true, // grouping enabled + }) + .expect("coin selection should succeed with grouping"); + // With grouping enabled, each address is treated as a group. + // For either address A or B, the group consists of both outputs: + // 1.0 btc + 0.5 btc = 1.5 btc in total. + // Thus we expect exactly 2 UTXOs to be selected. + assert_eq!( + res_group.selected.len(), + 2, + "expected 2 UTXOs selected when grouping is enabled" + ); + let selected_group = res_group.selected_amount(); + // The grouped selection should have a higher total (1.5 btc) than the non-grouped one. + assert!( + selected_group > selected_no_group, + "expected grouped selection amount to be larger" + ); + // Also check that both UTXOs in the grouped selection share the same script. + let common_script = res_group.selected[0].txout().script_pubkey.clone(); + for utxo in res_group.selected.iter() { + assert_eq!( + utxo.txout().script_pubkey, + common_script, + "all UTXOs in a grouped selection must belong to the same address" + ); + } + } + + #[test] + fn test_coin_selection_grouping_address_behavior_oldestfirst() { + // Using OldestFirstCoinSelection. + let fee_rate = FeeRate::ZERO; + let target = Amount::from_sat(200_000_000); // low target so a single coin would suffice + let drain_script = ScriptBuf::new(); + let utxos = generate_utxos_with_same_address(); + + // Case 1: Grouping disabled. + let mut rng = thread_rng(); + let res_no_group = OldestFirstCoinSelection + .coin_select(CoinSelectionParams { + required_utxos: vec![], // no required UTXOs + optional_utxos: utxos.clone(), // all UTXOs as optional + fee_rate, + target_amount: target, + drain_script: &drain_script, + rand: &mut rng, + avoid_partial_spends: false, // grouping disabled + }) + .expect("coin selection should succeed without grouping (OldestFirst)"); + // Expect the highest-value individual coin is chosen (here 1.0 btc). + assert_eq!( + res_no_group.selected.len(), + 1, + "expected 1 UTXO selected when not grouping (OldestFirst)" + ); + assert_eq!( + res_no_group.selected_amount(), + Amount::from_sat(1_000_000_000), + "expected non-grouped selection to pick the 1.0 btc output (OldestFirst)" + ); + + // Case 2: Grouping enabled. + let res_group = OldestFirstCoinSelection + .coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, + fee_rate, + target_amount: target, + drain_script: &drain_script, + rand: &mut rng, + avoid_partial_spends: true, // grouping enabled + }) + .expect("coin selection should succeed with grouping (OldestFirst)"); + // With grouping enabled, one group (either A’s or B’s) is used: both outputs (1.0+0.5). + assert_eq!( + res_group.selected.len(), + 2, + "expected 2 UTXOs selected when grouping is enabled (OldestFirst)" + ); + assert_eq!( + res_group.selected_amount(), + Amount::from_sat(1_500_000_000), + "expected grouped selection to pick outputs totaling 1.5 btc (OldestFirst)" + ); + let common_script = res_group.selected[0].txout().script_pubkey.clone(); + for utxo in res_group.selected.iter() { + assert_eq!( + utxo.txout().script_pubkey, + common_script, + "all UTXOs in grouped selection must belong to the same address (OldestFirst)" + ); + } + } + + #[test] + fn test_coin_selection_grouping_address_behavior_branch_and_bound() { + // Using BranchAndBoundCoinSelection with SingleRandomDraw as fallback. + let fee_rate = FeeRate::ZERO; + let target = Amount::from_sat(200_000_000); + let drain_script = ScriptBuf::new(); + let utxos = generate_utxos_with_same_address(); + + let mut rng = thread_rng(); + let bnb_algo = BranchAndBoundCoinSelection::::default(); + + // --- Case 1: Without avoid_partial_spends (grouping disabled) + let res_no_group = bnb_algo + .coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos.clone(), + fee_rate, + target_amount: target, + drain_script: &drain_script, + rand: &mut rng, + avoid_partial_spends: false, // grouping disabled + }) + .expect("coin selection should succeed without grouping (BnB)"); + // Expect exactly one UTXO selected. However, due to the fallback randomness + // the chosen coin's value could be either 1.0 btc or 0.5 btc. + assert_eq!( + res_no_group.selected.len(), + 1, + "expected 1 UTXO selected when not grouping (BnB)" + ); + let non_group_val = res_no_group.selected_amount(); + assert!( + non_group_val == Amount::from_sat(1_000_000_000) || non_group_val == Amount::from_sat(500_000_000), + "expected non-grouped selection in BnB to be either 1.0 btc (1_000_000_000 sat) or 0.5 btc (500_000_000 sat), got {}", + non_group_val + ); + + // --- Case 2: With avoid_partial_spends enabled (grouping enabled) + let res_group = bnb_algo + .coin_select(CoinSelectionParams { + required_utxos: vec![], + optional_utxos: utxos, + fee_rate, + target_amount: target, + drain_script: &drain_script, + rand: &mut rng, + avoid_partial_spends: true, // grouping enabled + }) + .expect("coin selection should succeed with grouping (BnB)"); + // With grouping, each address is treated as a group. + // For either address A or B, the group consists of both outputs: + // 1.0 btc + 0.5 btc = 1.5 btc in total. + // Thus we expect exactly 2 UTXOs to be selected. + assert_eq!( + res_group.selected.len(), + 2, + "expected 2 UTXOs selected when grouping is enabled (BnB)" + ); + assert_eq!( + res_group.selected_amount(), + Amount::from_sat(1_500_000_000), + "expected grouped selection to pick outputs totaling 1.5 btc (BnB)" + ); + let common_script = res_group.selected[0].txout().script_pubkey.clone(); + for utxo in res_group.selected.iter() { + assert_eq!( + utxo.txout().script_pubkey, + common_script, + "all UTXOs in grouped selection must belong to the same address (BnB)" + ); + } + } + #[test] + fn test_coin_selection_grouping_address_behavior_single_random_draw() { + // Using SingleRandomDraw algorithm. + let fee_rate = FeeRate::ZERO; + let target = Amount::from_sat(200_000_000); + let drain_script = ScriptBuf::new(); + let utxos = generate_utxos_with_same_address(); + let mut rng = thread_rng(); + + // --- Case 1: Without avoid_partial_spends (grouping disabled) + let res_no_group = SingleRandomDraw + .coin_select(CoinSelectionParams { + required_utxos: vec![], // no required UTXOs + optional_utxos: utxos.clone(), // all UTXOs as optional + fee_rate, + target_amount: target, + drain_script: &drain_script, + rand: &mut rng, + avoid_partial_spends: false, // grouping disabled + }) + .expect("coin selection should succeed without grouping (RandomDraw)"); + // Expect that exactly one UTXO is picked. + assert_eq!( + res_no_group.selected.len(), + 1, + "expected 1 UTXO selected when not grouping (RandomDraw)" + ); + let sel_amt = res_no_group.selected_amount(); + // Since SingleRandomDraw selects randomly, it may pick either the 1.0 btc or + // the 0.5 btc output. We allow both. + assert!( + sel_amt == Amount::from_sat(1_000_000_000) || sel_amt == Amount::from_sat(500_000_000), + "expected non-grouped selection to pick either the 1.0 btc or 0.5 btc output, got {}", + sel_amt + ); + + // --- Case 2: With avoid_partial_spends enabled (grouping enabled) + let res_group = SingleRandomDraw + .coin_select(CoinSelectionParams { + required_utxos: vec![], // no required UTXOs + optional_utxos: utxos, // all UTXOs as optional + fee_rate, + target_amount: target, + drain_script: &drain_script, + rand: &mut rng, + avoid_partial_spends: true, // grouping enabled + }) + .expect("coin selection should succeed with grouping (RandomDraw)"); + // With grouping enabled, the algorithm should select both UTXOs from one address. + assert_eq!( + res_group.selected.len(), + 2, + "expected 2 UTXOs selected when grouping is enabled (RandomDraw)" + ); + assert_eq!( + res_group.selected_amount(), + Amount::from_sat(1_500_000_000), + "expected grouped selection to pick outputs totaling 1.5 btc (RandomDraw)" + ); + let common_script = res_group.selected[0].txout().script_pubkey.clone(); + for utxo in res_group.selected.iter() { + assert_eq!( + utxo.txout().script_pubkey, + common_script, + "all UTXOs in grouped selection must belong to the same address (RandomDraw)" + ); + } + } } diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index f42f0bcd5..ccfda97b9 100644 --- a/crates/wallet/tests/wallet.rs +++ b/crates/wallet/tests/wallet.rs @@ -4303,3 +4303,41 @@ fn test_wallet_transactions_relevant() { assert!(full_tx_count_before < full_tx_count_after); assert!(canonical_tx_count_before < canonical_tx_count_after); } + +#[test] +fn test_avoid_partial_spends_effect() { + // Get a funded wallet. + let (descriptor, _) = get_test_wpkh_and_change_desc(); + let (mut wallet, _) = get_wallet_with_only_reused_outputs(descriptor, None); + + // Pick a destination (different from the above script) and a small amount. + let dest = Address::from_str("bcrt1qc6fweuf4xjvz4x3gx3t9e0fh4hvqyu2qw4wvxm") + .expect("valid destination") + .assume_checked(); + let amt = Amount::from_sat(1_000); + + // --- Case 1: Without avoid_partial_spenqds + let mut builder1 = wallet.build_tx(); + builder1.add_recipient(dest.script_pubkey(), amt); + let psbt1 = builder1.finish().expect("tx build should succeed"); + + // Without grouping, coin selection should choose a single UTXO. + assert_eq!( + psbt1.unsigned_tx.input.len(), + 1, + "Expected 1 input without avoid_partial_spends" + ); + + // --- Case 2: With avoid_partial_spends enabled + let mut builder2 = wallet.build_tx(); + builder2.add_recipient(dest.script_pubkey(), amt); + builder2.avoid_partial_spends(); + let psbt2 = builder2.finish().expect("tx build should succeed"); + + // With grouping enabled, both UTXOs (from the same script) will be selected. + assert_eq!( + psbt2.unsigned_tx.input.len(), + 2, + "Expected 2 inputs with avoid_partial_spends" + ); +}