Use engine API that matches hardfork version (#9253)

* Use engine API that matches HF

* Simplify engine client code by using EngineAPIVersion

* Return full method string from engine api version methods

* FCUVersion takes payload attributes

* Use a proper method

* Handle switch default case explicitly

* Return fcuV3 if attrs is nil

* Add test cases for engine api version methods

op-e2e/actions/l2_engine_test.go

@@ -154,7 +154,7 @@ func TestL2EngineAPIBlockBuilding(gt *testing.T) {
 			engine.ActL2IncludeTx(dp.Addresses.Alice)(t)
 		}
 
-		envelope, err := l2Cl.GetPayload(t.Ctx(), *fcRes.PayloadID)
+		envelope, err := l2Cl.GetPayload(t.Ctx(), eth.PayloadInfo{ID: *fcRes.PayloadID, Timestamp: uint64(nextBlockTime)})
 		payload := envelope.ExecutionPayload
 		require.NoError(t, err)
 		require.Equal(t, parent.Hash(), payload.ParentHash, "block builds on parent block")

op-e2e/op_geth.go

@@ -146,7 +146,7 @@ func (d *OpGeth) AddL2Block(ctx context.Context, txs ...*types.Transaction) (*et
 		return nil, err
 	}
 
-	envelope, err := d.l2Engine.GetPayload(ctx, *res.PayloadID)
+	envelope, err := d.l2Engine.GetPayload(ctx, eth.PayloadInfo{ID: *res.PayloadID, Timestamp: uint64(attrs.Timestamp)})
 	payload := envelope.ExecutionPayload
 
 	if err != nil {

op-e2e/op_geth_test.go

@@ -196,7 +196,7 @@ func TestGethOnlyPendingBlockIsLatest(t *testing.T) {
 	time.Sleep(time.Second * 4) // conservatively wait 4 seconds, CI might lag during block building.
 
 	// retrieve the block
-	envelope, err := opGeth.l2Engine.GetPayload(ctx, *res.PayloadID)
+	envelope, err := opGeth.l2Engine.GetPayload(ctx, eth.PayloadInfo{ID: *res.PayloadID, Timestamp: uint64(attrs.Timestamp)})
 	require.NoError(t, err)
 
 	payload := envelope.ExecutionPayload

op-node/rollup/derive/engine_controller.go

@@ -38,7 +38,7 @@ var _ EngineControl = (*EngineController)(nil)
 var _ LocalEngineControl = (*EngineController)(nil)
 
 type ExecEngine interface {
-	GetPayload(ctx context.Context, payloadId eth.PayloadID) (*eth.ExecutionPayloadEnvelope, error)
+	GetPayload(ctx context.Context, payloadInfo eth.PayloadInfo) (*eth.ExecutionPayloadEnvelope, error)
 	ForkchoiceUpdate(ctx context.Context, state *eth.ForkchoiceState, attr *eth.PayloadAttributes) (*eth.ForkchoiceUpdatedResult, error)
 	NewPayload(ctx context.Context, payload *eth.ExecutionPayload, parentBeaconBlockRoot *common.Hash) (*eth.PayloadStatusV1, error)
 	L2BlockRefByLabel(ctx context.Context, label eth.BlockLabel) (eth.L2BlockRef, error)
@@ -63,7 +63,7 @@ type EngineController struct {
 
 	// Building State
 	buildingOnto eth.L2BlockRef
-	buildingID   eth.PayloadID
+	buildingInfo eth.PayloadInfo
 	buildingSafe bool
 	safeAttrs    *AttributesWithParent
 }
@@ -104,7 +104,7 @@ func (e *EngineController) Finalized() eth.L2BlockRef {
 }
 
 func (e *EngineController) BuildingPayload() (eth.L2BlockRef, eth.PayloadID, bool) {
-	return e.buildingOnto, e.buildingID, e.buildingSafe
+	return e.buildingOnto, e.buildingInfo.ID, e.buildingSafe
 }
 
 func (e *EngineController) IsEngineSyncing() bool {
@@ -146,8 +146,8 @@ func (e *EngineController) StartPayload(ctx context.Context, parent eth.L2BlockR
 	if e.IsEngineSyncing() {
 		return BlockInsertTemporaryErr, fmt.Errorf("engine is in progess of p2p sync")
 	}
-	if e.buildingID != (eth.PayloadID{}) {
-		e.log.Warn("did not finish previous block building, starting new building now", "prev_onto", e.buildingOnto, "prev_payload_id", e.buildingID, "new_onto", parent)
+	if e.buildingInfo != (eth.PayloadInfo{}) {
+		e.log.Warn("did not finish previous block building, starting new building now", "prev_onto", e.buildingOnto, "prev_payload_id", e.buildingInfo.ID, "new_onto", parent)
 		// TODO(8841): maybe worth it to force-cancel the old payload ID here.
 	}
 	fc := eth.ForkchoiceState{
@@ -161,7 +161,7 @@ func (e *EngineController) StartPayload(ctx context.Context, parent eth.L2BlockR
 		return errTyp, err
 	}
 
-	e.buildingID = id
+	e.buildingInfo = eth.PayloadInfo{ID: id, Timestamp: uint64(attrs.attributes.Timestamp)}
 	e.buildingSafe = updateSafe
 	e.buildingOnto = parent
 	if updateSafe {
@@ -172,7 +172,7 @@ func (e *EngineController) StartPayload(ctx context.Context, parent eth.L2BlockR
 }
 
 func (e *EngineController) ConfirmPayload(ctx context.Context, agossip async.AsyncGossiper, sequencerConductor conductor.SequencerConductor) (out *eth.ExecutionPayloadEnvelope, errTyp BlockInsertionErrType, err error) {
-	if e.buildingID == (eth.PayloadID{}) {
+	if e.buildingInfo == (eth.PayloadInfo{}) {
 		return nil, BlockInsertPrestateErr, fmt.Errorf("cannot complete payload building: not currently building a payload")
 	}
 	if e.buildingOnto.Hash != e.unsafeHead.Hash { // E.g. when safe-attributes consolidation fails, it will drop the existing work.
@@ -185,9 +185,9 @@ func (e *EngineController) ConfirmPayload(ctx context.Context, agossip async.Asy
 	}
 	// Update the safe head if the payload is built with the last attributes in the batch.
 	updateSafe := e.buildingSafe && e.safeAttrs != nil && e.safeAttrs.isLastInSpan
-	envelope, errTyp, err := confirmPayload(ctx, e.log, e.engine, fc, e.buildingID, updateSafe, agossip, sequencerConductor)
+	envelope, errTyp, err := confirmPayload(ctx, e.log, e.engine, fc, e.buildingInfo, updateSafe, agossip, sequencerConductor)
 	if err != nil {
-		return nil, errTyp, fmt.Errorf("failed to complete building on top of L2 chain %s, id: %s, error (%d): %w", e.buildingOnto, e.buildingID, errTyp, err)
+		return nil, errTyp, fmt.Errorf("failed to complete building on top of L2 chain %s, id: %s, error (%d): %w", e.buildingOnto, e.buildingInfo.ID, errTyp, err)
 	}
 	ref, err := PayloadToBlockRef(e.rollupCfg, envelope.ExecutionPayload)
 	if err != nil {
@@ -211,14 +211,14 @@ func (e *EngineController) ConfirmPayload(ctx context.Context, agossip async.Asy
 }
 
 func (e *EngineController) CancelPayload(ctx context.Context, force bool) error {
-	if e.buildingID == (eth.PayloadID{}) { // only cancel if there is something to cancel.
+	if e.buildingInfo == (eth.PayloadInfo{}) { // only cancel if there is something to cancel.
 		return nil
 	}
 	// the building job gets wrapped up as soon as the payload is retrieved, there's no explicit cancel in the Engine API
-	e.log.Error("cancelling old block sealing job", "payload", e.buildingID)
-	_, err := e.engine.GetPayload(ctx, e.buildingID)
+	e.log.Error("cancelling old block sealing job", "payload", e.buildingInfo.ID)
+	_, err := e.engine.GetPayload(ctx, e.buildingInfo)
 	if err != nil {
-		e.log.Error("failed to cancel block building job", "payload", e.buildingID, "err", err)
+		e.log.Error("failed to cancel block building job", "payload", e.buildingInfo.ID, "err", err)
 		if !force {
 			return err
 		}
@@ -228,7 +228,7 @@ func (e *EngineController) CancelPayload(ctx context.Context, force bool) error
 }
 
 func (e *EngineController) resetBuildingState() {
-	e.buildingID = eth.PayloadID{}
+	e.buildingInfo = eth.PayloadInfo{}
 	e.buildingOnto = eth.L2BlockRef{}
 	e.buildingSafe = false
 	e.safeAttrs = nil

op-node/rollup/derive/engine_update.go

@@ -124,7 +124,7 @@ func confirmPayload(
 	log log.Logger,
 	eng ExecEngine,
 	fc eth.ForkchoiceState,
-	id eth.PayloadID,
+	payloadInfo eth.PayloadInfo,
 	updateSafe bool,
 	agossip async.AsyncGossiper,
 	sequencerConductor conductor.SequencerConductor,
@@ -140,7 +140,7 @@ func confirmPayload(
 			"parent", envelope.ExecutionPayload.ParentHash,
 			"txs", len(envelope.ExecutionPayload.Transactions))
 	} else {
-		envelope, err = eng.GetPayload(ctx, id)
+		envelope, err = eng.GetPayload(ctx, payloadInfo)
 	}
 	if err != nil {
 		// even if it is an input-error (unknown payload ID), it is temporary, since we will re-attempt the full payload building, not just the retrieval of the payload.

op-node/rollup/types.go

@@ -353,6 +353,46 @@ func (c *Config) IsInterop(timestamp uint64) bool {
 	return c.InteropTime != nil && timestamp >= *c.InteropTime
 }
 
+// ForkchoiceUpdatedVersion returns the EngineAPIMethod suitable for the chain hard fork version.
+func (c *Config) ForkchoiceUpdatedVersion(attr *eth.PayloadAttributes) eth.EngineAPIMethod {
+	if attr == nil {
+		// Don't begin payload build process.
+		return eth.FCUV3
+	}
+	ts := uint64(attr.Timestamp)
+	if c.IsEcotone(ts) {
+		// Cancun
+		return eth.FCUV3
+	} else if c.IsCanyon(ts) {
+		// Shanghai
+		return eth.FCUV2
+	} else {
+		// According to Ethereum engine API spec, we can use fcuV2 here,
+		// but upstream Geth v1.13.11 does not accept V2 before Shanghai.
+		return eth.FCUV1
+	}
+}
+
+// NewPayloadVersion returns the EngineAPIMethod suitable for the chain hard fork version.
+func (c *Config) NewPayloadVersion(timestamp uint64) eth.EngineAPIMethod {
+	if c.IsEcotone(timestamp) {
+		// Cancun
+		return eth.NewPayloadV3
+	} else {
+		return eth.NewPayloadV2
+	}
+}
+
+// GetPayloadVersion returns the EngineAPIMethod suitable for the chain hard fork version.
+func (c *Config) GetPayloadVersion(timestamp uint64) eth.EngineAPIMethod {
+	if c.IsEcotone(timestamp) {
+		// Cancun
+		return eth.GetPayloadV3
+	} else {
+		return eth.GetPayloadV2
+	}
+}
+
 // Description outputs a banner describing the important parts of rollup configuration in a human-readable form.
 // Optionally provide a mapping of L2 chain IDs to network names to label the L2 chain with if not unknown.
 // The config should be config.Check()-ed before creating a description.

op-node/rollup/types_test.go

@@ -508,3 +508,118 @@ func TestTimestampForBlock(t *testing.T) {
 	}
 
 }
+
+func TestForkchoiceUpdatedVersion(t *testing.T) {
+	config := randConfig()
+	tests := []struct {
+		name           string
+		canyonTime     uint64
+		ecotoneTime    uint64
+		attrs          *eth.PayloadAttributes
+		expectedMethod eth.EngineAPIMethod
+	}{
+		{
+			name:           "NoAttrs",
+			canyonTime:     10,
+			ecotoneTime:    20,
+			attrs:          nil,
+			expectedMethod: eth.FCUV3,
+		},
+		{
+			name:           "BeforeCanyon",
+			canyonTime:     10,
+			ecotoneTime:    20,
+			attrs:          &eth.PayloadAttributes{Timestamp: 5},
+			expectedMethod: eth.FCUV1,
+		},
+		{
+			name:           "Canyon",
+			canyonTime:     10,
+			ecotoneTime:    20,
+			attrs:          &eth.PayloadAttributes{Timestamp: 15},
+			expectedMethod: eth.FCUV2,
+		},
+		{
+			name:           "Ecotone",
+			canyonTime:     10,
+			ecotoneTime:    20,
+			attrs:          &eth.PayloadAttributes{Timestamp: 25},
+			expectedMethod: eth.FCUV3,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(fmt.Sprintf("TestForkchoiceUpdatedVersion_%s", test.name), func(t *testing.T) {
+			config.CanyonTime = &test.canyonTime
+			config.EcotoneTime = &test.ecotoneTime
+			assert.Equal(t, config.ForkchoiceUpdatedVersion(test.attrs), test.expectedMethod)
+		})
+	}
+}
+
+func TestNewPayloadVersion(t *testing.T) {
+	config := randConfig()
+	canyonTime := uint64(0)
+	config.CanyonTime = &canyonTime
+	tests := []struct {
+		name           string
+		ecotoneTime    uint64
+		payloadTime    uint64
+		expectedMethod eth.EngineAPIMethod
+	}{
+		{
+			name:           "BeforeEcotone",
+			ecotoneTime:    10,
+			payloadTime:    5,
+			expectedMethod: eth.NewPayloadV2,
+		},
+		{
+			name:           "Ecotone",
+			ecotoneTime:    10,
+			payloadTime:    15,
+			expectedMethod: eth.NewPayloadV3,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(fmt.Sprintf("TestNewPayloadVersion_%s", test.name), func(t *testing.T) {
+			config.EcotoneTime = &test.ecotoneTime
+			assert.Equal(t, config.NewPayloadVersion(test.payloadTime), test.expectedMethod)
+		})
+	}
+}
+
+func TestGetPayloadVersion(t *testing.T) {
+	config := randConfig()
+	canyonTime := uint64(0)
+	config.CanyonTime = &canyonTime
+	tests := []struct {
+		name           string
+		ecotoneTime    uint64
+		payloadTime    uint64
+		expectedMethod eth.EngineAPIMethod
+	}{
+		{
+			name:           "BeforeEcotone",
+			ecotoneTime:    10,
+			payloadTime:    5,
+			expectedMethod: eth.GetPayloadV2,
+		},
+		{
+			name:           "Ecotone",
+			ecotoneTime:    10,
+			payloadTime:    15,
+			expectedMethod: eth.GetPayloadV3,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(fmt.Sprintf("TestGetPayloadVersion_%s", test.name), func(t *testing.T) {
+			config.EcotoneTime = &test.ecotoneTime
+			assert.Equal(t, config.GetPayloadVersion(test.payloadTime), test.expectedMethod)
+		})
+	}
+}

op-program/client/l2/engine.go

@@ -50,8 +50,17 @@ func (o *OracleEngine) L2OutputRoot(l2ClaimBlockNum uint64) (eth.Bytes32, error)
 	return rollup.ComputeL2OutputRootV0(eth.HeaderBlockInfo(outBlock), withdrawalsTrie.Hash())
 }
 
-func (o *OracleEngine) GetPayload(ctx context.Context, payloadId eth.PayloadID) (*eth.ExecutionPayloadEnvelope, error) {
-	res, err := o.api.GetPayloadV3(ctx, payloadId)
+func (o *OracleEngine) GetPayload(ctx context.Context, payloadInfo eth.PayloadInfo) (*eth.ExecutionPayloadEnvelope, error) {
+	var res *eth.ExecutionPayloadEnvelope
+	var err error
+	switch method := o.rollupCfg.GetPayloadVersion(payloadInfo.Timestamp); method {
+	case eth.GetPayloadV3:
+		res, err = o.api.GetPayloadV3(ctx, payloadInfo.ID)
+	case eth.GetPayloadV2:
+		res, err = o.api.GetPayloadV2(ctx, payloadInfo.ID)
+	default:
+		return nil, fmt.Errorf("unsupported GetPayload version: %s", method)
+	}
 	if err != nil {
 		return nil, err
 	}
@@ -59,14 +68,26 @@ func (o *OracleEngine) GetPayload(ctx context.Context, payloadId eth.PayloadID)
 }
 
 func (o *OracleEngine) ForkchoiceUpdate(ctx context.Context, state *eth.ForkchoiceState, attr *eth.PayloadAttributes) (*eth.ForkchoiceUpdatedResult, error) {
-	return o.api.ForkchoiceUpdatedV3(ctx, state, attr)
+	switch method := o.rollupCfg.ForkchoiceUpdatedVersion(attr); method {
+	case eth.FCUV3:
+		return o.api.ForkchoiceUpdatedV3(ctx, state, attr)
+	case eth.FCUV2:
+		return o.api.ForkchoiceUpdatedV2(ctx, state, attr)
+	case eth.FCUV1:
+		return o.api.ForkchoiceUpdatedV1(ctx, state, attr)
+	default:
+		return nil, fmt.Errorf("unsupported ForkchoiceUpdated version: %s", method)
+	}
 }
 
 func (o *OracleEngine) NewPayload(ctx context.Context, payload *eth.ExecutionPayload, parentBeaconBlockRoot *common.Hash) (*eth.PayloadStatusV1, error) {
-	if o.rollupCfg.IsEcotone(uint64(payload.Timestamp)) {
+	switch method := o.rollupCfg.NewPayloadVersion(uint64(payload.Timestamp)); method {
+	case eth.NewPayloadV3:
 		return o.api.NewPayloadV3(ctx, payload, []common.Hash{}, parentBeaconBlockRoot)
-	} else {
+	case eth.NewPayloadV2:
 		return o.api.NewPayloadV2(ctx, payload)
+	default:
+		return nil, fmt.Errorf("unsupported NewPayload version: %s", method)
 	}
 }
 

op-service/eth/types.go

@@ -153,6 +153,10 @@ type Uint256Quantity = uint256.Int
 type Data = hexutil.Bytes
 
 type PayloadID = engine.PayloadID
+type PayloadInfo struct {
+	ID        PayloadID
+	Timestamp uint64
+}
 
 type ExecutionPayloadEnvelope struct {
 	ParentBeaconBlockRoot *common.Hash      `json:"parentBeaconBlockRoot,omitempty"`
@@ -454,3 +458,17 @@ func (v *Uint64String) UnmarshalText(b []byte) error {
 	*v = Uint64String(n)
 	return nil
 }
+
+type EngineAPIMethod string
+
+const (
+	FCUV1 EngineAPIMethod = "engine_forkchoiceUpdatedV1"
+	FCUV2 EngineAPIMethod = "engine_forkchoiceUpdatedV2"
+	FCUV3 EngineAPIMethod = "engine_forkchoiceUpdatedV3"
+
+	NewPayloadV2 EngineAPIMethod = "engine_newPayloadV2"
+	NewPayloadV3 EngineAPIMethod = "engine_newPayloadV3"
+
+	GetPayloadV2 EngineAPIMethod = "engine_getPayloadV2"
+	GetPayloadV3 EngineAPIMethod = "engine_getPayloadV3"
+)

op-service/sources/engine_client.go

@@ -58,7 +58,8 @@ func (s *EngineClient) ForkchoiceUpdate(ctx context.Context, fc *eth.ForkchoiceS
 	fcCtx, cancel := context.WithTimeout(ctx, time.Second*5)
 	defer cancel()
 	var result eth.ForkchoiceUpdatedResult
-	err := s.client.CallContext(fcCtx, &result, "engine_forkchoiceUpdatedV3", fc, attributes)
+	method := s.rollupCfg.ForkchoiceUpdatedVersion(attributes)
+	err := s.client.CallContext(fcCtx, &result, string(method), fc, attributes)
 	if err == nil {
 		tlog.Trace("Shared forkchoice-updated signal")
 		if attributes != nil { // block building is optional, we only get a payload ID if we are building a block
@@ -95,10 +96,13 @@ func (s *EngineClient) NewPayload(ctx context.Context, payload *eth.ExecutionPay
 	var result eth.PayloadStatusV1
 
 	var err error
-	if s.rollupCfg.IsEcotone(uint64(payload.Timestamp)) {
-		err = s.client.CallContext(execCtx, &result, "engine_newPayloadV3", payload, []common.Hash{}, parentBeaconBlockRoot)
-	} else {
-		err = s.client.CallContext(execCtx, &result, "engine_newPayloadV2", payload)
+	switch method := s.rollupCfg.NewPayloadVersion(uint64(payload.Timestamp)); method {
+	case eth.NewPayloadV3:
+		err = s.client.CallContext(execCtx, &result, string(method), payload, []common.Hash{}, parentBeaconBlockRoot)
+	case eth.NewPayloadV2:
+		err = s.client.CallContext(execCtx, &result, string(method), payload)
+	default:
+		return nil, fmt.Errorf("unsupported NewPayload version: %s", method)
 	}
 
 	e.Trace("Received payload execution result", "status", result.Status, "latestValidHash", result.LatestValidHash, "message", result.ValidationError)
@@ -113,13 +117,14 @@ func (s *EngineClient) NewPayload(ctx context.Context, payload *eth.ExecutionPay
 // There may be two types of error:
 // 1. `error` as eth.InputError: the payload ID may be unknown
 // 2. Other types of `error`: temporary RPC errors, like timeouts.
-func (s *EngineClient) GetPayload(ctx context.Context, payloadId eth.PayloadID) (*eth.ExecutionPayloadEnvelope, error) {
-	e := s.log.New("payload_id", payloadId)
+func (s *EngineClient) GetPayload(ctx context.Context, payloadInfo eth.PayloadInfo) (*eth.ExecutionPayloadEnvelope, error) {
+	e := s.log.New("payload_id", payloadInfo.ID)
 	e.Trace("getting payload")
 	var result eth.ExecutionPayloadEnvelope
-	err := s.client.CallContext(ctx, &result, "engine_getPayloadV3", payloadId)
+	method := s.rollupCfg.GetPayloadVersion(payloadInfo.Timestamp)
+	err := s.client.CallContext(ctx, &result, string(method), payloadInfo.ID)
 	if err != nil {
-		e.Warn("Failed to get payload", "payload_id", payloadId, "err", err)
+		e.Warn("Failed to get payload", "payload_id", payloadInfo.ID, "err", err)
 		if rpcErr, ok := err.(rpc.Error); ok {
 			code := eth.ErrorCode(rpcErr.ErrorCode())
 			switch code {

op-service/testutils/mock_engine.go

@@ -12,8 +12,8 @@ type MockEngine struct {
 	MockL2Client
 }
 
-func (m *MockEngine) GetPayload(ctx context.Context, payloadId eth.PayloadID) (*eth.ExecutionPayloadEnvelope, error) {
-	out := m.Mock.Called(payloadId)
+func (m *MockEngine) GetPayload(ctx context.Context, payloadInfo eth.PayloadInfo) (*eth.ExecutionPayloadEnvelope, error) {
+	out := m.Mock.Called(payloadInfo.ID)
 	return out.Get(0).(*eth.ExecutionPayloadEnvelope), out.Error(1)
 }