From 3c3548a70eedb8dcf6a4a8d605a4a12e814c7cac Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 8 Mar 2025 17:43:44 +0530 Subject: [PATCH] validation: clarify final |= BLOCK_FAILED_VALID in InvalidateBlock This has no functional affect, as the any CBlockIndex*s which to_mark_failed is set to will already have been marked failed. Also prevents a situation where block already marked as BLOCK_FAILED_CHILD is again unconditionally marked as BLOCK_FAILED_VALID in the final |= BLOCK_FAILED_VALID. --- src/test/blockchain_tests.cpp | 5 +++++ src/validation.cpp | 12 +++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/test/blockchain_tests.cpp b/src/test/blockchain_tests.cpp index 9bc05fdccf3..5da2841c4dd 100644 --- a/src/test/blockchain_tests.cpp +++ b/src/test/blockchain_tests.cpp @@ -147,6 +147,11 @@ BOOST_FIXTURE_TEST_CASE(invalidate_block, TestChain100Setup) WITH_LOCK(::cs_main, assert(pindex->nStatus & BLOCK_FAILED_CHILD)); pindex = pindex->pprev; } + + // don't mark already invalidated block (orig_tip is BLOCK_FAILED_CHILD) with BLOCK_FAILED_VALID again + m_node.chainman->ActiveChainstate().InvalidateBlock(state, orig_tip); + WITH_LOCK(::cs_main, assert(orig_tip->nStatus & BLOCK_FAILED_CHILD)); + WITH_LOCK(::cs_main, assert((orig_tip->nStatus & BLOCK_FAILED_VALID) == 0)); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/validation.cpp b/src/validation.cpp index 0714fb7f437..6185556c02c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3779,11 +3779,13 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde return false; } - // Mark pindex (or the last disconnected block) as invalid, even when it never was in the main chain - to_mark_failed->nStatus |= BLOCK_FAILED_VALID; - m_blockman.m_dirty_blockindex.insert(to_mark_failed); - setBlockIndexCandidates.erase(to_mark_failed); - m_chainman.m_failed_blocks.insert(to_mark_failed); + // Mark pindex as invalid if it never was in the main chain + if (!pindex_was_in_chain && !(pindex->nStatus & BLOCK_FAILED_MASK)) { + pindex->nStatus |= BLOCK_FAILED_VALID; + m_blockman.m_dirty_blockindex.insert(pindex); + setBlockIndexCandidates.erase(pindex); + m_chainman.m_failed_blocks.insert(pindex); + } // If any new blocks somehow arrived while we were disconnecting // (above), then the pre-calculation of what should go into