From 73600522abd9497eb4950261c2151b4cf91c413c Mon Sep 17 00:00:00 2001
From: LLFourn <lloyd.fourn@gmail.com>
Date: Fri, 22 Dec 2023 14:12:02 +1100
Subject: [PATCH] Write lowest fee test that hits important branch

That proptest is not hitting for some reason.
---
 src/metrics/lowest_fee.rs |  4 +-
 tests/lowest_fee.rs       | 84 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 85 insertions(+), 3 deletions(-)

diff --git a/src/metrics/lowest_fee.rs b/src/metrics/lowest_fee.rs
index 4f1cf2f..18fc7ad 100644
--- a/src/metrics/lowest_fee.rs
+++ b/src/metrics/lowest_fee.rs
@@ -67,7 +67,7 @@ impl BnbMetric for LowestFee {
 
                 if let Some((_, low_sats_per_wu_candidate)) = cs.unselected().next_back() {
                     let ev = low_sats_per_wu_candidate.effective_value(self.target.feerate);
-                    if ev < 0.0 {
+                    if ev < -0.0 {
                         // we can only reduce excess if ev is negative
                         let value_per_negative_effective_value =
                             low_sats_per_wu_candidate.value as f32 / ev.abs();
@@ -85,7 +85,7 @@ impl BnbMetric for LowestFee {
                             .drain_weights
                             .waste(self.target.feerate, self.long_term_feerate);
                         let best_score_without_change = Ordf32(
-                            current_score.0 - cost_of_change + cost_of_getting_rid_of_change,
+                            current_score.0 + cost_of_getting_rid_of_change - cost_of_change,
                         );
                         if best_score_without_change < current_score {
                             return Some(best_score_without_change);
diff --git a/tests/lowest_fee.rs b/tests/lowest_fee.rs
index 0c4102b..1ac0c7d 100644
--- a/tests/lowest_fee.rs
+++ b/tests/lowest_fee.rs
@@ -2,8 +2,8 @@
 
 mod common;
 use bdk_coin_select::metrics::{Changeless, LowestFee};
-use bdk_coin_select::ChangePolicy;
 use bdk_coin_select::{BnbMetric, Candidate, CoinSelector};
+use bdk_coin_select::{ChangePolicy, Drain, DrainWeights, FeeRate, Target};
 use proptest::prelude::*;
 
 proptest! {
@@ -160,3 +160,85 @@ fn combined_changeless_metric() {
 
     assert!(combined_rounds >= rounds);
 }
+
+/// This test considers the case where you could actually lower your long term fee by adding another input.
+#[test]
+fn adding_another_input_to_remove_change() {
+    let target = Target {
+        feerate: FeeRate::from_sat_per_vb(1.0),
+        min_fee: 0,
+        value: 99_870,
+    };
+
+    let candidates = vec![
+        Candidate {
+            value: 100_000,
+            weight: 100,
+            input_count: 1,
+            is_segwit: true,
+        },
+        Candidate {
+            value: 50_000,
+            weight: 100,
+            input_count: 1,
+            is_segwit: true,
+        },
+        // NOTE: this input has negative effective value
+        Candidate {
+            value: 10,
+            weight: 100,
+            input_count: 1,
+            is_segwit: true,
+        },
+    ];
+
+    let mut cs = CoinSelector::new(&candidates, 200);
+
+    let best_solution = {
+        let mut cs = cs.clone();
+        cs.select(0);
+        cs.select(2);
+        cs.excess(target, Drain::none());
+        assert!(cs.is_target_met(target));
+        cs
+    };
+
+    let drain_weights = DrainWeights {
+        output_weight: 100,
+        spend_weight: 1_000,
+    };
+
+    let excess_to_make_first_candidate_satisfy_but_have_change = {
+        let mut cs = cs.clone();
+        cs.select(0);
+        assert!(cs.is_target_met(target));
+        let with_change_excess = cs.excess(
+            target,
+            Drain {
+                value: 0,
+                weights: drain_weights,
+            },
+        );
+        assert!(with_change_excess > 0);
+        with_change_excess as u64
+    };
+
+    let change_policy = ChangePolicy {
+        min_value: excess_to_make_first_candidate_satisfy_but_have_change - 10,
+        drain_weights,
+    };
+
+    let mut metric = LowestFee {
+        target,
+        long_term_feerate: FeeRate::from_sat_per_vb(1.0),
+        change_policy,
+    };
+
+    let (score, _) = common::bnb_search(&mut cs, metric, 10).expect("finds solution");
+    let best_solution_score = metric.score(&best_solution).expect("must be a solution");
+
+    assert_eq!(best_solution.drain(target, change_policy), Drain::none());
+
+    assert!(score <= best_solution_score);
+    assert_eq!(cs.selected_indices(), best_solution.selected_indices());
+}