beacon/engine, eth/catalyst, miner: honor targetGasLimit from payload attributes

beacon/engine/types.go

@@ -74,12 +74,14 @@ type PayloadAttributes struct {
 	Withdrawals           []*types.Withdrawal `json:"withdrawals"`
 	BeaconRoot            *common.Hash        `json:"parentBeaconBlockRoot"`
 	SlotNumber            *uint64             `json:"slotNumber"`
+	TargetGasLimit        *uint64             `json:"targetGasLimit"`
 }
 
 // JSON type overrides for PayloadAttributes.
 type payloadAttributesMarshaling struct {
-	Timestamp  hexutil.Uint64
-	SlotNumber *hexutil.Uint64
+	Timestamp      hexutil.Uint64
+	SlotNumber     *hexutil.Uint64
+	TargetGasLimit *hexutil.Uint64
 }
 
 //go:generate go run github.com/fjl/gencodec -type ExecutableData -field-override executableDataMarshaling -out ed_codec.go

beacon/engine/types_test.go

@@ -17,6 +17,8 @@
 package engine
 
 import (
+	"encoding/json"
+	"reflect"
 	"testing"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -46,3 +48,42 @@ func TestBlobs(t *testing.T) {
 		t.Fatalf("Expect 128 proofs in blobs bundle, got %v", len(env.BlobsBundle.Proofs))
 	}
 }
+
+// TestPayloadAttributesJSON verifies the JSON encoding of PayloadAttributes,
+// in particular that the amsterdam targetGasLimit field survives a round trip
+// and that attributes as sent by a consensus client on forkchoiceUpdatedV4
+// decode correctly.
+func TestPayloadAttributesJSON(t *testing.T) {
+	// PayloadAttributesV4 as sent by a Gloas consensus client.
+	input := `{
+		"timestamp": "0x64",
+		"prevRandao": "0x0202020202020202020202020202020202020202020202020202020202020202",
+		"suggestedFeeRecipient": "0x0101010101010101010101010101010101010101",
+		"withdrawals": [],
+		"parentBeaconBlockRoot": "0x0303030303030303030303030303030303030303030303030303030303030303",
+		"slotNumber": "0x10",
+		"targetGasLimit": "0x11e1a300"
+	}`
+	var attr PayloadAttributes
+	if err := json.Unmarshal([]byte(input), &attr); err != nil {
+		t.Fatalf("failed to unmarshal payload attributes: %v", err)
+	}
+	if attr.SlotNumber == nil || *attr.SlotNumber != 16 {
+		t.Fatalf("wrong slotNumber: %v", attr.SlotNumber)
+	}
+	if attr.TargetGasLimit == nil || *attr.TargetGasLimit != 300_000_000 {
+		t.Fatalf("wrong targetGasLimit: %v", attr.TargetGasLimit)
+	}
+	// Round trip.
+	encoded, err := json.Marshal(&attr)
+	if err != nil {
+		t.Fatalf("failed to marshal payload attributes: %v", err)
+	}
+	var decoded PayloadAttributes
+	if err := json.Unmarshal(encoded, &decoded); err != nil {
+		t.Fatalf("failed to unmarshal re-encoded payload attributes: %v", err)
+	}
+	if !reflect.DeepEqual(attr, decoded) {
+		t.Fatalf("payload attributes changed in round trip:\nbefore: %+v\nafter: %+v", attr, decoded)
+	}
+}

eth/catalyst/api.go

@@ -216,7 +216,8 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV3(ctx context.Context, update engine.
 }
 
 // ForkchoiceUpdatedV4 is equivalent to V3 with the addition of slot number
-// in the payload attributes. It supports only PayloadAttributesV4.
+// and target gas limit in the payload attributes. It supports only
+// PayloadAttributesV4.
 func (api *ConsensusAPI) ForkchoiceUpdatedV4(ctx context.Context, update engine.ForkchoiceStateV1, params *engine.PayloadAttributes) (engine.ForkChoiceResponse, error) {
 	if params != nil {
 		switch {
@@ -226,6 +227,8 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV4(ctx context.Context, update engine.
 			return engine.STATUS_INVALID, attributesErr("missing beacon root")
 		case params.SlotNumber == nil:
 			return engine.STATUS_INVALID, attributesErr("missing slot number")
+		case params.TargetGasLimit == nil:
+			return engine.STATUS_INVALID, attributesErr("missing target gas limit")
 		case !api.checkFork(params.Timestamp, forks.Amsterdam):
 			return engine.STATUS_INVALID, unsupportedForkErr("fcuV4 must only be called for amsterdam payloads")
 		}
@@ -379,14 +382,15 @@ func (api *ConsensusAPI) forkchoiceUpdated(ctx context.Context, update engine.Fo
 	// will replace it arbitrarily many times in between.
 	if payloadAttributes != nil {
 		args := &miner.BuildPayloadArgs{
-			Parent:       update.HeadBlockHash,
-			Timestamp:    payloadAttributes.Timestamp,
-			FeeRecipient: payloadAttributes.SuggestedFeeRecipient,
-			Random:       payloadAttributes.Random,
-			Withdrawals:  payloadAttributes.Withdrawals,
-			BeaconRoot:   payloadAttributes.BeaconRoot,
-			SlotNum:      payloadAttributes.SlotNumber,
-			Version:      payloadVersion,
+			Parent:         update.HeadBlockHash,
+			Timestamp:      payloadAttributes.Timestamp,
+			FeeRecipient:   payloadAttributes.SuggestedFeeRecipient,
+			Random:         payloadAttributes.Random,
+			Withdrawals:    payloadAttributes.Withdrawals,
+			BeaconRoot:     payloadAttributes.BeaconRoot,
+			SlotNum:        payloadAttributes.SlotNumber,
+			TargetGasLimit: payloadAttributes.TargetGasLimit,
+			Version:        payloadVersion,
 		}
 		id := args.Id()
 		// If we already are busy generating this work, then we do not need

eth/catalyst/api_test.go

@@ -2014,3 +2014,45 @@ func runGetBlobs(t testing.TB, getBlobs getBlobsFn, start, limit int, fillRandom
 		t.Fatalf("Unexpected result for case %s", name)
 	}
 }
+
+// TestForkchoiceUpdatedV4TargetGasLimit checks that forkchoiceUpdatedV4
+// rejects payload attributes without the targetGasLimit field, which is
+// required as of PayloadAttributesV4 (execution-apis#796).
+func TestForkchoiceUpdatedV4TargetGasLimit(t *testing.T) {
+	genesis, blocks := generateMergeChain(10, true)
+	n, ethservice := startEthService(t, genesis, blocks)
+	defer n.Close()
+
+	var (
+		api        = newConsensusAPIWithoutHeartbeat(ethservice)
+		parent     = ethservice.BlockChain().CurrentHeader()
+		fcState    = engine.ForkchoiceStateV1{HeadBlockHash: parent.Hash()}
+		beaconRoot = common.Hash{}
+		slotNum    = uint64(1)
+		target     = uint64(300_000_000)
+	)
+	// Missing targetGasLimit must be rejected as invalid payload attributes.
+	attrs := &engine.PayloadAttributes{
+		Timestamp:   parent.Time + 1,
+		Withdrawals: []*types.Withdrawal{},
+		BeaconRoot:  &beaconRoot,
+		SlotNumber:  &slotNum,
+	}
+	_, err := api.ForkchoiceUpdatedV4(context.Background(), fcState, attrs)
+	if err == nil {
+		t.Fatal("expected error for missing targetGasLimit, got none")
+	}
+	if apiErr, ok := err.(*engine.EngineAPIError); !ok || apiErr.ErrorCode() != engine.InvalidPayloadAttributes.ErrorCode() {
+		t.Fatalf("expected invalid payload attributes error for missing targetGasLimit, got %v", err)
+	}
+	// With targetGasLimit present, validation must get past the attribute
+	// checks; on this pre-amsterdam chain it then fails the fork check.
+	attrs.TargetGasLimit = &target
+	_, err = api.ForkchoiceUpdatedV4(context.Background(), fcState, attrs)
+	if err == nil {
+		t.Fatal("expected unsupported fork error, got none")
+	}
+	if apiErr, ok := err.(*engine.EngineAPIError); !ok || apiErr.ErrorCode() != engine.UnsupportedFork.ErrorCode() {
+		t.Fatalf("expected unsupported fork error, got %v", err)
+	}
+}

miner/payload_building.go

@@ -40,14 +40,15 @@ import (
 // Check engine-api specification for more details.
 // https://github.com/ethereum/execution-apis/blob/main/src/engine/cancun.md#payloadattributesv3
 type BuildPayloadArgs struct {
-	Parent       common.Hash           // The parent block to build payload on top
-	Timestamp    uint64                // The provided timestamp of generated payload
-	FeeRecipient common.Address        // The provided recipient address for collecting transaction fee
-	Random       common.Hash           // The provided randomness value
-	Withdrawals  types.Withdrawals     // The provided withdrawals
-	BeaconRoot   *common.Hash          // The provided beaconRoot (Cancun)
-	SlotNum      *uint64               // The provided slotNumber
-	Version      engine.PayloadVersion // Versioning byte for payload id calculation.
+	Parent         common.Hash           // The parent block to build payload on top
+	Timestamp      uint64                // The provided timestamp of generated payload
+	FeeRecipient   common.Address        // The provided recipient address for collecting transaction fee
+	Random         common.Hash           // The provided randomness value
+	Withdrawals    types.Withdrawals     // The provided withdrawals
+	BeaconRoot     *common.Hash          // The provided beaconRoot (Cancun)
+	SlotNum        *uint64               // The provided slotNumber (Amsterdam)
+	TargetGasLimit *uint64               // The provided targetGasLimit (Amsterdam)
+	Version        engine.PayloadVersion // Versioning byte for payload id calculation.
 }
 
 // Id computes an 8-byte identifier by hashing the components of the payload arguments.
@@ -64,6 +65,12 @@ func (args *BuildPayloadArgs) Id() engine.PayloadID {
 	if args.SlotNum != nil {
 		binary.Write(hasher, binary.BigEndian, args.SlotNum)
 	}
+	if args.TargetGasLimit != nil {
+		// The leading tag byte keeps the preimage distinct from an args set
+		// where only slotNum is present, as both encode as 8 big-endian bytes.
+		hasher.Write([]byte{0x01})
+		binary.Write(hasher, binary.BigEndian, args.TargetGasLimit)
+	}
 	var out engine.PayloadID
 	copy(out[:], hasher.Sum(nil)[:8])
 	out[0] = byte(args.Version)
@@ -246,15 +253,16 @@ func (miner *Miner) buildPayload(ctx context.Context, args *BuildPayloadArgs, wi
 	// enough to run. The empty payload can at least make sure there is something
 	// to deliver for not missing slot.
 	emptyParams := &generateParams{
-		timestamp:   args.Timestamp,
-		forceTime:   true,
-		parentHash:  args.Parent,
-		coinbase:    args.FeeRecipient,
-		random:      args.Random,
-		withdrawals: args.Withdrawals,
-		beaconRoot:  args.BeaconRoot,
-		slotNum:     args.SlotNum,
-		noTxs:       true,
+		timestamp:      args.Timestamp,
+		forceTime:      true,
+		parentHash:     args.Parent,
+		coinbase:       args.FeeRecipient,
+		random:         args.Random,
+		withdrawals:    args.Withdrawals,
+		beaconRoot:     args.BeaconRoot,
+		slotNum:        args.SlotNum,
+		targetGasLimit: args.TargetGasLimit,
+		noTxs:          true,
 	}
 	empty := miner.generateWork(ctx, emptyParams, witness)
 	if empty.err != nil {
@@ -286,15 +294,16 @@ func (miner *Miner) buildPayload(ctx context.Context, args *BuildPayloadArgs, wi
 		endTimer := time.NewTimer(time.Second * 12)
 
 		fullParams := &generateParams{
-			timestamp:   args.Timestamp,
-			forceTime:   true,
-			parentHash:  args.Parent,
-			coinbase:    args.FeeRecipient,
-			random:      args.Random,
-			withdrawals: args.Withdrawals,
-			beaconRoot:  args.BeaconRoot,
-			slotNum:     args.SlotNum,
-			noTxs:       false,
+			timestamp:      args.Timestamp,
+			forceTime:      true,
+			parentHash:     args.Parent,
+			coinbase:       args.FeeRecipient,
+			random:         args.Random,
+			withdrawals:    args.Withdrawals,
+			beaconRoot:     args.BeaconRoot,
+			slotNum:        args.SlotNum,
+			targetGasLimit: args.TargetGasLimit,
+			noTxs:          false,
 		}
 		for {
 			select {
@@ -351,6 +360,7 @@ func (miner *Miner) BuildTestingPayload(args *BuildPayloadArgs, transactions []*
 		withdrawals:       args.Withdrawals,
 		beaconRoot:        args.BeaconRoot,
 		slotNum:           args.SlotNum,
+		targetGasLimit:    args.TargetGasLimit,
 		noTxs:             empty,
 		forceOverrides:    true,
 		overrideExtraData: extraData,