From 7e82c3fa73b3db40a8dc412ec7686578e4b320cc Mon Sep 17 00:00:00 2001 From: LLFourn Date: Thu, 25 Jan 2024 17:28:45 +1100 Subject: [PATCH 1/9] Remove base_weight, put weight in Target - CoinSelector no longer tracks anything but input weight - Previously the value of the target outputs was in `Target` but the weights were accounted for in CoinSelector. Now they're in all in target. - This allows us to actually figure out how many outputs there are and therefore the actual weight of the transaction accounting for varints. --- CHANGELOG.md | 3 + README.md | 229 ++++----------- src/change_policy.rs | 6 +- src/coin_selector.rs | 110 +++---- src/drain.rs | 45 ++- src/lib.rs | 17 +- src/metrics.rs | 2 - src/metrics/changeless.rs | 1 + src/metrics/lowest_fee.rs | 22 +- src/metrics/waste.rs | 243 ---------------- src/target.rs | 117 ++++++-- tests/bnb.rs | 58 ++-- tests/changeless.rs | 89 ++---- tests/common.rs | 29 +- tests/lowest_fee.rs | 77 ++--- tests/waste.proptest-regressions | 16 - tests/waste.rs | 485 ------------------------------- tests/weight.rs | 119 ++++---- 18 files changed, 444 insertions(+), 1224 deletions(-) delete mode 100644 src/metrics/waste.rs delete mode 100644 tests/waste.proptest-regressions delete mode 100644 tests/waste.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ee2cc2..4c10e3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,4 +4,7 @@ - Remove `min_fee` in favour of `replace` which allows you to replace a transaction - Remove `Drain` argument from `CoinSelector::select_until_target_met` because adding a drain won't change when the target is met. +- No more `base_weight` in `CoinSelector`. Weight of the outputs is tracked in `target`. +- You now account for the number of outputs in both drain and target and their weight. +- Removed waste metric because it was pretty broken and took a lot to maintain diff --git a/README.md b/README.md index db85e72..ba2d8ad 100644 --- a/README.md +++ b/README.md @@ -1,26 +1,30 @@ # BDK Coin Selection -`bdk_coin_select` is a tool to help you select inputs for making Bitcoin (ticker: BTC) transactions. -It's got zero dependencies so you can paste it into your project without concern. +`bdk_coin_select` is a zero-dependency tool to help you select inputs for making Bitcoin (ticker: BTC) transactions. > ⚠ This work is only ready to use by those who expect (potentially catastrophic) bugs and will have > the time to investigate them and contribute back to this crate. -## Constructing the `CoinSelector` - -The main structure is [`CoinSelector`](crate::CoinSelector). To construct it, we specify a list of -candidate UTXOs and a transaction `base_weight`. The `base_weight` includes the recipient outputs -and mandatory inputs (if any). +## Synopis ```rust use std::str::FromStr; -use bdk_coin_select::{ CoinSelector, Candidate, TR_KEYSPEND_TXIN_WEIGHT}; +use bdk_coin_select::{ CoinSelector, Candidate, TR_KEYSPEND_TXIN_WEIGHT, Drain, FeeRate, Target, ChangePolicy, TargetOutputs, TargetFee, DrainWeights}; use bitcoin::{ Address, Network, Transaction, TxIn, TxOut }; -// The address where we want to send our coins. let recipient_addr = Address::from_str("tb1pvjf9t34fznr53u5tqhejz4nr69luzkhlvsdsdfq9pglutrpve2xq7hps46").unwrap(); +let outputs = vec![TxOut { + value: 3_500_000, + script_pubkey: recipient_addr.payload.script_pubkey(), +}]; + +let target = Target { + outputs: TargetOutputs::fund_outputs(outputs.iter().map(|output| (output.weight() as u32, output.value))), + fee: TargetFee::from_feerate(FeeRate::from_sat_per_vb(42.0)) +}; + let candidates = vec![ Candidate { // How many inputs does this candidate represents. Needed so we can @@ -28,9 +32,9 @@ let candidates = vec![ input_count: 1, // the value of the input value: 1_000_000, - // the total weight of the input(s). + // the total weight of the input(s) including their witness/scriptSig // you may need to use miniscript to figure out the correct value here. - weight: TR_KEYSPEND_TXIN_WEIGHT, + weight: TR_KEYSPEND_TXIN_WEIGHT, // wether it's a segwit input. Needed so we know whether to include the // segwit header in total weight calculations. is_segwit: true @@ -45,91 +49,54 @@ let candidates = vec![ } ]; -let base_tx = Transaction { - input: vec![], - // include your recipient outputs here - output: vec![TxOut { - value: 900_000, - script_pubkey: recipient_addr.payload.script_pubkey(), - }], - lock_time: bitcoin::absolute::LockTime::from_height(0).unwrap(), - version: 0x02, -}; -let base_weight = base_tx.weight().to_wu() as u32; -println!("base weight: {}", base_weight); - // You can now select coins! -let mut coin_selector = CoinSelector::new(&candidates, base_weight); +let mut coin_selector = CoinSelector::new(&candidates); coin_selector.select(0); + +assert!(!coin_selector.is_target_met(target), "we didn't select enough"); +println!("we didn't select enough yet we're missing: {}", coin_selector.missing(target)); +coin_selector.select(1); +assert!(coin_selector.is_target_met(target), "we should have enough now"); + +// Now we need to know if we need a change output to drain the excess if we overshot too much +// +// We don't need to know exactly which change output we're going to use yet but we assume it's a taproot output +// that we'll use a keyspend to spend from. +let drain_weights = DrainWeights::TR_KEYSPEND; +// Our policy is to only add a change output if the value is over 1_000 sats +let change_policy = ChangePolicy::min_value(drain_weights, 1_000); +let change = coin_selector.drain(target, change_policy); +if change.is_some() { + println!("We need to add our change output to the transaction with {} value", change.value); +} else { + println!("Yay we don't need to add a change output"); +} ``` -## Change Policy +## Automatic selection with Branch and Bound -A change policy determines whether the drain output(s) should be in the final solution. The -determination is simple: if the excess value is above a threshold then the drain should be added. To -construct a change policy you always provide `DrainWeights` which tell the coin selector the weight -cost of adding the drain. `DrainWeights` includes two weights. One is the weight of the drain -output(s). The other is the weight of spending the drain output later on (the input weight). +You can use methods such as [`CoinSelector::select`] to manually select coins, or methods such as +[`CoinSelector::select_until_target_met`] for a rudimentary automatic selection. Probably you want +to use [`CoinSelector::run_bnb`] to do this in a smart way. +Built-in metrics are provided in the [`metrics`] submodule. Currently, only the +[`LowestFee`](metrics::LowestFee) metric is considered stable. Note you *can* try and write your own +metric by implementing the [`BnbMetric`] yourself but we don't recommend this. ```rust use std::str::FromStr; -use bdk_coin_select::{CoinSelector, Candidate, DrainWeights, TXIN_BASE_WEIGHT, ChangePolicy, TR_KEYSPEND_TXIN_WEIGHT}; -use bitcoin::{Address, Network, Transaction, TxIn, TxOut}; -let base_tx = Transaction { - input: vec![], - output: vec![/* include your recipient outputs here */], - lock_time: bitcoin::absolute::LockTime::from_height(0).unwrap(), - version: 0x02, -}; -let base_weight = base_tx.weight().to_wu() as u32; - -// The change output that may or may not be included in the final transaction. -let drain_addr = - Address::from_str("tb1pvjf9t34fznr53u5tqhejz4nr69luzkhlvsdsdfq9pglutrpve2xq7hps46") - .expect("address must be valid") - .require_network(Network::Testnet) - .expect("network must match"); - -// The drain output(s) may or may not be included in the final tx. We calculate -// the drain weight to include the output length varint weight changes from -// including the drain output(s). -let drain_output_weight = { - let mut tx_with_drain = base_tx.clone(); - tx_with_drain.output.push(TxOut { - script_pubkey: drain_addr.script_pubkey(), - ..Default::default() - }); - tx_with_drain.weight().to_wu() as u32 - base_weight -}; -println!("drain output weight: {}", drain_output_weight); - -let drain_weights = DrainWeights { - output_weight: drain_output_weight, - spend_weight: TR_KEYSPEND_TXIN_WEIGHT, -}; - -// This constructs a change policy that creates change when the change value is -// greater than or equal to the dust limit. -let change_policy = ChangePolicy::min_value( - drain_weights, - drain_addr.script_pubkey().dust_value().to_sat(), -); -``` - -## Branch and Bound +use bdk_coin_select::{ Candidate, CoinSelector, FeeRate, Target, TargetFee, TargetOutputs, ChangePolicy, TR_KEYSPEND_TXIN_WEIGHT, TR_DUST_RELAY_MIN_VALUE}; +use bdk_coin_select::metrics::LowestFee; +use bitcoin::{ Address, Network, Transaction, TxIn, TxOut }; -You can use methods such as [`CoinSelector::select`] to manually select coins, or methods such as -[`CoinSelector::select_until_target_met`] for a rudimentary automatic selection. However, if you -wish to automatically select coins to optimize for a given metric, [`CoinSelector::run_bnb`] can be -used. +let recipient_addr = + Address::from_str("tb1pvjf9t34fznr53u5tqhejz4nr69luzkhlvsdsdfq9pglutrpve2xq7hps46").unwrap(); -Built-in metrics are provided in the [`metrics`] submodule. Currently, only the -[`LowestFee`](metrics::LowestFee) metric is considered stable. +let outputs = vec![TxOut { + value: 210_000, + script_pubkey: recipient_addr.payload.script_pubkey(), +}]; -```rust -use bdk_coin_select::{ Candidate, CoinSelector, FeeRate, Target, TargetFee, ChangePolicy, TR_KEYSPEND_TXIN_WEIGHT }; -use bdk_coin_select::metrics::LowestFee; let candidates = [ Candidate { input_count: 1, @@ -150,21 +117,24 @@ let candidates = [ is_segwit: true } ]; -let base_weight = 0; let drain_weights = bdk_coin_select::DrainWeights::default(); -let dust_limit = 0; +// You could determine this by looking at the user's transaction history and taking an average of the feerate. let long_term_feerate = FeeRate::from_sat_per_vb(10.0); -let mut coin_selector = CoinSelector::new(&candidates, base_weight); +let mut coin_selector = CoinSelector::new(&candidates); let target = Target { fee: TargetFee::from_feerate(FeeRate::from_sat_per_vb(15.0)), - value: 210_000, + outputs: TargetOutputs::fund_outputs(outputs.iter().map(|output| (output.weight() as u32, output.value))), }; +// The change output must be at least this size to be relayed. +// To choose it you need to know the kind of script pubkey on your change txout. +// Here we assume it's a taproot output +let dust_limit = TR_DUST_RELAY_MIN_VALUE; + // We use a change policy that introduces a change output if doing so reduces -// the "waste" and that the change output's value is at least that of the -// `dust_limit`. +// the "waste" (i.e. adding change doesn't increase the fees we'd pay if we factor in the cost to spend the output later on). let change_policy = ChangePolicy::min_value_and_waste( drain_weights, dust_limit, @@ -172,12 +142,10 @@ let change_policy = ChangePolicy::min_value_and_waste( long_term_feerate, ); -// This metric minimizes transaction fees paid over time. The -// `long_term_feerate` is used to calculate the additional fee from spending -// the change output in the future. +// The LowestFee metric tries make selections that minimize your total fees paid over time. let metric = LowestFee { target, - long_term_feerate, + long_term_feerate, // used to calculate the cost of spending th change output if the future change_policy }; @@ -203,79 +171,6 @@ match coin_selector.run_bnb(metric, 100_000) { ``` -## Finalizing a Selection - -- [`is_target_met`] checks whether the current state of [`CoinSelector`] meets the [`Target`]. -- [`apply_selection`] applies the selection to the original list of candidate `TxOut`s. - -[`is_target_met`]: crate::CoinSelector::is_target_met -[`apply_selection`]: crate::CoinSelector::apply_selection -[`CoinSelector`]: crate::CoinSelector -[`Target`]: crate::Target - -```rust -use bdk_coin_select::{CoinSelector, Candidate, DrainWeights, Target, ChangePolicy, TR_KEYSPEND_TXIN_WEIGHT, Drain}; -use bitcoin::{Amount, TxOut, Address}; -let base_weight = 0_u32; -let drain_weights = DrainWeights::TR_KEYSPEND; -use core::str::FromStr; - -// A random target, as an example. -let target = Target { - value: 21_000, - ..Default::default() -}; -// Am arbitary drain policy, for the example. -let change_policy = ChangePolicy::min_value(drain_weights, 1337); - -// This is a list of candidate txouts for coin selection. If a txout is picked, -// our transaction's input will spend it. -let candidate_txouts = vec![ - TxOut { - value: 100_000, - script_pubkey: Address::from_str("bc1p5cyxnuxmeuwuvkwfem96lqzszd02n6xdcjrs20cac6yqjjwudpxqkedrcr").unwrap().payload.script_pubkey(), - }, - TxOut { - value: 150_000, - script_pubkey: Address::from_str("bc1p4qhjn9zdvkux4e44uhx8tc55attvtyu358kutcqkudyccelu0was9fqzwh").unwrap().payload.script_pubkey(), - }, - TxOut { - value: 200_000, - script_pubkey: Address::from_str("bc1p0d0rhyynq0awa9m8cqrcr8f5nxqx3aw29w4ru5u9my3h0sfygnzs9khxz8").unwrap().payload.script_pubkey() - } -]; -// We transform the candidate txouts into something `CoinSelector` can -// understand. -let candidates = candidate_txouts - .iter() - .map(|txout| Candidate { - input_count: 1, - value: txout.value, - weight: TR_KEYSPEND_TXIN_WEIGHT, // you need to figure out the weight of the txin somehow - is_segwit: txout.script_pubkey.is_witness_program(), - }) - .collect::>(); - -let mut selector = CoinSelector::new(&candidates, base_weight); -selector - .select_until_target_met(target) - .expect("we've got enough coins"); - -// Get a list of coins that are selected. -let selected_coins = selector - .apply_selection(&candidate_txouts) - .collect::>(); -assert_eq!(selected_coins.len(), 1); - -// Determine whether we should add a change output. -let drain = selector.drain(target, change_policy); - -if drain.is_some() { - // add our change output to the transaction - let change_value = drain.value; -} -``` - # Minimum Supported Rust Version (MSRV) This library is compiles on rust v1.54 and above diff --git a/src/change_policy.rs b/src/change_policy.rs index 026c1b3..268d3bc 100644 --- a/src/change_policy.rs +++ b/src/change_policy.rs @@ -39,7 +39,11 @@ impl ChangePolicy { ) -> Self { // The output waste of a changeless solution is the excess. let waste_with_change = drain_weights - .waste(target_feerate, long_term_feerate) + .waste( + target_feerate, + long_term_feerate, + 0, /* ignore varint cost for now */ + ) .ceil() as u64; Self { diff --git a/src/coin_selector.rs b/src/coin_selector.rs index d082410..1055ba8 100644 --- a/src/coin_selector.rs +++ b/src/coin_selector.rs @@ -1,9 +1,7 @@ use super::*; #[allow(unused)] // some bug in <= 1.48.0 sees this as unused when it isn't use crate::float::FloatExt; -use crate::{ - bnb::BnbMetric, change_policy::ChangePolicy, float::Ordf32, FeeRate, Target, TargetFee, -}; +use crate::{bnb::BnbMetric, change_policy::ChangePolicy, float::Ordf32, FeeRate, Target}; use alloc::{borrow::Cow, collections::BTreeSet, vec::Vec}; /// [`CoinSelector`] selects/deselects coins from a set of canididate coins. @@ -15,7 +13,6 @@ use alloc::{borrow::Cow, collections::BTreeSet, vec::Vec}; /// [`bnb_solutions`]: CoinSelector::bnb_solutions #[derive(Debug, Clone)] pub struct CoinSelector<'a> { - base_weight: u32, candidates: &'a [Candidate], selected: Cow<'a, BTreeSet>, banned: Cow<'a, BTreeSet>, @@ -34,9 +31,8 @@ impl<'a> CoinSelector<'a> { /// /// Note that methods in `CoinSelector` will refer to inputs by the index in the `candidates` /// slice you pass in. - pub fn new(candidates: &'a [Candidate], base_weight: u32) -> Self { + pub fn new(candidates: &'a [Candidate]) -> Self { Self { - base_weight, candidates, selected: Cow::Owned(Default::default()), banned: Cow::Owned(Default::default()), @@ -44,34 +40,6 @@ impl<'a> CoinSelector<'a> { } } - /// Creates a new coin selector from some candidate inputs and a list of `output_weights`. - /// - /// This is a convenience method to calculate the `base_weight` from a set of recipient output - /// weights. This is equivalent to calculating the `base_weight` yourself and calling - /// [`CoinSelector::new`]. - pub fn fund_outputs( - candidates: &'a [Candidate], - output_weights: impl IntoIterator, - ) -> Self { - let (output_count, output_weight_total) = output_weights - .into_iter() - .fold((0_usize, 0_u32), |(n, w), a| (n + 1, w + a)); - - let base_weight = (4 /* nVersion */ - + 4 /* nLockTime */ - + varint_size(0) /* inputs varint */ - + varint_size(output_count)/* outputs varint */) - * 4 - + output_weight_total; - - Self::new(candidates, base_weight) - } - - /// The weight of the transaction without any inputs and without a change output. - pub fn base_weight(&self) -> u32 { - self.base_weight - } - /// Iterate over all the candidates in their currently sorted order. Each item has the original /// index with the candidate. pub fn candidates( @@ -166,10 +134,9 @@ impl<'a> CoinSelector<'a> { pub fn input_weight(&self) -> u32 { let is_segwit_tx = self.selected().any(|(_, wv)| wv.is_segwit); let witness_header_extra_weight = is_segwit_tx as u32 * 2; - let vin_count_varint_extra_weight = { - let input_count = self.selected().map(|(_, wv)| wv.input_count).sum::(); - (varint_size(input_count) - 1) * 4 - }; + + let input_count = self.selected().map(|(_, wv)| wv.input_count).sum::(); + let input_varint_weight = varint_size(input_count) * 4; let selected_weight: u32 = self .selected() @@ -184,7 +151,7 @@ impl<'a> CoinSelector<'a> { }) .sum(); - selected_weight + witness_header_extra_weight + vin_count_varint_extra_weight + input_varint_weight + selected_weight + witness_header_extra_weight } /// Absolute value sum of all selected inputs. @@ -195,26 +162,41 @@ impl<'a> CoinSelector<'a> { .sum() } - /// Current weight of template tx + selected inputs. - pub fn weight(&self, drain_weight: u32) -> u32 { - self.base_weight + self.input_weight() + drain_weight + /// Current weight of transaction implied by the selection. + /// + /// If you don't have any drain outputs (only target outputs) just set drain_weights tio [`DrainWeights::NONE`]. + pub fn weight(&self, target_ouputs: TargetOutputs, drain_weight: DrainWeights) -> u32 { + TX_FIXED_FIELD_WEIGHT + + self.input_weight() + + target_ouputs.output_weight_with_drain(drain_weight) } /// How much the current selection overshoots the value needed to achieve `target`. /// - /// In order for the resulting transaction to be valid this must be 0. + /// In order for the resulting transaction to be valid this must be 0 or above. If it's above 0 + /// this means the transaction will overpay for what it needs to reach `target`. pub fn excess(&self, target: Target, drain: Drain) -> i64 { self.rate_excess(target, drain) .min(self.replacement_excess(target, drain)) } + /// How much extra value needs to be selected to reach the target. + pub fn missing(&self, target: Target) -> u64 { + let excess = self.excess(target, Drain::NONE); + if excess < 0 { + excess.unsigned_abs() + } else { + 0 + } + } + /// How much the current selection overshoots the value need to satisfy `target.fee.rate` and /// `target.value` (while ignoring `target.min_fee`). pub fn rate_excess(&self, target: Target, drain: Drain) -> i64 { self.selected_value() as i64 - - target.value as i64 + - target.value() as i64 - drain.value as i64 - - self.implied_fee_from_feerate(target.fee.rate, drain.weights.output_weight) as i64 + - self.implied_fee_from_feerate(target, drain.weights) as i64 } /// How much the current selection overshoots the value needed to satisfy RBF's rule 4. @@ -222,21 +204,22 @@ impl<'a> CoinSelector<'a> { let mut replacement_excess_needed = 0; if let Some(replace) = target.fee.replace { replacement_excess_needed = - replace.min_fee_to_do_replacement(self.weight(drain.weights.output_weight)) + replace.min_fee_to_do_replacement(self.weight(target.outputs, drain.weights)) } self.selected_value() as i64 - - target.value as i64 + - target.value() as i64 - drain.value as i64 - replacement_excess_needed as i64 } /// The feerate the transaction would have if we were to use this selection of inputs to achieve - /// the `target_value`. + /// the `target`'s value and weight. It is essentially telling you what target feerate you currently have. /// /// Returns `None` if the feerate would be negative or infinity. - pub fn implied_feerate(&self, target_value: u64, drain: Drain) -> Option { - let numerator = self.selected_value() as i64 - target_value as i64 - drain.value as i64; - let denom = self.weight(drain.weights.output_weight); + pub fn implied_feerate(&self, target_outputs: TargetOutputs, drain: Drain) -> Option { + let numerator = + self.selected_value() as i64 - target_outputs.value_sum as i64 - drain.value as i64; + let denom = self.weight(target_outputs, drain.weights); if numerator < 0 || denom == 0 { return None; } @@ -246,22 +229,19 @@ impl<'a> CoinSelector<'a> { /// The fee the current selection and `drain_weight` should pay to satisfy `target_fee`. /// /// `drain_weight` can be 0 to indicate no draining output - pub fn implied_fee(&self, target_fee: TargetFee, drain_weight: u32) -> u64 { - let mut implied_fee = self.implied_fee_from_feerate(target_fee.rate, drain_weight); + pub fn implied_fee(&self, target: Target, drain_weights: DrainWeights) -> u64 { + let mut implied_fee = self.implied_fee_from_feerate(target, drain_weights); - if let Some(replace) = target_fee.replace { - implied_fee = implied_fee.max(self.implied_fee_from_replace(replace, drain_weight)); + if let Some(replace) = target.fee.replace { + implied_fee = + replace.min_fee_to_do_replacement(self.weight(target.outputs, drain_weights)); } implied_fee } - fn implied_fee_from_replace(&self, replace: Replace, drain_weight: u32) -> u64 { - replace.min_fee_to_do_replacement(self.weight(drain_weight)) - } - - fn implied_fee_from_feerate(&self, feerate: FeeRate, drain_weight: u32) -> u64 { - (self.weight(drain_weight) as f32 * feerate.spwu()).ceil() as u64 + fn implied_fee_from_feerate(&self, target: Target, drain_weights: DrainWeights) -> u64 { + (self.weight(target.outputs, drain_weights) as f32 * target.fee.rate.spwu()).ceil() as u64 } /// The actual fee the selection would pay if it was used in a transaction that had @@ -348,8 +328,10 @@ impl<'a> CoinSelector<'a> { excess_waste *= excess_discount.max(0.0).min(1.0); waste += excess_waste; } else { - waste += drain.weights.output_weight as f32 * target.fee.rate.spwu() - + drain.weights.spend_weight as f32 * long_term_feerate.spwu(); + waste += + drain + .weights + .waste(target.fee.rate, long_term_feerate, target.outputs.n_outputs); } waste @@ -468,7 +450,7 @@ impl<'a> CoinSelector<'a> { weights: change_policy.drain_weights, value, }, - None => Drain::none(), + None => Drain::NONE, } } diff --git a/src/drain.rs b/src/drain.rs index e1871df..d59d119 100644 --- a/src/drain.rs +++ b/src/drain.rs @@ -1,4 +1,4 @@ -use crate::{FeeRate, TR_KEYSPEND_TXIN_WEIGHT, TR_SPK_WEIGHT, TXOUT_BASE_WEIGHT}; +use crate::{varint_size, FeeRate, TR_KEYSPEND_TXIN_WEIGHT, TR_SPK_WEIGHT, TXOUT_BASE_WEIGHT}; /// Represents the weight costs of a drain (a.k.a. change) output. /// @@ -11,20 +11,41 @@ pub struct DrainWeights { pub output_weight: u32, /// The weight of spending this drain output (in the future). pub spend_weight: u32, + /// The total number of outputs that the drain will use + pub n_outputs: usize, } impl DrainWeights { - /// `DrainWeights` that represents a drain output that will be spent with a taproot keyspend + /// `DrainWeights` for an output that will be spent with a taproot keyspend pub const TR_KEYSPEND: Self = Self { output_weight: TXOUT_BASE_WEIGHT + TR_SPK_WEIGHT, spend_weight: TR_KEYSPEND_TXIN_WEIGHT, + n_outputs: 1, + }; + + /// `DrainWeights` for no drain at all + pub const NONE: Self = Self { + output_weight: 0, + spend_weight: 0, + n_outputs: 0, }; /// The waste of adding this drain to a transaction according to the [waste metric]. /// + /// To get the precise answer you need to pass in the number of non-drain outputs (`n_target_outputs`) that you're + /// adding to the transaction so we can include the cost of increasing the varint size of the output length. + /// /// [waste metric]: https://bitcoin.stackexchange.com/questions/113622/what-does-waste-metric-mean-in-the-context-of-coin-selection - pub fn waste(&self, feerate: FeeRate, long_term_feerate: FeeRate) -> f32 { - self.output_weight as f32 * feerate.spwu() + pub fn waste( + &self, + feerate: FeeRate, + long_term_feerate: FeeRate, + n_target_outputs: usize, + ) -> f32 { + let extra_varint_weight = + (varint_size(n_target_outputs + self.n_outputs) - varint_size(n_target_outputs)) * 4; + let extra_output_weight = self.output_weight + extra_varint_weight; + extra_output_weight as f32 * feerate.spwu() + self.spend_weight as f32 * long_term_feerate.spwu() } @@ -50,6 +71,13 @@ pub struct Drain { } impl Drain { + /// The drain which represents no drain at all. We could but don't use `Option` because this + /// causes friction internally, instead we just use a `Drain` with all 0 values. + pub const NONE: Self = Drain { + weights: DrainWeights::NONE, + value: 0, + }; + /// A drian representing no drain at all. pub fn none() -> Self { Self::default() @@ -57,18 +85,11 @@ impl Drain { /// is the "none" drain pub fn is_none(&self) -> bool { - self == &Drain::none() + self == &Drain::NONE } /// Is not the "none" drain pub fn is_some(&self) -> bool { !self.is_none() } - - /// The waste of adding this drain to a transaction according to the [waste metric]. - /// - /// [waste metric]: https://bitcoin.stackexchange.com/questions/113622/what-does-waste-metric-mean-in-the-context-of-coin-selection - pub fn waste(&self, feerate: FeeRate, long_term_feerate: FeeRate) -> f32 { - self.weights.waste(feerate, long_term_feerate) - } } diff --git a/src/lib.rs b/src/lib.rs index 1106cbc..f7b109d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -41,18 +41,25 @@ pub const TXOUT_BASE_WEIGHT: u32 = // The spk length + (4 * 1); -/// The additional weight over [`TXIN_BASE_WEIGHT`] incurred by satisfying an input with a keyspend -/// and the default sighash. -pub const TR_KEYSPEND_SATISFACTION_WEIGHT: u32 = 66; +/// The weight of the `nVersion` and `nLockTime` transaction fields +pub const TX_FIXED_FIELD_WEIGHT: u32 = (4 /* nVersion */ + 4/* nLockTime */) * 4; -/// The additional weight of an output with segwit `v1` (taproot) script pubkey over a blank output (i.e. with weight [`TXOUT_BASE_WEIGHT`]). +/// The weight of a taproot keyspend witness +pub const TR_KEYSPEND_SATISFACTION_WEIGHT: u32 = 1 /*witness_len*/ + 1 /*item len*/ + 64 /*signature*/; + +/// The weight of a segwit `v1` (taproot) script pubkey in an outupt. This does not include the weight of +/// the `TxOut` itself or the script pubkey length field. pub const TR_SPK_WEIGHT: u32 = (1 + 1 + 32) * 4; // version + push + key /// The weight of a taproot TxIn with witness pub const TR_KEYSPEND_TXIN_WEIGHT: u32 = TXIN_BASE_WEIGHT + TR_KEYSPEND_SATISFACTION_WEIGHT; +/// The minimum value a taproot output can have to be relayed with Bitcoin core's default dust relay +/// fee +pub const TR_DUST_RELAY_MIN_VALUE: u64 = 330; + /// Helper to calculate varint size. `v` is the value the varint represents. -fn varint_size(v: usize) -> u32 { +const fn varint_size(v: usize) -> u32 { if v <= 0xfc { return 1; } diff --git a/src/metrics.rs b/src/metrics.rs index 8fb0d03..4554e3d 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -3,8 +3,6 @@ use crate::{ bnb::BnbMetric, change_policy::ChangePolicy, float::Ordf32, CoinSelector, Drain, Target, }; -mod waste; -pub use waste::*; mod lowest_fee; pub use lowest_fee::*; mod changeless; diff --git a/src/metrics/changeless.rs b/src/metrics/changeless.rs index fec7e6f..3dd784a 100644 --- a/src/metrics/changeless.rs +++ b/src/metrics/changeless.rs @@ -1,6 +1,7 @@ use super::change_lower_bound; use crate::{bnb::BnbMetric, change_policy::ChangePolicy, float::Ordf32, CoinSelector, Target}; +#[derive(Clone, Debug)] /// Metric for finding changeless solutions only. pub struct Changeless { /// The target parameters for the resultant selection. diff --git a/src/metrics/lowest_fee.rs b/src/metrics/lowest_fee.rs index d1c3e1e..30e4f21 100644 --- a/src/metrics/lowest_fee.rs +++ b/src/metrics/lowest_fee.rs @@ -32,7 +32,7 @@ impl BnbMetric for LowestFee { let long_term_fee = { let drain = cs.drain(self.target, self.change_policy); - let fee_for_the_tx = cs.fee(self.target.value, drain.value); + let fee_for_the_tx = cs.fee(self.target.value(), drain.value); assert!( fee_for_the_tx > 0, "must not be called unless selection has met target" @@ -80,10 +80,11 @@ impl BnbMetric for LowestFee { // the cost of removing the change output let cost_of_getting_rid_of_change = extra_value_needed_to_get_rid_of_change + drain_value as f32; - let cost_of_change = self - .change_policy - .drain_weights - .waste(self.target.fee.rate, self.long_term_feerate); + let cost_of_change = self.change_policy.drain_weights.waste( + self.target.fee.rate, + self.long_term_feerate, + self.target.outputs.n_outputs, + ); let best_score_without_change = Ordf32( current_score.0 + cost_of_getting_rid_of_change - cost_of_change, ); @@ -94,10 +95,11 @@ impl BnbMetric for LowestFee { } } else { // Ok but maybe adding change could improve the metric? - let cost_of_adding_change = self - .change_policy - .drain_weights - .waste(self.target.fee.rate, self.long_term_feerate); + let cost_of_adding_change = self.change_policy.drain_weights.waste( + self.target.fee.rate, + self.long_term_feerate, + self.target.outputs.n_outputs, + ); let cost_of_no_change = cs.excess(self.target, Drain::none()); let best_score_with_change = @@ -167,7 +169,7 @@ impl BnbMetric for LowestFee { assert!(scale.0 > 0.0); let ideal_fee = scale.0 * to_resize.value as f32 + cs.selected_value() as f32 - - self.target.value as f32; + - self.target.value() as f32; assert!(ideal_fee >= 0.0); Some(Ordf32(ideal_fee)) diff --git a/src/metrics/waste.rs b/src/metrics/waste.rs deleted file mode 100644 index 3d6483d..0000000 --- a/src/metrics/waste.rs +++ /dev/null @@ -1,243 +0,0 @@ -use super::change_lower_bound; -use crate::{ - bnb::BnbMetric, float::Ordf32, Candidate, ChangePolicy, CoinSelector, Drain, FeeRate, Target, -}; - -/// The "waste" metric used by bitcoin core. -/// -/// See this [great explanation](https://bitcoin.stackexchange.com/questions/113622/what-does-waste-metric-mean-in-the-context-of-coin-selection) -/// for an understanding of the waste metric. -/// -/// ## WARNING: Waste metric considered wasteful -/// -/// Note that bitcoin core at the time of writing use the waste metric to -/// -/// 1. minimise the waste while searching for changeless solutions. -/// 2. It tiebreaks multiple valid selections from different algorithms (which do not try and -/// minimise waste) with waste. -/// -/// This is **very** different from minimising waste in general which is what this metric will do -/// when used in [`CoinSelector::bnb_solutions`]. The waste metric tends to over consolidate funds. -/// If the `long_term_feerate` is even slightly higher than the current feerate (specified in -/// `target`) it will select all your coins! -#[derive(Clone, Copy, Debug)] -pub struct Waste { - /// The target parameters of the resultant selection. - pub target: Target, - /// The longterm feerate as part of the waste metric. - pub long_term_feerate: FeeRate, - /// Policy to determine the change output (if any) of a given selection. - pub change_policy: ChangePolicy, -} - -impl BnbMetric for Waste { - fn score(&mut self, cs: &CoinSelector<'_>) -> Option { - let drain = cs.drain(self.target, self.change_policy); - if !cs.is_target_met_with_drain(self.target, drain) { - return None; - } - let score = cs.waste(self.target, self.long_term_feerate, drain, 1.0); - Some(Ordf32(score)) - } - - fn bound(&mut self, cs: &CoinSelector<'_>) -> Option { - // Welcome my bretheren. This dungeon was authored by Lloyd Fournier A.K.A "LLFourn" with - // the assistance of chat GPT and the developers of the IOTA cryptocurrency. There are - // comments trying to make sense of the logic here but it's really just me pretending I know - // what's going on. I have tried to simplify the logic here many times but always end up - // making it fail proptests. - // - // Don't be afraid. This function is a "heuristic" lower bound. It doesn't need to be super - // duper correct. In testing it seems to come up with pretty good results pretty fast. - let rate_diff = self.target.fee.rate.spwu() - self.long_term_feerate.spwu(); - // whether from this coin selection it's possible to avoid change - let change_lower_bound = change_lower_bound(cs, self.target, self.change_policy); - const IGNORE_EXCESS: f32 = 0.0; - const INCLUDE_EXCESS: f32 = 1.0; - - if rate_diff >= 0.0 { - // Our lower bound algorithms differ depending on whether we have already met the target or not. - if cs.is_target_met_with_drain(self.target, change_lower_bound) { - let current_change = cs.drain(self.target, self.change_policy); - - // first lower bound candidate is just the selection itself - let mut lower_bound = cs.waste( - self.target, - self.long_term_feerate, - current_change, - INCLUDE_EXCESS, - ); - - // But don't stop there we might be able to select negative value inputs which might - // lower excess and reduce waste either by: - // - removing the need for a change output - // - reducing the excess if the current selection is changeless (only possible when rate_diff is small). - let should_explore_changeless = change_lower_bound.is_none(); - - if should_explore_changeless { - let selection_with_as_much_negative_ev_as_possible = cs - .clone() - .select_iter() - .rev() - .take_while(|(cs, _, wv)| { - wv.effective_value(self.target.fee.rate) < 0.0 - && cs.is_target_met(self.target) - }) - .last(); - - if let Some((cs, _, _)) = selection_with_as_much_negative_ev_as_possible { - let can_do_better_by_slurping = - cs.unselected().next_back().and_then(|(_, wv)| { - if wv.effective_value(self.target.fee.rate) < 0.0 { - Some(wv) - } else { - None - } - }); - let lower_bound_without_change = match can_do_better_by_slurping { - Some(finishing_input) => { - // NOTE we are slurping negative value here to try and reduce excess in - // the hopes of getting rid of the change output - let value_to_slurp = -cs.rate_excess(self.target, Drain::none()); - let weight_to_extinguish_excess = - slurp_wv(finishing_input, value_to_slurp, self.target.fee.rate); - let waste_to_extinguish_excess = - weight_to_extinguish_excess * rate_diff; - // return: waste after excess reduction - cs.waste( - self.target, - self.long_term_feerate, - Drain::none(), - IGNORE_EXCESS, - ) + waste_to_extinguish_excess - } - None => cs.waste( - self.target, - self.long_term_feerate, - Drain::none(), - INCLUDE_EXCESS, - ), - }; - - lower_bound = lower_bound.min(lower_bound_without_change); - } - } - - Some(Ordf32(lower_bound)) - } else { - // If feerate >= long_term_feerate, You *might* think that the waste lower bound - // here is just the fewest number of inputs we need to meet the target but **no**. - // Consider if there is 1 sat remaining to reach target. Should you add all the - // weight of the next input for the waste calculation? *No* this leaads to a - // pesimistic lower bound even if we ignore the excess because it adds too much - // weight. - // - // Step 1: select everything up until the input that hits the target. - let (mut cs, slurp_index, to_slurp) = - cs.clone().select_iter().find(|(cs, _, _)| { - cs.is_target_met_with_drain(self.target, change_lower_bound) - })?; - - cs.deselect(slurp_index); - - // Step 2: We pretend that the final input exactly cancels out the remaining excess - // by taking whatever value we want from it but at the value per weight of the real - // input. - let ideal_next_weight = { - // satisfying absolute and feerate constraints requires different calculations so we do them - // both independently and find which requires the most weight of the next input. - let remaining_rate = cs.rate_excess(self.target, change_lower_bound); - let remaining_abs = cs.replacement_excess(self.target, change_lower_bound); - - let weight_to_satisfy_abs = remaining_abs.min(0) as f32 / to_slurp.value_pwu(); - - let weight_to_satisfy_rate = - slurp_wv(to_slurp, remaining_rate.min(0), self.target.fee.rate); - - let weight_to_satisfy = weight_to_satisfy_abs.max(weight_to_satisfy_rate); - debug_assert!(weight_to_satisfy <= to_slurp.weight as f32); - weight_to_satisfy - }; - let weight_lower_bound = cs.input_weight() as f32 + ideal_next_weight; - let mut waste = weight_lower_bound * rate_diff; - waste += change_lower_bound.waste(self.target.fee.rate, self.long_term_feerate); - - Some(Ordf32(waste)) - } - } else { - // When long_term_feerate > current feerate each input by itself has negative waste. - // This doesn't mean that waste monotonically decreases as you add inputs because - // somewhere along the line adding an input might cause the change policy to add a - // change ouput which could increase waste. - // - // So we have to try two things and we which one is best to find the lower bound: - // 1. try selecting everything regardless of change - let mut lower_bound = { - let mut cs = cs.clone(); - // ... but first check that by selecting all effective we can actually reach target - cs.select_all_effective(self.target.fee.rate); - if !cs.is_target_met(self.target) { - return None; - } - let change_at_value_optimum = cs.drain(self.target, self.change_policy); - cs.select_all(); - // NOTE: we use the change from our "all effective" selection for min waste since - // selecting all might not have change but in that case we'll catch it below. - cs.waste( - self.target, - self.long_term_feerate, - change_at_value_optimum, - IGNORE_EXCESS, - ) - }; - - let look_for_changeless_solution = change_lower_bound.is_none(); - - if look_for_changeless_solution { - // 2. select the highest weight solution with no change - let highest_weight_selection_without_change = cs - .clone() - .select_iter() - .rev() - .take_while(|(cs, _, wv)| { - wv.effective_value(self.target.fee.rate) < 0.0 - || cs.drain_value(self.target, self.change_policy).is_none() - }) - .last(); - - if let Some((cs, _, _)) = highest_weight_selection_without_change { - let no_change_waste = cs.waste( - self.target, - self.long_term_feerate, - Drain::none(), - IGNORE_EXCESS, - ); - - lower_bound = lower_bound.min(no_change_waste) - } - } - - Some(Ordf32(lower_bound)) - } - } - - fn requires_ordering_by_descending_value_pwu(&self) -> bool { - true - } -} - -/// Returns the "perfect weight" for this candidate to slurp up a given value with `feerate` while -/// not changing the candidate's value/weight ratio. -/// -/// Used to pretend that a candidate had precisely `value_to_slurp` + fee needed to include it. It -/// tells you how much weight such a perfect candidate would have if it had the same value per -/// weight unit as `candidate`. This is useful for estimating a lower weight bound for a perfect -/// match. -fn slurp_wv(candidate: Candidate, value_to_slurp: i64, feerate: FeeRate) -> f32 { - // the value per weight unit this candidate offers at feerate - let value_per_wu = (candidate.value as f32 / candidate.weight as f32) - feerate.spwu(); - // return how much weight we need - let weight_needed = value_to_slurp as f32 / value_per_wu; - debug_assert!(weight_needed <= candidate.weight as f32); - weight_needed.min(0.0) -} diff --git a/src/target.rs b/src/target.rs index 2bbccac..6c4e65e 100644 --- a/src/target.rs +++ b/src/target.rs @@ -1,12 +1,69 @@ -use crate::FeeRate; +use crate::{varint_size, DrainWeights, FeeRate}; /// A target value to select for along with feerate constraints. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd, Default)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd)] pub struct Target { /// The fee constraints that must be satisfied by the selection pub fee: TargetFee, - /// The minmum value that should be left for the output - pub value: u64, + /// The aggregate properties of outputs you're trying to fund + pub outputs: TargetOutputs, +} + +impl Target { + /// The value target that we are trying to fund + pub fn value(&self) -> u64 { + self.outputs.value_sum + } +} + +/// Information about the outputs we're trying to fund. Note the fields are total values since we +/// don't care about the weights or the values of individual outputs for the purposes of coin +/// selection. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd)] +pub struct TargetOutputs { + /// The sum of the values of the individual `TxOuts`s. + pub value_sum: u64, + /// The sum of the weights of the individual `TxOut`s. + pub weight_sum: u32, + /// The total number of outputs + pub n_outputs: usize, +} + +impl TargetOutputs { + /// The output weight of the outptus we're trying to fund + pub fn output_weight(&self) -> u32 { + self.weight_sum + varint_size(self.n_outputs) * 4 + } + + /// The output weight of the target's outputs combined with the drain outputs defined by + /// `drain_weight`. + /// + /// This is not a simple addition of the `drain_weight` and [`output_weight`] because of how + /// adding the drain weights might add an extra vbyte for the length of the varint. + /// + /// [`output_weight`]: Self::output_weight + pub fn output_weight_with_drain(&self, drain_weight: DrainWeights) -> u32 { + let n_outputs = drain_weight.n_outputs + self.n_outputs; + varint_size(n_outputs) * 4 + drain_weight.output_weight + self.weight_sum + } + + /// Creates a `TargetOutputs` from a list of outputs represented as `(weight, value)` pairs. + pub fn fund_outputs(outputs: impl IntoIterator) -> Self { + let mut n_outputs = 0; + let mut weight_sum = 0; + let mut value_sum = 0; + + for (weight, value) in outputs { + n_outputs += 1; + weight_sum += weight; + value_sum += value; + } + Self { + n_outputs, + weight_sum, + value_sum, + } + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd)] @@ -30,6 +87,32 @@ pub struct TargetFee { pub replace: Option, } +impl Default for TargetFee { + /// The default is feerate set is [`FeeRate::DEFAULT_MIN_RELAY`] and doesn't replace anything. + fn default() -> Self { + Self { + rate: FeeRate::DEFAULT_MIN_RELAY, + replace: None, + } + } +} + +impl TargetFee { + /// A target fee of 0 sats per vbyte (and no replacement) + pub const ZERO: Self = TargetFee { + rate: FeeRate::ZERO, + replace: None, + }; + + /// Creates a target fee from a feerate. The target won't include a replacement. + pub fn from_feerate(feerate: FeeRate) -> Self { + Self { + rate: feerate, + replace: None, + } + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd)] /// The weight transaction(s) that this new transaction is replacing including the feerate. pub struct Replace { @@ -58,29 +141,3 @@ impl Replace { self.fee + min_fee_increment } } - -impl Default for TargetFee { - /// The default is feerate set is [`FeeRate::DEFAULT_MIN_RELAY`] and doesn't replace anything. - fn default() -> Self { - Self { - rate: FeeRate::DEFAULT_MIN_RELAY, - replace: None, - } - } -} - -impl TargetFee { - /// A target fee of 0 sats per vbyte (and no replacement) - pub const ZERO: Self = TargetFee { - rate: FeeRate::ZERO, - replace: None, - }; - - /// Creates a target fee from a feerate. The target won't include a replacement. - pub fn from_feerate(feerate: FeeRate) -> Self { - Self { - rate: feerate, - replace: None, - } - } -} diff --git a/tests/bnb.rs b/tests/bnb.rs index 1a2c359..434050c 100644 --- a/tests/bnb.rs +++ b/tests/bnb.rs @@ -1,6 +1,6 @@ mod common; use bdk_coin_select::{ - float::Ordf32, BnbMetric, Candidate, CoinSelector, Drain, Target, TargetFee, + float::Ordf32, BnbMetric, Candidate, CoinSelector, Drain, Target, TargetFee, TargetOutputs, }; #[macro_use] extern crate alloc; @@ -25,6 +25,7 @@ fn test_wv(mut rng: impl RngCore) -> impl Iterator { }) } +/// This is just an exhaustive search struct MinExcessThenWeight { target: Target, } @@ -47,12 +48,7 @@ impl BnbMetric for MinExcessThenWeight { fn bound(&mut self, cs: &CoinSelector<'_>) -> Option { let mut cs = cs.clone(); cs.select_until_target_met(self.target).ok()?; - if let Some(last_index) = cs.selected_indices().iter().last().copied() { - cs.deselect(last_index); - } - Some(Ordf32( - cs.excess(self.target, Drain::none()) as f32 * EXCESS_RATIO + cs.input_weight() as f32, - )) + Some(Ordf32(cs.input_weight() as f32)) } } @@ -71,21 +67,25 @@ fn bnb_finds_an_exact_solution_in_n_iter() { let solution: Vec = (0..solution_len).map(|_| wv.next().unwrap()).collect(); let solution_weight = { - let mut cs = CoinSelector::new(&solution, 0); + let mut cs = CoinSelector::new(&solution); cs.select_all(); cs.input_weight() }; - let target = solution.iter().map(|c| c.value).sum(); + let target_value = solution.iter().map(|c| c.value).sum(); let mut candidates = solution; candidates.extend(wv.take(num_additional_canidates)); candidates.sort_unstable_by_key(|wv| core::cmp::Reverse(wv.value)); - let cs = CoinSelector::new(&candidates, 0); + let cs = CoinSelector::new(&candidates); let target = Target { - value: target, + outputs: TargetOutputs { + value_sum: target_value, + weight_sum: 0, + n_outputs: 1, + }, // we're trying to find an exact selection value so set fees to 0 fee: TargetFee::ZERO, }; @@ -100,23 +100,27 @@ fn bnb_finds_an_exact_solution_in_n_iter() { .last() .expect("it found a solution"); - assert_eq!(rounds, 50168); + assert_eq!(rounds, 3150); assert_eq!(best.input_weight(), solution_weight); - assert_eq!(best.selected_value(), target.value, "score={:?}", score); + assert_eq!(best.selected_value(), target_value, "score={:?}", score); } #[test] fn bnb_finds_solution_if_possible_in_n_iter() { let num_inputs = 18; - let target = 8_314; + let target_value = 8_314; let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha); let wv = test_wv(&mut rng); let candidates = wv.take(num_inputs).collect::>(); - let cs = CoinSelector::new(&candidates, 0); + let cs = CoinSelector::new(&candidates); let target = Target { - value: target, + outputs: TargetOutputs { + value_sum: target_value, + weight_sum: 0, + n_outputs: 1, + }, fee: TargetFee::default(), }; @@ -130,28 +134,28 @@ fn bnb_finds_solution_if_possible_in_n_iter() { .last() .expect("found a solution"); - assert_eq!(rounds, 202); + assert_eq!(rounds, 193); let excess = sol.excess(target, Drain::none()); - assert_eq!(excess, 8); + assert_eq!(excess, 1); } proptest! { #[test] - fn bnb_always_finds_solution_if_possible(num_inputs in 1usize..18, target in 0u64..10_000) { + fn bnb_always_finds_solution_if_possible(num_inputs in 1usize..18, target_value in 0u64..10_000) { let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha); let wv = test_wv(&mut rng); let candidates = wv.take(num_inputs).collect::>(); - let cs = CoinSelector::new(&candidates, 0); + let cs = CoinSelector::new(&candidates); let target = Target { - value: target, + outputs: TargetOutputs { value_sum: target_value, weight_sum: 0, n_outputs: 1 }, fee: TargetFee::ZERO, }; let solutions = cs.bnb_solutions(MinExcessThenWeight { target }); match solutions.enumerate().filter_map(|(i, sol)| Some((i, sol?))).last() { - Some((_i, (sol, _score))) => assert!(sol.selected_value() >= target.value), + Some((_i, (sol, _score))) => assert!(sol.selected_value() >= target_value), _ => prop_assert!(!cs.is_selection_possible(target)), } } @@ -168,17 +172,17 @@ proptest! { let solution: Vec = (0..solution_len).map(|_| wv.next().unwrap()).collect(); let solution_weight = { - let mut cs = CoinSelector::new(&solution, 0); + let mut cs = CoinSelector::new(&solution); cs.select_all(); cs.input_weight() }; - let target = solution.iter().map(|c| c.value).sum(); + let target_value = solution.iter().map(|c| c.value).sum(); let mut candidates = solution; candidates.extend(wv.take(num_additional_canidates)); - let mut cs = CoinSelector::new(&candidates, 0); + let mut cs = CoinSelector::new(&candidates); for i in 0..num_preselected.min(solution_len) { @@ -189,7 +193,7 @@ proptest! { cs.sort_candidates_by_key(|(_, wv)| core::cmp::Reverse(wv.value)); let target = Target { - value: target, + outputs: TargetOutputs { value_sum: target_value, weight_sum: 0, n_outputs: 1 }, // we're trying to find an exact selection value so set fees to 0 fee: TargetFee::ZERO, }; @@ -203,6 +207,6 @@ proptest! { .expect("it found a solution"); prop_assert!(best.input_weight() <= solution_weight); - prop_assert_eq!(best.selected_value(), target.value); + prop_assert_eq!(best.selected_value(), target.value()); } } diff --git a/tests/changeless.rs b/tests/changeless.rs index d98a77c..9026119 100644 --- a/tests/changeless.rs +++ b/tests/changeless.rs @@ -1,8 +1,10 @@ #![allow(unused)] mod common; use bdk_coin_select::{ - float::Ordf32, metrics, Candidate, ChangePolicy, CoinSelector, Drain, DrainWeights, FeeRate, - Target, TargetFee, + float::Ordf32, + metrics::{self, Changeless}, + Candidate, ChangePolicy, CoinSelector, Drain, DrainWeights, FeeRate, Target, TargetFee, + TargetOutputs, }; use proptest::{prelude::*, proptest, test_runner::*}; use rand::{prelude::IteratorRandom, Rng, RngCore}; @@ -21,40 +23,46 @@ fn test_wv(mut rng: impl RngCore) -> impl Iterator { proptest! { #![proptest_config(ProptestConfig { - timeout: 1_000, - cases: 1_000, ..Default::default() })] + #[test] #[cfg(not(debug_assertions))] // too slow if compiling for debug - fn changeless_prop( - num_inputs in 0usize..15, - target in 0u64..15_000, - feerate in 1.0f32..10.0, - replace in common::maybe_replace(0u64..1_000), - base_weight in 0u32..500, - long_term_feerate_diff in -5.0f32..5.0, - change_weight in 1u32..100, - change_spend_weight in 1u32..100, + fn compare_against_benchmarks( + n_candidates in 0..50_usize, // candidates (n) + target_value in 500..1_000_000_u64, // target value (sats) + n_target_outputs in 1..150_usize, // the number of outputs we're funding + target_weight in 0..10_000_u32, // the sum of the weight of the outputs (wu) + replace in common::maybe_replace(0..10_000u64), // The weight of the transaction we're replacing + feerate in 1.0..100.0_f32, // feerate (sats/vb) + feerate_lt_diff in -5.0..50.0_f32, // longterm feerate diff (sats/vb) + drain_weight in 100..=500_u32, // drain weight (wu) + drain_spend_weight in 1..=2000_u32, // drain spend weight (wu) + drain_dust in 100..=1000_u64, // drain dust (sats) + n_drain_outputs in 1..150usize, // the number of drain outputs ) { println!("======================================="); let start = std::time::Instant::now(); let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha); - let long_term_feerate = FeeRate::from_sat_per_vb(0.0f32.max(feerate - long_term_feerate_diff)); let feerate = FeeRate::from_sat_per_vb(feerate); - let drain = DrainWeights { - output_weight: change_weight, - spend_weight: change_spend_weight, + let drain_weights = DrainWeights { + output_weight: drain_weight, + spend_weight: drain_spend_weight, + n_outputs: n_drain_outputs, }; - let change_policy = ChangePolicy::min_value_and_waste(drain, 0, feerate, long_term_feerate); + let change_policy = ChangePolicy::min_value(drain_weights, 100); let wv = test_wv(&mut rng); - let candidates = wv.take(num_inputs).collect::>(); + let candidates = wv.take(n_candidates).collect::>(); - let cs = CoinSelector::new(&candidates, base_weight); + let cs = CoinSelector::new(&candidates); let target = Target { - value: target, + outputs: TargetOutputs { + n_outputs: n_target_outputs, + value_sum: target_value, + weight_sum: target_weight, + }, fee: TargetFee { rate: feerate, replace, @@ -75,23 +83,8 @@ proptest! { match best { - Some((_i, (sol, _score))) => { - let mut cmp_benchmarks = vec![ - { - let mut naive_select = cs.clone(); - naive_select.sort_candidates_by_key(|(_, wv)| core::cmp::Reverse(Ordf32(wv.effective_value(target.fee.rate)))); - // we filter out failing onces below - let _ = naive_select.select_until_target_met(target); - naive_select - }, - ]; - - cmp_benchmarks.extend((0..10).map(|_|random_minimal_selection(&cs, target, long_term_feerate, change_policy, &mut rng))); - - let cmp_benchmarks = cmp_benchmarks.into_iter().filter(|cs| cs.is_target_met(target)); - for (_bench_id, bench) in cmp_benchmarks.enumerate() { - prop_assert!(bench.drain_value(target, change_policy).is_some() >= sol.drain_value(target, change_policy).is_some()); - } + Some((_i, (_sol, _score))) => { + /* there is nothing to check about a changeless solution */ } None => { let mut cs = cs.clone(); @@ -104,23 +97,3 @@ proptest! { dbg!(start.elapsed()); } } - -// this is probably a useful thing to have on CoinSelector but I don't want to design it yet -#[allow(unused)] -fn random_minimal_selection<'a>( - cs: &CoinSelector<'a>, - target: Target, - long_term_feerate: FeeRate, - change_policy: ChangePolicy, - rng: &mut impl RngCore, -) -> CoinSelector<'a> { - let mut cs = cs.clone(); - let mut last_waste: Option = None; - while let Some(next) = cs.unselected_indices().choose(rng) { - cs.select(next); - if cs.is_target_met(target) { - break; - } - } - cs -} diff --git a/tests/common.rs b/tests/common.rs index fa3bfa9..642a42f 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -2,7 +2,7 @@ use bdk_coin_select::{ float::Ordf32, BnbMetric, Candidate, ChangePolicy, CoinSelector, Drain, DrainWeights, FeeRate, - NoBnbSolution, Replace, Target, TargetFee, + NoBnbSolution, Replace, Target, TargetFee, TargetOutputs, }; use proptest::{ prelude::*, @@ -45,7 +45,7 @@ where let target = params.target(); - let mut selection = CoinSelector::new(&candidates, params.base_weight); + let mut selection = CoinSelector::new(&candidates); let mut exp_selection = selection.clone(); if metric.requires_ordering_by_descending_value_pwu() { @@ -67,11 +67,12 @@ where if exp_result.is_some() { let selected_value = exp_selection.selected_value(); let drain = exp_selection.drain(target, change_policy); - let target_value = target.value; + let target_value = target.value(); let replace_fee = params .replace .map(|replace| { - replace.min_fee_to_do_replacement(exp_selection.weight(drain.weights.output_weight)) + replace + .min_fee_to_do_replacement(exp_selection.weight(target.outputs, drain.weights)) }) .unwrap_or(0); assert!(selected_value - target_value - drain.value >= replace_fee); @@ -105,11 +106,12 @@ where // bonus check: ensure replacement fee is respected let selected_value = selection.selected_value(); let drain = selection.drain(target, change_policy); - let target_value = target.value; + let target_value = target.value(); let replace_fee = params .replace .map(|replace| { - replace.min_fee_to_do_replacement(selection.weight(drain.weights.output_weight)) + replace + .min_fee_to_do_replacement(selection.weight(target.outputs, drain.weights)) }) .unwrap_or(0); assert!(selected_value - target_value - drain.value >= replace_fee); @@ -140,7 +142,7 @@ where let target = params.target(); let init_cs = { - let mut cs = CoinSelector::new(&candidates, params.base_weight); + let mut cs = CoinSelector::new(&candidates); if metric.requires_ordering_by_descending_value_pwu() { cs.sort_candidates_by_descending_value_pwu(); } @@ -195,13 +197,15 @@ where pub struct StrategyParams { pub n_candidates: usize, pub target_value: u64, - pub base_weight: u32, + pub n_target_outputs: usize, + pub target_weight: u32, pub replace: Option, pub feerate: f32, pub feerate_lt_diff: f32, pub drain_weight: u32, pub drain_spend_weight: u32, pub drain_dust: u64, + pub n_drain_outputs: usize, } impl StrategyParams { @@ -211,7 +215,11 @@ impl StrategyParams { rate: FeeRate::from_sat_per_vb(self.feerate), replace: self.replace, }, - value: self.target_value, + outputs: TargetOutputs { + value_sum: self.target_value, + weight_sum: self.target_weight, + n_outputs: self.n_target_outputs, + }, } } @@ -227,6 +235,7 @@ impl StrategyParams { DrainWeights { output_weight: self.drain_weight, spend_weight: self.drain_spend_weight, + n_outputs: self.n_drain_outputs, } } } @@ -393,7 +402,7 @@ pub fn compare_against_benchmarks( let start = std::time::Instant::now(); let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha); let target = params.target(); - let cs = CoinSelector::new(&candidates, params.base_weight); + let cs = CoinSelector::new(&candidates); let solutions = cs.bnb_solutions(metric.clone()); let best = solutions diff --git a/tests/lowest_fee.rs b/tests/lowest_fee.rs index bee6809..f78704a 100644 --- a/tests/lowest_fee.rs +++ b/tests/lowest_fee.rs @@ -4,7 +4,7 @@ mod common; use bdk_coin_select::metrics::{Changeless, LowestFee}; use bdk_coin_select::{ BnbMetric, Candidate, ChangePolicy, CoinSelector, Drain, DrainWeights, FeeRate, Replace, - Target, TargetFee, + Target, TargetFee, TargetOutputs, TX_FIXED_FIELD_WEIGHT, }; use proptest::prelude::*; @@ -18,15 +18,17 @@ proptest! { fn can_eventually_find_best_solution( n_candidates in 1..20_usize, // candidates (n) target_value in 500..500_000_u64, // target value (sats) - base_weight in 0..1000_u32, // base weight (wu) - replace in common::maybe_replace(0u64..1_000), + n_target_outputs in 1usize..150, // the number of outputs we're funding + target_weight in 0..10_000_u32, // the sum of the weight of the outputs (wu) + replace in common::maybe_replace(0u64..10_000), // The weight of the transaction we're replacing feerate in 1.0..100.0_f32, // feerate (sats/vb) feerate_lt_diff in -5.0..50.0_f32, // longterm feerate diff (sats/vb) drain_weight in 100..=500_u32, // drain weight (wu) drain_spend_weight in 1..=2000_u32, // drain spend weight (wu) drain_dust in 100..=1000_u64, // drain dust (sats) + n_drain_outputs in 1usize..150, // the number of drain outputs ) { - let params = common::StrategyParams { n_candidates, target_value, base_weight, replace, feerate, feerate_lt_diff, drain_weight, drain_spend_weight, drain_dust }; + let params = common::StrategyParams { n_candidates, target_value, n_target_outputs, target_weight, replace, feerate, feerate_lt_diff, drain_weight, drain_spend_weight, drain_dust, n_drain_outputs }; let candidates = common::gen_candidates(params.n_candidates); let change_policy = ChangePolicy::min_value(params.drain_weights(), params.drain_dust); let metric = LowestFee { target: params.target(), long_term_feerate: params.long_term_feerate(), change_policy }; @@ -38,15 +40,17 @@ proptest! { fn ensure_bound_is_not_too_tight( n_candidates in 0..15_usize, // candidates (n) target_value in 500..500_000_u64, // target value (sats) - base_weight in 0..1000_u32, // base weight (wu) - replace in common::maybe_replace(0u64..1_000), + n_target_outputs in 1usize..150, // the number of outputs we're funding + target_weight in 0..10_000_u32, // the sum of the weight of the outputs (wu) + replace in common::maybe_replace(0u64..10_000), // The weight of the transaction we're replacing feerate in 1.0..100.0_f32, // feerate (sats/vb) feerate_lt_diff in -5.0..50.0_f32, // longterm feerate diff (sats/vb) drain_weight in 100..=500_u32, // drain weight (wu) - drain_spend_weight in 1..=1000_u32, // drain spend weight (wu) + drain_spend_weight in 1..=2000_u32, // drain spend weight (wu) drain_dust in 100..=1000_u64, // drain dust (sats) + n_drain_outputs in 1usize..150, // the number of drain outputs ) { - let params = common::StrategyParams { n_candidates, target_value, base_weight, replace, feerate, feerate_lt_diff, drain_weight, drain_spend_weight, drain_dust }; + let params = common::StrategyParams { n_candidates, target_value, n_target_outputs, target_weight, replace, feerate, feerate_lt_diff, drain_weight, drain_spend_weight, drain_dust, n_drain_outputs }; let candidates = common::gen_candidates(params.n_candidates); let change_policy = ChangePolicy::min_value(params.drain_weights(), params.drain_dust); let metric = LowestFee { target: params.target(), long_term_feerate: params.long_term_feerate(), change_policy }; @@ -58,27 +62,19 @@ proptest! { fn identical_candidates( n_candidates in 30..300_usize, target_value in 50_000..500_000_u64, // target value (sats) - base_weight in 0..641_u32, // base weight (wu) - replace in common::maybe_replace(0u64..1_000), - feerate in 1.0..10.0_f32, // feerate (sats/vb) - feerate_lt_diff in -5.0..5.0_f32, // longterm feerate diff (sats/vb) + n_target_outputs in 1usize..150, // the number of outputs we're funding + target_weight in 0..10_000_u32, // the sum of the weight of the outputs (wu) + replace in common::maybe_replace(0u64..10_000), // The weight of the transaction we're replacing + feerate in 1.0..100.0_f32, // feerate (sats/vb) + feerate_lt_diff in -5.0..50.0_f32, // longterm feerate diff (sats/vb) drain_weight in 100..=500_u32, // drain weight (wu) - drain_spend_weight in 1..=1000_u32, // drain spend weight (wu) + drain_spend_weight in 1..=2000_u32, // drain spend weight (wu) drain_dust in 100..=1000_u64, // drain dust (sats) + n_drain_outputs in 1usize..150, // the number of drain outputs ) { println!("== TEST =="); - let params = common::StrategyParams { - n_candidates, - target_value, - base_weight, - replace, - feerate, - feerate_lt_diff, - drain_weight, - drain_spend_weight, - drain_dust, - }; + let params = common::StrategyParams { n_candidates, target_value, n_target_outputs, target_weight, replace, feerate, feerate_lt_diff, drain_weight, drain_spend_weight, drain_dust, n_drain_outputs }; println!("{:?}", params); let candidates = core::iter::repeat(Candidate { @@ -90,7 +86,7 @@ proptest! { .take(params.n_candidates) .collect::>(); - let mut cs = CoinSelector::new(&candidates, params.base_weight); + let mut cs = CoinSelector::new(&candidates); let change_policy = ChangePolicy::min_value( params.drain_weights(), @@ -110,17 +106,21 @@ proptest! { #[test] #[cfg(not(debug_assertions))] // too slow if compiling for debug - fn compare_against_benchmarks(n_candidates in 0..50_usize, // candidates (n) + fn compare_against_benchmarks( + n_candidates in 0..50_usize, // candidates (n) target_value in 500..1_000_000_u64, // target value (sats) - base_weight in 0..1000_u32, // base weight (wu) - replace in common::maybe_replace(0u64..1_000), + n_target_outputs in 1usize..150, // the number of outputs we're funding + target_weight in 0..10_000_u32, // the sum of the weight of the outputs (wu) + replace in common::maybe_replace(0u64..10_000), // The weight of the transaction we're replacing feerate in 1.0..100.0_f32, // feerate (sats/vb) feerate_lt_diff in -5.0..50.0_f32, // longterm feerate diff (sats/vb) drain_weight in 100..=500_u32, // drain weight (wu) - drain_spend_weight in 1..=1000_u32, // drain spend weight (wu) + drain_spend_weight in 1..=2000_u32, // drain spend weight (wu) drain_dust in 100..=1000_u64, // drain dust (sats) + n_drain_outputs in 1usize..150, // the number of drain outputs ) { - let params = common::StrategyParams { n_candidates, target_value, base_weight, replace, feerate, feerate_lt_diff, drain_weight, drain_spend_weight, drain_dust }; + + let params = common::StrategyParams { n_candidates, target_value, n_target_outputs, target_weight, replace, feerate, feerate_lt_diff, drain_weight, drain_spend_weight, drain_dust, n_drain_outputs }; let candidates = common::gen_candidates(params.n_candidates); let change_policy = ChangePolicy::min_value(params.drain_weights(), params.drain_dust); let metric = LowestFee { target: params.target(), long_term_feerate: params.long_term_feerate(), change_policy }; @@ -134,18 +134,20 @@ fn combined_changeless_metric() { let params = common::StrategyParams { n_candidates: 100, target_value: 100_000, - base_weight: 1000, + target_weight: 1000 - TX_FIXED_FIELD_WEIGHT - 1, replace: None, feerate: 5.0, feerate_lt_diff: -4.0, drain_weight: 200, drain_spend_weight: 600, drain_dust: 200, + n_target_outputs: 1, + n_drain_outputs: 1, }; let candidates = common::gen_candidates(params.n_candidates); - let mut cs_a = CoinSelector::new(&candidates, params.base_weight); - let mut cs_b = CoinSelector::new(&candidates, params.base_weight); + let mut cs_a = CoinSelector::new(&candidates); + let mut cs_b = CoinSelector::new(&candidates); let change_policy = ChangePolicy::min_value(params.drain_weights(), params.drain_dust); @@ -180,7 +182,11 @@ fn combined_changeless_metric() { fn adding_another_input_to_remove_change() { let target = Target { fee: TargetFee::default(), - value: 99_870, + outputs: TargetOutputs { + value_sum: 99_870, + weight_sum: 200 - TX_FIXED_FIELD_WEIGHT - 1, + n_outputs: 1, + }, }; let candidates = vec![ @@ -205,7 +211,7 @@ fn adding_another_input_to_remove_change() { }, ]; - let mut cs = CoinSelector::new(&candidates, 200); + let mut cs = CoinSelector::new(&candidates); let best_solution = { let mut cs = cs.clone(); @@ -219,6 +225,7 @@ fn adding_another_input_to_remove_change() { let drain_weights = DrainWeights { output_weight: 100, spend_weight: 1_000, + n_outputs: 1, }; let excess_to_make_first_candidate_satisfy_but_have_change = { diff --git a/tests/waste.proptest-regressions b/tests/waste.proptest-regressions deleted file mode 100644 index 0f0bc32..0000000 --- a/tests/waste.proptest-regressions +++ /dev/null @@ -1,16 +0,0 @@ -# Seeds for failure cases proptest has generated in the past. It is -# automatically read and these particular cases re-run before any -# novel cases are generated. -# -# It is recommended to check this file in to source control so that -# everyone who runs the test benefits from these saved cases. -cc b526e3a05e5dffce95e0cf357f68d6819b5b92a1c4abd79fd8fe0e2582521352 # shrinks to num_inputs = 45, target = 16494, feerate = 3.0291684, min_fee = 0, base_weight = 155, long_term_feerate_diff = -0.70271873, change_weight = 58, change_spend_weight = 82 -cc f3c37a516004e7eda9183816d72bede9084ce678830d6582f2d63306f618adee # shrinks to num_inputs = 40, target = 6598, feerate = 8.487553, min_fee = 221, base_weight = 126, long_term_feerate_diff = 3.3214626, change_weight = 18, change_spend_weight = 18 -cc a6d03a6d93eb8d5a082d69a3d1677695377823acafe3dba954ac86519accf152 # shrinks to num_inputs = 49, target = 2917, feerate = 9.786607, min_fee = 0, base_weight = 4, long_term_feerate_diff = -0.75053596, change_weight = 77, change_spend_weight = 81 -cc a1eccddab6d7da9677575154a27a1e49b391041ed9e32b9bf937efd72ef0ab03 # shrinks to num_inputs = 12, target = 3988, feerate = 4.3125916, min_fee = 453, base_weight = 0, long_term_feerate_diff = -0.018570423, change_weight = 15, change_spend_weight = 32 -cc 4bb301aaba29e5f5311bb57c8737279045f7ad594adb91b94c5e080d3ba21933 # shrinks to num_inputs = 33, target = 2023, feerate = 4.4804115, min_fee = 965, base_weight = 0, long_term_feerate_diff = -0.30981845, change_weight = 80, change_spend_weight = 95 -cc 6c1e79f7bd7753a37c1aaebb72f3be418ac092a585e7629ab2331e0f9a585640 # shrinks to n_candidates = 11, target_value = 401712, base_weight = 33, min_fee = 0, feerate = 62.1756, feerate_lt_diff = 0.0, drain_weight = 100, drain_spend_weight = 253, drain_dust = 100 -cc 617e11dc77968b5d26748b10da6d4916210fb7004a120cff73784d9587816fee # shrinks to n_candidates = 6, target_value = 77118, base_weight = 996, min_fee = 661, feerate = 78.64882, feerate_lt_diff = 46.991302, drain_weight = 188, drain_spend_weight = 1242, drain_dust = 366 -cc 5905f9f223eb175556a89335da988256cb15f14e0f53f7ff512b1ff05ee74f83 # shrinks to n_candidates = 15, target_value = 497809, base_weight = 303, min_fee = 0, feerate = 32.44647, feerate_lt_diff = -2.8886793, drain_weight = 100, drain_spend_weight = 257, drain_dust = 100 -cc 414c6219145a3867c404ea0f54415ab6a1089f1497dede15c4989e7a88e9936a # shrinks to n_candidates = 3, target_value = 444025, base_weight = 770, min_fee = 0, feerate = 36.7444, feerate_lt_diff = 21.816896, drain_weight = 203, drain_spend_weight = 1921, drain_dust = 100 -cc 536487b3604db918a3ca5cfc3f38a3af6cef9b0140ddca59e7d2ea92af61e04e # shrinks to num_inputs = 17, target = 7008, feerate = 1.0, min_fee = 702, base_weight = 0, long_term_feerate_diff = -0.24188519, change_weight = 28, change_spend_weight = 44 diff --git a/tests/waste.rs b/tests/waste.rs deleted file mode 100644 index e465671..0000000 --- a/tests/waste.rs +++ /dev/null @@ -1,485 +0,0 @@ -#![allow(unused_imports)] - -mod common; - -use bdk_coin_select::{ - float::Ordf32, metrics::Waste, Candidate, ChangePolicy, CoinSelector, Drain, DrainWeights, - FeeRate, Target, TargetFee, -}; -use proptest::{ - prelude::*, - test_runner::{RngAlgorithm, TestRng}, -}; -use rand::prelude::IteratorRandom; - -#[test] -fn waste_all_selected_except_one_is_optimal_and_awkward() { - let num_inputs = 40; - let target = 15578; - let feerate = 8.190512; - let base_weight = 453; - let long_term_feerate_diff = -3.630499; - let change_weight = 1; - let change_spend_weight = 41; - let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha); - let long_term_feerate = - FeeRate::from_sat_per_vb((0.0f32).max(feerate - long_term_feerate_diff)); - let feerate = FeeRate::from_sat_per_vb(feerate); - let drain_weights = DrainWeights { - output_weight: change_weight, - spend_weight: change_spend_weight, - }; - - let change_policy = - ChangePolicy::min_value_and_waste(drain_weights, 0, feerate, long_term_feerate); - let wv = test_wv(&mut rng); - let candidates = wv.take(num_inputs).collect::>(); - - let cs = CoinSelector::new(&candidates, base_weight); - let target = Target { - value: target, - fee: TargetFee::from_feerate(feerate), - }; - - let solutions = cs.bnb_solutions(Waste { - target, - long_term_feerate, - change_policy, - }); - - let (_i, (best, score)) = solutions - .enumerate() - .filter_map(|(i, sol)| Some((i, sol?))) - .last() - .expect("it should have found solution"); - - let mut all_selected = cs.clone(); - all_selected.select_all(); - let target_waste = all_selected.waste( - target, - long_term_feerate, - cs.drain(target, change_policy), - 1.0, - ); - assert!(score.0 < target_waste); - assert_eq!(best.selected().len(), 39); -} - -#[test] -fn waste_naive_effective_value_shouldnt_be_better() { - let num_inputs = 23; - let target = 1475; - let feerate = 1.0; - // let min_fee = 989; - let base_weight = 0; - let long_term_feerate_diff = 3.8413858; - let change_weight = 1; - let change_spend_weight = 1; - let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha); - let long_term_feerate = - FeeRate::from_sat_per_vb((0.0f32).max(feerate - long_term_feerate_diff)); - let feerate = FeeRate::from_sat_per_vb(feerate); - let drain_weights = DrainWeights { - output_weight: change_weight, - spend_weight: change_spend_weight, - }; - let change_policy = - ChangePolicy::min_value_and_waste(drain_weights, 0, feerate, long_term_feerate); - let wv = test_wv(&mut rng); - let candidates = wv.take(num_inputs).collect::>(); - - let cs = CoinSelector::new(&candidates, base_weight); - - let target = Target { - value: target, - fee: TargetFee::from_feerate(feerate), - }; - - let solutions = cs.bnb_solutions(Waste { - target, - long_term_feerate, - change_policy, - }); - - let (_i, (_best, score)) = solutions - .enumerate() - .filter_map(|(i, sol)| Some((i, sol?))) - .last() - .expect("should find solution"); - - let mut naive_select = cs.clone(); - naive_select.sort_candidates_by_key(|(_, wv)| core::cmp::Reverse(Ordf32(wv.value_pwu()))); - // we filter out failing onces below - let _ = naive_select.select_until_target_met(target); - - let bench_waste = naive_select.waste( - target, - long_term_feerate, - naive_select.drain(target, change_policy), - 1.0, - ); - - assert!(score < Ordf32(bench_waste)); -} - -#[test] -fn waste_doesnt_take_too_long_to_finish() { - let start = std::time::Instant::now(); - let num_inputs = 22; - let target = 0; - let feerate = 4.9522414; - let base_weight = 2; - let long_term_feerate_diff = -0.17994404; - let change_weight = 1; - let change_spend_weight = 34; - - let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha); - let long_term_feerate = - FeeRate::from_sat_per_vb((0.0f32).max(feerate - long_term_feerate_diff)); - let feerate = FeeRate::from_sat_per_vb(feerate); - let drain_weights = DrainWeights { - output_weight: change_weight, - spend_weight: change_spend_weight, - }; - - let change_policy = - ChangePolicy::min_value_and_waste(drain_weights, 0, feerate, long_term_feerate); - let wv = test_wv(&mut rng); - let candidates = wv.take(num_inputs).collect::>(); - - let cs = CoinSelector::new(&candidates, base_weight); - - let target = Target { - value: target, - fee: TargetFee::from_feerate(feerate), - }; - - let solutions = cs.bnb_solutions(Waste { - target, - long_term_feerate, - change_policy, - }); - - solutions - .enumerate() - .inspect(|_| { - if start.elapsed().as_millis() > 1_000 { - panic!("took too long to finish") - } - }) - .filter_map(|(i, sol)| Some((i, sol?))) - .last() - .expect("should find solution"); -} - -/// When long term feerate is lower than current adding new inputs should in general make things -/// worse except in the case that we can get rid of the change output with negative effective -/// value inputs. In this case the right answer to select everything. -#[test] -fn waste_lower_long_term_feerate_but_still_need_to_select_all() { - let num_inputs = 16; - let target = 5586; - let feerate = 9.397041; - let base_weight = 91; - let long_term_feerate_diff = 0.22074795; - let change_weight = 1; - let change_spend_weight = 27; - - let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha); - let long_term_feerate = FeeRate::from_sat_per_vb(0.0f32.max(feerate - long_term_feerate_diff)); - let feerate = FeeRate::from_sat_per_vb(feerate); - let drain_weights = DrainWeights { - output_weight: change_weight, - spend_weight: change_spend_weight, - }; - - let change_policy = - ChangePolicy::min_value_and_waste(drain_weights, 0, feerate, long_term_feerate); - let wv = test_wv(&mut rng); - let candidates = wv.take(num_inputs).collect::>(); - - let cs = CoinSelector::new(&candidates, base_weight); - - let target = Target { - value: target, - fee: TargetFee::from_feerate(feerate), - }; - - let solutions = cs.bnb_solutions(Waste { - target, - long_term_feerate, - change_policy, - }); - let bench = { - let mut all_selected = cs.clone(); - all_selected.select_all(); - all_selected - }; - - let (_i, (_sol, waste)) = solutions - .enumerate() - .filter_map(|(i, sol)| Some((i, sol?))) - .last() - .expect("should find solution"); - - let bench_waste = bench.waste( - target, - long_term_feerate, - bench.drain(target, change_policy), - 1.0, - ); - - assert!(waste <= Ordf32(bench_waste)); -} - -#[test] -fn waste_low_but_non_negative_rate_diff_means_adding_more_inputs_might_reduce_excess() { - let num_inputs = 22; - let target = 7620; - let feerate = 8.173157; - let base_weight = 35; - let long_term_feerate_diff = 0.0; - let change_weight = 1; - let change_spend_weight = 47; - - let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha); - let long_term_feerate = FeeRate::from_sat_per_vb(0.0f32.max(feerate - long_term_feerate_diff)); - let feerate = FeeRate::from_sat_per_vb(feerate); - let drain_weights = DrainWeights { - output_weight: change_weight, - spend_weight: change_spend_weight, - }; - - let change_policy = - ChangePolicy::min_value_and_waste(drain_weights, 0, feerate, long_term_feerate); - let wv = test_wv(&mut rng); - let mut candidates = wv.take(num_inputs).collect::>(); - // HACK: for this test had to set segwit true to keep it working once we - // started properly accounting for legacy weight variations - candidates - .iter_mut() - .for_each(|candidate| candidate.is_segwit = true); - - let cs = CoinSelector::new(&candidates, base_weight); - - let target = Target { - value: target, - fee: TargetFee::from_feerate(feerate), - }; - - let solutions = cs.bnb_solutions(Waste { - target, - long_term_feerate, - change_policy, - }); - let bench = { - let mut all_selected = cs.clone(); - all_selected.select_all(); - all_selected - }; - - let (_i, (_sol, waste)) = solutions - .enumerate() - .filter_map(|(i, sol)| Some((i, sol?))) - .last() - .expect("should find solution"); - - let bench_waste = bench.waste( - target, - long_term_feerate, - bench.drain(target, change_policy), - 1.0, - ); - - assert!(waste <= Ordf32(bench_waste)); -} - -proptest! { - #![proptest_config(ProptestConfig { - timeout: 6_000, - cases: 1_000, - ..Default::default() - })] - // TODO: Because our waste bnb implementation has bounds that are too tight, sometimes the best - // solution is skipped. - // - // #[test] - // #[cfg(not(debug_assertions))] // too slow if compiling for debug - // fn waste_prop_waste( - // num_inputs in 0usize..20, - // target in 0u64..25_000, - // feerate in 1.0f32..10.0, - // min_fee in 0u64..1_000, - // base_weight in 0u32..500, - // long_term_feerate_diff in -5.0f32..5.0, - // change_weight in 1u32..100, - // change_spend_weight in 1u32..100, - // ) { - // println!("======================================="); - // let start = std::time::Instant::now(); - // let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha); - // let long_term_feerate = FeeRate::from_sat_per_vb(0.0f32.max(feerate - long_term_feerate_diff)); - // let feerate = FeeRate::from_sat_per_vb(feerate); - // let drain = DrainWeights { - // output_weight: change_weight, - // spend_weight: change_spend_weight, - // }; - // - // let change_policy = crate::change_policy::min_waste(drain, long_term_feerate); - // let wv = test_wv(&mut rng); - // let candidates = wv.take(num_inputs).collect::>(); - // - // let cs = CoinSelector::new(&candidates, base_weight); - // - // let target = Target { - // value: target, - // feerate, - // min_fee - // }; - // - // let solutions = cs.bnb_solutions(Waste { - // target, - // long_term_feerate, - // change_policy: &change_policy - // }); - // - // - // let best = solutions - // .enumerate() - // .filter_map(|(i, sol)| Some((i, sol?))) - // .last(); - // - // match best { - // Some((_i, (sol, _score))) => { - // - // let mut cmp_benchmarks = vec![ - // { - // let mut naive_select = cs.clone(); - // naive_select.sort_candidates_by_key(|(_, wv)| core::cmp::Reverse(wv.effective_value(target.fee.rate))); - // // we filter out failing onces below - // let _ = naive_select.select_until_target_met(target, Drain { weights: drain, value: 0 }); - // naive_select - // }, - // { - // let mut all_selected = cs.clone(); - // all_selected.select_all(); - // all_selected - // }, - // { - // let mut all_effective_selected = cs.clone(); - // all_effective_selected.select_all_effective(target.fee.rate); - // all_effective_selected - // } - // ]; - // - // // add some random selections -- technically it's possible that one of these is better but it's very unlikely if our algorithm is working correctly. - // cmp_benchmarks.extend((0..10).map(|_|randomly_satisfy_target_with_low_waste(&cs, target, long_term_feerate, &change_policy, &mut rng))); - // - // let cmp_benchmarks = cmp_benchmarks.into_iter().filter(|cs| cs.is_target_met(target, change_policy(&cs, target))); - // let sol_waste = sol.waste(target, long_term_feerate, change_policy(&sol, target), 1.0); - // - // for (_bench_id, mut bench) in cmp_benchmarks.enumerate() { - // let bench_waste = bench.waste(target, long_term_feerate, change_policy(&bench, target), 1.0); - // if sol_waste > bench_waste { - // dbg!(_bench_id); - // println!("bnb solution: {}", sol); - // bench.sort_candidates_by_descending_value_pwu(); - // println!("found better: {}", bench); - // } - // prop_assert!(sol_waste <= bench_waste); - // } - // }, - // None => { - // dbg!(feerate - long_term_feerate); - // prop_assert!(!cs.is_selection_plausible_with_change_policy(target, &change_policy)); - // } - // } - // - // dbg!(start.elapsed()); - // } - - // TODO: Because our waste bnb implementation has bounds that are too tight, sometimes the best - // solution is skipped. - // - // #[test] - // #[cfg(not(debug_assertions))] // too slow if compiling for debug - // fn can_eventually_find_best_solution( - // n_candidates in 1..20_usize, // candidates (n) - // target_value in 500..500_000_u64, // target value (sats) - // base_weight in 0..1000_u32, // base weight (wu) - // min_fee in 0..1_000_u64, // min fee (sats) - // feerate in 1.0..100.0_f32, // feerate (sats/vb) - // feerate_lt_diff in -5.0..50.0_f32, // longterm feerate diff (sats/vb) - // drain_weight in 100..=500_u32, // drain weight (wu) - // drain_spend_weight in 1..=2000_u32, // drain spend weight (wu) - // drain_dust in 100..=1000_u64, // drain dust (sats) - // ) { - // let params = common::StrategyParams { n_candidates, target_value, base_weight, min_fee, feerate, feerate_lt_diff, drain_weight, drain_spend_weight, drain_dust }; - // let candidates = common::gen_candidates(params.n_candidates); - // let change_policy = min_value_and_waste(params.drain_weights(), params.drain_dust, params.long_term_feerate()); - // let metric = Waste { target: params.target(), long_term_feerate: params.long_term_feerate(), change_policy: &change_policy }; - // common::can_eventually_find_best_solution(params, candidates, &change_policy, metric)?; - // } - - // TODO: Our waste bnb bounds are too tight! - // - // #[test] - // #[cfg(not(debug_assertions))] // too slow if compiling for debug - // fn ensure_bound_is_not_too_tight( - // n_candidates in 0..15_usize, // candidates (n) - // target_value in 500..500_000_u64, // target value (sats) - // base_weight in 0..641_u32, // base weight (wu) - // min_fee in 0..1_000_u64, // min fee (sats) - // feerate in 1.0..100.0_f32, // feerate (sats/vb) - // feerate_lt_diff in -5.0..50.0_f32, // longterm feerate diff (sats/vb) - // drain_weight in 100..=500_u32, // drain weight (wu) - // drain_spend_weight in 1..=1000_u32, // drain spend weight (wu) - // drain_dust in 100..=1000_u64, // drain dust (sats) - // ) { - // let params = common::StrategyParams { n_candidates, target_value, base_weight, min_fee, feerate, feerate_lt_diff, drain_weight, drain_spend_weight, drain_dust }; - // let candidates = common::gen_candidates(params.n_candidates); - // let change_policy = min_value_and_waste(params.drain_weights(), params.drain_dust, params.long_term_feerate()); - // let metric = Waste { target: params.target(), long_term_feerate: params.long_term_feerate(), change_policy: &change_policy }; - // common::ensure_bound_is_not_too_tight(params, candidates, &change_policy, metric)?; - // } -} - -fn test_wv(mut rng: impl RngCore) -> impl Iterator { - core::iter::repeat_with(move || { - let value = rng.gen_range(0..1_000); - Candidate { - value, - weight: rng.gen_range(0..100), - input_count: rng.gen_range(1..2), - is_segwit: rng.gen_bool(0.5), - } - }) -} - -// this is probably a useful thing to have on CoinSelector but I don't want to design it yet -#[allow(unused)] -fn randomly_satisfy_target_with_low_waste<'a>( - cs: &CoinSelector<'a>, - target: Target, - long_term_feerate: FeeRate, - change_policy: &impl Fn(&CoinSelector, Target) -> Drain, - rng: &mut impl RngCore, -) -> CoinSelector<'a> { - let mut cs = cs.clone(); - - let mut last_waste: Option = None; - while let Some(next) = cs.unselected_indices().choose(rng) { - cs.select(next); - let change = change_policy(&cs, target); - if cs.is_target_met_with_drain(target, change) { - let curr_waste = cs.waste(target, long_term_feerate, change, 1.0); - if let Some(last_waste) = last_waste { - if curr_waste > last_waste { - break; - } - } - last_waste = Some(curr_waste); - } - } - cs -} diff --git a/tests/weight.rs b/tests/weight.rs index e1ba021..cbb5900 100644 --- a/tests/weight.rs +++ b/tests/weight.rs @@ -1,9 +1,7 @@ #![allow(clippy::zero_prefixed_literal)] -use std::str::FromStr; - -use bdk_coin_select::{Candidate, CoinSelector, Drain}; -use bitcoin::{absolute::Height, consensus::Decodable, Address, ScriptBuf, Transaction, TxOut}; +use bdk_coin_select::{Candidate, CoinSelector, Drain, DrainWeights, TargetOutputs}; +use bitcoin::{consensus::Decodable, ScriptBuf, Transaction}; fn hex_val(c: u8) -> u8 { match c { @@ -28,11 +26,11 @@ fn segwit_one_input_one_output() { // FROM https://mempool.space/tx/e627fbb7f775a57fd398bf9b150655d4ac3e1f8afed4255e74ee10d7a345a9cc let mut tx_bytes = hex_decode("01000000000101b2ec00fd7d3f2c89eb27e3e280960356f69fc88a324a4bca187dd4b020aa36690000000000ffffffff01d0bb9321000000001976a9141dc94fe723f43299c6187094b1dc5a032d47b06888ac024730440220669b764de7e9dcedcba6d6d57c8c761be2acc4e1a66938ceecacaa6d494f582d02202641df89d1758eeeed84290079dd9ad36611c73cd9e381dd090b83f5e5b1422e012103f6544e4ffaff4f8649222003ada5d74bd6d960162bcd85af2b619646c8c45a5298290c00"); let mut cursor = std::io::Cursor::new(&mut tx_bytes); - let mut tx = Transaction::consensus_decode(&mut cursor).unwrap(); + let tx = Transaction::consensus_decode(&mut cursor).unwrap(); let input_values = vec![563_336_755]; - let inputs = core::mem::take(&mut tx.input); - let candidates = inputs + let candidates = tx + .input .iter() .zip(input_values) .map(|(txin, value)| Candidate { @@ -43,13 +41,22 @@ fn segwit_one_input_one_output() { }) .collect::>(); - let mut coin_selector = CoinSelector::new(&candidates, tx.weight().to_wu() as u32); + let target_ouputs = TargetOutputs { + value_sum: tx.output.iter().map(|output| output.value).sum(), + weight_sum: tx.output.iter().map(|output| output.weight() as u32).sum(), + n_outputs: tx.output.len(), + }; + + let mut coin_selector = CoinSelector::new(&candidates); coin_selector.select_all(); - assert_eq!(coin_selector.weight(0), 449); + assert_eq!( + coin_selector.weight(target_ouputs, DrainWeights::NONE), + tx.weight().to_wu() as u32 + ); assert_eq!( (coin_selector - .implied_feerate(tx.output[0].value, Drain::none()) + .implied_feerate(target_ouputs, Drain::NONE) .unwrap() .as_sat_vb() * 10.0) @@ -63,11 +70,11 @@ fn segwit_two_inputs_one_output() { // FROM https://mempool.space/tx/37d2883bdf1b4c110b54cb624d36ab6a30140f8710ed38a52678260a7685e708 let mut tx_bytes = hex_decode("020000000001021edcae5160b1ba2370a45ea9342b4c883a8941274539612bddf1c379ba7ecf180700000000ffffffff5c85e19bf4f0e293c0d5f9665cb05d2a55d8bba959edc5ef02075f6a1eb9fc120100000000ffffffff0168ce3000000000001976a9145ff742d992276a1f46e5113dde7382896ff86e2a88ac0247304402202e588db55227e0c24db7f07b65f221ebcae323fb595d13d2e1c360b773d809b0022008d2f57a618bd346cfd031549a3971f22464e3e3308cee340a976f1b47a96f0b012102effbcc87e6c59b810c2fa20b0bc3eb909a20b40b25b091cf005d416b85db8c8402483045022100bdc115b86e9c863279132b4808459cf9b266c8f6a9c14a3dfd956986b807e3320220265833b85197679687c5d5eed1b2637489b34249d44cf5d2d40bc7b514181a51012102077741a668889ce15d59365886375aea47a7691941d7a0d301697edbc773b45b00000000"); let mut cursor = std::io::Cursor::new(&mut tx_bytes); - let mut tx = Transaction::consensus_decode(&mut cursor).unwrap(); + let tx = Transaction::consensus_decode(&mut cursor).unwrap(); let input_values = vec![003_194_967, 000_014_068]; - let inputs = core::mem::take(&mut tx.input); - let candidates = inputs + let candidates = tx + .input .iter() .zip(input_values) .map(|(txin, value)| Candidate { @@ -78,13 +85,23 @@ fn segwit_two_inputs_one_output() { }) .collect::>(); - let mut coin_selector = CoinSelector::new(&candidates, tx.weight().to_wu() as u32); + let mut coin_selector = CoinSelector::new(&candidates); + + let target_ouputs = TargetOutputs { + value_sum: tx.output.iter().map(|output| output.value).sum(), + weight_sum: tx.output.iter().map(|output| output.weight() as u32).sum(), + n_outputs: tx.output.len(), + }; + coin_selector.select_all(); - assert_eq!(coin_selector.weight(0), 721); + assert_eq!( + coin_selector.weight(target_ouputs, DrainWeights::NONE), + tx.weight().to_wu() as u32 + ); assert_eq!( (coin_selector - .implied_feerate(tx.output[0].value, Drain::none()) + .implied_feerate(target_ouputs, Drain::none()) .unwrap() .as_sat_vb() * 10.0) @@ -98,11 +115,11 @@ fn legacy_three_inputs() { // FROM https://mempool.space/tx/5f231df4f73694b3cca9211e336451c20dab136e0a843c2e3166cdcb093e91f4 let mut tx_bytes = hex_decode("0100000003fe785783e14669f638ba902c26e8e3d7036fb183237bc00f8a10542191c7171300000000fdfd00004730440220418996f20477d143d02ad47e74e5949641b6c2904159ab7c592d2cfc659f9bd802205b18f18ac86b714971f84a8b74a4cb14ad5c1a5b9d0d939bb32c6ae4032f4ea10148304502210091296ff8dd87b5ebfc3d47cb82cfe4750d52c544a2b88a85970354a4d0d4b1db022069632067ee6f30f06145f649bc76d5e5d5e6404dbe985e006fcde938f778c297014c695221030502b8ade694d57a6e86998180a64f4ce993372830dc796c3d561ad8b2a504de210272b68e1c037c4630eff7ea5858640cc0748e36f5de82fb38529ef1fd0a89670d2103ba0544a3a2aa9f2314022760b78b5c833aebf6f88468a089550f93834a2886ed53aeffffffff7e048a7c53a8af656e24442c65fe4c4299b1494f6c7579fe0fd9fa741ce83e3279000000fc004730440220018fa343acccd048ed8f8f179e1b6ae27435a41b5fb2c1d96a5a772777acc6dc022074783814f2100c6fc4d4c976f941212be50825814502ca0cbe3f929db789979e0147304402206373f01b73fb09876d0f5ee3087e0614cab3be249934bc2b7eb64ee67f53dc8302200b50f8a327020172b82aaba7480c77ecf07bb32322a05f4afbc543aa97d2fde8014c69522103039d906b2494e310f6c7774c98618be552720d04781e073dd3ff25d5906f22662103d82026baa529619b103ec6341d548a7eb6d924061a8469a7416155513a3071c12102e452bc4aa726d44646ba80db70465683b30efde282a19aa35c6029ae8925df5e53aeffffffffef80f0b1cc543de4f73d59c02a3c575ae5d0af17c1e11e6be7abe3325c777507ad000000fdfd00004730440220220fee11bf836621a11a8ea9100a4600c109c13895f11468d3e2062210c5481902201c5c8a462175538e87b8248e1ed3927c3a461c66d1b46215641c875e86eb22c4014830450221008d2de8c2f20a720129c372791e595b9602b1a9bce99618497aec5266148ffc1302203a493359d700ed96323f8805ed03e909959ff0f22eff359028db6861486b1555014c6952210374a4add33567f09967592c5bcdc3db421fdbba67bac4636328f96d941da31bd221039636c2ffac90afb7499b16e265078113dfb2d77b54270e37353217c9eaeaf3052103d0bcea6d10cdd2f16018ea71572631708e26f457f67cda36a7f816a87f7791d253aeffffffff04977261000000000016001470385d054721987f41521648d7b2f5c77f735d6bee92030000000000225120d0cda1b675a0b369964cbfa381721aae3549dd2c9c6f2cf71ff67d5bc277afd3f2aaf30000000000160014ed2d41ba08313dbb2630a7106b2fedafc14aa121d4f0c70000000000220020e5c7c00d174631d2d1e365d6347b016fb87b6a0c08902d8e443989cb771fa7ec00000000"); let mut cursor = std::io::Cursor::new(&mut tx_bytes); - let mut tx = Transaction::consensus_decode(&mut cursor).unwrap(); + let tx = Transaction::consensus_decode(&mut cursor).unwrap(); let orig_weight = tx.weight(); let input_values = vec![022_680_000, 006_558_175, 006_558_200]; - let inputs = core::mem::take(&mut tx.input); - let candidates = inputs + let candidates = tx + .input .iter() .zip(input_values) .map(|(txin, value)| Candidate { @@ -113,13 +130,22 @@ fn legacy_three_inputs() { }) .collect::>(); - let mut coin_selector = CoinSelector::new(&candidates, tx.weight().to_wu() as u32); + let target_ouputs = TargetOutputs { + value_sum: tx.output.iter().map(|output| output.value).sum(), + weight_sum: tx.output.iter().map(|output| output.weight() as u32).sum(), + n_outputs: tx.output.len(), + }; + + let mut coin_selector = CoinSelector::new(&candidates); coin_selector.select_all(); - assert_eq!(coin_selector.weight(0), orig_weight.to_wu() as u32); + assert_eq!( + coin_selector.weight(target_ouputs, DrainWeights::NONE), + orig_weight.to_wu() as u32 + ); assert_eq!( (coin_selector - .implied_feerate(tx.output.iter().map(|o| o.value).sum(), Drain::none()) + .implied_feerate(target_ouputs, Drain::NONE) .unwrap() .as_sat_vb() * 10.0) @@ -141,10 +167,9 @@ fn legacy_three_inputs_one_segwit() { hex_decode("3045022100bdc115b86e9c863279132b4808459cf9b266c8f6a9c14a3dfd956986b807e3320220265833b85197679687c5d5eed1b2637489b34249d44cf5d2d40bc7b514181a5101"), hex_decode("02077741a668889ce15d59365886375aea47a7691941d7a0d301697edbc773b45b"), ].into(); - let orig_weight = tx.weight(); let input_values = vec![022_680_000, 006_558_175, 006_558_200]; - let inputs = core::mem::take(&mut tx.input); - let candidates = inputs + let candidates = tx + .input .iter() .zip(input_values) .enumerate() @@ -163,41 +188,17 @@ fn legacy_three_inputs_one_segwit() { }) .collect::>(); - let mut coin_selector = CoinSelector::new(&candidates, tx.weight().to_wu() as u32); - coin_selector.select_all(); - - assert_eq!(coin_selector.weight(0), orig_weight.to_wu() as u32); -} - -/// Ensure that `fund_outputs` caculates the same `base_weight` as `rust-bitcoin`. -/// -/// We test it with 3 different output counts (resulting in different varint output-count weights). -#[test] -fn fund_outputs() { - let txo = TxOut { - script_pubkey: Address::from_str("bc1q4hym5spvze5d4wand9mf9ed7ku00kg6cv3h9ct") - .expect("must parse address") - .assume_checked() - .script_pubkey(), - value: 50_000, + let target_ouputs = TargetOutputs { + value_sum: tx.output.iter().map(|output| output.value).sum(), + weight_sum: tx.output.iter().map(|output| output.weight() as u32).sum(), + n_outputs: tx.output.len(), }; - let txo_weight = txo.weight() as u32; - - let output_counts: &[usize] = &[0x01, 0xfd, 0x01_0000]; - for &output_count in output_counts { - let weight_from_fund_outputs = - CoinSelector::fund_outputs(&[], (0..=output_count).map(|_| txo_weight)).weight(0); - - let exp_weight = Transaction { - version: 0, - lock_time: bitcoin::absolute::LockTime::Blocks(Height::ZERO), - input: Vec::new(), - output: (0..=output_count).map(|_| txo.clone()).collect(), - } - .weight() - .to_wu() as u32; + let mut coin_selector = CoinSelector::new(&candidates); + coin_selector.select_all(); - assert_eq!(weight_from_fund_outputs, exp_weight); - } + assert_eq!( + coin_selector.weight(target_ouputs, DrainWeights::NONE), + tx.weight().to_wu() as u32 + ); } From 76e67a3b3d09f30db73af123ab86aedd39ea7612 Mon Sep 17 00:00:00 2001 From: LLFourn Date: Thu, 25 Jan 2024 17:41:41 +1100 Subject: [PATCH 2/9] Move ChangePolicy to drain.rs --- src/change_policy.rs | 53 --------------------------------------- src/coin_selector.rs | 2 +- src/drain.rs | 47 ++++++++++++++++++++++++++++++++++ src/lib.rs | 2 -- src/metrics.rs | 4 +-- src/metrics/changeless.rs | 2 +- src/metrics/lowest_fee.rs | 4 +-- 7 files changed, 51 insertions(+), 63 deletions(-) diff --git a/src/change_policy.rs b/src/change_policy.rs index 268d3bc..8b13789 100644 --- a/src/change_policy.rs +++ b/src/change_policy.rs @@ -1,54 +1 @@ -//! This module contains a collection of change policies. -//! -//! A change policy determines whether a given coin selection (presented by [`CoinSelector`]) should -//! construct a transaction with a change output. A change policy is represented as a function of -//! type `Fn(&CoinSelector, Target) -> Drain`. -#[allow(unused)] // some bug in <= 1.48.0 sees this as unused when it isn't -use crate::float::FloatExt; -use crate::{DrainWeights, FeeRate}; - -/// Describes when a change output (although it could represent several) should be added that drains -/// the excess in the coin selection. It includes the `drain_weights` to account for the cost of -/// adding this outupt(s). -#[derive(Clone, Copy, Debug, PartialEq)] -pub struct ChangePolicy { - /// The minimum amount of excesss there needs to be add a change output. - pub min_value: u64, - /// The weights of the drain that would be added according to the policy. - pub drain_weights: DrainWeights, -} - -impl ChangePolicy { - /// Construct a change policy that creates change when the change value is greater than - /// `min_value`. - pub fn min_value(drain_weights: DrainWeights, min_value: u64) -> Self { - Self { - drain_weights, - min_value, - } - } - - /// Construct a change policy that creates change when it would reduce the transaction waste - /// given that `min_value` is respected. - pub fn min_value_and_waste( - drain_weights: DrainWeights, - min_value: u64, - target_feerate: FeeRate, - long_term_feerate: FeeRate, - ) -> Self { - // The output waste of a changeless solution is the excess. - let waste_with_change = drain_weights - .waste( - target_feerate, - long_term_feerate, - 0, /* ignore varint cost for now */ - ) - .ceil() as u64; - - Self { - drain_weights, - min_value: waste_with_change.max(min_value), - } - } -} diff --git a/src/coin_selector.rs b/src/coin_selector.rs index 1055ba8..a9c169e 100644 --- a/src/coin_selector.rs +++ b/src/coin_selector.rs @@ -1,7 +1,7 @@ use super::*; #[allow(unused)] // some bug in <= 1.48.0 sees this as unused when it isn't use crate::float::FloatExt; -use crate::{bnb::BnbMetric, change_policy::ChangePolicy, float::Ordf32, FeeRate, Target}; +use crate::{bnb::BnbMetric, float::Ordf32, ChangePolicy, FeeRate, Target}; use alloc::{borrow::Cow, collections::BTreeSet, vec::Vec}; /// [`CoinSelector`] selects/deselects coins from a set of canididate coins. diff --git a/src/drain.rs b/src/drain.rs index d59d119..e416cc5 100644 --- a/src/drain.rs +++ b/src/drain.rs @@ -1,3 +1,5 @@ +#[allow(unused)] // some bug in <= 1.48.0 sees this as unused when it isn't +use crate::float::FloatExt; use crate::{varint_size, FeeRate, TR_KEYSPEND_TXIN_WEIGHT, TR_SPK_WEIGHT, TXOUT_BASE_WEIGHT}; /// Represents the weight costs of a drain (a.k.a. change) output. @@ -93,3 +95,48 @@ impl Drain { !self.is_none() } } + +/// Describes when a change output (although it could represent several) should be added that drains +/// the excess in the coin selection. It includes the `drain_weights` to account for the cost of +/// adding this outupt(s). +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct ChangePolicy { + /// The minimum amount of excesss there needs to be add a change output. + pub min_value: u64, + /// The weights of the drain that would be added according to the policy. + pub drain_weights: DrainWeights, +} + +impl ChangePolicy { + /// Construct a change policy that creates change when the change value is greater than + /// `min_value`. + pub fn min_value(drain_weights: DrainWeights, min_value: u64) -> Self { + Self { + drain_weights, + min_value, + } + } + + /// Construct a change policy that creates change when it would reduce the transaction waste + /// given that `min_value` is respected. + pub fn min_value_and_waste( + drain_weights: DrainWeights, + min_value: u64, + target_feerate: FeeRate, + long_term_feerate: FeeRate, + ) -> Self { + // The output waste of a changeless solution is the excess. + let waste_with_change = drain_weights + .waste( + target_feerate, + long_term_feerate, + 0, /* ignore varint cost for now */ + ) + .ceil() as u64; + + Self { + drain_weights, + min_value: waste_with_change.max(min_value), + } + } +} diff --git a/src/lib.rs b/src/lib.rs index f7b109d..e4c926b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,8 +22,6 @@ pub mod metrics; mod feerate; pub use feerate::*; -mod change_policy; -pub use change_policy::*; mod target; pub use target::*; mod drain; diff --git a/src/metrics.rs b/src/metrics.rs index 4554e3d..4b52572 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -1,8 +1,6 @@ //! Branch and bound metrics that can be passed to [`CoinSelector::bnb_solutions`] or //! [`CoinSelector::run_bnb`]. -use crate::{ - bnb::BnbMetric, change_policy::ChangePolicy, float::Ordf32, CoinSelector, Drain, Target, -}; +use crate::{bnb::BnbMetric, float::Ordf32, ChangePolicy, CoinSelector, Drain, Target}; mod lowest_fee; pub use lowest_fee::*; mod changeless; diff --git a/src/metrics/changeless.rs b/src/metrics/changeless.rs index 3dd784a..eddaa10 100644 --- a/src/metrics/changeless.rs +++ b/src/metrics/changeless.rs @@ -1,5 +1,5 @@ use super::change_lower_bound; -use crate::{bnb::BnbMetric, change_policy::ChangePolicy, float::Ordf32, CoinSelector, Target}; +use crate::{bnb::BnbMetric, float::Ordf32, ChangePolicy, CoinSelector, Target}; #[derive(Clone, Debug)] /// Metric for finding changeless solutions only. diff --git a/src/metrics/lowest_fee.rs b/src/metrics/lowest_fee.rs index 30e4f21..396fe03 100644 --- a/src/metrics/lowest_fee.rs +++ b/src/metrics/lowest_fee.rs @@ -1,6 +1,4 @@ -use crate::{ - change_policy::ChangePolicy, float::Ordf32, BnbMetric, CoinSelector, Drain, FeeRate, Target, -}; +use crate::{float::Ordf32, BnbMetric, ChangePolicy, CoinSelector, Drain, FeeRate, Target}; /// Metric that aims to minimize transaction fees. The future fee for spending the change output is /// included in this calculation. From c6f0682c93aad102eeb5c11bc6fafe2e51c5c30b Mon Sep 17 00:00:00 2001 From: LLFourn Date: Thu, 25 Jan 2024 17:51:52 +1100 Subject: [PATCH 3/9] Remove typo in lowest_fee comment --- src/metrics/lowest_fee.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/metrics/lowest_fee.rs b/src/metrics/lowest_fee.rs index 396fe03..873cf3c 100644 --- a/src/metrics/lowest_fee.rs +++ b/src/metrics/lowest_fee.rs @@ -35,7 +35,7 @@ impl BnbMetric for LowestFee { fee_for_the_tx > 0, "must not be called unless selection has met target" ); - // Why `spend_fee` rounds up here. We could use floats but I felt it was just better to + // `spend_fee` rounds up here. We could use floats but I felt it was just better to // accept the extra 1 sat penality to having a change output let fee_for_spending_drain = drain.weights.spend_fee(self.long_term_feerate); fee_for_the_tx as u64 + fee_for_spending_drain From 956685a72d25c433f791bd1da5a3581d8c8c2b07 Mon Sep 17 00:00:00 2001 From: LLFourn Date: Mon, 29 Jan 2024 17:26:29 +1100 Subject: [PATCH 4/9] [changeless] Revert number of candidates changeless should not be that slow but if it fails to find a changeless solution then the test will exhaustive search so that clearly will take forever. --- tests/changeless.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/changeless.rs b/tests/changeless.rs index 9026119..4b085ba 100644 --- a/tests/changeless.rs +++ b/tests/changeless.rs @@ -29,7 +29,7 @@ proptest! { #[test] #[cfg(not(debug_assertions))] // too slow if compiling for debug fn compare_against_benchmarks( - n_candidates in 0..50_usize, // candidates (n) + n_candidates in 0..15_usize, // candidates (n) target_value in 500..1_000_000_u64, // target value (sats) n_target_outputs in 1..150_usize, // the number of outputs we're funding target_weight in 0..10_000_u32, // the sum of the weight of the outputs (wu) From 064996c0b72ff38fed5f2d6248c0672d5c1f41fd Mon Sep 17 00:00:00 2001 From: LLFourn Date: Tue, 30 Jan 2024 11:31:57 +1100 Subject: [PATCH 5/9] [lowest-fee] Fix off by one in test --- tests/lowest_fee.proptest-regressions | 1 + tests/lowest_fee.rs | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/lowest_fee.proptest-regressions b/tests/lowest_fee.proptest-regressions index 9a23f55..ed2f8c4 100644 --- a/tests/lowest_fee.proptest-regressions +++ b/tests/lowest_fee.proptest-regressions @@ -9,3 +9,4 @@ cc e30499b75a1846759fc9ffd7ee558b08a4795598cf7919f6be6d62cc7a79d4cb # shrinks to cc c580ee452624915fc710d5fe724c7a9347472ccd178f66c9db9479cfc6168f48 # shrinks to n_candidates = 25, target_value = 488278, base_weight = 242, min_fee = 0, feerate = 6.952743, feerate_lt_diff = 0.0, drain_weight = 100, drain_spend_weight = 1, drain_dust = 100 cc 850e0115aeeb7ed50235fdb4b5183eb5bf8309a45874dc261e3d3fd2d8c84660 # shrinks to n_candidates = 8, target_value = 444541, base_weight = 253, min_fee = 0, feerate = 55.98181, feerate_lt_diff = 36.874306, drain_weight = 490, drain_spend_weight = 1779, drain_dust = 100 cc ca9831dfe4ad27fc705ae4af201b9b50739404bda0e1e130072858634f8d25d9 # added by llfourn from ci: has a case where the lowest fee metric would benefit by adding change +cc 85d9dc968a690553484d0f166f3d778c3e0ec7d9059809c2818b62d6609853c1 diff --git a/tests/lowest_fee.rs b/tests/lowest_fee.rs index f78704a..19694b3 100644 --- a/tests/lowest_fee.rs +++ b/tests/lowest_fee.rs @@ -99,9 +99,10 @@ proptest! { change_policy, }; - let (score, rounds) = common::bnb_search(&mut cs, metric, params.n_candidates)?; - println!("\t\tscore={} rounds={}", score, rounds); - prop_assert!(rounds <= params.n_candidates); + let (score, rounds) = common::bnb_search(&mut cs, metric, params.n_candidates * 10)?; + // the +1 is because the iterator will always try selecting nothing as a solution so we have + // to do one extra iteration to try that + prop_assert!(rounds <= params.n_candidates + 1, "\t\tscore={} rounds={}", score, rounds); } #[test] From 8eb582bc08bee173a41508a915fd2609e954f313 Mon Sep 17 00:00:00 2001 From: LLFourn Date: Tue, 30 Jan 2024 13:42:23 +1100 Subject: [PATCH 6/9] [lowest-fee] ignore failure when selection is not possible --- tests/lowest_fee.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/lowest_fee.rs b/tests/lowest_fee.rs index 19694b3..4e3ba99 100644 --- a/tests/lowest_fee.rs +++ b/tests/lowest_fee.rs @@ -98,11 +98,15 @@ proptest! { long_term_feerate: params.long_term_feerate(), change_policy, }; - - let (score, rounds) = common::bnb_search(&mut cs, metric, params.n_candidates * 10)?; - // the +1 is because the iterator will always try selecting nothing as a solution so we have - // to do one extra iteration to try that - prop_assert!(rounds <= params.n_candidates + 1, "\t\tscore={} rounds={}", score, rounds); + let is_impossible = !cs.is_selection_possible(params.target()); + match common::bnb_search(&mut cs, metric, params.n_candidates * 10) { + Ok((score, rounds)) => { + // the +1 is because the iterator will always try selecting nothing as a solution so we have + // to do one extra iteration to try that + prop_assert!(rounds <= params.n_candidates + 1, "\t\tscore={} rounds={}", score, rounds) + }, + Err(_e) => assert!(is_impossible), + } } #[test] From 2e26aa5802e755c2057110e59655a70515a3a44f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Fri, 29 Mar 2024 16:27:29 +0800 Subject: [PATCH 7/9] fix: rm `Drain::none` method We already have const `Drain::NONE` which should be used instead. --- src/coin_selector.rs | 6 +++--- src/drain.rs | 5 ----- src/metrics.rs | 2 +- src/metrics/lowest_fee.rs | 6 +++--- tests/bnb.rs | 4 ++-- tests/lowest_fee.rs | 4 ++-- tests/weight.rs | 2 +- 7 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/coin_selector.rs b/src/coin_selector.rs index a9c169e..f343657 100644 --- a/src/coin_selector.rs +++ b/src/coin_selector.rs @@ -391,7 +391,7 @@ impl<'a> CoinSelector<'a> { /// Whether the constraints of `Target` have been met. pub fn is_target_met(&self, target: Target) -> bool { - self.is_target_met_with_drain(target, Drain::none()) + self.is_target_met_with_drain(target, Drain::NONE) } /// Select all unselected candidates @@ -434,7 +434,7 @@ impl<'a> CoinSelector<'a> { } /// Figures out whether the current selection should have a change output given the - /// `change_policy`. If it shouldn't then it will return a [`Drain::none`]. The value of the + /// `change_policy`. If it should not, then it will return [`Drain::NONE`]. The value of the /// `Drain` will be the same as [`drain_value`]. /// /// If [`is_target_met`] returns true for this selection then [`is_target_met_with_drain`] will @@ -475,7 +475,7 @@ impl<'a> CoinSelector<'a> { pub fn select_until_target_met(&mut self, target: Target) -> Result<(), InsufficientFunds> { self.select_until(|cs| cs.is_target_met(target)) .ok_or_else(|| InsufficientFunds { - missing: self.excess(target, Drain::none()).unsigned_abs(), + missing: self.excess(target, Drain::NONE).unsigned_abs(), }) } diff --git a/src/drain.rs b/src/drain.rs index e416cc5..8d116bc 100644 --- a/src/drain.rs +++ b/src/drain.rs @@ -80,11 +80,6 @@ impl Drain { value: 0, }; - /// A drian representing no drain at all. - pub fn none() -> Self { - Self::default() - } - /// is the "none" drain pub fn is_none(&self) -> bool { self == &Drain::NONE diff --git a/src/metrics.rs b/src/metrics.rs index 4b52572..4e7d90d 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -28,7 +28,7 @@ fn change_lower_bound(cs: &CoinSelector, target: Target, change_policy: ChangePo least_excess.drain(target, change_policy) } else { - Drain::none() + Drain::NONE } } diff --git a/src/metrics/lowest_fee.rs b/src/metrics/lowest_fee.rs index 873cf3c..4b348f2 100644 --- a/src/metrics/lowest_fee.rs +++ b/src/metrics/lowest_fee.rs @@ -98,7 +98,7 @@ impl BnbMetric for LowestFee { self.long_term_feerate, self.target.outputs.n_outputs, ); - let cost_of_no_change = cs.excess(self.target, Drain::none()); + let cost_of_no_change = cs.excess(self.target, Drain::NONE); let best_score_with_change = Ordf32(current_score.0 - cost_of_no_change as f32 + cost_of_adding_change); @@ -131,7 +131,7 @@ impl BnbMetric for LowestFee { // scale = remaining_value_to_reach_feerate / effective_value_of_resized_input // // This should be intutive since we're finding out how to scale the input we're resizing to get the effective value we need. - let rate_excess = cs.rate_excess(self.target, Drain::none()) as f32; + let rate_excess = cs.rate_excess(self.target, Drain::NONE) as f32; let mut scale = Ordf32(0.0); if rate_excess < 0.0 { @@ -150,7 +150,7 @@ impl BnbMetric for LowestFee { // We can use the same approach for replacement we just have to use the // incremental_relay_feerate. if let Some(replace) = self.target.fee.replace { - let replace_excess = cs.replacement_excess(self.target, Drain::none()) as f32; + let replace_excess = cs.replacement_excess(self.target, Drain::NONE) as f32; if replace_excess < 0.0 { let remaining_value_to_reach_feerate = replace_excess.abs(); let effective_value_of_resized_input = diff --git a/tests/bnb.rs b/tests/bnb.rs index 434050c..cab483b 100644 --- a/tests/bnb.rs +++ b/tests/bnb.rs @@ -35,7 +35,7 @@ const EXCESS_RATIO: f32 = 1_000_000_f32; impl BnbMetric for MinExcessThenWeight { fn score(&mut self, cs: &CoinSelector<'_>) -> Option { - let excess = cs.excess(self.target, Drain::none()); + let excess = cs.excess(self.target, Drain::NONE); if excess < 0 { None } else { @@ -135,7 +135,7 @@ fn bnb_finds_solution_if_possible_in_n_iter() { .expect("found a solution"); assert_eq!(rounds, 193); - let excess = sol.excess(target, Drain::none()); + let excess = sol.excess(target, Drain::NONE); assert_eq!(excess, 1); } diff --git a/tests/lowest_fee.rs b/tests/lowest_fee.rs index 4e3ba99..3a6bf69 100644 --- a/tests/lowest_fee.rs +++ b/tests/lowest_fee.rs @@ -222,7 +222,7 @@ fn adding_another_input_to_remove_change() { let mut cs = cs.clone(); cs.select(0); cs.select(2); - cs.excess(target, Drain::none()); + cs.excess(target, Drain::NONE); assert!(cs.is_target_met(target)); cs }; @@ -262,7 +262,7 @@ fn adding_another_input_to_remove_change() { let (score, _) = common::bnb_search(&mut cs, metric, 10).expect("finds solution"); let best_solution_score = metric.score(&best_solution).expect("must be a solution"); - assert_eq!(best_solution.drain(target, change_policy), Drain::none()); + assert_eq!(best_solution.drain(target, change_policy), Drain::NONE); assert!(score <= best_solution_score); assert_eq!(cs.selected_indices(), best_solution.selected_indices()); diff --git a/tests/weight.rs b/tests/weight.rs index cbb5900..b7b3eb2 100644 --- a/tests/weight.rs +++ b/tests/weight.rs @@ -101,7 +101,7 @@ fn segwit_two_inputs_one_output() { ); assert_eq!( (coin_selector - .implied_feerate(target_ouputs, Drain::none()) + .implied_feerate(target_ouputs, Drain::NONE) .unwrap() .as_sat_vb() * 10.0) From 4d0ae4c710aa521fb266c8da22daf7fe16d8b32e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Fri, 29 Mar 2024 16:34:46 +0800 Subject: [PATCH 8/9] docs: fix typos --- src/coin_selector.rs | 3 ++- src/drain.rs | 2 +- src/lib.rs | 2 +- src/metrics/lowest_fee.rs | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coin_selector.rs b/src/coin_selector.rs index f343657..55fb710 100644 --- a/src/coin_selector.rs +++ b/src/coin_selector.rs @@ -164,7 +164,8 @@ impl<'a> CoinSelector<'a> { /// Current weight of transaction implied by the selection. /// - /// If you don't have any drain outputs (only target outputs) just set drain_weights tio [`DrainWeights::NONE`]. + /// If you don't have any drain outputs (only target outputs) just set drain_weights to + /// [`DrainWeights::NONE`]. pub fn weight(&self, target_ouputs: TargetOutputs, drain_weight: DrainWeights) -> u32 { TX_FIXED_FIELD_WEIGHT + self.input_weight() diff --git a/src/drain.rs b/src/drain.rs index 8d116bc..1881905 100644 --- a/src/drain.rs +++ b/src/drain.rs @@ -96,7 +96,7 @@ impl Drain { /// adding this outupt(s). #[derive(Clone, Copy, Debug, PartialEq)] pub struct ChangePolicy { - /// The minimum amount of excesss there needs to be add a change output. + /// The minimum amount of excess there needs to be add a change output. pub min_value: u64, /// The weights of the drain that would be added according to the policy. pub drain_weights: DrainWeights, diff --git a/src/lib.rs b/src/lib.rs index e4c926b..9bdabbd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -45,7 +45,7 @@ pub const TX_FIXED_FIELD_WEIGHT: u32 = (4 /* nVersion */ + 4/* nLockTime */) * 4 /// The weight of a taproot keyspend witness pub const TR_KEYSPEND_SATISFACTION_WEIGHT: u32 = 1 /*witness_len*/ + 1 /*item len*/ + 64 /*signature*/; -/// The weight of a segwit `v1` (taproot) script pubkey in an outupt. This does not include the weight of +/// The weight of a segwit `v1` (taproot) script pubkey in an output. This does not include the weight of /// the `TxOut` itself or the script pubkey length field. pub const TR_SPK_WEIGHT: u32 = (1 + 1 + 32) * 4; // version + push + key diff --git a/src/metrics/lowest_fee.rs b/src/metrics/lowest_fee.rs index 4b348f2..9e3e569 100644 --- a/src/metrics/lowest_fee.rs +++ b/src/metrics/lowest_fee.rs @@ -69,7 +69,7 @@ impl BnbMetric for LowestFee { if ev < -0.0 { let value_per_negative_effective_value = low_sats_per_wu_candidate.value as f32 / ev.abs(); - // this is how much abosolute value we have to add to cancel out the excess + // this is how much absolute value we have to add to cancel out the excess let extra_value_needed_to_get_rid_of_change = amount_above_change_threshold as f32 * value_per_negative_effective_value; From c6503060454e4cf8e04990c89c9e30177af77d17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Fri, 29 Mar 2024 17:24:09 +0800 Subject: [PATCH 9/9] fix: `CoinSelector::implied_fee` The logic was wrong before because it was making the RBF constraints take precedence. Instead, we should be taking the maximum between the fee calculated from the target feerate and the minimum fee that satisfies the RBF constraints. --- src/coin_selector.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/coin_selector.rs b/src/coin_selector.rs index 55fb710..e23729a 100644 --- a/src/coin_selector.rs +++ b/src/coin_selector.rs @@ -229,13 +229,18 @@ impl<'a> CoinSelector<'a> { /// The fee the current selection and `drain_weight` should pay to satisfy `target_fee`. /// - /// `drain_weight` can be 0 to indicate no draining output + /// This compares the fee calculated from the target feerate with the fee calculated from the + /// [`Replace`] constraints and returns the larger of the two. + /// + /// `drain_weight` can be 0 to indicate no draining output. pub fn implied_fee(&self, target: Target, drain_weights: DrainWeights) -> u64 { let mut implied_fee = self.implied_fee_from_feerate(target, drain_weights); if let Some(replace) = target.fee.replace { - implied_fee = - replace.min_fee_to_do_replacement(self.weight(target.outputs, drain_weights)); + implied_fee = Ord::max( + implied_fee, + replace.min_fee_to_do_replacement(self.weight(target.outputs, drain_weights)), + ); } implied_fee