feat(core): implement attester_duties, sync_committee_duties, attestation_data handlers

[varex83]

Summary

Implements three validator-API handlers, stacked over PR #449.

• attester_duties (POST /eth/v1/validator/duties/attester/{epoch}) — uses auto-gen GetAttesterDuties* envelope, accepts both numeric and string-encoded validator index arrays via a new ValIndexes dual-format deserializer, performs pubshare swap on the upstream response (unknown pubkey rejected).
• sync_committee_duties (POST /eth/v1/validator/duties/sync/{epoch}) — same shape as above, reuses ValIndexes.
• attestation_data (GET /eth/v1/validator/attestation_data) — query-string slot/committee_index, served from the local DutyDB (MemDB::await_attestation) rather than upstream. Component now holds an Arc<MemDB>.

All three return the auto-generated response envelope directly; attestation_data uses a small hand-rolled wrapper around phase0::AttestationData to avoid pointless typed-↔-string conversion.

Test plan

• cargo +nightly fmt --all --check
• cargo clippy -p pluto-core --all-targets --all-features -- -D warnings
• cargo test -p pluto-core validatorapi:: — 14/14 passing (5 new tests across router + component)

[varex83]

@varex83agent /review-pr

[varex83agent]

Implements three validator-API handlers (attester_duties, sync_committee_duties, attestation_data) with reasonable parity to Charon and unit tests for the swap helpers, the dual-format ValIndexes deserializer, and the DutyDB-backed attestation path. Code style is idiomatic. However, two bug-level issues need fixing before merge, plus several major hardening gaps on this attacker-reachable surface.

Bugs (must fix):

1. attestation_data awaits MemDB with no timeout, no request-cancel propagation, and no eviction signal — a request for a slot that never arrives becomes a permanent zombie task, contending on every legitimate store via notify_waiters. Charon plumbs the request ctx through awaitAttFunc; the Rust port loses this entirely. Wrap the await in a tokio::time::timeout (matching the slot deadline) and either wake on delete_duty or plumb a per-request CancellationToken.
2. format!("unexpected upstream … response: {other:?}") interpolates the upstream beacon node's Debug output — including BlindedBlock400Response.message and any stacktraces field — directly into the client-visible ApiError.message. This forwards upstream internals (and possibly file paths) to anyone reachable on the validator API. Move {other:?} into with_source(...) and use a generic message.

Majors:

• Component::new_insecure (which disables submit-side signature verification) is plain pub fn, not #[cfg(test)] — once submit endpoints land, this is a footgun.
• new_router installs no DefaultBodyLimit, no TimeoutLayer, and no concurrency cap. Combined with the unbounded await above, slowloris-style requests are trivially possible.
• All non-Ok upstream variants (BadRequest / InternalServerError / ServiceUnavailable / Unknown) are collapsed under 502. A syncing upstream (503) should propagate as 503 so the VC retries with backoff; a 400 from upstream means our forwarded request was bad.
• Test coverage is missing for: cancellation/timeout on attestation_data, the upstream non-Ok variants, malformed slot/committee_index query params, and empty indices body.

Minors and a nit inline. Verdict: REQUEST_CHANGES — please address the two bug-level findings before merge; majors can land in a follow-up or this PR at your discretion.

[varex83]

@varex83agent /review-pr

[varex83agent]

This PR wires up three validator-API handlers (attester_duties, sync_committee_duties, attestation_data) and is generally high quality — the hardening from the prior review round is solid (request timeouts, 64 KiB body cap, streaming ValIndexes visitor with an 8192 cap, no info leak into client-visible error messages, new_insecure gated to #[cfg(test)]), the await_data refactor is free of the classic lost-wakeup race (notified() is created before the lock-check), and the pubshare-swap parity with Charon (reject-unknown for attester/sync, pass-through for proposer) is correct. Test coverage is good.

Two must-fix issues before merge:

1. Unbounded memory growth in the evicted_* HashSets (dutydb/memory.rs). The four eviction-tracking sets introduced in the fix commit are only ever inserted into and never pruned, so every evicted slot/key is retained for the entire process lifetime — an unbounded leak in a long-running daemon and a divergence from Charon, which keeps no such set. This optimization needs a bound (e.g. a min_live_slot watermark, or pruning below a retention horizon).
2. Missing committee_index=0 dual-write breaks post-Electra VCs (dutydb/memory.rs). Charon's storeAttestationUnsafe deliberately stores each attestation twice — under the real committee index and under index 0 — so post-Electra VCs that request attestation_data?committee_index=0 resolve. Pluto's store_attestation writes only the real index; this PR makes the attestation_data endpoint live, so a conformant Electra VC asking for index 0 will now time out (408) and fail to attest. (Root cause is in pre-existing store logic, but it becomes reachable here.)

The remaining findings are minor/nit: an error-envelope inconsistency on the {epoch} path param, a ValIndexes parity superset (mixed numeric/string accepted where Charon is all-or-nothing), the std::io::Error::other debug-string carrier smell, and the fact that the source attached "for debug logging" is never actually logged. Requesting changes for the two majors; everything else is author discretion.

[varex83agent]

/loop-review-pr summary

Ran 3 review-and-fix iterations (plus 2 verification passes) against this PR, each driving the same four-agent pipeline as /review-pr (functional-equivalence, security, rust-style, code-quality). No inline comments were posted during the loop — this is the single summary.

Terminated by: completion (PR reached ideal state) — internal review produced zero bug/major findings for two consecutive passes and all gates are green.

Quality gates (final, full workspace, --all-features)

• cargo +nightly fmt --all --check — pass
• cargo clippy --workspace --all-targets --all-features -- -D warnings — pass
• cargo test --workspace --all-features — pass (40/40 test groups, 0 failures)

Resolved during the loop

Bugs (0)

• none

Major (1)

• Unbounded MemDB eviction state — the four evicted_* tombstone HashSets were insert-only (one entry per evicted slot, forever), an unbounded memory leak on a long-running node. Replaced the three slot-keyed sets with O(1) Option<u64> high-water marks (attester/proposer/sync-contribution); dropped aggregation eviction tracking since it is awaited by root only and relies on the request timeout, matching Charon's Go dutydb (which keeps no eviction record at all). Added a regression test. — crates/core/src/dutydb/memory.rs — fix in 6a8a94c

Minor (1)

• Malformed duties request body bypassed the {code,message} envelope and returned axum's default 422 for type errors. Now extracts Result<Json<ValIndexes>, JsonRejection> and maps via json_rejection_to_api_error to a uniform 400 (mirroring Charon's unmarshal and the router's own query-rejection path); the 413 body-size DoS defense is preserved explicitly. Added a router test. — crates/core/src/validatorapi/router.rs — fix in b5eb0ca

Nits (1)

• Regression-test arithmetic style (SLOT - 1 → SLOT.saturating_sub(1)) for consistency — folded into 6a8a94c

Outstanding (minor/nit — do not block "ideal"; left for author judgement)

• [minor] A duties POST with a missing Content-Type header returns 400, whereas Charon's wrap treats an absent/empty Content-Type as application/json and parses the body (→ 200). Genuine but narrow Go-parity edge case; real validator clients always send the header. The fix is invasive (a content-type-injecting middleware, or swapping Json for Bytes + manual serde_json), so it was not applied — flagging for your call.
• [minor] ValIndexes accepts mixed numeric+string arrays (e.g. [1, "2"]) that Charon rejects. This appears intentional — the PR has a test (val_indexes_accepts_mixed_elements) asserting it — so it was left as-is.
• [nit] Accepted/intentional deviations: VAL_INDEXES_MAX_LEN (8192) + 64 KiB body cap (DoS hardening Charon lacks); node_version arch token uses Rust naming (x86_64) vs Go's amd64 (cosmetic, no client impact); per-handler timeouts (12s/24s) vs Charon's uniform 10s (all resolve to 408).
• [nit] wake() takes DutyType by value (the clone is actually required — DutyType isn't Copy and delete_duty consumes the Duty); parse_bls_pubkey(&str) vs impl AsRef<str>; json_rejection_to_api_error could use an @-binding instead of re-constructing PAYLOAD_TOO_LARGE; store() holds the write lock across the async deadliner.add() (pre-existing, not worsened by this PR); the high-water-mark soundness invariant (deadliner expires in non-decreasing slot order) could carry a debug_assert.

Verdict

PR is ideal — all bug/major findings resolved, the dutydb concurrency fix was independently verified sound (monotonic-slot invariant, no TOCTOU, no lost wakeup), and fmt/clippy/test are green across the full workspace. Remaining items are minor/nit and either intentional or low-value.

ai

[iamquang95]

Why 12? Should be 10 seconds right?
defaultRequestTimeout = 10 * time.Second

[varex83agent]

Good catch — fixed in 12e2263 (timeout 12s → 10s, doc comment now points to core/validatorapi/router.go:61).

[iamquang95]

missing content-type is accepted in charon

if contentHeader == "" || strings.Contains(contentHeader, string(contentTypeJSON)) {
			typ = contentTypeJSON
		} else if strings.Contains(contentHeader, string(contentTypeSSZ)) {
			typ = contentTypeSSZ
		} else {
			writeError(ctx, w, endpoint, apiError{
				StatusCode: http.StatusUnsupportedMediaType,
				Message:    "unsupported media type " + contentHeader,
			})

			return
		}

[varex83agent]

Fixed in 12e2263 — added an enforce_json_content_type middleware on the duties POST routes that mirrors Charon's policy (missing or JSON Content-Type passes; anything else → 415 with the offending value in the message). SSZ is not supported yet, so for now non-JSON is rejected; the middleware is the right seam to add SSZ later. Two new router tests cover the missing-CT and non-JSON cases.