wallet: Replace CWalletTx in COutput with COutPoint and CTxOut

Instead of having a pointer to the CWalletTx in COutput, we can just
store the COutPoint and the CTxOut as those are the only things we need
from the CWalletTx. Other things CWalletTx used to provide were time and
fIsFromMe but these are also being stored by COutput.

Author: achow101

src/wallet/interfaces.cpp

@@ -115,10 +115,10 @@ WalletTxOut MakeWalletTxOut(const CWallet& wallet,
     const COutput& output) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
 {
     WalletTxOut result;
-    result.txout = output.tx->tx->vout[output.i];
+    result.txout = output.txout;
     result.time = output.time;
     result.depth_in_main_chain = output.depth;
-    result.is_spent = wallet.IsSpent(output.tx->GetHash(), output.i);
+    result.is_spent = wallet.IsSpent(output.outpoint.hash, output.outpoint.n);
     return result;
 }
 
@@ -430,7 +430,7 @@ class WalletImpl : public Wallet
         for (const auto& entry : ListCoins(*m_wallet)) {
             auto& group = result[entry.first];
             for (const auto& coin : entry.second) {
-                group.emplace_back(COutPoint(coin.tx->GetHash(), coin.i),
+                group.emplace_back(coin.outpoint,
                     MakeWalletTxOut(*m_wallet, coin));
             }
         }

src/wallet/rpc/coins.cpp

@@ -648,16 +648,16 @@ RPCHelpMan listunspent()
 
     for (const COutput& out : vecOutputs) {
         CTxDestination address;
-        const CScript& scriptPubKey = out.tx->tx->vout[out.i].scriptPubKey;
+        const CScript& scriptPubKey = out.txout.scriptPubKey;
         bool fValidAddress = ExtractDestination(scriptPubKey, address);
-        bool reused = avoid_reuse && pwallet->IsSpentKey(out.tx->GetHash(), out.i);
+        bool reused = avoid_reuse && pwallet->IsSpentKey(out.outpoint.hash, out.outpoint.n);
 
         if (destinations.size() && (!fValidAddress || !destinations.count(address)))
             continue;
 
         UniValue entry(UniValue::VOBJ);
-        entry.pushKV("txid", out.tx->GetHash().GetHex());
-        entry.pushKV("vout", out.i);
+        entry.pushKV("txid", out.outpoint.hash.GetHex());
+        entry.pushKV("vout", (int)out.outpoint.n);
 
         if (fValidAddress) {
             entry.pushKV("address", EncodeDestination(address));
@@ -702,12 +702,12 @@ RPCHelpMan listunspent()
         }
 
         entry.pushKV("scriptPubKey", HexStr(scriptPubKey));
-        entry.pushKV("amount", ValueFromAmount(out.tx->tx->vout[out.i].nValue));
+        entry.pushKV("amount", ValueFromAmount(out.txout.nValue));
         entry.pushKV("confirmations", out.depth);
         if (!out.depth) {
             size_t ancestor_count, descendant_count, ancestor_size;
             CAmount ancestor_fees;
-            pwallet->chain().getTransactionAncestry(out.tx->GetHash(), ancestor_count, descendant_count, &ancestor_size, &ancestor_fees);
+            pwallet->chain().getTransactionAncestry(out.outpoint.hash, ancestor_count, descendant_count, &ancestor_size, &ancestor_fees);
             if (ancestor_count) {
                 entry.pushKV("ancestorcount", uint64_t(ancestor_count));
                 entry.pushKV("ancestorsize", uint64_t(ancestor_size));

src/wallet/spend.cpp

@@ -31,7 +31,7 @@ int GetTxSpendSize(const CWallet& wallet, const CWalletTx& wtx, unsigned int out
 
 std::string COutput::ToString() const
 {
-    return strprintf("COutput(%s, %d, %d) [%s]", tx->GetHash().ToString(), i, depth, FormatMoney(tx->tx->vout[i].nValue));
+    return strprintf("COutput(%s, %d, %d) [%s]", outpoint.hash.ToString(), outpoint.n, depth, FormatMoney(txout.nValue));
 }
 
 int CalculateMaximumSignedInputSize(const CTxOut& txout, const SigningProvider* provider, bool use_max_sig)
@@ -221,7 +221,7 @@ CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinContr
     AvailableCoins(wallet, vCoins, coinControl);
     for (const COutput& out : vCoins) {
         if (out.spendable) {
-            balance += out.tx->tx->vout[out.i].nValue;
+            balance += out.txout.nValue;
         }
     }
     return balance;
@@ -245,6 +245,12 @@ const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const CTransactio
     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);
+}
+
 std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet)
 {
     AssertLockHeld(wallet.cs_wallet);
@@ -257,7 +263,7 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet)
     for (const COutput& coin : availableCoins) {
         CTxDestination address;
         if ((coin.spendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.solvable)) &&
-            ExtractDestination(FindNonChangeParentOutput(wallet, *coin.tx->tx, coin.i).scriptPubKey, address)) {
+            ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) {
             result[address].emplace_back(std::move(coin));
         }
     }
@@ -298,7 +304,7 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C
             if (!output.spendable) continue;
 
             size_t ancestors, descendants;
-            wallet.chain().getTransactionAncestry(output.tx->GetHash(), ancestors, descendants);
+            wallet.chain().getTransactionAncestry(output.outpoint.hash, ancestors, descendants);
             CInputCoin input_coin = output.GetInputCoin();
 
             // Make an OutputGroup containing just this output
@@ -324,7 +330,7 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C
         if (!output.spendable) continue;
 
         size_t ancestors, descendants;
-        wallet.chain().getTransactionAncestry(output.tx->GetHash(), ancestors, descendants);
+        wallet.chain().getTransactionAncestry(output.outpoint.hash, ancestors, descendants);
         CInputCoin input_coin = output.GetInputCoin();
         CScript spk = input_coin.txout.scriptPubKey;
 

src/wallet/spend.h

@@ -19,10 +19,11 @@ int GetTxSpendSize(const CWallet& wallet, const CWalletTx& wtx, unsigned int out
 class COutput
 {
 public:
-    const CWalletTx *tx;
+    /** The outpoint identifying this UTXO */
+    COutPoint outpoint;
 
-    /** Index in tx->vout. */
-    int i;
+    /** The output itself */
+    CTxOut txout;
 
     /**
      * Depth in block chain.
@@ -54,8 +55,8 @@ class COutput
     bool from_me;
 
     COutput(const CWallet& wallet, const CWalletTx& wtx, int iIn, int depth, int input_bytes, bool spendable, bool solvable, bool safe, int64_t time, bool from_me)
-        : tx(&wtx),
-        i(iIn),
+        : outpoint(COutPoint(wtx.GetHash(), iIn)),
+        txout(wtx.tx->vout.at(iIn)),
         depth(depth),
         input_bytes(input_bytes),
         spendable(spendable),
@@ -69,7 +70,7 @@ class COutput
 
     inline CInputCoin GetInputCoin() const
     {
-        return CInputCoin(tx->tx, i, input_bytes);
+        return CInputCoin(outpoint, txout, input_bytes);
     }
 };
 
@@ -100,6 +101,7 @@ 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);
 
 /**
  * Return list of available coins and locked coins grouped by non-change output address.

src/wallet/test/coinselector_tests.cpp

@@ -331,7 +331,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
         add_coin(coins, *wallet, 2 * CENT, 6 * 24, false, 0, true);
         CCoinControl coin_control;
         coin_control.fAllowOtherInputs = true;
-        coin_control.Select(COutPoint(coins.at(0).tx->GetHash(), coins.at(0).i));
+        coin_control.Select(coins.at(0).outpoint);
         coin_selection_params_bnb.m_effective_feerate = CFeeRate(0);
         const auto result10 = SelectCoins(*wallet, coins, 10 * CENT, coin_control, coin_selection_params_bnb);
         BOOST_CHECK(result10);

src/wallet/test/wallet_tests.cpp

@@ -588,7 +588,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup)
     for (const auto& group : list) {
         for (const auto& coin : group.second) {
             LOCK(wallet->cs_wallet);
-            wallet->LockCoin(COutPoint(coin.tx->GetHash(), coin.i));
+            wallet->LockCoin(coin.outpoint);
         }
     }
     {