block: separate parsing a header from validating it (stacked on #392) - #393
Open
xanimo wants to merge 2 commits into
Open
block: separate parsing a header from validating it (stacked on #392)#393xanimo wants to merge 2 commits into
xanimo wants to merge 2 commits into
Conversation
This was referenced Aug 4, 2026
dogecoin_block_header carried an auxpow member holding only the validation hook -- check, ctx and is -- never the proof itself. The proof was built into a local dogecoin_auxpow_block inside dogecoin_block_header_deserialize and freed at cleanup, so by the time the caller had its header the parent coinbase, merkle branches and parent header were gone. dogecoin_block_header_copy made that worse by looking complete: it copied the three hook fields, so a copied merge-mined header was silently missing its proof with nothing to indicate it. Anything needing the proof after the parse therefore had to re-parse the wire bytes, or retain them separately. That is why BIP152 keeps a header_raw span on dogecoin_compact_block: not because a compact block wants raw bytes, but because the parsed header could not answer for itself. Add dogecoin_auxpow_payload, owned by the header. The deserializer moves the parsed fields onto it and nulls them on the scratch block, so ownership transfers rather than duplicating; _free releases it; _copy deep-copies it. The payload deliberately has no back-pointer to its header, unlike dogecoin_auxpow_block, which owns both its header and its parent_header and frees them. A header holding one of those would own the thing that owns it. The payload owns only parent_header, a plain 80-byte header whose own payload is NULL, so ownership terminates. auxpow.check / auxpow.ctx are untouched. That is a validation hook whose context the caller supplies at call time -- validation.c passes the block directly -- not a reference to this data. dogecoin_block_header_copy assigns the copied payload rather than freeing what dest held. Every other field there is a plain overwrite: the function treats dest as raw memory, and callers pass uninitialised stack headers to it -- net_tests.c does, through dogecoin_block_header_deserialize. Freeing dest->auxpow_payload dereferenced whatever the stack contained, which is a SEGV on eight platforms and the reason ASAN caught this and a local build did not: every local caller happened to use dogecoin_block_header_new, where the pointer is NULL. A dest that already owns a payload is the caller's to release, as with every other member. The test extends the height-371338 vector: the proof survives the parse, a copy owns an independent one, and freeing the source leaves the copy intact, which it can only do if nothing is shared. Disabling just the deep copy fails it at line 351. WITH_NET=ON with -DBUILD_SHARED_LIBS=1: 78/78. WITH_NET=OFF: 72/72. ASAN+UBSAN: 78/78, no leaks.
check_auxpow ran inside dogecoin_block_header_deserialize, so every caller that wanted a header's fields paid for scrypt work over the parent chain during parsing, before any peer-level gating could decide whether the message was worth the effort. Core defers this to CheckBlock. Split into dogecoin_block_header_parse, which reads the base fields and, when version bit 0x100 is set, the AuxPoW proof, and dogecoin_block_header_validate, which runs check_auxpow and fills chainwork. The split is deliberately this way round. Making dogecoin_block_header_ deserialize the pure parse and adding a _checked variant would silently stop verifying proof of work for every existing caller of the name, with no compile error anywhere to catch it. Wrong direction for a symbol whose job is validation. Instead the existing name keeps its signature and its behaviour -- it is now parse followed by validate -- and the opt-out is explicit at the call site. deserialize_dogecoin_auxpow_block is split the same way and keeps its signature: parse_dogecoin_auxpow_fields does the reading, the public function adds the check. Validation needs the proof to still exist after parsing, which is what the preceding commit made possible. check_auxpow takes a dogecoin_auxpow_block, so validate builds one that borrows from the header and its payload. It is never freed: dogecoin_auxpow_block_free would take the header and parent_header with it, which is the ownership tangle the payload type exists to avoid. A header with no AuxPoW validates trivially. Its proof of work is over the 80 base bytes and belongs to the caller -- headersdb_file.c already runs check_pow itself for that case and fills chainwork. That asymmetry is unchanged. Verified by stubbing check_auxpow to always fail: dogecoin_block_header_ parse still succeeds, dogecoin_block_header_validate does not. A refactor that merely moved the call would fail both. The test also asserts the deferred chainwork equals what the parse-and-validate path computes, so the split does not change the answer. WITH_NET=ON with -DBUILD_SHARED_LIBS=1: 78/78. WITH_NET=OFF: 72/72. ASAN+UBSAN with leak detection: 78/78, clean.
xanimo
force-pushed
the
0.1.5-dev-auxpow-parse-split
branch
from
August 5, 2026 21:04
a600216 to
acd2e02
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #392. Review only the top commit.
check_auxpowran insidedogecoin_block_header_deserialize, so every callerthat wanted a header's fields paid for scrypt work over the parent chain
during parsing, before any peer-level gating could decide whether the
message was worth the effort. Core defers this to
CheckBlock.Split into:
dogecoin_block_header_parse— reads the base fields and, when versionbit
0x100is set, the AuxPoW proof. No validation.dogecoin_block_header_validate— runscheck_auxpow, fillschainwork.The direction matters
Making
dogecoin_block_header_deserializethe pure parse and adding a_checkedvariant would silently stop verifying proof of work for everyexisting caller of the name, with no compile error anywhere to catch it. Wrong
direction for a symbol whose job is validation.
So the existing name keeps its signature and its behaviour — it is now parse
followed by validate — and the opt-out is explicit at the call site.
deserialize_dogecoin_auxpow_blockis split the same way and also keeps itssignature.
Why #392 had to land first
Validation needs the proof to still exist after parsing.
check_auxpowtakes adogecoin_auxpow_block, sovalidatebuilds one that borrows from theheader and its payload. It is never freed —
dogecoin_auxpow_block_freewouldtake the header and
parent_headerwith it, which is exactly the ownershiptangle the payload type exists to avoid.
Unchanged asymmetry
A header with no AuxPoW validates trivially. Its proof of work is over the 80
base bytes and belongs to the caller —
headersdb_file.c:342already runscheck_powitself for that case and fillschainwork. Untouched.Verification
Stubbed
check_auxpowto always fail:dogecoin_block_header_parsedogecoin_block_header_validateA refactor that merely moved the call would fail both. The test also asserts the
deferred
chainworkequals what the parse-and-validate path computes, so thesplit does not change the answer.
WITH_NET=ONwith-DBUILD_SHARED_LIBS=1: 78/78.WITH_NET=OFF: 72/72.ASAN+UBSAN with leak detection: 78/78, clean — run locally this time rather than
left to CI.
What it unblocks
The
cmpctblockpath can now parse a header without doing proof-of-work on anunsolicited message. Wiring that is a separate change on the BIP152 branch, not
here.