Gas Optimizations

[code423n4]

Gas Report

Table of Contents

• Caching storage variables in memory to save gas
• Calldata instead of memory for RO function parameters
• Comparison operators
• Constant expressions
• Constructor parameters should be avoided when possible
• Custom errors
• Default value initialization
• Event emitting of local variable
• Immutable variables save storage
• Mathematical optimizations
• Modifier instead of duplicate require
• Prefix increments
• Require instead of AND
• Return statements
• Revert strings length
• Tautologies
• Tight variable packing
• unchecked arithmetic
• unnecessary functions
• unnecessary computation

Caching storage variables in memory to save gas

IMPACT

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

PROOF OF CONCEPT

Instances include:

lending-market/GovernorBravoDelegate.sol

scope: queue()

• newProposal.targets.length is read newProposal.targets.length times

l68: i < newProposal.targets.length

scope: queueOrRevertInternal()

• timelock is read twice

l78: !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 times

l90: i < proposal.targets.length

scope: _acceptAdmin()

• pendingAdmin is read 3 times

l164: msg.sender == pendingAdmin
l168 address oldPendingAdmin = pendingAdmin
l171 admin = pendingAdmin

lending-market/Comptroller.sol

scope: queue()

• allMarkets.length is read allMarkets.length times

  l959 i < allMarkets.length

scope: _setBorrowPaused()

• admin is read twice

  l1062 msg.sender == admin
  l1063 msg.sender == admin

scope: _setTransferPaused()

• admin is read twice

  l1071 msg.sender == admin
  l1072 msg.sender == admin

scope: _setSeizePaused()

• admin is read twice

  l1080 msg.sender == admin
  l1081 msg.sender == admin

lending-market/AccountantDelegate.sol

scope: initialize()

• note is read 4 times

  l28 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 twice

  l48 require(msg.sender == address(cnote)
  l49 cnote.mint(amount)

scope: redeemMarket()

• cnote is read 3 times

  l60 require(msg.sender == address(cnote)
  l62 Exp({mantissa: cnote.exchangeRateStored()})
  l66 cnote.redeem(amtToRedeem

scope: sweepInterest()

• note is read 3 times

  l76 note.balanceOf(address(this))
  l81 sub_(note.totalSupply(), noteBalance)
  l87 note.transfer(treasury, amtToSweep)

• cnote is read 3 times

  l77 cnote.balanceOf(address(this))
  l79 Exp({mantissa: cnote.exchangeRateStored()})
  l89 cnote.transfer(address(0), CNoteBalance)

lending-market/cnote.sol

scope: _setAccountantContract()

• _accountant is read twice

  l15 address(_accountant)
  l18 emit AccountantSet(accountant_, address(_accountant))

scope: borrowFresh()

• _accountant is read twice

  l45 address(_accountant)
  l48 _accountant.supplyMarket(borrowAmount)

scope: repayBorrowFresh()

• _accountant is read twice

  l134 _accountant.redeemMarket(amtRedeemed)
  l141 payable(_accountant)

scope: mintFresh()

• _accountant is read 3 times

  l179 address(_accountant)
  l180 address(_accountant)
  l225 payable(_accountant)

scope: redeemFresh()

• _accountant is read 3 times

  l259 address(_accountant)
  l260 payable(_accountant)
  l324 _accountant.supplyMarket(redeemAmount)

• comptroller is read twice

  l294 comptroller.redeemAllowed(address(this), redeemer, redeemTokens)
  l343 comptroller.redeemVerify(address(this), redeemer, redeemAmount, redeemTokens)

stableswap/BaseV1-core.sol

scope: acceptPauser()

• pendingPauser is read twice

  l503 require(msg.sender == pendingPauser);
  l504 pauser = pendingPauser

TOOLS USED

Manual Analysis

MITIGATION

cache these storage variables in memory

Calldata instead of memory for RO function parameters

PROBLEM

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.

PROOF OF CONCEPT

Instances include:

lending-market/GovernorBravoDelegate.sol

scope: queueOrRevertInternal

l77 string memory signature, bytes memory data

lending-market/AccountantDelegator.sol

scope: delegateTo

l82 bytes memory data

scope: delegateToImplementation

l98 bytes memory data

scope: delegateToViewImplementation

l109 bytes memory data

lending-market/TreasuryDelegator.sol

scope: delegateToImplementation

73 bytes memory data

scope: delegateToViewImplementation

l84 bytes memory data

scope: delegateTo

l101 bytes memory data

TOOLS USED

Manual Analysis

MITIGATION

Replace memory with calldata

Comparison Operators

IMPACT

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

PROOF OF CONCEPT

Instances include:

lending-market/WETH.sol

l29: _balanceOf[msg.sender] >= wad
l72: _balanceOf[src] >= wad
l29: _allowance[src][msg.sender] >= wad

lending-market/GovernorBravoDelegate.sol

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")

lending-market/Comptroller.sol

l491 borrowBalance >= repayAmount
l1243 supplyIndex >= compInitialIndex
l1282 borrowIndex >= compInitialIndex
l1379 amount <= compRemaining

lending-market/AccountantDelegate.sol

l83 cNoteConverted >= noteDifferential

lending-market/CNote.sol

l130 getCashPrior() >= actualRepayAmount

stableswap/BaseV1-core.sol

l309 _k(_balance0, _balance1) >= _k(_reserve0, _reserve1), 'K'
l348 y - y_prev <= 1
l352 y_prev - y <= 1
l413 deadline >= block.timestamp

stableswap/BaseV1-periphery.sol

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

TOOLS USED

Manual Analysis

MITIGATION

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

IMPACT

Constant expressions are re-calculated each time it is in use, costing an extra 97 gas than a constant every time they are called.

PROOF OF CONCEPT

Instances include:

lending-market/GovernorBravoDelegate.sol

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)")

TOOLS USED

Manual Analysis

MITIGATION

Mark these as immutable instead of constant

Constructor parameters should be avoided when possible

IMPACT

Constructor parameters are expensive. The contract deployment will be cheaper in gas if they are hard coded instead of using constructor parameters.

PROOF OF CONCEPT

Instances include:

lending-market/WETH.sol

l14: _name = name_;
l15: _symbol = symbol_;

stableswap/BaseV1-core.sol

l490 isPaused = false //false is the default value of isPaused, this line can be removed altogether

stableswap/BaseV1-periphery.sol

l76 factory = _factory
l77 pairCodeHash = IBaseV1Factory(_factory).pairCodeHash()
l78 wcanto = IWCANTO(_wcanto)

TOOLS USED

Manual Analysis

MITIGATION

Hardcode storage variables with their initial value instead of writing it during contract deployment with constructor parameters.

Custom Errors

IMPACT

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

PROOF OF CONCEPT

Instances include:

lending-market/WETH.sol

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")

lending-market/GovernorBravoDelegate.sol

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")

lending-market/Comptroller.sol

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")

lending-market/AccountantDelegate.sol

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")

lending-market/AccountantDelegator.sol

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")

lending-market/TreasuryDelegator.sol

l31 require(msg.sender == admin, "GovernorBravoDelegator::_setImplementation: admin only")
l32 require(implementation_ != address(0), "GovernorBravoDelegator::_setImplementation: invalid implementation address")

lending-market/CNote.sol

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")

stableswap/BaseV1-core.sol

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')

stableswap/BaseV1-periphery.sol

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")

TOOLS USED

Manual Analysis

MITIGATION

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();

Default value initialization

IMPACT

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.

PROOF OF CONCEPT

Instances include:

lending-market/GovernorBravoDelegate.sol

l68 uint i = 0
l90 uint i = 0

lending-market/Comptroller.sol

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

BaseV1-core.sol

l46 uint public totalSupply = 0
l207 uint i = 0
l223 uint nextIndex = 0
l224 uint index = 0
l337 uint i = 0

BaseV1-periphery.sol

l136 uint i = 0
l158 uint _totalSupply = 0
l362 uint i = 0

TOOLS USED

Manual Analysis

MITIGATION

Remove explicit initialization for default values.

Event emitting of local variable

PROBLEM

When emitting an event, using a local variable instead of a storage variable saves gas.

PROOF OF CONCEPT

Instances include:

lending-market/GovernorBravoDelegate.sol

l176 emit NewAdmin(oldAdmin, admin);
l177 emit NewPendingAdmin(oldPendingAdmin, pendingAdmin)

lending-market/NoteInterest.sol

l75 emit NewInterestParams(baseRatePerBlock)
l127 emit NewInterestParams(baseRatePerYear)
l144 emit NewBaseRate(oldBaseRatePerYear, baseRatePerYear)
l157 emit NewAdjusterCoefficient(oldAdjusterCoefficient, adjusterCoefficient)
l170 emit NewUpdateFrequency(oldUpdateFrequency, updateFrequency)

stableswap/BaseV1-core.sol

l170 emit Sync(reserve0, reserve1)

TOOLS USED

Manual Analysis

MITIGATION

Emit a local variable or function argument instead of storage variable

Immutable variables save storage

PROBLEM

If a variable is set in the constructor and never modified afterwrds, marking it as immutable can save a storage operation - 20,000 gas.

PROOF OF CONCEPT

Instances include:

lending-market/WETH.sol

l14: _name = name_;
l15: _symbol = symbol_;

TOOLS USED

Manual Analysis

MITIGATION

Mark these variables as immutable.

Mathematical optimizations

PROBLEM

X += Y costs 22 more gas than X = X + Y.

PROOF OF CONCEPT

Instances include:

lending-market/WETH.sol

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

stableswap/BaseV1-core.sol

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

TOOLS USED

Manual Analysis

MITIGATION

use X = X + Y instead of X += Y (same with -)

Modifier instead of duplicate require

PROBLEM

When a require statement is use multiple times, it is cheaper to use a modifier instead.

PROOF OF CONCEPT

Instances include:

lending-market/Comptroller.sol

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")

stableswap/BaseV1-core.sol

l498 require(msg.sender == pauser)
l508 require(msg.sender == pauser)

TOOLS USED

Manual Analysis

MITIGATION

Use modifiers for these repeated statements

Prefix increments

IMPACT

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

PROOF OF CONCEPT

Instances include:

lending-market/GovernorBravoDelegate.sol

l68 i++
l90 i++

lending-market/Comptroller.sol

l126 i++
l206 i++
l735 i++
l959 i++
l1005 i++
l1106 i++
l1347 i++
l1353 j++
l1359 j++
l1364 j++

stableswap/BaseV1-core.sol

l207 i++
l337 i++

stableswap/BaseV1-periphery.sol

l136 i++
l362 i++

TOOLS USED

Manual Analysis

MITIGATION

change variable++ to ++variable.

Require instead of AND

IMPACT

Require statements including conditions with the && operator can be broken down in multiple require statements to save gas.

PROOF OF CONCEPT

Instances include:

lending-market/GovernorBravoDelegate.sol

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")

lending-market/Comptroller.sol

l1003 require(numMarkets != 0 && numMarkets == numBorrowCaps, "invalid input")
l1411 require(numTokens == supplySpeeds.length && numTokens == borrowSpeeds.length, "Comptroller::_setCompSpeeds invalid input")

stableswap/BaseV1-core.sol

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))))

stableswap/BaseV1-periphery.sol

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")

TOOLS USED

Manual Analysis

MITIGATION

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

Return statements

IMPACT

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.

PROOF OF CONCEPT

Instances include:

lending-market/GovernorBravoDelegate.sol

l106 return (p.targets, p.values, p.signatures, p.calldatas)

stableswap/BaseV1-core.sol

l140 return (decimals0, decimals1, reserve0, reserve1, stable, token0, token1)
l210 return priceAverageCumulative / granularity

stableswap/BaseV1-periphery.sol

l128 return amountStable > amountVolatile ? (amountStable, true) : (amountVolatile, false)

TOOLS USED

Manual Analysis

MITIGATION

Replace the return statements as explained, using a local variable with the named return instead.

Revert strings length

IMPACT

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.

PROOF OF CONCEPT

Revert strings exceeding 32 bytes include:

lending-market/WETH.sol

l29 sender balance insufficient for withdrawal
l96 ERC20: approve from the zero address
l97 ERC20: approve to the zero address

lending-market/GovernorBravoDelegate.sol

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")

lending-market/Comptroller.sol

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")

lending-market/AccountantDelegate.sol

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")

lending-market/AccountantDelegator.sol

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")

lending-market/TreasuryDelegator.sol

l31 require(msg.sender == admin, "GovernorBravoDelegator::_setImplementation: admin only")
l32 require(implementation_ != address(0), "GovernorBravoDelegator::_setImplementation: invalid implementation address")

lending-market/CNote.sol

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")

stableswap/BaseV1-periphery.sol

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')

TOOLS USED

Manual Analysis

MITIGATION

Write the error strings so that they do not exceed 32 bytes. For further gas savings, consider also using custom errors.

Tautologies

PROBLEM

Tautologies should be avoided as they waste gas.

PROOF OF CONCEPT

Instances include:

lending-market/NoteInterest.sol

l97 uint newRatePerYear = ir >= 0 ? ir : 0

ir is a uint256, hence always >= 0.

TOOLS USED

Manual Analysis

MITIGATION

Replace

uint newRatePerYear = ir >= 0 ? ir : 0;

with

uint newRatePerYear = ir;

Tight Variable Packing

PROBLEM

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

PROOF OF CONCEPT

Instances include:

stableswap/BaseV1-core.sol

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;

TOOLS USED

Manual Analysis

MITIGATION

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;

Unchecked arithmetic

IMPACT

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

PROOF OF CONCEPT

Instances include:

lending-market/WETH.sol

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

lending-market/GovernorBravoDelegate.sol

l68 i++
l90 i++

lending-market/Comptroller.sol

l126 i++
l206 i++
l735 i++
l959 i++
l1005 i++
l1106 i++
l1347 i++
l1353 j++
l1359 j++
l1364 j++
l1413 ++i

stableswap/BaseV1-core.sol

l156 uint timeElapsed = blockTimestamp - blockTimestampLast //as per comment, underflow desired
l183 uint timeElapsed = blockTimestamp - _blockTimestampLast //as per comment, underflow desired
l207 i++
l337 i++

stableswap/BaseV1-periphery.sol

l136 i++
l276 msg.value - amountCANTO //cannot underflow because of check line 276
l362 i++

TOOLS USED

Manual Analysis

MITIGATION

Place the arithmetic operations in an unchecked block

Unnecessary functions

IMPACT

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.

PROOF OF CONCEPT

Instances include:

lending-market/GovernorBravoDelegate.sol

180 function add256
186 function sub256

lending-market/NoteInterest.sol

using SafeMath for uint

TOOLS USED

Manual Analysis

MITIGATION

Remove these functions

Unnecessary computation

IMPACT

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.

PROOF OF CONCEPT

Instances include:

lending-market/GovernorBravoDelegate.sol

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)

lending-market/Comptroller.sol

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);

lending-market/TreasuryDelegate.sol

scope: queryCantoBalance()

l26 uint treasuryCantoBalance = address(this).balance

scope: querynoteBalance()

l35 uint treasuryNoteBalance = note.balanceOf(address(this))

lending-market/TreasuryDelegator.sol

scope: _setImplementation

l34 address oldImplementation = implementation;
implementation = implementation_;

emit NewImplementation(oldImplementation, implementation_)

lending-market/NoteInterest.sol

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)

manifest/Proposal-Store.sol

l42 Proposal memory prop = Proposal(propId, title, desc, targets, values, signatures, calldatas);
l48 Proposal memory newProp = Proposal(propId, title, desc, targets, values, signatures, calldatas);

TOOLS USED

Manual Analysis

MITIGATION

Remove the unnecessary local variables:

• write to the storage variable directly instead of using an intermediate local variable
• Emit the event before writing to the storage variable to avoid the one local variable computation

e.g:

address oldPendingAdmin = pendingAdmin;
pendingAdmin = newPendingAdmin;
emit NewPendingAdmin(oldPendingAdmin, newPendingAdmin);

with

emit NewPendingAdmin(pendingAdmin, newPendingAdmin);
pendingAdmin = newPendingAdmin;

[GalloDaSballo]

About 5.5k (4.2k immutables) rest is less than 1k