Skip to content

Commit f2d6626

Browse files
mslippermslipper
authored
op-deployer: Fix broken init logic (#14827) (#14829)
Previously, any deployment using tagged releases on an official chain like Sepolia would use the predeployed OPCM. The predeployed OPCM would point these chains to the global `SuperchainConfig`, which may not have been the intended behavior. This PR updates the logic in the deployment pipeline to use the global `SuperchainConfig` under the following conditions: 1. The user specifies intent type `standard`. 2. The user specifies intent type `standard-overrides` and does not modify the Superchain roles.
1 parent e1516f0 commit f2d6626

File tree

5 files changed

+246
-21
lines changed

5 files changed

+246
-21
lines changed

‎op-deployer/pkg/deployer/pipeline/init.go

+25-16
Original file line numberDiff line numberDiff line change
@@ -38,25 +38,34 @@ func InitLiveStrategy(ctx context.Context, env *Env, intent *state.Intent, st *s
3838
return fmt.Errorf("unsupported L2 version: %s", intent.L2ContractsLocator.Tag)
3939
}
4040

41-
if isL1Tag && hasPredeployedOPCM {
42-
superCfg, err := standard.SuperchainFor(intent.L1ChainID)
41+
isStandardIntent := intent.ConfigType == state.IntentTypeStandard ||
42+
intent.ConfigType == state.IntentTypeStandardOverrides
43+
if isL1Tag && hasPredeployedOPCM && isStandardIntent {
44+
stdRoles, err := state.GetStandardSuperchainRoles(intent.L1ChainID)
4345
if err != nil {
44-
return fmt.Errorf("error getting superchain config: %w", err)
46+
return fmt.Errorf("error getting superchain roles: %w", err)
4547
}
4648

47-
proxyAdmin, err := standard.SuperchainProxyAdminAddrFor(intent.L1ChainID)
48-
if err != nil {
49-
return fmt.Errorf("error getting superchain proxy admin address: %w", err)
50-
}
51-
52-
st.SuperchainDeployment = &state.SuperchainDeployment{
53-
ProxyAdminAddress: proxyAdmin,
54-
ProtocolVersionsProxyAddress: superCfg.ProtocolVersionsAddr,
55-
SuperchainConfigProxyAddress: superCfg.SuperchainConfigAddr,
56-
}
57-
58-
st.ImplementationsDeployment = &state.ImplementationsDeployment{
59-
OpcmAddress: opcmAddress,
49+
if *intent.SuperchainRoles == *stdRoles {
50+
superCfg, err := standard.SuperchainFor(intent.L1ChainID)
51+
if err != nil {
52+
return fmt.Errorf("error getting superchain config: %w", err)
53+
}
54+
55+
proxyAdmin, err := standard.SuperchainProxyAdminAddrFor(intent.L1ChainID)
56+
if err != nil {
57+
return fmt.Errorf("error getting superchain proxy admin address: %w", err)
58+
}
59+
60+
st.SuperchainDeployment = &state.SuperchainDeployment{
61+
ProxyAdminAddress: proxyAdmin,
62+
ProtocolVersionsProxyAddress: superCfg.ProtocolVersionsAddr,
63+
SuperchainConfigProxyAddress: superCfg.SuperchainConfigAddr,
64+
}
65+
66+
st.ImplementationsDeployment = &state.ImplementationsDeployment{
67+
OpcmAddress: opcmAddress,
68+
}
6069
}
6170
}
6271

‎op-deployer/pkg/deployer/pipeline/init_test.go

+171
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
package pipeline
2+
3+
import (
4+
"context"
5+
"log/slog"
6+
"os"
7+
"testing"
8+
"time"
9+
10+
"github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/artifacts"
11+
"github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/standard"
12+
"github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/state"
13+
"github.com/ethereum-optimism/optimism/op-service/testlog"
14+
"github.com/ethereum-optimism/optimism/op-service/testutils/devnet"
15+
"github.com/ethereum/go-ethereum/common"
16+
"github.com/ethereum/go-ethereum/ethclient"
17+
"github.com/stretchr/testify/require"
18+
)
19+
20+
func TestInitLiveStrategy_OPCMReuseLogicSepolia(t *testing.T) {
21+
rpcURL := os.Getenv("SEPOLIA_RPC_URL")
22+
require.NotEmpty(t, rpcURL, "SEPOLIA_RPC_URL must be set")
23+
24+
lgr := testlog.Logger(t, slog.LevelInfo)
25+
retryProxy := devnet.NewRetryProxy(lgr, rpcURL)
26+
require.NoError(t, retryProxy.Start())
27+
t.Cleanup(func() {
28+
require.NoError(t, retryProxy.Stop())
29+
})
30+
31+
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
32+
defer cancel()
33+
34+
client, err := ethclient.Dial(retryProxy.Endpoint())
35+
require.NoError(t, err)
36+
37+
l1ChainID := uint64(11155111)
38+
t.Run("untagged L1 locator", func(t *testing.T) {
39+
st := &state.State{
40+
Version: 1,
41+
}
42+
require.NoError(t, InitLiveStrategy(
43+
ctx,
44+
&Env{
45+
L1Client: client,
46+
Logger: lgr,
47+
},
48+
&state.Intent{
49+
L1ChainID: l1ChainID,
50+
L1ContractsLocator: artifacts.MustNewLocatorFromURL("file:///not-a-path"),
51+
L2ContractsLocator: artifacts.MustNewLocatorFromURL("file:///not-a-path"),
52+
},
53+
st,
54+
))
55+
56+
// Defining a file locator will always deploy a new superchain and OPCM
57+
require.Nil(t, st.SuperchainDeployment)
58+
require.Nil(t, st.ImplementationsDeployment)
59+
})
60+
61+
t.Run("tagged L1 locator with standard intent types and standard roles", func(t *testing.T) {
62+
runTest := func(configType state.IntentType) {
63+
stdSuperchainRoles, err := state.GetStandardSuperchainRoles(l1ChainID)
64+
require.NoError(t, err)
65+
66+
intent := &state.Intent{
67+
ConfigType: configType,
68+
L1ChainID: l1ChainID,
69+
L1ContractsLocator: artifacts.DefaultL1ContractsLocator,
70+
L2ContractsLocator: artifacts.DefaultL2ContractsLocator,
71+
SuperchainRoles: stdSuperchainRoles,
72+
}
73+
st := &state.State{
74+
Version: 1,
75+
}
76+
require.NoError(t, InitLiveStrategy(
77+
ctx,
78+
&Env{
79+
L1Client: client,
80+
Logger: lgr,
81+
},
82+
intent,
83+
st,
84+
))
85+
86+
// Defining a file locator will always deploy a new superchain and OPCM
87+
superCfg, err := standard.SuperchainFor(l1ChainID)
88+
require.NoError(t, err)
89+
proxyAdmin, err := standard.SuperchainProxyAdminAddrFor(l1ChainID)
90+
require.NoError(t, err)
91+
opcmAddr, err := standard.ManagerImplementationAddrFor(l1ChainID, intent.L1ContractsLocator.Tag)
92+
require.NoError(t, err)
93+
94+
expDeployment := &state.SuperchainDeployment{
95+
ProxyAdminAddress: proxyAdmin,
96+
ProtocolVersionsProxyAddress: superCfg.ProtocolVersionsAddr,
97+
SuperchainConfigProxyAddress: superCfg.SuperchainConfigAddr,
98+
}
99+
100+
// Tagged locator will reuse the existing superchain and OPCM
101+
require.NotNil(t, st.SuperchainDeployment)
102+
require.NotNil(t, st.ImplementationsDeployment)
103+
require.Equal(t, *expDeployment, *st.SuperchainDeployment)
104+
require.Equal(t, opcmAddr, st.ImplementationsDeployment.OpcmAddress)
105+
}
106+
107+
runTest(state.IntentTypeStandard)
108+
runTest(state.IntentTypeStandardOverrides)
109+
})
110+
111+
t.Run("tagged L1 locator with standard intent types and modified roles", func(t *testing.T) {
112+
runTest := func(configType state.IntentType) {
113+
intent := &state.Intent{
114+
ConfigType: configType,
115+
L1ChainID: l1ChainID,
116+
L1ContractsLocator: artifacts.DefaultL1ContractsLocator,
117+
L2ContractsLocator: artifacts.DefaultL2ContractsLocator,
118+
SuperchainRoles: &state.SuperchainRoles{
119+
Guardian: common.Address{0: 99},
120+
},
121+
}
122+
st := &state.State{
123+
Version: 1,
124+
}
125+
require.NoError(t, InitLiveStrategy(
126+
ctx,
127+
&Env{
128+
L1Client: client,
129+
Logger: lgr,
130+
},
131+
intent,
132+
st,
133+
))
134+
135+
// Modified roles will cause a new superchain and OPCM to be deployed
136+
require.Nil(t, st.SuperchainDeployment)
137+
require.Nil(t, st.ImplementationsDeployment)
138+
}
139+
140+
runTest(state.IntentTypeStandard)
141+
runTest(state.IntentTypeStandardOverrides)
142+
})
143+
144+
t.Run("tagged locator with custom intent type", func(t *testing.T) {
145+
intent := &state.Intent{
146+
ConfigType: state.IntentTypeCustom,
147+
L1ChainID: l1ChainID,
148+
L1ContractsLocator: artifacts.DefaultL1ContractsLocator,
149+
L2ContractsLocator: artifacts.DefaultL2ContractsLocator,
150+
SuperchainRoles: &state.SuperchainRoles{
151+
Guardian: common.Address{0: 99},
152+
},
153+
}
154+
st := &state.State{
155+
Version: 1,
156+
}
157+
require.NoError(t, InitLiveStrategy(
158+
ctx,
159+
&Env{
160+
L1Client: client,
161+
Logger: lgr,
162+
},
163+
intent,
164+
st,
165+
))
166+
167+
// Custom intent types always deploy a new superchain and OPCM
168+
require.Nil(t, st.SuperchainDeployment)
169+
require.Nil(t, st.ImplementationsDeployment)
170+
})
171+
}

‎op-deployer/pkg/deployer/standard/standard.go

+11
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,17 @@ func L1ProxyAdminOwner(chainID uint64) (common.Address, error) {
166166
}
167167
}
168168

169+
func L2ProxyAdminOwner(chainID uint64) (common.Address, error) {
170+
switch chainID {
171+
case 1:
172+
return common.Address(validation.StandardConfigRolesMainnet.L2ProxyAdminOwner), nil
173+
case 11155111:
174+
return common.Address(validation.StandardConfigRolesSepolia.L2ProxyAdminOwner), nil
175+
default:
176+
return common.Address{}, fmt.Errorf("unsupported chain ID: %d", chainID)
177+
}
178+
}
179+
169180
func ProtocolVersionsOwner(chainID uint64) (common.Address, error) {
170181
switch chainID {
171182
case 1:

‎op-deployer/pkg/deployer/standard/standard_test.go

+23
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"strings"
77
"testing"
88

9+
"github.com/ethereum-optimism/superchain-registry/validation"
10+
911
"github.com/ethereum/go-ethereum/common"
1012

1113
"github.com/stretchr/testify/require"
@@ -62,3 +64,24 @@ func TestStandardAddresses(t *testing.T) {
6264
})
6365
}
6466
}
67+
68+
func TestL2ProxyAdminOwner(t *testing.T) {
69+
tests := []struct {
70+
chainID uint64
71+
expAddr validation.Address
72+
}{
73+
{
74+
1,
75+
validation.StandardConfigRolesMainnet.L2ProxyAdminOwner,
76+
},
77+
{
78+
11155111,
79+
validation.StandardConfigRolesSepolia.L2ProxyAdminOwner,
80+
},
81+
}
82+
for _, test := range tests {
83+
addr, err := L2ProxyAdminOwner(test.chainID)
84+
require.NoError(t, err)
85+
require.Equal(t, common.Address(test.expAddr), addr)
86+
}
87+
}

‎op-deployer/pkg/deployer/state/intent.go

+16-5
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func (c *Intent) validateStandardValues() error {
121121
return err
122122
}
123123

124-
standardSuperchainRoles, err := getStandardSuperchainRoles(c.L1ChainID)
124+
standardSuperchainRoles, err := GetStandardSuperchainRoles(c.L1ChainID)
125125
if err != nil {
126126
return fmt.Errorf("error getting standard superchain roles: %w", err)
127127
}
@@ -157,7 +157,7 @@ func (c *Intent) validateStandardValues() error {
157157
return nil
158158
}
159159

160-
func getStandardSuperchainRoles(l1ChainId uint64) (*SuperchainRoles, error) {
160+
func GetStandardSuperchainRoles(l1ChainId uint64) (*SuperchainRoles, error) {
161161
proxyAdminOwner, err := standard.L1ProxyAdminOwner(l1ChainId)
162162
if err != nil {
163163
return nil, fmt.Errorf("error getting L1ProxyAdminOwner: %w", err)
@@ -286,14 +286,24 @@ func NewIntentStandard(l1ChainId uint64, l2ChainIds []common.Hash) (Intent, erro
286286
L2ContractsLocator: artifacts.DefaultL2ContractsLocator,
287287
}
288288

289-
superchainRoles, err := getStandardSuperchainRoles(l1ChainId)
289+
superchainRoles, err := GetStandardSuperchainRoles(l1ChainId)
290290
if err != nil {
291291
return Intent{}, fmt.Errorf("error getting standard superchain roles: %w", err)
292292
}
293293
intent.SuperchainRoles = superchainRoles
294294

295-
challenger, _ := standard.ChallengerAddressFor(l1ChainId)
296-
l1ProxyAdminOwner, _ := standard.L1ProxyAdminOwner(l1ChainId)
295+
challenger, err := standard.ChallengerAddressFor(l1ChainId)
296+
if err != nil {
297+
return Intent{}, fmt.Errorf("error getting challenger address: %w", err)
298+
}
299+
l1ProxyAdminOwner, err := standard.L1ProxyAdminOwner(l1ChainId)
300+
if err != nil {
301+
return Intent{}, fmt.Errorf("error getting L1ProxyAdminOwner: %w", err)
302+
}
303+
l2ProxyAdminOwner, err := standard.L2ProxyAdminOwner(l1ChainId)
304+
if err != nil {
305+
return Intent{}, fmt.Errorf("error getting L2ProxyAdminOwner: %w", err)
306+
}
297307

298308
for _, l2ChainID := range l2ChainIds {
299309
intent.Chains = append(intent.Chains, &ChainIntent{
@@ -304,6 +314,7 @@ func NewIntentStandard(l1ChainId uint64, l2ChainIds []common.Hash) (Intent, erro
304314
Roles: ChainRoles{
305315
Challenger: challenger,
306316
L1ProxyAdminOwner: l1ProxyAdminOwner,
317+
L2ProxyAdminOwner: l2ProxyAdminOwner,
307318
},
308319
})
309320
}

0 commit comments

Comments
 (0)