Skip to content

Commit

Permalink
Merge pull request ethereum-optimism#402 from roberto-bayardo/eip1559…
Browse files Browse the repository at this point in the history
…params-in-execution-payload

Holocene: use extraData instead of nonce for eip-1559 parameters
  • Loading branch information
tynes authored Oct 17, 2024
2 parents 933707a + 6c32375 commit a7d3295
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 89 deletions.
69 changes: 53 additions & 16 deletions consensus/misc/eip1559/eip1559.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
28 changes: 13 additions & 15 deletions consensus/misc/eip1559/eip1559_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -189,31 +189,29 @@ 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{
Number: common.Big32,
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)
Expand Down
23 changes: 13 additions & 10 deletions eth/catalyst/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package catalyst

import (
"bytes"
"errors"
"fmt"
"strconv"
Expand Down Expand Up @@ -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 = &params
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"))
Expand All @@ -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
Expand Down Expand Up @@ -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()

Expand Down
4 changes: 2 additions & 2 deletions miner/payload_building.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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[:])
}

Expand Down
72 changes: 39 additions & 33 deletions miner/payload_building_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package miner

import (
"bytes"
"math/big"
"reflect"
"testing"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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")
Expand Down
Loading

0 comments on commit a7d3295

Please sign in to comment.