Infinity NFT Marketplace contest - MadWookie's results

Low Risk

Missing check for address(0x0) when assigning values to address state variables

1. File: InfinityStaker.sol#L50-51

    INFINITY_TOKEN = _tokenAddress;
    INFINITY_TREASURY = _infinityTreasury;

2. File: InfinityStaker.sol#L376

    INFINITY_TREASURY = _infinityTreasury;

3. File: InfinityExchange.sol#L115-116

    WETH = _WETH;
    MATCH_EXECUTOR = _matchExecutor;

Missing whenNotPause modifier on rageQuit()

This function allows users to withdraw tokens from the contract and should be given whenNotPause similar to unstake(). If the contract is paused an exploiter could still potentially drain the contract.

1. File: InfinityStaker.sol#L136

QA

Constants should be defined rather than using magic numbers

1. File: InfinityStaker.sol#L237

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

3. File: InfinityExchange.sol#L775

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

3. File: InfinityExchange.sol#L775

    uint256 protocolFee = (protocolFeeBps * execPrice) / 10000;

4. File: InfinityExchange.sol#L819

    uint256 protocolFee = (protocolFeeBps * execPrice) / 10000;

5. File: InfinityExchange.sol#L1135

    uint256 protocolFee = (PROTOCOL_FEE_BPS * amount) / 10000;

Events should use three indexed fields if three or more fields are used.

1. File: InfinityStaker.sol#L44

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

2. File: InfinityStaker.sol#L45

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

3. File: InfinityStaker.sol#L47

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

Consider adding an event when changing critical state variables.

updatePenalties()

1. File: InfinityStaker.sol#L364

updateStakeLevelThreshold()

2. File: InfinityStaker.sol#L351

Gas Report

Cache state variables in memory when reading multiple times in one function.

In this instance WETH_TRANSFER_GAS_UNITS is already cached as wethTransferGasUnits but wasn't used.

1. File: InfinityExchange.sol#L231

 uint256 gasCost = (startGas - gasleft() + WETH_TRANSFER_GAS_UNITS) * tx.gasprice;

cache userMinOrderNonce[msg.sender]

2. File: InfinityExchange.sol#L380-381

require(minNonce > userMinOrderNonce[msg.sender], 'nonce too low');
require(minNonce < userMinOrderNonce[msg.sender] + 1000000, 'too many');

cache userMinOrderNonce[msg.sender] before for loop

3. File: InfinityExchange.sol#L394

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

The bronze, silver, and gold threshold variables are each read twice. Caching them will make the function on average cheaper.

4. File: InfinityStaker.sol#L213-223

    if (totalPower <= BRONZE_STAKE_THRESHOLD) {
      return StakeLevel.NONE;
    } else if (totalPower > BRONZE_STAKE_THRESHOLD && totalPower <= SILVER_STAKE_THRESHOLD) {
      return StakeLevel.BRONZE;
    } else if (totalPower > SILVER_STAKE_THRESHOLD && totalPower <= GOLD_STAKE_THRESHOLD) {
      return StakeLevel.SILVER;
    } else if (totalPower > GOLD_STAKE_THRESHOLD && totalPower <= PLATINUM_STAKE_THRESHOLD) {
      return StakeLevel.GOLD;
    } else {
      return StakeLevel.PLATINUM;
    }

Variables that are only set in the constructor should be marked immutable. This makes reading much cheaper.

1. File: InfinityStaker.sol#L25

address public INFINITY_TOKEN;

Using unchecked{} on arithmetic reduces gas by removing overflow/underflow check.

The amount variable is checked to be less than or equal to the balance of the staker in each function. This means when performing math with an amount you know it will never overflow/underflow. You can enclose each of these statements in an unchecked{}.

1. File: InfinityStaker.sol#L71

    userstakedAmounts[msg.sender][duration].amount += amount;

2. File: InfinityStaker.sol#L99-100

    userstakedAmounts[msg.sender][oldDuration].amount -= amount;
    userstakedAmounts[msg.sender][newDuration].amount += amount;

2. File: InfinityStaker.sol#L156-159 )

    userstakedAmounts[user][Duration.NONE].amount +
    userstakedAmounts[user][Duration.THREE_MONTHS].amount +
    userstakedAmounts[user][Duration.SIX_MONTHS].amount +
    userstakedAmounts[user][Duration.TWELVE_MONTHS].amount;

3. File: InfinityStaker.sol#L234-237

    ((userstakedAmounts[user][Duration.NONE].amount * 1) +
    (userstakedAmounts[user][Duration.THREE_MONTHS].amount * 2) +
    (userstakedAmounts[user][Duration.SIX_MONTHS].amount * 3) +
    (userstakedAmounts[user][Duration.TWELVE_MONTHS].amount * 4)) / (10**18);

4. File: InfinityStaker.sol#L314

    userstakedAmounts[user][Duration.TWELVE_MONTHS].amount -= amount;

5. File: InfinityStaker.sol#L317

    userstakedAmounts[user][Duration.SIX_MONTHS].amount -= amount;

6. File: InfinityStaker.sol#L320

    userstakedAmounts[user][Duration.THREE_MONTHS].amount -= amount;

7. File: InfinityStaker.sol#L323

    userstakedAmounts[user][Duration.NONE].amount -= amount;

Placing require statements that use calldata before statements that use state variables will save gas when calldata require statements fail.

1. File: InfinityExchange.sol#L138-139

    require(msg.sender == MATCH_EXECUTOR, 'OME');
    require(numMakerOrders == makerOrders2.length, 'mismatched lengths');

2. File: InfinityExchange.sol#L263-264

    require(msg.sender == MATCH_EXECUTOR, 'OME');
    require(numSells == buys.length && numSells == constructs.length, 'mismatched lengths');

require() strings longer thn 32 bytes are more expensive

Consider changing these error string to something shorter than 32 bytes.

1. File: InfinityStaker.sol#L92-95

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

2. File: InfinityStaker.sol#L96

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