Gas Optimizations

[code423n4]

Overview

Risk Rating	Number of issues
Gas Issues	9

Table of Contents:

• 1. Unchecking arithmetics operations that can't underflow/overflow
• 2. >= is cheaper than > (and <= cheaper than <)
• 3. Splitting require() statements that use && saves gas
• 4. Using private rather than public for constants saves gas
• 5. It costs more gas to initialize variables with their default value than letting the default value be applied
• 6. Some variables should be immutable
• 7. Use Custom Errors instead of Revert Strings to save Gas
• 8. Functions guaranteed to revert when called by normal users can be marked payable
• 9. Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)

1. Unchecking arithmetics operations that can't underflow/overflow

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn't possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked block: https://docs.soliditylang.org/en/v0.8.10/control-structures.html#checked-or-unchecked-arithmetic

Consider wrapping with an unchecked block here:

core/InfinityExchange.sol:726:    uint256 remainingAmount = execPrice - protocolFee;
core/InfinityExchange.sol:776:    uint256 remainingAmount = execPrice - protocolFee;
core/InfinityExchange.sol:820:    uint256 remainingAmount = execPrice - protocolFee;
core/InfinityExchange.sol:874:    uint256 remainingAmount = execPrice - protocolFee;
core/InfinityExchange.sol:1136:    uint256 remainingAmount = amount - protocolFee;
core/InfinityExchange.sol:1156:    uint256 priceDiff = startPrice > endPrice ? startPrice - endPrice : endPrice - startPrice;
core/InfinityExchange.sol:1164:    return startPrice > endPrice ? startPrice - priceDiff : startPrice + priceDiff;
staking/InfinityStaker.sol:301:      amount = amount - noVesting;
staking/InfinityStaker.sol:305:        amount = amount - vestedThreeMonths;
staking/InfinityStaker.sol:309:          amount = amount - vestedSixMonths;
token/InfinityToken.sol:65:    uint256 epochsPassedSinceLastAdvance = (block.timestamp - previousEpochTimestamp) / getEpochDuration();

2. >= is cheaper than > (and <= cheaper than <)

Strict inequalities (>) are more expensive than non-strict ones (>=). This is due to some supplementary checks (ISZERO, 3 gas). This also holds true between <= and <.

Consider replacing strict inequalities with non-strict ones to save some gas here:

core/InfinityExchange.sol:1156:    uint256 priceDiff = startPrice > endPrice ? startPrice - endPrice : endPrice - startPrice;
core/InfinityExchange.sol:1162:    uint256 portionBps = elapsedTime > duration ? PRECISION : ((elapsedTime * PRECISION) / duration);
core/InfinityExchange.sol:1164:    return startPrice > endPrice ? startPrice - priceDiff : startPrice + priceDiff;
core/InfinityOrderBookComplication.sol:333:    uint256 priceDiff = startPrice > endPrice ? startPrice - endPrice : endPrice - startPrice;
core/InfinityOrderBookComplication.sol:339:    uint256 portionBps = elapsedTime > duration ? PRECISION : ((elapsedTime * PRECISION) / duration);
core/InfinityOrderBookComplication.sol:341:    return startPrice > endPrice ? startPrice - priceDiff : startPrice + priceDiff;

3. Splitting require() statements that use && saves gas

If you're using the Optimizer at 200, instead of using the && operator in a single require statement to check multiple conditions, Consider using multiple require statements with 1 condition per require statement:

core/InfinityExchange.sol:264:    require(numSells == buys.length && numSells == constructs.length, 'mismatched lengths');
core/InfinityExchange.sol:949:    require(makerOrderValid && executionValid, 'order not verified');

Please, note that this might not hold true at a higher number of runs for the Optimizer (10k). However, it indeed is true at 200.

4. Using private rather than public for constants saves gas

If needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table

token/InfinityToken.sol:25:  bytes32 public constant EPOCH_INFLATION = keccak256('Inflation');
token/InfinityToken.sol:26:  bytes32 public constant EPOCH_DURATION = keccak256('EpochDuration');
token/InfinityToken.sol:27:  bytes32 public constant EPOCH_CLIFF = keccak256('Cliff');
token/InfinityToken.sol:28:  bytes32 public constant MAX_EPOCHS = keccak256('MaxEpochs');
token/TimelockConfig.sol:9:  bytes32 public constant ADMIN = keccak256('Admin');
token/TimelockConfig.sol:10:  bytes32 public constant TIMELOCK = keccak256('Timelock'); 

5. It costs more gas to initialize variables with their default value than letting the default value be applied

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address...). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

As an example: for (uint256 i = 0; i < numIterations; ++i) { should be replaced with for (uint256 i; i < numIterations; ++i) {

Affected code:

core/InfinityExchange.sol:148:    for (uint256 i = 0; i < numMakerOrders; ) {
core/InfinityExchange.sol:200:      for (uint256 i = 0; i < ordersLength; ) {
core/InfinityExchange.sol:219:      for (uint256 i = 0; i < ordersLength; ) {
core/InfinityExchange.sol:272:    for (uint256 i = 0; i < numSells; ) {
core/InfinityExchange.sol:308:    for (uint256 i = 0; i < numMakerOrders; ) {
core/InfinityExchange.sol:349:    for (uint256 i = 0; i < ordersLength; ) {
core/InfinityExchange.sol:393:    for (uint256 i = 0; i < numNonces; ) {
core/InfinityExchange.sol:1048:    for (uint256 i = 0; i < numNfts; ) {
core/InfinityExchange.sol:1086:    for (uint256 i = 0; i < numTokens; ) {
core/InfinityExchange.sol:1109:    for (uint256 i = 0; i < numNfts; ) {
core/InfinityExchange.sol:1190:    for (uint256 i = 0; i < numNfts; ) {
core/InfinityExchange.sol:1206:    for (uint256 i = 0; i < numTokens; ) {
core/InfinityOrderBookComplication.sol:42:    bool _isPriceValid = false;
core/InfinityOrderBookComplication.sol:76:    for (uint256 i = 0; i < ordersLength; ) {
core/InfinityOrderBookComplication.sol:82:      for (uint256 j = 0; j < nftsLength; ) {
core/InfinityOrderBookComplication.sol:108:    bool _isPriceValid = false;
core/InfinityOrderBookComplication.sol:197:    uint256 numConstructedItems = 0;
core/InfinityOrderBookComplication.sol:199:    for (uint256 i = 0; i < nftsLength; ) {
core/InfinityOrderBookComplication.sol:214:    uint256 numTakerItems = 0;
core/InfinityOrderBookComplication.sol:216:    for (uint256 i = 0; i < nftsLength; ) {
core/InfinityOrderBookComplication.sol:244:    uint256 numCollsMatched = 0;
core/InfinityOrderBookComplication.sol:246:    for (uint256 i = 0; i < order2NftsLength; ) {
core/InfinityOrderBookComplication.sol:247:      for (uint256 j = 0; j < order1NftsLength; ) {
core/InfinityOrderBookComplication.sol:289:    uint256 numTokenIdsPerCollMatched = 0;
core/InfinityOrderBookComplication.sol:290:    for (uint256 k = 0; k < item2TokensLength; ) {
core/InfinityOrderBookComplication.sol:291:      for (uint256 l = 0; l < item1TokensLength; ) {
core/InfinityOrderBookComplication.sol:318:    uint256 sum = 0;
core/InfinityOrderBookComplication.sol:320:    for (uint256 i = 0; i < ordersLength; ) {

Consider removing explicit initializations for default values.

6. Some variables should be immutable

These variables are only set in the constructor and are never edited after that:

staking/InfinityStaker.sol:25:  address public INFINITY_TOKEN;

Consider marking them as immutable, as it would avoid the expensive storage-writing operation (around 20 000 gas)

7. Use Custom Errors instead of Revert Strings to save Gas

Solidity 0.8.4 introduced custom errors. They are more gas efficient than revert strings, when it comes to deploy cost as well as runtime cost when the revert condition is met. Use custom errors instead of revert strings for gas savings.

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met)

Source: https://blog.soliditylang.org/2021/04/21/custom-errors/:

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. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

Consider replacing all revert strings with custom errors in the solution.

core/InfinityExchange.sol:138:    require(msg.sender == MATCH_EXECUTOR, 'OME');
core/InfinityExchange.sol:139:    require(numMakerOrders == makerOrders2.length, 'mismatched lengths');
core/InfinityExchange.sol:150:      require(_complications.contains(makerOrders1[i].execParams[0]), 'invalid complication');
core/InfinityExchange.sol:155:      require(canExec, 'cannot execute');
core/InfinityExchange.sol:183:    require(msg.sender == MATCH_EXECUTOR, 'OME');
core/InfinityExchange.sol:184:    require(_complications.contains(makerOrder.execParams[0]), 'invalid complication');
core/InfinityExchange.sol:185:    require(
core/InfinityExchange.sol:190:    require(isOrderValid(makerOrder, makerOrderHash), 'invalid maker order');
core/InfinityExchange.sol:263:    require(msg.sender == MATCH_EXECUTOR, 'OME');
core/InfinityExchange.sol:264:    require(numSells == buys.length && numSells == constructs.length, 'mismatched lengths');
core/InfinityExchange.sol:279:      require(executionValid, 'cannot execute');
core/InfinityExchange.sol:306:      require(currency != address(0), 'offers only in ERC20');
core/InfinityExchange.sol:310:      require(isOrderValid(makerOrders[i], makerOrderHash), 'invalid maker order');
core/InfinityExchange.sol:313:      require(isTimeValid, 'invalid time');
core/InfinityExchange.sol:314:      require(currency == makerOrders[i].execParams[1], 'cannot mix currencies');
core/InfinityExchange.sol:315:      require(isMakerSeller == makerOrders[i].isSellOrder, 'cannot mix order sides');
core/InfinityExchange.sol:326:      require(msg.value >= totalPrice, 'invalid total price');
core/InfinityExchange.sol:342:    require(ordersLength == takerNfts.length, 'mismatched lengths');
core/InfinityExchange.sol:347:      require(currency != address(0), 'offers only in ERC20');
core/InfinityExchange.sol:350:      require(currency == makerOrders[i].execParams[1], 'cannot mix currencies');
core/InfinityExchange.sol:351:      require(isMakerSeller == makerOrders[i].isSellOrder, 'cannot mix order sides');
core/InfinityExchange.sol:362:      require(msg.value >= totalPrice, 'invalid total price');
core/InfinityExchange.sol:380:    require(minNonce > userMinOrderNonce[msg.sender], 'nonce too low');
core/InfinityExchange.sol:381:    require(minNonce < userMinOrderNonce[msg.sender] + 1000000, 'too many');
core/InfinityExchange.sol:392:    require(numNonces > 0, 'cannot be empty');
core/InfinityExchange.sol:394:      require(orderNonces[i] >= userMinOrderNonce[msg.sender], 'nonce too low');
core/InfinityExchange.sol:395:      require(!isUserOrderNonceExecutedOrCancelled[msg.sender][orderNonces[i]], 'nonce already executed or cancelled');
core/InfinityExchange.sol:587:    require(verifyMatchOneToOneOrders(sellOrderHash, buyOrderHash, sell, buy), 'order not verified');
core/InfinityExchange.sol:621:    require(verifyMatchOneToManyOrders(buyOrderHash, false, sell, buy), 'order not verified');
core/InfinityExchange.sol:649:    require(verifyMatchOneToManyOrders(sellOrderHash, true, sell, buy), 'order not verified');
core/InfinityExchange.sol:684:    require(verifyMatchOrders(sellOrderHash, buyOrderHash, sell, buy), 'order not verified');
core/InfinityExchange.sol:949:    require(makerOrderValid && executionValid, 'order not verified');
core/InfinityExchange.sol:1141:      require(sent, 'failed to send ether to seller');
core/InfinityExchange.sol:1231:    require(sent, 'failed');
core/InfinityOrderBookComplication.sol:255:          require(tokenIdsIntersect, 'tokenIds dont intersect');
libs/SignatureChecker.sol:27:    require(
libs/SignatureChecker.sol:32:    require(v == 27 || v == 28, 'Signature: Invalid v parameter');
libs/SignatureChecker.sol:36:    require(signer != address(0), 'Signature: Invalid signer');
staking/InfinityStaker.sol:68:    require(amount != 0, 'stake amount cant be 0');
staking/InfinityStaker.sol:69:    require(IERC20(INFINITY_TOKEN).balanceOf(msg.sender) >= amount, 'insufficient balance to stake');
staking/InfinityStaker.sol:91:    require(amount != 0, 'amount cant be 0');
staking/InfinityStaker.sol:92:    require(
staking/InfinityStaker.sol:96:    require(newDuration > oldDuration, 'new duration must be greater than old duration');
staking/InfinityStaker.sol:117:    require(amount != 0, 'stake amount cant be 0');
staking/InfinityStaker.sol:123:    require(totalVested >= amount, 'insufficient balance to unstake');
staking/InfinityStaker.sol:193:    require(totalStaked >= 0, 'nothing staked to rage quit');
staking/InfinityStaker.sol:347:    require(sent, 'Failed to send Ether');
token/InfinityToken.sol:61:    require(currentEpoch < getMaxEpochs(), 'no epochs left');
token/InfinityToken.sol:62:    require(block.timestamp >= currentEpochTimestamp + getCliff(), 'cliff not passed');
token/InfinityToken.sol:63:    require(block.timestamp >= previousEpochTimestamp + getEpochDuration(), 'not ready to advance');
token/TimelockConfig.sol:39:    require(msg.sender == address(uint160(_config[ADMIN])), 'only admin');
token/TimelockConfig.sol:51:    require(isPending(configId), 'No pending configId found');
token/TimelockConfig.sol:52:    require(block.timestamp >= _pending[configId].timestamp + _config[TIMELOCK], 'too early');
token/TimelockConfig.sol:94:    require(_configSet.contains(configId), 'not config');
token/TimelockConfig.sol:112:    require(_pendingSet.contains(configId), 'not pending');
token/TimelockConfig.sol:119:    require(_pendingSet.add(configId), 'request already exists');
token/TimelockConfig.sol:127:    require(_pendingSet.remove(configId), 'no pending request');

8. Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

core/InfinityExchange.sol:1235:  function addCurrency(address _currency) external onlyOwner {
core/InfinityExchange.sol:1240:  function addComplication(address _complication) external onlyOwner {
core/InfinityExchange.sol:1245:  function removeCurrency(address _currency) external onlyOwner {
core/InfinityExchange.sol:1250:  function removeComplication(address _complication) external onlyOwner {
core/InfinityExchange.sol:1255:  function updateMatchExecutor(address _matchExecutor) external onlyOwner {
core/InfinityExchange.sol:1260:  function updateWethTranferGas(uint32 _wethTransferGasUnits) external onlyOwner {
core/InfinityExchange.sol:1266:  function setProtocolFee(uint16 _protocolFeeBps) external onlyOwner {
staking/InfinityStaker.sol:351:  function updateStakeLevelThreshold(StakeLevel stakeLevel, uint16 threshold) external onlyOwner {
staking/InfinityStaker.sol:375:  function updateInfinityTreasury(address _infinityTreasury) external onlyOwner {
token/TimelockConfig.sol:118:  function requestChange(bytes32 configId, uint256 value) external onlyAdmin {
token/TimelockConfig.sol:126:  function cancelChange(bytes32 configId) external onlyAdmin {

9. Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)

core/InfinityExchange.sol:1161:    uint256 PRECISION = 10**4; // precision for division; similar to bps
core/InfinityOrderBookComplication.sol:338:    uint256 PRECISION = 10**4; // precision for division; similar to bps
staking/InfinityStaker.sol:237:        (userstakedAmounts[user][Duration.TWELVE_MONTHS].amount * 4)) / (10**18);

[nneverlander]

Thank you