Platform: Code4rena
Start Date: 11/08/2022
Pot Size: $40,000 USDC
Total HM: 8
Participants: 108
Period: 4 days
Judge: hickuphh3
Total Solo HM: 2
Id: 152
League: ETH
Rank: 90/108
Findings: 1
Award: $20.68
馃専 Selected for report: 0
馃殌 Solo Findings: 0
馃専 Selected for report: Dravee
Also found by: 0x040, 0x1f8b, 0xDjango, 0xHarry, 0xNazgul, 0xSmartContract, 0xbepresent, 0xkatana, Amithuddar, Aymen0909, Bnke0x0, Chom, CodingNameKiki, Deivitto, DevABDee, Diraco, ElKu, Fitraldys, Funen, IllIllI, JC, LeoS, Metatron, MiloTruck, Noah3o6, ReyAdmirado, Rohan16, Rolezn, Saw-mon_and_Natalie, Sm4rty, SpaceCake, TomJ, Tomio, Trabajo_de_mates, Waze, Yiko, __141345__, ajtra, apostle0x01, bobirichman, brgltd, bulej93, c3phas, cRat1st0s, carlitox477, d3e4, durianSausage, erictee, fatherOfBlocks, gerdusx, gogo, hakerbaya, ignacio, jag, joestakey, ladboy233, medikko, mics, newfork01, oyc_109, pfapostol, robee, rvierdiiev, sach1r0, saian, samruna, sikorico, simon135, wagmi, zeesaw, zkhorse, zuhaibmohd
20.6764 USDC - $20.68
Title: Consider make constant as private to save gas
Proof of Concept: NFTDropMarketFixedPriceSale.sol#L70 MinterRole.sol#L19
Recommended Mitigation Steps:
I suggest changing the visibility from public to internal or private
Title: Expression for constant values such as a call to keccak256(), should use immutable rather than constant
Proof of Concept: NFTDropMarketFixedPriceSale.sol#L70 MinterRole.sol#L19
Recommended Mitigation Steps:
Change from constant to immutable
reference: here
Title: Comparison operators
Proof of Concept: NFTDropMarketFixedPriceSale.sol#L240
Recommended Mitigation Steps:
Replace <= with <, and >= with > for gas optimization
Title: Use unchecked can save gas
Proof of Concept:
NFTDropMarketFixedPriceSale.sol#L245 (because of if() L#240)
Recommended Mitigation Steps:
Use unchecked
Title: Reduce the size of error messages (Long revert Strings)
Impact: 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.
Proof of Concept: AddressLibrary.sol#L31 NFTCollectionFactory.sol#L227 NFTCollectionFactory.sol#L262
Recommended Mitigation Steps: Consider shortening the revert strings to fit in 32 bytes
Title: Custom errors from Solidity 0.8.4 are cheaper than revert strings
Impact: Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) while providing the same amount of information
Custom errors are defined using the error statement reference: https://blog.soliditylang.org/2021/04/21/custom-errors/
Proof of Concept: AddressLibrary.sol#L31 NFTCollectionFactory.sol#L227 NFTCollectionFactory.sol#L262
Recommended Mitigation Steps: Replace require statements with custom errors.
Title: Using != in require statement is more gas efficient
Proof of Concept: NFTDropCollection.sol#L130-L131
Recommended Mitigation Steps:
Change > 0 to != 0
#0 - batu-inal
2022-08-19T12:30:38Z
Consider make constant as private to save gas
Agree but won't fix. For ease of use and consistency we will continue to expose some constants publicly.
Expression for constant values such as a call to keccak256(), should use immutable rather than constant
Valid but won't fix. While there may be gas savings here we like the readability and maintainability of this pattern. Adding hard-coded hashes makes the code fragile and harder to read.
Comparison operators
Invalid. This is semantically different and violates the boundary case.
Use unchecked can save gas
Valid. We will make this change.
Reduce the size of error messages (Long revert Strings)
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.
Custom errors from Solidity 0.8.4 are cheaper than revert strings
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.
Using != in require statement is more gas efficient
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