From 60306162d1f3d11c72133e37083400e3758c9c97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Sat, 15 Aug 2026 09:17:18 +0000 Subject: [PATCH 1/3] fix: compute LowestFee bound's ideal_fee in f64 and floor it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ideal_fee` cancels large sat amounts down to a small fee, so in `f32` the rounding error can exceed the result itself. The old code asserted the result non-negative, which a selection driving `scale = 52692/145266` trips: the product comes back ~0.0039 short of the target value. `assert!` is unconditional, so this panicked in release too. Clamping the negative case alone would only address the harmless direction — a bound that is too *low* just loosens the search. The dangerous direction is a bound that is too *high*: `bnb.rs` keeps a branch only when `best > bound`, so an inflated bound prunes the optimum. Computing in `f64` and rounding down covers both; every real score is a whole number of sats, so flooring stays admissible. Co-Authored-By: Claude Opus 5 --- src/metrics/lowest_fee.rs | 14 +++++-- tests/lowest_fee.rs | 82 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/src/metrics/lowest_fee.rs b/src/metrics/lowest_fee.rs index 5499777..51b3f1e 100644 --- a/src/metrics/lowest_fee.rs +++ b/src/metrics/lowest_fee.rs @@ -271,11 +271,17 @@ impl BnbMetric for LowestFee { // `scale` could be 0 even if `is_funded` 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 - - target.value() as f32; - assert!(ideal_fee >= 0.0); + // + // Computed in `f64` and rounded *down*. This is a small fee obtained by cancelling + // large sat amounts, so in `f32` the error can exceed the result itself — and in either + // direction. Erring low only loosens the bound, but erring high makes it inadmissible + // and prunes the optimum. Every real score is a whole number of sats, so flooring is + // admissible and absorbs what error `f64` leaves. (`as i64` truncates toward zero, + // which is `floor` for the non-negative case; `core` has no `f64::floor`.) + let ideal_fee = (scale.0 as f64 * to_resize.value as f64 + cs.selected_value() as f64 + - target.value() as f64) as i64; - Some(Ordf32(ideal_fee)) + Some(Ordf32(ideal_fee.max(0) as f32)) } } diff --git a/tests/lowest_fee.rs b/tests/lowest_fee.rs index d6b8cba..791b21f 100644 --- a/tests/lowest_fee.rs +++ b/tests/lowest_fee.rs @@ -434,3 +434,85 @@ fn run_bnb_reports_round_limit() { }, ); } + +/// `LowestFee::bound` used to compute `ideal_fee` in `f32` and assert it non-negative. The +/// expression cancels large sat amounts down to a small fee, so `f32` rounding could land it just +/// below zero and blow the assert — in release too, since it was a plain `assert!`. +/// +/// This selection drives `scale = 52692/145266`, and `scale * 145266` comes back ~0.0039 short. +#[test] +fn bound_does_not_panic_on_f32_rounding() { + let candidates = [ + Candidate { + value: 1_480_489, + weight: 2_663, + input_count: 1, + is_segwit: true, + }, + Candidate { + value: 4_167, + weight: 360, + input_count: 1, + is_segwit: true, + }, + Candidate { + value: 13_447, + weight: 7_597, + input_count: 1, + is_segwit: true, + }, + Candidate { + value: 17_281, + weight: 368, + input_count: 1, + is_segwit: true, + }, + Candidate { + value: 26_718, + weight: 755, + input_count: 1, + is_segwit: true, + }, + Candidate { + value: 237_889, + weight: 4_570, + input_count: 1, + is_segwit: true, + }, + Candidate { + value: 273_139, + weight: 997, + input_count: 1, + is_segwit: true, + }, + Candidate { + value: 145_266, + weight: 2_124, + input_count: 1, + is_segwit: true, + }, + ]; + let target = Target { + fee: TargetFee::ZERO, + outputs: TargetOutputs { + value_sum: 52_692, + weight_sum: 208, + n_outputs: 1, + }, + max_weight: None, + }; + let metric = LowestFee { + long_term_feerate: FeeRate::from_sat_per_wu(0.59), + dust_relay_feerate: FeeRate::from_sat_per_wu(1.4), + drain_weights: DrainWeights { + output_weight: 342, + spend_weight: 815, + n_outputs: 2, + }, + }; + + let mut cs = CoinSelector::new(&candidates); + // The target is trivially met (feerate and absolute fee are both zero), so this must find a + // solution rather than merely not panicking. + assert!(cs.run_bnb(target, metric, 100_000).is_ok()); +} From c8baf67e87d0984a7a76ef6b742d978ec83325de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Sat, 15 Aug 2026 09:24:33 +0000 Subject: [PATCH 2/3] feat!: represent FeeRate as an exact integer (sat/kvb) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `FeeRate` was an `f32` sat/wu value, and every fee derived from it went through float multiply-then-ceil. Those fees are whole satoshis and feed `CoinSelector::excess` -> `is_funded`, so a rounding error there doesn't merely pick a worse selection — it reports a selection as funded while the transaction is short of its target feerate, with nothing to signal it. The float computation was exact for everyday transactions; it diverged once the implied fee outgrew `f32`'s exact-integer range (from ~2^22 sats for `implied_fee_wu`, ~2^24 for `implied_fee`), then drifted up to 2 sats in either direction. `is_funded_flips_exactly_at_the_required_fee` pins the boundary and does catch the old code accepting a selection one sat short. Store sat per 1000 vbytes instead, matching Bitcoin Core's `CFeeRate`: finer resolution than sat/kwu, and it makes `implied_fee` literally Core's `GetFee`. All integer-valued fee functions (`implied_fee`, `implied_fee_wu`, `spend_fee`, `dust_threshold`, `CoinSelector::effective_value`, `ChangePolicy`'s waste floor) are now exact, with `u128` intermediates that saturate rather than wrap. The genuinely fractional quantities (`spwu`, `value_pwu`, `waste`) stay floats — they are search heuristics where an error costs a slightly worse pick, not validity. BREAKING: `FeeRate` constructors quantize to whole sat/kvb (0.001 sat/vb). `FeeRate::sub` saturates at zero rather than producing a negative feerate. `float::FloatExt` is removed — it existed only to supply `f32::ceil` under `no_std` for these fee paths, and nothing uses it now. Co-Authored-By: Claude Opus 5 --- src/coin_selector.rs | 6 +- src/drain.rs | 44 +++++--- src/feerate.rs | 233 +++++++++++++++++++++++++++++++++++++------ src/float.rs | 34 ------- 4 files changed, 237 insertions(+), 80 deletions(-) diff --git a/src/coin_selector.rs b/src/coin_selector.rs index 604abd8..c90a37e 100644 --- a/src/coin_selector.rs +++ b/src/coin_selector.rs @@ -1,6 +1,4 @@ use super::*; -#[allow(unused)] // some bug in <= 1.48.0 sees this as unused when it isn't -use crate::float::FloatExt; use crate::{bitset::Bitset, bnb::BnbMetric, float::Ordf32, ChangePolicy, FeeRate, Target}; use alloc::{sync::Arc, vec::Vec}; @@ -267,7 +265,7 @@ impl<'a> CoinSelector<'a> { if numerator < 0 || denom == 0 { return None; } - Some(FeeRate::from_sat_per_wu(numerator as f32 / denom as f32)) + Some(FeeRate::from_wu(numerator as u64, denom as usize)) } /// The fee the current selection and `drain_weight` should pay to satisfy `target_fee`. @@ -315,7 +313,7 @@ impl<'a> CoinSelector<'a> { /// The value of the current selected inputs minus the fee needed to pay for the selected inputs pub fn effective_value(&self, feerate: FeeRate) -> i64 { - self.selected_value() as i64 - (self.input_weight() as f32 * feerate.spwu()).ceil() as i64 + self.selected_value() as i64 - feerate.implied_fee_wu(self.input_weight()) as i64 } // /// Waste sum of all selected inputs. diff --git a/src/drain.rs b/src/drain.rs index 98067ef..309a02e 100644 --- a/src/drain.rs +++ b/src/drain.rs @@ -1,5 +1,3 @@ -#[allow(unused)] // some bug in <= 1.48.0 sees this as unused when it isn't -use crate::float::FloatExt; use crate::{varint_size, FeeRate, TR_KEYSPEND_TXIN_WEIGHT, TR_SPK_WEIGHT, TXOUT_BASE_WEIGHT}; /// Represents the weight costs of a drain (a.k.a. change) output. @@ -32,11 +30,22 @@ impl DrainWeights { n_outputs: 0, }; + /// The output weight this drain adds to the transaction, including the extra varint weight + /// from growing the output count past `n_target_outputs`. + fn added_output_weight(&self, n_target_outputs: usize) -> u64 { + let extra_varint_weight = + (varint_size(n_target_outputs + self.n_outputs) - varint_size(n_target_outputs)) * 4; + self.output_weight + extra_varint_weight + } + /// The waste of adding this drain to a transaction according to the [waste metric]. /// /// To get the precise answer you need to pass in the number of non-drain outputs (`n_target_outputs`) that you're /// adding to the transaction so we can include the cost of increasing the varint size of the output length. /// + /// This is a search heuristic, so it stays a float. Where the answer is a whole number of + /// satoshis, the crate uses the exact integer counterpart `waste_ceil` instead. + /// /// [waste metric]: https://bitcoin.stackexchange.com/questions/113622/what-does-waste-metric-mean-in-the-context-of-coin-selection pub fn waste( &self, @@ -44,16 +53,25 @@ impl DrainWeights { long_term_feerate: FeeRate, n_target_outputs: usize, ) -> f32 { - let extra_varint_weight = - (varint_size(n_target_outputs + self.n_outputs) - varint_size(n_target_outputs)) * 4; - let extra_output_weight = self.output_weight + extra_varint_weight; - extra_output_weight as f32 * feerate.spwu() + self.added_output_weight(n_target_outputs) as f32 * feerate.spwu() + self.spend_weight as f32 * long_term_feerate.spwu() } + /// Exact-integer counterpart of [`waste`](Self::waste): the satoshis it costs to add this + /// drain, rounding each fee component up. + fn waste_ceil( + &self, + feerate: FeeRate, + long_term_feerate: FeeRate, + n_target_outputs: usize, + ) -> u64 { + feerate.implied_fee_wu(self.added_output_weight(n_target_outputs)) + + self.spend_fee(long_term_feerate) + } + /// The fee you will pay to spend these change output(s) in the future. pub fn spend_fee(&self, long_term_feerate: FeeRate) -> u64 { - (self.spend_weight as f32 * long_term_feerate.spwu()).ceil() as u64 + long_term_feerate.implied_fee_wu(self.spend_weight) } /// The minimum value a change output with these weights must have to not be considered dust @@ -131,13 +149,11 @@ impl ChangePolicy { long_term_feerate: FeeRate, ) -> Self { // The output waste of a changeless solution is the excess. - let waste_with_change = drain_weights - .waste( - target_feerate, - long_term_feerate, - 0, /* ignore varint cost for now */ - ) - .ceil() as u64; + let waste_with_change = drain_weights.waste_ceil( + target_feerate, + long_term_feerate, + 0, /* ignore varint cost for now */ + ); Self { drain_weights, diff --git a/src/feerate.rs b/src/feerate.rs index b4f0490..38b1846 100644 --- a/src/feerate.rs +++ b/src/feerate.rs @@ -1,34 +1,61 @@ -#[cfg(not(feature = "std"))] -use crate::float::FloatExt; -use crate::float::Ordf32; use core::ops::{Add, Sub}; /// Fee rate #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] -// Internally stored as satoshi/weight unit -pub struct FeeRate(Ordf32); +// Internally stored as satoshi per 1000 vbytes, the same unit as Bitcoin Core's `CFeeRate`. +// +// This is an exact integer rather than a float on purpose. The fees derived from it +// ([`implied_fee`](Self::implied_fee) and friends) are whole satoshis that feed +// `CoinSelector::excess` and hence `is_funded`, so a rounding error there doesn't merely pick a +// worse selection — it calls a selection funded when it is short of the target feerate, silently. +// +// The old `f32` computation was exact for everyday transactions and only diverged once the implied +// fee outgrew `f32`'s exact-integer range: from ~2^22 sats for `implied_fee_wu` (reached by e.g. a +// 28k wu tx at 599 sat/vb) and ~2^24 sats for `implied_fee` (a 112k wu tx at 599 sat/vb). Past +// those points it was off by up to 2 sats in *either* direction — including underpaying. Integers +// cost nothing here and remove the threshold entirely. +pub struct FeeRate(u64); + +/// Weight units per 1000 vbytes. +const WU_PER_KVB: u128 = 4000; +/// Vbytes per 1000 vbytes. +const VB_PER_KVB: u128 = 1000; + +/// `ceil(numerator / denominator)`, saturating at `u64::MAX` instead of wrapping. +fn ceil_div(numerator: u128, denominator: u128) -> u64 { + let ceiled = (numerator + (denominator - 1)) / denominator; + if ceiled > u64::MAX as u128 { + u64::MAX + } else { + ceiled as u64 + } +} impl FeeRate { /// A feerate of zero - pub const ZERO: Self = Self(Ordf32(0.0)); + pub const ZERO: Self = Self(0); /// The default minimum relay fee that bitcoin core uses (1 sat per vbyte). The feerate your transaction has must /// be at least this to be forwarded by most nodes on the network. - pub const DEFAULT_MIN_RELAY: Self = Self(Ordf32(0.25)); + pub const DEFAULT_MIN_RELAY: Self = Self(1_000); /// The defualt incremental relay fee that bitcoin core uses (1 sat per vbyte). You must pay /// this fee over the fee of the transaction(s) you are replacing by through the replace-by-fee /// mechanism. This feerate is applied to the transaction that is replacing the old /// transactions. - pub const DEFUALT_RBF_INCREMENTAL_RELAY: Self = Self(Ordf32(0.25)); - /// Create a new instance checking the value provided + pub const DEFUALT_RBF_INCREMENTAL_RELAY: Self = Self(1_000); + + /// Create a new instance from a satoshi/kvb value, checking it and rounding it to the nearest + /// whole satoshi/kvb. /// /// ## Panics /// /// Panics if the value is not [normal](https://doc.rust-lang.org/std/primitive.f32.html#method.is_normal) (except if it's a positive zero) or negative. - fn new_checked(value: f32) -> Self { - assert!(value.is_normal() || value == 0.0); - assert!(value.is_sign_positive()); + fn new_checked(sat_per_kvb: f32) -> Self { + assert!(sat_per_kvb.is_normal() || sat_per_kvb == 0.0); + assert!(sat_per_kvb.is_sign_positive()); - Self(Ordf32(value)) + // Round to nearest. `core` has no `f32::round`, and the value is known non-negative here, + // so adding a half and truncating does it. + Self((sat_per_kvb as f64 + 0.5) as u64) } /// Create a new instance of [`FeeRate`] given a float fee rate in btc/kvbytes @@ -37,7 +64,7 @@ impl FeeRate { /// /// Panics if the value is not [normal](https://doc.rust-lang.org/std/primitive.f32.html#method.is_normal) (except if it's a positive zero) or negative. pub fn from_btc_per_kvb(btc_per_kvb: f32) -> Self { - Self::new_checked(btc_per_kvb * 1e5 / 4.0) + Self::new_checked(btc_per_kvb * 1e8) } /// Create a new instance of [`FeeRate`] given a float fee rate in satoshi/vbyte @@ -46,50 +73,57 @@ impl FeeRate { /// /// Panics if the value is not [normal](https://doc.rust-lang.org/std/primitive.f32.html#method.is_normal) (except if it's a positive zero) or negative. pub fn from_sat_per_vb(sat_per_vb: f32) -> Self { - Self::new_checked(sat_per_vb / 4.0) + Self::new_checked(sat_per_vb * VB_PER_KVB as f32) } /// Create a new [`FeeRate`] with the default min relay fee value #[deprecated(note = "use the DEFAULT_MIN_RELAY constant instead")] pub const fn default_min_relay_fee() -> Self { - Self(Ordf32(0.25)) + Self(1_000) } - /// Calculate fee rate from `fee` and weight units (`wu`). + /// Calculate fee rate from `fee` and weight units (`wu`), rounding down. pub fn from_wu(fee: u64, wu: usize) -> Self { - Self::from_sat_per_wu(fee as f32 / wu as f32) + Self(((fee as u128 * WU_PER_KVB) / wu as u128) as u64) } /// Calculate feerate from `satoshi/wu`. pub fn from_sat_per_wu(sats_per_wu: f32) -> Self { - Self::new_checked(sats_per_wu) + Self::new_checked(sats_per_wu * WU_PER_KVB as f32) } - /// Calculate fee rate from `fee` and `vbytes`. + /// Calculate fee rate from `fee` and `vbytes`, rounding down. pub fn from_vb(fee: u64, vbytes: usize) -> Self { - let rate = fee as f32 / vbytes as f32; - Self::from_sat_per_vb(rate) + Self(((fee as u128 * VB_PER_KVB) / vbytes as u128) as u64) + } + + /// Return the value as satoshi per 1000 vbytes. This is the exact internal representation; + /// prefer it over [`spwu`](Self::spwu) and [`as_sat_vb`](Self::as_sat_vb) when you need to do + /// exact arithmetic. + pub fn sat_per_kvb(&self) -> u64 { + self.0 } /// Return the value as satoshi/vbyte. pub fn as_sat_vb(&self) -> f32 { - self.0 .0 * 4.0 + self.0 as f32 / VB_PER_KVB as f32 } /// Return the value as satoshi/wu. pub fn spwu(&self) -> f32 { - self.0 .0 + self.0 as f32 / WU_PER_KVB as f32 } /// The fee that the transaction with weight `tx_weight` should pay in order to satisfy the fee rate given by `self`, /// where the fee rate is applied to the rounded-up vbytes obtained from `tx_weight`. pub fn implied_fee(&self, tx_weight: u64) -> u64 { - ((tx_weight as f32 / 4.0).ceil() * self.as_sat_vb()).ceil() as u64 + let vbytes = (tx_weight as u128 + 3) / 4; + ceil_div(vbytes * self.0 as u128, VB_PER_KVB) } /// Same as [implied_fee](Self::implied_fee) except the fee rate given by `self` is applied to `tx_weight` directly. pub fn implied_fee_wu(&self, tx_weight: u64) -> u64 { - (tx_weight as f32 * self.spwu()).ceil() as u64 + ceil_div(tx_weight as u128 * self.0 as u128, WU_PER_KVB) } } @@ -97,14 +131,157 @@ impl Add for FeeRate { type Output = Self; fn add(self, rhs: FeeRate) -> Self::Output { - Self(Ordf32(self.0 .0 + rhs.0 .0)) + Self(self.0.saturating_add(rhs.0)) } } impl Sub for FeeRate { type Output = Self; + /// Saturates at [`FeeRate::ZERO`] — a negative feerate isn't representable. fn sub(self, rhs: FeeRate) -> Self::Output { - Self(Ordf32(self.0 .0 - rhs.0 .0)) + Self(self.0.saturating_sub(rhs.0)) + } +} + +#[cfg(test)] +mod test { + use super::*; + + /// The unit conversions must round-trip through the documented accessors. + #[test] + fn unit_conversions() { + assert_eq!(FeeRate::from_sat_per_vb(1.0), FeeRate::DEFAULT_MIN_RELAY); + assert_eq!(FeeRate::from_sat_per_wu(0.25), FeeRate::DEFAULT_MIN_RELAY); + assert_eq!(FeeRate::from_btc_per_kvb(1e-5), FeeRate::DEFAULT_MIN_RELAY); + assert_eq!(FeeRate::from_vb(10, 10), FeeRate::DEFAULT_MIN_RELAY); + assert_eq!(FeeRate::from_wu(10, 40), FeeRate::DEFAULT_MIN_RELAY); + assert_eq!(FeeRate::DEFAULT_MIN_RELAY.as_sat_vb(), 1.0); + assert_eq!(FeeRate::DEFAULT_MIN_RELAY.spwu(), 0.25); + assert_eq!(FeeRate::DEFAULT_MIN_RELAY.sat_per_kvb(), 1000); + } + + /// `implied_fee` charges whole (rounded-up) vbytes; `implied_fee_wu` charges weight directly. + #[test] + fn implied_fees_round_up() { + let ten_sat_vb = FeeRate::from_sat_per_vb(10.0); + // 9 wu -> 3 vbytes -> 30 sats + assert_eq!(ten_sat_vb.implied_fee(9), 30); + assert_eq!(ten_sat_vb.implied_fee(12), 30); + assert_eq!(ten_sat_vb.implied_fee(13), 40); + // applied to weight directly: 9 wu * 2.5 sat/wu = 22.5 -> 23 + assert_eq!(ten_sat_vb.implied_fee_wu(9), 23); + assert_eq!(ten_sat_vb.implied_fee_wu(12), 30); + + assert_eq!(FeeRate::ZERO.implied_fee(1000), 0); + assert_eq!(FeeRate::ZERO.implied_fee_wu(1000), 0); + } + + /// The whole point of the integer representation: the fee is exactly `ceil(weight * feerate)` + /// with no float rounding drift, at *any* magnitude. + /// + /// The range here deliberately runs past where `f32` loses whole satoshis (implied fees above + /// ~2^22 / ~2^24 sats); the old computation was off by up to 2 sats there, in either direction. + #[test] + fn implied_fees_are_exact() { + for sat_vb in 1..600_u64 { + let feerate = FeeRate::from_sat_per_vb(sat_vb as f32); + for weight in (200..400_000_u64).step_by(311) { + assert_eq!( + feerate.implied_fee(weight), + ((weight + 3) / 4) * sat_vb, + "implied_fee({}) at {} sat/vb", + weight, + sat_vb + ); + assert_eq!( + feerate.implied_fee_wu(weight), + (weight * sat_vb + 3) / 4, + "implied_fee_wu({}) at {} sat/vb", + weight, + sat_vb + ); + } + } + } + + /// A feerate large enough to overflow `u64` in the intermediate product must saturate rather + /// than wrap to a tiny fee (which would read as "this selection is funded"). + #[test] + fn implied_fee_saturates_instead_of_wrapping() { + let absurd = FeeRate::from_sat_per_vb(1e18); + assert_eq!(absurd.implied_fee(400_000), u64::MAX); + assert_eq!(absurd.implied_fee_wu(400_000), u64::MAX); + } + + /// The invariant the integer representation exists to protect: `is_funded` must flip exactly at + /// the satoshi that covers the target feerate — never one or two sats early, which would call a + /// selection funded while the transaction is short of its feerate. + /// + /// Probes the boundary itself, where an off-by-a-couple-sats fee is decisive. The sweep runs + /// out to large weights and high feerates on purpose: that is where the old `f32` fee lost + /// whole satoshis and moved this boundary. + #[test] + fn is_funded_flips_exactly_at_the_required_fee() { + use crate::{Candidate, CoinSelector, DrainWeights, Target, TargetFee, TargetOutputs}; + + let outputs = TargetOutputs { + value_sum: 99_000, + weight_sum: 400, + n_outputs: 2, + }; + + for weight in (200..500_000_u64).step_by(4_093) { + for sat_vb in [1u64, 3, 10, 50, 135, 337, 599] { + let target = Target { + fee: TargetFee::from_feerate(FeeRate::from_sat_per_vb(sat_vb as f32)), + outputs, + max_weight: None, + }; + + // The tx weight doesn't depend on the candidate's value, so compute the required + // fee once, independently, in exact integer arithmetic. + let probe = [Candidate { + value: 0, + weight, + input_count: 1, + is_segwit: true, + }]; + let mut cs = CoinSelector::new(&probe); + cs.select(0); + let tx_weight = cs.weight(outputs, DrainWeights::NONE); + let required = (((tx_weight as u128 + 3) / 4) * sat_vb as u128) as u64; + let exact = outputs.value_sum + required; + + for (value, want_funded) in [(exact - 1, false), (exact, true), (exact + 1, true)] { + let candidates = [Candidate { + value, + weight, + input_count: 1, + is_segwit: true, + }]; + let mut cs = CoinSelector::new(&candidates); + cs.select(0); + assert_eq!( + cs.is_funded(target), + want_funded, + "weight={} sat/vb={} value={} (exact boundary {}, required fee {})", + weight, + sat_vb, + value, + exact, + required + ); + } + } + } + } + + #[test] + fn sub_saturates_at_zero() { + assert_eq!( + FeeRate::from_sat_per_vb(1.0) - FeeRate::from_sat_per_vb(5.0), + FeeRate::ZERO + ); } } diff --git a/src/float.rs b/src/float.rs index 61b756f..383de5d 100644 --- a/src/float.rs +++ b/src/float.rs @@ -60,37 +60,3 @@ impl core::fmt::Display for Ordf64 { self.0.fmt(f) } } - -/// Extension trait for adding basic float ops to f32 that don't exist in core for reasons. -pub trait FloatExt { - /// Adds the ceil method to `f32` - fn ceil(self) -> Self; -} - -impl FloatExt for f32 { - fn ceil(self) -> Self { - // From https://doc.rust-lang.org/reference/expressions/operator-expr.html#type-cast-expressions - // > Casting from a float to an integer will round the float towards zero - // > Casting from an integer to float will produce the closest possible float - let floored_towards_zero = (self as i32) as f32; - if self < 0.0 || floored_towards_zero == self { - floored_towards_zero - } else { - floored_towards_zero + 1.0 - } - } -} - -#[cfg(test)] -mod test { - use super::*; - #[test] - fn ceil32() { - assert_eq!((-1.1).ceil(), -1.0); - assert_eq!((-0.1).ceil(), 0.0); - assert_eq!((0.0).ceil(), 0.0); - assert_eq!((1.0).ceil(), 1.0); - assert_eq!((1.1).ceil(), 2.0); - assert_eq!((2.9).ceil(), 3.0); - } -} From 532ef503ad352f7ecec0dee783ebca512e11c221 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Sat, 15 Aug 2026 09:34:06 +0000 Subject: [PATCH 3/3] feat!: score and bound branch and bound in exact satoshis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `BnbMetric::score` and `bound` returned `Ordf32`, but both are whole satoshi counts: `LowestFee::fee_score` computes an exact `u64` and then threw it into an `f32`. The bound had it worse — it divided to get `scale`, multiplied back up by a candidate value, and cancelled that against the selected and target values. At Bitcoin-scale amounts that cancellation lost whole satoshis, and the error went in both directions. Erring low only loosens the bound. Erring high makes it inadmissible: `bnb.rs` keeps a branch only when `best > bound`, so an inflated bound discards the branch holding the optimum and branch and bound silently returns a worse selection. `bnb_does_not_prune_the_optimum_at_btc_scale` is a case found by search where it does exactly that, returning 1261 against a true optimum of 1258. Make both `Option`. `scale` is now kept as an exact `num/den` fraction and only multiplied out, so the bound never round-trips through a division; the `max_weight` prune compares cross-multiplied rather than in floats. The credit the bound gives itself for a change output a descendant might add now uses `waste_floor`, rounded down, so it can never exceed what a descendant would really pay. Comparisons between branches are exact integer comparisons. Verified against exhaustive search: the full proptest suite at 4000 cases, including `ensure_bound_is_not_too_tight`, which checks every branch's bound against the actual score of every descendant. BREAKING: `BnbMetric::score`/`bound` return `Option`; `CoinSelector::run_bnb` returns `(u64, Drain)` and `bnb_solutions` yields `(CoinSelector, u64)`. Co-Authored-By: Claude Opus 5 --- src/bnb.rs | 14 ++--- src/coin_selector.rs | 4 +- src/drain.rs | 17 +++++ src/feerate.rs | 2 +- src/metrics/changeless.rs | 6 +- src/metrics/lowest_fee.rs | 127 ++++++++++++++++++++++++-------------- tests/bnb.rs | 16 +++-- tests/common.rs | 10 +-- tests/lowest_fee.rs | 64 +++++++++++++++++++ 9 files changed, 186 insertions(+), 74 deletions(-) diff --git a/src/bnb.rs b/src/bnb.rs index 0498e5c..61e45a6 100644 --- a/src/bnb.rs +++ b/src/bnb.rs @@ -1,6 +1,6 @@ use core::cmp::Reverse; -use crate::{float::Ordf32, Drain, Target}; +use crate::{Drain, Target}; use super::CoinSelector; use alloc::collections::BinaryHeap; @@ -10,7 +10,7 @@ use alloc::collections::BinaryHeap; #[derive(Debug)] pub(crate) struct BnbIter<'a, M: BnbMetric> { queue: BinaryHeap>, - best: Option, + best: Option, /// The target the metric scores selections against. pub(crate) target: Target, /// The `BnBMetric` that will score each selection @@ -18,7 +18,7 @@ pub(crate) struct BnbIter<'a, M: BnbMetric> { } impl<'a, M: BnbMetric> Iterator for BnbIter<'a, M> { - type Item = Option<(CoinSelector<'a>, Ordf32)>; + type Item = Option<(CoinSelector<'a>, u64)>; fn next(&mut self) -> Option { // { @@ -165,7 +165,7 @@ impl<'a, M: BnbMetric> BnbIter<'a, M> { #[derive(Debug, Clone)] struct Branch<'a> { - lower_bound: Ordf32, + lower_bound: u64, selector: CoinSelector<'a>, is_exclusion: bool, } @@ -198,14 +198,14 @@ impl PartialEq for Branch<'_> { impl Eq for Branch<'_> {} -/// A branch and bound metric where we minimize the [`Ordf32`] score. +/// A branch and bound metric where we minimize a score measured in whole satoshis. /// /// 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 for `target`. /// /// If this returns `None`, the selection is invalid. - fn score(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option; + fn score(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option; /// Get the lower bound score using a heuristic for `target`. /// @@ -214,7 +214,7 @@ pub trait BnbMetric { /// /// 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; + fn bound(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option; /// 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. diff --git a/src/coin_selector.rs b/src/coin_selector.rs index c90a37e..a19d6da 100644 --- a/src/coin_selector.rs +++ b/src/coin_selector.rs @@ -688,7 +688,7 @@ impl<'a> CoinSelector<'a> { &self, target: Target, metric: M, - ) -> impl Iterator, Ordf32)>> { + ) -> impl Iterator, u64)>> { crate::bnb::BnbIter::new(self.clone(), target, metric) } @@ -704,7 +704,7 @@ impl<'a> CoinSelector<'a> { target: Target, metric: M, max_rounds: usize, - ) -> Result<(Ordf32, Drain), NoBnbSolution> { + ) -> Result<(u64, Drain), NoBnbSolution> { let mut iter = crate::bnb::BnbIter::new(self.clone(), target, metric); let mut rounds = 0_usize; let best = iter diff --git a/src/drain.rs b/src/drain.rs index 309a02e..a98e152 100644 --- a/src/drain.rs +++ b/src/drain.rs @@ -57,6 +57,23 @@ impl DrainWeights { + self.spend_weight as f32 * long_term_feerate.spwu() } + /// Exact-integer [`waste`](Self::waste) rounded **down**. + /// + /// This feeds `LowestFee`'s lower bound, which stays admissible only while every credit it + /// gives is an under-estimate of the real cost — so this must never overstate. The real cost + /// rounds each fee component up, so flooring the exact sum is always at or below it. + pub(crate) fn waste_floor( + &self, + feerate: FeeRate, + long_term_feerate: FeeRate, + n_target_outputs: usize, + ) -> u64 { + let scaled = self.added_output_weight(n_target_outputs) as u128 + * feerate.sat_per_kvb() as u128 + + self.spend_weight as u128 * long_term_feerate.sat_per_kvb() as u128; + (scaled / crate::feerate::WU_PER_KVB) as u64 + } + /// Exact-integer counterpart of [`waste`](Self::waste): the satoshis it costs to add this /// drain, rounding each fee component up. fn waste_ceil( diff --git a/src/feerate.rs b/src/feerate.rs index 38b1846..b906b77 100644 --- a/src/feerate.rs +++ b/src/feerate.rs @@ -17,7 +17,7 @@ use core::ops::{Add, Sub}; pub struct FeeRate(u64); /// Weight units per 1000 vbytes. -const WU_PER_KVB: u128 = 4000; +pub(crate) const WU_PER_KVB: u128 = 4000; /// Vbytes per 1000 vbytes. const VB_PER_KVB: u128 = 1000; diff --git a/src/metrics/changeless.rs b/src/metrics/changeless.rs index a9c9e32..26dacd3 100644 --- a/src/metrics/changeless.rs +++ b/src/metrics/changeless.rs @@ -1,4 +1,4 @@ -use crate::{bnb::BnbMetric, float::Ordf32, CoinSelector, Drain, Target}; +use crate::{bnb::BnbMetric, CoinSelector, Drain, Target}; /// Constrains an `inner` metric to only changeless solutions. /// @@ -49,7 +49,7 @@ impl BnbMetric for Changeless { Drain::NONE } - fn score(&mut self, cs: &CoinSelector<'_>, target: Target) -> 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. // @@ -62,7 +62,7 @@ impl BnbMetric for Changeless { self.0.score(cs, target) } - fn bound(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { + 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 diff --git a/src/metrics/lowest_fee.rs b/src/metrics/lowest_fee.rs index 51b3f1e..31f9df0 100644 --- a/src/metrics/lowest_fee.rs +++ b/src/metrics/lowest_fee.rs @@ -1,4 +1,6 @@ -use crate::{float::Ordf32, BnbMetric, CoinSelector, Drain, DrainWeights, FeeRate, Target}; +use crate::{ + feerate::WU_PER_KVB, BnbMetric, Candidate, CoinSelector, Drain, DrainWeights, FeeRate, Target, +}; /// Metric that aims to minimize transaction fees. The future fee for spending the change output is /// included in this calculation. @@ -71,7 +73,7 @@ impl LowestFee { /// inside [`bound`](BnbMetric::bound): deferring the changeless rejection only loosens the lower /// bound and never makes it inadmissible, and `score` reuses the returned drain for its cap /// check so the drain is decided once. - fn fee_score(&self, cs: &CoinSelector<'_>, target: Target) -> Option<(Ordf32, Drain)> { + fn fee_score(&self, cs: &CoinSelector<'_>, target: Target) -> Option<(u64, Drain)> { if !cs.is_funded(target) { return None; } @@ -88,13 +90,30 @@ impl LowestFee { fee_for_the_tx ); let fee_for_spending_drain = drain.weights.spend_fee(self.long_term_feerate); - Some(( - Ordf32((fee_for_the_tx as u64 + fee_for_spending_drain) as f32), - drain, - )) + Some((fee_for_the_tx as u64 + fee_for_spending_drain, drain)) } } +/// Raise `scale` to `num / den` if that fraction is larger. +/// +/// `scale` is kept as an exact fraction rather than a float because it is multiplied back up by a +/// candidate value to produce the bound; a float round-trip through the division is exactly what +/// used to make that bound wrong. Comparing is a cross-multiplication, which stays inside `u128` +/// for any weight and satoshi value Bitcoin can represent. +fn raise_scale(scale: &mut (u128, u128), num: u128, den: u128) { + if num * scale.1 > scale.0 * den { + *scale = (num, den); + } +} + +/// `candidate.effective_value(feerate)`, scaled by [`WU_PER_KVB`] so it stays an exact integer. +/// +/// Negative when the candidate costs more to spend than it is worth. +fn effective_value_scaled(candidate: &Candidate, feerate: FeeRate) -> i128 { + candidate.value as i128 * WU_PER_KVB as i128 + - candidate.weight as i128 * feerate.sat_per_kvb() as i128 +} + impl BnbMetric for LowestFee { fn drain(&mut self, cs: &CoinSelector<'_>, target: Target) -> Drain { self.drain_value(cs, target) @@ -104,7 +123,7 @@ impl BnbMetric for LowestFee { }) } - fn score(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { + fn score(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { let (score, drain) = self.fee_score(cs, target)?; // A final selection must fit the weight cap. `drain_value` already refuses an over-cap // change, but a changeless selection can still be too heavy on its own. Reuse the drain @@ -115,7 +134,7 @@ impl BnbMetric for LowestFee { Some(score) } - fn bound(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { + fn bound(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { // Weight hard-prune: input weight only grows as this branch is extended, so the lightest // solution in the subtree is this selection with no drain. If even that busts `max_weight`, // the whole subtree is infeasible -> prune. (Also keeps `fee_score(cs).unwrap()` below @@ -151,15 +170,20 @@ impl BnbMetric for LowestFee { // 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( + // Rounded down, so the credit this bound gives itself is never larger than the + // cost a real descendant would pay -- otherwise the bound could exceed the true + // optimum and prune it. + let cost_of_adding_change = self.drain_weights.waste_floor( target.fee.rate, self.long_term_feerate, target.outputs.n_outputs, ); - let cost_of_no_change = cs.excess(target, Drain::NONE); + // Non-negative: we're in the `is_funded` branch. + let cost_of_no_change = cs.excess(target, Drain::NONE) as u64; - let best_score_with_change = - Ordf32(current_score.0 - cost_of_no_change as f32 + cost_of_adding_change); + let best_score_with_change = current_score + .saturating_sub(cost_of_no_change) + .saturating_add(cost_of_adding_change); // max_weight-aware: realizing that improvement requires a change output AND at // least one more input to lift the excess over the dust/worthwhile threshold, both // of which only make the tx heavier. If there's no room for both under the cap the @@ -208,16 +232,25 @@ 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(target, Drain::NONE) as f32; - let mut scale = Ordf32(0.0); + // `scale` is held as the exact fraction `num / den`. It gets multiplied back up by + // `to_resize.value` below, and doing that round trip through an `f32` division is what + // made this bound unreliable: the result is a small fee obtained by cancelling large + // satoshi amounts, so the rounding error could exceed the answer, in either direction. + // Erring low merely loosens the bound; erring high makes it inadmissible and prunes the + // optimum. + let mut scale = (0u128, 1u128); - if rate_excess < 0.0 { - let remaining_value_to_reach_feerate = rate_excess.abs(); - 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; - scale = scale.max(Ordf32(feerate_scale)); + let rate_excess = cs.rate_excess_wu(target, Drain::NONE); + if rate_excess < 0 { + let remaining_value_to_reach_feerate = rate_excess.unsigned_abs() as u128; + let effective_value_of_resized_input = + effective_value_scaled(&to_resize, target.fee.rate); + if effective_value_of_resized_input > 0 { + raise_scale( + &mut scale, + remaining_value_to_reach_feerate * WU_PER_KVB, + effective_value_of_resized_input as u128, + ); } else { return None; // we can never satisfy the constraint } @@ -226,15 +259,17 @@ 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) = 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 replace_excess = cs.replacement_excess_wu(target, Drain::NONE); + if replace_excess < 0 { + let remaining_value_to_reach_feerate = replace_excess.unsigned_abs() as u128; let effective_value_of_resized_input = - to_resize.effective_value(replace.incremental_relay_feerate); - if effective_value_of_resized_input > 0.0 { - let replace_scale = - remaining_value_to_reach_feerate / effective_value_of_resized_input; - scale = scale.max(Ordf32(replace_scale)); + effective_value_scaled(&to_resize, replace.incremental_relay_feerate); + if effective_value_of_resized_input > 0 { + raise_scale( + &mut scale, + remaining_value_to_reach_feerate * WU_PER_KVB, + effective_value_of_resized_input as u128, + ); } else { return None; // we can never satisfy the constraint } @@ -243,12 +278,11 @@ 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(target, Drain::NONE) as f32; - if absolute_excess < 0.0 { - let remaining = absolute_excess.abs(); + let absolute_excess = cs.absolute_excess(target, Drain::NONE); + if absolute_excess < 0 { + let remaining = absolute_excess.unsigned_abs() as u128; if to_resize.value > 0 { - let absolute_scale = remaining / to_resize.value as f32; - scale = scale.max(Ordf32(absolute_scale)); + raise_scale(&mut scale, remaining, to_resize.value as u128); } else { return None; // we can never satisfy the constraint } @@ -259,11 +293,11 @@ impl BnbMetric for LowestFee { // so if the current weight plus even that (fractional) minimum already busts the cap, // no within-cap selection down this branch reaches the target -> prune. This is the // fractional relaxation, so it never prunes a branch with an (integer) within-cap - // solution. + // solution. Multiplied out by `scale.1` so the comparison stays exact. if let Some(max_weight) = target.max_weight { - if cs.weight(target.outputs, DrainWeights::NONE) as f32 - + scale.0 * to_resize.weight as f32 - > max_weight as f32 + let weight = cs.weight(target.outputs, DrainWeights::NONE) as u128; + if weight * scale.1 + scale.0 * to_resize.weight as u128 + > max_weight as u128 * scale.1 { return None; } @@ -272,16 +306,15 @@ impl BnbMetric for LowestFee { // `scale` could be 0 even if `is_funded` is `false` due to the latter being based on // rounded-up vbytes. // - // Computed in `f64` and rounded *down*. This is a small fee obtained by cancelling - // large sat amounts, so in `f32` the error can exceed the result itself — and in either - // direction. Erring low only loosens the bound, but erring high makes it inadmissible - // and prunes the optimum. Every real score is a whole number of sats, so flooring is - // admissible and absorbs what error `f64` leaves. (`as i64` truncates toward zero, - // which is `floor` for the non-negative case; `core` has no `f64::floor`.) - let ideal_fee = (scale.0 as f64 * to_resize.value as f64 + cs.selected_value() as f64 - - target.value() as f64) as i64; + // Rounded down: the true best score in this subtree is a whole number of satoshis no + // smaller than this (fractional) ideal, so flooring keeps the bound admissible. + // `selected_value` and `target.value()` are whole satoshis, so they can be added after + // the division without affecting where it floors. + let ideal_fee = (scale.0 * to_resize.value as u128 / scale.1) as i128 + + cs.selected_value() as i128 + - target.value() as i128; - Some(Ordf32(ideal_fee.max(0) as f32)) + Some(ideal_fee.max(0) as u64) } } diff --git a/tests/bnb.rs b/tests/bnb.rs index 45a22dc..d427f13 100644 --- a/tests/bnb.rs +++ b/tests/bnb.rs @@ -1,6 +1,6 @@ mod common; use bdk_coin_select::{ - float::Ordf32, BnbMetric, Candidate, CoinSelector, Drain, Target, TargetFee, TargetOutputs, + BnbMetric, Candidate, CoinSelector, Drain, Target, TargetFee, TargetOutputs, }; #[macro_use] extern crate alloc; @@ -28,25 +28,23 @@ fn test_wv(mut rng: impl RngCore) -> impl Iterator { /// This is just an exhaustive search struct MinExcessThenWeight; -/// Assumes tx weight is less than 1MB. -const EXCESS_RATIO: f32 = 1_000_000_f32; +/// Assumes tx weight is less than 1MB, so weight never carries into the excess digits. +const EXCESS_RATIO: u64 = 1_000_000; impl BnbMetric for MinExcessThenWeight { - fn score(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { + fn score(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { let excess = cs.excess(target, Drain::NONE); if excess < 0 { None } else { - Some(Ordf32( - excess as f32 * EXCESS_RATIO + cs.input_weight() as f32, - )) + Some((excess as u64).saturating_mul(EXCESS_RATIO) + cs.input_weight()) } } - fn bound(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { + fn bound(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option { let mut cs = cs.clone(); cs.select_until_target_met(target).ok()?; - Some(Ordf32(cs.input_weight() as f32)) + Some(cs.input_weight()) } fn drain(&mut self, _cs: &CoinSelector<'_>, _target: Target) -> Drain { diff --git a/tests/common.rs b/tests/common.rs index 273b830..cb8fb11 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -344,7 +344,7 @@ pub fn exhaustive_search( cs: &mut CoinSelector, target: Target, metric: &mut M, -) -> Option<(Ordf32, usize)> +) -> Option<(u64, usize)> where M: BnbMetric, { @@ -352,7 +352,7 @@ where cs.sort_candidates_by_descending_value_pwu(); } - let mut best = Option::<(CoinSelector, Ordf32)>::None; + let mut best = Option::<(CoinSelector, u64)>::None; let mut rounds = 0; let iter = ExhaustiveIter::new(cs)? @@ -404,7 +404,7 @@ pub fn bnb_search( target: Target, metric: M, max_rounds: usize, -) -> Result<(Ordf32, usize), NoBnbSolution> +) -> Result<(u64, usize), NoBnbSolution> where M: BnbMetric, { @@ -422,7 +422,7 @@ where Ok((score, rounds)) } -pub fn result_string(res: &Result<(Ordf32, usize), E>, change: Drain) -> String +pub fn result_string(res: &Result<(u64, usize), E>, change: Drain) -> String where E: std::fmt::Debug, { @@ -543,7 +543,7 @@ fn randomly_satisfy_target<'a, R: rand::Rng>( ) -> CoinSelector<'a> { let mut cs = cs.clone(); - let mut last_score: Option = None; + let mut last_score: Option = None; while let Some(next) = cs.unselected_indices().choose(rng) { cs.select(next); if cs.is_funded(target) { diff --git a/tests/lowest_fee.rs b/tests/lowest_fee.rs index 791b21f..5fd94ab 100644 --- a/tests/lowest_fee.rs +++ b/tests/lowest_fee.rs @@ -516,3 +516,67 @@ fn bound_does_not_panic_on_f32_rounding() { // solution rather than merely not panicking. assert!(cs.run_bnb(target, metric, 100_000).is_ok()); } + +/// At Bitcoin-scale amounts the old `f32` bound cancelled ~0.3 BTC quantities down to a fee of a +/// few hundred sats, so its rounding error could exceed the result and land *above* the true +/// optimum. `bnb.rs` keeps a branch only when `best > bound`, so an inflated bound prunes the +/// optimum outright — silently returning a worse selection rather than failing. +/// +/// These candidates have near-tied subset sums (a shared ~34.2M sat base plus small deltas), which +/// is where a few sats of bound inflation is decisive. With the float bound, branch and bound +/// returned 1261 here while the true optimum is 1258. +#[test] +fn bnb_does_not_prune_the_optimum_at_btc_scale() { + let values_and_weights = [ + (34_236_824_u64, 597_u64), + (34_236_835, 460), + (34_236_856, 504), + (34_236_827, 523), + (34_236_835, 578), + (34_236_886, 504), + (34_236_860, 555), + (34_236_880, 434), + (34_236_886, 412), + (34_236_858, 404), + ]; + let candidates = values_and_weights + .iter() + .map(|&(value, weight)| Candidate { + value, + weight, + input_count: 1, + is_segwit: true, + }) + .collect::>(); + + let target = Target { + fee: TargetFee::from_feerate(FeeRate::from_sat_per_vb(2.0)), + outputs: TargetOutputs { + value_sum: 136_946_115, + weight_sum: 500, + n_outputs: 2, + }, + max_weight: None, + }; + let metric = LowestFee { + long_term_feerate: FeeRate::from_sat_per_vb(8.0), + dust_relay_feerate: FeeRate::from_sat_per_vb(3.0), + drain_weights: DrainWeights::TR_KEYSPEND, + }; + + let mut exhaustive_cs = CoinSelector::new(&candidates); + let (best_score, _) = + common::exhaustive_search(&mut exhaustive_cs, target, &mut metric.clone()) + .expect("exhaustive search must find a solution"); + assert_eq!(best_score, 1258, "the true optimum for this selection"); + + let mut cs = CoinSelector::new(&candidates); + let (bnb_score, _) = cs + .run_bnb(target, metric, 500_000) + .expect("bnb must find a solution"); + + assert_eq!( + bnb_score, best_score, + "bnb pruned the optimum: an inflated lower bound discarded the branch containing it" + ); +}