simulators/ethereum/engine: Unique Payload ID Tests

simulators/ethereum/engine/client/engine.go

@@ -4,8 +4,8 @@ import (
 	"context"
 	"math/big"
 
-	"github.com/ethereum/go-ethereum/core"
 	api "github.com/ethereum/go-ethereum/beacon/engine"
+	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/hive/hivesim"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -32,10 +32,12 @@ type Eth interface {
 type Engine interface {
 	ForkchoiceUpdatedV1(ctx context.Context, fcState *api.ForkchoiceStateV1, pAttributes *api.PayloadAttributes) (api.ForkChoiceResponse, error)
 	ForkchoiceUpdatedV2(ctx context.Context, fcState *api.ForkchoiceStateV1, pAttributes *api.PayloadAttributes) (api.ForkChoiceResponse, error)
+	ForkchoiceUpdated(ctx context.Context, version int, fcState *api.ForkchoiceStateV1, pAttributes *api.PayloadAttributes) (api.ForkChoiceResponse, error)
 
 	GetPayloadV1(ctx context.Context, payloadId *api.PayloadID) (api.ExecutableData, error)
 	GetPayloadV2(ctx context.Context, payloadId *api.PayloadID) (api.ExecutableData, *big.Int, error)
 
+	NewPayload(ctx context.Context, version int, payload interface{}) (api.PayloadStatusV1, error)
 	NewPayloadV1(ctx context.Context, payload *client_types.ExecutableDataV1) (api.PayloadStatusV1, error)
 	NewPayloadV2(ctx context.Context, payload *api.ExecutableData) (api.PayloadStatusV1, error)
 
@@ -76,8 +78,10 @@ type EngineStarter interface {
 }
 
 var (
-	Head      *big.Int // Nil
-	Pending   = big.NewInt(-2)
-	Finalized = big.NewInt(-3)
-	Safe      = big.NewInt(-4)
+	Head                           *big.Int // Nil
+	Pending                        = big.NewInt(-2)
+	Finalized                      = big.NewInt(-3)
+	Safe                           = big.NewInt(-4)
+	LatestForkchoiceUpdatedVersion = 2
+	LatestNewPayloadVersion        = 2
 )

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

@@ -415,7 +415,7 @@ func (ec *HiveRPCEngineClient) GetPayloadBodiesByHashV1(ctx context.Context, has
 	return result, err
 }
 
-func (ec *HiveRPCEngineClient) newPayload(ctx context.Context, version int, payload interface{}) (api.PayloadStatusV1, error) {
+func (ec *HiveRPCEngineClient) NewPayload(ctx context.Context, version int, payload interface{}) (api.PayloadStatusV1, error) {
 	var result api.PayloadStatusV1
 	if err := ec.PrepareDefaultAuthCallToken(); err != nil {
 		return result, err
@@ -428,12 +428,12 @@ func (ec *HiveRPCEngineClient) newPayload(ctx context.Context, version int, payl
 func (ec *HiveRPCEngineClient) NewPayloadV1(ctx context.Context, payload *client_types.ExecutableDataV1) (api.PayloadStatusV1, error) {
 	ed := payload.ToExecutableData()
 	ec.latestPayloadSent = &ed
-	return ec.newPayload(ctx, 1, payload)
+	return ec.NewPayload(ctx, 1, payload)
 }
 
 func (ec *HiveRPCEngineClient) NewPayloadV2(ctx context.Context, payload *api.ExecutableData) (api.PayloadStatusV1, error) {
 	ec.latestPayloadSent = payload
-	return ec.newPayload(ctx, 2, payload)
+	return ec.NewPayload(ctx, 2, payload)
 }
 
 func (ec *HiveRPCEngineClient) ExchangeTransitionConfigurationV1(ctx context.Context, tConf *api.TransitionConfigurationV1) (api.TransitionConfigurationV1, error) {

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

@@ -695,6 +695,23 @@ func (n *GethNode) SetBlock(block *types.Block, parentNumber uint64, parentRoot
 }
 
 // Engine API
+func (n *GethNode) NewPayload(ctx context.Context, version int, pl interface{}) (beacon.PayloadStatusV1, error) {
+	switch version {
+	case 1:
+		if c, ok := pl.(*client_types.ExecutableDataV1); ok {
+			return n.NewPayloadV1(ctx, c)
+		} else {
+			return beacon.PayloadStatusV1{}, fmt.Errorf("wrong type %T", pl)
+		}
+	case 2:
+		if c, ok := pl.(*beacon.ExecutableData); ok {
+			return n.NewPayloadV2(ctx, c)
+		} else {
+			return beacon.PayloadStatusV1{}, fmt.Errorf("wrong type %T", pl)
+		}
+	}
+	return beacon.PayloadStatusV1{}, fmt.Errorf("unknown version %d", version)
+}
 func (n *GethNode) NewPayloadV1(ctx context.Context, pl *client_types.ExecutableDataV1) (beacon.PayloadStatusV1, error) {
 	ed := pl.ToExecutableData()
 	n.latestPayloadSent = &ed
@@ -708,7 +725,16 @@ func (n *GethNode) NewPayloadV2(ctx context.Context, pl *beacon.ExecutableData)
 	n.latestPayloadStatusReponse = &resp
 	return resp, err
 }
-
+func (n *GethNode) ForkchoiceUpdated(ctx context.Context, version int, fcs *beacon.ForkchoiceStateV1, payload *beacon.PayloadAttributes) (beacon.ForkChoiceResponse, error) {
+	switch version {
+	case 1:
+		return n.ForkchoiceUpdatedV1(ctx, fcs, payload)
+	case 2:
+		return n.ForkchoiceUpdatedV2(ctx, fcs, payload)
+	default:
+		return beacon.ForkChoiceResponse{}, fmt.Errorf("unknown version %d", version)
+	}
+}
 func (n *GethNode) ForkchoiceUpdatedV1(ctx context.Context, fcs *beacon.ForkchoiceStateV1, payload *beacon.PayloadAttributes) (beacon.ForkChoiceResponse, error) {
 	n.latestFcUStateSent = fcs
 	n.latestPAttrSent = payload

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

@@ -88,6 +88,11 @@ var Tests = []test.Spec{
 		Run:  preTTDFinalizedBlockHash,
 		TTD:  2,
 	},
+	{
+		Name:       "Unique Payload ID",
+		Run:        uniquePayloadID,
+		ForkConfig: test.LatestFork,
+	},
 	// Invalid Payload Tests
 	{
 		Name: "Bad Hash on NewPayload",
@@ -1513,6 +1518,70 @@ func preTTDFinalizedBlockHash(t *test.Env) {
 
 }
 
+// Check that the payload id returned on a forkchoiceUpdated call is different
+// when the attributes change
+func uniquePayloadID(t *test.Env) {
+	// Wait until TTD is reached by this client
+	t.CLMock.WaitForTTD()
+
+	parentHash := t.CLMock.LatestHeader.Hash()
+
+	fcState := &api.ForkchoiceStateV1{
+		HeadBlockHash: parentHash,
+	}
+	payloadAttributes := &api.PayloadAttributes{
+		Timestamp:             t.CLMock.LatestHeader.Time + 1,
+		Random:                common.Hash{},
+		SuggestedFeeRecipient: common.Address{},
+		Withdrawals:           []*types.Withdrawal{},
+	}
+
+	previousPayloadID := &api.PayloadID{}
+
+	r := t.TestEngine.TestEngineForkchoiceUpdated(fcState, payloadAttributes, -1)
+	r.ExpectUpdatedPayloadID(previousPayloadID)
+
+	// Modify timestamp
+	previousPayloadID = r.Response.PayloadID
+	payloadAttributes.Timestamp += 1
+	r = t.TestEngine.TestEngineForkchoiceUpdated(fcState, payloadAttributes, -1)
+	r.ExpectUpdatedPayloadID(previousPayloadID)
+
+	// Modify random
+	previousPayloadID = r.Response.PayloadID
+	payloadAttributes.Random = common.Hash{1}
+	r = t.TestEngine.TestEngineForkchoiceUpdated(fcState, payloadAttributes, -1)
+	r.ExpectUpdatedPayloadID(previousPayloadID)
+
+	// Modify fee recipient
+	previousPayloadID = r.Response.PayloadID
+	payloadAttributes.SuggestedFeeRecipient = common.Address{1}
+	r = t.TestEngine.TestEngineForkchoiceUpdated(fcState, payloadAttributes, -1)
+	r.ExpectUpdatedPayloadID(previousPayloadID)
+
+	// Add withdrawal
+	previousPayloadID = r.Response.PayloadID
+	newWithdrawal := &types.Withdrawal{}
+	payloadAttributes.Withdrawals = append(payloadAttributes.Withdrawals, newWithdrawal)
+	r = t.TestEngine.TestEngineForkchoiceUpdated(fcState, payloadAttributes, -1)
+	r.ExpectUpdatedPayloadID(previousPayloadID)
+
+	// Modify withdrawal
+	previousPayloadID = r.Response.PayloadID
+	newWithdrawal.Amount = 1
+	r = t.TestEngine.TestEngineForkchoiceUpdated(fcState, payloadAttributes, -1)
+	r.ExpectUpdatedPayloadID(previousPayloadID)
+
+	// Add another withdrawal
+	previousPayloadID = r.Response.PayloadID
+	newWithdrawal = &types.Withdrawal{Index: 1}
+	payloadAttributes.Withdrawals = append(payloadAttributes.Withdrawals, newWithdrawal)
+	r = t.TestEngine.TestEngineForkchoiceUpdated(fcState, payloadAttributes, -1)
+	r.ExpectUpdatedPayloadID(previousPayloadID)
+
+	// Add test here for future attributes!
+}
+
 // Corrupt the hash of a valid payload, client should reject the payload.
 // All possible scenarios:
 //    (fcU)

simulators/ethereum/engine/test/expect.go

@@ -9,8 +9,8 @@ import (
 	"runtime"
 	"strings"
 
-	"github.com/ethereum/go-ethereum/common"
 	api "github.com/ethereum/go-ethereum/beacon/engine"
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/rpc"
 	"github.com/ethereum/hive/simulators/ethereum/engine/client"
@@ -122,6 +122,9 @@ func (tec *TestEngineClient) TestEngineForkchoiceUpdatedV2(fcState *api.Forkchoi
 }
 
 func (tec *TestEngineClient) TestEngineForkchoiceUpdated(fcState *api.ForkchoiceStateV1, pAttributes *api.PayloadAttributes, version int) *ForkchoiceResponseExpectObject {
+	if version == -1 {
+		version = client.LatestForkchoiceUpdatedVersion
+	}
 	if version == 2 {
 		return tec.TestEngineForkchoiceUpdatedV2(fcState, pAttributes)
 	}
@@ -186,6 +189,21 @@ func (exp *ForkchoiceResponseExpectObject) ExpectPayloadID(pid *api.PayloadID) {
 	}
 }
 
+func (exp *ForkchoiceResponseExpectObject) ExpectUpdatedPayloadID(previousID *api.PayloadID) {
+	exp.ExpectNoError()
+	if exp.Response.PayloadID == nil || previousID == nil {
+		if exp.Response.PayloadID == previousID {
+			// Both are null
+			exp.Fatalf("FAIL (%v): Unexpected PayloadID on EngineForkchoiceUpdatedV%d: Expected change from %v", exp.TestName, exp.Version, previousID)
+		}
+	} else {
+		// Both are different from null
+		if *exp.Response.PayloadID == *previousID {
+			exp.Fatalf("FAIL (%v): Unexpected PayloadID on EngineForkchoiceUpdatedV%d: Expected change from %s", exp.TestName, exp.Version, previousID.String())
+		}
+	}
+}
+
 // NewPayload
 
 type NewPayloadResponseExpectObject struct {

simulators/ethereum/engine/test/spec.go

@@ -135,3 +135,7 @@ func (s Spec) GetTTD() int64 {
 func (s Spec) IsMiningDisabled() bool {
 	return s.DisableMining
 }
+
+var LatestFork = ForkConfig{
+	ShanghaiTimestamp: big.NewInt(0),
+}