code-423n4 / 2022-05-factorydao-findings

1 stars 1 forks source link

Gas Optimizations #219

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

[G-01] Short require strings save gas

Strings in solidity are handled in 32 byte chunks. A require string longer than 32 bytes uses more gas. Shortening these strings will save gas.

https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleVesting.sol#L141 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleVesting.sol#L145 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L214 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/VoterID.sol#L217 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/VoterID.sol#L305 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/VoterID.sol#L306

Recommended Mitigation Steps

Shorten all require strings to less than 32 characters

[G-02] Use != 0 instead of > 0

Using > 0 uses slightly more gas than using != 0. Use != 0 when comparing uint variables to zero, which cannot hold values below zero

Locations where this was found include https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/VoterID.sol#L348 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/SpeedBumpPriceGate.sol#L77 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/FixedPricePassThruGate.sol#L51

Recommended Mitigation Steps

Replace > 0 with != 0 to save gas

[G-03] Use Solidity errors instead of require

Solidity errors introduced in version 0.8.4 can save gas on revert conditions https://blog.soliditylang.org/2021/04/21/custom-errors/ https://twitter.com/PatrickAlphaC/status/1505197417884528640

Error messages are not used at all in the contracts. Some example of require usage that can be replaced with errors includes https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleVesting.sol#L141 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleVesting.sol#L145 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L214

Recommended Mitigation Steps

Replace require blocks with new solidity errors described in https://blog.soliditylang.org/2021/04/21/custom-errors/

[G-04] Use prefix not postfix in loops

Using a prefix increment (++i) instead of a postfix increment (i++) saves gas for each loop cycle and so can have a big gas impact when the loop executes on a large number of elements.

There are many examples of this in for loops https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L115 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L141 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L168 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L224 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L249 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L266

Recommended Mitigation Steps

Use prefix not postfix to increment in a loop

[G-05] Use simple comparison in if statement

The comparison operators >= and <= use more gas than >, <, or ==. Replacing the >= and ≤ operators with a comparison operator that has an opcode in the EVM saves gas

The existing code is https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleResistor.sol#L179-L185

if (block.timestamp >= tranche.endTime) {
    currentWithdrawal = tranche.currentCoins;
} else {
    // compute allowed withdrawal
    // secondsElapsedSinceLastWithdrawal * coinsPerSecond == coinsAccumulatedSinceLastWithdrawal
    currentWithdrawal = (block.timestamp - tranche.lastWithdrawalTime) * tranche.coinsPerSecond;
}

A simple comparison can be used for gas savings by reversing the logic

if (block.timestamp < tranche.endTime) {
    // compute allowed withdrawal
    // secondsElapsedSinceLastWithdrawal * coinsPerSecond == coinsAccumulatedSinceLastWithdrawal
    currentWithdrawal = (block.timestamp - tranche.lastWithdrawalTime) * tranche.coinsPerSecond;
} else {
    currentWithdrawal = tranche.currentCoins;
}

The same change can be used here https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleVesting.sol#L153-L158

Recommended Mitigation Steps

Replace the comparison operator and reverse the logic to save gas using the suggestions above

[G-06] Redundant zero initialization

Solidity does not recognize null as a value, so uint variables are initialized to zero. Setting a uint variable to zero is redundant and can waste gas.

There was one place where an int is initialized to zero https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleDropFactory.sol#L17 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/VoterID.sol#L69 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleEligibility.sol#L31 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleResistor.sol#L24 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleResistor.sol#L176 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleVesting.sol#L16 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleVesting.sol#L150 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L115 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L141 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L168 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L224 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L249 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L266

Recommended Mitigation Steps

Remove the redundant zero initialization uint256 i; instead of uint256 i = 0;

[G-07] Cache array length before loop

Caching the array length outside a loop saves reading it on each iteration, as long as the array's length is not changed during the loop. This saves gas.

This was found in many places https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/MerkleLib.sol#L22 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L115 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L141 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L168 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L224 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L249 https://github.com/code-423n4/2022-05-factorydao/blob/main/contracts/PermissionlessBasicPoolFactory.sol#L266

Recommended Mitigation Steps

Cache the array length before the for loop

illuzen commented 2 years ago

all duplicates