diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..dfac56f --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,56 @@ +on: + push: + branches: + - master + pull_request: + + +# Make sure CI fails on all warnings, including Clippy lints +env: + RUSTFLAGS: "-Dwarnings" + RUSTDOCFLAGS: "-Dwarnings" + +jobs: + fmt: + name: Rustfmt + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + with: + components: rustfmt + - run: cargo fmt --all -- --check + + clippy_check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + with: + components: clippy + - uses: Swatinem/rust-cache@v2 + - run: cargo clippy --all-targets --all-features --tests + + build-msrv: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@1.54.0 + # don't want dev-dependencies for MSRV check + - run: sed -i 's/\[dev-dependencies]/[ignore-this-warning-fren]/g' Cargo.toml + - run: cargo build --release + + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - run: cargo test --release + + doc-build: + name: doc-build + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - run: cargo doc --no-deps diff --git a/Cargo.toml b/Cargo.toml index 12e5c08..f2accde 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,11 +15,12 @@ readme = "README.md" [dependencies] # No dependencies! Please do not add any please! -[dev-dependencies] -rand = "0.7" -proptest = "0.10" -bitcoin = "0.30" [features] default = ["std"] std = [] + +[dev-dependencies] +rand = "0.8" +proptest = "1.4" +bitcoin = "0.30" diff --git a/README.md b/README.md index f5ab5ac..048a0cd 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,9 @@ `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. +> ⚠ 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 @@ -11,18 +14,12 @@ and mandatory inputs (if any). ```rust use std::str::FromStr; -use bdk_coin_select::{ CoinSelector, Candidate, TR_KEYSPEND_SATISFACTION_WEIGHT, TXIN_BASE_WEIGHT }; +use bdk_coin_select::{ CoinSelector, Candidate, TR_KEYSPEND_TXIN_WEIGHT}; use bitcoin::{ Address, Network, Transaction, TxIn, TxOut }; -// You should use miniscript to figure out the satisfaction weight for your coins! -const TR_INPUT_WEIGHT: u32 = TXIN_BASE_WEIGHT + TR_KEYSPEND_SATISFACTION_WEIGHT; - // The address where we want to send our coins. let recipient_addr = - Address::from_str("tb1pvjf9t34fznr53u5tqhejz4nr69luzkhlvsdsdfq9pglutrpve2xq7hps46") - .expect("address must be valid") - .require_network(Network::Testnet) - .expect("network must match"); + Address::from_str("tb1pvjf9t34fznr53u5tqhejz4nr69luzkhlvsdsdfq9pglutrpve2xq7hps46").unwrap(); let candidates = vec![ Candidate { @@ -32,7 +29,8 @@ let candidates = vec![ // the value of the input value: 1_000_000, // the total weight of the input(s). - weight: TR_INPUT_WEIGHT, + // you may need to use miniscript to figure out the correct value here. + 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 @@ -41,7 +39,7 @@ let candidates = vec![ // A candidate can represent multiple inputs in the case where you // always want some inputs to be spent together. input_count: 2, - weight: 2*TR_INPUT_WEIGHT, + weight: 2*TR_KEYSPEND_TXIN_WEIGHT, value: 3_000_000, is_segwit: true } @@ -52,7 +50,7 @@ let base_tx = Transaction { // include your recipient outputs here output: vec![TxOut { value: 900_000, - script_pubkey: recipient_addr.script_pubkey(), + script_pubkey: recipient_addr.payload.script_pubkey(), }], lock_time: bitcoin::absolute::LockTime::from_height(0).unwrap(), version: 0x02, @@ -67,30 +65,25 @@ coin_selector.select(0); ## Change Policy -A change policy determines whether the drain output(s) should be in the final solution. A change -policy is represented by a closure of signature `Fn(&CoinSelector, Target) -> Drain`. We provide 3 -built-in change policies; `min_value`, `min_waste` and `min_value_and_waste` (refer to the -[module-level docs](crate::change_policy) for more). +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). -Typically, to construct a change policy, the [`DrainWeights`] need to be provided. `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). ```rust -# use std::str::FromStr; -# use bdk_coin_select::{ CoinSelector, Candidate, DrainWeights, TXIN_BASE_WEIGHT }; -# use bitcoin::{ Address, Network, Transaction, TxIn, TxOut }; -use bdk_coin_select::change_policy::min_value; -# const TR_SATISFACTION_WEIGHT: u32 = 66; -# const TR_INPUT_WEIGHT: u32 = TXIN_BASE_WEIGHT + TR_SATISFACTION_WEIGHT; -# let base_tx = Transaction { -# input: vec![], -# // include your recipient outputs here -# output: vec![], -# lock_time: bitcoin::absolute::LockTime::from_height(0).unwrap(), -# version: 1, -# }; -# let base_weight = base_tx.weight().to_wu() as u32; +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}; +const TR_SATISFACTION_WEIGHT: u32 = 66; +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 = @@ -114,12 +107,12 @@ println!("drain output weight: {}", drain_output_weight); let drain_weights = DrainWeights { output_weight: drain_output_weight, - spend_weight: TR_INPUT_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 = min_value( +let change_policy = ChangePolicy::min_value( drain_weights, drain_addr.script_pubkey().dust_value().to_sat(), ); @@ -136,14 +129,13 @@ Built-in metrics are provided in the [`metrics`] submodule. Currently, only the [`LowestFee`](metrics::LowestFee) metric is considered stable. ```rust -use bdk_coin_select::{ Candidate, CoinSelector, FeeRate, Target }; +use bdk_coin_select::{ Candidate, CoinSelector, FeeRate, Target, ChangePolicy }; use bdk_coin_select::metrics::LowestFee; -use bdk_coin_select::change_policy::min_value_and_waste; -# let candidates = []; -# let base_weight = 0; -# let drain_weights = bdk_coin_select::DrainWeights::default(); -# let dust_limit = 0; -# let long_term_feerate = FeeRate::default_min_relay_fee(); +let candidates = []; +let base_weight = 0; +let drain_weights = bdk_coin_select::DrainWeights::default(); +let dust_limit = 0; +let long_term_feerate = FeeRate::default_min_relay_fee(); let mut coin_selector = CoinSelector::new(&candidates, base_weight); @@ -156,9 +148,10 @@ let target = Target { // 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`. -let change_policy = min_value_and_waste( +let change_policy = ChangePolicy::min_value_and_waste( drain_weights, dust_limit, + target.feerate, long_term_feerate, ); @@ -168,7 +161,7 @@ let change_policy = min_value_and_waste( let metric = LowestFee { target, long_term_feerate, - change_policy: &change_policy, + change_policy }; // We run the branch and bound algorithm with a max round limit of 100,000. @@ -180,10 +173,10 @@ match coin_selector.run_bnb(metric, 100_000) { let selection = coin_selector .apply_selection(&candidates) .collect::>(); - let change = change_policy(&coin_selector, target); + let change = coin_selector.drain(target, change_policy); println!("we selected {} inputs", selection.len()); - println!("are we including the change output? {}", change.is_some()); + println!("We are including a change output of {} value (0 means not change)", change.value); } }; ``` @@ -199,26 +192,34 @@ match coin_selector.run_bnb(metric, 100_000) { [`Target`]: crate::Target ```rust -use bdk_coin_select::{ CoinSelector, Candidate, DrainWeights, Target }; -use bdk_coin_select::change_policy::min_value; -use bitcoin::{ Amount, ScriptBuf, TxOut }; -# let base_weight = 0_u32; -# let drain_weights = DrainWeights::new_tr_keyspend(); +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::new_tr_keyspend(); +use core::str::FromStr; // A random target, as an example. let target = Target { value: 21_000, ..Default::default() }; -// A random drain policy, as an example. -let drain_policy = min_value(drain_weights, 0); +// 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: ScriptBuf::new(), + 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 @@ -228,19 +229,19 @@ let candidates = candidate_txouts .map(|txout| Candidate { input_count: 1, value: txout.value, - weight: txout.weight() as u32, + 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); let _result = selector - .select_until_target_met(target, drain_policy(&selector, target)); + .select_until_target_met(target, Drain::none()); // Determine what the drain output will be, based on our selection. -let drain = drain_policy(&selector, target); +let drain = selector.drain(target, change_policy); -// Check that selection is finished! +// In theory the target must always still be met at this point assert!(selector.is_target_met(target, drain)); // Get a list of coins that are selected. @@ -252,7 +253,7 @@ assert_eq!(selected_coins.len(), 1); # Minimum Supported Rust Version (MSRV) -This library should compile with Rust 1.54.0. +This library is tested to compile on 1.54 To build with the MSRV, you will need to pin the following dependencies: diff --git a/src/change_policy.rs b/src/change_policy.rs index 17e8591..026c1b3 100644 --- a/src/change_policy.rs +++ b/src/change_policy.rs @@ -6,84 +6,45 @@ #[allow(unused)] // some bug in <= 1.48.0 sees this as unused when it isn't use crate::float::FloatExt; -use crate::{CoinSelector, Drain, DrainWeights, FeeRate, Target}; -use core::convert::TryInto; - -/// 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, -) -> impl Fn(&CoinSelector, Target) -> Drain { - let min_value: i64 = min_value - .try_into() - .expect("min_value is ridiculously large"); - - move |cs, target| { - let mut drain = Drain { - weights: drain_weights, - ..Default::default() - }; +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, +} - let excess = cs.excess(target, drain); - if excess < min_value { - return Drain::none(); +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, } - - drain.value = excess - .try_into() - .expect("must be positive since it is greater than min_value (which is positive)"); - drain } -} -/// Construct a change policy that creates change when it would reduce the transaction waste. -/// -/// **WARNING:** This may result in a change value that is below dust limit. [`min_value_and_waste`] -/// is a more sensible default. -pub fn min_waste( - drain_weights: DrainWeights, - long_term_feerate: FeeRate, -) -> impl Fn(&CoinSelector, Target) -> Drain { - move |cs, target| { + /// 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_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 - } -} - -/// Construct a change policy that creates change when it would reduce the transaction waste given -/// that `min_value` is respected. -/// -/// This is equivalent to combining [`min_value`] with [`min_waste`], and including change when both -/// policies have change. -pub fn min_value_and_waste( - drain_weights: DrainWeights, - min_value: u64, - long_term_feerate: FeeRate, -) -> impl Fn(&CoinSelector, Target) -> Drain { - let min_waste_policy = crate::change_policy::min_waste(drain_weights, long_term_feerate); + .waste(target_feerate, long_term_feerate) + .ceil() as u64; - move |cs, target| { - let drain = min_waste_policy(cs, target); - if drain.value < min_value { - return Drain::none(); + Self { + drain_weights, + min_value: waste_with_change.max(min_value), } - drain } } diff --git a/src/coin_selector.rs b/src/coin_selector.rs index bf038d1..0d42a56 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, float::Ordf32, FeeRate}; +use crate::{bnb::BnbMetric, change_policy::ChangePolicy, float::Ordf32, FeeRate}; use alloc::{borrow::Cow, collections::BTreeSet, vec::Vec}; /// [`CoinSelector`] selects/deselects coins from a set of canididate coins. @@ -410,6 +410,16 @@ impl<'a> CoinSelector<'a> { self.excess(target, drain) >= 0 } + /// Whether the constrains of `Target` have been met if we include the drain (change) output + /// when `change_policy` decides it should be present. + pub fn is_target_met_with_change_policy( + &self, + target: Target, + change_policy: ChangePolicy, + ) -> bool { + self.is_target_met(target, self.drain(target, change_policy)) + } + /// Select all unselected candidates pub fn select_all(&mut self) { loop { @@ -419,6 +429,40 @@ impl<'a> CoinSelector<'a> { } } + /// The value of the change output should have to drain the excess value while maintaining the + /// constraints of `target` and respecting `change_policy`. + /// + /// If not change output should be added according to policy then it will return `None`. + pub fn drain_value(&self, target: Target, change_policy: ChangePolicy) -> Option { + let excess = self.excess( + target, + Drain { + weights: change_policy.drain_weights, + value: 0, + }, + ); + if excess > change_policy.min_value as i64 { + Some(excess as u64) + } else { + None + } + } + + /// Convienince method that calls [`drain_value`] and converts the result into `Drain` by using + /// the provided `DrainWeights`. Note carefully that the `change_policy` should have been + /// calculated with the same `DrainWeights`. + /// + /// [`drain_value`]: Self::drain_value + pub fn drain(&self, target: Target, change_policy: ChangePolicy) -> Drain { + match self.drain_value(target, change_policy) { + Some(value) => Drain { + weights: change_policy.drain_weights, + value, + }, + None => Drain::none(), + } + } + /// Select all candidates with an *effective value* greater than 0 at the provided `feerate`. /// /// A candidate if effective if it provides more value than it takes to pay for at `feerate`. @@ -581,9 +625,9 @@ impl Candidate { } } -/// A structure that represents the weight costs of a drain (a.k.a. change) output. +/// Represents the weight costs of a drain (a.k.a. change) output. /// -/// This structure can also represent multiple outputs. +/// May also represent multiple outputs. #[derive(Default, Debug, Clone, Copy, Hash, PartialEq, Eq)] pub struct DrainWeights { /// The weight of including this drain output. @@ -615,9 +659,8 @@ impl DrainWeights { /// A drain (A.K.A. change) output. /// Technically it could represent multiple outputs. /// -/// These are usually created by a [`change_policy`]. -/// -/// [`change_policy`]: crate::change_policy +/// This is returned from [`CoinSelector::drain`]. Note if `drain` returns a drain where `is_none()` +/// returns true then **no change should be added** to the transaction. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Default)] pub struct Drain { /// Weight of adding drain output and spending the drain output. diff --git a/src/lib.rs b/src/lib.rs index cd7dcef..68dca23 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,7 +22,8 @@ pub mod metrics; mod feerate; pub use feerate::*; -pub mod change_policy; +mod change_policy; +pub use change_policy::*; /// Txin "base" fields include `outpoint` (32+4) and `nSequence` (4) and 1 byte for the scriptSig /// length. @@ -43,6 +44,9 @@ pub const TR_KEYSPEND_SATISFACTION_WEIGHT: u32 = 66; /// The additional weight of an output with segwit `v1` (taproot) script pubkey over a blank output (i.e. with weight [`TXOUT_BASE_WEIGHT`]). 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; + /// Helper to calculate varint size. `v` is the value the varint represents. fn varint_size(v: usize) -> u32 { if v <= 0xfc { diff --git a/src/metrics.rs b/src/metrics.rs index e1dcabd..35cd757 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -1,6 +1,8 @@ //! Branch and bound metrics that can be passed to [`CoinSelector::bnb_solutions`] or //! [`CoinSelector::run_bnb`]. -use crate::{bnb::BnbMetric, float::Ordf32, CoinSelector, Drain, Target}; +use crate::{ + bnb::BnbMetric, change_policy::ChangePolicy, float::Ordf32, CoinSelector, Drain, Target, +}; mod waste; pub use waste::*; mod lowest_fee; @@ -16,12 +18,8 @@ pub use changeless::*; // // NOTE: this should stay private because it requires cs to be sorted such that all negative // effective value candidates are next to each other. -fn change_lower_bound<'a>( - cs: &CoinSelector<'a>, - target: Target, - change_policy: &impl Fn(&CoinSelector<'a>, Target) -> Drain, -) -> Drain { - let has_change_now = change_policy(cs, target).is_some(); +fn change_lower_bound(cs: &CoinSelector, target: Target, change_policy: ChangePolicy) -> Drain { + let has_change_now = cs.drain_value(target, change_policy).is_some(); if has_change_now { let mut least_excess = cs.clone(); @@ -32,7 +30,7 @@ fn change_lower_bound<'a>( least_excess.select(index); }); - change_policy(&least_excess, target) + least_excess.drain(target, change_policy) } else { Drain::none() } diff --git a/src/metrics/changeless.rs b/src/metrics/changeless.rs index c2cec09..133859e 100644 --- a/src/metrics/changeless.rs +++ b/src/metrics/changeless.rs @@ -1,35 +1,29 @@ use super::change_lower_bound; -use crate::{bnb::BnbMetric, float::Ordf32, CoinSelector, Drain, Target}; +use crate::{ + bnb::BnbMetric, change_policy::ChangePolicy, float::Ordf32, CoinSelector, Drain, Target, +}; /// Metric for finding changeless solutions only. -pub struct Changeless<'c, C> { +pub struct Changeless { /// The target parameters for the resultant selection. pub target: Target, /// Policy to determine whether a selection requires a change output. - pub change_policy: &'c C, + pub change_policy: ChangePolicy, } -impl<'c, C> BnbMetric for Changeless<'c, C> -where - for<'a, 'b> C: Fn(&'b CoinSelector<'a>, Target) -> Drain, -{ +impl BnbMetric for Changeless { fn score(&mut self, cs: &CoinSelector<'_>) -> Option { - let drain = (self.change_policy)(cs, self.target); - if cs.is_target_met(self.target, drain) && (*self.change_policy)(cs, self.target).is_none() + if cs.is_target_met(self.target, Drain::none()) + && cs.drain_value(self.target, self.change_policy).is_none() { Some(Ordf32(0.0)) } else { None } - // if !cs.is_target_met(self.target, drain) { - // None - // } else { - // Some(Ordf32(0.0)) - // } } fn bound(&mut self, cs: &CoinSelector<'_>) -> Option { - if change_lower_bound(cs, self.target, &self.change_policy).is_some() { + if change_lower_bound(cs, self.target, self.change_policy).is_some() { None } else { Some(Ordf32(0.0)) diff --git a/src/metrics/lowest_fee.rs b/src/metrics/lowest_fee.rs index ca2d77a..c9027a1 100644 --- a/src/metrics/lowest_fee.rs +++ b/src/metrics/lowest_fee.rs @@ -1,6 +1,6 @@ use crate::{ - float::Ordf32, metrics::change_lower_bound, BnbMetric, Candidate, CoinSelector, Drain, - DrainWeights, FeeRate, Target, + change_policy::ChangePolicy, float::Ordf32, metrics::change_lower_bound, BnbMetric, Candidate, + CoinSelector, Drain, DrainWeights, FeeRate, Target, }; /// Metric that aims to minimize transaction fees. The future fee for spending the change output is @@ -11,27 +11,17 @@ use crate::{ /// /// The scoring function for solutions with change: /// > (input_weight + change_output_weight) * feerate + change_spend_weight * long_term_feerate -pub struct LowestFee<'c, C> { +#[derive(Clone, Copy)] +pub struct LowestFee { /// The target parameters for the resultant selection. pub target: Target, /// The estimated feerate needed to spend our change output later. pub long_term_feerate: FeeRate, /// Policy to determine the change output (if any) of a given selection. - pub change_policy: &'c C, + pub change_policy: ChangePolicy, } -impl<'c, C> Clone for LowestFee<'c, C> { - fn clone(&self) -> Self { - *self - } -} - -impl<'c, C> Copy for LowestFee<'c, C> {} - -impl<'c, C> LowestFee<'c, C> -where - for<'a, 'b> C: Fn(&'b CoinSelector<'a>, Target) -> Drain, -{ +impl LowestFee { fn calc_metric(&self, cs: &CoinSelector<'_>, drain_weights: Option) -> f32 { self.calc_metric_lb(cs, drain_weights) + match drain_weights { @@ -58,12 +48,9 @@ where } } -impl<'c, C> BnbMetric for LowestFee<'c, C> -where - for<'a, 'b> C: Fn(&'b CoinSelector<'a>, Target) -> Drain, -{ +impl BnbMetric for LowestFee { fn score(&mut self, cs: &CoinSelector<'_>) -> Option { - let drain = (self.change_policy)(cs, self.target); + let drain = cs.drain(self.target, self.change_policy); if !cs.is_target_met(self.target, drain) { return None; } @@ -81,7 +68,7 @@ where // this either returns: // * None: change output may or may not exist // * Some: change output must exist from this branch onwards - let change_lb = change_lower_bound(cs, self.target, &self.change_policy); + let change_lb = change_lower_bound(cs, self.target, self.change_policy); let change_lb_weights = if change_lb.is_some() { Some(change_lb.weights) } else { diff --git a/src/metrics/waste.rs b/src/metrics/waste.rs index 088ee22..a8f5312 100644 --- a/src/metrics/waste.rs +++ b/src/metrics/waste.rs @@ -1,5 +1,7 @@ use super::change_lower_bound; -use crate::{bnb::BnbMetric, float::Ordf32, Candidate, CoinSelector, Drain, FeeRate, Target}; +use crate::{ + bnb::BnbMetric, float::Ordf32, Candidate, ChangePolicy, CoinSelector, Drain, FeeRate, Target, +}; /// The "waste" metric used by bitcoin core. /// @@ -18,29 +20,19 @@ use crate::{bnb::BnbMetric, float::Ordf32, Candidate, CoinSelector, Drain, FeeRa /// 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! -pub struct Waste<'c, C> { +#[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: &'c C, + pub change_policy: ChangePolicy, } -impl<'c, C> Clone for Waste<'c, C> { - fn clone(&self) -> Self { - *self - } -} - -impl<'c, C> Copy for Waste<'c, C> {} - -impl<'c, C> BnbMetric for Waste<'c, C> -where - for<'a, 'b> C: Fn(&'b CoinSelector<'a>, Target) -> Drain, -{ +impl BnbMetric for Waste { fn score(&mut self, cs: &CoinSelector<'_>) -> Option { - let drain = (self.change_policy)(cs, self.target); + let drain = cs.drain(self.target, self.change_policy); if !cs.is_target_met(self.target, drain) { return None; } @@ -59,14 +51,14 @@ where // duper correct. In testing it seems to come up with pretty good results pretty fast. let rate_diff = self.target.feerate.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); + 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(self.target, change_lower_bound) { - let current_change = (self.change_policy)(cs, self.target); + 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( @@ -188,7 +180,7 @@ where if !cs.is_target_met(self.target, Drain::none()) { return None; } - let change_at_value_optimum = (self.change_policy)(&cs, self.target); + 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. @@ -210,7 +202,7 @@ where .rev() .take_while(|(cs, _, wv)| { wv.effective_value(self.target.feerate).0 < 0.0 - || (self.change_policy)(cs, self.target).is_none() + || cs.drain_value(self.target, self.change_policy).is_none() }) .last(); diff --git a/tests/bnb.rs b/tests/bnb.rs index c9df3f5..686e107 100644 --- a/tests/bnb.rs +++ b/tests/bnb.rs @@ -8,11 +8,11 @@ use proptest::{prelude::*, proptest, test_runner::*}; fn test_wv(mut rng: impl RngCore) -> impl Iterator { core::iter::repeat_with(move || { - let value = rng.gen_range(0, 1_000); + let value = rng.gen_range(0..1_000); let mut candidate = Candidate { value, weight: 100, - input_count: rng.gen_range(1, 2), + input_count: rng.gen_range(1..2), is_segwit: rng.gen_bool(0.5), }; // HACK: set is_segwit = true for all these tests because you can't actually lower bound diff --git a/tests/changeless.rs b/tests/changeless.rs index 5607fde..06e085e 100644 --- a/tests/changeless.rs +++ b/tests/changeless.rs @@ -1,18 +1,19 @@ #![allow(unused)] mod common; use bdk_coin_select::{ - float::Ordf32, metrics, Candidate, CoinSelector, Drain, DrainWeights, FeeRate, Target, + float::Ordf32, metrics, Candidate, ChangePolicy, CoinSelector, Drain, DrainWeights, FeeRate, + Target, }; use proptest::{prelude::*, proptest, test_runner::*}; use rand::{prelude::IteratorRandom, Rng, RngCore}; fn test_wv(mut rng: impl RngCore) -> impl Iterator { core::iter::repeat_with(move || { - let value = rng.gen_range(0, 1_000); + let value = rng.gen_range(0..1_000); Candidate { value, - weight: rng.gen_range(0, 100), - input_count: rng.gen_range(1, 2), + weight: rng.gen_range(0..100), + input_count: rng.gen_range(1..2), is_segwit: rng.gen_bool(0.5), } }) @@ -46,7 +47,7 @@ proptest! { spend_weight: change_spend_weight, }; - let change_policy = bdk_coin_select::change_policy::min_waste(drain, long_term_feerate); + let change_policy = ChangePolicy::min_value_and_waste(drain, 0, feerate, long_term_feerate); let wv = test_wv(&mut rng); let candidates = wv.take(num_inputs).collect::>(); println!("candidates: {:#?}", candidates); @@ -61,7 +62,7 @@ proptest! { let solutions = cs.bnb_solutions(metrics::Changeless { target, - change_policy: &change_policy + change_policy }); @@ -83,16 +84,16 @@ proptest! { }, ]; - cmp_benchmarks.extend((0..10).map(|_|random_minimal_selection(&cs, target, long_term_feerate, &change_policy, &mut rng))); + 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, change_policy(&cs, target))); + let cmp_benchmarks = cmp_benchmarks.into_iter().filter(|cs| cs.is_target_met_with_change_policy(target, change_policy)); for (_bench_id, bench) in cmp_benchmarks.enumerate() { - prop_assert!(change_policy(&bench, target).is_some() >= change_policy(&sol, target).is_some()); + prop_assert!(bench.drain_value(target, change_policy).is_some() >= sol.drain_value(target, change_policy).is_some()); } } None => { let mut cs = cs.clone(); - let mut metric = metrics::Changeless { target, change_policy: &change_policy }; + let mut metric = metrics::Changeless { target, change_policy }; let has_solution = common::exhaustive_search(&mut cs, &mut metric).is_some(); assert!(!has_solution); } @@ -107,14 +108,14 @@ fn random_minimal_selection<'a>( cs: &CoinSelector<'a>, target: Target, long_term_feerate: FeeRate, - change_policy: &impl Fn(&CoinSelector, Target) -> Drain, + 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, change_policy(&cs, target)) { + if cs.is_target_met_with_change_policy(target, change_policy) { break; } } diff --git a/tests/common.rs b/tests/common.rs index 91d0cd0..2b72c07 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -3,8 +3,8 @@ use std::any::type_name; use bdk_coin_select::{ - float::Ordf32, BnbMetric, Candidate, CoinSelector, Drain, DrainWeights, FeeRate, NoBnbSolution, - Target, + float::Ordf32, BnbMetric, Candidate, ChangePolicy, CoinSelector, Drain, DrainWeights, FeeRate, + NoBnbSolution, Target, }; use proptest::{ prelude::*, @@ -17,15 +17,14 @@ use proptest::{ /// /// We don't restrict bnb rounds, so we expect that the bnb result to be equal to the exhaustive /// search result. -pub fn can_eventually_find_best_solution( +pub fn can_eventually_find_best_solution( params: StrategyParams, candidates: Vec, - change_policy: &P, + change_policy: ChangePolicy, mut metric: M, ) -> Result<(), proptest::test_runner::TestCaseError> where M: BnbMetric, - P: Fn(&CoinSelector, Target) -> Drain, { println!("== TEST =="); println!("{}", type_name::()); @@ -44,7 +43,7 @@ where println!("\texhaustive search:"); let now = std::time::Instant::now(); let exp_result = exhaustive_search(&mut exp_selection, &mut metric); - let exp_change = change_policy(&exp_selection, target); + let exp_change = exp_selection.drain(target, change_policy); let exp_result_str = result_string(&exp_result.ok_or("no possible solution"), exp_change); println!( "\t\telapsed={:8}s result={}", @@ -54,7 +53,7 @@ where // bonus check: ensure min_fee is respected if exp_result.is_some() { let selected_value = exp_selection.selected_value(); - let drain_value = change_policy(&exp_selection, target).value; + let drain_value = exp_selection.drain(target, change_policy).value; let target_value = target.value; assert!(selected_value - target_value - drain_value >= params.min_fee); } @@ -62,7 +61,7 @@ where println!("\tbranch and bound:"); let now = std::time::Instant::now(); let result = bnb_search(&mut selection, metric, usize::MAX); - let change = change_policy(&selection, target); + let change = selection.drain(target, change_policy); let result_str = result_string(&result, change); println!( "\t\telapsed={:8}s result={}", @@ -86,7 +85,7 @@ where // bonus check: ensure min_fee is respected let selected_value = selection.selected_value(); - let drain_value = change_policy(&selection, target).value; + let drain_value = selection.drain(target, change_policy).value; let target_value = target.value; assert!(selected_value - target_value - drain_value >= params.min_fee); } @@ -100,15 +99,14 @@ where /// scores of all descendant branches. /// /// If this fails, it means the metric's bound function is too tight. -pub fn ensure_bound_is_not_too_tight( +pub fn ensure_bound_is_not_too_tight( params: StrategyParams, candidates: Vec, - change_policy: &P, + change_policy: ChangePolicy, mut metric: M, ) -> Result<(), proptest::test_runner::TestCaseError> where M: BnbMetric, - P: Fn(&CoinSelector, Target) -> Drain, { println!("== TEST =="); println!("{}", type_name::()); @@ -136,7 +134,7 @@ where "checking branch: selection={} score={} change={} lb={}", cs, score, - change_policy(&cs, target).is_some(), + cs.drain_value(target, change_policy).is_some(), lb_score ); } @@ -154,11 +152,11 @@ where descendant={:8} change={} score={} ", cs, - change_policy(&cs, target).is_some(), + cs.drain_value(target, change_policy).is_some(), lb_score, cs.is_target_met(target, Drain::none()), descendant_cs, - change_policy(&descendant_cs, target).is_some(), + descendant_cs.drain_value(target, change_policy).is_some(), descendant_score, ); } @@ -209,9 +207,9 @@ impl StrategyParams { pub fn gen_candidates(n: usize) -> Vec { let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha); core::iter::repeat_with(move || { - let value = rng.gen_range(1, 500_001); - let weight = rng.gen_range(1, 2001); - let input_count = rng.gen_range(1, 3); + let value = rng.gen_range(1..500_001); + let weight = rng.gen_range(1..2001); + let input_count = rng.gen_range(1..3); let is_segwit = rng.gen_bool(0.01); Candidate { diff --git a/tests/lowest_fee.rs b/tests/lowest_fee.rs index 511dc13..58d000c 100644 --- a/tests/lowest_fee.rs +++ b/tests/lowest_fee.rs @@ -1,8 +1,8 @@ #![allow(unused_imports)] mod common; -use bdk_coin_select::change_policy::{self, min_value_and_waste}; use bdk_coin_select::metrics::{Changeless, LowestFee}; +use bdk_coin_select::ChangePolicy; use bdk_coin_select::{BnbMetric, Candidate, CoinSelector}; use proptest::prelude::*; @@ -26,9 +26,9 @@ proptest! { ) { 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 = LowestFee { 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)?; + let change_policy = ChangePolicy::min_value_and_waste(params.drain_weights(), params.drain_dust, params.feerate(), params.long_term_feerate()); + let metric = LowestFee { target: params.target(), long_term_feerate: params.long_term_feerate(), change_policy }; + common::can_eventually_find_best_solution(params, candidates, change_policy, metric)?; } #[test] @@ -46,9 +46,9 @@ proptest! { ) { 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 = LowestFee { 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)?; + let change_policy = ChangePolicy::min_value_and_waste(params.drain_weights(), params.drain_dust, params.feerate(), params.long_term_feerate()); + let metric = LowestFee { target: params.target(), long_term_feerate: params.long_term_feerate(), change_policy }; + common::ensure_bound_is_not_too_tight(params, candidates, change_policy, metric)?; } #[test] @@ -90,16 +90,17 @@ proptest! { let mut cs = CoinSelector::new(&candidates, params.base_weight); - let change_policy = min_value_and_waste( + let change_policy = ChangePolicy::min_value_and_waste( params.drain_weights(), params.drain_dust, + params.feerate(), params.long_term_feerate(), ); let metric = LowestFee { target: params.target(), long_term_feerate: params.long_term_feerate(), - change_policy: &change_policy, + change_policy, }; let (score, rounds) = common::bnb_search(&mut cs, metric, params.n_candidates)?; @@ -127,21 +128,22 @@ fn combined_changeless_metric() { let mut cs_a = CoinSelector::new(&candidates, params.base_weight); let mut cs_b = CoinSelector::new(&candidates, params.base_weight); - let change_policy = min_value_and_waste( + let change_policy = ChangePolicy::min_value_and_waste( params.drain_weights(), params.drain_dust, + params.feerate(), params.long_term_feerate(), ); let metric_lowest_fee = LowestFee { target: params.target(), long_term_feerate: params.long_term_feerate(), - change_policy: &change_policy, + change_policy, }; let metric_changeless = Changeless { target: params.target(), - change_policy: &change_policy, + change_policy, }; let metric_combined = ((metric_lowest_fee, 1.0_f32), (metric_changeless, 0.0_f32)); diff --git a/tests/waste.rs b/tests/waste.rs index 6e1dca0..f67145a 100644 --- a/tests/waste.rs +++ b/tests/waste.rs @@ -2,9 +2,8 @@ mod common; -use bdk_coin_select::change_policy::min_value_and_waste; use bdk_coin_select::{ - change_policy, float::Ordf32, metrics::Waste, Candidate, CoinSelector, Drain, DrainWeights, + float::Ordf32, metrics::Waste, Candidate, ChangePolicy, CoinSelector, Drain, DrainWeights, FeeRate, Target, }; use proptest::{ @@ -32,7 +31,8 @@ fn waste_all_selected_except_one_is_optimal_and_awkward() { spend_weight: change_spend_weight, }; - let change_policy = change_policy::min_waste(drain_weights, long_term_feerate); + 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::>(); @@ -46,7 +46,7 @@ fn waste_all_selected_except_one_is_optimal_and_awkward() { let solutions = cs.bnb_solutions(Waste { target, long_term_feerate, - change_policy: &change_policy, + change_policy, }); let (_i, (best, score)) = solutions @@ -60,7 +60,7 @@ fn waste_all_selected_except_one_is_optimal_and_awkward() { let target_waste = all_selected.waste( target, long_term_feerate, - change_policy(&all_selected, target), + cs.drain(target, change_policy), 1.0, ); assert!(score.0 < target_waste); @@ -90,7 +90,8 @@ fn waste_naive_effective_value_shouldnt_be_better() { value: 0, }; - let change_policy = change_policy::min_waste(drain_weights, long_term_feerate); + 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::>(); @@ -105,7 +106,7 @@ fn waste_naive_effective_value_shouldnt_be_better() { let solutions = cs.bnb_solutions(Waste { target, long_term_feerate, - change_policy: &change_policy, + change_policy, }); let (_i, (_best, score)) = solutions @@ -122,7 +123,7 @@ fn waste_naive_effective_value_shouldnt_be_better() { let bench_waste = naive_select.waste( target, long_term_feerate, - change_policy(&naive_select, target), + naive_select.drain(target, change_policy), 1.0, ); @@ -150,7 +151,8 @@ fn waste_doesnt_take_too_long_to_finish() { spend_weight: change_spend_weight, }; - let change_policy = change_policy::min_waste(drain_weights, long_term_feerate); + 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::>(); @@ -165,7 +167,7 @@ fn waste_doesnt_take_too_long_to_finish() { let solutions = cs.bnb_solutions(Waste { target, long_term_feerate, - change_policy: &change_policy, + change_policy, }); solutions @@ -202,7 +204,8 @@ fn waste_lower_long_term_feerate_but_still_need_to_select_all() { spend_weight: change_spend_weight, }; - let change_policy = change_policy::min_waste(drain_weights, long_term_feerate); + 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::>(); @@ -217,7 +220,7 @@ fn waste_lower_long_term_feerate_but_still_need_to_select_all() { let solutions = cs.bnb_solutions(Waste { target, long_term_feerate, - change_policy: &change_policy, + change_policy, }); let bench = { let mut all_selected = cs.clone(); @@ -234,7 +237,7 @@ fn waste_lower_long_term_feerate_but_still_need_to_select_all() { let bench_waste = bench.waste( target, long_term_feerate, - change_policy(&bench, target), + bench.drain(target, change_policy), 1.0, ); @@ -260,7 +263,8 @@ fn waste_low_but_non_negative_rate_diff_means_adding_more_inputs_might_reduce_ex spend_weight: change_spend_weight, }; - let change_policy = change_policy::min_waste(drain_weights, long_term_feerate); + 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 @@ -280,7 +284,7 @@ fn waste_low_but_non_negative_rate_diff_means_adding_more_inputs_might_reduce_ex let solutions = cs.bnb_solutions(Waste { target, long_term_feerate, - change_policy: &change_policy, + change_policy, }); let bench = { let mut all_selected = cs.clone(); @@ -297,7 +301,7 @@ fn waste_low_but_non_negative_rate_diff_means_adding_more_inputs_might_reduce_ex let bench_waste = bench.waste( target, long_term_feerate, - change_policy(&bench, target), + bench.drain(target, change_policy), 1.0, ); @@ -456,11 +460,11 @@ proptest! { fn test_wv(mut rng: impl RngCore) -> impl Iterator { core::iter::repeat_with(move || { - let value = rng.gen_range(0, 1_000); + let value = rng.gen_range(0..1_000); Candidate { value, - weight: rng.gen_range(0, 100), - input_count: rng.gen_range(1, 2), + weight: rng.gen_range(0..100), + input_count: rng.gen_range(1..2), is_segwit: rng.gen_bool(0.5), } })