wallet: Use wallet's TXO set in AvailableCoins

Instead of iterating every transaction and every output stored in wallet
when trying to figure out what outputs can be spent, iterate the TXO set
which should be a lot smaller.

Author: achow101

src/wallet/spend.cpp

@@ -330,137 +330,146 @@ CoinsResult AvailableCoins(const CWallet& wallet,
     std::vector<COutPoint> outpoints;
 
     std::set<Txid> trusted_parents;
-    for (const auto& entry : wallet.mapWallet)
-    {
-        const Txid& txid = entry.first;
-        const CWalletTx& wtx = entry.second;
-
-        if (wallet.IsTxImmatureCoinBase(wtx) && !params.include_immature_coinbase)
-            continue;
-
-        int nDepth = wallet.GetTxDepthInMainChain(wtx);
-        if (nDepth < 0)
-            continue;
-
-        // We should not consider coins which aren't at least in our mempool
-        // It's possible for these to be conflicted via ancestors which we may never be able to detect
-        if (nDepth == 0 && !wtx.InMempool())
-            continue;
-
-        bool safeTx = CachedTxIsTrusted(wallet, wtx, trusted_parents);
-
-        // We should not consider coins from transactions that are replacing
-        // other transactions.
-        //
-        // Example: There is a transaction A which is replaced by bumpfee
-        // transaction B. In this case, we want to prevent creation of
-        // a transaction B' which spends an output of B.
-        //
-        // Reason: If transaction A were initially confirmed, transactions B
-        // and B' would no longer be valid, so the user would have to create
-        // a new transaction C to replace B'. However, in the case of a
-        // one-block reorg, transactions B' and C might BOTH be accepted,
-        // when the user only wanted one of them. Specifically, there could
-        // be a 1-block reorg away from the chain where transactions A and C
-        // were accepted to another chain where B, B', and C were all
-        // accepted.
-        if (nDepth == 0 && wtx.mapValue.count("replaces_txid")) {
-            safeTx = false;
-        }
-
-        // Similarly, we should not consider coins from transactions that
-        // have been replaced. In the example above, we would want to prevent
-        // creation of a transaction A' spending an output of A, because if
-        // transaction B were initially confirmed, conflicting with A and
-        // A', we wouldn't want to the user to create a transaction D
-        // intending to replace A', but potentially resulting in a scenario
-        // where A, A', and D could all be accepted (instead of just B and
-        // D, or just A and A' like the user would want).
-        if (nDepth == 0 && wtx.mapValue.count("replaced_by_txid")) {
-            safeTx = false;
-        }
-
-        if (only_safe && !safeTx) {
-            continue;
-        }
+    // Cache for whether each tx passes the tx level checks (first bool), and whether the transaction is "safe" (second bool)
+    std::unordered_map<uint256, std::pair<bool, bool>, SaltedTxidHasher> tx_safe_cache;
+    for (const auto& [outpoint, txo] : wallet.GetTXOs()) {
+        const CWalletTx& wtx = txo.GetWalletTx();
+        const CTxOut& output = txo.GetTxOut();
 
-        if (nDepth < min_depth || nDepth > max_depth) {
+        if (tx_safe_cache.contains(outpoint.hash) && !tx_safe_cache.at(outpoint.hash).first) {
             continue;
         }
 
-        bool tx_from_me = CachedTxIsFromMe(wallet, wtx, ISMINE_ALL);
+        int nDepth = wallet.GetTxDepthInMainChain(wtx);
 
-        for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) {
-            const CTxOut& output = wtx.tx->vout[i];
-            const COutPoint outpoint(txid, i);
+        // Perform tx level checks if we haven't already come across outputs from this tx before.
+        if (!tx_safe_cache.contains(outpoint.hash)) {
+            tx_safe_cache[outpoint.hash] = {false, false};
 
-            if (output.nValue < params.min_amount || output.nValue > params.max_amount)
+            if (wallet.IsTxImmatureCoinBase(wtx) && !params.include_immature_coinbase)
                 continue;
 
-            // Skip manually selected coins (the caller can fetch them directly)
-            if (coinControl && coinControl->HasSelected() && coinControl->IsSelected(outpoint))
+            if (nDepth < 0)
                 continue;
 
-            if (wallet.IsLockedCoin(outpoint) && params.skip_locked)
+            // We should not consider coins which aren't at least in our mempool
+            // It's possible for these to be conflicted via ancestors which we may never be able to detect
+            if (nDepth == 0 && !wtx.InMempool())
                 continue;
 
-            if (wallet.IsSpent(outpoint))
-                continue;
+            bool safeTx = CachedTxIsTrusted(wallet, wtx, trusted_parents);
+
+            // We should not consider coins from transactions that are replacing
+            // other transactions.
+            //
+            // Example: There is a transaction A which is replaced by bumpfee
+            // transaction B. In this case, we want to prevent creation of
+            // a transaction B' which spends an output of B.
+            //
+            // Reason: If transaction A were initially confirmed, transactions B
+            // and B' would no longer be valid, so the user would have to create
+            // a new transaction C to replace B'. However, in the case of a
+            // one-block reorg, transactions B' and C might BOTH be accepted,
+            // when the user only wanted one of them. Specifically, there could
+            // be a 1-block reorg away from the chain where transactions A and C
+            // were accepted to another chain where B, B', and C were all
+            // accepted.
+            if (nDepth == 0 && wtx.mapValue.count("replaces_txid")) {
+                safeTx = false;
+            }
 
-            isminetype mine = wallet.IsMine(output);
+            // Similarly, we should not consider coins from transactions that
+            // have been replaced. In the example above, we would want to prevent
+            // creation of a transaction A' spending an output of A, because if
+            // transaction B were initially confirmed, conflicting with A and
+            // A', we wouldn't want to the user to create a transaction D
+            // intending to replace A', but potentially resulting in a scenario
+            // where A, A', and D could all be accepted (instead of just B and
+            // D, or just A and A' like the user would want).
+            if (nDepth == 0 && wtx.mapValue.count("replaced_by_txid")) {
+                safeTx = false;
+            }
 
-            if (mine == ISMINE_NO) {
+            if (only_safe && !safeTx) {
                 continue;
             }
 
-            if (!allow_used_addresses && wallet.IsSpentKey(output.scriptPubKey)) {
+            if (nDepth < min_depth || nDepth > max_depth) {
                 continue;
             }
 
-            std::unique_ptr<SigningProvider> provider = wallet.GetSolvingProvider(output.scriptPubKey);
-
-            int input_bytes = CalculateMaximumSignedInputSize(output, COutPoint(), provider.get(), can_grind_r, coinControl);
-            // Because CalculateMaximumSignedInputSize infers a solvable descriptor to get the satisfaction size,
-            // it is safe to assume that this input is solvable if input_bytes is greater than -1.
-            bool solvable = input_bytes > -1;
-            bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable));
-
-            // Filter by spendable outputs only
-            if (!spendable && params.only_spendable) continue;
-
-            // Obtain script type
-            std::vector<std::vector<uint8_t>> script_solutions;
-            TxoutType type = Solver(output.scriptPubKey, script_solutions);
-
-            // If the output is P2SH and solvable, we want to know if it is
-            // a P2SH (legacy) or one of P2SH-P2WPKH, P2SH-P2WSH (P2SH-Segwit). We can determine
-            // this from the redeemScript. If the output is not solvable, it will be classified
-            // as a P2SH (legacy), since we have no way of knowing otherwise without the redeemScript
-            bool is_from_p2sh{false};
-            if (type == TxoutType::SCRIPTHASH && solvable) {
-                CScript script;
-                if (!provider->GetCScript(CScriptID(uint160(script_solutions[0])), script)) continue;
-                type = Solver(script, script_solutions);
-                is_from_p2sh = true;
-            }
+            tx_safe_cache[outpoint.hash] = {true, safeTx};
+        }
+        const auto& [tx_ok, tx_safe] = tx_safe_cache.at(outpoint.hash);
+        if (!Assume(tx_ok)) {
+            continue;
+        }
 
-            result.Add(GetOutputType(type, is_from_p2sh),
-                       COutput(outpoint, output, nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me, feerate));
+        if (output.nValue < params.min_amount || output.nValue > params.max_amount)
+            continue;
 
-            outpoints.push_back(outpoint);
+        // Skip manually selected coins (the caller can fetch them directly)
+        if (coinControl && coinControl->HasSelected() && coinControl->IsSelected(outpoint))
+            continue;
 
-            // Checks the sum amount of all UTXO's.
-            if (params.min_sum_amount != MAX_MONEY) {
-                if (result.GetTotalAmount() >= params.min_sum_amount) {
-                    return result;
-                }
-            }
+        if (wallet.IsLockedCoin(outpoint) && params.skip_locked)
+            continue;
+
+        if (wallet.IsSpent(outpoint))
+            continue;
 
-            // Checks the maximum number of UTXO's.
-            if (params.max_count > 0 && result.Size() >= params.max_count) {
+        isminetype mine = wallet.IsMine(output);
+        assert(mine != ISMINE_NO);
+
+        if (!allow_used_addresses && wallet.IsSpentKey(output.scriptPubKey)) {
+            continue;
+        }
+
+        bool tx_from_me = CachedTxIsFromMe(wallet, wtx, ISMINE_ALL);
+
+        std::unique_ptr<SigningProvider> provider = wallet.GetSolvingProvider(output.scriptPubKey);
+
+        int input_bytes = CalculateMaximumSignedInputSize(output, COutPoint(), provider.get(), can_grind_r, coinControl);
+        // Because CalculateMaximumSignedInputSize infers a solvable descriptor to get the satisfaction size,
+        // it is safe to assume that this input is solvable if input_bytes is greater than -1.
+        bool solvable = input_bytes > -1;
+        bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable));
+
+        // Filter by spendable outputs only
+        if (!spendable && params.only_spendable) continue;
+
+        // Obtain script type
+        std::vector<std::vector<uint8_t>> script_solutions;
+        TxoutType type = Solver(output.scriptPubKey, script_solutions);
+
+        // If the output is P2SH and solvable, we want to know if it is
+        // a P2SH (legacy) or one of P2SH-P2WPKH, P2SH-P2WSH (P2SH-Segwit). We can determine
+        // this from the redeemScript. If the output is not solvable, it will be classified
+        // as a P2SH (legacy), since we have no way of knowing otherwise without the redeemScript
+        bool is_from_p2sh{false};
+        if (type == TxoutType::SCRIPTHASH && solvable) {
+            CScript script;
+            if (!provider->GetCScript(CScriptID(uint160(script_solutions[0])), script)) continue;
+            type = Solver(script, script_solutions);
+            is_from_p2sh = true;
+        }
+
+        result.Add(GetOutputType(type, is_from_p2sh),
+                   COutput(outpoint, output, nDepth, input_bytes, spendable, solvable, tx_safe, wtx.GetTxTime(), tx_from_me, feerate));
+
+        outpoints.push_back(outpoint);
+
+        // Checks the sum amount of all UTXO's.
+        if (params.min_sum_amount != MAX_MONEY) {
+            if (result.GetTotalAmount() >= params.min_sum_amount) {
                 return result;
             }
         }
+
+        // Checks the maximum number of UTXO's.
+        if (params.max_count > 0 && result.Size() >= params.max_count) {
+            return result;
+        }
     }
 
     if (feerate.has_value()) {