Yeti Finance contest - robee's results

Handle

robee

Vulnerability details

Some tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value.They must first be approved by zero and then the actual allowance must be approved. You don't first approve 0 in the following places in the codebase:

    approve without approving 0 first WJLP.sol, 127, JLP.approve(address(_MasterChefJoe), _amount); 
    approve without approving 0 first EchidnaProxy.sol, 134, return yusdToken.approve(spender, amount); 
    approve without approving 0 first sYETIToken.sol, 229, yusdToken.approve(routerAddress, YUSDToSell); 

Handle

robee

Vulnerability details

Need to use safeTransfer instead of transfer. As there are popular tokens, such as USDT that transfer/trasnferFrom method doesn’t return anything. The transfer return value has to be checked (as there are some other tokens that returns false instead revert), that means you must

1. Check the transfer return value Another popular possibility is to add a whiteList. Those are the appearances (solidity file, line number, actual line):

   YetiFinanceTreasury.sol, 24, _token.transfer(_to, _amount); 
   StabilityPool.sol, 946, IERC20(assets[i]).transfer(_to, amounts[i]); 
   

Handle

robee

Vulnerability details

The project is compiled with different versions of solidity, which is not recommended due ti undefined behaviors as a result of it.

Handle

robee

Vulnerability details

In the following files there are contract imports that aren't used. Import of unnecessary files costs deployment gas (and is a bad coding practice that is important to ignore). The following is a full list of all unused imports, we went through the whole code to find it :) <solidity file> <line number> <actual import line>:

    ActivePool.sol, line 6, import "./Interfaces/IStabilityPool.sol";
    ActivePool.sol, line 7, import "./Interfaces/IDefaultPool.sol";
    ActivePool.sol, line 13, import "./Dependencies/LiquityBase.sol";
    ERC20_8.sol, line 3, import "./Interfaces/IERC20_8.sol";
    IMasterChefJoeV2.sol, line 1, import "./IERC20_8.sol";
    IRewarder.sol, line 3, import "./IERC20_8.sol";
    LiquityBase.sol, line 8, import "../Interfaces/IWhitelist.sol";
    TroveManagerBase.sol, line 12, import "../Interfaces/IWhitelist.sol";
    Whitelist.sol, line 13, import "../Interfaces/IERC20.sol";
    Whitelist.sol, line 14, import "./LiquityMath.sol";
    YetiCustomBase.sol, line 6, import "../Interfaces/IERC20.sol";
    ICollSurplusPool.sol, line 4, import "../Dependencies/YetiCustomBase.sol";
    ILiquityBase.sol, line 4, import "./IPriceFeed.sol";
    MockAggregator.sol, line 5, import "hardhat/console.sol";
    NonPayable.sol, line 4, //import "hardhat/console.sol";
    SortedTrovesTester.sol, line 4, import "../Interfaces/ISortedTroves.sol";
    TroveManagerLiquidations.sol, line 6, import "hardhat/console.sol";
    sYETIToken.sol, line 9, import "./BoringCrypto/BoringBatchable.sol";

Handle

robee

Vulnerability details

The following require messages are of length more than 32 and we think are short enough to short them into exactly 32 characters such that it will be placed in one slot of memory and the require function will cost less gas. The list:

    Solidity file: ActivePool.sol, In line 252, Require message length to shorten: 35, The message: ActivePool: Caller is not whitelist
    Solidity file: BorrowerOperations.sol, In line 215, Require message length to shorten: 39, The message: BOps: colls and amounts length mismatch
    Solidity file: BorrowerOperations.sol, In line 874, Require message length to shorten: 33, The message: BOps: Collateral not in whitelist
    Solidity file: CollSurplusPool.sol, In line 167, Require message length to shorten: 40, The message: CollSurplusPool: Caller is not Whitelist
    Solidity file: DefaultPool.sol, In line 122, Require message length to shorten: 38, The message: DefaultPool: sending collateral failed
    Solidity file: DefaultPool.sol, In line 167, Require message length to shorten: 36, The message: DefaultPool: Caller is not whitelist
    Solidity file: LiquitySafeMath128.sol, In line 10, Require message length to shorten: 37, The message: LiquitySafeMath128: addition overflow
    Solidity file: LiquitySafeMath128.sol, In line 16, Require message length to shorten: 40, The message: LiquitySafeMath128: subtraction overflow
    Solidity file: SafeMath.sol, In line 87, Require message length to shorten: 33, The message: SafeMath: multiplication overflow
    Solidity file: Address.sol, In line 115, Require message length to shorten: 38, The message: Address: insufficient balance for call
    Solidity file: Address.sol, In line 140, Require message length to shorten: 36, The message: Address: static call to non-contract
    Solidity file: SortedTroves.sol, In line 131, Require message length to shorten: 34, The message: SortedTroves: ICR must be positive
    Solidity file: SortedTroves.sol, In line 230, Require message length to shorten: 34, The message: SortedTroves: ICR must be positive
    Solidity file: StabilityPool.sol, In line 1076, Require message length to shorten: 39, The message: StabilityPool: Caller is not ActivePool
    Solidity file: StabilityPool.sol, In line 1095, Require message length to shorten: 40, The message: StabilityPool: User must have no deposit
    Solidity file: StabilityPool.sol, In line 1099, Require message length to shorten: 38, The message: StabilityPool: Amount must be non-zero
    Solidity file: StabilityPool.sol, In line 1138, Require message length to shorten: 36, The message: DefaultPool: Caller is not whitelist
    Solidity file: SortedTrovesTester.sol, In line 23, Require message length to shorten: 34, The message: SortedTroves: ICR must be positive
    Solidity file: TroveManagerLiquidations.sol, In line 180, Require message length to shorten: 34, The message: TroveManager: nothing to liquidate
    Solidity file: TroveManagerRedemptions.sol, In line 520, Require message length to shorten: 34, The message: must be non zero redemption amount
    Solidity file: CommunityIssuance.sol, In line 131, Require message length to shorten: 35, The message: CommunityIssuance: caller is not SP
    Solidity file: YETIToken.sol, In line 198, Require message length to shorten: 36, The message: YETI: transfer from the zero address
    Solidity file: YETIToken.sol, In line 222, Require message length to shorten: 39, The message: YETI: caller must be the SYETI contract
    Solidity file: YUSDToken.sol, In line 294, Require message length to shorten: 37, The message: YUSD: Caller is not the StabilityPool

Handle

robee

Vulnerability details

Reading a storage variable is gas costly (SLOAD). In cases of multiple read of a storage variable in the same scope, caching the first read (i.e saving as a local variable) can save gas and decrease the overall gas uses. The following is a list of functions and the storage variables that you read twice:

    WJLP.sol Variable activePool is read 2 times in the function: unwrapFor
    WJLP.sol Variable stabilityPool is read 2 times in the function: unwrapFor
    Pool2Unipool.sol Variable periodFinish is read 2 times in the function: _notifyRewardAmount
    Pool2Unipool.sol Variable periodFinish is read 2 times in the function: _updatePeriodFinish
    Unipool.sol Variable periodFinish is read 2 times in the function: _notifyRewardAmount
    Unipool.sol Variable periodFinish is read 2 times in the function: _updatePeriodFinish
    PriceFeed.sol Variable lastGoodPrice is read 17 times in the function: fetchPrice
    PriceFeed.sol Variable lastGoodPrice is read 17 times in the function: fetchPrice_v
    EchidnaTester.sol Variable troveManager is read 9 times in the function: constructor
    EchidnaTester.sol Variable borrowerOperations is read 8 times in the function: constructor
    EchidnaTester.sol Variable activePool is read 6 times in the function: constructor
    EchidnaTester.sol Variable defaultPool is read 4 times in the function: constructor
    EchidnaTester.sol Variable stabilityPool is read 6 times in the function: constructor
    EchidnaTester.sol Variable gasPool is read 3 times in the function: constructor
    EchidnaTester.sol Variable troveManagerLiquidations is read 4 times in the function: constructor
    EchidnaTester.sol Variable troveManagerRedemptions is read 5 times in the function: constructor
    EchidnaTester.sol Variable yusdToken is read 5 times in the function: constructor
    EchidnaTester.sol Variable whitelist is read 4 times in the function: constructor
    EchidnaTester.sol Variable collSurplusPool is read 4 times in the function: constructor
    EchidnaTester.sol Variable priceFeedTestnet is read 3 times in the function: constructor
    EchidnaTester.sol Variable sortedTroves is read 4 times in the function: constructor
    EchidnaTester.sol Variable MCR is read 2 times in the function: constructor
    EchidnaTester.sol Variable CCR is read 2 times in the function: constructor
    ERC20.sol Variable totalSupply is read 2 times in the function: _mint
    TeamLockup.sol Variable totalClaimed is read 2 times in the function: claimYeti
    YETIToken.sol Variable _totalSupply is read 2 times in the function: constructor

Handle

robee

Vulnerability details

There are places in the code (especially in for-each loops) that loads the same array element more than once. In such cases, only one array boundaries check should take place, and the rest are unnecessary. Therefore, this array element should be cached in a local variable and then be loaded again using this local variable, skipping the redundent second array boundaries check:

    BorrowerOperations.sol, variable name: _colls times: 2 at: _requireNoDuplicateColls
    BorrowerOperations.sol, variable name: _indices times: 2 at: _requireRouterAVAXIndicesInOrder
    DefaultPool.sol, variable name: _amounts times: 2 at: sendCollsToActivePool
    YetiCustomBase.sol, variable name: _amounts times: 2 at: _subColls
    StabilityPool.sol, variable name: amounts times: 2 at: _sendGainsToDepositor
    TroveManager.sol, variable name: lastCollError_Redistribution times: 2 at: redistributeDebtAndColl
    TroveManager.sol, variable name: lastYUSDDebtError_Redistribution times: 2 at: redistributeDebtAndColl
    TroveManager.sol, variable name: totalStakes times: 2 at: redistributeDebtAndColl
    TroveManager.sol, variable name: allColls times: 2 at: _closeTrove

Handle

robee

Vulnerability details

Prefix increments are cheaper than postfix increments. Further more, using unchecked {++x} is even more gas efficient, and the gas saving accumulates every iteration and can make a real change There is no risk of overflow caused by increamenting the iteration index in for loops (the ++i in for (uint256 i = 0; i < numIterations; ++i)). But increments perform overflow checks that are not necessary in this case. These functions use not using prefix increments (++x) or not using the unchecked keyword:

    change to prefix increment and unchecked: ActivePool.sol, i, 133
    change to prefix increment and unchecked: ActivePool.sol, i, 167
    change to prefix increment and unchecked: ActivePool.sol, i, 184
    change to prefix increment and unchecked: BorrowerOperations.sol, i, 699
    change to prefix increment and unchecked: BorrowerOperations.sol, i, 737
    change to prefix increment and unchecked: BorrowerOperations.sol, i, 873
    change to prefix increment and unchecked: BorrowerOperations.sol, i, 890
    change to prefix increment and unchecked: BorrowerOperations.sol, i, 920
    change to prefix increment and unchecked: BorrowerOperations.sol, i, 929
    change to prefix increment and unchecked: BorrowerOperations.sol, i, 1061
    change to prefix increment and unchecked: BorrowerOperations.sol, i, 1068
    change to prefix increment and unchecked: DefaultPool.sol, i, 97
    change to prefix increment and unchecked: DefaultPool.sol, i, 133
    change to prefix increment and unchecked: LiquityBase.sol, i, 63
    change to prefix increment and unchecked: LiquityBase.sol, i, 97
    change to prefix increment and unchecked: LiquityBase.sol, i, 106
    change to prefix increment and unchecked: LiquityBase.sol, i, 115
    change to prefix increment and unchecked: LiquityBase.sol, i, 149
    change to prefix increment and unchecked: LiquityBase.sol, i, 158
    change to prefix increment and unchecked: LiquityBase.sol, i, 168
    change to prefix increment and unchecked: YetiCustomBase.sol, i, 36
    change to prefix increment and unchecked: YetiCustomBase.sol, i, 44
    change to prefix increment and unchecked: YetiCustomBase.sol, i, 59
    change to prefix increment and unchecked: YetiCustomBase.sol, i, 104
    change to prefix increment and unchecked: YetiCustomBase.sol, i, 122
    change to prefix increment and unchecked: YetiCustomBase.sol, i, 144
    change to prefix increment and unchecked: YetiCustomBase.sol, i, 152
    change to prefix increment and unchecked: YetiCustomBase.sol, i, 165
    change to prefix increment and unchecked: YetiCustomBase.sol, i, 178
    change to prefix increment and unchecked: HintHelpers.sol, i, 139
    just change to unchecked: MultiTroveGetter.sol, idx, 73
    just change to unchecked: MultiTroveGetter.sol, idx, 79
    just change to unchecked: MultiTroveGetter.sol, idx, 90
    just change to unchecked: MultiTroveGetter.sol, idx, 96
    change to prefix increment and unchecked: MultiTroveGetter.sol, i, 110
    change to prefix increment and unchecked: StabilityPool.sol, i, 562
    change to prefix increment and unchecked: StabilityPool.sol, i, 588
    change to prefix increment and unchecked: StabilityPool.sol, i, 592
    change to prefix increment and unchecked: StabilityPool.sol, i, 628
    change to prefix increment and unchecked: StabilityPool.sol, i, 720
    change to prefix increment and unchecked: StabilityPool.sol, i, 942
    change to prefix increment and unchecked: StabilityPool.sol, i, 1011
    change to prefix increment and unchecked: TeamAllocation.sol, i, 66
    change to prefix increment and unchecked: CDPManagerTester.sol, i, 14
    change to prefix increment and unchecked: EchidnaTester.sol, i, 103
    change to prefix increment and unchecked: EchidnaTester.sol, i, 121
    change to prefix increment and unchecked: TroveManager.sol, i, 234
    change to prefix increment and unchecked: TroveManager.sol, i, 348
    change to prefix increment and unchecked: TroveManager.sol, i, 374
    change to prefix increment and unchecked: TroveManager.sol, i, 397
    change to prefix increment and unchecked: TroveManager.sol, i, 420
    change to prefix increment and unchecked: TroveManager.sol, i, 460
    change to prefix increment and unchecked: TroveManager.sol, i, 476
    change to prefix increment and unchecked: TroveManager.sol, i, 525
    change to prefix increment and unchecked: TroveManager.sol, i, 582
    change to prefix increment and unchecked: TroveManager.sol, i, 603
    change to prefix increment and unchecked: TroveManagerLiquidations.sol, vars.i, 255
    change to prefix increment and unchecked: TroveManagerLiquidations.sol, vars.i, 334
    change to prefix increment and unchecked: TroveManagerLiquidations.sol, i, 394
    change to prefix increment and unchecked: TroveManagerLiquidations.sol, i, 475
    change to prefix increment and unchecked: TroveManagerLiquidations.sol, i, 808
    change to prefix increment and unchecked: TroveManagerLiquidations.sol, i, 840
    change to prefix increment and unchecked: TroveManagerRedemptions.sol, i, 304
    change to prefix increment and unchecked: TroveManagerRedemptions.sol, i, 517

Handle

robee

Vulnerability details

In the following files there are state variables that could be set immutable to save gas. The list of format <solidity file>, <state variable name that could be immutable>:

    tellor in TellorCaller.sol
    owner in Migrations.sol
    troveManager in MultiTroveGetter.sol
    sortedTroves in MultiTroveGetter.sol
    whitelist in MultiTroveGetter.sol
    team in TeamAllocation.sol
    allocations in TeamAllocation.sol
    _symbol in ERC20Token.sol
    _name in ERC20Token.sol
    _decimals in ERC20Token.sol
    _totalSupply in ERC20Token.sol
    _DOMAIN_SEPARATOR in Domain.sol
    DOMAIN_SEPARATOR_CHAIN_ID in Domain.sol
    deploymentTime in CommunityIssuance.sol
    YETI in TeamLockup.sol
    vestingStart in TeamLockup.sol
    vestingLength in TeamLockup.sol
    totalVest in TeamLockup.sol

Handle

robee

Vulnerability details

The following functions are payable but msg.value isn't used - therefore the function payable state modifier isn't necessary. Payable functions are more gas expensive than others, and it's danger the users if they send ETH by mistake.

    EchidnaProxy.sol, openTrovePrx is payable but doesn't use msg.value
    EchidnaProxy.sol, addCollPrx is payable but doesn't use msg.value
    NonPayable.sol, forward is payable but doesn't use msg.value

Handle

robee

Vulnerability details

Use bytes32 instead of string to save gas whenever possible. String is a dynamic data structure and therefore is more gas consuming then bytes32. You could use bytes32 instead of string in the following places: List format: <solidity file> <line number> <actual code line>

    ActivePool.sol, 27, string constant public NAME = "ActivePool"; 
    BorrowerOperations.sol, 23, string public constant NAME = "BorrowerOperations"; 
    CollSurplusPool.sol, 19, string public constant NAME = "CollSurplusPool"; 
    DefaultPool.sol, 25, string public constant NAME = "DefaultPool"; 
    HintHelpers.sol, 17, string constant public NAME = "HintHelpers"; 
    Pool2Unipool.sol, 74, string constant public NAME = "Pool2Unipool"; 
    Unipool.sol, 74, string constant public NAME = "Unipool"; 
    PriceFeed.sol, 25, string constant public NAME = "PriceFeed"; 
    SortedTroves.sol, 52, string constant public NAME = "SortedTroves"; 
    StabilityPool.sol, 153, string public constant NAME = "StabilityPool"; 
    TroveManager.sol, 24, string constant public NAME = "TroveManager"; 
    Domain.sol, 11, string private constant EIP191_PREFIX_FOR_EIP712_STRUCTURED_DATA = "\x19\x01"; 
    CommunityIssuance.sol, 18, string constant public NAME = "CommunityIssuance"; 
    LockupContract.sol, 22, string constant public NAME = "LockupContract"; 
    LockupContractFactory.sol, 28, string constant public NAME = "LockupContractFactory"; 
    ShortLockupContract.sol, 22, string constant public NAME = "LockupContract"; 
    sYETIToken.sol, 34, string public constant symbol = "sYETI"; 
    sYETIToken.sol, 35, string public constant name = "Staked YETI Tokens"; 
    YETIToken.sol, 38, string constant internal _NAME = "Yeti Finance"; 
    YETIToken.sol, 39, string constant internal _SYMBOL = "YETI"; 
    YETIToken.sol, 40, string constant internal _VERSION = "1"; 
    YUSDToken.sol, 30, string constant internal _NAME = "YUSD Stablecoin"; 
    YUSDToken.sol, 31, string constant internal _SYMBOL = "YUSD"; 
    YUSDToken.sol, 32, string constant internal _VERSION = "1"; 

Handle

robee

Vulnerability details

Users can mistakenly think that the return value is the named return, but it is actually the actualreturn statement that comes after. To know that the user needs to read the code and is confusing. Furthermore, removing either the actual return or the named return will save gas.

    ActivePool.sol, getVC
    ERC20_8.sol, balanceOf
    ERC20_8.sol, allowance
    ERC20_8.sol, transfer
    ERC20_8.sol, transferFrom
    ERC20_8.sol, _mint
    ERC20_8.sol, _burn
    BorrowerOperations.sol, _getTotalVariableDepositFee
    BorrowerOperations.sol, _getNewPortfolio
    BorrowerOperations.sol, _getNewPortfolio
    DefaultPool.sol, getVC
    LiquityBase.sol, getEntireSystemColl
    LiquityBase.sol, getEntireSystemDebt
    LiquityBase.sol, _getICRColls
    LiquityBase.sol, _getVC
    LiquityBase.sol, _getVCColls
    LiquityBase.sol, _getUSDColls
    LiquityBase.sol, _getTCR
    LiquityBase.sol, _getColls
    TellorCaller.sol, getTellorCurrentValue
    TellorCaller.sol, getTellorCurrentValue
    TellorCaller.sol, getTellorCurrentValue
    Whitelist.sol, getFee
    HintHelpers.sol, _calculateVCAfterRedemption
    HintHelpers.sol, getApproxHint
    HintHelpers.sol, getApproxHint
    HintHelpers.sol, getApproxHint
    ThreePieceWiseLinearPriceCurve.sol, getFeeCapAndTime
    ThreePieceWiseLinearPriceCurve.sol, getFeeCapAndTime
    ThreePieceWiseLinearPriceCurve.sol, getFee
    ThreePieceWiseLinearPriceCurve.sol, getFeeAndUpdate
    ThreePieceWiseLinearPriceCurve.sol, _getFeePoint
    ThreePieceWiseLinearPriceCurve.sol, calculateDecayedFee
    PriceFeed.sol, _getCurrentTellorResponse
    PriceFeed.sol, _getCurrentChainlinkResponse
    PriceFeed.sol, _getPrevChainlinkResponse
    StabilityPool.sol, _computeRewardsPerUnitStaked
    StabilityPool.sol, _computeRewardsPerUnitStaked
    StabilityPool.sol, getDepositorGains
    StabilityPool.sol, getDepositorGains
    StabilityPool.sol, _calculateGains
    StabilityPool.sol, _calculateGains
    EchidnaTester.sol, _getVC
    MockAggregator.sol, latestRoundData
    MockAggregator.sol, latestRoundData
    MockAggregator.sol, latestRoundData
    MockAggregator.sol, latestRoundData
    MockAggregator.sol, latestRoundData
    MockAggregator.sol, getRoundData
    MockAggregator.sol, getRoundData
    MockAggregator.sol, getRoundData
    MockAggregator.sol, getRoundData
    MockAggregator.sol, getRoundData
    sYETITokenTester.sol, getUserInfo
    sYETITokenTester.sol, getUserInfo
    ERC20Token.sol, balanceOf
    ERC20Token.sol, transfer
    ERC20Token.sol, mint
    ERC20Token.sol, approve
    ERC20Token.sol, transferFrom
    ERC20Token.sol, allowance
    YETITokenTester.sol, getChainId
    YUSDTokenTester.sol, getChainId
    TroveManager.sol, _getCurrentTroveState
    TroveManager.sol, _getCurrentTroveState
    TroveManager.sol, _getPendingCollRewards
    TroveManager.sol, getPendingYUSDDebtReward
    TroveManager.sol, addTroveOwnerToArray
    TroveManager.sol, _addTroveOwnerToArray
    TroveManagerLiquidations.sol, _liquidateNormalMode
    TroveManagerLiquidations.sol, _liquidateRecoveryMode
    TroveManagerLiquidations.sol, _addLiquidationValuesToTotals
    TroveManagerRedemptions.sol, _redeemCollateralFromTrove

Handle

robee

Vulnerability details

In for loops you initialize the index to start from 0, but it already initialized to 0 in default and this assignment cost gas. It is more clear and gas efficient to declare without assigning 0 and will have the same meaning:

    ActivePool.sol, 133
    ActivePool.sol, 167
    ActivePool.sol, 184
    BorrowerOperations.sol, 699
    BorrowerOperations.sol, 737
    BorrowerOperations.sol, 873
    BorrowerOperations.sol, 890
    BorrowerOperations.sol, 920
    BorrowerOperations.sol, 929
    BorrowerOperations.sol, 1061
    BorrowerOperations.sol, 1068
    DefaultPool.sol, 97
    DefaultPool.sol, 133
    LiquityBase.sol, 63
    LiquityBase.sol, 97
    LiquityBase.sol, 106
    LiquityBase.sol, 115
    LiquityBase.sol, 149
    LiquityBase.sol, 158
    LiquityBase.sol, 168
    YetiCustomBase.sol, 36
    YetiCustomBase.sol, 44
    YetiCustomBase.sol, 59
    YetiCustomBase.sol, 104
    YetiCustomBase.sol, 122
    YetiCustomBase.sol, 144
    YetiCustomBase.sol, 152
    YetiCustomBase.sol, 165
    YetiCustomBase.sol, 178
    HintHelpers.sol, 139
    MultiTroveGetter.sol, 73
    MultiTroveGetter.sol, 79
    MultiTroveGetter.sol, 90
    MultiTroveGetter.sol, 96
    MultiTroveGetter.sol, 110
    StabilityPool.sol, 562
    StabilityPool.sol, 588
    StabilityPool.sol, 592
    StabilityPool.sol, 628
    StabilityPool.sol, 720
    StabilityPool.sol, 942
    StabilityPool.sol, 1011
    TeamAllocation.sol, 66
    CDPManagerTester.sol, 14
    EchidnaTester.sol, 103
    EchidnaTester.sol, 121
    TroveManager.sol, 234
    TroveManager.sol, 348
    TroveManager.sol, 374
    TroveManager.sol, 397
    TroveManager.sol, 420
    TroveManager.sol, 460
    TroveManager.sol, 476
    TroveManager.sol, 525
    TroveManager.sol, 582
    TroveManager.sol, 603
    TroveManagerLiquidations.sol, 394
    TroveManagerLiquidations.sol, 475
    TroveManagerLiquidations.sol, 808
    TroveManagerLiquidations.sol, 840
    TroveManagerRedemptions.sol, 304
    TroveManagerRedemptions.sol, 517

Handle

robee

Vulnerability details

The following functions could be set external to save gas and improve code quality. External call cost is less expensive than of public functions.

    The function getCollateral in ActivePool.sol could be set external
    The function getAllCollateral in ActivePool.sol could be set external
    The function name in ERC20_8.sol could be set external
    The function transferFrom in ERC20_8.sol could be set external
    The function decimals in ERC20_8.sol could be set external
    The function symbol in ERC20_8.sol could be set external
    The function totalSupply in ERC20_8.sol could be set external
    The function allowance in ERC20_8.sol could be set external
    The function transfer in ERC20_8.sol could be set external
    The function balanceOf in ERC20_8.sol could be set external
    The function getCollateral in CollSurplusPool.sol could be set external
    The function getAllCollateral in CollSurplusPool.sol could be set external
    The function getCollateral in DefaultPool.sol could be set external
    The function getAllCollateral in DefaultPool.sol could be set external
    The function getEntireSystemColl in LiquityBase.sol could be set external
    The function getEntireSystemDebt in LiquityBase.sol could be set external
    The function owner in Ownable.sol could be set external
    The function isOwner in Ownable.sol could be set external
    The function constructor in TellorCaller.sol could be set external
    The function getIndex in Whitelist.sol could be set external
    The function getRatio in Whitelist.sol could be set external
    The function getPrice in Whitelist.sol could be set external
    The function getValueVC in Whitelist.sol could be set external
    The function withdraw in Pool2Unipool.sol could be set external
    The function stake in Pool2Unipool.sol could be set external
    The function claimReward in Pool2Unipool.sol could be set external
    The function approveInternal in ERC20Mock.sol could be set external
    The function constructor in ERC20Mock.sol could be set external
    The function mint in ERC20Mock.sol could be set external
    The function burn in ERC20Mock.sol could be set external
    The function transferInternal in ERC20Mock.sol could be set external
    The function withdraw in Unipool.sol could be set external
    The function stake in Unipool.sol could be set external
    The function claimReward in Unipool.sol could be set external
    The function upgrade in Migrations.sol could be set external
    The function setCompleted in Migrations.sol could be set external
    The function constructor in Migrations.sol could be set external
    The function constructor in MultiTroveGetter.sol could be set external
    The function setAddresses in ThreePieceWiseLinearPriceCurve.sol could be set external
    The function adjustParams in ThreePieceWiseLinearPriceCurve.sol could be set external
    The function calculateDecayedFee in ThreePieceWiseLinearPriceCurve.sol could be set external
    The function setDecayTime in ThreePieceWiseLinearPriceCurve.sol could be set external
    The function contains in SortedTroves.sol could be set external
    The function isEmpty in SortedTroves.sol could be set external
    The function isFull in SortedTroves.sol could be set external
    The function getFrontEndYETIGain in StabilityPool.sol could be set external
    The function getDepositorGains in StabilityPool.sol could be set external
    The function getAllCollateral in StabilityPool.sol could be set external
    The function getDepositorYETIGain in StabilityPool.sol could be set external
    The function getCompoundedYUSDDeposit in StabilityPool.sol could be set external
    The function getCompoundedFrontEndStake in StabilityPool.sol could be set external
    The function constructor in TeamAllocation.sol could be set external
    The function setGreeting in TellorDummy.sol could be set external
    The function greet in TellorDummy.sol could be set external
    The function constructor in TellorDummy.sol could be set external
    The function write in proxy.sol could be set external
    The function read in proxy.sol could be set external
    The function constructor in EchidnaProxy.sol could be set external
    The function constructor in EchidnaTester.sol could be set external
    The function getUserInfo in sYETITokenTester.sol could be set external
    The function name in ERC20Token.sol could be set external
    The function constructor in ERC20Token.sol could be set external
    The function transferFrom in ERC20Token.sol could be set external
    The function decimals in ERC20Token.sol could be set external
    The function symbol in ERC20Token.sol could be set external
    The function mint in ERC20Token.sol could be set external
    The function totalSupply in ERC20Token.sol could be set external
    The function approve in ERC20Token.sol could be set external
    The function allowance in ERC20Token.sol could be set external
    The function transfer in ERC20Token.sol could be set external
    The function balanceOf in ERC20Token.sol could be set external
    The function constructor in YETITokenTester.sol could be set external
    The function constructor in YUSDTokenTester.sol could be set external
    The function getPendingYUSDDebtReward in TroveManager.sol could be set external
    The function getRedemptionRateWithDecay in TroveManager.sol could be set external
    The function batchLiquidateTroves in TroveManager.sol could be set external
    The function getCurrentICR in TroveManager.sol could be set external
    The function getRedemptionRate in TroveManager.sol could be set external
    The function hasPendingRewards in TroveManager.sol could be set external
    The function getBorrowingRate in TroveManager.sol could be set external
    The function calcDecayedBaseRate in TroveManager.sol could be set external
    The function getEntireDebtAndColls in TroveManager.sol could be set external
    The function getBorrowingRateWithDecay in TroveManager.sol could be set external
    The function batchLiquidateTroves in TroveManagerLiquidations.sol could be set external
    The function permitToken in BoringBatchable.sol could be set external
    The function claimOwnership in BoringOwnable.sol could be set external
    The function transferOwnership in BoringOwnable.sol could be set external
    The function constructor in BoringOwnable.sol could be set external
    The function constructor in Domain.sol could be set external
    The function constructor in CommunityIssuance.sol could be set external
    The function constructor in LockupContract.sol could be set external
    The function isRegisteredLockup in LockupContractFactory.sol could be set external
    The function constructor in ShortLockupContract.sol could be set external
    The function constructor in TeamLockup.sol could be set external
    The function domainSeparator in YETIToken.sol could be set external
    The function constructor in YETIToken.sol could be set external
    The function constructor in YetiFinanceTreasury.sol could be set external
    The function domainSeparator in YUSDToken.sol could be set external
    The function constructor in YUSDToken.sol could be set external

Handle

robee

Vulnerability details

The following structs could change the order of their stored elements to decrease memory uses. and number of occupied slots. Therefore will save gas at every store and load from memory.

In PriceFeed.sol, TellorResponse is optimized to: 3 slots from: 4 slots. The new order of types (you choose the actual variables): 1. uint256 2. uint256 3. bool 4. bool

Handle

robee

Vulnerability details

You calculate the power of 10 every time you use it instead of caching it once as a constant variable and using it instead. Fix the following code lines:

    LiquityMath.sol, 80 : You should cache the used power of 10 as constant state variable since it's used several times (2): new collateral ratio is 10**18 times the collateral ratio. (150% => 1.5e18) 
    LiquityMath.sol, 83 : You should cache the used power of 10 as constant state variable since it's used several times (2): uint newCollRatio = _coll.mul(10**18).div(_debt); 
    Whitelist.sol, 29 : You should cache the used power of 10 as constant state variable since it's used several times (6): 10**18 * the ratio. i.e. ratio = .95 * 10**18 for 95%. More risky collateral has a lower ratio 
    Whitelist.sol, 296 : You should cache the used power of 10 as constant state variable since it's used several times (6): Returned as fee percentage * 10**18. View function for external callers. 
    Whitelist.sol, 308 : You should cache the used power of 10 as constant state variable since it's used several times (6): Returned as fee percentage * 10**18. Non view function for just borrower operations to call. 
    Whitelist.sol, 330 : You should cache the used power of 10 as constant state variable since it's used several times (6): should return 10**18 times the price in USD of 1 of the given _collateral 
    Whitelist.sol, 368 : You should cache the used power of 10 as constant state variable since it's used several times (6): div by 10**18 for price adjustment 

Handle

robee

Vulnerability details

Caching the array length is more gas efficient. This is because access to a local variable in solidity is more efficient than query storage / calldata / memory We recommend to change from: for (uint256 i=0; i<array.length; i++) { ... } to: uint len = array.length for (uint256 i=0; i<len; i++) { ... } These functions use not using prefix increments (++x) or not using the unchecked keyword:

    ActivePool.sol, tokens.poolColl, 133
    ActivePool.sol, _tokens, 167
    ActivePool.sol, _tokens, 184
    BorrowerOperations.sol, _tokensIn, 699
    BorrowerOperations.sol, _colls, 873
    BorrowerOperations.sol, arr, 890
    BorrowerOperations.sol, _colls1, 920
    BorrowerOperations.sol, _colls, 929
    BorrowerOperations.sol, _routers, 1061
    DefaultPool.sol, tokens.poolColl, 97
    DefaultPool.sol, _tokens, 133
    LiquityBase.sol, tokens._coll, 63
    LiquityBase.sol, _tokens, 97
    LiquityBase.sol, tokens._colls, 106
    LiquityBase.sol, tokens._colls, 115
    LiquityBase.sol, tokens, 149
    LiquityBase.sol, tokens.coll, 158
    LiquityBase.sol, tokens._coll, 168
    YetiCustomBase.sol, tokens._coll1, 36
    YetiCustomBase.sol, tokens._coll2, 44
    YetiCustomBase.sol, tokens.coll3, 59
    YetiCustomBase.sol, _tokens, 104
    YetiCustomBase.sol, _subTokens, 122
    YetiCustomBase.sol, tokens._coll1, 144
    YetiCustomBase.sol, _tokens, 152
    YetiCustomBase.sol, tokens.coll3, 165
    YetiCustomBase.sol, _arr, 178
    HintHelpers.sol, tokens.colls, 139
    MultiTroveGetter.sol, allColls.data, 110
    StabilityPool.sol, _amountsAdded, 562
    StabilityPool.sol, _amountsAdded, 588
    StabilityPool.sol, _amountsAdded, 592
    StabilityPool.sol, _assets, 628
    StabilityPool.sol, assets, 720
    StabilityPool.sol, assets, 942
    StabilityPool.sol, allColls, 1011
    CDPManagerTester.sol, _amounts, 14
    EchidnaTester.sol, _tokens, 121
    TroveManager.sol, _lowerHints, 234
    TroveManager.sol, allColls, 348
    TroveManager.sol, allColls, 374
    TroveManager.sol, allColls, 397
    TroveManager.sol, assets, 420
    TroveManager.sol, borrowerColls, 460
    TroveManager.sol, _tokens, 525
    TroveManager.sol, allColls, 582
    TroveManager.sol, _tokens, 603
    TroveManagerLiquidations.sol, _troveArray, 255
    TroveManagerLiquidations.sol, _troveArray, 334
    TroveManagerLiquidations.sol, tokens.collToLiquidate.vars, 394
    TroveManagerLiquidations.sol, tokens.collToLiquidate.vars, 475
    TroveManagerLiquidations.sol, _troveTokens, 808
    TroveManagerLiquidations.sol, tokens._colls, 840
    TroveManagerRedemptions.sol, tokens.colls, 304
    TroveManagerRedemptions.sol, amounts.coll, 517