From 451be19dc10381122f705bbb2127b083f1d598c6 Mon Sep 17 00:00:00 2001
From: Murch <murch@murch.one>
Date: Mon, 8 Jan 2024 15:08:57 -0500
Subject: [PATCH] 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           | 21 +++++++++++++++++++--
 src/wallet/test/coinselector_tests.cpp |  6 +++---
 2 files changed, 22 insertions(+), 5 deletions(-)

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<SelectionResult> CoinGrinder(std::vector<OutputGroup>& 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<SelectionResult> CoinGrinder(std::vector<OutputGroup>& 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()));
     }