From 14d04d5ad15ae56df56edee7ca9a202b52037889 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 18 Jan 2022 19:08:42 -0500 Subject: [PATCH] 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. --- src/wallet/interfaces.cpp | 6 +++--- src/wallet/rpc/coins.cpp | 12 ++++++------ src/wallet/spend.cpp | 16 +++++++++++----- src/wallet/spend.h | 14 ++++++++------ src/wallet/test/coinselector_tests.cpp | 2 +- src/wallet/test/wallet_tests.cpp | 2 +- 6 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 56ca2f0cdb1..087d1df2a97 100644 --- a/src/wallet/interfaces.cpp +++ b/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 @@ public: 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)); } } diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 297af389524..781ae3c6e04 100644 --- a/src/wallet/rpc/coins.cpp +++ b/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)); diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 0dc79e9c804..ce6fe8e4c95 100644 --- a/src/wallet/spend.cpp +++ b/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> ListCoins(const CWallet& wallet) { AssertLockHeld(wallet.cs_wallet); @@ -257,7 +263,7 @@ std::map> 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 GroupOutputs(const CWallet& wallet, const std::vectorGetHash(), 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 GroupOutputs(const CWallet& wallet, const std::vectorGetHash(), ancestors, descendants); + wallet.chain().getTransactionAncestry(output.outpoint.hash, ancestors, descendants); CInputCoin input_coin = output.GetInputCoin(); CScript spk = input_coin.txout.scriptPubKey; diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 029560a16c0..141ec833eee 100644 --- a/src/wallet/spend.h +++ b/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 @@ public: 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 @@ public: 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. diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index ebeb50bace8..393ec138c6c 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/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); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index c59f7e6f058..8456107ebfd 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/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); } } {