feat(core): implement validatorapi attestation + aggregation handlers#492
Open
varex83agent wants to merge 20 commits into
Open
feat(core): implement validatorapi attestation + aggregation handlers#492varex83agent wants to merge 20 commits into
varex83agent wants to merge 20 commits into
Conversation
Threads the Handler through Axum state via AppState<H> + with_state, wires the node_version route to the real handler, and adds a TestHandler mock that future PRs will extend per-endpoint.
Re-uses the auto-generated pluto_eth2api envelopes (GetProposerDutiesResponseResponse, GetVersionResponseResponse) as the on-the-wire shape rather than hand-rolling parallel types. node_version is migrated to the same pattern; the body.rs hand-rolled wrapper module is removed.
Drops the per-handler generic parameter and routes through Arc<dyn Handler> via AppState. The Handler trait is object-safe (Send + Sync + 'static + async_trait-generated methods), so this is a pure type change with no surface impact.
Adds the Handler impl that the router has been calling through.
node_version returns the obolnetwork/pluto/{version}-{commit}/{arch}-{os}
identity string; proposer_duties calls the upstream beacon node and
rewrites known DV root public keys to this node's public share so the
validator client sees keys matching its keystore. The remaining 17
trait methods are unimplemented!() stubs that land per-PR as their
router handlers are ported.
Wires POST /eth/v1/validator/duties/attester/{epoch}: dual-format
(numeric or string-encoded) validator index body, upstream call,
pubshare swap.
Wires POST /eth/v1/validator/duties/sync/{epoch}, reusing the
ValIndexes dual-format body extractor.
Wires GET /eth/v1/validator/attestation_data. The Component now holds an Arc<MemDB> and awaits unsigned attestation data from the local DutyDB rather than hitting upstream.
Bug fixes (must-fix per review):
- attestation_data: wrap MemDB::await_attestation in tokio::time::timeout
(24s) so a request for a slot that never produces consensus output
cannot hold a handler task indefinitely. delete_duty now records
evicted keys per duty type and notifies waiters, so await_data returns
Error::AwaitDutyExpired immediately when the awaited duty is gone
instead of spinning until the timeout fires. Maps to 408 on the wire.
- Stop leaking upstream BlindedBlock400Response Debug output (incl.
stacktraces) into the client-visible ApiError.message. The variant
payload is now attached as `source` for debug logs; the message stays
generic.
Hardening:
- new_insecure is gated behind #[cfg(test)] so the insecure_test flag
cannot reach production builds.
- new_router applies DefaultBodyLimit::max(64 KiB) on the two
POST /duties/{attester,sync}/{epoch} routes — defends against the
Vec<u64> parse amplification on the ValIndexes deserializer.
- All upstream eth2_cl calls are wrapped in tokio::time::timeout(12s)
so a hanging beacon node cannot stall handler tasks.
- proposer_duties / attester_duties / sync_committee_duties propagate
upstream BadRequest as 400 and ServiceUnavailable as 503 instead of
collapsing every non-Ok variant to 502 — the VC can now back off on
upstream syncing instead of treating it as a gateway failure.
- swap_attester_pubshares / swap_sync_committee_pubshares now return
500 (cluster misconfig) instead of 502 when a pubshare is missing —
the upstream returned well-formed data, the failure is local.
ValIndexes:
- Replace #[serde(untagged)] with a streaming Visitor that validates
each element via SeqAccess::next_element. Avoids the speculative
Vec<u64> parse and the serde Content cache. Now accepts mixed
numeric/string elements and rejects negative integers.
- Hard cap at 8192 indices per request.
ApiError:
- with_boxed_source for sources that aren't std::error::Error (e.g.
anyhow::Error from auto-gen request builders).
Router:
- attestation_data uses Result<Query<...>, QueryRejection> so 4xx
responses from missing/malformed query params share the same
{ code, message } envelope as the rest of the router.
Tests (+13):
- attestation_data: timeout when data never arrives; 408 when duty is
evicted while a waiter is parked; cancellation cleanup when the
handler future is dropped; negative lookup on wrong committee_index.
- Status-mapping helpers: confirm upstream Debug output is never
serialized into the message.
- Router: ApiError envelope on bad query; oversized body rejection;
ValIndexes empty/mixed/oversized/negative cases.
Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
Ports `Validators` from `core/validatorapi/validatorapi.go` (lines 1218–1296)
together with the `convertValidators` helper (lines 1305–1332). The handler
translates VC-supplied pubshares back to the cluster's root pubkeys before
calling the upstream `POST /eth/v1/beacon/states/{state_id}/validators`
endpoint, then rewrites each returned validator's inner pubkey from the
root key to this node's public share so the downstream VC sees the share
it is configured to sign with.
`ignoreNotFound` follows the Go semantics: when the request filtered by
indices, an upstream validator that is not part of this cluster surfaces
as 500 (`pubshare not found`); otherwise the entry passes through with
its root pubkey unchanged. Upstream timeouts surface as 504, transport
failures as 502, and a malformed pubkey from the upstream as 502.
Upstream 400 / 404 propagate faithfully without leaking the upstream
body into the client-visible message.
`Validator` is aliased to the auto-generated
`GetStateValidatorsResponseResponseDatum`, matching the established
pattern for the other duty types in `validatorapi/types.rs`.
Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
Resolve conflicts against squash-merged main: - types.rs / component.rs: keep the new `validators` handler, `convert_validators`, and `invert_pub_share_map` (PR's unique work), adapting the upstream request to main's struct-literal `PostStateValidatorsRequest`/`Component::new` (now takes a `validator_cache`); alias `Validator` to the eth2api datum. - router.rs / dutydb/memory.rs: take main's evolved versions (JSON rejection + content-type middleware; eviction high-water mark). Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
…lers Implement the axum router handlers that were todo!() for the already-merged component logic, plus the two infra pieces, so the validator API surface is reachable end to end: - proxy_handler fallback: reverse-proxy to the beacon node with basic-auth, Host rewrite, hop-by-hop header stripping, and a proxy-latency metric. - submit_proposal_preparations: no-op swallow (200). - propose_block_v3: versioned block response with the consensus version / payload-value headers; builder_enabled maxes the boost. - submit_proposal / submit_blinded_block: per-fork (de)serialization keyed by the Eth-Consensus-Version header (JSON or SSZ body). - get_validators / get_validator: id batch dispatched on the first element's 0x prefix, matching Charon's getValidatorsByID. new_router gains an upstream_base_url argument and AppState carries a reqwest client for the proxy. Adds public per-fork SSZ block-body decoders in ssz_codec and extends the TestHandler stubs. Go reference: charon core/validatorapi/router.go (v1.7.1). Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
- proxy: stream the upstream response body instead of buffering, so the long-lived SSE /eth/v1/events stream proxies incrementally (reqwest bytes_stream + axum Body::from_stream; enable reqwest "stream"). - proxy: strip the client Authorization header when the upstream URL carries credentials, avoiding a duplicate/conflicting Authorization. - propose_block_v3: always send builder_boost_factor (0 when builder mode is off, u64::MAX when on), matching Charon. - request_is_ssz: reject a non-ASCII Content-Type with 415 instead of silently treating it as JSON. - ssz_codec: drop the dead `blinded` parameter from decode_signed_proposal_block_body (full-block decoder only). - move the use blocks above the const declarations in router.rs. - add an SSZ-body submit test; drop Go line-number anchors from doc comments. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
Port the attestation/aggregation domain of the validator API from Charon v1.7.1 (core/validatorapi). Fills the four previously-unimplemented Component methods and wires their axum router handlers: - POST /eth/v2/beacon/pool/attestations -> submit_attestations - GET /eth/v2/validator/aggregate_attestation -> aggregate_attestation - POST /eth/v2/validator/aggregate_and_proofs -> submit_aggregate_attestations - POST /eth/v1/validator/beacon_committee_selections -> beacon_committee_selections The v1 attestation/aggregate routes stay 404 (Go parity). Component: - submit_attestations: resolves the validator index (explicit for Electra/Fulu, matched from the attester-duty set + single aggregation bit for pre-Electra), looks up the DV root pubkey via pub_key_by_att, verifies the partial attester signature (DOMAIN_BEACON_ATTESTER, epoch from the attestation target), and broadcasts under an attester duty. - aggregate_attestation: blocking await via the agg-attestation hook. - submit_aggregate_attestations: looks up the aggregator pubkey from the validator cache, verifies the inner selection proof (skipped under insecure_test) and the outer partial sig (DOMAIN_AGGREGATE_AND_PROOF), broadcasts under an aggregator duty. - beacon_committee_selections: verifies each slot signature (DOMAIN_SELECTION_PROOF), broadcasts under a prepare-aggregator duty, then awaits the aggregated selections from the AggSigDB. Router decodes versioned JSON/SSZ arrays per Eth-Consensus-Version, lifts Electra/Fulu SingleAttestation into the versioned wrapper, and returns the versioned aggregate / selection response shapes. Shared surface: adds electra::SingleAttestation, derives Serialize/ Deserialize on signeddata::AttesterDuty, and replaces the three attestation placeholder types in validatorapi::types with signeddata aliases. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
- submit_attestations: match Go's no-duty-match behaviour — leave the validator index at 0 and let the pubkey lookup fail, instead of returning an early error; move the single-aggregation-bit check inside the committee-matching loop, mirroring validatorapi.go. - beacon_committee_selections: resolve the AggSigDB hook up front so a misconfigured component fails before broadcasting partial selections. - router: rename the non-snake-case `AttestationPayload_phase0` helper to `phase0_attestation_payload`. - tests: add Electra/SSZ aggregate-and-proof decode, out-of-range committee-index rejection, and a multi-slot beacon-committee-selection broadcast test. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
Collaborator
Author
/loop-review-pr summaryRan 1 review-and-fix iteration against this PR (4 parallel review agents: functional-parity vs Charon v1.7.1, security, rust-style, code-quality). Terminated by: completion (clean after fixes). Quality gates (final)
Resolved during the loopBugs (0) Major (2)
Minor (2)
Nits (0 changed) Outstanding (accepted — parity-equivalent with Charon v1.7.1)
VerdictPR is ideal — all bug/major findings resolved, gates green. |
Resolve conflicts in validatorapi (component, router, testutils, types): combine the PR's proxy + proposal/validators handlers and tests with main's beacon/sync committee selections handlers and tests. Adapt main's router tests to the PR's new 3-arg new_router / test_state helpers and keep the PR's upstream-request timeout on the validators call. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
submit_proposal / submit_blinded_block take `body: Bytes` and inherited axum 0.8's 2 MiB DefaultBodyLimit. A blob-carrying Electra/Fulu block exceeds that: up to 12 blobs of 128 KiB each are `0x`-hex in the JSON encoding (~2x binary), ~3 MiB of blobs alone before the block body and kzg fields — so high-blob proposals would be rejected with 413 and the slot missed. Charon reads the body uncapped (router.go submitProposal, io.ReadAll). Add a `sized_post` route helper (body limit without the JSON content-type enforcement, since these routes also accept SSZ) and a PROPOSAL_BODY_LIMIT of 16 MiB — several times a realistic max-blob block while still bounding per-request memory. Covered by two new router tests. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
…atorapi-pr2-attestation-aggregation Re-sync PR2 onto its actual stack base (PR1, which already contains main) rather than merging main directly, so the PR diff stays scoped to PR2's attestation/aggregation changes. Conflict resolutions match the prior main-merge (validatorapi types/router/ component/testutils unions + the duty-definition model reconciliation onto main's #466 non-generic DutyDefinitionSet); additionally preserves PR1's 16 MiB PROPOSAL_BODY_LIMIT / sized_post block-submission change. fmt, clippy -D warnings, and the full pluto-core test suite (502) pass. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
varex83agent
force-pushed
the
bohdan/validatorapi-pr2-attestation-aggregation
branch
from
ef267c7 to
2b8093c
Compare
PR1 landed in main (squash #488), so the base retargeted to main. PR2 already contains all of PR1's content (it had merged the PR1 branch), so this merge is content-neutral: the resulting tree is identical to the pre-merge PR2 tree — conflicts in router.rs/testutils.rs resolved to ours since main re-introduces only PR1 content PR2 already has. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
emlautarom1
approved these changes
Jun 19, 2026
emlautarom1
left a comment
emlautarom1
left a comment
Collaborator
There was a problem hiding this comment.
LGTM.
Claude keeps gaslighting me regarding this insecure_test flag. Could you double check that the behavior is the same as in Charon?
Comment on lines
+261
to
+263
| /// bounded by [`UPSTREAM_REQUEST_TIMEOUT`]. Mirrors Go's | ||
| /// `c.eth2Cl.ActiveValidators(ctx)`, which is itself implemented via the | ||
| /// beacon-node validator cache. |
Collaborator
There was a problem hiding this comment.
nit: these comments add little not no value (AI artifacts)
Comment on lines
+2387
to
+2390
| /// Builds a core [`PubKey`] from a 48-byte BLS public key. | ||
| fn pubkey_from_bls(pubkey: &BLSPubKey) -> PubKey { | ||
| PubKey::new(*pubkey) | ||
| } |
| ApiError::new(StatusCode::BAD_GATEWAY, "could not resolve epoch from slot") | ||
| .with_source(err) | ||
| })?; | ||
| verify_par_signed_aggregate(self, &pubkey, epoch, &par_sig_data).await?; |
Collaborator
There was a problem hiding this comment.
In Charon, this check is gated behind self.insecure_test while here it's executed unconditionally.
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
3 participants
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.
Summary
Ports the attestation & aggregation domain of the validator API from Charon v1.7.1 (
core/validatorapi/{router.go,validatorapi.go,eth2types.go}). Fills the four previouslyunimplemented!()Componentmethods and wires their axum router handlers (previouslytodo!()).Stacks on #488 (PR 1 — proxy + proposal/validators wiring), which laid the producer-hook scaffolding (
pub_key_by_att_fn,await_agg_attestation_fn,await_agg_sig_db_fn,duty_def_fn) and theverify_partial_sighelper this PR consumes.Scope
Endpoints wired (v1 attestation/aggregate routes intentionally stay
404, matching Go):POST /eth/v2/beacon/pool/attestationssubmit_attestationsGET /eth/v2/validator/aggregate_attestationaggregate_attestationPOST /eth/v2/validator/aggregate_and_proofssubmit_aggregate_attestationsPOST /eth/v1/validator/beacon_committee_selectionsbeacon_committee_selectionsGo reference (v1.7.1,
core/validatorapi/):SubmitAttestations,AggregateAttestation,SubmitAggregateAttestations,BeaconCommitteeSelections, and the matching router handlers +createAggregateAttestation.Behaviour (Go parity)
pub_key_by_att, verifies the partial attester signature (DOMAIN_BEACON_ATTESTER, epoch from the attestation's target checkpoint), and broadcasts under an attester duty.Eth-Consensus-Versionheader and{ "version", "data" }body.insecure_test, as in Go) and the outer partial signature (DOMAIN_AGGREGATE_AND_PROOF), then broadcasts under an aggregator duty.DOMAIN_SELECTION_PROOF), broadcasts under a prepare-aggregator duty, then awaits the aggregated selections from the AggSigDB; returns{ "data": [...] }.Router decodes versioned JSON and SSZ arrays keyed off
Eth-Consensus-Version, lifts the Electra/FuluSingleAttestationwire form into the versioned wrapper (committee index → committee bitfield, attester index → validator index), and reproduces Go's response shapes.Shared surface (relevant to PR 3 / PR 4)
crates/eth2api/src/spec/electra.rs: addsSingleAttestation(SSZ + JSON, string-encoded indices).crates/core/src/signeddata.rs: derivesSerialize/DeserializeonAttesterDuty(so it can key aDutyDefinitionSet).crates/core/src/validatorapi/types.rs: replaces theVersionedAttestation/VersionedSignedAggregateAndProof/BeaconCommitteeSelectionplaceholder structs with aliases to the signeddata / eth2api wrappers.crates/core/src/validatorapi/testutils.rs: append-onlyTestHandlerrecording fields + setters for the four endpoints.Tests
router.rs): happy path + JSON/SSZ negotiation + missing-version-header + unsupported-content-type + malformed-body + v1-route-404 for each endpoint; versioned aggregate response headers/body; selections round-trip.component.rs): pre-Electra index resolution + broadcast, Electra explicit-index path, multi-aggregation-bit rejection, aggregate await + missing-hook 503, aggregate submit resolve+broadcast + unknown-validator rejection, selections broadcast + AggSigDB round-trip + unknown-validator rejection.Quality gates (all green)
cargo +nightly fmt --all --checkcargo clippy --workspace --all-targetsand--all-featurescargo test --workspace(pluto-core 426, eth2api 91, eth2util 144 — all pass)cargo build --workspace --all-featurescargo deny checkcargo test --all-featuresis Docker-gated in the sandbox (eth2apiintegrationfeature spins a Lighthouse testcontainer); the functional gate iscargo test --workspace(default features), and--all-featuresis confirmed to compile and be clippy-clean.🤖 Generated with Claude Code