kernel: Move block tree db open to block manager

Before this change the block tree db was needlessly re-opened during
startup when loading a completed snapshot. Improve this by letting the
block manager open it on construction. This also simplifies the test
code a bit.

The change was initially motivated to make it easier for users of the
kernel library to instantiate a BlockManager that may be used to read
data from disk without loading the block index into a cache.

Author: TheCharlatan

src/bitcoin-chainstate.cpp

@@ -106,6 +106,10 @@ int main(int argc, char* argv[])
     };
     auto notifications = std::make_unique<KernelNotifications>();
 
+    node::CacheSizes cache_sizes;
+    cache_sizes.block_tree_db = 2 << 20;
+    cache_sizes.coins_db = 2 << 22;
+    cache_sizes.coins = (450 << 20) - (2 << 20) - (2 << 22);
 
     // SETUP: Chainstate
     auto chainparams = CChainParams::Main();
@@ -119,14 +123,12 @@ int main(int argc, char* argv[])
         .chainparams = chainman_opts.chainparams,
         .blocks_dir = abs_datadir / "blocks",
         .notifications = chainman_opts.notifications,
+        .block_tree_db_dir = abs_datadir / "blocks" / "index",
+        .block_tree_db_cache_size = 2 << 20,
     };
     util::SignalInterrupt interrupt;
     ChainstateManager chainman{interrupt, chainman_opts, blockman_opts};
 
-    node::CacheSizes cache_sizes;
-    cache_sizes.block_tree_db = 2 << 20;
-    cache_sizes.coins_db = 2 << 22;
-    cache_sizes.coins = (450 << 20) - (2 << 20) - (2 << 22);
     node::ChainstateLoadOptions options;
     auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options);
     if (status != node::ChainstateLoadStatus::SUCCESS) {

src/init.cpp

@@ -3,6 +3,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include "dbwrapper.h"
 #include <config/bitcoin-config.h> // IWYU pragma: keep
 
 #include <init.h>
@@ -1069,6 +1070,8 @@ bool AppInitParameterInteraction(const ArgsManager& args)
             .chainparams = chainman_opts_dummy.chainparams,
             .blocks_dir = args.GetBlocksDirPath(),
             .notifications = chainman_opts_dummy.notifications,
+            .block_tree_db_dir = args.GetDataDirNet() / "blocks" / "index",
+            .block_tree_db_cache_size = 0,
         };
         auto blockman_result{ApplyArgsManOptions(args, blockman_opts_dummy)};
         if (!blockman_result) {
@@ -1217,10 +1220,15 @@ static ChainstateLoadResult InitAndLoadChainstate(
         .chainparams = chainman_opts.chainparams,
         .blocks_dir = args.GetBlocksDirPath(),
         .notifications = chainman_opts.notifications,
+        .block_tree_db_dir = args.GetDataDirNet() / "blocks" / "index",
+        .wipe_block_tree_db = do_reindex,
+        .block_tree_db_cache_size = cache_sizes.block_tree_db,
     };
     Assert(ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction
     try {
         node.chainman = std::make_unique<ChainstateManager>(*Assert(node.shutdown), chainman_opts, blockman_opts);
+    } catch (dbwrapper_error&) {
+        return {ChainstateLoadStatus::FAILURE, strprintf(Untranslated("Error opening block database."))};
     } catch (std::exception& e) {
         return {ChainstateLoadStatus::FAILURE_FATAL, strprintf(Untranslated("Failed to initialize ChainstateManager: %s"), e.what())};
     }
@@ -1247,7 +1255,6 @@ static ChainstateLoadResult InitAndLoadChainstate(
     };
     node::ChainstateLoadOptions options;
     options.mempool = Assert(node.mempool.get());
-    options.wipe_block_tree_db = do_reindex;
     options.wipe_chainstate_db = do_reindex || do_reindex_chainstate;
     options.prune = chainman.m_blockman.IsPruneMode();
     options.check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);

src/kernel/blockmanager_opts.h

@@ -5,6 +5,7 @@
 #ifndef BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H
 #define BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H
 
+#include <dbwrapper.h>
 #include <kernel/notifications_interface.h>
 #include <util/fs.h>
 
@@ -27,6 +28,14 @@ struct BlockManagerOpts {
     bool fast_prune{false};
     const fs::path blocks_dir;
     Notifications& notifications;
+    const fs::path block_tree_db_dir;
+    // 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};
+    bool block_tree_db_in_memory{false};
+    DBOptions block_tree_db_options{};
+    int64_t block_tree_db_cache_size;
 };
 
 } // namespace kernel

src/kernel/chainstatemanager_opts.h

@@ -42,7 +42,6 @@ struct ChainstateManagerOpts {
     std::optional<uint256> assumed_valid_block{};
     //! If the tip is older than this, the node is considered to be in initial block download.
     std::chrono::seconds max_tip_age{DEFAULT_MAX_TIP_AGE};
-    DBOptions block_tree_db{};
     DBOptions coins_db{};
     CoinsViewOptions coins_view{};
     Notifications& notifications;

src/node/blockmanager_args.cpp

@@ -6,6 +6,7 @@
 
 #include <common/args.h>
 #include <node/blockstorage.h>
+#include <node/database_args.h>
 #include <tinyformat.h>
 #include <util/result.h>
 #include <util/translation.h>
@@ -34,6 +35,8 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& args, BlockManager::Op
 
     if (auto value{args.GetBoolArg("-fastprune")}) opts.fast_prune = *value;
 
+    ReadDatabaseArgs(args, opts.block_tree_db_options);
+
     return {};
 }
 } // namespace node

src/node/blockstorage.cpp

@@ -36,6 +36,7 @@
 #include <util/translation.h>
 #include <validation.h>
 
+#include <cstddef>
 #include <map>
 #include <ranges>
 #include <unordered_map>
@@ -1180,10 +1181,27 @@ static auto InitBlocksdirXorKey(const BlockManager::Options& opts)
 BlockManager::BlockManager(const util::SignalInterrupt& interrupt, Options opts)
     : m_prune_mode{opts.prune_target > 0},
       m_xor_key{InitBlocksdirXorKey(opts)},
+      m_block_file_seq{FlatFileSeq{opts.blocks_dir, "blk", opts.fast_prune ? 0x4000 /* 16kB */ : BLOCKFILE_CHUNK_SIZE}},
+      m_undo_file_seq{FlatFileSeq{opts.blocks_dir, "rev", UNDOFILE_CHUNK_SIZE}},
       m_opts{std::move(opts)},
-      m_block_file_seq{FlatFileSeq{m_opts.blocks_dir, "blk", m_opts.fast_prune ? 0x4000 /* 16kB */ : BLOCKFILE_CHUNK_SIZE}},
-      m_undo_file_seq{FlatFileSeq{m_opts.blocks_dir, "rev", UNDOFILE_CHUNK_SIZE}},
-      m_interrupt{interrupt} {}
+      m_interrupt{interrupt}
+{
+    m_block_tree_db = std::make_unique<BlockTreeDB>(DBParams{
+        .path = m_opts.block_tree_db_dir,
+        .cache_bytes = static_cast<size_t>(m_opts.block_tree_db_cache_size),
+        .memory_only = m_opts.block_tree_db_in_memory,
+        .wipe_data = m_opts.wipe_block_tree_db,
+        .options = m_opts.block_tree_db_options});
+
+    if (m_opts.wipe_block_tree_db) {
+        m_block_tree_db->WriteReindexing(true);
+        m_blockfiles_indexed = false;
+        // If we're reindexing in prune mode, wipe away unusable block files and all undo data files
+        if (m_prune_mode) {
+            CleanupBlockRevFiles();
+        }
+    }
+}
 
 class ImportingNow
 {

src/node/blockstorage.h

@@ -259,8 +259,6 @@ class BlockManager
 
     BlockfileType BlockfileTypeForHeight(int height);
 
-    const kernel::BlockManagerOpts m_opts;
-
     const FlatFileSeq m_block_file_seq;
     const FlatFileSeq m_undo_file_seq;
 
@@ -269,6 +267,7 @@ class BlockManager
 
     explicit BlockManager(const util::SignalInterrupt& interrupt, Options opts);
 
+    const kernel::BlockManagerOpts m_opts;
     const util::SignalInterrupt& m_interrupt;
     std::atomic<bool> m_importing{false};
 

src/node/chainstate.cpp

@@ -37,26 +37,6 @@ static ChainstateLoadResult CompleteChainstateInitialization(
     const CacheSizes& cache_sizes,
     const ChainstateLoadOptions& options) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
 {
-    auto& pblocktree{chainman.m_blockman.m_block_tree_db};
-    // new BlockTreeDB tries to delete the existing file, which
-    // fails if it's still open from the previous loop. Close it first:
-    pblocktree.reset();
-    pblocktree = std::make_unique<BlockTreeDB>(DBParams{
-        .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.wipe_block_tree_db,
-        .options = chainman.m_options.block_tree_db});
-
-    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();
-        }
-    }
-
     if (chainman.m_interrupt) return {ChainstateLoadStatus::INTERRUPTED, {}};
 
     // LoadBlockIndex will load m_have_pruned if we've ever removed a
@@ -142,7 +122,7 @@ static ChainstateLoadResult CompleteChainstateInitialization(
         }
     }
 
-    if (!options.wipe_block_tree_db) {
+    if (!chainman.m_blockman.m_opts.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(); })) {

src/node/chainstate.h

@@ -20,12 +20,7 @@ struct CacheSizes;
 
 struct ChainstateLoadOptions {
     CTxMemPool* mempool{nullptr};
-    bool block_tree_db_in_memory{false};
     bool coins_db_in_memory{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};

src/node/chainstatemanager_args.cpp

@@ -49,7 +49,6 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManage
 
     if (auto value{args.GetIntArg("-maxtipage")}) opts.max_tip_age = std::chrono::seconds{*value};
 
-    ReadDatabaseArgs(args, opts.block_tree_db);
     ReadDatabaseArgs(args, opts.coins_db);
     ReadCoinsViewArgs(args, opts.coins_view);
 

src/test/blockmanager_tests.cpp

@@ -33,6 +33,8 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos)
         .chainparams = *params,
         .blocks_dir = m_args.GetBlocksDirPath(),
         .notifications = notifications,
+        .block_tree_db_dir = m_args.GetDataDirNet() / "blocks" / "index",
+        .block_tree_db_cache_size = 0,
     };
     BlockManager blockman{*Assert(m_node.shutdown), blockman_opts};
     // simulate adding a genesis block normally
@@ -140,6 +142,8 @@ BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file)
         .chainparams = Params(),
         .blocks_dir = m_args.GetBlocksDirPath(),
         .notifications = notifications,
+        .block_tree_db_dir = m_args.GetDataDirNet() / "blocks" / "index",
+        .block_tree_db_cache_size = 0,
     };
     BlockManager blockman{*Assert(m_node.shutdown), blockman_opts};
 

src/test/util/setup_common.cpp

@@ -62,7 +62,6 @@
 #include <stdexcept>
 
 using namespace util::hex_literals;
-using kernel::BlockTreeDB;
 using node::ApplyArgsManOptions;
 using node::BlockAssembler;
 using node::BlockManager;
@@ -244,14 +243,13 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts)
             .chainparams = chainman_opts.chainparams,
             .blocks_dir = m_args.GetBlocksDirPath(),
             .notifications = chainman_opts.notifications,
+            .block_tree_db_dir = m_args.GetDataDirNet() / "blocks" / "index",
+            .wipe_block_tree_db = m_args.GetBoolArg("-reindex", false),
+            .block_tree_db_in_memory = opts.block_tree_db_in_memory,
+            .block_tree_db_cache_size = m_cache_sizes.block_tree_db,
         };
         m_node.chainman = std::make_unique<ChainstateManager>(*Assert(m_node.shutdown), chainman_opts, blockman_opts);
         LOCK(m_node.chainman->GetMutex());
-        m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<BlockTreeDB>(DBParams{
-            .path = m_args.GetDataDirNet() / "blocks" / "index",
-            .cache_bytes = static_cast<size_t>(m_cache_sizes.block_tree_db),
-            .memory_only = true,
-        });
     };
     m_make_chainman();
 }
@@ -277,9 +275,7 @@ void ChainTestingSetup::LoadVerifyActivateChainstate()
     auto& chainman{*Assert(m_node.chainman)};
     node::ChainstateLoadOptions options;
     options.mempool = Assert(m_node.mempool.get());
-    options.block_tree_db_in_memory = m_block_tree_db_in_memory;
     options.coins_db_in_memory = m_coins_db_in_memory;
-    options.wipe_block_tree_db = m_args.GetBoolArg("-reindex", false);
     options.wipe_chainstate_db = m_args.GetBoolArg("-reindex", false) || m_args.GetBoolArg("-reindex-chainstate", false);
     options.prune = chainman.m_blockman.IsPruneMode();
     options.check_blocks = m_args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);
@@ -303,7 +299,6 @@ TestingSetup::TestingSetup(
     : ChainTestingSetup(chainType, opts)
 {
     m_coins_db_in_memory = opts.coins_db_in_memory;
-    m_block_tree_db_in_memory = opts.block_tree_db_in_memory;
     // Ideally we'd move all the RPC tests to the functional testing framework
     // instead of unit tests, but for now we need these here.
     RegisterAllCoreRPCCommands(tableRPC);

src/test/util/setup_common.h

@@ -85,7 +85,6 @@ struct BasicTestingSetup {
 struct ChainTestingSetup : public BasicTestingSetup {
     node::CacheSizes m_cache_sizes{};
     bool m_coins_db_in_memory{true};
-    bool m_block_tree_db_in_memory{true};
     std::function<void()> m_make_chainman{};
 
     explicit ChainTestingSetup(const ChainType chainType = ChainType::MAIN, TestOpts = {});

src/test/validation_chainstatemanager_tests.cpp

@@ -393,6 +393,9 @@ struct SnapshotTestSetup : TestChain100Setup {
                 .chainparams = chainman_opts.chainparams,
                 .blocks_dir = m_args.GetBlocksDirPath(),
                 .notifications = chainman_opts.notifications,
+                .block_tree_db_dir = chainman.m_blockman.m_opts.block_tree_db_dir,
+                .block_tree_db_in_memory = chainman.m_blockman.m_opts.block_tree_db_in_memory,
+                .block_tree_db_cache_size = chainman.m_blockman.m_opts.block_tree_db_cache_size,
             };
             // For robustness, ensure the old manager is destroyed before creating a
             // new one.