From 0469c6a9aeecdbe3ff80764271e2acfa887602e4 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 8 Dec 2021 18:29:54 +0100 Subject: [PATCH 1/6] Merge bitcoin/bitcoin#23712: test: interface_bitcoin_cli.py: check specified wallet type availability b57bf25cfee3d3d0e08e5565b23e5e12ea88aee5 test: interface_bitcoin_cli.py: check specified wallet type availability (Sebastian Falbesoner) Pull request description: Currently the test `interface_bitcoin_cli.py` performs the wallet-relevant parts if _any_ wallet type support is compiled in, independently of whether the test is run with legacy or descriptor wallet specified. This leads to a failure if the test is started with the `--legacy-wallet` parameter, but bitcoind is compiled without BDB support, see e.g https://github.com/bitcoin/bitcoin/pull/23686#issuecomment-987705540 Fix this by checking if the specified wallet type (BDB for legacy wallet, SQLite for descriptor wallet) is available. Should further pave the way for #23682. ACKs for top commit: achow101: ACK b57bf25cfee3d3d0e08e5565b23e5e12ea88aee5 Tree-SHA512: ddb5a94ba61133eff8de79d4946b3b9d476232b26e83bf768894cac4691e72602f88b6c02c72b992e12c2feb9bff1f0a2e0a265948a00954311104add1347184 --- test/functional/interface_bitcoin_cli.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index 671ba832dd..6e1a8d5b4d 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -66,10 +66,16 @@ def cli_get_info_string_to_dict(cli_get_info_string): class TestBitcoinCli(BitcoinTestFramework): + def is_specified_wallet_compiled(self): + if self.options.descriptors: + return self.is_sqlite_compiled() + else: + return self.is_bdb_compiled() + def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 1 - if self.is_wallet_compiled(): + if self.is_specified_wallet_compiled(): self.requires_wallet = True def skip_test_if_missing_module(self): @@ -122,7 +128,7 @@ class TestBitcoinCli(BitcoinTestFramework): assert_raises_process_error(1, "Invalid value for -color option. Valid values: always, auto, never.", self.nodes[0].cli('-getinfo', '-color=foo').send_cli) self.log.info("Test -getinfo returns expected network and blockchain info") - if self.is_wallet_compiled(): + if self.is_specified_wallet_compiled(): self.nodes[0].encryptwallet(password) cli_get_info_string = self.nodes[0].cli('-getinfo').send_cli() cli_get_info = cli_get_info_string_to_dict(cli_get_info_string) @@ -147,7 +153,7 @@ class TestBitcoinCli(BitcoinTestFramework): cli_get_info = cli_get_info_string_to_dict(cli_get_info_string) assert_equal(cli_get_info["Proxies"], "127.0.0.1:9050 (ipv4, ipv6, onion, cjdns), 127.0.0.1:7656 (i2p)") - if self.is_wallet_compiled(): + if self.is_specified_wallet_compiled(): self.log.info("Test -getinfo and dash-cli getwalletinfo return expected wallet info") assert_equal(Decimal(cli_get_info['Balance']), BALANCE) assert 'Balances' not in cli_get_info_string From 6801e0d1ed6ff851d09d7e820af56d8ff24e8ed9 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 9 Dec 2021 13:44:11 +0100 Subject: [PATCH 2/6] Merge bitcoin/bitcoin#23346: util, refactor: Improve headers for bitcoin-wallet tool 3431839c33fa3892c982322e4add39e28ddba719 util, refactor: Improve headers for bitcoin-wallet tool (Hennadii Stepanov) Pull request description: This PR: - removes unneeded `#include ` from `` - introduces class forward declaration in `` - added `#include ` to `wallet/wallettool.cpp` where the `USE_BDB` macro is used Top commit has no ACKs. Tree-SHA512: a0de560d821f8b570ae806a1165b9b382c9e0b339687d932052fa4c38ab2ba493e7e050f19adc02ad7db40c42cf88ac1d37209f9071494a0ab268ed33ff22b9f --- src/bitcoin-wallet.cpp | 7 +++++++ src/wallet/wallettool.cpp | 6 ++++++ src/wallet/wallettool.h | 5 +++-- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index 176b455ce9..81d9fc8d67 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -11,11 +11,18 @@ #include #include #include +#include +#include +#include +#include #include #include #include #include +#include +#include +#include const std::function G_TRANSLATION_FUN = nullptr; UrlDecodeFn* const URL_DECODE = nullptr; diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 96ca344c4e..98271775c5 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -2,6 +2,12 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#if defined(HAVE_CONFIG_H) +#include +#endif + +#include + #include #include #include diff --git a/src/wallet/wallettool.h b/src/wallet/wallettool.h index f4516bb5bc..09be1dac2c 100644 --- a/src/wallet/wallettool.h +++ b/src/wallet/wallettool.h @@ -5,11 +5,12 @@ #ifndef BITCOIN_WALLET_WALLETTOOL_H #define BITCOIN_WALLET_WALLETTOOL_H -#include +#include + +class ArgsManager; namespace WalletTool { -void WalletShowInfo(CWallet* wallet_instance); bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command); } // namespace WalletTool From 7bfcbb2f89843280d7dfdc8262dce77735800128 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 29 Sep 2021 08:08:08 +0200 Subject: [PATCH 3/6] Merge bitcoin/bitcoin#23120: test: Remove unused and confusing main parameter from script_util fa54efda9bc8f8f742dacbc3673516d88d9d601d test: pep-8 touched test (MarcoFalke) fa4676805910bfea5549f5b51460c8456bc8945c test: Remove unused and confusing main parameter from script_util (MarcoFalke) Pull request description: ACKs for top commit: fanquake: ACK fa54efda9bc8f8f742dacbc3673516d88d9d601d Tree-SHA512: 124e888e16c92bb24ab4d6f68a768d295500a48f8c6c0b4f069493effcd761f80be78dd93b31edbb529ebe4c8aaca764aaff48d1e3b23659dde722981dc787a5 --- test/functional/test_framework/script_util.py | 35 +++++++++++++------ 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/test/functional/test_framework/script_util.py b/test/functional/test_framework/script_util.py index 40381c6fc1..05cf25d6cc 100755 --- a/test/functional/test_framework/script_util.py +++ b/test/functional/test_framework/script_util.py @@ -3,7 +3,16 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Useful Script constants and utils.""" -from test_framework.script import CScript, hash160, OP_DUP, OP_HASH160, OP_CHECKSIG, OP_EQUAL, OP_EQUALVERIFY +from test_framework.script import ( + CScript, + hash160, + OP_0, + OP_DUP, + OP_HASH160, + OP_CHECKSIG, + OP_EQUAL, + OP_EQUALVERIFY, +) # To prevent a "tx-size-small" policy rule error, a transaction has to have a # size of at least 83 bytes (MIN_STANDARD_TX_SIZE in @@ -25,32 +34,38 @@ from test_framework.script import CScript, hash160, OP_DUP, OP_HASH160, OP_CHECK DUMMY_P2SH_SCRIPT = CScript([b'a' * 22]) DUMMY_2_P2SH_SCRIPT = CScript([b'b' * 22]) -def keyhash_to_p2pkh_script(hash, main = False): + +def keyhash_to_p2pkh_script(hash): assert len(hash) == 20 return CScript([OP_DUP, OP_HASH160, hash, OP_EQUALVERIFY, OP_CHECKSIG]) -def scripthash_to_p2sh_script(hash, main = False): + +def scripthash_to_p2sh_script(hash): assert len(hash) == 20 return CScript([OP_HASH160, hash, OP_EQUAL]) -def key_to_p2pkh_script(key, main = False): - key = check_key(key) - return keyhash_to_p2pkh_script(hash160(key), main) -def script_to_p2sh_script(script, main = False): +def key_to_p2pkh_script(key): + key = check_key(key) + return keyhash_to_p2pkh_script(hash160(key)) + + +def script_to_p2sh_script(script): script = check_script(script) - return scripthash_to_p2sh_script(hash160(script), main) + return scripthash_to_p2sh_script(hash160(script)) + def check_key(key): if isinstance(key, str): - key = bytes.fromhex(key) # Assuming this is hex string + key = bytes.fromhex(key) # Assuming this is hex string if isinstance(key, bytes) and (len(key) == 33 or len(key) == 65): return key assert False + def check_script(script): if isinstance(script, str): - script = bytes.fromhex(script) # Assuming this is hex string + script = bytes.fromhex(script) # Assuming this is hex string if isinstance(script, bytes) or isinstance(script, CScript): return script assert False From 9e7f0e0d6e9311123dd6cf1941119e169b76f654 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 2 Dec 2021 08:38:24 +0100 Subject: [PATCH 4/6] Merge bitcoin/bitcoin#23640: MOVEONLY: Move helper functions from rpcwallet to wallet/rpc/util MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ff945e553affbb7e25da1257a0dd47e413ec5164 MOVEONLY: Move utility functions from rpcwallet to wallet/rpc/util (Samuel Dobson) 7b04a064f6e8ee9d93e5a5ad54dab20b769083f2 Introduce wallet/rpc/util (Samuel Dobson) Pull request description: This is part one of multiple to split up rpcwallet.cpp into smaller, more logical units. See #23622 for context and overall plan. I'll open PRs in stages to hopefully minimise conflicts. Can be reviewed with `--color-moved=dimmed-zebra` The end goal can be seen here: https://github.com/meshcollider/bitcoin/tree/202111_split_walletrpc ACKs for top commit: MarcoFalke: nice, ACK ff945e553affbb7e25da1257a0dd47e413ec5164 🐰 shaavan: ACK ff945e553affbb7e25da1257a0dd47e413ec5164 Tree-SHA512: 6e3d1de6db770fe2fca540c8c4f30183ab8258c26e3a1fb46937714d28818a52933eafbfcafe2995f6a6e2551a3f3dd3ec93363b81de7912c0d81f5749d1c86d --- src/Makefile.am | 3 +- src/wallet/rpc/util.cpp | 122 +++++++++++++++++++++++++++++++++++++++ src/wallet/rpc/util.h | 38 ++++++++++++ src/wallet/rpcwallet.cpp | 101 +------------------------------- src/wallet/rpcwallet.h | 25 -------- 5 files changed, 163 insertions(+), 126 deletions(-) create mode 100644 src/wallet/rpc/util.cpp create mode 100644 src/wallet/rpc/util.h diff --git a/src/Makefile.am b/src/Makefile.am index 55ffd7a6c6..ff188f3a1f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -397,6 +397,7 @@ BITCOIN_CORE_H = \ wallet/ismine.h \ wallet/load.h \ wallet/rpcwallet.h \ + wallet/rpc/util.h \ wallet/salvage.h \ wallet/scriptpubkeyman.h \ wallet/sqlite.h \ @@ -585,8 +586,8 @@ libbitcoin_wallet_a_SOURCES = \ wallet/hdchain.cpp \ wallet/interfaces.cpp \ wallet/load.cpp \ + wallet/rpc/util.cpp \ wallet/rpcdump.cpp \ - wallet/rpcwallet.cpp \ wallet/scriptpubkeyman.cpp \ wallet/wallet.cpp \ wallet/walletdb.cpp \ diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp new file mode 100644 index 0000000000..b926bfc75f --- /dev/null +++ b/src/wallet/rpc/util.cpp @@ -0,0 +1,122 @@ +// Copyright (c) 2011-2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include + +#include + +static const std::string WALLET_ENDPOINT_BASE = "/wallet/"; +const std::string HELP_REQUIRING_PASSPHRASE{"\nRequires wallet passphrase to be set with walletpassphrase call if wallet is encrypted.\n"}; + +bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param) { + bool can_avoid_reuse = wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); + bool avoid_reuse = param.isNull() ? can_avoid_reuse : param.get_bool(); + + if (avoid_reuse && !can_avoid_reuse) { + throw JSONRPCError(RPC_WALLET_ERROR, "wallet does not have the \"avoid reuse\" feature enabled"); + } + + return avoid_reuse; +} + +/** Used by RPC commands that have an include_watchonly parameter. + * We default to true for watchonly wallets if include_watchonly isn't + * explicitly set. + */ +bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wallet) +{ + if (include_watchonly.isNull()) { + // if include_watchonly isn't explicitly set, then check if we have a watchonly wallet + return wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); + } + + // otherwise return whatever include_watchonly was set to + return include_watchonly.get_bool(); +} + +bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name) +{ + if (URL_DECODE && request.URI.substr(0, WALLET_ENDPOINT_BASE.size()) == WALLET_ENDPOINT_BASE) { + // wallet endpoint was used + wallet_name = URL_DECODE(request.URI.substr(WALLET_ENDPOINT_BASE.size())); + return true; + } + return false; +} + +std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& request) +{ + CHECK_NONFATAL(request.mode == JSONRPCRequest::EXECUTE); + WalletContext& context = EnsureWalletContext(request.context); + + std::string wallet_name; + if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) { + const std::shared_ptr pwallet = GetWallet(context, wallet_name); + if (!pwallet) throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Requested wallet does not exist or is not loaded"); + return pwallet; + } + + std::vector> wallets = GetWallets(context); + if (wallets.size() == 1) { + return wallets[0]; + } + + if (wallets.empty()) { + throw JSONRPCError( + RPC_WALLET_NOT_FOUND, "No wallet is loaded. Load a wallet using loadwallet or create a new one with createwallet. (Note: A default wallet is no longer automatically created)"); + } + throw JSONRPCError(RPC_WALLET_NOT_SPECIFIED, + "Wallet file not specified (must request wallet RPC through /wallet/ uri-path)."); +} + +void EnsureWalletIsUnlocked(const CWallet& wallet) +{ + if (wallet.IsLocked()) { + throw JSONRPCError(RPC_WALLET_UNLOCK_NEEDED, "Error: Please enter the wallet passphrase with walletpassphrase first."); + } +} + +WalletContext& EnsureWalletContext(const std::any& context) +{ + auto wallet_context = util::AnyPtr(context); + if (!wallet_context) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Wallet context not found"); + } + return *wallet_context; +} + +// also_create should only be set to true only when the RPC is expected to add things to a blank wallet and make it no longer blank +LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create) +{ + LegacyScriptPubKeyMan* spk_man = wallet.GetLegacyScriptPubKeyMan(); + if (!spk_man && also_create) { + spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan(); + } + if (!spk_man) { + throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command"); + } + return *spk_man; +} + +const LegacyScriptPubKeyMan& EnsureConstLegacyScriptPubKeyMan(const CWallet& wallet) +{ + const LegacyScriptPubKeyMan* spk_man = wallet.GetLegacyScriptPubKeyMan(); + if (!spk_man) { + throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command"); + } + return *spk_man; +} + +std::string LabelFromValue(const UniValue& value) +{ + std::string label = value.get_str(); + if (label == "*") + throw JSONRPCError(RPC_WALLET_INVALID_LABEL_NAME, "Invalid label name"); + return label; +} diff --git a/src/wallet/rpc/util.h b/src/wallet/rpc/util.h new file mode 100644 index 0000000000..a493a80a74 --- /dev/null +++ b/src/wallet/rpc/util.h @@ -0,0 +1,38 @@ +// Copyright (c) 2017-2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_WALLET_RPC_UTIL_H +#define BITCOIN_WALLET_RPC_UTIL_H + +#include +#include +#include + +class CWallet; +class JSONRPCRequest; +class LegacyScriptPubKeyMan; +class UniValue; +struct WalletContext; + +extern const std::string HELP_REQUIRING_PASSPHRASE; + +/** + * Figures out what wallet, if any, to use for a JSONRPCRequest. + * + * @param[in] request JSONRPCRequest that wishes to access a wallet + * @return nullptr if no wallet should be used, or a pointer to the CWallet + */ +std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& request); +bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name); + +void EnsureWalletIsUnlocked(const CWallet&); +WalletContext& EnsureWalletContext(const std::any& context); +LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create = false); +const LegacyScriptPubKeyMan& EnsureConstLegacyScriptPubKeyMan(const CWallet& wallet); + +bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param); +bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wallet); +std::string LabelFromValue(const UniValue& value); + +#endif // BITCOIN_WALLET_RPC_UTIL_H diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index b4ca6de160..8deb51496f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -49,35 +50,6 @@ using interfaces::FoundBlock; -static const std::string WALLET_ENDPOINT_BASE = "/wallet/"; - -static inline bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param) { - bool can_avoid_reuse = wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); - bool avoid_reuse = param.isNull() ? can_avoid_reuse : param.get_bool(); - - if (avoid_reuse && !can_avoid_reuse) { - throw JSONRPCError(RPC_WALLET_ERROR, "wallet does not have the \"avoid reuse\" feature enabled"); - } - - return avoid_reuse; -} - - -/** Used by RPC commands that have an include_watchonly parameter. - * We default to true for watchonly wallets if include_watchonly isn't - * explicitly set. - */ -static bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wallet) -{ - if (include_watchonly.isNull()) { - // if include_watchonly isn't explicitly set, then check if we have a watchonly wallet - return wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); - } - - // otherwise return whatever include_watchonly was set to - return include_watchonly.get_bool(); -} - /** Checks if a CKey is in the given CWallet compressed or otherwise*/ bool HaveKey(const SigningProvider& wallet, const CKey& key) @@ -87,69 +59,6 @@ bool HaveKey(const SigningProvider& wallet, const CKey& key) return wallet.HaveKey(key.GetPubKey().GetID()) || wallet.HaveKey(key2.GetPubKey().GetID()); } -bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name) -{ - if (URL_DECODE && request.URI.substr(0, WALLET_ENDPOINT_BASE.size()) == WALLET_ENDPOINT_BASE) { - // wallet endpoint was used - wallet_name = URL_DECODE(request.URI.substr(WALLET_ENDPOINT_BASE.size())); - return true; - } - return false; -} - -std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& request) -{ - CHECK_NONFATAL(request.mode == JSONRPCRequest::EXECUTE); - - std::string wallet_name; - if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) { - std::shared_ptr pwallet = GetWallet(wallet_name); - if (!pwallet) throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Requested wallet does not exist or is not loaded"); - return pwallet; - } - - std::vector> wallets = GetWallets(); - if (wallets.size() == 1) { - return wallets[0]; - } - - if (wallets.empty()) { - throw JSONRPCError( - RPC_WALLET_NOT_FOUND, "No wallet is loaded. Load a wallet using loadwallet or create a new one with createwallet. (Note: A default wallet is no longer automatically created)"); - } - throw JSONRPCError(RPC_WALLET_NOT_SPECIFIED, - "Wallet file not specified (must request wallet RPC through /wallet/ uri-path)."); -} - -void EnsureWalletIsUnlocked(const CWallet& wallet) -{ - if (wallet.IsLocked()) { - throw JSONRPCError(RPC_WALLET_UNLOCK_NEEDED, "Error: Please enter the wallet passphrase with walletpassphrase first."); - } -} - -// also_create should only be set to true only when the RPC is expected to add things to a blank wallet and make it no longer blank -LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create) -{ - LegacyScriptPubKeyMan* spk_man = wallet.GetLegacyScriptPubKeyMan(); - if (!spk_man && also_create) { - spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan(); - } - if (!spk_man) { - throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command"); - } - return *spk_man; -} - -WalletContext& EnsureWalletContext(const CoreContext& context) -{ - auto* wallet_context = GetContext(context); - if (!wallet_context) { - throw JSONRPCError(RPC_INTERNAL_ERROR, "Wallet context not found"); - } - return *wallet_context; -} - static void WalletTxToJSON(interfaces::Chain& chain, const CWalletTx& wtx, UniValue& entry) { int confirms = wtx.GetDepthInMainChain(); @@ -191,14 +100,6 @@ static void WalletTxToJSON(interfaces::Chain& chain, const CWalletTx& wtx, UniVa } -static std::string LabelFromValue(const UniValue& value) -{ - std::string label = value.get_str(); - if (label == "*") - throw JSONRPCError(RPC_WALLET_INVALID_LABEL_NAME, "Invalid label name"); - return label; -} - /** * Update coin control with fee estimation based on the given parameters * diff --git a/src/wallet/rpcwallet.h b/src/wallet/rpcwallet.h index 5c8c643a43..5da5623335 100644 --- a/src/wallet/rpcwallet.h +++ b/src/wallet/rpcwallet.h @@ -8,35 +8,10 @@ #include #include -#include -#include -#include - class CRPCCommand; -class CWallet; -class JSONRPCRequest; -class LegacyScriptPubKeyMan; -class UniValue; -class CTransaction; -struct PartiallySignedTransaction; -struct WalletContext; - -static const std::string HELP_REQUIRING_PASSPHRASE{"\nRequires wallet passphrase to be set with walletpassphrase call if wallet is encrypted.\n"}; Span GetWalletRPCCommands(); -/** - * Figures out what wallet, if any, to use for a JSONRPCRequest. - * - * @param[in] request JSONRPCRequest that wishes to access a wallet - * @return nullptr if no wallet should be used, or a pointer to the CWallet - */ -std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& request); - -void EnsureWalletIsUnlocked(const CWallet&); -WalletContext& EnsureWalletContext(const CoreContext& context); -LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create = false); - RPCHelpMan getaddressinfo(); RPCHelpMan getrawchangeaddress(); RPCHelpMan addmultisigaddress(); From 18ddb83fbc7148276477dcb52e12bdd744b1ced2 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 17 May 2021 16:31:06 -0400 Subject: [PATCH 5/6] Merge #22008: wallet: Cleanup and refactor CreateTransactionInternal -BEGIN VERIFY SCRIPT- sed -i 's/SelectCoinsMinConf/AttemptSelection/g' $(git grep -l SelectCoinsMinConf ./src) -END VERIFY SCRIPT- --- src/bench/coin_selection.cpp | 2 +- src/wallet/test/coinselector_tests.cpp | 84 +++++++++++++------------- src/wallet/wallet.cpp | 20 +++--- src/wallet/wallet.h | 4 +- 4 files changed, 55 insertions(+), 55 deletions(-) diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index e53755c046..eeec1f33ee 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -56,7 +56,7 @@ static void CoinSelection(benchmark::Bench& bench) std::set setCoinsRet; CAmount nValueRet; bool bnb_used; - bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used); + bool success = wallet.AttemptSelection(1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used); assert(success); assert(nValueRet == 1003 * COIN); assert(setCoinsRet.size() == 2); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index e7731a9c4d..15efed9070 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -255,7 +255,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 1 * CENT, 2 * CENT, selection, value_ret, not_input_fees)); } - // Make sure that effective value is working in SelectCoinsMinConf when BnB is used + // Make sure that effective value is working in AttemptSelection when BnB is used CoinSelectionParams coin_selection_params_bnb(/* use_bnb= */ true, /* change_output_size= */ 0, /* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(3000), /* long_term_feerate= */ CFeeRate(1000), /* discard_feerate= */ CFeeRate(1000), @@ -273,14 +273,14 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) add_coin(coins, *wallet, 1); coins.at(0).nInputBytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail - BOOST_CHECK(!wallet->SelectCoinsMinConf(1 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); + BOOST_CHECK(!wallet->AttemptSelection(1 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); // Test fees subtracted from output: coins.clear(); add_coin(coins, *wallet, 1 * CENT); coins.at(0).nInputBytes = 40; coin_selection_params_bnb.m_subtract_fee_outputs = true; - BOOST_CHECK(wallet->SelectCoinsMinConf(1 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(1 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); } @@ -326,24 +326,24 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) coins.clear(); // with an empty wallet we can't even pay one cent - BOOST_CHECK(!wallet->SelectCoinsMinConf( 1 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!wallet->AttemptSelection( 1 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); add_coin(coins, *wallet, 1*CENT, 4); // add a new 1 cent coin // with a new 1 cent coin, we still can't find a mature 1 cent - BOOST_CHECK(!wallet->SelectCoinsMinConf( 1 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!wallet->AttemptSelection( 1 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // but we can find a new 1 cent - BOOST_CHECK(wallet->SelectCoinsMinConf( 1 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection( 1 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); add_coin(coins, *wallet, 2*CENT); // add a mature 2 cent coin // we can't make 3 cents of mature coins - BOOST_CHECK(!wallet->SelectCoinsMinConf( 3 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!wallet->AttemptSelection( 3 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // we can make 3 cents of new coins - BOOST_CHECK(wallet->SelectCoinsMinConf( 3 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection( 3 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); add_coin(coins, *wallet, 5*CENT); // add a mature 5 cent coin, @@ -353,33 +353,33 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27. total = 38 // we can't make 38 cents only if we disallow new coins: - BOOST_CHECK(!wallet->SelectCoinsMinConf(38 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!wallet->AttemptSelection(38 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // we can't even make 37 cents if we don't allow new coins even if they're from us - BOOST_CHECK(!wallet->SelectCoinsMinConf(38 * CENT, filter_standard_extra, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!wallet->AttemptSelection(38 * CENT, filter_standard_extra, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK(wallet->SelectCoinsMinConf(37 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(37 * CENT, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); // and we can make 38 cents if we accept all new coins - BOOST_CHECK(wallet->SelectCoinsMinConf(38 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(38 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 38 * CENT); // try making 34 cents from 1,2,5,10,20 - we can't do it exactly - BOOST_CHECK(wallet->SelectCoinsMinConf(34 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(34 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 35 * CENT); // but 35 cents is closest BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) // when we try making 7 cents, the smaller coins (1,2,5) are enough. We should see just 2+5 - BOOST_CHECK(wallet->SelectCoinsMinConf( 7 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection( 7 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 7 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. - BOOST_CHECK(wallet->SelectCoinsMinConf( 8 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection( 8 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK(nValueRet == 8 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) - BOOST_CHECK(wallet->SelectCoinsMinConf( 9 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection( 9 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -393,30 +393,30 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(coins, *wallet, 30*CENT); // now we have 6+7+8+20+30 = 71 cents total // check that we have 71 and not 72 - BOOST_CHECK(wallet->SelectCoinsMinConf(71 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); - BOOST_CHECK(!wallet->SelectCoinsMinConf(72 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(71 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!wallet->AttemptSelection(72 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // now try making 16 cents. the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20 - BOOST_CHECK(wallet->SelectCoinsMinConf(16 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(16 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 20 * CENT); // we should get 20 in one coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); add_coin(coins, *wallet, 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20 - BOOST_CHECK(wallet->SelectCoinsMinConf(16 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(16 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 3 coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); add_coin(coins, *wallet, 18*CENT); // now we have 5+6+7+8+18+20+30 // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18 - BOOST_CHECK(wallet->SelectCoinsMinConf(16 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(16 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // because in the event of a tie, the biggest coin wins // now try making 11 cents. we should get 5+6 - BOOST_CHECK(wallet->SelectCoinsMinConf(11 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(11 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -425,11 +425,11 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(coins, *wallet, 2*COIN); add_coin(coins, *wallet, 3*COIN); add_coin(coins, *wallet, 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents - BOOST_CHECK(wallet->SelectCoinsMinConf(95 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(95 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * COIN); // we should get 1 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - BOOST_CHECK(wallet->SelectCoinsMinConf(195 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(195 * CENT, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -444,14 +444,14 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // try making 1 * MIN_CHANGE from the 1.5 * MIN_CHANGE // we'll get change smaller than MIN_CHANGE whatever happens, so can expect MIN_CHANGE exactly - BOOST_CHECK(wallet->SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(MIN_CHANGE, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // but if we add a bigger coin, small change is avoided add_coin(coins, *wallet, 1111*MIN_CHANGE); // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 - BOOST_CHECK(wallet->SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(1 * MIN_CHANGE, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // if we add more small coins: @@ -459,7 +459,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(coins, *wallet, MIN_CHANGE * 7 / 10); // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK(wallet->SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(1 * MIN_CHANGE, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // run the 'mtgox' test (see https://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) @@ -468,7 +468,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) for (int j = 0; j < 20; j++) add_coin(coins, *wallet, 50000 * COIN); - BOOST_CHECK(wallet->SelectCoinsMinConf(500000 * COIN, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(500000 * COIN, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins @@ -481,7 +481,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(coins, *wallet, MIN_CHANGE * 6 / 10); add_coin(coins, *wallet, MIN_CHANGE * 7 / 10); add_coin(coins, *wallet, 1111 * MIN_CHANGE); - BOOST_CHECK(wallet->SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(1 * MIN_CHANGE, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -491,7 +491,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(coins, *wallet, MIN_CHANGE * 6 / 10); add_coin(coins, *wallet, MIN_CHANGE * 8 / 10); add_coin(coins, *wallet, 1111 * MIN_CHANGE); - BOOST_CHECK(wallet->SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(MIN_CHANGE, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // in two coins 0.4+0.6 @@ -502,12 +502,12 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(coins, *wallet, MIN_CHANGE * 100); // trying to make 100.01 from these three coins - BOOST_CHECK(wallet->SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(MIN_CHANGE * 10001 / 100, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE * 10105 / 100); // we should get all coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change - BOOST_CHECK(wallet->SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(MIN_CHANGE * 9990 / 100, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); } @@ -521,7 +521,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // We only create the wallet once to save time, but we still run the coin selection RUN_TESTS times. for (int i = 0; i < RUN_TESTS; i++) { - BOOST_CHECK(wallet->SelectCoinsMinConf(2000, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(2000, filter_confirmed, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); if (amt - 2000 < MIN_CHANGE) { // needs more than one input: @@ -547,8 +547,8 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) for (int i = 0; i < RUN_TESTS; i++) { // picking 50 from 100 coins doesn't depend on the shuffle, // but does depend on randomness in the stochastic approximation code - BOOST_CHECK(wallet->SelectCoinsMinConf(50 * COIN, filter_standard, coins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); - BOOST_CHECK(wallet->SelectCoinsMinConf(50 * COIN, filter_standard, coins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(50 * COIN, filter_standard, coins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(50 * COIN, filter_standard, coins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK(!equal_sets(setCoinsRet, setCoinsRet2)); int fails = 0; @@ -556,8 +556,8 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) { // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time // run the test RANDOM_REPEATS times and only complain if all of them fail - BOOST_CHECK(wallet->SelectCoinsMinConf(COIN, filter_standard, coins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); - BOOST_CHECK(wallet->SelectCoinsMinConf(COIN, filter_standard, coins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(COIN, filter_standard, coins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(COIN, filter_standard, coins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } @@ -579,8 +579,8 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) { // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time // run the test RANDOM_REPEATS times and only complain if all of them fail - BOOST_CHECK(wallet->SelectCoinsMinConf(90*CENT, filter_standard, coins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); - BOOST_CHECK(wallet->SelectCoinsMinConf(90*CENT, filter_standard, coins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(90*CENT, filter_standard, coins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(90*CENT, filter_standard, coins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } @@ -606,7 +606,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) add_coin(coins, *wallet, 1000 * COIN); add_coin(coins, *wallet, 3 * COIN); - BOOST_CHECK(wallet->SelectCoinsMinConf(1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); } @@ -656,8 +656,8 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) CoinSet out_set; CAmount out_value = 0; bool bnb_used = false; - BOOST_CHECK(wallet->SelectCoinsMinConf(target, filter_standard, coins, out_set, out_value, coin_selection_params_bnb, bnb_used) || - wallet->SelectCoinsMinConf(target, filter_standard, coins, out_set, out_value, coin_selection_params_knapsack, bnb_used)); + BOOST_CHECK(wallet->AttemptSelection(target, filter_standard, coins, out_set, out_value, coin_selection_params_bnb, bnb_used) || + wallet->AttemptSelection(target, filter_standard, coins, out_set, out_value, coin_selection_params_knapsack, bnb_used)); BOOST_CHECK_GE(out_value, target); } } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 346a2ec199..b658a9ca70 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2881,7 +2881,7 @@ static bool isGroupISLocked(const OutputGroup& group, interfaces::Chain& chain) }); } -bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector coins, +bool CWallet::AttemptSelection(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector coins, std::set& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used, CoinType nCoinType) const { setCoinsRet.clear(); @@ -3014,32 +3014,32 @@ bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAm // If possible, fund the transaction with confirmed UTXOs only. Prefer at least six // confirmations on outputs received from other wallets and only spend confirmed change. - if (SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 6, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) return true; - if (SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 1, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) return true; + if (AttemptSelection(value_to_select, CoinEligibilityFilter(1, 6, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) return true; + if (AttemptSelection(value_to_select, CoinEligibilityFilter(1, 1, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) return true; // Fall back to using zero confirmation change (but with as few ancestors in the mempool as // possible) if we cannot fund the transaction otherwise. if (m_spend_zero_conf_change) { - if (SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, 2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) return true; - if (SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)), + if (AttemptSelection(value_to_select, CoinEligibilityFilter(0, 1, 2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) return true; + if (AttemptSelection(value_to_select, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) { return true; } - if (SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2), + if (AttemptSelection(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) { return true; } // If partial groups are allowed, relax the requirement of spending OutputGroups (groups // of UTXOs sent to the same address, which are obviously controlled by a single wallet) // in their entirety. - if (SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1, true /* include_partial_groups */), + if (AttemptSelection(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1, true /* include_partial_groups */), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) { return true; } // Try with unsafe inputs if they are allowed. This may spend unconfirmed outputs // received from other wallets. if (coin_control.m_include_unsafe_inputs - && SelectCoinsMinConf(value_to_select, + && AttemptSelection(value_to_select, CoinEligibilityFilter(0 /* conf_mine */, 0 /* conf_theirs */, max_ancestors-1, max_descendants-1, true /* include_partial_groups */), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) { return true; @@ -3047,7 +3047,7 @@ bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAm // Try with unlimited ancestors/descendants. The transaction will still need to meet // mempool ancestor/descendant policy to be accepted to mempool and broadcasted, but // OutputGroups use heuristics that may overestimate ancestor/descendant counts. - if (!fRejectLongChains && SelectCoinsMinConf(value_to_select, + if (!fRejectLongChains && AttemptSelection(value_to_select, CoinEligibilityFilter(0, 1, std::numeric_limits::max(), std::numeric_limits::max(), true /* include_partial_groups */), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) { return true; @@ -3057,7 +3057,7 @@ bool CWallet::SelectCoins(const std::vector& vAvailableCoins, const CAm return false; }(); - // SelectCoinsMinConf clears setCoinsRet, so add the preset inputs from coin_control to the coinset + // AttemptSelection clears setCoinsRet, so add the preset inputs from coin_control to the coinset util::insert(setCoinsRet, setPresetCoins); // add preset inputs to the total value selected diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 7b41966216..47a60f611e 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -858,7 +858,7 @@ private: // ScriptPubKeyMan::GetID. In many cases it will be the hash of an internal structure std::map> m_spk_managers; - bool CreateTransactionInternal(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign, int nExtraPayloadSize); + bool CreateTransactionInternal(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign, int nExtraPayloadSize) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);; /** * Catch wallet up to current chain, scanning new blocks, updating the best @@ -1003,7 +1003,7 @@ public: * param@[out] setCoinsRet Populated with the coins selected if successful. * param@[out] nValueRet Used to return the total value of selected coins. */ - bool SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector coins, std::set& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used, CoinType nCoinType = CoinType::ALL_COINS) const; + bool AttemptSelection(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector coins, std::set& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used, CoinType nCoinType = CoinType::ALL_COINS) const; // Coin selection bool SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::vector& vecTxDSInRet); From 8a58b5e244fcef55bcb96a5254add9abe09a245d Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Fri, 3 Sep 2021 21:05:00 +1200 Subject: [PATCH 6/6] Merge bitcoin/bitcoin#22100: refactor: Clean up new wallet spend, receive files added #21207 b11a195ef450bd138aa03204a5e74fdd3ddced26 refactor: Detach wallet transaction methods (followup for move-only) (Russell Yanofsky) Pull request description: This makes `CWallet` and `CWalletTx` methods in `spend.cpp` and `receive.cpp` files into standalone functions. It's a followup to [#21207 MOVEONLY: CWallet transaction code out of wallet.cpp/.h](https://github.com/bitcoin/bitcoin/pull/21207), which moved code from `wallet.cpp` to new `spend.cpp` and `receive.cpp` files. There are no changes in behavior. This is just making methods into functions and removing circular dependencies created by #21207. There are no comment or documentation changes, either. Removed comments from `transaction.h` are just migrated to `spend.h`, `receive.h`, and `wallet.h`. --- This commit was split off from #21206 so there are a few earlier review comments there ACKs for top commit: achow101: ACK b11a195ef450bd138aa03204a5e74fdd3ddced26 Sjors: utACK b11a195ef450bd138aa03204a5e74fdd3ddced26 meshcollider: light ACK b11a195ef450bd138aa03204a5e74fdd3ddced26 Tree-SHA512: 75ce818d3f03b728b14b12e2d21bd20b7be73978601989cb37ff98254393300d1bb7823281449cd3d9e40756d67d42bd9a46bbdafd2e8baa95aaf2cb1c84549f --- src/bench/coin_selection.cpp | 7 +- src/bench/wallet_balance.cpp | 5 +- src/wallet/interfaces.cpp | 32 ++++---- src/wallet/load.cpp | 1 + src/wallet/rpcwallet.cpp | 62 ++++++++------- src/wallet/test/coinselector_tests.cpp | 9 ++- src/wallet/test/psbt_wallet_tests.cpp | 4 +- src/wallet/test/wallet_tests.cpp | 33 ++++---- src/wallet/wallet.cpp | 56 +++++++------- src/wallet/wallet.h | 101 +++++++------------------ src/wallet/walletdb.cpp | 2 +- test/functional/wallet_basic.py | 2 +- 12 files changed, 138 insertions(+), 176 deletions(-) diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index eeec1f33ee..aca441e705 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -17,7 +18,7 @@ static void addCoin(const CAmount& nValue, const CWallet& wallet, std::vector(&wallet, MakeTransactionRef(std::move(tx)))); + wtxs.push_back(std::make_unique(MakeTransactionRef(std::move(tx)))); } // Simple benchmark for wallet coin selection. Note that it maybe be necessary @@ -45,7 +46,7 @@ static void CoinSelection(benchmark::Bench& bench) // Create coins std::vector coins; for (const auto& wtx : wtxs) { - coins.emplace_back(wtx.get(), 0 /* iIn */, 6 * 24 /* nDepthIn */, true /* spendable */, true /* solvable */, true /* safe */); + coins.emplace_back(wallet, *wtx, 0 /* iIn */, 6 * 24 /* nDepthIn */, true /* spendable */, true /* solvable */, true /* safe */); } const CoinEligibilityFilter filter_standard(1, 6, 0); const CoinSelectionParams coin_selection_params(/* use_bnb= */ true, /* change_output_size= */ 34, @@ -56,7 +57,7 @@ static void CoinSelection(benchmark::Bench& bench) std::set setCoinsRet; CAmount nValueRet; bool bnb_used; - bool success = wallet.AttemptSelection(1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used); + bool success = AttemptSelection(wallet, 1003 * COIN, filter_standard, coins, setCoinsRet, nValueRet, coin_selection_params, bnb_used); assert(success); assert(nValueRet == 1003 * COIN); assert(setCoinsRet.size() == 2); diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index cf70367a30..91c071b087 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -34,11 +35,11 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b } SyncWithValidationInterfaceQueue(); - auto bal = wallet.GetBalance(); // Cache + auto bal = GetBalance(wallet); // Cache bench.minEpochIterations(epoch_iters).run([&] { if (set_dirty) wallet.MarkDirty(); - bal = wallet.GetBalance(); + bal = GetBalance(wallet); if (add_mine) assert(bal.m_mine_trusted > 0); if (add_watchonly) assert(bal.m_watchonly_trusted > 0); }); diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index d1996ecc19..88aebd0337 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -25,7 +25,9 @@ #include #include #include +#include #include +#include #include #include @@ -76,9 +78,9 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) fOutputDenomFound = true; } } - result.credit = wtx.GetCredit(ISMINE_ALL); - result.debit = wtx.GetDebit(ISMINE_ALL); - result.change = wtx.GetChange(); + result.credit = CachedTxGetCredit(wallet, wtx, ISMINE_ALL); + result.debit = CachedTxGetDebit(wallet, wtx, ISMINE_ALL); + result.change = CachedTxGetChange(wallet, wtx); result.time = wtx.GetTxTime(); result.value_map = wtx.mapValue; result.is_coinbase = wtx.IsCoinBase(); @@ -96,15 +98,15 @@ WalletTxStatus MakeWalletTxStatus(const CWallet& wallet, const CWalletTx& wtx) { WalletTxStatus result; result.block_height = wtx.m_confirm.block_height > 0 ? wtx.m_confirm.block_height : std::numeric_limits::max(); - result.blocks_to_maturity = wtx.GetBlocksToMaturity(); - result.depth_in_main_chain = wtx.GetDepthInMainChain(); + result.blocks_to_maturity = wallet.GetTxBlocksToMaturity(wtx); + result.depth_in_main_chain = wallet.GetTxDepthInMainChain(wtx); result.time_received = wtx.nTimeReceived; result.lock_time = wtx.tx->nLockTime; result.is_final = wallet.chain().checkFinalTx(*wtx.tx); - result.is_trusted = wtx.IsTrusted(); + result.is_trusted = CachedTxIsTrusted(wallet, wtx); result.is_abandoned = wtx.isAbandoned(); result.is_coinbase = wtx.IsCoinBase(); - result.is_in_main_chain = wtx.IsInMainChain(); + result.is_in_main_chain = wallet.IsTxInMainChain(wtx); result.is_chainlocked = wtx.IsChainLocked(); result.is_islocked = wtx.IsLockedByInstantSend(); return result; @@ -274,7 +276,7 @@ public: ReserveDestination m_dest(m_wallet.get()); CTransactionRef tx; FeeCalculation fee_calc_out; - if (!m_wallet->CreateTransaction(recipients, tx, fee, change_pos, + if (!CreateTransaction(*m_wallet, recipients, tx, fee, change_pos, fail_reason, coin_control, fee_calc_out, sign)) { return {}; } @@ -378,7 +380,7 @@ public: } WalletBalances getBalances() override { - const auto bal = m_wallet->GetBalance(); + const auto bal = GetBalance(*m_wallet); WalletBalances result; result.balance = bal.m_mine_trusted; result.unconfirmed_balance = bal.m_mine_untrusted_pending; @@ -404,7 +406,7 @@ public: } CAmount getBalance() override { - return m_wallet->GetBalance().m_mine_trusted; + return GetBalance(*m_wallet).m_mine_trusted; } CAmount getAnonymizableBalance(bool fSkipDenominated, bool fSkipUnconfirmed) override { @@ -436,13 +438,13 @@ public: if (coin_control.IsUsingCoinJoin()) { return m_wallet->GetBalance(0, coin_control.m_avoid_address_reuse, false, &coin_control).m_anonymized; } else { - return m_wallet->GetAvailableBalance(&coin_control); + return GetAvailableBalance(*m_wallet, &coin_control); } } isminetype txinIsMine(const CTxIn& txin) override { LOCK(m_wallet->cs_wallet); - return m_wallet->IsMine(txin); + return InputIsMine(*m_wallet, txin); } isminetype txoutIsMine(const CTxOut& txout) override { @@ -457,13 +459,13 @@ public: CAmount getCredit(const CTxOut& txout, isminefilter filter) override { LOCK(m_wallet->cs_wallet); - return m_wallet->GetCredit(txout, filter); + return OutputGetCredit(*m_wallet, txout, filter); } CoinsList listCoins() override { LOCK(m_wallet->cs_wallet); CoinsList result; - for (const auto& entry : m_wallet->ListCoins()) { + 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), @@ -481,7 +483,7 @@ public: result.emplace_back(); auto it = m_wallet->mapWallet.find(output.hash); if (it != m_wallet->mapWallet.end()) { - int depth = it->second.GetDepthInMainChain(); + int depth = m_wallet->GetTxDepthInMainChain(it->second); if (depth >= 0) { result.back() = MakeWalletTxOut(*m_wallet, it->second, output.n, depth); } diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 6f5b661efd..bac0c217d5 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8deb51496f..69d021fb53 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -30,9 +30,11 @@ #include #include #include +#include #include #include #include +#include #include #include #include @@ -59,9 +61,11 @@ bool HaveKey(const SigningProvider& wallet, const CKey& key) return wallet.HaveKey(key.GetPubKey().GetID()) || wallet.HaveKey(key2.GetPubKey().GetID()); } -static void WalletTxToJSON(interfaces::Chain& chain, const CWalletTx& wtx, UniValue& entry) +static void WalletTxToJSON(const JSONRPCRequest& request, interfaces::Chain& chain, const CWalletTx& wtx, UniValue& entry) { int confirms = wtx.GetDepthInMainChain(); + interfaces::Chain& chain = wallet.chain(); + int confirms = wallet.GetTxDepthInMainChain(wtx); bool fLocked = chain.isInstantSendLockedTx(wtx.GetHash()); bool chainlock = false; if (confirms > 0) { @@ -84,12 +88,12 @@ static void WalletTxToJSON(interfaces::Chain& chain, const CWalletTx& wtx, UniVa CHECK_NONFATAL(chain.findBlock(wtx.m_confirm.hashBlock, FoundBlock().time(block_time))); entry.pushKV("blocktime", block_time); } else { - entry.pushKV("trusted", wtx.IsTrusted()); + entry.pushKV("trusted", CachedTxIsTrusted(wallet, wtx)); } uint256 hash = wtx.GetHash(); entry.pushKV("txid", hash.GetHex()); UniValue conflicts(UniValue::VARR); - for (const uint256& conflict : wtx.GetConflicts()) + for (const uint256& conflict : wallet.GetTxConflicts(wtx)) conflicts.push_back(conflict.GetHex()); entry.pushKV("walletconflicts", conflicts); entry.pushKV("time", wtx.GetTxTime()); @@ -300,7 +304,7 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto bilingual_str error; CTransactionRef tx; FeeCalculation fee_calc_out; - const bool fCreated = wallet.CreateTransaction(recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, fee_calc_out, true); + const bool fCreated = CreateTransaction(wallet, recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, fee_calc_out, true); if (!fCreated) { throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original); } @@ -472,8 +476,8 @@ static RPCHelpMan listaddressgroupings() LOCK(pwallet->cs_wallet); UniValue jsonGroupings(UniValue::VARR); - std::map balances = pwallet->GetAddressBalances(); - for (const std::set& grouping : pwallet->GetAddressGroupings()) { + std::map balances = GetAddressBalances(*pwallet); + for (const std::set& grouping : GetAddressGroupings(*pwallet)) { UniValue jsonGrouping(UniValue::VARR); for (const CTxDestination& address : grouping) { @@ -630,7 +634,7 @@ static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool b if (wtx.IsCoinBase() || !wallet.chain().checkFinalTx(*wtx.tx)) { continue; } - if (wtx.GetDepthInMainChain() < min_depth && !(fAddLocked && wtx.IsLockedByInstantSend())) continue; + if (wallet.GetTxDepthInMainChain(wtx) < min_depth && !(fAddLocked && wtx.IsLockedByInstantSend())) continue; for (const CTxOut& txout : wtx.tx->vout) { CTxDestination address; @@ -776,7 +780,7 @@ static RPCHelpMan getbalance() bool include_watchonly = ParseIncludeWatchonly(request.params[3], *pwallet); bool avoid_reuse = GetAvoidReuseFlag(*pwallet, request.params[4]); - const auto bal = pwallet->GetBalance(min_depth, avoid_reuse, fAddLocked); + const auto bal = GetBalance(*pwallet, min_depth, avoid_reuse, fAddLocked); return ValueFromAmount(bal.m_mine_trusted + (include_watchonly ? bal.m_watchonly_trusted : 0)); }, @@ -801,7 +805,7 @@ static RPCHelpMan getunconfirmedbalance() LOCK(pwallet->cs_wallet); - return ValueFromAmount(pwallet->GetBalance().m_mine_untrusted_pending); + return ValueFromAmount(GetBalance(*pwallet).m_mine_untrusted_pending); }, }; } @@ -1027,7 +1031,7 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, bool if (wtx.IsCoinBase() || !wallet.chain().checkFinalTx(*wtx.tx)) continue; - int nDepth = wtx.GetDepthInMainChain(); + int nDepth = wallet.GetTxDepthInMainChain(wtx); if ((nDepth < nMinDepth) && !(fAddLocked && wtx.IsLockedByInstantSend())) continue; @@ -1260,9 +1264,9 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM std::list listReceived; std::list listSent; - wtx.GetAmounts(listReceived, listSent, nFee, filter_ismine); + CachedTxGetAmounts(wallet, wtx, listReceived, listSent, nFee, filter_ismine); - bool involvesWatchonly = wtx.IsFromMe(ISMINE_WATCH_ONLY); + bool involvesWatchonly = CachedTxIsFromMe(wallet, wtx, ISMINE_WATCH_ONLY); // Sent if (!filter_label) @@ -1284,14 +1288,14 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM entry.pushKV("vout", s.vout); entry.pushKV("fee", ValueFromAmount(-nFee)); if (fLong) - WalletTxToJSON(wallet.chain(), wtx, entry); + WalletTxToJSON(wallet, wtx, entry); entry.pushKV("abandoned", wtx.isAbandoned()); ret.push_back(entry); } } // Received - if (listReceived.size() > 0 && ((wtx.GetDepthInMainChain() >= nMinDepth) || wtx.IsLockedByInstantSend())) + if (listReceived.size() > 0 && ((wallet.GetTxDepthInMainChain(wtx) >= nMinDepth) || wtx.IsLockedByInstantSend())) { for (const COutputEntry& r : listReceived) { @@ -1310,9 +1314,9 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM MaybePushAddress(entry, r.destination); if (wtx.IsCoinBase()) { - if (wtx.GetDepthInMainChain() < 1) + if (wallet.GetTxDepthInMainChain(wtx) < 1) entry.pushKV("category", "orphan"); - else if (wtx.IsImmatureCoinBase()) + else if (wallet.IsTxImmatureCoinBase(wtx)) entry.pushKV("category", "immature"); else entry.pushKV("category", "generate"); @@ -1331,7 +1335,7 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM } entry.pushKV("vout", r.vout); if (fLong) - WalletTxToJSON(wallet.chain(), wtx, entry); + WalletTxToJSON(wallet, wtx, entry); ret.push_back(entry); } } @@ -1574,7 +1578,7 @@ static RPCHelpMan listsinceblock() for (const std::pair& pairWtx : wallet.mapWallet) { const CWalletTx& tx = pairWtx.second; - if (depth == -1 || abs(tx.GetDepthInMainChain()) < depth) { + if (depth == -1 || abs(wallet.GetTxDepthInMainChain(tx)) < depth) { ListTransactions(wallet, tx, 0, true, transactions, filter, nullptr /* filter_label */); } } @@ -1694,16 +1698,16 @@ static RPCHelpMan gettransaction() } const CWalletTx& wtx = it->second; - CAmount nCredit = wtx.GetCredit(filter); - CAmount nDebit = wtx.GetDebit(filter); + CAmount nCredit = CachedTxGetCredit(*pwallet, wtx, filter); + CAmount nDebit = CachedTxGetDebit(*pwallet, wtx, filter); CAmount nNet = nCredit - nDebit; - CAmount nFee = (wtx.IsFromMe(filter) ? wtx.tx->GetValueOut() - nDebit : 0); + CAmount nFee = (CachedTxIsFromMe(*pwallet, wtx, filter) ? wtx.tx->GetValueOut() - nDebit : 0); entry.pushKV("amount", ValueFromAmount(nNet - nFee)); - if (wtx.IsFromMe(filter)) + if (CachedTxIsFromMe(*pwallet, wtx, filter)) entry.pushKV("fee", ValueFromAmount(nFee)); - WalletTxToJSON(pwallet->chain(), wtx, entry); + WalletTxToJSON(*pwallet, wtx, entry); UniValue details(UniValue::VARR); ListTransactions(*pwallet, wtx, 0, false, details, filter, nullptr /* filter_label */); @@ -2418,7 +2422,7 @@ static RPCHelpMan getbalances() LOCK(wallet.cs_wallet); - const auto bal = wallet.GetBalance(); + const auto bal = GetBalance(wallet); UniValue balances{UniValue::VOBJ}; { UniValue balances_mine{UniValue::VOBJ}; @@ -2428,7 +2432,7 @@ static RPCHelpMan getbalances() 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 = wallet.GetBalance(0, false); + 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("coinjoin", ValueFromAmount(bal.m_anonymized)); @@ -2511,7 +2515,7 @@ static RPCHelpMan getwalletinfo() bool fHDEnabled = spk_man && spk_man->GetHDChain(hdChainCurrent); UniValue obj(UniValue::VOBJ); - const auto bal = pwallet->GetBalance(); + const auto bal = GetBalance(*pwallet); int64_t kp_oldest = pwallet->GetOldestKeyPoolTime(); obj.pushKV("walletname", pwallet->GetName()); obj.pushKV("walletversion", pwallet->GetVersion()); @@ -3227,7 +3231,7 @@ static RPCHelpMan listunspent() coinControl.m_include_unsafe_inputs = include_unsafe; LOCK(pwallet->cs_wallet); - pwallet->AvailableCoins(vecOutputs, &coinControl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount); + AvailableCoins(*pwallet, vecOutputs, &coinControl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount); } LOCK(pwallet->cs_wallet); @@ -3406,7 +3410,7 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, bilingual_str error; - if (!wallet.FundTransaction(tx, fee_out, change_position, error, lockUnspents, setSubtractFeeFromOutputs, coinControl)) { + if (!FundTransaction(wallet, tx, fee_out, change_position, error, lockUnspents, setSubtractFeeFromOutputs, coinControl)) { throw JSONRPCError(RPC_WALLET_ERROR, error.original); } } @@ -3920,7 +3924,7 @@ RPCHelpMan getaddressinfo() UniValue detail = DescribeWalletAddress(*pwallet, dest); ret.pushKVs(detail); - ret.pushKV("ischange", pwallet->IsChange(scriptPubKey)); + ret.pushKV("ischange", ScriptIsChange(*pwallet, scriptPubKey)); ScriptPubKeyMan* spk_man = pwallet->GetScriptPubKeyMan(scriptPubKey); if (spk_man) { diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 15efed9070..0e60b8e642 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -80,7 +81,7 @@ static void add_coin(std::vector& coins, CWallet& wallet, const CAmount wtx->m_amounts[CWalletTx::DEBIT].Set(ISMINE_SPENDABLE, 1); wtx->m_is_cache_empty = false; } - COutput output(wtx, nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); + COutput output(wallet, *wtx, nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); coins.push_back(output); } @@ -302,7 +303,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) coin_control.fAllowOtherInputs = true; coin_control.Select(COutPoint(coins.at(0).tx->GetHash(), coins.at(0).i)); coin_selection_params_bnb.m_effective_feerate = CFeeRate(0); - BOOST_CHECK(wallet->SelectCoins(coins, 10 * CENT, setCoinsRet, nValueRet, coin_control, coin_selection_params_bnb, bnb_used)); + BOOST_CHECK(SelectCoins(*wallet, coins, 10 * CENT, setCoinsRet, nValueRet, coin_control, coin_selection_params_bnb, bnb_used)); BOOST_CHECK(bnb_used); BOOST_CHECK(coin_selection_params_bnb.use_bnb); } @@ -656,8 +657,8 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) CoinSet out_set; CAmount out_value = 0; bool bnb_used = false; - BOOST_CHECK(wallet->AttemptSelection(target, filter_standard, coins, out_set, out_value, coin_selection_params_bnb, bnb_used) || - wallet->AttemptSelection(target, filter_standard, coins, out_set, out_value, coin_selection_params_knapsack, bnb_used)); + BOOST_CHECK(AttemptSelection(wallet, target, filter_standard, coins, out_set, out_value, coin_selection_params_bnb, bnb_used) || + AttemptSelection(wallet, target, filter_standard, coins, out_set, out_value, coin_selection_params_knapsack, bnb_used)); BOOST_CHECK_GE(out_value, target); } } diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index 9680da5070..5e1efaf867 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -23,12 +23,12 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) CDataStream s_prev_tx1(ParseHex("0200000000010158e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd7501000000171600145f275f436b09a8cc9a2eb2a2f528485c68a56323feffffff02d8231f1b0100000017a914aed962d6654f9a2b36608eb9d64d2b260db4f1118700c2eb0b0000000017a914b7f5faf40e3d40a5a459b1db3535f2b72fa921e88702483045022100a22edcc6e5bc511af4cc4ae0de0fcd75c7e04d8c1c3a8aa9d820ed4b967384ec02200642963597b9b1bc22c75e9f3e117284a962188bf5e8a74c895089046a20ad770121035509a48eb623e10aace8bfd0212fdb8a8e5af3c94b0b133b95e114cab89e4f7965000000"), SER_NETWORK, PROTOCOL_VERSION); CTransactionRef prev_tx1; s_prev_tx1 >> prev_tx1; - m_wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(prev_tx1->GetHash()), std::forward_as_tuple(&m_wallet, prev_tx1)); + m_wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(prev_tx1->GetHash()), std::forward_as_tuple(prev_tx1)); CDataStream s_prev_tx2(ParseHex("0200000001aad73931018bd25f84ae400b68848be09db706eac2ac18298babee71ab656f8b0000000048473044022058f6fc7c6a33e1b31548d481c826c015bd30135aad42cd67790dab66d2ad243b02204a1ced2604c6735b6393e5b41691dd78b00f0c5942fb9f751856faa938157dba01feffffff0280f0fa020000000017a9140fb9463421696b82c833af241c78c17ddbde493487d0f20a270100000017a91429ca74f8a08f81999428185c97b5d852e4063f618765000000"), SER_NETWORK, PROTOCOL_VERSION); CTransactionRef prev_tx2; s_prev_tx2 >> prev_tx2; - m_wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(prev_tx2->GetHash()), std::forward_as_tuple(&m_wallet, prev_tx2)); + m_wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(prev_tx2->GetHash()), std::forward_as_tuple(prev_tx2)); // Add scripts CScript rs1; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index e7aa85c6d4..c771345337 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -25,8 +25,9 @@ #include #include #include +#include +#include #include - #include #include @@ -119,7 +120,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) BOOST_CHECK(result.last_failed_block.IsNull()); BOOST_CHECK(result.last_scanned_block.IsNull()); BOOST_CHECK(!result.last_scanned_height); - BOOST_CHECK_EQUAL(wallet.GetBalance().m_mine_immature, 0); + BOOST_CHECK_EQUAL(GetBalance(wallet).m_mine_immature, 0); } // Verify ScanForWalletTransactions picks up transactions in both the old @@ -138,7 +139,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) BOOST_CHECK(result.last_failed_block.IsNull()); BOOST_CHECK_EQUAL(result.last_scanned_block, newTip->GetBlockHash()); BOOST_CHECK_EQUAL(*result.last_scanned_height, newTip->nHeight); - BOOST_CHECK_EQUAL(wallet.GetBalance().m_mine_immature, 1000 * COIN); + BOOST_CHECK_EQUAL(GetBalance(wallet).m_mine_immature, 1000 * COIN); } // Prune the older block file. @@ -166,7 +167,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) BOOST_CHECK_EQUAL(result.last_failed_block, oldTip->GetBlockHash()); BOOST_CHECK_EQUAL(result.last_scanned_block, newTip->GetBlockHash()); BOOST_CHECK_EQUAL(*result.last_scanned_height, newTip->nHeight); - BOOST_CHECK_EQUAL(wallet.GetBalance().m_mine_immature, 500 * COIN); + BOOST_CHECK_EQUAL(GetBalance(wallet).m_mine_immature, 500 * COIN); } // Prune the remaining block file. @@ -192,7 +193,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) BOOST_CHECK_EQUAL(result.last_failed_block, newTip->GetBlockHash()); BOOST_CHECK(result.last_scanned_block.IsNull()); BOOST_CHECK(!result.last_scanned_height); - BOOST_CHECK_EQUAL(wallet.GetBalance().m_mine_immature, 0); + BOOST_CHECK_EQUAL(GetBalance(wallet).m_mine_immature, 0); } } @@ -408,7 +409,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) { CWallet wallet(m_node.chain.get(), m_node.coinjoin_loader.get(), "", CreateDummyWalletDatabase()); auto spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan(); - CWalletTx wtx(&wallet, m_coinbase_txns.back()); + CWalletTx wtx(m_coinbase_txns.back()); LOCK2(wallet.cs_wallet, spk_man->cs_KeyStore); wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); @@ -418,13 +419,13 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) // Call GetImmatureCredit() once before adding the key to the wallet to // cache the current immature credit amount, which is 0. - BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(), 0); + BOOST_CHECK_EQUAL(CachedTxGetImmatureCredit(wallet, wtx), 0); // Invalidate the cached value, add the key, and make sure a new immature // credit amount is calculated. wtx.MarkDirty(); BOOST_CHECK(spk_man->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey())); - BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(), 500*COIN); + BOOST_CHECK_EQUAL(CachedTxGetImmatureCredit(wallet, wtx), 500*COIN); } static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime) @@ -609,7 +610,7 @@ public: bilingual_str error; CCoinControl dummy; FeeCalculation fee_calc_out; - BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, fee, changePos, error, dummy, fee_calc_out)); + BOOST_CHECK(CreateTransaction(*wallet, {recipient}, tx, fee, changePos, error, dummy, fee_calc_out)); wallet->CommitTransaction(tx, {}, {}); CMutableTransaction blocktx; { @@ -630,7 +631,7 @@ public: std::unique_ptr wallet; }; -BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) +BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup) { std::string coinbaseAddress = coinbaseKey.GetPubKey().GetID().ToString(); @@ -639,14 +640,14 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) std::map> list; { LOCK(wallet->cs_wallet); - list = wallet->ListCoins(); + list = ListCoins(*wallet); } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(std::get(list.begin()->first).ToString(), coinbaseAddress); BOOST_CHECK_EQUAL(list.begin()->second.size(), 1U); // Check initial balance from one mature coinbase transaction. - BOOST_CHECK_EQUAL(500 * COIN, wallet->GetAvailableBalance()); + BOOST_CHECK_EQUAL(500 * COIN, GetAvailableBalance(*wallet)); // Add a transaction creating a change address, and confirm ListCoins still // returns the coin associated with the change address underneath the @@ -655,7 +656,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, false /* subtract fee */}); { LOCK(wallet->cs_wallet); - list = wallet->ListCoins(); + list = ListCoins(*wallet); } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(std::get(list.begin()->first).ToString(), coinbaseAddress); @@ -665,7 +666,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) { LOCK(wallet->cs_wallet); std::vector available; - wallet->AvailableCoins(available); + AvailableCoins(*wallet, available); BOOST_CHECK_EQUAL(available.size(), 2U); } for (const auto& group : list) { @@ -677,14 +678,14 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) { LOCK(wallet->cs_wallet); std::vector available; - wallet->AvailableCoins(available); + AvailableCoins(*wallet, available); BOOST_CHECK_EQUAL(available.size(), 0U); } // Confirm ListCoins still returns same result as before, despite coins // being locked. { LOCK(wallet->cs_wallet); - list = wallet->ListCoins(); + list = ListCoins(*wallet); } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(std::get(list.begin()->first).ToString(), coinbaseAddress); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b658a9ca70..7d43bc7c27 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -617,7 +617,7 @@ bool CWallet::IsSpent(const uint256& hash, unsigned int n) const const uint256& wtxid = it->second; std::map::const_iterator mit = mapWallet.find(wtxid); if (mit != mapWallet.end()) { - int depth = mit->second.GetDepthInMainChain(); + int depth = GetTxDepthInMainChain(mit->second); if (depth > 0 || (depth == 0 && !mit->second.isAbandoned())) return true; // Spent } @@ -905,7 +905,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio } // Inserts only if not already there, returns tx inserted or tx found - auto ret = mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(hash), std::forward_as_tuple(this, tx)); + auto ret = mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(hash), std::forward_as_tuple(tx)); CWalletTx& wtx = (*ret.first).second; bool fInsertedNew = ret.second; bool fUpdated = update_wtx && update_wtx(wtx, fInsertedNew); @@ -1014,7 +1014,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio bool CWallet::LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) { - const auto& ins = mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(hash), std::forward_as_tuple(this, nullptr)); + const auto& ins = mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(hash), std::forward_as_tuple(nullptr)); CWalletTx& wtx = ins.first->second; if (!fill_wtx(wtx, ins.second)) { return false; @@ -1111,7 +1111,7 @@ bool CWallet::TransactionCanBeAbandoned(const uint256& hashTx) const { LOCK(cs_wallet); const CWalletTx* wtx = GetWalletTx(hashTx); - return wtx && !wtx->isAbandoned() && wtx->GetDepthInMainChain() == 0 && !wtx->InMempool(); + return wtx && !wtx->isAbandoned() && GetTxDepthInMainChain(*wtx) == 0 && !wtx->InMempool(); } bool CWallet::TransactionCanBeResent(const uint256& hashTx) const @@ -1144,7 +1144,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) auto it = mapWallet.find(hashTx); assert(it != mapWallet.end()); const CWalletTx& origtx = it->second; - if (origtx.GetDepthInMainChain() != 0 || origtx.InMempool() || origtx.IsLockedByInstantSend()) { + if (GetTxDepthInMainChain(origtx) != 0 || origtx.InMempool() || origtx.IsLockedByInstantSend()) { return false; } @@ -1157,7 +1157,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) auto it = mapWallet.find(now); assert(it != mapWallet.end()); CWalletTx& wtx = it->second; - int currentconfirm = wtx.GetDepthInMainChain(); + int currentconfirm = GetTxDepthInMainChain(wtx); // If the orig tx was not in block, none of its spends can be assert(currentconfirm <= 0); // if (currentconfirm < 0) {Tx and spends are already conflicted, no need to abandon} @@ -1227,7 +1227,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c auto it = mapWallet.find(now); assert(it != mapWallet.end()); CWalletTx& wtx = it->second; - int currentconfirm = wtx.GetDepthInMainChain(); + int currentconfirm = GetTxDepthInMainChain(wtx); if (conflictconfirms < currentconfirm) { // Block is 'more conflicted' than current confirm; update. // Mark transaction as conflicted with this block. @@ -2140,7 +2140,7 @@ void CWallet::ReacceptWalletTransactions() CWalletTx& wtx = item.second; assert(wtx.GetHash() == wtxid); - int nDepth = wtx.GetDepthInMainChain(); + int nDepth = GetTxDepthInMainChain(wtx); if (!wtx.IsCoinBase() && (nDepth == 0 && !wtx.IsLockedByInstantSend() && !wtx.isAbandoned())) { mapSorted.insert(std::make_pair(wtx.nOrderPos, &wtx)); @@ -2151,7 +2151,7 @@ void CWallet::ReacceptWalletTransactions() for (const std::pair& item : mapSorted) { CWalletTx& wtx = *(item.second); bilingual_str unused_err_string; - wtx.SubmitMemoryPoolAndRelay(unused_err_string, false); + SubmitTxMemoryPoolAndRelay(wtx, unused_err_string, false); } } @@ -2176,7 +2176,7 @@ bool CWalletTx::SubmitMemoryPoolAndRelay(bilingual_str& err_string, bool relay) if (!CanBeResent()) return false; // Submit transaction to mempool for relay - pwallet->WalletLogPrintf("Submitting wtx %s to mempool for relay\n", GetHash().ToString()); + WalletLogPrintf("Submitting wtx %s to mempool for relay\n", wtx.GetHash().ToString()); // We must set fInMempool here - while it will be re-set to true by the // entered-mempool callback, if we did not there would be a race where a // user could call sendmoney in a loop and hit spurious out of funds errors @@ -2186,19 +2186,18 @@ bool CWalletTx::SubmitMemoryPoolAndRelay(bilingual_str& err_string, bool relay) // Irrespective of the failure reason, un-marking fInMempool // out-of-order is incorrect - it should be unmarked when // TransactionRemovedFromMempool fires. - bool ret = pwallet->chain().broadcastTransaction(tx, pwallet->m_default_max_tx_fee, relay, err_string); - fInMempool |= ret; + bool ret = chain().broadcastTransaction(wtx.tx, m_default_max_tx_fee, relay, err_string); + wtx.fInMempool |= ret; return ret; } -std::set CWalletTx::GetConflicts() const +std::set CWallet::GetTxConflicts(const CWalletTx& wtx) const { std::set result; - if (pwallet != nullptr) { AssertLockHeld(pwallet->cs_wallet); - uint256 myHash = GetHash(); - result = pwallet->GetConflicts(myHash); + uint256 myHash = wtx.GetHash(); + result = GetConflicts(myHash); result.erase(myHash); } return result; @@ -2484,11 +2483,11 @@ void CWallet::ResendWalletTransactions() for (std::pair& item : mapWallet) { CWalletTx& wtx = item.second; // Attempt to rebroadcast all txes more than 5 minutes older than - // the last block. SubmitMemoryPoolAndRelay() will not rebroadcast + // the last block. SubmitTxMemoryPoolAndRelay() will not rebroadcast // any confirmed or conflicting txs. if (wtx.nTimeReceived > m_best_block_time - 5 * 60) continue; bilingual_str unused_err_string; - if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true)) ++submitted_tx_count; + if (SubmitTxMemoryPoolAndRelay(wtx, unused_err_string, true)) ++submitted_tx_count; } } // cs_wallet @@ -3994,7 +3993,7 @@ void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve } bilingual_str err_string; - if (!wtx.SubmitMemoryPoolAndRelay(err_string, true)) { + if (!SubmitTxMemoryPoolAndRelay(wtx, err_string, true)) { WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", err_string.original); // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure. } @@ -5450,13 +5449,12 @@ CKeyPool::CKeyPool(const CPubKey& vchPubKeyIn, bool fInternalIn) fInternal = fInternalIn; } -int CWalletTx::GetDepthInMainChain() const +int CWallet::GetTxDepthInMainChain(const CWalletTx& wtx) const { - assert(pwallet != nullptr); - AssertLockHeld(pwallet->cs_wallet); - if (isUnconfirmed() || isAbandoned()) return 0; + AssertLockHeld(cs_wallet); + if (wtx.isUnconfirmed() || wtx.isAbandoned()) return 0; - return (pwallet->GetLastBlockHeight() - m_confirm.block_height + 1) * (isConflicted() ? -1 : 1); + return (GetLastBlockHeight() - wtx.m_confirm.block_height + 1) * (wtx.isConflicted() ? -1 : 1); } bool CWalletTx::IsLockedByInstantSend() const @@ -5483,19 +5481,19 @@ bool CWalletTx::IsChainLocked() const return fIsChainlocked; } -int CWalletTx::GetBlocksToMaturity() const +int CWallet::GetTxBlocksToMaturity(const CWalletTx& wtx) const { - if (!IsCoinBase()) + if (!wtx.IsCoinBase()) return 0; - int chain_depth = GetDepthInMainChain(); + int chain_depth = GetTxDepthInMainChain(wtx); assert(chain_depth >= 0); // coinbase tx should not be conflicted return std::max(0, (COINBASE_MATURITY+1) - chain_depth); } -bool CWalletTx::IsImmatureCoinBase() const +bool CWallet::IsTxImmatureCoinBase(const CWalletTx& wtx) const { // note GetBlocksToMaturity is 0 for non-coinbase tx - return GetBlocksToMaturity() > 0; + return GetTxBlocksToMaturity(wtx) > 0; } std::vector CWallet::GroupOutputs(const std::vector& outputs, bool separate_coins, const CFeeRate& effective_feerate, const CFeeRate& long_term_feerate, const CoinEligibilityFilter& filter, bool positive_only) const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 47a60f611e..02f73bebf5 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -541,7 +541,6 @@ public: bool IsEquivalentTo(const CWalletTx& tx) const; bool InMempool() const; - bool IsTrusted() const; int64_t GetTxTime() const; @@ -858,8 +857,6 @@ private: // ScriptPubKeyMan::GetID. In many cases it will be the hash of an internal structure std::map> m_spk_managers; - bool CreateTransactionInternal(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign, int nExtraPayloadSize) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);; - /** * Catch wallet up to current chain, scanning new blocks, updating the best * block locator and m_last_block_processed, and registering for @@ -880,17 +877,6 @@ public: return *m_database; } - /** - * Select a set of coins such that nValueRet >= nTargetValue and at least - * all coins from coin_control are selected; never select unconfirmed coins if they are not ours - * param@[out] setCoinsRet Populated with inputs including pre-selected inputs from - * coin_control and Coin Selection if successful. - * param@[out] nValueRet Total value of selected coins including pre-selected ones - * from coin_control and Coin Selection if successful. - */ - bool SelectCoins(const std::vector& vAvailableCoins, const CAmount& nTargetValue, std::set& setCoinsRet, CAmount& nValueRet, - const CCoinControl& coin_control, CoinSelectionParams& coin_selection_params, bool& bnb_used) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - /** Get a name for this wallet for logging/debugging purposes. */ const std::string& GetName() const { return m_name; } @@ -972,25 +958,37 @@ public: bool coinjoin_available() { return m_coinjoin_loader != nullptr; } const CWalletTx* GetWalletTx(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - bool IsTrusted(const CWalletTx& wtx, std::set& trusted_parents) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - //! check whether we support the named feature - bool CanSupportFeature(enum WalletFeature wf) const override EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); return IsFeatureSupported(nWalletVersion, wf); } + // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct + // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation + // "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid having to + // resolve the issue of member access into incomplete type CWallet. Note + // that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)" + // in place. + std::set GetTxConflicts(const CWalletTx& wtx) const NO_THREAD_SAFETY_ANALYSIS; /** - * populate vCoins with vector of available COutputs. + * Return depth of transaction in blockchain: + * <0 : conflicts with a transaction this deep in the blockchain + * 0 : in memory pool, waiting to be included in a block + * >=1 : this many blocks deep in the main chain */ - void AvailableCoins(std::vector& vCoins, const CCoinControl* coinControl = nullptr, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct + // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation + // "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid having to + // resolve the issue of member access into incomplete type CWallet. Note + // that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)" + // in place. + int GetTxDepthInMainChain(const CWalletTx& wtx) const NO_THREAD_SAFETY_ANALYSIS; + bool IsTxInMainChain(const CWalletTx& wtx) const { return GetTxDepthInMainChain(wtx) > 0; } /** - * Return list of available coins and locked coins grouped by non-change output address. + * @return number of blocks to maturity for this transaction: + * 0 : is not a coinbase transaction, or is a mature coinbase transaction + * >0 : is a coinbase transaction which matures in this many blocks */ - std::map> ListCoins() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - - /** - * Find non-change parent output. - */ - const CTxOut& FindNonChangeParentOutput(const CTransaction& tx, int output) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + int GetTxBlocksToMaturity(const CWalletTx& wtx) const; + bool IsTxImmatureCoinBase(const CWalletTx& wtx) const; /** * Shuffle and select coins until nTargetValue is reached while avoiding @@ -1030,8 +1028,6 @@ public: bool IsSpentKey(const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned int n, bool used, std::set& tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - std::vector GroupOutputs(const std::vector& outputs, bool separate_coins, const CFeeRate& effective_feerate, const CFeeRate& long_term_feerate, const CoinEligibilityFilter& filter, bool positive_only) const; - bool IsLockedCoin(uint256 hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void LockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void UnlockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -1115,31 +1111,12 @@ public: void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override; void ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void ResendWalletTransactions(); - 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_watchonly_trusted{0}; - CAmount m_watchonly_untrusted_pending{0}; - CAmount m_watchonly_immature{0}; - CAmount m_anonymized{0}; - CAmount m_denominated_trusted{0}; - CAmount m_denominated_untrusted_pending{0}; - }; - Balance GetBalance(const int min_depth = 0, const bool avoid_reuse = true, const bool fAddLocked = false, const CCoinControl* coinControl = nullptr) const; - CAmount GetAnonymizableBalance(bool fSkipDenominated = false, bool fSkipUnconfirmed = true) const; float GetAverageAnonymizedRounds() const; CAmount GetNormalizedAnonymizedBalance() const; bool GetBudgetSystemCollateralTX(CTransactionRef& tx, uint256 hash, CAmount amount, const COutPoint& outpoint=COutPoint()/*defaults null*/); - CAmount GetAvailableBalance(const CCoinControl* coinControl = nullptr) const; - /** - * Insert additional inputs into the transaction by - * calling CreateTransaction(); - */ - bool FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, bool lockUnspents, const std::set& setSubtractFeeFromOutputs, CCoinControl); /** Fetch the inputs and sign with SIGHASH_ALL. */ bool SignTransaction(CMutableTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** Sign the tx given the input coins and sighash. */ @@ -1171,12 +1148,6 @@ public: bool bip32derivs = true, size_t* n_signed = nullptr) const; - /** - * Create a new transaction paying the recipients with a set of coins - * selected by SelectCoins(); Also create the change output, when needed - * @note passing nChangePosInOut as -1 will result in setting a random position - */ - bool CreateTransaction(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign = true, int nExtraPayloadSize = 0); /** * Submit the transaction to the node's mempool and then relay to peers. * Should be called after CreateTransaction unless you want to abort @@ -1188,6 +1159,9 @@ public: */ void CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm); + /** Pass this transaction to node for mempool insertion and relay to peers if flag set to true */ + bool SubmitTxMemoryPoolAndRelay(const CWalletTx& wtx, std::string& err_string, bool relay) const; + bool DummySignTx(CMutableTransaction &txNew, const std::set &txouts, bool use_max_sig = false) const { std::vector v_txouts(txouts.size()); @@ -1230,9 +1204,6 @@ public: int64_t GetOldestKeyPoolTime() const; - std::set> GetAddressGroupings() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - std::map GetAddressBalances() const; - std::set GetLabelAddresses(const std::string& label) const; /** @@ -1246,25 +1217,16 @@ public: isminetype IsMine(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); isminetype IsMine(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - isminetype IsMine(const CTxIn& txin) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Returns amount of debit if the input matches the * filter, otherwise returns 0 */ CAmount GetDebit(const CTxIn& txin, const isminefilter& filter) const; isminetype IsMine(const CTxOut& txout) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - CAmount GetCredit(const CTxOut& txout, const isminefilter& filter) const; - bool IsChange(const CTxOut& txout) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - bool IsChange(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - CAmount GetChange(const CTxOut& txout) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsMine(const CTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** should probably be renamed to IsRelevantToMe */ bool IsFromMe(const CTransaction& tx) const; CAmount GetDebit(const CTransaction& tx, const isminefilter& filter) const; - /** Returns whether all of the inputs match the filter */ - bool IsAllFromMe(const CTransaction& tx, const isminefilter& filter) const; - CAmount GetCredit(const CTransaction& tx, const isminefilter& filter) const; - CAmount GetChange(const CTransaction& tx) const; void chainStateFlushed(const CBlockLocator& loc) override; DBErrors LoadWallet(); @@ -1564,15 +1526,6 @@ public: } } -}; - -/** Calculate the size of the transaction assuming all signatures are max size -* Use DummySignatureCreator, which inserts 71 byte signatures everywhere. -* NOTE: this requires that all inputs must be in mapWallet (eg the tx should -* be IsAllFromMe). */ -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig = false) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet); -int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector& txouts, bool use_max_sig = false); - //! Add wallet name to persistent configuration so it will be loaded on startup. bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 5992579d85..110717c47e 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -953,7 +953,7 @@ DBErrors WalletBatch::FindWalletTx(std::vector& vTxHash, std::list> hash; vTxHash.push_back(hash); - vWtx.emplace_back(nullptr /* wallet */, nullptr /* tx */); + vWtx.emplace_back(nullptr /* tx */); ssValue >> vWtx.back(); } } diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 647073cd80..284ac4fec1 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -589,7 +589,7 @@ class WalletTest(BitcoinTestFramework): total_txs = len(self.nodes[0].listtransactions("*", 99999)) # Try with walletrejectlongchains - # Double chain limit but require combining inputs, so we pass SelectCoinsMinConf + # Double chain limit but require combining inputs, so we pass AttemptSelection self.stop_node(0) extra_args = ["-walletrejectlongchains", "-limitancestorcount=" + str(2 * chainlimit)] self.start_node(0, extra_args=extra_args)