From 033477dba65151b1863214606d5a49686ab6f559 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 23 Jan 2024 10:58:25 -0500 Subject: [PATCH 1/2] doc: fix checkblockindex comments These exceptions are not related to situations specific to tests, but are required in general: Without the first check CheckBlockindex could fail for blocks where we only know the header. Without the second, it could fail when blocks are received out of order. --- src/validation.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 8c583c586c204..e47ce5edbdbc8 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4865,9 +4865,9 @@ void ChainstateManager::CheckBlockIndex() // Make sure nChainTx sum is correctly computed. unsigned int prev_chain_tx = pindex->pprev ? pindex->pprev->nChainTx : 0; assert((pindex->nChainTx == pindex->nTx + prev_chain_tx) - // For testing, allow transaction counts to be completely unset. + // Transaction may be completely unset - happens if only the header was accepted but the block hasn't been processed. || (pindex->nChainTx == 0 && pindex->nTx == 0) - // For testing, allow this nChainTx to be unset if previous is also unset. + // nChainTx may be unset, but nTx set (if a block has been accepted, but one of its predecessors hasn't been processed yet) || (pindex->nChainTx == 0 && prev_chain_tx == 0 && pindex->pprev) // Transaction counts prior to snapshot are unknown. || pindex->IsAssumedValid()); From 9819db4ccaa03519a78d4d9ecce9f89f5be669e5 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 23 Jan 2024 18:07:54 -0500 Subject: [PATCH 2/2] validation: move nChainTx assert down in CheckBlockIndex There is a designated section meant for the actual consistency checks, marked by a comment. --- src/validation.cpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index e47ce5edbdbc8..4dc3a0f512517 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4862,16 +4862,6 @@ void ChainstateManager::CheckBlockIndex() CBlockIndex* pindexFirstAssumeValid = nullptr; // Oldest ancestor of pindex which has BLOCK_ASSUMED_VALID while (pindex != nullptr) { nNodes++; - // Make sure nChainTx sum is correctly computed. - unsigned int prev_chain_tx = pindex->pprev ? pindex->pprev->nChainTx : 0; - assert((pindex->nChainTx == pindex->nTx + prev_chain_tx) - // Transaction may be completely unset - happens if only the header was accepted but the block hasn't been processed. - || (pindex->nChainTx == 0 && pindex->nTx == 0) - // nChainTx may be unset, but nTx set (if a block has been accepted, but one of its predecessors hasn't been processed yet) - || (pindex->nChainTx == 0 && prev_chain_tx == 0 && pindex->pprev) - // Transaction counts prior to snapshot are unknown. - || pindex->IsAssumedValid()); - if (pindexFirstAssumeValid == nullptr && pindex->nStatus & BLOCK_ASSUMED_VALID) pindexFirstAssumeValid = pindex; if (pindexFirstInvalid == nullptr && pindex->nStatus & BLOCK_FAILED_VALID) pindexFirstInvalid = pindex; if (pindexFirstMissing == nullptr && !(pindex->nStatus & BLOCK_HAVE_DATA)) { @@ -4954,6 +4944,15 @@ void ChainstateManager::CheckBlockIndex() // Checks for not-invalid blocks. assert((pindex->nStatus & BLOCK_FAILED_MASK) == 0); // The failed mask cannot be set for blocks without invalid parents. } + // Make sure nChainTx sum is correctly computed. + unsigned int prev_chain_tx = pindex->pprev ? pindex->pprev->nChainTx : 0; + assert((pindex->nChainTx == pindex->nTx + prev_chain_tx) + // Transaction may be completely unset - happens if only the header was accepted but the block hasn't been processed. + || (pindex->nChainTx == 0 && pindex->nTx == 0) + // nChainTx may be unset, but nTx set (if a block has been accepted, but one of its predecessors hasn't been processed yet) + || (pindex->nChainTx == 0 && prev_chain_tx == 0 && pindex->pprev) + // Transaction counts prior to snapshot are unknown. + || pindex->IsAssumedValid()); // Chainstate-specific checks on setBlockIndexCandidates for (auto c : GetAll()) { if (c->m_chain.Tip() == nullptr) continue;