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: 104/108
Findings: 1
Award: $20.60
š 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.6015 USDC - $20.60
There is no risk that the loop counter can overflow, using solidity's unchecked block saves gas.
There are 5 instances of this issue:
./mixins/shared/MarketFees.sol:126: for (uint256 i = 0; i < creatorRecipients.length; ++i) { ./mixins/shared/MarketFees.sol:198: for (uint256 i = 0; i < creatorShares.length; ++i) { ./mixins/shared/MarketFees.sol:484: for (uint256 i = 0; i < creatorRecipients.length; ++i) { ./libraries/BytesLibrary.sol:25: for (uint256 i = 0; i < 20; ++i) { ./libraries/BytesLibrary.sol:44: for (uint256 i = 0; i < 4; ++i) {
Gas Report using this gist:
āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¬āāāāāāāāāāāāāāāāāā¬āāāāāāā¬āāāāāāāāā¬āāāāāāā¬āāāāāāāāāā® ā src/test/Unchecked.t.sol:Contract0 contract ā ā ā ā ā ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāŖāāāāāāāāāāāāāāāāāāŖāāāāāāāŖāāāāāāāāāŖāāāāāāāŖāāāāāāāāāā” ā Deployment Cost ā Deployment Size ā ā ā ā ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¼āāāāāāāāāāāāāāāāāā¼āāāāāāā¼āāāāāāāāā¼āāāāāāā¼āāāāāāāāāā¤ ā 55105 ā 307 ā ā ā ā ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¼āāāāāāāāāāāāāāāāāā¼āāāāāāā¼āāāāāāāāā¼āāāāāāā¼āāāāāāāāāā¤ ā Function Name ā min ā avg ā median ā max ā # calls ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¼āāāāāāāāāāāāāāāāāā¼āāāāāāā¼āāāāāāāāā¼āāāāāāā¼āāāāāāāāāā¤ ā withOutUnChecked ā 2068 ā 2068 ā 2068 ā 2068 ā 1 ā ā°āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā“āāāāāāāāāāāāāāāāāā“āāāāāāā“āāāāāāāāā“āāāāāāā“āāāāāāāāāāÆ āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¬āāāāāāāāāāāāāāāāāā¬āāāāāāā¬āāāāāāāāā¬āāāāāāā¬āāāāāāāāāā® ā src/test/Unchecked.t.sol:Contract1 contract ā ā ā ā ā ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāŖāāāāāāāāāāāāāāāāāāŖāāāāāāāŖāāāāāāāāāŖāāāāāāāŖāāāāāāāāāā” ā Deployment Cost ā Deployment Size ā ā ā ā ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¼āāāāāāāāāāāāāāāāāā¼āāāāāāā¼āāāāāāāāā¼āāāāāāā¼āāāāāāāāāā¤ ā 53705 ā 300 ā ā ā ā ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¼āāāāāāāāāāāāāāāāāā¼āāāāāāā¼āāāāāāāāā¼āāāāāāā¼āāāāāāāāāā¤ ā Function Name ā min ā avg ā median ā max ā # calls ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¼āāāāāāāāāāāāāāāāāā¼āāāāāāā¼āāāāāāāāā¼āāāāāāā¼āāāāāāāāāā¤ ā withUnchecked ā 1408 ā 1408 ā 1408 ā 1408 ā 1 ā ā°āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā“āāāāāāāāāāāāāāāāāā“āāāāāāā“āāāāāāāāā“āāāāāāā“āāāāāāāāāāÆ
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.
There are 4 instances of this issue:
./mixins/shared/MarketFees.sol:126: for (uint256 i = 0; i < creatorRecipients.length; ++i) { ./mixins/shared/MarketFees.sol:198: for (uint256 i = 0; i < creatorShares.length; ++i) { ./mixins/shared/MarketFees.sol:484: for (uint256 i = 0; i < creatorRecipients.length; ++i) { ./mixins/shared/MarketFees.sol:503: for (uint256 i = 1; i < creatorRecipients.length; ) {
Gas Report using this gist:
āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¬āāāāāāāāāāāāāāāāāā¬āāāāāāā¬āāāāāāāāā¬āāāāāāā¬āāāāāāāāāā® ā src/test/ArrayLength.t.sol:Contract0 contract ā ā ā ā ā ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāŖāāāāāāāāāāāāāāāāāāŖāāāāāāāŖāāāāāāāāāŖāāāāāāāŖāāāāāāāāāā” ā Deployment Cost ā Deployment Size ā ā ā ā ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¼āāāāāāāāāāāāāāāāāā¼āāāāāāā¼āāāāāāāāā¼āāāāāāā¼āāāāāāāāāā¤ ā 137640 ā 412 ā ā ā ā ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¼āāāāāāāāāāāāāāāāāā¼āāāāāāā¼āāāāāāāāā¼āāāāāāā¼āāāāāāāāāā¤ ā Function Name ā min ā avg ā median ā max ā # calls ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¼āāāāāāāāāāāāāāāāāā¼āāāāāāā¼āāāāāāāāā¼āāāāāāā¼āāāāāāāāāā¤ ā withArrayLengthInside ā 3108 ā 3108 ā 3108 ā 3108 ā 1 ā ā°āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā“āāāāāāāāāāāāāāāāāā“āāāāāāā“āāāāāāāāā“āāāāāāā“āāāāāāāāāāÆ āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¬āāāāāāāāāāāāāāāāāā¬āāāāāāā¬āāāāāāāāā¬āāāāāāā¬āāāāāāāāāā® ā src/test/ArrayLength.t.sol:Contract1 contract ā ā ā ā ā ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāŖāāāāāāāāāāāāāāāāāāŖāāāāāāāŖāāāāāāāāāŖāāāāāāāŖāāāāāāāāāā” ā Deployment Cost ā Deployment Size ā ā ā ā ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¼āāāāāāāāāāāāāāāāāā¼āāāāāāā¼āāāāāāāāā¼āāāāāāā¼āāāāāāāāāā¤ ā 137840 ā 413 ā ā ā ā ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¼āāāāāāāāāāāāāāāāāā¼āāāāāāā¼āāāāāāāāā¼āāāāāāā¼āāāāāāāāāā¤ ā Function Name ā min ā avg ā median ā max ā # calls ā āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā¼āāāāāāāāāāāāāāāāāā¼āāāāāāā¼āāāāāāāāā¼āāāāāāā¼āāāāāāāāāā¤ ā withArrayLengthOutside ā 2813 ā 2813 ā 2813 ā 2813 ā 1 ā ā°āāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāāā“āāāāāāāāāāāāāāāāāā“āāāāāāā“āāāāāāāāā“āāāāāāā“āāāāāāāāāāÆ
Custom errors are available since solidity version 0.8.4. Custom errors save ~50 gas avoiding having to allocate and store the revert string. Custom errors are been used in the NFTDropMarketFixedPriceSale contract
There are 13 instances of this issue:
contracts/NFTCollectionFactory.sol::262 => require(bytes(symbol).length != 0, "NFTCollectionFactory: Symbol is required"); contracts/NFTCollection.sol::158 => require(tokenCreatorPaymentAddress != address(0), "NFTCollection: tokenCreatorPaymentAddress is required"); contracts/NFTCollection.sol::263 => require(bytes(tokenCID).length != 0, "NFTCollection: tokenCID is required"); contracts/NFTCollection.sol::264 => require(!cidToMinted[tokenCID], "NFTCollection: NFT was already minted"); contracts/NFTCollection.sol::268 => require(maxTokenId == 0 || tokenId <= maxTokenId, "NFTCollection: Max token count has already been minted"); contracts/NFTCollection.sol::327 => require(_exists(tokenId), "NFTCollection: URI query for nonexistent token"); contracts/NFTDropCollection.sol::88 => require(bytes(_baseURI).length > 0, "NFTDropCollection: `_baseURI` must be set"); contracts/NFTDropCollection.sol::93 => require(postRevealBaseURIHash != bytes32(0), "NFTDropCollection: Already revealed"); contracts/NFTDropCollection.sol::130 => require(bytes(_symbol).length > 0, "NFTDropCollection: `_symbol` must be set"); contracts/NFTDropCollection.sol::131 => require(_maxTokenId > 0, "NFTDropCollection: `_maxTokenId` must be set"); contracts/NFTDropCollection.sol::172 => require(count != 0, "NFTDropCollection: `count` must be greater than 0"); contracts/NFTDropCollection.sol::179 => require(latestTokenId <= maxTokenId, "NFTDropCollection: Exceeds max tokenId"); contracts/NFTDropCollection.sol::238 => require(_postRevealBaseURIHash != bytes32(0), "NFTDropCollection: use `reveal` instead");
#0 - HardlyDifficult
2022-08-17T15:20:17Z
1 - Unnecessary checked arithmetic in for loop.
4 of the 5 examples listed are 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.
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.
Use custom errors rather than revert()/require() strings to save gas
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.