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

0 stars 1 forks source link

Gas Optimizations #107

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1) State variables which are read more than once should be cached to save gas.

  1. ZcToken.sol
    1. protocol, redeemer and maturity state variables in convertToUnderlying function.
    2. protocol, redeemer and maturity state variables in convertToPrincipal function.

2) Remove Identical functions in ZcToken.sol.

  1. convertToUnderlying and previewRedeem functions are identical.
  2. convertToPrincipal and previewWithdraw functions are identical.

3) Convert frequenctly used revert checks into a modifier.

ZcToken.sol has the following revert check which repeats twice.

if (block.timestamp < maturity) {
            revert Maturity(maturity);
        }

4) Simplify formula for better readability and gas saving.

There are 6 functions in VaultTracker.sol ( addNotional, removeNotional, redeemInterest, transferNotionalFrom, transferNotionalFee and removeNotional ) which calculate interest in the following way:

      if (maturityRate > 0) { 
        // calculate marginal interest
          yield = ((maturityRate * 1e26) / sVault.exchangeRate) - 1e26;
      } else {
          yield = ((exchangeRate * 1e26) / sVault.exchangeRate) - 1e26;
      }
      uint256 interest = (yield * sVault.notional) / 1e26;

These can be re-written as below:

      if (maturityRate > 0) { 
        // calculate marginal interest
          interest = ((maturityRate * sVault.notional - sVault.notional) / sVault.exchangeRate);
      } else {
          interest = ((exchangeRate * sVault.notional - sVault.notional) / sVault.exchangeRate);
      }

5) Use already cached variable instead of indexing again.

There are two instances of this:

  1. The value of o[i] is already cached into memory variable order. So use it inside the following if-else statements instead of freshly indexing again.
  2. The same is happening in the exit function.
robrobbins commented 2 years ago
  1. good point, but the implementation used is the inverse of the suggestion here. to be consistent with other tickets pointing out reading from calldata here is better than memory it is more in pattern to remove the memory order and simply use o[i] throughout. this applies to every function called by these as well as changing the call to them with a memory order would require changing those signatures as well

that being said, marking this as resolved because pointing out order vs o[i] here is worth recognizing