diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index bba9e058cc0..9154ac05dd0 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -369,11 +369,14 @@ struct WalletBalances CAmount balance = 0; CAmount unconfirmed_balance = 0; CAmount immature_balance = 0; + CAmount used_balance = 0; + CAmount nonmempool_balance = 0; bool balanceChanged(const WalletBalances& prev) const { return balance != prev.balance || unconfirmed_balance != prev.unconfirmed_balance || - immature_balance != prev.immature_balance; + immature_balance != prev.immature_balance || + used_balance != prev.used_balance || nonmempool_balance != prev.nonmempool_balance; } }; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 3ffac043b95..ae6b9ff1911 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -380,6 +380,8 @@ public: result.balance = bal.m_mine_trusted; result.unconfirmed_balance = bal.m_mine_untrusted_pending; result.immature_balance = bal.m_mine_immature; + result.used_balance = bal.m_mine_used; + result.nonmempool_balance = bal.m_mine_nonmempool; return result; } bool tryGetBalances(WalletBalances& balances, uint256& block_hash) override diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index 7176dff5275..86bc63f8dbd 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -242,7 +242,7 @@ bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx) return CachedTxIsTrusted(wallet, wtx, trusted_parents); } -Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse) +Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse, bool include_nonmempool) { Balance ret; bool allow_used_addresses = !avoid_reuse || !wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); @@ -255,17 +255,38 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse) const bool is_trusted{CachedTxIsTrusted(wallet, wtx, trusted_parents)}; const int tx_depth{wallet.GetTxDepthInMainChain(wtx)}; - if (!wallet.IsSpent(outpoint) && (allow_used_addresses || !wallet.IsSpentKey(txo.GetTxOut().scriptPubKey))) { - // Get the amounts for mine - CAmount credit_mine = txo.GetTxOut().nValue; + bool nonmempool_spent = false; + switch (wallet.HowSpent(outpoint)) { + case CWallet::SpendType::CONFIRMED: + case CWallet::SpendType::MEMPOOL: + // treat as spent; ignore + break; + case CWallet::SpendType::NONMEMPOOL: + if (!include_nonmempool) break; + nonmempool_spent = true; + [[fallthrough]]; + case CWallet::SpendType::UNSPENT: + CAmount* bucket = nullptr; // Set the amounts in the return object if (wallet.IsTxImmatureCoinBase(wtx) && wtx.isConfirmed()) { - ret.m_mine_immature += credit_mine; + bucket = &ret.m_mine_immature; } else if (is_trusted && tx_depth >= min_depth) { - ret.m_mine_trusted += credit_mine; + bucket = &ret.m_mine_trusted; } else if (!is_trusted && wtx.InMempool()) { - ret.m_mine_untrusted_pending += credit_mine; + bucket = &ret.m_mine_untrusted_pending; + } + if (bucket) { + // Get the amounts for mine + CAmount credit_mine = txo.GetTxOut().nValue; + + if (!allow_used_addresses && wallet.IsSpentKey(txo.GetTxOut().scriptPubKey)) { + bucket = &ret.m_mine_used; + } + *bucket += credit_mine; + if (nonmempool_spent) { + ret.m_mine_nonmempool -= credit_mine; + } } } } diff --git a/src/wallet/receive.h b/src/wallet/receive.h index 2e550494b6b..15e3e605c39 100644 --- a/src/wallet/receive.h +++ b/src/wallet/receive.h @@ -47,8 +47,10 @@ struct Balance { CAmount m_mine_trusted{0}; //!< Trusted, at depth=GetBalance.min_depth or more CAmount m_mine_untrusted_pending{0}; //!< Untrusted, but in mempool (pending) CAmount m_mine_immature{0}; //!< Immature coinbases in the main chain + CAmount m_mine_used{0}; //!< Trusted/untrusted/immature funds in utxos that have already been spent from (only populated if AVOID REUSE wallet flag is set) + CAmount m_mine_nonmempool{0}; //!< Coins spent by wallet txs that are not in the mempool }; -Balance GetBalance(const CWallet& wallet, int min_depth = 0, bool avoid_reuse = true); +Balance GetBalance(const CWallet& wallet, int min_depth = 0, bool avoid_reuse = true, bool include_nonmempool = false); std::map GetAddressBalances(const CWallet& wallet); std::set> GetAddressGroupings(const CWallet& wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 79889b5a38d..ab869b0d3f4 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -412,6 +412,7 @@ RPCMethod getbalances() {RPCResult::Type::STR_AMOUNT, "trusted", "trusted balance (outputs created by the wallet or confirmed outputs)"}, {RPCResult::Type::STR_AMOUNT, "untrusted_pending", "untrusted pending balance (outputs created by others that are in the mempool)"}, {RPCResult::Type::STR_AMOUNT, "immature", "balance from immature coinbase outputs"}, + {RPCResult::Type::STR_AMOUNT, "nonmempool", "sum of coins that are spent by transactions not in the mempool (usually an over-estimate due to not accounting for change or spends that conflict with each other)"}, {RPCResult::Type::STR_AMOUNT, "used", /*optional=*/true, "(only present if avoid_reuse is set) balance from coins sent to addresses that were previously spent from (potentially privacy violating)"}, }}, RESULT_LAST_PROCESSED_BLOCK, @@ -432,18 +433,17 @@ RPCMethod getbalances() LOCK(wallet.cs_wallet); - const auto bal = GetBalance(wallet); + const auto bal = GetBalance(wallet, /*min_depth=*/0, /*avoid_reuse=*/true, /*include_nonmempool=*/true); + UniValue balances{UniValue::VOBJ}; { UniValue balances_mine{UniValue::VOBJ}; balances_mine.pushKV("trusted", ValueFromAmount(bal.m_mine_trusted)); balances_mine.pushKV("untrusted_pending", ValueFromAmount(bal.m_mine_untrusted_pending)); balances_mine.pushKV("immature", ValueFromAmount(bal.m_mine_immature)); + balances_mine.pushKV("nonmempool", ValueFromAmount(bal.m_mine_nonmempool)); if (wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) { - // If the AVOID_REUSE flag is set, bal has been set to just the un-reused address balance. Get - // the total balance, and then subtract bal to get the reused address balance. - const auto full_bal = GetBalance(wallet, 0, false); - balances_mine.pushKV("used", ValueFromAmount(full_bal.m_mine_trusted + full_bal.m_mine_untrusted_pending - bal.m_mine_trusted - bal.m_mine_untrusted_pending)); + balances_mine.pushKV("used", ValueFromAmount(bal.m_mine_used)); } balances.pushKV("mine", std::move(balances_mine)); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 1b8f547f903..d25c39128b2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -794,6 +794,29 @@ bool CWallet::IsSpent(const COutPoint& outpoint) const return false; } +CWallet::SpendType CWallet::HowSpent(const COutPoint& outpoint) const +{ + SpendType st{SpendType::UNSPENT}; + + std::pair range; + range = mapTxSpends.equal_range(outpoint); + + for (TxSpends::const_iterator it = range.first; it != range.second; ++it) { + const Txid& txid = it->second; + const auto mit = mapWallet.find(txid); + if (mit != mapWallet.end()) { + const auto& wtx = mit->second; + if (wtx.isConfirmed()) return SpendType::CONFIRMED; + if (wtx.InMempool()) { + st = SpendType::MEMPOOL; + } else if (!wtx.isAbandoned() && !wtx.isBlockConflicted() && !wtx.isMempoolConflicted()) { + if (st == SpendType::UNSPENT) st = SpendType::NONMEMPOOL; + } + } + } + return st; +} + void CWallet::AddToSpends(const COutPoint& outpoint, const Txid& txid) { mapTxSpends.insert(std::make_pair(outpoint, txid)); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 45b9b8abb12..9cd78258454 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -553,6 +553,13 @@ public: int GetTxBlocksToMaturity(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsTxImmatureCoinBase(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + enum class SpendType { + UNSPENT, + CONFIRMED, + MEMPOOL, + NONMEMPOOL, + }; + SpendType HowSpent(const COutPoint& outpoint) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsSpent(const COutPoint& outpoint) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); // Whether this or any known scriptPubKey with the same single key has been spent. diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index afd0f504eb2..a2e421fd2ba 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -115,10 +115,9 @@ class AbandonConflictTest(BitcoinTestFramework): # inputs are still spent, but change not received newbalance = alice.getbalance() assert_equal(newbalance, balance - signed3_change) - # Unconfirmed received funds that are not in mempool, also shouldn't show - # up in unconfirmed balance + # Unconfirmed received funds that are not in mempool balances = alice.getbalances()['mine'] - assert_equal(balances['untrusted_pending'] + balances['trusted'], newbalance) + assert_equal(balances['untrusted_pending'] + balances['trusted'] + balances['nonmempool'], newbalance) # Also shouldn't show up in listunspent assert txABC2 not in [utxo["txid"] for utxo in alice.listunspent(0)] balance = newbalance diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index b2d0be78208..da51b0e5f7e 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -145,10 +145,12 @@ class WalletTest(BitcoinTestFramework): # getbalances expected_balances_0 = {'mine': {'immature': Decimal('0E-8'), 'trusted': Decimal('9.99'), # change from node 0's send - 'untrusted_pending': Decimal('60.0')}} + 'untrusted_pending': Decimal('60.0'), + 'nonmempool': Decimal('0.0')}} expected_balances_1 = {'mine': {'immature': Decimal('0E-8'), 'trusted': Decimal('0E-8'), # node 1's send had an unsafe input - 'untrusted_pending': Decimal('30.0') - fee_node_1}} # Doesn't include output of node 0's send since it was spent + 'untrusted_pending': Decimal('30.0') - fee_node_1, # Doesn't include output of node 0's send since it was spent + 'nonmempool': Decimal('0.0')}} balances_0 = self.nodes[0].getbalances() balances_1 = self.nodes[1].getbalances() # remove lastprocessedblock keys (they will be tested later) diff --git a/test/functional/wallet_conflicts.py b/test/functional/wallet_conflicts.py index 80a0a851016..e5fca5af72e 100755 --- a/test/functional/wallet_conflicts.py +++ b/test/functional/wallet_conflicts.py @@ -305,8 +305,9 @@ class TxConflicts(BitcoinTestFramework): bob.sendrawtransaction(tx1_conflict_conflict) # kick tx1_conflict out of the mempool bob.sendrawtransaction(raw_tx1) #re-broadcast tx1 because it is no longer conflicted - # Now bob has no pending funds because tx1 and tx2 are spent by tx3, which hasn't been re-broadcast yet - assert_equal(bob.getbalances()["mine"]["untrusted_pending"], 0) + # Now bob has pending funds because tx1 and tx2 are spent by tx3, which hasn't been re-broadcast yet + bob_bal = bob.getbalances()["mine"] + assert_equal(bob_bal["untrusted_pending"], -bob_bal["nonmempool"]) bob.sendrawtransaction(raw_tx3) assert_equal(len(bob.getrawmempool()), 4) # The mempool contains: tx1, tx2, tx1_conflict_conflict, tx3 diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index b1554902da4..d4489eaa1c9 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -10,6 +10,7 @@ import random import shutil import struct import time +from decimal import Decimal from test_framework.address import ( key_to_p2pkh, @@ -524,6 +525,7 @@ class WalletMigrationTest(BitcoinTestFramework): self.generatetodescriptor(self.master_node, 1, desc) bals = wallet.getbalances() + bals["mine"]["nonmempool"] = Decimal('0.0') _, wallet = self.migrate_and_get_rpc("pkcb") @@ -539,6 +541,7 @@ class WalletMigrationTest(BitcoinTestFramework): txid = default.sendtoaddress(addr, 1) self.generate(self.master_node, 1) bals = wallet.getbalances() + bals["mine"]["nonmempool"] = Decimal('0.0') # Use self.migrate_and_get_rpc to test this error to get everything copied over to the master node assert_raises_rpc_error(-4, "Error: Wallet decryption failed, the wallet passphrase was not provided or was incorrect", self.migrate_and_get_rpc, "encrypted") @@ -579,6 +582,7 @@ class WalletMigrationTest(BitcoinTestFramework): txid = default.sendtoaddress(addr, 1) self.generate(self.master_node, 1) bals = wallet.getbalances() + bals["mine"]["nonmempool"] = Decimal('0.0') wallet.unloadwallet() @@ -618,6 +622,7 @@ class WalletMigrationTest(BitcoinTestFramework): txid = default.sendtoaddress(addr, 1) self.generate(self.master_node, 1) bals = wallet.getbalances() + bals["mine"]["nonmempool"] = Decimal('0.0') migrate_res, wallet = self.migrate_and_get_rpc(relative_name) @@ -638,6 +643,7 @@ class WalletMigrationTest(BitcoinTestFramework): self.old_node.restorewallet("relative_restored", migrate_res['backup_path']) wallet = self.old_node.get_wallet_rpc("relative_restored") assert wallet.gettransaction(txid) + del bals["mine"]["nonmempool"] assert_equal(bals, wallet.getbalances()) info = wallet.getwalletinfo() @@ -654,6 +660,7 @@ class WalletMigrationTest(BitcoinTestFramework): txid = default.sendtoaddress(addr, 1) self.generate(self.master_node, 1) bals = wallet.getbalances() + bals["mine"]["nonmempool"] = Decimal('0.0') _, wallet = self.migrate_and_get_rpc(wallet_path) @@ -1515,14 +1522,14 @@ class WalletMigrationTest(BitcoinTestFramework): _, wallet = self.migrate_and_get_rpc("miniscript") # The miniscript with all keys should be in the migrated wallet - assert_equal(wallet.getbalances()["mine"], {"trusted": 0.75, "untrusted_pending": 0, "immature": 0}) + assert_equal(wallet.getbalances()["mine"], {"trusted": 0.75, "untrusted_pending": 0, "immature": 0, "nonmempool": 0}) assert_equal(wallet.getaddressinfo(all_keys_addr)["ismine"], True) assert_equal(wallet.getaddressinfo(some_keys_addr)["ismine"], False) # The miniscript with some keys should be in the watchonly wallet assert "miniscript_watchonly" in self.master_node.listwallets() watchonly = self.master_node.get_wallet_rpc("miniscript_watchonly") - assert_equal(watchonly.getbalances()["mine"], {"trusted": 1, "untrusted_pending": 0, "immature": 0}) + assert_equal(watchonly.getbalances()["mine"], {"trusted": 1, "untrusted_pending": 0, "immature": 0, "nonmempool": 0}) assert_equal(watchonly.getaddressinfo(some_keys_addr)["ismine"], True) assert_equal(watchonly.getaddressinfo(all_keys_addr)["ismine"], False) @@ -1571,7 +1578,7 @@ class WalletMigrationTest(BitcoinTestFramework): res, wallet = self.migrate_and_get_rpc("taproot") # The rawtr should be migrated - assert_equal(wallet.getbalances()["mine"], {"trusted": 0.5, "untrusted_pending": 0, "immature": 0}) + assert_equal(wallet.getbalances()["mine"], {"trusted": 0.5, "untrusted_pending": 0, "immature": 0, "nonmempool": 0}) assert_equal(wallet.getaddressinfo(rawtr_addr)["ismine"], True) assert_equal(wallet.getaddressinfo(tr_addr)["ismine"], False) assert_equal(wallet.getaddressinfo(tr_script_addr)["ismine"], False) @@ -1579,7 +1586,7 @@ class WalletMigrationTest(BitcoinTestFramework): # The tr() with some keys should be in the watchonly wallet assert "taproot_watchonly" in self.master_node.listwallets() watchonly = self.master_node.get_wallet_rpc("taproot_watchonly") - assert_equal(watchonly.getbalances()["mine"], {"trusted": 5, "untrusted_pending": 0, "immature": 0}) + assert_equal(watchonly.getbalances()["mine"], {"trusted": 5, "untrusted_pending": 0, "immature": 0, "nonmempool": 0}) assert_equal(watchonly.getaddressinfo(rawtr_addr)["ismine"], False) assert_equal(watchonly.getaddressinfo(tr_addr)["ismine"], True) assert_equal(watchonly.getaddressinfo(tr_script_addr)["ismine"], True) diff --git a/test/functional/wallet_orphanedreward.py b/test/functional/wallet_orphanedreward.py index 2ab50f79ea5..b285f0b9679 100755 --- a/test/functional/wallet_orphanedreward.py +++ b/test/functional/wallet_orphanedreward.py @@ -46,6 +46,7 @@ class OrphanedBlockRewardTest(BitcoinTestFramework): "trusted": 10, "untrusted_pending": 0, "immature": 0, + "nonmempool": 0, }) # And the unconfirmed tx to be abandoned assert_equal(self.nodes[1].gettransaction(txid)["details"][0]["abandoned"], True) diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index e6a740501e3..213c475634d 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -32,8 +32,8 @@ class WalletSendTest(BitcoinTestFramework): self.noban_tx_relay = True self.supports_cli = False self.extra_args = [ - ["-walletrbf=1"], - ["-walletrbf=1"] + ["-walletrbf=1", "-datacarriersize=16"], + ["-walletrbf=1", "-datacarriersize=16"] ] getcontext().prec = 8 # Satoshi precision for Decimal @@ -45,7 +45,7 @@ class WalletSendTest(BitcoinTestFramework): conf_target=None, estimate_mode=None, fee_rate=None, add_to_wallet=None, psbt=None, inputs=None, add_inputs=None, include_unsafe=None, change_address=None, change_position=None, change_type=None, locktime=None, lock_unspents=None, replaceable=None, subtract_fee_from_outputs=None, - expect_error=None, solving_data=None, minconf=None): + expect_error=None, solving_data=None, minconf=None, nonmempool=False): assert_not_equal((amount is None), (data is None)) from_balance_before = from_wallet.getbalances()["mine"]["trusted"] @@ -171,16 +171,20 @@ class WalletSendTest(BitcoinTestFramework): tx = from_wallet.gettransaction(res["txid"]) assert tx assert_equal(tx["bip125-replaceable"], "yes" if replaceable else "no") - # Ensure transaction exists in the mempool: - tx = from_wallet.getrawtransaction(res["txid"], True) - assert tx - if amount: - if subtract_fee_from_outputs: - assert_equal(from_balance_before - from_balance, amount) - else: - assert_greater_than(from_balance_before - from_balance, amount) + if nonmempool: + assert_raises_rpc_error(-5, "No such mempool transaction", from_wallet.getrawtransaction, res["txid"]) + assert from_wallet.getbalances()["mine"]["nonmempool"] < 0 else: - assert next((out for out in tx["vout"] if out["scriptPubKey"]["asm"] == "OP_RETURN 35"), None) + # Ensure transaction exists in the mempool: + tx = from_wallet.getrawtransaction(res["txid"], True) + assert tx + if amount: + if subtract_fee_from_outputs: + assert_equal(from_balance_before - from_balance, amount) + else: + assert_greater_than(from_balance_before - from_balance, amount) + else: + assert next((out for out in tx["vout"] if out["scriptPubKey"]["asm"] == "OP_RETURN 35"), None) else: assert_equal(from_balance_before, from_balance) @@ -280,6 +284,9 @@ class WalletSendTest(BitcoinTestFramework): res = w2.walletprocesspsbt(res["psbt"]) assert res["complete"] + self.log.info("Create mempool-invalid tx (due to large OP_RETURN)...") + self.test_send(from_wallet=w0, data=b"The quick brown fox jumps over the lazy dog".hex(), nonmempool=True) + self.log.info("Test setting explicit fee rate") res1 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate="1", add_to_wallet=False) res2 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate="1", add_to_wallet=False) diff --git a/test/functional/wallet_v3_txs.py b/test/functional/wallet_v3_txs.py index 20fd0ebbbf6..446dc086a3c 100755 --- a/test/functional/wallet_v3_txs.py +++ b/test/functional/wallet_v3_txs.py @@ -43,19 +43,19 @@ def cleanup(func): func(self, *args) finally: self.generate(self.nodes[0], 1) - try: - self.alice.sendall([self.charlie.getnewaddress()]) - except JSONRPCException as e: - assert "Total value of UTXO pool too low to pay for transaction" in e.error['message'] - try: - self.bob.sendall([self.charlie.getnewaddress()]) - except JSONRPCException as e: - assert "Total value of UTXO pool too low to pay for transaction" in e.error['message'] + for wallet in [self.alice, self.bob]: + txs = set(tx["txid"] for tx in wallet.listtransactions("*", 1000) if tx["confirmations"] == 0 and not tx["abandoned"]) + for tx in txs: + wallet.abandontransaction(tx) + try: + wallet.sendall([self.charlie.getnewaddress()]) + except JSONRPCException as e: + assert "Total value of UTXO pool too low to pay for transaction" in e.error['message'] self.generate(self.nodes[0], 1) for wallet in [self.alice, self.bob]: balance = wallet.getbalances()["mine"] - for balance_type in ["untrusted_pending", "trusted", "immature"]: + for balance_type in ["untrusted_pending", "trusted", "immature", "nonmempool"]: assert_equal(balance[balance_type], 0) assert_equal(self.alice.getrawmempool(), [])