Platform: Code4rena
Start Date: 06/09/2022
Pot Size: $90,000 USDC
Total HM: 33
Participants: 168
Period: 9 days
Judge: GalloDaSballo
Total Solo HM: 10
Id: 157
League: ETH
Rank: 43/168
Findings: 2
Award: $402.55
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Lambda
Also found by: 0x1337, 0x1f8b, 0x4non, 0x85102, 0xA5DF, 0xNazgul, 0xSmartContract, 0xbepresent, 0xc0ffEE, 8olidity, Aymen0909, B2, Bnke0x0, CRYP70, Captainkay, CertoraInc, Ch_301, Chom, ChristianKuri, CodingNameKiki, Deivitto, Diana, DimitarDimitrov, ElKu, EthLedger, Franfran, Funen, GimelSec, JansenC, Jeiwan, Jujic, Lead_Belly, MEP, MasterCookie, MiloTruck, Noah3o6, PPrieditis, PaludoX0, Picodes, PwnPatrol, R2, Randyyy, RaymondFam, Respx, ReyAdmirado, Rolezn, Samatak, Tointer, Tomo, V_B, Waze, _Adam, __141345__, a12jmx, ak1, asutorufos, azephiar, ballx, bharg4v, bin2chen, bobirichman, brgltd, bulej93, c3phas, cccz, ch0bu, cloudjunky, cryptonue, cryptostellar5, cryptphi, csanuragjain, d3e4, datapunk, davidbrai, delfin454000, dharma09, dic0de, dipp, djxploit, eierina, erictee, fatherOfBlocks, gogo, hansfriese, hyh, imare, indijanc, izhuer, jonatascm, ladboy233, leosathya, lucacez, lukris02, m9800, martin, minhtrng, ne0n, neumo, oyc_109, p_crypt0, pashov, pauliax, pcarranzav, pedr02b2, peritoflores, pfapostol, rbserver, ret2basic, robee, rvierdiiev, sach1r0, sahar, scaraven, sikorico, simon135, slowmoses, sorrynotsorry, tnevler, tonisives, volky, yixxas, zkhorse, zzzitron
356.9288 USDC - $356.93
If the intention is for the Ether to be used, the function should call another function, otherwise it should revert
If the intention is for the Ether to be used, the function should call another function, otherwise it should revert.
Zero address should be checked for state variables, immutable variables. A zero address can lead into problems.
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L30-L32 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L32-L34 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/manager/Manager.sol#L62-L71 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/treasury/Treasury.sol#L31-L34 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L41-L43 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/auction/Auction.sol#L39-L42
Also this values should be checked even if they are not immutable, as they are not reassigned. https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L64-L66
Check zero address before assigning or using it
Risk of using block.timestamp for time should be considered.
block.timestamp is not an ideal proxy for time because of issues with synchronization, miner manipulation and changing block times.
This kind of issue may affect the code allowing or reverting the code before the expected deadline, modifying the normal functioning or reverting sometimes.
SWC ID: 116
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L116-L175 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L208-L242 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L353-L377 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L413-L456 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/treasury/Treasury.sol#L74-L78 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/treasury/Treasury.sol#L82-L84 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/treasury/Treasury.sol#L88-L90 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/auction/Auction.sol#L90-L154 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/auction/Auction.sol#L167-L201 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/auction/Auction.sol#L344-L365 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/token/ERC721Votes.sol#L59-L118 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/token/ERC721Votes.sol#L144-L174 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L177-L199
Consider using an oracle for precision Consider the risk of using block.timestamp as time proxy and evaluate if block numbers can be used as an approximation for the application logic. Both have risks that need to be factored in.
The initialize function that initializes important contract state can be called by anyone.
The attacker can initialize the contract before the legitimate deployer, hoping that the victim continues to use the same contract.
In the best case for the victim, they notice it and have to redeploy their contract costing gas.
In this case, deploy function what calls all initialize functions has no way of access control, leading to anybody being able to deploy before it's expected owners
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/manager/Manager.sol#L97-L102 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/manager/Manager.sol#L132-L144
Use the constructor to initialize non-proxied contracts.
For initializing proxy contracts deploy contracts using a factory contract that immediately calls initialize after deployment or make sure to call it immediately after deployment and verify the transaction succeeded.
If you are dealing with more than one dynamic data type, abi.encodePacked() can lead to collisions when used with a hash function.
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.
abi.encodePacked will only use the only use the minimal required memory to encode the data. E.g. an address will only use 20 bytes and for dynamic arrays only the elements will be stored without length. For more info see the Solidity docs for packed mode.
For the input of the keccak method it is important that you can ensure that the resulting bytes of the encoding are unique. So if you always encode the same types and arrays always have the same length then there is no problem. But if you switch the parameters that you encode or encode multiple dynamic arrays you might have conflicts.
Change abi.encodePacked to abi.encode when data collision may happen
Return values not being checked may lead into unexpected behaviors with functions. Not events/Error are being emitted if that fails, so functions would be called even of not being working as expect as for in _upgradeToAndCall
Check values and revert/emit events if needed
Magic numbers are hardcoded numbers used in the code which are ambiguous to their intended purpose. These should be replaced with constants to make code more readable and maintainable.
Values are hardcoded and would be more readable and maintainable if declared as a constant
100 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/auction/Auction.sol#L119 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L88 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L102 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L118 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L179 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L271
50000 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/auction/Auction.sol#L354 success := call(50000, _to, _amount, 0, 0, 0, 0)
10_000 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L468 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L475
16 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L179 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L197
Replace magic hardcoded numbers with declared constants.
Events without indexed event parameters make it harder and inefficient for off-chain tools to analyze them.
Indexed parameters (“topics”) are searchable event parameters. They are stored separately from unindexed event parameters in an efficient manner to allow for faster access. This is useful for efficient off-chain-analysis, but it is also more costly gas-wise.
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/auction/IAuction.sol#L22-L50 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/IGovernor.sol#L29-L57 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/treasury/ITreasury.sol#L14-L28 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/interfaces/IERC1967Upgrade.sol#L14 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/interfaces/IInitializable.sol#L13 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/interfaces/IPausable.sol#L14-L18 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/manager/IManager.sol#L20-L31 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/IToken.sol#L21 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/interfaces/IPropertyIPFSMetadataRenderer.sol#L15-L28
Consider which event parameters could be particularly useful to off-chain tools and should be indexed.
Only constants are suggested to use style CONSTANTS_WITH_UNDERSCORES, other variables are suggested to use camelCase
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/auction/Auction.sol#L28 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/EIP712.sol#L26-L35
Rename to camelCase
psuedo-random seed for a token id
Replace to pseudo
Code that is not used should be removed
Remove the code that is not used.
Name shadowing where two or more variables/functions share the same name could be confusing to developers and/or reviewers
Use of _owner as new value variable shadows Ownable _owner
Replace _owner variable in the function parameter to owner_, new_owner or a similar substitution
Rather than using abi.encodePacked for appending string, since version 0.8.12, string.concat() is enabled to be used instead of abi.encodePacked(<str>,<str>)
Code is expected to be deployed at 0.8.15 at least
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L208-L213 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L259 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L272-L280 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L290-L303 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L309
Consider changing to string.concat() for appending strings
Missing Natspec and regular comments affect readability and maintainability of a codebase.
Contracts has partial or full lack of comments
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/auction/Auction.sol#L276-L299 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L103 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L121 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L184-L345 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L413-L495 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L515-L556 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/treasury/Treasury.sol#L193-L202 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/treasury/Treasury.sol#L66-L116 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/proxy/ERC1967Proxy.sol#L30 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/proxy/ERC1967Upgrade.sol#L82-L85 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/proxy/UUPS.sol#L61-L63 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/token/ERC721.sol#L54 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/token/ERC721.sol#L61-L96 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/Address.sol#L25-L43 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/EIP712.sol#L55-L70 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/Ownable.sol#L51-L59 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/Pausable.sol#L43-L46 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/manager/Manager.sol#L97-L182 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L171-L339 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L76-L85 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L128-L162 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L177-L198 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L219-L296
https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/SafeCast.sol#L9-L49 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/TokenReceiver.sol#L6-L36
Names being so similar may lead to unexpected calls and assumptions.
https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/interfaces/IERC721Votes.sol#L53-L59 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/token/ERC721Votes.sol#L126-L135
Consider making more unique the names of this variables/functions. For example:
delegateVotes()Clearness of the code is important for the readability and maintainability.
receive and fallback functions are expected to be at the very end of the contract
Move the function to the very end
Some of the contracts include an unlocked pragma, e.g., pragma solidity >=0.8.4.
Locking the pragma helps ensure that contracts are not accidentally deployed using an old compiler version with unfixed bugs.
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/utils/TokenReceiver.sol#L2
pragma solidity ^0.8.0;
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/interfaces/IUUPS.sol#L2
pragma solidity ^0.8.15;
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/interfaces/IWETH.sol#L2
pragma solidity ^0.8.15;
And a lot of instances of:
pragma solidity ^0.8.4;
Lock pragmas to a specific Solidity version. Consider converting ^0.8.0 into 0.8.15 Consider converting ^0.8.4 into 0.8.15 Consider converting ^0.8.15 into 0.8.15
Long lines should be wrapped to conform with Solidity Style guidelines.
Lines that exceed the 79 (or 99) character length suggested by the Solidity Style guidelines. Reference: https://docs.soliditylang.org/en/v0.8.10/style-guide.html#maximum-line-length
Reduce line length to less than 99 at least to improve maintainability and readability of the code
Multiples of 10 can be declared as constants with scientific notation so it's easier to read them and less prone to miss/exceed a 0 of the expected value.
Values 100, 10_000 and 50000 can be used in scientific notation
100 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/auction/Auction.sol#L119 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L88 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L102 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L118 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L179 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L271
10_000 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L468 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L475
Replace hardcoded numbers with constants that represent the scientific corresponding notation
#0 - GalloDaSballo
2022-09-27T13:47:16Z
## address(0) L
L
R
L
3NC (pragma / typos / comments)
3L 1R 3NC
🌟 Selected for report: pfapostol
Also found by: 0x1f8b, 0x4non, 0x5rings, 0xA5DF, 0xSmartContract, 0xc0ffEE, 0xkatana, Aymen0909, Bnke0x0, CertoraInc, Chandr, CodingNameKiki, Cr4ckM3, Deivitto, DimSon, Franfran, JAGADESH, JC, Jeiwan, Lambda, LeoS, Matin, Metatron, Migue, MiloTruck, PPrieditis, PaludoX0, R2, RaymondFam, Respx, ReyAdmirado, Rolezn, Saintcode_, Samatak, SnowMan, StevenL, Tointer, TomJ, Tomo, WatchDogs, Waze, _Adam, __141345__, ajtra, asutorufos, ballx, brgltd, bulej93, c3phas, ch0bu, dharma09, djxploit, durianSausage, easy_peasy, fatherOfBlocks, gianganhnguyen, gogo, imare, leosathya, lucacez, martin, oyc_109, pauliax, peiw, prasantgupta52, ret2basic, rfa, robee, sikorico, simon135, tofunmi, volky, wagmi, zishansami
45.6207 USDC - $45.62
Constant expressions are left as expressions, not constants.
Reference to this kind of issue: https://github.com/ethereum/solidity/issues/9232
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L27 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/token/ERC721Votes.sol#L21 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/EIP712.sol#L19
Use immutable for this expressions
Functions should have the strictest visibility possible. Public functions may lead to more gas usage by forcing the copy of their parameters to memory from calldata.
If a function is never called from the contract it should be marked as external. This will save gas.
governance https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/treasury/Treasury.sol#L237-L244 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/treasury/Treasury.sol#L247-L255 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/treasury/Treasury.sol#L258-L266
lib https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/token/ERC721.sol#L54 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/token/ERC721.sol#L57 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/token/ERC721.sol#L91-L97 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/token/ERC721Votes.sol#L45-L54 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/token/ERC721Votes.sol#L59-L118 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/utils/Ownable.sol#L52-L54 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/utils/Ownable.sol#L57-L59 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/utils/Ownable.sol#L63-L67 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/utils/Ownable.sol#L71-L75 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/utils/Ownable.sol#L87-L91 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/utils/Ownable.sol#L78-L84 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/utils/EIP712.sol#L63-L65
Consider changing visibility from public to external.
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.
https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size than downcast where needed
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/utils/Initializable.sol#L17
uint8 internal _initialized;
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/utils/Initializable.sol#L17
uint8 internal _initialized;
Consider using some data type that uses 32 bytes, for example uint256
Booleans are more expensive than uint256 or any type that takes up a full word because each write operation emits an extra SLOAD to first read the slot's contents, replace the bits taken up by the boolean, and then write back. This is the compiler's defense against contract upgrades and pointer aliasing, and it cannot be disabled.
Here is one example of OpenZeppelin about this optimization https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past
https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/Pausable.sol#L15 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/Initializable.sol#L20
Consider using uint256 with values 0 and 1 rather than bool
All these variables could be combine in a Struct in order to reduce the gas cost.
As noticed in: https://gist.github.com/alexon1234/b101e3ac51bea3cbd9cf06f80eaa5bc2 When multiple mappings that access the same addresses, uints, etc, all of them can be mixed into an struct and then that data accessed like: mapping(datatype => newStructCreated) newStructMap; Also, you have this post where it explains the benefits of using Structs over mappings https://medium.com/@novablitz/storing-structs-is-costing-you-gas-774da988895e
https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/token/ERC721.sol#L26 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/token/ERC721.sol#L34
https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/token/ERC721.sol#L30 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/token/ERC721.sol#L38
Consider mixing different mappings into a struct when able in order to save 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
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L27
bytes32 public constant VOTE_TYPEHASH = keccak256("Vote(address voter,uint256 proposalId,uint256 support,uint256 nonce,uint256 deadline)");
Consider replacing public for private in constants for gas saving.
In for loops is not needed to initialize indexes to 0 as it is the default uint/int value. This saves gas.
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L119 for (uint256 i = 0; i < numNewProperties; ++i) {
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L133 for (uint256 i = 0; i < numNewItems; ++i) {
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L189 for (uint256 i = 0; i < numProperties; ++i) {
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L229 for (uint256 i = 0; i < numProperties; ++i) {
Don't initialize variables to default value
Variables read more than once improves gas usage when cached into local variable
In loops or state variables, this is even more gas saving
Cache variables used more than one into a local variable.
Shifting is cheaper than dividing by 2
A division by 2 can be calculated by shifting one to the right. 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.
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/token/ERC721Votes.sol#L95
middle = high - (high - low) / 2;
Consider replacing / 2 with >> 1 here
Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/token/ERC721.sol#L47-L50 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L130 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/Token.sol#L71 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/token/ERC721.sol#L211 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/Address.sol#L37 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/Address.sol#L46 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/Pausable.sol#L39 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/Pausable.sol#L56 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/SafeCast.sol#L45
Consider changing internal function only called once to inline code for gas savings
In general, abi.encodePacked is more gas-efficient.
Changing the abi.encode function to abi.encodePacked can save gas since the abi.encode function pads extra null bytes at the end of the call data, which is unnecessary. Also, in general, abi.encodePacked is more gas-efficient.
https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/utils/EIP712.sol#L69 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L104 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L230 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/treasury/Treasury.sol#L107 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/token/ERC721Votes.sol#L162 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/manager/Manager.sol#L68-L71 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L251
Consider changing abi.encode to abi.encodePacked
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
It's suggested to add payable to functions guaranteed to revert when called by normal users to improve gas costs
Strict inequalities ( > ) are more expensive than non-strict ones ( >= ). This is due to some supplementary checks (ISZERO, 3 gas)
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/proxy/ERC1967Upgrade.sol#L61 if (_data.length > 0 || _forceCall) {
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/token/ERC721Votes.sol#L203 if (_from != _to && _amount > 0) {
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/lib/utils/Address.sol#L50 if (_returndata.length > 0) {
Consider using >= 1 instead of > 0 to avoid some opcodes
x+=y costs more gas than x=x+y for state variables
https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L280 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L285 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L290
Don't use += for state variables as it cost more gas.
It is generally cheaper to load variables directly from calldata, rather than copying them to memory.
Only use memory if the variable needs to be modified
Contracts https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L116-L121 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/Governor.sol#L192-L198 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/proxy/UUPS.sol#L54-L58 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/MetadataRenderer.sol#L347-L367
Interfaces https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/IGovernor.sol#L134-L151 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/IGovernor.sol#L162-L166 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/governance/governor/IGovernor.sol#L195-L200 https://github.com/code-423n4/2022-09-nouns-builder/blob/c682e196982ca6953fe9fcb85df02748795394fd/src/lib/interfaces/IUUPS.sol#L35 https://github.com/code-423n4/2022-09-nouns-builder/blob/debe9b792cc70510eadf9b3728cde5b0f2ec9a1f/src/token/metadata/interfaces/IPropertyIPFSMetadataRenderer.sol#L74-L84
Use calldata rather than memory in external functions where the parameter is not modified but only read
#0 - GalloDaSballo
2022-09-26T15:23:39Z
80+% of this is just misguided
500 from the memory