feat(ci): codecov fallback - #1718
Merged
Merged
Conversation
A non-genesis slot must have parent < slot, but `parent_offset == slot` (i.e. parent == 0, chaining to genesis) is legal at any slot: agave `verify_shred_slots` (ledger/src/blockstore.rs) permits root=parent=0 with slot>0. Firedancer's parser rejects this case (fd_shred.c `fd_shred_parse`), but that is stricter than the protocol Agave implements. The differential fuzzer surfaces the divergence. Also drop the stale [firedancer] URL comment that decorated the data-branch opening — the surrounding text no longer tracks the linked commit.
`data.size` is a u16 field straight off the wire. The check `effective_size < header_size + payload_size + trailer_size` performs its arithmetic at u16, which wraps for declared sizes near 2^16 and lets a malformed shred with a bogus `data.size` slip past this bound. Widen the operands to u32 so the comparison matches the underlying usize arithmetic agave uses in `merkle.rs::get_data`, which bounds `size <= SIZE_OF_HEADERS + capacity`. Prevents a ReleaseSafe panic on the corresponding overflow reported by the differential fuzzer.
LAST_SHRED_IN_SLOT terminates the slot; SIMD-0317 fixes the FEC-set shape at 32 data + 32 coding, so the terminating shred must land at `slot_idx + 1 ≡ 0 (mod 32)`. Anything else lets a short trailing FEC set past the fixed-shape assumption downstream code relies on. Agave enforces this unconditionally via `misaligned_last_data_index` in `ledger/src/blockstore.rs`; unlike the neighbouring `UnexpectedDataCompleteShred` check, there is no feature gate.
Add `Receiver.dead_slots` as a downstream signal (not an insertion gate) for slots that hit a fatal protocol violation. Consumers OR `dead_slots.contains(slot)` into their reject verdict; the deshred- ring emission step at the end of `processPacket` suppresses output for any slot in the set. Agave enforces the same insertion / replay separation: `mark_slot_dead_if_not_full` writes the dead-slot flag, `get_slot_entries_with_shred_info` returns `DeadSlot` to the replay side, and the insertion path (`check_insert_data_shred`) does not consult it. Adds: - `Receiver.allocator` (borrowed) and `dead_slots` field. - `init`/`deinit`/`reset` and `updateSlotRange` prune. - `markSlotDead` API. OOM is silently dropped: the originating error already surfaced. - Downstream-only gate at the deshred-ring write site. - Conformance harness reads `dead_slots.count() > 0` into `block_parse_result = REJECTED_INVALID_HEADER`. No callers of `markSlotDead` yet; follow-up commits introduce the per-invariant triggers.
Every data shred in a slot must declare the same parent (`shred.slot - parent_offset`); a slot has a single position in the fork tree. Agave enforces this in `Blockstore::should_insert_data_shred`: mismatch returns `InvalidShred` and triggers `mark_slot_dead_if_not_full`. Without this gate, fuzz-crafted shreds whose proof bytes collide on one merkle root can smuggle mismatched parents past the merkle / chained-merkle checks. Adds: - `Receiver.slot_parents: AutoHashMapUnmanaged(Slot, Slot)`, first- seen parent for each slot. Pruned in `updateSlotRange`. - Hoisted parent-slot check in `processPacket` above ctx routing so a mismatched parent cannot mutate FEC-set state before rejection. - `error.ShredParentBeforeRoot` for shreds whose declared parent is older than the current root (agave's `ShredFilterContext::should_discard_shred` rejects these at the filter layer before insertion). - `error.ParentSlotMismatch` on a mismatch against the pinned first-seen parent; calls `markSlotDead(slot)` before returning.
…t on signature collisions Two related invariants. **Within-set chained_merkle_root.** Every shred in one FEC set declares the same `chained_merkle_root` (the merkle root of the previous FEC set); agave enforces this in `Blockstore::check_chained_merkle_root_consistency`. Pin the value on first-shred ctx creation and reject subsequent shreds whose declared root disagrees (`error.MismatchedChainedMerkleRoot`). Also use the pinned value when emitting the completed set instead of the current-iteration shred's copy, so completion output is deterministic regardless of arrival order. **Signature-collision dead-slot.** Two paths admitted a shred with a signature already known to the Receiver but a distinct `(slot, fec_set_idx)`: - `.mismatching_signature` on the `done` map: same id, different signature \u2014 leader equivocation or fuzz-crafted collision. - `containsId(fec_set_id)` on the `in_progress` map: same id observed in-flight with a different signature. Both return an error today but leave the slot recoverable. Agave hits the equivalent case via `mark_slot_dead_if_not_full`. Add `markSlotDead(shred.slot)` before each return so the block is consistently rejected downstream.
FEC set N's `chained_merkle_root` must equal the `merkle_root` of
FEC set N-32 in the same slot. Agave enforces this at insertion via
`Blockstore::check_forward/backwards_chained_merkle_root_consistency`
(the SIMD-0340 "encompassing" checks between fixed FEC-set boundaries
at `fec_set_idx = k * DATA_SHREDS_PER_FEC_BLOCK`); a break marks the
slot dead through `PossibleDuplicateShred::{Chained,FixedFECChained}
MerkleRootConflict`.
Cross-FEC check runs on every arriving shred (not just at completion),
so a single arriving shred from either side of a break is enough to
detect and mark the slot dead. Because signature-keyed ctx routing can
hide a neighbour FEC set behind a signature collision, an auxiliary
`merkle_root_pins` map records the check target independently of
routing: every structurally-parseable shred is pinned by
`(slot, fec_set_idx)` regardless of whether it makes it into a ctx.
Duplicate pin at the same id with a mismatching `merkle_root` is a
per-FEC-set merkle-root conflict (agave's
`check_merkle_root_consistency`) and marks the slot dead.
Adds:
- `FecSetRoots` pair type, plus `Receiver.merkle_root_pins`, its
init/deinit/reset/updateSlotRange prune, and
`pinFecSetRoots`/`lookupFecSetRoots` helpers. Lookup falls back
through in-progress ctx \u2192 done map \u2192 pin map, in decreasing
admission-fidelity order.
- `InProgressSets.getCtxById` / `fecSetIdOf` so `lookupFecSetRoots`
and the signature-collision check can index by `FecSetId` rather
than by signature.
- `DoneSets` grows `DoneItem` with the pinned roots and exposes
`getRoots`; `setDone` takes the pair.
- `processPacket` hoists `shred.merkleRoot()` above ctx routing so
both the pin write and the cross-FEC lookup use the shred's own
recomputed root. On `(slot, fec_set_idx)` collision inside a
signature-keyed ctx, return `error.SignatureCollisionDifferentFecSet`.
- Conformance harness reads `receiver.dead_slots` into
`block_parse_result` and drops its now-redundant harness-side
cross-FEC chain check.
RS recovery reconstructs missing data shreds by filling the erasure-protected region (header + payload); the trailer (chained_merkle_root, merkle proof, optional retransmitter sig) and the leading signature are left as whatever bytes happened to be in the backing buffer. Without a re-check, a maliciously crafted set of code shreds could reconstruct "data shreds" that violate structural invariants (merkle_count past cap, data_complete on a non-tail index, flag bits contradicting last_in_slot -> data_complete, etc.) and the malformed bytes would go straight to replay. Snapshot `data_shreds_received` before `reed_sol.recover64` so we know which indices were reconstructed, and after recovery run each reconstructed shred through `fromPacketChecked` plus positional checks against the ctx-pinned invariants (slot, fec_set_idx, variant, positional `slot_idx`). The merkle and chained-merkle roots are pinned on the ctx from the first wire shred, so a recovered shred cannot disagree with values it doesn't carry. On any recovered-shred failure, `markSlotDead(shred.slot)` + `error.RecoveredShredMalformed`. Agave runs the same gauntlet in `Blockstore::handle_shred_recovery` -> `check_insert_data_shred`; firedancer `fd_fec_resolver` re-parses recovered buffers against `fd_shred_parse` and discards failures. Also drops `pub` from `Shred.min_size`/`max_size` in `v2/lib/shred.zig` \u2014 they were unused outside the module and are now only referenced from within the shred module (`Receiver`'s re-validation loop constructs a `Packet` with `len = lib.shred.Shred.min_size`).
hamza-syndica
marked this pull request as ready for review
July 13, 2026 13:59
hamza-syndica
marked this pull request as draft
July 13, 2026 14:16
Contributor
https://github.com/Syndica/sig/actions/runs/29311329847/job/87015558865?pr=1718 vs Actual Comment: 
Merged the code of this pending PR to test. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Linux CI workflow to avoid Codecov upload failures on external fork pull requests and adds a no-secrets, local patch coverage fallback intended to surface changed-lines coverage in CI output (per #1696).
Changes:
- Add a
diff-coverstep onpull_requestruns to generate and publish a patch coverage report to the GitHub Actions step summary. - Gate the Codecov upload step so it runs only on non-PR events or PRs originating from the same repository (skips external forks).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dnut
suggested changes
Jul 14, 2026
Co-authored-by: Drew Nutter <dnut@users.noreply.github.com>
Co-authored-by: Drew Nutter <dnut@users.noreply.github.com>
dnut
approved these changes
Jul 15, 2026
jbuckmccready
approved these changes
Jul 15, 2026
hamza-syndica
enabled auto-merge
July 15, 2026 14:19
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.
Alternative to codecov that works for external contributors.
Closes #1696