V-Staykov
commented
2 years ago This is particularly high quality
Open code423n4 opened 2 years ago
V-Staykov
commented
2 years ago This is particularly high quality
Table of Contents:
require()statements that use&&saves gas++icosts less gas compared toi++ori += 1onlyWhitelisted: A modifier used only once should be inlinedCheap Contract Deployment Through Clones
See
@audittag:There's a way to save a significant amount of gas on deployment using Clones: https://www.youtube.com/watch?v=3Mw-pMmJ7TA .
This is a solution that was adopted, as an example, by Porter Finance. They realized that deploying using clones was 10x cheaper:
I suggest applying a similar pattern.
Caching storage values in memory
The code can be optimized by minimising the number of SLOADs. SLOADs are expensive (100 gas) compared to MLOADs/MSTOREs (3 gas). Here, storage values should get cached in memory (see the
@audittags for further details):Splitting
require()statements that use&&saves gasIf you're using the Optimizer at 200, instead of using the
&&operator in a single require statement to check multiple conditions, I suggest using multiple require statements with 1 condition per require statement:An array's length should be cached to save gas in for-loops
Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.
Caching the array length in the stack saves around 3 gas per iteration.
Here, I suggest storing the array's length in a variable before the for-loop, and use it instead:
++icosts less gas compared toi++ori += 1++icosts less gas compared toi++ori += 1for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.The same is also true for
i--.i++incrementsiand returns the initial value ofi. Which means:But
++ireturns the actual incremented value:In the first case, the compiler has to create a temporary variable (when used) for returning
1instead of2Instances include:
I suggest using
++iinstead ofi++to increment the value of an uint variable.Public functions to external
The following functions could be set external to save gas and improve code quality (extracted from Slither). An external call cost is less expensive than one of a public function.
onlyWhitelisted: A modifier used only once should be inlinedAffected code:
No need to explicitly initialize variables with default values
If a variable is not set/initialized, it is assumed to have the default value (
0foruint,falseforbool,address(0)for address...). Explicitly initializing it with its default value is an anti-pattern and wastes gas.As an example:
for (uint256 i = 0; i < numIterations; ++i) {should be replaced withfor (uint256 i; i < numIterations; ++i) {Instances include:
I suggest removing explicit initializations for default values.
Reduce the size of error messages (Long revert Strings)
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.
Revert strings > 32 bytes:
I suggest shortening the revert strings to fit in 32 bytes, or using custom errors as described next.