Merge b1ba1b178f into merged_master (Bitcoin PR bitcoin/bitcoin#30132)

src/node/blockmanager_args.cpp

@@ -33,8 +33,6 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& args, BlockManager::Op
 
     if (auto value{args.GetBoolArg("-fastprune")}) opts.fast_prune = *value;
 
-    if (auto value{args.GetBoolArg("-reindex")}) opts.reindex = *value;
-
     return {};
 }
 } // namespace node

src/node/blockstorage.cpp

@@ -644,7 +644,7 @@ bool BlockManager::LoadBlockIndexDB(ChainstateManager& chainman, const std::opti
     // Check whether we need to continue reindexing
     bool fReindexing = false;
     m_block_tree_db->ReadReindexing(fReindexing);
-    if (fReindexing) m_reindexing = true;
+    if (fReindexing) m_blockfiles_indexed = false;
 
     return true;
 }
@@ -1277,7 +1277,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFile
     ImportingNow imp{chainman.m_blockman.m_importing};
 
     // -reindex
-    if (chainman.m_blockman.m_reindexing) {
+    if (!chainman.m_blockman.m_blockfiles_indexed) {
         int nFile = 0;
         // Map of disk positions for blocks with unknown parent (only used for reindex);
         // parent hash -> child disk position, multiple children can have the same parent.
@@ -1300,7 +1300,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFile
             nFile++;
         }
         WITH_LOCK(::cs_main, chainman.m_blockman.m_block_tree_db->WriteReindexing(false));
-        chainman.m_blockman.m_reindexing = false;
+        chainman.m_blockman.m_blockfiles_indexed = true;
         LogPrintf("Reindexing finished\n");
         // To avoid ending up in a situation without genesis block, re-try initializing (no-op if reindexing worked):
         chainman.ActiveChainstate().LoadGenesisBlock();

src/node/blockstorage.h

@@ -281,18 +281,18 @@ public:
     explicit BlockManager(const util::SignalInterrupt& interrupt, Options opts)
         : m_prune_mode{opts.prune_target > 0},
           m_opts{std::move(opts)},
-          m_interrupt{interrupt},
-          m_reindexing{m_opts.reindex} {};
+          m_interrupt{interrupt} {}
 
     const util::SignalInterrupt& m_interrupt;
     std::atomic<bool> m_importing{false};
 
     /**
-     * Tracks if a reindex is currently in progress. Set to true when a reindex
-     * is requested and false when reindexing completes. Its value is persisted
-     * in the BlockTreeDB across restarts.
+     * Whether all blockfiles have been added to the block tree database.
+     * Normally true, but set to false when a reindex is requested and the
+     * database is wiped. The value is persisted in the database across restarts
+     * and will be false until reindexing completes.
      */
-    std::atomic_bool m_reindexing;
+    std::atomic_bool m_blockfiles_indexed{true};
 
     BlockMap m_block_index GUARDED_BY(cs_main);
 
@@ -373,7 +373,7 @@ public:
     [[nodiscard]] uint64_t GetPruneTarget() const { return m_opts.prune_target; }
     static constexpr auto PRUNE_TARGET_MANUAL{std::numeric_limits<uint64_t>::max()};
 
-    [[nodiscard]] bool LoadingBlocks() const { return m_importing || m_reindexing; }
+    [[nodiscard]] bool LoadingBlocks() const { return m_importing || !m_blockfiles_indexed; }
 
     /** Calculate the amount of disk space the block & undo files currently use */
     uint64_t CalculateCurrentUsage();

src/node/chainstate.cpp

@@ -45,11 +45,12 @@ static ChainstateLoadResult CompleteChainstateInitialization(
         .path = chainman.m_options.datadir / "blocks" / "index",
         .cache_bytes = static_cast<size_t>(cache_sizes.block_tree_db),
         .memory_only = options.block_tree_db_in_memory,
-        .wipe_data = options.reindex,
+        .wipe_data = options.wipe_block_tree_db,
         .options = chainman.m_options.block_tree_db});
 
-    if (options.reindex) {
+    if (options.wipe_block_tree_db) {
         pblocktree->WriteReindexing(true);
+        chainman.m_blockman.m_blockfiles_indexed = false;
         //If we're reindexing in prune mode, wipe away unusable block files and all undo data files
         if (options.prune) {
             chainman.m_blockman.CleanupBlockRevFiles();
@@ -60,8 +61,7 @@ static ChainstateLoadResult CompleteChainstateInitialization(
 
     // LoadBlockIndex will load m_have_pruned if we've ever removed a
     // block file from disk.
-    // Note that it also sets m_reindexing based on the disk flag!
-    // From here on, m_reindexing and options.reindex values may be different!
+    // Note that it also sets m_blockfiles_indexed based on the disk flag!
     if (!chainman.LoadBlockIndex()) {
         if (chainman.m_interrupt) return {ChainstateLoadStatus::INTERRUPTED, {}};
         return {ChainstateLoadStatus::FAILURE, _("Error loading block database")};
@@ -84,12 +84,12 @@ static ChainstateLoadResult CompleteChainstateInitialization(
     // If we're not mid-reindex (based on disk + args), add a genesis block on disk
     // (otherwise we use the one already on disk).
     // This is called again in ImportBlocks after the reindex completes.
-    if (!chainman.m_blockman.m_reindexing && !chainman.ActiveChainstate().LoadGenesisBlock()) {
+    if (chainman.m_blockman.m_blockfiles_indexed && !chainman.ActiveChainstate().LoadGenesisBlock()) {
         return {ChainstateLoadStatus::FAILURE, _("Error initializing block database")};
     }
 
     auto is_coinsview_empty = [&](Chainstate* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
-        return options.reindex || options.reindex_chainstate || chainstate->CoinsTip().GetBestBlock().IsNull();
+        return options.wipe_chainstate_db || chainstate->CoinsTip().GetBestBlock().IsNull();
     };
 
     assert(chainman.m_total_coinstip_cache > 0);
@@ -110,7 +110,7 @@ static ChainstateLoadResult CompleteChainstateInitialization(
         chainstate->InitCoinsDB(
             /*cache_size_bytes=*/chainman.m_total_coinsdb_cache * init_cache_fraction,
             /*in_memory=*/options.coins_db_in_memory,
-            /*should_wipe=*/options.reindex || options.reindex_chainstate);
+            /*should_wipe=*/options.wipe_chainstate_db);
 
         if (options.coins_error_cb) {
             chainstate->CoinsErrorCatcher().AddReadErrCallback(options.coins_error_cb);
@@ -142,7 +142,7 @@ static ChainstateLoadResult CompleteChainstateInitialization(
         }
     }
 
-    if (!options.reindex) {
+    if (!options.wipe_block_tree_db) {
         auto chainstates{chainman.GetAll()};
         if (std::any_of(chainstates.begin(), chainstates.end(),
                         [](const Chainstate* cs) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return cs->NeedsRedownload(); })) {
@@ -188,7 +188,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
     // Load a chain created from a UTXO snapshot, if any exist.
     bool has_snapshot = chainman.DetectSnapshotChainstate();
 
-    if (has_snapshot && (options.reindex || options.reindex_chainstate)) {
+    if (has_snapshot && options.wipe_chainstate_db) {
         LogPrintf("[snapshot] deleting snapshot chainstate due to reindexing\n");
         if (!chainman.DeleteSnapshotChainstate()) {
             return {ChainstateLoadStatus::FAILURE_FATAL, Untranslated("Couldn't remove snapshot chainstate.")};
@@ -247,7 +247,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
 ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const ChainstateLoadOptions& options)
 {
     auto is_coinsview_empty = [&](Chainstate* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
-        return options.reindex || options.reindex_chainstate || chainstate->CoinsTip().GetBestBlock().IsNull();
+        return options.wipe_chainstate_db || chainstate->CoinsTip().GetBestBlock().IsNull();
     };
 
     LOCK(cs_main);

src/node/chainstate.h

@@ -22,8 +22,13 @@ struct ChainstateLoadOptions {
     CTxMemPool* mempool{nullptr};
     bool block_tree_db_in_memory{false};
     bool coins_db_in_memory{false};
-    bool reindex{false};
-    bool reindex_chainstate{false};
+    // Whether to wipe the block tree database when loading it. If set, this
+    // will also set a reindexing flag so any existing block data files will be
+    // scanned and added to the database.
+    bool wipe_block_tree_db{false};
+    // Whether to wipe the chainstate database when loading it. If set, this
+    // will cause the chainstate database to be rebuilt starting from genesis.
+    bool wipe_chainstate_db{false};
     bool prune{false};
     //! Setting require_full_verification to true will require all checks at
     //! check_level (below) to succeed for loading to succeed. Setting it to