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).
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.