Merge #10753: test: Check RPC argument mapping

77aa9e59e test: Check RPC argument mapping (Wladimir J. van der Laan)

Pull request description:

  Parse the dispatch tables from the server implementation files, and the conversion table from the client (see #10751).

  Perform the following consistency checks:

  - Arguments defined in conversion table, must be present in dispatch table. If not, it was probably forgotten to add them to the dispatch table, and they will not work.

  - Arguments defined in conversion table must have the same names as in the dispatch table. If not, they will not work.

  - 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.

  Any of these results in an error.

  It also performs a consistency check to see if the same named argument is sometimes converted, and sometimes not. E.g. one RPC call might have a 'verbose' argument that is converted,
  another RPC call might have one that is not converted. This is not necessarily wrong, but points at a possible error (as well as makes the API harder to memorize) - so it is emitted as a warning (could upgrade this to error).

  This test is added to travis and run when `CHECK_DOC`. Currently fails with the following output:
  ```
  * Checking consistency between dispatch tables and vRPCConvertParams
  ERROR: createrawtransaction argument 3 (named optintorbf in vRPCConvertParams) is not defined in dispatch table
  ERROR: getblock argument ['verbosity', 'verbose'] has conflicts in vRPCConvertParams conversion specifier [True, False]
  WARNING: conversion mismatch for argument named verbose ([('getblock', False), ('getblockheader', True), ('getmempoolancestors', True), ('getmempooldescendants', True), ('getrawmempool', True), ('getrawtransaction', True)])
  ```
  - ~#10698 fixes the first ERROR~
  - #10747 fixes the second ERROR, as well as the WARNING

  Update: #10698 was merged, leaving:
  ```
  * Checking consistency between dispatch tables and vRPCConvertParams
  ERROR: getblock argument ['verbosity', 'verbose'] has conflicts in vRPCConvertParams conversion specifier [True, False]
  WARNING: conversion mismatch for argument named verbose ([('getblock', False), ('getblockheader', True), ('getmempoolancestors', True), ('getmempooldescendants', True), ('getrawmempool', True), ('getrawtransaction', True)])
  ```

Tree-SHA512: feabebfbeda5d4613b2b9d5265aa6bde4e1a0235297ffd48fa415ad7edc531d9ed7913fe76d191ac60d481a915a326f216bc93de3c671e45e1d14e97d07dea7a

.travis.yml

@@ -48,6 +48,7 @@ install:
 before_script:
     - if [ "$CHECK_DOC" = 1 -a "$TRAVIS_EVENT_TYPE" = "pull_request" ]; then contrib/devtools/commit-script-check.sh $TRAVIS_COMMIT_RANGE; fi
     - if [ "$CHECK_DOC" = 1 ]; then contrib/devtools/check-doc.py; fi
+    - if [ "$CHECK_DOC" = 1 ]; then contrib/devtools/check-rpc-mappings.py .; fi
     - unset CC; unset CXX
     - mkdir -p depends/SDKs depends/sdk-sources
     - if [ -n "$OSX_SDK" -a ! -f depends/sdk-sources/MacOSX${OSX_SDK}.sdk.tar.gz ]; then curl --location --fail $SDK_URL/MacOSX${OSX_SDK}.sdk.tar.gz -o depends/sdk-sources/MacOSX${OSX_SDK}.sdk.tar.gz; fi

contrib/devtools/check-rpc-mappings.py

@@ -0,0 +1,158 @@
+#!/usr/bin/env python3
+# Copyright (c) 2017 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") as f:
+        for line in f:
+            line = line.rstrip()
+            if not in_rpcs:
+                if re.match("static const CRPCCommand .*\[\] =", line):
+                    in_rpcs = True
+            else:
+                if line.startswith('};'):
+                    in_rpcs = False
+                elif '{' in line and '"' in line:
+                    m = re.search('{ *("[^"]*"), *("[^"]*"), *&([^,]*), *{([^}]*)} *},', 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, "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") 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('{ *("[^"]*"), *([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
+    return cmds
+
+def main():
+    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()