wait for extra close before switching to LM_SYNCED_STATE

Signed-off-by: Rafał Malinowski <[email protected]>

src/herder/HerderImpl.cpp

@@ -123,7 +123,7 @@ HerderImpl::bootstrap()
     assert(getSCP().isValidator());
     assert(mApp.getConfig().FORCE_SCP);
 
-    mLedgerManager.setState(LedgerManager::LM_SYNCED_STATE);
+    mLedgerManager.bootstrap();
     mHerderSCPDriver.bootstrap();
 
     ledgerClosed();

src/history/HistoryTests.cpp

@@ -141,7 +141,7 @@ TEST_CASE("Full history catchup", "[history][historycatchup]")
 
     uint32_t initLedger =
         catchupSimulation.getApp().getLedgerManager().getLastClosedLedgerNum() -
-        1;
+        2;
 
     std::vector<Application::pointer> apps;
 
@@ -219,7 +219,7 @@ TEST_CASE("History prefix catchup", "[history][historycatchup][prefixcatchup]")
         std::string("Catchup to prefix of published history"));
     apps.push_back(a);
     uint32_t freq = apps.back()->getHistoryManager().getCheckpointFrequency();
-    CHECK(apps.back()->getLedgerManager().getLedgerNum() == freq + 1);
+    CHECK(apps.back()->getLedgerManager().getLedgerNum() == freq + 2);
 
     // Then attempt catchup to 74, prefix of 128. Should round up to 128.
     // Should replay the 64th (since it gets externalized) and land on 129.
@@ -228,7 +228,7 @@ TEST_CASE("History prefix catchup", "[history][historycatchup][prefixcatchup]")
         Config::TESTDB_IN_MEMORY_SQLITE,
         std::string("Catchup to second prefix of published history"));
     apps.push_back(a);
-    CHECK(apps.back()->getLedgerManager().getLedgerNum() == 2 * freq + 1);
+    CHECK(apps.back()->getLedgerManager().getLedgerNum() == 2 * freq + 2);
 }
 
 TEST_CASE("Publish/catchup alternation, with stall",
@@ -245,7 +245,7 @@ TEST_CASE("Publish/catchup alternation, with stall",
 
     auto& lm = catchupSimulation.getApp().getLedgerManager();
 
-    uint32_t initLedger = lm.getLastClosedLedgerNum() - 1;
+    uint32_t initLedger = lm.getLastClosedLedgerNum() - 2;
 
     app2 = catchupSimulation.catchupNewApplication(
         initLedger, std::numeric_limits<uint32_t>::max(), false,
@@ -263,7 +263,7 @@ TEST_CASE("Publish/catchup alternation, with stall",
         // Now alternate between publishing new stuff and catching up to it.
         catchupSimulation.generateAndPublishHistory(i);
 
-        initLedger = lm.getLastClosedLedgerNum() - 1;
+        initLedger = lm.getLastClosedLedgerNum() - 2;
 
         catchupSimulation.catchupApplication(
             initLedger, std::numeric_limits<uint32_t>::max(), false, app2);
@@ -276,8 +276,8 @@ TEST_CASE("Publish/catchup alternation, with stall",
     // By now we should have had 3 + 1 + 2 + 3 = 9 publishes, and should
     // have advanced 1 ledger in to the 9th block.
     uint32_t freq = app2->getHistoryManager().getCheckpointFrequency();
-    CHECK(app2->getLedgerManager().getLedgerNum() == 9 * freq + 1);
-    CHECK(app3->getLedgerManager().getLedgerNum() == 9 * freq + 1);
+    CHECK(app2->getLedgerManager().getLedgerNum() == 9 * freq + 2);
+    CHECK(app3->getLedgerManager().getLedgerNum() == 9 * freq + 2);
 
     // Finally, publish a little more history than the last publish-point
     // but not enough to get to the _next_ publish-point:
@@ -290,7 +290,7 @@ TEST_CASE("Publish/catchup alternation, with stall",
     // by providing 30 cranks of the event loop and assuming that failure
     // to catch up within that time means 'stalled'.
 
-    initLedger = lm.getLastClosedLedgerNum() - 1;
+    initLedger = lm.getLastClosedLedgerNum() - 2;
 
     REQUIRE(!catchupSimulation.catchupApplication(
         initLedger, std::numeric_limits<uint32_t>::max(), false, app2));
@@ -482,22 +482,22 @@ TEST_CASE("too far behind / catchup restart", "[history][catchupstall]")
     // Catch up successfully the first time
     auto app2 = catchupSimulation.catchupNewApplication(
         catchupSimulation.getApp().getLedgerManager().getLastClosedLedgerNum() -
-            1,
+            2,
         std::numeric_limits<uint32_t>::max(), false,
         Config::TESTDB_IN_MEMORY_SQLITE, "app2");
 
     // Now generate a little more history
     catchupSimulation.generateAndPublishHistory(1);
 
     auto init = app2->getLedgerManager().getLastClosedLedgerNum() + 2;
-    REQUIRE(init == 66);
+    REQUIRE(init == 67);
 
     // Now start a catchup on that catchups as far as it can due to gap
     LOG(INFO) << "Starting catchup (with gap) from " << init;
     REQUIRE(catchupSimulation.catchupApplication(
         init, std::numeric_limits<uint32_t>::max(), false, app2, true,
         init + 10));
-    REQUIRE(app2->getLedgerManager().getLastClosedLedgerNum() == 75);
+    REQUIRE(app2->getLedgerManager().getLastClosedLedgerNum() == 76);
 
     app2->getWorkManager().clearChildren();
 
@@ -507,10 +507,10 @@ TEST_CASE("too far behind / catchup restart", "[history][catchupstall]")
     // And catchup successfully
     init =
         catchupSimulation.getApp().getLedgerManager().getLastClosedLedgerNum() -
-        1;
+        2;
     REQUIRE(catchupSimulation.catchupApplication(
         init, std::numeric_limits<uint32_t>::max(), false, app2));
-    REQUIRE(app2->getLedgerManager().getLastClosedLedgerNum() == 192);
+    REQUIRE(app2->getLedgerManager().getLastClosedLedgerNum() == 193);
 }
 
 /*
@@ -530,7 +530,7 @@ TEST_CASE("Catchup recent", "[history][catchuprecent]")
     // Network is currently sitting on ledger 0xc0 (192)
     uint32_t initLedger =
         catchupSimulation.getApp().getLedgerManager().getLastClosedLedgerNum() -
-        1;
+        2;
 
     // Check that isolated catchups work at a variety of boundary
     // conditions relative to the size of a checkpoint:
@@ -550,7 +550,7 @@ TEST_CASE("Catchup recent", "[history][catchuprecent]")
     catchupSimulation.generateAndPublishHistory(2);
     initLedger =
         catchupSimulation.getApp().getLedgerManager().getLastClosedLedgerNum() -
-        1;
+        2;
 
     for (auto a : apps)
     {
@@ -562,7 +562,7 @@ TEST_CASE("Catchup recent", "[history][catchuprecent]")
     catchupSimulation.generateAndPublishHistory(25);
     initLedger =
         catchupSimulation.getApp().getLedgerManager().getLastClosedLedgerNum() -
-        1;
+        2;
 
     for (auto a : apps)
     {
@@ -582,15 +582,15 @@ TEST_CASE("Catchup manual", "[history][catchupmanual]")
     catchupSimulation.generateAndPublishInitialHistory(6);
     auto initLedger1 =
         catchupSimulation.getApp().getLedgerManager().getLastClosedLedgerNum() -
-        1;
+        2;
     REQUIRE(initLedger1 == 383);
 
     // Now push network along a little bit and see that they can all still
     // catch up properly.
     catchupSimulation.generateAndPublishHistory(2);
     auto initLedger2 =
         catchupSimulation.getApp().getLedgerManager().getLastClosedLedgerNum() -
-        1;
+        2;
 
     for (auto const& test : stellar::gCatchupRangeCases)
     {

src/history/HistoryTestsUtils.cpp

@@ -335,6 +335,9 @@ CatchupSimulation::generateAndPublishHistory(size_t nPublishes)
         // to just-before-LCL
         generateRandomLedger();
         ++ledgerSeq;
+        // One more for trigger ledger
+        generateRandomLedger();
+        ++ledgerSeq;
         REQUIRE(lm.getCurrentLedgerHeader().ledgerSeq == ledgerSeq);
 
         // Advance until we've published (or failed to!)
@@ -349,7 +352,7 @@ CatchupSimulation::generateAndPublishHistory(size_t nPublishes)
     REQUIRE(hm.getPublishSuccessCount() == publishSuccesses + nPublishes);
     REQUIRE(lm.getLedgerNum() ==
             ((publishSuccesses + nPublishes) * hm.getCheckpointFrequency()) +
-                1);
+                2);
 }
 
 Application::pointer
@@ -449,9 +452,9 @@ CatchupSimulation::catchupApplication(uint32_t initLedger, uint32_t count,
     // externalize anything we haven't yet published, of course.
     if (!manual)
     {
-        uint32_t nextBlockStart =
-            mApp.getHistoryManager().nextCheckpointLedger(initLedger);
-        for (uint32_t n = initLedger + 1; n <= nextBlockStart; ++n)
+        uint32_t triggerLedger =
+            mApp.getHistoryManager().nextCheckpointLedger(initLedger) + 1;
+        for (uint32_t n = initLedger + 1; n <= triggerLedger; ++n)
         {
             // Remember the vectors count from 2, not 0.
             if (n - 2 >= mLedgerCloseDatas.size())
@@ -494,7 +497,10 @@ CatchupSimulation::catchupApplication(uint32_t initLedger, uint32_t count,
                           << "[" << mLedgerSeqs.front() << ", "
                           << mLedgerSeqs.back() << "]";
 
-    if (app2->getLedgerManager().getState() != LedgerManager::LM_SYNCED_STATE)
+    if (app2->getLedgerManager().getState() !=
+            LedgerManager::LM_CATCHING_UP_STATE ||
+        app2->getLedgerManager().getCatchupState() !=
+            LedgerManager::CatchupState::WAITING_FOR_CLOSING_LEDGER)
     {
         CLOG(INFO, "History") << "Catching up failed: state = "
                               << app2->getLedgerManager().getState();

src/ledger/LedgerManager.h

@@ -62,8 +62,17 @@ class LedgerManager
         LM_NUM_STATE
     };
 
-    virtual void setState(State s) = 0;
+    enum class CatchupState
+    {
+        NONE,
+        WAITING_FOR_TRIGGER_LEDGER,
+        APPLYING_HISTORY,
+        WAITING_FOR_CLOSING_LEDGER
+    };
+
+    virtual void bootstrap() = 0;
     virtual State getState() const = 0;
+    virtual CatchupState getCatchupState() const = 0;
     virtual std::string getStateHuman() const = 0;
 
     bool