foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-03 09:56:38 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#29299: validation: fix misleading checkblockindex comments

Browse source 
9819db4cca validation: move nChainTx assert down in CheckBlockIndex (Martin Zumsande)
033477dba6 doc: fix checkblockindex comments (Martin Zumsande)

Pull request description:

  The two assumptions there were described as test-only, which has led to confusion whether they should exist.
  However, they are necessary in general, as the changed comment explains - without them, the check would fail everywhere where it is enabled.
  The second commit moves this assert down to the other checks.

  Closes #29261

ACKs for top commit:
  maflcko:
    ACK 9819db4cca 🌦
  naumenkogs:
    ACK 9819db4cca
  ryanofsky:
    Code review ACK 9819db4cca. Thanks for figuring this issue out and fixing it. Would suggest changing pr name from "improve comments" to "fix misleading comments" since previous comments were wrong about the reasons the conditions are needed.

Tree-SHA512: 3f77791253eb0c97f8153dd8ae1c567f43f6387ea7a53efea94817463c672a4e11d548aa7eff62235346ff0713ff4d6fe08f9ec50d0c30a1e6b6d27b9918b419
This commit is contained in:
glozow 2024-01-30 12:03:10 +00:00
commit 7005766492
No known key found for this signature in database
GPG key ID: BA03F4DBE0C63FB4
1 changed files with 9 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
src/validation.cpp
View file

@ -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)
// For testing, allow transaction counts to be completely unset.
|| (pindex->nChainTx == 0 && pindex->nTx == 0)
// For testing, allow this nChainTx to be unset if previous is also unset.
|| (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;