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: 38/154
Findings: 2
Award: $370.05
🌟 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
Number | Issue details | Instances |
---|---|---|
L-1 | Use ofblock.timestamp | 20 |
L-2 | ecrecover() not checked for signer address of zero. | 2 |
L-3 | Avoid hardcoded values. | 2 |
L-4 | Lock pragmas to specific compiler version. | 4 |
Total: 4 issues.
Number | Issue details | Instances |
---|---|---|
NC-1 | Consider adding checks for signature malleability. | 1 |
NC-2 | Missing timelock for critical changes. | 7 |
NC-3 | Use scientific notation (e.g.1e18 ) rather than exponentiation (e.g.10**18 ). | 2 |
NC-4 | Lines are too long. | 11 |
NC-5 | For modern and more readable code, update import usages. | 102 |
NC-6 | Avoid whitespaces while declaring mapping variables. | 35 |
NC-7 | Use ofbytes.concat() instead of abi.encodePacked() . | 1 |
Total: 7 issues.
block.timestamp
Block timestamps have historically been used for a variety of applications, such as entropy for random numbers (see the Entropy Illusion for further details), locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.
Use oracle instead of block.timestamp
File: 2023-02-ethos/Ethos-Core/contracts/LQTY/CommunityIssuance.sol
87: uint256 endTimestamp = block.timestamp > lastDistributionTime ? lastDistributionTime : block.timestamp; 93: lastIssuanceTimestamp = block.timestamp; 113: lastDistributionTime = block.timestamp.add(distributionPeriod); 114: lastIssuanceTimestamp = block.timestamp;
File: 2023-02-ethos/Ethos-Core/contracts/LUSDToken.sol
128: deploymentStartTime = block.timestamp;
File: 2023-02-ethos/Ethos-Core/contracts/TroveManager.sol
1501: uint timePassed = block.timestamp.sub(lastFeeOperationTime); 1504: lastFeeOperationTime = block.timestamp; 1505: emit LastFeeOpTimeUpdated(block.timestamp); 1517: return (block.timestamp.sub(lastFeeOperationTime)).div(SECONDS_IN_ONE_MINUTE);
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperVaultV2.sol
121: constructionTime = block.timestamp; 122: lastReport = block.timestamp; 159: activation: block.timestamp, 165: lastReport: block.timestamp 419: uint256 lockedFundsRatio = (block.timestamp - lastReport) * lockedProfitDegradation; 540: strategy.lastReport = block.timestamp; 541: lastReport = block.timestamp;
File: 2023-02-ethos/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol
137: lastHarvestTimestamp = block.timestamp; 169: upgradeProposalTime = block.timestamp; 181: upgradeProposalTime = block.timestamp + FUTURE_NEXT_PROPOSAL_TIME; 192: upgradeProposalTime + UPGRADE_TIMELOCK < block.timestamp,
ecrecover()
not checked for signer address of zero.The ecrecover()
function returns an address of zero when the signature does not match. This can cause problems if address zero is ever the owner of assets, and someone uses the permit function on address zero.
You should check whether the return value is address(0) or not in terms of the security.
File: 2023-02-ethos/Ethos-Core/contracts/LUSDToken.sol
275: // EIP-2 still allows signature malleability for ecrecover(). Remove this possibility and make the signature 287: address recoveredAddress = ecrecover(digest, v, r, s);
It is not good practice to hardcode values, but if you are dealing with addresses much less, these can change between implementations, networks or projects, so it is convenient to remove these values from the source code.
Use the selector instead of the raw value.
File: 2023-02-ethos/Ethos-Core/contracts/LUSDToken.sol
42: bytes32 private constant _PERMIT_TYPEHASH = 0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9; 44: bytes32 private constant _TYPE_HASH = 0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f;
Pragma statements can be allowed to float when a contract is intended for consumption by other developers, as in the case with contracts in a library or EthPM package. Otherwise, the developer would need to manually update the pragma in order to compile locally.
Ethereum Smart Contract Best Practices - Lock pragmas to specific compiler version. solidity-specific/locking-pragmas
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol
3: pragma solidity ^0.8.0;
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperVaultERC4626.sol
3: pragma solidity ^0.8.0;
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperVaultV2.sol
3: pragma solidity ^0.8.0;
File: 2023-02-ethos/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol
3: pragma solidity ^0.8.0;
Consider adding checks for signature malleability
Use OpenZeppelin's ECDSA contract rather than calling ecrecover() directly
File: 2023-02-ethos/Ethos-Core/contracts/LUSDToken.sol
275: // EIP-2 still allows signature malleability for ecrecover(). Remove this possibility and make the signature 287: address recoveredAddress = ecrecover(digest, v, r, s);
A timelock should be added to functions that perform critical changes.
TODO: Add Timelock for the following functions.
File: 2023-02-ethos/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: 2023-02-ethos/Ethos-Core/contracts/LQTY/CommunityIssuance.sol
101: function fund(uint amount) external onlyOwner { 120: function updateDistributionPeriod(uint256 _newDistributionPeriod) external onlyOwner {
1e18
) rather than exponentiation (e.g.10**18
).Use scientific notation (e.g.1e18
) rather than exponentiation (e.g.10**18
) that could lead to errors.
Use scientific notation instead of exponentiation.
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperVaultV2.sol
40: uint256 public constant DEGRADATION_COEFFICIENT = 10**18; // The unit for calculating profit degradation. 125: lockedProfitDegradation = (DEGRADATION_COEFFICIENT * 46) / 10**6; // 6 hours in blocks
Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length
Reduce number of characters per line to improve readability.
File: 2023-02-ethos/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 { 282: function _adjustTrove(address _borrower, address _collateral, uint _collTopUp, uint _collWithdrawal, uint _LUSDChange, bool _isDebtIncrease, address _upperHint, address _lowerHint, uint _maxFeePercentage) internal { 343: (vars.newColl, vars.newDebt) = _updateTroveFromAdjustment(contractsCache.troveManager, _borrower, _collateral, vars.collChange, vars.isCollIncrease, vars.netDebtChange, _isDebtIncrease); 511: function _withdrawLUSD(IActivePool _activePool, ILUSDToken _lusdToken, address _collateral, address _account, uint _LUSDAmount, uint _netDebtIncrease) internal {
File: 2023-02-ethos/Ethos-Core/contracts/StabilityPool.sol
637: function _getCollateralGainFromSnapshots(uint initialDeposit, Snapshots storage snapshots) internal view returns (address[] memory assets, uint[] memory amounts) {
File: 2023-02-ethos/Ethos-Core/contracts/TroveManager.sol
351: emit TroveLiquidated(_borrower, _collateral, singleLiquidation.entireTroveDebt, singleLiquidation.entireTroveColl, TroveManagerOperation.liquidateInNormalMode); 393: emit TroveLiquidated(_borrower, _collateral, singleLiquidation.entireTroveDebt, singleLiquidation.entireTroveColl, TroveManagerOperation.liquidateInRecoveryMode); 407: emit TroveLiquidated(_borrower, _collateral, singleLiquidation.entireTroveDebt, singleLiquidation.entireTroveColl, TroveManagerOperation.liquidateInRecoveryMode); 428: emit TroveLiquidated(_borrower, _collateral, singleLiquidation.entireTroveDebt, singleLiquidation.collToSendToSP, TroveManagerOperation.liquidateInRecoveryMode); 934: * The redeemer swaps (debt - liquidation reserve) LUSD for (debt - liquidation reserve) worth of collateral, so the LUSD liquidation reserve left corresponds to the remaining debt. 1044: // Return the nominal collateral ratio (ICR) of a given Trove, without the price. Takes a trove's pending coll and debt rewards from redistributions into account.
Solidity code is cleaner in the following way: On the principle that clearer code is better code, you should import the things you want to use. Specific imports with curly braces allow us to apply this rule better. Check out this article
Import like this: import {contract1 , contract2} from "filename.sol";
All imports.
Style Guide helps to maintain code layout consistent and make code more readable.
Avoid whitespaces while declaring mapping variables.
File: 2023-02-ethos/Ethos-Core/contracts/ActivePool.sol
41: mapping (address => uint256) internal collAmount; // collateral => amount tracker 42: mapping (address => uint256) internal LUSDDebt; // collateral => corresponding debt tracker 44: mapping (address => uint256) public yieldingPercentage; // collateral => % to use for yield farming (in BPS, <= 10k) 45: mapping (address => uint256) public yieldingAmount; // collateral => actual wei amount being used for yield farming 46: mapping (address => address) public yieldGenerator; // collateral => corresponding ERC4626 vault 47: mapping (address => uint256) public yieldClaimThreshold; // collateral => minimum wei amount of yield to claim and redistribute
File: 2023-02-ethos/Ethos-Core/contracts/CollateralConfig.sol
35: mapping (address => Config) public collateralConfig; // for O(1) checking of collateral's validity and properties
File: 2023-02-ethos/Ethos-Core/contracts/LQTY/LQTYStaking.sol
28: mapping (address => uint) public F_Collateral; // Running sum of collateral fees per-LQTY-staked 32: mapping (address => Snapshot) public snapshots; 35: mapping (address => uint) F_Collateral_Snapshot;
File: 2023-02-ethos/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;
File: 2023-02-ethos/Ethos-Core/contracts/StabilityPool.sol
99: * In the current epoch, the latest value of S is stored upon each scale change, and the mapping (scale -> S) is stored for each epoch. 167: mapping (address => uint256) internal collAmounts; // deposited collateral tracker 179: mapping (address => uint) S; 186: mapping (address => Deposit) public deposits; // depositor address -> Deposit struct 187: mapping (address => Snapshots) public depositSnapshots; // depositor address -> snapshots struct 208: * The 'S' sums are stored in a nested mapping (epoch => scale => collateral => sum): 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;
File: 2023-02-ethos/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;
bytes.concat()
instead of abi.encodePacked()
.Rather than using abi.encodePacked
for appending bytes, since version 0.8.4
, bytes.concat()
is enabled.
Since version 0.8.4
for appending bytes, bytes.concat()
can be used instead of abi.encodePacked()
.
File: 2023-02-ethos/Ethos-Core/contracts/LUSDToken.sol
283: bytes32 digest = keccak256(abi.encodePacked('\x19\x01',
#0 - c4-judge
2023-03-10T16:52:05Z
trust1995 marked the issue as grade-b
#1 - c4-sponsor
2023-03-29T01:00:46Z
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
308.7866 USDC - $308.79
Number | Issue details | Instances |
---|---|---|
G-1 | ++i/i++ should be unchecked{++i} /unchecked{i++} when it is not possible for them to overflow, for example when used in for and while loops | 15 |
G-2 | Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate. | 5 |
G-3 | Using storage instead of memory for structs/arrays saves gas. | 12 |
G-4 | Usage ofuint s/int s smaller than 32 bytes (256 bits) incurs overhead. | 31 |
G-5 | abi.encode() is less efficient than abi.encodePacked() | 2 |
G-6 | Usebytes32 instead of string. | 9 |
G-7 | Internal functions only called once can be inlined to save gas. | 25 |
G-8 | >= costs less gas than > . | 39 |
G-9 | <x> += <y> costs more gas than <x> = <x> + <y> for state variables | 22 |
G-10 | Import only what you need. | 102 |
G-11 | Using fixed bytes is cheaper than usingstring . | 7 |
G-12 | Use assembly to check for address(0) | 4 |
Total: 12 issues.
The instances of the issue with ID G-12
differ from the ones on the c4udit output.
++i/i++
should be unchecked{++i}
/unchecked{i++}
when it is not possible for them to overflow, for example when used in for
and while
loopsIn 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. Example for loop:
for (uint i = 0; i < length; i++) { // do something that doesn't change the value of i }
In this example, the for loop post condition, i.e., i++
involves checked arithmetic, which is not required. This is because the value of i is always strictly less than length <= 2**256 - 1
. Therefore, the theoretical maximum value of i to enter the for-loop body is 2**256 - 2
. This means that the i++
in the for loop can never overflow. Regardless, the overflow checks are performed by the compiler.
Unfortunately, the Solidity optimizer is not smart enough to detect this and remove the checks. You should manually do this by:
for (uint i = 0; i < length; i = unchecked_inc(i)) { // do something that doesn't change the value of i } function unchecked_inc(uint i) returns (uint) { unchecked { return i + 1; } }
Or just:
for (uint i = 0; i < length;) { // do something that doesn't change the value of i unchecked { i++; } }
Note that it’s important that the call to unchecked_inc
is inlined. This is only possible for solidity versions starting from 0.8.2
.
Gas savings: roughly speaking this can save 30-40 gas per loop iteration. For lengthy loops, this can be significant! (This is only relevant if you are using the default solidity checked arithmetic.)
Use the unchecked
keyword
File: 2023-02-ethos/Ethos-Core/contracts/ActivePool.sol
108: for(uint256 i = 0; i < numCollaterals; i++) {
File: 2023-02-ethos/Ethos-Core/contracts/CollateralConfig.sol
56: for(uint256 i = 0; i < _collaterals.length; i++) {
File: 2023-02-ethos/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: 2023-02-ethos/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: 2023-02-ethos/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++) {
Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.
mapping(address => uint256) public nonces; -mapping (address => bool) public minters; -mapping (address => bool) public markets; +mapping (address => uint256) public markets_minters; + // address => uint256 => 1 (minters true) + // address => uint256 => 2 (minters false) + // address => uint256 => 3 (markets true) + // address => uint256 => 4 (markets false)``` ##### Recommendation Where appropriate, you can combine multiple address mappings into a single mapping of an address to a struct. ##### *Instances (5):* File: [2023-02-ethos/Ethos-Core/contracts/LUSDToken.sol](https://github.com/code-423n4/2023-02-ethos/tree/main/Ethos-Core/contracts/LUSDToken.sol#L58 ) ```solidity 58: mapping (address => mapping (address => uint256)) private _allowances;
File: 2023-02-ethos/Ethos-Core/contracts/StabilityPool.sol
214: mapping (uint128 => mapping(uint128 => mapping (address => uint))) public epochToScaleToSum; 223: mapping (uint128 => mapping(uint128 => uint)) public epochToScaleToG;
File: 2023-02-ethos/Ethos-Core/contracts/TroveManager.sol
86: mapping (address => mapping (address => Trove)) public Troves; 109: mapping (address => mapping (address => RewardSnapshot)) public rewardSnapshots;
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 declaring 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(src)
Use storage for struct/array
File: 2023-02-ethos/Ethos-Core/contracts/ActivePool.sol
105: address[] memory collaterals = ICollateralConfig(collateralConfigAddress).getAllowedCollaterals();
File: 2023-02-ethos/Ethos-Core/contracts/LQTY/LQTYStaking.sol
147: (address[] memory collGainAssets, uint[] memory collGainAmounts) = _getPendingCollateralGain(msg.sender); 226: address[] memory collaterals = collateralConfig.getAllowedCollaterals(); 227: uint[] memory amounts = new uint[](collaterals.length);
File: 2023-02-ethos/Ethos-Core/contracts/StabilityPool.sol
332: (address[] memory assets, uint[] memory amounts) = getDepositorCollateralGain(msg.sender); 376: (address[] memory assets, uint[] memory amounts) = getDepositorCollateralGain(msg.sender); 806: address[] memory collaterals = collateralConfig.getAllowedCollaterals(); 807: uint[] memory amounts = new uint[](collaterals.length); 857: address[] memory collaterals = collateralConfig.getAllowedCollaterals();
File: 2023-02-ethos/Ethos-Core/contracts/TroveManager.sol
313: address[] memory borrowers = new address[](1);
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperVaultV2.sol
660: bytes32[] memory cascadingAccessRoles = new bytes32[](5);
File: 2023-02-ethos/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol
204: bytes32[] memory cascadingAccessRoles = new bytes32[](5);
uint
s/int
s 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.
Use uint256
instead.
File: 2023-02-ethos/Ethos-Core/contracts/LUSDToken.sol
35: uint8 constant internal _DECIMALS = 18; 268: uint8 v, 407: function decimals() external view override returns (uint8) {
File: 2023-02-ethos/Ethos-Core/contracts/StabilityPool.sol
147: using LiquitySafeMath128 for uint128; 182: uint128 scale; 183: uint128 epoch; 200: uint128 public currentScale; 203: uint128 public currentEpoch; 214: mapping (uint128 => mapping(uint128 => mapping (address => uint))) public epochToScaleToSum; 223: mapping (uint128 => mapping(uint128 => uint)) public epochToScaleToG; 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); 558: uint128 currentScaleCached = currentScale; 559: uint128 currentEpochCached = currentEpoch; 648: uint128 epochSnapshot; 649: uint128 scaleSnapshot; 699: uint128 epochSnapshot = snapshots.epoch; 700: uint128 scaleSnapshot = snapshots.scale; 738: uint128 scaleSnapshot = snapshots.scale; 739: uint128 epochSnapshot = snapshots.epoch; 745: uint128 scaleDiff = currentScale.sub(scaleSnapshot); 817: uint128 currentScaleCached = currentScale; 818: uint128 currentEpochCached = currentEpoch;
File: 2023-02-ethos/Ethos-Core/contracts/TroveManager.sol
82: uint128 arrayIndex; 1321: function _addTroveOwnerToArray(address _borrower, address _collateral) internal returns (uint128 index) { 1329: index = uint128(TroveOwners[_collateral].length.sub(1)); 1344: uint128 index = Troves[_borrower][_collateral].arrayIndex;
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol
35: uint16 private constant LENDER_REFERRAL_CODE_NONE = 0;
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperVaultV2.sol
650: function decimals() public view override returns (uint8) {
abi.encode()
is less efficient than abi.encodePacked()
abi.encode
will apply ABI encoding rules. Therefore all elementary types are padded to 32 bytes and dynamic arrays include their length. Therefore it is possible to also decode this data again (with abi.decode
) when the type are known.
abi.encodePacked
will only use the only use the minimal required memory to encode the data. E.g. an address will only use 20 bytes and for dynamic arrays only the elements will be stored without length. For more info see the Solidity docs for packed mode.
For the input of the keccak
method it is important that you can ensure that the resulting bytes of the encoding are unique. So if you always encode the same types and arrays always have the same length then there is no problem. But if you switch the parameters that you encode or encode multiple dynamic arrays you might have conflicts.
For example:
abi.encodePacked(address(0x0000000000000000000000000000000000000001), uint(0))
encodes to the same as abi.encodePacked(uint(0x0000000000000000000000000000000000000001000000000000000000000000), address(0))
and abi.encodePacked(uint[](1,2), uint[](3))
encodes to the same as abi.encodePacked(uint[](1), uint[](2,3))
Therefore these examples will also generate the same hashes even so they are different inputs.
On the other hand you require less memory and therefore in most cases abi.encodePacked uses less gas than abi.encode.
Use abi.encodePacked()
where possible to save gas
File: 2023-02-ethos/Ethos-Core/contracts/LUSDToken.sol
284: domainSeparator(), keccak256(abi.encode( 305: return keccak256(abi.encode(typeHash, name, version, _chainID(), address(this)));
bytes32
instead of string.Use bytes32
instead of string
to save gas whenever possible. String is a dynamic data structure and therefore is more gas consuming than bytes32
.
Use bytes32
instead of string
File: 2023-02-ethos/Ethos-Core/contracts/ActivePool.sol
30: string constant public NAME = "ActivePool";
File: 2023-02-ethos/Ethos-Core/contracts/BorrowerOperations.sol
21: string constant public NAME = "BorrowerOperations";
File: 2023-02-ethos/Ethos-Core/contracts/LQTY/CommunityIssuance.sol
19: string constant public NAME = "CommunityIssuance";
File: 2023-02-ethos/Ethos-Core/contracts/LQTY/LQTYStaking.sol
23: string constant public NAME = "LQTYStaking";
File: 2023-02-ethos/Ethos-Core/contracts/LUSDToken.sol
32: string constant internal _NAME = "LUSD Stablecoin"; 33: string constant internal _SYMBOL = "LUSD"; 34: string constant internal _VERSION = "1";
File: 2023-02-ethos/Ethos-Core/contracts/StabilityPool.sol
150: string constant public NAME = "StabilityPool";
File: 2023-02-ethos/Ethos-Core/contracts/TroveManager.sol
19: // string constant public NAME = "TroveManager";
Not inlining costs 20 to 40 gas because of two extra JUMP
instructions and additional stack operations needed for function calls.
Check this out: link
Note: To sum up, when there is an internal function that is only called once, there is no point for that function to exist.
Remove the function and make it inline if it is a simple function.
File: 2023-02-ethos/Ethos-Core/contracts/ActivePool.sol
320: function _requireCallerIsBorrowerOperationsOrDefaultPool() internal view {
File: 2023-02-ethos/Ethos-Core/contracts/LQTY/LQTYStaking.sol
265: function _requireUserHasStake(uint currentStake) internal pure { 269: function _requireNonZeroAmount(uint _amount) internal pure {
File: 2023-02-ethos/Ethos-Core/contracts/LUSDToken.sol
380: function _requireCallerIsTroveMorSP() internal view { 393: function _requireMintingNotPaused() internal view {
File: 2023-02-ethos/Ethos-Core/contracts/StabilityPool.sol
597: function _moveOffsetCollAndDebt(address _collateral, uint _collToAdd, uint _debtToOffset) internal { 613: function _decreaseLUSD(uint _amount) internal { 637: function _getCollateralGainFromSnapshots(uint initialDeposit, Snapshots storage snapshots) internal view returns (address[] memory assets, uint[] memory amounts) { 657: function _getSingularCollateralGain(uint _initialDeposit, address _collateral, Snapshots storage _snapshots) internal view returns (uint) { 693: function _getLQTYGainFromSnapshots(uint initialDeposit, Snapshots memory snapshots) internal view returns (uint) { 776: function _sendLUSDtoStabilityPool(address _address, uint _amount) internal { 783: function _sendCollateralGainToDepositor(address _collateral, uint _amount) internal { 794: function _sendLUSDToDepositor(address _depositor, uint LUSDWithdrawal) internal { 840: function _payOutLQTYGains(ICommunityIssuance _communityIssuance, address _depositor) internal { 848: function _requireCallerIsActivePool() internal view { 869: function _requireUserHasDeposit(uint _initialDeposit) internal pure { 873: function _requireNonZeroAmount(uint _amount) internal pure {
File: 2023-02-ethos/Ethos-Core/contracts/TroveManager.sol
1538: function _requireMoreThanOneTroveInSystem(uint TroveOwnersArrayLength, address _collateral) internal view {
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol
74: function _adjustPosition(uint256 _debt) internal override { 104: function _liquidateAllPositions() internal override returns (uint256 amountFreed) { 114: function _harvestCore(uint256 _debt) internal override returns (int256 roi, uint256 repayment) { 206: function _claimRewards() internal {
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperVaultV2.sol
2418: function _calculateLockedProfit() internal view returns (uint256) { 432: function _reportLoss(address strategy, uint256 loss) internal { 462: function _chargeFees(address strategy, uint256 gain) internal returns (uint256) {
>=
costs less gas than >
.The compiler uses opcodes GT
and ISZERO
for solidity code that uses >
, but only requires LT
for >=
, which saves 3 gas
Use >=
instead if appropriate.
File: 2023-02-ethos/Ethos-Core/contracts/ActivePool.sol
264: if (vars.percentOfFinalBal > vars.yieldingPercentage && vars.percentOfFinalBal.sub(vars.yieldingPercentage) > yieldingPercentageDrift) { 269: } else if(vars.percentOfFinalBal < vars.yieldingPercentage && vars.yieldingPercentage.sub(vars.percentOfFinalBal) > yieldingPercentageDrift) { 278: if (vars.netAssetMovement > 0) {
File: 2023-02-ethos/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: 2023-02-ethos/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: 2023-02-ethos/Ethos-Core/contracts/LUSDToken.sol
279: if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {
File: 2023-02-ethos/Ethos-Core/contracts/StabilityPool.sol
591: assert(newP > 0);
File: 2023-02-ethos/Ethos-Core/contracts/TroveManager.sol
397: } else if ((_ICR > _100pct) && (_ICR < _MCR)) { 424: if (singleLiquidation.collSurplus > 0) { 431: } else { // if (_ICR >= _MCR && ( _ICR >= _TCR || singleLiquidation.entireTroveDebt > _LUSDInStabPool)) 452: if (_LUSDInStabPool > 0) { 495: } else if (_collDecimals > LiquityMath.CR_CALCULATION_DECIMALS) { 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); 1539: require (TroveOwnersArrayLength > 1 && sortedTroves.getSize(_collateral) > 1);
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol
80: if (wantBalance > _debt) { 99: if (_amountNeeded > liquidatedAmount) { 131: if (totalAssets > allocated) {
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperVaultV2.sol
234: if (stratCurrentAllocation > stratMaxAllocation) { 368: if (value > token.balanceOf(address(this))) { 400: if (value > vaultBalance) { 502: } else if (_roi > 0) { 518: } else if (vars.available > 0) { 525: if (vars.credit > vars.freeWantInStrat) { 534: if (vars.lockedProfitBeforeLoss > vars.loss) {
File: 2023-02-ethos/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol
122: } else if (amountFreed > debt) {
<x> += <y>
costs more gas than <x> = <x> + <y>
for state variablesUsing the addition operator instead of plus-equals saves 113 gas
Use <x> = <x> + <y>
instead.
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol
133: toFree += profit; 141: roi -= int256(loss);
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperVaultV2.sol
168: totalAllocBPS += _allocBPS; 194: totalAllocBPS -= strategies[_strategy].allocBPS; 196: totalAllocBPS += _allocBPS; 214: totalAllocBPS -= strategies[_strategy].allocBPS; 390: value -= loss; 391: totalLoss += loss; 395: strategies[stratAddr].allocated -= actualWithdrawn; 396: totalAllocated -= actualWithdrawn; 444: stratParams.allocBPS -= bpsChange; 445: totalAllocBPS -= bpsChange; 450: stratParams.losses += loss; 451: stratParams.allocated -= loss; 452: totalAllocated -= loss; 505: strategy.gains += vars.gain; 514: strategy.allocated -= vars.debtPayment; 515: totalAllocated -= vars.debtPayment; 516: vars.debt -= vars.debtPayment; // tracked for return value 520: strategy.allocated += vars.credit; 521: totalAllocated += vars.credit;
File: 2023-02-ethos/Ethos-Vault/contracts/abstract/ReaperBaseStrategyv4.sol
128: repayment -= uint256(-roi);
Importing only what is needed is ideal for gas optimization.
Import only what is necessary from file.
File: 2023-02-ethos/Ethos-Core/contracts/ActivePool.sol
6: import "./Interfaces/ICollateralConfig.sol"; 8: import "./Interfaces/ICollSurplusPool.sol"; 9: import "./Interfaces/ILQTYStaking.sol"; 10: import "./Interfaces/IStabilityPool.sol"; 11: import "./Interfaces/ITroveManager.sol"; 12: import "./Dependencies/SafeMath.sol"; 13: import "./Dependencies/Ownable.sol"; 14: import "./Dependencies/CheckContract.sol"; 15: import "./Dependencies/console.sol"; 16: import "./Dependencies/SafeERC20.sol"; 17: import "./Dependencies/IERC4626.sol";
File: 2023-02-ethos/Ethos-Core/contracts/BorrowerOperations.sol
5: import "./Interfaces/IBorrowerOperations.sol"; 6: import "./Interfaces/ICollateralConfig.sol"; 7: import "./Interfaces/ITroveManager.sol"; 8: import "./Interfaces/ILUSDToken.sol"; 9: import "./Interfaces/ICollSurplusPool.sol"; 10: import "./Interfaces/ISortedTroves.sol"; 11: import "./Interfaces/ILQTYStaking.sol"; 12: import "./Dependencies/LiquityBase.sol"; 13: import "./Dependencies/Ownable.sol"; 14: import "./Dependencies/CheckContract.sol"; 15: import "./Dependencies/console.sol"; 16: import "./Dependencies/SafeERC20.sol";
File: 2023-02-ethos/Ethos-Core/contracts/CollateralConfig.sol
5: import "./Dependencies/CheckContract.sol"; 6: import "./Dependencies/Ownable.sol"; 7: import "./Dependencies/SafeERC20.sol"; 8: import "./Interfaces/ICollateralConfig.sol";
File: 2023-02-ethos/Ethos-Core/contracts/LQTY/CommunityIssuance.sol
5: import "../Dependencies/IERC20.sol"; 6: import "../Interfaces/ICommunityIssuance.sol"; 7: import "../Dependencies/BaseMath.sol"; 8: import "../Dependencies/LiquityMath.sol"; 9: import "../Dependencies/Ownable.sol"; 10: import "../Dependencies/CheckContract.sol"; 11: import "../Dependencies/SafeMath.sol";
File: 2023-02-ethos/Ethos-Core/contracts/LQTY/LQTYStaking.sol
5: import "../Dependencies/BaseMath.sol"; 6: import "../Dependencies/SafeMath.sol"; 7: import "../Dependencies/Ownable.sol"; 8: import "../Dependencies/CheckContract.sol"; 9: import "../Dependencies/console.sol"; 10: import "../Dependencies/IERC20.sol"; 11: import "../Interfaces/ICollateralConfig.sol"; 12: import "../Interfaces/ILQTYStaking.sol"; 13: import "../Interfaces/ITroveManager.sol"; 14: import "../Dependencies/LiquityMath.sol"; 15: import "../Interfaces/ILUSDToken.sol"; 16: import "../Dependencies/SafeERC20.sol";
File: 2023-02-ethos/Ethos-Core/contracts/LUSDToken.sol
5: import "./Interfaces/ILUSDToken.sol"; 6: import "./Interfaces/ITroveManager.sol"; 7: import "./Dependencies/SafeMath.sol"; 8: import "./Dependencies/CheckContract.sol"; 9: import "./Dependencies/console.sol";
File: 2023-02-ethos/Ethos-Core/contracts/StabilityPool.sol
6: import "./Interfaces/ICollateralConfig.sol"; 12: import "./Interfaces/ICommunityIssuance.sol"; 13: import "./Dependencies/LiquityBase.sol"; 14: import "./Dependencies/SafeMath.sol"; 15: import "./Dependencies/LiquitySafeMath128.sol"; 16: import "./Dependencies/Ownable.sol"; 17: import "./Dependencies/CheckContract.sol"; 18: import "./Dependencies/console.sol"; 19: import "./Dependencies/SafeERC20.sol";
File: 2023-02-ethos/Ethos-Core/contracts/TroveManager.sol
5: import "./Interfaces/ICollateralConfig.sol"; 6: import "./Interfaces/ITroveManager.sol"; 7: import "./Interfaces/IStabilityPool.sol"; 8: import "./Interfaces/ICollSurplusPool.sol"; 9: import "./Interfaces/ILUSDToken.sol"; 10: import "./Interfaces/ISortedTroves.sol"; 11: import "./Interfaces/ILQTYStaking.sol"; 12: import "./Interfaces/IRedemptionHelper.sol"; 13: import "./Dependencies/LiquityBase.sol"; 14: // import "./Dependencies/Ownable.sol"; 15: import "./Dependencies/CheckContract.sol"; 16: import "./Dependencies/IERC20.sol";
File: 2023-02-ethos/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";
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperVaultERC4626.sol
5: import "./ReaperVaultV2.sol"; 6: import "./interfaces/IERC4626Functions.sol";
File: 2023-02-ethos/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";
File: 2023-02-ethos/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";
string
.As a rule of thumb, use bytes for arbitrary-length raw byte data and string
for arbitrary-length string (UTF-8) data. If you can limit the length to a certain number of bytes, always use one of bytes1
to bytes32
because they are much cheaper..
Use fixed bytes instead of string
.
File: 2023-02-ethos/Ethos-Core/contracts/LUSDToken.sol
399: function name() external view override returns (string memory) { 403: function symbol() external view override returns (string memory) { 411: function version() external view override returns (string memory) {
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperVaultERC4626.sol
18: string memory _name, 19: string memory _symbol,
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperVaultV2.sol
113: string memory _name, 114: string memory _symbol,
You can save about 6 gas per instance if using assembly to check for address(0)
Use assembly.
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperStrategyGranarySupplyOnly.sol
167: require(step[0] != address(0)); 168: require(step[1] != address(0));
File: 2023-02-ethos/Ethos-Vault/contracts/ReaperVaultV2.sol
151: require(_strategy != address(0), "Invalid strategy address"); 629: require(newTreasury != address(0), "Invalid address");
#0 - c4-judge
2023-03-09T17:34:41Z
trust1995 marked the issue as grade-a