simulators/ethereum/engine: Fix NewPayloadV3 tests

ethereum · Jun 30, 2023 · fc2f040 · fc2f040
1 parent 2baaf11
commit fc2f040
Show file tree

Hide file tree

Showing 3 changed files with 202 additions and 36 deletions.
diff --git a/simulators/ethereum/engine/client/hive_rpc/hive_rpc.go b/simulators/ethereum/engine/client/hive_rpc/hive_rpc.go
@@ -453,7 +453,7 @@ func (ec *HiveRPCEngineClient) NewPayload(ctx context.Context, version int, payl
 		return result, err
 	}
 
-	if versionedHashes != nil {
+	if version >= 3 {
 		err = ec.c.CallContext(ctx, &result, fmt.Sprintf("engine_newPayloadV%d", version), payload, versionedHashes)
 	} else {
 		err = ec.c.CallContext(ctx, &result, fmt.Sprintf("engine_newPayloadV%d", version), payload)

diff --git a/simulators/ethereum/engine/suites/blobs/steps.go b/simulators/ethereum/engine/suites/blobs/steps.go
@@ -253,11 +253,10 @@ func GetBlobDataInPayload(pool *TestBlobTxPool, payload *engine.ExecutableData)
 	return blobTxsInPayload, blobDataInPayload, nil
 }
 
-func (step NewPayloads) VerifyPayload(ctx context.Context, testEngine *test.TestEngineClient, blobTxsInPayload []*typ.TransactionWithBlobData, payload *engine.ExecutableData, previousPayload *engine.ExecutableData) error {
+func (step NewPayloads) VerifyPayload(ctx context.Context, forkConfig *globals.ForkConfig, testEngine *test.TestEngineClient, blobTxsInPayload []*typ.TransactionWithBlobData, payload *engine.ExecutableData, previousPayload *engine.ExecutableData) error {
 	var (
 		parentExcessDataGas = uint64(0)
 		parentDataGasUsed   = uint64(0)
-		isCancunYet         = true
 	)
 	if previousPayload != nil {
 		if previousPayload.ExcessDataGas != nil {
@@ -270,7 +269,7 @@ func (step NewPayloads) VerifyPayload(ctx context.Context, testEngine *test.Test
 	expectedExcessDataGas := CalcExcessDataGas(parentExcessDataGas, parentDataGasUsed)
 
 	// TODO: check whether the new payload should be in cancun or not
-	if isCancunYet {
+	if forkConfig.IsCancun(payload.Timestamp) {
 		if payload.ExcessDataGas == nil {
 			return fmt.Errorf("payload contains nil excessDataGas")
 		}
@@ -384,10 +383,7 @@ func (step NewPayloads) Execute(t *BlobTestContext) error {
 					payload    = &t.CLMock.LatestPayloadBuilt
 				)
 
-				if t.Env.Genesis.Config.CancunTime == nil {
-					panic("CancunTime is nil")
-				}
-				if payload.Timestamp < *t.Env.Genesis.Config.CancunTime {
+				if !t.Env.ForkConfig.IsCancun(payload.Timestamp) {
 					// Nothing to do
 					return
 				}
@@ -407,8 +403,9 @@ func (step NewPayloads) Execute(t *BlobTestContext) error {
 			OnNewPayloadBroadcast: func() {
 				// Send a test NewPayload directive with either a modified payload or modifed versioned hashes
 				var (
-					payload         *engine.ExecutableData
-					versionedHashes []common.Hash
+					payload                       = &t.CLMock.LatestPayloadBuilt
+					versionedHashes []common.Hash = nil
+					r               *test.NewPayloadResponseExpectObject
 					err             error
 				)
 				if step.VersionedHashes != nil {
@@ -417,10 +414,6 @@ func (step NewPayloads) Execute(t *BlobTestContext) error {
 					if err != nil {
 						t.Fatalf("FAIL: Error getting modified versioned hashes (payload %d/%d): %v", p+1, payloadCount, err)
 					}
-					r := t.TestEngine.TestEngineNewPayloadV3(&t.CLMock.LatestPayloadBuilt, versionedHashes)
-					// All tests that modify the versioned hashes expect an
-					// `INVALID` response, even if the client is out of sync
-					r.ExpectStatus(test.Invalid)
 				} else {
 					if t.CLMock.LatestBlobBundle != nil {
 						versionedHashes, err = t.CLMock.LatestBlobBundle.VersionedHashes(BLOB_COMMITMENT_VERSION_KZG)
@@ -432,19 +425,24 @@ func (step NewPayloads) Execute(t *BlobTestContext) error {
 
 				if step.PayloadCustomizer != nil {
 					// Send a custom new payload
-					payload, err = step.PayloadCustomizer.CustomizePayload(&t.CLMock.LatestPayloadBuilt)
+					payload, err = step.PayloadCustomizer.CustomizePayload(payload)
 					if err != nil {
 						t.Fatalf("FAIL: Error customizing payload (payload %d/%d): %v", p+1, payloadCount, err)
 					}
-				} else {
-					payload = &t.CLMock.LatestPayloadBuilt
 				}
 
-				var r *test.NewPayloadResponseExpectObject
+				version := step.Version
+				if version == 0 {
+					if t.Env.ForkConfig.IsCancun(payload.Timestamp) {
+						version = 3
+					} else {
+						version = 2
+					}
+				}
 
-				if step.Version == 0 || step.Version == 3 {
+				if version == 3 {
 					r = t.TestEngine.TestEngineNewPayloadV3(payload, versionedHashes)
-				} else if step.Version == 2 {
+				} else if version == 2 {
 					r = t.TestEngine.TestEngineNewPayloadV2(payload)
 				} else {
 					t.Fatalf("FAIL: Unknown version %d", step.Version)
@@ -466,7 +464,7 @@ func (step NewPayloads) Execute(t *BlobTestContext) error {
 				if err != nil {
 					t.Fatalf("FAIL: Error retrieving blob bundle (payload %d/%d): %v", p+1, payloadCount, err)
 				}
-				if err := step.VerifyPayload(t.TimeoutContext, t.TestEngine, blobTxsInPayload, payload, &previousPayload); err != nil {
+				if err := step.VerifyPayload(t.TimeoutContext, t.Env.ForkConfig, t.TestEngine, blobTxsInPayload, payload, &previousPayload); err != nil {
 					t.Fatalf("FAIL: Error verifying payload (payload %d/%d): %v", p+1, payloadCount, err)
 				}
 				previousPayload = t.CLMock.LatestPayloadBuilt

diff --git a/simulators/ethereum/engine/suites/blobs/tests.go b/simulators/ethereum/engine/suites/blobs/tests.go
@@ -27,6 +27,10 @@ var (
 	DATA_GASPRICE_UPDATE_FRACTION = uint64(3338477)
 
 	BLOB_COMMITMENT_VERSION_KZG = byte(0x01)
+
+	// Engine API errors
+	INVALID_PARAMS_ERROR   = pInt(-32602)
+	UNSUPPORTED_FORK_ERROR = pInt(-38005)
 )
 
 // Precalculate the first data gas cost increase
@@ -510,48 +514,212 @@ var Tests = []test.SpecInterface{
 			},
 		},
 	},
-	// NewPayload Version Negative Tests
+
+	// NewPayloadV3 Before Cancun, Negative Tests
 	&BlobsBaseSpec{
+		Spec: test.Spec{
+			Name: "NewPayloadV3 Before Cancun, Nil Data Fields, Nil Versioned Hashes",
+			About: `
+			Test sending NewPayloadV3 Before Cancun with:
+			- nil ExcessDataGas
+			- nil DataGasUsed
+			- nil Versioned Hashes Array
+			`,
+		},
 
+		CancunForkHeight: 2,
+
+		TestSequence: TestSequence{
+			NewPayloads{
+				ExpectedIncludedBlobCount: 0,
+				Version:                   3,
+				VersionedHashes: &VersionedHashes{
+					Blobs: nil,
+				},
+				// On DEVNET 8:
+				// ExpectedError:             INVALID_PARAMS_ERROR,
+			},
+		},
+	},
+	&BlobsBaseSpec{
 		Spec: test.Spec{
-			Name: "NewPayloadV2 After Cancun",
+			Name: "NewPayloadV3 Before Cancun, Nil ExcessDataGas, 0x00 DataGasUsed, Nil Versioned Hashes",
 			About: `
-			Test sending NewPayloadV2 after cancun fork, which
-			should result in error.
+			Test sending NewPayloadV3 Before Cancun with:
+			- nil ExcessDataGas
+			- 0x00 DataGasUsed
+			- nil Versioned Hashes Array
 			`,
 		},
 
-		// We fork on genesis
-		CancunForkHeight: 0,
+		CancunForkHeight: 2,
 
 		TestSequence: TestSequence{
-			// Send multiple blob transactions with the same nonce.
 			NewPayloads{
 				ExpectedIncludedBlobCount: 0,
-				Version:                   2,
-				ExpectedError:             pInt(-38005),
+				Version:                   3,
+				PayloadCustomizer: &helper.CustomPayloadData{
+					DataGasUsed: pUint64(0),
+				},
+				ExpectedError: INVALID_PARAMS_ERROR,
 			},
 		},
 	},
 	&BlobsBaseSpec{
+		Spec: test.Spec{
+			Name: "NewPayloadV3 Before Cancun, 0x00 ExcessDataGas, Nil DataGasUsed, Nil Versioned Hashes",
+			About: `
+			Test sending NewPayloadV3 Before Cancun with:
+			- 0x00 ExcessDataGas
+			- nil DataGasUsed
+			- nil Versioned Hashes Array
+			`,
+		},
+
+		CancunForkHeight: 2,
 
+		TestSequence: TestSequence{
+			NewPayloads{
+				ExpectedIncludedBlobCount: 0,
+				Version:                   3,
+				PayloadCustomizer: &helper.CustomPayloadData{
+					ExcessDataGas: pUint64(0),
+				},
+				ExpectedError: INVALID_PARAMS_ERROR,
+			},
+		},
+	},
+	/*
+		// Unspecified Outcome For Devnet 7, uncomment for Devnet 8
+		&BlobsBaseSpec{
+			Spec: test.Spec{
+				Name: "NewPayloadV3 Before Cancun, Nil Data Fields, Empty Array Versioned Hashes",
+				About: `
+				Test sending NewPayloadV3 Before Cancun with:
+				- nil ExcessDataGas
+				- nil DataGasUsed
+				- Empty Versioned Hashes Array
+				`,
+			},
+
+			CancunForkHeight: 2,
+
+			TestSequence: TestSequence{
+				NewPayloads{
+					ExpectedIncludedBlobCount: 0,
+					Version:                   3,
+					VersionedHashes: &VersionedHashes{
+						Blobs: []helper.BlobID{},
+					},
+					ExpectedError: INVALID_PARAMS_ERROR,
+				},
+			},
+		},
+	*/
+	&BlobsBaseSpec{
 		Spec: test.Spec{
-			Name: "NewPayloadV3 Before Cancun",
+			Name: "NewPayloadV3 Before Cancun, 0x00 Data Fields, Empty Array Versioned Hashes",
 			About: `
-			Test sending NewPayloadV3 before cancun fork, which
-			should result in error.
+			Test sending NewPayloadV3 Before Cancun with:
+			- 0x00 ExcessDataGas
+			- 0x00 DataGasUsed
+			- Empty Versioned Hashes Array
 			`,
 		},
 
-		// We fork on genesis
 		CancunForkHeight: 2,
 
 		TestSequence: TestSequence{
-			// Send multiple blob transactions with the same nonce.
 			NewPayloads{
 				ExpectedIncludedBlobCount: 0,
 				Version:                   3,
-				ExpectedError:             pInt(-38005),
+				VersionedHashes: &VersionedHashes{
+					Blobs: []helper.BlobID{},
+				},
+				PayloadCustomizer: &helper.CustomPayloadData{
+					ExcessDataGas: pUint64(0),
+					DataGasUsed:   pUint64(0),
+				},
+				ExpectedError: INVALID_PARAMS_ERROR,
+				// On DEVNET 8:
+				// ExpectedError: UNSUPPORTED_FORK_ERROR,
+			},
+		},
+	},
+
+	// NewPayloadV3 After Cancun, Negative Tests
+	/*
+		// Unspecified Outcome For Devnet 7, uncomment for Devnet 8
+		&BlobsBaseSpec{
+			Spec: test.Spec{
+				Name: "NewPayloadV3 After Cancun, 0x00 Data Fields, Nil Versioned Hashes",
+				About: `
+				Test sending NewPayloadV3 After Cancun with:
+				- 0x00 ExcessDataGas
+				- 0x00 DataGasUsed
+				- nil Versioned Hashes Array
+				`,
+			},
+
+			CancunForkHeight: 1,
+
+			TestSequence: TestSequence{
+				NewPayloads{
+					ExpectedIncludedBlobCount: 0,
+					Version:                   3,
+					VersionedHashes: &VersionedHashes{
+						Blobs: nil,
+					},
+					ExpectedError: INVALID_PARAMS_ERROR,
+				},
+			},
+		},
+	*/
+	&BlobsBaseSpec{
+		Spec: test.Spec{
+			Name: "NewPayloadV3 After Cancun, Nil ExcessDataGas, 0x00 DataGasUsed, Empty Array Versioned Hashes",
+			About: `
+			Test sending NewPayloadV3 After Cancun with:
+			- nil ExcessDataGas
+			- 0x00 DataGasUsed
+			- Empty Versioned Hashes Array
+			`,
+		},
+
+		CancunForkHeight: 1,
+
+		TestSequence: TestSequence{
+			NewPayloads{
+				ExpectedIncludedBlobCount: 0,
+				Version:                   3,
+				PayloadCustomizer: &helper.CustomPayloadData{
+					RemoveExcessDataGas: true,
+				},
+				ExpectedError: INVALID_PARAMS_ERROR,
+			},
+		},
+	},
+	&BlobsBaseSpec{
+		Spec: test.Spec{
+			Name: "NewPayloadV3 After Cancun, 0x00 ExcessDataGas, Nil DataGasUsed, Empty Array Versioned Hashes",
+			About: `
+			Test sending NewPayloadV3 After Cancun with:
+			- 0x00 ExcessDataGas
+			- nil DataGasUsed
+			- Empty Versioned Hashes Array
+			`,
+		},
+
+		CancunForkHeight: 1,
+
+		TestSequence: TestSequence{
+			NewPayloads{
+				ExpectedIncludedBlobCount: 0,
+				Version:                   3,
+				PayloadCustomizer: &helper.CustomPayloadData{
+					RemoveDataGasUsed: true,
+				},
+				ExpectedError: INVALID_PARAMS_ERROR,
 			},
 		},
 	},