Repository navigation
feat: add GET /eth/v2/debug/fork_choice and keep fork choice store extra data - #60
Conversation
…tra data Adds ForkChoiceV2Provider and apiv1.ForkChoiceV2 for the Gloas-aware fork choice endpoint proposed in ethereum/beacon-APIs#615, with one node per (block root, payload status) pair. The endpoint is not final and client implementations differ, so decoding is lenient: the response is accepted with or without the data wrapper, fields not every client provides yet (parent payload status, checkpoint epochs, PTC counts) are optional, and non-spec node fields are folded into ExtraData instead of being dropped. Tests use responses captured from Teku, Prysm and Lodestar on Sepolia. Also keeps the top-level extra_data of GET /eth/v1/debug/fork_choice, which carries fork choice store values such as the unrealized justified checkpoint and proposer boost root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Matches the repo's enum convention (attgo_enum_iota), following ForkChoiceNodeValidity: unknown as the zero value, then empty, full and pending, with a string table for (un)marshalling. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![redpandabot[bot]](https://avatars.githubusercontent.com/in/3749543?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adds a Gloas-aware GET /eth/v2/debug/fork_choice (ForkChoiceV2Provider) across http/multi/mock/testclients, with lenient decoding that accepts wrapped or unwrapped responses and folds client-specific fields into ExtraData, plus preserves the store-level extra_data on v1 ForkChoice. I verified the new types against the beacon-APIs spec at the referenced commit, traced the wrapper detection and unknown-field folding, and checked the unsupported-client api.Error path; the change is internally consistent and I found no real defects.
Reviewed 19 changed file(s) @ 140ad87c — no blocking issues found.
Red pandas are neither bears nor raccoons — they're the sole survivors of their own family, Ailuridae.
- Decode only unknown fields' values when folding them into ExtraData,
rather than the whole object (fork_choice_nodes included) a second
time, and decode a wrapped v2 response's data directly instead of
re-reading the body.
- Decode unrecognised node validities as unknown instead of failing the
whole fork choice: since Gloas, Lighthouse reports not_yet_revealed
for a block whose payload has not been revealed, which broke the v1
endpoint whenever such a block was in the tree. "unknown" now parses
for validities and payload statuses, so values round-trip.
- Parse payload statuses case-insensitively, like validities.
- Marshal a nil fork_choice_nodes as [] so a zero ForkChoiceV2 round
trips, and reject {"data": null} instead of returning nil data.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A node validity the spec does not define still decodes as unknown, but the original value is now kept under ExtraData["validity"] (unless the client already uses that key), so callers can tell Lighthouse's not_yet_revealed apart from garbage, and re-encoding keeps it. ForkChoiceNodeValidity's own decoder stays strict, since a lone validity has nowhere to keep the original. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Summary
The PR adds the Gloas-aware GET /eth/v2/debug/fork_choice across http/multi/mock/test clients with deliberately lenient cross-client decoding (wrapped or unwrapped, folded unknown fields, unrecognised validities/parent payload statuses preserved in ExtraData), keeps v1's top-level store extra_data, and the latest commit makes mixed multi pools surface the unsupported-endpoint error so callers can fall back to v1. The multi error handling, HTTP metadata/extra-data split, and deterministic field decoding all trace correctly. One open item from the earlier review is still present: v1 silently accepts a node with no validity.
Issues
- 🟡
api/v1/forkchoice.go:282— v1 silently accepts a fork choice node with no validity — see the thread on that line
Reviewed @ 2c54670f
Red pandas are neither bears nor raccoons — they're the sole survivors of their own family, Ailuridae.
- multi: skip clients without the v2 endpoint (400/404/405/501) without deactivating them, returning their error if none support it. - http: keep response metadata of unwrapped responses in Metadata rather than the fork choice's ExtraData, as for wrapped ones. - An unrecognised parent_payload_status is nil, with the original kept in ExtraData, rather than failing the whole response. - Decode each fork choice object once, folding unknown fields from the same parse. - Add JSON methods to ForkChoicePayloadStatus and document that a zero parent root means none. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- multi: when any client lacked the v2 endpoint and no client returned a fork choice, return the unsupported error (joined with the others'), so callers fall back to v1, which those clients can serve. - Take node fields in a fixed order, so the reported error is deterministic, without building a map per node. - Document and test that field names match exactly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- multi: join the unsupported error with the last other client error seen in the callback, rather than doCall's last error, which is "empty response" when the last client lacked the endpoint. - Clear a v2 node's parent root and parent payload status, and a v1 node's parent root, before decoding into an existing node. - v1: a missing validity is still an error, as before. - Explain why 400 counts as an unsupported endpoint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Adds a Gloas-aware GET /eth/v2/debug/fork_choice (ForkChoiceV2Provider) with lenient decoding across http/multi/mock/test clients, retains the v1 fork-choice store extra_data, and hardens the multi pool's unsupported-endpoint failover. The earlier open finding (v1 accepting a node with no validity) is fixed by the explicit validity-missing check, and the newest commit's error-tracking and reused-node clearing look correct; I found no new issues worth flagging.
Reviewed 20 changed file(s) @ 0b35f098 — no blocking issues found.
Red pandas are neither bears nor raccoons — they're the sole survivors of their own family, Ailuridae.
There was a problem hiding this comment.
Summary
The PR adds the /eth/v2/debug/fork_choice provider (api/http/mock/multi/testclients) with deliberately lenient decoding, and makes v1 keep the fork choice store's extra_data; the design and error handling look sound, and the previously open finding (v1 accepting a node with no validity) is now fixed by the validity missing check at api/v1/forkchoice.go:288. However, the final merge of master brought in ForkChoiceNodeValidityNotYetRevealed, which the v2 tests do not expect, so those tests now fail.
Issues
- 🔴
api/v1/forkchoicev2_test.go:191— v2 tests assert the pre-merge not_yet_revealed behaviour and now fail — see the thread on that line - 🟢
api/v1/forkchoice.go:285— Comment still cites not_yet_revealed as an unrecognised validity — The comment says Lighthouse'snot_yet_revealeddecodes asunknownand is kept inExtraData, but after the merge it is a recognised value (line 141); another unrecognised string would be a correct example. The same stale claim is repeated at api/v1/forkchoicev2.go:390.
Reviewed @ fbfb3b70
Red pandas are neither bears nor raccoons — they're the sole survivors of their own family, Ailuridae.
This comment has been minimized.
This comment has been minimized.
Align GET /eth/v2/debug/fork_choice with ethereum/beacon-APIs#615 as merged: - Nodes carry justified_checkpoint and finalized_checkpoint (Checkpoint) instead of optional justified_epoch / finalized_epoch. - Every field the spec requires must be present: parent_root, parent_payload_status (nullable), the PTC counts and extra_data on each node; extra_data and at least one node on the store. - payload_status and parent_payload_status accept only pending, empty and full, and validity only valid, invalid and optimistic. - The response must be wrapped in data. - ExtraData is the response's extra_data only; unknown fields are no longer folded into it. No client implements the merged shape yet. The captured Teku, Prysm and Lodestar responses are kept as tests documenting where each falls short, and a spec-shaped fixture covers decoding. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Adds ForkChoiceV2Provider / apiv1.ForkChoiceV2 for GET /eth/v2/debug/fork_choice and preserves v1 fork choice store extra_data. Both earlier open findings are resolved: v1 now rejects a node with no validity, and the v2 tests were updated to the merged beacon-APIs#615 shape, which the implementation follows strictly (data wrapper, nullable parent_payload_status, per-node checkpoints, PTC counts, extra_data, restricted enums). No remaining defects found; note the PR description's lenient-decoding section is now out of date.
Reviewed 21 changed file(s) @ 9de0cf34 — no blocking issues found.
Red pandas are neither bears nor raccoons — they're the sole survivors of their own family, Ailuridae.
Bump go-eth2-client to ethpandaops/go-eth2-client#60 as aligned with ethereum/beacon-APIs#615 as merged: - Nodes carry justified and finalized checkpoints. They are kept as the new ForkChoiceNodeV2 justified_checkpoint / finalized_checkpoint fields and, as before, as justified_epoch / finalized_epoch, and are round-tripped into extra_data for consumers of the v1 shape. - Weight and the PTC counts are always set. go-eth2-client now rejects v2 responses that do not follow the spec, which is every client today. Any v2 failure, not only an unsupported endpoint, now falls back to v1 for forkChoiceV2RetryInterval, so such nodes keep producing fork choice data without a warning on every fetch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… pool Review follow-ups: - The http client now returns ErrInvalidResponse (new) for a v2 response that does not follow the spec. The multi client skips such clients, as it does those without the endpoint, instead of deactivating them for every endpoint. Today that is Teku, Prysm and Lodestar. - v2 validity is matched exactly (valid, invalid, optimistic), like payload status; "VALID" or "Optimistic" are rejected. - v1: a parent root that is neither null nor 32 bytes is an error, and the comments no longer cite not_yet_revealed, which master already decodes. - multi tests cover 405, 501 and invalid responses, and check that skipped clients are tried again rather than only the first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 participants
Adds the Gloas-aware fork choice endpoint from ethereum/beacon-APIs#615 (merged 2026-10-07), and keeps the fork choice store's
extra_dataon v1.Changes
ForkChoiceV2Provider/apiv1.ForkChoiceV2:GET /eth/v2/debug/fork_choice, one node per(block_root, payload_status)pair, typed to #615 as merged. Each node has payload status, parent payload status, per-node justified and finalized checkpoints, weight, validity, execution block hash, PTC counts and extra data. Required values are plain fields (JustifiedCheckpoint/FinalizedCheckpoint phase0.Checkpoint,Weight phase0.Gwei, PTC countsuint64); onlyParentPayloadStatusis a pointer, nil when the parent is not retained. Implemented forhttp,multi,mockand the test clients.apiv1.ForkChoice(v1) keeps the response's top-levelextra_data(store values such as the unrealized justified checkpoint), which was previously dropped.v2 follows the spec
Decoding follows the merged spec rather than what clients currently return:
data.parent_root,parent_payload_status(nullable, for a parent not retained in the tree),justified_checkpoint/finalized_checkpoint(Checkpoint objects), the three PTC counts andextra_dataon every node; checkpoints,extra_dataand at least one node on the store.payload_status/parent_payload_statusaccept onlypending,empty,full, andvalidityonlyvalid,invalid,optimistic, matched exactly (case-sensitive).ExtraDatais exactly the response'sextra_data. Unknown fields are ignored, not folded into it, so encoding round-trips.No client implements the merged shape yet (checked on Sepolia after the Gloas fork), so none decode today:
datawrapperparent_payload_statusjustified_root,unrealised_*instead)extra_data, pending node onlyextra_dataUnsupported endpoint version: v2The captured Teku, Prysm and Lodestar responses are kept as tests asserting the first spec violation each hits, so they flip as clients catch up.
Clients without the endpoint surface as an
*api.Errorwith their status code, and responses that don't follow the spec as the newclient.ErrInvalidResponse, so callers can tell both apart from a failing node and fall back to v1. Themulticlient skips both (400/404/405/501, and invalid responses) without deactivating them, since they still serve every other endpoint, and tries the next client. If no client returns a fork choice, it returns the unsupported error (joined with the other clients' errors, if any), or the invalid-response error. Response fields besidedataland inResponse.Metadata.ForkChoicePayloadStatusis auint64enum (unknown= 0 as the zero value,empty,full,pending) with JSON methods, followingForkChoiceNodeValidity. Only the spec's three values decode.v1
extra_datais kept (previously dropped),omitemptyso output is unchanged for clients that don't send it.unknowninstead of failing the whole response, with the original kept under the node'sExtraData["validity"](unless the client already uses that key), so it survives re-encoding."unknown"itself now parses. Lighthouse'snot_yet_revealedwas already handled on master (Accept not_yet_revealed fork choice validity #61) and still decodes asForkChoiceNodeValidityNotYetRevealed.parent_rootdecodes as the zero root (also when reusing a node); a parent root that is neither null nor 32 bytes is now an error.Behaviour change on v1: an unrecognised validity used to fail decoding; it is now accepted as above. A missing validity is still an error.
Tests
unknown,not_yet_revealed, wrong case), and node reuse. Plus the v1 storeextra_dataround trip.http/apitesttests: a spec-shaped response (pending/empty/full nodes, parent links, checkpoints, PTC counts), the captured client responses failing as above, the Lighthouse 400 and Nimbus 404, and response metadata.multitests: clients answering 400/404/405/501 or with an invalid response are skipped and tried again on the next call (not deactivated), plus the error returned when none succeed.go test ./...on Go 1.25;golangci-lintv2.8.0 with theattgoplugin: 0 issues.Used by ethpandaops/xatu (sentry fork choice) and ethpandaops/forky.
🤖 Generated with Claude Code