diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index fb63e92bb8d..aca81353ef5 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -401,8 +401,9 @@ util::Result CoinGrinder(std::vector& utxo_pool, c }; SelectionResult result(selection_target, SelectionAlgorithm::CG); + bool is_done = false; size_t curr_try = 0; - while (true) { + while (!is_done) { bool should_shift{false}, should_cut{false}; // Select `next_utxo` OutputGroup& utxo = utxo_pool[next_utxo]; @@ -454,16 +455,32 @@ util::Result CoinGrinder(std::vector& utxo_pool, c 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/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index c335a32ec66..e1060cdfcc0 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -1180,7 +1180,7 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests) }); BOOST_CHECK(res); // Demonstrate how following improvements reduce iteration count and catch any regressions in the future. - size_t expected_attempts = 100'000; + size_t expected_attempts = 184; BOOST_CHECK_MESSAGE(res->GetSelectionsEvaluated() == expected_attempts, strprintf("Expected %i attempts, but got %i", expected_attempts, res->GetSelectionsEvaluated())); } @@ -1202,7 +1202,7 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests) add_coin(1 * COIN, 2, expected_result); BOOST_CHECK(EquivalentResult(expected_result, *res)); // Demonstrate how following improvements reduce iteration count and catch any regressions in the future. - size_t expected_attempts = 4; + size_t expected_attempts = 3; BOOST_CHECK_MESSAGE(res->GetSelectionsEvaluated() == expected_attempts, strprintf("Expected %i attempts, but got %i", expected_attempts, res->GetSelectionsEvaluated())); } @@ -1271,7 +1271,7 @@ BOOST_AUTO_TEST_CASE(coin_grinder_tests) BOOST_CHECK(EquivalentResult(expected_result, *res)); // Demonstrate how following improvements reduce iteration count and catch any regressions in the future. // If this takes more attempts, the implementation has regressed - size_t expected_attempts = 82'407; + size_t expected_attempts = 43; BOOST_CHECK_MESSAGE(res->GetSelectionsEvaluated() == expected_attempts, strprintf("Expected %i attempts, but got %i", expected_attempts, res->GetSelectionsEvaluated())); }