From 2df13ac6af09cedc6ab46dc3357a75bff79ee484 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 5 May 2026 13:18:26 -0400 Subject: [PATCH] fix: Fix regressions in `server_definitions` (#7008) --- src/test/rpc/ServerDefinitions_test.cpp | 51 ++++++++++++++----- .../server_info/ServerDefinitions.cpp | 25 ++++----- 2 files changed, 47 insertions(+), 29 deletions(-) diff --git a/src/test/rpc/ServerDefinitions_test.cpp b/src/test/rpc/ServerDefinitions_test.cpp index d53c845726..bac1b4f7a5 100644 --- a/src/test/rpc/ServerDefinitions_test.cpp +++ b/src/test/rpc/ServerDefinitions_test.cpp @@ -8,6 +8,9 @@ #include #include +#include +#include + namespace xrpl::test { class ServerDefinitions_test : public beast::unit_test::Suite @@ -37,20 +40,23 @@ public: { auto const firstField = result[jss::result][jss::FIELDS][0u]; - BEAST_EXPECT(firstField[0u].asString() == "Generic"); + BEAST_EXPECT(firstField[0u].asString() == "Invalid"); BEAST_EXPECT(firstField[1][jss::isSerialized].asBool() == false); BEAST_EXPECT(firstField[1][jss::isSigningField].asBool() == false); BEAST_EXPECT(firstField[1][jss::isVLEncoded].asBool() == false); - BEAST_EXPECT(firstField[1][jss::nth].asUInt() == 0); + BEAST_EXPECT(firstField[1][jss::nth].asInt() == -1); BEAST_EXPECT(firstField[1][jss::type].asString() == "Unknown"); } - BEAST_EXPECT( - result[jss::result][jss::LEDGER_ENTRY_TYPES]["AccountRoot"].asUInt() == 97); - BEAST_EXPECT( - result[jss::result][jss::TRANSACTION_RESULTS]["tecDIR_FULL"].asUInt() == 121); - BEAST_EXPECT(result[jss::result][jss::TRANSACTION_TYPES]["Payment"].asUInt() == 0); - BEAST_EXPECT(result[jss::result][jss::TYPES]["AccountID"].asUInt() == 8); + { + auto const field = result[jss::result][jss::FIELDS][6u]; + BEAST_EXPECT(field[0u].asString() == "LedgerEntryType"); + BEAST_EXPECT(field[1][jss::isSerialized].asBool() == true); + BEAST_EXPECT(field[1][jss::isSigningField].asBool() == true); + BEAST_EXPECT(field[1][jss::isVLEncoded].asBool() == false); + BEAST_EXPECT(field[1][jss::nth].asUInt() == 1); + BEAST_EXPECT(field[1][jss::type].asString() == "UInt16"); + } // check exception SFields { @@ -74,17 +80,34 @@ public: BEAST_EXPECT(fieldExists("index")); } + // verify no duplicate field names in FIELDS array + { + std::set fieldNames; + for (auto const& field : result[jss::result][jss::FIELDS]) + { + auto const name = field[0u].asString(); + BEAST_EXPECT(fieldNames.insert(name).second); + } + } + // test that base_uint types are replaced with "Hash" prefix { auto const types = result[jss::result][jss::TYPES]; - BEAST_EXPECT(types["Hash128"].asUInt() == 4); - BEAST_EXPECT(types["Hash160"].asUInt() == 17); - BEAST_EXPECT(types["Hash192"].asUInt() == 21); - BEAST_EXPECT(types["Hash256"].asUInt() == 5); - BEAST_EXPECT(types["Hash384"].asUInt() == 22); - BEAST_EXPECT(types["Hash512"].asUInt() == 23); + BEAST_EXPECT(types.isMember("Hash128") && types["Hash128"].asUInt() == 4); + BEAST_EXPECT(types.isMember("Hash160") && types["Hash160"].asUInt() == 17); + BEAST_EXPECT(types.isMember("Hash192") && types["Hash192"].asUInt() == 21); + BEAST_EXPECT(types.isMember("Hash256") && types["Hash256"].asUInt() == 5); + BEAST_EXPECT(types.isMember("Hash384") && types["Hash384"].asUInt() == 22); + BEAST_EXPECT(types.isMember("Hash512") && types["Hash512"].asUInt() == 23); } + BEAST_EXPECT( + result[jss::result][jss::LEDGER_ENTRY_TYPES]["AccountRoot"].asUInt() == 97); + BEAST_EXPECT( + result[jss::result][jss::TRANSACTION_RESULTS]["tecDIR_FULL"].asUInt() == 121); + BEAST_EXPECT(result[jss::result][jss::TRANSACTION_TYPES]["Payment"].asUInt() == 0); + BEAST_EXPECT(result[jss::result][jss::TYPES]["AccountID"].asUInt() == 8); + // test the properties of the LEDGER_ENTRY_FLAGS section { BEAST_EXPECT(result[jss::result].isMember(jss::LEDGER_ENTRY_FLAGS)); diff --git a/src/xrpld/rpc/handlers/server_info/ServerDefinitions.cpp b/src/xrpld/rpc/handlers/server_info/ServerDefinitions.cpp index ca887bb0c8..6f75dec61b 100644 --- a/src/xrpld/rpc/handlers/server_info/ServerDefinitions.cpp +++ b/src/xrpld/rpc/handlers/server_info/ServerDefinitions.cpp @@ -149,18 +149,6 @@ ServerDefinitions::ServerDefinitions() : defs_{json::ObjectValue} defs_[jss::FIELDS] = json::ArrayValue; uint32_t i = 0; - { - json::Value a = json::ArrayValue; - a[0U] = "Generic"; - json::Value v = json::ObjectValue; - v[jss::nth] = 0; - v[jss::isVLEncoded] = false; - v[jss::isSerialized] = false; - v[jss::isSigningField] = false; - v[jss::type] = "Unknown"; - a[1U] = v; - defs_[jss::FIELDS][i++] = a; - } { json::Value a = json::ArrayValue; @@ -227,21 +215,28 @@ ServerDefinitions::ServerDefinitions() : defs_{json::ObjectValue} defs_[jss::FIELDS][i++] = a; } - for (auto const& [code, field] : xrpl::SField::getKnownCodeToField()) + // copy into a sorted map to ensure deterministic output order (sorted by fieldCode) + static std::map const kSORTED_FIELDS( + xrpl::SField::getKnownCodeToField().begin(), xrpl::SField::getKnownCodeToField().end()); + + for (auto const& [code, field] : kSORTED_FIELDS) { if (field->fieldName.empty()) continue; json::Value innerObj = json::ObjectValue; - uint32_t type = field->fieldType; + int32_t const type = field->fieldType; innerObj[jss::nth] = field->fieldValue; // whether the field is variable-length encoded this means that the length is included // before the content innerObj[jss::isVLEncoded] = - (type == 7U /* Blob */ || type == 8U /* AccountID */ || type == 19U /* Vector256 */); + (type == STI_VL || type == STI_ACCOUNT || type == STI_VECTOR256); + static_assert( + STI_VL == 7U && STI_ACCOUNT == 8U && STI_VECTOR256 == 19U, + "STI_VL, STI_ACCOUNT, STI_VECTOR256 must be 7, 8, 19 respectively"); // whether the field is included in serialization innerObj[jss::isSerialized] =