Working commit

Author: marioevz

simulators/ethereum/engine/client/node/node.go

@@ -472,6 +472,9 @@ func (n *GethNode) NewPayloadV3(ctx context.Context, pl *typ.ExecutableData) (be
 	if err != nil {
 		return beacon.PayloadStatusV1{}, err
 	}
+	if pl.VersionedHashes == nil {
+		return beacon.PayloadStatusV1{}, fmt.Errorf("versioned hashes are nil")
+	}
 	resp, err := n.api.NewPayloadV3(ed, *pl.VersionedHashes, pl.ParentBeaconBlockRoot)
 	n.latestPayloadStatusReponse = &resp
 	return resp, err
@@ -483,6 +486,8 @@ func (n *GethNode) ForkchoiceUpdated(ctx context.Context, version int, fcs *beac
 		return n.ForkchoiceUpdatedV1(ctx, fcs, payload)
 	case 2:
 		return n.ForkchoiceUpdatedV2(ctx, fcs, payload)
+	case 3:
+		return n.ForkchoiceUpdatedV3(ctx, fcs, payload)
 	default:
 		return beacon.ForkChoiceResponse{}, fmt.Errorf("unknown version %d", version)
 	}
@@ -548,18 +553,27 @@ func (n *GethNode) GetPayloadV3(ctx context.Context, payloadId *beacon.PayloadID
 		return typ.ExecutableData{}, nil, nil, nil, err
 	}
 	ed, err := typ.FromBeaconExecutableData(p.ExecutionPayload)
-	// TODO: Convert and return the blobs bundle
-	return ed, p.BlockValue, nil, &p.Override, err
+	blobsBundle := &typ.BlobsBundle{}
+	if err := blobsBundle.FromBeaconBlobsBundle(p.BlobsBundle); err != nil {
+		return typ.ExecutableData{}, nil, nil, nil, err
+	}
+	return ed, p.BlockValue, blobsBundle, &p.Override, err
 }
 
 func (n *GethNode) GetPayload(ctx context.Context, version int, payloadId *beacon.PayloadID) (typ.ExecutableData, *big.Int, *typ.BlobsBundle, *bool, error) {
-	p, err := n.api.GetPayloadV3(*payloadId)
-	if p == nil || err != nil {
-		return typ.ExecutableData{}, nil, nil, nil, err
+
+	switch version {
+	case 1:
+		ed, err := n.GetPayloadV1(ctx, payloadId)
+		return ed, nil, nil, nil, err
+	case 2:
+		ed, value, err := n.GetPayloadV2(ctx, payloadId)
+		return ed, value, nil, nil, err
+	case 3:
+		return n.GetPayloadV3(ctx, payloadId)
+	default:
+		return typ.ExecutableData{}, nil, nil, nil, fmt.Errorf("unknown version %d", version)
 	}
-	ed, err := typ.FromBeaconExecutableData(p.ExecutionPayload)
-	// TODO: Convert and return the blobs bundle
-	return ed, p.BlockValue, nil, &p.Override, err
 }
 
 func (n *GethNode) GetPayloadBodiesByRangeV1(ctx context.Context, start uint64, count uint64) ([]*typ.ExecutionPayloadBodyV1, error) {

simulators/ethereum/engine/clmock/clmock.go

@@ -695,9 +695,11 @@ func (cl *CLMocker) ProduceSingleBlock(callbacks BlockProcessCallbacks) {
 		defer cancel()
 		newHeader, err := ec.HeaderByNumber(ctx, cl.LatestHeadNumber)
 		if err != nil {
+			cl.Logf("CLMocker: Client %v did not accept the new payload: %v", ec.ID(), err)
 			continue
 		}
 		if newHeader.Hash() != cl.LatestPayloadBuilt.BlockHash {
+			cl.Logf("CLMocker: Client %v produced a new header with incorrect hash: %v != %v", ec.ID(), newHeader.Hash(), cl.LatestPayloadBuilt.BlockHash)
 			continue
 		}
 		// Check that the new finalized header has the correct properties
@@ -726,6 +728,7 @@ func (cl *CLMocker) ProduceSingleBlock(callbacks BlockProcessCallbacks) {
 		cl.Fatalf("CLMocker: None of the clients accepted the newly constructed payload")
 	}
 	cl.HeaderHistory[cl.LatestHeadNumber.Uint64()] = cl.LatestHeader
+	cl.Logf("CLMocker: New block produced: number=%d, hash=%x", cl.LatestHeader.Number, cl.LatestHeader.Hash())
 }
 
 // Loop produce PoS blocks by using the Engine API
@@ -776,7 +779,7 @@ func (cl *CLMocker) BroadcastForkchoiceUpdated(fcstate *api.ForkchoiceStateV1, p
 			defer cancel()
 			fcUpdatedResp, err := ec.ForkchoiceUpdated(ctx, version, fcstate, payloadAttr)
 			if err != nil {
-				cl.Errorf("CLMocker: Could not ForkchoiceUpdatedV1: %v", err)
+				cl.Errorf("CLMocker: Could not ForkchoiceUpdated: %v", err)
 				responses[i].Error = err
 			} else {
 				responses[i].ForkchoiceResponse = &fcUpdatedResp

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

@@ -1824,4 +1824,24 @@ func init() {
 	for _, test := range suite_engine.Tests {
 		Tests = append(Tests, test.WithMainFork(config.Cancun))
 	}
+
+	// Cancun specific variants for pre-existing tests
+	// Payload Attributes
+	for _, t := range []suite_engine.InvalidPayloadAttributesTest{
+		{
+			BaseSpec: test.BaseSpec{
+				MainFork: config.Cancun,
+			},
+			Description: "Missing BeaconRoot",
+			Customizer: &helper.BasePayloadAttributesCustomizer{
+				RemoveBeaconRoot: true,
+			},
+			// Error is expected on syncing because V3 checks all fields to be present
+			ErrorOnSync: true,
+		},
+	} {
+		Tests = append(Tests, t)
+		t.Syncing = true
+		Tests = append(Tests, t)
+	}
 }

simulators/ethereum/engine/suites/engine/bad_hash.go

@@ -109,7 +109,11 @@ func (b BadHashOnNewPayload) Execute(t *test.Env) {
 			// - {status: INVALID_BLOCK_HASH, latestValidHash: null, validationError: null} if the blockHash validation has failed
 			// Starting from Shanghai, INVALID should be returned instead (https://github.com/ethereum/execution-apis/pull/338)
 			r := t.TestEngine.TestEngineNewPayload(&alteredPayload)
-			r.ExpectStatusEither(test.InvalidBlockHash, test.Invalid)
+			if r.Version >= 2 {
+				r.ExpectStatus(test.Invalid)
+			} else {
+				r.ExpectStatusEither(test.InvalidBlockHash, test.Invalid)
+			}
 			r.ExpectLatestValidHash(nil)
 		},
 	})
@@ -178,7 +182,11 @@ func (b ParentHashOnNewPayload) Execute(t *test.Env) {
 			// - {status: INVALID_BLOCK_HASH, latestValidHash: null, validationError: null} if the blockHash validation has failed
 			// Starting from Shanghai, INVALID should be returned instead (https://github.com/ethereum/execution-apis/pull/338)
 			r := t.TestEngine.TestEngineNewPayload(&alteredPayload)
-			r.ExpectStatusEither(test.InvalidBlockHash)
+			if r.Version >= 2 {
+				r.ExpectStatus(test.Invalid)
+			} else {
+				r.ExpectStatusEither(test.Invalid, test.InvalidBlockHash)
+			}
 			r.ExpectLatestValidHash(nil)
 		},
 	})

simulators/ethereum/engine/suites/engine/invalid_ancestor.go

@@ -202,7 +202,7 @@ func (tc InvalidMissingAncestorReOrgSyncTest) GetName() string {
 	if tc.EmptyTransactions {
 		name += "Empty Txs, "
 	}
-	name += fmt.Sprintf(", Invalid P%d', Reveal using sync", tc.InvalidIndex)
+	name += fmt.Sprintf("Invalid P%d', Reveal using sync", tc.InvalidIndex)
 	if tc.ReOrgFromCanonical {
 		name += " (ReOrg from Canonical)"
 	}
@@ -240,30 +240,17 @@ func (tc InvalidMissingAncestorReOrgSyncTest) Execute(t *test.Env) {
 	t.CLMock.WaitForTTD()
 
 	// Produce blocks before starting the test
-	var cA *types.Block
-
-	cAHeight := tc.CommonAncestorHeight
-	if cAHeight == nil {
-		// Default is to produce 5 PoS blocks before the common ancestor
-		cAHeight = big.NewInt(5)
+	// Default is to produce 5 PoS blocks before the common ancestor
+	cAHeight := 5
+	if tc.CommonAncestorHeight != nil {
+		cAHeight = int(tc.CommonAncestorHeight.Int64())
 	}
 
 	// Save the common ancestor
-	if cAHeight.Cmp(big0) == 0 {
-		// Common ancestor is the proof-of-work terminal block
-		ctx, cancel := context.WithTimeout(t.TestContext, globals.RPCTimeout)
-		defer cancel()
-		b, err := secondaryClient.BlockByNumber(ctx, nil)
-		if err != nil {
-			t.Fatalf("FAIL (%s): Error while getting latest block: %v", t.TestName, err)
-		}
-		cA = b
+	if cAHeight == 0 {
+		t.Fatalf("FAIL (%s): Invalid common ancestor height: %d", t.TestName, cAHeight)
 	} else {
-		t.CLMock.ProduceBlocks(int(cAHeight.Int64()), clmock.BlockProcessCallbacks{})
-		cA, err = typ.ExecutableDataToBlock(t.CLMock.LatestPayloadBuilt)
-		if err != nil {
-			t.Fatalf("FAIL (%s): Error converting payload to block: %v", t.TestName, err)
-		}
+		t.CLMock.ProduceBlocks(cAHeight, clmock.BlockProcessCallbacks{})
 	}
 
 	// Amount of blocks to deviate starting from the common ancestor
@@ -274,15 +261,17 @@ func (tc InvalidMissingAncestorReOrgSyncTest) Execute(t *test.Env) {
 	}
 
 	// Slice to save the side B chain
-	altChainPayloads := make([]*types.Block, 0)
+	altChainPayloads := make([]*typ.ExecutableData, 0)
 
 	// Append the common ancestor
-	altChainPayloads = append(altChainPayloads, cA)
+	cA := t.CLMock.LatestPayloadBuilt
+	altChainPayloads = append(altChainPayloads, &cA)
 
 	// Produce blocks but at the same time create an side chain which contains an invalid payload at some point (INV_P)
 	// CommonAncestor◄─▲── P1 ◄─ P2 ◄─ P3 ◄─ ... ◄─ Pn
 	//                 │
 	//                 └── P1' ◄─ P2' ◄─ ... ◄─ INV_P ◄─ ... ◄─ Pn'
+	t.Log("INFO: Starting canonical chain production")
 	t.CLMock.ProduceBlocks(n, clmock.BlockProcessCallbacks{
 
 		OnPayloadProducerSelected: func() {
@@ -312,7 +301,7 @@ func (tc InvalidMissingAncestorReOrgSyncTest) Execute(t *test.Env) {
 				err         error
 			)
 			// Insert extraData to ensure we deviate from the main payload, which contains empty extradata
-			pHash := altChainPayloads[len(altChainPayloads)-1].Hash()
+			pHash := altChainPayloads[len(altChainPayloads)-1].BlockHash
 			customizer := &helper.CustomPayloadData{
 				ParentHash: &pHash,
 				ExtraData:  &([]byte{0x01}),
@@ -327,7 +316,9 @@ func (tc InvalidMissingAncestorReOrgSyncTest) Execute(t *test.Env) {
 					t.Fatalf("FAIL (%s): Unable to customize payload: %v", t.TestName, err)
 				}
 			}
+			altChainPayloads = append(altChainPayloads, sidePayload)
 
+			// TODO: REMOVE THIS
 			sideBlock, err := typ.ExecutableDataToBlock(*sidePayload)
 			if err != nil {
 				t.Fatalf("FAIL (%s): Error converting payload to block: %v", t.TestName, err)
@@ -351,10 +342,9 @@ func (tc InvalidMissingAncestorReOrgSyncTest) Execute(t *test.Env) {
 					t.Fatalf("FAIL (%s): Unable to customize payload block: %v", t.TestName, err)
 				}
 			}
-
-			altChainPayloads = append(altChainPayloads, sideBlock)
 		},
 	})
+	t.Log("INFO: Starting side chain production")
 	t.CLMock.ProduceSingleBlock(clmock.BlockProcessCallbacks{
 		// Note: We perform the test in the middle of payload creation by the CL Mock, in order to be able to
 		// re-org back into this chain and use the new payload without issues.
@@ -369,30 +359,30 @@ func (tc InvalidMissingAncestorReOrgSyncTest) Execute(t *test.Env) {
 				} else if i > tc.InvalidIndex {
 					payloadValidStr = "VALID with INVALID ancestor"
 				}
-				payloadJs, _ := json.MarshalIndent(altChainPayloads[i].Header(), "", " ")
+				payloadJs, _ := json.MarshalIndent(altChainPayloads[i], "", " ")
 				t.Logf("INFO (%s): Invalid chain payload %d (%s):\n%s", t.TestName, i, payloadValidStr, payloadJs)
 
 				if i < tc.InvalidIndex {
-					p := typ.BlockToExecutableData(altChainPayloads[i], common.Big0)
-					r := secondaryTestClient.TestEngineNewPayload(&p)
+					p := altChainPayloads[i]
+					r := secondaryTestClient.TestEngineNewPayload(p)
 					r.ExpectationDescription = "Sent modified payload to secondary client, expected to be accepted"
 					r.ExpectStatusEither(test.Valid, test.Accepted)
 
 					s := secondaryTestClient.TestEngineForkchoiceUpdated(&api.ForkchoiceStateV1{
 						HeadBlockHash:      p.BlockHash,
-						SafeBlockHash:      cA.Hash(),
+						SafeBlockHash:      cA.BlockHash,
 						FinalizedBlockHash: common.Hash{},
 					}, nil, p.Timestamp)
 					s.ExpectationDescription = "Sent modified payload forkchoice updated to secondary client, expected to be accepted"
 					s.ExpectAnyPayloadStatus(test.Valid, test.Syncing)
 
 				} else {
-					invalidBlock := altChainPayloads[i]
+					invalidBlock, err := typ.ExecutableDataToBlock(*altChainPayloads[i])
 					if err != nil {
 						t.Fatalf("FAIL (%s): TEST ISSUE - Failed to create block from payload: %v", t.TestName, err)
 					}
 
-					if err := secondaryClient.SetBlock(invalidBlock, altChainPayloads[i-1].NumberU64(), altChainPayloads[i-1].Root()); err != nil {
+					if err := secondaryClient.SetBlock(invalidBlock, altChainPayloads[i-1].Number, altChainPayloads[i-1].StateRoot); err != nil {
 						t.Fatalf("FAIL (%s): TEST ISSUE - Failed to set invalid block: %v", t.TestName, err)
 					}
 					t.Logf("INFO (%s): Invalid block successfully set %d (%s): %v", t.TestName, i, payloadValidStr, invalidBlock.Hash())
@@ -406,8 +396,8 @@ func (tc InvalidMissingAncestorReOrgSyncTest) Execute(t *test.Env) {
 				t.Fatalf("FAIL (%s): TEST ISSUE - Secondary Node unable to reatrieve latest header: %v", t.TestName, err)
 
 			}
-			if head.Hash() != altChainPayloads[n-1].Hash() {
-				t.Fatalf("FAIL (%s): TEST ISSUE - Secondary Node has invalid blockhash got %v want %v gotNum %v wantNum %d", t.TestName, head.Hash(), altChainPayloads[n-1].Hash(), head.Number, altChainPayloads[n].NumberU64())
+			if head.Hash() != altChainPayloads[n-1].BlockHash {
+				t.Fatalf("FAIL (%s): TEST ISSUE - Secondary Node has invalid blockhash got %v want %v gotNum %v wantNum %d", t.TestName, head.Hash(), altChainPayloads[n-1].BlockHash, head.Number, altChainPayloads[n].Number)
 			} else {
 				t.Logf("INFO (%s): Secondary Node has correct block", t.TestName)
 			}
@@ -425,24 +415,23 @@ func (tc InvalidMissingAncestorReOrgSyncTest) Execute(t *test.Env) {
 				t.Logf("INFO (%s): Latest block on main client before sync: hash=%v, number=%d", t.TestName, l.Hash(), l.Number())
 			}
 			// If we are syncing through p2p, we need to keep polling until the client syncs the missing payloads
-			ed := typ.BlockToExecutableData(altChainPayloads[n], common.Big0)
 			for {
-				r := t.TestEngine.TestEngineNewPayload(&ed)
+				r := t.TestEngine.TestEngineNewPayload(altChainPayloads[n])
 				t.Logf("INFO (%s): Response from main client: %v", t.TestName, r.Status)
 				s := t.TestEngine.TestEngineForkchoiceUpdated(&api.ForkchoiceStateV1{
-					HeadBlockHash:      altChainPayloads[n].Hash(),
-					SafeBlockHash:      altChainPayloads[n].Hash(),
+					HeadBlockHash:      altChainPayloads[n].BlockHash,
+					SafeBlockHash:      altChainPayloads[n].BlockHash,
 					FinalizedBlockHash: common.Hash{},
-				}, nil, ed.Timestamp)
+				}, nil, altChainPayloads[n].Timestamp)
 				t.Logf("INFO (%s): Response from main client fcu: %v", t.TestName, s.Response.PayloadStatus)
 
 				if r.Status.Status == test.Invalid {
 					// We also expect that the client properly returns the LatestValidHash of the block on the
 					// side chain that is immediately prior to the invalid payload (or zero if parent is PoW)
 					var lvh common.Hash
-					if cAHeight.Cmp(big0) != 0 || tc.InvalidIndex != 1 {
+					if cAHeight != 0 || tc.InvalidIndex != 1 {
 						// Parent is NOT Proof of Work
-						lvh = altChainPayloads[tc.InvalidIndex-1].Hash()
+						lvh = altChainPayloads[tc.InvalidIndex-1].BlockHash
 					}
 					r.ExpectLatestValidHash(&lvh)
 					// Response on ForkchoiceUpdated should be the same

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

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"math/rand"
 
-	"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/helper"
@@ -16,6 +15,7 @@ type InvalidPayloadAttributesTest struct {
 	Description string
 	Customizer  helper.PayloadAttributesCustomizer
 	Syncing     bool
+	ErrorOnSync bool
 }
 
 func (s InvalidPayloadAttributesTest) WithMainFork(fork config.Fork) test.Spec {
@@ -41,17 +41,21 @@ func (tc InvalidPayloadAttributesTest) Execute(t *test.Env) {
 
 	// Send a forkchoiceUpdated with invalid PayloadAttributes
 	t.CLMock.ProduceSingleBlock(clmock.BlockProcessCallbacks{
-		OnPayloadAttributesGenerated: func() {
+		OnNewPayloadBroadcast: func() {
 			// Try to apply the new payload with invalid attributes
 			fcu := t.CLMock.LatestForkchoice
-			var blockHash common.Hash
 			if tc.Syncing {
 				// Setting a random hash will put the client into `SYNCING`
 				rand.Read(fcu.HeadBlockHash[:])
+			} else {
+				fcu.HeadBlockHash = t.CLMock.LatestPayloadBuilt.BlockHash
 			}
-			t.Logf("INFO (%s): Sending EngineForkchoiceUpdated (Syncing=%s) with invalid payload attributes: %s", t.TestName, tc.Syncing, tc.Description)
+			t.Logf("INFO (%s): Sending EngineForkchoiceUpdated (Syncing=%t) with invalid payload attributes: %s", t.TestName, tc.Syncing, tc.Description)
 
-			attr, err := tc.Customizer.GetPayloadAttributes(&t.CLMock.LatestPayloadAttributes)
+			// Get the payload attributes
+			originalAttr := t.CLMock.LatestPayloadAttributes
+			originalAttr.Timestamp += 1
+			attr, err := tc.Customizer.GetPayloadAttributes(&originalAttr)
 			if err != nil {
 				t.Fatalf("FAIL (%s): Unable to customize payload attributes: %v", t.TestName, err)
 			}
@@ -64,15 +68,20 @@ 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)
-				r.ExpectPayloadStatus(test.Syncing)
-				r.ExpectPayloadID(nil)
+				if tc.ErrorOnSync {
+					r.ExpectError()
+				} else {
+					r.ExpectPayloadStatus(test.Syncing)
+					r.ExpectPayloadID(nil)
+				}
 			} else {
 				r := t.TestEngine.TestEngineForkchoiceUpdated(&fcu, attr, t.CLMock.LatestPayloadBuilt.Timestamp)
 				r.ExpectError()
 
 				// Check that the forkchoice was applied, regardless of the error
 				s := t.TestEngine.TestHeaderByNumber(Head)
-				s.ExpectHash(blockHash)
+				s.ExpectationDescription = "Forkchoice is applied even on invalid payload attributes"
+				s.ExpectHash(fcu.HeadBlockHash)
 			}
 		},
 	})