Merge pull request #3057 from MonsieurNicolas/hardenBucketApplyPath

Harden bucket apply path Reviewed-by: marta-lokhova
stellar · May 20, 2021 · fbc0325 · fbc0325
2 parents 2b01403 + 1e0f0d1
commit fbc0325
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 30 deletions.
diff --git a/src/catchup/ApplyBucketsWork.cpp b/src/catchup/ApplyBucketsWork.cpp
@@ -77,6 +77,10 @@ ApplyBucketsWork::onReset()
 
     if (!isAborting())
     {
+        // clear ledgerTxn state of all ledger entries in preparation of bucket
+        // apply
+        mApp.resetLedgerState();
+
         auto addBucket = [this](std::shared_ptr<Bucket const> const& bucket) {
             if (bucket->getSize() > 0)
             {
@@ -114,18 +118,6 @@ ApplyBucketsWork::startLevel()
     bool applySnap = (i.snap != binToHex(level.getSnap()->getHash()));
     bool applyCurr = (i.curr != binToHex(level.getCurr()->getHash()));
 
-    if (!mApplying && !mApp.getConfig().MODE_USES_IN_MEMORY_LEDGER &&
-        (applySnap || applyCurr))
-    {
-        uint32_t oldestLedger = applySnap
-                                    ? BucketList::oldestLedgerInSnap(
-                                          mApplyState.currentLedger, mLevel)
-                                    : BucketList::oldestLedgerInCurr(
-                                          mApplyState.currentLedger, mLevel);
-        auto& lsRoot = mApp.getLedgerTxnRoot();
-        lsRoot.deleteObjectsModifiedOnOrAfterLedger(oldestLedger);
-    }
-
     if (mApplying || applySnap)
     {
         mSnapBucket = getBucket(i.snap);

diff --git a/src/herder/TransactionQueue.cpp b/src/herder/TransactionQueue.cpp
@@ -740,9 +740,6 @@ TransactionQueue::getMaxOpsToFloodThisPeriod() const
         opsToFlood = opsToFloodLedger;
     }
     releaseAssertOrThrow(opsToFlood >= 0);
-    releaseAssertOrThrow(
-        opsToFlood <=
-        static_cast<int64_t>(std::numeric_limits<ssize_t>::max()));
     return static_cast<size_t>(opsToFlood);
 }
 

diff --git a/src/main/Application.h b/src/main/Application.h
@@ -164,6 +164,10 @@ class Application
 
     virtual void initialize(bool createNewDB) = 0;
 
+    // reset the ledger state entirely
+    // (to be used before applying buckets)
+    virtual void resetLedgerState() = 0;
+
     // Return the time in seconds since the POSIX epoch, according to the
     // VirtualClock this Application is bound to. Convenience method.
     virtual uint64_t timeNow() = 0;

diff --git a/src/main/ApplicationImpl.cpp b/src/main/ApplicationImpl.cpp
@@ -164,19 +164,9 @@ ApplicationImpl::initialize(bool createNewDB)
     mBanManager = BanManager::create(*this);
     mStatusManager = std::make_unique<StatusManager>();
 
-#ifdef BEST_OFFER_DEBUGGING
-    auto const bestOfferDebuggingEnabled = mConfig.BEST_OFFER_DEBUGGING_ENABLED;
-#endif
-
     if (getConfig().MODE_USES_IN_MEMORY_LEDGER)
     {
-        mLedgerTxnRoot = std::make_unique<InMemoryLedgerTxnRoot>(
-#ifdef BEST_OFFER_DEBUGGING
-            bestOfferDebuggingEnabled
-#endif
-        );
-        mNeverCommittingLedgerTxn =
-            std::make_unique<LedgerTxn>(*mLedgerTxnRoot);
+        resetLedgerState();
     }
     else
     {
@@ -191,7 +181,7 @@ ApplicationImpl::initialize(bool createNewDB)
             *mDatabase, mConfig.ENTRY_CACHE_SIZE, mConfig.PREFETCH_BATCH_SIZE
 #ifdef BEST_OFFER_DEBUGGING
             ,
-            bestOfferDebuggingEnabled
+            mConfig.BEST_OFFER_DEBUGGING_ENABLED
 #endif
         );
     }
@@ -220,6 +210,27 @@ ApplicationImpl::initialize(bool createNewDB)
     LOG_DEBUG(DEFAULT_LOG, "Application constructed");
 }
 
+void
+ApplicationImpl::resetLedgerState()
+{
+    if (getConfig().MODE_USES_IN_MEMORY_LEDGER)
+    {
+        mNeverCommittingLedgerTxn.reset();
+        mLedgerTxnRoot = std::make_unique<InMemoryLedgerTxnRoot>(
+#ifdef BEST_OFFER_DEBUGGING
+            mConfig.BEST_OFFER_DEBUGGING_ENABLED
+#endif
+        );
+        mNeverCommittingLedgerTxn =
+            std::make_unique<LedgerTxn>(*mLedgerTxnRoot);
+    }
+    else
+    {
+        auto& lsRoot = getLedgerTxnRoot();
+        lsRoot.deleteObjectsModifiedOnOrAfterLedger(0);
+    }
+}
+
 void
 ApplicationImpl::newDB()
 {

diff --git a/src/main/ApplicationImpl.h b/src/main/ApplicationImpl.h
@@ -44,6 +44,8 @@ class ApplicationImpl : public Application
 
     virtual void initialize(bool newDB) override;
 
+    virtual void resetLedgerState() override;
+
     virtual uint64_t timeNow() override;
 
     virtual Config const& getConfig() override;

diff --git a/src/main/CommandLine.cpp b/src/main/CommandLine.cpp
@@ -774,8 +774,7 @@ runCatchup(CommandLineArgs const& args)
                     LOG_INFO(
                         DEFAULT_LOG,
                         "Resetting ledger state to genesis before catching up");
-                    auto& lsRoot = app->getLedgerTxnRoot();
-                    lsRoot.deleteObjectsModifiedOnOrAfterLedger(0);
+                    app->resetLedgerState();
                     lm.startNewLedger();
                 }
 

diff --git a/src/main/Config.cpp b/src/main/Config.cpp
@@ -22,6 +22,7 @@
 #include <fmt/chrono.h>
 #include <fmt/format.h>
 #include <functional>
+#include <numeric>
 #include <sstream>
 #include <unordered_set>
 
@@ -733,7 +734,7 @@ Config::processOpApplySleepTimeForTestingConfigs()
     ret.reserve(100);
     for (size_t i = 0; i < OP_APPLY_SLEEP_TIME_WEIGHT_FOR_TESTING.size(); i++)
     {
-        LOG_INFO(DEFAULT_LOG, "Sleeps for {} {}\% of the time",
+        LOG_INFO(DEFAULT_LOG, "Sleeps for {} {}% of the time",
                  OP_APPLY_SLEEP_TIME_DURATION_FOR_TESTING[i],
                  OP_APPLY_SLEEP_TIME_WEIGHT_FOR_TESTING[i]);
         for (size_t j = 0; j < OP_APPLY_SLEEP_TIME_WEIGHT_FOR_TESTING[i]; j++)