Index: Use prune locks for blockfilterindex

Prior to this change blocks could be pruned up to the last block before the blockfilterindex current best block.

Author: fjahr

src/index/base.cpp

@@ -65,9 +65,9 @@ bool BaseIndex::Init()
     LOCK(cs_main);
     CChain& active_chain = m_chainstate->m_chain;
     if (locator.IsNull()) {
-        m_best_block_index = nullptr;
+        SetBestBlockIndex(nullptr);
     } else {
-        m_best_block_index = m_chainstate->FindForkInGlobalIndex(locator);
+        SetBestBlockIndex(m_chainstate->FindForkInGlobalIndex(locator));
     }
     m_synced = m_best_block_index.load() == active_chain.Tip();
     if (!m_synced) {
@@ -134,7 +134,7 @@ void BaseIndex::ThreadSync()
         int64_t last_locator_write_time = 0;
         while (true) {
             if (m_interrupt) {
-                m_best_block_index = pindex;
+                SetBestBlockIndex(pindex);
                 // No need to handle errors in Commit. If it fails, the error will be already be
                 // logged. The best way to recover is to continue, as index cannot be corrupted by
                 // a missed commit to disk for an advanced index state.
@@ -146,7 +146,7 @@ void BaseIndex::ThreadSync()
                 LOCK(cs_main);
                 const CBlockIndex* pindex_next = NextSyncBlock(pindex, m_chainstate->m_chain);
                 if (!pindex_next) {
-                    m_best_block_index = pindex;
+                    SetBestBlockIndex(pindex);
                     m_synced = true;
                     // No need to handle errors in Commit. See rationale above.
                     Commit();
@@ -168,7 +168,7 @@ void BaseIndex::ThreadSync()
             }
 
             if (last_locator_write_time + SYNC_LOCATOR_WRITE_INTERVAL < current_time) {
-                m_best_block_index = pindex;
+                SetBestBlockIndex(pindex);
                 last_locator_write_time = current_time;
                 // No need to handle errors in Commit. See rationale above.
                 Commit();
@@ -226,10 +226,10 @@ bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_ti
     // out of sync may be possible but a users fault.
     // In case we reorg beyond the pruned depth, ReadBlockFromDisk would
     // throw and lead to a graceful shutdown
-    m_best_block_index = new_tip;
+    SetBestBlockIndex(new_tip);
     if (!Commit()) {
         // If commit fails, revert the best block index to avoid corruption.
-        m_best_block_index = current_tip;
+        SetBestBlockIndex(current_tip);
         return false;
     }
 
@@ -270,7 +270,7 @@ void BaseIndex::BlockConnected(const std::shared_ptr<const CBlock>& block, const
     }
 
     if (WriteBlock(*block, pindex)) {
-        m_best_block_index = pindex;
+        SetBestBlockIndex(pindex);
     } else {
         FatalError("%s: Failed to write block %s to index",
                    __func__, pindex->GetBlockHash().ToString());
@@ -377,3 +377,14 @@ IndexSummary BaseIndex::GetSummary() const
     summary.best_block_height = m_best_block_index ? m_best_block_index.load()->nHeight : 0;
     return summary;
 }
+
+void BaseIndex::SetBestBlockIndex(const CBlockIndex* block) {
+    assert(!node::fPruneMode || AllowPrune());
+
+    m_best_block_index = block;
+    if (AllowPrune() && block) {
+        node::PruneLockInfo prune_lock;
+        prune_lock.height_first = block->nHeight;
+        WITH_LOCK(::cs_main, m_chainstate->m_blockman.UpdatePruneLock(GetName(), prune_lock));
+    }
+}

src/index/base.h

@@ -75,6 +75,9 @@ class BaseIndex : public CValidationInterface
     /// to a chain reorganization), the index must halt until Commit succeeds or else it could end up
     /// getting corrupted.
     bool Commit();
+
+    virtual bool AllowPrune() const = 0;
+
 protected:
     CChainState* m_chainstate{nullptr};
 
@@ -103,6 +106,9 @@ class BaseIndex : public CValidationInterface
     /// Get the name of the index for display in logs.
     virtual const char* GetName() const = 0;
 
+    /// Update the internal best block index as well as the prune lock.
+    void SetBestBlockIndex(const CBlockIndex* block);
+
 public:
     /// Destructor interrupts sync thread if running and blocks until it exits.
     virtual ~BaseIndex();

src/index/blockfilterindex.h

@@ -38,6 +38,8 @@ class BlockFilterIndex final : public BaseIndex
     /** cache of block hash to filter header, to avoid disk access when responding to getcfcheckpt. */
     std::unordered_map<uint256, uint256, FilterHeaderHasher> m_headers_cache GUARDED_BY(m_cs_headers_cache);
 
+    bool AllowPrune() const override { return true; }
+
 protected:
     bool Init() override;
 

src/index/coinstatsindex.h

@@ -36,6 +36,8 @@ class CoinStatsIndex final : public BaseIndex
 
     bool ReverseBlock(const CBlock& block, const CBlockIndex* pindex);
 
+    bool AllowPrune() const override { return true; }
+
 protected:
     bool Init() override;
 

src/index/txindex.h

@@ -20,6 +20,8 @@ class TxIndex final : public BaseIndex
 private:
     const std::unique_ptr<DB> m_db;
 
+    bool AllowPrune() const override { return false; }
+
 protected:
     bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override;
 

src/validation.cpp

@@ -19,7 +19,6 @@
 #include <deploymentstatus.h>
 #include <flatfile.h>
 #include <hash.h>
-#include <index/blockfilterindex.h>
 #include <logging.h>
 #include <logging/timer.h>
 #include <node/blockstorage.h>
@@ -2349,10 +2348,6 @@ bool CChainState::FlushStateToDisk(
             int last_prune{m_chain.Height()}; // last height we can prune
             std::optional<std::string> limiting_lock; // prune lock that actually was the limiting factor, only used for logging
 
-            ForEachBlockFilterIndex([&](BlockFilterIndex& index) {
-                last_prune = std::max(1, std::min(last_prune, index.GetSummary().best_block_height));
-            });
-
             for (const auto& prune_lock : m_blockman.m_prune_locks) {
                 if (prune_lock.second.height_first == std::numeric_limits<int>::max()) continue;
                 // Remove the buffer and one additional block here to get actual height that is outside of the buffer

test/lint/lint-circular-dependencies.py

@@ -15,8 +15,6 @@
 EXPECTED_CIRCULAR_DEPENDENCIES = (
     "chainparamsbase -> util/system -> chainparamsbase",
     "node/blockstorage -> validation -> node/blockstorage",
-    "index/blockfilterindex -> node/blockstorage -> validation -> index/blockfilterindex",
-    "index/base -> validation -> index/blockfilterindex -> index/base",
     "index/coinstatsindex -> node/coinstats -> index/coinstatsindex",
     "policy/fees -> txmempool -> policy/fees",
     "qt/addresstablemodel -> qt/walletmodel -> qt/addresstablemodel",