Refactor ledger_entry RPC source code and tests (#5237)

This is a major refactor of LedgerEntry.cpp. It adds a number of helper functions to make the code easier to maintain.

It also splits up the ledger and ledger_entry tests into different files, and cleans up the ledger_entry tests to make them easier to write and maintain.

This refactor also caught a few bugs in some of the other RPC processing, so those are fixed along the way.

include/xrpl/protocol/ErrorCodes.h

@@ -157,7 +157,12 @@ enum error_code_i {
     // Pathfinding
     rpcDOMAIN_MALFORMED = 97,
 
-    rpcLAST = rpcDOMAIN_MALFORMED  // rpcLAST should always equal the last code.
+    // ledger_entry
+    rpcENTRY_NOT_FOUND = 98,
+    rpcUNEXPECTED_LEDGER_TYPE = 99,
+
+    rpcLAST =
+        rpcUNEXPECTED_LEDGER_TYPE  // rpcLAST should always equal the last code.
 };
 
 /** Codes returned in the `warnings` array of certain RPC commands.

include/xrpl/protocol/jss.h

@@ -68,9 +68,13 @@ JSS(Flags);                // in/out: TransactionSign; field.
 JSS(Holder);               // field.
 JSS(Invalid);              //
 JSS(Issuer);               // in: Credential transactions
+JSS(IssuingChainDoor);     // field.
+JSS(IssuingChainIssue);    // field.
 JSS(LastLedgerSequence);   // in: TransactionSign; field
 JSS(LastUpdateTime);       // field.
 JSS(LimitAmount);          // field.
+JSS(LockingChainDoor);     // field.
+JSS(LockingChainIssue);    // field.
 JSS(NetworkID);            // field.
 JSS(LPTokenOut);           // in: AMM Liquidity Provider deposit tokens
 JSS(LPTokenIn);            // in: AMM Liquidity Provider withdraw tokens

src/libxrpl/json/json_value.cpp

@@ -24,6 +24,7 @@
 #include <xrpl/json/json_value.h>
 #include <xrpl/json/json_writer.h>
 
+#include <cmath>
 #include <cstdlib>
 #include <cstring>
 #include <string>
@@ -685,7 +686,9 @@ Value::isConvertibleTo(ValueType other) const
                 (other == intValue && value_.real_ >= minInt &&
                  value_.real_ <= maxInt) ||
                 (other == uintValue && value_.real_ >= 0 &&
-                 value_.real_ <= maxUInt) ||
+                 value_.real_ <= maxUInt &&
+                 std::fabs(round(value_.real_) - value_.real_) <
+                     std::numeric_limits<double>::epsilon()) ||
                 other == realValue || other == stringValue ||
                 other == booleanValue;
 

src/libxrpl/protocol/ErrorCodes.cpp

@@ -117,7 +117,10 @@ constexpr static ErrorInfo unorderedErrorInfos[]{
     {rpcORACLE_MALFORMED,       "oracleMalformed",      "Oracle request is malformed.", 400},
     {rpcBAD_CREDENTIALS,        "badCredentials",       "Credentials do not exist, are not accepted, or have expired.", 400},
     {rpcTX_SIGNED,              "transactionSigned",    "Transaction should not be signed.", 400},
-    {rpcDOMAIN_MALFORMED,       "domainMalformed",      "Domain is malformed.", 400}};
+    {rpcDOMAIN_MALFORMED,       "domainMalformed",      "Domain is malformed.", 400},
+    {rpcENTRY_NOT_FOUND,        "entryNotFound",        "Entry not found.", 400},
+    {rpcUNEXPECTED_LEDGER_TYPE, "unexpectedLedgerType", "Unexpected ledger type.", 400},
+};
 // clang-format on
 
 // Sort and validate unorderedErrorInfos at compile time.  Should be

src/libxrpl/protocol/STXChainBridge.cpp

@@ -27,6 +27,7 @@
 #include <xrpl/protocol/STObject.h>
 #include <xrpl/protocol/STXChainBridge.h>
 #include <xrpl/protocol/Serializer.h>
+#include <xrpl/protocol/jss.h>
 
 #include <boost/format/free_funcs.hpp>
 
@@ -98,12 +99,10 @@ STXChainBridge::STXChainBridge(SField const& name, Json::Value const& v)
     };
     checkExtra(v);
 
-    Json::Value const& lockingChainDoorStr =
+    Json::Value const& lockingChainDoorStr = v[jss::LockingChainDoor];
-        v[sfLockingChainDoor.getJsonName()];
+    Json::Value const& lockingChainIssue = v[jss::LockingChainIssue];
-    Json::Value const& lockingChainIssue = v[sfLockingChainIssue.getJsonName()];
+    Json::Value const& issuingChainDoorStr = v[jss::IssuingChainDoor];
-    Json::Value const& issuingChainDoorStr =
+    Json::Value const& issuingChainIssue = v[jss::IssuingChainIssue];
-        v[sfIssuingChainDoor.getJsonName()];
-    Json::Value const& issuingChainIssue = v[sfIssuingChainIssue.getJsonName()];
 
     if (!lockingChainDoorStr.isString())
     {
@@ -161,10 +160,10 @@ Json::Value
 STXChainBridge::getJson(JsonOptions jo) const
 {
     Json::Value v;
-    v[sfLockingChainDoor.getJsonName()] = lockingChainDoor_.getJson(jo);
+    v[jss::LockingChainDoor] = lockingChainDoor_.getJson(jo);
-    v[sfLockingChainIssue.getJsonName()] = lockingChainIssue_.getJson(jo);
+    v[jss::LockingChainIssue] = lockingChainIssue_.getJson(jo);
-    v[sfIssuingChainDoor.getJsonName()] = issuingChainDoor_.getJson(jo);
+    v[jss::IssuingChainDoor] = issuingChainDoor_.getJson(jo);
-    v[sfIssuingChainIssue.getJsonName()] = issuingChainIssue_.getJson(jo);
+    v[jss::IssuingChainIssue] = issuingChainIssue_.getJson(jo);
     return v;
 }
 

src/test/app/Vault_test.cpp

@@ -3028,18 +3028,6 @@ class Vault_test : public beast::unit_test::suite
                 "malformedRequest");
         }
 
-        {
-            testcase("RPC ledger_entry zero seq");
-            Json::Value jvParams;
-            jvParams[jss::ledger_index] = jss::validated;
-            jvParams[jss::vault][jss::owner] = issuer.human();
-            jvParams[jss::vault][jss::seq] = 0;
-            auto jvVault = env.rpc("json", "ledger_entry", to_string(jvParams));
-            BEAST_EXPECT(
-                jvVault[jss::result][jss::error].asString() ==
-                "malformedRequest");
-        }
-
         {
             testcase("RPC ledger_entry negative seq");
             Json::Value jvParams;

src/test/jtx/impl/xchain_bridge.cpp

@@ -44,10 +44,10 @@ bridge(
     Issue const& issuingChainIssue)
 {
     Json::Value jv;
-    jv[sfLockingChainDoor.getJsonName()] = lockingChainDoor.human();
+    jv[jss::LockingChainDoor] = lockingChainDoor.human();
-    jv[sfLockingChainIssue.getJsonName()] = to_json(lockingChainIssue);
+    jv[jss::LockingChainIssue] = to_json(lockingChainIssue);
-    jv[sfIssuingChainDoor.getJsonName()] = issuingChainDoor.human();
+    jv[jss::IssuingChainDoor] = issuingChainDoor.human();
-    jv[sfIssuingChainIssue.getJsonName()] = to_json(issuingChainIssue);
+    jv[jss::IssuingChainIssue] = to_json(issuingChainIssue);
     return jv;
 }
 
@@ -60,10 +60,10 @@ bridge_rpc(
     Issue const& issuingChainIssue)
 {
     Json::Value jv;
-    jv[sfLockingChainDoor.getJsonName()] = lockingChainDoor.human();
+    jv[jss::LockingChainDoor] = lockingChainDoor.human();
-    jv[sfLockingChainIssue.getJsonName()] = to_json(lockingChainIssue);
+    jv[jss::LockingChainIssue] = to_json(lockingChainIssue);
-    jv[sfIssuingChainDoor.getJsonName()] = issuingChainDoor.human();
+    jv[jss::IssuingChainDoor] = issuingChainDoor.human();
-    jv[sfIssuingChainIssue.getJsonName()] = to_json(issuingChainIssue);
+    jv[jss::IssuingChainIssue] = to_json(issuingChainIssue);
     return jv;
 }
 

src/xrpld/rpc/detail/RPCHelpers.cpp

@@ -190,7 +190,7 @@ getAccountObjects(
 
     auto& jvObjects = (jvResult[jss::account_objects] = Json::arrayValue);
 
-    // this is a mutable version of limit, used to seemlessly switch
+    // this is a mutable version of limit, used to seamlessly switch
     // to iterating directory entries when nftokenpages are exhausted
     uint32_t mlimit = limit;
 
@@ -373,7 +373,7 @@ ledgerFromRequest(T& ledger, JsonContext& context)
             indexValue = legacyLedger;
     }
 
-    if (hashValue)
+    if (!hashValue.isNull())
     {
         if (!hashValue.isString())
             return {rpcINVALID_PARAMS, "ledgerHashNotString"};
@@ -384,6 +384,9 @@ ledgerFromRequest(T& ledger, JsonContext& context)
         return getLedger(ledger, ledgerHash, context);
     }
 
+    if (!indexValue.isConvertibleTo(Json::stringValue))
+        return {rpcINVALID_PARAMS, "ledgerIndexMalformed"};
+
     auto const index = indexValue.asString();
 
     if (index == "current" || index.empty())
@@ -395,11 +398,11 @@ ledgerFromRequest(T& ledger, JsonContext& context)
     if (index == "closed")
         return getLedger(ledger, LedgerShortcut::CLOSED, context);
 
-    std::uint32_t iVal;
+    std::uint32_t val;
-    if (beast::lexicalCastChecked(iVal, index))
+    if (!beast::lexicalCastChecked(val, index))
-        return getLedger(ledger, iVal, context);
+        return {rpcINVALID_PARAMS, "ledgerIndexMalformed"};
 
-    return {rpcINVALID_PARAMS, "ledgerIndexMalformed"};
+    return getLedger(ledger, val, context);
 }
 }  // namespace
 
@@ -586,7 +589,7 @@ getLedger(T& ledger, LedgerShortcut shortcut, Context& context)
     return Status::OK;
 }
 
-// Explicit instantiaion of above three functions
+// Explicit instantiation of above three functions
 template Status
 getLedger<>(std::shared_ptr<ReadView const>&, uint32_t, Context&);
 

src/xrpld/rpc/handlers/LedgerEntryHelpers.h

@@ -0,0 +1,299 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of rippled: https://github.com/ripple/rippled
+    Copyright (c) 2012-2025 Ripple Labs Inc.
+
+    Permission to use, copy, modify, and/or distribute this software for any
+    purpose  with  or without fee is hereby granted, provided that the above
+    copyright notice and this permission notice appear in all copies.
+
+    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
+    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+    ANY  SPECIAL ,  DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
+    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+*/
+//==============================================================================
+
+#include <xrpld/rpc/detail/RPCHelpers.h>
+
+#include <xrpl/basics/StringUtilities.h>
+#include <xrpl/basics/strHex.h>
+#include <xrpl/beast/core/LexicalCast.h>
+#include <xrpl/json/json_errors.h>
+#include <xrpl/protocol/ErrorCodes.h>
+#include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/RPCErr.h>
+#include <xrpl/protocol/STXChainBridge.h>
+#include <xrpl/protocol/jss.h>
+
+#include <functional>
+
+namespace ripple {
+
+namespace LedgerEntryHelpers {
+
+Unexpected<Json::Value>
+missingFieldError(
+    Json::StaticString const field,
+    std::optional<std::string> err = std::nullopt)
+{
+    Json::Value json = Json::objectValue;
+    auto error = RPC::missing_field_message(std::string(field.c_str()));
+    json[jss::error] = err.value_or("malformedRequest");
+    json[jss::error_code] = rpcINVALID_PARAMS;
+    json[jss::error_message] = std::move(error);
+    return Unexpected(json);
+}
+
+Unexpected<Json::Value>
+invalidFieldError(
+    std::string const& err,
+    Json::StaticString const field,
+    std::string const& type)
+{
+    Json::Value json = Json::objectValue;
+    auto error = RPC::expected_field_message(field, type);
+    json[jss::error] = err;
+    json[jss::error_code] = rpcINVALID_PARAMS;
+    json[jss::error_message] = std::move(error);
+    return Unexpected(json);
+}
+
+Unexpected<Json::Value>
+malformedError(std::string const& err, std::string const& message)
+{
+    Json::Value json = Json::objectValue;
+    json[jss::error] = err;
+    json[jss::error_code] = rpcINVALID_PARAMS;
+    json[jss::error_message] = message;
+    return Unexpected(json);
+}
+
+Expected<bool, Json::Value>
+hasRequired(
+    Json::Value const& params,
+    std::initializer_list<Json::StaticString> fields,
+    std::optional<std::string> err = std::nullopt)
+{
+    for (auto const field : fields)
+    {
+        if (!params.isMember(field) || params[field].isNull())
+        {
+            return missingFieldError(field, err);
+        }
+    }
+    return true;
+}
+
+template <class T>
+std::optional<T>
+parse(Json::Value const& param);
+
+template <class T>
+Expected<T, Json::Value>
+required(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    std::string const& err,
+    std::string const& expectedType)
+{
+    if (!params.isMember(fieldName) || params[fieldName].isNull())
+    {
+        return missingFieldError(fieldName);
+    }
+    if (auto obj = parse<T>(params[fieldName]))
+    {
+        return *obj;
+    }
+    return invalidFieldError(err, fieldName, expectedType);
+}
+
+template <>
+std::optional<AccountID>
+parse(Json::Value const& param)
+{
+    if (!param.isString())
+        return std::nullopt;
+
+    auto const account = parseBase58<AccountID>(param.asString());
+    if (!account || account->isZero())
+    {
+        return std::nullopt;
+    }
+
+    return account;
+}
+
+Expected<AccountID, Json::Value>
+requiredAccountID(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    std::string const& err)
+{
+    return required<AccountID>(params, fieldName, err, "AccountID");
+}
+
+std::optional<Blob>
+parseHexBlob(Json::Value const& param, std::size_t maxLength)
+{
+    if (!param.isString())
+        return std::nullopt;
+
+    auto const blob = strUnHex(param.asString());
+    if (!blob || blob->empty() || blob->size() > maxLength)
+        return std::nullopt;
+
+    return blob;
+}
+
+Expected<Blob, Json::Value>
+requiredHexBlob(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    std::size_t maxLength,
+    std::string const& err)
+{
+    if (!params.isMember(fieldName) || params[fieldName].isNull())
+    {
+        return missingFieldError(fieldName);
+    }
+    if (auto blob = parseHexBlob(params[fieldName], maxLength))
+    {
+        return *blob;
+    }
+    return invalidFieldError(err, fieldName, "hex string");
+}
+
+template <>
+std::optional<std::uint32_t>
+parse(Json::Value const& param)
+{
+    if (param.isUInt() || (param.isInt() && param.asInt() >= 0))
+        return param.asUInt();
+
+    if (param.isString())
+    {
+        std::uint32_t v;
+        if (beast::lexicalCastChecked(v, param.asString()))
+            return v;
+    }
+
+    return std::nullopt;
+}
+
+Expected<std::uint32_t, Json::Value>
+requiredUInt32(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    std::string const& err)
+{
+    return required<std::uint32_t>(params, fieldName, err, "number");
+}
+
+template <>
+std::optional<uint256>
+parse(Json::Value const& param)
+{
+    uint256 uNodeIndex;
+    if (!param.isString() || !uNodeIndex.parseHex(param.asString()))
+    {
+        return std::nullopt;
+    }
+
+    return uNodeIndex;
+}
+
+Expected<uint256, Json::Value>
+requiredUInt256(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    std::string const& err)
+{
+    return required<uint256>(params, fieldName, err, "Hash256");
+}
+
+template <>
+std::optional<uint192>
+parse(Json::Value const& param)
+{
+    uint192 field;
+    if (!param.isString() || !field.parseHex(param.asString()))
+    {
+        return std::nullopt;
+    }
+
+    return field;
+}
+
+Expected<uint192, Json::Value>
+requiredUInt192(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    std::string const& err)
+{
+    return required<uint192>(params, fieldName, err, "Hash192");
+}
+
+Expected<STXChainBridge, Json::Value>
+parseBridgeFields(Json::Value const& params)
+{
+    if (auto const value = hasRequired(
+            params,
+            {jss::LockingChainDoor,
+             jss::LockingChainIssue,
+             jss::IssuingChainDoor,
+             jss::IssuingChainIssue});
+        !value)
+    {
+        return Unexpected(value.error());
+    }
+
+    auto const lockingChainDoor = requiredAccountID(
+        params, jss::LockingChainDoor, "malformedLockingChainDoor");
+    if (!lockingChainDoor)
+    {
+        return Unexpected(lockingChainDoor.error());
+    }
+
+    auto const issuingChainDoor = requiredAccountID(
+        params, jss::IssuingChainDoor, "malformedIssuingChainDoor");
+    if (!issuingChainDoor)
+    {
+        return Unexpected(issuingChainDoor.error());
+    }
+
+    Issue lockingChainIssue;
+    try
+    {
+        lockingChainIssue = issueFromJson(params[jss::LockingChainIssue]);
+    }
+    catch (std::runtime_error const& ex)
+    {
+        return invalidFieldError(
+            "malformedIssue", jss::LockingChainIssue, "Issue");
+    }
+
+    Issue issuingChainIssue;
+    try
+    {
+        issuingChainIssue = issueFromJson(params[jss::IssuingChainIssue]);
+    }
+    catch (std::runtime_error const& ex)
+    {
+        return invalidFieldError(
+            "malformedIssue", jss::IssuingChainIssue, "Issue");
+    }
+
+    return STXChainBridge(
+        *lockingChainDoor,
+        lockingChainIssue,
+        *issuingChainDoor,
+        issuingChainIssue);
+}
+
+}  // namespace LedgerEntryHelpers
+
+}  // namespace ripple