From 7fbb1bc44b1461f008284533f1667677e729f0c0 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Mon, 20 Jan 2025 17:40:21 +0100 Subject: [PATCH] kernel: Move block tree db open to block manager This commit is done in preparation for the next commit. Here, the block tree options are moved to the blockmanager options and the block tree is instantiated through a helper method of the BlockManager, which is removed again in the next commit. Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> --- src/bitcoin-chainstate.cpp | 6 +++++- src/init.cpp | 11 ++++++++++- src/kernel/blockmanager_opts.h | 2 ++ src/kernel/chainstatemanager_opts.h | 1 - src/node/blockmanager_args.cpp | 3 +++ src/node/blockstorage.h | 2 ++ src/node/chainstate.cpp | 14 ++++---------- src/node/chainstate.h | 5 ----- src/node/chainstatemanager_args.cpp | 1 - src/test/blockmanager_tests.cpp | 8 ++++++++ src/test/util/setup_common.cpp | 15 +++++++-------- src/test/validation_chainstatemanager_tests.cpp | 5 +++++ 12 files changed, 46 insertions(+), 27 deletions(-) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 51c482607aa..e657f018715 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -106,6 +106,7 @@ int main(int argc, char* argv[]) }; auto notifications = std::make_unique(); + kernel::CacheSizes cache_sizes{DEFAULT_KERNEL_CACHE}; // SETUP: Chainstate auto chainparams = CChainParams::Main(); @@ -119,11 +120,14 @@ int main(int argc, char* argv[]) .chainparams = chainman_opts.chainparams, .blocks_dir = abs_datadir / "blocks", .notifications = chainman_opts.notifications, + .block_tree_db_params = DBParams{ + .path = abs_datadir / "blocks" / "index", + .cache_bytes = cache_sizes.block_tree_db, + }, }; util::SignalInterrupt interrupt; ChainstateManager chainman{interrupt, chainman_opts, blockman_opts}; - kernel::CacheSizes cache_sizes{DEFAULT_KERNEL_CACHE}; node::ChainstateLoadOptions options; auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options); if (status != node::ChainstateLoadStatus::SUCCESS) { diff --git a/src/init.cpp b/src/init.cpp index 116823c36f0..a306449dbdf 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1057,6 +1057,10 @@ bool AppInitParameterInteraction(const ArgsManager& args) .chainparams = chainman_opts_dummy.chainparams, .blocks_dir = args.GetBlocksDirPath(), .notifications = chainman_opts_dummy.notifications, + .block_tree_db_params = DBParams{ + .path = args.GetDataDirNet() / "blocks" / "index", + .cache_bytes = 0, + }, }; auto blockman_result{ApplyArgsManOptions(args, blockman_opts_dummy)}; if (!blockman_result) { @@ -1199,10 +1203,16 @@ static ChainstateLoadResult InitAndLoadChainstate( .signals = node.validation_signals.get(), }; Assert(ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction + BlockManager::Options blockman_opts{ .chainparams = chainman_opts.chainparams, .blocks_dir = args.GetBlocksDirPath(), .notifications = chainman_opts.notifications, + .block_tree_db_params = DBParams{ + .path = args.GetDataDirNet() / "blocks" / "index", + .cache_bytes = cache_sizes.block_tree_db, + .wipe_data = do_reindex, + }, }; Assert(ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction try { @@ -1233,7 +1243,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); diff --git a/src/kernel/blockmanager_opts.h b/src/kernel/blockmanager_opts.h index 261ec3be582..5c7b24d7170 100644 --- a/src/kernel/blockmanager_opts.h +++ b/src/kernel/blockmanager_opts.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H #define BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H +#include #include #include @@ -27,6 +28,7 @@ struct BlockManagerOpts { bool fast_prune{false}; const fs::path blocks_dir; Notifications& notifications; + DBParams block_tree_db_params; }; } // namespace kernel diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h index 1b605f3d55d..15a8fbec618 100644 --- a/src/kernel/chainstatemanager_opts.h +++ b/src/kernel/chainstatemanager_opts.h @@ -42,7 +42,6 @@ struct ChainstateManagerOpts { std::optional 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; diff --git a/src/node/blockmanager_args.cpp b/src/node/blockmanager_args.cpp index 0fc4e1646a1..5186b65546c 100644 --- a/src/node/blockmanager_args.cpp +++ b/src/node/blockmanager_args.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -34,6 +35,8 @@ util::Result ApplyArgsManOptions(const ArgsManager& args, BlockManager::Op if (auto value{args.GetBoolArg("-fastprune")}) opts.fast_prune = *value; + ReadDatabaseArgs(args, opts.block_tree_db_params.options); + return {}; } } // namespace node diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index ac6db0558df..9db80c26c58 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -268,6 +268,8 @@ public: using Options = kernel::BlockManagerOpts; explicit BlockManager(const util::SignalInterrupt& interrupt, Options opts); + auto MakeBlockTreeDb() { return std::make_unique(m_opts.block_tree_db_params); } + auto OptsWipeBlockTreeDb() { return m_opts.block_tree_db_params.wipe_data; } const util::SignalInterrupt& m_interrupt; std::atomic m_importing{false}; diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 10b6e52c78b..4fc7a851cdf 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -36,7 +36,6 @@ namespace node { // to ChainstateManager::InitializeChainstate(). static ChainstateLoadResult CompleteChainstateInitialization( ChainstateManager& chainman, - const CacheSizes& cache_sizes, const ChainstateLoadOptions& options) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { auto& pblocktree{chainman.m_blockman.m_block_tree_db}; @@ -44,18 +43,13 @@ static ChainstateLoadResult CompleteChainstateInitialization( // fails if it's still open from the previous loop. Close it first: pblocktree.reset(); try { - pblocktree = std::make_unique(DBParams{ - .path = chainman.m_options.datadir / "blocks" / "index", - .cache_bytes = 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}); + pblocktree = chainman.m_blockman.MakeBlockTreeDb(); } catch (dbwrapper_error& err) { LogError("%s\n", err.what()); return {ChainstateLoadStatus::FAILURE, _("Error opening block database")}; } - if (options.wipe_block_tree_db) { + if (chainman.m_blockman.OptsWipeBlockTreeDb()) { 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 @@ -206,7 +200,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize } } - auto [init_status, init_error] = CompleteChainstateInitialization(chainman, cache_sizes, options); + auto [init_status, init_error] = CompleteChainstateInitialization(chainman, options); if (init_status != ChainstateLoadStatus::SUCCESS) { return {init_status, init_error}; } @@ -242,7 +236,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize // for the fully validated chainstate. chainman.ActiveChainstate().ClearBlockIndexCandidates(); - auto [init_status, init_error] = CompleteChainstateInitialization(chainman, cache_sizes, options); + auto [init_status, init_error] = CompleteChainstateInitialization(chainman, options); if (init_status != ChainstateLoadStatus::SUCCESS) { return {init_status, init_error}; } diff --git a/src/node/chainstate.h b/src/node/chainstate.h index ce414fa0434..843e8e09a66 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -22,12 +22,7 @@ namespace node { 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}; diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp index 0ac96c55149..db36d03fd5c 100644 --- a/src/node/chainstatemanager_args.cpp +++ b/src/node/chainstatemanager_args.cpp @@ -49,7 +49,6 @@ util::Result 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); diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index c2b95dd861e..ec91f2b276e 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -33,6 +33,10 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) .chainparams = *params, .blocks_dir = m_args.GetBlocksDirPath(), .notifications = notifications, + .block_tree_db_params = DBParams{ + .path = m_args.GetDataDirNet() / "blocks" / "index", + .cache_bytes = 0, + }, }; BlockManager blockman{*Assert(m_node.shutdown_signal), blockman_opts}; // simulate adding a genesis block normally @@ -140,6 +144,10 @@ BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) .chainparams = Params(), .blocks_dir = m_args.GetBlocksDirPath(), .notifications = notifications, + .block_tree_db_params = DBParams{ + .path = m_args.GetDataDirNet() / "blocks" / "index", + .cache_bytes = 0, + }, }; BlockManager blockman{*Assert(m_node.shutdown_signal), blockman_opts}; diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 2cf25c4a2ad..83b78d7b5ad 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -62,7 +62,6 @@ #include using namespace util::hex_literals; -using kernel::BlockTreeDB; using node::ApplyArgsManOptions; using node::BlockAssembler; using node::BlockManager; @@ -250,14 +249,16 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts) .chainparams = chainman_opts.chainparams, .blocks_dir = m_args.GetBlocksDirPath(), .notifications = chainman_opts.notifications, + .block_tree_db_params = DBParams{ + .path = m_args.GetDataDirNet() / "blocks" / "index", + .cache_bytes = m_kernel_cache_sizes.block_tree_db, + .memory_only = opts.block_tree_db_in_memory, + .wipe_data = m_args.GetBoolArg("-reindex", false), + }, }; m_node.chainman = std::make_unique(*Assert(m_node.shutdown_signal), chainman_opts, blockman_opts); LOCK(m_node.chainman->GetMutex()); - m_node.chainman->m_blockman.m_block_tree_db = std::make_unique(DBParams{ - .path = m_args.GetDataDirNet() / "blocks" / "index", - .cache_bytes = m_kernel_cache_sizes.block_tree_db, - .memory_only = true, - }); + m_node.chainman->m_blockman.m_block_tree_db = m_node.chainman->m_blockman.MakeBlockTreeDb(); }; m_make_chainman(); } @@ -283,9 +284,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); diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 6c2a825e647..bf440ca6397 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -393,6 +393,11 @@ struct SnapshotTestSetup : TestChain100Setup { .chainparams = chainman_opts.chainparams, .blocks_dir = m_args.GetBlocksDirPath(), .notifications = chainman_opts.notifications, + .block_tree_db_params = DBParams{ + .path = chainman.m_options.datadir / "blocks" / "index", + .cache_bytes = m_kernel_cache_sizes.block_tree_db, + .memory_only = m_block_tree_db_in_memory, + }, }; // For robustness, ensure the old manager is destroyed before creating a // new one.