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

4 stars 0 forks source link

Gas Optimizations #274

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Title: Use Solidity v0.8.4 because of Solidity Custom Errors

Impact

Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Custom errors decrease both deploy and runtime gas costs. Source: https://blog.soliditylang.org/2021/04/21/custom-errors/

OpenZeppelin is also planing to use custom errors starting with next major release 5.0. Source release v5.0: https://github.com/OpenZeppelin/openzeppelin-contracts/issues/2961 Source OZ custom error issue: https://github.com/OpenZeppelin/openzeppelin-contracts/issues/2839

Recommended Mitigation Steps

Refactor code and use Solidity custom errors.


Title: Useless initializations to default values

Impact

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

Recommended Mitigation Steps

Remove explicit initialization for default values.

1 - InfinityExchange.sol#L148 for (uint256 i = 0; i < numMakerOrders; ) {

2 - InfinityExchange.sol#L200 for (uint256 i = 0; i < ordersLength; ) {

3 - InfinityExchange.sol#L219 for (uint256 i = 0; i < ordersLength; ) {

4 - InfinityExchange.sol#L272 for (uint256 i = 0; i < numSells; ) {

5 - InfinityExchange.sol#L308 for (uint256 i = 0; i < numMakerOrders; ) {

6 - InfinityExchange.sol#L349 for (uint256 i = 0; i < ordersLength; ) {

7 - InfinityExchange.sol#L393 for (uint256 i = 0; i < numNonces; ) {

8 - InfinityExchange.sol#L1048 for (uint256 i = 0; i < numNfts; ) {

9 - InfinityExchange.sol#L1086 for (uint256 i = 0; i < numNfts; ) {

10 - InfinityExchange.sol#L1190 for (uint256 i = 0; i < numNfts; ) {

11 - InfinityExchange.sol#L1206 for (uint256 i = 0; i < numTokens; ) {

1 - InfinityOrderBookComplication.sol#L42 bool _isPriceValid = false;

2 - https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityOrderBookComplication.sol#L73-L94 The code is: bool isOrdersTimeValid = true; bool itemsIntersect = true; uint256 ordersLength = manyMakerOrders.length; for (uint256 i = 0; i < ordersLength; ) { if (!isOrdersTimeValid || !itemsIntersect) { return false; // short circuit }

  uint256 nftsLength = manyMakerOrders[i].nfts.length;
  for (uint256 j = 0; j < nftsLength; ) {
    numItems += manyMakerOrders[i].nfts[j].tokens.length;
    unchecked {
      ++j;
    }
  }

  isOrdersTimeValid =
    isOrdersTimeValid &&
    manyMakerOrders[i].constraints[3] <= block.timestamp &&
    manyMakerOrders[i].constraints[4] >= block.timestamp;

  itemsIntersect = itemsIntersect && doItemsIntersect(makerOrder.nfts, manyMakerOrders[i].nfts);

But it should be: bool isOrdersTimeValid; // <———————— removed initial true value bool itemsIntersect; // <———————— removed initial true value uint256 ordersLength = manyMakerOrders.length; for (uint256 i = 0; i < ordersLength; ) { uint256 nftsLength = manyMakerOrders[i].nfts.length; for (uint256 j = 0; j < nftsLength; ) { numItems += manyMakerOrders[i].nfts[j].tokens.length; unchecked { ++j; } }

  isOrdersTimeValid =
    isOrdersTimeValid &&
    manyMakerOrders[i].constraints[3] <= block.timestamp &&
    manyMakerOrders[i].constraints[4] >= block.timestamp;

  itemsIntersect = itemsIntersect && doItemsIntersect(makerOrder.nfts, manyMakerOrders[i].nfts);

  if (!isOrdersTimeValid || !itemsIntersect) { // <——— changed position for short circuit
    return false; // short circuit
  }

3 - InfinityOrderBookComplication.sol#L108 bool _isPriceValid = false;

4 - InfinityOrderBookComplication.sol#L197 uint256 numConstructedItems = 0;

5 - InfinityOrderBookComplication.sol#L199 for (uint256 i = 0; i < nftsLength; ) {

6 - InfinityOrderBookComplication.sol#L214 uint256 numTakerItems = 0;

7 - InfinityOrderBookComplication.sol#L216 for (uint256 i = 0; i < nftsLength; ) {

8 - InfinityOrderBookComplication.sol#L244-L247 uint256 numCollsMatched = 0; // check if taker has all items in maker for (uint256 i = 0; i < order2NftsLength; ) { for (uint256 j = 0; j < order1NftsLength; ) {

9 - InfinityOrderBookComplication.sol#L289-L291 uint256 numTokenIdsPerCollMatched = 0; for (uint256 k = 0; k < item2TokensLength; ) { for (uint256 l = 0; l < item1TokensLength; ) {

10 - InfinityOrderBookComplication.sol#L318 uint256 sum = 0;

11 - InfinityOrderBookComplication.sol#L320 for (uint256 i = 0; i < ordersLength; ) {