Gas Optimizations - Githubissues

FINDINGS

Use immutable on variables that are only set in the constructor and never after

File: Swivel.sol line 33

  address public aaveAddr; // TODO immutable?

The above address is set in the constructor and never set again in the contract

Using unchecked blocks to save gas

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn’t possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block see resource

File:ZcToken.sol line 115

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

The above line cannot underflow due to the check on line 112 which ensures that allowance[holder][msg.sender] is greater than previewAmount before performing the arithmetic operation

The above can be modified to:

unchecked {
        allowance[holder][msg.sender] -= previewAmount;
}

File:ZcToken.sol line 134

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

The above line cannot underflow due to the check on line 133 which ensures that allowance[holder][msg.sender] is greater than principalAmount before performing the arithmetic operation

Cache storage values in memory to minimize SLOADs

The code can be optimized by minimizing the number of SLOADs. SLOADs are expensive 100 gas compared to MLOADs/MSTOREs(3gas) Storage value should get cached in memory

NB: Some functions have been truncated where necessary to just show affected parts of the code

vaultTracker.sol.addNotional(): maturityRate should be cached in memory

File: VaultTracker.sol line 59,60

  function addNotional(address o, uint256 a) external authorized(admin) returns (bool) {
    uint256 exchangeRate = Compounding.exchangeRate(protocol, cTokenAddr);

    Vault memory vlt = vaults[o];
    if (vlt.notional > 0) {
      uint256 yield;

      // if market has matured, calculate marginal interest between the maturity rate and previous position exchange rate
      // otherwise, calculate marginal exchange rate between current and previous exchange rate.
      if (maturityRate > 0) { // Calculate marginal interest
        yield = ((maturityRate * 1e26) / vlt.exchangeRate) - 1e26;
      } else {
        yield = ((exchangeRate * 1e26) / vlt.exchangeRate) - 1e26;
      }

    return true;
  }

In the above function maturityRate should be cached in memory to minimize the number of SLOADs

SLOAD 1: line 59 SLOAD 2: line 60

vaultTracker.sol.addNotional(): maturityRate should be cached in memory

File: VaultTracker.sol line 93,94

  function removeNotional(address o, uint256 a) external authorized(admin) returns (bool) {

    // if market has matured, calculate marginal interest between the maturity rate and previous position exchange rate
    // otherwise, calculate marginal exchange rate between current and previous exchange rate.
    uint256 yield;
    if (maturityRate > 0) { // Calculate marginal interest
      yield = ((maturityRate * 1e26) / vlt.exchangeRate) - 1e26;
    } else {
      // calculate marginal interest
      yield = ((exchangeRate * 1e26) / vlt.exchangeRate) - 1e26;
    }

    return true;
  }

In the above function maturityRate should be cached in memory to minimize the number of SLOADs

SLOAD 1: line 93 SLOAD 2: line 94

vaultTracker.sol.redeemInterest(): maturityRate should be cached in memory

File: VaultTracker.sol line 123,124

  function redeemInterest(address o) external authorized(admin) returns (uint256) {

    // if market has matured, calculate marginal interest between the maturity rate and previous position exchange rate
    // otherwise, calculate marginal exchange rate between current and previous exchange rate.
    uint256 yield;
    if (maturityRate > 0) { // Calculate marginal interest
      yield = ((maturityRate * 1e26) / vlt.exchangeRate) - 1e26;
    } else {
      // calculate marginal interest
      yield = ((exchangeRate * 1e26) / vlt.exchangeRate) - 1e26;
    }

    // return adds marginal interest to previously accrued redeemable interest
    return (redeemable + interest);
  }

In the above function maturityRate should be cached in memory to minimize the number of SLOADs

SLOAD 1: line 123 SLOAD 2: line 124

vaultTracker.sol.transferNotionalFrom(): maturityRate should be cached in memory

File: VaultTracker.sol line 165,167,184,186

  function transferNotionalFrom(address f, address t, uint256 a) external authorized(admin) returns (bool) {
    if (f == t) { revert Exception(32, 0, 0, f, t); }
    uint256 yield;
    if (maturityRate > 0) { 
      // calculate marginal interest
      yield = ((maturityRate * 1e26) / from.exchangeRate) - 1e26;
    } else {
      yield = ((exchangeRate * 1e26) / from.exchangeRate) - 1e26;
    }
        ...
    vaults[f] = from;
        ...
    // transfer notional to address "t", calculate interest if necessary
    if (to.notional > 0) {
      // if market has matured, calculate marginal interest between the maturity rate and previous position exchange rate
      // otherwise, calculate marginal exchange rate between current and previous exchange rate.
      if (maturityRate > 0) { 
        // calculate marginal interest
        yield = ((maturityRate * 1e26) / to.exchangeRate) - 1e26;
      } else {
        yield = ((exchangeRate * 1e26) / to.exchangeRate) - 1e26;
      }

    return true;
  }

In the above function maturityRate should be cached in memory to minimize the number of SLOADs(4 SLOADS)

SLOAD 1: line 165 SLOAD 2: line 167 SLOAD 3: line 184 SLOAD 4: line 186

vaultTracker.sol.transferNotionalFee(): maturityRate should be cached in memory ()

File: VaultTracker.sol line 222,224

  function transferNotionalFee(address f, uint256 a) external authorized(admin) returns(bool) {
    Vault memory oVault = vaults[f];

    uint256 yield;
    if (sVault.exchangeRate != exchangeRate) {
      // if market has matured, calculate marginal interest between the maturity rate and previous position exchange rate
      // otherwise, calculate marginal exchange rate between current and previous exchange rate.
      if (maturityRate > 0) { 
        // calculate marginal interest
          yield = ((maturityRate * 1e26) / sVault.exchangeRate) - 1e26;
      } else {
          yield = ((exchangeRate * 1e26) / sVault.exchangeRate) - 1e26;
      }

    return true;
  }

In the above function maturityRate should be cached in memory to minimize the number of SLOADs(4 SLOADS)

SLOAD 1: line 222 SLOAD 2: line 224

MarketPlace.sol.redeemZcToken(): maturityRate should be cached in memory

File: MarketPlace.sol line 180,188

  function redeemZcToken(uint8 p, address u, uint256 m, address t, uint256 a) external authorized(swivel) unpaused(p) returns (uint256) {
    Market memory market = markets[p][u][m];
   // if the market has not matured, mature it and redeem exactly the amount
    if (market.maturityRate == 0) {
      if (!matureMarket(p, u, m)) { revert Exception(30, 0, 0, address(0), address(0)); }
    }
    if (!IZcToken(market.zcToken).burn(t, a)) { revert Exception(29, 0, 0, address(0), address(0)); }
    emit RedeemZcToken(p, u, m, t, a);
    if (market.maturityRate == 0) {
      return a;
    } else { 
      // if the market was already mature the return should include the amount + marginal floating interest generated on Compound since maturity
      return calculateReturn(p, u, m, a);
    }
  }

In the above function maturityRate should be cached in memory to minimize the number of SLOADs

SLOAD 1: line 180 SLOAD 2: line 188

ZcToken.sol.convertToUnderlying(): redeemer should be cached in memory

File: ZcToken.sol line 47

    function convertToUnderlying(uint256 principalAmount) external override view returns (uint256 underlyingAmount){
        if (block.timestamp < maturity) {
            return 0;
        }
        return (principalAmount * IRedeemer(redeemer).getExchangeRate(protocol, cToken) / IRedeemer(redeemer).markets(protocol, underlying, maturity).maturityRate);
    }

In the above function redeemer should be cached in memory to minimize the number of SLOADs

It is being read twice from memory in the return statement

ZcToken.sol.convertToUnderlying(): redeemer should be cached in memory ()

File: ZcToken.sol line 56

        return (underlyingAmount * IRedeemer(redeemer).markets(protocol, underlying, maturity).maturityRate / IRedeemer(redeemer).getExchangeRate(protocol, cToken));

++i costs less gas compared to i++ or i += 1

++i costs less gas compared to i++ or i += 1 for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.

i++ increments i and returns the initial value of i. Which means:

uint i = 1;  
i++; // == 1 but i == 2

But ++i returns the actual incremented value:

uint i = 1;  
++i; // == 2 and i == 2 too, so no need for a temporary variable

In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2

Instances include:

File: Swivel.sol line 100

      unchecked {i++;}

The above should be modified to:

      unchecked {++i;}

Other instances File: Swivel.sol line 269

      unchecked {i++;}

File: Swivel.sol line 417-419

      unchecked {
        i++;
      }

File: Swivel.sol line 510-512

      unchecked {
        x++;
      }

File: Swivel.sol line 563-565

      unchecked {
        i++;
      }

Using private rather than public for constants, saves gas

If needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table.

File: Swivel.sol line 25

  string constant public NAME = 'Swivel Finance';

File: Swivel.sol line 26

  string constant public VERSION = '3.0.0';

File: Swivel.sol line 27

  uint256 constant public HOLD = 3 days;

File: Swivel.sol line 35

  uint16 constant public MIN_FEENOMINATOR = 33;

constants should be defined rather than using magic numbers

There are several occurrences of literal values with unexplained meaning .Literal values in the codebase without an explained meaning make the code harder to read, understand and maintain, thus hindering the experience of developers, auditors and external contributors alike.

Developers should define a constant variable for every magic value used , giving it a clear and self-explanatory name.

File: VaultTracker.sol line 60 1e26

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

File: VaultTracker.sol line 62 1e26

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

File: VaultTracker.sol line 65 1e26

      uint256 interest = (yield * vlt.notional) / 1e26;

Other instances File: VaultTracker.sol line 94 File: VaultTracker.sol line 97 File: VaultTracker.sol line 100 File: VaultTracker.sol line 124 File: VaultTracker.sol line 127 File: VaultTracker.sol line 130

code-423n4 / 2022-07-swivel-findings

Gas Optimizations #184

FINDINGS

Use immutable on variables that are only set in the constructor and never after

Using unchecked blocks to save gas

Cache storage values in memory to minimize SLOADs

vaultTracker.sol.addNotional(): maturityRate should be cached in memory

vaultTracker.sol.addNotional(): maturityRate should be cached in memory

vaultTracker.sol.redeemInterest(): maturityRate should be cached in memory

vaultTracker.sol.transferNotionalFrom(): maturityRate should be cached in memory

vaultTracker.sol.transferNotionalFee(): maturityRate should be cached in memory ()

MarketPlace.sol.redeemZcToken(): maturityRate should be cached in memory

ZcToken.sol.convertToUnderlying(): redeemer should be cached in memory

ZcToken.sol.convertToUnderlying(): redeemer should be cached in memory ()

++i costs less gas compared to i++ or i += 1

Using private rather than public for constants, saves gas

constants should be defined rather than using magic numbers