QA Report

[code423n4]

1. Rage quit require do not control anything (low)

Proof of Concept

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

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

Recommended Mitigation Steps

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

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

2. InfinityExchange and InfinityStaker fallback functions aren't needed (low)

The case when contract has received ether and there is no data is covered by receive(), i.e. fallback() isn't needed.

Proof of Concept

InfinityExchange and InfinityStaker has both payable fallback() and receive():

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

  fallback() external payable {}

  receive() external payable {}

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

  // Fallback
  fallback() external payable {}

  receive() external payable {}

Recommended Mitigation Steps

Consider leaving only receive() in both cases. Also, consider removing both for InfinityStaker as there is no need to receive ether there.

3. There is no pausing in InfinityExchange's user facing functions (low)

Proof of Concept

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

  /**
   @notice Batch buys or sells orders where maker orders can have unspecified NFTs. Transaction initiated by an end user.
   @param makerOrders The orders to fulfill
   @param takerNfts The specific NFTs that the taker is willing to take that intersect with the higher order intent of the maker
   Example: If a makerOrder is 'buy any one of these 2 specific NFTs', then the takerNfts would be 'this one specific NFT'.
  */
  function takeOrders(OrderTypes.MakerOrder[] calldata makerOrders, OrderTypes.OrderItem[][] calldata takerNfts)
    external
    payable
    nonReentrant
  {

Pausing functionality is already introduced in Staking:

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

  /**
   * @notice Untake tokens
   * @dev Storage updates are done for each stake level. See _updateUserStakedAmounts for more details
   * @param amount Amount of tokens to unstake
   */
  function unstake(uint256 amount) external override nonReentrant whenNotPaused {

Recommended Mitigation Steps

Consider expanding pausing to cover all user facing functions of InfinityExchange: takeOrders(), takeMultipleOneOrders(), transferMultipleNFTs(). For example, by introducing of the whenNotPaused modifier.

4. InfinityExchange events aren't indexed (non-critical)

Proof of Concept

InfinityExchange has core event not indexed:

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

  event CancelAllOrders(address user, uint256 newMinNonce);
  event CancelMultipleOrders(address user, uint256[] orderNonces);
  event NewWethTransferGasUnits(uint32 wethTransferGasUnits);
  event NewProtocolFee(uint16 protocolFee);

InfinityToken also:

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

  event EpochAdvanced(uint256 currentEpoch, uint256 supplyMinted);

Recommended Mitigation Steps

Consider adding indexes to ids and addresses in the all important events to improve their usability

5. There is no limit for new PROTOCOL_FEE_BPS (non-critical)

Owner can set PROTOCOL_FEE_BPS higher than 100%, as an operational mistake or with a malicious intent.

Proof of Concept

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

  /// @dev updates exchange fees
  function setProtocolFee(uint16 _protocolFeeBps) external onlyOwner {
    PROTOCOL_FEE_BPS = _protocolFeeBps;

Recommended Mitigation Steps

Require PROTOCOL_FEE_BPS to be within a predefined range, say in [0, 10%]

6. Basis points precision is hard coded (non-critical)

Proof of Concept

There are several instances across the code where 10000 is used:

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

    uint256 protocolFee = (protocolFeeBps * execPrice) / 10000;

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

    uint256 protocolFee = (PROTOCOL_FEE_BPS * amount) / 10000;

Recommended Mitigation Steps

Introduce the constant to reduce operational errors on future system development.