Nouns DAO contest - karanctf's results

A DAO-driven NFT project on Ethereum.

General Information

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

Nouns DAO

Findings Distribution

Researcher Performance

Rank: 136/160

Findings: 1

Award: $16.66

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Nouns DAO

Findings Information

🌟 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

Awards

16.6568 USDC - $16.66

Labels

bug

G (Gas Optimization)

External Links

Link to GitHub issue

[G-01] Splitting `require()` statements that use `&&` saves gas

If you're using the Optimizer at 200, instead of using the && operator in a single require statement to check multiple conditions, I suggest using multiple require statements with 1 condition per require statement:

governance/NounsDAOLogicV2.sol:

	137:   require(
            votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD,
            'NounsDAO::initialize: invalid voting period'
        );


	141:    require(
            votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY,
            'NounsDAO::initialize: invalid voting delay'
        );

	145:    require(
            proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS,
            'NounsDAO::initialize: invalid proposal threshold bps'
        );

	201:      require(
            targets.length == values.length &&
                targets.length == signatures.length &&
                targets.length == calldatas.length,
            'NounsDAO::propose: proposal function information arity mismatch'
        );

   623:      require(
       		newVotingDelay >= MIN_VOTING_DELAY && newVotingDelay <= MAX_VOTING_DELAY,
           'NounsDAO::_setVotingDelay: invalid voting delay'
       );
   639:    require(
       newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD,
       'NounsDAO::_setVotingPeriod: invalid voting period'
       );

   656:    require(
           newProposalThresholdBPS >= MIN_PROPOSAL_THRESHOLD_BPS &&
               newProposalThresholdBPS <= MAX_PROPOSAL_THRESHOLD_BPS,
           'NounsDAO::_setProposalThreshold: invalid proposal threshold bps'
       );

   677:    require(
           newMinQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS_LOWER_BOUND &&
               newMinQuorumVotesBPS <= MIN_QUORUM_VOTES_BPS_UPPER_BOUND,
           'NounsDAO::_setMinQuorumVotesBPS: invalid min quorum votes bps'
       );

   819:    require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only');

governance/NounsDAOLogicV2.sol:

	126:    require(
            votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD,
            'NounsDAO::initialize: invalid voting period'
        );

	130:     require(
            votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY,
            'NounsDAO::initialize: invalid voting delay'
        );

	134:     require(
            proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS,
            'NounsDAO::initialize: invalid proposal threshold'
        );

	138:     require(
            quorumVotesBPS_ >= MIN_QUORUM_VOTES_BPS && quorumVotesBPS_ <= MAX_QUORUM_VOTES_BPS,
            'NounsDAO::initialize: invalid proposal threshold'
        );

	191:    require(
            targets.length == values.length &&
                targets.length == signatures.length &&
                targets.length == calldatas.length,
            'NounsDAO::propose: proposal function information arity mismatch'
        );

	547:      require(
            newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD,
            'NounsDAO::_setVotingPeriod: invalid voting period'
        );

	564:       require(
            newProposalThresholdBPS >= MIN_PROPOSAL_THRESHOLD_BPS &&
                newProposalThresholdBPS <= MAX_PROPOSAL_THRESHOLD_BPS,
            'NounsDAO::_setProposalThreshold: invalid proposal threshold'
        );

	582:    require(
            newQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS && newQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS,
            'NounsDAO::_setProposalThreshold: invalid proposal threshold'
        );

	617:     require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only');

[G-2] `++i` costs less gas compared to `i++` or `i += 1`

++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Instances include:

governance/NounsDAOLogicV1.sol:281          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV1.sol:319          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV1.sol:346          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV1.sol:371          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV2.sol:292          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV2.sol:330          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV2.sol:357          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV2.sol:381          for (uint256 i = 0; i < proposal.targets.length; i++) {

[G-3] No need to explicitly initialize variables with default values

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address...).

base/ERC721Checkpointable.sol:41            uint8 public constant decimals = 0;
base/ERC721Checkpointable.sol:181           uint32 lower = 0;
governance/NounsDAOLogicV1.sol:281          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV1.sol:319          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV1.sol:346          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV1.sol:371          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV2.sol:292          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV2.sol:330          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV2.sol:357          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV2.sol:381          for (uint256 i = 0; i < proposal.targets.length; i++) {

[G-4] array.length should not be looked up in every loop of a for-loop

governance/NounsDAOLogicV1.sol:281          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV1.sol:319          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV1.sol:346          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV1.sol:371          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV2.sol:292          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV2.sol:330          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV2.sol:357          for (uint256 i = 0; i < proposal.targets.length; i++) {
governance/NounsDAOLogicV2.sol:381          for (uint256 i = 0; i < proposal.targets.length; i++) {

Nouns DAO contest - karanctf's results

A DAO-driven NFT project on Ethereum.

General Information

Findings Distribution

Researcher Performance

External Links

[G-87] Gas Report - $16.66

Findings Information

Awards

Labels

External Links

[G-01] Splitting require() statements that use && saves gas

[G-2] ++i costs less gas compared to i++ or i += 1

[G-3] No need to explicitly initialize variables with default values

[G-4] array.length should not be looked up in every loop of a for-loop

[G-01] Splitting `require()` statements that use `&&` saves gas

[G-2] `++i` costs less gas compared to `i++` or `i += 1`