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: 38/160
Findings: 2
Award: $54.67
🌟 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
Low Risk Issues
| Issue | Instances | |
|---|---|---|
| 1 | Misleading comments | 2 |
| 2 | Unused receive() function will lock Ether in contract | 1 |
| 3 | Misleading error messages | 2 |
Non-critical Issues
| Issue | Instances | |
|---|---|---|
| 1 | Comparison to boolean value | 2 |
Low Risk Issues
Misleading comments
650 // Check caller is pendingAdmin and pendingAdmin ≠address(0)
852 // Check caller is pendingAdmin and pendingAdmin ≠address(0)
Unused receive() function will lock Ether in contract
If the intention is for the Ether to be used, the function should call another function, otherwise it should revert
1030 receive() external payable {}
Misleading error messages.
138 require( 139 quorumVotesBPS_ >= MIN_QUORUM_VOTES_BPS && quorumVotesBPS_ <= MAX_QUORUM_VOTES_BPS, 140 'NounsDAO::initialize: invalid proposal threshold' 141 ); ... 582 require( 583 newQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS && newQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS, 584 'NounsDAO::_setProposalThreshold: invalid proposal threshold' 585 );
Non-critical Issues
Comparisons to boolean constant
code style, clarity A boolean comparison with a constant is not necessary. They can be directly used.
505 require(receipt.hasVoted == false, 'NounsDAO::castVoteInternal: voter already voted');
597 require(receipt.hasVoted == false, 'NounsDAO::castVoteInternal: voter already voted');
🌟 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
19.2277 USDC - $19.23
Gas savings are estimated using the gas report of existing yarn test tests (the sum of all deployment costs and the sum of the costs of calling all methods) and may vary depending on the implementation of the fix. I keep my version of the fix for each finding and can provide them if you need.
Some optimizations (mostly logical) cannot be scored with a exact gas quantity, so no score is displayed for them.
Gas Optimizations
| Issue | Instances | Estimated gas(deployments) | Estimated gas(method call) | |
|---|---|---|---|---|
| 1 | Use Custom Errors instead of Revert/Require Strings to save Gas | 94 | 2 041 729 | - |
| 2 | require()/revert() strings longer than 32 bytes cost extra gas | 90 | 1 291 762 | - |
| 3 | Use modifiers instead of repeating require | 36 | 427 492 | - |
| 4 | Using private rather than public for constants, saves gas | 28 | 453 903 | - |
| 5 | Do not manually check overflow in solidity version >= 0.8.0 | 193 752 | 5 892 | |
| 6 | _burnVetoPower repeats _setVetoer by calling it | 1 | 93 814 | - |
| 7 | Increment optimization | 10 | - | - |
| 7.1 | Prefix increments are cheaper than postfix increments, especially when it's used in for-loops | 10 | 7 692 | - |
| 7.2 | <x> = <x> + 1 even more efficient than pre increment | 10 | 50 842 | - |
| 7.3 | Expression can be unchecked when overflow is not possible | 8 | 83 728 | - |
| 8 | It costs more gas to initialize non-constant/non-immutable variables to zero than to let the default of zero be applied | 24 | 63 727 | 22 188 |
| 9 | Division by two should use bit shifting | 2 | 11 480 | - |
| 10 | Cache storage variable | 22 | 3 504 | 792 |
| 11 | Use built in block.chainid instead of assembler functions | 3 | 2 064 | |
| 12 | Use type(uintXXX).max instead of 2\*\*XXX | 2 | 228 | |
| 13 | Change the order of checking input parameters | 15 | - | - |
Total: 345 instances over 13 issues
Use Custom Errors instead of Revert/Require Strings to save Gas (94 instances)
Deployment Gas Saved: 2041729 Solidity 0.8.4 introduced custom errors. They are more gas efficient than revert strings, when it comes to deploy cost as well as runtime cost when the revert condition is met. Use custom errors instead of revert strings for gas savings.
Custom errors save ~50 gas each time they"re hitby avoiding having to allocate and store the revert string. Additional gas is saved due to the lack of defining string. https://blog.soliditylang.org/2021/04/21/custom-errors/#errors-in-depth
122 require(address(timelock) == address(0), 'NounsDAO::initialize: can only initialize once'); 123 require(msg.sender == admin, 'NounsDAO::initialize: admin only'); 124 require(timelock_ != address(0), 'NounsDAO::initialize: invalid timelock address'); 125 require(nouns_ != address(0), 'NounsDAO::initialize: invalid nouns address'); 126 require( 127 votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD, 128 'NounsDAO::initialize: invalid voting period' 129 ); 130 require( 131 votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY, 132 'NounsDAO::initialize: invalid voting delay' 133 ); 134 require( 135 proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS, 136 'NounsDAO::initialize: invalid proposal threshold' 137 ); 138 require( 139 quorumVotesBPS_ >= MIN_QUORUM_VOTES_BPS && quorumVotesBPS_ <= MAX_QUORUM_VOTES_BPS, 140 'NounsDAO::initialize: invalid proposal threshold' 141 ); ... 187 require( 188 nouns.getPriorVotes(msg.sender, block.number - 1) > temp.proposalThreshold, 189 'NounsDAO::propose: proposer votes below proposal threshold' 190 ); 191 require( 192 targets.length == values.length && 193 targets.length == signatures.length && 194 targets.length == calldatas.length, 195 'NounsDAO::propose: proposal function information arity mismatch' 196 ); 197 require(targets.length != 0, 'NounsDAO::propose: must provide actions'); 198 require(targets.length <= proposalMaxOperations, 'NounsDAO::propose: too many actions'); ... 203 require( 204 proposersLatestProposalState != ProposalState.Active, 205 'NounsDAO::propose: one live proposal per proposer, found an already active proposal' 206 ); 207 require( 208 proposersLatestProposalState != ProposalState.Pending, 209 'NounsDAO::propose: one live proposal per proposer, found an already pending proposal' 210 ); ... 275 require( 276 state(proposalId) == ProposalState.Succeeded, 277 'NounsDAO::queue: proposal can only be queued if it is succeeded' 278 ); ... 301 require( 302 !timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), 303 'NounsDAO::queueOrRevertInternal: identical proposal action already queued at eta' 304 ); ... 313 require( 314 state(proposalId) == ProposalState.Queued, 315 'NounsDAO::execute: proposal can only be executed if it is queued' 316 ); ... 336 require(state(proposalId) != ProposalState.Executed, 'NounsDAO::cancel: cannot cancel executed proposal'); ... 339 require( 340 msg.sender == proposal.proposer || 341 nouns.getPriorVotes(proposal.proposer, block.number - 1) < proposal.proposalThreshold, 342 'NounsDAO::cancel: proposer above threshold' 343 ); ... 364 require(vetoer != address(0), 'NounsDAO::veto: veto power burned'); 365 require(msg.sender == vetoer, 'NounsDAO::veto: only vetoer'); 366 require(state(proposalId) != ProposalState.Executed, 'NounsDAO::veto: cannot veto executed proposal'); ... 422 require(proposalCount >= proposalId, 'NounsDAO::state: invalid proposal id'); ... 485 require(signatory != address(0), 'NounsDAO::castVoteBySig: invalid signature'); ... 501 require(state(proposalId) == ProposalState.Active, 'NounsDAO::castVoteInternal: voting is closed'); 502 require(support <= 2, 'NounsDAO::castVoteInternal: invalid vote type'); ... 505 require(receipt.hasVoted == false, 'NounsDAO::castVoteInternal: voter already voted'); ... 530 require(msg.sender == admin, 'NounsDAO::_setVotingDelay: admin only'); 531 require( 532 newVotingDelay >= MIN_VOTING_DELAY && newVotingDelay <= MAX_VOTING_DELAY, 533 'NounsDAO::_setVotingDelay: invalid voting delay' 534 ); ... 546 require(msg.sender == admin, 'NounsDAO::_setVotingPeriod: admin only'); 547 require( 548 newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD, 549 'NounsDAO::_setVotingPeriod: invalid voting period' 550 ); ... 563 require(msg.sender == admin, 'NounsDAO::_setProposalThresholdBPS: admin only'); 564 require( 565 newProposalThresholdBPS >= MIN_PROPOSAL_THRESHOLD_BPS && 566 newProposalThresholdBPS <= MAX_PROPOSAL_THRESHOLD_BPS, 567 'NounsDAO::_setProposalThreshold: invalid proposal threshold' 568 ); ... 581 require(msg.sender == admin, 'NounsDAO::_setQuorumVotesBPS: admin only'); 582 require( 583 newQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS && newQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS, 584 'NounsDAO::_setProposalThreshold: invalid proposal threshold' 585 ); ... 599 require(msg.sender == admin, 'NounsDAO::_setPendingAdmin: admin only'); ... 617 require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only'); ... 638 require(msg.sender == vetoer, 'NounsDAO::_setVetoer: vetoer only'); ... 651 require(msg.sender == vetoer, 'NounsDAO::_burnVetoPower: vetoer only');
133 require(address(timelock) == address(0), "NounsDAO::initialize: can only initialize once"); 134 require(msg.sender == admin, "NounsDAO::initialize: admin only"); 135 require(timelock_ != address(0), "NounsDAO::initialize: invalid timelock address"); 136 require(nouns_ != address(0), "NounsDAO::initialize: invalid nouns address"); 137 require( 138 votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD, 139 "NounsDAO::initialize: invalid voting period" 140 ); 141 require( 142 votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY, 143 "NounsDAO::initialize: invalid voting delay" 144 ); 145 require( 146 proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS, 147 "NounsDAO::initialize: invalid proposal threshold bps" 148 ); ... 197 require( 198 nouns.getPriorVotes(msg.sender, block.number - 1) > temp.proposalThreshold, 199 "NounsDAO::propose: proposer votes below proposal threshold" 200 ); 201 require( 202 targets.length == values.length && 203 targets.length == signatures.length && 204 targets.length == calldatas.length, 205 "NounsDAO::propose: proposal function information arity mismatch" 206 ); 207 require(targets.length != 0, "NounsDAO::propose: must provide actions"); 208 require(targets.length <= proposalMaxOperations, "NounsDAO::propose: too many actions"); ... 213 require( 214 proposersLatestProposalState != ProposalState.Active, 215 "NounsDAO::propose: one live proposal per proposer, found an already active proposal" 216 ); 217 require( 218 proposersLatestProposalState != ProposalState.Pending, 219 "NounsDAO::propose: one live proposal per proposer, found an already pending proposal" 220 ); ... 286 require( 287 state(proposalId) == ProposalState.Succeeded, 288 "NounsDAO::queue: proposal can only be queued if it is succeeded" 289 ); ... 312 require( 313 !timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), 314 "NounsDAO::queueOrRevertInternal: identical proposal action already queued at eta" 315 ); ... 324 require( 325 state(proposalId) == ProposalState.Queued, 326 "NounsDAO::execute: proposal can only be executed if it is queued" 327 ); ... 347 require(state(proposalId) != ProposalState.Executed, "NounsDAO::cancel: cannot cancel executed proposal"); ... 350 require( 351 msg.sender == proposal.proposer || 352 nouns.getPriorVotes(proposal.proposer, block.number - 1) < proposal.proposalThreshold, 353 "NounsDAO::cancel: proposer above threshold" 354 ); ... 375 require(vetoer != address(0), "NounsDAO::veto: veto power burned"); 376 require(msg.sender == vetoer, "NounsDAO::veto: only vetoer"); 377 require(state(proposalId) != ProposalState.Executed, "NounsDAO::veto: cannot veto executed proposal"); ... 433 require(proposalCount >= proposalId, "NounsDAO::state: invalid proposal id"); ... 577 require(signatory != address(0), "NounsDAO::castVoteBySig: invalid signature"); ... 593 require(state(proposalId) == ProposalState.Active, "NounsDAO::castVoteInternal: voting is closed"); 594 require(support <= 2, 'NounsDAO::castVoteInternal: invalid vote type'); ... 597 require(receipt.hasVoted == false, "NounsDAO::castVoteInternal: voter already voted"); ... 622 require(msg.sender == admin, "NounsDAO::_setVotingDelay: admin only"); 623 require( 624 newVotingDelay >= MIN_VOTING_DELAY && newVotingDelay <= MAX_VOTING_DELAY, 625 "NounsDAO::_setVotingDelay: invalid voting delay" 626 ); ... 638 require(msg.sender == admin, "NounsDAO::_setVotingPeriod: admin only"); 639 require( 640 newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD, 641 "NounsDAO::_setVotingPeriod: invalid voting period" 642 ); ... 655 require(msg.sender == admin, "NounsDAO::_setProposalThresholdBPS: admin only"); 656 require( 657 newProposalThresholdBPS >= MIN_PROPOSAL_THRESHOLD_BPS && 658 newProposalThresholdBPS <= MAX_PROPOSAL_THRESHOLD_BPS, 659 "NounsDAO::_setProposalThreshold: invalid proposal threshold bps" 660 ); ... 674 require(msg.sender == admin, "NounsDAO::_setMinQuorumVotesBPS: admin only"); ... 677 require( 678 newMinQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS_LOWER_BOUND && 679 newMinQuorumVotesBPS <= MIN_QUORUM_VOTES_BPS_UPPER_BOUND, 680 "NounsDAO::_setMinQuorumVotesBPS: invalid min quorum votes bps" 681 ); 682 require( 683 newMinQuorumVotesBPS <= params.maxQuorumVotesBPS, 684 "NounsDAO::_setMinQuorumVotesBPS: min quorum votes bps greater than max" 685 ); ... 702 require(msg.sender == admin, "NounsDAO::_setMaxQuorumVotesBPS: admin only"); ... 705 require( 706 newMaxQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS_UPPER_BOUND, 707 "NounsDAO::_setMaxQuorumVotesBPS: invalid max quorum votes bps" 708 ); 709 require( 710 params.minQuorumVotesBPS <= newMaxQuorumVotesBPS, 711 "NounsDAO::_setMaxQuorumVotesBPS: min quorum votes bps greater than max" 712 ); ... 727 require(msg.sender == admin, "NounsDAO::_setQuorumCoefficient: admin only"); ... 801 require(msg.sender == admin, "NounsDAO::_setPendingAdmin: admin only"); ... 819 require(msg.sender == pendingAdmin && msg.sender != address(0), "NounsDAO::_acceptAdmin: pending admin only"); ... 840 require(msg.sender == vetoer, "NounsDAO::_setVetoer: vetoer only"); ... 853 require(msg.sender == vetoer, "NounsDAO::_burnVetoPower: vetoer only"); ... 1019 require(n <= type(uint32).max, errorMessage);
79 require(msg.sender == admin, "NounsDAOProxy::_setImplementation: admin only"); 80 require(implementation_ != address(0), "NounsDAOProxy::_setImplementation: invalid implementation address");
77 require(index < ERC721Enumerable.totalSupply(), "ERC721Enumerable: global index out of bounds");
140 require(signatory != address(0), "ERC721Checkpointable::delegateBySig: invalid signature"); 141 require(nonce == nonces[signatory]++, "ERC721Checkpointable::delegateBySig: invalid nonce"); 142 require(block.timestamp <= expiry, "ERC721Checkpointable::delegateBySig: signature expired"); ... 164 require(blockNumber < block.number, "ERC721Checkpointable::getPriorVotes: not yet determined"); ... 254 require(n < 2**32, errorMessage); ... 259 require(n < 2**96, errorMessage); ... 269 require(c >= a, errorMessage); ... 278 require(b <= a, errorMessage);
require()/revert() strings longer than 32 bytes cost extra gas (90 instances)
Deployment Gas Saved: 1291762
122 require(address(timelock) == address(0), 'NounsDAO::initialize: can only initialize once'); ... 124 require(timelock_ != address(0), 'NounsDAO::initialize: invalid timelock address'); 125 require(nouns_ != address(0), 'NounsDAO::initialize: invalid nouns address'); 126 require( 127 votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD, 128 'NounsDAO::initialize: invalid voting period' 129 ); 130 require( 131 votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY, 132 'NounsDAO::initialize: invalid voting delay' 133 ); 134 require( 135 proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS, 136 'NounsDAO::initialize: invalid proposal threshold' 137 ); 138 require( 139 quorumVotesBPS_ >= MIN_QUORUM_VOTES_BPS && quorumVotesBPS_ <= MAX_QUORUM_VOTES_BPS, 140 'NounsDAO::initialize: invalid proposal threshold' 141 ); ... 187 require( 188 nouns.getPriorVotes(msg.sender, block.number - 1) > temp.proposalThreshold, 189 'NounsDAO::propose: proposer votes below proposal threshold' 190 ); 191 require( 192 targets.length == values.length && 193 targets.length == signatures.length && 194 targets.length == calldatas.length, 195 'NounsDAO::propose: proposal function information arity mismatch' 196 ); 197 require(targets.length != 0, 'NounsDAO::propose: must provide actions'); 198 require(targets.length <= proposalMaxOperations, 'NounsDAO::propose: too many actions'); ... 203 require( 204 proposersLatestProposalState != ProposalState.Active, 205 'NounsDAO::propose: one live proposal per proposer, found an already active proposal' 206 ); 207 require( 208 proposersLatestProposalState != ProposalState.Pending, 209 'NounsDAO::propose: one live proposal per proposer, found an already pending proposal' 210 ); ... 275 require( 276 state(proposalId) == ProposalState.Succeeded, 277 'NounsDAO::queue: proposal can only be queued if it is succeeded' 278 ); ... 301 require( 302 !timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), 303 'NounsDAO::queueOrRevertInternal: identical proposal action already queued at eta' 304 ); ... 313 require( 314 state(proposalId) == ProposalState.Queued, 315 'NounsDAO::execute: proposal can only be executed if it is queued' 316 ); ... 336 require(state(proposalId) != ProposalState.Executed, 'NounsDAO::cancel: cannot cancel executed proposal'); ... 339 require( 340 msg.sender == proposal.proposer || 341 nouns.getPriorVotes(proposal.proposer, block.number - 1) < proposal.proposalThreshold, 342 'NounsDAO::cancel: proposer above threshold' 343 ); ... 364 require(vetoer != address(0), 'NounsDAO::veto: veto power burned'); ... 366 require(state(proposalId) != ProposalState.Executed, 'NounsDAO::veto: cannot veto executed proposal'); ... 422 require(proposalCount >= proposalId, 'NounsDAO::state: invalid proposal id'); ... 485 require(signatory != address(0), 'NounsDAO::castVoteBySig: invalid signature'); ... 501 require(state(proposalId) == ProposalState.Active, 'NounsDAO::castVoteInternal: voting is closed'); 502 require(support <= 2, 'NounsDAO::castVoteInternal: invalid vote type'); ... 505 require(receipt.hasVoted == false, 'NounsDAO::castVoteInternal: voter already voted'); ... 530 require(msg.sender == admin, 'NounsDAO::_setVotingDelay: admin only'); 531 require( 532 newVotingDelay >= MIN_VOTING_DELAY && newVotingDelay <= MAX_VOTING_DELAY, 533 'NounsDAO::_setVotingDelay: invalid voting delay' 534 ); ... 546 require(msg.sender == admin, 'NounsDAO::_setVotingPeriod: admin only'); 547 require( 548 newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD, 549 'NounsDAO::_setVotingPeriod: invalid voting period' 550 ); ... 563 require(msg.sender == admin, 'NounsDAO::_setProposalThresholdBPS: admin only'); 564 require( 565 newProposalThresholdBPS >= MIN_PROPOSAL_THRESHOLD_BPS && 566 newProposalThresholdBPS <= MAX_PROPOSAL_THRESHOLD_BPS, 567 'NounsDAO::_setProposalThreshold: invalid proposal threshold' 568 ); ... 581 require(msg.sender == admin, 'NounsDAO::_setQuorumVotesBPS: admin only'); 582 require( 583 newQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS && newQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS, 584 'NounsDAO::_setProposalThreshold: invalid proposal threshold' 585 ); ... 599 require(msg.sender == admin, 'NounsDAO::_setPendingAdmin: admin only'); ... 617 require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only'); ... 638 require(msg.sender == vetoer, 'NounsDAO::_setVetoer: vetoer only'); ... 651 require(msg.sender == vetoer, 'NounsDAO::_burnVetoPower: vetoer only');
133 require(address(timelock) == address(0), "NounsDAO::initialize: can only initialize once"); 135 require(timelock_ != address(0), "NounsDAO::initialize: invalid timelock address"); 136 require(nouns_ != address(0), "NounsDAO::initialize: invalid nouns address"); 137 require( 138 votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD, 139 "NounsDAO::initialize: invalid voting period" 140 ); 141 require( 142 votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY, 143 "NounsDAO::initialize: invalid voting delay" 144 ); 145 require( 146 proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS, 147 "NounsDAO::initialize: invalid proposal threshold bps" 148 ); ... 197 require( 198 nouns.getPriorVotes(msg.sender, block.number - 1) > temp.proposalThreshold, 199 "NounsDAO::propose: proposer votes below proposal threshold" 200 ); 201 require( 202 targets.length == values.length && 203 targets.length == signatures.length && 204 targets.length == calldatas.length, 205 "NounsDAO::propose: proposal function information arity mismatch" 206 ); 207 require(targets.length != 0, "NounsDAO::propose: must provide actions"); 208 require(targets.length <= proposalMaxOperations, "NounsDAO::propose: too many actions"); ... 213 require( 214 proposersLatestProposalState != ProposalState.Active, 215 "NounsDAO::propose: one live proposal per proposer, found an already active proposal" 216 ); 217 require( 218 proposersLatestProposalState != ProposalState.Pending, 219 "NounsDAO::propose: one live proposal per proposer, found an already pending proposal" 220 ); ... 286 require( 287 state(proposalId) == ProposalState.Succeeded, 288 "NounsDAO::queue: proposal can only be queued if it is succeeded" 289 ); ... 312 require( 313 !timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), 314 "NounsDAO::queueOrRevertInternal: identical proposal action already queued at eta" 315 ); ... 324 require( 325 state(proposalId) == ProposalState.Queued, 326 "NounsDAO::execute: proposal can only be executed if it is queued" 327 ); ... 347 require(state(proposalId) != ProposalState.Executed, "NounsDAO::cancel: cannot cancel executed proposal"); ... 350 require( 351 msg.sender == proposal.proposer || 352 nouns.getPriorVotes(proposal.proposer, block.number - 1) < proposal.proposalThreshold, 353 "NounsDAO::cancel: proposer above threshold" 354 ); ... 375 require(vetoer != address(0), "NounsDAO::veto: veto power burned"); 377 require(state(proposalId) != ProposalState.Executed, "NounsDAO::veto: cannot veto executed proposal"); ... 433 require(proposalCount >= proposalId, "NounsDAO::state: invalid proposal id"); ... 577 require(signatory != address(0), "NounsDAO::castVoteBySig: invalid signature"); ... 593 require(state(proposalId) == ProposalState.Active, "NounsDAO::castVoteInternal: voting is closed"); 594 require(support <= 2, 'NounsDAO::castVoteInternal: invalid vote type'); ... 597 require(receipt.hasVoted == false, "NounsDAO::castVoteInternal: voter already voted"); ... 622 require(msg.sender == admin, "NounsDAO::_setVotingDelay: admin only"); 623 require( 624 newVotingDelay >= MIN_VOTING_DELAY && newVotingDelay <= MAX_VOTING_DELAY, 625 "NounsDAO::_setVotingDelay: invalid voting delay" 626 ); ... 638 require(msg.sender == admin, "NounsDAO::_setVotingPeriod: admin only"); 639 require( 640 newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD, 641 "NounsDAO::_setVotingPeriod: invalid voting period" 642 ); ... 655 require(msg.sender == admin, "NounsDAO::_setProposalThresholdBPS: admin only"); 656 require( 657 newProposalThresholdBPS >= MIN_PROPOSAL_THRESHOLD_BPS && 658 newProposalThresholdBPS <= MAX_PROPOSAL_THRESHOLD_BPS, 659 "NounsDAO::_setProposalThreshold: invalid proposal threshold bps" 660 ); ... 674 require(msg.sender == admin, "NounsDAO::_setMinQuorumVotesBPS: admin only"); ... 677 require( 678 newMinQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS_LOWER_BOUND && 679 newMinQuorumVotesBPS <= MIN_QUORUM_VOTES_BPS_UPPER_BOUND, 680 "NounsDAO::_setMinQuorumVotesBPS: invalid min quorum votes bps" 681 ); 682 require( 683 newMinQuorumVotesBPS <= params.maxQuorumVotesBPS, 684 "NounsDAO::_setMinQuorumVotesBPS: min quorum votes bps greater than max" 685 ); ... 702 require(msg.sender == admin, "NounsDAO::_setMaxQuorumVotesBPS: admin only"); ... 705 require( 706 newMaxQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS_UPPER_BOUND, 707 "NounsDAO::_setMaxQuorumVotesBPS: invalid max quorum votes bps" 708 ); 709 require( 710 params.minQuorumVotesBPS <= newMaxQuorumVotesBPS, 711 "NounsDAO::_setMaxQuorumVotesBPS: min quorum votes bps greater than max" 712 ); ... 727 require(msg.sender == admin, "NounsDAO::_setQuorumCoefficient: admin only"); ... 801 require(msg.sender == admin, "NounsDAO::_setPendingAdmin: admin only"); ... 819 require(msg.sender == pendingAdmin && msg.sender != address(0), "NounsDAO::_acceptAdmin: pending admin only"); ... 840 require(msg.sender == vetoer, "NounsDAO::_setVetoer: vetoer only"); ... 853 require(msg.sender == vetoer, "NounsDAO::_burnVetoPower: vetoer only"); ... 1019 require(n <= type(uint32).max, errorMessage);
79 require(msg.sender == admin, "NounsDAOProxy::_setImplementation: admin only"); 80 require(implementation_ != address(0), "NounsDAOProxy::_setImplementation: invalid implementation address");
77 require(index < ERC721Enumerable.totalSupply(), "ERC721Enumerable: global index out of bounds");
140 require(signatory != address(0), "ERC721Checkpointable::delegateBySig: invalid signature"); 141 require(nonce == nonces[signatory]++, "ERC721Checkpointable::delegateBySig: invalid nonce"); 142 require(block.timestamp <= expiry, "ERC721Checkpointable::delegateBySig: signature expired"); ... 164 require(blockNumber < block.number, "ERC721Checkpointable::getPriorVotes: not yet determined"); ... 254 require(n < 2**32, errorMessage); ... 259 require(n < 2**96, errorMessage); ... 269 require(c >= a, errorMessage); ... 278 require(b <= a, errorMessage);
Use modifiers instead of repeating require (36 instances)
Deployment Gas Saved: 427492
IsAdmin:
Can be included in NounsDAOProxyStorage from contracts/governance/NounsDAOInterfaces.sol
Deployment Gas Saved: 182804
123 require(msg.sender == admin, "NounsDAO::initialize: admin only"); ... 530 require(msg.sender == admin, "NounsDAO::_setVotingDelay: admin only"); ... 546 require(msg.sender == admin, "NounsDAO::_setVotingPeriod: admin only"); ... 563 require(msg.sender == admin, 'NounsDAO::_setProposalThresholdBPS: admin only'); ... 581 require(msg.sender == admin, 'NounsDAO::_setQuorumVotesBPS: admin only'); ... 599 require(msg.sender == admin, "NounsDAO::_setPendingAdmin: admin only");
134 require(msg.sender == admin, 'NounsDAO::initialize: admin only'); ... 622 require(msg.sender == admin, 'NounsDAO::_setVotingDelay: admin only'); ... 638 require(msg.sender == admin, 'NounsDAO::_setVotingPeriod: admin only'); ... 655 require(msg.sender == admin, 'NounsDAO::_setProposalThresholdBPS: admin only'); ... 674 require(msg.sender == admin, 'NounsDAO::_setMinQuorumVotesBPS: admin only'); ... 702 require(msg.sender == admin, 'NounsDAO::_setMaxQuorumVotesBPS: admin only'); ... 727 require(msg.sender == admin, 'NounsDAO::_setQuorumCoefficient: admin only'); ... 801 require(msg.sender == admin, 'NounsDAO::_setPendingAdmin: admin only');
79 require(msg.sender == admin, 'NounsDAOProxy::_setImplementation: admin only');
ValidateVotingPeriod:
126 require( 127 votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD, 128 'NounsDAO::initialize: invalid voting period' 129 ); ... 547 require( 548 newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD, 549 'NounsDAO::_setVotingPeriod: invalid voting period' 550 );
137 require( 138 votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD, 139 'NounsDAO::initialize: invalid voting period' 140 ); ... 639 require( 640 newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD, 641 'NounsDAO::_setVotingPeriod: invalid voting period' 642 );
ValidateVotingDelay:
130 require( 131 votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY, 132 'NounsDAO::initialize: invalid voting delay' 133 ); ... 531 require( 532 newVotingDelay >= MIN_VOTING_DELAY && newVotingDelay <= MAX_VOTING_DELAY, 533 'NounsDAO::_setVotingDelay: invalid voting delay' 534 );
141 require( 142 votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY, 143 'NounsDAO::initialize: invalid voting delay' 144 ); ... 623 require( 624 newVotingDelay >= MIN_VOTING_DELAY && newVotingDelay <= MAX_VOTING_DELAY, 625 'NounsDAO::_setVotingDelay: invalid voting delay' 626 );
ValidateProposalThreshold:
134 require( 135 proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS, 136 'NounsDAO::initialize: invalid proposal threshold' 137 ); ... 564 require( 565 newProposalThresholdBPS >= MIN_PROPOSAL_THRESHOLD_BPS && 566 newProposalThresholdBPS <= MAX_PROPOSAL_THRESHOLD_BPS, 567 'NounsDAO::_setProposalThreshold: invalid proposal threshold' 568 );
145 require( 146 proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS, 147 'NounsDAO::initialize: invalid proposal threshold bps' 148 ); ... 656 require( 657 newProposalThresholdBPS >= MIN_PROPOSAL_THRESHOLD_BPS && 658 newProposalThresholdBPS <= MAX_PROPOSAL_THRESHOLD_BPS, 659 'NounsDAO::_setProposalThreshold: invalid proposal threshold bps' 660 );
ValidateQuorumVotes:
138 require( 139 quorumVotesBPS_ >= MIN_QUORUM_VOTES_BPS && quorumVotesBPS_ <= MAX_QUORUM_VOTES_BPS, 140 'NounsDAO::initialize: invalid proposal threshold' 141 ); ... 582 require( 583 newQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS && newQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS, 584 'NounsDAO::_setProposalThreshold: invalid proposal threshold' 585 );
IsVetoer:
638 require(msg.sender == vetoer, 'NounsDAO::_setVetoer: vetoer only'); ... 651 require(msg.sender == vetoer, 'NounsDAO::_burnVetoPower: vetoer only');
376 require(msg.sender == vetoer, 'NounsDAO::veto: only vetoer'); ... 840 require(msg.sender == vetoer, 'NounsDAO::_setVetoer: vetoer only'); ... 853 require(msg.sender == vetoer, 'NounsDAO::_burnVetoPower: vetoer only');
Optional, not included in the gas estimate:
modifier ZeroAddress(address _address) { require(_address != address(0), 'NounsDAO ZeroAddress'); _; }
Using private rather than public for constants, saves gas (28 instances)
If needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table
Deployment Gas Saved: 453903
66 /// @notice The name of this contract 67 string public constant name = 'Nouns DAO'; 68 69 /// @notice The minimum setable proposal threshold 70 uint256 public constant MIN_PROPOSAL_THRESHOLD_BPS = 1; // 1 basis point or 0.01% 71 72 /// @notice The maximum setable proposal threshold 73 uint256 public constant MAX_PROPOSAL_THRESHOLD_BPS = 1_000; // 1,000 basis points or 10% 74 75 /// @notice The minimum setable voting period 76 uint256 public constant MIN_VOTING_PERIOD = 5_760; // About 24 hours 77 78 /// @notice The max setable voting period 79 uint256 public constant MAX_VOTING_PERIOD = 80_640; // About 2 weeks 80 81 /// @notice The min setable voting delay 82 uint256 public constant MIN_VOTING_DELAY = 1; 83 84 /// @notice The max setable voting delay 85 uint256 public constant MAX_VOTING_DELAY = 40_320; // About 1 week 86 87 /// @notice The minimum setable quorum votes basis points 88 uint256 public constant MIN_QUORUM_VOTES_BPS = 200; // 200 basis points or 2% 89 90 /// @notice The maximum setable quorum votes basis points 91 uint256 public constant MAX_QUORUM_VOTES_BPS = 2_000; // 2,000 basis points or 20% 92 93 /// @notice The maximum number of actions that can be included in a proposal 94 uint256 public constant proposalMaxOperations = 10; // 10 actions 95 96 /// @notice The EIP-712 typehash for the contract's domain 97 bytes32 public constant DOMAIN_TYPEHASH = 98 keccak256('EIP712Domain(string name,uint256 chainId,address verifyingContract)'); 99 100 /// @notice The EIP-712 typehash for the ballot struct used by the contract 101 bytes32 public constant BALLOT_TYPEHASH = keccak256('Ballot(uint256 proposalId,uint8 support)');
58 /// @notice The name of this contract 59 string public constant name = 'Nouns DAO'; 60 61 /// @notice The minimum setable proposal threshold 62 uint256 public constant MIN_PROPOSAL_THRESHOLD_BPS = 1; // 1 basis point or 0.01% 63 64 /// @notice The maximum setable proposal threshold 65 uint256 public constant MAX_PROPOSAL_THRESHOLD_BPS = 1_000; // 1,000 basis points or 10% 66 67 /// @notice The minimum setable voting period 68 uint256 public constant MIN_VOTING_PERIOD = 5_760; // About 24 hours 69 70 /// @notice The max setable voting period 71 uint256 public constant MAX_VOTING_PERIOD = 80_640; // About 2 weeks 72 73 /// @notice The min setable voting delay 74 uint256 public constant MIN_VOTING_DELAY = 1; 75 76 /// @notice The max setable voting delay 77 uint256 public constant MAX_VOTING_DELAY = 40_320; // About 1 week 78 79 /// @notice The lower bound of minimum quorum votes basis points 80 uint256 public constant MIN_QUORUM_VOTES_BPS_LOWER_BOUND = 200; // 200 basis points or 2% 81 82 /// @notice The upper bound of minimum quorum votes basis points 83 uint256 public constant MIN_QUORUM_VOTES_BPS_UPPER_BOUND = 2_000; // 2,000 basis points or 20% 84 85 /// @notice The upper bound of maximum quorum votes basis points 86 uint256 public constant MAX_QUORUM_VOTES_BPS_UPPER_BOUND = 6_000; // 4,000 basis points or 60% 87 88 /// @notice The maximum setable quorum votes basis points 89 uint256 public constant MAX_QUORUM_VOTES_BPS = 2_000; // 2,000 basis points or 20% 90 91 /// @notice The maximum number of actions that can be included in a proposal 92 uint256 public constant proposalMaxOperations = 10; // 10 actions 93 94 /// @notice The maximum priority fee used to cap gas refunds in `castRefundableVote` 95 uint256 public constant MAX_REFUND_PRIORITY_FEE = 2 gwei; 96 97 /// @notice The vote refund gas overhead, including 7K for ETH transfer and 29K for general transaction overhead 98 uint256 public constant REFUND_BASE_GAS = 36000; 99 100 /// @notice The EIP-712 typehash for the contract's domain 101 bytes32 public constant DOMAIN_TYPEHASH = 102 keccak256('EIP712Domain(string name,uint256 chainId,address verifyingContract)'); 103 104 /// @notice The EIP-712 typehash for the ballot struct used by the contract 105 bytes32 public constant BALLOT_TYPEHASH = keccak256('Ballot(uint256 proposalId,uint8 support)');
Do not manually check overflow in solidity >= 0.8.0
Deployment Gas Saved: 193752 Method Call Gas Saved: 5892
Solidity version 0.8+ already implements overflow and underflow checks by default. Using the SafeMath library from OpenZeppelin (which is more gas expensive than the 0.8+ overflow checks) is therefore redundant.
1018 function safe32(uint256 n, string memory errorMessage) internal pure returns (uint32) { 1019 require(n <= type(uint32).max, errorMessage); 1020 return uint32(n); 1021 } 1022 1023 function safe16(uint256 n) internal pure returns (uint16) { 1024 if (n > type(uint16).max) { 1025 revert UnsafeUint16Cast(); 1026 } 1027 return uint16(n); 1028 }
253 function safe32(uint256 n, string memory errorMessage) internal pure returns (uint32) { 254 require(n < 2**32, errorMessage); 255 return uint32(n); 256 } 257 258 function safe96(uint256 n, string memory errorMessage) internal pure returns (uint96) { 259 require(n < 2**96, errorMessage); 260 return uint96(n); 261 } 262 263 function add96( 264 uint96 a, 265 uint96 b, 266 string memory errorMessage 267 ) internal pure returns (uint96) { 268 uint96 c = a + b; 269 require(c >= a, errorMessage); 270 return c; 271 } 272 273 function sub96( 274 uint96 a, 275 uint96 b, 276 string memory errorMessage 277 ) internal pure returns (uint96) { 278 require(b <= a, errorMessage); 279 return a - b; 280 }
_burnVetoPower repeats _setVetoer by calling it (1 instances)
Deployment Gas Saved: 93814
_burnVetoPower checks for sender == vetoer, then calls _setVetoer and checks sender == vetoer again.
function _setVetoer(address newVetoer) public { require(msg.sender == vetoer, 'NounsDAO::_setVetoer: vetoer only'); emit NewVetoer(vetoer, newVetoer); vetoer = newVetoer; } ... function _burnVetoPower() public { // Check caller is pendingAdmin and pendingAdmin ≠address(0) require(msg.sender == vetoer, 'NounsDAO::_burnVetoPower: vetoer only'); _setVetoer(address(0)); }
fix:
requirefunction _burnVetoPower() public { _setVetoer(address(0)); }
_setVetoer - expensivefunction _burnVetoPower() public { require(msg.sender == vetoer, 'NounsDAO::_burnVetoPower: vetoer only'); emit NewVetoer(vetoer, address(0)); // Or better to use new event like VetoPowerBurned(); delete vetoer; }
Increment optimization (10 instances)
Prefix increments are cheaper than postfix increments, especially when it's used in for-loops (10 instances).
Deployment Gas Saved: 7692
<x> = <x> + 1 even more efficient than pre increment.(10 instances) Gas saved:
Deployment Gas Saved: 50842
216 proposalCount++; ... 281 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 319 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 346 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 371 for (uint256 i = 0; i < proposal.targets.length; i++) {
226 proposalCount++; ... 292 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 330 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 357 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 382 for (uint256 i = 0; i < proposal.targets.length; i++) {
Expression can be unchecked when overflow is not possible.(8 instances) Gas saved:
Deployment Gas Saved: 83728
281 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 319 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 346 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 371 for (uint256 i = 0; i < proposal.targets.length; i++) {
292 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 330 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 357 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 382 for (uint256 i = 0; i < proposal.targets.length; i++) {
It costs more gas to initialize non-constant/non-immutable variables to zero than to let the default of zero be applied (24 instances)
Deployment Gas Saved: 63727 Method Call Gas Saved: 22188
223 newProposal.eta = 0; ... 230 newProposal.forVotes = 0; 231 newProposal.againstVotes = 0; 232 newProposal.abstainVotes = 0; 233 newProposal.canceled = false; 234 newProposal.executed = false; 235 newProposal.vetoed = false; ... 281 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 319 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 346 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 371 for (uint256 i = 0; i < proposal.targets.length; i++) {
231 newProposal.eta = 0; ... 238 newProposal.forVotes = 0; 239 newProposal.againstVotes = 0; 240 newProposal.abstainVotes = 0; 241 newProposal.canceled = false; 242 newProposal.executed = false; 243 newProposal.vetoed = false; ... 292 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 330 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 357 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 382 for (uint256 i = 0; i < proposal.targets.length; i++) { ... 948 uint256 lower = 0;
181 uint32 lower = 0;
Division by two should use bit shifting
Deployment Gas Saved: 11480
<x> / 2 is the same as <x> >> 1. The DIV opcode costs 5 gas, whereas SHR only costs 3 gas
184 uint32 center = upper - (upper - lower) / 2; // ceil, avoiding overflow
951 uint256 center = upper - (upper - lower) / 2;
Cache storage variable (22 instances)
Deployment Gas Saved: 3504 Method Call Gas Saved: 792
216 proposalCount++; 217 Proposal storage newProposal = proposals[proposalCount]; ... 219 newProposal.id = proposalCount; ... 237 latestProposalIds[newProposal.proposer] = newProposal.id; // Can use proposalCount ... 240 emit ProposalCreated( 241 newProposal.id, ... 253 emit ProposalCreatedWithRequirements( 254 newProposal.id, ... 267 return newProposal.id;
fix:
uint256 l_proposalCount = ++proposalCount; // @note First increment proposalCount then store it in a local variable. Proposal storage newProposal = proposals[l_proposalCount]; newProposal.id = l_proposalCount; latestProposalIds[newProposal.proposer] = l_proposalCount; emit ProposalCreated( l_proposalCount, emit ProposalCreatedWithRequirements( l_proposalCount, return l_proposalCount;
226 proposalCount++; 227 Proposal storage newProposal = _proposals[proposalCount]; 228 newProposal.id = proposalCount; ... 247 latestProposalIds[newProposal.proposer] = newProposal.id; ... 251 newProposal.id, ... 265 newProposal.id, ... 278 return newProposal.id;
fix:
uint256 l_proposalCount = ++proposalCount; // @note First increment proposalCount then store it in a local variable. Proposal storage newProposal = proposals[l_proposalCount]; newProposal.id = l_proposalCount; latestProposalIds[newProposal.proposer] = l_proposalCount; emit ProposalCreated( l_proposalCount, emit ProposalCreatedWithRequirements( l_proposalCount, return l_proposalCount;
Optional, caching the length of actions in a loop increases gas cost, but should save gas in the long run if a proposal contains more than two actions.
8 instances in 2 files for (uint256 i = 0; i < proposal.targets.length; i++) {
fix:
uint256 length = proposal.targets.length; for (uint256 i = 0; i < length; i++) {
Use built in block.chainid instead of assembler functions (3 instances)
Deployment Gas Saved: 2064
282 function getChainId() internal view returns (uint256) { 283 uint256 chainId; 284 assembly { 285 chainId := chainid() 286 } 287 return chainId; 288 }
676 function getChainIdInternal() internal view returns (uint256) { 677 uint256 chainId; 678 assembly { 679 chainId := chainid() 680 } 681 return chainId; 682 }
1010 function getChainIdInternal() internal view returns (uint256) { 1011 uint256 chainId; 1012 assembly { 1013 chainId := chainid() 1014 } 1015 return chainId; 1016 }
Use type(uintxxx).max instead of 2\*\*xxx
Deployment Gas Saved: 228
254 require(n < 2**32, errorMessage); ... 259 require(n < 2**96, errorMessage);
Change the order of checking input parameters
It's cheaper to first compare with constants, then local variables, then storage. Otherwise, if the transaction first checks the storage variable and then reverts, the user will not refund the gas spent reading from storage.
Extcalls are usually very expensive, so this should be the last possible check.
It's also better to put RBAC checks at the beginning. Because if a non-admin user calls an admin-only function, it doesn't matter if the input parameters are valid
Read the notes in the code for fixes.
122 require(address(timelock) == address(0), 'NounsDAO::initialize: can only initialize once'); 123 require(msg.sender == admin, 'NounsDAO::initialize: admin only'); // @note put at the beginning ↑ 124 require(timelock_ != address(0), 'NounsDAO::initialize: invalid timelock address'); 125 require(nouns_ != address(0), 'NounsDAO::initialize: invalid nouns address'); 126 require( 127 votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD, 128 'NounsDAO::initialize: invalid voting period' 129 ); // @note put before checks with address(0) ↑ 130 require( 131 votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY, 132 'NounsDAO::initialize: invalid voting delay' 133 ); // @note put before checks with address(0) ↑ 134 require( 135 proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS, 136 'NounsDAO::initialize: invalid proposal threshold' 137 ); // @note put before checks with address(0) ↑ 138 require( 139 quorumVotesBPS_ >= MIN_QUORUM_VOTES_BPS && quorumVotesBPS_ <= MAX_QUORUM_VOTES_BPS, 140 'NounsDAO::initialize: invalid proposal threshold' 141 ); // @note put before checks with address(0) ↑ ... 187 require( 188 nouns.getPriorVotes(msg.sender, block.number - 1) > temp.proposalThreshold, 189 'NounsDAO::propose: proposer votes below proposal threshold' 190 ); // @note extcall but checks for an eligible user, choose whatever is best for you 191 require( 192 targets.length == values.length && 193 targets.length == signatures.length && 194 targets.length == calldatas.length, 195 'NounsDAO::propose: proposal function information arity mismatch' 196 );// @note put under check for 0 and check for a constant ↓ 197 require(targets.length != 0, 'NounsDAO::propose: must provide actions'); 198 require(targets.length <= proposalMaxOperations, 'NounsDAO::propose: too many actions');
133 require(address(timelock) == address(0), 'NounsDAO::initialize: can only initialize once'); 134 require(msg.sender == admin, 'NounsDAO::initialize: admin only'); // @note put at the beginning ↑ 135 require(timelock_ != address(0), 'NounsDAO::initialize: invalid timelock address'); 136 require(nouns_ != address(0), 'NounsDAO::initialize: invalid nouns address'); 137 require( 138 votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD, 139 'NounsDAO::initialize: invalid voting period' 140 ); // @note put before checks with address(0) ↑ 141 require( 142 votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY, 143 'NounsDAO::initialize: invalid voting delay' 144 ); // @note put before checks with address(0) ↑ 145 require( 146 proposalThresholdBPS_ >= MIN_PROPOSAL_THRESHOLD_BPS && proposalThresholdBPS_ <= MAX_PROPOSAL_THRESHOLD_BPS, 147 'NounsDAO::initialize: invalid proposal threshold bps' 148 ); // @note put before checks with address(0) ↑ ... 197 require( 198 nouns.getPriorVotes(msg.sender, block.number - 1) > temp.proposalThreshold, 199 'NounsDAO::propose: proposer votes below proposal threshold' 200 ); // @note extcall but checks for an eligible user, choose whatever is best for you 201 require( 202 targets.length == values.length && 203 targets.length == signatures.length && 204 targets.length == calldatas.length, 205 'NounsDAO::propose: proposal function information arity mismatch' 206 ); // @note put under check for 0 and check for a constant ↓ 207 require(targets.length != 0, 'NounsDAO::propose: must provide actions'); 208 require(targets.length <= proposalMaxOperations, 'NounsDAO::propose: too many actions'); ... 675 DynamicQuorumParams memory params = getDynamicQuorumParamsAt(block.number); // @note put between require ↓ because if the first one fails, then this line is useless 676 677 require( 678 newMinQuorumVotesBPS >= MIN_QUORUM_VOTES_BPS_LOWER_BOUND && 679 newMinQuorumVotesBPS <= MIN_QUORUM_VOTES_BPS_UPPER_BOUND, 680 'NounsDAO::_setMinQuorumVotesBPS: invalid min quorum votes bps' 681 ); 682 require( 683 newMinQuorumVotesBPS <= params.maxQuorumVotesBPS, 684 'NounsDAO::_setMinQuorumVotesBPS: min quorum votes bps greater than max' 685 ); ... 703 DynamicQuorumParams memory params = getDynamicQuorumParamsAt(block.number); // @note put between require ↓ because if the first one fails, then this line is useless 704 705 require( 706 newMaxQuorumVotesBPS <= MAX_QUORUM_VOTES_BPS_UPPER_BOUND, 707 'NounsDAO::_setMaxQuorumVotesBPS: invalid max quorum votes bps' 708 ); 709 require( 710 params.minQuorumVotesBPS <= newMaxQuorumVotesBPS, 711 'NounsDAO::_setMaxQuorumVotesBPS: min quorum votes bps greater than max' 712 );