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

1 files changed, 60 insertions, 0 deletions

diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp
index a52530e179..21ccbe9648 100644
--- a/src/test/rpc_tests.cpp
+++ b/src/test/rpc_tests.cpp
@@ -17,12 +17,49 @@
 
 #include <boost/test/unit_test.hpp>
 
+static UniValue JSON(std::string_view json)
+{
+    UniValue value;
+    BOOST_CHECK(value.read(json.data(), json.size()));
+    return value;
+}
+
+class HasJSON
+{
+public:
+    explicit HasJSON(std::string json) : m_json(std::move(json)) {}
+    bool operator()(const UniValue& value) const
+    {
+        std::string json{value.write()};
+        BOOST_CHECK_EQUAL(json, m_json);
+        return json == m_json;
+    };
+
+private:
+    const std::string m_json;
+};
+
 class RPCTestingSetup : public TestingSetup
 {
 public:
+    UniValue TransformParams(const UniValue& params, std::vector<std::string> arg_names) const;
     UniValue CallRPC(std::string args);
 };
 
+UniValue RPCTestingSetup::TransformParams(const UniValue& params, std::vector<std::string> arg_names) const
+{
+    UniValue transformed_params;
+    CRPCTable table;
+    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);
+    JSONRPCRequest request;
+    request.strMethod = "method";
+    request.params = params;
+    if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished();
+    table.execute(request);
+    return transformed_params;
+}
+
 UniValue RPCTestingSetup::CallRPC(std::string args)
 {
     std::vector<std::string> vArgs{SplitString(args, ' ')};
@@ -45,6 +82,29 @@ UniValue RPCTestingSetup::CallRPC(std::string args)
 
 BOOST_FIXTURE_TEST_SUITE(rpc_tests, RPCTestingSetup)
 
+BOOST_AUTO_TEST_CASE(rpc_namedparams)
+{
+    const std::vector<std::string> arg_names{{"arg1", "arg2", "arg3", "arg4", "arg5"}};
+
+    // Make sure named arguments are transformed into positional arguments in correct places separated by nulls
+    BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg2": 2, "arg4": 4})"), arg_names).write(), "[null,2,null,4]");
+
+    // Make sure named and positional arguments can be combined.
+    BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg5": 5, "args": [1, 2], "arg4": 4})"), arg_names).write(), "[1,2,null,4,5]");
+
+    // Make sure a unknown named argument raises an exception
+    BOOST_CHECK_EXCEPTION(TransformParams(JSON(R"({"arg2": 2, "unknown": 6})"), arg_names), UniValue,
+                          HasJSON(R"({"code":-8,"message":"Unknown named parameter unknown"})"));
+
+    // Make sure an overlap between a named argument and positional argument raises an exception
+    BOOST_CHECK_EXCEPTION(TransformParams(JSON(R"({"args": [1,2,3], "arg4": 4, "arg2": 2})"), arg_names), UniValue,
+                          HasJSON(R"({"code":-8,"message":"Parameter arg2 specified twice both as positional and named argument"})"));
+
+    // Make sure extra positional arguments can be passed through to the method implemenation, as long as they don't overlap with named arguments.
+    BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"args": [1,2,3,4,5,6,7,8,9,10]})"), arg_names).write(), "[1,2,3,4,5,6,7,8,9,10]");
+    BOOST_CHECK_EQUAL(TransformParams(JSON(R"([1,2,3,4,5,6,7,8,9,10])"), arg_names).write(), "[1,2,3,4,5,6,7,8,9,10]");
+}
+
 BOOST_AUTO_TEST_CASE(rpc_rawparams)
 {
     // Test raw transaction API argument handling