simulators/ethereum/engine: Fixes for execution-apis#498 (ethereum#974)

* simulators/ethereum/engine: Fix expected error on fcu tests

* simulators/ethereum/engine: Get Payload delay at spec level

* simulators/ethereum/engine: Correct error code on inconsistent FcU

* simulators/ethereum/engine: Invaild Payload Attributes Expectation Modification

Author: marioevz

simulators/ethereum/engine/suites/cancun/tests.go

@@ -531,7 +531,7 @@ var Tests = []test.Spec{
 			- Payload Attributes uses Shanghai timestamp
 			- Payload Attributes Beacon Root is null
 
-			Verify that client returns INVALID_PARAMS_ERROR.
+			Verify that client returns INVALID_PAYLOAD_ATTRIBUTES.
 			`,
 			MainFork:   config.Cancun,
 			ForkHeight: 2,
@@ -541,12 +541,12 @@ var Tests = []test.Spec{
 			NewPayloads{
 				FcUOnPayloadRequest: &helper.UpgradeForkchoiceUpdatedVersion{
 					ForkchoiceUpdatedCustomizer: &helper.BaseForkchoiceUpdatedCustomizer{
-						ExpectedError: globals.INVALID_PARAMS_ERROR,
+						ExpectedError: globals.INVALID_PAYLOAD_ATTRIBUTES,
 					},
 				},
 				ExpectationDescription: fmt.Sprintf(`
-				ForkchoiceUpdatedV3 before Cancun with any null field must return INVALID_PARAMS_ERROR (code %d)
-				`, *globals.INVALID_PARAMS_ERROR),
+				ForkchoiceUpdatedV3 before Cancun with any null field must return INVALID_PAYLOAD_ATTRIBUTES (code %d)
+				`, *globals.INVALID_PAYLOAD_ATTRIBUTES),
 			},
 		},
 	},
@@ -585,13 +585,13 @@ var Tests = []test.Spec{
 	// ForkchoiceUpdatedV2 before cancun with beacon root
 	&CancunBaseSpec{
 		BaseSpec: test.BaseSpec{
-			Name: "ForkchoiceUpdatedV2 To Request Shanghai Payload, Non-Null Beacon Root ",
+			Name: "ForkchoiceUpdatedV2 To Request Shanghai Payload, Non-Null Beacon Root",
 			About: `
 			Test sending ForkchoiceUpdatedV2 to request a Shanghai payload:
 			- Payload Attributes uses Shanghai timestamp
 			- Payload Attributes Beacon Root is non-null
 
-			Verify that client returns INVALID_PARAMS_ERROR.
+			Verify that client returns INVALID_PAYLOAD_ATTRIBUTES.
 			`,
 			MainFork:   config.Cancun,
 			ForkHeight: 2,
@@ -603,11 +603,11 @@ var Tests = []test.Spec{
 					PayloadAttributesCustomizer: &helper.BasePayloadAttributesCustomizer{
 						BeaconRoot: &(common.Hash{}),
 					},
-					ExpectedError: globals.INVALID_PARAMS_ERROR,
+					ExpectedError: globals.INVALID_PAYLOAD_ATTRIBUTES,
 				},
 				ExpectationDescription: fmt.Sprintf(`
-				ForkchoiceUpdatedV2 before Cancun with beacon root field must return INVALID_PARAMS_ERROR (code %d)
-				`, *globals.INVALID_PARAMS_ERROR),
+				ForkchoiceUpdatedV2 before Cancun with beacon root field must return INVALID_PAYLOAD_ATTRIBUTES (code %d)
+				`, *globals.INVALID_PAYLOAD_ATTRIBUTES),
 			},
 		},
 	},
@@ -621,7 +621,7 @@ var Tests = []test.Spec{
 			- Payload Attributes uses Cancun timestamp
 			- Payload Attributes Beacon Root is non-null
 
-			Verify that client returns INVALID_PARAMS_ERROR.
+			Verify that client returns INVALID_PAYLOAD_ATTRIBUTES.
 			`,
 			MainFork:   config.Cancun,
 			ForkHeight: 1,
@@ -634,12 +634,12 @@ var Tests = []test.Spec{
 						PayloadAttributesCustomizer: &helper.BasePayloadAttributesCustomizer{
 							BeaconRoot: &(common.Hash{}),
 						},
-						ExpectedError: globals.INVALID_PARAMS_ERROR,
+						ExpectedError: globals.INVALID_PAYLOAD_ATTRIBUTES,
 					},
 				},
 				ExpectationDescription: fmt.Sprintf(`
-				ForkchoiceUpdatedV2 after Cancun with beacon root field must return INVALID_PARAMS_ERROR (code %d)
-				`, *globals.INVALID_PARAMS_ERROR),
+				ForkchoiceUpdatedV2 after Cancun with beacon root field must return INVALID_PAYLOAD_ATTRIBUTES (code %d)
+				`, *globals.INVALID_PAYLOAD_ATTRIBUTES),
 			},
 		},
 	},
@@ -1813,8 +1813,6 @@ func init() {
 			Customizer: &helper.BasePayloadAttributesCustomizer{
 				RemoveBeaconRoot: true,
 			},
-			// Error is expected on syncing because V3 checks all fields to be present
-			ErrorOnSync: true,
 		},
 	} {
 		Tests = append(Tests, t)

simulators/ethereum/engine/suites/engine/forkchoice.go

@@ -7,6 +7,7 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/hive/simulators/ethereum/engine/clmock"
 	"github.com/ethereum/hive/simulators/ethereum/engine/config"
+	"github.com/ethereum/hive/simulators/ethereum/engine/globals"
 	"github.com/ethereum/hive/simulators/ethereum/engine/helper"
 	"github.com/ethereum/hive/simulators/ethereum/engine/test"
 	typ "github.com/ethereum/hive/simulators/ethereum/engine/types"
@@ -79,7 +80,7 @@ func (tc InconsistentForkchoiceTest) Execute(t *test.Env) {
 		inconsistentFcU.FinalizedBlockHash = alternativePayloads[len(canonicalPayloads)-3].BlockHash
 	}
 	r := t.TestEngine.TestEngineForkchoiceUpdated(&inconsistentFcU, nil, t.CLMock.LatestPayloadBuilt.Timestamp)
-	r.ExpectError()
+	r.ExpectErrorCode(*globals.INVALID_FORKCHOICE_STATE)
 
 	// Return to the canonical chain
 	r = t.TestEngine.TestEngineForkchoiceUpdated(&t.CLMock.LatestForkchoice, nil, t.CLMock.LatestPayloadBuilt.Timestamp)

simulators/ethereum/engine/suites/engine/payload_attributes.go

@@ -14,7 +14,6 @@ type InvalidPayloadAttributesTest struct {
 	Description string
 	Customizer  helper.PayloadAttributesCustomizer
 	Syncing     bool
-	ErrorOnSync bool
 }
 
 func (s InvalidPayloadAttributesTest) WithMainFork(fork config.Fork) test.Spec {
@@ -69,12 +68,8 @@ func (tc InvalidPayloadAttributesTest) Execute(t *test.Env) {
 			if tc.Syncing {
 				// If we are SYNCING, the outcome should be SYNCING regardless of the validity of the payload atttributes
 				r := t.TestEngine.TestEngineForkchoiceUpdated(&fcu, attr, t.CLMock.LatestPayloadBuilt.Timestamp)
-				if tc.ErrorOnSync {
-					r.ExpectError()
-				} else {
-					r.ExpectPayloadStatus(test.Syncing)
-					r.ExpectPayloadID(nil)
-				}
+				r.ExpectPayloadStatus(test.Syncing)
+				r.ExpectPayloadID(nil)
 			} else {
 				r := t.TestEngine.TestEngineForkchoiceUpdated(&fcu, attr, t.CLMock.LatestPayloadBuilt.Timestamp)
 				r.ExpectError()

simulators/ethereum/engine/test/spec.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
+	"time"
 
 	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/hive/simulators/ethereum/engine/clmock"
@@ -60,6 +61,9 @@ type BaseSpec struct {
 	// CL Mocker configuration for time increments
 	BlockTimestampIncrement uint64
 
+	// CL Mocker configuration for payload delay in seconds
+	GetPayloadDelay uint64
+
 	// CL Mocker configuration for slots to `safe` and `finalized` respectively
 	SlotsToSafe      *big.Int
 	SlotsToFinalized *big.Int
@@ -109,6 +113,9 @@ func (s BaseSpec) ConfigureCLMock(cl *clmock.CLMocker) {
 		cl.SafeSlotsToImportOptimistically = s.SafeSlotsToImportOptimistically
 	}
 	cl.BlockTimestampIncrement = new(big.Int).SetUint64(s.GetBlockTimeIncrements())
+	if s.GetPayloadDelay != 0 {
+		cl.PayloadProductionClientDelay = time.Duration(s.GetPayloadDelay) * time.Second
+	}
 }
 
 func (s BaseSpec) GetAbout() string {