From 52263fad41b5900550f306f92899832694f52852 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 24 Aug 2023 17:19:30 +0800 Subject: [PATCH] WIP(coin_select): what am I doing? --- nursery/coin_select/src/bnb.rs | 53 ++++++++++++++-- nursery/coin_select/src/metrics.rs | 4 ++ nursery/coin_select/src/metrics/changeless.rs | 4 ++ nursery/coin_select/src/metrics/lowest_fee.rs | 10 ++- nursery/coin_select/src/metrics/waste.rs | 4 ++ nursery/coin_select/tests/bnb.rs | 4 ++ nursery/coin_select/tests/common.rs | 37 +++++++---- .../tests/lowest_fee.proptest-regressions | 10 +++ nursery/coin_select/tests/lowest_fee.rs | 63 +++++++++++++++++-- 9 files changed, 164 insertions(+), 25 deletions(-) create mode 100644 nursery/coin_select/tests/lowest_fee.proptest-regressions diff --git a/nursery/coin_select/src/bnb.rs b/nursery/coin_select/src/bnb.rs index 58ef851bd6..cb41aa4c81 100644 --- a/nursery/coin_select/src/bnb.rs +++ b/nursery/coin_select/src/bnb.rs @@ -1,3 +1,5 @@ +use core::cmp::Reverse; + use super::CoinSelector; use alloc::collections::BinaryHeap; @@ -22,9 +24,22 @@ impl<'a, M: BnbMetric> Iterator for BnbIter<'a, M> { // } let branch = self.queue.pop()?; + println!( + "\tbranch={} depth={}({}) lb={:?}, score={:?}", + branch.selector, + branch.depth, + if branch.is_exclusion { + "exclusion" + } else { + "inclusion" + }, + branch.lower_bound, + self.metric.score(&branch.selector), + ); if let Some(best) = &self.best { // If the next thing in queue is worse than our best we're done if *best < branch.lower_bound { + println!("\t\tDONE? best={:?} lb={:?}", *best, branch.lower_bound); return None; } } @@ -64,19 +79,27 @@ impl<'a, M: BnbMetric> BnbIter<'a, M> { selector.sort_candidates_by_descending_value_pwu(); } - iter.consider_adding_to_queue(&selector, false); + // iter.consider_adding_to_queue(&selector, false, 0); + iter.insert_new_branches(&selector); iter } - fn consider_adding_to_queue(&mut self, cs: &CoinSelector<'a>, is_exclusion: bool) { + fn consider_adding_to_queue( + &mut self, + cs: &CoinSelector<'a>, + is_exclusion: bool, + depth: usize, + ) { let bound = self.metric.bound(cs); if let Some(bound) = bound { if self.best.is_none() || self.best.as_ref().unwrap() > &bound { self.queue.push(Branch { lower_bound: bound, selector: cs.clone(), + is_target_met: self.metric.is_target_met(cs), is_exclusion, + depth, }); } } @@ -91,11 +114,11 @@ impl<'a, M: BnbMetric> BnbIter<'a, M> { let mut inclusion_cs = cs.clone(); inclusion_cs.select(next_unselected); - self.consider_adding_to_queue(&inclusion_cs, false); + self.consider_adding_to_queue(&inclusion_cs, false, next_unselected + 1); let mut exclusion_cs = cs.clone(); exclusion_cs.ban(next_unselected); - self.consider_adding_to_queue(&exclusion_cs, true); + self.consider_adding_to_queue(&exclusion_cs, true, next_unselected + 1); } } @@ -103,7 +126,9 @@ impl<'a, M: BnbMetric> BnbIter<'a, M> { struct Branch<'a, O> { lower_bound: O, selector: CoinSelector<'a>, + is_target_met: bool, is_exclusion: bool, + depth: usize, } impl<'a, O: Ord> Ord for Branch<'a, O> { @@ -112,7 +137,21 @@ impl<'a, O: Ord> Ord for Branch<'a, O> { // NOTE: We tiebreak equal scores based on whether it's exlusion or not (preferring inclusion). // We do this because we want to try and get to evaluating complete selection returning // actual scores as soon as possible. - (&other.lower_bound, other.is_exclusion).cmp(&(&self.lower_bound, self.is_exclusion)) + let s = ( + // self.is_target_met, + Reverse(&self.lower_bound), + self.depth, + !self.is_exclusion, + // self.depth, + ); + let o = ( + // other.is_target_met, + Reverse(&other.lower_bound), + other.depth, + !other.is_exclusion, + // other.depth, + ); + core::cmp::Ord::cmp(&s, &o) } } @@ -135,7 +174,11 @@ pub trait BnbMetric { type Score: Ord + Clone + core::fmt::Debug; fn score(&mut self, cs: &CoinSelector<'_>) -> Option; + fn bound(&mut self, cs: &CoinSelector<'_>) -> Option; + + fn is_target_met(&mut self, cs: &CoinSelector<'_>) -> bool; + fn requires_ordering_by_descending_value_pwu(&self) -> bool { false } diff --git a/nursery/coin_select/src/metrics.rs b/nursery/coin_select/src/metrics.rs index cab73b8f2d..498b65242b 100644 --- a/nursery/coin_select/src/metrics.rs +++ b/nursery/coin_select/src/metrics.rs @@ -54,6 +54,10 @@ macro_rules! impl_for_tuple { Some(($(self.$b.bound(cs)?),*)) } #[allow(unused)] + fn is_target_met(&mut self, cs: &CoinSelector<'_>) -> bool { + [$(self.$b.is_target_met(cs)),*].iter().all(|x| *x) + } + #[allow(unused)] fn requires_ordering_by_descending_value_pwu(&self) -> bool { [$(self.$b.requires_ordering_by_descending_value_pwu()),*].iter().all(|x| *x) diff --git a/nursery/coin_select/src/metrics/changeless.rs b/nursery/coin_select/src/metrics/changeless.rs index dfa629a04f..7b8bb28b7e 100644 --- a/nursery/coin_select/src/metrics/changeless.rs +++ b/nursery/coin_select/src/metrics/changeless.rs @@ -26,6 +26,10 @@ where Some(change_lower_bound(cs, self.target, &self.change_policy).is_some()) } + fn is_target_met(&mut self, cs: &CoinSelector<'_>) -> bool { + cs.is_target_met(self.target, (self.change_policy)(cs, self.target)) + } + fn requires_ordering_by_descending_value_pwu(&self) -> bool { true } diff --git a/nursery/coin_select/src/metrics/lowest_fee.rs b/nursery/coin_select/src/metrics/lowest_fee.rs index b5f011f99d..5216e9d9f1 100644 --- a/nursery/coin_select/src/metrics/lowest_fee.rs +++ b/nursery/coin_select/src/metrics/lowest_fee.rs @@ -88,9 +88,9 @@ where // If we do, can we get a better score? // First lower bound candidate is just the selection itself (include excess). - let mut lower_bound = self.calc_metric(cs, change_lb_weights); + let mut lower_bound = self.calc_metric_lb(cs, None); - if change_lb.is_none() { + if change_lb_weights.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 @@ -143,7 +143,7 @@ where .find(|(cs, _, _)| cs.is_target_met(self.target, change_lb))?; cs.deselect(slurp_index); - let mut lower_bound = self.calc_metric_lb(&cs, change_lb_weights); + let mut lower_bound = self.calc_metric_lb(&cs, None); if change_lb_weights.is_none() { // changeless solution is possible, find the max excess we need to rid of @@ -161,6 +161,10 @@ where Some(Ordf32(lower_bound)) } + fn is_target_met(&mut self, cs: &CoinSelector<'_>) -> bool { + cs.is_target_met(self.target, (self.change_policy)(cs, self.target)) + } + fn requires_ordering_by_descending_value_pwu(&self) -> bool { true } diff --git a/nursery/coin_select/src/metrics/waste.rs b/nursery/coin_select/src/metrics/waste.rs index 02d800258c..499e36689f 100644 --- a/nursery/coin_select/src/metrics/waste.rs +++ b/nursery/coin_select/src/metrics/waste.rs @@ -233,6 +233,10 @@ where } } + fn is_target_met(&mut self, cs: &CoinSelector<'_>) -> bool { + cs.is_target_met(self.target, (self.change_policy)(cs, self.target)) + } + fn requires_ordering_by_descending_value_pwu(&self) -> bool { true } diff --git a/nursery/coin_select/tests/bnb.rs b/nursery/coin_select/tests/bnb.rs index 90fb692ee4..0e29d37920 100644 --- a/nursery/coin_select/tests/bnb.rs +++ b/nursery/coin_select/tests/bnb.rs @@ -51,6 +51,10 @@ impl BnbMetric for MinExcessThenWeight { }; Some((lower_bound_excess, lower_bound_weight)) } + + fn is_target_met(&mut self, cs: &CoinSelector<'_>) -> bool { + cs.is_target_met(self.target, Drain::none()) + } } #[test] diff --git a/nursery/coin_select/tests/common.rs b/nursery/coin_select/tests/common.rs index b7b3a42f14..e59900bfb1 100644 --- a/nursery/coin_select/tests/common.rs +++ b/nursery/coin_select/tests/common.rs @@ -25,6 +25,7 @@ where { println!("== TEST =="); println!("{}", type_name::()); + println!("{:?}", params); let candidates = gen_candidates(params.n_candidates); { @@ -60,7 +61,7 @@ where println!("\tbranch and bound:"); let now = std::time::Instant::now(); - let result = bnb_search(&mut selection, metric); + let result = bnb_search(&mut selection, metric, usize::MAX); let change = change_policy(&selection, target); let result_str = result_string(&result, change); println!( @@ -102,6 +103,7 @@ where { println!("== TEST =="); println!("{}", type_name::()); + println!("{:?}", params); let candidates = gen_candidates(params.n_candidates); { @@ -126,7 +128,7 @@ where cs }; - for cs in ExhaustiveIter::new(&init_cs).into_iter().flatten() { + for (cs, _) in ExhaustiveIter::new(&init_cs).into_iter().flatten() { if let Some(lb_score) = metric.bound(&cs) { // 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! @@ -141,7 +143,11 @@ where ); } - for descendant_cs in ExhaustiveIter::new(&cs).into_iter().flatten() { + for (descendant_cs, _) in ExhaustiveIter::new(&cs) + .into_iter() + .flatten() + .filter(|(_, inc)| *inc) + { if let Some(descendant_score) = metric.score(&descendant_cs) { prop_assert!( descendant_score >= lb_score, @@ -158,6 +164,7 @@ where Ok(()) } +#[derive(Debug)] pub struct StrategyParams { pub n_candidates: usize, pub target_value: u64, @@ -248,15 +255,16 @@ impl<'a> ExhaustiveIter<'a> { } impl<'a> Iterator for ExhaustiveIter<'a> { - type Item = CoinSelector<'a>; + type Item = (CoinSelector<'a>, bool); fn next(&mut self) -> Option { loop { let (cs, inclusion) = self.stack.pop()?; let _more = self.push_branches(&cs); - if inclusion { - return Some(cs); - } + return Some((cs, inclusion)); + // if inclusion { + // return Some(cs); + // } } } } @@ -275,7 +283,8 @@ where let iter = ExhaustiveIter::new(cs)? .enumerate() .inspect(|(i, _)| rounds = *i) - .filter_map(|(_, cs)| metric.score(&cs).map(|score| (cs, score))); + .filter(|(_, (_, inclusion))| *inclusion) + .filter_map(|(_, (cs, _))| metric.score(&cs).map(|score| (cs, score))); for (child_cs, score) in iter { match &mut best { @@ -297,7 +306,11 @@ where best.map(|(_, score)| (score, rounds)) } -pub fn bnb_search(cs: &mut CoinSelector, metric: M) -> Result<(Ordf32, usize), NoBnbSolution> +pub fn bnb_search( + cs: &mut CoinSelector, + metric: M, + max_rounds: usize, +) -> Result<(Ordf32, usize), NoBnbSolution> where M: BnbMetric, { @@ -305,12 +318,10 @@ where let (selection, score) = cs .bnb_solutions(metric) .inspect(|_| rounds += 1) + .take(max_rounds) .flatten() .last() - .ok_or(NoBnbSolution { - max_rounds: usize::MAX, - rounds, - })?; + .ok_or(NoBnbSolution { max_rounds, rounds })?; println!("\t\tsolution={}, score={}", selection, score); *cs = selection; diff --git a/nursery/coin_select/tests/lowest_fee.proptest-regressions b/nursery/coin_select/tests/lowest_fee.proptest-regressions new file mode 100644 index 0000000000..2e9a467865 --- /dev/null +++ b/nursery/coin_select/tests/lowest_fee.proptest-regressions @@ -0,0 +1,10 @@ +# 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 9c841bb85574de2412972df187e7ebd01f7a06a178a67f4d99c0178dd578ac34 # shrinks to n_candidates = 30, target_value = 76632, base_weight = 480, min_fee = 0, feerate = 8.853, feerate_lt_diff = 0.0, drain_weight = 100, drain_spend_weight = 1, drain_dust = 100 +cc e30499b75a1846759fc9ffd7ee558b08a4795598cf7919f6be6d62cc7a79d4cb # shrinks to n_candidates = 25, target_value = 56697, base_weight = 621, min_fee = 0, feerate = 9.417939, feerate_lt_diff = 0.0, drain_weight = 100, drain_spend_weight = 1, drain_dust = 100 +cc c580ee452624915fc710d5fe724c7a9347472ccd178f66c9db9479cfc6168f48 # shrinks to n_candidates = 25, target_value = 488278, base_weight = 242, min_fee = 0, feerate = 6.952743, feerate_lt_diff = 0.0, drain_weight = 100, drain_spend_weight = 1, drain_dust = 100 +cc 850e0115aeeb7ed50235fdb4b5183eb5bf8309a45874dc261e3d3fd2d8c84660 # shrinks to n_candidates = 8, target_value = 444541, base_weight = 253, min_fee = 0, feerate = 55.98181, feerate_lt_diff = 36.874306, drain_weight = 490, drain_spend_weight = 1779, drain_dust = 100 diff --git a/nursery/coin_select/tests/lowest_fee.rs b/nursery/coin_select/tests/lowest_fee.rs index e1855ca8f4..7b7b84c7a9 100644 --- a/nursery/coin_select/tests/lowest_fee.rs +++ b/nursery/coin_select/tests/lowest_fee.rs @@ -1,6 +1,7 @@ mod common; use bdk_coin_select::change_policy::min_value_and_waste; use bdk_coin_select::metrics::LowestFee; +use bdk_coin_select::{Candidate, CoinSelector}; use proptest::prelude::*; proptest! { @@ -13,7 +14,7 @@ proptest! { n_candidates in 1..20_usize, // candidates (n) target_value in 500..500_000_u64, // target value (sats) base_weight in 0..1000_u32, // base weight (wu) - min_fee in 0..1_000_u64, // min fee (sats) + min_fee in 0..1000_u64, // min fee (sats) feerate in 1.0..100.0_f32, // feerate (sats/vb) feerate_lt_diff in -5.0..50.0_f32, // longterm feerate diff (sats/vb) drain_weight in 100..=500_u32, // drain weight (wu) @@ -49,10 +50,10 @@ proptest! { #[test] fn ensure_bound_is_not_too_tight( - n_candidates in 0..15_usize, // candidates (n) + n_candidates in 0..20_usize, // candidates (n) target_value in 500..500_000_u64, // target value (sats) - base_weight in 0..641_u32, // base weight (wu) - min_fee in 0..1_000_u64, // min fee (sats) + base_weight in 0..1000_u32, // base weight (wu) + min_fee in 0..1000_u64, // min fee (sats) feerate in 1.0..100.0_f32, // feerate (sats/vb) feerate_lt_diff in -5.0..50.0_f32, // longterm feerate diff (sats/vb) drain_weight in 100..=500_u32, // drain weight (wu) @@ -85,4 +86,58 @@ proptest! { }, )?; } + + #[test] + fn should_prefer_inclusion_branch( + n_candidates in 30..1000_usize, + target_value in 50_000..500_000_u64, // target value (sats) + base_weight in 0..641_u32, // base weight (wu) + min_fee in 0..1_000_u64, // min fee (sats) + feerate in 1.0..10.0_f32, // feerate (sats/vb) + feerate_lt_diff in -5.0..5.0_f32, // longterm feerate diff (sats/vb) + drain_weight in 100..=500_u32, // drain weight (wu) + drain_spend_weight in 1..=1000_u32, // drain spend weight (wu) + drain_dust in 100..=1000_u64, // drain dust (sats) + ) { + println!("== TEST =="); + + let params = common::StrategyParams { + n_candidates, + target_value, + base_weight, + min_fee, + feerate, + feerate_lt_diff, + drain_weight, + drain_spend_weight, + drain_dust, + }; + println!("{:?}", params); + + let candidates = core::iter::repeat(Candidate { + value: 20_000, + weight: (32 + 4 + 4 + 1) * 4 + 64 + 32, + input_count: 1, + is_segwit: true, + }) + .take(params.n_candidates) + .collect::>(); + + let mut cs = CoinSelector::new(&candidates, params.base_weight); + + 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, + }; + + let (score, rounds) = common::bnb_search(&mut cs, metric, params.n_candidates+1)?; + println!("\t\tscore={} rounds={}", score, rounds); + } }