Make fee math exact: integer FeeRate and integer BnB scores

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,15 +10,15 @@ use alloc::collections::BinaryHeap;
 #[derive(Debug)]
 pub(crate) struct BnbIter<'a, M: BnbMetric> {
     queue: BinaryHeap<Branch<'a>>,
-    best: Option<Ordf32>,
+    best: Option<u64>,
     /// The target the metric scores selections against.
     pub(crate) target: Target,
     /// The `BnBMetric` that will score each selection
     pub(crate) metric: M,
 }
 
 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<Self::Item> {
         // {
@@ -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<Ordf32>;
+    fn score(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option<u64>;
 
     /// 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<Ordf32>;
+    fn bound(&mut self, cs: &CoinSelector<'_>, target: Target) -> Option<u64>;
 
     /// 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.

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.
@@ -690,7 +688,7 @@ impl<'a> CoinSelector<'a> {
         &self,
         target: Target,
         metric: M,
-    ) -> impl Iterator<Item = Option<(CoinSelector<'a>, Ordf32)>> {
+    ) -> impl Iterator<Item = Option<(CoinSelector<'a>, u64)>> {
         crate::bnb::BnbIter::new(self.clone(), target, metric)
     }
 
@@ -706,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

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,28 +30,65 @@ 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,
         feerate: FeeRate,
         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 [`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(
+        &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 +166,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,