Skip to content

Commit 6e9a8b1

Browse files
marioevzmarioevz
committed
simulators/ethereum/engine: Fix NewPayloadV3 tests
1 parent 46a0c68 commit 6e9a8b1

File tree

3 files changed

+202
-36
lines changed

3 files changed

+202
-36
lines changed

‎simulators/ethereum/engine/client/hive_rpc/hive_rpc.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ func (ec *HiveRPCEngineClient) NewPayload(ctx context.Context, version int, payl
453453
return result, err
454454
}
455455

456-
if versionedHashes != nil {
456+
if version >= 3 {
457457
err = ec.c.CallContext(ctx, &result, fmt.Sprintf("engine_newPayloadV%d", version), payload, versionedHashes)
458458
} else {
459459
err = ec.c.CallContext(ctx, &result, fmt.Sprintf("engine_newPayloadV%d", version), payload)

‎simulators/ethereum/engine/suites/blobs/steps.go‎

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,10 @@ func GetBlobDataInPayload(pool *TestBlobTxPool, payload *engine.ExecutableData)
253253
return blobTxsInPayload, blobDataInPayload, nil
254254
}
255255

256-
func (step NewPayloads) VerifyPayload(ctx context.Context, testEngine *test.TestEngineClient, blobTxsInPayload []*typ.TransactionWithBlobData, payload *engine.ExecutableData, previousPayload *engine.ExecutableData) error {
256+
func (step NewPayloads) VerifyPayload(ctx context.Context, forkConfig *globals.ForkConfig, testEngine *test.TestEngineClient, blobTxsInPayload []*typ.TransactionWithBlobData, payload *engine.ExecutableData, previousPayload *engine.ExecutableData) error {
257257
var (
258258
parentExcessDataGas = uint64(0)
259259
parentDataGasUsed = uint64(0)
260-
isCancunYet = true
261260
)
262261
if previousPayload != nil {
263262
if previousPayload.ExcessDataGas != nil {
@@ -270,7 +269,7 @@ func (step NewPayloads) VerifyPayload(ctx context.Context, testEngine *test.Test
270269
expectedExcessDataGas := CalcExcessDataGas(parentExcessDataGas, parentDataGasUsed)
271270

272271
// TODO: check whether the new payload should be in cancun or not
273-
if isCancunYet {
272+
if forkConfig.IsCancun(payload.Timestamp) {
274273
if payload.ExcessDataGas == nil {
275274
return fmt.Errorf("payload contains nil excessDataGas")
276275
}
@@ -384,10 +383,7 @@ func (step NewPayloads) Execute(t *BlobTestContext) error {
384383
payload = &t.CLMock.LatestPayloadBuilt
385384
)
386385

387-
if t.Env.Genesis.Config.CancunTime == nil {
388-
panic("CancunTime is nil")
389-
}
390-
if payload.Timestamp < *t.Env.Genesis.Config.CancunTime {
386+
if !t.Env.ForkConfig.IsCancun(payload.Timestamp) {
391387
// Nothing to do
392388
return
393389
}
@@ -407,8 +403,9 @@ func (step NewPayloads) Execute(t *BlobTestContext) error {
407403
OnNewPayloadBroadcast: func() {
408404
// Send a test NewPayload directive with either a modified payload or modifed versioned hashes
409405
var (
410-
payload *engine.ExecutableData
411-
versionedHashes []common.Hash
406+
payload = &t.CLMock.LatestPayloadBuilt
407+
versionedHashes []common.Hash = nil
408+
r *test.NewPayloadResponseExpectObject
412409
err error
413410
)
414411
if step.VersionedHashes != nil {
@@ -417,10 +414,6 @@ func (step NewPayloads) Execute(t *BlobTestContext) error {
417414
if err != nil {
418415
t.Fatalf("FAIL: Error getting modified versioned hashes (payload %d/%d): %v", p+1, payloadCount, err)
419416
}
420-
r := t.TestEngine.TestEngineNewPayloadV3(&t.CLMock.LatestPayloadBuilt, versionedHashes)
421-
// All tests that modify the versioned hashes expect an
422-
// `INVALID` response, even if the client is out of sync
423-
r.ExpectStatus(test.Invalid)
424417
} else {
425418
if t.CLMock.LatestBlobBundle != nil {
426419
versionedHashes, err = t.CLMock.LatestBlobBundle.VersionedHashes(BLOB_COMMITMENT_VERSION_KZG)
@@ -432,19 +425,24 @@ func (step NewPayloads) Execute(t *BlobTestContext) error {
432425

433426
if step.PayloadCustomizer != nil {
434427
// Send a custom new payload
435-
payload, err = step.PayloadCustomizer.CustomizePayload(&t.CLMock.LatestPayloadBuilt)
428+
payload, err = step.PayloadCustomizer.CustomizePayload(payload)
436429
if err != nil {
437430
t.Fatalf("FAIL: Error customizing payload (payload %d/%d): %v", p+1, payloadCount, err)
438431
}
439-
} else {
440-
payload = &t.CLMock.LatestPayloadBuilt
441432
}
442433

443-
var r *test.NewPayloadResponseExpectObject
434+
version := step.Version
435+
if version == 0 {
436+
if t.Env.ForkConfig.IsCancun(payload.Timestamp) {
437+
version = 3
438+
} else {
439+
version = 2
440+
}
441+
}
444442

445-
if step.Version == 0 || step.Version == 3 {
443+
if version == 3 {
446444
r = t.TestEngine.TestEngineNewPayloadV3(payload, versionedHashes)
447-
} else if step.Version == 2 {
445+
} else if version == 2 {
448446
r = t.TestEngine.TestEngineNewPayloadV2(payload)
449447
} else {
450448
t.Fatalf("FAIL: Unknown version %d", step.Version)
@@ -466,7 +464,7 @@ func (step NewPayloads) Execute(t *BlobTestContext) error {
466464
if err != nil {
467465
t.Fatalf("FAIL: Error retrieving blob bundle (payload %d/%d): %v", p+1, payloadCount, err)
468466
}
469-
if err := step.VerifyPayload(t.TimeoutContext, t.TestEngine, blobTxsInPayload, payload, &previousPayload); err != nil {
467+
if err := step.VerifyPayload(t.TimeoutContext, t.Env.ForkConfig, t.TestEngine, blobTxsInPayload, payload, &previousPayload); err != nil {
470468
t.Fatalf("FAIL: Error verifying payload (payload %d/%d): %v", p+1, payloadCount, err)
471469
}
472470
previousPayload = t.CLMock.LatestPayloadBuilt

‎simulators/ethereum/engine/suites/blobs/tests.go‎

Lines changed: 183 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ var (
2727
DATA_GASPRICE_UPDATE_FRACTION = uint64(3338477)
2828

2929
BLOB_COMMITMENT_VERSION_KZG = byte(0x01)
30+
31+
// Engine API errors
32+
INVALID_PARAMS_ERROR = pInt(-32602)
33+
UNSUPPORTED_FORK_ERROR = pInt(-38005)
3034
)
3135

3236
// Precalculate the first data gas cost increase
@@ -510,48 +514,212 @@ var Tests = []test.SpecInterface{
510514
},
511515
},
512516
},
513-
// NewPayload Version Negative Tests
517+
518+
// NewPayloadV3 Before Cancun, Negative Tests
514519
&BlobsBaseSpec{
520+
Spec: test.Spec{
521+
Name: "NewPayloadV3 Before Cancun, Nil Data Fields, Nil Versioned Hashes",
522+
About: `
523+
Test sending NewPayloadV3 Before Cancun with:
524+
- nil ExcessDataGas
525+
- nil DataGasUsed
526+
- nil Versioned Hashes Array
527+
`,
528+
},
515529

530+
CancunForkHeight: 2,
531+
532+
TestSequence: TestSequence{
533+
NewPayloads{
534+
ExpectedIncludedBlobCount: 0,
535+
Version: 3,
536+
VersionedHashes: &VersionedHashes{
537+
Blobs: nil,
538+
},
539+
// On DEVNET 8:
540+
// ExpectedError: INVALID_PARAMS_ERROR,
541+
},
542+
},
543+
},
544+
&BlobsBaseSpec{
516545
Spec: test.Spec{
517-
Name: "NewPayloadV2 After Cancun",
546+
Name: "NewPayloadV3 Before Cancun, Nil ExcessDataGas, 0x00 DataGasUsed, Nil Versioned Hashes",
518547
About: `
519-
Test sending NewPayloadV2 after cancun fork, which
520-
should result in error.
548+
Test sending NewPayloadV3 Before Cancun with:
549+
- nil ExcessDataGas
550+
- 0x00 DataGasUsed
551+
- nil Versioned Hashes Array
521552
`,
522553
},
523554

524-
// We fork on genesis
525-
CancunForkHeight: 0,
555+
CancunForkHeight: 2,
526556

527557
TestSequence: TestSequence{
528-
// Send multiple blob transactions with the same nonce.
529558
NewPayloads{
530559
ExpectedIncludedBlobCount: 0,
531-
Version: 2,
532-
ExpectedError: pInt(-38005),
560+
Version: 3,
561+
PayloadCustomizer: &helper.CustomPayloadData{
562+
DataGasUsed: pUint64(0),
563+
},
564+
ExpectedError: INVALID_PARAMS_ERROR,
533565
},
534566
},
535567
},
536568
&BlobsBaseSpec{
569+
Spec: test.Spec{
570+
Name: "NewPayloadV3 Before Cancun, 0x00 ExcessDataGas, Nil DataGasUsed, Nil Versioned Hashes",
571+
About: `
572+
Test sending NewPayloadV3 Before Cancun with:
573+
- 0x00 ExcessDataGas
574+
- nil DataGasUsed
575+
- nil Versioned Hashes Array
576+
`,
577+
},
578+
579+
CancunForkHeight: 2,
537580

581+
TestSequence: TestSequence{
582+
NewPayloads{
583+
ExpectedIncludedBlobCount: 0,
584+
Version: 3,
585+
PayloadCustomizer: &helper.CustomPayloadData{
586+
ExcessDataGas: pUint64(0),
587+
},
588+
ExpectedError: INVALID_PARAMS_ERROR,
589+
},
590+
},
591+
},
592+
/*
593+
// Unspecified Outcome For Devnet 7, uncomment for Devnet 8
594+
&BlobsBaseSpec{
595+
Spec: test.Spec{
596+
Name: "NewPayloadV3 Before Cancun, Nil Data Fields, Empty Array Versioned Hashes",
597+
About: `
598+
Test sending NewPayloadV3 Before Cancun with:
599+
- nil ExcessDataGas
600+
- nil DataGasUsed
601+
- Empty Versioned Hashes Array
602+
`,
603+
},
604+
605+
CancunForkHeight: 2,
606+
607+
TestSequence: TestSequence{
608+
NewPayloads{
609+
ExpectedIncludedBlobCount: 0,
610+
Version: 3,
611+
VersionedHashes: &VersionedHashes{
612+
Blobs: []helper.BlobID{},
613+
},
614+
ExpectedError: INVALID_PARAMS_ERROR,
615+
},
616+
},
617+
},
618+
*/
619+
&BlobsBaseSpec{
538620
Spec: test.Spec{
539-
Name: "NewPayloadV3 Before Cancun",
621+
Name: "NewPayloadV3 Before Cancun, 0x00 Data Fields, Empty Array Versioned Hashes",
540622
About: `
541-
Test sending NewPayloadV3 before cancun fork, which
542-
should result in error.
623+
Test sending NewPayloadV3 Before Cancun with:
624+
- 0x00 ExcessDataGas
625+
- 0x00 DataGasUsed
626+
- Empty Versioned Hashes Array
543627
`,
544628
},
545629

546-
// We fork on genesis
547630
CancunForkHeight: 2,
548631

549632
TestSequence: TestSequence{
550-
// Send multiple blob transactions with the same nonce.
551633
NewPayloads{
552634
ExpectedIncludedBlobCount: 0,
553635
Version: 3,
554-
ExpectedError: pInt(-38005),
636+
VersionedHashes: &VersionedHashes{
637+
Blobs: []helper.BlobID{},
638+
},
639+
PayloadCustomizer: &helper.CustomPayloadData{
640+
ExcessDataGas: pUint64(0),
641+
DataGasUsed: pUint64(0),
642+
},
643+
ExpectedError: INVALID_PARAMS_ERROR,
644+
// On DEVNET 8:
645+
// ExpectedError: UNSUPPORTED_FORK_ERROR,
646+
},
647+
},
648+
},
649+
650+
// NewPayloadV3 After Cancun, Negative Tests
651+
/*
652+
// Unspecified Outcome For Devnet 7, uncomment for Devnet 8
653+
&BlobsBaseSpec{
654+
Spec: test.Spec{
655+
Name: "NewPayloadV3 After Cancun, 0x00 Data Fields, Nil Versioned Hashes",
656+
About: `
657+
Test sending NewPayloadV3 After Cancun with:
658+
- 0x00 ExcessDataGas
659+
- 0x00 DataGasUsed
660+
- nil Versioned Hashes Array
661+
`,
662+
},
663+
664+
CancunForkHeight: 1,
665+
666+
TestSequence: TestSequence{
667+
NewPayloads{
668+
ExpectedIncludedBlobCount: 0,
669+
Version: 3,
670+
VersionedHashes: &VersionedHashes{
671+
Blobs: nil,
672+
},
673+
ExpectedError: INVALID_PARAMS_ERROR,
674+
},
675+
},
676+
},
677+
*/
678+
&BlobsBaseSpec{
679+
Spec: test.Spec{
680+
Name: "NewPayloadV3 After Cancun, Nil ExcessDataGas, 0x00 DataGasUsed, Empty Array Versioned Hashes",
681+
About: `
682+
Test sending NewPayloadV3 After Cancun with:
683+
- nil ExcessDataGas
684+
- 0x00 DataGasUsed
685+
- Empty Versioned Hashes Array
686+
`,
687+
},
688+
689+
CancunForkHeight: 1,
690+
691+
TestSequence: TestSequence{
692+
NewPayloads{
693+
ExpectedIncludedBlobCount: 0,
694+
Version: 3,
695+
PayloadCustomizer: &helper.CustomPayloadData{
696+
RemoveExcessDataGas: true,
697+
},
698+
ExpectedError: INVALID_PARAMS_ERROR,
699+
},
700+
},
701+
},
702+
&BlobsBaseSpec{
703+
Spec: test.Spec{
704+
Name: "NewPayloadV3 After Cancun, 0x00 ExcessDataGas, Nil DataGasUsed, Empty Array Versioned Hashes",
705+
About: `
706+
Test sending NewPayloadV3 After Cancun with:
707+
- 0x00 ExcessDataGas
708+
- nil DataGasUsed
709+
- Empty Versioned Hashes Array
710+
`,
711+
},
712+
713+
CancunForkHeight: 1,
714+
715+
TestSequence: TestSequence{
716+
NewPayloads{
717+
ExpectedIncludedBlobCount: 0,
718+
Version: 3,
719+
PayloadCustomizer: &helper.CustomPayloadData{
720+
RemoveDataGasUsed: true,
721+
},
722+
ExpectedError: INVALID_PARAMS_ERROR,
555723
},
556724
},
557725
},

0 commit comments

Comments
 (0)