From c4e3b7d6a154e82cdb902fd7bcb7b725aebde5ea Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Tue, 22 Nov 2022 11:51:33 -0300
Subject: [PATCH] wallet: SelectCoins, return early if wallet's UTXOs cannot
 cover the target

The CoinsResult class will now count the raw total amount and the effective
total amount internally (inside the 'CoinsResult::Add' and 'CoinsResult::Erase'
methods).
So there is no discrepancy between what we add/erase and the total values.
(which is what was happening on the coinselector_test because the 'CoinsResult'
object is manually created there, and we were not keeping the total amount
in sync with the outputs being added/removed).
---
 src/wallet/coinselection.h |  2 ++
 src/wallet/spend.cpp       | 23 ++++++++++++++++++++---
 src/wallet/spend.h         |  4 +++-
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h
index b23dd10867f..b02e0064355 100644
--- a/src/wallet/coinselection.h
+++ b/src/wallet/coinselection.h
@@ -110,6 +110,8 @@ public:
         assert(effective_value.has_value());
         return effective_value.value();
     }
+
+    bool HasEffectiveValue() const { return effective_value.has_value(); }
 };
 
 /** Parameters for one iteration of Coin Selection. */
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 5f4e3e79c72..19f9048dbfc 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -106,7 +106,13 @@ void CoinsResult::Erase(const std::unordered_set<COutPoint, SaltedOutpointHasher
 {
     for (auto& [type, vec] : coins) {
         auto remove_it = std::remove_if(vec.begin(), vec.end(), [&](const COutput& coin) {
-            return coins_to_remove.count(coin.outpoint) == 1;
+            // remove it if it's on the set
+            if (coins_to_remove.count(coin.outpoint) == 0) return false;
+
+            // update cached amounts
+            total_amount -= coin.txout.nValue;
+            if (coin.HasEffectiveValue()) total_effective_amount = *total_effective_amount - coin.GetEffectiveValue();
+            return true;
         });
         vec.erase(remove_it, vec.end());
     }
@@ -122,6 +128,11 @@ void CoinsResult::Shuffle(FastRandomContext& rng_fast)
 void CoinsResult::Add(OutputType type, const COutput& out)
 {
     coins[type].emplace_back(out);
+    total_amount += out.txout.nValue;
+    if (out.HasEffectiveValue()) {
+        total_effective_amount = total_effective_amount.has_value() ?
+                *total_effective_amount + out.GetEffectiveValue() : out.GetEffectiveValue();
+    }
 }
 
 static OutputType GetOutputType(TxoutType type, bool is_from_p2sh)
@@ -319,8 +330,6 @@ CoinsResult AvailableCoins(const CWallet& wallet,
             result.Add(GetOutputType(type, is_from_p2sh),
                        COutput(outpoint, output, nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me, feerate));
 
-            // Cache total amount as we go
-            result.total_amount += output.nValue;
             // Checks the sum amount of all UTXO's.
             if (params.min_sum_amount != MAX_MONEY) {
                 if (result.total_amount >= params.min_sum_amount) {
@@ -575,6 +584,14 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
         return result;
     }
 
+    // Return early if we cannot cover the target with the wallet's UTXO.
+    // We use the total effective value if we are not subtracting fee from outputs and 'available_coins' contains the data.
+    CAmount available_coins_total_amount = coin_selection_params.m_subtract_fee_outputs ? available_coins.total_amount :
+            (available_coins.total_effective_amount.has_value() ? *available_coins.total_effective_amount : 0);
+    if (selection_target > available_coins_total_amount) {
+        return std::nullopt; // Insufficient funds
+    }
+
     // Start wallet Coin Selection procedure
     auto op_selection_result = AutomaticCoinSelection(wallet, available_coins, selection_target, coin_control, coin_selection_params);
     if (!op_selection_result) return op_selection_result;
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index df57fc13621..8612ce49adc 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -51,8 +51,10 @@ struct CoinsResult {
     void Shuffle(FastRandomContext& rng_fast);
     void Add(OutputType type, const COutput& out);
 
-    /** Sum of all available coins */
+    /** Sum of all available coins raw value */
     CAmount total_amount{0};
+    /** Sum of all available coins effective value (each output value minus fees required to spend it) */
+    std::optional<CAmount> total_effective_amount{0};
 };
 
 struct CoinFilterParams {