Gas Optimizations

[code423n4]

• function transfer in NestedReserve is never used and can only be called by the factory (onlyFactory), so consider removing it because I think the factory uses a withdraw function from the Reserve.

• Currently never used:

  function setReserve onlyFactory

  You can remove it to save some gas, or leave it if it was intended for future use with other factories.

• functions that add or remove operators or shareholders iterate over the whole array, so you can consider using EnumerableSet to store them: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/structs/EnumerableSet.sol

• Could just use msg.sender and do not call an owner() function here:

  function unlockTokens(IERC20 _token) external override onlyOwner {
      ...
      _token.safeTransfer(owner(), amount);

• There are several functions that call _checkMsgValue. This function is quite expensive as it iterates over all the _batchedOrders and is only relevant when the inputToken is ETH. Later the callers will have to iterate over all the _batchedOrders again anyway, so I think this function should be refactored to significantly reduce gas. My suggestion: because processInputOrders and processInputAndOutputOrders both call _processInputOrders, the logic from _checkMsgValue could be moved to _processInputOrders. function create then can be refactored to re-use _processInputOrders. I see 2 discrepancies here: _fromReserve is always false when _submitInOrders is called from create (could be solved if _processInputOrders takes extra parameter), and _processInputOrders has this extra line:

  require(nestedRecords.getAssetReserve(_nftId) == address(reserve), "NF: RESERVE_MISMATCH");

  but this could be solved if you first mint the NFT and then invoke _processInputOrders from create.

• function withdraw calls nestedRecords twice:

  uint256 assetTokensLength = nestedRecords.getAssetTokensLength(_nftId);
  ...
  address token = nestedRecords.getAssetTokens(_nftId)[_tokenIndex];

  I think it could just substitute these links by first fetching all the tokens, and then calculating the length itself instead of making 2 external calls for pretty much the same data.

  • Could use 'unchecked' maths here, as underflow is not possible:

    if (_amountToSpend > amounts[1]) {
    IERC20(_inputToken).safeTransfer(_msgSender(), _amountToSpend - amounts[1]);
    }

[maximebrugel]

"You can remove it to save some gas, or leave it if it was intended for future use with other factories" (Acknowledged)

Consider using EnumerableSet to store operators (Acknowledged)

"Could just use msg.sender and do not call an owner() function here" (Confirmed)

_checkMsgValue refactoring (Disputed)

"Could be". You need to pre-calculate the amount of ETH needed to check msg.value in a simple way.

"function withdraw calls nestedRecords twice" (Acknowledged)

"Could use 'unchecked' maths here, as underflow is not possible" (Confirmed)

[harleythedogC4]

My personal judgments:

1. "function transfer in NestedReserve is never used". Valid and medium-optimization.
2. "EnumerableSet to store them". Valid and small-optimization.
3. "Could just use msg.sender". Valid and small-optimization.
4. "_checkMsgValue refactoring". The idea of refactoring the reserve check to be in the combined function is valid. Valid and small-optimization.
5. "withdraw calls nestedRecords twice". Valid and small-optimization.
6. "Could use 'unchecked' here". This was disputed in other gas reports as this already surfaced in the first contest. To be fair, this should be marked as invalid too. Invalid.

[harleythedogC4]

Also, #66 has the additional issue:

7. "remove this unused variable to save some gas (ETH)". Valid and small-optimization.

[harleythedogC4]

Now, here is the methodology I used for calculating a score for each gas report. I first assigned each submission to be either small-optimization (1 point), medium-optimization (5 points) or large-optimization (10 points), depending on how useful the optimization is. The score of a gas report is the sum of these points, divided by the maximum number of points achieved by a gas report. This maximum number was 10 points, achieved by #67.

The number of points achieved by this report is 10 points. Thus the final score of this gas report is (10/10)*100 = 100.