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

4 stars 0 forks source link

Gas Optimizations #289

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

The cycle in InfinityExchange performs expensive enough checks more times than it's needed:

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

    for (uint256 i = 0; i < ordersLength; ) {
      require(currency == makerOrders[i].execParams[1], 'cannot mix currencies');
      require(isMakerSeller == makerOrders[i].isSellOrder, 'cannot mix order sides');
      uint256 execPrice = _getCurrentPrice(makerOrders[i]);
      totalPrice += execPrice;
      _takeOrders(makerOrders[i], takerNfts[i], execPrice);
      unchecked {
        ++i;
      }
    }

Recommended Mitigation Steps

The cycle can be rewritten this way:

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

    for (uint256 i = 0; ; ) {
      uint256 execPrice = _getCurrentPrice(makerOrders[i]);
      totalPrice += execPrice;
      _takeOrders(makerOrders[i], takerNfts[i], execPrice);
      unchecked {
        if (++i >= ordersLength) break;
        require(currency == makerOrders[i].execParams[1], 'cannot mix currencies');
        require(isMakerSeller == makerOrders[i].isSellOrder, 'cannot mix order sides');
      }
    }
nneverlander commented 2 years ago

Thanks