maint(ct): clean up L1XDM tests (#2459)

Cleans up the L1CrossDomainMessenger tests using the new deploy
function. Removes some unnecessary code.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

packages/contracts/test/contracts/L1/messaging/L1CrossDomainMessenger.spec.ts

-/* External Imports */
 import { ethers } from 'hardhat'
-import { Signer, ContractFactory, Contract, BigNumber } from 'ethers'
-import {
-  smock,
-  MockContractFactory,
-  FakeContract,
-} from '@defi-wonderland/smock'
-import {
-  remove0x,
-  toHexString,
-  applyL1ToL2Alias,
-} from '@eth-optimism/core-utils'
+import { Contract, BigNumber } from 'ethers'
+import { smock, FakeContract, MockContract } from '@defi-wonderland/smock'
+import { toHexString, applyL1ToL2Alias } from '@eth-optimism/core-utils'
+import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'
 
-/* Internal Imports */
 import { expect } from '../../../setup'
 import {
-  makeAddressManager,
   setProxyTarget,
   NON_NULL_BYTES32,
   NON_ZERO_ADDRESS,
@@ -28,46 +18,22 @@ import {
   encodeXDomainCalldata,
   getEthTime,
   setEthTime,
+  deploy,
 } from '../../../helpers'
 import { predeploys } from '../../../../src'
 
 const MAX_GAS_LIMIT = 8_000_000
 
-const deployProxyXDomainMessenger = async (
-  addressManager: Contract,
-  l1XDomainMessenger: Contract
-): Promise<Contract> => {
-  await addressManager.setAddress(
-    'L1CrossDomainMessenger',
-    l1XDomainMessenger.address
-  )
-  const proxy = await (
-    await ethers.getContractFactory('Lib_ResolvedDelegateProxy')
-  ).deploy(addressManager.address, 'L1CrossDomainMessenger')
-  return l1XDomainMessenger.attach(proxy.address)
-}
-
 describe('L1CrossDomainMessenger', () => {
-  let signer: Signer
-  let signer2: Signer
+  let signer1: SignerWithAddress
+  let signer2: SignerWithAddress
   before(async () => {
-    ;[signer, signer2] = await ethers.getSigners()
-  })
-
-  let AddressManager: Contract
-  before(async () => {
-    AddressManager = await makeAddressManager()
+    ;[signer1, signer2] = await ethers.getSigners()
   })
 
   let Fake__TargetContract: FakeContract
   let Fake__L2CrossDomainMessenger: FakeContract
   let Fake__StateCommitmentChain: FakeContract
-
-  let Factory__CanonicalTransactionChain: ContractFactory
-  let Factory__ChainStorageContainer: ContractFactory
-  let Factory__L1CrossDomainMessenger: ContractFactory
-
-  let CanonicalTransactionChain: Contract
   before(async () => {
     Fake__TargetContract = await smock.fake<Contract>('Helper_SimpleProxy')
     Fake__L2CrossDomainMessenger = await smock.fake<Contract>(
@@ -79,6 +45,12 @@ describe('L1CrossDomainMessenger', () => {
     Fake__StateCommitmentChain = await smock.fake<Contract>(
       'StateCommitmentChain'
     )
+  })
+
+  let AddressManager: Contract
+  let CanonicalTransactionChain: Contract
+  before(async () => {
+    AddressManager = await deploy('Lib_AddressManager')
 
     await AddressManager.setAddress(
       'L2CrossDomainMessenger',
@@ -91,32 +63,18 @@ describe('L1CrossDomainMessenger', () => {
       Fake__StateCommitmentChain
     )
 
-    Factory__CanonicalTransactionChain = await ethers.getContractFactory(
-      'CanonicalTransactionChain'
-    )
-
-    Factory__ChainStorageContainer = await ethers.getContractFactory(
-      'ChainStorageContainer'
-    )
-
-    Factory__L1CrossDomainMessenger = await ethers.getContractFactory(
-      'L1CrossDomainMessenger'
-    )
-    CanonicalTransactionChain = await Factory__CanonicalTransactionChain.deploy(
-      AddressManager.address,
-      MAX_GAS_LIMIT,
-      L2_GAS_DISCOUNT_DIVISOR,
-      ENQUEUE_GAS_COST
-    )
+    CanonicalTransactionChain = await deploy('CanonicalTransactionChain', {
+      args: [
+        AddressManager.address,
+        MAX_GAS_LIMIT,
+        L2_GAS_DISCOUNT_DIVISOR,
+        ENQUEUE_GAS_COST,
+      ],
+    })
 
-    const batches = await Factory__ChainStorageContainer.deploy(
-      AddressManager.address,
-      'CanonicalTransactionChain'
-    )
-    await Factory__ChainStorageContainer.deploy(
-      AddressManager.address,
-      'CanonicalTransactionChain'
-    )
+    const batches = await deploy('ChainStorageContainer', {
+      args: [AddressManager.address, 'CanonicalTransactionChain'],
+    })
 
     await AddressManager.setAddress(
       'ChainStorageContainer-CTC-batches',
@@ -131,12 +89,19 @@ describe('L1CrossDomainMessenger', () => {
 
   let L1CrossDomainMessenger: Contract
   beforeEach(async () => {
-    const xDomainMessengerImpl = await Factory__L1CrossDomainMessenger.deploy()
-    // We use an upgradable proxy for the XDomainMessenger--deploy & set up the proxy.
-    L1CrossDomainMessenger = await deployProxyXDomainMessenger(
-      AddressManager,
-      xDomainMessengerImpl
+    const xDomainMessengerImpl = await deploy('L1CrossDomainMessenger')
+
+    await AddressManager.setAddress(
+      'L1CrossDomainMessenger',
+      xDomainMessengerImpl.address
     )
+
+    const proxy = await deploy('Lib_ResolvedDelegateProxy', {
+      args: [AddressManager.address, 'L1CrossDomainMessenger'],
+    })
+
+    L1CrossDomainMessenger = xDomainMessengerImpl.attach(proxy.address)
+
     await L1CrossDomainMessenger.initialize(AddressManager.address)
   })
 
@@ -170,7 +135,7 @@ describe('L1CrossDomainMessenger', () => {
 
       const calldata = encodeXDomainCalldata(
         target,
-        await signer.getAddress(),
+        signer1.address,
         message,
         0
       )
@@ -207,14 +172,9 @@ describe('L1CrossDomainMessenger', () => {
     const oldGasLimit = 100_000
     const newGasLimit = 200_000
 
-    let sender: string
-    before(async () => {
-      sender = await signer.getAddress()
-    })
-
     let queueIndex: number
     beforeEach(async () => {
-      await L1CrossDomainMessenger.connect(signer).sendMessage(
+      await L1CrossDomainMessenger.connect(signer1).sendMessage(
         target,
         message,
         oldGasLimit
@@ -229,7 +189,7 @@ describe('L1CrossDomainMessenger', () => {
         await expect(
           L1CrossDomainMessenger.replayMessage(
             ethers.constants.AddressZero, // Wrong target
-            sender,
+            signer1.address,
             message,
             queueIndex,
             oldGasLimit,
@@ -255,7 +215,7 @@ describe('L1CrossDomainMessenger', () => {
         await expect(
           L1CrossDomainMessenger.replayMessage(
             target,
-            sender,
+            signer1.address,
             '0x', // Wrong message
             queueIndex,
             oldGasLimit,
@@ -268,7 +228,7 @@ describe('L1CrossDomainMessenger', () => {
         await expect(
           L1CrossDomainMessenger.replayMessage(
             target,
-            sender,
+            signer1.address,
             message,
             queueIndex - 1, // Wrong queue index
             oldGasLimit,
@@ -281,7 +241,7 @@ describe('L1CrossDomainMessenger', () => {
         await expect(
           L1CrossDomainMessenger.replayMessage(
             target,
-            sender,
+            signer1.address,
             message,
             queueIndex,
             oldGasLimit + 1, // Wrong gas limit
@@ -296,7 +256,7 @@ describe('L1CrossDomainMessenger', () => {
         await expect(
           L1CrossDomainMessenger.replayMessage(
             target,
-            sender,
+            signer1.address,
             message,
             queueIndex,
             oldGasLimit,
@@ -313,7 +273,7 @@ describe('L1CrossDomainMessenger', () => {
         await expect(
           L1CrossDomainMessenger.replayMessage(
             target,
-            sender,
+            signer1.address,
             message,
             queueIndex,
             oldGasLimit,
@@ -325,7 +285,7 @@ describe('L1CrossDomainMessenger', () => {
             applyL1ToL2Alias(L1CrossDomainMessenger.address),
             Fake__L2CrossDomainMessenger.address,
             newGasLimit,
-            encodeXDomainCalldata(target, sender, message, queueIndex),
+            encodeXDomainCalldata(target, signer1.address, message, queueIndex),
             newQueueIndex,
             newTimestamp
           )
@@ -339,7 +299,7 @@ describe('L1CrossDomainMessenger', () => {
       await expect(
         L1CrossDomainMessenger.replayMessage(
           target,
-          await signer.getAddress(),
+          signer1.address,
           message,
           queueLength - 1,
           oldGasLimit,
@@ -350,24 +310,22 @@ describe('L1CrossDomainMessenger', () => {
   })
 
   describe('xDomainMessageSender', () => {
-    let Mock__Factory__L1CrossDomainMessenger: MockContractFactory<ContractFactory>
-    let Mock__L1CrossDomainMessenger
+    let Mock__L1CrossDomainMessenger: MockContract<Contract>
     before(async () => {
-      Mock__Factory__L1CrossDomainMessenger = await smock.mock(
-        'L1CrossDomainMessenger'
-      )
-      Mock__L1CrossDomainMessenger =
-        await Mock__Factory__L1CrossDomainMessenger.deploy()
+      Mock__L1CrossDomainMessenger = await (
+        await smock.mock('L1CrossDomainMessenger')
+      ).deploy()
     })
 
     it('should return the xDomainMsgSender address', async () => {
       await Mock__L1CrossDomainMessenger.setVariable(
         'xDomainMsgSender',
-        '0x0000000000000000000000000000000000000000'
+        NON_ZERO_ADDRESS
       )
+
       expect(
         await Mock__L1CrossDomainMessenger.xDomainMessageSender()
-      ).to.equal('0x0000000000000000000000000000000000000000')
+      ).to.equal(NON_ZERO_ADDRESS)
     })
   })
 
@@ -388,10 +346,17 @@ describe('L1CrossDomainMessenger', () => {
     )
 
     const storageKey = ethers.utils.keccak256(
-      ethers.utils.keccak256(
-        calldata + remove0x(Fake__L2CrossDomainMessenger.address)
-      ) + '00'.repeat(32)
+      ethers.utils.hexConcat([
+        ethers.utils.keccak256(
+          ethers.utils.hexConcat([
+            calldata,
+            Fake__L2CrossDomainMessenger.address,
+          ])
+        ),
+        ethers.constants.HashZero,
+      ])
     )
+
     const storageGenerator = await TrieTestGenerator.fromNodes({
       nodes: [
         {
@@ -435,7 +400,6 @@ describe('L1CrossDomainMessenger', () => {
 
   describe('relayMessage', () => {
     let target: string
-    let sender: string
     let message: string
     let proof: any
     let calldata: string
@@ -444,11 +408,10 @@ describe('L1CrossDomainMessenger', () => {
       message = Fake__TargetContract.interface.encodeFunctionData('setTarget', [
         NON_ZERO_ADDRESS,
       ])
-      sender = await signer.getAddress()
 
       const mockProof = await generateMockRelayMessageProof(
         target,
-        sender,
+        signer1.address,
         message
       )
       proof = mockProof.proof
@@ -472,21 +435,27 @@ describe('L1CrossDomainMessenger', () => {
       }
 
       await expect(
-        L1CrossDomainMessenger.relayMessage(target, sender, message, 0, proof1)
+        L1CrossDomainMessenger.relayMessage(
+          target,
+          signer1.address,
+          message,
+          0,
+          proof1
+        )
       ).to.be.revertedWith('Provided message could not be verified.')
     })
 
     it('should revert if attempting to relay a message sent to an L1 system contract', async () => {
       const maliciousProof = await generateMockRelayMessageProof(
         CanonicalTransactionChain.address,
-        sender,
+        signer1.address,
         message
       )
 
       await expect(
         L1CrossDomainMessenger.relayMessage(
           CanonicalTransactionChain.address,
-          sender,
+          signer1.address,
           message,
           0,
           maliciousProof.proof
@@ -508,25 +477,43 @@ describe('L1CrossDomainMessenger', () => {
       }
 
       await expect(
-        L1CrossDomainMessenger.relayMessage(target, sender, message, 0, proof1)
+        L1CrossDomainMessenger.relayMessage(
+          target,
+          signer1.address,
+          message,
+          0,
+          proof1
+        )
       ).to.be.revertedWith('Provided message could not be verified.')
     })
 
     it('should revert if provided an invalid storage trie witness', async () => {
       await expect(
-        L1CrossDomainMessenger.relayMessage(target, sender, message, 0, {
-          ...proof,
-          storageTrieWitness: '0x',
-        })
+        L1CrossDomainMessenger.relayMessage(
+          target,
+          signer1.address,
+          message,
+          0,
+          {
+            ...proof,
+            storageTrieWitness: '0x',
+          }
+        )
       ).to.be.reverted
     })
 
     it('should revert if provided an invalid state trie witness', async () => {
       await expect(
-        L1CrossDomainMessenger.relayMessage(target, sender, message, 0, {
-          ...proof,
-          stateTrieWitness: '0x',
-        })
+        L1CrossDomainMessenger.relayMessage(
+          target,
+          signer1.address,
+          message,
+          0,
+          {
+            ...proof,
+            stateTrieWitness: '0x',
+          }
+        )
       ).to.be.reverted
     })
 
@@ -535,7 +522,7 @@ describe('L1CrossDomainMessenger', () => {
 
       await L1CrossDomainMessenger.relayMessage(
         target,
-        sender,
+        signer1.address,
         message,
         0,
         proof
@@ -550,12 +537,14 @@ describe('L1CrossDomainMessenger', () => {
       expect(
         await L1CrossDomainMessenger.relayedMessages(
           ethers.utils.keccak256(
-            calldata +
-              remove0x(await signer.getAddress()) +
-              remove0x(BigNumber.from(blockNumber).toHexString()).padStart(
-                64,
-                '0'
-              )
+            ethers.utils.hexConcat([
+              calldata,
+              signer1.address,
+              ethers.utils.hexZeroPad(
+                BigNumber.from(blockNumber).toHexString(),
+                32
+              ),
+            ])
           )
         )
       ).to.equal(true)
@@ -565,13 +554,15 @@ describe('L1CrossDomainMessenger', () => {
       await expect(
         L1CrossDomainMessenger.xDomainMessageSender()
       ).to.be.revertedWith('xDomainMessageSender is not set')
+
       await L1CrossDomainMessenger.relayMessage(
         target,
-        sender,
+        signer1.address,
         message,
         0,
         proof
       )
+
       await expect(
         L1CrossDomainMessenger.xDomainMessageSender()
       ).to.be.revertedWith('xDomainMessageSender is not set')
@@ -580,14 +571,20 @@ describe('L1CrossDomainMessenger', () => {
     it('should revert if trying to send the same message twice', async () => {
       await L1CrossDomainMessenger.relayMessage(
         target,
-        sender,
+        signer1.address,
         message,
         0,
         proof
       )
 
       await expect(
-        L1CrossDomainMessenger.relayMessage(target, sender, message, 0, proof)
+        L1CrossDomainMessenger.relayMessage(
+          target,
+          signer1.address,
+          message,
+          0,
+          proof
+        )
       ).to.be.revertedWith('Provided message has already been received.')
     })
 
@@ -595,13 +592,20 @@ describe('L1CrossDomainMessenger', () => {
       await L1CrossDomainMessenger.pause()
 
       await expect(
-        L1CrossDomainMessenger.relayMessage(target, sender, message, 0, proof)
+        L1CrossDomainMessenger.relayMessage(
+          target,
+          signer1.address,
+          message,
+          0,
+          proof
+        )
       ).to.be.revertedWith('Pausable: paused')
     })
 
     describe('blockMessage and allowMessage', () => {
       it('should revert if called by an account other than the owner', async () => {
         const L1CrossDomainMessenger2 = L1CrossDomainMessenger.connect(signer2)
+
         await expect(
           L1CrossDomainMessenger2.blockMessage(ethers.utils.keccak256(calldata))
         ).to.be.revertedWith('Ownable: caller is not the owner')
@@ -617,7 +621,13 @@ describe('L1CrossDomainMessenger', () => {
         )
 
         await expect(
-          L1CrossDomainMessenger.relayMessage(target, sender, message, 0, proof)
+          L1CrossDomainMessenger.relayMessage(
+            target,
+            signer1.address,
+            message,
+            0,
+            proof
+          )
         ).to.be.revertedWith('Provided message has been blocked.')
       })
 
@@ -627,7 +637,13 @@ describe('L1CrossDomainMessenger', () => {
         )
 
         await expect(
-          L1CrossDomainMessenger.relayMessage(target, sender, message, 0, proof)
+          L1CrossDomainMessenger.relayMessage(
+            target,
+            signer1.address,
+            message,
+            0,
+            proof
+          )
         ).to.be.revertedWith('Provided message has been blocked.')
 
         await L1CrossDomainMessenger.allowMessage(
@@ -635,7 +651,13 @@ describe('L1CrossDomainMessenger', () => {
         )
 
         await expect(
-          L1CrossDomainMessenger.relayMessage(target, sender, message, 0, proof)
+          L1CrossDomainMessenger.relayMessage(
+            target,
+            signer1.address,
+            message,
+            0,
+            proof
+          )
         ).to.not.be.reverted
       })
     })