From e02f2d331ce6d9c5b6b8f62a79dd40695d2283a2 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Feb 2024 11:50:17 -0500 Subject: [PATCH 01/13] bench: Have AvailableCoins benchmark include a lot of unrelated utxos One of the main issues with AvailableCoins is its performance when txs have unrelated outputs, so update the benchmark to check the performance of that. --- src/bench/wallet_create_tx.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp index 3c4b2f4f83..7fb535f4c7 100644 --- a/src/bench/wallet_create_tx.cpp +++ b/src/bench/wallet_create_tx.cpp @@ -71,10 +71,17 @@ void generateFakeBlock(const CChainParams& params, coinbase_tx.vin[0].prevout.SetNull(); coinbase_tx.vout.resize(2); coinbase_tx.vout[0].scriptPubKey = coinbase_out_script; - coinbase_tx.vout[0].nValue = 49 * COIN; + coinbase_tx.vout[0].nValue = 48 * COIN; coinbase_tx.vin[0].scriptSig = CScript() << ++tip.tip_height << OP_0; coinbase_tx.vout[1].scriptPubKey = coinbase_out_script; // extra output coinbase_tx.vout[1].nValue = 1 * COIN; + + // Fill the coinbase with outputs that don't belong to the wallet in order to benchmark + // AvailableCoins' behavior with unnecessary TXOs + for (int i = 0; i < 50; ++i) { + coinbase_tx.vout.emplace_back(1 * COIN / 50, CScript(OP_TRUE)); + } + block.vtx = {MakeTransactionRef(std::move(coinbase_tx))}; block.nVersion = VERSIONBITS_LAST_OLD_BLOCK_VERSION; @@ -129,14 +136,14 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type // Check available balance auto bal = WITH_LOCK(wallet.cs_wallet, return wallet::AvailableCoins(wallet).GetTotalAmount()); // Cache - assert(bal == 50 * COIN * (chain_size - COINBASE_MATURITY)); + assert(bal == 49 * COIN * (chain_size - COINBASE_MATURITY)); wallet::CCoinControl coin_control; coin_control.m_allow_other_inputs = allow_other_inputs; CAmount target = 0; if (preset_inputs) { - // Select inputs, each has 49 BTC + // Select inputs, each has 48 BTC wallet::CoinFilterParams filter_coins; filter_coins.max_count = preset_inputs->num_of_internal_inputs; const auto& res = WITH_LOCK(wallet.cs_wallet, @@ -152,7 +159,7 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type if (coin_control.m_allow_other_inputs) target += 50 * COIN; std::vector recipients = {{dest, target, true}}; - bench.epochIterations(5).run([&] { + bench.run([&] { LOCK(wallet.cs_wallet); const auto& tx_res = CreateTransaction(wallet, recipients, /*change_pos=*/std::nullopt, coin_control); assert(tx_res); @@ -189,9 +196,9 @@ static void AvailableCoins(benchmark::Bench& bench, const std::vector Date: Thu, 15 May 2025 13:34:49 -0700 Subject: [PATCH 02/13] wallet: MarkDirty after AddWalletDescriptor After adding a wallet descriptor (typically by import), mark all balance caches dirty. This allows transactions that the wallet already knows about that have outputs that are now ISMINE_SPENDABLE after the import to actually be shown in balance calculations. Legacy wallet imports would do this, but importdescriptors did not. --- src/wallet/wallet.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 991b138b85..840d7d5bc9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3735,6 +3735,9 @@ util::Result> CWallet::AddWall // Save the descriptor to DB spk_man->WriteDescriptor(); + // Break balance caches so that outputs that are now IsMine in already known txs will be included in the balance + MarkDirty(); + return std::reference_wrapper(*spk_man); } From 5cc32ee2a7addb38ae4a4c97d306d0c5d9cc2d5e Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Feb 2024 11:50:02 -0500 Subject: [PATCH 03/13] test: Test for balance update due to untracked output becoming spendable --- test/functional/wallet_balance.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index c3d9596b8a..91f58efc62 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -4,15 +4,18 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the wallet balance RPC methods.""" from decimal import Decimal +import time from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE as ADDRESS_WATCHONLY from test_framework.blocktools import COINBASE_MATURITY +from test_framework.descriptors import descsum_create from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_is_hash_string, assert_raises_rpc_error, ) +from test_framework.wallet_util import get_generate_key def create_transactions(node, address, amt, fees): @@ -279,5 +282,30 @@ class WalletTest(BitcoinTestFramework): assert_equal(tx_info['lastprocessedblock']['height'], prev_height) assert_equal(tx_info['lastprocessedblock']['hash'], prev_hash) + self.log.info("Test that the balance is updated by an import that makes an untracked output in an existing tx \"mine\"") + default = self.nodes[0].get_wallet_rpc(self.default_wallet_name) + self.nodes[0].createwallet("importupdate") + wallet = self.nodes[0].get_wallet_rpc("importupdate") + + import_key1 = get_generate_key() + import_key2 = get_generate_key() + wallet.importdescriptors([{"desc": descsum_create(f"wpkh({import_key1.privkey})"), "timestamp": "now"}]) + + amount = 15 + default.send([{import_key1.p2wpkh_addr: amount},{import_key2.p2wpkh_addr: amount}]) + self.generate(self.nodes[0], 1) + # Mock the time forward by 1 day so that "now" will exclude the block we just mined + self.nodes[0].setmocktime(int(time.time()) + 86400) + # Mine 11 blocks to move the MTP past the block we just mined + self.generate(self.nodes[0], 11, sync_fun=self.no_op) + + balances = wallet.getbalances() + assert_equal(balances["mine"]["trusted"], amount) + + # Don't rescan to make sure that the import updates the wallet txos + wallet.importdescriptors([{"desc": descsum_create(f"wpkh({import_key2.privkey})"), "timestamp": "now"}]) + balances = wallet.getbalances() + assert_equal(balances["mine"]["trusted"], amount * 2) + if __name__ == '__main__': WalletTest(__file__).main() From 0f269bc48c3905c0782c9d175ad487b27ebaf54b Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Feb 2024 11:49:51 -0500 Subject: [PATCH 04/13] walletdb: Load Txs last Need to load txs last so that IsMine works. --- src/wallet/walletdb.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 6f55207663..c23686818b 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1163,14 +1163,14 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // Load address book result = std::max(LoadAddressBookRecords(pwallet, *m_batch), result); - // Load tx records - result = std::max(LoadTxRecords(pwallet, *m_batch, any_unordered), result); - // Load SPKMs result = std::max(LoadActiveSPKMs(pwallet, *m_batch), result); // Load decryption keys result = std::max(LoadDecryptionKeys(pwallet, *m_batch), result); + + // Load tx records + result = std::max(LoadTxRecords(pwallet, *m_batch, any_unordered), result); } catch (std::runtime_error& e) { // Exceptions that can be ignored or treated as non-critical are handled by the individual loading functions. // Any uncaught exceptions will be caught here and treated as critical. From ae0876ec4273a8b978e2c602435a9ed25f48c976 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Feb 2024 11:49:53 -0500 Subject: [PATCH 05/13] wallet: Keep track of transaction outputs owned by the wallet When loading transactions to the wallet, check the outputs, and keep track of the ones that are IsMine. --- src/wallet/transaction.h | 25 +++++++++++++++++++++++++ src/wallet/wallet.cpp | 26 ++++++++++++++++++++++++++ src/wallet/wallet.h | 6 ++++++ 3 files changed, 57 insertions(+) diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index 1ec62b992b..8f64d181e0 100644 --- a/src/wallet/transaction.h +++ b/src/wallet/transaction.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -362,6 +363,30 @@ struct WalletTxOrderComparator { return a->nOrderPos < b->nOrderPos; } }; + +class WalletTXO +{ +private: + const CWalletTx& m_wtx; + const CTxOut& m_output; + isminetype m_ismine; + +public: + WalletTXO(const CWalletTx& wtx, const CTxOut& output, const isminetype ismine) + : m_wtx(wtx), + m_output(output), + m_ismine(ismine) + { + Assume(std::ranges::find(wtx.tx->vout, output) != wtx.tx->vout.end()); + } + + const CWalletTx& GetWalletTx() const { return m_wtx; } + + const CTxOut& GetTxOut() const { return m_output; } + + isminetype GetIsMine() const { return m_ismine; } + void SetIsMine(isminetype ismine) { m_ismine = ismine; } +}; } // namespace wallet #endif // BITCOIN_WALLET_TRANSACTION_H diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 840d7d5bc9..49f1cf22bc 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1094,6 +1094,9 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const // Break debit/credit balance caches: wtx.MarkDirty(); + // Cache the outputs that belong to the wallet + RefreshTXOsFromTx(wtx); + // Notify UI of new or updated transaction NotifyTransactionChanged(hash, fInsertedNew ? CT_NEW : CT_UPDATED); @@ -1157,6 +1160,8 @@ bool CWallet::LoadToWallet(const Txid& hash, const UpdateWalletTxFn& fill_wtx) // Update birth time when tx time is older than it. MaybeUpdateBirthTime(wtx.GetTxTime()); + // Make sure the tx outputs are known by the wallet + RefreshTXOsFromTx(wtx); return true; } @@ -2324,6 +2329,9 @@ util::Result CWallet::RemoveTxs(WalletBatch& batch, std::vector& txs wtxOrdered.erase(it->second.m_it_wtxOrdered); for (const auto& txin : it->second.tx->vin) mapTxSpends.erase(txin.prevout); + for (unsigned int i = 0; i < it->second.tx->vout.size(); ++i) { + m_txos.erase(COutPoint(Txid::FromUint256(hash), i)); + } mapWallet.erase(it); NotifyTransactionChanged(hash, CT_DELETED); } @@ -4425,4 +4433,22 @@ void CWallet::WriteBestBlock() const batch.WriteBestBlock(loc); } } + +void CWallet::RefreshTXOsFromTx(const CWalletTx& wtx) +{ + AssertLockHeld(cs_wallet); + for (uint32_t i = 0; i < wtx.tx->vout.size(); ++i) { + const CTxOut& txout = wtx.tx->vout.at(i); + isminetype ismine = IsMine(txout); + if (ismine == ISMINE_NO) { + continue; + } + COutPoint outpoint(wtx.GetHash(), i); + if (m_txos.contains(outpoint)) { + m_txos.at(outpoint).SetIsMine(ismine); + } else { + m_txos.emplace(outpoint, WalletTXO{wtx, txout, ismine}); + } + } +} } // namespace wallet diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 03c2ca2ffb..4d7a369baf 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -438,6 +438,9 @@ private: //! Cache of descriptor ScriptPubKeys used for IsMine. Maps ScriptPubKey to set of spkms std::unordered_map, SaltedSipHasher> m_cached_spks; + //! Set of both spent and unspent transaction outputs owned by this wallet + std::unordered_map m_txos GUARDED_BY(cs_wallet); + /** * Catch wallet up to current chain, scanning new blocks, updating the best * block locator and m_last_block_processed, and registering for @@ -520,6 +523,9 @@ public: std::set GetTxConflicts(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + /** Cache outputs that belong to the wallet from a single transaction */ + void RefreshTXOsFromTx(const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + /** * Return depth of transaction in blockchain: * <0 : conflicts with a transaction this deep in the blockchain From ae888c38d080c9c41925faeb53044e5bd0b7f9ee Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Feb 2024 11:49:56 -0500 Subject: [PATCH 06/13] wallet: Exit IsTrustedTx early if wtx is already in trusted_parents --- src/wallet/receive.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index f0d7d00763..620d2bb12c 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -257,6 +257,10 @@ bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx, const isminef bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set& trusted_parents) { AssertLockHeld(wallet.cs_wallet); + + // This wtx is already trusted + if (trusted_parents.contains(wtx.GetHash())) return true; + if (wtx.isConfirmed()) return true; if (wtx.isBlockConflicted()) return false; // using wtx's cached debit From 96e7a89c5e0bf5e1a4497d8296e8841edd7ebbf1 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Feb 2024 11:50:00 -0500 Subject: [PATCH 07/13] wallet: Recalculate the wallet's txos after any imports --- src/wallet/rpc/addresses.cpp | 1 + src/wallet/rpc/backup.cpp | 1 + src/wallet/wallet.cpp | 8 ++++++++ src/wallet/wallet.h | 2 ++ 4 files changed, 12 insertions(+) diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 39df2166be..eece081511 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -250,6 +250,7 @@ RPCHelpMan keypoolrefill() if (pwallet->GetKeyPoolSize() < kpSize) { throw JSONRPCError(RPC_WALLET_ERROR, "Error refreshing keypool."); } + pwallet->RefreshAllTXOs(); return UniValue::VNULL; }, diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index f7d37239c6..edd1fb86c9 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -394,6 +394,7 @@ RPCHelpMan importdescriptors() } } pwallet->ConnectScriptPubKeyManNotifiers(); + pwallet->RefreshAllTXOs(); } // Rescan the blockchain using the lowest timestamp diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 49f1cf22bc..ecbddf341f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4451,4 +4451,12 @@ void CWallet::RefreshTXOsFromTx(const CWalletTx& wtx) } } } + +void CWallet::RefreshAllTXOs() +{ + AssertLockHeld(cs_wallet); + for (const auto& [_, wtx] : mapWallet) { + RefreshTXOsFromTx(wtx); + } +} } // namespace wallet diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4d7a369baf..2b52842236 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -525,6 +525,8 @@ public: /** Cache outputs that belong to the wallet from a single transaction */ void RefreshTXOsFromTx(const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + /** Cache outputs that belong to the wallet for all transactions in the wallet */ + void RefreshAllTXOs() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Return depth of transaction in blockchain: From dde7cbe105ba6daaa636466d0fa3a83d15609417 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Feb 2024 11:49:58 -0500 Subject: [PATCH 08/13] wallet: Change balance calculation to use m_txos Since we track the outputs owned by the wallet with m_txos, we can now calculate the balance of the wallet by iterating m_txos and summing up the amounts of the unspent txos. As ISMINE_USED is not an actual isminetype that we attach to outputs and was just passed into `CachedTxGetAvailableCredit` for convenience, we pull the same determining logic from that function into `GetBalances` in order to preserve existing behavior. --- src/wallet/receive.cpp | 75 ++++++++++++++++++++++-------------------- src/wallet/wallet.h | 2 ++ 2 files changed, 42 insertions(+), 35 deletions(-) diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index 620d2bb12c..30644b4bec 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -297,27 +298,41 @@ bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx) Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse) { Balance ret; - isminefilter reuse_filter = avoid_reuse ? ISMINE_NO : ISMINE_USED; + bool allow_used_addresses = !avoid_reuse || !wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); { LOCK(wallet.cs_wallet); std::set trusted_parents; - for (const auto& entry : wallet.mapWallet) - { - const CWalletTx& wtx = entry.second; + for (const auto& [outpoint, txo] : wallet.GetTXOs()) { + const CWalletTx& wtx = txo.GetWalletTx(); + const bool is_trusted{CachedTxIsTrusted(wallet, wtx, trusted_parents)}; const int tx_depth{wallet.GetTxDepthInMainChain(wtx)}; - const CAmount tx_credit_mine{CachedTxGetAvailableCredit(wallet, wtx, ISMINE_SPENDABLE | reuse_filter)}; - const CAmount tx_credit_watchonly{CachedTxGetAvailableCredit(wallet, wtx, ISMINE_WATCH_ONLY | reuse_filter)}; - if (is_trusted && tx_depth >= min_depth) { - ret.m_mine_trusted += tx_credit_mine; - ret.m_watchonly_trusted += tx_credit_watchonly; + + if (!wallet.IsSpent(outpoint) && (allow_used_addresses || !wallet.IsSpentKey(txo.GetTxOut().scriptPubKey))) { + // Get the amounts for mine and watchonly + CAmount credit_mine = 0; + CAmount credit_watchonly = 0; + if (txo.GetIsMine() == ISMINE_SPENDABLE) { + credit_mine = txo.GetTxOut().nValue; + } else if (txo.GetIsMine() == ISMINE_WATCH_ONLY) { + credit_watchonly = txo.GetTxOut().nValue; + } else { + // We shouldn't see any other isminetypes + Assume(false); + } + + // Set the amounts in the return object + if (wallet.IsTxImmatureCoinBase(wtx) && wtx.isConfirmed()) { + ret.m_mine_immature += credit_mine; + ret.m_watchonly_immature += credit_watchonly; + } else if (is_trusted && tx_depth >= min_depth) { + ret.m_mine_trusted += credit_mine; + ret.m_watchonly_trusted += credit_watchonly; + } else if (!is_trusted && wtx.InMempool()) { + ret.m_mine_untrusted_pending += credit_mine; + ret.m_watchonly_untrusted_pending += credit_watchonly; + } } - if (!is_trusted && tx_depth == 0 && wtx.InMempool()) { - ret.m_mine_untrusted_pending += tx_credit_mine; - ret.m_watchonly_untrusted_pending += tx_credit_watchonly; - } - ret.m_mine_immature += CachedTxGetImmatureCredit(wallet, wtx, ISMINE_SPENDABLE); - ret.m_watchonly_immature += CachedTxGetImmatureCredit(wallet, wtx, ISMINE_WATCH_ONLY); } } return ret; @@ -330,31 +345,21 @@ std::map GetAddressBalances(const CWallet& wallet) { LOCK(wallet.cs_wallet); std::set trusted_parents; - for (const auto& walletEntry : wallet.mapWallet) - { - const CWalletTx& wtx = walletEntry.second; + for (const auto& [outpoint, txo] : wallet.GetTXOs()) { + const CWalletTx& wtx = txo.GetWalletTx(); - if (!CachedTxIsTrusted(wallet, wtx, trusted_parents)) - continue; - - if (wallet.IsTxImmatureCoinBase(wtx)) - continue; + if (!CachedTxIsTrusted(wallet, wtx, trusted_parents)) continue; + if (wallet.IsTxImmatureCoinBase(wtx)) continue; int nDepth = wallet.GetTxDepthInMainChain(wtx); - if (nDepth < (CachedTxIsFromMe(wallet, wtx, ISMINE_ALL) ? 0 : 1)) - continue; + if (nDepth < (CachedTxIsFromMe(wallet, wtx, ISMINE_ALL) ? 0 : 1)) continue; - for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) { - const auto& output = wtx.tx->vout[i]; - CTxDestination addr; - if (!wallet.IsMine(output)) - continue; - if(!ExtractDestination(output.scriptPubKey, addr)) - continue; + CTxDestination addr; + Assume(wallet.IsMine(txo.GetTxOut())); + if(!ExtractDestination(txo.GetTxOut().scriptPubKey, addr)) continue; - CAmount n = wallet.IsSpent(COutPoint(walletEntry.first, i)) ? 0 : output.nValue; - balances[addr] += n; - } + CAmount n = wallet.IsSpent(outpoint) ? 0 : txo.GetTxOut().nValue; + balances[addr] += n; } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 2b52842236..3d6f050886 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -523,6 +523,8 @@ public: std::set GetTxConflicts(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + const std::unordered_map& GetTXOs() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); return m_txos; }; + /** Cache outputs that belong to the wallet from a single transaction */ void RefreshTXOsFromTx(const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** Cache outputs that belong to the wallet for all transactions in the wallet */ From c1801b78f1c11e596378a44458a484b6a9de71d8 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Feb 2024 11:50:07 -0500 Subject: [PATCH 09/13] 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. --- src/wallet/spend.cpp | 219 ++++++++++++++++++++++--------------------- 1 file changed, 114 insertions(+), 105 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index d96946e7b8..9d6cf41a42 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -330,137 +330,146 @@ CoinsResult AvailableCoins(const CWallet& wallet, std::vector outpoints; std::set trusted_parents; - for (const auto& entry : wallet.mapWallet) - { - const Txid& txid = entry.first; - const CWalletTx& wtx = entry.second; + // Cache for whether each tx passes the tx level checks (first bool), and whether the transaction is "safe" (second bool) + std::unordered_map, SaltedTxidHasher> tx_safe_cache; + for (const auto& [outpoint, txo] : wallet.GetTXOs()) { + const CWalletTx& wtx = txo.GetWalletTx(); + const CTxOut& output = txo.GetTxOut(); - if (wallet.IsTxImmatureCoinBase(wtx) && !params.include_immature_coinbase) + if (tx_safe_cache.contains(outpoint.hash) && !tx_safe_cache.at(outpoint.hash).first) { 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; + // 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}; - bool safeTx = CachedTxIsTrusted(wallet, wtx, trusted_parents); + if (wallet.IsTxImmatureCoinBase(wtx) && !params.include_immature_coinbase) + continue; - // 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; + 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; + } + + if (nDepth < min_depth || nDepth > max_depth) { + continue; + } + + tx_safe_cache[outpoint.hash] = {true, safeTx}; } - - // 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) { + const auto& [tx_ok, tx_safe] = tx_safe_cache.at(outpoint.hash); + if (!Assume(tx_ok)) { continue; } - if (nDepth < min_depth || nDepth > max_depth) { + if (output.nValue < params.min_amount || output.nValue > params.max_amount) + continue; + + // Skip manually selected coins (the caller can fetch them directly) + if (coinControl && coinControl->HasSelected() && coinControl->IsSelected(outpoint)) + continue; + + if (wallet.IsLockedCoin(outpoint) && params.skip_locked) + continue; + + if (wallet.IsSpent(outpoint)) + continue; + + 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); - for (unsigned int i = 0; i < wtx.tx->vout.size(); i++) { - const CTxOut& output = wtx.tx->vout[i]; - const COutPoint outpoint(txid, i); + std::unique_ptr provider = wallet.GetSolvingProvider(output.scriptPubKey); - if (output.nValue < params.min_amount || output.nValue > params.max_amount) - continue; + 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)); - // Skip manually selected coins (the caller can fetch them directly) - if (coinControl && coinControl->HasSelected() && coinControl->IsSelected(outpoint)) - continue; + // Filter by spendable outputs only + if (!spendable && params.only_spendable) continue; - if (wallet.IsLockedCoin(outpoint) && params.skip_locked) - continue; + // Obtain script type + std::vector> script_solutions; + TxoutType type = Solver(output.scriptPubKey, script_solutions); - if (wallet.IsSpent(outpoint)) - continue; + // 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; + } - isminetype mine = wallet.IsMine(output); + result.Add(GetOutputType(type, is_from_p2sh), + COutput(outpoint, output, nDepth, input_bytes, spendable, solvable, tx_safe, wtx.GetTxTime(), tx_from_me, feerate)); - if (mine == ISMINE_NO) { - continue; - } + outpoints.push_back(outpoint); - if (!allow_used_addresses && wallet.IsSpentKey(output.scriptPubKey)) { - continue; - } - - std::unique_ptr 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> 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, safeTx, 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) { + // 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()) { From 764016eb2259676d834cf6829f5b0e04b135d407 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Feb 2024 11:50:09 -0500 Subject: [PATCH 10/13] wallet: Retrieve TXO directly in FetchSelectedInputs Instead of searching mapWallet for the preselected inputs, search m_txos. wallet_fundrawtransaction.py spends external inputs and needs the change output to also belong to the test wallet for the oversized tx test. --- src/wallet/spend.cpp | 8 ++------ src/wallet/wallet.cpp | 10 ++++++++++ src/wallet/wallet.h | 1 + 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 9d6cf41a42..fe3f4b57b9 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -277,12 +277,8 @@ util::Result FetchSelectedInputs(const CWallet& wallet, const input_bytes = GetVirtualTransactionSize(input_bytes, 0, 0); } CTxOut txout; - if (auto ptr_wtx = wallet.GetWalletTx(outpoint.hash)) { - // Clearly invalid input, fail - if (ptr_wtx->tx->vout.size() <= outpoint.n) { - return util::Error{strprintf(_("Invalid pre-selected input %s"), outpoint.ToString())}; - } - txout = ptr_wtx->tx->vout.at(outpoint.n); + if (auto txo = wallet.GetTXO(outpoint)) { + txout = txo->GetTxOut(); if (input_bytes == -1) { input_bytes = CalculateMaximumSignedInputSize(txout, &wallet, &coin_control); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ecbddf341f..7d2e9eec76 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4459,4 +4459,14 @@ void CWallet::RefreshAllTXOs() RefreshTXOsFromTx(wtx); } } + +std::optional CWallet::GetTXO(const COutPoint& outpoint) const +{ + AssertLockHeld(cs_wallet); + const auto& it = m_txos.find(outpoint); + if (it == m_txos.end()) { + return std::nullopt; + } + return it->second; +} } // namespace wallet diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3d6f050886..2db522c4f4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -524,6 +524,7 @@ public: std::set GetTxConflicts(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); const std::unordered_map& GetTXOs() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); return m_txos; }; + std::optional GetTXO(const COutPoint& outpoint) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** Cache outputs that belong to the wallet from a single transaction */ void RefreshTXOsFromTx(const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); From 17d453cb3a6fdbb0ebc8538c228c89712c989499 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Feb 2024 11:50:11 -0500 Subject: [PATCH 11/13] wallet: Recompute wallet TXOs after descriptor migration When a legacy wallet has been migrated to contain descriptors, but before the transactions have been updated to match, we need to recompute the wallet TXOs so that the transaction update will work correctly. --- src/wallet/wallet.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7d2e9eec76..373daed091 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3901,6 +3901,10 @@ util::Result CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch, return util::Error{_("Error: Unable to read wallet's best block locator record")}; } + // Update m_txos to match the descriptors remaining in this wallet + m_txos.clear(); + RefreshAllTXOs(); + // Check if the transactions in the wallet are still ours. Either they belong here, or they belong in the watchonly wallet. // We need to go through these in the tx insertion order so that lookups to spends works. std::vector txids_to_delete; From 49675de035e7c668d6857a32d929b7e1e85e83e3 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Feb 2024 11:50:13 -0500 Subject: [PATCH 12/13] wallet: Have GetDebit use the wallet's TXO set Instead of looking up the previous tx in mapWallet, and then calling IsMine on the specified output, use the TXO set and its cached IsMine value. --- src/wallet/wallet.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 373daed091..36da439610 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1543,16 +1543,10 @@ void CWallet::BlockUntilSyncedToCurrentChain() const { // and a not-"is mine" (according to the filter) input. CAmount CWallet::GetDebit(const CTxIn &txin, const isminefilter& filter) const { - { - LOCK(cs_wallet); - const auto mi = mapWallet.find(txin.prevout.hash); - if (mi != mapWallet.end()) - { - const CWalletTx& prev = (*mi).second; - if (txin.prevout.n < prev.tx->vout.size()) - if (IsMine(prev.tx->vout[txin.prevout.n]) & filter) - return prev.tx->vout[txin.prevout.n].nValue; - } + LOCK(cs_wallet); + auto txo = GetTXO(txin.prevout); + if (txo && (txo->GetIsMine() & filter)) { + return txo->GetTxOut().nValue; } return 0; } From 215e5999e2070a38c68e343c5c3f1dc37d567f58 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Feb 2024 11:50:15 -0500 Subject: [PATCH 13/13] 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 --- src/wallet/receive.cpp | 46 ----------------- src/wallet/receive.h | 4 -- src/wallet/test/wallet_tests.cpp | 89 -------------------------------- src/wallet/transaction.h | 4 +- 4 files changed, 1 insertion(+), 142 deletions(-) diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index 30644b4bec..1ba85b6b2d 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 b795689de5..330fbf21a6 100644 --- a/src/wallet/receive.h +++ b/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; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 1b0cfe2c4f..8f8da3c302 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/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 diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index 8f64d181e0..b44f0c7d05 100644 --- a/src/wallet/transaction.h +++ b/src/wallet/transaction.h @@ -219,7 +219,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 @@ -316,8 +316,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; }