From 7d438f1c44baaa2e63b645b45ef8c217c73f0847 Mon Sep 17 00:00:00 2001 From: han Date: Sun, 19 Jan 2025 17:22:25 +0700 Subject: [PATCH 1/2] finish up contract audit comments --- src/TokenDivider.sol | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/TokenDivider.sol b/src/TokenDivider.sol index 15dcfa0..f699f65 100644 --- a/src/TokenDivider.sol +++ b/src/TokenDivider.sol @@ -223,19 +223,24 @@ contract TokenDivider is IERC721Receiver, Ownable { revert TokenDivider__AmountCantBeZero(); } + // @audit: no check for 0 price? + ERC20Info memory tokenInfo = nftToErc20Info[nftPegged]; if (balances[msg.sender][tokenInfo.erc20Address] < amount) { revert TokenDivider__InsuficientBalance(); } - balances[msg.sender][tokenInfo.erc20Address] -= amount; + balances[msg.sender][tokenInfo.erc20Address] -= amount; // token amount reduced from its holder on sell order even tho no one buy it + // push sell order to array of sender s_userToSellOrders[msg.sender].push( SellOrder({seller: msg.sender, erc20Address: tokenInfo.erc20Address, price: price, amount: amount}) ); emit OrderPublished(amount, msg.sender, nftPegged); + // send erc20 token from owner to this contract + // @audit: what if the owner of ERC20 rejected this transaction? IERC20(tokenInfo.erc20Address).transferFrom(msg.sender, address(this), amount); } @@ -254,19 +259,24 @@ contract TokenDivider is IERC721Receiver, Ownable { SellOrder memory order = s_userToSellOrders[seller][orderIndex]; + // msg.value can be greater than order price? if (msg.value < order.price) { revert TokenDivider__IncorrectEtherAmount(); } + // if price 100, then fee is 1 uint256 fee = order.price / 100; + // if price 100, then seller fee is 0.5 uint256 sellerFee = fee / 2; if (msg.value < order.price + sellerFee) { revert TokenDivider__InsuficientEtherForFees(); } + // record erc20 amount on storage variable onchain balances[msg.sender][order.erc20Address] += order.amount; + // override the current orderIndex with tail, then pop the tail out of array s_userToSellOrders[seller][orderIndex] = s_userToSellOrders[seller][s_userToSellOrders[seller].length - 1]; s_userToSellOrders[seller].pop(); @@ -274,18 +284,23 @@ contract TokenDivider is IERC721Receiver, Ownable { // Transfer The Ether + // send ether to seller, reduced with seller fee (bool success,) = payable(order.seller).call{value: (order.price - sellerFee)}(""); if (!success) { revert TokenDivider__TransferFailed(); } + // send "fee" to "owner"? + // @audit: if buyer needs to pay price + seller fee. then seller is given price - seller fee. does fee is guarranted to be paid by buyer? + // @audit: who owner? (bool taxSuccess,) = payable(owner()).call{value: fee}(""); if (!taxSuccess) { revert TokenDivider__TransferFailed(); } + // send erc20 token to buyer address IERC20(order.erc20Address).transfer(msg.sender, order.amount); } From 1c7ff313226a1d2446c1130df168c74a5543c3c7 Mon Sep 17 00:00:00 2001 From: han Date: Sun, 19 Jan 2025 17:57:32 +0700 Subject: [PATCH 2/2] prep audit test for divideNft --- .../TokenDividerTest.audit.divideNft.t.sol | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 test/unit/TokenDividerTest.audit.divideNft.t.sol diff --git a/test/unit/TokenDividerTest.audit.divideNft.t.sol b/test/unit/TokenDividerTest.audit.divideNft.t.sol new file mode 100644 index 0000000..a03332e --- /dev/null +++ b/test/unit/TokenDividerTest.audit.divideNft.t.sol @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.18; + + +import {Test, console} from 'forge-std/Test.sol'; +import {DeployTokenDivider} from 'script/DeployTokenDivider.s.sol'; +import {TokenDivider} from 'src/TokenDivider.sol'; +import {ERC721Mock} from '../mocks/ERC721Mock.sol'; +import {ERC20Mock} from '@openzeppelin/contracts/mocks/token/ERC20Mock.sol'; +import {Ownable} from '@openzeppelin/contracts/access/Ownable.sol'; + + +contract TokenDiverTest is Test { + DeployTokenDivider deployer; + TokenDivider tokenDivider; + ERC721Mock erc721Mock; + + address public USER = makeAddr("user"); + address public USER2 = makeAddr("user2"); + uint256 constant public STARTING_USER_BALANCE = 10e18; + uint256 constant public AMOUNT = 2e18; + uint256 constant public TOKEN_ID = 0; + + function setUp() public { + deployer = new DeployTokenDivider(); + tokenDivider = deployer.run(); + + erc721Mock = new ERC721Mock(); + + erc721Mock.mint(USER); // @auditor: user has the NFT + vm.deal(USER2, STARTING_USER_BALANCE); // @auditor: user2 got 10 ether + } + + function test_assertSetUp() public { + // then what? + + // USER has no ERC721 NFT of TOKEN_ID + assertTrue(address(erc721Mock.ownerOf(TOKEN_ID)) == address(USER2)); + // USER has no balance + // USER2 has STARTING_USER_BALANCE + } + + /* + function testDivideNft() public { + + vm.startPrank(USER); + erc721Mock.approve(address(tokenDivider), TOKEN_ID); + + tokenDivider.divideNft(address(erc721Mock), TOKEN_ID, AMOUNT); + vm.stopPrank(); + ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address); + + 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); + + } + + + 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); + erc721Mock.approve(address(tokenDivider), TOKEN_ID); + + vm.startPrank(USER2); + vm.expectRevert(TokenDivider.TokenDivider__NotFromNftOwner.selector); + tokenDivider.divideNft(address(erc721Mock), TOKEN_ID, AMOUNT); + vm.stopPrank(); + } + */ + +}