From 29fa1620ee6354ba03b2ec2a738b5b40bbc63d07 Mon Sep 17 00:00:00 2001 From: Blaine Malone Date: Fri, 11 Oct 2024 17:36:33 -0400 Subject: [PATCH 1/6] op-deployer: Most implementation addresses not set in state.json when standard release tag used --- op-deployer/pkg/deployer/opcm/contract.go | 24 ++++++++++--- op-deployer/pkg/deployer/pipeline/opchain.go | 38 +++++++++++++++++++- 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/op-deployer/pkg/deployer/opcm/contract.go b/op-deployer/pkg/deployer/opcm/contract.go index c81222aafe88..fb921a767cb9 100644 --- a/op-deployer/pkg/deployer/opcm/contract.go +++ b/op-deployer/pkg/deployer/opcm/contract.go @@ -29,14 +29,30 @@ func (c *Contract) ProtocolVersions(ctx context.Context) (common.Address, error) } func (c *Contract) getAddress(ctx context.Context, name string) (common.Address, error) { + return c.callContractMethod(ctx, name, abi.Arguments{}) +} + +// Used to call getAddress(string) on legacy ResolvedDelegateProxy contract +func (c *Contract) GetAddressByName(ctx context.Context, name string) (common.Address, error) { + inputs := abi.Arguments{ + abi.Argument{ + Name: "_name", + Type: mustType("string"), + Indexed: false, + }, + } + return c.callContractMethod(ctx, "getAddress", inputs, name) +} + +func (c *Contract) callContractMethod(ctx context.Context, methodName string, inputs abi.Arguments, args ...interface{}) (common.Address, error) { method := abi.NewMethod( - name, - name, + methodName, + methodName, abi.Function, "view", true, false, - abi.Arguments{}, + inputs, abi.Arguments{ abi.Argument{ Name: "address", @@ -46,7 +62,7 @@ func (c *Contract) getAddress(ctx context.Context, name string) (common.Address, }, ) - calldata, err := method.Inputs.Pack() + calldata, err := method.Inputs.Pack(args...) if err != nil { return common.Address{}, fmt.Errorf("failed to pack inputs: %w", err) } diff --git a/op-deployer/pkg/deployer/pipeline/opchain.go b/op-deployer/pkg/deployer/pipeline/opchain.go index 353f1bb92358..b3417918fc94 100644 --- a/op-deployer/pkg/deployer/pipeline/opchain.go +++ b/op-deployer/pkg/deployer/pipeline/opchain.go @@ -2,14 +2,18 @@ package pipeline import ( "context" + "encoding/hex" "fmt" "math/big" + "github.com/ethereum-optimism/optimism/op-chain-ops/genesis" "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/broadcaster" "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/opcm" + "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/state" state2 "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/state" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/ethclient" ) func DeployOPChain(ctx context.Context, env *Env, bundle ArtifactsBundle, intent *state2.Intent, st *state2.State, chainID common.Hash) error { @@ -91,10 +95,42 @@ func DeployOPChain(ctx context.Context, env *Env, bundle ArtifactsBundle, intent DelayedWETHPermissionlessGameProxyAddress: dco.DelayedWETHPermissionlessGameProxy, }) + currentBlock, _ := env.L1Client.BlockNumber(ctx) + block, _ := env.L1Client.BlockByNumber(ctx, big.NewInt(int64(currentBlock))) + currentBlockHash := block.Hash() + + // If any of the implementations addresses (excluding OpcmProxy) are empty, + // we need to set them using the addresses of the corresponding proxies. + // The reason these might be empty is because we're only invoking DeployOPChain.s.sol as part of the pipeline. + // TODO: Need to initialize 'mipsSingletonAddress' and 'preimageOracleSingletonAddress' + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.DelayedWETHPermissionedGameProxy, currentBlockHash, &st.ImplementationsDeployment.DelayedWETHImplAddress) + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.OptimismPortalProxy, currentBlockHash, &st.ImplementationsDeployment.OptimismPortalImplAddress) + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.SystemConfigProxy, currentBlockHash, &st.ImplementationsDeployment.SystemConfigImplAddress) + setRDPImplementationAddress(ctx, env.L1Client, dco.AddressManager, &st.ImplementationsDeployment.L1CrossDomainMessengerImplAddress) + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.L1ERC721BridgeProxy, currentBlockHash, &st.ImplementationsDeployment.L1ERC721BridgeImplAddress) + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.L1StandardBridgeProxy, currentBlockHash, &st.ImplementationsDeployment.L1StandardBridgeImplAddress) + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.OptimismMintableERC20FactoryProxy, currentBlockHash, &st.ImplementationsDeployment.OptimismMintableERC20FactoryImplAddress) + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.DisputeGameFactoryProxy, currentBlockHash, &st.ImplementationsDeployment.DisputeGameFactoryImplAddress) + return nil } -func shouldDeployOPChain(intent *state2.Intent, st *state2.State, chainID common.Hash) bool { +func setRDPImplementationAddress(ctx context.Context, client *ethclient.Client, addressManager common.Address, implAddress *common.Address) { + if *implAddress == (common.Address{}) { + contract := opcm.NewContract(addressManager, client) + address, _ := contract.GetAddressByName(ctx, "OVM_L1CrossDomainMessenger") + *implAddress = address + } +} + +func setEIP1967ImplementationAddress(ctx context.Context, client *ethclient.Client, proxy common.Address, currentBlockHash common.Hash, implAddress *common.Address) { + if *implAddress == (common.Address{}) { + storageValue, _ := client.StorageAtHash(ctx, proxy, genesis.ImplementationSlot, currentBlockHash) + *implAddress = common.HexToAddress(hex.EncodeToString(storageValue)) + } +} + +func shouldDeployOPChain(intent *state.Intent, st *state.State, chainID common.Hash) bool { for _, chain := range st.Chains { if chain.ID == chainID { return false From 8cb32cee4630bd57e42e215783e20468e378f77f Mon Sep 17 00:00:00 2001 From: Blaine Malone Date: Wed, 16 Oct 2024 14:55:19 -0400 Subject: [PATCH 2/6] Update op-chain-ops/deployer/pipeline/opchain.go Co-authored-by: Matt Solomon --- op-deployer/pkg/deployer/pipeline/opchain.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-deployer/pkg/deployer/pipeline/opchain.go b/op-deployer/pkg/deployer/pipeline/opchain.go index b3417918fc94..e123656c8afc 100644 --- a/op-deployer/pkg/deployer/pipeline/opchain.go +++ b/op-deployer/pkg/deployer/pipeline/opchain.go @@ -100,7 +100,7 @@ func DeployOPChain(ctx context.Context, env *Env, bundle ArtifactsBundle, intent currentBlockHash := block.Hash() // If any of the implementations addresses (excluding OpcmProxy) are empty, - // we need to set them using the addresses of the corresponding proxies. + // we need to set them using the implementation address read from their corresponding proxy. // The reason these might be empty is because we're only invoking DeployOPChain.s.sol as part of the pipeline. // TODO: Need to initialize 'mipsSingletonAddress' and 'preimageOracleSingletonAddress' setEIP1967ImplementationAddress(ctx, env.L1Client, dco.DelayedWETHPermissionedGameProxy, currentBlockHash, &st.ImplementationsDeployment.DelayedWETHImplAddress) From dfab32d0604cf8fbfe767ac4c9924b7fbdafdd51 Mon Sep 17 00:00:00 2001 From: Blaine Malone Date: Wed, 16 Oct 2024 17:00:22 -0400 Subject: [PATCH 3/6] fix: parallel execution of commands. --- op-deployer/pkg/deployer/pipeline/opchain.go | 51 +++++++++++++++++--- 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/op-deployer/pkg/deployer/pipeline/opchain.go b/op-deployer/pkg/deployer/pipeline/opchain.go index e123656c8afc..2505b99a47b1 100644 --- a/op-deployer/pkg/deployer/pipeline/opchain.go +++ b/op-deployer/pkg/deployer/pipeline/opchain.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "fmt" "math/big" + "sync" "github.com/ethereum-optimism/optimism/op-chain-ops/genesis" "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/broadcaster" @@ -103,18 +104,52 @@ func DeployOPChain(ctx context.Context, env *Env, bundle ArtifactsBundle, intent // we need to set them using the implementation address read from their corresponding proxy. // The reason these might be empty is because we're only invoking DeployOPChain.s.sol as part of the pipeline. // TODO: Need to initialize 'mipsSingletonAddress' and 'preimageOracleSingletonAddress' - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.DelayedWETHPermissionedGameProxy, currentBlockHash, &st.ImplementationsDeployment.DelayedWETHImplAddress) - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.OptimismPortalProxy, currentBlockHash, &st.ImplementationsDeployment.OptimismPortalImplAddress) - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.SystemConfigProxy, currentBlockHash, &st.ImplementationsDeployment.SystemConfigImplAddress) - setRDPImplementationAddress(ctx, env.L1Client, dco.AddressManager, &st.ImplementationsDeployment.L1CrossDomainMessengerImplAddress) - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.L1ERC721BridgeProxy, currentBlockHash, &st.ImplementationsDeployment.L1ERC721BridgeImplAddress) - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.L1StandardBridgeProxy, currentBlockHash, &st.ImplementationsDeployment.L1StandardBridgeImplAddress) - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.OptimismMintableERC20FactoryProxy, currentBlockHash, &st.ImplementationsDeployment.OptimismMintableERC20FactoryImplAddress) - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.DisputeGameFactoryProxy, currentBlockHash, &st.ImplementationsDeployment.DisputeGameFactoryImplAddress) + setImplementationAddressTasks := []func(){ + func() { + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.DelayedWETHPermissionedGameProxy, currentBlockHash, &st.ImplementationsDeployment.DelayedWETHImplAddress) + }, + func() { + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.OptimismPortalProxy, currentBlockHash, &st.ImplementationsDeployment.OptimismPortalImplAddress) + }, + func() { + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.SystemConfigProxy, currentBlockHash, &st.ImplementationsDeployment.SystemConfigImplAddress) + }, + func() { + setRDPImplementationAddress(ctx, env.L1Client, dco.AddressManager, &st.ImplementationsDeployment.L1CrossDomainMessengerImplAddress) + }, + func() { + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.L1ERC721BridgeProxy, currentBlockHash, &st.ImplementationsDeployment.L1ERC721BridgeImplAddress) + }, + func() { + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.L1StandardBridgeProxy, currentBlockHash, &st.ImplementationsDeployment.L1StandardBridgeImplAddress) + }, + func() { + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.OptimismMintableERC20FactoryProxy, currentBlockHash, &st.ImplementationsDeployment.OptimismMintableERC20FactoryImplAddress) + }, + func() { + setEIP1967ImplementationAddress(ctx, env.L1Client, dco.DisputeGameFactoryProxy, currentBlockHash, &st.ImplementationsDeployment.DisputeGameFactoryImplAddress) + }, + } + + // Execute all set implementation address tasks concurrently + executeTasksConcurrently(setImplementationAddressTasks) return nil } +// Helper function to execute tasks concurrently +func executeTasksConcurrently(tasks []func()) { + var wg sync.WaitGroup + for _, task := range tasks { + wg.Add(1) + go func(t func()) { + defer wg.Done() + t() + }(task) + } + wg.Wait() +} + func setRDPImplementationAddress(ctx context.Context, client *ethclient.Client, addressManager common.Address, implAddress *common.Address) { if *implAddress == (common.Address{}) { contract := opcm.NewContract(addressManager, client) From 53f058aafdb3c5c97c7f131ba8553e27dac5765c Mon Sep 17 00:00:00 2001 From: Blaine Malone Date: Wed, 16 Oct 2024 17:24:39 -0400 Subject: [PATCH 4/6] fix: tests added --- .vscode/settings.json | 3 ++- .../pkg/deployer/integration_test/apply_test.go | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index d8a05abe18af..3addcdec8e81 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,4 +1,5 @@ { "editorconfig.generateAuto": false, - "files.trimTrailingWhitespace": true + "files.trimTrailingWhitespace": true, + "makefile.configureOnOpen": false } diff --git a/op-deployer/pkg/deployer/integration_test/apply_test.go b/op-deployer/pkg/deployer/integration_test/apply_test.go index e7acd0a24a14..a91890ddc2c7 100644 --- a/op-deployer/pkg/deployer/integration_test/apply_test.go +++ b/op-deployer/pkg/deployer/integration_test/apply_test.go @@ -275,6 +275,16 @@ func validateOPChainDeployment(t *testing.T, ctx context.Context, l1Client *ethc }) } + require.NotEmpty(t, st.ImplementationsDeployment.DelayedWETHImplAddress, "DelayedWETHImplAddress should be set") + require.NotEmpty(t, st.ImplementationsDeployment.OptimismPortalImplAddress, "OptimismPortalImplAddress should be set") + require.NotEmpty(t, st.ImplementationsDeployment.SystemConfigImplAddress, "SystemConfigImplAddress should be set") + require.NotEmpty(t, st.ImplementationsDeployment.L1CrossDomainMessengerImplAddress, "L1CrossDomainMessengerImplAddress should be set") + require.NotEmpty(t, st.ImplementationsDeployment.L1ERC721BridgeImplAddress, "L1ERC721BridgeImplAddress should be set") + require.NotEmpty(t, st.ImplementationsDeployment.L1StandardBridgeImplAddress, "L1StandardBridgeImplAddress should be set") + require.NotEmpty(t, st.ImplementationsDeployment.OptimismMintableERC20FactoryImplAddress, "OptimismMintableERC20FactoryImplAddress should be set") + require.NotEmpty(t, st.ImplementationsDeployment.DisputeGameFactoryImplAddress, "DisputeGameFactoryImplAddress should be set") + // TODO: Need to check that 'mipsSingletonAddress' and 'preimageOracleSingletonAddress' are set + t.Run("l2 genesis", func(t *testing.T) { require.Greater(t, len(chainState.Allocs), 0) l2Allocs, _ := chainState.UnmarshalAllocs() From 79c4c70ce31e3066c4349f355e1dc4d9eaf586a2 Mon Sep 17 00:00:00 2001 From: Blaine Malone Date: Wed, 16 Oct 2024 17:25:17 -0400 Subject: [PATCH 5/6] fix: vscode revert --- .vscode/settings.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 3addcdec8e81..d8a05abe18af 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,5 +1,4 @@ { "editorconfig.generateAuto": false, - "files.trimTrailingWhitespace": true, - "makefile.configureOnOpen": false + "files.trimTrailingWhitespace": true } From 90e9b585d7f9a3f36c954b8ea7f2fb6c0f6d2ec3 Mon Sep 17 00:00:00 2001 From: Matthew Slipper Date: Wed, 16 Oct 2024 15:59:18 -0600 Subject: [PATCH 6/6] use channel approach --- op-deployer/pkg/deployer/pipeline/opchain.go | 70 +++++++++++--------- 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/op-deployer/pkg/deployer/pipeline/opchain.go b/op-deployer/pkg/deployer/pipeline/opchain.go index 2505b99a47b1..4279876154e4 100644 --- a/op-deployer/pkg/deployer/pipeline/opchain.go +++ b/op-deployer/pkg/deployer/pipeline/opchain.go @@ -5,14 +5,12 @@ import ( "encoding/hex" "fmt" "math/big" - "sync" "github.com/ethereum-optimism/optimism/op-chain-ops/genesis" "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/broadcaster" "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/opcm" "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/state" state2 "github.com/ethereum-optimism/optimism/op-deployer/pkg/deployer/state" - "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/ethclient" ) @@ -100,69 +98,81 @@ func DeployOPChain(ctx context.Context, env *Env, bundle ArtifactsBundle, intent block, _ := env.L1Client.BlockByNumber(ctx, big.NewInt(int64(currentBlock))) currentBlockHash := block.Hash() + errCh := make(chan error, 8) + // If any of the implementations addresses (excluding OpcmProxy) are empty, // we need to set them using the implementation address read from their corresponding proxy. // The reason these might be empty is because we're only invoking DeployOPChain.s.sol as part of the pipeline. // TODO: Need to initialize 'mipsSingletonAddress' and 'preimageOracleSingletonAddress' setImplementationAddressTasks := []func(){ func() { - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.DelayedWETHPermissionedGameProxy, currentBlockHash, &st.ImplementationsDeployment.DelayedWETHImplAddress) + setEIP1967ImplementationAddress(ctx, env.L1Client, errCh, dco.DelayedWETHPermissionedGameProxy, currentBlockHash, &st.ImplementationsDeployment.DelayedWETHImplAddress) }, func() { - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.OptimismPortalProxy, currentBlockHash, &st.ImplementationsDeployment.OptimismPortalImplAddress) + setEIP1967ImplementationAddress(ctx, env.L1Client, errCh, dco.OptimismPortalProxy, currentBlockHash, &st.ImplementationsDeployment.OptimismPortalImplAddress) }, func() { - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.SystemConfigProxy, currentBlockHash, &st.ImplementationsDeployment.SystemConfigImplAddress) + setEIP1967ImplementationAddress(ctx, env.L1Client, errCh, dco.SystemConfigProxy, currentBlockHash, &st.ImplementationsDeployment.SystemConfigImplAddress) }, func() { - setRDPImplementationAddress(ctx, env.L1Client, dco.AddressManager, &st.ImplementationsDeployment.L1CrossDomainMessengerImplAddress) + setRDPImplementationAddress(ctx, env.L1Client, errCh, dco.AddressManager, &st.ImplementationsDeployment.L1CrossDomainMessengerImplAddress) }, func() { - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.L1ERC721BridgeProxy, currentBlockHash, &st.ImplementationsDeployment.L1ERC721BridgeImplAddress) + setEIP1967ImplementationAddress(ctx, env.L1Client, errCh, dco.L1ERC721BridgeProxy, currentBlockHash, &st.ImplementationsDeployment.L1ERC721BridgeImplAddress) }, func() { - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.L1StandardBridgeProxy, currentBlockHash, &st.ImplementationsDeployment.L1StandardBridgeImplAddress) + setEIP1967ImplementationAddress(ctx, env.L1Client, errCh, dco.L1StandardBridgeProxy, currentBlockHash, &st.ImplementationsDeployment.L1StandardBridgeImplAddress) }, func() { - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.OptimismMintableERC20FactoryProxy, currentBlockHash, &st.ImplementationsDeployment.OptimismMintableERC20FactoryImplAddress) + setEIP1967ImplementationAddress(ctx, env.L1Client, errCh, dco.OptimismMintableERC20FactoryProxy, currentBlockHash, &st.ImplementationsDeployment.OptimismMintableERC20FactoryImplAddress) }, func() { - setEIP1967ImplementationAddress(ctx, env.L1Client, dco.DisputeGameFactoryProxy, currentBlockHash, &st.ImplementationsDeployment.DisputeGameFactoryImplAddress) + setEIP1967ImplementationAddress(ctx, env.L1Client, errCh, dco.DisputeGameFactoryProxy, currentBlockHash, &st.ImplementationsDeployment.DisputeGameFactoryImplAddress) }, } + for _, task := range setImplementationAddressTasks { + go task() + } - // Execute all set implementation address tasks concurrently - executeTasksConcurrently(setImplementationAddressTasks) + var lastTaskErr error + for i := 0; i < len(setImplementationAddressTasks); i++ { + taskErr := <-errCh + if lastTaskErr != nil { + lastTaskErr = taskErr + } + } + if lastTaskErr != nil { + return fmt.Errorf("failed to set implementation addresses: %w", lastTaskErr) + } return nil } -// Helper function to execute tasks concurrently -func executeTasksConcurrently(tasks []func()) { - var wg sync.WaitGroup - for _, task := range tasks { - wg.Add(1) - go func(t func()) { - defer wg.Done() - t() - }(task) +func setRDPImplementationAddress(ctx context.Context, client *ethclient.Client, errCh chan error, addressManager common.Address, implAddress *common.Address) { + if *implAddress != (common.Address{}) { + errCh <- nil + return } - wg.Wait() -} -func setRDPImplementationAddress(ctx context.Context, client *ethclient.Client, addressManager common.Address, implAddress *common.Address) { - if *implAddress == (common.Address{}) { - contract := opcm.NewContract(addressManager, client) - address, _ := contract.GetAddressByName(ctx, "OVM_L1CrossDomainMessenger") + contract := opcm.NewContract(addressManager, client) + address, err := contract.GetAddressByName(ctx, "OVM_L1CrossDomainMessenger") + if err == nil { *implAddress = address } + errCh <- err } -func setEIP1967ImplementationAddress(ctx context.Context, client *ethclient.Client, proxy common.Address, currentBlockHash common.Hash, implAddress *common.Address) { - if *implAddress == (common.Address{}) { - storageValue, _ := client.StorageAtHash(ctx, proxy, genesis.ImplementationSlot, currentBlockHash) +func setEIP1967ImplementationAddress(ctx context.Context, client *ethclient.Client, errCh chan error, proxy common.Address, currentBlockHash common.Hash, implAddress *common.Address) { + if *implAddress != (common.Address{}) { + errCh <- nil + return + } + + storageValue, err := client.StorageAtHash(ctx, proxy, genesis.ImplementationSlot, currentBlockHash) + if err == nil { *implAddress = common.HexToAddress(hex.EncodeToString(storageValue)) } + errCh <- err } func shouldDeployOPChain(intent *state.Intent, st *state.State, chainID common.Hash) bool {