Infinity NFT Marketplace contest - IllIllI's results

Judge has assessed an item in Issue #187 as High risk. The relevant finding follows:

1. rescueETH() cannot rescue Ether

rescueETH() sends msg.value to the destination address, which means it requires the caller of rescueETH() to provide the Ether to send. Essentially the owner is directly paying the destination address, and the Ether in the contract remains untouched

There is 1 instance of this issue:

File: contracts/staking/InfinityStaker.sol   #1

345    function rescueETH(address destination) external payable onlyOwner {
346      (bool sent, ) = destination.call{value: msg.value}('');
347      require(sent, 'Failed to send Ether');
348:   }

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L345-L348

Summary

Low Risk Issues

Total: 14 instances over 5 issues

Non-critical Issues

Total: 111 instances over 11 issues

Low Risk Issues

1. rescueETH() cannot rescue Ether

rescueETH() sends msg.value to the destination address, which means it requires the caller of rescueETH() to provide the Ether to send. Essentially the owner is directly paying the destination address, and the Ether in the contract remains untouched

There is 1 instance of this issue:

File: contracts/staking/InfinityStaker.sol   #1

345    function rescueETH(address destination) external payable onlyOwner {
346      (bool sent, ) = destination.call{value: msg.value}('');
347      require(sent, 'Failed to send Ether');
348:   }

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L345-L348

2. Vulnerable to cross-chain replay attacks due to static DOMAIN_SEPARATOR

See this issue from a prior contest for details

There is 1 instance of this issue:

File: contracts/core/InfinityExchange.sol   #1

57:     bytes32 public immutable DOMAIN_SEPARATOR;

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

3. Unused/empty receive()/fallback() function

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth)))

There are 4 instances of this issue:

File: contracts/staking/InfinityStaker.sol   #1

55:     fallback() external payable {}

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L55

File: contracts/staking/InfinityStaker.sol   #2

57:     receive() external payable {}

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L57

File: contracts/core/InfinityExchange.sol   #3

119:    fallback() external payable {}

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

File: contracts/core/InfinityExchange.sol   #4

121:    receive() external payable {}

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

4. Missing checks for address(0x0) when assigning values to address state variables

There are 6 instances of this issue:

File: contracts/staking/InfinityStaker.sol

50:       INFINITY_TOKEN = _tokenAddress;

51:       INFINITY_TREASURY = _infinityTreasury;

376:      INFINITY_TREASURY = _infinityTreasury;

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L50

File: contracts/core/InfinityExchange.sol

115:      WETH = _WETH;

116:      MATCH_EXECUTOR = _matchExecutor;

1256:     MATCH_EXECUTOR = _matchExecutor;

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

5. abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead.

If all arguments are strings and or bytes, bytes.concat() should be used instead

There are 2 instances of this issue:

File: contracts/core/InfinityExchange.sol   #1

1197:     bytes32 nftsHash = keccak256(abi.encodePacked(hashes));

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

File: contracts/core/InfinityExchange.sol   #2

1213:     bytes32 tokensHash = keccak256(abi.encodePacked(hashes));

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

Non-critical Issues

1. public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public.

There is 1 instance of this issue:

File: contracts/token/InfinityToken.sol   #1

117:    function getTimelock() public view returns (uint256 timelock) {

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L117

2. constants should be defined rather than using magic numbers

Even assembly can benefit from using readable constants instead of hex/numeric literals

There are 56 instances of this issue:

File: contracts/staking/InfinityStaker.sol

/// @audit 3
236:          (userstakedAmounts[user][Duration.SIX_MONTHS].amount * 3) +

/// @audit 4
237:          (userstakedAmounts[user][Duration.TWELVE_MONTHS].amount * 4)) / (10**18);

/// @audit 18
237:          (userstakedAmounts[user][Duration.TWELVE_MONTHS].amount * 4)) / (10**18);

/// @audit 4
246:      StakeAmount[] memory stakingInfo = new StakeAmount[](4);

/// @audit 3
250:      stakingInfo[3] = userstakedAmounts[user][Duration.TWELVE_MONTHS];

/// @audit 90
277:        return 90 days;

/// @audit 180
279:        return 180 days;

/// @audit 360
281:        return 360 days;

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L236

File: contracts/core/InfinityOrderBookComplication.sol

/// @audit 3
38:       bool _isTimeValid = makerOrder2.constraints[3] <= block.timestamp &&

/// @audit 4
39:         makerOrder2.constraints[4] >= block.timestamp &&

/// @audit 3
40:         makerOrder1.constraints[3] <= block.timestamp &&

/// @audit 4
41:         makerOrder1.constraints[4] >= block.timestamp;

/// @audit 3
91:           manyMakerOrders[i].constraints[3] <= block.timestamp &&

/// @audit 4
92:           manyMakerOrders[i].constraints[4] >= block.timestamp;

/// @audit 3
102:        makerOrder.constraints[3] <= block.timestamp &&

/// @audit 4
103:        makerOrder.constraints[4] >= block.timestamp;

/// @audit 3
160:      return (makerOrder.constraints[3] <= block.timestamp &&

/// @audit 4
161:        makerOrder.constraints[4] >= block.timestamp &&

/// @audit 3
175:        sell.constraints[3] <= block.timestamp &&

/// @audit 4
176:        sell.constraints[4] >= block.timestamp &&

/// @audit 3
177:        buy.constraints[3] <= block.timestamp &&

/// @audit 4
178:        buy.constraints[4] >= block.timestamp;

/// @audit 4
332:      uint256 duration = order.constraints[4] - order.constraints[3];

/// @audit 3
332:      uint256 duration = order.constraints[4] - order.constraints[3];

/// @audit 3
337:      uint256 elapsedTime = block.timestamp - order.constraints[3];

/// @audit 4
338:      uint256 PRECISION = 10**4; // precision for division; similar to bps

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

File: contracts/core/InfinityExchange.sol

/// @audit 20000
202:          uint256 startGasPerOrder = gasleft() + ((startGas + 20000 - gasleft()) / ordersLength);

/// @audit 5
216:        isUserOrderNonceExecutedOrCancelled[makerOrder.signer][makerOrder.constraints[5]] = true;

/// @audit 5
230:        isUserOrderNonceExecutedOrCancelled[makerOrder.signer][makerOrder.constraints[5]] = true;

/// @audit 3
311:        bool isTimeValid = makerOrders[i].constraints[3] <= block.timestamp &&

/// @audit 4
312:          makerOrders[i].constraints[4] >= block.timestamp;

/// @audit 1000000
381:      require(minNonce < userMinOrderNonce[msg.sender] + 1000000, 'too many');

/// @audit 5
515:      bool orderExpired = isUserOrderNonceExecutedOrCancelled[order.signer][order.constraints[5]] ||

/// @audit 5
516:        order.constraints[5] < userMinOrderNonce[order.signer];

/// @audit 5
723:      isUserOrderNonceExecutedOrCancelled[sell.signer][sell.constraints[5]] = true;

/// @audit 5
724:      isUserOrderNonceExecutedOrCancelled[buy.signer][buy.constraints[5]] = true;

/// @audit 10000
725:      uint256 protocolFee = (protocolFeeBps * execPrice) / 10000;

/// @audit 5
774:      isUserOrderNonceExecutedOrCancelled[buy.signer][buy.constraints[5]] = true;

/// @audit 10000
775:      uint256 protocolFee = (protocolFeeBps * execPrice) / 10000;

/// @audit 5
818:      isUserOrderNonceExecutedOrCancelled[sell.signer][sell.constraints[5]] = true;

/// @audit 10000
819:      uint256 protocolFee = (protocolFeeBps * execPrice) / 10000;

/// @audit 10000
873:      uint256 protocolFee = (protocolFeeBps * execPrice) / 10000;

/// @audit 5
878:        sell.constraints[5],

/// @audit 5
879:        buy.constraints[5],

/// @audit 5
970:      isUserOrderNonceExecutedOrCancelled[makerOrder.signer][makerOrder.constraints[5]] = true;

/// @audit 5
995:      isUserOrderNonceExecutedOrCancelled[makerOrder.signer][makerOrder.constraints[5]] = true;

/// @audit 0x80ac58cd
1067:     if (IERC165(item.collection).supportsInterface(0x80ac58cd)) {

/// @audit 0xd9b67a26
1069:     } else if (IERC165(item.collection).supportsInterface(0xd9b67a26)) {

/// @audit 10000
1135:     uint256 protocolFee = (PROTOCOL_FEE_BPS * amount) / 10000;

/// @audit 4
1155:     uint256 duration = order.constraints[4] - order.constraints[3];

/// @audit 3
1155:     uint256 duration = order.constraints[4] - order.constraints[3];

/// @audit 3
1160:     uint256 elapsedTime = block.timestamp - order.constraints[3];

/// @audit 4
1161:     uint256 PRECISION = 10**4; // precision for division; similar to bps

/// @audit 0x7bcfb5a29031e6b8d34ca1a14dd0a1f5cb11b20f755bb2a31ee3c4b143477e4a
1170:     bytes32 ORDER_HASH = 0x7bcfb5a29031e6b8d34ca1a14dd0a1f5cb11b20f755bb2a31ee3c4b143477e4a;

/// @audit 0xf73f37e9f570369ceaab59cef16249ae1c0ad1afd592d656afac0be6f63b87e0
1187:     bytes32 ORDER_ITEM_HASH = 0xf73f37e9f570369ceaab59cef16249ae1c0ad1afd592d656afac0be6f63b87e0;

/// @audit 0x88f0bd19d14f8b5d22c0605a15d9fffc285ebc8c86fb21139456d305982906f1
1203:     bytes32 TOKEN_INFO_HASH = 0x88f0bd19d14f8b5d22c0605a15d9fffc285ebc8c86fb21139456d305982906f1;

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

3. Large multiples of ten should use scientific notation (e.g. 1e6) rather than decimal literals (e.g. 1000000), for readability

There is 1 instance of this issue:

File: contracts/core/InfinityExchange.sol   #1

381:      require(minNonce < userMinOrderNonce[msg.sender] + 1000000, 'too many');

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

4. Missing event and timelock for critical parameter change

Events help non-contract tools to track changes, and events prevent users from being surprised by changes

There are 4 instances of this issue:

File: contracts/staking/InfinityStaker.sol   #1

351     function updateStakeLevelThreshold(StakeLevel stakeLevel, uint16 threshold) external onlyOwner {
352       if (stakeLevel == StakeLevel.BRONZE) {
353         BRONZE_STAKE_THRESHOLD = threshold;
354       } else if (stakeLevel == StakeLevel.SILVER) {
355         SILVER_STAKE_THRESHOLD = threshold;
356       } else if (stakeLevel == StakeLevel.GOLD) {
357         GOLD_STAKE_THRESHOLD = threshold;
358       } else if (stakeLevel == StakeLevel.PLATINUM) {
359         PLATINUM_STAKE_THRESHOLD = threshold;
360       }
361:    }

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L351-L361

File: contracts/staking/InfinityStaker.sol   #2

364     function updatePenalties(
365       uint16 threeMonthPenalty,
366       uint16 sixMonthPenalty,
367       uint16 twelveMonthPenalty
368     ) external onlyOwner {
369       THREE_MONTH_PENALTY = threeMonthPenalty;
370       SIX_MONTH_PENALTY = sixMonthPenalty;
371       TWELVE_MONTH_PENALTY = twelveMonthPenalty;
372:    }

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L364-L372

File: contracts/staking/InfinityStaker.sol   #3

375     function updateInfinityTreasury(address _infinityTreasury) external onlyOwner {
376       INFINITY_TREASURY = _infinityTreasury;
377:    }

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L375-L377

File: contracts/core/InfinityExchange.sol   #4

1255    function updateMatchExecutor(address _matchExecutor) external onlyOwner {
1256      MATCH_EXECUTOR = _matchExecutor;
1257:   }

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

5. Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

There are 4 instances of this issue:

File: contracts/token/InfinityToken.sol   #1

25:     bytes32 public constant EPOCH_INFLATION = keccak256('Inflation');

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L25

File: contracts/token/InfinityToken.sol   #2

26:     bytes32 public constant EPOCH_DURATION = keccak256('EpochDuration');

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L26

File: contracts/token/InfinityToken.sol   #3

27:     bytes32 public constant EPOCH_CLIFF = keccak256('Cliff');

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L27

File: contracts/token/InfinityToken.sol   #4

28:     bytes32 public constant MAX_EPOCHS = keccak256('MaxEpochs');

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L28

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

There are 3 instances of this issue:

File: contracts/staking/InfinityStaker.sol   #1

237:          (userstakedAmounts[user][Duration.TWELVE_MONTHS].amount * 4)) / (10**18);

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L237

File: contracts/core/InfinityOrderBookComplication.sol   #2

338:      uint256 PRECISION = 10**4; // precision for division; similar to bps

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

File: contracts/core/InfinityExchange.sol   #3

1161:     uint256 PRECISION = 10**4; // precision for division; similar to bps

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

7. Inconsistent spacing in comments

Some lines use // x and some use //x. The instances below point out the usages that don't follow the majority, within each file

There are 3 instances of this issue:

File: contracts/staking/InfinityStaker.sol   #1

22:     ///@dev Storage variable to keep track of the staker's staked duration and amounts

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L22

File: contracts/staking/InfinityStaker.sol   #2

26:     ///@dev Infinity treasury address - will be a EOA/multisig

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L26

File: contracts/staking/InfinityStaker.sol   #3

38:     ///@dev Penalties if staked tokens are rageQuit early. Example: If 100 tokens are staked for twelve months but rageQuit right away,

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L38

8. Variable names that consist of all capital letters should be reserved for constant/immutable variables

If the variable needs to be different based on which class it comes from, a view/pure function should be used instead (e.g. like this).

There are 18 instances of this issue:

File: contracts/staking/InfinityStaker.sol

25:     address public INFINITY_TOKEN;

27:     address public INFINITY_TREASURY;

33:     uint16 public BRONZE_STAKE_THRESHOLD = 1000;

34:     uint16 public SILVER_STAKE_THRESHOLD = 5000;

35:     uint16 public GOLD_STAKE_THRESHOLD = 10000;

36:     uint16 public PLATINUM_STAKE_THRESHOLD = 20000;

40:     uint16 public THREE_MONTH_PENALTY = 2;

41:     uint16 public SIX_MONTH_PENALTY = 3;

42:     uint16 public TWELVE_MONTH_PENALTY = 4;

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L25

File: contracts/core/InfinityOrderBookComplication.sol

338:      uint256 PRECISION = 10**4; // precision for division; similar to bps

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

File: contracts/core/InfinityExchange.sol

59:     address public MATCH_EXECUTOR;

61:     uint32 public WETH_TRANSFER_GAS_UNITS = 50000;

63:     uint16 public PROTOCOL_FEE_BPS = 250;

104:    constructor(address _WETH, address _matchExecutor) {

1161:     uint256 PRECISION = 10**4; // precision for division; similar to bps

1170:     bytes32 ORDER_HASH = 0x7bcfb5a29031e6b8d34ca1a14dd0a1f5cb11b20f755bb2a31ee3c4b143477e4a;

1187:     bytes32 ORDER_ITEM_HASH = 0xf73f37e9f570369ceaab59cef16249ae1c0ad1afd592d656afac0be6f63b87e0;

1203:     bytes32 TOKEN_INFO_HASH = 0x88f0bd19d14f8b5d22c0605a15d9fffc285ebc8c86fb21139456d305982906f1;

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

9. NatSpec is incomplete

There are 4 instances of this issue:

File: contracts/core/InfinityExchange.sol   #1

/// @audit Missing: '@param verifySellOrder'
452     /**
453      * @notice Checks whether orders are valid
454      * @dev Checks whether currencies match, sides match, complications match and if each order is valid (see isOrderValid)
455      * @param orderHash hash of the order
456      * @param sell the sell order
457      * @param buy the buy order
458      * @return whether orders are valid
459      */
460     function verifyMatchOneToManyOrders(
461       bytes32 orderHash,
462       bool verifySellOrder,
463       OrderTypes.MakerOrder calldata sell,
464       OrderTypes.MakerOrder calldata buy
465:    ) public view returns (bool) {

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

File: contracts/core/InfinityExchange.sol   #2

/// @audit Missing: '@return'
512      * @param orderHash computed hash of the order
513      */
514:    function isOrderValid(OrderTypes.MakerOrder calldata order, bytes32 orderHash) public view returns (bool) {

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

File: contracts/core/InfinityExchange.sol   #3

/// @audit Missing: '@return'
640      * @param protocolFeeBps exchange fee
641      */
642     function _matchOneMakerBuyToManyMakerSells(
643       bytes32 buyOrderHash,
644       OrderTypes.MakerOrder calldata sell,
645       OrderTypes.MakerOrder calldata buy,
646       uint16 protocolFeeBps
647:    ) internal returns (uint256) {

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

File: contracts/core/InfinityExchange.sol   #4

/// @audit Missing: '@param execPrice'
661     /**
662      * @notice Internal helper function to match orders specified via a higher order intent
663      * @param sell the sell order
664      * @param buy the buy order
665      * @param constructedNfts the nfts constructed by an off chain matching that are guaranteed to intersect
666               with the user specified signed intents (orders)
667      * @param startGasPerOrder start gas when this order started execution
668      * @param protocolFeeBps exchange fee
669      * @param wethTransferGasUnits gas units that a WETH transfer will use
670      * @param weth WETH address
671      */
672     function _matchOrders(
673       OrderTypes.MakerOrder calldata sell,
674       OrderTypes.MakerOrder calldata buy,
675       OrderTypes.OrderItem[] calldata constructedNfts,
676       uint256 startGasPerOrder,
677       uint256 execPrice,
678       uint16 protocolFeeBps,
679       uint32 wethTransferGasUnits,
680:      address weth

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

10. Event is missing indexed fields

Each event should use three indexed fields if there are three or more fields

There are 11 instances of this issue:

File: contracts/token/InfinityToken.sol

35:     event EpochAdvanced(uint256 currentEpoch, uint256 supplyMinted);

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L35

File: contracts/staking/InfinityStaker.sol

44:     event Staked(address indexed user, uint256 amount, Duration duration);

45:     event DurationChanged(address indexed user, uint256 amount, Duration oldDuration, Duration newDuration);

46:     event UnStaked(address indexed user, uint256 amount);

47:     event RageQuit(address indexed user, uint256 totalToUser, uint256 penalty);

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L44

File: contracts/core/InfinityExchange.sol

80:     event CancelAllOrders(address user, uint256 newMinNonce);

81:     event CancelMultipleOrders(address user, uint256[] orderNonces);

82:     event NewWethTransferGasUnits(uint32 wethTransferGasUnits);

83:     event NewProtocolFee(uint16 protocolFee);

85      event MatchOrderFulfilled(
86        bytes32 sellOrderHash,
87        bytes32 buyOrderHash,
88        address seller,
89        address buyer,
90        address complication, // address of the complication that defines the execution
91        address currency, // token address of the transacting currency
92        uint256 amount // amount spent on the order
93:     );

95      event TakeOrderFulfilled(
96        bytes32 orderHash,
97        address seller,
98        address buyer,
99        address complication, // address of the complication that defines the execution
100       address currency, // token address of the transacting currency
101       uint256 amount // amount spent on the order
102:    );

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

11. Not using the named return variables anywhere in the function is confusing

Consider changing the variable to be an unnamed one

There are 6 instances of this issue:

File: contracts/token/InfinityToken.sol

/// @audit admin
113:    function getAdmin() public view returns (address admin) {

/// @audit timelock
117:    function getTimelock() public view returns (uint256 timelock) {

/// @audit inflation
121:    function getInflation() public view returns (uint256 inflation) {

/// @audit cliff
125:    function getCliff() public view returns (uint256 cliff) {

/// @audit totalEpochs
129:    function getMaxEpochs() public view returns (uint256 totalEpochs) {

/// @audit epochDuration
133:    function getEpochDuration() public view returns (uint256 epochDuration) {

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L113

Summary

Gas Optimizations

Total: 199 instances over 19 issues

Gas Optimizations

1. Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.

There is 1 instance of this issue:

File: contracts/core/InfinityExchange.sol   #1

70      mapping(address => uint256) public userMinOrderNonce;
71    
72      /// @dev This records already executed or cancelled orders to prevent replay attacks.
73:     mapping(address => mapping(uint256 => bool)) public isUserOrderNonceExecutedOrCancelled;

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

2. State variables only set in the constructor should be declared immutable

Avoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmacces (100 gas) with a PUSH32 (3 gas).

There are 2 instances of this issue:

File: contracts/token/InfinityToken.sol   #1

31:     uint256 public currentEpochTimestamp;

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L31

File: contracts/staking/InfinityStaker.sol   #2

25:     address public INFINITY_TOKEN;

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L25

3. State variables can be packed into fewer storage slots

If variables occupying the same slot are both written the same function or by the constructor, avoids a separate Gsset (20000 gas). Reads of the variables can also be cheaper

There is 1 instance of this issue:

File: contracts/staking/InfinityStaker.sol   #1

/// @audit Variable ordering with 3 slots instead of the current 4:
/// @audit  mapping(32):userstakedAmounts, address(20):INFINITY_TOKEN, uint16(2):BRONZE_STAKE_THRESHOLD, uint16(2):SILVER_STAKE_THRESHOLD, uint16(2):GOLD_STAKE_THRESHOLD, uint16(2):PLATINUM_STAKE_THRESHOLD, uint16(2):THREE_MONTH_PENALTY, uint16(2):SIX_MONTH_PENALTY, address(20):INFINITY_TREASURY, uint16(2):TWELVE_MONTH_PENALTY
23:     mapping(address => mapping(Duration => StakeAmount)) public userstakedAmounts;

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L23

4. State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. Caching of a state variable replace each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.

There are 9 instances of this issue:

File: contracts/token/InfinityToken.sol

/// @audit currentEpoch on line 61
66:       uint256 epochsLeft = getMaxEpochs() - currentEpoch;

/// @audit currentEpoch on line 72
81:       emit EpochAdvanced(currentEpoch, supplyToMint);

/// @audit previousEpochTimestamp on line 63
65:       uint256 epochsPassedSinceLastAdvance = (block.timestamp - previousEpochTimestamp) / getEpochDuration();

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L66

File: contracts/staking/InfinityStaker.sol

/// @audit INFINITY_TOKEN on line 69
74:       IERC20(INFINITY_TOKEN).safeTransferFrom(msg.sender, address(this), amount);

/// @audit INFINITY_TOKEN on line 141
142:      IERC20(INFINITY_TOKEN).safeTransfer(INFINITY_TREASURY, penalty);

/// @audit BRONZE_STAKE_THRESHOLD on line 213
215:      } else if (totalPower > BRONZE_STAKE_THRESHOLD && totalPower <= SILVER_STAKE_THRESHOLD) {

/// @audit SILVER_STAKE_THRESHOLD on line 215
217:      } else if (totalPower > SILVER_STAKE_THRESHOLD && totalPower <= GOLD_STAKE_THRESHOLD) {

/// @audit GOLD_STAKE_THRESHOLD on line 217
219:      } else if (totalPower > GOLD_STAKE_THRESHOLD && totalPower <= PLATINUM_STAKE_THRESHOLD) {

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L74

File: contracts/core/InfinityExchange.sol

/// @audit WETH_TRANSFER_GAS_UNITS on line 197
231:        uint256 gasCost = (startGas - gasleft() + WETH_TRANSFER_GAS_UNITS) * tx.gasprice;

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

5. Multiple accesses of a mapping/array should use a local variable cache

The instances below point to the second+ access of a value inside a mapping/array, within a function. Caching a mapping's value in a local storage or calldata variable when the value is accessed multiple times, saves ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations. Caching an array's struct avoids recalculating the array offsets into memory/calldata

There are 13 instances of this issue:

File: contracts/core/InfinityOrderBookComplication.sol

/// @audit manyMakerOrders[i] on line 81
83:           numItems += manyMakerOrders[i].nfts[j].tokens.length;

/// @audit manyMakerOrders[i] on line 83
91:           manyMakerOrders[i].constraints[3] <= block.timestamp &&

/// @audit manyMakerOrders[i] on line 91
92:           manyMakerOrders[i].constraints[4] >= block.timestamp;

/// @audit manyMakerOrders[i] on line 92
94:         itemsIntersect = itemsIntersect && doItemsIntersect(makerOrder.nfts, manyMakerOrders[i].nfts);

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

File: contracts/core/InfinityExchange.sol

/// @audit makerOrders1[i] on line 150
151:        (bool canExec, uint256 execPrice) = IComplication(makerOrders1[i].execParams[0]).canExecMatchOneToOne(

/// @audit makerOrders[0] on line 303
304:      bool isMakerSeller = makerOrders[0].isSellOrder;

/// @audit makerOrders[i] on line 311
312:          makerOrders[i].constraints[4] >= block.timestamp;

/// @audit makerOrders[i] on line 312
314:        require(currency == makerOrders[i].execParams[1], 'cannot mix currencies');

/// @audit makerOrders[i] on line 314
315:        require(isMakerSeller == makerOrders[i].isSellOrder, 'cannot mix order sides');

/// @audit makerOrders[0] on line 344
345:      bool isMakerSeller = makerOrders[0].isSellOrder;

/// @audit makerOrders[i] on line 350
351:        require(isMakerSeller == makerOrders[i].isSellOrder, 'cannot mix order sides');

/// @audit nfts[i] on line 1191
1191:       bytes32 hash = keccak256(abi.encode(ORDER_ITEM_HASH, nfts[i].collection, _tokensHash(nfts[i].tokens)));

/// @audit tokens[i] on line 1207
1207:       bytes32 hash = keccak256(abi.encode(TOKEN_INFO_HASH, tokens[i].tokenId, tokens[i].numTokens));

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

6. internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

There are 22 instances of this issue:

File: contracts/staking/InfinityStaker.sol

275:    function _getDurationInSeconds(Duration duration) internal pure returns (uint256) {

290     function _updateUserStakedAmounts(
291       address user,
292       uint256 amount,
293       uint256 noVesting,
294       uint256 vestedThreeMonths,
295       uint256 vestedSixMonths,
296:      uint256 vestedTwelveMonths

328:    function _clearUserStakedAmounts(address user) internal {

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L275

File: contracts/core/InfinityOrderBookComplication.sol

317:    function _sumCurrentPrices(OrderTypes.MakerOrder[] calldata orders) internal view returns (uint256) {

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

File: contracts/core/InfinityExchange.sol

574     function _matchOneToOneOrders(
575       OrderTypes.MakerOrder calldata makerOrder1,
576       OrderTypes.MakerOrder calldata makerOrder2,
577       uint256 startGasPerOrder,
578       uint256 execPrice,
579       uint16 protocolFeeBps,
580       uint32 wethTransferGasUnits,
581:      address weth

611     function _matchOneMakerSellToManyMakerBuys(
612       bytes32 sellOrderHash,
613       OrderTypes.MakerOrder calldata sell,
614       OrderTypes.MakerOrder calldata buy,
615       uint256 startGasPerOrder,
616       uint16 protocolFeeBps,
617       uint32 wethTransferGasUnits,
618:      address weth

642     function _matchOneMakerBuyToManyMakerSells(
643       bytes32 buyOrderHash,
644       OrderTypes.MakerOrder calldata sell,
645       OrderTypes.MakerOrder calldata buy,
646       uint16 protocolFeeBps
647:    ) internal returns (uint256) {

672     function _matchOrders(
673       OrderTypes.MakerOrder calldata sell,
674       OrderTypes.MakerOrder calldata buy,
675       OrderTypes.OrderItem[] calldata constructedNfts,
676       uint256 startGasPerOrder,
677       uint256 execPrice,
678       uint16 protocolFeeBps,
679       uint32 wethTransferGasUnits,
680:      address weth

712     function _execMatchOneToOneOrders(
713       bytes32 sellOrderHash,
714       bytes32 buyOrderHash,
715       OrderTypes.MakerOrder calldata sell,
716       OrderTypes.MakerOrder calldata buy,
717       uint256 startGasPerOrder,
718       uint256 execPrice,
719       uint16 protocolFeeBps,
720       uint32 wethTransferGasUnits,
721:      address weth

763     function _execMatchOneMakerSellToManyMakerBuys(
764       bytes32 sellOrderHash,
765       bytes32 buyOrderHash,
766       OrderTypes.MakerOrder calldata sell,
767       OrderTypes.MakerOrder calldata buy,
768       uint256 startGasPerOrder,
769       uint256 execPrice,
770       uint16 protocolFeeBps,
771       uint32 wethTransferGasUnits,
772:      address weth

810     function _execMatchOneMakerBuyToManyMakerSells(
811       bytes32 sellOrderHash,
812       bytes32 buyOrderHash,
813       OrderTypes.MakerOrder calldata sell,
814       OrderTypes.MakerOrder calldata buy,
815       uint256 execPrice,
816       uint16 protocolFeeBps
817:    ) internal returns (uint256) {

861     function _execMatchOrders(
862       bytes32 sellOrderHash,
863       bytes32 buyOrderHash,
864       OrderTypes.MakerOrder calldata sell,
865       OrderTypes.MakerOrder calldata buy,
866       OrderTypes.OrderItem[] calldata constructedNfts,
867       uint256 startGasPerOrder,
868       uint256 execPrice,
869       uint16 protocolFeeBps,
870       uint32 wethTransferGasUnits,
871:      address weth

905     function _execMatchOrder(
906       address seller,
907       address buyer,
908       uint256 sellNonce,
909       uint256 buyNonce,
910       OrderTypes.OrderItem[] calldata constructedNfts,
911       address currency,
912:      uint256 amount

941     function _takeOrders(
942       OrderTypes.MakerOrder calldata makerOrder,
943       OrderTypes.OrderItem[] calldata takerItems,
944:      uint256 execPrice

963     function _execTakeOrders(
964       bytes32 makerOrderHash,
965       OrderTypes.MakerOrder calldata makerOrder,
966       OrderTypes.OrderItem[] calldata takerItems,
967       bool isMakerSeller,
968:      uint256 execPrice

989     function _execTakeOneOrder(
990       bytes32 makerOrderHash,
991       OrderTypes.MakerOrder calldata makerOrder,
992       bool isMakerSeller,
993:      uint256 execPrice

1062    function _transferNFTs(
1063      address from,
1064      address to,
1065:     OrderTypes.OrderItem calldata item

1080    function _transferERC721s(
1081      address from,
1082      address to,
1083:     OrderTypes.OrderItem calldata item

1101    function _transferERC1155s(
1102      address from,
1103      address to,
1104:     OrderTypes.OrderItem calldata item

1128    function _transferFees(
1129      address seller,
1130      address buyer,
1131      uint256 amount,
1132:     address currency

1185:   function _nftsHash(OrderTypes.OrderItem[] calldata nfts) internal pure returns (bytes32) {

1201:   function _tokensHash(OrderTypes.TokenInfo[] calldata tokens) internal pure returns (bytes32) {

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

7. Add unchecked {} for subtractions where the operands cannot underflow because of a previous require() or if-statement

require(a <= b); x = b - a => require(a <= b); unchecked { x = b - a }

There are 3 instances of this issue:

File: contracts/staking/InfinityStaker.sol   #1

/// @audit if-condition on line 298
301:        amount = amount - noVesting;

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L301

File: contracts/staking/InfinityStaker.sol   #2

/// @audit if-condition on line 302
305:          amount = amount - vestedThreeMonths;

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L305

File: contracts/staking/InfinityStaker.sol   #3

/// @audit if-condition on line 306
309:            amount = amount - vestedSixMonths;

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L309

8. require()/revert() strings longer than 32 bytes cost extra gas

Each extra memory word of bytes past the original 32 incurs an MSTORE which costs 3 gas

There are 3 instances of this issue:

File: contracts/staking/InfinityStaker.sol   #1

92        require(
93          userstakedAmounts[msg.sender][oldDuration].amount >= amount,
94          'insufficient staked amount to change duration'
95:       );

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L92-L95

File: contracts/staking/InfinityStaker.sol   #2

96:       require(newDuration > oldDuration, 'new duration must be greater than old duration');

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L96

File: contracts/core/InfinityExchange.sol   #3

395:        require(!isUserOrderNonceExecutedOrCancelled[msg.sender][orderNonces[i]], 'nonce already executed or cancelled');

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

9. Using bools for storage incurs overhead

    // Booleans are more expensive than uint256 or any type that takes up a full
    // word because each write operation emits an extra SLOAD to first read the
    // slot's contents, replace the bits taken up by the boolean, and then write
    // back. This is the compiler's defense against contract upgrades and
    // pointer aliasing, and it cannot be disabled.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from 'false' to 'true', after having been 'true' in the past

There is 1 instance of this issue:

File: contracts/core/InfinityExchange.sol   #1

73:     mapping(address => mapping(uint256 => bool)) public isUserOrderNonceExecutedOrCancelled;

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

10. Using > 0 costs more gas than != 0 when used on a uint in a require() statement

This change saves 6 gas per instance. The optimization works until solidity version 0.8.13 where there is a regression in gas costs.

There is 1 instance of this issue:

File: contracts/core/InfinityExchange.sol   #1

392:      require(numNonces > 0, 'cannot be empty');

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

11. >= costs less gas than >

The compiler uses opcodes GT and ISZERO for solidity code that uses >, but only requires LT for >=, which saves 3 gas

There is 1 instance of this issue:

File: contracts/token/InfinityToken.sol   #1

67        epochsPassedSinceLastAdvance = epochsPassedSinceLastAdvance > epochsLeft
68          ? epochsLeft
69:         : epochsPassedSinceLastAdvance;

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L67-L69

12. It costs more gas to initialize non-constant/non-immutable variables to zero than to let the default of zero be applied

Not overwriting the default for stack variables saves 8 gas. Storage and memory variables have larger savings

There are 26 instances of this issue:

File: contracts/core/InfinityOrderBookComplication.sol

76:       for (uint256 i = 0; i < ordersLength; ) {

82:         for (uint256 j = 0; j < nftsLength; ) {

197:      uint256 numConstructedItems = 0;

199:      for (uint256 i = 0; i < nftsLength; ) {

214:      uint256 numTakerItems = 0;

216:      for (uint256 i = 0; i < nftsLength; ) {

244:      uint256 numCollsMatched = 0;

246:      for (uint256 i = 0; i < order2NftsLength; ) {

247:        for (uint256 j = 0; j < order1NftsLength; ) {

289:      uint256 numTokenIdsPerCollMatched = 0;

290:      for (uint256 k = 0; k < item2TokensLength; ) {

291:        for (uint256 l = 0; l < item1TokensLength; ) {

318:      uint256 sum = 0;

320:      for (uint256 i = 0; i < ordersLength; ) {

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

File: contracts/core/InfinityExchange.sol

148:      for (uint256 i = 0; i < numMakerOrders; ) {

200:        for (uint256 i = 0; i < ordersLength; ) {

219:        for (uint256 i = 0; i < ordersLength; ) {

272:      for (uint256 i = 0; i < numSells; ) {

308:      for (uint256 i = 0; i < numMakerOrders; ) {

349:      for (uint256 i = 0; i < ordersLength; ) {

393:      for (uint256 i = 0; i < numNonces; ) {

1048:     for (uint256 i = 0; i < numNfts; ) {

1086:     for (uint256 i = 0; i < numTokens; ) {

1109:     for (uint256 i = 0; i < numNfts; ) {

1190:     for (uint256 i = 0; i < numNfts; ) {

1206:     for (uint256 i = 0; i < numTokens; ) {

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

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

See this issue which describes the fact that there is a larger deployment gas cost, but with enough runtime calls, the change ends up being cheaper

There are 2 instances of this issue:

File: contracts/core/InfinityExchange.sol   #1

264:      require(numSells == buys.length && numSells == constructs.length, 'mismatched lengths');

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

File: contracts/core/InfinityExchange.sol   #2

949:      require(makerOrderValid && executionValid, 'order not verified');

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

14. Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed

There are 39 instances of this issue:

File: contracts/staking/InfinityStaker.sol

33:     uint16 public BRONZE_STAKE_THRESHOLD = 1000;

34:     uint16 public SILVER_STAKE_THRESHOLD = 5000;

35:     uint16 public GOLD_STAKE_THRESHOLD = 10000;

36:     uint16 public PLATINUM_STAKE_THRESHOLD = 20000;

40:     uint16 public THREE_MONTH_PENALTY = 2;

41:     uint16 public SIX_MONTH_PENALTY = 3;

42:     uint16 public TWELVE_MONTH_PENALTY = 4;

351:    function updateStakeLevelThreshold(StakeLevel stakeLevel, uint16 threshold) external onlyOwner {

365:      uint16 threeMonthPenalty,

366:      uint16 sixMonthPenalty,

367:      uint16 twelveMonthPenalty

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L33

File: contracts/core/InfinityExchange.sol

61:     uint32 public WETH_TRANSFER_GAS_UNITS = 50000;

63:     uint16 public PROTOCOL_FEE_BPS = 250;

82:     event NewWethTransferGasUnits(uint32 wethTransferGasUnits);

83:     event NewProtocolFee(uint16 protocolFee);

145:      uint16 protocolFeeBps = PROTOCOL_FEE_BPS;

146:      uint32 wethTransferGasUnits = WETH_TRANSFER_GAS_UNITS;

196:      uint16 protocolFeeBps = PROTOCOL_FEE_BPS;

197:      uint32 wethTransferGasUnits = WETH_TRANSFER_GAS_UNITS;

269:      uint16 protocolFeeBps = PROTOCOL_FEE_BPS;

270:      uint32 wethTransferGasUnits = WETH_TRANSFER_GAS_UNITS;

423:      (bytes32 r, bytes32 s, uint8 v) = abi.decode(order.sig, (bytes32, bytes32, uint8));

518:      (bytes32 r, bytes32 s, uint8 v) = abi.decode(order.sig, (bytes32, bytes32, uint8));

579:      uint16 protocolFeeBps,

580:      uint32 wethTransferGasUnits,

616:      uint16 protocolFeeBps,

617:      uint32 wethTransferGasUnits,

646:      uint16 protocolFeeBps

678:      uint16 protocolFeeBps,

679:      uint32 wethTransferGasUnits,

719:      uint16 protocolFeeBps,

720:      uint32 wethTransferGasUnits,

770:      uint16 protocolFeeBps,

771:      uint32 wethTransferGasUnits,

816:      uint16 protocolFeeBps

869:      uint16 protocolFeeBps,

870:      uint32 wethTransferGasUnits,

1260:   function updateWethTranferGas(uint32 _wethTransferGasUnits) external onlyOwner {

1266:   function setProtocolFee(uint16 _protocolFeeBps) external onlyOwner {

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

15. Using private rather than public for constants, saves gas

If needed, the value can be read from the verified contract source code. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table

There are 5 instances of this issue:

File: contracts/token/InfinityToken.sol

25:     bytes32 public constant EPOCH_INFLATION = keccak256('Inflation');

26:     bytes32 public constant EPOCH_DURATION = keccak256('EpochDuration');

27:     bytes32 public constant EPOCH_CLIFF = keccak256('Cliff');

28:     bytes32 public constant MAX_EPOCHS = keccak256('MaxEpochs');

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L25

File: contracts/core/InfinityExchange.sol

57:     bytes32 public immutable DOMAIN_SEPARATOR;

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

16. Duplicated require()/revert() checks should be refactored to a modifier or function

Saves deployment costs

There are 6 instances of this issue:

File: contracts/staking/InfinityStaker.sol

117:      require(amount != 0, 'stake amount cant be 0');

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L117

File: contracts/core/InfinityExchange.sol

183:      require(msg.sender == MATCH_EXECUTOR, 'OME');

347:        require(currency != address(0), 'offers only in ERC20');

350:        require(currency == makerOrders[i].execParams[1], 'cannot mix currencies');

351:        require(isMakerSeller == makerOrders[i].isSellOrder, 'cannot mix order sides');

362:        require(msg.value >= totalPrice, 'invalid total price');

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

17. Empty blocks should be removed or emit something

The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting. If the contract is meant to be extended, the contract should be abstract and the function signatures be added without any default implementation. If the block is an empty if-statement block to avoid doing subsequent checks in the else-if/else conditions, the else-if/else conditions should be nested under the negation of the if-statement, because they involve different classes of checks, which may lead to the introduction of errors when the code is later modified (if(x){}else if(y){...}else{...} => if(!x){if(y){...}else{...}})

There are 4 instances of this issue:

File: contracts/staking/InfinityStaker.sol   #1

55:     fallback() external payable {}

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L55

File: contracts/staking/InfinityStaker.sol   #2

57:     receive() external payable {}

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L57

File: contracts/core/InfinityExchange.sol   #3

119:    fallback() external payable {}

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

File: contracts/core/InfinityExchange.sol   #4

121:    receive() external payable {}

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

18. Use custom errors rather than revert()/require() strings to save gas

Custom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hitby avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas

There are 47 instances of this issue:

File: contracts/token/InfinityToken.sol

61:       require(currentEpoch < getMaxEpochs(), 'no epochs left');

62:       require(block.timestamp >= currentEpochTimestamp + getCliff(), 'cliff not passed');

63:       require(block.timestamp >= previousEpochTimestamp + getEpochDuration(), 'not ready to advance');

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L61

File: contracts/staking/InfinityStaker.sol

68:       require(amount != 0, 'stake amount cant be 0');

69:       require(IERC20(INFINITY_TOKEN).balanceOf(msg.sender) >= amount, 'insufficient balance to stake');

91:       require(amount != 0, 'amount cant be 0');

92        require(
93          userstakedAmounts[msg.sender][oldDuration].amount >= amount,
94          'insufficient staked amount to change duration'
95:       );

96:       require(newDuration > oldDuration, 'new duration must be greater than old duration');

117:      require(amount != 0, 'stake amount cant be 0');

123:      require(totalVested >= amount, 'insufficient balance to unstake');

193:      require(totalStaked >= 0, 'nothing staked to rage quit');

347:      require(sent, 'Failed to send Ether');

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L68

File: contracts/core/InfinityOrderBookComplication.sol

255:            require(tokenIdsIntersect, 'tokenIds dont intersect');

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

File: contracts/core/InfinityExchange.sol

138:      require(msg.sender == MATCH_EXECUTOR, 'OME');

139:      require(numMakerOrders == makerOrders2.length, 'mismatched lengths');

150:        require(_complications.contains(makerOrders1[i].execParams[0]), 'invalid complication');

155:        require(canExec, 'cannot execute');

183:      require(msg.sender == MATCH_EXECUTOR, 'OME');

184:      require(_complications.contains(makerOrder.execParams[0]), 'invalid complication');

185       require(
186         IComplication(makerOrder.execParams[0]).canExecMatchOneToMany(makerOrder, manyMakerOrders),
187         'cannot execute'
188:      );

190:      require(isOrderValid(makerOrder, makerOrderHash), 'invalid maker order');

263:      require(msg.sender == MATCH_EXECUTOR, 'OME');

264:      require(numSells == buys.length && numSells == constructs.length, 'mismatched lengths');

279:        require(executionValid, 'cannot execute');

306:        require(currency != address(0), 'offers only in ERC20');

310:        require(isOrderValid(makerOrders[i], makerOrderHash), 'invalid maker order');

313:        require(isTimeValid, 'invalid time');

314:        require(currency == makerOrders[i].execParams[1], 'cannot mix currencies');

315:        require(isMakerSeller == makerOrders[i].isSellOrder, 'cannot mix order sides');

326:        require(msg.value >= totalPrice, 'invalid total price');

342:      require(ordersLength == takerNfts.length, 'mismatched lengths');

347:        require(currency != address(0), 'offers only in ERC20');

350:        require(currency == makerOrders[i].execParams[1], 'cannot mix currencies');

351:        require(isMakerSeller == makerOrders[i].isSellOrder, 'cannot mix order sides');

362:        require(msg.value >= totalPrice, 'invalid total price');

380:      require(minNonce > userMinOrderNonce[msg.sender], 'nonce too low');

381:      require(minNonce < userMinOrderNonce[msg.sender] + 1000000, 'too many');

392:      require(numNonces > 0, 'cannot be empty');

394:        require(orderNonces[i] >= userMinOrderNonce[msg.sender], 'nonce too low');

395:        require(!isUserOrderNonceExecutedOrCancelled[msg.sender][orderNonces[i]], 'nonce already executed or cancelled');

587:      require(verifyMatchOneToOneOrders(sellOrderHash, buyOrderHash, sell, buy), 'order not verified');

621:      require(verifyMatchOneToManyOrders(buyOrderHash, false, sell, buy), 'order not verified');

649:      require(verifyMatchOneToManyOrders(sellOrderHash, true, sell, buy), 'order not verified');

684:      require(verifyMatchOrders(sellOrderHash, buyOrderHash, sell, buy), 'order not verified');

949:      require(makerOrderValid && executionValid, 'order not verified');

1141:       require(sent, 'failed to send ether to seller');

1231:     require(sent, 'failed');

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

19. 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. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

There are 13 instances of this issue:

File: contracts/staking/InfinityStaker.sol

345:    function rescueETH(address destination) external payable onlyOwner {

351:    function updateStakeLevelThreshold(StakeLevel stakeLevel, uint16 threshold) external onlyOwner {

364     function updatePenalties(
365       uint16 threeMonthPenalty,
366       uint16 sixMonthPenalty,
367       uint16 twelveMonthPenalty
368:    ) external onlyOwner {

375:    function updateInfinityTreasury(address _infinityTreasury) external onlyOwner {

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L345

File: contracts/core/InfinityExchange.sol

1220    function rescueTokens(
1221      address destination,
1222      address currency,
1223      uint256 amount
1224:   ) external onlyOwner {

1229:   function rescueETH(address destination) external payable onlyOwner {

1235:   function addCurrency(address _currency) external onlyOwner {

1240:   function addComplication(address _complication) external onlyOwner {

1245:   function removeCurrency(address _currency) external onlyOwner {

1250:   function removeComplication(address _complication) external onlyOwner {

1255:   function updateMatchExecutor(address _matchExecutor) external onlyOwner {

1260:   function updateWethTranferGas(uint32 _wethTransferGasUnits) external onlyOwner {

1266:   function setProtocolFee(uint16 _protocolFeeBps) external onlyOwner {

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