wallet: Remove unused CachedTxGet{Available,Immature}Credit

These two functions are no longer used as GetBalances now uses the TXO
set rather than per-tx cached balances

Author: achow101

src/wallet/receive.cpp

@@ -146,52 +146,6 @@ CAmount CachedTxGetChange(const CWallet& wallet, const CWalletTx& wtx)
     return wtx.nChangeCached;
 }
 
-CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
-{
-    AssertLockHeld(wallet.cs_wallet);
-
-    if (wallet.IsTxImmatureCoinBase(wtx) && wtx.isConfirmed()) {
-        return GetCachableAmount(wallet, wtx, CWalletTx::IMMATURE_CREDIT, filter);
-    }
-
-    return 0;
-}
-
-CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
-{
-    AssertLockHeld(wallet.cs_wallet);
-
-    // Avoid caching ismine for NO or ALL cases (could remove this check and simplify in the future).
-    bool allow_cache = (filter & ISMINE_ALL) && (filter & ISMINE_ALL) != ISMINE_ALL;
-
-    // Must wait until coinbase is safely deep enough in the chain before valuing it
-    if (wallet.IsTxImmatureCoinBase(wtx))
-        return 0;
-
-    if (allow_cache && wtx.m_amounts[CWalletTx::AVAILABLE_CREDIT].m_cached[filter]) {
-        return wtx.m_amounts[CWalletTx::AVAILABLE_CREDIT].m_value[filter];
-    }
-
-    bool allow_used_addresses = (filter & ISMINE_USED) || !wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE);
-    CAmount nCredit = 0;
-    Txid hashTx = wtx.GetHash();
-    for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) {
-        const CTxOut& txout = wtx.tx->vout[i];
-        if (!wallet.IsSpent(COutPoint(hashTx, i)) && (allow_used_addresses || !wallet.IsSpentKey(txout.scriptPubKey))) {
-            nCredit += OutputGetCredit(wallet, txout, filter);
-            if (!MoneyRange(nCredit))
-                throw std::runtime_error(std::string(__func__) + " : value out of range");
-        }
-    }
-
-    if (allow_cache) {
-        wtx.m_amounts[CWalletTx::AVAILABLE_CREDIT].Set(filter, nCredit);
-        wtx.m_is_cache_empty = false;
-    }
-
-    return nCredit;
-}
-
 void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
                   std::list<COutputEntry>& listReceived,
                   std::list<COutputEntry>& listSent, CAmount& nFee, const isminefilter& filter,

src/wallet/receive.h

@@ -30,10 +30,6 @@ CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const ism
 //! filter decides which addresses will count towards the debit
 CAmount CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter);
 CAmount CachedTxGetChange(const CWallet& wallet, const CWalletTx& wtx);
-CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
-    EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
-CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter = ISMINE_SPENDABLE)
-    EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
 struct COutputEntry
 {
     CTxDestination destination;

src/wallet/test/wallet_tests.cpp

@@ -244,35 +244,6 @@ BOOST_FIXTURE_TEST_CASE(write_wallet_settings_concurrently, TestingSetup)
                             /*num_expected_wallets=*/0);
 }
 
-// Check that GetImmatureCredit() returns a newly calculated value instead of
-// the cached value after a MarkDirty() call.
-//
-// This is a regression test written to verify a bugfix for the immature credit
-// function. Similar tests probably should be written for the other credit and
-// debit functions.
-BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup)
-{
-    CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
-
-    LOCK(wallet.cs_wallet);
-    LOCK(Assert(m_node.chainman)->GetMutex());
-    CWalletTx wtx{m_coinbase_txns.back(), TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*index=*/0}};
-    wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
-    wallet.SetupDescriptorScriptPubKeyMans();
-
-    wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash());
-
-    // Call GetImmatureCredit() once before adding the key to the wallet to
-    // cache the current immature credit amount, which is 0.
-    BOOST_CHECK_EQUAL(CachedTxGetImmatureCredit(wallet, wtx, ISMINE_SPENDABLE), 0);
-
-    // Invalidate the cached value, add the key, and make sure a new immature
-    // credit amount is calculated.
-    wtx.MarkDirty();
-    AddKey(wallet, coinbaseKey);
-    BOOST_CHECK_EQUAL(CachedTxGetImmatureCredit(wallet, wtx, ISMINE_SPENDABLE), 50*COIN);
-}
-
 static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime)
 {
     CMutableTransaction tx;
@@ -751,65 +722,5 @@ BOOST_FIXTURE_TEST_CASE(RemoveTxs, TestChain100Setup)
     TestUnloadWallet(std::move(wallet));
 }
 
-/**
- * Checks a wallet invalid state where the inputs (prev-txs) of a new arriving transaction are not marked dirty,
- * while the transaction that spends them exist inside the in-memory wallet tx map (not stored on db due a db write failure).
- */
-BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestingSetup)
-{
-    CWallet wallet(m_node.chain.get(), "", CreateMockableWalletDatabase());
-    {
-        LOCK(wallet.cs_wallet);
-        wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
-        wallet.SetupDescriptorScriptPubKeyMans();
-    }
-
-    // Add tx to wallet
-    const auto op_dest{*Assert(wallet.GetNewDestination(OutputType::BECH32M, ""))};
-
-    CMutableTransaction mtx;
-    mtx.vout.emplace_back(COIN, GetScriptForDestination(op_dest));
-    mtx.vin.emplace_back(Txid::FromUint256(m_rng.rand256()), 0);
-    const auto& tx_id_to_spend = wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInMempool{})->GetHash();
-
-    {
-        // Cache and verify available balance for the wtx
-        LOCK(wallet.cs_wallet);
-        const CWalletTx* wtx_to_spend = wallet.GetWalletTx(tx_id_to_spend);
-        BOOST_CHECK_EQUAL(CachedTxGetAvailableCredit(wallet, *wtx_to_spend), 1 * COIN);
-    }
-
-    // Now the good case:
-    // 1) Add a transaction that spends the previously created transaction
-    // 2) Verify that the available balance of this new tx and the old one is updated (prev tx is marked dirty)
-
-    mtx.vin.clear();
-    mtx.vin.emplace_back(tx_id_to_spend, 0);
-    wallet.transactionAddedToMempool(MakeTransactionRef(mtx));
-    const auto good_tx_id{mtx.GetHash()};
-
-    {
-        // Verify balance update for the new tx and the old one
-        LOCK(wallet.cs_wallet);
-        const CWalletTx* new_wtx = wallet.GetWalletTx(good_tx_id);
-        BOOST_CHECK_EQUAL(CachedTxGetAvailableCredit(wallet, *new_wtx), 1 * COIN);
-
-        // Now the old wtx
-        const CWalletTx* wtx_to_spend = wallet.GetWalletTx(tx_id_to_spend);
-        BOOST_CHECK_EQUAL(CachedTxGetAvailableCredit(wallet, *wtx_to_spend), 0 * COIN);
-    }
-
-    // Now the bad case:
-    // 1) Make db always fail
-    // 2) Try to add a transaction that spends the previously created transaction and
-    //    verify that we are not moving forward if the wallet cannot store it
-    GetMockableDatabase(wallet).m_pass = false;
-    mtx.vin.clear();
-    mtx.vin.emplace_back(good_tx_id, 0);
-    BOOST_CHECK_EXCEPTION(wallet.transactionAddedToMempool(MakeTransactionRef(mtx)),
-                          std::runtime_error,
-                          HasReason("DB error adding transaction to wallet, write failed"));
-}
-
 BOOST_AUTO_TEST_SUITE_END()
 } // namespace wallet

src/wallet/transaction.h

@@ -219,7 +219,7 @@ class CWalletTx
     std::multimap<int64_t, CWalletTx*>::const_iterator m_it_wtxOrdered;
 
     // memory only
-    enum AmountType { DEBIT, CREDIT, IMMATURE_CREDIT, AVAILABLE_CREDIT, AMOUNTTYPE_ENUM_ELEMENTS };
+    enum AmountType { DEBIT, CREDIT, AMOUNTTYPE_ENUM_ELEMENTS };
     mutable CachableAmount m_amounts[AMOUNTTYPE_ENUM_ELEMENTS];
     /**
      * This flag is true if all m_amounts caches are empty. This is particularly
@@ -316,8 +316,6 @@ class CWalletTx
     {
         m_amounts[DEBIT].Reset();
         m_amounts[CREDIT].Reset();
-        m_amounts[IMMATURE_CREDIT].Reset();
-        m_amounts[AVAILABLE_CREDIT].Reset();
         fChangeCached = false;
         m_is_cache_empty = true;
     }