Merge bitcoin/bitcoin#23083: rpc: Fail to return undocumented or misdocumented JSON

fc892c3 rpc: Fail to return undocumented or misdocumented JSON (MarcoFalke)
f4bc4a7 rpc: Add m_skip_type_check to RPCResult (MarcoFalke)

Pull request description:

  This avoids documentation shortcomings such as the ones fixed in commit e7b6272, 138d55e, 577bd51, f8c84e0, 0ee9a00, 13f4185, or faecb2e

ACKs for top commit:
  fanquake:
    ACK fc892c3 - tested that this catches issue, i.e #24691:

Tree-SHA512: 9d0d7e6291bfc6f67541a4ff746d374ad8751fefcff6d103d8621c0298b190ab1d209ce96cfc3a0d4a6a5460a9f9bb790eb96027b16e5ff91f2512e40c92ca84

Author: fanquake

src/rpc/util.cpp

@@ -774,7 +774,7 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const
     // Elements in a JSON structure (dictionary or array) are separated by a comma
     const std::string maybe_separator{outer_type != OuterType::NONE ? "," : ""};
 
-    // The key name if recursed into an dictionary
+    // The key name if recursed into a dictionary
     const std::string maybe_key{
         outer_type == OuterType::OBJ ?
             "\"" + this->m_key_name + "\" : " :
@@ -865,10 +865,11 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const
 
 bool RPCResult::MatchesType(const UniValue& result) const
 {
-    switch (m_type) {
-    case Type::ELISION: {
-        return false;
+    if (m_skip_type_check) {
+        return true;
     }
+    switch (m_type) {
+    case Type::ELISION:
     case Type::ANY: {
         return true;
     }
@@ -889,11 +890,52 @@ bool RPCResult::MatchesType(const UniValue& result) const
     }
     case Type::ARR_FIXED:
     case Type::ARR: {
-        return UniValue::VARR == result.getType();
+        if (UniValue::VARR != result.getType()) return false;
+        for (size_t i{0}; i < result.get_array().size(); ++i) {
+            // If there are more results than documented, re-use the last doc_inner.
+            const RPCResult& doc_inner{m_inner.at(std::min(m_inner.size() - 1, i))};
+            if (!doc_inner.MatchesType(result.get_array()[i])) return false;
+        }
+        return true; // empty result array is valid
     }
     case Type::OBJ_DYN:
     case Type::OBJ: {
-        return UniValue::VOBJ == result.getType();
+        if (UniValue::VOBJ != result.getType()) return false;
+        if (!m_inner.empty() && m_inner.at(0).m_type == Type::ELISION) return true;
+        if (m_type == Type::OBJ_DYN) {
+            const RPCResult& doc_inner{m_inner.at(0)}; // Assume all types are the same, randomly pick the first
+            for (size_t i{0}; i < result.get_obj().size(); ++i) {
+                if (!doc_inner.MatchesType(result.get_obj()[i])) {
+                    return false;
+                }
+            }
+            return true; // empty result obj is valid
+        }
+        std::set<std::string> doc_keys;
+        for (const auto& doc_entry : m_inner) {
+            doc_keys.insert(doc_entry.m_key_name);
+        }
+        std::map<std::string, UniValue> result_obj;
+        result.getObjMap(result_obj);
+        for (const auto& result_entry : result_obj) {
+            if (doc_keys.find(result_entry.first) == doc_keys.end()) {
+                return false; // missing documentation
+            }
+        }
+
+        for (const auto& doc_entry : m_inner) {
+            const auto result_it{result_obj.find(doc_entry.m_key_name)};
+            if (result_it == result_obj.end()) {
+                if (!doc_entry.m_optional) {
+                    return false; // result is missing a required key
+                }
+                continue;
+            }
+            if (!doc_entry.MatchesType(result_it->second)) {
+                return false; // wrong type
+            }
+        }
+        return true;
     }
     } // no default case, so the compiler can warn about missing cases
     CHECK_NONFATAL(false);

src/rpc/util.h

@@ -256,6 +256,7 @@ struct RPCResult {
     const std::string m_key_name;         //!< Only used for dicts
     const std::vector<RPCResult> m_inner; //!< Only used for arrays or dicts
     const bool m_optional;
+    const bool m_skip_type_check;
     const std::string m_description;
     const std::string m_cond;
 
@@ -270,6 +271,7 @@ struct RPCResult {
           m_key_name{std::move(m_key_name)},
           m_inner{std::move(inner)},
           m_optional{optional},
+          m_skip_type_check{false},
           m_description{std::move(description)},
           m_cond{std::move(cond)}
     {
@@ -290,11 +292,13 @@ struct RPCResult {
         const std::string m_key_name,
         const bool optional,
         const std::string description,
-        const std::vector<RPCResult> inner = {})
+        const std::vector<RPCResult> inner = {},
+        bool skip_type_check = false)
         : m_type{std::move(type)},
           m_key_name{std::move(m_key_name)},
           m_inner{std::move(inner)},
           m_optional{optional},
+          m_skip_type_check{skip_type_check},
           m_description{std::move(description)},
           m_cond{}
     {
@@ -305,8 +309,9 @@ struct RPCResult {
         const Type type,
         const std::string m_key_name,
         const std::string description,
-        const std::vector<RPCResult> inner = {})
-        : RPCResult{type, m_key_name, false, description, inner} {}
+        const std::vector<RPCResult> inner = {},
+        bool skip_type_check = false)
+        : RPCResult{type, m_key_name, false, description, inner, skip_type_check} {}
 
     /** Append the sections of the result. */
     void ToSections(Sections& sections, OuterType outer_type = OuterType::NONE, const int current_indent = 0) const;

src/wallet/rpc/wallet.cpp

@@ -56,7 +56,7 @@ static RPCHelpMan getwalletinfo()
                         {
                             {RPCResult::Type::NUM, "duration", "elapsed seconds since scan start"},
                             {RPCResult::Type::NUM, "progress", "scanning progress percentage [0.0, 1.0]"},
-                        }},
+                        }, /*skip_type_check=*/true},
                         {RPCResult::Type::BOOL, "descriptors", "whether this wallet uses descriptors for scriptPubKey management"},
                         {RPCResult::Type::BOOL, "external_signer", "whether this wallet is configured to use an external signer such as a hardware wallet"},
                     }},