From 5999e85e35e733789ce8842f89d11cd8ebf3b91b Mon Sep 17 00:00:00 2001 From: Roger Bai Date: Fri, 13 Nov 2026 09:24:47 -0500 Subject: [PATCH 1/2] check ERC721 tokens have not already been bridged --- snapshots/semver-lock.json | 4 +-- src/L1/L1ERC721Bridge.sol | 5 ++-- test/L1/L1ERC721Bridge.t.sol | 45 ++++++++++++++++++++++++++++ test/L2/L2CrossDomainMessenger.t.sol | 2 +- 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/snapshots/semver-lock.json b/snapshots/semver-lock.json index 5505ad2ef..b4702b3f4 100644 --- a/snapshots/semver-lock.json +++ b/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", diff --git a/src/L1/L1ERC721Bridge.sol b/src/L1/L1ERC721Bridge.sol index 32d8693a6..b945ab657 100644 --- a/src/L1/L1ERC721Bridge.sol +++ b/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 }); diff --git a/test/L1/L1ERC721Bridge.t.sol b/test/L1/L1ERC721Bridge.t.sol index 98ef265bf..fa1a98169 100644 --- a/test/L1/L1ERC721Bridge.t.sol +++ b/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); diff --git a/test/L2/L2CrossDomainMessenger.t.sol b/test/L2/L2CrossDomainMessenger.t.sol index 51625e764..c9bf9b1ec 100644 --- a/test/L2/L2CrossDomainMessenger.t.sol +++ b/test/L2/L2CrossDomainMessenger.t.sol @@ -271,7 +271,7 @@ 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 + // 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 From 3d548d43d24bfe6dc1c7745986e6b420efc1bbb3 Mon Sep 17 00:00:00 2001 From: Roger Bai Date: Fri, 13 Nov 2026 09:28:12 -0500 Subject: [PATCH 2/2] forge fmt --- test/L2/L2CrossDomainMessenger.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/L2/L2CrossDomainMessenger.t.sol b/test/L2/L2CrossDomainMessenger.t.sol index c9bf9b1ec..936e1f6a0 100644 --- a/test/L2/L2CrossDomainMessenger.t.sol +++ b/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 (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 + // 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.