HardlyDifficult
commented
2 years ago
- Use assembly to check for zero address.
Won't fix. While this may save a tiny bit of gas, we reserve the use of assembly to areas where it would provide significant benefit or accomplish something that's otherwise not possible with Solidity alone.
- 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.
- Use != 0 instead of > 0
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 gas
- Functions guaranteed to revert when called by normal users can be marked payable
Disagree. This could save gas but 1) this seems like a poor practice to encourage and could be error prone. and 2) we don't care about optimizing admin-only actions.
- ++i costs less than i++
Agree and will fix.
- Use 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.
- Don't initialize variables with default values.
Invalid. This optimization technique is no longer applicable with the current version of Solidity.
[G-08] ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops.
5 examples provided, 4 are already unchecked - so those are 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.
- 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.
- Using private rather than public for constants to saves gas.
Agree but won't fix. For ease of use and consistency we will continue to expose some constants publicly.
- += COSTS MORE GAS THAN = + FOR STATE VARIABLES
No impact. We made the recommended change and ran gas-reporter for our entire test suite. 0 impact detected. Even if there were a small benefit, we feel this hurts readability and would not be worth trivial savings.
- Using bools for storage incurs overhead.
Valid for cidToMinted, saving ~200 gas. Not seeing any benefit for assumePrimarySale, potentially because it's an immutable variable.
[G-13] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead.
Potentially valid but won't fix. The input here is 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.
[G-01] Use assembly to check for zero address.
Impact
Save 6 gas when assembly is used to check for zero address. e.g:
Findings:
[G-02] Use custom errors rather than
revert()/require()string to save gasImpact
Custom errors are available from solidity version 0.8.4, it saves around 50 gas each time they are hit by avoiding having to allocate and store the revert string.
Findings:
[G-03] Using
> 0costs more gas than!= 0when used on a uint in arequire()statement.Impact
When working with unsigned integer types, comparisons with != 0 are cheaper than with > 0 . This changes saves 6 gas per instance.
Findings:
[G-04] Functions guaranteed to revert when called by normal users can be declared as payable.
Impact
If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost.
Findings:
[G-05] ++i costs less gas than i++, especially when it's used in for loops.
Impact
Saves 6 gas per loop.
Findings:
[G-06] Revert message greater than 32 bytes
Impact
Keep revert message lower than or equal to 32 bytes to save gas.
Findings:
[G-07] Explicit initialization with zero not required
Impact
Explicit initialization with zero is not required for variable declaration because uints are 0 by default. Removing this will reduce contract size and save a bit of gas.
Findings:
[G-08]
++i/i++should beunchecked{++i}/unchecked{i++}when it is not possible for them to overflow, as is the case when used in for- and while-loops.Impact
The
uncheckedkeyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop.Findings:
[G-09] Cache Array Length Outside of Loop
Impact
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.
Findings:
[G-10] Using private rather than public for constants to saves gas.
Impact
If needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table.
Findings:
[G-11]
X += Ycosts more gas thanX = X + Yfor state variables.Impact
Consider changing
X += YtoX = X + Yto save gas.Findings:
[G-12] Using bools for storage incurs overhead.
Impact
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use
uint256(1)anduint256(2)for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the pastFindings:
[G-13] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead.
Impact
When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size. https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed.
Findings: