From: 志宇 Date: Wed, 28 Aug 2024 09:49:55 +0000 (+0000) Subject: feat(wallet)!: allow custom fallback algorithm for bnb X-Git-Tag: v1.0.0-beta.3~3^2~1 X-Git-Url: http://internal-gitweb-vhost/script/%22https:/database/scripts/enum.InsertDescriptorError.html?a=commitdiff_plain;h=c18204d05925c4398e139c6e62c9640dacb67e17;p=bdk feat(wallet)!: allow custom fallback algorithm for bnb Signature of `CoinSelectionAlgorithm::coin_select` has been changed to take in a `&mut RangCore`. This allows us to pass the random number generator directly to the cs algorithm. Single random draw is now it's own type `SingleRandomDraw` and impls `CoinSelectionAlgorithm`. `BranchAndBoundCoinSelection` now handles it's own fallback algorithm internally, and a generic type parameter is added to specify the fallback algorithm. `coin_selection::Error` is renamed to `InsufficientFunds` and the BnB error variants are removed. The BnB error variants are no longer needed since those cases are handled internally by `BranchAndBoundCoinSelection` (via calling the fallback algorithm). Add test_bnb_fallback_algorithm test and docs cleanup suggested by @ValuedMammal. --- diff --git a/crates/wallet/src/wallet/coin_selection.rs b/crates/wallet/src/wallet/coin_selection.rs index 8dc36b19..30612401 100644 --- a/crates/wallet/src/wallet/coin_selection.rs +++ b/crates/wallet/src/wallet/coin_selection.rs @@ -31,18 +31,20 @@ //! # use bdk_wallet::*; //! # use bdk_wallet::coin_selection::decide_change; //! # use anyhow::Error; +//! # use rand_core::RngCore; //! #[derive(Debug)] //! struct AlwaysSpendEverything; //! //! impl CoinSelectionAlgorithm for AlwaysSpendEverything { -//! fn coin_select( +//! fn coin_select( //! &self, //! required_utxos: Vec, //! optional_utxos: Vec, //! fee_rate: FeeRate, //! target_amount: u64, //! drain_script: &Script, -//! ) -> Result { +//! rand: &mut R, +//! ) -> Result { //! let mut selected_amount = 0; //! let mut additional_weight = Weight::ZERO; //! let all_utxos_selected = required_utxos @@ -63,7 +65,7 @@ //! let additional_fees = (fee_rate * additional_weight).to_sat(); //! let amount_needed_with_fees = additional_fees + target_amount; //! if selected_amount < amount_needed_with_fees { -//! return Err(coin_selection::Error::InsufficientFunds { +//! return Err(coin_selection::InsufficientFunds { //! needed: amount_needed_with_fees, //! available: selected_amount, //! }); @@ -118,44 +120,31 @@ use rand_core::RngCore; use super::utils::shuffle_slice; /// Default coin selection algorithm used by [`TxBuilder`](super::tx_builder::TxBuilder) if not /// overridden -pub type DefaultCoinSelectionAlgorithm = BranchAndBoundCoinSelection; +pub type DefaultCoinSelectionAlgorithm = BranchAndBoundCoinSelection; -/// Errors that can be thrown by the [`coin_selection`](crate::wallet::coin_selection) module -#[derive(Debug)] -pub enum Error { - /// Wallet's UTXO set is not enough to cover recipient's requested plus fee - InsufficientFunds { - /// Sats needed for some transaction - needed: u64, - /// Sats available for spending - available: u64, - }, - /// Branch and bound coin selection tries to avoid needing a change by finding the right inputs for - /// the desired outputs plus fee, if there is not such combination this error is thrown - BnBNoExactMatch, - /// Branch and bound coin selection possible attempts with sufficiently big UTXO set could grow - /// exponentially, thus a limit is set, and when hit, this error is thrown - BnBTotalTriesExceeded, +/// Wallet's UTXO set is not enough to cover recipient's requested plus fee. +/// +/// This is thrown by [`CoinSelectionAlgorithm`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct InsufficientFunds { + /// Sats needed for some transaction + pub needed: u64, + /// Sats available for spending + pub available: u64, } -impl fmt::Display for Error { +impl fmt::Display for InsufficientFunds { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - match self { - Self::InsufficientFunds { needed, available } => write!( - f, - "Insufficient funds: {} sat available of {} sat needed", - available, needed - ), - Self::BnBTotalTriesExceeded => { - write!(f, "Branch and bound coin selection: total tries exceeded") - } - Self::BnBNoExactMatch => write!(f, "Branch and bound coin selection: not exact match"), - } + write!( + f, + "Insufficient funds: {} sat available of {} sat needed", + self.available, self.needed + ) } } #[cfg(feature = "std")] -impl std::error::Error for Error {} +impl std::error::Error for InsufficientFunds {} #[derive(Debug)] /// Remaining amount after performing coin selection @@ -216,8 +205,6 @@ impl CoinSelectionResult { pub trait CoinSelectionAlgorithm: core::fmt::Debug { /// Perform the coin selection /// - /// - `database`: a reference to the wallet's database that can be used to lookup additional - /// details for a specific UTXO /// - `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 @@ -226,15 +213,17 @@ pub trait CoinSelectionAlgorithm: core::fmt::Debug { /// - `target_amount`: the outgoing amount in satoshis and the fees already /// accumulated from added 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`] #[allow(clippy::too_many_arguments)] - fn coin_select( + fn coin_select( &self, required_utxos: Vec, optional_utxos: Vec, fee_rate: FeeRate, target_amount: u64, drain_script: &Script, - ) -> Result; + rand: &mut R, + ) -> Result; } /// Simple and dumb coin selection @@ -245,14 +234,15 @@ pub trait CoinSelectionAlgorithm: core::fmt::Debug { pub struct LargestFirstCoinSelection; impl CoinSelectionAlgorithm for LargestFirstCoinSelection { - fn coin_select( + fn coin_select( &self, required_utxos: Vec, mut optional_utxos: Vec, fee_rate: FeeRate, target_amount: u64, drain_script: &Script, - ) -> Result { + _: &mut R, + ) -> Result { // 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 = { @@ -275,14 +265,15 @@ impl CoinSelectionAlgorithm for LargestFirstCoinSelection { pub struct OldestFirstCoinSelection; impl CoinSelectionAlgorithm for OldestFirstCoinSelection { - fn coin_select( + fn coin_select( &self, required_utxos: Vec, mut optional_utxos: Vec, fee_rate: FeeRate, target_amount: u64, drain_script: &Script, - ) -> Result { + _: &mut R, + ) -> Result { // 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 @@ -334,7 +325,7 @@ fn select_sorted_utxos( fee_rate: FeeRate, target_amount: u64, drain_script: &Script, -) -> Result { +) -> Result { let mut selected_amount = 0; let mut fee_amount = 0; let selected = utxos @@ -359,7 +350,7 @@ fn select_sorted_utxos( let amount_needed_with_fees = target_amount + fee_amount; if selected_amount < amount_needed_with_fees { - return Err(Error::InsufficientFunds { + return Err(InsufficientFunds { needed: amount_needed_with_fees, available: selected_amount, }); @@ -407,56 +398,73 @@ impl OutputGroup { /// /// Code adapted from Bitcoin Core's implementation and from Mark Erhardt Master's Thesis: #[derive(Debug, Clone)] -pub struct BranchAndBoundCoinSelection { +pub struct BranchAndBoundCoinSelection { size_of_change: u64, + fallback_algorithm: FA, +} + +/// Error returned by branch and bond coin selection. +#[derive(Debug)] +enum BnBError { + /// Branch and bound coin selection tries to avoid needing a change by finding the right inputs for + /// the desired outputs plus fee, if there is not such combination this error is thrown + NoExactMatch, + /// Branch and bound coin selection possible attempts with sufficiently big UTXO set could grow + /// exponentially, thus a limit is set, and when hit, this error is thrown + TotalTriesExceeded, } -impl Default for BranchAndBoundCoinSelection { +impl Default for BranchAndBoundCoinSelection { fn default() -> Self { Self { // P2WPKH cost of change -> value (8 bytes) + script len (1 bytes) + script (22 bytes) size_of_change: 8 + 1 + 22, + fallback_algorithm: FA::default(), } } } -impl BranchAndBoundCoinSelection { - /// Create new instance with target size for change output - pub fn new(size_of_change: u64) -> Self { - Self { size_of_change } +impl BranchAndBoundCoinSelection { + /// Create new instance with a target `size_of_change` and `fallback_algorithm`. + pub fn new(size_of_change: u64, fallback_algorithm: FA) -> Self { + Self { + size_of_change, + fallback_algorithm, + } } } const BNB_TOTAL_TRIES: usize = 100_000; -impl CoinSelectionAlgorithm for BranchAndBoundCoinSelection { - fn coin_select( +impl CoinSelectionAlgorithm for BranchAndBoundCoinSelection { + fn coin_select( &self, required_utxos: Vec, optional_utxos: Vec, fee_rate: FeeRate, target_amount: u64, drain_script: &Script, - ) -> Result { + rand: &mut R, + ) -> Result { // Mapping every (UTXO, usize) to an output group - let required_utxos: Vec = required_utxos - .into_iter() - .map(|u| OutputGroup::new(u, fee_rate)) + let required_ogs: Vec = required_utxos + .iter() + .map(|u| OutputGroup::new(u.clone(), fee_rate)) .collect(); // Mapping every (UTXO, usize) to an output group, filtering UTXOs with a negative // effective value - let optional_utxos: Vec = optional_utxos - .into_iter() - .map(|u| OutputGroup::new(u, fee_rate)) + let optional_ogs: Vec = optional_utxos + .iter() + .map(|u| OutputGroup::new(u.clone(), fee_rate)) .filter(|u| u.effective_value.is_positive()) .collect(); - let curr_value = required_utxos + let curr_value = required_ogs .iter() .fold(0, |acc, x| acc + x.effective_value); - let curr_available_value = optional_utxos + let curr_available_value = optional_ogs .iter() .fold(0, |acc, x| acc + x.effective_value); @@ -480,57 +488,63 @@ impl CoinSelectionAlgorithm for BranchAndBoundCoinSelection { _ => { // Assume we spend all the UTXOs we can (all the required + all the optional with // positive effective value), sum their value and their fee cost. - let (utxo_fees, utxo_value) = required_utxos - .iter() - .chain(optional_utxos.iter()) - .fold((0, 0), |(mut fees, mut value), utxo| { + let (utxo_fees, utxo_value) = required_ogs.iter().chain(optional_ogs.iter()).fold( + (0, 0), + |(mut fees, mut value), utxo| { fees += utxo.fee; value += utxo.weighted_utxo.utxo.txout().value.to_sat(); (fees, value) - }); + }, + ); // Add to the target the fee cost of the UTXOs - return Err(Error::InsufficientFunds { + return Err(InsufficientFunds { needed: target_amount + utxo_fees, available: utxo_value, }); } } - let target_amount = target_amount + let signed_target_amount = target_amount .try_into() .expect("Bitcoin amount to fit into i64"); - if curr_value > target_amount { + if curr_value > signed_target_amount { // remaining_amount can't be negative as that would mean the // selection wasn't successful // target_amount = amount_needed + (fee_amount - vin_fees) - let remaining_amount = (curr_value - target_amount) as u64; + let remaining_amount = (curr_value - signed_target_amount) as u64; let excess = decide_change(remaining_amount, fee_rate, drain_script); - return Ok(BranchAndBoundCoinSelection::calculate_cs_result( - vec![], - required_utxos, - excess, - )); + return Ok(calculate_cs_result(vec![], required_ogs, excess)); } - self.bnb( - required_utxos.clone(), - optional_utxos.clone(), + match self.bnb( + required_ogs.clone(), + optional_ogs.clone(), curr_value, curr_available_value, - target_amount, + signed_target_amount, cost_of_change, drain_script, fee_rate, - ) + ) { + Ok(r) => Ok(r), + Err(_) => self.fallback_algorithm.coin_select( + required_utxos, + optional_utxos, + fee_rate, + target_amount, + drain_script, + rand, + ), + } } } -impl BranchAndBoundCoinSelection { +impl BranchAndBoundCoinSelection { // TODO: make this more Rust-onic :) // (And perhaps refactor with less arguments?) #[allow(clippy::too_many_arguments)] @@ -544,7 +558,7 @@ impl BranchAndBoundCoinSelection { cost_of_change: u64, drain_script: &Script, fee_rate: FeeRate, - ) -> Result { + ) -> Result { // current_selection[i] will contain true if we are using optional_utxos[i], // false otherwise. Note that current_selection.len() could be less than // optional_utxos.len(), it just means that we still haven't decided if we should keep @@ -600,7 +614,7 @@ impl BranchAndBoundCoinSelection { // We have walked back to the first utxo and no branch is untraversed. All solutions searched // If best selection is empty, then there's no exact match if best_selection.is_empty() { - return Err(Error::BnBNoExactMatch); + return Err(BnBError::NoExactMatch); } break; } @@ -627,7 +641,7 @@ impl BranchAndBoundCoinSelection { // Check for solution if best_selection.is_empty() { - return Err(Error::BnBTotalTriesExceeded); + return Err(BnBError::TotalTriesExceeded); } // Set output set @@ -646,30 +660,32 @@ impl BranchAndBoundCoinSelection { let excess = decide_change(remaining_amount, fee_rate, drain_script); - Ok(BranchAndBoundCoinSelection::calculate_cs_result( - selected_utxos, - required_utxos, - excess, - )) + Ok(calculate_cs_result(selected_utxos, required_utxos, excess)) } +} - fn calculate_cs_result( - 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 selected = selected_utxos - .into_iter() - .map(|u| u.weighted_utxo.utxo) - .collect::>(); +/// Pull UTXOs at random until we have enough to meet the target. +#[derive(Debug, Clone, Copy, Default)] +pub struct SingleRandomDraw; - CoinSelectionResult { - selected, - fee_amount, - excess, - } +impl CoinSelectionAlgorithm for SingleRandomDraw { + fn coin_select( + &self, + required_utxos: Vec, + optional_utxos: Vec, + fee_rate: FeeRate, + target_amount: u64, + drain_script: &Script, + rand: &mut R, + ) -> Result { + Ok(single_random_draw( + required_utxos, + optional_utxos, + target_amount, + drain_script, + fee_rate, + rand, + )) } } @@ -722,7 +738,26 @@ pub(crate) fn single_random_draw( let excess = decide_change(remaining_amount, fee_rate, drain_script); - BranchAndBoundCoinSelection::calculate_cs_result(selected_utxos.1, required_utxos, excess) + calculate_cs_result(selected_utxos.1, required_utxos, excess) +} + +fn calculate_cs_result( + 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 selected = selected_utxos + .into_iter() + .map(|u| u.weighted_utxo.utxo) + .collect::>(); + + CoinSelectionResult { + selected, + fee_amount, + excess, + } } /// Remove duplicate UTXOs. @@ -758,7 +793,7 @@ mod test { use crate::wallet::coin_selection::filter_duplicates; use rand::prelude::SliceRandom; - use rand::{Rng, RngCore, SeedableRng}; + use rand::{thread_rng, Rng, RngCore, SeedableRng}; // signature len (1WU) + signature and sighash (72WU) // + pubkey len (1WU) + pubkey (33WU) @@ -907,6 +942,7 @@ mod test { FeeRate::from_sat_per_vb_unchecked(1), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); @@ -928,6 +964,7 @@ mod test { FeeRate::from_sat_per_vb_unchecked(1), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); @@ -949,6 +986,7 @@ mod test { FeeRate::from_sat_per_vb_unchecked(1), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); @@ -971,6 +1009,7 @@ mod test { FeeRate::from_sat_per_vb_unchecked(1), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); } @@ -989,6 +1028,7 @@ mod test { FeeRate::from_sat_per_vb_unchecked(1000), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); } @@ -1006,6 +1046,7 @@ mod test { FeeRate::from_sat_per_vb_unchecked(1), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); @@ -1027,6 +1068,7 @@ mod test { FeeRate::from_sat_per_vb_unchecked(1), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); @@ -1048,6 +1090,7 @@ mod test { FeeRate::from_sat_per_vb_unchecked(1), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); @@ -1070,6 +1113,7 @@ mod test { FeeRate::from_sat_per_vb_unchecked(1), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); } @@ -1093,6 +1137,7 @@ mod test { FeeRate::from_sat_per_vb_unchecked(1000), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); } @@ -1106,13 +1151,14 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 250_000 + FEE_AMOUNT; - let result = BranchAndBoundCoinSelection::default() + let result = BranchAndBoundCoinSelection::::default() .coin_select( vec![], utxos, FeeRate::from_sat_per_vb_unchecked(1), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); @@ -1127,13 +1173,14 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 20_000 + FEE_AMOUNT; - let result = BranchAndBoundCoinSelection::default() + let result = BranchAndBoundCoinSelection::::default() .coin_select( utxos.clone(), utxos, FeeRate::from_sat_per_vb_unchecked(1), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); @@ -1149,13 +1196,14 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 299756 + FEE_AMOUNT; - let result = BranchAndBoundCoinSelection::default() + let result = BranchAndBoundCoinSelection::::default() .coin_select( vec![], utxos, FeeRate::from_sat_per_vb_unchecked(1), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); @@ -1206,13 +1254,14 @@ mod test { let target_amount = 150_000 + FEE_AMOUNT; - let result = BranchAndBoundCoinSelection::default() + let result = BranchAndBoundCoinSelection::::default() .coin_select( required, optional, FeeRate::from_sat_per_vb_unchecked(1), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); @@ -1228,13 +1277,14 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 500_000 + FEE_AMOUNT; - BranchAndBoundCoinSelection::default() + BranchAndBoundCoinSelection::::default() .coin_select( vec![], utxos, FeeRate::from_sat_per_vb_unchecked(1), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); } @@ -1246,13 +1296,14 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 250_000 + FEE_AMOUNT; - BranchAndBoundCoinSelection::default() + BranchAndBoundCoinSelection::::default() .coin_select( vec![], utxos, FeeRate::from_sat_per_vb_unchecked(1000), target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); } @@ -1264,8 +1315,15 @@ mod test { let target_amount = 99932; // first utxo's effective value let feerate = FeeRate::BROADCAST_MIN; - let result = BranchAndBoundCoinSelection::new(0) - .coin_select(vec![], utxos, feerate, target_amount, &drain_script) + let result = BranchAndBoundCoinSelection::new(0, SingleRandomDraw) + .coin_select( + vec![], + utxos, + feerate, + target_amount, + &drain_script, + &mut thread_rng(), + ) .unwrap(); assert_eq!(result.selected.len(), 1); @@ -1286,13 +1344,14 @@ mod test { let mut optional_utxos = generate_random_utxos(&mut rng, 16); let target_amount = sum_random_utxos(&mut rng, &mut optional_utxos); let drain_script = ScriptBuf::default(); - let result = BranchAndBoundCoinSelection::new(0) + let result = BranchAndBoundCoinSelection::new(0, SingleRandomDraw) .coin_select( vec![], optional_utxos, FeeRate::ZERO, target_amount, &drain_script, + &mut thread_rng(), ) .unwrap(); assert_eq!(result.selected_amount(), target_amount); @@ -1300,7 +1359,7 @@ mod test { } #[test] - #[should_panic(expected = "BnBNoExactMatch")] + #[should_panic(expected = "NoExactMatch")] fn test_bnb_function_no_exact_match() { let fee_rate = FeeRate::from_sat_per_vb_unchecked(10); let utxos: Vec = get_test_utxos() @@ -1315,7 +1374,7 @@ mod test { let drain_script = ScriptBuf::default(); let target_amount = 20_000 + FEE_AMOUNT; - BranchAndBoundCoinSelection::new(size_of_change) + BranchAndBoundCoinSelection::new(size_of_change, SingleRandomDraw) .bnb( vec![], utxos, @@ -1330,7 +1389,7 @@ mod test { } #[test] - #[should_panic(expected = "BnBTotalTriesExceeded")] + #[should_panic(expected = "TotalTriesExceeded")] fn test_bnb_function_tries_exceeded() { let fee_rate = FeeRate::from_sat_per_vb_unchecked(10); let utxos: Vec = generate_same_value_utxos(100_000, 100_000) @@ -1346,7 +1405,7 @@ mod test { let drain_script = ScriptBuf::default(); - BranchAndBoundCoinSelection::new(size_of_change) + BranchAndBoundCoinSelection::new(size_of_change, SingleRandomDraw) .bnb( vec![], utxos, @@ -1382,7 +1441,7 @@ mod test { let drain_script = ScriptBuf::default(); - let result = BranchAndBoundCoinSelection::new(size_of_change) + let result = BranchAndBoundCoinSelection::new(size_of_change, SingleRandomDraw) .bnb( vec![], utxos, @@ -1422,7 +1481,7 @@ mod test { let drain_script = ScriptBuf::default(); - let result = BranchAndBoundCoinSelection::new(0) + let result = BranchAndBoundCoinSelection::new(0, SingleRandomDraw) .bnb( vec![], optional_utxos, @@ -1443,17 +1502,18 @@ mod test { let utxos = get_test_utxos(); let drain_script = ScriptBuf::default(); - let selection = BranchAndBoundCoinSelection::default().coin_select( + let selection = BranchAndBoundCoinSelection::::default().coin_select( vec![], utxos, FeeRate::from_sat_per_vb_unchecked(10), 500_000, &drain_script, + &mut thread_rng(), ); assert_matches!( selection, - Err(Error::InsufficientFunds { + Err(InsufficientFunds { available: 300_000, .. }) @@ -1469,17 +1529,18 @@ mod test { |u| matches!(u, WeightedUtxo { utxo, .. } if utxo.txout().value.to_sat() < 1000), ); - let selection = BranchAndBoundCoinSelection::default().coin_select( + let selection = BranchAndBoundCoinSelection::::default().coin_select( required, optional, FeeRate::from_sat_per_vb_unchecked(10), 500_000, &drain_script, + &mut thread_rng(), ); assert_matches!( selection, - Err(Error::InsufficientFunds { + Err(InsufficientFunds { available: 300_010, .. }) @@ -1491,23 +1552,46 @@ mod test { let utxos = get_test_utxos(); let drain_script = ScriptBuf::default(); - let selection = BranchAndBoundCoinSelection::default().coin_select( + let selection = BranchAndBoundCoinSelection::::default().coin_select( utxos, vec![], FeeRate::from_sat_per_vb_unchecked(10_000), 500_000, &drain_script, + &mut thread_rng(), ); assert_matches!( selection, - Err(Error::InsufficientFunds { + Err(InsufficientFunds { available: 300_010, .. }) ); } + #[test] + fn test_bnb_fallback_algorithm() { + // utxo value + // 120k + 80k + 300k + let optional_utxos = get_oldest_first_test_utxos(); + let feerate = FeeRate::BROADCAST_MIN; + let target_amount = 190_000; + let drain_script = ScriptBuf::new(); + // bnb won't find exact match and should select oldest first + let res = BranchAndBoundCoinSelection::::default() + .coin_select( + vec![], + optional_utxos, + feerate, + target_amount, + &drain_script, + &mut thread_rng(), + ) + .unwrap(); + assert_eq!(res.selected_amount(), 200_000); + } + #[test] fn test_filter_duplicates() { fn utxo(txid: &str, value: u64) -> WeightedUtxo { diff --git a/crates/wallet/src/wallet/error.rs b/crates/wallet/src/wallet/error.rs index b6c9375a..2264aac9 100644 --- a/crates/wallet/src/wallet/error.rs +++ b/crates/wallet/src/wallet/error.rs @@ -89,7 +89,7 @@ pub enum CreateTxError { /// Output created is under the dust limit, 546 satoshis OutputBelowDustLimit(usize), /// There was an error with coin selection - CoinSelection(coin_selection::Error), + CoinSelection(coin_selection::InsufficientFunds), /// Cannot build a tx without recipients NoRecipients, /// Partially signed bitcoin transaction error @@ -204,8 +204,8 @@ impl From for CreateTxError { } } -impl From for CreateTxError { - fn from(err: coin_selection::Error) -> Self { +impl From for CreateTxError { + fn from(err: coin_selection::InsufficientFunds) -> Self { CreateTxError::CoinSelection(err) } } diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index a70a70b9..5c908ea2 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -75,7 +75,7 @@ pub mod error; pub use utils::IsDust; -use coin_selection::DefaultCoinSelectionAlgorithm; +use coin_selection::{DefaultCoinSelectionAlgorithm, InsufficientFunds}; use signer::{SignOptions, SignerOrdering, SignersContainer, TransactionSigner}; use tx_builder::{FeePolicy, TxBuilder, TxParams}; use utils::{check_nsequence_rbf, After, Older, SecpCtx}; @@ -91,8 +91,6 @@ use crate::types::*; use crate::wallet::coin_selection::Excess::{self, Change, NoChange}; use crate::wallet::error::{BuildFeeBumpError, CreateTxError, MiniscriptPsbtError}; -use self::coin_selection::Error; - const COINBASE_MATURITY: u32 = 100; /// A Bitcoin wallet @@ -1497,31 +1495,16 @@ impl Wallet { let (required_utxos, optional_utxos) = coin_selection::filter_duplicates(required_utxos, optional_utxos); - let coin_selection = match coin_selection.coin_select( - required_utxos.clone(), - optional_utxos.clone(), - fee_rate, - outgoing.to_sat() + fee_amount.to_sat(), - &drain_script, - ) { - Ok(res) => res, - Err(e) => match e { - coin_selection::Error::InsufficientFunds { .. } => { - return Err(CreateTxError::CoinSelection(e)); - } - coin_selection::Error::BnBNoExactMatch - | coin_selection::Error::BnBTotalTriesExceeded => { - coin_selection::single_random_draw( - required_utxos, - optional_utxos, - outgoing.to_sat() + fee_amount.to_sat(), - &drain_script, - fee_rate, - rng, - ) - } - }, - }; + let coin_selection = coin_selection + .coin_select( + required_utxos.clone(), + optional_utxos.clone(), + fee_rate, + outgoing.to_sat() + fee_amount.to_sat(), + &drain_script, + rng, + ) + .map_err(CreateTxError::CoinSelection)?; fee_amount += Amount::from_sat(coin_selection.fee_amount); let excess = &coin_selection.excess; @@ -1551,7 +1534,7 @@ impl Wallet { change_fee, } = excess { - return Err(CreateTxError::CoinSelection(Error::InsufficientFunds { + return Err(CreateTxError::CoinSelection(InsufficientFunds { needed: *dust_threshold, available: remaining_amount.saturating_sub(*change_fee), })); @@ -2657,7 +2640,7 @@ macro_rules! doctest_wallet { script_pubkey: address.script_pubkey(), }], }; - let txid = tx.txid(); + let txid = tx.compute_txid(); let block_id = BlockId { height: 500, hash: BlockHash::all_zeros() }; let _ = wallet.insert_checkpoint(block_id); let _ = wallet.insert_checkpoint(BlockId { height: 1_000, hash: BlockHash::all_zeros() }); diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index 2a2a68fa..63759378 100644 --- a/crates/wallet/tests/wallet.rs +++ b/crates/wallet/tests/wallet.rs @@ -730,7 +730,7 @@ fn test_create_tx_change_policy() { assert!(matches!( builder.finish(), Err(CreateTxError::CoinSelection( - coin_selection::Error::InsufficientFunds { .. } + coin_selection::InsufficientFunds { .. } )), )); } @@ -3943,7 +3943,7 @@ fn test_spend_coinbase() { assert!(matches!( builder.finish(), Err(CreateTxError::CoinSelection( - coin_selection::Error::InsufficientFunds { + coin_selection::InsufficientFunds { needed: _, available: 0 } @@ -3958,7 +3958,7 @@ fn test_spend_coinbase() { assert_matches!( builder.finish(), Err(CreateTxError::CoinSelection( - coin_selection::Error::InsufficientFunds { + coin_selection::InsufficientFunds { needed: _, available: 0 }