Merge branch 'develop' into ximinez/fix/validator-cache

- If any [validator_list_keys] are not available after all
  [validator_list_sites] have had a chance to be queried, then fall
  back to loading cache files. Currently, cache files are only used if
  no sites are defined, or the request to one of them has an error. It
  does not include cases where not enough sites are defined, or if a
  site returns an invalid VL (or something else entirely).
- Resolves #5320

.github/scripts/strategy-matrix/generate.py

@@ -20,8 +20,8 @@ class Config:
 Generate a strategy matrix for GitHub Actions CI.
 
 On each PR commit we will build a selection of Debian, RHEL, Ubuntu, MacOS, and
-Windows configurations, while upon merge into the develop or release branches,
+Windows configurations, while upon merge into the develop, release, or master
-we will build all configurations, and test most of them.
+branches, we will build all configurations, and test most of them.
 
 We will further set additional CMake arguments as follows:
 - All builds will have the `tests`, `werr`, and `xrpld` options.

.github/workflows/on-pr.yml

@@ -125,7 +125,7 @@ jobs:
     needs:
       - should-run
       - build-test
-    if: ${{ needs.should-run.outputs.go == 'true' && startsWith(github.ref, 'refs/heads/release') }}
+    if: ${{ needs.should-run.outputs.go == 'true' && (startsWith(github.base_ref, 'release') || github.base_ref == 'master') }}
     uses: ./.github/workflows/reusable-notify-clio.yml
     secrets:
       clio_notify_token: ${{ secrets.CLIO_NOTIFY_TOKEN }}

.github/workflows/on-trigger.yml

@@ -1,8 +1,9 @@
-# This workflow runs all workflows to build and test the code on various Linux
+# This workflow runs all workflows to build the dependencies required for the
-# flavors, as well as on MacOS and Windows, on a scheduled basis, on merge into
+# project on various Linux flavors, as well as on MacOS and Windows, on a
-# the 'develop' or 'release*' branches, or when requested manually. Upon
+# scheduled basis, on merge into the 'develop', 'release', or 'master' branches,
-# successful completion, it also uploads the built libxrpl package to the Conan
+# or manually. The missing commits check is only run when the code is merged
-# remote.
+# into the 'develop' or 'release' branches, and the documentation is built when
+# the code is merged into the 'develop' branch.
 name: Trigger
 
 on:
@@ -10,6 +11,7 @@ on:
     branches:
       - "develop"
       - "release*"
+      - "master"
     paths:
       # These paths are unique to `on-trigger.yml`.
       - ".github/workflows/on-trigger.yml"
@@ -68,10 +70,10 @@ jobs:
     with:
       # Enable ccache only for events targeting the XRPLF repository, since
       # other accounts will not have access to our remote cache storage.
-      # However, we do not enable ccache for events targeting a release branch,
+      # However, we do not enable ccache for events targeting the master or a
-      # to protect against the rare case that the output produced by ccache is
+      # release branch, to protect against the rare case that the output
-      # not identical to a regular compilation.
+      # produced by ccache is not identical to a regular compilation.
-      ccache_enabled: ${{ github.repository_owner == 'XRPLF' && !startsWith(github.ref, 'refs/heads/release') }}
+      ccache_enabled: ${{ github.repository_owner == 'XRPLF' && !(github.base_ref == 'master' || startsWith(github.base_ref, 'release')) }}
       os: ${{ matrix.os }}
       strategy_matrix: ${{ github.event_name == 'schedule' && 'all' || 'minimal' }}
     secrets:

.github/workflows/pre-commit.yml

@@ -3,9 +3,7 @@ name: Run pre-commit hooks
 on:
   pull_request:
   push:
-    branches:
+    branches: [develop, release, master]
-      - "develop"
-      - "release*"
   workflow_dispatch:
 
 jobs:

src/test/rpc/LedgerEntry_test.cpp

@@ -5,8 +5,6 @@
 #include <test/jtx/multisign.h>
 #include <test/jtx/xchain_bridge.h>
 
-#include <xrpld/app/tx/apply.h>
-
 #include <xrpl/beast/unit_test.h>
 #include <xrpl/json/json_value.h>
 #include <xrpl/protocol/AccountID.h>
@@ -32,7 +30,6 @@ enum class FieldType {
     CurrencyField,
     HashField,
     HashOrObjectField,
-    FixedHashField,
     IssueField,
     ObjectField,
     StringField,
@@ -89,7 +86,6 @@ getTypeName(FieldType typeID)
         case FieldType::CurrencyField:
             return "Currency";
         case FieldType::HashField:
-        case FieldType::FixedHashField:
             return "hex string";
         case FieldType::HashOrObjectField:
             return "hex string or object";
@@ -206,7 +202,6 @@ class LedgerEntry_test : public beast::unit_test::suite
         static auto const& badBlobValues = remove({3, 7, 8, 16});
         static auto const& badCurrencyValues = remove({14});
         static auto const& badHashValues = remove({2, 3, 7, 8, 16});
-        static auto const& badFixedHashValues = remove({1, 2, 3, 4, 7, 8, 16});
         static auto const& badIndexValues = remove({12, 16, 18, 19});
         static auto const& badUInt32Values = remove({2, 3});
         static auto const& badUInt64Values = remove({2, 3});
@@ -227,8 +222,6 @@ class LedgerEntry_test : public beast::unit_test::suite
                 return badHashValues;
             case FieldType::HashOrObjectField:
                 return badIndexValues;
-            case FieldType::FixedHashField:
-                return badFixedHashValues;
             case FieldType::IssueField:
                 return badIssueValues;
             case FieldType::UInt32Field:
@@ -724,12 +717,7 @@ class LedgerEntry_test : public beast::unit_test::suite
         }
 
         // negative tests
-        testMalformedField(
+        runLedgerEntryTest(env, jss::amendments);
-            env,
-            Json::Value{},
-            jss::amendments,
-            FieldType::FixedHashField,
-            "malformedRequest");
     }
 
     void
@@ -1550,12 +1538,7 @@ class LedgerEntry_test : public beast::unit_test::suite
         }
 
         // negative tests
-        testMalformedField(
+        runLedgerEntryTest(env, jss::fee);
-            env,
-            Json::Value{},
-            jss::fee,
-            FieldType::FixedHashField,
-            "malformedRequest");
     }
 
     void
@@ -1578,12 +1561,7 @@ class LedgerEntry_test : public beast::unit_test::suite
         }
 
         // negative tests
-        testMalformedField(
+        runLedgerEntryTest(env, jss::hashes);
-            env,
-            Json::Value{},
-            jss::hashes,
-            FieldType::FixedHashField,
-            "malformedRequest");
     }
 
     void
@@ -1708,12 +1686,7 @@ class LedgerEntry_test : public beast::unit_test::suite
         }
 
         // negative tests
-        testMalformedField(
+        runLedgerEntryTest(env, jss::nunl);
-            env,
-            Json::Value{},
-            jss::nunl,
-            FieldType::FixedHashField,
-            "malformedRequest");
     }
 
     void
@@ -2370,438 +2343,6 @@ class LedgerEntry_test : public beast::unit_test::suite
         }
     }
 
-    /// Test the ledger entry types that don't take parameters
-    void
-    testFixed()
-    {
-        using namespace test::jtx;
-
-        Account const alice{"alice"};
-        Account const bob{"bob"};
-
-        Env env{*this, envconfig([](auto cfg) {
-                    cfg->START_UP = Config::FRESH;
-                    return cfg;
-                })};
-
-        env.close();
-
-        /** Verifies that the RPC result has the expected data
-         *
-         * @param good: Indicates that the request should have succeeded
-         *   and returned a ledger object of `expectedType` type.
-         * @param jv: The RPC result Json value
-         * @param expectedType: The type that the ledger object should
-         *   have if "good".
-         * @param expectedError: Optional. The expected error if not
-         *   good. Defaults to "entryNotFound".
-         */
-        auto checkResult =
-            [&](bool good,
-                Json::Value const& jv,
-                Json::StaticString const& expectedType,
-                std::optional<std::string> const& expectedError = {}) {
-                if (good)
-                {
-                    BEAST_EXPECTS(
-                        jv.isObject() && jv.isMember(jss::result) &&
-                            !jv[jss::result].isMember(jss::error) &&
-                            jv[jss::result].isMember(jss::node) &&
-                            jv[jss::result][jss::node].isMember(
-                                sfLedgerEntryType.jsonName) &&
-                            jv[jss::result][jss::node]
-                              [sfLedgerEntryType.jsonName] == expectedType,
-                        to_string(jv));
-                }
-                else
-                {
-                    BEAST_EXPECTS(
-                        jv.isObject() && jv.isMember(jss::result) &&
-                            jv[jss::result].isMember(jss::error) &&
-                            !jv[jss::result].isMember(jss::node) &&
-                            jv[jss::result][jss::error] ==
-                                expectedError.value_or("entryNotFound"),
-                        to_string(jv));
-                }
-            };
-
-        /** Runs a series of tests for a given fixed-position ledger
-         * entry.
-         *
-         * @param field: The Json request field to use.
-         * @param expectedType: The type that the ledger object should
-         *   have if "good".
-         * @param expectedKey: The keylet of the fixed object.
-         * @param good: Indicates whether the object is expected to
-         *   exist.
-         */
-        auto test = [&](Json::StaticString const& field,
-                        Json::StaticString const& expectedType,
-                        Keylet const& expectedKey,
-                        bool good) {
-            testcase << expectedType.c_str() << (good ? "" : " not")
-                     << " found";
-
-            auto const hexKey = strHex(expectedKey.key);
-
-            {
-                // Test bad values
-                // "field":null
-                Json::Value params;
-                params[jss::ledger_index] = jss::validated;
-                params[field] = Json::nullValue;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, expectedType, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // "field":"string"
-                params[jss::ledger_index] = jss::validated;
-                params[field] = "arbitrary string";
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, expectedType, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // "field":false
-                params[jss::ledger_index] = jss::validated;
-                params[field] = false;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, expectedType, "invalidParams");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-
-                // "field":[incorrect index hash]
-                auto const badKey = strHex(expectedKey.key + uint256{1});
-                params[jss::ledger_index] = jss::validated;
-                params[field] = badKey;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, expectedType, "entryNotFound");
-                BEAST_EXPECTS(
-                    jv[jss::result][jss::index] == badKey, to_string(jv));
-            }
-
-            {
-                Json::Value params;
-                // "index":"field" using API 2
-                params[jss::ledger_index] = jss::validated;
-                params[jss::index] = field;
-                params[jss::api_version] = 2;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, expectedType, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            std::string const pdIdx = [&]() {
-                {
-                    Json::Value params;
-                    // Test good values
-                    // Use the "field":true notation
-                    params[jss::ledger_index] = jss::validated;
-                    params[field] = true;
-                    auto const jv =
-                        env.rpc("json", "ledger_entry", to_string(params));
-                    // Index will always be returned for valid parameters.
-                    std::string const pdIdx =
-                        jv[jss::result][jss::index].asString();
-                    BEAST_EXPECTS(hexKey == pdIdx, to_string(jv));
-                    checkResult(good, jv, expectedType);
-
-                    return pdIdx;
-                }
-            }();
-
-            {
-                Json::Value params;
-                // "field":"[index hash]"
-                params[jss::ledger_index] = jss::validated;
-                params[field] = hexKey;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(good, jv, expectedType);
-                BEAST_EXPECT(jv[jss::result][jss::index].asString() == hexKey);
-            }
-
-            {
-                // Bad value
-                // Use the "index":"field" notation with API 2
-                Json::Value params;
-                params[jss::ledger_index] = jss::validated;
-                params[jss::index] = field;
-                params[jss::api_version] = 2;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, expectedType, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // Use the "index":"field" notation with API 3
-                params[jss::ledger_index] = jss::validated;
-                params[jss::index] = field;
-                params[jss::api_version] = 3;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                // Index is correct either way
-                BEAST_EXPECT(jv[jss::result][jss::index].asString() == hexKey);
-                checkResult(good, jv, expectedType);
-            }
-
-            {
-                Json::Value params;
-                // Use the "index":"[index hash]" notation
-                params[jss::ledger_index] = jss::validated;
-                params[jss::index] = pdIdx;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                // Index is correct either way
-                BEAST_EXPECT(jv[jss::result][jss::index].asString() == hexKey);
-                checkResult(good, jv, expectedType);
-            }
-        };
-
-        test(jss::amendments, jss::Amendments, keylet::amendments(), true);
-        test(jss::fee, jss::FeeSettings, keylet::fees(), true);
-        // There won't be an nunl
-        test(jss::nunl, jss::NegativeUNL, keylet::negativeUNL(), false);
-        // Can only get the short skip list this way
-        test(jss::hashes, jss::LedgerHashes, keylet::skip(), true);
-    }
-
-    void
-    testHashes()
-    {
-        using namespace test::jtx;
-
-        Account const alice{"alice"};
-        Account const bob{"bob"};
-
-        Env env{*this, envconfig([](auto cfg) {
-                    cfg->START_UP = Config::FRESH;
-                    return cfg;
-                })};
-
-        env.close();
-
-        /** Verifies that the RPC result has the expected data
-         *
-         * @param good: Indicates that the request should have succeeded
-         *   and returned a ledger object of `expectedType` type.
-         * @param jv: The RPC result Json value
-         * @param expectedCount: The number of Hashes expected in the
-         *   object if "good".
-         * @param expectedError: Optional. The expected error if not
-         *   good. Defaults to "entryNotFound".
-         */
-        auto checkResult =
-            [&](bool good,
-                Json::Value const& jv,
-                int expectedCount,
-                std::optional<std::string> const& expectedError = {}) {
-                if (good)
-                {
-                    BEAST_EXPECTS(
-                        jv.isObject() && jv.isMember(jss::result) &&
-                            !jv[jss::result].isMember(jss::error) &&
-                            jv[jss::result].isMember(jss::node) &&
-                            jv[jss::result][jss::node].isMember(
-                                sfLedgerEntryType.jsonName) &&
-                            jv[jss::result][jss::node]
-                              [sfLedgerEntryType.jsonName] == jss::LedgerHashes,
-                        to_string(jv));
-                    BEAST_EXPECTS(
-                        jv[jss::result].isMember(jss::node) &&
-                            jv[jss::result][jss::node].isMember("Hashes") &&
-                            jv[jss::result][jss::node]["Hashes"].size() ==
-                                expectedCount,
-                        to_string(jv[jss::result][jss::node]["Hashes"].size()));
-                }
-                else
-                {
-                    BEAST_EXPECTS(
-                        jv.isObject() && jv.isMember(jss::result) &&
-                            jv[jss::result].isMember(jss::error) &&
-                            !jv[jss::result].isMember(jss::node) &&
-                            jv[jss::result][jss::error] ==
-                                expectedError.value_or("entryNotFound"),
-                        to_string(jv));
-                }
-            };
-
-        /** Runs a series of tests for a given ledger index.
-         *
-         * @param ledger: The ledger index value of the "hashes" request
-         *   parameter. May not necessarily be a number.
-         * @param expectedKey: The expected keylet of the object.
-         * @param good: Indicates whether the object is expected to
-         *   exist.
-         * @param expectedCount: The number of Hashes expected in the
-         *   object if "good".
-         */
-        auto test = [&](Json::Value ledger,
-                        Keylet const& expectedKey,
-                        bool good,
-                        int expectedCount = 0) {
-            testcase << "LedgerHashes: seq: " << env.current()->header().seq
-                     << " \"hashes\":" << to_string(ledger)
-                     << (good ? "" : " not") << " found";
-
-            auto const hexKey = strHex(expectedKey.key);
-
-            {
-                // Test bad values
-                // "hashes":null
-                Json::Value params;
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = Json::nullValue;
-                auto jv = env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, 0, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // "hashes":"non-uint string"
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = "arbitrary string";
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, 0, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // "hashes":"uint string" is invalid, too
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = "10";
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, 0, "malformedRequest");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // "hashes":false
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = false;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, 0, "invalidParams");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            {
-                Json::Value params;
-                // "hashes":-1
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = -1;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, 0, "internal");
-                BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
-            }
-
-            // "hashes":[incorrect index hash]
-            {
-                Json::Value params;
-                auto const badKey = strHex(expectedKey.key + uint256{1});
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = badKey;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(false, jv, 0, "entryNotFound");
-                BEAST_EXPECT(jv[jss::result][jss::index] == badKey);
-            }
-
-            {
-                Json::Value params;
-                // Test good values
-                // Use the "hashes":ledger notation
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = ledger;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(good, jv, expectedCount);
-                // Index will always be returned for valid parameters.
-                std::string const pdIdx =
-                    jv[jss::result][jss::index].asString();
-                BEAST_EXPECTS(hexKey == pdIdx, strHex(pdIdx));
-            }
-
-            {
-                Json::Value params;
-                // "hashes":"[index hash]"
-                params[jss::ledger_index] = jss::validated;
-                params[jss::hashes] = hexKey;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(good, jv, expectedCount);
-                // Index is correct either way
-                BEAST_EXPECTS(
-                    hexKey == jv[jss::result][jss::index].asString(),
-                    strHex(jv[jss::result][jss::index].asString()));
-            }
-
-            {
-                Json::Value params;
-                // Use the "index":"[index hash]" notation
-                params[jss::ledger_index] = jss::validated;
-                params[jss::index] = hexKey;
-                auto const jv =
-                    env.rpc("json", "ledger_entry", to_string(params));
-                checkResult(good, jv, expectedCount);
-                // Index is correct either way
-                BEAST_EXPECTS(
-                    hexKey == jv[jss::result][jss::index].asString(),
-                    strHex(jv[jss::result][jss::index].asString()));
-            }
-        };
-
-        // short skip list
-        test(true, keylet::skip(), true, 2);
-        // long skip list at index 0
-        test(1, keylet::skip(1), false);
-        // long skip list at index 1
-        test(1 << 17, keylet::skip(1 << 17), false);
-
-        // Close more ledgers, but stop short of the flag ledger
-        for (auto i = env.current()->seq(); i <= 250; ++i)
-            env.close();
-
-        // short skip list
-        test(true, keylet::skip(), true, 249);
-        // long skip list at index 0
-        test(1, keylet::skip(1), false);
-        // long skip list at index 1
-        test(1 << 17, keylet::skip(1 << 17), false);
-
-        // Close a flag ledger so the first "long" skip list is created
-        for (auto i = env.current()->seq(); i <= 260; ++i)
-            env.close();
-
-        // short skip list
-        test(true, keylet::skip(), true, 256);
-        // long skip list at index 0
-        test(1, keylet::skip(1), true, 1);
-        // long skip list at index 1
-        test(1 << 17, keylet::skip(1 << 17), false);
-    }
-
     void
     testCLI()
     {
@@ -2859,8 +2400,6 @@ public:
         testOracleLedgerEntry();
         testMPT();
         testPermissionedDomain();
-        testFixed();
-        testHashes();
         testCLI();
     }
 };

src/xrpld/app/misc/detail/ValidatorSite.cpp

@@ -129,7 +129,12 @@ ValidatorSite::load(
     {
         try
         {
-            sites_.emplace_back(uri);
+            // This is not super efficient, but it doesn't happen often.
+            bool found = std::ranges::any_of(sites_, [&uri](auto const& site) {
+                return site.loadedResource->uri == uri;
+            });
+            if (!found)
+                sites_.emplace_back(uri);
         }
         catch (std::exception const& e)
         {
@@ -191,6 +196,17 @@ ValidatorSite::setTimer(
     std::lock_guard<std::mutex> const& site_lock,
     std::lock_guard<std::mutex> const& state_lock)
 {
+    if (!sites_.empty() &&  //
+        std::ranges::all_of(sites_, [](auto const& site) {
+            return site.lastRefreshStatus.has_value();
+        }))
+    {
+        // If all of the sites have been handled at least once (including
+        // errors and timeouts), call missingSite, which will load the cache
+        // files for any lists that are still unavailable.
+        missingSite(site_lock);
+    }
+
     auto next = std::min_element(
         sites_.begin(), sites_.end(), [](Site const& a, Site const& b) {
             return a.nextRefresh < b.nextRefresh;
@@ -303,13 +319,16 @@ ValidatorSite::onRequestTimeout(std::size_t siteIdx, error_code const& ec)
         // processes a network error. Usually, this function runs first,
         // but on extremely rare occasions, the response handler can run
         // first, which will leave activeResource empty.
-        auto const& site = sites_[siteIdx];
+        auto& site = sites_[siteIdx];
         if (site.activeResource)
             JLOG(j_.warn()) << "Request for " << site.activeResource->uri
                             << " took too long";
         else
             JLOG(j_.error()) << "Request took too long, but a response has "
                                 "already been processed";
+        if (!site.lastRefreshStatus)
+            site.lastRefreshStatus.emplace(Site::Status{
+                clock_type::now(), ListDisposition::invalid, "timeout"});
     }
 
     std::lock_guard lock_state{state_mutex_};

src/xrpld/rpc/handlers/LedgerEntry.cpp

@@ -18,32 +18,6 @@
 
 namespace xrpl {
 
-using FunctionType = std::function<Expected<uint256, Json::Value>(
-    Json::Value const&,
-    Json::StaticString const,
-    unsigned const apiVersion)>;
-
-static Expected<uint256, Json::Value>
-parseFixed(
-    Keylet const& keylet,
-    Json::Value const& params,
-    Json::StaticString const& fieldName,
-    unsigned const apiVersion);
-
-// Helper function to return FunctionType for objects that have a fixed
-// location. That is, they don't take parameters to compute the index.
-// e.g. amendments, fees, negative UNL, etc.
-static FunctionType
-fixed(Keylet const& keylet)
-{
-    return [keylet](
-               Json::Value const& params,
-               Json::StaticString const fieldName,
-               unsigned const apiVersion) -> Expected<uint256, Json::Value> {
-        return parseFixed(keylet, params, fieldName, apiVersion);
-    };
-}
-
 static Expected<uint256, Json::Value>
 parseObjectID(
     Json::Value const& params,
@@ -59,33 +33,13 @@ parseObjectID(
 }
 
 static Expected<uint256, Json::Value>
-parseIndex(
+parseIndex(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    unsigned const apiVersion)
 {
-    if (apiVersion > 2u && params.isString())
-    {
-        std::string const index = params.asString();
-        if (index == jss::amendments.c_str())
-            return keylet::amendments().key;
-        if (index == jss::fee.c_str())
-            return keylet::fees().key;
-        if (index == jss::nunl)
-            return keylet::negativeUNL().key;
-        if (index == jss::hashes)
-            // Note this only finds the "short" skip list. Use "hashes":index to
-            // get the long list.
-            return keylet::skip().key;
-    }
     return parseObjectID(params, fieldName, "hex string");
 }
 
 static Expected<uint256, Json::Value>
-parseAccountRoot(
+parseAccountRoot(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (auto const account = LedgerEntryHelpers::parse<AccountID>(params))
     {
@@ -96,13 +50,14 @@ parseAccountRoot(
         "malformedAddress", fieldName, "AccountID");
 }
 
-auto const parseAmendments = fixed(keylet::amendments());
+static Expected<uint256, Json::Value>
+parseAmendments(Json::Value const& params, Json::StaticString const fieldName)
+{
+    return parseObjectID(params, fieldName, "hex string");
+}
 
 static Expected<uint256, Json::Value>
-parseAMM(
+parseAMM(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!params.isObject())
     {
@@ -130,10 +85,7 @@ parseAMM(
 }
 
 static Expected<uint256, Json::Value>
-parseBridge(
+parseBridge(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!params.isMember(jss::bridge))
     {
@@ -164,19 +116,13 @@ parseBridge(
 }
 
 static Expected<uint256, Json::Value>
-parseCheck(
+parseCheck(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
 
 static Expected<uint256, Json::Value>
-parseCredential(
+parseCredential(Json::Value const& cred, Json::StaticString const fieldName)
-    Json::Value const& cred,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!cred.isObject())
     {
@@ -207,10 +153,7 @@ parseCredential(
 }
 
 static Expected<uint256, Json::Value>
-parseDelegate(
+parseDelegate(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!params.isObject())
     {
@@ -301,10 +244,7 @@ parseAuthorizeCredentials(Json::Value const& jv)
 }
 
 static Expected<uint256, Json::Value>
-parseDepositPreauth(
+parseDepositPreauth(Json::Value const& dp, Json::StaticString const fieldName)
-    Json::Value const& dp,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!dp.isObject())
     {
@@ -357,10 +297,7 @@ parseDepositPreauth(
 }
 
 static Expected<uint256, Json::Value>
-parseDID(
+parseDID(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     auto const account = LedgerEntryHelpers::parse<AccountID>(params);
     if (!account)
@@ -375,8 +312,7 @@ parseDID(
 static Expected<uint256, Json::Value>
 parseDirectoryNode(
     Json::Value const& params,
-    Json::StaticString const fieldName,
+    Json::StaticString const fieldName)
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!params.isObject())
     {
@@ -429,10 +365,7 @@ parseDirectoryNode(
 }
 
 static Expected<uint256, Json::Value>
-parseEscrow(
+parseEscrow(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!params.isObject())
     {
@@ -451,53 +384,20 @@ parseEscrow(
     return keylet::escrow(*id, *seq).key;
 }
 
-auto const parseFeeSettings = fixed(keylet::fees());
-
 static Expected<uint256, Json::Value>
-parseFixed(
+parseFeeSettings(Json::Value const& params, Json::StaticString const fieldName)
-    Keylet const& keylet,
-    Json::Value const& params,
-    Json::StaticString const& fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
-    if (!params.isBool())
+    return parseObjectID(params, fieldName, "hex string");
-    {
-        return parseObjectID(params, fieldName, "hex string");
-    }
-    if (!params.asBool())
-    {
-        return LedgerEntryHelpers::invalidFieldError(
-            "invalidParams", fieldName, "true");
-    }
-
-    return keylet.key;
 }
 
 static Expected<uint256, Json::Value>
-parseLedgerHashes(
+parseLedgerHashes(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    unsigned const apiVersion)
 {
-    if (params.isUInt() || params.isInt())
+    return parseObjectID(params, fieldName, "hex string");
-    {
-        // If the index doesn't parse as a UInt, throw
-        auto const index = params.asUInt();
-
-        // Return the "long" skip list for the given ledger index.
-        auto const keylet = keylet::skip(index);
-        return keylet.key;
-    }
-    // Return the key in `params` or the "short" skip list, which contains
-    // hashes since the last flag ledger.
-    return parseFixed(keylet::skip(), params, fieldName, apiVersion);
 }
 
 static Expected<uint256, Json::Value>
-parseLoanBroker(
+parseLoanBroker(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!params.isObject())
     {
@@ -517,10 +417,7 @@ parseLoanBroker(
 }
 
 static Expected<uint256, Json::Value>
-parseLoan(
+parseLoan(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!params.isObject())
     {
@@ -540,10 +437,7 @@ parseLoan(
 }
 
 static Expected<uint256, Json::Value>
-parseMPToken(
+parseMPToken(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!params.isObject())
     {
@@ -566,8 +460,7 @@ parseMPToken(
 static Expected<uint256, Json::Value>
 parseMPTokenIssuance(
     Json::Value const& params,
-    Json::StaticString const fieldName,
+    Json::StaticString const fieldName)
-    [[maybe_unused]] unsigned const apiVersion)
 {
     auto const mptIssuanceID = LedgerEntryHelpers::parse<uint192>(params);
     if (!mptIssuanceID)
@@ -578,30 +471,25 @@ parseMPTokenIssuance(
 }
 
 static Expected<uint256, Json::Value>
-parseNFTokenOffer(
+parseNFTokenOffer(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
 
 static Expected<uint256, Json::Value>
-parseNFTokenPage(
+parseNFTokenPage(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
 
-auto const parseNegativeUNL = fixed(keylet::negativeUNL());
+static Expected<uint256, Json::Value>
+parseNegativeUNL(Json::Value const& params, Json::StaticString const fieldName)
+{
+    return parseObjectID(params, fieldName, "hex string");
+}
 
 static Expected<uint256, Json::Value>
-parseOffer(
+parseOffer(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!params.isObject())
     {
@@ -622,10 +510,7 @@ parseOffer(
 }
 
 static Expected<uint256, Json::Value>
-parseOracle(
+parseOracle(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!params.isObject())
     {
@@ -646,10 +531,7 @@ parseOracle(
 }
 
 static Expected<uint256, Json::Value>
-parsePayChannel(
+parsePayChannel(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
@@ -657,8 +539,7 @@ parsePayChannel(
 static Expected<uint256, Json::Value>
 parsePermissionedDomain(
     Json::Value const& pd,
-    Json::StaticString const fieldName,
+    Json::StaticString const fieldName)
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (pd.isString())
     {
@@ -687,8 +568,7 @@ parsePermissionedDomain(
 static Expected<uint256, Json::Value>
 parseRippleState(
     Json::Value const& jvRippleState,
-    Json::StaticString const fieldName,
+    Json::StaticString const fieldName)
-    [[maybe_unused]] unsigned const apiVersion)
 {
     Currency uCurrency;
 
@@ -738,19 +618,13 @@ parseRippleState(
 }
 
 static Expected<uint256, Json::Value>
-parseSignerList(
+parseSignerList(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
 
 static Expected<uint256, Json::Value>
-parseTicket(
+parseTicket(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!params.isObject())
     {
@@ -771,10 +645,7 @@ parseTicket(
 }
 
 static Expected<uint256, Json::Value>
-parseVault(
+parseVault(Json::Value const& params, Json::StaticString const fieldName)
-    Json::Value const& params,
-    Json::StaticString const fieldName,
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!params.isObject())
     {
@@ -797,8 +668,7 @@ parseVault(
 static Expected<uint256, Json::Value>
 parseXChainOwnedClaimID(
     Json::Value const& claim_id,
-    Json::StaticString const fieldName,
+    Json::StaticString const fieldName)
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!claim_id.isObject())
     {
@@ -823,8 +693,7 @@ parseXChainOwnedClaimID(
 static Expected<uint256, Json::Value>
 parseXChainOwnedCreateAccountClaimID(
     Json::Value const& claim_id,
-    Json::StaticString const fieldName,
+    Json::StaticString const fieldName)
-    [[maybe_unused]] unsigned const apiVersion)
 {
     if (!claim_id.isObject())
     {
@@ -848,6 +717,10 @@ parseXChainOwnedCreateAccountClaimID(
     return keylet.key;
 }
 
+using FunctionType = Expected<uint256, Json::Value> (*)(
+    Json::Value const&,
+    Json::StaticString const);
+
 struct LedgerEntry
 {
     Json::StaticString fieldName;
@@ -880,7 +753,7 @@ doLedgerEntry(RPC::JsonContext& context)
         {jss::ripple_state, parseRippleState, ltRIPPLE_STATE},
     });
 
-    auto const hasMoreThanOneMember = [&]() {
+    auto hasMoreThanOneMember = [&]() {
         int count = 0;
 
         for (auto const& ledgerEntry : ledgerEntryParsers)
@@ -924,8 +797,8 @@ doLedgerEntry(RPC::JsonContext& context)
                 Json::Value const& params = ledgerEntry.fieldName == jss::bridge
                     ? context.params
                     : context.params[ledgerEntry.fieldName];
-                auto const result = ledgerEntry.parseFunction(
+                auto const result =
-                    params, ledgerEntry.fieldName, context.apiVersion);
+                    ledgerEntry.parseFunction(params, ledgerEntry.fieldName);
                 if (!result)
                     return result.error();
 
@@ -956,13 +829,9 @@ doLedgerEntry(RPC::JsonContext& context)
             throw;
     }
 
-    // Return the computed index regardless of whether the node exists.
-    jvResult[jss::index] = to_string(uNodeIndex);
-
     if (uNodeIndex.isZero())
     {
-        RPC::inject_error(rpcENTRY_NOT_FOUND, jvResult);
+        return RPC::make_error(rpcENTRY_NOT_FOUND);
-        return jvResult;
     }
 
     auto const sleNode = lpLedger->read(keylet::unchecked(uNodeIndex));
@@ -974,14 +843,12 @@ doLedgerEntry(RPC::JsonContext& context)
     if (!sleNode)
     {
         // Not found.
-        RPC::inject_error(rpcENTRY_NOT_FOUND, jvResult);
+        return RPC::make_error(rpcENTRY_NOT_FOUND);
-        return jvResult;
     }
 
     if ((expectedType != ltANY) && (expectedType != sleNode->getType()))
     {
-        RPC::inject_error(rpcUNEXPECTED_LEDGER_TYPE, jvResult);
+        return RPC::make_error(rpcUNEXPECTED_LEDGER_TYPE);
-        return jvResult;
     }
 
     if (bNodeBinary)
@@ -991,10 +858,12 @@ doLedgerEntry(RPC::JsonContext& context)
         sleNode->add(s);
 
         jvResult[jss::node_binary] = strHex(s.peekData());
+        jvResult[jss::index] = to_string(uNodeIndex);
     }
     else
     {
         jvResult[jss::node] = sleNode->getJson(JsonOptions::none);
+        jvResult[jss::index] = to_string(uNodeIndex);
     }
 
     return jvResult;