Merge bitcoin/bitcoin#21796: index: Avoid async shutdown on init error

faad68fcd4 index: Avoid async shutdown on init error (MarcoFalke)

Pull request description:

  An async shutdown during init is confusing when a simple boolean return value can be used for a synchronous shutdown.

  This also changes the error message on stderr from:

  ```
  Error: A fatal internal error occurred, see debug.log for details
  Error: A fatal internal error occurred, see debug.log for details
  ```

  To:

  ```
  Error: basic block filter index best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)

ACKs for top commit:
  laanwj:
    Code review ACK faad68fcd4

Tree-SHA512: 92dd895266d6d15a6b1a5c081c9b83f83d5c82e9bfceb3ea0664f48540812239e274c829ff0271c4a0afb6d6a8f67d89c5af20d719982ad62999a41ca0623274

src/index/base.cpp

@@ -98,9 +98,7 @@ bool BaseIndex::Init()
             }
         }
         if (prune_violation) {
-            // throw error and graceful shutdown if we can't build the index
+            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()));
-            FatalError("%s: %s best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)", __func__, GetName());
-            return false;
         }
     }
     return true;
@@ -339,17 +337,17 @@ void BaseIndex::Interrupt()
     m_interrupt();
 }
 
-void BaseIndex::Start()
+bool BaseIndex::Start()
 {
     // Need to register this ValidationInterface before running Init(), so that
     // callbacks are not missed if Init sets m_synced to true.
     RegisterValidationInterface(this);
     if (!Init()) {
-        FatalError("%s: %s failed to initialize", __func__, GetName());
+        return false;
-        return;
     }
 
     m_thread_sync = std::thread(&util::TraceThread, GetName(), [this] { ThreadSync(); });
+    return true;
 }
 
 void BaseIndex::Stop()

src/index/base.h

@@ -84,7 +84,7 @@ protected:
     const CBlockIndex* CurrentIndex() { return m_best_block_index.load(); };
 
     /// Initialize internal state from the database and block index.
-    virtual bool Init();
+    [[nodiscard]] virtual bool Init();
 
     /// Write update index entries for a newly connected block.
     virtual bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) { return true; }
@@ -117,7 +117,7 @@ public:
 
     /// Start initializes the sync state and registers the instance as a
     /// ValidationInterface so that it stays in sync with blockchain updates.
-    void Start();
+    [[nodiscard]] bool Start();
 
     /// Stops the instance from staying in sync with blockchain updates.
     void Stop();

src/init.cpp

@@ -1550,17 +1550,23 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     // ********************************************************* Step 8: start indexers
     if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) {
         g_txindex = std::make_unique<TxIndex>(nTxIndexCache, false, fReindex);
-        g_txindex->Start();
+        if (!g_txindex->Start()) {
+            return false;
+        }
     }
 
     for (const auto& filter_type : g_enabled_filter_types) {
         InitBlockFilterIndex(filter_type, filter_index_cache, false, fReindex);
-        GetBlockFilterIndex(filter_type)->Start();
+        if (!GetBlockFilterIndex(filter_type)->Start()) {
+            return false;
+        }
     }
 
     if (args.GetBoolArg("-coinstatsindex", DEFAULT_COINSTATSINDEX)) {
         g_coin_stats_index = std::make_unique<CoinStatsIndex>(/* cache size */ 0, false, fReindex);
-        g_coin_stats_index->Start();
+        if (!g_coin_stats_index->Start()) {
+            return false;
+        }
     }
 
     // ********************************************************* Step 9: load wallet

src/test/blockfilter_index_tests.cpp

@@ -131,7 +131,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup)
     // BlockUntilSyncedToCurrentChain should return false before index is started.
     BOOST_CHECK(!filter_index.BlockUntilSyncedToCurrentChain());
 
-    filter_index.Start();
+    BOOST_REQUIRE(filter_index.Start());
 
     // Allow filter index to catch up with the block index.
     constexpr int64_t timeout_ms = 10 * 1000;

src/test/coinstatsindex_tests.cpp

@@ -32,7 +32,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup)
     // is started.
     BOOST_CHECK(!coin_stats_index.BlockUntilSyncedToCurrentChain());
 
-    coin_stats_index.Start();
+    BOOST_REQUIRE(coin_stats_index.Start());
 
     // Allow the CoinStatsIndex to catch up with the block index that is syncing
     // in a background thread.

src/test/txindex_tests.cpp

@@ -27,7 +27,7 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup)
     // BlockUntilSyncedToCurrentChain should return false before txindex is started.
     BOOST_CHECK(!txindex.BlockUntilSyncedToCurrentChain());
 
-    txindex.Start();
+    BOOST_REQUIRE(txindex.Start());
 
     // Allow tx index to catch up with the block index.
     constexpr int64_t timeout_ms = 10 * 1000;

test/functional/feature_blockfilterindex_prune.py

@@ -54,11 +54,13 @@ class FeatureBlockfilterindexPruneTest(BitcoinTestFramework):
         self.stop_node(0)
 
         self.log.info("make sure we get an init error when starting the node again with block filters")
-        with self.nodes[0].assert_debug_log(["basic block filter index best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)"]):
+        self.nodes[0].assert_start_raises_init_error(
-            self.nodes[0].assert_start_raises_init_error(extra_args=["-fastprune", "-prune=1", "-blockfilterindex=1"])
+            extra_args=["-fastprune", "-prune=1", "-blockfilterindex=1"],
+            expected_msg="Error: basic block filter index best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)",
+        )
 
         self.log.info("make sure the node starts again with the -reindex arg")
-        self.start_node(0, extra_args = ["-fastprune", "-prune=1", "-blockfilterindex", "-reindex"])
+        self.start_node(0, extra_args=["-fastprune", "-prune=1", "-blockfilterindex", "-reindex"])
 
 
 if __name__ == '__main__':