Code423n4 #118, implement approvals for ERC721

RealityCards · Jun 21, 2021 · a628ac8 · a628ac8
1 parent 5cb3287
commit a628ac8
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 31 deletions.
diff --git a/contracts/nfthubs/RCNftHubL2.sol b/contracts/nfthubs/RCNftHubL2.sol
@@ -94,9 +94,9 @@ contract RCNftHubL2 is
             "ChildMintableERC721: TOKEN_EXISTS_ON_ROOT_CHAIN"
         );
         require(msgSender() == factoryAddress, "Not factory");
+        marketTracker[_tokenId] = _originalOwner;
         _mint(_originalOwner, _tokenId);
         _setTokenURI(_tokenId, _tokenURI);
-        marketTracker[_tokenId] = _originalOwner;
         return true;
     }
 
@@ -211,35 +211,25 @@ contract RCNftHubL2 is
       ║           OVERRIDES             ║
       ╚═════════════════════════════════╝*/
     /// @dev ensures NFTs can only be moved when market is resolved
-
-    function transferFrom(
+    function _beforeTokenTransfer(
         address from,
         address to,
         uint256 tokenId
-    ) public override {
-        executeTransfer(from, to, tokenId);
-    }
-
-    function safeTransferFrom(
-        address from,
-        address to,
-        uint256 tokenId,
-        bytes memory _data
-    ) public override {
-        executeTransfer(from, to, tokenId);
-        _data;
+    ) internal virtual override {
+        super._beforeTokenTransfer(from, to, tokenId);
+
+        if (
+            msgSender() != factoryAddress &&
+            msgSender() != marketTracker[tokenId]
+        ) {
+            IRCMarket market = IRCMarket(marketTracker[tokenId]);
+            require(
+                market.state() == IRCMarket.States.WITHDRAW,
+                "Incorrect state"
+            );
+        }
     }
 
-    function executeTransfer(
-        address from,
-        address to,
-        uint256 tokenId
-    ) internal {
-        IRCMarket market = IRCMarket(marketTracker[tokenId]);
-        require(market.state() == IRCMarket.States.WITHDRAW, "Incorrect state");
-        require(ownerOf(tokenId) == msgSender(), "Not owner");
-        _transfer(from, to, tokenId);
-    }
     /*
          ▲  
         ▲ ▲ 

diff --git a/test/OldTests/testsFundamentals.js b/test/OldTests/testsFundamentals.js
@@ -818,7 +818,7 @@ contract('TestFundamentals', (accounts) => {
     }
     // sum of all prices is 19 + 11 = 30
     await expectRevert(realitycards.rentAllCards(web3.utils.toWei('25', 'ether')), "Prices too high");
-    realitycards.rentAllCards(web3.utils.toWei('30', 'ether'));
+    realitycards.rentAllCards(web3.utils.toWei('35', 'ether'));
     // withdraw
     await time.increase(time.duration.minutes(10));
     await withdrawDeposit(1000, user0);

diff --git a/test/OldTests/testsRequireStatements.js b/test/OldTests/testsRequireStatements.js
@@ -288,18 +288,18 @@ contract('TestRequireStatements', (accounts) => {
     await time.increase(time.duration.years(1));
     await realitycards.lockMarket();
     // should fail cos LOCKED
-    await expectRevert(nftHubL2.transferFrom(user, user1, 2), "Incorrect state");
-    await expectRevert(nftHubL2.safeTransferFrom(user, user1, 2), "Incorrect state");
-    await expectRevert(nftHubL2.safeTransferFrom(user, user1, 2, web3.utils.asciiToHex("123456789")), "Incorrect state");
+    await expectRevert(nftHubL2.transferFrom(user, user1, 2), "ERC721: transfer caller is not owner nor approved");
+    await expectRevert(nftHubL2.safeTransferFrom(user, user1, 2), "ERC721: transfer caller is not owner nor approved");
+    await expectRevert(nftHubL2.safeTransferFrom(user, user1, 2, web3.utils.asciiToHex("123456789")), "ERC721: transfer caller is not owner nor approved");
     await realitio.setResult(realitycards.address, 2);
     await realitycards.getWinnerFromOracle();
     // await realitycards.determineWinner();
     await realitycards.claimCard(2, { from: user });
     // these shoudl all fail cos wrong owner:
     var owner = await realitycards.ownerOf(2);
     assert.equal(owner, user);
-    await expectRevert(nftHubL2.transferFrom(user, user1, 2, { from: user1 }), "Not owner");
-    await expectRevert(nftHubL2.safeTransferFrom(user1, user1, 2, { from: user1 }), "Not owner");
+    await expectRevert(nftHubL2.transferFrom(user, user1, 2, { from: user1 }), "ERC721: transfer caller is not owner nor approved");
+    await expectRevert(nftHubL2.safeTransferFrom(user1, user1, 2, { from: user1 }), "ERC721: transfer caller is not owner nor approved");
     // these should not
     await nftHubL2.transferFrom(user, user1, 2, { from: user });
     await nftHubL2.safeTransferFrom(user1, user, 2, { from: user1 });