feat: add SSZ-first Engine API transport with JSON fallback#8993
feat: add SSZ-first Engine API transport with JSON fallback#8993lodekeeper wants to merge 2 commits intoChainSafe:unstablefrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the communication protocol with the execution layer by introducing a new SSZ-first transport for the Engine API. This change aims to improve performance and standardization by prioritizing binary serialization while maintaining full compatibility with existing JSON-RPC endpoints through an intelligent fallback mechanism. The new architecture provides a robust and future-proof foundation for execution layer interactions. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces an SSZ-first transport for the Engine API, with a fallback to JSON-RPC. This is a significant feature that should improve performance by using a more efficient serialization format. The changes are well-structured, with generic SSZ dispatch logic placed in the @lodestar/api package and integrated into the beacon-node. The implementation includes capability negotiation, SSZ-first dispatch logic, and fallback mechanisms for unsupported methods or errors. The addition of comprehensive unit and e2e tests is commendable. I have a few suggestions to improve the code quality and maintainability, including removing some duplicated/unused code and improving a validation function.
| return ( | ||
| (upper.startsWith("GET ") || upper.startsWith("POST ")) && | ||
| (lower.includes(`${ENGINE_REST_PREFIX}`) || lower.endsWith("/engine/v1/capabilities")) | ||
| ); |
There was a problem hiding this comment.
The check lower.includes("/engine/") is too broad and could incorrectly identify SSZ capabilities. For example, a string like "GET /some/other/api/that/mentions/engine/in/path" would be considered a valid engine SSZ endpoint. The type EngineSszEndpoint implies that the path must start with /engine/, and this function should enforce that more strictly.
Additionally, the condition lower.endsWith("/engine/v1/capabilities") seems incorrect. engine_exchangeCapabilities is a JSON-RPC method for negotiation, not an SSZ endpoint itself.
const parts = trimmed.split(/\s+/);
if (parts.length < 2) return false;
const method = parts[0].toUpperCase();
if (method !== "GET" && method !== "POST") {
return false;
}
const path = parts.slice(1).join(" ").toLowerCase();
return path.startsWith("/engine/");There was a problem hiding this comment.
Good catch — tightened this parser to require explicit GET|POST + a path that starts with /engine/, and removed the previous overly-broad includes("/engine/") / .../capabilities special-case.
Fixed in 4173745.
| /* biome-ignore-all lint/style/useNamingConvention: Engine API method names are protocol-defined. */ | ||
| import type {EngineApiRpcParamTypes} from "./types.js"; | ||
|
|
||
| export type EngineApiMethod = keyof EngineApiRpcParamTypes; | ||
| export type EngineSszHttpMethod = "GET" | "POST"; | ||
|
|
||
| export type EngineSszRequestDescriptor = { | ||
| httpMethod: EngineSszHttpMethod; | ||
| path: string; | ||
| }; | ||
|
|
||
| const FIXED_METHOD_TO_ENDPOINT: Partial<Record<EngineApiMethod, EngineSszRequestDescriptor>> = { | ||
| engine_newPayloadV1: {httpMethod: "POST", path: "/engine/v1/payloads"}, | ||
| engine_newPayloadV2: {httpMethod: "POST", path: "/engine/v2/payloads"}, | ||
| engine_newPayloadV3: {httpMethod: "POST", path: "/engine/v3/payloads"}, | ||
| engine_newPayloadV4: {httpMethod: "POST", path: "/engine/v4/payloads"}, | ||
|
|
||
| engine_forkchoiceUpdatedV1: {httpMethod: "POST", path: "/engine/v1/forkchoice"}, | ||
| engine_forkchoiceUpdatedV2: {httpMethod: "POST", path: "/engine/v2/forkchoice"}, | ||
| engine_forkchoiceUpdatedV3: {httpMethod: "POST", path: "/engine/v3/forkchoice"}, | ||
|
|
||
| engine_getPayloadBodiesByHashV1: {httpMethod: "POST", path: "/engine/v1/payloads/bodies/by-hash"}, | ||
| engine_getPayloadBodiesByRangeV1: {httpMethod: "POST", path: "/engine/v1/payloads/bodies/by-range"}, | ||
|
|
||
| engine_getClientVersionV1: {httpMethod: "POST", path: "/engine/v1/client/version"}, | ||
|
|
||
| engine_getBlobsV1: {httpMethod: "POST", path: "/engine/v1/blobs"}, | ||
| engine_getBlobsV2: {httpMethod: "POST", path: "/engine/v2/blobs"}, | ||
| }; | ||
|
|
||
| /** | ||
| * Engine API Binary SSZ transport endpoint mapping. | ||
| * | ||
| * Reference: execution-apis PR #764 (src/engine/ssz-encoding.md) | ||
| * | ||
| * Note: For methods without a mapping, the caller should fallback to existing | ||
| * JSON-RPC transport. | ||
| */ | ||
| export function getEngineSszRequestDescriptor( | ||
| method: EngineApiMethod, | ||
| params: EngineApiRpcParamTypes[EngineApiMethod] | ||
| ): EngineSszRequestDescriptor | null { | ||
| switch (method) { | ||
| case "engine_getPayloadV1": | ||
| return {httpMethod: "GET", path: `/engine/v1/payloads/${normalizePayloadId(params[0] as string)}`}; | ||
| case "engine_getPayloadV2": | ||
| return {httpMethod: "GET", path: `/engine/v2/payloads/${normalizePayloadId(params[0] as string)}`}; | ||
| case "engine_getPayloadV3": | ||
| return {httpMethod: "GET", path: `/engine/v3/payloads/${normalizePayloadId(params[0] as string)}`}; | ||
| case "engine_getPayloadV4": | ||
| return {httpMethod: "GET", path: `/engine/v4/payloads/${normalizePayloadId(params[0] as string)}`}; | ||
| case "engine_getPayloadV5": | ||
| return {httpMethod: "GET", path: `/engine/v5/payloads/${normalizePayloadId(params[0] as string)}`}; | ||
| default: | ||
| return FIXED_METHOD_TO_ENDPOINT[method] ?? null; | ||
| } | ||
| } | ||
|
|
||
| function normalizePayloadId(payloadId: string): string { | ||
| if (typeof payloadId !== "string" || !payloadId.startsWith("0x")) { | ||
| throw Error(`Invalid payloadId format: ${String(payloadId)}`); | ||
| } | ||
| return payloadId.toLowerCase(); | ||
| } |
There was a problem hiding this comment.
This file and its logic appear to be a duplicate of what's implemented in packages/api/src/utils/client/engineSszMethodMap.ts. The main execution logic in packages/beacon-node/src/execution/engine/http.ts uses buildEngineDispatchPlan from the @lodestar/api package, which in turn uses engineSszMethodMap.ts. This file and its corresponding test file (packages/beacon-node/test/unit/execution/engine/sszEndpointMap.test.ts) seem to be unused. To avoid code duplication and reduce future maintenance overhead, please consider removing them.
There was a problem hiding this comment.
Agreed. This was duplicate mapping logic relative to the API-layer dispatch path. I removed sszEndpointMap.ts and its unit test to avoid drift.
Removed in 4173745.
| const zeroRootHex = bytesToData(new Uint8Array(32)); | ||
| const zeroPayloadIdHex = bytesToData(new Uint8Array(8)); | ||
|
|
||
| function serializePayloadStatusFromSsz(value: { |
There was a problem hiding this comment.
The function name serializePayloadStatusFromSsz is a bit misleading. It takes an object that is the result of SSZ deserialization and transforms it into an application-level type. The term "serialize" usually implies converting to a wire format. A more descriptive name like fromSszPayloadStatus or parseSszPayloadStatus would better reflect that it's converting from an SSZ-derived structure to the application's domain model, improving code clarity.
There was a problem hiding this comment.
Agree — renamed for clarity to a parse/from-wire direction name. serializePayloadStatusFromSsz -> parsePayloadStatusFromSsz.
Updated in 4173745.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c940d92a9
ℹ️ 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".
| try { | ||
| return await this.fetchSszWithRetries(method, dispatchPlan.request, methodOpts); | ||
| } catch (e) { |
There was a problem hiding this comment.
Preserve engine state transitions on SSZ request errors
When a method is negotiated for SSZ, this branch calls fetchSszWithRetries() directly instead of this.rpc.fetchWithRetries(), so JsonRpcHttpClientEvent.ERROR/RESPONSE are not emitted for those calls. ExecutionEngineHttp currently uses those emitter callbacks to move between ONLINE/OFFLINE and trigger metadata negotiation, so SSZ timeout/auth/network failures can leave state stale (for example, a failing SSZ notifyForkchoiceUpdate path now throws without transitioning to OFFLINE). Please propagate SSZ outcomes through the same state-update/event path as JSON-RPC.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Implemented. SSZ dispatch path now updates engine state explicitly:
- on SSZ success: transition through ONLINE target path
- on non-unsupported SSZ errors: propagate through
getExecutionEngineState({payloadError: ...}) - unsupported endpoint statuses still go through JSON fallback without forcing a failure-state transition.
This preserves state transitions for SSZ timeout/auth/network failures while keeping fallback semantics intact.
Fixed in 4173745.
| status === HttpStatusCode.NOT_IMPLEMENTED || | ||
| status === HttpStatusCode.UNSUPPORTED_MEDIA_TYPE || | ||
| status === HttpStatusCode.BAD_REQUEST |
There was a problem hiding this comment.
Stop treating HTTP 400 as SSZ endpoint unsupported
Including BAD_REQUEST in isEngineSszUnsupportedStatus() causes fetchWithSelectedTransport() to fall back to JSON-RPC on any 400 from SSZ, but 400 commonly means malformed SSZ payload or invalid input on a supported endpoint rather than missing capability. In that case, fallback masks real SSZ codec/request bugs and can return a different result path than the negotiated transport. Restrict the unsupported classification to true capability-miss statuses (404/415/501).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. Removed 400 from isEngineSszUnsupportedStatus() so fallback now only triggers on capability-miss statuses (404/415/501).
Fixed in 4173745.
Summary
Adds Engine API SSZ transport plumbing and enables SSZ-first dispatch with automatic JSON-RPC fallback for methods currently used by Lodestar.
What this adds
ExecutionEngineHttp.fetchWithSelectedTransport()dispatches SSZ when negotiated404/415/501)Notes on current EL interoperability
bbusa/geth:ssz, Lodestar now attempts SSZ for negotiated mapped methods and falls back to JSON-RPC when SSZ endpoints are unsupported./engine/v1/client/version,/engine/v1/payloads/bodies/by-*) are treated as follow-up enhancement when a compatible EL target is available; current behavior remains non-blocking due to fallback.Verification
pnpm vitest packages/api/test/unit/client/engineSszCapabilities.test.ts packages/api/test/unit/client/engineSszDispatchPlan.test.ts packages/api/test/unit/client/engineSszHttp.test.ts packages/api/test/unit/client/engineSszLodestarProfile.test.ts packages/api/test/unit/client/engineSszMethodMap.test.ts packages/api/test/unit/client/engineSszNegotiation.test.ts packages/api/test/unit/client/engineSszTransportSelector.test.ts packages/beacon-node/test/unit/execution/engine/sszEndpointMap.test.ts packages/beacon-node/test/unit/execution/engine/sszTransport.test.ts packages/beacon-node/test/unit/execution/engine/http.sszFallback.test.ts packages/beacon-node/test/unit/execution/engine/http.sszGethE2e.test.ts packages/beacon-node/test/unit/execution/engine/http.sszPositiveE2e.test.tspnpm lint -- <changed-files>AI assistance
Implemented with AI assistance (Lodekeeper + sub-agent review workflow).