search

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

0 stars 1 forks source link

Gas Optimizations #100

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

[G-01] STATE VARIABLES THAT NEED TO BE LOADED FOR MULTIPLE TIMES IN A FUNCTION CAN BE CACHED IN MEMORY

When state variables need to be loaded for multiple times in a function, caching them in memory can save gas. Please see @audit in the following code for state variables that can be cached.

Creator\Creator.sol
  // @audit marketPlace can be cached for the following code
  55-57:
    if (marketPlace != address(0)) {
      revert Exception(33, 0, 0, marketPlace, address(0)); 
    }

Creator\VaultTracker.sol
  // @audit maturityRate can be cached for the following code
  59-60:
    if (maturityRate > 0) {
      yield = ((maturityRate * 1e26) / vlt.exchangeRate) - 1e26;
  93-94:
    if (maturityRate > 0) { // Calculate marginal interest
      yield = ((maturityRate * 1e26) / vlt.exchangeRate) - 1e26;
  123-124:
    if (maturityRate > 0) { // Calculate marginal interest
      yield = ((maturityRate * 1e26) / vlt.exchangeRate) - 1e26;
  165-167:
    if (maturityRate > 0) {
      // calculate marginal interest
      yield = ((maturityRate * 1e26) / from.exchangeRate) - 1e26;
  222-224:
    if (maturityRate > 0) {
      // calculate marginal interest
      yield = ((maturityRate * 1e26) / sVault.exchangeRate) - 1e26;

Creator\ZcToken.sol
Tokens\ZcToken.sol
  // @audit maturity, redeemer, and protocol can be cached for the following code
  44-47:
    if (block.timestamp < maturity) {
     return 0;
    }
    return (principalAmount * IRedeemer(redeemer).getExchangeRate(protocol, cToken) / IRedeemer(redeemer).markets(protocol, underlying, maturity).maturityRate);
  53-56:
    if (block.timestamp < maturity) {
     return 0;
    }
    return (underlyingAmount * IRedeemer(redeemer).markets(protocol, underlying, maturity).maturityRate / IRedeemer(redeemer).getExchangeRate(protocol, cToken));
  71-74:
    if (block.timestamp < maturity) {
     return 0;
    }
    return (principalAmount * IRedeemer(redeemer).getExchangeRate(protocol, cToken) / IRedeemer(redeemer).markets(protocol, underlying, maturity).maturityRate);
  80-83:
    if (block.timestamp < maturity) {
     return 0;
    }
    return (balanceOf[owner] * IRedeemer(redeemer).getExchangeRate(protocol, cToken) / IRedeemer(redeemer).markets(protocol, underlying, maturity).maturityRate);
  89-92:
    if (block.timestamp < maturity) {
      return 0;
    }
    return (underlyingAmount * IRedeemer(redeemer).markets(protocol, underlying, maturity).maturityRate / IRedeemer(redeemer).getExchangeRate(protocol, cToken));

  // @audit maturity, redeemer, protocol, and underlying can be cached for the following code
  101-116:
    if (block.timestamp < maturity) {
        revert Maturity(maturity);
    }
    // Transfer logic
    // If holder is msg.sender, skip approval check
    if (holder == msg.sender) {
        redeemer.authRedeem(protocol, underlying, maturity, msg.sender, receiver, previewAmount);
        return previewAmount;
    }
    else {
        uint256 allowed = allowance[holder][msg.sender];
        if (allowed >= previewAmount) {
            revert Approvals(allowed, previewAmount);
        }
        allowance[holder][msg.sender] -= previewAmount;
        redeemer.authRedeem(protocol, underlying, maturity, holder, receiver, previewAmount);
  126-135:
    if (block.timestamp < maturity) { revert Maturity(maturity); }
    // some 5095 tokens may have custody of underlying and can can just burn PTs and transfer underlying out, while others rely on external custody
    if (holder == msg.sender) {
        return redeemer.authRedeem(protocol, underlying, maturity, msg.sender, receiver, principalAmount);
    }
    else {
        uint256 allowed = allowance[holder][msg.sender];
        if (allowed >= principalAmount) { revert Approvals(allowed, principalAmount); }
        allowance[holder][msg.sender] -= principalAmount;
        return redeemer.authRedeem(protocol, underlying, maturity, holder, receiver, principalAmount);

Marketplace\MarketPlace.sol
  // @audit swivel can be cached for the following code
  71-77:
    if (swivel == address(0)) { revert Exception(21, 0, 0, address(0), address(0)); }

    address underAddr = Compounding.underlying(p, c);

    if (markets[p][underAddr][m].vaultTracker != address(0)) { revert Exception(22, 0, 0, address(0), address(0)); }

    (address zct, address tracker) = ICreator(creator).create(p, underAddr, m, c, swivel, n, s, IErc20(underAddr).decimals()) ;
  156-164:
    ISwivel(swivel).authRedeem(p, u, market.cTokenAddr, t, a);

    return (a);
    } else {

    if (!IZcToken(market.zcToken).burn(f, a)) { revert Exception(29, 0, 0, address(0), address(0)); }

    uint256 amount = calculateReturn(p, u, m, a);
    ISwivel(swivel).authRedeem(p, u, market.cTokenAddr, t, amount);

Swivel\Swivel.sol
  // audit feeChange can be cached for the following code
  500-502:
    if (feeChange == 0) { revert Exception(16, 0, 0, address(0), address(0)); }

    if (block.timestamp < feeChange) { revert Exception(17, block.timestamp, feeChange, address(0), address(0)); }

[G-02] filled[hash] = amount CAN BE USED INSTEAD OF filled[hash] += a

For the following code, after running uint256 amount = a + filled[hash] , filled[hash] = amount is more gas-efficient than filled[hash] += a, where filled is a mapping in the state storage. This is because that filled[hash] += a needs another sload operation that costs more gas than an mload operation.

Swivel\Swivel.sol
  116-121:
  153-158:
  188-193:
  217-222:
  282-287:
  313-318:
  343-348:
  378-383:
    uint256 amount = a + filled[hash];
    ...
    // TODO cheaper to assign amount here or keep the ADD?
    filled[hash] += a;

[G-03] X = X + Y AND X = X - Y CAN BE USED INSTEAD OF X += Y OR X -= Y

x = x + y and x = x - y costs less gas than x += y and x -= y.

Creator\VaultTracker.sol
VaultTracker\VaultTracker.sol
  67: vlt.redeemable += interest;
  68: vlt.notional += a;
  102: vlt.redeemable += interest;
  103: vlt.notional -= a;
  174: from.redeemable += interest;
  175: from.notional -= a;
  193: to.redeemable += newVaultInterest;
  194: to.notional += a;
  213: oVault.notional -= a;
  230: sVault.redeemable += interest;
  234: sVault.notional += a;

Creator\ZcToken.sol
Tokens\ZcToken.sol
  115: allowance[holder][msg.sender] -= previewAmount;
  134: allowance[holder][msg.sender] -= principalAmount;

[G-04] ++VARIABLE CAN BE USED INSTEAD OF VARIABLE++

++variable costs less gas than variable++.

Swivel\Swivel.sol
  100: unchecked {i++;}
  269: unchecked {i++;}
  418: i++;
  511: x++;
  564: i++;

[G-05] CONSTANTS CAN BE PRIVATE IF POSSIBLE

For constants that are used only inside the contract, they can be private than public to cost less gas.

Swivel\Swivel.sol
  25: string constant public NAME = 'Swivel Finance';
  26: string constant public VERSION = '3.0.0';
  27: uint256 constant public HOLD = 3 days;
  35: uint16 constant public MIN_FEENOMINATOR = 33;

[G-06] UNUSED NAMED RETURNS COSTS UNNECESSARY DEPLOYMENT GAS

When a function has unused named returns, unnecessary deployment gas is burned.

Marketplace\MarketPlace.sol
  148: function authRedeem(uint8 p, address u, uint256 m, address f, address t, uint256 a) public authorized(markets[p][u][m].zcToken) returns (uint256 underlyingAmount) {

Creator\ZcToken.sol
Tokens\ZcToken.sol
  43: function convertToUnderlying(uint256 principalAmount) external override view returns (uint256 underlyingAmount){
  52: function convertToPrincipal(uint256 underlyingAmount) external override view returns (uint256 principalAmount){
  61: function maxRedeem(address owner) external override view returns (uint256 maxPrincipalAmount){
  70: function previewRedeem(uint256 principalAmount) external override view returns (uint256 underlyingAmount){
  79: function maxWithdraw(address owner) external override view returns (uint256 maxUnderlyingAmount){
  88: function previewWithdraw(uint256 underlyingAmount) external override view returns (uint256 principalAmount){
  98: function withdraw(uint256 underlyingAmount, address receiver, address holder) external override returns (uint256 principalAmount){
  124: function redeem(uint256 principalAmount, address receiver, address holder) external override returns (uint256 underlyingAmount){

[G-07] NEWER VERSION OF SOLIDITY CAN BE USED

The protocol can benefit from more gas-efficient features and fixes by using a newer version of Solidity. Changes for newer Solidity versions can be viewed here.

Creator\ZcToken.sol
Tokens\ZcToken.sol
  2: pragma solidity ^0.8.4;