Fractional v2 contest - sashik_eth's results

L01 - Missing input validation of array lengths

Function install fails to perform input validation on arrays to verify the lengths match. A mismatch could lead to an exception or undefined behavior.

src/Vault.sol:73    function install(bytes4[] memory _selectors, address[] memory _plugins)

https://github.com/code-423n4/2022-07-fractional/blob/main/src/Vault.sol#L73

N01 - Not specified visibility for variable

src/references/SupplyReference.sol:12   address immutable registry; 

https://github.com/code-423n4/2022-07-fractional/blob/main/src/references/SupplyReference.sol#L12

src/utils/Metadata.sol:11   address immutable token; 

https://github.com/code-423n4/2022-07-fractional/blob/main/src/utils/Metadata.sol#L11

N02 - Typos

src/interfaces/IMigration.sol:23 // Address for the new vault to migrate to (if buyout is succesful) src/interfaces/IMigration.sol:25 // Boolean status to check if the propoal is active src/interfaces/IMigration.sol:26 bool isCommited; src/interfaces/IMigration.sol:29 // New fraction supply for a given vault that has succesfully migrated src/modules/Migration.sol:322 // Udpates ether balance of caller src/utils/SafeSend.sol:15 /// @param _to Address attemping to send to

G01 - Variables that could be immutables

It's possible to avoid storage access and save gas using immutable keyword for the following variables:

    /// @notice Address of VaultRegistry contract
    address public registry;
    /// @notice Address of Supply target contract
    address public supply;
    /// @notice Address of Transfer target contract
    address public transfer; 

https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Buyout.sol#L29-L33

    /// @notice Address of Buyout module contract
    address payable public buyout; 
    /// @notice Address of VaultRegistry contract
    address public registry; 

https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Migration.sol#L37-L39

G02 - unchecked block can be used for gas efficiency of the expression that can't overflow/underflow

i++ could be unchecked since counter couldn't overflow:

src/Vault.sol:104   for (uint256 i = 0; i < length; i++) {

https://github.com/code-423n4/2022-07-fractional/blob/main/src/Vault.sol#L104

L88 could be unchecked since if totalSupply == 0 previous line (L87) would be reverted:

src/modules/Buyout.sol:86   uint256 buyoutPrice = (msg.value * 100) /
src/modules/Buyout.sol:87    (100 - ((depositAmount * 100) / totalSupply));
src/modules/Buyout.sol:88   uint256 fractionPrice = buyoutPrice / totalSupply; 

https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Buyout.sol#L88

G03 - Caching storage variables

Since reading from memory is much cheaper than reading from storage, state variables that are called more than 1 SLOAD inside the function should be cached.

src/modules/Buyout.sol:477  ISupply(supply).burn.selector // supply 2 SLOADs

https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Buyout.sol#L477

src/modules/Buyout.sol:501  ITransfer(transfer).ERC1155BatchTransferFrom.selector  // transfer 8 SLOADs

https://github.com/code-423n4/2022-07-fractional/blob/main/src/modules/Buyout.sol#L501

G04 - Using custom error instead of require() string

Custom errors are available from solidity version 0.8.4. They cost cheaper than require/revert strings.

src/FERC1155.sol:267    "NOT_AUTHORIZED" 

https://github.com/code-423n4/2022-07-fractional/blob/main/src/FERC1155.sol#L267

src/FERC1155.sol:285    "UNSAFE_RECIPIENT" 

https://github.com/code-423n4/2022-07-fractional/blob/main/src/FERC1155.sol#L285

G05 - Using prefix increment

Prefix increment is cheaper than postfix increment. Consider using ++i in next line:

src/Vault.sol:104   for (uint256 i = 0; i < length; i++) {

https://github.com/code-423n4/2022-07-fractional/blob/main/src/Vault.sol#L104