From 2bafc462610cf5e6fd901706bbaedaf3cf35b2fd Mon Sep 17 00:00:00 2001 From: Murch Date: Fri, 1 Mar 2024 14:29:13 -0500 Subject: [PATCH 1/7] test: Recreate simple BnB success tests Recreates the tests in a new test suite coinselection_tests.cpp that is based on UTXOs being created per their effective values rather than nominal values and uses transactions with non-zero feerates. --- src/wallet/test/CMakeLists.txt | 1 + src/wallet/test/coinselection_tests.cpp | 122 ++++++++++++++++++++++++ src/wallet/test/coinselector_tests.cpp | 45 --------- 3 files changed, 123 insertions(+), 45 deletions(-) create mode 100644 src/wallet/test/coinselection_tests.cpp 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 new file mode 100644 index 00000000000..f211e507528 --- /dev/null +++ b/src/wallet/test/coinselection_tests.cpp @@ -0,0 +1,122 @@ +// 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 +#include +#include +#include + +#include + +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(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& utxo_pool, std::vector 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 a_amts; + std::vector 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(); +} + +static std::string InputAmountsToString(const SelectionResult& selection) +{ + return "[" + util::Join(selection.GetInputSet(), " ", [](const auto& input){ return util::ToString(input->txout.nValue);}) + "]"; +} + +static void TestBnBSuccess(std::string test_title, std::vector& utxo_pool, const CAmount& selection_target, const std::vector& 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, InputAmountsToString(expected_result), InputAmountsToString(*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 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 fbe48a9cdfb..ad872de192a 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 7db6f012c08c796645c38b87d35e89cf6c18ed3b Mon Sep 17 00:00:00 2001 From: Murch Date: Fri, 1 Mar 2024 15:06:50 -0500 Subject: [PATCH 2/7] 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 f211e507528..392caabe2da 100644 --- a/src/wallet/test/coinselection_tests.cpp +++ b/src/wallet/test/coinselection_tests.cpp @@ -86,12 +86,12 @@ static std::string InputAmountsToString(const SelectionResult& selection) return "[" + util::Join(selection.GetInputSet(), " ", [](const auto& input){ return util::ToString(input->txout.nValue);}) + "]"; } -static void TestBnBSuccess(std::string test_title, std::vector& utxo_pool, const CAmount& selection_target, const std::vector& expected_input_amounts, const CoinSelectionParams& cs_params = default_cs_params) +static void TestBnBSuccess(std::string test_title, std::vector& utxo_pool, const CAmount& selection_target, const std::vector& 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); } @@ -118,5 +118,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 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 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 ad872de192a..7fb88f5f9de 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 a94030ae985bb6c9be98cf8258394dd50d34323a Mon Sep 17 00:00:00 2001 From: Murch Date: Thu, 27 Jun 2024 11:50:15 -0400 Subject: [PATCH 3/7] 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 392caabe2da..5fb61a8e798 100644 --- a/src/wallet/test/coinselection_tests.cpp +++ b/src/wallet/test/coinselection_tests.cpp @@ -62,6 +62,13 @@ static void AddCoins(std::vector& utxo_pool, std::vector c } } +/** Make multiple coins that share the same effective value */ +static void AddDuplicateCoins(std::vector& 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) @@ -116,6 +123,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 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 7fb88f5f9de..65226b45199 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 4781f5c8be55e41fb8144cdec6ec1989a5db72ae Mon Sep 17 00:00:00 2001 From: Murch Date: Fri, 1 Mar 2024 15:41:19 -0500 Subject: [PATCH 4/7] 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 5fb61a8e798..844fa22514e 100644 --- a/src/wallet/test/coinselection_tests.cpp +++ b/src/wallet/test/coinselection_tests.cpp @@ -109,9 +109,18 @@ static void TestBnBSuccess(std::string test_title, std::vector& 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& 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 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 @@ -124,6 +133,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 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 65226b45199..141664d1fb8 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 2a1b2754f14a2686e78547b2e8a51a4fb35e3816 Mon Sep 17 00:00:00 2001 From: Murch Date: Fri, 1 Mar 2024 15:42:08 -0500 Subject: [PATCH 5/7] 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 141664d1fb8..4e9c094ab8d 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 d610951c154663053a0e39a850dffd96f808581b Mon Sep 17 00:00:00 2001 From: Murch Date: Fri, 1 Mar 2024 15:50:16 -0500 Subject: [PATCH 6/7] test: Recreate BnB iteration exhaustion test --- src/wallet/test/coinselection_tests.cpp | 36 +++++++++++++++++++++++++ src/wallet/test/coinselector_tests.cpp | 32 ---------------------- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/src/wallet/test/coinselection_tests.cpp b/src/wallet/test/coinselection_tests.cpp index 844fa22514e..5ed5bdf0380 100644 --- a/src/wallet/test/coinselection_tests.cpp +++ b/src/wallet/test/coinselection_tests.cpp @@ -146,6 +146,42 @@ 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 doppelganger_pool; + std::vector doppelgangers; + std::vector 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 4e9c094ab8d..fd35d147b93 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& 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& 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& GroupCoins(const std::vector& available_coins, bool subtract_fee_outputs = false) { static std::vector static_groups; @@ -195,17 +174,6 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) std::vector 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 85368aafa0d1772626d5f615e272b1c1a580b42f Mon Sep 17 00:00:00 2001 From: Murch Date: Mon, 28 Apr 2025 20:36:55 -0700 Subject: [PATCH 7/7] test: Run simple tests at various feerates --- src/wallet/test/coinselection_tests.cpp | 124 +++++++++++++----------- 1 file changed, 66 insertions(+), 58 deletions(-) diff --git a/src/wallet/test/coinselection_tests.cpp b/src/wallet/test/coinselection_tests.cpp index 5ed5bdf0380..4e4b4b67aa1 100644 --- a/src/wallet/test/coinselection_tests.cpp +++ b/src/wallet/test/coinselection_tests.cpp @@ -63,9 +63,9 @@ static void AddCoins(std::vector& utxo_pool, std::vector c } /** Make multiple coins that share the same effective value */ -static void AddDuplicateCoins(std::vector& utxo_pool, int count, int amount) { +static void AddDuplicateCoins(std::vector& utxo_pool, int count, int amount, CoinSelectionParams cs_params = default_cs_params) { for (int i = 0 ; i < count; ++i) { - utxo_pool.push_back(MakeCoin(amount)); + utxo_pool.push_back(MakeCoin(amount, true, cs_params)); } } @@ -116,72 +116,80 @@ static void TestBnBFail(std::string test_title, std::vector& utxo_p BOOST_AUTO_TEST_CASE(bnb_test) { - std::vector utxo_pool; + std::vector feerates = {0, 1, 5'000, 10'000, 25'000, 59'764, 500'000, 999'000, 1'500'000}; - // Fail for empty UTXO pool - TestBnBFail("Empty UTXO pool", utxo_pool, /*selection_target=*/1 * CENT); + for (int feerate : feerates) { + std::vector utxo_pool; - AddCoins(utxo_pool, {1 * CENT, 3 * CENT, 5 * CENT}); + CoinSelectionParams cs_params = init_default_params(); + cs_params.m_effective_feerate = CFeeRate{feerate}; - // 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}); + // Fail for empty UTXO pool + TestBnBFail("Empty UTXO pool", utxo_pool, /*selection_target=*/1 * 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}); + AddCoins(utxo_pool, {1 * CENT, 3 * CENT, 5 * CENT}, cs_params); - // 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 success cases + TestBnBSuccess("Select smallest UTXO", utxo_pool, /*selection_target=*/1 * CENT, /*expected_input_amounts=*/{1 * CENT}, cs_params); + TestBnBSuccess("Select middle UTXO", utxo_pool, /*selection_target=*/3 * CENT, /*expected_input_amounts=*/{3 * CENT}, cs_params); + TestBnBSuccess("Select biggest UTXO", utxo_pool, /*selection_target=*/5 * CENT, /*expected_input_amounts=*/{5 * CENT}, cs_params); + TestBnBSuccess("Select two UTXOs", utxo_pool, /*selection_target=*/4 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT}, cs_params); + TestBnBSuccess("Select all UTXOs", utxo_pool, /*selection_target=*/9 * CENT, /*expected_input_amounts=*/{1 * CENT, 3 * CENT, 5 * CENT}, cs_params); - // 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); + // 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}, cs_params); - // Test skipping of equivalent input sets - std::vector 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}); + // 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); - /* 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 doppelganger_pool; - std::vector doppelgangers; - std::vector 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); + // 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 clone_pool; + AddCoins(clone_pool, {2 * CENT, 7 * CENT, 7 * CENT}, cs_params); + AddDuplicateCoins(clone_pool, 50'000, 5 * CENT, cs_params); + TestBnBSuccess("Skip equivalent input sets", clone_pool, /*selection_target=*/16 * CENT, /*expected_input_amounts=*/{2 * CENT, 7 * CENT, 7 * CENT}, cs_params); + + /* 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 doppelganger_pool; + std::vector doppelgangers; + std::vector 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); + AddCoins(doppelganger_pool, doppelgangers, cs_params); + // 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, cs_params); - // 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); + // 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}, cs_params); + TestBnBFail("Exhaust looking for smallest 8 of 18 unique UTXOs", doppelganger_pool, /*selection_target=*/8 * CENT); + } } BOOST_AUTO_TEST_CASE(bnb_feerate_sensitivity_test)