feat: implement head_v2 event#9486
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for the head_v2 Server-Sent Events (SSE) event type, defining its schema, serialization logic, and emitting it upon block import. The review feedback highlights three key improvements: handling undefined payload status for pre-Bellatrix blocks in toApiPayloadStatus to prevent serialization errors, using newHead.slot instead of blockSlot to accurately determine the fork name for the event version, and simplifying the event type mapping by utilizing the existing WithVersion helper.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 142d9c5b03
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…dStatusCache to chain
|
Only tests left to add |
|
Now ready for review |
head_v2 event
|
type check was failing, it should pass now (together with other checks) |
nflaig
left a comment
There was a problem hiding this comment.
few comments from a quick pass
| * Cache of head block payload status at time of head_v2 emission, used to detect | ||
| * empty -> full transitions and emit a second head_v2 event accordingly. | ||
| */ | ||
| readonly headV2PayloadStatusCache = new Map<RootHex, {status: PayloadStatus; slot: Slot}>(); |
There was a problem hiding this comment.
not entirely clear to me why we need a cache for this, the second head event can be emitted if your head in fork choice moves from empty to full
There was a problem hiding this comment.
well, I added the cache to prevent duplicate emissions between importBlock and importExecutionPayload, but after investigating further I see the importExecutionPayload emission doesn't work correctly since the head is never updated there.
Should I add a head update call after onExecutionPayload, or should both emission types come from importBlock?
Either way the cache can be removed. Let me know what works best.
There was a problem hiding this comment.
I think I raised something similar in the PR itself, to me emitting head twice per slot still feels strange but that's what we are supposed to do, I would need to re-read what the decision was on the PR, if it's up to me I would probably get rid of payload_status field or report the payload status of the parent, need to take more time to think about this and re-read what we discussed on the PR
| import {isOptimisticBlock} from "../../util/forkChoice.js"; | ||
| import {BeaconChain} from "../chain.js"; | ||
|
|
||
| export function emitHeadV2(this: BeaconChain, head: ProtoBlock, headChanged: boolean) { |
There was a problem hiding this comment.
I think reusing the same function in importBlock and importExecutionPayload is a bad abstraction
There was a problem hiding this comment.
i will keep this in mind and address it once I handle the emissions properly
| previousDutyDependentRoot: currentEpochDependentRoot, | ||
| currentDutyDependentRoot: nextEpochDependentRoot, |
There was a problem hiding this comment.
maybe we refactor our head event data and add the shim to v1 event, the new semantics of head v2 are much nicer
There was a problem hiding this comment.
i'm glad to make that change, but maybe it is best to do it in another PR to keep this one readable.
I did some greps and it looks like change touches a lot of places.
let me know if you agree and I will make an issue for this
Description
Implements server-side emission of the
head_v2event as specified in ethereum/beacon-APIs#590, alongside the existing head event which is kept for backwards compatibility.Closes #9472
Changes
head_v2toEventTypeenum andeventTypesmap inpackages/apiHeadV2SSZ container type andEventDatashapehead_v2from importBlock alongside headtoApiPayloadStatusmapper from fork choicePayloadStatusenum to API string valueshead_v2TODO
head_v2with fallback to headchainHeadTracker.ts)