Platform: Code4rena
Start Date: 27/05/2022
Pot Size: $75,000 USDC
Total HM: 20
Participants: 58
Period: 7 days
Judge: GalloDaSballo
Total Solo HM: 15
Id: 131
League: ETH
Rank: 40/58
Findings: 2
Award: $171.45
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x29A, 0xNazgul, 0xf15ers, BowTiedWardens, Chom, Funen, Kaiziron, Kumpa, MiloTruck, Picodes, Ruhum, SecureZeroX, Sm4rty, SmartSek, StyxRave, WatchPug, Waze, asutorufos, bardamu, berndartmueller, c3phas, catchup, cccz, codexploder, cryptphi, defsec, delfin454000, dipp, fatherOfBlocks, gzeon, hake, hansfriese, hyh, masterchief, oyc_109, sach1r0, sashik_eth, shenwilly, simon135, unforgiven
113.5243 USDC - $113.52
#1 Immutable
Impact the state can't be initialize by constructor.
Proof Of Concept
Tool Used Manual Review
Recommended Mitigation Steps the state must add immutable because in the constructor parameter mention fundAdmin to initialize. so i suggest to add immutable on it.
address public fundAdmin;
to
address public immutable fundAdmin;
#2 Typo
Impact missleading
Proof of Concept https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/BkdLocker.sol#L173
Tools Used manual review
Recommended Mitigation Steps fix the typo to increase readibility. fix it from
* @dev This does not invlude the gov. tokens queued for withdrawal.
to
* @dev This does not includes the gov. tokens queued for withdrawal.
#0 - GalloDaSballo
2022-06-20T15:31:33Z
Disagree as the variable is changed in a setter
Valid non-critical finding
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, Chom, Dravee, Fitraldys, Funen, Kaiziron, MiloTruck, Picodes, Randyyy, RoiEvenHaim, SecureZeroX, Sm4rty, SmartSek, StyxRave, Tadashi, Tomio, Waze, asutorufos, berndartmueller, c3phas, catchup, csanuragjain, defsec, delfin454000, djxploit, fatherOfBlocks, gzeon, hake, hansfriese, oyc_109, robee, sach1r0, sashik_eth, scaraven, simon135
57.93 USDC - $57.93
#1 Memory to storage
use storage instead of memory can reduce the gas. i suggest to change
address[] memory pools = addressProvider.allPools();
to
address[] storage pools = addressProvider.allPools();
apply to others
#2 Use memory instead calldata
In the external functions where the function argument is read-only, the function() has an inputed parameter that using memory, if this function didnt change the parameter, its cheaper to use calldata then memory. so we suggest to change it.
function burnToTarget(address[] memory tokens_, address targetLpToken_)
to
function burnToTarget(address[] calldata tokens_, address targetLpToken_)
apply to others.
#3 use != 0 instead of >0
for unsigned integer, >0 is less efficient then !=0, so use !=0 instead of >0. do to all line code.
#4 Caching lpgauge
ILpGauge(lpGauge).userCheckpoint(src); ILpGauge(lpGauge).userCheckpoint(dst);
to the memory for reduce the gas fee because it use multiple times.
#5 Pre increment
using pre increment more cheaper than post increment. so, i sugget to change
epoch++;
to
++epoch;
#6 change string to bytes32
reduce size of error message can reduce the gas fee. i suggest to convert string to bytes32
#7 Caching the length
caching the array length can reduce gas it caused access to a local variable is more cheap than query storage / calldata / memory in solidity.
#0 - GalloDaSballo
2022-06-17T23:15:39Z
Would love to see a detailed POC as common sense dictates that storage is more expensive
Same as above
Only for require, <=0.8.13, using optimizer, saves 3 gas
3 * 2 = 6
Disagree, it's already cached, the casting has no gas cost and it's a higher language construct
Saves 5 gas
Saves 6 gas per discussion in #17
## Caching the length Saves 3 gas
3 * 2 = 6
Total Gas Saved 23