Nouns DAO contest - oyc_109'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: 36/160

Findings: 2

Award: $54.91

🌟 Selected for report: 0

πŸš€ Solo Findings: 0

[L-01] Unspecific Compiler Version Pragma

Avoid floating pragmas for non-library contracts.

While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.

A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.

It is recommended to pin to a concrete compiler version.

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::35 => pragma solidity ^0.8.6; 2022-08-nounsdao/contracts/base/ERC721Enumerable.sol::28 => pragma solidity ^0.8.0; 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::33 => pragma solidity ^0.8.6; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::61 => pragma solidity ^0.8.6; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::53 => pragma solidity ^0.8.6; 2022-08-nounsdao/contracts/governance/NounsDAOProxy.sol::36 => pragma solidity ^0.8.6;

[L-02] Use of Block.timestamp

Block timestamps have historically been used for a variety of applications, such as entropy for random numbers (see the Entropy Illusion for further details), locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::142 => require(block.timestamp <= expiry, 'ERC721Checkpointable::delegateBySig: signature expired'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::280 => uint256 eta = block.timestamp + timelock.delay(); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::438 => } else if (block.timestamp >= proposal.eta + timelock.GRACE_PERIOD()) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::291 => uint256 eta = block.timestamp + timelock.delay(); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::449 => } else if (block.timestamp >= proposal.eta + timelock.GRACE_PERIOD()) {

[L-03] Unused receive()/fallback() function

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert

2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::1030 => receive() external payable {}

[L-04] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). Unless there is a compelling reason, abi.encode should be preferred. If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead.

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::138 => bytes32 digest = keccak256(abi.encodePacked('\x19\x01', domainSeparator, structHash)); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::483 => bytes32 digest = keccak256(abi.encodePacked('\x19\x01', domainSeparator, structHash)); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::575 => bytes32 digest = keccak256(abi.encodePacked('\x19\x01', domainSeparator, structHash));

[L-05] lack of zero address checks

https://github.com/code-423n4/2022-08-nounsdao/blob/452695d4764ba9d5e1d3eef0d5ecca3d004f215a/contracts/governance/NounsDAOLogicV1.sol#L642 https://github.com/code-423n4/2022-08-nounsdao/blob/452695d4764ba9d5e1d3eef0d5ecca3d004f215a/contracts/governance/NounsDAOProxy.sol#L71

[N-01] Use a more recent version of solidity

Use a solidity version of at least 0.8.4 to get bytes.concat() instead of abi.encodePacked(<bytes>,<bytes>) Use a solidity version of at least 0.8.12 to get string.concat() instead of abi.encodePacked(<str>,<str>) Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::35 => pragma solidity ^0.8.6; 2022-08-nounsdao/contracts/base/ERC721Enumerable.sol::28 => pragma solidity ^0.8.0; 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::33 => pragma solidity ^0.8.6; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::61 => pragma solidity ^0.8.6; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::53 => pragma solidity ^0.8.6; 2022-08-nounsdao/contracts/governance/NounsDAOProxy.sol::36 => pragma solidity ^0.8.6;

[N-02] Large multiples of ten should use scientific notation

Use (e.g. 1e6) rather than decimal literals (e.g. 1000000), for better code readability

2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::673 => return (number * bps) / 10000; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::908 => uint256 againstVotesBPS = (10000 * againstVotes) / totalSupply; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::1007 => return (number * bps) / 10000;

[N-03] Event is missing indexed fields

Each event should use three indexed fields if there are three or more fields

2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::73 => event ProposalCanceled(uint256 id); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::76 => event ProposalQueued(uint256 id, uint256 eta); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::79 => event ProposalExecuted(uint256 id); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::82 => event ProposalVetoed(uint256 id); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::85 => event VotingDelaySet(uint256 oldVotingDelay, uint256 newVotingDelay); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::88 => event VotingPeriodSet(uint256 oldVotingPeriod, uint256 newVotingPeriod); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::91 => event NewImplementation(address oldImplementation, address newImplementation); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::94 => event ProposalThresholdBPSSet(uint256 oldProposalThresholdBPS, uint256 newProposalThresholdBPS); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::97 => event QuorumVotesBPSSet(uint256 oldQuorumVotesBPS, uint256 newQuorumVotesBPS); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::100 => event NewPendingAdmin(address oldPendingAdmin, address newPendingAdmin); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::103 => event NewAdmin(address oldAdmin, address newAdmin); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::106 => event NewVetoer(address oldVetoer, address newVetoer); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::111 => event MinQuorumVotesBPSSet(uint16 oldMinQuorumVotesBPS, uint16 newMinQuorumVotesBPS); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::114 => event MaxQuorumVotesBPSSet(uint16 oldMaxQuorumVotesBPS, uint16 newMaxQuorumVotesBPS); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::117 => event QuorumCoefficientSet(uint32 oldQuorumCoefficient, uint32 newQuorumCoefficient); 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::123 => event Withdraw(uint256 amount, bool sent);

[N-04] Public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public.

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::163 => function getPriorVotes(address account, uint256 blockNumber) public view returns (uint96) { 2022-08-nounsdao/contracts/base/ERC721Enumerable.sol::61 => function tokenOfOwnerByIndex(address owner, uint256 index) public view virtual override returns (uint256) { 2022-08-nounsdao/contracts/base/ERC721Enumerable.sol::76 => function tokenByIndex(uint256 index) public view virtual override returns (uint256) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::649 => function _burnVetoPower() public { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::660 => function proposalThreshold() public view returns (uint256) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::668 => function quorumVotes() public view returns (uint256) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::851 => function _burnVetoPower() public { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::862 => function proposalThreshold() public view returns (uint256) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::1002 => function maxQuorumVotes() public view returns (uint256) {

[N-05] Consider addings checks for signature malleability

Use OpenZeppelin's ECDSA contract rather than calling ecrecover() directly

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::139 => address signatory = ecrecover(digest, v, r, s); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::484 => address signatory = ecrecover(digest, v, r, s); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::576 => address signatory = ecrecover(digest, v, r, s);

[N-06] Non-assembly method available

assembly{ id := chainid() } => uint256 id = block.chainid assembly { size := extcodesize() } => uint256 size = address().code.length

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::285 => chainId := chainid() 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::679 => chainId := chainid() 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::1013 => chainId := chainid()

[N-07] Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

instances:

2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::101 => bytes32 public constant BALLOT_TYPEHASH = keccak256('Ballot(uint256 proposalId,uint8 support)'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::105 => bytes32 public constant BALLOT_TYPEHASH = keccak256('Ballot(uint256 proposalId,uint8 support)');

[N-08] Lines are too long

Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length

2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::156 => /// @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 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::181 => /// @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 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::256 => /// @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 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::281 => /// @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 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::375 => /// @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 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::507 => /// @notice: Unlike GovernerBravo, votes are considered from the block the proposal was created in order to normalize quorumVotes and proposalThreshold metrics 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::599 => /// @notice: Unlike GovernerBravo, votes are considered from the block the proposal was created in order to normalize quorumVotes and proposalThreshold metrics

[G-01] Don't Initialize Variables with Default Value

Uninitialized variables are assigned with the types default value. Explicitly initializing a variable with it's default value costs unnecesary gas.

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::181 => uint32 lower = 0; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::281 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::319 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::346 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::371 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::292 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::330 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::357 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::382 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::948 => uint256 lower = 0;
2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol 230: newProposal.forVotes = 0; 231: newProposal.againstVotes = 0; 232: newProposal.abstainVotes = 0; 233: newProposal.canceled = false; 234: newProposal.executed = false; 235: newProposal.vetoed = false;

[G-02] Cache Array Length Outside of Loop

Caching the array length outside a loop saves reading it on each iteration, as long as the array's length is not changed during the loop.

2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::281 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::319 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::346 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::371 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::292 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::330 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::357 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::382 => for (uint256 i = 0; i < proposal.targets.length; i++) {

[G-03] Use Shift Right/Left instead of Division/Multiplication if possible

A division/multiplication by any number x being a power of 2 can be calculated by shifting log2(x) to the right/left.

While the DIV opcode uses 5 gas, the SHR opcode only uses 3 gas. Furthermore, Solidity's division operation also includes a division-by-0 prevention which is bypassed using shifting.

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::184 => uint32 center = upper - (upper - lower) / 2; // ceil, avoiding overflow 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::951 => uint256 center = upper - (upper - lower) / 2;

[G-04] Use calldata instead of memory

Use calldata instead of memory for function parameters saves gas if the function argument is only read.

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::253 => function safe32(uint256 n, string memory errorMessage) internal pure returns (uint32) { 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::258 => function safe96(uint256 n, string memory errorMessage) internal pure returns (uint96) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::1018 => function safe32(uint256 n, string memory errorMessage) internal pure returns (uint32) {

[G-05] Empty blocks should be removed or emit something

The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting.

2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::1030 => receive() external payable {}

[G-06] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::41 => uint8 public constant decimals = 0; 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::48 => uint32 fromBlock; 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::152 => uint32 nCheckpoints = numCheckpoints[account]; 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::166 => uint32 nCheckpoints = numCheckpoints[account]; 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::181 => uint32 lower = 0; 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::182 => uint32 upper = nCheckpoints - 1; 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::218 => uint8 support; 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::322 => uint8 support; 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::352 => uint16 minQuorumVotesBPS; 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::354 => uint16 maxQuorumVotesBPS; 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::357 => uint32 quorumCoefficient; 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::363 => uint32 fromBlock; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::687 => uint16 oldMinQuorumVotesBPS = params.minQuorumVotesBPS; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::714 => uint16 oldMaxQuorumVotesBPS = params.maxQuorumVotesBPS; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::730 => uint32 oldQuorumCoefficient = params.quorumCoefficient; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::923 => uint32 blockNumber = safe32(blockNumber_, 'NounsDAO::getDynamicQuorumParamsAt: block number exceeds 32 bits'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::965 => uint32 blockNumber = safe32(block.number, 'block number exceeds 32 bits');

[G-07] ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, for example when used in for- and while-loops

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::141 => require(nonce == nonces[signatory]++, 'ERC721Checkpointable::delegateBySig: invalid nonce'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::216 => proposalCount++; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::281 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::319 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::346 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::371 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::226 => proposalCount++; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::292 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::330 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::357 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::382 => for (uint256 i = 0; i < proposal.targets.length; i++) {

[G-08] abi.encode() is less efficient than abi.encodePacked()

use abi.encodePacked() where possible to save gas

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::135 => abi.encode(DOMAIN_TYPEHASH, keccak256(bytes(name())), getChainId(), address(this)) 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::137 => bytes32 structHash = keccak256(abi.encode(DELEGATION_TYPEHASH, delegatee, nonce, expiry)); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::302 => !timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::480 => abi.encode(DOMAIN_TYPEHASH, keccak256(bytes(name)), getChainIdInternal(), address(this)) 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::482 => bytes32 structHash = keccak256(abi.encode(BALLOT_TYPEHASH, proposalId, support)); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::313 => !timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::572 => abi.encode(DOMAIN_TYPEHASH, keccak256(bytes(name)), getChainIdInternal(), address(this)) 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::574 => bytes32 structHash = keccak256(abi.encode(BALLOT_TYPEHASH, proposalId, support));

[G-09] Use a more recent version of solidity

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

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::35 => pragma solidity ^0.8.6; 2022-08-nounsdao/contracts/base/ERC721Enumerable.sol::28 => pragma solidity ^0.8.0; 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::33 => pragma solidity ^0.8.6; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::61 => pragma solidity ^0.8.6; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::53 => pragma solidity ^0.8.6; 2022-08-nounsdao/contracts/governance/NounsDAOProxy.sol::36 => pragma solidity ^0.8.6;

[G-10] Prefix increments cheaper than Postfix increments

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

2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::281 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::319 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::346 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::371 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::292 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::330 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::357 => for (uint256 i = 0; i < proposal.targets.length; i++) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::382 => for (uint256 i = 0; i < proposal.targets.length; i++) {

[G-11] Use bytes32 instead of string

Use bytes32 instead of string to save gas whenever possible. String is a dynamic data structure and therefore is more gas consuming then bytes32.

2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::67 => string public constant name = 'Nouns DAO'; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::59 => string public constant name = 'Nouns DAO';

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

Saves 16 gas per instance. If you're using the Optimizer at 200, instead of using the && operator in a single require statement to check multiple conditions, multiple require statements with 1 condition per require statement should be used to save gas:

2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::617 => require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::819 => require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only');

[G-13] Public functions not called by the contract should be declared external instead

Contracts are allowed to override their parents' functions and change the visibility from external to public and can save gas by doing so.

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::163 => function getPriorVotes(address account, uint256 blockNumber) public view returns (uint96) { 2022-08-nounsdao/contracts/base/ERC721Enumerable.sol::61 => function tokenOfOwnerByIndex(address owner, uint256 index) public view virtual override returns (uint256) { 2022-08-nounsdao/contracts/base/ERC721Enumerable.sol::76 => function tokenByIndex(uint256 index) public view virtual override returns (uint256) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::649 => function _burnVetoPower() public { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::660 => function proposalThreshold() public view returns (uint256) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::668 => function quorumVotes() public view returns (uint256) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::851 => function _burnVetoPower() public { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::862 => function proposalThreshold() public view returns (uint256) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::1002 => function maxQuorumVotes() public view returns (uint256) {

[G-14] Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::44 => mapping(address => address) private _delegates; 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::53 => mapping(address => mapping(uint32 => Checkpoint)) public checkpoints; 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::56 => mapping(address => uint32) public numCheckpoints; 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::67 => mapping(address => uint256) public nonces;
2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::172 => mapping(address => uint256) public latestProposalIds; 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::210 => mapping(address => Receipt) receipts; 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::272 => mapping(address => uint256) public latestProposalIds; 2022-08-nounsdao/contracts/governance/NounsDAOInterfaces.sol::310 => mapping(address => Receipt) receipts;

[G-15] Use assembly to check for address(0)

Saves 6 gas per instance if using assembly to check for address(0)

e.g.

assembly { if iszero(_addr) { mstore(0x00, "zero address") revert(0x00, 0x20) } }

instances:

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::140 => require(signatory != address(0), 'ERC721Checkpointable::delegateBySig: invalid signature'); 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::216 => if (srcRep != address(0)) { 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::223 => if (dstRep != address(0)) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::124 => require(timelock_ != address(0), 'NounsDAO::initialize: invalid timelock address'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::125 => require(nouns_ != address(0), 'NounsDAO::initialize: invalid nouns address'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::364 => require(vetoer != address(0), 'NounsDAO::veto: veto power burned'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::485 => require(signatory != address(0), 'NounsDAO::castVoteBySig: invalid signature'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::617 => require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::135 => require(timelock_ != address(0), 'NounsDAO::initialize: invalid timelock address'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::136 => require(nouns_ != address(0), 'NounsDAO::initialize: invalid nouns address'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::375 => require(vetoer != address(0), 'NounsDAO::veto: veto power burned'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::577 => require(signatory != address(0), 'NounsDAO::castVoteBySig: invalid signature'); 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::819 => require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only'); 2022-08-nounsdao/contracts/governance/NounsDAOProxy.sol::80 => require(implementation_ != address(0), 'NounsDAOProxy::_setImplementation: invalid implementation address');

[G-16] Use selfbalance()

Use selfbalance() instead of address(this).balance when getting your contract's balance of ETH to save gas.

2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::788 => uint256 amount = address(this).balance; 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::976 => uint256 balance = address(this).balance;

[G-17] internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::253 => function safe32(uint256 n, string memory errorMessage) internal pure returns (uint32) { 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::258 => function safe96(uint256 n, string memory errorMessage) internal pure returns (uint96) { 2022-08-nounsdao/contracts/base/ERC721Checkpointable.sol::282 => function getChainId() internal view returns (uint256) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol::676 => function getChainIdInternal() internal view returns (uint256) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::866 => function proposalCreationBlock(Proposal storage proposal) internal view returns (uint256) { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::974 => function _refundGas(uint256 startGas) internal { 2022-08-nounsdao/contracts/governance/NounsDAOLogicV2.sol::1010 => function getChainIdInternal() internal view returns (uint256) { 2022-08-nounsdao/contracts/governance/NounsDAOProxy.sol::94 => function delegateTo(address callee, bytes memory data) internal {

[G-18] Use calldata instead of memory

2022-08-nounsdao/contracts/governance/NounsDAOLogicV1.sol 175: address[] memory targets, 176: uint256[] memory values, 177: string[] memory signatures, 178: bytes[] memory calldatas, 179: string memory description
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