Platform: Code4rena
Start Date: 08/03/2023
Pot Size: $60,500 USDC
Total HM: 2
Participants: 123
Period: 7 days
Judge: hansfriese
Id: 220
League: ETH
Rank: 55/123
Findings: 1
Award: $149.89
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: JCN
Also found by: 0x1f8b, 0xSmartContract, 0xSolus, 0xhacksmithh, 0xnev, Angry_Mustache_Man, Aymen0909, Diana, Flora, Inspex, Madalad, MatricksDeCoder, MiniGlome, R-Nemes, RaymondFam, ReyAdmirado, Rolezn, SAAJ, Sathish9098, Shubham, Udsen, Viktor_Cortess, arialblack14, atharvasama, ayden, c3phas, carlitox477, descharre, dharma09, durianSausage, fatherOfBlocks, ginlee, glcanvas, hunter_w3b, leopoldjoy, matrix_0wl, mrpathfindr, nadin, oyc_109, pipoca, schrodinger, slvDev, ulqiorra, volodya
149.8945 USDC - $149.89
If variables occupying the same slot are both written the same function or by the constructor, avoids a separate Gsset (20000 gas). Reads of the variables are also cheaper.
lpLocked
(#L511) and LP
(#232) can be put together in the same slot if they be declared near each other. they are accessed in the same function only once so mainly saves Gsset of 20000 for a slot and near 40 gas.
Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key’s keccak256 hash (Gkeccak256 - 30 gas) and that calculation’s associated stack operations.
stakedS1
(#L372) and stakedS2
(#L405) and stakerLPPosition
(#L434) can be put together to save lots of gas because they are used in the same function multiple times
_stakerS1Position
(#L378) and _stakerS2Position
(#L411) can be put together to save lots of gas because they are used in the same function multiple times
The instances below point to the second+ access of a state variable within a function. Caching of a state variable replace each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.
_stakerS1Position[_staker].length
can be cached before #L715 because its used twice (in #L716 and #L717) and the cached version can be used instead
_stakerS2Position[_staker].length
can be cached before #L732 because its used twice (in #L733 and #L734) and the cached version can be used instead
stakerLPPosition[msg.sender].multiplier
will be checked in #L1144 and if it fails we gonna have another complex SLOAD so we can cache it before the first check and risk losing 3 gas if the first check happens to be true otherwise we reduce a complex SLOAD. cache it before #L1144
cache LP
because its being read twice (in #L1137 and #L1170). so cache it before #L1137 to save 97 gas.
stakedCitizen.timelockEndTime
is being read twice and should be cached to save gas. first SLOAD is in #L1467 and the second one depending on if the #L1467 condition be true or false will be #L1467 or #L1472. so cache it before #L1467
possibly reduce a complex SLOAD by risking losing 3 gas. caching stakedCitizen.stakedBytes
will save gas if the condition happens and if it doesnt we only lose 3 gas for caching. cache before #L1477
possibly reduce a complex SLOAD by risking losing 3 gas. caching stakedCitizen.stakedVaultId
will save gas if the condition happens and if it doesnt we only lose 3 gas for caching. cache before #L1482
cache LP
because its being read twice (in #L1618 and #L1641). so cache it before #L1618 to save 97 gas.
<x> += <y>
costs more gas than <x> = <x> + <y>
for state variablesUsing the addition operator instead of plus-equals saves gas (13 or 113 each dependant on the usage see here)
make the variable outside and only give the value to variable inside
calldata
instead of memory
for read-only arguments in external functions saves gasWhen a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution.
Using ternary operator instead of the if else statement saves gas.
before transfer we should check for amount being 0 so the function doesnt run when its not gonna do anything
amount
Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions.
See more here. you can use this tool to get the optimized version for function and properties signitures
In the EVM, there is no opcode for non-strict inequalities (>=, <=) and two operations are performed (> + = or < + =).
consider replacing >= with the strict counterpart > and add - 1
to the second side
You can cut out 10 opcodes in the creation-time EVM bytecode if you declare a constructor payable. Making the constructor payable eliminates the need for an initial check of msg.value == 0 and saves 13 gas on deployment with no security risks.
delete
assigning to zero uses more gas than using delete
, and they both assign variable to default value. so it is encouraged if the data is no longer needed use delete instead.
5 instances here
3 instances here
1 here
just pre calculate the answer of these parts and use the answer inside the code instead. use a comment to show what it was to the readers
_DIVISOR / _DIVISOR
can be pre calculated and given to the contract as a constant. this stops one operation
100 * timelockMultiplier / _DIVISOR
can just use timelockMultiplier
because the other operations result will be 1
100 / _BYTES_PER_POINT
can be pre calculated and given to the contract as a constant. this stops one operation
amount * 100 / 1e18 * timelockMultiplier / _DIVISOR
we can pre calculate the 100 / 1e18 / _DIVISOR
part to reduce use of 2 operations
100 * _DIVISOR
can be pre calculated and given to the contract as a constant. this stops one operation
amount * 100 / 1e18 * lpPosition.multiplier / _DIVISOR
we can pre calculate the 100 / 1e18 / _DIVISOR
part to reduce use of 2 operations
#0 - c4-judge
2023-03-17T04:00:20Z
hansfriese marked the issue as grade-a