WIP(coin_select): prop tests for lowest fee metric

`can_eventually_find_best_solution` ensures that the lowest fee metric
is always able to find the best solution. We find the best solution via
an exhaustive search. Essentially, this ensures that our bounding method
never undershoots.

`ensure_bound_does_not_undershoot` is a more fine-grained test for the
above. The checks descendant solutions after a branch, and ensures that
the branch's lower-bound is never bet with the descendant solutions.

Author: evanlinjin

nursery/coin_select/src/bnb.rs

@@ -18,7 +18,7 @@ impl<'a, M: BnbMetric> Iterator for BnbIter<'a, M> {
         //     for thing in self.queue.iter() {
         //         println!("{} {:?}", &thing.selector, thing.lower_bound);
         //     }
-        //     let _ = std::io::stdin().read_line(&mut String::new());
+        //     let _ = std::io::stdin().read_line(&mut alloc::string::String::new());
         // }
 
         let branch = self.queue.pop()?;
@@ -88,14 +88,14 @@ impl<'a, M: BnbMetric> BnbIter<'a, M> {
         }
 
         let next_unselected = cs.unselected_indices().next().unwrap();
-        let mut inclusion_cs = cs.clone();
-        inclusion_cs.select(next_unselected);
+
         let mut exclusion_cs = cs.clone();
         exclusion_cs.ban(next_unselected);
+        self.consider_adding_to_queue(&exclusion_cs, true);
 
-        for (child_cs, is_exclusion) in &[(&inclusion_cs, false), (&exclusion_cs, true)] {
-            self.consider_adding_to_queue(child_cs, *is_exclusion)
-        }
+        let mut inclusion_cs = cs.clone();
+        inclusion_cs.select(next_unselected);
+        self.consider_adding_to_queue(&inclusion_cs, false);
     }
 }
 

nursery/coin_select/src/coin_selector.rs

@@ -12,7 +12,7 @@ use alloc::{borrow::Cow, collections::BTreeSet, vec::Vec};
 /// [`bnb_solutions`]: CoinSelector::bnb_solutions
 #[derive(Debug, Clone)]
 pub struct CoinSelector<'a> {
-    base_weight: u32,
+    pub base_weight: u32,
     candidates: &'a [Candidate],
     selected: Cow<'a, BTreeSet<usize>>,
     banned: Cow<'a, BTreeSet<usize>>,
@@ -672,8 +672,8 @@ impl std::error::Error for InsufficientFunds {}
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct NoBnbSolution {
-    max_rounds: usize,
-    rounds: usize,
+    pub max_rounds: usize,
+    pub rounds: usize,
 }
 
 impl core::fmt::Display for NoBnbSolution {

nursery/coin_select/src/metrics/lowest_fee.rs

@@ -9,11 +9,35 @@ pub struct LowestFee<'c, C> {
     pub change_policy: &'c C,
 }
 
+impl<'c, C> Clone for LowestFee<'c, C> {
+    fn clone(&self) -> Self {
+        Self {
+            target: self.target.clone(),
+            long_term_feerate: self.long_term_feerate.clone(),
+            change_policy: self.change_policy.clone(),
+        }
+    }
+}
+
+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,
 {
-    fn calculate_metric(&self, cs: &CoinSelector<'_>, drain_weights: Option<DrainWeights>) -> f32 {
+    fn calc_metric(&self, cs: &CoinSelector<'_>, drain_weights: Option<DrainWeights>) -> f32 {
+        self.calc_metric_lb(cs, drain_weights)
+            + match drain_weights {
+                Some(_) => {
+                    let selected_value = cs.selected_value();
+                    assert!(selected_value >= self.target.value);
+                    (cs.selected_value() - self.target.value) as f32
+                }
+                None => 0.0,
+            }
+    }
+
+    fn calc_metric_lb(&self, cs: &CoinSelector<'_>, drain_weights: Option<DrainWeights>) -> f32 {
         match drain_weights {
             // with change
             Some(drain_weights) => {
@@ -22,10 +46,7 @@ where
                     + drain_weights.spend_weight as f32 * self.long_term_feerate.spwu()
             }
             // changeless
-            None => {
-                cs.input_weight() as f32 * self.target.feerate.spwu()
-                    + (cs.selected_value() - self.target.value) as f32
-            }
+            None => cs.input_weight() as f32 * self.target.feerate.spwu(),
         }
     }
 }
@@ -48,7 +69,7 @@ where
             None
         };
 
-        Some(Ordf32(self.calculate_metric(cs, drain_weights)))
+        Some(Ordf32(self.calc_metric(cs, drain_weights)))
     }
 
     fn bound(&mut self, cs: &CoinSelector<'_>) -> Option<Self::Score> {
@@ -66,11 +87,12 @@ where
             // Target is met, is it possible to add further inputs to remove drain output?
             // If we do, can we get a better score?
 
-            // First lower bound candidate is just the selection itself.
-            let mut lower_bound = self.calculate_metric(cs, change_lb_weights);
+            // First lower bound candidate is just the selection itself (include excess).
+            let mut lower_bound = self.calc_metric(cs, change_lb_weights);
 
-            // Since a changeless solution may exist, we should try reduce the excess
             if change_lb.is_none() {
+                // Since a changeless solution may exist, we should try minimize the excess with by
+                // adding as much -ev candidates as possible
                 let selection_with_as_much_negative_ev_as_possible = cs
                     .clone()
                     .select_iter()
@@ -95,18 +117,19 @@ where
                         });
                     let lower_bound_changeless = match can_do_better_by_slurping {
                         Some(finishing_input) => {
-                            let excess = cs.rate_excess(self.target, Drain::none());
+                            let excess = dbg!(cs.rate_excess(self.target, Drain::none()));
 
                             // change the input's weight to make it's effective value match the excess
                             let perfect_input_weight =
-                                slurp_candidate(finishing_input, excess, self.target.feerate);
+                                slurp(&cs, self.target, excess, finishing_input);
 
-                            (cs.input_weight() as f32 + perfect_input_weight)
+                            (cs.input_weight() as f32 + dbg!(perfect_input_weight))
                                 * self.target.feerate.spwu()
                         }
-                        None => self.calculate_metric(&cs, None),
+                        None => self.calc_metric(&cs, None),
                     };
 
+                    // assert!(lower_bound_changeless > lower_bound);
                     lower_bound = lower_bound.min(lower_bound_changeless)
                 }
             }
@@ -122,54 +145,46 @@ where
             .find(|(cs, _, _)| cs.is_target_met(self.target, change_lb))?;
         cs.deselect(slurp_index);
 
-        let perfect_excess = i64::min(
-            cs.rate_excess(self.target, Drain::none()),
-            cs.absolute_excess(self.target, Drain::none()),
-        );
+        let mut lower_bound = self.calc_metric_lb(&cs, change_lb_weights);
 
-        match change_lb_weights {
-            // must have change!
-            Some(change_weights) => {
-                // [todo] This will not be perfect, just a placeholder for now
-                let lowest_fee = (cs.input_weight() + change_weights.output_weight) as f32
-                    * self.target.feerate.spwu()
-                    + change_weights.spend_weight as f32 * self.long_term_feerate.spwu();
+        if change_lb_weights.is_none() {
+            // changeless solution is possible, find the max excess we need to rid of
+            let perfect_excess = i64::max(
+                cs.rate_excess(self.target, Drain::none()),
+                cs.absolute_excess(self.target, Drain::none()),
+            );
 
-                Some(Ordf32(lowest_fee))
-            }
-            // can be changeless!
-            None => {
-                // use the lowest excess to find "perfect candidate weight"
-                let perfect_input_weight =
-                    slurp_candidate(candidate_to_slurp, perfect_excess, self.target.feerate);
+            // use the lowest excess to find "perfect candidate weight"
+            let perfect_input_weight = slurp(&cs, self.target, perfect_excess, candidate_to_slurp);
 
-                // the perfect input weight canned the excess and we assume no change
-                let lowest_fee =
-                    (cs.input_weight() as f32 + perfect_input_weight) * self.target.feerate.spwu();
-
-                Some(Ordf32(lowest_fee))
-            }
+            lower_bound += perfect_input_weight * self.target.feerate.spwu();
         }
+
+        Some(Ordf32(lower_bound))
     }
 
     fn requires_ordering_by_descending_value_pwu(&self) -> bool {
         true
     }
 }
 
-fn slurp_candidate(candidate: Candidate, excess: i64, feerate: FeeRate) -> f32 {
-    let candidate_weight = candidate.weight as f32;
-
-    // this equation is dervied from:
-    // * `input_effective_value = input_value - input_weight * feerate`
-    // * `input_value * new_input_weight = new_input_value * input_weight`
-    //      (ensure we have the same value:weight ratio)
-    // where we want `input_effective_value` to match `-excess`.
-    let perfect_weight = -(candidate_weight * excess as f32)
-        / (candidate.value as f32 - candidate_weight * feerate.spwu());
-
-    debug_assert!(perfect_weight <= candidate_weight);
-
-    // we can't allow the weight to go negative
-    perfect_weight.min(0.0)
+fn slurp(_cs: &CoinSelector, target: Target, excess: i64, candidate: Candidate) -> f32 {
+    // let input_eff_sum = cs.effective_value(target.feerate) as f32;
+    // let eff_target = cs.base_weight as f32 * target.feerate.spwu() + target.value as f32;
+    let vpw = candidate.value_pwu().0;
+
+    // let perfect_weight =
+    //     (-old_excess as f32 + eff_target - input_eff_sum) / (vpw - target.feerate.spwu());
+    let perfect_weight = -excess as f32 / (vpw - target.feerate.spwu());
+
+    debug_assert!(
+        {
+            let perfect_value = (candidate.value as f32 * perfect_weight) / candidate.weight as f32;
+            let perfect_vpw = perfect_value / perfect_weight;
+            (vpw - perfect_vpw).abs() < 0.01
+        },
+        "value:weight ratio must stay the same"
+    );
+
+    perfect_weight.max(0.0)
 }

nursery/coin_select/src/metrics/waste.rs

@@ -24,6 +24,18 @@ pub struct Waste<'c, C> {
     pub change_policy: &'c C,
 }
 
+impl<'c, C> Clone for Waste<'c, C> {
+    fn clone(&self) -> Self {
+        Self {
+            target: self.target.clone(),
+            long_term_feerate: self.long_term_feerate.clone(),
+            change_policy: self.change_policy.clone(),
+        }
+    }
+}
+
+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,

nursery/coin_select/tests/changeless.rs

@@ -1,5 +1,7 @@
 #![allow(unused)]
-use bdk_coin_select::{float::Ordf32, metrics, Candidate, CoinSelector, Drain, FeeRate, Target};
+use bdk_coin_select::{
+    float::Ordf32, metrics, Candidate, CoinSelector, Drain, DrainWeights, FeeRate, Target,
+};
 use proptest::{
     prelude::*,
     test_runner::{RngAlgorithm, TestRng},
@@ -41,10 +43,9 @@ proptest! {
         let mut rng = TestRng::deterministic_rng(RngAlgorithm::ChaCha);
         let long_term_feerate = FeeRate::from_sat_per_vb(0.0f32.max(feerate - long_term_feerate_diff));
         let feerate = FeeRate::from_sat_per_vb(feerate);
-        let drain = Drain {
-            weight: change_weight,
+        let drain = DrainWeights {
+            output_weight: change_weight,
             spend_weight: change_spend_weight,
-            value: 0
         };
 
         let change_policy = bdk_coin_select::change_policy::min_waste(drain, long_term_feerate);
@@ -59,7 +60,7 @@ proptest! {
             min_fee
         };
 
-        let solutions = cs.branch_and_bound(metrics::Changeless {
+        let solutions = cs.bnb_solutions(metrics::Changeless {
             target,
             change_policy: &change_policy
         });
@@ -78,7 +79,7 @@ proptest! {
                         let mut naive_select = cs.clone();
                         naive_select.sort_candidates_by_key(|(_, wv)| core::cmp::Reverse(wv.effective_value(target.feerate)));
                         // we filter out failing onces below
-                        let _ = naive_select.select_until_target_met(target, drain);
+                        let _ = naive_select.select_until_target_met(target, Drain { weights: drain, value: 0 });
                         naive_select
                     },
                 ];

nursery/coin_select/tests/metrics_lowest_fee.proptest-regressions

@@ -0,0 +1,9 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc a75fec9349c584791ef633224614369ec3721bccbe91bb24fe96978637a01098 # shrinks to n_candidates = 13, target_value = 16056, base_weight = 484, min_fee = 0, feerate = 37.709625, feerate_lt_diff = 0.0, drain_weight = 220, drain_spend_weight = 369, drain_dust = 100
+cc 19a016e1ac5ec54d3b513b0e09a1a287ab49c6454313e2220b881be5bfa18df5 # shrinks to n_candidates = 0, target_value = 200, base_weight = 164, min_fee = 0, feerate = 1.0, feerate_lt_diff = 0.0, drain_weight = 100, drain_spend_weight = 250, drain_dust = 100
+cc 9ded3861c49dc9c6992657857771e24b5538b38917c701104b527de3ba155f5f # shrinks to n_candidates = 1, target_value = 200, base_weight = 164, min_fee = 0, feerate = 1.0, feerate_lt_diff = 0.0, drain_weight = 100, drain_spend_weight = 250, drain_dust = 100