fix(eth2api): filter proposer duties to the cluster's validators

[iamquang95]

Fix #565

This should be included in the MVP since there are many warn logs

[iamquang95]

@claude review this PR

[claude]

Claude finished @iamquang95's task in 4m 12s —— View job

---

Review: fix(eth2api): filter proposer duties to the cluster's validators

• Gather context (diff, Charon Go reference, call sites)
• Review crates/eth2api/src/extensions.rs
• Review crates/core/src/scheduler.rs
• Post review feedback

Verdict: approach is correct and matches Charon. A few simplification / robustness nits below, nothing blocking.

---

Parity check ✅

Charon calls s.eth2Cl.ProposerDuties(ctx, opts) with Indices: vals.Indexes() (core/scheduler/scheduler.go:420-425), but /eth/v1/validator/duties/proposer/{epoch} has no validator parameter — so the narrowing happens inside go-eth2-client, exactly the layer this PR puts it in. Keeping the Ignoring unexpected proposer duty warn + InvalidDutyPubkey check in the scheduler preserves the rest of resolveProDuties verbatim. Good call not filtering in the scheduler loop.

I could not run cargo test / cargo clippy in this environment (command approval), so the notes below are from reading only. I did verify statically that the new epoch check won't break the existing scheduler tests: proposer_duties_response in crates/testutil/src/beaconmock/defaults.rs:537-542 always emits slots_per_epoch * epoch + offset with offset < slots_per_epoch, so mock duties are always in-epoch.

---

1. Every duty is parsed twice, and the second parse loses the error detail

extensions.rs:389-399 parses validator_index, slot, and the pubkey — then discards all three (except index, used only for the filter) and returns the raw GetProposerDutiesResponseResponseDatum. scheduler.rs:973-979 immediately re-parses the same three fields via TryFrom (crates/core/src/types.rs:568-576).

Beyond the wasted work, the fidelity is asymmetric: a malformed duty for a filtered-out validator surfaces a precise ParseError("proposer duty slot"), while the identical defect on a kept validator gets flattened to UnexpectedResponse by the scheduler's .map_err(|_| ...UnexpectedResponse). Same input class, two different errors, and the less useful one is the one you'll actually hit in production.

Consider returning parsed data instead of the raw datum — e.g. a small ProposerDuty { index, slot, pubkey } — and letting the scheduler map that infallibly into ProposerDutyDefinition. Fix this →

2. The intermediate validated Vec is unnecessary

extensions.rs:387-418 builds a Vec<(u64, Datum)> for the whole epoch just to filter it afterwards. The comment says filtering first would let a malformed unrequested duty slip through — true if you skip before validating, but you can validate and filter in the same pass without weakening that guarantee, because the parse errors return early regardless of whether the duty is kept:

let mut result = Vec::new();
for duty in duties {
    let index = /* parse */;
    let slot = /* parse */;
    let _: phase0::BLSPubKey = /* decode */;
    if slot / slots_per_epoch != epoch {
        return Err(EthBeaconNodeApiClientError::DutySlotOutsideEpoch { slot, epoch });
    }
    if indices.is_empty() || indices.contains(&index) {
        result.push(duty);
    }
}

Identical semantics, one allocation instead of two, and the "validate everything" property is still enforced by the early returns. Fix this →

3. Dead error branch on checked_div

extensions.rs:404-406: slots_per_epoch == 0 is already rejected at line 370, so checked_div can never return None and ZeroSlotDurationOrSlotsPerEpoch there is unreachable and untestable. I assume checked_div is there to satisfy arithmetic_side_effects = "deny" (Cargo.toml:168) — if so, taking slots_per_epoch: NonZeroU64 would encode the invariant in the type and let you write slot / slots_per_epoch.get() honestly, with the zero check moved to the caller. Failing that, an expect with a comment pointing at line 370 reads more truthfully than a fake error path.

4. Missing test coverage

The four new tests are well-targeted (I especially like rejects_malformed_unrequested_duty — it pins the "validate before filter" contract). Gaps worth one line each:

• non-Ok response → UnexpectedResponse (extensions.rs:381)
• slots_per_epoch == 0 → early error (extensions.rs:370)
• malformed slot / validator_index (only pubkey is covered)

5. Nit: test fixture makes index and slot indistinguishable

proposer_duties_server calls proposer_duty(i, i), so validator index == slot in every case. A filter that accidentally matched on slot instead of validator_index would pass keeps_only_requested_indices unnoticed. Offsetting them (e.g. proposer_duty(i, i + 2), keeping slots inside epoch 0) makes the assertion actually discriminate.

6. Note, not a defect: failure blast radius

The out-of-epoch check runs before filtering, so one bogus duty for a validator that isn't ours now hard-fails the entire epoch's proposer-duty resolution. That's the right call for parity (go-eth2-client decodes and range-checks the full response before applying Indices), and the alternative — silently trusting a BN that reports wrong slots — is worse. Just flagging that the PR is motivated by reducing sensitivity to other operators' validators, while this one path increases it.

7. Nit: comment at scheduler.rs:993

"Unreachable unless the client's filter let something through" holds only because resolve_duties returns early on vals.is_empty() (scheduler.rs:466) — otherwise empty indices means unfiltered and every duty hits the warn. Worth saying so, since fetch_proposer_duties is a free function that tests call directly.

---

_{• branch feat/fix-565}

[emlautarom1]

LGTM.