diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index fe06499012f..67c90a00731 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -618,6 +618,94 @@ BOOST_FIXTURE_TEST_CASE(loadblockindex_invalid_descendants, TestChain100Setup) BOOST_CHECK(child->nStatus & BLOCK_FAILED_VALID); } +//! Verify that ReconsiderBlock clears failure flags for the target block, its ancestors, and descendants, +//! but not for sibling forks that diverge from a shared ancestor. +BOOST_FIXTURE_TEST_CASE(invalidate_block_and_reconsider_fork, TestChain100Setup) +{ + ChainstateManager& chainman = *Assert(m_node.chainman); + Chainstate& chainstate = chainman.ActiveChainstate(); + + // we have a chain of 100 blocks: genesis(0) <- ... <- block98 <- block99 <- block100 + CBlockIndex* block98; + CBlockIndex* block99; + CBlockIndex* block100; + { + LOCK(chainman.GetMutex()); + block98 = chainman.ActiveChain()[98]; + block99 = chainman.ActiveChain()[99]; + block100 = chainman.ActiveChain()[100]; + } + + // create the following block constellation: + // genesis(0) <- ... <- block98 <- block99 <- block100 + // <- block99' <- block100' + // by temporarily invalidating block99. the chain tip now falls to block98, + // mine 2 new blocks on top of block 98 (block99' and block100') and then restore block99 and block 100. + BlockValidationState state; + BOOST_REQUIRE(chainstate.InvalidateBlock(state, block99)); + BOOST_REQUIRE(WITH_LOCK(cs_main, return chainman.ActiveChain().Tip()) == block98); + CScript coinbase_script = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG; + for (int i = 0; i < 2; ++i) { + CreateAndProcessBlock({}, coinbase_script); + } + const CBlockIndex* fork_block99; + const CBlockIndex* fork_block100; + { + LOCK(chainman.GetMutex()); + fork_block99 = chainman.ActiveChain()[99]; + BOOST_REQUIRE(fork_block99->pprev == block98); + fork_block100 = chainman.ActiveChain()[100]; + BOOST_REQUIRE(fork_block100->pprev == fork_block99); + } + // Restore original block99 and block100 + { + LOCK(chainman.GetMutex()); + chainstate.ResetBlockFailureFlags(block99); + chainman.RecalculateBestHeader(); + } + chainstate.ActivateBestChain(state); + BOOST_REQUIRE(WITH_LOCK(cs_main, return chainman.ActiveChain().Tip()) == block100); + + { + LOCK(chainman.GetMutex()); + BOOST_CHECK(!(block100->nStatus & BLOCK_FAILED_VALID)); + BOOST_CHECK(!(block99->nStatus & BLOCK_FAILED_VALID)); + BOOST_CHECK(!(fork_block100->nStatus & BLOCK_FAILED_VALID)); + BOOST_CHECK(!(fork_block99->nStatus & BLOCK_FAILED_VALID)); + } + + // Invalidate block98 + BOOST_REQUIRE(chainstate.InvalidateBlock(state, block98)); + + { + LOCK(chainman.GetMutex()); + // block98 and all descendants of block98 are marked BLOCK_FAILED_VALID + BOOST_CHECK(block98->nStatus & BLOCK_FAILED_VALID); + BOOST_CHECK(block99->nStatus & BLOCK_FAILED_VALID); + BOOST_CHECK(block100->nStatus & BLOCK_FAILED_VALID); + BOOST_CHECK(fork_block99->nStatus & BLOCK_FAILED_VALID); + BOOST_CHECK(fork_block100->nStatus & BLOCK_FAILED_VALID); + } + + // Reconsider block99. ResetBlockFailureFlags clears BLOCK_FAILED_VALID from + // block99 and its ancestors (block98) and descendants (block100) + // but NOT from block99' and block100' (not a direct ancestor/descendant) + { + LOCK(chainman.GetMutex()); + chainstate.ResetBlockFailureFlags(block99); + chainman.RecalculateBestHeader(); + } + chainstate.ActivateBestChain(state); + { + LOCK(chainman.GetMutex()); + BOOST_CHECK(!(block98->nStatus & BLOCK_FAILED_VALID)); + BOOST_CHECK(!(block99->nStatus & BLOCK_FAILED_VALID)); + BOOST_CHECK(!(block100->nStatus & BLOCK_FAILED_VALID)); + BOOST_CHECK(fork_block99->nStatus & BLOCK_FAILED_VALID); + BOOST_CHECK(fork_block100->nStatus & BLOCK_FAILED_VALID); + } +} + //! Ensure that snapshot chainstate can be loaded when found on disk after a //! restart, and that new blocks can be connected to both chainstates. BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup) diff --git a/src/validation.cpp b/src/validation.cpp index 2b0de23db3b..ff2fdea8d20 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3127,9 +3127,8 @@ CBlockIndex* Chainstate::FindMostWorkChain() // Check whether all blocks on the path between the currently active chain and the candidate are valid. // Just going until the active chain is an optimization, as we know all blocks in it are valid already. - CBlockIndex *pindexTest = pindexNew; bool fInvalidAncestor = false; - while (pindexTest && !m_chain.Contains(pindexTest)) { + for (CBlockIndex *pindexTest = pindexNew; pindexTest && !m_chain.Contains(pindexTest); pindexTest = pindexTest->pprev) { assert(pindexTest->HaveNumChainTxs() || pindexTest->nHeight == 0); // Pruned nodes may have entries in setBlockIndexCandidates for @@ -3143,27 +3142,21 @@ CBlockIndex* Chainstate::FindMostWorkChain() if (fFailedChain && (m_chainman.m_best_invalid == nullptr || pindexNew->nChainWork > m_chainman.m_best_invalid->nChainWork)) { m_chainman.m_best_invalid = pindexNew; } - CBlockIndex *pindexFailed = pindexNew; // Remove the entire chain from the set. - while (pindexTest != pindexFailed) { - if (fFailedChain) { - pindexFailed->nStatus |= BLOCK_FAILED_VALID; - m_blockman.m_dirty_blockindex.insert(pindexFailed); - } else if (fMissingData) { - // If we're missing data, then add back to m_blocks_unlinked, - // so that if the block arrives in the future we can try adding - // to setBlockIndexCandidates again. + for (CBlockIndex *pindexFailed = pindexNew; pindexFailed != pindexTest; pindexFailed = pindexFailed->pprev) { + if (fMissingData && !fFailedChain) { + // If we're missing data and not a descendant of an invalid block, + // then add back to m_blocks_unlinked, so that if the block arrives in the future + // we can try adding to setBlockIndexCandidates again. m_blockman.m_blocks_unlinked.insert( std::make_pair(pindexFailed->pprev, pindexFailed)); } setBlockIndexCandidates.erase(pindexFailed); - pindexFailed = pindexFailed->pprev; } setBlockIndexCandidates.erase(pindexTest); fInvalidAncestor = true; break; } - pindexTest = pindexTest->pprev; } if (!fInvalidAncestor) return pindexNew;