From bd8f6db5caffe5ed1c28d964d7dd2c548e74809c Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Thu, 14 Nov 2024 21:26:59 -0500 Subject: [PATCH 01/14] op-deployer: add config-intent-type --- op-deployer/pkg/deployer/flags.go | 13 ++ op-deployer/pkg/deployer/init.go | 52 +----- op-deployer/pkg/deployer/pipeline/opchain.go | 2 +- op-deployer/pkg/deployer/standard/standard.go | 2 +- op-deployer/pkg/deployer/state/intent.go | 150 ++++++++++++++---- 5 files changed, 142 insertions(+), 77 deletions(-) diff --git a/op-deployer/pkg/deployer/flags.go b/op-deployer/pkg/deployer/flags.go index 2611957f0ad..853ad09b7f6 100644 --- a/op-deployer/pkg/deployer/flags.go +++ b/op-deployer/pkg/deployer/flags.go @@ -20,6 +20,7 @@ const ( OutdirFlagName = "outdir" PrivateKeyFlagName = "private-key" DeploymentStrategyFlagName = "deployment-strategy" + IntentConfigTypeFlagName = "intent-config-type" ) var ( @@ -62,6 +63,17 @@ var ( EnvVars: PrefixEnvVar("DEPLOYMENT_STRATEGY"), Value: string(state.DeploymentStrategyLive), } + IntentConfigTypeFlag = &cli.StringFlag{ + Name: IntentConfigTypeFlagName, + Usage: fmt.Sprintf("Intent config type to use. Options: %s (default), %s, %s, %s, %s", + state.IntentConfigTypeStandard, + state.IntentConfigTypeCustom, + state.IntentConfigTypeStrict, + state.IntentConfigTypeStandardOverrides, + state.IntentConfigTypeStrictOverrides), + EnvVars: PrefixEnvVar("INTENT_CONFIG_TYPE"), + Value: string(state.IntentConfigTypeStandard), + } ) var GlobalFlags = append([]cli.Flag{}, oplog.CLIFlags(EnvVarPrefix)...) @@ -71,6 +83,7 @@ var InitFlags = []cli.Flag{ L2ChainIDsFlag, WorkdirFlag, DeploymentStrategyFlag, + IntentConfigTypeFlag, } var ApplyFlags = []cli.Flag{ diff --git a/op-deployer/pkg/deployer/init.go b/op-deployer/pkg/deployer/init.go index 3a8ae27d2ae..4c0d6c930a5 100644 --- a/op-deployer/pkg/deployer/init.go +++ b/op-deployer/pkg/deployer/init.go @@ -7,19 +7,17 @@ import ( "path" "strings" - "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/artifacts" - "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/state" op_service "github.com/ethereum-optimism/optimism/op-service" - "github.com/ethereum-optimism/optimism/op-chain-ops/devkeys" "github.com/ethereum/go-ethereum/common" "github.com/urfave/cli/v2" ) type InitConfig struct { DeploymentStrategy state.DeploymentStrategy + IntentConfigType state.IntentConfigType L1ChainID uint64 Outdir string L2ChainIDs []common.Hash @@ -51,6 +49,7 @@ func InitCLI() func(ctx *cli.Context) error { l1ChainID := ctx.Uint64(L1ChainIDFlagName) outdir := ctx.String(OutdirFlagName) l2ChainIDsRaw := ctx.String(L2ChainIDsFlagName) + intentConfigType := ctx.String(IntentConfigTypeFlagName) if len(l2ChainIDsRaw) == 0 { return fmt.Errorf("must specify at least one L2 chain ID") @@ -68,6 +67,7 @@ func InitCLI() func(ctx *cli.Context) error { err := Init(InitConfig{ DeploymentStrategy: state.DeploymentStrategy(deploymentStrategy), + IntentConfigType: state.IntentConfigType(intentConfigType), L1ChainID: l1ChainID, Outdir: outdir, L2ChainIDs: l2ChainIDs, @@ -88,52 +88,14 @@ func Init(cfg InitConfig) error { intent := &state.Intent{ DeploymentStrategy: cfg.DeploymentStrategy, + IntentConfigType: cfg.IntentConfigType, L1ChainID: cfg.L1ChainID, - FundDevAccounts: true, - L1ContractsLocator: artifacts.DefaultL1ContractsLocator, - L2ContractsLocator: artifacts.DefaultL2ContractsLocator, } - l1ChainIDBig := intent.L1ChainIDBig() - - dk, err := devkeys.NewMnemonicDevKeys(devkeys.TestMnemonic) - if err != nil { - return fmt.Errorf("failed to create dev keys: %w", err) - } - - addrFor := func(key devkeys.Key) common.Address { - // The error below should never happen, so panic if it does. - addr, err := dk.Address(key) - if err != nil { - panic(err) + if cfg.IntentConfigType == state.IntentConfigTypeTest { + if err := intent.SetTestValues(cfg.L2ChainIDs); err != nil { + return err } - return addr - } - intent.SuperchainRoles = &state.SuperchainRoles{ - ProxyAdminOwner: addrFor(devkeys.L1ProxyAdminOwnerRole.Key(l1ChainIDBig)), - ProtocolVersionsOwner: addrFor(devkeys.SuperchainProtocolVersionsOwner.Key(l1ChainIDBig)), - Guardian: addrFor(devkeys.SuperchainConfigGuardianKey.Key(l1ChainIDBig)), - } - - for _, l2ChainID := range cfg.L2ChainIDs { - l2ChainIDBig := l2ChainID.Big() - intent.Chains = append(intent.Chains, &state.ChainIntent{ - ID: l2ChainID, - BaseFeeVaultRecipient: addrFor(devkeys.BaseFeeVaultRecipientRole.Key(l2ChainIDBig)), - L1FeeVaultRecipient: addrFor(devkeys.L1FeeVaultRecipientRole.Key(l2ChainIDBig)), - SequencerFeeVaultRecipient: addrFor(devkeys.SequencerFeeVaultRecipientRole.Key(l2ChainIDBig)), - Eip1559Denominator: 50, - Eip1559Elasticity: 6, - Roles: state.ChainRoles{ - L1ProxyAdminOwner: addrFor(devkeys.L1ProxyAdminOwnerRole.Key(l2ChainIDBig)), - L2ProxyAdminOwner: addrFor(devkeys.L2ProxyAdminOwnerRole.Key(l2ChainIDBig)), - SystemConfigOwner: addrFor(devkeys.SystemConfigOwner.Key(l2ChainIDBig)), - UnsafeBlockSigner: addrFor(devkeys.SequencerP2PRole.Key(l2ChainIDBig)), - Batcher: addrFor(devkeys.BatcherRole.Key(l2ChainIDBig)), - Proposer: addrFor(devkeys.ProposerRole.Key(l2ChainIDBig)), - Challenger: addrFor(devkeys.ChallengerRole.Key(l2ChainIDBig)), - }, - }) } st := &state.State{ diff --git a/op-deployer/pkg/deployer/pipeline/opchain.go b/op-deployer/pkg/deployer/pipeline/opchain.go index 90cc665e077..192f7fcfd67 100644 --- a/op-deployer/pkg/deployer/pipeline/opchain.go +++ b/op-deployer/pkg/deployer/pipeline/opchain.go @@ -128,7 +128,7 @@ func makeDCIV160(intent *state.Intent, thisIntent *state.ChainIntent, chainID co L2ChainId: chainID.Big(), Opcm: st.ImplementationsDeployment.OpcmAddress, SaltMixer: st.Create2Salt.String(), // passing through salt generated at state initialization - GasLimit: standard.GasLimit, + GasLimit: standard.BlockGasLimit, DisputeGameType: proofParams.DisputeGameType, DisputeAbsolutePrestate: proofParams.DisputeAbsolutePrestate, DisputeMaxGameDepth: proofParams.DisputeMaxGameDepth, diff --git a/op-deployer/pkg/deployer/standard/standard.go b/op-deployer/pkg/deployer/standard/standard.go index 75902424ac7..cd39ef83150 100644 --- a/op-deployer/pkg/deployer/standard/standard.go +++ b/op-deployer/pkg/deployer/standard/standard.go @@ -12,7 +12,7 @@ import ( ) const ( - GasLimit uint64 = 60_000_000 + BlockGasLimit uint64 = 60_000_000 BasefeeScalar uint32 = 1368 BlobBaseFeeScalar uint32 = 801949 WithdrawalDelaySeconds uint64 = 604800 diff --git a/op-deployer/pkg/deployer/state/intent.go b/op-deployer/pkg/deployer/state/intent.go index 86e6cfc1ed4..2087cfff96e 100644 --- a/op-deployer/pkg/deployer/state/intent.go +++ b/op-deployer/pkg/deployer/state/intent.go @@ -5,6 +5,7 @@ import ( "fmt" "math/big" + "github.com/ethereum-optimism/optimism/op-chain-ops/devkeys" "github.com/ethereum-optimism/optimism/op-chain-ops/genesis" "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/artifacts" @@ -32,35 +33,132 @@ func (d DeploymentStrategy) Check() error { } } +type IntentConfigType string + +const ( + IntentConfigTypeTest IntentConfigType = "test" + IntentConfigTypeStandard IntentConfigType = "standard" + IntentConfigTypeCustom IntentConfigType = "custom" + IntentConfigTypeStrict IntentConfigType = "strict" + IntentConfigTypeStandardOverrides IntentConfigType = "standard-overrides" + IntentConfigTypeStrictOverrides IntentConfigType = "strict-overrides" +) + var emptyAddress common.Address type Intent struct { - DeploymentStrategy DeploymentStrategy `json:"deploymentStrategy" toml:"deploymentStrategy"` + DeploymentStrategy DeploymentStrategy `json:"deploymentStrategy" toml:"deploymentStrategy"` + IntentConfigType IntentConfigType `json:"intentConfigType" toml:"intentConfigType"` + L1ChainID uint64 `json:"l1ChainID" toml:"l1ChainID"` + SuperchainRoles *SuperchainRoles `json:"superchainRoles" toml:"superchainRoles,omitempty"` + FundDevAccounts bool `json:"fundDevAccounts" toml:"fundDevAccounts"` + UseInterop bool `json:"useInterop" toml:"useInterop"` + L1ContractsLocator *artifacts.Locator `json:"l1ContractsLocator" toml:"l1ContractsLocator"` + L2ContractsLocator *artifacts.Locator `json:"l2ContractsLocator" toml:"l2ContractsLocator"` + Chains []*ChainIntent `json:"chains" toml:"chains"` + GlobalDeployOverrides map[string]any `json:"globalDeployOverrides" toml:"globalDeployOverrides"` +} - L1ChainID uint64 `json:"l1ChainID" toml:"l1ChainID"` +func (c *Intent) L1ChainIDBig() *big.Int { + return big.NewInt(int64(c.L1ChainID)) +} - SuperchainRoles *SuperchainRoles `json:"superchainRoles" toml:"superchainRoles,omitempty"` +func (c *Intent) ValidateIntentConfig() error { + switch c.IntentConfigType { + case IntentConfigTypeStandard: + return c.validateStandardConfig() - FundDevAccounts bool `json:"fundDevAccounts" toml:"fundDevAccounts"` + case IntentConfigTypeCustom: + return c.validateCustomConfig() - UseInterop bool `json:"useInterop" toml:"useInterop"` + case IntentConfigTypeStrict: + return c.validateStrictConfig() - L1ContractsLocator *artifacts.Locator `json:"l1ContractsLocator" toml:"l1ContractsLocator"` + case IntentConfigTypeStandardOverrides, IntentConfigTypeStrictOverrides: + return nil + default: + return fmt.Errorf("intent config type is invalid") + } +} - L2ContractsLocator *artifacts.Locator `json:"l2ContractsLocator" toml:"l2ContractsLocator"` +func (c *Intent) validateStandardConfig() error { + if c.SuperchainRoles != nil || c.L1ContractsLocator != nil || c.L2ContractsLocator != nil { + return errors.New("standard config: only certain fields should be set") + } - Chains []*ChainIntent `json:"chains" toml:"chains"` + // Block time and gas limit could be validated further if they are part of Intent's parameters. - GlobalDeployOverrides map[string]any `json:"globalDeployOverrides" toml:"globalDeployOverrides"` + return nil } -func (c *Intent) L1ChainIDBig() *big.Int { - return big.NewInt(int64(c.L1ChainID)) +func (c *Intent) validateCustomConfig() error { + if c.L1ChainID == 0 || c.L1ContractsLocator == nil || c.L2ContractsLocator == nil || len(c.Chains) == 0 { + return errors.New("custom config: all fields must be explicitly specified") + } + return nil +} + +func (c *Intent) validateStrictConfig() error { + for _, chain := range c.Chains { + if chain.BaseFeeVaultRecipient != (common.Address{}) { + return errors.New("for 'strict' config, opChainProxyAdminOwner and challenger cannot be set") + } + } + return nil +} + +func (c *Intent) SetTestValues(l2ChainIds []common.Hash) error { + c.FundDevAccounts = true + c.L1ContractsLocator = artifacts.DefaultL1ContractsLocator + c.L2ContractsLocator = artifacts.DefaultL2ContractsLocator + + l1ChainIDBig := c.L1ChainIDBig() + + dk, err := devkeys.NewMnemonicDevKeys(devkeys.TestMnemonic) + if err != nil { + return fmt.Errorf("failed to create dev keys: %w", err) + } + + addrFor := func(key devkeys.Key) common.Address { + // The error below should never happen, so panic if it does. + addr, err := dk.Address(key) + if err != nil { + panic(err) + } + return addr + } + c.SuperchainRoles = &SuperchainRoles{ + ProxyAdminOwner: addrFor(devkeys.L1ProxyAdminOwnerRole.Key(l1ChainIDBig)), + ProtocolVersionsOwner: addrFor(devkeys.SuperchainProtocolVersionsOwner.Key(l1ChainIDBig)), + Guardian: addrFor(devkeys.SuperchainConfigGuardianKey.Key(l1ChainIDBig)), + } + + for _, l2ChainID := range l2ChainIds { + l2ChainIDBig := l2ChainID.Big() + c.Chains = append(c.Chains, &ChainIntent{ + ID: l2ChainID, + BaseFeeVaultRecipient: addrFor(devkeys.BaseFeeVaultRecipientRole.Key(l2ChainIDBig)), + L1FeeVaultRecipient: addrFor(devkeys.L1FeeVaultRecipientRole.Key(l2ChainIDBig)), + SequencerFeeVaultRecipient: addrFor(devkeys.SequencerFeeVaultRecipientRole.Key(l2ChainIDBig)), + Eip1559Denominator: 50, + Eip1559Elasticity: 6, + Roles: ChainRoles{ + L1ProxyAdminOwner: addrFor(devkeys.L1ProxyAdminOwnerRole.Key(l2ChainIDBig)), + L2ProxyAdminOwner: addrFor(devkeys.L2ProxyAdminOwnerRole.Key(l2ChainIDBig)), + SystemConfigOwner: addrFor(devkeys.SystemConfigOwner.Key(l2ChainIDBig)), + UnsafeBlockSigner: addrFor(devkeys.SequencerP2PRole.Key(l2ChainIDBig)), + Batcher: addrFor(devkeys.BatcherRole.Key(l2ChainIDBig)), + Proposer: addrFor(devkeys.ProposerRole.Key(l2ChainIDBig)), + Challenger: addrFor(devkeys.ChallengerRole.Key(l2ChainIDBig)), + }, + }) + } + return nil } func (c *Intent) Check() error { - if c.DeploymentStrategy != DeploymentStrategyLive && c.DeploymentStrategy != DeploymentStrategyGenesis { - return fmt.Errorf("deploymentStrategy must be 'live' or 'local'") + if err := c.DeploymentStrategy.Check(); err != nil { + return err } if c.L1ChainID == 0 { @@ -151,23 +249,15 @@ type SuperchainRoles struct { } type ChainIntent struct { - ID common.Hash `json:"id" toml:"id"` - - BaseFeeVaultRecipient common.Address `json:"baseFeeVaultRecipient" toml:"baseFeeVaultRecipient"` - - L1FeeVaultRecipient common.Address `json:"l1FeeVaultRecipient" toml:"l1FeeVaultRecipient"` - - SequencerFeeVaultRecipient common.Address `json:"sequencerFeeVaultRecipient" toml:"sequencerFeeVaultRecipient"` - - Eip1559Denominator uint64 `json:"eip1559Denominator" toml:"eip1559Denominator"` - - Eip1559Elasticity uint64 `json:"eip1559Elasticity" toml:"eip1559Elasticity"` - - Roles ChainRoles `json:"roles" toml:"roles"` - - DeployOverrides map[string]any `json:"deployOverrides" toml:"deployOverrides"` - - DangerousAltDAConfig genesis.AltDADeployConfig `json:"dangerousAltDAConfig,omitempty" toml:"dangerousAltDAConfig,omitempty"` + ID common.Hash `json:"id" toml:"id"` + BaseFeeVaultRecipient common.Address `json:"baseFeeVaultRecipient" toml:"baseFeeVaultRecipient"` + L1FeeVaultRecipient common.Address `json:"l1FeeVaultRecipient" toml:"l1FeeVaultRecipient"` + SequencerFeeVaultRecipient common.Address `json:"sequencerFeeVaultRecipient" toml:"sequencerFeeVaultRecipient"` + Eip1559Denominator uint64 `json:"eip1559Denominator" toml:"eip1559Denominator"` + Eip1559Elasticity uint64 `json:"eip1559Elasticity" toml:"eip1559Elasticity"` + Roles ChainRoles `json:"roles" toml:"roles"` + DeployOverrides map[string]any `json:"deployOverrides" toml:"deployOverrides"` + DangerousAltDAConfig genesis.AltDADeployConfig `json:"dangerousAltDAConfig,omitempty" toml:"dangerousAltDAConfig,omitempty"` } type ChainRoles struct { From 8c3b1771ef8e8ec0ceaf632fe40771b7d24a1abd Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Tue, 19 Nov 2024 10:53:40 -0500 Subject: [PATCH 02/14] op-deployer: revert name change of GasLimit --- op-deployer/pkg/deployer/pipeline/opchain.go | 2 +- op-deployer/pkg/deployer/standard/standard.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/op-deployer/pkg/deployer/pipeline/opchain.go b/op-deployer/pkg/deployer/pipeline/opchain.go index 192f7fcfd67..90cc665e077 100644 --- a/op-deployer/pkg/deployer/pipeline/opchain.go +++ b/op-deployer/pkg/deployer/pipeline/opchain.go @@ -128,7 +128,7 @@ func makeDCIV160(intent *state.Intent, thisIntent *state.ChainIntent, chainID co L2ChainId: chainID.Big(), Opcm: st.ImplementationsDeployment.OpcmAddress, SaltMixer: st.Create2Salt.String(), // passing through salt generated at state initialization - GasLimit: standard.BlockGasLimit, + GasLimit: standard.GasLimit, DisputeGameType: proofParams.DisputeGameType, DisputeAbsolutePrestate: proofParams.DisputeAbsolutePrestate, DisputeMaxGameDepth: proofParams.DisputeMaxGameDepth, diff --git a/op-deployer/pkg/deployer/standard/standard.go b/op-deployer/pkg/deployer/standard/standard.go index cd39ef83150..75902424ac7 100644 --- a/op-deployer/pkg/deployer/standard/standard.go +++ b/op-deployer/pkg/deployer/standard/standard.go @@ -12,7 +12,7 @@ import ( ) const ( - BlockGasLimit uint64 = 60_000_000 + GasLimit uint64 = 60_000_000 BasefeeScalar uint32 = 1368 BlobBaseFeeScalar uint32 = 801949 WithdrawalDelaySeconds uint64 = 604800 From af222da7d332988fa54d5215e83c3b9851838dab Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Thu, 21 Nov 2024 16:11:42 -0500 Subject: [PATCH 03/14] op-deployer: add Intent.setStandardValues and Intent.validateStandardValues --- op-deployer/pkg/deployer/init.go | 10 +- .../deployer/integration_test/apply_test.go | 1 + op-deployer/pkg/deployer/pipeline/init.go | 10 +- op-deployer/pkg/deployer/standard/standard.go | 27 ++- .../pkg/deployer/state/chain_intent.go | 83 ++++++++ op-deployer/pkg/deployer/state/intent.go | 191 ++++++++++-------- 6 files changed, 228 insertions(+), 94 deletions(-) create mode 100644 op-deployer/pkg/deployer/state/chain_intent.go diff --git a/op-deployer/pkg/deployer/init.go b/op-deployer/pkg/deployer/init.go index 4c0d6c930a5..4253b1a4354 100644 --- a/op-deployer/pkg/deployer/init.go +++ b/op-deployer/pkg/deployer/init.go @@ -92,10 +92,12 @@ func Init(cfg InitConfig) error { L1ChainID: cfg.L1ChainID, } - if cfg.IntentConfigType == state.IntentConfigTypeTest { - if err := intent.SetTestValues(cfg.L2ChainIDs); err != nil { - return err - } + if err := intent.SetInitValues(cfg.L2ChainIDs); err != nil { + return err + } + + if err := intent.Check(); err != nil { + return err } st := &state.State{ diff --git a/op-deployer/pkg/deployer/integration_test/apply_test.go b/op-deployer/pkg/deployer/integration_test/apply_test.go index 682aaccfbc0..3a661c23a14 100644 --- a/op-deployer/pkg/deployer/integration_test/apply_test.go +++ b/op-deployer/pkg/deployer/integration_test/apply_test.go @@ -714,6 +714,7 @@ func newIntent( l2Loc *artifacts.Locator, ) (*state.Intent, *state.State) { intent := &state.Intent{ + IntentConfigType: state.IntentConfigTypeTest, DeploymentStrategy: state.DeploymentStrategyLive, L1ChainID: l1ChainID.Uint64(), SuperchainRoles: &state.SuperchainRoles{ diff --git a/op-deployer/pkg/deployer/pipeline/init.go b/op-deployer/pkg/deployer/pipeline/init.go index 88caf760f41..a32e5f5e951 100644 --- a/op-deployer/pkg/deployer/pipeline/init.go +++ b/op-deployer/pkg/deployer/pipeline/init.go @@ -26,7 +26,7 @@ func InitLiveStrategy(ctx context.Context, env *Env, intent *state.Intent, st *s lgr := env.Logger.New("stage", "init", "strategy", "live") lgr.Info("initializing pipeline") - if err := initCommonChecks(st); err != nil { + if err := initCommonChecks(st, intent); err != nil { return err } @@ -100,7 +100,11 @@ func InitLiveStrategy(ctx context.Context, env *Env, intent *state.Intent, st *s return nil } -func initCommonChecks(st *state.State) error { +func initCommonChecks(st *state.State, intent *state.Intent) error { + if err := intent.ValidateIntentConfigType(); err != nil { + return fmt.Errorf("failed to validate against intent config type: %w", err) + } + // Ensure the state version is supported. if !IsSupportedStateVersion(st.Version) { return fmt.Errorf("unsupported state version: %d", st.Version) @@ -119,7 +123,7 @@ func InitGenesisStrategy(env *Env, intent *state.Intent, st *state.State) error lgr := env.Logger.New("stage", "init", "strategy", "genesis") lgr.Info("initializing pipeline") - if err := initCommonChecks(st); err != nil { + if err := initCommonChecks(st, intent); err != nil { return err } diff --git a/op-deployer/pkg/deployer/standard/standard.go b/op-deployer/pkg/deployer/standard/standard.go index 75902424ac7..51f97304b15 100644 --- a/op-deployer/pkg/deployer/standard/standard.go +++ b/op-deployer/pkg/deployer/standard/standard.go @@ -26,6 +26,9 @@ const ( DisputeSplitDepth uint64 = 30 DisputeClockExtension uint64 = 10800 DisputeMaxClockDuration uint64 = 302400 + Eip1559DenominatorCanyon uint64 = 250 + Eip1559Denominator uint64 = 50 + Eip1559Elasticity uint64 = 6 ContractsV160Tag = "op-contracts/v1.6.0" ContractsV170Beta1L2Tag = "op-contracts/v1.7.0-beta.1+l2-contracts" @@ -97,6 +100,28 @@ func L1VersionsFor(chainID uint64) (L1Versions, error) { } } +func GuardianAddressFor(chainID uint64) (common.Address, error) { + switch chainID { + case 1: + return common.HexToAddress("0x09f7150D8c019BeF34450d6920f6B3608ceFdAf2"), nil + case 11155111: + return common.HexToAddress("0x7a50f00e8D05b95F98fE38d8BeE366a7324dCf7E"), nil + default: + return common.Address{}, fmt.Errorf("unsupported chain ID: %d", chainID) + } +} + +func ChallengerAddressFor(chainID uint64) (common.Address, error) { + switch chainID { + case 1: + return common.HexToAddress("0x9BA6e03D8B90dE867373Db8cF1A58d2F7F006b3A"), nil + case 11155111: + return common.HexToAddress("0xfd1D2e729aE8eEe2E146c033bf4400fE75284301"), nil + default: + return common.Address{}, fmt.Errorf("unsupported chain ID: %d", chainID) + } +} + func SuperchainFor(chainID uint64) (*superchain.Superchain, error) { switch chainID { case 1: @@ -115,7 +140,7 @@ func ChainNameFor(chainID uint64) (string, error) { case 11155111: return "sepolia", nil default: - return "", fmt.Errorf("unrecognized chain ID: %d", chainID) + return "", fmt.Errorf("unrecognized l1 chain ID: %d", chainID) } } diff --git a/op-deployer/pkg/deployer/state/chain_intent.go b/op-deployer/pkg/deployer/state/chain_intent.go new file mode 100644 index 00000000000..b0347d8a3b9 --- /dev/null +++ b/op-deployer/pkg/deployer/state/chain_intent.go @@ -0,0 +1,83 @@ +package state + +import ( + "fmt" + "reflect" + + "github.com/ethereum-optimism/optimism/op-chain-ops/genesis" + "github.com/ethereum/go-ethereum/common" +) + +type ChainIntent struct { + ID common.Hash `json:"id" toml:"id"` + BaseFeeVaultRecipient common.Address `json:"baseFeeVaultRecipient" toml:"baseFeeVaultRecipient"` + L1FeeVaultRecipient common.Address `json:"l1FeeVaultRecipient" toml:"l1FeeVaultRecipient"` + SequencerFeeVaultRecipient common.Address `json:"sequencerFeeVaultRecipient" toml:"sequencerFeeVaultRecipient"` + Eip1559DenominatorCanyon uint64 `json:"eip1559DenominatorCanyon" toml:"eip1559DenominatorCanyon"` + Eip1559Denominator uint64 `json:"eip1559Denominator" toml:"eip1559Denominator"` + Eip1559Elasticity uint64 `json:"eip1559Elasticity" toml:"eip1559Elasticity"` + Roles ChainRoles `json:"roles" toml:"roles"` + DeployOverrides map[string]any `json:"deployOverrides" toml:"deployOverrides"` + DangerousAltDAConfig genesis.AltDADeployConfig `json:"dangerousAltDAConfig,omitempty" toml:"dangerousAltDAConfig,omitempty"` +} + +type ChainRoles struct { + L1ProxyAdminOwner common.Address `json:"l1ProxyAdminOwner" toml:"l1ProxyAdminOwner"` + L2ProxyAdminOwner common.Address `json:"l2ProxyAdminOwner" toml:"l2ProxyAdminOwner"` + SystemConfigOwner common.Address `json:"systemConfigOwner" toml:"systemConfigOwner"` + UnsafeBlockSigner common.Address `json:"unsafeBlockSigner" toml:"unsafeBlockSigner"` + Batcher common.Address `json:"batcher" toml:"batcher"` + Proposer common.Address `json:"proposer" toml:"proposer"` + Challenger common.Address `json:"challenger" toml:"challenger"` +} + +func (c *ChainIntent) Check() error { + var emptyHash common.Hash + if c.ID == emptyHash { + return fmt.Errorf("id must be set") + } + + if c.Roles.L1ProxyAdminOwner == emptyAddress { + return fmt.Errorf("proxyAdminOwner must be set") + } + + if c.Roles.L2ProxyAdminOwner == emptyAddress { + return fmt.Errorf("l2ProxyAdminOwner must be set") + } + + if c.Roles.SystemConfigOwner == emptyAddress { + c.Roles.SystemConfigOwner = c.Roles.L1ProxyAdminOwner + } + + if c.Roles.UnsafeBlockSigner == emptyAddress { + return fmt.Errorf("unsafeBlockSigner must be set") + } + + if c.Roles.Batcher == emptyAddress { + return fmt.Errorf("batcher must be set") + } + + if c.DangerousAltDAConfig.UseAltDA { + return c.DangerousAltDAConfig.Check(nil) + } + + return nil +} + +// Returns an error if any fields in ChainRoles is set to common.Address{} +func (cr *ChainRoles) CheckNoZeroAddresses() error { + val := reflect.ValueOf(*cr) + typ := reflect.TypeOf(*cr) + + // Iterate through all the fields + for i := 0; i < val.NumField(); i++ { + fieldValue := val.Field(i) + fieldName := typ.Field(i).Name + + if fieldValue.Interface() == (common.Address{}) { + return fmt.Errorf("ChainRoles %s contains is set to zero-value address", fieldName) + } + } + + return nil +} diff --git a/op-deployer/pkg/deployer/state/intent.go b/op-deployer/pkg/deployer/state/intent.go index 2087cfff96e..4a94808e1a8 100644 --- a/op-deployer/pkg/deployer/state/intent.go +++ b/op-deployer/pkg/deployer/state/intent.go @@ -6,7 +6,6 @@ import ( "math/big" "github.com/ethereum-optimism/optimism/op-chain-ops/devkeys" - "github.com/ethereum-optimism/optimism/op-chain-ops/genesis" "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/artifacts" @@ -45,6 +44,7 @@ const ( ) var emptyAddress common.Address +var emptyHash common.Hash type Intent struct { DeploymentStrategy DeploymentStrategy `json:"deploymentStrategy" toml:"deploymentStrategy"` @@ -59,14 +59,20 @@ type Intent struct { GlobalDeployOverrides map[string]any `json:"globalDeployOverrides" toml:"globalDeployOverrides"` } +type SuperchainRoles struct { + ProxyAdminOwner common.Address `json:"proxyAdminOwner" toml:"proxyAdminOwner"` + ProtocolVersionsOwner common.Address `json:"protocolVersionsOwner" toml:"protocolVersionsOwner"` + Guardian common.Address `json:"guardian" toml:"guardian"` +} + func (c *Intent) L1ChainIDBig() *big.Int { return big.NewInt(int64(c.L1ChainID)) } -func (c *Intent) ValidateIntentConfig() error { +func (c *Intent) ValidateIntentConfigType() error { switch c.IntentConfigType { case IntentConfigTypeStandard: - return c.validateStandardConfig() + return c.validateStandardValues() case IntentConfigTypeCustom: return c.validateCustomConfig() @@ -74,6 +80,9 @@ func (c *Intent) ValidateIntentConfig() error { case IntentConfigTypeStrict: return c.validateStrictConfig() + case IntentConfigTypeTest: + return nil + case IntentConfigTypeStandardOverrides, IntentConfigTypeStrictOverrides: return nil default: @@ -81,16 +90,6 @@ func (c *Intent) ValidateIntentConfig() error { } } -func (c *Intent) validateStandardConfig() error { - if c.SuperchainRoles != nil || c.L1ContractsLocator != nil || c.L2ContractsLocator != nil { - return errors.New("standard config: only certain fields should be set") - } - - // Block time and gas limit could be validated further if they are part of Intent's parameters. - - return nil -} - func (c *Intent) validateCustomConfig() error { if c.L1ChainID == 0 || c.L1ContractsLocator == nil || c.L2ContractsLocator == nil || len(c.Chains) == 0 { return errors.New("custom config: all fields must be explicitly specified") @@ -101,13 +100,101 @@ func (c *Intent) validateCustomConfig() error { func (c *Intent) validateStrictConfig() error { for _, chain := range c.Chains { if chain.BaseFeeVaultRecipient != (common.Address{}) { - return errors.New("for 'strict' config, opChainProxyAdminOwner and challenger cannot be set") + return errors.New("strict config: opChainProxyAdminOwner and challenger cannot be set") } } return nil } -func (c *Intent) SetTestValues(l2ChainIds []common.Hash) error { +func (c *Intent) SetInitValues(l2ChainIds []common.Hash) error { + switch c.IntentConfigType { + case IntentConfigTypeStandard: + return c.setStandardValues(l2ChainIds) + + case IntentConfigTypeTest: + return c.setTestValues(l2ChainIds) + + default: + return fmt.Errorf("intent config type is invalid") + } + +} + +// Ensures the following: +// 1. no zero-values for non-standard fields (user should have populated these) +// 2. no non-standard values for standard fields (user should not have changed these) +func (c *Intent) validateStandardValues() error { + standardSuperchainRoles, err := getStandardSuperchainRoles(c.L1ChainID) + if err != nil { + return fmt.Errorf("error getting standard superchain roles: %w", err) + } + if *c.SuperchainRoles != *standardSuperchainRoles { + return fmt.Errorf("SuperchainRoles does not match standard value") + } + + challenger, _ := standard.ChallengerAddressFor(c.L1ChainID) + for _, chain := range c.Chains { + if chain.ID == emptyHash { + return fmt.Errorf("missing l2 chain ID") + } + if err := chain.Roles.CheckNoZeroAddresses(); err != nil { + return err + } + if chain.Eip1559DenominatorCanyon != standard.Eip1559DenominatorCanyon || + chain.Eip1559Denominator != standard.Eip1559Denominator || + chain.Eip1559Elasticity != standard.Eip1559Elasticity || + chain.Roles.Challenger != challenger { + return fmt.Errorf("l2 chain has non-standard value") + } + } + + return nil +} + +func getStandardSuperchainRoles(l1ChainId uint64) (*SuperchainRoles, error) { + superCfg, err := standard.SuperchainFor(l1ChainId) + if err != nil { + return nil, fmt.Errorf("error getting superchain config: %w", err) + } + + proxyAdmin, _ := standard.ManagerOwnerAddrFor(l1ChainId) + guardian, _ := standard.GuardianAddressFor(l1ChainId) + + superchainRoles := &SuperchainRoles{ + ProxyAdminOwner: proxyAdmin, + ProtocolVersionsOwner: common.Address(*superCfg.Config.ProtocolVersionsAddr), + Guardian: guardian, + } + + return superchainRoles, nil +} + +func (c *Intent) setStandardValues(l2ChainIds []common.Hash) error { + superchainRoles, err := getStandardSuperchainRoles(c.L1ChainID) + if err != nil { + return fmt.Errorf("error getting standard superchain roles: %w", err) + } + c.SuperchainRoles = superchainRoles + + c.L1ContractsLocator = artifacts.DefaultL1ContractsLocator + c.L2ContractsLocator = artifacts.DefaultL2ContractsLocator + + challenger, _ := standard.ChallengerAddressFor(c.L1ChainID) + for _, l2ChainID := range l2ChainIds { + c.Chains = append(c.Chains, &ChainIntent{ + ID: l2ChainID, + Eip1559DenominatorCanyon: standard.Eip1559DenominatorCanyon, + Eip1559Denominator: standard.Eip1559Denominator, + Eip1559Elasticity: standard.Eip1559Elasticity, + Roles: ChainRoles{ + Challenger: challenger, + }, + }) + } + return nil +} + +func (c *Intent) setTestValues(l2ChainIds []common.Hash) error { c.FundDevAccounts = true c.L1ContractsLocator = artifacts.DefaultL1ContractsLocator c.L2ContractsLocator = artifacts.DefaultL2ContractsLocator @@ -140,8 +227,9 @@ func (c *Intent) SetTestValues(l2ChainIds []common.Hash) error { BaseFeeVaultRecipient: addrFor(devkeys.BaseFeeVaultRecipientRole.Key(l2ChainIDBig)), L1FeeVaultRecipient: addrFor(devkeys.L1FeeVaultRecipientRole.Key(l2ChainIDBig)), SequencerFeeVaultRecipient: addrFor(devkeys.SequencerFeeVaultRecipientRole.Key(l2ChainIDBig)), - Eip1559Denominator: 50, - Eip1559Elasticity: 6, + Eip1559DenominatorCanyon: standard.Eip1559DenominatorCanyon, + Eip1559Denominator: standard.Eip1559Denominator, + Eip1559Elasticity: standard.Eip1559Elasticity, Roles: ChainRoles{ L1ProxyAdminOwner: addrFor(devkeys.L1ProxyAdminOwnerRole.Key(l2ChainIDBig)), L2ProxyAdminOwner: addrFor(devkeys.L2ProxyAdminOwnerRole.Key(l2ChainIDBig)), @@ -239,72 +327,3 @@ func (c *Intent) checkL2Prod() error { _, err := standard.ArtifactsURLForTag(c.L2ContractsLocator.Tag) return err } - -type SuperchainRoles struct { - ProxyAdminOwner common.Address `json:"proxyAdminOwner" toml:"proxyAdminOwner"` - - ProtocolVersionsOwner common.Address `json:"protocolVersionsOwner" toml:"protocolVersionsOwner"` - - Guardian common.Address `json:"guardian" toml:"guardian"` -} - -type ChainIntent struct { - ID common.Hash `json:"id" toml:"id"` - BaseFeeVaultRecipient common.Address `json:"baseFeeVaultRecipient" toml:"baseFeeVaultRecipient"` - L1FeeVaultRecipient common.Address `json:"l1FeeVaultRecipient" toml:"l1FeeVaultRecipient"` - SequencerFeeVaultRecipient common.Address `json:"sequencerFeeVaultRecipient" toml:"sequencerFeeVaultRecipient"` - Eip1559Denominator uint64 `json:"eip1559Denominator" toml:"eip1559Denominator"` - Eip1559Elasticity uint64 `json:"eip1559Elasticity" toml:"eip1559Elasticity"` - Roles ChainRoles `json:"roles" toml:"roles"` - DeployOverrides map[string]any `json:"deployOverrides" toml:"deployOverrides"` - DangerousAltDAConfig genesis.AltDADeployConfig `json:"dangerousAltDAConfig,omitempty" toml:"dangerousAltDAConfig,omitempty"` -} - -type ChainRoles struct { - L1ProxyAdminOwner common.Address `json:"l1ProxyAdminOwner" toml:"l1ProxyAdminOwner"` - - L2ProxyAdminOwner common.Address `json:"l2ProxyAdminOwner" toml:"l2ProxyAdminOwner"` - - SystemConfigOwner common.Address `json:"systemConfigOwner" toml:"systemConfigOwner"` - - UnsafeBlockSigner common.Address `json:"unsafeBlockSigner" toml:"unsafeBlockSigner"` - - Batcher common.Address `json:"batcher" toml:"batcher"` - - Proposer common.Address `json:"proposer" toml:"proposer"` - - Challenger common.Address `json:"challenger" toml:"challenger"` -} - -func (c *ChainIntent) Check() error { - var emptyHash common.Hash - if c.ID == emptyHash { - return fmt.Errorf("id must be set") - } - - if c.Roles.L1ProxyAdminOwner == emptyAddress { - return fmt.Errorf("proxyAdminOwner must be set") - } - - if c.Roles.SystemConfigOwner == emptyAddress { - c.Roles.SystemConfigOwner = c.Roles.L1ProxyAdminOwner - } - - if c.Roles.L2ProxyAdminOwner == emptyAddress { - return fmt.Errorf("l2ProxyAdminOwner must be set") - } - - if c.Roles.UnsafeBlockSigner == emptyAddress { - return fmt.Errorf("unsafeBlockSigner must be set") - } - - if c.Roles.Batcher == emptyAddress { - return fmt.Errorf("batcher must be set") - } - - if c.DangerousAltDAConfig.UseAltDA { - return c.DangerousAltDAConfig.Check(nil) - } - - return nil -} From c3ad6943d5ca5bc65bcfbfb500a8f749999c3b31 Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Fri, 22 Nov 2024 13:15:08 -0500 Subject: [PATCH 04/14] op-deployer: improve standard intent-config err messages --- op-deployer/pkg/deployer/flags.go | 2 +- op-deployer/pkg/deployer/pipeline/init.go | 6 ++-- op-deployer/pkg/deployer/state/intent.go | 36 +++++++++++------------ 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/op-deployer/pkg/deployer/flags.go b/op-deployer/pkg/deployer/flags.go index 853ad09b7f6..a58d35bbbb8 100644 --- a/op-deployer/pkg/deployer/flags.go +++ b/op-deployer/pkg/deployer/flags.go @@ -36,7 +36,7 @@ var ( Name: L1ChainIDFlagName, Usage: "Chain ID of the L1 chain.", EnvVars: PrefixEnvVar("L1_CHAIN_ID"), - Value: 900, + Value: 11155111, } L2ChainIDsFlag = &cli.StringFlag{ Name: L2ChainIDsFlagName, diff --git a/op-deployer/pkg/deployer/pipeline/init.go b/op-deployer/pkg/deployer/pipeline/init.go index a32e5f5e951..ba1fe43d147 100644 --- a/op-deployer/pkg/deployer/pipeline/init.go +++ b/op-deployer/pkg/deployer/pipeline/init.go @@ -102,7 +102,7 @@ func InitLiveStrategy(ctx context.Context, env *Env, intent *state.Intent, st *s func initCommonChecks(st *state.State, intent *state.Intent) error { if err := intent.ValidateIntentConfigType(); err != nil { - return fmt.Errorf("failed to validate against intent config type: %w", err) + return err } // Ensure the state version is supported. @@ -145,10 +145,10 @@ func displayWarning() error { ####################### WARNING! WARNING WARNING! ####################### You are deploying a tagged release to a chain with no pre-deployed OPCM. -The contracts you are deploying may not be audited, or match a governance +The contracts you are deploying may not be audited, or match a governance approved release. -USE OF THIS DEPLOYMENT IS NOT RECOMMENDED FOR PRODUCTION. USE AT YOUR OWN +USE OF THIS DEPLOYMENT IS NOT RECOMMENDED FOR PRODUCTION. USE AT YOUR OWN RISK. BUGS OR LOSS OF FUNDS MAY OCCUR. WE HOPE YOU KNOW WHAT YOU ARE DOING. diff --git a/op-deployer/pkg/deployer/state/intent.go b/op-deployer/pkg/deployer/state/intent.go index 4a94808e1a8..aa35e9ee7ae 100644 --- a/op-deployer/pkg/deployer/state/intent.go +++ b/op-deployer/pkg/deployer/state/intent.go @@ -72,37 +72,32 @@ func (c *Intent) L1ChainIDBig() *big.Int { func (c *Intent) ValidateIntentConfigType() error { switch c.IntentConfigType { case IntentConfigTypeStandard: - return c.validateStandardValues() - + if err := c.validateStandardValues(); err != nil { + return fmt.Errorf("failed to validate intent-config-type=standard: %w", err) + } case IntentConfigTypeCustom: - return c.validateCustomConfig() - + if err := c.validateCustomConfig(); err != nil { + return fmt.Errorf("failed to validate intent-config-type=custom: %w", err) + } case IntentConfigTypeStrict: - return c.validateStrictConfig() - + if err := c.validateStrictConfig(); err != nil { + return fmt.Errorf("failed to validate intent-config-type=strict: %w", err) + } case IntentConfigTypeTest: return nil - case IntentConfigTypeStandardOverrides, IntentConfigTypeStrictOverrides: return nil default: - return fmt.Errorf("intent config type is invalid") + return fmt.Errorf("intent-config-type unsupported: %s", c.IntentConfigType) } + return nil } func (c *Intent) validateCustomConfig() error { - if c.L1ChainID == 0 || c.L1ContractsLocator == nil || c.L2ContractsLocator == nil || len(c.Chains) == 0 { - return errors.New("custom config: all fields must be explicitly specified") - } return nil } func (c *Intent) validateStrictConfig() error { - for _, chain := range c.Chains { - if chain.BaseFeeVaultRecipient != (common.Address{}) { - return errors.New("strict config: opChainProxyAdminOwner and challenger cannot be set") - } - } return nil } @@ -115,7 +110,7 @@ func (c *Intent) SetInitValues(l2ChainIds []common.Hash) error { return c.setTestValues(l2ChainIds) default: - return fmt.Errorf("intent config type is invalid") + return fmt.Errorf("intent config type not supported") } } @@ -144,7 +139,12 @@ func (c *Intent) validateStandardValues() error { chain.Eip1559Denominator != standard.Eip1559Denominator || chain.Eip1559Elasticity != standard.Eip1559Elasticity || chain.Roles.Challenger != challenger { - return fmt.Errorf("l2 chain has non-standard value") + return fmt.Errorf("%w: chainId=%s", ErrNonStandardValue, chain.ID) + } + if chain.BaseFeeVaultRecipient == emptyAddress || + chain.L1FeeVaultRecipient == emptyAddress || + chain.SequencerFeeVaultRecipient == emptyAddress { + return fmt.Errorf("%w: chainId=%s", ErrFeeVaultZeroAddress, chain.ID) } } From 38bc334706345ba8bd946a1c726c2810fcbb6418 Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Fri, 22 Nov 2024 13:16:19 -0500 Subject: [PATCH 05/14] op-deployr: chain intent custom err types --- op-deployer/pkg/deployer/state/chain_intent.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/op-deployer/pkg/deployer/state/chain_intent.go b/op-deployer/pkg/deployer/state/chain_intent.go index b0347d8a3b9..e28f85bb6ac 100644 --- a/op-deployer/pkg/deployer/state/chain_intent.go +++ b/op-deployer/pkg/deployer/state/chain_intent.go @@ -31,6 +31,10 @@ type ChainRoles struct { Challenger common.Address `json:"challenger" toml:"challenger"` } +var ErrChainRoleZeroAddress = fmt.Errorf("ChainRole is set to zero address") +var ErrFeeVaultZeroAddress = fmt.Errorf("chain has a fee vault set to zero address") +var ErrNonStandardValue = fmt.Errorf("chain contains non-standard config value") + func (c *ChainIntent) Check() error { var emptyHash common.Hash if c.ID == emptyHash { @@ -75,7 +79,7 @@ func (cr *ChainRoles) CheckNoZeroAddresses() error { fieldName := typ.Field(i).Name if fieldValue.Interface() == (common.Address{}) { - return fmt.Errorf("ChainRoles %s contains is set to zero-value address", fieldName) + return fmt.Errorf("%w: %s", ErrChainRoleZeroAddress, fieldName) } } From 552a5a92afa35b02f95b9d6fd89cd8f81b6e5d5c Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Fri, 22 Nov 2024 13:16:54 -0500 Subject: [PATCH 06/14] op-deployer: add unit test for inent.validateStandardValues --- op-deployer/pkg/deployer/state/intent_test.go | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 op-deployer/pkg/deployer/state/intent_test.go diff --git a/op-deployer/pkg/deployer/state/intent_test.go b/op-deployer/pkg/deployer/state/intent_test.go new file mode 100644 index 00000000000..d2f76e468ba --- /dev/null +++ b/op-deployer/pkg/deployer/state/intent_test.go @@ -0,0 +1,53 @@ +package state + +import ( + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/stretchr/testify/require" +) + +func TestValidateStandardValues(t *testing.T) { + intent := initStandardIntent() + err := intent.ValidateIntentConfigType() + require.Error(t, err) + require.ErrorIs(t, err, ErrChainRoleZeroAddress) + + setChainRoles(intent) + err = intent.ValidateIntentConfigType() + require.Error(t, err) + require.ErrorIs(t, err, ErrFeeVaultZeroAddress) + + setFeeAddresses(intent) + err = intent.ValidateIntentConfigType() + require.NoError(t, err) + + intent.Chains[0].Eip1559Denominator = 3 + err = intent.ValidateIntentConfigType() + require.Error(t, err) + require.ErrorIs(t, err, ErrNonStandardValue) +} + +func initStandardIntent() *Intent { + intent := Intent{ + L1ChainID: 1, + IntentConfigType: IntentConfigTypeStandard, + } + intent.setStandardValues([]common.Hash{common.HexToHash("0x336")}) + return &intent +} + +func setChainRoles(intent *Intent) { + intent.Chains[0].Roles.L1ProxyAdminOwner = common.HexToAddress("0x01") + intent.Chains[0].Roles.L2ProxyAdminOwner = common.HexToAddress("0x01") + intent.Chains[0].Roles.SystemConfigOwner = common.HexToAddress("0x01") + intent.Chains[0].Roles.UnsafeBlockSigner = common.HexToAddress("0x01") + intent.Chains[0].Roles.Batcher = common.HexToAddress("0x01") + intent.Chains[0].Roles.Proposer = common.HexToAddress("0x01") +} + +func setFeeAddresses(intent *Intent) { + intent.Chains[0].BaseFeeVaultRecipient = common.HexToAddress("0x08") + intent.Chains[0].L1FeeVaultRecipient = common.HexToAddress("0x09") + intent.Chains[0].SequencerFeeVaultRecipient = common.HexToAddress("0x0A") +} From b5be8cc83ad64fb0ccc453439e30fe5891e45c49 Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Fri, 22 Nov 2024 13:22:11 -0500 Subject: [PATCH 07/14] linter fix --- op-deployer/pkg/deployer/state/intent_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/op-deployer/pkg/deployer/state/intent_test.go b/op-deployer/pkg/deployer/state/intent_test.go index d2f76e468ba..99516fcbe6b 100644 --- a/op-deployer/pkg/deployer/state/intent_test.go +++ b/op-deployer/pkg/deployer/state/intent_test.go @@ -33,17 +33,17 @@ func initStandardIntent() *Intent { L1ChainID: 1, IntentConfigType: IntentConfigTypeStandard, } - intent.setStandardValues([]common.Hash{common.HexToHash("0x336")}) + _ = intent.setStandardValues([]common.Hash{common.HexToHash("0x336")}) return &intent } func setChainRoles(intent *Intent) { intent.Chains[0].Roles.L1ProxyAdminOwner = common.HexToAddress("0x01") - intent.Chains[0].Roles.L2ProxyAdminOwner = common.HexToAddress("0x01") - intent.Chains[0].Roles.SystemConfigOwner = common.HexToAddress("0x01") - intent.Chains[0].Roles.UnsafeBlockSigner = common.HexToAddress("0x01") - intent.Chains[0].Roles.Batcher = common.HexToAddress("0x01") - intent.Chains[0].Roles.Proposer = common.HexToAddress("0x01") + intent.Chains[0].Roles.L2ProxyAdminOwner = common.HexToAddress("0x02") + intent.Chains[0].Roles.SystemConfigOwner = common.HexToAddress("0x03") + intent.Chains[0].Roles.UnsafeBlockSigner = common.HexToAddress("0x04") + intent.Chains[0].Roles.Batcher = common.HexToAddress("0x05") + intent.Chains[0].Roles.Proposer = common.HexToAddress("0x06") } func setFeeAddresses(intent *Intent) { From 3961576d536eaac4174851714c4afd9d948193b8 Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Fri, 22 Nov 2024 14:36:43 -0500 Subject: [PATCH 08/14] add intent-config-type custom --- op-deployer/pkg/deployer/init.go | 4 - .../pkg/deployer/state/chain_intent.go | 24 ++++++ op-deployer/pkg/deployer/state/intent.go | 83 +++++++++++++++---- 3 files changed, 93 insertions(+), 18 deletions(-) diff --git a/op-deployer/pkg/deployer/init.go b/op-deployer/pkg/deployer/init.go index 4253b1a4354..f34897df156 100644 --- a/op-deployer/pkg/deployer/init.go +++ b/op-deployer/pkg/deployer/init.go @@ -96,10 +96,6 @@ func Init(cfg InitConfig) error { return err } - if err := intent.Check(); err != nil { - return err - } - st := &state.State{ Version: 1, } diff --git a/op-deployer/pkg/deployer/state/chain_intent.go b/op-deployer/pkg/deployer/state/chain_intent.go index e28f85bb6ac..f444a766abc 100644 --- a/op-deployer/pkg/deployer/state/chain_intent.go +++ b/op-deployer/pkg/deployer/state/chain_intent.go @@ -1,6 +1,7 @@ package state import ( + "errors" "fmt" "reflect" @@ -34,6 +35,7 @@ type ChainRoles struct { var ErrChainRoleZeroAddress = fmt.Errorf("ChainRole is set to zero address") var ErrFeeVaultZeroAddress = fmt.Errorf("chain has a fee vault set to zero address") var ErrNonStandardValue = fmt.Errorf("chain contains non-standard config value") +var ErrEip1559ZeroValue = fmt.Errorf("eip1559 param is set to zero value") func (c *ChainIntent) Check() error { var emptyHash common.Hash @@ -68,6 +70,28 @@ func (c *ChainIntent) Check() error { return nil } +func (c *ChainIntent) CheckNoZeroValues() error { + if c.ID == emptyHash { + return errors.New("missing l2 chain ID") + } + if err := c.Roles.CheckNoZeroAddresses(); err != nil { + return err + } + + if c.Eip1559DenominatorCanyon == 0 || + c.Eip1559Denominator == 0 || + c.Eip1559Elasticity == 0 { + return fmt.Errorf("%w: chainId=%s", ErrEip1559ZeroValue, c.ID) + } + if c.BaseFeeVaultRecipient == emptyAddress || + c.L1FeeVaultRecipient == emptyAddress || + c.SequencerFeeVaultRecipient == emptyAddress { + return fmt.Errorf("%w: chainId=%s", ErrFeeVaultZeroAddress, c.ID) + } + + return nil +} + // Returns an error if any fields in ChainRoles is set to common.Address{} func (cr *ChainRoles) CheckNoZeroAddresses() error { val := reflect.ValueOf(*cr) diff --git a/op-deployer/pkg/deployer/state/intent.go b/op-deployer/pkg/deployer/state/intent.go index aa35e9ee7ae..4fdee710e4a 100644 --- a/op-deployer/pkg/deployer/state/intent.go +++ b/op-deployer/pkg/deployer/state/intent.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "math/big" + "reflect" "github.com/ethereum-optimism/optimism/op-chain-ops/devkeys" @@ -65,6 +66,24 @@ type SuperchainRoles struct { Guardian common.Address `json:"guardian" toml:"guardian"` } +var ErrSuperchainRoleZeroAddress = fmt.Errorf("SuperchainRole is set to zero address") + +func (s *SuperchainRoles) CheckNoZeroAddresses() error { + val := reflect.ValueOf(*s) + typ := reflect.TypeOf(*s) + + // Iterate through all the fields + for i := 0; i < val.NumField(); i++ { + fieldValue := val.Field(i) + fieldName := typ.Field(i).Name + + if fieldValue.Interface() == (common.Address{}) { + return fmt.Errorf("%w: %s", ErrSuperchainRoleZeroAddress, fieldName) + } + } + return nil +} + func (c *Intent) L1ChainIDBig() *big.Int { return big.NewInt(int64(c.L1ChainID)) } @@ -94,25 +113,32 @@ func (c *Intent) ValidateIntentConfigType() error { } func (c *Intent) validateCustomConfig() error { - return nil -} - -func (c *Intent) validateStrictConfig() error { - return nil -} + if c.L1ContractsLocator == nil || c.L1ContractsLocator.Tag == "undefined" { + return errors.New("L1ContractsLocator undefined") + } + if c.L2ContractsLocator == nil || c.L2ContractsLocator.Tag == "undefined" { + return errors.New("L2ContractsLocator undefined") + } -func (c *Intent) SetInitValues(l2ChainIds []common.Hash) error { - switch c.IntentConfigType { - case IntentConfigTypeStandard: - return c.setStandardValues(l2ChainIds) + if err := c.SuperchainRoles.CheckNoZeroAddresses(); err != nil { + return err + } - case IntentConfigTypeTest: - return c.setTestValues(l2ChainIds) + if len(c.Chains) == 0 { + return errors.New("must define at least one l2 chain") + } - default: - return fmt.Errorf("intent config type not supported") + for _, chain := range c.Chains { + if err := chain.CheckNoZeroValues(); err != nil { + return err + } } + return nil +} + +func (c *Intent) validateStrictConfig() error { + return nil } // Ensures the following: @@ -169,6 +195,35 @@ func getStandardSuperchainRoles(l1ChainId uint64) (*SuperchainRoles, error) { return superchainRoles, nil } +func (c *Intent) SetInitValues(l2ChainIds []common.Hash) error { + switch c.IntentConfigType { + case IntentConfigTypeCustom: + return c.setCustomValues(l2ChainIds) + + case IntentConfigTypeStandard: + return c.setStandardValues(l2ChainIds) + + case IntentConfigTypeTest: + return c.setTestValues(l2ChainIds) + + default: + return fmt.Errorf("intent config type not supported") + } +} + +func (c *Intent) setCustomValues(l2ChainIds []common.Hash) error { + c.L1ContractsLocator = &artifacts.Locator{Tag: "undefined"} + c.L2ContractsLocator = &artifacts.Locator{Tag: "undefined"} + + c.SuperchainRoles = &SuperchainRoles{} + for _, l2ChainID := range l2ChainIds { + c.Chains = append(c.Chains, &ChainIntent{ + ID: l2ChainID, + }) + } + return nil +} + func (c *Intent) setStandardValues(l2ChainIds []common.Hash) error { superchainRoles, err := getStandardSuperchainRoles(c.L1ChainID) if err != nil { From 394810e88579b1fec6df18b0c8ca436691c8c9f7 Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Fri, 22 Nov 2024 15:50:49 -0500 Subject: [PATCH 09/14] add intent-config-type strict --- op-deployer/pkg/deployer/standard/standard.go | 11 +++++ op-deployer/pkg/deployer/state/intent.go | 44 ++++++++++++++++--- op-deployer/pkg/deployer/state/intent_test.go | 1 + 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/op-deployer/pkg/deployer/standard/standard.go b/op-deployer/pkg/deployer/standard/standard.go index 51f97304b15..d2fe5e5d942 100644 --- a/op-deployer/pkg/deployer/standard/standard.go +++ b/op-deployer/pkg/deployer/standard/standard.go @@ -198,6 +198,17 @@ func SystemOwnerAddrFor(chainID uint64) (common.Address, error) { } } +func L1ProxyAdminOwner(chainID uint64) (common.Address, error) { + switch chainID { + case 1: + return common.HexToAddress("0x5a0Aae59D09fccBdDb6C6CcEB07B7279367C3d2A"), nil + case 11155111: + return common.HexToAddress("0x1Eb2fFc903729a0F03966B917003800b145F56E2"), nil + default: + return common.Address{}, fmt.Errorf("unsupported chain ID: %d", chainID) + } +} + func ArtifactsURLForTag(tag string) (*url.URL, error) { switch tag { case "op-contracts/v1.6.0": diff --git a/op-deployer/pkg/deployer/state/intent.go b/op-deployer/pkg/deployer/state/intent.go index 4fdee710e4a..6a45cf6c5bf 100644 --- a/op-deployer/pkg/deployer/state/intent.go +++ b/op-deployer/pkg/deployer/state/intent.go @@ -138,6 +138,21 @@ func (c *Intent) validateCustomConfig() error { } func (c *Intent) validateStrictConfig() error { + if err := c.validateStandardValues(); err != nil { + return err + } + + challenger, _ := standard.ChallengerAddressFor(c.L1ChainID) + l1ProxyAdminOwner, _ := standard.L1ProxyAdminOwner(c.L1ChainID) + for chainIndex := range c.Chains { + if c.Chains[chainIndex].Roles.Challenger != challenger { + return fmt.Errorf("invalid challenger address for chain: %s", c.Chains[chainIndex].ID) + } + if c.Chains[chainIndex].Roles.L1ProxyAdminOwner != l1ProxyAdminOwner { + return fmt.Errorf("invalid l1ProxyAdminOwner address for chain: %s", c.Chains[chainIndex].ID) + } + } + return nil } @@ -153,7 +168,6 @@ func (c *Intent) validateStandardValues() error { return fmt.Errorf("SuperchainRoles does not match standard value") } - challenger, _ := standard.ChallengerAddressFor(c.L1ChainID) for _, chain := range c.Chains { if chain.ID == emptyHash { return fmt.Errorf("missing l2 chain ID") @@ -163,8 +177,7 @@ func (c *Intent) validateStandardValues() error { } if chain.Eip1559DenominatorCanyon != standard.Eip1559DenominatorCanyon || chain.Eip1559Denominator != standard.Eip1559Denominator || - chain.Eip1559Elasticity != standard.Eip1559Elasticity || - chain.Roles.Challenger != challenger { + chain.Eip1559Elasticity != standard.Eip1559Elasticity { return fmt.Errorf("%w: chainId=%s", ErrNonStandardValue, chain.ID) } if chain.BaseFeeVaultRecipient == emptyAddress || @@ -203,6 +216,9 @@ func (c *Intent) SetInitValues(l2ChainIds []common.Hash) error { case IntentConfigTypeStandard: return c.setStandardValues(l2ChainIds) + case IntentConfigTypeStrict: + return c.setStrictValues(l2ChainIds) + case IntentConfigTypeTest: return c.setTestValues(l2ChainIds) @@ -211,6 +227,8 @@ func (c *Intent) SetInitValues(l2ChainIds []common.Hash) error { } } +// Sets all Intent fields to their zero value with the expectation that the +// user will populate the values before running 'apply' func (c *Intent) setCustomValues(l2ChainIds []common.Hash) error { c.L1ContractsLocator = &artifacts.Locator{Tag: "undefined"} c.L2ContractsLocator = &artifacts.Locator{Tag: "undefined"} @@ -234,21 +252,33 @@ func (c *Intent) setStandardValues(l2ChainIds []common.Hash) error { c.L1ContractsLocator = artifacts.DefaultL1ContractsLocator c.L2ContractsLocator = artifacts.DefaultL2ContractsLocator - challenger, _ := standard.ChallengerAddressFor(c.L1ChainID) for _, l2ChainID := range l2ChainIds { c.Chains = append(c.Chains, &ChainIntent{ ID: l2ChainID, Eip1559DenominatorCanyon: standard.Eip1559DenominatorCanyon, Eip1559Denominator: standard.Eip1559Denominator, Eip1559Elasticity: standard.Eip1559Elasticity, - Roles: ChainRoles{ - Challenger: challenger, - }, }) } return nil } +// Same as setStandardValues, but also sets l2 Challenger and L1ProxyAdminOwner +// addresses to standard values +func (c *Intent) setStrictValues(l2ChainIds []common.Hash) error { + if err := c.setStandardValues(l2ChainIds); err != nil { + return err + } + + challenger, _ := standard.ChallengerAddressFor(c.L1ChainID) + l1ProxyAdminOwner, _ := standard.ManagerOwnerAddrFor(c.L1ChainID) + for chainIndex := range c.Chains { + c.Chains[chainIndex].Roles.Challenger = challenger + c.Chains[chainIndex].Roles.L1ProxyAdminOwner = l1ProxyAdminOwner + } + return nil +} + func (c *Intent) setTestValues(l2ChainIds []common.Hash) error { c.FundDevAccounts = true c.L1ContractsLocator = artifacts.DefaultL1ContractsLocator diff --git a/op-deployer/pkg/deployer/state/intent_test.go b/op-deployer/pkg/deployer/state/intent_test.go index 99516fcbe6b..5b468392155 100644 --- a/op-deployer/pkg/deployer/state/intent_test.go +++ b/op-deployer/pkg/deployer/state/intent_test.go @@ -44,6 +44,7 @@ func setChainRoles(intent *Intent) { intent.Chains[0].Roles.UnsafeBlockSigner = common.HexToAddress("0x04") intent.Chains[0].Roles.Batcher = common.HexToAddress("0x05") intent.Chains[0].Roles.Proposer = common.HexToAddress("0x06") + intent.Chains[0].Roles.Challenger = common.HexToAddress("0x07") } func setFeeAddresses(intent *Intent) { From 745e17c7565c8badcab9995411c790c1c18149da Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Fri, 22 Nov 2024 16:05:44 -0500 Subject: [PATCH 10/14] add intent-config-type standard-overrides,strict-overrides --- op-deployer/pkg/deployer/state/intent.go | 12 ++++++++++-- op-deployer/pkg/deployer/state/intent_test.go | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/op-deployer/pkg/deployer/state/intent.go b/op-deployer/pkg/deployer/state/intent.go index 6a45cf6c5bf..35e84f7fb2f 100644 --- a/op-deployer/pkg/deployer/state/intent.go +++ b/op-deployer/pkg/deployer/state/intent.go @@ -102,9 +102,11 @@ func (c *Intent) ValidateIntentConfigType() error { if err := c.validateStrictConfig(); err != nil { return fmt.Errorf("failed to validate intent-config-type=strict: %w", err) } - case IntentConfigTypeTest: - return nil case IntentConfigTypeStandardOverrides, IntentConfigTypeStrictOverrides: + if err := c.validateCustomConfig(); err != nil { + return fmt.Errorf("failed to validate intent-config-type=%s: %w", c.IntentConfigType, err) + } + case IntentConfigTypeTest: return nil default: return fmt.Errorf("intent-config-type unsupported: %s", c.IntentConfigType) @@ -219,6 +221,12 @@ func (c *Intent) SetInitValues(l2ChainIds []common.Hash) error { case IntentConfigTypeStrict: return c.setStrictValues(l2ChainIds) + case IntentConfigTypeStrictOverrides: + return c.setStrictValues(l2ChainIds) + + case IntentConfigTypeStandardOverrides: + return c.setStandardValues(l2ChainIds) + case IntentConfigTypeTest: return c.setTestValues(l2ChainIds) diff --git a/op-deployer/pkg/deployer/state/intent_test.go b/op-deployer/pkg/deployer/state/intent_test.go index 5b468392155..edda1f03d0d 100644 --- a/op-deployer/pkg/deployer/state/intent_test.go +++ b/op-deployer/pkg/deployer/state/intent_test.go @@ -22,7 +22,7 @@ func TestValidateStandardValues(t *testing.T) { err = intent.ValidateIntentConfigType() require.NoError(t, err) - intent.Chains[0].Eip1559Denominator = 3 + intent.Chains[0].Eip1559Denominator = 3 // set to non-standard value err = intent.ValidateIntentConfigType() require.Error(t, err) require.ErrorIs(t, err, ErrNonStandardValue) From c023a1e645c37e50d1b7476f9d118930fdcc11f7 Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Mon, 25 Nov 2024 15:29:37 -0500 Subject: [PATCH 11/14] refactor intent.ConfigType validation methods, add intent NewIntent* funcs --- op-deployer/pkg/deployer/apply.go | 3 + op-deployer/pkg/deployer/init.go | 11 +- .../deployer/integration_test/apply_test.go | 7 +- op-deployer/pkg/deployer/pipeline/init.go | 10 +- op-deployer/pkg/deployer/state/intent.go | 305 ++++++++---------- op-deployer/pkg/deployer/state/intent_test.go | 25 +- 6 files changed, 160 insertions(+), 201 deletions(-) diff --git a/op-deployer/pkg/deployer/apply.go b/op-deployer/pkg/deployer/apply.go index f02716c655a..b4ff343de35 100644 --- a/op-deployer/pkg/deployer/apply.go +++ b/op-deployer/pkg/deployer/apply.go @@ -141,6 +141,9 @@ func ApplyPipeline( opts ApplyPipelineOpts, ) error { intent := opts.Intent + if err := intent.Check(); err != nil { + return err + } st := opts.State progressor := func(curr, total int64) { diff --git a/op-deployer/pkg/deployer/init.go b/op-deployer/pkg/deployer/init.go index f34897df156..deca136558c 100644 --- a/op-deployer/pkg/deployer/init.go +++ b/op-deployer/pkg/deployer/init.go @@ -86,15 +86,12 @@ func Init(cfg InitConfig) error { return fmt.Errorf("invalid config for init: %w", err) } - intent := &state.Intent{ - DeploymentStrategy: cfg.DeploymentStrategy, - IntentConfigType: cfg.IntentConfigType, - L1ChainID: cfg.L1ChainID, - } - - if err := intent.SetInitValues(cfg.L2ChainIDs); err != nil { + intent, err := state.NewIntent(cfg.IntentConfigType, cfg.DeploymentStrategy, cfg.L1ChainID, cfg.L2ChainIDs) + if err != nil { return err } + intent.DeploymentStrategy = cfg.DeploymentStrategy + intent.ConfigType = cfg.IntentConfigType st := &state.State{ Version: 1, diff --git a/op-deployer/pkg/deployer/integration_test/apply_test.go b/op-deployer/pkg/deployer/integration_test/apply_test.go index 3a661c23a14..98a5c89b851 100644 --- a/op-deployer/pkg/deployer/integration_test/apply_test.go +++ b/op-deployer/pkg/deployer/integration_test/apply_test.go @@ -714,7 +714,7 @@ func newIntent( l2Loc *artifacts.Locator, ) (*state.Intent, *state.State) { intent := &state.Intent{ - IntentConfigType: state.IntentConfigTypeTest, + ConfigType: state.IntentConfigTypeCustom, DeploymentStrategy: state.DeploymentStrategyLive, L1ChainID: l1ChainID.Uint64(), SuperchainRoles: &state.SuperchainRoles{ @@ -741,8 +741,9 @@ func newChainIntent(t *testing.T, dk *devkeys.MnemonicDevKeys, l1ChainID *big.In BaseFeeVaultRecipient: addrFor(t, dk, devkeys.BaseFeeVaultRecipientRole.Key(l1ChainID)), L1FeeVaultRecipient: addrFor(t, dk, devkeys.L1FeeVaultRecipientRole.Key(l1ChainID)), SequencerFeeVaultRecipient: addrFor(t, dk, devkeys.SequencerFeeVaultRecipientRole.Key(l1ChainID)), - Eip1559Denominator: 50, - Eip1559Elasticity: 6, + Eip1559DenominatorCanyon: standard.Eip1559DenominatorCanyon, + Eip1559Denominator: standard.Eip1559Denominator, + Eip1559Elasticity: standard.Eip1559Elasticity, Roles: state.ChainRoles{ L1ProxyAdminOwner: addrFor(t, dk, devkeys.L2ProxyAdminOwnerRole.Key(l1ChainID)), L2ProxyAdminOwner: addrFor(t, dk, devkeys.L2ProxyAdminOwnerRole.Key(l1ChainID)), diff --git a/op-deployer/pkg/deployer/pipeline/init.go b/op-deployer/pkg/deployer/pipeline/init.go index ba1fe43d147..f7be4eae03d 100644 --- a/op-deployer/pkg/deployer/pipeline/init.go +++ b/op-deployer/pkg/deployer/pipeline/init.go @@ -26,7 +26,7 @@ func InitLiveStrategy(ctx context.Context, env *Env, intent *state.Intent, st *s lgr := env.Logger.New("stage", "init", "strategy", "live") lgr.Info("initializing pipeline") - if err := initCommonChecks(st, intent); err != nil { + if err := initCommonChecks(st); err != nil { return err } @@ -100,11 +100,7 @@ func InitLiveStrategy(ctx context.Context, env *Env, intent *state.Intent, st *s return nil } -func initCommonChecks(st *state.State, intent *state.Intent) error { - if err := intent.ValidateIntentConfigType(); err != nil { - return err - } - +func initCommonChecks(st *state.State) error { // Ensure the state version is supported. if !IsSupportedStateVersion(st.Version) { return fmt.Errorf("unsupported state version: %d", st.Version) @@ -123,7 +119,7 @@ func InitGenesisStrategy(env *Env, intent *state.Intent, st *state.State) error lgr := env.Logger.New("stage", "init", "strategy", "genesis") lgr.Info("initializing pipeline") - if err := initCommonChecks(st, intent); err != nil { + if err := initCommonChecks(st); err != nil { return err } diff --git a/op-deployer/pkg/deployer/state/intent.go b/op-deployer/pkg/deployer/state/intent.go index 35e84f7fb2f..e8a805bef27 100644 --- a/op-deployer/pkg/deployer/state/intent.go +++ b/op-deployer/pkg/deployer/state/intent.go @@ -6,8 +6,6 @@ import ( "math/big" "reflect" - "github.com/ethereum-optimism/optimism/op-chain-ops/devkeys" - "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/artifacts" "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/standard" @@ -36,7 +34,6 @@ func (d DeploymentStrategy) Check() error { type IntentConfigType string const ( - IntentConfigTypeTest IntentConfigType = "test" IntentConfigTypeStandard IntentConfigType = "standard" IntentConfigTypeCustom IntentConfigType = "custom" IntentConfigTypeStrict IntentConfigType = "strict" @@ -49,7 +46,7 @@ var emptyHash common.Hash type Intent struct { DeploymentStrategy DeploymentStrategy `json:"deploymentStrategy" toml:"deploymentStrategy"` - IntentConfigType IntentConfigType `json:"intentConfigType" toml:"intentConfigType"` + ConfigType IntentConfigType `json:"configType" toml:"configType"` L1ChainID uint64 `json:"l1ChainID" toml:"l1ChainID"` SuperchainRoles *SuperchainRoles `json:"superchainRoles" toml:"superchainRoles,omitempty"` FundDevAccounts bool `json:"fundDevAccounts" toml:"fundDevAccounts"` @@ -88,32 +85,6 @@ func (c *Intent) L1ChainIDBig() *big.Int { return big.NewInt(int64(c.L1ChainID)) } -func (c *Intent) ValidateIntentConfigType() error { - switch c.IntentConfigType { - case IntentConfigTypeStandard: - if err := c.validateStandardValues(); err != nil { - return fmt.Errorf("failed to validate intent-config-type=standard: %w", err) - } - case IntentConfigTypeCustom: - if err := c.validateCustomConfig(); err != nil { - return fmt.Errorf("failed to validate intent-config-type=custom: %w", err) - } - case IntentConfigTypeStrict: - if err := c.validateStrictConfig(); err != nil { - return fmt.Errorf("failed to validate intent-config-type=strict: %w", err) - } - case IntentConfigTypeStandardOverrides, IntentConfigTypeStrictOverrides: - if err := c.validateCustomConfig(); err != nil { - return fmt.Errorf("failed to validate intent-config-type=%s: %w", c.IntentConfigType, err) - } - case IntentConfigTypeTest: - return nil - default: - return fmt.Errorf("intent-config-type unsupported: %s", c.IntentConfigType) - } - return nil -} - func (c *Intent) validateCustomConfig() error { if c.L1ContractsLocator == nil || c.L1ContractsLocator.Tag == "undefined" { return errors.New("L1ContractsLocator undefined") @@ -162,6 +133,13 @@ func (c *Intent) validateStrictConfig() error { // 1. no zero-values for non-standard fields (user should have populated these) // 2. no non-standard values for standard fields (user should not have changed these) func (c *Intent) validateStandardValues() error { + if c.L1ContractsLocator == nil || c.L1ContractsLocator != artifacts.DefaultL1ContractsLocator { + return fmt.Errorf("L1ContractsLocator does not match standard value") + } + if c.L2ContractsLocator == nil || c.L2ContractsLocator != artifacts.DefaultL2ContractsLocator { + return fmt.Errorf("L2ContractsLocator does not match standard value") + } + standardSuperchainRoles, err := getStandardSuperchainRoles(c.L1ChainID) if err != nil { return fmt.Errorf("error getting standard superchain roles: %w", err) @@ -171,10 +149,7 @@ func (c *Intent) validateStandardValues() error { } for _, chain := range c.Chains { - if chain.ID == emptyHash { - return fmt.Errorf("missing l2 chain ID") - } - if err := chain.Roles.CheckNoZeroAddresses(); err != nil { + if err := chain.CheckNoZeroValues(); err != nil { return err } if chain.Eip1559DenominatorCanyon != standard.Eip1559DenominatorCanyon || @@ -182,11 +157,6 @@ func (c *Intent) validateStandardValues() error { chain.Eip1559Elasticity != standard.Eip1559Elasticity { return fmt.Errorf("%w: chainId=%s", ErrNonStandardValue, chain.ID) } - if chain.BaseFeeVaultRecipient == emptyAddress || - chain.L1FeeVaultRecipient == emptyAddress || - chain.SequencerFeeVaultRecipient == emptyAddress { - return fmt.Errorf("%w: chainId=%s", ErrFeeVaultZeroAddress, chain.ID) - } } return nil @@ -210,140 +180,33 @@ func getStandardSuperchainRoles(l1ChainId uint64) (*SuperchainRoles, error) { return superchainRoles, nil } -func (c *Intent) SetInitValues(l2ChainIds []common.Hash) error { - switch c.IntentConfigType { - case IntentConfigTypeCustom: - return c.setCustomValues(l2ChainIds) - - case IntentConfigTypeStandard: - return c.setStandardValues(l2ChainIds) - - case IntentConfigTypeStrict: - return c.setStrictValues(l2ChainIds) - - case IntentConfigTypeStrictOverrides: - return c.setStrictValues(l2ChainIds) - - case IntentConfigTypeStandardOverrides: - return c.setStandardValues(l2ChainIds) - - case IntentConfigTypeTest: - return c.setTestValues(l2ChainIds) - - default: - return fmt.Errorf("intent config type not supported") - } -} - -// Sets all Intent fields to their zero value with the expectation that the -// user will populate the values before running 'apply' -func (c *Intent) setCustomValues(l2ChainIds []common.Hash) error { - c.L1ContractsLocator = &artifacts.Locator{Tag: "undefined"} - c.L2ContractsLocator = &artifacts.Locator{Tag: "undefined"} - - c.SuperchainRoles = &SuperchainRoles{} - for _, l2ChainID := range l2ChainIds { - c.Chains = append(c.Chains, &ChainIntent{ - ID: l2ChainID, - }) - } - return nil -} - -func (c *Intent) setStandardValues(l2ChainIds []common.Hash) error { - superchainRoles, err := getStandardSuperchainRoles(c.L1ChainID) - if err != nil { - return fmt.Errorf("error getting standard superchain roles: %w", err) - } - c.SuperchainRoles = superchainRoles - - c.L1ContractsLocator = artifacts.DefaultL1ContractsLocator - c.L2ContractsLocator = artifacts.DefaultL2ContractsLocator - - for _, l2ChainID := range l2ChainIds { - c.Chains = append(c.Chains, &ChainIntent{ - ID: l2ChainID, - Eip1559DenominatorCanyon: standard.Eip1559DenominatorCanyon, - Eip1559Denominator: standard.Eip1559Denominator, - Eip1559Elasticity: standard.Eip1559Elasticity, - }) - } - return nil -} - -// Same as setStandardValues, but also sets l2 Challenger and L1ProxyAdminOwner -// addresses to standard values -func (c *Intent) setStrictValues(l2ChainIds []common.Hash) error { - if err := c.setStandardValues(l2ChainIds); err != nil { - return err - } - - challenger, _ := standard.ChallengerAddressFor(c.L1ChainID) - l1ProxyAdminOwner, _ := standard.ManagerOwnerAddrFor(c.L1ChainID) - for chainIndex := range c.Chains { - c.Chains[chainIndex].Roles.Challenger = challenger - c.Chains[chainIndex].Roles.L1ProxyAdminOwner = l1ProxyAdminOwner - } - return nil -} - -func (c *Intent) setTestValues(l2ChainIds []common.Hash) error { - c.FundDevAccounts = true - c.L1ContractsLocator = artifacts.DefaultL1ContractsLocator - c.L2ContractsLocator = artifacts.DefaultL2ContractsLocator - - l1ChainIDBig := c.L1ChainIDBig() - - dk, err := devkeys.NewMnemonicDevKeys(devkeys.TestMnemonic) - if err != nil { - return fmt.Errorf("failed to create dev keys: %w", err) - } - - addrFor := func(key devkeys.Key) common.Address { - // The error below should never happen, so panic if it does. - addr, err := dk.Address(key) - if err != nil { - panic(err) - } - return addr - } - c.SuperchainRoles = &SuperchainRoles{ - ProxyAdminOwner: addrFor(devkeys.L1ProxyAdminOwnerRole.Key(l1ChainIDBig)), - ProtocolVersionsOwner: addrFor(devkeys.SuperchainProtocolVersionsOwner.Key(l1ChainIDBig)), - Guardian: addrFor(devkeys.SuperchainConfigGuardianKey.Key(l1ChainIDBig)), - } - - for _, l2ChainID := range l2ChainIds { - l2ChainIDBig := l2ChainID.Big() - c.Chains = append(c.Chains, &ChainIntent{ - ID: l2ChainID, - BaseFeeVaultRecipient: addrFor(devkeys.BaseFeeVaultRecipientRole.Key(l2ChainIDBig)), - L1FeeVaultRecipient: addrFor(devkeys.L1FeeVaultRecipientRole.Key(l2ChainIDBig)), - SequencerFeeVaultRecipient: addrFor(devkeys.SequencerFeeVaultRecipientRole.Key(l2ChainIDBig)), - Eip1559DenominatorCanyon: standard.Eip1559DenominatorCanyon, - Eip1559Denominator: standard.Eip1559Denominator, - Eip1559Elasticity: standard.Eip1559Elasticity, - Roles: ChainRoles{ - L1ProxyAdminOwner: addrFor(devkeys.L1ProxyAdminOwnerRole.Key(l2ChainIDBig)), - L2ProxyAdminOwner: addrFor(devkeys.L2ProxyAdminOwnerRole.Key(l2ChainIDBig)), - SystemConfigOwner: addrFor(devkeys.SystemConfigOwner.Key(l2ChainIDBig)), - UnsafeBlockSigner: addrFor(devkeys.SequencerP2PRole.Key(l2ChainIDBig)), - Batcher: addrFor(devkeys.BatcherRole.Key(l2ChainIDBig)), - Proposer: addrFor(devkeys.ProposerRole.Key(l2ChainIDBig)), - Challenger: addrFor(devkeys.ChallengerRole.Key(l2ChainIDBig)), - }, - }) - } - return nil -} - func (c *Intent) Check() error { + if c.L1ChainID == 0 { + return fmt.Errorf("l1ChainID must be set") + } if err := c.DeploymentStrategy.Check(); err != nil { return err } - if c.L1ChainID == 0 { - return fmt.Errorf("l1ChainID must be set") + switch c.ConfigType { + case IntentConfigTypeStandard: + if err := c.validateStandardValues(); err != nil { + return fmt.Errorf("failed to validate intent-config-type=standard: %w", err) + } + case IntentConfigTypeCustom: + if err := c.validateCustomConfig(); err != nil { + return fmt.Errorf("failed to validate intent-config-type=custom: %w", err) + } + case IntentConfigTypeStrict: + if err := c.validateStrictConfig(); err != nil { + return fmt.Errorf("failed to validate intent-config-type=strict: %w", err) + } + case IntentConfigTypeStandardOverrides, IntentConfigTypeStrictOverrides: + if err := c.validateCustomConfig(); err != nil { + return fmt.Errorf("failed to validate intent-config-type=%s: %w", c.ConfigType, err) + } + default: + return fmt.Errorf("intent-config-type unsupported: %s", c.ConfigType) } if c.L1ContractsLocator == nil { @@ -420,3 +283,109 @@ func (c *Intent) checkL2Prod() error { _, err := standard.ArtifactsURLForTag(c.L2ContractsLocator.Tag) return err } + +func NewIntent(configType IntentConfigType, deploymentStrategy DeploymentStrategy, l1ChainId uint64, l2ChainIds []common.Hash) (Intent, error) { + switch configType { + case IntentConfigTypeCustom: + return NewIntentCustom(deploymentStrategy, l1ChainId, l2ChainIds) + + case IntentConfigTypeStandard: + return NewIntentStandard(deploymentStrategy, l1ChainId, l2ChainIds) + + case IntentConfigTypeStandardOverrides: + return NewIntentStandardOverrides(deploymentStrategy, l1ChainId, l2ChainIds) + + case IntentConfigTypeStrict: + return NewIntentStrict(deploymentStrategy, l1ChainId, l2ChainIds) + + case IntentConfigTypeStrictOverrides: + return NewIntentStrictOverrides(deploymentStrategy, l1ChainId, l2ChainIds) + + default: + return Intent{}, fmt.Errorf("intent config type not supported") + } +} + +// Sets all Intent fields to their zero value with the expectation that the +// user will populate the values before running 'apply' +func NewIntentCustom(deploymentStrategy DeploymentStrategy, l1ChainId uint64, l2ChainIds []common.Hash) (Intent, error) { + intent := Intent{ + DeploymentStrategy: deploymentStrategy, + ConfigType: IntentConfigTypeCustom, + L1ChainID: l1ChainId, + L1ContractsLocator: &artifacts.Locator{Tag: ""}, + L2ContractsLocator: &artifacts.Locator{Tag: ""}, + SuperchainRoles: &SuperchainRoles{}, + } + + for _, l2ChainID := range l2ChainIds { + intent.Chains = append(intent.Chains, &ChainIntent{ + ID: l2ChainID, + }) + } + return intent, nil +} + +func NewIntentStandard(deploymentStrategy DeploymentStrategy, l1ChainId uint64, l2ChainIds []common.Hash) (Intent, error) { + intent := Intent{ + DeploymentStrategy: deploymentStrategy, + ConfigType: IntentConfigTypeStandard, + L1ChainID: l1ChainId, + L1ContractsLocator: artifacts.DefaultL1ContractsLocator, + L2ContractsLocator: artifacts.DefaultL2ContractsLocator, + } + + superchainRoles, err := getStandardSuperchainRoles(l1ChainId) + if err != nil { + return Intent{}, fmt.Errorf("error getting standard superchain roles: %w", err) + } + intent.SuperchainRoles = superchainRoles + + for _, l2ChainID := range l2ChainIds { + intent.Chains = append(intent.Chains, &ChainIntent{ + ID: l2ChainID, + Eip1559DenominatorCanyon: standard.Eip1559DenominatorCanyon, + Eip1559Denominator: standard.Eip1559Denominator, + Eip1559Elasticity: standard.Eip1559Elasticity, + }) + } + return intent, nil +} + +func NewIntentStandardOverrides(deploymentStrategy DeploymentStrategy, l1ChainId uint64, l2ChainIds []common.Hash) (Intent, error) { + intent, err := NewIntentStandard(deploymentStrategy, l1ChainId, l2ChainIds) + if err != nil { + return Intent{}, err + } + intent.ConfigType = IntentConfigTypeStandardOverrides + + return intent, nil +} + +// Same as NewIntentStandard, but also sets l2 Challenger and L1ProxyAdminOwner +// addresses to standard values +func NewIntentStrict(deploymentStrategy DeploymentStrategy, l1ChainId uint64, l2ChainIds []common.Hash) (Intent, error) { + intent, err := NewIntentStandard(deploymentStrategy, l1ChainId, l2ChainIds) + if err != nil { + return Intent{}, err + } + intent.ConfigType = IntentConfigTypeStrict + + challenger, _ := standard.ChallengerAddressFor(l1ChainId) + l1ProxyAdminOwner, _ := standard.ManagerOwnerAddrFor(l1ChainId) + for chainIndex := range intent.Chains { + intent.Chains[chainIndex].Roles.Challenger = challenger + intent.Chains[chainIndex].Roles.L1ProxyAdminOwner = l1ProxyAdminOwner + } + return intent, nil +} + +func NewIntentStrictOverrides(deploymentStrategy DeploymentStrategy, l1ChainId uint64, l2ChainIds []common.Hash) (Intent, error) { + intent, err := NewIntentStrict(deploymentStrategy, l1ChainId, l2ChainIds) + if err != nil { + return Intent{}, err + } + intent.ConfigType = IntentConfigTypeStrictOverrides + + return intent, nil +} diff --git a/op-deployer/pkg/deployer/state/intent_test.go b/op-deployer/pkg/deployer/state/intent_test.go index edda1f03d0d..07f5c8d4b70 100644 --- a/op-deployer/pkg/deployer/state/intent_test.go +++ b/op-deployer/pkg/deployer/state/intent_test.go @@ -8,35 +8,28 @@ import ( ) func TestValidateStandardValues(t *testing.T) { - intent := initStandardIntent() - err := intent.ValidateIntentConfigType() + intent, err := NewIntentStandard(DeploymentStrategyLive, 1, []common.Hash{common.HexToHash("0x336")}) + require.NoError(t, err) + + err = intent.Check() require.Error(t, err) require.ErrorIs(t, err, ErrChainRoleZeroAddress) - setChainRoles(intent) - err = intent.ValidateIntentConfigType() + setChainRoles(&intent) + err = intent.Check() require.Error(t, err) require.ErrorIs(t, err, ErrFeeVaultZeroAddress) - setFeeAddresses(intent) - err = intent.ValidateIntentConfigType() + setFeeAddresses(&intent) + err = intent.Check() require.NoError(t, err) intent.Chains[0].Eip1559Denominator = 3 // set to non-standard value - err = intent.ValidateIntentConfigType() + err = intent.Check() require.Error(t, err) require.ErrorIs(t, err, ErrNonStandardValue) } -func initStandardIntent() *Intent { - intent := Intent{ - L1ChainID: 1, - IntentConfigType: IntentConfigTypeStandard, - } - _ = intent.setStandardValues([]common.Hash{common.HexToHash("0x336")}) - return &intent -} - func setChainRoles(intent *Intent) { intent.Chains[0].Roles.L1ProxyAdminOwner = common.HexToAddress("0x01") intent.Chains[0].Roles.L2ProxyAdminOwner = common.HexToAddress("0x02") From aaa8d608d247901a242f38821ea31e7e0a3155a7 Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Mon, 25 Nov 2024 20:19:55 -0500 Subject: [PATCH 12/14] add TestValidateCustomValues, refactor contract tag check --- op-deployer/pkg/deployer/artifacts/locator.go | 6 +- op-deployer/pkg/deployer/state/intent.go | 94 +++++++------------ op-deployer/pkg/deployer/state/intent_test.go | 42 +++++++++ 3 files changed, 75 insertions(+), 67 deletions(-) diff --git a/op-deployer/pkg/deployer/artifacts/locator.go b/op-deployer/pkg/deployer/artifacts/locator.go index aa44d43644c..42b838f66a7 100644 --- a/op-deployer/pkg/deployer/artifacts/locator.go +++ b/op-deployer/pkg/deployer/artifacts/locator.go @@ -70,11 +70,7 @@ func (a *Locator) MarshalText() ([]byte, error) { return []byte(a.URL.String()), nil } - if a.Tag != "" { - return []byte("tag://" + a.Tag), nil - } - - return nil, fmt.Errorf("no URL, path or tag set") + return []byte("tag://" + a.Tag), nil } func (a *Locator) IsTag() bool { diff --git a/op-deployer/pkg/deployer/state/intent.go b/op-deployer/pkg/deployer/state/intent.go index e8a805bef27..fde5f92f3b4 100644 --- a/op-deployer/pkg/deployer/state/intent.go +++ b/op-deployer/pkg/deployer/state/intent.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "math/big" + "net/url" "reflect" "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/artifacts" @@ -63,7 +64,9 @@ type SuperchainRoles struct { Guardian common.Address `json:"guardian" toml:"guardian"` } -var ErrSuperchainRoleZeroAddress = fmt.Errorf("SuperchainRole is set to zero address") +var ErrSuperchainRoleZeroAddress = errors.New("SuperchainRole is set to zero address") +var ErrL1ContractsLocatorUndefined = errors.New("L1ContractsLocator undefined") +var ErrL2ContractsLocatorUndefined = errors.New("L2ContractsLocator undefined") func (s *SuperchainRoles) CheckNoZeroAddresses() error { val := reflect.ValueOf(*s) @@ -86,11 +89,13 @@ func (c *Intent) L1ChainIDBig() *big.Int { } func (c *Intent) validateCustomConfig() error { - if c.L1ContractsLocator == nil || c.L1ContractsLocator.Tag == "undefined" { - return errors.New("L1ContractsLocator undefined") + if c.L1ContractsLocator == nil || + (c.L1ContractsLocator.Tag == "" && c.L1ContractsLocator.URL == &url.URL{}) { + return ErrL1ContractsLocatorUndefined } - if c.L2ContractsLocator == nil || c.L2ContractsLocator.Tag == "undefined" { - return errors.New("L2ContractsLocator undefined") + if c.L2ContractsLocator == nil || + (c.L2ContractsLocator.Tag == "" && c.L2ContractsLocator.URL == &url.URL{}) { + return ErrL2ContractsLocatorUndefined } if err := c.SuperchainRoles.CheckNoZeroAddresses(); err != nil { @@ -133,11 +138,11 @@ func (c *Intent) validateStrictConfig() error { // 1. no zero-values for non-standard fields (user should have populated these) // 2. no non-standard values for standard fields (user should not have changed these) func (c *Intent) validateStandardValues() error { - if c.L1ContractsLocator == nil || c.L1ContractsLocator != artifacts.DefaultL1ContractsLocator { - return fmt.Errorf("L1ContractsLocator does not match standard value") + if err := c.checkL1Prod(); err != nil { + return err } - if c.L2ContractsLocator == nil || c.L2ContractsLocator != artifacts.DefaultL2ContractsLocator { - return fmt.Errorf("L2ContractsLocator does not match standard value") + if err := c.checkL2Prod(); err != nil { + return err } standardSuperchainRoles, err := getStandardSuperchainRoles(c.L1ChainID) @@ -182,55 +187,36 @@ func getStandardSuperchainRoles(l1ChainId uint64) (*SuperchainRoles, error) { func (c *Intent) Check() error { if c.L1ChainID == 0 { - return fmt.Errorf("l1ChainID must be set") + return fmt.Errorf("l1ChainID cannot be 0") } + if err := c.DeploymentStrategy.Check(); err != nil { return err } - switch c.ConfigType { - case IntentConfigTypeStandard: - if err := c.validateStandardValues(); err != nil { - return fmt.Errorf("failed to validate intent-config-type=standard: %w", err) - } - case IntentConfigTypeCustom: - if err := c.validateCustomConfig(); err != nil { - return fmt.Errorf("failed to validate intent-config-type=custom: %w", err) - } - case IntentConfigTypeStrict: - if err := c.validateStrictConfig(); err != nil { - return fmt.Errorf("failed to validate intent-config-type=strict: %w", err) - } - case IntentConfigTypeStandardOverrides, IntentConfigTypeStrictOverrides: - if err := c.validateCustomConfig(); err != nil { - return fmt.Errorf("failed to validate intent-config-type=%s: %w", c.ConfigType, err) - } - default: - return fmt.Errorf("intent-config-type unsupported: %s", c.ConfigType) - } - if c.L1ContractsLocator == nil { - return errors.New("l1ContractsLocator must be set") + return ErrL1ContractsLocatorUndefined } if c.L2ContractsLocator == nil { - return errors.New("l2ContractsLocator must be set") + return ErrL2ContractsLocatorUndefined } var err error - if c.L1ContractsLocator.IsTag() { - err = c.checkL1Prod() - } else { - err = c.checkL1Dev() + switch c.ConfigType { + case IntentConfigTypeStandard: + err = c.validateStandardValues() + case IntentConfigTypeCustom: + err = c.validateCustomConfig() + case IntentConfigTypeStrict: + err = c.validateStrictConfig() + case IntentConfigTypeStandardOverrides, IntentConfigTypeStrictOverrides: + err = c.validateCustomConfig() + default: + return fmt.Errorf("intent-config-type unsupported: %s", c.ConfigType) } if err != nil { - return err - } - - if c.L2ContractsLocator.IsTag() { - if err := c.checkL2Prod(); err != nil { - return err - } + return fmt.Errorf("failed to validate intent-config-type=%s: %w", c.ConfigType, err) } return nil @@ -263,22 +249,6 @@ func (c *Intent) checkL1Prod() error { return nil } -func (c *Intent) checkL1Dev() error { - if c.SuperchainRoles.ProxyAdminOwner == emptyAddress { - return fmt.Errorf("proxyAdminOwner must be set") - } - - if c.SuperchainRoles.ProtocolVersionsOwner == emptyAddress { - c.SuperchainRoles.ProtocolVersionsOwner = c.SuperchainRoles.ProxyAdminOwner - } - - if c.SuperchainRoles.Guardian == emptyAddress { - c.SuperchainRoles.Guardian = c.SuperchainRoles.ProxyAdminOwner - } - - return nil -} - func (c *Intent) checkL2Prod() error { _, err := standard.ArtifactsURLForTag(c.L2ContractsLocator.Tag) return err @@ -313,8 +283,8 @@ func NewIntentCustom(deploymentStrategy DeploymentStrategy, l1ChainId uint64, l2 DeploymentStrategy: deploymentStrategy, ConfigType: IntentConfigTypeCustom, L1ChainID: l1ChainId, - L1ContractsLocator: &artifacts.Locator{Tag: ""}, - L2ContractsLocator: &artifacts.Locator{Tag: ""}, + L1ContractsLocator: &artifacts.Locator{URL: &url.URL{}}, + L2ContractsLocator: &artifacts.Locator{URL: &url.URL{}}, SuperchainRoles: &SuperchainRoles{}, } diff --git a/op-deployer/pkg/deployer/state/intent_test.go b/op-deployer/pkg/deployer/state/intent_test.go index 07f5c8d4b70..1d8c12375f8 100644 --- a/op-deployer/pkg/deployer/state/intent_test.go +++ b/op-deployer/pkg/deployer/state/intent_test.go @@ -30,6 +30,48 @@ func TestValidateStandardValues(t *testing.T) { require.ErrorIs(t, err, ErrNonStandardValue) } +func TestValidateCustomValues(t *testing.T) { + intent, err := NewIntentCustom(DeploymentStrategyLive, 1, []common.Hash{common.HexToHash("0x336")}) + require.NoError(t, err) + + err = intent.Check() + require.Error(t, err) + require.ErrorIs(t, err, ErrSuperchainRoleZeroAddress) + + setSuperchainRoles(&intent) + err = intent.Check() + require.Error(t, err) + require.ErrorIs(t, err, ErrChainRoleZeroAddress) + + setChainRoles(&intent) + err = intent.Check() + require.Error(t, err) + require.ErrorIs(t, err, ErrEip1559ZeroValue) + + setEip1559Params(&intent) + err = intent.Check() + require.Error(t, err) + require.ErrorIs(t, err, ErrFeeVaultZeroAddress) + + setFeeAddresses(&intent) + err = intent.Check() + require.NoError(t, err) +} + +func setSuperchainRoles(intent *Intent) { + intent.SuperchainRoles = &SuperchainRoles{ + ProxyAdminOwner: common.HexToAddress("0xa"), + ProtocolVersionsOwner: common.HexToAddress("0xb"), + Guardian: common.HexToAddress("0xc"), + } +} + +func setEip1559Params(intent *Intent) { + intent.Chains[0].Eip1559Denominator = 5000 + intent.Chains[0].Eip1559DenominatorCanyon = 5000 + intent.Chains[0].Eip1559Elasticity = 5000 +} + func setChainRoles(intent *Intent) { intent.Chains[0].Roles.L1ProxyAdminOwner = common.HexToAddress("0x01") intent.Chains[0].Roles.L2ProxyAdminOwner = common.HexToAddress("0x02") From 3af78505199b03b60f79ce3449e43ed6ad95bb84 Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Wed, 27 Nov 2024 10:43:55 -0500 Subject: [PATCH 13/14] remove duplicate chainIntent.Check() code --- .../pkg/deployer/state/chain_intent.go | 37 ++----------------- op-deployer/pkg/deployer/state/intent.go | 4 +- 2 files changed, 6 insertions(+), 35 deletions(-) diff --git a/op-deployer/pkg/deployer/state/chain_intent.go b/op-deployer/pkg/deployer/state/chain_intent.go index f444a766abc..bb6693f56b8 100644 --- a/op-deployer/pkg/deployer/state/chain_intent.go +++ b/op-deployer/pkg/deployer/state/chain_intent.go @@ -1,7 +1,6 @@ package state import ( - "errors" "fmt" "reflect" @@ -38,42 +37,10 @@ var ErrNonStandardValue = fmt.Errorf("chain contains non-standard config value") var ErrEip1559ZeroValue = fmt.Errorf("eip1559 param is set to zero value") func (c *ChainIntent) Check() error { - var emptyHash common.Hash if c.ID == emptyHash { return fmt.Errorf("id must be set") } - if c.Roles.L1ProxyAdminOwner == emptyAddress { - return fmt.Errorf("proxyAdminOwner must be set") - } - - if c.Roles.L2ProxyAdminOwner == emptyAddress { - return fmt.Errorf("l2ProxyAdminOwner must be set") - } - - if c.Roles.SystemConfigOwner == emptyAddress { - c.Roles.SystemConfigOwner = c.Roles.L1ProxyAdminOwner - } - - if c.Roles.UnsafeBlockSigner == emptyAddress { - return fmt.Errorf("unsafeBlockSigner must be set") - } - - if c.Roles.Batcher == emptyAddress { - return fmt.Errorf("batcher must be set") - } - - if c.DangerousAltDAConfig.UseAltDA { - return c.DangerousAltDAConfig.Check(nil) - } - - return nil -} - -func (c *ChainIntent) CheckNoZeroValues() error { - if c.ID == emptyHash { - return errors.New("missing l2 chain ID") - } if err := c.Roles.CheckNoZeroAddresses(); err != nil { return err } @@ -89,6 +56,10 @@ func (c *ChainIntent) CheckNoZeroValues() error { return fmt.Errorf("%w: chainId=%s", ErrFeeVaultZeroAddress, c.ID) } + if c.DangerousAltDAConfig.UseAltDA { + return c.DangerousAltDAConfig.Check(nil) + } + return nil } diff --git a/op-deployer/pkg/deployer/state/intent.go b/op-deployer/pkg/deployer/state/intent.go index fde5f92f3b4..371c6d3d8a1 100644 --- a/op-deployer/pkg/deployer/state/intent.go +++ b/op-deployer/pkg/deployer/state/intent.go @@ -107,7 +107,7 @@ func (c *Intent) validateCustomConfig() error { } for _, chain := range c.Chains { - if err := chain.CheckNoZeroValues(); err != nil { + if err := chain.Check(); err != nil { return err } } @@ -154,7 +154,7 @@ func (c *Intent) validateStandardValues() error { } for _, chain := range c.Chains { - if err := chain.CheckNoZeroValues(); err != nil { + if err := chain.Check(); err != nil { return err } if chain.Eip1559DenominatorCanyon != standard.Eip1559DenominatorCanyon || From 55376b2bd3342ed62c6d2373af1c0229ab030ee9 Mon Sep 17 00:00:00 2001 From: Samuel Stokes Date: Wed, 27 Nov 2024 14:33:48 -0500 Subject: [PATCH 14/14] fix SuperchainRoles.ProxyAdminOwner for standard chains --- op-deployer/pkg/deployer/state/intent.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/op-deployer/pkg/deployer/state/intent.go b/op-deployer/pkg/deployer/state/intent.go index 371c6d3d8a1..ab76eb4fb4f 100644 --- a/op-deployer/pkg/deployer/state/intent.go +++ b/op-deployer/pkg/deployer/state/intent.go @@ -173,11 +173,11 @@ func getStandardSuperchainRoles(l1ChainId uint64) (*SuperchainRoles, error) { return nil, fmt.Errorf("error getting superchain config: %w", err) } - proxyAdmin, _ := standard.ManagerOwnerAddrFor(l1ChainId) + proxyAdminOwner, _ := standard.L1ProxyAdminOwner(l1ChainId) guardian, _ := standard.GuardianAddressFor(l1ChainId) superchainRoles := &SuperchainRoles{ - ProxyAdminOwner: proxyAdmin, + ProxyAdminOwner: proxyAdminOwner, ProtocolVersionsOwner: common.Address(*superCfg.Config.ProtocolVersionsAddr), Guardian: guardian, }