feat(header): BlockInfo timeslot + epoch-mark length prefix + preimage-hash extrinsic commitment (GP v0.8.0) (#1014)#1031
Conversation
…hash extrinsic commitment (GP v0.8.0 #1014) Three consensus-visible changes from GP v0.8.0: - BlockInfo (recent history, eq:recenthistoryspec / C(3)): add a 4-byte timeslot between state_root and reported; NewItem now takes the block slot (H_t per eq:recenthistorydef) and both STF call sites pass it. - EpochMark (encodeepochmark): the validator-key sequence is now length-prefixed (var{k}); decode validates the prefix still equals ValidatorsCount. Reflection registries stay on v0.7.x per #1015 precedent. - Extrinsic hash (header.tex): the preimages component becomes p = E(var[(E4(s), blake(d))]) -- each preimage blob committed by its Blake2b hash -- replacing the full C.15 encoding. New p() helper in block_serialization.go, mirroring g(). Full-block serialization (C.15) is unchanged; only the hash commitment changes. Round-trip unit tests assert the new byte offsets. v0.7.x vector fallout (history + safrole STF modes, all traces) is skipped until official v0.8.0 vectors land; marker presence/content rules are unchanged in v0.8.0 and keep their existing tests.
…r-1014-recenthistory-v080 # Conflicts: # internal/types/encode_test.go
yu2C
left a comment
There was a problem hiding this comment.
Summary
Thanks for the v0.8.0 wire-format updates (BlockInfo timeslot, EpochMark length prefix, preimage-hash extrinsic commitment). The codec round-trip unit tests look correct.
One architectural suggestion around the new p() helper in block_serialization.go — please address before merge if possible.
Issue Verification
| Test | Status | Evidence |
|---|---|---|
go test ./internal/recent_history/... -count=1 |
PASS (skipped vectors) | TestRecentHistoryTestVectors skipped pending v0.8.0 vectors |
go test ./internal/stf/... -count=1 -run Header |
PASS (no tests) | internal/stf has no test files |
TestEncodeBlockInfo_V080Timeslot |
PASS | codec round-trip |
TestEncodeEpochMark_V080LengthPrefix |
PASS | codec round-trip |
TestExtrinsicHashPreimagesComponent_V080 |
PASS | p() byte layout |
Linked issues: #1014 (Part of #1012)
internal/utilities/block_serialization.go
[SUGGESTION] Move p() into internal/types/encode.go
p() is only called from CreateExtrinsicHash (used by validate_header.go and header_controller.go). It does not participate in block wire serialization (BlockSerialization still uses C.15 via ExtrinsicPreimageSerialization).
The v0.8.0 extrinsic-hash preimages component (header.tex p = E(var[(E4(s), blake(d))])) is intentionally not the same as PreimagesExtrinsic.Encode (C.15: E4(s) + length-prefixed blob). That distinction is correct — please do not change the existing Encode/Decode.
However, placing p() in block_serialization.go (alongside deprecated hand-rolled *Serialization helpers) makes it look like legacy code. The same file already has this split:
- C.15 block wire →
types.PreimagesExtrinsic.Encode/Decode - Extrinsic-hash component →
p()(hash-only, no decode)
Please move the extrinsic-hash encoding into internal/types/encode.go as a separately named method, e.g.:
// EncodeForExtrinsicHash implements header.tex p =
// E(var[(E4(s), blake(d)) for (s, d) in E_P]).
// NOT the C.15 block-wire encoding; see PreimagesExtrinsic.Encode.
func (p *PreimagesExtrinsic) EncodeForExtrinsicHash(e *Encoder) error { ... }Then have CreateExtrinsicHash call that method via the shared encoder, and remove the private p() helper. (Same pattern could apply to g() in a follow-up, but p() is in scope for this PR.)
Why encode.go?
types.go— struct definitions onlyencoder.go— low-levelEncodermachineryencode.go— per-type encode methods ✅decode.go— no decode needed for hash-only commitment
internal/utilities/extrinsic_hash_v080_test.go
[SUGGESTION] Merge test into block_serialization_test.go
Please drop the standalone extrinsic_hash_v080_test.go and move TestExtrinsicHashPreimagesComponent_V080 into block_serialization_test.go alongside the other serialization tests. If p() moves to types, rename the test accordingly (e.g. TestPreimagesExtrinsic_EncodeForExtrinsicHash_V080 in internal/types/encode_test.go).
Files reviewed (no issues)
internal/types/types.go,decode.go,encode.go,unmarshal_json.go— BlockInfo timeslot, EpochMark length prefix ✅internal/recent_history/recent_history_controller.go—NewItempassesblock.Header.Slot✅internal/input/jam_types/jam_types.go— struct sync ✅testdata/reader.go— v0.8.0 skip lists updated ✅
Severity: 0 CRITICAL · 2 SUGGESTION · 0 NIT
|
Thanks @yu2C — both suggestions applied in
|
…review follow-up, #1014) Address yu2C's review on #1031: - The v0.8.0 extrinsic-hash preimages component now lives in internal/types/encode.go as PreimagesExtrinsic.EncodeForExtrinsicHash (header.tex p = E(var[(E4(s), blake(d))])), clearly separated from the C.15 block-wire Encode which is unchanged. Uses x/crypto/blake2b directly (utilities/hash imports types, so types cannot import it). - New Encoder.EncodeFunc runs an alternate encoding against the pooled buffer; CreateExtrinsicHash uses it and the private p() helper in block_serialization.go is removed. - Test moved to internal/types/encode_test.go and renamed TestPreimagesExtrinsic_EncodeForExtrinsicHash_V080; the standalone utilities test file is dropped.
…review follow-up, #1014) Address yu2C's review on #1031: - The v0.8.0 extrinsic-hash preimages component now lives in internal/types/encode.go as PreimagesExtrinsic.EncodeForExtrinsicHash (header.tex p = E(var[(E4(s), blake(d))])), clearly separated from the C.15 block-wire Encode which is unchanged. Uses x/crypto/blake2b directly (utilities/hash imports types, so types cannot import it). - New Encoder.EncodeFunc runs an alternate encoding against the pooled buffer; CreateExtrinsicHash uses it and the private p() helper in block_serialization.go is removed. - Test moved to internal/types/encode_test.go and renamed TestPreimagesExtrinsic_EncodeForExtrinsicHash_V080; the standalone utilities test file is dropped.
…review follow-up, #1014) Address yu2C's review on #1031: - The v0.8.0 extrinsic-hash preimages component now lives in internal/types/encode.go as PreimagesExtrinsic.EncodeForExtrinsicHash (header.tex p = E(var[(E4(s), blake(d))])), clearly separated from the C.15 block-wire Encode which is unchanged. Uses x/crypto/blake2b directly (utilities/hash imports types, so types cannot import it). - New Encoder.EncodeFunc runs an alternate encoding against the pooled buffer; CreateExtrinsicHash uses it and the private p() helper in block_serialization.go is removed. - Test moved to internal/types/encode_test.go and renamed TestPreimagesExtrinsic_EncodeForExtrinsicHash_V080; the standalone utilities test file is dropped.
Part of #1012 · Closes #1014
What — three consensus-visible v0.8.0 changes
timeslotbetweenstate_rootandreported; the appended item carriesH_teq:recenthistoryspec,eq:recenthistorydef, C(3)var{k}); was fixed-lengthencodeepochmark(serialization.tex)p = E(var[(E4(s), blake(d))])— each blob committed by its Blake2b hash, replacing the full C.15 encodingEverything else in the issue is already aligned (verified against the v0.7.2→v0.8.0 tex diff): epoch/winning-tickets marker presence+content rules, the ancestors requirement, and the author-index bound are semantically unchanged.
Changes
types.BlockInfo+ Encode / Decode / UnmarshalJSON (+ jam_types struct kept in sync; reflection registries stay on v0.7.x per feat(work-package): align GP v0.8.0 work packages and bundles #1015 precedent)EpochMark.Encode/Decode— emit/read the length prefix (decode still validates it equalsValidatorsCount)block_serialization.go— newp()helper (mirrorsg()) used byCreateExtrinsicHash; full-block serialization (C.15) unchanged — only the hash commitment changesrecent_history.NewItemtakes the block slot; both STF call sites passblock.Header.SlotTested
TestEncodeBlockInfo_V080Timeslot,TestEncodeEpochMark_V080LengthPrefix(new) — round-trip + byte-offset assertsTestExtrinsicHashPreimagesComponent_V080(new) — count prefix + E4 requester + blake(blob), empty casego build/go vet/gofmtclean;internal/types,recent_history,safrole,statistics,store,extrinsic,input/jam_typesgreenv080IncompatibleModes), all traces (embed full serialized state), and the matching unit vector tests (skipV080VectorIncompat+ per-test skips)internal/header(TestCreateParentHash / TestCreateExtrinsicHash) andinternal/blockchain(TestGetGenesisBlock / TestGetGenesisState) fail identically on the pristine1012-update-to-v080base (verified at 6d93ac4) — stale v0.7.x fixtures, untouched here