Switch to querying LCL data from read-only snapshots only

src/bucket/BucketSnapshotManager.cpp

@@ -98,22 +98,24 @@ template <>
 void
 BucketSnapshotManager::maybeUpdateSnapshot<LiveBucket>(
     SnapshotPtrT<LiveBucket>& snapshot,
-    std::map<uint32_t, SnapshotPtrT<LiveBucket>>& historicalSnapshots) const
+    std::map<uint32_t, SnapshotPtrT<LiveBucket>>& historicalSnapshots,
+    bool forceUpdate) const
 {
     maybeUpdateSnapshotInternal(snapshot, historicalSnapshots,
-                                mCurrLiveSnapshot, mLiveHistoricalSnapshots);
+                                mCurrLiveSnapshot, mLiveHistoricalSnapshots,
+                                forceUpdate);
 }
 
 template <>
 void
 BucketSnapshotManager::maybeUpdateSnapshot<HotArchiveBucket>(
     SnapshotPtrT<HotArchiveBucket>& snapshot,
-    std::map<uint32_t, SnapshotPtrT<HotArchiveBucket>>& historicalSnapshots)
-    const
+    std::map<uint32_t, SnapshotPtrT<HotArchiveBucket>>& historicalSnapshots,
+    bool forceUpdate) const
 {
     maybeUpdateSnapshotInternal(snapshot, historicalSnapshots,
                                 mCurrHotArchiveSnapshot,
-                                mHotArchiveHistoricalSnapshots);
+                                mHotArchiveHistoricalSnapshots, forceUpdate);
 }
 
 template <class BucketT>
@@ -122,8 +124,8 @@ BucketSnapshotManager::maybeUpdateSnapshotInternal(
     SnapshotPtrT<BucketT>& snapshot,
     std::map<uint32_t, SnapshotPtrT<BucketT>>& historicalSnapshots,
     SnapshotPtrT<BucketT> const& managerSnapshot,
-    std::map<uint32_t, SnapshotPtrT<BucketT>> const& managerHistoricalSnapshots)
-    const
+    std::map<uint32_t, SnapshotPtrT<BucketT>> const& managerHistoricalSnapshots,
+    bool forceUpdate) const
 {
     BUCKET_TYPE_ASSERT(BucketT);
 
@@ -134,11 +136,14 @@ BucketSnapshotManager::maybeUpdateSnapshotInternal(
 
     // First update current snapshot
     if (!snapshot ||
-        snapshot->getLedgerSeq() != managerSnapshot->getLedgerSeq())
+        snapshot->getLedgerSeq() != managerSnapshot->getLedgerSeq() ||
+        forceUpdate)
     {
         // Should only update with a newer snapshot
-        releaseAssert(!snapshot || snapshot->getLedgerSeq() <
-                                       managerSnapshot->getLedgerSeq());
+        releaseAssert(!snapshot ||
+                      snapshot->getLedgerSeq() <
+                          managerSnapshot->getLedgerSeq() ||
+                      forceUpdate);
         snapshot = std::make_unique<BucketListSnapshot<BucketT> const>(
             *managerSnapshot);
     }

src/bucket/BucketSnapshotManager.h

@@ -72,7 +72,8 @@ class BucketSnapshotManager : NonMovableOrCopyable
         std::map<uint32_t, SnapshotPtrT<BucketT>>& historicalSnapshots,
         SnapshotPtrT<BucketT> const& managerSnapshot,
         std::map<uint32_t, SnapshotPtrT<BucketT>> const&
-            managerHistoricalSnapshots) const;
+            managerHistoricalSnapshots,
+        bool forceUpdate = false) const;
 
   public:
     // Called by main thread to update snapshots whenever the BucketList
@@ -98,7 +99,8 @@ class BucketSnapshotManager : NonMovableOrCopyable
     template <class BucketT>
     void maybeUpdateSnapshot(
         SnapshotPtrT<BucketT>& snapshot,
-        std::map<uint32_t, SnapshotPtrT<BucketT>>& historicalSnapshots) const;
+        std::map<uint32_t, SnapshotPtrT<BucketT>>& historicalSnapshots,
+        bool forceUpdate = false) const;
 
     // All metric recording functions must only be called by the main thread
     void startPointLoadTimer() const;

src/bucket/SearchableBucketList.cpp

@@ -64,7 +64,7 @@ SearchableLiveBucketListSnapshot::scanForEviction(
 void
 SearchableLiveBucketListSnapshot::updateSnapshotToLatest()
 {
-    mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+    mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots, true);
 }
 
 template <class BucketT>

src/catchup/CatchupManagerImpl.cpp

@@ -439,8 +439,9 @@ void
 CatchupManagerImpl::tryApplySyncingLedgers()
 {
     ZoneScoped;
-    auto const& ledgerHeader =
-        mApp.getLedgerManager().getLastClosedLedgerHeader();
+    auto getLcl = [&]() {
+        return mApp.getLedgerManager().getLastClosedLedgerHeader();
+    };
 
     // We can apply multiple ledgers here, which might be slow. This is a rare
     // occurrence so we should be fine.
@@ -450,14 +451,14 @@ CatchupManagerImpl::tryApplySyncingLedgers()
         auto const& lcd = it->second;
 
         // we still have a missing ledger
-        if (ledgerHeader.header.ledgerSeq + 1 != lcd.getLedgerSeq())
+        if (getLcl().header.ledgerSeq + 1 != lcd.getLedgerSeq())
         {
             break;
         }
 
         mApp.getLedgerManager().closeLedger(lcd);
         CLOG_INFO(History, "Closed buffered ledger: {}",
-                  LedgerManager::ledgerAbbrev(ledgerHeader));
+                  LedgerManager::ledgerAbbrev(getLcl()));
 
         ++it;
     }

src/herder/TxSetFrame.cpp

@@ -875,7 +875,7 @@ ApplicableTxSetFrame::checkValid(Application& app,
                                  uint64_t upperBoundCloseTimeOffset) const
 {
     ZoneScoped;
-    auto& lcl = app.getLedgerManager().getLastClosedLedgerHeader();
+    auto const& lcl = app.getLedgerManager().getLastClosedLedgerHeader();
 
     // Start by checking previousLedgerHash
     if (lcl.hash != mPreviousLedgerHash)

src/herder/test/HerderTests.cpp

@@ -1870,7 +1870,9 @@ testSCPDriver(uint32 protocolVersion, uint32_t maxTxSetSize, size_t expectedOps)
 
     Application::pointer app = createTestApplication(clock, cfg);
 
-    auto const& lcl = app->getLedgerManager().getLastClosedLedgerHeader();
+    auto getLcl = [&]() {
+        return app->getLedgerManager().getLastClosedLedgerHeader();
+    };
 
     auto root = TestAccount::createRoot(*app);
     std::vector<TestAccount> accounts;
@@ -1958,7 +1960,7 @@ testSCPDriver(uint32 protocolVersion, uint32_t maxTxSetSize, size_t expectedOps)
         std::vector<size_t> txSetSizes;
         std::vector<size_t> txSetOpSizes;
         std::vector<TimePoint> closeTimes;
-        std::vector<decltype(lcl.header.baseFee)> baseFees;
+        std::vector<decltype(getLcl().header.baseFee)> baseFees;
 
         auto addCandidateThenTest = [&](CandidateSpec const& spec) {
             // Create a transaction set using the given parameters, combine
@@ -1974,13 +1976,13 @@ testSCPDriver(uint32 protocolVersion, uint32_t maxTxSetSize, size_t expectedOps)
             auto [txSet, applicableTxSet] =
                 makeTransactions(spec.n, spec.nbOps, spec.feeMulti);
             txSetHashes.push_back(txSet->getContentsHash());
-            txSetSizes.push_back(applicableTxSet->size(lcl.header));
+            txSetSizes.push_back(applicableTxSet->size(getLcl().header));
             txSetOpSizes.push_back(applicableTxSet->sizeOpTotal());
             closeTimes.push_back(spec.closeTime);
             if (spec.baseFeeIncrement)
             {
                 auto const baseFee =
-                    lcl.header.baseFee + *spec.baseFeeIncrement;
+                    getLcl().header.baseFee + *spec.baseFeeIncrement;
                 baseFees.push_back(baseFee);
                 LedgerUpgrade ledgerUpgrade;
                 ledgerUpgrade.type(LEDGER_UPGRADE_BASE_FEE);
@@ -2143,13 +2145,13 @@ testSCPDriver(uint32 protocolVersion, uint32_t maxTxSetSize, size_t expectedOps)
         auto& herder = static_cast<HerderImpl&>(app->getHerder());
         auto& scp = herder.getHerderSCPDriver();
 
-        auto const lclCloseTime = lcl.header.scpValue.closeTime;
+        auto const lclCloseTime = getLcl().header.scpValue.closeTime;
 
         auto testTxBounds = [&](TimePoint const minTime,
                                 TimePoint const maxTime,
                                 TimePoint const nextCloseTime,
                                 bool const expectValid) {
-            REQUIRE(nextCloseTime > lcl.header.scpValue.closeTime);
+            REQUIRE(nextCloseTime > getLcl().header.scpValue.closeTime);
             // Build a transaction set containing one transaction (which
             // could be any transaction that is valid in all ways aside from
             // its time bounds) with the given minTime and maxTime.
@@ -4215,8 +4217,8 @@ static void
 externalize(SecretKey const& sk, LedgerManager& lm, HerderImpl& herder,
             std::vector<TransactionFrameBasePtr> const& txs, Application& app)
 {
-    auto const& lcl = lm.getLastClosedLedgerHeader();
-    auto ledgerSeq = lcl.header.ledgerSeq + 1;
+    auto getLcl = [&]() { return lm.getLastClosedLedgerHeader(); };
+    auto ledgerSeq = getLcl().header.ledgerSeq + 1;
 
     auto classicTxs = txs;
 
@@ -4243,7 +4245,7 @@ externalize(SecretKey const& sk, LedgerManager& lm, HerderImpl& herder,
     herder.getPendingEnvelopes().putTxSet(txSet->getContentsHash(), ledgerSeq,
                                           txSet);
 
-    auto lastCloseTime = lcl.header.scpValue.closeTime;
+    auto lastCloseTime = getLcl().header.scpValue.closeTime;
 
     StellarValue sv =
         herder.makeStellarValue(txSet->getContentsHash(), lastCloseTime,

src/herder/test/TxSetTests.cpp

@@ -613,10 +613,11 @@ TEST_CASE("generalized tx set XDR conversion", "[txset]")
     }
     SECTION("built from transactions")
     {
-        auto const& lclHeader =
-            app->getLedgerManager().getLastClosedLedgerHeader();
+        auto getLclHeader = [&]() {
+            return app->getLedgerManager().getLastClosedLedgerHeader();
+        };
         std::vector<TransactionFrameBasePtr> txs =
-            createTxs(5, lclHeader.header.baseFee, /* isSoroban */ false);
+            createTxs(5, getLclHeader().header.baseFee, /* isSoroban */ false);
         std::vector<TransactionFrameBasePtr> sorobanTxs =
             createTxs(5, 10'000'000, /* isSoroban */ true);
 
@@ -631,7 +632,7 @@ TEST_CASE("generalized tx set XDR conversion", "[txset]")
                          .phases[0]
                          .v0Components()[0]
                          .txsMaybeDiscountedFee()
-                         .baseFee == lclHeader.header.baseFee);
+                         .baseFee == getLclHeader().header.baseFee);
             REQUIRE(txSetXdr.v1TxSet()
                         .phases[0]
                         .v0Components()[0]
@@ -658,7 +659,7 @@ TEST_CASE("generalized tx set XDR conversion", "[txset]")
                         REQUIRE(phase.v0Components().size() == 1);
                         REQUIRE(*phase.v0Components()[0]
                                      .txsMaybeDiscountedFee()
-                                     .baseFee == lclHeader.header.baseFee);
+                                     .baseFee == getLclHeader().header.baseFee);
                         REQUIRE(phase.v0Components()[0]
                                     .txsMaybeDiscountedFee()
                                     .txs.size() == 5);
@@ -684,7 +685,7 @@ TEST_CASE("generalized tx set XDR conversion", "[txset]")
                     {
                         auto const& phase = txSetXdr.v1TxSet().phases[i];
                         auto expectedBaseFee =
-                            i == 0 ? lclHeader.header.baseFee
+                            i == 0 ? getLclHeader().header.baseFee
                                    : higherFeeSorobanTxs[0]->getInclusionFee();
                         REQUIRE(phase.v0Components().size() == 1);
                         REQUIRE(*phase.v0Components()[0]