fix(contracts): various cleanup tasks in deploy process

packages/contracts/contracts/libraries/deployment/AddressDictator.sol

@@ -5,6 +5,7 @@ import { Lib_AddressManager } from "../resolver/Lib_AddressManager.sol";
 
 /**
  * @title AddressDictator
+ * @dev (glory to Arstotzka)
  */
 contract AddressDictator {
     /*********

packages/contracts/deploy/001-OVM_ChainStorageContainer_ctc_batches.deploy.ts

@@ -21,6 +21,6 @@ const deployFn: DeployFunction = async (hre) => {
   })
 }
 
-deployFn.tags = ['upgrade', 'ChainStorageContainer_ctc_batches']
+deployFn.tags = ['ChainStorageContainer_ctc_batches', 'upgrade']
 
 export default deployFn

packages/contracts/deploy/002-OVM_ChainStorageContainer_scc_batches.deploy.ts

@@ -21,6 +21,6 @@ const deployFn: DeployFunction = async (hre) => {
   })
 }
 
-deployFn.tags = ['upgrade', 'ChainStorageContainer_scc_batches']
+deployFn.tags = ['ChainStorageContainer_scc_batches', 'upgrade']
 
 export default deployFn

packages/contracts/deploy/003-OVM_CanonicalTransactionChain.deploy.ts

@@ -25,6 +25,6 @@ const deployFn: DeployFunction = async (hre) => {
   })
 }
 
-deployFn.tags = ['upgrade', 'CanonicalTransactionChain']
+deployFn.tags = ['CanonicalTransactionChain', 'upgrade']
 
 export default deployFn

packages/contracts/deploy/004-OVM_StateCommitmentChain.deploy.ts

@@ -24,6 +24,6 @@ const deployFn: DeployFunction = async (hre) => {
   })
 }
 
-deployFn.tags = ['upgrade', 'StateCommitmentChain']
+deployFn.tags = ['StateCommitmentChain', 'upgrade']
 
 export default deployFn

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

@@ -20,6 +20,6 @@ const deployFn: DeployFunction = async (hre) => {
   })
 }
 
-deployFn.tags = ['upgrade', 'BondManager']
+deployFn.tags = ['BondManager', 'upgrade']
 
 export default deployFn

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

@@ -39,6 +39,6 @@ const deployFn: DeployFunction = async (hre) => {
   })
 }
 
-deployFn.tags = ['upgrade', 'L1CrossDomainMessenger']
+deployFn.tags = ['L1CrossDomainMessenger', 'upgrade']
 
 export default deployFn

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

@@ -15,9 +15,6 @@ const deployFn: DeployFunction = async (hre) => {
     'Lib_AddressManager'
   )
 
-  // todo: this fails when trying to do a fresh deploy, because Lib_ResolvedDelegateProxy
-  // requires that the implementation has already been set in the Address Manager.
-  // The revert message is: 'Target address must be initialized'
   await deployAndPostDeploy({
     hre,
     name: 'Proxy__OVM_L1CrossDomainMessenger',

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

@@ -135,6 +135,6 @@ const deployFn: DeployFunction = async (hre) => {
   })
 }
 
-deployFn.tags = ['upgrade', 'L1StandardBridge']
+deployFn.tags = ['L1StandardBridge', 'upgrade']
 
 export default deployFn

packages/contracts/deploy/010-AddressSetter.deploy.ts → packages/contracts/deploy/010-AddressDictator.deploy.ts

@@ -64,9 +64,14 @@ const deployFn: DeployFunction = async (hre) => {
 
   // Filter out all addresses that will not change, so that the log statement is maximally
   // verifiable and readable.
-  namesAndAddresses = namesAndAddresses.filter(async ({ name, address }) => {
-    const existingAddress = await Lib_AddressManager.getAddress(name)
-    return !hexStringEquals(existingAddress, address)
+  const existingAddresses = {}
+  for (const pair of namesAndAddresses) {
+    existingAddresses[pair.name] = await Lib_AddressManager.getAddress(
+      pair.name
+    )
+  }
+  namesAndAddresses = namesAndAddresses.filter(({ name, address }) => {
+    return !hexStringEquals(existingAddresses[name], address)
   })
 
   await deployAndPostDeploy({

packages/contracts/deploy/011-set-addresses.ts

@@ -11,65 +11,75 @@ import {
 
 const deployFn: DeployFunction = async (hre) => {
   const { deployer } = await hre.getNamedAccounts()
-  const addressDictator = await getContractFromArtifact(
+
+  // We use this task to print out the list of addresses that will be updated by the
+  // AddressDictator contract. The idea here is that the owner of the AddressManager will then
+  // review these names and addresses before transferring ownership to the AddressDictator.
+  // Once ownership has been transferred to the AddressDictator, we execute `setAddresses` which
+  // triggers a series of setAddress calls on the AddressManager and then transfers ownership back
+  // to the original owner.
+
+  // First get relevant contract references.
+  const AddressDictator = await getContractFromArtifact(
     hre,
     'AddressDictator',
     {
       signerOrProvider: deployer,
     }
   )
-  const libAddressManager = await getContractFromArtifact(
+  const Lib_AddressManager = await getContractFromArtifact(
     hre,
     'Lib_AddressManager'
   )
-  const namedAddresses = await addressDictator.getNamedAddresses()
-  const finalOwner = await addressDictator.finalOwner()
-  let currentOwner = await libAddressManager.owner()
-
-  console.log(
-    '\n',
-    'An Address Dictator contract has been deployed, with the following name/address pairs:'
-  )
-  for (const namedAddress of namedAddresses) {
-    // Set alignment for readability
-    const padding = ' '.repeat(40 - namedAddress.name.length)
-    console.log(`${namedAddress.name}${padding}  ${namedAddress.addr}`)
-  }
-  console.log(
-    '\n',
-    'Please verify the values above, and the deployment steps up to this point,'
-  )
-  console.log(
-    `  then transfer ownership of the Address Manager at (${libAddressManager.address})`
-  )
-  console.log(
-    `  to the Address Dictator contract at ${addressDictator.address}.`
-  )
+  const namedAddresses: Array<{ name: string; addr: string }> =
+    await AddressDictator.getNamedAddresses()
+  const finalOwner = await AddressDictator.finalOwner()
+  const currentOwner = await Lib_AddressManager.owner()
 
+  // Check if the hardhat runtime environment has the owner of the AddressManager. This will only
+  // happen in CI. If this is the case, we can skip directly to transferring ownership over to the
+  // AddressDictator contract.
   const hreSigners = await hre.ethers.getSigners()
-  const hreSignerAddresses = hreSigners.map((signer) => {
-    return signer.address
+  const hreHasOwner = hreSigners.some((signer) => {
+    return hexStringEquals(signer.address, currentOwner)
   })
-  if (
-    hreSignerAddresses.some((addr) => {
-      return hexStringEquals(addr, currentOwner)
-    })
-  ) {
-    console.log(
-      'Deploy script owns the address manager, this must be CI. Setting addresses...'
-    )
+
+  if (hreHasOwner) {
+    // Hardhat has the owner loaded into it, we can skip directly to transferOwnership.
     const owner = await hre.ethers.getSigner(currentOwner)
-    await libAddressManager
-      .connect(owner)
-      .transferOwnership(addressDictator.address)
+    await Lib_AddressManager.connect(owner).transferOwnership(
+      AddressDictator.address
+    )
+  } else {
+    console.log(`
+      The AddressDictator contract (glory to Arstotzka) has been deployed.
+
+      Name/Address pairs:
+      ${namedAddresses.map((namedAddress) => {
+        const padding = ' '.repeat(40 - namedAddress.name.length)
+        return `
+          ${namedAddress.name}${padding}  ${namedAddress.addr}
+        `
+      })}
+
+      Current AddressManager owner: ${currentOwner}
+      Final AddressManager owner: ${finalOwner}
+
+      Please verify the values above, and the deployment steps up to this point,
+        then transfer ownership of the AddressManager at ${
+          Lib_AddressManager.address
+        }
+        to the AddressDictator contract at ${AddressDictator.address}.
+    `)
   }
 
+  // Wait for ownership to be transferred to the AddressDictator contract.
   await waitUntilTrue(
     async () => {
-      console.log('Checking ownership of Lib_AddressManager... ')
-      currentOwner = await libAddressManager.owner()
-      console.log('Lib_AddressManager owner is now set to AddressDictator.')
-      return hexStringEquals(currentOwner, addressDictator.address)
+      return hexStringEquals(
+        await Lib_AddressManager.owner(),
+        AddressDictator.address
+      )
     },
     {
       // Try every 30 seconds for 500 minutes.
@@ -80,22 +90,15 @@ const deployFn: DeployFunction = async (hre) => {
 
   // Set the addresses!
   console.log('Ownership successfully transferred. Invoking setAddresses...')
-  await addressDictator.setAddresses()
+  await AddressDictator.setAddresses()
 
-  await waitUntilTrue(
-    async () => {
-      console.log('Verifying final ownership of Lib_AddressManager')
-      currentOwner = await libAddressManager.owner()
-      return hexStringEquals(currentOwner, finalOwner)
-    },
-    {
-      // Try every 10 seconds
-      delay: 10_000,
-      retries: 1000,
-    }
-  )
+  // Make sure ownership has been correctly sent back to the original owner.
+  console.log('Verifying final ownership of Lib_AddressManager...')
+  await waitUntilTrue(async () => {
+    return hexStringEquals(await Lib_AddressManager.owner(), finalOwner)
+  })
 }
 
-deployFn.tags = ['upgrade', 'set-addresses']
+deployFn.tags = ['set-addresses', 'upgrade']
 
 export default deployFn

packages/contracts/deploy/012-initialize-Proxy__L1CrossDomainMessenger.ts

@@ -11,10 +11,13 @@ import {
 const deployFn: DeployFunction = async (hre) => {
   const { deployer } = await hre.getNamedAccounts()
 
-  // There is a risk that on a fresh deployment we could get front-run,
-  // and the Proxy would be bricked. But that feels unlikely, and we can recover from it.
   console.log(`Initializing Proxy__L1CrossDomainMessenger...`)
-  const proxy = await getContractFromArtifact(
+
+  // There's a risk that we could get front-run during a fresh deployment, which would brick this
+  // contract and require that the proxy be re-deployed. We will not have this risk once we move
+  // entirely to chugsplash-style deployments. It's unlikely to happen and relatively easy to
+  // recover from so let's just ignore it for now.
+  const Proxy__OVM_L1CrossDomainMessenger = await getContractFromArtifact(
     hre,
     'Proxy__OVM_L1CrossDomainMessenger',
     {
@@ -22,17 +25,19 @@ const deployFn: DeployFunction = async (hre) => {
       signerOrProvider: deployer,
     }
   )
-  const libAddressManager = await getContractFromArtifact(
+
+  const Lib_AddressManager = await getContractFromArtifact(
     hre,
     'Lib_AddressManager'
   )
-  await proxy.initialize(libAddressManager.address)
+
+  await Proxy__OVM_L1CrossDomainMessenger.initialize(Lib_AddressManager.address)
 
   console.log(`Checking that contract was correctly initialized...`)
   await waitUntilTrue(async () => {
     return hexStringEquals(
-      await proxy.libAddressManager(),
-      libAddressManager.address
+      await Proxy__OVM_L1CrossDomainMessenger.libAddressManager(),
+      Lib_AddressManager.address
     )
   })
 }