conformance(v2:shred): local conformance for v2 shred parse harness - #1715
conformance(v2:shred): local conformance for v2 shred parse harness#1715yewman wants to merge 18 commits into
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (72.03%) is below the target coverage (85.00%). You can increase the patch coverage or adjust the target coverage.
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
27519e2 to
53f1471
Compare
344e9a8 to
11e785b
Compare
`data.size` is a u16 field straight off the wire, so `header_size + payload_size + trailer_size` overflows u16 for declared sizes near 2^16 and the old signed compare could accept packets that agave's `ledger/src/shred/merkle.rs::get_data` rejects (its bound is `size <= SIZE_OF_HEADERS + capacity` in usize). Widen the sum to u32 to match.
`parent_offset == slot` means `parent == 0`, i.e. chaining to genesis. Agave's `verify_shred_slots` permits root=parent=0 with slot>0; only `parent == slot` at `slot != 0` (`parent_offset == 0`) is illegal. Firedancer's `fd_shred_parse` rejects both, which is stricter than the protocol Agave implements and shows up as a differential-fuzzer divergence. Match agave. Also drops a stray firedancer permalink from the comment block above the parent_offset checks.
LAST_SHRED_IN_SLOT terminates the slot, so the shred must sit at the end of a fixed 32-shred FEC set: `(slot_idx + 1) % fec_shred_count` must be zero. Agave applies the same unconditionally as `misaligned_last_data_index`; rejecting at parse prevents the fuzzer from smuggling a short trailing FEC set past the fixed-shape (SIMD-0317) assumption every downstream check relies on.
Agave's `ShredFilterContext::should_discard_shred` rejects the same at the layout level via `verify_shred_slots` (in `ledger/src/shred/filter.rs`) before the shred reaches `insert_shreds`. Per-slot parent-slot *consistency* (any two data shreds in a slot must declare the same parent) is not enforced here; the conformance harness reconstructs it from admitted `in_progress` ctxs' data shreds.
RS recovery fills the erasure-protected region (header + payload)
but leaves the trailer (chained_merkle_root, merkle proof, optional
retransmitter sig) and the leading signature untouched, so this can
only re-check invariants derivable from the recovered bytes:
structural layout, slot / fec_set_idx vs the pinned ctx, variant
consistency, and positional slot_idx. Agave runs the equivalent
gauntlet in `Blockstore::handle_shred_recovery` →
`check_insert_data_shred` and marks the slot dead on failure; here
the FEC set is dropped and the harness derives the same verdict
post-hoc.
Two ctx-lifecycle details load-bearing for the re-validation not
to silently regress:
- Snapshot `data_shreds_received` into a `wire_received` local
BEFORE the RS decode. RS decode sets
`data_shreds_received = .initFull()` unconditionally to represent
the post-decode state, so a loop guard reading the live bitset
would skip every index (all bits set) and validate nothing. The
snapshot preserves the pre-decode 'wire-received' set so the
guard correctly excludes wire shreds (already validated at
admission) and iterates only over RS-recovered ones.
- Replace the pre-refactor `errdefer comptime unreachable` guard
with `errdefer state.in_progress.removeFinishedSet(fec_set_ctx)`.
The RS re-validation legitimately returns errors after ctx
creation + RS decode; a bare error return would leave the ctx in
`in_progress` with an impossible bitset
(`data_shreds_received = .initFull()` + whatever code bits were
set). A later code shred at an unseen index would bump
`totalShredsReceived()` past the SIMD-0317 threshold and trip
the `total <= fec_shred_count` assertion. Retiring the ctx on
error routes subsequent shreds through the `new_set` path
against `done` instead.
Also promotes `Shred.{min_size,max_size}` to `pub` (used from the
re-validation loop's `Packet` construction), and deletes a stale
're-validate the data shreds that we recovered' TODO in the deshred
emission path.
Move MerkleForest (per-FEC-set tree of MerkleNodes, LCRS layout, orphan adoption via chained_merkle_root pointers) out of the replay service and into a standalone library module. Motivation: the conformance shred-parse harness needs to walk the forest to derive the block-parse verdict from replay-side state, and dragging in the full replay service to do so is unworkable. The move also gains: - `MerkleForest.reset` for callers that reuse a single forest across many independent inputs (the conformance harness runs one fixture per invocation and must not leak state between them). - Publicly-exposed inner types (`MerkleNode`, `BlockPool`, `insertFecSet`, `FecOnlySlotError`) so downstream can consume them. - Equivocating siblings are legitimate forest entries: two `MerkleNode`s at the same `(slot, fec_set_idx)` with distinct merkle roots attach as siblings under a shared MerkleForest parent rather than panicking. Consumers detect equivocation structurally. `v2/services/replay.zig` shrinks by ~650 lines; the caller sites switch to `replay.MerkleForest` / `replay.insertFecSet` names.
Every shred in a FEC set carries the same `chained_merkle_root` — the merkle root of the previous FEC set (SIMD-0340). Previously the Receiver only pinned `merkle_root` at ctx creation and read `chained_merkle_root` off the completing shred; two consequences: - Two shreds in the same FEC set could declare disagreeing chained roots and both admit. Agave and firedancer reject the second. - The completion path took `chained_merkle_root` from the current- iteration shred, so the emitted `DeshreddedFecSet` value depended on arrival order. Pin `chained_merkle_root` on `FecSetCtx` at ctx creation and reject subsequent shreds whose declared value disagrees (`error.MismatchedChainedMerkleRoot`). Emit the pinned value. `DoneItem` grows the pin too, and `DoneSets.setDone` learns to accept both roots. Add `DoneSets.getRoots(id) ?FecSetRoots`, a small accessor for downstream consumers that need the completed set's pinned roots (e.g. the harness's cross-FEC chain check). `FecSetRoots` is exported publicly for that reason.
Every data shred in a FEC set declares the same `parent_offset` (merkle-hashed `DataHeader` field, so shreds sharing a merkle root share a parent). Downstream consumers need it to enforce per-slot parent-slot agreement across a slot's FEC sets — agave's `should_insert_data_shred` rejects mismatched shreds via `slot_meta.parent_slot` and marks the slot dead. Populate `DeshreddedFecSet.parent_offset` on completion from data shred index 0 (RS recovery ensures this slot is populated before emission if the wire shred was missing). Carry through into `MerkleNode` so replay's forest walk and the conformance harness's per-slot parent-offset check can both read it without dipping into per-shred state. Snapshot-bootstrap MerkleForest node uses `parent_offset = 0` (the snapshot doesn't carry it).
Signature-primary keying routes two shreds at the same `(slot, fec_set_idx)` with different signatures to different ctxs. That's cryptographically infeasible in production but reachable via sig-verify-off fuzz (mutator-synthesised signature collisions) and via case-A equivocation (leader signs the same merkle root twice with different randomised nonces; agave admits both because `check_merkle_root_consistency` is signature-blind). Add `InProgressSets.id_map: AutoHashMap(FecSetId, *FecSetCtx)` as the authoritative primary. Ctx resolution in `processPacket` becomes two-tier: signature fast-path, then id-secondary fallback. `signature_map` is a fast-path index only — under sig collision the newer ctx claims the slot and the older is reachable only via `id_map`. `removeEvictedSet` verifies sig_map still points at the evicting ctx before removing it. Case-A / case-B discrimination is now natural: the pinned merkle-root check in the `existing_set` branch (case A admits, case B returns `error.MismatchedMerkleRoot`); the `.mismatching_signature` arm on `DoneSets.lookupStatus` compares against the pinned root and returns `.fec_set_already_finished` on match or `error.MerkleRootConflict` on mismatch. `setDone` sees at most one call per `FecSetId` by construction.
Agave's ShredFilterContext bank allows shreds up to half an epoch into the future (`max_shred_slot` in `agave/ledger/src/shred/filter.rs` reads "Allow shreds up to half an epoch into the future"). Sig's harness bound was `2 *| slots_in_epoch` — twice too many epochs in one direction — so fixtures with shred slots between half-an-epoch and two epochs ahead of root were accepted here and rejected by agave. Match the agave bound.
…arker
Agave's `get_slot_entries_with_shred_info` deserializes each
data-complete batch as a wincode `BlockComponent`. Non-empty batches
decode to `EntryBatch(entries)` and skip the marker branch; empty
batches must present a valid `VersionedBlockMarker` (outer u16 tag
== 1, inner u8 tag in {0..3}, LengthPrefixed u16 size, and for tags
0..2 an inner Versioned* u8 tag == 1). Sig doesn't yet implement
`BlockComponent`, so the marker validation lives in the harness as a
parity check that mirrors agave's wincode failure surface for the
fuzz corpus's random-byte payloads.
11e785b to
57aef34
Compare
Reconstruct `ShredParseEffects.block_parse_result` and `fec_set_results` from replay-side state (`MerkleForest`) and Receiver admission-time state (`in_progress` ctxs, returned errors), mirroring agave's `mark_slot_dead_if_not_full` sinks in `check_insert_data_shred` / `handle_shred_recovery` / `check_backwards_chained_merkle_root_consistency`. New `HarnessState.forest`. Each completed FEC set the Receiver writes to the deshred ring is also inserted into the forest so the harness can walk cross-set invariants without dragging in the full replay service. `deriveBlockParseResult` folds the following into a per-slot dead set: parent-offset divergence across in-progress ctxs; SIMD-0340 chained-merkle-root check for consecutive `(slot, k) / (slot, k+32)`; case-B equivocation (defensive; the per-packet loop's `dead_slots_from_errors` normally catches these first). Also introduces `verifyTicksFromDataShreds` / `captureShredIfNew` so tick verification runs against the DATA_COMPLETE-batch view even when the Receiver's ctx has been retired.
Agave's should_insert_data_shred (blockstore.rs:3329) enforces two
order-dependent last-index invariants that drop shreds from
data_shred_cf:
- last_in_slot=true at slot_idx < slot_meta.received (a leader
can't declare 'last shred' below an already-received index)
- slot_idx >= slot_meta.last_index after some last_in_slot was
admitted
Both return InsertDataShredError::InvalidShred, which calls
mark_slot_dead_if_not_full. The rejected shred never lands in
data_shred_cf, so agave's harness emits no fec_set_result for the
enclosing FEC set.
Sig's Receiver has no slot_meta.received / last_index equivalent in
v2 (design decision doc 14 keeps admission stateless).
buildProtoEffects now walks st.data_shreds in arrival order,
tracking per-slot received and (once pinned) last_index. On either
rejection: add (slot, fec_set_idx) to suppressed_fec_sets and flag
dead_slots[slot]. Emission loop skips suppressed sets. Same shape as
the existing first_seen_parent suppression for parent-offset
conflicts.
Fixture: cd6060f18a7d216985b653895bb4dd584de2f401.fix (crash
898b95ea8d10, slot=38219: idx=31 last_in_slot=true rejected because
idx=34 fec_set=32 admitted first).
57aef34 to
b6764dd
Compare
There was a problem hiding this comment.
Pull request overview
This PR brings the v2 shred parse pipeline to local differential-fuzz parity with agave by adding a full conformance harness for shred parsing/FEC completion and tightening receiver/replay protocol-invariant enforcement surfaced by the fixture corpus.
Changes:
- Adds a local
shred_parseconformance harness that drives the v2 Receiver, captures completions, and derives block-level verdicts using a replay-sideMerkleForest. - Tightens v2 shred receiver invariants (root pinning, equivocation/id routing, RS-recovered shred re-validation, parent/root bounds, last-index alignment).
- Extracts
MerkleForestandinsertFecSetintov2/lib/replay.zigand updates the replay service to use the shared module and newparent_offsetfield.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| v2/services/replay.zig | Switches service to replay.MerkleForest / replay.insertFecSet, adds synthetic-root parent_offset. |
| v2/lib/shred/receiver.zig | Adds additional admission checks, pins chained root, re-validates RS-recovered shreds, and enriches done-set tracking with pinned roots. |
| v2/lib/shred.zig | Adds parent_offset to deshredded completions and aligns shred size validation with agave using a u32 bound. |
| v2/lib/replay.zig | Extracts MerkleForest/MerkleNode and insertFecSet into a reusable library module with reset support. |
| conformance/src/shred_parse.zig | Implements local conformance harness logic, including replay-backed cross-FEC validation and tick verification tweaks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // fec set is already being built. This branch will be taken for 31/64 shreds (assuming | ||
| // zero packet loss). | ||
| var shred_merkle_root: Hash = undefined; | ||
| try shred.merkleRoot(&shred_merkle_root); |
There was a problem hiding this comment.
if you're going to unconditionally pre-calculate merkle root and allow different signatures, why not just key by merkle root instead of signature?
i'd rather use the optimal design for the current protocol, rather than use the optimal design for some other protocol that we prefer plus some hacks to make it work with the actual protocol we're stuck with. it feels convoluted with the current approach
There was a problem hiding this comment.
I understand where you are coming from, but that would close the door to a future optimisation which we should do, but is not a priority right now. Added a comment for clarity. 060a521
Follows dnut's review guidance (r3670525232): move MerkleForest / MerkleNode / insertFecSet out of v2/components/replay/api.zig into a sibling forest.zig surfaced via component.zig, keeping api.zig as a lightweight public surface. - v2/components/replay/forest.zig: MerkleForest/MerkleNode/insertFecSet impl. - v2/components/replay/component.zig: re-exports the forest symbols. - v2/components/replay/api.zig: trimmed back to lightweight pool/exec types. - build.zig: two-pass component wiring so the replay component's impl files can import shred_api (declared via extra_api_deps). - v2/services/replay.zig: forest.MerkleForest and forest.MerkleNode reached via the 'replay' component module; api.zig now only owns BlockPool etc. - conformance/build.zig: adds replay_api and replay component modules. - conformance/src/shred_parse.zig: sig_v2.replay.MerkleForest -> replay_impl.MerkleForest; sig_v2.shred.FecSetId -> shred_api.FecSetId.
Collapse the three parallel pending_component_names / pending_comp_dirs / pending_apis ArrayLists into one Pending struct list, hoist the base imports (lib/tracy/build-options) out of both passes, and lift the sibling-api lookup into a small findApi helper so the extra_api_deps branch reads as a single line instead of a nested for/for-else/@Panic.
a815bd2 to
e30360a
Compare
Intent
Bring the v2 shred parse pipeline to differential-fuzz parity with agave on the local
shred_parsefixture corpus.The work splits three ways:
Harness —
conformance/src/shred_parse.zig. Implementsol_compat_shred_parse_v1end-to-end: decodeShredParseContext, drive every shred through the v2 Receiver, and emitShredParseEffects(per-FEC-set outcomes + block-level verdict) derived from Receiver admission state and the replay-sideMerkleForest. Adds parent-offset-conflict / SIMD-0340 chained-root / equivocation reconstruction,VersionedBlockMarkervalidation for emptyEntryBatchpayloads, the half-epoch future-slot bound, and mirrors agave's order-dependentLastIndexConflictadmission check.Receiver —
v2/lib/shred/receiver.zig,v2/lib/shred.zig. Tighten protocol-invariant enforcement surfaced by the corpus: pinchained_merkle_rooton the FEC-set ctx (reject disagreement, emit the pinned value); id-secondary erasure-set routing so case-A equivocation and sig-verify-off signature collisions still collapse to one ctx perFecSetId; re-run structural / positional / variant checks on RS-recovered data shreds (with awire_receivedsnapshot so the guard sees the pre-decode bitset, and anerrdeferthat retires the ctx on failure); rejectLAST_SHRED_IN_SLOTat misaligned data index; drop data shreds whose parent slot sits below the current root; acceptparent_offset == slotat any slot (chaining to genesis); widen theheader + payload + trailerbound check to u32 to match agave'smerkle::get_data.Replay —
v2/lib/replay.zig,v2/services/replay.zig. ExtractMerkleForest(LCRS tree ofMerkleNodes, orphan adoption viachained_merkle_root) into a standalone library module so the harness can walk cross-FEC-set invariants without pulling in the replay service. GainsMerkleForest.reset(harness reuses one forest across fixtures) and sibling attachment for equivocating(slot, fec_set_idx)entries so consumers detect equivocation structurally.Notes
-Ddebug-skip-shred-sig-verify=true; shred-version enforcement stays on.processPacketare not visible to the harness's tick-verify path — a known fidelity gap versus agave's blockstore-driven reconstruction.MerkleForest), not in the Receiver.BlockComponentdecoding lives in the harness pending first-class support in sig.