Infinity NFT Marketplace contest - ElKu's results

1. Define the magic numbers as constant. a. (10**18) b. require(minNonce < userMinOrderNonce[msg.sender] + 1000000, 'too many'); c. uint256 protocolFee = (protocolFeeBps * execPrice) / 10000; d. uint256 protocolFee = (protocolFeeBps * execPrice) / 10000; e. uint256 protocolFee = (protocolFeeBps * execPrice) / 10000; f. uint256 protocolFee = (protocolFeeBps * execPrice) / 10000; g. uint256 protocolFee = (PROTOCOL_FEE_BPS * amount) / 10000;

2. The constraints uint256 array inside the MakerOrder structure contains 6 parameters. I would use a struct here instead of a uint array for readability. Several places in the code, the access of important variables happen with an index instead of a human-friendly word. uint256[] constraints;

Similarly address[] execParams; could be spilt into two address type variables named in a reader-friendly way.

3. The new INFINITY_TREASURY is not zero-checked. https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L375

GAS OPTIMIZATION:

1. These two require statements could be put at the top of the for loop. As they don’t need to be calculated like other require statements above them. require(currency == makerOrders[i].execParams[1], 'cannot mix currencies'); require(isMakerSeller == makerOrders[i].isSellOrder, 'cannot mix order sides');

2. a != 0 is more gas friendly than a > 0. a. require(numNonces > 0, 'cannot be empty');

3. https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L210 Reduce the number of comparisons in this function by rewriting it like this:

if (totalPower <= BRONZE_STAKE_THRESHOLD) { return StakeLevel.NONE; } else if (totalPower <= SILVER_STAKE_THRESHOLD) { return StakeLevel.BRONZE; } else if (totalPower <= GOLD_STAKE_THRESHOLD) { return StakeLevel.SILVER; } else if (totalPower <= PLATINUM_STAKE_THRESHOLD) { return StakeLevel.GOLD; } else { return StakeLevel.PLATINUM; }

4. Instead of require statement we can use custom error messages. Many of these errors have the same string, so we could save lot of gas if we use same custom error to replace these.

For example: require(a > 0, "a must be > 0");

can be rewritten as:

if(a == 0) a_must_be_gt0();

References below: a. require(msg.sender == MATCH_EXECUTOR, 'OME'); require(numMakerOrders == makerOrders2.length, 'mismatched lengths'); b. require(_complications.contains(makerOrders1[i].execParams[0]), 'invalid complication'); c. require(canExec, 'cannot execute'); d. require(msg.sender == MATCH_EXECUTOR, 'OME'); require(_complications.contains(makerOrder.execParams[0]), 'invalid complication'); require( IComplication(makerOrder.execParams[0]).canExecMatchOneToMany(makerOrder, manyMakerOrders), 'cannot execute' ); e. require(isOrderValid(makerOrder, makerOrderHash), 'invalid maker order'); f. require(msg.sender == MATCH_EXECUTOR, 'OME'); require(numSells == buys.length && numSells == constructs.length, 'mismatched lengths'); g. require(executionValid, 'cannot execute'); h. require(currency != address(0), 'offers only in ERC20'); i. require(isOrderValid(makerOrders[i], makerOrderHash), 'invalid maker order'); j. require(isTimeValid, 'invalid time'); require(currency == makerOrders[i].execParams[1], 'cannot mix currencies'); require(isMakerSeller == makerOrders[i].isSellOrder, 'cannot mix order sides'); k. require(msg.value >= totalPrice, 'invalid total price'); l. require(ordersLength == takerNfts.length, 'mismatched lengths'); m. require(currency != address(0), 'offers only in ERC20'); n. require(currency == makerOrders[i].execParams[1], 'cannot mix currencies'); require(isMakerSeller == makerOrders[i].isSellOrder, 'cannot mix order sides'); o. require(msg.value >= totalPrice, 'invalid total price'); p. require(minNonce > userMinOrderNonce[msg.sender], 'nonce too low'); require(minNonce < userMinOrderNonce[msg.sender] + 1000000, 'too many'); q. require(numNonces > 0, 'cannot be empty'); r. require(orderNonces[i] >= userMinOrderNonce[msg.sender], 'nonce too low'); require(!isUserOrderNonceExecutedOrCancelled[msg.sender][orderNonces[i]], 'nonce already executed or cancelled'); s. require(verifyMatchOneToOneOrders(sellOrderHash, buyOrderHash, sell, buy), 'order not verified'); t. require(verifyMatchOneToManyOrders(buyOrderHash, false, sell, buy), 'order not verified'); u. require(verifyMatchOneToManyOrders(sellOrderHash, true, sell, buy), 'order not verified'); v. require(verifyMatchOrders(sellOrderHash, buyOrderHash, sell, buy), 'order not verified'); w. require(makerOrderValid && executionValid, 'order not verified'); x. require(sent, 'failed to send ether to seller'); y. require(sent, 'failed'); z. require(amount != 0, 'stake amount cant be 0'); require(IERC20(INFINITY_TOKEN).balanceOf(msg.sender) >= amount, 'insufficient balance to stake'); aa. require(amount != 0, 'amount cant be 0'); require( userstakedAmounts[msg.sender][oldDuration].amount >= amount, 'insufficient staked amount to change duration' ); require(newDuration > oldDuration, 'new duration must be greater than old duration'); bb. require(amount != 0, 'stake amount cant be 0'); cc. require(totalVested >= amount, 'insufficient balance to unstake'); dd. require(totalStaked >= 0, 'nothing staked to rage quit'); ee. require(sent, 'Failed to send Ether');

5. Initializing uint256 to 0 or bool to false is redundant. As its initialized to 0 by default.

a. for (uint256 i = 0; i < numMakerOrders; ) b. for (uint256 i = 0; i < ordersLength; ) c. for (uint256 i = 0; i < ordersLength; ) d. for (uint256 i = 0; i < numSells; ) e. for (uint256 i = 0; i < numMakerOrders; ) f. for (uint256 i = 0; i < ordersLength; ) g. for (uint256 i = 0; i < numNonces; ) h. for (uint256 i = 0; i < numNfts; ) i. for (uint256 i = 0; i < numTokens; ) j. for (uint256 i = 0; i < numNfts; ) { k. for (uint256 i = 0; i < numNfts; ) l. for (uint256 i = 0; i < numTokens; ) m. for (uint256 i = 0; i < ordersLength; ) n. for (uint256 j = 0; j < nftsLength; ) o. for (uint256 i = 0; i < nftsLength; ) p. for (uint256 i = 0; i < nftsLength; ) q. bool _isPriceValid = false; r. bool _isPriceValid = false; s. uint256 numTakerItems = 0; t. uint256 numCollsMatched = 0; u. for (uint256 i = 0; i < order2NftsLength; ) { for (uint256 j = 0; j < order1NftsLength; ) { v. for (uint256 k = 0; k < item2TokensLength; ) { for (uint256 l = 0; l < item1TokensLength; ) { w. uint256 sum = 0; x. for (uint256 i = 0; i < ordersLength; )

6. uint256 portionBps = elapsedTime > duration ? PRECISION : ((elapsedTime * PRECISION) / duration); priceDiff = (priceDiff * portionBps) / PRECISION;

The above two lines can be combined in a single line, thus saving gas.

priceDiff = elapsedTime > duration ? priceDiff : ((elapsedTime * priceDiff ) / duration);

Same thing can be done here too: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityOrderBookComplication.sol#L339-L340

7. The following could be declared as a constant. a. bytes32 ORDER_HASH = 0x7bcfb5a29031e6b8d34ca1a14dd0a1f5cb11b20f755bb2a31ee3c4b143477e4a; b. bytes32 ORDER_ITEM_HASH = 0xf73f37e9f570369ceaab59cef16249ae1c0ad1afd592d656afac0be6f63b87e0; c. bytes32 TOKEN_INFO_HASH = 0x88f0bd19d14f8b5d22c0605a15d9fffc285ebc8c86fb21139456d305982906f1; d. uint256 PRECISION = 10**4;

8. The following arithmetic operations could be placed inside unchecked block. a. uint256 startGasPerOrder = gasleft() + ((startGas + 20000 - gasleft()) / ordersLength); b. uint256 gasCost = (startGas - gasleft() + WETH_TRANSFER_GAS_UNITS) * tx.gasprice; c. uint256 startGasPerOrder = gasleft() + ((startGas - gasleft()) / numSells); d. uint256 protocolFee = (protocolFeeBps * execPrice) / 10000; uint256 remainingAmount = execPrice - protocolFee; e. uint256 gasCost = (startGasPerOrder - gasleft() + wethTransferGasUnits) * tx.gasprice; f. uint256 gasCost = (startGasPerOrder - gasleft() + wethTransferGasUnits) * tx.gasprice; g. uint256 protocolFee = (protocolFeeBps * execPrice) / 10000; uint256 remainingAmount = execPrice - protocolFee; h. uint256 gasCost = (startGasPerOrder - gasleft() + wethTransferGasUnits) * tx.gasprice; i. uint256 protocolFee = (PROTOCOL_FEE_BPS * amount) / 10000; uint256 remainingAmount = amount - protocolFee; j. amount = amount - noVesting; k. amount = amount - vestedThreeMonths; l. amount = amount - vestedSixMonths;

9. The return statement in these functions include a lot of && operations. Instead of doing this all together after all the calculations are done, we could use a series of if statements to not do calculations which are not required. For example return(a && b && c); If c is readily available and a and b need to be calculated then there is no need to calculate a and b before checking the value of c. So using if statements, we can write:

   If(!c) return false; //check this first as its value is already available. //now calculate a If(!a) return false; //now calculate b If(!b) return false; Return true; //if all above fails.

Instances where this could be applied are: a. https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L444 b. https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L474 c. https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L499 d. https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L520 e. https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L949 f. https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityOrderBookComplication.sol#L53 g. https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityOrderBookComplication.sol#L115 h. https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityOrderBookComplication.sol#L205