block: serialize a whole block (stacked on #394) - #395
Open
xanimo wants to merge 4 commits into
Open
Conversation
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.
There was no way to write an AuxPoW proof back out. The tree could parse one and, since the preceding commits, retain it, but nothing could emit it, so a header that arrived over the wire could not be reproduced. Add dogecoin_auxpow_payload_serialize, in the wire order parse_dogecoin_auxpow_fields reads, and dogecoin_block_header_serialize_full, which writes the 80 base bytes and then the proof when the header carries one. dogecoin_block_header_serialize is untouched and still emits exactly 80 bytes. That is deliberate: its output is what the block hash, the scrypt proof of work, check_auxpow's own hashing and the fixed-width headers.db record are computed over. Making it AuxPoW-aware would change all four. This is the same split Core draws between CPureBlockHeader and CBlockHeader, and the same reason. The full form keys off whether the proof is present rather than off version bit 0x100 alone, so a header carrying the bit without a proof serializes as the 80 bytes it actually has instead of emitting a truncated blob. Tested by round-tripping the height-371338 mainnet vector: parse it, write it back, and require the result to be byte-identical to the bytes it came from. Emitting something merely well-formed is not enough -- the point of this is that short IDs and hashes computed over the output match Core's, which only holds if the bytes match exactly. The pure form is asserted to stay at 80 bytes and to match the first 80 of the input. The first attempt at checking that test had no bite: it swapped parent_merkle_index with aux_merkle_index, which are both zero in this block, so the output was identical and the test passed either way. Zeroing parent_hash instead fails it at the memcmp, which is what a real serialization bug would do. WITH_NET=ON with -DBUILD_SHARED_LIBS=1: 78/78. WITH_NET=OFF: 72/72. ASAN+UBSAN with leak detection: 78/78, clean.
The tree could serialize a header and it could serialize a transaction, but nothing could serialize a block. Anything holding a header and a set of transactions -- a block assembled locally, or one reconstructed from a compact block -- had no way to produce the bytes the rest of the client parses, because every path that consumes a block takes wire bytes and deserializes them. dogecoin_block_serialize writes the header in wire form, so AuxPoW and all, then the transaction vector. It uses the full header serializer rather than the pure one for that reason: a block carries the header a peer sent, not the 80 bytes the block hash is computed over. It stops rather than emitting a short block if the transaction array contains a NULL. A vector with a hole in it is a reconstruction that did not finish, and a block that is well-formed but missing transactions is worse than no output at all. Tested by round-tripping the whole height-371338 mainnet block: parse the header span, parse the transaction vector, assert the vector accounts for every remaining byte, serialize it all back, and require byte-identity with the input. Omitting the transaction count alone fails it at the memcmp. 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-block-serialize
branch
from
August 5, 2026 21:04
d353b72 to
c72ca60
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 #394 (→ #393 → #392). Review only the top commit.
The tree could serialize a header and it could serialize a transaction, but
nothing could serialize a block. Anything holding a header and a set of
transactions — a block assembled locally, or one reconstructed from a compact
block — had no way to produce the bytes the rest of the client parses, because
every path that consumes a block takes wire bytes and deserializes them.
dogecoin_block_serializewrites the header in wire form, AuxPoW and all, thenthe transaction vector. It uses
dogecoin_block_header_serialize_fullratherthan the pure form for exactly that reason: a block carries the header a peer
sent, not the 80 bytes the block hash is computed over.
It stops rather than emitting a short block if the transaction array
contains a NULL. A vector with a hole in it is a reconstruction that did not
finish, and a block that is well-formed but missing transactions is worse than
no output at all.
Test
Round-trips the whole height-371338 mainnet block: parse the header span, parse
the transaction vector, assert the vector accounts for every remaining byte,
serialize it all back, require byte-identity with the input.
Bite-checked: omitting the transaction count alone fails it at the
memcmp.WITH_NET=ONwith-DBUILD_SHARED_LIBS=1: 78/78.WITH_NET=OFF: 72/72.ASAN+UBSAN with leak detection: 78/78, clean.
Why this exists, and what it is not for
It was found while attempting the BIP152 relay loop — the reconstructed block
had nowhere to go. That relay loop is not being pursued for the SPV client,
and this PR does not add it.
Compact block relay saves bandwidth only when you already hold most of a block's
transactions, which means a full node with a mempool. An SPV client does not
want full blocks at all; it wants headers plus filter matches. Wiring the relay
loop into
spv.cwould make SPV heavier in exchange for a saving it structurallycannot collect.
That leaves #379 correctly scoped as a library primitive — wire format,
negotiation, hardened parsing — for a full-node consumer, rather than as an
unfinished SPV feature. This serializer is likewise a primitive: useful to
anything assembling a block, independent of whether BIP152 relay is ever wired.