From 61b36dd0cfd2b221619a8bf16fabe59d84b058a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 4 Oct 2022 22:20:23 +0800 Subject: [PATCH 01/16] Workaround for `pkh` max satisfaction weight There is a bug in miniscript where they fail to take into consideration an `OP_PUSH..` (4 weight units) when calculating `max_satisfaction_weight` for `pkh` script types. We add this weight explicitly for `pkh` script types during coin selection. --- src/wallet/mod.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 2e3d9fdff..f204fca3e 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -30,7 +30,7 @@ use bitcoin::{ Txid, Witness, }; -use miniscript::descriptor::DescriptorTrait; +use miniscript::descriptor::{DescriptorTrait, DescriptorType}; use miniscript::psbt::PsbtInputSatisfier; use miniscript::ToPublicKey; @@ -1014,13 +1014,21 @@ where .borrow() .get_path_from_script_pubkey(&txout.script_pubkey)? { - Some((keychain, _)) => ( - self._get_descriptor_for_keychain(keychain) - .0 - .max_satisfaction_weight() - .unwrap(), - keychain, - ), + Some((keychain, _)) => { + let (desc, _) = self._get_descriptor_for_keychain(keychain); + + // WORKAROUND: There is a bug in miniscript where they fail to take into + // consideration an `OP_PUSH..` (4 weight units) for `pkh` script types. + let workaround_weight = match desc.desc_type() { + DescriptorType::Pkh => 4_usize, + _ => 0_usize, + }; + + ( + desc.max_satisfaction_weight().unwrap() + workaround_weight, + keychain, + ) + } None => { // estimate the weight based on the scriptsig/witness size present in the // original transaction From fd9c6dc47bb7193afc3f3feaf46bf4d55b3a33a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 4 Oct 2022 10:47:01 +0800 Subject: [PATCH 02/16] `FeeRate` calculation fixes Weight units should always be whole units. However, virtual bytes will need decimal places and the conversion from weight unit to vbytes should not be rounded. This commit has the following changes: * vbytes should always be represented in `f32` * conversion between vbytes and weight units should never be rounded * simple calculations (such as those for feerates) should be explicitly defined (instead of using multiple small functions) --- src/testutils/blockchain_tests.rs | 2 +- src/types.rs | 23 +++++++++++++---------- src/wallet/coin_selection.rs | 5 ++--- src/wallet/mod.rs | 2 +- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/testutils/blockchain_tests.rs b/src/testutils/blockchain_tests.rs index a3d7c2b17..a75d7647a 100644 --- a/src/testutils/blockchain_tests.rs +++ b/src/testutils/blockchain_tests.rs @@ -1021,7 +1021,7 @@ macro_rules! bdk_blockchain_tests { assert_eq!(details.received, 1_000 - details.fee.unwrap_or(0), "incorrect received after send"); let mut builder = wallet.build_fee_bump(details.txid).unwrap(); - builder.fee_rate(FeeRate::from_sat_per_vb(123.0)); + builder.fee_rate(FeeRate::from_sat_per_vb(124.0)); let (mut new_psbt, new_details) = builder.finish().unwrap(); println!("{:#?}", new_details); diff --git a/src/types.rs b/src/types.rs index bae86477f..1ce687356 100644 --- a/src/types.rs +++ b/src/types.rs @@ -98,13 +98,15 @@ impl FeeRate { /// Calculate fee rate from `fee` and weight units (`wu`). pub fn from_wu(fee: u64, wu: usize) -> FeeRate { - Self::from_vb(fee, wu.vbytes()) + let vbytes = wu as f32 / 4.0; + let rate = fee as f32 / vbytes; + Self::new_checked(rate) } /// Calculate fee rate from `fee` and `vbytes`. - pub fn from_vb(fee: u64, vbytes: usize) -> FeeRate { - let rate = fee as f32 / vbytes as f32; - Self::from_sat_per_vb(rate) + pub fn from_vb(fee: u64, vbytes: f32) -> FeeRate { + let rate = fee as f32 / vbytes; + Self::new_checked(rate) } /// Return the value as satoshi/vbyte @@ -114,12 +116,13 @@ impl FeeRate { /// Calculate absolute fee in Satoshis using size in weight units. pub fn fee_wu(&self, wu: usize) -> u64 { - self.fee_vb(wu.vbytes()) + let vbytes = wu as f32 / 4.0; + (self.0 * vbytes).ceil() as u64 } /// Calculate absolute fee in Satoshis using size in virtual bytes. - pub fn fee_vb(&self, vbytes: usize) -> u64 { - (self.as_sat_per_vb() * vbytes as f32).ceil() as u64 + pub fn fee_vb(&self, vbytes: f32) -> u64 { + (self.0 * vbytes).ceil() as u64 } } @@ -140,13 +143,13 @@ impl Sub for FeeRate { /// Trait implemented by types that can be used to measure weight units. pub trait Vbytes { /// Convert weight units to virtual bytes. - fn vbytes(self) -> usize; + fn vbytes(self) -> f32; } impl Vbytes for usize { - fn vbytes(self) -> usize { + fn vbytes(self) -> f32 { // ref: https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#transaction-size-calculations - (self as f32 / 4.0).ceil() as usize + self as f32 / 4.0 } } diff --git a/src/wallet/coin_selection.rs b/src/wallet/coin_selection.rs index 702ba1855..3ef22912f 100644 --- a/src/wallet/coin_selection.rs +++ b/src/wallet/coin_selection.rs @@ -304,9 +304,8 @@ impl CoinSelectionAlgorithm for OldestFirstCoinSelection { /// - `fee_rate`: required fee rate for the current selection /// - `drain_script`: script to consider change creation pub fn decide_change(remaining_amount: u64, fee_rate: FeeRate, drain_script: &Script) -> Excess { - // drain_output_len = size(len(script_pubkey)) + len(script_pubkey) + size(output_value) - let drain_output_len = serialize(drain_script).len() + 8usize; - let change_fee = fee_rate.fee_vb(drain_output_len); + let drain_output_len = serialize(drain_script).len() + 8_usize; + let change_fee = fee_rate.fee_vb(drain_output_len as f32); let drain_val = remaining_amount.saturating_sub(change_fee); if drain_val.is_dust(drain_script) { diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index f204fca3e..ed9623a6a 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -2657,7 +2657,7 @@ pub(crate) mod test { builder .drain_to(addr.script_pubkey()) .drain_wallet() - .fee_rate(FeeRate::from_sat_per_vb(453.0)); + .fee_rate(FeeRate::from_sat_per_vb(455.0)); builder.finish().unwrap(); } From 75d17b2948d2d6f7ac9d74dae17560a6d6d8b427 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 4 Oct 2022 12:24:56 +0800 Subject: [PATCH 03/16] Rename `Vbytes` to `WeightUnits` --- src/types.rs | 8 ++++---- src/wallet/coin_selection.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/types.rs b/src/types.rs index 1ce687356..42e2494cc 100644 --- a/src/types.rs +++ b/src/types.rs @@ -141,13 +141,13 @@ impl Sub for FeeRate { } /// Trait implemented by types that can be used to measure weight units. -pub trait Vbytes { +pub trait WeightUnits { /// Convert weight units to virtual bytes. - fn vbytes(self) -> f32; + fn to_vbytes(self) -> f32; } -impl Vbytes for usize { - fn vbytes(self) -> f32 { +impl WeightUnits for usize { + fn to_vbytes(self) -> f32 { // ref: https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#transaction-size-calculations self as f32 / 4.0 } diff --git a/src/wallet/coin_selection.rs b/src/wallet/coin_selection.rs index 3ef22912f..823dd447a 100644 --- a/src/wallet/coin_selection.rs +++ b/src/wallet/coin_selection.rs @@ -727,7 +727,7 @@ mod test { use super::*; use crate::database::{BatchOperations, MemoryDatabase}; use crate::types::*; - use crate::wallet::Vbytes; + use crate::wallet::WeightUnits; use rand::rngs::StdRng; use rand::seq::SliceRandom; @@ -1318,7 +1318,7 @@ mod test { assert_eq!(result.selected.len(), 1); assert_eq!(result.selected_amount(), 100_000); - let input_size = (TXIN_BASE_WEIGHT + P2WPKH_SATISFACTION_SIZE).vbytes(); + let input_size = (TXIN_BASE_WEIGHT + P2WPKH_SATISFACTION_SIZE).to_vbytes(); // the final fee rate should be exactly the same as the fee rate given assert!((1.0 - (result.fee_amount as f32 / input_size as f32)).abs() < f32::EPSILON); } From 4624ef811d67564b976471f632a117c686a74861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 29 Sep 2022 16:55:34 +0800 Subject: [PATCH 04/16] Remove generic `D: Database` from `CoinSelectionAlgorithm` Since `CoinSelectionAlgorithm` is a trait, if the database is really needed, one can create a struct that contains a database field which implements the `CoinSelectionAlgorithm` trait. Most `CoinSelectionAlgorithm` implementations do not require a database. --- src/wallet/coin_selection.rs | 87 +++++++++++++----------------------- src/wallet/mod.rs | 3 +- src/wallet/tx_builder.rs | 6 +-- 3 files changed, 34 insertions(+), 62 deletions(-) diff --git a/src/wallet/coin_selection.rs b/src/wallet/coin_selection.rs index 823dd447a..e711f15b2 100644 --- a/src/wallet/coin_selection.rs +++ b/src/wallet/coin_selection.rs @@ -34,10 +34,9 @@ //! #[derive(Debug)] //! struct AlwaysSpendEverything; //! -//! impl CoinSelectionAlgorithm for AlwaysSpendEverything { +//! impl CoinSelectionAlgorithm for AlwaysSpendEverything { //! fn coin_select( //! &self, -//! database: &D, //! required_utxos: Vec, //! optional_utxos: Vec, //! fee_rate: FeeRate, @@ -109,6 +108,7 @@ use rand::thread_rng; use rand::{rngs::StdRng, SeedableRng}; use std::collections::HashMap; use std::convert::TryInto; +use std::fmt::Debug; /// Default coin selection algorithm used by [`TxBuilder`](super::tx_builder::TxBuilder) if not /// overridden @@ -177,7 +177,7 @@ impl CoinSelectionResult { /// selection algorithm when it creates transactions. /// /// For an example see [this module](crate::wallet::coin_selection)'s documentation. -pub trait CoinSelectionAlgorithm: std::fmt::Debug { +pub trait CoinSelectionAlgorithm: std::fmt::Debug { /// Perform the coin selection /// /// - `database`: a reference to the wallet's database that can be used to lookup additional @@ -193,7 +193,6 @@ pub trait CoinSelectionAlgorithm: std::fmt::Debug { #[allow(clippy::too_many_arguments)] fn coin_select( &self, - database: &D, required_utxos: Vec, optional_utxos: Vec, fee_rate: FeeRate, @@ -209,10 +208,9 @@ pub trait CoinSelectionAlgorithm: std::fmt::Debug { #[derive(Debug, Default, Clone, Copy)] pub struct LargestFirstCoinSelection; -impl CoinSelectionAlgorithm for LargestFirstCoinSelection { +impl CoinSelectionAlgorithm for LargestFirstCoinSelection { fn coin_select( &self, - _database: &D, required_utxos: Vec, mut optional_utxos: Vec, fee_rate: FeeRate, @@ -243,13 +241,27 @@ impl CoinSelectionAlgorithm for LargestFirstCoinSelection { /// /// This coin selection algorithm sorts the available UTXOs by blockheight and then picks them starting /// from the oldest ones until the required amount is reached. -#[derive(Debug, Default, Clone, Copy)] -pub struct OldestFirstCoinSelection; +#[derive(Clone, Copy)] +pub struct OldestFirstCoinSelection<'d, D> { + database: &'d D, +} + +impl<'d, D: Database> OldestFirstCoinSelection<'d, D> { + /// Creates a new instance of [`OldestFirstCoinSelection`]. + pub fn new(database: &'d D) -> Self { + Self { database } + } +} + +impl<'d, D> Debug for OldestFirstCoinSelection<'d, D> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_tuple("OldestFirstCoinSelection").finish() + } +} -impl CoinSelectionAlgorithm for OldestFirstCoinSelection { +impl<'d, D: Database> CoinSelectionAlgorithm for OldestFirstCoinSelection<'d, D> { fn coin_select( &self, - database: &D, required_utxos: Vec, mut optional_utxos: Vec, fee_rate: FeeRate, @@ -266,7 +278,7 @@ impl CoinSelectionAlgorithm for OldestFirstCoinSelection { if bh_acc.contains_key(&txid) { Ok(bh_acc) } else { - database.get_tx(&txid, false).map(|details| { + self.database.get_tx(&txid, false).map(|details| { bh_acc.insert( txid, details.and_then(|d| d.confirmation_time.map(|ct| ct.height)), @@ -421,10 +433,9 @@ impl BranchAndBoundCoinSelection { const BNB_TOTAL_TRIES: usize = 100_000; -impl CoinSelectionAlgorithm for BranchAndBoundCoinSelection { +impl CoinSelectionAlgorithm for BranchAndBoundCoinSelection { fn coin_select( &self, - _database: &D, required_utxos: Vec, optional_utxos: Vec, fee_rate: FeeRate, @@ -876,13 +887,11 @@ mod test { #[test] fn test_largest_first_coin_selection_success() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let target_amount = 250_000 + FEE_AMOUNT; let result = LargestFirstCoinSelection::default() .coin_select( - &database, utxos, vec![], FeeRate::from_sat_per_vb(1.0), @@ -899,13 +908,11 @@ mod test { #[test] fn test_largest_first_coin_selection_use_all() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let target_amount = 20_000 + FEE_AMOUNT; let result = LargestFirstCoinSelection::default() .coin_select( - &database, utxos, vec![], FeeRate::from_sat_per_vb(1.0), @@ -922,13 +929,11 @@ mod test { #[test] fn test_largest_first_coin_selection_use_only_necessary() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let target_amount = 20_000 + FEE_AMOUNT; let result = LargestFirstCoinSelection::default() .coin_select( - &database, vec![], utxos, FeeRate::from_sat_per_vb(1.0), @@ -946,13 +951,11 @@ mod test { #[should_panic(expected = "InsufficientFunds")] fn test_largest_first_coin_selection_insufficient_funds() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let target_amount = 500_000 + FEE_AMOUNT; LargestFirstCoinSelection::default() .coin_select( - &database, vec![], utxos, FeeRate::from_sat_per_vb(1.0), @@ -966,13 +969,11 @@ mod test { #[should_panic(expected = "InsufficientFunds")] fn test_largest_first_coin_selection_insufficient_funds_high_fees() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let target_amount = 250_000 + FEE_AMOUNT; LargestFirstCoinSelection::default() .coin_select( - &database, vec![], utxos, FeeRate::from_sat_per_vb(1000.0), @@ -989,9 +990,8 @@ mod test { let drain_script = Script::default(); let target_amount = 180_000 + FEE_AMOUNT; - let result = OldestFirstCoinSelection::default() + let result = OldestFirstCoinSelection::new(&database) .coin_select( - &database, vec![], utxos, FeeRate::from_sat_per_vb(1.0), @@ -1048,9 +1048,8 @@ mod test { let target_amount = 180_000 + FEE_AMOUNT; - let result = OldestFirstCoinSelection::default() + let result = OldestFirstCoinSelection::new(&database) .coin_select( - &database, vec![], vec![utxo3, utxo1, utxo2], FeeRate::from_sat_per_vb(1.0), @@ -1071,9 +1070,8 @@ mod test { let drain_script = Script::default(); let target_amount = 20_000 + FEE_AMOUNT; - let result = OldestFirstCoinSelection::default() + let result = OldestFirstCoinSelection::new(&database) .coin_select( - &database, utxos, vec![], FeeRate::from_sat_per_vb(1.0), @@ -1094,9 +1092,8 @@ mod test { let drain_script = Script::default(); let target_amount = 20_000 + FEE_AMOUNT; - let result = OldestFirstCoinSelection::default() + let result = OldestFirstCoinSelection::new(&database) .coin_select( - &database, vec![], utxos, FeeRate::from_sat_per_vb(1.0), @@ -1118,9 +1115,8 @@ mod test { let drain_script = Script::default(); let target_amount = 600_000 + FEE_AMOUNT; - OldestFirstCoinSelection::default() + OldestFirstCoinSelection::new(&database) .coin_select( - &database, vec![], utxos, FeeRate::from_sat_per_vb(1.0), @@ -1139,9 +1135,8 @@ mod test { let target_amount: u64 = utxos.iter().map(|wu| wu.utxo.txout().value).sum::() - 50; let drain_script = Script::default(); - OldestFirstCoinSelection::default() + OldestFirstCoinSelection::new(&database) .coin_select( - &database, vec![], utxos, FeeRate::from_sat_per_vb(1000.0), @@ -1157,14 +1152,12 @@ mod test { // select three outputs let utxos = generate_same_value_utxos(100_000, 20); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let target_amount = 250_000 + FEE_AMOUNT; let result = BranchAndBoundCoinSelection::default() .coin_select( - &database, vec![], utxos, FeeRate::from_sat_per_vb(1.0), @@ -1181,13 +1174,11 @@ mod test { #[test] fn test_bnb_coin_selection_required_are_enough() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let target_amount = 20_000 + FEE_AMOUNT; let result = BranchAndBoundCoinSelection::default() .coin_select( - &database, utxos.clone(), utxos, FeeRate::from_sat_per_vb(1.0), @@ -1204,13 +1195,11 @@ mod test { #[test] fn test_bnb_coin_selection_optional_are_enough() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let target_amount = 299756 + FEE_AMOUNT; let result = BranchAndBoundCoinSelection::default() .coin_select( - &database, vec![], utxos, FeeRate::from_sat_per_vb(1.0), @@ -1227,7 +1216,6 @@ mod test { #[test] fn test_bnb_coin_selection_required_not_enough() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let required = vec![utxos[0].clone()]; let mut optional = utxos[1..].to_vec(); @@ -1244,7 +1232,6 @@ mod test { let result = BranchAndBoundCoinSelection::default() .coin_select( - &database, required, optional, FeeRate::from_sat_per_vb(1.0), @@ -1262,13 +1249,11 @@ mod test { #[should_panic(expected = "InsufficientFunds")] fn test_bnb_coin_selection_insufficient_funds() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let target_amount = 500_000 + FEE_AMOUNT; BranchAndBoundCoinSelection::default() .coin_select( - &database, vec![], utxos, FeeRate::from_sat_per_vb(1.0), @@ -1282,13 +1267,11 @@ mod test { #[should_panic(expected = "InsufficientFunds")] fn test_bnb_coin_selection_insufficient_funds_high_fees() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let target_amount = 250_000 + FEE_AMOUNT; BranchAndBoundCoinSelection::default() .coin_select( - &database, vec![], utxos, FeeRate::from_sat_per_vb(1000.0), @@ -1301,13 +1284,11 @@ mod test { #[test] fn test_bnb_coin_selection_check_fee_rate() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let target_amount = 99932; // first utxo's effective value let result = BranchAndBoundCoinSelection::new(0) .coin_select( - &database, vec![], utxos, FeeRate::from_sat_per_vb(1.0), @@ -1327,7 +1308,6 @@ mod test { fn test_bnb_coin_selection_exact_match() { let seed = [0; 32]; let mut rng: StdRng = SeedableRng::from_seed(seed); - let database = MemoryDatabase::default(); for _i in 0..200 { let mut optional_utxos = generate_random_utxos(&mut rng, 16); @@ -1335,7 +1315,6 @@ mod test { let drain_script = Script::default(); let result = BranchAndBoundCoinSelection::new(0) .coin_select( - &database, vec![], optional_utxos, FeeRate::from_sat_per_vb(0.0), @@ -1517,12 +1496,10 @@ mod test { #[test] fn test_bnb_exclude_negative_effective_value() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let err = BranchAndBoundCoinSelection::default() .coin_select( - &database, vec![], utxos, FeeRate::from_sat_per_vb(10.0), @@ -1543,7 +1520,6 @@ mod test { #[test] fn test_bnb_include_negative_effective_value_when_required() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let (required, optional) = utxos @@ -1552,7 +1528,6 @@ mod test { let err = BranchAndBoundCoinSelection::default() .coin_select( - &database, required, optional, FeeRate::from_sat_per_vb(10.0), @@ -1573,12 +1548,10 @@ mod test { #[test] fn test_bnb_sum_of_effective_value_negative() { let utxos = get_test_utxos(); - let database = MemoryDatabase::default(); let drain_script = Script::default(); let err = BranchAndBoundCoinSelection::default() .coin_select( - &database, utxos, vec![], FeeRate::from_sat_per_vb(10_000.0), diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index ed9623a6a..5f96fceb0 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -605,7 +605,7 @@ where } } - pub(crate) fn create_tx>( + pub(crate) fn create_tx( &self, coin_selection: Cs, params: TxParams, @@ -853,7 +853,6 @@ where }; let coin_selection = coin_selection.coin_select( - self.database.borrow().deref(), required_utxos, optional_utxos, fee_rate, diff --git a/src/wallet/tx_builder.rs b/src/wallet/tx_builder.rs index c02ff3a27..b645d2cea 100644 --- a/src/wallet/tx_builder.rs +++ b/src/wallet/tx_builder.rs @@ -181,7 +181,7 @@ impl<'a, Cs: Clone, Ctx, D> Clone for TxBuilder<'a, D, Cs, Ctx> { } // methods supported by both contexts, for any CoinSelectionAlgorithm -impl<'a, D: BatchDatabase, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> +impl<'a, D: BatchDatabase, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> TxBuilder<'a, D, Cs, Ctx> { /// Set a custom fee rate @@ -505,7 +505,7 @@ impl<'a, D: BatchDatabase, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> /// Overrides the [`DefaultCoinSelectionAlgorithm`](super::coin_selection::DefaultCoinSelectionAlgorithm). /// /// Note that this function consumes the builder and returns it so it is usually best to put this as the first call on the builder. - pub fn coin_selection>( + pub fn coin_selection( self, coin_selection: P, ) -> TxBuilder<'a, D, P, Ctx> { @@ -571,7 +571,7 @@ impl<'a, D: BatchDatabase, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> } } -impl<'a, D: BatchDatabase, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> { +impl<'a, D: BatchDatabase, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> { /// Replace the recipients already added with a new list pub fn set_recipients(&mut self, recipients: Vec<(Script, u64)>) -> &mut Self { self.params.recipients = recipients; From c93c57b37d864ad1b2ffee9bb64806424b25ae8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 29 Sep 2022 15:40:56 +0800 Subject: [PATCH 05/16] Deprecate `list_unspent` which has an unnecessary `.collect()` Use `iter_unspent` instead, as it returns an iterator directly. --- src/testutils/blockchain_tests.rs | 20 ++++++++++---------- src/wallet/mod.rs | 31 +++++++++++++++++++------------ 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/testutils/blockchain_tests.rs b/src/testutils/blockchain_tests.rs index a75d7647a..926f1e010 100644 --- a/src/testutils/blockchain_tests.rs +++ b/src/testutils/blockchain_tests.rs @@ -455,7 +455,7 @@ macro_rules! bdk_blockchain_tests { assert!(wallet.database().deref().get_sync_time().unwrap().is_some(), "sync_time hasn't been updated"); assert_eq!(wallet.get_balance().unwrap().untrusted_pending, 50_000, "incorrect balance"); - assert_eq!(wallet.list_unspent().unwrap()[0].keychain, KeychainKind::External, "incorrect keychain kind"); + assert_eq!(wallet.iter_unspent().unwrap().next().unwrap().keychain, KeychainKind::External, "incorrect keychain kind"); let list_tx_item = &wallet.list_transactions(false).unwrap()[0]; assert_eq!(list_tx_item.txid, txid, "incorrect txid"); @@ -518,7 +518,7 @@ macro_rules! bdk_blockchain_tests { assert_eq!(wallet.get_balance().unwrap().untrusted_pending, 105_000, "incorrect balance"); assert_eq!(wallet.list_transactions(false).unwrap().len(), 1, "incorrect number of txs"); - assert_eq!(wallet.list_unspent().unwrap().len(), 3, "incorrect number of unspents"); + assert_eq!(wallet.iter_unspent().unwrap().count(), 3, "incorrect number of unspents"); let list_tx_item = &wallet.list_transactions(false).unwrap()[0]; assert_eq!(list_tx_item.txid, txid, "incorrect txid"); @@ -542,7 +542,7 @@ macro_rules! bdk_blockchain_tests { assert_eq!(wallet.get_balance().unwrap().untrusted_pending, 75_000, "incorrect balance"); assert_eq!(wallet.list_transactions(false).unwrap().len(), 2, "incorrect number of txs"); - assert_eq!(wallet.list_unspent().unwrap().len(), 2, "incorrect number of unspent"); + assert_eq!(wallet.iter_unspent().unwrap().count(), 2, "incorrect number of unspent"); } #[test] @@ -576,7 +576,7 @@ macro_rules! bdk_blockchain_tests { assert_eq!(wallet.get_balance().unwrap().untrusted_pending, 50_000, "incorrect balance"); assert_eq!(wallet.list_transactions(false).unwrap().len(), 1, "incorrect number of txs"); - assert_eq!(wallet.list_unspent().unwrap().len(), 1, "incorrect unspent"); + assert_eq!(wallet.iter_unspent().unwrap().count(), 1, "incorrect unspent"); let list_tx_item = &wallet.list_transactions(false).unwrap()[0]; assert_eq!(list_tx_item.txid, txid, "incorrect txid"); @@ -590,7 +590,7 @@ macro_rules! bdk_blockchain_tests { assert_eq!(wallet.get_balance().unwrap().untrusted_pending, 50_000, "incorrect balance after bump"); assert_eq!(wallet.list_transactions(false).unwrap().len(), 1, "incorrect number of txs after bump"); - assert_eq!(wallet.list_unspent().unwrap().len(), 1, "incorrect unspent after bump"); + assert_eq!(wallet.iter_unspent().unwrap().count(), 1, "incorrect unspent after bump"); let list_tx_item = &wallet.list_transactions(false).unwrap()[0]; assert_eq!(list_tx_item.txid, new_txid, "incorrect txid after bump"); @@ -613,7 +613,7 @@ macro_rules! bdk_blockchain_tests { wallet.sync(&blockchain, SyncOptions::default()).unwrap(); assert_eq!(wallet.get_balance().unwrap().get_spendable(), 50_000, "incorrect balance"); assert_eq!(wallet.list_transactions(false).unwrap().len(), 1, "incorrect number of txs"); - assert_eq!(wallet.list_unspent().unwrap().len(), 1, "incorrect number of unspents"); + assert_eq!(wallet.iter_unspent().unwrap().count(), 1, "incorrect number of unspents"); let list_tx_item = &wallet.list_transactions(false).unwrap()[0]; assert_eq!(list_tx_item.txid, txid, "incorrect txid"); @@ -661,7 +661,7 @@ macro_rules! bdk_blockchain_tests { assert_eq!(wallet.get_balance().unwrap().confirmed, details.received, "incorrect balance after send"); assert_eq!(wallet.list_transactions(false).unwrap().len(), 2, "incorrect number of txs"); - assert_eq!(wallet.list_unspent().unwrap().len(), 1, "incorrect number of unspents"); + assert_eq!(wallet.iter_unspent().unwrap().count(), 1, "incorrect number of unspents"); } // Syncing wallet should not result in wallet address index to decrement. @@ -1178,7 +1178,7 @@ macro_rules! bdk_blockchain_tests { wallet.sync(&blockchain, SyncOptions::default()).unwrap(); assert_eq!(wallet.get_balance().unwrap().get_spendable(), details.received, "wallet has incorrect balance after send"); assert_eq!(wallet.list_transactions(false).unwrap().len(), 2, "wallet has incorrect number of txs"); - assert_eq!(wallet.list_unspent().unwrap().len(), 1, "wallet has incorrect number of unspents"); + assert_eq!(wallet.iter_unspent().unwrap().count(), 1, "wallet has incorrect number of unspents"); test_client.generate(1, None); // 5. Verify 25_000 sats are received by test bitcoind node taproot wallet @@ -1249,7 +1249,7 @@ macro_rules! bdk_blockchain_tests { let initial_tx = psbt.extract_tx(); let _sent_txid = blockchain.broadcast(&initial_tx).unwrap(); wallet.sync(&blockchain, SyncOptions::default()).unwrap(); - for utxo in wallet.list_unspent().unwrap() { + for utxo in wallet.iter_unspent().unwrap() { // Making sure the TXO we just spent is not returned by list_unspent assert!(utxo.outpoint != initial_tx.input[0].previous_output, "wallet displays spent txo in unspents"); } @@ -1265,7 +1265,7 @@ macro_rules! bdk_blockchain_tests { builder .add_utxo(initial_tx.input[0].previous_output) .expect("Can't manually add an UTXO spent"); - for utxo in wallet.list_unspent().unwrap() { + for utxo in wallet.iter_unspent().unwrap() { // Making sure the TXO we just spent is not returned by list_unspent assert!(utxo.outpoint != initial_tx.input[0].previous_output, "wallet displays spent txo in unspents"); } diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 5f96fceb0..ccafb4aaf 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -411,14 +411,22 @@ where /// /// Note that this method only operates on the internal database, which first needs to be /// [`Wallet::sync`] manually. + #[deprecated = "Use Wallet::iter_unspent"] pub fn list_unspent(&self) -> Result, Error> { + self.iter_unspent().map(|iter| iter.collect()) + } + + /// Returns an iterator of unspent outputs (UTXOs) of this wallet. + /// + /// Note that this method only operates on the internal database, which first needs to be + /// [`Wallet::sync`] manually. + pub fn iter_unspent(&self) -> Result + '_, Error> { Ok(self .database .borrow() .iter_utxos()? .into_iter() - .filter(|l| !l.is_spent) - .collect()) + .filter(|l| !l.is_spent)) } /// Returns the `UTXO` owned by this wallet corresponding to `outpoint` if it exists in the @@ -474,7 +482,7 @@ where let mut trusted_pending = 0; let mut untrusted_pending = 0; let mut confirmed = 0; - let utxos = self.list_unspent()?; + let database = self.database.borrow(); let last_sync_height = match database .get_sync_time()? @@ -484,7 +492,7 @@ where // None means database was never synced None => return Ok(Balance::default()), }; - for u in utxos { + for u in self.iter_unspent()? { // Unwrap used since utxo set is created from database let tx = database .get_tx(&u.outpoint.txid, true)? @@ -1427,8 +1435,7 @@ where fn get_available_utxos(&self) -> Result, Error> { Ok(self - .list_unspent()? - .into_iter() + .iter_unspent()? .map(|utxo| { let keychain = utxo.keychain; ( @@ -3017,7 +3024,7 @@ pub(crate) mod test { get_funded_wallet("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)"); let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap(); - let utxo = wallet2.list_unspent().unwrap().remove(0); + let utxo = wallet2.iter_unspent().unwrap().next().unwrap(); let foreign_utxo_satisfaction = wallet2 .get_descriptor_for_keychain(KeychainKind::External) .max_satisfaction_weight() @@ -3082,7 +3089,7 @@ pub(crate) mod test { fn test_add_foreign_utxo_invalid_psbt_input() { let (wallet, _, _) = get_funded_wallet(get_test_wpkh()); let mut builder = wallet.build_tx(); - let outpoint = wallet.list_unspent().unwrap()[0].outpoint; + let outpoint = wallet.iter_unspent().unwrap().next().unwrap().outpoint; let foreign_utxo_satisfaction = wallet .get_descriptor_for_keychain(KeychainKind::External) .max_satisfaction_weight() @@ -3098,7 +3105,7 @@ pub(crate) mod test { let (wallet2, _, txid2) = get_funded_wallet("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)"); - let utxo2 = wallet2.list_unspent().unwrap().remove(0); + let utxo2 = wallet2.iter_unspent().unwrap().next().unwrap(); let tx1 = wallet1 .database .borrow() @@ -3156,7 +3163,7 @@ pub(crate) mod test { let (wallet2, _, txid2) = get_funded_wallet("wpkh(cVbZ8ovhye9AoAHFsqobCf7LxbXDAECy9Kb8TZdfsDYMZGBUyCnm)"); let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap(); - let utxo2 = wallet2.list_unspent().unwrap().remove(0); + let utxo2 = wallet2.iter_unspent().unwrap().next().unwrap(); let satisfaction_weight = wallet2 .get_descriptor_for_keychain(KeychainKind::External) @@ -3225,7 +3232,7 @@ pub(crate) mod test { fn test_get_psbt_input() { // this should grab a known good utxo and set the input let (wallet, _, _) = get_funded_wallet(get_test_wpkh()); - for utxo in wallet.list_unspent().unwrap() { + for utxo in wallet.iter_unspent().unwrap() { let psbt_input = wallet.get_psbt_input(utxo, None, false).unwrap(); assert!(psbt_input.witness_utxo.is_some() || psbt_input.non_witness_utxo.is_some()); } @@ -5020,7 +5027,7 @@ pub(crate) mod test { let (wallet2, _, _) = get_funded_wallet(get_test_tr_single_sig()); let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap(); - let utxo = wallet2.list_unspent().unwrap().remove(0); + let utxo = wallet2.iter_unspent().unwrap().next().unwrap(); let psbt_input = wallet2.get_psbt_input(utxo.clone(), None, false).unwrap(); let foreign_utxo_satisfaction = wallet2 .get_descriptor_for_keychain(KeychainKind::External) From d6df1d03b5b5077f838d0b452634d9d6ed07fcba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 29 Sep 2022 13:06:03 +0800 Subject: [PATCH 06/16] `get_checksum_bytes` now checks input data for checksum If `exclude_hash` is set, we split the input data, and if a checksum already existed within the original data, we check the calculated checksum against the original checksum. Additionally, the implementation of `IntoWalletDescriptor` for `&str` has been refactored for clarity. --- src/descriptor/checksum.rs | 22 +++++++++++++++++++--- src/descriptor/mod.rs | 21 +++++++++------------ src/wallet/mod.rs | 13 ++++--------- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/descriptor/checksum.rs b/src/descriptor/checksum.rs index 5ed1151bd..8dfdac49b 100644 --- a/src/descriptor/checksum.rs +++ b/src/descriptor/checksum.rs @@ -41,12 +41,21 @@ fn poly_mod(mut c: u64, val: u64) -> u64 { c } -/// Computes the checksum bytes of a descriptor -pub fn get_checksum_bytes(desc: &str) -> Result<[u8; 8], DescriptorError> { +/// Computes the checksum bytes of a descriptor. +/// `exclude_hash = true` ignores all data after the first '#' (inclusive). +pub fn get_checksum_bytes(mut desc: &str, exclude_hash: bool) -> Result<[u8; 8], DescriptorError> { let mut c = 1; let mut cls = 0; let mut clscount = 0; + let mut original_checksum = None; + if exclude_hash { + if let Some(split) = desc.split_once('#') { + desc = split.0; + original_checksum = Some(split.1); + } + } + for ch in desc.as_bytes() { let pos = INPUT_CHARSET .iter() @@ -72,13 +81,20 @@ pub fn get_checksum_bytes(desc: &str) -> Result<[u8; 8], DescriptorError> { checksum[j] = CHECKSUM_CHARSET[((c >> (5 * (7 - j))) & 31) as usize]; } + // if input data already had a checksum, check calculated checksum against original checksum + if let Some(original_checksum) = original_checksum { + if original_checksum.as_bytes() != &checksum { + return Err(DescriptorError::InvalidDescriptorChecksum); + } + } + Ok(checksum) } /// Compute the checksum of a descriptor pub fn get_checksum(desc: &str) -> Result { // unsafe is okay here as the checksum only uses bytes in `CHECKSUM_CHARSET` - get_checksum_bytes(desc).map(|b| unsafe { String::from_utf8_unchecked(b.to_vec()) }) + get_checksum_bytes(desc, true).map(|b| unsafe { String::from_utf8_unchecked(b.to_vec()) }) } #[cfg(test)] diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 802ccd19c..7c51d27fc 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -40,6 +40,7 @@ pub mod policy; pub mod template; pub use self::checksum::get_checksum; +use self::checksum::get_checksum_bytes; pub use self::derived::{AsDerived, DerivedDescriptorKey}; pub use self::error::Error as DescriptorError; pub use self::policy::Policy; @@ -84,19 +85,15 @@ impl IntoWalletDescriptor for &str { secp: &SecpCtx, network: Network, ) -> Result<(ExtendedDescriptor, KeyMap), DescriptorError> { - let descriptor = if self.contains('#') { - let parts: Vec<&str> = self.splitn(2, '#').collect(); - if !get_checksum(parts[0]) - .ok() - .map(|computed| computed == parts[1]) - .unwrap_or(false) - { - return Err(DescriptorError::InvalidDescriptorChecksum); + let descriptor = match self.split_once('#') { + Some((desc, original_checksum)) => { + let checksum = get_checksum_bytes(desc, false)?; + if original_checksum.as_bytes() != &checksum { + return Err(DescriptorError::InvalidDescriptorChecksum); + } + desc } - - parts[0] - } else { - self + None => self, }; ExtendedDescriptor::parse_descriptor(secp, descriptor)? diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index ccafb4aaf..c5a1c8bfe 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -1957,15 +1957,10 @@ pub(crate) mod test { let (wallet, _, _) = get_funded_wallet(get_test_wpkh()); let checksum = wallet.descriptor_checksum(KeychainKind::External); assert_eq!(checksum.len(), 8); - - let raw_descriptor = wallet - .descriptor - .to_string() - .split_once('#') - .unwrap() - .0 - .to_string(); - assert_eq!(get_checksum(&raw_descriptor).unwrap(), checksum); + assert_eq!( + get_checksum(&wallet.descriptor.to_string()).unwrap(), + checksum + ); } #[test] From e92810c84c760f88cb4a6f0cdd014f88c461251e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 29 Sep 2022 14:24:28 +0800 Subject: [PATCH 07/16] Ensure backward compatibility of the "checksum inception" bug `Wallet` stores the descriptors' checksum in the database for safety. Previously, the checksum used was a checksum of a descriptor that already had a checksum. This PR allows for backward-compatibility of databases created with this bug. --- src/descriptor/checksum.rs | 2 +- src/descriptor/mod.rs | 2 +- src/wallet/mod.rs | 56 +++++++++++++++++++++++++++++++++----- 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/descriptor/checksum.rs b/src/descriptor/checksum.rs index 8dfdac49b..9eedb45b7 100644 --- a/src/descriptor/checksum.rs +++ b/src/descriptor/checksum.rs @@ -83,7 +83,7 @@ pub fn get_checksum_bytes(mut desc: &str, exclude_hash: bool) -> Result<[u8; 8], // if input data already had a checksum, check calculated checksum against original checksum if let Some(original_checksum) = original_checksum { - if original_checksum.as_bytes() != &checksum { + if original_checksum.as_bytes() != checksum { return Err(DescriptorError::InvalidDescriptorChecksum); } } diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 7c51d27fc..4955f8ff8 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -88,7 +88,7 @@ impl IntoWalletDescriptor for &str { let descriptor = match self.split_once('#') { Some((desc, original_checksum)) => { let checksum = get_checksum_bytes(desc, false)?; - if original_checksum.as_bytes() != &checksum { + if original_checksum.as_bytes() != checksum { return Err(DescriptorError::InvalidDescriptorChecksum); } desc diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index c5a1c8bfe..6b51bbfb8 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -64,6 +64,7 @@ use utils::{check_nlocktime, check_nsequence_rbf, After, Older, SecpCtx}; use crate::blockchain::{GetHeight, NoopProgress, Progress, WalletSync}; use crate::database::memory::MemoryDatabase; use crate::database::{AnyDatabase, BatchDatabase, BatchOperations, DatabaseUtils, SyncTime}; +use crate::descriptor::checksum::get_checksum_bytes; use crate::descriptor::derived::AsDerived; use crate::descriptor::policy::BuildSatisfaction; use crate::descriptor::{ @@ -203,18 +204,21 @@ where let secp = Secp256k1::new(); let (descriptor, keymap) = into_wallet_descriptor_checked(descriptor, &secp, network)?; - database.check_descriptor_checksum( + Self::db_checksum( + &mut database, + &descriptor.to_string(), KeychainKind::External, - get_checksum(&descriptor.to_string())?.as_bytes(), )?; let signers = Arc::new(SignersContainer::build(keymap, &descriptor, &secp)); + let (change_descriptor, change_signers) = match change_descriptor { Some(desc) => { let (change_descriptor, change_keymap) = into_wallet_descriptor_checked(desc, &secp, network)?; - database.check_descriptor_checksum( + Self::db_checksum( + &mut database, + &change_descriptor.to_string(), KeychainKind::Internal, - get_checksum(&change_descriptor.to_string())?.as_bytes(), )?; let change_signers = Arc::new(SignersContainer::build( @@ -222,9 +226,6 @@ where &change_descriptor, &secp, )); - // if !parsed.same_structure(descriptor.as_ref()) { - // return Err(Error::DifferentDescriptorStructure); - // } (Some(change_descriptor), change_signers) } @@ -243,6 +244,19 @@ where }) } + /// This checks the checksum within [`BatchDatabase`] twice (if needed). The first time with the + /// actual checksum, and the second time with the checksum of `descriptor+checksum`. The second + /// check is necessary for backwards compatibility of a checksum-inception bug. + fn db_checksum(db: &mut D, desc: &str, kind: KeychainKind) -> Result<(), Error> { + let checksum = get_checksum_bytes(desc, true)?; + if db.check_descriptor_checksum(kind, checksum).is_ok() { + return Ok(()); + } + + let checksum_inception = get_checksum_bytes(desc, false)?; + db.check_descriptor_checksum(kind, checksum_inception) + } + /// Get the Bitcoin network the wallet is using. pub fn network(&self) -> Network { self.network @@ -1963,6 +1977,34 @@ pub(crate) mod test { ); } + #[test] + fn test_db_checksum() { + let (wallet, _, _) = get_funded_wallet(get_test_wpkh()); + let desc = wallet.descriptor.to_string(); + + let checksum = get_checksum_bytes(&desc, true).unwrap(); + let checksum_inception = get_checksum_bytes(&desc, false).unwrap(); + let checksum_invalid = [b'q'; 8]; + + let mut db = MemoryDatabase::new(); + db.check_descriptor_checksum(KeychainKind::External, checksum) + .expect("failed to save actual checksum"); + Wallet::db_checksum(&mut db, &desc, KeychainKind::External) + .expect("db that uses actual checksum should be supported"); + + let mut db = MemoryDatabase::new(); + db.check_descriptor_checksum(KeychainKind::External, checksum_inception) + .expect("failed to save checksum inception"); + Wallet::db_checksum(&mut db, &desc, KeychainKind::External) + .expect("db that uses checksum inception should be supported"); + + let mut db = MemoryDatabase::new(); + db.check_descriptor_checksum(KeychainKind::External, checksum_invalid) + .expect("failed to save invalid checksum"); + Wallet::db_checksum(&mut db, &desc, KeychainKind::External) + .expect_err("db that uses invalid checksum should fail"); + } + #[test] fn test_get_funded_wallet_balance() { let (wallet, _, _) = get_funded_wallet(get_test_wpkh()); From 361ada76df36cd26c1d35541e1ee53f49e8c6761 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Fri, 30 Sep 2022 15:00:58 +0800 Subject: [PATCH 08/16] Introduce `CoinFilterParams` This begins work on the `wallet::coin_control` module. The idea is to filter coins, and determine which coins are fit for coin selection. `Wallet::avaliable_utxos` is introduced to use `CoinFilterParams`. --- src/wallet/coin_control.rs | 69 ++++++++++++++++++++++++++++++++++++++ src/wallet/mod.rs | 21 +++++++++++- 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 src/wallet/coin_control.rs diff --git a/src/wallet/coin_control.rs b/src/wallet/coin_control.rs new file mode 100644 index 000000000..05ed80284 --- /dev/null +++ b/src/wallet/coin_control.rs @@ -0,0 +1,69 @@ +// Bitcoin Dev Kit +// Written in 2020 by Alekos Filini +// +// Copyright (c) 2020-2021 Bitcoin Dev Kit Developers +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +//! Coin control +//! +//! This module defines how coins are to be sorted and/or grouped before coin selection. + +use std::collections::HashMap; + +use bitcoin::OutPoint; + +use crate::TransactionDetails; + +use super::COINBASE_MATURITY; + +/// Parameters to determine which coins/outputs to filter. +pub struct CoinFilterParams { + /// Outpoints to manually keep (`true`) or skip (`false`). This overrides all other parameters. + pub manual: HashMap, + + /// Whether we should filter out unconfirmed transactions. + /// TODO: Use minimum confirmations instead. + pub filter_unconfirmed: bool, + + /// Whether we should filter out immature coinbase outputs. + /// Coinbase transaction outputs need to be at least 100 blocks deep before being spendable. + /// If this is set, but `current_height == None`, all coinbase outputs will be filtered out. + pub filter_immature_coinbase: bool, + + /// Current block height. + pub current_height: Option, +} + +impl CoinFilterParams { + /// Returns true if coin is to be kept, false if coin is to be filtered out. + pub(crate) fn keep(&self, tx: &TransactionDetails, outpoint: &OutPoint) -> bool { + let raw_tx = tx.transaction.as_ref().expect("failed to obtain raw tx"); + + if let Some(&keep) = self.manual.get(outpoint) { + if keep { + return true; + } else { + return false; + } + } + + if self.filter_unconfirmed && tx.confirmation_time.is_none() { + return false; + } + + // https://github.com/bitcoin/bitcoin/blob/c5e67be03bb06a5d7885c55db1f016fbf2333fe3/src/validation.cpp#L373-L375 + if self.filter_immature_coinbase + && raw_tx.is_coin_base() + && !matches!((self.current_height, &tx.confirmation_time), (Some(tip), Some(conf)) if tip.saturating_sub(conf.height) >= COINBASE_MATURITY) + { + return false; + } + + return true; + } +} diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 6b51bbfb8..66ea14dfe 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -38,6 +38,7 @@ use miniscript::ToPublicKey; use log::{debug, error, info, trace}; pub mod address_validator; +pub mod coin_control; pub mod coin_selection; pub mod export; pub mod signer; @@ -79,6 +80,8 @@ use crate::testutils; use crate::types::*; use crate::wallet::coin_selection::Excess::{Change, NoChange}; +use self::coin_control::CoinFilterParams; + const CACHE_ADDR_BATCH_SIZE: u32 = 100; const COINBASE_MATURITY: u32 = 100; @@ -1462,6 +1465,22 @@ where .collect()) } + /// Yes, we can! + pub fn available_utxos( + &self, + params: CoinFilterParams, + ) -> Result + '_, Error> { + let db = self.database.borrow(); + + Ok(self.iter_unspent()?.filter(move |utxo| { + if let Ok(Some(tx)) = db.get_tx(&utxo.outpoint.txid, true) { + params.keep(&tx, &utxo.outpoint) + } else { + false + } + })) + } + /// Given the options returns the list of utxos that must be used to form the /// transaction and any further that may be used if needed. #[allow(clippy::type_complexity)] @@ -1472,7 +1491,7 @@ where unspendable: &HashSet, manually_selected: Vec, must_use_all_available: bool, - manual_only: bool, + manual_only: bool, // We can just manually call `CoinSelector::select`, then `CoinSelector::finish` must_only_use_confirmed_tx: bool, current_height: Option, ) -> Result<(Vec, Vec), Error> { From e58e3a530953a7478d851ff149d33451283e67b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Mon, 3 Oct 2022 14:01:14 +0800 Subject: [PATCH 09/16] Add `bdk_core::coin_select` module --- src/bdk_core/coin_select/bnb.rs | 656 ++++++++++++++++++++++ src/bdk_core/coin_select/coin_selector.rs | 653 +++++++++++++++++++++ src/bdk_core/coin_select/mod.rs | 246 ++++++++ src/bdk_core/mod.rs | 17 + src/lib.rs | 1 + src/wallet/coin_control.rs | 8 +- 6 files changed, 1575 insertions(+), 6 deletions(-) create mode 100644 src/bdk_core/coin_select/bnb.rs create mode 100644 src/bdk_core/coin_select/coin_selector.rs create mode 100644 src/bdk_core/coin_select/mod.rs create mode 100644 src/bdk_core/mod.rs diff --git a/src/bdk_core/coin_select/bnb.rs b/src/bdk_core/coin_select/bnb.rs new file mode 100644 index 000000000..eaf5acd40 --- /dev/null +++ b/src/bdk_core/coin_select/bnb.rs @@ -0,0 +1,656 @@ +use std::fmt::Display; + +use super::*; + +/// Strategy in which we should branch. +pub enum BranchStrategy { + /// We continue exploring subtrees of this node, starting with the inclusion branch. + Continue, + /// We continue exploring ONY the omission branch of this node, skipping the inclusion branch. + SkipInclusion, + /// We skip both the inclusion and omission branches of this node. + SkipBoth, +} + +impl BranchStrategy { + /// Returns true if branch strategy is one that continues (does not backtrack). + pub fn will_continue(&self) -> bool { + matches!(self, Self::Continue | Self::SkipInclusion) + } +} + +/// [`Bnb`] represents the current state of the BnB algorithm. +pub struct Bnb<'c, S> { + /// Candidate pool. + pub pool: Vec<(usize, &'c WeightedValue)>, + /// Current position within candidate pool. + pub pool_pos: usize, + /// Current best score (we want to minimize this). + pub best_score: S, + + /// Current selection. + pub selection: CoinSelector<'c>, + /// Remaining absolute value of candidate pool. + pub rem_abs: u64, + /// Remaining effective value of candidate pool. + pub rem_eff: i64, +} + +impl<'c, S: Ord> Bnb<'c, S> { + /// Creates a new [`Bnb`]. + pub fn new(selector: CoinSelector<'c>, pool: Vec<(usize, &'c WeightedValue)>, max: S) -> Self { + let (rem_abs, rem_eff) = pool.iter().fold((0, 0), |(abs, eff), (_, c)| { + ( + abs + c.value, + eff + c.effective_value(selector.opts.target_feerate), + ) + }); + + Self { + pool, + pool_pos: 0, + best_score: max, + selection: selector, + rem_abs, + rem_eff, + } + } + + /// Turns our [`Bnb`] state into an iterator. + /// + /// `strategy` should assess our current selection/node and determine the branching strategy and + /// whether this selection is a candidate solution (if so, return the score of the selection). + pub fn into_iter<'f>( + self, + strategy: &'f dyn Fn(&Self) -> (BranchStrategy, Option), + ) -> BnbIter<'c, 'f, S> { + BnbIter { + state: self, + done: false, + strategy, + } + } + + /// Attempt to backtrack to the previously selected node's omission branch, return false + /// otherwise (no more solutions). + pub fn backtrack(&mut self) -> bool { + (0..self.pool_pos).rev().any(|pos| { + let (index, candidate) = self.pool[pos]; + + if self.selection.is_selected(index) { + // deselect last `pos`, so next round will check omission branch + self.pool_pos = pos; + self.selection.deselect(index); + true + } else { + self.rem_abs += candidate.value; + self.rem_eff += candidate.effective_value(self.selection.opts.target_feerate); + false + } + }) + } + + /// Continue down this branch, skip inclusion branch if specified. + pub fn forward(&mut self, skip: bool) { + let (index, candidate) = self.pool[self.pool_pos]; + self.rem_abs -= candidate.value; + self.rem_eff -= candidate.effective_value(self.selection.opts.target_feerate); + + if !skip { + self.selection.select(index); + } + } + + /// Compare advertised score with current best. New best will be the smaller value. Return true + /// if best is replaced. + pub fn advertise_new_score(&mut self, score: S) -> bool { + if score <= self.best_score { + self.best_score = score; + return true; + } + + false + } +} + +/// An iterator for branch and bound. +pub struct BnbIter<'c, 'f, S> { + state: Bnb<'c, S>, + done: bool, + + /// Check our current selection (node), and returns the branching strategy, alongside a score + /// (if the current selection is a candidate solution). + strategy: &'f dyn Fn(&Bnb<'c, S>) -> (BranchStrategy, Option), +} + +impl<'c, 'f, S: Ord + Copy + Display> Iterator for BnbIter<'c, 'f, S> { + type Item = Option>; + + fn next(&mut self) -> Option { + if self.done { + return None; + } + + let (strategy, score) = (self.strategy)(&self.state); + + let mut found_best = Option::::None; + + if let Some(score) = score { + if self.state.advertise_new_score(score) { + found_best = Some(self.state.selection.clone()); + } + } + + debug_assert!( + !strategy.will_continue() || self.state.pool_pos < self.state.pool.len(), + "Faulty strategy implementation! Strategy suggested that we continue traversing, however we have already reached the end of the candidates pool! pool_len={}, pool_pos={}", + self.state.pool.len(), self.state.pool_pos, + ); + + match strategy { + BranchStrategy::Continue => { + self.state.forward(false); + } + BranchStrategy::SkipInclusion => { + self.state.forward(true); + } + BranchStrategy::SkipBoth => { + if !self.state.backtrack() { + self.done = true; + } + } + }; + + // increment selection pool position for next round + self.state.pool_pos += 1; + + if found_best.is_some() || !self.done { + Some(found_best) + } else { + // we have traversed all branches + None + } + } +} + +/// Determines how we should limit rounds of branch and bound. +pub enum BnbLimit { + /// Limit by round count. + Rounds(usize), + /// Limit by duration. + Duration(core::time::Duration), +} + +impl From for BnbLimit { + fn from(v: usize) -> Self { + Self::Rounds(v) + } +} + +#[cfg(feature = "std")] +impl From for BnbLimit { + fn from(v: core::time::Duration) -> Self { + Self::Duration(v) + } +} + +/// This is a variation of the Branch and Bound Coin Selection algorithm designed by Murch (as seen +/// in Bitcoin Core). +/// +/// The differences are as follows: +/// * In additional to working with effective values, we also work with absolute values. +/// This way, we can use bounds of absolute values to enforce `min_absolute_fee` (which is used by +/// RBF), and `max_extra_target` (which can be used to increase the possible solution set, given +/// that the sender is okay with sending extra to the receiver). +/// +/// Murch's Master Thesis: https://murch.one/wp-content/uploads/2016/11/erhardt2016coinselection.pdf +/// Bitcoin Core Implementation: https://github.com/bitcoin/bitcoin/blob/23.x/src/wallet/coinselection.cpp#L65 +/// +/// TODO: Another optimization we could do is figure out candidate with smallest waste, and +/// if we find a result with waste equal to this, we can just break. +pub fn coin_select_bnb(limit: L, selector: CoinSelector) -> Option +where + L: Into, +{ + let opts = selector.opts; + + // prepare pool of candidates to select from: + // * filter out candidates with negative/zero effective values + // * sort candidates by descending effective value + let pool = { + let mut pool = selector + .unselected() + .filter(|(_, c)| c.effective_value(opts.target_feerate) > 0) + .collect::>(); + pool.sort_unstable_by(|(_, a), (_, b)| { + let a = a.effective_value(opts.target_feerate); + let b = b.effective_value(opts.target_feerate); + b.cmp(&a) + }); + pool + }; + + let feerate_decreases = opts.target_feerate > opts.long_term_feerate(); + + let target_abs = opts.target_value.unwrap_or(0) + opts.min_absolute_fee; + let target_eff = selector.effective_target(); + + let upper_bound_abs = target_abs + (opts.drain_weight as f32 * opts.target_feerate) as u64; + let upper_bound_eff = target_eff + opts.drain_waste(); + + let strategy = |bnb: &Bnb| -> (BranchStrategy, Option) { + let selected_abs = bnb.selection.selected_absolute_value(); + let selected_eff = bnb.selection.selected_effective_value(); + + // backtrack if remaining value is not enough to reach target + if selected_abs + bnb.rem_abs < target_abs || selected_eff + bnb.rem_eff < target_eff { + return (BranchStrategy::SkipBoth, None); + } + + // backtrack if selected value already surpassed upper bounds + if selected_abs > upper_bound_abs && selected_eff > upper_bound_eff { + return (BranchStrategy::SkipBoth, None); + } + + let selected_waste = bnb.selection.selected_waste(); + + // when feerate decreases, waste without excess is guaranteed to increase with each + // selection. So if we have already surpassed best score, we can backtrack. + if feerate_decreases && selected_waste > bnb.best_score { + return (BranchStrategy::SkipBoth, None); + } + + // solution? + if selected_abs >= target_abs && selected_eff >= target_eff { + let waste = selected_waste + bnb.selection.current_excess(); + return (BranchStrategy::SkipBoth, Some(waste)); + } + + // early bailout optimization: + // If the candidate at the previous position is NOT selected and has the same weight and + // value as the current candidate, we can skip selecting the current candidate. + if !bnb.selection.is_empty() { + let (_, candidate) = bnb.pool[bnb.pool_pos]; + let (prev_index, prev_candidate) = bnb.pool[bnb.pool_pos - 1]; + + if !bnb.selection.is_selected(prev_index) + && candidate.value == prev_candidate.value + && candidate.weight == prev_candidate.weight + { + return (BranchStrategy::SkipInclusion, None); + } + } + + // check out inclusion branch first + (BranchStrategy::Continue, None) + }; + + // determine sum of absolute and effective values for current selection + let (selected_abs, selected_eff) = selector.selected().fold((0, 0), |(abs, eff), (_, c)| { + ( + abs + c.value, + eff + c.effective_value(selector.opts.target_feerate), + ) + }); + + let bnb = Bnb::new(selector, pool, i64::MAX); + + // not enough to select anyway + if selected_abs + bnb.rem_abs < target_abs || selected_eff + bnb.rem_eff < target_eff { + return None; + } + + match limit.into() { + BnbLimit::Rounds(rounds) => { + bnb.into_iter(&strategy) + .take(rounds) + .reduce(|b, c| if c.is_some() { c } else { b }) + } + BnbLimit::Duration(duration) => { + let start = std::time::SystemTime::now(); + bnb.into_iter(&strategy) + .take_while(|_| start.elapsed().expect("failed to get system time") <= duration) + .reduce(|b, c| if c.is_some() { c } else { b }) + } + }? +} + +#[cfg(test)] +mod test { + use bitcoin::secp256k1::Secp256k1; + + use crate::bdk_core::coin_select::evaluate_cs::evaluate; + use crate::bdk_core::coin_select::ExcessStrategyKind; + + use super::{ + coin_select_bnb, + evaluate_cs::{Evaluation, EvaluationFailure}, + tester::Tester, + CoinSelector, CoinSelectorOpt, WeightedValue, + }; + + fn tester() -> Tester { + const DESC_STR: &str = "tr(xprv9uBuvtdjghkz8D1qzsSXS9Vs64mqrUnXqzNccj2xcvnCHPpXKYE1U2Gbh9CDHk8UPyF2VuXpVkDA7fk5ZP4Hd9KnhUmTscKmhee9Dp5sBMK)"; + Tester::new(&Secp256k1::default(), DESC_STR) + } + + fn evaluate_bnb( + initial_selector: CoinSelector, + max_tries: usize, + ) -> Result { + evaluate(initial_selector, |cs| { + coin_select_bnb(max_tries, cs.clone()).map_or(false, |new_cs| { + *cs = new_cs; + true + }) + }) + } + + #[test] + fn not_enough_coins() { + let t = tester(); + let candidates: Vec = vec![ + t.gen_candidate(0, 100_000).into(), + t.gen_candidate(1, 100_000).into(), + ]; + let opts = t.gen_opts(200_000); + let selector = CoinSelector::new(&candidates, &opts); + assert!(coin_select_bnb(10_000, selector).is_none()); + } + + #[test] + fn exactly_enough_coins_preselected() { + let t = tester(); + let candidates: Vec = vec![ + t.gen_candidate(0, 100_000).into(), // to preselect + t.gen_candidate(1, 100_000).into(), // to preselect + t.gen_candidate(2, 100_000).into(), + ]; + let opts = CoinSelectorOpt { + target_feerate: 0.0, + ..t.gen_opts(200_000) + }; + let selector = { + let mut selector = CoinSelector::new(&candidates, &opts); + selector.select(0); // preselect + selector.select(1); // preselect + selector + }; + + let evaluation = evaluate_bnb(selector, 10_000).expect("eval failed"); + println!("{}", evaluation); + assert_eq!(evaluation.solution.selected, (0..=1).collect()); + assert_eq!(evaluation.solution.excess_strategies.len(), 1); + assert_eq!( + evaluation.feerate_offset(ExcessStrategyKind::ToFee).floor(), + 0.0 + ); + } + + /// `cost_of_change` acts as the upper-bound in Bnb, we check whether these boundaries are + /// enforced in code + #[test] + fn cost_of_change() { + let t = tester(); + let candidates: Vec = vec![ + t.gen_candidate(0, 200_000).into(), + t.gen_candidate(1, 200_000).into(), + t.gen_candidate(2, 200_000).into(), + ]; + + // lowest and highest possible `recipient_value` opts for derived `drain_waste`, assuming + // that we want 2 candidates selected + let (lowest_opts, highest_opts) = { + let opts = t.gen_opts(0); + + let fee_from_inputs = + (candidates[0].weight as f32 * opts.target_feerate).ceil() as u64 * 2; + let fee_from_template = + ((opts.base_weight + 2) as f32 * opts.target_feerate).ceil() as u64; + + let lowest_opts = CoinSelectorOpt { + target_value: Some( + 400_000 - fee_from_inputs - fee_from_template - opts.drain_waste() as u64, + ), + ..opts + }; + + let highest_opts = CoinSelectorOpt { + target_value: Some(400_000 - fee_from_inputs - fee_from_template), + ..opts + }; + + (lowest_opts, highest_opts) + }; + + // test lowest possible target we are able to select + let lowest_eval = evaluate_bnb(CoinSelector::new(&candidates, &lowest_opts), 10_000); + assert!(lowest_eval.is_ok()); + let lowest_eval = lowest_eval.unwrap(); + println!("LB {}", lowest_eval); + assert_eq!(lowest_eval.solution.selected.len(), 2); + assert_eq!(lowest_eval.solution.excess_strategies.len(), 1); + assert_eq!( + lowest_eval + .feerate_offset(ExcessStrategyKind::ToFee) + .floor(), + 0.0 + ); + + // test highest possible target we are able to select + let highest_eval = evaluate_bnb(CoinSelector::new(&candidates, &highest_opts), 10_000); + assert!(highest_eval.is_ok()); + let highest_eval = highest_eval.unwrap(); + println!("UB {}", highest_eval); + assert_eq!(highest_eval.solution.selected.len(), 2); + assert_eq!(highest_eval.solution.excess_strategies.len(), 1); + assert_eq!( + highest_eval + .feerate_offset(ExcessStrategyKind::ToFee) + .floor(), + 0.0 + ); + + // test lower out of bounds + let loob_opts = CoinSelectorOpt { + target_value: lowest_opts.target_value.map(|v| v - 1), + ..lowest_opts + }; + let loob_eval = evaluate_bnb(CoinSelector::new(&candidates, &loob_opts), 10_000); + assert!(loob_eval.is_err()); + println!("Lower OOB: {}", loob_eval.unwrap_err()); + + // test upper out of bounds + let uoob_opts = CoinSelectorOpt { + target_value: highest_opts.target_value.map(|v| v + 1), + ..highest_opts + }; + let uoob_eval = evaluate_bnb(CoinSelector::new(&candidates, &uoob_opts), 10_000); + assert!(uoob_eval.is_err()); + println!("Upper OOB: {}", uoob_eval.unwrap_err()); + } + + #[test] + fn try_select() { + let t = tester(); + let candidates: Vec = vec![ + t.gen_candidate(0, 300_000).into(), + t.gen_candidate(1, 300_000).into(), + t.gen_candidate(2, 300_000).into(), + t.gen_candidate(3, 200_000).into(), + t.gen_candidate(4, 200_000).into(), + ]; + let make_opts = |v: u64| -> CoinSelectorOpt { + CoinSelectorOpt { + target_feerate: 0.0, + ..t.gen_opts(v) + } + }; + + let test_cases = vec![ + (make_opts(100_000), false, 0), + (make_opts(200_000), true, 1), + (make_opts(300_000), true, 1), + (make_opts(500_000), true, 2), + (make_opts(1_000_000), true, 4), + (make_opts(1_200_000), false, 0), + (make_opts(1_300_000), true, 5), + (make_opts(1_400_000), false, 0), + ]; + + for (opts, expect_solution, expect_selected) in test_cases { + let res = evaluate_bnb(CoinSelector::new(&candidates, &opts), 10_000); + assert_eq!(res.is_ok(), expect_solution); + + match res { + Ok(eval) => { + println!("{}", eval); + assert_eq!(eval.feerate_offset(ExcessStrategyKind::ToFee), 0.0); + assert_eq!(eval.solution.selected.len(), expect_selected as usize); + } + Err(err) => println!("expected failure: {}", err), + } + } + } + + #[test] + fn early_bailout_optimization() { + let t = tester(); + + // target: 300_000 + // candidates: 2x of 125_000, 1000x of 100_000, 1x of 50_000 + // expected solution: 2x 125_000, 1x 50_000 + // set bnb max tries: 1100, should succeed + let candidates = { + let mut candidates: Vec = vec![ + t.gen_candidate(0, 125_000).into(), + t.gen_candidate(1, 125_000).into(), + t.gen_candidate(2, 50_000).into(), + ]; + (3..3 + 1000_u32) + .for_each(|index| candidates.push(t.gen_candidate(index, 100_000).into())); + candidates + }; + let opts = CoinSelectorOpt { + target_feerate: 0.0, + ..t.gen_opts(300_000) + }; + + let result = evaluate_bnb(CoinSelector::new(&candidates, &opts), 1100); + assert!(result.is_ok()); + + let eval = result.unwrap(); + println!("{}", eval); + assert_eq!(eval.solution.selected, (0..=2).collect()); + } + + #[test] + fn should_exhaust_iteration() { + static MAX_TRIES: usize = 1000; + let t = tester(); + let candidates = (0..MAX_TRIES + 1) + .map(|index| t.gen_candidate(index as _, 10_000).into()) + .collect::>(); + let opts = t.gen_opts(10_001 * MAX_TRIES as u64); + let result = evaluate_bnb(CoinSelector::new(&candidates, &opts), MAX_TRIES); + assert!(result.is_err()); + println!("error as expected: {}", result.unwrap_err()); + } + + /// Solution should have fee >= min_absolute_fee (or no solution at all) + #[test] + fn min_absolute_fee() { + let t = tester(); + let candidates = { + let mut candidates = Vec::new(); + t.gen_weighted_values(&mut candidates, 5, 10_000); + t.gen_weighted_values(&mut candidates, 5, 20_000); + t.gen_weighted_values(&mut candidates, 5, 30_000); + t.gen_weighted_values(&mut candidates, 10, 10_300); + t.gen_weighted_values(&mut candidates, 10, 10_500); + t.gen_weighted_values(&mut candidates, 10, 10_700); + t.gen_weighted_values(&mut candidates, 10, 10_900); + t.gen_weighted_values(&mut candidates, 10, 11_000); + t.gen_weighted_values(&mut candidates, 10, 12_000); + t.gen_weighted_values(&mut candidates, 10, 13_000); + candidates + }; + let mut opts = CoinSelectorOpt { + min_absolute_fee: 1, + ..t.gen_opts(100_000) + }; + + (1..=120_u64).for_each(|fee_factor| { + opts.min_absolute_fee = fee_factor * 31; + + let result = evaluate_bnb(CoinSelector::new(&candidates, &opts), 21_000); + match result { + Ok(result) => { + println!("Solution {}", result); + let fee = result.solution.excess_strategies[&ExcessStrategyKind::ToFee].fee; + assert!(fee >= opts.min_absolute_fee); + assert_eq!(result.solution.excess_strategies.len(), 1); + } + Err(err) => { + println!("No Solution: {}", err); + } + } + }); + } + + /// For a decreasing feerate (longterm feerate is lower than effective feerate), we should + /// select less. For increasing feerate (longterm feerate is higher than effective feerate), we + /// should select more. + #[test] + fn feerate_difference() { + let t = tester(); + let candidates = { + let mut candidates = Vec::new(); + t.gen_weighted_values(&mut candidates, 10, 2_000); + t.gen_weighted_values(&mut candidates, 10, 5_000); + t.gen_weighted_values(&mut candidates, 10, 20_000); + candidates + }; + + let decreasing_feerate_opts = CoinSelectorOpt { + target_feerate: 1.25, + long_term_feerate: Some(0.25), + ..t.gen_opts(100_000) + }; + + let increasing_feerate_opts = CoinSelectorOpt { + target_feerate: 0.25, + long_term_feerate: Some(1.25), + ..t.gen_opts(100_000) + }; + + let decreasing_res = evaluate_bnb( + CoinSelector::new(&candidates, &decreasing_feerate_opts), + 21_000, + ) + .expect("no result"); + let decreasing_len = decreasing_res.solution.selected.len(); + + let increasing_res = evaluate_bnb( + CoinSelector::new(&candidates, &increasing_feerate_opts), + 21_000, + ) + .expect("no result"); + let increasing_len = increasing_res.solution.selected.len(); + + println!("decreasing_len: {}", decreasing_len); + println!("increasing_len: {}", increasing_len); + assert!(decreasing_len < increasing_len); + } + + /// TODO: UNIMPLEMENTED TESTS: + /// * Excess strategies: + /// * We should always have `ExcessStrategy::ToFee`. + /// * We should only have `ExcessStrategy::ToRecipient` when `max_extra_target > 0`. + /// * We should only have `ExcessStrategy::ToDrain` when `drain_value >= min_drain_value`. + /// * Fuzz + /// * Solution feerate should never be lower than target feerate + /// * Solution fee should never be lower than `min_absolute_fee` + /// * Preselected should always remain selected + fn _todo() {} +} diff --git a/src/bdk_core/coin_select/coin_selector.rs b/src/bdk_core/coin_select/coin_selector.rs new file mode 100644 index 000000000..677104bae --- /dev/null +++ b/src/bdk_core/coin_select/coin_selector.rs @@ -0,0 +1,653 @@ +use std::collections::{BTreeSet, HashMap}; + +use bitcoin::{Transaction, TxOut}; + +use super::*; + +/// A [`WeightedValue`] represents an input candidate for [`CoinSelector`]. This can either be a +/// single UTXO, or a group of UTXOs that should be spent together. +#[derive(Debug, Clone, Copy)] +pub struct WeightedValue { + /// Total value of the UTXO(s) that this [`WeightedValue`] represents. + pub value: u64, + /// Total weight of including this/these UTXO(s). + /// `txin` fields: `prevout`, `nSequence`, `scriptSigLen`, `scriptSig`, `scriptWitnessLen`, + /// `scriptWitness` should all be included. + pub weight: u32, + /// Total number of inputs; so we can calculate extra `varint` weight due to `vin` len changes. + pub input_count: usize, + /// Whether this [`WeightedValue`] contains at least one segwit spend. + pub is_segwit: bool, +} + +impl WeightedValue { + /// Create a new [`WeightedValue`] that represents a single input. + /// + /// `satisfaction_weight` is the weight of `scriptSigLen + scriptSig + scriptWitnessLen + + /// scriptWitness`. + pub fn new(value: u64, satisfaction_weight: u32, is_segwit: bool) -> WeightedValue { + let weight = TXIN_BASE_WEIGHT + satisfaction_weight; + WeightedValue { + value, + weight, + input_count: 1, + is_segwit, + } + } + + /// Effective value of this input candidate: `actual_value - input_weight * feerate (sats/wu)`. + pub fn effective_value(&self, effective_feerate: f32) -> i64 { + // We prefer undershooting the candidate's effective value (so we over estimate the fee of a + // candidate). If we overshoot the candidate's effective value, it may be possible to find a + // solution which does not meet the target feerate. + self.value as i64 - (self.weight as f32 * effective_feerate).ceil() as i64 + } +} + +/// Options for [`CoinSelector`]. +#[derive(Debug, Clone, Copy)] +pub struct CoinSelectorOpt { + /// The value we need to select. + /// If the value is `None` then the selection will be complete if it can pay for the drain + /// output and satisfy the other constraints (e.g. minimum fees). + pub target_value: Option, + /// Additional leeway for the target value. + pub max_extra_target: u64, // TODO: Maybe out of scope here? + + /// The feerate we should try and achieve in sats per weight unit. + pub target_feerate: f32, + /// The feerate + pub long_term_feerate: Option, // TODO: Maybe out of scope? (waste) + /// The minimum absolute fee. I.e. needed for RBF. + pub min_absolute_fee: u64, + + /// The weight of the template transaction including fixed fields and outputs. + pub base_weight: u32, + /// Additional weight if we include the drain (change) output. + pub drain_weight: u32, + /// Weight of spending the drain (change) output in the future. + pub spend_drain_weight: u32, // TODO: Maybe out of scope? (waste) + + /// Minimum value allowed for a drain (change) output. + pub min_drain_value: u64, +} + +impl CoinSelectorOpt { + fn from_weights(base_weight: u32, drain_weight: u32, spend_drain_weight: u32) -> Self { + // 0.25 sats/wu == 1 sat/vb + let target_feerate = 0.25_f32; + + // set `min_drain_value` to dust limit + let min_drain_value = + 3 * ((drain_weight + spend_drain_weight) as f32 * target_feerate) as u64; + + Self { + target_value: None, + max_extra_target: 0, + target_feerate, + long_term_feerate: None, + min_absolute_fee: 0, + base_weight, + drain_weight, + spend_drain_weight, + min_drain_value, + } + } + + /// Generate [`CoinSelectorOpt`] from `txout`s. + pub fn fund_outputs( + txouts: &[TxOut], + drain_output: &TxOut, + drain_satisfaction_weight: u32, + ) -> Self { + let mut tx = Transaction { + input: vec![], + version: 1, + lock_time: 0, + output: txouts.to_vec(), + }; + let base_weight = tx.weight(); + // this awkward calculation is necessary since TxOut doesn't have \.weight() + let drain_weight = { + tx.output.push(drain_output.clone()); + tx.weight() - base_weight + }; + Self { + target_value: if txouts.is_empty() { + None + } else { + Some(txouts.iter().map(|txout| txout.value).sum()) + }, + ..Self::from_weights( + base_weight as u32, + drain_weight as u32, + TXIN_BASE_WEIGHT + drain_satisfaction_weight, + ) + } + } + + /// Obtain long term feerate. + pub fn long_term_feerate(&self) -> f32 { + self.long_term_feerate.unwrap_or(self.target_feerate) + } + + /// Calculate the "cost of drain". + pub fn drain_waste(&self) -> i64 { + (self.drain_weight as f32 * self.target_feerate + + self.spend_drain_weight as f32 * self.long_term_feerate()) as i64 + } +} + +/// [`CoinSelector`] is responsible for selecting and deselecting from a set of canididates. +#[derive(Debug, Clone)] +pub struct CoinSelector<'a> { + /// Options. + pub opts: &'a CoinSelectorOpt, + /// Input candidates. + pub candidates: &'a Vec, + selected: BTreeSet, +} + +impl<'a> CoinSelector<'a> { + /// Get input candidate at index. + pub fn candidate(&self, index: usize) -> &WeightedValue { + &self.candidates[index] + } + + /// New [`CoinSelector`]. + pub fn new(candidates: &'a Vec, opts: &'a CoinSelectorOpt) -> Self { + Self { + candidates, + selected: Default::default(), + opts, + } + } + + /// Select candidate at index. + pub fn select(&mut self, index: usize) -> bool { + assert!(index < self.candidates.len()); + self.selected.insert(index) + } + + /// Deselect candidate at index. + pub fn deselect(&mut self, index: usize) -> bool { + self.selected.remove(&index) + } + + /// Whether candidate at index is selected. + pub fn is_selected(&self, index: usize) -> bool { + self.selected.contains(&index) + } + + /// Returns true when there is nothing selected. + pub fn is_empty(&self) -> bool { + self.selected.is_empty() + } + + /// Weight sum of all selected inputs. + pub fn selected_weight(&self) -> u32 { + self.selected + .iter() + .map(|&index| self.candidates[index].weight) + .sum() + } + + /// Effective value sum of all selected inputs. + pub fn selected_effective_value(&self) -> i64 { + self.selected + .iter() + .map(|&index| self.candidates[index].effective_value(self.opts.target_feerate)) + .sum() + } + + /// Absolute value sum of all selected inputs. + pub fn selected_absolute_value(&self) -> u64 { + self.selected + .iter() + .map(|&index| self.candidates[index].value) + .sum() + } + + /// Waste sum of all selected inputs. + pub fn selected_waste(&self) -> i64 { + (self.selected_weight() as f32 * (self.opts.target_feerate - self.opts.long_term_feerate())) + as i64 + } + + /// Current weight of template tx + selected inputs. + pub fn current_weight(&self) -> u32 { + let witness_header_extra_weight = self + .selected() + .find(|(_, wv)| wv.is_segwit) + .map(|_| 2) + .unwrap_or(0); + let vin_count_varint_extra_weight = { + let input_count = self.selected().map(|(_, wv)| wv.input_count).sum::(); + (varint_size(input_count) - 1) * 4 + }; + self.opts.base_weight + + self.selected_weight() + + witness_header_extra_weight + + vin_count_varint_extra_weight + } + + /// Current excess. + pub fn current_excess(&self) -> i64 { + self.selected_effective_value() - self.effective_target() + } + + /// This is the effective target value. + pub fn effective_target(&self) -> i64 { + let (has_segwit, max_input_count) = self + .candidates + .iter() + .fold((false, 0_usize), |(is_segwit, input_count), c| { + (is_segwit || c.is_segwit, input_count + c.input_count) + }); + + let effective_base_weight = self.opts.base_weight + + if has_segwit { 2_u32 } else { 0_u32 } + + (varint_size(max_input_count) - 1) * 4; + + self.opts.target_value.unwrap_or(0) as i64 + + (effective_base_weight as f32 * self.opts.target_feerate).ceil() as i64 + } + + /// Returns selected count. + pub fn selected_count(&self) -> usize { + self.selected.len() + } + + /// Iterates selected candidates. + pub fn selected(&self) -> impl Iterator + '_ { + self.selected + .iter() + .map(move |&index| (index, &self.candidates[index])) + } + + /// Iterates unselected candidates. + pub fn unselected(&self) -> impl Iterator + '_ { + self.candidates + .iter() + .enumerate() + .filter(move |(index, _)| !self.selected.contains(index)) + } + + /// Iterates indexes of selected candidates. + pub fn selected_indexes(&self) -> impl Iterator + '_ { + self.selected.iter().cloned() + } + + /// Iterates indexes of unselected candidates. + pub fn unselected_indexes(&self) -> impl Iterator + '_ { + (0..self.candidates.len()).filter(move |index| !self.selected.contains(index)) + } + + /// Returns true when all input candidates are selected. + pub fn all_selected(&self) -> bool { + self.selected.len() == self.candidates.len() + } + + /// Select all candidates. + pub fn select_all(&mut self) { + self.selected = (0..self.candidates.len()).collect(); + } + + /// Attempt to select until we can call `finish` successfully. + pub fn select_until_finished(&mut self) -> Result { + let mut selection = self.finish(); + + if selection.is_ok() { + return selection; + } + + let unselected = self.unselected_indexes().collect::>(); + + for index in unselected { + self.select(index); + selection = self.finish(); + + if selection.is_ok() { + break; + } + } + + selection + } + + /// Succeeds when parameters are all satisfied and we have a valid selection. + pub fn finish(&self) -> Result { + let weight_without_drain = self.current_weight(); + let weight_with_drain = weight_without_drain + self.opts.drain_weight; + + let fee_without_drain = + (weight_without_drain as f32 * self.opts.target_feerate).ceil() as u64; + let fee_with_drain = (weight_with_drain as f32 * self.opts.target_feerate).ceil() as u64; + + let inputs_minus_outputs = { + let target_value = self.opts.target_value.unwrap_or(0); + let selected = self.selected_absolute_value(); + + // find the largest unsatisfied constraint (if any), and return error of that constraint + // "selected" should always be greater than or equal to these selected values + [ + ( + SelectionConstraint::TargetValue, + target_value.saturating_sub(selected), + ), + ( + SelectionConstraint::TargetFee, + (target_value + fee_without_drain).saturating_sub(selected), + ), + ( + SelectionConstraint::MinAbsoluteFee, + (target_value + self.opts.min_absolute_fee).saturating_sub(selected), + ), + ( + SelectionConstraint::MinDrainValue, + // when we have no target value (hence no recipient txouts), we need to ensure + // the selected amount can satisfy requirements for a drain output (so we at + // least have one txout) + if self.opts.target_value.is_none() { + (fee_with_drain + self.opts.min_drain_value).saturating_sub(selected) + } else { + 0 + }, + ), + ] + .iter() + .filter(|&(_, v)| *v > 0) + .max_by_key(|&(_, v)| v) + .map_or(Ok(()), |&(constraint, missing)| { + Err(SelectionFailure { + selected, + missing, + constraint, + }) + })?; + + (selected - target_value) as u64 + }; + + let fee_without_drain = fee_without_drain.max(self.opts.min_absolute_fee); + let fee_with_drain = fee_with_drain.max(self.opts.min_absolute_fee); + + let excess_without_drain = inputs_minus_outputs - fee_without_drain; + let input_waste = self.selected_waste(); + + // begin preparing excess strategies for final selection + let mut excess_strategies = HashMap::new(); + + // only allow `ToFee` and `ToRecipient` excess strategies when we have a `target_value`, + // otherwise we will result in a result with no txouts, or attempt to add value to an output + // that does not exist + if self.opts.target_value.is_some() { + // no drain, excess to fee + excess_strategies.insert( + ExcessStrategyKind::ToFee, + ExcessStrategy { + recipient_value: self.opts.target_value, + drain_value: None, + fee: fee_without_drain + excess_without_drain, + weight: weight_without_drain, + waste: input_waste + excess_without_drain as i64, + }, + ); + + // no drain, excess to recipient + // if `excess == 0`, this result will be the same as the previous, so don't consider it + // if `max_extra_target == 0`, there is no leeway for this strategy + if excess_without_drain > 0 && self.opts.max_extra_target > 0 { + let extra_recipient_value = + core::cmp::min(self.opts.max_extra_target, excess_without_drain); + let extra_fee = excess_without_drain - extra_recipient_value; + excess_strategies.insert( + ExcessStrategyKind::ToRecipient, + ExcessStrategy { + recipient_value: self.opts.target_value.map(|v| v + extra_recipient_value), + drain_value: None, + fee: fee_without_drain + extra_fee, + weight: weight_without_drain, + waste: input_waste + extra_fee as i64, + }, + ); + } + } + + // with drain + if fee_with_drain > self.opts.min_absolute_fee + && inputs_minus_outputs >= fee_with_drain + self.opts.min_drain_value + { + excess_strategies.insert( + ExcessStrategyKind::ToDrain, + ExcessStrategy { + recipient_value: self.opts.target_value, + drain_value: Some(inputs_minus_outputs.saturating_sub(fee_with_drain)), + fee: fee_with_drain, + weight: weight_with_drain, + waste: input_waste + self.opts.drain_waste(), + }, + ); + } + + debug_assert!( + !excess_strategies.is_empty(), + "should have at least one excess strategy" + ); + + Ok(Selection { + selected: self.selected.clone(), + excess: excess_without_drain, + excess_strategies, + }) + } +} + +/// Represents a selection failure. +#[derive(Clone, Debug)] +pub struct SelectionFailure { + selected: u64, + missing: u64, + constraint: SelectionConstraint, +} + +impl core::fmt::Display for SelectionFailure { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!( + f, + "insufficient coins selected; selected={}, missing={}, unsatisfied_constraint={:?}", + self.selected, self.missing, self.constraint + ) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for SelectionFailure {} + +/// Represents a constraint that is not met for coin selection. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SelectionConstraint { + /// The target is not met + TargetValue, + /// The target fee (given the feerate) is not met + TargetFee, + /// Min absolute fee is not met + MinAbsoluteFee, + /// Min drain value is not met + MinDrainValue, +} + +impl core::fmt::Display for SelectionConstraint { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + SelectionConstraint::TargetValue => core::write!(f, "target_value"), + SelectionConstraint::TargetFee => core::write!(f, "target_fee"), + SelectionConstraint::MinAbsoluteFee => core::write!(f, "min_absolute_fee"), + SelectionConstraint::MinDrainValue => core::write!(f, "min_drain_value"), + } + } +} + +/// Coin selection result. +#[derive(Clone, Debug)] +pub struct Selection { + /// Selected indexes. + pub selected: BTreeSet, + /// Excess value (in satoshis). + pub excess: u64, + /// Avaliable excess strategies. + pub excess_strategies: HashMap, +} + +/// The kind of excess strategy. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, core::hash::Hash)] +pub enum ExcessStrategyKind { + /// Excess goes to fee. + ToFee, + /// Excess goes to recipient. + ToRecipient, + /// Excess goes to drain output. + ToDrain, +} + +/// Strategy of dealing with excess. +#[derive(Clone, Copy, Debug)] +pub struct ExcessStrategy { + /// Recipient value. + pub recipient_value: Option, + /// Drain output value. + pub drain_value: Option, + /// Miner fee. + pub fee: u64, + /// Weight of transaction. + pub weight: u32, + /// Waste. + pub waste: i64, +} + +impl Selection { + /// Apply selection to original candidates. + pub fn apply_selection<'a, T>( + &'a self, + candidates: &'a [T], + ) -> impl Iterator + 'a { + self.selected.iter().map(move |i| &candidates[*i]) + } + + /// Returns the [`ExcessStrategy`] that results in the least waste. + pub fn best_strategy(&self) -> (&ExcessStrategyKind, &ExcessStrategy) { + self.excess_strategies + .iter() + .min_by_key(|&(_, a)| a.waste) + .expect("selection has no excess strategy") + } +} + +impl core::fmt::Display for ExcessStrategyKind { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + ExcessStrategyKind::ToFee => core::write!(f, "to_fee"), + ExcessStrategyKind::ToRecipient => core::write!(f, "to_recipient"), + ExcessStrategyKind::ToDrain => core::write!(f, "to_drain"), + } + } +} + +impl ExcessStrategy { + /// Returns feerate in sats/wu. + pub fn feerate(&self) -> f32 { + self.fee as f32 / self.weight as f32 + } +} + +#[cfg(test)] +mod test { + use crate::bdk_core::coin_select::{ExcessStrategyKind, SelectionConstraint}; + + use super::{CoinSelector, CoinSelectorOpt, WeightedValue}; + + /// Ensure `target_value` is respected. Can't have no disrespect. + #[test] + fn target_value_respected() { + let target_value = 1000_u64; + + let candidates = (500..1500_u64) + .map(|value| WeightedValue { + value, + weight: 100, + input_count: 1, + is_segwit: false, + }) + .collect::>(); + + let opts = CoinSelectorOpt { + target_value: Some(target_value), + max_extra_target: 0, + target_feerate: 0.00, + long_term_feerate: None, + min_absolute_fee: 0, + base_weight: 10, + drain_weight: 10, + spend_drain_weight: 10, + min_drain_value: 10, + }; + + for (index, v) in candidates.iter().enumerate() { + let mut selector = CoinSelector::new(&candidates, &opts); + assert!(selector.select(index)); + + let res = selector.finish(); + if v.value < opts.target_value.unwrap_or(0) { + let err = res.expect_err("should have failed"); + assert_eq!(err.selected, v.value); + assert_eq!(err.missing, target_value - v.value); + assert_eq!(err.constraint, SelectionConstraint::MinAbsoluteFee); + } else { + let sel = res.expect("should have succeeded"); + assert_eq!(sel.excess, v.value - opts.target_value.unwrap_or(0)); + } + } + } + + #[test] + fn drain_all() { + let candidates = (0..100) + .map(|_| WeightedValue { + value: 666, + weight: 166, + input_count: 1, + is_segwit: false, + }) + .collect::>(); + + let opts = CoinSelectorOpt { + target_value: None, + max_extra_target: 0, + target_feerate: 0.25, + long_term_feerate: None, + min_absolute_fee: 0, + base_weight: 10, + drain_weight: 100, + spend_drain_weight: 66, + min_drain_value: 1000, + }; + + let selection = CoinSelector::new(&candidates, &opts) + .select_until_finished() + .expect("should succeed"); + + assert!(selection.selected.len() > 1); + assert_eq!(selection.excess_strategies.len(), 1); + + let (kind, strategy) = selection.best_strategy(); + assert_eq!(*kind, ExcessStrategyKind::ToDrain); + assert!(strategy.recipient_value.is_none()); + assert!(strategy.drain_value.is_some()); + } + + /// TODO: Tests to add: + /// * `finish` should ensure at least `target_value` is selected. + /// * actual feerate should be equal or higher than `target_feerate`. + /// * actual drain value should be equal or higher than `min_drain_value` (or else no drain). + fn _todo() {} +} diff --git a/src/bdk_core/coin_select/mod.rs b/src/bdk_core/coin_select/mod.rs new file mode 100644 index 000000000..601a2d1a4 --- /dev/null +++ b/src/bdk_core/coin_select/mod.rs @@ -0,0 +1,246 @@ +mod bnb; +pub use bnb::*; + +mod coin_selector; +pub use coin_selector::*; + +/// Txin "base" fields include `outpoint` (32+4) and `nSequence` (4). This does not include +/// `scriptSigLen` or `scriptSig`. +pub const TXIN_BASE_WEIGHT: u32 = (32 + 4 + 4) * 4; + +/// Helper to calculate varint size. `v` is the value the varint represents. +pub fn varint_size(v: usize) -> u32 { + if v <= 0xfc { + return 1; + } + if v <= 0xffff { + return 3; + } + if v <= 0xffff_ffff { + return 5; + } + + 9 +} + +pub mod evaluate_cs { + //! Coin Select Evaluation + //! + //! Module to evaluate coin selection. + + use super::{CoinSelector, ExcessStrategyKind, Selection}; + + /// Evaluates a coin selection algorithm. + pub fn evaluate( + initial_selector: CoinSelector, + mut select: F, + ) -> Result + where + F: FnMut(&mut CoinSelector) -> bool, + { + let mut selector = initial_selector.clone(); + let start_time = std::time::SystemTime::now(); + let has_solution = select(&mut selector); + let elapsed = start_time.elapsed().expect("system time error"); + + if has_solution { + let solution = selector.finish().expect("failed to finish what we started"); + + let elapsed_per_candidate = elapsed / selector.candidates.len() as _; + + let waste_vec = solution + .excess_strategies + .iter() + .map(|(_, s)| s.waste) + .collect::>(); + + let waste_mean = waste_vec.iter().sum::() as f32 / waste_vec.len() as f32; + let waste_median = if waste_vec.len() % 2 != 0 { + waste_vec[waste_vec.len() / 2] as f32 + } else { + (waste_vec[(waste_vec.len() - 1) / 2] + waste_vec[waste_vec.len() / 2]) as f32 / 2.0 + }; + + Ok(Evaluation { + initial_selector, + solution, + elapsed, + elapsed_per_candidate, + waste_median, + waste_mean, + }) + } else { + Err(EvaluationFailure { + initial: initial_selector, + elapsed, + }) + } + } + + /// Evaluation result of a coin selection. + #[derive(Debug, Clone)] + pub struct Evaluation<'a> { + /// Initial [`CoinSelector`]. + pub initial_selector: CoinSelector<'a>, + /// Final solution. + pub solution: Selection, + + /// Elapsed duration of coin selection. + pub elapsed: std::time::Duration, + /// Elapsed duration per candidate. + pub elapsed_per_candidate: std::time::Duration, + + /// Median waste. + pub waste_median: f32, + /// Mean waste. + pub waste_mean: f32, + } + + impl<'a> Evaluation<'a> { + /// Obtain waste of specified excess strategy kind. + pub fn waste(&self, strategy_kind: ExcessStrategyKind) -> i64 { + self.solution.excess_strategies[&strategy_kind].waste + } + + /// Obtain feerate offset of specified excess strategy kind. + pub fn feerate_offset(&self, strategy_kind: ExcessStrategyKind) -> f32 { + let target_rate = self.initial_selector.opts.target_feerate; + let actual_rate = self.solution.excess_strategies[&strategy_kind].feerate(); + actual_rate - target_rate + } + } + + impl<'a> core::fmt::Display for Evaluation<'a> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + writeln!(f, "Evaluation:")?; + writeln!( + f, + "\t* Candidates: {}", + self.initial_selector.candidates.len() + )?; + writeln!( + f, + "\t* Initial selection: {}", + self.initial_selector.selected_count() + )?; + writeln!(f, "\t* Final selection: {}", self.solution.selected.len())?; + writeln!(f, "\t* Elapsed: {:?}", self.elapsed)?; + writeln!( + f, + "\t* Elapsed per candidate: {:?}", + self.elapsed_per_candidate + )?; + Ok(()) + } + } + + /// Evaluation failure. + #[derive(Debug, Clone)] + pub struct EvaluationFailure<'a> { + initial: CoinSelector<'a>, + elapsed: std::time::Duration, + } + + impl<'a> core::fmt::Display for EvaluationFailure<'a> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!( + f, + "cs algorithm failed to find a solution: elapsed={}s target_feerate={}sats/wu", + self.elapsed.as_secs(), + self.initial.opts.target_feerate + ) + } + } + + impl<'a> std::error::Error for EvaluationFailure<'a> {} +} + +#[cfg(test)] +pub mod tester { + use super::*; + use bitcoin::{ + secp256k1::{All, Secp256k1}, + TxOut, + }; + use miniscript::{ + // plan::{Assets, Plan}, + Descriptor, + DescriptorPublicKey, + DescriptorTrait, + }; + + #[derive(Debug, Clone)] + pub struct TestCandidate { + pub txo: TxOut, + // pub plan: Plan, + } + + impl From for WeightedValue { + fn from(test_candidate: TestCandidate) -> Self { + Self { + value: test_candidate.txo.value, + // weight: TXIN_BASE_WEIGHT + test_candidate.plan.expected_weight() as u32, + weight: TXIN_BASE_WEIGHT + 65, // TODO: Is this correct for schnorr? + input_count: 1, + // is_segwit: test_candidate.plan.witness_version().is_some(), + is_segwit: true, + } + } + } + + pub struct Tester { + descriptor: Descriptor, + // assets: Assets, + } + + impl Tester { + pub fn new(secp: &Secp256k1, desc_str: &str) -> Self { + // let desc_str = "tr(xprv9uBuvtdjghkz8D1qzsSXS9Vs64mqrUnXqzNccj2xcvnCHPpXKYE1U2Gbh9CDHk8UPyF2VuXpVkDA7fk5ZP4Hd9KnhUmTscKmhee9Dp5sBMK)"; + let (descriptor, _seckeys) = + Descriptor::::parse_descriptor(secp, desc_str).unwrap(); + + // let assets = Assets { + // keys: seckeys.keys().cloned().collect(), + // ..Default::default() + // }; + + // Self { descriptor, assets } + Self { descriptor } + } + + pub fn gen_candidate(&self, derivation_index: u32, value: u64) -> TestCandidate { + let secp = Secp256k1::new(); + // let descriptor = self.descriptor.at_derivation_index(derivation_index); + let descriptor = self + .descriptor + .derived_descriptor(&secp, derivation_index) + .expect("woops"); + // let plan = descriptor.plan_satisfaction(&self.assets).unwrap(); + let txo = TxOut { + value, + script_pubkey: descriptor.script_pubkey(), + }; + // TestCandidate { txo, plan } + TestCandidate { txo } + } + + pub fn gen_weighted_value(&self, value: u64) -> WeightedValue { + self.gen_candidate(0, value).into() + } + + pub fn gen_weighted_values(&self, out: &mut Vec, count: usize, value: u64) { + (0..count).for_each(|_| out.push(self.gen_candidate(0, value).into())) + } + + pub fn gen_opts(&self, recipient_value: u64) -> CoinSelectorOpt { + let recipient = self.gen_candidate(0, recipient_value); + let drain = self.gen_candidate(0, 0); + CoinSelectorOpt::fund_outputs( + &[recipient.txo], + &drain.txo, + 65, + // drain.plan.expected_weight() as u32, + ) + } + } +} diff --git a/src/bdk_core/mod.rs b/src/bdk_core/mod.rs new file mode 100644 index 000000000..0329588c0 --- /dev/null +++ b/src/bdk_core/mod.rs @@ -0,0 +1,17 @@ +// Bitcoin Dev Kit +// Written in 2020 by Alekos Filini +// +// Copyright (c) 2020-2021 Bitcoin Dev Kit Developers +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +//! BDK Core +//! +//! This is the core modules of BDK. + +mod coin_select; +pub use coin_select::*; diff --git a/src/lib.rs b/src/lib.rs index 65e35a72a..068557161 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -257,6 +257,7 @@ pub mod testutils; #[allow(unused_imports)] #[macro_use] pub(crate) mod error; +pub mod bdk_core; pub mod blockchain; pub mod database; pub mod descriptor; diff --git a/src/wallet/coin_control.rs b/src/wallet/coin_control.rs index 05ed80284..74aa92679 100644 --- a/src/wallet/coin_control.rs +++ b/src/wallet/coin_control.rs @@ -45,11 +45,7 @@ impl CoinFilterParams { let raw_tx = tx.transaction.as_ref().expect("failed to obtain raw tx"); if let Some(&keep) = self.manual.get(outpoint) { - if keep { - return true; - } else { - return false; - } + return keep; } if self.filter_unconfirmed && tx.confirmation_time.is_none() { @@ -64,6 +60,6 @@ impl CoinFilterParams { return false; } - return true; + true } } From 1b17cfb49597ef1d29e32e8da83fda190182869d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Mon, 3 Oct 2022 16:19:50 +0800 Subject: [PATCH 10/16] WIP: Add bdk_core coin select --- src/bdk_core/coin_select/bnb.rs | 1 + src/error.rs | 6 + src/wallet/coin_selection.rs | 1882 +++++++++++++------------------ src/wallet/mod.rs | 223 ++-- 4 files changed, 926 insertions(+), 1186 deletions(-) diff --git a/src/bdk_core/coin_select/bnb.rs b/src/bdk_core/coin_select/bnb.rs index eaf5acd40..e2c5baf7e 100644 --- a/src/bdk_core/coin_select/bnb.rs +++ b/src/bdk_core/coin_select/bnb.rs @@ -173,6 +173,7 @@ impl<'c, 'f, S: Ord + Copy + Display> Iterator for BnbIter<'c, 'f, S> { } } +#[derive(Debug, Clone, Copy)] /// Determines how we should limit rounds of branch and bound. pub enum BnbLimit { /// Limit by round count. diff --git a/src/error.rs b/src/error.rs index c3f9ea15e..164f70f51 100644 --- a/src/error.rs +++ b/src/error.rs @@ -240,3 +240,9 @@ impl From for Error { Error::Esplora(Box::new(other)) } } + +impl From for Error { + fn from(f: crate::bdk_core::SelectionFailure) -> Self { + Error::Generic(format!("bdk_core: {}", f)) + } +} diff --git a/src/wallet/coin_selection.rs b/src/wallet/coin_selection.rs index e711f15b2..f25af1639 100644 --- a/src/wallet/coin_selection.rs +++ b/src/wallet/coin_selection.rs @@ -93,21 +93,16 @@ //! # Ok::<(), bdk::Error>(()) //! ``` -use crate::types::FeeRate; -use crate::wallet::utils::IsDust; +use crate::bdk_core::{self, BnbLimit}; use crate::{database::Database, WeightedUtxo}; use crate::{error::Error, Utxo}; -use bitcoin::consensus::encode::serialize; -use bitcoin::Script; - use rand::seq::SliceRandom; #[cfg(not(test))] use rand::thread_rng; #[cfg(test)] use rand::{rngs::StdRng, SeedableRng}; use std::collections::HashMap; -use std::convert::TryInto; use std::fmt::Debug; /// Default coin selection algorithm used by [`TxBuilder`](super::tx_builder::TxBuilder) if not @@ -119,7 +114,7 @@ pub type DefaultCoinSelectionAlgorithm = LargestFirstCoinSelection; // make the // Base weight of a Txin, not counting the weight needed for satisfying it. // prev_txid (32 bytes) + prev_vout (4 bytes) + sequence (4 bytes) -pub(crate) const TXIN_BASE_WEIGHT: usize = (32 + 4 + 4) * 4; +// pub(crate) const TXIN_BASE_WEIGHT: usize = (32 + 4 + 4) * 4; #[derive(Debug)] /// Remaining amount after performing coin selection @@ -193,12 +188,9 @@ pub trait CoinSelectionAlgorithm: std::fmt::Debug { #[allow(clippy::too_many_arguments)] fn coin_select( &self, - required_utxos: Vec, - optional_utxos: Vec, - fee_rate: FeeRate, - target_amount: u64, - drain_script: &Script, - ) -> Result; + raw_candidates: &Vec, + selector: bdk_core::CoinSelector, + ) -> Result; } /// Simple and dumb coin selection @@ -211,29 +203,33 @@ pub struct LargestFirstCoinSelection; impl CoinSelectionAlgorithm for LargestFirstCoinSelection { fn coin_select( &self, - required_utxos: Vec, - mut optional_utxos: Vec, - fee_rate: FeeRate, - target_amount: u64, - drain_script: &Script, - ) -> Result { + _raw_candidates: &Vec, + mut selector: bdk_core::CoinSelector, + ) -> Result { log::debug!( "target_amount = `{}`, fee_rate = `{:?}`", - target_amount, - fee_rate + selector.opts.target_value.unwrap_or(0), + selector.opts.target_feerate, ); - // 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 - .into_iter() - .map(|utxo| (true, utxo)) - .chain(optional_utxos.into_iter().rev().map(|utxo| (false, utxo))) + let pool = { + let mut pool = selector.unselected().collect::>(); + pool.sort_unstable_by_key(|(_, candidate)| candidate.value); + pool }; - select_sorted_utxos(utxos, fee_rate, target_amount, drain_script) + let mut selection = selector.finish(); + + for (index, _candidate) in pool { + if selection.is_ok() { + return selection.map_err(Error::from); + } + + selector.select(index); + selection = selector.finish(); + } + + selection.map_err(Error::from) } } @@ -262,14 +258,11 @@ impl<'d, D> Debug for OldestFirstCoinSelection<'d, D> { impl<'d, D: Database> CoinSelectionAlgorithm for OldestFirstCoinSelection<'d, D> { fn coin_select( &self, - required_utxos: Vec, - mut optional_utxos: Vec, - fee_rate: FeeRate, - target_amount: u64, - drain_script: &Script, - ) -> Result { + raw_candidates: &Vec, + mut selector: bdk_core::CoinSelector, + ) -> Result { // query db and create a blockheight lookup table - let blockheights = optional_utxos + let blockheights = raw_candidates .iter() .map(|wu| wu.utxo.outpoint().txid) // fold is used so we can skip db query for txid that already exist in hashmap acc @@ -289,121 +282,31 @@ impl<'d, D: Database> CoinSelectionAlgorithm for OldestFirstCoinSelection<'d, D> }) })?; - // 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 blockheights.get(&wu.utxo.outpoint().txid) { - Some(Some(blockheight)) => blockheight, + let pool = { + let mut pool = selector.unselected().collect::>(); + pool.sort_unstable_by_key(|&(index, _candidate)| { + let raw = &raw_candidates[index]; + let txid = &raw.utxo.outpoint().txid; + match blockheights.get(txid) { + Some(Some(height)) => height, _ => &u32::MAX, } }); - - required_utxos - .into_iter() - .map(|utxo| (true, utxo)) - .chain(optional_utxos.into_iter().map(|utxo| (false, utxo))) + pool }; - select_sorted_utxos(utxos, fee_rate, target_amount, drain_script) - } -} - -/// Decide if change can be created -/// -/// - `remaining_amount`: the amount in which the selected coins exceed the target amount -/// - `fee_rate`: required fee rate for the current selection -/// - `drain_script`: script to consider change creation -pub fn decide_change(remaining_amount: u64, fee_rate: FeeRate, drain_script: &Script) -> Excess { - let drain_output_len = serialize(drain_script).len() + 8_usize; - let change_fee = fee_rate.fee_vb(drain_output_len as f32); - let drain_val = remaining_amount.saturating_sub(change_fee); - - if drain_val.is_dust(drain_script) { - let dust_threshold = drain_script.dust_value().as_sat(); - Excess::NoChange { - dust_threshold, - change_fee, - remaining_amount, - } - } else { - Excess::Change { - amount: drain_val, - fee: change_fee, - } - } -} - -fn select_sorted_utxos( - utxos: impl Iterator, - fee_rate: FeeRate, - target_amount: u64, - drain_script: &Script, -) -> Result { - let mut selected_amount = 0; - let mut fee_amount = 0; - let selected = utxos - .scan( - (&mut selected_amount, &mut fee_amount), - |(selected_amount, fee_amount), (must_use, weighted_utxo)| { - if must_use || **selected_amount < target_amount + **fee_amount { - **fee_amount += - fee_rate.fee_wu(TXIN_BASE_WEIGHT + weighted_utxo.satisfaction_weight); - **selected_amount += weighted_utxo.utxo.txout().value; - - log::debug!( - "Selected {}, updated fee_amount = `{}`", - weighted_utxo.utxo.outpoint(), - fee_amount - ); - - Some(weighted_utxo.utxo) - } else { - None - } - }, - ) - .collect::>(); - - let amount_needed_with_fees = target_amount + fee_amount; - if selected_amount < amount_needed_with_fees { - return Err(Error::InsufficientFunds { - needed: amount_needed_with_fees, - available: selected_amount, - }); - } - - let remaining_amount = selected_amount - amount_needed_with_fees; - - let excess = decide_change(remaining_amount, fee_rate, drain_script); - - Ok(CoinSelectionResult { - selected, - fee_amount, - excess, - }) -} + let mut selection = selector.finish(); -#[derive(Debug, Clone)] -// Adds fee information to an UTXO. -struct OutputGroup { - weighted_utxo: WeightedUtxo, - // Amount of fees for spending a certain utxo, calculated using a certain FeeRate - fee: u64, - // The effective value of the UTXO, i.e., the utxo value minus the fee for spending it - effective_value: i64, -} + for (index, _candidate) in pool { + if selection.is_ok() { + return selection.map_err(Error::from); + } -impl OutputGroup { - fn new(weighted_utxo: WeightedUtxo, fee_rate: FeeRate) -> Self { - let fee = fee_rate.fee_wu(TXIN_BASE_WEIGHT + weighted_utxo.satisfaction_weight); - let effective_value = weighted_utxo.utxo.txout().value as i64 - fee as i64; - OutputGroup { - weighted_utxo, - fee, - effective_value, + selector.select(index); + selection = selector.finish(); } + + selection.map_err(Error::from) } } @@ -412,320 +315,63 @@ impl OutputGroup { /// Code adapted from Bitcoin Core's implementation and from Mark Erhardt Master's Thesis: #[derive(Debug)] pub struct BranchAndBoundCoinSelection { - size_of_change: u64, + limit: BnbLimit, } 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, + limit: BnbLimit::Rounds(100_000), } } } impl BranchAndBoundCoinSelection { /// Create new instance with target size for change output - pub fn new(size_of_change: u64) -> Self { - Self { size_of_change } + pub fn new>(limit: L) -> Self { + Self { + limit: limit.into(), + } } } -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: u64, - drain_script: &Script, - ) -> Result { - // Mapping every (UTXO, usize) to an output group - let required_utxos: Vec = required_utxos - .into_iter() - .map(|u| OutputGroup::new(u, 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)) - .filter(|u| u.effective_value.is_positive()) - .collect(); - - let curr_value = required_utxos - .iter() - .fold(0, |acc, x| acc + x.effective_value); - - let curr_available_value = optional_utxos - .iter() - .fold(0, |acc, x| acc + x.effective_value); - - let cost_of_change = self.size_of_change as f32 * fee_rate.as_sat_per_vb(); - - // `curr_value` and `curr_available_value` are both the sum of *effective_values* of - // the UTXOs. For the optional UTXOs (curr_available_value) we filter out UTXOs with - // negative effective value, so it will always be positive. - // - // Since we are required to spend the required UTXOs (curr_value) we have to consider - // all their effective values, even when negative, which means that curr_value could - // be negative as well. - // - // If the sum of curr_value and curr_available_value is negative or lower than our target, - // we can immediately exit with an error, as it's guaranteed we will never find a solution - // if we actually run the BnB. - let total_value: Result = (curr_available_value + curr_value).try_into(); - match total_value { - Ok(v) if v >= target_amount => {} - _ => { - // 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| { - fees += utxo.fee; - value += utxo.weighted_utxo.utxo.txout().value; - - (fees, value) - }); - - // Add to the target the fee cost of the UTXOs - return Err(Error::InsufficientFunds { - needed: target_amount + utxo_fees, - available: utxo_value, - }); - } - } - - let target_amount = target_amount - .try_into() - .expect("Bitcoin amount to fit into i64"); - - if curr_value > 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 excess = decide_change(remaining_amount, fee_rate, drain_script); - - return Ok(BranchAndBoundCoinSelection::calculate_cs_result( - vec![], - required_utxos, - excess, - )); + _raw_candidates: &Vec, + mut selector: bdk_core::CoinSelector, + ) -> Result { + if let Some(final_selector) = bdk_core::coin_select_bnb(self.limit, selector.clone()) { + return final_selector.finish().map_err(Error::from); } - Ok(self - .bnb( - required_utxos.clone(), - optional_utxos.clone(), - curr_value, - curr_available_value, - target_amount, - cost_of_change, - drain_script, - fee_rate, - ) - .unwrap_or_else(|_| { - self.single_random_draw( - required_utxos, - optional_utxos, - curr_value, - target_amount, - drain_script, - fee_rate, - ) - })) - } -} + // FALLBACK: single random draw + let pool = { + let mut pool = selector + .unselected() + .filter(|(_, c)| c.effective_value(selector.opts.target_feerate) > 0) + .collect::>(); -impl BranchAndBoundCoinSelection { - // TODO: make this more Rust-onic :) - // (And perhaps refactor with less arguments?) - #[allow(clippy::too_many_arguments)] - fn bnb( - &self, - required_utxos: Vec, - mut optional_utxos: Vec, - mut curr_value: i64, - mut curr_available_value: i64, - target_amount: i64, - cost_of_change: f32, - drain_script: &Script, - fee_rate: FeeRate, - ) -> 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 - // certain optional_utxos or not. - 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.reverse(); - - // Contains the best selection we found - let mut best_selection = Vec::new(); - let mut best_selection_value = None; - - // Depth First search loop for choosing the UTXOs - for _ in 0..BNB_TOTAL_TRIES { - // Conditions for starting a backtrack - let mut backtrack = false; - // Cannot possibly reach target with the amount remaining in the curr_available_value, - // or the selected value is out of range. - // Go back and try other branch - if curr_value + curr_available_value < target_amount - || curr_value > target_amount + cost_of_change as i64 - { - backtrack = true; - } else if curr_value >= target_amount { - // Selected value is within range, there's no point in going forward. Start - // backtracking - backtrack = true; - - // If we found a solution better than the previous one, or if there wasn't previous - // solution, update the best solution - if best_selection_value.is_none() || curr_value < best_selection_value.unwrap() { - best_selection = current_selection.clone(); - best_selection_value = Some(curr_value); - } + #[cfg(not(test))] + pool.shuffle(&mut thread_rng()); + #[cfg(test)] + pool.shuffle(&mut StdRng::from_seed([0; 32])); - // If we found a perfect match, break here - if curr_value == target_amount { - break; - } - } - - // Backtracking, moving backwards - if backtrack { - // 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; - } - - if current_selection.last_mut().is_none() { - // 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); - } - break; - } - - if let Some(c) = current_selection.last_mut() { - // Output was included on previous iterations, try excluding now. - *c = false; - } - - let utxo = &optional_utxos[current_selection.len() - 1]; - curr_value -= utxo.effective_value; - } else { - // Moving forwards, continuing down this branch - let utxo = &optional_utxos[current_selection.len()]; + pool + }; - // Remove this utxo from the curr_available_value utxo amount - curr_available_value -= utxo.effective_value; + let mut selection = selector.finish(); - // Inclusion branch first (Largest First Exploration) - current_selection.push(true); - curr_value += utxo.effective_value; + for (index, _candidate) in pool { + if selection.is_ok() { + return selection.map_err(Error::from); } - } - // Check for solution - if best_selection.is_empty() { - return Err(Error::BnBTotalTriesExceeded); + selector.select(index); + selection = selector.finish(); } - // Set output set - let selected_utxos = optional_utxos - .into_iter() - .zip(best_selection) - .filter_map(|(optional, is_in_best)| if is_in_best { Some(optional) } else { None }) - .collect::>(); - - let selected_amount = best_selection_value.unwrap(); - - // 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 = (selected_amount - target_amount) as u64; - - let excess = decide_change(remaining_amount, fee_rate, drain_script); - - Ok(BranchAndBoundCoinSelection::calculate_cs_result( - selected_utxos, - required_utxos, - excess, - )) - } - - #[allow(clippy::too_many_arguments)] - fn single_random_draw( - &self, - required_utxos: Vec, - mut optional_utxos: Vec, - curr_value: i64, - target_amount: i64, - drain_script: &Script, - fee_rate: FeeRate, - ) -> CoinSelectionResult { - #[cfg(not(test))] - optional_utxos.shuffle(&mut thread_rng()); - #[cfg(test)] - { - let seed = [0; 32]; - let mut rng: StdRng = SeedableRng::from_seed(seed); - optional_utxos.shuffle(&mut rng); - } - - let selected_utxos = optional_utxos.into_iter().fold( - (curr_value, vec![]), - |(mut amount, mut utxos), utxo| { - if amount >= target_amount { - (amount, utxos) - } else { - amount += utxo.effective_value; - utxos.push(utxo); - (amount, utxos) - } - }, - ); - - // 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 = (selected_utxos.0 - target_amount) as u64; - - let excess = decide_change(remaining_amount, fee_rate, drain_script); - - BranchAndBoundCoinSelection::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, - } + selection.map_err(Error::from) } } @@ -736,13 +382,13 @@ mod test { use bitcoin::{OutPoint, Script, TxOut}; use super::*; - use crate::database::{BatchOperations, MemoryDatabase}; + // use crate::database::{BatchOperations, MemoryDatabase}; use crate::types::*; - use crate::wallet::WeightUnits; + // use crate::wallet::WeightUnits; use rand::rngs::StdRng; use rand::seq::SliceRandom; - use rand::{Rng, SeedableRng}; + use rand::Rng; // n. of items on witness (1WU) + signature len (1WU) + signature and sighash (72WU) // + pubkey len (1WU) + pubkey (33WU) + script sig len (1 byte, 4WU) @@ -883,689 +529,687 @@ mod test { .map(|u| u.utxo.txout().value) .sum() } - - #[test] - fn test_largest_first_coin_selection_success() { - let utxos = get_test_utxos(); - let drain_script = Script::default(); - let target_amount = 250_000 + FEE_AMOUNT; - - let result = LargestFirstCoinSelection::default() - .coin_select( - utxos, - vec![], - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - - assert_eq!(result.selected.len(), 3); - assert_eq!(result.selected_amount(), 300_010); - assert_eq!(result.fee_amount, 204) - } - - #[test] - fn test_largest_first_coin_selection_use_all() { - let utxos = get_test_utxos(); - let drain_script = Script::default(); - let target_amount = 20_000 + FEE_AMOUNT; - - let result = LargestFirstCoinSelection::default() - .coin_select( - utxos, - vec![], - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - - assert_eq!(result.selected.len(), 3); - assert_eq!(result.selected_amount(), 300_010); - assert_eq!(result.fee_amount, 204); - } - - #[test] - fn test_largest_first_coin_selection_use_only_necessary() { - let utxos = get_test_utxos(); - let drain_script = Script::default(); - let target_amount = 20_000 + FEE_AMOUNT; - - let result = LargestFirstCoinSelection::default() - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - - assert_eq!(result.selected.len(), 1); - assert_eq!(result.selected_amount(), 200_000); - assert_eq!(result.fee_amount, 68); - } - - #[test] - #[should_panic(expected = "InsufficientFunds")] - fn test_largest_first_coin_selection_insufficient_funds() { - let utxos = get_test_utxos(); - let drain_script = Script::default(); - let target_amount = 500_000 + FEE_AMOUNT; - - LargestFirstCoinSelection::default() - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - } - - #[test] - #[should_panic(expected = "InsufficientFunds")] - fn test_largest_first_coin_selection_insufficient_funds_high_fees() { - let utxos = get_test_utxos(); - let drain_script = Script::default(); - let target_amount = 250_000 + FEE_AMOUNT; - - LargestFirstCoinSelection::default() - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb(1000.0), - target_amount, - &drain_script, - ) - .unwrap(); - } - - #[test] - fn test_oldest_first_coin_selection_success() { - let mut database = MemoryDatabase::default(); - let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); - let drain_script = Script::default(); - let target_amount = 180_000 + FEE_AMOUNT; - - let result = OldestFirstCoinSelection::new(&database) - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - - assert_eq!(result.selected.len(), 2); - assert_eq!(result.selected_amount(), 200_000); - assert_eq!(result.fee_amount, 136) - } - - #[test] - fn test_oldest_first_coin_selection_utxo_not_in_db_will_be_selected_last() { - // ensure utxos are from different tx - let utxo1 = utxo(120_000, 1); - let utxo2 = utxo(80_000, 2); - let utxo3 = utxo(300_000, 3); - let drain_script = Script::default(); - - let mut database = MemoryDatabase::default(); - - // add tx to DB so utxos are sorted by blocktime asc - // utxos will be selected by the following order - // utxo1(blockheight 1) -> utxo2(blockheight 2), utxo3 (not exist in DB) - // timestamp are all set as the same to ensure that only block height is used in sorting - let utxo1_tx_details = TransactionDetails { - transaction: None, - txid: utxo1.utxo.outpoint().txid, - received: 1, - sent: 0, - fee: None, - confirmation_time: Some(BlockTime { - height: 1, - timestamp: 1231006505, - }), - }; - - let utxo2_tx_details = TransactionDetails { - transaction: None, - txid: utxo2.utxo.outpoint().txid, - received: 1, - sent: 0, - fee: None, - confirmation_time: Some(BlockTime { - height: 2, - timestamp: 1231006505, - }), - }; - - database.set_tx(&utxo1_tx_details).unwrap(); - database.set_tx(&utxo2_tx_details).unwrap(); - - let target_amount = 180_000 + FEE_AMOUNT; - - let result = OldestFirstCoinSelection::new(&database) - .coin_select( - vec![], - vec![utxo3, utxo1, utxo2], - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - - assert_eq!(result.selected.len(), 2); - assert_eq!(result.selected_amount(), 200_000); - assert_eq!(result.fee_amount, 136) - } - - #[test] - fn test_oldest_first_coin_selection_use_all() { - let mut database = MemoryDatabase::default(); - let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); - let drain_script = Script::default(); - let target_amount = 20_000 + FEE_AMOUNT; - - let result = OldestFirstCoinSelection::new(&database) - .coin_select( - utxos, - vec![], - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - - assert_eq!(result.selected.len(), 3); - assert_eq!(result.selected_amount(), 500_000); - assert_eq!(result.fee_amount, 204); - } - - #[test] - fn test_oldest_first_coin_selection_use_only_necessary() { - let mut database = MemoryDatabase::default(); - let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); - let drain_script = Script::default(); - let target_amount = 20_000 + FEE_AMOUNT; - - let result = OldestFirstCoinSelection::new(&database) - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - - assert_eq!(result.selected.len(), 1); - assert_eq!(result.selected_amount(), 120_000); - assert_eq!(result.fee_amount, 68); - } - - #[test] - #[should_panic(expected = "InsufficientFunds")] - fn test_oldest_first_coin_selection_insufficient_funds() { - let mut database = MemoryDatabase::default(); - let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); - let drain_script = Script::default(); - let target_amount = 600_000 + FEE_AMOUNT; - - OldestFirstCoinSelection::new(&database) - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - } - - #[test] - #[should_panic(expected = "InsufficientFunds")] - fn test_oldest_first_coin_selection_insufficient_funds_high_fees() { - let mut database = MemoryDatabase::default(); - let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); - - let target_amount: u64 = utxos.iter().map(|wu| wu.utxo.txout().value).sum::() - 50; - let drain_script = Script::default(); - - OldestFirstCoinSelection::new(&database) - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb(1000.0), - target_amount, - &drain_script, - ) - .unwrap(); - } - - #[test] - fn test_bnb_coin_selection_success() { - // In this case bnb won't find a suitable match and single random draw will - // select three outputs - let utxos = generate_same_value_utxos(100_000, 20); - - let drain_script = Script::default(); - - let target_amount = 250_000 + FEE_AMOUNT; - - let result = BranchAndBoundCoinSelection::default() - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - - assert_eq!(result.selected.len(), 3); - assert_eq!(result.selected_amount(), 300_000); - assert_eq!(result.fee_amount, 204); - } - - #[test] - fn test_bnb_coin_selection_required_are_enough() { - let utxos = get_test_utxos(); - let drain_script = Script::default(); - let target_amount = 20_000 + FEE_AMOUNT; - - let result = BranchAndBoundCoinSelection::default() - .coin_select( - utxos.clone(), - utxos, - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - - assert_eq!(result.selected.len(), 3); - assert_eq!(result.selected_amount(), 300_010); - assert_eq!(result.fee_amount, 204); - } - - #[test] - fn test_bnb_coin_selection_optional_are_enough() { - let utxos = get_test_utxos(); - let drain_script = Script::default(); - let target_amount = 299756 + FEE_AMOUNT; - - let result = BranchAndBoundCoinSelection::default() - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - - assert_eq!(result.selected.len(), 2); - assert_eq!(result.selected_amount(), 300000); - assert_eq!(result.fee_amount, 136); - } - - #[test] - fn test_bnb_coin_selection_required_not_enough() { - let utxos = get_test_utxos(); - - let required = vec![utxos[0].clone()]; - let mut optional = utxos[1..].to_vec(); - optional.push(utxo(500_000, 3)); - - // Defensive assertions, for sanity and in case someone changes the test utxos vector. - let amount: u64 = required.iter().map(|u| u.utxo.txout().value).sum(); - assert_eq!(amount, 100_000); - let amount: u64 = optional.iter().map(|u| u.utxo.txout().value).sum(); - assert!(amount > 150_000); - let drain_script = Script::default(); - - let target_amount = 150_000 + FEE_AMOUNT; - - let result = BranchAndBoundCoinSelection::default() - .coin_select( - required, - optional, - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - - assert_eq!(result.selected.len(), 2); - assert_eq!(result.selected_amount(), 300_000); - assert_eq!(result.fee_amount, 136); - } - - #[test] - #[should_panic(expected = "InsufficientFunds")] - fn test_bnb_coin_selection_insufficient_funds() { - let utxos = get_test_utxos(); - let drain_script = Script::default(); - let target_amount = 500_000 + FEE_AMOUNT; - - BranchAndBoundCoinSelection::default() - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - } - - #[test] - #[should_panic(expected = "InsufficientFunds")] - fn test_bnb_coin_selection_insufficient_funds_high_fees() { - let utxos = get_test_utxos(); - let drain_script = Script::default(); - let target_amount = 250_000 + FEE_AMOUNT; - - BranchAndBoundCoinSelection::default() - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb(1000.0), - target_amount, - &drain_script, - ) - .unwrap(); - } - - #[test] - fn test_bnb_coin_selection_check_fee_rate() { - let utxos = get_test_utxos(); - let drain_script = Script::default(); - let target_amount = 99932; // first utxo's effective value - - let result = BranchAndBoundCoinSelection::new(0) - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb(1.0), - target_amount, - &drain_script, - ) - .unwrap(); - - assert_eq!(result.selected.len(), 1); - assert_eq!(result.selected_amount(), 100_000); - let input_size = (TXIN_BASE_WEIGHT + P2WPKH_SATISFACTION_SIZE).to_vbytes(); - // the final fee rate should be exactly the same as the fee rate given - assert!((1.0 - (result.fee_amount as f32 / input_size as f32)).abs() < f32::EPSILON); - } - - #[test] - fn test_bnb_coin_selection_exact_match() { - let seed = [0; 32]; - let mut rng: StdRng = SeedableRng::from_seed(seed); - - for _i in 0..200 { - let mut optional_utxos = generate_random_utxos(&mut rng, 16); - let target_amount = sum_random_utxos(&mut rng, &mut optional_utxos); - let drain_script = Script::default(); - let result = BranchAndBoundCoinSelection::new(0) - .coin_select( - vec![], - optional_utxos, - FeeRate::from_sat_per_vb(0.0), - target_amount, - &drain_script, - ) - .unwrap(); - assert_eq!(result.selected_amount(), target_amount); - } - } - - #[test] - #[should_panic(expected = "BnBNoExactMatch")] - fn test_bnb_function_no_exact_match() { - let fee_rate = FeeRate::from_sat_per_vb(10.0); - let utxos: Vec = get_test_utxos() - .into_iter() - .map(|u| OutputGroup::new(u, fee_rate)) - .collect(); - - let curr_available_value = utxos.iter().fold(0, |acc, x| acc + x.effective_value); - - let size_of_change = 31; - let cost_of_change = size_of_change as f32 * fee_rate.as_sat_per_vb(); - - let drain_script = Script::default(); - let target_amount = 20_000 + FEE_AMOUNT; - BranchAndBoundCoinSelection::new(size_of_change) - .bnb( - vec![], - utxos, - 0, - curr_available_value, - target_amount as i64, - cost_of_change, - &drain_script, - fee_rate, - ) - .unwrap(); - } - - #[test] - #[should_panic(expected = "BnBTotalTriesExceeded")] - fn test_bnb_function_tries_exceeded() { - let fee_rate = FeeRate::from_sat_per_vb(10.0); - let utxos: Vec = generate_same_value_utxos(100_000, 100_000) - .into_iter() - .map(|u| OutputGroup::new(u, fee_rate)) - .collect(); - - let curr_available_value = utxos.iter().fold(0, |acc, x| acc + x.effective_value); - - let size_of_change = 31; - let cost_of_change = size_of_change as f32 * fee_rate.as_sat_per_vb(); - let target_amount = 20_000 + FEE_AMOUNT; - - let drain_script = Script::default(); - - BranchAndBoundCoinSelection::new(size_of_change) - .bnb( - vec![], - utxos, - 0, - curr_available_value, - target_amount as i64, - cost_of_change, - &drain_script, - fee_rate, - ) - .unwrap(); - } - - // The match won't be exact but still in the range - #[test] - fn test_bnb_function_almost_exact_match_with_fees() { - let fee_rate = FeeRate::from_sat_per_vb(1.0); - let size_of_change = 31; - let cost_of_change = size_of_change as f32 * fee_rate.as_sat_per_vb(); - - let utxos: Vec<_> = generate_same_value_utxos(50_000, 10) - .into_iter() - .map(|u| OutputGroup::new(u, fee_rate)) - .collect(); - - let curr_value = 0; - - let curr_available_value = utxos.iter().fold(0, |acc, x| acc + x.effective_value); - - // 2*(value of 1 utxo) - 2*(1 utxo fees with 1.0sat/vbyte fee rate) - - // cost_of_change + 5. - let target_amount = 2 * 50_000 - 2 * 67 - cost_of_change.ceil() as i64 + 5; - - let drain_script = Script::default(); - - let result = BranchAndBoundCoinSelection::new(size_of_change) - .bnb( - vec![], - utxos, - curr_value, - curr_available_value, - target_amount, - cost_of_change, - &drain_script, - fee_rate, - ) - .unwrap(); - assert_eq!(result.selected_amount(), 100_000); - assert_eq!(result.fee_amount, 136); - } - - // TODO: bnb() function should be optimized, and this test should be done with more utxos - #[test] - fn test_bnb_function_exact_match_more_utxos() { - let seed = [0; 32]; - let mut rng: StdRng = SeedableRng::from_seed(seed); - let fee_rate = FeeRate::from_sat_per_vb(0.0); - - for _ in 0..200 { - let optional_utxos: Vec<_> = generate_random_utxos(&mut rng, 40) - .into_iter() - .map(|u| OutputGroup::new(u, fee_rate)) - .collect(); - - let curr_value = 0; - - let curr_available_value = optional_utxos - .iter() - .fold(0, |acc, x| acc + x.effective_value); - - let target_amount = - optional_utxos[3].effective_value + optional_utxos[23].effective_value; - - let drain_script = Script::default(); - - let result = BranchAndBoundCoinSelection::new(0) - .bnb( - vec![], - optional_utxos, - curr_value, - curr_available_value, - target_amount, - 0.0, - &drain_script, - fee_rate, - ) - .unwrap(); - assert_eq!(result.selected_amount(), target_amount as u64); - } - } - - #[test] - fn test_single_random_draw_function_success() { - let seed = [0; 32]; - let mut rng: StdRng = SeedableRng::from_seed(seed); - let mut utxos = generate_random_utxos(&mut rng, 300); - let target_amount = sum_random_utxos(&mut rng, &mut utxos) + FEE_AMOUNT; - - let fee_rate = FeeRate::from_sat_per_vb(1.0); - let utxos: Vec = utxos - .into_iter() - .map(|u| OutputGroup::new(u, fee_rate)) - .collect(); - - let drain_script = Script::default(); - - let result = BranchAndBoundCoinSelection::default().single_random_draw( - vec![], - utxos, - 0, - target_amount as i64, - &drain_script, - fee_rate, - ); - - assert!(result.selected_amount() > target_amount); - assert_eq!(result.fee_amount, (result.selected.len() * 68) as u64); - } - - #[test] - fn test_bnb_exclude_negative_effective_value() { - let utxos = get_test_utxos(); - let drain_script = Script::default(); - - let err = BranchAndBoundCoinSelection::default() - .coin_select( - vec![], - utxos, - FeeRate::from_sat_per_vb(10.0), - 500_000, - &drain_script, - ) - .unwrap_err(); - - assert!(matches!( - err, - Error::InsufficientFunds { - available: 300_000, - .. - } - )); - } - - #[test] - fn test_bnb_include_negative_effective_value_when_required() { - let utxos = get_test_utxos(); - let drain_script = Script::default(); - - let (required, optional) = utxos - .into_iter() - .partition(|u| matches!(u, WeightedUtxo { utxo, .. } if utxo.txout().value < 1000)); - - let err = BranchAndBoundCoinSelection::default() - .coin_select( - required, - optional, - FeeRate::from_sat_per_vb(10.0), - 500_000, - &drain_script, - ) - .unwrap_err(); - - assert!(matches!( - err, - Error::InsufficientFunds { - available: 300_010, - .. - } - )); - } - - #[test] - fn test_bnb_sum_of_effective_value_negative() { - let utxos = get_test_utxos(); - let drain_script = Script::default(); - - let err = BranchAndBoundCoinSelection::default() - .coin_select( - utxos, - vec![], - FeeRate::from_sat_per_vb(10_000.0), - 500_000, - &drain_script, - ) - .unwrap_err(); - - assert!(matches!( - err, - Error::InsufficientFunds { - available: 300_010, - .. - } - )); - } + // fn test_largest_first_coin_selection_success() { + // let utxos = get_test_utxos(); + // let drain_script = Script::default(); + // let target_amount = 250_000 + FEE_AMOUNT; + + // let result = LargestFirstCoinSelection::default() + // .coin_select( + // utxos, + // vec![], + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + + // assert_eq!(result.selected.len(), 3); + // assert_eq!(result.selected_amount(), 300_010); + // assert_eq!(result.fee_amount, 204) + // } + + // #[test] + // fn test_largest_first_coin_selection_use_all() { + // let utxos = get_test_utxos(); + // let drain_script = Script::default(); + // let target_amount = 20_000 + FEE_AMOUNT; + + // let result = LargestFirstCoinSelection::default() + // .coin_select( + // utxos, + // vec![], + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + + // assert_eq!(result.selected.len(), 3); + // assert_eq!(result.selected_amount(), 300_010); + // assert_eq!(result.fee_amount, 204); + // } + + // #[test] + // fn test_largest_first_coin_selection_use_only_necessary() { + // let utxos = get_test_utxos(); + // let drain_script = Script::default(); + // let target_amount = 20_000 + FEE_AMOUNT; + + // let result = LargestFirstCoinSelection::default() + // .coin_select( + // vec![], + // utxos, + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + + // assert_eq!(result.selected.len(), 1); + // assert_eq!(result.selected_amount(), 200_000); + // assert_eq!(result.fee_amount, 68); + // } + + // #[test] + // #[should_panic(expected = "InsufficientFunds")] + // fn test_largest_first_coin_selection_insufficient_funds() { + // let utxos = get_test_utxos(); + // let drain_script = Script::default(); + // let target_amount = 500_000 + FEE_AMOUNT; + + // LargestFirstCoinSelection::default() + // .coin_select( + // vec![], + // utxos, + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + // } + + // #[test] + // #[should_panic(expected = "InsufficientFunds")] + // fn test_largest_first_coin_selection_insufficient_funds_high_fees() { + // let utxos = get_test_utxos(); + // let drain_script = Script::default(); + // let target_amount = 250_000 + FEE_AMOUNT; + + // LargestFirstCoinSelection::default() + // .coin_select( + // vec![], + // utxos, + // FeeRate::from_sat_per_vb(1000.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + // } + + // #[test] + // fn test_oldest_first_coin_selection_success() { + // let mut database = MemoryDatabase::default(); + // let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); + // let drain_script = Script::default(); + // let target_amount = 180_000 + FEE_AMOUNT; + + // let result = OldestFirstCoinSelection::new(&database) + // .coin_select( + // vec![], + // utxos, + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + + // assert_eq!(result.selected.len(), 2); + // assert_eq!(result.selected_amount(), 200_000); + // assert_eq!(result.fee_amount, 136) + // } + + // #[test] + // fn test_oldest_first_coin_selection_utxo_not_in_db_will_be_selected_last() { + // // ensure utxos are from different tx + // let utxo1 = utxo(120_000, 1); + // let utxo2 = utxo(80_000, 2); + // let utxo3 = utxo(300_000, 3); + // let drain_script = Script::default(); + + // let mut database = MemoryDatabase::default(); + + // // add tx to DB so utxos are sorted by blocktime asc + // // utxos will be selected by the following order + // // utxo1(blockheight 1) -> utxo2(blockheight 2), utxo3 (not exist in DB) + // // timestamp are all set as the same to ensure that only block height is used in sorting + // let utxo1_tx_details = TransactionDetails { + // transaction: None, + // txid: utxo1.utxo.outpoint().txid, + // received: 1, + // sent: 0, + // fee: None, + // confirmation_time: Some(BlockTime { + // height: 1, + // timestamp: 1231006505, + // }), + // }; + + // let utxo2_tx_details = TransactionDetails { + // transaction: None, + // txid: utxo2.utxo.outpoint().txid, + // received: 1, + // sent: 0, + // fee: None, + // confirmation_time: Some(BlockTime { + // height: 2, + // timestamp: 1231006505, + // }), + // }; + + // database.set_tx(&utxo1_tx_details).unwrap(); + // database.set_tx(&utxo2_tx_details).unwrap(); + + // let target_amount = 180_000 + FEE_AMOUNT; + + // let result = OldestFirstCoinSelection::new(&database) + // .coin_select( + // vec![], + // vec![utxo3, utxo1, utxo2], + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + + // assert_eq!(result.selected.len(), 2); + // assert_eq!(result.selected_amount(), 200_000); + // assert_eq!(result.fee_amount, 136) + // } + + // #[test] + // fn test_oldest_first_coin_selection_use_all() { + // let mut database = MemoryDatabase::default(); + // let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); + // let drain_script = Script::default(); + // let target_amount = 20_000 + FEE_AMOUNT; + + // let result = OldestFirstCoinSelection::new(&database) + // .coin_select( + // utxos, + // vec![], + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + + // assert_eq!(result.selected.len(), 3); + // assert_eq!(result.selected_amount(), 500_000); + // assert_eq!(result.fee_amount, 204); + // } + + // #[test] + // fn test_oldest_first_coin_selection_use_only_necessary() { + // let mut database = MemoryDatabase::default(); + // let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); + // let drain_script = Script::default(); + // let target_amount = 20_000 + FEE_AMOUNT; + + // let result = OldestFirstCoinSelection::new(&database) + // .coin_select( + // vec![], + // utxos, + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + + // assert_eq!(result.selected.len(), 1); + // assert_eq!(result.selected_amount(), 120_000); + // assert_eq!(result.fee_amount, 68); + // } + + // #[test] + // #[should_panic(expected = "InsufficientFunds")] + // fn test_oldest_first_coin_selection_insufficient_funds() { + // let mut database = MemoryDatabase::default(); + // let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); + // let drain_script = Script::default(); + // let target_amount = 600_000 + FEE_AMOUNT; + + // OldestFirstCoinSelection::new(&database) + // .coin_select( + // vec![], + // utxos, + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + // } + + // #[test] + // #[should_panic(expected = "InsufficientFunds")] + // fn test_oldest_first_coin_selection_insufficient_funds_high_fees() { + // let mut database = MemoryDatabase::default(); + // let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); + + // let target_amount: u64 = utxos.iter().map(|wu| wu.utxo.txout().value).sum::() - 50; + // let drain_script = Script::default(); + + // OldestFirstCoinSelection::new(&database) + // .coin_select( + // vec![], + // utxos, + // FeeRate::from_sat_per_vb(1000.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + // } + + // #[test] + // fn test_bnb_coin_selection_success() { + // // In this case bnb won't find a suitable match and single random draw will + // // select three outputs + // let utxos = generate_same_value_utxos(100_000, 20); + + // let drain_script = Script::default(); + + // let target_amount = 250_000 + FEE_AMOUNT; + + // let result = BranchAndBoundCoinSelection::default() + // .coin_select( + // vec![], + // utxos, + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + + // assert_eq!(result.selected.len(), 3); + // assert_eq!(result.selected_amount(), 300_000); + // assert_eq!(result.fee_amount, 204); + // } + + // #[test] + // fn test_bnb_coin_selection_required_are_enough() { + // let utxos = get_test_utxos(); + // let drain_script = Script::default(); + // let target_amount = 20_000 + FEE_AMOUNT; + + // let result = BranchAndBoundCoinSelection::default() + // .coin_select( + // utxos.clone(), + // utxos, + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + + // assert_eq!(result.selected.len(), 3); + // assert_eq!(result.selected_amount(), 300_010); + // assert_eq!(result.fee_amount, 204); + // } + + // #[test] + // fn test_bnb_coin_selection_optional_are_enough() { + // let utxos = get_test_utxos(); + // let drain_script = Script::default(); + // let target_amount = 299756 + FEE_AMOUNT; + + // let result = BranchAndBoundCoinSelection::default() + // .coin_select( + // vec![], + // utxos, + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + + // assert_eq!(result.selected.len(), 2); + // assert_eq!(result.selected_amount(), 300000); + // assert_eq!(result.fee_amount, 136); + // } + + // #[test] + // fn test_bnb_coin_selection_required_not_enough() { + // let utxos = get_test_utxos(); + + // let required = vec![utxos[0].clone()]; + // let mut optional = utxos[1..].to_vec(); + // optional.push(utxo(500_000, 3)); + + // // Defensive assertions, for sanity and in case someone changes the test utxos vector. + // let amount: u64 = required.iter().map(|u| u.utxo.txout().value).sum(); + // assert_eq!(amount, 100_000); + // let amount: u64 = optional.iter().map(|u| u.utxo.txout().value).sum(); + // assert!(amount > 150_000); + // let drain_script = Script::default(); + + // let target_amount = 150_000 + FEE_AMOUNT; + + // let result = BranchAndBoundCoinSelection::default() + // .coin_select( + // required, + // optional, + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + + // assert_eq!(result.selected.len(), 2); + // assert_eq!(result.selected_amount(), 300_000); + // assert_eq!(result.fee_amount, 136); + // } + + // #[test] + // #[should_panic(expected = "InsufficientFunds")] + // fn test_bnb_coin_selection_insufficient_funds() { + // let utxos = get_test_utxos(); + // let drain_script = Script::default(); + // let target_amount = 500_000 + FEE_AMOUNT; + + // BranchAndBoundCoinSelection::default() + // .coin_select( + // vec![], + // utxos, + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + // } + + // #[test] + // #[should_panic(expected = "InsufficientFunds")] + // fn test_bnb_coin_selection_insufficient_funds_high_fees() { + // let utxos = get_test_utxos(); + // let drain_script = Script::default(); + // let target_amount = 250_000 + FEE_AMOUNT; + + // BranchAndBoundCoinSelection::default() + // .coin_select( + // vec![], + // utxos, + // FeeRate::from_sat_per_vb(1000.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + // } + + // #[test] + // fn test_bnb_coin_selection_check_fee_rate() { + // let utxos = get_test_utxos(); + // let drain_script = Script::default(); + // let target_amount = 99932; // first utxo's effective value + + // let result = BranchAndBoundCoinSelection::new(0) + // .coin_select( + // vec![], + // utxos, + // FeeRate::from_sat_per_vb(1.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + + // assert_eq!(result.selected.len(), 1); + // assert_eq!(result.selected_amount(), 100_000); + // let input_size = (TXIN_BASE_WEIGHT + P2WPKH_SATISFACTION_SIZE).vbytes(); + // // the final fee rate should be exactly the same as the fee rate given + // assert!((1.0 - (result.fee_amount as f32 / input_size as f32)).abs() < f32::EPSILON); + // } + + // #[test] + // fn test_bnb_coin_selection_exact_match() { + // let seed = [0; 32]; + // let mut rng: StdRng = SeedableRng::from_seed(seed); + + // for _i in 0..200 { + // let mut optional_utxos = generate_random_utxos(&mut rng, 16); + // let target_amount = sum_random_utxos(&mut rng, &mut optional_utxos); + // let drain_script = Script::default(); + // let result = BranchAndBoundCoinSelection::new(0) + // .coin_select( + // vec![], + // optional_utxos, + // FeeRate::from_sat_per_vb(0.0), + // target_amount, + // &drain_script, + // ) + // .unwrap(); + // assert_eq!(result.selected_amount(), target_amount); + // } + // } + + // #[test] + // #[should_panic(expected = "BnBNoExactMatch")] + // fn test_bnb_function_no_exact_match() { + // let fee_rate = FeeRate::from_sat_per_vb(10.0); + // let utxos: Vec = get_test_utxos() + // .into_iter() + // .map(|u| OutputGroup::new(u, fee_rate)) + // .collect(); + + // let curr_available_value = utxos.iter().fold(0, |acc, x| acc + x.effective_value); + + // let size_of_change = 31; + // let cost_of_change = size_of_change as f32 * fee_rate.as_sat_per_vb(); + + // let drain_script = Script::default(); + // let target_amount = 20_000 + FEE_AMOUNT; + // BranchAndBoundCoinSelection::new(size_of_change) + // .bnb( + // vec![], + // utxos, + // 0, + // curr_available_value, + // target_amount as i64, + // cost_of_change, + // &drain_script, + // fee_rate, + // ) + // .unwrap(); + // } + + // #[test] + // #[should_panic(expected = "BnBTotalTriesExceeded")] + // fn test_bnb_function_tries_exceeded() { + // let fee_rate = FeeRate::from_sat_per_vb(10.0); + // let utxos: Vec = generate_same_value_utxos(100_000, 100_000) + // .into_iter() + // .map(|u| OutputGroup::new(u, fee_rate)) + // .collect(); + + // let curr_available_value = utxos.iter().fold(0, |acc, x| acc + x.effective_value); + + // let size_of_change = 31; + // let cost_of_change = size_of_change as f32 * fee_rate.as_sat_per_vb(); + // let target_amount = 20_000 + FEE_AMOUNT; + + // let drain_script = Script::default(); + + // BranchAndBoundCoinSelection::new(size_of_change) + // .bnb( + // vec![], + // utxos, + // 0, + // curr_available_value, + // target_amount as i64, + // cost_of_change, + // &drain_script, + // fee_rate, + // ) + // .unwrap(); + // } + + // // The match won't be exact but still in the range + // #[test] + // fn test_bnb_function_almost_exact_match_with_fees() { + // let fee_rate = FeeRate::from_sat_per_vb(1.0); + // let size_of_change = 31; + // let cost_of_change = size_of_change as f32 * fee_rate.as_sat_per_vb(); + + // let utxos: Vec<_> = generate_same_value_utxos(50_000, 10) + // .into_iter() + // .map(|u| OutputGroup::new(u, fee_rate)) + // .collect(); + + // let curr_value = 0; + + // let curr_available_value = utxos.iter().fold(0, |acc, x| acc + x.effective_value); + + // // 2*(value of 1 utxo) - 2*(1 utxo fees with 1.0sat/vbyte fee rate) - + // // cost_of_change + 5. + // let target_amount = 2 * 50_000 - 2 * 67 - cost_of_change.ceil() as i64 + 5; + + // let drain_script = Script::default(); + + // let result = BranchAndBoundCoinSelection::new(size_of_change) + // .bnb( + // vec![], + // utxos, + // curr_value, + // curr_available_value, + // target_amount, + // cost_of_change, + // &drain_script, + // fee_rate, + // ) + // .unwrap(); + // assert_eq!(result.selected_amount(), 100_000); + // assert_eq!(result.fee_amount, 136); + // } + + // // TODO: bnb() function should be optimized, and this test should be done with more utxos + // #[test] + // fn test_bnb_function_exact_match_more_utxos() { + // let seed = [0; 32]; + // let mut rng: StdRng = SeedableRng::from_seed(seed); + // let fee_rate = FeeRate::from_sat_per_vb(0.0); + + // for _ in 0..200 { + // let optional_utxos: Vec<_> = generate_random_utxos(&mut rng, 40) + // .into_iter() + // .map(|u| OutputGroup::new(u, fee_rate)) + // .collect(); + + // let curr_value = 0; + + // let curr_available_value = optional_utxos + // .iter() + // .fold(0, |acc, x| acc + x.effective_value); + + // let target_amount = + // optional_utxos[3].effective_value + optional_utxos[23].effective_value; + + // let drain_script = Script::default(); + + // let result = BranchAndBoundCoinSelection::new(0) + // .bnb( + // vec![], + // optional_utxos, + // curr_value, + // curr_available_value, + // target_amount, + // 0.0, + // &drain_script, + // fee_rate, + // ) + // .unwrap(); + // assert_eq!(result.selected_amount(), target_amount as u64); + // } + // } + + // #[test] + // fn test_single_random_draw_function_success() { + // let seed = [0; 32]; + // let mut rng: StdRng = SeedableRng::from_seed(seed); + // let mut utxos = generate_random_utxos(&mut rng, 300); + // let target_amount = sum_random_utxos(&mut rng, &mut utxos) + FEE_AMOUNT; + + // let fee_rate = FeeRate::from_sat_per_vb(1.0); + // let utxos: Vec = utxos + // .into_iter() + // .map(|u| OutputGroup::new(u, fee_rate)) + // .collect(); + + // let drain_script = Script::default(); + + // let result = BranchAndBoundCoinSelection::default().single_random_draw( + // vec![], + // utxos, + // 0, + // target_amount as i64, + // &drain_script, + // fee_rate, + // ); + + // assert!(result.selected_amount() > target_amount); + // assert_eq!(result.fee_amount, (result.selected.len() * 68) as u64); + // } + + // #[test] + // fn test_bnb_exclude_negative_effective_value() { + // let utxos = get_test_utxos(); + // let drain_script = Script::default(); + + // let err = BranchAndBoundCoinSelection::default() + // .coin_select( + // vec![], + // utxos, + // FeeRate::from_sat_per_vb(10.0), + // 500_000, + // &drain_script, + // ) + // .unwrap_err(); + + // assert!(matches!( + // err, + // Error::InsufficientFunds { + // available: 300_000, + // .. + // } + // )); + // } + + // #[test] + // fn test_bnb_include_negative_effective_value_when_required() { + // let utxos = get_test_utxos(); + // let drain_script = Script::default(); + + // let (required, optional) = utxos + // .into_iter() + // .partition(|u| matches!(u, WeightedUtxo { utxo, .. } if utxo.txout().value < 1000)); + + // let err = BranchAndBoundCoinSelection::default() + // .coin_select( + // required, + // optional, + // FeeRate::from_sat_per_vb(10.0), + // 500_000, + // &drain_script, + // ) + // .unwrap_err(); + + // assert!(matches!( + // err, + // Error::InsufficientFunds { + // available: 300_010, + // .. + // } + // )); + // } + + // #[test] + // fn test_bnb_sum_of_effective_value_negative() { + // let utxos = get_test_utxos(); + // let drain_script = Script::default(); + + // let err = BranchAndBoundCoinSelection::default() + // .coin_select( + // utxos, + // vec![], + // FeeRate::from_sat_per_vb(10_000.0), + // 500_000, + // &drain_script, + // ) + // .unwrap_err(); + + // assert!(matches!( + // err, + // Error::InsufficientFunds { + // available: 300_010, + // .. + // } + // )); + // } } diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 66ea14dfe..2704129b5 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -76,9 +76,8 @@ use crate::descriptor::{ use crate::error::Error; use crate::psbt::PsbtUtils; use crate::signer::SignerError; -use crate::testutils; use crate::types::*; -use crate::wallet::coin_selection::Excess::{Change, NoChange}; +use crate::{bdk_core, testutils}; use self::coin_control::CoinFilterParams; @@ -811,7 +810,6 @@ where // we keep it as a float while we accumulate it, and only round it at the end let mut outgoing: u64 = 0; - let mut received: u64 = 0; let recipients = params.recipients.iter().map(|(r, v)| (r, *v)); @@ -823,10 +821,6 @@ where return Err(Error::OutputBelowDustLimit(index)); } - if self.is_mine(script_pubkey)? { - received += value; - } - let new_out = TxOut { script_pubkey: script_pubkey.clone(), value, @@ -877,81 +871,176 @@ where .script_pubkey(), }; - let coin_selection = coin_selection.coin_select( - required_utxos, - optional_utxos, - fee_rate, - outgoing + fee_amount, - &drain_script, - )?; - fee_amount += coin_selection.fee_amount; - let excess = &coin_selection.excess; + let drain_output = TxOut { + value: 0, + script_pubkey: drain_script.clone(), + }; + + let drain_satisfaction_weight = self + .database + .borrow() + .get_path_from_script_pubkey(&drain_script)? + .map(|(keychain, _)| { + self._get_descriptor_for_keychain(keychain) + .0 + .max_satisfaction_weight() + }) + .transpose()? + .unwrap_or(0) as u32; + + let cs_opts = bdk_core::CoinSelectorOpt { + target_value: if params.recipients.is_empty() { + None + } else { + Some(outgoing) + }, + target_feerate: fee_rate.as_sat_per_vb() * 4.0, // sats/vb -> sats/wu + min_absolute_fee: fee_amount, + ..bdk_core::CoinSelectorOpt::fund_outputs( + &tx.output, + &drain_output, + drain_satisfaction_weight, + ) + }; - tx.input = coin_selection - .selected + let raw_candidates = required_utxos + .iter() + .chain(&optional_utxos) + .cloned() + .collect::>(); + let cs_candidates = required_utxos .iter() - .map(|u| bitcoin::TxIn { - previous_output: u.outpoint(), + .chain(&optional_utxos) + .map(|utxo| { + bdk_core::WeightedValue::new( + utxo.utxo.txout().value, + utxo.satisfaction_weight as u32, + utxo.utxo.txout().script_pubkey.is_witness_program(), + ) + }) + .collect::>(); + + let selector = { + let mut selector = bdk_core::CoinSelector::new(&cs_candidates, &cs_opts); + (0..required_utxos.len()).for_each(|index| { + selector.select(index); + }); + selector + }; + + let selection = coin_selection.coin_select(&raw_candidates, selector)?; + + // fee_amount += coin_selection.fee_amount; + // let excess = &coin_selection.excess; + + tx.input = selection + .apply_selection(&raw_candidates) + .map(|utxo| bitcoin::TxIn { + previous_output: utxo.utxo.outpoint(), script_sig: Script::default(), sequence: n_sequence, witness: Witness::new(), }) .collect(); - if tx.output.is_empty() { - // Uh oh, our transaction has no outputs. - // We allow this when: - // - We have a drain_to address and the utxos we must spend (this happens, - // for example, when we RBF) - // - We have a drain_to address and drain_wallet set - // Otherwise, we don't know who we should send the funds to, and how much - // we should send! - if params.drain_to.is_some() && (params.drain_wallet || !params.utxos.is_empty()) { - if let NoChange { - dust_threshold, - remaining_amount, - change_fee, - } = excess - { - return Err(Error::InsufficientFunds { - needed: *dust_threshold, - available: remaining_amount.saturating_sub(*change_fee), - }); - } - } else { - return Err(Error::NoRecipients); - } - } - - match excess { - NoChange { - remaining_amount, .. - } => fee_amount += remaining_amount, - Change { amount, fee } => { - if self.is_mine(&drain_script)? { - received += amount; - } - fee_amount += fee; + let (_, excess_strategy) = selection.best_strategy(); - // create drain output - let drain_output = TxOut { - value: *amount, - script_pubkey: drain_script, - }; + if let Some(drain_value) = excess_strategy.drain_value { + tx.output.push(TxOut { + value: drain_value, + script_pubkey: drain_script, + }); + } - // TODO: We should pay attention when adding a new output: this might increase - // the lenght of the "number of vouts" parameter by 2 bytes, potentially making - // our feerate too low - tx.output.push(drain_output); - } - }; + // tx.input = coin_selection + // .selected + // .iter() + // .map(|u| bitcoin::TxIn { + // previous_output: u.outpoint(), + // script_sig: Script::default(), + // sequence: n_sequence, + // witness: Witness::new(), + // }) + // .collect(); + + // if tx.output.is_empty() { + // // Uh oh, our transaction has no outputs. + // // We allow this when: + // // - We have a drain_to address and the utxos we must spend (this happens, + // // for example, when we RBF) + // // - We have a drain_to address and drain_wallet set + // // Otherwise, we don't know who we should send the funds to, and how much + // // we should send! + // if params.drain_to.is_some() && (params.drain_wallet || !params.utxos.is_empty()) { + // if let NoChange { + // dust_threshold, + // remaining_amount, + // change_fee, + // } = excess + // { + // return Err(Error::InsufficientFunds { + // needed: *dust_threshold, + // available: remaining_amount.saturating_sub(*change_fee), + // }); + // } + // } else { + // return Err(Error::NoRecipients); + // } + // } + + // match excess { + // NoChange { + // remaining_amount, .. + // } => fee_amount += remaining_amount, + // Change { amount, fee } => { + // if self.is_mine(&drain_script)? { + // received += amount; + // } + // fee_amount += fee; + + // // create drain output + // let drain_output = TxOut { + // value: *amount, + // script_pubkey: drain_script, + // }; + + // // TODO: We should pay attention when adding a new output: this might increase + // // the lenght of the "number of vouts" parameter by 2 bytes, potentially making + // // our feerate too low + // tx.output.push(drain_output); + // } + // }; // sort input/outputs according to the chosen algorithm params.ordering.sort_tx(&mut tx); let txid = tx.txid(); - let sent = coin_selection.local_selected_amount(); - let psbt = self.complete_transaction(tx, coin_selection.selected, params)?; + + let selected = selection + .apply_selection(&raw_candidates) + .map(|utxo| utxo.utxo.clone()) + .collect::>(); + + let sent = selected.iter().map(|utxo| utxo.txout().value).sum::(); + + let received = tx + .output + .iter() + .map(|txo| { + if self + .database + .borrow() + .is_mine(&txo.script_pubkey) + .unwrap_or(false) + { + txo.value + } else { + 0 + } + }) + .sum::(); + + let psbt = self.complete_transaction(tx, selected, params)?; let transaction_details = TransactionDetails { transaction: None, From e2797d434d0c330360a17431bb51ec03ec7d52ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Mon, 3 Oct 2022 16:33:15 +0800 Subject: [PATCH 11/16] Fix: min abs fee --- src/bdk_core/coin_select/coin_selector.rs | 2 +- src/wallet/mod.rs | 25 +++++++++-------------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/bdk_core/coin_select/coin_selector.rs b/src/bdk_core/coin_select/coin_selector.rs index 677104bae..7e87e0ad1 100644 --- a/src/bdk_core/coin_select/coin_selector.rs +++ b/src/bdk_core/coin_select/coin_selector.rs @@ -415,7 +415,7 @@ impl<'a> CoinSelector<'a> { } // with drain - if fee_with_drain > self.opts.min_absolute_fee + if fee_with_drain >= self.opts.min_absolute_fee && inputs_minus_outputs >= fee_with_drain + self.opts.min_drain_value { excess_strategies.insert( diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 2704129b5..892f7e732 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -797,6 +797,14 @@ where } }; + if params.recipients.is_empty() { + let is_drain_all = params.drain_wallet && params.drain_to.is_some(); + let is_rbf = !params.utxos.is_empty() && params.drain_to.is_some(); + if !(is_drain_all || is_rbf) { + return Err(Error::NoRecipients); + } + } + let mut tx = Transaction { version, lock_time, @@ -831,19 +839,6 @@ where outgoing += value; } - fee_amount += fee_rate.fee_wu(tx.weight()); - - // Segwit transactions' header is 2WU larger than legacy txs' header, - // as they contain a witness marker (1WU) and a witness flag (1WU) (see BIP144). - // At this point we really don't know if the resulting transaction will be segwit - // or legacy, so we just add this 2WU to the fee_amount - overshooting the fee amount - // is better than undershooting it. - // If we pass a fee_amount that is slightly higher than the final fee_amount, we - // end up with a transaction with a slightly higher fee rate than the requested one. - // If, instead, we undershoot, we may end up with a feerate lower than the requested one - // - we might come up with non broadcastable txs! - fee_amount += fee_rate.fee_wu(2); - if params.change_policy != tx_builder::ChangeSpendPolicy::ChangeAllowed && self.change_descriptor.is_none() { @@ -894,7 +889,7 @@ where } else { Some(outgoing) }, - target_feerate: fee_rate.as_sat_per_vb() * 4.0, // sats/vb -> sats/wu + target_feerate: fee_rate.as_sat_per_vb() / 4.0, // sats/vb -> sats/wu (prefer overshoot) min_absolute_fee: fee_amount, ..bdk_core::CoinSelectorOpt::fund_outputs( &tx.output, @@ -1048,7 +1043,7 @@ where confirmation_time: None, received, sent, - fee: Some(fee_amount), + fee: Some(excess_strategy.fee), }; Ok((psbt, transaction_details)) From 86fdda91ffc8ab48b4667b685c051034b79837a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 4 Oct 2022 17:34:49 +0800 Subject: [PATCH 12/16] WIP: JUST ONE MORE TEST TO FIX! --- src/bdk_core/coin_select/coin_selector.rs | 1 + src/psbt/mod.rs | 18 ++++- src/wallet/coin_selection.rs | 2 + src/wallet/mod.rs | 99 ++++++++++++++++------- 4 files changed, 88 insertions(+), 32 deletions(-) diff --git a/src/bdk_core/coin_select/coin_selector.rs b/src/bdk_core/coin_select/coin_selector.rs index 7e87e0ad1..ee0cdb8a0 100644 --- a/src/bdk_core/coin_select/coin_selector.rs +++ b/src/bdk_core/coin_select/coin_selector.rs @@ -26,6 +26,7 @@ impl WeightedValue { /// `satisfaction_weight` is the weight of `scriptSigLen + scriptSig + scriptWitnessLen + /// scriptWitness`. pub fn new(value: u64, satisfaction_weight: u32, is_segwit: bool) -> WeightedValue { + println!("- wv satisfaction weight: {}", satisfaction_weight); let weight = TXIN_BASE_WEIGHT + satisfaction_weight; WeightedValue { value, diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index f06b5297c..d06539d0b 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -167,6 +167,10 @@ mod test { let fee_amount = psbt.fee_amount(); assert!(fee_amount.is_some()); + let weight = psbt.clone().extract_tx().weight(); + println!("psbt weight: {}", weight); + println!("inputs: {}", psbt.clone().extract_tx().input.len()); + let unfinalized_fee_rate = psbt.fee_rate().unwrap(); let finalized = wallet.sign(&mut psbt, Default::default()).unwrap(); @@ -188,15 +192,27 @@ mod test { let mut builder = wallet.build_tx(); builder.drain_to(addr.script_pubkey()).drain_wallet(); builder.fee_rate(FeeRate::from_sat_per_vb(expected_fee_rate)); - let (mut psbt, _) = builder.finish().unwrap(); + let (mut psbt, details) = builder.finish().unwrap(); let fee_amount = psbt.fee_amount(); assert!(fee_amount.is_some()); let unfinalized_fee_rate = psbt.fee_rate().unwrap(); + println!("unfinalized fee rate: {:?}", unfinalized_fee_rate); let finalized = wallet.sign(&mut psbt, Default::default()).unwrap(); assert!(finalized); + let tx = psbt.clone().extract_tx(); + // println!("tx: {:#?}", tx); + println!("scriptSig weight: {}", tx.input[0].script_sig.len() * 4); + println!("weight: {}", tx.weight()); + let vb = tx.weight() as f32 / 4.0; + println!("vbytes: {}", vb); + let fee = details.fee.unwrap_or(0); + println!("fee: {}", details.fee.unwrap_or(0)); + println!("feerate: {} sats/vb", fee as f32 / vb); + let finalized_fee_rate = psbt.fee_rate().unwrap(); + println!("finalized fee rate: {:?}", finalized_fee_rate); assert!(finalized_fee_rate.as_sat_per_vb() >= expected_fee_rate); assert!(finalized_fee_rate.as_sat_per_vb() < unfinalized_fee_rate.as_sat_per_vb()); } diff --git a/src/wallet/coin_selection.rs b/src/wallet/coin_selection.rs index f25af1639..c9bd8d2a0 100644 --- a/src/wallet/coin_selection.rs +++ b/src/wallet/coin_selection.rs @@ -215,6 +215,7 @@ impl CoinSelectionAlgorithm for LargestFirstCoinSelection { let pool = { let mut pool = selector.unselected().collect::>(); pool.sort_unstable_by_key(|(_, candidate)| candidate.value); + pool.reverse(); pool }; @@ -226,6 +227,7 @@ impl CoinSelectionAlgorithm for LargestFirstCoinSelection { } selector.select(index); + println!("selected: index={}, value={}", index, _candidate.value); selection = selector.finish(); } diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 892f7e732..17703f192 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -924,6 +924,17 @@ where }; let selection = coin_selection.coin_select(&raw_candidates, selector)?; + let (_, excess_strategy) = selection.best_strategy(); + println!( + "create_tx: weight={}, fee={}, feerate={}sats/wu", + excess_strategy.weight, + excess_strategy.fee, + excess_strategy.feerate() + ); + println!( + "create_tx: feerate={}sats/vb", + excess_strategy.feerate() * 4.0 + ); // fee_amount += coin_selection.fee_amount; // let excess = &coin_selection.excess; @@ -938,8 +949,6 @@ where }) .collect(); - let (_, excess_strategy) = selection.best_strategy(); - if let Some(drain_value) = excess_strategy.drain_value { tx.output.push(TxOut { value: drain_value, @@ -1016,23 +1025,27 @@ where .map(|utxo| utxo.utxo.clone()) .collect::>(); - let sent = selected.iter().map(|utxo| utxo.txout().value).sum::(); + let sent = selected + .iter() + .filter(|utxo| { + self.database + .borrow() + .is_mine(&utxo.txout().script_pubkey) + .unwrap_or(false) + }) + .map(|utxo| utxo.txout().value) + .sum::(); let received = tx .output .iter() - .map(|txo| { - if self - .database + .filter(|txo| { + self.database .borrow() .is_mine(&txo.script_pubkey) .unwrap_or(false) - { - txo.value - } else { - 0 - } }) + .map(|txo| txo.value) .sum::(); let psbt = self.complete_transaction(tx, selected, params)?; @@ -2748,7 +2761,7 @@ pub(crate) mod test { } #[test] - #[should_panic(expected = "InsufficientFunds")] + // #[should_panic(expected = "InsufficientFunds")] fn test_create_tx_absolute_high_fee() { let (wallet, _, _) = get_funded_wallet(get_test_wpkh()); let addr = wallet.get_address(New).unwrap(); @@ -2757,7 +2770,10 @@ pub(crate) mod test { .drain_to(addr.script_pubkey()) .drain_wallet() .fee_absolute(60_000); - let (_psbt, _details) = builder.finish().unwrap(); + // let (_psbt, _details) = builder.finish().unwrap(); + assert!( + matches!(builder.finish(), Err(Error::Generic(s)) if s.contains("insufficient coins")) + ); } #[test] @@ -2794,7 +2810,7 @@ pub(crate) mod test { } #[test] - #[should_panic(expected = "InsufficientFunds")] + // #[should_panic(expected = "InsufficientFunds")] fn test_create_tx_drain_to_dust_amount() { let (wallet, _, _) = get_funded_wallet(get_test_wpkh()); let addr = wallet.get_address(New).unwrap(); @@ -2804,7 +2820,10 @@ pub(crate) mod test { .drain_to(addr.script_pubkey()) .drain_wallet() .fee_rate(FeeRate::from_sat_per_vb(455.0)); - builder.finish().unwrap(); + // builder.finish().unwrap(); + assert!( + matches!(builder.finish(), Err(Error::Generic(s)) if s.contains("insufficient coins")) + ); } #[test] @@ -3050,7 +3069,7 @@ pub(crate) mod test { } #[test] - #[should_panic(expected = "InsufficientFunds")] + // #[should_panic(expected = "InsufficientFunds")] fn test_create_tx_manually_selected_insufficient() { let (wallet, descriptors, _) = get_funded_wallet(get_test_wpkh()); let small_output_txid = crate::populate_test_db!( @@ -3069,7 +3088,10 @@ pub(crate) mod test { }) .unwrap() .manually_selected_only(); - builder.finish().unwrap(); + // builder.finish().unwrap(); + assert!( + matches!(builder.finish(), Err(Error::Generic(s)) if s.contains("insufficient coins")) + ); } #[test] @@ -3788,7 +3810,7 @@ pub(crate) mod test { } #[test] - #[should_panic(expected = "InsufficientFunds")] + // #[should_panic(expected = "InsufficientFunds")] fn test_bump_fee_remove_output_manually_selected_only() { let (wallet, descriptors, _) = get_funded_wallet(get_test_wpkh()); // receive an extra tx so that our wallet has two utxos. then we manually pick only one of @@ -3836,11 +3858,15 @@ pub(crate) mod test { builder .manually_selected_only() .fee_rate(FeeRate::from_sat_per_vb(255.0)); - builder.finish().unwrap(); + // builder.finish().unwrap(); + assert!( + matches!(builder.finish(), Err(Error::Generic(s)) if s.contains("insufficient coins")) + ); } #[test] fn test_bump_fee_add_input() { + // funded wallet already has 50_000 utxo let (wallet, descriptors, _) = get_funded_wallet(get_test_wpkh()); crate::populate_test_db!( wallet.database.borrow_mut(), @@ -4262,7 +4288,7 @@ pub(crate) mod test { } #[test] - #[should_panic(expected = "InsufficientFunds")] + // #[should_panic(expected = "InsufficientFunds")] fn test_bump_fee_unconfirmed_inputs_only() { // We try to bump the fee, but: // - We can't reduce the change, as we have no change @@ -4304,7 +4330,10 @@ pub(crate) mod test { let mut builder = wallet.build_fee_bump(txid).unwrap(); builder.fee_rate(FeeRate::from_sat_per_vb(25.0)); - builder.finish().unwrap(); + assert!( + matches!(builder.finish(), Err(Error::Generic(s)) if s.contains("insufficient coins")) + ); + // builder.finish().unwrap(); } #[test] @@ -5564,12 +5593,16 @@ pub(crate) mod test { .add_recipient(addr.script_pubkey(), balance.immature / 2) .current_height(confirmation_time); assert!(matches!( - builder.finish().unwrap_err(), - Error::InsufficientFunds { - needed: _, - available: 0 - } + builder.finish(), + Err(Error::Generic(s)) if s.contains("insufficient coins") )); + // assert!(matches!( + // builder.finish().unwrap_err(), + // Error::InsufficientFunds { + // needed: _, + // available: 0 + // } + // )); // Still unspendable... let mut builder = wallet.build_tx(); @@ -5577,12 +5610,16 @@ pub(crate) mod test { .add_recipient(addr.script_pubkey(), balance.immature / 2) .current_height(not_yet_mature_time); assert!(matches!( - builder.finish().unwrap_err(), - Error::InsufficientFunds { - needed: _, - available: 0 - } + builder.finish(), + Err(Error::Generic(s)) if s.contains("insufficient coins") )); + // assert!(matches!( + // builder.finish().unwrap_err(), + // Error::InsufficientFunds { + // needed: _, + // available: 0 + // } + // )); // ...Now the coinbase is mature :) let sync_time = SyncTime { From 543b263b54ca2227b036a1df72dacfaf5e3892fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 4 Oct 2022 22:39:42 +0800 Subject: [PATCH 13/16] miniscript bug workaround --- src/wallet/mod.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 17703f192..955a169b3 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -1551,12 +1551,18 @@ where Ok(self .iter_unspent()? .map(|utxo| { - let keychain = utxo.keychain; + let desc = self.get_descriptor_for_keychain(utxo.keychain); + + // WORKAROUND: There is a bug in miniscript where they fail to take into + // consideration an `OP_PUSH..` (4 weight units) for `pkh` script types. + let workaround_weight = match desc.desc_type() { + DescriptorType::Pkh => 4_usize, + _ => 0_usize, + }; + ( utxo, - self.get_descriptor_for_keychain(keychain) - .max_satisfaction_weight() - .unwrap(), + desc.max_satisfaction_weight().unwrap() + workaround_weight, ) }) .collect()) From 729f0ec5d9f71ff154a0237288d61958d26fad66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 4 Oct 2022 22:44:33 +0800 Subject: [PATCH 14/16] Some clippy fixes --- src/wallet/coin_selection.rs | 8 ++++---- src/wallet/mod.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/wallet/coin_selection.rs b/src/wallet/coin_selection.rs index c9bd8d2a0..21d85fdc9 100644 --- a/src/wallet/coin_selection.rs +++ b/src/wallet/coin_selection.rs @@ -188,7 +188,7 @@ pub trait CoinSelectionAlgorithm: std::fmt::Debug { #[allow(clippy::too_many_arguments)] fn coin_select( &self, - raw_candidates: &Vec, + raw_candidates: &[WeightedUtxo], selector: bdk_core::CoinSelector, ) -> Result; } @@ -203,7 +203,7 @@ pub struct LargestFirstCoinSelection; impl CoinSelectionAlgorithm for LargestFirstCoinSelection { fn coin_select( &self, - _raw_candidates: &Vec, + _raw_candidates: &[WeightedUtxo], mut selector: bdk_core::CoinSelector, ) -> Result { log::debug!( @@ -260,7 +260,7 @@ impl<'d, D> Debug for OldestFirstCoinSelection<'d, D> { impl<'d, D: Database> CoinSelectionAlgorithm for OldestFirstCoinSelection<'d, D> { fn coin_select( &self, - raw_candidates: &Vec, + raw_candidates: &[WeightedUtxo], mut selector: bdk_core::CoinSelector, ) -> Result { // query db and create a blockheight lookup table @@ -340,7 +340,7 @@ impl BranchAndBoundCoinSelection { impl CoinSelectionAlgorithm for BranchAndBoundCoinSelection { fn coin_select( &self, - _raw_candidates: &Vec, + _raw_candidates: &[WeightedUtxo], mut selector: bdk_core::CoinSelector, ) -> Result { if let Some(final_selector) = bdk_core::coin_select_bnb(self.limit, selector.clone()) { diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 955a169b3..1b4a04b10 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -768,7 +768,7 @@ where (Some(rbf), _) => rbf.get_value(), }; - let (fee_rate, mut fee_amount) = match params + let (fee_rate, fee_amount) = match params .fee_policy .as_ref() .unwrap_or(&FeePolicy::FeeRate(FeeRate::default())) From 429e5059daec1dd21e8247ec4da3ae4847007076 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 4 Oct 2022 23:27:21 +0800 Subject: [PATCH 15/16] Add back coin_selection tests --- src/psbt/mod.rs | 16 +- src/wallet/coin_selection.rs | 358 +++++++++++++++++++---------------- 2 files changed, 200 insertions(+), 174 deletions(-) diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index d06539d0b..3dcb8ab15 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -192,27 +192,15 @@ mod test { let mut builder = wallet.build_tx(); builder.drain_to(addr.script_pubkey()).drain_wallet(); builder.fee_rate(FeeRate::from_sat_per_vb(expected_fee_rate)); - let (mut psbt, details) = builder.finish().unwrap(); + let (mut psbt, _) = builder.finish().unwrap(); let fee_amount = psbt.fee_amount(); assert!(fee_amount.is_some()); - let unfinalized_fee_rate = psbt.fee_rate().unwrap(); - println!("unfinalized fee rate: {:?}", unfinalized_fee_rate); + let unfinalized_fee_rate = psbt.fee_rate().unwrap(); let finalized = wallet.sign(&mut psbt, Default::default()).unwrap(); assert!(finalized); - let tx = psbt.clone().extract_tx(); - // println!("tx: {:#?}", tx); - println!("scriptSig weight: {}", tx.input[0].script_sig.len() * 4); - println!("weight: {}", tx.weight()); - let vb = tx.weight() as f32 / 4.0; - println!("vbytes: {}", vb); - let fee = details.fee.unwrap_or(0); - println!("fee: {}", details.fee.unwrap_or(0)); - println!("feerate: {} sats/vb", fee as f32 / vb); - let finalized_fee_rate = psbt.fee_rate().unwrap(); - println!("finalized fee rate: {:?}", finalized_fee_rate); assert!(finalized_fee_rate.as_sat_per_vb() >= expected_fee_rate); assert!(finalized_fee_rate.as_sat_per_vb() < unfinalized_fee_rate.as_sat_per_vb()); } diff --git a/src/wallet/coin_selection.rs b/src/wallet/coin_selection.rs index 21d85fdc9..67e87a0b9 100644 --- a/src/wallet/coin_selection.rs +++ b/src/wallet/coin_selection.rs @@ -216,6 +216,7 @@ impl CoinSelectionAlgorithm for LargestFirstCoinSelection { let mut pool = selector.unselected().collect::>(); pool.sort_unstable_by_key(|(_, candidate)| candidate.value); pool.reverse(); + println!("pool: {:?}", pool); pool }; @@ -384,6 +385,8 @@ mod test { use bitcoin::{OutPoint, Script, TxOut}; use super::*; + use crate::bdk_core::{CoinSelector, CoinSelectorOpt, WeightedValue}; + use crate::database::{BatchOperations, MemoryDatabase}; // use crate::database::{BatchOperations, MemoryDatabase}; use crate::types::*; // use crate::wallet::WeightUnits; @@ -419,12 +422,38 @@ mod test { } } - fn get_test_utxos() -> Vec { - vec![ + fn get_test_utxos() -> (Vec, Vec) { + let utxos = vec![ utxo(100_000, 0), utxo(FEE_AMOUNT as u64 - 40, 1), utxo(200_000, 2), - ] + ]; + + let candidates = utxos + .iter() + .map(|utxo| { + bdk_core::WeightedValue::new( + utxo.utxo.txout().value, + utxo.satisfaction_weight as u32, + utxo.utxo.txout().script_pubkey.is_witness_program(), + ) + }) + .collect::>(); + + (utxos, candidates) + } + + fn get_test_candidates(utxos: &[WeightedUtxo]) -> Vec { + utxos + .iter() + .map(|utxo| { + bdk_core::WeightedValue::new( + utxo.utxo.txout().value, + utxo.satisfaction_weight as u32, + utxo.utxo.txout().script_pubkey.is_witness_program(), + ) + }) + .collect::>() } fn setup_database_and_get_oldest_first_test_utxos( @@ -531,183 +560,192 @@ mod test { .map(|u| u.utxo.txout().value) .sum() } - // fn test_largest_first_coin_selection_success() { - // let utxos = get_test_utxos(); - // let drain_script = Script::default(); - // let target_amount = 250_000 + FEE_AMOUNT; - // let result = LargestFirstCoinSelection::default() - // .coin_select( - // utxos, - // vec![], - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); + fn new_opts(target_value: u64, feerate: f32, drain_script: Script) -> CoinSelectorOpt { + CoinSelectorOpt { + target_feerate: feerate, + ..CoinSelectorOpt::fund_outputs( + &[TxOut { + value: target_value, + script_pubkey: Script::default(), + }], + &TxOut { + value: 0, + script_pubkey: drain_script, + }, + 0, + ) + } + } - // assert_eq!(result.selected.len(), 3); - // assert_eq!(result.selected_amount(), 300_010); - // assert_eq!(result.fee_amount, 204) - // } + #[test] + fn test_largest_first_coin_selection_success() { + let (utxos, candidates) = get_test_utxos(); + let target_amount = 250_000 + FEE_AMOUNT; - // #[test] - // fn test_largest_first_coin_selection_use_all() { - // let utxos = get_test_utxos(); - // let drain_script = Script::default(); - // let target_amount = 20_000 + FEE_AMOUNT; + let opts = new_opts(target_amount, 0.25, Script::default()); + let selector = bdk_core::CoinSelector::new(&candidates, &opts); - // let result = LargestFirstCoinSelection::default() - // .coin_select( - // utxos, - // vec![], - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); + let result = LargestFirstCoinSelection::default() + .coin_select(&utxos, selector) + .unwrap(); + let (strategy_kind, strategy) = result.best_strategy(); + println!("strategy_kind: {}", strategy_kind); - // assert_eq!(result.selected.len(), 3); - // assert_eq!(result.selected_amount(), 300_010); - // assert_eq!(result.fee_amount, 204); - // } + assert_eq!(result.selected.len(), 2); + assert_eq!(strategy.fee, 164); + } - // #[test] - // fn test_largest_first_coin_selection_use_only_necessary() { - // let utxos = get_test_utxos(); - // let drain_script = Script::default(); - // let target_amount = 20_000 + FEE_AMOUNT; + #[test] + fn test_largest_first_coin_selection_use_all() { + let (utxos, candidates) = get_test_utxos(); + let target_amount = 20_000 + FEE_AMOUNT; - // let result = LargestFirstCoinSelection::default() - // .coin_select( - // vec![], - // utxos, - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); + let opts = new_opts(target_amount, 0.25, Script::default()); + let mut selector = bdk_core::CoinSelector::new(&candidates, &opts); + selector.select_all(); - // assert_eq!(result.selected.len(), 1); - // assert_eq!(result.selected_amount(), 200_000); - // assert_eq!(result.fee_amount, 68); - // } + let result = LargestFirstCoinSelection::default() + .coin_select(&utxos, selector) + .unwrap(); + let (_, strategy) = result.best_strategy(); - // #[test] - // #[should_panic(expected = "InsufficientFunds")] - // fn test_largest_first_coin_selection_insufficient_funds() { - // let utxos = get_test_utxos(); - // let drain_script = Script::default(); - // let target_amount = 500_000 + FEE_AMOUNT; + assert_eq!(result.selected.len(), 3); + // assert_eq!(result.selected_amount(), 300_010); + assert_eq!(strategy.fee, 232); + } - // LargestFirstCoinSelection::default() - // .coin_select( - // vec![], - // utxos, - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); - // } + #[test] + fn test_largest_first_coin_selection_use_only_necessary() { + let (utxos, candidates) = get_test_utxos(); + let drain_script = Script::default(); + let target_amount = 20_000 + FEE_AMOUNT; - // #[test] - // #[should_panic(expected = "InsufficientFunds")] - // fn test_largest_first_coin_selection_insufficient_funds_high_fees() { - // let utxos = get_test_utxos(); - // let drain_script = Script::default(); - // let target_amount = 250_000 + FEE_AMOUNT; + let opts = new_opts(target_amount, 0.25, drain_script); + let selector = CoinSelector::new(&candidates, &opts); - // LargestFirstCoinSelection::default() - // .coin_select( - // vec![], - // utxos, - // FeeRate::from_sat_per_vb(1000.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); - // } + let result = LargestFirstCoinSelection::default() + .coin_select(&utxos, selector) + .unwrap(); + let (_, strategy) = result.best_strategy(); - // #[test] - // fn test_oldest_first_coin_selection_success() { - // let mut database = MemoryDatabase::default(); - // let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); - // let drain_script = Script::default(); - // let target_amount = 180_000 + FEE_AMOUNT; + assert_eq!(result.selected.len(), 1); + // assert_eq!(result.selected_amount(), 200_000); + assert_eq!(strategy.fee, 96); + } - // let result = OldestFirstCoinSelection::new(&database) - // .coin_select( - // vec![], - // utxos, - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); + #[test] + fn test_largest_first_coin_selection_insufficient_funds() { + let (utxos, candidates) = get_test_utxos(); + let drain_script = Script::default(); + let target_amount = 500_000 + FEE_AMOUNT; - // assert_eq!(result.selected.len(), 2); - // assert_eq!(result.selected_amount(), 200_000); - // assert_eq!(result.fee_amount, 136) - // } + let opts = new_opts(target_amount, 0.25, drain_script); + let selector = CoinSelector::new(&candidates, &opts); - // #[test] - // fn test_oldest_first_coin_selection_utxo_not_in_db_will_be_selected_last() { - // // ensure utxos are from different tx - // let utxo1 = utxo(120_000, 1); - // let utxo2 = utxo(80_000, 2); - // let utxo3 = utxo(300_000, 3); - // let drain_script = Script::default(); + let err = LargestFirstCoinSelection::default() + .coin_select(&utxos, selector) + .expect_err("should fail"); + assert!(matches!(err, Error::Generic(s) if s.contains("insufficient coins"))); + } - // let mut database = MemoryDatabase::default(); + #[test] + fn test_largest_first_coin_selection_insufficient_funds_high_fees() { + let (utxos, candidates) = get_test_utxos(); + let drain_script = Script::default(); + let target_amount = 250_000 + FEE_AMOUNT; - // // add tx to DB so utxos are sorted by blocktime asc - // // utxos will be selected by the following order - // // utxo1(blockheight 1) -> utxo2(blockheight 2), utxo3 (not exist in DB) - // // timestamp are all set as the same to ensure that only block height is used in sorting - // let utxo1_tx_details = TransactionDetails { - // transaction: None, - // txid: utxo1.utxo.outpoint().txid, - // received: 1, - // sent: 0, - // fee: None, - // confirmation_time: Some(BlockTime { - // height: 1, - // timestamp: 1231006505, - // }), - // }; - - // let utxo2_tx_details = TransactionDetails { - // transaction: None, - // txid: utxo2.utxo.outpoint().txid, - // received: 1, - // sent: 0, - // fee: None, - // confirmation_time: Some(BlockTime { - // height: 2, - // timestamp: 1231006505, - // }), - // }; - - // database.set_tx(&utxo1_tx_details).unwrap(); - // database.set_tx(&utxo2_tx_details).unwrap(); - - // let target_amount = 180_000 + FEE_AMOUNT; + let opts = new_opts(target_amount, 1000.0 / 4.0, drain_script); + let selector = CoinSelector::new(&candidates, &opts); - // let result = OldestFirstCoinSelection::new(&database) - // .coin_select( - // vec![], - // vec![utxo3, utxo1, utxo2], - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); + let err = LargestFirstCoinSelection::default() + .coin_select(&utxos, selector) + .expect_err("should fail"); + assert!(matches!(err, Error::Generic(s) if s.contains("insufficient coins"))); + } - // assert_eq!(result.selected.len(), 2); - // assert_eq!(result.selected_amount(), 200_000); - // assert_eq!(result.fee_amount, 136) - // } + #[test] + fn test_oldest_first_coin_selection_success() { + let mut database = MemoryDatabase::default(); + let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); + let candidates = get_test_candidates(&utxos); + let drain_script = Script::default(); + let target_amount = 180_000 + FEE_AMOUNT; + + let opts = new_opts(target_amount, 0.25, drain_script); + let selector = CoinSelector::new(&candidates, &opts); + + let result = OldestFirstCoinSelection::new(&database) + .coin_select(&utxos, selector) + .unwrap(); + let (_, strategy) = result.best_strategy(); + + assert_eq!(result.selected.len(), 2); + // assert_eq!(result.selected_amount(), 200_000); + assert!(strategy.feerate() >= 0.25); + } + + #[test] + fn test_oldest_first_coin_selection_utxo_not_in_db_will_be_selected_last() { + // ensure utxos are from different tx + let utxo1 = utxo(120_000, 1); + let utxo2 = utxo(80_000, 2); + let utxo3 = utxo(300_000, 3); + let drain_script = Script::default(); + + let mut database = MemoryDatabase::default(); + + // add tx to DB so utxos are sorted by blocktime asc + // utxos will be selected by the following order + // utxo1(blockheight 1) -> utxo2(blockheight 2), utxo3 (not exist in DB) + // timestamp are all set as the same to ensure that only block height is used in sorting + let utxo1_tx_details = TransactionDetails { + transaction: None, + txid: utxo1.utxo.outpoint().txid, + received: 1, + sent: 0, + fee: None, + confirmation_time: Some(BlockTime { + height: 1, + timestamp: 1231006505, + }), + }; + + let utxo2_tx_details = TransactionDetails { + transaction: None, + txid: utxo2.utxo.outpoint().txid, + received: 1, + sent: 0, + fee: None, + confirmation_time: Some(BlockTime { + height: 2, + timestamp: 1231006505, + }), + }; + + database.set_tx(&utxo1_tx_details).unwrap(); + database.set_tx(&utxo2_tx_details).unwrap(); + + let target_amount = 180_000 + FEE_AMOUNT; + + let opts = new_opts(target_amount, 0.25, drain_script); + let utxos = vec![utxo3, utxo1, utxo2]; + let candidates = get_test_candidates(&utxos); + let selector = CoinSelector::new(&candidates, &opts); + + let result = OldestFirstCoinSelection::new(&database) + .coin_select(&utxos, selector) + .unwrap(); + let (_, strategy) = result.best_strategy(); + + assert_eq!(result.selected.len(), 2); + let selected_sum = result + .apply_selection(&utxos) + .map(|u| u.utxo.txout().value) + .sum::(); + assert_eq!(selected_sum, 200_000); + assert!(strategy.feerate() >= 0.25); + } // #[test] // fn test_oldest_first_coin_selection_use_all() { From 4cd3a060f1181c0e858be58d75a570b75f05d89d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Wed, 5 Oct 2022 13:26:15 +0800 Subject: [PATCH 16/16] Add back more tests, branch strategy fn is now a type --- src/bdk_core/coin_select/bnb.rs | 15 +- src/wallet/coin_selection.rs | 636 +++++++++++++++----------------- 2 files changed, 313 insertions(+), 338 deletions(-) diff --git a/src/bdk_core/coin_select/bnb.rs b/src/bdk_core/coin_select/bnb.rs index e2c5baf7e..316be1bcd 100644 --- a/src/bdk_core/coin_select/bnb.rs +++ b/src/bdk_core/coin_select/bnb.rs @@ -2,6 +2,10 @@ use std::fmt::Display; use super::*; +/// Check our current selection (node), and returns the branching strategy, alongside a score +/// (if the current selection is a candidate solution). +pub type StrategyFn<'c, S> = dyn Fn(&Bnb<'c, S>) -> (BranchStrategy, Option); + /// Strategy in which we should branch. pub enum BranchStrategy { /// We continue exploring subtrees of this node, starting with the inclusion branch. @@ -60,10 +64,7 @@ impl<'c, S: Ord> Bnb<'c, S> { /// /// `strategy` should assess our current selection/node and determine the branching strategy and /// whether this selection is a candidate solution (if so, return the score of the selection). - pub fn into_iter<'f>( - self, - strategy: &'f dyn Fn(&Self) -> (BranchStrategy, Option), - ) -> BnbIter<'c, 'f, S> { + pub fn into_iter<'f>(self, strategy: &'f StrategyFn<'c, S>) -> BnbIter<'c, 'f, S> { BnbIter { state: self, done: false, @@ -120,7 +121,7 @@ pub struct BnbIter<'c, 'f, S> { /// Check our current selection (node), and returns the branching strategy, alongside a score /// (if the current selection is a candidate solution). - strategy: &'f dyn Fn(&Bnb<'c, S>) -> (BranchStrategy, Option), + strategy: &'f StrategyFn<'c, S>, } impl<'c, 'f, S: Ord + Copy + Display> Iterator for BnbIter<'c, 'f, S> { @@ -239,7 +240,7 @@ where let upper_bound_abs = target_abs + (opts.drain_weight as f32 * opts.target_feerate) as u64; let upper_bound_eff = target_eff + opts.drain_waste(); - let strategy = |bnb: &Bnb| -> (BranchStrategy, Option) { + let strategy = move |bnb: &Bnb| -> (BranchStrategy, Option) { let selected_abs = bnb.selection.selected_absolute_value(); let selected_eff = bnb.selection.selected_effective_value(); @@ -270,7 +271,7 @@ where // early bailout optimization: // If the candidate at the previous position is NOT selected and has the same weight and // value as the current candidate, we can skip selecting the current candidate. - if !bnb.selection.is_empty() { + if bnb.pool_pos > 0 && !bnb.selection.is_empty() { let (_, candidate) = bnb.pool[bnb.pool_pos]; let (prev_index, prev_candidate) = bnb.pool[bnb.pool_pos - 1]; diff --git a/src/wallet/coin_selection.rs b/src/wallet/coin_selection.rs index 67e87a0b9..3b7de8a5b 100644 --- a/src/wallet/coin_selection.rs +++ b/src/wallet/coin_selection.rs @@ -94,8 +94,8 @@ //! ``` use crate::bdk_core::{self, BnbLimit}; +use crate::error::Error; use crate::{database::Database, WeightedUtxo}; -use crate::{error::Error, Utxo}; use rand::seq::SliceRandom; #[cfg(not(test))] @@ -116,56 +116,6 @@ pub type DefaultCoinSelectionAlgorithm = LargestFirstCoinSelection; // make the // prev_txid (32 bytes) + prev_vout (4 bytes) + sequence (4 bytes) // pub(crate) const TXIN_BASE_WEIGHT: usize = (32 + 4 + 4) * 4; -#[derive(Debug)] -/// Remaining amount after performing coin selection -pub enum Excess { - /// It's not possible to create spendable output from excess using the current drain output - NoChange { - /// Threshold to consider amount as dust for this particular change script_pubkey - dust_threshold: u64, - /// Exceeding amount of current selection over outgoing value and fee costs - remaining_amount: u64, - /// The calculated fee for the drain TxOut with the selected script_pubkey - change_fee: u64, - }, - /// It's possible to create spendable output from excess using the current drain output - Change { - /// Effective amount available to create change after deducting the change output fee - amount: u64, - /// The deducted change output fee - fee: u64, - }, -} - -/// Result of a successful coin selection -#[derive(Debug)] -pub struct CoinSelectionResult { - /// List of outputs selected for use as inputs - pub selected: Vec, - /// Total fee amount for the selected utxos in satoshis - pub fee_amount: u64, - /// Remaining amount after deducing fees and outgoing outputs - pub excess: Excess, -} - -impl CoinSelectionResult { - /// The total value of the inputs selected. - pub fn selected_amount(&self) -> u64 { - self.selected.iter().map(|u| u.txout().value).sum() - } - - /// The total value of the inputs selected from the local wallet. - pub fn local_selected_amount(&self) -> u64 { - self.selected - .iter() - .filter_map(|u| match u { - Utxo::Local(_) => Some(u.txout().value), - _ => None, - }) - .sum() - } -} - /// Trait for generalized coin selection algorithms /// /// This trait can be implemented to make the [`Wallet`](super::Wallet) use a customized coin @@ -319,21 +269,24 @@ impl<'d, D: Database> CoinSelectionAlgorithm for OldestFirstCoinSelection<'d, D> #[derive(Debug)] pub struct BranchAndBoundCoinSelection { limit: BnbLimit, + use_fallback: bool, } impl Default for BranchAndBoundCoinSelection { fn default() -> Self { Self { limit: BnbLimit::Rounds(100_000), + use_fallback: true, } } } impl BranchAndBoundCoinSelection { /// Create new instance with target size for change output - pub fn new>(limit: L) -> Self { + pub fn new>(limit: L, use_fallback: bool) -> Self { Self { limit: limit.into(), + use_fallback, } } } @@ -348,6 +301,10 @@ impl CoinSelectionAlgorithm for BranchAndBoundCoinSelection { return final_selector.finish().map_err(Error::from); } + if !self.use_fallback { + return Err(Error::BnBNoExactMatch); + } + // FALLBACK: single random draw let pool = { let mut pool = selector @@ -382,10 +339,10 @@ impl CoinSelectionAlgorithm for BranchAndBoundCoinSelection { mod test { use std::str::FromStr; - use bitcoin::{OutPoint, Script, TxOut}; + use bitcoin::{OutPoint, Script, Transaction, TxOut}; use super::*; - use crate::bdk_core::{CoinSelector, CoinSelectorOpt, WeightedValue}; + use crate::bdk_core::{CoinSelector, CoinSelectorOpt, Selection, WeightedValue}; use crate::database::{BatchOperations, MemoryDatabase}; // use crate::database::{BatchOperations, MemoryDatabase}; use crate::types::*; @@ -578,6 +535,26 @@ mod test { } } + fn sum_selected(selection: &Selection, utxos: &[WeightedUtxo]) -> u64 { + selection + .apply_selection(utxos) + .map(|u| u.utxo.txout().value) + .sum() + } + + fn tx_template_fee(recipient_script: Script, feerate: f32) -> i64 { + let tx = Transaction { + version: 0, + lock_time: 0, + input: Vec::new(), + output: vec![TxOut { + value: 0, + script_pubkey: recipient_script, + }], + }; + (tx.weight() as f32 * feerate).ceil() as _ + } + #[test] fn test_largest_first_coin_selection_success() { let (utxos, candidates) = get_test_utxos(); @@ -739,337 +716,334 @@ mod test { let (_, strategy) = result.best_strategy(); assert_eq!(result.selected.len(), 2); - let selected_sum = result - .apply_selection(&utxos) - .map(|u| u.utxo.txout().value) - .sum::(); - assert_eq!(selected_sum, 200_000); + assert_eq!(sum_selected(&result, &utxos), 200_000); assert!(strategy.feerate() >= 0.25); } - // #[test] - // fn test_oldest_first_coin_selection_use_all() { - // let mut database = MemoryDatabase::default(); - // let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); - // let drain_script = Script::default(); - // let target_amount = 20_000 + FEE_AMOUNT; + #[test] + fn test_oldest_first_coin_selection_use_all() { + let mut database = MemoryDatabase::default(); + let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); + let candidates = get_test_candidates(&utxos); + let target_amount = 20_000 + FEE_AMOUNT; - // let result = OldestFirstCoinSelection::new(&database) - // .coin_select( - // utxos, - // vec![], - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); + let opts = new_opts(target_amount, 0.25, Script::default()); + let mut selector = CoinSelector::new(&candidates, &opts); + selector.select_all(); - // assert_eq!(result.selected.len(), 3); - // assert_eq!(result.selected_amount(), 500_000); - // assert_eq!(result.fee_amount, 204); - // } + let result = OldestFirstCoinSelection::new(&database) + .coin_select(&utxos, selector) + .unwrap(); + let (_, strategy) = result.best_strategy(); - // #[test] - // fn test_oldest_first_coin_selection_use_only_necessary() { - // let mut database = MemoryDatabase::default(); - // let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); - // let drain_script = Script::default(); - // let target_amount = 20_000 + FEE_AMOUNT; + assert_eq!(result.selected.len(), 3); + assert!(strategy.feerate() >= 0.25); + assert_eq!(sum_selected(&result, &utxos), 500_000); + } - // let result = OldestFirstCoinSelection::new(&database) - // .coin_select( - // vec![], - // utxos, - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); + #[test] + fn test_oldest_first_coin_selection_use_only_necessary() { + let mut database = MemoryDatabase::default(); + let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); + let candidates = get_test_candidates(&utxos); + let drain_script = Script::default(); + let target_amount = 20_000 + FEE_AMOUNT; - // assert_eq!(result.selected.len(), 1); - // assert_eq!(result.selected_amount(), 120_000); - // assert_eq!(result.fee_amount, 68); - // } + let opts = new_opts(target_amount, 0.25, drain_script); + let selector = CoinSelector::new(&candidates, &opts); - // #[test] + let result = OldestFirstCoinSelection::new(&database) + .coin_select(&utxos, selector) + .unwrap(); + let (_, strategy) = result.best_strategy(); + + assert_eq!(result.selected.len(), 1); + assert_eq!(sum_selected(&result, &utxos), 120_000); + assert!(strategy.feerate() >= 0.25); + } + + #[test] // #[should_panic(expected = "InsufficientFunds")] - // fn test_oldest_first_coin_selection_insufficient_funds() { - // let mut database = MemoryDatabase::default(); - // let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); - // let drain_script = Script::default(); - // let target_amount = 600_000 + FEE_AMOUNT; + fn test_oldest_first_coin_selection_insufficient_funds() { + let mut database = MemoryDatabase::default(); + let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); + let candidates = get_test_candidates(&utxos); + let drain_script = Script::default(); + let target_amount = 600_000 + FEE_AMOUNT; - // OldestFirstCoinSelection::new(&database) - // .coin_select( - // vec![], - // utxos, - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); - // } + let opts = new_opts(target_amount, 0.25, drain_script); + let selector = CoinSelector::new(&candidates, &opts); - // #[test] + let err = OldestFirstCoinSelection::new(&database) + .coin_select(&utxos, selector) + .expect_err("should fail"); + + assert!(matches!(err, Error::Generic(s) if s.contains("insufficient coins"))); + } + + #[test] // #[should_panic(expected = "InsufficientFunds")] - // fn test_oldest_first_coin_selection_insufficient_funds_high_fees() { - // let mut database = MemoryDatabase::default(); - // let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); + fn test_oldest_first_coin_selection_insufficient_funds_high_fees() { + let mut database = MemoryDatabase::default(); + let utxos = setup_database_and_get_oldest_first_test_utxos(&mut database); + let candidates = get_test_candidates(&utxos); - // let target_amount: u64 = utxos.iter().map(|wu| wu.utxo.txout().value).sum::() - 50; - // let drain_script = Script::default(); + let target_amount: u64 = utxos.iter().map(|wu| wu.utxo.txout().value).sum::() - 50; + let drain_script = Script::default(); - // OldestFirstCoinSelection::new(&database) - // .coin_select( - // vec![], - // utxos, - // FeeRate::from_sat_per_vb(1000.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); - // } + let opts = new_opts(target_amount, 1000.0 / 4.0, drain_script); + let selector = CoinSelector::new(&candidates, &opts); - // #[test] - // fn test_bnb_coin_selection_success() { - // // In this case bnb won't find a suitable match and single random draw will - // // select three outputs - // let utxos = generate_same_value_utxos(100_000, 20); + let err = OldestFirstCoinSelection::new(&database) + .coin_select(&utxos, selector) + .expect_err("should fail"); - // let drain_script = Script::default(); + assert!(matches!(err, Error::Generic(s) if s.contains("insufficient coins"))); + } - // let target_amount = 250_000 + FEE_AMOUNT; + #[test] + fn test_bnb_coin_selection_success() { + // In this case bnb won't find a suitable match and single random draw will + // select three outputs + let utxos = generate_same_value_utxos(100_000, 20); + let candidates = get_test_candidates(&utxos); - // let result = BranchAndBoundCoinSelection::default() - // .coin_select( - // vec![], - // utxos, - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); + let drain_script = Script::default(); + let target_amount = 250_000 + FEE_AMOUNT; + let feerate = 1.0_f32 / 4.0; - // assert_eq!(result.selected.len(), 3); - // assert_eq!(result.selected_amount(), 300_000); - // assert_eq!(result.fee_amount, 204); - // } + let opts = new_opts(target_amount, feerate, drain_script); + let selector = CoinSelector::new(&candidates, &opts); - // #[test] - // fn test_bnb_coin_selection_required_are_enough() { - // let utxos = get_test_utxos(); - // let drain_script = Script::default(); - // let target_amount = 20_000 + FEE_AMOUNT; + let result = BranchAndBoundCoinSelection::default() + .coin_select(&utxos, selector) + .unwrap(); + let (_, strategy) = result.best_strategy(); - // let result = BranchAndBoundCoinSelection::default() - // .coin_select( - // utxos.clone(), - // utxos, - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); + assert_eq!(result.selected.len(), 3); + assert_eq!(sum_selected(&result, &utxos), 300_000); + assert!(strategy.feerate() >= feerate); + } - // assert_eq!(result.selected.len(), 3); - // assert_eq!(result.selected_amount(), 300_010); - // assert_eq!(result.fee_amount, 204); - // } + #[test] + fn test_bnb_coin_selection_required_are_enough() { + let (utxos, candidates) = { + let (u1, c1) = get_test_utxos(); + let (u2, c2) = get_test_utxos(); + let u = u1.into_iter().chain(u2).collect::>(); + let c = c1.into_iter().chain(c2).collect::>(); + (u, c) + }; + let drain_script = Script::default(); + let target_amount = 20_000 + FEE_AMOUNT; + let feerate = 1.0_f32 / 4.0; - // #[test] - // fn test_bnb_coin_selection_optional_are_enough() { - // let utxos = get_test_utxos(); - // let drain_script = Script::default(); - // let target_amount = 299756 + FEE_AMOUNT; + let opts = new_opts(target_amount, feerate, drain_script); + let mut selector = CoinSelector::new(&candidates, &opts); + (0..utxos.len() / 2).for_each(|index| assert!(selector.select(index))); - // let result = BranchAndBoundCoinSelection::default() - // .coin_select( - // vec![], - // utxos, - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); + let result = BranchAndBoundCoinSelection::default() + .coin_select(&utxos, selector) + .unwrap(); + let (_, strategy) = result.best_strategy(); - // assert_eq!(result.selected.len(), 2); - // assert_eq!(result.selected_amount(), 300000); - // assert_eq!(result.fee_amount, 136); - // } + assert_eq!(result.selected.len(), 3); + assert_eq!(sum_selected(&result, &utxos), 300_010); + assert!(strategy.feerate() >= feerate); + } - // #[test] - // fn test_bnb_coin_selection_required_not_enough() { - // let utxos = get_test_utxos(); + #[test] + fn test_bnb_coin_selection_optional_are_enough() { + let (utxos, candidates) = get_test_utxos(); + let drain_script = Script::default(); + let target_amount = 299756 + FEE_AMOUNT; + let feerate = 1.0_f32 / 4.0; - // let required = vec![utxos[0].clone()]; - // let mut optional = utxos[1..].to_vec(); - // optional.push(utxo(500_000, 3)); + let opts = new_opts(target_amount, feerate, drain_script); + let selector = CoinSelector::new(&candidates, &opts); - // // Defensive assertions, for sanity and in case someone changes the test utxos vector. - // let amount: u64 = required.iter().map(|u| u.utxo.txout().value).sum(); - // assert_eq!(amount, 100_000); - // let amount: u64 = optional.iter().map(|u| u.utxo.txout().value).sum(); - // assert!(amount > 150_000); - // let drain_script = Script::default(); + let result = BranchAndBoundCoinSelection::default() + .coin_select(&utxos, selector) + .unwrap(); + let (_, strategy) = result.best_strategy(); - // let target_amount = 150_000 + FEE_AMOUNT; + assert_eq!(result.selected.len(), 2); + assert_eq!(sum_selected(&result, &utxos), 300000); + assert!(strategy.feerate() >= feerate); + } - // let result = BranchAndBoundCoinSelection::default() - // .coin_select( - // required, - // optional, - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); + #[test] + fn test_bnb_coin_selection_required_not_enough() { + let (utxos, _) = get_test_utxos(); - // assert_eq!(result.selected.len(), 2); - // assert_eq!(result.selected_amount(), 300_000); - // assert_eq!(result.fee_amount, 136); - // } + let required = vec![utxos[0].clone()]; + let mut optional = utxos[1..].to_vec(); + optional.push(utxo(500_000, 3)); - // #[test] + // Defensive assertions, for sanity and in case someone changes the test utxos vector. + let amount: u64 = required.iter().map(|u| u.utxo.txout().value).sum(); + assert_eq!(amount, 100_000); + let amount: u64 = optional.iter().map(|u| u.utxo.txout().value).sum(); + assert!(amount > 150_000); + + let drain_script = Script::default(); + let target_amount = 150_000 + FEE_AMOUNT; + let feerate = 1.0_f32 / 4.0; + + let all_utxos = required + .iter() + .chain(&optional) + .cloned() + .collect::>(); + let all_candidates = get_test_candidates(&all_utxos); + + let opts = new_opts(target_amount, feerate, drain_script); + let mut selector = CoinSelector::new(&all_candidates, &opts); + (0..required.len()).for_each(|index| assert!(selector.select(index))); + + let result = BranchAndBoundCoinSelection::default() + .coin_select(&all_utxos, selector) + .unwrap(); + let (_, strategy) = result.best_strategy(); + + assert_eq!(result.selected.len(), 2); + assert_eq!(sum_selected(&result, &all_utxos), 300_000); + assert!(strategy.feerate() >= feerate); + } + + #[test] // #[should_panic(expected = "InsufficientFunds")] - // fn test_bnb_coin_selection_insufficient_funds() { - // let utxos = get_test_utxos(); - // let drain_script = Script::default(); - // let target_amount = 500_000 + FEE_AMOUNT; + fn test_bnb_coin_selection_insufficient_funds() { + let (utxos, candidates) = get_test_utxos(); + let drain_script = Script::default(); + let target_amount = 500_000 + FEE_AMOUNT; + let feerate = 1.0_f32 / 4.0; - // BranchAndBoundCoinSelection::default() - // .coin_select( - // vec![], - // utxos, - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); - // } + let opts = new_opts(target_amount, feerate, drain_script); + let selector = CoinSelector::new(&candidates, &opts); - // #[test] + let err = BranchAndBoundCoinSelection::default() + .coin_select(&utxos, selector) + .expect_err("should fail"); + + assert!(matches!(err, Error::Generic(s) if s.contains("insufficient coins"))); + } + + #[test] // #[should_panic(expected = "InsufficientFunds")] - // fn test_bnb_coin_selection_insufficient_funds_high_fees() { - // let utxos = get_test_utxos(); - // let drain_script = Script::default(); - // let target_amount = 250_000 + FEE_AMOUNT; + fn test_bnb_coin_selection_insufficient_funds_high_fees() { + let (utxos, candidates) = get_test_utxos(); + let drain_script = Script::default(); + let target_amount = 250_000 + FEE_AMOUNT; + let feerate = 1000_f32 / 4.0; - // BranchAndBoundCoinSelection::default() - // .coin_select( - // vec![], - // utxos, - // FeeRate::from_sat_per_vb(1000.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); - // } + let opts = new_opts(target_amount, feerate, drain_script); + let selector = CoinSelector::new(&candidates, &opts); - // #[test] - // fn test_bnb_coin_selection_check_fee_rate() { - // let utxos = get_test_utxos(); - // let drain_script = Script::default(); - // let target_amount = 99932; // first utxo's effective value + let err = BranchAndBoundCoinSelection::default() + .coin_select(&utxos, selector) + .expect_err("should fail"); - // let result = BranchAndBoundCoinSelection::new(0) - // .coin_select( - // vec![], - // utxos, - // FeeRate::from_sat_per_vb(1.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); + assert!(matches!(err, Error::Generic(s) if s.contains("insufficient coins"))); + } - // assert_eq!(result.selected.len(), 1); - // assert_eq!(result.selected_amount(), 100_000); - // let input_size = (TXIN_BASE_WEIGHT + P2WPKH_SATISFACTION_SIZE).vbytes(); - // // the final fee rate should be exactly the same as the fee rate given - // assert!((1.0 - (result.fee_amount as f32 / input_size as f32)).abs() < f32::EPSILON); - // } + #[test] + fn test_bnb_coin_selection_check_fee_rate() { + let (utxos, candidates) = get_test_utxos(); + let drain_script = Script::default(); + let feerate = 1.0_f32 / 4.0; - // #[test] - // fn test_bnb_coin_selection_exact_match() { - // let seed = [0; 32]; - // let mut rng: StdRng = SeedableRng::from_seed(seed); + // let target_amount = 99932; // first utxo's effective value + let template_fee = tx_template_fee(Script::default(), feerate) as u64; + let candidate_cost = candidates[0].effective_value(feerate) as u64; + let target_amount = candidate_cost - template_fee; - // for _i in 0..200 { - // let mut optional_utxos = generate_random_utxos(&mut rng, 16); - // let target_amount = sum_random_utxos(&mut rng, &mut optional_utxos); - // let drain_script = Script::default(); - // let result = BranchAndBoundCoinSelection::new(0) - // .coin_select( - // vec![], - // optional_utxos, - // FeeRate::from_sat_per_vb(0.0), - // target_amount, - // &drain_script, - // ) - // .unwrap(); - // assert_eq!(result.selected_amount(), target_amount); - // } - // } + let opts = new_opts(target_amount, feerate, drain_script); + let selector = CoinSelector::new(&candidates, &opts); - // #[test] + let result = BranchAndBoundCoinSelection::default() + .coin_select(&utxos, selector) + .unwrap(); + let (_, strategy) = result.best_strategy(); + + assert_eq!(result.selected.len(), 1); + assert_eq!(sum_selected(&result, &utxos), 100_000); + + // let input_size = (TXIN_BASE_WEIGHT as usize + P2WPKH_SATISFACTION_SIZE).to_vbytes(); + // the final fee rate should be exactly the same as the fee rate given + // assert!((1.0 - (strategy.fee as f32 / input_size as f32)).abs() < f32::EPSILON); + assert_eq!(strategy.feerate(), feerate); + } + + #[test] + fn test_bnb_coin_selection_exact_match() { + let seed = [0; 32]; + let mut rng: StdRng = SeedableRng::from_seed(seed); + + for _i in 0..200 { + let feerate = 0_f32; + let template_fee = tx_template_fee(Script::default(), feerate); + + let mut optional_utxos = generate_random_utxos(&mut rng, 16); + let target_amount = + sum_random_utxos(&mut rng, &mut optional_utxos) - template_fee as u64; + let candidates = get_test_candidates(&optional_utxos); + let drain_script = Script::default(); + + let opts = new_opts(target_amount, feerate, drain_script); + let selector = CoinSelector::new(&candidates, &opts); + + let result = BranchAndBoundCoinSelection::default() + .coin_select(&optional_utxos, selector) + .unwrap(); + assert_eq!(sum_selected(&result, &optional_utxos), target_amount); + } + } + + #[test] // #[should_panic(expected = "BnBNoExactMatch")] - // fn test_bnb_function_no_exact_match() { - // let fee_rate = FeeRate::from_sat_per_vb(10.0); - // let utxos: Vec = get_test_utxos() - // .into_iter() - // .map(|u| OutputGroup::new(u, fee_rate)) - // .collect(); + fn test_bnb_function_no_exact_match() { + let fee_rate = FeeRate::from_sat_per_vb(10.0); + let (utxos, candidates) = get_test_utxos(); - // let curr_available_value = utxos.iter().fold(0, |acc, x| acc + x.effective_value); + // let curr_available_value = utxos.iter().fold(0, |acc, x| acc + x.effective_value); + // let size_of_change = 31; + // let cost_of_change = size_of_change as f32 * fee_rate.as_sat_per_vb(); - // let size_of_change = 31; - // let cost_of_change = size_of_change as f32 * fee_rate.as_sat_per_vb(); + let drain_script = Script::default(); + let target_amount = 20_000 + FEE_AMOUNT; - // let drain_script = Script::default(); - // let target_amount = 20_000 + FEE_AMOUNT; - // BranchAndBoundCoinSelection::new(size_of_change) - // .bnb( - // vec![], - // utxos, - // 0, - // curr_available_value, - // target_amount as i64, - // cost_of_change, - // &drain_script, - // fee_rate, - // ) - // .unwrap(); - // } + let opts = new_opts(target_amount, fee_rate.as_sat_per_vb() / 4.0, drain_script); + let selector = CoinSelector::new(&candidates, &opts); - // #[test] + let err = BranchAndBoundCoinSelection::new(10_000, false) + .coin_select(&utxos, selector) + .expect_err("should fail"); + assert!(matches!(err, Error::BnBNoExactMatch)); + } + + #[test] // #[should_panic(expected = "BnBTotalTriesExceeded")] - // fn test_bnb_function_tries_exceeded() { - // let fee_rate = FeeRate::from_sat_per_vb(10.0); - // let utxos: Vec = generate_same_value_utxos(100_000, 100_000) - // .into_iter() - // .map(|u| OutputGroup::new(u, fee_rate)) - // .collect(); + fn test_bnb_function_tries_exceeded() { + let fee_rate = FeeRate::from_sat_per_vb(10.0); + let utxos = generate_same_value_utxos(100_000, 100_000); + let candidates = get_test_candidates(&utxos); - // let curr_available_value = utxos.iter().fold(0, |acc, x| acc + x.effective_value); + // let curr_available_value = utxos.iter().fold(0, |acc, x| acc + x.effective_value); - // let size_of_change = 31; - // let cost_of_change = size_of_change as f32 * fee_rate.as_sat_per_vb(); - // let target_amount = 20_000 + FEE_AMOUNT; + // let size_of_change = 31; + // let cost_of_change = size_of_change as f32 * fee_rate.as_sat_per_vb(); + let target_amount = 20_000 + FEE_AMOUNT; + let drain_script = Script::default(); - // let drain_script = Script::default(); + let opts = new_opts(target_amount, fee_rate.as_sat_per_vb() / 4.0, drain_script); + let selector = CoinSelector::new(&candidates, &opts); - // BranchAndBoundCoinSelection::new(size_of_change) - // .bnb( - // vec![], - // utxos, - // 0, - // curr_available_value, - // target_amount as i64, - // cost_of_change, - // &drain_script, - // fee_rate, - // ) - // .unwrap(); - // } + let err = BranchAndBoundCoinSelection::new(10_000, false) + .coin_select(&utxos, selector) + .expect_err("should fail"); + assert!(matches!(err, Error::BnBNoExactMatch)); + } // // The match won't be exact but still in the range // #[test]