Give CoinSelector its target instead of threading it through every call

CHANGELOG.md

@@ -1,6 +1,6 @@
 # Unreleased
 
-- **Breaking:** `BnbMetric`'s `score`, `bound`, and `drain` take the `target: Target` as a parameter, and `CoinSelector::run_bnb`/`bnb_solutions` gain a leading `target` argument. Consequently `LowestFee` and `Changeless` no longer store a `target` field. This removes the target that `Changeless<M>` previously had to keep in sync with its inner metric, and aligns the metric API with the rest of `CoinSelector`, where `target` is always passed in.
+- **Breaking:** `CoinSelector` now owns its `Target`. `CoinSelector::new(candidates, target)` takes it and `CoinSelector::target()` returns it, and it is fixed for the life of the selector. Every method that took a `target: Target` argument no longer does, including `excess`, `missing`, `rate_excess`, `implied_fee`, `is_funded`, `is_within_max_weight`, `drain`, `select_until_target_met`, `select_srd`, `run_bnb`, and `bnb_solutions`. The same goes for arguments that merely restated part of the target: `weight` and `implied_feerate` no longer take `TargetOutputs`, `fee` no longer takes `target_value`, and `effective_value` and `select_all_effective` no longer take a `FeeRate`. `BnbMetric`'s methods read the target from the `CoinSelector` they are given — they are now `fn score(&mut self, cs: &CoinSelector<'_>) -> Option<Ordf32>`, `fn bound(&mut self, cs: &CoinSelector<'_>) -> Option<Ordf32>` and `fn drain(&mut self, cs: &CoinSelector<'_>) -> Drain` — so `LowestFee` and `Changeless` no longer store a `target` field. This removes the target that `Changeless<M>` previously had to keep in sync with its inner metric. To measure a selection against a second target, use `CoinSelector::with_target(target)`, which copies the selection, the bans and the candidate order over to the new target; `CoinSelector::new` starts from an empty selection.
 - **Breaking:** `BnbMetric` metrics now decide the change output themselves. The trait gains a `drain(&mut self, cs) -> Drain` method; call it on a branch-and-bound solution (or the `LowestFee` metric directly) to get the change output the metric optimized against, instead of computing a separate `ChangePolicy`.
 - **Breaking:** `CoinSelector::run_bnb` now returns `(Ordf32, Drain)` instead of just `Ordf32`, handing back the change output the metric decided on for the winning selection.
 - **Breaking:** `LowestFee` no longer takes a `change_policy`. It now takes `dust_relay_feerate: FeeRate` and `drain_weights: DrainWeights`, and adds change only when doing so lowers the long-term fee and the change would not be dust.

README.md

@@ -54,13 +54,13 @@ let candidates = vec![
 ];
 
 // You can now select coins!
-let mut coin_selector = CoinSelector::new(&candidates);
+let mut coin_selector = CoinSelector::new(&candidates, target);
 coin_selector.select(0);
 
-assert!(!coin_selector.is_funded(target), "we didn't select enough");
-println!("we didn't select enough yet we're missing: {}", coin_selector.missing(target));
+assert!(!coin_selector.is_funded(), "we didn't select enough");
+println!("we didn't select enough yet we're missing: {}", coin_selector.missing());
 coin_selector.select(1);
-assert!(coin_selector.is_funded(target), "we should have enough now");
+assert!(coin_selector.is_funded(), "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
 //
@@ -69,7 +69,7 @@ assert!(coin_selector.is_funded(target), "we should have enough now");
 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);
+let change = coin_selector.drain(change_policy);
 if change.is_some() {
     println!("We need to add our change output to the transaction with {} value", change.value);
 } else {
@@ -127,14 +127,14 @@ let drain_weights = bdk_coin_select::DrainWeights::default();
 // 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);
-
 let target = Target {
     fee: TargetFee::from_feerate(FeeRate::from_sat_per_vb(15.0)),
     outputs: TargetOutputs::fund_outputs(outputs.iter().map(|output| (output.weight().to_wu(), output.value.to_sat()))),
     max_weight: None,
 };
 
+let mut coin_selector = CoinSelector::new(&candidates, target);
+
 // The feerate used to work out whether a change output would be dust (and so shouldn't be added).
 // The standard dust relay feerate is 3 sat/vb.
 let dust_relay_feerate = FeeRate::from_sat_per_vb(3.0);
@@ -150,13 +150,13 @@ let mut metric = LowestFee {
 
 // We run the branch and bound algorithm with a max round limit of 100,000.
 // On success it returns the score along with the change output the metric decided on.
-let change = match coin_selector.run_bnb(target, metric, 100_000) {
+let change = match coin_selector.run_bnb(metric, 100_000) {
     Err(err) => {
         println!("failed to find a solution: {}", err);
         // fall back to naive selection
-        coin_selector.select_until_target_met(target).expect("a selection was impossible!");
+        coin_selector.select_until_target_met().expect("a selection was impossible!");
         // the metric still decides the change output for whatever we end up selecting
-        metric.drain(&coin_selector, target)
+        metric.drain(&coin_selector)
     }
     Ok((score, change)) => {
         println!("we found a solution with score {}", score);

benches/coin_selector.rs

@@ -56,7 +56,8 @@ fn bench_coin_selector_clone(c: &mut Criterion) {
     let mut group = c.benchmark_group("clone");
     for &n in &[64usize, 256, 1024, 4096] {
         let candidates = make_candidates(n);
-        let mut selector = CoinSelector::new(&candidates);
+        let (target, _) = make_bnb_inputs(&candidates);
+        let mut selector = CoinSelector::new(&candidates, target);
         // Select ~10% of candidates so `selected` is non-trivial to copy.
         for i in (0..n).step_by(10) {
             selector.select(i);
@@ -74,8 +75,8 @@ fn bench_run_bnb_lowest_fee(c: &mut Criterion) {
     group.sample_size(20);
     for &n in &[20usize, 50, 100, 200] {
         let candidates = make_candidates(n);
-        let selector = CoinSelector::new(&candidates);
         let (target, long_term_feerate) = make_bnb_inputs(&candidates);
+        let selector = CoinSelector::new(&candidates, target);
         group.bench_with_input(BenchmarkId::from_parameter(n), &n, |b, _| {
             b.iter_batched(
                 || selector.clone(),
@@ -85,7 +86,7 @@ fn bench_run_bnb_lowest_fee(c: &mut Criterion) {
                         dust_relay_feerate: FeeRate::from_sat_per_vb(1.0),
                         drain_weights: DrainWeights::TR_KEYSPEND,
                     };
-                    let _ = sel.run_bnb(target, metric, black_box(100_000));
+                    let _ = sel.run_bnb(metric, black_box(100_000));
                     sel
                 },
                 BatchSize::SmallInput,

src/bnb.rs

@@ -1,6 +1,6 @@
 use core::cmp::Reverse;
 
-use crate::{float::Ordf32, Drain, Target};
+use crate::{float::Ordf32, Drain};
 
 use super::CoinSelector;
 use alloc::collections::BinaryHeap;
@@ -11,8 +11,6 @@ use alloc::collections::BinaryHeap;
 pub(crate) struct BnbIter<'a, M: BnbMetric> {
     queue: BinaryHeap<Branch<'a>>,
     best: Option<Ordf32>,
-    /// The target the metric scores selections against.
-    pub(crate) target: Target,
     /// The `BnBMetric` that will score each selection
     pub(crate) metric: M,
 }
@@ -55,7 +53,7 @@ impl<'a, M: BnbMetric> Iterator for BnbIter<'a, M> {
 
         let mut return_val = None;
         if !branch.is_exclusion {
-            if let Some(score) = self.metric.score(&selector, self.target) {
+            if let Some(score) = self.metric.score(&selector) {
                 let better = match self.best {
                     Some(best_score) => score < best_score,
                     None => true,
@@ -73,11 +71,10 @@ impl<'a, M: BnbMetric> Iterator for BnbIter<'a, M> {
 }
 
 impl<'a, M: BnbMetric> BnbIter<'a, M> {
-    pub(crate) fn new(mut selector: CoinSelector<'a>, target: Target, metric: M) -> Self {
+    pub(crate) fn new(mut selector: CoinSelector<'a>, metric: M) -> Self {
         let mut iter = BnbIter {
             queue: BinaryHeap::default(),
             best: None,
-            target,
             metric,
         };
 
@@ -91,7 +88,7 @@ impl<'a, M: BnbMetric> BnbIter<'a, M> {
     }
 
     fn consider_adding_to_queue(&mut self, cs: &CoinSelector<'a>, is_exclusion: bool) {
-        let bound = self.metric.bound(cs, self.target);
+        let bound = self.metric.bound(cs);
         if let Some(bound) = bound {
             let is_good_enough = match self.best {
                 Some(best) => best > bound,
@@ -201,26 +198,36 @@ impl Eq for Branch<'_> {}
 /// A branch and bound metric where we minimize the [`Ordf32`] score.
 ///
 /// This is to be used as input for [`CoinSelector::run_bnb`] or [`CoinSelector::bnb_solutions`].
+///
+/// Every selection passed to these methods carries its own [`Target`](crate::Target), reachable via
+/// [`CoinSelector::target`]. [`bound`] is only a valid lower bound on the [`score`] of the
+/// descendants of `cs` because the search clones `cs` to build them, so every node of a single
+/// search is guaranteed to have the same target.
+///
+/// [`bound`]: BnbMetric::bound
+/// [`score`]: BnbMetric::score
 pub trait BnbMetric {
-    /// Get the score of a given selection for `target`.
+    /// Get the score of the selection `cs` against [`cs.target()`](CoinSelector::target).
     ///
     /// If this returns `None`, the selection is invalid.
-    fn score(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option<Ordf32>;
+    fn score(&mut self, cs: &CoinSelector<'_>) -> Option<Ordf32>;
 
-    /// Get the lower bound score using a heuristic for `target`.
+    /// Get the lower bound score, using a heuristic, against
+    /// [`cs.target()`](CoinSelector::target).
     ///
     /// This represents the best possible score of all descendant branches (according to the
     /// heuristic).
     ///
     /// If this returns `None`, the current branch and all descendant branches will not have valid
     /// solutions.
-    fn bound(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option<Ordf32>;
+    fn bound(&mut self, cs: &CoinSelector<'_>) -> Option<Ordf32>;
 
-    /// The change output (a.k.a. drain) this metric decides on for the given selection and `target`,
-    /// or [`Drain::NONE`] if it decides there should be no change.
+    /// The change output (a.k.a. drain) this metric decides on for the selection `cs` and
+    /// [`cs.target()`](CoinSelector::target), or [`Drain::NONE`] if it decides there should be no
+    /// change.
     ///
     /// Call this on a branch-and-bound solution to get the change output the metric optimized against.
-    fn drain(&mut self, cs: &CoinSelector<'_>, target: Target) -> Drain;
+    fn drain(&mut self, cs: &CoinSelector<'_>) -> Drain;
 
     /// Returns whether the metric requies we order candidates by descending value per weight unit.
     fn requires_ordering_by_descending_value_pwu(&self) -> bool {