Merge bitcoin/bitcoin#19762: rpc: Allow named and positional arguments to be used together

d8b12a75dbfdc1d3e62352f0fa815bbbdc685caf rpc: Allow named and positional arguments to be used together (Ryan Ofsky)

Pull request description:

  It's nice to be able to use named options and positional arguments together.

  Most shell tools accept both, and python functions combine options and arguments allowing them to be passed with even more flexibility. This change adds support for python's approach so as a motivating example:

  ```sh
  bitcoin-cli -named createwallet wallet_name=mywallet load_on_startup=1
  ```

  Can be shortened to:

  ```sh
  bitcoin-cli -named createwallet mywallet load_on_startup=1
  ```

  JSON-RPC standard doesn't have a convention for passing named and positional parameters together, so this implementation makes one up and interprets any unused `"args"` named parameter as a positional parameter array.

  This change is backwards compatible. It doesn't change the interpretation of any previously valid calls, just treats some previously invalid calls as valid.

  Another use case even if you only occasionally use named arguments is that you can define an alias:

  ```
  alias bcli='bitcoin-cli -named'
  ```

  And now use both named named and unnamed arguments from the same alias without having to manually add `-named` option for named arguments or see annoying error "No '=' in named argument... this needs to be present for every argument (even if it is empty)`" for unnamed arguments

ACKs for top commit:
  achow101:
    ACK d8b12a75dbfdc1d3e62352f0fa815bbbdc685caf
  stickies-v:
    re-ACK d8b12a75d
  aureleoules:
    re-ACK d8b12a75dbfdc1d3e62352f0fa815bbbdc685caf

Tree-SHA512: 0cff8b50f584bcbbd376624adccf40536566ed8d1bcd6c88ad565dbc208f19d5e7a48c994efd6329d42b560149340d330397278f08a2912af5f3418d8c8837a9

2 files changed, 34 insertions, 2 deletions

diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
index 8688263ef5..b3434b80c7 100644
--- a/src/rpc/client.cpp
+++ b/src/rpc/client.cpp
@@ -277,11 +277,13 @@ UniValue RPCConvertValues(const std::string &strMethod, const std::vector<std::s
 UniValue RPCConvertNamedValues(const std::string &strMethod, const std::vector<std::string> &strParams)
 {
     UniValue params(UniValue::VOBJ);
+    UniValue positional_args{UniValue::VARR};
 
     for (const std::string &s: strParams) {
         size_t pos = s.find('=');
         if (pos == std::string::npos) {
-            throw(std::runtime_error("No '=' in named argument '"+s+"', this needs to be present for every argument (even if it is empty)"));
+            positional_args.push_back(rpcCvtTable.convert(strMethod, positional_args.size()) ? ParseNonRFCJSONValue(s) : s);
+            continue;
         }
 
         std::string name = s.substr(0, pos);
@@ -296,5 +298,9 @@ UniValue RPCConvertNamedValues(const std::string &strMethod, const std::vector<s
         }
     }
 
+    if (!positional_args.empty()) {
+        params.pushKV("args", positional_args);
+    }
+
     return params;
 }
diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp
index 1d7bd2eb94..f57133f75b 100644
--- a/src/rpc/server.cpp
+++ b/src/rpc/server.cpp
@@ -401,8 +401,16 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c
     for (size_t i=0; i<keys.size(); ++i) {
         argsIn[keys[i]] = &values[i];
     }
-    // Process expected parameters.
+    // Process expected parameters. If any parameters were left unspecified in
+    // the request before a parameter that was specified, null values need to be
+    // inserted at the unspecifed parameter positions, and the "hole" variable
+    // below tracks the number of null values that need to be inserted.
+    // The "initial_hole_size" variable stores the size of the initial hole,
+    // i.e. how many initial positional arguments were left unspecified. This is
+    // used after the for-loop to add initial positional arguments from the
+    // "args" parameter, if present.
     int hole = 0;
+    int initial_hole_size = 0;
     for (const std::string &argNamePattern: argNames) {
         std::vector<std::string> vargNames = SplitString(argNamePattern, '|');
         auto fr = argsIn.end();
@@ -424,6 +432,24 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c
             argsIn.erase(fr);
         } else {
             hole += 1;
+            if (out.params.empty()) initial_hole_size = hole;
+        }
+    }
+    // If leftover "args" param was found, use it as a source of positional
+    // arguments and add named arguments after. This is a convenience for
+    // clients that want to pass a combination of named and positional
+    // arguments as described in doc/JSON-RPC-interface.md#parameter-passing
+    auto positional_args{argsIn.extract("args")};
+    if (positional_args && positional_args.mapped()->isArray()) {
+        const bool has_named_arguments{initial_hole_size < (int)argNames.size()};
+        if (initial_hole_size < (int)positional_args.mapped()->size() && has_named_arguments) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + argNames[initial_hole_size] + " specified twice both as positional and named argument");
+        }
+        // Assign positional_args to out.params and append named_args after.
+        UniValue named_args{std::move(out.params)};
+        out.params = *positional_args.mapped();
+        for (size_t i{out.params.size()}; i < named_args.size(); ++i) {
+            out.params.push_back(named_args[i]);
         }
     }
     // If there are still arguments in the argsIn map, this is an error.