Infinity NFT Marketplace contest - Tomio's results

Title: Reduce the size of error messages (Long revert Strings)

Impact: Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met. Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Proof of Concept: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L92-L96

Recommended Mitigation Steps: Consider shortening the revert strings to fit in 32 bytes

---

Title: Custom errors from Solidity 0.8.4 are cheaper than revert strings

Impact: Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) while providing the same amount of information

Custom errors are defined using the error statement reference: https://blog.soliditylang.org/2021/04/21/custom-errors/

Proof of Concept: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol

Recommended Mitigation Steps: Replace require statements with custom errors.

---

Title: Consider make constant as private to save gas

Proof of Concept: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/token/InfinityToken.sol#L25-L28 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/token/TimelockConfig.sol#L9-L10

Recommended Mitigation Steps: I suggest changing the visibility from public to internal or private

---

Title: Comparison operators

Proof of Concept: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/token/InfinityToken.sol#L62-L63 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/token/TimelockConfig.sol#L52

Recommended Mitigation Steps: Replace <= with <, and >= with > for gas opt

---

Title: Gas improvement on calling SafeERC20.function

Proof of Concept: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L16

Recommended Mitigation Steps: by removing L#16 and directly call SafeERC20

Example L#74:

  SafeERC20.safeTransferFrom(INFINITY_TOKEN, msg.sender, address(this), amount);

---

Title: Using storage to declare Struct variable inside function

Proof of Concept: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L246

Recommended Mitigation Steps:

  StakeAmount[] storage stakingInfo = new StakeAmount[](4);

---

Title: >= is cheaper than >

Impact: Strict inequalities (>) are more expensive than non-strict ones (>=). This is due to some supplementary checks (ISZERO, 3 gas)

Proof of Concept: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L1156 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L1164 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityOrderBookComplication.sol#L341 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/token/InfinityToken.sol#L67

Recommended Mitigation Steps: Consider using >= instead of > to avoid some opcodes

---

Title: Using != is more gas efficient

Proof of Concept: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L392

Recommended Mitigation Steps: Change from > to !=

---

Title: Expression for constant values such as a call to keccak256(), should use immutable rather than constant

Proof of Concept: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/token/InfinityToken.sol#L25-L28 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/token/TimelockConfig.sol#L9-L10

Recommended Mitigation Steps: Change from constant to immutable reference: https://github.com/ethereum/solidity/issues/9232

---

Title: Default value initialization

Impact: If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

Proof of Concept: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityOrderBookComplication.sol#L42 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityOrderBookComplication.sol#L108 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityOrderBookComplication.sol#L214 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityOrderBookComplication.sol#L244

Recommended Mitigation Steps: Remove explicit initialization for default values.

---

Title: Using unchecked can save gas

Proof of Concept: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L301 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L305 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/staking/InfinityStaker.sol#L309

Recommended Mitigation Steps: Because of the condition in L#298, 302, 306

unchecked{
  amount = amount - noVesting;
}

---

Title: Using multiple require instead && can save gas

Proof of Concept: https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L949 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L264

Recommended Mitigation Steps: Change to:

	require(makerOrderValid, 'order not verified');
	require(executionValid, 'order not verified');

---