Ethos Reserve contest - Phantasmagoria'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: 61/154

Findings: 2

Award: $103.33

QA:

grade-b

Gas:

grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Ethos Reserve

Findings Information

🌟 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

Awards

61.2601 USDC - $61.26

Labels

bug

grade-b

QA (Quality Assurance)

edited-by-warden

Q-54

External Links

Link to GitHub issue

Low Risk Issues

	Issue	Instances
1	Deprecated safeApprove() function	1
2	_safeMint() should be used rather than _mint() wherever possible	2

Total: 3 instances

Non-Critical Issues

	Issue	Instances
1	Imports can be grouped together	3
2	Constant redefined elsewhere	2
3	Named imports can be used	3
4	Remove console.log import	5
5	Interchangeable usage of uint and uint256	3
6	Add a limit for the maximum number of characters per line	1
7	Can be rewritten in many lines	2
8	NatSpec comments should be increased in contracts	2
9	Commented code	1
10	Unused events	2
11	Proper use of _ as a function name prefix	1
12	Use named returns for local variables where it is possible	22
13	No need to use return	4
14	Code is not properly formatted	5
15	Use _ with big numbers	1

Total: 57 instances

[L-01] Deprecated safeApprove() function

Deprecated in favor of safeIncreaseAllowance() and safeDecreaseAllowance(). If only setting the initial allowance to the value that means infinite, safeIncreaseAllowance() can be used instead

Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol

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

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol#L74

[L-02] _safeMint() should be used rather than _mint() wherever possible

_mint() is discouraged in favor of _safeMint() which ensures that the recipient is either an EOA or implements IERC721Receiver. Both OpenZeppelin and solmate have versions of this function

Ethos-Vault/contracts/ReaperVaultV2.sol

336: _mint(_receiver, shares);

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L336

Ethos-Vault/contracts/ReaperVaultV2.sol

467: _mint(treasury, shares);

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L467

[NC-1] Imports can be grouped together

Consider importing OZ first, then all interfaces, then all utils

Ethos-Vault/contracts/ReaperVaultV2.sol

5: import "./interfaces/IERC4626Events.sol";
6: import "./interfaces/IStrategy.sol";
7: import "./libraries/ReaperMathUtils.sol";
8: import "./mixins/ReaperAccessControl.sol";
9: import "@openzeppelin/contracts/access/AccessControlEnumerable.sol";
10: import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
11: import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
12: import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
13: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
14: import "@openzeppelin/contracts/utils/math/Math.sol";

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L5-L14

Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol

5: import "../interfaces/IStrategy.sol";
6: import "../interfaces/IVault.sol";
7: import "../libraries/ReaperMathUtils.sol";
8: import "../mixins/ReaperAccessControl.sol";
9: import "@openzeppelin/contracts-upgradeable/access/AccessControlEnumerableUpgradeable.sol";
10: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
11: import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
12: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol#L5-L12

Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol

5: import "./abstract/ReaperBaseStrategyv4.sol";
6: import "./interfaces/IAToken.sol";
7: import "./interfaces/IAaveProtocolDataProvider.sol";
8: import "./interfaces/ILendingPool.sol";
9: import "./interfaces/ILendingPoolAddressesProvider.sol";
10: import "./interfaces/IRewardsController.sol";
11: import "./libraries/ReaperMathUtils.sol";
12: import "./mixins/VeloSolidMixin.sol";
13: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";
14: import "@openzeppelin/contracts-upgradeable/utils/math/MathUpgradeable.sol";

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol#L5-L14

[NC-2] Constant redefined elsewhere

Consider defining in only one contract so that values cannot become out of sync when only one location is updated. A cheap way to store constants in a single location is to create an internal constant in a library. If the variable is a local cache of another contract’s value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don’t get out of sync.

Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol

50: bytes32 public constant STRATEGIST = keccak256("STRATEGIST");
51: bytes32 public constant GUARDIAN = keccak256("GUARDIAN");
52: bytes32 public constant ADMIN = keccak256("ADMIN");

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol#L50-L52

Ethos-Vault/contracts/ReaperVaultV2.sol

74: bytes32 public constant STRATEGIST = keccak256("STRATEGIST");
75: bytes32 public constant GUARDIAN = keccak256("GUARDIAN");
76: bytes32 public constant ADMIN = keccak256("ADMIN");

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L74-L76

[NC-3] Named imports can be used

It’s possible to name the imports to improve code readability.

Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol

13: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";
14: import "@openzeppelin/contracts-upgradeable/utils/math/MathUpgradeable.sol";

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol#L13-L14

Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol

9: import "@openzeppelin/contracts-upgradeable/access/AccessControlEnumerableUpgradeable.sol";
10: import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
11: import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
12: import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol#L9-L12

Ethos-Vault/contracts/ReaperVaultV2.sol

9: import "@openzeppelin/contracts/access/AccessControlEnumerable.sol";
10: import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
11: import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
12: import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
13: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
14: import "@openzeppelin/contracts/utils/math/Math.sol";

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L9-L14

[NC-4] Remove console.log import

Ethos-Core/contracts/BorrowerOperations.sol

15: import "./Dependencies/console.sol";

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/BorrowerOperations.sol#L15

Ethos-Core/contracts/ActivePool.sol

15: import "./Dependencies/console.sol";

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/ActivePool.sol#L15

Ethos-Core/contracts/StabilityPool.sol

18: import "./Dependencies/console.sol";

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/StabilityPool.sol#L18

Ethos-Core/contracts/LQTY/LQTYStaking.sol

9: import "./Dependencies/console.sol";

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/LQTY/LQTYStaking.sol#L9

Ethos-Core/contracts/LUSDToken.sol

9: import "./Dependencies/console.sol";

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/LUSDToken.sol#L9

[NC-5] Interchangeable usage of uint and uint256

Consider using only one approach throughout the codebase, e.g. only uint or only uint256.

Ethos-Core/contracts/BorrowerOperations.sol

557: uint _price,
558: uint256 _CCR,
559: uint256 _collateralDecimals

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/BorrowerOperations.sol#L557-L559

Ethos-Core/contracts/BorrowerOperations.sol

663: uint _coll,
664: uint _debt,
665: uint _collChange,

667: uint _debtChange,

669: uint256 _collateralDecimals

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/BorrowerOperations.sol#L663-L669

Ethos-Core/contracts/TroveManager.sol

362: uint _ICR,
363: uint _LUSDInStabPool,
364: uint _TCR,
365: uint _price,
366: uint256 _MCR

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/TroveManager.sol#L362-L366

[NC-6] Add a limit for the maximum number of characters per line

The solidity documentation recommends a maximum of 120 characters. Consider adding a limit of 120 characters or less to prevent large lines. https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/TroveManager.sol#L934

[NC-7] Can be rewritten in many lines

To increase readability consider to rewrite following functions like this https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/BorrowerOperations.sol#L268 before:

Ethos-Core/contracts/BorrowerOperations.sol

268: function adjustTrove(address _collateral, uint _maxFeePercentage, uint _collTopUp, uint _collWithdrawal, uint _LUSDChange, bool _isDebtIncrease, address _upperHint, address _lowerHint) external override {}

after:

function adjustTrove(
    address _collateral,
    uint _maxFeePercentage,
    uint _collTopUp,
    uint _collWithdrawal,
    uint _LUSDChange,
    bool _isDebtIncrease,
    address _upperHint,
    address _lowerHint
    ) 
	external 
    {}

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/BorrowerOperations.sol#L282 before:

Ethos-Core/contracts/BorrowerOperations.sol

282: function _adjustTrove(address _borrower, address _collateral, uint _collTopUp, uint _collWithdrawal, uint _LUSDChange, bool _isDebtIncrease, address _upperHint, address _lowerHint, uint _maxFeePercentage) internal {}

after:

function _adjustTrove(
    address _borrower, 
    address _collateral, 
    uint _collTopUp, 
    uint _collWithdrawal, 
    uint _LUSDChange, 
    bool _isDebtIncrease, 
    address _upperHint, 
    address _lowerHint, 
    uint _maxFeePercentage
    ) 
      internal 
    {}

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

Consider adding NATSPEC on all public/external functions to improve documentation

Ethos-Core/contracts/ActivePool.sol

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

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/ActivePool.sol#L144

Ethos-Core/contracts/ActivePool.sol

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

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/ActivePool.sol#L138

[NC-9] Commented code

Remove this comment because it's already not needed

Ethos-Core/contracts/TroveManager.sol

1413: //assert(newBaseRate <= DECIMAL_PRECISION); // This is already enforced in the line above

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/TroveManager.sol#L1413

[NC-10] Unused events

These events are not used anywhere else

Ethos-Core/contracts/StabilityPool.sol

240: event DefaultPoolAddressChanged(address _newDefaultPoolAddress);

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/StabilityPool.sol#L240

Ethos-Core/contracts/LQTY/LQTYStaking.sol

50: event LUSDTokenAddressSet(address _lusdTokenAddress);

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/LQTY/LQTYStaking.sol#L50

[NC-11] Proper use of _ as a function name prefix

It is a common pattern is to prefix internal and private function names with _

Ethos-Vault/contracts/ReaperVaultERC4626.sol

269: function roundUpDiv(uint256 x, uint256 y) internal pure returns (uint256)

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultERC4626.sol#L269

[NC-12] Use named returns for local variables where it is possible

Consider using local variables, like in the examples below https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/BorrowerOperations.sol#L432

Ethos-Core/contracts/BorrowerOperations.sol

432: function _getUSDValue(uint _coll, uint _price, uint256 _collDecimals) internal pure returns (uint){
433:    uint usdValue = _price.mul(_coll).div(10**_collDecimals);
434:
435:    return usdValue;
436: }

fix:

function _getUSDValue(uint _coll, uint _price, uint256 _collDecimals) internal pure returns (uint usdValue) {
    uint usdValue = _price.mul(_coll).div(10**_collDecimals);
}

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/BorrowerOperations.sol#L455

Ethos-Core/contracts/BorrowerOperations.sol

455: function _updateTroveFromAdjustment
456:    (
457:         ITroveManager _troveManager,
458:         address _borrower,
459:         address _collateral,
460:         uint _collChange,
461:         bool _isCollIncrease,
462:         uint _debtChange,
463:         bool _isDebtIncrease
464:     )
465:         internal
466:         returns (uint, uint)
467:     {
468:         uint newColl = (_isCollIncrease) ? _troveManager.increaseTroveColl(_borrower, _collateral, _collChange)
469:                                         : _troveManager.decreaseTroveColl(_borrower, _collateral, _collChange);
470:         uint newDebt = (_isDebtIncrease) ? _troveManager.increaseTroveDebt(_borrower, _collateral, _debtChange)
471:                                         : _troveManager.decreaseTroveDebt(_borrower, _collateral, _debtChange);
472: 
473:        return (newColl, newDebt);
474:    }

fix:

function _updateTroveFromAdjustment
    (
        ITroveManager _troveManager,
        address _borrower,
        address _collateral,
        uint _collChange,
        bool _isCollIncrease,
        uint _debtChange,
        bool _isDebtIncrease
    )
        internal
        returns (uint newColl, uint newDebt)
{
    uint newColl = (_isCollIncrease) ? _troveManager.increaseTroveColl(_borrower, _collateral, _collChange)
                                        : _troveManager.decreaseTroveColl(_borrower, _collateral, _collChange);
    uint newDebt = (_isDebtIncrease) ? _troveManager.increaseTroveDebt(_borrower, _collateral, _debtChange)
                                        : _troveManager.decreaseTroveDebt(_borrower, _collateral, _debtChange);
}

[NC-13] No need to use return

Do not use return at the end of the function

Ethos-Core/contracts/TroveManager.sol

353: return singleLiquidation;

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/TroveManager.sol#L353

Ethos-Core/contracts/TroveManager.sol

436: return singleLiquidation;

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/TroveManager.sol#L436

Ethos-Core/contracts/TroveManager.sol

1332: return index;

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/TroveManager.sol#L1332

Ethos-Core/contracts/StabilityPool.sol

543: return (collGainPerUnitStaked, LUSDLossPerUnitStaked);

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/StabilityPool.sol#L543

[NC-14] Code is not properly formatted

Use only " or ' throughout codebase. Some instances:

Ethos-Core/contracts/StabilityPool.sol

5: import './Interfaces/IBorrowerOperations.sol';
6: import "./Interfaces/ICollateralConfig.sol";
7: import './Interfaces/IStabilityPool.sol';
8: import './Interfaces/IBorrowerOperations.sol';
9: import './Interfaces/ITroveManager.sol';
10: import './Interfaces/ILUSDToken.sol';
11: import './Interfaces/ISortedTroves.sol';
12: import "./Interfaces/ICommunityIssuance.sol";
13: import "./Dependencies/LiquityBase.sol";
14: import "./Dependencies/SafeMath.sol";

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/StabilityPool.sol#L5-L14

Ethos-Core/contracts/StabilityPool.sol

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

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/StabilityPool.sol#L870

Ethos-Core/contracts/StabilityPool.sol

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

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/StabilityPool.sol#L874 Unneeded space in require statements

Ethos-Core/contracts/BorrowerOperations.sol

633: require (_netDebt >= MIN_NET_DEBT, "BorrowerOps: Trove's net debt must be greater than minimum");

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/BorrowerOperations.sol#L633

Ethos-Core/contracts/TroveManager.sol

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

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/TroveManager.sol#L1539

[NC-15] Use _ with big numbers

Consider using underscores for number values to improve readability. https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L41 before:

Ethos-Vault/contracts/ReaperVaultV2.sol

41: uint256 public constant PERCENT_DIVISOR = 10000;

after:

uint256 public constant PERCENT_DIVISOR = 10_000;

#0 - c4-judge
2023-03-09T13:10:17Z

trust1995 marked the issue as grade-b

Findings Information

🌟 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

Awards

42.0697 USDC - $42.07

Labels

bug

G (Gas Optimization)

grade-b

edited-by-warden

G-47

External Links

Link to GitHub issue

Gas Optimizations

	Issue	Instances
1	`require()`/`revert()` strings longer than 32 bytes cost extra gas	14
2	Use custom errors rather than `revert()`/`require()` strings to save gas	14
3	Splitting `require()` statements that use `&&` saves gas	4
4	Event fields are redundant	1
5	Use a more recent version of solidity	8
6	Usage of `uints`/`ints` smaller than 32 bytes (256 bits) incurs overhead	2
7	`require()` or `revert()` statements that check input arguments should be at the top of the function	6
8	`internal` functions only called once can be inlined to save gas	53
9	Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate	8
10	Using `storage` instead of `memory` for structs/arrays saves gas	2
11	Multiple accesses of a mapping/array should use a local variable cache	6
12	Not using the named return variables when a function returns, wastes deployment gas	11
13	Avoid compound assignment operator in state variables	9
14	Should use arguments instead of state variable	1
15	Expressions for constant values such as a call to `keccak256()`, should use immutable rather than constant	8
16	`abi.encode()` is less efficient than `abi.encodePacked()`	1
17	Unused imports	3
18	Duplicated `require()`/`revert()` checks should be refactored to a `modifier` or `function`	2
19	The result of external function calls should be cached rather than re-calling the function	9

Total: 162 instances

[G-01] require()/revert() strings longer than 32 bytes cost extra gas

[G-02] Use custom errors rather than revert()/require() strings to save gas

[G-03] Splitting require() statements that use && saves gas

Instead of using the && operator in a single require statement to check multiple conditions,using multiple require statements with 1 condition per require statement will save 8 GAS per && The gas difference would only be realized if the revert condition is realized(met). There are 4 instances of this issue https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/BorrowerOperations.sol#L653 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/TroveManager.sol#L1539 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/LUSDToken.sol#L347 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/LUSDToken.sol#L352

[G-04] Event fields are redundant

block.number and block.timestamp are added to the event information by default, so adding them manually will waste additional gas. There is 1 instances of this issue https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/TroveManager.sol#L1505

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

[G-06] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

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. There are 2 instances of this issue https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/LUSDToken.sol#L35 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol#L35

[G-07] require() or revert() statements that check input arguments should be at the top of the function

Checks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting a Gcoldsload (2100 gas*) in a function that may ultimately revert in the unhappy case. There are 6 instances of this issue https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/CollateralConfig.sol#L52-L54 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/ActivePool.sol#L93 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/ActivePool.sol#L127 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L322 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L151 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol#L97

[G-08] internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls. When we define internal functions to perform computation:

[G-09] Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate

[G-10] Using storage instead of memory for structs/arrays saves gas

When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read.

Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct There are 2 instances of this issue https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/TroveManager.sol#L313 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/BorrowerOperations.sol#L175 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/BorrowerOperations.sol#L283

[G-11] Multiple accesses of a mapping/array should use a local variable cache

[G-12] Not using the named return variables when a function returns, wastes deployment gas

[G-13] Avoid compound assignment operator in state variables

[G-14] Should use arguments instead of state variable

There is 1 instances of this issue https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L581

[G-15] Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

[G-16] abi.encode() is less efficient than abi.encodePacked()

Changing abi.encode function to abi.encodePacked can save gas since the abi.encode function pads extra null bytes at the end of the call data, which is unnecessary. Also, in general, abi.encodePacked is more gas-efficient (see Solidity-Encode-Gas-Comparison) There is 1 instances of this issue https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/LUSDToken.sol#L284

[G-17] Unused imports

Importing pointless files costs gas during deployment and is a bad coding practice that is important to ignore. There are 3 instances of this issue https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/LQTY/CommunityIssuance.sol#L8 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Core/contracts/LUSDToken.sol#L6 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol#L10

[G-18] Duplicated require()/revert() checks should be refactored to a modifier or function

Saves deployment costs There are 2 instances of this issue https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L155 https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L180

[G-19] The result of external function calls should be cached rather than re-calling the function

There are 9 instances of this issue decimals() and totalSupply() https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L296 totalSupply() https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L331 totalSupply() and _freeFunds() https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultERC4626.sol#L52 totalSupply() https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultERC4626.sol#L67 totalSupply() and _freeFunds() https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultERC4626.sol#L184 balanceOfWant() https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol#L91

#0 - c4-judge
2023-03-09T17:58:52Z

trust1995 marked the issue as grade-b

Ethos Reserve contest - Phantasmagoria's results

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

General Information

Findings Distribution

Researcher Performance

External Links

[Q-54] QA Report - $61.26

Findings Information

Awards

Labels

External Links

Low Risk Issues

Non-Critical Issues

[L-01] Deprecated safeApprove() function

[L-02] _safeMint() should be used rather than _mint() wherever possible

[NC-1] Imports can be grouped together

[NC-2] Constant redefined elsewhere

[NC-3] Named imports can be used

[NC-4] Remove console.log import

[NC-5] Interchangeable usage of uint and uint256

[NC-6] Add a limit for the maximum number of characters per line

[NC-7] Can be rewritten in many lines

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

[NC-9] Commented code

[NC-10] Unused events

[NC-11] Proper use of _ as a function name prefix

[NC-12] Use named returns for local variables where it is possible

[NC-13] No need to use return

[NC-14] Code is not properly formatted

[NC-15] Use _ with big numbers

#0 - c4-judge2023-03-09T13:10:17Z

[G-47] Gas Report - $42.07

Findings Information

Awards

Labels

External Links

Gas Optimizations

[G-01] require()/revert() strings longer than 32 bytes cost extra gas

[G-02] Use custom errors rather than revert()/require() strings to save gas

[G-03] Splitting require() statements that use && saves gas

[G-04] Event fields are redundant

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

[G-06] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

[G-07] require() or revert() statements that check input arguments should be at the top of the function

[G-08] internal functions only called once can be inlined to save gas

[G-09] Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate

[G-10] Using storage instead of memory for structs/arrays saves gas

[G-11] Multiple accesses of a mapping/array should use a local variable cache

[G-12] Not using the named return variables when a function returns, wastes deployment gas

[G-13] Avoid compound assignment operator in state variables

[G-14] Should use arguments instead of state variable

[G-15] Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

[G-16] abi.encode() is less efficient than abi.encodePacked()

[G-17] Unused imports

[G-18] Duplicated require()/revert() checks should be refactored to a modifier or function

[G-19] The result of external function calls should be cached rather than re-calling the function

#0 - c4-judge2023-03-09T17:58:52Z

#0 - c4-judge
2023-03-09T13:10:17Z

#0 - c4-judge
2023-03-09T17:58:52Z