Platform: Code4rena
Start Date: 14/06/2022
Pot Size: $100,000 USDC
Total HM: 26
Participants: 59
Period: 7 days
Judge: GalloDaSballo
Total Solo HM: 9
Id: 133
League: ETH
Rank: 23/59
Findings: 2
Award: $951.63
🌟 Selected for report: 1
🚀 Solo Findings: 0
🌟 Selected for report: joestakey
Also found by: 0x1f8b, 0x29A, 0x52, 0xDjango, 0xNazgul, 0xf15ers, 0xmint, Bronicle, Dravee, Funen, JMukesh, Limbooo, MadWookie, Picodes, Ruhum, TerrierLover, TomJ, Tutturu, WatchPug, Waze, _Adam, asutorufos, c3phas, catchup, cccz, codexploder, cryptphi, csanuragjain, defsec, fatherOfBlocks, gzeon, hake, hansfriese, hyh, ignacio, k, nxrblsrpr, oyc_109, robee, sach1r0, saian, simon135, technicallyty, zzzitron
687.9945 CANTO - $111.11
708.3931 USDC - $708.39
The general concerns are with the use of deprecated methods:
Properly functioning code should never reach a failing assert statement. If it happened, it would indicate the presence of a bug in the contract. A failing assert uses all the remaining gas, which can be financially painful for a user.
Low
Instances include:
l214 assert(assetIndex < len) l360 assert(markets[cToken].accountMembership[borrower])
l82 assert(msg.sender == address(wcanto)) l227 assert(amountAOptimal <= amountADesired) l273 assert(wcanto.transfer(pair, amountCANTO)) l419 assert(wcanto.transfer(pairFor(routes[0].from, routes[0].to, routes[0].stable), amounts[0]))
Manual Analysis
Replace the assert statements with a require statement or a custom error
In Comptroller.sol
, it is mentioned that closeFactorMantissa
should be greater than closeFactorMinMantissa
and less than closeFactorMaxMantissa
. But in _setCloseFactor
, these are not checked, meaning closeFactorMantissa
can be set to a value outside the boundaries defined by the protocol.
Low
Instances include:
l81-l85 // closeFactorMantissa must be strictly greater than this value uint internal constant closeFactorMinMantissa = 0.05e18; // 0.05 // closeFactorMantissa must not exceed this value uint internal constant closeFactorMaxMantissa = 0.9e18; // 0.9
l850-l859 function _setCloseFactor(uint newCloseFactorMantissa) external returns (uint) { // Check caller is admin require(msg.sender == admin, "only admin can set close factor"); uint oldCloseFactorMantissa = closeFactorMantissa; closeFactorMantissa = newCloseFactorMantissa; emit NewCloseFactor(oldCloseFactorMantissa, closeFactorMantissa); return uint(Error.NO_ERROR); }
Manual Analysis
Add checks in _setCloseFactor
to ensure closeFactorMantissa
is greater than closeFactorMinMantissa
and less than closeFactorMaxMantissa
strings and bytes are encoded with padding when using abi.encodePacked
. This can lead to hash collision when passing the result to keccak256
Low
Instances include:
pair = address(uint160(uint256(keccak256(abi.encodePacked( hex'ff', factory, keccak256(abi.encodePacked(token0, token1, stable)), @audit this is a bytes type argument pairCodeHash // init code hash )))))
Manual Analysis
Use abi.encode()
instead.
constructors should check the address written in an immutable address variable is not the zero address
Low
Instances include:
l107 (token0, token1, stable) = (_token0, _token1, _stable)
l75factory = _factory; pairCodeHash = IBaseV1Factory(_factory).pairCodeHash(); wcanto = IWCANTO(_wcanto);
Manual Analysis
Add a zero address check for the immutable variables aforementioned.
in AccountantDelegate
and TreasuryDelegate
, the initialize()
function has no check to make sure it has not been called before. This means a malicious admin
can call these functions more than once and change the note
and Cnote
token contracts used.
Low
Instances include:
l15 function initialize
l15 function initialize
Manual Analysis
Add a require
statement or modifier to ensure initialize()
can only be called once.
The old approve()
method of managing allowances has a race condition issue. Users of this token will be open to front-running attacks.
Low
l91 function _approve( address owner, address spender, uint256 amount ) internal { require(owner != address(0), "ERC20: approve from the zero address"); require(spender != address(0), "ERC20: approve to the zero address"); _allowance[owner][spender] = amount; emit Approval(owner, spender, amount); }
Manual Analysis
Use an increase/decrease allowance type of methods instead.
AccountantDelegate
has a receive()
function, but does not have any withdrawal function. Any Manifest mistakenly sent to this contract would be locked.
Low
l94 receive() external override payable {}
Manual Analysis
Add require(0 == msg.value)
in receive()
or remove the function altogether.
Setters and initializers should check the input value - ie make revert if it is the zero address or zero
Low
Instances include:
l144 function _setPendingAdmin()
l826 function _setPriceOracle() l1015 function _setBorrowCapGuardian() l1033 function _setPauseGuardian() l1394 function _grantComp() //not a setter, but distribution function so should also check input address l1423 function _setContributorCompSpeed()
l15 function initialize()
l20 function initialize() // treasury
l35 constructor // admin
l46 function sendFund()
l21 constructor // admin
l14 function _setAccountantContract
l497 function setPauser()
Manual Analysis
Add non-zero checks - address or uint256 - to the setters aforementioned.
ERC20 token implementations typically include zero-address checks on both sender and recipient addresses of transfer functions.
This is not the case in WETH.sol
, where no check is performed on the recipient
Low
Instances include:
l65-l83 function transferFrom(address src, address dst, uint wad) public returns (bool) { require(_balanceOf[src] >= wad); if (src != msg.sender && _allowance[src][msg.sender] != type(uint).max) { require(_allowance[src][msg.sender] >= wad); _allowance[src][msg.sender] -= wad; } _balanceOf[src] -= wad; _balanceOf[dst] += wad; emit Transfer(src, dst, wad); return true; }
Manual Analysis
Add a zero-address check on dst
Underflow is desired in several price update functions of stableswap/BaseV1Pair
, but as overflow/underflow checks are automatically performed since Solidity 0.8.0, the functions currently revert if there is underflow
Low
Instances include:
l156 uint timeElapsed = blockTimestamp - blockTimestampLast; // overflow is desired l183 uint timeElapsed = blockTimestamp - _blockTimestampLast
Manual Analysis
Place these statements in an unchecked
block to allow underflow
Some of the function comments are missing function parameters or returns
Non-Critical
Instances include:
l452 @param borrowerIndex l526 @param seizeTokens l677 @param accounts l689 @param accounts l826 @param newOracle l1210 @param marketBorrowIndex l1270 @param marketBorrowIndex
l31 @param borrower
l92 @param cash l92 @param borrows l92 @param reserves l109 @param cash l109 @param borrows l109 @param reserves l109 @param reserveFactorMantissa
Manual Analysis
Add a comment for these parameters
There are portions of commented code in some files.
Non-Critical
Instances include:
l24 /* emit Deposit(msg.sender, msg.value); */ l32 /* emit Withdrawal(msg.sender, wad); */
l167 //comptroller.repayBorrowVerify(address(this), payer, borrower, vars.actualRepayAmount, vars.borrowerIndex)
l362 //amountIn -= amountIn / 10000; // remove fee from amount received
Manual Analysis
Remove commented code
It is best practice to use constant variables rather than literal values to make the code easier to understand and maintain.
Non-Critical
Instances include:
l95 100 l96 100
Manual Analysis
Define constant variables for the literal values aforementioned.
Visibility (public / external) is not needed for constructors anymore since Solidity 0.7.0, see here
Non-Critical
Instances include:
l16 constructor( address implementation_, address admin_, address cnoteAddress_, address noteAddress_, address comptrollerAddress_, address treasury_) public
Manual Analysis
Remove the public
modifier from constructors.
It is not recommended to emit events before the end of the computations, as the function might revert based on conditions ahead of the event emission
Non-Critical
Instances include:
emit Redeem(redeemer, redeemAmount, redeemTokens); /* We call the defense hook */ comptroller.redeemVerify(address(this), redeemer, redeemAmount, redeemTokens)
Manual Analysis
Place the defense hook before the two event emissions.
Events should use indexed fields
Non-Critical
Instances include:
l19 event MarketListed(CToken cToken) l22 event MarketEntered(CToken cToken, address account) l25 event MarketExited(CToken cToken, address account) l28 event NewCloseFactor(uint oldCloseFactorMantissa, uint newCloseFactorMantissa) l31 event NewCollateralFactor(CToken cToken, uint oldCollateralFactorMantissa, uint newCollateralFactorMantissa) l34 event NewLiquidationIncentive(uint oldLiquidationIncentiveMantissa, uint newLiquidationIncentiveMantissa) l37 event NewPriceOracle(PriceOracle oldPriceOracle, PriceOracle newPriceOracle) l40 event NewPauseGuardian(address oldPauseGuardian, address newPauseGuardian) l43 event ActionPaused(string action, bool pauseState) l46 event ActionPaused(CToken cToken, string action, bool pauseState) l49 event CompBorrowSpeedUpdated(CToken indexed cToken, uint newSpeed) l52 event CompSupplySpeedUpdated(CToken indexed cToken, uint newSpeed) l55 event ContributorCompSpeedUpdated(address indexed contributor, uint newSpeed) l58 event DistributedSupplierComp(CToken indexed cToken, address indexed supplier, uint compDelta, uint compSupplyIndex) l61 event DistributedBorrowerComp(CToken indexed cToken, address indexed borrower, uint compDelta, uint compBorrowIndex) l64 event NewBorrowCap(CToken indexed cToken, uint newBorrowCap) l67 event NewBorrowCapGuardian(address oldBorrowCapGuardian, address newBorrowCapGuardian) l70 event CompGranted(address recipient, uint amount) l73 event CompAccruedAdjusted(address indexed user, uint oldCompAccrued, uint newCompAccrued) l76 event CompReceivableUpdated(address indexed user, uint oldCompReceivable, uint newCompReceivable)
l15 event AcctInit(address lendingMarketAddress) l16 event AcctSupplied(uint amount, uint err) l25 event NewImplementation(address oldImplementation, address newImplementation)
l17 event NewImplementation(address oldImplementation, address newImplementation)
l10 event AccountantSet(address accountant, address accountantPrior)
l17 event NewInterestParams(uint baserateperblock) l61 event NewBaseRate(uint oldBaseRateMantissa, uint newBaseRateMantissa) l64 event NewAdjusterCoefficient(uint oldAdjusterCoefficient, uint newAdjusterCoefficient) l67 event NewUpdateFrequency(uint oldUpdateFrequency, uint newUpdateFrequency)
l88 event Mint(address indexed sender, uint amount0, uint amount1); l89 event Burn(address indexed sender, uint amount0, uint amount1, address indexed to); l90 event Swap( address indexed sender, uint amount0In, uint amount1In, uint amount0Out, uint amount1Out, address indexed to ); l98 event Sync(uint reserve0, uint reserve1); l99 event Claim(address indexed sender, address indexed recipient, uint amount0, uint amount1); l101 event Transfer(address indexed from, address indexed to, uint amount); l102 event Approval(address indexed owner, address indexed spender, uint amount) l486 event PairCreated(address indexed token0, address indexed token1, bool stable, address pair, uint)
Manual Analysis
Add indexed fields to these events so that they have the maximum number of indexed fields possible.
Setters should emit an event so that Dapps can detect important changes to storage
Non-Critical
Instances include:
l22 function deposit() l28 function withdraw()
l131 function _initiate()
l497 function setPauser() l507 function setPause()
Manual Analysis
emit an event in all setters
Some functions are missing Natspec comments
Non-Critical
Instances include:
l46 function AddProposal l52 function QueryProp
All the functions are missing comments
l77 function queueOrRevertInternal l180 function add256 l186 function sub256 l191 function getChainIdInternal()
l294 function redeemAllowedInternal l180 function add256 l186 function sub256 l191 function getChainIdInternal() l958 function _addMarketInternal() l965 function _initializeMarket l1050 function _setMintPaused l1060 function _setBorrowPaused l1070 function _setTransferPaused l1079 function _setSeizePaused l1088 function _become l1094 function fixBadAccruals l1144 function adminOrInitializing l1461 function getBlockNumber
l14 function _setAccountantContract l23 function getAccountant
All the functions are missing proper Natspec comments.
All the functions are missing proper Natspec comments.
Manual Analysis
Add comments to these functions
Functions should be ordered following the Soldiity conventions: receive()
function should be placed after the constructor and before every other function.
Non-Critical
Several contracts have receive()
and fallback()
at the end:
lending-market/AccountantDelegate.sol
lending-market/AccountantDelegator.sol
lending-market/TreasuryDelegator.sol
Manual Analysis
Place the receive()
and fallback()
functions after the constructor, before all the other functions.
In lending-market/NoteInterest.sol
, there is local variable shadowing: the constructor parameter has the same name as the storage variable baseRatePerYear
. This will not lead to any error but can be confusing, especially in the constructor where baseRatePerBlock
is computed using the constructor parameter baseRatePerYear
.
Non-critical
Instances include:
constructor(uint baseRatePerYear) { baseRatePerBlock = baseRatePerYear.div(blocksPerYear)
Manual Analysis
Add an underscore to the constructor parameter (_baseRatePerYear
) to avoid shadowing.
contracts should be compiled using a fixed compiler version. Locking the pragma helps ensure that contracts do not accidentally get deployed using a different compiler version with which they have been tested the most
Non-Critical
Instances include:
WETH.sol
, GovernorBravoDelegate.sol
, Comptroller.sol
, AccountantDelegate.sol
, AccountantDelegator.sol
, AccountantInterfaces.sol
, TreasuryDelegate.sol
, TreasuryDelegator.sol
, TreasuryInterfaces.sol
, CNote.sol
and NoteInterest.sol
have floating pragmas.
Manual Analysis
Used a fixed compiler version
contracts within the scope should be compiled using the same compiler version.
Non-Critical
WETH.sol
, GovernorBravoDelegate.sol
, Comptroller.sol
, AccountantDelegate.sol
, AccountantDelegator.sol
, AccountantInterfaces.sol
, TreasuryDelegate.sol
, TreasuryDelegator.sol
, TreasuryInterfaces.sol
, CNote.sol
and NoteInterest.sol
have the compiler version set to ^0.8.10
, while BaseV1-core.sol
and BaseV1-periphery
have the 0.8.11
version.
Manual Analysis
Use the same compiler version throughout the contracts
There are open TODOs in the code. Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment.
Non-Critical
Instances include:
l1232 // TODO: Don't distribute supplier COMP if the user is not in the supplier market. l1271 // TODO: Don't distribute supplier COMP if the user is not in the borrower market.
Manual Analysis
Remove the TODOs
It is good practice to mark functions as external
instead of public
if they are not called by the contract where they are defined.
Non-Critical
Instances include:
l46 function AddProposal() l52 function QueryProp()
l24 function initialize()
l122 function enterMarkets() l677 function getAccountLiquidity() l703 function getHypotheticalAccountLiquidity() l826 function _setPriceOracle() l1033 function _setPauseGuardian() l1050 function _setMintPaused() l1060 function _setBorrowPaused() l1070 function _setTransferPaused() l1079 function _setSeizePaused() l1088 function _become() l1324 function claimComp(address holder) l1394 function _grantComp() l1407 function _setCompSpeeds() l1423 function _setContributorCompSpeed() l1444 function getAllMarkets()
l15 function initialize()
l109 delegateToViewImplementation()
l15 function initialize()
l84 delegateToViewImplementation()
l14 function _setAccountantContract
l118 function updateBaseRate
Manual Analysis
Declare these functions as external
instead of public
When there are mappings that use the same key value, having separate fields is error prone, for instance in case of deletion or with future new fields.
Non-Critical
Instances include:
l85 mapping(address => uint) public supplyIndex0 l86 mapping(address => uint) public supplyIndex1
Manual Analysis
Group the related data in a struct and use one mapping:
struct SupplyIndex { uint256 index0; uint256 index1; }
And it would be used as a state variable:
mapping(address => SupplyIndex) public supplyIndexes;
Some require statements are missing error strings, which makes it more difficult to debug when the function reverts.
Non-critical
l69 require(_balanceOf[src] >= wad) l72 require(_allowance[src][msg.sender] >= wad)
l53 require(proposals[unigovProposal.id].id == 0)
l125 require(_unlocked == 1) l285 require(!BaseV1Factory(factory).isPaused()); l465 require(token.code.length > 0) l468 require(success && (data.length == 0 || abi.decode(data, (bool)))) l498 require(msg.sender == pauser) l503 require(msg.sender == pendingPauser) l508 require(msg.sender == pauser)
l210 require(amountADesired >= amountAMin); l211 require(amountBDesired >= amountBMin) l291 require(IBaseV1Pair(pair).transferFrom(msg.sender, pair, liquidity)) l456 require(token.code.length > 0) l459 require(success && (data.length == 0 || abi.decode(data, (bool)))) l463 require(token.code.length > 0, "token code length faialure") l466 require(success && (data.length == 0 || abi.decode(data, (bool))), "failing here")
Manual Analysis
Add error strings to all require statements.
For readability, it is best to use scientific notation (e.g 10e5
) rather than decimal literals(100000
) or exponentiation(10**5
)
Non-Critical
Instances include:
l67 uint internal constant MINIMUM_LIQUIDITY = 10**3
Manual Analysis
Replace 10**3
with 10e3
There should be space between operands in mathematical computations
Non-Critical
Instances include:
l134 routes.length+1 l139 amounts[i+1] l366 routes[i+1].from, routes[i+1].to, routes[i+1].stable
Manual Analysis
Add spaces, e.g
-routes.length+1 +routes.length + 1
There are a few typos in the contracts.
Non-Critical
Instances include:
l89 irrelevent
l463 faialure
Manual Analysis
Correct the typos.
uint
is an alias for uint256
.
It is better to use uint256: it brings readability and consistency in the code, and it future proofs it in case of any changes to the alias of uint
Non-Critical
All the contracts in scope use uint
instead of uint256
Manual Analysis
replace uint
with
uint256
Use a solidity version of at least 0.8.12 to get string.concat()
to be used instead of abi.encodePacked()
Non-Critical
All the contracts in scope have a Solidity compiler version <0.8.12, and string.concat
could be used in the following location:
l109 name = string(abi.encodePacked("StableV1 AMM - ", erc20(_token0).symbol(), "/", erc20(_token1).symbol())); symbol = string(abi.encodePacked("sAMM-", erc20(_token0).symbol(), "/", erc20(_token1).symbol())); } else { name = string(abi.encodePacked("VolatileV1 AMM - ", erc20(_token0).symbol(), "/", erc20(_token1).symbol())); symbol = string(abi.encodePacked("vAMM-", erc20(_token0).symbol(), "/", erc20(_token1).symbol()))
Manual Analysis
Use Solidity 0.8.12
and replace string(abi.encodePacked(..)
with string.concat()
#0 - GalloDaSballo
2022-08-03T00:59:44Z
Valid L
L -> May bump up
Disagree because you'd need to be able to create a second pair with the same addresses, which you cannot
Valid L
Disagree as once note
is non-zero you cannot initialize
#1 - GalloDaSballo
2022-08-03T23:11:56Z
Disputed that's on the caller to be aware not on the token dev
Valid Low
Valid Bulked with the zero check above
#2 - GalloDaSballo
2022-08-03T23:19:37Z
L
I ran the math on a previous contest and it would take longer for the sun to extinguish than the overflow to happen, for that reason NC
NC
R
NC
Disputed as Slither will give false positives and some devs do that as mitigation
NC
NC
NC
##Â NC08 - Function order R
NC
NC
R
Dispute in lack of details
NC
R
NC
NC
Rest I disagree
Overall this report feels like a dump of regex based queries
#3 - GalloDaSballo
2022-08-03T23:20:38Z
5 L 4 R 11 NC
#4 - GalloDaSballo
2022-08-16T13:43:29Z
Headings for report
L01 - assert statement should not be used
L02 - CloseFactor unbounded
L03 - Immutable addresses lack zero-address check
L04 Receive function
L05 - Local variable shadowing
L06 - AVOID USING .TRANSFER
TO TRANSFER NATIVE TOKENS (#142)
NC01 - Underflow desired but not possible
NC02 -Comment Missing function parameter
NC03 - Constants instead of magic numbers
NC04 - Constructor visibility
NC05 - Events indexing
NC06 - Event should be emitted in setters
NC07 - Function missing comments
NC08 - Function order
NC09 - Non-library files should use fixed compiler versions
NC10 - open TODOs
NC11 - Public functions can be external
NC12 - Require statements should have descriptive strings
NC13 - Scientific notation
NC14 - Styling
NC15 - Typos
🌟 Selected for report: _Adam
Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 0xmint, Chom, Dravee, Fitraldys, Funen, JC, Limbooo, MadWookie, Picodes, Ruhum, TerrierLover, TomJ, Tomio, Waze, ak1, c3phas, catchup, defsec, fatherOfBlocks, gzeon, hake, hansfriese, joestakey, k, oyc_109, rfa, robee, sach1r0, saian, simon135, ynnad
396.9199 CANTO - $64.10
68.0285 USDC - $68.03
Anytime you are reading from storage more than once, it is cheaper in gas cost to cache the variable in memory: a SLOAD cost 100gas, while MLOAD and MSTORE cost 3 gas.
In particular, in for
loops, when using the length of a storage array as the condition being checked after each loop, caching the array length in memory can yield significant gas savings if the array length is high
Instances include:
scope: queue()
newProposal.targets.length
is read newProposal.targets.length
timesl68: i < newProposal.targets.length
scope: queueOrRevertInternal()
timelock
is read twicel78: !timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))) l79: timelock.queueTransaction(target, value, signature, data, eta)
scope: execute()
proposal.targets.length
is read proposal.targets.length
timesl90: i < proposal.targets.length
scope: _acceptAdmin()
pendingAdmin
is read 3 timesl164: msg.sender == pendingAdmin l168 address oldPendingAdmin = pendingAdmin l171 admin = pendingAdmin
scope: queue()
allMarkets.length
is read allMarkets.length
timesl959 i < allMarkets.length
scope: _setBorrowPaused()
admin
is read twicel1062 msg.sender == admin l1063 msg.sender == admin
scope: _setTransferPaused()
admin
is read twicel1071 msg.sender == admin l1072 msg.sender == admin
scope: _setSeizePaused()
admin
is read twicel1080 msg.sender == admin l1081 msg.sender == admin
scope: initialize()
note
is read 4 timesl28 note._mint_to_Accountant(msg.sender) l29 require(note.balanceOf(msg.sender) == note._initialSupply() l39 note.approve(cnoteAddress_, type(uint).max)
scope: supplyMarket()
cnote
is read twicel48 require(msg.sender == address(cnote) l49 cnote.mint(amount)
scope: redeemMarket()
cnote
is read 3 timesl60 require(msg.sender == address(cnote) l62 Exp({mantissa: cnote.exchangeRateStored()}) l66 cnote.redeem(amtToRedeem
scope: sweepInterest()
note
is read 3 timesl76 note.balanceOf(address(this)) l81 sub_(note.totalSupply(), noteBalance) l87 note.transfer(treasury, amtToSweep)
cnote
is read 3 timesl77 cnote.balanceOf(address(this)) l79 Exp({mantissa: cnote.exchangeRateStored()}) l89 cnote.transfer(address(0), CNoteBalance)
scope: _setAccountantContract()
_accountant
is read twicel15 address(_accountant) l18 emit AccountantSet(accountant_, address(_accountant))
scope: borrowFresh()
_accountant
is read twicel45 address(_accountant) l48 _accountant.supplyMarket(borrowAmount)
scope: repayBorrowFresh()
_accountant
is read twicel134 _accountant.redeemMarket(amtRedeemed) l141 payable(_accountant)
scope: mintFresh()
_accountant
is read 3 timesl179 address(_accountant) l180 address(_accountant) l225 payable(_accountant)
scope: redeemFresh()
_accountant
is read 3 timesl259 address(_accountant) l260 payable(_accountant) l324 _accountant.supplyMarket(redeemAmount)
comptroller
is read twicel294 comptroller.redeemAllowed(address(this), redeemer, redeemTokens) l343 comptroller.redeemVerify(address(this), redeemer, redeemAmount, redeemTokens)
scope: acceptPauser()
pendingPauser
is read twicel503 require(msg.sender == pendingPauser); l504 pauser = pendingPauser
Manual Analysis
cache these storage variables in memory
If a reference type function parameter is read-only, it is cheaper in gas to use calldata instead of memory. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory.
Try to use calldata as a data location because it will avoid copies and also makes sure that the data cannot be modified.
Instances include:
scope: queueOrRevertInternal
l77 string memory signature, bytes memory data
scope: delegateTo
l82 bytes memory data
scope: delegateToImplementation
l98 bytes memory data
scope: delegateToViewImplementation
l109 bytes memory data
scope: delegateToImplementation
73 bytes memory data
scope: delegateToViewImplementation
l84 bytes memory data
scope: delegateTo
l101 bytes memory data
Manual Analysis
Replace memory
with calldata
In the EVM, there is no opcode for >=
or <=
.
When using greater than or equal, two operations are performed: >
and =
.
Using strict comparison operators hence saves gas, approximately 20
gas in require
and if
statements
Instances include:
l29: _balanceOf[msg.sender] >= wad l72: _balanceOf[src] >= wad l29: _allowance[src][msg.sender] >= wad
l47 require(unigovProposal.targets.length <= proposalMaxOperations, "GovernorBravo::propose: too many actions") l115 proposalCount >= proposalId l119 block.timestamp >= add256(proposal.eta, timelock.GRACE_PERIOD())) l182 require(c >= a, "addition overflow") l187 require(b <= a, "subtraction underflow")
l491 borrowBalance >= repayAmount l1243 supplyIndex >= compInitialIndex l1282 borrowIndex >= compInitialIndex l1379 amount <= compRemaining
l83 cNoteConverted >= noteDifferential
l130 getCashPrior() >= actualRepayAmount
l309 _k(_balance0, _balance1) >= _k(_reserve0, _reserve1), 'K' l348 y - y_prev <= 1 l352 y_prev - y <= 1 l413 deadline >= block.timestamp
l71 deadline >= block.timestamp l133 routes.length >= 1 l169 amountBOptimal <= amountBDesired l210 amountADesired >= amountAMin l211 amountBDesired >= amountBMin l222 amountBOptimal <= amountBDesired l223 amountBOptimal >= amountBMin l227 amountAOptimal <= amountADesired l228 amountAOptimal >= amountAMin l295 amountA >= amountAMin l296 amountB >= amountBMin l387 amounts[amounts.length - 1] >= amountOutMin l402 amounts[amounts.length - 1] >= amountOutMin l417 amounts[amounts.length - 1] >= amountOutMin l430 amounts[amounts.length - 1] >= amountOutMin
Manual Analysis
Replace <=
with <
, and >=
with >
. Do not forget to increment/decrement the compared variable
example:
-proposalCount >= proposalId; +proposalCount > proposalId - 1;
However, if 1
is negligible compared to the value of the variable, we can omit the increment.
Constant expressions are re-calculated each time it is in use, costing an extra 97
gas than a constant every time they are called.
Instances include:
l15 bytes32 public constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)") l18 bytes32 public constant BALLOT_TYPEHASH = keccak256("Ballot(uint256 proposalId,uint8 support)")
Manual Analysis
Mark these as immutable
instead of constant
Constructor parameters are expensive. The contract deployment will be cheaper in gas if they are hard coded instead of using constructor parameters.
Instances include:
l14: _name = name_; l15: _symbol = symbol_;
l490 isPaused = false //false is the default value of isPaused, this line can be removed altogether
l76 factory = _factory l77 pairCodeHash = IBaseV1Factory(_factory).pairCodeHash() l78 wcanto = IWCANTO(_wcanto)
Manual Analysis
Hardcode storage variables with their initial value instead of writing it during contract deployment with constructor parameters.
Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) while providing the same amount of information, as explained here.
It not only saves gas upon deployment - ~5500
gas saved per custom error instead of a require statement, but it is also cheaper in a function call, 22
gas saved per require statement replaced with a custom error.
Custom errors are defined using the error statement
Instances include:
l29 require(_balanceOf[msg.sender] >= wad, "sender balance insufficient for withdrawal") l69 require(_balanceOf[src] >= wad) l72 require(_allowance[src][msg.sender] >= wad) l96 require(owner != address(0), "ERC20: approve from the zero address") l97 require(spender != address(0), "ERC20: approve to the zero address")
l25 require(address(timelock) == address(0), "GovernorBravo::initialize: can only initialize once"); l26 require(msg.sender == admin, "GovernorBravo::initialize: admin only"); l27 require(timelock_ != address(0), "GovernorBravo::initialize: invalid timelock address") l42 require(unigovProposal.targets.length == unigovProposal.values.length && unigovProposal.targets.length == unigovProposal.signatures.length && unigovProposal.targets.length == unigovProposal.calldatas.length, "GovernorBravo::propose: proposal function information arity mismatch") l46 require(unigovProposal.targets.length != 0, "GovernorBravo::propose: must provide actions"); l47 require(unigovProposal.targets.length <= proposalMaxOperations, "GovernorBravo::propose: too many actions") l53 require(proposals[unigovProposal.id].id == 0) l78 require(!timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), "GovernorBravo::queueOrRevertInternal: identical proposal action already queued at eta") l87 require(state(proposalId) == ProposalState.Queued, "GovernorBravo::execute: proposal can only be executed if it is queued") l115 require(proposalCount >= proposalId && proposalId > initialProposalId, "GovernorBravo::state: invalid proposal id") l132 require(msg.sender == admin, "GovernorBravo::_initiate: admin only") l133 require(initialProposalId == 0, "GovernorBravo::_initiate: can only initiate once") l146 require(msg.sender == admin, "GovernorBravo:_setPendingAdmin: admin only") l164 require(msg.sender == pendingAdmin && msg.sender != address(0), "GovernorBravo:_acceptAdmin: pending admin only") l182 require(c >= a, "addition overflow") l187 require(b <= a, "subtraction underflow")
l178 require(oErr == 0, "exitMarket: getAccountSnapshot failed") l237 require(!mintGuardianPaused[cToken], "mint is paused") l343 require(!borrowGuardianPaused[cToken], "borrow is paused") l351 require(msg.sender == cToken, "sender must be cToken") l373 require(nextTotalBorrows < borrowCap, "market borrow cap reached") l491 require(borrowBalance >= repayAmount, "Can not repay more than the total borrow") l556 require(!seizeGuardianPaused, "seize is paused") l614 require(!transferGuardianPaused, "transfer is paused") l852 require(msg.sender == admin, "only admin can set close factor") l960 require(allMarkets[i] != CToken(cToken), "market already added") l998 require(msg.sender == admin || msg.sender == borrowCapGuardian, "only admin or borrow cap guardian can set borrow caps") l1003 require(numMarkets != 0 && numMarkets == numBorrowCaps, "invalid input") l1016 require(msg.sender == admin, "only admin can set borrow cap guardian") l1051 require(markets[address(cToken)].isListed, "cannot pause a market that is not listed"); l1052 require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause"); l1053 require(msg.sender == admin || state == true, "only admin can unpause") l1061 require(markets[address(cToken)].isListed, "cannot pause a market that is not listed"); l1062 require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause"); l1063 require(msg.sender == admin || state == true, "only admin can unpause") l1071 require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause"); l1072 require(msg.sender == admin || state == true, "only admin can unpause") l1080 require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause"); l1081 require(msg.sender == admin || state == true, "only admin can unpause") l1089 require(msg.sender == unitroller.admin(), "only unitroller admin can change brains"); l1090 require(unitroller._acceptImplementation() == 0, "change not authorized"); l1095 require(msg.sender == admin, "Only admin can call this function"); // Only the timelock can call this function l1096 require(!proposal65FixExecuted, "Already executed this one-off function"); // Require that this function is only called once l1097 require(affectedUsers.length == amounts.length, "Invalid input") l1158 require(market.isListed, "comp market is not listed") l1349 require(markets[address(cToken)].isListed, "market must be listed") l1395 require(adminOrInitializing(), "only admin can grant comp") l1397 require(amountLeft == 0, "insufficient comp for grant") l1408 require(adminOrInitializing(), "only admin can set comp speed") l1411 require(numTokens == supplySpeeds.length && numTokens == borrowSpeeds.length, "Comptroller::_setCompSpeeds invalid input") l1424 require(adminOrInitializing(), "only admin can set comp speed")
l17 require(msg.sender == admin, "AccountantDelegate::initialize: only admin can call this function"); l18 require(noteAddress_ != address(0), "AccountantDelegate::initialize: note Address invalid") l29 require(note.balanceOf(msg.sender) == note._initialSupply(), "AccountantDelegate::initiatlize: Accountant has not received payment") l48 require(msg.sender == address(cnote), "AccountantDelegate::supplyMarket: Only the CNote contract can supply market") l60 require(msg.sender == address(cnote), "AccountantDelegate::redeemMarket: Only the CNote contract can redeem market") l83 require(cNoteConverted >= noteDifferential, "Note Loaned to LendingMarket must increase in value")
l43 require(msg.sender == admin, "AccountantDelegator::_setImplementation: admin only") l44 require(implementation_ != address(0), "AccountantDelegator::_setImplementation: invalid implementation address") l124 require(msg.value == 0,"AccountantDelegator:fallback: cannot send value to fallback")
l31 require(msg.sender == admin, "GovernorBravoDelegator::_setImplementation: admin only") l32 require(implementation_ != address(0), "GovernorBravoDelegator::_setImplementation: invalid implementation address")
l16 require(msg.sender == admin, "CNote::_setAccountantContract:Only admin may call this function") l43 require(getCashPrior() == 0, "CNote::borrowFresh:Impossible reserves in CNote market Place") l45 require(address(_accountant) != address(0), "CNote::borrowFresh:Accountant has not been initialized") l54 require(getCashPrior() == borrowAmount, "CNote::borrowFresh:Error in Accountant supply") l77 require(getCashPrior() == 0,"CNote::borrowFresh: Error in doTransferOut, impossible Liquidity in LendingMarket") l114 require(getCashPrior() == 0, "CNote::repayBorrowFresh:Liquidity in Note Lending Market is always flashed") l130 require(getCashPrior() >= actualRepayAmount, "CNote::repayBorrowFresh: doTransferIn supplied incorrect amount") l146 require(getCashPrior() == 0, "CNote::repayBorrowFresh: Error in Accountant.redeemMarket") l198 require(getCashPrior() == 0, "CNote::mintFresh: Any Liquidity in the Lending Market is flashed") l214 require(getCashPrior() == actualMintAmount, "CNote::mintFresh: Error in doTransferIn, CNote reserves >= mint Amount") l229 require(getCashPrior() == 0) l264 require(redeemTokensIn == 0 || redeemAmountIn == 0, "one of redeemTokensIn or redeemAmountIn must be zero") l310 require(getCashPrior() == 0, "CNote::redeemFresh, LendingMarket has > 0 Cash before Accountant Supplies") l330 require(getCashPrior() == redeemAmount, "CNote::redeemFresh: Accountant has supplied incorrect Amount") l353 require(_notEntered, "re-entered")
l125 require(_unlocked == 1) l253 require(liquidity > 0, 'ILM') l272 require(amount0 > 0 && amount1 > 0, 'ILB') l285 require(!BaseV1Factory(factory).isPaused()); l286 require(amount0Out > 0 || amount1Out > 0, 'IOA') l288 require(amount0Out < _reserve0 && amount1Out < _reserve1, 'IL') l294 require(to != _token0 && to != _token1, 'IT') l303 require(amount0In > 0 || amount1In > 0, 'IIA') l309 require(_k(_balance0, _balance1) >= _k(_reserve0, _reserve1), 'K') l413 require(deadline >= block.timestamp, 'BaseV1: EXPIRED') l431 require(recoveredAddress != address(0) && recoveredAddress == owner, 'BaseV1: INVALID_SIGNATURE') l465 require(token.code.length > 0) l468 require(success && (data.length == 0 || abi.decode(data, (bool)))) l498 require(msg.sender == pauser) l503 require(msg.sender == pendingPauser) l508 require(msg.sender == pauser) l521 require(tokenA != tokenB, 'IA') l523 require(token0 != address(0), 'ZA') l524 require(getPair[token0][token1][stable] == address(0), 'PE')
l71 require(deadline >= block.timestamp, 'BaseV1Router: EXPIRED') l86 require(tokenA != tokenB, 'BaseV1Router: IDENTICAL_ADDRESSES') l88 require(token0 != address(0), 'BaseV1Router: ZERO_ADDRESS') l104 require(amountA > 0, 'BaseV1Router: INSUFFICIENT_AMOUNT') l105 require(reserveA > 0 && reserveB > 0, 'BaseV1Router: INSUFFICIENT_LIQUIDITY') l133 require(routes.length >= 1, 'BaseV1Router: INVALID_PATH') l210 require(amountADesired >= amountAMin); l211 require(amountBDesired >= amountBMin) l223 require(amountBOptimal >= amountBMin, 'BaseV1Router: INSUFFICIENT_B_AMOUNT') l228 require(amountAOptimal >= amountAMin, 'BaseV1Router: INSUFFICIENT_A_AMOUNT') l291 require(IBaseV1Pair(pair).transferFrom(msg.sender, pair, liquidity)) l295 require(amountA >= amountAMin, 'BaseV1Router: INSUFFICIENT_A_AMOUNT'); l296 require(amountB >= amountBMin, 'BaseV1Router: INSUFFICIENT_B_AMOUNT') l387 require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT') l402 require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT') l415 require(routes[0].from == address(wcanto), 'BaseV1Router: INVALID_PATH') l417 require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT') l428 require(routes[routes.length - 1].to == address(wcanto), 'BaseV1Router: INVALID_PATH') l430 require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT') l452 require(success, 'TransferHelper: ETH_TRANSFER_FAILED') l456 require(token.code.length > 0) l459 require(success && (data.length == 0 || abi.decode(data, (bool)))) l463 require(token.code.length > 0, "token code length faialure") l466 require(success && (data.length == 0 || abi.decode(data, (bool))), "failing here")
Manual Analysis
Replace require and revert statements with custom errors.
For instance, in lending-market/GovernorBravoDelegate.sol
:
Replace
require(address(timelock) == address(0), "GovernorBravo::initialize: can only initialize once")
with
if (address(timelock) != address(0)) { revert IsInitialized(); }
and define the custom error in the contract
error IsInitialized();
If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type).
Explicitly initializing it with its default value is an anti-pattern and wastes 22
gas per variable initialized.
Instances include:
l68 uint i = 0 l90 uint i = 0
l126 uint i = 0 l206 uint i = 0 l735 uint i = 0 l959 uint i = 0 l1005 uint i = 0 l1106 uint i = 0 l1347 uint i = 0 l1353 uint j = 0 l1359 uint j = 0 l1364 uint j = 0 l1413 uint i = 0
l46 uint public totalSupply = 0 l207 uint i = 0 l223 uint nextIndex = 0 l224 uint index = 0 l337 uint i = 0
l136 uint i = 0 l158 uint _totalSupply = 0 l362 uint i = 0
Manual Analysis
Remove explicit initialization for default values.
When emitting an event, using a local variable instead of a storage variable saves gas.
Instances include:
l176 emit NewAdmin(oldAdmin, admin); l177 emit NewPendingAdmin(oldPendingAdmin, pendingAdmin)
l75 emit NewInterestParams(baseRatePerBlock) l127 emit NewInterestParams(baseRatePerYear) l144 emit NewBaseRate(oldBaseRatePerYear, baseRatePerYear) l157 emit NewAdjusterCoefficient(oldAdjusterCoefficient, adjusterCoefficient) l170 emit NewUpdateFrequency(oldUpdateFrequency, updateFrequency)
l170 emit Sync(reserve0, reserve1)
Manual Analysis
Emit a local variable or function argument instead of storage variable
If a variable is set in the constructor and never modified afterwrds, marking it as immutable
can save a storage operation - 20,000
gas.
Instances include:
l14: _name = name_; l15: _symbol = symbol_;
Manual Analysis
Mark these variables as immutable
.
X += Y costs 22
more gas than X = X + Y.
Instances include:
l23 _balanceOf[msg.sender] += msg.value l30 _balanceOf[msg.sender] -= wad l73 _allowance[src][msg.sender] -= wad l76 _balanceOf[src] -= wad l77 _balanceOf[dst] += wad
l158 reserve0CumulativeLast += _reserve0 * timeElapsed; l159 reserve1CumulativeLast += _reserve1 * timeElapsed; l184 reserve0Cumulative += _reserve0 * timeElapsed; l185 reserve1Cumulative += _reserve1 * timeElapsed l208 priceAverageCumulative += _prices[i] l226 i+=window l394 totalSupply += amount l395 balanceOf[dst] += amount l400 totalSupply -= amount l401 balanceOf[dst] -= amount l458 balanceOf[src] -= amount l459 balanceOf[dst] += amount
Manual Analysis
use X = X + Y
instead of X += Y
(same with -
)
When a require
statement is use multiple times, it is cheaper to use a modifier instead.
Instances include:
l1051 require(markets[address(cToken)].isListed, "cannot pause a market that is not listed") l1061 require(markets[address(cToken)].isListed, "cannot pause a market that is not listed")
l1052 require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause") l1062 require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause") l1071 require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause") l1080 require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause")
l1053 require(msg.sender == admin || state == true, "only admin can unpause") l1063 require(msg.sender == admin || state == true, "only admin can unpause") l1072 require(msg.sender == admin || state == true, "only admin can unpause") l1081 require(msg.sender == admin || state == true, "only admin can unpause")
l1408 require(adminOrInitializing(), "only admin can set comp speed") l1424 require(adminOrInitializing(), "only admin can set comp speed")
l498 require(msg.sender == pauser) l508 require(msg.sender == pauser)
Manual Analysis
Use modifiers for these repeated statements
Prefix increments are cheaper than postfix increments: it returns the incremented variable instead of returning a temporary variable storing the initial value of the variable. It saves 5
gas per iteration
Instances include:
l68 i++ l90 i++
l126 i++ l206 i++ l735 i++ l959 i++ l1005 i++ l1106 i++ l1347 i++ l1353 j++ l1359 j++ l1364 j++
l207 i++ l337 i++
l136 i++ l362 i++
Manual Analysis
change variable++
to ++variable
.
Require statements including conditions with the &&
operator can be broken down in multiple require statements to save gas.
Instances include:
l42 require(unigovProposal.targets.length == unigovProposal.values.length && unigovProposal.targets.length == unigovProposal.signatures.length && unigovProposal.targets.length == unigovProposal.calldatas.length, "GovernorBravo::propose: proposal function information arity mismatch") l115 require(proposalCount >= proposalId && proposalId > initialProposalId, "GovernorBravo::state: invalid proposal id") l164 require(msg.sender == pendingAdmin && msg.sender != address(0), "GovernorBravo:_acceptAdmin: pending admin only")
l1003 require(numMarkets != 0 && numMarkets == numBorrowCaps, "invalid input") l1411 require(numTokens == supplySpeeds.length && numTokens == borrowSpeeds.length, "Comptroller::_setCompSpeeds invalid input")
l272 require(amount0 > 0 && amount1 > 0, 'ILB') l288 require(amount0Out < _reserve0 && amount1Out < _reserve1, 'IL') l294 require(to != _token0 && to != _token1, 'IT') l431 require(recoveredAddress != address(0) && recoveredAddress == owner, 'BaseV1: INVALID_SIGNATURE') l468 require(success && (data.length == 0 || abi.decode(data, (bool))))
l105 require(reserveA > 0 && reserveB > 0, 'BaseV1Router: INSUFFICIENT_LIQUIDITY') l459 require(success && (data.length == 0 || abi.decode(data, (bool)))) l466 require(success && (data.length == 0 || abi.decode(data, (bool))), "failing here")
Manual Analysis
Break down the statements in multiple require statements.
-require(msg.sender == pendingAdmin && msg.sender != address(0), "GovernorBravo:_acceptAdmin: pending admin only") +require(msg.sender == pendingAdmin) +require(msg.sender != address(0))
You can also improve gas savings by using custom errors
Named returns are the most gas efficient return statements, but there is no gas saving if the named return is unused and a return
statement is used - costing an extra 2,000
gas per function call.
Instances include:
l106 return (p.targets, p.values, p.signatures, p.calldatas)
l140 return (decimals0, decimals1, reserve0, reserve1, stable, token0, token1) l210 return priceAverageCumulative / granularity
l128 return amountStable > amountVolatile ? (amountStable, true) : (amountVolatile, false)
Manual Analysis
Replace the return
statements as explained, using a local variable with the named return instead.
Revert strings cost more gas to deploy if the string is larger than 32 bytes. Each string exceeding that 32-byte size adds an extra 9,500
gas upon deployment.
Revert strings exceeding 32 bytes include:
l29 sender balance insufficient for withdrawal l96 ERC20: approve from the zero address l97 ERC20: approve to the zero address
l25 require(address(timelock) == address(0), "GovernorBravo::initialize: can only initialize once"); l26 require(msg.sender == admin, "GovernorBravo::initialize: admin only"); l27 require(timelock_ != address(0), "GovernorBravo::initialize: invalid timelock address") l42 require(unigovProposal.targets.length == unigovProposal.values.length && unigovProposal.targets.length == unigovProposal.signatures.length && unigovProposal.targets.length == unigovProposal.calldatas.length, "GovernorBravo::propose: proposal function information arity mismatch") l46 require(unigovProposal.targets.length != 0, "GovernorBravo::propose: must provide actions"); l47 require(unigovProposal.targets.length <= proposalMaxOperations, "GovernorBravo::propose: too many actions") l78 require(!timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), "GovernorBravo::queueOrRevertInternal: identical proposal action already queued at eta") l87 require(state(proposalId) == ProposalState.Queued, "GovernorBravo::execute: proposal can only be executed if it is queued") l115 require(proposalCount >= proposalId && proposalId > initialProposalId, "GovernorBravo::state: invalid proposal id") l132 require(msg.sender == admin, "GovernorBravo::_initiate: admin only") l133 require(initialProposalId == 0, "GovernorBravo::_initiate: can only initiate once") l146 require(msg.sender == admin, "GovernorBravo:_setPendingAdmin: admin only") l164 require(msg.sender == pendingAdmin && msg.sender != address(0), "GovernorBravo:_acceptAdmin: pending admin only")
l178 require(oErr == 0, "exitMarket: getAccountSnapshot failed") l491 require(borrowBalance >= repayAmount, "Can not repay more than the total borrow") l998 require(msg.sender == admin || msg.sender == borrowCapGuardian, "only admin or borrow cap guardian can set borrow caps") l1016 require(msg.sender == admin, "only admin can set borrow cap guardian") l1051 require(markets[address(cToken)].isListed, "cannot pause a market that is not listed"); l1052 require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause"); l1061 require(markets[address(cToken)].isListed, "cannot pause a market that is not listed"); l1062 require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause"); l1071 require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause"); l1080 require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause"); l1089 require(msg.sender == unitroller.admin(), "only unitroller admin can change brains"); l1095 require(msg.sender == admin, "Only admin can call this function"); l1096 require(!proposal65FixExecuted, "Already executed this one-off function"); l1411 require(numTokens == supplySpeeds.length && numTokens == borrowSpeeds.length, "Comptroller::_setCompSpeeds invalid input")
l17 require(msg.sender == admin, "AccountantDelegate::initialize: only admin can call this function"); l18 require(noteAddress_ != address(0), "AccountantDelegate::initialize: note Address invalid") l29 require(note.balanceOf(msg.sender) == note._initialSupply(), "AccountantDelegate::initiatlize: Accountant has not received payment") l48 require(msg.sender == address(cnote), "AccountantDelegate::supplyMarket: Only the CNote contract can supply market") l60 require(msg.sender == address(cnote), "AccountantDelegate::redeemMarket: Only the CNote contract can redeem market") l83 require(cNoteConverted >= noteDifferential, "Note Loaned to LendingMarket must increase in value")
l43 require(msg.sender == admin, "AccountantDelegator::_setImplementation: admin only") l44 require(implementation_ != address(0), "AccountantDelegator::_setImplementation: invalid implementation address") l124 require(msg.value == 0,"AccountantDelegator:fallback: cannot send value to fallback")
l31 require(msg.sender == admin, "GovernorBravoDelegator::_setImplementation: admin only") l32 require(implementation_ != address(0), "GovernorBravoDelegator::_setImplementation: invalid implementation address")
l16 require(msg.sender == admin, "CNote::_setAccountantContract:Only admin may call this function") l43 require(getCashPrior() == 0, "CNote::borrowFresh:Impossible reserves in CNote market Place") l45 require(address(_accountant) != address(0), "CNote::borrowFresh:Accountant has not been initialized") l54 require(getCashPrior() == borrowAmount, "CNote::borrowFresh:Error in Accountant supply") l77 require(getCashPrior() == 0,"CNote::borrowFresh: Error in doTransferOut, impossible Liquidity in LendingMarket") l114 require(getCashPrior() == 0, "CNote::repayBorrowFresh:Liquidity in Note Lending Market is always flashed") l130 require(getCashPrior() >= actualRepayAmount, "CNote::repayBorrowFresh: doTransferIn supplied incorrect amount") l146 require(getCashPrior() == 0, "CNote::repayBorrowFresh: Error in Accountant.redeemMarket") l198 require(getCashPrior() == 0, "CNote::mintFresh: Any Liquidity in the Lending Market is flashed") l214 require(getCashPrior() == actualMintAmount, "CNote::mintFresh: Error in doTransferIn, CNote reserves >= mint Amount") l264 require(redeemTokensIn == 0 || redeemAmountIn == 0, "one of redeemTokensIn or redeemAmountIn must be zero") l310 require(getCashPrior() == 0, "CNote::redeemFresh, LendingMarket has > 0 Cash before Accountant Supplies") l330 require(getCashPrior() == redeemAmount, "CNote::redeemFresh: Accountant has supplied incorrect Amount")
l86 require(tokenA != tokenB, 'BaseV1Router: IDENTICAL_ADDRESSES') l104 require(amountA > 0, 'BaseV1Router: INSUFFICIENT_AMOUNT') l105 require(reserveA > 0 && reserveB > 0, 'BaseV1Router: INSUFFICIENT_LIQUIDITY') l223 require(amountBOptimal >= amountBMin, 'BaseV1Router: INSUFFICIENT_B_AMOUNT') l228 require(amountAOptimal >= amountAMin, 'BaseV1Router: INSUFFICIENT_A_AMOUNT') l295 require(amountA >= amountAMin, 'BaseV1Router: INSUFFICIENT_A_AMOUNT'); l296 require(amountB >= amountBMin, 'BaseV1Router: INSUFFICIENT_B_AMOUNT') l387 require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT') l402 require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT') l415 require(routes[0].from == address(wcanto), 'BaseV1Router: INVALID_PATH') l417 require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT') l430 require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT') l452 require(success, 'TransferHelper: ETH_TRANSFER_FAILED')
Manual Analysis
Write the error strings so that they do not exceed 32 bytes. For further gas savings, consider also using custom errors.
Tautologies should be avoided as they waste gas.
Instances include:
l97 uint newRatePerYear = ir >= 0 ? ir : 0
ir
is a uint256, hence always >= 0.
Manual Analysis
Replace
uint newRatePerYear = ir >= 0 ? ir : 0;
with
uint newRatePerYear = ir;
Solidity contracts have contiguous 32 bytes (256 bits) slots used in storage. By arranging the variables, it is possible to minimize the number of slots used within a contract's storage and therefore reduce deployment costs.
address type variables are each of 20 bytes size (way less than 32 bytes). However, they here take up a whole 32 bytes slot (they are contiguous).
As uint8 and bool type variables are of size 1 byte, there's a slot here that can get saved by moving an address closer to a bool and or a uint8
Instances include:
uint8 public constant decimals = 18; // Used to denote stable or volatile pair, not immutable since construction happens in the initialize method for CREATE2 deterministic addresses bool public immutable stable; uint public totalSupply = 0; mapping(address => mapping (address => uint)) public allowance; mapping(address => uint) public balanceOf; bytes32 internal DOMAIN_SEPARATOR; // keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); bytes32 internal constant PERMIT_TYPEHASH = 0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9; mapping(address => uint) public nonces; uint internal constant MINIMUM_LIQUIDITY = 10**3; address public immutable token0; address public immutable token1; address immutable factory;
Manual Analysis
Place factory
before decimals
to save one storage slot
+address immutable factory; uint8 public constant decimals = 18; // Used to denote stable or volatile pair, not immutable since construction happens in the initialize method for CREATE2 deterministic addresses bool public immutable stable; uint public totalSupply = 0; mapping(address => mapping (address => uint)) public allowance; mapping(address => uint) public balanceOf; bytes32 internal DOMAIN_SEPARATOR; // keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"); bytes32 internal constant PERMIT_TYPEHASH = 0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9; mapping(address => uint) public nonces; uint internal constant MINIMUM_LIQUIDITY = 10**3; address public immutable token0; address public immutable token1;
The default "checked" behavior costs more gas when adding/diving/multiplying, because under-the-hood those checks are implemented as a series of opcodes that, prior to performing the actual arithmetic, check for under/overflow and revert if it is detected.
if it can statically be determined there is no possible way for your arithmetic to under/overflow (such as a condition in an if statement), surrounding the arithmetic in an unchecked
block will save gas
Instances include:
l30 _balanceOf[msg.sender] -= wad //cannot underflow because of check line 29 l73 _allowance[src][msg.sender] -= wad //cannot underflow because of check line 72 l76 _balanceOf[src] -= wad //cannot underflow because of check line 69
l68 i++ l90 i++
l126 i++ l206 i++ l735 i++ l959 i++ l1005 i++ l1106 i++ l1347 i++ l1353 j++ l1359 j++ l1364 j++ l1413 ++i
l156 uint timeElapsed = blockTimestamp - blockTimestampLast //as per comment, underflow desired l183 uint timeElapsed = blockTimestamp - _blockTimestampLast //as per comment, underflow desired l207 i++ l337 i++
l136 i++ l276 msg.value - amountCANTO //cannot underflow because of check line 276 l362 i++
Manual Analysis
Place the arithmetic operations in an unchecked
block
As of Solidity 0.8.0, underflow and overflow checks are default in mathematical operations. Using functions to perform these checks is redundant and wastes gas.
Instances include:
180 function add256 186 function sub256
using SafeMath for uint
Manual Analysis
Remove these functions
There are several instances where a local variable is used but is not necessary. For instance, when emitting an event that includes a new and an old value, it is cheaper in gas to avoid caching the old value in memory. Instead, emit the event, then save the new value in storage.
Instances include:
scope: _initiate()
l134:proposalCount = GovernorAlpha(governorAlpha).proposalCount()
scope: _setPendingAdmin()
address oldPendingAdmin = pendingAdmin; // Store pendingAdmin with value newPendingAdmin pendingAdmin = newPendingAdmin; // Emit NewPendingAdmin(oldPendingAdmin, newPendingAdmin) emit NewPendingAdmin(oldPendingAdmin, newPendingAdmin);
scope: _acceptAdmin()
address oldAdmin = admin; address oldPendingAdmin = pendingAdmin; // Store admin with value pendingAdmin admin = pendingAdmin; // Clear the pending value pendingAdmin = address(0); emit NewAdmin(oldAdmin, admin); emit NewPendingAdmin(oldPendingAdmin, pendingAdmin)
scope: _setPriceOracle()
l833: PriceOracle oldOracle = oracle; // Set comptroller's oracle to newOracle oracle = newOracle; // Emit NewPriceOracle(oldOracle, newOracle) emit NewPriceOracle(oldOracle, newOracle)
scope: _setCloseFactor()
l854: uint oldCloseFactorMantissa = closeFactorMantissa; closeFactorMantissa = newCloseFactorMantissa; emit NewCloseFactor(oldCloseFactorMantissa, closeFactorMantissa)
scope: _setLiquidationIncentive()
l916 uint oldLiquidationIncentiveMantissa = liquidationIncentiveMantissa; // Set liquidation incentive to new incentive liquidationIncentiveMantissa = newLiquidationIncentiveMantissa; // Emit event with old incentive, new incentive emit NewLiquidationIncentive(oldLiquidationIncentiveMantissa, newLiquidationIncentiveMantissa)
scope: _setBorrowCapGuardian()
l1019 address oldBorrowCapGuardian = borrowCapGuardian; // Store borrowCapGuardian with value newBorrowCapGuardian borrowCapGuardian = newBorrowCapGuardian; // Emit NewBorrowCapGuardian(OldBorrowCapGuardian, NewBorrowCapGuardian) emit NewBorrowCapGuardian(oldBorrowCapGuardian, newBorrowCapGuardian)
scope: _setPauseGuardian()
l1038 address oldPauseGuardian = pauseGuardian; // Store pauseGuardian with value newPauseGuardian pauseGuardian = newPauseGuardian; // Emit NewPauseGuardian(OldPauseGuardian, NewPauseGuardian) emit NewPauseGuardian(oldPauseGuardian, pauseGuardian);
scope: queryCantoBalance()
l26 uint treasuryCantoBalance = address(this).balance
scope: querynoteBalance()
l35 uint treasuryNoteBalance = note.balanceOf(address(this))
scope: _setImplementation
l34 address oldImplementation = implementation; implementation = implementation_; emit NewImplementation(oldImplementation, implementation_)
scope: _setBaseRatePerYear
l142 uint oldBaseRatePerYear = baseRatePerYear; baseRatePerYear = newBaseRateMantissa; emit NewBaseRate(oldBaseRatePerYear, baseRatePerYear)
scope: _setAdjusterCoefficient
l155 uint oldAdjusterCoefficient = adjusterCoefficient; adjusterCoefficient = newAdjusterCoefficient; emit NewAdjusterCoefficient(oldAdjusterCoefficient, adjusterCoefficient)
scope: _setUpdateFrequency
l168 uint oldUpdateFrequency = updateFrequency; updateFrequency = newUpdateFrequency; emit NewUpdateFrequency(oldUpdateFrequency, updateFrequency)
l42 Proposal memory prop = Proposal(propId, title, desc, targets, values, signatures, calldatas); l48 Proposal memory newProp = Proposal(propId, title, desc, targets, values, signatures, calldatas);
Manual Analysis
Remove the unnecessary local variables:
e.g:
address oldPendingAdmin = pendingAdmin; pendingAdmin = newPendingAdmin; emit NewPendingAdmin(oldPendingAdmin, newPendingAdmin);
with
emit NewPendingAdmin(pendingAdmin, newPendingAdmin); pendingAdmin = newPendingAdmin;
#0 - GalloDaSballo
2022-08-04T00:30:15Z
About 5.5k (4.2k immutables) rest is less than 1k