refactor: simplify implementation of RPC composite commands

src/interfaces/node.h

@@ -253,7 +253,7 @@ public:
     virtual UniValue executeRpc(const std::string& command, const UniValue& params, const std::string& uri) = 0;
 
     //! List rpc commands.
-    virtual std::vector<std::pair<std::string, std::string>> listRpcCommands() = 0;
+    virtual std::vector<std::string> listRpcCommands() = 0;
 
     //! Set RPC timer interface if unset.
     virtual void rpcSetTimerInterfaceIfUnset(RPCTimerInterface* iface) = 0;

src/node/interfaces.cpp

@@ -486,7 +486,7 @@ public:
         req.URI = uri;
         return ::tableRPC.execute(req);
     }
-    std::vector<std::pair<std::string, std::string>> listRpcCommands() override { return ::tableRPC.listCommands(); }
+    std::vector<std::string> 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 @@ public:
                 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);
         }
     }
 

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<std::pair<std::string, std::string>> commandList = m_node.listRpcCommands();
+        std::vector<std::string> commandList = m_node.listRpcCommands();
         for (size_t i = 0; i < commandList.size(); ++i)
         {
-            std::string command = commandList[i].first;
+            wordList << commandList[i].c_str();
-            if (!commandList[i].second.empty()) {
+            wordList << ("help " + commandList[i]).c_str();
-                command = command + " " + commandList[i].second;
-            }
-            wordList << command.c_str();
-            wordList << ("help " + command).c_str();
         }
 
         wordList << "help-console";

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

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

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

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

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

src/rpc/server.cpp

@@ -85,7 +85,7 @@ void RPCServer::OnStopped(std::function<void ()> 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<std::pair<std::string, const CRPCCommand*> > 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()) {
-    if (it == mapCommands.end() && !subcommand.empty()) {
+        it = mapCommands.find(request.strMethod);
-        subcommand = "";
+        subcommand.clear();
-        it = mapCommands.find(std::make_pair(request.strMethod, subcommand));
     }
     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<std::pair<std::string, std::string>> CRPCTable::listCommands() const
+std::vector<std::string> CRPCTable::listCommands() const
 {
-    std::vector<std::pair<std::string, std::string>> commandList;
+    std::vector<std::string> 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);

src/rpc/server.h

@@ -94,8 +94,8 @@ public:
     using Actor = std::function<bool(const JSONRPCRequest& request, UniValue& result, bool last_handler)>;
 
     //! Constructor taking Actor callback supporting multiple handlers.
-    CRPCCommand(std::string category, std::string name, std::string subname, Actor actor, std::vector<std::string> args, intptr_t unique_id)
+    CRPCCommand(std::string category, std::string name, Actor actor, std::vector<std::string> args, intptr_t unique_id)
-        : category(std::move(category)), name(std::move(name)), subname(subname), actor(std::move(actor)), argNames(std::move(args)),
+        : category(std::move(category)), name(std::move(name)), actor(std::move(actor)), argNames(std::move(args)),
           unique_id(unique_id)
     {
     }
@@ -105,35 +105,14 @@ public:
         : 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<std::string> 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<std::string> argNames;
     intptr_t unique_id;
@@ -145,11 +124,11 @@ public:
 class CRPCTable
 {
 private:
-    std::map<std::pair<std::string, std::string>, std::vector<const CRPCCommand*>> mapCommands;
+    std::map<std::string, std::vector<const CRPCCommand*>> mapCommands;
     std::multimap<std::string, std::vector<UniValue>> 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 @@ public:
     * Returns a list of registered commands
     * @returns List of registered commands.
     */
-    std::vector<std::pair<std::string, std::string>> listCommands() const;
+    std::vector<std::string> listCommands() const;
 
     /**
      * Return all named arguments that need to be converted by the client from string to another JSON type
@@ -185,8 +164,7 @@ public:
      * 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 CRPCCommand* pcmd);
-    bool removeCommand(const std::string& name, const std::string& subname, const CRPCCommand* pcmd);
 };
 
 bool IsDeprecatedRPCEnabled(const std::string& method);

src/test/rpc_tests.cpp

@@ -49,7 +49,7 @@ UniValue RPCTestingSetup::TransformParams(const UniValue& params, std::vector<st
 {
     UniValue transformed_params;
     CRPCTable table;
-    CRPCCommand command{"category", "method", "subcommand", [&](const JSONRPCRequest& request, UniValue&, bool) -> 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);

src/wallet/interfaces.cpp

@@ -572,7 +572,7 @@ public:
     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()));