aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rwxr-xr-xci/lint/06_script.sh1
-rw-r--r--src/rpc/server.cpp19
-rw-r--r--src/rpc/server.h4
-rw-r--r--src/rpc/util.cpp18
-rw-r--r--src/rpc/util.h2
-rwxr-xr-xtest/functional/rpc_help.py64
-rwxr-xr-xtest/lint/check-rpc-mappings.py162
7 files changed, 106 insertions, 164 deletions
diff --git a/ci/lint/06_script.sh b/ci/lint/06_script.sh
index de89a09d6a..e38cfe8eef 100755
--- a/ci/lint/06_script.sh
+++ b/ci/lint/06_script.sh
@@ -21,7 +21,6 @@ test/lint/git-subtree-check.sh src/univalue
test/lint/git-subtree-check.sh src/leveldb
test/lint/git-subtree-check.sh src/crc32c
test/lint/check-doc.py
-test/lint/check-rpc-mappings.py .
test/lint/lint-all.sh
if [ "$CIRRUS_REPO_FULL_NAME" = "bitcoin/bitcoin" ] && [ -n "$CIRRUS_CRON" ]; then
diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp
index f32d9abac6..e8abc020da 100644
--- a/src/rpc/server.cpp
+++ b/src/rpc/server.cpp
@@ -144,8 +144,13 @@ static RPCHelpMan help()
[&](const RPCHelpMan& self, const JSONRPCRequest& jsonRequest) -> UniValue
{
std::string strCommand;
- if (jsonRequest.params.size() > 0)
+ if (jsonRequest.params.size() > 0) {
strCommand = jsonRequest.params[0].get_str();
+ }
+ if (strCommand == "dump_all_command_conversions") {
+ // Used for testing only, undocumented
+ return tableRPC.dumpArgMap();
+ }
return tableRPC.help(strCommand, jsonRequest);
},
@@ -479,6 +484,18 @@ std::vector<std::string> 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 040192b455..d2f44334ae 100644
--- a/src/rpc/server.h
+++ b/src/rpc/server.h
@@ -147,6 +147,10 @@ public:
*/
std::vector<std::string> 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 31072114da..bfdba5253c 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -549,6 +549,24 @@ 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<std::string> arg_names;
+ boost::split(arg_names, arg.m_names, boost::is_any_of("|"));
+ 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 942c243718..444a013ca1 100644
--- a/src/rpc/util.h
+++ b/src/rpc/util.h
@@ -336,6 +336,8 @@ public:
RPCHelpMan(std::string name, std::string description, std::vector<RPCArg> 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/test/functional/rpc_help.py b/test/functional/rpc_help.py
index c83157a843..1eefd109f8 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 @@ class HelpRpcTest(BitcoinTestFramework):
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 0a4cc875d0..0000000000
--- a/test/lint/check-rpc-mappings.py
+++ /dev/null
@@ -1,162 +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/mining.cpp",
- "src/rpc/misc.cpp",
- "src/rpc/net.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)
- 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()