feat(ctp): simplify and standardize ERC721 bridge (#2773)

* feat(ctp): simplify and standardize ERC721 bridge

Significantly simplifies the ERC721 bridge contracts and standardizes
them according to our new seaport-style standard. Removes some
unnecessary code and updates the interface for the bridge to match the
expected interface for the ERC20 bridge after Bedrock.

* integration-tests: fixup

* integration-tests: fix constructor

* fix: circular dep in itests

* tests: fixes

* fix(ctp): add version to contracts

* fix: test failures

* fix: tests

* itests: additional check

* whoops

* fix: address review feedback

* fix: rename data to extraData

Co-authored-by: Mark Tyneway <mark.tyneway@gmail.com>

.changeset/happy-carpets-rush.md

@@ -0,0 +1,5 @@
+---
+'@eth-optimism/contracts-periphery': patch
+---
+
+Simplify, cleanup, and standardize ERC721 bridge contracts.

integration-tests/contracts/FakeL2StandardERC721.sol → integration-tests/contracts/FakeOptimismMintableERC721.sol

@@ -3,14 +3,14 @@ pragma solidity ^0.8.9;
 
 import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
 
-contract FakeL2StandardERC721 is ERC721 {
+contract FakeOptimismMintableERC721 is ERC721 {
 
-    address public immutable l1Token;
-    address public immutable l2Bridge;
+    address public immutable remoteToken;
+    address public immutable bridge;
 
-    constructor(address _l1Token, address _l2Bridge) ERC721("FakeERC721", "FAKE") {
-        l1Token = _l1Token;
-        l2Bridge = _l2Bridge;
+    constructor(address _remoteToken, address _bridge) ERC721("FakeERC721", "FAKE") {
+        remoteToken = _remoteToken;
+        bridge = _bridge;
     }
 
     function mint(address to, uint256 tokenId) public {

integration-tests/test/nft-bridge.spec.ts

@@ -5,8 +5,8 @@ import { predeploys } from '@eth-optimism/contracts'
 import Artifact__TestERC721 from '@eth-optimism/contracts-periphery/artifacts/contracts/testing/helpers/TestERC721.sol/TestERC721.json'
 import Artifact__L1ERC721Bridge from '@eth-optimism/contracts-periphery/artifacts/contracts/L1/messaging/L1ERC721Bridge.sol/L1ERC721Bridge.json'
 import Artifact__L2ERC721Bridge from '@eth-optimism/contracts-periphery/artifacts/contracts/L2/messaging/L2ERC721Bridge.sol/L2ERC721Bridge.json'
-import Artifact__L2StandardERC721Factory from '@eth-optimism/contracts-periphery/artifacts/contracts/L2/messaging/L2StandardERC721Factory.sol/L2StandardERC721Factory.json'
-import Artifact__L2StandardERC721 from '@eth-optimism/contracts-periphery/artifacts/contracts/standards/L2StandardERC721.sol/L2StandardERC721.json'
+import Artifact__OptimismMintableERC721Factory from '@eth-optimism/contracts-periphery/artifacts/contracts/universal/op-erc721/OptimismMintableERC721Factory.sol/OptimismMintableERC721Factory.json'
+import Artifact__OptimismMintableERC721 from '@eth-optimism/contracts-periphery/artifacts/contracts/universal/op-erc721/OptimismMintableERC721.sol/OptimismMintableERC721.json'
 
 /* Imports: Internal */
 import { expect } from './shared/setup'
@@ -49,7 +49,7 @@ describe('ERC721 Bridge', () => {
   let Factory__L1ERC721: ContractFactory
   let Factory__L1ERC721Bridge: ContractFactory
   let Factory__L2ERC721Bridge: ContractFactory
-  let Factory__L2StandardERC721Factory: ContractFactory
+  let Factory__OptimismMintableERC721Factory: ContractFactory
   before(async () => {
     Factory__L1ERC721 = await ethers.getContractFactory(
       Artifact__TestERC721.abi,
@@ -66,58 +66,68 @@ describe('ERC721 Bridge', () => {
       Artifact__L2ERC721Bridge.bytecode,
       bobWalletL2
     )
-    Factory__L2StandardERC721Factory = await ethers.getContractFactory(
-      Artifact__L2StandardERC721Factory.abi,
-      Artifact__L2StandardERC721Factory.bytecode,
+    Factory__OptimismMintableERC721Factory = await ethers.getContractFactory(
+      Artifact__OptimismMintableERC721Factory.abi,
+      Artifact__OptimismMintableERC721Factory.bytecode,
       bobWalletL2
     )
   })
 
   let L1ERC721: Contract
   let L1ERC721Bridge: Contract
   let L2ERC721Bridge: Contract
-  let L2StandardERC721Factory: Contract
-  let L2StandardERC721: Contract
+  let OptimismMintableERC721Factory: Contract
+  let OptimismMintableERC721: Contract
   beforeEach(async () => {
     L1ERC721 = await Factory__L1ERC721.deploy()
     await L1ERC721.deployed()
 
-    L2ERC721Bridge = await Factory__L2ERC721Bridge.deploy(
-      predeploys.L2CrossDomainMessenger
-    )
-    await L2ERC721Bridge.deployed()
-
     L1ERC721Bridge = await Factory__L1ERC721Bridge.deploy(
       env.messenger.contracts.l1.L1CrossDomainMessenger.address,
-      L2ERC721Bridge.address
+      ethers.utils.getContractAddress({
+        from: await Factory__L2ERC721Bridge.signer.getAddress(),
+        nonce: await Factory__L2ERC721Bridge.signer.getTransactionCount(),
+      })
     )
     await L1ERC721Bridge.deployed()
 
-    L2StandardERC721Factory = await Factory__L2StandardERC721Factory.deploy(
+    L2ERC721Bridge = await Factory__L2ERC721Bridge.deploy(
+      predeploys.L2CrossDomainMessenger,
+      L1ERC721Bridge.address
+    )
+    await L2ERC721Bridge.deployed()
+
+    OptimismMintableERC721Factory =
+      await Factory__OptimismMintableERC721Factory.deploy(
+        L2ERC721Bridge.address
+      )
+    await OptimismMintableERC721Factory.deployed()
+
+    expect(await L1ERC721Bridge.otherBridge()).to.equal(L2ERC721Bridge.address)
+    expect(await L2ERC721Bridge.otherBridge()).to.equal(L1ERC721Bridge.address)
+
+    expect(await OptimismMintableERC721Factory.bridge()).to.equal(
       L2ERC721Bridge.address
     )
-    await L2StandardERC721Factory.deployed()
 
     // Create a L2 Standard ERC721 with the Standard ERC721 Factory
-    const tx = await L2StandardERC721Factory.createStandardL2ERC721(
-      L1ERC721.address,
-      'L2ERC721',
-      'L2'
-    )
-    await tx.wait()
+    const tx =
+      await OptimismMintableERC721Factory.createStandardOptimismMintableERC721(
+        L1ERC721.address,
+        'L2ERC721',
+        'L2'
+      )
+    const receipt = await tx.wait()
 
-    // Retrieve the deployed L2 Standard ERC721
-    const L2StandardERC721Address =
-      await L2StandardERC721Factory.standardERC721Mapping(L1ERC721.address)
-    L2StandardERC721 = await ethers.getContractAt(
-      Artifact__L2StandardERC721.abi,
-      L2StandardERC721Address
-    )
-    await L2StandardERC721.deployed()
+    // Get the OptimismMintableERC721Created event
+    const erc721CreatedEvent = receipt.events[0]
+    expect(erc721CreatedEvent.event).to.be.eq('OptimismMintableERC721Created')
 
-    // Initialize the L2 bridge contract
-    const tx1 = await L2ERC721Bridge.initialize(L1ERC721Bridge.address)
-    await tx1.wait()
+    OptimismMintableERC721 = await ethers.getContractAt(
+      Artifact__OptimismMintableERC721.abi,
+      erc721CreatedEvent.args.localToken
+    )
+    await OptimismMintableERC721.deployed()
 
     // Mint an L1 ERC721 to Bob on L1
     const tx2 = await L1ERC721.mint(bobAddress, TOKEN_ID)
@@ -128,11 +138,11 @@ describe('ERC721 Bridge', () => {
     await tx3.wait()
   })
 
-  it('depositERC721', async () => {
+  it('bridgeERC721', async () => {
     await env.messenger.waitForMessageReceipt(
-      await L1ERC721Bridge.depositERC721(
+      await L1ERC721Bridge.bridgeERC721(
         L1ERC721.address,
-        L2StandardERC721.address,
+        OptimismMintableERC721.address,
         TOKEN_ID,
         FINALIZATION_GAS,
         NON_NULL_BYTES
@@ -143,14 +153,14 @@ describe('ERC721 Bridge', () => {
     expect(await L1ERC721.ownerOf(TOKEN_ID)).to.equal(L1ERC721Bridge.address)
 
     // Bob owns the NFT on L2
-    expect(await L2StandardERC721.ownerOf(TOKEN_ID)).to.equal(bobAddress)
+    expect(await OptimismMintableERC721.ownerOf(TOKEN_ID)).to.equal(bobAddress)
   })
 
-  it('depositERC721To', async () => {
+  it('bridgeERC721To', async () => {
     await env.messenger.waitForMessageReceipt(
-      await L1ERC721Bridge.depositERC721To(
+      await L1ERC721Bridge.bridgeERC721To(
         L1ERC721.address,
-        L2StandardERC721.address,
+        OptimismMintableERC721.address,
         aliceAddress,
         TOKEN_ID,
         FINALIZATION_GAS,
@@ -162,15 +172,17 @@ describe('ERC721 Bridge', () => {
     expect(await L1ERC721.ownerOf(TOKEN_ID)).to.equal(L1ERC721Bridge.address)
 
     // Alice owns the NFT on L2
-    expect(await L2StandardERC721.ownerOf(TOKEN_ID)).to.equal(aliceAddress)
+    expect(await OptimismMintableERC721.ownerOf(TOKEN_ID)).to.equal(
+      aliceAddress
+    )
   })
 
-  withdrawalTest('withdrawERC721', async () => {
+  withdrawalTest('bridgeERC721', async () => {
     // Deposit an NFT into L2 so that there's something to withdraw
     await env.messenger.waitForMessageReceipt(
-      await L1ERC721Bridge.depositERC721(
+      await L1ERC721Bridge.bridgeERC721(
         L1ERC721.address,
-        L2StandardERC721.address,
+        OptimismMintableERC721.address,
         TOKEN_ID,
         FINALIZATION_GAS,
         NON_NULL_BYTES
@@ -181,10 +193,11 @@ describe('ERC721 Bridge', () => {
     expect(await L1ERC721.ownerOf(TOKEN_ID)).to.equal(L1ERC721Bridge.address)
 
     // Also check that Bob owns the NFT on L2 initially
-    expect(await L2StandardERC721.ownerOf(TOKEN_ID)).to.equal(bobAddress)
+    expect(await OptimismMintableERC721.ownerOf(TOKEN_ID)).to.equal(bobAddress)
 
-    const tx = await L2ERC721Bridge.withdrawERC721(
-      L2StandardERC721.address,
+    const tx = await L2ERC721Bridge.bridgeERC721(
+      OptimismMintableERC721.address,
+      L1ERC721.address,
       TOKEN_ID,
       0,
       NON_NULL_BYTES
@@ -196,15 +209,15 @@ describe('ERC721 Bridge', () => {
     expect(await L1ERC721.ownerOf(TOKEN_ID)).to.equal(bobAddress)
 
     // L2 NFT is burned
-    await expect(L2StandardERC721.ownerOf(TOKEN_ID)).to.be.reverted
+    await expect(OptimismMintableERC721.ownerOf(TOKEN_ID)).to.be.reverted
   })
 
-  withdrawalTest('withdrawERC721To', async () => {
+  withdrawalTest('bridgeERC721To', async () => {
     // Deposit an NFT into L2 so that there's something to withdraw
     await env.messenger.waitForMessageReceipt(
-      await L1ERC721Bridge.depositERC721(
+      await L1ERC721Bridge.bridgeERC721(
         L1ERC721.address,
-        L2StandardERC721.address,
+        OptimismMintableERC721.address,
         TOKEN_ID,
         FINALIZATION_GAS,
         NON_NULL_BYTES
@@ -215,10 +228,11 @@ describe('ERC721 Bridge', () => {
     expect(await L1ERC721.ownerOf(TOKEN_ID)).to.equal(L1ERC721Bridge.address)
 
     // Also check that Bob owns the NFT on L2 initially
-    expect(await L2StandardERC721.ownerOf(TOKEN_ID)).to.equal(bobAddress)
+    expect(await OptimismMintableERC721.ownerOf(TOKEN_ID)).to.equal(bobAddress)
 
-    const tx = await L2ERC721Bridge.withdrawERC721To(
-      L2StandardERC721.address,
+    const tx = await L2ERC721Bridge.bridgeERC721To(
+      OptimismMintableERC721.address,
+      L1ERC721.address,
       aliceAddress,
       TOKEN_ID,
       0,
@@ -231,17 +245,17 @@ describe('ERC721 Bridge', () => {
     expect(await L1ERC721.ownerOf(TOKEN_ID)).to.equal(aliceAddress)
 
     // L2 NFT is burned
-    await expect(L2StandardERC721.ownerOf(TOKEN_ID)).to.be.reverted
+    await expect(OptimismMintableERC721.ownerOf(TOKEN_ID)).to.be.reverted
   })
 
   withdrawalTest(
     'should not allow an arbitrary L2 NFT to be withdrawn in exchange for a legitimate L1 NFT',
     async () => {
       // First, deposit the legitimate L1 NFT.
       await env.messenger.waitForMessageReceipt(
-        await L1ERC721Bridge.depositERC721(
+        await L1ERC721Bridge.bridgeERC721(
           L1ERC721.address,
-          L2StandardERC721.address,
+          OptimismMintableERC721.address,
           TOKEN_ID,
           FINALIZATION_GAS,
           NON_NULL_BYTES
@@ -253,25 +267,26 @@ describe('ERC721 Bridge', () => {
       // Deploy a fake L2 ERC721, which:
       // - Returns the address of the legitimate L1 token from its l1Token() getter.
       // - Allows the L2 bridge to call its burn() function.
-      const FakeL2StandardERC721 = await (
-        await ethers.getContractFactory('FakeL2StandardERC721', bobWalletL2)
+      const FakeOptimismMintableERC721 = await (
+        await ethers.getContractFactory('FakeOptimismMintableERC721', bobWalletL2)
       ).deploy(L1ERC721.address, L2ERC721Bridge.address)
-      await FakeL2StandardERC721.deployed()
+      await FakeOptimismMintableERC721.deployed()
 
       // Use the fake contract to mint Alice an NFT with the same token ID
-      const tx = await FakeL2StandardERC721.mint(aliceAddress, TOKEN_ID)
+      const tx = await FakeOptimismMintableERC721.mint(aliceAddress, TOKEN_ID)
       await tx.wait()
 
       // Check that Alice owns the NFT from the fake ERC721 contract
-      expect(await FakeL2StandardERC721.ownerOf(TOKEN_ID)).to.equal(
+      expect(await FakeOptimismMintableERC721.ownerOf(TOKEN_ID)).to.equal(
         aliceAddress
       )
 
       // Alice withdraws the NFT from the fake contract to L1, hoping to receive the legitimate L1 NFT.
       const withdrawalTx = await L2ERC721Bridge.connect(
         aliceWalletL2
-      ).withdrawERC721(
-        FakeL2StandardERC721.address,
+      ).bridgeERC721(
+        FakeOptimismMintableERC721.address,
+        L1ERC721.address,
         TOKEN_ID,
         0,
         NON_NULL_BYTES

packages/contracts-periphery/.depcheckrc

@@ -1,5 +1,6 @@
 ignores: [
   "@openzeppelin/contracts",
+  "@openzeppelin/contracts-upgradeable",
   "@rari-capital/solmate",
   "@types/node",
   "hardhat-deploy",