prep audit test for divideNft

.github/workflows/test.yml

@@ -1,45 +0,0 @@
-name: CI
-
-on:
-  push:
-  pull_request:
-  workflow_dispatch:
-
-env:
-  FOUNDRY_PROFILE: ci
-
-jobs:
-  check:
-    strategy:
-      fail-fast: true
-
-    name: Foundry project
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v4
-        with:
-          submodules: recursive
-
-      - name: Install Foundry
-        uses: foundry-rs/foundry-toolchain@v1
-        with:
-          version: nightly
-
-      - name: Show Forge version
-        run: |
-          forge --version
-
-      - name: Run Forge fmt
-        run: |
-          forge fmt --check
-        id: fmt
-
-      - name: Run Forge build
-        run: |
-          forge build --sizes
-        id: build
-
-      - name: Run Forge tests
-        run: |
-          forge test -vvv
-        id: test

.gitignore

@@ -12,3 +12,6 @@ docs/
 
 # Dotenv file
 .env
+
+# Vim buffer file
+*.swp

.gitmodules

@@ -3,7 +3,7 @@
 	url = https://github.com/foundry-rs/forge-std
 [submodule "lib/openzeppelin-contracts"]
 	path = lib/openzeppelin-contracts
-	url = https://github.com/openzeppelin/openzeppelin-contracts
+	url = https://github.com/OpenZeppelin/openzeppelin-contracts
 [submodule "lib/foundry-devops"]
 	path = lib/foundry-devops
 	url = https://github.com/Cyfrin/foundry-devops

src/TokenDivider.sol

@@ -16,12 +16,9 @@ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
  * as possible.This is the first project that i code alone, in blockchain, foundry and solidity.
  * Thank you so much for read it.
  */
- 
-
-contract  TokenDivider is IERC721Receiver,Ownable   {
-
+contract TokenDivider is IERC721Receiver, Ownable {
     error TokenDivider__NotFromNftOwner();
-    error  TokenDivider__NotEnoughErc20Balance();
+    error TokenDivider__NotEnoughErc20Balance();
     error TokenDivider__NftTransferFailed();
     error TokenDivider__InsuficientBalance();
     error TokenDivider__CantTransferToAddressZero();
@@ -45,20 +42,18 @@ contract  TokenDivider is IERC721Receiver,Ownable   {
         uint256 amount;
     }
 
-
     /**
      * @dev balances Relates a user with an amount of a erc20 token, this erc20 tokens is an nft fraction
-     
-     @dev nftToErc20Info Relates an nft with the erc20 pegged, and othe data like the erc20 amount, or the tokenId
-     
-     @dev s_userToSellOrders Relates a user with an array of sell orders, that each sell order 
-     has a seller, an erc20 that is the token to sell, a price and an amount of erc20 to sell
-     
+     *
+     *  @dev nftToErc20Info Relates an nft with the erc20 pegged, and othe data like the erc20 amount, or the tokenId
+     *
+     *  @dev s_userToSellOrders Relates a user with an array of sell orders, that each sell order 
+     *  has a seller, an erc20 that is the token to sell, a price and an amount of erc20 to sell
      */
     mapping(address user => mapping(address erc20Address => uint256 amount)) balances;
     mapping(address nft => ERC20Info) nftToErc20Info;
     mapping(address user => SellOrder[] orders) s_userToSellOrders;
-    mapping(address erc20 =>  address nft) erc20ToNft;
+    mapping(address erc20 => address nft) erc20ToNft;
     mapping(address erc20 => uint256 totalErc20Minted) erc20ToMintedAmount;
 
     event NftDivided(address indexed nftAddress, uint256 indexed amountErc20Minted, address indexed erc20Minted);
@@ -67,13 +62,12 @@ contract  TokenDivider is IERC721Receiver,Ownable   {
     event OrderPublished(uint256 indexed amount, address indexed seller, address indexed nftPegged);
     event OrderSelled(address indexed buyer, uint256 price);
 
-
     /**
      *
      *  Only the owner of the nft can call a function with this modifier
      */
     modifier onlyNftOwner(address nft, uint256 tokenId) {
-        if(msg.sender != IERC721(nft).ownerOf(tokenId)) {
+        if (msg.sender != IERC721(nft).ownerOf(tokenId)) {
             revert TokenDivider__NotFromNftOwner();
         }
         _;
@@ -81,15 +75,13 @@ contract  TokenDivider is IERC721Receiver,Ownable   {
 
     constructor() Ownable(msg.sender) {}
 
-
-
     /**
      * @dev Handles the receipt of an ERC721 token. This function is called whenever an ERC721 token is transferred to this contract.
      */
     function onERC721Received(
-        address /* operator */,
-        address /* from */,
-        uint256 /*  tokenId */,
+        address, /* operator */
+        address, /* from */
+        uint256, /*  tokenId */
         bytes calldata /* data */
     ) external pure override returns (bytes4) {
         // Return this value to confirm the receipt of the NFT
@@ -105,25 +97,31 @@ contract  TokenDivider is IERC721Receiver,Ownable   {
      * @dev in this function, the nft passed as parameter, is locked by transfering it to this contract, then, it gives to the
      * person calling this function an amount of erc20, beeing like a fraction of this nft.
      */
-
-    function divideNft(address nftAddress, uint256 tokenId, uint256 amount) onlyNftOwner(nftAddress, tokenId) onlyNftOwner(nftAddress ,tokenId) external {
-        
-
-        if(nftAddress == address(0)) { revert TokenDivider__NftAddressIsZero(); }
-        if(amount == 0) { revert TokenDivider__AmountCantBeZero(); }
+    function divideNft(address nftAddress, uint256 tokenId, uint256 amount)
+        external
+        onlyNftOwner(nftAddress, tokenId)
+        onlyNftOwner(nftAddress, tokenId)
+    {
+        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
 
         ERC20ToGenerateNftFraccion erc20Contract = new ERC20ToGenerateNftFraccion(
             string(abi.encodePacked(ERC721(nftAddress).name(), "Fraccion")),
-            string(abi.encodePacked("F", ERC721(nftAddress).symbol())));
+            string(abi.encodePacked("F", ERC721(nftAddress).symbol()))
+        );
 
+	// @audit: can we mint again? out of this function?
         erc20Contract.mint(address(this), amount);
         address erc20 = address(erc20Contract);
 
         IERC721(nftAddress).safeTransferFrom(msg.sender, address(this), tokenId, "");
 
-        if(IERC721(nftAddress).ownerOf(tokenId) == msg.sender) { revert TokenDivider__NftTransferFailed(); }
+	// @audit: what if we send the NFT to other address during this period?
+        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?
         nftToErc20Info[nftAddress] = ERC20Info({erc20Address: erc20, tokenId: tokenId});
         erc20ToMintedAmount[erc20] = amount;
         erc20ToNft[erc20] = nftAddress;
@@ -131,7 +129,7 @@ contract  TokenDivider is IERC721Receiver,Ownable   {
         emit NftDivided(nftAddress, amount, erc20);
 
         bool transferSuccess = IERC20(erc20).transfer(msg.sender, amount);
-        if(!transferSuccess) {
+        if (!transferSuccess) {
             revert TokenDivider__TransferFailed();
         }
     }
@@ -143,30 +141,31 @@ contract  TokenDivider is IERC721Receiver,Ownable   {
      * @dev in this function, if you have all the erc20 minted for the nft, you can call this function to claim the nft,
      * giving to the contract all the erc20 and it will give you back the nft
      */
-
     function claimNft(address nftAddress) external {
-
-        if(nftAddress == address(0)) {
+        if (nftAddress == address(0)) {
             revert TokenDivider__NftAddressIsZero();
         }
 
+	// @audit: what if the nft address is not stored?
         ERC20Info storage tokenInfo = nftToErc20Info[nftAddress];
 
-        if(balances[msg.sender][tokenInfo.erc20Address] < erc20ToMintedAmount[tokenInfo.erc20Address]) {
+        if (balances[msg.sender][tokenInfo.erc20Address] < erc20ToMintedAmount[tokenInfo.erc20Address]) {
             revert TokenDivider__NotEnoughErc20Balance();
         }
 
-        ERC20ToGenerateNftFraccion(tokenInfo.erc20Address).burnFrom(msg.sender, erc20ToMintedAmount[tokenInfo.erc20Address]);
+        ERC20ToGenerateNftFraccion(tokenInfo.erc20Address).burnFrom(
+            msg.sender, erc20ToMintedAmount[tokenInfo.erc20Address]
+        );
 
         balances[msg.sender][tokenInfo.erc20Address] = 0;
         erc20ToMintedAmount[tokenInfo.erc20Address] = 0;
 
         emit NftClaimed(nftAddress);
 
+	// @audit: what happen if there is overlaps in same NFT address with different token id? will someone get a different NFT?
         IERC721(nftAddress).safeTransferFrom(address(this), msg.sender, tokenInfo.tokenId);
     }
 
-
     /**
      *
      * @param nftAddress The nft address pegged to the erc20
@@ -175,27 +174,25 @@ 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 {
-        
-        if(nftAddress == address(0)) {
+    function transferErcTokens(address nftAddress, address to, uint256 amount) external {
+        if (nftAddress == address(0)) {
             revert TokenDivider__NftAddressIsZero();
         }
 
-        if(to == address(0)) {
+        if (to == address(0)) {
             revert TokenDivider__CantTransferToAddressZero();
         }
 
-        if(amount == 0) {
+        if (amount == 0) {
             revert TokenDivider__AmountCantBeZero();
         }
 
         ERC20Info memory tokenInfo = nftToErc20Info[nftAddress];
 
-        if(to == address(0)) {
+        if (to == address(0)) {
             revert TokenDivider__CantTransferToAddressZero();
         }
-        if(balances[msg.sender][tokenInfo.erc20Address] < amount) {
+        if (balances[msg.sender][tokenInfo.erc20Address] < amount) {
             revert TokenDivider__NotEnoughErc20Balance();
         }
 
@@ -204,7 +201,7 @@ contract  TokenDivider is IERC721Receiver,Ownable   {
 
         emit TokensTransfered(amount, tokenInfo.erc20Address);
 
-        IERC20(tokenInfo.erc20Address).transferFrom(msg.sender,to, amount);
+        IERC20(tokenInfo.erc20Address).transferFrom(msg.sender, to, amount);
     }
 
     /**
@@ -217,38 +214,36 @@ contract  TokenDivider is IERC721Receiver,Ownable   {
      * firstly, once you call this function, the amount of tokens that you passed into as a parameter, get blocked,  by sending it
      * to this contract, then a new order is created and published.
      */
-
-    function sellErc20(address nftPegged, uint256 price,uint256 amount) external {
-        if(nftPegged == address(0)) {
+    function sellErc20(address nftPegged, uint256 price, uint256 amount) external {
+        if (nftPegged == address(0)) {
             revert TokenDivider__NftAddressIsZero();
         }
 
-        if( amount == 0) {
+        if (amount == 0) {
             revert TokenDivider__AmountCantBeZero();
         }
 
+	// @audit: no check for 0 price?
+
         ERC20Info memory tokenInfo = nftToErc20Info[nftPegged];
-        if(balances[msg.sender][tokenInfo.erc20Address] < amount) {
+        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
-            })
+            SellOrder({seller: msg.sender, erc20Address: tokenInfo.erc20Address, price: price, amount: amount})
         );
 
-        emit OrderPublished(amount,msg.sender, nftPegged);
+        emit OrderPublished(amount, msg.sender, nftPegged);
 
-        IERC20(tokenInfo.erc20Address).transferFrom(msg.sender,address(this), amount);
+	// 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);
     }
 
-
     /**
      *
      * @param orderIndex The index of the order in all the orders array of the seller (the seller can have multiple orders active)
@@ -257,31 +252,31 @@ contract  TokenDivider is IERC721Receiver,Ownable   {
      * @dev when the buyer call this function, the eth or any token accepted to pay, is sent to the seller
      * 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 {
-        if(seller == address(0)) {
+        if (seller == address(0)) {
             revert TokenDivider__InvalidSeller();
         }
 
-        
-        
         SellOrder memory order = s_userToSellOrders[seller][orderIndex];
 
-        if(msg.value < order.price) {
+	// 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) {
+        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();
 
@@ -289,39 +284,42 @@ contract  TokenDivider is IERC721Receiver,Ownable   {
 
         // Transfer The Ether
 
-        (bool success, ) = payable(order.seller).call{value: (order.price - sellerFee)}("");
+	// send ether to seller, reduced with seller fee
+        (bool success,) = payable(order.seller).call{value: (order.price - sellerFee)}("");
 
-        if(!success) {
+        if (!success) {
             revert TokenDivider__TransferFailed();
         }
 
-        (bool taxSuccess, ) = payable(owner()).call{value: fee}("");
+	// 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) {
+        if (!taxSuccess) {
             revert TokenDivider__TransferFailed();
         }
 
+	// send erc20 token to buyer address
         IERC20(order.erc20Address).transfer(msg.sender, order.amount);
-
     }
 
-    /** Getters */
-
-    function getBalanceOf(address user, address token) public view returns(uint256) {
+    /**
+     * Getters
+     */
+    function getBalanceOf(address user, address token) public view returns (uint256) {
         return balances[user][token];
     }
 
-    function getErc20TotalMintedAmount(address erc20) public view returns(uint256) {
+    function getErc20TotalMintedAmount(address erc20) public view returns (uint256) {
         return erc20ToMintedAmount[erc20];
     }
 
-    function getErc20InfoFromNft(address nft) public view returns(ERC20Info memory) {
+    function getErc20InfoFromNft(address nft) public view returns (ERC20Info memory) {
         return nftToErc20Info[nft];
     }
 
-    function getOrderPrice(address seller, uint256 index) public view returns(uint256 price) {
-        price =  s_userToSellOrders[seller][index].price;
+    function getOrderPrice(address seller, uint256 index) public view returns (uint256 price) {
+        price = s_userToSellOrders[seller][index].price;
     }
-
 }

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();
+    }
+     */
+
+}

test/unit/TokenDividerTest.t.sol

@@ -27,8 +27,8 @@ contract TokenDiverTest is Test {
         
         erc721Mock = new ERC721Mock();
         
-        erc721Mock.mint(USER);
-        vm.deal(USER2, STARTING_USER_BALANCE);
+        erc721Mock.mint(USER); // @auditor: user has the NFT
+        vm.deal(USER2, STARTING_USER_BALANCE); // @auditor: user2 got 10 ether
     }
 
     function testDivideNft() public {