From 8c41fbcc8527ba3e6e47f346636cc2ccd2b78b18 Mon Sep 17 00:00:00 2001
From: Greg Pfeil <greg@electriccoin.co>
Date: Tue, 7 Mar 2023 18:31:59 -0700
Subject: [PATCH] Make pool selection order more flexible
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, all pools had to be listed in the selection order. This had a few problems
- if a pool was missing from selection, we’d `assert`
- a logic error could result in note selection from pools that we thought were not needed for
  selection (and thus might subvert our strategy re: hiding sender, fewer pools, opportunistic
  shielding, etc.)

Now, we only include pools that are intended to be part of the selection order, and all notes from
other pools are cleared before we do any selection.
---
 src/wallet/wallet.cpp | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index c289b0883..91e9d3418 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -8063,9 +8063,6 @@ bool SpendableInputs::LimitToAmount(
         // Fully shielded.
         selectionOrder = {
             OutputPool::Orchard,
-            // Pools below here are erased.
-            OutputPool::Transparent,
-            OutputPool::Sapling,
         };
     } else if (
         recipientPools.count(OutputPool::Transparent) &&
@@ -8075,9 +8072,6 @@ bool SpendableInputs::LimitToAmount(
         // Fewer pools.
         selectionOrder = {
             OutputPool::Orchard,
-            // Pools below here are erased.
-            OutputPool::Transparent,
-            OutputPool::Sapling,
         };
     } else if (wouldSuffice(availableSapling + availableOrchard)) {
         // Hide sender.
@@ -8087,8 +8081,6 @@ bool SpendableInputs::LimitToAmount(
         selectionOrder = {
             OutputPool::Sapling,
             OutputPool::Orchard,
-            // Pools below here are erased.
-            OutputPool::Transparent,
         };
     } else {
         // Opportunistic shielding.
@@ -8100,14 +8092,25 @@ bool SpendableInputs::LimitToAmount(
         opportunisticShielding = true;
     }
 
-    // Ensure we provided a total selection order (so that all unselected notes
-    // and coins are erased).
+    // Erase all notes and coins from pools that aren’t used in selection.
     for (auto pool : available) {
         bool poolIsPresent = false;
         for (auto entry : selectionOrder) {
             poolIsPresent |= entry == pool;
         }
-        assert(poolIsPresent);
+        if (!poolIsPresent) {
+            switch (pool) {
+                case OutputPool::Transparent:
+                    utxos.clear();
+                    break;
+                case OutputPool::Sapling:
+                    saplingNoteEntries.clear();
+                    break;
+                case OutputPool::Orchard:
+                    orchardNoteMetadata.clear();
+                    break;
+            }
+        }
     }
 
     // Finally, select the remaining notes and coins based on this order.