Ethos Reserve contest - tnevler's results

Report

Non-Critical Issues

[N-1]: Function defines a named return variable but then it uses return statements

Context:

1. return singleLiquidation; L353
2. return zeroVals; L433
3. return singleLiquidation; L436
4. return newTotals; L912
5. return _addTroveOwnerToArray(_borrower, _collateral); L1318
6. return index; L1332
7. return (collGainPerUnitStaked, LUSDLossPerUnitStaked); L543
8. return _getCollateralGainFromSnapshots(initialDeposit, snapshots); L632
9. return address(token); L30
10. return balance(); L38
11. return (assets * totalSupply()) / _freeFunds(); L53
12. return (shares * _freeFunds()) / totalSupply(); L68
13. if (tvlCap == type(uint256).max) return type(uint256).max; L81
14. return tvlCap - balance(); L82
15. return convertToShares(assets); L97
16. shares = _deposit(assets, receiver); L111
17. if (tvlCap == type(uint256).max) return type(uint256).max; L124
18. if (totalSupply() == 0) return shares; L140
19. return convertToAssets(balanceOf(owner)); L166
20. if (totalSupply() == 0 || _freeFunds() == 0) return 0; L184
21. return balanceOf(owner); L221
22. return convertToAssets(shares); L241
23. return balanceOfWant(); L106

Recommendation:

Choose named return variable or return statement. It is unnecessary to use both.

[N-2]: Use of immutable instead of constant keccak expression

Context:

1. bytes32 public constant DEPOSITOR = keccak256("DEPOSITOR"); L73
2. bytes32 public constant STRATEGIST = keccak256("STRATEGIST"); L74
3. bytes32 public constant GUARDIAN = keccak256("GUARDIAN"); L75
4. bytes32 public constant ADMIN = keccak256("ADMIN"); L76
5. bytes32 public constant KEEPER = keccak256("KEEPER"); L49
6. bytes32 public constant STRATEGIST = keccak256("STRATEGIST"); L50
7. bytes32 public constant GUARDIAN = keccak256("GUARDIAN"); L51
8. bytes32 public constant ADMIN = keccak256("ADMIN"); L52

Description:

According to official solidity documentation for a constant variable, the expression assigned to it is copied to all the places where it is accessed and also re-evaluated each time. It is recommended to use immutable instead.

[N-3]: Variable is unused

Context:

uint LUSDLoss = initialDeposit.sub(compoundedLUSDDeposit); // Needed only for event log L384

[N-4]: Follow Solidity standard naming conventions

Context:

1. struct LocalVariables_adjustTrove { L47
2. struct LocalVariables_openTrove { L66
3. mapping (address => uint256) internal collAmount; // collateral => amount tracker L41 (collAmount should start with _)
4. mapping (address => uint256) internal LUSDDebt; // collateral => corresponding debt tracker L42 (LUSDDebt should start with _)
5. mapping (address => uint256) internal collAmounts; // deposited collateral tracker L167 (collAmounts should start with _)
6. uint256 internal totalLUSDDeposits; L170 (totalLUSDSeposits should start with _)
7. mapping (address => uint) public lastCollateralError_Offset; L228 (Variable name must be in mixedCase)
8. uint public lastLUSDLossError_Offset; L229 (Variable name must be in mixedCase)
9. function depositSnapshots_S(address _depositor, address _collateral) external override view returns (uint) { L410 (Function name must be in mixedCase)
10. IERC20 public OathToken; L37 (Variable name must be in mixedCase)
11. function sendOath(address _account, uint _OathAmount) external override { L124 (Variable name _OathAmount must be in mixedCase)
12. function increaseF_Collateral(address _collateral, uint _collFee) external override { L177 (Function name must be in mixedCase)
13. function increaseF_LUSD(uint _LUSDFee) external override { L187 (Function name must be in mixedCase)
14. function roundUpDiv(uint256 x, uint256 y) internal pure returns (uint256) { L269 (roundUpDiv should start with _)
15. uint16 private constant LENDER_REFERRAL_CODE_NONE = 0; L35 (LENDER_REFERRAL_CODE_NONE should start with _)

Description:

The above codes don't follow Solidity's standard naming convention.

• Internal and private functions should use mixedCase format starting with an underscore.
• Structs should be named using the CapWords style. Examples: MyCoin, Position, PositionXY.
• Events should be named using the CapWords style. Examples: Deposit, Transfer, Approval, BeforeTransfer, AfterTransfer.
• Functions should use mixedCase. Examples: getBalance, transfer, verifyOwner, addMember, changeOwner.
• Function arguments should use mixedCase. Examples: initialSupply, account, recipientAddress, senderAddress, newOwner.
• Local and State Variable should use mixedCase. Examples: totalSupply, remainingSupply, balancesOf, creatorAddress, isPreSale, tokenExchangeRate.
• Constants should be named with all capital letters with underscores separating words. Examples: MAX_BLOCKS, TOKEN_NAME, TOKEN_TICKER, CONTRACT_VERSION.
• Modifier should use mixedCase. Examples: onlyBy, onlyAfter, onlyDuringThePreSale.
• Enums, in the style of simple type declarations, should be named using the CapWords style. Examples: TokenGroup, Frame, HashStyle, CharacterLocation.

[N-5]: Wrong order of functions

Context:

1. function sendCollateral(address _collateral, address _account, uint _amount) external override { L171 (external function can not go after external view function)
2. function setWithdrawalQueue(address[] calldata _withdrawalQueue) external { L258 (extarnal function can not go after public function)
3. function maxDeposit(address) external view override returns (uint256 maxAssets) { L79 (external function can not go after public function)
4. function updateVeloSwapPath( L147 (external function can not go after internal function)

Description:

According to official solidity documentation functions should be grouped according to their visibility and ordered:

• constructor

• receive function (if exists)

• fallback function (if exists)

• external

• public

• internal

• private

Within a grouping, place the view and pure functions last.

Recommendation:

Put the functions in the correct order according to the documentation.

[N-6]: NatSpec comments should be increased in contracts

Context:

All contracts.

Description:

https://docs.soliditylang.org/en/v0.8.19/natspec-format.html

Recommendation:

Increase NatSpec comments in all contracts.

[N-7]: Line is too long

Context:

1. event TroveUpdated(address indexed _borrower, address _collateral, uint _debt, uint _coll, uint stake, BorrowerOperation operation); L105
2. function openTrove(address _collateral, uint _collAmount, uint _maxFeePercentage, uint _LUSDAmount, address _upperHint, address _lowerHint) external override { L172
3. vars.LUSDFee = _triggerBorrowingFee(contractsCache.troveManager, contractsCache.lusdToken, _LUSDAmount, _maxFeePercentage); L190
4. _withdrawLUSD(contractsCache.activePool, contractsCache.lusdToken, _collateral, msg.sender, _LUSDAmount, vars.netDebt); L233
5. _withdrawLUSD(contractsCache.activePool, contractsCache.lusdToken, _collateral, gasPoolAddress, LUSD_GAS_COMPENSATION, LUSD_GAS_COMPENSATION); L235
6. emit TroveUpdated(msg.sender, _collateral, vars.compositeDebt, _collAmount, vars.stake, BorrowerOperation.openTrove); L237
7. function moveCollateralGainToTrove(address _borrower, address _collateral, uint _collAmount, address _upperHint, address _lowerHint) external override { L248
8. function withdrawColl(address _collateral, uint _collWithdrawal, address _upperHint, address _lowerHint) external override { L254
9. function withdrawLUSD(address _collateral, uint _maxFeePercentage, uint _LUSDAmount, address _upperHint, address _lowerHint) external override { L259
10. function repayLUSD(address _collateral, uint _LUSDAmount, address _upperHint, address _lowerHint) external override { L264
11. function adjustTrove(address _collateral, uint _maxFeePercentage, uint _collTopUp, uint _collWithdrawal, uint _LUSDChange, bool _isDebtIncrease, address _upperHint, address _lowerHint) external override { L268
12. _adjustTrove(msg.sender, _collateral, _collTopUp, _collWithdrawal, _LUSDChange, _isDebtIncrease, _upperHint, _lowerHint, _maxFeePercentage); L272
13. function _adjustTrove(address _borrower, address _collateral, uint _collTopUp, uint _collWithdrawal, uint _LUSDChange, bool _isDebtIncrease, address _upperHint, address _lowerHint, uint _maxFeePercentage) internal { L282
14. vars.LUSDFee = _triggerBorrowingFee(contractsCache.troveManager, contractsCache.lusdToken, _LUSDChange, _maxFeePercentage); L312
15. (vars.newColl, vars.newDebt) = _updateTroveFromAdjustment(contractsCache.troveManager, _borrower, _collateral, vars.collChange, vars.isCollIncrease, vars.netDebtChange, _isDebtIncrease); L343
16. function _triggerBorrowingFee(ITroveManager _troveManager, ILUSDToken _lusdToken, uint _LUSDAmount, uint _maxFeePercentage) internal returns (uint) { L419
17. function _withdrawLUSD(IActivePool _activePool, ILUSDToken _lusdToken, address _collateral, address _account, uint _LUSDAmount, uint _netDebtIncrease) internal { L511
18. function _repayLUSD(IActivePool _activePool, ILUSDToken _lusdToken, address _collateral, address _account, uint _LUSD) internal { L517
19. function _requireSufficientCollateralBalanceAndAllowance(address _user, address _collateral, uint _collAmount) internal view { L528
20. require(IERC20(_collateral).balanceOf(_user) >= _collAmount, "BorrowerOperations: Insufficient user collateral balance"); L529
21. require(IERC20(_collateral).allowance(_user, address(this)) >= _collAmount, "BorrowerOperations: Insufficient collateral allowance"); L530
22. function _requireTroveisNotActive(ITroveManager _troveManager, address _borrower, address _collateral) internal view { L546
23. require(_debtRepayment <= _currentDebt.sub(LUSD_GAS_COMPENSATION), "BorrowerOps: Amount repaid must not be larger than the Trove's debt"); L637
24. function _requireSufficientLUSDBalance(ILUSDToken _lusdToken, address _borrower, uint _debtRepayment) internal view { L644
25. require(_lusdToken.balanceOf(_borrower) >= _debtRepayment, "BorrowerOps: Caller doesnt have enough LUSD to make repayment"); L645
26. (uint newColl, uint newDebt) = _getNewTroveAmounts(_coll, _debt, _collChange, _isCollIncrease, _debtChange, _isDebtIncrease); L675
27. (uint newColl, uint newDebt) = _getNewTroveAmounts(_coll, _debt, _collChange, _isCollIncrease, _debtChange, _isDebtIncrease); L697
28. mapping (address => uint256) public yieldClaimThreshold; // collateral => minimum wei amount of yield to claim and redistribute L47
29. function setYieldDistributionParams(uint256 _treasurySplit, uint256 _SPSplit, uint256 _stakingSplit) external onlyOwner { L144
30. vars.percentOfFinalBal = vars.finalBalance == 0 ? uint256(-1) : vars.currentAllocated.mul(10_000).div(vars.finalBalance); L258
31. if (vars.percentOfFinalBal > vars.yieldingPercentage && vars.percentOfFinalBal.sub(vars.yieldingPercentage) > yieldingPercentageDrift) { L264
32. } else if(vars.percentOfFinalBal < vars.yieldingPercentage && vars.yieldingPercentage.sub(vars.percentOfFinalBal) > yieldingPercentageDrift) { L269
33. IERC4626(yieldGenerator[_collateral]).withdraw(uint256(-vars.netAssetMovement), address(this), address(this)); L282
34. // profit is ultimately (coll at hand) + (coll allocated to yield generator) - (recorded total coll Amount in pool) L287
35. vars.profit = IERC20(_collateral).balanceOf(address(this)).add(vars.yieldingAmount).sub(collAmount[_collateral]); L288
36. uint LUSDLossPerUnitStaked) = _computeRewardsPerUnitStaked(_collateral, _collToAdd, _debtToOffset, totalLUSD); L474
37. function _updateRewardSumAndProduct(address _collateral, uint _collGainPerUnitStaked, uint _LUSDLossPerUnitStaked) internal { L547
38. function getDepositorCollateralGain(address _depositor) public view override returns (address[] memory assets, uint[] memory amounts) { L626
39. function _getCollateralGainFromSnapshots(uint initialDeposit, Snapshots storage snapshots) internal view returns (address[] memory assets, uint[] memory amounts) { L637
40. function _getSingularCollateralGain(uint _initialDeposit, address _collateral, Snapshots storage _snapshots) internal view returns (uint) { L657
41. vars.secondPortion = epochToScaleToSum[vars.epochSnapshot][vars.scaleSnapshot.add(1)][_collateral].div(SCALE_FACTOR); L671
42. vars.gain = _initialDeposit.mul(vars.firstPortion.add(vars.secondPortion)).div(vars.P_Snapshot).div(DECIMAL_PRECISION); L673
43. function _getPendingCollateralGain(address _user) internal view returns (address[] memory assets, uint[] memory amounts) { L203
44. shares = previewWithdraw(assets); // previewWithdraw() rounds up so exactly "assets" are withdrawn and not 1 wei less L207

Description:

Maximum suggested line length is 120 characters.