From b5e91e946c8a07d5a2915d067eeae68473b24d24 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Wed, 3 Jun 2026 21:19:05 +0200 Subject: [PATCH 1/3] wallet: Remove CoinsResult::Clear() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead we fully reset CoinsResult-instances without forgetting any fields. Fixes: https://github.com/bitcoin/bitcoin/issues/35449 Variant resetting the other fields in Clear(): ₿ hyperfine --warmup 2 "./build/bin/test_bitcoin -t coinselector_tests/knapsack_solver_test" Benchmark 1: ./build/bin/test_bitcoin -t coinselector_tests/knapsack_solver_test Time (mean ± σ): 4.289 s ± 0.008 s [User: 4.252 s, System: 0.027 s] Range (min … max): 4.279 s … 4.301 s 10 runs This variant: ₿ hyperfine --warmup 2 "./build/bin/test_bitcoin -t coinselector_tests/knapsack_solver_test" Benchmark 1: ./build/bin/test_bitcoin -t coinselector_tests/knapsack_solver_test Time (mean ± σ): 4.279 s ± 0.005 s [User: 4.244 s, System: 0.026 s] Range (min … max): 4.271 s … 4.287 s 10 runs --- src/wallet/spend.cpp | 4 ---- src/wallet/spend.h | 3 +-- src/wallet/test/coinselector_tests.cpp | 20 ++++++++++---------- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 9384299d9a1..dba7b882959 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -210,10 +210,6 @@ std::vector CoinsResult::All() const return all; } -void CoinsResult::Clear() { - coins.clear(); -} - void CoinsResult::Erase(const std::unordered_set& coins_to_remove) { for (auto& [type, vec] : coins) { diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 925111dce03..b013a3fabdf 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -40,7 +40,7 @@ TxSize CalculateMaximumSignedTxSize(const CTransaction& tx, const CWallet* walle * This struct is really just a wrapper around OutputType vectors with a convenient * method for concatenating and returning all COutputs as one vector. * - * Size(), Clear(), Erase(), Shuffle(), and Add() methods are implemented to + * Size(), Erase(), Shuffle(), and Add() methods are implemented to * allow easy interaction with the struct. */ struct CoinsResult { @@ -54,7 +54,6 @@ struct CoinsResult { size_t Size() const; /** Return how many different output types this struct stores */ size_t TypesCount() const { return coins.size(); } - void Clear(); void Erase(const std::unordered_set& coins_to_remove); void Shuffle(FastRandomContext& rng_fast); void Add(OutputType type, const COutput& out); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index ee6a639a43d..5713466e418 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -202,7 +202,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) BOOST_CHECK(!SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change)); // Test fees subtracted from output: - available_coins.Clear(); + available_coins = {}; add_coin(available_coins, *wallet, 1 * CENT, coin_selection_params_bnb.m_effective_feerate); available_coins.All().at(0).input_bytes = 40; const auto result9 = SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change); @@ -343,7 +343,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // test multiple times to allow for differences in the shuffle order for (int i = 0; i < RUN_TESTS; i++) { - available_coins.Clear(); + available_coins = {}; // with an empty wallet we can't even pay one cent BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_standard), 1 * CENT, CENT)); @@ -412,7 +412,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) BOOST_CHECK_EQUAL(result8->GetInputSet().size(), 1U); // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin - available_coins.Clear(); + available_coins = {}; add_coin(available_coins, *wallet, 6*CENT); add_coin(available_coins, *wallet, 7*CENT); @@ -470,7 +470,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // empty the wallet and start again, now with fractions of a cent, to test small change avoidance - available_coins.Clear(); + available_coins = {}; add_coin(available_coins, *wallet, CENT * 1 / 10); add_coin(available_coins, *wallet, CENT * 2 / 10); add_coin(available_coins, *wallet, CENT * 3 / 10); @@ -502,7 +502,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // run the 'mtgox' test (see https://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change - available_coins.Clear(); + available_coins = {}; for (int j = 0; j < 20; j++) add_coin(available_coins, *wallet, 50000 * COIN); @@ -515,7 +515,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // we need to try finding an exact subset anyway // sometimes it will fail, and so we use the next biggest coin: - available_coins.Clear(); + available_coins = {}; add_coin(available_coins, *wallet, CENT * 5 / 10); add_coin(available_coins, *wallet, CENT * 6 / 10); add_coin(available_coins, *wallet, CENT * 7 / 10); @@ -526,7 +526,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) BOOST_CHECK_EQUAL(result20->GetInputSet().size(), 1U); // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) - available_coins.Clear(); + available_coins = {}; add_coin(available_coins, *wallet, CENT * 4 / 10); add_coin(available_coins, *wallet, CENT * 6 / 10); add_coin(available_coins, *wallet, CENT * 8 / 10); @@ -537,7 +537,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) BOOST_CHECK_EQUAL(result21->GetInputSet().size(), 2U); // in two coins 0.4+0.6 // test avoiding small change - available_coins.Clear(); + available_coins = {}; add_coin(available_coins, *wallet, CENT * 5 / 100); add_coin(available_coins, *wallet, CENT * 1); add_coin(available_coins, *wallet, CENT * 100); @@ -557,7 +557,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // test with many inputs for (CAmount amt=1500; amt < COIN; amt*=10) { - available_coins.Clear(); + available_coins = {}; // Create 676 inputs (= (old MAX_STANDARD_TX_SIZE == 100000) / 148 bytes per input) for (uint16_t j = 0; j < 676; j++) add_coin(available_coins, *wallet, amt); @@ -583,7 +583,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // test randomness { - available_coins.Clear(); + available_coins = {}; for (int i2 = 0; i2 < 100; i2++) add_coin(available_coins, *wallet, COIN); From 43ca54ca000e66ebf20e8f537f0addad02a3dfbc Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Wed, 3 Jun 2026 22:06:21 +0200 Subject: [PATCH 2/3] refactor(test): Make CAmount arg explicit for BuildCreditingTransaction() --- src/test/util/transaction_utils.cpp | 2 +- src/test/util/transaction_utils.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/util/transaction_utils.cpp b/src/test/util/transaction_utils.cpp index b65a9568c65..cf4ed934669 100644 --- a/src/test/util/transaction_utils.cpp +++ b/src/test/util/transaction_utils.cpp @@ -7,7 +7,7 @@ #include