diff --git a/src/chain.h b/src/chain.h index 56771486d..aa039e83f 100644 --- a/src/chain.h +++ b/src/chain.h @@ -113,10 +113,9 @@ enum BlockStatus: uint32_t { //! Scripts & signatures ok. Implies all parents are also at least SCRIPTS. BLOCK_VALID_SCRIPTS = 5, - // flag to check if contextual check block has passed in Accept block, if it has not check at connect block. - BLOCK_VALID_CONTEXT = 6, - //! All validity bits. + //! NOTE: the levels above are sequential VALUES occupying this 3-bit field, not independent + //! bits, so any flag stored in nStatus must live entirely outside this mask. BLOCK_VALID_MASK = BLOCK_VALID_HEADER | BLOCK_VALID_TREE | BLOCK_VALID_TRANSACTIONS | BLOCK_VALID_CHAIN | BLOCK_VALID_SCRIPTS, @@ -129,9 +128,29 @@ enum BlockStatus: uint32_t { BLOCK_FAILED_MASK = BLOCK_FAILED_VALID | BLOCK_FAILED_CHILD, BLOCK_ACTIVATES_UPGRADE = 128, //! block activates a network upgrade - BLOCK_IN_TMPFILE = 256 + BLOCK_IN_TMPFILE = 256, + + //! ContextualCheckBlock already passed in AcceptBlock, so ConnectBlock may skip re-running it. + //! Was 6 until v1.3.0, which put it INSIDE BLOCK_VALID_MASK (1|2|3|4|5 == 7): `nStatus |= + //! BLOCK_VALID_CONTEXT` then overwrote the validity level rather than setting a flag, so a + //! block that was only written to disk read back as BLOCK_VALID_SCRIPTS and every later + //! RaiseValidity() silently no-opped. Detected by CheckBlockIndex's "CHAIN valid implies all + //! parents are CHAIN valid" assert, which aborts any node doing out-of-order block download + //! (regtest only, where fDefaultConsistencyChecks is true). Validity was only ever inflated, + //! never deflated, so ConnectBlock's full validation was never skipped -- see git history. + //! Legacy block indexes still carry the polluted low bits; they resolve on reindex, and until + //! then simply re-run the contextual check they used to skip. + BLOCK_VALID_CONTEXT = 512 }; +//! The validity level is a small integer packed into BLOCK_VALID_MASK, so every other nStatus flag +//! must be disjoint from it. Enforced here so this class of bug cannot be reintroduced silently. +static_assert((BLOCK_VALID_CONTEXT & BLOCK_VALID_MASK) == 0, "BLOCK_VALID_CONTEXT overlaps the validity-level field"); +static_assert((BLOCK_HAVE_MASK & BLOCK_VALID_MASK) == 0, "BLOCK_HAVE_MASK overlaps the validity-level field"); +static_assert((BLOCK_FAILED_MASK & BLOCK_VALID_MASK) == 0, "BLOCK_FAILED_MASK overlaps the validity-level field"); +static_assert((BLOCK_ACTIVATES_UPGRADE & BLOCK_VALID_MASK) == 0, "BLOCK_ACTIVATES_UPGRADE overlaps the validity-level field"); +static_assert((BLOCK_IN_TMPFILE & BLOCK_VALID_MASK) == 0, "BLOCK_IN_TMPFILE overlaps the validity-level field"); + //! Short-hand for the highest consensus validity we implement. //! Blocks with this validity are assumed to satisfy all consensus rules. static const BlockStatus BLOCK_VALID_CONSENSUS = BLOCK_VALID_SCRIPTS;