QA

[Q-01] Use a wider uncheck

In some code blocks you use a lot of uncheckeds scopes, a cleanner pattern would be to use a wider unchecked scope.

`InfinityOrderBookComplication.sol`

diff --git a/contracts/core/InfinityOrderBookComplication.sol b/contracts/core/InfinityOrderBookComplication.sol
index 17b2b45..dbf4997 100644
--- a/contracts/core/InfinityOrderBookComplication.sol
+++ b/contracts/core/InfinityOrderBookComplication.sol
@@ -242,25 +242,21 @@ contract InfinityOrderBookComplication is IComplication, Ownable {
     }

     uint256 numCollsMatched = 0;
-    // check if taker has all items in maker
-    for (uint256 i = 0; i < order2NftsLength; ) {
-      for (uint256 j = 0; j < order1NftsLength; ) {
-        if (order1Nfts[j].collection == order2Nfts[i].collection) {
-          // increment numCollsMatched
-          unchecked {
+    unchecked {
+      // check if taker has all items in maker
+      for (uint256 i = 0; i < order2NftsLength; ) {
+        for (uint256 j = 0; j < order1NftsLength; ) {
+          if (order1Nfts[j].collection == order2Nfts[i].collection) {
+            // increment numCollsMatched
             ++numCollsMatched;
+            // check if tokenIds intersect
+            bool tokenIdsIntersect = doTokenIdsIntersect(order1Nfts[j], order2Nfts[i]);
+            require(tokenIdsIntersect, 'tokenIds dont intersect');
+            // short circuit
+            break;
           }
-          // check if tokenIds intersect
-          bool tokenIdsIntersect = doTokenIdsIntersect(order1Nfts[j], order2Nfts[i]);
-          require(tokenIdsIntersect, 'tokenIds dont intersect');
-          // short circuit
-          break;
-        }
-        unchecked {
           ++j;
         }
-      }
-      unchecked {
         ++i;
       }
     }
@@ -287,23 +283,19 @@ contract InfinityOrderBookComplication is IComplication, Ownable {
       return true;
     }
     uint256 numTokenIdsPerCollMatched = 0;
-    for (uint256 k = 0; k < item2TokensLength; ) {
-      for (uint256 l = 0; l < item1TokensLength; ) {
-        if (
-          item1.tokens[l].tokenId == item2.tokens[k].tokenId && item1.tokens[l].numTokens == item2.tokens[k].numTokens
-        ) {
-          // increment numTokenIdsPerCollMatched
-          unchecked {
+    unchecked {
+      for (uint256 k = 0; k < item2TokensLength; ) {
+        for (uint256 l = 0; l < item1TokensLength; ) {
+          if (
+            item1.tokens[l].tokenId == item2.tokens[k].tokenId && item1.tokens[l].numTokens == item2.tokens[k].numTokens
+          ) {
+            // increment numTokenIdsPerCollMatched
             ++numTokenIdsPerCollMatched;
+            // short circuit
+            break;
           }
-          // short circuit
-          break;
-        }
-        unchecked {
           ++l;
         }
-      }
-      unchecked {
         ++k;
       }
     }

[Q-02] Use a common pattern

On InfinityExchange.sol#L1128 the signature of the function is function _transferFees(address seller, address buyer, uint256 amount, address currency) I recommend you to change it to a more common pattern; function _transferFees(address seller, address buyer, address currency, uint256 amount)

[Q-03] Struct definition is never use

On OrderTypes.sol#L46-L48 the struct TakerOrder is never used

[Q-04] Natspect typo, weth should be uppercase

on InfinityExchange.sol#L859 the comment is;

   * @param weth weth address

And should be

   * @param weth WETH address

Non-critical

[N-01] UNUSED IMPORT

IStaker.sol

In IStaker.sol, the import {OrderTypes} from '../libs/OrderTypes.sol'; is unused

Out of scope but InfinityToken.sol inherits from IStaker.sol

InfinityOrderBookComplication.sol

Import Ownable on InfinityOrderBookComplication.sol

InfinityOrderBookComplication contract is importing Ownable but no one is using it. My recommendation is to remove it.

diff --git a/contracts/core/InfinityOrderBookComplication.sol b/contracts/core/InfinityOrderBookComplication.sol
index 17b2b45..6e4cf77 100644
--- a/contracts/core/InfinityOrderBookComplication.sol
+++ b/contracts/core/InfinityOrderBookComplication.sol
@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.14;

-import {Ownable} from '@openzeppelin/contracts/access/Ownable.sol';

 import {OrderTypes} from '../libs/OrderTypes.sol';
 import {IComplication} from '../interfaces/IComplication.sol';
@@ -11,7 +10,7 @@ import {IComplication} from '../interfaces/IComplication.sol';
  * @author nneverlander. Twitter @nneverlander
  * @notice Complication to execute orderbook orders
  */
-contract InfinityOrderBookComplication is IComplication, Ownable {
+contract InfinityOrderBookComplication is IComplication {
   // ======================================================= EXTERNAL FUNCTIONS ==================================================

   /**

[N-02] DECLARE AS `IERC20`

In InfinityStaker.sol:L25, declare INFINITY_TOKEN as IERC20 would avoid it cast to IERC20 when use it and improve code clarity:

- address public INFINITY_TOKEN;
+ IERC20 public INFINITY_TOKEN;

Cast in the constructor, or declare _tokenAddress as IERC20

 constructor(address _tokenAddress, address _infinityTreasury) {
-   INFINITY_TOKEN = _tokenAddress;
+   INFINITY_TOKEN = IERC20(_tokenAddress);
    INFINITY_TREASURY = _infinityTreasury;
  }

Remove cast on:

L69: 
- require(IERC20(INFINITY_TOKEN).balanceOf(msg.sender) >= amount, 'insufficient balance to stake');
+ require(INFINITY_TOKEN.balanceOf(msg.sender) >= amount, 'insufficient balance to stake');
L74:
- IERC20(INFINITY_TOKEN).safeTransferFrom(msg.sender, address(this), amount);
+ INFINITY_TOKEN.safeTransferFrom(msg.sender, address(this), amount);
L128:
- IERC20(INFINITY_TOKEN).safeTransfer(msg.sender, amount);
+ INFINITY_TOKEN.safeTransfer(msg.sender, amount);
L141:
- IERC20(INFINITY_TOKEN).safeTransfer(msg.sender, totalToUser);
+ INFINITY_TOKEN.safeTransfer(msg.sender, totalToUser);
L142:
- IERC20(INFINITY_TOKEN).safeTransfer(INFINITY_TREASURY, penalty);
+ INFINITY_TOKEN.safeTransfer(INFINITY_TREASURY, penalty);

[N-03] NOT DECLARE VISIBILITY

In TimelockConfig.sol, L28, L29, L31, L32: By default, the visibility of variables in solidity is internal, but it is good practice to declare it.

Out of scope but InfinityToken.sol inherits from TimelockConfig.sol

[N-04] TYPO

In InfinityExchange.sol:

L858: * @param weth weth address to * @param weth WETH address
L1260: The name of function updateWethTranferGas to updateWethTransferGas

Low Risk

[L-01] EVENTS ARE NOT INDEXED

The emitted events are not indexed, making off-chain scripts such as front-ends of dApps difficult to filter the events efficiently.

Recommended Mitigation Steps: Add the indexed keyword in filter parameters of the events

Instances include:

InfinityExchange.sol:L80-L102:  
- event CancelAllOrders(address user, uint256 newMinNonce);
+ event CancelAllOrders(address indexed user, uint256 newMinNonce);
- event CancelMultipleOrders(address user, uint256[] orderNonces);
+ event CancelMultipleOrders(address indexed user, uint256[] orderNonces);
  event NewWethTransferGasUnits(uint32 wethTransferGasUnits);
  event NewProtocolFee(uint16 protocolFee);

  event MatchOrderFulfilled(
    bytes32 sellOrderHash,
    bytes32 buyOrderHash,
-   address seller,
+   address indexed seller,
-   address buyer,
+   address indexed buyer,
    address complication, // address of the complication that defines the execution
-   address currency, // token address of the transacting currency
+   address indexed currency, // token address of the transacting currency
    uint256 amount // amount spent on the order
  );

  event TakeOrderFulfilled(
    bytes32 orderHash,
-   address seller,
+   address indexed seller,
-   address buyer,
+   address indexed buyer,
    address complication, // address of the complication that defines the execution
-   address currency, // token address of the transacting currency
+   address indexed currency, // token address of the transacting currency
    uint256 amount // amount spent on the order
  );

InfinityToken.sol:L35:  
- event EpochAdvanced(uint256 currentEpoch, uint256 supplyMinted);
+ event EpochAdvanced(uint256 indexed currentEpoch, uint256 supplyMinted);

TimelockConfig.sol:L34-L36:  
- event ChangeRequested(bytes32 configId, uint256 value);
+ event ChangeRequested(bytes32 indexed configId, uint256 value);
- event ChangeConfirmed(bytes32 configId, uint256 value);
+ event ChangeConfirmed(bytes32 indexed configId, uint256 value);
- event ChangeCanceled(bytes32 configId, uint256 value);
+ event ChangeCanceled(bytes32 indexed configId, uint256 value);

Out of scope but InfinityToken.sol inherits from TimelockConfig.sol

[L-02] `require` NEVER REVERT

In InfinityStaker.sol, L193: This require never reverts the transaction, any number of uint256 data type it's greater or equal to 0, by definition

Remove the equal in therequire comparation

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

This also save gas, if the user dont have staked balance

[L-02] Unnecesary math operations

Unnecesary multiplication by one and unnecesary exponetiation.

`InfinityStaker.sol`

diff --git a/contracts/staking/InfinityStaker.sol b/contracts/staking/InfinityStaker.sol
index 9b36cb5..872a254 100644
--- a/contracts/staking/InfinityStaker.sol
+++ b/contracts/staking/InfinityStaker.sol
@@ -231,10 +231,10 @@ contract InfinityStaker is IStaker, Ownable, Pausable, ReentrancyGuard {
    */
   function getUserStakePower(address user) public view override returns (uint256) {
     return
-      ((userstakedAmounts[user][Duration.NONE].amount * 1) +
+      ((userstakedAmounts[user][Duration.NONE].amount) +
         (userstakedAmounts[user][Duration.THREE_MONTHS].amount * 2) +
         (userstakedAmounts[user][Duration.SIX_MONTHS].amount * 3) +
-        (userstakedAmounts[user][Duration.TWELVE_MONTHS].amount * 4)) / (10**18);
+        (userstakedAmounts[user][Duration.TWELVE_MONTHS].amount * 4)) / 1e18;
   }

   /**

InfinityExchange.sol

diff --git a/contracts/core/InfinityExchange.sol b/contracts/core/InfinityExchange.sol
index 3639554..d58b965 100644
--- a/contracts/core/InfinityExchange.sol
+++ b/contracts/core/InfinityExchange.sol
@@ -1158,7 +1158,7 @@ contract InfinityExchange is ReentrancyGuard, Ownable {
       return startPrice;
     }
     uint256 elapsedTime = block.timestamp - order.constraints[3];
-    uint256 PRECISION = 10**4; // precision for division; similar to bps
+    uint256 PRECISION = 1e4; // precision for division; similar to bps
     uint256 portionBps = elapsedTime > duration ? PRECISION : ((elapsedTime * PRECISION) / duration);
     priceDiff = (priceDiff * portionBps) / PRECISION;
     return startPrice > endPrice ? startPrice - priceDiff : startPrice + priceDiff;

`InfinityOrderBookComplication.sol`

diff --git a/contracts/core/InfinityOrderBookComplication.sol b/contracts/core/InfinityOrderBookComplication.sol
index 17b2b45..f1975bf 100644
--- a/contracts/core/InfinityOrderBookComplication.sol
+++ b/contracts/core/InfinityOrderBookComplication.sol
@@ -335,7 +335,7 @@ contract InfinityOrderBookComplication is IComplication, Ownable {
       return startPrice;
     }
     uint256 elapsedTime = block.timestamp - order.constraints[3];
-    uint256 PRECISION = 10**4; // precision for division; similar to bps
+    uint256 PRECISION = 1e4; // precision for division; similar to bps
     uint256 portionBps = elapsedTime > duration ? PRECISION : ((elapsedTime * PRECISION) / duration);
     priceDiff = (priceDiff * portionBps) / PRECISION;
     return startPrice > endPrice ? startPrice - priceDiff : startPrice + priceDiff;

[L-03] No event emmision on critical function

`InfinityStaker.sol`

InfinityStaker.sol#L351 function updateStakeLevelThreshold should emit event InfinityStaker.sol#L364 function updatePenalties should emit event InfinityStaker.sol#L375 function updateInfinityTreasury should emit event

`InfinityExchange.sol`

InfinityExchange.sol

Critical admin functions rescueTokens, rescueETH, addCurrency, addComplication, removeCurrency, removeComplication and updateMatchExecutor should emit events

[L-04] Solidity version

You are currently using solidity 0.8.14, best recommendation is to switch to 0.8.15 two major bugs introduced on previus versions were fixes, plus its more gas efficient.

SOURCE
tldr;

code-423n4 / 2022-06-infinity-findings

QA Report #300

QA

[Q-01] Use a wider uncheck

`InfinityOrderBookComplication.sol`

[Q-02] Use a common pattern

[Q-03] Struct definition is never use

[Q-04] Natspect typo, weth should be uppercase

Non-critical

[N-01] UNUSED IMPORT

IStaker.sol

InfinityOrderBookComplication.sol

[N-02] DECLARE AS `IERC20`

[N-03] NOT DECLARE VISIBILITY

[N-04] TYPO

Low Risk

[L-01] EVENTS ARE NOT INDEXED

[L-02] `require` NEVER REVERT

[L-02] Unnecesary math operations

`InfinityStaker.sol`

InfinityExchange.sol

`InfinityOrderBookComplication.sol`

[L-03] No event emmision on critical function

`InfinityStaker.sol`

`InfinityExchange.sol`

[L-04] Solidity version

code-423n4 / 2022-06-infinity-findings

QA Report #300

QA

[Q-01] Use a wider uncheck

InfinityOrderBookComplication.sol

[Q-02] Use a common pattern

[Q-03] Struct definition is never use

[Q-04] Natspect typo, weth should be uppercase

Non-critical

[N-01] UNUSED IMPORT

IStaker.sol

InfinityOrderBookComplication.sol

[N-02] DECLARE AS IERC20

[N-03] NOT DECLARE VISIBILITY

[N-04] TYPO

Low Risk

[L-01] EVENTS ARE NOT INDEXED

[L-02] require NEVER REVERT

[L-02] Unnecesary math operations

InfinityStaker.sol

InfinityExchange.sol

InfinityOrderBookComplication.sol

[L-03] No event emmision on critical function

InfinityStaker.sol

InfinityExchange.sol

[L-04] Solidity version

`InfinityOrderBookComplication.sol`

[N-02] DECLARE AS `IERC20`

[L-02] `require` NEVER REVERT

`InfinityStaker.sol`

`InfinityOrderBookComplication.sol`

`InfinityStaker.sol`

`InfinityExchange.sol`