Swivel v3 contest - m_Rassska's results

Table of contents

• [0x0] Disclaimer
• [G-01] Try ++i instead of i++
• [G-02] Consider a = a + b instead of a += b
• [G-03] Consider marking onlyOwner functions as payable
• [G-04] Cache state variables, MLOAD << SLOAD
• [G-05] Declare immutable instead of state variables
• [G-06] Define constants/immutable/state as private/internal
• [G-07] Check out calldataloud vs mload
• [G-08] Internal functions can be inlined
• [G-09] Redundant gas usage
• [G-10] Use uint instead of uint8, uint64, if it's possible
• [G-11] Tight up state variables

Disclaimer<a name="0x0"></a>

• Please, consider everything described below as a general recommendation. These notes will represent potential possibilities to optimize gas consumption. It's okay, if something is not suitable in your case. Just let me know the reason in the comments section. Enjoy!

[G-01] Try ++i instead of i++<a name="G-01"></a>

Description:

• In case of i++, the compiler needs to create a temp variable to return and then it gets incremented.
• In case of ++i, the compiler just simply returns already incremented value.

Recommendations:

• Use prefix increment instead of postfix.

All occurances:

• Contracts:

    file: Swivel/Swivel.sol
    ...............................
    
      // Lines: [100-100]
        unchecked {i++;}
        
      // Lines: [269-269]
        unchecked {i++;}
  
      // Lines: [417-417]
        unchecked {
          i++;
        }
  
      // Lines: [563-563]
        unchecked {
          i++;
        }

[G-02] Consider a = a + b instead of a += b<a name="G-02"></a>

Description:

• It has an impact on the deployment cost and the cost for distinct transaction.

All occurances:

• Contracts:

    file: Swivel/Swivel.sol
    ...............................
    
      // Lines: [121-121, 158-158, 193-193, 222-222, 287-287, 318-318, 348-348, 383-383]
        filled[hash] += a;
  
    file: MarketPlace/Erc20.sol
    ...............................
  
      // Lines: [92-92; 114-114; 202-202]
          balanceOf[to] += amount;
  
      // Lines: [87-87; 109-109, 209;209]
          balanceOf[msg.sender] -= amount;
  
      // Lines: [197-197; 114-114;]
          totalSupply += amount; 
  
      // Lines: [214-214]
          totalSupply -= amount; 
  
    file: VaultTracker/VaultTracker.sol
    ...............................
      // Lines: [67-68]
        vlt.redeemable += interest;
        vlt.notional += a;
  
      // Lines: [102-103]
        vlt.redeemable += interest;
        vlt.notional -= a;
  
      // Lines: [174-175]
        from.redeemable += interest;
        from.notional -= a;
  
      // Lines: [193-194]
          to.redeemable += newVaultInterest;
          to.notional += a;
  
      // Lines: [230-234]
          sVault.redeemable += interest;
          sVault.notional += a;
  

[G-03] Consider marking onlyOwner functions as payable<a name="G-03"></a>

Description:

• A little optmization in comparison between payable and non-payable functions. Also, there is a little tradeoff here between readability and optimization.

All occurances:

• Contracts:

    file: Creator/Creator.sol
    ...............................
    
      // Lines: [30-39]
        function create (
          uint8 p,
          address u,
          uint256 m,
          address c,
          address sw,
          string memory n,
          string memory s,
          uint8 d
        ) external authorized(marketPlace) returns (address, address) {}
  
      // Lines: [47-47]
        function setAdmin(address a) external authorized(admin) returns (bool) {}
  
      // Lines: [54-54]
        function setMarketPlace(address m) external authorized(admin) returns (bool) {}
  
      ```

Also in files VaultTracker.sol, MarketPlace.sol, Swivel.sol where there is a authorized(admin) modifier

[G-04] Cache state variables, MLOAD << SLOAD<a name="G-04"></a>

Description:

• MLOAD costs only 3 units of gas, SLOAD(warm access) is about 100 units. Therefore, cache, when it's possible.

All occurances:

• Contracts:

    file: MarketPlace/MarketPlace.sol
    ...............................
  
      // Lines: [46-46]
        if (swivel != address(0)) { revert Exception(20, 0, 0, swivel, address(0));  } // swivel could be cached
  
      // Lines: [71-77]
        if (swivel == address(0)) { revert Exception(21, 0, 0, address(0), address(0)); }
        (address zct, address tracker) = ICreator(creator).create(p, underAddr, m, c, swivel, n, s, IErc20(underAddr).decimals()) ; // swivel could be cached
  
      // Lines: [156-164]
        ISwivel(swivel).authRedeem(p, u, market.cTokenAddr, t, a);
        ISwivel(swivel).authRedeem(p, u, market.cTokenAddr, t, amount); // swivel could be cached
  
      ```

[G-05] Declare immutable instead of state variables<a name="G-05"></a>

Description:

• Since it's initialized once, there is no reason for state variable allocation.

All occurances:

• Contracts:

    file: Swivel/Swivel.sol
    ...............................
    
      // Lines: [33-33]
        address public aaveAddr; // TODO immutable?
  
    file: MarketPlace/Erc20.sol
    ...............................
      // Lines: [23-26]
        string public name;
        string public symbol;
  
    file: MarketPlace/MarketPlace.sol
    ...............................
      // Lines: [25-25]
        address public swivel;
  

[G-06] Define public constants/immutable/state as private/internal<a name="G-06"></a>

Description:

• Declaring state variables as private/internal doesn't generate getter functions. For mappings it could be quite expensive.

All occurances:

• Contracts:

    file: Swivel/Swivel.sol
    ...............................
    
      // Lines: [17-21]
        mapping (bytes32 => bool) public cancelled;
        mapping (bytes32 => uint256) public filled;
        mapping (address => uint256) public withdrawals;
        mapping (address => uint256) public approvals;
  
      // Lines: [25-29]
        string constant public NAME = 'Swivel Finance';
        string constant public VERSION = '3.0.0';
        uint256 constant public HOLD = 3 days;
        bytes32 public immutable domain;
        address public immutable marketPlace;
  
      // Lines: [35-35]
        uint16 constant public MIN_FEENOMINATOR = 33;
  
      // Lines: [37-37]
        uint16[4] public feenominators;
  
      // Lines: [39-39]
        uint256 public feeChange;
  
    file: MarketPlace/Erc20.sol
    ...............................
      // Lines: [35-37]
        mapping(address => uint256) public balanceOf;
        mapping(address => mapping(address => uint256)) public allowance;
        mapping(address => uint256) public nonces;
  
    file: MarketPlace/MarketPlace.sol
    ...............................
      // Lines: [21-25]
        mapping (uint8 => mapping (address => mapping (uint256 => Market))) public markets;
        mapping (uint8 => bool) public paused;
        address public admin;
        address public swivel;

[G-07] Check out calldataloud vs mload<a name="G-07"></a>

Description:

• Consider reading args directly from the calldata instead of memory, if args doesn't require any changes.

All occurances:

• Contracts:

    file: Swivel/Swivel.sol
    ...............................
    
      // Lines: [495-495]
        function setFee(uint16[] memory i, uint16[] memory d) external authorized(admin) returns (bool) {}
  
    file: MarketPlace/MarketPlace.sol
    ...............................
      // Lines: [64-70]
        function createMarket(
          uint8 p,
          uint256 m,
          address c,
          string memory n,
          string memory s
        ) external authorized(admin) unpaused(p) returns (bool) {}

[G-08] Internal functions can be inlined<a name="G-08"></a>

Description:

• It takes some extra JUMPs which costs around 12 gas uints for each JUMP.

All occurances:

• Contracts:

    file: MarketPlace/MarketPlace.sol
    ...............................
    
      // Lines: [215-215]
        function calculateReturn(uint8 p, address u, uint256 m, uint256 a) internal view returns (uint256) {}

[G-09] Redundant gas usage<a name="G-09"></a>

Description:

• Extra gas usage without the reason, use _selectors.length in loops.

All occurances:

• Contracts:

    file: Swivel/Swivel.sol
    ...............................
    
      // Lines: [496-496]
        uint256 len = i.length; // there is no reason for this allocation. Use `i.length` in loops.
  
      // Lines: [545-545]
        uint256 len = u.length;
  
      // Lines: [549-549]
        uint256 max = 2**256 - 1; // just use const without invoking pow()
  

[G-10] Use uint instead of uint8, uint64, if it's possible<a name="G-10"></a>

Description:

• Stack slot is 32bytes, therefore using explicit casts like uint8, uint64 leads to overheads.

All occurances:

• Contracts:

    file: Swivel/Swivel.sol
    ...............................
    
      // Lines: [15-15]
        error Exception(uint8, uint256, uint256, address, address);
  
      // Lines: [15-15]
        uint16 constant public MIN_FEENOMINATOR = 33;
  
      // Lines: [15-15]
        uint16 constant public MIN_FEENOMINATOR = 33;
  
      // Lines: [15-15]
        uint16 constant public MIN_FEENOMINATOR = 33;
  
      // Lines: [495-495]
        function setFee(uint16[] memory i, uint16[] memory d) external authorized(admin) returns (bool) {}
  
      // Lines: [578-578]
        function splitUnderlying(uint8 p, address u, uint256 m, uint256 a) external returns (bool) {}
  
      // Lines: [600-600]
        function combineTokens(uint8 p, address u, uint256 m, uint256 a) external returns (bool) {}
        
      // Lines: [620-620]
        function authRedeemZcToken(uint8 p, address u, address c, address t, uint256 a) external authorized(marketPlace) returns(bool) {}
  
      // Lines: [634-634]
        function redeemZcToken(uint8 p, address u, uint256 m, uint256 a) external returns (bool) {}
  
      // Lines: [651-651]
        function redeemVaultInterest(uint8 p, address u, uint256 m) external returns (bool) {}
  
      // Lines: [670-670]
        function redeemSwivelVaultInterest(uint8 p, address u, uint256 m) external returns (bool) {}
  
      // Lines: [706-706]
        function deposit(uint8 p, address u, address c, uint256 a) internal returns (bool) {}
  
      // Lines: [739-739]
        function withdraw(uint8 p, address u, address c, uint256 a) internal returns (bool) {}
  
      file: MarketPlace/Compounding.sol
      ...............................
        // Lines: [50-50]
          function underlying(uint8 p, address c) internal view returns (address) {}
  
        // Lines: [68-68]
          function exchangeRate(uint8 p, address c) internal view returns (uint256) {}
  
      file: MarketPlace/MarketPlace.sol
      ...............................
        // Lines: [28-36]
          event Create(uint8 indexed protocol, address indexed underlying, uint256 indexed maturity, address cToken, address zcToken, address vaultTracker);
          event Mature(uint8 indexed protocol, address indexed underlying, uint256 indexed maturity, uint256 maturityRate, uint256 matured);
          event RedeemZcToken(uint8 indexed protocol, address indexed underlying, uint256 indexed maturity, address sender, uint256 amount);
          event RedeemVaultInterest(uint8 indexed protocol, address indexed underlying, uint256 indexed maturity, address sender);
          event CustodialInitiate(uint8 indexed protocol, address indexed underlying, uint256 indexed maturity, address zcTarget, address nTarget, uint256 amount);
          event CustodialExit(uint8 indexed protocol, address indexed underlying, uint256 indexed maturity, address zcTarget, address nTarget, uint256 amount);
          event P2pZcTokenExchange(uint8 indexed protocol, address indexed underlying, uint256 indexed maturity, address from, address to, uint256 amount);
          event P2pVaultExchange(uint8 indexed protocol, address indexed underlying, uint256 indexed maturity, address from, address to, uint256 amount);
          event TransferVaultNotional(uint8 indexed protocol, address indexed underlying, uint256 indexed maturity, address from, address to, uint256 amount);
  
    ```

[G-11] Tight up state variables<a name="G-11"></a>

Description:

• Each slot is 32bytes in size, therefore, try to tight up those slots.

All occurances:

• Contracts:

    file: Swivel/Swivel.sol
    ...............................
    
      // Lines: [30-30]
        address public admin; // could be tight up with `feenominators` 
    ```
  

Summing up:

• I didn't note every possible occurance, but feel free to find similar cases and resolve them all accross the whole project. Have a nice day!