code-423n4 / 2022-06-infinity-findings

4 stars 0 forks source link

Gas Optimizations #235

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

revert first to save gas

Proof of Concept

https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityOrderBookComplication.sol#L246-L247

    for (uint256 i = 0; i < order2NftsLength; ) {
      for (uint256 j = 0; j < order1NftsLength; ) {

Because all nfts in order2Nfts should be matched. If the for loop of L247 never runs ++numCollsMatched;, then it can return false early.

Recommendation

  function doItemsIntersect(OrderTypes.OrderItem[] calldata order1Nfts, OrderTypes.OrderItem[] calldata order2Nfts)
    public
    pure
    returns (bool)
  {
    uint256 order1NftsLength = order1Nfts.length;
    uint256 order2NftsLength = order2Nfts.length;
    // case where maker/taker didn't specify any items
    if (order1NftsLength == 0 || order2NftsLength == 0) {
      return true;
    }

    uint256 numCollsMatched = 0;
    // check if taker has all items in maker
    for (uint256 i = 0; i < order2NftsLength; ) {
      uint256 j = 0;
      for (; j < order1NftsLength; ) {
        if (order1Nfts[j].collection == order2Nfts[i].collection) {
          // increment numCollsMatched
          unchecked {
            ++numCollsMatched;
          }
          // check if tokenIds intersect
          bool tokenIdsIntersect = doTokenIdsIntersect(order1Nfts[j], order2Nfts[i]);
          require(tokenIdsIntersect, 'tokenIds dont intersect');
          // short circuit
          break;
        }
        unchecked {
          ++j;
        }
      }
      if (j == order1NftsLength) {
          return false;
      }
      unchecked {
        ++i;
      }
    }

    return numCollsMatched == order2NftsLength;
  }