From a1eee29a450e50c75e09507709d207a7bb6e0e8c Mon Sep 17 00:00:00 2001 From: Garand Tyson Date: Thu, 27 Jun 2024 14:19:26 -0700 Subject: [PATCH] BucketList consistency invariant for BucketListDBD --- src/catchup/AssumeStateWork.cpp | 5 + .../BucketListIsConsistentWithDatabase.cpp | 117 +++++++++++++++++- .../BucketListIsConsistentWithDatabase.h | 2 + src/invariant/Invariant.h | 6 + src/invariant/InvariantManager.h | 2 + src/invariant/InvariantManagerImpl.cpp | 19 +++ src/invariant/InvariantManagerImpl.h | 2 + src/invariant/test/InvariantTests.cpp | 6 + 8 files changed, 154 insertions(+), 5 deletions(-) diff --git a/src/catchup/AssumeStateWork.cpp b/src/catchup/AssumeStateWork.cpp index aa7e9a8e4f..e12ed8ac98 100644 --- a/src/catchup/AssumeStateWork.cpp +++ b/src/catchup/AssumeStateWork.cpp @@ -8,6 +8,7 @@ #include "catchup/IndexBucketsWork.h" #include "crypto/Hex.h" #include "history/HistoryArchive.h" +#include "invariant/InvariantManager.h" #include "work/WorkSequence.h" #include "work/WorkWithCallback.h" @@ -79,6 +80,10 @@ AssumeStateWork::doWork() // Drop bucket references once assume state complete since buckets // now referenced by BucketList buckets.clear(); + + // Check invariants after state has been assumed + app.getInvariantManager().checkAfterAssumeState(has.currentLedger); + return true; }; auto work = std::make_shared(mApp, "assume-state", diff --git a/src/invariant/BucketListIsConsistentWithDatabase.cpp b/src/invariant/BucketListIsConsistentWithDatabase.cpp index 142a3f6b73..e12da7b724 100644 --- a/src/invariant/BucketListIsConsistentWithDatabase.cpp +++ b/src/invariant/BucketListIsConsistentWithDatabase.cpp @@ -5,6 +5,7 @@ #include "invariant/BucketListIsConsistentWithDatabase.h" #include "bucket/Bucket.h" #include "bucket/BucketInputIterator.h" +#include "bucket/BucketList.h" #include "bucket/BucketManager.h" #include "crypto/Hex.h" #include "history/HistoryArchive.h" @@ -258,6 +259,96 @@ BucketListIsConsistentWithDatabase::checkEntireBucketlist() } } +std::string +BucketListIsConsistentWithDatabase::checkAfterAssumeState(uint32_t newestLedger) +{ + // If BucketListDB is disabled, we've already enforced the invariant on a + // per-Bucket level + if (!mApp.getConfig().isUsingBucketListDB()) + { + return {}; + } + + EntryCounts counts; + LedgerKeySet seenKeys; + + auto perBucketCheck = [&](auto bucket, auto& ltx) { + for (BucketInputIterator iter(bucket); iter; ++iter) + { + auto const& e = *iter; + + if (e.type() == LIVEENTRY || e.type() == INITENTRY) + { + if (e.liveEntry().data.type() != OFFER) + { + continue; + } + + // If this is the newest version of the key in the BucketList, + // check against the db + auto key = LedgerEntryKey(e.liveEntry()); + auto [_, newKey] = seenKeys.emplace(key); + if (newKey) + { + counts.countLiveEntry(e.liveEntry()); + + auto s = checkAgainstDatabase(ltx, e.liveEntry()); + if (!s.empty()) + { + return s; + } + } + } + else if (e.type() == DEADENTRY) + { + if (e.deadEntry().type() != OFFER) + { + continue; + } + + // If this is the newest version of the key in the BucketList, + // check against the db + auto [_, newKey] = seenKeys.emplace(e.deadEntry()); + if (newKey) + { + auto s = checkAgainstDatabase(ltx, e.deadEntry()); + if (!s.empty()) + { + return s; + } + } + } + } + + return std::string{}; + }; + + { + LedgerTxn ltx(mApp.getLedgerTxnRoot()); + auto& bl = mApp.getBucketManager().getBucketList(); + + for (uint32_t i = 0; i < BucketList::kNumLevels; ++i) + { + auto const& level = bl.getLevel(i); + for (auto const& bucket : {level.getCurr(), level.getSnap()}) + { + auto s = perBucketCheck(bucket, ltx); + if (!s.empty()) + { + return s; + } + } + } + } + + auto range = + LedgerRange::inclusive(LedgerManager::GENESIS_LEDGER_SEQ, newestLedger); + + // SQL only stores offers when BucketListDB is enabled + return counts.checkDbEntryCounts( + mApp, range, [](LedgerEntryType let) { return let == OFFER; }); +} + std::string BucketListIsConsistentWithDatabase::checkOnBucketApply( std::shared_ptr bucket, uint32_t oldestLedger, @@ -306,16 +397,25 @@ BucketListIsConsistentWithDatabase::checkOnBucketApply( if (entryTypeFilter(e.liveEntry().data.type())) { counts.countLiveEntry(e.liveEntry()); - auto s = checkAgainstDatabase(ltx, e.liveEntry()); - if (!s.empty()) + + // BucketListDB is not compatible with per-Bucket database + // consistency checks + if (!mApp.getConfig().isUsingBucketListDB()) { - return s; + auto s = checkAgainstDatabase(ltx, e.liveEntry()); + if (!s.empty()) + { + return s; + } } } } else if (e.type() == DEADENTRY) { - if (entryTypeFilter(e.deadEntry().type())) + // BucketListDB is not compatible with per-Bucket database + // consistency checks + if (entryTypeFilter(e.deadEntry().type()) && + !mApp.getConfig().isUsingBucketListDB()) { auto s = checkAgainstDatabase(ltx, e.deadEntry()); if (!s.empty()) @@ -328,6 +428,13 @@ BucketListIsConsistentWithDatabase::checkOnBucketApply( } auto range = LedgerRange::inclusive(oldestLedger, newestLedger); - return counts.checkDbEntryCounts(mApp, range, entryTypeFilter); + + // BucketListDB not compatible with per-Bucket database consistency checks + if (!mApp.getConfig().isUsingBucketListDB()) + { + return counts.checkDbEntryCounts(mApp, range, entryTypeFilter); + } + + return std::string{}; } } diff --git a/src/invariant/BucketListIsConsistentWithDatabase.h b/src/invariant/BucketListIsConsistentWithDatabase.h index 508b582c68..b98253dbc9 100644 --- a/src/invariant/BucketListIsConsistentWithDatabase.h +++ b/src/invariant/BucketListIsConsistentWithDatabase.h @@ -38,6 +38,8 @@ class BucketListIsConsistentWithDatabase : public Invariant uint32_t newestLedger, std::function entryTypeFilter) override; + virtual std::string checkAfterAssumeState(uint32_t newestLedger) override; + // Secondary entrypoint to database-vs-bucket consistency checking, designed // to be run offline via self-check. Throws an exception on any error. void checkEntireBucketlist(); diff --git a/src/invariant/Invariant.h b/src/invariant/Invariant.h index 0c7a34d173..ddb235795d 100644 --- a/src/invariant/Invariant.h +++ b/src/invariant/Invariant.h @@ -50,6 +50,12 @@ class Invariant return std::string{}; } + virtual std::string + checkAfterAssumeState(uint32_t newestLedger) + { + return std::string{}; + } + virtual std::string checkOnOperationApply(Operation const& operation, OperationResult const& result, diff --git a/src/invariant/InvariantManager.h b/src/invariant/InvariantManager.h index 53cf80914e..361afc150a 100644 --- a/src/invariant/InvariantManager.h +++ b/src/invariant/InvariantManager.h @@ -40,6 +40,8 @@ class InvariantManager std::shared_ptr bucket, uint32_t ledger, uint32_t level, bool isCurr, std::function entryTypeFilter) = 0; + virtual void checkAfterAssumeState(uint32_t newestLedger) = 0; + virtual void checkOnOperationApply(Operation const& operation, OperationResult const& opres, LedgerTxnDelta const& ltxDelta) = 0; diff --git a/src/invariant/InvariantManagerImpl.cpp b/src/invariant/InvariantManagerImpl.cpp index b8d66bb22f..df0ca6f61a 100644 --- a/src/invariant/InvariantManagerImpl.cpp +++ b/src/invariant/InvariantManagerImpl.cpp @@ -98,6 +98,25 @@ InvariantManagerImpl::checkOnBucketApply( } } +void +InvariantManagerImpl::checkAfterAssumeState(uint32_t newestLedger) +{ + for (auto invariant : mEnabled) + { + auto result = invariant->checkAfterAssumeState(newestLedger); + if (result.empty()) + { + continue; + } + + auto message = fmt::format( + FMT_STRING( + R"(invariant "{}" does not hold after assume state: {})"), + invariant->getName(), result); + onInvariantFailure(invariant, message, 0); + } +} + void InvariantManagerImpl::checkOnOperationApply(Operation const& operation, OperationResult const& opres, diff --git a/src/invariant/InvariantManagerImpl.h b/src/invariant/InvariantManagerImpl.h index f47ba5945a..5e495bcf3c 100644 --- a/src/invariant/InvariantManagerImpl.h +++ b/src/invariant/InvariantManagerImpl.h @@ -46,6 +46,8 @@ class InvariantManagerImpl : public InvariantManager bool isCurr, std::function entryTypeFilter) override; + virtual void checkAfterAssumeState(uint32_t newestLedger) override; + virtual void registerInvariant(std::shared_ptr invariant) override; diff --git a/src/invariant/test/InvariantTests.cpp b/src/invariant/test/InvariantTests.cpp index c2983e15a3..020e94037d 100644 --- a/src/invariant/test/InvariantTests.cpp +++ b/src/invariant/test/InvariantTests.cpp @@ -61,6 +61,12 @@ class TestInvariant : public Invariant return mShouldFail ? "fail" : ""; } + virtual std::string + checkAfterAssumeState(uint32_t newestLedger) override + { + return mShouldFail ? "fail" : ""; + } + virtual std::string checkOnOperationApply(Operation const& operation, OperationResult const& result,