QA Report

✅ N-1: Consider addings checks for signature malleability

📝 Description

You should consider addings checks for signature malleability

💡 Recommendation

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

🔍 Findings:

2022-09-nouns-builder/blob/main/src/governance/governor/Governor.sol#L236 address recoveredAddress = ecrecover(digest, _v, _r, _s);

2022-09-nouns-builder/blob/main/src/lib/token/ERC721Votes.sol#L167 address recoveredAddress = ecrecover(digest, _v, _r, _s);

✅ N-2: Expressions for constant values such as a call to `keccak256()`, should use `immutable` rather than `constant`

📝 Description

Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

💡 Recommendation

You should use immutable instead of constant

🔍 Findings:

2022-09-nouns-builder/blob/main/src/governance/governor/Governor.sol#L27 bytes32 public constant VOTE_TYPEHASH = keccak256("Vote(address voter,uint256 proposalId,uint256 support,uint256 nonce,uint256 deadline)");

2022-09-nouns-builder/blob/main/src/lib/token/ERC721Votes.sol#L21 bytes32 internal constant DELEGATION_TYPEHASH = keccak256("Delegation(address from,address to,uint256 nonce,uint256 deadline)");

2022-09-nouns-builder/blob/main/src/lib/utils/EIP712.sol#L19 bytes32 internal constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");

✅ N-3: No use of two-phase ownership transfers

📝 Description

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.

💡 Recommendation

Consider implementing a two step process where the admin nominates an account and the nominated account needs to call an acceptOwnership() function for the transfer of admin to fully succeed. This ensures the nominated EOA account is a valid and active account.

🔍 Findings:

2022-09-nouns-builder/blob/main/src/auction/Auction.sol#L5 import { Ownable } from "../lib/utils/Ownable.sol";

2022-09-nouns-builder/blob/main/src/auction/Auction.sol#L22 contract Auction is IAuction, UUPS, Ownable, ReentrancyGuard, Pausable, AuctionStorageV1 {

2022-09-nouns-builder/blob/main/src/governance/governor/Governor.sol#L5 import { Ownable } from "../../lib/utils/Ownable.sol";

2022-09-nouns-builder/blob/main/src/governance/governor/Governor.sol#L21 contract Governor is IGovernor, UUPS, Ownable, EIP712, GovernorStorageV1 {

2022-09-nouns-builder/blob/main/src/governance/treasury/Treasury.sol#L19 contract Treasury is ITreasury, UUPS, Ownable, TreasuryStorageV1 {

2022-09-nouns-builder/blob/main/src/manager/Manager.sol#L19 contract Manager is IManager, UUPS, Ownable, ManagerStorageV1 {

2022-09-nouns-builder/blob/main/src/token/metadata/MetadataRenderer.sol#L19 contract MetadataRenderer is IPropertyIPFSMetadataRenderer, UUPS, Ownable, MetadataRendererStorageV1 {

✅ N-4: Use a more recent version of solidity

📝 Description

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

💡 Recommendation

Use more recent version of solidity.

🔍 Findings:

2022-09-nouns-builder/blob/main/src/governance/treasury/types/TreasuryTypesV1.sol#L2 pragma solidity 0.8.15;

2022-09-nouns-builder/blob/main/src/lib/interfaces/IEIP712.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/interfaces/IERC1967Upgrade.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/interfaces/IERC721.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/interfaces/IERC721Votes.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/interfaces/IInitializable.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/interfaces/IOwnable.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/interfaces/IPausable.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/proxy/ERC1967Proxy.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/proxy/ERC1967Upgrade.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/proxy/UUPS.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/token/ERC721.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/token/ERC721Votes.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/utils/Address.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/utils/EIP712.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/utils/Initializable.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/utils/Ownable.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/utils/Pausable.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/utils/ReentrancyGuard.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/utils/SafeCast.sol#L2 pragma solidity ^0.8.4;

2022-09-nouns-builder/blob/main/src/lib/utils/TokenReceiver.sol#L2 pragma solidity ^0.8.0;

✅ N-5: Use `string.concat()` or`bytes.concat()`

📝 Description

Solidity version 0.8.4 introduces bytes.concat() (vs abi.encodePacked(<bytes>,<bytes>))Solidity version 0.8.12 introduces string.concat()(vs abi.encodePacked(<str>,<str>))