diff --git a/src/coin_selector.rs b/src/coin_selector.rs index 0d42a56..8fd13ad 100644 --- a/src/coin_selector.rs +++ b/src/coin_selector.rs @@ -169,29 +169,10 @@ impl<'a> CoinSelector<'a> { /// enough value. /// /// [`ban`]: Self::ban - pub fn is_selection_possible(&self, target: Target, drain: Drain) -> bool { + pub fn is_selection_possible(&self, target: Target) -> bool { let mut test = self.clone(); test.select_all_effective(target.feerate); - test.is_target_met(target, drain) - } - - /// Is meeting the target *plausible* with this `change_policy`. - /// Note this will respect [`ban`]ned candidates. - /// - /// This is very similar to [`is_selection_possible`] except that you pass in a change policy. - /// This method will give the right answer as long as `change_policy` is monotone but otherwise - /// can it can give false negatives. - /// - /// [`ban`]: Self::ban - /// [`is_selection_possible`]: Self::is_selection_possible - pub fn is_selection_plausible_with_change_policy( - &self, - target: Target, - change_policy: &impl Fn(&CoinSelector<'a>, Target) -> Drain, - ) -> bool { - let mut test = self.clone(); - test.select_all_effective(target.feerate); - test.is_target_met(target, change_policy(&test, target)) + test.is_target_met(target) } /// Returns true if no candidates have been selected. @@ -405,11 +386,22 @@ impl<'a> CoinSelector<'a> { self.unselected_indices().next().is_none() } - /// Whether the constraints of `Target` have been met if we include the `drain` ouput. - pub fn is_target_met(&self, target: Target, drain: Drain) -> bool { + /// Whether the constraints of `Target` have been met if we include a specific `drain` ouput. + /// + /// Note if [`is_target_met`] is true and the `drain` is produced from the [`drain`] method then + /// this method will also always be true. + /// + /// [`is_target_met`]: Self::is_target_met + /// [`drain`]: Self::drain + pub fn is_target_met_with_drain(&self, target: Target, drain: Drain) -> bool { self.excess(target, drain) >= 0 } + /// Whether the constraints of `Target` have been met. + pub fn is_target_met(&self, target: Target) -> bool { + self.is_target_met_with_drain(target, Drain::none()) + } + /// 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( @@ -417,7 +409,7 @@ impl<'a> CoinSelector<'a> { target: Target, change_policy: ChangePolicy, ) -> bool { - self.is_target_met(target, self.drain(target, change_policy)) + self.is_target_met_with_drain(target, self.drain(target, change_policy)) } /// Select all unselected candidates @@ -442,17 +434,34 @@ impl<'a> CoinSelector<'a> { }, ); if excess > change_policy.min_value as i64 { + debug_assert_eq!( + self.is_target_met(target), + self.is_target_met_with_drain( + target, + Drain { + weights: change_policy.drain_weights, + value: excess as u64 + } + ), + "if the target is met without a drain it must be met after adding the drain" + ); 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`. + /// Figures out whether the current selection should have a change output given the + /// `change_policy`. If it shouldn't then it will return a `Drain` where [`Drain::is_none`] is + /// true. The value of the `Drain` will be the same as [`drain_value`]. + /// + /// If [`is_target_met`] returns true for this selection then [`is_target_met_with_drain`] will + /// also be true if you pass in the drain returned from this method. /// /// [`drain_value`]: Self::drain_value + /// [`is_target_met_with_drain`]: Self::is_target_met_with_drain + /// [`is_target_met`]: Self::is_target_met + #[must_use] pub fn drain(&self, target: Target, change_policy: ChangePolicy) -> Drain { match self.drain_value(target, change_policy) { Some(value) => Drain { @@ -486,7 +495,7 @@ impl<'a> CoinSelector<'a> { target: Target, drain: Drain, ) -> Result<(), InsufficientFunds> { - self.select_until(|cs| cs.is_target_met(target, drain)) + self.select_until(|cs| cs.is_target_met_with_drain(target, drain)) .ok_or_else(|| InsufficientFunds { missing: self.excess(target, drain).unsigned_abs(), }) diff --git a/src/metrics/changeless.rs b/src/metrics/changeless.rs index 133859e..fec7e6f 100644 --- a/src/metrics/changeless.rs +++ b/src/metrics/changeless.rs @@ -1,7 +1,5 @@ use super::change_lower_bound; -use crate::{ - bnb::BnbMetric, change_policy::ChangePolicy, float::Ordf32, CoinSelector, Drain, Target, -}; +use crate::{bnb::BnbMetric, change_policy::ChangePolicy, float::Ordf32, CoinSelector, Target}; /// Metric for finding changeless solutions only. pub struct Changeless { @@ -13,7 +11,7 @@ pub struct Changeless { impl BnbMetric for Changeless { fn score(&mut self, cs: &CoinSelector<'_>) -> Option { - if cs.is_target_met(self.target, Drain::none()) + if cs.is_target_met(self.target) && cs.drain_value(self.target, self.change_policy).is_none() { Some(Ordf32(0.0)) diff --git a/src/metrics/lowest_fee.rs b/src/metrics/lowest_fee.rs index c9027a1..b8b3e39 100644 --- a/src/metrics/lowest_fee.rs +++ b/src/metrics/lowest_fee.rs @@ -51,7 +51,7 @@ impl LowestFee { impl BnbMetric for LowestFee { fn score(&mut self, cs: &CoinSelector<'_>) -> Option { let drain = cs.drain(self.target, self.change_policy); - if !cs.is_target_met(self.target, drain) { + if !cs.is_target_met_with_drain(self.target, drain) { return None; } @@ -76,7 +76,7 @@ impl BnbMetric for LowestFee { }; // println!("\tchange lb: {:?}", change_lb_weights); - if cs.is_target_met(self.target, change_lb) { + if cs.is_target_met_with_drain(self.target, change_lb) { // Target is met, is it possible to add further inputs to remove drain output? // If we do, can we get a better score? @@ -92,7 +92,7 @@ impl BnbMetric for LowestFee { .rev() .take_while(|(cs, _, candidate)| { candidate.effective_value(self.target.feerate).0 < 0.0 - && cs.is_target_met(self.target, Drain::none()) + && cs.is_target_met_with_drain(self.target, Drain::none()) }) .last() .map(|(cs, _, _)| cs); @@ -133,7 +133,7 @@ impl BnbMetric for LowestFee { let (mut cs, slurp_index, candidate_to_slurp) = cs .clone() .select_iter() - .find(|(cs, _, _)| cs.is_target_met(self.target, change_lb))?; + .find(|(cs, _, _)| cs.is_target_met_with_drain(self.target, change_lb))?; cs.deselect(slurp_index); let mut lower_bound = self.calc_metric_lb(&cs, change_lb_weights); diff --git a/src/metrics/waste.rs b/src/metrics/waste.rs index a8f5312..430a504 100644 --- a/src/metrics/waste.rs +++ b/src/metrics/waste.rs @@ -33,7 +33,7 @@ pub struct Waste { impl BnbMetric for Waste { fn score(&mut self, cs: &CoinSelector<'_>) -> Option { let drain = cs.drain(self.target, self.change_policy); - if !cs.is_target_met(self.target, drain) { + if !cs.is_target_met_with_drain(self.target, drain) { return None; } let score = cs.waste(self.target, self.long_term_feerate, drain, 1.0); @@ -57,7 +57,7 @@ impl BnbMetric for Waste { 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) { + if cs.is_target_met_with_drain(self.target, change_lower_bound) { let current_change = cs.drain(self.target, self.change_policy); // first lower bound candidate is just the selection itself @@ -80,8 +80,8 @@ impl BnbMetric for Waste { .select_iter() .rev() .take_while(|(cs, _, wv)| { - wv.effective_value(self.target.feerate).0 < 0.0 - && cs.is_target_met(self.target, Drain::none()) + wv.effective_value(self.target.feerate) < Ordf32(0.0) + && cs.is_target_met(self.target) }) .last(); @@ -133,10 +133,10 @@ impl BnbMetric for Waste { // weight. // // Step 1: select everything up until the input that hits the target. - let (mut cs, slurp_index, to_slurp) = cs - .clone() - .select_iter() - .find(|(cs, _, _)| cs.is_target_met(self.target, change_lower_bound))?; + let (mut cs, slurp_index, to_slurp) = + cs.clone().select_iter().find(|(cs, _, _)| { + cs.is_target_met_with_drain(self.target, change_lower_bound) + })?; cs.deselect(slurp_index); @@ -177,7 +177,7 @@ impl BnbMetric for Waste { let mut cs = cs.clone(); // ... but first check that by selecting all effective we can actually reach target cs.select_all_effective(self.target.feerate); - if !cs.is_target_met(self.target, Drain::none()) { + if !cs.is_target_met(self.target) { return None; } let change_at_value_optimum = cs.drain(self.target, self.change_policy); diff --git a/tests/bnb.rs b/tests/bnb.rs index 686e107..d39a77a 100644 --- a/tests/bnb.rs +++ b/tests/bnb.rs @@ -154,7 +154,7 @@ proptest! { match solutions.enumerate().filter_map(|(i, sol)| Some((i, sol?))).last() { Some((_i, (sol, _score))) => assert!(sol.selected_value() >= target.value), - _ => prop_assert!(!cs.is_selection_possible(target, Drain::none())), + _ => prop_assert!(!cs.is_selection_possible(target)), } } diff --git a/tests/common.rs b/tests/common.rs index 2b72c07..fad3baa 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -154,7 +154,7 @@ where cs, cs.drain_value(target, change_policy).is_some(), lb_score, - cs.is_target_met(target, Drain::none()), + cs.is_target_met(target), descendant_cs, descendant_cs.drain_value(target, change_policy).is_some(), descendant_score, diff --git a/tests/waste.rs b/tests/waste.rs index f67145a..4fcb390 100644 --- a/tests/waste.rs +++ b/tests/waste.rs @@ -485,7 +485,7 @@ fn randomly_satisfy_target_with_low_waste<'a>( while let Some(next) = cs.unselected_indices().choose(rng) { cs.select(next); let change = change_policy(&cs, target); - if cs.is_target_met(target, change) { + if cs.is_target_met_with_drain(target, change) { let curr_waste = cs.waste(target, long_term_feerate, change, 1.0); if let Some(last_waste) = last_waste { if curr_waste > last_waste {