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: 107/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.6 USDC - $20.60
contracts/PercentSplitETH.sol: lines 115, 135, 320contracts/libraries/BytesLibrary.sol:lines 25, 45contracts/mixins/shared/MarketFees.sol: lines 126, 198, 484This extra calculation can be avoided by caching the length of the array outside of the loop in a Variable
contracts/PercentSplitETH.sol: lines 115, 135, 227, 261, 320
contracts/mixins/shared/MarketFees.sol: lines 126, 198, 503
!=) is less costly than the greater than (>) check. So for unsigned integers, prefer using != 0, rather than > 0contracts/NFTDropCollection.sol: lines 88, 130, 131
contracts/mixins/shared/MarketFees.sol: lines 244
contracts/mixins/shared/OZERC165Checker.sol: lines 36
revert/require statements should be less than 32 bytes in UTF-8 encoding.
Lengths of strings can be quickly checked in https://mothereff.in/byte-countercontracts/NFTCollection.sol: lines 158, 263, 264, 268, 327
contracts/NFTCollectionFactory.sol: lines 173, 182, 203, 227, 262
contracts/NFTDropCollection.sol: lines 88, 93, 130, 131, 172, 179, 238
contracts/PercentSplitETH.sol: lines 121, 136, 148
contracts/libraries/AddressLibrary.sol: lines 31
contracts/mixins/collections/SequentialMintCollection.sol: lines 58, 63, 74, 87, 88, 89
contracts/mixins/roles/AdminRole.sol: lines 19
contracts/mixins/roles/MinterRole.sol: lines 22
contracts/mixins/shared/ContractFactory.sol: lines 22
contracts/mixins/shared/ContractFactory.sol: lines 31
contracts/mixins/treasury/AdminRole.sol: lines 20
contracts/mixins/treasury/OZAccessControlUpgradeable.sol: lines 137, 152
transfer should be replaced by call function
Consensys SC security best practicescontracts/PercentSplitETH.sol: lines 236, 245
#0 - HardlyDifficult
2022-08-18T23:47:50Z
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.
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
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.
Unsafe ERC20 Operation
This should have been submitted as a QA report