Merge bitcoin#20459: rpc: Fail to return undocumented return values

fa8192f rpc: Fail to return undocumented return values (MarcoFalke)

Pull request description:

  Currently a few return values are undocumented. This is causing confusion at the least. See for example bitcoin#18476

  Fix this by treating it as an internal bug to return undocumented return values.

ACKs for top commit:
  ryanofsky:
    Code review ACK fa8192f. Only changes: rebase, no const_cast suggestion, and tostring cleanups needed after suggestion

Tree-SHA512: c006905639bafe3045de152b00c34d9864731becb3c4f468bdd61a392f10d7e7cd89a54862c8daa8c11ac4eea0eb5f13b0f647d21e21a0a797b54191cff7238c

src/qt/test/rpcnestedtests.cpp

@@ -24,7 +24,7 @@ static RPCHelpMan rpcNestedTest_rpc()
             {"arg2", RPCArg::Type::STR, RPCArg::Optional::OMITTED, ""},
             {"arg3", RPCArg::Type::STR, RPCArg::Optional::OMITTED, ""},
         },
-        {},
+        RPCResult{RPCResult::Type::ANY, "", ""},
         RPCExamples{""},
         [](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue {
             return request.params.write(0, 0);

src/rpc/misc.cpp

@@ -628,7 +628,7 @@ static RPCHelpMan echo(const std::string& name)
                     {"arg8", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
                     {"arg9", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""},
                 },
-                RPCResult{RPCResult::Type::NONE, "", "Returns whatever was passed in"},
+                RPCResult{RPCResult::Type::ANY, "", "Returns whatever was passed in"},
                 RPCExamples{""},
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {

src/rpc/server.cpp

@@ -137,8 +137,9 @@ static RPCHelpMan help()
                 {
                     {"command", RPCArg::Type::STR, /* default */ "all commands", "The command to get help on"},
                 },
-                RPCResult{
-                    RPCResult::Type::STR, "", "The help text"
+                {
+                    RPCResult{RPCResult::Type::STR, "", "The help text"},
+                    RPCResult{RPCResult::Type::ANY, "", ""},
                 },
                 RPCExamples{""},
         [&](const RPCHelpMan& self, const JSONRPCRequest& jsonRequest) -> UniValue

src/rpc/util.cpp

@@ -442,6 +442,7 @@ std::string RPCResults::ToDescriptionString() const
 {
     std::string result;
     for (const auto& r : m_results) {
+        if (r.m_type == RPCResult::Type::ANY) continue; // for testing only
         if (r.m_cond.empty()) {
             result += "\nResult:\n";
         } else {
@@ -459,7 +460,7 @@ std::string RPCExamples::ToDescriptionString() const
     return m_examples.empty() ? m_examples : "\nExamples:\n" + m_examples;
 }
 
-UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request)
+UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request) const
 {
     if (request.mode == JSONRPCRequest::GET_ARGS) {
         return GetArgMap();
@@ -471,7 +472,9 @@ UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request)
     if (request.mode == JSONRPCRequest::GET_HELP || !IsValidNumArgs(request.params.size())) {
         throw std::runtime_error(ToString());
     }
-    return m_fun(*this, request);
+    const UniValue ret = m_fun(*this, request);
+    CHECK_NONFATAL(std::any_of(m_results.m_results.begin(), m_results.m_results.end(), [ret](const RPCResult& res) { return res.MatchesType(ret); }));
+    return ret;
 }
 
 bool RPCHelpMan::IsValidNumArgs(size_t num_args) const
@@ -677,6 +680,9 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const
         sections.PushSection({indent + "..." + maybe_separator, m_description});
         return;
     }
+    case Type::ANY: {
+        CHECK_NONFATAL(false); // Only for testing
+    }
     case Type::NONE: {
         sections.PushSection({indent + "null" + maybe_separator, Description("json null")});
         return;
@@ -742,6 +748,42 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const
     CHECK_NONFATAL(false);
 }
 
+bool RPCResult::MatchesType(const UniValue& result) const
+{
+    switch (m_type) {
+    case Type::ELISION: {
+        return false;
+    }
+    case Type::ANY: {
+        return true;
+    }
+    case Type::NONE: {
+        return UniValue::VNULL == result.getType();
+    }
+    case Type::STR:
+    case Type::STR_HEX: {
+        return UniValue::VSTR == result.getType();
+    }
+    case Type::NUM:
+    case Type::STR_AMOUNT:
+    case Type::NUM_TIME: {
+        return UniValue::VNUM == result.getType();
+    }
+    case Type::BOOL: {
+        return UniValue::VBOOL == result.getType();
+    }
+    case Type::ARR_FIXED:
+    case Type::ARR: {
+        return UniValue::VARR == result.getType();
+    }
+    case Type::OBJ_DYN:
+    case Type::OBJ: {
+        return UniValue::VOBJ == result.getType();
+    }
+    } // no default case, so the compiler can warn about missing cases
+    CHECK_NONFATAL(false);
+}
+
 std::string RPCArg::ToStringObj(const bool oneline) const
 {
     std::string res;

src/rpc/util.h

@@ -223,6 +223,7 @@ struct RPCResult {
         NUM,
         BOOL,
         NONE,
+        ANY,        //!< Special type to disable type checks (for testing only)
         STR_AMOUNT, //!< Special string to represent a floating point amount
         STR_HEX,    //!< Special string with only hex chars
         OBJ_DYN,    //!< Special dictionary with keys that are not literals
@@ -295,6 +296,8 @@ struct RPCResult {
     std::string ToStringObj() const;
     /** Return the description string, including the result type. */
     std::string ToDescriptionString() const;
+    /** Check whether the result JSON type matches. */
+    bool MatchesType(const UniValue& result) const;
 };
 
 struct RPCResults {
@@ -333,7 +336,7 @@ class RPCHelpMan
     using RPCMethodImpl = std::function<UniValue(const RPCHelpMan&, const JSONRPCRequest&)>;
     RPCHelpMan(std::string name, std::string description, std::vector<RPCArg> args, RPCResults results, RPCExamples examples, RPCMethodImpl fun);
 
-    UniValue HandleRequest(const JSONRPCRequest& request);
+    UniValue HandleRequest(const JSONRPCRequest& request) const;
     std::string ToString() const;
     /** Return the named args that need to be converted from string to another JSON type */
     UniValue GetArgMap() const;