From ac6751b84520da08cd759cc64b8d3ba304343bbe Mon Sep 17 00:00:00 2001 From: MariusVanDerWijden Date: Wed, 17 Jun 2026 08:31:47 +0200 Subject: [PATCH] eth/engineapi: proper pre-amsterdam support --- beacon/engine/ssz/envelopes.go | 16 +++++--- eth/engineapi/bodies.go | 2 +- eth/engineapi/forkchoice.go | 16 ++++---- eth/engineapi/forkresolve.go | 42 +++++++++++++++++++++ eth/engineapi/get_payload.go | 7 +++- eth/engineapi/payloads.go | 6 +-- eth/engineapi/router_test.go | 69 +++++++++++++++++++++++++++++++--- 7 files changed, 134 insertions(+), 24 deletions(-) diff --git a/beacon/engine/ssz/envelopes.go b/beacon/engine/ssz/envelopes.go index 944cd7e328..48a7dbfc6a 100644 --- a/beacon/engine/ssz/envelopes.go +++ b/beacon/engine/ssz/envelopes.go @@ -125,23 +125,29 @@ type ForkchoiceUpdateAmsterdam struct { } func (f *ForkchoiceUpdateAmsterdam) SizeSSZ(siz *ssz.Sizer, fixed bool) uint32 { - // 96 (state) + 4(offset attrs) + 4(offset custody) - size := uint32(96 + 8) + // 96 (state) + 4(offset attrs); the custody offset only exists from + // Amsterdam, matching the OnFork gating below. + size := uint32(96 + 4) + if siz.Fork() >= forkAmsterdam { + size += 4 // offset custody + } if fixed { return size } size += ssz.SizeSliceOfDynamicObjects(siz, f.PayloadAttributes) - size += ssz.SizeSliceOfStaticObjects(siz, f.CustodyColumns) + if siz.Fork() >= forkAmsterdam { + size += ssz.SizeSliceOfStaticObjects(siz, f.CustodyColumns) + } return size } func (f *ForkchoiceUpdateAmsterdam) DefineSSZ(c *ssz.Codec) { ssz.DefineStaticObject(c, &f.ForkchoiceState) ssz.DefineSliceOfDynamicObjectsOffset(c, &f.PayloadAttributes, 1) - ssz.DefineSliceOfStaticObjectsOffset(c, &f.CustodyColumns, 1) + ssz.DefineSliceOfStaticObjectsOffsetOnFork(c, &f.CustodyColumns, 1, fromAmsterdam) ssz.DefineSliceOfDynamicObjectsContent(c, &f.PayloadAttributes, 1) - ssz.DefineSliceOfStaticObjectsContent(c, &f.CustodyColumns, 1) + ssz.DefineSliceOfStaticObjectsContentOnFork(c, &f.CustodyColumns, 1, fromAmsterdam) } // Validate enforces the Optional[T] = List[T,1] length invariants. diff --git a/eth/engineapi/bodies.go b/eth/engineapi/bodies.go index 6dd77209b8..75f76136a3 100644 --- a/eth/engineapi/bodies.go +++ b/eth/engineapi/bodies.go @@ -79,7 +79,7 @@ func buildBodiesResponse(b Backend, fork forks.Fork, sf ssz.Fork, bodies []*type } for i, body := range bodies { entry := &sszt.BodyEntry{Body: new(sszt.ExecutionPayloadBody)} - if body != nil && b.ForkFromTimestamp(ts[i]) == fork { + if body != nil && baseFork(b.ForkFromTimestamp(ts[i])) == fork { entry.Available = true entry.Body = bodyToSSZ(body, sf) } diff --git a/eth/engineapi/forkchoice.go b/eth/engineapi/forkchoice.go index 15acd786c6..a433cfa8fa 100644 --- a/eth/engineapi/forkchoice.go +++ b/eth/engineapi/forkchoice.go @@ -26,9 +26,9 @@ import ( // handleForkchoice implements POST /engine/v2/{fork}/forkchoice. func (rt *Router) handleForkchoice(w http.ResponseWriter, r *http.Request, fork forks.Fork) { - // The execution-apis spec currently only defines the forkchoice envelope - // for Amsterdam; the inner attributes shape is fork-driven by the codec. - sf, ok := resolveFork(w, fork, forks.Amsterdam) + // The forkchoice envelope shape is fork-driven by the codec; every fork + // from Paris on has a valid wire shape. + sf, ok := resolveFork(w, fork, forks.Paris) if !ok { return } @@ -49,10 +49,10 @@ func (rt *Router) handleForkchoice(w http.ResponseWriter, r *http.Request, fork return } // If PayloadAttributes is present the URL fork MUST match the fork - // the new payload would belong to. Today only Amsterdam URL exists - // in this implementation so the timestamp check is implicit; we - // keep an explicit guard for future fork URLs. - if rt.backend.ForkFromTimestamp(attr.Timestamp) != fork { + // the new payload would belong to. ForkFromTimestamp can return a BPO + // fork (which has no URL segment of its own); collapse it onto the + // named fork it layers on before comparing. + if baseFork(rt.backend.ForkFromTimestamp(attr.Timestamp)) != fork { writeProblem(w, http.StatusBadRequest, ErrUnsupportedFork, "payload_attributes timestamp does not match URL fork") return @@ -64,7 +64,7 @@ func (rt *Router) handleForkchoice(w http.ResponseWriter, r *http.Request, fork // underlying miner gains the corresponding setting. } - resp, err := rt.backend.ForkchoiceUpdated(r.Context(), state, attrs, engine.PayloadV4) + resp, err := rt.backend.ForkchoiceUpdated(r.Context(), state, attrs, payloadVersionFor(fork)) if err != nil { mapBackendErr(w, err) return diff --git a/eth/engineapi/forkresolve.go b/eth/engineapi/forkresolve.go index 3f4af2c657..ec6ed6479b 100644 --- a/eth/engineapi/forkresolve.go +++ b/eth/engineapi/forkresolve.go @@ -19,6 +19,7 @@ package engineapi import ( "net/http" + "github.com/ethereum/go-ethereum/beacon/engine" sszt "github.com/ethereum/go-ethereum/beacon/engine/ssz" "github.com/ethereum/go-ethereum/params/forks" "github.com/karalabe/ssz" @@ -39,3 +40,44 @@ func resolveFork(w http.ResponseWriter, fork, min forks.Fork) (ssz.Fork, bool) { } return sf, true } + +// baseFork collapses a BPO fork onto the named fork that it layers on. BPO1..5 +// sit between Osaka and Amsterdam in params/forks but have no Engine API URL +// segment of their own: a chain in a BPO era still negotiates /osaka/*. Named +// forks map to themselves. +func baseFork(f forks.Fork) forks.Fork { + switch f { + case forks.BPO1, forks.BPO2, forks.BPO3, forks.BPO4, forks.BPO5: + return forks.Osaka + default: + return f + } +} + +// eraForks returns the set of params/forks values that share a URL fork's +// wire shape: the named fork plus any BPO forks that layer on it. checkFork in +// the catalyst layer derives a cached payload's fork via LatestFork, which can +// return a BPO fork, so the allowed set passed to GetPayload must include them. +func eraForks(fork forks.Fork) []forks.Fork { + if fork == forks.Osaka { + return []forks.Fork{forks.Osaka, forks.BPO1, forks.BPO2, forks.BPO3, forks.BPO4, forks.BPO5} + } + return []forks.Fork{fork} +} + +// payloadVersionFor selects the JSON-RPC PayloadVersion that the catalyst layer +// expects for a given URL fork, mirroring the per-fork version table on the +// JSON-RPC ForkchoiceUpdatedVx path (Amsterdam -> V4, Cancun..Osaka -> V3, +// Shanghai -> V2, Paris -> V1). +func payloadVersionFor(fork forks.Fork) engine.PayloadVersion { + switch { + case fork >= forks.Amsterdam: + return engine.PayloadV4 + case fork >= forks.Cancun: + return engine.PayloadV3 + case fork >= forks.Shanghai: + return engine.PayloadV2 + default: + return engine.PayloadV1 + } +} diff --git a/eth/engineapi/get_payload.go b/eth/engineapi/get_payload.go index 5975c0114f..5e7ab3a80a 100644 --- a/eth/engineapi/get_payload.go +++ b/eth/engineapi/get_payload.go @@ -29,7 +29,7 @@ import ( // handleGetPayload implements GET /engine/v2/{fork}/payloads/{payloadId}. func (rt *Router) handleGetPayload(w http.ResponseWriter, r *http.Request, fork forks.Fork, idHex string) { - sf, ok := resolveFork(w, fork, forks.Amsterdam) + sf, ok := resolveFork(w, fork, forks.Paris) if !ok { return } @@ -41,7 +41,10 @@ func (rt *Router) handleGetPayload(w http.ResponseWriter, r *http.Request, fork var id engine.PayloadID copy(id[:], raw) - env, err := rt.backend.GetPayload(id, []forks.Fork{forks.Amsterdam}) + // allowedForks is the URL fork's era: the named fork plus any BPO forks + // that layer on it. The catalyst layer derives the cached payload's fork + // via LatestFork, which can yield a BPO fork. + env, err := rt.backend.GetPayload(id, eraForks(fork)) if err != nil { mapBackendErr(w, err) return diff --git a/eth/engineapi/payloads.go b/eth/engineapi/payloads.go index 6e4e946fb7..7fa92d0eb5 100644 --- a/eth/engineapi/payloads.go +++ b/eth/engineapi/payloads.go @@ -27,9 +27,9 @@ import ( // handleNewPayload implements POST /engine/v2/{fork}/payloads. func (rt *Router) handleNewPayload(w http.ResponseWriter, r *http.Request, fork forks.Fork) { - // The execution-apis spec currently only defines the payload envelope for - // Amsterdam; the inner payload shape is fork-driven by the codec. - sf, ok := resolveFork(w, fork, forks.Amsterdam) + // The payload envelope shape is fork-driven by the codec; every fork from + // Paris on has a valid wire shape. + sf, ok := resolveFork(w, fork, forks.Paris) if !ok { return } diff --git a/eth/engineapi/router_test.go b/eth/engineapi/router_test.go index 3f3534e7f7..2e55c31006 100644 --- a/eth/engineapi/router_test.go +++ b/eth/engineapi/router_test.go @@ -196,7 +196,7 @@ func TestRouterUnsupportedFork(t *testing.T) { srv := newTestServer(t, &stubBackend{}) defer srv.Close() - // Bogus fork in URL. + // Bogus fork in URL: the router does not recognise the segment at all. resp, err := srv.Client().Post(srv.URL+BasePath+"/bogus/payloads", sszContentType, bytes.NewReader(nil)) if err != nil { t.Fatal(err) @@ -205,15 +205,74 @@ func TestRouterUnsupportedFork(t *testing.T) { if resp.StatusCode != 400 { t.Fatalf("want 400, got %d", resp.StatusCode) } +} - // Real fork but not Amsterdam. - resp, err = srv.Client().Post(srv.URL+BasePath+"/cancun/payloads", sszContentType, bytes.NewReader(nil)) +// TestRouterAdvertisedForkRoutable asserts that a fork the capabilities +// endpoint advertises (here osaka) is actually routable end-to-end: forkchoice +// with attributes, newPayload, and getPayload all succeed. This guards against +// advertising a fork the handlers reject (the failure mode where capabilities +// outran the per-handler fork gating). +func TestRouterAdvertisedForkRoutable(t *testing.T) { + id := engine.PayloadID{4, 4, 4} + env := &engine.ExecutionPayloadEnvelope{ + ExecutionPayload: &engine.ExecutableData{LogsBloom: make([]byte, 256)}, + BlockValue: uint256.NewInt(0).ToBig(), + } + b := &stubBackend{ + fcuStatus: engine.PayloadStatusV1{Status: engine.VALID}, + fcuID: &id, + npStatus: engine.PayloadStatusV1{Status: engine.VALID}, + envelope: env, + // Osaka chain sitting in a BPO era: ForkFromTimestamp returns a BPO + // fork, which must collapse onto the osaka URL fork. + forkAtTime: func(uint64) forks.Fork { return forks.BPO1 }, + } + srv := newTestServer(t, b) + defer srv.Close() + + osaka, _ := sszt.ForkFor(forks.Osaka) + blob, excess := uint64(0), uint64(0) + + // forkchoice with payload attributes (proposal path). + fcu := &sszt.ForkchoiceUpdateAmsterdam{ + ForkchoiceState: &sszt.ForkchoiceState{HeadBlockHash: common.Hash{0xaa}}, + PayloadAttributes: []*sszt.PayloadAttributes{{ + Timestamp: 1, + ParentBeaconBlockRoot: &common.Hash{0x55}, + }}, + } + resp := sszPost(t, srv, "/osaka/forkchoice", fcu, osaka) + if resp.StatusCode != 200 { + t.Fatalf("forkchoice: want 200, got %d", resp.StatusCode) + } + resp.Body.Close() + if b.lastFCUAttrs == nil { + t.Fatal("forkchoice: attributes not forwarded to backend") + } + + // newPayload. + penv := &sszt.ExecutionPayloadEnvelopeAmsterdam{ + Payload: &sszt.ExecutionPayload{ + BaseFeePerGas: uint256.NewInt(7e9), + BlobGasUsed: &blob, + ExcessBlobGas: &excess, + }, + ParentBeaconBlockRoot: common.Hash{0x55}, + } + resp = sszPost(t, srv, "/osaka/payloads", penv, osaka) + if resp.StatusCode != 200 { + t.Fatalf("newPayload: want 200, got %d", resp.StatusCode) + } + resp.Body.Close() + + // getPayload. + resp, err := srv.Client().Get(srv.URL + BasePath + "/osaka/payloads/0x0102030405060708") if err != nil { t.Fatal(err) } resp.Body.Close() - if resp.StatusCode != 400 { - t.Fatalf("want 400 for cancun, got %d", resp.StatusCode) + if resp.StatusCode != 200 { + t.Fatalf("getPayload: want 200, got %d", resp.StatusCode) } }