Merge branch 'develop' into ximinez/parse-guard

- Rename oldExpected to oldExpected_.
- Prevent copying, and moving.
- Make getParseFailureGuard [[nodiscard]].

cfg/xrpld-example.cfg

@@ -1059,11 +1059,10 @@
 #                           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, 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 2.
+#                           Default is 5.
 #
 #   Notes:
 #       The 'node_db' entry configures the primary, persistent storage.

src/test/app/Batch_test.cpp

@@ -498,7 +498,7 @@ class Batch_test : public beast::unit_test::Suite
             auto const batchFee = batch::calcBatchFee(env, 0, 2);
             auto tx1 = batch::Inner(pay(alice, bob, XRP(1)), seq + 1);
             tx1[jss::Fee] = "1.5";
-            env.setParseFailureExpected(true);
+            auto const g = env.getParseFailureGuard(true);
             try
             {
                 env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
@@ -510,7 +510,6 @@ class Batch_test : public beast::unit_test::Suite
             {
                 BEAST_EXPECT(true);
             }
-            env.setParseFailureExpected(false);
         }
 
         // temSEQ_AND_TICKET: Batch: inner txn cannot have both Sequence

src/test/app/LedgerMaster_test.cpp

@@ -5,21 +5,17 @@
 #include <test/jtx/noop.h>
 
 #include <xrpld/app/ledger/LedgerMaster.h>
-#include <xrpld/app/misc/SHAMapStore.h>
 #include <xrpld/core/Config.h>
 
-#include <xrpl/basics/ToString.h>
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/protocol/Feature.h>
-#include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STObject.h>
 #include <xrpl/protocol/STTx.h>
 
 #include <cstdint>
 #include <memory>
-#include <sstream>
 #include <vector>
 
 namespace xrpl::test {
@@ -115,73 +111,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 onlineDelete(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();
-        BEAST_EXPECT(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();
-            BEAST_EXPECT(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
@@ -195,7 +124,6 @@ public:
     testWithFeats(FeatureBitset features)
     {
         testTxnIdFromIndex(features);
-        testCompleteLedgerRange(features);
     }
 };
 

src/test/app/SHAMapStore_test.cpp

@@ -1,9 +1,7 @@
 #include <test/jtx/Env.h>
 #include <test/jtx/amount.h>
 #include <test/jtx/envconfig.h>
-#include <test/jtx/noop.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,7 +9,6 @@
 #include <xrpld/core/Config.h>
 
 #include <xrpl/basics/ByteUtilities.h>
-#include <xrpl/basics/Number.h>
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/config/BasicConfig.h>
@@ -28,15 +25,12 @@
 #include <boost/filesystem/path.hpp>
 
 #include <atomic>
-#include <cstddef>
 #include <cstdint>
 #include <limits>
 #include <map>
 #include <memory>
 #include <optional>
-#include <sstream>
 #include <string>
-#include <thread>
 #include <utility>
 
 namespace xrpl::test {
@@ -48,7 +42,10 @@ class SHAMapStore_test : public beast::unit_test::Suite
     static auto
     onlineDelete(std::unique_ptr<Config> cfg)
     {
-        return jtx::onlineDelete(std::move(cfg), kDeleteInterval);
+        cfg->ledgerHistory = kDeleteInterval;
+        auto& section = cfg->section(Sections::kNodeDatabase);
+        section.set(Keys::kOnlineDelete, std::to_string(kDeleteInterval));
+        return cfg;
     }
 
     static auto
@@ -146,11 +143,11 @@ class SHAMapStore_test : public beast::unit_test::Suite
         auto& store = env.app().getSHAMapStore();
 
         int ledgerSeq = 3;
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
         BEAST_EXPECT(!store.getLastRotated());
 
         env.close();
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         auto ledger = env.rpc("ledger", "validated");
         BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++)));
@@ -230,7 +227,7 @@ public:
             BEAST_EXPECT(goodLedger(env, ledger, std::to_string(kDeleteInterval + 4)));
         }
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         BEAST_EXPECT(store.getLastRotated() == kDeleteInterval + 3);
         lastRotated = store.getLastRotated();
@@ -257,7 +254,7 @@ public:
                 !getHash(ledgers[i]).empty());
         }
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         BEAST_EXPECT(store.getLastRotated() == kDeleteInterval + lastRotated);
 
@@ -295,7 +292,7 @@ public:
             BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true));
         }
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         // The database will always have back to ledger 2,
         // regardless of lastRotated.
@@ -310,7 +307,7 @@ public:
             BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true));
         }
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         ledgerCheck(env, ledgerSeq - lastRotated, lastRotated);
         BEAST_EXPECT(lastRotated != store.getLastRotated());
@@ -326,7 +323,7 @@ public:
             BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true));
         }
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         ledgerCheck(env, kDeleteInterval + 1, lastRotated);
         BEAST_EXPECT(lastRotated != store.getLastRotated());
@@ -365,7 +362,7 @@ public:
             BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true));
         }
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         ledgerCheck(env, ledgerSeq - 2, 2);
         BEAST_EXPECT(lastRotated == store.getLastRotated());
@@ -375,7 +372,7 @@ public:
         BEAST_EXPECT(!RPC::containsError(canDelete[jss::result]));
         BEAST_EXPECT(canDelete[jss::result][jss::can_delete] == ledgerSeq + (kDeleteInterval / 2));
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         ledgerCheck(env, ledgerSeq - 2, 2);
         BEAST_EXPECT(store.getLastRotated() == lastRotated);
@@ -388,7 +385,7 @@ public:
             BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true));
         }
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         ledgerCheck(env, ledgerSeq - lastRotated, lastRotated);
 
@@ -404,7 +401,7 @@ public:
             BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true));
         }
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         BEAST_EXPECT(store.getLastRotated() == lastRotated);
 
@@ -416,7 +413,7 @@ public:
             BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true));
         }
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         ledgerCheck(env, ledgerSeq - firstBatch, firstBatch);
 
@@ -438,7 +435,7 @@ public:
             BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true));
         }
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         BEAST_EXPECT(store.getLastRotated() == lastRotated);
 
@@ -450,7 +447,7 @@ public:
             BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true));
         }
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         ledgerCheck(env, ledgerSeq - lastRotated, lastRotated);
 
@@ -471,7 +468,7 @@ public:
             BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq), true));
         }
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         BEAST_EXPECT(store.getLastRotated() == lastRotated);
 
@@ -483,7 +480,7 @@ public:
             BEAST_EXPECT(goodLedger(env, ledger, std::to_string(ledgerSeq++), true));
         }
 
-        BEAST_EXPECT(store.rendezvous());
+        store.rendezvous();
 
         ledgerCheck(env, ledgerSeq - lastRotated, lastRotated);
 
@@ -606,166 +603,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 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();
-        BEAST_EXPECT(store.rendezvous());
-        LedgerIndex lastRotated = store.getLastRotated();
-        BEAST_EXPECTS(maxSeq == 3, std::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();
-            BEAST_EXPECT(store.rendezvous());
-
-            ++maxSeq;
-
-            if (maxSeq + 1 == lastRotated + kDeleteInterval)
-            {
-                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;
-                {
-                    std::size_t iterations = 30;
-                    while (!lm.haveLedger(deleteSeq) && --iterations > 0)
-                    {
-                        std::this_thread::sleep_for(100ms);
-                    }
-                    BEAST_EXPECTS(iterations > 25, to_string(iterations));
-                }
-                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() without a timeout parameter! 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 a little bit 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();
-                    ++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()));
-                    // The Store is "stuck" in healthWait() and won't finish the run() loop until
-                    // it's backfilled
-                    BEAST_EXPECT(!store.rendezvous(100ms));
-                }
-
-                // Put the missing ledger back in LedgerMaster
-                lm.setLedgerRangePresent(deleteSeq, deleteSeq);
-
-                // Wait for the rotation to finish
-                BEAST_EXPECT(store.rendezvous());
-
-                minSeq = lastRotated;
-                lastRotated = deleteSeq + 1;
-            }
-            BEAST_EXPECT(maxSeq != lastRotated + kDeleteInterval);
-            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
     {
@@ -773,7 +610,6 @@ public:
         testAutomatic();
         testCanDelete();
         testRotate();
-        testLedgerGaps();
     }
 };
 

src/test/app/Vault_test.cpp

@@ -5451,9 +5451,9 @@ class Vault_test : public beast::unit_test::Suite
             env.close();
 
             // 2. Mantissa larger than uint64 max
-            env.setParseFailureExpected(true);
             try
             {
+                auto const g = env.getParseFailureGuard(true);
                 tx[sfAssetsMaximum] = "18446744073709551617e5";  // uint64 max + 1
                 env(tx);
                 BEAST_EXPECTS(false, "Expected parse_error for mantissa larger than uint64 max");
@@ -5464,7 +5464,6 @@ class Vault_test : public beast::unit_test::Suite
                 BEAST_EXPECT(
                     e.what() == "invalidParamsField 'tx_json.AssetsMaximum' has invalid data."s);
             }
-            env.setParseFailureExpected(false);
         }
     }
 

src/test/jtx/Env.h

@@ -482,6 +482,46 @@ public:
         parseFailureExpected_ = b;
     }
 
+    /** RAII class to set and restore the parse failure flag (setParseFailureExpected).
+     *
+     * Can be created directly, or through the `getParseFailureGuard(bool)` function.
+     */
+    class ParseFailureGuard final
+    {
+        Env& self_;
+        bool oldExpected_ = self_.parseFailureExpected_;
+
+    public:
+        ParseFailureGuard(Env& self, bool b) : self_(self)
+        {
+            self_.setParseFailureExpected(b);
+        }
+
+        ~ParseFailureGuard()
+        {
+            self_.setParseFailureExpected(oldExpected_);
+        }
+
+        // No copy, no move
+        ParseFailureGuard(ParseFailureGuard const&) = delete;
+        ParseFailureGuard&
+        operator=(ParseFailureGuard const&) = delete;
+        ParseFailureGuard(ParseFailureGuard&& other) = delete;
+        ParseFailureGuard&
+        operator=(ParseFailureGuard&&) = delete;
+    };
+
+    /** Gets an RAII guard to set and restore the parse failure flag
+     *
+     * Usage:
+     * auto const guard = env.getParseFailureGuard(true/false);
+     */
+    [[nodiscard]] ParseFailureGuard
+    getParseFailureGuard(bool b)
+    {
+        return ParseFailureGuard{*this, b};
+    }
+
     /** Turn off signature checks. */
     void
     disableSigs()

src/test/jtx/envconfig.h

@@ -51,17 +51,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>
-onlineDelete(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/Env.cpp

@@ -602,7 +602,7 @@ Env::autofill(JTx& jt)
     catch (ParseError const&)
     {
         if (!parseFailureExpected_)
-            test.log << "parse failed:\n" << pretty(jv) << std::endl;
+            test.log << "parse failure:\n" << pretty(jv) << std::endl;
         rethrow();
     }
 }

src/test/jtx/impl/envconfig.cpp

@@ -7,10 +7,8 @@
 #include <xrpl/config/Constants.h>
 
 #include <atomic>
-#include <cstdint>
 #include <map>
 #include <memory>
-#include <string>
 #include <vector>
 
 namespace xrpl::test {
@@ -62,15 +60,6 @@ setupConfigForUnitTests(Config& cfg)
 
 namespace jtx {
 
-std::unique_ptr<Config>
-onlineDelete(std::unique_ptr<Config> cfg, std::uint32_t deleteInterval)
-{
-    cfg->ledgerHistory = deleteInterval;
-    auto& section = cfg->section(Sections::kNodeDatabase);
-    section.set(Keys::kOnlineDelete, std::to_string(deleteInterval));
-    return cfg;
-}
-
 std::unique_ptr<Config>
 noAdmin(std::unique_ptr<Config> cfg)
 {

src/xrpld/app/ledger/LedgerMaster.h

@@ -105,10 +105,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.
@@ -325,7 +322,7 @@ private:
     // A set of transactions to replay during the next close
     std::unique_ptr<LedgerReplay> replayData_;
 
-    std::recursive_mutex mutable completeLock_;
+    std::recursive_mutex completeLock_;
     RangeSet<std::uint32_t> completeLedgers_;
 
     // Publish thread is running.

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

@@ -56,7 +56,6 @@
 #include <xrpl/shamap/SHAMapMissingNode.h>
 #include <xrpl/shamap/SHAMapTreeNode.h>
 
-#include <boost/icl/concept/interval_associator.hpp>
 #include <boost/icl/concept/interval_set.hpp>
 
 #include <xrpl.pb.h>
@@ -1571,25 +1570,12 @@ LedgerMaster::getPublishedLedger()
 }
 
 std::string
-LedgerMaster::getCompleteLedgers() const
+LedgerMaster::getCompleteLedgers()
 {
     std::scoped_lock const sl(completeLock_);
     return to_string(completeLedgers_);
 }
 
-std::size_t
-LedgerMaster::missingFromCompleteLedgerRange(LedgerIndex first, LedgerIndex last) const
-{
-    RangeSet<LedgerIndex> const target{range(first, last)};
-
-    auto const missing = [&target, this] {
-        std::scoped_lock const sl(completeLock_);
-        return target - completeLedgers_;
-    }();
-
-    return boost::icl::size(missing);
-}
-
 std::optional<NetClock::time_point>
 LedgerMaster::getCloseTimeBySeq(LedgerIndex ledgerIndex)
 {

src/xrpld/app/misc/SHAMapStore.h

@@ -5,7 +5,6 @@
 #include <xrpl/ledger/Ledger.h>
 #include <xrpl/nodestore/Manager.h>
 
-#include <chrono>
 #include <optional>
 
 namespace xrpl {
@@ -28,8 +27,8 @@ public:
     virtual void
     start() = 0;
 
-    [[nodiscard]] virtual bool
-    rendezvous(std::optional<std::chrono::milliseconds> const& timeout = {}) const = 0;
+    virtual void
+    rendezvous() const = 0;
 
     virtual void
     stop() = 0;

src/xrpld/app/misc/SHAMapStoreImp.cpp

@@ -8,7 +8,6 @@
 #include <xrpl/basics/ByteUtilities.h>
 #include <xrpl/basics/Log.h>
 #include <xrpl/basics/contract.h>
-#include <xrpl/basics/scope.h>
 #include <xrpl/beast/core/CurrentThreadName.h>
 #include <xrpl/beast/utility/Journal.h>
 #include <xrpl/beast/utility/instrumentation.h>
@@ -30,7 +29,6 @@
 #include <boost/filesystem/path.hpp>
 
 #include <algorithm>
-#include <chrono>
 #include <cstdint>
 #include <functional>
 #include <limits>
@@ -235,21 +233,14 @@ SHAMapStoreImp::onLedgerClosed(std::shared_ptr<Ledger const> const& ledger)
     cond_.notify_one();
 }
 
-bool
-SHAMapStoreImp::rendezvous(std::optional<std::chrono::milliseconds> const& timeout) const
+void
+SHAMapStoreImp::rendezvous() const
 {
     if (!working_)
-        return true;
-
-    auto notWorking = [&] { return !working_; };
+        return;
 
     std::unique_lock<std::mutex> lock(mutex_);
-    if (timeout)
-    {
-        return rendezvous_.wait_for(lock, *timeout, notWorking);
-    }
-    rendezvous_.wait(lock, notWorking);
-    return true;
+    rendezvous_.wait(lock, [&] { return !working_; });
 }
 
 int
@@ -320,16 +311,6 @@ SHAMapStoreImp::run()
         bool const readyToRotate = validatedSeq >= lastRotated + deleteInterval_ &&
             canDelete_ >= lastRotated - 1 && healthWait() == HealthResult::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> const lock(mutex_);
-            lastGoodValidatedLedger_ = validatedSeq;
-        }
-
         // will delete up to (not including) lastRotated
         if (readyToRotate)
         {
@@ -337,8 +318,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() == HealthResult::Stopping)
@@ -398,9 +378,7 @@ SHAMapStoreImp::run()
                     clearCaches(validatedSeq);
                 });
 
-            JLOG(journal_.warn()) << "finished rotation. validatedSeq: " << validatedSeq
-                                  << ", lastRotated: " << lastRotated
-                                  << ". Complete ledgers: " << ledgerMaster_->getCompleteLedgers();
+            JLOG(journal_.warn()) << "finished rotation " << validatedSeq;
         }
     }
 }
@@ -644,63 +622,20 @@ SHAMapStoreImp::clearPrior(LedgerIndex lastRotated)
 SHAMapStoreImp::HealthResult
 SHAMapStoreImp::healthWait()
 {
-    // Gets the current status of the server from ledgerMaster_ and netOPs_. Must be called
-    // while mutex_ is unlocked to avoid unlikely, but possible, deadlock with ledgerMaster_'s
-    // completeLock_.
-    // Releasing the lock may mean that status will be slightly out of date when the lock is
-    // reacquired, but it's close enough. In a normal rotation, healthWait() is called frequently,
-    // so a false positive will be detected on the next call, and a false negative wil be detected
-    // in the next loop interation. Database rotation is important, but not timely, so an extra
-    // delay is fine.
-    auto readServerStatus = [this](
-                                LedgerIndex& index,
-                                std::chrono::seconds& age,
-                                OperatingMode& mode,
-                                std::size_t& numMissing,
-                                LedgerIndex const lowerBound,
-                                ScopeUnlock<decltype(mutex_)> const&) {
-        index = ledgerMaster_->getValidLedgerIndex();
+    auto age = ledgerMaster_->getValidatedLedgerAge();
+    OperatingMode mode = netOPs_->getOperatingMode();
+    std::unique_lock lock(mutex_);
+    while (!stop_ && (mode != OperatingMode::FULL || age > ageThreshold_))
+    {
+        lock.unlock();
+        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_);
         age = ledgerMaster_->getValidatedLedgerAge();
         mode = netOPs_->getOperatingMode();
-
-        numMissing =
-            lowerBound == 0 ? 0 : ledgerMaster_->missingFromCompleteLedgerRange(lowerBound, index);
-    };
-
-    // Tracked server status properties
-    LedgerIndex index;
-    std::chrono::seconds age;
-    OperatingMode mode;
-    std::size_t numMissing;
-
-    std::unique_lock lock(mutex_);
-
-    auto const waitTime = recoveryWaitTime_;
-    auto const ageThreshold = ageThreshold_;
-    {
-        auto const lowerBound = lastGoodValidatedLedger_;
-
-        ScopeUnlock const unlock(lock);
-
-        readServerStatus(index, age, mode, numMissing, lowerBound, unlock);
-    }
-    while (!stop_ && (mode != OperatingMode::FULL || age > ageThreshold || numMissing > 0))
-    {
-        // this value shouldn't change, so grab it while we have the
-        // lock
-        auto const lowerBound = lastGoodValidatedLedger_;
-
-        ScopeUnlock const unlock(lock);
-
-        auto const stream =
-            mode != OperatingMode::FULL || age > ageThreshold ? journal_.warn() : journal_.info();
-        JLOG(stream) << "Waiting " << waitTime.count() << "s for node to stabilize. state: "
-                     << app_.getOPs().strOperatingMode(mode, false) << ". age " << age.count()
-                     << "s. Missing ledgers: " << numMissing << ".  Expect: " << lowerBound << "-"
-                     << index << ". Complete ledgers: " << ledgerMaster_->getCompleteLedgers();
-        std::this_thread::sleep_for(waitTime);
-
-        readServerStatus(index, age, mode, numMissing, lowerBound, unlock);
+        lock.lock();
     }
 
     return stop_ ? HealthResult::Stopping : HealthResult::KeepGoing;

src/xrpld/app/misc/SHAMapStoreImp.h

@@ -72,11 +72,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_;
@@ -90,11 +85,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 xrpld-example.cfg
-    std::chrono::seconds recoveryWaitTime_{2};
+    std::chrono::seconds recoveryWaitTime_{5};
 
     // these do not exist upon SHAMapStore creation, but do exist
     // as of run() or before
@@ -150,8 +145,8 @@ public:
     void
     onLedgerClosed(std::shared_ptr<Ledger const> const& ledger) override;
 
-    bool
-    rendezvous(std::optional<std::chrono::milliseconds> const& timeout = {}) const override;
+    void
+    rendezvous() const override;
     int
     fdRequired() const override;