Merge pull request #30 from jp1ac4/fix-lowestfee-bound

Loosen LowestFee metric bound

Author: LLFourn

src/coin_selector.rs

@@ -200,6 +200,15 @@ impl<'a> CoinSelector<'a> {
             - self.implied_fee_from_feerate(target, drain.weights) as i64
     }
 
+    /// Same as [rate_excess](Self::rate_excess) except `target.fee.rate` is applied to the
+    /// implied transaction's weight units directly without any conversion to vbytes.
+    pub fn rate_excess_wu(&self, target: Target, drain: Drain) -> i64 {
+        self.selected_value() as i64
+            - target.value() as i64
+            - drain.value as i64
+            - self.implied_fee_from_feerate_wu(target, drain.weights) as i64
+    }
+
     /// How much the current selection overshoots the value needed to satisfy RBF's rule 4.
     pub fn replacement_excess(&self, target: Target, drain: Drain) -> i64 {
         let mut replacement_excess_needed = 0;
@@ -213,6 +222,20 @@ impl<'a> CoinSelector<'a> {
             - replacement_excess_needed as i64
     }
 
+    /// Same as [replacement_excess](Self::replacement_excess) except the replacement fee
+    /// is calculated using weight units directly without any conversion to vbytes.
+    pub fn replacement_excess_wu(&self, target: Target, drain: Drain) -> i64 {
+        let mut replacement_excess_needed = 0;
+        if let Some(replace) = target.fee.replace {
+            replacement_excess_needed =
+                replace.min_fee_to_do_replacement_wu(self.weight(target.outputs, drain.weights))
+        }
+        self.selected_value() as i64
+            - target.value() as i64
+            - drain.value as i64
+            - replacement_excess_needed as i64
+    }
+
     /// The feerate the transaction would have if we were to use this selection of inputs to achieve
     /// the `target`'s value and weight. It is essentially telling you what target feerate you currently have.
     ///
@@ -253,6 +276,13 @@ impl<'a> CoinSelector<'a> {
             .implied_fee(self.weight(target.outputs, drain_weights))
     }
 
+    fn implied_fee_from_feerate_wu(&self, target: Target, drain_weights: DrainWeights) -> u64 {
+        target
+            .fee
+            .rate
+            .implied_fee_wu(self.weight(target.outputs, drain_weights))
+    }
+
     /// The actual fee the selection would pay if it was used in a transaction that had
     /// `target_value` value for outputs and change output of `drain_value`.
     ///

src/feerate.rs

@@ -79,11 +79,16 @@ impl FeeRate {
         self.0 .0
     }
 
-    /// The fee that the transaction with weight `tx_weight` should pay in order to satisfy the fee rate given by `self`.
+    /// 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 {
-        // The fee rate is applied to the rounded-up vbytes.
         ((tx_weight as f32 / 4.0).ceil() * self.as_sat_vb()).ceil() as u64
     }
+
+    /// 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
+    }
 }
 
 impl Add<FeeRate> for FeeRate {

src/metrics/lowest_fee.rs

@@ -115,6 +115,10 @@ impl BnbMetric for LowestFee {
                 .select_iter()
                 .find(|(cs, _, _)| cs.is_target_met(self.target))?;
 
+            // If this selection is already perfect, return its score directly.
+            if cs.excess(self.target, Drain::NONE) == 0 {
+                return Some(self.score(&cs).unwrap());
+            };
             cs.deselect(resize_index);
 
             // We need to find the minimum fee we'd pay if we satisfy the feerate constraint. We do
@@ -131,7 +135,10 @@ impl BnbMetric for LowestFee {
             // scale = remaining_value_to_reach_feerate / effective_value_of_resized_input
             //
             // This should be intutive since we're finding out how to scale the input we're resizing to get the effective value we need.
-            let rate_excess = cs.rate_excess(self.target, Drain::NONE) as f32;
+            //
+            // 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(self.target, Drain::NONE) as f32;
             let mut scale = Ordf32(0.0);
 
             if rate_excess < 0.0 {
@@ -150,7 +157,7 @@ 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) = self.target.fee.replace {
-                let replace_excess = cs.replacement_excess(self.target, Drain::NONE) as f32;
+                let replace_excess = cs.replacement_excess_wu(self.target, Drain::NONE) as f32;
                 if replace_excess < 0.0 {
                     let remaining_value_to_reach_feerate = replace_excess.abs();
                     let effective_value_of_resized_input =
@@ -164,8 +171,8 @@ impl BnbMetric for LowestFee {
                     }
                 }
             }
-
-            assert!(scale.0 > 0.0);
+            // `scale` could be 0 even if `is_target_met` 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
                 - self.target.value() as f32;
             assert!(ideal_fee >= 0.0);

src/target.rs

@@ -141,4 +141,13 @@ impl Replace {
                 .incremental_relay_feerate
                 .implied_fee(replacing_tx_weight)
     }
+
+    /// Same as (min_fee_to_do_replacement)[Self::min_fee_to_do_replacement] except the additional fee
+    /// is calculated using `replacing_tx_weight` directly without any conversion to vbytes.
+    pub fn min_fee_to_do_replacement_wu(&self, replacing_tx_weight: u64) -> u64 {
+        self.fee
+            + self
+                .incremental_relay_feerate
+                .implied_fee_wu(replacing_tx_weight)
+    }
 }

tests/lowest_fee.proptest-regressions

@@ -10,3 +10,4 @@ cc c580ee452624915fc710d5fe724c7a9347472ccd178f66c9db9479cfc6168f48 # shrinks to
 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
 cc ca9831dfe4ad27fc705ae4af201b9b50739404bda0e1e130072858634f8d25d9 # added by llfourn from ci: has a case where the lowest fee metric would benefit by adding change
 cc 85d9dc968a690553484d0f166f3d778c3e0ec7d9059809c2818b62d6609853c1
+cc b32bb279f62e2300a14c9053cff09f6a953bf1020b878958cb510258ffe65028 # added by jp1ac4 from ci