Platform: Code4rena
Start Date: 23/06/2023
Pot Size: $60,500 USDC
Total HM: 31
Participants: 132
Period: 10 days
Judge: 0xean
Total Solo HM: 10
Id: 254
League: ETH
Rank: 85/132
Findings: 2
Award: $70.37
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0xnev
Also found by: 0xRobocop, 0xbrett8571, 0xkazim, 0xnacho, 3agle, 8olidity, ABAIKUNANBAEV, Bauchibred, Co0nan, CrypticShepherd, D_Auditor, DelerRH, HE1M, Iurii3, Kaysoft, MrPotatoMagic, RedOneN, RedTiger, Rolezn, SanketKogekar, Sathish9098, Timenov, Toshii, Vagner, bart1e, bytes032, codetilda, devival, halden, hals, kutugu, m_Rassska, naman1778, nonseodion, seth_lawson, solsaver, squeaky_cactus, totomanov, y51r, yudan, zaevlad
57.9031 USDC - $57.90
Issue | Contexts | |
---|---|---|
LOW‑1 | IERC20 approve() Is Deprecated | 2 |
LOW‑2 | Consider the case where totalsupply is 0 | 1 |
LOW‑3 | Do not allow fees to be set to 100% | 2 |
LOW‑4 | External calls in an un-bounded for- loop may result in a DOS | 2 |
LOW‑5 | Hardcoded String that is repeatedly used can be replaced with a constant | 4 |
LOW‑6 | Lack of checks-effects-interactions | 2 |
LOW‑7 | Minting tokens to the zero address should be avoided | 1 |
LOW‑8 | The Contract Should approve(0) First | 2 |
LOW‑9 | Contracts are not using their OZ Upgradeable counterparts | 23 |
LOW‑10 | There should be an upper limit for flash loan settings | 2 |
LOW‑11 | Remove unused code | 8 |
LOW‑12 | Unbounded loop | 1 |
LOW‑13 | Upgrade OpenZeppelin Contract Dependency | 1 |
LOW‑14 | Use safeTransferOwnership instead of transferOwnership function | 4 |
LOW‑15 | _creditTo call should avoid setting to 0 amount and to zero address | 1 |
Total: 56 contexts over 15 issues
Issue | Contexts | |
---|---|---|
NC‑1 | No need for == true or == false checks | 1 |
NC‑2 | Critical Changes Should Use Two-step Procedure | 33 |
NC‑3 | block.timestamp is already used when emitting events, no need to input timestamp | 36 |
NC‑4 | Immutables can be named using the same convention | 18 |
NC‑5 | Initial value check is missing in Set Functions | 31 |
NC‑6 | Omissions in Events | 1 |
NC‑7 | Strings should use double quotes rather than single quotes | 1 |
NC‑8 | Using underscore at the end of variable name | 1 |
NC‑9 | Large multiples of ten should use scientific notation | 11 |
NC‑10 | Use named function calls | 1 |
NC‑11 | maxSupply should be set as a constant | 2 |
Total: 136 contexts over 11 issues
approve()
Is Deprecatedapprove()
is subject to a known front-running attack. It is deprecated in favor of safeIncreaseAllowance()
and safeDecreaseAllowance()
. If only setting the initial allowance to the value that means infinite, safeIncreaseAllowance()
can be used instead.
https://docs.openzeppelin.com/contracts/3.x/api/token/erc20#IERC20-approve-address-uint256-
302: EUSD.approve(address(curvePool), balance);
35: lido.approve(address(collateralAsset), msg.value);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraWstETHVault.sol#L35
</details>Consider using safeIncreaseAllowance()
/ safeDecreaseAllowance()
instead.
Consider the case where totalsupply
is 0. When totalsupply
is 0, it should return 0 directly, because there will be an error of dividing by 0.
This would cause the affected functions to revert and as a result can lead to potential loss.
</details>79: return rewardPerTokenStored + (rewardRatio * (lastTimeRewardApplicable() - updatedAt) * 1e18) / totalSupply;
Add check for zero value and return 0.
if ( totalSupply() == 0) return 0;
100%
It is recommended from a risk perspective to disallow setting 100% fees at all. A reasonable fee maximum should be checked for instead.
function setRedemptionFee(uint256 newFee) external checkRole(TIMELOCK) { require(newFee <= 500, "Max Redemption Fee is 5%"); redemptionFee = newFee; emit RedemptionFeeChanged(newFee); }
</details>function setFlashloanFee(uint256 fee) external checkRole(TIMELOCK) { if (fee > 10_000) revert('EL'); emit FlashloanFeeUpdated(fee); flashloanFee = fee; }
for-
loop may result in a DOSConsider limiting the number of iterations in for-
loops that make external calls
236: for (uint256 i = 0; i < _contracts.length; i++) {
</details>94: for (uint i = 0; i < _pools.length; i++) {
Some strings are repeatedly used in the contracts. For better maintainability, please consider replacing it with a constant
.
100: return "eUSD";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/EUSD.sol#L100
108: return "eUSD";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/EUSD.sol#L108
</details>It's recommended to execute external calls after state changes, to prevent reentrancy bugs.
Consider moving the external calls after the state changes on the following instances.
85: function stake(uint256 _amount) external updateReward(msg.sender) { require(_amount > 0, "amount = 0"); bool success = stakingToken.transferFrom(msg.sender, address(this), _amount); require(success, "TF"); balanceOf[msg.sender] += _amount; totalSupply += _amount; emit StakeToken(msg.sender, _amount, block.timestamp); }
</details>75: function depositAssetToMint(uint256 assetAmount, uint256 mintAmount) external virtual { require(assetAmount >= 1 ether, "Deposit should not be less than 1 stETH."); bool success = collateralAsset.transferFrom(msg.sender, address(this), assetAmount); require(success, "TF"); totalDepositedAsset += assetAmount; depositedAsset[msg.sender] += assetAmount; depositedTime[msg.sender] = block.timestamp; if (mintAmount > 0) { _mintEUSD(msg.sender, msg.sender, mintAmount, getAssetPrice()); } emit DepositAsset(msg.sender, address(collateralAsset), assetAmount, block.timestamp); }
The core function mint
is used by users to mint an option position by providing token1 as collateral and borrowing the max amount of liquidity. Address(0)
check is missing in both this function and the internal function _mint
, which is triggered to mint the tokens to the to address. Consider applying a check in the function to ensure tokens aren't minted to the zero address.
function mint(address user, uint256 amount) external returns (bool) { require(configurator.tokenMiner(msg.sender), "not authorized"); require(totalSupply() + amount <= maxSupply, "exceeding the maximum supply quantity."); try IProtocolRewardsPool(configurator.getProtocolRewardsPool()).refreshReward(user) {} catch {} _mint(user, amount); return true; }
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/esLBR.sol#L30
</details>approve(0)
FirstSome tokens (like USDT L199) do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved.
302: EUSD.approve(address(curvePool), balance);
35: lido.approve(address(collateralAsset), msg.value);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraWstETHVault.sol#L35
</details>Approve with a zero amount first before setting the actual amount.
The non-upgradeable standard version of OpenZeppelin’s library are inherited / used by the contracts. It would be safer to use the upgradeable versions of the library contracts to avoid unexpected behaviour.
5: import "@openzeppelin/contracts/governance/TimelockController.sol";
5: import "@openzeppelin/contracts/governance/TimelockController.sol";
5: import "@openzeppelin/contracts/governance/extensions/GovernorTimelockControl.sol";
5: import "@openzeppelin/contracts/access/Ownable.sol";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/miner/esLBRBoost.sol#L5
12: import "@openzeppelin/contracts/access/Ownable.sol";
12: import "@openzeppelin/contracts/access/Ownable.sol"; 13: import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
4: import "@openzeppelin/contracts/access/Ownable.sol"; 6: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
7: import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraRETHVault.sol#L7
7: import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraWbETHVault.sol#L7
7: import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraWstETHVault.sol#L7
7: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
7: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
5: import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/Proxy/LybraProxy.sol#L5
4: import "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/Proxy/LybraProxyAdmin.sol#L4
10: import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Votes.sol";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/esLBR.sol#L10
5: import "@openzeppelin/contracts/utils/math/SafeMath.sol"; 6: import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; 7: import "@openzeppelin/contracts/utils/Context.sol";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/EUSD.sol#L5-L7
9: import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/LBR.sol#L9
5: import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/PeUSD.sol#L5
</details>16: import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
Where applicable, use the contracts from @openzeppelin/contracts-upgradeable
instead of @openzeppelin/contracts
.
See https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/tree/master/contracts for list of available upgradeable contracts
If the admin sets the flash loan too high, the flash loan functionality might be useless as users won't use it.
</details>56: uint256 public flashloanFee = 500; 256: flashloanFee = fee;
This code is not used in the main project contract files, remove it or add event-emit Code that is not in use, suggests that they should not be present and could potentially contain insecure functionalities.
function _quorumReached
function _voteSucceeded
function _countVote
function _getVotes
function _debitFrom
function _creditTo
function _transferFrom
</details>function _ld2sdRatio
New items are pushed into the following arrays but there is no option to pop
them out. Currently, the array can grow indefinitely. E.g. there's no maximum limit and there's no functionality to remove array values.
If the array grows too large, calling relevant functions might run out of gas and revert. Calling these functions could result in a DOS condition.
34: esLBRLockSettings.push(setting);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/miner/esLBRBoost.sol#L34
</details>Add a functionality to delete array values or add a maximum size limit for arrays.
An outdated OZ version is used (which has known vulnerabilities, see: https://github.com/OpenZeppelin/openzeppelin-contracts/security/advisories). Release: https://github.com/OpenZeppelin/openzeppelin-contracts/releases/tag/v4.9.0
Require dependencies to be at least version of 4.9.0 to include critical vulnerability patches.
<details>"@openzeppelin/contracts": "^4.9.1"
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/../package.json#L13
</details>Update OpenZeppelin Contracts Usage in package.json and require at least version of 4.9.0 via >=4.9.0
safeTransferOwnership
instead of transferOwnership
functionUse safeTransferOwnership
which is safer. Use it as it is more secure due to 2-stage ownership transfer.
The Ownable.sol
only uses transferOwnership
rather than safeTransferOwnership
5: import "@openzeppelin/contracts/access/Ownable.sol";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/miner/esLBRBoost.sol#L5
12: import "@openzeppelin/contracts/access/Ownable.sol";
12: import "@openzeppelin/contracts/access/Ownable.sol";
4: import "@openzeppelin/contracts/access/Ownable.sol";
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/miner/stakerewardV2pool.sol#L4
</details>Use <a href="https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/Ownable2Step.sol">Ownable2Step.sol</a>
function acceptOwnership() external { address sender = _msgSender(); require(pendingOwner() == sender, "Ownable2Step: caller is not the new owner"); _transferOwnership(sender); } }
_creditTo
call should avoid setting to 0 _amount
and to zero address
The _creditTo
function can be called with 0 _amount
and with zero address
to _toAddress
which should be avoided.
function _creditTo(uint16, address _toAddress, uint _amount) internal virtual override returns (uint) {
Add relevant require statements against zero value calls
== true
or == false
checksThere is no need to verify that == true
or == false
when the variable checked upon is a boolean as well.
</details>82: require(receipt.hasVoted == false, "GovernorBravo::castVoteInternal: voter already voted");
Instead simply check for variable
or !variable
The critical procedures should be two step process.
See similar findings in previous Code4rena contests for reference: https://code4rena.com/reports/2022-06-illuminate/#2-critical-changes-should-use-two-step-procedure
109: function setMintVault(address pool, bool isActive) external onlyRole(DAO) {
119: function setMintVaultMaxSupply(address pool, uint256 maxSupply) external onlyRole(DAO) {
126: function setBadCollateralRatio(address pool, uint256 newRatio) external onlyRole(DAO) {
137: function setProtocolRewardsPool(address addr) external checkRole(TIMELOCK) {
147: function setEUSDMiningIncentives(address addr) external checkRole(TIMELOCK) {
158: function setvaultBurnPaused(address pool, bool isActive) external checkRole(TIMELOCK) {
167: function setPremiumTradingEnabled(bool isActive) external checkRole(TIMELOCK) {
177: function setvaultMintPaused(address pool, bool isActive) external checkRole(ADMIN) {
186: function setRedemptionFee(uint256 newFee) external checkRole(TIMELOCK) {
198: function setSafeCollateralRatio(address pool, uint256 newRatio) external checkRole(TIMELOCK) {
213: function setBorrowApy(address pool, uint256 newApy) external checkRole(TIMELOCK) {
224: function setKeeperRatio(address pool,uint256 newRatio) external checkRole(TIMELOCK) {
235: function setTokenMiner(address[] calldata _contracts, bool[] calldata _bools) external checkRole(TIMELOCK) {
246: function setMaxStableRatio(uint256 _ratio) external checkRole(TIMELOCK) {
253: function setFlashloanFee(uint256 fee) external checkRole(TIMELOCK) {
261: function setProtocolRewardsToken(address _token) external checkRole(TIMELOCK) {
38: function setLockStatus(uint256 id) external {
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/miner/esLBRBoost.sol#L38
84: function setToken(address _lbr, address _eslbr) external onlyOwner {
89: function setLBROracle(address _lbrOracle) external onlyOwner {
93: function setPools(address[] memory _pools) external onlyOwner {
100: function setBiddingCost(uint256 _biddingRatio) external onlyOwner {
105: function setExtraRatio(uint256 ratio) external onlyOwner {
110: function setPeUSDExtraRatio(uint256 ratio) external onlyOwner {
115: function setBoost(address _boost) external onlyOwner {
119: function setRewardsDuration(uint256 _duration) external onlyOwner {
124: function setEthlbrStakeInfo(address _pool, address _lp) external onlyOwner {
128: function setEUSDBuyoutAllowed(bool _bool) external onlyOwner {
52: function setTokenAddress(address _eslbr, address _lbr, address _boost) external onlyOwner {
58: function setGrabCost(uint256 _ratio) external onlyOwner {
121: function setRewardsDuration(uint256 _duration) external onlyOwner {
127: function setBoost(address _boost) external onlyOwner {
41: function setRkPool(address addr) external {
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraRETHVault.sol#L41
25: function setLidoRebaseTime(uint256 _time) external {
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraStETHVault.sol#L25
</details>Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.
Immutables should be in uppercase per naming convention. As shown below, some immutables are named using capital letters and underscores while some are not. For a better code quality, please consider naming these immutables using the same naming convention.
28: Iconfigurator public immutable configurator; 30: IEUSD public immutable EUSD;
18: IEUSD public immutable EUSD; 19: IERC20 public immutable collateralAsset; 20: Iconfigurator public immutable configurator; 21: uint256 public immutable badCollateralRatio = 150 * 1e18; 22: IPriceFeed immutable etherOracle; 30: uint8 immutable vaultType = 0;
14: IPeUSD public immutable PeUSD; 15: IERC20 public immutable collateralAsset; 16: Iconfigurator public immutable configurator; 18: uint8 immutable vaultType = 1; 19: IPriceFeed immutable etherOracle;
14: Iconfigurator public immutable configurator; 16: uint internal immutable ld2sdRatio;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/LBR.sol#L14
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/LBR.sol#L16
</details>30: IEUSD public immutable EUSD; 31: Iconfigurator public immutable configurator; 32: uint internal immutable ld2sdRatio;
A check regarding whether the current value and the new value are the same should be added
109: function setMintVault(address pool, bool isActive) external onlyRole(DAO) { mintVault[pool] = isActive; }
119: function setMintVaultMaxSupply(address pool, uint256 maxSupply) external onlyRole(DAO) { mintVaultMaxSupply[pool] = maxSupply; }
126: function setBadCollateralRatio(address pool, uint256 newRatio) external onlyRole(DAO) { require(newRatio >= 130 * 1e18 && newRatio <= 150 * 1e18 && newRatio <= vaultSafeCollateralRatio[pool] + 1e19, "LNA"); vaultBadCollateralRatio[pool] = newRatio; emit SafeCollateralRatioChanged(pool, newRatio); }
137: function setProtocolRewardsPool(address addr) external checkRole(TIMELOCK) { lybraProtocolRewardsPool = IProtocolRewardsPool(addr); emit ProtocolRewardsPoolChanged(addr, block.timestamp); }
147: function setEUSDMiningIncentives(address addr) external checkRole(TIMELOCK) { eUSDMiningIncentives = IeUSDMiningIncentives(addr); emit EUSDMiningIncentivesChanged(addr, block.timestamp); }
158: function setvaultBurnPaused(address pool, bool isActive) external checkRole(TIMELOCK) { vaultBurnPaused[pool] = isActive; }
167: function setPremiumTradingEnabled(bool isActive) external checkRole(TIMELOCK) { premiumTradingEnabled = isActive; }
177: function setvaultMintPaused(address pool, bool isActive) external checkRole(ADMIN) { vaultMintPaused[pool] = isActive; }
186: function setRedemptionFee(uint256 newFee) external checkRole(TIMELOCK) { require(newFee <= 500, "Max Redemption Fee is 5%"); redemptionFee = newFee; emit RedemptionFeeChanged(newFee); }
213: function setBorrowApy(address pool, uint256 newApy) external checkRole(TIMELOCK) { require(newApy <= 200, "Borrow APY cannot exceed 2%"); vaultMintFeeApy[pool] = newApy; emit BorrowApyChanged(pool, newApy); }
224: function setKeeperRatio(address pool,uint256 newRatio) external checkRole(TIMELOCK) { require(newRatio <= 5, "Max Keeper reward is 5%"); vaultKeeperRatio[pool] = newRatio; emit KeeperRatioChanged(pool, newRatio); }
235: function setTokenMiner(address[] calldata _contracts, bool[] calldata _bools) external checkRole(TIMELOCK) { for (uint256 i = 0; i < _contracts.length; i++) { tokenMiner[_contracts[i]] = _bools[i]; emit tokenMinerChanges(_contracts[i], _bools[i]); } }
246: function setMaxStableRatio(uint256 _ratio) external checkRole(TIMELOCK) { require(_ratio <= 10_000, "The maximum value is 10000"); maxStableRatio = _ratio; }
253: function setFlashloanFee(uint256 fee) external checkRole(TIMELOCK) { if (fee > 10_000) revert('EL'); emit FlashloanFeeUpdated(fee); flashloanFee = fee; }
261: function setProtocolRewardsToken(address _token) external checkRole(TIMELOCK) { stableToken = _token; }
84: function setToken(address _lbr, address _eslbr) external onlyOwner { LBR = _lbr; esLBR = _eslbr; }
89: function setLBROracle(address _lbrOracle) external onlyOwner { lbrPriceFeed = AggregatorV3Interface(_lbrOracle); }
93: function setPools(address[] memory _pools) external onlyOwner { for (uint i = 0; i < _pools.length; i++) { require(configurator.mintVault(_pools[i]), "NOT_VAULT"); } pools = _pools; }
100: function setBiddingCost(uint256 _biddingRatio) external onlyOwner { require(_biddingRatio <= 8000, "BCE"); biddingFeeRatio = _biddingRatio; }
105: function setExtraRatio(uint256 ratio) external onlyOwner { require(ratio <= 1e20, "BCE"); extraRatio = ratio; }
110: function setPeUSDExtraRatio(uint256 ratio) external onlyOwner { require(ratio <= 1e20, "BCE"); peUSDExtraRatio = ratio; }
115: function setBoost(address _boost) external onlyOwner { esLBRBoost = IesLBRBoost(_boost); }
119: function setRewardsDuration(uint256 _duration) external onlyOwner { require(finishAt < block.timestamp, "reward duration not finished"); duration = _duration; }
124: function setEthlbrStakeInfo(address _pool, address _lp) external onlyOwner { ethlbrStakePool = _pool; ethlbrLpToken = _lp; }
128: function setEUSDBuyoutAllowed(bool _bool) external onlyOwner { isEUSDBuyoutAllowed = _bool; }
52: function setTokenAddress(address _eslbr, address _lbr, address _boost) external onlyOwner { esLBR = IesLBR(_eslbr); LBR = IesLBR(_lbr); esLBRBoost = IesLBRBoost(_boost); }
58: function setGrabCost(uint256 _ratio) external onlyOwner { require(_ratio <= 8000, "BCE"); grabFeeRatio = _ratio; }
121: function setRewardsDuration(uint256 _duration) external onlyOwner { require(finishAt < block.timestamp, "reward duration not finished"); duration = _duration; }
127: function setBoost(address _boost) external onlyOwner { esLBRBoost = IesLBRBoost(_boost); }
41: function setRkPool(address addr) external { require(configurator.hasRole(keccak256("TIMELOCK"), msg.sender)); rkPool = IRkPool(addr); }
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraRETHVault.sol#L41
25: function setLidoRebaseTime(uint256 _time) external { require(configurator.hasRole(keccak256("ADMIN"), msg.sender), "not authorized"); lidoRebaseTime = _time; }
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraStETHVault.sol#L25
</details>Throughout the codebase, events are generally emitted when sensitive changes are made to the contracts. However, some events are missing important parameters The following events should also add the previous original value in addition to the new value.
</details>74: event FlashloanFeeUpdated(uint256 fee);
The events should include the new value and old value where possible.
See the Solidity Style <a href="https://docs.soliditylang.org/en/v0.8.20/style-guide.html#other-recommendations">Guide</a>
</details>254: if (fee > 10_000) revert('EL');
The use of underscore at the end of the variable name is uncommon and also suggests that the variable name was not completely changed. Consider refactoring variableName_
to variableName
.
</details>39: timelock_;
Use (e.g. 1e6) rather than decimal literals (e.g. 100000), for better code readability.
247: require(_ratio <= 10_000, "The maximum value is 10000");
189: return (stakedLBRLpValue(user) * 10000) / stakedOf(user) < 500;
210: uint256 biddingFee = (reward * biddingFeeRatio) / 10000;
134: LBR.burn(msg.sender, (amount * grabFeeRatio) / 10000);
66: uint256 payAmount = (((realAmount * getAssetPrice()) / 1e18) * getDutchAuctionDiscountPrice()) / 10000;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraStETHVault.sol#L66
103: if (time < 30 minutes) return 10000;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraStETHVault.sol#L103
104: return 10000 - (time / 30 minutes - 1) * 100;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraStETHVault.sol#L104
239: uint256 collateralAmount = (((eusdAmount * 1e18) / assetPrice) * (10000 - configurator.redemptionFee())) / 10000;
301: return (poolTotalEUSDCirculation * configurator.vaultMintFeeApy(address(this)) * (block.timestamp - lastReportTime)) / (86400 * 365) / 10000;
164: uint256 collateralAmount = (((peusdAmount * 1e18) / assetPrice) * (10000 - configurator.redemptionFee())) / 10000;
</details>238: return (borrowed[user] * configurator.vaultMintFeeApy(address(this)) * (block.timestamp - feeUpdatedAt[user])) / (86400 * 365) / 10000;
Code base has an extensive use of named function calls, but it somehow missed one instance where this would be appropriate.
It should use named function calls on function call, as such:
lido.submit{value: msg.value}({ _configurator: address(configurator) });
32: uint256 sharesAmount = lido.submit{value: msg.value}(address(configurator));
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraWstETHVault.sol#L32
</details>maxSupply
should be set as a constant
The maxSupply
should be set a constant
since it is not going to change.
uint256 maxSupply = 100_000_000 * 1e18;
https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/esLBR.sol#L20
uint256 maxSupply = 100_000_000 * 1e18;
https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/token/LBR.sol#L15
#0 - c4-pre-sort
2023-07-27T19:36:10Z
JeffCX marked the issue as high quality report
#1 - c4-judge
2023-07-27T23:58:32Z
0xean marked the issue as grade-a
🌟 Selected for report: JCN
Also found by: 0xAnah, DavidGiladi, MohammedRizwan, Rageur, Raihan, ReyAdmirado, Rolezn, SAAJ, SAQ, SM3_SS, Sathish9098, ayo_dev, dharma09, fatherOfBlocks, hunter_w3b, mgf15, mrudenko, naman1778, shamsulhaq123, souilos, turvy_fuzz
12.4743 USDC - $12.47
Issue | Contexts | Estimated Gas Saved | |
---|---|---|---|
GAS‑1 | Consider activating via-ir for deploying | 1 | - |
GAS‑2 | Use assembly to emit events | 53 | 2014 |
GAS‑3 | Use assembly to write address storage values | 4 | - |
GAS‑4 | Structs can be packed into fewer storage slots by editing time variables | 1 | 2000 |
GAS‑5 | State variables can be packed into fewer storage slots | 3 | 6000 |
GAS‑6 | Use solidity version 0.8.20 to gain some gas boost | 21 | 1848 |
Total: 83 contexts over 6 issues
via-ir
for deployingThe IR-based code generator was introduced with an aim to not only allow code generation to be more transparent and auditable but also to enable more powerful optimization passes that span across functions.
You can enable it on the command line using --via-ir
or with the option {"viaIR": true}
.
This will take longer to compile, but you can just simple test it before deploying and if you got a better benchmark then you can add --via-ir to your deploy command
More on: https://docs.soliditylang.org/en/v0.8.17/ir-breaking-changes.html
We can use assembly to emit events efficiently by utilizing scratch space
and the free memory pointer
. This will allow us to potentially avoid memory expansion costs.
Note: In order to do this optimization safely, we will need to cache and restore the free memory pointer.
For example, for a generic emit
event for eventSentAmountExample
:
// uint256 id, uint256 value, uint256 amount emit eventSentAmountExample(id, value, amount);
We can use the following assembly emit events:
assembly { let memptr := mload(0x40) mstore(0x00, calldataload(0x44)) mstore(0x20, calldataload(0xa4)) mstore(0x40, amount) log1( 0x00, 0x60, // keccak256("eventSentAmountExample(uint256,uint256,uint256)") 0xa622cf392588fbf2cd020ff96b2f4ebd9c76d7a4bc7f3e6b2f18012312e76bc3 ) mstore(0x40, memptr) }
129: emit SafeCollateralRatioChanged(pool, newRatio);
139: emit ProtocolRewardsPoolChanged(addr, block.timestamp);
149: emit EUSDMiningIncentivesChanged(addr, block.timestamp);
189: emit RedemptionFeeChanged(newFee);
205: emit SafeCollateralRatioChanged(pool, newRatio);
216: emit BorrowApyChanged(pool, newApy);
227: emit KeeperRatioChanged(pool, newRatio);
238: emit tokenMinerChanges(_contracts[i], _bools[i]);
255: emit FlashloanFeeUpdated(fee);
271: emit RedemptionProvider(msg.sender, _bool);
198: emit ClaimReward(msg.sender, reward, block.timestamp);
222: emit ClaimedOtherEarnings(msg.sender, user, reward, biddingFee, useEUSD, block.timestamp);
241: emit NotifyRewardChanged(amount, block.timestamp);
76: emit StakeLBR(msg.sender, amount, block.timestamp);
97: emit UnstakeLBR(msg.sender, amount, block.timestamp);
106: emit WithdrawLBR(user, amount, block.timestamp);
146: emit Restake(msg.sender, amount, block.timestamp);
203: emit ClaimReward(msg.sender, EUSD.getMintedEUSDByShares(eUSDShare), address(peUSD), reward - eUSDShare, block.timestamp); 211: emit ClaimReward(msg.sender, EUSD.getMintedEUSDByShares(eUSDShare), address(token), reward - eUSDShare, block.timestamp); 214: emit ClaimReward(msg.sender, EUSD.getMintedEUSDByShares(eUSDShare), address(0), 0, block.timestamp);
89: emit StakeToken(msg.sender, _amount, block.timestamp);
98: emit WithdrawToken(msg.sender, _amount, block.timestamp);
116: emit ClaimReward(msg.sender, reward, block.timestamp);
144: emit NotifyRewardChanged(_amount, block.timestamp);
38: emit DepositEther(msg.sender, address(collateralAsset), msg.value,balance - preBalance, block.timestamp);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraRETHVault.sol#L38
51: emit DepositEther(msg.sender, address(collateralAsset), msg.value, msg.value, block.timestamp);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraStETHVault.sol#L51
83: emit FeeDistribution(address(configurator), income, block.timestamp); 89: emit FeeDistribution(address(configurator), payAmount, block.timestamp); 94: emit LSDValueCaptured(realAmount, payAmount, getDutchAuctionDiscountPrice(), block.timestamp);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraStETHVault.sol#L83
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraStETHVault.sol#L89
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraStETHVault.sol#L94
31: emit DepositEther(msg.sender, address(collateralAsset), msg.value,balance - preBalance, block.timestamp);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraWbETHVault.sol#L31
45: emit DepositEther(msg.sender, address(collateralAsset), msg.value,wstETHAmount, block.timestamp);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraWstETHVault.sol#L45
85: emit DepositAsset(msg.sender, address(collateralAsset), assetAmount, block.timestamp);
111: emit WithdrawAsset(msg.sender, address(collateralAsset), onBehalfOf, withdrawal, block.timestamp);
175: emit LiquidationRecord(provider, msg.sender, onBehalfOf, eusdAmount, reducedAsset, reward2keeper, false, block.timestamp);
210: emit LiquidationRecord(provider, msg.sender, onBehalfOf, eusdAmount, assetAmount, reward2keeper, true, block.timestamp);
243: emit RigidRedemption(msg.sender, provider, eusdAmount, collateralAmount, block.timestamp);
268: emit Mint(msg.sender, _onBehalfOf, _mintAmount, block.timestamp);
285: emit Burn(_provider, _onBehalfOf, amount, block.timestamp);
69: emit DepositAsset(msg.sender, address(collateralAsset), assetAmount, block.timestamp);
145: emit LiquidationRecord(provider, msg.sender, onBehalfOf, peusdAmount, reducedAsset, reward2keeper, false, block.timestamp);
167: emit RigidRedemption(msg.sender, provider, peusdAmount, collateralAmount, block.timestamp);
184: emit Mint(_provider, _onBehalfOf, _mintAmount, block.timestamp);
209: emit Burn(_provider, _onBehalfOf, amount, block.timestamp);
219: emit WithdrawAsset(_provider, address(collateralAsset), _onBehalfOf, _amount, block.timestamp);
337: emit TransferShares(owner, _recipient, _sharesAmount); 339: emit Transfer(owner, _recipient, tokensAmount);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/EUSD.sol#L337
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/EUSD.sol#L339
351: emit Transfer(_sender, _recipient, _amount); 352: emit TransferShares(_sender, _recipient, _sharesToTransfer);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/EUSD.sol#L351-L352
371: emit Approval(_owner, _spender, _amount);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/EUSD.sol#L371
427: emit Transfer(address(0), _recipient, _mintAmount);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/EUSD.sol#L427
446: emit Transfer(_account, address(0), _burnAmount);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/EUSD.sol#L446
477: emit SharesBurnt(_account, preRebaseTokenAmount, postRebaseTokenAmount, _sharesAmount);
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/EUSD.sol#L477
</details>138: emit Flashloaned(receiver, eusdAmount, burnShare);
assembly
to write address storage values421: _totalShares = newTotalShares;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/EUSD.sol#L421
471: _totalShares = newTotalShares;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/EUSD.sol#L471
</details>The following structs contain time variables that are unlikely to ever reach max uint256
then it is possible to set them as uint32
, saving gas slots. Max value of uint32
is equal to Year 2016, which is more than enough.
18: struct LockStatus { uint256 unlockTime; uint256 duration; uint256 miningBoost; }
Can save 1 storage slot by changing to:
18: struct LockStatus { uint128 unlockTime; uint128 duration; uint256 miningBoost; }
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/miner/esLBRBoost.sol#L18
</details>If variables occupying the same slot are both written the same function or by the constructor, avoids a separate Gsset (20000 gas). Reads of the variables can also be cheaper
37: contract Configurator { ... uint256 public flashloanFee = 500; // Limiting the maximum percentage of eUSD that can be cross-chain transferred to L2 in relation to the total supply. uint256 maxStableRatio = 5_000; ...
Can save 1 storage slot by changing to:
37: contract Configurator { ... uint128 public flashloanFee = 500; // Limiting the maximum percentage of eUSD that can be cross-chain transferred to L2 in relation to the total supply. uint128 maxStableRatio = 5_000; ...
27: contract EUSDMiningIncentives is Ownable { ... // Duration of rewards to be paid out (in seconds) uint256 public duration = 2_592_000; // Timestamp of when the rewards finish uint256 public finishAt; // Minimum of last updated time and reward finish time uint256 public updatedAt;
Can save 2 storage slots by changing to:
</details>27: contract EUSDMiningIncentives is Ownable { ... // Duration of rewards to be paid out (in seconds) uint64 public duration = 2_592_000; // Timestamp of when the rewards finish uint64 public finishAt; // Minimum of last updated time and reward finish time uint64 public updatedAt;
Upgrade to the latest solidity version 0.8.20 to get additional gas savings. See latest release for reference: https://blog.soliditylang.org/2023/05/10/solidity-0.8.20-release-announcement/
pragma solidity ^0.8.17;
pragma solidity ^0.8.17;
pragma solidity ^0.8.17;
pragma solidity ^0.8.17;
pragma solidity ^0.8.17;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/miner/esLBRBoost.sol#L3
pragma solidity ^0.8.17;
pragma solidity ^0.8.17;
pragma solidity ^0.8;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/miner/stakerewardV2pool.sol#L2
pragma solidity ^0.8.17;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraRETHVault.sol#L3
pragma solidity ^0.8.17;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraStETHVault.sol#L3
pragma solidity ^0.8.17;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraWbETHVault.sol#L3
pragma solidity ^0.8.17;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/pools/LybraWstETHVault.sol#L3
pragma solidity ^0.8.17;
pragma solidity ^0.8.17;
pragma solidity ^0.8.17;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/Proxy/LybraProxy.sol#L3
pragma solidity ^0.8.17;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/Proxy/LybraProxyAdmin.sol#L3
pragma solidity ^0.8.17;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/esLBR.sol#L3
pragma solidity ^0.8.17;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/EUSD.sol#L3
pragma solidity ^0.8.17;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/LBR.sol#L3
pragma solidity ^0.8.17;
https://github.com/code-423n4/2023-06-lybra/tree/main/contracts/lybra/token/PeUSD.sol#L3
</details>pragma solidity ^0.8.17;
#0 - c4-pre-sort
2023-07-27T21:31:31Z
JeffCX marked the issue as low quality report
#1 - c4-judge
2023-07-27T23:39:46Z
0xean marked the issue as grade-c
#2 - c4-judge
2023-07-29T22:27:58Z
0xean marked the issue as grade-b