diff options
author | MarcoFalke <falke.marco@gmail.com> | 2020-11-19 14:18:56 +0100 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2020-11-19 14:19:05 +0100 |
commit | 71d068db405883d35f3800aedd4e3a2d4e854762 (patch) | |
tree | dc5c595244d5828dc564ffde83544d12b107d824 | |
parent | 9ab9665c74dd27d80f44f0713daf3694bbe700e4 (diff) | |
parent | faaf9c58e4aa809019d4ca12747dd47411988e37 (diff) |
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
-rw-r--r-- | src/qt/test/rpcnestedtests.cpp | 25 | ||||
-rw-r--r-- | src/rpc/mining.cpp | 5 | ||||
-rw-r--r-- | src/rpc/misc.cpp | 2 | ||||
-rw-r--r-- | src/rpc/net.cpp | 6 | ||||
-rw-r--r-- | src/rpc/server.h | 9 | ||||
-rwxr-xr-x | test/lint/lint-rpc-help.sh | 24 |
6 files changed, 20 insertions, 51 deletions
diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index de1fbcb94c..ea7b5f0c9e 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -14,17 +14,26 @@ #include <QDir> #include <QtGlobal> -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 6522c0d73e..7d45ad9434 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -242,12 +242,7 @@ static RPCHelpMan generatetodescriptor() 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 0c982317f5..b3102a236d 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -624,8 +624,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 f98ea63782..e72ef24d12 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -288,10 +288,10 @@ static RPCHelpMan addnode() std::string strCommand; if (!request.params[1].isNull()) strCommand = request.params[1].get_str(); - if (request.fHelp || request.params.size() != 2 || - (strCommand != "onetry" && strCommand != "add" && strCommand != "remove")) + if (strCommand != "onetry" && strCommand != "add" && strCommand != "remove") { throw std::runtime_error( self.ToString()); + } NodeContext& node = EnsureNodeContext(request.context); if(!node.connman) @@ -628,7 +628,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()); } NodeContext& node = EnsureNodeContext(request.context); diff --git a/src/rpc/server.h b/src/rpc/server.h index b2358ac5b2..7d13edb8b0 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -85,7 +85,6 @@ void RPCUnsetTimerInterface(RPCTimerInterface *iface); */ void RPCRunLater(const std::string& name, std::function<void()> func, int64_t nSeconds); -typedef UniValue(*rpcfn_type)(const JSONRPCRequest& jsonRequest); typedef RPCHelpMan (*RpcMethodFnType)(); class CRPCCommand @@ -116,14 +115,6 @@ public: 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<const char*> 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; Actor actor; diff --git a/test/lint/lint-rpc-help.sh b/test/lint/lint-rpc-help.sh deleted file mode 100755 index faac5d43e2..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 --perl-regexp --null-data --only-matching 'runtime_error\(\n\s*".*\\n"\n' $(git ls-files -- "*.cpp")) -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} |