fix(ctp): bugs in Mintable ERC721 (#2864)

Fixes two minor bugs in the Optimism Mintable ERC721 implementation.
Includes a new required input (remoteChainId) and fixes two issues with
the generated baseURI.
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

.changeset/strange-pets-draw.md

+---
+'@eth-optimism/contracts-periphery': minor
+---
+
+Fixes a bug in the OptimismMintableERC721. Requires an interface change, so this is a minor and not patch.

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

 /* Imports: External */
 import { Contract, ContractFactory, utils, Wallet } from 'ethers'
 import { ethers } from 'hardhat'
+import { getChainId } from '@eth-optimism/core-utils'
 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'
@@ -99,7 +100,8 @@ describe('ERC721 Bridge', () => {
 
     OptimismMintableERC721Factory =
       await Factory__OptimismMintableERC721Factory.deploy(
-        L2ERC721Bridge.address
+        L2ERC721Bridge.address,
+        await getChainId(env.l1Wallet.provider)
       )
     await OptimismMintableERC721Factory.deployed()
 
@@ -268,7 +270,10 @@ describe('ERC721 Bridge', () => {
       // - Returns the address of the legitimate L1 token from its l1Token() getter.
       // - Allows the L2 bridge to call its burn() function.
       const FakeOptimismMintableERC721 = await (
-        await ethers.getContractFactory('FakeOptimismMintableERC721', bobWalletL2)
+        await ethers.getContractFactory(
+          'FakeOptimismMintableERC721',
+          bobWalletL2
+        )
       ).deploy(L1ERC721.address, L2ERC721Bridge.address)
       await FakeOptimismMintableERC721.deployed()
 

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

@@ -25,6 +25,11 @@ interface IOptimismMintableERC721 is IERC721 {
      */
     event Burn(address indexed account, uint256 tokenId);
 
+    /**
+     * @notice Chain ID of the chain where the remote token is deployed.
+     */
+    function remoteChainId() external returns (uint256);
+
     /**
      * @notice Address of the token on the remote domain.
      */

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

@@ -16,12 +16,17 @@ contract OptimismMintableERC721 is ERC721, IOptimismMintableERC721 {
     /**
      * @inheritdoc IOptimismMintableERC721
      */
-    address public remoteToken;
+    uint256 public immutable remoteChainId;
 
     /**
      * @inheritdoc IOptimismMintableERC721
      */
-    address public bridge;
+    address public immutable remoteToken;
+
+    /**
+     * @inheritdoc IOptimismMintableERC721
+     */
+    address public immutable bridge;
 
     /**
      * @notice Base token URI for this token.
@@ -30,16 +35,19 @@ contract OptimismMintableERC721 is ERC721, IOptimismMintableERC721 {
 
     /**
      * @param _bridge        Address of the bridge on this network.
+     * @param _remoteChainId Chain ID where the remote token is deployed.
      * @param _remoteToken   Address of the corresponding token on the other network.
      * @param _name          ERC721 name.
      * @param _symbol        ERC721 symbol.
      */
     constructor(
         address _bridge,
+        uint256 _remoteChainId,
         address _remoteToken,
         string memory _name,
         string memory _symbol
     ) ERC721(_name, _symbol) {
+        remoteChainId = _remoteChainId;
         remoteToken = _remoteToken;
         bridge = _bridge;
 
@@ -48,9 +56,9 @@ contract OptimismMintableERC721 is ERC721, IOptimismMintableERC721 {
         baseTokenURI = string(
             abi.encodePacked(
                 "ethereum:",
-                Strings.toHexString(uint160(_remoteToken)),
+                Strings.toHexString(uint160(_remoteToken), 20),
                 "@",
-                Strings.toString(block.chainid),
+                Strings.toString(_remoteChainId),
                 "/tokenURI?uint256="
             )
         );

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

@@ -14,7 +14,7 @@ contract OptimismMintableERC721Factory is OwnableUpgradeable {
     /**
      * @notice Contract version number.
      */
-    uint8 public constant VERSION = 1;
+    uint8 public constant VERSION = 2;
 
     /**
      * @notice Emitted whenever a new OptimismMintableERC721 contract is created.
@@ -29,6 +29,11 @@ contract OptimismMintableERC721Factory is OwnableUpgradeable {
      */
     address public bridge;
 
+    /**
+     * @notice Chain ID for the remote network.
+     */
+    uint256 public remoteChainId;
+
     /**
      * @notice Tracks addresses created by this factory.
      */
@@ -37,8 +42,8 @@ contract OptimismMintableERC721Factory is OwnableUpgradeable {
     /**
      * @param _bridge Address of the ERC721 bridge on this network.
      */
-    constructor(address _bridge) {
-        initialize(_bridge);
+    constructor(address _bridge, uint256 _remoteChainId) {
+        initialize(_bridge, _remoteChainId);
     }
 
     /**
@@ -46,8 +51,9 @@ contract OptimismMintableERC721Factory is OwnableUpgradeable {
      *
      * @param _bridge Address of the ERC721 bridge on this network.
      */
-    function initialize(address _bridge) public reinitializer(VERSION) {
+    function initialize(address _bridge, uint256 _remoteChainId) public reinitializer(VERSION) {
         bridge = _bridge;
+        remoteChainId = _remoteChainId;
 
         // Initialize upgradable OZ contracts
         __Ownable_init();
@@ -69,6 +75,7 @@ contract OptimismMintableERC721Factory is OwnableUpgradeable {
             _remoteToken != address(0),
             "OptimismMintableERC721Factory: L1 token address cannot be address(0)"
         );
+
         require(
             bridge != address(0),
             "OptimismMintableERC721Factory: bridge address must be initialized"
@@ -76,6 +83,7 @@ contract OptimismMintableERC721Factory is OwnableUpgradeable {
 
         OptimismMintableERC721 localToken = new OptimismMintableERC721(
             bridge,
+            remoteChainId,
             _remoteToken,
             _name,
             _symbol

packages/contracts-periphery/deploy/nft-bridge/OptimismMintableERC721FactoryImplementation.ts

@@ -7,7 +7,7 @@ const deployFn: DeployFunction = async (hre) => {
 
   await hre.deployments.deploy('OptimismMintableERC721Factory', {
     from: deployer,
-    args: [ethers.constants.AddressZero],
+    args: [ethers.constants.AddressZero, 0],
     log: true,
   })
 }

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

@@ -60,6 +60,7 @@ describe('L2ERC721Bridge', () => {
       await ethers.getContractFactory('OptimismMintableERC721')
     ).deploy(
       L2ERC721Bridge.address,
+      100,
       DUMMY_L1ERC721_ADDRESS,
       'L2Token',
       'L2T',
@@ -225,6 +226,7 @@ describe('L2ERC721Bridge', () => {
         await smock.mock('OptimismMintableERC721')
       ).deploy(
         L2ERC721Bridge.address,
+        100,
         DUMMY_L1ERC721_ADDRESS,
         'L2Token',
         'L2T',

packages/contracts-periphery/test/contracts/universal/OptimismMintableERC721.spec.ts

@@ -8,7 +8,7 @@ import { expect } from '../../setup'
 
 const TOKEN_ID = 10
 const DUMMY_L1ERC721_ADDRESS: string =
-  '0x2234223412342234223422342234223422342234'
+  '0x0034223412342234223422342234223422342234'
 
 describe('OptimismMintableERC721', () => {
   let l2BridgeImpersonator: Signer
@@ -18,19 +18,18 @@ describe('OptimismMintableERC721', () => {
   let l2BridgeImpersonatorAddress: string
   let aliceAddress: string
   let baseUri: string
-  let chainId: number
+  const remoteChainId = 100
 
   before(async () => {
     ;[l2BridgeImpersonator, alice] = await ethers.getSigners()
     l2BridgeImpersonatorAddress = await l2BridgeImpersonator.getAddress()
     aliceAddress = await alice.getAddress()
 
-    chainId = await alice.getChainId()
     baseUri = ''.concat(
       'ethereum:',
       DUMMY_L1ERC721_ADDRESS,
       '@',
-      chainId.toString(),
+      remoteChainId.toString(),
       '/tokenURI?uint256='
     )
 
@@ -38,6 +37,7 @@ describe('OptimismMintableERC721', () => {
       await ethers.getContractFactory('OptimismMintableERC721')
     ).deploy(
       l2BridgeImpersonatorAddress,
+      remoteChainId,
       DUMMY_L1ERC721_ADDRESS,
       'L2ERC721',
       'ERC',
@@ -101,8 +101,8 @@ describe('OptimismMintableERC721', () => {
       expect(await OptimismMintableERC721.supportsInterface(0x01ffc9a7)).to.be
         .true
 
-      // OptimismMintablERC721
-      expect(await OptimismMintableERC721.supportsInterface(0xec4fc8e3)).to.be
+      // OptimismMintableERC721
+      expect(await OptimismMintableERC721.supportsInterface(0x051e4975)).to.be
         .true
 
       // ERC721

packages/contracts-periphery/test/contracts/universal/OptimismMintableERC721Factory.spec.ts

@@ -20,7 +20,7 @@ describe('OptimismMintableERC721Factory', () => {
   let L1ERC721: MockContract<Contract>
   let OptimismMintableERC721Factory: Contract
   let baseURI: string
-  let chainId: number
+  const remoteChainId = 100
 
   beforeEach(async () => {
     ;[signer] = await ethers.getSigners()
@@ -33,14 +33,13 @@ describe('OptimismMintableERC721Factory', () => {
 
     OptimismMintableERC721Factory = await (
       await ethers.getContractFactory('OptimismMintableERC721Factory')
-    ).deploy(DUMMY_L2_BRIDGE_ADDRESS)
+    ).deploy(DUMMY_L2_BRIDGE_ADDRESS, remoteChainId)
 
-    chainId = await signer.getChainId()
     baseURI = ''.concat(
       'ethereum:',
       L1ERC721.address.toLowerCase(),
       '@',
-      chainId.toString(),
+      remoteChainId.toString(),
       '/tokenURI?uint256='
     )
   })