From 9773192b833fe0d0e071b0a75f72aab82cb124ef Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Fri, 1 Mar 2024 14:29:13 -0500
Subject: [PATCH 01/14] test: Create coinselection_tests

Adds a Test Suite, default coin selection parameters as well as helper
functions for creating available coins and to check results.
---
 src/wallet/test/coinselection_tests.cpp | 84 +++++++++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100644 src/wallet/test/coinselection_tests.cpp

diff --git a/src/wallet/test/coinselection_tests.cpp b/src/wallet/test/coinselection_tests.cpp
new file mode 100644
index 00000000000..46a0bce4fb5
--- /dev/null
+++ b/src/wallet/test/coinselection_tests.cpp
@@ -0,0 +1,84 @@
+// Copyright (c) 2024 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <consensus/amount.h>
+#include <wallet/coinselection.h>
+#include <wallet/test/wallet_test_fixture.h>
+
+#include <boost/test/unit_test.hpp>
+
+namespace wallet {
+BOOST_FIXTURE_TEST_SUITE(coinselection_tests, TestingSetup)
+
+static int next_lock_time = 0;
+static FastRandomContext default_rand;
+
+/** Default coin selection parameters (dcsp) allow us to only explicitly set
+ * parameters when a diverging value is relevant in the context of a test.
+ * We use P2WPKH input and output weights for the change weights. */
+static CoinSelectionParams init_default_params()
+{
+    CoinSelectionParams dcsp{
+        /*rng_fast*/default_rand,
+        /*change_output_size=*/31,
+        /*change_spend_size=*/68,
+        /*min_change_target=*/50'000,
+        /*effective_feerate=*/CFeeRate(5000),
+        /*long_term_feerate=*/CFeeRate(10'000),
+        /*discard_feerate=*/CFeeRate(3000),
+        /*tx_noinputs_size=*/11 + 31, //static header size + output size
+        /*avoid_partial=*/false,
+    };
+    dcsp.m_change_fee = /*155 sats=*/dcsp.m_effective_feerate.GetFee(dcsp.change_output_size);
+    dcsp.min_viable_change = /*204 sats=*/dcsp.m_discard_feerate.GetFee(dcsp.change_spend_size);
+    dcsp.m_cost_of_change = /*204 + 155 sats=*/dcsp.min_viable_change + dcsp.m_change_fee;
+    dcsp.m_subtract_fee_outputs = false;
+    return dcsp;
+}
+
+static const CoinSelectionParams default_cs_params = init_default_params();
+
+/** Make one OutputGroup with a single UTXO that either has a given effective value (default) or a given amount (`is_eff_value = false`). */
+static OutputGroup MakeCoin(const CAmount& amount, bool is_eff_value = true, CoinSelectionParams cs_params = default_cs_params, int custom_spending_vsize = 68)
+{
+    // Always assume that we only have one input
+    CMutableTransaction tx;
+    tx.vout.resize(1);
+    CAmount fees = cs_params.m_effective_feerate.GetFee(custom_spending_vsize);
+    tx.vout[0].nValue = amount + int(is_eff_value) * fees;
+    tx.nLockTime = next_lock_time++;        // so all transactions get different hashes
+    OutputGroup group(cs_params);
+    group.Insert(std::make_shared<COutput>(COutPoint(tx.GetHash(), 0), tx.vout.at(0), /*depth=*/1, /*input_bytes=*/custom_spending_vsize, /*spendable=*/true, /*solvable=*/true, /*safe=*/true, /*time=*/0, /*from_me=*/false, /*fees=*/fees), /*ancestors=*/0, /*descendants=*/0);
+    return group;
+}
+
+/** Make multiple OutputGroups with the given values as their effective value */
+static void AddCoins(std::vector<OutputGroup>& utxo_pool, std::vector<CAmount> coins, CoinSelectionParams cs_params = default_cs_params)
+{
+    for (CAmount c : coins) {
+        utxo_pool.push_back(MakeCoin(c, true, cs_params));
+    }
+}
+
+/** Check if SelectionResult a is equivalent to SelectionResult b.
+ * Two results are equivalent if they are composed of the same input values, even if they have different inputs (i.e., same value, different prevout) */
+static bool HaveEquivalentValues(const SelectionResult& a, const SelectionResult& b)
+{
+    std::vector<CAmount> a_amts;
+    std::vector<CAmount> b_amts;
+    for (const auto& coin : a.GetInputSet()) {
+        a_amts.push_back(coin->txout.nValue);
+    }
+    for (const auto& coin : b.GetInputSet()) {
+        b_amts.push_back(coin->txout.nValue);
+    }
+    std::sort(a_amts.begin(), a_amts.end());
+    std::sort(b_amts.begin(), b_amts.end());
+
+    auto ret = std::mismatch(a_amts.begin(), a_amts.end(), b_amts.begin());
+    return ret.first == a_amts.end() && ret.second == b_amts.end();
+}
+
+BOOST_AUTO_TEST_SUITE_END()
+} // namespace wallet

From 66200b3ffa21605fc3234ccbda7b424381f3319a Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Thu, 27 Jun 2024 13:31:20 -0400
Subject: [PATCH 02/14] test: Recreate simple BnB success tests

---
 src/wallet/test/CMakeLists.txt          |  1 +
 src/wallet/test/coinselection_tests.cpp | 43 +++++++++++++++++++++++
 src/wallet/test/coinselector_tests.cpp  | 45 -------------------------
 3 files changed, 44 insertions(+), 45 deletions(-)

diff --git a/src/wallet/test/CMakeLists.txt b/src/wallet/test/CMakeLists.txt
index f14a78ca1d2..8564eface55 100644
--- a/src/wallet/test/CMakeLists.txt
+++ b/src/wallet/test/CMakeLists.txt
@@ -10,6 +10,7 @@ target_sources(test_bitcoin
     wallet_test_fixture.cpp
     db_tests.cpp
     coinselector_tests.cpp
+    coinselection_tests.cpp
     feebumper_tests.cpp
     group_outputs_tests.cpp
     init_tests.cpp
diff --git a/src/wallet/test/coinselection_tests.cpp b/src/wallet/test/coinselection_tests.cpp
index 46a0bce4fb5..73bd49ca9c5 100644
--- a/src/wallet/test/coinselection_tests.cpp
+++ b/src/wallet/test/coinselection_tests.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <consensus/amount.h>
+#include <policy/policy.h>
 #include <wallet/coinselection.h>
 #include <wallet/test/wallet_test_fixture.h>
 
@@ -80,5 +81,47 @@ static bool HaveEquivalentValues(const SelectionResult& a, const SelectionResult
     return ret.first == a_amts.end() && ret.second == b_amts.end();
 }
 
+static std::string InputsToString(const SelectionResult& selection)
+{
+    std::string res = "[ ";
+    for (const auto& input : selection.GetInputSet()) {
+        res += util::ToString(input->txout.nValue);
+        res += " ";
+    }
+    return res + "]";
+}
+
+static void TestBnBSuccess(std::string test_title, std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const std::vector<CAmount>& expected_input_amounts, const CoinSelectionParams& cs_params = default_cs_params)
+{
+    SelectionResult expected_result(CAmount(0), SelectionAlgorithm::BNB);
+    CAmount expected_amount = 0;
+    for (CAmount input_amount : expected_input_amounts) {
+        OutputGroup group = MakeCoin(input_amount, true, cs_params);
+        expected_amount += group.m_value;
+        expected_result.AddInput(group);
+    }
+
+    const auto result = SelectCoinsBnB(utxo_pool, selection_target, /*cost_of_change=*/default_cs_params.m_cost_of_change, /*max_selection_weight=*/MAX_STANDARD_TX_WEIGHT);
+    BOOST_CHECK_MESSAGE(result, "Falsy result in BnB-Success: " + test_title);
+    BOOST_CHECK_MESSAGE(HaveEquivalentValues(expected_result, *result), strprintf("Result mismatch in BnB-Success: %s. Expected %s, but got %s", test_title, InputsToString(expected_result), InputsToString(*result)));
+    BOOST_CHECK_MESSAGE(result->GetSelectedValue() == expected_amount, strprintf("Selected amount mismatch in BnB-Success: %s. Expected %d, but got %d", test_title, expected_amount, result->GetSelectedValue()));
+}
+
+BOOST_AUTO_TEST_CASE(bnb_test)
+{
+    std::vector<OutputGroup> utxo_pool;
+    AddCoins(utxo_pool, {1 * CENT, 3 * CENT, 5 * CENT});
+
+    // Simple success cases
+    TestBnBSuccess("Select smallest UTXO", utxo_pool, /*selection_target=*/1 * CENT, /*expected_input_amounts=*/{1 * CENT});
+    TestBnBSuccess("Select middle UTXO", utxo_pool, /*selection_target=*/3 * CENT, /*expected_input_amounts=*/{3 * CENT});
+    TestBnBSuccess("Select biggest UTXO", utxo_pool, /*selection_target=*/5 * CENT, /*expected_input_amounts=*/{5 * CENT});
+    TestBnBSuccess("Select two UTXOs", utxo_pool, /*selection_target=*/4 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT});
+    TestBnBSuccess("Select all UTXOs", utxo_pool, /*selection_target=*/9 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT, 5 * CENT});
+
+    // BnB finds changeless solution while overshooting by up to cost_of_change
+    TestBnBSuccess("Select upper bound", utxo_pool, /*selection_target=*/4 * CENT - default_cs_params.m_cost_of_change, /*expected_input_amounts=*/{1 * CENT, 3 * CENT});
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 } // namespace wallet
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index eb9c349c22e..f9efd052612 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -208,59 +208,14 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
     add_coin(3 * CENT, 3, utxo_pool);
     add_coin(4 * CENT, 4, utxo_pool);
 
-    // Select 1 Cent
-    add_coin(1 * CENT, 1, expected_result);
-    const auto result1 = SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 0.5 * CENT);
-    BOOST_CHECK(result1);
-    BOOST_CHECK(EquivalentResult(expected_result, *result1));
-    BOOST_CHECK_EQUAL(result1->GetSelectedValue(), 1 * CENT);
-    expected_result.Clear();
-
-    // Select 2 Cent
-    add_coin(2 * CENT, 2, expected_result);
-    const auto result2 = SelectCoinsBnB(GroupCoins(utxo_pool), 2 * CENT, 0.5 * CENT);
-    BOOST_CHECK(result2);
-    BOOST_CHECK(EquivalentResult(expected_result, *result2));
-    BOOST_CHECK_EQUAL(result2->GetSelectedValue(), 2 * CENT);
-    expected_result.Clear();
-
-    // Select 5 Cent
-    add_coin(3 * CENT, 3, expected_result);
-    add_coin(2 * CENT, 2, expected_result);
-    const auto result3 = SelectCoinsBnB(GroupCoins(utxo_pool), 5 * CENT, 0.5 * CENT);
-    BOOST_CHECK(result3);
-    BOOST_CHECK(EquivalentResult(expected_result, *result3));
-    BOOST_CHECK_EQUAL(result3->GetSelectedValue(), 5 * CENT);
-    expected_result.Clear();
-
     // Select 11 Cent, not possible
     BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 11 * CENT, 0.5 * CENT));
     expected_result.Clear();
 
-    // Cost of change is greater than the difference between target value and utxo sum
-    add_coin(1 * CENT, 1, expected_result);
-    const auto result4 = SelectCoinsBnB(GroupCoins(utxo_pool), 0.9 * CENT, 0.5 * CENT);
-    BOOST_CHECK(result4);
-    BOOST_CHECK_EQUAL(result4->GetSelectedValue(), 1 * CENT);
-    BOOST_CHECK(EquivalentResult(expected_result, *result4));
-    expected_result.Clear();
-
     // Cost of change is less than the difference between target value and utxo sum
     BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.9 * CENT, 0));
     expected_result.Clear();
 
-    // Select 10 Cent
-    add_coin(5 * CENT, 5, utxo_pool);
-    add_coin(4 * CENT, 4, expected_result);
-    add_coin(3 * CENT, 3, expected_result);
-    add_coin(2 * CENT, 2, expected_result);
-    add_coin(1 * CENT, 1, expected_result);
-    const auto result5 = SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 0.5 * CENT);
-    BOOST_CHECK(result5);
-    BOOST_CHECK(EquivalentResult(expected_result, *result5));
-    BOOST_CHECK_EQUAL(result5->GetSelectedValue(), 10 * CENT);
-    expected_result.Clear();
-
     // Select 0.25 Cent, not possible
     BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.25 * CENT, 0.5 * CENT));
     expected_result.Clear();

From afd4b807ff1300e4f74ceab6a683f3ff1376369d Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Fri, 1 Mar 2024 15:06:50 -0500
Subject: [PATCH 03/14] test: Move BnB feerate sensitivity tests

Originally these tests verified that at a SelectCoins level that a
solution with fewer inputs gets preferred at high feerates, and a
solution with more inputs gets preferred at low feerates. This outcome
relies on the behavior of BnB, so we move these tests under the umbrella
of BnB tests.

Originally these tests relied on SFFO to work.
---
 src/wallet/test/coinselection_tests.cpp | 27 +++++++++++++++--
 src/wallet/test/coinselector_tests.cpp  | 39 ++-----------------------
 2 files changed, 28 insertions(+), 38 deletions(-)

diff --git a/src/wallet/test/coinselection_tests.cpp b/src/wallet/test/coinselection_tests.cpp
index 73bd49ca9c5..a4c8b13fa79 100644
--- a/src/wallet/test/coinselection_tests.cpp
+++ b/src/wallet/test/coinselection_tests.cpp
@@ -91,12 +91,12 @@ static std::string InputsToString(const SelectionResult& selection)
     return res + "]";
 }
 
-static void TestBnBSuccess(std::string test_title, std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const std::vector<CAmount>& expected_input_amounts, const CoinSelectionParams& cs_params = default_cs_params)
+static void TestBnBSuccess(std::string test_title, std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const std::vector<CAmount>& expected_input_amounts, const CoinSelectionParams& cs_params = default_cs_params, int custom_spending_vsize = 68)
 {
     SelectionResult expected_result(CAmount(0), SelectionAlgorithm::BNB);
     CAmount expected_amount = 0;
     for (CAmount input_amount : expected_input_amounts) {
-        OutputGroup group = MakeCoin(input_amount, true, cs_params);
+        OutputGroup group = MakeCoin(input_amount, true, cs_params, custom_spending_vsize);
         expected_amount += group.m_value;
         expected_result.AddInput(group);
     }
@@ -123,5 +123,28 @@ BOOST_AUTO_TEST_CASE(bnb_test)
     TestBnBSuccess("Select upper bound", utxo_pool, /*selection_target=*/4 * CENT - default_cs_params.m_cost_of_change, /*expected_input_amounts=*/{1 * CENT, 3 * CENT});
 }
 
+BOOST_AUTO_TEST_CASE(bnb_feerate_sensitivity_test)
+{
+    // Create sets of UTXOs with the same effective amounts at different feerates (but different absolute amounts)
+    std::vector<OutputGroup> low_feerate_pool; // 5 sat/vB (default, and lower than long_term_feerate of 10 sat/vB)
+    AddCoins(low_feerate_pool, {2 * CENT, 3 * CENT, 5 * CENT, 10 * CENT});
+    TestBnBSuccess("Select many inputs at low feerates", low_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{2 * CENT, 3 * CENT, 5 * CENT});
+
+    CoinSelectionParams high_feerate_params = init_default_params();
+    high_feerate_params.m_effective_feerate = CFeeRate{25'000};
+    std::vector<OutputGroup> high_feerate_pool; // 25 sat/vB (greater than long_term_feerate of 10 sat/vB)
+    AddCoins(high_feerate_pool, {2 * CENT, 3 * CENT, 5 * CENT, 10 * CENT}, high_feerate_params);
+    TestBnBSuccess("Select one input at high feerates", high_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{10 * CENT}, high_feerate_params);
+
+    // Add heavy inputs {6, 7} to existing {2, 3, 5, 10}
+    low_feerate_pool.push_back(MakeCoin(6 * CENT, true, default_cs_params, /*custom_spending_vsize=*/500));
+    low_feerate_pool.push_back(MakeCoin(7 * CENT, true, default_cs_params, /*custom_spending_vsize=*/500));
+    TestBnBSuccess("Prefer two heavy inputs over two light inputs at low feerates", low_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{6 * CENT, 7 * CENT}, default_cs_params, /*custom_spending_vsize=*/500);
+
+    high_feerate_pool.push_back(MakeCoin(6 * CENT, true, high_feerate_params, /*custom_spending_vsize=*/500));
+    high_feerate_pool.push_back(MakeCoin(7 * CENT, true, high_feerate_params, /*custom_spending_vsize=*/500));
+    TestBnBSuccess("Prefer two light inputs over two heavy inputs at high feerates", high_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{3 * CENT, 10 * CENT}, high_feerate_params);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 } // namespace wallet
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index f9efd052612..5e96c5979fb 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -321,7 +321,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
 
         CoinsResult available_coins;
 
-        // single coin should be selected when effective fee > long term fee
+        // pre selected coin should be selected even if disadvantageous
         coin_selection_params_bnb.m_effective_feerate = CFeeRate(5000);
         coin_selection_params_bnb.m_long_term_feerate = CFeeRate(3000);
 
@@ -332,42 +332,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         add_coin(available_coins, *wallet, 1 * CENT + input_fee, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
 
         expected_result.Clear();
-        add_coin(10 * CENT + input_fee, 2, expected_result);
+        add_coin(9 * CENT + input_fee, 2, expected_result);
+        add_coin(1 * CENT + input_fee, 2, expected_result);
         CCoinControl coin_control;
-        const auto result11 = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/{}, 10 * CENT, coin_control, coin_selection_params_bnb);
-        BOOST_CHECK(EquivalentResult(expected_result, *result11));
-        available_coins.Clear();
-
-        // more coins should be selected when effective fee < long term fee
-        coin_selection_params_bnb.m_effective_feerate = CFeeRate(3000);
-        coin_selection_params_bnb.m_long_term_feerate = CFeeRate(5000);
-
-        // Add selectable outputs, increasing their raw amounts by their input fee to make the effective value equal to the raw amount
-        input_fee = coin_selection_params_bnb.m_effective_feerate.GetFee(/*num_bytes=*/68); // bech32 input size (default test output type)
-        add_coin(available_coins, *wallet, 10 * CENT + input_fee, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
-        add_coin(available_coins, *wallet, 9 * CENT + input_fee, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
-        add_coin(available_coins, *wallet, 1 * CENT + input_fee, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
-
-        expected_result.Clear();
-        add_coin(9 * CENT + input_fee, 2, expected_result);
-        add_coin(1 * CENT + input_fee, 2, expected_result);
-        const auto result12 = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/{}, 10 * CENT, coin_control, coin_selection_params_bnb);
-        BOOST_CHECK(EquivalentResult(expected_result, *result12));
-        available_coins.Clear();
-
-        // pre selected coin should be selected even if disadvantageous
-        coin_selection_params_bnb.m_effective_feerate = CFeeRate(5000);
-        coin_selection_params_bnb.m_long_term_feerate = CFeeRate(3000);
-
-        // Add selectable outputs, increasing their raw amounts by their input fee to make the effective value equal to the raw amount
-        input_fee = coin_selection_params_bnb.m_effective_feerate.GetFee(/*num_bytes=*/68); // bech32 input size (default test output type)
-        add_coin(available_coins, *wallet, 10 * CENT + input_fee, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
-        add_coin(available_coins, *wallet, 9 * CENT + input_fee, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
-        add_coin(available_coins, *wallet, 1 * CENT + input_fee, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true);
-
-        expected_result.Clear();
-        add_coin(9 * CENT + input_fee, 2, expected_result);
-        add_coin(1 * CENT + input_fee, 2, expected_result);
         coin_control.m_allow_other_inputs = true;
         COutput select_coin = available_coins.All().at(1); // pre select 9 coin
         coin_control.Select(select_coin.outpoint);

From 9d7db26b7b556784c16e41572ba2d2edc6dd6c24 Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Thu, 27 Jun 2024 11:50:15 -0400
Subject: [PATCH 04/14] test: Recreate BnB clone skipping test

---
 src/wallet/test/coinselection_tests.cpp | 13 +++++++++++++
 src/wallet/test/coinselector_tests.cpp  | 20 --------------------
 2 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/src/wallet/test/coinselection_tests.cpp b/src/wallet/test/coinselection_tests.cpp
index a4c8b13fa79..1afcee4570d 100644
--- a/src/wallet/test/coinselection_tests.cpp
+++ b/src/wallet/test/coinselection_tests.cpp
@@ -62,6 +62,13 @@ static void AddCoins(std::vector<OutputGroup>& utxo_pool, std::vector<CAmount> c
     }
 }
 
+/** Make multiple coins that share the same effective value */
+static void AddDuplicateCoins(std::vector<OutputGroup>& utxo_pool, int count, int amount) {
+    for (int i = 0 ; i < count; ++i) {
+        utxo_pool.push_back(MakeCoin(amount));
+    }
+}
+
 /** Check if SelectionResult a is equivalent to SelectionResult b.
  * Two results are equivalent if they are composed of the same input values, even if they have different inputs (i.e., same value, different prevout) */
 static bool HaveEquivalentValues(const SelectionResult& a, const SelectionResult& b)
@@ -121,6 +128,12 @@ BOOST_AUTO_TEST_CASE(bnb_test)
 
     // BnB finds changeless solution while overshooting by up to cost_of_change
     TestBnBSuccess("Select upper bound", utxo_pool, /*selection_target=*/4 * CENT - default_cs_params.m_cost_of_change, /*expected_input_amounts=*/{1 * CENT, 3 * CENT});
+
+    // Test skipping of equivalent input sets
+    std::vector<OutputGroup> clone_pool;
+    AddCoins(clone_pool, {2 * CENT, 7 * CENT, 7 * CENT});
+    AddDuplicateCoins(clone_pool, 50'000, 5 * CENT);
+    TestBnBSuccess("Skip equivalent input sets", clone_pool, /*selection_target=*/16 * CENT, /*expected_input_amounts=*/{2 * CENT, 7 * CENT, 7 * CENT});
 }
 
 BOOST_AUTO_TEST_CASE(bnb_feerate_sensitivity_test)
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 5e96c5979fb..8d42f570bc2 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -227,26 +227,6 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
     const auto result7 = SelectCoinsBnB(GroupCoins(utxo_pool), target, 1); // Should not exhaust
     BOOST_CHECK(result7);
 
-    // Test same value early bailout optimization
-    utxo_pool.clear();
-    add_coin(7 * CENT, 7, expected_result);
-    add_coin(7 * CENT, 7, expected_result);
-    add_coin(7 * CENT, 7, expected_result);
-    add_coin(7 * CENT, 7, expected_result);
-    add_coin(2 * CENT, 7, expected_result);
-    add_coin(7 * CENT, 7, utxo_pool);
-    add_coin(7 * CENT, 7, utxo_pool);
-    add_coin(7 * CENT, 7, utxo_pool);
-    add_coin(7 * CENT, 7, utxo_pool);
-    add_coin(2 * CENT, 7, utxo_pool);
-    for (int i = 0; i < 50000; ++i) {
-        add_coin(5 * CENT, 7, utxo_pool);
-    }
-    const auto result8 = SelectCoinsBnB(GroupCoins(utxo_pool), 30 * CENT, 5000);
-    BOOST_CHECK(result8);
-    BOOST_CHECK_EQUAL(result8->GetSelectedValue(), 30 * CENT);
-    BOOST_CHECK(EquivalentResult(expected_result, *result8));
-
     ////////////////////
     // Behavior tests //
     ////////////////////

From 65521465da036616172f4fbeef2855b8ddefd75f Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Fri, 1 Mar 2024 15:41:19 -0500
Subject: [PATCH 05/14] test: Recreate simple BnB failure tests

---
 src/wallet/test/coinselection_tests.cpp | 17 +++++++++++++++++
 src/wallet/test/coinselector_tests.cpp  | 21 ---------------------
 2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/src/wallet/test/coinselection_tests.cpp b/src/wallet/test/coinselection_tests.cpp
index 1afcee4570d..19391a3920c 100644
--- a/src/wallet/test/coinselection_tests.cpp
+++ b/src/wallet/test/coinselection_tests.cpp
@@ -114,9 +114,18 @@ static void TestBnBSuccess(std::string test_title, std::vector<OutputGroup>& utx
     BOOST_CHECK_MESSAGE(result->GetSelectedValue() == expected_amount, strprintf("Selected amount mismatch in BnB-Success: %s. Expected %d, but got %d", test_title, expected_amount, result->GetSelectedValue()));
 }
 
+static void TestBnBFail(std::string test_title, std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target)
+{
+    BOOST_CHECK_MESSAGE(!SelectCoinsBnB(utxo_pool, selection_target, /*cost_of_change=*/default_cs_params.m_cost_of_change, /*max_selection_weight=*/MAX_STANDARD_TX_WEIGHT), "BnB-Fail: " + test_title);
+}
+
 BOOST_AUTO_TEST_CASE(bnb_test)
 {
     std::vector<OutputGroup> utxo_pool;
+
+    // Fail for empty UTXO pool
+    TestBnBFail("Empty UTXO pool", utxo_pool, /*selection_target=*/1 * CENT);
+
     AddCoins(utxo_pool, {1 * CENT, 3 * CENT, 5 * CENT});
 
     // Simple success cases
@@ -129,6 +138,14 @@ BOOST_AUTO_TEST_CASE(bnb_test)
     // BnB finds changeless solution while overshooting by up to cost_of_change
     TestBnBSuccess("Select upper bound", utxo_pool, /*selection_target=*/4 * CENT - default_cs_params.m_cost_of_change, /*expected_input_amounts=*/{1 * CENT, 3 * CENT});
 
+    // BnB fails to find changeless solution when overshooting by cost_of_change + 1 sat
+    TestBnBFail("Overshoot upper bound", utxo_pool, /*selection_target=*/4 * CENT - default_cs_params.m_cost_of_change - 1);
+
+    // Simple cases without BnB solution
+    TestBnBFail("Smallest combination too big", utxo_pool, /*selection_target=*/0.5 * CENT);
+    TestBnBFail("No UTXO combination in target window", utxo_pool, /*selection_target=*/7 * CENT);
+    TestBnBFail("Select more than available", utxo_pool, /*selection_target=*/10 * CENT);
+
     // Test skipping of equivalent input sets
     std::vector<OutputGroup> clone_pool;
     AddCoins(clone_pool, {2 * CENT, 7 * CENT, 7 * CENT});
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 8d42f570bc2..ea7e1745158 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -199,27 +199,6 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
     // Known Outcome tests //
     /////////////////////////
 
-    // Empty utxo pool
-    BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 0.5 * CENT));
-
-    // Add utxos
-    add_coin(1 * CENT, 1, utxo_pool);
-    add_coin(2 * CENT, 2, utxo_pool);
-    add_coin(3 * CENT, 3, utxo_pool);
-    add_coin(4 * CENT, 4, utxo_pool);
-
-    // Select 11 Cent, not possible
-    BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 11 * CENT, 0.5 * CENT));
-    expected_result.Clear();
-
-    // Cost of change is less than the difference between target value and utxo sum
-    BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.9 * CENT, 0));
-    expected_result.Clear();
-
-    // Select 0.25 Cent, not possible
-    BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.25 * CENT, 0.5 * CENT));
-    expected_result.Clear();
-
     // Iteration exhaustion test
     CAmount target = make_hard_case(17, utxo_pool);
     BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), target, 1)); // Should exhaust

From 28574e2c4ff28b785077c14cdda299003a74f38b Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Fri, 1 Mar 2024 15:42:08 -0500
Subject: [PATCH 06/14] test: Remove redundant repeated test

We do not need to repeat the same test multiple times because BnB is
deterministic and will therefore always have the same outcome.
Additionally, this test was redundant because it repeats the "Smallest
combination too big" test.
---
 src/wallet/test/coinselector_tests.cpp | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index ea7e1745158..dda0f9a4460 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -209,15 +209,6 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
     ////////////////////
     // Behavior tests //
     ////////////////////
-    // Select 1 Cent with pool of only greater than 5 Cent
-    utxo_pool.clear();
-    for (int i = 5; i <= 20; ++i) {
-        add_coin(i * CENT, i, utxo_pool);
-    }
-    // Run 100 times, to make sure it is never finding a solution
-    for (int i = 0; i < 100; ++i) {
-        BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 2 * CENT));
-    }
 
     // Make sure that effective value is working in AttemptSelection when BnB is used
     CoinSelectionParams coin_selection_params_bnb{

From 1fb24c68a13de965f4f99664a5c1d1a4bb6eddc3 Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Fri, 1 Mar 2024 15:50:16 -0500
Subject: [PATCH 07/14] test: Recreate BnB iteration exhaustion test

---
 src/wallet/test/coinselection_tests.cpp | 35 +++++++++++++++++++++++++
 src/wallet/test/coinselector_tests.cpp  | 32 ----------------------
 2 files changed, 35 insertions(+), 32 deletions(-)

diff --git a/src/wallet/test/coinselection_tests.cpp b/src/wallet/test/coinselection_tests.cpp
index 19391a3920c..08922b44ab8 100644
--- a/src/wallet/test/coinselection_tests.cpp
+++ b/src/wallet/test/coinselection_tests.cpp
@@ -151,6 +151,41 @@ BOOST_AUTO_TEST_CASE(bnb_test)
     AddCoins(clone_pool, {2 * CENT, 7 * CENT, 7 * CENT});
     AddDuplicateCoins(clone_pool, 50'000, 5 * CENT);
     TestBnBSuccess("Skip equivalent input sets", clone_pool, /*selection_target=*/16 * CENT, /*expected_input_amounts=*/{2 * CENT, 7 * CENT, 7 * CENT});
+
+    /* Test BnB attempt limit (`TOTAL_TRIES`)
+     *
+     * Generally, on a diverse UTXO pool BnB will quickly pass over UTXOs bigger than the target and then start
+     * combining small counts of UTXOs that in sum remain under the selection_target+cost_of_change. When there are
+     * multiple UTXOs that have matching amount and cost, combinations with equivalent input sets are skipped. The UTXO
+     * pool for this test is specifically crafted to create as much branching as possible. The selection target is
+     * 8 CENT while all UTXOs are slightly bigger than 1 CENT. The smallest eight are 100,000…100,007 sats, while the larger
+     * nine are 100,368…100,375 (i.e., 100,008…100,016 sats plus cost_of_change (359 sats)).
+     *
+     * Because BnB will only select input sets that fall between selection_target and selection_target + cost_of_change,
+     * and the search traverses the UTXO pool from large to small amounts, the search will visit every single
+     * combination of eight inputs. All except the last combination will overshoot by more than cost_of_change on the eighth input, because the larger nine inputs each exceed 1 CENT by more than cost_of_change.
+     * Only the last combination consisting of the eight smallest UTXOs falls into the target window.
+     */
+    std::vector<OutputGroup> doppelganger_pool;
+    std::vector<CAmount> doppelgangers;
+    std::vector<CAmount> expected_inputs;
+    for (int i = 0; i < 17; ++i) {
+        if (i < 8) {
+            // The eight smallest UTXOs can be combined to create expected_result
+            doppelgangers.push_back(1 * CENT + i);
+            expected_inputs.push_back(doppelgangers[i]);
+        } else {
+            // Any eight UTXOs including at least one UTXO with the added cost_of_change will exceed target window
+            doppelgangers.push_back(1 * CENT + default_cs_params.m_cost_of_change + i);
+        }
+    }
+    AddCoins(doppelganger_pool, doppelgangers);
+    // Among up to 17 unique UTXOs of similar effective value we will find a solution composed of the eight smallest UTXOs
+    TestBnBSuccess("Combine smallest 8 of 17 unique UTXOs", doppelganger_pool, /*selection_target=*/8 * CENT, /*expected_input_amounts=*/expected_inputs);
+
+    // Starting with 18 unique UTXOs of similar effective value we will not find the solution due to exceeding the attempt limit
+    AddCoins(doppelganger_pool, {1 * CENT + default_cs_params.m_cost_of_change + 17});
+    TestBnBFail("Exhaust looking for smallest 8 of 18 unique UTXOs", doppelganger_pool, /*selection_target=*/8 * CENT);
 }
 
 BOOST_AUTO_TEST_CASE(bnb_feerate_sensitivity_test)
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index dda0f9a4460..5529365e241 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -37,15 +37,6 @@ static const CoinEligibilityFilter filter_confirmed(1, 1, 0);
 static const CoinEligibilityFilter filter_standard_extra(6, 6, 0);
 static int nextLockTime = 0;
 
-static void add_coin(const CAmount& nValue, int nInput, std::vector<COutput>& set)
-{
-    CMutableTransaction tx;
-    tx.vout.resize(nInput + 1);
-    tx.vout[nInput].nValue = nValue;
-    tx.nLockTime = nextLockTime++;        // so all transactions get different hashes
-    set.emplace_back(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, /*fees=*/ 0);
-}
-
 static void add_coin(const CAmount& nValue, int nInput, SelectionResult& result)
 {
     CMutableTransaction tx;
@@ -133,18 +124,6 @@ static bool EqualResult(const SelectionResult& a, const SelectionResult& b)
     return ret.first == a.GetInputSet().end() && ret.second == b.GetInputSet().end();
 }
 
-static CAmount make_hard_case(int utxos, std::vector<COutput>& utxo_pool)
-{
-    utxo_pool.clear();
-    CAmount target = 0;
-    for (int i = 0; i < utxos; ++i) {
-        target += CAmount{1} << (utxos+i);
-        add_coin(CAmount{1} << (utxos+i), 2*i, utxo_pool);
-        add_coin((CAmount{1} << (utxos+i)) + (CAmount{1} << (utxos-1-i)), 2*i + 1, utxo_pool);
-    }
-    return target;
-}
-
 inline std::vector<OutputGroup>& GroupCoins(const std::vector<COutput>& available_coins, bool subtract_fee_outputs = false)
 {
     static std::vector<OutputGroup> static_groups;
@@ -195,17 +174,6 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
     std::vector<COutput> utxo_pool;
     SelectionResult expected_result(CAmount(0), SelectionAlgorithm::BNB);
 
-    /////////////////////////
-    // Known Outcome tests //
-    /////////////////////////
-
-    // Iteration exhaustion test
-    CAmount target = make_hard_case(17, utxo_pool);
-    BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), target, 1)); // Should exhaust
-    target = make_hard_case(14, utxo_pool);
-    const auto result7 = SelectCoinsBnB(GroupCoins(utxo_pool), target, 1); // Should not exhaust
-    BOOST_CHECK(result7);
-
     ////////////////////
     // Behavior tests //
     ////////////////////

From 0de559f5c532e9a5ef6cbca1d00f4543571c947a Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Wed, 26 Mar 2025 15:32:43 -0700
Subject: [PATCH 08/14] coinselection: Count BnB iterations

---
 src/wallet/coinselection.cpp            |  1 +
 src/wallet/test/coinselection_tests.cpp | 27 +++++++++++++------------
 src/wallet/test/coinselector_tests.cpp  | 13 ++++++++++++
 3 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 6e6d7e053bd..4272e613020 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -122,6 +122,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
 
     // Depth First search loop for choosing the UTXOs
     for (size_t curr_try = 0, utxo_pool_index = 0; curr_try < TOTAL_TRIES; ++curr_try, ++utxo_pool_index) {
+        result.SetSelectionsEvaluated(curr_try);
         // Conditions for starting a backtrack
         bool backtrack = false;
         if (curr_value + curr_available_value < selection_target || // Cannot possibly reach target with the amount remaining in the curr_available_value.
diff --git a/src/wallet/test/coinselection_tests.cpp b/src/wallet/test/coinselection_tests.cpp
index 08922b44ab8..7d8f21389f3 100644
--- a/src/wallet/test/coinselection_tests.cpp
+++ b/src/wallet/test/coinselection_tests.cpp
@@ -98,7 +98,7 @@ static std::string InputsToString(const SelectionResult& selection)
     return res + "]";
 }
 
-static void TestBnBSuccess(std::string test_title, std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const std::vector<CAmount>& expected_input_amounts, const CoinSelectionParams& cs_params = default_cs_params, int custom_spending_vsize = 68)
+static void TestBnBSuccess(std::string test_title, std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const std::vector<CAmount>& expected_input_amounts, size_t expected_attempts, const CoinSelectionParams& cs_params = default_cs_params, int custom_spending_vsize = 68)
 {
     SelectionResult expected_result(CAmount(0), SelectionAlgorithm::BNB);
     CAmount expected_amount = 0;
@@ -112,6 +112,7 @@ static void TestBnBSuccess(std::string test_title, std::vector<OutputGroup>& utx
     BOOST_CHECK_MESSAGE(result, "Falsy result in BnB-Success: " + test_title);
     BOOST_CHECK_MESSAGE(HaveEquivalentValues(expected_result, *result), strprintf("Result mismatch in BnB-Success: %s. Expected %s, but got %s", test_title, InputsToString(expected_result), InputsToString(*result)));
     BOOST_CHECK_MESSAGE(result->GetSelectedValue() == expected_amount, strprintf("Selected amount mismatch in BnB-Success: %s. Expected %d, but got %d", test_title, expected_amount, result->GetSelectedValue()));
+    BOOST_CHECK_MESSAGE(result->GetSelectionsEvaluated() == expected_attempts, strprintf("Unexpected number of attempts in BnB-Success: %s. Expected %i attempts, but got %i", test_title, expected_attempts, result->GetSelectionsEvaluated()));
 }
 
 static void TestBnBFail(std::string test_title, std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target)
@@ -129,14 +130,14 @@ BOOST_AUTO_TEST_CASE(bnb_test)
     AddCoins(utxo_pool, {1 * CENT, 3 * CENT, 5 * CENT});
 
     // Simple success cases
-    TestBnBSuccess("Select smallest UTXO", utxo_pool, /*selection_target=*/1 * CENT, /*expected_input_amounts=*/{1 * CENT});
-    TestBnBSuccess("Select middle UTXO", utxo_pool, /*selection_target=*/3 * CENT, /*expected_input_amounts=*/{3 * CENT});
-    TestBnBSuccess("Select biggest UTXO", utxo_pool, /*selection_target=*/5 * CENT, /*expected_input_amounts=*/{5 * CENT});
-    TestBnBSuccess("Select two UTXOs", utxo_pool, /*selection_target=*/4 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT});
-    TestBnBSuccess("Select all UTXOs", utxo_pool, /*selection_target=*/9 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT, 5 * CENT});
+    TestBnBSuccess("Select smallest UTXO", utxo_pool, /*selection_target=*/1 * CENT, /*expected_input_amounts=*/{1 * CENT}, /*expected_attempts=*/6);
+    TestBnBSuccess("Select middle UTXO", utxo_pool, /*selection_target=*/3 * CENT, /*expected_input_amounts=*/{3 * CENT}, /*expected_attempts=*/4);
+    TestBnBSuccess("Select biggest UTXO", utxo_pool, /*selection_target=*/5 * CENT, /*expected_input_amounts=*/{5 * CENT}, /*expected_attempts=*/2);
+    TestBnBSuccess("Select two UTXOs", utxo_pool, /*selection_target=*/4 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT}, /*expected_attempts=*/6);
+    TestBnBSuccess("Select all UTXOs", utxo_pool, /*selection_target=*/9 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT, 5 * CENT}, /*expected_attempts=*/6);
 
     // BnB finds changeless solution while overshooting by up to cost_of_change
-    TestBnBSuccess("Select upper bound", utxo_pool, /*selection_target=*/4 * CENT - default_cs_params.m_cost_of_change, /*expected_input_amounts=*/{1 * CENT, 3 * CENT});
+    TestBnBSuccess("Select upper bound", utxo_pool, /*selection_target=*/4 * CENT - default_cs_params.m_cost_of_change, /*expected_input_amounts=*/{1 * CENT, 3 * CENT}, /*expected_attempts=*/6);
 
     // BnB fails to find changeless solution when overshooting by cost_of_change + 1 sat
     TestBnBFail("Overshoot upper bound", utxo_pool, /*selection_target=*/4 * CENT - default_cs_params.m_cost_of_change - 1);
@@ -150,7 +151,7 @@ BOOST_AUTO_TEST_CASE(bnb_test)
     std::vector<OutputGroup> clone_pool;
     AddCoins(clone_pool, {2 * CENT, 7 * CENT, 7 * CENT});
     AddDuplicateCoins(clone_pool, 50'000, 5 * CENT);
-    TestBnBSuccess("Skip equivalent input sets", clone_pool, /*selection_target=*/16 * CENT, /*expected_input_amounts=*/{2 * CENT, 7 * CENT, 7 * CENT});
+    TestBnBSuccess("Skip equivalent input sets", clone_pool, /*selection_target=*/16 * CENT, /*expected_input_amounts=*/{2 * CENT, 7 * CENT, 7 * CENT}, /*expected_attempts=*/99'999);
 
     /* Test BnB attempt limit (`TOTAL_TRIES`)
      *
@@ -181,7 +182,7 @@ BOOST_AUTO_TEST_CASE(bnb_test)
     }
     AddCoins(doppelganger_pool, doppelgangers);
     // Among up to 17 unique UTXOs of similar effective value we will find a solution composed of the eight smallest UTXOs
-    TestBnBSuccess("Combine smallest 8 of 17 unique UTXOs", doppelganger_pool, /*selection_target=*/8 * CENT, /*expected_input_amounts=*/expected_inputs);
+    TestBnBSuccess("Combine smallest 8 of 17 unique UTXOs", doppelganger_pool, /*selection_target=*/8 * CENT, /*expected_input_amounts=*/expected_inputs, /*expected_attempts=*/87'514);
 
     // Starting with 18 unique UTXOs of similar effective value we will not find the solution due to exceeding the attempt limit
     AddCoins(doppelganger_pool, {1 * CENT + default_cs_params.m_cost_of_change + 17});
@@ -193,22 +194,22 @@ BOOST_AUTO_TEST_CASE(bnb_feerate_sensitivity_test)
     // Create sets of UTXOs with the same effective amounts at different feerates (but different absolute amounts)
     std::vector<OutputGroup> low_feerate_pool; // 5 sat/vB (default, and lower than long_term_feerate of 10 sat/vB)
     AddCoins(low_feerate_pool, {2 * CENT, 3 * CENT, 5 * CENT, 10 * CENT});
-    TestBnBSuccess("Select many inputs at low feerates", low_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{2 * CENT, 3 * CENT, 5 * CENT});
+    TestBnBSuccess("Select many inputs at low feerates", low_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{2 * CENT, 3 * CENT, 5 * CENT}, /*expected_attempts=*/8);
 
     CoinSelectionParams high_feerate_params = init_default_params();
     high_feerate_params.m_effective_feerate = CFeeRate{25'000};
     std::vector<OutputGroup> high_feerate_pool; // 25 sat/vB (greater than long_term_feerate of 10 sat/vB)
     AddCoins(high_feerate_pool, {2 * CENT, 3 * CENT, 5 * CENT, 10 * CENT}, high_feerate_params);
-    TestBnBSuccess("Select one input at high feerates", high_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{10 * CENT}, high_feerate_params);
+    TestBnBSuccess("Select one input at high feerates", high_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{10 * CENT}, /*expected_attempts=*/6, high_feerate_params);
 
     // Add heavy inputs {6, 7} to existing {2, 3, 5, 10}
     low_feerate_pool.push_back(MakeCoin(6 * CENT, true, default_cs_params, /*custom_spending_vsize=*/500));
     low_feerate_pool.push_back(MakeCoin(7 * CENT, true, default_cs_params, /*custom_spending_vsize=*/500));
-    TestBnBSuccess("Prefer two heavy inputs over two light inputs at low feerates", low_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{6 * CENT, 7 * CENT}, default_cs_params, /*custom_spending_vsize=*/500);
+    TestBnBSuccess("Prefer two heavy inputs over two light inputs at low feerates", low_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{6 * CENT, 7 * CENT}, /*expected_attempts=*/28, default_cs_params, /*custom_spending_vsize=*/500);
 
     high_feerate_pool.push_back(MakeCoin(6 * CENT, true, high_feerate_params, /*custom_spending_vsize=*/500));
     high_feerate_pool.push_back(MakeCoin(7 * CENT, true, high_feerate_params, /*custom_spending_vsize=*/500));
-    TestBnBSuccess("Prefer two light inputs over two heavy inputs at high feerates", high_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{3 * CENT, 10 * CENT}, high_feerate_params);
+    TestBnBSuccess("Prefer two light inputs over two heavy inputs at high feerates", high_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{3 * CENT, 10 * CENT}, /*expected_attempts=*/14, high_feerate_params);
 }
 
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 5529365e241..51e1fce0142 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -173,6 +173,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
     // Setup
     std::vector<COutput> utxo_pool;
     SelectionResult expected_result(CAmount(0), SelectionAlgorithm::BNB);
+    size_t expected_attempts;
 
     ////////////////////
     // Behavior tests //
@@ -210,6 +211,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         const auto result9 = SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change);
         BOOST_CHECK(result9);
         BOOST_CHECK_EQUAL(result9->GetSelectedValue(), 1 * CENT);
+        // Demonstrate how following improvements reduce iteration count and catch any regressions in the future.
+        expected_attempts = 2;
+        BOOST_CHECK_MESSAGE(result9->GetSelectionsEvaluated() == expected_attempts, strprintf("Expected %i attempts, but got %i", expected_attempts, result9->GetSelectionsEvaluated()));
     }
 
     {
@@ -232,6 +236,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         LOCK(wallet->cs_wallet);
         const auto result10 = SelectCoins(*wallet, available_coins, selected_input, 10 * CENT, coin_control, coin_selection_params_bnb);
         BOOST_CHECK(result10);
+        // Demonstrate how following improvements reduce iteration count and catch any regressions in the future.
+        expected_attempts = 4;
+        BOOST_CHECK_MESSAGE(result10->GetSelectionsEvaluated() == expected_attempts, strprintf("Expected %i attempts, but got %i", expected_attempts, result10->GetSelectionsEvaluated()));
     }
     {
         std::unique_ptr<CWallet> wallet = NewWallet(m_node);
@@ -261,6 +268,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         available_coins.Erase({(++available_coins.coins[OutputType::BECH32].begin())->outpoint});
         const auto result13 = SelectCoins(*wallet, available_coins, selected_input, 10 * CENT, coin_control, coin_selection_params_bnb);
         BOOST_CHECK(EquivalentResult(expected_result, *result13));
+        // Demonstrate how following improvements reduce iteration count and catch any regressions in the future.
+        expected_attempts = 4;
+        BOOST_CHECK_MESSAGE(result13->GetSelectionsEvaluated() == expected_attempts, strprintf("Expected %i attempts, but got %i", expected_attempts, result13->GetSelectionsEvaluated()));
     }
 
     {
@@ -292,6 +302,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         add_coin(5 * CENT, 2, expected_result);
         add_coin(3 * CENT, 2, expected_result);
         BOOST_CHECK(EquivalentResult(expected_result, *res));
+        // Demonstrate how following improvements reduce iteration count and catch any regressions in the future.
+        expected_attempts = 38;
+        BOOST_CHECK_MESSAGE(res->GetSelectionsEvaluated() == expected_attempts, strprintf("Expected %i attempts, but got %i", expected_attempts, res->GetSelectionsEvaluated()));
     }
 }
 

From d765ba9ae567cf3192706223d873d041f71f9c82 Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Wed, 26 Mar 2025 16:18:01 -0700
Subject: [PATCH 09/14] coinselection: rewrite BnB in CoinGrinder-style

In the original implementation of BnB, the state of the search is
backtracked by explicitly walking back to the omission branch and then
testing again. This retests an equivalent candidate set as before, e.g.,
after backtracking from {ABC}, it would evaluate {AB_}, before trying
{AB_D}, but {AB_} is equivalent to {AB} which was tested before.

CoinGrinder tracks the state of the search instead by remembering which
UTXO was last added and explicitly shifting from that UTXO directly to
the next, so after {ABC}, it will immediately move on to {AB_D}. We
replicate this approach here.
---
 src/wallet/coinselection.cpp            | 171 +++++++++++++-----------
 src/wallet/test/coinselection_tests.cpp |  20 +--
 src/wallet/test/coinselector_tests.cpp  |   8 +-
 3 files changed, 107 insertions(+), 92 deletions(-)

diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 4272e613020..0b74ccf5064 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -55,11 +55,11 @@ struct {
  * cost of creating and spending a change output. The algorithm uses a depth-first search on a binary
  * tree. In the binary tree, each node corresponds to the inclusion or the omission of a UTXO. UTXOs
  * are sorted by their effective values and the tree is explored deterministically per the inclusion
- * branch first. At each node, the algorithm checks whether the selection is within the target range.
+ * branch first. For each new input set candidate, the algorithm checks whether the selection is within the target range.
  * While the selection has not reached the target range, more UTXOs are included. When a selection's
  * value exceeds the target range, the complete subtree deriving from this selection can be omitted.
  * At that point, the last included UTXO is deselected and the corresponding omission branch explored
- * instead. The search ends after the complete tree has been searched or after a limited number of tries.
+ * instead starting by adding the subsequent UTXO. The search ends after the complete tree has been searched or after a limited number of tries.
  *
  * The search continues to search for better solutions after one solution has been found. The best
  * solution is chosen by minimizing the waste metric. The waste metric is defined as the cost to
@@ -68,9 +68,9 @@ struct {
  *
  * waste = selectionTotal - target + inputs × (currentFeeRate - longTermFeeRate)
  *
- * The algorithm uses two additional optimizations. A lookahead keeps track of the total value of
+ * The algorithm uses two additional optimizations. A lookahead (tk) keeps track of the total value of
  * the unexplored UTXOs. A subtree is not explored if the lookahead indicates that the target range
- * cannot be reached. Further, it is unnecessary to test equivalent combinations. This allows us
+ * cannot be reached. Further, it is unnecessary to test equivalent combinations (tk). This allows us
  * to skip testing the inclusion of UTXOs that match the effective value and waste of an omitted
  * predecessor.
  *
@@ -93,114 +93,129 @@ static const size_t TOTAL_TRIES = 100000;
 util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change,
                                              int max_selection_weight)
 {
-    SelectionResult result(selection_target, SelectionAlgorithm::BNB);
-    CAmount curr_value = 0;
-    std::vector<size_t> curr_selection; // selected utxo indexes
-    int curr_selection_weight = 0; // sum of selected utxo weight
-
-    // Calculate curr_available_value
-    CAmount curr_available_value = 0;
+    // Check that there are sufficient funds
+    CAmount total_available = 0;
     for (const OutputGroup& utxo : utxo_pool) {
-        // Assert that this utxo is not negative. It should never be negative,
-        // effective value calculation should have removed it
-        assert(utxo.GetSelectionAmount() > 0);
-        curr_available_value += utxo.GetSelectionAmount();
+        // Assert UTXOs with non-positive effective value have been filtered
+        Assume(utxo.GetSelectionAmount() > 0);
+        total_available += utxo.GetSelectionAmount();
     }
-    if (curr_available_value < selection_target) {
+
+    if (total_available < selection_target) {
+        // Insufficient funds
         return util::Error();
     }
 
-    // Sort the utxo_pool
     std::sort(utxo_pool.begin(), utxo_pool.end(), descending);
 
-    CAmount curr_waste = 0;
+    // The current selection and the best input set found so far, stored as the utxo_pool indices of the UTXOs forming them
+    std::vector<size_t> curr_selection;
     std::vector<size_t> best_selection;
+
+    // The currently selected effective amount
+    CAmount curr_amount = 0;
+
+    // The waste score of the current section, and the best waste score so far
+    CAmount curr_selection_waste = 0;
     CAmount best_waste = MAX_MONEY;
 
-    bool is_feerate_high = utxo_pool.at(0).fee > utxo_pool.at(0).long_term_fee;
+    // The weight of the currently selected input set
+    int curr_weight = 0;
+
+    // Whether the input sets generated during this search have exceeded the maximum transaction weight at any point
     bool max_tx_weight_exceeded = false;
 
-    // Depth First search loop for choosing the UTXOs
-    for (size_t curr_try = 0, utxo_pool_index = 0; curr_try < TOTAL_TRIES; ++curr_try, ++utxo_pool_index) {
-        result.SetSelectionsEvaluated(curr_try);
-        // Conditions for starting a backtrack
-        bool backtrack = false;
-        if (curr_value + curr_available_value < selection_target || // Cannot possibly reach target with the amount remaining in the curr_available_value.
-            curr_value > selection_target + cost_of_change || // Selected value is out of range, go back and try other branch
-            (curr_waste > best_waste && is_feerate_high)) { // Don't select things which we know will be more wasteful if the waste is increasing
-            backtrack = true;
-        } else if (curr_selection_weight > max_selection_weight) { // Selected UTXOs weight exceeds the maximum weight allowed, cannot find more solutions by adding more inputs
-            max_tx_weight_exceeded = true; // at least one selection attempt exceeded the max weight
-            backtrack = true;
-        } else if (curr_value >= selection_target) {       // Selected value is within range
-            curr_waste += (curr_value - selection_target); // This is the excess value which is added to the waste for the below comparison
-            // Adding another UTXO after this check could bring the waste down if the long term fee is higher than the current fee.
-            // However we are not going to explore that because this optimization for the waste is only done when we have hit our target
-            // value. Adding any more UTXOs will be just burning the UTXO; it will go entirely to fees. Thus we aren't going to
-            // explore any more UTXOs to avoid burning money like that.
+    // Index of the next UTXO to consider in utxo_pool
+    size_t next_utxo = 0;
+
+    auto deselect_last = [&]() {
+        OutputGroup& utxo = utxo_pool[curr_selection.back()];
+        curr_amount -= utxo.GetSelectionAmount();
+        curr_weight -= utxo.m_weight;
+        curr_selection_waste -= utxo.fee - utxo.long_term_fee;
+        curr_selection.pop_back();
+    };
+
+    size_t curr_try = 0;
+    while (true) {
+        bool should_shift{false}, should_cut{false};
+        // Select `next_utxo`
+        OutputGroup& utxo = utxo_pool[next_utxo];
+        curr_amount += utxo.GetSelectionAmount();
+        curr_weight += utxo.m_weight;
+        curr_selection_waste += utxo.fee - utxo.long_term_fee;
+        curr_selection.push_back(next_utxo);
+        ++next_utxo;
+        ++curr_try;
+
+        // EVALUATE current selection: check for solutions and see whether we can CUT or SHIFT before EXPLORING further
+        if (curr_weight > max_selection_weight) {
+            // max_weight exceeded: SHIFT
+            max_tx_weight_exceeded = true;
+            should_shift  = true;
+        } else if (curr_amount > selection_target + cost_of_change) {
+            // Overshot target range: SHIFT
+            should_shift = true;
+        } else if (curr_amount >= selection_target) {
+            // Selection is within target window: potential solution
+            // Adding more UTXOs only increases fees and cannot be better: SHIFT
+            should_shift  = true;
+            // The amount exceeding the selection_target (the "excess"), would be dropped to the fees: it is waste.
+            CAmount curr_excess = curr_amount - selection_target;
+            CAmount curr_waste = curr_selection_waste + curr_excess;
             if (curr_waste <= best_waste) {
+                // New best solution
                 best_selection = curr_selection;
                 best_waste = curr_waste;
             }
-            curr_waste -= (curr_value - selection_target); // Remove the excess value as we will be selecting different coins now
-            backtrack = true;
         }
 
-        if (backtrack) { // Backtracking, moving backwards
-            if (curr_selection.empty()) { // We have walked back to the first utxo and no branch is untraversed. All solutions searched
+        if (curr_try >= TOTAL_TRIES) {
+            // Solution is not guaranteed to be optimal if `curr_try` hit TOTAL_TRIES
+            break;
+        }
+
+        if (next_utxo == utxo_pool.size()) {
+            // Last added UTXO was end of UTXO pool, nothing left to add on inclusion or omission branch: CUT
+            should_cut = true;
+        }
+
+        if (should_cut) {
+            // Neither adding to the current selection nor exploring the omission branch of the last selected UTXO can
+            // find any solutions. Redirect to exploring the Omission branch of the penultimate selected UTXO (i.e.
+            // set `next_utxo` to one after the penultimate selected, then deselect the last two selected UTXOs)
+            should_cut = false;
+            deselect_last();
+            should_shift  = true;
+        }
+
+        if (should_shift) {
+            // Set `next_utxo` to one after last selected, then deselect last selected UTXO
+            if (curr_selection.empty()) {
+                // Exhausted search space before running into attempt limit
                 break;
             }
-
-            // Add omitted UTXOs back to lookahead before traversing the omission branch of last included UTXO.
-            for (--utxo_pool_index; utxo_pool_index > curr_selection.back(); --utxo_pool_index) {
-                curr_available_value += utxo_pool.at(utxo_pool_index).GetSelectionAmount();
-            }
-
-            // Output was included on previous iterations, try excluding now.
-            assert(utxo_pool_index == curr_selection.back());
-            OutputGroup& utxo = utxo_pool.at(utxo_pool_index);
-            curr_value -= utxo.GetSelectionAmount();
-            curr_waste -= utxo.fee - utxo.long_term_fee;
-            curr_selection_weight -= utxo.m_weight;
-            curr_selection.pop_back();
-        } else { // Moving forwards, continuing down this branch
-            OutputGroup& utxo = utxo_pool.at(utxo_pool_index);
-
-            // Remove this utxo from the curr_available_value utxo amount
-            curr_available_value -= utxo.GetSelectionAmount();
-
-            if (curr_selection.empty() ||
-                // The previous index is included and therefore not relevant for exclusion shortcut
-                (utxo_pool_index - 1) == curr_selection.back() ||
-                // Avoid searching a branch if the previous UTXO has the same value and same waste and was excluded.
-                // Since the ratio of fee to long term fee is the same, we only need to check if one of those values match in order to know that the waste is the same.
-                utxo.GetSelectionAmount() != utxo_pool.at(utxo_pool_index - 1).GetSelectionAmount() ||
-                utxo.fee != utxo_pool.at(utxo_pool_index - 1).fee)
-            {
-                // Inclusion branch first (Largest First Exploration)
-                curr_selection.push_back(utxo_pool_index);
-                curr_value += utxo.GetSelectionAmount();
-                curr_waste += utxo.fee - utxo.long_term_fee;
-                curr_selection_weight += utxo.m_weight;
-            }
+            next_utxo = curr_selection.back() + 1;
+            deselect_last();
+            should_shift  = false;
         }
     }
 
-    // Check for solution
+    SelectionResult result(selection_target, SelectionAlgorithm::BNB);
+    result.SetSelectionsEvaluated(curr_try);
+
     if (best_selection.empty()) {
         return max_tx_weight_exceeded ? ErrorMaxWeightExceeded() : util::Error();
     }
 
-    // Set output set
     for (const size_t& i : best_selection) {
         result.AddInput(utxo_pool.at(i));
     }
-    result.RecalculateWaste(cost_of_change, cost_of_change, CAmount{0});
-    assert(best_waste == result.GetWaste());
 
     return result;
 }
 
+
 /*
  * TL;DR: Coin Grinder is a DFS-based algorithm that deterministically searches for the minimum-weight input set to fund
  * the transaction. The algorithm is similar to the Branch and Bound algorithm, but will produce a transaction _with_ a
diff --git a/src/wallet/test/coinselection_tests.cpp b/src/wallet/test/coinselection_tests.cpp
index 7d8f21389f3..32be616a682 100644
--- a/src/wallet/test/coinselection_tests.cpp
+++ b/src/wallet/test/coinselection_tests.cpp
@@ -130,14 +130,14 @@ BOOST_AUTO_TEST_CASE(bnb_test)
     AddCoins(utxo_pool, {1 * CENT, 3 * CENT, 5 * CENT});
 
     // Simple success cases
-    TestBnBSuccess("Select smallest UTXO", utxo_pool, /*selection_target=*/1 * CENT, /*expected_input_amounts=*/{1 * CENT}, /*expected_attempts=*/6);
-    TestBnBSuccess("Select middle UTXO", utxo_pool, /*selection_target=*/3 * CENT, /*expected_input_amounts=*/{3 * CENT}, /*expected_attempts=*/4);
-    TestBnBSuccess("Select biggest UTXO", utxo_pool, /*selection_target=*/5 * CENT, /*expected_input_amounts=*/{5 * CENT}, /*expected_attempts=*/2);
-    TestBnBSuccess("Select two UTXOs", utxo_pool, /*selection_target=*/4 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT}, /*expected_attempts=*/6);
-    TestBnBSuccess("Select all UTXOs", utxo_pool, /*selection_target=*/9 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT, 5 * CENT}, /*expected_attempts=*/6);
+    TestBnBSuccess("Select smallest UTXO", utxo_pool, /*selection_target=*/1 * CENT, /*expected_input_amounts=*/{1 * CENT}, /*expected_attempts=*/3);
+    TestBnBSuccess("Select middle UTXO", utxo_pool, /*selection_target=*/3 * CENT, /*expected_input_amounts=*/{3 * CENT}, /*expected_attempts=*/3);
+    TestBnBSuccess("Select biggest UTXO", utxo_pool, /*selection_target=*/5 * CENT, /*expected_input_amounts=*/{5 * CENT}, /*expected_attempts=*/4);
+    TestBnBSuccess("Select two UTXOs", utxo_pool, /*selection_target=*/4 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT}, /*expected_attempts=*/4);
+    TestBnBSuccess("Select all UTXOs", utxo_pool, /*selection_target=*/9 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT, 5 * CENT}, /*expected_attempts=*/7);
 
     // BnB finds changeless solution while overshooting by up to cost_of_change
-    TestBnBSuccess("Select upper bound", utxo_pool, /*selection_target=*/4 * CENT - default_cs_params.m_cost_of_change, /*expected_input_amounts=*/{1 * CENT, 3 * CENT}, /*expected_attempts=*/6);
+    TestBnBSuccess("Select upper bound", utxo_pool, /*selection_target=*/4 * CENT - default_cs_params.m_cost_of_change, /*expected_input_amounts=*/{1 * CENT, 3 * CENT}, /*expected_attempts=*/4);
 
     // BnB fails to find changeless solution when overshooting by cost_of_change + 1 sat
     TestBnBFail("Overshoot upper bound", utxo_pool, /*selection_target=*/4 * CENT - default_cs_params.m_cost_of_change - 1);
@@ -151,7 +151,7 @@ BOOST_AUTO_TEST_CASE(bnb_test)
     std::vector<OutputGroup> clone_pool;
     AddCoins(clone_pool, {2 * CENT, 7 * CENT, 7 * CENT});
     AddDuplicateCoins(clone_pool, 50'000, 5 * CENT);
-    TestBnBSuccess("Skip equivalent input sets", clone_pool, /*selection_target=*/16 * CENT, /*expected_input_amounts=*/{2 * CENT, 7 * CENT, 7 * CENT}, /*expected_attempts=*/99'999);
+    TestBnBSuccess("Skip equivalent input sets", clone_pool, /*selection_target=*/16 * CENT, /*expected_input_amounts=*/{2 * CENT, 7 * CENT, 7 * CENT}, /*expected_attempts=*/100'000);
 
     /* Test BnB attempt limit (`TOTAL_TRIES`)
      *
@@ -182,7 +182,7 @@ BOOST_AUTO_TEST_CASE(bnb_test)
     }
     AddCoins(doppelganger_pool, doppelgangers);
     // Among up to 17 unique UTXOs of similar effective value we will find a solution composed of the eight smallest UTXOs
-    TestBnBSuccess("Combine smallest 8 of 17 unique UTXOs", doppelganger_pool, /*selection_target=*/8 * CENT, /*expected_input_amounts=*/expected_inputs, /*expected_attempts=*/87'514);
+    TestBnBSuccess("Combine smallest 8 of 17 unique UTXOs", doppelganger_pool, /*selection_target=*/8 * CENT, /*expected_input_amounts=*/expected_inputs, /*expected_attempts=*/65'535);
 
     // Starting with 18 unique UTXOs of similar effective value we will not find the solution due to exceeding the attempt limit
     AddCoins(doppelganger_pool, {1 * CENT + default_cs_params.m_cost_of_change + 17});
@@ -200,7 +200,7 @@ BOOST_AUTO_TEST_CASE(bnb_feerate_sensitivity_test)
     high_feerate_params.m_effective_feerate = CFeeRate{25'000};
     std::vector<OutputGroup> high_feerate_pool; // 25 sat/vB (greater than long_term_feerate of 10 sat/vB)
     AddCoins(high_feerate_pool, {2 * CENT, 3 * CENT, 5 * CENT, 10 * CENT}, high_feerate_params);
-    TestBnBSuccess("Select one input at high feerates", high_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{10 * CENT}, /*expected_attempts=*/6, high_feerate_params);
+    TestBnBSuccess("Select one input at high feerates", high_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{10 * CENT}, /*expected_attempts=*/8, high_feerate_params);
 
     // Add heavy inputs {6, 7} to existing {2, 3, 5, 10}
     low_feerate_pool.push_back(MakeCoin(6 * CENT, true, default_cs_params, /*custom_spending_vsize=*/500));
@@ -209,7 +209,7 @@ BOOST_AUTO_TEST_CASE(bnb_feerate_sensitivity_test)
 
     high_feerate_pool.push_back(MakeCoin(6 * CENT, true, high_feerate_params, /*custom_spending_vsize=*/500));
     high_feerate_pool.push_back(MakeCoin(7 * CENT, true, high_feerate_params, /*custom_spending_vsize=*/500));
-    TestBnBSuccess("Prefer two light inputs over two heavy inputs at high feerates", high_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{3 * CENT, 10 * CENT}, /*expected_attempts=*/14, high_feerate_params);
+    TestBnBSuccess("Prefer two light inputs over two heavy inputs at high feerates", high_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{3 * CENT, 10 * CENT}, /*expected_attempts=*/28, high_feerate_params);
 }
 
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 51e1fce0142..08593c4de6e 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -212,7 +212,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         BOOST_CHECK(result9);
         BOOST_CHECK_EQUAL(result9->GetSelectedValue(), 1 * CENT);
         // Demonstrate how following improvements reduce iteration count and catch any regressions in the future.
-        expected_attempts = 2;
+        expected_attempts = 1;
         BOOST_CHECK_MESSAGE(result9->GetSelectionsEvaluated() == expected_attempts, strprintf("Expected %i attempts, but got %i", expected_attempts, result9->GetSelectionsEvaluated()));
     }
 
@@ -237,7 +237,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         const auto result10 = SelectCoins(*wallet, available_coins, selected_input, 10 * CENT, coin_control, coin_selection_params_bnb);
         BOOST_CHECK(result10);
         // Demonstrate how following improvements reduce iteration count and catch any regressions in the future.
-        expected_attempts = 4;
+        expected_attempts = 3;
         BOOST_CHECK_MESSAGE(result10->GetSelectionsEvaluated() == expected_attempts, strprintf("Expected %i attempts, but got %i", expected_attempts, result10->GetSelectionsEvaluated()));
     }
     {
@@ -269,7 +269,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         const auto result13 = SelectCoins(*wallet, available_coins, selected_input, 10 * CENT, coin_control, coin_selection_params_bnb);
         BOOST_CHECK(EquivalentResult(expected_result, *result13));
         // Demonstrate how following improvements reduce iteration count and catch any regressions in the future.
-        expected_attempts = 4;
+        expected_attempts = 2;
         BOOST_CHECK_MESSAGE(result13->GetSelectionsEvaluated() == expected_attempts, strprintf("Expected %i attempts, but got %i", expected_attempts, result13->GetSelectionsEvaluated()));
     }
 
@@ -303,7 +303,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         add_coin(3 * CENT, 2, expected_result);
         BOOST_CHECK(EquivalentResult(expected_result, *res));
         // Demonstrate how following improvements reduce iteration count and catch any regressions in the future.
-        expected_attempts = 38;
+        expected_attempts = 39;
         BOOST_CHECK_MESSAGE(res->GetSelectionsEvaluated() == expected_attempts, strprintf("Expected %i attempts, but got %i", expected_attempts, res->GetSelectionsEvaluated()));
     }
 }

From b6bf22cc1dc65f4118b8a8e5d24e5ef41d56472b Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Wed, 26 Mar 2025 17:18:18 -0700
Subject: [PATCH 10/14] coinselection: Track whether BnB completed

BnB may not be able to exhaustively search all potentially interesting
combinations for large UTXO pools, so we keep track of whether the
search was terminated by the iteration limit.
---
 src/wallet/coinselection.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 0b74ccf5064..d86d3288371 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -137,6 +137,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
     };
 
     size_t curr_try = 0;
+    SelectionResult result(selection_target, SelectionAlgorithm::BNB);
     while (true) {
         bool should_shift{false}, should_cut{false};
         // Select `next_utxo`
@@ -172,6 +173,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
 
         if (curr_try >= TOTAL_TRIES) {
             // Solution is not guaranteed to be optimal if `curr_try` hit TOTAL_TRIES
+            result.SetAlgoCompleted(false);
             break;
         }
 
@@ -193,6 +195,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
             // Set `next_utxo` to one after last selected, then deselect last selected UTXO
             if (curr_selection.empty()) {
                 // Exhausted search space before running into attempt limit
+                result.SetAlgoCompleted(true);
                 break;
             }
             next_utxo = curr_selection.back() + 1;
@@ -201,7 +204,6 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
         }
     }
 
-    SelectionResult result(selection_target, SelectionAlgorithm::BNB);
     result.SetSelectionsEvaluated(curr_try);
 
     if (best_selection.empty()) {

From 71adfa7e23ab72eaff52e595521ad54cbb16b5a0 Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Wed, 26 Mar 2025 18:17:27 -0700
Subject: [PATCH 11/14] coinselection: BnB skip exploring high waste

---
 src/wallet/coinselection.cpp            | 4 ++++
 src/wallet/test/coinselection_tests.cpp | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index d86d3288371..15b2de938ee 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -138,6 +138,7 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
 
     size_t curr_try = 0;
     SelectionResult result(selection_target, SelectionAlgorithm::BNB);
+    bool is_feerate_high = utxo_pool.at(0).fee > utxo_pool.at(0).long_term_fee;
     while (true) {
         bool should_shift{false}, should_cut{false};
         // Select `next_utxo`
@@ -157,6 +158,9 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
         } else if (curr_amount > selection_target + cost_of_change) {
             // Overshot target range: SHIFT
             should_shift = true;
+        } else if (is_feerate_high && curr_selection_waste > best_waste) {
+            // Waste is already worse than best selection and adding more inputs will not improve it: SHIFT
+            should_shift = true;
         } else if (curr_amount >= selection_target) {
             // Selection is within target window: potential solution
             // Adding more UTXOs only increases fees and cannot be better: SHIFT
diff --git a/src/wallet/test/coinselection_tests.cpp b/src/wallet/test/coinselection_tests.cpp
index 32be616a682..8d308eb3eb1 100644
--- a/src/wallet/test/coinselection_tests.cpp
+++ b/src/wallet/test/coinselection_tests.cpp
@@ -200,7 +200,7 @@ BOOST_AUTO_TEST_CASE(bnb_feerate_sensitivity_test)
     high_feerate_params.m_effective_feerate = CFeeRate{25'000};
     std::vector<OutputGroup> high_feerate_pool; // 25 sat/vB (greater than long_term_feerate of 10 sat/vB)
     AddCoins(high_feerate_pool, {2 * CENT, 3 * CENT, 5 * CENT, 10 * CENT}, high_feerate_params);
-    TestBnBSuccess("Select one input at high feerates", high_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{10 * CENT}, /*expected_attempts=*/8, high_feerate_params);
+    TestBnBSuccess("Select one input at high feerates", high_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{10 * CENT}, /*expected_attempts=*/7, high_feerate_params);
 
     // Add heavy inputs {6, 7} to existing {2, 3, 5, 10}
     low_feerate_pool.push_back(MakeCoin(6 * CENT, true, default_cs_params, /*custom_spending_vsize=*/500));
@@ -209,7 +209,7 @@ BOOST_AUTO_TEST_CASE(bnb_feerate_sensitivity_test)
 
     high_feerate_pool.push_back(MakeCoin(6 * CENT, true, high_feerate_params, /*custom_spending_vsize=*/500));
     high_feerate_pool.push_back(MakeCoin(7 * CENT, true, high_feerate_params, /*custom_spending_vsize=*/500));
-    TestBnBSuccess("Prefer two light inputs over two heavy inputs at high feerates", high_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{3 * CENT, 10 * CENT}, /*expected_attempts=*/28, high_feerate_params);
+    TestBnBSuccess("Prefer two light inputs over two heavy inputs at high feerates", high_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{3 * CENT, 10 * CENT}, /*expected_attempts=*/15, high_feerate_params);
 }
 
 BOOST_AUTO_TEST_SUITE_END()

From aac608a121e4b168e7d13d718c40ac3bf9cb3f38 Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Wed, 26 Mar 2025 18:24:22 -0700
Subject: [PATCH 12/14] coinselection: Track effective_value lookahead

Introduces a dedicated data structure to track the total
effective_value available in the remaining UTXOs at each index of the
UTXO pool. In contrast to the original approach in BnB, this allows us
to immediately jump to a lower index instead of visiting every UTXO to
add back their eff_value to the lookahead.
---
 src/wallet/coinselection.cpp            | 24 ++++++++++++++++--------
 src/wallet/test/coinselection_tests.cpp | 22 +++++++++++++---------
 src/wallet/test/coinselector_tests.cpp  |  2 +-
 3 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 15b2de938ee..7ec4154decc 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -68,7 +68,7 @@ struct {
  *
  * waste = selectionTotal - target + inputs × (currentFeeRate - longTermFeeRate)
  *
- * The algorithm uses two additional optimizations. A lookahead (tk) keeps track of the total value of
+ * The algorithm uses two additional optimizations. A lookahead keeps track of the total value of
  * the unexplored UTXOs. A subtree is not explored if the lookahead indicates that the target range
  * cannot be reached. Further, it is unnecessary to test equivalent combinations (tk). This allows us
  * to skip testing the inclusion of UTXOs that match the effective value and waste of an omitted
@@ -93,12 +93,18 @@ static const size_t TOTAL_TRIES = 100000;
 util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change,
                                              int max_selection_weight)
 {
-    // Check that there are sufficient funds
+    std::sort(utxo_pool.begin(), utxo_pool.end(), descending);
+    // The sum of UTXO amounts after this UTXO index, e.g. lookahead[5] = Σ(UTXO[6+].amount)
+    std::vector<CAmount> lookahead(utxo_pool.size());
+
+    // Calculate lookahead values, and check that there are sufficient funds
     CAmount total_available = 0;
-    for (const OutputGroup& utxo : utxo_pool) {
-        // Assert UTXOs with non-positive effective value have been filtered
-        Assume(utxo.GetSelectionAmount() > 0);
-        total_available += utxo.GetSelectionAmount();
+    for (size_t i = 0; i < utxo_pool.size(); ++i) {
+        size_t index = utxo_pool.size() - 1 - i; // Loop over every element in reverse order
+        lookahead[index] = total_available;
+        // UTXOs with non-positive effective value must have been filtered
+        Assume(utxo_pool[index].GetSelectionAmount() > 0);
+        total_available += utxo_pool[index].GetSelectionAmount();
     }
 
     if (total_available < selection_target) {
@@ -106,7 +112,6 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
         return util::Error();
     }
 
-    std::sort(utxo_pool.begin(), utxo_pool.end(), descending);
 
     // The current selection and the best input set found so far, stored as the utxo_pool indices of the UTXOs forming them
     std::vector<size_t> curr_selection;
@@ -151,7 +156,10 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
         ++curr_try;
 
         // EVALUATE current selection: check for solutions and see whether we can CUT or SHIFT before EXPLORING further
-        if (curr_weight > max_selection_weight) {
+        if (curr_amount + lookahead[curr_selection.back()] < selection_target) {
+            // Insufficient funds with lookahead: CUT
+            should_cut = true;
+        } else if (curr_weight > max_selection_weight) {
             // max_weight exceeded: SHIFT
             max_tx_weight_exceeded = true;
             should_shift  = true;
diff --git a/src/wallet/test/coinselection_tests.cpp b/src/wallet/test/coinselection_tests.cpp
index 8d308eb3eb1..3db85e2e044 100644
--- a/src/wallet/test/coinselection_tests.cpp
+++ b/src/wallet/test/coinselection_tests.cpp
@@ -132,9 +132,9 @@ BOOST_AUTO_TEST_CASE(bnb_test)
     // Simple success cases
     TestBnBSuccess("Select smallest UTXO", utxo_pool, /*selection_target=*/1 * CENT, /*expected_input_amounts=*/{1 * CENT}, /*expected_attempts=*/3);
     TestBnBSuccess("Select middle UTXO", utxo_pool, /*selection_target=*/3 * CENT, /*expected_input_amounts=*/{3 * CENT}, /*expected_attempts=*/3);
-    TestBnBSuccess("Select biggest UTXO", utxo_pool, /*selection_target=*/5 * CENT, /*expected_input_amounts=*/{5 * CENT}, /*expected_attempts=*/4);
+    TestBnBSuccess("Select biggest UTXO", utxo_pool, /*selection_target=*/5 * CENT, /*expected_input_amounts=*/{5 * CENT}, /*expected_attempts=*/2);
     TestBnBSuccess("Select two UTXOs", utxo_pool, /*selection_target=*/4 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT}, /*expected_attempts=*/4);
-    TestBnBSuccess("Select all UTXOs", utxo_pool, /*selection_target=*/9 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT, 5 * CENT}, /*expected_attempts=*/7);
+    TestBnBSuccess("Select all UTXOs", utxo_pool, /*selection_target=*/9 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT, 5 * CENT}, /*expected_attempts=*/5);
 
     // BnB finds changeless solution while overshooting by up to cost_of_change
     TestBnBSuccess("Select upper bound", utxo_pool, /*selection_target=*/4 * CENT - default_cs_params.m_cost_of_change, /*expected_input_amounts=*/{1 * CENT, 3 * CENT}, /*expected_attempts=*/4);
@@ -182,11 +182,15 @@ BOOST_AUTO_TEST_CASE(bnb_test)
     }
     AddCoins(doppelganger_pool, doppelgangers);
     // Among up to 17 unique UTXOs of similar effective value we will find a solution composed of the eight smallest UTXOs
-    TestBnBSuccess("Combine smallest 8 of 17 unique UTXOs", doppelganger_pool, /*selection_target=*/8 * CENT, /*expected_input_amounts=*/expected_inputs, /*expected_attempts=*/65'535);
+    TestBnBSuccess("Combine smallest 8 of 17 unique UTXOs", doppelganger_pool, /*selection_target=*/8 * CENT, /*expected_input_amounts=*/expected_inputs, /*expected_attempts=*/51'765);
 
-    // Starting with 18 unique UTXOs of similar effective value we will not find the solution due to exceeding the attempt limit
+    // Among up to 18 unique UTXOs of similar effective value we will find a solution composed of the eight smallest UTXOs
     AddCoins(doppelganger_pool, {1 * CENT + default_cs_params.m_cost_of_change + 17});
-    TestBnBFail("Exhaust looking for smallest 8 of 18 unique UTXOs", doppelganger_pool, /*selection_target=*/8 * CENT);
+    TestBnBSuccess("Combine smallest 8 of 18 unique UTXOs", doppelganger_pool, /*selection_target=*/8 * CENT, /*expected_input_amounts=*/expected_inputs, /*expected_attempts=*/87'957);
+
+    // Starting with 19 unique UTXOs of similar effective value we will not find the solution due to exceeding the attempt limit
+    AddCoins(doppelganger_pool, {1 * CENT + default_cs_params.m_cost_of_change + 18});
+    TestBnBFail("Exhaust looking for smallest 8 of 19 unique UTXOs", doppelganger_pool, /*selection_target=*/8 * CENT);
 }
 
 BOOST_AUTO_TEST_CASE(bnb_feerate_sensitivity_test)
@@ -194,22 +198,22 @@ BOOST_AUTO_TEST_CASE(bnb_feerate_sensitivity_test)
     // Create sets of UTXOs with the same effective amounts at different feerates (but different absolute amounts)
     std::vector<OutputGroup> low_feerate_pool; // 5 sat/vB (default, and lower than long_term_feerate of 10 sat/vB)
     AddCoins(low_feerate_pool, {2 * CENT, 3 * CENT, 5 * CENT, 10 * CENT});
-    TestBnBSuccess("Select many inputs at low feerates", low_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{2 * CENT, 3 * CENT, 5 * CENT}, /*expected_attempts=*/8);
+    TestBnBSuccess("Select many inputs at low feerates", low_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{2 * CENT, 3 * CENT, 5 * CENT}, /*expected_attempts=*/6);
 
     CoinSelectionParams high_feerate_params = init_default_params();
     high_feerate_params.m_effective_feerate = CFeeRate{25'000};
     std::vector<OutputGroup> high_feerate_pool; // 25 sat/vB (greater than long_term_feerate of 10 sat/vB)
     AddCoins(high_feerate_pool, {2 * CENT, 3 * CENT, 5 * CENT, 10 * CENT}, high_feerate_params);
-    TestBnBSuccess("Select one input at high feerates", high_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{10 * CENT}, /*expected_attempts=*/7, high_feerate_params);
+    TestBnBSuccess("Select one input at high feerates", high_feerate_pool, /*selection_target=*/10 * CENT, /*expected_input_amounts=*/{10 * CENT}, /*expected_attempts=*/5, high_feerate_params);
 
     // Add heavy inputs {6, 7} to existing {2, 3, 5, 10}
     low_feerate_pool.push_back(MakeCoin(6 * CENT, true, default_cs_params, /*custom_spending_vsize=*/500));
     low_feerate_pool.push_back(MakeCoin(7 * CENT, true, default_cs_params, /*custom_spending_vsize=*/500));
-    TestBnBSuccess("Prefer two heavy inputs over two light inputs at low feerates", low_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{6 * CENT, 7 * CENT}, /*expected_attempts=*/28, default_cs_params, /*custom_spending_vsize=*/500);
+    TestBnBSuccess("Prefer two heavy inputs over two light inputs at low feerates", low_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{6 * CENT, 7 * CENT}, /*expected_attempts=*/18, default_cs_params, /*custom_spending_vsize=*/500);
 
     high_feerate_pool.push_back(MakeCoin(6 * CENT, true, high_feerate_params, /*custom_spending_vsize=*/500));
     high_feerate_pool.push_back(MakeCoin(7 * CENT, true, high_feerate_params, /*custom_spending_vsize=*/500));
-    TestBnBSuccess("Prefer two light inputs over two heavy inputs at high feerates", high_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{3 * CENT, 10 * CENT}, /*expected_attempts=*/15, high_feerate_params);
+    TestBnBSuccess("Prefer two light inputs over two heavy inputs at high feerates", high_feerate_pool, /*selection_target=*/13 * CENT, /*expected_input_amounts=*/{3 * CENT, 10 * CENT}, /*expected_attempts=*/9, high_feerate_params);
 }
 
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 08593c4de6e..c69dab4ef81 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -303,7 +303,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         add_coin(3 * CENT, 2, expected_result);
         BOOST_CHECK(EquivalentResult(expected_result, *res));
         // Demonstrate how following improvements reduce iteration count and catch any regressions in the future.
-        expected_attempts = 39;
+        expected_attempts = 25;
         BOOST_CHECK_MESSAGE(res->GetSelectionsEvaluated() == expected_attempts, strprintf("Expected %i attempts, but got %i", expected_attempts, res->GetSelectionsEvaluated()));
     }
 }

From b8ebcb039d11662a0d77c7c57bc9abade155d28a Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Wed, 26 Mar 2025 18:46:21 -0700
Subject: [PATCH 13/14] opt: Skip evaluation of equivalent input sets

When two successive UTXOs match in effective value and weight, we can
skip the second if the prior is not selected: adding it would create an
equivalent input set to a previously evaluated.

E.g. if we have three UTXOs with effective values {5, 3, 3} of the same
weight each, we want to evaluate
{5, _, _}, {5, 3, _}, {5, 3, 3}, {_, 3, _}, {_, 3, 3},
but skip {5, _, 3}, and {_, _, 3}, because the first 3 is not selected,
and we therefore do not need to evaluate the second 3 at the same
position in the input set.

If we reach the end of the branch, we must SHIFT the previously selected
UTXO group instead.
---
 src/wallet/coinselection.cpp            | 23 ++++++++++++++++++++---
 src/wallet/test/coinselection_tests.cpp |  2 +-
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 7ec4154decc..e090ff0d29f 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -70,7 +70,7 @@ struct {
  *
  * The algorithm uses two additional optimizations. A lookahead keeps track of the total value of
  * the unexplored UTXOs. A subtree is not explored if the lookahead indicates that the target range
- * cannot be reached. Further, it is unnecessary to test equivalent combinations (tk). This allows us
+ * cannot be reached. Further, it is unnecessary to test equivalent combinations. This allows us
  * to skip testing the inclusion of UTXOs that match the effective value and waste of an omitted
  * predecessor.
  *
@@ -143,8 +143,9 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
 
     size_t curr_try = 0;
     SelectionResult result(selection_target, SelectionAlgorithm::BNB);
+    bool is_done = false;
     bool is_feerate_high = utxo_pool.at(0).fee > utxo_pool.at(0).long_term_fee;
-    while (true) {
+    while (!is_done) {
         bool should_shift{false}, should_cut{false};
         // Select `next_utxo`
         OutputGroup& utxo = utxo_pool[next_utxo];
@@ -203,16 +204,32 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
             should_shift  = true;
         }
 
-        if (should_shift) {
+        while (should_shift) {
             // Set `next_utxo` to one after last selected, then deselect last selected UTXO
             if (curr_selection.empty()) {
                 // Exhausted search space before running into attempt limit
+                is_done = true;
                 result.SetAlgoCompleted(true);
                 break;
             }
             next_utxo = curr_selection.back() + 1;
             deselect_last();
             should_shift  = false;
+
+            // After SHIFTing to an omission branch, the `next_utxo` might have the same value and same weight as the
+            // UTXO we just omitted (i.e. it is a "clone"). If so, selecting `next_utxo` would produce an equivalent
+            // selection as one we previously evaluated. In that case, increment `next_utxo` until we find a UTXO with a
+            // differing amount or weight.
+            while (utxo_pool[next_utxo - 1].GetSelectionAmount() == utxo_pool[next_utxo].GetSelectionAmount()
+                    && utxo_pool[next_utxo - 1].m_weight == utxo_pool[next_utxo].m_weight) {
+                if (next_utxo >= utxo_pool.size() - 1) {
+                    // Reached end of UTXO pool skipping clones: SHIFT instead
+                    should_shift = true;
+                    break;
+                }
+                // Skip clone: previous UTXO is equivalent and unselected
+                ++next_utxo;
+            }
         }
     }
 
diff --git a/src/wallet/test/coinselection_tests.cpp b/src/wallet/test/coinselection_tests.cpp
index 3db85e2e044..882d86b6498 100644
--- a/src/wallet/test/coinselection_tests.cpp
+++ b/src/wallet/test/coinselection_tests.cpp
@@ -151,7 +151,7 @@ BOOST_AUTO_TEST_CASE(bnb_test)
     std::vector<OutputGroup> clone_pool;
     AddCoins(clone_pool, {2 * CENT, 7 * CENT, 7 * CENT});
     AddDuplicateCoins(clone_pool, 50'000, 5 * CENT);
-    TestBnBSuccess("Skip equivalent input sets", clone_pool, /*selection_target=*/16 * CENT, /*expected_input_amounts=*/{2 * CENT, 7 * CENT, 7 * CENT}, /*expected_attempts=*/100'000);
+    TestBnBSuccess("Skip equivalent input sets", clone_pool, /*selection_target=*/16 * CENT, /*expected_input_amounts=*/{2 * CENT, 7 * CENT, 7 * CENT}, /*expected_attempts=*/16);
 
     /* Test BnB attempt limit (`TOTAL_TRIES`)
      *

From 44bab80284f3710ee30805ec3c5da06dd06d4d71 Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Wed, 26 Mar 2025 18:59:30 -0700
Subject: [PATCH 14/14] opt: Skip UTXOs with worse waste, same eff_value

When two successive UTXOs differ in waste but match in effective value,
we can skip the second if the first is not selected, because all input
sets we can generate by swapping out a less wasteful UTXOs with a more
wastefull UTXO of matching effective value would be strictly worse.
---
 src/wallet/coinselection.cpp           | 11 ++++++-----
 src/wallet/test/coinselector_tests.cpp |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index e090ff0d29f..c072afae143 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -216,12 +216,13 @@ util::Result<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool
             deselect_last();
             should_shift  = false;
 
-            // After SHIFTing to an omission branch, the `next_utxo` might have the same value and same weight as the
-            // UTXO we just omitted (i.e. it is a "clone"). If so, selecting `next_utxo` would produce an equivalent
+            // After SHIFTing to an omission branch, the `next_utxo` might have the same effective value as the
+            // UTXO we just omitted. Since lower waste is our tiebreaker on UTXOs with equal effective value for sorting, if it
+            // ties on the effective value, it _must_ have the same waste (i.e. be a "clone" of the prior UTXO) or a
+            // higher waste.  If so, selecting `next_utxo` would produce an equivalent or worse
             // selection as one we previously evaluated. In that case, increment `next_utxo` until we find a UTXO with a
-            // differing amount or weight.
-            while (utxo_pool[next_utxo - 1].GetSelectionAmount() == utxo_pool[next_utxo].GetSelectionAmount()
-                    && utxo_pool[next_utxo - 1].m_weight == utxo_pool[next_utxo].m_weight) {
+            // differing amount.
+            while (utxo_pool[next_utxo - 1].GetSelectionAmount() == utxo_pool[next_utxo].GetSelectionAmount()) {
                 if (next_utxo >= utxo_pool.size() - 1) {
                     // Reached end of UTXO pool skipping clones: SHIFT instead
                     should_shift = true;
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index c69dab4ef81..bd72a994496 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -303,7 +303,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         add_coin(3 * CENT, 2, expected_result);
         BOOST_CHECK(EquivalentResult(expected_result, *res));
         // Demonstrate how following improvements reduce iteration count and catch any regressions in the future.
-        expected_attempts = 25;
+        expected_attempts = 22;
         BOOST_CHECK_MESSAGE(res->GetSelectionsEvaluated() == expected_attempts, strprintf("Expected %i attempts, but got %i", expected_attempts, res->GetSelectionsEvaluated()));
     }
 }