Platform: Code4rena
Start Date: 22/08/2022
Pot Size: $50,000 USDC
Total HM: 4
Participants: 160
Period: 5 days
Judge: gzeon
Total Solo HM: 2
Id: 155
League: ETH
Rank: 75/160
Findings: 2
Award: $52.10
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: IllIllI
Also found by: 0bi, 0x040, 0x1337, 0x1f8b, 0xDjango, 0xNazgul, 0xNineDec, 0xRajeev, 0xSky, 0xSmartContract, 0xbepresent, 0xkatana, 0xmatt, 8olidity, Aymen0909, Bjorn_bug, Bnke0x0, CertoraInc, Ch_301, Chom, CodingNameKiki, Deivitto, DevABDee, DimitarDimitrov, Dravee, ElKu, Funen, GalloDaSballo, GimelSec, Guardian, Haruxe, JC, JansenC, Jeiwan, JohnSmith, KIntern_NA, Lambda, LeoS, Noah3o6, Olivierdem, R2, RaymondFam, Respx, ReyAdmirado, Rohan16, Rolezn, Ruhum, Saintcode_, Sm4rty, SooYa, Soosh, TomJ, Tomo, Trabajo_de_mates, Waze, _Adam, __141345__, ajtra, android69, asutorufos, auditor0517, berndartmueller, bobirichman, brgltd, c3phas, cRat1st0s, carlitox477, catchup, cccz, csanuragjain, d3e4, delfin454000, dipp, djxploit, durianSausage, erictee, exd0tpy, fatherOfBlocks, gogo, hyh, ladboy233, lukris02, mics, mrpathfindr, natzuu, oyc_109, p_crypt0, pashov, pauliax, pfapostol, prasantgupta52, rajatbeladiya, rbserver, ret2basic, rfa, robee, rokinot, rvierdiiev, sach1r0, saian, seyni, shenwilly, sikorico, simon135, sryysryy, sseefried, throttle, tnevler, tonisives, wagmi, xiaoming90, yixxas, z3s, zkhorse, zzzitron
35.4387 USDC - $35.44
Description: Non Library/Interface contracts should be deployed with a locked pragma version. This prevents the contract being deployed with a version that wasn't thoroughly tested against in development.
LOC: NounsDAOLogicV1.sol#L61 NounsDAOLogicV2.sol#L53 NounsDAOProxy.sol#L36 ERC721Checkpointable.sol#L35 ERC721Enumerable.sol#L28
Recommendation: Lock the pragma the version that was used in testing.
Description: A comment has been copy and pasted from a previous function and doesnt match the code it is referencing.
Recommendation: Delete/update to correct comment.
Description: There is a typo in the comment, 4,000 is written where 6,000 was intended.
Recommendation: Change to // 6,000 basis points or 60%
🌟 Selected for report: IllIllI
Also found by: 0x040, 0x1f8b, 0xDjango, 0xNazgul, 0xNineDec, 0xSmartContract, 0xbepresent, 0xc0ffEE, 0xkatana, 2997ms, ACai, Amithuddar, Aymen0909, Ben, BipinSah, Bjorn_bug, Bnke0x0, CertoraInc, Ch_301, Chom, CodingNameKiki, Deivitto, DevABDee, DimitarDimitrov, Diraco, Dravee, ElKu, EthLedger, Fitraldys, Funen, GalloDaSballo, GimelSec, Guardian, IgnacioB, JC, JohnSmith, Junnon, KIntern_NA, Lambda, LeoS, Noah3o6, Olivierdem, Polandia94, R2, Randyyy, RaymondFam, Respx, ReyAdmirado, Rohan16, RoiEvenHaim, Rolezn, Ruhum, SaharAP, Saintcode_, SerMyVillage, Shishigami, Sm4rty, SooYa, TomJ, Tomio, Tomo, Waze, Yiko, _Adam, __141345__, a12jmx, ajtra, ak1, bobirichman, brgltd, bulej93, c3phas, cRat1st0s, carlitox477, catchup, ch0bu, d3e4, delfin454000, djxploit, durianSausage, erictee, exolorkistis, fatherOfBlocks, francoHacker, gogo, hyh, ignacio, jag, joestakey, karanctf, ladboy233, lucacez, lukris02, m_Rassska, martin, medikko, mics, mrpathfindr, natzuu, newfork01, oyc_109, pauliax, peritoflores, pfapostol, prasantgupta52, rbserver, ret2basic, rfa, robee, rokinot, rotcivegaf, rvierdiiev, sach1r0, saian, samruna, seyni, shark, shr1ftyy, sikorico, simon135, sryysryy, tay054, tnevler, wagmi, zishansami
16.6568 USDC - $16.66
Description: In NounsDAOLogicV2 the exact same require statement that is checked in burnVetoPower on line 853 is also checked in setVetoer on line 840. It is unnecessary to check that msg.sender == vetoer twice and can be removed from burnVetoPower. (Same for v1 just on line 651 instead)
LOC: NounsDAOLogicV2.sol#L853 NounsDAOLogicV1.sol#L651
Recommendation: Remove the duplicate require statement from burnVetoPower.
Description: When incrementing i in for loops there is no chance of overflow so unchecked can be used to save gas. You can also save a small amount of gas by replacing post increments with pre. It is also more expensive to loop over a state variable and you can save some gas by cacheing the value.
LOC: NounsDAOLogicV1.sol#L281-L289 NounsDAOLogicV1.sol#L319-L327 NounsDAOLogicV1.sol#L346-L354 NounsDAOLogicV1.sol#L371-L379 NounsDAOLogicV2.sol#L292-L300 NounsDAOLogicV2.sol#L330-L338 NounsDAOLogicV2.sol#L357-L365 NounsDAOLogicV2.sol#L382-L390
Recommendation: Change for loops from:
for (uint256 i; i < variable.length; i++) { } to uint256 len = variable.length; (uint256 i; i < len;) { // for loop body unchecked { ++i; } }
Description: When initialising state variables to their default value it is cheaper to just leave the value blank.
LOC: ERC721Checkpointable.sol#L41 NounsDAOLogicV2.sol#L231 NounsDAOLogicV2.sol#L238-L243
Recommendation: Remove = 0/false from the variable initialisations.
Description: As your using a solidity version > 0.8.4 you can replace revert strings with cheaper custom errors.
LOC: There are 95 revert strings throughout the various contracts that can be replaced with custom errors.
Recommendation: Replace revert strings with custom errors.
Description: If you opt not to use custom errors keeping revert strings <= 32 bytes in length will save gas.
LOC: There are 87 revert strings > 32 bytes in length throughout the various contracts.
Recommendation: Either replace with custom errors or shorten the revert strings to be < 32 bytes in length.