Gas Optimizations

[code423n4]

Gas Optimization for Infinity NFT Marketplace

G1

Link: https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L361

See @audit comment.

  function takeOrders(OrderTypes.MakerOrder[] calldata makerOrders, OrderTypes.OrderItem[][] calldata takerNfts)
    external
    payable
    nonReentrant
  {
    ...
      
    if (!isMakerSeller) {
      require(currency != address(0), 'offers only in ERC20');
    }
    
    ...
    
    // @audit it should only check `currency == address(0)` since 
    // at Line 9 it is already guaranteed that when currency == address(0), 
    // isMakerSeller must be true
    // 
    // it is worth to optimize since this is within a loop (which 
    // is in the root caller)
    if (isMakerSeller && currency == address(0)) {
      require(msg.value >= totalPrice, 'invalid total price');
    }
  }

There are other similar code snippets whose gas can be further optimized in a similar way.

• https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L325

G2

Link: https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1152-L1165

See @audit comment.

  function _getCurrentPrice(OrderTypes.MakerOrder calldata order) internal view returns (uint256) {
    (uint256 startPrice, uint256 endPrice) = (order.constraints[1], order.constraints[2]);
    uint256 duration = order.constraints[4] - order.constraints[3];
    uint256 priceDiff = startPrice > endPrice ? startPrice - endPrice : endPrice - startPrice;
    if (priceDiff == 0 || duration == 0) {
      return startPrice;
    }
    uint256 elapsedTime = block.timestamp - order.constraints[3];
    uint256 PRECISION = 10**4; // precision for division; similar to bps
    uint256 portionBps = elapsedTime > duration ? PRECISION : ((elapsedTime * PRECISION) / duration);
    priceDiff = (priceDiff * portionBps) / PRECISION;
      
    // @audit it should be unchecked since the return value is 
    // always between the startPrice and endPrice
    // 
    // it is worth to optimize since this is within a loop
    return startPrice > endPrice ? startPrice - priceDiff : startPrice + priceDiff;
  }

There are other similar code snippets whose gas can be further optimized in a similar way.

• https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L330-L342

G3

Link: https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1152-L1165

See @audit comment.

  /// @dev Gets current order price for orders that vary in price over time (dutch and reverse dutch auctions)
  function _getCurrentPrice(OrderTypes.MakerOrder calldata order) internal view returns (uint256) {
    (uint256 startPrice, uint256 endPrice) = (order.constraints[1], order.constraints[2]);
    uint256 duration = order.constraints[4] - order.constraints[3];
    uint256 priceDiff = startPrice > endPrice ? startPrice - endPrice : endPrice - startPrice;
    if (priceDiff == 0 || duration == 0) {
      return startPrice;
    }
    uint256 elapsedTime = block.timestamp - order.constraints[3];
    uint256 PRECISION = 10**4; // precision for division; similar to bps
    
    // @audit it should be unchecked since the block.timestamp * PRECISION 
    // cannot exceed the limit
    // 
    // it is worth to optimize since this is within a loop
    uint256 portionBps = elapsedTime > duration ? PRECISION : ((elapsedTime * PRECISION) / duration);
    priceDiff = (priceDiff * portionBps) / PRECISION;
      
    return startPrice > endPrice ? startPrice - priceDiff : startPrice + priceDiff;
  }

There are other similar code snippets whose gas can be further optimized in a similar way.

• https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L330-L342

G4

Link: https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L149

  function matchOneToOneOrders(
    OrderTypes.MakerOrder[] calldata makerOrders1,
    OrderTypes.MakerOrder[] calldata makerOrders2
  ) external {
    uint256 startGas = gasleft();
   
    ...  
      
    for (uint256 i = 0; i < numMakerOrders; ) {
      
      // @audit should be unchecked since gas limit cannot exceed 
      // type(uint256).max
      // 
      // it is worth to optimize since this is within a loop
      uint256 startGasPerOrder = gasleft() + ((startGas - gasleft()) / numMakerOrders);
      ...
       
    }

There are other similar code snippets whose gas can be further optimized in a similar way.

• https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L202
• https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L231
• https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L273
• https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L739
• https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L787
• https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L893

G5

Link: https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L72

See @audit comment.

  function advanceEpoch() external {
    require(currentEpoch < getMaxEpochs(), 'no epochs left');
    require(block.timestamp >= currentEpochTimestamp + getCliff(), 'cliff not passed');
    require(block.timestamp >= previousEpochTimestamp + getEpochDuration(), 'not ready to advance');

    uint256 epochsPassedSinceLastAdvance = (block.timestamp - previousEpochTimestamp) / getEpochDuration();
    uint256 epochsLeft = getMaxEpochs() - currentEpoch;
    epochsPassedSinceLastAdvance = epochsPassedSinceLastAdvance > epochsLeft
      ? epochsLeft
      : epochsPassedSinceLastAdvance;

    // update epochs
    // @audit could be unchecked, since currentEpoch cannot be greater than getMaxEpochs() 
    currentEpoch += epochsPassedSinceLastAdvance;
    previousEpochTimestamp = block.timestamp;

  }

G6

Link:https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L332

See @audit comment:

function _getCurrentPrice(OrderTypes.MakerOrder calldata order) internal view returns (uint256) {
    (uint256 startPrice, uint256 endPrice) = (order.constraints[1], order.constraints[2]);
    
    // @audit can be unchecked, as order.constraints[4] and order.constraints[3] are both uint256, 
    // and checks for [3] <= [4] exist in all the functions that 
    // call _getCurrentPrice()
    //
    // worth to optimize since the code is involved in a root loop
    uint256 duration = order.constraints[4] - order.constraints[3];
    uint256 priceDiff = startPrice > endPrice ? startPrice - endPrice : endPrice - startPrice;
    if (priceDiff == 0 || duration == 0) {
      return startPrice;
    }

    ...
  }

G7

Link: https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L69

see @audit comment:

  function stake(uint256 amount, Duration duration) external override nonReentrant whenNotPaused {
    ...
    
    // @audit can be removed, since `safeTransferFrom` will guarantee 
    // there are enough funds.
    //
    // Can optimize out an external call
    require(IERC20(INFINITY_TOKEN).balanceOf(msg.sender) >= amount, 'insufficient balance to stake');
    
    ...
    
    IERC20(INFINITY_TOKEN).safeTransferFrom(msg.sender, address(this), amount);
   
    ...
  }

G8

Link: https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L92

see @audit comment:

  function changeDuration(
    uint256 amount,
    Duration oldDuration,
    Duration newDuration
  ) external override nonReentrant whenNotPaused {
    ...
     
    // @audit can be removed, since the following checked substraction will
    // guard the underflow
    //
    // can optimize out a few SLOAD opcodes
    require(
      userstakedAmounts[msg.sender][oldDuration].amount >= amount,
      'insufficient staked amount to change duration'
    );
    require(newDuration > oldDuration, 'new duration must be greater than old duration');

    userstakedAmounts[msg.sender][oldDuration].amount -= amount;
    
    ...
  }

G9

Link: https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L65-L73

see @audit comment:

function advanceEpoch() external {
    require(currentEpoch < getMaxEpochs(), 'no epochs left');
    require(block.timestamp >= currentEpochTimestamp + getCliff(), 'cliff not passed');
    require(block.timestamp >= previousEpochTimestamp + getEpochDuration(), 'not ready to advance');

    // @audit all the following calculations can be unchecked, 
    // which is guarded by the previous checks
    uint256 epochsPassedSinceLastAdvance = (block.timestamp - previousEpochTimestamp) / getEpochDuration();
    uint256 epochsLeft = getMaxEpochs() - currentEpoch;
    epochsPassedSinceLastAdvance = epochsPassedSinceLastAdvance > epochsLeft
      ? epochsLeft
      : epochsPassedSinceLastAdvance;

    // update epochs
    currentEpoch += epochsPassedSinceLastAdvance;
    previousEpochTimestamp = block.timestamp;

    ...
  }

G10

Link: https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L132

  function matchOneToOneOrders(
    OrderTypes.MakerOrder[] calldata makerOrders1,
    OrderTypes.MakerOrder[] calldata makerOrders2
  ) external {
    ...
      
    for (uint256 i = 0; i < numMakerOrders; ) {
      ...
        
      require(_complications.contains(makerOrders1[i].execParams[0]), 'invalid complication');
      
       ...
      }
    }
  }

Link: https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L178

  function matchOneToManyOrders(
    OrderTypes.MakerOrder calldata makerOrder,
    OrderTypes.MakerOrder[] calldata manyMakerOrders
  ) external {
    ...
      
    require(_complications.contains(makerOrder.execParams[0]), 'invalid complication');
    
    ...
  }

Link: https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L256

  function matchOrders(
    OrderTypes.MakerOrder[] calldata sells,
    OrderTypes.MakerOrder[] calldata buys,
    OrderTypes.OrderItem[][] calldata constructs
  ) external {
    uint256 startGas = gasleft();
    uint256 numSells = sells.length;
    require(msg.sender == MATCH_EXECUTOR, 'OME');
    require(numSells == buys.length && numSells == constructs.length, 'mismatched lengths');
    
      ...
  }

Functions matchOneToOneOrders and matchOneToManyOrders check whether _complications.contains(makerOrder.execParams[0]) while matchOrders does not.

Essentially, this _complications.contains(makerOrder.execParams[0]) is redundant since each orde will be checked by isOrderValid later. In redundant, the complication will be checked.

Also note that _complications.contains(makerOrder.execParams[0]) is not that gas-friendly. Hence:

• If preferring gas-saving, remove the complication checks in functions matchOneToOneOrders and matchOneToManyOrders.
• If preferring readability (and consistency), add the complication check in function matchOrders

G11

Link:https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L337

See @audit comment:

    if (priceDiff == 0 || duration == 0) {
      return startPrice;
    }
    //@audit can be unchecked, as block.timestamp 
    //>= order.constraints[3] is already called in all 
    //functions that call this function.
    uint256 elapsedTime = block.timestamp - order.constraints[3];
    uint256 PRECISION = 10**4; // precision for division; similar to bps
    uint256 portionBps = elapsedTime > duration ? PRECISION : ((elapsedTime * PRECISION) / duration);
    priceDiff = (priceDiff * portionBps) / PRECISION;
    return startPrice > endPrice ? startPrice - priceDiff : startPrice + priceDiff;

[nneverlander]

Thank you