Skip to content

Commit

Permalink
op-node/rollup/attributes: Add missing EIP1559Params consolidation ch…
Browse files Browse the repository at this point in the history
…ecks (#14179)

* op-node/rollup/attributes: Add missing EIP1559Params consolidation checks

* all: use OptimismConfig in consolidation to translate zero attribs

* TEST using default config if it cannot be fetched

* op-node/node: add ChainOpConfig override

and use it instead of a default config

* Add ChainConfig's OptimismConfig into rollup.Config

* Fix TestGetRollupConfig

* revert passing OptimismConfig around

it's now part of the rollup.Config
  • Loading branch information
sebastianst authored and maurelian committed Feb 11, 2025
1 parent 3e1d8ff commit dbff55b
Show file tree
Hide file tree
Showing 9 changed files with 358 additions and 111 deletions.
8 changes: 8 additions & 0 deletions op-chain-ops/genesis/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -976,6 +976,13 @@ func (d *DeployConfig) RollupConfig(l1StartBlock *types.Header, l2GenesisBlockHa
if d.SystemConfigProxy == (common.Address{}) {
return nil, errors.New("SystemConfigProxy cannot be address(0)")
}

chainOpConfig := &params.OptimismConfig{
EIP1559Elasticity: d.EIP1559Elasticity,
EIP1559Denominator: d.EIP1559Denominator,
EIP1559DenominatorCanyon: &d.EIP1559DenominatorCanyon,
}

var altDA *rollup.AltDAConfig
if d.UseAltDA {
altDA = &rollup.AltDAConfig{
Expand Down Expand Up @@ -1021,6 +1028,7 @@ func (d *DeployConfig) RollupConfig(l1StartBlock *types.Header, l2GenesisBlockHa
InteropTime: d.InteropTime(l1StartTime),
ProtocolVersionsAddress: d.ProtocolVersionsProxy,
AltDAConfig: altDA,
ChainOpConfig: chainOpConfig,
}, nil
}

Expand Down
8 changes: 7 additions & 1 deletion op-e2e/e2eutils/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/ethereum/go-ethereum/core"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/params"

"github.com/stretchr/testify/require"

Expand All @@ -26,7 +27,7 @@ var testingJWTSecret = [32]byte{123}
func WriteDefaultJWT(t TestingBase) string {
// Sadly the geth node config cannot load JWT secret from memory, it has to be a file
jwtPath := path.Join(t.TempDir(), "jwt_secret")
if err := os.WriteFile(jwtPath, []byte(hexutil.Encode(testingJWTSecret[:])), 0600); err != nil {
if err := os.WriteFile(jwtPath, []byte(hexutil.Encode(testingJWTSecret[:])), 0o600); err != nil {
t.Fatalf("failed to prepare jwt file for geth: %v", err)
}
return jwtPath
Expand Down Expand Up @@ -210,6 +211,11 @@ func Setup(t require.TestingT, deployParams *DeployParams, alloc *AllocParams) *
IsthmusTime: deployConf.IsthmusTime(uint64(deployConf.L1GenesisBlockTimestamp)),
InteropTime: deployConf.InteropTime(uint64(deployConf.L1GenesisBlockTimestamp)),
AltDAConfig: pcfg,
ChainOpConfig: &params.OptimismConfig{
EIP1559Elasticity: deployConf.EIP1559Elasticity,
EIP1559Denominator: deployConf.EIP1559Denominator,
EIP1559DenominatorCanyon: &deployConf.EIP1559DenominatorCanyon,
},
}

require.NoError(t, rollupCfg.Check())
Expand Down
6 changes: 6 additions & 0 deletions op-e2e/system/e2esys/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ func IsthmusSystemConfig(t *testing.T, isthmusTimeOffset *hexutil.Uint64, opts .
cfg.DeployConfig.L2GenesisIsthmusTimeOffset = isthmusTimeOffset
return cfg
}

func writeDefaultJWT(t testing.TB) string {
// Sadly the geth node config cannot load JWT secret from memory, it has to be a file
jwtPath := path.Join(t.TempDir(), "jwt_secret")
Expand Down Expand Up @@ -649,6 +650,11 @@ func (cfg SystemConfig) Start(t *testing.T, startOpts ...StartOption) (*System,
InteropTime: cfg.DeployConfig.InteropTime(uint64(cfg.DeployConfig.L1GenesisBlockTimestamp)),
ProtocolVersionsAddress: cfg.L1Deployments.ProtocolVersionsProxy,
AltDAConfig: rollupAltDAConfig,
ChainOpConfig: &params.OptimismConfig{
EIP1559Elasticity: cfg.DeployConfig.EIP1559Elasticity,
EIP1559Denominator: cfg.DeployConfig.EIP1559Denominator,
EIP1559DenominatorCanyon: &cfg.DeployConfig.EIP1559DenominatorCanyon,
},
}
}
defaultConfig := makeRollupConfig()
Expand Down
19 changes: 15 additions & 4 deletions op-node/chaincfg/chains_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/ethereum-optimism/optimism/op-node/rollup"
"github.com/ethereum-optimism/optimism/op-service/eth"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/params"
"github.com/stretchr/testify/require"
)

Expand All @@ -27,13 +28,20 @@ func TestGetRollupConfig(t *testing.T) {
}

for name, expectedCfg := range configsByName {
gotCfg, err := GetRollupConfig(name)
require.NoError(t, err)

require.Equalf(t, expectedCfg, *gotCfg, "rollup-configs from superchain-registry must match for %v", name)
t.Run(name, func(t *testing.T) {
gotCfg, err := GetRollupConfig(name)
require.NoError(t, err)
require.Equalf(t, expectedCfg, *gotCfg, "rollup-configs from superchain-registry must match for %v", name)
})
}
}

var defaultOpConfig = &params.OptimismConfig{
EIP1559Elasticity: 6,
EIP1559Denominator: 50,
EIP1559DenominatorCanyon: u64Ptr(250),
}

var mainnetCfg = rollup.Config{
Genesis: rollup.Genesis{
L1: eth.BlockID{
Expand Down Expand Up @@ -69,6 +77,7 @@ var mainnetCfg = rollup.Config{
GraniteTime: u64Ptr(1726070401),
HoloceneTime: u64Ptr(1736445601),
ProtocolVersionsAddress: common.HexToAddress("0x8062AbC286f5e7D9428a0Ccb9AbD71e50d93b935"),
ChainOpConfig: defaultOpConfig,
}

var sepoliaCfg = rollup.Config{
Expand Down Expand Up @@ -106,6 +115,7 @@ var sepoliaCfg = rollup.Config{
GraniteTime: u64Ptr(1723478400),
HoloceneTime: u64Ptr(1732633200),
ProtocolVersionsAddress: common.HexToAddress("0x79ADD5713B383DAa0a138d3C4780C7A1804a8090"),
ChainOpConfig: defaultOpConfig,
}

var sepoliaDev0Cfg = rollup.Config{
Expand Down Expand Up @@ -143,6 +153,7 @@ var sepoliaDev0Cfg = rollup.Config{
GraniteTime: u64Ptr(1723046400),
HoloceneTime: u64Ptr(1731682800),
ProtocolVersionsAddress: common.HexToAddress("0x252CbE9517F731C618961D890D534183822dcC8d"),
ChainOpConfig: defaultOpConfig,
}

func u64Ptr(v uint64) *uint64 {
Expand Down
28 changes: 28 additions & 0 deletions op-node/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"io"
"math/big"
gosync "sync"
"sync/atomic"
"time"
Expand All @@ -17,6 +18,7 @@ import (
"github.com/ethereum/go-ethereum"
gethevent "github.com/ethereum/go-ethereum/event"
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/params"

altda "github.com/ethereum-optimism/optimism/op-alt-da"
"github.com/ethereum-optimism/optimism/op-node/metrics"
Expand All @@ -35,6 +37,7 @@ import (
"github.com/ethereum-optimism/optimism/op-service/oppprof"
"github.com/ethereum-optimism/optimism/op-service/retry"
"github.com/ethereum-optimism/optimism/op-service/sources"
"github.com/ethereum-optimism/optimism/op-service/superutil"
)

var ErrAlreadyClosed = errors.New("node is already closed")
Expand Down Expand Up @@ -434,11 +437,36 @@ func (n *OpNode) initL2(ctx context.Context, cfg *Config) error {
} else {
n.safeDB = safedb.Disabled
}

if cfg.Rollup.ChainOpConfig == nil {
chainCfg, err := loadOrFetchChainConfig(ctx, cfg.Rollup.L2ChainID, rpcClient)
if err != nil {
return fmt.Errorf("failed to load or fetch chain config for id %v: %w", cfg.Rollup.L2ChainID, err)
}
cfg.Rollup.ChainOpConfig = chainCfg.Optimism
}

n.l2Driver = driver.NewDriver(n.eventSys, n.eventDrain, &cfg.Driver, &cfg.Rollup, n.l2Source, n.l1Source,
n.beacon, n, n, n.log, n.metrics, cfg.ConfigPersistence, n.safeDB, &cfg.Sync, sequencerConductor, altDA, managedMode)
return nil
}

func loadOrFetchChainConfig(ctx context.Context, id *big.Int, cl client.RPC) (*params.ChainConfig, error) {
if id.IsUint64() {
cfg, err := superutil.LoadOPStackChainConfigFromChainID(id.Uint64())
if err == nil {
return cfg, nil
}
// ignore error, try to fetch chain config in full
}
// if not already recognized, then fetch the chain config manually
var config params.ChainConfig
if err := cl.CallContext(ctx, &config, "debug_chainConfig"); err != nil {
return nil, fmt.Errorf("fetching: %w", err)
}
return &config, nil
}

func (n *OpNode) initRPCServer(cfg *Config) error {
server, err := newRPCServer(&cfg.RPC, &cfg.Rollup, n.l2Source.L2Client, n.l2Driver, n.safeDB, n.log, n.appVersion, n.metrics)
if err != nil {
Expand Down
50 changes: 49 additions & 1 deletion op-node/rollup/attributes/engine_consolidate.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import (

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/consensus/misc/eip1559"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/params"

"github.com/ethereum-optimism/optimism/op-node/rollup"
"github.com/ethereum-optimism/optimism/op-node/rollup/derive"
Expand Down Expand Up @@ -73,6 +75,10 @@ func AttributesMatchBlock(rollupCfg *rollup.Config, attrs *eth.PayloadAttributes
if attrs.SuggestedFeeRecipient != block.FeeRecipient {
return fmt.Errorf("fee recipient data does not match, expected %s but got %s", block.FeeRecipient, attrs.SuggestedFeeRecipient)
}
if err := checkEIP1559ParamsMatch(rollupCfg.ChainOpConfig, attrs.EIP1559Params, block.ExtraData); err != nil {
return err
}

return nil
}

Expand All @@ -91,6 +97,49 @@ func checkParentBeaconBlockRootMatch(attrRoot, blockRoot *common.Hash) error {
return nil
}

func checkEIP1559ParamsMatch(opCfg *params.OptimismConfig, attrParams *eth.Bytes8, blockExtraData []byte) error {
// Note that we can assume that the attributes' eip1559params are non-nil iff Holocene is active
// according to the local rollup config.
if attrParams != nil {
params := (*attrParams)[:]

// The validity checks are necessary because the Decode functions return 0,0 if the inputs are invalid.
// But 0,0 are valid values during derivation, namely, if the SystemConfig doesn't set the parameters yet,
// they are set to 0,0 and then the execution layer must translate them to the pre-Holocene constants.
if err := eip1559.ValidateHolocene1559Params(params); err != nil {
// This would be a critical error, because the attributes are generated by derivation and must be valid.
return fmt.Errorf("invalid attributes EIP1559 parameters: %w", err)
} else if err := eip1559.ValidateHoloceneExtraData(blockExtraData); err != nil {
// This can happen if the unsafe chain contains invalid (in particular, empty) extraData while Holocene
// is active. The extraData field of blocks from sequencer gossip isn't currently checked during import.
return fmt.Errorf("invalid block extraData: %w", err)
}

ad, ae := eip1559.DecodeHolocene1559Params(params)
var translated bool
// Translate 0,0 to the pre-Holocene protocol constants, like the EL does too.
if ad == 0 {
// If attrParams are non-nil, Holocene, and so Canyon, must be active.
ad = *opCfg.EIP1559DenominatorCanyon
ae = opCfg.EIP1559Elasticity
translated = true
}

bd, be := eip1559.DecodeHoloceneExtraData(blockExtraData)
if ad != bd || ae != be {
extraErr := ""
if translated {
extraErr = " (translated from 0,0)"
}
return fmt.Errorf("eip1559 parameters do not match, attributes: %d, %d%s, block: %d, %d", ad, ae, extraErr, bd, be)
}
} else if len(blockExtraData) > 0 {
// When deriving pre-Holocene blocks, the extraData must be empty.
return fmt.Errorf("nil EIP1559Params in attributes but non-nil extraData in block: %v", blockExtraData)
}
return nil
}

// checkWithdrawals checks if the withdrawals list and withdrawalsRoot are as expected in the attributes and block,
// based on the active hard fork.
func checkWithdrawals(rollupCfg *rollup.Config, attrs *eth.PayloadAttributes, block *eth.ExecutionPayload) error {
Expand Down Expand Up @@ -122,7 +171,6 @@ func checkWithdrawals(rollupCfg *rollup.Config, attrs *eth.PayloadAttributes, bl
// bedrock: the withdrawals list should be nil
if attrWithdrawals != nil {
return fmt.Errorf("%w: got %d", ErrBedrockMustHaveEmptyWithdrawals, len(*attrWithdrawals))

}
}

Expand Down
Loading

0 comments on commit dbff55b

Please sign in to comment.