diff options
author | Suhas Daftuar <sdaftuar@gmail.com> | 2019-09-10 13:58:41 -0400 |
---|---|---|
committer | Suhas Daftuar <sdaftuar@gmail.com> | 2019-09-10 14:54:50 -0400 |
commit | 2a4e60b48261d3f0ec3d85f97af998ef989134e0 (patch) | |
tree | 9c80a637185174eef64b1c77d1004dc926d5e0ad /src | |
parent | 1985c4efda56b48f6f9c04f39d69268ee8f0b40a (diff) | |
download | bitcoin-2a4e60b48261d3f0ec3d85f97af998ef989134e0.tar.xz |
Fix block index inconsistency in InvalidateBlock()
Previously, we could release cs_main while leaving the block index in a state
that would fail CheckBlockIndex, because setBlockIndexCandidates was not being
fully populated before releasing cs_main.
Diffstat (limited to 'src')
-rw-r--r-- | src/validation.cpp | 54 |
1 files changed, 52 insertions, 2 deletions
diff --git a/src/validation.cpp b/src/validation.cpp index d470fd5b6e..62de023e2d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2778,6 +2778,38 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c bool pindex_was_in_chain = false; int disconnected = 0; + // We do not allow ActivateBestChain() to run while InvalidateBlock() is + // running, as that could cause the tip to change while we disconnect + // blocks. + LOCK(m_cs_chainstate); + + // We'll be acquiring and releasing cs_main below, to allow the validation + // callbacks to run. However, we should keep the block index in a + // consistent state as we disconnect blocks -- in particular we need to + // add equal-work blocks to setBlockIndexCandidates as we disconnect. + // To avoid walking the block index repeatedly in search of candidates, + // build a map once so that we can look up candidate blocks by chain + // work as we go. + std::multimap<const arith_uint256, CBlockIndex *> candidate_blocks_by_work; + + { + LOCK(cs_main); + for (const auto& entry : m_blockman.m_block_index) { + CBlockIndex *candidate = entry.second; + // We don't need to put anything in our active chain into the + // multimap, because those candidates will be found and considered + // as we disconnect. + // Instead, consider only non-active-chain blocks that have at + // least as much work as where we expect the new tip to end up. + if (!m_chain.Contains(candidate) && + !CBlockIndexWorkComparator()(candidate, pindex->pprev) && + candidate->IsValid(BLOCK_VALID_TRANSACTIONS) && + candidate->HaveTxsDownloaded()) { + candidate_blocks_by_work.insert(std::make_pair(candidate->nChainWork, candidate)); + } + } + } + // Disconnect (descendants of) pindex, and mark them invalid. while (true) { if (ShutdownRequested()) break; @@ -2820,11 +2852,24 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c setDirtyBlockIndex.insert(to_mark_failed); } + // Add any equal or more work headers to setBlockIndexCandidates + auto candidate_it = candidate_blocks_by_work.lower_bound(invalid_walk_tip->pprev->nChainWork); + while (candidate_it != candidate_blocks_by_work.end()) { + if (!CBlockIndexWorkComparator()(candidate_it->second, invalid_walk_tip->pprev)) { + setBlockIndexCandidates.insert(candidate_it->second); + candidate_it = candidate_blocks_by_work.erase(candidate_it); + } else { + ++candidate_it; + } + } + // Track the last disconnected block, so we can correct its BLOCK_FAILED_CHILD status in future // iterations, or, if it's the last one, call InvalidChainFound on it. to_mark_failed = invalid_walk_tip; } + CheckBlockIndex(chainparams.GetConsensus()); + { LOCK(cs_main); if (m_chain.Contains(to_mark_failed)) { @@ -2838,8 +2883,13 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c setBlockIndexCandidates.erase(to_mark_failed); m_blockman.m_failed_blocks.insert(to_mark_failed); - // The resulting new best tip may not be in setBlockIndexCandidates anymore, so - // add it again. + // If any new blocks somehow arrived while we were disconnecting + // (above), then the pre-calculation of what should go into + // setBlockIndexCandidates may have missed entries. This would + // technically be an inconsistency in the block index, but if we clean + // it up here, this should be an essentially unobservable error. + // Loop back over all block index entries and add any missing entries + // to setBlockIndexCandidates. BlockMap::iterator it = m_blockman.m_block_index.begin(); while (it != m_blockman.m_block_index.end()) { if (it->second->IsValid(BLOCK_VALID_TRANSACTIONS) && it->second->HaveTxsDownloaded() && !setBlockIndexCandidates.value_comp()(it->second, m_chain.Tip())) { |