Fractional v2 contest - hrishibhat's results

Gas Optimizations

1. Following variables can be made immutable.

• registry in the BaseVault.sol
• registry in Buyout.sol
• supply in Buyout.sol
• transfer in Buyout.sol
• supply in Minter.sol
• buyout in Migration.sol
• registry in Migration.col
• implementation in VaultFactory.sol

2. Use calldata instead of memory in

• FERC1155.sol#L68
• FERC1155.sol#L261
• FERC1155.sol#L83
• Vault.sol#L73
• Vault.sol#L101
• VaultRegistry.sol#L70
• VaultRegistry.sol#L71
• VaultRegistry.sol#L85
• VaultRegistry.sol#L86
• VaultRegistry.sol#L105
• VaultRegistry.sol#L150
• VaultRegistry.sol#L151
• VaultRegistry.sol#L168
• VaultRegistry.sol#L169
• Metadata.sol#L24

3. Use ++i instead of i++ & Add unchecked { ++i } in all following for loops.

• Vault.sol#L78

  - for (uint256 i = 0; i < length; i++) {
  -    methods[_selectors[i]] = _plugins[i];
  - }
  + for (uint256 i = 0; i < length; ) {
  +    methods[_selectors[i]] = _plugins[i];
  +    unchecked { ++i; }
  + }          

• Vault.sol#L104

  - for (uint256 i = 0; i < length; i++) {
  -     methods[_selectors[i]] = address(0);
  - }
  + for (uint256 i = 0; i < length; ) {
  +     methods[_selectors[i]] = address(0);
  +     unchecked { ++i; }
  + }          

4. Remove unnecessary variables:

• VaultFactory.sol#L68 data is not necessary is variable

  - bytes memory data = abi.encodePacked();
  - vault = implementation.clone(salt, data);
  + vault = implementation.clone(salt, abi.encodePacked());    

• Vault.sol#L60 leaf is not necessary and can be replaced by

  - bytes32 leaf = keccak256(abi.encode(msg.sender, _target, selector));
  - if (!MerkleProof.verify(_proof, merkleRoot, leaf)) {    
  + if (!MerkleProof.verify(_proof, merkleRoot, keccak256(abi.encode(msg.sender, _target, selector)))) {

• VaultFactory#L46 'data' is not necessary and can be replaced by

  - bytes32 data = keccak256(
  -     abi.encodePacked(bytes1(0xff), address(this), salt, creationHash)
  - );
  - vault = address(uint160(uint256(data)));    
  + vault = address(uint160(uint256(keccak256(
  +         abi.encodePacked(bytes1(0xff), address(this), salt, creationHash)
  +     ))));

5. Use storage pointer to set value

• Migration.sol#L279

  -migrationInfo[_vault][_proposalId].fractionsMigrated = true;    
  +proposal.fractionsMigrated = true;        

<!-- ### 6. Internal functions which are used only once can be inlined * https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/FERC1155.sol#L324 * https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/FERC1155.sol#L350 -->

7. Mark function calls from known callers as payable to bypass the check

• Functions with onlyController & onlyRegistry modifier in FERC1155.sol

  - function setContractURI(string calldata _uri) external onlyController {
  + function setContractURI(string calldata _uri) external payable onlyController {
  
  - function setMetadata(address _metadata, uint256 _id) external onlyController {
  + function setMetadata(address _metadata, uint256 _id) external payable onlyController {
  
  - function setRoyalties( uint256 _id, address _receiver, uint256 _percentage ) external onlyController { 
  + function setRoyalties(uint256 _id, address _receiver, uint256 _percentage ) external payable onlyController {
  
  - function transferController(address _newController) external onlyController {
  + function transferController(address _newController) external payable onlyController {  
  
  - function burn(address _from, uint256 _id, uint256 _amount) external onlyRegistry {  
  + function burn(address _from, uint256 _id, uint256 _amount) external payable onlyRegistry {
  
  - function mint(address _to, uint256 _id, uint256 _amount, bytes memory _data) external onlyRegistry {
  + function mint(address _to, uint256 _id, uint256 _amount, bytes memory _data) external payable onlyRegistry {    

8. Use >> 1 instead of / 2 to save gas. Right shift x >> y is same as x / 2**y.

• MerkleBase.sol#L100

  - result = new bytes32[](length / 2 + 1);
  + result = new bytes32[]((length >> 2) + 1);
  .
  .
  - result = new bytes32[](length / 2);
  + result = new bytes32[](length >> 2);
  .
  .
  - _node = _node / 2;
  + _node = _node >> 2;
  

9. Add unchecked block for where arithmetic overflow cannot happen

• In cash() function in Buyout.sol#L244:

  + unchecked {
  +    uint256 totalSupply = IVaultRegistry(registry).totalSupply(_vault);
  +    uint256 buyoutShare = (tokenBalance * ethBalance) /
  +        (totalSupply + tokenBalance);
  +    _sendEthOrWeth(msg.sender, buyoutShare);
  +    // Emits event for cashing out of buyout pool
  +    emit Cash(_vault, msg.sender, buyoutShare);
  + }

• In sellFractions() function in Buyout.sol#L112

  + unchecked { 
  +    uint256 endTime = startTime + PROPOSAL_PERIOD;
  +    if (block.timestamp > endTime)
  +        revert TimeExpired(block.timestamp, endTime);    
  + }
  

• In buyFractions() function in Buyout.sol#L149

  + unchecked { 
  +    uint256 endTime = startTime + REJECTION_PERIOD;
  +    if (block.timestamp > endTime)
  +        revert TimeExpired(block.timestamp, endTime);
  + }

• In end() function in Buyout.sol#L184

  + unchecked { 
  +    uint256 endTime = startTime + REJECTION_PERIOD;
  +    if (block.timestamp <= endTime)
  +        revert TimeNotElapsed(block.timestamp, endTime);
  +
  +    uint256 tokenBalance = IERC1155(token).balanceOf(address(this), id);
  +    // Checks totalSupply of auction pool to determine if buyout is successful or not
  +    if (
  +        (tokenBalance * 1000) /
  +            IVaultRegistry(registry).totalSupply(_vault) >
  +        500
  +    ) {
  +        // Initializes vault transaction
  +        bytes memory data = abi.encodeCall(
  +            ISupply.burn,
  +            (address(this), tokenBalance)
  +        );
  +        // Executes burn of fractional tokens from pool
  +        IVault(payable(_vault)).execute(supply, data, _burnProof);
  +        // Sets buyout state to successful
  +        buyoutInfo[_vault].state = State.SUCCESS;
  +        // Emits event for ending successful auction
  +        emit End(_vault, State.SUCCESS, proposer);
  +    } else {
  +        // Deletes auction info
  +        delete buyoutInfo[_vault];
  +        // Transfers fractions and ether back to proposer of the buyout pool
  +        IERC1155(token).safeTransferFrom(
  +            address(this),
  +            proposer,
  +            id,
  +            tokenBalance,
  +            ""
  +        );
  +        _sendEthOrWeth(proposer, ethBalance);
  +        // Emits event for ending unsuccessful auction
  +        emit End(_vault, State.INACTIVE, proposer);
  + }

• In the proposal() function in Migration.sol#L72

  + unchecked {
  +    Proposal storage proposal = migrationInfo[_vault][++nextId];
  +    proposal.startTime = block.timestamp;
  +    proposal.targetPrice = _targetPrice;
  +    proposal.modules = _modules;
  +    proposal.plugins = _plugins;
  +    proposal.selectors = _selectors;
  +    proposal.oldFractionSupply = IVaultRegistry(registry).totalSupply(
  +        _vault
  +    );
  +    proposal.newFractionSupply = _newFractionSupply;    
  + }

10. Using a = a + b instead of a += b for state variables saves gas

• In Migration.sol#L123

  - proposal.totalEth += msg.value;
  + proposal.totalEth = proposal.totalEth + msg.value;    

• In Migration.sol#L134

  - proposal.totalFractions += msg.value;
  + proposal.totalFractions = proposal.totalFractions + msg.value;

11. Looking up array length from memory every time in a loop costs more gas.

• In Buyout.sol#L454

  - for (uint256 i; i < permissions.length; ) {
  + uint256 length;    
  + for (uint256 i; i < length; ) {