Nouns DAO contest - ElKu'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: 40/160

Findings: 2

Award: $52.65

🌟 Selected for report: 0

🚀 Solo Findings: 0

Non-critical Issues

Summary Of Findings:

 Issue
1Custom errors could be used with parameters for better user experience
2Checks for admin can be converted into a modifier
3Avoid hardcoding gas values

Details on Findings:

1. <ins>Custom errors could be used with parameters for better user experience</ins>

Custom errors can take in parameters which can help the user identify the exact reason for the revert. The following custom errors can take in a parameter to make the protocol more user-friendly.

File: NounsDAOLogicV2.sol

    error InvalidMinQuorumVotesBPS();
    error InvalidMaxQuorumVotesBPS();
    error MinQuorumBPSGreaterThanMaxQuorumBPS();
    error UnsafeUint16Cast();

2. <ins>Checks for admin can be converted into a modifier</ins>

The contracts, NounsDAOLogicV1 and NounsDAOLogicV2 have several functions where there is a need to check if the msg.sender is admin. The method used for this is very inconsistent within these contracts. For increasing readability and even for saving gas, its recommended to create a modifier for this with a custom error(instead of a revert string).

Instances below:

  1. NounsDAOLogicV1
Line 123:      require(msg.sender == admin, 'NounsDAO::initialize: admin only');
Line 530:      require(msg.sender == admin, 'NounsDAO::_setVotingDelay: admin only');
Line 546:      require(msg.sender == admin, 'NounsDAO::_setVotingPeriod: admin only');
Line 563:      require(msg.sender == admin, 'NounsDAO::_setProposalThresholdBPS: admin only');
Line 581:      require(msg.sender == admin, 'NounsDAO::_setQuorumVotesBPS: admin only');
Line 599:      require(msg.sender == admin, 'NounsDAO::_setPendingAdmin: admin only');     
  1. NounsDAOLogicV2
Line 134:      require(msg.sender == admin, 'NounsDAO::initialize: admin only');
Line 622:      require(msg.sender == admin, 'NounsDAO::_setVotingDelay: admin only');
Line 638:      require(msg.sender == admin, 'NounsDAO::_setVotingPeriod: admin only');
Line 655:      require(msg.sender == admin, 'NounsDAO::_setProposalThresholdBPS: admin only');
Line 674:      require(msg.sender == admin, 'NounsDAO::_setMinQuorumVotesBPS: admin only');
Line 702:      require(msg.sender == admin, 'NounsDAO::_setMaxQuorumVotesBPS: admin only');    
Line 727:      require(msg.sender == admin, 'NounsDAO::_setQuorumCoefficient: admin only');
Line 753:      if (msg.sender != admin) {
                    revert AdminOnly();
                }
Line 784:      if (msg.sender != admin) {
                    revert AdminOnly();
                }
Line 801:      require(msg.sender == admin, 'NounsDAO::_setPendingAdmin: admin only');     

An example mitigation would look like this:

// declare a modifier with "require string"
    modifier onlyAdmin {
        require(msg.sender == admin, "only admin");
        _;
    }
// OR you can declare a modifier with a "custom error"
    error AdminOnly();  //declare a custom error
    modifier onlyAdmin {
        if(msg.sender != admin)
            revert AdminOnly();
        _;
    }
// use it in a function
    function _withdraw() external onlyAdmin {
        // statements
    }

3. <ins>Avoid hardcoding gas values</ins>

In NounsDAOLogicV2 contract the _refundGas method uses calculations based on hardcoded gas values. This is not recommended. The gas cost of EVM instructions may change significantly during hard forks which will result in, the users who need to get a refund, gets less than what they are supposed to get. See SWC-134 for more information.

Concerned declarations are of MAX_REFUND_PRIORITY_FEE and REFUND_BASE_GAS

    /// @notice The maximum priority fee used to cap gas refunds in `castRefundableVote`
    uint256 public constant MAX_REFUND_PRIORITY_FEE = 2 gwei;

    /// @notice The vote refund gas overhead, including 7K for ETH transfer and 29K for general transaction overhead
    uint256 public constant REFUND_BASE_GAS = 36000;

Mitigation would be to declare them as state variables and make them settable.

Non-critical Issues

Summary Of Findings:

 Issue
1Unused variables should be indicated as such
2Spelling mistakes in Natspec comments
3Non-library/interface files should use fixed compiler versions, not floating ones
4Use a newer version of Solidity

Details on Findings:

1. <ins>Unused variables should be indicated as such</ins>

In NounsDAOLogicV2, the variable, MAX_QUORUM_VOTES_BPS is declared but not used. Either it should be removed or it should be mentioned in the comment to avoid confusion.

    /// @notice The maximum setable quorum votes basis points
    uint256 public constant MAX_QUORUM_VOTES_BPS = 2_000; // 2,000 basis points or 20%

2. <ins>Spelling mistakes in Natspec comments</ins>

The following spelling mistakes were noticed:

  1. NounsDAOLogicV2
Line 061:      setable 
Line 064:      setable 
Line 067:      setable 
Line 070:      setable 
Line 073:      setable 
Line 076:      setable 
Line 088:      setable 
Line 115:      contructor
Line 582:      caries
Line 848:      priviledges
  1. NounsDAOLogicV1
Line 069:      setable 
Line 072:      setable 
Line 075:      setable 
Line 078:      setable 
Line 081:      setable 
Line 084:      setable 
Line 087:      setable 
Line 090:      setable
Line 104:      contructor
Line 490:      caries
Line 646:      priviledges
  1. NounsDAOProxy
Line 072:      canceled
Line 197:      favor
Line 203:      canceled
Line 297:      favor
Line 303:      canceled
Line 383:      favor
Line 389:      canceled

3. <ins>Non-library/interface files should use fixed compiler versions, not floating ones</ins>

Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

https://swcregistry.io/docs/SWC-103

All the files under scope uses floating pragma like the one below:

pragma solidity ^0.8.6;

Mitigation would be to change it to this:

pragma solidity 0.8.6;

4. <ins>Use a newer version of Solidity</ins>

The codebase uses Solidity version 0.8.6 which was released in June 2021. Though its not possible to keep up with the version changes of Solidity, its advisable to use a relatively newer version. The current Solidity version is 0.8.16 which was released in August 2022 (almost one year later than the current version used by the codebase).

By using an older version you might be missing the following features:

  • Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value
  • Use a solidity version of at least 0.8.16 to have more efficient code for checked addition and subtraction.

See this for a more detailed list of features and bug fixes in each solidity version.

Gas Optimizations

Summary Of Findings:

 Issue
1propose function in NounsDAOLogicV1 and NounsDAOLogicV2 contract
2Optimizing the for loops
3Placing emit statements before state-variable update to save gas
4Remove redundant safe32 function.
5Use custom errors instead of revert strings

Detailed Report on Gas Optimization Findings:

1. <ins>propose function in NounsDAOLogicV1 and NounsDAOLogicV2 contract</ins>

Several gas optimizations can be applied to the propose function.

  1. State variable proposalCount is accessed 3 times, which could be cached to save gas. Also incrementing of this variable could be converted to ++proposalCount to save extra gas.
        uint256 cachedproposalCount = ++proposalCount;
        Proposal storage newProposal = proposals[cachedproposalCount];
        newProposal.id = cachedproposalCount;
  1. Several fields in the Proposal struct are written with their default values. These statements are redundant and can be removed to save gas.
        newProposal.eta = 0;
        newProposal.forVotes = 0;
        newProposal.againstVotes = 0;
        newProposal.abstainVotes = 0;
        newProposal.canceled = false;
        newProposal.executed = false;
        newProposal.vetoed = false;
  1. Line 237 could be rewritten to save local variables.
        latestProposalIds[newProposal.proposer] = newProposal.id;
    //could be rewritten as:
        latestProposalIds[newProposal.proposer] = cachedproposalCount;
  1. The emit statements could use local variables, instead of the state variables.
        emit ProposalCreated(
            cachedproposalCount,    //gas saving
            msg.sender,
            targets,
            values,
            signatures,
            calldatas,
            temp.startBlock,    //gas saving
            temp.endBlock,    //gas saving
            description
        );

        /// @notice Updated event with `proposalThreshold` and `quorumVotes`
        emit ProposalCreatedWithRequirements(
            cachedproposalCount,    //gas saving
            msg.sender,
            targets,
            values,
            signatures,
            calldatas,
            temp.startBlock,    //gas saving
            temp.endBlock,    //gas saving
            temp.proposalThreshold,    //gas saving
            newProposal.quorumVotes,
            description
        );
  1. The function is returning a recently written state variable, instead of which we could return the local variable.
      return newProposal.id;
   // can be written as:
      return cachedproposalCount;

Hardhat gas report for propose function:

 Original ReportOptimized ReportGas savings
Min34831233622412088
Max59968658759812088
Avg43321342111912094

We can see that we saved an average of 12094 gas.

As for NounsDAOLogicV2 contract, we are using the same function there as well, so we can account for a saving of 12000 gas there too.

Total Amount of gas saved so far = 12000 + 12000 = 24000.

2. <ins>Optimizing the for loops</ins>

The for loops in the code base generally follow the following pattern:

      for (uint256 i = 0; i < proposal.targets.length; i++) {
            // do something
        }

This can be optimized in 3 ways:

      len = proposal.targets.length;  // 1. cache the array length
      for (uint256 i; i < len; ) {  // 2. remove redundant initialization of i
            // do something
            unchecked {  // 3. using unchecked block for incrementing the index
              ++i;   //use pre-increment instead of post-increment
            }
        }

The following instances could be optimized in this way:

  1. NounsDAOLogicV2.sol
Line 292:        for (uint256 i = 0; i < proposal.targets.length; i++) 
Line 330:        for (uint256 i = 0; i < proposal.targets.length; i++)
Line 357:        for (uint256 i = 0; i < proposal.targets.length; i++)
Line 382:        for (uint256 i = 0; i < proposal.targets.length; i++)
  1. NounsDAOLogicV1.sol uses the same set of functions as NounsDAOLogicV2.
Line 281:        for (uint256 i = 0; i < proposal.targets.length; i++) 
Line 319:        for (uint256 i = 0; i < proposal.targets.length; i++)
Line 346:        for (uint256 i = 0; i < proposal.targets.length; i++)
Line 371:        for (uint256 i = 0; i < proposal.targets.length; i++)

After the above for loops were optimized, the gas savings as per Hardhat were as follows:

<ins>Hardhat Gas report:</ins>

 ContractMethodOriginal Avg Gas CostOptimized Avg Gas CostAvg Gas Saved
1NounsDAOLogicV1queue148150147935215
2NounsDAOLogicV1execute170128169958170
3NounsDAOLogicV1cancel999629992141
4NounsDAOLogicV1veto964169633086

Total gas saved in the V1 contract alone is 512.

NounsDAOLogicV2 implements the same set of functions, so the total gas saved in both of these contracts = 512 + 512 = 1024.

3. <ins>Placing emit statements before state-variable update to save gas</ins>

The emit statements are generally placed at the end of a function. But sometimes, placing them before we update an admin variable can save gas. This is already done in the _setVetoer function in both NounsDAOLogicV1 and NounsDAOLogicV2 contracts:

    function _setVetoer(address newVetoer) public {
        require(msg.sender == vetoer, 'NounsDAO::_setVetoer: vetoer only');
        emit NewVetoer(vetoer, newVetoer);  //emitting before updating state variable.
        vetoer = newVetoer;
    }

The same could be done in the following functions as well to save gas.

  1. _setVotingDelay function.
  2. _setVotingPeriod function.
  3. _setProposalThresholdBPS function.
  4. _setPendingAdmin function.
  5. _setVotingDelay function.
  6. _setVotingPeriod function.
  7. _setProposalThresholdBPS function.
  8. _setPendingAdmin function.
  9. _acceptAdmin function in NounsDAOLogicV1 and NounsDAOLogicV2 can do a few more optimizations as given below:
    function _acceptAdmin() external {
        // Check caller is pendingAdmin and pendingAdmin ≠ address(0)
        require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only');
        address cachedpendingAdmin = pendingAdmin;   //cache state variable into local storage
        emit NewAdmin(admin, cachedpendingAdmin);   //first emit
        emit NewPendingAdmin(cachedpendingAdmin, address(0));   //second emit

        // Store admin with value pendingAdmin
        admin = cachedpendingAdmin;    //update state variable from local storage

        // Clear the pending value
        pendingAdmin = address(0);    //update another state variable
    }

After the above rearrangements of emit statements, Hardhat showed a gas saving of 25 gas per function. Since we have 5 functions each in 2 of these contracts, we are able to save 25 * 5 * 2 = 250 gas in total.

4. <ins>Remove redundant safe32 function</ins>

safe32 function is used in NounsDAOLogicV2 contract to make sure that the current block.number is less than 2 ^ 32. But this in an unnecessary concern as at the current rate at which blocks are processed, it will still take more than 1500 years to reach the limit. So we can just safely typecast it into an uint32.

    uint32 blockNumber = safe32(block.number, 'block number exceeds 32 bits');
//becomes
    uint32 blockNumber = uint32(block.number);

The saved gas was observed through the gas savings of _setDynamicQuorumParams method which uses safe32 in one of its internal functions. The average gas savings was 280.

5. <ins>Use custom errors instead of revert strings</ins>

Another major area in which we could save deployment cost would be in converting the revert strings into custom errors. If the function does revert, you can also save on dynamic gas cost. See this example implementation to understand the dynamics of custom errors. It shows that each require string converted into a custom error saves you around 11000 gas.

The codebase uses some custom errors already. The rest is listed below:

  1. NounsDAOProxy.sol (2 instances)
Line 079:        require(msg.sender == admin, 'NounsDAOProxy::_setImplementation: admin only');
Line 080:        require(implementation_ != address(0), 'NounsDAOProxy::_setImplementation: invalid implementation address');     
  1. ERC721Checkpointable.sol (4 instances)
Line 140:        require(signatory != address(0), 'ERC721Checkpointable::delegateBySig: invalid signature');
Line 141:        require(nonce == nonces[signatory]++, 'ERC721Checkpointable::delegateBySig: invalid nonce');
Line 142:        require(block.timestamp <= expiry, 'ERC721Checkpointable::delegateBySig: signature expired');
Line 164:        require(blockNumber < block.number, 'ERC721Checkpointable::getPriorVotes: not yet determined');       
  1. ERC721Enumerable.sol (2 instances)
Line 062:        require(index < ERC721.balanceOf(owner), 'ERC721Enumerable: owner index out of bounds');
Line 077:        require(index < ERC721Enumerable.totalSupply(), 'ERC721Enumerable: global index out of bounds');
  1. NounsDAOLogicV1.sol (39 instances)
Line 122:        require(address(timelock) == address(0), 'NounsDAO::initialize: can only initialize once');
                 require(msg.sender == admin, 'NounsDAO::initialize: admin only');
                 require(timelock_ != address(0), 'NounsDAO::initialize: invalid timelock address');
                 require(nouns_ != address(0), 'NounsDAO::initialize: invalid nouns address');
                 require(
                    votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD,
                    'NounsDAO::initialize: invalid voting period'
                 );
                 require(
                    votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY,
                    'NounsDAO::initialize: invalid voting delay'
                 );
                 require(
                    proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS,
                    'NounsDAO::initialize: invalid proposal threshold'
                 );
                 require(
                    quorumVotesBPS_ >= MIN_QUORUM_VOTES_BPS && quorumVotesBPS_ <= MAX_QUORUM_VOTES_BPS,
                    'NounsDAO::initialize: invalid proposal threshold'
                 );    

Line 187:        require(
                      nouns.getPriorVotes(msg.sender, block.number - 1) > temp.proposalThreshold,
                      'NounsDAO::propose: proposer votes below proposal threshold'
                 );
                 require(
                      targets.length == values.length &&
                          targets.length == signatures.length &&
                          targets.length == calldatas.length,
                      'NounsDAO::propose: proposal function information arity mismatch'
                 );
                 require(targets.length != 0, 'NounsDAO::propose: must provide actions');
                 require(targets.length <= proposalMaxOperations, 'NounsDAO::propose: too many actions');
                 require(
                      proposersLatestProposalState != ProposalState.Active,
                      'NounsDAO::propose: one live proposal per proposer, found an already active proposal'
                 );
                 require(
                      proposersLatestProposalState != ProposalState.Pending,
                      'NounsDAO::propose: one live proposal per proposer, found an already pending proposal'
                 );

Line 275:        require(
                      state(proposalId) == ProposalState.Succeeded,
                      'NounsDAO::queue: proposal can only be queued if it is succeeded'
                  );
Line 301:        require(
                    !timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))),
                    'NounsDAO::queueOrRevertInternal: identical proposal action already queued at eta'
                );

Line 313:        require(
                    state(proposalId) == ProposalState.Queued,
                    'NounsDAO::execute: proposal can only be executed if it is queued'
                );

Line 336:        require(state(proposalId) != ProposalState.Executed, 'NounsDAO::cancel: cannot cancel executed proposal');

Line 339:        require(
                    msg.sender == proposal.proposer ||
                        nouns.getPriorVotes(proposal.proposer, block.number - 1) < proposal.proposalThreshold,
                    'NounsDAO::cancel: proposer above threshold'
                );

Line 364:        require(vetoer != address(0), 'NounsDAO::veto: veto power burned');
                 require(msg.sender == vetoer, 'NounsDAO::veto: only vetoer');
                 require(state(proposalId) != ProposalState.Executed, 'NounsDAO::veto: cannot veto executed proposal');

Line 422:        require(proposalCount >= proposalId, 'NounsDAO::state: invalid proposal id');
Line 485:        require(signatory != address(0), 'NounsDAO::castVoteBySig: invalid signature');
Line 501:        require(state(proposalId) == ProposalState.Active, 'NounsDAO::castVoteInternal: voting is closed');
Line 502:        require(support <= 2, 'NounsDAO::castVoteInternal: invalid vote type');
Line 505:        require(receipt.hasVoted == false, 'NounsDAO::castVoteInternal: voter already voted');
Line 530:        require(msg.sender == admin, 'NounsDAO::_setVotingDelay: admin only');
Line 531:        require(
                    newVotingDelay >= MIN_VOTING_DELAY && newVotingDelay <= MAX_VOTING_DELAY,
                    'NounsDAO::_setVotingDelay: invalid voting delay'
                );
Line 546:        require(msg.sender == admin, 'NounsDAO::_setVotingPeriod: admin only');
Line 547:        require(
                      newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD,
                      'NounsDAO::_setVotingPeriod: invalid voting period'
                  );
Line 563:        require(msg.sender == admin, 'NounsDAO::_setProposalThresholdBPS: admin only');
                 require(
                      newProposalThresholdBPS >= MIN_PROPOSAL_THRESHOLD_BPS &&
                          newProposalThresholdBPS <= MAX_PROPOSAL_THRESHOLD_BPS,
                      'NounsDAO::_setProposalThreshold: invalid proposal threshold'
                  );
Line 581:        require(msg.sender == admin, 'NounsDAO::_setQuorumVotesBPS: admin only');
                 require(
                    newQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS && newQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS,
                    'NounsDAO::_setProposalThreshold: invalid proposal threshold'
                 );
Line 599:        require(msg.sender == admin, 'NounsDAO::_setPendingAdmin: admin only');
Line 617:        require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only');
Line 638:        require(msg.sender == vetoer, 'NounsDAO::_setVetoer: vetoer only');
Line 651:        require(msg.sender == vetoer, 'NounsDAO::_burnVetoPower: vetoer only');       
  1. NounsDAOLogicV2.sol (43 instances)
Line 133:       require(address(timelock) == address(0), 'NounsDAO::initialize: can only initialize once');
                require(msg.sender == admin, 'NounsDAO::initialize: admin only');
                require(timelock_ != address(0), 'NounsDAO::initialize: invalid timelock address');
                require(nouns_ != address(0), 'NounsDAO::initialize: invalid nouns address');
                require(
                    votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD,
                    'NounsDAO::initialize: invalid voting period'
                );
                require(
                    votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY,
                    'NounsDAO::initialize: invalid voting delay'
                );
                require(
                    proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS,
                    'NounsDAO::initialize: invalid proposal threshold bps'
                );
Line 197:       require(
                    nouns.getPriorVotes(msg.sender, block.number - 1) > temp.proposalThreshold,
                    'NounsDAO::propose: proposer votes below proposal threshold'
                );
                require(
                    targets.length == values.length &&
                        targets.length == signatures.length &&
                        targets.length == calldatas.length,
                    'NounsDAO::propose: proposal function information arity mismatch'
                );
                require(targets.length != 0, 'NounsDAO::propose: must provide actions');
                require(targets.length <= proposalMaxOperations, 'NounsDAO::propose: too many actions');
Line 213:       require(
                    proposersLatestProposalState != ProposalState.Active,
                    'NounsDAO::propose: one live proposal per proposer, found an already active proposal'
                );
                require(
                    proposersLatestProposalState != ProposalState.Pending,
                    'NounsDAO::propose: one live proposal per proposer, found an already pending proposal'
                );
Line 286:       require(
                    state(proposalId) == ProposalState.Succeeded,
                    'NounsDAO::queue: proposal can only be queued if it is succeeded'
                );
Line 312:       require(
                    !timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))),
                    'NounsDAO::queueOrRevertInternal: identical proposal action already queued at eta'
                );
Line 324:       require(
                    state(proposalId) == ProposalState.Queued,
                    'NounsDAO::execute: proposal can only be executed if it is queued'
                );
Line 347:       require(state(proposalId) != ProposalState.Executed, 'NounsDAO::cancel: cannot cancel executed proposal');
Line 350:       require(
                    msg.sender == proposal.proposer ||
                        nouns.getPriorVotes(proposal.proposer, block.number - 1) < proposal.proposalThreshold,
                    'NounsDAO::cancel: proposer above threshold'
                );
Line 375:       require(vetoer != address(0), 'NounsDAO::veto: veto power burned');
                require(msg.sender == vetoer, 'NounsDAO::veto: only vetoer');
                require(state(proposalId) != ProposalState.Executed, 'NounsDAO::veto: cannot veto executed proposal');
Line 433:       require(proposalCount >= proposalId, 'NounsDAO::state: invalid proposal id');
Line 577:       require(signatory != address(0), 'NounsDAO::castVoteBySig: invalid signature');
Line 593:       require(state(proposalId) == ProposalState.Active, 'NounsDAO::castVoteInternal: voting is closed');
                require(support <= 2, 'NounsDAO::castVoteInternal: invalid vote type');
Line 597:       require(receipt.hasVoted == false, 'NounsDAO::castVoteInternal: voter already voted');
Line 622:       require(msg.sender == admin, 'NounsDAO::_setVotingDelay: admin only');
                require(
                    newVotingDelay >= MIN_VOTING_DELAY && newVotingDelay <= MAX_VOTING_DELAY,
                    'NounsDAO::_setVotingDelay: invalid voting delay'
                );
Line 638:       require(msg.sender == admin, 'NounsDAO::_setVotingPeriod: admin only');
                require(
                    newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD,
                    'NounsDAO::_setVotingPeriod: invalid voting period'
                );
Line 655:       require(msg.sender == admin, 'NounsDAO::_setProposalThresholdBPS: admin only');
                require(
                    newProposalThresholdBPS >= MIN_PROPOSAL_THRESHOLD_BPS &&
                        newProposalThresholdBPS <= MAX_PROPOSAL_THRESHOLD_BPS,
                    'NounsDAO::_setProposalThreshold: invalid proposal threshold bps'
                );
Line 674:       require(msg.sender == admin, 'NounsDAO::_setMinQuorumVotesBPS: admin only');
Line 677:       require(
                    newMinQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS_LOWER_BOUND &&
                        newMinQuorumVotesBPS <= MIN_QUORUM_VOTES_BPS_UPPER_BOUND,
                    'NounsDAO::_setMinQuorumVotesBPS: invalid min quorum votes bps'
                );
                require(
                    newMinQuorumVotesBPS <= params.maxQuorumVotesBPS,
                    'NounsDAO::_setMinQuorumVotesBPS: min quorum votes bps greater than max'
                );
Line 702:       require(msg.sender == admin, 'NounsDAO::_setMaxQuorumVotesBPS: admin only');
Line 705:       require(
                    newMaxQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS_UPPER_BOUND,
                    'NounsDAO::_setMaxQuorumVotesBPS: invalid max quorum votes bps'
                );
                require(
                    params.minQuorumVotesBPS <= newMaxQuorumVotesBPS,
                    'NounsDAO::_setMaxQuorumVotesBPS: min quorum votes bps greater than max'
                );
Line 727:       require(msg.sender == admin, 'NounsDAO::_setQuorumCoefficient: admin only');
Line 801:       require(msg.sender == admin, 'NounsDAO::_setPendingAdmin: admin only');
Line 819:       require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only');
Line 840:       require(msg.sender == vetoer, 'NounsDAO::_setVetoer: vetoer only');
Line 853:       require(msg.sender == vetoer, 'NounsDAO::_burnVetoPower: vetoer only');     

Total number of instances = 2 + 4 + 2 + 39 + 43 = 90. Approximate deployment cost that can be saved = 90 * 11000 = 990,000.

Conclusions:

The above 4 major changes saved a very significant amount of gas in the overall code base. The exact amounts are tabulated below:

 IssueGas Saved
1propose function in NounsDAOLogicV1 and NounsDAOLogicV2 contract24000
2Optimizing the for loops1024
3Placing emit statements before state-variable update to save gas250
4Remove redundant safe32 function280
5Use custom errors instead of revert strings*
*saved deployment cost

TOTAL GAS SAVED = 24000 + 1024 + 250 + 280 = <ins>25554</ins>.

Total deployment gas saved = <ins>990000</ins> (approximately 1 million).

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter