Merge #14875: RPCHelpMan: Support required arguments after optional ones

fa9a5bc1a0 RPCHelpMan: Support required arguments after optional ones (MarcoFalke)

Pull request description:

  There was a requirement that required arguments could not be positioned after an optional argument, but the deprecation of priority made the second argument to `prioritisetransaction` optional. So support that in `RPCHelpMan`.

  Also format all named arguments in the same way (without the wrapping `"` even for strings), since the extended description already mentions the type and it feels odd to special case strings.

Tree-SHA512: c125145afb4a63abc995aaf0a89489efc0f470a720727a1ca6ee0bfd2bcbc59e87c38128dd1e0cdf03dbb5b18e84867887c3dabf6ec8378e66cb1f4cecb9e407

2 files changed, 8 insertions, 10 deletions

diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 93fa3a2728..29684d0e6c 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -241,7 +241,7 @@ static UniValue prioritisetransaction(const JSONRPCRequest& request)
                 "Accepts the transaction into mined blocks at a higher (or lower) priority\n",
                 {
                     {"txid", RPCArg::Type::STR_HEX, /* opt */ false, /* default_val */ "", "The transaction id."},
-                    {"dummy", RPCArg::Type::NUM, /* opt */ false, /* default_val */ "", "API-Compatibility for previous API. Must be zero or null.\n"
+                    {"dummy", RPCArg::Type::NUM, /* opt */ true, /* default_val */ "null", "API-Compatibility for previous API. Must be zero or null.\n"
             "                  DEPRECATED. For forward compatibility use named arguments and omit this parameter."},
                     {"fee_delta", RPCArg::Type::NUM, /* opt */ false, /* default_val */ "", "The fee value (in satoshis) to add (or subtract, if negative).\n"
             "                  Note, that this value is not a fee rate. It is a value to modify absolute fee of the TX.\n"
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index 740f8351fe..b91baee4ac 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -258,20 +258,19 @@ std::string RPCHelpMan::ToString() const
 
     // Oneline summary
     ret += m_name;
-    bool is_optional{false};
+    bool was_optional{false};
     for (const auto& arg : m_args) {
         ret += " ";
         if (arg.m_optional) {
-            if (!is_optional) ret += "( ";
-            is_optional = true;
+            if (!was_optional) ret += "( ";
+            was_optional = true;
         } else {
-            // Currently we still support unnamed arguments, so any argument following an optional argument must also be optional
-            // If support for positional arguments is deprecated in the future, remove this line
-            assert(!is_optional);
+            if (was_optional) ret += ") ";
+            was_optional = false;
         }
         ret += arg.ToString(/* oneline */ true);
     }
-    if (is_optional) ret += " )";
+    if (was_optional) ret += " )";
     ret += "\n";
 
     // Description
@@ -285,8 +284,7 @@ std::string RPCHelpMan::ToString() const
         if (i == 0) ret += "\nArguments:\n";
 
         // Push named argument name and description
-        const auto str_wrapper = (arg.m_type == RPCArg::Type::STR || arg.m_type == RPCArg::Type::STR_HEX) ? "\"" : "";
-        sections.m_sections.emplace_back(std::to_string(i + 1) + ". " + str_wrapper + arg.m_name + str_wrapper, arg.ToDescriptionString());
+        sections.m_sections.emplace_back(std::to_string(i + 1) + ". " + arg.m_name, arg.ToDescriptionString());
         sections.m_max_pad = std::max(sections.m_max_pad, sections.m_sections.back().m_left.size());
 
         // Recursively push nested args