diff --git a/turbo/engineapi/engine_server.go b/turbo/engineapi/engine_server.go index 0af5be5fb30..d84d5550e59 100644 --- a/turbo/engineapi/engine_server.go +++ b/turbo/engineapi/engine_server.go @@ -5,7 +5,6 @@ import ( "context" "errors" "fmt" - "github.com/erigontech/erigon/consensus/misc" "github.com/erigontech/erigon/eth/ethconfig" "math/big" "sync" @@ -165,10 +164,9 @@ func (s *EngineServer) newPayload(ctx context.Context, req *engine_types.Executi TxHash: types.DeriveSha(types.BinaryTransactions(txs)), } - // Payload must have eip-1559 params in ExtraData after Holocene - if s.config.IsHolocene(req.Timestamp.Uint64()) { - if err := misc.ValidateHoloceneExtraData(req.ExtraData); err != nil { - return nil, &rpc.InvalidParamsError{Message: "holocene payloads must have eip-1559 params, got none"} + if s.config.IsOptimism() { + if err := checkOptimismPayload(req, s.config); err != nil { + return nil, &rpc.InvalidParamsError{Message: err.Error()} } } @@ -596,16 +594,12 @@ func (s *EngineServer) forkchoiceUpdated(ctx context.Context, forkchoiceState *e } if s.config.Optimism != nil { - if payloadAttributes.GasLimit == nil { + if err := checkOptimismPayloadAttributes(payloadAttributes, s.config); err != nil { return nil, &engine_helpers.InvalidPayloadAttributesErr } + if s.config.IsHolocene(payloadAttributes.Timestamp.Uint64()) { - if err := misc.ValidateHolocene1559Params(payloadAttributes.EIP1559Params); err != nil { - return nil, err - } req.Eip_1559Params = bytes.Clone(payloadAttributes.EIP1559Params) - } else if len(payloadAttributes.EIP1559Params) != 0 { - return nil, &engine_helpers.InvalidPayloadAttributesErr } } diff --git a/turbo/engineapi/engine_server_optimism.go b/turbo/engineapi/engine_server_optimism.go new file mode 100644 index 00000000000..b1e59ec953f --- /dev/null +++ b/turbo/engineapi/engine_server_optimism.go @@ -0,0 +1,68 @@ +package engineapi + +import ( + "errors" + "github.com/erigontech/erigon-lib/chain" + "github.com/erigontech/erigon/consensus/misc" + "github.com/erigontech/erigon/turbo/engineapi/engine_types" +) + +// checkOptimismPayload performs Optimism-specific checks on the payload data (called during [(*EngineServer).newPayload]). +func checkOptimismPayload(params *engine_types.ExecutionPayload, cfg *chain.Config) error { + // (non)-nil withdrawals is already checked by Shanghai rules. + // Canyon - empty withdrawals + if cfg.IsCanyon(params.Timestamp.Uint64()) { + if len(params.Withdrawals) != 0 { + return errors.New("non-empty withdrawals post-Canyon") + } + } + + // Holocene - extraData + if cfg.IsHolocene(params.Timestamp.Uint64()) { + if err := misc.ValidateHoloceneExtraData(params.ExtraData); err != nil { + return err + } + } else if len(params.ExtraData) > 0 { // pre-Holocene + return errors.New("extraData must be empty before Holocene") + } + + // Isthmus - withdrawalsRoot + if cfg.IsIsthmus(params.Timestamp.Uint64()) { + if params.WithdrawalsRoot == nil { + return errors.New("nil withdrawalsRoot post-Isthmus") + } + } else if params.WithdrawalsRoot != nil { // pre-Isthmus + return errors.New("non-nil withdrawalsRoot pre-Isthmus") + } + + return nil +} + +// checkOptimismPayloadAttributes performs Optimism-specific checks on the payload attributes (called during [(*EngineServer).forkchoiceUpdated]. +// Will panic if payloadAttributes is nil. +func checkOptimismPayloadAttributes(payloadAttributes *engine_types.PayloadAttributes, cfg *chain.Config) error { + if payloadAttributes.GasLimit == nil { + return errors.New("gasLimit parameter is required") + } + + // (non)-nil withdrawals is already checked by Shanghai rules. + // Canyon - empty withdrawals + if cfg.IsCanyon(payloadAttributes.Timestamp.Uint64()) { + if len(payloadAttributes.Withdrawals) != 0 { + return errors.New("non-empty withdrawals post-Canyon") + } + } + + // Holocene - extraData + if cfg.IsHolocene(payloadAttributes.Timestamp.Uint64()) { + if err := misc.ValidateHolocene1559Params(payloadAttributes.EIP1559Params); err != nil { + return err + } + } else if len(payloadAttributes.EIP1559Params) != 0 { // pre-Holocene + return errors.New("non-empty eip155Params pre-Holocene") + } + + // Note: PayloadAttributes don't contain the Isthmus withdrawalsRoot, it's set during block assembly. + + return nil +} diff --git a/turbo/engineapi/engine_server_optimism_test.go b/turbo/engineapi/engine_server_optimism_test.go new file mode 100644 index 00000000000..4486c816d21 --- /dev/null +++ b/turbo/engineapi/engine_server_optimism_test.go @@ -0,0 +1,242 @@ +package engineapi + +import ( + "errors" + "github.com/erigontech/erigon-lib/chain" + "github.com/erigontech/erigon-lib/common/hexutil" + "github.com/erigontech/erigon/core/types" + "github.com/erigontech/erigon/turbo/engineapi/engine_types" + "math/big" + "testing" + + "github.com/stretchr/testify/require" +) + +func preCanyon() *chain.Config { + cfg := new(chain.Config) + // Mark as an Optimism chain so IsOptimismFoo is true when FooTime is active + cfg.Optimism = &chain.OptimismConfig{} + return cfg +} + +func postCanyon() *chain.Config { + cfg := preCanyon() + cfg.CanyonTime = new(big.Int) + return cfg +} + +func postHolocene() *chain.Config { + cfg := postCanyon() + cfg.HoloceneTime = new(big.Int) + return cfg +} + +func postIsthmus() *chain.Config { + cfg := postHolocene() + cfg.IsthmusTime = new(big.Int) + return cfg +} + +var valid1559Params = []byte{0, 1, 2, 3, 4, 5, 6, 7} +var validExtraData = []byte{0, 1, 2, 3, 4, 5, 6, 7, 8} +var emptyWithdrawals = make([]*types.Withdrawal, 0) +var validJovianExtraData = append(append([]byte{1}, valid1559Params...), make([]byte, 8)...) // version=1, 8 bytes params, 8 byte minBaseFee + +func TestCheckOptimismPayload(t *testing.T) { + tests := []struct { + name string + params engine_types.ExecutionPayload + cfg *chain.Config + expected error + }{ + { + name: "valid payload pre-Canyon", + params: engine_types.ExecutionPayload{ + ExtraData: []byte{}, + }, + cfg: preCanyon(), + expected: nil, + }, + { + name: "valid payload post-Canyon", + params: engine_types.ExecutionPayload{ + ExtraData: []byte{}, + Withdrawals: emptyWithdrawals, + }, + cfg: postCanyon(), + }, + { + name: "invalid empty withdrawals post-Canyon", + params: engine_types.ExecutionPayload{ + ExtraData: []byte{}, + Withdrawals: make([]*types.Withdrawal, 1), + }, + cfg: postCanyon(), + expected: errors.New("non-empty withdrawals post-Canyon"), + }, + { + name: "non-nil withdrawalsRoot pre-Isthmus", + params: engine_types.ExecutionPayload{ + ExtraData: []byte{}, + Withdrawals: emptyWithdrawals, + WithdrawalsRoot: &types.EmptyRootHash, + }, + cfg: postCanyon(), + expected: errors.New("non-nil withdrawalsRoot pre-Isthmus"), + }, + { + name: "invalid non-empty extraData pre-Holocene", + params: engine_types.ExecutionPayload{ + ExtraData: []byte{1, 2, 3}, + Withdrawals: emptyWithdrawals, + }, + cfg: postCanyon(), + expected: errors.New("extraData must be empty before Holocene"), + }, + { + name: "invalid extraData post-Holocene", + params: engine_types.ExecutionPayload{ + ExtraData: []byte{1, 2, 3}, + Withdrawals: emptyWithdrawals, + }, + cfg: postHolocene(), + expected: errors.New("holocene extraData should be 9 bytes, got 3"), + }, + { + name: "valid payload post-Holocene with extraData", + params: engine_types.ExecutionPayload{ + ExtraData: validExtraData, + Withdrawals: emptyWithdrawals, + }, + cfg: postHolocene(), + expected: nil, + }, + { + name: "invalid non-nil withdrawalsRoot post-Holocene", + params: engine_types.ExecutionPayload{ + ExtraData: validExtraData, + Withdrawals: emptyWithdrawals, + WithdrawalsRoot: &types.EmptyRootHash, + }, + cfg: postHolocene(), + expected: errors.New("non-nil withdrawalsRoot pre-Isthmus"), + }, + { + name: "valid payload post-Isthmus", + params: engine_types.ExecutionPayload{ + ExtraData: validExtraData, + Withdrawals: emptyWithdrawals, + WithdrawalsRoot: &types.EmptyRootHash, + }, + cfg: postIsthmus(), + expected: nil, + }, + { + name: "invalid nil withdrawals root post-isthmus", + params: engine_types.ExecutionPayload{ + Withdrawals: emptyWithdrawals, + ExtraData: validExtraData, + }, + cfg: postIsthmus(), + expected: errors.New("nil withdrawalsRoot post-Isthmus"), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := checkOptimismPayload(&test.params, test.cfg) + if test.expected == nil { + require.NoError(t, err) + } else { + require.EqualError(t, err, test.expected.Error()) + } + }) + } +} + +func TestCheckOptimismPayloadAttributes(t *testing.T) { + tests := []struct { + name string + payloadAttributes *engine_types.PayloadAttributes + cfg *chain.Config + expected error + shouldPanic bool + }{ + { + name: "nil payload attributes panic", + payloadAttributes: nil, + cfg: preCanyon(), + shouldPanic: true, + }, + { + name: "valid payload attributes pre-Canyon", + payloadAttributes: &engine_types.PayloadAttributes{ + GasLimit: new(hexutil.Uint64), + }, + cfg: preCanyon(), + expected: nil, + }, + { + name: "invalid nil gasLimit", + payloadAttributes: &engine_types.PayloadAttributes{ + GasLimit: nil, + }, + cfg: preCanyon(), + expected: errors.New("gasLimit parameter is required"), + }, + { + name: "invalid non-empty withdrawals post-Canyon", + payloadAttributes: &engine_types.PayloadAttributes{ + GasLimit: new(hexutil.Uint64), + EIP1559Params: valid1559Params, + Withdrawals: make([]*types.Withdrawal, 1), + }, + cfg: postCanyon(), + expected: errors.New("non-empty withdrawals post-Canyon"), + }, + { + name: "invalid non-empty eip1559Params pre-Holocene", + payloadAttributes: &engine_types.PayloadAttributes{ + GasLimit: new(hexutil.Uint64), + EIP1559Params: valid1559Params, + }, + cfg: postCanyon(), + expected: errors.New("non-empty eip155Params pre-Holocene"), + }, + { + name: "invalid eip1559Params post-Holocene", + payloadAttributes: &engine_types.PayloadAttributes{ + GasLimit: new(hexutil.Uint64), + EIP1559Params: append(valid1559Params, 77), + }, + cfg: postHolocene(), + expected: errors.New("holocene eip-1559 params should be 8 bytes, got 9"), + }, + { + name: "valid payload attributes post-Holocene", + payloadAttributes: &engine_types.PayloadAttributes{ + GasLimit: new(hexutil.Uint64), + EIP1559Params: valid1559Params, + }, + cfg: postHolocene(), + expected: nil, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if test.shouldPanic { + require.Panics(t, func() { + checkOptimismPayloadAttributes(test.payloadAttributes, test.cfg) + }) + } else { + err := checkOptimismPayloadAttributes(test.payloadAttributes, test.cfg) + if test.expected == nil { + require.NoError(t, err) + } else { + require.EqualError(t, err, test.expected.Error()) + } + } + }) + } +} diff --git a/turbo/engineapi/engine_types/jsonrpc.go b/turbo/engineapi/engine_types/jsonrpc.go index 09dd7d1d81f..d0816046738 100644 --- a/turbo/engineapi/engine_types/jsonrpc.go +++ b/turbo/engineapi/engine_types/jsonrpc.go @@ -222,7 +222,9 @@ func ConvertPayloadFromRpc(payload *types2.ExecutionPayload) *ExecutionPayload { } if payload.WithdrawlsRoot != nil { root := common.Hash(gointerfaces.ConvertH256ToHash(payload.WithdrawlsRoot)) - res.WithdrawalsRoot = &root + if root != types.EmptyRootHash { + res.WithdrawalsRoot = &root + } } return res }