Skip to content

feat(core): implement validatorapi sync committee handlers#490

Merged
varex83 merged 18 commits into
mainfrom
bohdan/validatorapi-pr3-sync-committee
Jun 24, 2026
Merged

feat(core): implement validatorapi sync committee handlers#490
varex83 merged 18 commits into
mainfrom
bohdan/validatorapi-pr3-sync-committee

Conversation

@varex83agent

Copy link
Copy Markdown
Collaborator

Summary

Wires the four sync-committee validator-API endpoints end to end — component logic + axum router handlers — porting Charon v1.7.1 core/validatorapi. Nothing lands as dead code: each endpoint is reachable from the router and exercised by tests.

This is PR 3 of the validatorapi port (sync committee domain). It stacks on PR 1 (#488, proxy + proposal/validators wiring) and builds on the shared surface PR 1 introduced (new_router(handler, builder_enabled, upstream_base_url), AppState, TestHandler setters).

Scope

Endpoint Route Handler
submit sync messages POST /eth/v1/beacon/pool/sync_committees submit_sync_committee_messages
sync contribution GET /eth/v1/validator/sync_committee_contribution sync_committee_contribution
submit contribution+proofs POST /eth/v1/validator/contribution_and_proofs submit_sync_committee_contributions
sync selections POST /eth/v1/validator/sync_committee_selections sync_committee_selections

The previously unimplemented!() Component methods and todo!() router handlers are now implemented. The placeholder validatorapi sync types are replaced with the real altair / v1 spec types.

Behavior (Go parity, v1.7.1)

  • submit_sync_committee_messages (component.go:958): resolve each validator_index against the active-validators cache ("validator not found" on miss), build a partial SignedSyncMessage, verify against this node's share (domain DOMAIN_SYNC_COMMITTEE, message root = beacon_block_root), group by slot, broadcast SyncMessage duties.
  • submit_sync_committee_contributions (component.go:1009): verify the inner selection proof against the aggregator's full validator pubkey (domain DOMAIN_SYNC_COMMITTEE_SELECTION_PROOF, root = SyncAggregatorSelectionData), mirroring Go's VerifyEth2SignedData; then verify the outer partial signature against this node's share (domain DOMAIN_CONTRIBUTION_AND_PROOF, root = Message.HashTreeRoot); group by slot, broadcast SyncContribution duties.
  • sync_committee_selections (component.go:1072): verify each partial SyncCommitteeSelection against the share (domain DOMAIN_SYNC_COMMITTEE_SELECTION_PROOF), broadcast PrepareSyncContribution duties, then await the aggregated selection per (duty, pubkey) from the aggsigdb ("invalid sync committee selection" if the stored type is wrong) and return them.
  • sync_committee_contribution (component.go:948): forward (slot, subcommittee_index, beacon_block_root) to the registered await_sync_contribution hook and wrap the result.
  • Signature verification is skipped under insecure_test on both the share and full-pubkey paths, matching Go's c.insecureTest guard.
  • All four endpoints are JSON-only, matching Charon's declared Encodings: [JSON]. Responses use the { "data": ... } envelope (wrapResponse).

Go references (v1.7.1)

  • core/validatorapi/validatorapi.go: SubmitSyncCommitteeMessages, SyncCommitteeContribution, SubmitSyncCommitteeContributions, SyncCommitteeSelections, verifyPartialSig.
  • core/eth2signeddata.go: VerifyEth2SignedData + the SignedSyncMessage / SignedSyncContributionAndProof / SyncCommitteeSelection domain & epoch impls.
  • core/signeddata.go: the NewPartialSignedSync* constructors.
  • core/validatorapi/router.go: submitSyncCommitteeMessages, syncCommitteeContribution, submitContributionAndProofs, syncCommitteeSelections.

Test coverage

  • Component: per-slot grouping + subscriber fan-out for messages and contributions; validator not found rejection; selection broadcast + aggsigdb collection; wrong-aggregated-type rejection; unregistered-hook 503; contribution hook forwarding.
  • Router: happy-path array forwarding; JSON data envelopes for contribution (GET query parse) and selections; 415 on non-JSON content type; 400 on empty body; 400 on missing query parameter.

Gates (run from worktree)

  • cargo +nightly fmt --all --check — clean
  • cargo clippy --workspace --all-targets --all-features -- -D warnings — clean
  • cargo test --workspace — pass (--all-features test is Docker-gated in this sandbox via the eth2api integration feature; confirmed it compiles and is clippy-clean)
  • cargo deny check — ok

🤖 Generated with Claude Code

varex83 and others added 15 commits May 28, 2026 14:08
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>
Wire the four sync-committee validator-API endpoints end to end
(component logic + axum router handlers), porting Charon v1.7.1
core/validatorapi.

- submit_sync_committee_messages: POST /eth/v1/beacon/pool/sync_committees
- sync_committee_contribution: GET /eth/v1/validator/sync_committee_contribution
- submit_sync_committee_contributions: POST /eth/v1/validator/contribution_and_proofs
- sync_committee_selections: POST /eth/v1/validator/sync_committee_selections

Submit handlers resolve the validator index against the active-validators
cache ("validator not found" on a miss), verify partial signatures against
this node's share (sync-committee message / contribution-and-proof / sync
selection domains), and broadcast SyncMessage / SyncContribution /
PrepareSyncContribution duties to subscribers grouped by slot. The
contribution submitter additionally verifies the inner selection proof
against the aggregator's full validator pubkey, mirroring Go's
VerifyEth2SignedData. Selections then await the aggregated proof per
(duty, pubkey) from the aggsigdb. All four endpoints are JSON-only, matching
Charon's declared encodings.

Replaces the placeholder validatorapi sync types with the real altair / v1
spec types and extends the router TestHandler to record sync submissions.

Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
Address self-review findings on the sync-committee port:

- parse_json_array now rejects a non-UTF8 Content-Type with 415 (surfacing
  the raw header), matching the sibling request_is_ssz helper instead of
  silently defaulting to JSON.
- Drop the now-stale #[allow(dead_code)] on verify_partial_sig (it is
  consumed by the sync submit handlers in this PR) and refresh its doc.
- Add component tests: two aggregators grouped into one slot for
  contributions; subscriber failure surfaces 500 and aborts the fan-out;
  empty input arrays broadcast nothing and return an empty selections set;
  selections collects multiple (duty, pubkey) entries from the aggsigdb.

Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
@varex83agent

Copy link
Copy Markdown
Collaborator Author

/loop-review-pr summary

Ran 1 review-and-fix iteration against this PR (four parallel reviewers: functional-equivalence, security, rust-style, code-quality). Terminated by: completion_promise — no bug/major findings; only minor/nit items, the actionable ones fixed.

Quality gates (final)

  • cargo +nightly fmt --all --check — pass
  • cargo clippy --workspace --all-targets --all-features -- -D warnings — pass
  • cargo test --workspace — pass (default features; --all-features is Docker-gated in CI sandbox via the eth2api integration feature, confirmed to compile + clippy-clean)
  • cargo deny check — pass

Resolved during the loop

Bugs (0) — none.

Major (0) — none.

Minor (3)

  • parse_json_array collapsed a non-UTF8 Content-Type to "" and returned a 415 that dropped the offending value — now mirrors the sibling request_is_ssz (415 surfacing the raw header). crates/core/src/validatorapi/router.rse610d16
  • Test gaps: added component tests for two aggregators grouped into one slot (contributions), subscriber-failure 500 + fan-out abort, empty-input arrays broadcasting nothing, and selections collecting multiple (duty, pubkey) entries from the aggsigdb. — e610d16
  • (rust-style) newly-added Go-cross-reference doc comments: deferred — this comment style is the pervasive established convention across these files (PR 1 merged with it); changing only the new ones would be inconsistent and out of scope.

Nits (3)

  • Removed the now-stale #[allow(dead_code, reason = "…later PRs")] on verify_partial_sig (consumed by this PR's submit handlers) and refreshed its doc. — e610d16
  • Empty selections serializing as [] vs Go's null; GET content-type gate; SigningError→400 classification — deferred as intentional parity / established patterns consistent with the existing proposal handlers.

Outstanding

None blocking. Deferred items above are documented design/parity choices, not defects.

Verdict

PR is ideal — zero bug/major findings, gates green. Confirmed functional equivalence with Charon v1.7.1 across all four sync-committee endpoints (BLS domains, message roots, epoch derivation, duty grouping, inner-vs-outer signature verification, error strings, JSON-only encoding, response envelopes).

🤖 Generated with Claude Code

Base automatically changed from bohdan/validatorapi-pr1-proxy-wiring to main June 19, 2026 17:47
Comment thread crates/core/src/validatorapi/router.rs Outdated
Comment on lines 653 to 655

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If configured beacon node is http://beacon/internal and VC requests /eth/v1/events, Rust sends upstream request to: http://beacon/eth/v1/events

Charon uses httputil.NewSingleHostReverseProxy(targetURL) Go’s reverse proxy joins target base path + request path, so same request becomes: http://beacon/internal/eth/v1/events

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 772282d. The proxy now joins the upstream base path with the request path (singleJoiningSlash) instead of replacing it, so http://beacon/internal + /eth/v1/eventshttp://beacon/internal/eth/v1/events. Base and request query strings are joined the same way the Go director does. Covered by proxy_preserves_upstream_base_path.

fn is_hop_by_hop_header(name: &HeaderName) -> bool {
matches!(
name.as_str(),
"connection"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing proxy-connection

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 772282d. Added proxy-connection to is_hop_by_hop_header, matching Go's httputil hopHeaders. Covered by proxy_connection_is_hop_by_hop.

Comment thread crates/core/src/validatorapi/router.rs Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 772282d. 0x-hex query params now use a single-occurrence helper that treats a duplicated value as absent (Charon's hexQuery rejects len(valueA) != 1); uintQuery still takes the first value via url.Values.Get. Covered by hex_query_rejects_duplicate_but_uint_takes_first.

varex83agent and others added 2 commits June 22, 2026 19:48
Main independently landed the sync-committee Component logic and the
selections router handler (#459, #462), so this branch's remaining
contribution is the three router handlers main left as todo!() stubs:
submit_sync_committee_messages, sync_committee_contribution, and
submit_contribution_and_proofs (plus their route tests and the
parse_json_array/uint_query helpers).

Conflict resolution:
- types.rs, component.rs: take main (complete, tested superset)
- testutils.rs: union of TestHandler fields/setters/impls
- router.rs: keep main's selections handler + body limits + tests;
  graft the PR's three handlers, helpers, and route tests
- Cargo.lock: take main

Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
Three review findings on the reverse-proxy / query handling, matching
Charon v1.7.1 behaviour:

- Proxy path: join the upstream base path with the request path
  (`singleJoiningSlash`) instead of replacing it, so an operator-configured
  prefix (e.g. http://beacon/internal) is preserved. Also join the base and
  request query strings, mirroring Go's NewSingleHostReverseProxy director.
- Strip the non-standard `Proxy-Connection` hop-by-hop header, as Go's
  httputil hopHeaders does.
- Treat a duplicated 0x-hex query parameter as absent (Charon's hexQuery
  rejects len(valueA) != 1) via a single-occurrence helper; uintQuery still
  takes the first value.

Adds router tests covering each.

Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>

@emlautarom1 emlautarom1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The router module has a lot of helpers related to deserializing parameters/urls that should probably be handled by the library. Could probably be cleaned up in a follow up PR, but for now they're OK.

Generally LGTM, make sure to merge main and resolve the conflicts.

Resolve conflicts in validatorapi router/testutils: union of PR3
sync-committee handlers and main's attestation/aggregation handlers.
Dropped a duplicate uint_query helper introduced on both branches.

Co-Authored-By: varex83 <ivan.balatsko@nethermind.io>
@varex83 varex83 merged commit 6a4f0ba into main Jun 24, 2026
12 checks passed
@varex83 varex83 deleted the bohdan/validatorapi-pr3-sync-committee branch June 24, 2026 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants