fix: accept identifier-only stream as empty payload#8
Merged
Conversation
decode and decodeFromReader rejected a stream that contained only the 10-byte stream-identifier chunk with FrameError.NotFramed, even though the Snappy framing spec treats it as a valid representation of an empty payload. Go's snappy.NewReader and Rust's snap::read::FrameDecoder both accept the same input and decode it to an empty slice; cross-client interop fixtures (e.g. leanSpec) emit exactly this 10-byte form for empty input. The terminal post-loop check in both decode paths now requires that both saw_stream_identifier and saw_data_chunk are unset to declare the input unframed. A stream with the identifier alone — and no data chunks — returns an empty slice. Adds two regression tests against the canonical 10-byte "\xff\x06\x00\x00sNaPpY" input (decode and decodeFromReader). The existing "frame roundtrip samples" test already covered round-tripping "" through the lib's own encoder, but the encoder appends an empty data chunk in finish(), which masked the gap on the decode side. Closes blockblaz#7.
There was a problem hiding this comment.
Pull request overview
Adjusts Snappy frame decoding to treat a stream containing only the stream-identifier chunk (no data chunks) as a valid empty payload, matching the Snappy framing spec and other implementations (Go/Rust), and adds regressions for this case.
Changes:
- Loosened the end-of-decode guard in both
decodeFramedanddecodeFromReaderto accept identifier-only streams as empty. - Added regression tests covering the canonical 10-byte identifier-only input for both decode paths.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
GrapeBaBa
added a commit
to blockblaz/zeam
that referenced
this pull request
Apr 29, 2026
blockblaz/snappyframesz#8 (closes #7) makes decode accept a identifier-only frame as a valid empty payload. Bump the pin to the post-merge tip and drop the local skip in the snappy_frame runner. The snappy_frame_empty fixture now exercises the same round-trip path as every other snappy fixture: leanSpec's 10-byte "\xff\x06\x00\x00sNaPpY" stream decodes to an empty slice through zeam's snappyframesz, and zeam's encoder→decoder pair round-trips the empty input cleanly. Total runtime skips drop from 83 to 82.
2 tasks
GrapeBaBa
added a commit
to blockblaz/zeam
that referenced
this pull request
May 11, 2026
Resolve conflicts in build.zig.zon, pkgs/node/src/{chain,constants,node}.zig.
build.zig.zon:
- ssz: keep GrapeBaBa fork (cherry-picked array deserialize fix on a
0.16-compatible base) — PR-required for ssz array roundtrip in spectest.
- snappyframesz: keep cb0a08e4 (empty-stream decode fix from
blockblaz/snappyframesz#8) — PR-required for networking_codec runner.
- hash_zig: dropped — main removed it in the zig 0.16 upgrade and the
PR branch never used the dep in code.
pkgs/node/src/constants.zig:
- GOSSIP_DISPARITY_INTERVALS = 1 (PR #754, leanSpec gossip attestation rule)
- MAX_FUTURE_SLOT_TOLERANCE = 1 retained for blocks; documented that
gossip attestations use the stricter GOSSIP_DISPARITY_INTERVALS bound
and blocks keep slot-grained tolerance.
pkgs/node/src/chain.zig:
- Took main's superset for block validateBlock (FutureSlot /
FutureSlotQueueable error variants + queueable window logic) and
chain_worker submission paths in onGossip block handler.
- Kept main's pending_blocks queue, locks (states_lock,
pending_blocks_lock, etc.), chain_worker pointer.
- Reconstructed test section: HEAD's
processFinalizationAdvancement-below-PRUNE_NODE_THRESHOLD regression
test placed first, followed by main's full BorrowedState / BlockCache /
concurrent-stress test set.
pkgs/node/src/node.zig:
- Took main's cacheFutureBlock helper (purely additive from main).
GrapeBaBa
added a commit
to blockblaz/zeam
that referenced
this pull request
May 11, 2026
Drive the remaining 23 failures from the leanSpec bump down to 0. The
failures broke into five buckets — each fixed below.
1. SSZ runner: support new boundary / decode-rejection fixture types
(pkgs/spectest/src/runner/ssz_runner.zig)
- Register BoundaryBitvector{1,7,9,255,256,257}, BoundaryBitlist256,
BoundaryUint64List32 (leanSpec PR #646 merkleization boundaries).
- Register DecodeBitvector16, DecodeBitlist8, SmokeBitlist8 (PRs #649
decode_rejections / decode_failure_smoke).
- Read `rawBytes` in addition to `serialized` and honour the
`expectException` field — when present, the runner now asserts the
malformed input is rejected (instead of treating a successful
roundtrip as a pass).
2. Forkchoice gossip-attestation disparity bound (PR #682)
(pkgs/spectest/src/runner/fork_choice_runner.zig)
- `validateAttestationDataForGossip` used the old "1 whole slot"
tolerance against `getCurrentSlot()`. The spec now compares
`data.slot * INTERVALS_PER_SLOT` against
`store.time + GOSSIP_DISPARITY_INTERVALS`, exercised by the four
`*_just_beyond_disparity_boundary_rejected` /
`*_one_full_slot_in_future_rejected` fixtures.
3. Forkchoice update_safe_target ignores block-pool attestations (PR #680)
(pkgs/node/src/forkchoice.zig)
- `onAttestation(is_from_block=true)` used to mirror the new
`latestKnown` into `latestNew`, smuggling block-pool weight back
into `update_safe_target` (which reads from the "new" tracker per
PR #680). Drop the mirror so the pools stay strictly separate; the
`test_safe_target_ignores_known_pool_at_interval_3` fixture pins the
contract.
4. Attestation-tracker as ground truth for `attestationChecks` (PR #690)
(pkgs/spectest/src/runner/fork_choice_runner.zig)
- The check used to iterate `latest_*_aggregated_payloads`
(`std.AutoHashMap`) and pick the entry with max slot. With same-slot
equivocation the result depends on hashmap iteration order — the
spec relies on Python dict insertion order. Switch to reading
`tracker.latestKnown` / `tracker.latestNew`: zeam's tracker is
populated by `onAttestation` with a strict-`>` comparison, so it
already encodes "first attestation at a given slot wins" — exactly
the spec semantics under equivocation.
5. State-transition slots-only fixture support (PR #643)
(pkgs/spectest/src/runner/state_transition_runner.zig)
- `test_process_slots_target_equal_to_state_slot_rejected` ships
`pre` + `expectException` with an empty `blocks` array. Previously
the runner rejected this shape outright. Now: when no blocks are
supplied and an `expectException` is set, call
`pre_state.process_slots(state.slot)` and verify it returns an
error (zeam surfaces this as `InvalidPreState`; the spec name is
`AssertionError` "Target slot must be in the future").
6. Networking codec: empty snappy frame (leanSpec
`test_snappy_frame_empty`, blocked on
blockblaz/snappyframesz#8)
(build.zig.zon, pkgs/third_party/snappyframesz/...)
- Upstream `v0.16.0` branch lacks the empty-stream decode fix; the
branch that has the fix (`main` at cb0a08e) is still on a 0.15.2
commit base. Vendor the v0.16.0 source under
`pkgs/third_party/snappyframesz` and apply the 2-line
`if (!saw_data_chunk)` → `if (!saw_stream_identifier and !saw_data_chunk)`
guard in both `decodeFromReader` and `decodeFramed`. Switch the
`snappyframesz` dependency to `.path = "pkgs/third_party/..."`.
Build / runner plumbing fixes that surfaced along the way:
- pkgs/spectest/src/generator.zig: `resolveFixturesRoot` now returns
`[:0]u8` instead of `[]u8`. `realPathFileAlloc` in 0.16 returns a
null-terminated slice; demoting to `[]u8` shortens the slice length
by one and crashes the DebugAllocator with
"Allocation size N bytes does not match free size N-1" at the
`defer test_allocator.free(...)` site.
- build.zig: add `spectests.step.dependOn(&run_spectest_generate.step)`.
Without it, the test-binary compile and the generator race when zig
fans out parallel build steps, intermittently producing
"file contents changed during update" or `FileNotFound` on
generated/index.zig. The new edge makes `zig build spectest`
idempotent.
Pre-existing merge fallout cleaned up while here (chain.zig):
- BeamChain.init was missing the `pending_blocks` field
initialisation that main's #788 pending-queue refactor added — fill
it with `.empty`.
- HEAD's `processFinalizationAdvancement: below PRUNE_NODE_THRESHOLD
keeps pre-finalized ancestors` test (preserved from the merge) used
the zig 0.15 `Dir.realpathAlloc` API and the pre-refactor
`std.StringHashMap(PeerInfo)` for ConnectedPeers. Port both call sites
to the current API (`Dir.realPathFileAlloc(io, ".", gpa)` and
`network.ConnectedPeers`).
Verification:
- `zig build spectest` — All 513 tests passed (idempotent across
re-invocations).
- `zig build test` — All 14 test groups pass (251 unit tests).
GrapeBaBa
added a commit
that referenced
this pull request
May 11, 2026
`decode` and `decodeFromReader` rejected a stream that contained only the 10-byte stream-identifier chunk with `FrameError.NotFramed`, even though the Snappy framing spec treats it as a valid representation of an empty payload. Go's `snappy.NewReader` and Rust's `snap::read::FrameDecoder` both accept the same input and decode it to an empty slice; cross-client interop fixtures (e.g. leanSpec's `test_snappy_frame_empty`) emit exactly this 10-byte form for empty input. The terminal post-loop check in both decode paths now requires that both `saw_stream_identifier` and `saw_data_chunk` be unset to declare the input unframed. A stream with the identifier alone — and no data chunks — returns an empty slice. Adds two regression tests against the canonical 10-byte `"\xff\x06\x00\x00sNaPpY"` input (`decode` and `decodeFromReader`). The existing `frame roundtrip samples` test already covered round-tripping `""` through the lib's own encoder, but the encoder appends an empty data chunk in `finish()`, which masked the gap on the decode side. This is the zig-0.16-ready version of #8 (which was cut against the 0.15.2 commit base and never reconciled with the `v0.16.0` branch).
GrapeBaBa
added a commit
that referenced
this pull request
May 11, 2026
fix: accept identifier-only stream as empty payload (zig-0.16 rebase of #8)
1 task
GrapeBaBa
added a commit
that referenced
this pull request
May 11, 2026
Reconcile the `main` and `v0.16.0` branches so the default branch ships zig 0.16 support together with the empty-stream decode fix. Both branches independently cherry-picked the same logical empty-fix on different bases (main on 0.15.2 via #8, v0.16.0 on 0.16 via #9), which produced two collisions when merging: 1. The terminal `if (!saw_data_chunk)` check in `decodeFromReader` and `decodeFramed` is the same line on both sides; keep one copy with v0.16.0's longer comment (more interop context). 2. `test "decode accepts identifier-only stream as empty payload"` and `test "decodeFromReader accepts identifier-only stream as empty payload"` appear on both branches. The main copy still uses `std.io.fixedBufferStream` / `ArrayListUnmanaged.writer`, which were removed in zig 0.16 — drop those duplicates and keep the v0.16.0 versions that use the new `std.Io.Reader.fixed(...)` / `Writer.Allocating` API. After the merge `main` carries the zig 0.16 upgrade (`df262c6`, `f939ed6`) plus a single canonical empty-fix regression suite, and `zig build test` is green under zig 0.16.0 (3/3 build steps, 13/13 tests). Downstream consumers can now point at the default branch instead of the `v0.16.0` release-style branch (which has been the only 0.16-ready snapshot until today).
ch4r10t33r
added a commit
to blockblaz/zeam
that referenced
this pull request
May 12, 2026
…715) * feat(spectest): add SSZ roundtrip runner with comptime type dispatch Add SSZ fixture runner that tests serialization roundtrips for all 18 consensus types. Uses test-mode type mirrors with 424-byte signatures for leanEnv=test fixtures, enabling all 34 SSZ tests to pass without requiring hashsig infrastructure changes. * chore: update leanSpec submodule to latest main Update leanSpec to 1177800 which includes the latest spectest fixture vectors for SSZ, verify_signatures, and other test categories. * fix(spectest): handle gossipAggregatedAttestation as unsupported step type The updated leanSpec generates new fork_choice fixtures with gossipAggregatedAttestation steps. Mark this step type as unsupported (graceful skip) alongside the existing attestation step type, so these tests don't fail with InvalidFixture. * feat(spectest): implement attestation and gossipAggregatedAttestation fork choice steps Add full attestation step support to the fork choice runner instead of skipping them as unsupported. This resolves 15 previously failing fork choice spec tests. Changes: - Add processAttestationStep with validator bounds check, attestation data validation, and signature error detection - Add processGossipAggregatedAttestationStep with aggregation proof handling and store integration - Add validateAttestationDataForGossip implementing leanSpec validation rules (block existence, slot relationships, future slot tolerance) - Align forkchoice.zig future attestation check with leanSpec (allow +1 slot tolerance) - Consolidate KeyManager error variants into ValidatorKeyNotFound - Move MAX_ATTESTATIONS_DATA to constants module * fix: revert unrelated chain/key-manager/constants changes from spectest commit These changes (KeyManager error consolidation, MAX_ATTESTATIONS_DATA constant move, chain.zig error renaming) are out of scope for this PR and caused CI failures due to missing max_attestations_data struct field in test initializers. * feat(spectest): update leanSpec fixtures; support all fork_choice/state_transition cases Bump leanSpec submodule to 9b89651 (50 commits ahead) and regenerate fixtures — 468 total, up from 123. All fork_choice and state_transition tests now run without skips. production (forkchoice): * forkchoice.onBlock now enforces leanSpec store.process_block rules: reject blocks with duplicate AttestationData or more than MAX_ATTESTATIONS_DATA (16) distinct entries. Previously this lived only in chain.zig and was skipped by the spectest path. * Remove the NotFinalizedDesendant rejection from onBlockUnlocked — leanSpec store.on_block accepts every block whose parent is known, and get_head naturally ignores forks off pre-finalization ancestors because it walks best descendants from latest_justified. * Promote isFinalizedDescendant to pub and move the DoS defense to chain.validateBlock (gossip attack surface) as a new BlockValidationError.NotFinalizedDescendant. Production continues to reject mal-parented blocks at the chain layer; forkchoice stays spec-pure so spectests can exercise the leanSpec semantics directly. spectest runner — state_transition: * buildBlock now parses full aggregated attestations (aggregationBits.data + AttestationData) and hands them to apply_transition. The prior "attestations unsupported" early-return silently skipped 30 justification/finalization cases. spectest runner — fork_choice: * Tick steps accept either `interval` or `time` plus `hasProposal` (new leanSpec schema). * blockAttestations check treats attestationSlot/targetSlot as optional (both were made optional in leanSpec test_types). * attestationChecks mirrors leanSpec extract_attestations_from_aggregated_payloads: iterate latest_new/known_aggregated_payloads and pick the largest-slot entry that includes the validator (previous code read the wrong AttestationTracker source). * Implement nine previously-UnsupportedFixture checks so 10 cases stop silently skipping: - safeTargetSlot / safeTargetRootLabel - latestFinalizedRootLabel - filledBlockRootLabel (tracks the block root just processed) - labelsInStore (asserts labelled roots still live in protoArray) - reorgDepth (walks new-head ancestry, then counts from old head) - attestationSignatureTargetSlots - latestNewAggregatedTargetSlots - latestKnownAggregatedTargetSlots * processAttestationStep now writes into attestation_signatures with ZERO_SIGBYTES placeholder. leanSpec's store.on_attestation inserts here on the gossip path; the runner was only calling onAttestation (validator tracker only), leaving attestation_signatures perpetually empty and the new *TargetSlots checks unobservable. * processBlockStep calls pruneStaleAttestationData when latest_finalized advances, matching leanSpec store.on_block's tail behaviour. Without this, attestation/payload maps kept stale entries that failed the *TargetSlots checks after finalization. results: * 159/230 generated tests pass; remaining 71 failures are all SSZ basic-type fixtures (new leanSpec types: Uint16/Uint32/Uint64/Fp/ Boolean/Bytes4/Bytes64/Bitvector/Bitlist/List/Vector/HashTreeOpening) — intentionally out of scope. * 0 fork_choice skips, 0 state_transition skips (down from 10 fork_choice + 30 state_transition silently skipped). * zig build test still exits 0. * feat(spectest): SSZ runner supports new leanSpec basic types Adds SSZ roundtrip coverage for the scalar, byte-array, bitvector, integer-vector, bitlist and list types introduced alongside the new test_basic_types / test_xmss_containers fixtures: Uint8, Uint16, Uint32, Uint64, Fp, Boolean Bytes4, Bytes32, Bytes52, Bytes64 SampleBitvector8/64, AttestationSubnets, SyncCommitteeSubnets SampleUint16Vector3, SampleUint64Vector4 SampleBitlist16, SampleBytes32List8, SampleUint32List16 ByteListMiB, HashTreeOpening (empty), SignedAggregatedAttestation Wires `@zeam/xmss` into the zeam_spectests module so ByteListMiB and HashDigest types are reachable from the runner. Known-skipped via explicit skip list (previously silently unsupported): - SampleUnionNone / SampleUnionTypes — zeam's SSZ library has no `union(enum)` path yet; leanSpec introduced SSZUnion in this release. - SampleUint16Vector3 / SampleUint64Vector4 / HashTreeLayer / HashTreeOpening (typical) — blockblaz/ssz.zig's deserialize for `.array` with element size > 1 conflates byte offset with element index, so only out[0], out[2], ... get written and the roundtrip diverges. Upstream bug; filed for separate fix. Result: 230/230 spectests pass, 0 skips in the runner path (40 previously silent SkippedFixture cases now run). * fix: align attestation future slot tolerance with leanSpec leanSpec store.py:320 allows current_slot + 1 for all attestations (gossip and block) as clock disparity tolerance. zeam previously only allowed +1 for block attestations and rejected gossip attestations for future slots. Align with leanSpec's unified behavior. * fix: tolerate pruned source blocks in attestation validation When finalization advances, old blocks are pruned from the protoarray. Aggregations referencing a source checkpoint that was valid when the attestation was created can fail validation if finalization pruned the source block between attestation creation and aggregation publishing. Accept source blocks whose slot <= finalized_slot as valid even when absent from the protoarray — they were canonical and are now below the finalization horizon. Root cause: race between finalization pruning (interval 0 of new slot) and aggregation publishing (interval 2 of same slot), where the aggregation's attestation data references a justified checkpoint that got pruned during the finalization advance. * fix: remove attestation data validation from onGossipAggregatedAttestation Per leanSpec store.py on_gossip_aggregated_attestation: aggregated attestations only verify the aggregated signature proof, they do NOT call validate_attestation on the attestation data. nlean follows the same pattern. The attestation data was already validated when individual gossip attestations arrived. Re-validating in the aggregation path caused UnknownSourceBlock errors when finalization pruned the source block from protoarray between attestation creation and aggregation publishing. This also reverts the "tolerate pruned source blocks" workaround in validateAttestationData since the root cause is now fixed. * fix(forkchoice): gate proto-array rebase on PRUNE_NODE_THRESHOLD Root-cause the attestation-race previously worked around in chain.onGossipAggregatedAttestation and validateAttestationData. Symptom: every time finalization advanced, processFinalizationAdvancement called forkChoice.rebase(...) unconditionally. Rebase drops pre-finalized ancestors from proto-array and remaps attestation-tracker indices. Any in-flight attestation whose source / target / head referenced one of those dropped blocks then failed validateAttestationData's existence check with Unknown{Source,Target,Head}Block — even though the vote was perfectly valid when the validator produced it. 3SF-mini's fast finalization cadence (one advance every few slots) puts the race window on the same order of magnitude as normal gossip propagation, so attestations drop across every finalization boundary under normal load. Sync catch-up, which can advance finalization multiple steps in a single tick, makes it much worse. Fix — lighthouse-style lazy prune: * Introduce constants.PRUNE_NODE_THRESHOLD = 64. Lighthouse uses 256 for mainnet; 3SF-mini's shorter wall-clock per finalization step makes 64 (~1 eth epoch worth of grace) the better trade between race coverage and memory footprint. * Gate the rebase call in processFinalizationAdvancement on the finalized node's index in the proto-array. While that index is below the threshold, skip rebase and leave the pre-finalized prefix addressable so in-flight attestations still resolve. * Expose ForkChoice.getProtoNodeIndex for the gate (mutex-safe wrapper over protoArray.indices). Revert of the prior tolerance workaround: * onGossipAggregatedAttestation now re-validates the attestation data again. The "skip validate" workaround exists only because source blocks could vanish mid-finalization; with the threshold gate keeping them in place, the stricter check is safe again and catches malformed aggregates that used to slip through. Test: * Add regression test driving the mock chain with default onBlock opts (pruneForkchoice = true) through 5 slots. The finalized node's index stays well under the threshold, so pre-finalized ancestors must remain resolvable via getProtoNode after finalization advances. Before this fix, the rebase call would have removed roots[1] and roots[2] (slots < latestFinalized.slot) from the protoArray and the assertion would trip. * Update the existing "Test 9: Attestation too far in future" case to use slot=current+2 — the spec-aligned +1 tolerance change from 09712ac made the prior slot=current+1 trip point pass validation; the test was not updated at the time. The future-slot +1 tolerance fix (09712ac) is orthogonal and preserved — spec alignment with store.py:320. * fix(forkchoice): drop time-based block queue; align with leanSpec zeam was the only client among leanSpec / ream / qlean-mini / ethlambda / nlean that queued inbound blocks on the local clock. Every other implementation processes a block the moment its parent state is known — their only "pending" caches are for orphan blocks (parent unknown, awaiting backfill), never for blocks whose slot just hasn't started yet locally. The queue also kept block body attestations out of fork choice until the next interval tick, delaying head updates by a full interval and duplicating the future-slot rejection that already lives in chain.validateBlock. Changes: * Remove BeamChain.pending_blocks field, its init / deinit hooks, and the append site on the gossip path. * Remove BeamChain.processPendingBlocks (replay helper) and its caller in node.onInterval. * Drop the duplicate future-slot gate in forkchoice.onBlockUnlocked. Gossip-level DoS filtering continues to happen in chain.validateBlock (MAX_FUTURE_SLOT_TOLERANCE = 1, matching leanSpec store.py:320); forkchoice now mirrors leanSpec store.on_block and only requires a known parent plus non-pre-finalized slot. * Remove ForkChoiceError.FutureSlot — no caller returns it. * Update the fork-choice block-tree unit test: the old test drove "reject future slot" against forkchoice.onBlock; that responsibility has moved up to chain.validateBlock, so the test now just ticks the clock and processes the block. validateBlock still rejects block.slot > current_slot + 1 with BlockValidationError.FutureSlot; node.zig's handler for error.FutureSlot still caches those blocks for later retry, so the "block arrived genuinely too early" case is covered exactly as before — just without the redundant inner-layer queue. * fix(forkchoice): add missing leanSpec attestation checks leanSpec `store.py:validate_attestation` enforces two topology / consistency rules that zeam had been skipping: - `data.head.slot >= data.target.slot` — head checkpoint cannot be older than the target (topology check, store.py:304). - `head_block.slot == data.head.slot` — head checkpoint slot must match the actual block slot the checkpoint points at (consistency check, store.py:314). Without these, malformed gossip / aggregated attestations with a stale or mismatched head pointer slipped through zeam's validation while leanSpec and the rest of the ecosystem reject them. Add the two checks with matching `HeadOlderThanTarget` / `HeadCheckpointSlotMismatch` error variants and remove the stale `_ = head_block; // Will be used in future` placeholder now that the node is actually consulted. * chore(chain): drop stale 'earlier revision' comment on aggregated path The block explained why re-validation was restored — context that is meaningful in a PR description but rots as a source comment: future readers have no "earlier revision" to compare against, and the rebase gating it references is documented at the rebase site itself. * fix(chain): address Copilot review on PR #754 Five follow-ups from the review pass: - Close RPC / sync future-slot gap: BeamChain.onBlock now enforces the MAX_FUTURE_SLOT_TOLERANCE (+1) bound itself. Previously only the gossip path funnelled through chain.validateBlock; RPC handlers went straight to onBlock and would happily run STF + fork-choice on blocks arbitrarily far in the future, a cheap DoS vector. - Make the PRUNE_NODE_THRESHOLD regression test actually exercise the gate by calling onBlockFollowup (where processFinalizationAdvancement — and the rebase call it gates — fires). Previously the test only called onBlock and would have passed even without the fix. - Update validateAttestationData doc and `attestation validation - gossip vs block future slot handling` test to describe the unified `current_slot + 1` rule both paths now follow; the test is renamed and now asserts both the accepted current+1 case and the rejected current+2 case on both is_from_block values. - Clarify PRUNE_NODE_THRESHOLD: the threshold is a proto-array node count, not a slot distance. Under heavy forking, node count advances faster than slot distance, so the wall-clock grace can shrink. Updated the constants.zig comment and the rebase call site accordingly. - Log loudly instead of silently skipping the rebase when latestFinalized.root is missing from proto-array. Under normal operation it is always present (getCanonicalViewAndAnalysis resolves it via protoArray.indices); surfacing the invariant violation beats hiding it behind "nothing happened". * forkchoice: align attestation time check with interval-grained leanSpec rule Switch the gossip-attestation time check from slot-grained to interval-grained: data.slot * INTERVALS_PER_SLOT <= store.time + GOSSIP_DISPARITY_INTERVALS A whole-slot tolerance let an adversary pre-publish next-slot aggregates ahead of any honest validator (~800 ms head start at SECONDS_PER_SLOT=4 / INTERVALS_PER_SLOT=5). One interval bounds that head start to NTP drift. Block-included attestations skip the time check; they are trusted under the block's signature + STF validation. The block path itself drops the slot-grained future-slot bound entirely — block admission is gated by parent / signature / STF only. Removed: - constants.MAX_FUTURE_SLOT_TOLERANCE (was reused across attestation validation and validateBlock / onBlock; the block usages had no spec basis to begin with) - chain.validateBlock future-slot rejection - chain.onBlock entry-point future-slot rejection - chain.validateAttestationData block-path slot bound - BlockValidationError.FutureSlot variant - node.zig error.FutureSlot handlers and cacheFutureBlock helper (the cacheFutureBlock path had no replay mechanism after the time-based block queue was removed earlier in this PR) Added: - constants.GOSSIP_DISPARITY_INTERVALS = 1 - chain.validateAttestationData gossip-only interval bound - chain.zig boundary tests covering the gossip / block divergence * spectest: bump ssz.zig pin to pick up array deserialize fix and remove skip workarounds Upstream blockblaz/ssz.zig fixed the fixed-size array deserialize bug in PR #61 (`iterate fixed-size arrays by element, not byte step`), but the fix landed on master after the 0.16 upgrade. Use the GrapeBaBa fork's `backport-array-fix-015x` branch, which cherry-picks the fix on top of 0.15.2. Drop the corresponding skip entries (SampleUint16Vector3, SampleUint64Vector4, HashTreeLayer, HashTreeOpening) from the SSZ runner. Register a TestHashTreeLayer mirror so the dispatch table can roundtrip it. Eight previously-skipped fixture cases (4 fixture files × 2 cases) now run real serialize/deserialize roundtrips instead of silent pass. SSZ Union (SampleUnionNone, SampleUnionTypes — 5 cases) remains skipped pending union(enum) registration, tracked separately. * spectest: implement justifiability and verify_signatures runners Two leanSpec fixture kinds were skipped at parse time because zeam's spectest framework only knew about state_transition / fork_choice / ssz. Add the two missing kinds. Justifiability (33 fixtures, all real): each fixture pins a (slot, finalizedSlot) pair and an expected (delta, isJustifiable) output. The runner calls types.IsJustifiableSlot(finalized, slot) and asserts both the delta arithmetic and the predicate result match. verify_signatures (7 fixtures): the runner is wired through and deserialization is sketched, but every current fixture is generated with leanEnv=test (signature_len ≈ 700 bytes), while zeam runs against the production scheme (SIGSIZE=2536). Loading these into a production SignedBlock is a hard size mismatch, so each case validates the fixture's structural shape and then surfaces a clear skip message naming the scheme gap. When zeam grows a test-mode XMSS path (or leanSpec emits prod-scheme fixtures), the production branch in runCase will pick them up without further wiring. Adds: - FixtureKind variants: justifiability, verify_signatures - runner/justifiability_runner.zig - runner/verify_signatures_runner.zig - lib.zig exports Test counts: 270/270 pass (33 new justifiability + 7 new verify_signatures skipped with explicit reason; previously 230 generated and the 40 here were silently dropped at parse time). * spectest: real verification path for verify_signatures fixtures leanSpec generates verify_signatures fixtures with leanEnv=test (LOG_LIFETIME=8, DIMENSION=4, ~424-byte signatures), which the production scheme (LOG_LIFETIME=32, DIMENSION=46, 2536-byte signatures) cannot parse. Earlier the runner short-circuited every case with a scheme-mismatch skip. Add a parallel test-scheme XMSS verify FFI that always compiles alongside the production path: - rust/hashsig-glue: new test_scheme module exporting hashsig_test_verify_ssz, instantiated against SchemeAbortingTargetSumLifetime8Dim46Base8 (note: leansig type alias name is misleading — the constants inside use DIMENSION=4, matching leanSpec TEST_CONFIG exactly). - pkgs/xmss: extern declaration + xmss.verifySszTest wrapper. Runner now parses the anchor state and signed block with the existing state_transition_runner helpers (made pub for reuse), allocates the proposer signature as a variable-length []u8 (sidestepping the prod SIGSIZE=2536 fixed array), hashes the block body, and dispatches to verifySsz / verifySszTest based on leanEnv. expectException maps to an expected verification failure. 6 of 7 fixtures now run real cryptography: - test_invalid_proposer_signature → verifySszTest rejects - test_proposer_signature, test_single_validator_attestation, test_proposer_and_attester_signatures, test_all_four_validators_attesting, test_multiple_attestation_groups_same_data → verifySszTest accepts The remaining fixture (test_invalid_aggregated_attestation_signature) carries a body attestation; it still skips with a clear note pending a test-scheme leanMultisig wrapper. Reuses the SignedBlock/BeamBlock types directly. Only the proposer signature differs in byte count between the two schemes; the validator public keys (52 bytes), block container, and AggregatedSignatureProof (variable byte list) are scheme-agnostic. * spectest: cover body-attestation verify path in verify_signatures runner The previous commit ran real cryptographic verification on the proposer signature only, and skipped the lone fixture (test_invalid_aggregated_attestation_signature) that carries body attestations. Wire that case through. After verifying the proposer signature, walk each block-included aggregated attestation: parse participants and proofData out of the fixture, load attestation pubkeys from the anchor state, hash the attestation data, and call AggregatedSignatureProof.verify (which dispatches to leanMultisig). leanMultisig's Rust glue is hardcoded to the production scheme; with test-scheme bytes from the fixture the prod path will reject at deserialization. That happens to be the expected outcome here — the fixture asserts the implementation rejects the corrupted aggregated signature — so the path closes cleanly without forking leanMultisig. The runner now treats expectException as "at least one signature rejected" (proposer or any body attestation), and absent-exception as "all signatures verified". The bookkeeping is in a single any_failure flag rather than the previous proposer-only branch. A future leanSpec valid-case fixture with body attestations would need a parallel test-scheme leanMultisig FFI; the limitation is documented in verifyBodyAttestations. 7/7 verify_signatures fixtures now run real verification, 0 skips. * spectest: skip body-attestation verify under leanEnv=test instead of relying on prod-rejects-test coincidence The previous commit covered the body-attestation path by routing the fixture through zeam's production leanMultisig FFI. For the lone fixture that exercises that path (test_invalid_aggregated_attestation_signature), the prod path rejects test-scheme bytes at AggregatedXMSS::deserialize — which happens to match expectException, so the test "passes". That is structural coincidence, not real cryptographic rejection: a future valid-case body-attestation fixture under leanEnv=test would also be rejected by the prod path and the test would falsely report a failure. Rather than carry that latent false-positive, branch explicitly. When leanEnv=test and the block carries any body attestations, surface a clear skip naming the leanMultisig test-scheme FFI gap. The 6 fixtures without body attestations still run real test-scheme XMSS verification through the proposer signature path. Production-scheme fixtures (none today) would continue to exercise the body-attestation path through the prod leanMultisig glue. Genuine coverage of the seventh fixture requires forking leanMultisig to add a parallel test-scheme rec_aggregation (parallel constants, types, cached bytecode, and verify entry point). Tracked separately — not blocking PR #715. * spectest: implement slot_clock and api_endpoint runners slot_clock (25 fixtures, all real): pure-arithmetic runner covering the five operations leanSpec exposes — current_slot, current_interval, total_intervals, from_slot, from_unix_time. Each one is a single arithmetic identity against the embedded clock config, with a clamp to zero for inputs that fall before genesis. Operations are derived locally from SECONDS_PER_SLOT (params) and INTERVALS_PER_SLOT (node constants) so the spec config is sanity-checked against zeam's compile- time constants on every fixture. api_endpoint (12 fixtures, 7 real + 5 skipped): - /lean/v0/health (1) — fixed body parity check. - /lean/v0/admin/aggregator GET (2) — mirrors handleAggregatorAdmin's response shape against initialIsAggregator. - /lean/v0/admin/aggregator POST (4) — mirrors handleAggregatorPost's {is_aggregator, previous} response, including the idempotent enable / disable cases. - /lean/v0/states/finalized, /lean/v0/fork_choice, /lean/v0/checkpoints/justified (5) — surface a clear skip naming the endpoint; covering them needs a constructed BeamChain (anchor state, validators, finalized checkpoint, proto-array nodes) that the spec runner doesn't currently build. The runner is a behavioural-parity test, not a full handler integration: it reproduces each handler's response generation locally and compares against the fixture's expectedStatusCode / expectedContentType / expectedBody. Content-type is matched as a prefix because zeam emits "application/json; charset=utf-8" while leanSpec pins "application/json". 307/307 tests pass; 6 explicit skips total (5 api_endpoint chain-state endpoints + 1 leftover verify_signatures body-attestation case). * spectest: implement networking_codec runner (varint, snappy, gossip topic/msg id, discovery routing) leanSpec emits 135 networking_codec fixtures across 9 codec families, and zeam previously dropped them all at parse time. Add a unified runner that dispatches on codecName and reuses zeam's production codecs where they exist: - varint (14): pure LEB128 encode + length check - gossip_topic (8): network.LeanNetworkTopic.encode - gossip_message_id (8): SHA-256(domain || topic_len_le_u64 || topic || data) truncated to 20 bytes — mirrors libp2p-glue's message_id_fn - log2_distance (8) + xor_distance (6): bitwise on 32-byte node IDs - snappy_block (8) + snappy_frame (7): round-trip via snappyz / snappyframesz. Snappy compression is non-canonical — emitter choices about literal/copy tags or chunk layout differ between implementations — so wire-format interop is verified by decode(leanSpec output) == input AND encode(input)→decode→input, not by byte-equal encoder agreement. Codecs requiring infrastructure zeam doesn't expose yet (RLP for ENR / discv5, protobuf for gossipsub RPC, secp256k1 for peer_id, varint+ssz framing for reqresp) are skipped at runtime with a clear codec-name reason. 75 such skips covering reqresp_codec (21), gossipsub_rpc (26), enr_and_peer_id (14), discv5_messages (15) — these are the natural follow-on PRs. Module wiring: zeam_spectests now imports @zeam/network, snappyz, and snappyframesz so the runner can call zeam's codecs directly. Empty-input snappy_frame is skipped with a separate note: zeam's snappyframesz rejects a stream that contains only the 10-byte magic header (no data chunks); leanSpec's encoder emits exactly that. Tracked separately as a snappyframesz library gap. 442/442 tests pass; 83 total runtime skips. networking_codec contributes 59 real and 76 skipped. * spectest: bump snappyframesz to pick up empty-stream decode fix blockblaz/snappyframesz#8 (closes #7) makes decode accept a identifier-only frame as a valid empty payload. Bump the pin to the post-merge tip and drop the local skip in the snappy_frame runner. The snappy_frame_empty fixture now exercises the same round-trip path as every other snappy fixture: leanSpec's 10-byte "\xff\x06\x00\x00sNaPpY" stream decodes to an empty slice through zeam's snappyframesz, and zeam's encoder→decoder pair round-trips the empty input cleanly. Total runtime skips drop from 83 to 82. * fix(lint): reorder imports in hashsig-glue test_scheme to satisfy rustfmt * chore: bump leanSpec to a5a05f9 (lstar fork rename + 56 commits) leanSpec changes since 9b89651: - Stages 1-6 of #686: large refactor of forks/lstar (decoupled subspecs, ForkProtocol surface, generic Store). - PR #710: renamed devnet fork to lstar everywhere. - 30+ new test-vector PRs across networking, SSZ, fork choice, state transition, sync, poseidon_permutation, gossipsub_handler, discovery_crypto fixture formats. zeam side: - pkgs/spectest/src/fork.zig: rename `devnet` Fork to `lstar` (name=Lstar, path=lstar, symbol=forks.lstar) to match leanSpec PR #710 directory layout. - Generated index (pkgs/spectest/src/generated/) is gitignored; will be regenerated next `zig build spectest:generate`. * spectest: port to zig 0.16 (post-main-merge) The PR branch's full spectest infrastructure (runners + lib glue) was written against zig 0.15. Main's #784 upgraded zeam to zig 0.16 and silently broke the spectest because main only had a minimal runner set (fork_choice + state_transition + ssz) — the regression wasn't visible until this branch's full suite was merged. Changes: build.zig.zon: - ssz: switch back to blockblaz/ssz.zig (404762835d), the 0.16-compatible upstream. The GrapeBaBa cherry-pick fork pinned in earlier PR commits has `array_list.Aligned(u8,null).writer` API that 0.16 removed. The array-deserialize fix the fork existed for is now on blockblaz/ssz.zig master. - snappyframesz: switch back to df262c69. The newer cb0a08e4 (empty- stream decode fix) uses `meta.intToEnum` which 0.16 removed; the upstream fix branch has not been ported to 0.16. The networking_codec test_snappy_frame_empty fixture is the one that needs it — it stays failing until upstream snappyframesz is ported. pkgs/spectest/src/lib.zig: - Replace `std.testing.refAllDecls` with a local `refAllDeclsRecursive` (the std variant was removed in 0.16). With the shallow version, only the wrapper test was discovered ("1/1 generated fixtures...OK" with zero inner tests actually run); the recursive walk now expands the full ~500-test tree. pkgs/spectest/src/generator.zig: - Drop the `else => return err` prong from the emitted `resolveFixturesRoot` switch — `ResolveError` is a single-variant error set, and 0.16 rejects unreachable else prongs. pkgs/spectest/src/runner/*.zig: - `std.fs.Dir` → `std.Io.Dir` (the directory type moved in 0.16). - `dir.readFileAlloc(allocator, path, max)` → `dir.readFileAlloc(std.testing.io, path, allocator, .limited(max))` — the 0.16 signature takes an explicit `io` first, swaps `(path, allocator)` order, and uses `Io.Limit` instead of usize. - `std.json.ObjectMap.init(allocator)` → `var x: ObjectMap = .empty;` + per-call `put(allocator, ...)` — ObjectMap is the unmanaged map in 0.16 and `init` now takes `(gpa, key_list, value_list)`. Updated `putString` helper to thread the allocator through. - `std.ArrayList(T){}` → `: std.ArrayList(T) = .empty;` — the 0.16 unmanaged ArrayList rejects the empty-struct literal (it has multiple fields now). pkgs/spectest/src/skip.zig: - Replace `std.once` (removed in 0.16) with a hand-rolled CAS-based one-shot guard using `std.atomic.Value(bool)`. - Replace `std.process.getEnvVarOwned` (removed in 0.16) with `std.testing.environ.getPosix(...)` — the test runner initialises `std.testing.environ` and that's the only env-read surface we need here (skip.zig is consulted exclusively from spectest test code). Test results: `zig build spectest` now compiles and runs the full suite. 490 / 513 pass. The 23 failures break down as: - 11 new ssz fixtures (test_decode_failure_smoke / test_decode_rejections / test_merkleization_boundaries from leanSpec PRs #646/#649) — runner doesn't yet recognise bitlist / bitvector / merkleization-boundary fixture variants; ports flagged as UnsupportedFixture. - 6 new fork-choice attestation validation fixtures (test_safe_target, test_gossip_attestation_validation / test_gossip_aggregated_*, test_equivocation from leanSpec PRs #682/#680/#644) — fixtures expect specific reject behaviour at the disparity boundary that the runner needs to align with. - 1 new state_transition test_slot_monotonicity case (#643). - 1 new networking_codec test_snappy_frame_empty case (needs the not-yet-0.16-ported empty-stream snappy fix). - 4 misc fixtures whose runner mapping changed shape post-leanSpec refactor (Stage 1-6 of #686). Net: the merge + leanSpec bump is mechanically clean and the spectest runner is back in business under 0.16. The 23 failures are content-level deltas from the leanSpec update, not regressions introduced by this merge. * spectest: align with leanSpec a5a05f9; full 513/513 pass Drive the remaining 23 failures from the leanSpec bump down to 0. The failures broke into five buckets — each fixed below. 1. SSZ runner: support new boundary / decode-rejection fixture types (pkgs/spectest/src/runner/ssz_runner.zig) - Register BoundaryBitvector{1,7,9,255,256,257}, BoundaryBitlist256, BoundaryUint64List32 (leanSpec PR #646 merkleization boundaries). - Register DecodeBitvector16, DecodeBitlist8, SmokeBitlist8 (PRs #649 decode_rejections / decode_failure_smoke). - Read `rawBytes` in addition to `serialized` and honour the `expectException` field — when present, the runner now asserts the malformed input is rejected (instead of treating a successful roundtrip as a pass). 2. Forkchoice gossip-attestation disparity bound (PR #682) (pkgs/spectest/src/runner/fork_choice_runner.zig) - `validateAttestationDataForGossip` used the old "1 whole slot" tolerance against `getCurrentSlot()`. The spec now compares `data.slot * INTERVALS_PER_SLOT` against `store.time + GOSSIP_DISPARITY_INTERVALS`, exercised by the four `*_just_beyond_disparity_boundary_rejected` / `*_one_full_slot_in_future_rejected` fixtures. 3. Forkchoice update_safe_target ignores block-pool attestations (PR #680) (pkgs/node/src/forkchoice.zig) - `onAttestation(is_from_block=true)` used to mirror the new `latestKnown` into `latestNew`, smuggling block-pool weight back into `update_safe_target` (which reads from the "new" tracker per PR #680). Drop the mirror so the pools stay strictly separate; the `test_safe_target_ignores_known_pool_at_interval_3` fixture pins the contract. 4. Attestation-tracker as ground truth for `attestationChecks` (PR #690) (pkgs/spectest/src/runner/fork_choice_runner.zig) - The check used to iterate `latest_*_aggregated_payloads` (`std.AutoHashMap`) and pick the entry with max slot. With same-slot equivocation the result depends on hashmap iteration order — the spec relies on Python dict insertion order. Switch to reading `tracker.latestKnown` / `tracker.latestNew`: zeam's tracker is populated by `onAttestation` with a strict-`>` comparison, so it already encodes "first attestation at a given slot wins" — exactly the spec semantics under equivocation. 5. State-transition slots-only fixture support (PR #643) (pkgs/spectest/src/runner/state_transition_runner.zig) - `test_process_slots_target_equal_to_state_slot_rejected` ships `pre` + `expectException` with an empty `blocks` array. Previously the runner rejected this shape outright. Now: when no blocks are supplied and an `expectException` is set, call `pre_state.process_slots(state.slot)` and verify it returns an error (zeam surfaces this as `InvalidPreState`; the spec name is `AssertionError` "Target slot must be in the future"). 6. Networking codec: empty snappy frame (leanSpec `test_snappy_frame_empty`, blocked on blockblaz/snappyframesz#8) (build.zig.zon, pkgs/third_party/snappyframesz/...) - Upstream `v0.16.0` branch lacks the empty-stream decode fix; the branch that has the fix (`main` at cb0a08e) is still on a 0.15.2 commit base. Vendor the v0.16.0 source under `pkgs/third_party/snappyframesz` and apply the 2-line `if (!saw_data_chunk)` → `if (!saw_stream_identifier and !saw_data_chunk)` guard in both `decodeFromReader` and `decodeFramed`. Switch the `snappyframesz` dependency to `.path = "pkgs/third_party/..."`. Build / runner plumbing fixes that surfaced along the way: - pkgs/spectest/src/generator.zig: `resolveFixturesRoot` now returns `[:0]u8` instead of `[]u8`. `realPathFileAlloc` in 0.16 returns a null-terminated slice; demoting to `[]u8` shortens the slice length by one and crashes the DebugAllocator with "Allocation size N bytes does not match free size N-1" at the `defer test_allocator.free(...)` site. - build.zig: add `spectests.step.dependOn(&run_spectest_generate.step)`. Without it, the test-binary compile and the generator race when zig fans out parallel build steps, intermittently producing "file contents changed during update" or `FileNotFound` on generated/index.zig. The new edge makes `zig build spectest` idempotent. Pre-existing merge fallout cleaned up while here (chain.zig): - BeamChain.init was missing the `pending_blocks` field initialisation that main's #788 pending-queue refactor added — fill it with `.empty`. - HEAD's `processFinalizationAdvancement: below PRUNE_NODE_THRESHOLD keeps pre-finalized ancestors` test (preserved from the merge) used the zig 0.15 `Dir.realpathAlloc` API and the pre-refactor `std.StringHashMap(PeerInfo)` for ConnectedPeers. Port both call sites to the current API (`Dir.realPathFileAlloc(io, ".", gpa)` and `network.ConnectedPeers`). Verification: - `zig build spectest` — All 513 tests passed (idempotent across re-invocations). - `zig build test` — All 14 test groups pass (251 unit tests). * forkchoice: drop lazy proto-array prune (PR #754 extra scope rollback) PR #754 introduced a `PRUNE_NODE_THRESHOLD = 64` gate in `chain.processFinalizationAdvancement` that skipped the post-finalization `forkChoice.rebase` call until at least 64 pre-finalized nodes sat before the finalized anchor in proto-array. The intent was to widen the grace window for in-flight attestations whose `source` / `target` / `head` still referenced an ancestor about to be pruned, where the eager rebase would otherwise surface `Unknown{Source,Target,Head}Block` from `validateAttestationData`. On reflection that mitigation is heavier than the symptom it addresses under current network conditions, and 3SF-mini already tolerates the attestation-bounce. Roll the threshold gate back so finalization advance unconditionally rebases proto-array — matches the behaviour zeam shipped before PR #754 and keeps proto-array memory bounded by finalization rather than by a node-count constant. Changes: - pkgs/node/src/constants.zig: remove `PRUNE_NODE_THRESHOLD`. - pkgs/node/src/chain.zig: * `processFinalizationAdvancement` step 5 collapses back to the unconditional `if (pruneForkchoice) try self.forkChoice.rebase(latestFinalized.root, &canonical_view);` shape (incl. dropping the `getProtoNodeIndex` lookup + invariant- violation log path that only existed to gate the threshold). * Delete the `processFinalizationAdvancement: below PRUNE_NODE_THRESHOLD keeps pre-finalized ancestors` regression test — it was pinning the now-removed contract. PR #754's other scope items remain in place: - GOSSIP_DISPARITY_INTERVALS attestation rule (spec alignment, leanSpec PR #682). - Split attestation validation (gossip interval-bound, block +1 slot). The two further scope items (lazy proto-array prune, dropping zeam's block-level future-slot rejection / time-based block queue) are intentionally NOT brought across. Verification: - `zig build spectest` — All 513 tests passed. - `zig build test` — 14/14 test groups pass. * spectest: drop duplicate `zig_snappy` entry from vendored snappyframesz The vendored copy of `pkgs/third_party/snappyframesz/build.zig.zon` carried two identical dependency entries — `.snappyz` and `.zig_snappy` — with the same URL and hash. The library's `build.zig` only references the first one (`b.dependency("snappyz", ...)`); the second is dead weight inherited from the upstream `v0.16.0` tip. Drop it here so the vendored copy doesn't propagate the noise. The same change is also stacked on the upstream PR that promotes `v0.16.0` to `main` (blockblaz/snappyframesz#10) so the next sync won't reintroduce it. * deps: pin snappyframesz to upstream main; drop vendored copy Upstream blockblaz/snappyframesz#10 merged the zig 0.16 upgrade + the empty-stream decode fix into main. Pin e95759d9 and delete pkgs/third_party/snappyframesz/. * deps: actually pin snappyframesz URL (build.zig.zon) Fix-up for 6a1a813 — that commit staged the `git rm` of the vendored copy but the `build.zig.zon` edit was never staged (the chained `git add build.zig.zon pkgs/third_party` aborted on the now-missing `pkgs/third_party` path before reaching the first argument). CI on 6a1a813 failed with `unable to open '.../pkgs/third_party/snappyframesz': FileNotFound` because the manifest still pointed `.snappyframesz.path` there. Now the manifest carries the `git+https://github.com/blockblaz/snappyframesz#e95759d9...` pin that 6a1a813's commit message claimed. * ci: rename `--fork=devnet` to `--fork=Lstar` in fixture-gen step leanSpec PR #710 renamed the `devnet` fork to `lstar`; the CI step `Generate LeanSpec fixtures` still passed `--fork=devnet` to `uv run fill`, so on the bumped submodule the command exits with `Unsupported fork for consensus layer: devnet` (exit 4) and the test job is cancelled before zig tests run. Switch to `Lstar` (the case that `fill` expects, matching the rest of this branch's leanSpec interactions). --------- Co-authored-by: Anshal Shukla <[email protected]> Co-authored-by: Parthasarathy Ramanujam <[email protected]>
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
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.
Summary
Closes #7. `decode` (and `decodeFromReader`) rejected a 10-byte stream that contained only the magic identifier chunk and no data chunks, returning `FrameError.NotFramed`. The Snappy framing spec, Go's `snappy.NewReader`, and Rust's `snap::read::FrameDecoder` all accept the same input and decode it to an empty slice; cross-client interop fixtures (notably leanSpec's `networking_codec` suite) emit exactly this form for empty input.
Fix
The terminal post-loop guard in both decode paths required `saw_data_chunk` to be set:
```zig
if (!saw_data_chunk) return FrameError.NotFramed;
```
Loosen it to accept any stream that has at least seen the identifier:
```zig
if (!saw_stream_identifier and !saw_data_chunk) return FrameError.NotFramed;
```
A stream with the identifier alone (no data chunks) is a valid empty payload and now returns an empty slice.
Why existing tests didn't catch it
`test "frame roundtrip samples"` already round-trips `""`, but the lib's own encoder appends an empty data chunk in `finish()` (`writeEmptyChunk`), so the encoded output never has only the identifier. The bug was decode-side only.
Two new regression tests cover the canonical 10-byte input (`\xff\x06\x00\x00sNaPpY`) for both `decode` and `decodeFromReader`:
```
test "decode accepts identifier-only stream as empty payload"
test "decodeFromReader accepts identifier-only stream as empty payload"
```
Both fail on `main` and pass on this branch.
Test plan
Discovered while implementing the leanSpec `networking_codec` runner in zeam PR #715.