diff --git a/CHANGELOG.md b/CHANGELOG.md index a208b9e..b6efaf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +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` 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:** `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. diff --git a/README.md b/README.md index 9c8b920..098a027 100644 --- a/README.md +++ b/README.md @@ -140,7 +140,6 @@ let dust_relay_feerate = FeeRate::from_sat_per_vb(3.0); // decides for itself whether to add a change output: change is added whenever doing so reduces the // long-term fee (factoring in the cost to spend the output later on) and the change wouldn't be dust. let mut metric = LowestFee { - target, long_term_feerate, // used to calculate the cost of spending the change output in the future dust_relay_feerate, drain_weights, @@ -148,13 +147,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(metric, 100_000) { +let change = match coin_selector.run_bnb(target, 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!"); // the metric still decides the change output for whatever we end up selecting - metric.drain(&coin_selector) + metric.drain(&coin_selector, target) } Ok((score, change)) => { println!("we found a solution with score {}", score); diff --git a/benches/coin_selector.rs b/benches/coin_selector.rs index 6b4015a..76a8da9 100644 --- a/benches/coin_selector.rs +++ b/benches/coin_selector.rs @@ -75,12 +75,11 @@ fn bench_run_bnb_lowest_fee(c: &mut Criterion) { || selector.clone(), |mut sel| { let metric = LowestFee { - target, long_term_feerate, dust_relay_feerate: FeeRate::from_sat_per_vb(1.0), drain_weights: DrainWeights::TR_KEYSPEND, }; - let _ = sel.run_bnb(metric, black_box(100_000)); + let _ = sel.run_bnb(target, metric, black_box(100_000)); sel }, BatchSize::SmallInput, diff --git a/src/bnb.rs b/src/bnb.rs index fe879ae..0498e5c 100644 --- a/src/bnb.rs +++ b/src/bnb.rs @@ -1,6 +1,6 @@ use core::cmp::Reverse; -use crate::{float::Ordf32, Drain}; +use crate::{float::Ordf32, Drain, Target}; use super::CoinSelector; use alloc::collections::BinaryHeap; @@ -11,6 +11,8 @@ use alloc::collections::BinaryHeap; pub(crate) struct BnbIter<'a, M: BnbMetric> { queue: BinaryHeap>, best: Option, + /// The target the metric scores selections against. + pub(crate) target: Target, /// The `BnBMetric` that will score each selection pub(crate) metric: M, } @@ -53,7 +55,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) { + if let Some(score) = self.metric.score(&selector, self.target) { let better = match self.best { Some(best_score) => score < best_score, None => true, @@ -71,10 +73,11 @@ impl<'a, M: BnbMetric> Iterator for BnbIter<'a, M> { } impl<'a, M: BnbMetric> BnbIter<'a, M> { - pub(crate) fn new(mut selector: CoinSelector<'a>, metric: M) -> Self { + pub(crate) fn new(mut selector: CoinSelector<'a>, target: Target, metric: M) -> Self { let mut iter = BnbIter { queue: BinaryHeap::default(), best: None, + target, metric, }; @@ -88,7 +91,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); + let bound = self.metric.bound(cs, self.target); if let Some(bound) = bound { let is_good_enough = match self.best { Some(best) => best > bound, @@ -199,25 +202,25 @@ impl Eq for Branch<'_> {} /// /// This is to be used as input for [`CoinSelector::run_bnb`] or [`CoinSelector::bnb_solutions`]. pub trait BnbMetric { - /// Get the score of a given selection. + /// Get the score of a given selection for `target`. /// /// If this returns `None`, the selection is invalid. - fn score(&mut self, cs: &CoinSelector<'_>) -> Option; + fn score(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option; - /// Get the lower bound score using a heuristic. + /// Get the lower bound score using a heuristic for `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<'_>) -> Option; + fn bound(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option; - /// The change output (a.k.a. drain) this metric decides on for the given selection, or - /// [`Drain::NONE`] if it decides there should be no change. + /// 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. /// /// Call this on a branch-and-bound solution to get the change output the metric optimized against. - fn drain(&mut self, cs: &CoinSelector<'_>) -> Drain; + fn drain(&mut self, cs: &CoinSelector<'_>, target: Target) -> Drain; /// Returns whether the metric requies we order candidates by descending value per weight unit. fn requires_ordering_by_descending_value_pwu(&self) -> bool { diff --git a/src/coin_selector.rs b/src/coin_selector.rs index 0cb8844..f1b439b 100644 --- a/src/coin_selector.rs +++ b/src/coin_selector.rs @@ -562,9 +562,10 @@ impl<'a> CoinSelector<'a> { /// Most of the time, you would want to use [`CoinSelector::run_bnb`] instead. pub fn bnb_solutions( &self, + target: Target, metric: M, ) -> impl Iterator, Ordf32)>> { - crate::bnb::BnbIter::new(self.clone(), metric) + crate::bnb::BnbIter::new(self.clone(), target, metric) } /// Run branch and bound to minimize the score of the provided [`BnbMetric`]. @@ -576,10 +577,11 @@ impl<'a> CoinSelector<'a> { /// Use [`CoinSelector::bnb_solutions`] to access the branch and bound iterator directly. pub fn run_bnb( &mut self, + target: Target, metric: M, max_rounds: usize, ) -> Result<(Ordf32, Drain), NoBnbSolution> { - let mut iter = crate::bnb::BnbIter::new(self.clone(), metric); + let mut iter = crate::bnb::BnbIter::new(self.clone(), target, metric); let mut rounds = 0_usize; let best = iter .by_ref() @@ -588,7 +590,7 @@ impl<'a> CoinSelector<'a> { .flatten() .last(); let (selector, score) = best.ok_or(NoBnbSolution { max_rounds, rounds })?; - let drain = iter.metric.drain(&selector); + let drain = iter.metric.drain(&selector, target); *self = selector; Ok((score, drain)) } diff --git a/src/metrics/changeless.rs b/src/metrics/changeless.rs index 7234863..a9c9e32 100644 --- a/src/metrics/changeless.rs +++ b/src/metrics/changeless.rs @@ -5,17 +5,11 @@ use crate::{bnb::BnbMetric, float::Ordf32, CoinSelector, Drain, Target}; /// A selection is scored by `inner` only if the inner metric decides it should *not* have a change /// output (see [`BnbMetric::drain`]); otherwise it is treated as invalid. This lets you find, for /// example, the lowest-fee changeless solution via `Changeless`. -/// -/// `target` must match the target `inner` is optimizing for. It's used only by the branch-pruning -/// heuristic (to tell which candidates reduce the excess); the change decision and the scoring are -/// delegated entirely to `inner`. #[derive(Clone, Copy, Debug)] -pub struct Changeless { - /// The target of the resultant selection. Must match the target of `inner`. - pub target: Target, +pub struct Changeless( /// The inner metric that scores changeless solutions and owns the change decision. - pub inner: M, -} + pub M, +); impl Changeless { /// Whether every selection reachable down this branch (the current one and any superset of it) @@ -32,50 +26,50 @@ impl Changeless { /// are next to each other, which [`requires_ordering_by_descending_value_pwu`] guarantees. /// /// [`requires_ordering_by_descending_value_pwu`]: BnbMetric::requires_ordering_by_descending_value_pwu - fn change_unavoidable(&mut self, cs: &CoinSelector<'_>) -> bool { - if self.inner.drain(cs).is_none() { + fn change_unavoidable(&mut self, cs: &CoinSelector<'_>, target: Target) -> bool { + if self.0.drain(cs, target).is_none() { return false; } let mut least_excess = cs.clone(); cs.unselected() .rev() - .take_while(|(_, wv)| wv.effective_value(self.target.fee.rate) < 0.0) + .take_while(|(_, wv)| wv.effective_value(target.fee.rate) < 0.0) .for_each(|(index, _)| { least_excess.select(index); }); - self.inner.drain(&least_excess).is_some() + self.0.drain(&least_excess, target).is_some() } } impl BnbMetric for Changeless { - fn drain(&mut self, _cs: &CoinSelector<'_>) -> Drain { + fn drain(&mut self, _cs: &CoinSelector<'_>, _target: Target) -> Drain { // by definition a changeless selection never has a change output Drain::NONE } - fn score(&mut self, cs: &CoinSelector<'_>) -> Option { + fn score(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { // Reject selections that have change. We don't need an explicit target-met check: `inner` // returns `None` for invalid (e.g. not-target-met) selections. // // NOTE: for metrics whose `score` recomputes the drain (e.g. `LowestFee`), this evaluates // the drain decision twice per node. Sharing it would mean threading the drain into // `score`, which we avoid to keep metrics composable. - if self.inner.drain(cs).is_some() { + if self.0.drain(cs, target).is_some() { return None; } - self.inner.score(cs) + self.0.score(cs, target) } - fn bound(&mut self, cs: &CoinSelector<'_>) -> Option { - if self.change_unavoidable(cs) { + fn bound(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { + if self.change_unavoidable(cs, target) { // every descendant has change, so no changeless solution is reachable None } else { // the changeless-constrained optimum is no better than the inner metric's unconstrained // optimum, so the inner bound is a valid lower bound - self.inner.bound(cs) + self.0.bound(cs, target) } } diff --git a/src/metrics/lowest_fee.rs b/src/metrics/lowest_fee.rs index 0b4c14b..744bdd0 100644 --- a/src/metrics/lowest_fee.rs +++ b/src/metrics/lowest_fee.rs @@ -17,8 +17,6 @@ use crate::{float::Ordf32, BnbMetric, CoinSelector, Drain, DrainWeights, FeeRate /// dust threshold implied by `dust_relay_feerate`. #[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, /// The feerate used to determine the dust threshold of the change output. @@ -29,11 +27,11 @@ pub struct LowestFee { impl LowestFee { /// The value the change output should have, or `None` if this selection should be changeless. - fn drain_value(&self, cs: &CoinSelector<'_>) -> Option { + fn drain_value(&self, cs: &CoinSelector<'_>, target: Target) -> Option { // The change output pays for its own weight, so the value we'd actually recover is the // excess remaining after accounting for that weight. let excess_with_drain_weight = cs.excess( - self.target, + target, Drain { weights: self.drain_weights, value: 0, @@ -60,21 +58,22 @@ impl LowestFee { } impl BnbMetric for LowestFee { - fn drain(&mut self, cs: &CoinSelector<'_>) -> Drain { - self.drain_value(cs).map_or(Drain::NONE, |value| Drain { - weights: self.drain_weights, - value, - }) + fn drain(&mut self, cs: &CoinSelector<'_>, target: Target) -> Drain { + self.drain_value(cs, target) + .map_or(Drain::NONE, |value| Drain { + weights: self.drain_weights, + value, + }) } - fn score(&mut self, cs: &CoinSelector<'_>) -> Option { - if !cs.is_target_met(self.target) { + fn score(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { + if !cs.is_target_met(target) { return None; } let long_term_fee = { - let drain = self.drain(cs); - let fee_for_the_tx = cs.fee(self.target.value(), drain.value); + let drain = self.drain(cs, target); + let fee_for_the_tx = cs.fee(target.value(), drain.value); assert!( fee_for_the_tx >= 0, "must not be called unless selection has met target: fee={}", @@ -89,9 +88,9 @@ impl BnbMetric for LowestFee { Some(Ordf32(long_term_fee as f32)) } - fn bound(&mut self, cs: &CoinSelector<'_>) -> Option { - if cs.is_target_met(self.target) { - let current_score = self.score(cs).unwrap(); + fn bound(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { + if cs.is_target_met(target) { + let current_score = self.score(cs, target).unwrap(); // `current_score` is already a valid lower bound for a selection that has change: a // descendant can never lower the fee by removing an existing (worthwhile) change @@ -112,17 +111,17 @@ impl BnbMetric for LowestFee { // `drain_value`, where `change_value` is `excess_with_drain_weight` and `spend_fee` is // `drain_spend_cost`). With `v >= 0` the difference is strictly positive: B always // costs more. - if self.drain_value(cs).is_none() { + if self.drain_value(cs, target).is_none() { // But a descendant might *add* a change output that improves the metric. This // happens when the current selection is changeless only because the change would be // dust: a descendant with more excess could clear the dust threshold and recover // value that is currently burned to fees. let cost_of_adding_change = self.drain_weights.waste( - self.target.fee.rate, + target.fee.rate, self.long_term_feerate, - self.target.outputs.n_outputs, + target.outputs.n_outputs, ); - let cost_of_no_change = cs.excess(self.target, Drain::NONE); + let cost_of_no_change = cs.excess(target, Drain::NONE); let best_score_with_change = Ordf32(current_score.0 - cost_of_no_change as f32 + cost_of_adding_change); @@ -137,11 +136,11 @@ impl BnbMetric for LowestFee { let (mut cs, resize_index, to_resize) = cs .clone() .select_iter() - .find(|(cs, _, _)| cs.is_target_met(self.target))?; + .find(|(cs, _, _)| cs.is_target_met(target))?; // If this selection is already perfect, return its score directly. - if cs.excess(self.target, Drain::NONE) == 0 { - return Some(self.score(&cs).unwrap()); + if cs.excess(target, Drain::NONE) == 0 { + return Some(self.score(&cs, target).unwrap()); }; cs.deselect(resize_index); @@ -162,13 +161,12 @@ impl BnbMetric for LowestFee { // // In the perfect scenario, no additional fee would be required to pay for rounding up when converting from weight units to // vbytes and so all fee calculations below are performed on weight units directly. - let rate_excess = cs.rate_excess_wu(self.target, Drain::NONE) as f32; + let rate_excess = cs.rate_excess_wu(target, Drain::NONE) as f32; let mut scale = Ordf32(0.0); if rate_excess < 0.0 { let remaining_value_to_reach_feerate = rate_excess.abs(); - let effective_value_of_resized_input = - to_resize.effective_value(self.target.fee.rate); + let effective_value_of_resized_input = to_resize.effective_value(target.fee.rate); if effective_value_of_resized_input > 0.0 { let feerate_scale = remaining_value_to_reach_feerate / effective_value_of_resized_input; @@ -180,8 +178,8 @@ 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_wu(self.target, Drain::NONE) as f32; + if let Some(replace) = target.fee.replace { + let replace_excess = cs.replacement_excess_wu(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 = @@ -198,7 +196,7 @@ impl BnbMetric for LowestFee { // Handle absolute fee constraint. Unlike feerate and replacement, the // absolute fee is a fixed amount (not weight-proportional), so we just // need enough raw value to cover the gap. - let absolute_excess = cs.absolute_excess(self.target, Drain::NONE) as f32; + let absolute_excess = cs.absolute_excess(target, Drain::NONE) as f32; if absolute_excess < 0.0 { let remaining = absolute_excess.abs(); if to_resize.value > 0 { @@ -212,7 +210,7 @@ impl BnbMetric for LowestFee { // `scale` could be 0 even if `is_target_met` is `false` due to the latter being based on // rounded-up vbytes. let ideal_fee = scale.0 * to_resize.value as f32 + cs.selected_value() as f32 - - self.target.value() as f32; + - target.value() as f32; assert!(ideal_fee >= 0.0); Some(Ordf32(ideal_fee)) diff --git a/tests/bnb.rs b/tests/bnb.rs index 1d4c148..3e65022 100644 --- a/tests/bnb.rs +++ b/tests/bnb.rs @@ -26,16 +26,14 @@ fn test_wv(mut rng: impl RngCore) -> impl Iterator { } /// This is just an exhaustive search -struct MinExcessThenWeight { - target: Target, -} +struct MinExcessThenWeight; /// Assumes tx weight is less than 1MB. 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); + fn score(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { + let excess = cs.excess(target, Drain::NONE); if excess < 0 { None } else { @@ -45,13 +43,13 @@ impl BnbMetric for MinExcessThenWeight { } } - fn bound(&mut self, cs: &CoinSelector<'_>) -> Option { + fn bound(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { let mut cs = cs.clone(); - cs.select_until_target_met(self.target).ok()?; + cs.select_until_target_met(target).ok()?; Some(Ordf32(cs.input_weight() as f32)) } - fn drain(&mut self, _cs: &CoinSelector<'_>) -> Drain { + fn drain(&mut self, _cs: &CoinSelector<'_>, _target: Target) -> Drain { Drain::NONE } } @@ -94,7 +92,7 @@ fn bnb_finds_an_exact_solution_in_n_iter() { fee: TargetFee::ZERO, }; - let solutions = cs.bnb_solutions(MinExcessThenWeight { target }); + let solutions = cs.bnb_solutions(target, MinExcessThenWeight); let mut rounds = 0; let (best, score) = solutions @@ -128,7 +126,7 @@ fn bnb_finds_solution_if_possible_in_n_iter() { fee: TargetFee::default(), }; - let solutions = cs.bnb_solutions(MinExcessThenWeight { target }); + let solutions = cs.bnb_solutions(target, MinExcessThenWeight); let mut rounds = 0; let (sol, _score) = solutions @@ -156,7 +154,7 @@ proptest! { fee: TargetFee::ZERO, }; - let solutions = cs.bnb_solutions(MinExcessThenWeight { target }); + let solutions = cs.bnb_solutions(target, MinExcessThenWeight); match solutions.enumerate().filter_map(|(i, sol)| Some((i, sol?))).last() { Some((_i, (sol, _score))) => assert!(sol.selected_value() >= target_value), @@ -202,7 +200,7 @@ proptest! { fee: TargetFee::ZERO, }; - let solutions = cs.bnb_solutions(MinExcessThenWeight { target }); + let solutions = cs.bnb_solutions(target, MinExcessThenWeight); let (_i, (best, _score)) = solutions .enumerate() diff --git a/tests/changeless.rs b/tests/changeless.rs index 207d13b..37b8da5 100644 --- a/tests/changeless.rs +++ b/tests/changeless.rs @@ -68,17 +68,15 @@ proptest! { } }; - let make_metric = || Changeless { - target, - inner: LowestFee { - target, + let make_metric = || { + Changeless(LowestFee { long_term_feerate: feerate, dust_relay_feerate: FeeRate::from_sat_per_vb(1.0), drain_weights, - }, + }) }; - let solutions = cs.bnb_solutions(make_metric()); + let solutions = cs.bnb_solutions(target, make_metric()); println!("candidates: {:#?}", cs.candidates().collect::>()); @@ -95,7 +93,7 @@ proptest! { None => { let mut cs = cs.clone(); let mut metric = make_metric(); - let has_solution = common::exhaustive_search(&mut cs, &mut metric).is_some(); + let has_solution = common::exhaustive_search(&mut cs, target, &mut metric).is_some(); dbg!(format!("{}", cs)); assert!(!has_solution); } diff --git a/tests/common.rs b/tests/common.rs index aa6d8cd..3728c22 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -54,8 +54,8 @@ where println!("\texhaustive search:"); let now = std::time::Instant::now(); - let exp_result = exhaustive_search(&mut exp_selection, &mut metric); - let exp_change = metric.drain(&exp_selection); + let exp_result = exhaustive_search(&mut exp_selection, target, &mut metric); + let exp_change = metric.drain(&exp_selection, target); let exp_result_str = result_string(&exp_result.ok_or("no possible solution"), exp_change); println!( "\t\telapsed={:8}s result={}", @@ -65,7 +65,7 @@ where // bonus check: ensure replacement fee is respected if exp_result.is_some() { let selected_value = exp_selection.selected_value(); - let drain = metric.drain(&exp_selection); + let drain = metric.drain(&exp_selection, target); let target_value = target.value(); let replace_fee = params .replace @@ -80,8 +80,8 @@ where println!("\tbranch and bound:"); let now = std::time::Instant::now(); let mut bnb_metric = metric.clone(); - let result = bnb_search(&mut selection, metric, usize::MAX); - let change = bnb_metric.drain(&selection); + let result = bnb_search(&mut selection, target, metric, usize::MAX); + let change = bnb_metric.drain(&selection, target); let result_str = result_string(&result, change); println!( "\t\telapsed={:8}s result={}", @@ -105,7 +105,7 @@ where // bonus check: ensure replacement fee is respected let selected_value = selection.selected_value(); - let drain = bnb_metric.drain(&selection); + let drain = bnb_metric.drain(&selection, target); let target_value = target.value(); let replace_fee = params .replace @@ -150,12 +150,12 @@ where print_candidates(¶ms, &init_cs); for (cs, _) in ExhaustiveIter::new(&init_cs).into_iter().flatten() { - if let Some(lb_score) = metric.bound(&cs) { + if let Some(lb_score) = metric.bound(&cs, target) { // This is the branch's lower bound. In other words, this is the BEST selection // possible (can overshoot) traversing down this branch. Let's check that! - if let Some(score) = metric.score(&cs) { - let has_change = metric.drain(&cs).is_some(); + if let Some(score) = metric.score(&cs, target) { + let has_change = metric.drain(&cs, target).is_some(); prop_assert!( score >= lb_score, "checking branch: selection={} score={} change={} lb={}", @@ -171,9 +171,9 @@ where .flatten() .filter(|(_, inc)| *inc) { - if let Some(descendant_score) = metric.score(&descendant_cs) { - let parent_has_change = metric.drain(&cs).is_some(); - let descendant_has_change = metric.drain(&descendant_cs).is_some(); + if let Some(descendant_score) = metric.score(&descendant_cs, target) { + let parent_has_change = metric.drain(&cs, target).is_some(); + let descendant_has_change = metric.drain(&descendant_cs, target).is_some(); prop_assert!( descendant_score >= lb_score, " @@ -248,7 +248,6 @@ impl StrategyParams { pub fn lowest_fee_metric(&self) -> LowestFee { LowestFee { - target: self.target(), long_term_feerate: self.long_term_feerate(), dust_relay_feerate: self.dust_relay_feerate(), drain_weights: self.drain_weights(), @@ -332,7 +331,11 @@ impl<'a> Iterator for ExhaustiveIter<'a> { } } -pub fn exhaustive_search(cs: &mut CoinSelector, metric: &mut M) -> Option<(Ordf32, usize)> +pub fn exhaustive_search( + cs: &mut CoinSelector, + target: Target, + metric: &mut M, +) -> Option<(Ordf32, usize)> where M: BnbMetric, { @@ -347,7 +350,7 @@ where .enumerate() .inspect(|(i, _)| rounds = *i) .filter(|(_, (_, inclusion))| *inclusion) - .filter_map(|(_, (cs, _))| metric.score(&cs).map(|score| (cs, score))); + .filter_map(|(_, (cs, _))| metric.score(&cs, target).map(|score| (cs, score))); for (child_cs, score) in iter { match &mut best { @@ -371,6 +374,7 @@ where pub fn bnb_search( cs: &mut CoinSelector, + target: Target, metric: M, max_rounds: usize, ) -> Result<(Ordf32, usize), NoBnbSolution> @@ -379,7 +383,7 @@ where { let mut rounds = 0_usize; let (selection, score) = cs - .bnb_solutions(metric) + .bnb_solutions(target, metric) .inspect(|_| rounds += 1) .take(max_rounds) .flatten() @@ -418,7 +422,7 @@ pub fn compare_against_benchmarks( let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha); let target = params.target(); let cs = CoinSelector::new(&candidates); - let solutions = cs.bnb_solutions(metric.clone()); + let solutions = cs.bnb_solutions(target, metric.clone()); let best = solutions .enumerate() @@ -457,10 +461,10 @@ pub fn compare_against_benchmarks( let cmp_benchmarks = cmp_benchmarks .into_iter() .filter(|cs| cs.is_target_met(target)); - let sol_score = metric.score(&sol); + let sol_score = metric.score(&sol, target); for (_bench_id, mut bench) in cmp_benchmarks.enumerate() { - let bench_score = metric.score(&bench); + let bench_score = metric.score(&bench, target); if sol_score > bench_score { dbg!(_bench_id); println!("bnb solution: {}", sol); @@ -492,7 +496,7 @@ fn randomly_satisfy_target<'a, R: rand::Rng>( while let Some(next) = cs.unselected_indices().choose(rng) { cs.select(next); if cs.is_target_met(target) { - let curr_score = metric.score(&cs); + let curr_score = metric.score(&cs, target); if let Some(last_score) = last_score { if curr_score.is_none() || curr_score.unwrap() > last_score { break; diff --git a/tests/lowest_fee.rs b/tests/lowest_fee.rs index 112bbe1..efbbd36 100644 --- a/tests/lowest_fee.rs +++ b/tests/lowest_fee.rs @@ -88,7 +88,7 @@ proptest! { let metric = params.lowest_fee_metric(); let is_impossible = !cs.is_selection_possible(params.target()); - match common::bnb_search(&mut cs, metric, params.n_candidates * 10) { + match common::bnb_search(&mut cs, params.target(), 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 @@ -144,21 +144,20 @@ fn combined_changeless_metric() { let mut cs_a = CoinSelector::new(&candidates); let mut cs_b = CoinSelector::new(&candidates); + let target = params.target(); let metric_lowest_fee = params.lowest_fee_metric(); - let metric_changeless = Changeless { - target: params.target(), - inner: params.lowest_fee_metric(), - }; + let metric_changeless = Changeless(params.lowest_fee_metric()); // cs_a uses the unconstrained metric - let (score, rounds) = - common::bnb_search(&mut cs_a, metric_lowest_fee, usize::MAX).expect("must find solution"); + let (score, rounds) = common::bnb_search(&mut cs_a, target, metric_lowest_fee, usize::MAX) + .expect("must find solution"); println!("score={:?} rounds={}", score, rounds); // cs_b uses the changeless-constrained metric let (combined_score, combined_rounds) = - common::bnb_search(&mut cs_b, metric_changeless, usize::MAX).expect("must find solution"); + common::bnb_search(&mut cs_b, target, metric_changeless, usize::MAX) + .expect("must find solution"); println!("score={:?} rounds={}", combined_score, combined_rounds); assert!(combined_rounds >= rounds); @@ -211,13 +210,12 @@ fn does_not_create_change_below_spend_cost() { }; let mut metric = LowestFee { - target, long_term_feerate: FeeRate::from_sat_per_vb(1.0), dust_relay_feerate: FeeRate::from_sat_per_vb(1.0), drain_weights, }; - let (score, _) = common::bnb_search(&mut cs, metric, 10).expect("finds solution"); + let (score, _) = common::bnb_search(&mut cs, target, metric, 10).expect("finds solution"); // The optimal selection is candidate 0 alone, and it must be changeless. let expected = { @@ -227,7 +225,7 @@ fn does_not_create_change_below_spend_cost() { }; assert_eq!(cs.selected_indices(), expected.selected_indices()); assert!( - metric.drain(&cs).is_none(), + metric.drain(&cs, target).is_none(), "optimal selection must be changeless" ); @@ -237,7 +235,12 @@ fn does_not_create_change_below_spend_cost() { with_extra_input.select(2); with_extra_input }; - assert!(score <= metric.score(&with_extra_input).expect("target is met")); + assert!( + score + <= metric + .score(&with_extra_input, target) + .expect("target is met") + ); } #[test] @@ -281,10 +284,10 @@ fn zero_fee_tx() { let mut cs = CoinSelector::new(&candidates); let metric = LowestFee { - target, long_term_feerate, dust_relay_feerate: FeeRate::from_sat_per_vb(1.0), drain_weights, }; - let (_score, _rounds) = common::bnb_search(&mut cs, metric, 1000).expect("must find solution"); + let (_score, _rounds) = + common::bnb_search(&mut cs, target, metric, 1000).expect("must find solution"); }