Merge bitcoin#21035: Remove pointer cast in CRPCTable::dumpArgMap

9048c58 Remove pointer cast in CRPCTable::dumpArgMap (Russell Yanofsky)
14f3d9b refactor: Add RPC server ExecuteCommands function (Russell Yanofsky)
6158a6d 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 bitcoin#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 bitcoin#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 9048c58 👑

Tree-SHA512: cd1a01c1daa5bde2c2455b63548371ee4cf39688313969ad2016d9a0fd4344102e3fd43034058f253364518e9632d57cf21abffad0d6a2c0c94b7a6921cbe615

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)
     {
     }

src/rpc/server.cpp

@@ -90,7 +90,7 @@ std::string CRPCTable::help(const std::string& strCommand, const std::string& st
     sort(vCommands.begin(), vCommands.end());
 
     JSONRPCRequest jreq = helpreq;
-    jreq.fHelp = true;
+    jreq.mode = JSONRPCRequest::GET_HELP;
     jreq.params = UniValue();
 
     for (const std::pair<std::string, const CRPCCommand*>& command : vCommands)
@@ -479,6 +479,16 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c
     return out;
 }
 
+static bool ExecuteCommands(const std::vector<const CRPCCommand*>& commands, const JSONRPCRequest& request, UniValue& result)
+{
+    for (const auto& command : commands) {
+        if (ExecuteCommand(*command, request, result, &command == &commands.back())) {
+            return true;
+        }
+    }
+    return false;
+}
+
 UniValue CRPCTable::execute(const JSONRPCRequest &request) const
 {
     // Return immediately if in warmup
@@ -492,10 +502,8 @@ UniValue CRPCTable::execute(const JSONRPCRequest &request) const
     auto it = mapCommands.find(request.strMethod);
     if (it != mapCommands.end()) {
         UniValue result;
-        for (const auto& command : it->second) {
-            if (ExecuteCommand(*command, request, result, &command == &it->second.back(), mapPlatformRestrictions)) {
-                return result;
-            }
+        if (ExecuteCommands(it->second, request, result, mapPlatformRestrictions)) {
+            return result;
         }
     }
     throw JSONRPCError(RPC_METHOD_NOT_FOUND, "Method not found");

src/rpc/util.cpp

@@ -432,6 +432,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;

src/rpc/util.h

@@ -336,12 +336,10 @@ class RPCHelpMan
     using RPCMethodImpl = std::function<UniValue(const RPCHelpMan&, const JSONRPCRequest&)>;
     RPCHelpMan(std::string name, std::string description, std::vector<RPCArg> args, RPCResults results, RPCExamples examples, RPCMethodImpl fun);
 
+    UniValue HandleRequest(const JSONRPCRequest& request);
     std::string ToString() 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;
     /**

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);