diff --git a/example-crates/example_cli/src/lib.rs b/example-crates/example_cli/src/lib.rs index 47044d82f6..1706a238dc 100644 --- a/example-crates/example_cli/src/lib.rs +++ b/example-crates/example_cli/src/lib.rs @@ -1,6 +1,6 @@ pub use anyhow; use anyhow::Context; -use bdk_coin_select::{Candidate, CoinSelector}; +use bdk_coin_select::{Candidate, CoinSelector, Drain}; use bdk_file_store::Store; use serde::{de::DeserializeOwned, Serialize}; use std::{cmp::Reverse, collections::HashMap, path::PathBuf, sync::Mutex}; @@ -466,13 +466,13 @@ where }; let target = bdk_coin_select::Target { - feerate: bdk_coin_select::FeeRate::from_sat_per_vb(1.0), + feerate: bdk_coin_select::FeeRate::from_sat_per_vb(5.0), min_fee: 0, value: transaction.output.iter().map(|txo| txo.value).sum(), }; - let drain = bdk_coin_select::Drain { - weight: { + let drain_weights = bdk_coin_select::DrainWeights { + output_weight: { // we calculate the weight difference of including the drain output in the base tx // this method will detect varint size changes of txout count let tx_weight = transaction.weight(); @@ -486,11 +486,14 @@ where }; (tx_weight_with_drain - tx_weight).to_wu() as u32 - 1 }, - value: 0, spend_weight: change_plan.expected_weight() as u32, }; - let long_term_feerate = bdk_coin_select::FeeRate::from_sat_per_wu(0.25); - let drain_policy = bdk_coin_select::change_policy::min_waste(drain, long_term_feerate); + let long_term_feerate = bdk_coin_select::FeeRate::from_sat_per_vb(1.0); + let drain_policy = bdk_coin_select::change_policy::min_value_and_waste( + drain_weights, + change_script.dust_value().to_sat(), + long_term_feerate, + ); let mut selector = CoinSelector::new(&candidates, transaction.weight().to_wu() as u32); match cs_algorithm { @@ -503,16 +506,10 @@ where let (final_selection, _score) = selector .branch_and_bound(metric) .take(50_000) - // we only process viable solutions + // skip exclusion branches (as they are not scored) .flatten() - .reduce(|(best_sol, best_score), (curr_sol, curr_score)| { - // we are reducing waste - if curr_score < best_score { - (curr_sol, curr_score) - } else { - (best_sol, best_score) - } - }) + // the last result is always the best score + .last() .ok_or(anyhow::format_err!("no bnb solution found"))?; selector = final_selection; } @@ -531,7 +528,13 @@ where }), CoinSelectionAlgo::BranchAndBound => unreachable!("bnb variant is matched already"), } - selector.select_until_target_met(target, drain)? + selector.select_until_target_met( + target, + Drain { + weights: drain_weights, + value: 0, + }, + )? } }; diff --git a/nursery/coin_select/src/change_policy.rs b/nursery/coin_select/src/change_policy.rs index b86199f1dd..3ae5bceabb 100644 --- a/nursery/coin_select/src/change_policy.rs +++ b/nursery/coin_select/src/change_policy.rs @@ -1,28 +1,34 @@ #[allow(unused)] // some bug in <= 1.48.0 sees this as unused when it isn't use crate::float::FloatExt; -use crate::{CoinSelector, Drain, FeeRate, Target}; +use crate::{CoinSelector, Drain, DrainWeights, FeeRate, Target}; use core::convert::TryInto; /// Add a change output if the change value would be greater than or equal to `min_value`. /// /// Note that the value field of the `drain` is ignored. -pub fn min_value(mut drain: Drain, min_value: u64) -> impl Fn(&CoinSelector, Target) -> Drain { - debug_assert!(drain.is_some()); +pub fn min_value( + drain_weights: DrainWeights, + min_value: u64, +) -> impl Fn(&CoinSelector, Target) -> Drain { let min_value: i64 = min_value .try_into() .expect("min_value is ridiculously large"); - drain.value = 0; + move |cs, target| { + let mut drain = Drain { + weights: drain_weights, + ..Default::default() + }; + let excess = cs.excess(target, drain); - if excess >= min_value { - let mut drain = drain; - drain.value = excess.try_into().expect( - "cannot be negative since we checked it against min_value which is positive", - ); - drain - } else { - Drain::none() + if excess < min_value { + return Drain::none(); } + + drain.value = excess + .try_into() + .expect("must be positive since it is greater than min_value (which is positive)"); + drain } } @@ -31,23 +37,49 @@ pub fn min_value(mut drain: Drain, min_value: u64) -> impl Fn(&CoinSelector, Tar /// Note that the value field of the `drain` is ignored. /// The `value` will be set to whatever needs to be to reach the given target. pub fn min_waste( - mut drain: Drain, + drain_weights: DrainWeights, + long_term_feerate: FeeRate, +) -> impl Fn(&CoinSelector, Target) -> Drain { + move |cs, target| { + // The output waste of a changeless solution is the excess. + let waste_changeless = cs.excess(target, Drain::none()); + let waste_with_change = drain_weights + .waste(target.feerate, long_term_feerate) + .ceil() as i64; + + if waste_changeless <= waste_with_change { + return Drain::none(); + } + + let mut drain = Drain { + weights: drain_weights, + value: 0, + }; + drain.value = cs + .excess(target, drain) + .try_into() + .expect("the excess must be positive because drain free excess was > waste"); + drain + } +} + +/// Add a change output if the change value is greater than or equal to `min_value` and if it would +/// reduce the overall waste of the transaction. +/// +/// Note that the value field of the `drain` is ignored. [`Drain`] is just used for the drain weight +/// and drain spend weight. +pub fn min_value_and_waste( + drain_weights: DrainWeights, + min_value: u64, long_term_feerate: FeeRate, ) -> impl Fn(&CoinSelector, Target) -> Drain { - debug_assert!(drain.is_some()); - drain.value = 0; + let min_waste_policy = crate::change_policy::min_waste(drain_weights, long_term_feerate); move |cs, target| { - let excess = cs.excess(target, Drain::none()); - if excess > drain.waste(target.feerate, long_term_feerate).ceil() as i64 { - let mut drain = drain; - drain.value = cs - .excess(target, drain) - .try_into() - .expect("the excess must be positive because drain free excess was > waste"); - drain - } else { - Drain::none() + let drain = min_waste_policy(cs, target); + if drain.value < min_value { + return Drain::none(); } + drain } } diff --git a/nursery/coin_select/src/coin_selector.rs b/nursery/coin_select/src/coin_selector.rs index dc30a01b40..ca106825cc 100644 --- a/nursery/coin_select/src/coin_selector.rs +++ b/nursery/coin_select/src/coin_selector.rs @@ -208,14 +208,14 @@ impl<'a> CoinSelector<'a> { self.base_weight + self.input_weight() + drain_weight } - /// How much the current selection overshoots the value needed to acheive `target`. + /// 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. pub fn excess(&self, target: Target, drain: Drain) -> i64 { self.selected_value() as i64 - target.value as i64 - drain.value as i64 - - self.implied_fee(target.feerate, target.min_fee, drain.weight) as i64 + - self.implied_fee(target.feerate, target.min_fee, drain.weights.output_weight) as i64 } /// How much the current selection overshoots the value need to satisfy `target.feerate` and @@ -224,7 +224,7 @@ impl<'a> CoinSelector<'a> { self.selected_value() as i64 - target.value as i64 - drain.value as i64 - - self.implied_fee_from_feerate(target.feerate, drain.weight) as i64 + - self.implied_fee_from_feerate(target.feerate, drain.weights.output_weight) as i64 } /// How much the current selection overshoots the value needed to satisfy `target.min_fee` and @@ -236,11 +236,11 @@ impl<'a> CoinSelector<'a> { - target.min_fee as i64 } - /// The feerate the transaction would have if we were to use this selection of inputs to acheive - /// the `target_value` + /// The feerate the transaction would have if we were to use this selection of inputs to achieve + /// the `target_value`. pub fn implied_feerate(&self, target_value: u64, drain: Drain) -> FeeRate { let numerator = self.selected_value() as i64 - target_value as i64 - drain.value as i64; - let denom = self.weight(drain.weight); + let denom = self.weight(drain.weights.output_weight); FeeRate::from_sat_per_wu(numerator as f32 / denom as f32) } @@ -327,8 +327,8 @@ impl<'a> CoinSelector<'a> { excess_waste *= excess_discount.max(0.0).min(1.0); waste += excess_waste; } else { - waste += drain.weight as f32 * target.feerate.spwu() - + drain.spend_weight as f32 * long_term_feerate.spwu(); + waste += drain.weights.output_weight as f32 * target.feerate.spwu() + + drain.weights.spend_weight as f32 * long_term_feerate.spwu(); } waste @@ -514,6 +514,34 @@ impl Candidate { } } +/// A structure that represents the weight costs of a drain (a.k.a. change) output. +/// +/// This structure can also represent multiple outputs. +#[derive(Default, Debug, Clone, Copy, Hash, PartialEq, Eq)] +pub struct DrainWeights { + /// The weight of adding this drain output. + pub output_weight: u32, + /// The weight of spending this drain output (in the future). + pub spend_weight: u32, +} + +impl DrainWeights { + /// 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.output_weight as f32 * feerate.spwu() + + self.spend_weight as f32 * long_term_feerate.spwu() + } + + pub fn new_tr_keyspend() -> Self { + Self { + output_weight: TXOUT_BASE_WEIGHT + TR_SPK_WEIGHT, + spend_weight: TXIN_BASE_WEIGHT + TR_KEYSPEND_SATISFACTION_WEIGHT, + } + } +} + /// A drain (A.K.A. change) output. /// Technically it could represent multiple outputs. /// @@ -522,12 +550,10 @@ impl Candidate { /// [`change_policy`]: crate::change_policy #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Default)] pub struct Drain { - /// The weight of adding this drain - pub weight: u32, - /// The value that should be assigned to the drain + /// Weight of adding drain output and spending the drain output. + pub weights: DrainWeights, + /// The value that should be assigned to the drain. pub value: u64, - /// The weight of spending this drain - pub spend_weight: u32, } impl Drain { @@ -546,19 +572,11 @@ impl Drain { !self.is_none() } - pub fn new_tr_keyspend() -> Self { - Self { - weight: TXOUT_BASE_WEIGHT + TR_SPK_WEIGHT, - value: 0, - spend_weight: TXIN_BASE_WEIGHT + TR_KEYSPEND_SATISFACTION_WEIGHT, - } - } - /// 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.weight as f32 * feerate.spwu() + self.spend_weight as f32 * long_term_feerate.spwu() + self.weights.waste(feerate, long_term_feerate) } } diff --git a/nursery/coin_select/tests/waste.rs b/nursery/coin_select/tests/waste.rs index dc0fad499b..2a4fdfdbe6 100644 --- a/nursery/coin_select/tests/waste.rs +++ b/nursery/coin_select/tests/waste.rs @@ -1,5 +1,6 @@ use bdk_coin_select::{ - change_policy, float::Ordf32, metrics::Waste, Candidate, CoinSelector, Drain, FeeRate, Target, + change_policy, float::Ordf32, metrics::Waste, Candidate, CoinSelector, Drain, DrainWeights, + FeeRate, Target, }; use proptest::{ prelude::*, @@ -21,13 +22,12 @@ fn waste_all_selected_except_one_is_optimal_and_awkward() { 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 = Drain { - weight: change_weight, + let drain_weights = DrainWeights { + output_weight: change_weight, spend_weight: change_spend_weight, - value: 0, }; - let change_policy = change_policy::min_waste(drain, long_term_feerate); + let change_policy = change_policy::min_waste(drain_weights, long_term_feerate); let wv = test_wv(&mut rng); let candidates = wv.take(num_inputs).collect::>(); @@ -76,13 +76,16 @@ fn waste_naive_effective_value_shouldnt_be_better() { 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 = Drain { - weight: change_weight, + let drain_weights = DrainWeights { + output_weight: change_weight, spend_weight: change_spend_weight, + }; + let drain = Drain { + weights: drain_weights, value: 0, }; - let change_policy = change_policy::min_waste(drain, long_term_feerate); + let change_policy = change_policy::min_waste(drain_weights, long_term_feerate); let wv = test_wv(&mut rng); let candidates = wv.take(num_inputs).collect::>(); @@ -137,13 +140,12 @@ fn waste_doesnt_take_too_long_to_finish() { 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 = Drain { - weight: change_weight, + let drain_weights = DrainWeights { + output_weight: change_weight, spend_weight: change_spend_weight, - value: 0, }; - let change_policy = change_policy::min_waste(drain, long_term_feerate); + let change_policy = change_policy::min_waste(drain_weights, long_term_feerate); let wv = test_wv(&mut rng); let candidates = wv.take(num_inputs).collect::>(); @@ -190,13 +192,12 @@ fn waste_lower_long_term_feerate_but_still_need_to_select_all() { 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 = Drain { - weight: change_weight, + let drain_weights = DrainWeights { + output_weight: change_weight, spend_weight: change_spend_weight, - value: 0, }; - let change_policy = change_policy::min_waste(drain, long_term_feerate); + let change_policy = change_policy::min_waste(drain_weights, long_term_feerate); let wv = test_wv(&mut rng); let candidates = wv.take(num_inputs).collect::>(); @@ -249,13 +250,12 @@ fn waste_low_but_non_negative_rate_diff_means_adding_more_inputs_might_reduce_ex 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 = Drain { - weight: change_weight, + let drain_weights = DrainWeights { + output_weight: change_weight, spend_weight: change_spend_weight, - value: 0, }; - let change_policy = change_policy::min_waste(drain, long_term_feerate); + let change_policy = change_policy::min_waste(drain_weights, 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