diff --git a/beacon/engine/pa_codec.go b/beacon/engine/pa_codec.go index 3a5d7ae593..3d28897b0e 100644 --- a/beacon/engine/pa_codec.go +++ b/beacon/engine/pa_codec.go @@ -22,6 +22,7 @@ func (p PayloadAttributes) MarshalJSON() ([]byte, error) { Withdrawals []*types.Withdrawal `json:"withdrawals"` BeaconRoot *common.Hash `json:"parentBeaconBlockRoot"` SlotNumber *hexutil.Uint64 `json:"slotNumber"` + TargetGasLimit *hexutil.Uint64 `json:"targetGasLimit"` } var enc PayloadAttributes enc.Timestamp = hexutil.Uint64(p.Timestamp) @@ -30,6 +31,7 @@ func (p PayloadAttributes) MarshalJSON() ([]byte, error) { enc.Withdrawals = p.Withdrawals enc.BeaconRoot = p.BeaconRoot enc.SlotNumber = (*hexutil.Uint64)(p.SlotNumber) + enc.TargetGasLimit = (*hexutil.Uint64)(p.TargetGasLimit) return json.Marshal(&enc) } @@ -42,6 +44,7 @@ func (p *PayloadAttributes) UnmarshalJSON(input []byte) error { Withdrawals []*types.Withdrawal `json:"withdrawals"` BeaconRoot *common.Hash `json:"parentBeaconBlockRoot"` SlotNumber *hexutil.Uint64 `json:"slotNumber"` + TargetGasLimit *hexutil.Uint64 `json:"targetGasLimit"` } var dec PayloadAttributes if err := json.Unmarshal(input, &dec); err != nil { @@ -68,5 +71,8 @@ func (p *PayloadAttributes) UnmarshalJSON(input []byte) error { if dec.SlotNumber != nil { p.SlotNumber = (*uint64)(dec.SlotNumber) } + if dec.TargetGasLimit != nil { + p.TargetGasLimit = (*uint64)(dec.TargetGasLimit) + } return nil } diff --git a/beacon/engine/types.go b/beacon/engine/types.go index e536747882..344c48e9d3 100644 --- a/beacon/engine/types.go +++ b/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 diff --git a/beacon/engine/types_test.go b/beacon/engine/types_test.go index 5716a19627..ab0e81028f 100644 --- a/beacon/engine/types_test.go +++ b/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) + } +} diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index beecc7c744..01d8d00398 100644 --- a/eth/catalyst/api.go +++ b/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 diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index 849c123979..a7f09eb4bf 100644 --- a/eth/catalyst/api_test.go +++ b/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) + } +} diff --git a/miner/payload_building.go b/miner/payload_building.go index c4322e3317..456910cf79 100644 --- a/miner/payload_building.go +++ b/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, diff --git a/miner/payload_building_test.go b/miner/payload_building_test.go index 734a5605ba..8e5b4860ec 100644 --- a/miner/payload_building_test.go +++ b/miner/payload_building_test.go @@ -249,8 +249,72 @@ func TestBuildPayloadAmsterdamTransition(t *testing.T) { } } +// TestBuildPayloadTargetGasLimit verifies that the CL-provided target gas +// limit (engine API PayloadAttributesV4 targetGasLimit) steers the gas limit +// of locally built payloads, and that the locally configured gas ceiling +// still applies when no target is provided. +func TestBuildPayloadTargetGasLimit(t *testing.T) { + config := new(params.ChainConfig) + *config = *params.MergedTestChainConfig + config.AmsterdamTime = new(uint64) + *config.AmsterdamTime = 1 + + newUint64 := func(v uint64) *uint64 { return &v } + for _, tt := range []struct { + name string + target *uint64 + want func(parentGasLimit uint64) uint64 + }{ + { + // Target above the parent gas limit: move up towards it. + name: "target-above-parent", + target: newUint64(2 * params.GenesisGasLimit), + want: func(parent uint64) uint64 { return core.CalcGasLimit(parent, 2*params.GenesisGasLimit) }, + }, + { + // Target below the parent gas limit: move down towards it. + name: "target-below-parent", + target: newUint64(params.MinGasLimit), + want: func(parent uint64) uint64 { return core.CalcGasLimit(parent, params.MinGasLimit) }, + }, + { + // No target provided: fall back to the configured gas ceiling. + name: "no-target-falls-back-to-gas-ceil", + target: nil, + want: func(parent uint64) uint64 { return core.CalcGasLimit(parent, testConfig.GasCeil) }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + var ( + db = rawdb.NewMemoryDatabase() + beaconRoot = common.Hash{0x01} + slotNum = uint64(1) + ) + w, b := newTestWorker(t, config, beacon.New(ethash.NewFaker()), db, 0) + parent := b.chain.CurrentBlock() + + payload, err := w.buildPayload(context.Background(), &BuildPayloadArgs{ + Parent: parent.Hash(), + Timestamp: 1, + FeeRecipient: common.HexToAddress("0xdeadbeef"), + Withdrawals: types.Withdrawals{}, + BeaconRoot: &beaconRoot, + SlotNum: &slotNum, + TargetGasLimit: tt.target, + }, false) + if err != nil { + t.Fatalf("failed to build payload: %v", err) + } + if got, want := payload.empty.GasLimit(), tt.want(parent.GasLimit); got != want { + t.Fatalf("gas limit mismatch: got %d, want %d (parent %d)", got, want, parent.GasLimit) + } + }) + } +} + func TestPayloadId(t *testing.T) { t.Parallel() + newUint64 := func(v uint64) *uint64 { return &v } ids := make(map[string]int) for i, tt := range []*BuildPayloadArgs{ { @@ -317,6 +381,41 @@ func TestPayloadId(t *testing.T) { }, }, }, + // With slot number + { + Parent: common.Hash{2}, + Timestamp: 2, + Random: common.Hash{0x2}, + FeeRecipient: common.Address{0x2}, + SlotNum: newUint64(5), + }, + // With target gas limit only, numerically equal to the slot number of + // the previous case; the id must still be distinct. + { + Parent: common.Hash{2}, + Timestamp: 2, + Random: common.Hash{0x2}, + FeeRecipient: common.Address{0x2}, + TargetGasLimit: newUint64(5), + }, + // With both slot number and target gas limit + { + Parent: common.Hash{2}, + Timestamp: 2, + Random: common.Hash{0x2}, + FeeRecipient: common.Address{0x2}, + SlotNum: newUint64(5), + TargetGasLimit: newUint64(300000000), + }, + // Different target gas limit + { + Parent: common.Hash{2}, + Timestamp: 2, + Random: common.Hash{0x2}, + FeeRecipient: common.Address{0x2}, + SlotNum: newUint64(5), + TargetGasLimit: newUint64(150000000), + }, } { id := tt.Id().String() if prev, exists := ids[id]; exists { diff --git a/miner/worker.go b/miner/worker.go index 7f0e11f30a..da4f31c7ce 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -117,15 +117,16 @@ type newPayloadResult struct { // generateParams wraps various settings for generating sealing task. type generateParams struct { - timestamp uint64 // The timestamp for sealing task - forceTime bool // Flag whether the given timestamp is immutable or not - parentHash common.Hash // Parent block hash, empty means the latest chain head - coinbase common.Address // The fee recipient address for including transaction - random common.Hash // The randomness generated by beacon chain, empty before the merge - withdrawals types.Withdrawals // List of withdrawals to include in block (shanghai field) - beaconRoot *common.Hash // The beacon root (cancun field). - slotNum *uint64 // The slot number (amsterdam field). - noTxs bool // Flag whether an empty block without any transaction is expected + timestamp uint64 // The timestamp for sealing task + forceTime bool // Flag whether the given timestamp is immutable or not + parentHash common.Hash // Parent block hash, empty means the latest chain head + coinbase common.Address // The fee recipient address for including transaction + random common.Hash // The randomness generated by beacon chain, empty before the merge + withdrawals types.Withdrawals // List of withdrawals to include in block (shanghai field) + beaconRoot *common.Hash // The beacon root (cancun field). + slotNum *uint64 // The slot number (amsterdam field). + targetGasLimit *uint64 // The gas limit to aim for, provided by the CL (amsterdam field). + noTxs bool // Flag whether an empty block without any transaction is expected forceOverrides bool // Flag whether we should overwrite extraData and transactions overrideExtraData []byte @@ -267,11 +268,18 @@ func (miner *Miner) prepareWork(ctx context.Context, genParams *generateParams, } timestamp = parent.Time + 1 } + // Determine the gas limit to aim for. Post-amsterdam the consensus layer + // supplies it with each payload request (engine API targetGasLimit); if it + // is absent, fall back to the locally configured gas ceiling. + gasCeil := miner.config.GasCeil + if genParams.targetGasLimit != nil { + gasCeil = *genParams.targetGasLimit + } // Construct the sealing block header. header := &types.Header{ ParentHash: parent.Hash(), Number: new(big.Int).Add(parent.Number, common.Big1), - GasLimit: core.CalcGasLimit(parent.GasLimit, miner.config.GasCeil), + GasLimit: core.CalcGasLimit(parent.GasLimit, gasCeil), Time: timestamp, Coinbase: genParams.coinbase, } @@ -291,7 +299,7 @@ func (miner *Miner) prepareWork(ctx context.Context, genParams *generateParams, header.BaseFee = eip1559.CalcBaseFee(miner.chainConfig, parent) if !miner.chainConfig.IsLondon(parent.Number) { parentGasLimit := parent.GasLimit * miner.chainConfig.ElasticityMultiplier() - header.GasLimit = core.CalcGasLimit(parentGasLimit, miner.config.GasCeil) + header.GasLimit = core.CalcGasLimit(parentGasLimit, gasCeil) } } // Run the consensus preparation with the default or customized consensus engine.