fix: remove ERC721 token exit functionality with `TransferWithMetadat…

…a` event

Signed-off-by: Anjan Roy <hello@itzmeanjan.in>

contracts/root/TokenPredicates/ERC721Predicate.sol

@@ -1,6 +1,6 @@
 pragma solidity 0.6.6;
 
-import {IRootERC721} from "../RootToken/IRootERC721.sol";
+import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
 import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
 import {RLPReader} from "../../lib/RLPReader.sol";
 import {ITokenPredicate} from "./ITokenPredicate.sol";
@@ -15,8 +15,6 @@ contract ERC721Predicate is ITokenPredicate, AccessControlMixin, Initializable,
     bytes32 public constant TOKEN_TYPE = keccak256("ERC721");
     // keccak256("Transfer(address,address,uint256)")
     bytes32 public constant TRANSFER_EVENT_SIG = 0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef;
-    // keccak256("TransferWithMetadata(address,address,uint256,bytes)")
-    bytes32 public constant TRANSFER_WITH_METADATA_EVENT_SIG = 0xf94915c6d1fd521cee85359239227480c7e8776d7caf1fc3bacad5c269b66a14;
 
     // limit batching of tokens due to gas limit restrictions
     uint256 public constant BATCH_LIMIT = 20;
@@ -91,7 +89,7 @@ contract ERC721Predicate is ITokenPredicate, AccessControlMixin, Initializable,
         if (depositData.length == 32) {
             uint256 tokenId = abi.decode(depositData, (uint256));
             emit LockedERC721(depositor, depositReceiver, rootToken, tokenId);
-            IRootERC721(rootToken).safeTransferFrom(depositor, address(this), tokenId);
+            IERC721(rootToken).safeTransferFrom(depositor, address(this), tokenId);
 
         // deposit batch
         } else {
@@ -100,7 +98,7 @@ contract ERC721Predicate is ITokenPredicate, AccessControlMixin, Initializable,
             uint256 length = tokenIds.length;
             require(length <= BATCH_LIMIT, "ERC721Predicate: EXCEEDS_BATCH_LIMIT");
             for (uint256 i; i < length; i++) {
-                IRootERC721(rootToken).safeTransferFrom(depositor, address(this), tokenIds[i]);
+                IERC721(rootToken).safeTransferFrom(depositor, address(this), tokenIds[i]);
             }
         }
     }
@@ -125,49 +123,21 @@ contract ERC721Predicate is ITokenPredicate, AccessControlMixin, Initializable,
         RLPReader.RLPItem[] memory logTopicRLPList = logRLPList[1].toList(); // topics
         address withdrawer = address(logTopicRLPList[1].toUint()); // topic1 is from address
 
-        if (bytes32(logTopicRLPList[0].toUint()) == TRANSFER_EVENT_SIG) { // topic0 is event sig
-            require(
-                address(logTopicRLPList[2].toUint()) == address(0), // topic2 is to address
-                "ERC721Predicate: INVALID_RECEIVER"
-            );
+        require(bytes32(logTopicRLPList[0].toUint()) == TRANSFER_EVENT_SIG, "ERC721Predicate: INVALID_SIGNATURE");
 
-            uint256 tokenId = logTopicRLPList[3].toUint(); // topic3 is tokenId field
+        require(
+            address(logTopicRLPList[2].toUint()) == address(0), // topic2 is to address
+            "ERC721Predicate: INVALID_RECEIVER"
+        );
 
-            IRootERC721(rootToken).safeTransferFrom(
-                address(this),
-                withdrawer,
-                tokenId
-            );
+        uint256 tokenId = logTopicRLPList[3].toUint(); // topic3 is tokenId field
 
-            emit ExitedERC721(withdrawer, rootToken, tokenId);
+        IERC721(rootToken).safeTransferFrom(
+            address(this),
+            withdrawer,
+            tokenId
+        );
 
-        } else if (bytes32(logTopicRLPList[0].toUint()) == TRANSFER_WITH_METADATA_EVENT_SIG) { 
-            // If this is when NFT exit is done with arbitrary metadata on L2
-
-            require(
-                address(logTopicRLPList[2].toUint()) == address(0), // topic2 is to address
-                "ERC721Predicate: INVALID_RECEIVER"
-            );
-
-            IRootERC721 token = IRootERC721(rootToken);
-            uint256 tokenId = logTopicRLPList[3].toUint(); // topic3 is tokenId field
-
-            token.safeTransferFrom(address(this), withdrawer, tokenId);
-            // This function will be invoked for passing arbitrary
-            // metadata, obtained from event emitted in L2, to
-            // L1 ERC721, so that it can decode & do further processing
-            //
-            // @note Make sure you've implemented this method
-            // if you're interested in exiting with metadata
-            bytes memory logData = logRLPList[2].toBytes();
-            bytes memory metaData = abi.decode(logData, (bytes));
-
-            token.setTokenMetadata(tokenId, metaData);
-
-            emit ExitedERC721(withdrawer, rootToken, tokenId);
-
-        } else {
-            revert("ERC721Predicate: INVALID_SIGNATURE");
-        }
+        emit ExitedERC721(withdrawer, rootToken, tokenId);
     }
 }

test/predicates/ERC721Predicate.test.js

@@ -336,7 +336,7 @@ contract('ERC721Predicate', (accounts) => {
     it('exitTokens should pass with another Transfer event', async () => {
       const burnLog = getERC721TransferLog({ from: withdrawer, to: mockValues.zeroAddress, tokenId: tokenIdB })
       exitTokensTxB = await erc721Predicate.exitTokens(withdrawer, dummyERC721.address, burnLog)
-      should.exist(exitTokensTxA)
+      should.exist(exitTokensTxB)
     })
 
     it('tokenIdB should be transferred to withdrawer', async () => {
@@ -345,8 +345,7 @@ contract('ERC721Predicate', (accounts) => {
     })
   })
 
-  describe('exitTokens with `TransferWithMetadata` event signature', () => {
-
+  describe('exitTokens failing with TransferWithMetadata, while passing with Transfer', () => {
     const tokenId = mockValues.numbers[5]
     const withdrawer = mockValues.addresses[8]
     const metaData = 'https://nft.matic.network'
@@ -375,14 +374,19 @@ contract('ERC721Predicate', (accounts) => {
       owner.should.equal(erc721Predicate.address)
     })
 
-    it('Should be able to receive exitTokens tx', async () => {
+    it('exitTokens should revert with TransferWithMetadata event', async () => {
       const burnLog = getERC721TransferWithMetadataLog({
         from: withdrawer,
         to: mockValues.zeroAddress,
         tokenId,
         metaData
       })
 
+      await expectRevert(erc721Predicate.exitTokens(withdrawer, dummyERC721.address, burnLog), 'ERC721Predicate: INVALID_SIGNATURE')
+    })
+
+    it('exitTokens should pass with Transfer event', async () => {
+      const burnLog = getERC721TransferLog({ from: withdrawer, to: mockValues.zeroAddress, tokenId: tokenId })
       exitTokensTx = await erc721Predicate.exitTokens(withdrawer, dummyERC721.address, burnLog)
       should.exist(exitTokensTx)
     })
@@ -418,12 +422,6 @@ contract('ERC721Predicate', (accounts) => {
       const owner = await dummyERC721.ownerOf(tokenId)
       owner.should.equal(withdrawer)
     })
-
-    it('Token URI should match with transferred metadata', async () => {
-      const _metaData = await dummyERC721.tokenURI(tokenId)
-      _metaData.should.equal(metaData)
-    })
-
   })
 
   describe('exitTokens called by different user', () => {

test/root/Withdraw.test.js

@@ -1072,7 +1072,7 @@ contract('RootChainManager', async(accounts) => {
       owner.should.equal(user)
     })
 
-    it('User should be able to exit tokenId2 with Transfer', async() => {
+    it('User should be able to exit tokenId3 with Transfer', async() => {
       const logIndices = []
       withdrawTx.receipt.rawLogs.forEach((e, i) => {
         if (e.topics[0].toLowerCase() === ERC721_TRANSFER_EVENT_SIG.toLowerCase()) {