Merge branch 'develop' into ximinez/fixed-ledger-entries

* upstream/develop:
  Refactor `ledger_entry` RPC source code and tests (5237)

cfg/rippled-example.cfg

@@ -1051,11 +1051,10 @@
 #                           The online delete process checks periodically
 #                           that rippled is still in sync with the network,
 #                           and that the validated ledger is less than
-#                           'age_threshold_seconds' old, and that all
-#                           recent ledgers are available. If not, then continue
+#                           'age_threshold_seconds' old. If not, then continue
 #                           sleeping for this number of seconds and
 #                           checking until healthy.
-#                           Default is 1.
+#                           Default is 5.
 #
 #   Notes:
 #       The 'node_db' entry configures the primary, persistent storage.

src/test/app/LedgerMaster_test.cpp

@@ -2,7 +2,6 @@
 #include <test/jtx/Env.h>
 
 #include <xrpld/app/ledger/LedgerMaster.h>
-#include <xrpld/app/misc/SHAMapStore.h>
 
 namespace ripple {
 namespace test {
@@ -101,89 +100,6 @@ class LedgerMaster_test : public beast::unit_test::suite
         }
     }
 
-    void
-    testCompleteLedgerRange(FeatureBitset features)
-    {
-        // Note that this test is intentionally very similar to
-        // SHAMapStore_test::testLedgerGaps, but has a different
-        // focus.
-
-        testcase("Complete Ledger operations");
-
-        using namespace test::jtx;
-
-        auto const deleteInterval = 8;
-
-        Env env{*this, envconfig([](auto cfg) {
-                    return online_delete(std::move(cfg), deleteInterval);
-                })};
-
-        auto const alice = Account("alice");
-        env.fund(XRP(1000), alice);
-        env.close();
-
-        auto& lm = env.app().getLedgerMaster();
-        LedgerIndex minSeq = 2;
-        LedgerIndex maxSeq = env.closed()->info().seq;
-        auto& store = env.app().getSHAMapStore();
-        store.rendezvous();
-        LedgerIndex lastRotated = store.getLastRotated();
-        BEAST_EXPECTS(maxSeq == 3, to_string(maxSeq));
-        BEAST_EXPECTS(
-            lm.getCompleteLedgers() == "2-3", lm.getCompleteLedgers());
-        BEAST_EXPECTS(lastRotated == 3, to_string(lastRotated));
-        BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0);
-        BEAST_EXPECT(
-            lm.missingFromCompleteLedgerRange(minSeq + 1, maxSeq - 1) == 0);
-        BEAST_EXPECT(
-            lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2);
-        BEAST_EXPECT(
-            lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2);
-        BEAST_EXPECT(
-            lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2);
-
-        // Close enough ledgers to rotate a few times
-        for (int i = 0; i < 24; ++i)
-        {
-            for (int t = 0; t < 3; ++t)
-            {
-                env(noop(alice));
-            }
-            env.close();
-            store.rendezvous();
-
-            ++maxSeq;
-
-            if (maxSeq == lastRotated + deleteInterval)
-            {
-                minSeq = lastRotated;
-                lastRotated = store.getLastRotated();
-                BEAST_EXPECT(lastRotated = maxSeq + 2);
-            }
-            BEAST_EXPECTS(
-                env.closed()->info().seq == maxSeq,
-                to_string(env.closed()->info().seq));
-            BEAST_EXPECTS(
-                store.getLastRotated() == lastRotated,
-                to_string(store.getLastRotated()));
-            std::stringstream expectedRange;
-            expectedRange << minSeq << "-" << maxSeq;
-            BEAST_EXPECTS(
-                lm.getCompleteLedgers() == expectedRange.str(),
-                lm.getCompleteLedgers());
-            BEAST_EXPECT(
-                lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0);
-            BEAST_EXPECT(
-                lm.missingFromCompleteLedgerRange(minSeq + 1, maxSeq - 1) == 0);
-            BEAST_EXPECT(
-                lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2);
-            BEAST_EXPECT(
-                lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2);
-            BEAST_EXPECT(
-                lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2);
-        }
-    }
-
 public:
     void
     run() override
@@ -197,7 +113,6 @@ public:
     testWithFeats(FeatureBitset features)
     {
         testTxnIdFromIndex(features);
-        testCompleteLedgerRange(features);
     }
 };
 

src/test/app/SHAMapStore_test.cpp

@@ -1,7 +1,6 @@
 #include <test/jtx.h>
 #include <test/jtx/envconfig.h>
 
-#include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/app/main/Application.h>
 #include <xrpld/app/main/NodeStoreScheduler.h>
 #include <xrpld/app/misc/SHAMapStore.h>
@@ -11,8 +10,6 @@
 #include <xrpl/nodestore/detail/DatabaseRotatingImp.h>
 #include <xrpl/protocol/jss.h>
 
-#include <thread>
-
 namespace ripple {
 namespace test {
 
@@ -23,8 +20,10 @@ class SHAMapStore_test : public beast::unit_test::suite
     static auto
     onlineDelete(std::unique_ptr<Config> cfg)
     {
-        using namespace jtx;
-        return online_delete(std::move(cfg), deleteInterval);
+        cfg->LEDGER_HISTORY = deleteInterval;
+        auto& section = cfg->section(ConfigSection::nodeDatabase());
+        section.set("online_delete", std::to_string(deleteInterval));
+        return cfg;
     }
 
     static auto
@@ -227,10 +226,9 @@ public:
 
         store.rendezvous();
 
-        BEAST_EXPECT(env.closed()->info().seq == deleteInterval + 3);
+        BEAST_EXPECT(store.getLastRotated() == deleteInterval + 3);
         lastRotated = store.getLastRotated();
-        BEAST_EXPECT(lastRotated == deleteInterval + 5);
-        BEAST_EXPECT(lastRotated == 13);
+        BEAST_EXPECT(lastRotated == 11);
 
         // That took care of the fake hashes
         ledgerCheck(env, deleteInterval + 1, 3);
@@ -238,11 +236,9 @@ public:
         accountTransactionCheck(env, 2 * deleteInterval);
 
         // The last iteration of this loop should trigger a rotate
-        for (auto i = lastRotated - 3; i < lastRotated + deleteInterval - 1;
+        for (auto i = lastRotated - 1; i < lastRotated + deleteInterval - 1;
              ++i)
         {
-            BEAST_EXPECT(store.getLastRotated() == lastRotated);
-
             env.close();
 
             ledgerTmp = env.rpc("ledger", "current");
@@ -260,8 +256,7 @@ public:
 
         store.rendezvous();
 
-        BEAST_EXPECT(
-            store.getLastRotated() == deleteInterval + lastRotated + 2);
+        BEAST_EXPECT(store.getLastRotated() == deleteInterval + lastRotated);
 
         ledgerCheck(env, deleteInterval + 1, lastRotated);
         transactionCheck(env, 0);
@@ -402,8 +397,8 @@ public:
 
         ledgerCheck(env, ledgerSeq - lastRotated, lastRotated);
 
-        lastRotated = store.getLastRotated();
-        BEAST_EXPECT(lastRotated == ledgerSeq + 1);
+        BEAST_EXPECT(store.getLastRotated() == ledgerSeq - 1);
+        lastRotated = ledgerSeq - 1;
 
         for (; ledgerSeq < lastRotated + deleteInterval; ++ledgerSeq)
         {
@@ -430,10 +425,10 @@ public:
 
         store.rendezvous();
 
-        ledgerCheck(env, ledgerSeq - firstBatch - 2, firstBatch + 2);
+        ledgerCheck(env, ledgerSeq - firstBatch, firstBatch);
 
-        lastRotated = store.getLastRotated();
-        BEAST_EXPECT(lastRotated == ledgerSeq + 1);
+        BEAST_EXPECT(store.getLastRotated() == ledgerSeq - 1);
+        lastRotated = ledgerSeq - 1;
 
         // This does not kick off a cleanup
         canDelete = env.rpc("can_delete", "always");
@@ -469,13 +464,13 @@ public:
 
         ledgerCheck(env, ledgerSeq - lastRotated, lastRotated);
 
-        lastRotated = store.getLastRotated();
-        BEAST_EXPECT(lastRotated == ledgerSeq + 1);
+        BEAST_EXPECT(store.getLastRotated() == ledgerSeq - 1);
+        lastRotated = ledgerSeq - 1;
 
         // This does not kick off a cleanup
         canDelete = env.rpc("can_delete", "now");
         BEAST_EXPECT(!RPC::contains_error(canDelete[jss::result]));
-        BEAST_EXPECT(canDelete[jss::result][jss::can_delete] == lastRotated);
+        BEAST_EXPECT(canDelete[jss::result][jss::can_delete] == ledgerSeq - 1);
 
         for (; ledgerSeq < lastRotated + deleteInterval; ++ledgerSeq)
         {
@@ -504,8 +499,8 @@ public:
 
         ledgerCheck(env, ledgerSeq - lastRotated, lastRotated);
 
-        lastRotated = store.getLastRotated();
-        BEAST_EXPECT(lastRotated == ledgerSeq + 1);
+        BEAST_EXPECT(store.getLastRotated() == ledgerSeq - 1);
+        lastRotated = ledgerSeq - 1;
     }
 
     std::unique_ptr<NodeStore::Backend>
@@ -631,212 +626,6 @@ public:
         BEAST_EXPECT(dbr->getName() == "3");
     }
 
-    void
-    testLedgerGaps()
-    {
-        // Note that this test is intentionally very similar to
-        // LedgerMaster_test::testCompleteLedgerRange, but has a different
-        // focus.
-
-        testcase("Wait for ledger gaps to fill in");
-
-        using namespace test::jtx;
-
-        Env env{*this, envconfig(onlineDelete)};
-
-        auto& ns = env.app().getNodeStore();
-        std::map<LedgerIndex, uint256> hashes;
-        auto storeHash = [&]() {
-            hashes.emplace(
-                env.current()->info().seq, env.current()->info().hash);
-
-            auto const& root =
-                ns.fetchNodeObject(hashes[env.current()->info().seq]);
-            BEAST_EXPECT(root);
-        };
-
-        auto failureMessage = [&](char const* label,
-                                  auto expected,
-                                  auto actual) {
-            std::stringstream ss;
-            ss << label << ": Expected: " << expected << ", Got: " << actual;
-            return ss.str();
-        };
-
-        auto const alice = Account("alice");
-        env.fund(XRP(1000), alice);
-        env.close();
-        storeHash();
-
-        auto& lm = env.app().getLedgerMaster();
-        LedgerIndex minSeq = 2;
-        LedgerIndex maxSeq = env.closed()->info().seq;
-        auto& store = env.app().getSHAMapStore();
-        store.rendezvous();
-        LedgerIndex lastRotated = store.getLastRotated();
-        BEAST_EXPECTS(maxSeq == 3, to_string(maxSeq));
-        BEAST_EXPECTS(
-            lm.getCompleteLedgers() == "2-3", lm.getCompleteLedgers());
-        BEAST_EXPECTS(lastRotated == 3, to_string(lastRotated));
-        BEAST_EXPECT(lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0);
-        BEAST_EXPECT(
-            lm.missingFromCompleteLedgerRange(minSeq + 1, maxSeq - 1) == 0);
-        BEAST_EXPECT(
-            lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2);
-        BEAST_EXPECT(
-            lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2);
-        BEAST_EXPECT(
-            lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2);
-
-        // Close enough ledgers to rotate a few times
-        while (maxSeq < 28)
-        {
-            for (int t = 0; t < 3; ++t)
-            {
-                env(noop(alice));
-            }
-            env.close();
-            storeHash();
-            store.rendezvous();
-
-            ++maxSeq;
-
-            if (maxSeq + 1 == lastRotated + deleteInterval)
-            {
-                using namespace std::chrono_literals;
-
-                // The next ledger will trigger a rotation. Delete the
-                // current ledger from LedgerMaster.
-                std::this_thread::sleep_for(100ms);
-                LedgerIndex const deleteSeq = maxSeq;
-                while (!lm.haveLedger(deleteSeq))
-                {
-                    std::this_thread::sleep_for(100ms);
-                }
-                lm.clearLedger(deleteSeq);
-
-                auto expectedRange =
-                    [](auto minSeq, auto deleteSeq, auto maxSeq) {
-                        std::stringstream expectedRange;
-                        expectedRange << minSeq << "-" << (deleteSeq - 1);
-                        if (deleteSeq + 1 == maxSeq)
-                            expectedRange << "," << maxSeq;
-                        else if (deleteSeq < maxSeq)
-                            expectedRange << "," << (deleteSeq + 1) << "-"
-                                          << maxSeq;
-                        return expectedRange.str();
-                    };
-                BEAST_EXPECTS(
-                    lm.getCompleteLedgers() ==
-                        expectedRange(minSeq, deleteSeq, maxSeq),
-                    failureMessage(
-                        "Complete ledgers",
-                        expectedRange(minSeq, deleteSeq, maxSeq),
-                        lm.getCompleteLedgers()));
-                BEAST_EXPECT(
-                    lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 1);
-
-                // Close another ledger, which will trigger a rotation, but the
-                // rotation will be stuck until the missing ledger is filled in.
-                env.close();
-                storeHash();
-                // DO NOT CALL rendezvous()! You'll end up with a deadlock.
-                ++maxSeq;
-
-                // Nothing has changed
-                BEAST_EXPECTS(
-                    store.getLastRotated() == lastRotated,
-                    failureMessage(
-                        "lastRotated", lastRotated, store.getLastRotated()));
-                BEAST_EXPECTS(
-                    lm.getCompleteLedgers() ==
-                        expectedRange(minSeq, deleteSeq, maxSeq),
-                    failureMessage(
-                        "Complete ledgers",
-                        expectedRange(minSeq, deleteSeq, maxSeq),
-                        lm.getCompleteLedgers()));
-
-                // Close 5 more ledgers, waiting one second in between to
-                // simulate the ledger making progress while online delete waits
-                // for the missing ledger to be filled in.
-                // This ensures the healthWait check has time to run and
-                // detect the gap.
-                for (int l = 0; l < 5; ++l)
-                {
-                    env.close();
-                    storeHash();
-                    // DO NOT CALL rendezvous()! You'll end up with a deadlock.
-                    ++maxSeq;
-                    // Nothing has changed
-                    BEAST_EXPECTS(
-                        store.getLastRotated() == lastRotated,
-                        failureMessage(
-                            "lastRotated",
-                            lastRotated,
-                            store.getLastRotated()));
-                    BEAST_EXPECTS(
-                        lm.getCompleteLedgers() ==
-                            expectedRange(minSeq, deleteSeq, maxSeq),
-                        failureMessage(
-                            "Complete Ledgers",
-                            expectedRange(minSeq, deleteSeq, maxSeq),
-                            lm.getCompleteLedgers()));
-                    std::this_thread::sleep_for(1s);
-                }
-
-                // Put the missing ledger back in LedgerMaster
-                lm.setLedgerRangePresent(deleteSeq, deleteSeq);
-
-                // Wait for the rotation to finish
-                store.rendezvous();
-
-                minSeq = lastRotated;
-                lastRotated = maxSeq + 2;
-
-                // Bypass caching and try to load the ledger roots from the node
-                // store
-                for (auto const& [seq, hash] : hashes)
-                {
-                    auto const nodeObject = ns.fetchNodeObject(hash);
-                    std::stringstream ss;
-                    ss << "minSeq: " << minSeq << ", maxSeq: " << maxSeq
-                       << ", search: " << seq << ". Should "
-                       << (seq < minSeq ? "NOT " : "") << "be found";
-                    if (seq < minSeq)
-                        BEAST_EXPECTS(!nodeObject, ss.str());
-                    else
-                        BEAST_EXPECTS(nodeObject, ss.str());
-                }
-            }
-            BEAST_EXPECT(maxSeq != lastRotated + deleteInterval);
-            BEAST_EXPECTS(
-                env.closed()->info().seq == maxSeq,
-                failureMessage("maxSeq", maxSeq, env.closed()->info().seq));
-            BEAST_EXPECTS(
-                store.getLastRotated() == lastRotated,
-                failureMessage(
-                    "lastRotated", lastRotated, store.getLastRotated()));
-            std::stringstream expectedRange;
-            expectedRange << minSeq << "-" << maxSeq;
-            BEAST_EXPECTS(
-                lm.getCompleteLedgers() == expectedRange.str(),
-                failureMessage(
-                    "CompleteLedgers",
-                    expectedRange.str(),
-                    lm.getCompleteLedgers()));
-            BEAST_EXPECT(
-                lm.missingFromCompleteLedgerRange(minSeq, maxSeq) == 0);
-            BEAST_EXPECT(
-                lm.missingFromCompleteLedgerRange(minSeq + 1, maxSeq - 1) == 0);
-            BEAST_EXPECT(
-                lm.missingFromCompleteLedgerRange(minSeq - 1, maxSeq + 1) == 2);
-            BEAST_EXPECT(
-                lm.missingFromCompleteLedgerRange(minSeq - 2, maxSeq - 2) == 2);
-            BEAST_EXPECT(
-                lm.missingFromCompleteLedgerRange(minSeq + 2, maxSeq + 2) == 2);
-        }
-    }
-
     void
     run() override
     {
@@ -844,7 +633,6 @@ public:
         testAutomatic();
         testCanDelete();
         testRotate();
-        testLedgerGaps();
     }
 };
 

src/test/jtx/envconfig.h

@@ -58,17 +58,6 @@ envconfig(F&& modfunc, Args&&... args)
     return modfunc(envconfig(), std::forward<Args>(args)...);
 }
 
-/// @brief adjust config to enable online_delete
-///
-/// @param cfg config instance to be modified
-///
-/// @param deleteInterval how many new ledgers should be available before
-/// rotating. Defaults to 8, because the standalone minimum is 8.
-///
-/// @return unique_ptr to Config instance
-std::unique_ptr<Config>
-online_delete(std::unique_ptr<Config> cfg, std::uint32_t deleteInterval = 8);
-
 /// @brief adjust config so no admin ports are enabled
 ///
 /// this is intended for use with envconfig, as in

src/test/jtx/impl/envconfig.cpp

@@ -53,15 +53,6 @@ setupConfigForUnitTests(Config& cfg)
 
 namespace jtx {
 
-std::unique_ptr<Config>
-online_delete(std::unique_ptr<Config> cfg, std::uint32_t deleteInterval)
-{
-    cfg->LEDGER_HISTORY = deleteInterval;
-    auto& section = cfg->section(ConfigSection::nodeDatabase());
-    section.set("online_delete", std::to_string(deleteInterval));
-    return cfg;
-}
-
 std::unique_ptr<Config>
 no_admin(std::unique_ptr<Config> cfg)
 {

src/test/rpc/LedgerEntry_test.cpp

@@ -5,6 +5,8 @@
 #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>
@@ -2008,6 +2010,370 @@ class LedgerEntry_test : public beast::unit_test::suite
         }
     }
 
+    /// Test the ledger entry types that don't take parameters
+    void
+    testLedgerEntryFixed()
+    {
+        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 << "ledger_entry " << 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 jv = env.rpc("json", "ledger_entry", to_string(params));
+            checkResult(false, jv, expectedType, "malformedRequest");
+            BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
+
+            // "field":"string"
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[field] = "arbitrary string";
+            jv = env.rpc("json", "ledger_entry", to_string(params));
+            checkResult(false, jv, expectedType, "malformedRequest");
+            BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
+
+            // "field":false
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[field] = false;
+            jv = env.rpc("json", "ledger_entry", to_string(params));
+            checkResult(false, jv, expectedType, "invalidParams");
+            BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
+
+            {
+                // "field":[incorrect index hash]
+                auto const badKey = strHex(expectedKey.key + uint256{1});
+                params.clear();
+                params[jss::ledger_index] = jss::validated;
+                params[field] = badKey;
+                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));
+            }
+
+            // "index":"field" using API 2
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[jss::index] = field;
+            params[jss::api_version] = 2;
+            jv = env.rpc("json", "ledger_entry", to_string(params));
+            checkResult(false, jv, expectedType, "malformedRequest");
+            BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
+
+            // Test good values
+            // Use the "field":true notation
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[field] = true;
+            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);
+
+            // "field":"[index hash]"
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[field] = hexKey;
+            jv = env.rpc("json", "ledger_entry", to_string(params));
+            checkResult(good, jv, expectedType);
+            BEAST_EXPECT(jv[jss::result][jss::index].asString() == hexKey);
+
+            // Use the "index":"field" notation with API 3
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[jss::index] = field;
+            params[jss::api_version] = 3;
+            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);
+
+            // Use the "index":"[index hash]" notation
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[jss::index] = pdIdx;
+            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
+    testLedgerEntryHashes()
+    {
+        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 << "ledger_entry LedgerHashes: seq: "
+                     << env.current()->info().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));
+
+            // "hashes":"non-uint string"
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[jss::hashes] = "arbitrary string";
+            jv = env.rpc("json", "ledger_entry", to_string(params));
+            checkResult(false, jv, 0, "malformedRequest");
+            BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
+
+            // "hashes":"uint string" is invalid, too
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[jss::hashes] = "10";
+            jv = env.rpc("json", "ledger_entry", to_string(params));
+            checkResult(false, jv, 0, "malformedRequest");
+            BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
+
+            // "hashes":false
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[jss::hashes] = false;
+            jv = env.rpc("json", "ledger_entry", to_string(params));
+            checkResult(false, jv, 0, "invalidParams");
+            BEAST_EXPECT(!jv[jss::result].isMember(jss::index));
+
+            // "hashes":-1
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[jss::hashes] = -1;
+            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]
+            {
+                auto const badKey = strHex(expectedKey.key + uint256{1});
+                params.clear();
+                params[jss::ledger_index] = jss::validated;
+                params[jss::hashes] = badKey;
+                jv = env.rpc("json", "ledger_entry", to_string(params));
+                checkResult(false, jv, 0, "entryNotFound");
+                BEAST_EXPECT(jv[jss::result][jss::index] == badKey);
+            }
+
+            // Test good values
+            // Use the "hashes":ledger notation
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[jss::hashes] = ledger;
+            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));
+
+            // "hashes":"[index hash]"
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[jss::hashes] = hexKey;
+            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()));
+
+            // Use the "index":"[index hash]" notation
+            params.clear();
+            params[jss::ledger_index] = jss::validated;
+            params[jss::index] = hexKey;
+            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
     testLedgerEntryCLI()
     {
@@ -2057,6 +2423,8 @@ public:
         testOracleLedgerEntry();
         testLedgerEntryMPT();
         testLedgerEntryPermissionedDomain();
+        testLedgerEntryFixed();
+        testLedgerEntryHashes();
         testLedgerEntryCLI();
     }
 };

src/xrpld/app/ledger/LedgerMaster.h

@@ -108,10 +108,7 @@ public:
     failedSave(std::uint32_t seq, uint256 const& hash);
 
     std::string
-    getCompleteLedgers() const;
-
-    std::size_t
-    missingFromCompleteLedgerRange(LedgerIndex first, LedgerIndex last) const;
+    getCompleteLedgers();
 
     /** Apply held transactions to the open ledger
         This is normally called as we close the ledger.
@@ -328,7 +325,7 @@ private:
     // A set of transactions to replay during the next close
     std::unique_ptr<LedgerReplay> replayData;
 
-    std::recursive_mutex mutable mCompleteLock;
+    std::recursive_mutex mCompleteLock;
     RangeSet<std::uint32_t> mCompleteLedgers;
 
     // Publish thread is running.

src/xrpld/app/ledger/detail/LedgerMaster.cpp

@@ -1571,36 +1571,12 @@ LedgerMaster::getPublishedLedger()
 }
 
 std::string
-LedgerMaster::getCompleteLedgers() const
+LedgerMaster::getCompleteLedgers()
 {
     std::lock_guard sl(mCompleteLock);
     return to_string(mCompleteLedgers);
 }
 
-std::size_t
-LedgerMaster::missingFromCompleteLedgerRange(
-    LedgerIndex first,
-    LedgerIndex last) const
-{
-    // Make a copy of the range to avoid holding the lock
-    auto const range = [&] {
-        std::lock_guard sl(mCompleteLock);
-        return mCompleteLedgers;
-    }();
-
-    std::size_t missing = 0;
-
-    for (LedgerIndex idx = first; idx <= last; ++idx)
-    {
-        if (!boost::icl::contains(range, idx))
-        {
-            ++missing;
-        }
-    }
-
-    return missing;
-}
-
 std::optional<NetClock::time_point>
 LedgerMaster::getCloseTimeBySeq(LedgerIndex ledgerIndex)
 {

src/xrpld/app/misc/SHAMapStoreImp.cpp

@@ -289,18 +289,6 @@ SHAMapStoreImp::run()
             validatedSeq >= lastRotated + deleteInterval_ &&
             canDelete_ >= lastRotated - 1 && healthWait() == keepGoing;
 
-        JLOG(journal_.debug())
-            << "run: Setting lastGoodValidatedLedger_ to " << validatedSeq;
-
-        {
-            // Note that this is set after the healthWait() check, so that we
-            // don't start the rotation until the validated ledger is fully
-            // processed. It is not guaranteed to be done at this point. It also
-            // allows the testLedgerGaps unit test to work.
-            std::unique_lock<std::mutex> lock(mutex_);
-            lastGoodValidatedLedger_ = validatedSeq;
-        }
-
         // will delete up to (not including) lastRotated
         if (readyToRotate)
         {
@@ -309,9 +297,7 @@ SHAMapStoreImp::run()
                 << lastRotated << " deleteInterval " << deleteInterval_
                 << " canDelete_ " << canDelete_ << " state "
                 << app_.getOPs().strOperatingMode(false) << " age "
-                << ledgerMaster_->getValidatedLedgerAge().count()
-                << "s. Complete ledgers: "
-                << ledgerMaster_->getCompleteLedgers();
+                << ledgerMaster_->getValidatedLedgerAge().count() << 's';
 
             clearPrior(lastRotated);
             if (healthWait() == stopping)
@@ -359,13 +345,7 @@ SHAMapStoreImp::run()
             if (healthWait() == stopping)
                 return;
 
-            // The rotation process takes time, and more ledgers - possibly many
-            // more - may have been written to the now-archive database before
-            // the writable database was created. Update the validatedSeq to the
-            // current validated ledger sequence plus a buffer to ensure that
-            // all modified nodes for that ledger forward are all written to the
-            // new writable database.
-            lastRotated = ledgerMaster_->getValidLedgerIndex() + 2;
+            lastRotated = validatedSeq;
 
             dbRotating_->rotate(
                 std::move(newBackend),
@@ -380,10 +360,7 @@ SHAMapStoreImp::run()
                     clearCaches(validatedSeq);
                 });
 
-            JLOG(journal_.warn())
-                << "finished rotation. validatedSeq: " << validatedSeq
-                << ", lastRotated: " << lastRotated << ". Complete ledgers: "
-                << ledgerMaster_->getCompleteLedgers();
+            JLOG(journal_.warn()) << "finished rotation " << validatedSeq;
         }
     }
 }
@@ -638,47 +615,22 @@ SHAMapStoreImp::clearPrior(LedgerIndex lastRotated)
 SHAMapStoreImp::HealthResult
 SHAMapStoreImp::healthWait()
 {
-    auto index = ledgerMaster_->getValidLedgerIndex();
     auto age = ledgerMaster_->getValidatedLedgerAge();
     OperatingMode mode = netOPs_->getOperatingMode();
     std::unique_lock lock(mutex_);
-
-    auto numMissing = ledgerMaster_->missingFromCompleteLedgerRange(
-        lastGoodValidatedLedger_, index);
-    while (
-        !stop_ &&
-        (mode != OperatingMode::FULL || age > ageThreshold_ || numMissing > 0))
+    while (!stop_ && (mode != OperatingMode::FULL || age > ageThreshold_))
     {
-        // this value shouldn't change, so grab it while we have the
-        // lock
-        auto const lastGood = lastGoodValidatedLedger_;
-
         lock.unlock();
-
-        auto const stream = mode != OperatingMode::FULL || age > ageThreshold_
-            ? journal_.warn()
-            : journal_.info();
-        JLOG(stream) << "Waiting " << recoveryWaitTime_.count()
-                     << "s for node to stabilize. state: "
-                     << app_.getOPs().strOperatingMode(mode, false) << ". age "
-                     << age.count() << "s. Missing ledgers: " << numMissing
-                     << ". Expect: " << lastGood << "-" << index
-                     << ". Complete ledgers: "
-                     << ledgerMaster_->getCompleteLedgers();
+        JLOG(journal_.warn()) << "Waiting " << recoveryWaitTime_.count()
+                              << "s for node to stabilize. state: "
+                              << app_.getOPs().strOperatingMode(mode, false)
+                              << ". age " << age.count() << 's';
         std::this_thread::sleep_for(recoveryWaitTime_);
-        index = ledgerMaster_->getValidLedgerIndex();
         age = ledgerMaster_->getValidatedLedgerAge();
         mode = netOPs_->getOperatingMode();
-        numMissing =
-            ledgerMaster_->missingFromCompleteLedgerRange(lastGood, index);
-
         lock.lock();
     }
 
-    JLOG(journal_.debug()) << "healthWait: Setting lastGoodValidatedLedger_ to "
-                           << index;
-    lastGoodValidatedLedger_ = index;
-
     return stop_ ? stopping : keepGoing;
 }
 

src/xrpld/app/misc/SHAMapStoreImp.h

@@ -71,11 +71,6 @@ private:
     std::thread thread_;
     bool stop_ = false;
     bool healthy_ = true;
-    // Used to prevent ledger gaps from forming during online deletion. Keeps
-    // track of the last validated ledger that was processed without gaps. There
-    // are no guarantees about gaps while online delete is not running. For
-    // that, use advisory_delete and check for gaps externally.
-    LedgerIndex lastGoodValidatedLedger_ = 0;
     mutable std::condition_variable cond_;
     mutable std::condition_variable rendezvous_;
     mutable std::mutex mutex_;
@@ -89,11 +84,11 @@ private:
     std::uint32_t deleteBatch_ = 100;
     std::chrono::milliseconds backOff_{100};
     std::chrono::seconds ageThreshold_{60};
-    /// If  the node is out of sync, or any recent ledgers are not
-    /// available during an online_delete healthWait() call, sleep
-    /// the thread for this time, and continue checking until recovery.
+    /// If  the node is out of sync during an online_delete healthWait()
+    /// call, sleep the thread for this time, and continue checking until
+    /// recovery.
     /// See also: "recovery_wait_seconds" in rippled-example.cfg
-    std::chrono::seconds recoveryWaitTime_{1};
+    std::chrono::seconds recoveryWaitTime_{5};
 
     // these do not exist upon SHAMapStore creation, but do exist
     // as of run() or before
@@ -217,8 +212,6 @@ private:
     enum HealthResult { stopping, keepGoing };
     [[nodiscard]] HealthResult
     healthWait();
-    bool
-    hasCompleteRange(LedgerIndex first, LedgerIndex last);
 
 public:
     void

src/xrpld/rpc/handlers/LedgerEntry.cpp

@@ -20,6 +20,32 @@
 
 namespace ripple {
 
+using FunctionType = std::function<Expected<uint256, Json::Value>(
+    Json::Value const&,
+    Json::StaticString const,
+    unsigned apiVersion)>;
+
+static Expected<uint256, Json::Value>
+parseFixed(
+    Keylet const& keylet,
+    Json::Value const& params,
+    Json::StaticString const& fieldName,
+    unsigned 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 apiVersion) -> Expected<uint256, Json::Value> {
+        return parseFixed(keylet, params, fieldName, apiVersion);
+    };
+}
+
 static Expected<uint256, Json::Value>
 parseObjectID(
     Json::Value const& params,
@@ -35,13 +61,33 @@ parseObjectID(
 }
 
 static Expected<uint256, Json::Value>
-parseIndex(Json::Value const& params, Json::StaticString const fieldName)
+parseIndex(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned 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(Json::Value const& params, Json::StaticString const fieldName)
+parseAccountRoot(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (auto const account = LedgerEntryHelpers::parse<AccountID>(params))
     {
@@ -52,14 +98,13 @@ parseAccountRoot(Json::Value const& params, Json::StaticString const fieldName)
         "malformedAddress", fieldName, "AccountID");
 }
 
-static Expected<uint256, Json::Value>
-parseAmendments(Json::Value const& params, Json::StaticString const fieldName)
-{
-    return parseObjectID(params, fieldName, "hex string");
-}
+auto const parseAmendments = fixed(keylet::amendments());
 
 static Expected<uint256, Json::Value>
-parseAMM(Json::Value const& params, Json::StaticString const fieldName)
+parseAMM(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!params.isObject())
     {
@@ -86,7 +131,10 @@ parseAMM(Json::Value const& params, Json::StaticString const fieldName)
 }
 
 static Expected<uint256, Json::Value>
-parseBridge(Json::Value const& params, Json::StaticString const fieldName)
+parseBridge(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!params.isMember(jss::bridge))
     {
@@ -117,13 +165,19 @@ parseBridge(Json::Value const& params, Json::StaticString const fieldName)
 }
 
 static Expected<uint256, Json::Value>
-parseCheck(Json::Value const& params, Json::StaticString const fieldName)
+parseCheck(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
 
 static Expected<uint256, Json::Value>
-parseCredential(Json::Value const& cred, Json::StaticString const fieldName)
+parseCredential(
+    Json::Value const& cred,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!cred.isObject())
     {
@@ -154,7 +208,10 @@ parseCredential(Json::Value const& cred, Json::StaticString const fieldName)
 }
 
 static Expected<uint256, Json::Value>
-parseDelegate(Json::Value const& params, Json::StaticString const fieldName)
+parseDelegate(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!params.isObject())
     {
@@ -222,7 +279,10 @@ parseAuthorizeCredentials(Json::Value const& jv)
 }
 
 static Expected<uint256, Json::Value>
-parseDepositPreauth(Json::Value const& dp, Json::StaticString const fieldName)
+parseDepositPreauth(
+    Json::Value const& dp,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!dp.isObject())
     {
@@ -282,7 +342,10 @@ parseDepositPreauth(Json::Value const& dp, Json::StaticString const fieldName)
 }
 
 static Expected<uint256, Json::Value>
-parseDID(Json::Value const& params, Json::StaticString const fieldName)
+parseDID(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     auto const account = LedgerEntryHelpers::parse<AccountID>(params);
     if (!account)
@@ -297,7 +360,8 @@ parseDID(Json::Value const& params, Json::StaticString const fieldName)
 static Expected<uint256, Json::Value>
 parseDirectoryNode(
     Json::Value const& params,
-    Json::StaticString const fieldName)
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!params.isObject())
     {
@@ -350,7 +414,10 @@ parseDirectoryNode(
 }
 
 static Expected<uint256, Json::Value>
-parseEscrow(Json::Value const& params, Json::StaticString const fieldName)
+parseEscrow(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!params.isObject())
     {
@@ -369,20 +436,53 @@ parseEscrow(Json::Value const& params, Json::StaticString const fieldName)
     return keylet::escrow(*id, *seq).key;
 }
 
+auto const parseFeeSettings = fixed(keylet::fees());
+
 static Expected<uint256, Json::Value>
-parseFeeSettings(Json::Value const& params, Json::StaticString const fieldName)
+parseFixed(
+    Keylet const& keylet,
+    Json::Value const& params,
+    Json::StaticString const& fieldName,
+    unsigned apiVersion)
 {
-    return parseObjectID(params, fieldName, "hex string");
+    if (!params.isBool())
+    {
+        return parseObjectID(params, fieldName, "hex string");
+    }
+    if (!params.asBool())
+    {
+        return LedgerEntryHelpers::invalidFieldError(
+            "invalidParams", fieldName, "true");
+    }
+
+    return keylet.key;
 }
 
 static Expected<uint256, Json::Value>
-parseLedgerHashes(Json::Value const& params, Json::StaticString const fieldName)
+parseLedgerHashes(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
-    return parseObjectID(params, fieldName, "hex string");
+    if (params.isUInt() || params.isInt())
+    {
+        // 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>
-parseMPToken(Json::Value const& params, Json::StaticString const fieldName)
+parseMPToken(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!params.isObject())
     {
@@ -405,7 +505,8 @@ parseMPToken(Json::Value const& params, Json::StaticString const fieldName)
 static Expected<uint256, Json::Value>
 parseMPTokenIssuance(
     Json::Value const& params,
-    Json::StaticString const fieldName)
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     auto const mptIssuanceID = LedgerEntryHelpers::parse<uint192>(params);
     if (!mptIssuanceID)
@@ -416,25 +517,30 @@ parseMPTokenIssuance(
 }
 
 static Expected<uint256, Json::Value>
-parseNFTokenOffer(Json::Value const& params, Json::StaticString const fieldName)
+parseNFTokenOffer(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
 
 static Expected<uint256, Json::Value>
-parseNFTokenPage(Json::Value const& params, Json::StaticString const fieldName)
+parseNFTokenPage(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
 
-static Expected<uint256, Json::Value>
-parseNegativeUNL(Json::Value const& params, Json::StaticString const fieldName)
-{
-    return parseObjectID(params, fieldName, "hex string");
-}
+auto const parseNegativeUNL = fixed(keylet::negativeUNL());
 
 static Expected<uint256, Json::Value>
-parseOffer(Json::Value const& params, Json::StaticString const fieldName)
+parseOffer(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!params.isObject())
     {
@@ -455,7 +561,10 @@ parseOffer(Json::Value const& params, Json::StaticString const fieldName)
 }
 
 static Expected<uint256, Json::Value>
-parseOracle(Json::Value const& params, Json::StaticString const fieldName)
+parseOracle(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!params.isObject())
     {
@@ -476,7 +585,10 @@ parseOracle(Json::Value const& params, Json::StaticString const fieldName)
 }
 
 static Expected<uint256, Json::Value>
-parsePayChannel(Json::Value const& params, Json::StaticString const fieldName)
+parsePayChannel(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
@@ -484,7 +596,8 @@ parsePayChannel(Json::Value const& params, Json::StaticString const fieldName)
 static Expected<uint256, Json::Value>
 parsePermissionedDomain(
     Json::Value const& pd,
-    Json::StaticString const fieldName)
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (pd.isString())
     {
@@ -513,7 +626,8 @@ parsePermissionedDomain(
 static Expected<uint256, Json::Value>
 parseRippleState(
     Json::Value const& jvRippleState,
-    Json::StaticString const fieldName)
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     Currency uCurrency;
 
@@ -563,13 +677,19 @@ parseRippleState(
 }
 
 static Expected<uint256, Json::Value>
-parseSignerList(Json::Value const& params, Json::StaticString const fieldName)
+parseSignerList(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     return parseObjectID(params, fieldName, "hex string");
 }
 
 static Expected<uint256, Json::Value>
-parseTicket(Json::Value const& params, Json::StaticString const fieldName)
+parseTicket(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!params.isObject())
     {
@@ -590,7 +710,10 @@ parseTicket(Json::Value const& params, Json::StaticString const fieldName)
 }
 
 static Expected<uint256, Json::Value>
-parseVault(Json::Value const& params, Json::StaticString const fieldName)
+parseVault(
+    Json::Value const& params,
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!params.isObject())
     {
@@ -613,7 +736,8 @@ parseVault(Json::Value const& params, Json::StaticString const fieldName)
 static Expected<uint256, Json::Value>
 parseXChainOwnedClaimID(
     Json::Value const& claim_id,
-    Json::StaticString const fieldName)
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!claim_id.isObject())
     {
@@ -638,7 +762,8 @@ parseXChainOwnedClaimID(
 static Expected<uint256, Json::Value>
 parseXChainOwnedCreateAccountClaimID(
     Json::Value const& claim_id,
-    Json::StaticString const fieldName)
+    Json::StaticString const fieldName,
+    unsigned apiVersion)
 {
     if (!claim_id.isObject())
     {
@@ -662,10 +787,6 @@ parseXChainOwnedCreateAccountClaimID(
     return keylet.key;
 }
 
-using FunctionType = Expected<uint256, Json::Value> (*)(
-    Json::Value const&,
-    Json::StaticString const);
-
 struct LedgerEntry
 {
     Json::StaticString fieldName;
@@ -698,7 +819,7 @@ doLedgerEntry(RPC::JsonContext& context)
         {jss::ripple_state, parseRippleState, ltRIPPLE_STATE},
     });
 
-    auto hasMoreThanOneMember = [&]() {
+    auto const hasMoreThanOneMember = [&]() {
         int count = 0;
 
         for (auto const& ledgerEntry : ledgerEntryParsers)
@@ -742,8 +863,8 @@ doLedgerEntry(RPC::JsonContext& context)
                 Json::Value const& params = ledgerEntry.fieldName == jss::bridge
                     ? context.params
                     : context.params[ledgerEntry.fieldName];
-                auto const result =
-                    ledgerEntry.parseFunction(params, ledgerEntry.fieldName);
+                auto const result = ledgerEntry.parseFunction(
+                    params, ledgerEntry.fieldName, context.apiVersion);
                 if (!result)
                     return result.error();
 
@@ -774,9 +895,13 @@ doLedgerEntry(RPC::JsonContext& context)
             throw;
     }
 
+    // Return the computed index regardless of whether the node exists.
+    jvResult[jss::index] = to_string(uNodeIndex);
+
     if (uNodeIndex.isZero())
     {
-        return RPC::make_error(rpcENTRY_NOT_FOUND);
+        RPC::inject_error(rpcENTRY_NOT_FOUND, jvResult);
+        return jvResult;
     }
 
     auto const sleNode = lpLedger->read(keylet::unchecked(uNodeIndex));
@@ -788,12 +913,14 @@ doLedgerEntry(RPC::JsonContext& context)
     if (!sleNode)
     {
         // Not found.
-        return RPC::make_error(rpcENTRY_NOT_FOUND);
+        RPC::inject_error(rpcENTRY_NOT_FOUND, jvResult);
+        return jvResult;
     }
 
     if ((expectedType != ltANY) && (expectedType != sleNode->getType()))
     {
-        return RPC::make_error(rpcUNEXPECTED_LEDGER_TYPE);
+        RPC::inject_error(rpcUNEXPECTED_LEDGER_TYPE, jvResult);
+        return jvResult;
     }
 
     if (bNodeBinary)
@@ -803,12 +930,10 @@ 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;