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
Rank: 63/154
Findings: 2
Award: $103.33
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: GalloDaSballo
Also found by: 0x3b, 0xAgro, 0xSmartContract, 0xTheC0der, 0xackermann, 0xnev, 0xsomeone, ABA, BRONZEDISC, Bjorn_bug, Bnke0x0, Breeje, Co0nan, CodeFoxInc, CodingNameKiki, DadeKuma, DeFiHackLabs, IceBear, Josiah, Kaysoft, Lavishq, MohammedRizwan, PaludoX0, PawelK, Phantasmagoria, Raiders, RaymondFam, Rickard, Rolezn, Sathish9098, SleepingBugs, SuperRayss, UdarTeam, Udsen, Viktor_Cortess, arialblack14, ast3ros, bin2chen, brgltd, btk, catellatech, ch0bu, chaduke, chrisdior4, codeislight, cryptonue, delfin454000, descharre, dontonka, emmac002, fs0c, hacker-dom, hansfriese, imare, lukris02, luxartvinsec, martin, matrix_0wl, peanuts, rbserver, shark, tnevler, trustindistrust, tsvetanovv, vagrant, yongskiws, zzzitron
61.2601 USDC - $61.26
Exclusive findings is below the automated findings.
Issue | Instances | |
---|---|---|
NC-1 | Missing checks for address(0) when assigning values to address state variables | 26 |
NC-2 | require() / revert() statements should have descriptive reason strings | 10 |
NC-3 | Return values of approve() not checked | 5 |
NC-4 | TODO Left in the code | 2 |
NC-5 | Event is missing indexed fields | 93 |
NC-6 | Functions not used internally could be marked external | 2 |
NC-7 | Typos | 32 |
address(0)
when assigning values to address state variablesInstances (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;
File: Ethos-Core/contracts/BorrowerOperations.sol 146: stabilityPoolAddress = _stabilityPoolAddress; 147: gasPoolAddress = _gasPoolAddress; 152: lqtyStakingAddress = _lqtyStakingAddress;
File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol 75: stabilityPoolAddress = _stabilityPoolAddress;
File: Ethos-Core/contracts/LQTY/LQTYStaking.sol 88: troveManagerAddress = _troveManagerAddress; 89: borrowerOperationsAddress = _borrowerOperationsAddress; 90: activePoolAddress = _activePoolAddress;
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;
File: Ethos-Core/contracts/TroveManager.sol 266: borrowerOperationsAddress = _borrowerOperationsAddress; 271: gasPoolAddress = _gasPoolAddress;
require()
/ revert()
statements should have descriptive reason stringsInstances (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);
approve()
not checkedNot 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);
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.
indexed
fieldsIndex 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);
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);
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);
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);
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);
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);
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);
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(
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) {
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
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)
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.
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
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 ---
File: Ethos-Core/contracts/StabilityPool.sol - 728: // Internal function, used to calculcate compounded deposits. + 728: // Internal function, used to calculate compounded deposits.
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
File: Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol - 1: // SPDX-License-Identifier: BUSL1.1 + 1: // SPDX-License-Identifier: BSL-1.0
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,
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
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
Issue | Instances | |
---|---|---|
L-1 | abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256() | 1 |
L-2 | Do not use deprecated library functions | 1 |
L-3 | Empty Function Body - Consider commenting why | 2 |
L-4 | Initializers could be front-run | 8 |
L-5 | Unsafe ERC20 operation(s) | 4 |
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',
Instances (1):
File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol 74: IERC20Upgradeable(want).safeApprove(vault, type(uint256).max);
Instances (2):
File: Ethos-Vault/contracts/ReaperVaultERC4626.sol 24: ) ReaperVaultV2(_token, _name, _symbol, _tvlCap, _treasury, _strategists, _multisigRoles) {}
File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol 61: constructor() initializer {}
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(
File: Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol 62: function initialize( 67: ) public initializer { 70: __ReaperBaseStrategy_init(_vault, want, _strategists, _multisigRoles);
File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol 61: constructor() initializer {} 63: function __ReaperBaseStrategy_init( 69: __UUPSUpgradeable_init(); 70: __AccessControlEnumerable_init();
Instances (4):
File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol 103: OathToken.transferFrom(msg.sender, address(this), amount); 127: OathToken.transfer(_account, _OathAmount);
File: Ethos-Core/contracts/LQTY/LQTYStaking.sol 135: lusdToken.transfer(msg.sender, LUSDGain); 171: lusdToken.transfer(msg.sender, LUSDGain);
Issue | Instances | |
---|---|---|
M-1 | Centralization Risk for trusted owners | 21 |
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 {
File: Ethos-Core/contracts/BorrowerOperations.sol 18: contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOperations {
File: Ethos-Core/contracts/CollateralConfig.sol 15: contract CollateralConfig is ICollateralConfig, CheckContract, Ownable { 50: ) external override onlyOwner { 89: ) external onlyOwner checkCollateral(_collateral) {
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 {
File: Ethos-Core/contracts/LQTY/LQTYStaking.sol 18: contract LQTYStaking is ILQTYStaking, Ownable, CheckContract, BaseMath { 76: onlyOwner
File: Ethos-Core/contracts/StabilityPool.sol 146: contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {
File: Ethos-Core/contracts/TroveManager.sol 18: contract TroveManager is LiquityBase, /*Ownable,*/ CheckContract, ITroveManager {
File: Ethos-Vault/contracts/ReaperVaultV2.sol 21: contract ReaperVaultV2 is ReaperAccessControl, ERC20, IERC4626Events, AccessControlEnumerable, ReentrancyGuard { 21: contract ReaperVaultV2 is ReaperAccessControl, ERC20, IERC4626Events, AccessControlEnumerable, ReentrancyGuard {
File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol 15: ReaperAccessControl,
Issue | Count |
---|---|
Low Risk Issues | 3 |
Non Critical Issues | 12 |
Total | 15 |
No. | Issue |
---|---|
1 | Loss of precision due to rounding |
2 | Always use safeTransferFrom instead of transferFrom |
3 | Owner can renounce Ownership |
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:
safeTransferFrom
instead of transferFrom
Lines of Code:
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:
onlyOwner
functions:
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:
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:
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:
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; } }
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:
Description:
For example, 1000
should be written as 1_000
Lines of Code:
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:
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:
Description:
For example, 365 days
should be written as 365 days; // 31_536_000 (365*24*60*60)
Lines of Code:
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:
// 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:
// 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
🌟 Selected for report: c3phas
Also found by: 0x3b, 0x6980, 0x73696d616f, 0xSmartContract, 0xackermann, 0xhacksmithh, 0xsomeone, Bnke0x0, Bough, Budaghyan, Darshan, DeFiHackLabs, Deivitto, GalloDaSballo, JCN, LethL, Madalad, MiniGlome, Morraez, P-384, PaludoX0, Phantasmagoria, Praise, RHaO-sec, Rageur, RaymondFam, ReyAdmirado, Rickard, Rolezn, SaeedAlipoor01988, Saintcode_, Sathish9098, TheSavageTeddy, Tomio, Viktor_Cortess, abiih, arialblack14, atharvasama, banky, codeislight, cryptonue, ddimitrov22, dec3ntraliz3d, descharre, dharma09, emmac002, favelanky, hl_, hunter_w3b, kaden, kodyvim, matrix_0wl, oyc_109, pavankv, scokaf, seeu, yamapyblack
42.0697 USDC - $42.07
Exclusive findings is below the automated findings.
Automated findings output for the Ethos-Core package and Ethos-Vault package.
Issue | Instances | |
---|---|---|
GAS-1 | Use assembly to check for address(0) | 8 |
GAS-2 | Using bools for storage incurs overhead | 9 |
GAS-3 | Cache array length outside of loop | 8 |
GAS-4 | State variables should be cached in stack variables rather than re-reading them from storage | 10 |
GAS-5 | Use calldata instead of memory for function arguments that do not get mutated | 1 |
GAS-6 | Use Custom Errors | 73 |
GAS-7 | Don't initialize variables with default value | 23 |
GAS-8 | Long revert strings | 35 |
GAS-9 | Functions guaranteed to revert when called by normal users can be marked payable | 7 |
GAS-10 | ++i costs less gas than i++ , especially when it's used in for -loops (--i /i-- too) | 17 |
GAS-11 | Using private rather than public for constants, saves gas | 30 |
GAS-12 | Use != 0 instead of > 0 for unsigned integer comparison | 27 |
GAS-13 | internal functions not called by the contract should be removed | 1 |
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");
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) &&
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;
File: Ethos-Core/contracts/CollateralConfig.sol 18: bool public initialized = false;
File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol 21: bool public initialized = false;
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;
File: Ethos-Vault/contracts/ReaperVaultV2.sol 49: bool public emergencyShutdown;
File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol 30: bool public emergencyExit;
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++) {
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++) {
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++) {
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++) {
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);
File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol 113: lastDistributionTime = block.timestamp.add(distributionPeriod); 116: emit LogRewardPerSecond(rewardPerSecond);
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);
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 {
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");
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");
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");
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");
File: Ethos-Core/contracts/LQTY/LQTYStaking.sol 262: require(msg.sender == borrowerOperationsAddress, "LQTYStaking: caller is not BorrowerOps");
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");
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");
File: Ethos-Vault/contracts/ReaperVaultERC4626.sol 270: require(y != 0, "Division by 0");
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");
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");
Instances (23):
File: Ethos-Core/contracts/ActivePool.sol 108: for(uint256 i = 0; i < numCollaterals; i++) {
File: Ethos-Core/contracts/CollateralConfig.sol 56: for(uint256 i = 0; i < _collaterals.length; i++) {
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++) {
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++) {
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()) {
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()) {
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;
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");
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");
File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol 133: require(msg.sender == stabilityPoolAddress, "CommunityIssuance: caller is not SP");
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');
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");
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');
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%");
File: Ethos-Vault/contracts/abstract/ReaperBaseStrategyV4.sol 98: require(_amount <= balanceOf(), "Ammount must be less than balance");
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 {
File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol 101: function fund(uint amount) external onlyOwner { 120: function updateDistributionPeriod(uint256 _newDistributionPeriod) external onlyOwner {
++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++) {
File: Ethos-Core/contracts/CollateralConfig.sol 56: for(uint256 i = 0; i < _collaterals.length; i++) {
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++) {
File: Ethos-Core/contracts/LUSDToken.sol 286: _nonces[owner]++, deadline))));
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++) {
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++) {
File: Ethos-Vault/contracts/ReaperVaultERC4626.sol 273: if (x % y != 0) q++;
private
rather than public
for constants, saves gasIf 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";
File: Ethos-Core/contracts/BorrowerOperations.sol 21: string constant public NAME = "BorrowerOperations";
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%
File: Ethos-Core/contracts/LQTY/CommunityIssuance.sol 19: string constant public NAME = "CommunityIssuance";
File: Ethos-Core/contracts/LQTY/LQTYStaking.sol 23: string constant public NAME = "LQTYStaking";
File: Ethos-Core/contracts/StabilityPool.sol 150: string constant public NAME = "StabilityPool"; 197: uint public constant SCALE_FACTOR = 1e9;
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;
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);
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");
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");
Instances (27):
File: Ethos-Core/contracts/ActivePool.sol 278: if (vars.netAssetMovement > 0) {
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");
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');
File: Ethos-Core/contracts/LUSDToken.sol 279: if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {
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');
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
File: Ethos-Vault/contracts/ReaperVaultV2.sol 502: } else if (_roi > 0) { 518: } else if (vars.available > 0) {
internal
functions not called by the contract should be removedIf 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) {
require
which contains &&
saves gasDescription:
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:
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:
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:
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:
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:
Lines of Code:
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:
Lines of Code:
constructor
to payable
[~13 gas per instance]Lines of Code:
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:
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:
x += y
costs more gas than x = x + y
for state variablesLines of Code:
<array>.length
should not be looked up in every loop of a for-loopDescription:
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