check ERC721 tokens have not already been bridged

snapshots/semver-lock.json

@@ -4,8 +4,8 @@
     "sourceCodeHash": "0x1f3e2c1d357ca4ab1973be2e6a49c67ccac1a325f3ca618dceff54ef47d20ea8"
   },
   "src/L1/L1ERC721Bridge.sol:L1ERC721Bridge": {
-    "initCodeHash": "0x4ab53d1f58ddf1d106f805ad4633cf76a493af324f62405d994e427f17165a53",
-    "sourceCodeHash": "0x052fa698c0526d026bc91ebc033d75160ce9f0e3aa395834c00707e2896a3c3c"
+    "initCodeHash": "0xbda36f7b524d142c331f1a159f2a75501ebc686d71d89e9e4fa22e233b161000",
+    "sourceCodeHash": "0x05e2c445079107dce5cdd29db88cfb02a1e64f833306c8c49ab720dece6663dd"
   },
   "src/L1/L1StandardBridge.sol:L1StandardBridge": {
     "initCodeHash": "0xb19625cd26ea7d8f79a19e292e409ab505e730e5273f2179eba8b2c6008480e8",

src/L1/L1ERC721Bridge.sol

@@ -32,8 +32,8 @@ contract L1ERC721Bridge is ERC721Bridge, ProxyAdminOwnedBase, ReinitializableBas
     address private spacer_50_0_20;
 
     /// @notice Semantic version.
-    /// @custom:semver 3.0.0
-    string public constant version = "3.0.0";
+    /// @custom:semver 3.0.1
+    string public constant version = "3.0.1";
 
     /// @notice Address of the SystemConfig contract.
     ISystemConfig public systemConfig;
@@ -131,6 +131,7 @@ contract L1ERC721Bridge is ERC721Bridge, ProxyAdminOwnedBase, ReinitializableBas
         );
 
         // Lock token into bridge
+        require(deposits[_localToken][_remoteToken][_tokenId] == false, "L1ERC721Bridge: Token ID is already escrowed");
         deposits[_localToken][_remoteToken][_tokenId] = true;
         IERC721(_localToken).transferFrom({ from: _from, to: address(this), tokenId: _tokenId });
 

test/L1/L1ERC721Bridge.t.sol

@@ -103,6 +103,23 @@ abstract contract L1ERC721Bridge_TestInit is CommonTest {
         assertFalse(l1ERC721Bridge.deposits(address(localToken), address(remoteToken), tokenId));
         assertEq(localToken.ownerOf(tokenId), _owner);
     }
+
+    /// @dev Bridges the token, then moves it out of the bridge back to alice while leaving the
+    ///      deposit flag set, as a non-standard token with forced transfers could.
+    function _bridgeAndReleaseToken() internal {
+        _bridgeToken();
+
+        vm.prank(address(l1ERC721Bridge));
+        localToken.transferFrom(address(l1ERC721Bridge), alice, tokenId);
+
+        vm.prank(alice);
+        localToken.approve(address(l1ERC721Bridge), tokenId);
+    }
+
+    function _assertDepositFlaggedButReleased() internal view {
+        assertTrue(l1ERC721Bridge.deposits(address(localToken), address(remoteToken), tokenId));
+        assertEq(localToken.ownerOf(tokenId), alice);
+    }
 }
 
 abstract contract L1ERC721Bridge_Bridge_TestInit is L1ERC721Bridge_TestInit {
@@ -422,6 +439,20 @@ contract L1ERC721Bridge_Uncategorized_Test is L1ERC721Bridge_Bridge_TestInit {
         _assertTokenNotEscrowed(alice);
     }
 
+    /// @notice Tests that the ERC721 bridge reverts when the token ID is already escrowed for the
+    ///         same L1/L2 token pair.
+    function test_bridgeERC721_alreadyEscrowed_reverts() external {
+        _bridgeAndReleaseToken();
+
+        vm.prank(alice, alice);
+        vm.expectRevert("L1ERC721Bridge: Token ID is already escrowed");
+        l1ERC721Bridge.bridgeERC721(
+            address(localToken), address(remoteToken), tokenId, DEFAULT_MIN_GAS_LIMIT, EXTRA_DATA
+        );
+
+        _assertDepositFlaggedButReleased();
+    }
+
     /// @notice Tests that the ERC721 bridge successfully sends a token to a different address than
     ///         the owner.
     function test_bridgeERC721To_succeeds() external {
@@ -468,6 +499,20 @@ contract L1ERC721Bridge_Uncategorized_Test is L1ERC721Bridge_Bridge_TestInit {
         _assertTokenNotEscrowed(alice);
     }
 
+    /// @notice Tests that the ERC721 bridge reverts when the token ID is already escrowed for the
+    ///         same L1/L2 token pair when sending to a different address than the owner.
+    function test_bridgeERC721To_alreadyEscrowed_reverts() external {
+        _bridgeAndReleaseToken();
+
+        vm.prank(alice);
+        vm.expectRevert("L1ERC721Bridge: Token ID is already escrowed");
+        l1ERC721Bridge.bridgeERC721To(
+            address(localToken), address(remoteToken), bob, tokenId, DEFAULT_MIN_GAS_LIMIT, EXTRA_DATA
+        );
+
+        _assertDepositFlaggedButReleased();
+    }
+
     /// @notice Tests that `bridgeERC721To` reverts if the to address is the zero address.
     function test_bridgeERC721To_toZeroAddress_reverts() external {
         vm.prank(bob);

test/L2/L2CrossDomainMessenger.t.sol

@@ -271,8 +271,8 @@ contract L2CrossDomainMessenger_Uncategorized_Test is L2CrossDomainMessenger_Tes
         skipIfForkTest("L2CrossDomainMessenger doesn't exist on L1 in forked test");
 
         // Using smaller uint so the fuzzer doesn't give as many massive values that get bounded.
-        // TODO: Known issue, messages above 34k can actually OOG on the receiving side if the
-        // target uses all available gas. Can be resolved by capping data sizes on the XDM or by
+        // TODO: Known issue, messages above 34k (13k with delegated account) can actually OOG on the receiving side if
+        // the target uses all available gas. Can be resolved by capping data sizes on the XDM or by
         // increasing the amount of available relay gas to ~100k. If increasing relay gas, should
         // have the relay gas only increase when the calldata size is large to avoid disrupting
         // in-flight L2 -> L1 messages.