Swivel v3 contest - simon135's results

code commented out it is best practice to remove commented out code

https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Swivel/Hash.sol#L27-L35 https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Swivel/Hash.sol#L38-L52

typos

instead of tranferring use : transferring https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Swivel/Interfaces.sol#L125 instead of ddress use : address https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Marketplace/Compounding.sol#L40

instead of a unlicensed spdx-License

use like mit one or which ever one is best your your project but you should have one as best practice https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Swivel/Sig.sol#L1 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Swivel/Interfaces.sol#L1 https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Swivel/Hash.sol#L1

no address 0 check

https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Marketplace/Compounding.sol#L61 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L48 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L36-L41 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L164

if address is not a contract then the getting the underlying token will revert.

c=can equal any address so maybe = tx.origan

    if (p == uint8(Protocols.Compound)) {

            return ICompoundToken(c).underlying();

https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Marketplace/Compounding.sol#L61

if 4626 erc standord token dosnt have correct or weird convertToAssets the function will revert or it will fail silenly

https://github.com/code-423n4/2022-07-swivel/blob/fbf94f87994d91dce75c605a1822ec6d6d7e9e74/Marketplace/Compounding.sol#L83

no emiting events after importent functions

https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L48 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L55 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L164 https://github.com/code-423n4/2022-07-swivel/blob/67c6900222cc4045d7fe2227a1ea73e0251374ed/Creator/ZcToken.sol#L139-L156

make a 2 step for making the new admin

because instead of giving the admin rights away the new admin should approve his role https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L48 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L55 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L164

make a or m variables into real words to make it make sence.

https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L48 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/Creator/Creator.sol#L56 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L164 https://github.com/code-423n4/2022-07-swivel/blob/67c6900222cc4045d7fe2227a1ea73e0251374ed/Creator/ZcToken.sol#L139-L156

instead of a : newadmin instead of m: newmarketplace instead of c: newmaturityRate https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L275 instead of o: user

make sure an admin cant get comprimed and increase the maturityRate causing the malious admin to gain funds

https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L164 https://github.com/code-423n4/2022-07-swivel/blob/67c6900222cc4045d7fe2227a1ea73e0251374ed/Creator/ZcToken.sol#L139-L156

missing natspec comments

this function is missing the @parm holder comment https://github.com/code-423n4/2022-07-swivel/blob/67c6900222cc4045d7fe2227a1ea73e0251374ed/Creator/ZcToken.sol#L98

instead of abi.encode use abi.encodepacked to save gas

because abi.encode padds up with zeros and abi.encodepacked packes.

instead of sloading the variable which costs 100 gas on warm ,make it memory (3 gas)

    uint256 maturityRatememory=maturityRate;
    yield = ((maturityRatememory * 1e26) / vlt.exchangeRate) - 1e26;
    

https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L70 change maturityRate to memory same thing with the if statement above make it into a memory variable when you start the function to save gas instead of sloads which costs more then mloads after you do that one sload to get the mstore then it costs less gas (200 gas saving each time ) https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L68 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L108-L110 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L142-L144 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L163 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L193-L195 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L212-L214 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L254-L257

make admin functions payable to save gas

because there is no check on msg.value =0 so it saves gas and you should use it only on admin functions https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L92 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L108-L110 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L142-L144 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L163 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L176 https://github.com/code-423n4/2022-07-swivel/blob/e64987b0f35f90c6578feb8e789c1fd53092fc7b/VaultTracker/VaultTracker.sol#L236

you can squaze in another 96bit variable at no cost

because an addres takes up 160 bites and there are 256 bit slot you can add 96 bit varaible for free

    struct Market {

        address cTokenAddr;
        uint96 
        address zcToken;
        uint96
        address vaultTracker;
        uint96 
        uint256 maturityRate;

    }

https://github.com/code-423n4/2022-07-swivel/blob/67c6900222cc4045d7fe2227a1ea73e0251374ed/Creator/IRedeemer.sol#L6-L11

allowance used multiple times in the function but allowance never changes when both are used

     uint256 allowed = allowance[holder][msg.sender];

            if (allowed >= previewAmount) {

                revert Approvals(allowed, previewAmount);

            }

            allowance[holder][msg.sender] -= previewAmount;

just use allowed -=previwAmount to save 100 gas on sload

https://github.com/code-423n4/2022-07-swivel/blob/67c6900222cc4045d7fe2227a1ea73e0251374ed/Creator/ZcToken.sol#L115 https://github.com/code-423n4/2022-07-swivel/blob/67c6900222cc4045d7fe2227a1ea73e0251374ed/Creator/ZcToken.sol#L115