From a2675897e2a499aacbd0183fdccf1401953e8de5 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Fri, 6 Dec 2024 11:37:10 -0500 Subject: [PATCH 1/2] validation: Don't loop over all chainstates in LoadExternalBlock This simplifies the code. The only reason to call ActivateBestChain() here is to allow the main init thread to finish startup in a case of -reindex. In this situation no second chainstate can exist anyway because -reindex would have deleted any snapshot chainstate earlier. This could change behavior slightly if -loadblocks was used when there is a snapshot chainstate. In this case, there is no reason to call ActivateBestChain() for that chainstate here - it will be called in ImportBlocks() after all blocks have been indexed. --- src/validation.cpp | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 0227ccec2fd..0e9b8da48aa 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5158,15 +5158,8 @@ void ChainstateManager::LoadExternalBlockFile( // Activate the genesis block so normal node progress can continue if (hash == params.GetConsensus().hashGenesisBlock) { - bool genesis_activation_failure = false; - for (auto c : GetAll()) { - BlockValidationState state; - if (!c->ActivateBestChain(state, nullptr)) { - genesis_activation_failure = true; - break; - } - } - if (genesis_activation_failure) { + BlockValidationState state; + if (!ActiveChainstate().ActivateBestChain(state, nullptr)) { break; } } From c9136ca90605bbe29f005f538b92ff96ca360a13 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Fri, 6 Dec 2024 11:49:10 -0500 Subject: [PATCH 2/2] validation: fix issue with an interrupted -reindex If a reindex was interrupted while it was iterating through the block files, genesis will already be connected when the reindex resumes at the next startup. In this case, a call to ActivateBestChainState() is not only unnecessary, but it would connect multiple blocks without applying -assumevalid, which is much slower. This is because assumevalid requires us to have a header above the minimum chainwork, but that header is unknown to us if it's in a later blockfile not indexed yet. --- src/validation.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index 0e9b8da48aa..b1874a2adae 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5157,7 +5157,13 @@ void ChainstateManager::LoadExternalBlockFile( } // Activate the genesis block so normal node progress can continue - if (hash == params.GetConsensus().hashGenesisBlock) { + // During first -reindex, this will only connect Genesis since + // ActivateBestChain only connects blocks which are in the block tree db, + // which only contains blocks whose parents are in it. + // But do this only if genesis isn't activated yet, to avoid connecting many blocks + // without assumevalid in the case of a continuation of a reindex that + // was interrupted by the user. + if (hash == params.GetConsensus().hashGenesisBlock && WITH_LOCK(::cs_main, return ActiveHeight()) == -1) { BlockValidationState state; if (!ActiveChainstate().ActivateBestChain(state, nullptr)) { break;