feat: engine API transport SSZ-REST - #9382
nazarhussain wants to merge 36 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements EIP-8161, introducing an SSZ-REST transport for the Engine API to improve communication efficiency. It adds a new SszRestClient and specialized encoding/decoding logic, updating the ExecutionEngineHttp class to prioritize SSZ-REST for key methods with a fallback to JSON-RPC. Review feedback identified critical bugs in the SSZ encoding and decoding for getBlobs requests and responses, specifically regarding the incorrect use of offsets for fixed-size lists. Additionally, improvements were suggested to enhance redundancy by supporting multiple engine URLs, refactor duplicated versioning logic into a helper method, and utilize existing utility functions for hex-to-byte conversions.
Performance Report🚀🚀 Significant benchmark improvement detected
Full benchmark results
|
2d17bb9 to
c5e1e38
Compare
## Summary Implements SSZ-REST Engine API transport on the consensus layer (client side), as specified in [ethereum/execution-apis#764](ethereum/execution-apis#764). - New CLI flag `--execution.sszRestUrl` to configure SSZ-REST endpoint - SSZ-encoded request/response bodies for all Engine API methods - Automatic fallback to JSON-RPC on network errors - Supports: `new_payload` (v1-v5), `forkchoice_updated` (v1-v3), `get_payload` (v1-v5), `exchange_capabilities` - Proper fork-based version selection for Deneb/Electra/Fulu --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Replace the hand-rolled byte-level encoders/decoders in sszRestEncoding.ts with @chainsafe/ssz ContainerType definitions for every Engine API request and response. This fixes a number of wire-format defects from #8994: - NewPayloadV1/V2 now carry the required Container offset prefix. - PayloadAttributes encoding matches the per-fork shape (V1 lacks withdrawals, V2 lacks parentBeaconBlockRoot, etc.) instead of always writing the V3 layout. - execution_requests is encoded as the spec's flat List[ByteList, 256] with proper SSZ list framing, not a flat concatenation of typed blobs. - GetPayloadResponse V2 (Shanghai) and the V5/V6 Osaka/Amsterdam shapes are now decodable. - GetBlobs V2 cell proofs (List[Bytes48, CELLS_PER_EXT_BLOB]) decode correctly; the previous fixed-stride scan only worked for V1. - Fork → version mapping is centralized in newPayloadVersion, getPayloadVersion, forkchoiceUpdatedVersion, and getBlobsVersion, fixing the prior fulu→v5 mismapping for newPayload and the v4 gap for forkchoiceUpdated. PayloadAttributes containers are redefined locally because ssz.{fork}.PayloadAttributes from @lodestar/types declares suggestedFeeRecipient with a JSON-only stringType that throws on SSZ serialize. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sszRest The SSZ-REST Engine API transport from #8994 was constructed unconditionally and probed on every Engine call, then silently fell back to JSON-RPC on network errors. Until ethereum/execution-apis#764 stabilises and the ELs we test against advertise support consistently, this probing is wasted traffic against vanilla EL deployments and can mask transient infra issues. Add a `sszRest` flag to ExecutionEngineHttpOpts and a hidden `--execution.sszRest` CLI flag. The SszRestClient is only constructed when the flag is set; otherwise the JSON-RPC path is used exclusively. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the local hexToBytes helper in favour of fromHex from @lodestar/utils, matching the convention used elsewhere in the package. Addresses gemini-code-assist feedback on #8994. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…baseUrl CodeQL (js/polynomial-redos) flagged engineUrl.replace(/\/+$/, "") at the SszRestClient init site. The regex is O(N^2) on N trailing slashes due to greedy + backtracking against the `$` anchor. The input is operator-supplied (--execution.urls), so the alert is not exploitable in our threat model, but the fix is trivial and clears the security alert. Use a linear charCode scan instead, and drop the redundant duplicate strip inside SszRestClient (the caller already normalises). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add SSZ containers for ExecutionPayloadBodyV1, PayloadBodiesV1Response, and the two request shapes from execution-apis#764, plus encoder/decoder helpers and the two HTTP call sites. Advertise the new endpoints in supportedSszRestEndpoints so the EL knows we support them; both methods negotiate via engine_exchangeCapabilities and fall back to JSON-RPC on network errors. Payload bodies can be sizeable (transactions + withdrawals), so binary SSZ avoids the hex-encoding bloat of the JSON-RPC equivalent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add SSZ containers for ClientVersionV1, GetClientVersionV1Request, and GetClientVersionV1Response from execution-apis#764, plus the call site in the existing getClientVersion path. Split the response handling into fetchClientVersions (raw transport) and the surrounding code mapping (ClientCode enum + strip 0x prefix). Advertise POST /engine/v1/client/version in supportedSszRestEndpoints; the call negotiates via engine_exchangeCapabilities and falls back to JSON-RPC on network errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Re-add POST /engine/v1/blobs to supportedSszRestEndpoints (removed in e4d5d11) and flip the v1 SSZ-REST test to assert the new behaviour. Spec v1 returns List[BlobAndProofV1, MAX_BLOB_HASHES_REQUEST] with no per-element nullability, while the JSON-RPC v1 contract returns a same-length array with null for missing blobs. Map the gap by padding the SSZ response up to the request length with null, assuming the EL returns results in request order with any trailing positions missing. This is a Lodestar-side assumption since the spec is silent on response ordering for v1; revisit if interop testing surfaces ELs that return out-of-order results. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Osaka SSZ spec defines both v2 (all-or-nothing) and v3 (per-element nullable) blob endpoints. Lodestar wires only v2; record the four reasons inline on getBlobsVersion so a future reader doesn't have to reconstruct them: - IExecutionEngine.getBlobs post-Fulu is all-or-nothing by design - Transport-symmetric with the existing JSON-RPC v2 path - Matches the spec's own guidance for all-or-nothing consumers - Buffer-reuse optimisation in block production assumes all-or-nothing Plus a note on when to revisit (if a granular blob-fetch consumer appears) and that picking v2 has no interop cost since the major ELs (Nethermind, Erigon) serve both. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
13a1eee to
3534a7a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #9382 +/- ##
============================================
- Coverage 52.55% 52.54% -0.01%
============================================
Files 848 848
Lines 60950 60927 -23
Branches 4487 4486 -1
============================================
- Hits 32034 32016 -18
+ Misses 28854 28849 -5
Partials 62 62 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3534a7aa12
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const PayloadStatusV1 = new ContainerType( | ||
| {status: Uint8, latestValidHash: NullableHash, validationError: ValidationErrorBytes}, | ||
| {typeName: "PayloadStatusV1"} |
There was a problem hiding this comment.
Encode PayloadStatus with spec field shapes
This container uses latestValidHash as List[Bytes32, 1], but the SSZ-REST Engine schema defines PayloadStatusV1.latest_valid_hash as fixed Bytes32 (zero-hash sentinel for absence), and ForkchoiceUpdatedResponseV1.payload_id similarly as fixed Bytes8. Using nullable-list wrappers here changes the wire layout, so a compliant EL response will fail deserialization (or be misdecoded) and interop will break for newPayload/forkchoiceUpdated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Current head follows merged execution-apis#793, where PayloadStatus.latest_valid_hash and ForkchoiceUpdateResponse.payload_id are Optional[...] (List[T, 1]). The older fixed-field #764 shape this comment was based on no longer applies.
| case 2: | ||
| return ExecutionPayloadStatus.SYNCING; | ||
| case 3: | ||
| return ExecutionPayloadStatus.ACCEPTED; | ||
| default: | ||
| throw Error(`Unknown payload status byte=${byte}`); |
There was a problem hiding this comment.
Handle INVALID_BLOCK_HASH status byte
statusByteToEnum maps only values 0-3 and throws on any other byte, but PayloadStatusV1 also includes INVALID_BLOCK_HASH (value 4). When an EL returns that valid status for newPayload, the SSZ path will throw instead of returning a structured ExecutePayloadResponse, turning a protocol-level response into an unexpected exception path during block verification.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
execution-apis#793 removes INVALID_BLOCK_HASH from the REST PayloadStatus enum; valid REST status bytes are 0..3. Current head intentionally rejects byte 4 and has coverage for that, so this stale JSON-RPC status concern no longer applies to the REST path.
lodekeeper
left a comment
There was a problem hiding this comment.
I found two additional blockers in the SSZ-REST transport. I also agree with the existing unresolved comments on the PayloadStatusV1 / ForkchoiceUpdatedResponseV1 wire shape and the missing INVALID_BLOCK_HASH status byte; those need to be fixed before this can interop with the current EIP-8178 shape.
| // (matches the JSON-RPC v1 contract). This assumes ELs return | ||
| // results in request order with trailing missing entries. | ||
| const found = decodeGetBlobsV1Response(resp); | ||
| return versionedHashes.map((_, i) => found[i] ?? null); |
There was a problem hiding this comment.
🔴 This loses the request-to-response mapping for partial pre-Fulu blob responses.
The existing JSON-RPC engine_getBlobsV1 contract returns one entry per requested hash, with null in the exact missing positions; Lodestar relies on that at the caller by indexing the response back into blobMeta. This SSZ v1 response is a compact List[BlobAndProofV1], so after a request like [A, B, C] where the EL only has A and C, found is [A, C] and this maps it to [A, C, null]. The blob/proof for C is then attached to B's blob index and commitment.
Because BlobAndProofV1 does not carry the requested versioned hash, the client cannot reconstruct arbitrary missing positions from this wire shape. Please either keep v1 blobs on JSON-RPC / fall back when the SSZ v1 length differs from the request length, or use a response shape with per-element nullability before adapting it to Lodestar's indexed (BlobAndProof | null)[] API.
There was a problem hiding this comment.
This blocker is addressed on the current head. The #793 /blobs/v1 and /blobs/v2 responses now use per-request-entry availability (BlobEntry {available, contents}), and the decoder preserves the requested-index mapping by returning null at unavailable positions. There is also regression coverage for v1: available=false -> null at that index and the HTTP path's partial-response behavior.
I do not have permission to resolve the thread in GitHub, but from my side this stale blocker is closed.
| parentBlockRoot, | ||
| executionRequests | ||
| ); | ||
| const resp = await this.rpcFetchQueue.push<Uint8Array>(async () => { |
There was a problem hiding this comment.
🔴 The fallback is outside the serialized queue item, so a failed SSZ request can let later Engine calls overtake its JSON-RPC retry.
JobFnQueue rejects this job and immediately starts the next queued job before the awaiting caller reaches this catch and enqueues the JSON-RPC fallback below. If newPayload is in flight over SSZ, a later FCU is already queued, and the SSZ request times out, the queue can run the later FCU before the earlier newPayload JSON-RPC fallback. That breaks the ordering invariant described above this queue, exactly in the sync path where newPayload/FCU ordering matters.
Please queue the whole logical Engine call as one unit: inside the queued function, try SSZ and perform the JSON-RPC fallback before resolving/rejecting, so no later queued Engine call can observe or overtake the retry.
There was a problem hiding this comment.
This blocker is addressed on the current head. The old per-request SSZ-then-JSON retry path is gone; REST newPayload / forkchoiceUpdated calls are pushed directly through the existing serialized rpcFetchQueue, while unsupported forks or unavailable capabilities route before dispatch. That removes the queue-rejection-then-later-JSON-retry overtake path this comment described.
I do not have permission to resolve the thread in GitHub, but from my side this stale blocker is closed.
…#793 Fork header map (CL fork -> EL fork name), spec MAX_* constants, and the fork-invariant PayloadStatus / ForkchoiceUpdateResponse containers with Optional[String] validation_error. Removes the #764 versioned containers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Drops expected_blob_versioned_hashes; folds parent_beacon_block_root and execution_requests into the envelope in spec field order. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Amsterdam carries custody_columns as an absent Optional[Bitvector[128]]. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… order execution_requests now precedes should_override_builder (was reversed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Eth-Execution-Version and X-Engine-Client-Version headers, JWT without clv, 204 -> null, RFC 7807 problem+json errors as SszRestError extends HttpRpcError. Removes the per-request network-error fallback classifier. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…n SszRestClient
toSszRestError no longer discards the raw response text as `detail` when a JSON
error body parses but isn't RFC 7807-shaped (e.g. legacy {code,message} bodies).
send() now reads the response body inside the same try/finally as fetch(), so
the abort timer also bounds a stalled 200/204 body read, not just the initial
request; a DOMException abort from that read is normalized to the same
FetchError/ERR_ABORTED shape callers already see for request-level timeouts.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…robe event policy Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…paths Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ntics Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…test Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Probe /engine/v1/capabilities once; route forks it advertises to REST and everything else to JSON-RPC. Removes engine_exchangeCapabilities negotiation and the per-request JSON-RPC fallback. Also fixes pre-existing tsgo errors in sszRestEncoding.test.ts (payloadFor overloads; entry/response generic-bound simplification) blocking a clean repo-wide check-types. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ntics Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Commit 21c173a accidentally started forwarding caller-provided buffers to the JSON-RPC engine_getBlobsV2 call. At merge-base with unstable, buffers were silently dropped on that path. Restore that behaviour so nodes that never enable sszRest see no change; the REST transport legitimately forwards buffers via rest.blobsV2(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
produceBlockBody.ts carried a pre-existing branch commit adding a getTargetGasLimit() helper and an unconditional targetGasLimit override for gloas payload attributes. unstable already sets targetGasLimit inside preparePayloadAttributes via getProposerTargetGasLimit, so the override was redundant and diverged from the SSE payload-attributes path. Restore the merge-base file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The REST branch of notifyNewPayload dispatched before validating versionedHashes/parentBlockRoot/executionRequests, so a caller bug would throw inside the queued REST closure, get caught by the .catch() that maps errors to UNAVAILABLE, and silently mark a healthy EL OFFLINE. Move the same checks (same conditions/messages as the JSON-RPC branch) to the top of the method so both transports fail synchronously before any request, and drop the now-duplicate checks from the JSON-RPC branch. Add a regression test asserting the REST route is never hit and engine.state stays ONLINE when preconditions fail. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The REST transport only ever talks to opts.urls[0]. When more than one execution URL is configured, log a warning at startup so operators relying on JSON-RPC-style fallback URLs notice the REST transport doesn't provide it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- sszRestEngine.ts: drop the unused `ready` field, make `ssz()`/`json()`
private (no subclasses reference them as protected).
- sszRestEncoding.ts: EXECUTION_PAYLOAD_BY_EL_FORK is only used within
the file, drop the export.
- sszRestEngine.ts getPayload: validate payloadId against
/^0x[0-9a-f]{16}$/i before building the URL, with a regression test
asserting no request is made for a malformed id.
- httpSszRest.test.ts: replace the console logger with a silent local
stub instead of leaking test noise to the console.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolve http.ts conflict: serializeExecutionRequests now takes the fork. Adapt the SSZ-REST codec to unstable's gloas changes: builder deposit / exit requests (0x03 / 0x04) in the execution_requests list, and targetGasLimit as UintBn64 to match PayloadAttributes.targetGasLimit: bigint. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The codec reuses ssz.gloas.ExecutionPayload, and the codec tests built their oracle from the same symbol — so a consensus-specs change moved implementation and oracle in lockstep and the suite stayed green while the Engine API wire format drifted from the spec. This nearly bit when gloas became an EIP-7688 ProgressiveContainer: the bytes still matched, but nothing checked that. Pin the spec's field list literally, and assert the progressive framing stays serialization-identical to a plain SSZ container. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
blobsRevision() was derived from ForkSeq alone, so a fork with no Eth-Execution-Version mapping (heze, added on unstable) would still take /engine/v1/blobs/v2 while every other method correctly fell back to JSON-RPC. /blobs/vN is unscoped so nothing in the request needed the mapping, but a fork the transport cannot name is one whose blob semantics it cannot vouch for. Return null for such forks so blobs behave like the rest of the surface. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Superseded by #10155. This PR was opened against execution-apis#764, which was closed without merging in favour of #793. #793 changes the wire format substantially — versioning moves from per-method URLs ( Rather than keep rewriting in place, #10155 is a fresh branch off Both blockers raised here are resolved there:
The stale review comments above refer to the #764 encoding and no longer apply; thanks for the reviews regardless — the two blockers were real and shaped the replacement. |
Summary
Implements the SSZ-REST Engine API transport on the consensus-layer side, following
ethereum/execution-apis#793
(
src/engine/refactor.md,src/engine/refactor-ssz.md). Supersedes the earlier#764-based wire format this PR started from.
--execution.sszRest; default off.GET /engine/v1/capabilitiesprobe; REST is used for the forks the EL advertises, JSON-RPC for everything else. No per-request fallback (spec § Transition-window behavior).POST /payloads,POST /forkchoice,GET /payloads/{id},POST /bodies/hash,GET /bodies?from&count,POST /blobs/v1,POST /blobs/v2,GET /identity.Eth-Execution-Versionfork header (bellatrix→paris … gloas→amsterdam),X-Engine-Client-Versionheader, JWT withoutclv, RFC 7807application/problem+jsonerrors,204 No Contenton blobs.sszRestEncoding.ts— the only file that knows the wire shape.Out of scope (follow-ups; need
IExecutionEnginechanges)POST /blobs/v3,POST /blobs/v4, forkchoicecustody_columns(encoded as absent on Amsterdam), HTTP/2 client.lodestar_execution_engine_http_client_*metrics yet (JSON-RPC metrics unchanged).getBlobsdoes not forward caller-provided buffers on the JSON-RPC path; left as-is here, to be fixed separately with its own test.limits.payload.max_bytesfromGET /capabilitiesis parsed but not enforced client-side; an oversize body surfaces as the EL's413rather than failing locally.getPayloadBodiesByHash/ByRangehave no production callers in this repo today.Testing
Unit tests only: spec-derived oracle containers and a Fastify fake EL under
packages/beacon-node/test/unit/executionEngine/sszRest*.test.tsandhttpSszRest.test.ts.http.test.ts(JSON-RPC) is unchanged and green.getPayloadBodiesByHash/ByRangehave no production callers in this repo today; the REST bodies path is exercised by unit tests only.No execution client serves #793 yet, so there is no interop test.
refactor-ssz.mdmarksMAX_BAL_BYTES/MAX_BYTES_PER_EXECUTION_REQUESTas placeholders; they are single constantsat the top of
sszRestEncoding.ts.AI disclosure
Implemented with AI assistance (Claude Code); design, review, and validation by the author.
🤖 Generated with Claude Code