diff options
48 files changed, 469 insertions, 230 deletions
diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index 9bfe555243..53fe6d961f 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -93,7 +93,7 @@ elif [ "$CI_USE_APT_INSTALL" != "no" ]; then CI_EXEC_ROOT add-apt-repository ppa:hadret/bpfcc fi if [[ -n "${APPEND_APT_SOURCES_LIST}" ]]; then - CI_EXEC_ROOT echo "${APPEND_APT_SOURCES_LIST}" >> /etc/apt/sources.list + CI_EXEC_ROOT echo "${APPEND_APT_SOURCES_LIST}" \>\> /etc/apt/sources.list fi ${CI_RETRY_EXE} CI_EXEC_ROOT apt-get update ${CI_RETRY_EXE} CI_EXEC_ROOT apt-get install --no-install-recommends --no-upgrade -y "$PACKAGES" "$CI_BASE_PACKAGES" diff --git a/configure.ac b/configure.ac index c982ee8b8c..2638742232 100644 --- a/configure.ac +++ b/configure.ac @@ -448,7 +448,7 @@ if test "$CXXFLAGS_overridden" = "no"; then AX_CHECK_COMPILE_FLAG([-Wvla], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wvla"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Wshadow-field], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wshadow-field"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Wthread-safety], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wthread-safety"], [], [$CXXFLAG_WERROR]) - AX_CHECK_COMPILE_FLAG([-Wloop-analysis], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wrange-loop-analysis"], [], [$CXXFLAG_WERROR]) + AX_CHECK_COMPILE_FLAG([-Wloop-analysis], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wloop-analysis"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Wredundant-decls], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wredundant-decls"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Wunused-member-function], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunused-member-function"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Wdate-time], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdate-time"], [], [$CXXFLAG_WERROR]) diff --git a/depends/packages/sqlite.mk b/depends/packages/sqlite.mk index 820d724214..c8f3555ab2 100644 --- a/depends/packages/sqlite.mk +++ b/depends/packages/sqlite.mk @@ -6,10 +6,15 @@ $(package)_sha256_hash=5af07de982ba658fd91a03170c945f99c971f6955bc79df3266544373 define $(package)_set_vars $(package)_config_opts=--disable-shared --disable-readline --disable-dynamic-extensions --enable-option-checking +$(package)_config_opts+= --disable-rtree --disable-fts4 --disable-fts5 $(package)_config_opts_linux=--with-pic $(package)_config_opts_freebsd=--with-pic $(package)_config_opts_netbsd=--with-pic $(package)_config_opts_openbsd=--with-pic +$(package)_config_opts_debug=--enable-debug +$(package)_cflags=-DSQLITE_DQS=0 -DSQLITE_DEFAULT_MEMSTATUS=0 -DSQLITE_OMIT_DEPRECATED +$(package)_cflags+=-DSQLITE_OMIT_SHARED_CACHE -DSQLITE_OMIT_JSON -DSQLITE_LIKE_DOESNT_MATCH_BLOBS +$(package)_cflags+=-DSQLITE_OMIT_DECLTYPE -DSQLITE_OMIT_PROGRESS_CALLBACK -DSQLITE_OMIT_AUTOINIT endef define $(package)_preprocess_cmds diff --git a/depends/packages/systemtap.mk b/depends/packages/systemtap.mk index a57f1b6d36..ad74323d98 100644 --- a/depends/packages/systemtap.mk +++ b/depends/packages/systemtap.mk @@ -1,6 +1,6 @@ package=systemtap $(package)_version=4.7 -$(package)_download_path=https://sourceware.org/systemtap/ftp/releases/ +$(package)_download_path=https://sourceware.org/ftp/systemtap/releases/ $(package)_file_name=$(package)-$($(package)_version).tar.gz $(package)_sha256_hash=43a0a3db91aa4d41e28015b39a65e62059551f3cc7377ebf3a3a5ca7339e7b1f $(package)_patches=remove_SDT_ASM_SECTION_AUTOGROUP_SUPPORT_check.patch diff --git a/doc/reduce-memory.md b/doc/reduce-memory.md index 296b172bde..097cc9f001 100644 --- a/doc/reduce-memory.md +++ b/doc/reduce-memory.md @@ -16,11 +16,11 @@ The size of some in-memory caches can be reduced. As caches trade off memory usa - The minimum value for `-maxmempool` is 5. - A lower maximum mempool size means that transactions will be evicted sooner. This will affect any uses of `bitcoind` that process unconfirmed transactions. -- To completely disable mempool functionality there is the option `-blocksonly`. This will make the client opt out of receiving (and thus relaying) transactions completely, except as part of blocks. +- Since `0.14.0`, unused memory allocated to the mempool (default: 300MB) is shared with the UTXO cache, so when trying to reduce memory usage you should limit the mempool, with the `-maxmempool` command line argument. - - Do not use this when using the client to broadcast transactions as any transaction sent will stick out like a sore thumb, affecting privacy. When used with the wallet it should be combined with `-walletbroadcast=0` and `-spendzeroconfchange=0`. Another mechanism for broadcasting outgoing transactions (if any) should be used. +- To disable most of the mempool functionality there is the `-blocksonly` option. This will reduce the default memory usage to 5MB and make the client opt out of receiving (and thus relaying) transactions, except from whitelisted peers and as part of blocks. -- Since `0.14.0`, unused memory allocated to the mempool (default: 300MB) is shared with the UTXO cache, so when trying to reduce memory usage you should limit the mempool, with the `-maxmempool` command line argument. + - Do not use this when using the client to broadcast transactions as any transaction sent will stick out like a sore thumb, affecting privacy. When used with the wallet it should be combined with `-walletbroadcast=0` and `-spendzeroconfchange=0`. Another mechanism for broadcasting outgoing transactions (if any) should be used. ## Number of peers diff --git a/doc/release-notes-26471.md b/doc/release-notes-26471.md new file mode 100644 index 0000000000..2cb74804ca --- /dev/null +++ b/doc/release-notes-26471.md @@ -0,0 +1,13 @@ +Updated settings +---------------- + +- Setting `-blocksonly` will now reduce the maximum mempool memory + to 5MB (users may still use `-maxmempool` to override). Previously, + the default 300MB would be used, leading to unexpected memory usage + for users running with `-blocksonly` expecting it to eliminate + mempool memory usage. + + As unused mempool memory is shared with dbcache, this also reduces + the dbcache size for users running with `-blocksonly`, potentially + impacting performance. + diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 1a29e9a47a..4d867fdc2f 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -293,6 +293,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/parse_numbers.cpp \ test/fuzz/parse_script.cpp \ test/fuzz/parse_univalue.cpp \ + test/fuzz/partially_downloaded_block.cpp \ test/fuzz/policy_estimator.cpp \ test/fuzz/policy_estimator_io.cpp \ test/fuzz/pow.cpp \ diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index bcb86d75cc..0d5575e5d5 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -52,7 +52,8 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c if (cmpctblock.shorttxids.size() + cmpctblock.prefilledtxn.size() > MAX_BLOCK_WEIGHT / MIN_SERIALIZABLE_TRANSACTION_WEIGHT) return READ_STATUS_INVALID; - assert(header.IsNull() && txn_available.empty()); + if (!header.IsNull() || !txn_available.empty()) return READ_STATUS_INVALID; + header = cmpctblock.header; txn_available.resize(cmpctblock.BlockTxCount()); @@ -167,14 +168,18 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c return READ_STATUS_OK; } -bool PartiallyDownloadedBlock::IsTxAvailable(size_t index) const { - assert(!header.IsNull()); +bool PartiallyDownloadedBlock::IsTxAvailable(size_t index) const +{ + if (header.IsNull()) return false; + assert(index < txn_available.size()); return txn_available[index] != nullptr; } -ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector<CTransactionRef>& vtx_missing) { - assert(!header.IsNull()); +ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector<CTransactionRef>& vtx_missing) +{ + if (header.IsNull()) return READ_STATUS_INVALID; + uint256 hash = header.GetHash(); block = header; block.vtx.resize(txn_available.size()); @@ -197,7 +202,8 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector< return READ_STATUS_INVALID; BlockValidationState state; - if (!CheckBlock(block, state, Params().GetConsensus())) { + CheckBlockFn check_block = m_check_block_mock ? m_check_block_mock : CheckBlock; + if (!check_block(block, state, Params().GetConsensus(), /*fCheckPoW=*/true, /*fCheckMerkleRoot=*/true)) { // TODO: We really want to just check merkle tree manually here, // but that is expensive, and CheckBlock caches a block's // "checked-status" (in the CBlock?). CBlock should be able to diff --git a/src/blockencodings.h b/src/blockencodings.h index e60c1e3db4..7207ff1ae2 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -7,8 +7,13 @@ #include <primitives/block.h> +#include <functional> class CTxMemPool; +class BlockValidationState; +namespace Consensus { +struct Params; +}; // Transaction compression schemes for compact block relay can be introduced by writing // an actual formatter here. @@ -129,6 +134,11 @@ protected: const CTxMemPool* pool; public: CBlockHeader header; + + // Can be overriden for testing + using CheckBlockFn = std::function<bool(const CBlock&, BlockValidationState&, const Consensus::Params&, bool, bool)>; + CheckBlockFn m_check_block_mock{nullptr}; + explicit PartiallyDownloadedBlock(CTxMemPool* poolIn) : pool(poolIn) {} // extra_txn is a list of extra transactions to look at, in <witness hash, reference> form @@ -35,7 +35,7 @@ public: // Allow path objects arguments for compatibility. path(std::filesystem::path path) : std::filesystem::path::path(std::move(path)) {} path& operator=(std::filesystem::path path) { std::filesystem::path::operator=(std::move(path)); return *this; } - path& operator/=(std::filesystem::path path) { std::filesystem::path::operator/=(std::move(path)); return *this; } + path& operator/=(std::filesystem::path path) { std::filesystem::path::operator/=(path); return *this; } // Allow literal string arguments, which are safe as long as the literals are ASCII. path(const char* c) : std::filesystem::path(c) {} diff --git a/src/init.cpp b/src/init.cpp index 685583bcbd..75eb114163 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -603,7 +603,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-rest", strprintf("Accept public REST requests (default: %u)", DEFAULT_REST_ENABLE), ArgsManager::ALLOW_ANY, OptionsCategory::RPC); argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); argsman.AddArg("-rpcauth=<userpw>", "Username and HMAC-SHA-256 hashed password for JSON-RPC connections. The field <userpw> comes in the format: <USERNAME>:<SALT>$<HASH>. A canonical python script is included in share/rpcauth. The client then connects normally using the rpcuser=<USERNAME>/rpcpassword=<PASSWORD> pair of arguments. This option can be specified multiple times", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::RPC); - argsman.AddArg("-rpcbind=<addr>[:port]", "Bind to given address to listen for JSON-RPC connections. Do not expose the RPC server to untrusted networks such as the public internet! This option is ignored unless -rpcallowip is also passed. Port is optional and overrides -rpcport. Use [host]:port notation for IPv6. This option can be specified multiple times (default: 127.0.0.1 and ::1 i.e., localhost)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY | ArgsManager::SENSITIVE, OptionsCategory::RPC); + argsman.AddArg("-rpcbind=<addr>[:port]", "Bind to given address to listen for JSON-RPC connections. Do not expose the RPC server to untrusted networks such as the public internet! This option is ignored unless -rpcallowip is also passed. Port is optional and overrides -rpcport. Use [host]:port notation for IPv6. This option can be specified multiple times (default: 127.0.0.1 and ::1 i.e., localhost)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::RPC); argsman.AddArg("-rpcdoccheck", strprintf("Throw a non-fatal error at runtime if the documentation for an RPC is incorrect (default: %u)", DEFAULT_RPC_DOC_CHECK), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC); argsman.AddArg("-rpccookiefile=<loc>", "Location of the auth cookie. Relative paths will be prefixed by a net-specific datadir location. (default: data dir)", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); argsman.AddArg("-rpcpassword=<pw>", "Password for JSON-RPC connections", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::RPC); @@ -736,10 +736,13 @@ void InitParameterInteraction(ArgsManager& args) LogPrintf("%s: parameter interaction: -externalip set -> setting -discover=0\n", __func__); } - // disable whitelistrelay in blocksonly mode if (args.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY)) { + // disable whitelistrelay in blocksonly mode if (args.SoftSetBoolArg("-whitelistrelay", false)) LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -whitelistrelay=0\n", __func__); + // Reduce default mempool size in blocksonly mode to avoid unexpected resource usage + if (args.SoftSetArg("-maxmempool", ToString(DEFAULT_BLOCKSONLY_MAX_MEMPOOL_SIZE_MB))) + LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -maxmempool=%d\n", __func__, DEFAULT_BLOCKSONLY_MAX_MEMPOOL_SIZE_MB); } // Forcing relay from whitelisted hosts implies we will accept relays from them in the first place. diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h index dad6f14c39..a5a0bae86d 100644 --- a/src/kernel/mempool_options.h +++ b/src/kernel/mempool_options.h @@ -18,6 +18,8 @@ class CBlockPolicyEstimator; /** Default for -maxmempool, maximum megabytes of mempool memory usage */ static constexpr unsigned int DEFAULT_MAX_MEMPOOL_SIZE_MB{300}; +//** Default for -maxmempool when blocksonly is set */ +static constexpr unsigned int DEFAULT_BLOCKSONLY_MAX_MEMPOOL_SIZE_MB{5}; /** Default for -mempoolexpiry, expiration time for mempool transactions in hours */ static constexpr unsigned int DEFAULT_MEMPOOL_EXPIRY_HOURS{336}; /** Default for -mempoolfullrbf, if the transaction replaceability signaling is ignored */ diff --git a/src/randomenv.cpp b/src/randomenv.cpp index 35d090c71d..3e4d5a587d 100644 --- a/src/randomenv.cpp +++ b/src/randomenv.cpp @@ -13,21 +13,21 @@ #include <compat/cpuid.h> #include <crypto/sha512.h> #include <support/cleanse.h> -#include <util/time.h> // for GetTime() -#ifdef WIN32 -#include <compat/compat.h> -#endif +#include <util/time.h> #include <algorithm> #include <atomic> +#include <cstdint> +#include <cstring> #include <chrono> #include <climits> #include <thread> #include <vector> -#include <stdint.h> -#include <string.h> -#ifndef WIN32 +#ifdef WIN32 +#include <windows.h> +#include <winreg.h> +#else #include <sys/types.h> // must go before a number of other headers #include <fcntl.h> #include <netinet/in.h> diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 2b39580043..1b2543c77a 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2307,7 +2307,7 @@ static RPCHelpMan scanblocks() RPCArg{"start_height", RPCArg::Type::NUM, RPCArg::Default{0}, "Height to start to scan from"}, RPCArg{"stop_height", RPCArg::Type::NUM, RPCArg::DefaultHint{"chain tip"}, "Height to stop to scan"}, RPCArg{"filtertype", RPCArg::Type::STR, RPCArg::Default{BlockFilterTypeName(BlockFilterType::BASIC)}, "The type name of the filter"}, - RPCArg{"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", + RPCArg{"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { {"filter_false_positives", RPCArg::Type::BOOL, RPCArg::Default{false}, "Filter false positives (slower and may fail on pruned nodes). Otherwise they may occur at a rate of 1/M"}, }, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 764c4c675b..8753f845a5 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -441,7 +441,7 @@ static RPCHelpMan prioritisetransaction() "Accepts the transaction into mined blocks at a higher (or lower) priority\n", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id."}, - {"dummy", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "API-Compatibility for previous API. Must be zero or null.\n" + {"dummy", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "API-Compatibility for previous API. Must be zero or null.\n" " DEPRECATED. For forward compatibility use named arguments and omit this parameter."}, {"fee_delta", RPCArg::Type::NUM, RPCArg::Optional::NO, "The fee value (in satoshis) to add (or subtract, if negative).\n" " Note, that this value is not a fee rate. It is a value to modify absolute fee of the TX.\n" @@ -513,8 +513,8 @@ static RPCHelpMan getblocktemplate() { {"template_request", RPCArg::Type::OBJ, RPCArg::Default{UniValue::VOBJ}, "Format of the template", { - {"mode", RPCArg::Type::STR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "This must be set to \"template\", \"proposal\" (see BIP 23), or omitted"}, - {"capabilities", RPCArg::Type::ARR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "A list of strings", + {"mode", RPCArg::Type::STR, /* treat as named arg */ RPCArg::Optional::OMITTED, "This must be set to \"template\", \"proposal\" (see BIP 23), or omitted"}, + {"capabilities", RPCArg::Type::ARR, /* treat as named arg */ RPCArg::Optional::OMITTED, "A list of strings", { {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported feature, 'longpoll', 'coinbasevalue', 'proposal', 'serverlist', 'workid'"}, }}, diff --git a/src/rpc/node.cpp b/src/rpc/node.cpp index 79b8277968..5918bc6e38 100644 --- a/src/rpc/node.cpp +++ b/src/rpc/node.cpp @@ -240,11 +240,11 @@ static RPCHelpMan logging() " - \"none\", \"0\" : even if other logging categories are specified, ignore all of them.\n" , { - {"include", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "The categories to add to debug logging", + {"include", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "The categories to add to debug logging", { {"include_category", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "the valid logging category"}, }}, - {"exclude", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "The categories to remove from debug logging", + {"exclude", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "The categories to remove from debug logging", { {"exclude_category", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "the valid logging category"}, }}, @@ -294,16 +294,16 @@ static RPCHelpMan echo(const std::string& name) "\nThe difference between echo and echojson is that echojson has argument conversion enabled in the client-side table in " "bitcoin-cli and the GUI. There is no server-side difference.", { - {"arg0", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}}, - {"arg1", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}}, - {"arg2", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}}, - {"arg3", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}}, - {"arg4", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}}, - {"arg5", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}}, - {"arg6", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}}, - {"arg7", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}}, - {"arg8", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}}, - {"arg9", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "", RPCArgOptions{.skip_type_check = true}}, + {"arg0", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "", RPCArgOptions{.skip_type_check = true}}, + {"arg1", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "", RPCArgOptions{.skip_type_check = true}}, + {"arg2", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "", RPCArgOptions{.skip_type_check = true}}, + {"arg3", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "", RPCArgOptions{.skip_type_check = true}}, + {"arg4", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "", RPCArgOptions{.skip_type_check = true}}, + {"arg5", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "", RPCArgOptions{.skip_type_check = true}}, + {"arg6", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "", RPCArgOptions{.skip_type_check = true}}, + {"arg7", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "", RPCArgOptions{.skip_type_check = true}}, + {"arg8", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "", RPCArgOptions{.skip_type_check = true}}, + {"arg9", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "", RPCArgOptions{.skip_type_check = true}}, }, RPCResult{RPCResult::Type::ANY, "", "Returns whatever was passed in"}, RPCExamples{""}, @@ -376,7 +376,7 @@ static RPCHelpMan getindexinfo() return RPCHelpMan{"getindexinfo", "\nReturns the status of one or all available indices currently running in the node.\n", { - {"index_name", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Filter results for an index with a specific name."}, + {"index_name", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Filter results for an index with a specific name."}, }, RPCResult{ RPCResult::Type::OBJ_DYN, "", "", { diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp index 911c769e61..bb04f58424 100644 --- a/src/rpc/output_script.cpp +++ b/src/rpc/output_script.cpp @@ -230,7 +230,7 @@ static RPCHelpMan deriveaddresses() "For more information on output descriptors, see the documentation in the doc/descriptors.md file.\n"}, { {"descriptor", RPCArg::Type::STR, RPCArg::Optional::NO, "The descriptor."}, - {"range", RPCArg::Type::RANGE, RPCArg::Optional::OMITTED_NAMED_ARG, "If a ranged descriptor is used, this specifies the end or the range (in [begin,end] notation) to derive."}, + {"range", RPCArg::Type::RANGE, RPCArg::Optional::OMITTED, "If a ranged descriptor is used, this specifies the end or the range (in [begin,end] notation) to derive."}, }, RPCResult{ RPCResult::Type::ARR, "", "", diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 981dead3b8..39f3260ca6 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -187,7 +187,7 @@ static RPCHelpMan getrawtransaction() {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"}, {"verbosity|verbose", RPCArg::Type::NUM, RPCArg::Default{0}, "0 for hex-encoded data, 1 for a JSON object, and 2 for JSON object with fee and prevout", RPCArgOptions{.skip_type_check = true}}, - {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED_NAMED_ARG, "The block in which to look for the transaction"}, + {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "The block in which to look for the transaction"}, }, { RPCResult{"if verbosity is not set or set to 0", @@ -639,7 +639,7 @@ static RPCHelpMan signrawtransactionwithkey() {"privatekey", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "private key in base58-encoding"}, }, }, - {"prevtxs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "The previous dependent transaction outputs", + {"prevtxs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "The previous dependent transaction outputs", { {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { @@ -1578,7 +1578,7 @@ static RPCHelpMan utxoupdatepsbt() "\nUpdates all segwit inputs and outputs in a PSBT with data from output descriptors, the UTXO set or the mempool.\n", { {"psbt", RPCArg::Type::STR, RPCArg::Optional::NO, "A base64 string of a PSBT"}, - {"descriptors", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "An array of either strings or objects", { + {"descriptors", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "An array of either strings or objects", { {"", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "An output descriptor"}, {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "An object with an output descriptor and extra information", { {"desc", RPCArg::Type::STR, RPCArg::Optional::NO, "An output descriptor"}, diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 9f57a56297..44d7e2676b 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -168,7 +168,7 @@ static RPCHelpMan stop() // to the client (intended for testing) "\nRequest a graceful shutdown of " PACKAGE_NAME ".", { - {"wait", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "how long to wait in ms", RPCArgOptions{.hidden=true}}, + {"wait", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "how long to wait in ms", RPCArgOptions{.hidden=true}}, }, RPCResult{RPCResult::Type::STR, "", "A string with the content '" + RESULT + "'"}, RPCExamples{""}, diff --git a/src/rpc/txoutproof.cpp b/src/rpc/txoutproof.cpp index 8c5468634d..8eae2ef884 100644 --- a/src/rpc/txoutproof.cpp +++ b/src/rpc/txoutproof.cpp @@ -34,7 +34,7 @@ static RPCHelpMan gettxoutproof() {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "A transaction hash"}, }, }, - {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED_NAMED_ARG, "If specified, looks for txid in the block with this hash"}, + {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, "If specified, looks for txid in the block with this hash"}, }, RPCResult{ RPCResult::Type::STR, "data", "A string that is a serialized, hex-encoded data for the proof." diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 9c29387dde..a1020c3b2b 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -796,7 +796,6 @@ std::string RPCArg::ToDescriptionString(bool is_named_arg) const ret += ", optional, default=" + std::get<RPCArg::Default>(m_fallback).write(); } else { switch (std::get<RPCArg::Optional>(m_fallback)) { - case RPCArg::Optional::OMITTED_NAMED_ARG: // Deprecated alias for OMITTED, can be removed case RPCArg::Optional::OMITTED: { if (is_named_arg) ret += ", optional"; // Default value is "null" in dicts. Otherwise, // nothing to do. Element is treated as if not present and has no default value diff --git a/src/rpc/util.h b/src/rpc/util.h index 0d60e29fa4..e3783c8f76 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -149,18 +149,14 @@ struct RPCArg { /** Required arg */ NO, /** - * The arg is optional for one of two reasons: - * - * Optional arg that is a named argument and has a default value of - * `null`. - * - * Optional argument with default value omitted because they are - * implicitly clear. That is, elements in an array may not - * exist by default. + * Optional argument for which the default value is omitted from + * help text for one of two reasons: + * - It's a named argument and has a default value of `null`. + * - Its default value is implicitly clear. That is, elements in an + * array may not exist by default. * When possible, the default value should be specified. */ OMITTED, - OMITTED_NAMED_ARG, // Deprecated alias for OMITTED, can be removed }; /** Hint for default value */ using DefaultHint = std::string; diff --git a/src/script/miniscript.h b/src/script/miniscript.h index fa3b0350e9..3a3f724f03 100644 --- a/src/script/miniscript.h +++ b/src/script/miniscript.h @@ -1378,7 +1378,7 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx) assert(constructed.size() == 1); assert(constructed[0]->ScriptSize() == script_size); if (in.size() > 0) return {}; - const NodeRef<Key> tl_node = std::move(constructed.front()); + NodeRef<Key> tl_node = std::move(constructed.front()); tl_node->DuplicateKeyCheck(ctx); return tl_node; } @@ -1813,7 +1813,7 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx) } } if (constructed.size() != 1) return {}; - const NodeRef<Key> tl_node = std::move(constructed.front()); + NodeRef<Key> tl_node = std::move(constructed.front()); tl_node->DuplicateKeyCheck(ctx); // Note that due to how ComputeType works (only assign the type to the node if the // subs' types are valid) this would fail if any node of tree is badly typed. diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index fb59324f7a..8b05743330 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -19,6 +19,9 @@ #endif #include <algorithm> +#include <limits> +#include <stdexcept> +#include <utility> #ifdef ARENA_DEBUG #include <iomanip> #include <iostream> diff --git a/src/support/lockedpool.h b/src/support/lockedpool.h index 03e4e371a3..66fbc218ab 100644 --- a/src/support/lockedpool.h +++ b/src/support/lockedpool.h @@ -5,11 +5,11 @@ #ifndef BITCOIN_SUPPORT_LOCKEDPOOL_H #define BITCOIN_SUPPORT_LOCKEDPOOL_H -#include <stdint.h> +#include <cstddef> #include <list> #include <map> -#include <mutex> #include <memory> +#include <mutex> #include <unordered_map> /** diff --git a/src/test/fuzz/partially_downloaded_block.cpp b/src/test/fuzz/partially_downloaded_block.cpp new file mode 100644 index 0000000000..f8ba4f08d9 --- /dev/null +++ b/src/test/fuzz/partially_downloaded_block.cpp @@ -0,0 +1,142 @@ +#include <blockencodings.h> +#include <consensus/merkle.h> +#include <consensus/validation.h> +#include <primitives/block.h> +#include <primitives/transaction.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <test/fuzz/util/mempool.h> +#include <test/util/setup_common.h> +#include <test/util/txmempool.h> +#include <txmempool.h> + +#include <cstddef> +#include <cstdint> +#include <limits> +#include <memory> +#include <optional> +#include <set> +#include <vector> + +namespace { +const TestingSetup* g_setup; +} // namespace + +void initialize_pdb() +{ + static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(); + g_setup = testing_setup.get(); +} + +PartiallyDownloadedBlock::CheckBlockFn FuzzedCheckBlock(std::optional<BlockValidationResult> result) +{ + return [result](const CBlock&, BlockValidationState& state, const Consensus::Params&, bool, bool) { + if (result) { + return state.Invalid(*result); + } + + return true; + }; +} + +FUZZ_TARGET_INIT(partially_downloaded_block, initialize_pdb) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + + auto block{ConsumeDeserializable<CBlock>(fuzzed_data_provider)}; + if (!block || block->vtx.size() == 0 || + block->vtx.size() >= std::numeric_limits<uint16_t>::max()) { + return; + } + + CBlockHeaderAndShortTxIDs cmpctblock{*block}; + + CTxMemPool pool{MemPoolOptionsForTest(g_setup->m_node)}; + PartiallyDownloadedBlock pdb{&pool}; + + // Set of available transactions (mempool or extra_txn) + std::set<uint16_t> available; + // The coinbase is always available + available.insert(0); + + std::vector<std::pair<uint256, CTransactionRef>> extra_txn; + for (size_t i = 1; i < block->vtx.size(); ++i) { + auto tx{block->vtx[i]}; + + bool add_to_extra_txn{fuzzed_data_provider.ConsumeBool()}; + bool add_to_mempool{fuzzed_data_provider.ConsumeBool()}; + + if (add_to_extra_txn) { + extra_txn.emplace_back(tx->GetWitnessHash(), tx); + available.insert(i); + } + + if (add_to_mempool) { + LOCK2(cs_main, pool.cs); + pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, *tx)); + available.insert(i); + } + } + + auto init_status{pdb.InitData(cmpctblock, extra_txn)}; + + std::vector<CTransactionRef> missing; + // Whether we skipped a transaction that should be included in `missing`. + // FillBlock should never return READ_STATUS_OK if that is the case. + bool skipped_missing{false}; + for (size_t i = 0; i < cmpctblock.BlockTxCount(); i++) { + // If init_status == READ_STATUS_OK then a available transaction in the + // compact block (i.e. IsTxAvailable(i) == true) implies that we marked + // that transaction as available above (i.e. available.count(i) > 0). + // The reverse is not true, due to possible compact block short id + // collisions (i.e. available.count(i) > 0 does not imply + // IsTxAvailable(i) == true). + if (init_status == READ_STATUS_OK) { + assert(!pdb.IsTxAvailable(i) || available.count(i) > 0); + } + + bool skip{fuzzed_data_provider.ConsumeBool()}; + if (!pdb.IsTxAvailable(i) && !skip) { + missing.push_back(block->vtx[i]); + } + + skipped_missing |= (!pdb.IsTxAvailable(i) && skip); + } + + // Mock CheckBlock + bool fail_check_block{fuzzed_data_provider.ConsumeBool()}; + auto validation_result = + fuzzed_data_provider.PickValueInArray( + {BlockValidationResult::BLOCK_RESULT_UNSET, + BlockValidationResult::BLOCK_CONSENSUS, + BlockValidationResult::BLOCK_RECENT_CONSENSUS_CHANGE, + BlockValidationResult::BLOCK_CACHED_INVALID, + BlockValidationResult::BLOCK_INVALID_HEADER, + BlockValidationResult::BLOCK_MUTATED, + BlockValidationResult::BLOCK_MISSING_PREV, + BlockValidationResult::BLOCK_INVALID_PREV, + BlockValidationResult::BLOCK_TIME_FUTURE, + BlockValidationResult::BLOCK_CHECKPOINT, + BlockValidationResult::BLOCK_HEADER_LOW_WORK}); + pdb.m_check_block_mock = FuzzedCheckBlock( + fail_check_block ? + std::optional<BlockValidationResult>{validation_result} : + std::nullopt); + + CBlock reconstructed_block; + auto fill_status{pdb.FillBlock(reconstructed_block, missing)}; + switch (fill_status) { + case READ_STATUS_OK: + assert(!skipped_missing); + assert(!fail_check_block); + assert(block->GetHash() == reconstructed_block.GetHash()); + break; + case READ_STATUS_CHECKBLOCK_FAILED: [[fallthrough]]; + case READ_STATUS_FAILED: + assert(fail_check_block); + break; + case READ_STATUS_INVALID: + break; + } +} diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index e933167341..0cabaf323b 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -311,7 +311,7 @@ FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); const auto& node = g_setup->m_node; - auto& chainstate = node.chainman->ActiveChainstate(); + auto& chainstate{static_cast<DummyChainState&>(node.chainman->ActiveChainstate())}; MockTime(fuzzed_data_provider, chainstate); @@ -329,6 +329,8 @@ FUZZ_TARGET_INIT(tx_pool, initialize_tx_pool) CTxMemPool tx_pool_{MakeMempool(fuzzed_data_provider, node)}; MockedTxPool& tx_pool = *static_cast<MockedTxPool*>(&tx_pool_); + chainstate.SetMempool(&tx_pool); + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 300) { const auto mut_tx = ConsumeTransaction(fuzzed_data_provider, txids); diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 300760282b..d4cede8f7c 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -1827,4 +1827,14 @@ BOOST_AUTO_TEST_CASE(compute_tapbranch) BOOST_CHECK_EQUAL(ComputeTapbranchHash(hash1, hash2), result); } +BOOST_AUTO_TEST_CASE(compute_tapleaf) +{ + const uint8_t script[6] = {'f','o','o','b','a','r'}; + uint256 tlc0 = uint256S("edbc10c272a1215dcdcc11d605b9027b5ad6ed97cd45521203f136767b5b9c06"); + uint256 tlc2 = uint256S("8b5c4f90ae6bf76e259dbef5d8a59df06359c391b59263741b25eca76451b27a"); + + BOOST_CHECK_EQUAL(ComputeTapleafHash(0xc0, Span(script)), tlc0); + BOOST_CHECK_EQUAL(ComputeTapleafHash(0xc2, Span(script)), tlc2); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 4ec3ac2189..dc14a60595 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -8,6 +8,7 @@ #include <wallet/bdb.h> #include <wallet/db.h> +#include <util/check.h> #include <util/strencodings.h> #include <util/translation.h> @@ -220,17 +221,17 @@ BerkeleyEnvironment::BerkeleyEnvironment() : m_use_shared_memory(false) fMockDb = true; } -BerkeleyBatch::SafeDbt::SafeDbt() +SafeDbt::SafeDbt() { m_dbt.set_flags(DB_DBT_MALLOC); } -BerkeleyBatch::SafeDbt::SafeDbt(void* data, size_t size) +SafeDbt::SafeDbt(void* data, size_t size) : m_dbt(data, size) { } -BerkeleyBatch::SafeDbt::~SafeDbt() +SafeDbt::~SafeDbt() { if (m_dbt.get_data() != nullptr) { // Clear memory, e.g. in case it was a private key @@ -244,17 +245,17 @@ BerkeleyBatch::SafeDbt::~SafeDbt() } } -const void* BerkeleyBatch::SafeDbt::get_data() const +const void* SafeDbt::get_data() const { return m_dbt.get_data(); } -uint32_t BerkeleyBatch::SafeDbt::get_size() const +uint32_t SafeDbt::get_size() const { return m_dbt.get_size(); } -BerkeleyBatch::SafeDbt::operator Dbt*() +SafeDbt::operator Dbt*() { return &m_dbt; } @@ -307,7 +308,7 @@ BerkeleyDatabase::~BerkeleyDatabase() } } -BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const bool read_only, bool fFlushOnCloseIn) : pdb(nullptr), activeTxn(nullptr), m_cursor(nullptr), m_database(database) +BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const bool read_only, bool fFlushOnCloseIn) : pdb(nullptr), activeTxn(nullptr), m_database(database) { database.AddRef(); database.Open(); @@ -398,7 +399,6 @@ void BerkeleyBatch::Close() activeTxn->abort(); activeTxn = nullptr; pdb = nullptr; - CloseCursor(); if (fFlushOnClose) Flush(); @@ -476,15 +476,15 @@ bool BerkeleyDatabase::Rewrite(const char* pszSkip) fSuccess = false; } - if (db.StartCursor()) { + std::unique_ptr<DatabaseCursor> cursor = db.GetNewCursor(); + if (cursor) { while (fSuccess) { CDataStream ssKey(SER_DISK, CLIENT_VERSION); CDataStream ssValue(SER_DISK, CLIENT_VERSION); - bool complete; - bool ret1 = db.ReadAtCursor(ssKey, ssValue, complete); - if (complete) { + DatabaseCursor::Status ret1 = cursor->Next(ssKey, ssValue); + if (ret1 == DatabaseCursor::Status::DONE) { break; - } else if (!ret1) { + } else if (ret1 == DatabaseCursor::Status::FAIL) { fSuccess = false; break; } @@ -502,7 +502,7 @@ bool BerkeleyDatabase::Rewrite(const char* pszSkip) if (ret2 > 0) fSuccess = false; } - db.CloseCursor(); + cursor.reset(); } if (fSuccess) { db.Close(); @@ -656,30 +656,30 @@ void BerkeleyDatabase::ReloadDbEnv() env->ReloadDbEnv(); } -bool BerkeleyBatch::StartCursor() +BerkeleyCursor::BerkeleyCursor(BerkeleyDatabase& database) { - assert(!m_cursor); - if (!pdb) - return false; - int ret = pdb->cursor(nullptr, &m_cursor, 0); - return ret == 0; + if (!database.m_db.get()) { + throw std::runtime_error(STR_INTERNAL_BUG("BerkeleyDatabase does not exist")); + } + int ret = database.m_db->cursor(nullptr, &m_cursor, 0); + if (ret != 0) { + throw std::runtime_error(STR_INTERNAL_BUG(strprintf("BDB Cursor could not be created. Returned %d", ret).c_str())); + } } -bool BerkeleyBatch::ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) +DatabaseCursor::Status BerkeleyCursor::Next(CDataStream& ssKey, CDataStream& ssValue) { - complete = false; - if (m_cursor == nullptr) return false; + if (m_cursor == nullptr) return Status::FAIL; // Read at cursor SafeDbt datKey; SafeDbt datValue; int ret = m_cursor->get(datKey, datValue, DB_NEXT); if (ret == DB_NOTFOUND) { - complete = true; + return Status::DONE; + } + if (ret != 0 || datKey.get_data() == nullptr || datValue.get_data() == nullptr) { + return Status::FAIL; } - if (ret != 0) - return false; - else if (datKey.get_data() == nullptr || datValue.get_data() == nullptr) - return false; // Convert to streams ssKey.SetType(SER_DISK); @@ -688,16 +688,22 @@ bool BerkeleyBatch::ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& ssValue.SetType(SER_DISK); ssValue.clear(); ssValue.write({AsBytePtr(datValue.get_data()), datValue.get_size()}); - return true; + return Status::MORE; } -void BerkeleyBatch::CloseCursor() +BerkeleyCursor::~BerkeleyCursor() { if (!m_cursor) return; m_cursor->close(); m_cursor = nullptr; } +std::unique_ptr<DatabaseCursor> BerkeleyBatch::GetNewCursor() +{ + if (!pdb) return nullptr; + return std::make_unique<BerkeleyCursor>(m_database); +} + bool BerkeleyBatch::TxnBegin() { if (!pdb || activeTxn) diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 40a1031c8e..3881419d45 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -165,29 +165,41 @@ public: std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override; }; -/** RAII class that provides access to a Berkeley database */ -class BerkeleyBatch : public DatabaseBatch +/** RAII class that automatically cleanses its data on destruction */ +class SafeDbt final { - /** RAII class that automatically cleanses its data on destruction */ - class SafeDbt final - { - Dbt m_dbt; + Dbt m_dbt; - public: - // construct Dbt with internally-managed data - SafeDbt(); - // construct Dbt with provided data - SafeDbt(void* data, size_t size); - ~SafeDbt(); +public: + // construct Dbt with internally-managed data + SafeDbt(); + // construct Dbt with provided data + SafeDbt(void* data, size_t size); + ~SafeDbt(); + + // delegate to Dbt + const void* get_data() const; + uint32_t get_size() const; + + // conversion operator to access the underlying Dbt + operator Dbt*(); +}; - // delegate to Dbt - const void* get_data() const; - uint32_t get_size() const; +class BerkeleyCursor : public DatabaseCursor +{ +private: + Dbc* m_cursor; - // conversion operator to access the underlying Dbt - operator Dbt*(); - }; +public: + explicit BerkeleyCursor(BerkeleyDatabase& database); + ~BerkeleyCursor() override; + + Status Next(CDataStream& key, CDataStream& value) override; +}; +/** RAII class that provides access to a Berkeley database */ +class BerkeleyBatch : public DatabaseBatch +{ private: bool ReadKey(CDataStream&& key, CDataStream& value) override; bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite = true) override; @@ -198,7 +210,6 @@ protected: Db* pdb; std::string strFile; DbTxn* activeTxn; - Dbc* m_cursor; bool fReadOnly; bool fFlushOnClose; BerkeleyEnvironment *env; @@ -214,9 +225,7 @@ public: void Flush() override; void Close() override; - bool StartCursor() override; - bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override; - void CloseCursor() override; + std::unique_ptr<DatabaseCursor> GetNewCursor() override; bool TxnBegin() override; bool TxnCommit() override; bool TxnAbort() override; diff --git a/src/wallet/db.h b/src/wallet/db.h index f09844c37e..d040af0d14 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -22,6 +22,25 @@ struct bilingual_str; namespace wallet { void SplitWalletPath(const fs::path& wallet_path, fs::path& env_directory, std::string& database_filename); +class DatabaseCursor +{ +public: + explicit DatabaseCursor() {} + virtual ~DatabaseCursor() {} + + DatabaseCursor(const DatabaseCursor&) = delete; + DatabaseCursor& operator=(const DatabaseCursor&) = delete; + + enum class Status + { + FAIL, + MORE, + DONE, + }; + + virtual Status Next(CDataStream& key, CDataStream& value) { return Status::FAIL; } +}; + /** RAII class that provides access to a WalletDatabase */ class DatabaseBatch { @@ -92,9 +111,7 @@ public: return HasKey(std::move(ssKey)); } - virtual bool StartCursor() = 0; - virtual bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) = 0; - virtual void CloseCursor() = 0; + virtual std::unique_ptr<DatabaseCursor> GetNewCursor() = 0; virtual bool TxnBegin() = 0; virtual bool TxnCommit() = 0; virtual bool TxnAbort() = 0; @@ -156,6 +173,11 @@ public: virtual std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) = 0; }; +class DummyCursor : public DatabaseCursor +{ + Status Next(CDataStream& key, CDataStream& value) override { return Status::FAIL; } +}; + /** RAII class that provides access to a DummyDatabase. Never fails. */ class DummyBatch : public DatabaseBatch { @@ -169,9 +191,7 @@ public: void Flush() override {} void Close() override {} - bool StartCursor() override { return true; } - bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override { return true; } - void CloseCursor() override {} + std::unique_ptr<DatabaseCursor> GetNewCursor() override { return std::make_unique<DummyCursor>(); } bool TxnBegin() override { return true; } bool TxnCommit() override { return true; } bool TxnAbort() override { return true; } diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index efa548ad91..901e23b90a 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -47,7 +47,8 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error) std::unique_ptr<DatabaseBatch> batch = db.MakeBatch(); bool ret = true; - if (!batch->StartCursor()) { + std::unique_ptr<DatabaseCursor> cursor = batch->GetNewCursor(); + if (!cursor) { error = _("Error: Couldn't create cursor into database"); ret = false; } @@ -68,13 +69,13 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error) while (true) { CDataStream ss_key(SER_DISK, CLIENT_VERSION); CDataStream ss_value(SER_DISK, CLIENT_VERSION); - bool complete; - ret = batch->ReadAtCursor(ss_key, ss_value, complete); - if (complete) { + DatabaseCursor::Status status = cursor->Next(ss_key, ss_value); + if (status == DatabaseCursor::Status::DONE) { ret = true; break; - } else if (!ret) { + } else if (status == DatabaseCursor::Status::FAIL) { error = _("Error reading next record from wallet database"); + ret = false; break; } std::string key_str = HexStr(ss_key); @@ -85,7 +86,7 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error) } } - batch->CloseCursor(); + cursor.reset(); batch.reset(); // Close the wallet after we're done with it. The caller won't be doing this diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 95e1ba4dd9..da63d45d11 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -226,7 +226,7 @@ RPCHelpMan addmultisigaddress() {"key", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "bitcoin address or hex-encoded public key"}, }, }, - {"label", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "A label to assign the addresses to."}, + {"label", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A label to assign the addresses to."}, {"address_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -addresstype"}, "The address type to use. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."}, }, RPCResult{ @@ -696,7 +696,7 @@ RPCHelpMan listlabels() return RPCHelpMan{"listlabels", "\nReturns the list of all labels, or labels that are assigned to addresses with a specific purpose.\n", { - {"purpose", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Address purpose to list labels for ('send','receive'). An empty string is the same as not providing this argument."}, + {"purpose", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Address purpose to list labels for ('send','receive'). An empty string is the same as not providing this argument."}, }, RPCResult{ RPCResult::Type::ARR, "", "", diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index ab46706084..71a913c9e0 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1292,7 +1292,7 @@ RPCHelpMan importmulti() }, }, RPCArgOptions{.oneline_description="\"requests\""}}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions after all imports."}, }, @@ -1891,7 +1891,7 @@ RPCHelpMan restorewallet() { {"wallet_name", RPCArg::Type::STR, RPCArg::Optional::NO, "The name that will be applied to the restored wallet"}, {"backup_file", RPCArg::Type::STR, RPCArg::Optional::NO, "The backup file that will be used to restore the wallet."}, - {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, + {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 82642194c2..32151d5b5c 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -165,7 +165,7 @@ RPCHelpMan getbalance() "The available balance is what the wallet considers currently spendable, and is\n" "thus affected by options which limit spendability such as -spendzeroconfchange.\n", { - {"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Remains for backward compatibility. Must be excluded or set to \"*\"."}, + {"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Remains for backward compatibility. Must be excluded or set to \"*\"."}, {"minconf", RPCArg::Type::NUM, RPCArg::Default{0}, "Only include transactions confirmed at least this many times."}, {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also include balance in watch-only addresses (see 'importaddress')"}, {"avoid_reuse", RPCArg::Type::BOOL, RPCArg::Default{true}, "(only available if avoid_reuse wallet flag is set) Do not include balance in dirty outputs; addresses are considered dirty if they have previously been used in a transaction."}, @@ -509,7 +509,7 @@ RPCHelpMan listunspent() }, {"include_unsafe", RPCArg::Type::BOOL, RPCArg::Default{true}, "Include outputs that are not safe to spend\n" "See description of \"safe\" attribute below."}, - {"query_options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "JSON with query options", + {"query_options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "JSON with query options", { {"minimumAmount", RPCArg::Type::AMOUNT, RPCArg::Default{FormatMoney(0)}, "Minimum value of each UTXO in " + CURRENCY_UNIT + ""}, {"maximumAmount", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"unlimited"}, "Maximum value of each UTXO in " + CURRENCY_UNIT + ""}, diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index f7fd6e479f..cab797bbce 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -217,9 +217,9 @@ RPCHelpMan sendtoaddress() { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address to send to."}, {"amount", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The amount in " + CURRENCY_UNIT + " to send. eg 0.1"}, - {"comment", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "A comment used to store what the transaction is for.\n" + {"comment", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A comment used to store what the transaction is for.\n" "This is not part of the transaction, just kept in your wallet."}, - {"comment_to", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "A comment to store the name of the person or organization\n" + {"comment_to", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A comment to store the name of the person or organization\n" "to which you're sending the transaction. This is not part of the \n" "transaction, just kept in your wallet."}, {"subtractfeefromamount", RPCArg::Type::BOOL, RPCArg::Default{false}, "The fee will be deducted from the amount being sent.\n" @@ -326,9 +326,9 @@ RPCHelpMan sendmany() {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The bitcoin address is the key, the numeric amount (can be string) in " + CURRENCY_UNIT + " is the value"}, }, }, - {"minconf", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "Ignored dummy value"}, - {"comment", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "A comment"}, - {"subtractfeefrom", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "The addresses.\n" + {"minconf", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "Ignored dummy value"}, + {"comment", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A comment"}, + {"subtractfeefrom", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "The addresses.\n" "The fee will be equally deducted from the amount of each selected address.\n" "Those recipients will receive less bitcoins than you enter in their corresponding amount field.\n" "If no addresses are specified here, the sender pays the fee.", @@ -462,7 +462,7 @@ static std::vector<RPCArg> FundTxDoc(bool solving_data = true) }, }; if (solving_data) { - args.push_back({"solving_data", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "Keys and scripts needed for producing a final transaction with a dummy signature.\n" + args.push_back({"solving_data", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "Keys and scripts needed for producing a final transaction with a dummy signature.\n" "Used for fee estimation during coin selection.", { { @@ -758,7 +758,7 @@ RPCHelpMan fundrawtransaction() "Only pay-to-pubkey, multisig, and P2SH versions thereof are currently supported for watch-only\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "for backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}", + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "for backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}", Cat<std::vector<RPCArg>>( { {"add_inputs", RPCArg::Type::BOOL, RPCArg::Default{true}, "For a transaction with existing inputs, automatically include more if they are not enough."}, @@ -784,7 +784,7 @@ RPCHelpMan fundrawtransaction() {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, }, }, - {"input_weights", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "Inputs and their corresponding weights", + {"input_weights", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "Inputs and their corresponding weights", { {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { @@ -870,7 +870,7 @@ RPCHelpMan signrawtransactionwithwallet() HELP_REQUIRING_PASSPHRASE, { {"hexstring", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction hex string"}, - {"prevtxs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "The previous dependent transaction outputs", + {"prevtxs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "The previous dependent transaction outputs", { {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { @@ -977,7 +977,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) "* WARNING: before version 0.21, fee_rate was in " + CURRENCY_UNIT + "/kvB. As of 0.21, fee_rate is in " + CURRENCY_ATOM + "/vB. *\n", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The txid to be bumped"}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks\n"}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, @@ -1161,7 +1161,7 @@ RPCHelpMan send() {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n" "\"" + FeeModes("\"\n\"") + "\""}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", Cat<std::vector<RPCArg>>( { {"add_inputs", RPCArg::Type::BOOL, RPCArg::DefaultHint{"false when \"inputs\" are specified, true otherwise"},"Automatically include coins from the wallet to cover the target amount.\n"}, @@ -1276,7 +1276,7 @@ RPCHelpMan sendall() "\"" + FeeModes("\"\n\"") + "\""}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, { - "options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", + "options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", Cat<std::vector<RPCArg>>( { {"add_to_wallet", RPCArg::Type::BOOL, RPCArg::Default{true}, "When false, returns the serialized transaction without broadcasting or adding it to the wallet"}, @@ -1586,7 +1586,7 @@ RPCHelpMan walletcreatefundedpsbt() "All existing inputs must either have their previous output transaction be in the wallet\n" "or be in the UTXO set. Solving data must be provided for non-wallet inputs.\n", { - {"inputs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "Leave empty to add inputs automatically. See add_inputs option.", + {"inputs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "Leave empty to add inputs automatically. See add_inputs option.", { {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { @@ -1620,7 +1620,7 @@ RPCHelpMan walletcreatefundedpsbt() }, RPCArgOptions{.skip_type_check = true}}, {"locktime", RPCArg::Type::NUM, RPCArg::Default{0}, "Raw locktime. Non-0 value also locktime-activates inputs"}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", Cat<std::vector<RPCArg>>( { {"add_inputs", RPCArg::Type::BOOL, RPCArg::DefaultHint{"false when \"inputs\" are specified, true otherwise"}, "Automatically include coins from the wallet to cover the target amount.\n"}, diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index f571f8bcb2..4a076a7040 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -206,7 +206,7 @@ RPCHelpMan listreceivedbyaddress() {"minconf", RPCArg::Type::NUM, RPCArg::Default{1}, "The minimum number of confirmations before payments are included."}, {"include_empty", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether to include addresses that haven't received any payments."}, {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Whether to include watch-only addresses (see 'importaddress')"}, - {"address_filter", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "If present and non-empty, only return information on this address."}, + {"address_filter", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "If present and non-empty, only return information on this address."}, {"include_immature_coinbase", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include immature coinbase transactions."}, }, RPCResult{ @@ -434,7 +434,7 @@ RPCHelpMan listtransactions() "\nIf a label name is provided, this will return only incoming transactions paying to addresses with the specified label.\n" "\nReturns up to 'count' most recent transactions skipping the first 'from' transactions.\n", { - {"label|dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "If set, should be a valid label name to return only incoming transactions\n" + {"label|dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "If set, should be a valid label name to return only incoming transactions\n" "with the specified label, or \"*\" to disable filtering and return all transactions."}, {"count", RPCArg::Type::NUM, RPCArg::Default{10}, "The number of transactions to return"}, {"skip", RPCArg::Type::NUM, RPCArg::Default{0}, "The number of transactions to skip"}, @@ -545,13 +545,13 @@ RPCHelpMan listsinceblock() "If \"blockhash\" is no longer a part of the main chain, transactions from the fork point onward are included.\n" "Additionally, if include_removed is set, transactions affecting the wallet which were removed are returned in the \"removed\" array.\n", { - {"blockhash", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "If set, the block hash to list transactions since, otherwise list all transactions."}, + {"blockhash", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "If set, the block hash to list transactions since, otherwise list all transactions."}, {"target_confirmations", RPCArg::Type::NUM, RPCArg::Default{1}, "Return the nth block hash from the main chain. e.g. 1 would mean the best block hash. Note: this is not used as a filter, but only affects [lastblock] in the return value"}, {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Include transactions to watch-only addresses (see 'importaddress')"}, {"include_removed", RPCArg::Type::BOOL, RPCArg::Default{true}, "Show transactions that were removed due to a reorg in the \"removed\" array\n" "(not guaranteed to work on pruned nodes)"}, {"include_change", RPCArg::Type::BOOL, RPCArg::Default{false}, "Also add entries for change outputs.\n"}, - {"label", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Return only incoming transactions paying to addresses with the specified label.\n"}, + {"label", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Return only incoming transactions paying to addresses with the specified label.\n"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -848,7 +848,7 @@ RPCHelpMan rescanblockchain() "and block filters are available (using startup option \"-blockfilterindex=1\").\n", { {"start_height", RPCArg::Type::NUM, RPCArg::Default{0}, "block height where the rescan should start"}, - {"stop_height", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "the last block height that should be scanned. If none is provided it will rescan up to the tip at return time of this call."}, + {"stop_height", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "the last block height that should be scanned. If none is provided it will rescan up to the tip at return time of this call."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 63be95fdd3..23a88cd51b 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -201,7 +201,7 @@ static RPCHelpMan loadwallet() "\napplied to the new wallet.\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet directory or .dat file."}, - {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, + {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -323,12 +323,12 @@ static RPCHelpMan createwallet() {"wallet_name", RPCArg::Type::STR, RPCArg::Optional::NO, "The name for the new wallet. If this is a path, the wallet will be created at the path location."}, {"disable_private_keys", RPCArg::Type::BOOL, RPCArg::Default{false}, "Disable the possibility of private keys (only watchonlys are possible in this mode)."}, {"blank", RPCArg::Type::BOOL, RPCArg::Default{false}, "Create a blank wallet. A blank wallet has no keys or HD seed. One can be set using sethdseed."}, - {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Encrypt the wallet with this passphrase."}, + {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Encrypt the wallet with this passphrase."}, {"avoid_reuse", RPCArg::Type::BOOL, RPCArg::Default{false}, "Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind."}, {"descriptors", RPCArg::Type::BOOL, RPCArg::Default{true}, "Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation." " Setting to \"false\" will create a legacy wallet; however, the legacy wallet type is being deprecated and" " support for creating and opening legacy wallets will be removed in the future."}, - {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, + {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, {"external_signer", RPCArg::Type::BOOL, RPCArg::Default{false}, "Use an external signer such as a hardware wallet. Requires -signer to be configured. Wallet creation will fail if keys cannot be fetched. Requires disable_private_keys and descriptors set to true."}, }, RPCResult{ @@ -419,7 +419,7 @@ static RPCHelpMan unloadwallet() "Specifying the wallet name on a wallet endpoint is invalid.", { {"wallet_name", RPCArg::Type::STR, RPCArg::DefaultHint{"the wallet name from the RPC endpoint"}, "The name of the wallet to unload. If provided both here and in the RPC endpoint, the two must be identical."}, - {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, + {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "warning", "Warning message if wallet was not unloaded cleanly."}, @@ -608,12 +608,12 @@ RPCHelpMan simulaterawtransaction() return RPCHelpMan{"simulaterawtransaction", "\nCalculate the balance change resulting in the signing and broadcasting of the given transaction(s).\n", { - {"rawtxs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "An array of hex strings of raw transactions.\n", + {"rawtxs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "An array of hex strings of raw transactions.\n", { {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""}, }, }, - {"options", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED_NAMED_ARG, "Options", + {"options", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "Options", { {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Whether to include watch-only addresses (see RPC importaddress)"}, }, diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index f2b9909851..7a0c4b1806 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -125,7 +125,6 @@ void SQLiteBatch::SetupSQLStatements() {&m_insert_stmt, "INSERT INTO main VALUES(?, ?)"}, {&m_overwrite_stmt, "INSERT or REPLACE into main values(?, ?)"}, {&m_delete_stmt, "DELETE FROM main WHERE key = ?"}, - {&m_cursor_stmt, "SELECT key, value FROM main"}, }; for (const auto& [stmt_prepared, stmt_text] : statements) { @@ -374,7 +373,6 @@ void SQLiteBatch::Close() {&m_insert_stmt, "insert"}, {&m_overwrite_stmt, "overwrite"}, {&m_delete_stmt, "delete"}, - {&m_cursor_stmt, "cursor"}, }; for (const auto& [stmt_prepared, stmt_description] : statements) { @@ -472,28 +470,15 @@ bool SQLiteBatch::HasKey(CDataStream&& key) return res == SQLITE_ROW; } -bool SQLiteBatch::StartCursor() +DatabaseCursor::Status SQLiteCursor::Next(CDataStream& key, CDataStream& value) { - assert(!m_cursor_init); - if (!m_database.m_db) return false; - m_cursor_init = true; - return true; -} - -bool SQLiteBatch::ReadAtCursor(CDataStream& key, CDataStream& value, bool& complete) -{ - complete = false; - - if (!m_cursor_init) return false; - int res = sqlite3_step(m_cursor_stmt); if (res == SQLITE_DONE) { - complete = true; - return true; + return Status::DONE; } if (res != SQLITE_ROW) { - LogPrintf("SQLiteBatch::ReadAtCursor: Unable to execute cursor step: %s\n", sqlite3_errstr(res)); - return false; + LogPrintf("%s: Unable to execute cursor step: %s\n", __func__, sqlite3_errstr(res)); + return Status::FAIL; } // Leftmost column in result is index 0 @@ -503,13 +488,32 @@ bool SQLiteBatch::ReadAtCursor(CDataStream& key, CDataStream& value, bool& compl const std::byte* value_data{AsBytePtr(sqlite3_column_blob(m_cursor_stmt, 1))}; size_t value_data_size(sqlite3_column_bytes(m_cursor_stmt, 1)); value.write({value_data, value_data_size}); - return true; + return Status::MORE; } -void SQLiteBatch::CloseCursor() +SQLiteCursor::~SQLiteCursor() { sqlite3_reset(m_cursor_stmt); - m_cursor_init = false; + int res = sqlite3_finalize(m_cursor_stmt); + if (res != SQLITE_OK) { + LogPrintf("%s: cursor closed but could not finalize cursor statement: %s\n", + __func__, sqlite3_errstr(res)); + } +} + +std::unique_ptr<DatabaseCursor> SQLiteBatch::GetNewCursor() +{ + if (!m_database.m_db) return nullptr; + auto cursor = std::make_unique<SQLiteCursor>(); + + const char* stmt_text = "SELECT key, value FROM main"; + int res = sqlite3_prepare_v2(m_database.m_db, stmt_text, -1, &cursor->m_cursor_stmt, nullptr); + if (res != SQLITE_OK) { + throw std::runtime_error(strprintf( + "%s: Failed to setup cursor SQL statement: %s\n", __func__, sqlite3_errstr(res))); + } + + return cursor; } bool SQLiteBatch::TxnBegin() diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index 7680bdd07b..b35d24fe46 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -14,19 +14,27 @@ struct bilingual_str; namespace wallet { class SQLiteDatabase; +class SQLiteCursor : public DatabaseCursor +{ +public: + sqlite3_stmt* m_cursor_stmt{nullptr}; + + explicit SQLiteCursor() {} + ~SQLiteCursor() override; + + Status Next(CDataStream& key, CDataStream& value) override; +}; + /** RAII class that provides access to a WalletDatabase */ class SQLiteBatch : public DatabaseBatch { private: SQLiteDatabase& m_database; - bool m_cursor_init = false; - sqlite3_stmt* m_read_stmt{nullptr}; sqlite3_stmt* m_insert_stmt{nullptr}; sqlite3_stmt* m_overwrite_stmt{nullptr}; sqlite3_stmt* m_delete_stmt{nullptr}; - sqlite3_stmt* m_cursor_stmt{nullptr}; void SetupSQLStatements(); @@ -44,9 +52,7 @@ public: void Close() override; - bool StartCursor() override; - bool ReadAtCursor(CDataStream& key, CDataStream& value, bool& complete) override; - void CloseCursor() override; + std::unique_ptr<DatabaseCursor> GetNewCursor() override; bool TxnBegin() override; bool TxnCommit() override; bool TxnAbort() override; diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index 88597bd320..f9fb99e11e 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -50,7 +50,7 @@ std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, // Get a cursor to the original database auto batch = database.MakeBatch(); - batch->StartCursor(); + std::unique_ptr<wallet::DatabaseCursor> cursor = batch->GetNewCursor(); // Get a batch for the new database auto new_batch = new_database->MakeBatch(); @@ -59,9 +59,9 @@ std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, while (true) { CDataStream key(SER_DISK, CLIENT_VERSION); CDataStream value(SER_DISK, CLIENT_VERSION); - bool complete; - batch->ReadAtCursor(key, value, complete); - if (complete) break; + DatabaseCursor::Status status = cursor->Next(key, value); + assert(status != DatabaseCursor::Status::FAIL); + if (status == DatabaseCursor::Status::DONE) break; new_batch->Write(key, value); } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index b6e50e961a..31fc3344cc 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -907,6 +907,12 @@ BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup) TestUnloadWallet(std::move(wallet)); } +class FailCursor : public DatabaseCursor +{ +public: + Status Next(CDataStream& key, CDataStream& value) override { return Status::FAIL; } +}; + /** RAII class that provides access to a FailDatabase. Which fails if needed. */ class FailBatch : public DatabaseBatch { @@ -922,9 +928,7 @@ public: void Flush() override {} void Close() override {} - bool StartCursor() override { return true; } - bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override { return false; } - void CloseCursor() override {} + std::unique_ptr<DatabaseCursor> GetNewCursor() override { return std::make_unique<FailCursor>(); } bool TxnBegin() override { return false; } bool TxnCommit() override { return false; } bool TxnAbort() override { return false; } diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index 24d21c2f22..f74bf54d9e 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -54,13 +54,15 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_unknown_descriptor, TestingSetup) bool HasAnyRecordOfType(WalletDatabase& db, const std::string& key) { std::unique_ptr<DatabaseBatch> batch = db.MakeBatch(false); - BOOST_CHECK(batch->StartCursor()); + BOOST_CHECK(batch); + std::unique_ptr<DatabaseCursor> cursor = batch->GetNewCursor(); + BOOST_CHECK(cursor); while (true) { CDataStream ssKey(SER_DISK, CLIENT_VERSION); CDataStream ssValue(SER_DISK, CLIENT_VERSION); - bool complete; - BOOST_CHECK(batch->ReadAtCursor(ssKey, ssValue, complete)); - if (complete) break; + DatabaseCursor::Status status = cursor->Next(ssKey, ssValue); + assert(status != DatabaseCursor::Status::FAIL); + if (status == DatabaseCursor::Status::DONE) break; std::string type; ssKey >> type; if (type == key) return true; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6158ff033c..b938858315 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2995,7 +2995,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri if (args.IsArgSet("-mintxfee")) { std::optional<CAmount> min_tx_fee = ParseMoney(args.GetArg("-mintxfee", "")); - if (!min_tx_fee || min_tx_fee.value() == 0) { + if (!min_tx_fee) { error = AmountErrMsg("mintxfee", args.GetArg("-mintxfee", "")); return nullptr; } else if (min_tx_fee.value() > HIGH_TX_FEE_PER_KB) { @@ -3775,26 +3775,27 @@ bool CWallet::MigrateToSQLite(bilingual_str& error) // Get all of the records for DB type migration std::unique_ptr<DatabaseBatch> batch = m_database->MakeBatch(); + std::unique_ptr<DatabaseCursor> cursor = batch->GetNewCursor(); std::vector<std::pair<SerializeData, SerializeData>> records; - if (!batch->StartCursor()) { + if (!cursor) { error = _("Error: Unable to begin reading all records in the database"); return false; } - bool complete = false; + DatabaseCursor::Status status = DatabaseCursor::Status::FAIL; while (true) { CDataStream ss_key(SER_DISK, CLIENT_VERSION); CDataStream ss_value(SER_DISK, CLIENT_VERSION); - bool ret = batch->ReadAtCursor(ss_key, ss_value, complete); - if (!ret) { + status = cursor->Next(ss_key, ss_value); + if (status != DatabaseCursor::Status::MORE) { break; } SerializeData key(ss_key.begin(), ss_key.end()); SerializeData value(ss_value.begin(), ss_value.end()); records.emplace_back(key, value); } - batch->CloseCursor(); + cursor.reset(); batch.reset(); - if (!complete) { + if (status != DatabaseCursor::Status::DONE) { error = _("Error: Unable to read all records in the database"); return false; } diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index b393c35112..7c59b6aac0 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -812,7 +812,8 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) #endif // Get cursor - if (!m_batch->StartCursor()) + std::unique_ptr<DatabaseCursor> cursor = m_batch->GetNewCursor(); + if (!cursor) { pwallet->WalletLogPrintf("Error getting wallet database cursor\n"); return DBErrors::CORRUPT; @@ -823,14 +824,11 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // Read next record CDataStream ssKey(SER_DISK, CLIENT_VERSION); CDataStream ssValue(SER_DISK, CLIENT_VERSION); - bool complete; - bool ret = m_batch->ReadAtCursor(ssKey, ssValue, complete); - if (complete) { + DatabaseCursor::Status status = cursor->Next(ssKey, ssValue); + if (status == DatabaseCursor::Status::DONE) { break; - } - else if (!ret) - { - m_batch->CloseCursor(); + } else if (status == DatabaseCursor::Status::FAIL) { + cursor.reset(); pwallet->WalletLogPrintf("Error reading next record from wallet database\n"); return DBErrors::CORRUPT; } @@ -878,7 +876,6 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) } catch (...) { result = DBErrors::CORRUPT; } - m_batch->CloseCursor(); // Set the active ScriptPubKeyMans for (auto spk_man_pair : wss.m_active_external_spks) { @@ -986,7 +983,8 @@ DBErrors WalletBatch::FindWalletTxHashes(std::vector<uint256>& tx_hashes) } // Get cursor - if (!m_batch->StartCursor()) + std::unique_ptr<DatabaseCursor> cursor = m_batch->GetNewCursor(); + if (!cursor) { LogPrintf("Error getting wallet database cursor\n"); return DBErrors::CORRUPT; @@ -997,12 +995,10 @@ DBErrors WalletBatch::FindWalletTxHashes(std::vector<uint256>& tx_hashes) // Read next record CDataStream ssKey(SER_DISK, CLIENT_VERSION); CDataStream ssValue(SER_DISK, CLIENT_VERSION); - bool complete; - bool ret = m_batch->ReadAtCursor(ssKey, ssValue, complete); - if (complete) { + DatabaseCursor::Status status = cursor->Next(ssKey, ssValue); + if (status == DatabaseCursor::Status::DONE) { break; - } else if (!ret) { - m_batch->CloseCursor(); + } else if (status == DatabaseCursor::Status::FAIL) { LogPrintf("Error reading next record from wallet database\n"); return DBErrors::CORRUPT; } @@ -1018,7 +1014,6 @@ DBErrors WalletBatch::FindWalletTxHashes(std::vector<uint256>& tx_hashes) } catch (...) { result = DBErrors::CORRUPT; } - m_batch->CloseCursor(); return result; } @@ -1111,7 +1106,8 @@ bool WalletBatch::WriteWalletFlags(const uint64_t flags) bool WalletBatch::EraseRecords(const std::unordered_set<std::string>& types) { // Get cursor - if (!m_batch->StartCursor()) + std::unique_ptr<DatabaseCursor> cursor = m_batch->GetNewCursor(); + if (!cursor) { return false; } @@ -1122,14 +1118,10 @@ bool WalletBatch::EraseRecords(const std::unordered_set<std::string>& types) // Read next record CDataStream key(SER_DISK, CLIENT_VERSION); CDataStream value(SER_DISK, CLIENT_VERSION); - bool complete; - bool ret = m_batch->ReadAtCursor(key, value, complete); - if (complete) { + DatabaseCursor::Status status = cursor->Next(key, value); + if (status == DatabaseCursor::Status::DONE) { break; - } - else if (!ret) - { - m_batch->CloseCursor(); + } else if (status == DatabaseCursor::Status::FAIL) { return false; } @@ -1143,7 +1135,6 @@ bool WalletBatch::EraseRecords(const std::unordered_set<std::string>& types) m_batch->Erase(key_data); } } - m_batch->CloseCursor(); return true; } diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index d5e5ed47d6..a37d614535 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -126,7 +126,6 @@ class ConfArgsTest(BitcoinTestFramework): expected_msgs=[ 'Command-line arg: addnode="some.node"', 'Command-line arg: rpcauth=****', - 'Command-line arg: rpcbind=****', 'Command-line arg: rpcpassword=****', 'Command-line arg: rpcuser=****', 'Command-line arg: torpassword=****', @@ -135,14 +134,17 @@ class ConfArgsTest(BitcoinTestFramework): ], unexpected_msgs=[ 'alice:f7efda5c189b999524f151318c0c86$d5b51b3beffbc0', - '127.1.1.1', 'secret-rpcuser', 'secret-torpassword', + 'Command-line arg: rpcbind=****', + 'Command-line arg: rpcallowip=****', ]): self.start_node(0, extra_args=[ '-addnode=some.node', '-rpcauth=alice:f7efda5c189b999524f151318c0c86$d5b51b3beffbc0', '-rpcbind=127.1.1.1', + '-rpcbind=127.0.0.1', + "-rpcallowip=127.0.0.1", '-rpcpassword=', '-rpcuser=secret-rpcuser', '-torpassword=secret-torpassword', diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py index 0a84a66a8f..947d2e8273 100755 --- a/test/functional/feature_rbf.py +++ b/test/functional/feature_rbf.py @@ -392,11 +392,11 @@ class ReplaceByFeeTest(BitcoinTestFramework): enough transactions off of each root UTXO to exceed the MAX_REPLACEMENT_LIMIT. Then create a conflicting RBF replacement transaction. """ - normal_node = self.nodes[1] - wallet = MiniWallet(normal_node) # Clear mempools to avoid cross-node sync failure. for node in self.nodes: self.generate(node, 1) + normal_node = self.nodes[1] + wallet = MiniWallet(normal_node) # This has to be chosen so that the total number of transactions can exceed # MAX_REPLACEMENT_LIMIT without having any one tx graph run into the descendant diff --git a/test/functional/p2p_headers_sync_with_minchainwork.py b/test/functional/p2p_headers_sync_with_minchainwork.py index b07077c668..832fd7e0e9 100755 --- a/test/functional/p2p_headers_sync_with_minchainwork.py +++ b/test/functional/p2p_headers_sync_with_minchainwork.py @@ -27,6 +27,7 @@ NODE2_BLOCKS_REQUIRED = 2047 class RejectLowDifficultyHeadersTest(BitcoinTestFramework): def set_test_params(self): + self.rpc_timeout *= 4 # To avoid timeout when generating BLOCKS_TO_MINE self.setup_clean_chain = True self.num_nodes = 4 # Node0 has no required chainwork; node1 requires 15 blocks on top of the genesis block; node2 requires 2047 |