Platform: Code4rena
Start Date: 25/10/2022
Pot Size: $50,000 USDC
Total HM: 18
Participants: 127
Period: 5 days
Judge: 0xean
Total Solo HM: 9
Id: 175
League: ETH
Rank: 56/127
Findings: 2
Award: $55.74
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0x1f8b
Also found by: 0xNazgul, 0xSmartContract, Aymen0909, B2, Bnke0x0, Deivitto, Diana, Dinesh11G, ElKu, JC, Josiah, Rahoz, RaymondFam, ReyAdmirado, Rolezn, Waze, __141345__, adriro, aphak5010, brgltd, c3phas, c7e7eff, carlitox477, cducrest, ch0bu, chrisdior4, cryptonue, cryptostellar5, cylzxje, d3e4, delfin454000, enckrish, evmwanderer, fatherOfBlocks, gogo, hansfriese, horsefacts, immeas, leosathya, lukris02, neumo, oyc_109, pedr02b2, rbserver, robee, rotcivegaf, rvierdiiev, sakshamguruji, shark, simon135, tnevler, trustindistrust, wagmi
36.7345 USDC - $36.73
oprator --> operator
replen --> replenishmentPriceBps
allowe --> allowed
addres --> address
liquidateable --> liquidatable (used as liquidatable in the code)
Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.
Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it’s not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.
Even assembly can benefit from using readable constants instead of hex/numeric literals
Usually lines in source code are limited to 80 characters. Its advised to keep lines lower than 120 characters. Today’s screens are much larger so it’s reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length
Whenever a function is not being called internally in the code, it can be easily declared as external. While the entire code base have explicit visibilities for every function, some of them can be changed to be external.
There are multiple occasions where certain numbers have been hardcoded, either in variables or in the code itself. Large numbers can become hard to read.
require(msg.sender == gov) 3 uses, first use shown below
require(msg.sender == chair) 3 uses, first use shown below
require(dbr.markets(address(market)) 2 uses, first use shown below
require(markets[msg.sender]) 3 uses, first use shown below
require(balanceOf(from) >= amount) 2 uses, first use shown below
require(deadline >= block.timestamp 2 uses, first use shown below
require(price > 011) 2 uses, first use shown below
the modifier onlyGov was made for this scenario, use it instead of the require() in the rest of contract
here are the require()s
might contain something other than constant which will be confusing if used in all caps
Use abi.encode instead If all the arguments are strings bytes.concat() should be used
To improve algorithm precision instead of using division in comparison use multiplication in the following scenario:
Instead of a < b / c use a * c < b
Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment
#0 - c4-judge
2022-11-08T00:32:13Z
0xean marked the issue as grade-b
🌟 Selected for report: pfapostol
Also found by: 0x1f8b, 0xRoxas, 0xSmartContract, Amithuddar, Aymen0909, B2, Bnke0x0, Chandr, CloudX, Deivitto, Diana, Dinesh11G, ElKu, HardlyCodeMan, JC, JrNet, KoKo, Mathieu, Ozy42, Rahoz, RaymondFam, ReyAdmirado, Rolezn, Shinchan, __141345__, adriro, ajtra, aphak5010, ballx, c3phas, carlitox477, ch0bu, chaduke, cryptostellar5, djxploit, durianSausage, enckrish, exolorkistis, fatherOfBlocks, gogo, horsefacts, kaden, karanctf, leosathya, martin, mcwildy, oyc_109, ret2basic, robee, sakman, sakshamguruji, shark, skyle, tnevler
19.0072 USDC - $19.01
Avoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmaccess (100 gas) with a PUSH32 (3 gas).
remove it to save gas
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.
put it near one of the addresses (L15-16)
consider putting these near gov because borrowPaused has been used with it so it will cause cheaper read.(we focus on borrowPaused cheaper read because callOnDepositCallback is immutable and has much cheaper read)
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.
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
>= instead of > because if the operands are equal they will be 0 as wellthis will not let the operation after the if statement occur which will save gas
<x> += <y> costs more gas than <x> = <x> + <y> for state variablesUsing the addition operator instead of plus-equals saves gas
Each extra memory word of bytes past the original 32 incurs an MSTORE which costs 3 gas
&& saves gasthis will have a large deployment gas cost but with enough runtime calls the split require version will be 3 gas cheaper
Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
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
If data can fit into 32 bytes, then you should use bytes32 datatype rather than bytes or strings as it is cheaper in solidity.
require(<x> == true) => require(<x>)
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.
collateralFactorBps
liquidationFeeBpshere mostly require gonna pass so its possible to have 2 fewer state var read if we cache pendingOperator before the require check(use the cached pendingOperator stack var for emit instead of Operator)
If the variable is only accessed once, it’s cheaper to use the state variable directly that one time, and save the 3 gas the extra stack assignment would spend
supply
debt
replenishmentCost
implementation
deployer
collateralBalance
collateralValue
credit
limit
feedDecimals
tokenDecimals
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.
#0 - c4-judge
2022-11-05T23:51:42Z
0xean marked the issue as grade-b