From b3f4e827378e010cd2a5d1b876d01db52c054d26 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Wed, 20 Jul 2022 00:19:06 -0300
Subject: [PATCH 1/2] wallet: simplify ListCoins implementation

Can remove the locked coins lookup if we include them directly
inside the AvailableCoins result
---
 src/wallet/spend.cpp | 34 ++++++++--------------------------
 1 file changed, 8 insertions(+), 26 deletions(-)

diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 4d2182e079f..28f939f0fd3 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -392,37 +392,19 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet)
 
     std::map<CTxDestination, std::vector<COutput>> result;
 
-    for (COutput& coin : AvailableCoinsListUnspent(wallet).All()) {
+    CCoinControl coin_control;
+    // Include watch-only for LegacyScriptPubKeyMan wallets without private keys
+    coin_control.fAllowWatchOnly = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
+    CoinFilterParams coins_params;
+    coins_params.only_spendable = false;
+    coins_params.skip_locked = false;
+    for (const COutput& coin : AvailableCoins(wallet, &coin_control, /*feerate=*/std::nullopt, coins_params).All()) {
         CTxDestination address;
         if ((coin.spendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.solvable)) &&
             ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) {
-            result[address].emplace_back(std::move(coin));
+            result[address].emplace_back(coin);
         }
     }
-
-    std::vector<COutPoint> lockedCoins;
-    wallet.ListLockedCoins(lockedCoins);
-    // Include watch-only for LegacyScriptPubKeyMan wallets without private keys
-    const bool include_watch_only = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
-    const isminetype is_mine_filter = include_watch_only ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
-    for (const COutPoint& output : lockedCoins) {
-        auto it = wallet.mapWallet.find(output.hash);
-        if (it != wallet.mapWallet.end()) {
-            const auto& wtx = it->second;
-            int depth = wallet.GetTxDepthInMainChain(wtx);
-            if (depth >= 0 && output.n < wtx.tx->vout.size() &&
-                wallet.IsMine(wtx.tx->vout[output.n]) == is_mine_filter
-            ) {
-                CTxDestination address;
-                if (ExtractDestination(FindNonChangeParentOutput(wallet, *wtx.tx, output.n).scriptPubKey, address)) {
-                    const auto out = wtx.tx->vout.at(output.n);
-                    result[address].emplace_back(
-                            COutPoint(wtx.GetHash(), output.n), out, depth, CalculateMaximumSignedInputSize(out, &wallet, /*coin_control=*/nullptr), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ false, wtx.GetTxTime(), CachedTxIsFromMe(wallet, wtx, ISMINE_ALL));
-                }
-            }
-        }
-    }
-
     return result;
 }
 

From a2ac6f9582c4c996fa36e4801fa0aac756235754 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Wed, 20 Jul 2022 13:24:05 -0300
Subject: [PATCH 2/2] wallet: unify FindNonChangeParentOutput functions

The function is only used in ListCoins.
---
 src/wallet/spend.cpp | 34 +++++++++++++++-------------------
 src/wallet/spend.h   |  1 -
 2 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 28f939f0fd3..0e573badc23 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -362,28 +362,24 @@ CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinContr
     return AvailableCoins(wallet, coinControl).GetTotalAmount();
 }
 
-const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const CTransaction& tx, int output)
-{
-    AssertLockHeld(wallet.cs_wallet);
-    const CTransaction* ptx = &tx;
-    int n = output;
-    while (OutputIsChange(wallet, ptx->vout[n]) && ptx->vin.size() > 0) {
-        const COutPoint& prevout = ptx->vin[0].prevout;
-        auto it = wallet.mapWallet.find(prevout.hash);
-        if (it == wallet.mapWallet.end() || it->second.tx->vout.size() <= prevout.n ||
-            !wallet.IsMine(it->second.tx->vout[prevout.n])) {
-            break;
-        }
-        ptx = it->second.tx.get();
-        n = prevout.n;
-    }
-    return ptx->vout[n];
-}
-
 const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const COutPoint& outpoint)
 {
     AssertLockHeld(wallet.cs_wallet);
-    return FindNonChangeParentOutput(wallet, *wallet.GetWalletTx(outpoint.hash)->tx, outpoint.n);
+    const CWalletTx* wtx{Assert(wallet.GetWalletTx(outpoint.hash))};
+
+    const CTransaction* ptx = wtx->tx.get();
+    int n = outpoint.n;
+    while (OutputIsChange(wallet, ptx->vout[n]) && ptx->vin.size() > 0) {
+        const COutPoint& prevout = ptx->vin[0].prevout;
+        const CWalletTx* it = wallet.GetWalletTx(prevout.hash);
+        if (!it || it->tx->vout.size() <= prevout.n ||
+            !wallet.IsMine(it->tx->vout[prevout.n])) {
+            break;
+        }
+        ptx = it->tx.get();
+        n = prevout.n;
+    }
+    return ptx->vout[n];
 }
 
 std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet)
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index 281a6ca9e81..14a057b1ea5 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -99,7 +99,6 @@ CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinContr
 /**
  * Find non-change parent output.
  */
-const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const CTransaction& tx, int output) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
 const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const COutPoint& outpoint) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
 
 /**