feat: cleanup deployment process

.changeset/dull-shirts-develop.md

+---
+'@eth-optimism/contracts': patch
+---
+
+Cleans up the contract deployment process

packages/contracts/deploy/000-Lib_AddressManager.deploy.ts

@@ -15,18 +15,25 @@ const deployFn: DeployFunction = async (hre) => {
     log: true,
   })
 
+  // L2CrossDomainMessenger is the address of the predeploy on L2. We can refactor off-chain
+  // services such that we can remove the need to set this address, but for now it's easier
+  // to simply keep setting the address.
   await registerAddress({
     hre,
     name: 'L2CrossDomainMessenger',
     address: predeploys.L2CrossDomainMessenger,
   })
 
+  // OVM_Sequencer is the address allowed to submit "Sequencer" blocks to the
+  // CanonicalTransactionChain.
   await registerAddress({
     hre,
     name: 'OVM_Sequencer',
     address: (hre as any).deployConfig.ovmSequencerAddress,
   })
 
+  // OVM_Proposer is the address allowed to submit state roots (transaction results) to the
+  // StateCommitmentChain.
   await registerAddress({
     hre,
     name: 'OVM_Proposer',
@@ -34,6 +41,6 @@ const deployFn: DeployFunction = async (hre) => {
   })
 }
 
-deployFn.tags = ['Lib_AddressManager', 'required']
+deployFn.tags = ['Lib_AddressManager']
 
 export default deployFn

packages/contracts/deploy/006-OVM_BondManager.deploy.ts

@@ -2,31 +2,22 @@
 import { DeployFunction } from 'hardhat-deploy/dist/types'
 
 /* Imports: Internal */
-import { getDeployedContract } from '../src/hardhat-deploy-ethers'
+import {
+  deployAndRegister,
+  getDeployedContract,
+} from '../src/hardhat-deploy-ethers'
 
 const deployFn: DeployFunction = async (hre) => {
-  const { deploy } = hre.deployments
-  const { deployer } = await hre.getNamedAccounts()
-
   const Lib_AddressManager = await getDeployedContract(
     hre,
-    'Lib_AddressManager',
-    {
-      signerOrProvider: deployer,
-    }
+    'Lib_AddressManager'
   )
 
-  const result = await deploy('BondManager', {
-    from: deployer,
+  await deployAndRegister({
+    hre,
+    name: 'BondManager',
     args: [Lib_AddressManager.address],
-    log: true,
   })
-
-  if (!result.newlyDeployed) {
-    return
-  }
-
-  await Lib_AddressManager.setAddress('BondManager', result.address)
 }
 
 deployFn.dependencies = ['Lib_AddressManager']

packages/contracts/deploy/007-OVM_L1CrossDomainMessenger.deploy.ts

 /* Imports: External */
 import { DeployFunction } from 'hardhat-deploy/dist/types'
+import { hexStringEquals } from '@eth-optimism/core-utils'
 
 /* Imports: Internal */
-import { getDeployedContract } from '../src/hardhat-deploy-ethers'
+import {
+  getDeployedContract,
+  deployAndRegister,
+  waitUntilTrue,
+} from '../src/hardhat-deploy-ethers'
 
 const deployFn: DeployFunction = async (hre) => {
-  const { deploy } = hre.deployments
-  const { deployer } = await hre.getNamedAccounts()
-
   const Lib_AddressManager = await getDeployedContract(
     hre,
-    'Lib_AddressManager',
-    {
-      signerOrProvider: deployer,
-    }
+    'Lib_AddressManager'
   )
 
-  const result = await deploy('L1CrossDomainMessenger', {
-    from: deployer,
+  await deployAndRegister({
+    hre,
+    name: 'L1CrossDomainMessenger',
     args: [],
-    log: true,
+    postDeployAction: async (contract) => {
+      // Theoretically it's not necessary to initialize this contract since it sits behind
+      // a proxy. However, it's best practice to initialize it anyway just in case there's
+      // some unknown security hole. It also prevents another user from appearing like an
+      // official address because it managed to call the initialization function.
+      console.log(`Initializing L1CrossDomainMessenger...`)
+      await contract.initialize(Lib_AddressManager.address)
+
+      console.log(`Checking that contract was correctly initialized...`)
+      await waitUntilTrue(async () => {
+        return hexStringEquals(
+          await contract.libAddressManager(),
+          Lib_AddressManager.address
+        )
+      })
+    },
   })
-
-  if (!result.newlyDeployed) {
-    return
-  }
-
-  const L1CrossDomainMessenger = await getDeployedContract(
-    hre,
-    'L1CrossDomainMessenger',
-    {
-      signerOrProvider: deployer,
-    }
-  )
-
-  // NOTE: this initialization is *not* technically required (we only need to initialize the proxy)
-  // but it feels safer to initialize this anyway. Otherwise someone else could come along and
-  // initialize this.
-  await L1CrossDomainMessenger.initialize(Lib_AddressManager.address)
-
-  const libAddressManager = await L1CrossDomainMessenger.libAddressManager()
-  if (libAddressManager !== Lib_AddressManager.address) {
-    throw new Error(
-      `\n**FATAL ERROR. THIS SHOULD NEVER HAPPEN. CHECK YOUR DEPLOYMENT.**:\n` +
-        `L1CrossDomainMessenger could not be succesfully initialized.\n` +
-        `Attempted to set Lib_AddressManager to: ${Lib_AddressManager.address}\n` +
-        `Actual address after initialization: ${libAddressManager}\n` +
-        `This could indicate a compromised deployment.`
-    )
-  }
-
-  await Lib_AddressManager.setAddress('L1CrossDomainMessenger', result.address)
 }
 
 deployFn.dependencies = ['Lib_AddressManager']

packages/contracts/deploy/008-Proxy__OVM_L1CrossDomainMessenger.deploy.ts

 /* Imports: External */
 import { DeployFunction } from 'hardhat-deploy/dist/types'
+import { hexStringEquals } from '@eth-optimism/core-utils'
 
 /* Imports: Internal */
-import { getDeployedContract } from '../src/hardhat-deploy-ethers'
+import {
+  getDeployedContract,
+  deployAndRegister,
+  waitUntilTrue,
+} from '../src/hardhat-deploy-ethers'
 
 const deployFn: DeployFunction = async (hre) => {
-  const { deploy } = hre.deployments
-  const { deployer } = await hre.getNamedAccounts()
-
   const Lib_AddressManager = await getDeployedContract(
     hre,
-    'Lib_AddressManager',
-    {
-      signerOrProvider: deployer,
-    }
+    'Lib_AddressManager'
   )
 
-  const result = await deploy('Proxy__L1CrossDomainMessenger', {
+  await deployAndRegister({
+    hre,
+    name: 'Proxy__L1CrossDomainMessenger',
     contract: 'Lib_ResolvedDelegateProxy',
-    from: deployer,
+    iface: 'L1CrossDomainMessenger',
     args: [Lib_AddressManager.address, 'L1CrossDomainMessenger'],
-    log: true,
+    postDeployAction: async (contract) => {
+      console.log(`Initializing Proxy__L1CrossDomainMessenger...`)
+      await contract.initialize(Lib_AddressManager.address)
+
+      console.log(`Checking that contract was correctly initialized...`)
+      await waitUntilTrue(async () => {
+        return hexStringEquals(
+          await contract.libAddressManager(),
+          Lib_AddressManager.address
+        )
+      })
+    },
   })
-
-  if (!result.newlyDeployed) {
-    return
-  }
-
-  const Proxy__L1CrossDomainMessenger = await getDeployedContract(
-    hre,
-    'Proxy__L1CrossDomainMessenger',
-    {
-      signerOrProvider: deployer,
-      iface: 'L1CrossDomainMessenger',
-    }
-  )
-
-  await Proxy__L1CrossDomainMessenger.initialize(Lib_AddressManager.address)
-
-  const libAddressManager =
-    await Proxy__L1CrossDomainMessenger.libAddressManager()
-  if (libAddressManager !== Lib_AddressManager.address) {
-    throw new Error(
-      `\n**FATAL ERROR. THIS SHOULD NEVER HAPPEN. CHECK YOUR DEPLOYMENT.**:\n` +
-        `Proxy__L1CrossDomainMessenger could not be succesfully initialized.\n` +
-        `Attempted to set Lib_AddressManager to: ${Lib_AddressManager.address}\n` +
-        `Actual address after initialization: ${libAddressManager}\n` +
-        `This could indicate a compromised deployment.`
-    )
-  }
-
-  await Lib_AddressManager.setAddress(
-    'Proxy__L1CrossDomainMessenger',
-    result.address
-  )
 }
 
 deployFn.dependencies = ['Lib_AddressManager', 'L1CrossDomainMessenger']

packages/contracts/deploy/009-Proxy__OVM_L1StandardBridge.deploy.ts

 /* Imports: External */
 import { DeployFunction } from 'hardhat-deploy/dist/types'
+import { ethers } from 'ethers'
+import { hexStringEquals } from '@eth-optimism/core-utils'
 
 /* Imports: Internal */
-import { getDeployedContract } from '../src/hardhat-deploy-ethers'
 import { predeploys } from '../src/predeploys'
-import { NON_ZERO_ADDRESS } from '../test/helpers/constants'
-import { getContractFactory } from '../src/contract-defs'
-
-import l1StandardBridgeJson from '../artifacts/contracts/L1/messaging/L1StandardBridge.sol/L1StandardBridge.json'
+import {
+  getContractInterface,
+  getContractDefinition,
+} from '../src/contract-defs'
+import {
+  getDeployedContract,
+  deployAndRegister,
+  waitUntilTrue,
+} from '../src/hardhat-deploy-ethers'
 
 const deployFn: DeployFunction = async (hre) => {
-  const { deploy } = hre.deployments
   const { deployer } = await hre.getNamedAccounts()
-
   const Lib_AddressManager = await getDeployedContract(
     hre,
-    'Lib_AddressManager',
-    {
-      signerOrProvider: deployer,
-    }
+    'Lib_AddressManager'
   )
 
-  const result = await deploy('Proxy__L1StandardBridge', {
+  await deployAndRegister({
+    hre,
+    name: 'Proxy__L1StandardBridge',
     contract: 'L1ChugSplashProxy',
-    from: deployer,
+    iface: 'L1StandardBridge',
     args: [deployer],
-    log: true,
-  })
+    postDeployAction: async (contract) => {
+      // Because of the `iface` parameter supplied to the deployment function above, the `contract`
+      // variable that we here will have the interface of the L1StandardBridge contract. However,
+      // we also need to interact with the contract as if it were a L1ChugSplashProxy contract so
+      // we instantiate a new ethers.Contract object with the same address and signer but with the
+      // L1ChugSplashProxy interface.
+      const proxy = new ethers.Contract(
+        contract.address,
+        getContractInterface('L1ChugSplashProxy'),
+        contract.signer
+      )
 
-  if (!result.newlyDeployed) {
-    return
-  }
+      // First we need to set the correct implementation code. We'll set the code and then check
+      // that the code was indeed correctly set.
+      const bridgeArtifact = getContractDefinition('L1StandardBridge')
+      const bridgeCode = bridgeArtifact.deployedBytecode
 
-  // Create a contract object at the Proxy address with the proxy interface.
-  const Proxy__WithChugSplashInterface = await getDeployedContract(
-    hre,
-    'Proxy__L1StandardBridge',
-    {
-      signerOrProvider: deployer,
-      iface: 'L1ChugSplashProxy',
-    }
-  )
+      console.log(`Setting bridge code...`)
+      await proxy.setCode(bridgeCode)
 
-  // Create a contract object at the Proxy address with the brige implementation interface.
-  const Proxy__WithBridgeInterface = await getDeployedContract(
-    hre,
-    'Proxy__L1StandardBridge',
-    {
-      signerOrProvider: deployer,
-      iface: 'L1StandardBridge',
-    }
-  )
+      console.log(`Confirming that bridge code is correct...`)
+      await waitUntilTrue(async () => {
+        const implementation = await proxy.callStatic.getImplementation()
+        return (
+          !hexStringEquals(implementation, ethers.constants.AddressZero) &&
+          hexStringEquals(
+            await contract.provider.getCode(implementation),
+            bridgeCode
+          )
+        )
+      })
 
-  // Set the implementation code
-  const bridgeCode = l1StandardBridgeJson.deployedBytecode
-  await Proxy__WithChugSplashInterface.setCode(bridgeCode)
+      // Next we need to set the `messenger` address by executing a setStorage operation. We'll
+      // check that this operation was correctly executed by calling `messenger()` and checking
+      // that the result matches the value we initialized.
+      const l1CrossDomainMessengerAddress = await Lib_AddressManager.getAddress(
+        'Proxy__L1CrossDomainMessenger'
+      )
 
-  // Set slot 0 to the L1 Messenger Address
-  const l1MessengerAddress = await Lib_AddressManager.getAddress(
-    'Proxy__L1CrossDomainMessenger'
-  )
-  await Proxy__WithChugSplashInterface.setStorage(
-    hre.ethers.constants.HashZero,
-    hre.ethers.utils.hexZeroPad(l1MessengerAddress, 32)
-  )
-  // Verify that the slot was set correctly
-  const l1MessengerStored =
-    await Proxy__WithBridgeInterface.callStatic.messenger()
-  console.log('l1MessengerStored:', l1MessengerStored)
-  if (l1MessengerStored !== l1MessengerAddress) {
-    throw new Error(
-      'L1 messenger address was not correctly set, check the key value used in setStorage'
-    )
-  }
+      console.log(`Setting messenger address...`)
+      await proxy.setStorage(
+        ethers.utils.hexZeroPad('0x00', 32),
+        ethers.utils.hexZeroPad(l1CrossDomainMessengerAddress, 32)
+      )
 
-  // Set Slot 1 to the L2 Standard Bridge Address
-  await Proxy__WithChugSplashInterface.setStorage(
-    hre.ethers.utils.hexZeroPad('0x01', 32),
-    hre.ethers.utils.hexZeroPad(predeploys.L2StandardBridge, 32)
-  )
-  // Verify that the slot was set correctly
-  const l2TokenBridgeStored =
-    await Proxy__WithBridgeInterface.callStatic.l2TokenBridge()
-  console.log('l2TokenBridgeStored:', l2TokenBridgeStored)
-  if (l2TokenBridgeStored !== predeploys.L2StandardBridge) {
-    throw new Error(
-      'L2 bridge address was not correctly set, check the key value used in setStorage'
-    )
-  }
+      console.log(`Confirming that messenger address was correctly set...`)
+      await waitUntilTrue(async () => {
+        return hexStringEquals(
+          await contract.messenger(),
+          l1CrossDomainMessengerAddress
+        )
+      })
 
-  // transfer ownership to Address Manager owner
-  const addressManagerOwner = Lib_AddressManager.callStatic.owner()
-  await Proxy__WithChugSplashInterface.setOwner(addressManagerOwner)
+      // Now we set the bridge address in the same manner as the messenger address.
+      console.log(`Setting l2 bridge address...`)
+      await proxy.setStorage(
+        ethers.utils.hexZeroPad('0x01', 32),
+        ethers.utils.hexZeroPad(predeploys.L2StandardBridge, 32)
+      )
 
-  // Todo: remove this after adding chugsplash proxy
-  await Lib_AddressManager.setAddress('Proxy__L1StandardBridge', result.address)
+      console.log(`Confirming that l2 bridge address was correctly set...`)
+      await waitUntilTrue(async () => {
+        return hexStringEquals(
+          await contract.l2TokenBridge(),
+          predeploys.L2StandardBridge
+        )
+      })
+
+      // Finally we transfer ownership of the proxy to the ovmAddressManagerOwner address.
+      console.log(`Setting owner address...`)
+      const owner = (hre as any).deployConfig.ovmAddressManagerOwner
+      await proxy.setOwner(owner)
+
+      console.log(`Confirming that owner address was correctly set...`)
+      await waitUntilTrue(async () => {
+        return hexStringEquals(await proxy.callStatic.getOwner(), owner)
+      })
+    },
+  })
 }
 
 deployFn.dependencies = ['Lib_AddressManager', 'L1StandardBridge']

packages/contracts/deploy/010-finalize.ts

 /* Imports: External */
 import { DeployFunction } from 'hardhat-deploy/dist/types'
+import { hexStringEquals } from '@eth-optimism/core-utils'
 
 /* Imports: Internal */
-import { getDeployedContract } from '../src/hardhat-deploy-ethers'
+import {
+  getDeployedContract,
+  waitUntilTrue,
+} from '../src/hardhat-deploy-ethers'
 
 const deployFn: DeployFunction = async (hre) => {
   const { deployer } = await hre.getNamedAccounts()
@@ -16,7 +20,7 @@ const deployFn: DeployFunction = async (hre) => {
 
   const owner = (hre as any).deployConfig.ovmAddressManagerOwner
   const remoteOwner = await Lib_AddressManager.owner()
-  if (remoteOwner === owner) {
+  if (hexStringEquals(owner, remoteOwner)) {
     console.log(
       `✓ Not changing owner of Lib_AddressManager because it's already correctly set`
     )
@@ -24,19 +28,12 @@ const deployFn: DeployFunction = async (hre) => {
   }
 
   console.log(`Transferring ownership of Lib_AddressManager to ${owner}...`)
-  const tx = await Lib_AddressManager.transferOwnership(owner)
-  await tx.wait()
-
-  const newRemoteOwner = await Lib_AddressManager.owner()
-  if (newRemoteOwner !== owner) {
-    throw new Error(
-      `\n**FATAL ERROR. THIS SHOULD NEVER HAPPEN. CHECK YOUR DEPLOYMENT.**:\n` +
-        `Could not transfer ownership of Lib_AddressManager.\n` +
-        `Attempted to set owner of Lib_AddressManager to: ${owner}\n` +
-        `Actual owner after transaction: ${newRemoteOwner}\n` +
-        `This could indicate a compromised deployment.`
-    )
-  }
+  await Lib_AddressManager.transferOwnership(owner)
+
+  console.log(`Confirming transfer was successful...`)
+  await waitUntilTrue(async () => {
+    return hexStringEquals(await Lib_AddressManager.owner(), owner)
+  })
 
   console.log(`✓ Set owner of Lib_AddressManager to: ${owner}`)
 }

packages/contracts/src/hardhat-deploy-ethers.ts

@@ -2,6 +2,27 @@
 import { Contract } from 'ethers'
 import { Provider } from '@ethersproject/abstract-provider'
 import { Signer } from '@ethersproject/abstract-signer'
+import { sleep, hexStringEquals } from '@eth-optimism/core-utils'
+
+export const waitUntilTrue = async (
+  check: () => Promise<boolean>,
+  opts: {
+    retries?: number
+    delay?: number
+  } = {}
+) => {
+  opts.retries = opts.retries || 10
+  opts.delay = opts.delay || 5000
+
+  let retries = 0
+  while (!(await check())) {
+    if (retries > opts.retries) {
+      throw new Error(`check failed after ${opts.retries} attempts`)
+    }
+    retries++
+    await sleep(opts.delay)
+  }
+}
 
 export const registerAddress = async ({
   hre,
@@ -27,19 +48,12 @@ export const registerAddress = async ({
   }
 
   console.log(`Registering address for ${name} to ${address}...`)
-  const tx = await Lib_AddressManager.setAddress(name, address)
-  await tx.wait()
-
-  const remoteAddress = await Lib_AddressManager.getAddress(name)
-  if (remoteAddress !== address) {
-    throw new Error(
-      `\n**FATAL ERROR. THIS SHOULD NEVER HAPPEN. CHECK YOUR DEPLOYMENT.**:\n` +
-        `Call to Lib_AddressManager.setAddress(${name}) was unsuccessful.\n` +
-        `Attempted to set address to: ${address}\n` +
-        `Actual address was set to: ${remoteAddress}\n` +
-        `This could indicate a compromised deployment.`
-    )
-  }
+  await Lib_AddressManager.setAddress(name, address)
+
+  console.log(`Waiting for registration to reflect on-chain...`)
+  await waitUntilTrue(async () => {
+    return hexStringEquals(await Lib_AddressManager.getAddress(name), address)
+  })
 
   console.log(`✓ Registered address for ${name}`)
 }
@@ -49,11 +63,15 @@ export const deployAndRegister = async ({
   name,
   args,
   contract,
+  iface,
+  postDeployAction,
 }: {
   hre: any
   name: string
   args: any[]
   contract?: string
+  iface?: string
+  postDeployAction?: (contract: Contract) => Promise<void>
 }) => {
   const { deploy } = hre.deployments
   const { deployer } = await hre.getNamedAccounts()
@@ -68,6 +86,17 @@ export const deployAndRegister = async ({
   await hre.ethers.provider.waitForTransaction(result.transactionHash)
 
   if (result.newlyDeployed) {
+    if (postDeployAction) {
+      const signer = hre.ethers.provider.getSigner(deployer)
+      let abi = result.abi
+      if (iface !== undefined) {
+        const factory = await hre.ethers.getContractFactory(iface)
+        abi = factory.interface
+      }
+      const instance = new Contract(result.address, abi, signer)
+      await postDeployAction(instance)
+    }
+
     await registerAddress({
       hre,
       name,

packages/core-utils/src/common/hex-strings.ts

 /* Imports: External */
-import { BigNumber } from 'ethers'
+import { BigNumber, ethers } from 'ethers'
 
 /**
  * Removes "0x" from start of a string if it exists.
@@ -80,3 +80,15 @@ export const padHexString = (str: string, length: number): string => {
 
 export const encodeHex = (val: any, len: number) =>
   remove0x(BigNumber.from(val).toHexString()).padStart(len, '0')
+
+export const hexStringEquals = (stringA: string, stringB: string): boolean => {
+  if (!ethers.utils.isHexString(stringA)) {
+    throw new Error(`input is not a hex string: ${stringA}`)
+  }
+
+  if (!ethers.utils.isHexString(stringB)) {
+    throw new Error(`input is not a hex string: ${stringB}`)
+  }
+
+  return stringA.toLowerCase() === stringB.toLowerCase()
+}