refactor: move index class members from protected to private

src/index/base.cpp

@@ -101,7 +101,7 @@ bool BaseIndex::Init()
             }
         }
         if (prune_violation) {
-            return InitError(strprintf(Untranslated("%s best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)"), GetName()));
+            return InitError(strprintf(Untranslated("%s best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)"), GetIndexName()));
         }
     }
     return true;
@@ -154,7 +154,7 @@ void BaseIndex::ThreadSync()
                 }
                 if (pindex_next->pprev != pindex && !Rewind(pindex, pindex_next->pprev)) {
                     FatalError("%s: Failed to rewind index %s to a previous chain tip",
-                               __func__, GetName());
+                               __func__, GetIndexName());
                     return;
                 }
                 pindex = pindex_next;
@@ -163,7 +163,7 @@ void BaseIndex::ThreadSync()
             int64_t current_time = GetTime();
             if (last_log_time + SYNC_LOG_INTERVAL < current_time) {
                 LogPrintf("Syncing %s with block chain from height %d\n",
-                          GetName(), pindex->nHeight);
+                          GetIndexName(), pindex->nHeight);
                 last_log_time = current_time;
             }
 
@@ -189,17 +189,17 @@ void BaseIndex::ThreadSync()
     }
 
     if (pindex) {
-        LogPrintf("%s is enabled at height %d\n", GetName(), pindex->nHeight);
+        LogPrintf("%s is enabled at height %d\n", GetIndexName(), pindex->nHeight);
     } else {
-        LogPrintf("%s is enabled\n", GetName());
+        LogPrintf("%s is enabled\n", GetIndexName());
     }
 }
 
 bool BaseIndex::Commit()
 {
     CDBBatch batch(GetDB());
     if (!CommitInternal(batch) || !GetDB().WriteBatch(batch)) {
-        return error("%s: Failed to commit latest %s state", __func__, GetName());
+        return error("%s: Failed to commit latest %s state", __func__, GetIndexName());
     }
     return true;
 }
@@ -264,7 +264,7 @@ void BaseIndex::BlockConnected(const std::shared_ptr<const CBlock>& block, const
         }
         if (best_block_index != pindex->pprev && !Rewind(best_block_index, pindex->pprev)) {
             FatalError("%s: Failed to rewind index %s to a previous chain tip",
-                       __func__, GetName());
+                       __func__, GetIndexName());
             return;
         }
     }
@@ -336,7 +336,7 @@ bool BaseIndex::BlockUntilSyncedToCurrentChain() const
         }
     }
 
-    LogPrintf("%s: %s is catching up on block notifications\n", __func__, GetName());
+    LogPrintf("%s: %s is catching up on block notifications\n", __func__, GetIndexName());
     SyncWithValidationInterfaceQueue();
     return true;
 }
@@ -356,7 +356,7 @@ bool BaseIndex::Start(CChainState& active_chainstate)
         return false;
     }
 
-    m_thread_sync = std::thread(&util::TraceThread, GetName(), [this] { ThreadSync(); });
+    m_thread_sync = std::thread(&util::TraceThread, GetIndexName(), [this] { ThreadSync(); });
     return true;
 }
 
@@ -372,19 +372,20 @@ void BaseIndex::Stop()
 IndexSummary BaseIndex::GetSummary() const
 {
     IndexSummary summary{};
-    summary.name = GetName();
+    summary.name = GetIndexName();
     summary.synced = m_synced;
     summary.best_block_height = m_best_block_index ? m_best_block_index.load()->nHeight : 0;
     return summary;
 }
 
-void BaseIndex::SetBestBlockIndex(const CBlockIndex* block) {
+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));
+        WITH_LOCK(::cs_main, m_chainstate->m_blockman.UpdatePruneLock(GetIndexName(), prune_lock));
     }
 }

src/index/base.h

@@ -26,14 +26,37 @@ struct IndexSummary {
  */
 class BaseIndex : public CValidationInterface
 {
+public:
+    /// Destructor interrupts sync thread if running and blocks until it exits.
+    virtual ~BaseIndex();
+
+    /// Blocks the current thread until the index is caught up to the current
+    /// state of the block chain. This only blocks if the index has gotten in
+    /// sync once and only needs to process blocks in the ValidationInterface
+    /// queue. If the index is catching up from far behind, this method does
+    /// not block and immediately returns false.
+    bool BlockUntilSyncedToCurrentChain() const LOCKS_EXCLUDED(::cs_main);
+
+    void Interrupt();
+
+    /// Start initializes the sync state and registers the instance as a
+    /// ValidationInterface so that it stays in sync with blockchain updates.
+    [[nodiscard]] bool Start(CChainState& active_chainstate);
+
+    /// Stops the instance from staying in sync with blockchain updates.
+    void Stop();
+
+    /// Get a summary of the index and its state.
+    IndexSummary GetSummary() const;
+
 protected:
     /**
      * The database stores a block locator of the chain the database is synced to
      * so that the index can efficiently determine the point it last stopped at.
      * A locator is used instead of a simple hash of the chain tip because blocks
      * and block index entries may not be flushed to disk until after this database
      * is updated.
-    */
+     */
     class DB : public CDBWrapper
     {
     public:
@@ -47,6 +70,14 @@ class BaseIndex : public CValidationInterface
         void WriteBestBlock(CDBBatch& batch, const CBlockLocator& locator);
     };
 
+    CChainState* m_chainstate{nullptr};
+
+    void BlockConnected(const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex) override;
+
+    void ChainStateFlushed(const CBlockLocator& locator) override;
+
+    const CBlockIndex* CurrentIndex() { return m_best_block_index.load(); };
+
 private:
     /// Whether the index is in sync with the main chain. The flag is flipped
     /// from false to true once, after which point this starts processing
@@ -76,23 +107,16 @@ class BaseIndex : public CValidationInterface
     /// getting corrupted.
     bool Commit();
 
-    virtual bool AllowPrune() const = 0;
+    /// Update the internal best block index as well as the prune lock.
+    void SetBestBlockIndex(const CBlockIndex* block);
 
 protected:
-    CChainState* m_chainstate{nullptr};
-
-    void BlockConnected(const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex) override;
-
-    void ChainStateFlushed(const CBlockLocator& locator) override;
-
-    const CBlockIndex* CurrentIndex() { return m_best_block_index.load(); };
+    // The following virtual functions can be overridden but the inherited base
+    // implementation is used by some of the child classes.
 
     /// Initialize internal state from the database and block index.
     [[nodiscard]] virtual bool Init();
 
-    /// Write update index entries for a newly connected block.
-    virtual bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) { return true; }
-
     /// Virtual method called internally by Commit that can be overridden to atomically
     /// commit more index state.
     virtual bool CommitInternal(CDBBatch& batch);
@@ -101,36 +125,19 @@ class BaseIndex : public CValidationInterface
     /// be an ancestor of the current best block.
     virtual bool Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip);
 
-    virtual DB& GetDB() const = 0;
-
-    /// 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();
-
-    /// Blocks the current thread until the index is caught up to the current
-    /// state of the block chain. This only blocks if the index has gotten in
-    /// sync once and only needs to process blocks in the ValidationInterface
-    /// queue. If the index is catching up from far behind, this method does
-    /// not block and immediately returns false.
-    bool BlockUntilSyncedToCurrentChain() const LOCKS_EXCLUDED(::cs_main);
+private:
+    // The following virtual functions are fully overridden, i.e. no inherited
+    // base implementation is used by the child classes.
 
-    void Interrupt();
+    virtual bool AllowPrune() const = 0;
 
-    /// Start initializes the sync state and registers the instance as a
-    /// ValidationInterface so that it stays in sync with blockchain updates.
-    [[nodiscard]] bool Start(CChainState& active_chainstate);
+    virtual DB& GetDB() const = 0;
 
-    /// Stops the instance from staying in sync with blockchain updates.
-    void Stop();
+    /// Write update index entries for a newly connected block.
+    virtual bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) { return true; }
 
-    /// Get a summary of the index and its state.
-    IndexSummary GetSummary() const;
+    /// Get the name of the index for display in logs.
+    virtual const char* GetIndexName() const = 0;
 };
 
 #endif // BITCOIN_INDEX_BASE_H

src/index/blockfilterindex.cpp

@@ -116,7 +116,7 @@ bool BlockFilterIndex::Init()
         // further corruption.
         if (m_db->Exists(DB_FILTER_POS)) {
             return error("%s: Cannot read current %s state; index may be corrupted",
-                         __func__, GetName());
+                         __func__, GetIndexName());
         }
 
         // If the DB_FILTER_POS is not set, then initialize to the first location.

src/index/blockfilterindex.h

@@ -27,6 +27,7 @@ class BlockFilterIndex final : public BaseIndex
     BlockFilterType m_filter_type;
     std::string m_name;
     std::unique_ptr<BaseIndex::DB> m_db;
+    BaseIndex::DB& GetDB() const override { return *m_db; }
 
     FlatFilePos m_next_filter_pos;
     std::unique_ptr<FlatFileSeq> m_filter_fileseq;
@@ -40,19 +41,18 @@ class BlockFilterIndex final : public BaseIndex
 
     bool AllowPrune() const override { return true; }
 
-protected:
+    /// Write update index entries for a newly connected block.
+    bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override;
+
+    /// Get the name of the index for display in logs.
+    const char* GetIndexName() const override { return m_name.c_str(); }
+
     bool Init() override;
 
     bool CommitInternal(CDBBatch& batch) override;
 
-    bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override;
-
     bool Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip) override;
 
-    BaseIndex::DB& GetDB() const override { return *m_db; }
-
-    const char* GetName() const override { return m_name.c_str(); }
-
 public:
     /** Constructs the index, which becomes available to be queried. */
     explicit BlockFilterIndex(BlockFilterType filter_type,

src/index/coinstatsindex.cpp

@@ -348,7 +348,7 @@ bool CoinStatsIndex::Init()
         // failure, and starting the index would cause further corruption.
         if (m_db->Exists(DB_MUHASH)) {
             return error("%s: Cannot read current %s state; index may be corrupted",
-                         __func__, GetName());
+                         __func__, GetIndexName());
         }
     }
 
@@ -360,14 +360,14 @@ bool CoinStatsIndex::Init()
         DBVal entry;
         if (!LookUpOne(*m_db, pindex, entry)) {
             return error("%s: Cannot read current %s state; index may be corrupted",
-                         __func__, GetName());
+                         __func__, GetIndexName());
         }
 
         uint256 out;
         m_muhash.Finalize(out);
         if (entry.muhash != out) {
             return error("%s: Cannot read current %s state; index may be corrupted",
-                         __func__, GetName());
+                         __func__, GetIndexName());
         }
 
         m_transaction_output_count = entry.transaction_output_count;

src/index/coinstatsindex.h

@@ -19,6 +19,7 @@ class CoinStatsIndex final : public BaseIndex
 private:
     std::string m_name;
     std::unique_ptr<BaseIndex::DB> m_db;
+    BaseIndex::DB& GetDB() const override { return *m_db; }
 
     MuHash3072 m_muhash;
     uint64_t m_transaction_output_count{0};
@@ -38,19 +39,18 @@ class CoinStatsIndex final : public BaseIndex
 
     bool AllowPrune() const override { return true; }
 
-protected:
+    /// Write update index entries for a newly connected block.
+    bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override;
+
+    /// Get the name of the index for display in logs.
+    const char* GetIndexName() const override { return "coinstatsindex"; }
+
     bool Init() override;
 
     bool CommitInternal(CDBBatch& batch) override;
 
-    bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override;
-
     bool Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip) override;
 
-    BaseIndex::DB& GetDB() const override { return *m_db; }
-
-    const char* GetName() const override { return "coinstatsindex"; }
-
 public:
     // Constructs the index, which becomes available to be queried.
     explicit CoinStatsIndex(size_t n_cache_size, bool f_memory = false, bool f_wipe = false);

src/index/txindex.h

@@ -14,20 +14,18 @@
  */
 class TxIndex final : public BaseIndex
 {
-protected:
-    class DB;
-
 private:
+    class DB;
     const std::unique_ptr<DB> m_db;
+    BaseIndex::DB& GetDB() const override;
 
     bool AllowPrune() const override { return false; }
 
-protected:
+    /// Write update index entries for a newly connected block.
     bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override;
 
-    BaseIndex::DB& GetDB() const override;
-
-    const char* GetName() const override { return "txindex"; }
+    /// Get the name of the index for display in logs.
+    const char* GetIndexName() const override { return "txindex"; }
 
 public:
     /// Constructs the index, which becomes available to be queried.

src/node/blockstorage.cpp

@@ -232,7 +232,8 @@ void BlockManager::FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPr
            nLastBlockWeCanPrune, count);
 }
 
-void BlockManager::UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) {
+void BlockManager::UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info)
+{
     AssertLockHeld(::cs_main);
     m_prune_locks[name] = lock_info;
 }