diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 354f9490024..bd684b7e86c 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -392,7 +392,7 @@ std::string JSONRPCExecBatch(const JSONRPCRequest& jreq, const UniValue& vReq) * Process named arguments into a vector of positional arguments, based on the * passed-in specification for the RPC call's arguments. */ -static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, const std::vector& argNames) +static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, const std::vector>& argNames) { JSONRPCRequest out = in; out.params = UniValue(UniValue::VARR); @@ -417,7 +417,9 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c // "args" parameter, if present. int hole = 0; int initial_hole_size = 0; - for (const std::string &argNamePattern: argNames) { + const std::string* initial_param = nullptr; + UniValue options{UniValue::VOBJ}; + for (const auto& [argNamePattern, named_only]: argNames) { std::vector vargNames = SplitString(argNamePattern, '|'); auto fr = argsIn.end(); for (const std::string & argName : vargNames) { @@ -426,7 +428,22 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c break; } } - if (fr != argsIn.end()) { + + // Handle named-only parameters by pushing them into a temporary options + // object, and then pushing the accumulated options as the next + // positional argument. + if (named_only) { + if (fr != argsIn.end()) { + if (options.exists(fr->first)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + fr->first + " specified multiple times"); + } + options.__pushKV(fr->first, *fr->second); + argsIn.erase(fr); + } + continue; + } + + if (!options.empty() || fr != argsIn.end()) { for (int i = 0; i < hole; ++i) { // Fill hole between specified parameters with JSON nulls, // but not at the end (for backwards compatibility with calls @@ -434,12 +451,26 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c out.params.push_back(UniValue()); } hole = 0; - out.params.push_back(*fr->second); - argsIn.erase(fr); + if (!initial_param) initial_param = &argNamePattern; } else { hole += 1; if (out.params.empty()) initial_hole_size = hole; } + + // If named input parameter "fr" is present, push it onto out.params. If + // options are present, push them onto out.params. If both are present, + // throw an error. + if (fr != argsIn.end()) { + if (!options.empty()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + fr->first + " conflicts with parameter " + options.getKeys().front()); + } + out.params.push_back(*fr->second); + argsIn.erase(fr); + } + if (!options.empty()) { + out.params.push_back(std::move(options)); + options = UniValue{UniValue::VOBJ}; + } } // If leftover "args" param was found, use it as a source of positional // arguments and add named arguments after. This is a convenience for @@ -447,9 +478,8 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c // 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"); + if (initial_hole_size < (int)positional_args.mapped()->size() && initial_param) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + *initial_param + " 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)}; diff --git a/src/rpc/server.h b/src/rpc/server.h index 01e85560502..24658ddb8bc 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -95,7 +95,7 @@ public: using Actor = std::function; //! Constructor taking Actor callback supporting multiple handlers. - CRPCCommand(std::string category, std::string name, Actor actor, std::vector args, intptr_t unique_id) + CRPCCommand(std::string category, std::string name, Actor actor, std::vector> args, intptr_t unique_id) : category(std::move(category)), name(std::move(name)), actor(std::move(actor)), argNames(std::move(args)), unique_id(unique_id) { @@ -115,7 +115,16 @@ public: std::string category; std::string name; Actor actor; - std::vector argNames; + //! List of method arguments and whether they are named-only. Incoming RPC + //! requests contain a "params" field that can either be an array containing + //! unnamed arguments or an object containing named arguments. The + //! "argNames" vector is used in the latter case to transform the params + //! object into an array. Each argument in "argNames" gets mapped to a + //! unique position in the array, based on the order it is listed, unless + //! the argument is a named-only argument with argNames[x].second set to + //! true. Named-only arguments are combined into a JSON object that is + //! appended after other arguments, see transformNamedArguments for details. + std::vector> argNames; intptr_t unique_id; }; diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 1f3f37d0a0f..2fa1732faaf 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -389,7 +389,8 @@ struct Sections { case RPCArg::Type::NUM: case RPCArg::Type::AMOUNT: case RPCArg::Type::RANGE: - case RPCArg::Type::BOOL: { + case RPCArg::Type::BOOL: + case RPCArg::Type::OBJ_NAMED_PARAMS: { if (is_top_level_arg) return; // Nothing more to do for non-recursive types on first recursion auto left = indent; if (arg.m_opts.type_str.size() != 0 && push_name) { @@ -605,12 +606,17 @@ bool RPCHelpMan::IsValidNumArgs(size_t num_args) const return num_required_args <= num_args && num_args <= m_args.size(); } -std::vector RPCHelpMan::GetArgNames() const +std::vector> RPCHelpMan::GetArgNames() const { - std::vector ret; + std::vector> ret; ret.reserve(m_args.size()); for (const auto& arg : m_args) { - ret.emplace_back(arg.m_names); + if (arg.m_type == RPCArg::Type::OBJ_NAMED_PARAMS) { + for (const auto& inner : arg.m_inner) { + ret.emplace_back(inner.m_names, /*named_only=*/true); + } + } + ret.emplace_back(arg.m_names, /*named_only=*/false); } return ret; } @@ -642,20 +648,31 @@ std::string RPCHelpMan::ToString() const // Arguments Sections sections; + Sections named_only_sections; for (size_t i{0}; i < m_args.size(); ++i) { const auto& arg = m_args.at(i); if (arg.m_opts.hidden) break; // Any arg that follows is also hidden - if (i == 0) ret += "\nArguments:\n"; - // Push named argument name and description sections.m_sections.emplace_back(::ToString(i + 1) + ". " + arg.GetFirstName(), arg.ToDescriptionString(/*is_named_arg=*/true)); sections.m_max_pad = std::max(sections.m_max_pad, sections.m_sections.back().m_left.size()); // Recursively push nested args sections.Push(arg); + + // Push named-only argument sections + if (arg.m_type == RPCArg::Type::OBJ_NAMED_PARAMS) { + for (const auto& arg_inner : arg.m_inner) { + named_only_sections.PushSection({arg_inner.GetFirstName(), arg_inner.ToDescriptionString(/*is_named_arg=*/true)}); + named_only_sections.Push(arg_inner); + } + } } + + if (!sections.m_sections.empty()) ret += "\nArguments:\n"; ret += sections.ToString(); + if (!named_only_sections.m_sections.empty()) ret += "\nNamed Arguments:\n"; + ret += named_only_sections.ToString(); // Result ret += m_results.ToDescriptionString(); @@ -669,17 +686,30 @@ std::string RPCHelpMan::ToString() const UniValue RPCHelpMan::GetArgMap() const { UniValue arr{UniValue::VARR}; + + auto push_back_arg_info = [&arr](const std::string& rpc_name, int pos, const std::string& arg_name, const RPCArg::Type& type) { + UniValue map{UniValue::VARR}; + map.push_back(rpc_name); + map.push_back(pos); + map.push_back(arg_name); + map.push_back(type == RPCArg::Type::STR || + type == RPCArg::Type::STR_HEX); + arr.push_back(map); + }; + for (int i{0}; i < int(m_args.size()); ++i) { const auto& arg = m_args.at(i); std::vector arg_names = SplitString(arg.m_names, '|'); for (const auto& arg_name : arg_names) { - UniValue map{UniValue::VARR}; - map.push_back(m_name); - map.push_back(i); - map.push_back(arg_name); - map.push_back(arg.m_type == RPCArg::Type::STR || - arg.m_type == RPCArg::Type::STR_HEX); - arr.push_back(map); + push_back_arg_info(m_name, i, arg_name, arg.m_type); + if (arg.m_type == RPCArg::Type::OBJ_NAMED_PARAMS) { + for (const auto& inner : arg.m_inner) { + std::vector inner_names = SplitString(inner.m_names, '|'); + for (const std::string& inner_name : inner_names) { + push_back_arg_info(m_name, i, inner_name, inner.m_type); + } + } + } } } return arr; @@ -708,6 +738,7 @@ static std::optional ExpectedType(RPCArg::Type type) return UniValue::VBOOL; } case Type::OBJ: + case Type::OBJ_NAMED_PARAMS: case Type::OBJ_USER_KEYS: { return UniValue::VOBJ; } @@ -781,6 +812,7 @@ std::string RPCArg::ToDescriptionString(bool is_named_arg) const break; } case Type::OBJ: + case Type::OBJ_NAMED_PARAMS: case Type::OBJ_USER_KEYS: { ret += "json object"; break; @@ -809,6 +841,7 @@ std::string RPCArg::ToDescriptionString(bool is_named_arg) const } // no default case, so the compiler can warn about missing cases } ret += ")"; + if (m_type == Type::OBJ_NAMED_PARAMS) ret += " Options object that can be used to pass named arguments, listed below."; ret += m_description.empty() ? "" : " " + m_description; return ret; } @@ -1054,6 +1087,7 @@ std::string RPCArg::ToStringObj(const bool oneline) const } return res + "...]"; case Type::OBJ: + case Type::OBJ_NAMED_PARAMS: case Type::OBJ_USER_KEYS: // Currently unused, so avoid writing dead code NONFATAL_UNREACHABLE(); @@ -1077,6 +1111,7 @@ std::string RPCArg::ToString(const bool oneline) const return GetFirstName(); } case Type::OBJ: + case Type::OBJ_NAMED_PARAMS: case Type::OBJ_USER_KEYS: { const std::string res = Join(m_inner, ",", [&](const RPCArg& i) { return i.ToStringObj(oneline); }); if (m_type == Type::OBJ) { diff --git a/src/rpc/util.h b/src/rpc/util.h index bb5c30a2f44..d017812e97a 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -139,6 +139,13 @@ struct RPCArg { STR, NUM, BOOL, + OBJ_NAMED_PARAMS, //!< Special type that behaves almost exactly like + //!< OBJ, defining an options object with a list of + //!< pre-defined keys. The only difference between OBJ + //!< and OBJ_NAMED_PARAMS is that OBJ_NAMED_PARMS + //!< also allows the keys to be passed as top-level + //!< named parameters, as a more convenient way to pass + //!< options to the RPC method without nesting them. OBJ_USER_KEYS, //!< Special type where the user must set the keys e.g. to define multiple addresses; as opposed to e.g. an options object where the keys are predefined AMOUNT, //!< Special type representing a floating point amount (can be either NUM or STR) STR_HEX, //!< Special type that is a STR with only hex chars @@ -183,7 +190,7 @@ struct RPCArg { m_description{std::move(description)}, m_opts{std::move(opts)} { - CHECK_NONFATAL(type != Type::ARR && type != Type::OBJ && type != Type::OBJ_USER_KEYS); + CHECK_NONFATAL(type != Type::ARR && type != Type::OBJ && type != Type::OBJ_NAMED_PARAMS && type != Type::OBJ_USER_KEYS); } RPCArg( @@ -200,7 +207,7 @@ struct RPCArg { m_description{std::move(description)}, m_opts{std::move(opts)} { - CHECK_NONFATAL(type == Type::ARR || type == Type::OBJ || type == Type::OBJ_USER_KEYS); + CHECK_NONFATAL(type == Type::ARR || type == Type::OBJ || type == Type::OBJ_NAMED_PARAMS || type == Type::OBJ_USER_KEYS); } bool IsOptional() const; @@ -369,7 +376,8 @@ public: UniValue GetArgMap() const; /** If the supplied number of args is neither too small nor too high */ bool IsValidNumArgs(size_t num_args) const; - std::vector GetArgNames() const; + //! Return list of arguments and whether they are named-only. + std::vector> GetArgNames() const; const std::string m_name; diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 791c9ddf312..f8975dd9b29 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -42,11 +42,11 @@ private: class RPCTestingSetup : public TestingSetup { public: - UniValue TransformParams(const UniValue& params, std::vector arg_names) const; + UniValue TransformParams(const UniValue& params, std::vector> arg_names) const; UniValue CallRPC(std::string args); }; -UniValue RPCTestingSetup::TransformParams(const UniValue& params, std::vector arg_names) const +UniValue RPCTestingSetup::TransformParams(const UniValue& params, std::vector> arg_names) const { UniValue transformed_params; CRPCTable table; @@ -84,7 +84,7 @@ BOOST_FIXTURE_TEST_SUITE(rpc_tests, RPCTestingSetup) BOOST_AUTO_TEST_CASE(rpc_namedparams) { - const std::vector arg_names{"arg1", "arg2", "arg3", "arg4", "arg5"}; + const std::vector> arg_names{{"arg1", false}, {"arg2", false}, {"arg3", false}, {"arg4", false}, {"arg5", false}}; // 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]"); @@ -109,6 +109,28 @@ BOOST_AUTO_TEST_CASE(rpc_namedparams) 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_namedonlyparams) +{ + const std::vector> arg_names{{"arg1", false}, {"arg2", false}, {"opt1", true}, {"opt2", true}, {"options", false}}; + + // Make sure optional parameters are really optional. + BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg1": 1, "arg2": 2})"), arg_names).write(), "[1,2]"); + + // Make sure named-only parameters are passed as options. + BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg1": 1, "arg2": 2, "opt1": 10, "opt2": 20})"), arg_names).write(), R"([1,2,{"opt1":10,"opt2":20}])"); + + // Make sure options can be passed directly. + BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg1": 1, "arg2": 2, "options":{"opt1": 10, "opt2": 20}})"), arg_names).write(), R"([1,2,{"opt1":10,"opt2":20}])"); + + // Make sure options and named parameters conflict. + BOOST_CHECK_EXCEPTION(TransformParams(JSON(R"({"arg1": 1, "arg2": 2, "opt1": 10, "options":{"opt1": 10}})"), arg_names), UniValue, + HasJSON(R"({"code":-8,"message":"Parameter options conflicts with parameter opt1"})")); + + // Make sure options object specified through args array conflicts. + BOOST_CHECK_EXCEPTION(TransformParams(JSON(R"({"args": [1, 2, {"opt1": 10}], "opt2": 20})"), arg_names), UniValue, + HasJSON(R"({"code":-8,"message":"Parameter options specified twice both as positional and named argument"})")); +} + BOOST_AUTO_TEST_CASE(rpc_rawparams) { // Test raw transaction API argument handling