index: prevent race by calling 'CustomInit' prior setting 'synced' flag

The 'm_synced' flag enables 'BlockConnected' events to be processed by
the index. If we set the flag before calling 'CustomInit', we could be
dispatching a block connected event to an uninitialized index child
class.

e.g. BlockFilterIndex, initializes the next filter position
inside 'CustomInit'. So, if `CustomInit` is not called prior receiving
the block event, the index will use 'next_filter_position=0' which
overwrites the first filter in disk.

Author: furszy

src/index/base.cpp

@@ -103,23 +103,20 @@ bool BaseIndex::Init()
         SetBestBlockIndex(locator_index);
     }
 
-    // Note: this will latch to true immediately if the user starts up with an empty
-    // datadir and an index enabled. If this is the case, indexation will happen solely
-    // via `BlockConnected` signals until, possibly, the next restart.
-    m_synced = m_best_block_index.load() == active_chain.Tip();
-
     // Skip pruning check if indexes are not ready to sync (because reindex-chainstate has wiped the chain).
-    if (!m_synced && g_indexes_ready_to_sync) {
+    const CBlockIndex* start_block = m_best_block_index.load();
+    bool synced = start_block == active_chain.Tip();
+    if (!synced && g_indexes_ready_to_sync) {
         bool prune_violation = false;
-        if (!m_best_block_index) {
+        if (!start_block) {
             // index is not built yet
             // make sure we have all block data back to the genesis
             prune_violation = m_chainstate->m_blockman.GetFirstStoredBlock(*active_chain.Tip()) != active_chain.Genesis();
         }
         // in case the index has a best block set and is not fully synced
         // check if we have the required blocks to continue building the index
         else {
-            const CBlockIndex* block_to_test = m_best_block_index.load();
+            const CBlockIndex* block_to_test = start_block;
             if (!active_chain.Contains(block_to_test)) {
                 // if the bestblock is not part of the mainchain, find the fork
                 // and make sure we have all data down to the fork
@@ -143,6 +140,16 @@ bool BaseIndex::Init()
             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()));
         }
     }
+
+    // Child init
+    if (!CustomInit(start_block ? std::make_optional(interfaces::BlockKey{start_block->GetBlockHash(), start_block->nHeight}) : std::nullopt)) {
+        return false;
+    }
+
+    // Note: this will latch to true immediately if the user starts up with an empty
+    // datadir and an index enabled. If this is the case, indexation will happen solely
+    // via `BlockConnected` signals until, possibly, the next restart.
+    m_synced = synced;
     return true;
 }
 
@@ -408,11 +415,6 @@ bool BaseIndex::Start()
     RegisterValidationInterface(this);
     if (!Init()) return false;
 
-    const CBlockIndex* index = m_best_block_index.load();
-    if (!CustomInit(index ? std::make_optional(interfaces::BlockKey{index->GetBlockHash(), index->nHeight}) : std::nullopt)) {
-        return false;
-    }
-
     m_thread_sync = std::thread(&util::TraceThread, GetName(), [this] { ThreadSync(); });
     return true;
 }