From 162d4ad10f28c5fa38551d69ce9b296ab3933c77 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Thu, 2 Jun 2022 14:45:04 -0300
Subject: [PATCH] wallet: add 'only_spendable' filter to AvailableCoins
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We are skipping the non-spendable coins that appear in vCoins ('AvailableCoins' result) later, in several parts of the CreateTransaction and GetBalance flows:

GetAvailableBalance (1) gets all the available coins calling AvailableCoins and, right away, walk through the entire vector, skipping the non-spendable coins, to calculate the total balance.

Inside CreateTransactionInternal —> SelectCoins(vCoins,...), we have several calls to AttemptSelection which, on each of them internally, we call twice to GroupOutputs which internally has two for-loops over the entire vCoins vector that skip the non-spendable coins.

So, Purpose is not add the non-spendable coins into the AvailableCoins result (vCoins) in the first place for the processes that aren’t using them at all, so we don’t waste resources skipping them later so many times.

Note: this speedup is for all the processes that call to CreateTransaction and GetBalance* internally.
---
 src/wallet/spend.cpp | 17 ++++++++++++++---
 src/wallet/spend.h   | 10 +++++++++-
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index c598f2c9257..63e04c7a9b0 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -90,7 +90,8 @@ CoinsResult AvailableCoins(const CWallet& wallet,
                            const CAmount& nMinimumAmount,
                            const CAmount& nMaximumAmount,
                            const CAmount& nMinimumSumAmount,
-                           const uint64_t nMaximumCount)
+                           const uint64_t nMaximumCount,
+                           bool only_spendable)
 {
     AssertLockHeld(wallet.cs_wallet);
 
@@ -199,6 +200,10 @@ CoinsResult AvailableCoins(const CWallet& wallet,
 
             bool solvable = provider ? IsSolvable(*provider, output.scriptPubKey) : false;
             bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable));
+
+            // Filter by spendable outputs only
+            if (!spendable && only_spendable) continue;
+
             int input_bytes = GetTxSpendSize(wallet, wtx, i, (coinControl && coinControl->fAllowWatchOnly));
             result.coins.emplace_back(outpoint, output, nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me, feerate);
 
@@ -223,7 +228,7 @@ CoinsResult AvailableCoins(const CWallet& wallet,
 
 CoinsResult AvailableCoinsListUnspent(const CWallet& wallet, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount& nMinimumSumAmount, const uint64_t nMaximumCount)
 {
-    return AvailableCoins(wallet, coinControl, /*feerate=*/ std::nullopt, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount);
+    return AvailableCoins(wallet, coinControl, /*feerate=*/ std::nullopt, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount, /*only_spendable=*/false);
 }
 
 CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinControl)
@@ -796,7 +801,13 @@ static std::optional<CreatedTransactionResult> CreateTransactionInternal(
     CAmount selection_target = recipients_sum + not_input_fees;
 
     // Get available coins
-    auto res_available_coins = AvailableCoins(wallet, &coin_control, coin_selection_params.m_effective_feerate, 1, MAX_MONEY, MAX_MONEY, 0);
+    auto res_available_coins = AvailableCoins(wallet,
+                                              &coin_control,
+                                              coin_selection_params.m_effective_feerate,
+                                              1,            /*nMinimumAmount*/
+                                              MAX_MONEY,    /*nMaximumAmount*/
+                                              MAX_MONEY,    /*nMinimumSumAmount*/
+                                              0);           /*nMaximumCount*/
 
     // Choose coins to use
     std::optional<SelectionResult> result = SelectCoins(wallet, res_available_coins.coins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params);
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index dde2523eb7b..84628517b00 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -39,8 +39,16 @@ struct CoinsResult {
 };
 /**
  * Return vector of available COutputs.
+ * By default, returns only the spendable coins.
  */
-CoinsResult AvailableCoins(const CWallet& wallet, const CCoinControl* coinControl = nullptr, std::optional<CFeeRate> feerate = std::nullopt, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
+CoinsResult AvailableCoins(const CWallet& wallet,
+                           const CCoinControl* coinControl = nullptr,
+                           std::optional<CFeeRate> feerate = std::nullopt,
+                           const CAmount& nMinimumAmount = 1,
+                           const CAmount& nMaximumAmount = MAX_MONEY,
+                           const CAmount& nMinimumSumAmount = MAX_MONEY,
+                           const uint64_t nMaximumCount = 0,
+                           bool only_spendable = true) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
 
 /**
  * Wrapper function for AvailableCoins which skips the `feerate` parameter. Use this function