DragonX/dragonx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 3 Wiki Activity

consensus: stop BLOCK_VALID_CONTEXT overwriting the block validity level

Browse Source 
BLOCK_VALID_CONTEXT was 6. The validity levels above it are sequential VALUES
packed into a 3-bit field, not independent bits, so BLOCK_VALID_MASK is
1|2|3|4|5 == 7 and the value 6 sat entirely inside it. `pindex->nStatus |=
BLOCK_VALID_CONTEXT` (main.cpp:5616, and :3285) therefore did not set a flag --
it overwrote the validity level.

Consequences, all long-standing:
  - A header-only block raised to BLOCK_VALID_TREE(2) became 2|6 == 6, which
    reads as >= BLOCK_VALID_CHAIN(4) and >= BLOCK_VALID_SCRIPTS(5). A block
    merely written to disk reported full script validity.
  - Every later RaiseValidity() silently no-opped, because 6 >= every level.
    ConnectBlock's RaiseValidity(BLOCK_VALID_SCRIPTS) was a permanent no-op.
  - CheckBlockIndex's "CHAIN valid implies all parents are CHAIN valid" invariant
    was violated whenever a stored block sat above a still-header-only ancestor,
    i.e. ordinary out-of-order parallel block download. fDefaultConsistencyChecks
    is true only for regtest, so the abort was regtest-only -- but the garbled
    index is written identically on mainnet, where only the detection is off.

Not a v1.3.0 regression: introduced upstream in Komodo fa309e5b0 (2019-04-02),
inherited via Hush, and byte-identical in v1.0.3, which the production network
runs today. Validity was only ever INFLATED, never deflated, so no valid block
was rejected and no invalid block skipped validation -- ConnectBlock's CheckBlock
and full script/proof verification always ran. The user-visible effects were
misreports: getchaintips labelling never-connected forks "valid-fork",
submitblock answering "duplicate" for unvalidated blocks, and ProcessGetData
serving them. The material cost was to QA: multi-node regtest tests aborted the
syncing node at random, making the rpc-test suite unusable.

Moves the flag to 512, the next free bit above BLOCK_IN_TMPFILE(256), and adds
static_asserts that every nStatus flag is disjoint from BLOCK_VALID_MASK so this
cannot be reintroduced silently. nStatus is serialized as VARINT, so the wider
value needs no format change.

Verified under gdb on regtest. A connected block's nStatus:
  before   0x1e  validity field 6            (measured on the previous binary)
  after   0x21d  validity field 5 = SCRIPTS, context bit set
wallet_sapling.py, which aborted the syncing node deterministically, now runs to
completion with no assert.

No migration: the original level is unrecoverable from a polluted entry (1|6,
3|6 and 5|6 all give 7; 2|6 and 4|6 both give 6), and the only safe guess is
downward, which would risk revalidation work on a 3.25M-block index for a
cosmetic gain. Legacy entries simply have the flag bit clear, so they re-run the
contextual check they previously skipped -- more checking, not less -- and
resolve on any reindex.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FU87LdsJZiZkfq1eXubpeo
This commit is contained in:
DanS
2026-08-31 02:12:17 -05:00
parent c5fde12485
commit 6d282db216
1 changed files with 23 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
src/chain.h
View File

@@ -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;