Ethos Reserve contest - 0xackermann's results

A CDP-backed stablecoin platform designed to generate yield on underlying assets to establish a sustainable DeFi stable interest rate.

General Information

Platform: Code4rena

Start Date: 16/02/2023

Pot Size: $144,750 USDC

Total HM: 17

Participants: 154

Period: 19 days

Judge: Trust

Total Solo HM: 5

Id: 216

League: ETH

Ethos Reserve

Findings Distribution

Researcher Performance

Rank: 63/154

Findings: 2

Award: $103.33

QA:
grade-b
Gas:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

QA Report

Exclusive findings is below the automated findings.


Automated Findings

Non Critical Issues

IssueInstances
NC-1Missing checks for address(0) when assigning values to address state variables26
NC-2require() / revert() statements should have descriptive reason strings10
NC-3Return values of approve() not checked5
NC-4TODO Left in the code2
NC-5Event is missing indexed fields93
NC-6Functions not used internally could be marked external2
NC-7Typos32

<a name="NC-1"></a>[NC-1] Missing checks for address(0) when assigning values to address state variables

Instances (26):

File: Ethos-Core/contracts/ActivePool.sol

96:         collateralConfigAddress = _collateralConfigAddress;

97:         borrowerOperationsAddress = _borrowerOperationsAddress;

98:         troveManagerAddress = _troveManagerAddress;

99:         stabilityPoolAddress = _stabilityPoolAddress;

100:         defaultPoolAddress = _defaultPoolAddress;

101:         collSurplusPoolAddress = _collSurplusPoolAddress;

103:         lqtyStakingAddress = _lqtyStakingAddress;

Link to code

File: Ethos-Core/contracts/BorrowerOperations.sol

146:         stabilityPoolAddress = _stabilityPoolAddress;

147:         gasPoolAddress = _gasPoolAddress;

152:         lqtyStakingAddress = _lqtyStakingAddress;

Link to code

File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol

75:         stabilityPoolAddress = _stabilityPoolAddress;

Link to code

File: Ethos-Core/contracts/LQTY/LQTYStaking.sol

88:         troveManagerAddress = _troveManagerAddress;

89:         borrowerOperationsAddress = _borrowerOperationsAddress;

90:         activePoolAddress = _activePoolAddress;

Link to code

File: Ethos-Core/contracts/LUSDToken.sol

102:         troveManagerAddress = _troveManagerAddress;

106:         stabilityPoolAddress = _stabilityPoolAddress;

110:         borrowerOperationsAddress = _borrowerOperationsAddress;

114:         governanceAddress = _governanceAddress;

117:         guardianAddress = _guardianAddress;

149:         governanceAddress = _newGovernanceAddress;

156:         guardianAddress = _newGuardianAddress;

170:         troveManagerAddress = _newTroveManagerAddress;

174:         stabilityPoolAddress = _newStabilityPoolAddress;

178:         borrowerOperationsAddress = _newBorrowerOperationsAddress;

Link to code

File: Ethos-Core/contracts/TroveManager.sol

266:         borrowerOperationsAddress = _borrowerOperationsAddress;

271:         gasPoolAddress = _gasPoolAddress;

Link to code

<a name="NC-2"></a>[NC-2] require() / revert() statements should have descriptive reason strings

Instances (10):

File: Ethos-Core/contracts/TroveManager.sol

250:         require(msg.sender == owner);

551:         require(totals.totalDebtInSequence > 0);

716:         require(_troveArray.length != 0);

754:         require(totals.totalDebtInSequence > 0);

1450:         require(redemptionFee < _collateralDrawn);

1523:         require(msg.sender == borrowerOperationsAddress);

1527:         require(msg.sender == address(redemptionHelper));

1531:         require(msg.sender == borrowerOperationsAddress || msg.sender == address(redemptionHelper));

1535:         require(Troves[_borrower][_collateral].status == Status.active);

1539:         require (TroveOwnersArrayLength > 1 && sortedTroves.getSize(_collateral) > 1);

Link to code

<a name="NC-3"></a>[NC-3] Return values of approve() not checked

Not all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything

Instances (5):

File: Ethos-Core/contracts/LUSDToken.sol

231:         _approve(msg.sender, spender, amount);

238:         _approve(sender, msg.sender, _allowances[sender][msg.sender].sub(amount, "ERC20: transfer amount exceeds allowance"));

243:         _approve(msg.sender, spender, _allowances[msg.sender][spender].add(addedValue));

248:         _approve(msg.sender, spender, _allowances[msg.sender][spender].sub(subtractedValue, "ERC20: decreased allowance below zero"));

289:         _approve(owner, spender, amount);

Link to code

<a name="NC-4"></a>[NC-4] TODO Left in the code

TODOs may signal that a feature is missing or not ready for audit, consider resolving the issue and removing the TODO comment

Instances (2):

File: Ethos-Core/contracts/StabilityPool.sol

335:         /* TODO tess3rac7 unused var, but previously included in ETHGainWithdrawn event log.

380:         /* TODO tess3rac7 unused var, but previously included in ETHGainWithdrawn event log.

Link to code

<a name="NC-5"></a>[NC-5] Event is missing indexed fields

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

Instances (93):

File: Ethos-Core/contracts/ActivePool.sol

58:     event CollateralConfigAddressChanged(address _newCollateralConfigAddress);

59:     event BorrowerOperationsAddressChanged(address _newBorrowerOperationsAddress);

60:     event TroveManagerAddressChanged(address _newTroveManagerAddress);

61:     event CollSurplusPoolAddressChanged(address _collSurplusPoolAddress);

62:     event ActivePoolLUSDDebtUpdated(address _collateral, uint _LUSDDebt);

63:     event ActivePoolCollateralBalanceUpdated(address _collateral, uint _amount);

64:     event YieldingPercentageUpdated(address _collateral, uint256 _bps);

65:     event YieldingPercentageDriftUpdated(uint256 _driftBps);

66:     event YieldClaimThresholdUpdated(address _collateral, uint256 _threshold);

67:     event YieldDistributionParamsUpdated(uint256 _treasurySplit, uint256 _SPSplit, uint256 _stakingSplit);

Link to code

File: Ethos-Core/contracts/BorrowerOperations.sol

92:     event CollateralConfigAddressChanged(address _newCollateralConfigAddress);

93:     event TroveManagerAddressChanged(address _newTroveManagerAddress);

94:     event ActivePoolAddressChanged(address _activePoolAddress);

95:     event DefaultPoolAddressChanged(address _defaultPoolAddress);

96:     event StabilityPoolAddressChanged(address _stabilityPoolAddress);

97:     event GasPoolAddressChanged(address _gasPoolAddress);

98:     event CollSurplusPoolAddressChanged(address _collSurplusPoolAddress);

99:     event PriceFeedAddressChanged(address  _newPriceFeedAddress);

100:     event SortedTrovesAddressChanged(address _sortedTrovesAddress);

101:     event LUSDTokenAddressChanged(address _lusdTokenAddress);

102:     event LQTYStakingAddressChanged(address _lqtyStakingAddress);

104:     event TroveCreated(address indexed _borrower, address _collateral, uint arrayIndex);

105:     event TroveUpdated(address indexed _borrower, address _collateral, uint _debt, uint _coll, uint stake, BorrowerOperation operation);

106:     event LUSDBorrowingFeePaid(address indexed _borrower, address _collateral, uint _LUSDFee);

Link to code

File: Ethos-Core/contracts/CollateralConfig.sol

37:     event CollateralWhitelisted(address _collateral, uint256 _decimals, uint256 _MCR, uint256 _CCR);

38:     event CollateralRatiosUpdated(address _collateral, uint256 _MCR, uint256 _CCR);

Link to code

File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol

50:     event OathTokenAddressSet(address _oathTokenAddress);

51:     event LogRewardPerSecond(uint _rewardPerSecond);

52:     event StabilityPoolAddressSet(address _stabilityPoolAddress);

53:     event TotalOATHIssuedUpdated(uint _totalOATHIssued);

Link to code

File: Ethos-Core/contracts/LQTY/LQTYStaking.sol

49:     event LQTYTokenAddressSet(address _lqtyTokenAddress);

50:     event LUSDTokenAddressSet(address _lusdTokenAddress);

51:     event TroveManagerAddressSet(address _troveManager);

52:     event BorrowerOperationsAddressSet(address _borrowerOperationsAddress);

53:     event ActivePoolAddressSet(address _activePoolAddress);

54:     event CollateralConfigAddressSet(address _collateralConfigAddress);

56:     event StakeChanged(address indexed staker, uint newStake);

57:     event StakingGainsWithdrawn(address indexed staker, uint LUSDGain, address[] _assets, uint[] _amounts);

58:     event F_CollateralUpdated(address _collateral, uint _F_Collateral);

59:     event F_LUSDUpdated(uint _F_LUSD);

60:     event TotalLQTYStakedUpdated(uint _totalLQTYStaked);

61:     event CollateralSent(address _account, address _collateral, uint _amount);

62:     event StakerSnapshotsUpdated(address _staker, address[] _assets, uint[] _amounts, uint _F_LUSD);

Link to code

File: Ethos-Core/contracts/LUSDToken.sol

78:     event TroveManagerAddressChanged(address _troveManagerAddress);

79:     event StabilityPoolAddressChanged(address _newStabilityPoolAddress);

80:     event BorrowerOperationsAddressChanged(address _newBorrowerOperationsAddress);

81:     event GovernanceAddressChanged(address _governanceAddress);

82:     event GuardianAddressChanged(address _guardianAddress);

Link to code

File: Ethos-Core/contracts/StabilityPool.sol

233:     event StabilityPoolCollateralBalanceUpdated(address _collateral, uint _newBalance);

234:     event StabilityPoolLUSDBalanceUpdated(uint _newBalance);

236:     event BorrowerOperationsAddressChanged(address _newBorrowerOperationsAddress);

237:     event CollateralConfigAddressChanged(address _newCollateralConfigAddress);

238:     event TroveManagerAddressChanged(address _newTroveManagerAddress);

239:     event ActivePoolAddressChanged(address _newActivePoolAddress);

240:     event DefaultPoolAddressChanged(address _newDefaultPoolAddress);

241:     event LUSDTokenAddressChanged(address _newLUSDTokenAddress);

242:     event SortedTrovesAddressChanged(address _newSortedTrovesAddress);

243:     event PriceFeedAddressChanged(address _newPriceFeedAddress);

244:     event CommunityIssuanceAddressChanged(address _newCommunityIssuanceAddress);

246:     event P_Updated(uint _P);

247:     event S_Updated(address _collateral, uint _S, uint128 _epoch, uint128 _scale);

248:     event G_Updated(uint _G, uint128 _epoch, uint128 _scale);

249:     event EpochUpdated(uint128 _currentEpoch);

250:     event ScaleUpdated(uint128 _currentScale);

252:     event DepositSnapshotUpdated(address indexed _depositor, uint _P, address[] _assets, uint[] _amounts, uint _G);

253:     event UserDepositChanged(address indexed _depositor, uint _newDeposit);

255:     event CollateralGainWithdrawn(address indexed _depositor, address _collateral, uint _collAmount);

256:     event LQTYPaidToDepositor(address indexed _depositor, uint _LQTY);

257:     event CollateralSent(address _collateral, address _to, uint _amount);

Link to code

File: Ethos-Core/contracts/TroveManager.sol

186:     event BorrowerOperationsAddressChanged(address _newBorrowerOperationsAddress);

187:     event CollateralConfigAddressChanged(address _newCollateralConfigAddress);

188:     event PriceFeedAddressChanged(address _newPriceFeedAddress);

189:     event LUSDTokenAddressChanged(address _newLUSDTokenAddress);

190:     event ActivePoolAddressChanged(address _activePoolAddress);

191:     event DefaultPoolAddressChanged(address _defaultPoolAddress);

192:     event StabilityPoolAddressChanged(address _stabilityPoolAddress);

193:     event GasPoolAddressChanged(address _gasPoolAddress);

194:     event CollSurplusPoolAddressChanged(address _collSurplusPoolAddress);

195:     event SortedTrovesAddressChanged(address _sortedTrovesAddress);

196:     event LQTYTokenAddressChanged(address _lqtyTokenAddress);

197:     event LQTYStakingAddressChanged(address _lqtyStakingAddress);

198:     event RedemptionHelperAddressChanged(address _redemptionHelperAddress);

200:     event Liquidation(address _collateral, uint _liquidatedDebt, uint _liquidatedColl, uint _collGasCompensation, uint _LUSDGasCompensation);

201:     event TroveUpdated(address indexed _borrower, address _collateral, uint _debt, uint _coll, uint _stake, TroveManagerOperation _operation);

202:     event TroveLiquidated(address indexed _borrower, address _collateral, uint _debt, uint _coll, TroveManagerOperation _operation);

203:     event BaseRateUpdated(uint _baseRate);

204:     event LastFeeOpTimeUpdated(uint _lastFeeOpTime);

205:     event TotalStakesUpdated(address _collateral, uint _newTotalStakes);

206:     event SystemSnapshotsUpdated(address _collateral, uint _totalStakesSnapshot, uint _totalCollateralSnapshot);

207:     event LTermsUpdated(address _collateral, uint _L_Collateral, uint _L_LUSDDebt);

208:     event TroveSnapshotsUpdated(address _collateral, uint _L_Collateral, uint _L_LUSDDebt);

209:     event TroveIndexUpdated(address _borrower, address _collateral, uint _newIndex);

210:     event Redemption(

Link to code

<a name="NC-6"></a>[NC-6] Functions not used internally could be marked external

Instances (2):

File: Ethos-Core/contracts/TroveManager.sol

1045:     function getNominalICR(address _borrower, address _collateral) public view override returns (uint) {

1440:     function getRedemptionFee(uint _collateralDrawn) public view override returns (uint) {

Link to code

<a name="NC-7"></a>[NC-7] Typos

Instances (32):

File: Ethos-Core/contracts/ActivePool.sol

- 49:     uint256 public yieldingPercentageDrift = 100; // rebalance iff % is off by more than 100 BPS
+ 49:     uint256 public yieldingPercentageDrift = 100; // rebalance if % is off by more than 100 BPS

Link to code

File: Ethos-Core/contracts/BorrowerOperations.sol

- 127:         // This makes impossible to open a trove with zero withdrawn LUSD
+ 127:         // This makes it impossible to open a trove with zero withdrawn LUSD

- 230:         // Pull collateral, move it to the Active Pool, and mint the LUSDAmount to the borrower
+ 230:         // Pull collateral, move it to the Active Pool, and mint the LUSD amount to the borrower

- 241:     // Send more collateral to an existing trove
+ 241:     // Send more collateral to an existing Trove

- 253:     // Withdraw collateral from a trove
+ 253:     // Withdraw collateral from a Trove

- 258:     // Withdraw LUSD tokens from a trove: mint new LUSD tokens to the owner, and increase the trove's debt accordingly
+ 258:     // Withdraw LUSD tokens from a Trove: mint new LUSD tokens to the owner, and increase the Trove's debt accordingly

- 305:         // Get the collChange based on whether or not collateral was sent in the transaction
+ 305:         // Get the collChange based on whether or not collateral was sent in the transaction (i.e. if the Stability Pool is sending collateral to a Trove)

Link to code

File: Ethos-Core/contracts/CollateralConfig.sol

- 23:     // Smallest allowed value for Critical system collateral ratio.
+ 23:     // Smallest allowed value for the critical system collateral ratio.

- 34:     address[] public collaterals; // for returning entire list of allowed collaterals
+ 34:     address[] public collaterals; // for returning the entire list of allowed collaterals.

Link to code

File: Ethos-Core/contracts/LQTY/LQTYStaking.sol

- 140:     // Unstake the LQTY and send the it back to the caller, along with their accumulated LUSD & collateral gains.
+ 140:     // Unstake the LQTY and send it back to the caller, along with their accumulated LUSD & collateral gains

- 146:         // Grab any accumulated ETH and LUSD gains from the current stake
+ 146:         // Grab any accumulated collateral and LUSD gains from the current stake

- 170:         // Send accumulated LUSD and ETH gains to the caller
+ 170:         // Send accumulated LUSD and collateral gains to the caller

Link to code

File: Ethos-Core/contracts/LUSDToken.sol

- 73:     // Copied from LQTYToken.sol; since we deleted that file, we use LUSDToken's initialization
+ 73:     // Copied from LQTYToken.sol; since we deleted that file, we use LQTYToken's initialization

- 183:     // --- Functions for intra-Liquity calls ---
+ 183:     // --- Functions for intra-Liquity calls --- // TODO: remove this comment

- 308:     // --- Internal operations ---
+ 308:     // Warning: sanity checks (for sender and recipient) should have been done before calling these internal functions

- 309:     // Warning: sanity checks (for sender and recipient) should have been done before calling these internal functions
+ 309:     // --- 'require' functions ---

- 344:     // --- 'require' functions ---
+ 344:         // only latest borrowerOps version can mint

- 361:         // only latest borrowerOps version can mint
+ 361:         // old versions of the protocol may still burn

- 366:         // old versions of the protocol may still burn
+ 366:         // only latest stabilityPool can accept new deposits

- 376:         // only latest stabilityPool can accept new deposits
+ 376:         // old versions of the protocol may still:

- 381:         // old versions of the protocol may still:
+ 381:         // 1. send lusd gas reserve to liquidator

- 382:         // 1. send lusd gas reserve to liquidator
+ 382:         // 2. be able to return users their lusd from the stability pool

- 383:         // 2. be able to return users their lusd from the stability pool
+ 383:     // --- Optional functions ---

397:     // --- Optional functions ---

Link to code

File: Ethos-Core/contracts/StabilityPool.sol

- 728:     // Internal function, used to calculcate compounded deposits.
+ 728:     // Internal function, used to calculate compounded deposits.

Link to code

File: Ethos-Core/contracts/TroveManager.sol

- 41:     // A doubly linked list of Troves, sorted by their sorted by their collateral ratios
+ 41:     // A doubly linked list of Troves, sorted by their collateral ratios

- 114:     // Array of all active trove addresses - used to to compute an approximate hint off-chain, for the sorted list insertion
+ 114:     // Array of all active trove addresses - used to compute an approximate hint off-chain, for the sorted list insertion

Link to code

File: Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol

- 1: // SPDX-License-Identifier: BUSL1.1
+ 1: // SPDX-License-Identifier: BSL-1.0

Link to code

File: Ethos-Vault/contracts/ReaperVaultERC4626.sol

- 224:     // Allows an on-chain or off-chain user to simulate the effects of their redeemption at the current block,
+ 224:     // Allows an on-chain or off-chain user to simulate the effects of their redemption at the current block,

Link to code

File: Ethos-Vault/contracts/ReaperVaultV2.sol

- 32:         uint256 lastReport; // block.timestamp of the last time a report occured
+ 32:         uint256 lastReport; // block.timestamp of the last time a report occurred

- 434:         // Loss can only be up the amount of capital allocated to the strategy
+ 434:         // Loss can only be up to the amount of capital allocated to the strategy

Link to code

File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol

- 24:     uint256 public constant UPGRADE_TIMELOCK = 48 hours; // minimum 48 hours for RF
+ 24:     uint256 public constant UPGRADE_TIMELOCK = 48 hours; // minimum 48 hours for RFP

Link to code

Low Issues

IssueInstances
L-1abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()1
L-2Do not use deprecated library functions1
L-3Empty Function Body - Consider commenting why2
L-4Initializers could be front-run8
L-5Unsafe ERC20 operation(s)4

<a name="L-1"></a>[L-1] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead. If all arguments are strings and or bytes, bytes.concat() should be used instead

Instances (1):

File: Ethos-Core/contracts/LUSDToken.sol

283:         bytes32 digest = keccak256(abi.encodePacked('\x19\x01', 

Link to code

<a name="L-2"></a>[L-2] Do not use deprecated library functions

Instances (1):

File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol

74:         IERC20Upgradeable(want).safeApprove(vault, type(uint256).max);

Link to code

<a name="L-3"></a>[L-3] Empty Function Body - Consider commenting why

Instances (2):

File: Ethos-Vault/contracts/ReaperVaultERC4626.sol

24:     ) ReaperVaultV2(_token, _name, _symbol, _tvlCap, _treasury, _strategists, _multisigRoles) {}

Link to code

File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol

61:     constructor() initializer {}

Link to code

<a name="L-4"></a>[L-4] Initializers could be front-run

Initializers could be front-run, allowing an attacker to either set their own values, take ownership of the contract, and in the best case forcing a re-deployment

Instances (8):

File: Ethos-Core/contracts/CollateralConfig.sol

46:     function initialize(

Link to code

File: Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol

62:     function initialize(

67:     ) public initializer {

70:         __ReaperBaseStrategy_init(_vault, want, _strategists, _multisigRoles);

Link to code

File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol

61:     constructor() initializer {}

63:     function __ReaperBaseStrategy_init(

69:         __UUPSUpgradeable_init();

70:         __AccessControlEnumerable_init();

Link to code

<a name="L-5"></a>[L-5] Unsafe ERC20 operation(s)

Instances (4):

File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol

103:         OathToken.transferFrom(msg.sender, address(this), amount);

127:         OathToken.transfer(_account, _OathAmount);

Link to code

File: Ethos-Core/contracts/LQTY/LQTYStaking.sol

135:             lusdToken.transfer(msg.sender, LUSDGain);

171:         lusdToken.transfer(msg.sender, LUSDGain);

Link to code

Medium Issues

IssueInstances
M-1Centralization Risk for trusted owners21

<a name="M-1"></a>[M-1] Centralization Risk for trusted owners

Impact:

Contracts have owners with privileged rights to perform admin tasks and need to be trusted to not perform malicious updates or drain funds.

Instances (21):

File: Ethos-Core/contracts/ActivePool.sol

26: contract ActivePool is Ownable, CheckContract, IActivePool {

125:     function setYieldingPercentage(address _collateral, uint256 _bps) external onlyOwner {

132:     function setYieldingPercentageDrift(uint256 _driftBps) external onlyOwner {

138:     function setYieldClaimThreshold(address _collateral, uint256 _threshold) external onlyOwner {

144:     function setYieldDistributionParams(uint256 _treasurySplit, uint256 _SPSplit, uint256 _stakingSplit) external onlyOwner {

214:     function manualRebalance(address _collateral, uint256 _simulatedAmountLeavingPool) external onlyOwner {

Link to code

File: Ethos-Core/contracts/BorrowerOperations.sol

18: contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOperations {

Link to code

File: Ethos-Core/contracts/CollateralConfig.sol

15: contract CollateralConfig is ICollateralConfig, CheckContract, Ownable {

50:     ) external override onlyOwner {

89:     ) external onlyOwner checkCollateral(_collateral) {

Link to code

File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol

14: contract CommunityIssuance is ICommunityIssuance, Ownable, CheckContract, BaseMath {

67:         onlyOwner 

101:     function fund(uint amount) external onlyOwner {

120:     function updateDistributionPeriod(uint256 _newDistributionPeriod) external onlyOwner {

Link to code

File: Ethos-Core/contracts/LQTY/LQTYStaking.sol

18: contract LQTYStaking is ILQTYStaking, Ownable, CheckContract, BaseMath {

76:         onlyOwner 

Link to code

File: Ethos-Core/contracts/StabilityPool.sol

146: contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {

Link to code

File: Ethos-Core/contracts/TroveManager.sol

18: contract TroveManager is LiquityBase, /*Ownable,*/ CheckContract, ITroveManager {

Link to code

File: Ethos-Vault/contracts/ReaperVaultV2.sol

21: contract ReaperVaultV2 is ReaperAccessControl, ERC20, IERC4626Events, AccessControlEnumerable, ReentrancyGuard {

21: contract ReaperVaultV2 is ReaperAccessControl, ERC20, IERC4626Events, AccessControlEnumerable, ReentrancyGuard {

Link to code

File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol

15:     ReaperAccessControl,

Link to code


Exclusive Findings


Low Risk Issues


[L-01] Loss of precision due to rounding


Description:

Typically occurs while using / operator. It is recommended to use SafeMath library for all arithmetic operations.

Recommendation:

Add scalar so roundings are negligible.

Lines of Code:


[L-02] Always use safeTransferFrom instead of transferFrom


Lines of Code:


[L-03] Owner can renounce Ownership


Description:

Typically, the contract’s owner is the account that deploys the contract. As a result, the owner is able to perform certain privileged activities.

The non-fungible Ownable used in this project contract implements renounceOwnership. This can represent a certain risk if the ownership is renounced for any other reason than by design. Renouncing ownership will leave the contract without an owner, thereby removing any functionality that is only available to the owner.

Lines of Code:


[L-04] onlyOwner functions


onlyOwner functions:


Non Critical Issues


[NC-01] For modern and more readable code; update import usages


Description:

Our Solidity code is also cleaner in another way that might not be noticeable: the struct Point. We were importing it previously with global import but not using it. The Point struct polluted the source code with an unnecessary object we were not using because we did not need it. This was breaking the rule of modularity and modular programming: only import what you need Specific imports with curly braces allow us to apply this rule better.

Lines of Code:


[NC-02] Use require instead of assert


Description:

Assert should not be used except for tests, require should be used

Prior to Solidity 0.8.0, pressing a confirm consumes the remainder of the process's available gas instead of returning it, as request()/revert() did.

Assertion() should be avoided even after solidity version 0.8.0, because its documentation states 'The Assert function generates an error of type Panic(uint256). Code that works properly should never Panic, even on invalid external input. If this happens, you need to fix it in your contract. there's a mistake'.

Lines of Code:


[NC-03] NatSpec comments should be increased in contracts


Description:

It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability. https://docs.soliditylang.org/en/v0.8.15/natspec-format.html

Lines of Code:


[NC-04] Tokens accidentally sent to the contract cannot be recovered


Description:

It can't be recovered if the tokens accidentally arrive at the contract address, which has happened to many popular projects, so I recommend adding a recovery code to your critical contracts.

Recommendation:

Add this code:

/**
	* @notice Sends ERC20 tokens trapped in contract to external address
	* @dev Onlyowner is allowed to make this function call
	* @param account is the receiving address
	* @param externalToken is the token being sent
	* @param amount is the quantity being sent
	* @return boolean value indicating whether the operation succeeded.
	*
*/
	function rescueERC20(address account, address externalToken, uint256 amount) public onlyOwner returns (bool) {
		IERC20(externalToken).transfer(account, amount);
		return true;
	}
}

[NC-05] Use a more recent version of Solidity


Description:

For security, it is best practice to use the latest Solidity version. For the security fix list in the versions: https://github.com/ethereum/solidity/blob/develop/Changelog.md

Recommendation:

Old version of Solidity is used , newer version can be used (0.8.17)

Lines of Code:


[NC-06] Use underscores for number literals


Description:

For example, 1000 should be written as 1_000

Lines of Code:


[NC-07] Long lines are not suitable for the ‘Solidity Style Guide’


Description:

It is generally recommended that lines in the source code should not exceed 80-120 characters. Today's screens are much larger, so in some cases it makes sense to expand that. The lines above should be split when they reach that length, as the files will most likely be on GitHub and GitHub always uses a scrollbar when the length is more than 164 characters.

(why-is-80-characters-the-standard-limit-for-code-width)[https://softwareengineering.stackexchange.com/questions/148677/why-is-80-characters-the-standard-limit-for-code-width]

Recommendation:

Multiline output parameters and return statements should follow the same style recommended for wrapping long lines found in the Maximum Line Length section.

https://docs.soliditylang.org/en/v0.8.17/style-guide.html#introduction

Lines of Code:


[NC-08] Omissions in Events


Description:

Throughout the codebase, events are generally emitted when sensitive changes are made to the contracts. However, some events are missing important parameters

Lines of Code:


[NC-09] Showing the actual values of numbers in NatSpec comments makes checking and reading code easier


Description:

For example, 365 days should be written as 365 days; // 31_536_000 (365*24*60*60)

Lines of Code:


[NC-10] Constant values such as a call to keccak256(), should used to immutable rather than constant


Description:

There is a difference between constant variables and immutable variables, and they should each be used in their appropriate contexts.

While it doesn't save any gas because the compiler knows that developers often make this mistake, it's still best to use the right tool for the task at hand.

Constants should be used for literal values written into the code, and immutable variables should be used for expressions, or values calculated in, or passed into the constructor.

Lines of Code:


[NC-11] Return parameters are declared, hence no need return variable.


// TroveManager.sol

function _liquidateNormalMode(
		IActivePool _activePool,
		IDefaultPool _defaultPool,
		address _collateral,
		address _borrower,
		uint _LUSDInStabPool
)
		internal
		returns (LiquidationValues memory singleLiquidation)
{
		LocalVariables_InnerSingleLiquidateFunction memory vars;

		(singleLiquidation.entireTroveDebt,
		singleLiquidation.entireTroveColl,
		vars.pendingDebtReward,
		vars.pendingCollReward) = getEntireDebtAndColl(_borrower, _collateral);

		_movePendingTroveRewardsToActivePool(_activePool, _defaultPool, _collateral, vars.pendingDebtReward, vars.pendingCollReward);
		_removeStake(_borrower, _collateral);

		singleLiquidation.collGasCompensation = _getCollGasCompensation(singleLiquidation.entireTroveColl);
		singleLiquidation.LUSDGasCompensation = LUSD_GAS_COMPENSATION;
		uint collToLiquidate = singleLiquidation.entireTroveColl.sub(singleLiquidation.collGasCompensation);

		(singleLiquidation.debtToOffset,
		singleLiquidation.collToSendToSP,
		singleLiquidation.debtToRedistribute,
		singleLiquidation.collToRedistribute) = _getOffsetAndRedistributionVals(singleLiquidation.entireTroveDebt, collToLiquidate, _LUSDInStabPool);

		_closeTrove(_borrower, _collateral, Status.closedByLiquidation);
		emit TroveLiquidated(_borrower, _collateral, singleLiquidation.entireTroveDebt, singleLiquidation.entireTroveColl, TroveManagerOperation.liquidateInNormalMode);
		emit TroveUpdated(_borrower, _collateral, 0, 0, 0, TroveManagerOperation.liquidateInNormalMode);
--		return singleLiquidation;
}

Lines of Code:


[NC-12] Utilise named return variable


// TroveManager.sol

function _liquidateRecoveryMode(
		IActivePool _activePool,
		IDefaultPool _defaultPool,
		address _collateral,
		address _borrower,
		uint _ICR,
		uint _LUSDInStabPool,
		uint _TCR,
		uint _price,
		uint256 _MCR
)
		internal
		returns (LiquidationValues memory singleLiquidation)
{
	// Lines of codes

	else if ((_ICR >= _MCR) && (_ICR < _TCR) && (singleLiquidation.entireTroveDebt <= _LUSDInStabPool)) {
				_movePendingTroveRewardsToActivePool(_activePool, _defaultPool, _collateral, vars.pendingDebtReward, vars.pendingCollReward);
				assert(_LUSDInStabPool != 0);

				_removeStake(_borrower, _collateral);
				uint collDecimals = collateralConfig.getCollateralDecimals(_collateral);
				singleLiquidation = _getCappedOffsetVals(singleLiquidation.entireTroveDebt, singleLiquidation.entireTroveColl, _price, _MCR, collDecimals);

				_closeTrove(_borrower, _collateral, Status.closedByLiquidation);
				if (singleLiquidation.collSurplus > 0) {
						collSurplusPool.accountSurplus(_borrower, _collateral, singleLiquidation.collSurplus);
				}

				emit TroveLiquidated(_borrower, _collateral, singleLiquidation.entireTroveDebt, singleLiquidation.collToSendToSP, TroveManagerOperation.liquidateInRecoveryMode);
				emit TroveUpdated(_borrower, _collateral, 0, 0, 0, TroveManagerOperation.liquidateInRecoveryMode);

		} else { // if (_ICR >= _MCR && ( _ICR >= _TCR || singleLiquidation.entireTroveDebt > _LUSDInStabPool))
-				LiquidationValues memory zeroVals;
-				return zeroVals;
+				singleLiquidation = LiqudationValues();
		}
}

Lines of Code:

#0 - c4-judge

2023-03-09T14:17:17Z

trust1995 marked the issue as grade-b

#1 - c4-sponsor

2023-03-28T20:10:41Z

0xBebis marked the issue as sponsor confirmed

Gas Report

Exclusive findings is below the automated findings.


Automated Findings

Automated findings output for the Ethos-Core package and Ethos-Vault package.

Gas Optimizations

IssueInstances
GAS-1Use assembly to check for address(0)8
GAS-2Using bools for storage incurs overhead9
GAS-3Cache array length outside of loop8
GAS-4State variables should be cached in stack variables rather than re-reading them from storage10
GAS-5Use calldata instead of memory for function arguments that do not get mutated1
GAS-6Use Custom Errors73
GAS-7Don't initialize variables with default value23
GAS-8Long revert strings35
GAS-9Functions guaranteed to revert when called by normal users can be marked payable7
GAS-10++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)17
GAS-11Using private rather than public for constants, saves gas30
GAS-12Use != 0 instead of > 0 for unsigned integer comparison27
GAS-13internal functions not called by the contract should be removed1

<a name="GAS-1"></a>[GAS-1] Use assembly to check for address(0)

Saves 6 gas per instance

Instances (8):

File: Ethos-Core/contracts/ActivePool.sol

93:         require(_treasuryAddress != address(0), "Treasury cannot be 0 address");

Link to code

File: Ethos-Core/contracts/LUSDToken.sol

312:         assert(sender != address(0));

313:         assert(recipient != address(0));

321:         assert(account != address(0));

329:         assert(account != address(0));

337:         assert(owner != address(0));

338:         assert(spender != address(0));

348:             _recipient != address(0) && 

Link to code

<a name="GAS-2"></a>[GAS-2] Using bools for storage incurs overhead

Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past. See source.

Instances (9):

File: Ethos-Core/contracts/ActivePool.sol

32:     bool public addressesSet = false;

Link to code

File: Ethos-Core/contracts/CollateralConfig.sol

18:     bool public initialized = false;

Link to code

File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol

21:     bool public initialized = false;

Link to code

File: Ethos-Core/contracts/LUSDToken.sol

37:     bool public mintingPaused = false;

62:     mapping (address => bool) public troveManagers;

63:     mapping (address => bool) public stabilityPools;

64:     mapping (address => bool) public borrowerOperations;

Link to code

File: Ethos-Vault/contracts/ReaperVaultV2.sol

49:     bool public emergencyShutdown;

Link to code

File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol

30:     bool public emergencyExit;

Link to code

<a name="GAS-3"></a>[GAS-3] Cache array length outside of loop

If not cached, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first).

Instances (8):

File: Ethos-Core/contracts/CollateralConfig.sol

56:         for(uint256 i = 0; i < _collaterals.length; i++) {

Link to code

File: Ethos-Core/contracts/LQTY/LQTYStaking.sol

206:         for (uint i = 0; i < assets.length; i++) {

228:         for (uint i = 0; i < collaterals.length; i++) {

Link to code

File: Ethos-Core/contracts/StabilityPool.sol

640:         for (uint i = 0; i < assets.length; i++) {

810:             for (uint i = 0; i < collaterals.length; i++) {

831:         for (uint i = 0; i < collaterals.length; i++) {

Link to code

File: Ethos-Core/contracts/TroveManager.sol

808:         for (vars.i = 0; vars.i < _troveArray.length; vars.i++) {

882:         for (vars.i = 0; vars.i < _troveArray.length; vars.i++) {

Link to code

<a name="GAS-4"></a>[GAS-4] State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.

Saves 100 gas per instance

Instances (10):

File: Ethos-Core/contracts/ActivePool.sol

181:             IDefaultPool(defaultPoolAddress).pullCollateralFromActivePool(_collateral, _amount);

184:             ICollSurplusPool(collSurplusPoolAddress).pullCollateralFromActivePool(_collateral, _amount);

299:                     ILQTYStaking(lqtyStakingAddress).increaseF_Collateral(_collateral, vars.stakingSplit);

305:                     IStabilityPool(stabilityPoolAddress).updateRewardSum(_collateral, vars.stabilityPoolSplit);   

Link to code

File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol

113:         lastDistributionTime = block.timestamp.add(distributionPeriod);

116:         emit LogRewardPerSecond(rewardPerSecond);

Link to code

File: Ethos-Core/contracts/StabilityPool.sol

577:             emit EpochUpdated(currentEpoch);

579:             emit ScaleUpdated(currentScale);

754:             compoundedDeposit = initialDeposit.mul(P).div(snapshot_P).div(SCALE_FACTOR);

754:             compoundedDeposit = initialDeposit.mul(P).div(snapshot_P).div(SCALE_FACTOR);

Link to code

<a name="GAS-5"></a>[GAS-5] Use calldata instead of memory for function arguments that do not get mutated

Mark data types as calldata instead of memory where possible. This makes it so that the data is not automatically loaded into memory. If the data passed into the function does not need to be changed (like updating values in an array), it can be passed in as calldata. The one exception to this is if the argument must later be passed into another function that takes an argument that specifies memory storage.

Instances (1):

File: Ethos-Core/contracts/TroveManager.sol

715:     function batchLiquidateTroves(address _collateral, address[] memory _troveArray) public override {

Link to code

<a name="GAS-6"></a>[GAS-6] Use Custom Errors

Source Instead of using error strings, to reduce deployment and runtime cost, you should use Custom Errors. This would save both deployment and runtime cost.

Instances (73):

File: Ethos-Core/contracts/ActivePool.sol

85:         require(!addressesSet, "Can call setAddresses only once");

93:         require(_treasuryAddress != address(0), "Treasury cannot be 0 address");

107:         require(numCollaterals == _erc4626vaults.length, "Vaults array length must match number of collaterals");

111:             require(IERC4626(vault).asset() == collateral, "Vault asset must be collateral");

127:         require(_bps <= 10_000, "Invalid BPS value");

133:         require(_driftBps <= 500, "Exceeds max allowed value of 500 BPS");

145:         require(_treasurySplit + _SPSplit + _stakingSplit == 10_000, "Splits must add up to 10000 BPS");

Link to code

File: Ethos-Core/contracts/BorrowerOperations.sol

525:         require(collateralConfig.isCollateralAllowed(_collateral), "BorrowerOps: Invalid collateral address");

529:         require(IERC20(_collateral).balanceOf(_user) >= _collAmount, "BorrowerOperations: Insufficient user collateral balance");

530:         require(IERC20(_collateral).allowance(_user, address(this)) >= _collAmount, "BorrowerOperations: Insufficient collateral allowance");

534:         require(_collTopUp == 0 || _collWithdrawal == 0, "BorrowerOperations: Cannot withdraw and add coll");

538:         require(_collTopUp != 0 || _collWithdrawal != 0 || _LUSDChange != 0, "BorrowerOps: There must be either a collateral change or a debt change");

543:         require(status == 1, "BorrowerOps: Trove does not exist or is closed");

548:         require(status != 1, "BorrowerOps: Trove is active");

552:         require(_LUSDChange > 0, "BorrowerOps: Debt increase requires non-zero debtChange");

568:         require(_collWithdrawal == 0, "BorrowerOps: Collateral withdrawal not permitted Recovery Mode");

617:         require(_newICR >= _MCR, "BorrowerOps: An operation that would result in ICR < MCR is not permitted");

621:         require(_newICR >= _CCR, "BorrowerOps: Operation must leave trove with ICR >= CCR");

625:         require(_newICR >= _oldICR, "BorrowerOps: Cannot decrease your Trove's ICR in Recovery Mode");

629:         require(_newTCR >= _CCR, "BorrowerOps: An operation that would result in TCR < CCR is not permitted");

637:         require(_debtRepayment <= _currentDebt.sub(LUSD_GAS_COMPENSATION), "BorrowerOps: Amount repaid must not be larger than the Trove's debt");

641:         require(msg.sender == stabilityPoolAddress, "BorrowerOps: Caller is not Stability Pool");

645:         require(_lusdToken.balanceOf(_borrower) >= _debtRepayment, "BorrowerOps: Caller doesnt have enough LUSD to make repayment");

Link to code

File: Ethos-Core/contracts/CollateralConfig.sol

51:         require(!initialized, "Can only initialize once");

52:         require(_collaterals.length != 0, "At least one collateral required");

53:         require(_MCRs.length == _collaterals.length, "Array lengths must match");

54:         require(_CCRs.length == _collaterals.length, "Array lenghts must match");

66:             require(_MCRs[i] >= MIN_ALLOWED_MCR, "MCR below allowed minimum");

69:             require(_CCRs[i] >= MIN_ALLOWED_CCR, "CCR below allowed minimum");

91:         require(_MCR <= config.MCR, "Can only walk down the MCR");

92:         require(_CCR <= config.CCR, "Can only walk down the CCR");

94:         require(_MCR >= MIN_ALLOWED_MCR, "MCR below allowed minimum");

97:         require(_CCR >= MIN_ALLOWED_CCR, "CCR below allowed minimum");

129:         require(collateralConfig[_collateral].allowed, "Invalid collateral address");

Link to code

File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol

70:         require(!initialized, "issuance has been initialized");

102:         require(amount != 0, "cannot fund 0");

133:         require(msg.sender == stabilityPoolAddress, "CommunityIssuance: caller is not SP");

Link to code

File: Ethos-Core/contracts/LQTY/LQTYStaking.sol

262:         require(msg.sender == borrowerOperationsAddress, "LQTYStaking: caller is not BorrowerOps");

Link to code

File: Ethos-Core/contracts/LUSDToken.sol

362:         require(msg.sender == borrowerOperationsAddress, "LUSDToken: Caller is not BorrowerOperations");

377:         require(msg.sender == stabilityPoolAddress, "LUSD: Caller is not the StabilityPool");

390:         require(msg.sender == governanceAddress, "LUSDToken: Caller is not Governance");

394:         require(!mintingPaused, "LUSDToken: Minting is currently paused");

Link to code

File: Ethos-Core/contracts/StabilityPool.sol

849:         require( msg.sender == address(activePool), "StabilityPool: Caller is not ActivePool");

853:         require(msg.sender == address(troveManager), "StabilityPool: Caller is not TroveManager");

865:             require(ICR >= collMCR, "StabilityPool: Cannot withdraw while there are troves with ICR < MCR");

Link to code

File: Ethos-Vault/contracts/ReaperVaultERC4626.sol

270:         require(y != 0, "Division by 0");

Link to code

File: Ethos-Vault/contracts/ReaperVaultV2.sol

150:         require(!emergencyShutdown, "Cannot add strategy during emergency shutdown");

151:         require(_strategy != address(0), "Invalid strategy address");

152:         require(strategies[_strategy].activation == 0, "Strategy already added");

153:         require(address(this) == IStrategy(_strategy).vault(), "Strategy's vault does not match");

154:         require(address(token) == IStrategy(_strategy).want(), "Strategy's want does not match");

155:         require(_feeBPS <= PERCENT_DIVISOR / 5, "Fee cannot be higher than 20 BPS");

156:         require(_allocBPS + totalAllocBPS <= PERCENT_DIVISOR, "Invalid allocBPS value");

180:         require(strategies[_strategy].activation != 0, "Invalid strategy address");

181:         require(_feeBPS <= PERCENT_DIVISOR / 5, "Fee cannot be higher than 20 BPS");

193:         require(strategies[_strategy].activation != 0, "Invalid strategy address");

197:         require(totalAllocBPS <= PERCENT_DIVISOR, "Invalid BPS value");

261:         require(queueLength != 0, "Queue must not be empty");

267:             require(params.activation != 0, "Invalid strategy address");

321:         require(!emergencyShutdown, "Cannot deposit during emergency shutdown");

322:         require(_amount != 0, "Invalid amount");

324:         require(pool + _amount <= tvlCap, "Vault is full");

364:         require(_shares != 0, "Invalid amount");

436:         require(loss <= allocation, "Strategy loss cannot be greater than allocation");

497:         require(strategy.activation != 0, "Unauthorized strategy");

568:         require(_withdrawMaxLoss <= PERCENT_DIVISOR, "Invalid BPS value");

619:         require(degradation <= DEGRADATION_COEFFICIENT, "Degradation cannot be more than 100%");

629:         require(newTreasury != address(0), "Invalid address");

639:         require(_token != address(token), "!token");

Link to code

File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol

81:         require(_multisigRoles.length == 3, "Invalid number of multisig roles");

96:         require(msg.sender == vault, "Only vault can withdraw");

97:         require(_amount != 0, "Amount cannot be zero");

98:         require(_amount <= balanceOf(), "Ammount must be less than balance");

Link to code

<a name="GAS-7"></a>[GAS-7] Don't initialize variables with default value

Instances (23):

File: Ethos-Core/contracts/ActivePool.sol

108:         for(uint256 i = 0; i < numCollaterals; i++) {

Link to code

File: Ethos-Core/contracts/CollateralConfig.sol

56:         for(uint256 i = 0; i < _collaterals.length; i++) {

Link to code

File: Ethos-Core/contracts/LQTY/LQTYStaking.sol

206:         for (uint i = 0; i < assets.length; i++) {

228:         for (uint i = 0; i < collaterals.length; i++) {

240:         for (uint i = 0; i < numCollaterals; i++) {

Link to code

File: Ethos-Core/contracts/StabilityPool.sol

351:         for (uint i = 0; i < numCollaterals; i++) {

397:         for (uint i = 0; i < numCollaterals; i++) {

640:         for (uint i = 0; i < assets.length; i++) {

810:             for (uint i = 0; i < collaterals.length; i++) {

831:         for (uint i = 0; i < collaterals.length; i++) {

859:         for (uint i = 0; i < numCollaterals; i++) {

Link to code

File: Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol

35:     uint16 private constant LENDER_REFERRAL_CODE_NONE = 0;

117:         for (uint256 i = 0; i < numSteps; i = i.uncheckedInc()) {

165:         for (uint256 i = 0; i < numSteps; i = i.uncheckedInc()) {

Link to code

File: Ethos-Vault/contracts/ReaperVaultV2.sol

128:         for (uint256 i = 0; i < numStrategists; i = i.uncheckedInc()) {

264:         for (uint256 i = 0; i < queueLength; i = i.uncheckedInc()) {

369:             uint256 totalLoss = 0;

371:             uint256 vaultBalance = 0;

372:             for (uint256 i = 0; i < queueLength; i = i.uncheckedInc()) {

Link to code

File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol

77:         for (uint256 i = 0; i < numStrategists; i = i.uncheckedInc()) {

100:         uint256 amountFreed = 0;

112:         uint256 debt = 0;

117:         uint256 repayment = 0;

Link to code

<a name="GAS-8"></a>[GAS-8] Long revert strings

Instances (35):

File: Ethos-Core/contracts/ActivePool.sol

107:         require(numCollaterals == _erc4626vaults.length, "Vaults array length must match number of collaterals");

133:         require(_driftBps <= 500, "Exceeds max allowed value of 500 BPS");

Link to code

File: Ethos-Core/contracts/BorrowerOperations.sol

525:         require(collateralConfig.isCollateralAllowed(_collateral), "BorrowerOps: Invalid collateral address");

529:         require(IERC20(_collateral).balanceOf(_user) >= _collAmount, "BorrowerOperations: Insufficient user collateral balance");

530:         require(IERC20(_collateral).allowance(_user, address(this)) >= _collAmount, "BorrowerOperations: Insufficient collateral allowance");

534:         require(_collTopUp == 0 || _collWithdrawal == 0, "BorrowerOperations: Cannot withdraw and add coll");

538:         require(_collTopUp != 0 || _collWithdrawal != 0 || _LUSDChange != 0, "BorrowerOps: There must be either a collateral change or a debt change");

543:         require(status == 1, "BorrowerOps: Trove does not exist or is closed");

552:         require(_LUSDChange > 0, "BorrowerOps: Debt increase requires non-zero debtChange");

568:         require(_collWithdrawal == 0, "BorrowerOps: Collateral withdrawal not permitted Recovery Mode");

617:         require(_newICR >= _MCR, "BorrowerOps: An operation that would result in ICR < MCR is not permitted");

621:         require(_newICR >= _CCR, "BorrowerOps: Operation must leave trove with ICR >= CCR");

625:         require(_newICR >= _oldICR, "BorrowerOps: Cannot decrease your Trove's ICR in Recovery Mode");

629:         require(_newTCR >= _CCR, "BorrowerOps: An operation that would result in TCR < CCR is not permitted");

637:         require(_debtRepayment <= _currentDebt.sub(LUSD_GAS_COMPENSATION), "BorrowerOps: Amount repaid must not be larger than the Trove's debt");

641:         require(msg.sender == stabilityPoolAddress, "BorrowerOps: Caller is not Stability Pool");

645:         require(_lusdToken.balanceOf(_borrower) >= _debtRepayment, "BorrowerOps: Caller doesnt have enough LUSD to make repayment");

Link to code

File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol

133:         require(msg.sender == stabilityPoolAddress, "CommunityIssuance: caller is not SP");

Link to code

File: Ethos-Core/contracts/LQTY/LQTYStaking.sol

262:         require(msg.sender == borrowerOperationsAddress, "LQTYStaking: caller is not BorrowerOps");

266:         require(currentStake > 0, 'LQTYStaking: User must have a non-zero stake');  

270:         require(_amount > 0, 'LQTYStaking: Amount must be non-zero');

Link to code

File: Ethos-Core/contracts/LUSDToken.sol

362:         require(msg.sender == borrowerOperationsAddress, "LUSDToken: Caller is not BorrowerOperations");

377:         require(msg.sender == stabilityPoolAddress, "LUSD: Caller is not the StabilityPool");

390:         require(msg.sender == governanceAddress, "LUSDToken: Caller is not Governance");

394:         require(!mintingPaused, "LUSDToken: Minting is currently paused");

Link to code

File: Ethos-Core/contracts/StabilityPool.sol

849:         require( msg.sender == address(activePool), "StabilityPool: Caller is not ActivePool");

853:         require(msg.sender == address(troveManager), "StabilityPool: Caller is not TroveManager");

865:             require(ICR >= collMCR, "StabilityPool: Cannot withdraw while there are troves with ICR < MCR");

870:         require(_initialDeposit > 0, 'StabilityPool: User must have a non-zero deposit');

874:         require(_amount > 0, 'StabilityPool: Amount must be non-zero');

Link to code

File: Ethos-Vault/contracts/ReaperVaultV2.sol

150:         require(!emergencyShutdown, "Cannot add strategy during emergency shutdown");

321:         require(!emergencyShutdown, "Cannot deposit during emergency shutdown");

436:         require(loss <= allocation, "Strategy loss cannot be greater than allocation");

619:         require(degradation <= DEGRADATION_COEFFICIENT, "Degradation cannot be more than 100%");

Link to code

File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol

98:         require(_amount <= balanceOf(), "Ammount must be less than balance");

Link to code

<a name="GAS-9"></a>[GAS-9] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

Instances (7):

File: Ethos-Core/contracts/ActivePool.sol

125:     function setYieldingPercentage(address _collateral, uint256 _bps) external onlyOwner {

132:     function setYieldingPercentageDrift(uint256 _driftBps) external onlyOwner {

138:     function setYieldClaimThreshold(address _collateral, uint256 _threshold) external onlyOwner {

144:     function setYieldDistributionParams(uint256 _treasurySplit, uint256 _SPSplit, uint256 _stakingSplit) external onlyOwner {

214:     function manualRebalance(address _collateral, uint256 _simulatedAmountLeavingPool) external onlyOwner {

Link to code

File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol

101:     function fund(uint amount) external onlyOwner {

120:     function updateDistributionPeriod(uint256 _newDistributionPeriod) external onlyOwner {

Link to code

<a name="GAS-10"></a>[GAS-10] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Saves 5 gas per loop

Instances (17):

File: Ethos-Core/contracts/ActivePool.sol

108:         for(uint256 i = 0; i < numCollaterals; i++) {

Link to code

File: Ethos-Core/contracts/CollateralConfig.sol

56:         for(uint256 i = 0; i < _collaterals.length; i++) {

Link to code

File: Ethos-Core/contracts/LQTY/LQTYStaking.sol

206:         for (uint i = 0; i < assets.length; i++) {

228:         for (uint i = 0; i < collaterals.length; i++) {

240:         for (uint i = 0; i < numCollaterals; i++) {

Link to code

File: Ethos-Core/contracts/LUSDToken.sol

286:                          _nonces[owner]++, deadline))));

Link to code

File: Ethos-Core/contracts/StabilityPool.sol

351:         for (uint i = 0; i < numCollaterals; i++) {

397:         for (uint i = 0; i < numCollaterals; i++) {

640:         for (uint i = 0; i < assets.length; i++) {

810:             for (uint i = 0; i < collaterals.length; i++) {

831:         for (uint i = 0; i < collaterals.length; i++) {

859:         for (uint i = 0; i < numCollaterals; i++) {

Link to code

File: Ethos-Core/contracts/TroveManager.sol

608:         for (vars.i = 0; vars.i < _n && vars.user != firstUser; vars.i++) {

690:         for (vars.i = 0; vars.i < _n; vars.i++) {

808:         for (vars.i = 0; vars.i < _troveArray.length; vars.i++) {

882:         for (vars.i = 0; vars.i < _troveArray.length; vars.i++) {

Link to code

File: Ethos-Vault/contracts/ReaperVaultERC4626.sol

273:         if (x % y != 0) q++;

Link to code

<a name="GAS-11"></a>[GAS-11] Using private rather than public for constants, saves gas

If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table

Instances (30):

File: Ethos-Core/contracts/ActivePool.sol

30:     string constant public NAME = "ActivePool";

Link to code

File: Ethos-Core/contracts/BorrowerOperations.sol

21:     string constant public NAME = "BorrowerOperations";

Link to code

File: Ethos-Core/contracts/CollateralConfig.sol

21:     uint256 constant public MIN_ALLOWED_MCR = 1.1 ether; // 110%

25:     uint256 constant public MIN_ALLOWED_CCR = 1.5 ether; // 150%

Link to code

File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol

19:     string constant public NAME = "CommunityIssuance";

Link to code

File: Ethos-Core/contracts/LQTY/LQTYStaking.sol

23:     string constant public NAME = "LQTYStaking";

Link to code

File: Ethos-Core/contracts/StabilityPool.sol

150:     string constant public NAME = "StabilityPool";

197:     uint public constant SCALE_FACTOR = 1e9;

Link to code

File: Ethos-Core/contracts/TroveManager.sol

48:     uint constant public SECONDS_IN_ONE_MINUTE = 60;

53:     uint constant public MINUTE_DECAY_FACTOR = 999037758833783000;

54:     uint constant public override REDEMPTION_FEE_FLOOR = DECIMAL_PRECISION / 1000 * 5; // 0.5%

55:     uint constant public MAX_BORROWING_FEE = DECIMAL_PRECISION / 100 * 5; // 5%

61:     uint constant public BETA = 2;

Link to code

File: Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol

24:     address public constant VELO_ROUTER = 0xa132DAB612dB5cB9fC9Ac426A0Cc215A3423F9c9;

25:     ILendingPoolAddressesProvider public constant ADDRESSES_PROVIDER =

27:     IAaveProtocolDataProvider public constant DATA_PROVIDER =

29:     IRewardsController public constant REWARDER = IRewardsController(0x6A0406B8103Ec68EE9A713A073C7bD587c5e04aD);

Link to code

File: Ethos-Vault/contracts/ReaperVaultV2.sol

40:     uint256 public constant DEGRADATION_COEFFICIENT = 10**18; // The unit for calculating profit degradation.

41:     uint256 public constant PERCENT_DIVISOR = 10000;

73:     bytes32 public constant DEPOSITOR = keccak256("DEPOSITOR");

74:     bytes32 public constant STRATEGIST = keccak256("STRATEGIST");

75:     bytes32 public constant GUARDIAN = keccak256("GUARDIAN");

76:     bytes32 public constant ADMIN = keccak256("ADMIN");

Link to code

File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol

23:     uint256 public constant PERCENT_DIVISOR = 10_000;

24:     uint256 public constant UPGRADE_TIMELOCK = 48 hours; // minimum 48 hours for RF

25:     uint256 public constant FUTURE_NEXT_PROPOSAL_TIME = 365 days * 100;

49:     bytes32 public constant KEEPER = keccak256("KEEPER");

50:     bytes32 public constant STRATEGIST = keccak256("STRATEGIST");

51:     bytes32 public constant GUARDIAN = keccak256("GUARDIAN");

52:     bytes32 public constant ADMIN = keccak256("ADMIN");

Link to code

<a name="GAS-12"></a>[GAS-12] Use != 0 instead of > 0 for unsigned integer comparison

Instances (27):

File: Ethos-Core/contracts/ActivePool.sol

278:         if (vars.netAssetMovement > 0) {

Link to code

File: Ethos-Core/contracts/BorrowerOperations.sol

128:         assert(MIN_NET_DEBT > 0);

197:         assert(vars.compositeDebt > 0);

301:         assert(msg.sender == _borrower || (msg.sender == stabilityPoolAddress && _collTopUp > 0 && _LUSDChange == 0));

337:         if (!_isDebtIncrease && _LUSDChange > 0) {

552:         require(_LUSDChange > 0, "BorrowerOps: Debt increase requires non-zero debtChange");

Link to code

File: Ethos-Core/contracts/LQTY/LQTYStaking.sol

152:         if (_LQTYamount > 0) {

181:         if (totalLQTYStaked > 0) {collFeePerLQTYStaked = _collFee.mul(DECIMAL_PRECISION).div(totalLQTYStaked);}

191:         if (totalLQTYStaked > 0) {LUSDFeePerLQTYStaked = _LUSDFee.mul(DECIMAL_PRECISION).div(totalLQTYStaked);}

266:         require(currentStake > 0, 'LQTYStaking: User must have a non-zero stake');  

270:         require(_amount > 0, 'LQTYStaking: Amount must be non-zero');

Link to code

File: Ethos-Core/contracts/LUSDToken.sol

279:         if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {

Link to code

File: Ethos-Core/contracts/StabilityPool.sol

591:         assert(newP > 0);

870:         require(_initialDeposit > 0, 'StabilityPool: User must have a non-zero deposit');

874:         require(_amount > 0, 'StabilityPool: Amount must be non-zero');

Link to code

File: Ethos-Core/contracts/TroveManager.sol

424:             if (singleLiquidation.collSurplus > 0) {

452:         if (_LUSDInStabPool > 0) {

551:         require(totals.totalDebtInSequence > 0);

563:         if (totals.totalCollSurplus > 0) {

754:         require(totals.totalDebtInSequence > 0);

766:         if (totals.totalCollSurplus > 0) {

916:         if (_LUSD > 0) {

920:         if (_collAmount > 0) {

1224:             assert(totalStakesSnapshot[_collateral] > 0);

1414:         assert(newBaseRate > 0); // Base rate is always non-zero after redemption

Link to code

File: Ethos-Vault/contracts/ReaperVaultV2.sol

502:         } else if (_roi > 0) {

518:         } else if (vars.available > 0) {

Link to code

<a name="GAS-13"></a>[GAS-13] internal functions not called by the contract should be removed

If the functions are required by an interface, the contract should inherit from that interface and use the override keyword

Instances (1):

File: Ethos-Core/contracts/BorrowerOperations.sol

432:     function _getUSDValue(uint _coll, uint _price, uint256 _collDecimals) internal pure returns (uint) {

Link to code


Exclusive Findings


Gas Optimizations Report


[G-01] Splitting require which contains && saves gas


Description:

Using multiple require instead of operator && can save more gas. When a require statement has 2 or more expressions with &&, separate them into individual expression results in lesser gas.

Lines of Code:


[G-02] Reduce the size of error messages (Long revert Strings)


Description:

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Recommendation:

Shorten the revert strings to fit in 32 bytes.

Or in contracts using solc version 0.8.4 or greater use the Custom Errors feature.

Lines of Code:


[G-03] Use Custom Errors rather than revert() / require() strings to save deployment gas [68 gas per instance]


Description:

Custom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they’re hitby avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas.

https://blog.soliditylang.org/2021/04/21/custom-errors/

Recommendation:

Use the Custom Errors feature.

Lines of Code:


[G-04] Usage of uint/int smaller than 32 bytes (256 bits) incurs overhead


Description:

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size. Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so.

Recommendation:

Use a larger size then downcast where needed

Lines of Code:


[G-05] ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for-loop and while-loops


Description:

In Solidity 0.8+, there’s a default overflow check on unsigned integers. It’s possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline.

Recommendation:

Using Solidity's unchecked block saves the overflow checks.

Proof Of Concept:

https://github.com/byterocket/c4-common-issues/blob/main/0-Gas-Optimizations.md#g011---unnecessary-checked-arithmetic-in-for-loop

Lines of Code:


[G-06] Optimize names to save gas [22 gas per instance]


Description:

Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions.

Recommendation:

Find a lower method ID name for the most called functions for example Call() vs. Call1() is cheaper by 22 gas. For example, the function IDs in the L1GraphTokenGateway.sol contract will be the most used; A lower method ID may be given.

Proof Of Concept:

https://medium.com/joyso/solidity-how-does-function-name-affect-gas-consumption-in-smart-contract-47d270d8ac92

Lines of Code:


[G-07] Setting the constructor to payable [~13 gas per instance]


Lines of Code:


[G-08] Comparison operators


Description:

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.

Recommendation:

Replace <= with <, and >= with >. Do not forget to increment/decrement the compared variable.

Lines of Code:


[G-09] Use a more recent version of solidity


Description:

Solidity 0.8.10 has a useful change that reduced gas costs of external calls which expect a return value.

In 0.8.15 the conditions necessary for inlining are relaxed. Benchmarks show that the change significantly decreases the bytecode size (which impacts the deployment cost) while the effect on the runtime gas usage is smaller.

In 0.8.17 prevent the incorrect removal of storage writes before calls to Yul functions that conditionally terminate the external EVM call; Simplify the starting offset of zero-length operations to zero. More efficient overflow checks for multiplication.

Lines of Code:


[G-10] x += y costs more gas than x = x + y for state variables


Lines of Code:


[G-11] <array>.length should not be looked up in every loop of a for-loop


Description:

The overheads outlined below are PER LOOP, excluding the first loop

  • storage arrays incur a Gwarmaccess (100 gas)

  • memory arrays use MLOAD (3 gas)

  • calldata arrays use CALLDATALOAD (3 gas)

Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

Lines of Code:

#0 - c4-judge

2023-03-09T14:18:14Z

trust1995 marked the issue as grade-a

#1 - c4-judge

2023-03-10T17:06:39Z

trust1995 marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter