fix(ctp): post-audit cleanup for the erc721 bridge

.changeset/khaki-wombats-notice.md

@@ -0,0 +1,5 @@
+---
+'@eth-optimism/contracts-periphery': patch
+---
+
+Adds input validation to the ERC721Bridge constructor, fixes a typo in the L1ERC721Bridge, and removes the ERC721Refunded event declaration.

packages/contracts-periphery/contracts/L1/L1ERC721Bridge.sol

@@ -10,7 +10,7 @@ import { Semver } from "@eth-optimism/contracts-bedrock/contracts/universal/Semv
  * @title L1ERC721Bridge
  * @notice The L1 ERC721 bridge is a contract which works together with the L2 ERC721 bridge to
  *         make it possible to transfer ERC721 tokens from Ethereum to Optimism. This contract
- *         acts as an escrow for ERC721 tokens deposted into L2.
+ *         acts as an escrow for ERC721 tokens deposited into L2.
  */
 contract L1ERC721Bridge is ERC721Bridge, Semver {
     /**

packages/contracts-periphery/contracts/universal/op-erc721/ERC721Bridge.sol

@@ -30,24 +30,6 @@ abstract contract ERC721Bridge {
         bytes extraData
     );
 
-    /**
-     * @notice Emitted when an NFT is refunded to the owner after an ERC721 bridge from the other
-     *         chain fails.
-     *
-     * @param localToken  Address of the token on this domain.
-     * @param remoteToken Address of the token on the remote domain.
-     * @param to          Address to receive the refunded token.
-     * @param tokenId     ID of the specific token being refunded.
-     * @param extraData   Extra data for use on the client-side.
-     */
-    event ERC721Refunded(
-        address indexed localToken,
-        address indexed remoteToken,
-        address indexed to,
-        uint256 tokenId,
-        bytes extraData
-    );
-
     /**
      * @notice Emitted when an ERC721 bridge from the other network is finalized.
      *
@@ -125,6 +107,9 @@ abstract contract ERC721Bridge {
      * @param _otherBridge Address of the ERC721 bridge on the other network.
      */
     constructor(address _messenger, address _otherBridge) {
+        require(_messenger != address(0), "ERC721Bridge: messenger cannot be address(0)");
+        require(_otherBridge != address(0), "ERC721Bridge: other bridge cannot be address(0)");
+
         messenger = CrossDomainMessenger(_messenger);
         otherBridge = _otherBridge;
     }

packages/contracts-periphery/test/contracts/L1/L1ERC721Bridge.spec.ts

@@ -85,6 +85,24 @@ describe('L1ERC721Bridge', () => {
 
   describe('constructor', async () => {
     it('initializes correctly', async () => {
+      it('reverts if cross domain messenger is address(0)', async () => {
+        await expect(
+          Factory__L1ERC721Bridge.deploy(
+            constants.AddressZero,
+            DUMMY_L2_BRIDGE_ADDRESS
+          )
+        ).to.be.revertedWith('ERC721Bridge: messenger cannot be address(0)')
+      })
+
+      it('reverts if other bridge is address(0)', async () => {
+        await expect(
+          Factory__L1ERC721Bridge.deploy(
+            Fake__L1CrossDomainMessenger.address,
+            constants.AddressZero
+          )
+        ).to.be.revertedWith('ERC721Bridge: other bridge cannot be address(0)')
+      })
+
       expect(await L1ERC721Bridge.messenger()).equals(
         Fake__L1CrossDomainMessenger.address
       )

packages/contracts-periphery/test/contracts/L2/L2ERC721Bridge.spec.ts

@@ -36,6 +36,7 @@ describe('L2ERC721Bridge', () => {
     Factory__L1ERC721Bridge = await ethers.getContractFactory('L1ERC721Bridge')
   })
 
+  let Factory__L2ERC721Bridge: ContractFactory
   let L2ERC721Bridge: Contract
   let L2ERC721: Contract
   let Fake__L2CrossDomainMessenger: FakeContract
@@ -48,9 +49,11 @@ describe('L2ERC721Bridge', () => {
     )
 
     // Deploy the contract under test
-    L2ERC721Bridge = await (
-      await ethers.getContractFactory('L2ERC721Bridge')
-    ).deploy(Fake__L2CrossDomainMessenger.address, DUMMY_L1BRIDGE_ADDRESS)
+    Factory__L2ERC721Bridge = await ethers.getContractFactory('L2ERC721Bridge')
+    L2ERC721Bridge = await Factory__L2ERC721Bridge.deploy(
+      Fake__L2CrossDomainMessenger.address,
+      DUMMY_L1BRIDGE_ADDRESS
+    )
 
     // Deploy an L2 ERC721
     L2ERC721 = await (
@@ -66,6 +69,24 @@ describe('L2ERC721Bridge', () => {
   })
 
   describe('constructor', async () => {
+    it('reverts if cross domain messenger is address(0)', async () => {
+      await expect(
+        Factory__L2ERC721Bridge.deploy(
+          constants.AddressZero,
+          DUMMY_L1BRIDGE_ADDRESS
+        )
+      ).to.be.revertedWith('ERC721Bridge: messenger cannot be address(0)')
+    })
+
+    it('reverts if other bridge is address(0)', async () => {
+      await expect(
+        Factory__L2ERC721Bridge.deploy(
+          Fake__L2CrossDomainMessenger.address,
+          constants.AddressZero
+        )
+      ).to.be.revertedWith('ERC721Bridge: other bridge cannot be address(0)')
+    })
+
     it('initializes correctly', async () => {
       expect(await L2ERC721Bridge.messenger()).equals(
         Fake__L2CrossDomainMessenger.address