diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index 5ff3b9110a2..6de5e7d0daa 100644 --- a/src/wallet/receive.cpp +++ b/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& listReceived, std::list& listSent, CAmount& nFee, const isminefilter& filter, diff --git a/src/wallet/receive.h b/src/wallet/receive.h index d50644b4cf9..6cccd323e0e 100644 --- a/src/wallet/receive.h +++ b/src/wallet/receive.h @@ -29,10 +29,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; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index b5de4b4b3d3..165fc2c03e7 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -363,35 +363,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; @@ -961,65 +932,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.ToUint256()); - 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 diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index ac9a8db8d9b..fef7d833997 100644 --- a/src/wallet/transaction.h +++ b/src/wallet/transaction.h @@ -225,7 +225,7 @@ public: std::multimap::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 @@ -322,8 +322,6 @@ public: { 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; }