From def753f2f6486c646808805a0f2241972e910f89 Mon Sep 17 00:00:00 2001 From: Roberto Bayardo Date: Sat, 12 Oct 2024 15:54:01 -0700 Subject: [PATCH] Use extraData instead of nonce for 1559 parameters --- consensus/misc/eip1559/eip1559.go | 55 +++++++++++++++++++------ eth/catalyst/api.go | 23 ++++++----- miner/payload_building.go | 4 +- miner/payload_building_test.go | 68 ++++++++++++++++--------------- miner/worker.go | 20 ++++----- 5 files changed, 102 insertions(+), 68 deletions(-) diff --git a/consensus/misc/eip1559/eip1559.go b/consensus/misc/eip1559/eip1559.go index a7195f44a6..7305fa0841 100644 --- a/consensus/misc/eip1559/eip1559.go +++ b/consensus/misc/eip1559/eip1559.go @@ -56,25 +56,52 @@ func VerifyEIP1559Header(config *params.ChainConfig, parent, header *types.Heade return nil } +func ValidateHoloceneExtraData(extra []byte) error { + if len(extra) != 9 { + return fmt.Errorf("holocene extraData should be 9 bytes, got %d", len(extra)) + } + if extra[0] != 0 { + return fmt.Errorf("holocene extraData should have 0 version byte, got %d", extra[0]) + } + return ValidateHoloceneParams(extra[1:]) +} + // DecodeHolocene1599Params extracts the Holcene 1599 parameters from the encoded form: // https://github.com/ethereum-optimism/specs/blob/main/specs/protocol/holocene/exec-engine.md#eip1559params-encoding -func DecodeHolocene1559Params(params types.BlockNonce) (uint64, uint64) { - elasticity := binary.BigEndian.Uint32(params[4:]) +// +// Returns 0,0 if the format is invalid, though ValidateHoloceneParams should be used instead of +// this function for validity checking. +func DecodeHolocene1559Params(params []byte) (uint64, uint64) { + if len(params) != 8 { + return 0, 0 + } denominator := binary.BigEndian.Uint32(params[:4]) - return uint64(elasticity), uint64(denominator) + elasticity := binary.BigEndian.Uint32(params[4:]) + return uint64(denominator), uint64(elasticity) } -func EncodeHolocene1559Params(elasticity, denom uint32) types.BlockNonce { - var nonce types.BlockNonce - binary.BigEndian.PutUint32(nonce[4:], elasticity) - binary.BigEndian.PutUint32(nonce[:4], denom) - return nonce +func EncodeHolocene1559Params(denom, elasticity uint32) []byte { + r := make([]byte, 8) + binary.BigEndian.PutUint32(r[:4], denom) + binary.BigEndian.PutUint32(r[4:], elasticity) + return r +} + +func EncodeHoloceneExtraData(denom, elasticity uint32) []byte { + r := make([]byte, 9) + // leave version byte 0 + binary.BigEndian.PutUint32(r[1:5], denom) + binary.BigEndian.PutUint32(r[5:], elasticity) + return r } // ValidateHoloceneParams checks if the encoded parameters are valid according to the Holocene // upgrade. -func ValidateHoloceneParams(params types.BlockNonce) error { - e, d := DecodeHolocene1559Params(params) +func ValidateHoloceneParams(params []byte) error { + if len(params) != 8 { + return fmt.Errorf("holocene eip-1559 params should be 8 bytes, got %d", len(params)) + } + d, e := DecodeHolocene1559Params(params) if e != 0 && d == 0 { return errors.New("holocene params cannot have a 0 denominator unless elasticity is also 0") } @@ -93,8 +120,12 @@ func CalcBaseFee(config *params.ChainConfig, parent *types.Header, time uint64) if config.IsHolocene(time) { // Holocene requires we get the 1559 parameters from the nonce field of the parent header // unless the field is zero, in which case we use the Canyon values. - if parent.Nonce != types.BlockNonce([8]byte{}) { - elasticity, denominator = DecodeHolocene1559Params(parent.Nonce) + if len(parent.Extra) != 0 { + denominator, elasticity = DecodeHolocene1559Params(parent.Extra) + if denominator == 0 { + // this shouldn't happen as the ExtraData should have been validated prior + panic("invalid eip-1559 params in extradata") + } } } parentGasTarget := parent.GasLimit / elasticity diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index fe27b59d59..8124edebe8 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -437,22 +437,17 @@ func (api *ConsensusAPI) forkchoiceUpdated(update engine.ForkchoiceStateV1, payl // will replace it arbitrarily many times in between. if payloadAttributes != nil { - var nonce *types.BlockNonce + var eip1559Params []byte if api.eth.BlockChain().Config().Optimism != nil { if payloadAttributes.GasLimit == nil { return engine.STATUS_INVALID, engine.InvalidPayloadAttributes.With(errors.New("gasLimit parameter is required")) } if api.eth.BlockChain().Config().IsHolocene(payloadAttributes.Timestamp) { - var params types.BlockNonce - copy(params[:], payloadAttributes.EIP1559Params) - if len(payloadAttributes.EIP1559Params) != 8 { - return engine.STATUS_INVALID, - engine.InvalidPayloadAttributes.With(errors.New("eip1559Params is required when Holocene is active")) - } - if err := eip1559.ValidateHoloceneParams(params); err != nil { + if err := eip1559.ValidateHoloceneParams(payloadAttributes.EIP1559Params); err != nil { return engine.STATUS_INVALID, engine.InvalidPayloadAttributes.With(err) } - nonce = ¶ms + eip1559Params = make([]byte, 8) + copy(eip1559Params, payloadAttributes.EIP1559Params) } else if len(payloadAttributes.EIP1559Params) != 0 { return engine.STATUS_INVALID, engine.InvalidPayloadAttributes.With(errors.New("eip155Params not supported prior to Holocene upgrade")) @@ -477,7 +472,7 @@ func (api *ConsensusAPI) forkchoiceUpdated(update engine.ForkchoiceStateV1, payl Transactions: transactions, GasLimit: payloadAttributes.GasLimit, Version: payloadVersion, - EIP1559Params: nonce, + EIP1559Params: eip1559Params, } id := args.Id() // If we already are busy generating this work, then we do not need @@ -846,6 +841,14 @@ func (api *ConsensusAPI) newPayload(params engine.ExecutableData, versionedHashe // sequentially. // Hence, we use a lock here, to be sure that the previous call has finished before we // check whether we already have the block locally. + + // Payload must have eip-1559 params in ExtraData after Holocene + if api.eth.BlockChain().Config().IsHolocene(params.Timestamp) { + if err := eip1559.ValidateHoloceneExtraData(params.ExtraData); err != nil { + return api.invalid(err, nil), nil + } + } + api.newPayloadLock.Lock() defer api.newPayloadLock.Unlock() diff --git a/miner/payload_building.go b/miner/payload_building.go index eef8e62021..30ecfff720 100644 --- a/miner/payload_building.go +++ b/miner/payload_building.go @@ -50,7 +50,7 @@ type BuildPayloadArgs struct { NoTxPool bool // Optimism addition: option to disable tx pool contents from being included Transactions []*types.Transaction // Optimism addition: txs forced into the block via engine API GasLimit *uint64 // Optimism addition: override gas limit of the block to build - EIP1559Params *types.BlockNonce // Optimism addition: encodes Holocene EIP-1559 params + EIP1559Params []byte // Optimism addition: encodes Holocene EIP-1559 params } // Id computes an 8-byte identifier by hashing the components of the payload arguments. @@ -76,7 +76,7 @@ func (args *BuildPayloadArgs) Id() engine.PayloadID { if args.GasLimit != nil { binary.Write(hasher, binary.BigEndian, *args.GasLimit) } - if args.EIP1559Params != nil { + if len(args.EIP1559Params) != 0 { hasher.Write(args.EIP1559Params[:]) } diff --git a/miner/payload_building_test.go b/miner/payload_building_test.go index 62b9f0267e..6564cd759d 100644 --- a/miner/payload_building_test.go +++ b/miner/payload_building_test.go @@ -17,6 +17,7 @@ package miner import ( + "bytes" "math/big" "reflect" "testing" @@ -156,16 +157,16 @@ func TestBuildPayload(t *testing.T) { // the builder routine t.Run("with-tx-pool", func(t *testing.T) { testBuildPayload(t, false, false, nil) }) t.Run("with-tx-pool-interrupt", func(t *testing.T) { testBuildPayload(t, false, true, nil) }) - nonce := types.BlockNonce([8]byte{0, 1, 2, 3, 4, 5, 6, 7}) - t.Run("with-nonce", func(t *testing.T) { testBuildPayload(t, false, false, &nonce) }) - t.Run("with-nonce-no-tx-pool", func(t *testing.T) { testBuildPayload(t, true, false, &nonce) }) - t.Run("with-nonce-interrupt", func(t *testing.T) { testBuildPayload(t, false, true, &nonce) }) + params1559 := []byte{0, 1, 2, 3, 4, 5, 6, 7} + t.Run("with-params", func(t *testing.T) { testBuildPayload(t, false, false, params1559) }) + t.Run("with-params-no-tx-pool", func(t *testing.T) { testBuildPayload(t, true, false, params1559) }) + t.Run("with-params-interrupt", func(t *testing.T) { testBuildPayload(t, false, true, params1559) }) - t.Run("wrong-config-no-nonce", func(t *testing.T) { testBuildPayloadWrongConfig(t, nil) }) - t.Run("wrong-config-nonce", func(t *testing.T) { testBuildPayloadWrongConfig(t, &nonce) }) + t.Run("wrong-config-no-params", func(t *testing.T) { testBuildPayloadWrongConfig(t, nil) }) + t.Run("wrong-config-params", func(t *testing.T) { testBuildPayloadWrongConfig(t, params1559) }) - var zeroNonce types.BlockNonce - t.Run("with-zero-nonce", func(t *testing.T) { testBuildPayload(t, true, false, &zeroNonce) }) + zeroParams := make([]byte, 8) + t.Run("with-zero-params", func(t *testing.T) { testBuildPayload(t, true, false, zeroParams) }) } func holoceneConfig() *params.ChainConfig { @@ -183,24 +184,24 @@ func holoceneConfig() *params.ChainConfig { return &config } -// newPayloadArgs returns a BuildPaylooadArgs with the given parentHash and nonce, testTimestamp -// for Timestamp, and testRecipient for recipient. NoTxPool is set to true. -func newPayloadArgs(parentHash common.Hash, nonce *types.BlockNonce) *BuildPayloadArgs { +// newPayloadArgs returns a BuildPaylooadArgs with the given parentHash and eip-1559 params, +// testTimestamp for Timestamp, and testRecipient for recipient. NoTxPool is set to true. +func newPayloadArgs(parentHash common.Hash, params1559 []byte) *BuildPayloadArgs { return &BuildPayloadArgs{ Parent: parentHash, Timestamp: testTimestamp, Random: common.Hash{}, FeeRecipient: testRecipient, NoTxPool: true, - EIP1559Params: nonce, + EIP1559Params: params1559, } } -func testBuildPayload(t *testing.T, noTxPool, interrupt bool, nonce *types.BlockNonce) { +func testBuildPayload(t *testing.T, noTxPool, interrupt bool, params1559 []byte) { t.Parallel() db := rawdb.NewMemoryDatabase() config := params.TestChainConfig - if nonce != nil { + if len(params1559) != 0 { config = holoceneConfig() } w, b := newTestWorker(t, config, ethash.NewFaker(), db, 0) @@ -213,7 +214,7 @@ func testBuildPayload(t *testing.T, noTxPool, interrupt bool, nonce *types.Block b.txPool.Add(txs, true, false) } - args := newPayloadArgs(b.chain.CurrentBlock().Hash(), nonce) + args := newPayloadArgs(b.chain.CurrentBlock().Hash(), params1559) args.NoTxPool = noTxPool // payload resolution now interrupts block building, so we have to @@ -247,21 +248,22 @@ func testBuildPayload(t *testing.T, noTxPool, interrupt bool, nonce *types.Block } } - // make sure the nonce we've specied (if any) ends up in both the full and empty block headers - var expected types.BlockNonce - if nonce != nil { - if *nonce == expected { - expected = eip1559.EncodeHolocene1559Params(6, 250) // canyon defaults + // make sure the 1559 params we've specied (if any) ends up in both the full and empty block headers + var expected []byte + if len(params1559) != 0 { + expected = []byte{0} + d, _ := eip1559.DecodeHolocene1559Params(params1559) + if d == 0 { + expected = append(expected, eip1559.EncodeHolocene1559Params(250, 6)...) // canyon defaults } else { - expected = *nonce + expected = append(expected, params1559...) } } - t.Logf("expected nonce: %x\n", expected[:]) - if payload.full != nil && payload.full.Header().Nonce != expected { - t.Fatalf("Nonces don't match. want: %x, got %x", expected, payload.full.Header().Nonce) + if payload.full != nil && !bytes.Equal(payload.full.Header().Extra, expected) { + t.Fatalf("ExtraData doesn't match. want: %x, got %x", expected, payload.full.Header().Extra) } - if payload.empty != nil && payload.empty.Header().Nonce != expected { - t.Fatalf("Nonces don't match on empty block. want: %x, got %x", expected, payload.empty.Header().Nonce) + if payload.empty != nil && !bytes.Equal(payload.empty.Header().Extra, expected) { + t.Fatalf("ExtraData doesn't match on empty block. want: %x, got %x", expected, payload.empty.Header().Extra) } if noTxPool { @@ -288,33 +290,33 @@ func testBuildPayload(t *testing.T, noTxPool, interrupt bool, nonce *types.Block } } -func testBuildPayloadWrongConfig(t *testing.T, nonce *types.BlockNonce) { +func testBuildPayloadWrongConfig(t *testing.T, params1559 []byte) { t.Parallel() db := rawdb.NewMemoryDatabase() config := holoceneConfig() - if nonce != nil { - // deactivate holocene and make sure non-nil nonce gets rejected + if len(params1559) != 0 { + // deactivate holocene and make sure non-empty params get rejected config.HoloceneTime = nil } w, b := newTestWorker(t, config, ethash.NewFaker(), db, 0) - args := newPayloadArgs(b.chain.CurrentBlock().Hash(), nonce) + args := newPayloadArgs(b.chain.CurrentBlock().Hash(), params1559) payload, err := w.buildPayload(args, false) if err == nil && (payload == nil || payload.err == nil) { t.Fatalf("expected error, got none") } } -func TestBuildPayloadInvalidHoloceneNonce(t *testing.T) { +func TestBuildPayloadInvalidHoloceneParams(t *testing.T) { t.Parallel() db := rawdb.NewMemoryDatabase() config := holoceneConfig() w, b := newTestWorker(t, config, ethash.NewFaker(), db, 0) // 0 denominators shouldn't be allowed - badNonce := eip1559.EncodeHolocene1559Params(6, 0) + badParams := eip1559.EncodeHolocene1559Params(0, 6) - args := newPayloadArgs(b.chain.CurrentBlock().Hash(), &badNonce) + args := newPayloadArgs(b.chain.CurrentBlock().Hash(), badParams) payload, err := w.buildPayload(args, false) if err == nil && (payload == nil || payload.err == nil) { t.Fatalf("expected error, got none") diff --git a/miner/worker.go b/miner/worker.go index f1a3cf724e..6e94ac1d5a 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -109,7 +109,7 @@ type generateParams struct { txs types.Transactions // Deposit transactions to include at the start of the block gasLimit *uint64 // Optional gas limit override - eip1559Params *types.BlockNonce // Optional EIP-1559 parameters + eip1559Params []byte // Optional EIP-1559 parameters interrupt *atomic.Int32 // Optional interruption signal to pass down to worker.generateWork isUpdate bool // Optional flag indicating that this is building a discardable update } @@ -229,7 +229,8 @@ func (miner *Miner) prepareWork(genParams *generateParams, witness bool) (*envir Coinbase: genParams.coinbase, } // Set the extra field. - if len(miner.config.ExtraData) != 0 && miner.chainConfig.Optimism == nil { // Optimism chains must not set any extra data. + if len(miner.config.ExtraData) != 0 && miner.chainConfig.Optimism == nil { + // Optimism chains have their own ExtraData handling rules header.Extra = miner.config.ExtraData } // Set the randomness field from the beacon chain if it's available. @@ -251,20 +252,17 @@ func (miner *Miner) prepareWork(genParams *generateParams, witness bool) (*envir header.GasLimit = miner.config.GasCeil } if miner.chainConfig.IsHolocene(header.Time) { - if genParams.eip1559Params == nil { - return nil, errors.New("expected eip1559 params, got none") - } - if err := eip1559.ValidateHoloceneParams(*genParams.eip1559Params); err != nil { + if err := eip1559.ValidateHoloceneParams(genParams.eip1559Params); err != nil { return nil, err } - header.Nonce = *genParams.eip1559Params // If this is a holocene block and the params are 0, we must convert them to their Canyon // defaults in the header. - if header.Nonce == types.BlockNonce([8]byte{}) { - elasticity := miner.chainConfig.ElasticityMultiplier() - denominator := miner.chainConfig.BaseFeeChangeDenominator(header.Time) - header.Nonce = eip1559.EncodeHolocene1559Params(uint32(elasticity), uint32(denominator)) + d, e := eip1559.DecodeHolocene1559Params(genParams.eip1559Params) + if d == 0 { + d = miner.chainConfig.BaseFeeChangeDenominator(header.Time) + e = miner.chainConfig.ElasticityMultiplier() } + header.Extra = eip1559.EncodeHoloceneExtraData(uint32(d), uint32(e)) } else if genParams.eip1559Params != nil { return nil, errors.New("got eip1559 params, expected none") }