feat(core): tracker inclusion ported from charon

[mskrzypkows]

Inclusion checking for broadcast duties ported to pluto.

[mskrzypkows]

@claude review

[claude]

Claude finished @mskrzypkows's task in 4m 19s —— View job

---

Review: feat(core): tracker inclusion ported from charon

I compared crates/core/src/tracker/inclusion.rs against charon/core/tracker/inclusion.go (the inclusionCore portion). Overall this is a faithful, careful port of the I/O-free core — control flow, error handling, callback ordering, metrics, and trim/cleanup semantics all match the Go source. A few notes below, mostly about test coverage and one untested code path.

✅ Verified equivalent to Go

• submitted / Submitted: duty-type gating via incl_supported() matches Go's inclSupported() (Proposer always, Attester/Aggregator behind AttestationInclusion). Synthetic proposals report inclusion immediately and skip caching — same as Go. Dropping the core.SignedData callback arg is fine: Go's Tracker.InclusionChecked(_ core.SignedData, …) ignores it (tracker.go:830).
• is_synthetic() unifying Go's blinded/non-blinded branches is correct — graffiti() (versioned.rs:107) covers all blinded variants, so it's actually cleaner than the Go original.
• check_block_and_atts: the err→warn→still-report-included behavior matches Go's else if !ok fall-through exactly (Err(e) => acts.push(Include)). Callback order (att_included_fn then tracker_incl_fn) and the beacon_committees GC (checked_sub(32) ≡ Go's slot >= InclMissedLag) match.
• trim, report_missed, report_att_inclusion, metrics (inclusion_missed_total, inclusion_delay) all line up with Go.
• Collecting acts then mutating in a second pass (vs Go deleting mid-iteration) is observably identical since callbacks only receive &Submission/&Block.

⚠️ Untested: the Electra/Fulu offset path (most important)

check_attestation_inclusion (inclusion.rs:446-472) ports the Electra/Fulu committee-offset arithmetic, but no test exercises it. The inclusion test feeds non-versioned Attestation/SignedAggregateAndProof, which fail the downcast_ref::<VersionedAttestation>() / VersionedSignedAggregateAndProof and hit the error→still-included branch — so the genuine Ok(true) bit-matching path (phase0 contains and the Electra bit_at(previous_validators + offset) computation) is never run.

Worth adding a test that builds a real VersionedAttestation whose data root matches a seeded block attestation with overlapping aggregation bits, plus an Electra case that validates the previous_validators + validator_committee_index offset. The offset math is the subtlest logic in the file and currently has zero coverage. Add Electra/phase0 positive-path inclusion tests →

This path also depends on the follow-up networked driver to reconstruct the block attestation's aggregation bits as the full per-committee concatenation (Go's conjugateAggregationBitsElectra / updateAggregationBits, not in this PR). The take(preceding) + bit_at logic is only correct if the driver pads/orders committee bits exactly as Go does — worth a // TODO or doc note pinning that contract so the follow-up doesn't silently diverge.

Minor

• inclusion.rs:307 — inside the None downcast arm of check_block, the log reads duty = %self.submissions[&key].duty via a redundant map re-lookup, even though sub is already bound on line 302. Use sub.duty. Fix this →
• Intentional, safe divergences from Go (no action needed, just noting): Rust's take(preceding) silently caps when the committee index exceeds beacon_committees.len(), whereas Go's block.BeaconCommitees[idx] would panic; report_att_inclusion uses saturating_sub where Go's blockSlot - attSlot could wrap. Both are strictly safer.
• A phase0 length-mismatch in contains() surfaces as BitfieldError::DifferentLength → warn → counted as included. This matches Go (prysm Contains returns an error on differing lengths, handled the same way), so it's correct — just non-obvious.
• check_block's unreachable!("bug: unexpected type") (and check_block_and_atts' default arm) mirror Go's panic. Correctness relies on the driver only calling check_block when AttestationInclusion is disabled (so only proposers are cached), same invariant as Go.

Note

I was unable to run cargo test/clippy in this environment (command permission denied), so the above is from static comparison against the Go source. The #![allow(dead_code)] for the not-yet-wired core items is reasonable and documented.

Nice work — the port reads cleanly and the doc comments are excellent. The main ask is positive-path test coverage for the attestation/Electra logic before the follow-up driver lands on top of it.
· tracker/inclusion

[mskrzypkows]

@claude I corrected your suggestions, check them

[claude]

Claude finished @mskrzypkows's task in 1m 50s —— View job

---

Re-review: corrections look good ✅

I checked the three follow-up commits (55df124, b4436da, 5aa2bfa) against my earlier notes.

• Reviewed redundant map-lookup fix (inclusion.rs:307)
• Reviewed new positive-path tests (phase0 + Electra)
• Confirmed tests compile & pass (CI Build and test code ✅)

1. Redundant map lookup — fixed correctly

55df124 swaps self.submissions[&key].duty for the already-bound sub.duty in the None downcast arm of check_block. Exactly the change requested, no behavior change.

2. Electra/phase0 positive-path coverage — this is the important one, and it's well done

b4436da adds the four tests I asked for, and they genuinely exercise the previously-uncovered Ok(true)/Ok(false) branches rather than the error fall-through:

• phase0 (check_attestation_inclusion_phase0_contains / _not_contained): builds a real VersionedAttestation (Deneb) whose att_data_root matches a seeded block attestation, then asserts the genuine att_bits.contains(&sub_bits) path — block {0,1} ⊇ {0} → included; block {1} ⊉ {0} → not included. ✅

• Electra offset (check_attestation_inclusion_electra_offset / _not_included): this is the subtle one. Committee 0 has 3 validators, committee 1 has 4; the submission sits at committee_index=1, validator_committee_index=2. So previous_validators = 3 (sum of preceding committees) and offset = 2 → global bit 5. The test asserts block bit 5 set → included, and bit 4 set (5 unset) → not included. That precisely pins the previous_validators + validator_committee_index arithmetic at inclusion.rs:462-471 — exactly the contract I flagged as zero-coverage. ✅

I verified the helpers used (att_data, submission, BitList::with_bits, BitVector::with_bits, bit_at, contains) all exist with matching signatures, and the Block is seeded with the matching att_duties + beacon_committees so the offset lookup resolves rather than short-circuiting.

3. Clippy — 5aa2bfa

assert_eq!(x, true) → assert!(x) and the false variants → assert!(!x). Correct fix for clippy::bool_assert_comparison.

Verdict

All three of my earlier points are addressed correctly, and the new tests close the most valuable gap (the Electra committee-offset math). CI's Build and test code job is green, so the tests compile and pass. The only outstanding item from my original review is the follow-up networked driver contract (the block's aggregation bits must be reconstructed as the full per-committee concatenation, à la Go's conjugateAggregationBitsElectra) — out of scope for this PR, just worth keeping pinned for the follow-up. Nothing blocking here. Nice work. 👍
· tracker/inclusion