Merge bitcoin/bitcoin#24117: index: make indices robust against init aborts

bfcd60f5d505334230013de4115483b22a7898ee test: activate all index types in feature_init.py (Martin Zumsande)
0243907faee0aa6af09974131d9a46a7f9c3ef38 index: Don't commit without valid m_best_block_index (Martin Zumsande)

Pull request description:

  When an index thread receives an interrupt during init before it got to index anything (so `m_best_block_index == nullptr` still), it will still try to commit previous "work" before stopping the thread. That means that `BaseIndex::CommitInternal()` calls `GetLocator(nullptr)`, which returns an locator to the tip ([code](06b6369766/src/chain.cpp (L31-L32))), and saves it to the index DB.
  On the next startup, this locator will be read and it will be assumed that we have successfully synced the index to the tip, when in reality we have indexed nothing.
  In the case of coinstatsindex, this would lead to a shutdown of bitcoind without any indication what went wrong. For the other indexes, there would be no immediate shutdown, but the index would be corrupt.

  This PR fixes this by not committing when `m_best_block_index==nullptr`, and it also adds an error log message to the silent coinstatsindex shutdown path.

  This is another small bug found by `feature_init.py` - the second commit enables blockfilterindex and coinstatsindex for this test, enabling coinstatsindex without the first commit would have led to frequent failures.

ACKs for top commit:
  fjahr:
    reACK bfcd60f5d505334230013de4115483b22a7898ee
  shaavan:
    reACK bfcd60f5d505334230013de4115483b22a7898ee

Tree-SHA512: 8e2bac0fc40cde209518a9e59b597ae0a5a875a2a90898673987c91733718d40e528dada942bf552b58bc021bf46e59da2d0cc5a61045f48f9bae2b1baf6033b

src/index/base.cpp

@@ -211,6 +211,11 @@ bool BaseIndex::Commit()
 bool BaseIndex::CommitInternal(CDBBatch& batch)
 {
     LOCK(cs_main);
+    // Don't commit anything if we haven't indexed any block yet
+    // (this could happen if init is interrupted).
+    if (m_best_block_index == nullptr) {
+        return false;
+    }
     GetDB().WriteBestBlock(batch, m_chainstate->m_chain.GetLocator(m_best_block_index));
     return true;
 }

src/index/coinstatsindex.cpp

@@ -360,9 +360,9 @@ bool CoinStatsIndex::Init()
     if (pindex) {
         DBVal entry;
         if (!LookUpOne(*m_db, pindex, entry)) {
-            return false;
+            return error("%s: Cannot read current %s state; index may be corrupted",
+                         __func__, GetName());
         }
-
         m_transaction_output_count = entry.transaction_output_count;
         m_bogo_size = entry.bogo_size;
         m_total_amount = entry.total_amount;

test/functional/feature_init.py

@@ -64,6 +64,8 @@ class InitStressTest(BitcoinTestFramework):
             'addcon thread start',
             'loadblk thread start',
             'txindex thread start',
+            'block filter index thread start',
+            'coinstatsindex thread start',
             'msghand thread start',
             'net thread start',
             'addcon thread start',
@@ -74,7 +76,7 @@ class InitStressTest(BitcoinTestFramework):
         for terminate_line in lines_to_terminate_after:
             self.log.info(f"Starting node and will exit after line '{terminate_line}'")
             with node.wait_for_debug_log([terminate_line], ignore_case=True):
-                node.start(extra_args=['-txindex=1'])
+                node.start(extra_args=['-txindex=1', '-blockfilterindex=1', '-coinstatsindex=1'])
             self.log.debug("Terminating node after terminate line was found")
             sigterm_node()
 
@@ -109,7 +111,7 @@ class InitStressTest(BitcoinTestFramework):
             # investigate doing this later.
 
             node.assert_start_raises_init_error(
-                extra_args=['-txindex=1'],
+                extra_args=['-txindex=1', '-blockfilterindex=1', '-coinstatsindex=1'],
                 expected_msg=err_fragment,
                 match=ErrorMatch.PARTIAL_REGEX,
             )