diff --git a/op-e2e/actions/l2_engine_test.go b/op-e2e/actions/l2_engine_test.go index c497f7f5a34c2..cd59763cd59a7 100644 --- a/op-e2e/actions/l2_engine_test.go +++ b/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") diff --git a/op-e2e/op_geth.go b/op-e2e/op_geth.go index aed070af1b590..3dbd0add55c1f 100644 --- a/op-e2e/op_geth.go +++ b/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 { diff --git a/op-e2e/op_geth_test.go b/op-e2e/op_geth_test.go index 9f69113675e70..03a1f8c4234e1 100644 --- a/op-e2e/op_geth_test.go +++ b/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 diff --git a/op-node/rollup/derive/engine_controller.go b/op-node/rollup/derive/engine_controller.go index 8ffb1f872b857..10112e2f1a795 100644 --- a/op-node/rollup/derive/engine_controller.go +++ b/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 diff --git a/op-node/rollup/derive/engine_update.go b/op-node/rollup/derive/engine_update.go index 5408ef495ec3c..b23a270040048 100644 --- a/op-node/rollup/derive/engine_update.go +++ b/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. diff --git a/op-node/rollup/types.go b/op-node/rollup/types.go index f47bf0a0fa907..3562fbcf74352 100644 --- a/op-node/rollup/types.go +++ b/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. diff --git a/op-node/rollup/types_test.go b/op-node/rollup/types_test.go index a5bbc0575e16c..17343430f262b 100644 --- a/op-node/rollup/types_test.go +++ b/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: ð.PayloadAttributes{Timestamp: 5}, + expectedMethod: eth.FCUV1, + }, + { + name: "Canyon", + canyonTime: 10, + ecotoneTime: 20, + attrs: ð.PayloadAttributes{Timestamp: 15}, + expectedMethod: eth.FCUV2, + }, + { + name: "Ecotone", + canyonTime: 10, + ecotoneTime: 20, + attrs: ð.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) + }) + } +} diff --git a/op-program/client/l2/engine.go b/op-program/client/l2/engine.go index 260561bccee38..d4cb64529a64b 100644 --- a/op-program/client/l2/engine.go +++ b/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) } } diff --git a/op-service/eth/types.go b/op-service/eth/types.go index bb7e4c6cdd14a..2c39e5d333e1b 100644 --- a/op-service/eth/types.go +++ b/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" +) diff --git a/op-service/sources/engine_client.go b/op-service/sources/engine_client.go index e80280a317b82..70c63c2375334 100644 --- a/op-service/sources/engine_client.go +++ b/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 { diff --git a/op-service/testutils/mock_engine.go b/op-service/testutils/mock_engine.go index b6bfa5d98f5b8..99dadf534ec40 100644 --- a/op-service/testutils/mock_engine.go +++ b/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) }