Merge bitcoin/bitcoin#29277: RPC: access RPC arguments by name

30a6c99935 rpc: access some args by name (stickies-v)
bbb31269bf rpc: add named arg helper (stickies-v)
13525e0c24 rpc: add arg helper unit test (stickies-v)

Pull request description:

  Adds string overloads for the `RPCHelpMan::Arg` and `RPCHelpMan::MaybeArg` helpers to be able to access RPC arguments by name instead of index number. Especially in RPCs with a large number of parameters, this can be quite helpful.

  Example usage:
  ```cpp
  const auto action{self.Arg<std::string>("action")};
  ```

  Most of the LoC is adding test coverage and documentation updates. No behaviour change.

  An alternative approach to #27788 with significantly less overhaul.

ACKs for top commit:
  fjahr:
    Code review ACK 30a6c99935
  maflcko:
    ACK 30a6c99935 🥑
  ryanofsky:
    Code review ACK 30a6c99935. Nice change! Implementation is surprisingly simple and additional unit test coverage is welcome, too.

Tree-SHA512: 4904f5f914fe1d421d32f60edb7c5a028c8ea0f140a2f207a106b4752d441164e073066a6bf2e17693f859fe847815a96609d3cf521e0ac4178d8cd09362ea3d

src/rpc/blockchain.cpp

@@ -2174,7 +2174,8 @@ static RPCHelpMan scantxoutset()
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
     UniValue result(UniValue::VOBJ);
-    if (request.params[0].get_str() == "status") {
+    const auto action{self.Arg<std::string>("action")};
+    if (action == "status") {
         CoinsViewScanReserver reserver;
         if (reserver.reserve()) {
             // no scan in progress
@@ -2182,7 +2183,7 @@ static RPCHelpMan scantxoutset()
         }
         result.pushKV("progress", g_scan_progress.load());
         return result;
-    } else if (request.params[0].get_str() == "abort") {
+    } else if (action == "abort") {
         CoinsViewScanReserver reserver;
         if (reserver.reserve()) {
             // reserve was possible which means no scan was running
@@ -2191,7 +2192,7 @@ static RPCHelpMan scantxoutset()
         // set the abort flag
         g_should_abort_scan = true;
         return true;
-    } else if (request.params[0].get_str() == "start") {
+    } else if (action == "start") {
         CoinsViewScanReserver reserver;
         if (!reserver.reserve()) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Scan already in progress, use action \"abort\" or \"status\"");
@@ -2260,7 +2261,7 @@ static RPCHelpMan scantxoutset()
         result.pushKV("unspents", unspents);
         result.pushKV("total_amount", ValueFromAmount(total_in));
     } else {
-        throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid action '%s'", request.params[0].get_str()));
+        throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid action '%s'", action));
     }
     return result;
 },

src/rpc/mining.cpp

@@ -122,7 +122,7 @@ static RPCHelpMan getnetworkhashps()
 {
     ChainstateManager& chainman = EnsureAnyChainman(request.context);
     LOCK(cs_main);
-    return GetNetworkHashPS(self.Arg<int>(0), self.Arg<int>(1), chainman.ActiveChain());
+    return GetNetworkHashPS(self.Arg<int>("nblocks"), self.Arg<int>("height"), chainman.ActiveChain());
 },
     };
 }
@@ -229,12 +229,12 @@ static RPCHelpMan generatetodescriptor()
             "\nGenerate 11 blocks to mydesc\n" + HelpExampleCli("generatetodescriptor", "11 \"mydesc\"")},
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    const auto num_blocks{self.Arg<int>(0)};
-    const auto max_tries{self.Arg<uint64_t>(2)};
+    const auto num_blocks{self.Arg<int>("num_blocks")};
+    const auto max_tries{self.Arg<uint64_t>("maxtries")};
 
     CScript coinbase_script;
     std::string error;
-    if (!getScriptFromDescriptor(self.Arg<std::string>(1), coinbase_script, error)) {
+    if (!getScriptFromDescriptor(self.Arg<std::string>("descriptor"), coinbase_script, error)) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error);
     }
 

src/rpc/net.cpp

@@ -322,7 +322,7 @@ static RPCHelpMan addnode()
                 },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
-    const std::string command{request.params[1].get_str()};
+    const auto command{self.Arg<std::string>("command")};
     if (command != "onetry" && command != "add" && command != "remove") {
         throw std::runtime_error(
             self.ToString());
@@ -331,9 +331,9 @@ static RPCHelpMan addnode()
     NodeContext& node = EnsureAnyNodeContext(request.context);
     CConnman& connman = EnsureConnman(node);
 
-    const std::string node_arg{request.params[0].get_str()};
+    const auto node_arg{self.Arg<std::string>("node")};
     bool node_v2transport = connman.GetLocalServices() & NODE_P2P_V2;
-    bool use_v2transport = self.MaybeArg<bool>(2).value_or(node_v2transport);
+    bool use_v2transport = self.MaybeArg<bool>("v2transport").value_or(node_v2transport);
 
     if (use_v2transport && !node_v2transport) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: v2transport requested but not enabled (see -v2transport)");

src/rpc/signmessage.cpp

@@ -38,9 +38,9 @@ static RPCHelpMan verifymessage()
         },
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
         {
-            std::string strAddress = request.params[0].get_str();
-            std::string strSign = request.params[1].get_str();
-            std::string strMessage = request.params[2].get_str();
+            std::string strAddress = self.Arg<std::string>("address");
+            std::string strSign = self.Arg<std::string>("signature");
+            std::string strMessage = self.Arg<std::string>("message");
 
             switch (MessageVerify(strAddress, strSign, strMessage)) {
             case MessageVerificationResult::ERR_INVALID_ADDRESS:

src/rpc/util.cpp

@@ -24,6 +24,8 @@
 #include <util/string.h>
 #include <util/translation.h>
 
+#include <algorithm>
+#include <iterator>
 #include <string_view>
 #include <tuple>
 
@@ -729,6 +731,16 @@ std::vector<std::pair<std::string, bool>> RPCHelpMan::GetArgNames() const
     return ret;
 }
 
+size_t RPCHelpMan::GetParamIndex(std::string_view key) const
+{
+    auto it{std::find_if(
+        m_args.begin(), m_args.end(), [&key](const auto& arg) { return arg.GetName() == key;}
+    )};
+
+    CHECK_NONFATAL(it != m_args.end());  // TODO: ideally this is checked at compile time
+    return std::distance(m_args.begin(), it);
+}
+
 std::string RPCHelpMan::ToString() const
 {
     std::string ret;

src/rpc/util.h

@@ -404,18 +404,25 @@ public:
 
     UniValue HandleRequest(const JSONRPCRequest& request) const;
     /**
-     * Helper to get a request argument.
-     * This function only works during m_fun(), i.e. it should only be used in
-     * RPC method implementations. The helper internally checks whether the
-     * user-passed argument isNull() and parses (from JSON) and returns the
-     * user-passed argument, or the default value derived from the RPCArg
-     * documentation, or a falsy value if no default was given.
+     * @brief Helper to get a required or default-valued request argument.
      *
-     * Use Arg<Type>(i) to get the argument or its default value. Otherwise,
-     * use MaybeArg<Type>(i) to get the optional argument or a falsy value.
+     * Use this function when the argument is required or when it has a default value. If the
+     * argument is optional and may not be provided, use MaybeArg instead.
      *
-     * The Type passed to this helper must match the corresponding
-     * RPCArg::Type.
+     * This function only works during m_fun(), i.e., it should only be used in
+     * RPC method implementations. It internally checks whether the user-passed
+     * argument isNull() and parses (from JSON) and returns the user-passed argument,
+     * or the default value derived from the RPCArg documentation.
+     *
+     * There are two overloads of this function:
+     * - Use Arg<Type>(size_t i) to get the argument (or the default value) by index.
+     * - Use Arg<Type>(const std::string& key) to get the argument (or the default value) by key.
+     *
+     * The Type passed to this helper must match the corresponding RPCArg::Type.
+     *
+     * @return The value of the RPC argument (or the default value) cast to type Type.
+     *
+     * @see MaybeArg for handling optional arguments without default values.
      */
     template <typename R>
     auto Arg(size_t i) const
@@ -429,6 +436,34 @@ public:
             return ArgValue<const R&>(i);
         }
     }
+    template<typename R>
+    auto Arg(std::string_view key) const
+    {
+        return Arg<R>(GetParamIndex(key));
+    }
+    /**
+     * @brief Helper to get an optional request argument.
+     *
+     * Use this function when the argument is optional and does not have a default value. If the
+     * argument is required or has a default value, use Arg instead.
+     *
+     * This function only works during m_fun(), i.e., it should only be used in
+     * RPC method implementations. It internally checks whether the user-passed
+     * argument isNull() and parses (from JSON) and returns the user-passed argument,
+     * or a falsy value if no argument was passed.
+     *
+     * There are two overloads of this function:
+     * - Use MaybeArg<Type>(size_t i) to get the optional argument by index.
+     * - Use MaybeArg<Type>(const std::string& key) to get the optional argument by key.
+     *
+     * The Type passed to this helper must match the corresponding RPCArg::Type.
+     *
+     * @return For integral and floating-point types, a std::optional<Type> is returned.
+    *          For other types, a Type* pointer to the argument is returned. If the
+    *          argument is not provided, std::nullopt or a null pointer is returned.
+    *
+     * @see Arg for handling arguments that are required or have a default value.
+     */
     template <typename R>
     auto MaybeArg(size_t i) const
     {
@@ -441,6 +476,11 @@ public:
             return ArgValue<const R*>(i);
         }
     }
+    template<typename R>
+    auto MaybeArg(std::string_view key) const
+    {
+        return MaybeArg<R>(GetParamIndex(key));
+    }
     std::string ToString() const;
     /** Return the named args that need to be converted from string to another JSON type */
     UniValue GetArgMap() const;
@@ -460,6 +500,8 @@ private:
     mutable const JSONRPCRequest* m_req{nullptr}; // A pointer to the request for the duration of m_fun()
     template <typename R>
     R ArgValue(size_t i) const;
+    //! Return positional index of a parameter using its name as key.
+    size_t GetParamIndex(std::string_view key) const;
 };
 
 /**