From ed4462cc78afd2065bbf5bd79728852b65b9ad7f Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 15 Jun 2023 05:11:34 -0300 Subject: [PATCH 1/9] init: start indexes sync earlier The mempool load can take a while, and it is not needed for the indexes' synchronization. Also, having the mempool load function call inside 'blockstorage.cpp' wasn't structurally correct. --- src/init.cpp | 5 ++++- src/node/blockstorage.cpp | 4 ++-- src/node/blockstorage.h | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 988976028dd..545f6068d9f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1657,7 +1657,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } chainman.m_load_block = std::thread(&util::TraceThread, "loadblk", [=, &chainman, &args] { - ThreadImport(chainman, vImportFiles, ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{}); + // Import blocks + ThreadImport(chainman, vImportFiles); + // Load mempool from disk + chainman.ActiveChainstate().LoadMempool(ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{}); }); // Wait for genesis block to be processed diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 191b6dc2f5f..461ec10f66c 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -868,7 +868,7 @@ public: } }; -void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles, const fs::path& mempool_path) +void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles) { ScheduleBatchPriority(); @@ -939,7 +939,7 @@ void ThreadImport(ChainstateManager& chainman, std::vector vImportFile return; } } // End scope of ImportingNow - chainman.ActiveChainstate().LoadMempool(mempool_path); + g_indexes_ready_to_sync = true; } } // namespace node diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 36b8aa48061..4895e582e90 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -255,7 +255,7 @@ public: void CleanupBlockRevFiles() const; }; -void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles, const fs::path& mempool_path); +void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles); } // namespace node #endif // BITCOIN_NODE_BLOCKSTORAGE_H From 04575106b2529f495ce8110ddf7ed2247d4bc339 Mon Sep 17 00:00:00 2001 From: furszy Date: Sat, 17 Jun 2023 11:11:51 -0300 Subject: [PATCH 2/9] scripted-diff: rename 'loadblk' thread name to 'initload' The thread does not only load blocks, it loads the mempool and, in a future commit, will start the indexes as well. Also, renamed the 'ThreadImport' function to 'ImportBlocks' And the 'm_load_block' class member to 'm_thread_load'. -BEGIN VERIFY SCRIPT- sed -i "s/ThreadImport/ImportBlocks/g" $(git grep -l ThreadImport -- ':!/doc/') sed -i "s/loadblk/initload/g" $(git grep -l loadblk -- ':!/doc/release-notes/') sed -i "s/m_load_block/m_thread_load/g" $(git grep -l m_load_block) -END VERIFY SCRIPT- --- doc/developer-notes.md | 2 +- src/bitcoin-chainstate.cpp | 2 +- src/init.cpp | 12 ++++++------ src/node/blockstorage.cpp | 2 +- src/node/blockstorage.h | 2 +- src/node/chainstate.cpp | 2 +- src/validation.h | 2 +- test/functional/feature_init.py | 2 +- test/functional/test_framework/test_node.py | 2 +- 9 files changed, 14 insertions(+), 14 deletions(-) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index fca72914a3a..8e96f3ceb18 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -621,7 +621,7 @@ Threads : Started from `main()` in `bitcoind.cpp`. Responsible for starting up and shutting down the application. -- [ThreadImport (`b-loadblk`)](https://doxygen.bitcoincore.org/namespacenode.html#ab4305679079866f0f420f7dbf278381d) +- [ThreadImport (`b-initload`)](https://doxygen.bitcoincore.org/namespacenode.html#ab4305679079866f0f420f7dbf278381d) : Loads blocks from `blk*.dat` files or `-loadblock=` on startup. - [CCheckQueue::Loop (`b-scriptch.x`)](https://doxygen.bitcoincore.org/class_c_check_queue.html#a6e7fa51d3a25e7cb65446d4b50e6a987) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index c36233207e8..45fd239e20f 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -287,7 +287,7 @@ epilogue: // Without this precise shutdown sequence, there will be a lot of nullptr // dereferencing and UB. scheduler.stop(); - if (chainman.m_load_block.joinable()) chainman.m_load_block.join(); + if (chainman.m_thread_load.joinable()) chainman.m_thread_load.join(); StopScriptCheckWorkerThreads(); GetMainSignals().FlushBackgroundCallbacks(); diff --git a/src/init.cpp b/src/init.cpp index 545f6068d9f..80205df000f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -131,7 +131,7 @@ using node::LoadChainstate; using node::MempoolPath; using node::NodeContext; using node::ShouldPersistMempool; -using node::ThreadImport; +using node::ImportBlocks; using node::VerifyLoadedChainstate; static constexpr bool DEFAULT_PROXYRANDOMIZE{true}; @@ -268,7 +268,7 @@ void Shutdown(NodeContext& node) // After everything has been shut down, but before things get flushed, stop the // CScheduler/checkqueue, scheduler and load block thread. if (node.scheduler) node.scheduler->stop(); - if (node.chainman && node.chainman->m_load_block.joinable()) node.chainman->m_load_block.join(); + if (node.chainman && node.chainman->m_thread_load.joinable()) node.chainman->m_thread_load.join(); StopScriptCheckWorkerThreads(); // After the threads that potentially access these pointers have been stopped, @@ -1545,7 +1545,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 8: start indexers - // If reindex-chainstate was specified, delay syncing indexes until ThreadImport has reindexed the chain + // If reindex-chainstate was specified, delay syncing indexes until ImportBlocks has reindexed the chain if (!fReindexChainState) g_indexes_ready_to_sync = true; if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) { auto result{WITH_LOCK(cs_main, return CheckLegacyTxindex(*Assert(chainman.m_blockman.m_block_tree_db)))}; @@ -1656,9 +1656,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) vImportFiles.push_back(fs::PathFromString(strFile)); } - chainman.m_load_block = std::thread(&util::TraceThread, "loadblk", [=, &chainman, &args] { + chainman.m_thread_load = std::thread(&util::TraceThread, "initload", [=, &chainman, &args] { // Import blocks - ThreadImport(chainman, vImportFiles); + ImportBlocks(chainman, vImportFiles); // Load mempool from disk chainman.ActiveChainstate().LoadMempool(ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{}); }); @@ -1667,7 +1667,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) { WAIT_LOCK(g_genesis_wait_mutex, lock); // We previously could hang here if StartShutdown() is called prior to - // ThreadImport getting started, so instead we just wait on a timer to + // ImportBlocks getting started, so instead we just wait on a timer to // check ShutdownRequested() regularly. while (!fHaveGenesis && !ShutdownRequested()) { g_genesis_wait_cv.wait_for(lock, std::chrono::milliseconds(500)); diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 461ec10f66c..0e9ae0ae270 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -868,7 +868,7 @@ public: } }; -void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles) +void ImportBlocks(ChainstateManager& chainman, std::vector vImportFiles) { ScheduleBatchPriority(); diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 4895e582e90..2c219e7a598 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -255,7 +255,7 @@ public: void CleanupBlockRevFiles() const; }; -void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles); +void ImportBlocks(ChainstateManager& chainman, std::vector vImportFiles); } // namespace node #endif // BITCOIN_NODE_BLOCKSTORAGE_H diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 3900d2e6200..255d8be0ecb 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -82,7 +82,7 @@ static ChainstateLoadResult CompleteChainstateInitialization( // At this point blocktree args are consistent with what's on disk. // 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 ThreadImport after the reindex completes. + // This is called again in ImportBlocks after the reindex completes. if (!fReindex && !chainman.ActiveChainstate().LoadGenesisBlock()) { return {ChainstateLoadStatus::FAILURE, _("Error initializing block database")}; } diff --git a/src/validation.h b/src/validation.h index 66bc036e668..49860f2d6af 100644 --- a/src/validation.h +++ b/src/validation.h @@ -987,7 +987,7 @@ public: const util::SignalInterrupt& m_interrupt; const Options m_options; - std::thread m_load_block; + std::thread m_thread_load; //! A single BlockManager instance is shared across each constructed //! chainstate to avoid duplicating block metadata. node::BlockManager m_blockman; diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index 7af67730bdf..64ca312b842 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -71,7 +71,7 @@ class InitStressTest(BitcoinTestFramework): b'init message: Starting network threads', b'net thread start', b'addcon thread start', - b'loadblk thread start', + b'initload thread start', b'txindex thread start', b'block filter index thread start', b'coinstatsindex thread start', diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 5111d88e15e..e5e3061def9 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -250,7 +250,7 @@ class TestNode(): # Wait for the node to finish reindex, block import, and # loading the mempool. Usually importing happens fast or # even "immediate" when the node is started. However, there - # is no guarantee and sometimes ThreadImport might finish + # is no guarantee and sometimes ImportBlocks might finish # later. This is going to cause intermittent test failures, # because generally the tests assume the node is fully # ready after being started. From 2ebc7e68cc9d347807b646f601b27940c9590c89 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 27 Jun 2023 19:25:04 -0300 Subject: [PATCH 3/9] doc: describe 'init load' thread actions --- doc/developer-notes.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 8e96f3ceb18..80353bcdd26 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -621,8 +621,9 @@ Threads : Started from `main()` in `bitcoind.cpp`. Responsible for starting up and shutting down the application. -- [ThreadImport (`b-initload`)](https://doxygen.bitcoincore.org/namespacenode.html#ab4305679079866f0f420f7dbf278381d) - : Loads blocks from `blk*.dat` files or `-loadblock=` on startup. +- [Init load (`b-initload`)](https://doxygen.bitcoincore.org/namespacenode.html#ab4305679079866f0f420f7dbf278381d) + : Performs various loading tasks that are part of init but shouldn't block the node from being started: external block import, + reindex, reindex-chainstate, main chain activation, spawn indexes background sync threads and mempool load. - [CCheckQueue::Loop (`b-scriptch.x`)](https://doxygen.bitcoincore.org/class_c_check_queue.html#a6e7fa51d3a25e7cb65446d4b50e6a987) : Parallel script validation threads for transactions in blocks. From 225e213110602b4fd1d345167f5f92d26557f6c1 Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 16 Jun 2023 10:16:22 -0300 Subject: [PATCH 4/9] refactor: init indexes, decouple 'Start()' from the creation step No behavior change. The goal here is to group indexes, so we can perform the same initialization and verification process equally for all of them. The checks performed inside `StartIndexes` will be expanded in the subsequent commits. --- src/init.cpp | 21 ++++++++++++--------- src/init.h | 3 +++ src/node/context.h | 4 +++- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 80205df000f..7d44ccc17f2 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1554,25 +1554,22 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } g_txindex = std::make_unique(interfaces::MakeChain(node), cache_sizes.tx_index, false, fReindex); - if (!g_txindex->Start()) { - return false; - } + node.indexes.emplace_back(g_txindex.get()); } for (const auto& filter_type : g_enabled_filter_types) { InitBlockFilterIndex([&]{ return interfaces::MakeChain(node); }, filter_type, cache_sizes.filter_index, false, fReindex); - if (!GetBlockFilterIndex(filter_type)->Start()) { - return false; - } + node.indexes.emplace_back(GetBlockFilterIndex(filter_type)); } if (args.GetBoolArg("-coinstatsindex", DEFAULT_COINSTATSINDEX)) { g_coin_stats_index = std::make_unique(interfaces::MakeChain(node), /*cache_size=*/0, false, fReindex); - if (!g_coin_stats_index->Start()) { - return false; - } + node.indexes.emplace_back(g_coin_stats_index.get()); } + // Now that all indexes are loaded, start them + StartIndexes(node); + // ********************************************************* Step 9: load wallet for (const auto& client : node.chain_clients) { if (!client->load()) { @@ -1878,3 +1875,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return true; } + +bool StartIndexes(NodeContext& node) +{ + for (auto index : node.indexes) if (!index->Start()) return false; + return true; +} diff --git a/src/init.h b/src/init.h index 4dcf3fc51ee..a050a76b3cc 100644 --- a/src/init.h +++ b/src/init.h @@ -73,4 +73,7 @@ bool AppInitMain(node::NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip */ void SetupServerArgs(ArgsManager& argsman); +/** Validates requirements to run the indexes and spawns each index initial sync thread */ +bool StartIndexes(node::NodeContext& node); + #endif // BITCOIN_INIT_H diff --git a/src/node/context.h b/src/node/context.h index 91b68fa5bbf..dae900ff7f3 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -15,8 +15,9 @@ #include class ArgsManager; -class BanMan; class AddrMan; +class BanMan; +class BaseIndex; class CBlockPolicyEstimator; class CConnman; class CScheduler; @@ -58,6 +59,7 @@ struct NodeContext { std::unique_ptr chainman; std::unique_ptr banman; ArgsManager* args{nullptr}; // Currently a raw pointer because the memory is not managed by this struct + std::vector indexes; // raw pointers because memory is not managed by this struct std::unique_ptr chain; //! List of all chain clients (wallet processes or other client) connected to node. std::vector> chain_clients; From 430e7027a18870a296abb0bbd9332cbe40d8fdc0 Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 17 May 2023 00:55:09 -0300 Subject: [PATCH 5/9] refactor: index, decouple 'Init' from 'Start' So indexes can be initialized without spawning the sync thread. This makes asynchronous indexes startup possible in the following commits. --- src/index/base.cpp | 18 ++++++++++-------- src/index/base.h | 14 ++++++++------ src/init.cpp | 9 ++++++--- src/init.h | 2 +- src/test/blockfilter_index_tests.cpp | 3 ++- src/test/coinstatsindex_tests.cpp | 9 ++++++--- src/test/txindex_tests.cpp | 3 ++- 7 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index cf07cae2864..0f25881804f 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -81,6 +81,13 @@ BaseIndex::~BaseIndex() bool BaseIndex::Init() { + // m_chainstate member gives indexing code access to node internals. It is + // removed in followup https://github.com/bitcoin/bitcoin/pull/24230 + m_chainstate = &m_chain->context()->chainman->ActiveChainstate(); + // Register to validation interface before setting the 'm_synced' flag, so that + // callbacks are not missed once m_synced is true. + RegisterValidationInterface(this); + CBlockLocator locator; if (!GetDB().ReadBestBlock(locator)) { locator.SetNull(); @@ -147,6 +154,7 @@ bool BaseIndex::Init() // datadir and an index enabled. If this is the case, indexation will happen solely // via `BlockConnected` signals until, possibly, the next restart. m_synced = synced; + m_init = true; return true; } @@ -401,15 +409,9 @@ void BaseIndex::Interrupt() m_interrupt(); } -bool BaseIndex::Start() +bool BaseIndex::StartBackgroundSync() { - // m_chainstate member gives indexing code access to node internals. It is - // removed in followup https://github.com/bitcoin/bitcoin/pull/24230 - m_chainstate = &m_chain->context()->chainman->ActiveChainstate(); - // 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()) return false; + if (!m_init) throw std::logic_error("Error: Cannot start a non-initialized index"); m_thread_sync = std::thread(&util::TraceThread, GetName(), [this] { ThreadSync(); }); return true; diff --git a/src/index/base.h b/src/index/base.h index 8affee90f86..8b986fe8c42 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -54,6 +54,8 @@ protected: }; private: + /// Whether the index has been initialized or not. + std::atomic m_init{false}; /// Whether the index is in sync with the main chain. The flag is flipped /// from false to true once, after which point this starts processing /// ValidationInterface notifications to stay in sync. @@ -69,9 +71,6 @@ private: std::thread m_thread_sync; CThreadInterrupt m_interrupt; - /// Read best block locator and check that data needed to sync has not been pruned. - bool Init(); - /// Sync the index with the block index starting from the current best block. /// Intended to be run in its own thread, m_thread_sync, and can be /// interrupted with m_interrupt. Once the index gets in sync, the m_synced @@ -142,9 +141,12 @@ public: void Interrupt(); - /// Start initializes the sync state and registers the instance as a - /// ValidationInterface so that it stays in sync with blockchain updates. - [[nodiscard]] bool Start(); + /// Initializes the sync state and registers the instance to the + /// validation interface so that it stays in sync with blockchain updates. + [[nodiscard]] bool Init(); + + /// Starts the initial sync process. + [[nodiscard]] bool StartBackgroundSync(); /// Stops the instance from staying in sync with blockchain updates. void Stop(); diff --git a/src/init.cpp b/src/init.cpp index 7d44ccc17f2..102e7932cd0 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1567,8 +1567,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node.indexes.emplace_back(g_coin_stats_index.get()); } + // Init indexes + for (auto index : node.indexes) if (!index->Init()) return false; + // Now that all indexes are loaded, start them - StartIndexes(node); + if (!StartIndexBackgroundSync(node)) return false; // ********************************************************* Step 9: load wallet for (const auto& client : node.chain_clients) { @@ -1876,8 +1879,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return true; } -bool StartIndexes(NodeContext& node) +bool StartIndexBackgroundSync(NodeContext& node) { - for (auto index : node.indexes) if (!index->Start()) return false; + for (auto index : node.indexes) if (!index->StartBackgroundSync()) return false; return true; } diff --git a/src/init.h b/src/init.h index a050a76b3cc..f27d6120ef3 100644 --- a/src/init.h +++ b/src/init.h @@ -74,6 +74,6 @@ bool AppInitMain(node::NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip void SetupServerArgs(ArgsManager& argsman); /** Validates requirements to run the indexes and spawns each index initial sync thread */ -bool StartIndexes(node::NodeContext& node); +bool StartIndexBackgroundSync(node::NodeContext& node); #endif // BITCOIN_INIT_H diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index 24af51cce13..9bd5c7c2b6e 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -113,6 +113,7 @@ bool BuildChainTestingSetup::BuildChain(const CBlockIndex* pindex, BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) { BlockFilterIndex filter_index(interfaces::MakeChain(m_node), BlockFilterType::BASIC, 1 << 20, true); + BOOST_REQUIRE(filter_index.Init()); uint256 last_header; @@ -139,7 +140,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) // BlockUntilSyncedToCurrentChain should return false before index is started. BOOST_CHECK(!filter_index.BlockUntilSyncedToCurrentChain()); - BOOST_REQUIRE(filter_index.Start()); + BOOST_REQUIRE(filter_index.StartBackgroundSync()); // Allow filter index to catch up with the block index. IndexWaitSynced(filter_index); diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp index 9dc88ea671d..74d6d7231a7 100644 --- a/src/test/coinstatsindex_tests.cpp +++ b/src/test/coinstatsindex_tests.cpp @@ -18,6 +18,7 @@ BOOST_AUTO_TEST_SUITE(coinstatsindex_tests) BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup) { CoinStatsIndex coin_stats_index{interfaces::MakeChain(m_node), 1 << 20, true}; + BOOST_REQUIRE(coin_stats_index.Init()); const CBlockIndex* block_index; { @@ -32,7 +33,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup) // is started. BOOST_CHECK(!coin_stats_index.BlockUntilSyncedToCurrentChain()); - BOOST_REQUIRE(coin_stats_index.Start()); + BOOST_REQUIRE(coin_stats_index.StartBackgroundSync()); IndexWaitSynced(coin_stats_index); @@ -83,7 +84,8 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup) const CChainParams& params = Params(); { CoinStatsIndex index{interfaces::MakeChain(m_node), 1 << 20}; - BOOST_REQUIRE(index.Start()); + BOOST_REQUIRE(index.Init()); + BOOST_REQUIRE(index.StartBackgroundSync()); IndexWaitSynced(index); std::shared_ptr new_block; CBlockIndex* new_block_index = nullptr; @@ -109,8 +111,9 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup) { CoinStatsIndex index{interfaces::MakeChain(m_node), 1 << 20}; + BOOST_REQUIRE(index.Init()); // Make sure the index can be loaded. - BOOST_REQUIRE(index.Start()); + BOOST_REQUIRE(index.StartBackgroundSync()); index.Stop(); } } diff --git a/src/test/txindex_tests.cpp b/src/test/txindex_tests.cpp index 2677502ef03..cb80dbed69c 100644 --- a/src/test/txindex_tests.cpp +++ b/src/test/txindex_tests.cpp @@ -17,6 +17,7 @@ BOOST_AUTO_TEST_SUITE(txindex_tests) BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup) { TxIndex txindex(interfaces::MakeChain(m_node), 1 << 20, true); + BOOST_REQUIRE(txindex.Init()); CTransactionRef tx_disk; uint256 block_hash; @@ -29,7 +30,7 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup) // BlockUntilSyncedToCurrentChain should return false before txindex is started. BOOST_CHECK(!txindex.BlockUntilSyncedToCurrentChain()); - BOOST_REQUIRE(txindex.Start()); + BOOST_REQUIRE(txindex.StartBackgroundSync()); // Allow tx index to catch up with the block index. IndexWaitSynced(txindex); From c82ef91eae38f385d408b095ebbc8a180ce52ebb Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 16 Jun 2023 17:32:21 -0300 Subject: [PATCH 6/9] make GetFirstStoredBlock assert that 'start_block' always has data And transfer the responsibility of verifying whether 'start_block' has data or not to the caller. This is because the 'GetFirstStoredBlock' function responsibility is to return the first block containing data. And the current implementation can return 'start_block' when it has no data!. Which is misleading at least. Edge case behavior change: Previously, if the block tip lacked data but all preceding blocks contained data, there was no prune violation. And now, such scenario will result in a prune violation. --- src/index/base.cpp | 3 ++- src/node/blockstorage.cpp | 1 + src/node/blockstorage.h | 4 +++- src/rpc/blockchain.cpp | 7 +++---- src/test/blockmanager_tests.cpp | 31 +++++++++++++++++++++++++++++++ 5 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 0f25881804f..ef3ba72cad2 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -115,7 +115,8 @@ bool BaseIndex::Init() if (!start_block) { // index is not built yet // make sure we have all block data back to the genesis - prune_violation = m_chainstate->m_blockman.GetFirstStoredBlock(*active_chain.Tip()) != active_chain.Genesis(); + bool has_tip_data = active_chain.Tip()->nStatus & BLOCK_HAVE_DATA; + prune_violation = !has_tip_data || m_chainstate->m_blockman.GetFirstStoredBlock(*active_chain.Tip()) != active_chain.Genesis(); } // in case the index has a best block set and is not fully synced // check if we have the required blocks to continue building the index diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 0e9ae0ae270..cd057dd708a 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -407,6 +407,7 @@ const CBlockIndex* BlockManager::GetFirstStoredBlock(const CBlockIndex& start_bl { AssertLockHeld(::cs_main); const CBlockIndex* last_block = &start_block; + assert(last_block->nStatus & BLOCK_HAVE_DATA); while (last_block->pprev && (last_block->pprev->nStatus & BLOCK_HAVE_DATA)) { last_block = last_block->pprev; } diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 2c219e7a598..a963cad358f 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -222,7 +222,9 @@ public: //! Returns last CBlockIndex* that is a checkpoint const CBlockIndex* GetLastCheckpoint(const CCheckpointData& data) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - //! Find the first block that is not pruned + //! Find the first stored ancestor of start_block immediately after the last + //! pruned ancestor. Return value will never be null. Caller is responsible + //! for ensuring that start_block has data is not pruned. const CBlockIndex* GetFirstStoredBlock(const CBlockIndex& start_block LIFETIMEBOUND) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** True if any block files have ever been pruned. */ diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index ee3237638ef..717a119b562 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -812,9 +812,7 @@ static RPCHelpMan pruneblockchain() PruneBlockFilesManual(active_chainstate, height); const CBlockIndex& block{*CHECK_NONFATAL(active_chain.Tip())}; - const CBlockIndex* last_block{active_chainstate.m_blockman.GetFirstStoredBlock(block)}; - - return static_cast(last_block->nHeight - 1); + return block.nStatus & BLOCK_HAVE_DATA ? active_chainstate.m_blockman.GetFirstStoredBlock(block)->nHeight - 1 : block.nHeight; }, }; } @@ -1267,7 +1265,8 @@ RPCHelpMan getblockchaininfo() obj.pushKV("size_on_disk", chainman.m_blockman.CalculateCurrentUsage()); obj.pushKV("pruned", chainman.m_blockman.IsPruneMode()); if (chainman.m_blockman.IsPruneMode()) { - obj.pushKV("pruneheight", chainman.m_blockman.GetFirstStoredBlock(tip)->nHeight); + bool has_tip_data = tip.nStatus & BLOCK_HAVE_DATA; + obj.pushKV("pruneheight", has_tip_data ? chainman.m_blockman.GetFirstStoredBlock(tip)->nHeight : tip.nHeight + 1); const bool automatic_pruning{chainman.m_blockman.GetPruneTarget() != BlockManager::PRUNE_TARGET_MANUAL}; obj.pushKV("automatic_pruning", automatic_pruning); diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index 2ab2fa55f0a..58ab49a3297 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -90,4 +90,35 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain BOOST_CHECK(!AutoFile(blockman.OpenBlockFile(new_pos, true)).IsNull()); } +BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup) +{ + LOCK(::cs_main); + auto& chainman = m_node.chainman; + auto& blockman = chainman->m_blockman; + const CBlockIndex& tip = *chainman->ActiveTip(); + + // Function to prune all blocks from 'last_pruned_block' down to the genesis block + const auto& func_prune_blocks = [&](CBlockIndex* last_pruned_block) + { + LOCK(::cs_main); + CBlockIndex* it = last_pruned_block; + while (it != nullptr && it->nStatus & BLOCK_HAVE_DATA) { + it->nStatus &= ~BLOCK_HAVE_DATA; + it = it->pprev; + } + }; + + // 1) Return genesis block when all blocks are available + BOOST_CHECK_EQUAL(blockman.GetFirstStoredBlock(tip), chainman->ActiveChain()[0]); + + // Prune half of the blocks + int height_to_prune = tip.nHeight / 2; + CBlockIndex* first_available_block = chainman->ActiveChain()[height_to_prune + 1]; + CBlockIndex* last_pruned_block = first_available_block->pprev; + func_prune_blocks(last_pruned_block); + + // 2) The last block not pruned is in-between upper-block and the genesis block + BOOST_CHECK_EQUAL(blockman.GetFirstStoredBlock(tip), first_available_block); +} + BOOST_AUTO_TEST_SUITE_END() From 2ec89f1970935d27631bcd17b7923a79cdb1edbb Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 16 May 2023 19:19:06 -0300 Subject: [PATCH 7/9] refactor: simplify pruning violation check By generalizing 'GetFirstStoredBlock' and implementing 'CheckBlockDataAvailability' we can dedup code and avoid repeating work when multiple indexes are enabled. E.g. get the oldest block across all indexes and perform the pruning violation check from that point up to the tip only once (this feature is being introduced in a follow-up commit). This commit shouldn't change behavior in any way. Co-authored-by: Ryan Ofsky --- src/index/base.cpp | 46 ++++++++++++--------------------- src/node/blockstorage.cpp | 20 +++++++++++--- src/node/blockstorage.h | 7 ++++- src/test/blockmanager_tests.cpp | 10 ++++++- 4 files changed, 48 insertions(+), 35 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index ef3ba72cad2..8accc2a6a44 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -111,37 +111,23 @@ bool BaseIndex::Init() const CBlockIndex* start_block = m_best_block_index.load(); bool synced = start_block == active_chain.Tip(); if (!synced && g_indexes_ready_to_sync) { - bool prune_violation = false; - if (!start_block) { - // index is not built yet - // make sure we have all block data back to the genesis - bool has_tip_data = active_chain.Tip()->nStatus & BLOCK_HAVE_DATA; - prune_violation = !has_tip_data || m_chainstate->m_blockman.GetFirstStoredBlock(*active_chain.Tip()) != active_chain.Genesis(); + const CBlockIndex* block_to_test = start_block ? start_block : active_chain.Genesis(); + + // Assert block_to_test is not null here. It can't be null because the + // genesis block can't be null here. The genesis block will be null + // during this BaseIndex::Init() call if the node is being started for + // the first time, or if -reindex is used. But in both of these cases + // m_best_block_index is also null so this branch is not reached. + assert(block_to_test); + + if (!active_chain.Contains(block_to_test)) { + // if the bestblock is not part of the mainchain, find the fork + // so we can make sure we have all data down to the fork + block_to_test = active_chain.FindFork(block_to_test); } - // in case the index has a best block set and is not fully synced - // check if we have the required blocks to continue building the index - else { - const CBlockIndex* block_to_test = start_block; - if (!active_chain.Contains(block_to_test)) { - // if the bestblock is not part of the mainchain, find the fork - // and make sure we have all data down to the fork - block_to_test = active_chain.FindFork(block_to_test); - } - const CBlockIndex* block = active_chain.Tip(); - prune_violation = true; - // check backwards from the tip if we have all block data until we reach the indexes bestblock - while (block_to_test && block && (block->nStatus & BLOCK_HAVE_DATA)) { - if (block_to_test == block) { - prune_violation = false; - break; - } - // block->pprev must exist at this point, since block_to_test is part of the chain - // and thus must be encountered when going backwards from the tip - assert(block->pprev); - block = block->pprev; - } - } - if (prune_violation) { + + // make sure we have all block data back to the start block + if (!m_chainstate->m_blockman.CheckBlockDataAvailability(*active_chain.Tip(), *Assert(block_to_test))) { 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())); } } diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index cd057dd708a..e3c6b062b2e 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -403,17 +403,31 @@ bool BlockManager::IsBlockPruned(const CBlockIndex* pblockindex) return (m_have_pruned && !(pblockindex->nStatus & BLOCK_HAVE_DATA) && pblockindex->nTx > 0); } -const CBlockIndex* BlockManager::GetFirstStoredBlock(const CBlockIndex& start_block) +const CBlockIndex* BlockManager::GetFirstStoredBlock(const CBlockIndex& upper_block, const CBlockIndex* lower_block) { AssertLockHeld(::cs_main); - const CBlockIndex* last_block = &start_block; - assert(last_block->nStatus & BLOCK_HAVE_DATA); + const CBlockIndex* last_block = &upper_block; + assert(last_block->nStatus & BLOCK_HAVE_DATA); // 'upper_block' must have data while (last_block->pprev && (last_block->pprev->nStatus & BLOCK_HAVE_DATA)) { + if (lower_block) { + // Return if we reached the lower_block + if (last_block == lower_block) return lower_block; + // if range was surpassed, means that 'lower_block' is not part of the 'upper_block' chain + // and so far this is not allowed. + assert(last_block->nHeight >= lower_block->nHeight); + } last_block = last_block->pprev; } + assert(last_block != nullptr); return last_block; } +bool BlockManager::CheckBlockDataAvailability(const CBlockIndex& upper_block, const CBlockIndex& lower_block) +{ + if (!(upper_block.nStatus & BLOCK_HAVE_DATA)) return false; + return GetFirstStoredBlock(upper_block, &lower_block) == &lower_block; +} + // If we're using -prune with -reindex, then delete block files that will be ignored by the // reindex. Since reindexing works by starting at block file 0 and looping until a blockfile // is missing, do the same here to delete any later block files after a gap. Also delete all diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index a963cad358f..07ae223cbf9 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -222,10 +222,15 @@ public: //! Returns last CBlockIndex* that is a checkpoint const CBlockIndex* GetLastCheckpoint(const CCheckpointData& data) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + //! Check if all blocks in the [upper_block, lower_block] range have data available. + //! The caller is responsible for ensuring that lower_block is an ancestor of upper_block + //! (part of the same chain). + bool CheckBlockDataAvailability(const CBlockIndex& upper_block LIFETIMEBOUND, const CBlockIndex& lower_block LIFETIMEBOUND) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + //! Find the first stored ancestor of start_block immediately after the last //! pruned ancestor. Return value will never be null. Caller is responsible //! for ensuring that start_block has data is not pruned. - const CBlockIndex* GetFirstStoredBlock(const CBlockIndex& start_block LIFETIMEBOUND) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + const CBlockIndex* GetFirstStoredBlock(const CBlockIndex& start_block LIFETIMEBOUND, const CBlockIndex* lower_block=nullptr) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** True if any block files have ever been pruned. */ bool m_have_pruned = false; diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index 58ab49a3297..e4ed861b12d 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -92,6 +92,7 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup) { + // The goal of the function is to return the first not pruned block in the range [upper_block, lower_block]. LOCK(::cs_main); auto& chainman = m_node.chainman; auto& blockman = chainman->m_blockman; @@ -110,6 +111,11 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup) // 1) Return genesis block when all blocks are available BOOST_CHECK_EQUAL(blockman.GetFirstStoredBlock(tip), chainman->ActiveChain()[0]); + BOOST_CHECK(blockman.CheckBlockDataAvailability(tip, *chainman->ActiveChain()[0])); + + // 2) Check lower_block when all blocks are available + CBlockIndex* lower_block = chainman->ActiveChain()[tip.nHeight / 2]; + BOOST_CHECK(blockman.CheckBlockDataAvailability(tip, *lower_block)); // Prune half of the blocks int height_to_prune = tip.nHeight / 2; @@ -117,8 +123,10 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup) CBlockIndex* last_pruned_block = first_available_block->pprev; func_prune_blocks(last_pruned_block); - // 2) The last block not pruned is in-between upper-block and the genesis block + // 3) The last block not pruned is in-between upper-block and the genesis block BOOST_CHECK_EQUAL(blockman.GetFirstStoredBlock(tip), first_available_block); + BOOST_CHECK(blockman.CheckBlockDataAvailability(tip, *first_available_block)); + BOOST_CHECK(!blockman.CheckBlockDataAvailability(tip, *last_pruned_block)); } BOOST_AUTO_TEST_SUITE_END() From fcbdaeef4d5a63e3e5b479c6fcad730eb86fb923 Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 17 May 2023 16:06:55 -0300 Subject: [PATCH 8/9] init: don't start indexes sync thread prematurely By moving the 'StartIndexes()' call into the 'initload' thread, we can remove the threads active wait. Optimizing the available resources. The only difference with the current state is that now the indexes threads will only be started when they can process work and not before it. --- src/index/base.cpp | 6 ------ src/init.cpp | 11 +++++++---- test/functional/feature_index_prune.py | 4 +++- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 8accc2a6a44..1babfacb157 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -163,12 +163,6 @@ static const CBlockIndex* NextSyncBlock(const CBlockIndex* pindex_prev, CChain& void BaseIndex::ThreadSync() { - // Wait for a possible reindex-chainstate to finish until continuing - // with the index sync - while (!g_indexes_ready_to_sync) { - if (!m_interrupt.sleep_for(std::chrono::milliseconds(500))) return; - } - const CBlockIndex* pindex = m_best_block_index.load(); if (!m_synced) { std::chrono::steady_clock::time_point last_log_time{0s}; diff --git a/src/init.cpp b/src/init.cpp index 102e7932cd0..389654fe4f8 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1570,9 +1570,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // Init indexes for (auto index : node.indexes) if (!index->Init()) return false; - // Now that all indexes are loaded, start them - if (!StartIndexBackgroundSync(node)) return false; - // ********************************************************* Step 9: load wallet for (const auto& client : node.chain_clients) { if (!client->load()) { @@ -1656,9 +1653,15 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) vImportFiles.push_back(fs::PathFromString(strFile)); } - chainman.m_thread_load = std::thread(&util::TraceThread, "initload", [=, &chainman, &args] { + chainman.m_thread_load = std::thread(&util::TraceThread, "initload", [=, &chainman, &args, &node] { // Import blocks ImportBlocks(chainman, vImportFiles); + // Start indexes initial sync + if (!StartIndexBackgroundSync(node)) { + bilingual_str err_str = _("Failed to start indexes, shutting down.."); + chainman.GetNotifications().fatalError(err_str.original, err_str); + return; + } // Load mempool from disk chainman.ActiveChainstate().LoadMempool(ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{}); }); diff --git a/test/functional/feature_index_prune.py b/test/functional/feature_index_prune.py index 77a056346a6..d6e802b399e 100755 --- a/test/functional/feature_index_prune.py +++ b/test/functional/feature_index_prune.py @@ -3,6 +3,7 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test indices in conjunction with prune.""" +import os from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -127,8 +128,9 @@ class FeatureIndexPruneTest(BitcoinTestFramework): self.log.info("make sure we get an init error when starting the nodes again with the indices") filter_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)" stats_msg = "Error: coinstatsindex best block of the index goes beyond pruned data. Please disable the index or reindex (which will download the whole blockchain again)" + end_msg = f"{os.linesep}Error: Failed to start indexes, shutting down.." for i, msg in enumerate([filter_msg, stats_msg, filter_msg]): - self.nodes[i].assert_start_raises_init_error(extra_args=self.extra_args[i], expected_msg=msg) + self.nodes[i].assert_start_raises_init_error(extra_args=self.extra_args[i], expected_msg=msg+end_msg) self.log.info("make sure the nodes start again with the indices and an additional -reindex arg") for i in range(3): From ca91c244ef1ba7eac6643d66a5fc56d3a2a8b550 Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 16 May 2023 18:35:35 -0300 Subject: [PATCH 9/9] index: verify blocks data existence only once At present, during init, we traverse the chain (once per index) to confirm that all necessary blocks to sync each index up to the current tip are present. To make the process more efficient, we can fetch the oldest block from the indexers and perform the chain data existence check from that point only once. This also moves the pruning violation check to the end of the 'loadinit' thread, which is where the reindex, block loading and chain activation processes happen. Making the node's startup process faster, allowing us to remove the global g_indexes_ready_to_sync flag, and enabling the execution of the pruning violation verification even when the reindex or reindex-chainstate flags are enabled (which has being skipped so far). --- src/index/base.cpp | 38 ++++++++----------------------- src/index/base.h | 1 + src/init.cpp | 41 +++++++++++++++++++++++++++++++--- src/node/blockstorage.cpp | 3 --- src/node/blockstorage.h | 1 - src/test/util/setup_common.cpp | 1 - 6 files changed, 48 insertions(+), 37 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 1babfacb157..55fb154d99c 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -23,8 +23,6 @@ #include #include -using node::g_indexes_ready_to_sync; - constexpr uint8_t DB_BEST_BLOCK{'B'}; constexpr auto SYNC_LOG_INTERVAL{30s}; @@ -107,32 +105,8 @@ bool BaseIndex::Init() SetBestBlockIndex(locator_index); } - // Skip pruning check if indexes are not ready to sync (because reindex-chainstate has wiped the chain). - const CBlockIndex* start_block = m_best_block_index.load(); - bool synced = start_block == active_chain.Tip(); - if (!synced && g_indexes_ready_to_sync) { - const CBlockIndex* block_to_test = start_block ? start_block : active_chain.Genesis(); - - // Assert block_to_test is not null here. It can't be null because the - // genesis block can't be null here. The genesis block will be null - // during this BaseIndex::Init() call if the node is being started for - // the first time, or if -reindex is used. But in both of these cases - // m_best_block_index is also null so this branch is not reached. - assert(block_to_test); - - if (!active_chain.Contains(block_to_test)) { - // if the bestblock is not part of the mainchain, find the fork - // so we can make sure we have all data down to the fork - block_to_test = active_chain.FindFork(block_to_test); - } - - // make sure we have all block data back to the start block - if (!m_chainstate->m_blockman.CheckBlockDataAvailability(*active_chain.Tip(), *Assert(block_to_test))) { - 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())); - } - } - // Child init + const CBlockIndex* start_block = m_best_block_index.load(); if (!CustomInit(start_block ? std::make_optional(interfaces::BlockKey{start_block->GetBlockHash(), start_block->nHeight}) : std::nullopt)) { return false; } @@ -140,7 +114,7 @@ bool BaseIndex::Init() // Note: this will latch to true immediately if the user starts up with an empty // datadir and an index enabled. If this is the case, indexation will happen solely // via `BlockConnected` signals until, possibly, the next restart. - m_synced = synced; + m_synced = start_block == active_chain.Tip(); m_init = true; return true; } @@ -412,7 +386,13 @@ IndexSummary BaseIndex::GetSummary() const IndexSummary summary{}; summary.name = GetName(); summary.synced = m_synced; - summary.best_block_height = m_best_block_index ? m_best_block_index.load()->nHeight : 0; + if (const auto& pindex = m_best_block_index.load()) { + summary.best_block_height = pindex->nHeight; + summary.best_block_hash = pindex->GetBlockHash(); + } else { + summary.best_block_height = 0; + summary.best_block_hash = m_chain->getBlockHash(0); + } return summary; } diff --git a/src/index/base.h b/src/index/base.h index 8b986fe8c42..9b2a41dc92b 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -23,6 +23,7 @@ struct IndexSummary { std::string name; bool synced{false}; int best_block_height{0}; + uint256 best_block_hash; }; /** diff --git a/src/init.cpp b/src/init.cpp index 389654fe4f8..e96485b3aac 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -125,7 +125,6 @@ using node::CalculateCacheSizes; using node::DEFAULT_PERSIST_MEMPOOL; using node::DEFAULT_PRINTPRIORITY; using node::fReindex; -using node::g_indexes_ready_to_sync; using node::KernelNotifications; using node::LoadChainstate; using node::MempoolPath; @@ -1545,8 +1544,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 8: start indexers - // If reindex-chainstate was specified, delay syncing indexes until ImportBlocks has reindexed the chain - if (!fReindexChainState) g_indexes_ready_to_sync = true; if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) { auto result{WITH_LOCK(cs_main, return CheckLegacyTxindex(*Assert(chainman.m_blockman.m_block_tree_db)))}; if (!result) { @@ -1884,6 +1881,44 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) bool StartIndexBackgroundSync(NodeContext& node) { + // Find the oldest block among all indexes. + // This block is used to verify that we have the required blocks' data stored on disk, + // starting from that point up to the current tip. + // indexes_start_block='nullptr' means "start from height 0". + std::optional indexes_start_block; + std::string older_index_name; + + ChainstateManager& chainman = *Assert(node.chainman); + for (auto index : node.indexes) { + const IndexSummary& summary = index->GetSummary(); + if (summary.synced) continue; + + // Get the last common block between the index best block and the active chain + LOCK(::cs_main); + const CChain& active_chain = chainman.ActiveChain(); + const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(summary.best_block_hash); + if (!active_chain.Contains(pindex)) { + pindex = active_chain.FindFork(pindex); + } + + if (!indexes_start_block || !pindex || pindex->nHeight < indexes_start_block.value()->nHeight) { + indexes_start_block = pindex; + older_index_name = summary.name; + if (!pindex) break; // Starting from genesis so no need to look for earlier block. + } + }; + + // Verify all blocks needed to sync to current tip are present. + if (indexes_start_block) { + LOCK(::cs_main); + const CBlockIndex* start_block = *indexes_start_block; + if (!start_block) start_block = chainman.ActiveChain().Genesis(); + if (!chainman.m_blockman.CheckBlockDataAvailability(*chainman.ActiveChain().Tip(), *Assert(start_block))) { + 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)"), older_index_name)); + } + } + + // Start threads for (auto index : node.indexes) if (!index->StartBackgroundSync()) return false; return true; } diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index e3c6b062b2e..b3ef29a4453 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -26,7 +26,6 @@ namespace node { std::atomic_bool fReindex(false); -std::atomic_bool g_indexes_ready_to_sync{false}; bool CBlockIndexWorkComparator::operator()(const CBlockIndex* pa, const CBlockIndex* pb) const { @@ -954,7 +953,5 @@ void ImportBlocks(ChainstateManager& chainman, std::vector vImportFile return; } } // End scope of ImportingNow - - g_indexes_ready_to_sync = true; } } // namespace node diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 07ae223cbf9..0b8b6de6ad6 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -50,7 +50,6 @@ static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = CMessageHeader::MESSAGE_START_SIZE + sizeof(unsigned int); extern std::atomic_bool fReindex; -extern std::atomic_bool g_indexes_ready_to_sync; // Because validation code takes pointers to the map's CBlockIndex objects, if // we ever switch to another associative container, we need to either use a diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 3e2f0ab88d5..d8f30bdc6e6 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -157,7 +157,6 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto noui_connect(); noui_connected = true; } - node::g_indexes_ready_to_sync = true; } BasicTestingSetup::~BasicTestingSetup()