Platform: Code4rena
Start Date: 12/08/2022
Pot Size: $35,000 USDC
Total HM: 10
Participants: 126
Period: 3 days
Judge: Justin Goro
Total Solo HM: 3
Id: 154
League: ETH
Rank: 21/126
Findings: 3
Award: $358.86
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: CertoraInc
Also found by: 0x1f8b, 0xSky, CodingNameKiki, DecorativePineapple, Noah3o6, Waze, jonatascm, oyc_109, pedr02b2, peritoflores
https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L426 https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L486 https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L546 https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L657
Its a good to add require() statement to checks the return value of token transfer or using safetransfer or safetransferFrom on Openzeppelin to ensure the token revert when transfer failure. Failure to do so will cause silent failures of transfer and affect token accountng in contract.
VSCode
consider using safetransfer/safetransferFrom or require() consistently
#0 - lacoop6tu
2022-08-16T14:19:17Z
Duplicate of #231
🌟 Selected for report: oyc_109
Also found by: 0x1f8b, 0x52, 0xDjango, 0xLovesleep, 0xNazgul, 0xNineDec, 0xbepresent, 0xmatt, 0xsolstars, Aymen0909, Bahurum, Bnke0x0, CertoraInc, Chom, CodingNameKiki, DecorativePineapple, Deivitto, Dravee, ElKu, Funen, GalloDaSballo, IllIllI, JC, JohnSmith, Junnon, KIntern_NA, Lambda, LeoS, MiloTruck, Noah3o6, PaludoX0, RedOneN, Respx, ReyAdmirado, Rohan16, RoiEvenHaim, Rolezn, Ruhum, Sm4rty, TomJ, Vexjon, Waze, Yiko, __141345__, a12jmx, ajtra, ak1, apostle0x01, asutorufos, auditor0517, bin2chen, bobirichman, brgltd, bulej93, byndooa, c3phas, cRat1st0s, cryptphi, csanuragjain, d3e4, defsec, delfin454000, djxploit, durianSausage, ellahi, erictee, exd0tpy, fatherOfBlocks, gogo, jonatascm, ladboy233, medikko, mics, natzuu, neumo, p_crypt0, paribus, pfapostol, rbserver, reassor, ret2basic, robee, rokinot, rvierdiiev, sach1r0, saneryee, seyni, sikorico, simon135, sseefried, wagmi, wastewa
29.892 USDC - $29.89
#1 Manager and ve must be immutable
the state manager and ve can't be initialize by constructor. the constructor parameter mention state manager and ve to initialize. so i suggest to add immutable on manager and ve.
#2 Token, owner and penaltyRecipient must be immutable
the state Token, owner and penaltyRecipient can't be initialize by constructor. the constructor parameter mention state Token, owner and penaltyRecipient to initialize. so i suggest to add immutable on Token, owner and penaltyRecipient.
#3 Missing natspec comment
isContract() was missing natspec comment. add natspec comment to isContract() to give knowledge to the user about the function and params
#4 Missing indexed field
Each event should use indexed fields to reach clarity. add indexed in owner, blocklist, and recipient.
#5 Missing check for address
constructor have three params address, so to avoid vulnerability we suggest to add simple check for the params Add simple check e.g
require( _owner != address(0), "address cant be 0"); require( _blocklist != address(0), "address cant be 0"); require( _recipient != address(0), "address cant be 0");
🌟 Selected for report: IllIllI
Also found by: 0x040, 0x1f8b, 0xDjango, 0xHarry, 0xLovesleep, 0xNazgul, 0xNineDec, 0xSmartContract, 0xackermann, 0xbepresent, 2997ms, Amithuddar, Aymen0909, Bnke0x0, CRYP70, CertoraInc, Chom, CodingNameKiki, Deivitto, Dravee, ElKu, Fitraldys, Funen, GalloDaSballo, JC, JohnSmith, Junnon, LeoS, Metatron, MiloTruck, Noah3o6, NoamYakov, PaludoX0, RedOneN, Respx, ReyAdmirado, Rohan16, Rolezn, Ruhum, Sm4rty, SooYa, SpaceCake, TomJ, Tomio, Waze, Yiko, __141345__, a12jmx, ajtra, ak1, apostle0x01, asutorufos, bobirichman, brgltd, bulej93, c3phas, cRat1st0s, carlitox477, chrisdior4, csanuragjain, d3e4, defsec, delfin454000, djxploit, durianSausage, ellahi, erictee, fatherOfBlocks, gerdusx, gogo, ignacio, jag, ladboy233, m_Rassska, medikko, mics, natzuu, newfork01, oyc_109, paribus, pfapostol, rbserver, reassor, ret2basic, robee, rokinot, rvierdiiev, sach1r0, saian, sashik_eth, sikorico, simon135
14.9497 USDC - $14.95
#1 Visibility
Reducing from public to private or internal can save gas when a constant isn’t used outside of its contract. I suggest changing the visibility from public to internal or private.
#2 Use Storage instead memory
Use storage instead of memory to reduce the gas fee. i suggest to change this.
#3 oldLocked should get cached
cache the oldLocked to the local because use multiple times for saving the gas fee. because mload is cheaper than sload.
#4 lastPoint should be cached
cache the lastPoint to the local because use multiple times for saving the gas fee. because mload is cheaper than sload.
#5 Use ++epoch for increment
https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L340 change this code to reduce the gas fee
epoch = epoch + 1; to ++epoch;
#6 Use != instead of >
!= 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled (6 gas)
#7 Unlock time should get cached
cache the recipients.length to the local too for saving the gas fee. because mload is cheaper than sload.
#8 operation
for saving more gas do change code from
locked_.amount += int128(int256(_value));
to
locked_.amount = locked_.amount + int128(int256(_value));
apply to others
#9 Use custom revert
Custom errors from Solidity 0.8.0 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met)
#10 toLocked.end should get cached
cache the toLocked.end to the local because use multiple times for saving the gas fee. because mload is cheaper than sload.
#11 Default value
default value uint is 0 so remove unnecassary explicit value can reduce gas.
#12 Looping
default value uint is 0 so remove unnecassary explicit value can reduce gas. pre increment e.g ++i more cheaper gas than post increment e.g i++. i suggest to use pre increment.