Platform: Code4rena
Start Date: 16/02/2023
Pot Size: $144,750 USDC
Total HM: 17
Participants: 154
Period: 19 days
Judge: Trust
Total Solo HM: 5
Id: 216
League: ETH
Rank: 44/154
Findings: 1
Award: $308.79
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: c3phas
Also found by: 0x3b, 0x6980, 0x73696d616f, 0xSmartContract, 0xackermann, 0xhacksmithh, 0xsomeone, Bnke0x0, Bough, Budaghyan, Darshan, DeFiHackLabs, Deivitto, GalloDaSballo, JCN, LethL, Madalad, MiniGlome, Morraez, P-384, PaludoX0, Phantasmagoria, Praise, RHaO-sec, Rageur, RaymondFam, ReyAdmirado, Rickard, Rolezn, SaeedAlipoor01988, Saintcode_, Sathish9098, TheSavageTeddy, Tomio, Viktor_Cortess, abiih, arialblack14, atharvasama, banky, codeislight, cryptonue, ddimitrov22, dec3ntraliz3d, descharre, dharma09, emmac002, favelanky, hl_, hunter_w3b, kaden, kodyvim, matrix_0wl, oyc_109, pavankv, scokaf, seeu, yamapyblack
308.7866 USDC - $308.79
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.
stabilityPoolAddress and initialized can be put together in one slot instead of 2 if they are defined near each other. make stabilityPoolAddress right after L21
mintingPaused and guardianAddress can be put together to use one less slot. make guardianAddress right after L37
treasury and emergencyShutdown can be put together in one slot. declare them near each other
emergencyExit and want are in the same slot but they are never used together in a function. instead put emergencyExit and vault together which emergencyExit is always used with in the same functions. this will not save storage slot but will have cheaper reads to save 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.
troveManagers and stabilityPools and borrowerOperations can be combined and should use a mapping of a address to a struct. they can be combined into one slot instead of 3. they are accessed in the same places so there will be extra gas saves too.
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.
use _collateralConfigAddress argument instead of collateralConfigAddress SLoad to save 100 gas.
cache defaultPoolAddress before the if statement. this will risk wasting 3 gas but will save 297 gas if the condition happens
cache collSurplusPoolAddress before the if statement. this will risk wasting 3 gas but will save 297 gas if the condition happens
cache troveManagerAddress before L328. using cached version saves 97 gas here
currentEpoch and currentScale should be cached before L434. they are used 4 times and they dont change in this part of code, so cache them and save 300 gas for each of them
cache lastDistributionTime before the if statement. we will risk using 3 extra gas but has a high possibility to save 100 or 200 gas depending on the ternary operation result
cache lastIssuanceTimestamp before the if statement. we will risk using 3 extra gas but has a high possibility to save 100
cache amount.div(distributionPeriod) in some variable and give the cached version to rewardPerSecond in the next line so we can use the cached version in the emit of L116. this will save 97 gas
cache stakes[_user] before the for loop because it doesnt change with the value of i so it doesnt need to be read every loop. skips a complex storage read for every iteration.
cache F_Collateral[collateral] in a uint variable and give snapshots[_user].F_Collateral_Snapshot[collateral] and amounts[i] the cached variable. this stops one complex storage read per iteration
cache troveManagerAddress before L252. using cached version saves 97 gas here
cache strategies[_strategy].allocBPS before L210. because its in the condition of if statement it will be checked once and if the check fails it will be read again in the next line. so we can cache it before the if statement and risk losing 3 gas to use one less complex SLOAD with a high chance of happening.
cache lockedProfit inside the if statement. saves 100 gas
cache tvlCap before the first if statement. there is a chance to save 200 gas with only risking losing 3 gas. mostly the checks fail and we save gas.
cache vault at the start of the function. saves 100 gas
cache want at the start of the if statement. saves 100 gas
can lose 1 complex storage load per instance here. cache collAmount[_collateral].sub(_amount) in some stack variable and give the cached version to collAmount[_collateral] in the next line instead. with this change we can use the cached variable that we made instead of doing a complex read from storage.
there is different codes but same concept is used so there is just one of them to show the concept
collAmount[_collateral] = collAmount[_collateral].sub(_amount); emit ActivePoolCollateralBalanceUpdated(_collateral, collAmount[_collateral]);
change to
uint256 someVariable = collAmount[_collateral].sub(_amount); collAmount[_collateral] = someVariable emit ActivePoolCollateralBalanceUpdated(_collateral, someVariable);
caching these next ones are the same concept as the last ones but they are only simple SLoads and only save 100 gas each
<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
collateral
amount
currentS
price and lowestTrove and collMCR and ICR
F_Collateral_Snapshot
strategy
5 variables here
&& saves gasInstead of using operator && on a single require. Using a two require can save more gas.
Checks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting a Gcoldsload (2100 gas*) in a function that may ultimately revert in the unhappy case.
bring the poistion of the first require to last of them. because its the most expensive require
do the requires for L94 and L97 at the start of the function and then carry on with the rest of the function
ReaperVaultV2.sol#L150-L156 these be sorted like this
require(_strategy != address(0), "Invalid strategy address"); require(_feeBPS <= PERCENT_DIVISOR / 5, "Fee cannot be higher than 20 BPS"); require(address(this) == IStrategy(_strategy).vault(), "Strategy's vault does not match"); require(address(token) == IStrategy(_strategy).want(), "Strategy's want does not match"); require(!emergencyShutdown, "Cannot add strategy during emergency shutdown"); require(strategies[_strategy].activation == 0, "Strategy already added"); require(_allocBPS + totalAllocBPS <= PERCENT_DIVISOR, "Invalid allocBPS value");
swap position of the requires
swap position of the requires
put the L96 require as the last one
There is a chance that the first part will be true so the second part doesn’t need to be checked, so it is better to use the part that we have instead of the part that needs to be called.
redemptionHelper can be put at the start of require because we already have it so 2 SLOADS can be avoided
Use a solidity version of at least 0.8.0 to get overflow protection without SafeMath and support of unchecked keyword which gives possibily for lots of gas saves
Use a solidity version of at least 0.8.2 to get compiler automatic inlining
Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads
Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings
Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value
Use a solidity version of at least 0.8.12 to get string.concat() to be used instead of abi.encodePacked(<str>,<str>)
Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions
Use a solidity version of at least 0.8.15 because the conditions necessary for inlining are relaxed. Benchmarks show that the change significantly decreases the bytecode size (which impacts the deployment cost) while the effect on the runtime gas usage is smaller.
Use a solidity version of at least 0.8.17 to get prevention of the incorrect removal of storage writes before calls to Yul functions that conditionally terminate the external EVM call; Simplify the starting offset of zero-length operations to zero. More efficient overflow checks for multiplication.
Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
there is no need give them the value false because they are already false
initialized
addressesSet
mintingPaused
As a rule of thumb, use bytes for arbitrary-length raw byte data and string for arbitrary-length string (UTF-8) data. If you can limit the length to a certain number of bytes, always use one of bytes1 to bytes32 because they are much cheaper.
these strings can be put into bytes32 instead to save gas
Using ternary operator instead of the if else statement saves gas.
Contracts are allowed to override their parents’ functions and change the visibility from external to public and can save gas by doing so.
even if they are for readability, consider making them comments instead
lqtyTokenAddress is not used anywhere in the contract. remove it to save gas
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
11 instances in
11 instances in
deleteassigning 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.
(DEGRADATION_COEFFICIENT * 46) / 10**6 is made of constants, so just pre calculate the result and put the answer instead. put (DEGRADATION_COEFFICIENT * 46) / 10**6 in a comment to show how its calculated to not lower readability of the code
using the argument is way cheaper than doing a SLOAD
instead of want and vault use _want and _vault
#0 - c4-judge
2023-03-09T17:23:38Z
trust1995 marked the issue as grade-a