Nouns Builder contest - CodingNameKiki's results

1. Bool success can be applied better in the execute() function.

It is better choice for the function to use return statement (bool success), instead of adding the bool variable out of nowhere: https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/Treasury.sol#L164

Recommend: Applying return statement on the fuction execute() and replacing the "bool success" in the fuction only with "success". https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/Treasury.sol#L141-L146

(Before) 141: function execute( address[] calldata _targets, uint256[] calldata _values, bytes[] calldata _calldatas, bytes32 _descriptionHash) external payable onlyOwner { (After) 141: function execute( address[] calldata _targets, uint256[] calldata _values, bytes[] calldata _calldatas, bytes32 _descriptionHash) external payable onlyOwner returns (bool success){

(Before) 164: (bool success, ) = _targets[i].call{ value: _values[i] }(_calldatas[i]); (After) 164: (success, ) = _targets[i].call{ value: _values[i] }(_calldatas[i]);

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/ITreasury.sol#L107-L112 (Before) 107: function execute( address[] calldata targets, uint256[] calldata values, bytes[] calldata calldatas, bytes32 descriptionHash) external payable; (After) 107: function execute( address[] calldata targets, uint256[] calldata values, bytes[] calldata calldatas, bytes32 descriptionHash) external payable returns (bool success);

2. Function Naming suggestions

Proper use of _ as a function name prefix A common pattern is to prefix internal and private function names with _. This pattern is correctly applied in the Nouns Builder contracts, however there are some inconsistencies in the libraries.

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/Address.sol#L25 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/Address.sol#L30 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/Address.sol#L37 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/Address.sol#L46 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/SafeCast.sol#L9 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/SafeCast.sol#L15 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/SafeCast.sol#L21 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/SafeCast.sol#L27 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/SafeCast.sol#L33 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/SafeCast.sol#L39 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/SafeCast.sol#L45

3. Payable is used unnecessarily on the constructor.

There is no reason to add payable to the constructor in these situations. The "payable" keyword is used, when you want the constructor to accept Ether on the deploying time. If for some reason Ether is sent to the contract on deploying time, the ether will be stuck in the contract without a way for retrieving it, because there isn't a single function for sending this particular Ether out of the contract.

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/MetadataRenderer.sol#L32 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L30 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L55 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L41 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/Treasury.sol#L32 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L39

4. Proper use of get as a function name prefix

Clear function names can increase readability. Follow a standard convertion function names such as using get for getter (view/pure) functions. Examples:

src/auction/Auction.sol (Before) 277: function treasury() external view returns (address) { (After) 277: function getTreasuryAddress() external view returns (address) { (Before) 282: function duration() external view returns (uint256) { (After) 282: function getAuctionDuration() external view returns (uint256) { (Before) 287: function reservePrice() external view returns (uint256) { (After) 287: function getAuctionReservePrice() external view returns (uint256) { (Before) 292: function timeBuffer() external view returns (uint256) { (After) 292: function getMinimumTimeBuffer() external view returns (uint256) {

5. Consider addings checks for signature malleability

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

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L236

6. Expressions for constant values such as a call to keccak256() is prefered to use immutable rather than constant.

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L27 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/token/ERC721Votes.sol#L21 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/EIP712.sol#L19

7. Floating pragma version in the libraries

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

All contracts in the library section - https://github.com/code-423n4/2022-09-nouns-builder/tree/main/src/lib

8. Immutable addresses lack zero-address check.

Constructors should check the address written in an immutable address variable is not the zero address

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L40 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L41 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L42 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/Treasury.sol#L33 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L62 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L63 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L64 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L65 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L66 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L31 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/MetadataRenderer.sol#L33

9. _safeMint() should be used rather than _mint, whenever possible.

_mint() is discouraged in favor of _safeMint() which ensures that the recipient is either an EOA or implements IERC721Receiver. Both OpenZeppelin and solmate have versions of this function

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L161 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L169 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L188

10. Several critical operations do not trigger events, which will make it difficult to review the correct behavior of the contracts once deployed. In this case the owner upgrades the contract, whithout using event.

Auction Upgrade - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L374 Governor Upgrade - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L618 Treasury Upgrade - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/Treasury.sol#L278 Manager Upgrade -https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L209 Token Upgrade - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L305 Metadata Upgrade - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/MetadataRenderer.sol#L376

11. Use two-phase ownership transfers

Consider adding a two-phase transfer, where the current owner nominates the next owner, and the next owner has to call accept*() to become the new owner. This prevents passing the ownership to an account that is unable to use it.

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L30 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/MetadataRenderer.sol#L32 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L41 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/Treasury.sol#L32 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L39

12. Override function arguments that are unused should have the variable name removed or commented out to avoid compiler warnings

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L374 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L618 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/Treasury.sol#L278 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L209 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L305 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/MetadataRenderer.sol#L376

13. Constant redefined elsewhere

Consider defining in only one contract so that values cannot become out of sync when only one location is updated. A cheap way to store constants in a single location is to create an internal constant in a library. If the variable is a local cache of another contract's value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don't get out of sync.

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L25 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L28 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L31 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L34 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L37

14. return value of transfer() not checked

Not all IERC20 implementations revert() when there’s a failure in transfer(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually making a payment

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L363

15. Event indexing

Events should use indexed fields

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/IAuction.sol#L22 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/IAuction.sol#L28 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/IAuction.sol#L34 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/IAuction.sol#L38 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/IAuction.sol#L42 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/IAuction.sol#L46 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/IAuction.sol#L50 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/IGovernor.sol#L18 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/IGovernor.sol#L29 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/IGovernor.sol#L33 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/IGovernor.sol#L36 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/IGovernor.sol#L39 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/IGovernor.sol#L42 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/IGovernor.sol#L45 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/IGovernor.sol#L48 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/IGovernor.sol#L51 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/IGovernor.sol#L54 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/IGovernor.sol#L57 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/ITreasury.sol#L16 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/ITreasury.sol#L19 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/ITreasury.sol#L22 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/ITreasury.sol#L25 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/ITreasury.sol#L28 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/IManager.sol#L21 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/IManager.sol#L26 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/IManager.sol#L31 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/IToken.sol#L21 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/interfaces/IPropertyIPFSMetadataRenderer.sol#L16 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/interfaces/IPropertyIPFSMetadataRenderer.sol#L19 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/interfaces/IPropertyIPFSMetadataRenderer.sol#L22 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/interfaces/IPropertyIPFSMetadataRenderer.sol#L25 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/interfaces/IPropertyIPFSMetadataRenderer.sol#L28

16. Unused empty receive()/fallback() function

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth)))

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/Treasury.sol#L269

17. 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.

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L226 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L68 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L69 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L70 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/manager/Manager.sol#L71

18. Constants should be defined rather than using magic numbers

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L81 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L119 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L80

19. Low level calls don't check for contract existence

Low level calls return success if called on a destructed contract. See OpenZeppelin’s Address.so which checks address.code.length

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/Treasury.sol#L164

20. Auction do not implent ERC721TOKENRECEIVER

Auction.sol is specifically involve in ERC721 tokens. Therefore the contract should implement ERC721TokenReceiver, or customer transfers involving transferFrom() calls will revert

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L22

21. State/flags should use enums rather than separate constants.

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/ReentrancyGuard.sol#L14 https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/lib/utils/ReentrancyGuard.sol#L17

1. Storing "setting.timebuffer" into uint cache in the function createBid() can save gas.

In the following function, setting.timebuffer should be cached as it is read several times.

Create a cache: uint256 cache = setting.timebuffer and replace them. src/auction/Auction.sol (Before) 141: extend = (_auction.endTime - block.timestamp) < settings.timeBuffer; (After) 141: extend = (_auction.endTime - block.timestamp) < cache;

(Before) 149: auction.endTime = uint40(block.timestamp + settings.timeBuffer); (After) 149: auction.endTime = uint40(block.timestamp + cache);

2. Storing "auction.settled" into bool cache in the function _settleAuction() can save gas.

In the following function, auction.settled should be cached as it is read several times

Create a cache: bool settled = auction.settled and replace them. src/auction/Auction.sol (Before) 172: if (auction.settled) revert AUCTION_SETTLED(); (After) 172: if (settled) revert AUCTION_SETTLED();

(Before) 181: auction.settled = true; (After) 181: settled = true;

3. Two functions in the Auction contract use a memory location to retrieve auction

Using storage pointer instead of memory location can save a decent amount of gas.

a) The first function is createBid(), make sure to declare the memory location to a storage one

src/auction/Auction.sol (Before) 92: Auction memory _auction = auction; (After) 92: Auction storage _auction = auction;

And replacing the storage cache with these three saves extra gas. (Before) 130: auction.highestBid = msg.value; (After) 130: _auction.highestBid = msg.value;

(Before) 133: auction.highestBidder = msg.sender; (After) 133: _auction.highestBidder = msg.sender;

(Before) 149: auction.endTime = uint40(block.timestamp + settings.timeBuffer); (After) 149: _auction.endTime = uint40(block.timestamp + settings.timeBuffer);

b) The second function is _settleAuction(), make sure to declared the memory location to a storage one

src/auction/Auction.sol (Before) 169: Auction memory _auction = auction; (After) 169: Auction storage _auction = auction;

And replacing the storage cache with these two saves extra gas. (Before) 172: if (auction.settled) revert AUCTION_SETTLED(); (After) 172: if (_auction.settled) revert AUCTION_SETTLED();

(Before) 181: auction.settled = true; (After) 181: _auction.settled = true;

There is no need for boolean comprasions, so l am going to delete the "true". (Before) 181: _auction.settled = true; (After) 181: !_auction.settled;

// Note that the b) method can't be used at the same time with the "2. Storing "auction.settled" into bool cache ", this here saves a lot more gas than the bool cache.

4. l am going to do few changes in these functions. And l think it's better to form them into one report here:

-In the function cancle() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L353

a) l am going to replace the memory location with storage: (Before) 358: Proposal memory proposal = proposals[_proposalId]; (After) 358: Proposal storage proposal = proposals[_proposalId];

b) After that l will replace the "proposals[_proposalId].canceled" with the storage variable "proposal", instead of fetching the reference in a map or an array: (Before) 368: proposals[_proposalId].canceled = true; (After) 368: proposal.canceled = true;

d) And finally there is no need for boolean comprasions, so l am going to delete the "true". (Before) 368: proposal.canceled = true; (After) 368: !proposal.canceled;

-In the function veto() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L385

a) same as the last function, the boolean comprasions is no needed and therefore we will delete the "true" to save gas. (Before) 396: proposal.vetoed = true; (After) 396: !proposal.vetoed;

-In the function state() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L413

a) going to replace the memory location with storage. (Before) 415: Proposal memory proposal = proposals[_proposalId]; (After) 415: Proposal storage proposal = proposals[_proposalId];

-In the function execute() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L324

a) same as the last function, the boolean comprasions is no needed and therefore we will delete the "true" to save gas. (Before) 337: proposals[proposalId].executed = true; (After) 337: !proposals[proposalId].executed;

-In the function _castVote() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L248

a) ">=" cost less gas than ">" The compiler uses opcodes GT and ISZERO for solidity code that uses >, but only requires LT for >=, which saves gas. (Before) 261: if (_support > 2) revert INVALID_VOTE(); (After) 261: if (_support >= 3) revert INVALID_VOTE();

b) same as the last function, the boolean comprasions is no needed and therefore we will delete the "true" to save gas. (Before) 264: hasVoted[_proposalId][_voter] = true; (After) 264: !hasVoted[_proposalId][_voter];

-In the function propose() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/Governor.sol#L116

a) l am going to create two caches for the arrays "_value.length" and "_calldata.length" (Add) 133: + uint256 numValue = _values.length; (Add) 134: + uint256 numCalldatas = _calldatas.length; (Before) 138: if (numTargets != _values.length) revert PROPOSAL_LENGTH_MISMATCH(); (After) 138: if (numTargets != numValue) revert PROPOSAL_LENGTH_MISMATCH(); (Before) 139: if (numTargets != _calldatas.length) revert PROPOSAL_LENGTH_MISMATCH(); (After) 139: if (numTargets != numCalldatas) revert PROPOSAL_LENGTH_MISMATCH();

-In the function queue() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/Treasury.sol#L116

a) l am going to create an uint cache, which will hold timestamps[_proposalId]. (Add) 126: + uint _timestamps = timestamps[_proposalId]; (Before) 127: timestamps[_proposalId] = eta; (After) 127: _timestamps = eta;

-In the function execute() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/Treasury.sol#L141

a) l am going to create and uint cache, which will hold timestamps[proposalId] (Add) 153: + uint _timestamps = timestamps[proposalId]; (Before) 154: delete timestamps[proposalId]; (After) 154: delete _timestamps;

b) setting variables as their default values is unnecessary and wastes gas. (Before) 162: for (uint256 i = 0; i < numTargets; ++i) { (After) 162: for (uint256 i; i < numTargets; ++i) {

-In the function cancel() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/Treasury.sol#L180

a) l am going to create and uint cache, which will hold timestamps[proposalId] (Add) 184: + uint _timestamps = timestamps[_proposalId]; (Before) 185: delete timestamps[_proposalId]; (After) 185: delete _timestamps;

-In the function _addFounders() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L71

a) ">=" cost less gas than ">" (Before) 88: if ((totalOwnership += uint8(founderPct)) > 100) revert INVALID_FOUNDER_OWNERSHIP(); (After) 88: if ((totalOwnership += uint8(founderPct)) >= 101) revert INVALID_FOUNDER_OWNERSHIP();

b) l am going to create a storage for the struct "Founder", which will replace "tokenRecipient[baseTokenId]". (Add) 112: + Founder storage _tokenRecipient = tokenRecipient[baseTokenId]; (Before) 113: tokenRecipient[baseTokenId] = newFounder; (After) 113: _tokenRecipient = newFounder;

-In the function _getNextTokenId() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L130

a) l am going to create a storage for the mapping "Founder", which will replace "tokenRecipient[_tokenId]", instead of repeatedly fetching the reference in a map or an array. (Add) 131: +Founder storage _tokenRecipient = tokenRecipient[_tokenId]; (Before) 132: while (tokenRecipient[_tokenId].wallet != address(0)) ++_tokenId; (After) 132: while (_tokenRecipient.wallet != address(0)) ++_tokenId;

-In the function _isForFounder() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L177

a) l am going to create a storage for the mapping "Founder", which will replace "tokenRecipient[baseTokenId]", instead of repeatedly fetching the reference in a map or an array. (Add) 181: + Founder storage _tokenRecipient = tokenRecipient[baseTokenId]; (Before) 182: if (tokenRecipient[baseTokenId].wallet == address(0)) { (After) 182: if (_tokenRecipient.wallet == address(0)) { (Before) 186: } else if (block.timestamp < tokenRecipient[baseTokenId].vestExpiry) { (After) 186: } else if (block.timestamp < _tokenRecipient.vestExpiry) { (Before) 188: _mint(tokenRecipient[baseTokenId].wallet, _tokenId); (After) 188: _mint(_tokenRecipient.wallet, _tokenId);

-In the function addProperties() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/MetadataRenderer.sol#L91

a) No need to explicitly initialize variables with default values: (Before) 119: for (uint256 i = 0; i < numNewProperties; ++i) { (After) 119: for (uint256 i; i < numNewProperties; ++i) { (Before) 133: for (uint256 i = 0; i < numNewItems; ++i) { (After) 133: for (uint256 i; i < numNewItems; ++i) {

-In the function onMinted() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/MetadataRenderer.sol#L171

a) No need to explicitly initialize variables with default values: (Before) 189: for (uint256 i = 0; i < numProperties; ++i) { (After) 189: for (uint256 i; i < numProperties; ++i) {

b) creating a uint cache, which will hold "tokenAttributes[i + 1]" (Add) 193: + uint256 _tokenAttributes = tokenAttributes[i + 1]; (Before) 194: tokenAttributes[i + 1] = uint16(seed % numItems); (After) 194: _tokenAttributes = uint16(seed % numItems);

-In the function getAttributes() - https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/MetadataRenderer.sol#L206

a) Using storage pointer instead of memory location can save a decent amount of gas. (Before) 216: uint16[16] memory tokenAttributes = attributes[_tokenId]; (After) 216: uint16[16] storage tokenAttributes = attributes[_tokenId]; (Before) 234: Property memory property = properties[i]; (After) 234: Property storage property = properties[i]; (Before) 240: Item memory item = property.items[attribute]; (After) 240: Item storage item = property.items[attribute];

b) No need to explicitly initialize variables with default values: (Before) 229: for (uint256 i = 0; i < numProperties; ++i) { (After) 229: for (uint256 i; i < numProperties; ++i) {