Function InfinityStaker.getRageQuitAmounts uses require statement to check if uint256 variable totalStaked is bigger or equal to 0. Variable of type uint256 is always bigger or equal to 0 which makes this check completely obsolete.
Proof of Concept
staking/InfinityStaker.so:193 require(totalStaked >= 0, 'nothing staked to rage quit');
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to remove obsolete check or change it to check if totalStaked value is bigger > than 0.
2. Use scientific notation
Impact
Multiple contracts are using math exponent calculation to express big numbers. This consumes additional gas and its better to use scienfic notation.
Proof of Concept
core/InfinityOrderBookComplication.sol:338: uint256 PRECISION = 10**4; // precision for division; similar to bps
core/InfinityExchange.sol:1161: uint256 PRECISION = 10**4; // precision for division; similar to bps
staking/InfinityStaker.sol:237: (userstakedAmounts[user][Duration.TWELVE_MONTHS].amount * 4)) / (10**18);
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to use scientific notation, for example: 1e18.
3. Long revert error messages
Impact
Shortening revert error messages to fit in 32 bytes will decrease gas costs for deployment and gas costs when the revert condition has been met.
Proof of Concept
core/InfinityExchange.sol:399: require(!isUserOrderNonceExecutedOrCancelled[msg.sender][orderNonces[i]], 'nonce already executed or cancelled');
--
staking/InfinityStaker.sol:92: require(
userstakedAmounts[msg.sender][oldDuration].amount >= amount,
'insufficient staked amount to change duration'
);
--
staking/InfinityStaker.sol:96: require(newDuration > oldDuration, 'new duration must be greater than old duration');
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to decrease revert messages to maximum 32 bytes. Alternatively custom error types should be used.
4. Use custom errors instead of revert strings to save gas
Impact
Usage of custom errors reduces the gas cost.
Proof of Concept
Contracts that should be using custom errors:
InfinityExchange.sol
InfinityOrderBookComplication.sol
InfinityStaker.sol
InfinityToken.sol
TimelockConfig.sol
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to add custom errors to listed contracts.
5. No need to explicitly initialize variables with default values
Impact
If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for addresses). Explicitly initializing it with its default value is an anti-pattern and waste of gas.
Proof of Concept
core/InfinityOrderBookComplication.sol:42: bool _isPriceValid = false;
core/InfinityOrderBookComplication.sol:108: bool _isPriceValid = false;
core/InfinityOrderBookComplication.sol:76: for (uint256 i = 0; i < ordersLength; ) {
core/InfinityOrderBookComplication.sol:82: for (uint256 j = 0; j < nftsLength; ) {
core/InfinityOrderBookComplication.sol:197: uint256 numConstructedItems = 0;
core/InfinityOrderBookComplication.sol:199: for (uint256 i = 0; i < nftsLength; ) {
core/InfinityOrderBookComplication.sol:214: uint256 numTakerItems = 0;
core/InfinityOrderBookComplication.sol:216: for (uint256 i = 0; i < nftsLength; ) {
core/InfinityOrderBookComplication.sol:244: uint256 numCollsMatched = 0;
core/InfinityOrderBookComplication.sol:246: for (uint256 i = 0; i < order2NftsLength; ) {
core/InfinityOrderBookComplication.sol:247: for (uint256 j = 0; j < order1NftsLength; ) {
core/InfinityOrderBookComplication.sol:289: uint256 numTokenIdsPerCollMatched = 0;
core/InfinityOrderBookComplication.sol:290: for (uint256 k = 0; k < item2TokensLength; ) {
core/InfinityOrderBookComplication.sol:291: for (uint256 l = 0; l < item1TokensLength; ) {
core/InfinityOrderBookComplication.sol:318: uint256 sum = 0;
core/InfinityOrderBookComplication.sol:320: for (uint256 i = 0; i < ordersLength; ) {
core/InfinityExchange.sol:148: for (uint256 i = 0; i < numMakerOrders; ) {
core/InfinityExchange.sol:200: for (uint256 i = 0; i < ordersLength; ) {
core/InfinityExchange.sol:219: for (uint256 i = 0; i < ordersLength; ) {
core/InfinityExchange.sol:272: for (uint256 i = 0; i < numSells; ) {
core/InfinityExchange.sol:308: for (uint256 i = 0; i < numMakerOrders; ) {
core/InfinityExchange.sol:349: for (uint256 i = 0; i < ordersLength; ) {
core/InfinityExchange.sol:393: for (uint256 i = 0; i < numNonces; ) {
core/InfinityExchange.sol:1048: for (uint256 i = 0; i < numNfts; ) {
core/InfinityExchange.sol:1086: for (uint256 i = 0; i < numTokens; ) {
core/InfinityExchange.sol:1109: for (uint256 i = 0; i < numNfts; ) {
core/InfinityExchange.sol:1190: for (uint256 i = 0; i < numNfts; ) {
core/InfinityExchange.sol:1206: for (uint256 i = 0; i < numTokens; ) {
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to remove explicit initializations with default values.
6. Use != 0 instead of > 0 for unsigned integer comparison
Impact
When dealing with unsigned integer types, comparisons with != 0 are cheaper than with > 0.
Proof of Concept
core/InfinityExchange.sol:392: require(numNonces > 0, 'cannot be empty');
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to use != 0 instead of > 0.
7. Pack integer values
Impact
Packing integer variables into storage slots saves gas.
Functions declared as public that are never called internally within the contract should be marked as external in order save gas (especially in the case where the function takes arguments, since external functions can read arguments directly from calldata instead of allocating memory).
1. Obsolete checks
Impact
Function
InfinityStaker.getRageQuitAmounts
uses require statement to check ifuint256
variabletotalStaked
is bigger or equal to0
. Variable of typeuint256
is always bigger or equal to0
which makes this check completely obsolete.Proof of Concept
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to remove obsolete check or change it to check if
totalStaked
value is bigger>
than0
.2. Use scientific notation
Impact
Multiple contracts are using math exponent calculation to express big numbers. This consumes additional gas and its better to use scienfic notation.
Proof of Concept
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to use scientific notation, for example:
1e18
.3. Long revert error messages
Impact
Shortening revert error messages to fit in 32 bytes will decrease gas costs for deployment and gas costs when the revert condition has been met.
Proof of Concept
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to decrease revert messages to maximum 32 bytes. Alternatively custom error types should be used.
4. Use custom errors instead of revert strings to save gas
Impact
Usage of custom errors reduces the gas cost.
Proof of Concept
Contracts that should be using custom errors:
InfinityExchange.sol
InfinityOrderBookComplication.sol
InfinityStaker.sol
InfinityToken.sol
TimelockConfig.sol
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to add custom errors to listed contracts.
5. No need to explicitly initialize variables with default values
Impact
If a variable is not set/initialized, it is assumed to have the default value (
0
for uint,false
for bool,address(0)
for addresses). Explicitly initializing it with its default value is an anti-pattern and waste of gas.Proof of Concept
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to remove explicit initializations with default values.
6. Use != 0 instead of > 0 for unsigned integer comparison
Impact
When dealing with unsigned integer types, comparisons with
!= 0
are cheaper than with> 0
.Proof of Concept
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to use
!= 0
instead of> 0
.7. Pack integer values
Impact
Packing integer variables into storage slots saves gas.
Proof of Concept
InfinityStaker.sol
:Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
If
amount
is not expected to reach specific values it is recommended to pack it in192 bits
and use64 bits
for timestamp.8. Declare as external
Impact
Functions declared as public that are never called internally within the contract should be marked as external in order save gas (especially in the case where the function takes arguments, since external functions can read arguments directly from calldata instead of allocating memory).
Proof of Concept
InfinityStaker.sol
:getUserTotalStaked
can be set as external - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L154getUserTotalVested
can be set as external - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L167Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to change all public functions that are not called internally to external visibility.