Platform: Code4rena
Start Date: 22/08/2022
Pot Size: $50,000 USDC
Total HM: 4
Participants: 160
Period: 5 days
Judge: gzeon
Total Solo HM: 2
Id: 155
League: ETH
Rank: 122/160
Findings: 1
Award: $16.97
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: IllIllI
Also found by: 0x040, 0x1f8b, 0xDjango, 0xNazgul, 0xNineDec, 0xSmartContract, 0xbepresent, 0xc0ffEE, 0xkatana, 2997ms, ACai, Amithuddar, Aymen0909, Ben, BipinSah, Bjorn_bug, Bnke0x0, CertoraInc, Ch_301, Chom, CodingNameKiki, Deivitto, DevABDee, DimitarDimitrov, Diraco, Dravee, ElKu, EthLedger, Fitraldys, Funen, GalloDaSballo, GimelSec, Guardian, IgnacioB, JC, JohnSmith, Junnon, KIntern_NA, Lambda, LeoS, Noah3o6, Olivierdem, Polandia94, R2, Randyyy, RaymondFam, Respx, ReyAdmirado, Rohan16, RoiEvenHaim, Rolezn, Ruhum, SaharAP, Saintcode_, SerMyVillage, Shishigami, Sm4rty, SooYa, TomJ, Tomio, Tomo, Waze, Yiko, _Adam, __141345__, a12jmx, ajtra, ak1, bobirichman, brgltd, bulej93, c3phas, cRat1st0s, carlitox477, catchup, ch0bu, d3e4, delfin454000, djxploit, durianSausage, erictee, exolorkistis, fatherOfBlocks, francoHacker, gogo, hyh, ignacio, jag, joestakey, karanctf, ladboy233, lucacez, lukris02, m_Rassska, martin, medikko, mics, mrpathfindr, natzuu, newfork01, oyc_109, pauliax, peritoflores, pfapostol, prasantgupta52, rbserver, ret2basic, rfa, robee, rokinot, rotcivegaf, rvierdiiev, sach1r0, saian, samruna, seyni, shark, shr1ftyy, sikorico, simon135, sryysryy, tay054, tnevler, wagmi, zishansami
16.9732 USDC - $16.97
memory instead of storage (80 ~ 100 gas per instance)using memory mark for read or write data in blockchain can be save way a lot of gas instead of using storage. according to <a href="https://eips.ethereum.org/EIPS/eip-2929">EIP-2929</a> gas cost for SLOAD(read from storage) and SSTORE(write into storage) is 100 gas (2600 gas if never accessed before), it's very contrast with MLOAD (read from memory) and MSTORE(write into memory) which only cost 3 gas.
SOLUTION:
memory variable and pass the value from storagememory variablestorageExample: The existing code from Nouns DAO
Proposal storage newProposal = proposals[proposalCount]; //SLOAD 100 gas // for every line below cost 100 gas for SSTORE newProposal.id = proposalCount; newProposal.proposer = msg.sender; newProposal.proposalThreshold = temp.proposalThreshold; newProposal.quorumVotes = bps2Uint(quorumVotesBPS, temp.totalSupply); newProposal.eta = 0; newProposal.targets = targets; newProposal.values = values; newProposal.signatures = signatures; newProposal.calldatas = calldatas; newProposal.startBlock = temp.startBlock; newProposal.endBlock = temp.endBlock; newProposal.forVotes = 0; newProposal.againstVotes = 0; newProposal.abstainVotes = 0; newProposal.canceled = false; newProposal.executed = false; newProposal.vetoed = false; //Total 1400 gas + 100 gas = 1500 gas
The Cheaper Version
Proposal memory newProposal ; //SSTORE 3 gas //for every SSTORE below cost 3 gas newProposal.id = proposalCount; newProposal.proposer = msg.sender; newProposal.proposalThreshold = temp.proposalThreshold; newProposal.quorumVotes = bps2Uint(quorumVotesBPS, temp.totalSupply); newProposal.eta = 0; newProposal.targets = targets; newProposal.values = values; newProposal.signatures = signatures; newProposal.calldatas = calldatas; newProposal.startBlock = temp.startBlock; newProposal.endBlock = temp.endBlock; newProposal.forVotes = 0; newProposal.againstVotes = 0; newProposal.abstainVotes = 0; newProposal.canceled = false; newProposal.executed = false; newProposal.vetoed = false; // subtotal 3*14 = 42 Gas proposals[proposalCount] = newProposal; //SSTORE 100 Gas & MLOAD 3 gas // TOTAL = 3 + 42 + 103 = 148 gas
from example above we can decrease cost from 1500 gas to 148 gas by using memory instead of storage and there's even more in contracts in scope
NOTE!!:
external view function will not affected by this gas optimizing but i will still report it with mark EV.INSTANCE:
a. NounsDAOLogicV1.sol
EV)
https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/governance/NounsDAOLogicV1.sol#L402-403b. NounsDAOLogicV2.sol
EV)
https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/governance/NounsDAOLogicV2.sol#L413-L414requiresave strings for require can be costly if the string have more than 32 characters (1 char = 1 bytes). Since ethereum storage slot use 32 bytes, strings that use more than 32 characters will use more than 1 storage slot. it will be costly for deploying and operating the contract.
SOLUTION: i recommend 2 solutions for optimizing gas cost you can choose one:
revert and custom errors.
example//defining error OnlyAdmin(); //useage function example() external { if (msg.sender != admin) revert OnlyAdmin(); ... //implementation }
//existing in NounsDAOLogicV1 require(address(timelock) == address(0), 'NounsDAO::initialize: can only initialize once') //46 bytes //after optimizing require(address(timelock) == address(0), 'NounsDAO::initialize: only once') //31 bytes
NOTE: theres several instance that using other function to use require but it's still defining string that have more than 32 bytes so its can be considered to. (example using safe32())
INSTANCE: a. NounsDAOLogicV1.sol
b. NounsDAOLogicV2.sol
safe32()) Line 923
https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/governance/NounsDAOLogicV2.sol#L923safe32()) Line 965
https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/governance/NounsDAOLogicV2.sol#L965c. NounsDAOProxy.sol
d. ERC721Checkpointable.sol
safe32()) Line 238-241
https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/base/ERC721Checkpointable.sol#L238-L241safe96()) Line 80
https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/base/ERC721Checkpointable.sol#L80sub96() and add96())
https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/base/ERC721Checkpointable.sol#L219
https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/base/ERC721Checkpointable.sol#L226Loop is a gas costly operation, it can be very expensive in term of gas useage to use Loop in a contract. but loop still can be a option to operate something with only one transaction, with some optimization. for optimizing loop we need to do this step below:
i, integer have default value 0.
example :// expensive version uint i = 0; // cheaper version uint i;
i, for most case it's a length (example: array.length). Instead pass it to new variable first and use that variable to compare i;
example:// expesinve version for (uint i = 0, i < array.length, i++) {}; //cheaper version uint length = array.length; for (uint i, i < length , i++) {};
++i instead of i++ and consider using unchecked for it (don't worry i will never overflowing since it's start by 0 to some value and for most case gas will run out first instead of i being more than 32 bytes uint)
Example ://expensive version for(uint i = 0, i < array.length, i++) {}; //cheaper version uint length = array.length; for(uint i, i < length) { ...//implementation of loop unchecked{ //at the end of loop ++i; } }
NOTE: for informational only some instance may be a 'external view' function which not effected with this kind of gas optimization. for external view function i will still report it with EV mark.
INSTANCE: a. NounsDAOLogicV1.sol
b. NounsDAOLogicV2.sol
Like in the loop section, avoiding default value passing can be optimize. I will make this optimization only difference instance than the loop.
INSTANCE:
Proposal storage newProposal = _proposals[proposalCount]; newProposal.id = proposalCount; newProposal.proposer = msg.sender; newProposal.proposalThreshold = temp.proposalThreshold; newProposal.eta = 0; //default newProposal.targets = targets; newProposal.values = values; newProposal.signatures = signatures; newProposal.calldatas = calldatas; newProposal.startBlock = temp.startBlock; newProposal.endBlock = temp.endBlock; newProposal.forVotes = 0;//default newProposal.againstVotes = 0;//default newProposal.abstainVotes = 0;//default newProposal.canceled = false;//default newProposal.executed = false;//default newProposal.vetoed = false;//default newProposal.totalSupply = temp.totalSupply; newProposal.creationBlock = block.number;
https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/governance/NounsDAOLogicV2.sol#L227-L245 https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/governance/NounsDAOLogicV1.sol#L217-L235
SOLUTION: Leave it empty like this
//expensive uint a = 0 bool b = false //cheaper uint a bool b
in above case it can be deleted from the line instead, Like this:
newProposal.id = proposalCount; newProposal.proposer = msg.sender; newProposal.proposalThreshold = temp.proposalThreshold; newProposal.targets = targets; newProposal.values = values; newProposal.signatures = signatures; newProposal.calldatas = calldatas; newProposal.startBlock = temp.startBlock; newProposal.endBlock = temp.endBlock; newProposal.totalSupply = temp.totalSupply; newProposal.creationBlock = block.number;
constructor or initialize can be deleted and move the logic inside the contructor or initializedefining new function is cost gas. if the function is internal but only called once in constructor or initialize, we can get rid the function and move the function logic inside contructor or initialize.
INSTANCE:
NounsDAOProxy::delegateTo()
Only called here: (constructor)
constructor( address timelock_, address nouns_, address vetoer_, address admin_, address implementation_, uint256 votingPeriod_, uint256 votingDelay_, uint256 proposalThresholdBPS_, uint256 quorumVotesBPS_ ) { // Admin set to msg.sender for initialization admin = msg.sender; delegateTo( //@audit only called here!! implementation_, abi.encodeWithSignature( 'initialize(address,address,address,uint256,uint256,uint256,uint256)', timelock_, nouns_, vetoer_, votingPeriod_, votingDelay_, proposalThresholdBPS_, quorumVotesBPS_ ) ); _setImplementation(implementation_); admin = admin_; }