Platform: Code4rena
Start Date: 06/01/2023
Pot Size: $210,500 USDC
Total HM: 27
Participants: 73
Period: 14 days
Judge: 0xean
Total Solo HM: 18
Id: 203
League: ETH
Rank: 60/73
Findings: 1
Award: $72.44
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: IllIllI
Also found by: 0xA5DF, 0xSmartContract, 0xhacksmithh, AkshaySrivastav, Awesome, Aymen0909, Bauer, Bnke0x0, Breeje, Budaghyan, Cyfrin, Madalad, NoamYakov, RHaO-sec, Rageur, RaymondFam, ReyAdmirado, Rolezn, SAAJ, SaharDevep, Sathish9098, __141345__, amshirif, arialblack14, c3phas, carlitox477, chaduke, delfin454000, descharre, nadin, oyc_109, pavankv, saneryee, shark
72.4433 USDC - $72.44
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.
sort the constants like this to save slots: uint48 public constant MAX_UNSTAKING_DELAY = 31536000; uint192 public constant MAX_REWARD_RATIO = FIX_ONE; uint48 public constant MAX_REWARD_PERIOD = 31536000; uint192 private constant MAX_STAKE_RATE = 1e9 * FIX_ONE; uint8 public constant decimals = 18; like this we will use 2 less slots
bring unstakingDelay after stakeRate and bring payoutLastPaid after draftRate, doin so will use 2 less slots
move MAX_AUCTION_LENGTH before auctionLength to use one less slot
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 or 300 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.
rsrTrader is highly possible to be read and type casted to address twice in #L176 and #L241. instead cache the casted to address version in memory and use it later.
cache it before the if statement of #L173 because its highly possible to happen in both places.
(if the above solution didnt get accepted make sure to cache address(rsrTrader) before the #L238 for loop because address(rsrTrader) is being read every iteration without being changed.
address(rTokenTrader) is being read every iteration of for loop without being changed cache it before the for loop
cache other.erc20s[i] at start of the for loop to save reading it twice more per iteration
cache config.erc20s[i] at start of the for loopto save 1 more usage per iteration
cache basket.erc20s[i] at start of the loop to save 1 more usage per iteration
cache basket.erc20s[i] at start of the loop to save 2 more usage per iteration (as casted version is already cached can use erc20s[i] instead of address(basket.erc20s[i]) in #L421)
cache rsr and IERC20(address(rToken)) and IERC20(address(stRSR)) to not read them every iteration because they dont change
there is a high chance that require will pass so we can cache rsr with a small chance to lose 3 gas but high chance to save 100
destinations.length() is being read twice so cache it before
cache period.(usually if statement at #L71 passes so consider caching before it, small chance of losing 3 gas but high chance to save 100).
cache lastPayout because its used twice.
cache address(distributor) to save gas for one read and cast
cache tokenToBuy because its being read twice, once in the if statement #L59 and once in #L68. so cache it before the if statement.
cache queue.basketNonce to save one read. cache before the if statement
cache basketsNeeded. saves one read
cache lastIssRate right before this line. reduces one extra read
cache basketsNeeded to reduce 2 extra read
cache basketsNeeded to reduce one extra read
cache era before the require because there is high chance it passes and we save 100 gas for a small risk of losing 3
cache stakeRate
cache draftEra before the first use to stop 3 extra read
cache draftRate
cache stakeRSR can stop up to 4 extra read if we cache before #L391 and at least saves once extra use
cache draftRSRcan stop up to 4 extra read if we cache before #L406 and at least saves once extra use
cache stakeRate stops one extra use
cache draftEra stops once extra use
cache rewardPeriod stops 2 extra use if the if statement be true. cache before the #L497
cache totalStakes for possible 2 extra use and at least one extra use save. cache before 504
cache stakeRSR
cache unfreezeAt to save one extra read
cache broker
unchecked {} for subtractions where the operands cannot underflow because of a previous require() or if statementrequire(a <= b); x = b - a => require(a <= b); unchecked { x = b - a } if(a <= b); x = b - a => if(a <= b); unchecked { x = b - a } this will stop the check for overflow and underflow so it will save gas
<x> += <y> costs more gas than <x> = <x> + <y> for state variablesUsing the addition operator instead of plus-equals saves gas
make the variable outside and only give the value to variable inside
asset and req
erc20
s
qty and lowP and highP
coll and bal and refPerTok and q
erc20 and targetIndexand targetWeight
size and backupLength and j and assigned and needed
erc20
addrTo and numberOfSharesand transferAmt
t
bal and low and high and lotLow and val
q and neededand held
++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for-loop and while-loopsIn Solidity 0.8+, there’s a default overflow check on unsigned integers. It’s possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline.
all 21 instances in - BasketHandler.sol
all 12 instances in - RToken.sol
&& saves gasthis will have a large deployment gas cost but with enough runtime calls the split require version will be 3 gas cheaper
should turn this one to 15 different requires(high chance to save a lotta gas if sorted correctly)
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.
first require should be the last one of the three
bring the last require to first one
swap the requires here
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.
if the size == 0 happens we dont use a SLOAD
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
Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
setFrom
basketRange
pushDraft
_useNonce
if the functions are required by the interface, the contract should inherit from that interface and use override keyword
_authorizeUpgrade
When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size. Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed
Contracts are allowed to override their parents’ functions and change the visibility from external to public and can save gas by doing so.
nonces
permit
decreaseAllowance
increaseAllowance
saves 6 gas per instance
15 instances here
address(basket.erc20s[i]) is already cached in erc20s[i]
use less gas with reading less from storage
save IERC20(_erc20s.at(i)) and give the saved parameter to the next line to get the assets. this will possibly have a big impact because its inside a for loop
use the operation form for cheaper result
use basketsNeeded_ - baskets instead of basketsNeeded
#0 - c4-judge
2023-01-24T23:04:01Z
0xean marked the issue as grade-b