Infinity NFT Marketplace contest - Chom's results

Resolution of penalty division may not high enough

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

  ///@dev Penalties if staked tokens are rageQuit early. Example: If 100 tokens are staked for twelve months but rageQuit right away,
  /// the user will get back 100/4 tokens.
  uint16 public THREE_MONTH_PENALTY = 2;
  uint16 public SIX_MONTH_PENALTY = 3;
  uint16 public TWELVE_MONTH_PENALTY = 4;

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

    uint256 totalToUser = totalVested +
      ((threeMonthLock - threeMonthVested) / THREE_MONTH_PENALTY) +
      ((sixMonthLock - sixMonthVested) / SIX_MONTH_PENALTY) +
      ((twelveMonthLock - twelveMonthVested) / TWELVE_MONTH_PENALTY);

    uint256 penalty = totalStaked - totalToUser;

What if you want to reduce penalty to around 10%? You should set penalty to around 1.1 which is not possible as it is uint not float.

You should write penalty in BPS format

  ///@dev Penalties if staked tokens are rageQuit early. Example: If 100 tokens are staked for twelve months but rageQuit right away,
  /// the user will get back 100/4 tokens.
  uint16 public THREE_MONTH_PENALTY = 20000;
  uint16 public SIX_MONTH_PENALTY = 30000;
  uint16 public TWELVE_MONTH_PENALTY = 40000;

    uint256 totalToUser = totalVested +
      ((threeMonthLock - threeMonthVested) * 10000 / THREE_MONTH_PENALTY) +
      ((sixMonthLock - sixMonthVested) * 10000 / SIX_MONTH_PENALTY) +
      ((twelveMonthLock - twelveMonthVested) * 10000 / TWELVE_MONTH_PENALTY);

    uint256 penalty = totalStaked - totalToUser;

Use Seaport gas optimized signature verification contract for signature verification

Currently, verify function takes too much gas on Address.isContract(signer)

Address.isContract(signer) = extcodesize will cause an unnecessary 2600 upfront gas cost on every transaction. While it can be avoided for majority of case where it is EOA wallet.

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

  function verify(
    bytes32 orderHash,
    address signer,
    bytes32 r,
    bytes32 s,
    uint8 v,
    bytes32 domainSeparator
  ) internal view returns (bool) {
    // \x19\x01 is the standardized encoding prefix
    // https://eips.ethereum.org/EIPS/eip-712#specification
    bytes32 digest = keccak256(abi.encodePacked('\x19\x01', domainSeparator, orderHash));

    if (Address.isContract(signer)) {
      // 0x1626ba7e is the interfaceId for signature contracts (see IERC1271)
      return IERC1271(signer).isValidSignature(digest, abi.encodePacked(r, s, v)) == 0x1626ba7e;
    } else {
      return recover(digest, r, s, v) == signer;
    }
  }

2600 gas on Address.isContract(signer) can be avoided by using Seaport implementation

https://github.com/ProjectOpenSea/seaport/blob/main/contracts/lib/SignatureVerification.sol

Consider using custom errors instead of revert strings

This reduce gas cost as show here https://forum.openzeppelin.com/t/a-collection-of-gas-optimisation-tricks/19966/5

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

Any require statement in your code can be replaced with custom error for example:

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

Can be replaced with

// declare error before contract declaration
error OrderNotVerified();

if(!verifyMatchOneToManyOrders(buyOrderHash, false, sell, buy)) revert OrderNotVerified();