From 6c32375dda12d3f0b8f3498404f00fe1ae872547 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 | 69 ++++++++++++++++++------ consensus/misc/eip1559/eip1559_test.go | 28 +++++----- eth/catalyst/api.go | 23 ++++---- miner/payload_building.go | 4 +- miner/payload_building_test.go | 72 ++++++++++++++------------ miner/worker.go | 24 ++++----- 6 files changed, 131 insertions(+), 89 deletions(-) diff --git a/consensus/misc/eip1559/eip1559.go b/consensus/misc/eip1559/eip1559.go index a7195f44a6..08a88c5ff5 100644 --- a/consensus/misc/eip1559/eip1559.go +++ b/consensus/misc/eip1559/eip1559.go @@ -58,29 +58,66 @@ func VerifyEIP1559Header(config *params.ChainConfig, parent, header *types.Heade // 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 ValidateHolocene1559Params 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 +// Decodes holocene extra data without performing full validation. +func DecodeHoloceneExtraData(extra []byte) (uint64, uint64) { + if len(extra) != 9 { + return 0, 0 + } + return DecodeHolocene1559Params(extra[1:]) +} + +func EncodeHolocene1559Params(denom, elasticity uint32) []byte { + r := make([]byte, 8) + binary.BigEndian.PutUint32(r[:4], denom) + binary.BigEndian.PutUint32(r[4:], elasticity) + return r } -// ValidateHoloceneParams checks if the encoded parameters are valid according to the Holocene +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 +} + +// ValidateHolocene1559Params checks if the encoded parameters are valid according to the Holocene // upgrade. -func ValidateHoloceneParams(params types.BlockNonce) error { - e, d := DecodeHolocene1559Params(params) +func ValidateHolocene1559Params(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") } return nil } +// ValidateHoloceneExtraData checks if the header extraData is valid according to the Holocene +// upgrade. +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 ValidateHolocene1559Params(extra[1:]) +} + // CalcBaseFee calculates the basefee of the header. // The time belongs to the new block to check which upgrades are active. func CalcBaseFee(config *params.ChainConfig, parent *types.Header, time uint64) *big.Int { @@ -90,11 +127,11 @@ func CalcBaseFee(config *params.ChainConfig, parent *types.Header, time uint64) } elasticity := config.ElasticityMultiplier() denominator := config.BaseFeeChangeDenominator(time) - 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 config.IsHolocene(parent.Time) { + denominator, elasticity = DecodeHoloceneExtraData(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/consensus/misc/eip1559/eip1559_test.go b/consensus/misc/eip1559/eip1559_test.go index a58e4eb480..f9cd1f40a2 100644 --- a/consensus/misc/eip1559/eip1559_test.go +++ b/consensus/misc/eip1559/eip1559_test.go @@ -177,8 +177,8 @@ func TestCalcBaseFeeOptimism(t *testing.T) { t.Errorf("test %d: have %d want %d, ", i, have, want) } if test.postCanyon { - // make sure Holocene activation doesn't change the outcome; since these tests have a - // zero nonce, they should be handled using the Canyon config. + // make sure Holocene activation doesn't change the outcome; since these tests have empty eip1559 params, + // they should be handled using the Canyon config. parent.Time = 10 if have, want := CalcBaseFee(opConfig(), parent, parent.Time+2), big.NewInt(test.expectedBaseFee); have.Cmp(want) != 0 { t.Errorf("test %d: have %d want %d, ", i, have, want) @@ -189,22 +189,20 @@ func TestCalcBaseFeeOptimism(t *testing.T) { // TestCalcBaseFeeHolocene assumes all blocks are Optimism blocks post-Holocene upgrade func TestCalcBaseFeeOptimismHolocene(t *testing.T) { - elasticity2Denom10Nonce := EncodeHolocene1559Params(2, 10) - elasticity10Denom2Nonce := EncodeHolocene1559Params(10, 2) parentBaseFee := int64(10_000_000) parentGasLimit := uint64(30_000_000) tests := []struct { - parentGasUsed uint64 - expectedBaseFee int64 - nonce types.BlockNonce + parentGasUsed uint64 + expectedBaseFee int64 + denom, elasticity uint32 }{ - {parentGasLimit / 2, parentBaseFee, elasticity2Denom10Nonce}, // target - {10_000_000, 9_666_667, elasticity2Denom10Nonce}, // below - {20_000_000, 10_333_333, elasticity2Denom10Nonce}, // above - {parentGasLimit / 10, parentBaseFee, elasticity10Denom2Nonce}, // target - {1_000_000, 6_666_667, elasticity10Denom2Nonce}, // below - {30_000_000, 55_000_000, elasticity10Denom2Nonce}, // above + {parentGasLimit / 2, parentBaseFee, 10, 2}, // target + {10_000_000, 9_666_667, 10, 2}, // below + {20_000_000, 10_333_333, 10, 2}, // above + {parentGasLimit / 10, parentBaseFee, 2, 10}, // target + {1_000_000, 6_666_667, 2, 10}, // below + {30_000_000, 55_000_000, 2, 10}, // above } for i, test := range tests { parent := &types.Header{ @@ -212,8 +210,8 @@ func TestCalcBaseFeeOptimismHolocene(t *testing.T) { GasLimit: parentGasLimit, GasUsed: test.parentGasUsed, BaseFee: big.NewInt(parentBaseFee), - Time: 10, - Nonce: test.nonce, + Time: 12, + Extra: EncodeHoloceneExtraData(test.denom, test.elasticity), } if have, want := CalcBaseFee(opConfig(), parent, parent.Time+2), big.NewInt(test.expectedBaseFee); have.Cmp(want) != 0 { t.Errorf("test %d: have %d want %d, ", i, have, want) diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index fe27b59d59..5200dae2fc 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -18,6 +18,7 @@ package catalyst import ( + "bytes" "errors" "fmt" "strconv" @@ -437,22 +438,16 @@ 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.ValidateHolocene1559Params(payloadAttributes.EIP1559Params); err != nil { return engine.STATUS_INVALID, engine.InvalidPayloadAttributes.With(err) } - nonce = ¶ms + eip1559Params = bytes.Clone(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..bf76a2040e 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" @@ -125,6 +126,10 @@ func newTestWorkerBackend(t *testing.T, chainConfig *params.ChainConfig, engine default: t.Fatalf("unexpected consensus engine type: %T", engine) } + if chainConfig.HoloceneTime != nil { + // genesis block extraData needs to be correct format + gspec.ExtraData = []byte{0, 0, 1, 2, 3, 4, 5, 6, 7} + } chain, err := core.NewBlockChain(db, &core.CacheConfig{TrieDirtyDisabled: true}, gspec, nil, engine, vm.Config{}, nil) if err != nil { t.Fatalf("core.NewBlockChain failed: %v", err) @@ -156,16 +161,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 +188,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 +218,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 +252,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 +294,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..ac9588e821 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.ValidateHolocene1559Params(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)) + // If this is a holocene block and the params are 0, we must convert them to their previous + // constants in the header. + 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") }