diff --git a/src/TokenDivider.sol b/src/TokenDivider.sol index 0705108..05d705c 100644 --- a/src/TokenDivider.sol +++ b/src/TokenDivider.sol @@ -105,6 +105,7 @@ contract TokenDivider is IERC721Receiver, Ownable { if (nftAddress == address(0)) revert TokenDivider__NftAddressIsZero(); if (amount == 0) revert TokenDivider__AmountCantBeZero(); // @audit: there is no checking of inExistence tokenId, it might handled on modifier but need to check + // @audit-report: dependency has its own check ERC20ToGenerateNftFraccion erc20Contract = new ERC20ToGenerateNftFraccion( string(abi.encodePacked(ERC721(nftAddress).name(), "Fraccion")), @@ -112,16 +113,22 @@ contract TokenDivider is IERC721Receiver, Ownable { ); // @audit: can we mint again? out of this function? + // @audit-finding: low + // @audit-report: yes it can be minted again. It should not be able to do so. + // @audit-report: but so far this contract only handle the recorded ERC20 transactions erc20Contract.mint(address(this), amount); address erc20 = address(erc20Contract); IERC721(nftAddress).safeTransferFrom(msg.sender, address(this), tokenId, ""); // @audit: what if we send the NFT to other address during this period? + // @audit-report: you cannot send the NFT to other address, related to unit testing test_failIf_sendNFTDuringDivide if (IERC721(nftAddress).ownerOf(tokenId) == msg.sender) revert TokenDivider__NftTransferFailed(); balances[msg.sender][erc20] = amount; // @audit: what happen if the given nft address already registered in this CA, then another token id register again? + // @audit-finding: high (critical) + // @audit-report: it overrides the previous divided NFT. The previously ERC20 tokens of divided NFT is no longer usable. nftToErc20Info[nftAddress] = ERC20Info({erc20Address: erc20, tokenId: tokenId}); erc20ToMintedAmount[erc20] = amount; erc20ToNft[erc20] = nftAddress; @@ -142,6 +149,7 @@ contract TokenDivider is IERC721Receiver, Ownable { * giving to the contract all the erc20 and it will give you back the nft */ function claimNft(address nftAddress) external { + // @audit: check with "counterfeit" token if (nftAddress == address(0)) { revert TokenDivider__NftAddressIsZero(); } @@ -175,6 +183,7 @@ contract TokenDivider is IERC721Receiver, Ownable { * @dev you can use this function to transfer nft franctions 100% securily and registered by te contract */ function transferErcTokens(address nftAddress, address to, uint256 amount) external { + // @audit: check with "counterfeit" token if (nftAddress == address(0)) { revert TokenDivider__NftAddressIsZero(); } @@ -215,6 +224,7 @@ contract TokenDivider is IERC721Receiver, Ownable { * to this contract, then a new order is created and published. */ function sellErc20(address nftPegged, uint256 price, uint256 amount) external { + // @audit: check with "counterfeit" token if (nftPegged == address(0)) { revert TokenDivider__NftAddressIsZero(); } @@ -252,6 +262,7 @@ contract TokenDivider is IERC721Receiver, Ownable { * if the transfer executed correctly, then this contract, wich has all the tokens, send the tokens to the msg.sender */ function buyOrder(uint256 orderIndex, address seller) external payable { + // @audit: check with "counterfeit" token if (seller == address(0)) { revert TokenDivider__InvalidSeller(); } diff --git a/test/unit/TokenDividerTest.audit.divideNft.t.sol b/test/unit/TokenDividerTest.audit.divideNft.t.sol index 54d50b8..0f8db23 100644 --- a/test/unit/TokenDividerTest.audit.divideNft.t.sol +++ b/test/unit/TokenDividerTest.audit.divideNft.t.sol @@ -32,8 +32,6 @@ contract TokenDiverTest is Test { } function test_assertSetUp() public view { - // then what? - // USER has the ERC721 NFT of TOKEN_ID assertTrue(address(erc721Mock.ownerOf(TOKEN_ID)) == address(USER)); @@ -41,44 +39,243 @@ contract TokenDiverTest is Test { assertTrue(address(erc721Mock.ownerOf(TOKEN_ID)) != address(USER2)); } - /* - function testDivideNft() public { - - vm.startPrank(USER); + function test_divideNftSuccessfully() public { + vm.startPrank(USER); + + // approve the NFT to receive the NFT in USER erc721Mock.approve(address(tokenDivider), TOKEN_ID); - tokenDivider.divideNft(address(erc721Mock), TOKEN_ID, AMOUNT); - vm.stopPrank(); - ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address); + tokenDivider.divideNft( + address(erc721Mock), + TOKEN_ID, + AMOUNT + ); + vm.stopPrank(); - console.log("ERC20 Token name is: ", erc20Mock.name()); - console.log("ERC20 Token symbol is: ", erc20Mock.symbol()); - assertEq(tokenDivider.getErc20TotalMintedAmount(address(erc20Mock)), AMOUNT); - assertEq(erc721Mock.ownerOf(TOKEN_ID), address(tokenDivider)); - assertEq(tokenDivider.getBalanceOf(USER, address(erc20Mock)), AMOUNT); + ERC20Mock erc20Mock = ERC20Mock( + tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address + ); + // assert if NFT already moved to tokenDivider + assertEq( + erc721Mock.ownerOf(TOKEN_ID), + address(tokenDivider) + ); + // assert if USER has the ERC20 token + assertEq( + tokenDivider.getBalanceOf(USER, address(erc20Mock)), + AMOUNT + ); } + function test_failIf_sendNFTDuringDivide() public { + vm.startPrank(USER); - modifier nftDivided() { - vm.startPrank(USER); - erc721Mock.approve(address(tokenDivider), TOKEN_ID); - tokenDivider.divideNft(address(erc721Mock), TOKEN_ID, AMOUNT); - vm.stopPrank(); - - _; - - } - - function testDivideNftFailsIsSenderIsNotNftOwner() public { - vm.prank(USER); + // approve the NFT to receive the NFT in USER erc721Mock.approve(address(tokenDivider), TOKEN_ID); - vm.startPrank(USER2); - vm.expectRevert(TokenDivider.TokenDivider__NotFromNftOwner.selector); - tokenDivider.divideNft(address(erc721Mock), TOKEN_ID, AMOUNT); - vm.stopPrank(); + // send the NFT to USER2 + erc721Mock.safeTransferFrom( + address(USER), + address(USER2), + TOKEN_ID + ); + + vm.expectRevert( + TokenDivider.TokenDivider__NotFromNftOwner.selector + ); + tokenDivider.divideNft( + address(erc721Mock), + TOKEN_ID, + AMOUNT + ); + + vm.stopPrank(); + } + + function test_sendNFTBeforeDivide() public { + vm.startPrank(USER); + + // send the NFT to USER2 + erc721Mock.safeTransferFrom( + address(USER), + address(USER2), + TOKEN_ID + ); + + vm.stopPrank(); + + vm.startPrank(USER2); + + // approve the NFT to receive the NFT in USER + erc721Mock.approve(address(tokenDivider), TOKEN_ID); + + tokenDivider.divideNft( + address(erc721Mock), + TOKEN_ID, + AMOUNT + ); + + vm.stopPrank(); + + ERC20Mock erc20Mock = ERC20Mock( + tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address + ); + + // assert if USER2 has the ERC20 token + assertEq( + tokenDivider.getBalanceOf(USER2, address(erc20Mock)), + AMOUNT + ); + } + + function test_failIf_nonExistenceNftTokenId() public { + vm.startPrank(USER); + + // approve the NFT to receive the NFT in USER + erc721Mock.approve(address(tokenDivider), TOKEN_ID); + + vm.expectRevert(); + tokenDivider.divideNft( + address(erc721Mock), + 404, + AMOUNT + ); + vm.stopPrank(); + } + + function test_failWhen_reMintTheDividedERC20Token() public { + vm.startPrank(USER); + + // approve the NFT to receive the NFT in USER + erc721Mock.approve(address(tokenDivider), TOKEN_ID); + + tokenDivider.divideNft( + address(erc721Mock), + TOKEN_ID, + AMOUNT + ); + vm.stopPrank(); + + ERC20Mock erc20Mock = ERC20Mock( + tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address + ); + + // assert if USER has the ERC20 token + assertEq( + tokenDivider.getBalanceOf(USER, address(erc20Mock)), + AMOUNT + ); + + vm.startPrank(USER2); + erc20Mock.mint(address(USER2), AMOUNT); + vm.stopPrank(); + + assertEq( + tokenDivider.getBalanceOf( + USER, + address(erc20Mock) + ), + AMOUNT + ); + assertEq( + tokenDivider.getBalanceOf( + USER2, + address(erc20Mock) + ), + erc20Mock.balanceOf(address(USER2)) + ); + assertEq( + erc20Mock.totalSupply(), + AMOUNT + ); + } + + function test_divideSameNFTAddressWithDifferentTokenId() public { + vm.startPrank(USER); + + // approve the NFT to receive the NFT in USER + erc721Mock.approve(address(tokenDivider), TOKEN_ID); + + tokenDivider.divideNft( + address(erc721Mock), + TOKEN_ID, + AMOUNT + ); + vm.stopPrank(); + + ERC20Mock erc20Mock = ERC20Mock( + tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address + ); + + // assert if NFT already moved to tokenDivider + assertEq( + erc721Mock.ownerOf(TOKEN_ID), + address(tokenDivider) + ); + // assert if USER has the ERC20 token + assertEq( + tokenDivider.getBalanceOf(USER, address(erc20Mock)), + AMOUNT + ); + + uint256 counterfeitTokenId = TOKEN_ID + 1; + erc721Mock.mint(USER2); + assertEq( + erc721Mock.ownerOf(counterfeitTokenId), + address(USER2) + ); + + // USER2 divide the counterfeit + vm.startPrank(USER2); + + // approve the NFT to receive the NFT in USER2 + erc721Mock.approve( + address(tokenDivider), + counterfeitTokenId + ); + + tokenDivider.divideNft( + address(erc721Mock), + counterfeitTokenId, + AMOUNT + ); + vm.stopPrank(); + + // now both of users (USER and USER2) has the same amount of registered ERC20 tokens despite it refer to different NFT Token Id + + ERC20Mock erc20MockCounterfeit = ERC20Mock( + tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address + ); + // assert if NFT already moved to tokenDivider + + assertEq( + erc721Mock.ownerOf(counterfeitTokenId), + address(tokenDivider) + ); + + // assert if previous NFT is still being hold by contract + assertEq( + erc721Mock.ownerOf(TOKEN_ID), + address(tokenDivider) + ); + + // assert if USER2 has the ERC20 token + assertEq( + tokenDivider.getBalanceOf( + USER2, + address(erc20MockCounterfeit) + ), + AMOUNT + ); + // assert if USER has the ERC20 token + assertEq( + tokenDivider.getBalanceOf( + USER, + address(erc20MockCounterfeit) + ), + AMOUNT + ); } - */ }