From 5db84e2a5b8fae5fcffa3a86b683b2fdf9c1c3a3 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 11 Jul 2024 16:31:07 +0000 Subject: [PATCH 01/21] revert: partial dash#3003 (Fix 2 common Travis failures which happen when Travis has network issues) commits reverted: - 9a4524162111870308fcf3313b3e0cf5402f079f - 41f46f5ea68df4b6bd97baf5660f9076643294ba --- depends/Makefile | 1 + depends/builders/darwin.mk | 2 +- depends/builders/linux.mk | 2 +- depends/funcs.mk | 2 -- 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/depends/Makefile b/depends/Makefile index 823be933a7..67d4fae792 100644 --- a/depends/Makefile +++ b/depends/Makefile @@ -49,6 +49,7 @@ PATCHES_PATH = $(BASEDIR)/patches BASEDIR = $(CURDIR) HASH_LENGTH:=11 DOWNLOAD_CONNECT_TIMEOUT:=30 +DOWNLOAD_RETRIES:=3 HOST_ID_SALT ?= salt BUILD_ID_SALT ?= salt diff --git a/depends/builders/darwin.mk b/depends/builders/darwin.mk index e298d302ca..4721c1a354 100644 --- a/depends/builders/darwin.mk +++ b/depends/builders/darwin.mk @@ -7,7 +7,7 @@ build_darwin_OTOOL:=$(shell xcrun -f otool) build_darwin_NM:=$(shell xcrun -f nm) build_darwin_INSTALL_NAME_TOOL:=$(shell xcrun -f install_name_tool) build_darwin_SHA256SUM=shasum -a 256 -build_darwin_DOWNLOAD=curl --location --fail --connect-timeout $(DOWNLOAD_CONNECT_TIMEOUT) -o +build_darwin_DOWNLOAD=curl --location --fail --connect-timeout $(DOWNLOAD_CONNECT_TIMEOUT) --retry $(DOWNLOAD_RETRIES) -o #darwin host on darwin builder. overrides darwin host preferences. darwin_CC=$(shell xcrun -f clang) -mmacosx-version-min=$(OSX_MIN_VERSION) -isysroot$(shell xcrun --show-sdk-path) diff --git a/depends/builders/linux.mk b/depends/builders/linux.mk index 9af0d066a0..b03f424010 100644 --- a/depends/builders/linux.mk +++ b/depends/builders/linux.mk @@ -1,2 +1,2 @@ build_linux_SHA256SUM = sha256sum -build_linux_DOWNLOAD = curl --location --fail --connect-timeout $(DOWNLOAD_CONNECT_TIMEOUT) -o +build_linux_DOWNLOAD = curl --location --fail --connect-timeout $(DOWNLOAD_CONNECT_TIMEOUT) --retry $(DOWNLOAD_RETRIES) -o diff --git a/depends/funcs.mk b/depends/funcs.mk index deca7806cc..87052bf6e0 100644 --- a/depends/funcs.mk +++ b/depends/funcs.mk @@ -37,8 +37,6 @@ endef define fetch_file ( test -f $$($(1)_source_dir)/$(4) || \ ( $(call fetch_file_inner,$(1),$(2),$(3),$(4),$(5)) || \ - (sleep 5 && $(call fetch_file_inner,$(1),$(2),$(3),$(4),$(5))) || \ - (sleep 10 && $(call fetch_file_inner,$(1),$(2),$(3),$(4),$(5))) || \ $(call fetch_file_inner,$(1),$(FALLBACK_DOWNLOAD_PATH),$(3),$(4),$(5)))) endef From f15e1db4770691e88890e98e1d2842a4bb57c145 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 11 Jul 2024 16:37:02 +0000 Subject: [PATCH 02/21] fix: make `std::unary_function` suppression flag no longer contingent on `--enable-suppress-external-warnings` This is in line with how it's done in 880d4aaf upstream. This wasn't noticed with CI as we build with `--enable-suppress-external-warnings` by default. --- configure.ac | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index 84eeaa7299..c4ace5c7de 100644 --- a/configure.ac +++ b/configure.ac @@ -1458,6 +1458,12 @@ AX_BOOST_BASE([1.64.0],[],[AC_MSG_ERROR([Boost is not available!])]) if test x$want_boost = xno; then AC_MSG_ERROR([[only libdashconsensus can be built without boost]]) fi + +dnl Prevent use of std::unary_function, which was removed in C++17, +dnl and will generate warnings with newer compilers. +dnl See: https://github.com/boostorg/container_hash/issues/22. +BOOST_CPPFLAGS="$BOOST_CPPFLAGS -DBOOST_NO_CXX98_FUNCTION_BASE" + AX_BOOST_FILESYSTEM dnl Opt-in to Boost Process @@ -1471,11 +1477,6 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include ]], fi if test x$suppress_external_warnings != xno; then - dnl Prevent use of std::unary_function, which was removed in C++17, - dnl and will generate warnings with newer compilers. - dnl See: https://github.com/boostorg/container_hash/issues/22. - BOOST_CPPFLAGS="$BOOST_CPPFLAGS -DBOOST_NO_CXX98_FUNCTION_BASE" - BOOST_CPPFLAGS=SUPPRESS_WARNINGS($BOOST_CPPFLAGS) fi From 3c622a3916d25b6462ad30bd7a246b56ce004a45 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 11 Jul 2024 15:30:01 +0000 Subject: [PATCH 03/21] revert: partial dash#5610 (make depends compilable with Xcode 15 on macos) reverted commit: - aa7ba58804b6d514ad94b1654dd0b89970d05ab8 --- depends/packages/boost.mk | 6 ------ 1 file changed, 6 deletions(-) diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index d0e3498269..09d12c210e 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -21,12 +21,8 @@ $(package)_config_opts_i686_android=address-model=32 $(package)_config_opts_aarch64_android=address-model=64 $(package)_config_opts_x86_64_android=address-model=64 $(package)_config_opts_armv7a_android=address-model=32 -unary_function=unary_function ifneq (,$(findstring clang,$($(package)_cxx))) $(package)_toolset_$(host_os)=clang -ifeq ($(build_os),darwin) -unary_function=__unary_function -endif else $(package)_toolset_$(host_os)=gcc endif @@ -39,9 +35,7 @@ $(package)_cxxflags_android=-fPIC $(package)_cxxflags_x86_64=-fcf-protection=full endef -# Fix missing unary_function in clang15 on macos, can be removed after upgrading to 1.81 define $(package)_preprocess_cmds - sed -i.old "s/unary_function/$(unary_function)/" boost/container_hash/hash.hpp && \ echo "using $($(package)_toolset_$(host_os)) : : $($(package)_cxx) : \"$($(package)_cflags)\" \"$($(package)_cxxflags)\" \"$($(package)_cppflags)\" \"$($(package)_ldflags)\" \"$($(package)_ar)\" \"$(host_STRIP)\" \"$(host_RANLIB)\" \"$(host_WINDRES)\" : ;" > user-config.jam endef From bbc99571f36ecfd817dc33b883c5e6f120240270 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 11 Jul 2024 15:31:08 +0000 Subject: [PATCH 04/21] fix: sidestep c++17 std::unary_function removal by compiling boost with c++11 we can revert this after upgrading to boost 1.81 --- depends/packages/boost.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index 09d12c210e..77ada65e0f 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -27,7 +27,7 @@ else $(package)_toolset_$(host_os)=gcc endif $(package)_config_libraries=filesystem,test -$(package)_cxxflags=-std=c++17 +$(package)_cxxflags=-std=c++11 $(package)_cxxflags_linux=-fPIC $(package)_cxxflags_freebsd=-fPIC $(package)_cxxflags_openbsd=-fPIC From 37bd4009c1850017bd7a53345101217ebf654509 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 11 Jul 2024 22:20:12 +0700 Subject: [PATCH 05/21] refactor: use monostate instead std::optional in CoreContext --- src/context.h | 4 ++-- src/node/interfaces.cpp | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/context.h b/src/context.h index 11b25b35b7..70fc00cd95 100644 --- a/src/context.h +++ b/src/context.h @@ -5,8 +5,8 @@ #ifndef BITCOIN_CONTEXT_H #define BITCOIN_CONTEXT_H +#include #include -#include class ChainstateManager; class CTxMemPool; @@ -15,7 +15,7 @@ struct LLMQContext; struct NodeContext; struct WalletContext; -using CoreContext = std::variant, std::reference_wrapper, std::reference_wrapper, diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index be4a3de421..f74983b871 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -67,6 +67,7 @@ #include #include #include +#include using interfaces::BlockTip; using interfaces::Chain; @@ -584,11 +585,11 @@ class NodeImpl : public Node if (context) { m_context_ref = *context; } else { - m_context_ref = std::nullopt; + m_context_ref = std::monostate{}; } } NodeContext* m_context{nullptr}; - CoreContext m_context_ref{std::nullopt}; + CoreContext m_context_ref; }; bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock& lock, const CChain& active) From d3e181f516767b79c31240f7eb06bd36fef608b5 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 12 Jul 2024 03:20:15 +0700 Subject: [PATCH 06/21] fix: add missing client's argument parsing for RPC commands --- src/rpc/client.cpp | 4 ++++ src/rpc/misc.cpp | 2 +- src/rpc/quorums.cpp | 6 +++--- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 863fbfd6d8..4adc8953b7 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -225,6 +225,10 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getnodeaddresses", 0, "count"}, { "addpeeraddress", 1, "port"}, { "stop", 0, "wait" }, + { "verifychainlock", 2, "blockHeight" }, + { "verifyislock", 3, "maxHeight" }, + { "submitchainlock", 2, "blockHeight" }, + { "mnauth", 0, "nodeId" }, }; // clang-format on diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 377acc2679..479bb3227c 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -617,7 +617,7 @@ static RPCHelpMan mnauth() if (!Params().MineBlocksOnDemand()) throw std::runtime_error("mnauth for regression testing (-regtest mode) only"); - int nodeId = ParseInt64V(request.params[0], "nodeId"); + int64_t nodeId = request.params[0].get_int64(); uint256 proTxHash = ParseHashV(request.params[1], "proTxHash"); if (proTxHash.IsNull()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "proTxHash invalid"); diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index ee6c990391..c0494a2d7a 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -974,7 +974,7 @@ static RPCHelpMan verifychainlock() } nBlockHeight = pIndex->nHeight; } else { - nBlockHeight = ParseInt32V(request.params[2], "blockHeight"); + nBlockHeight = request.params[2].get_int(); LOCK(cs_main); if (nBlockHeight < 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Block height out of range"); @@ -1040,7 +1040,7 @@ static RPCHelpMan verifyislock() int maxHeight{-1}; if (!request.params[3].isNull()) { - maxHeight = ParseInt32V(request.params[3], "maxHeight"); + maxHeight = request.params[3].get_int(); } int signHeight; @@ -1094,7 +1094,7 @@ static RPCHelpMan submitchainlock() { const uint256 nBlockHash(ParseHashV(request.params[0], "blockHash")); - const int nBlockHeight = ParseInt32V(request.params[2], "blockHeight"); + const int nBlockHeight = request.params[2].get_int(); if (nBlockHeight <= 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid block height"); } From 68c5da41dc4fb4e3fa3bdbbf89418d223af875cc Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 12 Jul 2024 12:22:23 +0700 Subject: [PATCH 07/21] feat: fix help message - all subcommands support it now! --- src/rpc/server.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index a47772a689..194fa9c094 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -167,7 +167,7 @@ static RPCHelpMan help() "\nList all commands, or get help for a specified command.\n", { {"command", RPCArg::Type::STR, /* default */ "all commands", "The command to get help on"}, - {"subcommand", RPCArg::Type::STR, /* default */ "all subcommands", "The subcommand to get help on. Please note that not all subcommands support this at the moment"}, + {"subcommand", RPCArg::Type::STR, /* default */ "all subcommands", "The subcommand to get help on."}, }, RPCResult{ RPCResult::Type::STR, "", "The help text" From a7e538d7ae6bdc37d719ee8b04685983a063c3ca Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 12 Jul 2024 13:38:09 +0700 Subject: [PATCH 08/21] fix: missing changes from bitcoin#19250 --- src/wallet/rpcwallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index f9b29f28e5..4a70b3cfbc 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -110,7 +110,7 @@ std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& reques } std::vector> wallets = GetWallets(); - if (wallets.size() == 1 || (request.fHelp && wallets.size() > 0)) { + if (wallets.size() == 1) { return wallets[0]; } From 1d87ce4e86bd359b5867d9c7cccb64ad93133a78 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 19 Nov 2020 14:18:56 +0100 Subject: [PATCH 09/21] Merge #18531: rpc: remove deprecated CRPCCommand constructor faaf9c58e4aa809019d4ca12747dd47411988e37 remove CRPCCommand constructor that takes rpcfn_type function pointer (MarcoFalke) fa19bb2cd8c575593583138a84e6bb3444d6196d remove dead rpc code (MarcoFalke) Pull request description: Remove the CRPCCommand arguments, now that they are asserted to be equal and thus redundant ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue #18607 * The changes itself fixed bug #19250 ACKs for top commit: fjahr: tested ACK faaf9c58e4aa809019d4ca12747dd47411988e37 promag: Tested ACK faaf9c58e4aa809019d4ca12747dd47411988e37. ryanofsky: Code review ACK faaf9c58e4aa809019d4ca12747dd47411988e37. Two obviously good simplifications. Tree-SHA512: 5de3b440f7b2ed2c3e86655d4f0e2e5df9c67e8ce3c7817d5ea5311d1a38690f2f3e28fab41aad6936be9fc884326d037e5f19e85d4d2fe281474dada13911ee --- src/qt/test/rpcnestedtests.cpp | 25 +++++++++++++++++-------- src/rpc/mining.cpp | 4 ---- src/rpc/misc.cpp | 2 -- src/rpc/net.cpp | 6 +++++- src/rpc/server.h | 9 --------- test/lint/lint-rpc-help.sh | 24 ------------------------ 6 files changed, 22 insertions(+), 48 deletions(-) delete mode 100755 test/lint/lint-rpc-help.sh diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index 3a0541b167..65780c2996 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -16,17 +16,26 @@ #include #include -static UniValue rpcNestedTest_rpc(const JSONRPCRequest& request) +static RPCHelpMan rpcNestedTest_rpc() { - if (request.fHelp) { - return "help message"; - } - return request.params.write(0, 0); + return RPCHelpMan{ + "rpcNestedTest", + "echo the passed string(s)", + { + {"arg1", RPCArg::Type::STR, RPCArg::Optional::OMITTED, ""}, + {"arg2", RPCArg::Type::STR, RPCArg::Optional::OMITTED, ""}, + {"arg3", RPCArg::Type::STR, RPCArg::Optional::OMITTED, ""}, + }, + {}, + RPCExamples{""}, + [](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + return request.params.write(0, 0); + }, + }; } -static const CRPCCommand vRPCCommands[] = -{ - { "test", "rpcNestedTest", &rpcNestedTest_rpc, {} }, +static const CRPCCommand vRPCCommands[] = { + {"test", "rpcNestedTest", &rpcNestedTest_rpc, {"arg1", "arg2", "arg3"}}, }; void RPCNestedTests::rpcNestedTests() diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 9ddf9f6b7e..dd83686da7 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -434,11 +434,7 @@ static RPCHelpMan generate() { return RPCHelpMan{"generate", "has been replaced by the -generate cli option. Refer to -help for more information.", {}, {}, RPCExamples{""}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - if (request.fHelp) { - throw std::runtime_error(self.ToString()); - } else { throw JSONRPCError(RPC_METHOD_NOT_FOUND, self.ToString()); - } }}; } diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 479bb3227c..8ceedf0da2 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -1386,8 +1386,6 @@ static RPCHelpMan echo(const std::string& name) RPCExamples{""}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - if (request.fHelp) throw std::runtime_error(self.ToString()); - if (request.params[9].isStr()) { CHECK_NONFATAL(request.params[9].get_str() != "trigger_internal_bug"); } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index c300fb5ee8..acbb5230e4 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -310,6 +310,10 @@ static RPCHelpMan addnode() std::string strCommand; if (!request.params[1].isNull()) strCommand = request.params[1].get_str(); + if (strCommand != "onetry" && strCommand != "add" && strCommand != "remove") { + throw std::runtime_error( + self.ToString()); + } const NodeContext& node = EnsureAnyNodeContext(request.context); CConnman& connman = EnsureConnman(node); @@ -715,7 +719,7 @@ static RPCHelpMan setban() std::string strCommand; if (!request.params[1].isNull()) strCommand = request.params[1].get_str(); - if (request.fHelp || !help.IsValidNumArgs(request.params.size()) || (strCommand != "add" && strCommand != "remove")) { + if (strCommand != "add" && strCommand != "remove") { throw std::runtime_error(help.ToString()); } const NodeContext& node = EnsureAnyNodeContext(request.context); diff --git a/src/rpc/server.h b/src/rpc/server.h index e83cd9bdf0..56744fd47f 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -83,7 +83,6 @@ void RPCUnsetTimerInterface(RPCTimerInterface *iface); */ void RPCRunLater(const std::string& name, std::function func, int64_t nSeconds); -typedef UniValue(*rpcfn_type)(const JSONRPCRequest& jsonRequest); typedef RPCHelpMan (*RpcMethodFnType)(); class CRPCCommand @@ -134,14 +133,6 @@ class CRPCCommand CHECK_NONFATAL(fn().GetArgNames() == args_in); } - //! Simplified constructor taking plain rpcfn_type function pointer. - CRPCCommand(const char* category, const char* name, rpcfn_type fn, std::initializer_list args) - : CRPCCommand(category, name, "", - [fn](const JSONRPCRequest& request, UniValue& result, bool) { result = fn(request); return true; }, - {args.begin(), args.end()}, intptr_t(fn)) - { - } - std::string category; std::string name; std::string subname; diff --git a/test/lint/lint-rpc-help.sh b/test/lint/lint-rpc-help.sh deleted file mode 100755 index 8e494a8d2e..0000000000 --- a/test/lint/lint-rpc-help.sh +++ /dev/null @@ -1,24 +0,0 @@ -#!/usr/bin/env bash -# -# Copyright (c) 2018 The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. -# -# Check that all RPC help texts are generated by RPCHelpMan. - -export LC_ALL=C - -EXIT_CODE=0 - -# Assume that all multiline strings passed into a runtime_error are help texts. -# This is potentially fragile, but the linter is only temporary and can safely -# be removed early 2019. - -non_autogenerated_help=$(grep -A1 'runtime_error($' $(git ls-files -- "*.cpp") | grep '".*\\n"$') -if [[ ${non_autogenerated_help} != "" ]]; then - echo "Must use RPCHelpMan to generate the help for the following RPC methods:" - echo "${non_autogenerated_help}" - echo - EXIT_CODE=1 -fi -exit ${EXIT_CODE} From d55759fa791a72ad15340ef2c4825652a630d2c6 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 28 Jan 2021 19:25:07 +0100 Subject: [PATCH 10/21] Merge #20012: rpc: Remove duplicate name and argNames from CRPCCommand fa04f9b4ddffc5ef23c2ee7f3cc72a7c2ae49204 rpc: Remove duplicate name and argNames from CRPCCommand (MarcoFalke) fa92912b4bb4629addcbfdfb7cc000be701614af rpc: Use RPCHelpMan for check-rpc-mappings linter (MarcoFalke) faf835680be39811827504f77005b6603165f53e rpc: [refactor] Use concise C++11 code in CRPCConvertTable constructor (MarcoFalke) Pull request description: Currently, the RPC argument names are specified twice to simplify consistency linting. To avoid having to specify the argnames twice when adding new arguments, remove the linter and add an equivalent test based on RPCHelpMan. ACKs for top commit: laanwj: ACK fa04f9b4ddffc5ef23c2ee7f3cc72a7c2ae49204 Tree-SHA512: 3f5f32f5a09b22d879f24aa67031639d2612cff481d6aebc6cfe6fd757cafb3e7bf72120b30466f59292a260747b71e57322c189d5478b668519b9f32fcde31a --- ci/dash/build_src.sh | 2 - ci/lint/06_script.sh | 1 - src/qt/test/rpcnestedtests.cpp | 2 +- src/rpc/blockchain.cpp | 78 +++++++------- src/rpc/client.cpp | 11 +- src/rpc/coinjoin.cpp | 16 +-- src/rpc/evo.cpp | 58 +++++------ src/rpc/governance.cpp | 37 ++++--- src/rpc/masternode.cpp | 26 ++--- src/rpc/mining.cpp | 54 +++++----- src/rpc/misc.cpp | 52 +++++----- src/rpc/net.cpp | 38 +++---- src/rpc/quorums.cpp | 44 ++++---- src/rpc/rawtransaction.cpp | 48 ++++----- src/rpc/server.cpp | 34 +++++-- src/rpc/server.h | 9 +- src/rpc/util.cpp | 17 ++++ src/rpc/util.h | 2 + src/wallet/rpcwallet.cpp | 140 +++++++++++++------------- src/zmq/zmqrpc.cpp | 6 +- test/functional/rpc_help.py | 64 ++++++++++++ test/lint/check-rpc-mappings.py | 173 -------------------------------- 22 files changed, 421 insertions(+), 491 deletions(-) delete mode 100755 test/lint/check-rpc-mappings.py diff --git a/ci/dash/build_src.sh b/ci/dash/build_src.sh index 3a2f108909..8cf4116c7c 100755 --- a/ci/dash/build_src.sh +++ b/ci/dash/build_src.sh @@ -24,8 +24,6 @@ if [ "$CHECK_DOC" = 1 ]; then #test/lint/git-subtree-check.sh src/leveldb # TODO: Check docs (re-enable after all Bitcoin PRs have been merged and docs fully fixed) #test/lint/check-doc.py - # Check rpc consistency - test/lint/check-rpc-mappings.py . # Run all linters test/lint/lint-all.sh test/lint/extended-lint-all.sh diff --git a/ci/lint/06_script.sh b/ci/lint/06_script.sh index 2b7a0751c0..1c3f589883 100755 --- a/ci/lint/06_script.sh +++ b/ci/lint/06_script.sh @@ -20,7 +20,6 @@ test/lint/git-subtree-check.sh src/secp256k1 test/lint/git-subtree-check.sh src/univalue test/lint/git-subtree-check.sh src/leveldb test/lint/check-doc.py -test/lint/check-rpc-mappings.py . test/lint/lint-all.sh if [ "$CIRRUS_REPO_FULL_NAME" = "dashpay/dash" ] && [ -n "$CIRRUS_CRON" ]; then diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index 65780c2996..36f4ffc136 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -35,7 +35,7 @@ static RPCHelpMan rpcNestedTest_rpc() } static const CRPCCommand vRPCCommands[] = { - {"test", "rpcNestedTest", &rpcNestedTest_rpc, {"arg1", "arg2", "arg3"}}, + {"test", &rpcNestedTest_rpc}, }; void RPCNestedTests::rpcNestedTests() diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 524736d560..8b4e7b40f5 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -3038,47 +3038,47 @@ void RegisterBlockchainRPCCommands(CRPCTable &t) { // clang-format off static const CRPCCommand commands[] = -{ // category name actor (function) argNames - // --------------------- ------------------------ ----------------------- ---------- - { "blockchain", "getblockchaininfo", &getblockchaininfo, {} }, - { "blockchain", "getchaintxstats", &getchaintxstats, {"nblocks", "blockhash"} }, - { "blockchain", "getblockstats", &getblockstats, {"hash_or_height", "stats"} }, - { "blockchain", "getbestblockhash", &getbestblockhash, {} }, - { "blockchain", "getbestchainlock", &getbestchainlock, {} }, - { "blockchain", "getblockcount", &getblockcount, {} }, - { "blockchain", "getblock", &getblock, {"blockhash","verbosity|verbose"} }, - { "blockchain", "getblockfrompeer", &getblockfrompeer, {"block_hash", "peer_id"}}, - { "blockchain", "getblockhashes", &getblockhashes, {"high","low"} }, - { "blockchain", "getblockhash", &getblockhash, {"height"} }, - { "blockchain", "getblockheader", &getblockheader, {"blockhash","verbose"} }, - { "blockchain", "getblockheaders", &getblockheaders, {"blockhash","count","verbose"} }, - { "blockchain", "getmerkleblocks", &getmerkleblocks, {"filter","blockhash","count"} }, - { "blockchain", "getchaintips", &getchaintips, {"count","branchlen"} }, - { "blockchain", "getdifficulty", &getdifficulty, {} }, - { "blockchain", "getmempoolancestors", &getmempoolancestors, {"txid","verbose"} }, - { "blockchain", "getmempooldescendants", &getmempooldescendants, {"txid","verbose"} }, - { "blockchain", "getmempoolentry", &getmempoolentry, {"txid"} }, - { "blockchain", "getmempoolinfo", &getmempoolinfo, {} }, - { "blockchain", "getrawmempool", &getrawmempool, {"verbose"} }, - { "blockchain", "getspecialtxes", &getspecialtxes, {"blockhash", "type", "count", "skip", "verbosity"} }, - { "blockchain", "gettxout", &gettxout, {"txid","n","include_mempool"} }, - { "blockchain", "gettxoutsetinfo", &gettxoutsetinfo, {"hash_type", "hash_or_height", "use_index"} }, - { "blockchain", "pruneblockchain", &pruneblockchain, {"height"} }, - { "blockchain", "savemempool", &savemempool, {} }, - { "blockchain", "verifychain", &verifychain, {"checklevel","nblocks"} }, - - { "blockchain", "preciousblock", &preciousblock, {"blockhash"} }, - { "blockchain", "scantxoutset", &scantxoutset, {"action", "scanobjects"} }, - { "blockchain", "getblockfilter", &getblockfilter, {"blockhash", "filtertype"} }, +{ // category actor (function) + // --------------------- ------------------------ + { "blockchain", &getblockchaininfo, }, + { "blockchain", &getchaintxstats, }, + { "blockchain", &getblockstats, }, + { "blockchain", &getbestblockhash, }, + { "blockchain", &getbestchainlock, }, + { "blockchain", &getblockcount, }, + { "blockchain", &getblock, }, + { "blockchain", &getblockfrompeer, }, + { "blockchain", &getblockhashes, }, + { "blockchain", &getblockhash, }, + { "blockchain", &getblockheader, }, + { "blockchain", &getblockheaders, }, + { "blockchain", &getmerkleblocks, }, + { "blockchain", &getchaintips, }, + { "blockchain", &getdifficulty, }, + { "blockchain", &getmempoolancestors, }, + { "blockchain", &getmempooldescendants, }, + { "blockchain", &getmempoolentry, }, + { "blockchain", &getmempoolinfo, }, + { "blockchain", &getrawmempool, }, + { "blockchain", &getspecialtxes, }, + { "blockchain", &gettxout, }, + { "blockchain", &gettxoutsetinfo, }, + { "blockchain", &pruneblockchain, }, + { "blockchain", &savemempool, }, + { "blockchain", &verifychain, }, + + { "blockchain", &preciousblock, }, + { "blockchain", &scantxoutset, }, + { "blockchain", &getblockfilter, }, /* Not shown in help */ - { "hidden", "invalidateblock", &invalidateblock, {"blockhash"} }, - { "hidden", "reconsiderblock", &reconsiderblock, {"blockhash"} }, - { "hidden", "waitfornewblock", &waitfornewblock, {"timeout"} }, - { "hidden", "waitforblock", &waitforblock, {"blockhash","timeout"} }, - { "hidden", "waitforblockheight", &waitforblockheight, {"height","timeout"} }, - { "hidden", "syncwithvalidationinterfacequeue", &syncwithvalidationinterfacequeue, {} }, - { "hidden", "dumptxoutset", &dumptxoutset, {"path"} }, + { "hidden", &invalidateblock, }, + { "hidden", &reconsiderblock, }, + { "hidden", &waitfornewblock, }, + { "hidden", &waitforblock, }, + { "hidden", &waitforblockheight, }, + { "hidden", &syncwithvalidationinterfacequeue, }, + { "hidden", &dumptxoutset, }, }; // clang-format on for (const auto& c : commands) { diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 4adc8953b7..b7b2e79604 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -251,14 +251,9 @@ class CRPCConvertTable CRPCConvertTable::CRPCConvertTable() { - const unsigned int n_elem = - (sizeof(vRPCConvertParams) / sizeof(vRPCConvertParams[0])); - - for (unsigned int i = 0; i < n_elem; i++) { - members.insert(std::make_pair(vRPCConvertParams[i].methodName, - vRPCConvertParams[i].paramIdx)); - membersByName.insert(std::make_pair(vRPCConvertParams[i].methodName, - vRPCConvertParams[i].paramName)); + for (const auto& cp : vRPCConvertParams) { + members.emplace(cp.methodName, cp.paramIdx); + membersByName.emplace(cp.methodName, cp.paramName); } } diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp index 29cf8fbb41..45f4e52f93 100644 --- a/src/rpc/coinjoin.cpp +++ b/src/rpc/coinjoin.cpp @@ -281,15 +281,15 @@ void RegisterCoinJoinRPCCommands(CRPCTable &t) { // clang-format off static const CRPCCommand commands[] = - { // category name actor (function) argNames - // ------------------------------------------------------------------------------------------------------ - { "dash", "getpoolinfo", &getpoolinfo, {} }, - { "dash", "getcoinjoininfo", &getcoinjoininfo, {} }, +{ // category actor (function) + // --------------------- ----------------------- + { "dash", &getpoolinfo, }, + { "dash", &getcoinjoininfo, }, #ifdef ENABLE_WALLET - { "dash", "coinjoin", &coinjoin, {"command"} }, - { "dash", "coinjoin", "reset", &coinjoin_reset, {} }, - { "dash", "coinjoin", "start", &coinjoin_start, {} }, - { "dash", "coinjoin", "stop", &coinjoin_stop, {} }, + { "dash", &coinjoin, }, + { "dash", &coinjoin_reset, }, + { "dash", &coinjoin_start, }, + { "dash", &coinjoin_stop, }, #endif // ENABLE_WALLET }; // clang-format on diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 66f435dafd..a0e2ffdb97 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -1869,37 +1869,37 @@ void RegisterEvoRPCCommands(CRPCTable &tableRPC) { // clang-format off static const CRPCCommand commands[] = -{ // category name actor (function) - // --------------------- ------------------------ ----------------------- - { "evo", "bls", &bls_help, {"command"} }, - { "evo", "bls", "generate", &bls_generate, {"legacy"} }, - { "evo", "bls", "fromsecret", &bls_fromsecret, {"secret", "legacy"} }, - { "evo", "protx", &protx_help, {"command"} }, +{ // category actor (function) + // --------------------- ----------------------- + { "evo", &bls_help, }, + { "evo", &bls_generate, }, + { "evo", &bls_fromsecret, }, + { "evo", &protx_help, }, #ifdef ENABLE_WALLET - { "evo", "protx", "register", &protx_register, {"collateralHash", "collateralIndex", "ipAndPort", "ownerAddress", "operatorPubKey", "votingAddress", "operatorReward", "payoutAddress", "feeSourceAddress", "submit"} }, - { "evo", "protx", "register_evo", &protx_register_evo, {"collateralHash", "collateralIndex", "ipAndPort", "ownerAddress", "operatorPubKey", "votingAddress", "operatorReward", "payoutAddress", "platformNodeID", "platformP2PPort", "platformHTTPPort", "feeSourceAddress", "submit"} }, - { "evo", "protx", "register_hpmn", &protx_register_hpmn, {"collateralHash", "collateralIndex", "ipAndPort", "ownerAddress", "operatorPubKey", "votingAddress", "operatorReward", "payoutAddress", "platformNodeID", "platformP2PPort", "platformHTTPPort", "feeSourceAddress", "submit"} }, - { "evo", "protx", "register_legacy", &protx_register_legacy, {"collateralHash", "collateralIndex", "ipAndPort", "ownerAddress", "operatorPubKey", "votingAddress", "operatorReward", "payoutAddress", "feeSourceAddress", "submit"} }, - { "evo", "protx", "register_fund", &protx_register_fund, {"collateralAddress", "ipAndPort", "ownerAddress", "operatorPubKey", "votingAddress", "operatorReward", "payoutAddress", "fundAddress", "submit"} }, - { "evo", "protx", "register_fund_legacy", &protx_register_fund_legacy, {"collateralAddress", "ipAndPort", "ownerAddress", "operatorPubKey", "votingAddress", "operatorReward", "payoutAddress", "fundAddress", "submit"} }, - { "evo", "protx", "register_fund_evo", &protx_register_fund_evo, {"collateralAddress", "ipAndPort", "ownerAddress", "operatorPubKey", "votingAddress", "operatorReward", "payoutAddress", "platformNodeID", "platformP2PPort", "platformHTTPPort", "fundAddress", "submit"} }, - { "evo", "protx", "register_fund_hpmn", &protx_register_fund_hpmn, {"collateralAddress", "ipAndPort", "ownerAddress", "operatorPubKey", "votingAddress", "operatorReward", "payoutAddress", "platformNodeID", "platformP2PPort", "platformHTTPPort", "fundAddress", "submit"} }, - { "evo", "protx", "register_prepare", &protx_register_prepare, {"collateralHash", "collateralIndex", "ipAndPort", "ownerAddress", "operatorPubKey", "votingAddress", "operatorReward", "payoutAddress", "feeSourceAddress"} }, - { "evo", "protx", "register_prepare_evo", &protx_register_prepare_evo, {"collateralHash", "collateralIndex", "ipAndPort", "ownerAddress", "operatorPubKey", "votingAddress", "operatorReward", "payoutAddress", "platformNodeID", "platformP2PPort", "platformHTTPPort", "feeSourceAddress"} }, - { "evo", "protx", "register_prepare_hpmn", &protx_register_prepare_hpmn, {"collateralHash", "collateralIndex", "ipAndPort", "ownerAddress", "operatorPubKey", "votingAddress", "operatorReward", "payoutAddress", "platformNodeID", "platformP2PPort", "platformHTTPPort", "feeSourceAddress"} }, - { "evo", "protx", "register_prepare_legacy", &protx_register_prepare_legacy, {"collateralHash", "collateralIndex", "ipAndPort", "ownerAddress", "operatorPubKey", "votingAddress", "operatorReward", "payoutAddress", "feeSourceAddress"} }, - { "evo", "protx", "update_service", &protx_update_service, {"proTxHash", "ipAndPort", "operatorKey", "operatorPayoutAddress", "feeSourceAddress"} }, - { "evo", "protx", "update_service_evo", &protx_update_service_evo, {"proTxHash", "ipAndPort", "operatorKey", "platformNodeID", "platformP2PPort", "platformHTTPPort", "operatorPayoutAddress", "feeSourceAddress"} }, - { "evo", "protx", "update_service_hpmn", &protx_update_service_hpmn, {"proTxHash", "ipAndPort", "operatorKey", "platformNodeID", "platformP2PPort", "platformHTTPPort", "operatorPayoutAddress", "feeSourceAddress"} }, - { "evo", "protx", "register_submit", &protx_register_submit, {"tx", "sig"} }, - { "evo", "protx", "update_registrar", &protx_update_registrar, {"proTxHash", "operatorPubKey", "votingAddress", "payoutAddress", "feeSourceAddress"} }, - { "evo", "protx", "update_registrar_legacy", &protx_update_registrar_legacy, {"proTxHash", "operatorPubKey", "votingAddress", "payoutAddress", "feeSourceAddress"} }, - { "evo", "protx", "revoke", &protx_revoke, {"proTxHash", "operatorKey", "reason", "feeSourceAddress"} }, + { "evo", &protx_register, }, + { "evo", &protx_register_evo, }, + { "evo", &protx_register_hpmn, }, + { "evo", &protx_register_legacy, }, + { "evo", &protx_register_fund, }, + { "evo", &protx_register_fund_legacy, }, + { "evo", &protx_register_fund_evo, }, + { "evo", &protx_register_fund_hpmn, }, + { "evo", &protx_register_prepare, }, + { "evo", &protx_register_prepare_evo, }, + { "evo", &protx_register_prepare_hpmn, }, + { "evo", &protx_register_prepare_legacy, }, + { "evo", &protx_update_service, }, + { "evo", &protx_update_service_evo, }, + { "evo", &protx_update_service_hpmn, }, + { "evo", &protx_register_submit, }, + { "evo", &protx_update_registrar, }, + { "evo", &protx_update_registrar_legacy, }, + { "evo", &protx_revoke, }, #endif - { "evo", "protx", "list", &protx_list, {"type", "detailed", "height"} }, - { "evo", "protx", "info", &protx_info, {"proTxHash", "blockHash"} }, - { "evo", "protx", "diff", &protx_diff, {"baseBlock", "block", "extended"} }, - { "evo", "protx", "listdiff", &protx_listdiff, {"baseBlock", "block"} }, + { "evo", &protx_list, }, + { "evo", &protx_info, }, + { "evo", &protx_diff, }, + { "evo", &protx_listdiff, }, }; // clang-format on for (const auto& command : commands) { diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 4b1af3dc3d..8c92e8c08b 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -1057,28 +1057,27 @@ void RegisterGovernanceRPCCommands(CRPCTable &t) { // clang-format off static const CRPCCommand commands[] = -{ // category name actor (function) argNames - // --------------------- ------------------------ ----------------------- ---------- +{ // category actor (function) + // --------------------- ----------------------- /* Dash features */ - { "dash", "getgovernanceinfo", &getgovernanceinfo, {} }, - { "dash", "getsuperblockbudget", &getsuperblockbudget, {"index"} }, - { "dash", "gobject", &gobject, {"command"} }, - { "dash", "gobject", "count", &gobject_count, {"mode"} }, - { "dash", "gobject", "deserialize", &gobject_deserialize, {"hex_data"} }, - { "dash", "gobject", "check", &gobject_check, {"hex_data"} }, + { "dash", &getgovernanceinfo, }, + { "dash", &getsuperblockbudget, }, + { "dash", &gobject, }, + { "dash", &gobject_count, }, + { "dash", &gobject_deserialize, }, + { "dash", &gobject_check, }, #ifdef ENABLE_WALLET - { "dash", "gobject", "prepare", &gobject_prepare, {"parent-hash", "revision", "time", "data-hex", "use-IS", "outputHash", "outputIndex"} }, - { "dash", "gobject", "list-prepared", &gobject_list_prepared, {"count"} }, - { "dash", "gobject", "vote-many", &gobject_vote_many, {"governance-hash", "vote", "vote-outcome"} }, - { "dash", "gobject", "vote-alias", &gobject_vote_alias, {"governance-hash", "vote", "vote-outcome", "protx-hash"} }, + { "dash", &gobject_prepare, }, + { "dash", &gobject_list_prepared, }, + { "dash", &gobject_vote_many, }, + { "dash", &gobject_vote_alias, }, #endif - { "dash", "gobject", "submit", &gobject_submit, {"parent-hash", "revision", "time", "data-hex", "fee-txid"} }, - { "dash", "gobject", "list", &gobject_list, {"signal", "type"} }, - { "dash", "gobject", "diff", &gobject_diff, {"signal", "type"} }, - { "dash", "gobject", "get", &gobject_get, {"governance-hash"} }, - { "dash", "gobject", "getcurrentvotes", &gobject_getcurrentvotes, {"governance-hash", "txid", "vout"} }, - { "dash", "voteraw", &voteraw, {"mn-collateral-tx-hash","mn-collateral-tx-index","governance-hash","vote-signal","vote-outcome","time","vote-sig"} }, - + { "dash", &gobject_submit, }, + { "dash", &gobject_list, }, + { "dash", &gobject_diff, }, + { "dash", &gobject_get, }, + { "dash", &gobject_getcurrentvotes, }, + { "dash", &voteraw, }, }; // clang-format on for (const auto& command : commands) { diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index 923cdc0993..bc2550ea0c 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -722,21 +722,21 @@ void RegisterMasternodeRPCCommands(CRPCTable &t) { // clang-format off static const CRPCCommand commands[] = -{ // category name actor (function) argNames - // --------------------- ------------------------ ----------------------- ---------- - { "dash", "masternode", &masternode_help, {"command"} }, - { "dash", "masternode", "list", &masternodelist_composite, {"mode", "filter"} }, - { "dash", "masternodelist", &masternodelist, {"mode", "filter"} }, - { "dash", "masternode", "connect", &masternode_connect, {"address"} }, - { "dash", "masternode", "count", &masternode_count, {} }, +{ // category actor (function) + // --------------------- ----------------------- + { "dash", &masternode_help, }, + { "dash", &masternodelist_composite, }, + { "dash", &masternodelist, }, + { "dash", &masternode_connect, }, + { "dash", &masternode_count, }, #ifdef ENABLE_WALLET - { "dash", "masternode", "outputs", &masternode_outputs, {} }, + { "dash", &masternode_outputs, }, #endif // ENABLE_WALLET - { "dash", "masternode", "status", &masternode_status, {} }, - { "dash", "masternode", "payments", &masternode_payments, {"blockhash", "count"} }, - { "dash", "masternode", "winners", &masternode_winners, {"count", "filter"} }, - { "dash", "masternode", "current", &masternode_current, {} }, - { "dash", "masternode", "winner", &masternode_winner, {} }, + { "dash", &masternode_status, }, + { "dash", &masternode_payments, }, + { "dash", &masternode_winners, }, + { "dash", &masternode_current, }, + { "dash", &masternode_winner, }, }; // clang-format on for (const auto& command : commands) { diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index dd83686da7..9e75dabfe3 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -416,17 +416,24 @@ static RPCHelpMan generateblock() }; } #else -static UniValue generatetoaddress(const JSONRPCRequest& request) +static RPCHelpMan generatetoaddress() { - throw JSONRPCError(RPC_METHOD_NOT_FOUND, "This call is not available because RPC miner isn't compiled"); + return RPCHelpMan{"generatetoaddress", "This call is not available because RPC miner isn't compiled", {}, {}, RPCExamples{""}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + throw JSONRPCError(RPC_METHOD_NOT_FOUND, "This call is not available because RPC miner isn't compiled"); + }}; } -static UniValue generatetodescriptor(const JSONRPCRequest& request) + +static RPCHelpMan generatetodescriptor() { - throw JSONRPCError(RPC_METHOD_NOT_FOUND, "This call is not available because RPC miner isn't compiled"); + return RPCHelpMan{"generatetodescriptor", "This call is not available because RPC miner isn't compiled", {}, {}, RPCExamples{""}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + throw JSONRPCError(RPC_METHOD_NOT_FOUND, "This call is not available because RPC miner isn't compiled"); + }}; } -static UniValue generateblock(const JSONRPCRequest& request) +static RPCHelpMan generateblock() { - throw JSONRPCError(RPC_METHOD_NOT_FOUND, "This call is not available because RPC miner isn't compiled"); + return RPCHelpMan{"generateblock", "This call is not available because RPC miner isn't compiled", {}, {}, RPCExamples{""}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + throw JSONRPCError(RPC_METHOD_NOT_FOUND, "This call is not available because RPC miner isn't compiled"); + }}; } #endif // ENABLE_MINER @@ -1295,29 +1302,30 @@ void RegisterMiningRPCCommands(CRPCTable &t) { // clang-format off static const CRPCCommand commands[] = -{ // category name actor (function) argNames - // --------------------- ------------------------ ----------------------- ---------- - { "mining", "getnetworkhashps", &getnetworkhashps, {"nblocks","height"} }, - { "mining", "getmininginfo", &getmininginfo, {} }, - { "mining", "prioritisetransaction", &prioritisetransaction, {"txid","fee_delta"} }, - { "mining", "getblocktemplate", &getblocktemplate, {"template_request"} }, - { "mining", "submitblock", &submitblock, {"hexdata","dummy"} }, - { "mining", "submitheader", &submitheader, {"hexdata"} }, +{ // category actor (function) + // --------------------- ----------------------- + { "mining", &getnetworkhashps, }, + { "mining", &getmininginfo, }, + { "mining", &prioritisetransaction, }, + { "mining", &getblocktemplate, }, + { "mining", &submitblock, }, + { "mining", &submitheader, }, #if ENABLE_MINER - { "generating", "generatetoaddress", &generatetoaddress, {"nblocks","address","maxtries"} }, - { "generating", "generatetodescriptor", &generatetodescriptor, {"num_blocks","descriptor","maxtries"} }, - { "generating", "generateblock", &generateblock, {"output","transactions"} }, + { "generating", &generatetoaddress, }, + { "generating", &generatetodescriptor, }, + { "generating", &generateblock, }, #else - { "hidden", "generatetoaddress", &generatetoaddress, {"nblocks","address","maxtries"} }, // Hidden as it isn't functional, just an error to let people know if miner isn't compiled - { "hidden", "generatetodescriptor", &generatetodescriptor, {"num_blocks","descriptor","maxtries"} }, - { "hidden", "generateblock", &generateblock, {"address","transactions"} }, +// Hidden as it isn't functional, just an error to let people know if miner isn't compiled + { "hidden", &generatetoaddress, }, + { "hidden", &generatetodescriptor, }, + { "hidden", &generateblock, }, #endif // ENABLE_MINER - { "util", "estimatesmartfee", &estimatesmartfee, {"conf_target", "estimate_mode"} }, + { "util", &estimatesmartfee, }, - { "hidden", "estimaterawfee", &estimaterawfee, {"conf_target", "threshold"} }, - { "hidden", "generate", &generate, {} }, + { "hidden", &estimaterawfee, }, + { "hidden", &generate, }, }; // clang-format on for (const auto& c : commands) { diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 8ceedf0da2..9706c94bda 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -1458,38 +1458,38 @@ static RPCHelpMan echojson() { return echo("echojson"); } void RegisterMiscRPCCommands(CRPCTable &t) { static const CRPCCommand commands[] = -{ // category name actor (function) argNames - // --------------------- ------------------------ ----------------------- ---------- - { "control", "debug", &debug, {"category"} }, - { "control", "getmemoryinfo", &getmemoryinfo, {"mode"} }, - { "control", "logging", &logging, {"include", "exclude"}}, - { "util", "validateaddress", &validateaddress, {"address"} }, - { "util", "createmultisig", &createmultisig, {"nrequired","keys"} }, - { "util", "deriveaddresses", &deriveaddresses, {"descriptor", "range"} }, - { "util", "getdescriptorinfo", &getdescriptorinfo, {"descriptor"} }, - { "util", "verifymessage", &verifymessage, {"address","signature","message"} }, - { "util", "signmessagewithprivkey", &signmessagewithprivkey, {"privkey","message"} }, - { "util", "getindexinfo", &getindexinfo, {"index_name"} }, - { "blockchain", "getspentinfo", &getspentinfo, {"request"} }, +{ // category actor (function) + // --------------------- ------------------------ + { "control", &debug, }, + { "control", &getmemoryinfo, }, + { "control", &logging, }, + { "util", &validateaddress, }, + { "util", &createmultisig, }, + { "util", &deriveaddresses, }, + { "util", &getdescriptorinfo, }, + { "util", &verifymessage, }, + { "util", &signmessagewithprivkey, }, + { "util", &getindexinfo, }, + { "blockchain", &getspentinfo, }, /* Address index */ - { "addressindex", "getaddressmempool", &getaddressmempool, {"addresses"} }, - { "addressindex", "getaddressutxos", &getaddressutxos, {"addresses"} }, - { "addressindex", "getaddressdeltas", &getaddressdeltas, {"addresses"} }, - { "addressindex", "getaddresstxids", &getaddresstxids, {"addresses"} }, - { "addressindex", "getaddressbalance", &getaddressbalance, {"addresses"} }, + { "addressindex", &getaddressmempool, }, + { "addressindex", &getaddressutxos, }, + { "addressindex", &getaddressdeltas, }, + { "addressindex", &getaddresstxids, }, + { "addressindex", &getaddressbalance, }, /* Dash features */ - { "dash", "mnsync", &mnsync, {"mode"} }, - { "dash", "spork", &spork, {"command"} }, - { "dash", "sporkupdate", &sporkupdate, {"name","value"} }, + { "dash", &mnsync, }, + { "dash", &spork, }, + { "dash", &sporkupdate, }, /* Not shown in help */ - { "hidden", "setmocktime", &setmocktime, {"timestamp"}}, - { "hidden", "mockscheduler", &mockscheduler, {"delta_time"}}, - { "hidden", "echo", &echo, {"arg0","arg1","arg2","arg3","arg4","arg5","arg6","arg7","arg8","arg9"}}, - { "hidden", "echojson", &echojson, {"arg0","arg1","arg2","arg3","arg4","arg5","arg6","arg7","arg8","arg9"}}, - { "hidden", "mnauth", &mnauth, {"nodeId", "proTxHash", "publicKey"}}, + { "hidden", &setmocktime, }, + { "hidden", &mockscheduler, }, + { "hidden", &echo, }, + { "hidden", &echojson, }, + { "hidden", &mnauth, }, }; // clang-format on for (const auto& c : commands) { diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index acbb5230e4..8a33030510 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -1013,25 +1013,25 @@ void RegisterNetRPCCommands(CRPCTable &t) { // clang-format off static const CRPCCommand commands[] = -{ // category name actor (function) argNames - // --------------------- ------------------------ ----------------------- ---------- - { "network", "getconnectioncount", &getconnectioncount, {} }, - { "network", "ping", &ping, {} }, - { "network", "getpeerinfo", &getpeerinfo, {} }, - { "network", "addnode", &addnode, {"node","command"} }, - { "network", "disconnectnode", &disconnectnode, {"address", "nodeid"} }, - { "network", "getaddednodeinfo", &getaddednodeinfo, {"node"} }, - { "network", "getnettotals", &getnettotals, {} }, - { "network", "getnetworkinfo", &getnetworkinfo, {} }, - { "network", "setban", &setban, {"subnet", "command", "bantime", "absolute"} }, - { "network", "listbanned", &listbanned, {} }, - { "network", "clearbanned", &clearbanned, {} }, - { "network", "cleardiscouraged", &cleardiscouraged, {} }, - { "network", "setnetworkactive", &setnetworkactive, {"state"} }, - { "network", "getnodeaddresses", &getnodeaddresses, {"count", "network"} }, - - { "hidden", "addconnection", &addconnection, {"address", "connection_type"} }, - { "hidden", "addpeeraddress", &addpeeraddress, {"address", "port"} }, +{ // category actor + // --------------------- ----------------------- + { "network", &getconnectioncount, }, + { "network", &ping, }, + { "network", &getpeerinfo, }, + { "network", &addnode, }, + { "network", &disconnectnode, }, + { "network", &getaddednodeinfo, }, + { "network", &getnettotals, }, + { "network", &getnetworkinfo, }, + { "network", &setban, }, + { "network", &listbanned, }, + { "network", &clearbanned, }, + { "network", &cleardiscouraged, }, + { "network", &setnetworkactive, }, + { "network", &getnodeaddresses, }, + + { "hidden", &addconnection, }, + { "hidden", &addpeeraddress, }, }; // clang-format on for (const auto& c : commands) { diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index c0494a2d7a..311d90cf7b 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -1132,28 +1132,28 @@ void RegisterQuorumsRPCCommands(CRPCTable &tableRPC) { // clang-format off static const CRPCCommand commands[] = -{ // category name actor (function) - // --------------------- ------------------------ ----------------------- - { "evo", "quorum", &quorum_help, {"command"} }, - { "evo", "quorum", "list", &quorum_list, {"count"} }, - { "evo", "quorum", "listextended", &quorum_list_extended, {"height"} }, - { "evo", "quorum", "info", &quorum_info, {"llmqType", "quorumHash", "includeSkShare"} }, - { "evo", "quorum", "dkginfo", &quorum_dkginfo, {} }, - { "evo", "quorum", "dkgstatus", &quorum_dkgstatus, {"detail_level"} }, - { "evo", "quorum", "memberof", &quorum_memberof, {"proTxHash", "scanQuorumsCount"} }, - { "evo", "quorum", "sign", &quorum_sign, {"llmqType", "id", "msgHash", "quorumHash", "submit"} }, - { "evo", "quorum", "platformsign", &quorum_platformsign, {"id", "msgHash", "quorumHash", "submit"} }, - { "evo", "quorum", "verify", &quorum_verify, {"llmqType", "id", "msgHash", "signature", "quorumHash", "signHeight"} }, - { "evo", "quorum", "hasrecsig", &quorum_hasrecsig, {"llmqType", "id", "msgHash"} }, - { "evo", "quorum", "getrecsig", &quorum_getrecsig, {"llmqType", "id", "msgHash"} }, - { "evo", "quorum", "isconflicting",&quorum_isconflicting, {"llmqType", "id", "msgHash"} }, - { "evo", "quorum", "selectquorum", &quorum_selectquorum, {"llmqType", "id"} }, - { "evo", "quorum", "dkgsimerror", &quorum_dkgsimerror, {"type", "rate"} }, - { "evo", "quorum", "getdata", &quorum_getdata, {"nodeId", "llmqType", "quorumHash", "dataMask", "proTxHash"} }, - { "evo", "quorum", "rotationinfo", &quorum_rotationinfo, {"blockRequestHash", "extraShare", "baseBlockHash..."} }, - { "evo", "submitchainlock", &submitchainlock, {"blockHash", "signature", "blockHeight"} }, - { "evo", "verifychainlock", &verifychainlock, {"blockHash", "signature", "blockHeight"} }, - { "evo", "verifyislock", &verifyislock, {"id", "txid", "signature", "maxHeight"} }, +{ // category actor (function) + // --------------------- ----------------------- + { "evo", &quorum_help, }, + { "evo", &quorum_list, }, + { "evo", &quorum_list_extended, }, + { "evo", &quorum_info, }, + { "evo", &quorum_dkginfo, }, + { "evo", &quorum_dkgstatus, }, + { "evo", &quorum_memberof, }, + { "evo", &quorum_sign, }, + { "evo", &quorum_platformsign, }, + { "evo", &quorum_verify, }, + { "evo", &quorum_hasrecsig, }, + { "evo", &quorum_getrecsig, }, + { "evo", &quorum_isconflicting, }, + { "evo", &quorum_selectquorum, }, + { "evo", &quorum_dkgsimerror, }, + { "evo", &quorum_getdata, }, + { "evo", &quorum_rotationinfo, }, + { "evo", &submitchainlock, }, + { "evo", &verifychainlock, }, + { "evo", &verifyislock, }, }; // clang-format on for (const auto& command : commands) { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index da0010acc6..6c7111ccd5 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -2079,30 +2079,30 @@ void RegisterRawTransactionRPCCommands(CRPCTable &t) { // clang-format off static const CRPCCommand commands[] = -{ // category name actor (function) argNames - // --------------------- ------------------------ ----------------------- ---------- - { "rawtransactions", "getassetunlockstatuses", &getassetunlockstatuses, {"indexes","height"} }, - { "rawtransactions", "getrawtransaction", &getrawtransaction, {"txid","verbose","blockhash"} }, - { "rawtransactions", "getrawtransactionmulti", &getrawtransactionmulti, {"transactions","verbose"} }, - { "rawtransactions", "gettxchainlocks", &gettxchainlocks, {"txids"} }, - { "rawtransactions", "createrawtransaction", &createrawtransaction, {"inputs","outputs","locktime"} }, - { "rawtransactions", "decoderawtransaction", &decoderawtransaction, {"hexstring"} }, - { "rawtransactions", "decodescript", &decodescript, {"hexstring"} }, - { "rawtransactions", "sendrawtransaction", &sendrawtransaction, {"hexstring","maxfeerate","instantsend","bypasslimits"} }, - { "rawtransactions", "combinerawtransaction", &combinerawtransaction, {"txs"} }, - { "rawtransactions", "signrawtransactionwithkey", &signrawtransactionwithkey, {"hexstring","privkeys","prevtxs","sighashtype"} }, - { "rawtransactions", "testmempoolaccept", &testmempoolaccept, {"rawtxs","maxfeerate"} }, - { "rawtransactions", "decodepsbt", &decodepsbt, {"psbt"} }, - { "rawtransactions", "combinepsbt", &combinepsbt, {"txs"} }, - { "rawtransactions", "finalizepsbt", &finalizepsbt, {"psbt", "extract"} }, - { "rawtransactions", "createpsbt", &createpsbt, {"inputs","outputs","locktime"} }, - { "rawtransactions", "converttopsbt", &converttopsbt, {"hexstring","permitsigdata"} }, - { "rawtransactions", "utxoupdatepsbt", &utxoupdatepsbt, {"psbt", "descriptors"} }, - { "rawtransactions", "joinpsbts", &joinpsbts, {"txs"} }, - { "rawtransactions", "analyzepsbt", &analyzepsbt, {"psbt"} }, - - { "blockchain", "gettxoutproof", &gettxoutproof, {"txids", "blockhash"} }, - { "blockchain", "verifytxoutproof", &verifytxoutproof, {"proof"} }, +{ // category actor (function) + // --------------------- ----------------------- + { "rawtransactions", &getassetunlockstatuses, }, + { "rawtransactions", &getrawtransaction, }, + { "rawtransactions", &getrawtransactionmulti, }, + { "rawtransactions", &gettxchainlocks, }, + { "rawtransactions", &createrawtransaction, }, + { "rawtransactions", &decoderawtransaction, }, + { "rawtransactions", &decodescript, }, + { "rawtransactions", &sendrawtransaction, }, + { "rawtransactions", &combinerawtransaction, }, + { "rawtransactions", &signrawtransactionwithkey, }, + { "rawtransactions", &testmempoolaccept, }, + { "rawtransactions", &decodepsbt, }, + { "rawtransactions", &combinepsbt, }, + { "rawtransactions", &finalizepsbt, }, + { "rawtransactions", &createpsbt, }, + { "rawtransactions", &converttopsbt, }, + { "rawtransactions", &utxoupdatepsbt, }, + { "rawtransactions", &joinpsbts, }, + { "rawtransactions", &analyzepsbt, }, + + { "blockchain", &gettxoutproof, }, + { "blockchain", &verifytxoutproof, }, }; // clang-format on for (const auto& c : commands) { diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 194fa9c094..08de516717 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -176,10 +176,16 @@ static RPCHelpMan help() [&](const RPCHelpMan& self, const JSONRPCRequest& jsonRequest) -> UniValue { std::string strCommand, strSubCommand; - if (jsonRequest.params.size() > 0) + if (jsonRequest.params.size() > 0) { strCommand = jsonRequest.params[0].get_str(); - if (jsonRequest.params.size() > 1) + } + if (jsonRequest.params.size() > 1) { strSubCommand = jsonRequest.params[1].get_str(); + } + if (strCommand == "dump_all_command_conversions") { + // Used for testing only, undocumented + return tableRPC.dumpArgMap(); + } return tableRPC.help(strCommand, strSubCommand, jsonRequest); }, @@ -280,13 +286,13 @@ static RPCHelpMan getrpcinfo() } // clang-format off static const CRPCCommand vRPCCommands[] = -{ // category name actor (function) argNames - // --------------------- ------------------------ ----------------------- ---------- +{ // category actor (function) + // --------------------- ----------------------- /* Overall control/query calls */ - { "control", "getrpcinfo", &getrpcinfo, {} }, - { "control", "help", &help, {"command","subcommand"} }, - { "control", "stop", &stop, {"wait"} }, - { "control", "uptime", &uptime, {} }, + { "control", &getrpcinfo, }, + { "control", &help, }, + { "control", &stop, }, + { "control", &uptime, }, }; // clang-format on @@ -622,6 +628,18 @@ std::vector> CRPCTable::listCommands() const return commandList; } +UniValue CRPCTable::dumpArgMap() const +{ + UniValue ret{UniValue::VARR}; + for (const auto& cmd : mapCommands) { + for (const auto& c : cmd.second) { + const auto help = RpcMethodFnType(c->unique_id)(); + help.AppendArgMap(ret); + } + } + return ret; +} + void RPCSetTimerInterfaceIfUnset(RPCTimerInterface *iface) { if (!timerInterface) diff --git a/src/rpc/server.h b/src/rpc/server.h index 56744fd47f..5cb93c1fc6 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -101,7 +101,7 @@ class CRPCCommand } //! Simplified constructor taking plain RpcMethodFnType function pointer. - CRPCCommand(std::string category, std::string name_in, RpcMethodFnType fn, std::vector args_in) + CRPCCommand(std::string category, RpcMethodFnType fn) : CRPCCommand( category, fn().m_name, @@ -110,8 +110,6 @@ class CRPCCommand fn().GetArgNames(), intptr_t(fn)) { - CHECK_NONFATAL(fn().m_name == name_in); - CHECK_NONFATAL(fn().GetArgNames() == args_in); } //! Simplified constructor taking plain RpcMethodFnType function pointer with sub-command. @@ -169,6 +167,11 @@ class CRPCTable */ std::vector> listCommands() const; + /** + * Return all named arguments that need to be converted by the client from string to another JSON type + */ + UniValue dumpArgMap() const; + /** * Appends a CRPCCommand to the dispatch table. * diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index c9292461c1..5afb7efa94 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -566,6 +566,23 @@ std::string RPCHelpMan::ToString() const return ret; } +void RPCHelpMan::AppendArgMap(UniValue& arr) const +{ + for (int i{0}; i < int(m_args.size()); ++i) { + const auto& arg = m_args.at(i); + std::vector arg_names = SplitString(arg.m_names, '|'); + for (const auto& arg_name : arg_names) { + UniValue map{UniValue::VARR}; + map.push_back(m_name); + map.push_back(i); + map.push_back(arg_name); + map.push_back(arg.m_type == RPCArg::Type::STR || + arg.m_type == RPCArg::Type::STR_HEX); + arr.push_back(map); + } + } +} + std::string RPCArg::GetFirstName() const { return m_names.substr(0, m_names.find("|")); diff --git a/src/rpc/util.h b/src/rpc/util.h index 57759a2a11..958995ec8c 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -341,6 +341,8 @@ class RPCHelpMan RPCHelpMan(std::string name, std::string description, std::vector args, RPCResults results, RPCExamples examples, RPCMethodImpl fun); std::string ToString() const; + /** Append the named args that need to be converted from string to another JSON type */ + void AppendArgMap(UniValue& arr) const; UniValue HandleRequest(const JSONRPCRequest& request) { Check(request); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 4a70b3cfbc..b4cd4a1146 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4718,76 +4718,76 @@ Span GetWalletRPCCommands() { // clang-format off static const CRPCCommand commands[] = -{ // category name actor (function) argNames - // --------------------- ------------------------ ----------------------- ---------- - { "hidden", "instantsendtoaddress", &instantsendtoaddress, {} }, - { "rawtransactions", "fundrawtransaction", &fundrawtransaction, {"hexstring","options"} }, - { "wallet", "abandontransaction", &abandontransaction, {"txid"} }, - { "wallet", "abortrescan", &abortrescan, {} }, - { "wallet", "addmultisigaddress", &addmultisigaddress, {"nrequired","keys","label"} }, - { "wallet", "backupwallet", &backupwallet, {"destination"} }, - { "wallet", "createwallet", &createwallet, {"wallet_name", "disable_private_keys", "blank", "passphrase", "avoid_reuse", "descriptors", "load_on_startup"} }, - { "wallet", "restorewallet", &restorewallet, {"wallet_name","backup_file", "load_on_startup"} }, - { "wallet", "dumphdinfo", &dumphdinfo, {} }, - { "wallet", "dumpprivkey", &dumpprivkey, {"address"} }, - { "wallet", "dumpwallet", &dumpwallet, {"filename"} }, - { "wallet", "encryptwallet", &encryptwallet, {"passphrase"} }, - { "wallet", "getaddressesbylabel", &getaddressesbylabel, {"label"} }, - { "wallet", "getaddressinfo", &getaddressinfo, {"address"} }, - { "wallet", "getbalance", &getbalance, {"dummy","minconf","addlocked","include_watchonly", "avoid_reuse"} }, - { "wallet", "getnewaddress", &getnewaddress, {"label"} }, - { "wallet", "getrawchangeaddress", &getrawchangeaddress, {} }, - { "wallet", "getreceivedbyaddress", &getreceivedbyaddress, {"address","minconf","addlocked"} }, - { "wallet", "getreceivedbylabel", &getreceivedbylabel, {"label","minconf","addlocked"} }, - { "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly","verbose"} }, - { "wallet", "getunconfirmedbalance", &getunconfirmedbalance, {} }, - { "wallet", "getbalances", &getbalances, {} }, - { "wallet", "getwalletinfo", &getwalletinfo, {} }, - { "wallet", "importaddress", &importaddress, {"address","label","rescan","p2sh"} }, - { "wallet", "importelectrumwallet", &importelectrumwallet, {"filename", "index"} }, - { "wallet", "importdescriptors", &importdescriptors, {"requests"} }, - { "wallet", "importmulti", &importmulti, {"requests","options"} }, - { "wallet", "importprivkey", &importprivkey, {"privkey","label","rescan"} }, - { "wallet", "importprunedfunds", &importprunedfunds, {"rawtransaction","txoutproof"} }, - { "wallet", "importpubkey", &importpubkey, {"pubkey","label","rescan"} }, - { "wallet", "importwallet", &importwallet, {"filename"} }, - { "wallet", "keypoolrefill", &keypoolrefill, {"newsize"} }, - { "wallet", "listaddressbalances", &listaddressbalances, {"minamount"} }, - { "wallet", "listaddressgroupings", &listaddressgroupings, {} }, - { "wallet", "listdescriptors", &listdescriptors, {} }, - { "wallet", "listlabels", &listlabels, {"purpose"} }, - { "wallet", "listlockunspent", &listlockunspent, {} }, - { "wallet", "listreceivedbyaddress", &listreceivedbyaddress, {"minconf","addlocked","include_empty","include_watchonly","address_filter"} }, - { "wallet", "listreceivedbylabel", &listreceivedbylabel, {"minconf","addlocked","include_empty","include_watchonly"} }, - { "wallet", "listsinceblock", &listsinceblock, {"blockhash","target_confirmations","include_watchonly","include_removed"} }, - { "wallet", "listtransactions", &listtransactions, {"label|dummy","count","skip","include_watchonly"} }, - { "wallet", "listunspent", &listunspent, {"minconf","maxconf","addresses","include_unsafe","query_options"} }, - { "wallet", "listwalletdir", &listwalletdir, {} }, - { "wallet", "listwallets", &listwallets, {} }, - { "wallet", "loadwallet", &loadwallet, {"filename", "load_on_startup"} }, - { "wallet", "lockunspent", &lockunspent, {"unlock","transactions"} }, - { "wallet", "removeprunedfunds", &removeprunedfunds, {"txid"} }, - { "wallet", "rescanblockchain", &rescanblockchain, {"start_height", "stop_height"} }, - { "wallet", "send", &send, {"outputs","conf_target","estimate_mode","options"} }, - { "wallet", "sendmany", &sendmany, {"dummy","amounts","minconf","addlocked","comment","subtractfeefrom","use_is","use_cj","conf_target","estimate_mode","verbose"} }, - { "wallet", "sendtoaddress", &sendtoaddress, {"address","amount","comment","comment_to","subtractfeefromamount","use_is","use_cj","conf_target","estimate_mode", "avoid_reuse","verbose"} }, - { "wallet", "sethdseed", &sethdseed, {"newkeypool","seed"} }, - { "wallet", "setcoinjoinrounds", &setcoinjoinrounds, {"rounds"} }, - { "wallet", "setcoinjoinamount", &setcoinjoinamount, {"amount"} }, - { "wallet", "setlabel", &setlabel, {"address","label"} }, - { "wallet", "settxfee", &settxfee, {"amount"} }, - { "wallet", "setwalletflag", &setwalletflag, {"flag","value"} }, - { "wallet", "signmessage", &signmessage, {"address","message"} }, - { "wallet", "signrawtransactionwithwallet", &signrawtransactionwithwallet, {"hexstring","prevtxs","sighashtype"} }, - { "wallet", "unloadwallet", &unloadwallet, {"wallet_name", "load_on_startup"} }, - { "wallet", "upgradewallet", &upgradewallet, {"version"} }, - { "wallet", "upgradetohd", &upgradetohd, {"mnemonic", "mnemonicpassphrase", "walletpassphrase", "rescan"} }, - { "wallet", "walletlock", &walletlock, {} }, - { "wallet", "walletpassphrasechange", &walletpassphrasechange, {"oldpassphrase","newpassphrase"} }, - { "wallet", "walletpassphrase", &walletpassphrase, {"passphrase","timeout","mixingonly"} }, - { "wallet", "walletprocesspsbt", &walletprocesspsbt, {"psbt","sign","sighashtype","bip32derivs"} }, - { "wallet", "walletcreatefundedpsbt", &walletcreatefundedpsbt, {"inputs","outputs","locktime","options","bip32derivs"} }, - { "wallet", "wipewallettxes", &wipewallettxes, {"keep_confirmed"} }, +{ // category actor (function) + // ------------------ ------------------------ + { "hidden", &instantsendtoaddress, }, + { "rawtransactions", &fundrawtransaction, }, + { "wallet", &abandontransaction, }, + { "wallet", &abortrescan, }, + { "wallet", &addmultisigaddress, }, + { "wallet", &backupwallet, }, + { "wallet", &createwallet, }, + { "wallet", &restorewallet, }, + { "wallet", &dumphdinfo, }, + { "wallet", &dumpprivkey, }, + { "wallet", &dumpwallet, }, + { "wallet", &encryptwallet, }, + { "wallet", &getaddressesbylabel, }, + { "wallet", &getaddressinfo, }, + { "wallet", &getbalance, }, + { "wallet", &getnewaddress, }, + { "wallet", &getrawchangeaddress, }, + { "wallet", &getreceivedbyaddress, }, + { "wallet", &getreceivedbylabel, }, + { "wallet", &gettransaction, }, + { "wallet", &getunconfirmedbalance, }, + { "wallet", &getbalances, }, + { "wallet", &getwalletinfo, }, + { "wallet", &importaddress, }, + { "wallet", &importelectrumwallet, }, + { "wallet", &importdescriptors, }, + { "wallet", &importmulti, }, + { "wallet", &importprivkey, }, + { "wallet", &importprunedfunds, }, + { "wallet", &importpubkey, }, + { "wallet", &importwallet, }, + { "wallet", &keypoolrefill, }, + { "wallet", &listaddressbalances, }, + { "wallet", &listaddressgroupings, }, + { "wallet", &listdescriptors, }, + { "wallet", &listlabels, }, + { "wallet", &listlockunspent, }, + { "wallet", &listreceivedbyaddress, }, + { "wallet", &listreceivedbylabel, }, + { "wallet", &listsinceblock, }, + { "wallet", &listtransactions, }, + { "wallet", &listunspent, }, + { "wallet", &listwalletdir, }, + { "wallet", &listwallets, }, + { "wallet", &loadwallet, }, + { "wallet", &lockunspent, }, + { "wallet", &removeprunedfunds, }, + { "wallet", &rescanblockchain, }, + { "wallet", &send, }, + { "wallet", &sendmany, }, + { "wallet", &sendtoaddress, }, + { "wallet", &sethdseed, }, + { "wallet", &setcoinjoinrounds, }, + { "wallet", &setcoinjoinamount, }, + { "wallet", &setlabel, }, + { "wallet", &settxfee, }, + { "wallet", &setwalletflag, }, + { "wallet", &signmessage, }, + { "wallet", &signrawtransactionwithwallet, }, + { "wallet", &unloadwallet, }, + { "wallet", &upgradewallet, }, + { "wallet", &upgradetohd, }, + { "wallet", &walletlock, }, + { "wallet", &walletpassphrasechange, }, + { "wallet", &walletpassphrase, }, + { "wallet", &walletprocesspsbt, }, + { "wallet", &walletcreatefundedpsbt, }, + { "wallet", &wipewallettxes, }, }; // clang-format on return commands; diff --git a/src/zmq/zmqrpc.cpp b/src/zmq/zmqrpc.cpp index 5abb2d0304..5a425430aa 100644 --- a/src/zmq/zmqrpc.cpp +++ b/src/zmq/zmqrpc.cpp @@ -52,9 +52,9 @@ static RPCHelpMan getzmqnotifications() } const CRPCCommand commands[] = -{ // category name actor (function) argNames - // ----------------- ------------------------ ----------------------- ---------- - { "zmq", "getzmqnotifications", &getzmqnotifications, {} }, +{ // category actor (function) + // ----------------- ----------------------- + { "zmq", &getzmqnotifications, }, }; } // anonymous namespace diff --git a/test/functional/rpc_help.py b/test/functional/rpc_help.py index 9df7a934ae..2e821e5939 100755 --- a/test/functional/rpc_help.py +++ b/test/functional/rpc_help.py @@ -7,7 +7,39 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_raises_rpc_error +from collections import defaultdict import os +import re + + +def parse_string(s): + assert s[0] == '"' + assert s[-1] == '"' + return s[1:-1] + + +def process_mapping(fname): + """Find and parse conversion table in implementation file `fname`.""" + cmds = [] + in_rpcs = False + with open(fname, "r", encoding="utf8") as f: + for line in f: + line = line.rstrip() + if not in_rpcs: + if line == 'static const CRPCConvertParam vRPCConvertParams[] =': + in_rpcs = True + else: + if line.startswith('};'): + in_rpcs = False + elif '{' in line and '"' in line: + m = re.search(r'{ *("[^"]*"), *([0-9]+) *, *("[^"]*") *},', line) + assert m, 'No match to table expression: %s' % line + name = parse_string(m.group(1)) + idx = int(m.group(2)) + argname = parse_string(m.group(3)) + cmds.append((name, idx, argname)) + assert not in_rpcs and cmds + return cmds class HelpRpcTest(BitcoinTestFramework): @@ -16,11 +48,43 @@ def set_test_params(self): self.supports_cli = False def run_test(self): + self.test_client_conversion_table() self.test_categories() self.dump_help() if self.is_wallet_compiled(): self.wallet_help() + def test_client_conversion_table(self): + file_conversion_table = os.path.join(self.config["environment"]["SRCDIR"], 'src', 'rpc', 'client.cpp') + mapping_client = process_mapping(file_conversion_table) + # Ignore echojson in client table + mapping_client = [m for m in mapping_client if m[0] != 'echojson'] + + mapping_server = self.nodes[0].help("dump_all_command_conversions") + # Filter all RPCs whether they need conversion + mapping_server_conversion = [tuple(m[:3]) for m in mapping_server if not m[3]] + + # Only check if all RPC methods have been compiled (i.e. wallet is enabled) + if self.is_wallet_compiled() and sorted(mapping_client) != sorted(mapping_server_conversion): + raise AssertionError("RPC client conversion table ({}) and RPC server named arguments mismatch!\n{}".format( + file_conversion_table, + set(mapping_client).symmetric_difference(mapping_server_conversion), + )) + + # Check for conversion difference by argument name. + # It is preferable for API consistency that arguments with the same name + # have the same conversion, so bin by argument name. + all_methods_by_argname = defaultdict(list) + converts_by_argname = defaultdict(list) + for m in mapping_server: + all_methods_by_argname[m[2]].append(m[0]) + converts_by_argname[m[2]].append(m[3]) + + for argname, convert in converts_by_argname.items(): + if all(convert) != any(convert): + # Only allow dummy to fail consistency check + assert argname == 'dummy', ('WARNING: conversion mismatch for argument named %s (%s)' % (argname, list(zip(all_methods_by_argname[argname], converts_by_argname[argname])))) + def test_categories(self): node = self.nodes[0] diff --git a/test/lint/check-rpc-mappings.py b/test/lint/check-rpc-mappings.py deleted file mode 100755 index c9596dbc77..0000000000 --- a/test/lint/check-rpc-mappings.py +++ /dev/null @@ -1,173 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (c) 2017-2019 The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. -"""Check RPC argument consistency.""" - -from collections import defaultdict -import os -import re -import sys - -# Source files (relative to root) to scan for dispatch tables -SOURCES = [ - "src/rpc/server.cpp", - "src/rpc/blockchain.cpp", - "src/rpc/coinjoin.cpp", - "src/rpc/evo.cpp", - "src/rpc/governance.cpp", - "src/rpc/masternode.cpp", - "src/rpc/mining.cpp", - "src/rpc/misc.cpp", - "src/rpc/net.cpp", - "src/rpc/quorums.cpp", - "src/rpc/rawtransaction.cpp", - "src/wallet/rpcwallet.cpp", -] -# Source file (relative to root) containing conversion mapping -SOURCE_CLIENT = 'src/rpc/client.cpp' -# Argument names that should be ignored in consistency checks -IGNORE_DUMMY_ARGS = {'dummy', 'arg0', 'arg1', 'arg2', 'arg3', 'arg4', 'arg5', 'arg6', 'arg7', 'arg8', 'arg9'} - -class RPCCommand: - def __init__(self, name, args): - self.name = name - self.args = args - -class RPCArgument: - def __init__(self, names, idx): - self.names = names - self.idx = idx - self.convert = False - -def parse_string(s): - assert s[0] == '"' - assert s[-1] == '"' - return s[1:-1] - -def process_commands(fname): - """Find and parse dispatch table in implementation file `fname`.""" - cmds = [] - in_rpcs = False - with open(fname, "r", encoding="utf8") as f: - for line in f: - line = line.rstrip() - if not in_rpcs: - if re.match(r"static const CRPCCommand .*\[\] =", line): - in_rpcs = True - else: - if line.startswith('};'): - in_rpcs = False - elif '{' in line and '"' in line: - m = re.search(r'{ *("[^"]*"), *("[^"]*"), *&([^,]*), *{([^}]*)} *},', line) - # that's a quick fix for composite command - # no proper implementation is needed so far as this linter would be removed soon with bitcoin#20012 - if not m: - m = re.search(r'{ *("[^"]*"), *("[^"]*"), *("[^"]*"), *&([^,]*), *{([^}]*)} *},', line) - if m: - continue - assert m, 'No match to table expression: %s' % line - name = parse_string(m.group(2)) - args_str = m.group(4).strip() - if args_str: - args = [RPCArgument(parse_string(x.strip()).split('|'), idx) for idx, x in enumerate(args_str.split(','))] - else: - args = [] - cmds.append(RPCCommand(name, args)) - assert not in_rpcs and cmds, "Something went wrong with parsing the C++ file: update the regexps" - return cmds - -def process_mapping(fname): - """Find and parse conversion table in implementation file `fname`.""" - cmds = [] - in_rpcs = False - with open(fname, "r", encoding="utf8") as f: - for line in f: - line = line.rstrip() - if not in_rpcs: - if line == 'static const CRPCConvertParam vRPCConvertParams[] =': - in_rpcs = True - else: - if line.startswith('};'): - in_rpcs = False - elif '{' in line and '"' in line: - m = re.search(r'{ *("[^"]*"), *([0-9]+) *, *("[^"]*") *},', line) - assert m, 'No match to table expression: %s' % line - name = parse_string(m.group(1)) - idx = int(m.group(2)) - argname = parse_string(m.group(3)) - cmds.append((name, idx, argname)) - assert not in_rpcs and cmds - return cmds - -def main(): - if len(sys.argv) != 2: - print('Usage: {} ROOT-DIR'.format(sys.argv[0]), file=sys.stderr) - sys.exit(1) - - root = sys.argv[1] - - # Get all commands from dispatch tables - cmds = [] - for fname in SOURCES: - cmds += process_commands(os.path.join(root, fname)) - - cmds_by_name = {} - for cmd in cmds: - cmds_by_name[cmd.name] = cmd - - # Get current convert mapping for client - client = SOURCE_CLIENT - mapping = set(process_mapping(os.path.join(root, client))) - - print('* Checking consistency between dispatch tables and vRPCConvertParams') - - # Check mapping consistency - errors = 0 - for (cmdname, argidx, argname) in mapping: - try: - rargnames = cmds_by_name[cmdname].args[argidx].names - except IndexError: - print('ERROR: %s argument %i (named %s in vRPCConvertParams) is not defined in dispatch table' % (cmdname, argidx, argname)) - errors += 1 - continue - if argname not in rargnames: - print('ERROR: %s argument %i is named %s in vRPCConvertParams but %s in dispatch table' % (cmdname, argidx, argname, rargnames), file=sys.stderr) - errors += 1 - - # Check for conflicts in vRPCConvertParams conversion - # All aliases for an argument must either be present in the - # conversion table, or not. Anything in between means an oversight - # and some aliases won't work. - for cmd in cmds: - for arg in cmd.args: - convert = [((cmd.name, arg.idx, argname) in mapping) for argname in arg.names] - if any(convert) != all(convert): - print('ERROR: %s argument %s has conflicts in vRPCConvertParams conversion specifier %s' % (cmd.name, arg.names, convert)) - errors += 1 - arg.convert = all(convert) - - # Check for conversion difference by argument name. - # It is preferable for API consistency that arguments with the same name - # have the same conversion, so bin by argument name. - all_methods_by_argname = defaultdict(list) - converts_by_argname = defaultdict(list) - for cmd in cmds: - for arg in cmd.args: - for argname in arg.names: - all_methods_by_argname[argname].append(cmd.name) - converts_by_argname[argname].append(arg.convert) - - for argname, convert in converts_by_argname.items(): - if all(convert) != any(convert): - if argname in IGNORE_DUMMY_ARGS: - # these are testing or dummy, don't warn for them - continue - print('WARNING: conversion mismatch for argument named %s (%s)' % - (argname, list(zip(all_methods_by_argname[argname], converts_by_argname[argname])))) - - sys.exit(errors > 0) - - -if __name__ == '__main__': - main() From 3270becc9b9e7cf09049bae98e905d205cd6f5bc Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 12 Jul 2024 03:27:43 +0700 Subject: [PATCH 11/21] chore: add TODO to make client parsing for composite commands --- src/rpc/server.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 08de516717..b08fc680aa 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -632,6 +632,8 @@ UniValue CRPCTable::dumpArgMap() const { UniValue ret{UniValue::VARR}; for (const auto& cmd : mapCommands) { + // TODO: implement mapping argument to type for composite commands + if (!cmd.first.second.empty()) continue; for (const auto& c : cmd.second) { const auto help = RpcMethodFnType(c->unique_id)(); help.AppendArgMap(ret); From d3b1ef374cad20862f3ed880e5e0d99d281bc148 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 12 Jul 2024 03:26:34 +0700 Subject: [PATCH 12/21] refactor: simplify implementation of RPC composite commands --- src/interfaces/node.h | 2 +- src/node/interfaces.cpp | 6 +++--- src/qt/rpcconsole.cpp | 10 +++------ src/rpc/coinjoin.cpp | 2 +- src/rpc/evo.cpp | 2 +- src/rpc/governance.cpp | 2 +- src/rpc/masternode.cpp | 2 +- src/rpc/quorums.cpp | 2 +- src/rpc/server.cpp | 44 +++++++++++++++++++-------------------- src/rpc/server.h | 34 ++++++------------------------ src/test/rpc_tests.cpp | 2 +- src/wallet/interfaces.cpp | 2 +- 12 files changed, 41 insertions(+), 69 deletions(-) diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 1680ec3a55..46b6f64317 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -253,7 +253,7 @@ class Node virtual UniValue executeRpc(const std::string& command, const UniValue& params, const std::string& uri) = 0; //! List rpc commands. - virtual std::vector> listRpcCommands() = 0; + virtual std::vector listRpcCommands() = 0; //! Set RPC timer interface if unset. virtual void rpcSetTimerInterfaceIfUnset(RPCTimerInterface* iface) = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index f74983b871..7339fa1a44 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -486,7 +486,7 @@ class NodeImpl : public Node req.URI = uri; return ::tableRPC.execute(req); } - std::vector> listRpcCommands() override { return ::tableRPC.listCommands(); } + std::vector listRpcCommands() override { return ::tableRPC.listCommands(); } void rpcSetTimerInterfaceIfUnset(RPCTimerInterface* iface) override { RPCSetTimerInterfaceIfUnset(iface); } void rpcUnsetTimerInterface(RPCTimerInterface* iface) override { RPCUnsetTimerInterface(iface); } bool getUnspentOutput(const COutPoint& output, Coin& coin) override @@ -689,14 +689,14 @@ class RpcHandlerImpl : public Handler throw; } }; - ::tableRPC.appendCommand(m_command.name, m_command.subname, &m_command); + ::tableRPC.appendCommand(m_command.name, &m_command); } void disconnect() override final { if (m_wrapped_command) { m_wrapped_command = nullptr; - ::tableRPC.removeCommand(m_command.name, m_command.subname, &m_command); + ::tableRPC.removeCommand(m_command.name, &m_command); } } diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 130c6183c4..e763bb13ad 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -773,15 +773,11 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_ //Setup autocomplete and attach it QStringList wordList; - std::vector> commandList = m_node.listRpcCommands(); + std::vector commandList = m_node.listRpcCommands(); for (size_t i = 0; i < commandList.size(); ++i) { - std::string command = commandList[i].first; - if (!commandList[i].second.empty()) { - command = command + " " + commandList[i].second; - } - wordList << command.c_str(); - wordList << ("help " + command).c_str(); + wordList << commandList[i].c_str(); + wordList << ("help " + commandList[i]).c_str(); } wordList << "help-console"; diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp index 45f4e52f93..709d808543 100644 --- a/src/rpc/coinjoin.cpp +++ b/src/rpc/coinjoin.cpp @@ -294,6 +294,6 @@ static const CRPCCommand commands[] = }; // clang-format on for (const auto& command : commands) { - t.appendCommand(command.name, command.subname, &command); + t.appendCommand(command.name, &command); } } diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index a0e2ffdb97..aa5a3d55f7 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -1903,6 +1903,6 @@ static const CRPCCommand commands[] = }; // clang-format on for (const auto& command : commands) { - tableRPC.appendCommand(command.name, command.subname, &command); + tableRPC.appendCommand(command.name, &command); } } diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 8c92e8c08b..7cf035070d 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -1081,6 +1081,6 @@ static const CRPCCommand commands[] = }; // clang-format on for (const auto& command : commands) { - t.appendCommand(command.name, command.subname, &command); + t.appendCommand(command.name, &command); } } diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index bc2550ea0c..9a1bfddace 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -740,6 +740,6 @@ static const CRPCCommand commands[] = }; // clang-format on for (const auto& command : commands) { - t.appendCommand(command.name, command.subname, &command); + t.appendCommand(command.name, &command); } } diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 311d90cf7b..8994e54c77 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -1157,6 +1157,6 @@ static const CRPCCommand commands[] = }; // clang-format on for (const auto& command : commands) { - tableRPC.appendCommand(command.name, command.subname, &command); + tableRPC.appendCommand(command.name, &command); } } diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index b08fc680aa..7757dad6c0 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -85,7 +85,7 @@ void RPCServer::OnStopped(std::function slot) g_rpcSignals.Stopped.connect(slot); } -std::string CRPCTable::help(const std::string& strCommand, const std::string& strSubCommand, const JSONRPCRequest& helpreq) const +std::string CRPCTable::help(const std::string& strCommand, const JSONRPCRequest& helpreq) const { std::string strRet; std::string category; @@ -93,7 +93,7 @@ std::string CRPCTable::help(const std::string& strCommand, const std::string& st std::vector > vCommands; for (const auto& entry : mapCommands) - vCommands.push_back(make_pair(entry.second.front()->category + entry.first.first + entry.first.second, entry.second.front())); + vCommands.push_back(make_pair(entry.second.front()->category + entry.first, entry.second.front())); sort(vCommands.begin(), vCommands.end()); JSONRPCRequest jreq = helpreq; @@ -107,14 +107,16 @@ std::string CRPCTable::help(const std::string& strCommand, const std::string& st if ((strCommand != "" || pcmd->category == "hidden") && strMethod != strCommand) continue; - if (strSubCommand != pcmd->subname) continue; + const auto pos_separator{strMethod.find(' ')}; + const bool is_composite{pos_separator != std::string::npos}; + if (strCommand.empty() && is_composite) continue; jreq.strMethod = strMethod; try { - if (!strSubCommand.empty()) { + if (is_composite) { jreq.params.setArray(); - jreq.params.push_back(strSubCommand); + jreq.params.push_back(strCommand.substr(pos_separator + 1)); } UniValue unused_result; if (setDone.insert(pcmd->unique_id).second) @@ -180,14 +182,14 @@ static RPCHelpMan help() strCommand = jsonRequest.params[0].get_str(); } if (jsonRequest.params.size() > 1) { - strSubCommand = jsonRequest.params[1].get_str(); + strCommand += " " + jsonRequest.params[1].get_str(); } if (strCommand == "dump_all_command_conversions") { // Used for testing only, undocumented return tableRPC.dumpArgMap(); } - return tableRPC.help(strCommand, strSubCommand, jsonRequest); + return tableRPC.help(strCommand, jsonRequest); }, }; } @@ -304,20 +306,15 @@ CRPCTable::CRPCTable() } void CRPCTable::appendCommand(const std::string& name, const CRPCCommand* pcmd) -{ - appendCommand(name, "", pcmd); -} - -void CRPCTable::appendCommand(const std::string& name, const std::string& subname, const CRPCCommand* pcmd) { CHECK_NONFATAL(!IsRPCRunning()); // Only add commands before rpc is running - mapCommands[std::make_pair(name, subname)].push_back(pcmd); + mapCommands[name].push_back(pcmd); } -bool CRPCTable::removeCommand(const std::string& name, const std::string& subname, const CRPCCommand* pcmd) +bool CRPCTable::removeCommand(const std::string& name, const CRPCCommand* pcmd) { - auto it = mapCommands.find(std::make_pair(name, subname)); + auto it = mapCommands.find(name); if (it != mapCommands.end()) { auto new_end = std::remove(it->second.begin(), it->second.end(), pcmd); if (it->second.end() != new_end) { @@ -516,16 +513,18 @@ UniValue CRPCTable::execute(const JSONRPCRequest &request) const throw JSONRPCError(RPC_IN_WARMUP, rpcWarmupStatus); } + auto it = mapCommands.end(); + std::string subcommand; if (request.params.size() > 0 && request.params[0].isStr()) { subcommand = request.params[0].get_str(); + it = mapCommands.find(request.strMethod + " " + subcommand); } // Find method - auto it = mapCommands.find(std::make_pair(request.strMethod, subcommand)); - if (it == mapCommands.end() && !subcommand.empty()) { - subcommand = ""; - it = mapCommands.find(std::make_pair(request.strMethod, subcommand)); + if (it == mapCommands.end()) { + it = mapCommands.find(request.strMethod); + subcommand.clear(); } if (it != mapCommands.end()) { UniValue result; @@ -546,7 +545,6 @@ static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& req if (node.mn_activeman && request.authUser == gArgs.GetArg("-platform-user", defaultPlatformUser)) { // replace this with structured binding in c++20 std::string command_name = command.name; - if (!command.subname.empty()) command_name += " " + command.subname; const auto& it = mapPlatformRestrictions.equal_range(command_name); const auto& allowed_begin = it.first; const auto& allowed_end = it.second; @@ -621,9 +619,9 @@ static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& req } } -std::vector> CRPCTable::listCommands() const +std::vector CRPCTable::listCommands() const { - std::vector> commandList; + std::vector commandList; for (const auto& i : mapCommands) commandList.emplace_back(i.first); return commandList; } @@ -633,7 +631,7 @@ UniValue CRPCTable::dumpArgMap() const UniValue ret{UniValue::VARR}; for (const auto& cmd : mapCommands) { // TODO: implement mapping argument to type for composite commands - if (!cmd.first.second.empty()) continue; + if (cmd.first.find(' ') != std::string::npos) continue; for (const auto& c : cmd.second) { const auto help = RpcMethodFnType(c->unique_id)(); help.AppendArgMap(ret); diff --git a/src/rpc/server.h b/src/rpc/server.h index 5cb93c1fc6..8b6b64041a 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -94,8 +94,8 @@ class CRPCCommand using Actor = std::function; //! Constructor taking Actor callback supporting multiple handlers. - CRPCCommand(std::string category, std::string name, std::string subname, Actor actor, std::vector args, intptr_t unique_id) - : category(std::move(category)), name(std::move(name)), subname(subname), actor(std::move(actor)), argNames(std::move(args)), + CRPCCommand(std::string category, std::string name, Actor actor, std::vector args, intptr_t unique_id) + : category(std::move(category)), name(std::move(name)), actor(std::move(actor)), argNames(std::move(args)), unique_id(unique_id) { } @@ -105,35 +105,14 @@ class CRPCCommand : CRPCCommand( category, fn().m_name, - "", [fn](const JSONRPCRequest& request, UniValue& result, bool) { result = fn().HandleRequest(request); return true; }, fn().GetArgNames(), intptr_t(fn)) { } - //! Simplified constructor taking plain RpcMethodFnType function pointer with sub-command. - CRPCCommand(std::string category, std::string name_in, std::string subname_in, RpcMethodFnType fn, std::vector args_in) - : CRPCCommand( - category, - name_in, - subname_in, - [fn](const JSONRPCRequest& request, UniValue& result, bool) { result = fn().HandleRequest(request); return true; }, - fn().GetArgNames(), - intptr_t(fn)) - { - if (subname_in.empty()) { - CHECK_NONFATAL(fn().m_name == name_in); - } else { - CHECK_NONFATAL(fn().m_name == name_in + " " + subname_in); - } - - CHECK_NONFATAL(fn().GetArgNames() == args_in); - } - std::string category; std::string name; - std::string subname; Actor actor; std::vector argNames; intptr_t unique_id; @@ -145,11 +124,11 @@ class CRPCCommand class CRPCTable { private: - std::map, std::vector> mapCommands; + std::map> mapCommands; std::multimap> mapPlatformRestrictions; public: CRPCTable(); - std::string help(const std::string& name, const std::string& strSubCommand, const JSONRPCRequest& helpreq) const; + std::string help(const std::string& name, const JSONRPCRequest& helpreq) const; void InitPlatformRestrictions(); @@ -165,7 +144,7 @@ class CRPCTable * Returns a list of registered commands * @returns List of registered commands. */ - std::vector> listCommands() const; + std::vector listCommands() const; /** * Return all named arguments that need to be converted by the client from string to another JSON type @@ -185,8 +164,7 @@ class CRPCTable * register different names, types, and numbers of parameters. */ void appendCommand(const std::string& name, const CRPCCommand* pcmd); - void appendCommand(const std::string& name, const std::string& subname, const CRPCCommand* pcmd); - bool removeCommand(const std::string& name, const std::string& subname, const CRPCCommand* pcmd); + bool removeCommand(const std::string& name, const CRPCCommand* pcmd); }; bool IsDeprecatedRPCEnabled(const std::string& method); diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index a39992bfed..1e3d55673c 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -49,7 +49,7 @@ UniValue RPCTestingSetup::TransformParams(const UniValue& params, std::vector bool { transformed_params = request.params; return true; }, arg_names, /*unique_id=*/0}; + CRPCCommand command{"category", "method", [&](const JSONRPCRequest& request, UniValue&, bool) -> bool { transformed_params = request.params; return true; }, arg_names, /*unique_id=*/0}; table.appendCommand("method", &command); CoreContext context{m_node}; JSONRPCRequest request(context); diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 9f1ebb49fd..c1e11ab09f 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -572,7 +572,7 @@ class WalletLoaderImpl : public WalletLoader void registerRpcs() override { for (const CRPCCommand& command : GetWalletRPCCommands()) { - m_rpc_commands.emplace_back(command.category, command.name, command.subname, [this, &command](const JSONRPCRequest& request, UniValue& result, bool last_handler) { + m_rpc_commands.emplace_back(command.category, command.name, [this, &command](const JSONRPCRequest& request, UniValue& result, bool last_handler) { return command.actor({request, m_context}, result, last_handler); }, command.argNames, command.unique_id); m_rpc_handlers.emplace_back(m_context.chain->handleRpc(m_rpc_commands.back())); From 2f7814acdd6786ad3cd3630cbe89d1cd99cb0a4e Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 15 Mar 2021 10:13:10 +0100 Subject: [PATCH 13/21] Merge #21035: Remove pointer cast in CRPCTable::dumpArgMap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 9048c58e10841d9e1d709c0a325dd14684cec325 Remove pointer cast in CRPCTable::dumpArgMap (Russell Yanofsky) 14f3d9b908ed9e78997bfaad3d8a06357a89d46e refactor: Add RPC server ExecuteCommands function (Russell Yanofsky) 6158a6d3978a18d5ac4166ca2f59056d8ef71c3d refactor: Replace JSONRPCRequest fHelp field with mode field (Russell Yanofsky) Pull request description: This change is needed to fix the `rpc_help.py` test failing in #10102: https://cirrus-ci.com/task/5469433013469184?command=ci#L2275 The [`CRPCTable::dumpArgMap`](https://github.com/bitcoin/bitcoin/blob/16b784d953365bb2d7ae65acd2b20a79ef8ba7b6/src/rpc/server.cpp#L492) method currently works by casting RPC `unique_id` integer field to a function pointer, and then calling it. The `unique_id` field wasn't supposed to be used this way (it's meant to be used to detect RPC aliases) and as a result, this code segfaults in the `rpc_help.py` test in multiprocess PR #10102 because wallet RPC functions aren't directly accessible from the node process. Fix this by adding a new `GET_ARGS` RPC request mode to retrieve argument information similar to the way the `GET_HELP` mode retrieves help information. --- This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). ACKs for top commit: MarcoFalke: re-ACK 9048c58e10841d9e1d709c0a325dd14684cec325 👑 Tree-SHA512: cd1a01c1daa5bde2c2455b63548371ee4cf39688313969ad2016d9a0fd4344102e3fd43034058f253364518e9632d57cf21abffad0d6a2c0c94b7a6921cbe615 --- src/rpc/request.h | 6 +++--- src/rpc/server.cpp | 35 ++++++++++++++++++++++++----------- src/rpc/server.h | 2 +- src/rpc/util.cpp | 19 ++++++++++++++++++- src/rpc/util.h | 19 +++---------------- src/test/rpc_tests.cpp | 1 - src/wallet/rpcwallet.cpp | 4 ++-- 7 files changed, 51 insertions(+), 35 deletions(-) diff --git a/src/rpc/request.h b/src/rpc/request.h index 4ab5253a78..d849f6ee21 100644 --- a/src/rpc/request.h +++ b/src/rpc/request.h @@ -32,19 +32,19 @@ class JSONRPCRequest UniValue id; std::string strMethod; UniValue params; - bool fHelp; + enum Mode { EXECUTE, GET_HELP, GET_ARGS } mode = EXECUTE; std::string URI; std::string authUser; std::string peerAddr; const CoreContext& context; - explicit JSONRPCRequest(const CoreContext& context) : id(NullUniValue), params(NullUniValue), fHelp(false), context(context) {} + explicit JSONRPCRequest(const CoreContext& context) : id(NullUniValue), params(NullUniValue), context(context) {} //! Initializes request information from another request object and the //! given context. The implementation should be updated if any members are //! added or removed above. JSONRPCRequest(const JSONRPCRequest& other, const CoreContext& context) - : id(other.id), strMethod(other.strMethod), params(other.params), fHelp(other.fHelp), URI(other.URI), + : id(other.id), strMethod(other.strMethod), params(other.params), mode(other.mode), URI(other.URI), authUser(other.authUser), peerAddr(other.peerAddr), context(context) { } diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 7757dad6c0..73ba4b75c6 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -97,7 +97,7 @@ std::string CRPCTable::help(const std::string& strCommand, const JSONRPCRequest& sort(vCommands.begin(), vCommands.end()); JSONRPCRequest jreq = helpreq; - jreq.fHelp = true; + jreq.mode = JSONRPCRequest::GET_HELP; jreq.params = UniValue(); for (const std::pair& command : vCommands) @@ -186,7 +186,7 @@ static RPCHelpMan help() } if (strCommand == "dump_all_command_conversions") { // Used for testing only, undocumented - return tableRPC.dumpArgMap(); + return tableRPC.dumpArgMap(jsonRequest); } return tableRPC.help(strCommand, jsonRequest); @@ -504,6 +504,16 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c return out; } +static bool ExecuteCommands(const std::vector& commands, const JSONRPCRequest& request, UniValue& result, const std::multimap>& mapPlatformRestrictions) +{ + for (const auto& command : commands) { + if (ExecuteCommand(*command, request, result, &command == &commands.back(), mapPlatformRestrictions)) { + return true; + } + } + return false; +} + UniValue CRPCTable::execute(const JSONRPCRequest &request) const { // Return immediately if in warmup @@ -528,11 +538,9 @@ UniValue CRPCTable::execute(const JSONRPCRequest &request) const } if (it != mapCommands.end()) { UniValue result; - for (const auto& command : it->second) { - const JSONRPCRequest new_request{subcommand.empty() ? request : request.squashed() }; - if (ExecuteCommand(*command, new_request, result, &command == &it->second.back(), mapPlatformRestrictions)) { - return result; - } + const JSONRPCRequest new_request{subcommand.empty() ? request : request.squashed() }; + if (ExecuteCommands(it->second, new_request, result, mapPlatformRestrictions)) { + return result; } } throw JSONRPCError(RPC_METHOD_NOT_FOUND, "Method not found"); @@ -626,15 +634,20 @@ std::vector CRPCTable::listCommands() const return commandList; } -UniValue CRPCTable::dumpArgMap() const +UniValue CRPCTable::dumpArgMap(const JSONRPCRequest& args_request) const { + JSONRPCRequest request(args_request); + request.mode = JSONRPCRequest::GET_ARGS; + UniValue ret{UniValue::VARR}; for (const auto& cmd : mapCommands) { // TODO: implement mapping argument to type for composite commands if (cmd.first.find(' ') != std::string::npos) continue; - for (const auto& c : cmd.second) { - const auto help = RpcMethodFnType(c->unique_id)(); - help.AppendArgMap(ret); + UniValue result; + if (ExecuteCommands(cmd.second, request, result, mapPlatformRestrictions)) { + for (const auto& values : result.getValues()) { + ret.push_back(values); + } } } return ret; diff --git a/src/rpc/server.h b/src/rpc/server.h index 8b6b64041a..5a03ba1b4a 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -149,7 +149,7 @@ class CRPCTable /** * Return all named arguments that need to be converted by the client from string to another JSON type */ - UniValue dumpArgMap() const; + UniValue dumpArgMap(const JSONRPCRequest& request) const; /** * Appends a CRPCCommand to the dispatch table. diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 5afb7efa94..265fb438c8 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -493,6 +493,21 @@ std::string RPCExamples::ToDescriptionString() const return m_examples.empty() ? m_examples : "\nExamples:\n" + m_examples; } +UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request) +{ + if (request.mode == JSONRPCRequest::GET_ARGS) { + return GetArgMap(); + } + /* + * Check if the given request is valid according to this command or if + * the user is asking for help information, and throw help when appropriate. + */ + if (request.mode == JSONRPCRequest::GET_HELP || !IsValidNumArgs(request.params.size())) { + throw std::runtime_error(ToString()); + } + return m_fun(*this, request); +} + bool RPCHelpMan::IsValidNumArgs(size_t num_args) const { size_t num_required_args = 0; @@ -566,8 +581,9 @@ std::string RPCHelpMan::ToString() const return ret; } -void RPCHelpMan::AppendArgMap(UniValue& arr) const +UniValue RPCHelpMan::GetArgMap() const { + UniValue arr{UniValue::VARR}; for (int i{0}; i < int(m_args.size()); ++i) { const auto& arg = m_args.at(i); std::vector arg_names = SplitString(arg.m_names, '|'); @@ -581,6 +597,7 @@ void RPCHelpMan::AppendArgMap(UniValue& arr) const arr.push_back(map); } } + return arr; } std::string RPCArg::GetFirstName() const diff --git a/src/rpc/util.h b/src/rpc/util.h index 958995ec8c..ae422dc744 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -340,25 +340,12 @@ class RPCHelpMan using RPCMethodImpl = std::function; RPCHelpMan(std::string name, std::string description, std::vector args, RPCResults results, RPCExamples examples, RPCMethodImpl fun); + UniValue HandleRequest(const JSONRPCRequest& request); std::string ToString() const; - /** Append the named args that need to be converted from string to another JSON type */ - void AppendArgMap(UniValue& arr) const; - UniValue HandleRequest(const JSONRPCRequest& request) - { - Check(request); - return m_fun(*this, request); - } + /** Return the named args that need to be converted from string to another JSON type */ + UniValue GetArgMap() const; /** If the supplied number of args is neither too small nor too high */ bool IsValidNumArgs(size_t num_args) const; - /** - * Check if the given request is valid according to this command or if - * the user is asking for help information, and throw help when appropriate. - */ - inline void Check(const JSONRPCRequest& request) const { - if (request.fHelp || !IsValidNumArgs(request.params.size())) { - throw std::runtime_error(ToString()); - } - } std::vector GetArgNames() const; diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 1e3d55673c..75f9b96bd2 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -69,7 +69,6 @@ UniValue RPCTestingSetup::CallRPC(std::string args) JSONRPCRequest request(context); request.strMethod = strMethod; request.params = RPCConvertValues(strMethod, vArgs); - request.fHelp = false; if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished(); try { UniValue result = tableRPC.execute(request); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index b4cd4a1146..d050a9587d 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -102,6 +102,8 @@ bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& 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); @@ -114,8 +116,6 @@ std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& reques return wallets[0]; } - if (request.fHelp) return nullptr; - 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)"); From c0cdb0488b432cf01c295fdf6b519cc22662b52e Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 7 Apr 2021 10:53:20 +0200 Subject: [PATCH 14/21] Merge #21572: Fix wrong wallet RPC context set after #21366 937fd4a66f048780bffc5e714d0c800de987ce93 Fix wrong wallet RPC context set after #21366 (Russell Yanofsky) Pull request description: This bug doesn't have any effects currently because it only affects external signer RPCs which aren't currently using the wallet context, but it does cause an appveyor failure in a upcoming PR: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/38512882 This bug is subtle and could have been avoided if JSONRPCRequest didn't have constructors that were so loose with type checking. Suggested change https://github.com/bitcoin/bitcoin/pull/21366#issuecomment-792044351 eliminates these and would be a good followup for a future PR. This PR just implements the simplest possible fix. ACKs for top commit: theStack: Code-review ACK 937fd4a66f048780bffc5e714d0c800de987ce93 meshcollider: Code review ACK 937fd4a66f048780bffc5e714d0c800de987ce93 Tree-SHA512: 53e6265ed6c7abb47d2b3e77d1604edfeb993c3a2440f0c19679cfeb23516965e6707ff486196a0acfbeff21c79a9a08b5cd33bae9a232d33d0134bca1bd0ff3 --- src/wallet/interfaces.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index c1e11ab09f..9f18bf7e35 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -573,7 +573,7 @@ class WalletLoaderImpl : public WalletLoader { for (const CRPCCommand& command : GetWalletRPCCommands()) { m_rpc_commands.emplace_back(command.category, command.name, [this, &command](const JSONRPCRequest& request, UniValue& result, bool last_handler) { - return command.actor({request, m_context}, result, last_handler); + return command.actor({request, &m_context}, result, last_handler); }, command.argNames, command.unique_id); m_rpc_handlers.emplace_back(m_context.chain->handleRpc(m_rpc_commands.back())); } From bfc083e9b792d9c342b336a121de046f668f9799 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 8 Apr 2021 09:07:57 +0200 Subject: [PATCH 15/21] Merge #21574: Drop JSONRPCRequest constructors after #21366 9044522ef76f880760165d98fab024802ccfc062 Drop JSONRPCRequest constructors after #21366 (Russell Yanofsky) Pull request description: This just makes an additional simplification after #21366 replaced util::Ref with std::any. It was originally suggested https://github.com/bitcoin/bitcoin/pull/21366#issuecomment-792044351 but delayed for a followup. It would have prevented usage bug https://github.com/bitcoin/bitcoin/pull/21572. ACKs for top commit: promag: ACK 9044522ef76f880760165d98fab024802ccfc062, fixed conflict in src/wallet/interfaces.cpp. Tree-SHA512: e909411b8f75013620b94e1a609296befb832fdcb574cd2e6689bfe3c636b03cd4ac1ccb2b32b532daf0f2131bb043464024966310fffc7e3cad77713d4bd0ef --- src/bitcoind.cpp | 2 +- src/httprpc.cpp | 6 +++--- src/init.cpp | 10 +++++----- src/init.h | 2 +- src/node/interfaces.cpp | 12 +++--------- src/rest.cpp | 5 +++-- src/rpc/request.cpp | 2 +- src/rpc/request.h | 15 ++------------- src/test/rpc_tests.cpp | 6 ++++-- src/wallet/interfaces.cpp | 4 +++- src/wallet/test/wallet_tests.cpp | 17 ++++++++--------- 11 files changed, 34 insertions(+), 47 deletions(-) diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 1ec5867f36..ce8384026e 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -234,7 +234,7 @@ static bool AppInit(int argc, char* argv[]) // If locking the data directory failed, exit immediately return false; } - fRet = AppInitInterfaces(node) && AppInitMain(context, node); + fRet = AppInitInterfaces(node) && AppInitMain(node); } catch (...) { PrintExceptionContinue(std::current_exception(), "AppInit()"); } diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 5a5f0f601c..fbf2951a12 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -201,8 +201,8 @@ static bool HTTPReq_JSONRPC(const CoreContext& context, HTTPRequest* req) return rpcRequest.send_reply(HTTP_UNAUTHORIZED); } - JSONRPCRequest jreq(context); - + JSONRPCRequest jreq; + jreq.context = context; jreq.peerAddr = req->GetPeer().ToString(); if (!RPCAuthorized(authHeader.second, rpcRequest.user)) { LogPrintf("ThreadRPCServer incorrect password attempt from %s\n", jreq.peerAddr); @@ -335,7 +335,7 @@ bool StartHTTPRPC(const CoreContext& context) if (!InitRPCAuthentication()) return false; - auto handle_rpc = [&context](HTTPRequest* req, const std::string&) { return HTTPReq_JSONRPC(context, req); }; + auto handle_rpc = [context](HTTPRequest* req, const std::string&) { return HTTPReq_JSONRPC(context, req); }; RegisterHTTPHandler("/", true, handle_rpc); if (g_wallet_init_interface.HasWalletSupport()) { RegisterHTTPHandler("/wallet/", false, handle_rpc); diff --git a/src/init.cpp b/src/init.cpp index 005cca71c5..27b9b10dc6 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -960,7 +960,7 @@ static bool InitSanityCheck() return true; } -static bool AppInitServers(const CoreContext& context, NodeContext& node) +static bool AppInitServers(NodeContext& node) { const ArgsManager& args = *Assert(node.args); RPCServer::OnStarted(&OnRPCStarted); @@ -969,9 +969,9 @@ static bool AppInitServers(const CoreContext& context, NodeContext& node) return false; StartRPC(); node.rpc_interruption_point = RpcInterruptionPoint; - if (!StartHTTPRPC(context)) + if (!StartHTTPRPC(node)) return false; - if (args.GetBoolArg("-rest", DEFAULT_REST_ENABLE)) StartREST(context); + if (args.GetBoolArg("-rest", DEFAULT_REST_ENABLE)) StartREST(node); StartHTTPServer(); return true; } @@ -1536,7 +1536,7 @@ bool AppInitInterfaces(NodeContext& node) return true; } -bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) +bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) { const ArgsManager& args = *Assert(node.args); const CChainParams& chainparams = Params(); @@ -1643,7 +1643,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc */ if (args.GetBoolArg("-server", false)) { uiInterface.InitMessage_connect(SetRPCWarmupStatus); - if (!AppInitServers(context, node)) + if (!AppInitServers(node)) return InitError(_("Unable to start HTTP server. See debug log for details.")); } diff --git a/src/init.h b/src/init.h index d4316f9c87..a529770cb0 100644 --- a/src/init.h +++ b/src/init.h @@ -62,7 +62,7 @@ bool AppInitInterfaces(NodeContext& node); * @note This should only be done after daemonization. Call Shutdown() if this function fails. * @pre Parameters should be parsed and config file should be read, AppInitLockDataDirectory should have been called. */ -bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr); +bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr); void PrepareShutdown(NodeContext& node); /** diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 7339fa1a44..7c51e7255a 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -318,7 +318,7 @@ class NodeImpl : public Node } bool appInitMain(interfaces::BlockAndHeaderTipInfo* tip_info) override { - return AppInitMain(m_context_ref, *m_context, tip_info); + return AppInitMain(*m_context, tip_info); } void appShutdown() override { @@ -480,7 +480,8 @@ class NodeImpl : public Node CFeeRate getDustRelayFee() override { return ::dustRelayFee; } UniValue executeRpc(const std::string& command, const UniValue& params, const std::string& uri) override { - JSONRPCRequest req(m_context_ref); + JSONRPCRequest req; + req.context = *m_context; req.params = params; req.strMethod = command; req.URI = uri; @@ -581,15 +582,8 @@ class NodeImpl : public Node m_gov.setContext(context); m_llmq.setContext(context); m_masternodeSync.setContext(context); - - if (context) { - m_context_ref = *context; - } else { - m_context_ref = std::monostate{}; - } } NodeContext* m_context{nullptr}; - CoreContext m_context_ref; }; bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock& lock, const CChain& active) diff --git a/src/rest.cpp b/src/rest.cpp index 3ba29564fe..ad2f6a1049 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -346,7 +346,8 @@ static bool rest_chaininfo(const CoreContext& context, HTTPRequest* req, const s switch (rf) { case RetFormat::JSON: { - JSONRPCRequest jsonRequest(context); + JSONRPCRequest jsonRequest; + jsonRequest.context = context; jsonRequest.params = UniValue(UniValue::VARR); UniValue chainInfoObject = getblockchaininfo().HandleRequest(jsonRequest); std::string strJSON = chainInfoObject.write() + "\n"; @@ -724,7 +725,7 @@ static const struct { void StartREST(const CoreContext& context) { for (const auto& up : uri_prefixes) { - auto handler = [&context, up](HTTPRequest* req, const std::string& prefix) { return up.handler(context, req, prefix); }; + auto handler = [context, up](HTTPRequest* req, const std::string& prefix) { return up.handler(context, req, prefix); }; RegisterHTTPHandler(up.prefix, false, handler); } } diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp index 992b76e766..4c45c33c43 100644 --- a/src/rpc/request.cpp +++ b/src/rpc/request.cpp @@ -185,7 +185,7 @@ void JSONRPCRequest::parse(const UniValue& valRequest) throw JSONRPCError(RPC_INVALID_REQUEST, "Params must be an array or object"); } -const JSONRPCRequest JSONRPCRequest::squashed() const +JSONRPCRequest JSONRPCRequest::squashed() const { if (params.empty()) { return *this; diff --git a/src/rpc/request.h b/src/rpc/request.h index d849f6ee21..d9a669b78f 100644 --- a/src/rpc/request.h +++ b/src/rpc/request.h @@ -36,22 +36,11 @@ class JSONRPCRequest std::string URI; std::string authUser; std::string peerAddr; - const CoreContext& context; - - explicit JSONRPCRequest(const CoreContext& context) : id(NullUniValue), params(NullUniValue), context(context) {} - - //! Initializes request information from another request object and the - //! given context. The implementation should be updated if any members are - //! added or removed above. - JSONRPCRequest(const JSONRPCRequest& other, const CoreContext& context) - : id(other.id), strMethod(other.strMethod), params(other.params), mode(other.mode), URI(other.URI), - authUser(other.authUser), peerAddr(other.peerAddr), context(context) - { - } + CoreContext context; void parse(const UniValue& valRequest); // Returns new JSONRPCRequest with the first param "squashed' into strMethod - const JSONRPCRequest squashed() const; + JSONRPCRequest squashed() const; }; #endif // BITCOIN_RPC_REQUEST_H diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 75f9b96bd2..687530b49f 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -52,7 +52,8 @@ UniValue RPCTestingSetup::TransformParams(const UniValue& params, std::vector bool { transformed_params = request.params; return true; }, arg_names, /*unique_id=*/0}; table.appendCommand("method", &command); CoreContext context{m_node}; - JSONRPCRequest request(context); + JSONRPCRequest request; + request.context = context; request.strMethod = "method"; request.params = params; if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished(); @@ -66,7 +67,8 @@ UniValue RPCTestingSetup::CallRPC(std::string args) std::string strMethod = vArgs[0]; vArgs.erase(vArgs.begin()); CoreContext context{m_node}; - JSONRPCRequest request(context); + JSONRPCRequest request; + request.context = context; request.strMethod = strMethod; request.params = RPCConvertValues(strMethod, vArgs); if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished(); diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 9f18bf7e35..ac422a870d 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -573,7 +573,9 @@ class WalletLoaderImpl : public WalletLoader { for (const CRPCCommand& command : GetWalletRPCCommands()) { m_rpc_commands.emplace_back(command.category, command.name, [this, &command](const JSONRPCRequest& request, UniValue& result, bool last_handler) { - return command.actor({request, &m_context}, result, last_handler); + JSONRPCRequest wallet_request = request; + wallet_request.context = m_context; + return command.actor(wallet_request, result, last_handler); }, command.argNames, command.unique_id); m_rpc_handlers.emplace_back(m_context.chain->handleRpc(m_rpc_commands.back())); } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index c6fc807db1..6ec9cfbfd1 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -229,8 +229,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) key.pushKV("timestamp", newTip->GetBlockTimeMax() + TIMESTAMP_WINDOW + 1); key.pushKV("internal", UniValue(true)); keys.push_back(key); - CoreContext context{m_node}; - JSONRPCRequest request(context); + JSONRPCRequest request; request.params.setArray(); request.params.push_back(keys); @@ -281,9 +280,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) AddWallet(wallet); wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); } - CoreContext context{m_node}; - JSONRPCRequest request(context); - + JSONRPCRequest request; request.params.setArray(); request.params.push_back(backup_file); @@ -298,8 +295,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) LOCK(wallet->cs_wallet); wallet->SetupLegacyScriptPubKeyMan(); - CoreContext context{m_node}; - JSONRPCRequest request(context); + JSONRPCRequest request; request.params.setArray(); request.params.push_back(backup_file); AddWallet(wallet); @@ -324,7 +320,8 @@ BOOST_FIXTURE_TEST_CASE(rpc_getaddressinfo, TestChain100Setup) wallet->SetupLegacyScriptPubKeyMan(); AddWallet(wallet); CoreContext context{m_node}; - JSONRPCRequest request(context); + JSONRPCRequest request; + request.context = context; UniValue response; // test p2pkh @@ -798,7 +795,9 @@ class CreateTransactionTestSetup : public TestChain100Setup CoreContext context{m_node}; std::vector vecRecipients; for (auto entry : vecEntries) { - vecRecipients.push_back({GetScriptForDestination(DecodeDestination(getnewaddress().HandleRequest(JSONRPCRequest(context)).get_str())), entry.first, entry.second}); + JSONRPCRequest request; + request.context = context; + vecRecipients.push_back({GetScriptForDestination(DecodeDestination(getnewaddress().HandleRequest(request).get_str())), entry.first, entry.second}); } return vecRecipients; } From 878bce0f4523d678ac4ad4ccceec75beb6f07548 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 16 Jul 2024 15:29:10 +0000 Subject: [PATCH 16/21] docs: update SECURITY.md supported versions --- SECURITY.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 3a7bd95cf5..e88192ddc0 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -4,9 +4,9 @@ | Version | Supported | | ------- | ------------------ | -| 0.17 | :white_check_mark: | -| 0.16 | :white_check_mark: | -| < 0.16 | :x: | +| 21 | :white_check_mark: | +| 20.1 | :white_check_mark: | +| < 20.1 | :x: | ## Reporting a Vulnerability From 163d31861c14d37c0a407d70d10f847050b67e37 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 17 Jul 2024 16:31:30 +0000 Subject: [PATCH 17/21] wallet: unify HD chain generation in LegacyScriptPubKeyMan most of the steps are overlapping except for the encryption sections, which we can make contingent on supplying of the keying material. `res` isn't used anywhere since we plan on hard crashing if they fail anyway, so if we got that far, we've already succeeded. we will validate vMasterKey's size before starting the encryption routine to prevent a value outside of our control from hard-crashing the client. --- src/wallet/scriptpubkeyman.cpp | 58 +++++++++++++--------------------- src/wallet/scriptpubkeyman.h | 3 +- src/wallet/wallet.cpp | 5 +-- 3 files changed, 24 insertions(+), 42 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 678022e1f2..631fcac566 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -377,55 +377,41 @@ void LegacyScriptPubKeyMan::UpgradeKeyMetadata() } } -void LegacyScriptPubKeyMan::GenerateNewCryptedHDChain(const SecureString& secureMnemonic, const SecureString& secureMnemonicPassphrase, CKeyingMaterial vMasterKey) +void LegacyScriptPubKeyMan::GenerateNewHDChain(const SecureString& secureMnemonic, const SecureString& secureMnemonicPassphrase, std::optional vMasterKeyOpt) { assert(!m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); - - - CHDChain hdChainTmp; + CHDChain newHdChain; // NOTE: an empty mnemonic means "generate a new one for me" // NOTE: default mnemonic passphrase is an empty string - if (!hdChainTmp.SetMnemonic(secureMnemonic, secureMnemonicPassphrase, true)) { + if (!newHdChain.SetMnemonic(secureMnemonic, secureMnemonicPassphrase, /* fUpdateID = */ true)) { throw std::runtime_error(std::string(__func__) + ": SetMnemonic failed"); } - // add default account - hdChainTmp.AddAccount(); + // Add default account + newHdChain.AddAccount(); - // We need to safe chain for validation further - CHDChain hdChainPrev = hdChainTmp; - bool res = EncryptHDChain(vMasterKey, hdChainTmp); - assert(res); - res = LoadHDChain(hdChainTmp); - assert(res); + // Encryption routine if vMasterKey has been supplied + if (vMasterKeyOpt.has_value()) { + auto vMasterKey = vMasterKeyOpt.value(); + if (vMasterKey.size() != WALLET_CRYPTO_KEY_SIZE) { + throw std::runtime_error(strprintf("%s : invalid vMasterKey size, got %zd (expected %lld)", __func__, vMasterKey.size(), WALLET_CRYPTO_KEY_SIZE)); + } - CHDChain hdChainCrypted; - res = GetHDChain(hdChainCrypted); - assert(res); + // Maintain an unencrypted copy of the chain for sanity checking + CHDChain prevHdChain{newHdChain}; - // ids should match, seed hashes should not - assert(hdChainPrev.GetID() == hdChainCrypted.GetID()); - assert(hdChainPrev.GetSeedHash() != hdChainCrypted.GetSeedHash()); + bool res = EncryptHDChain(vMasterKey, newHdChain); + assert(res); + res = LoadHDChain(newHdChain); + assert(res); + res = GetHDChain(newHdChain); + assert(res); - if (!AddHDChainSingle(hdChainCrypted)) { - throw std::runtime_error(std::string(__func__) + ": AddHDChainSingle failed"); + // IDs should match, seed hashes should not + assert(prevHdChain.GetID() == newHdChain.GetID()); + assert(prevHdChain.GetSeedHash() != newHdChain.GetSeedHash()); } -} - -void LegacyScriptPubKeyMan::GenerateNewHDChain(const SecureString& secureMnemonic, const SecureString& secureMnemonicPassphrase) -{ - assert(!m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); - CHDChain newHdChain; - - // NOTE: an empty mnemonic means "generate a new one for me" - // NOTE: default mnemonic passphrase is an empty string - if (!newHdChain.SetMnemonic(secureMnemonic, secureMnemonicPassphrase, true)) { - throw std::runtime_error(std::string(__func__) + ": SetMnemonic failed"); - } - - // add default account - newHdChain.AddAccount(); if (!AddHDChainSingle(newHdChain)) { throw std::runtime_error(std::string(__func__) + ": AddHDChainSingle failed"); diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 9c639b73eb..5ebb757628 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -463,8 +463,7 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv bool GetDecryptedHDChain(CHDChain& hdChainRet); /* Generates a new HD chain */ - void GenerateNewCryptedHDChain(const SecureString& secureMnemonic, const SecureString& secureMnemonicPassphrase, CKeyingMaterial vMasterKey); - void GenerateNewHDChain(const SecureString& secureMnemonic, const SecureString& secureMnemonicPassphrase); + void GenerateNewHDChain(const SecureString& secureMnemonic, const SecureString& secureMnemonicPassphrase, std::optional vMasterKey = std::nullopt); /** * Explicitly make the wallet learn the related scripts for outputs to the diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 29d6104270..68e1919955 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -5677,16 +5677,13 @@ bool CWallet::GenerateNewHDChainEncrypted(const SecureString& secureMnemonic, co } - spk_man->GenerateNewCryptedHDChain(secureMnemonic, secureMnemonicPassphrase, vMasterKey); + spk_man->GenerateNewHDChain(secureMnemonic, secureMnemonicPassphrase, vMasterKey); Lock(); if (!Unlock(secureWalletPassphrase)) { // this should never happen throw std::runtime_error(std::string(__func__) + ": Unlock failed"); } - if (!spk_man->NewKeyPool()) { - throw std::runtime_error(std::string(__func__) + ": NewKeyPool failed"); - } return true; } From 619b640a774337af5a3ad02551bc2585f2b0c919 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 15 Jul 2024 19:15:45 +0000 Subject: [PATCH 18/21] wallet: unify HD chain generation in CWallet additionally, let's do the lock-unlock validation test *before* trying to generate a new chain. as a bonus, it lets us make sure that we encrypt the HD chain using a key that's actually capable of unlocking the wallet. also, `upgradetohd` never checked if the passphrase was handed to us (which matters in encrypted blank wallets) before calling UpgradeToHD, so let's check it for them. --- src/wallet/wallet.cpp | 77 ++++++++++++++++++++++++------------------- src/wallet/wallet.h | 2 +- 2 files changed, 45 insertions(+), 34 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 68e1919955..a5daf29e21 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -337,7 +337,7 @@ std::shared_ptr CreateWallet(interfaces::Chain& chain, interfaces::Coin // TODO: drop this condition after removing option to create non-HD wallets // related backport bitcoin#11250 if (wallet->GetVersion() >= FEATURE_HD) { - if (!wallet->GenerateNewHDChainEncrypted(/*secureMnemonic=*/"", /*secureMnemonicPassphrase=*/"", passphrase)) { + if (!wallet->GenerateNewHDChain(/*secureMnemonic=*/"", /*secureMnemonicPassphrase=*/"", passphrase)) { error = Untranslated("Error: Failed to generate encrypted HD wallet"); status = DatabaseStatus::FAILED_CREATE; return nullptr; @@ -5022,18 +5022,9 @@ bool CWallet::UpgradeToHD(const SecureString& secureMnemonic, const SecureString WalletLogPrintf("Upgrading wallet to HD\n"); SetMinVersion(FEATURE_HD); - // TODO: replace to GetLegacyScriptPubKeyMan() when `sethdseed` is backported - auto spk_man = GetOrCreateLegacyScriptPubKeyMan(); - bool prev_encrypted = IsCrypted(); - // TODO: unify encrypted and plain chains usages here - if (prev_encrypted) { - if (!GenerateNewHDChainEncrypted(secureMnemonic, secureMnemonicPassphrase, secureWalletPassphrase)) { - error = Untranslated("Failed to generate encrypted HD wallet"); - return false; - } - Lock(); - } else { - spk_man->GenerateNewHDChain(secureMnemonic, secureMnemonicPassphrase); + if (!GenerateNewHDChain(secureMnemonic, secureMnemonicPassphrase, secureWalletPassphrase)) { + error = Untranslated("Failed to generate HD wallet"); + return false; } return true; } @@ -5651,39 +5642,59 @@ void CWallet::ConnectScriptPubKeyManNotifiers() } } -bool CWallet::GenerateNewHDChainEncrypted(const SecureString& secureMnemonic, const SecureString& secureMnemonicPassphrase, const SecureString& secureWalletPassphrase) +bool CWallet::GenerateNewHDChain(const SecureString& secureMnemonic, const SecureString& secureMnemonicPassphrase, const SecureString& secureWalletPassphrase) { auto spk_man = GetLegacyScriptPubKeyMan(); if (!spk_man) { throw std::runtime_error(strprintf("%s: spk_man is not available", __func__)); } - if (!HasEncryptionKeys()) { - return false; - } + if (IsCrypted()) { + if (secureWalletPassphrase.empty()) { + throw std::runtime_error(strprintf("%s: encrypted but supplied empty wallet passphrase", __func__)); + } - CCrypter crypter; - CKeyingMaterial vMasterKey; + bool is_locked = IsLocked(); - LOCK(cs_wallet); - for (const CWallet::MasterKeyMap::value_type& pMasterKey : mapMasterKeys) { - if (!crypter.SetKeyFromPassphrase(secureWalletPassphrase, pMasterKey.second.vchSalt, pMasterKey.second.nDeriveIterations, pMasterKey.second.nDerivationMethod)) { - return false; - } - // get vMasterKey to encrypt new hdChain - if (crypter.Decrypt(pMasterKey.second.vchCryptedKey, vMasterKey)) { - break; + CCrypter crypter; + CKeyingMaterial vMasterKey; + + // We are intentionally re-locking the wallet so we can validate vMasterKey + // by verifying if it can unlock the wallet + Lock(); + + LOCK(cs_wallet); + for (const auto& [_, master_key] : mapMasterKeys) { + CKeyingMaterial _vMasterKey; + if (!crypter.SetKeyFromPassphrase(secureWalletPassphrase, master_key.vchSalt, master_key.nDeriveIterations, master_key.nDerivationMethod)) { + return false; + } + // Try another key if it cannot be decrypted or the key is incapable of encrypting + if (!crypter.Decrypt(master_key.vchCryptedKey, _vMasterKey) || _vMasterKey.size() != WALLET_CRYPTO_KEY_SIZE) { + continue; + } + // The likelihood of the plaintext being gibberish but also of the expected size is low but not zero. + // If it can unlock the wallet, it's a good key. + if (Unlock(_vMasterKey)) { + vMasterKey = _vMasterKey; + break; + } } - } + // We got a gibberish key... + if (vMasterKey.empty()) { + throw std::runtime_error(strprintf("%s: supplied incorrect passphrase", __func__)); + } - spk_man->GenerateNewHDChain(secureMnemonic, secureMnemonicPassphrase, vMasterKey); + spk_man->GenerateNewHDChain(secureMnemonic, secureMnemonicPassphrase, vMasterKey); - Lock(); - if (!Unlock(secureWalletPassphrase)) { - // this should never happen - throw std::runtime_error(std::string(__func__) + ": Unlock failed"); + if (is_locked) { + Lock(); + } + } else { + spk_man->GenerateNewHDChain(secureMnemonic, secureMnemonicPassphrase); } + return true; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b2f967cf11..0ac9666989 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1343,7 +1343,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati // TODO: move it to scriptpubkeyman /* Generates a new HD chain */ - bool GenerateNewHDChainEncrypted(const SecureString& secureMnemonic, const SecureString& secureMnemonicPassphrase, const SecureString& secureWalletPassphrase); + bool GenerateNewHDChain(const SecureString& secureMnemonic, const SecureString& secureMnemonicPassphrase, const SecureString& secureWalletPassphrase = ""); /* Returns true if the wallet can give out new addresses. This means it has keys in the keypool or can generate new keys */ bool CanGetAddresses(bool internal = false) const; From 69c37f4ec2570c85f3e8b668da8bd30400a54e5f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 17 Jul 2024 13:20:22 +0000 Subject: [PATCH 19/21] rpc: make sure `upgradetohd` always has the passphrase for `UpgradeToHD` earlier it was possible to make it all the way to `EncryptSecret` without actually having the passphrase in hand until being told off by `CCrypter::SetKey`, we should avoid that. also, let's get rid of checks that `UpgradeToHD` is now taking responsibility for. no point in checking if the wallet is unlocked as it has no bearing on your ability to upgrade the wallet. --- src/wallet/rpcwallet.cpp | 22 +++++++++++++--------- src/wallet/wallet.cpp | 4 +++- test/functional/wallet_upgradetohd.py | 4 ++-- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index f9b29f28e5..2107cec953 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2779,11 +2779,13 @@ static RPCHelpMan upgradetohd() { return RPCHelpMan{"upgradetohd", "\nUpgrades non-HD wallets to HD.\n" + "\nIf your wallet is encrypted, the wallet passphrase must be supplied. Supplying an incorrect" + "\npassphrase may result in your wallet getting locked.\n" "\nWarning: You will need to make a new backup of your wallet after setting the HD wallet mnemonic.\n", { {"mnemonic", RPCArg::Type::STR, /* default */ "", "Mnemonic as defined in BIP39 to use for the new HD wallet. Use an empty string \"\" to generate a new random mnemonic."}, {"mnemonicpassphrase", RPCArg::Type::STR, /* default */ "", "Optional mnemonic passphrase as defined in BIP39"}, - {"walletpassphrase", RPCArg::Type::STR, /* default */ "", "If your wallet is encrypted you must have your wallet passphrase here. If your wallet is not encrypted specifying wallet passphrase will trigger wallet encryption."}, + {"walletpassphrase", RPCArg::Type::STR, /* default */ "", "If your wallet is encrypted you must have your wallet passphrase here. If your wallet is not encrypted, specifying wallet passphrase will trigger wallet encryption."}, {"rescan", RPCArg::Type::BOOL, /* default */ "false if mnemonic is empty", "Whether to rescan the blockchain for missing transactions or not"}, }, RPCResult{ @@ -2793,6 +2795,7 @@ static RPCHelpMan upgradetohd() HelpExampleCli("upgradetohd", "") + HelpExampleCli("upgradetohd", "\"mnemonicword1 ... mnemonicwordN\"") + HelpExampleCli("upgradetohd", "\"mnemonicword1 ... mnemonicwordN\" \"mnemonicpassphrase\"") + + HelpExampleCli("upgradetohd", "\"mnemonicword1 ... mnemonicwordN\" \"\" \"walletpassphrase\"") + HelpExampleCli("upgradetohd", "\"mnemonicword1 ... mnemonicwordN\" \"mnemonicpassphrase\" \"walletpassphrase\"") }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue @@ -2803,17 +2806,17 @@ static RPCHelpMan upgradetohd() bool generate_mnemonic = request.params[0].isNull() || request.params[0].get_str().empty(); SecureString secureWalletPassphrase; secureWalletPassphrase.reserve(100); - // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string) - // Alternately, find a way to make request.params[0] mlock()'d to begin with. - if (!request.params[2].isNull()) { - secureWalletPassphrase = request.params[2].get_str().c_str(); - if (!pwallet->Unlock(secureWalletPassphrase)) { - throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "The wallet passphrase entered was incorrect"); + + if (request.params[2].isNull()) { + if (pwallet->IsCrypted()) { + throw JSONRPCError(RPC_WALLET_UNLOCK_NEEDED, "Error: Wallet encrypted but passphrase not supplied to RPC."); } + } else { + // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string) + // Alternately, find a way to make request.params[0] mlock()'d to begin with. + secureWalletPassphrase = request.params[2].get_str().c_str(); } - EnsureWalletIsUnlocked(pwallet.get()); - SecureString secureMnemonic; secureMnemonic.reserve(256); if (!generate_mnemonic) { @@ -2825,6 +2828,7 @@ static RPCHelpMan upgradetohd() if (!request.params[1].isNull()) { secureMnemonicPassphrase = request.params[1].get_str().c_str(); } + // TODO: breaking changes kept for v21! // instead upgradetohd let's use more straightforward 'sethdseed' constexpr bool is_v21 = false; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a5daf29e21..36cdf3f171 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -5683,7 +5683,9 @@ bool CWallet::GenerateNewHDChain(const SecureString& secureMnemonic, const Secur // We got a gibberish key... if (vMasterKey.empty()) { - throw std::runtime_error(strprintf("%s: supplied incorrect passphrase", __func__)); + // Mimicking the error message of RPC_WALLET_PASSPHRASE_INCORRECT as it's possible + // that the user may see this error when interacting with the upgradetohd RPC + throw std::runtime_error("Error: The wallet passphrase entered was incorrect"); } spk_man->GenerateNewHDChain(secureMnemonic, secureMnemonicPassphrase, vMasterKey); diff --git a/test/functional/wallet_upgradetohd.py b/test/functional/wallet_upgradetohd.py index 10bb44f6b6..b1d09534b8 100755 --- a/test/functional/wallet_upgradetohd.py +++ b/test/functional/wallet_upgradetohd.py @@ -190,8 +190,8 @@ def run_test(self): node.stop() node.wait_until_stopped() self.start_node(0, extra_args=['-rescan']) - assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first.", node.upgradetohd, mnemonic) - assert_raises_rpc_error(-14, "The wallet passphrase entered was incorrect", node.upgradetohd, mnemonic, "", "wrongpass") + assert_raises_rpc_error(-13, "Error: Wallet encrypted but passphrase not supplied to RPC.", node.upgradetohd, mnemonic) + assert_raises_rpc_error(-1, "Error: The wallet passphrase entered was incorrect", node.upgradetohd, mnemonic, "", "wrongpass") assert node.upgradetohd(mnemonic, "", walletpass) assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first.", node.dumphdinfo) node.walletpassphrase(walletpass, 100) From a376e9357a20cee56f6b9f17e524f16399ad6b8c Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 18 Jul 2024 02:13:28 +0000 Subject: [PATCH 20/21] fix: init `g_txindex` only once, downgrade from assert to exception `g_txindex` should be initialized in `TestChainSetup`'s constructor but in bitcoin#19806 (dash#5236), when portions of the constructor were split into `mineBlocks()`, `g_txindex`'s init was left behind in the latter instead of the former. This meant that every `mineBlocks()` call would re-create a `TxIndex` instance, which is not intended behaviour. Also, a runtime exception is more appropriate and closer to the usage of `BOOST_REQUIRE` in other index `Start()` calls than the harsher `assert` --- src/test/util/setup_common.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 6bd00529f0..0d831147c5 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -323,6 +323,13 @@ TestChainSetup::TestChainSetup(int num_blocks, const std::vector& e // Generate a num_blocks length chain: this->mineBlocks(num_blocks); + // Initialize transaction index *after* chain has been constructed + g_txindex = std::make_unique(1 << 20, true); + if (!g_txindex->Start(m_node.chainman->ActiveChainstate())) { + throw std::runtime_error("TxIndex::Start() failed."); + } + IndexWaitSynced(*g_txindex); + CCheckpointData checkpoints{ { /* TestChainDATSetup */ @@ -361,11 +368,10 @@ void TestChainSetup::mineBlocks(int num_blocks) m_coinbase_txns.push_back(b.vtx[0]); } - g_txindex = std::make_unique(1 << 20, true); - assert(g_txindex->Start(m_node.chainman->ActiveChainstate())); - // Allow tx index to catch up with the block index. - IndexWaitSynced(*g_txindex); + if (g_txindex) { + IndexWaitSynced(*g_txindex); + } } CBlock TestChainSetup::CreateAndProcessBlock(const std::vector& txns, const CScript& scriptPubKey) From 77915de40aba32ef8dcd69d40a2ea12f4a7454bf Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 18 Jul 2024 01:36:10 +0000 Subject: [PATCH 21/21] test: run `Interrupt()` before `Stop()`, add additional sanity check --- src/test/util/setup_common.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 0d831147c5..85281ef164 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -325,6 +325,7 @@ TestChainSetup::TestChainSetup(int num_blocks, const std::vector& e // Initialize transaction index *after* chain has been constructed g_txindex = std::make_unique(1 << 20, true); + assert(!g_txindex->BlockUntilSyncedToCurrentChain()); if (!g_txindex->Start(m_node.chainman->ActiveChainstate())) { throw std::runtime_error("TxIndex::Start() failed."); } @@ -506,6 +507,7 @@ TestChainSetup::~TestChainSetup() // we might be destroying it while scheduler still has some work for it // e.g. via BlockConnected signal IndexWaitSynced(*g_txindex); + g_txindex->Interrupt(); g_txindex->Stop(); SyncWithValidationInterfaceQueue(); g_txindex.reset();