highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-10 22:18:54 +01:00
Code Issues Packages Projects Releases Wiki Activity

Move CheckBlockIndex() from Chainstate to ChainstateManager

Browse Source 
Also rewrite CheckBlockIndex() to perform tests on all chainstates.

This increases sanity-check coverage, as any place in our code where we were
invoke CheckBlockIndex() on a single chainstate will now invoke the sanity
checks on all chainstates.

This change also tightens up the checks on setBlockIndexCandidates and
mapBlocksUnlinked, to more precisely match what we aim for even in the presence
of assumed-valid blocks.
This commit is contained in:
Suhas Daftuar
2023-06-18 12:33:58 -04:00
parent 0ce805b632
commit 3556b85022
2 changed files with 55 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

86
src/validation.cpp
View File

@@ -3193,7 +3193,8 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr<
// that the best block hash is non-null. // that the best block hash is non-null.
if (m_chainman.m_interrupt) break; if (m_chainman.m_interrupt) break;
} while (pindexNewTip != pindexMostWork); } while (pindexNewTip != pindexMostWork);
CheckBlockIndex();
m_chainman.CheckBlockIndex();
// Write changes periodically to disk, after relay. // Write changes periodically to disk, after relay.
if (!FlushStateToDisk(state, FlushStateMode::PERIODIC)) { if (!FlushStateToDisk(state, FlushStateMode::PERIODIC)) {
@@ -3339,7 +3340,7 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde
to_mark_failed = invalid_walk_tip; to_mark_failed = invalid_walk_tip;
} }
CheckBlockIndex(); m_chainman.CheckBlockIndex();
{ {
LOCK(cs_main); LOCK(cs_main);
@@ -3882,7 +3883,7 @@ bool ChainstateManager::ProcessNewBlockHeaders(const std::vector<CBlockHeader>&
for (const CBlockHeader& header : headers) { for (const CBlockHeader& header : headers) {
CBlockIndex *pindex = nullptr; // Use a temp pindex instead of ppindex to avoid a const_cast CBlockIndex *pindex = nullptr; // Use a temp pindex instead of ppindex to avoid a const_cast
bool accepted{AcceptBlockHeader(header, state, &pindex, min_pow_checked)}; bool accepted{AcceptBlockHeader(header, state, &pindex, min_pow_checked)};
ActiveChainstate().CheckBlockIndex(); CheckBlockIndex();
if (!accepted) { if (!accepted) {
return false; return false;
@@ -3940,7 +3941,7 @@ bool ChainstateManager::AcceptBlock(const std::shared_ptr<const CBlock>& pblock,
CBlockIndex *&pindex = ppindex ? *ppindex : pindexDummy; CBlockIndex *&pindex = ppindex ? *ppindex : pindexDummy;
bool accepted_header{AcceptBlockHeader(block, state, &pindex, min_pow_checked)}; bool accepted_header{AcceptBlockHeader(block, state, &pindex, min_pow_checked)};
ActiveChainstate().CheckBlockIndex(); CheckBlockIndex();
if (!accepted_header) if (!accepted_header)
return false; return false;
@@ -4017,7 +4018,7 @@ bool ChainstateManager::AcceptBlock(const std::shared_ptr<const CBlock>& pblock,
// background validation yet). // background validation yet).
ActiveChainstate().FlushStateToDisk(state, FlushStateMode::NONE); ActiveChainstate().FlushStateToDisk(state, FlushStateMode::NONE);
ActiveChainstate().CheckBlockIndex(); CheckBlockIndex();
return true; return true;
} }
@@ -4676,9 +4677,9 @@ void ChainstateManager::LoadExternalBlockFile(
LogPrintf("Loaded %i blocks from external file in %dms\n", nLoaded, Ticks<std::chrono::milliseconds>(SteadyClock::now() - start)); LogPrintf("Loaded %i blocks from external file in %dms\n", nLoaded, Ticks<std::chrono::milliseconds>(SteadyClock::now() - start));
} }
void Chainstate::CheckBlockIndex() void ChainstateManager::CheckBlockIndex()
{ {
if (!m_chainman.ShouldCheckBlockIndex()) { if (!ShouldCheckBlockIndex()) {
return; return;
} }
@@ -4687,7 +4688,7 @@ void Chainstate::CheckBlockIndex()
// During a reindex, we read the genesis block and call CheckBlockIndex before ActivateBestChain, // During a reindex, we read the genesis block and call CheckBlockIndex before ActivateBestChain,
// so we have the genesis block in m_blockman.m_block_index but no active chain. (A few of the // so we have the genesis block in m_blockman.m_block_index but no active chain. (A few of the
// tests when iterating the block tree require that m_chain has been initialized.) // tests when iterating the block tree require that m_chain has been initialized.)
if (m_chain.Height() < 0) { if (ActiveChain().Height() < 0) {
assert(m_blockman.m_block_index.size() <= 1); assert(m_blockman.m_block_index.size() <= 1);
return; return;
} }
@@ -4751,8 +4752,12 @@ void Chainstate::CheckBlockIndex()
// Begin: actual consistency checks. // Begin: actual consistency checks.
if (pindex->pprev == nullptr) { if (pindex->pprev == nullptr) {
// Genesis block checks. // Genesis block checks.
assert(pindex->GetBlockHash() == m_chainman.GetConsensus().hashGenesisBlock); // Genesis block's hash must match. assert(pindex->GetBlockHash() == GetConsensus().hashGenesisBlock); // Genesis block's hash must match.
assert(pindex == m_chain.Genesis()); // The current active chain's genesis block must be this block. for (auto c : GetAll()) {
if (c->m_chain.Genesis() != nullptr) {
assert(pindex == c->m_chain.Genesis()); // The chain's genesis block must be this block.
}
}
} }
if (!pindex->HaveTxsDownloaded()) assert(pindex->nSequenceId <= 0); // nSequenceId can't be set positive for blocks that aren't linked (negative is used for preciousblock) if (!pindex->HaveTxsDownloaded()) assert(pindex->nSequenceId <= 0); // nSequenceId can't be set positive for blocks that aren't linked (negative is used for preciousblock)
// VALID_TRANSACTIONS is equivalent to nTx > 0 for all nodes (whether or not pruning has occurred). // VALID_TRANSACTIONS is equivalent to nTx > 0 for all nodes (whether or not pruning has occurred).
@@ -4798,27 +4803,32 @@ void Chainstate::CheckBlockIndex()
// Checks for not-invalid blocks. // Checks for not-invalid blocks.
assert((pindex->nStatus & BLOCK_FAILED_MASK) == 0); // The failed mask cannot be set for blocks without invalid parents. assert((pindex->nStatus & BLOCK_FAILED_MASK) == 0); // The failed mask cannot be set for blocks without invalid parents.
} }
if (!CBlockIndexWorkComparator()(pindex, m_chain.Tip()) && pindexFirstNeverProcessed == nullptr) { // Chainstate-specific checks on setBlockIndexCandidates
if (pindexFirstInvalid == nullptr) { for (auto c : GetAll()) {
const bool is_active = this == &m_chainman.ActiveChainstate(); if (c->m_chain.Tip() == nullptr) continue;
if (!CBlockIndexWorkComparator()(pindex, c->m_chain.Tip()) && pindexFirstNeverProcessed == nullptr) {
// If this block sorts at least as good as the current tip and if (pindexFirstInvalid == nullptr) {
// is valid and we have all data for its parents, it must be in const bool is_active = c == &ActiveChainstate();
// setBlockIndexCandidates. m_chain.Tip() must also be there // If this block sorts at least as good as the current tip and
// even if some data has been pruned. // is valid and we have all data for its parents, it must be in
// // setBlockIndexCandidates. m_chain.Tip() must also be there
// Don't perform this check for the background chainstate since // even if some data has been pruned.
// its setBlockIndexCandidates shouldn't have some entries (i.e. those past the //
// snapshot block) which do exist in the block index for the active chainstate. if ((pindexFirstMissing == nullptr || pindex == c->m_chain.Tip())) {
if (is_active && (pindexFirstMissing == nullptr || pindex == m_chain.Tip())) { // The active chainstate should always have this block
assert(setBlockIndexCandidates.count(pindex)); // as a candidate, but a background chainstate should
// only have it if it is an ancestor of the snapshot base.
if (is_active || GetSnapshotBaseBlock()->GetAncestor(pindex->nHeight) == pindex) {
assert(c->setBlockIndexCandidates.count(pindex));
}
}
// If some parent is missing, then it could be that this block was in
// setBlockIndexCandidates but had to be removed because of the missing data.
// In this case it must be in m_blocks_unlinked -- see test below.
} }
// If some parent is missing, then it could be that this block was in } else { // If this block sorts worse than the current tip or some ancestor's block has never been seen, it cannot be in setBlockIndexCandidates.
// setBlockIndexCandidates but had to be removed because of the missing data. assert(c->setBlockIndexCandidates.count(pindex) == 0);
// In this case it must be in m_blocks_unlinked -- see test below.
} }
} else { // If this block sorts worse than the current tip or some ancestor's block has never been seen, it cannot be in setBlockIndexCandidates.
assert(setBlockIndexCandidates.count(pindex) == 0);
} }
// Check whether this block is in m_blocks_unlinked. // Check whether this block is in m_blocks_unlinked.
std::pair<std::multimap<CBlockIndex*,CBlockIndex*>::iterator,std::multimap<CBlockIndex*,CBlockIndex*>::iterator> rangeUnlinked = m_blockman.m_blocks_unlinked.equal_range(pindex->pprev); std::pair<std::multimap<CBlockIndex*,CBlockIndex*>::iterator,std::multimap<CBlockIndex*,CBlockIndex*>::iterator> rangeUnlinked = m_blockman.m_blocks_unlinked.equal_range(pindex->pprev);
@@ -4846,16 +4856,16 @@ void Chainstate::CheckBlockIndex()
// - we tried switching to that descendant but were missing // - we tried switching to that descendant but were missing
// data for some intermediate block between m_chain and the // data for some intermediate block between m_chain and the
// tip. // tip.
// So if this block is itself better than m_chain.Tip() and it wasn't in // So if this block is itself better than any m_chain.Tip() and it wasn't in
// setBlockIndexCandidates, then it must be in m_blocks_unlinked. // setBlockIndexCandidates, then it must be in m_blocks_unlinked.
if (!CBlockIndexWorkComparator()(pindex, m_chain.Tip()) && setBlockIndexCandidates.count(pindex) == 0) { for (auto c : GetAll()) {
if (pindexFirstInvalid == nullptr && pindexFirstAssumeValid == nullptr) { const bool is_active = c == &ActiveChainstate();
// If this is a chain based on an assumeutxo snapshot, then if (!CBlockIndexWorkComparator()(pindex, c->m_chain.Tip()) && c->setBlockIndexCandidates.count(pindex) == 0) {
// this block could either be in mapBlocksUnlinked or in if (pindexFirstInvalid == nullptr) {
// setBlockIndexCandidates; it may take a call to if (is_active || GetSnapshotBaseBlock()->GetAncestor(pindex->nHeight) == pindex) {
// FindMostWorkChain() to figure out whether all the blocks assert(foundInUnlinked);
// between the tip and this block are actually available. }
assert(foundInUnlinked); }
} }
} }
} }

14
src/validation.h
View File

@@ -706,13 +706,6 @@ public:
/** Find the last common block of this chain and a locator. */ /** Find the last common block of this chain and a locator. */
const CBlockIndex* FindForkInGlobalIndex(const CBlockLocator& locator) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); const CBlockIndex* FindForkInGlobalIndex(const CBlockLocator& locator) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/**
* Make various assertions about the state of the block index.
*
* By default this only executes fully when using the Regtest chain; see: m_options.check_block_index.
*/
void CheckBlockIndex();
/** Load the persisted mempool from disk */ /** Load the persisted mempool from disk */
void LoadMempool(const fs::path& load_path, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); void LoadMempool(const fs::path& load_path, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen);
@@ -927,6 +920,13 @@ public:
const uint256& AssumedValidBlock() const { return *Assert(m_options.assumed_valid_block); } const uint256& AssumedValidBlock() const { return *Assert(m_options.assumed_valid_block); }
kernel::Notifications& GetNotifications() const { return m_options.notifications; }; kernel::Notifications& GetNotifications() const { return m_options.notifications; };
/**
* Make various assertions about the state of the block index.
*
* By default this only executes fully when using the Regtest chain; see: m_options.check_block_index.
*/
void CheckBlockIndex();
/** /**
* Alias for ::cs_main. * Alias for ::cs_main.
* Should be used in new code to make it easier to make ::cs_main a member * Should be used in new code to make it easier to make ::cs_main a member