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: 59/160
Findings: 2
Award: $52.11
🌟 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.4484 USDC - $35.45
In theory, comments over 79 characters should wrap using multi-line comment syntax. Even if somewhat longer comments are acceptable, there are cases where very long comments interfere with readability. Below are instances of extra-long comments whose readability could be improved by wrapping, as shown:
NounsDAOInterfaces.sol: L179-182
/// @notice The number of votes needed to create a proposal at the time of proposal creation. *DIFFERS from GovernerBravo uint256 proposalThreshold; /// @notice The number of votes in support of a proposal required in order for a quorum to be reached and for a vote to succeed at the time of proposal creation. *DIFFERS from GovernerBravo uint256 quorumVotes;
Suggestion:
/// @notice The number of votes needed to create a proposal at the time /// of proposal creation — *DIFFERS from GovernerBravo. uint256 proposalThreshold; /// @notice The minimum number of votes in support of a proposal required /// in order for a quorum to be reached and for a vote to succeed at the time /// of proposal creation — *DIFFERS from GovernerBravo. uint256 quorumVotes;
Similarly for the following:
NounsDAOInterfaces.sol: L279-282
NounsDAOInterfaces.sol: L373-376
/// @notice The basis point number of votes in support of a proposal required in order for a quorum to be reached and for a vote to succeed. *DIFFERS from GovernerBravo
Suggestion:
/// @notice The basis point number of votes in support of a proposal required in order /// for a quorum to be reached and for a vote to succeed — *DIFFERS from GovernerBravo.
Similarly for the following:
/// @notice: Unlike GovernerBravo, votes are considered from the block the proposal was created in order to normalize quorumVotes and proposalThreshold metrics
Suggestion:
/// @notice: Unlike GovernerBravo, votes are considered from the block the proposal /// was created in order to normalize quorumVotes and proposalThreshold metrics.
Similarly for the following:
ERC721Checkpointable.sol: L104
ERC721Checkpointable.sol: L198
* @param params Configurable parameters for calculating the quorum based on againstVotes. See `DynamicQuorumParams` definition for additional details.
Suggestion:
* @param params Configurable parameters for calculating the quorum based on againstVotes — * see `DynamicQuorumParams` definition for additional details.
* @dev Admin function to begin change of admin. The newPendingAdmin must call `_acceptAdmin` to finalize the transfer.
Suggestion:
* @dev Admin function to begin change of admin — the newPendingAdmin * must call `_acceptAdmin` to finalize the transfer.
Similarly for the following:
Require message is inadequateA revert string should provide enough information for users to understand reason for failure
NounsDAOLogicV2.sol: L1018-1021
function safe32(uint256 n, string memory errorMessage) internal pure returns (uint32) { require(n <= type(uint32).max, errorMessage); return uint32(n); }
Similarly for the following:
ERC721Checkpointable.sol: L253-256
ERC721Checkpointable.sol: L258-261
ERC721Checkpointable.sol: L263-271
ERC721Checkpointable.sol: L273-280
The same typo (setable) occurs in all fifteen lines referenced below:
Example (NounsDAOLogicV2.sol: L61):
/// @notice The minimum setable proposal threshold
Change setable to settable in each case
The same typo (contructor) occurs in both lines below:
* @notice Used to initialize the contract during delegator contructor
Change contructor to constructor in both cases
The same typo (priviledges) occurs in both lines below:
* @notice Burns veto priviledges
Change priviledges to privileges in both cases
🌟 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
Require revert string is too longThe revert strings below should be shortened to 32 characters or fewer to save gas or else consider using custom error codes, which could save even more gas
require(address(timelock) == address(0), 'NounsDAO::initialize: can only initialize once');
Similarly for the following error messages:
Note that I have not included L134 since its string length is 32
Note that I have not included L375 and L376 since their string lengths are less than 32
Note that I have not included L123 since its string length is 32
Note that I have not included L365 since its string length is 28
ERC721Checkpointable.sol: L140
ERC721Checkpointable.sol: L141
ERC721Checkpointable.sol: L142
ERC721Checkpointable.sol: L164
I was not able to work on the other gas optimization opportunities