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: 29/154
Findings: 1
Award: $455.47
🌟 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
455.4712 USDC - $455.47
As of Solidity 0.8 overflows are handled automatically; however, not for casting. Also, many files in the codebase use Solidity 0.6.11 which does not have overflow protection. For example uint32(4294967300)
will result in 4
without reversion. Consider using OpenZepplin's SafeCast library.
/Ethos-Core/contracts/TroveManager.sol
1329: index = uint128(TroveOwners[_collateral].length.sub(1));
/Ethos-Core/contracts/ActivePool.sol
277: vars.netAssetMovement = int256(vars.toDeposit) - int256(vars.toWithdraw) - int256(vars.profit);
/Ethos-Vault/contracts/ReaperVaultV2.sol
228: return -int256(strategies[stratAddr].allocated); 235: return -int256(stratCurrentAllocation - stratMaxAllocation); 248: return int256(available);
/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol
121: roi = -int256(debt - amountFreed); 123: roi = int256(amountFreed - debt);
/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol
134: roi = int256(profit); 136: roi = -int256(allocated - totalAssets); 141: roi -= int256(loss);
Ethos Reserve uses a fork of the Liquity Protocol which uses a solidity version of 0.6.11
. It may be better for security to use battle-tested software; however, underflow and overflow oversights can potentially be a large problem, which is fixed post Solidity 0.8.0
(automatic reversion). Consider consulting a Liquity dev to help upgrade past Solidity version 0.8.0
or upgrade yourself depending on your confidence level with the system.
There is a comment in TroveManager.sol that reads // break if the loop reaches a Trove with ICR >= MCR
. The comment is not exactly true. The else
block that is commented is triggered if vars.backToNormalMode
AND !vars.backToNormalMode
OR vars.ICR >= vars.collMCR
. Over simplifying comments may lead to users making uninformed decisions when reading the code functionality.
In BorrowerOperations.sol there is a require statement that reads Max fee percentage must less than or equal to 100%
which is checked if the system is in recovery mode. If the system is not in recovery another require statement reads Max fee percentage must be between 0.5% and 100%
. Both statements check if the max fee percentage falls within inclusive ranges, yet the second require message uses the partially ambiguous (usually said for non-inclusivity) word between
. Consider changing this error message from Max fee percentage must be between 0.5% and 100%
to Max fee percentage must be between 0.5% and 100% inclusively
.
The code coverage of the files in scope is not 100%. Consider a test coverage of 100%.
assert
should only be used in tests. Consider changing all occurrences of assert
to require
. Prior to Solidity 0.8.0
require
will refund all remaining gas whereas assert
will not. Even after Solidity 0.8 assert
will result in a panic which should not occur in production code. As stated in the Solidity Documentation: "[p]roperly functioning code should never create a Panic".
/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)); 331: assert(_collWithdrawal <= vars.coll);
/Ethos-Core/contracts/TroveManager.sol
417: assert(_LUSDInStabPool != 0); 1224: assert(totalStakesSnapshot[_collateral] > 0); 1279: assert(closedStatus != Status.nonExistent && closedStatus != Status.active); 1342: assert(troveStatus != Status.nonExistent && troveStatus != Status.active); 1348: assert(index <= idxLast); 1414: assert(newBaseRate > 0); 1489: assert(decayedBaseRate <= DECIMAL_PRECISION);
/Ethos-Core/contracts/StabilityPool.sol
526: assert(_debtToOffset <= _totalLUSDDeposits); 551: assert(_LUSDLossPerUnitStaked <= DECIMAL_PRECISION); 591: assert(newP > 0);
/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));
Ethos-Core and Ethos-Vault have drastically different compiler versions. One uses 0.6.11
(which is forked from Liquity) and the other uses ^0.8.0
. One major difference between the compiler versions is automatic overflow and underflow checks. In future development or third-party development of the system the two parts are going to need different attention to detail that may introduce bugs. It is best to keep the system as a whole on a similar compiler version.
EIP20 states in regard to the decimals
method: "OPTIONAL - This method can be used to improve usability, but interfaces and other contracts MUST NOT expect these values to be present". The protocol assumes the decimals
method is present which may affect the system if whole tokens are used.
/Ethos-Core/contracts/CollateralConfig.sol
uint256 decimals = IERC20(collateral).decimals();
This issue is from a previous audit of the Liquity Protocol that has not been fixed in Ethos
The liquidate
function of TroveManager.sol checks if a trove is active while batchLiquidateTroves
does not. Both are publically callable functions.
Please see finding ID: CI-LQY-02
in the 2021 Coinspect Security Audit for more information.
This issue is from a previous audit of the Liquity Protocol that has not been fixed in Ethos
When transferring LUSD there is a check done (_requireValidRecipient
) to make sure tokens cannot be sent to an invalid address. The mint
is missing this check.
Please see finding ID: TOB-LQT-003
in the 2021 Liquity Protocol Trail of Bits Audit for more information.
In the protocol overview video at time 2:40, Justin states, "liquity is designed to only support ETH, and we support anything".
Rebasing tokens are not checked for (ex). Balances must be adjusted for during transfers.
Please see this issue utilizing similar code from a previous code4rena audit.
Consider using OpenZeppelin's ECDSA contract instead of raw ecrecover
to avoid signature malleability.
/Ethos-Core/contracts/LUSDToken.sol
287: address recoveredAddress = ecrecover(digest, v, r, s);
The 2022 Topshelf Protocol PeckShield Audit which uses the Liquity Protocol code also suggests checking the owner is not the zero address. See finding ID: PVE-001
.
Consider using the terms allowlist / denylist instead of whitelist / blacklist.
/Ethos-Core/contracts/CollateralConfig.sol
37: event CollateralWhitelisted(address _collateral, uint256 _decimals, uint256 _MCR, uint256 _CCR); 72: emit CollateralWhitelisted(collateral, decimals, _MCRs[i], _CCRs[i]);
As described in the Solidity documentation, "uint
and int
are aliases for uint256
and int256
, respectively". There are moments in the codebase where uint
/ int
is used instead of the explicit uint256
/ int256
. It is best to be explicit with variable names to lessen confusion. Consider replacing instances of uint
/ int
with uint256
/ int256
.
/Ethos-Core/contracts/BorrowerOperations.sol
/Ethos-Core/contracts/TroveManager.sol
/Ethos-Core/contracts/ActivePool.sol
/Ethos-Core/contracts/StabilityPool.sol
/Ethos-Core/contracts/LQTY/CommunityIssuance.sol
/Ethos-Core/contracts/LQTY/LQTYStaking.sol
/Ethos-Core/contracts/LUSDToken.sol
There is an inconsistency with the explicitness of variable scopes. Variable scopes should be explicit and not left out if public
(the default). Most variables are explicitly scoped (ex1, ex2, ...); however, not all. The following are not explicitly deemed public
:
/Ethos-Core/contracts/BorrowerOperations.sol
/Ethos-Core/contracts/TroveManager.sol
If the Solidity version of LUSDToken.sol is upgraded to 0.8.0
, the complexity brought by assembly can be avoided by using built in functions. assembly { chainID := chainid() }
can be replaced by uint256 chainID = block.chainid
.
/Ethos-Core/contracts/LUSDToken.sol
300: chainID := chainid()
now
is deprecated, consider using block.timestamp
(as used in other parts of the codebase).
/Ethos-Core/contracts/LUSDToken.sol
282: require(deadline >= now, 'LUSD: expired deadline');
If the contracts are upgraded to Solidity version >= 0.8.4
consider using bytes.concat()
instead of abi.encodePacked()
.
/Ethos-Core/contracts/LUSDToken.sol
283: bytes32 digest = keccak256(abi.encodePacked('\x19\x01',
No contracts in scope have a fully annotated NatSpec contract header (@title
, @author
, etc. see here for reference). Some contracts have a partial NatSpec header like this, some have none, and others have non-NatSpec header comments. Consider adding properly annotated NatSpec headers.
There is a NAME
state variable in BorrowerOperations.sol, ActivePool.sol, LQTYStaking.sol, and CommunityIssuance.sol that doesn't seem needed. The same NAME
state variable is commented out of TroveManager.sol.
If developers want to know the title of the contract they can reference the source contract name. If there isn't an explicit reason to keep the NAME
state variables otherwise, consider removing them.
Even if risk seems infinitesimally small it should never be stated as impossible.
For example, in the context of a properly functioning EVM the following line will never overflow (no risk):
uint256 foo = 32;
The next example can overflow in modern Solidity if someArray.length
> type(uint32).max
even if there are social or intended code concepts keeping the length within a proper length:
uint32 foo = uint32(someArray.length);
L1322-L1223 of TroveManager.sol
reads: "No risk of overflow, since troves have minimum LUSD debt of liquidation reserve plus MIN_NET_DEBT. 3e30 LUSD dwarfs the value of all wealth in the world ( which is < 1e15 USD)"
. This assumes the code keeping the minumum LUSD debt works as described and also assumes the US dollar never experiences extreme hyperinflation. This may be an infinitesimally small risk, but not zero.
Consider changing the line to "Little to no risk of overflow, since troves have minimum LUSD debt of liquidation reserve plus MIN_NET_DEBT. 3e30 LUSD dwarfs the value of all wealth in the world ( which is < 1e15 USD)"
.
Consider using underscore notation to help with contract readability (Ex. 23453
-> 23_453
).
/Ethos-Core/contracts/TroveManager.sol
53: uint constant public MINUTE_DECAY_FACTOR = 999037758833783000;
Power of ten literals > 1e2
are easier to read when expressed in scientific notation. Consider expressing large powers of ten in scientific notation.
/Ethos-Core/contracts/TroveManager.sol
54: uint constant public override REDEMPTION_FEE_FLOOR = DECIMAL_PRECISION / 1000 * 5;
/Ethos-Vault/contracts/ReaperVaultV2.sol
41: uint256 public constant PERCENT_DIVISOR = 10000;
/Ethos-Core/contracts/ActivePool.sol
145: require(_treasurySplit + _SPSplit + _stakingSplit == 10_000, "Splits must add up to 10000 BPS");
For better style and less computation consider replacing any power of 10 exponentiation (10**3
) with its equivalent scientific notation (1e3
).
/Ethos-Vault/contracts/ReaperVaultV2.sol
40: uint256 public constant DEGRADATION_COEFFICIENT = 10**18; 125: lockedProfitDegradation = (DEGRADATION_COEFFICIENT * 46) / 10**6;
Some functions use named returns and others do not. It is best for code clearity to keep a consistent style.
returns(uint256 foo)
): CommunityIssuance.sol.returns(uint256)
): CollateralConfig.sol, and ActivePool.sol.There are some spelling mistakes throughout the codebase. Consider fixing all spelling mistakes. The following were not included in the automated findings:
/Ethos-Core/contracts/CollateralConfig.sol
lengths
is misspelled as lenghts
./Ethos-Core/contracts/TroveManager.sol
elsewhere
is misspelled as elswhere
.borrowers'
is misspelled as borrowers's
.There are a few commented code lines in TroveManager.sol (L14, L18, L19, L1413). Commented code should be removed prior to production as they no longer serve a purpose and only add bulk or distraction.
There is an inconsistency in the capitalization of comments. For example, this line is not capitilized while - like most - this line is. Given almost all comments are capitilized, consider capitilizing all comments.
It appears that the single line comments in the codebase generally follow the following format:
.
is left out (ex1, ex2, ex3, ...).
is added (ex1, ex2, ex3, ...)There are few, but some comments void this general style:
Consider sticking to a single comment style for a less distracting developer experience.
More documentation is almost always better than none at all; however, there is no need to document basic programming knowledge. Anyone who would want to read the Ethos Reserve code should have basic programming knowledge (knowledge of if
/ else
control statements). There are times in the codebase where basic programming knowledge is commented. In some case like L665, the comment may aid in understanding certain abstraction (a more complex case comparison, although L665 does not tell its full story [see L3]), these are not included. The following comments should be considered for removal in TroveManager.sol: 539, 708, 742.
Time keywords like hours
or days
are used in the codebase (ex1, ex2) yet not used here. Consider changing SECONDS_IN_ONE_MINUTE = 60;
to SECONDS_IN_ONE_MINUTE = 1 minutes;
for consistency.
Some single line code does not include braces (ex1, ex2, ...) and others do (ex1, ex2, ex3, ex4, ...). Some single line brace code also have differing spacing (Type1, Type2). In other parts of the codebase single line braces are expanded (ex).
Consider sticking to a single line brace style.
Some comments use a single quote for quotations and others use a double quote. The Solidity Style Guide states that "[s]trings should be quoted with double-quotes instead of single-quotes". Although comments are not seen as active string (most likely does not void the Style Guide), it is best to stick to double quotes for consistency.
All contracts in scope have a space between the license statement and pragma (ex). Although it does not void the Solidity Style Guide, all example contracts in the Style Guide do not have a space between the license statement and pragma (ex). This format can also be seen throughout the Solidy Documentation (ex). Consider removing the needless space.
There are times in TroveManager.sol where extra spaces or new lines are present when they are not needed:
else if
brace and else
), 742 (extra space after else
brace and after //
), 852 (2 extra new lines), 1323 (space after the opening parenthesis).CommunityIssuance.sol has only basic structural headers (describes basic elements of the contract) which can be removed if _requireCallerIsStabilityPool
were converted to a modifier and the contract followed the Solidity Style Guide layout.
Some contracts like ActivePool.sol have meaningful headers that help developers understand the structure of the code. Some contracts like CollateralConfig.sol have no headers at all.
There is no need for CommunityIssuance.sol to have headers if properly formatted. Consider removing the headers or add headers to all contracts.
Some function declarations like this do not drop the opening parenthesis on a new line, but some do.
This does not void the Solidity Style Guide which states: "[f]or long function declarations, it is recommended to drop each argument onto its own line at the same indentation level as the function body. The closing parenthesis and opening bracket should be placed on their own line as well at the same indentation level as the function declaration". There is no comment on the placement of the opening parenthesis; however, consider sticking to a single style.
There are times in the codebase where a variable of the name vars
is used to hold local variables of a function. Although the type of these variables are explicitly named like LocalVariables_openTrove
it is best to not use general names like vars
as it could confuse the reader. Consider being more specific in naming variables.
In order to understand variables in the codebase the Liquity Documentation was needed. For example, in the openTrove
function, the use of the _maxFeePercentage
variable was unclear. Consider adding a NatSpec header to all functions which include @param
tags detailing the use of each variable.
Similarily to functions all state variables should be documented with NatSpec tags. To understand what baseRate
was, the Liquity Documentation was needed. It is not good to force readers to reference forked code documentation to understand variable use. Consider documenting state varaibles with NatSpec.
Non-symmetric spacing voids the Solidity Style Guide; however, it is not clear if exponentiation should have a trailing and leading space or no spacing (a**b
vs a ** b
). Nevertheless, spacing in exponentiation should be consistant.
In this example the a**b
style is used, but in this example the a ** b
style is used.
Consider sticking to a single style.
Some contracts in the codebase are ownable; however, they use different styles of ownership. For example BorrowerOperations.sol uses an Ownable contract, whereas TroveManager.sol uses a single variable owner
. It is understood that the single variable use in TroveManager.sol is "to circumvent contract size limit". Nonetheless, the same style should be used to avoid possible security confusion and future implementation overlooks.
Also consider using battle-tested ownable contracts like Openzepplin's.
Some functions in the codebase use named returns but still return a variable. This can confuse a reader.
/Ethos-Core/contracts/TroveManager.sol
/Ethos-Core/contracts/StabilityPool.sol
/Ethos-Vault/contracts/ReaperVaultERC4626.sol
/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol
uint256(-1)
is used to get the max value of a uint256
. Since Solidity 0.6.8
type(uint256).max
may be used instead of forcing an overflow. Solidity 0.8.0
introduced automatic overflow detection, forcing non-unchecked instances of uint256(-1)
to revert. Future developers may be confused by the use of uint256(-1)
, consider using type(uint256).max
.
/Ethos-Core/contracts/ActivePool.sol
258: vars.percentOfFinalBal = vars.finalBalance == 0 ? uint256(-1) : vars.currentAllocated.mul(10_000).div(vars.finalBalance);
Lines with greater length than 120 characters are used. The Solidity Style Guide suggests that all lines should be 120 characters or less in width.
The following lines are longer than 120 characters, it is suggested to shorten these lines:
/Ethos-Core/contracts/BorrowerOperations.sol
/Ethos-Core/contracts/TroveManager.sol
/Ethos-Core/contracts/ActivePool.sol
/Ethos-Core/contracts/StabilityPool.sol
/Ethos-Core/contracts/LQTY/LQTYStaking.sol
/Ethos-Core/contracts/LUSDToken.sol
/Ethos-Vault/contracts/ReaperVaultERC4626.sol
The Solidity Style Guide suggests the following function order: constructor, receive function (if exists), fallback function (if exists), external, public, internal, private.
The following contracts are not compliant (examples are only to prove the functions are out of order NOT a full description):
The Solidity Style Guide recommends to "[a]void extraneous whitespace [i]mmediately inside parenthesis, brackets or braces, with the exception of single line function declarations".
/Ethos-Core/contracts/StabilityPool.sol
368: if (_amount !=0) {_requireNoUnderCollateralizedTroves();} 849: require( msg.sender == address(activePool), "StabilityPool: Caller is not ActivePool");
/Ethos-Core/contracts/TroveManager.sol
1127: if ( rewardPerUnitStaked == 0 || Troves[_borrower][_collateral].status != Status.active) { return 0; } 1142: if ( rewardPerUnitStaked == 0 || Troves[_borrower][_collateral].status != Status.active) { return 0; }
The Solidity Style Guide states that structs should "close on their own line at the same indentation level as the beginning of the declaration". The struct seen here is defined using a single line which voids the second Style Guide requirement.
The Solidity Style Guide states that "[f]or if blocks which have an else or else if clause, the else should be placed on the same line as the if’s closing brace". This is followed perfectly here and in all contracts except TroveManager.sol: 651, 853.
The Solidity Style Guide states that "[f]or short function declarations, it is recommended for the opening brace of the function body to be kept on the same line as the function declaration". It is not clear what length is exactly meant by "short" or "long". The maximum line length of 120 characters may be an indication, and in that case any expanded function declaration under 120 characters should be on a single line.
The following functions in their respective contracts are not compliant by this standard:
/Ethos-Core/contracts/TroveManager.sol
getCurrentICR
./Ethos-Core/contracts/LQTY/CommunityIssuance.sol
setAddresses
./Ethos-Vault/contracts/ReaperVaultV2.sol
addStrategy
./Ethos-Vault/contracts/ReaperVaultERC4626.sol
withdraw
, redeem
./Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol
updateVeloSwapPath
.The Solidity Style Guide states that "[f]or long function declarations, it is recommended to drop each argument onto its own line at the same indentation level as the function body. The closing parenthesis and opening bracket should be placed on their own line as well at the same indentation level as the function declaration."
This function violates the description.
The Solidity Style Guide suggests the following modifer order: Visibility, Mutability, Virtual, Override, Custom modifiers.
/Ethos-Core/contracts/CollateralConfig.sol
getAllowedCollaterals
('external', 'override', 'view'): view (Mutability) positioned after override (Override).isCollateralAllowed
('external', 'override', 'view'): view (Mutability) positioned after override (Override).getCollateralDecimals
('external', 'override', 'view', 'Custom'): view (Mutability) positioned after override (Override).getCollateralMCR
('external', 'override', 'view', 'Custom'): view (Mutability) positioned after override (Override).getCollateralCCR
('external', 'override', 'view', 'Custom'): view (Mutability) positioned after override (Override)./Ethos-Core/contracts/BorrowerOperations.sol
_getNewNominalICRFromTroveChange
('pure', 'internal'): internal (Visibility) positioned after pure (Mutability)._getNewICRFromTroveChange
('pure', 'internal'): internal (Visibility) positioned after pure (Mutability)./Ethos-Core/contracts/StabilityPool.sol
depositSnapshots_S
('external', 'override', 'view'): view (Mutability) positioned after override (Override)./Ethos-Core/contracts/LQTY/CommunityIssuance.sol
setAddresses
('external', 'onlyOwner', 'override'): override (Override) positioned after onlyOwner (Custom Modifiers)./Ethos-Core/contracts/LQTY/LQTYStaking.sol
setAddresses
('external', 'onlyOwner', 'override'): override (Override) positioned after onlyOwner (Custom Modifiers).Mapping whitespace not in accordance with the Solidity Style Guide. There are many voiding whitespaces (see link).
/Ethos-Core/contracts/CollateralConfig.sol
35: mapping (address => Config) public collateralConfig;
/Ethos-Core/contracts/TroveManager.sol
86: mapping (address => mapping (address => Trove)) public Troves; 88: mapping (address => uint) public totalStakes; 91: mapping (address => uint) public totalStakesSnapshot; 94: mapping (address => uint) public totalCollateralSnapshot; 104: mapping (address => uint) public L_Collateral; 105: mapping (address => uint) public L_LUSDDebt; 109: mapping (address => mapping (address => RewardSnapshot)) public rewardSnapshots; 116: mapping (address => address[]) public TroveOwners; 119: mapping (address => uint) public lastCollateralError_Redistribution; 120: mapping (address => uint) public lastLUSDDebtError_Redistribution;
/Ethos-Core/contracts/ActivePool.sol
41: mapping (address => uint256) internal collAmount; 42: mapping (address => uint256) internal LUSDDebt; 44: mapping (address => uint256) public yieldingPercentage; 45: mapping (address => uint256) public yieldingAmount; 46: mapping (address => address) public yieldGenerator; 47: mapping (address => uint256) public yieldClaimThreshold;
/Ethos-Core/contracts/StabilityPool.sol
167: mapping (address => uint256) internal collAmounts; 179: mapping (address => uint) S; 186: mapping (address => Deposit) public deposits; 187: mapping (address => Snapshots) public depositSnapshots; 214: mapping (uint128 => mapping(uint128 => mapping (address => uint))) public epochToScaleToSum; 223: mapping (uint128 => mapping(uint128 => uint)) public epochToScaleToG; 228: mapping (address => uint) public lastCollateralError_Offset;
/Ethos-Core/contracts/LQTY/LQTYStaking.sol
25: mapping( address => uint) public stakes; 28: mapping (address => uint) public F_Collateral; 32: mapping (address => Snapshot) public snapshots; 35: mapping (address => uint) F_Collateral_Snapshot;
/Ethos-Core/contracts/LUSDToken.sol
54: mapping (address => uint256) private _nonces; 57: mapping (address => uint256) private _balances; 58: mapping (address => mapping (address => uint256)) private _allowances; 62: mapping (address => bool) public troveManagers; 63: mapping (address => bool) public stabilityPools; 64: mapping (address => bool) public borrowerOperations;
The Solidity Style Guide states that "[s]trings should be quoted with double-quotes instead of single-quotes". Consider using double-quotes to be compliant.
/Ethos-Core/contracts/ActivePool.sol
5: import './Interfaces/IActivePool.sol'; 7: import './Interfaces/IDefaultPool.sol';
/Ethos-Core/contracts/StabilityPool.sol
5: import './Interfaces/IBorrowerOperations.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'; 870: require(_initialDeposit > 0, 'StabilityPool: User must have a non-zero deposit'); 874: require(_amount > 0, 'StabilityPool: Amount must be non-zero');
/Ethos-Core/contracts/LQTY/LQTYStaking.sol
266: require(currentStake > 0, 'LQTYStaking: User must have a non-zero stake'); 270: require(_amount > 0, 'LQTYStaking: Amount must be non-zero');
/Ethos-Core/contracts/LUSDToken.sol
280: revert('LUSD: Invalid s value'); 282: require(deadline >= now, 'LUSD: expired deadline'); 283: bytes32 digest = keccak256(abi.encodePacked('\x19\x01', 283: bytes32 digest = keccak256(abi.encodePacked('\x19\x01', 288: require(recoveredAddress == owner, 'LUSD: invalid signature');
The Solidity Style Guide suggests the following contract layout order: Type declarations, State variables, Events, Modifiers, Functions.
The following contracts are not compliant (examples are only to prove the layout are out of order NOT a full description):
The NatSpec notation requires the use of ///
or /**
to differentiate from regular comments. There is an instance in the codebase where a //
is used instead of ///
and where a /*
is used instead of /**
.
/Ethos-Core/contracts/LQTY/CommunityIssuance.sol
83: // @dev issues a set amount of Oath to the stability pool
97: /* 98: @dev funds the contract and updates the distribution 99: @param amount: amount of $OATH to send to the contract 100: */
#0 - c4-judge
2023-03-10T09:54:44Z
trust1995 marked the issue as grade-a
#1 - c4-sponsor
2023-03-28T21:30:36Z
0xBebis marked the issue as sponsor confirmed