HardlyDifficult
commented
2 years ago Don't initialize variables with default values.
Invalid. This optimization technique is no longer applicable with the current version of Solidity.
Cache Array Length Outside of Loop
May be theoretically valid, but won't fix. I tested this: gas-reporter and our gas-stories suite is reporting a small regression using this technique. It also hurts readability a bit so we wouldn't want to include it unless it was a clear win.
For-Loops: Index increments can be left unchecked
4 of the 5 examples were already unchecked -- invalid.
getFeesAndRecipients is a read only function not intended to be used on-chain, but as a best practice we will add unchecked there as well.
Arithmetics: ++i costs less gas compared to i++ or i += 1
Agree and will fix.
Arithmetics: Use != 0 instead of > 0 for unsigned integers in require statements
Invalid. We tested the recommendation and got the following results:
createNFTDropCollection gas reporter results:
using > 0 (current):
- 319246 · 319578 · 319361
using != 0 (recommendation):
- 319252 · 319584 · 319367
impact: +6 gasUse short error messages
Agree but won't fix. We use up to 64 bytes, aiming to respect the incremental cost but 32 bytes is a bit too short to provide descriptive error messages for our users.
Duplicated require() checks should be refactored to a modifier or function
Agree, we'll consider this change.
Custom errors
Agree but won't fix at this time. We use these in the market but not in collections. Unfortunately custom errors are still not as good of an experience for users (e.g. on etherscan). We used them in the market originally because we were nearing the max contract size limit and this was a good way to reduce the bytecode. We'll consider this in the future as tooling continues to improve.
State variables should be cached
Agree, will make some improvements here.
Using bool for storage incurs overhead
Valid for cidToMinted, saving ~200 gas. Not seeing any benefit for assumePrimarySale, potentially because it's an immutable variable.
calldata
Valid & will fix. This saves ~50 gas on createNFTCollection and ~60 gas on createNFTDropCollectionWithPaymentFactory
Updated startsWith, NFTCollection, and the AddressLibrary & call
uints/ints smaller than 32 bytes (256 bits) incurs overhead.
Potentially valid but won't fix. This is uint32 to allow packing storage for a significant gas savings. Accepting the input as uint256 would incur additional overhead in checking for overflow before saving these values. Other inputs are logically limited to the size exposed, huge values cannot be used. Accepting the input as uint256 would incur additional overhead in checking for overflow before using these values.
Not using the named return variables when a function returns, wastes deployment gas
Agree for code consistency with other parts of our code. Saves 0.013 bytes on the bytecode size.
Gas Report
++icosts less gas compared toi++ori += 1!= 0instead of> 0for unsigned integers inrequirestatementsrequire()checks should be refactored to a modifier or functionboolfor storage incurs overheadcalldatainstead ofmemoryfor read-only arguments in external functionsuints/intssmaller than 32 bytes (256 bits) incurs overheadTotal: 121 instances over 13 issues
For-loops: Index initialized with default value
Uninitialized
uintvariables are assigned with a default value of0.Thus, in for-loops, explicitly initializing an index with
0costs unnecesary gas. For example, the following code:can be written without explicitly setting the index to
0:There are 5 instances of this issue:
For-Loops: Cache array length outside of loops
Reading an array's length at each iteration has the following gas overheads:
mload(3 gas)calldataload(3 gas)Caching the length changes each of these to a
DUP<N>(3 gas), and gets rid of the extraDUP<N>needed to store the stack offset. This would save around 3 gas per iteration.For example:
can be changed to:
There are 4 instances of this issue:
For-Loops: Index increments can be left unchecked
From Solidity v0.8 onwards, all arithmetic operations come with implicit overflow and underflow checks.
In for-loops, as it is impossible for the index to overflow, index increments can be left unchecked to save 30-40 gas per loop iteration.
For example, the code below:
can be changed to:
There are 5 instances of this issue:
Arithmetics:
++icosts less gas compared toi++ori += 1++icosts less gas compared toi++ori += 1for unsigned integers, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.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 of2, thus it costs more gas.The same logic applies for
--iandi--.There are 2 instances of this issue:
Arithmetics: Use
!= 0instead of> 0for unsigned integers inrequirestatementsA variable of type
uintwill never go below 0. Thus, checking!= 0rather than> 0is sufficient, which would save 6 gas per instance.There are 3 instances of this issue:
Errors: Reduce the length of error messages (long revert strings)
As seen here, revert strings longer than 32 bytes would incur an additional
mstore(3 gas) for each extra 32-byte chunk. Furthermore, there are additional runtime costs for memory expansion and stack operations when the revert condition is met.Thus, shortening revert strings to fit within 32 bytes would help to reduce runtime gas costs when the revert condition is met.
There are 28 instances of this issue:
Duplicated
require()checks should be refactored to a modifier or functionIf a
require()statement that is used to validate function parameters or global variables is present across multiple functions in a contract, it should be rewritten into modifier if possible. This would help to reduce code deployment size, which saves gas, and improve code readability.There are 2 instances of this issue:
Errors: Use custom errors instead of revert strings
Since Solidity v0.8.4, custom errors can be used rather than
requirestatements.Taken from Custom Errors in Solidity:
Custom errors reduce runtime gas costs as they save about 50 gas each time they are hit by avoiding having to allocate and store the revert string. Furthermore, not definiting error strings also help to reduce deployment gas costs.
There are 28 instances of this issue:
State variables should be cached in stack variables rather than re-reading them from storage
If a state variable is read from storage multiple times in a function, it should be cached in a stack variable.
Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.
There are 4 instances of this issue:
maxTokenIdin_mint():baseURI_in_baseURI():latestTokenIdinmintCountTo():maxTokenIdin_updateMaxTokenId():Using
boolfor storage incurs overheadDeclaring storage variables as
boolis more expensive compared touint256, as explained here:Use
uint256(1)anduint256(2)rather than true/false to avoid a Gwarmaccess (100 gas) for the extraSLOAD, and to avoid Gsset (20000 gas) when changing from 'false' to 'true', after having been 'true' in the past.There are 2 instances of this issue:
Use
calldatainstead ofmemoryfor read-only arguments in external functionsWhen an external function with a
memoryarray is called, theabi.decode()step has to use a for-loop to copy each index of thecalldatato thememoryindex. Each iteration of this for-loop costs at least 60 gas (i.e.60 * <mem_array>.length).Using
calldatadirectly instead ofmemoryhelps to save gas as values are read directly fromcalldatausingcalldataload, thus removing the need for such a loop in the contract code during runtime execution.Also, structs have the same overhead as an array of length one.
There are 3 instances of this issue:
Usage of
uints/intssmaller than 32 bytes (256 bits) incurs overheadAs seen from here:
However, this does not apply to storage values as using reduced-size types might be beneficial to pack multiple elements into a single storage slot. Thus, where appropriate, use
uint256/int256and downcast when needed.There are 16 instances of this issue:
Not using the named return variables when a function returns, wastes deployment gas
There are 19 instances of this issue: