Merge pull request #4293 from ethereum-optimism/sc/ctb-require-fso-controller

feat(ctb): make owner and controller required

packages/contracts-bedrock/deploy-config/devnetL1.json

@@ -21,6 +21,7 @@
   "sequencerFeeVaultRecipient": "0xfabb0ac9d68b0b445fb7357272ff202c5651694a",
   "proxyAdminOwner": "0xBcd4042DE499D14e55001CcbB24a551F3b954096",
   "finalSystemOwner": "0xBcd4042DE499D14e55001CcbB24a551F3b954096",
+  "controller": "0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266",
   "finalizationPeriodSeconds": 2,
   "deploymentWaitConfirmations": 1,
   "fundDevAccounts": true,

packages/contracts-bedrock/deploy-config/goerli-forked.json

 {
   "finalSystemOwner": "0x62790eFcB3a5f3A5D398F95B47930A9Addd83807",
+  "controller": "0x62790eFcB3a5f3A5D398F95B47930A9Addd83807",
 
   "l1StartingBlockTag": "0xcbcbf93fa6ef953a491466311fce85846252a808aa3cc6a8fd432afc5ea4487b",
   "l1ChainID": 5,

packages/contracts-bedrock/deploy-config/hardhat.json

 {
   "finalSystemOwner": "0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc",
+  "controller": "0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266",
 
   "l1StartingBlockTag": "earliest",
   "l1ChainID": 900,

packages/contracts-bedrock/deploy/017-SystemConfigImpl.ts

-import { ethers } from 'ethers'
 import { DeployFunction } from 'hardhat-deploy/dist/types'
 import '@eth-optimism/hardhat-deploy-config'
 
 import { assertContractVariable, deploy } from '../src/deploy-utils'
 
 const deployFn: DeployFunction = async (hre) => {
-  const { deployer } = await hre.getNamedAccounts()
-
-  let finalOwner = hre.deployConfig.finalSystemOwner
-  if (finalOwner === ethers.constants.AddressZero) {
-    if (hre.network.config.live === false) {
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!! A proxy admin owner address was not provided.`)
-      console.log(
-        `WARNING!!! Make sure you are ONLY doing this on a test network.`
-      )
-      finalOwner = deployer
-    } else {
-      throw new Error(`must specify the finalSystemOwner on live networks`)
-    }
-  }
-
   const batcherHash = hre.ethers.utils.hexZeroPad(
     hre.deployConfig.batchSenderAddress,
     32
@@ -32,14 +13,18 @@ const deployFn: DeployFunction = async (hre) => {
     hre,
     name: 'SystemConfig',
     args: [
-      finalOwner,
+      hre.deployConfig.finalSystemOwner,
       hre.deployConfig.gasPriceOracleOverhead,
       hre.deployConfig.gasPriceOracleScalar,
       batcherHash,
       hre.deployConfig.l2GenesisBlockGasLimit,
     ],
     postDeployAction: async (contract) => {
-      await assertContractVariable(contract, 'owner', finalOwner)
+      await assertContractVariable(
+        contract,
+        'owner',
+        hre.deployConfig.finalSystemOwner
+      )
       await assertContractVariable(
         contract,
         'overhead',

packages/contracts-bedrock/deploy/019-SystemDictatorInit.ts

@@ -14,40 +14,6 @@ import {
 const deployFn: DeployFunction = async (hre) => {
   const { deployer } = await hre.getNamedAccounts()
 
-  let controller = hre.deployConfig.controller
-  if (controller === ethers.constants.AddressZero) {
-    if (hre.network.config.live === false) {
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!! A controller address was not provided.`)
-      console.log(
-        `WARNING!!! Make sure you are ONLY doing this on a test network.`
-      )
-      controller = deployer
-    } else {
-      throw new Error(
-        `controller address MUST NOT be the deployer on live networks`
-      )
-    }
-  }
-
-  let finalOwner = hre.deployConfig.finalSystemOwner
-  if (finalOwner === ethers.constants.AddressZero) {
-    if (hre.network.config.live === false) {
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!! A proxy admin owner address was not provided.`)
-      console.log(
-        `WARNING!!! Make sure you are ONLY doing this on a test network.`
-      )
-      finalOwner = deployer
-    } else {
-      throw new Error(`must specify the finalSystemOwner on live networks`)
-    }
-  }
-
   // Load the contracts we need to interact with.
   const [
     SystemDictator,
@@ -77,8 +43,8 @@ const deployFn: DeployFunction = async (hre) => {
   const config = {
     globalConfig: {
       proxyAdmin: await getDeploymentAddress(hre, 'ProxyAdmin'),
-      controller,
-      finalOwner,
+      controller: hre.deployConfig.controller,
+      finalOwner: hre.deployConfig.finalSystemOwner,
       addressManager: await getDeploymentAddress(hre, 'Lib_AddressManager'),
     },
     proxyAddressConfig: {
@@ -125,7 +91,7 @@ const deployFn: DeployFunction = async (hre) => {
       systemConfigImpl: await getDeploymentAddress(hre, 'SystemConfig'),
     },
     systemConfigConfig: {
-      owner: finalOwner,
+      owner: hre.deployConfig.finalSystemOwner,
       overhead: hre.deployConfig.gasPriceOracleOverhead,
       scalar: hre.deployConfig.gasPriceOracleDecimals,
       batcherHash: hre.ethers.utils.hexZeroPad(
@@ -197,12 +163,14 @@ const deployFn: DeployFunction = async (hre) => {
   if (
     (await SystemDictatorProxy.callStatic.admin({
       from: ethers.constants.AddressZero,
-    })) !== controller
+    })) !== hre.deployConfig.controller
   ) {
     console.log('Transferring ownership of the SystemDictator proxy...')
 
     // Transfer ownership to the controller address.
-    await SystemDictatorProxyWithSigner.transferOwnership(controller)
+    await SystemDictatorProxyWithSigner.transferOwnership(
+      hre.deployConfig.controller
+    )
 
     // Wait for the transaction to execute properly.
     await awaitCondition(
@@ -210,7 +178,7 @@ const deployFn: DeployFunction = async (hre) => {
         return (
           (await SystemDictatorProxy.callStatic.admin({
             from: ethers.constants.AddressZero,
-          })) === controller
+          })) === hre.deployConfig.controller
         )
       },
       30000,

packages/contracts-bedrock/deploy/020-SystemDictatorSteps.ts

@@ -16,43 +16,6 @@ import {
 const deployFn: DeployFunction = async (hre) => {
   const { deployer } = await hre.getNamedAccounts()
 
-  let isLiveDeployer = false
-  let controller = hre.deployConfig.controller
-  if (controller === ethers.constants.AddressZero) {
-    if (hre.network.config.live === false) {
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!! A controller address was not provided.`)
-      console.log(
-        `WARNING!!! Make sure you are ONLY doing this on a test network.`
-      )
-      console.log('using a live deployer')
-      isLiveDeployer = true
-      controller = deployer
-    } else {
-      throw new Error(
-        `controller address MUST NOT be the deployer on live networks`
-      )
-    }
-  }
-
-  let finalOwner = hre.deployConfig.finalSystemOwner
-  if (finalOwner === ethers.constants.AddressZero) {
-    if (hre.network.config.live === false) {
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!!`)
-      console.log(`WARNING!!! A proxy admin owner address was not provided.`)
-      console.log(
-        `WARNING!!! Make sure you are ONLY doing this on a test network.`
-      )
-      finalOwner = deployer
-    } else {
-      throw new Error(`must specify the finalSystemOwner on live networks`)
-    }
-  }
-
   // Set up required contract references.
   const [
     SystemDictator,
@@ -128,6 +91,10 @@ const deployFn: DeployFunction = async (hre) => {
     },
   ])
 
+  // If we have the key for the controller then we don't need to wait for external txns.
+  const isLiveDeployer =
+    deployer.toLowerCase() === hre.deployConfig.controller.toLowerCase()
+
   // Transfer ownership of the ProxyAdmin to the SystemDictator.
   if ((await ProxyAdmin.owner()) !== SystemDictator.address) {
     console.log(`Setting ProxyAdmin owner to MSD`)
@@ -570,8 +537,16 @@ const deployFn: DeployFunction = async (hre) => {
       1000
     )
 
-    await assertContractVariable(L1CrossDomainMessenger, 'owner', finalOwner)
-    await assertContractVariable(ProxyAdmin, 'owner', finalOwner)
+    await assertContractVariable(
+      L1CrossDomainMessenger,
+      'owner',
+      hre.deployConfig.finalSystemOwner
+    )
+    await assertContractVariable(
+      ProxyAdmin,
+      'owner',
+      hre.deployConfig.finalSystemOwner
+    )
   }
 }
 

packages/contracts-bedrock/src/deploy-config.ts

@@ -165,11 +165,9 @@ export const deployConfigSpec: {
   },
   finalSystemOwner: {
     type: 'address',
-    default: ethers.constants.AddressZero,
   },
   controller: {
     type: 'address',
-    default: ethers.constants.AddressZero,
   },
   l1StartingBlockTag: {
     type: 'string',