Merge branch 'develop' into ximinez/online-delete-gaps

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

cfg/xrpld-example.cfg

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

include/xrpl/net/AutoSocket.h

@@ -134,7 +134,7 @@ public:
             {
                 lowest_layer().shutdown(plain_socket::shutdown_both);
             }
-            catch (boost::system::system_error const& e)
+            catch (boost::system::system_error& e)
             {
                 ec = e.code();
             }

include/xrpl/tx/paths/detail/StrandFlow.h

@@ -246,7 +246,7 @@ flow(
             EitherAmount stepIn(*strand[0]->cachedIn());
             for (auto i = 0; i < s; ++i)
             {
-                bool valid = false;
+                bool valid;
                 std::tie(valid, stepIn) = strand[i]->validFwd(checkSB, checkAfView, stepIn);
                 if (!valid)
                 {

src/libxrpl/ledger/ApplyStateTable.cpp

@@ -5,8 +5,6 @@
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/st.h>
 
-#include <stdexcept>
-
 namespace xrpl {
 namespace detail {
 
@@ -376,14 +374,14 @@ ApplyStateTable::erase(ReadView const& base, std::shared_ptr<SLE> const& sle)
 {
     auto const iter = items_.find(sle->key());
     if (iter == items_.end())
-        Throw<std::logic_error>("ApplyStateTable::erase: missing key");
+        LogicError("ApplyStateTable::erase: missing key");
     auto& item = iter->second;
     if (item.second != sle)
-        Throw<std::logic_error>("ApplyStateTable::erase: unknown SLE");
+        LogicError("ApplyStateTable::erase: unknown SLE");
     switch (item.first)
     {
         case Action::erase:
-            Throw<std::logic_error>("ApplyStateTable::erase: double erase");
+            LogicError("ApplyStateTable::erase: double erase");
             break;
         case Action::insert:
             items_.erase(iter);
@@ -407,7 +405,7 @@ ApplyStateTable::rawErase(ReadView const& base, std::shared_ptr<SLE> const& sle)
     switch (item.first)
     {
         case Action::erase:
-            Throw<std::logic_error>("ApplyStateTable::rawErase: double erase");
+            LogicError("ApplyStateTable::rawErase: double erase");
             break;
         case Action::insert:
             items_.erase(result.first);
@@ -438,11 +436,11 @@ ApplyStateTable::insert(ReadView const& base, std::shared_ptr<SLE> const& sle)
     switch (item.first)
     {
         case Action::cache:
-            Throw<std::logic_error>("ApplyStateTable::insert: already cached");
+            LogicError("ApplyStateTable::insert: already cached");
         case Action::insert:
-            Throw<std::logic_error>("ApplyStateTable::insert: already inserted");
+            LogicError("ApplyStateTable::insert: already inserted");
         case Action::modify:
-            Throw<std::logic_error>("ApplyStateTable::insert: already modified");
+            LogicError("ApplyStateTable::insert: already modified");
         case Action::erase:
             break;
     }
@@ -468,7 +466,7 @@ ApplyStateTable::replace(ReadView const& base, std::shared_ptr<SLE> const& sle)
     switch (item.first)
     {
         case Action::erase:
-            Throw<std::logic_error>("ApplyStateTable::replace: already erased");
+            LogicError("ApplyStateTable::replace: already erased");
         case Action::cache:
             item.first = Action::modify;
             break;
@@ -484,14 +482,14 @@ ApplyStateTable::update(ReadView const& base, std::shared_ptr<SLE> const& sle)
 {
     auto const iter = items_.find(sle->key());
     if (iter == items_.end())
-        Throw<std::logic_error>("ApplyStateTable::update: missing key");
+        LogicError("ApplyStateTable::update: missing key");
     auto& item = iter->second;
     if (item.second != sle)
-        Throw<std::logic_error>("ApplyStateTable::update: unknown SLE");
+        LogicError("ApplyStateTable::update: unknown SLE");
     switch (item.first)
     {
         case Action::erase:
-            Throw<std::logic_error>("ApplyStateTable::update: erased");
+            LogicError("ApplyStateTable::update: erased");
             break;
         case Action::cache:
             item.first = Action::modify;

src/libxrpl/ledger/ApplyView.cpp

@@ -4,7 +4,6 @@
 #include <xrpl/protocol/Protocol.h>
 
 #include <limits>
-#include <stdexcept>
 #include <type_traits>
 
 namespace xrpl {
@@ -41,8 +40,10 @@ findPreviousPage(ApplyView& view, Keylet const& directory, SLE::ref start)
     {
         node = view.peek(keylet::page(directory, page));
         if (!node)
-            Throw<std::logic_error>(
-                "Directory chain: root back-pointer broken.");  // LCOV_EXCL_LINE
+        {  // LCOV_EXCL_START
+            LogicError("Directory chain: root back-pointer broken.");
+            // LCOV_EXCL_STOP
+        }
     }
 
     auto indexes = node->getFieldV256(sfIndexes);
@@ -61,20 +62,21 @@ insertKey(
     if (preserveOrder)
     {
         if (std::find(indexes.begin(), indexes.end(), key) != indexes.end())
-            Throw<std::logic_error>("dirInsert: double insertion");  // LCOV_EXCL_LINE
+            LogicError("dirInsert: double insertion");  // LCOV_EXCL_LINE
 
         indexes.push_back(key);
     }
     else
     {
-        // We can't be sure if this page is already sorted because it may be a
-        // legacy page we haven't yet touched. Take the time to sort it.
+        // We can't be sure if this page is already sorted because
+        // it may be a legacy page we haven't yet touched. Take
+        // the time to sort it.
         std::sort(indexes.begin(), indexes.end());
 
         auto pos = std::lower_bound(indexes.begin(), indexes.end(), key);
 
         if (pos != indexes.end() && key == *pos)
-            Throw<std::logic_error>("dirInsert: double insertion");  // LCOV_EXCL_LINE
+            LogicError("dirInsert: double insertion");  // LCOV_EXCL_LINE
 
         indexes.insert(pos, key);
     }
@@ -127,7 +129,8 @@ insertPage(
     node->setFieldH256(sfRootIndex, directory.key);
     node->setFieldV256(sfIndexes, indexes);
 
-    // Save some space by not specifying the value 0 since it's the default.
+    // Save some space by not specifying the value 0 since
+    // it's the default.
     if (page != 1)
         node->setFieldU64(sfIndexPrevious, page - 1);
     XRPL_ASSERT_PARTS(!nextPage, "xrpl::directory::insertPage", "nextPage has default value");
@@ -196,24 +199,28 @@ ApplyView::emptyDirDelete(Keylet const& directory)
     auto nextPage = node->getFieldU64(sfIndexNext);
 
     if (nextPage == rootPage && prevPage != rootPage)
-        Throw<std::logic_error>("Directory chain: fwd link broken");  // LCOV_EXCL_LINE
+        LogicError("Directory chain: fwd link broken");  // LCOV_EXCL_LINE
 
     if (prevPage == rootPage && nextPage != rootPage)
-        Throw<std::logic_error>("Directory chain: rev link broken");  // LCOV_EXCL_LINE
+        LogicError("Directory chain: rev link broken");  // LCOV_EXCL_LINE
 
-    // Older versions of the code would, in some cases, allow the last page to
-    // be empty. Remove such pages:
+    // Older versions of the code would, in some cases, allow the last
+    // page to be empty. Remove such pages:
     if (nextPage == prevPage && nextPage != rootPage)
     {
         auto last = peek(keylet::page(directory, nextPage));
 
         if (!last)
-            Throw<std::logic_error>("Directory chain: fwd link broken.");  // LCOV_EXCL_LINE
+        {  // LCOV_EXCL_START
+            LogicError("Directory chain: fwd link broken.");
+            // LCOV_EXCL_STOP
+        }
 
         if (!last->getFieldV256(sfIndexes).empty())
             return false;
 
-        // Update the first page's linked list and mark it as updated.
+        // Update the first page's linked list and
+        // mark it as updated.
         node->setFieldU64(sfIndexNext, rootPage);
         node->setFieldU64(sfIndexPrevious, rootPage);
         update(node);
@@ -221,7 +228,8 @@ ApplyView::emptyDirDelete(Keylet const& directory)
         // And erase the empty last page:
         erase(last);
 
-        // Make sure our local values reflect the updated information:
+        // Make sure our local values reflect the
+        // updated information:
         nextPage = rootPage;
         prevPage = rootPage;
     }
@@ -261,33 +269,46 @@ ApplyView::dirRemove(Keylet const& directory, std::uint64_t page, uint256 const&
             return true;
     }
 
-    // The current page is now empty; check if it can be deleted, and, if so,
-    // whether the entire directory can now be removed.
+    // The current page is now empty; check if it can be
+    // deleted, and, if so, whether the entire directory
+    // can now be removed.
     auto prevPage = node->getFieldU64(sfIndexPrevious);
     auto nextPage = node->getFieldU64(sfIndexNext);
 
-    // The first page is the directory's root node and is treated specially: it
-    // can never be deleted even if it is empty, unless we plan on removing the
-    // entire directory.
+    // The first page is the directory's root node and is
+    // treated specially: it can never be deleted even if
+    // it is empty, unless we plan on removing the entire
+    // directory.
     if (page == rootPage)
     {
         if (nextPage == page && prevPage != page)
-            Throw<std::logic_error>("Directory chain: fwd link broken");  // LCOV_EXCL_LINE
+        {  // LCOV_EXCL_START
+            LogicError("Directory chain: fwd link broken");
+            // LCOV_EXCL_STOP
+        }
 
         if (prevPage == page && nextPage != page)
-            Throw<std::logic_error>("Directory chain: rev link broken");  // LCOV_EXCL_LINE
+        {  // LCOV_EXCL_START
+            LogicError("Directory chain: rev link broken");
+            // LCOV_EXCL_STOP
+        }
 
-        // Older versions of the code would, in some cases, allow the last page
-        // to be empty. Remove such pages if we stumble on them:
+        // Older versions of the code would, in some cases,
+        // allow the last page to be empty. Remove such
+        // pages if we stumble on them:
         if (nextPage == prevPage && nextPage != page)
         {
             auto last = peek(keylet::page(directory, nextPage));
             if (!last)
-                Throw<std::logic_error>("Directory chain: fwd link broken.");  // LCOV_EXCL_LINE
+            {  // LCOV_EXCL_START
+                LogicError("Directory chain: fwd link broken.");
+                // LCOV_EXCL_STOP
+            }
 
             if (last->getFieldV256(sfIndexes).empty())
             {
-                // Update the first page's linked list and mark it as updated.
+                // Update the first page's linked list and
+                // mark it as updated.
                 node->setFieldU64(sfIndexNext, page);
                 node->setFieldU64(sfIndexPrevious, page);
                 update(node);
@@ -295,7 +316,8 @@ ApplyView::dirRemove(Keylet const& directory, std::uint64_t page, uint256 const&
                 // And erase the empty last page:
                 erase(last);
 
-                // Make sure our local values reflect the updated information:
+                // Make sure our local values reflect the
+                // updated information:
                 nextPage = page;
                 prevPage = page;
             }
@@ -313,24 +335,25 @@ ApplyView::dirRemove(Keylet const& directory, std::uint64_t page, uint256 const&
 
     // This can never happen for nodes other than the root:
     if (nextPage == page)
-        Throw<std::logic_error>("Directory chain: fwd link broken");  // LCOV_EXCL_LINE
+        LogicError("Directory chain: fwd link broken");  // LCOV_EXCL_LINE
 
     if (prevPage == page)
-        Throw<std::logic_error>("Directory chain: rev link broken");  // LCOV_EXCL_LINE
+        LogicError("Directory chain: rev link broken");  // LCOV_EXCL_LINE
 
-    // This node isn't the root, so it can either be in the middle of the list,
-    // or at the end. Unlink it first and then check if that leaves the list
-    // with only a root:
+    // This node isn't the root, so it can either be in the
+    // middle of the list, or at the end. Unlink it first
+    // and then check if that leaves the list with only a
+    // root:
     auto prev = peek(keylet::page(directory, prevPage));
     if (!prev)
-        Throw<std::logic_error>("Directory chain: fwd link broken.");  // LCOV_EXCL_LINE
+        LogicError("Directory chain: fwd link broken.");  // LCOV_EXCL_LINE
     // Fix previous to point to its new next.
     prev->setFieldU64(sfIndexNext, nextPage);
     update(prev);
 
     auto next = peek(keylet::page(directory, nextPage));
     if (!next)
-        Throw<std::logic_error>("Directory chain: rev link broken.");  // LCOV_EXCL_LINE
+        LogicError("Directory chain: rev link broken.");  // LCOV_EXCL_LINE
     // Fix next to point to its new previous.
     next->setFieldU64(sfIndexPrevious, prevPage);
     update(next);
@@ -338,12 +361,13 @@ ApplyView::dirRemove(Keylet const& directory, std::uint64_t page, uint256 const&
     // The page is no longer linked. Delete it.
     erase(node);
 
-    // Check whether the next page is the last page and, if so, whether it's
-    // empty. If it is, delete it.
+    // Check whether the next page is the last page and, if
+    // so, whether it's empty. If it is, delete it.
     if (nextPage != rootPage && next->getFieldU64(sfIndexNext) == rootPage &&
         next->getFieldV256(sfIndexes).empty())
     {
-        // Since next doesn't point to the root, it can't be pointing to prev.
+        // Since next doesn't point to the root, it
+        // can't be pointing to prev.
         erase(next);
 
         // The previous page is now the last page:
@@ -353,16 +377,18 @@ ApplyView::dirRemove(Keylet const& directory, std::uint64_t page, uint256 const&
         // And the root points to the last page:
         auto root = peek(keylet::page(directory, rootPage));
         if (!root)
-            Throw<std::logic_error>("Directory chain: root link broken.");  // LCOV_EXCL_LINE
-
+        {  // LCOV_EXCL_START
+            LogicError("Directory chain: root link broken.");
+            // LCOV_EXCL_STOP
+        }
         root->setFieldU64(sfIndexPrevious, prevPage);
         update(root);
 
         nextPage = rootPage;
     }
 
-    // If we're not keeping the root, then check to see if it's left empty.
-    // If so, delete it as well.
+    // If we're not keeping the root, then check to see if
+    // it's left empty. If so, delete it as well.
     if (!keepRoot && nextPage == rootPage && prevPage == rootPage)
     {
         if (prev->getFieldV256(sfIndexes).empty())

src/libxrpl/ledger/Ledger.cpp

@@ -12,7 +12,6 @@
 #include <xrpl/protocol/digest.h>
 #include <xrpl/protocol/jss.h>
 
-#include <stdexcept>
 #include <utility>
 #include <vector>
 
@@ -462,14 +461,14 @@ void
 Ledger::rawErase(std::shared_ptr<SLE> const& sle)
 {
     if (!stateMap_.delItem(sle->key()))
-        Throw<std::logic_error>("Ledger::rawErase: key not found");
+        LogicError("Ledger::rawErase: key not found");
 }
 
 void
 Ledger::rawErase(uint256 const& key)
 {
     if (!stateMap_.delItem(key))
-        Throw<std::logic_error>("Ledger::rawErase: key not found");
+        LogicError("Ledger::rawErase: key not found");
 }
 
 void
@@ -479,7 +478,7 @@ Ledger::rawInsert(std::shared_ptr<SLE> const& sle)
     sle->add(ss);
     if (!stateMap_.addGiveItem(
             SHAMapNodeType::tnACCOUNT_STATE, make_shamapitem(sle->key(), ss.slice())))
-        Throw<std::logic_error>("Ledger::rawInsert: key already exists");
+        LogicError("Ledger::rawInsert: key already exists");
 }
 
 void
@@ -489,7 +488,7 @@ Ledger::rawReplace(std::shared_ptr<SLE> const& sle)
     sle->add(ss);
     if (!stateMap_.updateGiveItem(
             SHAMapNodeType::tnACCOUNT_STATE, make_shamapitem(sle->key(), ss.slice())))
-        Throw<std::logic_error>("Ledger::rawReplace: key not found");
+        LogicError("Ledger::rawReplace: key not found");
 }
 
 void
@@ -505,7 +504,7 @@ Ledger::rawTxInsert(
     s.addVL(txn->peekData());
     s.addVL(metaData->peekData());
     if (!txMap_.addGiveItem(SHAMapNodeType::tnTRANSACTION_MD, make_shamapitem(key, s.slice())))
-        Throw<std::logic_error>("duplicate_tx: " + to_string(key));
+        LogicError("duplicate_tx: " + to_string(key));
 }
 
 uint256
@@ -523,7 +522,7 @@ Ledger::rawTxInsertWithHash(
     auto item = make_shamapitem(key, s.slice());
     auto hash = sha512Half(HashPrefix::txNode, item->slice(), item->key());
     if (!txMap_.addGiveItem(SHAMapNodeType::tnTRANSACTION_MD, std::move(item)))
-        Throw<std::logic_error>("duplicate_tx: " + to_string(key));
+        LogicError("duplicate_tx: " + to_string(key));
 
     return hash;
 }

src/libxrpl/ledger/OpenView.cpp

@@ -1,8 +1,6 @@
 #include <xrpl/basics/contract.h>
 #include <xrpl/ledger/OpenView.h>
 
-#include <stdexcept>
-
 namespace xrpl {
 
 class OpenView::txs_iter_impl : public txs_type::iter_base
@@ -249,7 +247,7 @@ OpenView::rawTxInsert(
     auto const result = txs_.emplace(
         std::piecewise_construct, std::forward_as_tuple(key), std::forward_as_tuple(txn, metaData));
     if (!result.second)
-        Throw<std::logic_error>("rawTxInsert: duplicate TX id: " + to_string(key));
+        LogicError("rawTxInsert: duplicate TX id: " + to_string(key));
 }
 
 }  // namespace xrpl

src/libxrpl/ledger/RawStateTable.cpp

@@ -1,8 +1,6 @@
 #include <xrpl/basics/contract.h>
 #include <xrpl/ledger/detail/RawStateTable.h>
 
-#include <stdexcept>
-
 namespace xrpl {
 namespace detail {
 
@@ -243,7 +241,7 @@ RawStateTable::erase(std::shared_ptr<SLE> const& sle)
     switch (item.action)
     {
         case Action::erase:
-            Throw<std::logic_error>("RawStateTable::erase: already erased");
+            LogicError("RawStateTable::erase: already erased");
             break;
         case Action::insert:
             items_.erase(result.first);
@@ -272,10 +270,10 @@ RawStateTable::insert(std::shared_ptr<SLE> const& sle)
             item.sle = sle;
             break;
         case Action::insert:
-            Throw<std::logic_error>("RawStateTable::insert: already inserted");
+            LogicError("RawStateTable::insert: already inserted");
             break;
         case Action::replace:
-            Throw<std::logic_error>("RawStateTable::insert: already exists");
+            LogicError("RawStateTable::insert: already exists");
             break;
     }
 }
@@ -293,7 +291,7 @@ RawStateTable::replace(std::shared_ptr<SLE> const& sle)
     switch (item.action)
     {
         case Action::erase:
-            Throw<std::logic_error>("RawStateTable::replace: was erased");
+            LogicError("RawStateTable::replace: was erased");
             break;
         case Action::insert:
         case Action::replace:

src/libxrpl/ledger/helpers/AccountRootHelpers.cpp

@@ -8,7 +8,6 @@
 
 #include <algorithm>
 #include <limits>
-#include <stdexcept>
 
 namespace xrpl {
 
@@ -154,7 +153,7 @@ getPseudoAccountFields()
         if (!ar)
         {
             // LCOV_EXCL_START
-            Throw<std::logic_error>(
+            LogicError(
                 "xrpl::getPseudoAccountFields : unable to find account root "
                 "ledger format");
             // LCOV_EXCL_STOP

src/test/app/LedgerMaster_test.cpp

@@ -2,6 +2,7 @@
 #include <test/jtx/Env.h>
 
 #include <xrpld/app/ledger/LedgerMaster.h>
+#include <xrpld/app/misc/SHAMapStore.h>
 
 namespace xrpl {
 namespace test {
@@ -97,6 +98,73 @@ 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()->header().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 = maxSeq;
+            }
+            BEAST_EXPECTS(
+                env.closed()->header().seq == maxSeq, to_string(env.closed()->header().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
@@ -110,6 +178,7 @@ public:
     testWithFeats(FeatureBitset features)
     {
         testTxnIdFromIndex(features);
+        testCompleteLedgerRange(features);
     }
 };
 

src/test/app/SHAMapStore_test.cpp

@@ -1,6 +1,7 @@
 #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>
@@ -10,6 +11,8 @@
 #include <xrpl/nodestore/detail/DatabaseRotatingImp.h>
 #include <xrpl/protocol/jss.h>
 
+#include <thread>
+
 namespace xrpl {
 namespace test {
 
@@ -20,10 +23,8 @@ class SHAMapStore_test : public beast::unit_test::suite
     static auto
     onlineDelete(std::unique_ptr<Config> cfg)
     {
-        cfg->LEDGER_HISTORY = deleteInterval;
-        auto& section = cfg->section(ConfigSection::nodeDatabase());
-        section.set("online_delete", std::to_string(deleteInterval));
-        return cfg;
+        using namespace jtx;
+        return online_delete(std::move(cfg), deleteInterval);
     }
 
     static auto
@@ -563,6 +564,158 @@ 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)};
+
+        std::map<LedgerIndex, uint256> hashes;
+
+        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();
+
+        auto& lm = env.app().getLedgerMaster();
+        LedgerIndex minSeq = 2;
+        LedgerIndex maxSeq = env.closed()->header().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 < 20)
+        {
+            for (int t = 0; t < 3; ++t)
+            {
+                env(noop(alice));
+            }
+            env.close();
+            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();
+                // 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();
+                    // 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 = deleteSeq + 1;
+            }
+            BEAST_EXPECT(maxSeq != lastRotated + deleteInterval);
+            BEAST_EXPECTS(
+                env.closed()->header().seq == maxSeq,
+                failureMessage("maxSeq", maxSeq, env.closed()->header().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
     {
@@ -570,6 +723,7 @@ public:
         testAutomatic();
         testCanDelete();
         testRotate();
+        testLedgerGaps();
     }
 };
 

src/test/core/Config_test.cpp

@@ -507,7 +507,7 @@ port_wss_admin
             {
                 c.loadFromString(boost::str(configTemplate % validationSeed % token));
             }
-            catch (std::runtime_error const& e)
+            catch (std::runtime_error& e)
             {
                 error = e.what();
             }
@@ -528,7 +528,7 @@ port_wss_admin
 main
 )xrpldConfig");
         }
-        catch (std::runtime_error const& e)
+        catch (std::runtime_error& e)
         {
             error = e.what();
         }
@@ -541,7 +541,7 @@ main
             c.loadFromString(R"xrpldConfig(
 )xrpldConfig");
         }
-        catch (std::runtime_error const& e)
+        catch (std::runtime_error& e)
         {
             error = e.what();
         }
@@ -556,7 +556,7 @@ main
 255
 )xrpldConfig");
         }
-        catch (std::runtime_error const& e)
+        catch (std::runtime_error& e)
         {
             error = e.what();
         }
@@ -571,7 +571,7 @@ main
 10000
 )xrpldConfig");
         }
-        catch (std::runtime_error const& e)
+        catch (std::runtime_error& e)
         {
             error = e.what();
         }
@@ -598,7 +598,7 @@ main
                 Config c;
                 c.loadFromString(boost::str(cc % missingPath));
             }
-            catch (std::runtime_error const& e)
+            catch (std::runtime_error& e)
             {
                 error = e.what();
             }
@@ -617,7 +617,7 @@ main
                 Config c;
                 c.loadFromString(boost::str(cc % invalidFile.string()));
             }
-            catch (std::runtime_error const& e)
+            catch (std::runtime_error& e)
             {
                 error = e.what();
             }
@@ -725,7 +725,7 @@ trust-these-validators.gov
                 c.loadFromString(toLoad);
                 fail();
             }
-            catch (std::runtime_error const& e)
+            catch (std::runtime_error& e)
             {
                 error = e.what();
             }
@@ -754,7 +754,7 @@ value = 2
                 c.loadFromString(toLoad);
                 fail();
             }
-            catch (std::runtime_error const& e)
+            catch (std::runtime_error& e)
             {
                 error = e.what();
             }
@@ -802,7 +802,7 @@ trust-these-validators.gov
                 c.loadFromString(toLoad);
                 fail();
             }
-            catch (std::runtime_error const& e)
+            catch (std::runtime_error& e)
             {
                 error = e.what();
             }
@@ -948,7 +948,7 @@ trust-these-validators.gov
                 c.loadFromString(boost::str(cc % vtg.validatorsFile()));
                 fail();
             }
-            catch (std::runtime_error const& e)
+            catch (std::runtime_error& e)
             {
                 error = e.what();
             }
@@ -974,7 +974,7 @@ trust-these-validators.gov
                 Config c2;
                 c2.loadFromString(boost::str(cc % vtg.validatorsFile()));
             }
-            catch (std::runtime_error const& e)
+            catch (std::runtime_error& e)
             {
                 error = e.what();
             }
@@ -1451,7 +1451,7 @@ r.ripple.com:51235
                     fail();
                 }
             }
-            catch (std::runtime_error const&)
+            catch (std::runtime_error&)
             {
                 if (!shouldPass)
                 {
@@ -1477,7 +1477,7 @@ r.ripple.com:51235
                 c.loadFromString("[overlay]\nmax_unknown_time=" + value);
                 return c.MAX_UNKNOWN_TIME;
             }
-            catch (std::runtime_error const&)
+            catch (std::runtime_error&)
             {
                 return {};
             }
@@ -1511,7 +1511,7 @@ r.ripple.com:51235
                 c.loadFromString("[overlay]\nmax_diverged_time=" + value);
                 return c.MAX_DIVERGED_TIME;
             }
-            catch (std::runtime_error const&)
+            catch (std::runtime_error&)
             {
                 return {};
             }

src/test/jtx/envconfig.h

@@ -57,6 +57,17 @@ 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/WSClient.cpp

@@ -184,7 +184,14 @@ public:
                 jp[jss::command] = cmd;
             }
             auto const s = to_string(jp);
-            ws_.write_some(true, buffer(s));
+
+            // Use the error_code overload to avoid an unhandled exception
+            // when the server closes the WebSocket connection (e.g. after
+            // booting a client that exceeded resource thresholds).
+            error_code ec;
+            ws_.write_some(true, buffer(s), ec);
+            if (ec)
+                return {};
         }
 
         auto jv =

src/test/jtx/impl/envconfig.cpp

@@ -53,6 +53,15 @@ 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/overlay/reduce_relay_test.cpp

@@ -1345,7 +1345,7 @@ vp_enable=0
             {
                 c.loadFromString(toLoad);
             }
-            catch (std::runtime_error const& e)
+            catch (std::runtime_error& e)
             {
                 error = e.what();
             }
@@ -1389,7 +1389,7 @@ vp_base_squelch_max_selected_peers=2
             {
                 c2.loadFromString(toLoad);
             }
-            catch (std::runtime_error const& e)
+            catch (std::runtime_error& e)
             {
                 error = e.what();
             }

src/test/protocol/STParsedJSON_test.cpp

@@ -2130,7 +2130,7 @@ class STParsedJSON_test : public beast::unit_test::suite
                 STParsedJSONObject const parsed("test", faultyJson);
                 BEAST_EXPECT(!parsed.object);
             }
-            catch (std::runtime_error const& e)
+            catch (std::runtime_error& e)
             {
                 std::string const what(e.what());
                 unexpected(what.find("First level children of `Template`") != 0);

src/xrpld/app/ledger/LedgerMaster.h

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

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

@@ -1529,12 +1529,34 @@ LedgerMaster::getPublishedLedger()
 }
 
 std::string
-LedgerMaster::getCompleteLedgers()
+LedgerMaster::getCompleteLedgers() const
 {
     std::lock_guard const 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

@@ -263,6 +263,17 @@ SHAMapStoreImp::run()
         bool const readyToRotate = 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)
         {
@@ -270,7 +281,8 @@ SHAMapStoreImp::run()
                                   << lastRotated << " deleteInterval " << deleteInterval_
                                   << " canDelete_ " << canDelete_ << " state "
                                   << app_.getOPs().strOperatingMode(false) << " age "
-                                  << ledgerMaster_->getValidatedLedgerAge().count() << 's';
+                                  << ledgerMaster_->getValidatedLedgerAge().count()
+                                  << "s. Complete ledgers: " << ledgerMaster_->getCompleteLedgers();
 
             clearPrior(lastRotated);
             if (healthWait() == stopping)
@@ -330,7 +342,9 @@ SHAMapStoreImp::run()
                     clearCaches(validatedSeq);
                 });
 
-            JLOG(journal_.warn()) << "finished rotation " << validatedSeq;
+            JLOG(journal_.warn()) << "finished rotation. validatedSeq: " << validatedSeq
+                                  << ", lastRotated: " << lastRotated
+                                  << ". Complete ledgers: " << ledgerMaster_->getCompleteLedgers();
         }
     }
 }
@@ -570,22 +584,40 @@ 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_);
-    while (!stop_ && (mode != OperatingMode::FULL || age > ageThreshold_))
+
+    auto numMissing =
+        ledgerMaster_->missingFromCompleteLedgerRange(lastGoodValidatedLedger_, index);
+    while (!stop_ && (mode != OperatingMode::FULL || age > ageThreshold_ || numMissing > 0))
     {
+        // this value shouldn't change, so grab it while we have the
+        // lock
+        auto const lastGood = lastGoodValidatedLedger_;
+
         lock.unlock();
-        JLOG(journal_.warn()) << "Waiting " << recoveryWaitTime_.count()
-                              << "s for node to stabilize. state: "
-                              << app_.getOPs().strOperatingMode(mode, false) << ". age "
-                              << age.count() << 's';
+
+        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();
         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

@@ -70,6 +70,11 @@ 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_;
@@ -83,11 +88,11 @@ private:
     std::uint32_t deleteBatch_ = 100;
     std::chrono::milliseconds backOff_{100};
     std::chrono::seconds ageThreshold_{60};
-    /// If  the node is out of sync during an online_delete healthWait()
-    /// call, sleep the thread for this time, and continue checking until
-    /// recovery.
+    /// 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.
     /// See also: "recovery_wait_seconds" in xrpld-example.cfg
-    std::chrono::seconds recoveryWaitTime_{5};
+    std::chrono::seconds recoveryWaitTime_{1};
 
     // these do not exist upon SHAMapStore creation, but do exist
     // as of run() or before
@@ -204,6 +209,8 @@ private:
     enum HealthResult { stopping, keepGoing };
     [[nodiscard]] HealthResult
     healthWait();
+    bool
+    hasCompleteRange(LedgerIndex first, LedgerIndex last);
 
 public:
     void

src/xrpld/rpc/detail/RPCCall.cpp

@@ -1632,7 +1632,7 @@ rpcClient(
         // YYY We could have a command line flag for single line output for
         // scripts. YYY We would intercept output here and simplify it.
     }
-    catch (RequestNotParsable const& e)
+    catch (RequestNotParsable& e)
     {
         jvOutput = rpcError(rpcINVALID_PARAMS);
         jvOutput["error_what"] = e.what();

src/xrpld/rpc/detail/TransactionSign.cpp

@@ -614,7 +614,7 @@ transactionPreProcessImpl(
 
         stTx = std::make_shared<STTx>(std::move(parsed.object.value()));
     }
-    catch (STObject::FieldErr const& err)
+    catch (STObject::FieldErr& err)
     {
         return RPC::make_error(rpcINVALID_PARAMS, err.what());
     }
@@ -1286,7 +1286,7 @@ transactionSubmitMultiSigned(
         {
             stTx = std::make_shared<STTx>(std::move(parsedTx_json.object.value()));
         }
-        catch (STObject::FieldErr const& err)
+        catch (STObject::FieldErr& err)
         {
             return RPC::make_error(rpcINVALID_PARAMS, err.what());
         }

src/xrpld/rpc/handlers/ledger/LedgerEntry.cpp

@@ -897,7 +897,7 @@ doLedgerEntry(RPC::JsonContext& context)
             return RPC::make_param_error("No ledger_entry params provided.");
         }
     }
-    catch (Json::error const& e)
+    catch (Json::error& e)
     {
         if (context.apiVersion > 1u)
         {

src/xrpld/rpc/handlers/subscribe/Subscribe.cpp

@@ -65,7 +65,7 @@ doSubscribe(RPC::JsonContext& context)
                 ispSub =
                     context.netOps.addRpcSub(strUrl, std::dynamic_pointer_cast<InfoSub>(rspSub));
             }
-            catch (std::runtime_error const& ex)
+            catch (std::runtime_error& ex)
             {
                 return RPC::make_param_error(ex.what());
             }