Nouns Builder contest - oyc_109's results

[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-09-nouns-builder/src/lib/interfaces/IEIP712.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IERC1967Upgrade.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IERC721.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IERC721Votes.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IInitializable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IOwnable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IPausable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IUUPS.sol::2 => pragma solidity ^0.8.15;
2022-09-nouns-builder/src/lib/interfaces/IWETH.sol::2 => pragma solidity ^0.8.15;
2022-09-nouns-builder/src/lib/proxy/ERC1967Proxy.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/proxy/ERC1967Upgrade.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/proxy/UUPS.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/token/ERC721.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/Address.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/EIP712.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/Initializable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/Ownable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/Pausable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/ReentrancyGuard.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/SafeCast.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/TokenReceiver.sol::2 => pragma solidity ^0.8.0;

[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-09-nouns-builder/src/auction/Auction.sol::98 => if (block.timestamp >= _auction.endTime) revert AUCTION_OVER();
2022-09-nouns-builder/src/auction/Auction.sol::141 => extend = (_auction.endTime - block.timestamp) < settings.timeBuffer;
2022-09-nouns-builder/src/auction/Auction.sol::149 => auction.endTime = uint40(block.timestamp + settings.timeBuffer);
2022-09-nouns-builder/src/auction/Auction.sol::178 => if (block.timestamp < _auction.endTime) revert AUCTION_ACTIVE();
2022-09-nouns-builder/src/auction/Auction.sol::211 => uint256 startTime = block.timestamp;
2022-09-nouns-builder/src/governance/governor/Governor.sol::128 => if (getVotes(msg.sender, block.timestamp - 1) < proposalThreshold()) revert BELOW_PROPOSAL_THRESHOLD();
2022-09-nouns-builder/src/governance/governor/Governor.sol::160 => snapshot = block.timestamp + settings.votingDelay;
2022-09-nouns-builder/src/governance/governor/Governor.sol::170 => proposal.timeCreated = uint32(block.timestamp);
2022-09-nouns-builder/src/governance/governor/Governor.sol::218 => if (block.timestamp > _deadline) revert EXPIRED_SIGNATURE();
2022-09-nouns-builder/src/governance/governor/Governor.sol::363 => if (msg.sender != proposal.proposer && getVotes(proposal.proposer, block.timestamp - 1) > proposal.proposalThreshold)
2022-09-nouns-builder/src/governance/governor/Governor.sol::433 => } else if (block.timestamp < proposal.voteStart) {
2022-09-nouns-builder/src/governance/governor/Governor.sol::437 => } else if (block.timestamp < proposal.voteEnd) {
2022-09-nouns-builder/src/governance/treasury/Treasury.sol::76 => return block.timestamp > (timestamps[_proposalId] + settings.gracePeriod);
2022-09-nouns-builder/src/governance/treasury/Treasury.sol::89 => return timestamps[_proposalId] != 0 && block.timestamp >= timestamps[_proposalId];
2022-09-nouns-builder/src/governance/treasury/Treasury.sol::123 => eta = block.timestamp + settings.delay;
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::61 => if (_timestamp >= block.timestamp) revert INVALID_TIMESTAMP();
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::153 => if (block.timestamp > _deadline) revert EXPIRED_SIGNATURE();
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::253 => checkpoint.timestamp = uint64(block.timestamp);
2022-09-nouns-builder/src/token/Token.sol::186 => } else if (block.timestamp < tokenRecipient[baseTokenId].vestExpiry) {
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::251 => return uint256(keccak256(abi.encode(_tokenId, blockhash(block.number), block.coinbase, block.timestamp)));

[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-09-nouns-builder/src/governance/treasury/Treasury.sol::269 => 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-09-nouns-builder/src/manager/Manager.sol::68 => metadataHash = keccak256(abi.encodePacked(type(ERC1967Proxy).creationCode, abi.encode(_metadataImpl, "")));
2022-09-nouns-builder/src/manager/Manager.sol::69 => auctionHash = keccak256(abi.encodePacked(type(ERC1967Proxy).creationCode, abi.encode(_auctionImpl, "")));
2022-09-nouns-builder/src/manager/Manager.sol::70 => treasuryHash = keccak256(abi.encodePacked(type(ERC1967Proxy).creationCode, abi.encode(_treasuryImpl, "")));
2022-09-nouns-builder/src/manager/Manager.sol::71 => governorHash = keccak256(abi.encodePacked(type(ERC1967Proxy).creationCode, abi.encode(_governorImpl, "")));
2022-09-nouns-builder/src/manager/Manager.sol::167 => metadata = address(uint160(uint256(keccak256(abi.encodePacked(bytes1(0xff), address(this), salt, metadataHash)))));
2022-09-nouns-builder/src/manager/Manager.sol::168 => auction = address(uint160(uint256(keccak256(abi.encodePacked(bytes1(0xff), address(this), salt, auctionHash)))));
2022-09-nouns-builder/src/manager/Manager.sol::169 => treasury = address(uint160(uint256(keccak256(abi.encodePacked(bytes1(0xff), address(this), salt, treasuryHash)))));
2022-09-nouns-builder/src/manager/Manager.sol::170 => governor = address(uint160(uint256(keccak256(abi.encodePacked(bytes1(0xff), address(this), salt, governorHash)))));

[L-05] Unsafe use of transfer()/transferFrom() with IERC20

Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether (USDT)'s transfer() and transferFrom() functions do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast to IERC20, their function signatures do not match and therefore the calls made, revert. Use OpenZeppelin’s SafeERC20's safeTransfer()/safeTransferFrom() instead

2022-09-nouns-builder/src/auction/Auction.sol::192 => token.transferFrom(address(this), _auction.highestBidder, _auction.tokenId);
2022-09-nouns-builder/src/auction/Auction.sol::363 => IWETH(WETH).transfer(_to, _amount);

[L-06] Missing Zero address checks

Zero-address checks are a best practice for input validation of critical address parameters. While the codebase applies this to most cases, there are many places where this is missing in constructors and setters. Impact: Accidental use of zero-addresses may result in exceptions, burn fees/tokens, or force redeployment of contracts.

2022-09-nouns-builder/src/auction/Auction.sol::41 => WETH = _weth;
2022-09-nouns-builder/src/auction/Auction.sol::79 => settings.treasury = _treasury;
2022-09-nouns-builder/src/governance/governor/Governor.sol::76 => settings.vetoer = _vetoer;
2022-09-nouns-builder/src/manager/Manager.sol::62 => tokenImpl = _tokenImpl;
2022-09-nouns-builder/src/manager/Manager.sol::63 => metadataImpl = _metadataImpl;
2022-09-nouns-builder/src/manager/Manager.sol::64 => auctionImpl = _auctionImpl;
2022-09-nouns-builder/src/manager/Manager.sol::65 => treasuryImpl = _treasuryImpl;
2022-09-nouns-builder/src/manager/Manager.sol::66 => governorImpl = _governorImpl;
2022-09-nouns-builder/src/token/Token.sol::66 => settings.auction = _auction;
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::65 => settings.token = _token;
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::66 => settings.treasury = _treasury;

[L-07] Events not emitted for important state changes

When changing state variables events are not emitted. Emitting events allows monitoring activities with off-chain monitoring tools.

2022-09-nouns-builder/src/auction/Auction.sol::268 => function settleAuction() external nonReentrant whenPaused {

[N-1] 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-09-nouns-builder/src/lib/interfaces/IEIP712.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IERC1967Upgrade.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IERC721.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IERC721Votes.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IInitializable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IOwnable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IPausable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/proxy/ERC1967Proxy.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/proxy/ERC1967Upgrade.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/proxy/UUPS.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/token/ERC721.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/Address.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/EIP712.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/Initializable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/Ownable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/Pausable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/ReentrancyGuard.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/SafeCast.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/TokenReceiver.sol::2 => pragma solidity ^0.8.0;

[N-2] Event is missing indexed fields

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

2022-09-nouns-builder/src/auction/IAuction.sol::22 => event AuctionBid(uint256 tokenId, address bidder, uint256 amount, bool extended, uint256 endTime);
2022-09-nouns-builder/src/auction/IAuction.sol::28 => event AuctionSettled(uint256 tokenId, address winner, uint256 amount);
2022-09-nouns-builder/src/auction/IAuction.sol::34 => event AuctionCreated(uint256 tokenId, uint256 startTime, uint256 endTime);
2022-09-nouns-builder/src/auction/IAuction.sol::38 => event DurationUpdated(uint256 duration);
2022-09-nouns-builder/src/auction/IAuction.sol::42 => event ReservePriceUpdated(uint256 reservePrice);
2022-09-nouns-builder/src/auction/IAuction.sol::46 => event MinBidIncrementPercentageUpdated(uint256 minBidIncrementPercentage);
2022-09-nouns-builder/src/auction/IAuction.sol::50 => event TimeBufferUpdated(uint256 timeBuffer);
2022-09-nouns-builder/src/governance/governor/IGovernor.sol::29 => event ProposalQueued(bytes32 proposalId, uint256 eta);
2022-09-nouns-builder/src/governance/governor/IGovernor.sol::33 => event ProposalExecuted(bytes32 proposalId);
2022-09-nouns-builder/src/governance/governor/IGovernor.sol::36 => event ProposalCanceled(bytes32 proposalId);
2022-09-nouns-builder/src/governance/governor/IGovernor.sol::39 => event ProposalVetoed(bytes32 proposalId);
2022-09-nouns-builder/src/governance/governor/IGovernor.sol::42 => event VoteCast(address voter, bytes32 proposalId, uint256 support, uint256 weight, string reason);
2022-09-nouns-builder/src/governance/governor/IGovernor.sol::45 => event VotingDelayUpdated(uint256 prevVotingDelay, uint256 newVotingDelay);
2022-09-nouns-builder/src/governance/governor/IGovernor.sol::48 => event VotingPeriodUpdated(uint256 prevVotingPeriod, uint256 newVotingPeriod);
2022-09-nouns-builder/src/governance/governor/IGovernor.sol::51 => event ProposalThresholdBpsUpdated(uint256 prevBps, uint256 newBps);
2022-09-nouns-builder/src/governance/governor/IGovernor.sol::54 => event QuorumVotesBpsUpdated(uint256 prevBps, uint256 newBps);
2022-09-nouns-builder/src/governance/governor/IGovernor.sol::57 => event VetoerUpdated(address prevVetoer, address newVetoer);
2022-09-nouns-builder/src/governance/treasury/ITreasury.sol::16 => event TransactionScheduled(bytes32 proposalId, uint256 timestamp);
2022-09-nouns-builder/src/governance/treasury/ITreasury.sol::19 => event TransactionCanceled(bytes32 proposalId);
2022-09-nouns-builder/src/governance/treasury/ITreasury.sol::22 => event TransactionExecuted(bytes32 proposalId, address[] targets, uint256[] values, bytes[] payloads);
2022-09-nouns-builder/src/governance/treasury/ITreasury.sol::25 => event DelayUpdated(uint256 prevDelay, uint256 newDelay);
2022-09-nouns-builder/src/governance/treasury/ITreasury.sol::28 => event GracePeriodUpdated(uint256 prevGracePeriod, uint256 newGracePeriod);
2022-09-nouns-builder/src/lib/interfaces/IERC1967Upgrade.sol::14 => event Upgraded(address impl);
2022-09-nouns-builder/src/lib/interfaces/IInitializable.sol::13 => event Initialized(uint256 version);
2022-09-nouns-builder/src/lib/interfaces/IPausable.sol::14 => event Paused(address user);
2022-09-nouns-builder/src/lib/interfaces/IPausable.sol::18 => event Unpaused(address user);
2022-09-nouns-builder/src/manager/IManager.sol::21 => event DAODeployed(address token, address metadata, address auction, address treasury, address governor);
2022-09-nouns-builder/src/manager/IManager.sol::26 => event UpgradeRegistered(address baseImpl, address upgradeImpl);
2022-09-nouns-builder/src/manager/IManager.sol::31 => event UpgradeRemoved(address baseImpl, address upgradeImpl);
2022-09-nouns-builder/src/token/IToken.sol::21 => event MintScheduled(uint256 baseTokenId, uint256 founderId, Founder founder);
2022-09-nouns-builder/src/token/metadata/interfaces/IPropertyIPFSMetadataRenderer.sol::16 => event PropertyAdded(uint256 id, string name);
2022-09-nouns-builder/src/token/metadata/interfaces/IPropertyIPFSMetadataRenderer.sol::19 => event ItemAdded(uint256 propertyId, uint256 index);
2022-09-nouns-builder/src/token/metadata/interfaces/IPropertyIPFSMetadataRenderer.sol::22 => event ContractImageUpdated(string prevImage, string newImage);
2022-09-nouns-builder/src/token/metadata/interfaces/IPropertyIPFSMetadataRenderer.sol::25 => event RendererBaseUpdated(string prevRendererBase, string newRendererBase);
2022-09-nouns-builder/src/token/metadata/interfaces/IPropertyIPFSMetadataRenderer.sol::28 => event DescriptionUpdated(string prevDescription, string newDescription);

[N-3] Missing event for critical parameter change

Emitting events after sensitive changes take place, to facilitate tracking and notify off-chain clients following changes to the contract.

2022-09-nouns-builder/src/auction/Auction.sol::268 => function settleAuction() external nonReentrant whenPaused {
2022-09-nouns-builder/src/auction/IAuction.sol::106 => function settleCurrentAndCreateNewAuction() external;
2022-09-nouns-builder/src/auction/IAuction.sol::109 => function settleAuction() external;
2022-09-nouns-builder/src/auction/IAuction.sol::131 => function setDuration(uint256 duration) external;
2022-09-nouns-builder/src/auction/IAuction.sol::135 => function setReservePrice(uint256 reservePrice) external;
2022-09-nouns-builder/src/auction/IAuction.sol::139 => function setTimeBuffer(uint256 timeBuffer) external;
2022-09-nouns-builder/src/auction/IAuction.sol::143 => function setMinimumBidIncrement(uint256 percentage) external;
2022-09-nouns-builder/src/lib/interfaces/IERC721.sol::78 => function setApprovalForAll(address operator, bool approved) external;

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

instances:

2022-09-nouns-builder/src/governance/governor/Governor.sol::27 => bytes32 public constant VOTE_TYPEHASH = keccak256("Vote(address voter,uint256 proposalId,uint256 support,uint256 nonce,uint256 deadline)");
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::21 => bytes32 internal constant DELEGATION_TYPEHASH = keccak256("Delegation(address from,address to,uint256 nonce,uint256 deadline)");
2022-09-nouns-builder/src/lib/utils/EIP712.sol::19 => bytes32 internal constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");

[N-5] 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-09-nouns-builder/src/lib/token/ERC721Votes.sol::10 => /// @notice Modified from OpenZeppelin Contracts v4.7.3 (token/ERC721/extensions/draft-ERC721Votes.sol) & Nouns DAO ERC721Checkpointable.sol commit 2cbe6c7 - licensed under the BSD-3-Clause license.

[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-09-nouns-builder/src/governance/treasury/Treasury.sol::162 => for (uint256 i = 0; i < numTargets; ++i) {
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::119 => for (uint256 i = 0; i < numNewProperties; ++i) {
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::133 => for (uint256 i = 0; i < numNewItems; ++i) {
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::189 => for (uint256 i = 0; i < numProperties; ++i) {
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::229 => for (uint256 i = 0; i < numProperties; ++i) {

[G-02] 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-09-nouns-builder/src/lib/token/ERC721Votes.sol::95 => middle = high - (high - low) / 2;

[G-03] Use calldata instead of memory

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

2022-09-nouns-builder/src/lib/utils/Address.sol::37 => function functionDelegateCall(address _target, bytes memory _data) internal returns (bytes memory) {
2022-09-nouns-builder/src/lib/utils/Address.sol::46 => function verifyCallResult(bool _success, bytes memory _returndata) internal pure returns (bytes memory) {
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::255 => function _getItemImage(Item memory _item, string memory _propertyName) private view returns (string memory) {
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::308 => function _encodeAsJson(bytes memory _jsonBlob) private pure returns (string memory) {

[G-04] Using private rather than public for constants, saves gas

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

2022-09-nouns-builder/src/manager/Manager.sol::25 => address public immutable tokenImpl;
2022-09-nouns-builder/src/manager/Manager.sol::28 => address public immutable metadataImpl;
2022-09-nouns-builder/src/manager/Manager.sol::31 => address public immutable auctionImpl;
2022-09-nouns-builder/src/manager/Manager.sol::34 => address public immutable treasuryImpl;
2022-09-nouns-builder/src/manager/Manager.sol::37 => address public immutable governorImpl;

[G-05] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

2022-09-nouns-builder/src/auction/Auction.sol::244 => function unpause() external onlyOwner {
2022-09-nouns-builder/src/auction/Auction.sol::263 => function pause() external onlyOwner {
2022-09-nouns-builder/src/auction/Auction.sol::307 => function setDuration(uint256 _duration) external onlyOwner {
2022-09-nouns-builder/src/auction/Auction.sol::315 => function setReservePrice(uint256 _reservePrice) external onlyOwner {
2022-09-nouns-builder/src/auction/Auction.sol::323 => function setTimeBuffer(uint256 _timeBuffer) external onlyOwner {
2022-09-nouns-builder/src/auction/Auction.sol::331 => function setMinimumBidIncrement(uint256 _percentage) external onlyOwner {
2022-09-nouns-builder/src/governance/governor/Governor.sol::564 => function updateVotingDelay(uint256 _newVotingDelay) external onlyOwner {
2022-09-nouns-builder/src/governance/governor/Governor.sol::572 => function updateVotingPeriod(uint256 _newVotingPeriod) external onlyOwner {
2022-09-nouns-builder/src/governance/governor/Governor.sol::580 => function updateProposalThresholdBps(uint256 _newProposalThresholdBps) external onlyOwner {
2022-09-nouns-builder/src/governance/governor/Governor.sol::588 => function updateQuorumThresholdBps(uint256 _newQuorumVotesBps) external onlyOwner {
2022-09-nouns-builder/src/governance/governor/Governor.sol::596 => function updateVetoer(address _newVetoer) external onlyOwner {
2022-09-nouns-builder/src/governance/governor/Governor.sol::605 => function burnVetoer() external onlyOwner {
2022-09-nouns-builder/src/governance/treasury/Treasury.sol::116 => function queue(bytes32 _proposalId) external onlyOwner returns (uint256 eta) {
2022-09-nouns-builder/src/governance/treasury/Treasury.sol::180 => function cancel(bytes32 _proposalId) external onlyOwner {
2022-09-nouns-builder/src/lib/proxy/UUPS.sol::47 => function upgradeTo(address _newImpl) external onlyProxy {
2022-09-nouns-builder/src/lib/utils/Ownable.sol::63 => function transferOwnership(address _newOwner) public onlyOwner {
2022-09-nouns-builder/src/lib/utils/Ownable.sol::71 => function safeTransferOwnership(address _newOwner) public onlyOwner {
2022-09-nouns-builder/src/lib/utils/Ownable.sol::78 => function acceptOwnership() public onlyPendingOwner {
2022-09-nouns-builder/src/lib/utils/Ownable.sol::87 => function cancelOwnershipTransfer() public onlyOwner {
2022-09-nouns-builder/src/manager/Manager.sol::187 => function registerUpgrade(address _baseImpl, address _upgradeImpl) external onlyOwner {
2022-09-nouns-builder/src/manager/Manager.sol::196 => function removeUpgrade(address _baseImpl, address _upgradeImpl) external onlyOwner {
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::347 => function updateContractImage(string memory _newContractImage) external onlyOwner {
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::355 => function updateRendererBase(string memory _newRendererBase) external onlyOwner {
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::363 => function updateDescription(string memory _newDescription) external onlyOwner {

[G-06] 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-09-nouns-builder/src/governance/treasury/Treasury.sol::269 => receive() external payable {}
2022-09-nouns-builder/src/lib/token/ERC721.sol::54 => function tokenURI(uint256 _tokenId) public view virtual returns (string memory) {}
2022-09-nouns-builder/src/lib/token/ERC721.sol::57 => function contractURI() public view virtual returns (string memory) {}
2022-09-nouns-builder/src/manager/Manager.sol::209 => function _authorizeUpgrade(address _newImpl) internal override onlyOwner {}

[G-07] 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-09-nouns-builder/src/auction/types/AuctionTypesV1.sol::18 => uint8 minBidIncrement;
2022-09-nouns-builder/src/governance/governor/types/GovernorTypesV1.sol::21 => uint16 proposalThresholdBps;
2022-09-nouns-builder/src/governance/governor/types/GovernorTypesV1.sol::22 => uint16 quorumThresholdBps;
2022-09-nouns-builder/src/governance/governor/types/GovernorTypesV1.sol::44 => uint32 timeCreated;
2022-09-nouns-builder/src/governance/governor/types/GovernorTypesV1.sol::45 => uint32 againstVotes;
2022-09-nouns-builder/src/governance/governor/types/GovernorTypesV1.sol::46 => uint32 forVotes;
2022-09-nouns-builder/src/governance/governor/types/GovernorTypesV1.sol::47 => uint32 abstainVotes;
2022-09-nouns-builder/src/governance/governor/types/GovernorTypesV1.sol::48 => uint32 voteStart;
2022-09-nouns-builder/src/governance/governor/types/GovernorTypesV1.sol::49 => uint32 voteEnd;
2022-09-nouns-builder/src/governance/governor/types/GovernorTypesV1.sol::50 => uint32 proposalThreshold;
2022-09-nouns-builder/src/governance/governor/types/GovernorTypesV1.sol::51 => uint32 quorumVotes;
2022-09-nouns-builder/src/governance/treasury/types/TreasuryTypesV1.sol::12 => uint128 gracePeriod;
2022-09-nouns-builder/src/governance/treasury/types/TreasuryTypesV1.sol::13 => uint128 delay;
2022-09-nouns-builder/src/lib/interfaces/IERC721Votes.sol::36 => uint64 timestamp;
2022-09-nouns-builder/src/lib/utils/Initializable.sol::17 => uint8 internal _initialized;
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::179 => uint16[16] storage tokenAttributes = attributes[_tokenId];
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::216 => uint16[16] memory tokenAttributes = attributes[_tokenId];
2022-09-nouns-builder/src/token/metadata/types/MetadataRendererTypesV1.sol::20 => uint16 referenceSlot;
2022-09-nouns-builder/src/token/types/TokenTypesV1.sol::20 => uint8 numFounders;
2022-09-nouns-builder/src/token/types/TokenTypesV1.sol::21 => uint8 totalOwnership;
2022-09-nouns-builder/src/token/types/TokenTypesV1.sol::30 => uint8 ownershipPct;
2022-09-nouns-builder/src/token/types/TokenTypesV1.sol::31 => uint32 vestExpiry;

[G-08] ++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-09-nouns-builder/src/governance/treasury/Treasury.sol::162 => for (uint256 i = 0; i < numTargets; ++i) {
2022-09-nouns-builder/src/lib/token/ERC721.sol::141 => ++balances[_to];
2022-09-nouns-builder/src/lib/token/ERC721.sol::199 => ++balances[_to];
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::207 => uint256 nCheckpoints = numCheckpoints[_from]++;
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::222 => uint256 nCheckpoints = numCheckpoints[_to]++;
2022-09-nouns-builder/src/token/Token.sol::80 => for (uint256 i; i < numFounders; ++i) {
2022-09-nouns-builder/src/token/Token.sol::91 => uint256 founderId = settings.numFounders++;
2022-09-nouns-builder/src/token/Token.sol::108 => for (uint256 j; j < founderPct; ++j) {
2022-09-nouns-builder/src/token/Token.sol::132 => while (tokenRecipient[_tokenId].wallet != address(0)) ++_tokenId;
2022-09-nouns-builder/src/token/Token.sol::154 => tokenId = settings.totalSupply++;
2022-09-nouns-builder/src/token/Token.sol::261 => for (uint256 i; i < numFounders; ++i) founders[i] = founder[i];
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::119 => for (uint256 i = 0; i < numNewProperties; ++i) {
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::133 => for (uint256 i = 0; i < numNewItems; ++i) {
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::189 => for (uint256 i = 0; i < numProperties; ++i) {
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::229 => for (uint256 i = 0; i < numProperties; ++i) {

[G-09] <x> += <y> costs more gas than <x> = <x> + <y> for state variables

use <x> = <x> + <y> or <x> = <x> - <y> instead to save gas

2022-09-nouns-builder/src/governance/governor/Governor.sol::280 => proposal.againstVotes += uint32(weight);
2022-09-nouns-builder/src/governance/governor/Governor.sol::285 => proposal.forVotes += uint32(weight);
2022-09-nouns-builder/src/governance/governor/Governor.sol::290 => proposal.abstainVotes += uint32(weight);
2022-09-nouns-builder/src/token/Token.sol::88 => if ((totalOwnership += uint8(founderPct)) > 100) revert INVALID_FOUNDER_OWNERSHIP();
2022-09-nouns-builder/src/token/Token.sol::118 => (baseTokenId += schedule) % 100;
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::140 => _propertyId += numStoredProperties;

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

use abi.encodePacked() where possible to save gas

2022-09-nouns-builder/src/governance/governor/Governor.sol::104 => return keccak256(abi.encode(_targets, _values, _calldatas, _descriptionHash));
2022-09-nouns-builder/src/governance/governor/Governor.sol::230 => keccak256(abi.encode(VOTE_TYPEHASH, _voter, _proposalId, _support, nonces[_voter]++, _deadline))
2022-09-nouns-builder/src/governance/treasury/Treasury.sol::107 => return keccak256(abi.encode(_targets, _values, _calldatas, _descriptionHash));
2022-09-nouns-builder/src/lib/utils/EIP712.sol::69 => return keccak256(abi.encode(DOMAIN_TYPEHASH, HASHED_NAME, HASHED_VERSION, block.chainid, address(this)));
2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::251 => return uint256(keccak256(abi.encode(_tokenId, blockhash(block.number), block.coinbase, block.timestamp)));

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

Use a solidity version of at least 0.8.2 to get compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings 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-09-nouns-builder/src/lib/interfaces/IEIP712.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IERC1967Upgrade.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IERC721.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IERC721Votes.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IInitializable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IOwnable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/interfaces/IPausable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/proxy/ERC1967Proxy.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/proxy/ERC1967Upgrade.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/proxy/UUPS.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/token/ERC721.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/Address.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/EIP712.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/Initializable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/Ownable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/Pausable.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/ReentrancyGuard.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/SafeCast.sol::2 => pragma solidity ^0.8.4;
2022-09-nouns-builder/src/lib/utils/TokenReceiver.sol::2 => pragma solidity ^0.8.0;

[G-12] 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-09-nouns-builder/src/lib/token/ERC721.sol::30 => mapping(address => uint256) internal balances;
2022-09-nouns-builder/src/lib/token/ERC721.sol::38 => mapping(address => mapping(address => bool)) internal operatorApprovals;
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::29 => mapping(address => address) internal delegation;
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::33 => mapping(address => uint256) internal numCheckpoints;
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::37 => mapping(address => mapping(uint256 => Checkpoint)) internal checkpoints;
2022-09-nouns-builder/src/manager/storage/ManagerStorageV1.sol::10 => mapping(address => mapping(address => bool)) internal isUpgrade;

[G-13] 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-09-nouns-builder/src/auction/Auction.sol::184 => if (_auction.highestBidder != address(0)) {
2022-09-nouns-builder/src/lib/token/ERC721.sol::194 => if (owners[_tokenId] != address(0)) revert ALREADY_MINTED();
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::205 => if (_from != address(0)) {
2022-09-nouns-builder/src/lib/token/ERC721Votes.sol::220 => if (_to != address(0)) {
2022-09-nouns-builder/src/token/Token.sol::132 => while (tokenRecipient[_tokenId].wallet != address(0)) ++_tokenId;

[G-14] Use selfbalance()

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

2022-09-nouns-builder/src/auction/Auction.sol::346 => if (address(this).balance < _amount) revert INSOLVENT();

[G-15] Using storage instead of memory for structs/arrays saves gas

When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read.

Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct

2022-09-nouns-builder/src/token/metadata/MetadataRenderer.sol::216 => uint16[16] memory tokenAttributes = attributes[_tokenId];

[G-16] 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-09-nouns-builder/src/token/Token.sol::71 => function _addFounders(IManager.FounderParams[] calldata _founders) internal {
2022-09-nouns-builder/src/token/Token.sol::130 => function _getNextTokenId(uint256 _tokenId) internal view returns (uint256) {