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: 52/160
Findings: 2
Award: $52.34
🌟 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.6777 USDC - $35.68
_moveDelegates() will revert if user not delegated to selfFunction delegates() returns delegator's own address instead of address(0) if not self delegated, if a user delegates votes to another address function will revert due to underflow
If delegates() returns address(0) instead of user's own address, condition if (srcRep != address(0)) fails, function will not revert.
So users have to self delegate before delegating votes to other users
function delegate(address delegatee) public { if (delegatee == address(0)) delegatee = msg.sender; return _delegate(msg.sender, delegatee); }
if (srcRep != address(0)) { .... uint96 srcRepNew = sub96(srcRepOld, amount, 'ERC721Checkpointable::_moveDelegates: amount underflows');
add96() will fail without error messageIf expression a + b overflows execution will revert and require condition is not executed and errorMessage is not used
Condition can be placed in a unchecked to prevent revert due to overflow
function add96( uint96 a, uint96 b, string memory errorMessage ) internal pure returns (uint96) { uint96 c = a + b; require(c >= a, errorMessage); return c; }
Some natspec comments are missing @param comments
/** * @dev See {IERC165-supportsInterface}. */ function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC721) returns (bool)
/** * @dev See {IERC721Enumerable-tokenOfOwnerByIndex}. */ function tokenOfOwnerByIndex(address owner, uint256 index) public view virtual override returns (uint256)
/** * @dev See {IERC721Enumerable-totalSupply}. */ function totalSupply() public view virtual override returns (uint256)
/** * @dev See {IERC721Enumerable-tokenByIndex}. */ function tokenByIndex(uint256 index) public view virtual override returns (uint256)
/** * @notice The votes a delegator can delegate, which is the current balance of the delegator. * @dev Used when calling `_delegate()` */ function votesToDelegate(address delegator) public view returns (uint96)
/** * @notice Overrides the standard `Comp.sol` delegates mapping to return * the delegator's own address if they haven't delegated. * This avoids having to delegate to oneself. */ function delegates(address delegator) public view returns (address)
/** * @notice Cast a vote for a proposal by signature * @dev External function that accepts EIP-712 signatures for voting on proposals. */ function castVoteBySig( uint256 proposalId, uint8 support, uint8 v, bytes32 r, bytes32 s ) external
/** * @notice Changes vetoer address * @dev Vetoer function for updating vetoer address */ function _setVetoer(address newVetoer)
/** * @notice Current proposal threshold using Noun Total Supply * Differs from `GovernerBravo` which uses fixed amount */ function proposalThreshold() public view returns (uint256)
/** * @notice Cast a vote for a proposal by signature * @dev External function that accepts EIP-712 signatures for voting on proposals. */ function castVoteBySig( uint256 proposalId, uint8 support, uint8 v, bytes32 r, bytes32 s ) external
/** * @notice Quorum votes required for a specific proposal to succeed * Differs from `GovernerBravo` which uses fixed amount */ function quorumVotes(uint256 proposalId) public view returns (uint256)
🌟 Selected for report: IllIllI
Also found by: 0x040, 0x1f8b, 0xDjango, 0xNazgul, 0xNineDec, 0xSmartContract, 0xbepresent, 0xc0ffEE, 0xkatana, 2997ms, ACai, Amithuddar, Aymen0909, Ben, BipinSah, Bjorn_bug, Bnke0x0, CertoraInc, Ch_301, Chom, CodingNameKiki, Deivitto, DevABDee, DimitarDimitrov, Diraco, Dravee, ElKu, EthLedger, Fitraldys, Funen, GalloDaSballo, GimelSec, Guardian, IgnacioB, JC, JohnSmith, Junnon, KIntern_NA, Lambda, LeoS, Noah3o6, Olivierdem, Polandia94, R2, Randyyy, RaymondFam, Respx, ReyAdmirado, Rohan16, RoiEvenHaim, Rolezn, Ruhum, SaharAP, Saintcode_, SerMyVillage, Shishigami, Sm4rty, SooYa, TomJ, Tomio, Tomo, Waze, Yiko, _Adam, __141345__, a12jmx, ajtra, ak1, bobirichman, brgltd, bulej93, c3phas, cRat1st0s, carlitox477, catchup, ch0bu, d3e4, delfin454000, djxploit, durianSausage, erictee, exolorkistis, fatherOfBlocks, francoHacker, gogo, hyh, ignacio, jag, joestakey, karanctf, ladboy233, lucacez, lukris02, m_Rassska, martin, medikko, mics, mrpathfindr, natzuu, newfork01, oyc_109, pauliax, peritoflores, pfapostol, prasantgupta52, rbserver, ret2basic, rfa, robee, rokinot, rotcivegaf, rvierdiiev, sach1r0, saian, samruna, seyni, shark, shr1ftyy, sikorico, simon135, sryysryy, tay054, tnevler, wagmi, zishansami
16.6573 USDC - $16.66
If a variables is not initialized, it is assumed to contain default values (0, address(0), false), explicitly intialising with default values is not necessary
uint32 lower = 0;
newProposal.eta = 0; ... newProposal.forVotes = 0; newProposal.againstVotes = 0; newProposal.abstainVotes = 0; newProposal.canceled = false; newProposal.executed = false; newProposal.vetoed = false;
newProposal.eta = 0; ... newProposal.forVotes = 0; newProposal.againstVotes = 0; newProposal.abstainVotes = 0; newProposal.canceled = false; newProposal.executed = false; newProposal.vetoed = false;
uint256 lower = 0;
msg.sender != address(0)
require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only');
Function that are not called within the contract can be external instead of public
function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC721) returns (bool) { return interfaceId == type(IERC721Enumerable).interfaceId || super.supportsInterface(interfaceId); }
function tokenOfOwnerByIndex(address owner, uint256 index) public view virtual override returns (uint256) { require(index < ERC721.balanceOf(owner), 'ERC721Enumerable: owner index out of bounds'); return _ownedTokens[owner][index]; }
function delegate(address delegatee) public { if (delegatee == address(0)) delegatee = msg.sender; return _delegate(msg.sender, delegatee); }
function delegateBySig( address delegatee, uint256 nonce, uint256 expiry, uint8 v, bytes32 r, bytes32 s ) public
function getPriorVotes(address account, uint256 blockNumber) public view returns (uint96) {
Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met. Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.
require(index < ERC721.balanceOf(owner), 'ERC721Enumerable: owner index out of bounds');
require(index < ERC721Enumerable.totalSupply(), 'ERC721Enumerable: global index out of bounds');
return safe96(balanceOf(delegator), 'ERC721Checkpointable::votesToDelegate: amount exceeds 96 bits');
require(signatory != address(0), 'ERC721Checkpointable::delegateBySig: invalid signature'); require(nonce == nonces[signatory]++, 'ERC721Checkpointable::delegateBySig: invalid nonce'); require(block.timestamp <= expiry, 'ERC721Checkpointable::delegateBySig: signature expired');
require(blockNumber < block.number, 'ERC721Checkpointable::getPriorVotes: not yet determined');
uint96 srcRepNew = sub96(srcRepOld, amount, 'ERC721Checkpointable::_moveDelegates: amount underflows');
uint96 dstRepNew = add96(dstRepOld, amount, 'ERC721Checkpointable::_moveDelegates: amount overflows');
uint32 blockNumber = safe32( block.number, 'ERC721Checkpointable::_writeCheckpoint: block number exceeds 32 bits' );
require(msg.sender == admin, 'NounsDAOProxy::_setImplementation: admin only'); require(implementation_ != address(0), 'NounsDAOProxy::_setImplementation: invalid implementation address');
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' );
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' );
require( state(proposalId) == ProposalState.Succeeded, 'NounsDAO::queue: proposal can only be queued if it is succeeded' );
require( !timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), 'NounsDAO::queueOrRevertInternal: identical proposal action already queued at eta' );
require( state(proposalId) == ProposalState.Queued, 'NounsDAO::execute: proposal can only be executed if it is queued' );
require(state(proposalId) != ProposalState.Executed, 'NounsDAO::cancel: cannot cancel executed proposal');
require( msg.sender == proposal.proposer || nouns.getPriorVotes(proposal.proposer, block.number - 1) < proposal.proposalThreshold, 'NounsDAO::cancel: proposer above threshold' );
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');
require(proposalCount >= proposalId, 'NounsDAO::state: invalid proposal id');
require(signatory != address(0), 'NounsDAO::castVoteBySig: invalid signature');
require(state(proposalId) == ProposalState.Active, 'NounsDAO::castVoteInternal: voting is closed'); require(support <= 2, 'NounsDAO::castVoteInternal: invalid vote type'); ... require(receipt.hasVoted == false, 'NounsDAO::castVoteInternal: voter already voted');
require(msg.sender == admin, 'NounsDAO::_setVotingDelay: admin only'); require( newVotingDelay >= MIN_VOTING_DELAY && newVotingDelay <= MAX_VOTING_DELAY, 'NounsDAO::_setVotingDelay: invalid voting delay' );
require(msg.sender == admin, 'NounsDAO::_setVotingPeriod: admin only'); require( newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD, 'NounsDAO::_setVotingPeriod: invalid voting period' );
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' );
require(msg.sender == admin, 'NounsDAO::_setMinQuorumVotesBPS: admin only'); 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' );
require(msg.sender == admin, 'NounsDAO::_setMaxQuorumVotesBPS: admin only'); DynamicQuorumParams memory params = getDynamicQuorumParamsAt(block.number); 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' );
require(msg.sender == admin, 'NounsDAO::_setQuorumCoefficient: admin only');
require(msg.sender == admin, 'NounsDAO::_setPendingAdmin: admin only');
require(msg.sender == pendingAdmin && msg.sender != address(0), 'NounsDAO::_acceptAdmin: pending admin only');
require(msg.sender == vetoer, 'NounsDAO::_setVetoer: vetoer only');
require(msg.sender == vetoer, 'NounsDAO::_burnVetoPower: vetoer only');
Custom errors from solidity 0.8.4 are more efficient than revert strings with cheaper deployment and runtime time costs when revert condition is met
Refer: https://blog.soliditylang.org/2021/04/21/custom-errors/](https://blog.soliditylang.org/2021/04/21/custom-errors/
unchecked to save gasIn Solidity 0.8.4 and above there is a default underflow/overflow checks on unsigned integers. it is possible to avoid this check by using unchecked. If the expression does not cause overflow/underflow, it can be unchecked to save some gas.
require(b <= a, errorMessage); return a - b;
Storage reads in events can be replaced with function parameters and local variables to save gas
implementation can be replaced by implementation_
emit NewImplementation(oldImplementation, implementation);
newProposal.startBlock, newProposal.endBlock newProposal.proposalThreshold can be replaced by temp.startblock, temp.endBlock, temp.proposalThreshold
emit ProposalCreated( newProposal.id, msg.sender, targets, values, signatures, calldatas, newProposal.startBlock, newProposal.endBlock, description ); emit ProposalCreatedWithRequirements( newProposal.id, msg.sender, targets, values, signatures, calldatas, newProposal.startBlock, newProposal.endBlock, newProposal.proposalThreshold, newProposal.quorumVotes, description );
Use newVotingDelay instead of votingDelay
emit VotingDelaySet(oldVotingDelay, votingDelay);
Use newVotingPeriod instead of votingPeriod
emit VotingPeriodSet(oldVotingPeriod, votingPeriod);
Use newProposalThresholdBPS instead of proposalThresholdBPS
emit ProposalThresholdBPSSet(oldProposalThresholdBPS, proposalThresholdBPS);
Use newQuorumVotesBPS instead of quorumVotesBPS
emit QuorumVotesBPSSet(oldQuorumVotesBPS, quorumVotesBPS);
Use oldPendingAdmin, address(0) instead of admin, pendingAdmin
emit NewAdmin(oldAdmin, admin); emit NewPendingAdmin(oldPendingAdmin, pendingAdmin);
Use msg.sender instead of vetoer
emit NewVetoer(vetoer, newVetoer);
newProposal.startBlock, newProposal.endBlock newProposal.proposalThreshold can be replaced by temp.startblock, temp.endBlock, temp.proposalThreshold
emit ProposalCreated( newProposal.id, msg.sender, targets, values, signatures, calldatas, newProposal.startBlock, newProposal.endBlock, description ); ... emit ProposalCreatedWithRequirements( newProposal.id, msg.sender, targets, values, signatures, calldatas, newProposal.startBlock, newProposal.endBlock, newProposal.proposalThreshold, minQuorumVotes(), description );
Use newVotingDelay instead of votingDelay
emit VotingDelaySet(oldVotingDelay, votingDelay);
Use newVotingPeriod instead of votingPeriod
emit VotingPeriodSet(oldVotingPeriod, votingPeriod);
Use newProposalThresholdBPS instead of proposalThresholdBPS
emit ProposalThresholdBPSSet(oldProposalThresholdBPS, proposalThresholdBPS);
Use oldPendingAdmin, address(0) instead of admin, pendingAdmin
emit NewPendingAdmin(oldPendingAdmin, pendingAdmin);
Storage values that are read multiple times in the same function can be cached to avoid expensive SLOAD and save gas
proposalCount can be cached in a variable
proposalCount++; Proposal storage newProposal = proposals[proposalCount]; newProposal.id = proposalCount;
proposalCount can be cached in a variable
proposalCountTemp = ++proposalCount; Proposal storage newProposal = proposals[proposalCountTemp]; newProposal.id = proposalCountTemp; ... return proposalCountTemp;
proposalCount++; Proposal storage newProposal = _proposals[proposalCount]; newProposal.id = proposalCount;
proposalCount can be cached in a variable
proposalCountTemp = ++proposalCount; Proposal storage newProposal = proposals[proposalCountTemp]; newProposal.id = proposalCountTemp; ... return proposalCountTemp;
Array length can be cached and used in the loop condition instead of reading it in every iteration and save gas
for (uint256 i = 0; i < proposal.targets.length; i++)
for (uint256 i = 0; i < proposal.targets.length; i++)
for (uint256 i = 0; i < proposal.targets.length; i++)
for (uint256 i = 0; i < proposal.targets.length; i++)
for (uint256 i = 0; i < proposal.targets.length; i++)
for (uint256 i = 0; i < proposal.targets.length; i++)
for (uint256 i = 0; i < proposal.targets.length; i++)
for (uint256 i = 0; i < proposal.targets.length; i++)
Boolean comparison with a boolean constant is not necessary
require(receipt.hasVoted == false, 'NounsDAO::castVoteInternal: voter already voted');
require(receipt.hasVoted == false, 'NounsDAO::castVoteInternal: voter already voted');
Division by 2 can be performed by shifting right by 1 bit which costs 3 gas
uint256 center = upper - (upper - lower) / 2;
Replace / with >>
uint256 center = upper - (upper - lower) >> 2;