Platform: Code4rena
Start Date: 11/11/2022
Pot Size: $90,500 USDC
Total HM: 52
Participants: 92
Period: 7 days
Judge: LSDan
Total Solo HM: 20
Id: 182
League: ETH
Rank: 15/92
Findings: 2
Award: $1,550.20
š Selected for report: 1
š Solo Findings: 0
š Selected for report: 0xSmartContract
Also found by: 0x4non, 0xNazgul, 0xRoxas, 0xdeadbeef0x, 0xmuxyz, 9svR6w, Awesome, Aymen0909, B2, Bnke0x0, CloudX, Deivitto, Diana, Franfran, IllIllI, Josiah, RaymondFam, ReyAdmirado, Rolezn, Sathish9098, Secureverse, SmartSek, Trust, Udsen, a12jmx, aphak5010, brgltd, bulej93, c3phas, ch0bu, chaduke, chrisdior4, clems4ever, cryptostellar5, datapunk, delfin454000, fs0c, gogo, gz627, hl_, immeas, joestakey, lukris02, martin, nogo, oyc_109, pashov, pavankv, peanuts, pedr02b2, rbserver, rotcivegaf, sahar, sakman, shark, tnevler, trustindistrust, zaskoh, zgo
475.5642 USDC - $475.56
| Issue | Instances | |
|---|---|---|
| [Lā01] | Unused/empty receive()/fallback() function | 2 |
| [Lā02] | Missing checks for address(0x0) when assigning values to address state variables | 3 |
| [Lā03] | Open TODOs | 1 |
Total: 6 instances over 3 issues
| Issue | Instances | |
|---|---|---|
| [Nā01] | Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions | 1 |
| [Nā02] | Duplicate import statements | 1 |
| [Nā03] | The nonReentrant modifier should occur before all other modifiers | 3 |
| [Nā04] | public functions not called by the contract should be declared external instead | 10 |
| [Nā05] | 2**<n> - 1 should be re-written as type(uint<n>).max | 1 |
| [Nā06] | constants should be defined rather than using magic numbers | 39 |
| [Nā07] | Missing event and or timelock for critical parameter change | 1 |
| [Nā08] | Constant redefined elsewhere | 2 |
| [Nā09] | Inconsistent spacing in comments | 1 |
| [Nā10] | Lines are too long | 5 |
| [Nā11] | Variable names that consist of all capital letters should be reserved for constant/immutable variables | 1 |
| [Nā12] | Non-library/interface files should use fixed compiler versions, not floating ones | 16 |
| [Nā13] | File is missing NatSpec | 4 |
| [Nā14] | NatSpec is incomplete | 20 |
| [Nā15] | Event is missing indexed fields | 35 |
| [Nā16] | Not using the named return variables anywhere in the function is confusing | 2 |
| [Nā17] | Duplicated require()/revert() checks should be refactored to a modifier or function | 19 |
Total: 161 instances over 17 issues
receive()/fallback() functionIf the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth))). Having no access control on the function means that someone may send Ether to the contract, and have no way to get anything back out, which is a loss of funds
There are 2 instances of this issue:
File: contracts/liquid-staking/LiquidStakingManager.sol 620: receive() external payable {}
File: contracts/liquid-staking/SyndicateRewardsProcessor.sol 98: receive() external payable {}
address(0x0) when assigning values to address state variablesThere are 3 instances of this issue:
File: contracts/liquid-staking/GiantLP.sol 25: pool = _pool;
File: contracts/liquid-staking/LPToken.sol 38: deployer = _deployer;
File: contracts/syndicate/SyndicateFactory.sol 17: syndicateImplementation = _syndicateImpl;
Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment
There is 1 instance of this issue:
File: contracts/syndicate/Syndicate.sol 195: // todo - check else case for any ETH lost
__gap[50] storage variable to allow for new storage variables in later versionsSee this link for a description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.
There is 1 instance of this issue:
File: contracts/liquid-staking/LPToken.sol 11: contract LPToken is ILPTokenInit, ILiquidStakingManagerChildContract, Initializable, ERC20PermitUpgradeable {
There is 1 instance of this issue:
File: contracts/liquid-staking/GiantMevAndFeesPool.sol 11: import { LPToken } from "./LPToken.sol";
nonReentrant modifier should occur before all other modifiersThis is a best-practice to protect against reentrancy in other modifiers
There are 3 instances of this issue:
File: contracts/liquid-staking/SavETHVault.sol 203: ) public onlyManager nonReentrant returns (uint256) {
File: contracts/liquid-staking/StakingFundsVault.sol 239: function withdrawETH(address _wallet, uint256 _amount) public onlyManager nonReentrant returns (uint256) { 259: ) external onlyManager nonReentrant {
public functions not called by the contract should be declared external insteadContracts are allowed to override their parents' functions and change the visibility from external to public.
There are 10 instances of this issue:
File: contracts/liquid-staking/GiantPoolBase.sol 34: function depositETH(uint256 _amount) public payable {
File: contracts/liquid-staking/GiantSavETHVaultPool.sol 29 function batchDepositETHForStaking( 30 address[] calldata _savETHVaults, 31 uint256[] calldata _ETHTransactionAmounts, 32 bytes[][] calldata _blsPublicKeys, 33: uint256[][] calldata _stakeAmounts
File: contracts/liquid-staking/LSDNFactory.sol 73 function deployNewLiquidStakingDerivativeNetwork( 74 address _dao, 75 uint256 _optionalCommission, 76 bool _deployOptionalHouseGatekeeper, 77 string calldata _stakehouseTicker 78: ) public returns (address) {
File: contracts/liquid-staking/SavETHVault.sol 83: function depositETHForStaking(bytes calldata _blsPublicKeyOfKnot, uint256 _amount) public payable returns (uint256) { 200 function withdrawETHForStaking( 201 address _smartWallet, 202 uint256 _amount 203: ) public onlyManager nonReentrant returns (uint256) { 223: function isBLSPublicKeyBanned(bytes calldata _blsPublicKeyOfKnot) public view returns (bool) {
File: contracts/liquid-staking/StakingFundsVault.sol 113: function depositETHForStaking(bytes calldata _blsPublicKeyOfKnot, uint256 _amount) public payable returns (uint256) { 239: function withdrawETH(address _wallet, uint256 _amount) public onlyManager nonReentrant returns (uint256) {
File: contracts/syndicate/Syndicate.sol 285: function claimAsStaker(address _recipient, bytes[] calldata _blsPubKeys) public nonReentrant { 458: function calculateNewAccumulatedETHPerCollateralizedSharePerKnot() public view returns (uint256) {
2**<n> - 1 should be re-written as type(uint<n>).maxEarlier versions of solidity can use uint<n>(-1) instead. Expressions not including the - 1 can often be re-written to accomodate the change (e.g. by using a > rather than a >=, which will also save some gas)
There is 1 instance of this issue:
File: contracts/liquid-staking/LiquidStakingManager.sol 861: sETH.approve(syndicate, (2 ** 256) - 1);
constants should be defined rather than using magic numbersEven assembly can benefit from using readable constants instead of hex/numeric literals
There are 39 instances of this issue:
File: contracts/liquid-staking/ETHPoolLPFactory.sol /// @audit 30 82: require(_oldLPToken.lastInteractedTimestamp(msg.sender) + 30 minutes < block.timestamp, "Liquidity is still fresh"); /// @audit 24 83: require(_amount + _newLPToken.totalSupply() <= 24 ether, "Not enough mintable tokens"); /// @audit 48 88: require(blsPublicKeyOfPreviousKnot.length == 48, "Incorrect BLS public key"); /// @audit 48 89: require(blsPublicKeyOfNewKnot.length == 48, "Incorrect BLS public key"); /// @audit 48 112: require(_blsPublicKeyOfKnot.length == 48, "Invalid BLS public key");
File: contracts/liquid-staking/GiantMevAndFeesPool.sol /// @audit 0.5 116: require(lpTokenETH.balanceOf(msg.sender) >= 0.5 ether, "No common interest");
File: contracts/liquid-staking/GiantSavETHVaultPool.sol /// @audit 0.5 127: require(lpTokenETH.balanceOf(msg.sender) >= 0.5 ether, "No common interest");
File: contracts/liquid-staking/LiquidStakingManager.sol /// @audit 3 256: require(bytes(_newTicker).length >= 3, "String must be 3-5 characters long"); /// @audit 5 257: require(bytes(_newTicker).length <= 5, "String must be 3-5 characters long"); /// @audit 4 333: require(associatedSmartWallet.balance >= 4 ether, "Insufficient balance"); /// @audit 4 343: 4 ether /// @audit 4 432: require(msg.value == len * 4 ether, "Insufficient ether provided"); /// @audit 3 653: require(bytes(_stakehouseTicker).length >= 3, "String must be 3-5 characters long"); /// @audit 5 654: require(bytes(_stakehouseTicker).length <= 5, "String must be 3-5 characters long"); /// @audit 24 740: savETHVault.withdrawETHForStaking(smartWallet, 24 ether); /// @audit 4 743: stakingFundsVault.withdrawETH(smartWallet, 4 ether); /// @audit 32 757: 32 ether /// @audit 256 861: sETH.approve(syndicate, (2 ** 256) - 1); /// @audit 12 873: uint256 stakeAmount = 12 ether; /// @audit 4 927: require(associatedSmartWallet.balance >= 4 ether, "Smart wallet balance must be at least 4 ether"); /// @audit 4 931: require(stakingFundsLP.totalSupply() == 4 ether, "DAO staking funds vault balance must be at least 4 ether"); /// @audit 24 935: require(savETHVaultLP.totalSupply() == 24 ether, "KNOT must have 24 ETH in savETH vault");
File: contracts/liquid-staking/SavETHVault.sol /// @audit 30 141: bool isStaleLiquidity = _lpToken.lastInteractedTimestamp(msg.sender) + 30 minutes < block.timestamp; /// @audit 24 161: require(dETHBalance >= 24 ether, "Nothing to withdraw"); /// @audit 24 174: redemptionValue = (dETHDetails.savETHBalance * _amount) / 24 ether; /// @audit 24 204: require(_amount >= 24 ether, "Amount cannot be less than 24 ether"); /// @audit 24 244: maxStakingAmountPerValidator = 24 ether;
File: contracts/liquid-staking/StakingFundsVault.sol /// @audit 4 58: totalShares += 4 ether; /// @audit 30 184: require(_lpToken.lastInteractedTimestamp(msg.sender) + 30 minutes < block.timestamp, "Too new"); /// @audit 30 230: require(token.lastInteractedTimestamp(msg.sender) + 30 minutes < block.timestamp, "Last transfer too recent"); /// @audit 4 240: require(_amount >= 4 ether, "Amount cannot be less than 4 ether"); /// @audit 4 380: maxStakingAmountPerValidator = 4 ether;
File: contracts/syndicate/Syndicate.sol /// @audit 12 221: if (totalStaked == 12 ether) revert KnotIsFullyStakedWithFreeFloatingSlotTokens(); /// @audit 12 223: if (_sETHAmount + totalStaked > 12 ether) revert InvalidStakeAmount(); /// @audit 4 429: if (currentSlashedAmount < 4 ether) { /// @audit 4 431: balance * unprocessedForKnot / (4 ether - currentSlashedAmount); /// @audit 4 504: if (currentSlashedAmount < 4 ether) { /// @audit 4 522: balance * unprocessedETHForCurrentKnot / (4 ether - currentSlashedAmount); /// @audit 4 546: return (_ethSinceLastUpdate * PRECISION) / (numberOfRegisteredKnots * 4 ether);
Events help non-contract tools to track changes, and events prevent users from being surprised by changes
There is 1 instance of this issue:
File: contracts/syndicate/Syndicate.sol 168 function updatePriorityStakingBlock(uint256 _endBlock) external onlyOwner { 169 updateAccruedETHPerShares(); 170 priorityStakingEndBlock = _endBlock; 171: }
Consider defining in only one contract so that values cannot become out of sync when only one location is updated. A cheap way to store constants in a single location is to create an internal constant in a library. If the variable is a local cache of another contract's value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don't get out of sync.
There are 2 instances of this issue:
File: contracts/liquid-staking/GiantPoolBase.sol /// @audit seen in contracts/liquid-staking/ETHPoolLPFactory.sol 22: uint256 public constant MIN_STAKING_AMOUNT = 0.001 ether;
File: contracts/syndicate/Syndicate.sol /// @audit seen in contracts/liquid-staking/SyndicateRewardsProcessor.sol 66: uint256 public constant PRECISION = 1e24;
Some lines use // x and some use //x. The instances below point out the usages that don't follow the majority, within each file
There is 1 instance of this issue:
File: contracts/liquid-staking/LiquidStakingManager.sol 59: ///@notice signalize removal of representative from smart wallet
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
There are 5 instances of this issue:
File: contracts/liquid-staking/LiquidStakingManager.sol 222: /// @notice In preparation of a rage quit, restore sETH to a smart wallet which are recoverable with the execution methods in the event this step does not go to plan
File: contracts/syndicate/Syndicate.sol 116: /// @notice Whether a BLS public key, that has been previously registered, is no longer part of the syndicate and its shares (free floating or SLOT) cannot earn any more rewards 119: /// @notice Once a BLS public key is no longer part of the syndicate, the accumulated ETH per free floating SLOT share is snapshotted so historical earnings can be drawn down correctly 398: /// @notice Preview the amount of unclaimed ETH available for a collatearlized SLOT staker against a KNOT which factors in unprocessed rewards from new ETH sent to contract 490: /// Given an amount of ETH allocated to the collateralized SLOT owners of a KNOT, distribute this amongs the current set of collateralized owners (a dynamic set of addresses and balances)
constant/immutable variablesIf the variable needs to be different based on which class it comes from, a view/pure function should be used instead (e.g. like this).
There is 1 instance of this issue:
File: contracts/liquid-staking/LiquidStakingManager.sol 158: uint256 public MODULO = 100_00000;
There are 16 instances of this issue:
File: contracts/liquid-staking/GiantLP.sol 1: pragma solidity ^0.8.13;
File: contracts/liquid-staking/GiantMevAndFeesPool.sol 1: pragma solidity ^0.8.13;
File: contracts/liquid-staking/GiantPoolBase.sol 1: pragma solidity ^0.8.13;
File: contracts/liquid-staking/GiantSavETHVaultPool.sol 1: pragma solidity ^0.8.13;
File: contracts/liquid-staking/LiquidStakingManager.sol 3: pragma solidity ^0.8.13;
File: contracts/liquid-staking/LPTokenFactory.sol 1: pragma solidity ^0.8.13;
File: contracts/liquid-staking/LPToken.sol 1: pragma solidity ^0.8.13;
File: contracts/liquid-staking/LSDNFactory.sol 1: pragma solidity ^0.8.13;
File: contracts/liquid-staking/OptionalGatekeeperFactory.sol 3: pragma solidity ^0.8.13;
File: contracts/liquid-staking/OptionalHouseGatekeeper.sol 1: pragma solidity ^0.8.13;
File: contracts/liquid-staking/SavETHVaultDeployer.sol 1: pragma solidity ^0.8.13;
File: contracts/liquid-staking/SavETHVault.sol 3: pragma solidity ^0.8.13;
File: contracts/liquid-staking/StakingFundsVaultDeployer.sol 3: pragma solidity ^0.8.13;
File: contracts/liquid-staking/StakingFundsVault.sol 3: pragma solidity ^0.8.13;
File: contracts/smart-wallet/OwnableSmartWalletFactory.sol 3: pragma solidity ^0.8.13;
File: contracts/smart-wallet/OwnableSmartWallet.sol 3: pragma solidity ^0.8.13;
There are 4 instances of this issue:
File: contracts/liquid-staking/OptionalGatekeeperFactory.sol
File: contracts/liquid-staking/SavETHVaultDeployer.sol
File: contracts/liquid-staking/StakingFundsVaultDeployer.sol
File: contracts/syndicate/SyndicateErrors.sol
There are 20 instances of this issue:
File: contracts/liquid-staking/GiantMevAndFeesPool.sol /// @audit Missing: '@param _newLPTokens' 100 /// @notice Any ETH supplied to a BLS key registered with a liquid staking network can be rotated to another key if it never gets staked 101 /// @param _stakingFundsVaults List of staking funds vaults this contract will contact 102 /// @param _oldLPTokens List of savETH vault LP tokens that the vault has 103 /// @param _oldLPTokens List of new savETH vault LP tokens that the vault wants to receive in exchange for moving ETH to a new KNOT 104 /// @param _amounts Amount of old being swapped for new per LP token 105 function batchRotateLPTokens( 106 address[] calldata _stakingFundsVaults, 107 LPToken[][] calldata _oldLPTokens, 108 LPToken[][] calldata _newLPTokens, 109: uint256[][] calldata _amounts
File: contracts/liquid-staking/GiantSavETHVaultPool.sol /// @audit Missing: '@param _newLPTokens' 111 /// @notice Any ETH supplied to a BLS key registered with a liquid staking network can be rotated to another key if it never gets staked 112 /// @param _savETHVaults List of savETH vaults this contract will contact 113 /// @param _oldLPTokens List of savETH vault LP tokens that the vault has 114 /// @param _oldLPTokens List of new savETH vault LP tokens that the vault wants to receive in exchange for moving ETH to a new KNOT 115 /// @param _amounts Amount of old being swapped for new per LP token 116 function batchRotateLPTokens( 117 address[] calldata _savETHVaults, 118 LPToken[][] calldata _oldLPTokens, 119 LPToken[][] calldata _newLPTokens, 120: uint256[][] calldata _amounts
File: contracts/liquid-staking/LiquidStakingManager.sol /// @audit Missing: '@return' 265 /// @notice function to change whether node runner whitelisting of node runners is required by the DAO 266 /// @param _changeWhitelist boolean value. true to enable and false to disable 267: function updateWhitelisting(bool _changeWhitelist) external onlyDAO returns (bool) { /// @audit Missing: '@param _recipient' 323 /// @notice Allow node runners to withdraw ETH from their smart wallet. ETH can only be withdrawn until the KNOT has not been staked. 324 /// @dev A banned node runner cannot withdraw ETH for the KNOT. 325 /// @param _blsPublicKeyOfKnot BLS public key of the KNOT for which the ETH needs to be withdrawn 326: function withdrawETHForKnot(address _recipient, bytes calldata _blsPublicKeyOfKnot) external {
File: contracts/liquid-staking/LPTokenFactory.sol /// @audit Missing: '@param _deployer' /// @audit Missing: '@param _transferHookProcessor' /// @audit Missing: '@return' 24 /// @notice Deploys a new LP token 25 /// @param _tokenSymbol Symbol of the LP token to be deployed 26 /// @param _tokenName Name of the LP token to be deployed 27 function deployLPToken( 28 address _deployer, 29 address _transferHookProcessor, 30 string calldata _tokenSymbol, 31 string calldata _tokenName 32: ) external returns (address) {
File: contracts/liquid-staking/LPToken.sol /// @audit Missing: '@param _tokenSymbol' /// @audit Missing: '@param _tokenName' 30 /// @param _deployer Address of the account deploying the LP token 31 /// @param _transferHookProcessor Optional contract account that can be notified about transfer hooks 32 function init( 33 address _deployer, 34 address _transferHookProcessor, 35 string calldata _tokenSymbol, 36 string calldata _tokenName 37: ) external override initializer {
File: contracts/liquid-staking/LSDNFactory.sol /// @audit Missing: '@param _optionalCommission' /// @audit Missing: '@param _deployOptionalHouseGatekeeper' /// @audit Missing: '@return' 70 /// @notice Deploys a new LSDN and the liquid staking manger required to manage the network 71 /// @param _dao Address of the entity that will govern the liquid staking network 72 /// @param _stakehouseTicker Liquid staking derivative network ticker (between 3-5 chars) 73 function deployNewLiquidStakingDerivativeNetwork( 74 address _dao, 75 uint256 _optionalCommission, 76 bool _deployOptionalHouseGatekeeper, 77 string calldata _stakehouseTicker 78: ) public returns (address) {
File: contracts/liquid-staking/StakingFundsVault.sol /// @audit Missing: '@param _lpTokenFactory' 45 /// @param _liquidStakingNetworkManager address of the liquid staking network manager 46: function init(address _liquidStakingNetworkManager, LPTokenFactory _lpTokenFactory) external virtual initializer { /// @audit Missing: '@return' 111 /// @param _blsPublicKeyOfKnot BLS public key of validator registered by a node runner 112 /// @param _amount Amount of ETH being staked 113: function depositETHForStaking(bytes calldata _blsPublicKeyOfKnot, uint256 _amount) public payable returns (uint256) { /// @audit Missing: '@param _recipient' 197 /// @notice Any LP tokens for BLS keys that have had their derivatives minted can claim ETH from the syndicate contract 198 /// @param _blsPubKeys List of BLS public keys being processed 199 function claimRewards( 200 address _recipient, 201 bytes[] calldata _blsPubKeys 202: ) external nonReentrant { /// @audit Missing: '@param _sETHRecipient' 252 /// @notice For any knots that are no longer part of syndicate facilitate unstaking so that knot can rage quit 253 /// @param _blsPublicKeys List of BLS public keys being processed (assuming DAO only has BLS pub keys from correct smart wallet) 254 /// @param _amounts Amounts of free floating sETH that will be unstaked 255 function unstakeSyndicateSharesForRageQuit( 256 address _sETHRecipient, 257 bytes[] calldata _blsPublicKeys, 258 uint256[] calldata _amounts 259: ) external onlyManager nonReentrant {
File: contracts/syndicate/Syndicate.sol /// @audit Missing: '@param _recipient' 289 /// @param _blsPubKeys List of BLS public keys that the caller has staked against 290 function claimAsCollateralizedSLOTOwner( 291 address _recipient, 292 bytes[] calldata _blsPubKeys 293: ) external nonReentrant { /// @audit Missing: '@return' 357 /// @param _blsPubKey BLS public key of the KNOT that is registered with the syndicate 358 /// @param _user The address of a user that has staked sETH against the BLS public key 359: function calculateUnclaimedFreeFloatingETHShare(bytes memory _blsPubKey, address _user) public view returns (uint256) { /// @audit Missing: '@return' 382 /// @param _blsPubKey BLS public key of the KNOT that is registered with the syndicate 383 /// @param _staker The address of a user that has staked sETH against the BLS public key 384 function previewUnclaimedETHAsFreeFloatingStaker( 385 address _staker, 386 bytes calldata _blsPubKey 387: ) external view returns (uint256) { /// @audit Missing: '@return' 399 /// @param _staker Address of a collateralized SLOT owner for a KNOT 400 /// @param _blsPubKey BLS public key of the KNOT that is registered with the syndicate 401 function previewUnclaimedETHAsCollateralizedSlotOwner( 402 address _staker, 403 bytes calldata _blsPubKey 404: ) external view returns (uint256) {
indexed fieldsIndex event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.
There are 35 instances of this issue:
File: contracts/liquid-staking/ETHPoolLPFactory.sol 16: event ETHWithdrawnByDepositor(address depositor, uint256 amount); 19: event LPTokenBurnt(bytes blsPublicKeyOfKnot, address token, address depositor, uint256 amount); 22: event NewLPTokenIssued(bytes blsPublicKeyOfKnot, address token, address firstDepositor, uint256 amount); 25: event LPTokenMinted(bytes blsPublicKeyOfKnot, address token, address depositor, uint256 amount);
File: contracts/liquid-staking/GiantPoolBase.sol 13: event ETHDeposited(address indexed sender, uint256 amount); 16: event LPBurnedForETH(address indexed sender, uint256 amount); 19: event LPSwappedForVaultLP(address indexed vaultLPToken, address indexed sender, uint256 amount);
File: contracts/liquid-staking/GiantSavETHVaultPool.sol 16: event LPBurnedForDETH(address indexed savETHVaultLPToken, address indexed sender, uint256 amount);
File: contracts/liquid-staking/LiquidStakingManager.sol 36: event WhitelistingStatusChanged(address indexed dao, bool updatedStatus); 39: event NodeRunnerWhitelistingStatusChanged(address indexed nodeRunner, bool updatedStatus); 48: event WalletCredited(address indexed smartWallet, uint256 amount); 51: event KnotStaked(bytes _blsPublicKeyOfKnot, address indexed trigerringAddress); 54: event StakehouseCreated(string stakehouseTicker, address indexed stakehouse); 57: event StakehouseJoined(bytes blsPubKey); 63: event DormantRepresentative(address indexed associatedSmartWallet, address representative); 66: event ETHWithdrawnFromSmartWallet(address indexed associatedSmartWallet, bytes blsPublicKeyOfKnot, address nodeRunner); 69: event NetworkTickerUpdated(string newTicker); 84: event DAOCommissionUpdated(uint256 old, uint256 newCommission); 87: event NewLSDValidatorRegistered(address indexed nodeRunner, bytes blsPublicKey);
File: contracts/liquid-staking/SavETHVault.sol 19: event DETHRedeemed(address depositor, uint256 amount); 22: event ETHWithdrawnForStaking(address withdrawalAddress, address liquidStakingManager, uint256 amount); 121: event CurrentStamp(uint256 stamp, uint256 last, bool isConditionTrue);
File: contracts/liquid-staking/StakingFundsVault.sol 25: event ETHDeposited(address sender, uint256 amount); 28: event ETHWithdrawn(address receiver, address admin, uint256 amount); 31: event ERC20Recovered(address admin, address recipient, uint256 amount); 34: event WETHUnwrapped(address admin, uint256 amount);
File: contracts/liquid-staking/SyndicateRewardsProcessor.sol 9: event ETHReceived(uint256 amount); 12: event ETHDistributed(address indexed user, address indexed recipient, uint256 amount);
File: contracts/syndicate/Syndicate.sol 42: event UpdateAccruedETH(uint256 unprocessed); 45: event CollateralizedSLOTReCalibrated(bytes BLSPubKey); 48: event KNOTRegistered(bytes BLSPubKey); 51: event KnotDeRegistered(bytes BLSPubKey); 57: event Staked(bytes BLSPubKey, uint256 amount); 60: event UnStaked(bytes BLSPubKey, uint256 amount); 63: event ETHClaimed(bytes BLSPubKey, address indexed user, address recipient, uint256 claim, bool indexed isCollateralizedClaim);
Consider changing the variable to be an unnamed one
There are 2 instances of this issue:
File: contracts/liquid-staking/LiquidStakingManager.sol /// @audit _nodeRunner /// @audit _dao 912: function _calculateCommission(uint256 _received) internal virtual view returns (uint256 _nodeRunner, uint256 _dao) {
require()/revert() checks should be refactored to a modifier or functionThe compiler will inline the function, which will avoid JUMP instructions usually associated with functions
There are 19 instances of this issue:
File: contracts/liquid-staking/GiantLP.sol 35: require(msg.sender == pool, "Only pool");
File: contracts/liquid-staking/GiantMevAndFeesPool.sol 171: require(msg.sender == address(lpTokenETH), "Caller is not giant LP");
File: contracts/liquid-staking/GiantPoolBase.sol 94: require(_amount >= MIN_STAKING_AMOUNT, "Invalid amount");
File: contracts/liquid-staking/GiantSavETHVaultPool.sol 143: require(numOfVaults > 0, "Empty arrays"); 144: require(numOfVaults == _lpTokens.length, "Inconsistent arrays"); 145: require(numOfVaults == _amounts.length, "Inconsistent arrays");
File: contracts/liquid-staking/LiquidStakingManager.sol 676: require(_nodeRunner != address(0), "Zero address"); 309: require(_newRepresentative != address(0), "Zero address"); 435: require(isNodeRunnerBanned(msg.sender) == false, "Node runner is banned from LSD network"); 312: require(smartWallet != address(0), "No smart wallet"); 313: require(stakedKnotsOfSmartWallet[smartWallet] == 0, "Not all KNOTs are minted"); 314: require(smartWalletRepresentative[smartWallet] != _newRepresentative, "Invalid rotation to same EOA"); 385: require(_recipient != address(0), "Zero address"); 414: require(transferResult, "Failed to transfer");
File: contracts/liquid-staking/SavETHVault.sol 114: require(numOfTokens > 0, "Empty arrays"); 210: require(result, "Transfer failed");
File: contracts/liquid-staking/StakingFundsVault.sol 164: require(numOfTokens > 0, "Empty arrays"); 165: require(numOfTokens == _amounts.length, "Inconsistent array length"); 245: require(result, "Transfer failed");
These findings are excluded from awards calculations because there are publicly-available automated tools that find them. The valid ones appear here for completeness
| Issue | Instances | |
|---|---|---|
| [Nā01] | Return values of approve() not checked | 1 |
Total: 1 instances over 1 issues
approve() not checkedNot all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything
There is 1 instance of this issue:
File: contracts/liquid-staking/LiquidStakingManager.sol /// @audit (valid but excluded finding) 861: sETH.approve(syndicate, (2 ** 256) - 1);
#0 - c4-judge
2022-12-02T20:03:46Z
dmvt marked the issue as grade-a
š Selected for report: IllIllI
Also found by: 0xSmartContract, Awesome, Aymen0909, CloudX, Deivitto, ReyAdmirado, Saintcode_, bharg4v, brgltd, btk, c3phas, chrisdior4, ignacio, imare, lukris02, skyle, tnevler
1074.637 USDC - $1,074.64
| Issue | Instances | Total Gas Saved | |
|---|---|---|---|
| [Gā01] | Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate | 1 | - |
| [Gā02] | State variables only set in the constructor should be declared immutable | 33 | 31201 |
| [Gā03] | Using calldata instead of memory for read-only arguments in external functions saves gas | 8 | 960 |
| [Gā04] | State variables should be cached in stack variables rather than re-reading them from storage | 16 | 1552 |
| [Gā05] | The result of function calls should be cached rather than re-calling the function | 1 | - |
| [Gā06] | <x> += <y> costs more gas than <x> = <x> + <y> for state variables | 16 | 1808 |
| [Gā07] | internal functions only called once can be inlined to save gas | 9 | 180 |
| [Gā08] | ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops | 38 | 2280 |
| [Gā09] | require()/revert() strings longer than 32 bytes cost extra gas | 39 | - |
| [Gā10] | Optimize names to save gas | 16 | 352 |
| [Gā11] | internal functions not called by the contract should be removed to save deployment gas | 2 | - |
| [Gā12] | Don't compare boolean expressions to boolean literals | 19 | 171 |
| [Gā13] | Ternary unnecessary | 1 | - |
| [Gā14] | Division by two should use bit shifting | 1 | 20 |
| [Gā15] | Stack variable used as a cheaper cache for a state variable is only used once | 1 | 3 |
| [Gā16] | Empty blocks should be removed or emit something | 1 | - |
| [Gā17] | Use custom errors rather than revert()/require() strings to save gas | 21 | - |
| [Gā18] | Functions guaranteed to revert when called by normal users can be marked payable | 20 | 420 |
Total: 243 instances over 18 issues with 38947 gas saved
Gas totals use lower bounds of ranges and count two iterations of each for-loop. All values above are runtime, not deployment, values; deployment values are listed in the individual issue descriptions. The table above as well as its gas numbers do not include any of the excluded findings.
address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriateSaves 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.
There is 1 instance of this issue:
File: contracts/liquid-staking/LiquidStakingManager.sol 134 /// @notice Node runner issued to Smart wallet. Smart wallet address <> Node runner address 135 mapping(address => address) public nodeRunnerOfSmartWallet; 136 137 /// @notice Track number of staked KNOTs of a smart wallet 138: mapping(address => uint256) public stakedKnotsOfSmartWallet;
immutableAvoids a Gsset (20000 gas) in the constructor, and replaces the first access in each transaction (Gcoldsload - 2100 gas) and each access thereafter (Gwarmacces - 100 gas) with a PUSH32 (3 gas).
While strings are not value types, and therefore cannot be immutable/constant if not hard-coded outside of the constructor, the same behavior can be achieved by making the current contract abstract with virtual functions for the string accessors, and having a child contract override the functions with the hard-coded implementation-specific values.
There are 33 instances of this issue:
File: contracts/liquid-staking/GiantLP.sol /// @audit pool (constructor) 25: pool = _pool; /// @audit pool (access) 30: require(msg.sender == pool, "Only pool"); /// @audit pool (access) 35: require(msg.sender == pool, "Only pool"); /// @audit transferHookProcessor (constructor) 26: transferHookProcessor = ITransferHookProcessor(_transferHookProcessor); /// @audit transferHookProcessor (access) /// @audit transferHookProcessor (access) 40: if (address(transferHookProcessor) != address(0)) ITransferHookProcessor(transferHookProcessor).beforeTokenTransfer(_from, _to, _amount); /// @audit transferHookProcessor (access) /// @audit transferHookProcessor (access) 46: if (address(transferHookProcessor) != address(0)) ITransferHookProcessor(transferHookProcessor).afterTokenTransfer(_from, _to, _amount);
File: contracts/liquid-staking/LPTokenFactory.sol /// @audit lpTokenImplementation (constructor) 21: lpTokenImplementation = _lpTokenImplementation; /// @audit lpTokenImplementation (access) 37: address newInstance = Clones.clone(lpTokenImplementation);
File: contracts/liquid-staking/LSDNFactory.sol /// @audit liquidStakingManagerImplementation (constructor) 60: liquidStakingManagerImplementation = _liquidStakingManagerImplementation; /// @audit liquidStakingManagerImplementation (access) 81: address newInstance = Clones.clone(liquidStakingManagerImplementation); /// @audit syndicateFactory (constructor) 61: syndicateFactory = _syndicateFactory; /// @audit syndicateFactory (access) 84: syndicateFactory, /// @audit smartWalletFactory (constructor) 63: smartWalletFactory = _smartWalletFactory; /// @audit smartWalletFactory (access) 85: smartWalletFactory, /// @audit brand (constructor) 64: brand = _brand; /// @audit brand (access) 87: brand, /// @audit savETHVaultDeployer (constructor) 65: savETHVaultDeployer = _savETHVaultDeployer; /// @audit savETHVaultDeployer (access) 88: savETHVaultDeployer, /// @audit stakingFundsVaultDeployer (constructor) 66: stakingFundsVaultDeployer = _stakingFundsVaultDeployer; /// @audit stakingFundsVaultDeployer (access) 89: stakingFundsVaultDeployer, /// @audit optionalGatekeeperDeployer (constructor) 67: optionalGatekeeperDeployer = _optionalGatekeeperDeployer; /// @audit optionalGatekeeperDeployer (access) 90: optionalGatekeeperDeployer,
File: contracts/liquid-staking/OptionalHouseGatekeeper.sol /// @audit liquidStakingManager (constructor) 15: liquidStakingManager = ILiquidStakingManager(_manager); /// @audit liquidStakingManager (access) 20: return liquidStakingManager.isBLSPublicKeyPartOfLSDNetwork(_blsPublicKeyOfKnot);
File: contracts/liquid-staking/SavETHVaultDeployer.sol /// @audit implementation (constructor) 15: implementation = address(new SavETHVault()); /// @audit implementation (access) 19: address newVault = Clones.clone(implementation);
File: contracts/liquid-staking/StakingFundsVaultDeployer.sol /// @audit implementation (constructor) 15: implementation = address(new StakingFundsVault()); /// @audit implementation (access) 19: address newVault = Clones.clone(implementation);
File: contracts/syndicate/SyndicateFactory.sol /// @audit syndicateImplementation (constructor) 17: syndicateImplementation = _syndicateImpl; /// @audit syndicateImplementation (access) 29: syndicateImplementation, /// @audit syndicateImplementation (access) 54: return Clones.predictDeterministicAddress(syndicateImplementation, salt);
diff --git a/contracts/liquid-staking/GiantLP.sol b/contracts/liquid-staking/GiantLP.sol index e6026f9..ba475cd 100644 --- a/contracts/liquid-staking/GiantLP.sol +++ b/contracts/liquid-staking/GiantLP.sol @@ -8,10 +8,10 @@ import { ITransferHookProcessor } from "../interfaces/ITransferHookProcessor.sol contract GiantLP is ERC20 { /// @notice Address of giant pool that deployed the giant LP token - address public pool; + address public immutable pool; /// @notice Optional address of contract that will process transfers of giant LP - ITransferHookProcessor public transferHookProcessor; + ITransferHookProcessor public immutable transferHookProcessor; /// @notice Last interacted timestamp for a given address mapping(address => uint256) public lastInteractedTimestamp; @@ -45,4 +45,4 @@ contract GiantLP is ERC20 { lastInteractedTimestamp[_to] = block.timestamp; if (address(transferHookProcessor) != address(0)) ITransferHookProcessor(transferHookProcessor).afterTokenTransfer(_from, _to, _amount); } -} \ No newline at end of file +} diff --git a/contracts/liquid-staking/LPTokenFactory.sol b/contracts/liquid-staking/LPTokenFactory.sol index 06d32b0..b810954 100644 --- a/contracts/liquid-staking/LPTokenFactory.sol +++ b/contracts/liquid-staking/LPTokenFactory.sol @@ -12,7 +12,7 @@ contract LPTokenFactory { event LPTokenDeployed(address indexed factoryCloneToken); /// @notice Address of LP token implementation that is cloned on each LP token - address public lpTokenImplementation; + address public immutable lpTokenImplementation; /// @param _lpTokenImplementation Address of LP token implementation that is cloned on each LP token deployment constructor(address _lpTokenImplementation) { @@ -46,4 +46,4 @@ contract LPTokenFactory { return newInstance; } -} \ No newline at end of file +} diff --git a/contracts/liquid-staking/LSDNFactory.sol b/contracts/liquid-staking/LSDNFactory.sol index 7007e1a..afd569d 100644 --- a/contracts/liquid-staking/LSDNFactory.sol +++ b/contracts/liquid-staking/LSDNFactory.sol @@ -12,28 +12,28 @@ contract LSDNFactory { event LSDNDeployed(address indexed LiquidStakingManager); /// @notice Address of the liquid staking manager implementation that is cloned on each deployment - address public liquidStakingManagerImplementation; + address public immutable liquidStakingManagerImplementation; /// @notice Address of the factory that will deploy a syndicate for the network after the first knot is created - address public syndicateFactory; + address public syndicateFactory; // left this one since it requires large test changes /// @notice Address of the factory for deploying LP tokens in exchange for ETH supplied to stake a KNOT address public lpTokenFactory; /// @notice Address of the factory for deploying smart wallets used by node runners during staking - address public smartWalletFactory; + address public immutable smartWalletFactory; /// @notice Address of brand NFT address public brand; /// @notice Address of the contract that can deploy new instances of SavETHVault - address public savETHVaultDeployer; + address public immutable savETHVaultDeployer; /// @notice Address of the contract that can deploy new instances of StakingFundsVault - address public stakingFundsVaultDeployer; + address public immutable stakingFundsVaultDeployer; /// @notice Address of the contract that can deploy new instances of optional gatekeepers for controlling which knots can join the LSDN house - address public optionalGatekeeperDeployer; + address public immutable optionalGatekeeperDeployer; /// @notice Establishes whether a given liquid staking manager address was deployed by this factory mapping(address => bool) public isLiquidStakingManager; @@ -100,4 +100,4 @@ contract LSDNFactory { return newInstance; } -} \ No newline at end of file +} diff --git a/contracts/liquid-staking/OptionalHouseGatekeeper.sol b/contracts/liquid-staking/OptionalHouseGatekeeper.sol index fabedeb..96d49f4 100644 --- a/contracts/liquid-staking/OptionalHouseGatekeeper.sol +++ b/contracts/liquid-staking/OptionalHouseGatekeeper.sol @@ -9,7 +9,7 @@ import { ILiquidStakingManager } from "../interfaces/ILiquidStakingManager.sol"; contract OptionalHouseGatekeeper is IGateKeeper { /// @notice Address of the core registry for the associated liquid staking network - ILiquidStakingManager public liquidStakingManager; + ILiquidStakingManager public immutable liquidStakingManager; constructor(address _manager) { liquidStakingManager = ILiquidStakingManager(_manager); @@ -19,4 +19,4 @@ contract OptionalHouseGatekeeper is IGateKeeper { function isMemberPermitted(bytes calldata _blsPublicKeyOfKnot) external override view returns (bool) { return liquidStakingManager.isBLSPublicKeyPartOfLSDNetwork(_blsPublicKeyOfKnot); } -} \ No newline at end of file +} diff --git a/contracts/liquid-staking/SavETHVaultDeployer.sol b/contracts/liquid-staking/SavETHVaultDeployer.sol index 59e897e..f17a9ef 100644 --- a/contracts/liquid-staking/SavETHVaultDeployer.sol +++ b/contracts/liquid-staking/SavETHVaultDeployer.sol @@ -10,7 +10,7 @@ contract SavETHVaultDeployer { event NewVaultDeployed(address indexed instance); - address implementation; + address immutable implementation; constructor() { implementation = address(new SavETHVault()); } @@ -23,4 +23,4 @@ contract SavETHVaultDeployer { return newVault; } -} \ No newline at end of file +} diff --git a/contracts/liquid-staking/StakingFundsVaultDeployer.sol b/contracts/liquid-staking/StakingFundsVaultDeployer.sol index 628e1bd..3e7fee2 100644 --- a/contracts/liquid-staking/StakingFundsVaultDeployer.sol +++ b/contracts/liquid-staking/StakingFundsVaultDeployer.sol @@ -10,7 +10,7 @@ contract StakingFundsVaultDeployer { event NewVaultDeployed(address indexed instance); - address implementation; + address immutable implementation; constructor() { implementation = address(new StakingFundsVault()); } @@ -23,4 +23,4 @@ contract StakingFundsVaultDeployer { return newVault; } -} \ No newline at end of file +} diff --git a/contracts/syndicate/SyndicateFactory.sol b/contracts/syndicate/SyndicateFactory.sol index a405392..cf6ffb4 100644 --- a/contracts/syndicate/SyndicateFactory.sol +++ b/contracts/syndicate/SyndicateFactory.sol @@ -10,7 +10,7 @@ import { ISyndicateInit } from "../interfaces/ISyndicateInit.sol"; contract SyndicateFactory is ISyndicateFactory { /// @notice Address of syndicate implementation that is cloned on each syndicate deployment - address public syndicateImplementation; + address public immutable syndicateImplementation; /// @param _syndicateImpl Address of syndicate implementation that is cloned on each syndicate deployment constructor(address _syndicateImpl) { @@ -62,4 +62,4 @@ contract SyndicateFactory is ISyndicateFactory { ) public override pure returns (bytes32) { return keccak256(abi.encode(_deployer, _contractOwner, _numberOfInitialKnots)); } -} \ No newline at end of file +}
Note that the numbers below are an underreporting of the gas changes due to this forge issue
diff --git a/tmp/gas_before b/tmp/gas_after index fba0986..4737598 100644 --- a/tmp/gas_before +++ b/tmp/gas_after @@ -6 +6 @@ -ā 783840 ā 4497 ā ā ā ā ā +ā 782932 ā 4746 ā ā ā ā ā @@ -12 +12 @@ -ā burn ā 3112 ā 3112 ā 3112 ā 3112 ā 1 ā +ā burn ā 2872 ā 2872 ā 2872 ā 2872 ā 1 ā @@ -16 +16 @@ -ā mint ā 95651 ā 100798 ā 100798 ā 105946 ā 2 ā +ā mint ā 91351 ā 96392 ā 96392 ā 101434 ā 2 ā @@ -25 +25 @@ -ā 2818760 ā 14268 ā ā ā ā ā +ā 2817876 ā 14517 ā ā ā ā ā @@ -31 +31 @@ -ā batchDepositETHForStaking ā 464810 ā 464810 ā 464810 ā 464810 ā 1 ā +ā batchDepositETHForStaking ā 464689 ā 464689 ā 464689 ā 464689 ā 1 ā @@ -35 +35 @@ -ā depositETH ā 136571 ā 136571 ā 136571 ā 136571 ā 1 ā +ā depositETH ā 132059 ā 132059 ā 132059 ā 132059 ā 1 ā @@ -71 +71 @@ -ā 244047 ā 1376 ā ā ā ā ā +ā 230141 ā 1415 ā ā ā ā ā @@ -75 +75 @@ -ā deployLPToken ā 1003 ā 203531 ā 210012 ā 229912 ā 51 ā +ā deployLPToken ā 1003 ā 202593 ā 207891 ā 227791 ā 51 ā @@ -77 +77 @@ -ā lpTokenImplementation ā 2325 ā 2325 ā 2325 ā 2325 ā 1 ā +ā lpTokenImplementation ā 204 ā 204 ā 204 ā 204 ā 1 ā @@ -84 +84 @@ -ā 201045 ā 1036 ā ā ā ā ā +ā 210451 ā 1083 ā ā ā ā ā @@ -88 +88 @@ -ā deploy ā 159472 ā 159472 ā 159472 ā 159472 ā 8 ā +ā deploy ā 147378 ā 147378 ā 147378 ā 147378 ā 8 ā @@ -148 +148 @@ -ā 2401073 ā 12256 ā ā ā ā ā +ā 2420089 ā 12505 ā ā ā ā ā @@ -152 +152 @@ -ā batchDepositETHForStaking ā 442116 ā 442116 ā 442116 ā 442116 ā 1 ā +ā batchDepositETHForStaking ā 439995 ā 439995 ā 439995 ā 439995 ā 1 ā @@ -154 +154 @@ -ā depositETH ā 124557 ā 124557 ā 124557 ā 124557 ā 1 ā +ā depositETH ā 120257 ā 120257 ā 120257 ā 120257 ā 1 ā @@ -158 +158 @@ -ā withdrawDETH ā 119578 ā 119578 ā 119578 ā 119578 ā 1 ā +ā withdrawDETH ā 119338 ā 119338 ā 119338 ā 119338 ā 1 ā @@ -210 +185 @@ -ā getNetworkFeeRecipient ā 8652 ā 8652 ā 8652 ā 8652 ā 1 ā +ā getNetworkFeeRecipient ā 6528 ā 6528 ā 6528 ā 6528 ā 1 ā @@ -212 +187 @@ -ā init ā 7845306 ā 7876536 ā 7845306 ā 8027382 ā 46 ā +ā init ā 7845306 ā 7874433 ā 7845306 ā 8015288 ā 46 ā @@ -218 +193 @@ -ā mintDerivatives ā 165229 ā 1169725 ā 1295056 ā 1296030 ā 9 ā +ā mintDerivatives ā 165229 ā 1168059 ā 1292932 ā 1293906 ā 9 ā @@ -277 +252 @@ -ā batchDepositETHForStaking ā 441100 ā 441100 ā 441100 ā 441100 ā 1 ā +ā batchDepositETHForStaking ā 438979 ā 438979 ā 438979 ā 438979 ā 1 ā @@ -289 +264 @@ -ā depositETHForStaking ā 6515 ā 340328 ā 439226 ā 441966 ā 22 ā +ā depositETHForStaking ā 6515 ā 339143 ā 437105 ā 439845 ā 22 ā @@ -318 +293 @@ -ā batchDepositETHForStaking ā 960 ā 474698 ā 472169 ā 1362765 ā 14 ā +ā batchDepositETHForStaking ā 960 ā 473577 ā 471048 ā 1360402 ā 14 ā @@ -332 +307 @@ -ā depositETHForStaking ā 6730 ā 257794 ā 428647 ā 487587 ā 13 ā +ā depositETHForStaking ā 6730 ā 257575 ā 428526 ā 485466 ā 13 ā @@ -492 +467 @@ -ā 3494833 ā 17331 ā ā ā ā ā +ā 3484573 ā 17408 ā ā ā ā ā @@ -496 +471 @@ -ā calculateSyndicateDeploymentAddress ā 3207 ā 3207 ā 3207 ā 3207 ā 1 ā +ā calculateSyndicateDeploymentAddress ā 1083 ā 1083 ā 1083 ā 1083 ā 1 ā @@ -498 +473 @@ -ā deployMockSyndicate ā 183696 ā 183696 ā 183696 ā 183696 ā 7 ā +ā deployMockSyndicate ā 183572 ā 183572 ā 183572 ā 183572 ā 7 ā @@ -500 +475 @@ -ā deploySyndicate ā 181812 ā 182057 ā 182092 ā 182092 ā 8 ā +ā deploySyndicate ā 179968 ā 180183 ā 179968 ā 181688 ā 8 ā @@ -582,0 +558 @@ +Overall gas change: -97881 (-27.471%)
calldata instead of memory for read-only arguments in external functions saves gasWhen a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution. Note that even if an interface defines a function as having memory arguments, it's still valid for implementation contracs to use calldata arguments instead.
If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it's still more gass-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one
Note that I've also flagged instances where the function is public but can be marked as external since it's not called by the contract, and cases where a constructor is involved
There are 8 instances of this issue:
File: contracts/liquid-staking/StakingFundsVault.sol /// @audit _blsPubKeys 355: function claimFundsFromSyndicateForDistribution(bytes[] memory _blsPubKeys) external {
File: contracts/smart-wallet/OwnableSmartWallet.sol /// @audit callData 41 function execute(address target, bytes memory callData) 42 external 43 override 44 payable 45 onlyOwner // F: [OSW-6A] 46: returns (bytes memory) /// @audit callData 52 function execute( 53 address target, 54 bytes memory callData, 55 uint256 value 56 ) 57 external 58 override 59 payable 60 onlyOwner // F: [OSW-6A] 61: returns (bytes memory) /// @audit callData 67 function rawExecute( 68 address target, 69 bytes memory callData, 70 uint256 value 71 ) 72 external 73 override 74 payable 75 onlyOwner 76: returns (bytes memory)
File: contracts/syndicate/Syndicate.sol /// @audit _priorityStakers /// @audit _blsPubKeysForSyndicateKnots 129 function initialize( 130 address _contractOwner, 131 uint256 _priorityStakingEndBlock, 132 address[] memory _priorityStakers, 133 bytes[] memory _blsPubKeysForSyndicateKnots 134: ) external virtual override initializer { /// @audit _blsPubKey 337: function updateCollateralizedSlotOwnersAccruedETH(bytes memory _blsPubKey) external { /// @audit _blsPubKeys 343: function batchUpdateCollateralizedSlotOwnersAccruedETH(bytes[] memory _blsPubKeys) external {
The instances below point to the second+ access of a state variable within a function. Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.
There are 16 instances of this issue:
File: contracts/liquid-staking/ETHPoolLPFactory.sol /// @audit numberOfLPTokensIssued on line 131 141: numberOfLPTokensIssued++; /// @audit lpTokenFactory on line 137 138: LPToken(lpTokenFactory.deployLPToken(address(this), address(0), tokenSymbol, tokenName));
File: contracts/liquid-staking/LiquidStakingManager.sol /// @audit dao on line 241 243: emit UpdateDAOAddress(dao, _newAddress); /// @audit stakehouseTicker on line 822 866: emit StakehouseCreated(stakehouseTicker, stakehouse); /// @audit stakehouse on line 833 834: IERC20 sETH = IERC20(getSlotRegistry().stakeHouseShareTokens(stakehouse)); /// @audit stakehouse on line 834 838: stakehouse, /// @audit stakehouse on line 838 846: IStakeHouseRegistry(stakehouse).setGateKeeper(address(gatekeeper)); /// @audit stakehouse on line 846 866: emit StakehouseCreated(stakehouseTicker, stakehouse); /// @audit brand on line 780 788: IBrandNFT(brand).lowercaseBrandTickerToTokenId(lowerTicker), /// @audit syndicate on line 853 861: sETH.approve(syndicate, (2 ** 256) - 1); /// @audit enableWhitelisting on line 269 270: emit WhitelistingStatusChanged(msg.sender, enableWhitelisting); /// @audit enableWhitelisting on line 270 272: return enableWhitelisting; /// @audit daoCommissionPercentage on line 915 916: uint256 daoAmount = (_received * daoCommissionPercentage) / MODULO;
File: contracts/syndicate/Syndicate.sol /// @audit accumulatedETHPerCollateralizedSlotPerKnot on line 494 527: totalETHProcessedPerCollateralizedKnot[_blsPubKey] = accumulatedETHPerCollateralizedSlotPerKnot; /// @audit totalFreeFloatingShares on line 551 551: return totalFreeFloatingShares > 0 ? (_ethSinceLastUpdate * PRECISION) / totalFreeFloatingShares : 0; /// @audit numberOfRegisteredKnots on line 177 189: uint256 collateralizedUnprocessed = ((totalEthPerSlotType - lastSeenETHPerCollateralizedSlotPerKnot) / numberOfRegisteredKnots);
diff --git a/contracts/liquid-staking/ETHPoolLPFactory.sol b/contracts/liquid-staking/ETHPoolLPFactory.sol index 4bd43b2..6207f41 100644 --- a/contracts/liquid-staking/ETHPoolLPFactory.sol +++ b/contracts/liquid-staking/ETHPoolLPFactory.sol @@ -128,17 +128,16 @@ abstract contract ETHPoolLPFactory is StakehouseAPI { else { // mint new LP tokens for the new KNOT // add the KNOT in the mapping - string memory tokenNumber = Strings.toString(numberOfLPTokensIssued); + // and increase the count of LP tokens + string memory tokenNumber = Strings.toString(numberOfLPTokensIssued++); string memory tokenName = string(abi.encodePacked(baseLPTokenName, tokenNumber)); string memory tokenSymbol = string(abi.encodePacked(baseLPTokenSymbol, tokenNumber)); // deploy new LP token and optionally enable transfer notifications + LPTokenFactory lpTokenFactoryCache = lpTokenFactory; LPToken newLPToken = _enableTransferHook ? - LPToken(lpTokenFactory.deployLPToken(address(this), address(this), tokenSymbol, tokenName)) : - LPToken(lpTokenFactory.deployLPToken(address(this), address(0), tokenSymbol, tokenName)); - - // increase the count of LP tokens - numberOfLPTokensIssued++; + LPToken(lpTokenFactoryCache.deployLPToken(address(this), address(this), tokenSymbol, tokenName)) : + LPToken(lpTokenFactoryCache.deployLPToken(address(this), address(0), tokenSymbol, tokenName)); // register the BLS Public Key with the LP token lpTokenForKnot[_blsPublicKeyOfKnot] = newLPToken; @@ -149,4 +148,4 @@ abstract contract ETHPoolLPFactory is StakehouseAPI { emit NewLPTokenIssued(_blsPublicKeyOfKnot, address(newLPToken), msg.sender, _amount); } } -} \ No newline at end of file +} diff --git a/contracts/liquid-staking/LiquidStakingManager.sol b/contracts/liquid-staking/LiquidStakingManager.sol index 5fe8c49..a939b64 100644 --- a/contracts/liquid-staking/LiquidStakingManager.sol +++ b/contracts/liquid-staking/LiquidStakingManager.sol @@ -238,9 +238,10 @@ contract LiquidStakingManager is ILiquidStakingManager, Initializable, Reentranc /// @notice Allow DAO to migrate to a new address function updateDAOAddress(address _newAddress) external onlyDAO { require(_newAddress != address(0), "Zero address"); - require(_newAddress != dao, "Same address"); + address daoCache = dao; + require(_newAddress != daoCache, "Same address"); - emit UpdateDAOAddress(dao, _newAddress); + emit UpdateDAOAddress(daoCache, _newAddress); dao = _newAddress; } @@ -266,10 +267,9 @@ contract LiquidStakingManager is ILiquidStakingManager, Initializable, Reentranc /// @param _changeWhitelist boolean value. true to enable and false to disable function updateWhitelisting(bool _changeWhitelist) external onlyDAO returns (bool) { require(_changeWhitelist != enableWhitelisting, "Unnecessary update to same status"); - enableWhitelisting = _changeWhitelist; - emit WhitelistingStatusChanged(msg.sender, enableWhitelisting); + emit WhitelistingStatusChanged(msg.sender, enableWhitelisting = _changeWhitelist); - return enableWhitelisting; + return _changeWhitelist; } /// @notice function to enable/disable whitelisting of a noderunner @@ -777,7 +777,8 @@ contract LiquidStakingManager is ILiquidStakingManager, Initializable, Reentranc address associatedSmartWallet = smartWalletOfKnot[_blsPubKey]; // Join the LSDN stakehouse - string memory lowerTicker = IBrandNFT(brand).toLowerCase(stakehouseTicker); + address brandCache = brand; + string memory lowerTicker = IBrandNFT(brandCache).toLowerCase(stakehouseTicker); IOwnableSmartWallet(associatedSmartWallet).execute( address(getTransactionRouter()), abi.encodeWithSelector( @@ -785,7 +786,7 @@ contract LiquidStakingManager is ILiquidStakingManager, Initializable, Reentranc associatedSmartWallet, _blsPubKey, stakehouse, - IBrandNFT(brand).lowercaseBrandTickerToTokenId(lowerTicker), + IBrandNFT(brandCache).lowercaseBrandTickerToTokenId(lowerTicker), savETHVault.indexOwnedByTheVault(), _beaconChainBalanceReport, _reportSignature @@ -813,13 +814,14 @@ contract LiquidStakingManager is ILiquidStakingManager, Initializable, Reentranc // The savETH will go to the savETH vault, the collateralized SLOT for syndication owned by the smart wallet // sETH will also be minted in the smart wallet but will be moved out and distributed to the syndicate for claiming by the DAO address associatedSmartWallet = smartWalletOfKnot[_blsPublicKeyOfKnot]; + string memory stakehouseTickerCache = stakehouseTicker; IOwnableSmartWallet(associatedSmartWallet).execute( address(getTransactionRouter()), abi.encodeWithSelector( ITransactionRouter.createStakehouse.selector, associatedSmartWallet, _blsPublicKeyOfKnot, - stakehouseTicker, + stakehouseTickerCache, savETHVault.indexOwnedByTheVault(), _beaconChainBalanceReport, _reportSignature @@ -830,12 +832,13 @@ contract LiquidStakingManager is ILiquidStakingManager, Initializable, Reentranc numberOfKnots += 1; // Capture the address of the Stakehouse for future knots to join - stakehouse = getStakeHouseUniverse().memberKnotToStakeHouse(_blsPublicKeyOfKnot); - IERC20 sETH = IERC20(getSlotRegistry().stakeHouseShareTokens(stakehouse)); + address stakehouseCache = + stakehouse = getStakeHouseUniverse().memberKnotToStakeHouse(_blsPublicKeyOfKnot); + IERC20 sETH = IERC20(getSlotRegistry().stakeHouseShareTokens(stakehouseCache)); // Give liquid staking manager ability to manage keepers and set a house keeper if decided by the network IOwnableSmartWallet(associatedSmartWallet).execute( - stakehouse, + stakehouseCache, abi.encodeWithSelector( Ownable.transferOwnership.selector, address(this) @@ -843,14 +846,14 @@ contract LiquidStakingManager is ILiquidStakingManager, Initializable, Reentranc ); if (address(gatekeeper) != address(0)) { - IStakeHouseRegistry(stakehouse).setGateKeeper(address(gatekeeper)); + IStakeHouseRegistry(stakehouseCache).setGateKeeper(address(gatekeeper)); } // Deploy the EIP1559 transaction reward sharing contract but no priority required because sETH will be auto staked address[] memory priorityStakers = new address[](0); bytes[] memory initialKnots = new bytes[](1); initialKnots[0] = _blsPublicKeyOfKnot; - syndicate = syndicateFactory.deploySyndicate( + address syndicateCache = syndicate = syndicateFactory.deploySyndicate( address(this), 0, priorityStakers, @@ -858,12 +861,12 @@ contract LiquidStakingManager is ILiquidStakingManager, Initializable, Reentranc ); // Contract approves syndicate to take sETH on behalf of the DAO - sETH.approve(syndicate, (2 ** 256) - 1); + sETH.approve(syndicateCache, (2 ** 256) - 1); // Auto-stake sETH by pulling sETH out the smart wallet and staking in the syndicate _autoStakeWithSyndicate(associatedSmartWallet, _blsPublicKeyOfKnot); - emit StakehouseCreated(stakehouseTicker, stakehouse); + emit StakehouseCreated(stakehouseTickerCache, stakehouseCache); } /// @dev Remove the sETH from the node runner smart wallet in order to auto-stake the sETH in the syndicate @@ -912,8 +915,9 @@ contract LiquidStakingManager is ILiquidStakingManager, Initializable, Reentranc function _calculateCommission(uint256 _received) internal virtual view returns (uint256 _nodeRunner, uint256 _dao) { require(_received > 0, "Nothing received"); - if (daoCommissionPercentage > 0) { - uint256 daoAmount = (_received * daoCommissionPercentage) / MODULO; + uint256 daoCommissionPercentageCache = daoCommissionPercentage; + if (daoCommissionPercentageCache > 0) { + uint256 daoAmount = (_received * daoCommissionPercentageCache) / MODULO; uint256 rest = _received - daoAmount; return (rest, daoAmount); } diff --git a/contracts/syndicate/Syndicate.sol b/contracts/syndicate/Syndicate.sol index a3c6450..7dff745 100644 --- a/contracts/syndicate/Syndicate.sol +++ b/contracts/syndicate/Syndicate.sol @@ -174,7 +174,8 @@ contract Syndicate is ISyndicateInit, Initializable, Ownable, ReentrancyGuard, S function updateAccruedETHPerShares() public { // Ensure there are registered KNOTs. Syndicates are deployed with at least 1 registered but this can fall to zero. // Fee recipient should be re-assigned in the event that happens as any further ETH can be collected by owner - if (numberOfRegisteredKnots > 0) { + uint256 numberOfRegisteredKnotsCache = numberOfRegisteredKnots; + if (numberOfRegisteredKnotsCache > 0) { // All time, total ETH that was earned per slot type (free floating or collateralized) uint256 totalEthPerSlotType = calculateETHForFreeFloatingOrCollateralizedHolders(); @@ -186,7 +187,7 @@ contract Syndicate is ISyndicateInit, Initializable, Ownable, ReentrancyGuard, S lastSeenETHPerFreeFloating = totalEthPerSlotType; } - uint256 collateralizedUnprocessed = ((totalEthPerSlotType - lastSeenETHPerCollateralizedSlotPerKnot) / numberOfRegisteredKnots); + uint256 collateralizedUnprocessed = ((totalEthPerSlotType - lastSeenETHPerCollateralizedSlotPerKnot) / numberOfRegisteredKnotsCache); accumulatedETHPerCollateralizedSlotPerKnot += collateralizedUnprocessed; lastSeenETHPerCollateralizedSlotPerKnot = totalEthPerSlotType; @@ -490,8 +491,9 @@ contract Syndicate is ISyndicateInit, Initializable, Ownable, ReentrancyGuard, S /// Given an amount of ETH allocated to the collateralized SLOT owners of a KNOT, distribute this amongs the current set of collateralized owners (a dynamic set of addresses and balances) function _updateCollateralizedSlotOwnersLiabilitySnapshot(bytes memory _blsPubKey) internal { // Establish how much new ETH is for the new KNOT + uint256 accumulatedETHPerCollateralizedSlotPerKnotCache = accumulatedETHPerCollateralizedSlotPerKnot; uint256 unprocessedETHForCurrentKnot = - accumulatedETHPerCollateralizedSlotPerKnot - totalETHProcessedPerCollateralizedKnot[_blsPubKey]; + accumulatedETHPerCollateralizedSlotPerKnotCache - totalETHProcessedPerCollateralizedKnot[_blsPubKey]; // Get information about the knot i.e. associated house and whether its active (address stakeHouse,,,,,bool isActive) = getStakeHouseUniverse().stakeHouseKnotInfo(_blsPubKey); @@ -524,7 +526,7 @@ contract Syndicate is ISyndicateInit, Initializable, Ownable, ReentrancyGuard, S } // record so unprocessed goes to zero - totalETHProcessedPerCollateralizedKnot[_blsPubKey] = accumulatedETHPerCollateralizedSlotPerKnot; + totalETHProcessedPerCollateralizedKnot[_blsPubKey] = accumulatedETHPerCollateralizedSlotPerKnotCache; } } @@ -548,7 +550,8 @@ contract Syndicate is ISyndicateInit, Initializable, Ownable, ReentrancyGuard, S /// @dev Business logic for calculating per free floating share how much ETH from 1559 rewards is owed function _calculateNewAccumulatedETHPerFreeFloatingShare(uint256 _ethSinceLastUpdate) internal view returns (uint256) { - return totalFreeFloatingShares > 0 ? (_ethSinceLastUpdate * PRECISION) / totalFreeFloatingShares : 0; + uint256 totalFreeFloatingSharesCache = totalFreeFloatingShares; + return totalFreeFloatingSharesCache > 0 ? (_ethSinceLastUpdate * PRECISION) / totalFreeFloatingSharesCache : 0; } /// @dev Business logic for adding a new set of knots to the syndicate for collecting revenue @@ -678,4 +681,4 @@ contract Syndicate is ISyndicateInit, Initializable, Ownable, ReentrancyGuard, S } } } -} \ No newline at end of file +}
Note that the numbers below are an underreporting of the gas changes due to this forge issue
diff --git a/tmp/gas_before b/tmp/gas_after index fba0986..97bb2fa 100644 --- a/tmp/gas_before +++ b/tmp/gas_after @@ -31 +31 @@ -ā batchDepositETHForStaking ā 464810 ā 464810 ā 464810 ā 464810 ā 1 ā +ā batchDepositETHForStaking ā 464737 ā 464737 ā 464737 ā 464737 ā 1 ā @@ -64 +64 @@ -ā transfer ā 125385 ā 125385 ā 125385 ā 125385 ā 1 ā +ā transfer ā 125191 ā 125191 ā 125191 ā 125191 ā 1 ā @@ -99 +99 @@ -ā execute(address,bytes)(bytes) ā 2508 ā 110200 ā 3537 ā 846684 ā 68 ā +ā execute(address,bytes)(bytes) ā 2508 ā 110188 ā 3537 ā 846684 ā 68 ā @@ -129 +129 @@ -ā approve(address,uint256) ā 24624 ā 24624 ā 24624 ā 24624 ā 1 ā +ā approve(address,uint256) ā 24624 ā 24624 ā 24624 ā 24624 ā 2 ā @@ -131 +131 @@ -ā approve(address,uint256)(bool) ā 22524 ā 24507 ā 24624 ā 24624 ā 18 ā +ā approve(address,uint256)(bool) ā 22524 ā 24500 ā 24624 ā 24624 ā 17 ā @@ -139 +139 @@ -ā transferFrom(address,address,uint256) ā 20508 ā 20508 ā 20508 ā 20508 ā 1 ā +ā transferFrom(address,address,uint256) ā 20508 ā 20508 ā 20508 ā 20508 ā 2 ā @@ -141 +141 @@ -ā transferFrom(address,address,uint256)(bool) ā 2988 ā 16645 ā 20508 ā 26127 ā 19 ā +ā transferFrom(address,address,uint256)(bool) ā 2988 ā 16430 ā 20508 ā 26127 ā 18 ā @@ -152 +152 @@ -ā batchDepositETHForStaking ā 442116 ā 442116 ā 442116 ā 442116 ā 1 ā +ā batchDepositETHForStaking ā 442043 ā 442043 ā 442043 ā 442043 ā 1 ā @@ -165 +165 @@ -ā 7497782 ā 37351 ā ā ā ā ā +ā 7496782 ā 37346 ā ā ā ā ā @@ -173 +173 @@ -ā deployNewMockLiquidStakingDerivativeNetwork ā 7912949 ā 7944581 ā 7912949 ā 8095025 ā 46 ā +ā deployNewMockLiquidStakingDerivativeNetwork ā 7914560 ā 7946192 ā 7914560 ā 8096636 ā 46 ā @@ -190 +190 @@ -ā 12369773 ā 61714 ā ā ā ā ā +ā 12402840 ā 61879 ā ā ā ā ā @@ -196 +196 @@ -ā claimRewardsAsNodeRunner ā 21013 ā 118803 ā 110983 ā 232235 ā 4 ā +ā claimRewardsAsNodeRunner ā 20830 ā 118604 ā 110858 ā 231871 ā 4 ā @@ -200 +200 @@ -ā deRegisterKnotFromSyndicate ā 151263 ā 151263 ā 151263 ā 151263 ā 1 ā +ā deRegisterKnotFromSyndicate ā 151036 ā 151036 ā 151036 ā 151036 ā 1 ā @@ -212 +212 @@ -ā init ā 7845306 ā 7876536 ā 7845306 ā 8027382 ā 46 ā +ā init ā 7846917 ā 7878147 ā 7846917 ā 8028993 ā 46 ā @@ -218 +218 @@ -ā mintDerivatives ā 165229 ā 1169725 ā 1295056 ā 1296030 ā 9 ā +ā mintDerivatives ā 164815 ā 1169899 ā 1295366 ā 1296240 ā 9 ā @@ -224 +224 @@ -ā restoreFreeFloatingSharesToSmartWalletForRageQuit ā 123847 ā 123847 ā 123847 ā 123847 ā 1 ā +ā restoreFreeFloatingSharesToSmartWalletForRageQuit ā 123857 ā 123857 ā 123857 ā 123857 ā 1 ā @@ -258 +258 @@ -ā updateDAOAddress ā 2592 ā 3992 ā 3992 ā 5392 ā 2 ā +ā updateDAOAddress ā 2515 ā 3915 ā 3915 ā 5315 ā 2 ā @@ -260 +260 @@ -ā updateWhitelisting ā 5455 ā 5455 ā 5455 ā 5455 ā 1 ā +ā updateWhitelisting ā 5321 ā 5321 ā 5321 ā 5321 ā 1 ā @@ -269 +269 @@ -ā 3049390 ā 15294 ā ā ā ā ā +ā 3050198 ā 15298 ā ā ā ā ā @@ -277 +277 @@ -ā batchDepositETHForStaking ā 441100 ā 441100 ā 441100 ā 441100 ā 1 ā +ā batchDepositETHForStaking ā 441027 ā 441027 ā 441027 ā 441027 ā 1 ā @@ -289 +289 @@ -ā depositETHForStaking ā 6515 ā 340328 ā 439226 ā 441966 ā 22 ā +ā depositETHForStaking ā 6515 ā 340271 ā 439153 ā 441893 ā 22 ā @@ -312 +312 @@ -ā 3990984 ā 19996 ā ā ā ā ā +ā 3991784 ā 20000 ā ā ā ā ā @@ -318 +318 @@ -ā batchDepositETHForStaking ā 960 ā 474698 ā 472169 ā 1362765 ā 14 ā +ā batchDepositETHForStaking ā 960 ā 474625 ā 472096 ā 1362546 ā 14 ā @@ -320 +320 @@ -ā beforeTokenTransfer ā 1813 ā 5228 ā 1813 ā 74855 ā 29 ā +ā beforeTokenTransfer ā 1813 ā 5221 ā 1813 ā 74661 ā 29 ā @@ -328 +328 @@ -ā claimRewards ā 55587 ā 172294 ā 168285 ā 351598 ā 8 ā +ā claimRewards ā 55393 ā 172124 ā 168091 ā 351404 ā 8 ā @@ -332 +332 @@ -ā depositETHForStaking ā 6730 ā 257794 ā 428647 ā 487587 ā 13 ā +ā depositETHForStaking ā 6730 ā 257755 ā 428574 ā 487514 ā 13 ā @@ -344 +344 @@ -ā previewAccumulatedETH ā 9704 ā 13421 ā 11032 ā 17032 ā 12 ā +ā previewAccumulatedETH ā 9606 ā 13323 ā 10934 ā 16934 ā 12 ā @@ -352 +352 @@ -ā unstakeSyndicateSharesForRageQuit ā 121907 ā 121907 ā 121907 ā 121907 ā 1 ā +ā unstakeSyndicateSharesForRageQuit ā 121917 ā 121917 ā 121917 ā 121917 ā 1 ā @@ -492 +492 @@ -ā 3494833 ā 17331 ā ā ā ā ā +ā 3493833 ā 17326 ā ā ā ā ā @@ -511 +511 @@ -ā 2990771 ā 15093 ā ā ā ā ā +ā 2989771 ā 15088 ā ā ā ā ā @@ -525 +525 @@ -ā calculateNewAccumulatedETHPerFreeFloatingShare ā 1199 ā 1199 ā 1199 ā 1199 ā 1 ā +ā calculateNewAccumulatedETHPerFreeFloatingShare ā 1101 ā 1101 ā 1101 ā 1101 ā 1 ā @@ -527 +527 @@ -ā claimAsCollateralizedSLOTOwner ā 10767 ā 114826 ā 114333 ā 245733 ā 13 ā +ā claimAsCollateralizedSLOTOwner ā 10584 ā 114589 ā 114050 ā 245450 ā 13 ā @@ -529 +529 @@ -ā claimAsStaker ā 8220 ā 61175 ā 47989 ā 202304 ā 25 ā +ā claimAsStaker ā 8026 ā 60989 ā 47795 ā 202110 ā 25 ā @@ -531 +531 @@ -ā deRegisterKnots ā 148072 ā 148072 ā 148072 ā 148072 ā 1 ā +ā deRegisterKnots ā 147845 ā 147845 ā 147845 ā 147845 ā 1 ā @@ -553 +553 @@ -ā previewUnclaimedETHAsFreeFloatingStaker ā 2806 ā 3351 ā 2806 ā 4806 ā 22 ā +ā previewUnclaimedETHAsFreeFloatingStaker ā 2708 ā 3253 ā 2708 ā 4708 ā 22 ā @@ -555 +555 @@ -ā registerKnotsToSyndicate ā 6503 ā 57756 ā 64257 ā 108857 ā 5 ā +ā registerKnotsToSyndicate ā 6309 ā 57582 ā 64063 ā 108663 ā 5 ā @@ -565 +565 @@ -ā stake ā 37539 ā 89145 ā 99152 ā 160976 ā 21 ā +ā stake ā 37384 ā 89027 ā 99056 ā 160880 ā 21 ā @@ -579 +579 @@ -ā unstake ā 15073 ā 16506 ā 16506 ā 17939 ā 2 ā +ā unstake ā 15078 ā 16431 ā 16431 ā 17784 ā 2 ā @@ -581 +581 @@ -ā updateAccruedETHPerShares ā 83678 ā 83678 ā 83678 ā 83678 ā 1 ā +ā updateAccruedETHPerShares ā 83484 ā 83484 ā 83484 ā 83484 ā 1 ā @@ -582,0 +583 @@ +Overall gas change: -13020 (-1.742%)
The instances below point to the second+ call of the function within a single function
There is 1 instance of this issue:
File: contracts/liquid-staking/StakingFundsVault.sol /// @audit liquidStakingNetworkManager.syndicate() on line 215 219: liquidStakingNetworkManager.syndicate(),
<x> += <y> costs more gas than <x> = <x> + <y> for state variablesUsing the addition operator instead of plus-equals saves 113 gas
There are 16 instances of this issue:
File: contracts/liquid-staking/GiantPoolBase.sol 39: idleETH += msg.value; 57: idleETH -= _amount;
File: contracts/liquid-staking/LiquidStakingManager.sol 773: numberOfKnots += 1; 830: numberOfKnots += 1;
File: contracts/liquid-staking/StakingFundsVault.sol 58: totalShares += 4 ether;
File: contracts/liquid-staking/SyndicateRewardsProcessor.sol 65: totalClaimed += due; 85: accumulatedETHPerLPShare += (unprocessed * PRECISION) / _numOfShares;
File: contracts/syndicate/Syndicate.sol 185: accumulatedETHPerFreeFloatingShare += _calculateNewAccumulatedETHPerFreeFloatingShare(freeFloatingUnprocessed); 190: accumulatedETHPerCollateralizedSlotPerKnot += collateralizedUnprocessed; 225: totalFreeFloatingShares += _sETHAmount; 269: totalFreeFloatingShares -= _sETHAmount; 317: totalClaimed += unclaimedUserShare; 558: numberOfRegisteredKnots += knotsToRegister; 621: totalFreeFloatingShares -= sETHTotalStakeForKnot[_blsPublicKey]; 624: numberOfRegisteredKnots -= 1; 658: totalClaimed += unclaimedUserShare;
internal functions only called once can be inlined to save gasNot inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
There are 9 instances of this issue:
File: contracts/liquid-staking/LiquidStakingManager.sol 636 function _init( 637 address _dao, 638 address _syndicateFactory, 639 address _smartWalletFactory, 640 address _lpTokenFactory, 641 address _brand, 642 address _savETHVaultDeployer, 643 address _stakingFundsVaultDeployer, 644 address _optionalGatekeeperDeployer, 645 uint256 _optionalCommission, 646 bool _deployOptionalGatekeeper, 647: string calldata _stakehouseTicker 675: function _isNodeRunnerValid(address _nodeRunner) internal view returns (bool) { 730 function _stake( 731 bytes calldata _blsPublicKey, 732 bytes calldata _cipherText, 733 bytes calldata _aesEncryptorKey, 734 IDataStructures.EIP712Signature calldata _encryptionSignature, 735: bytes32 dataRoot 767 function _joinLSDNStakehouse( 768 bytes calldata _blsPubKey, 769 IDataStructures.ETH2DataReport calldata _beaconChainBalanceReport, 770: IDataStructures.EIP712Signature calldata _reportSignature 807 function _createLSDNStakehouse( 808 bytes calldata _blsPublicKeyOfKnot, 809 IDataStructures.ETH2DataReport calldata _beaconChainBalanceReport, 810: IDataStructures.EIP712Signature calldata _reportSignature 925: function _assertEtherIsReadyForValidatorStaking(bytes calldata blsPubKey) internal view {
File: contracts/liquid-staking/SavETHVault.sol 235: function _init(address _liquidStakingManagerAddress, LPTokenFactory _lpTokenFactory) internal {
File: contracts/syndicate/Syndicate.sol 469 function _initialize( 470 address _contractOwner, 471 uint256 _priorityStakingEndBlock, 472 address[] memory _priorityStakers, 473: bytes[] memory _blsPubKeysForSyndicateKnots 597: function _deRegisterKnots(bytes[] calldata _blsPublicKeys) internal {
++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loopsThe unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop
There are 38 instances of this issue:
File: contracts/liquid-staking/ETHPoolLPFactory.sol 63: for (uint256 i; i < numOfRotations; ++i) {
File: contracts/liquid-staking/GiantMevAndFeesPool.sol 38: for (uint256 i; i < numOfVaults; ++i) { 64: for (uint256 i; i < numOfVaults; ++i) { 90: for (uint256 i; i < _stakingFundsVaults.length; ++i) { 117: for (uint256 i; i < numOfRotations; ++i) { 135: for (uint256 i; i < numOfVaults; ++i) { 183: for (uint256 i; i < _lpTokens.length; ++i) {
File: contracts/liquid-staking/GiantPoolBase.sol 76: for (uint256 i; i < amountOfTokens; ++i) {
File: contracts/liquid-staking/GiantSavETHVaultPool.sol 42: for (uint256 i; i < numOfSavETHVaults; ++i) { 78: for (uint256 i; i < numOfVaults; ++i) { 82: for (uint256 j; j < _lpTokens[i].length; ++j) { 128: for (uint256 i; i < numOfRotations; ++i) { 146: for (uint256 i; i < numOfVaults; ++i) { 148: for (uint256 j; j < _lpTokens[i].length; ++j) {
File: contracts/liquid-staking/LiquidStakingManager.sol 390: for(uint256 i; i < _blsPubKeys.length; ++i) { 463: for(uint256 i; i < len; ++i) { 529: for (uint256 i; i < numOfValidators; ++i) { 578: for (uint256 i; i < numOfKnotsToProcess; ++i) {
File: contracts/liquid-staking/SavETHVault.sol 63: for (uint256 i; i < numOfValidators; ++i) { 103: for (uint256 i; i < numOfTokens; ++i) { 116: for (uint256 i; i < numOfTokens; ++i) {
File: contracts/liquid-staking/StakingFundsVault.sol 78: for (uint256 i; i < numOfValidators; ++i) { 152: for (uint256 i; i < numOfTokens; ++i) { 166: for (uint256 i; i < numOfTokens; ++i) { 203: for (uint256 i; i < _blsPubKeys.length; ++i) { 266: for (uint256 i; i < _blsPublicKeys.length; ++i) { 276: for (uint256 i; i < _blsPubKeys.length; ++i) { 286: for (uint256 i; i < _token.length; ++i) {
File: contracts/syndicate/Syndicate.sol 211: for (uint256 i; i < _blsPubKeys.length; ++i) { 259: for (uint256 i; i < _blsPubKeys.length; ++i) { 301: for (uint256 i; i < _blsPubKeys.length; ++i) { 346: for (uint256 i; i < _blsPubKeys.length; ++i) { 420: for (uint256 i; i < numberOfCollateralisedSlotOwnersForKnot; ++i) { 513: for (uint256 i; i < numberOfCollateralisedSlotOwnersForKnot; ++i) { 560: for (uint256 i; i < knotsToRegister; ++i) { 585: for (uint256 i; i < _priorityStakers.length; ++i) { 598: for (uint256 i; i < _blsPublicKeys.length; ++i) { 648: for (uint256 i; i < _blsPubKeys.length; ++i) {
require()/revert() strings longer than 32 bytes cost extra gasEach extra memory word of bytes past the original 32 incurs an MSTORE which costs 3 gas
There are 39 instances of this issue:
File: contracts/liquid-staking/ETHPoolLPFactory.sol 122: require(lpToken.totalSupply() + _amount <= maxStakingAmountPerValidator, "Amount exceeds the staking limit for the validator");
File: contracts/liquid-staking/GiantPoolBase.sol 55: require(idleETH >= _amount, "Come back later or withdraw less ETH");
File: contracts/liquid-staking/GiantSavETHVaultPool.sol 149 require( 150 vault.isDETHReadyForWithdrawal(address(_lpTokens[i][j])) == false, 151 "ETH is either staked or derivatives minted" 152: );
File: contracts/liquid-staking/LiquidStakingManager.sol 256: require(bytes(_newTicker).length >= 3, "String must be 3-5 characters long"); 257: require(bytes(_newTicker).length <= 5, "String must be 3-5 characters long"); 258: require(numberOfKnots == 0, "Cannot change ticker once house is created"); 268: require(_changeWhitelist != enableWhitelisting, "Unnecessary update to same status"); 280: require(isNodeRunnerWhitelisted[_nodeRunner] != isNodeRunnerWhitelisted[_nodeRunner], "Unnecessary update to same status"); 291: require(isNodeRunnerBanned(msg.sender) == false, "Node runner is banned from LSD network"); 328: require(isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key has already withdrawn or not a part of LSD network"); 331: require(smartWalletOfNodeRunner[msg.sender] == associatedSmartWallet, "Not the node runner for the smart wallet "); 332: require(isNodeRunnerBanned(nodeRunnerOfSmartWallet[associatedSmartWallet]) == false, "Node runner is banned from LSD network"); 391: require(isBLSPublicKeyBanned(_blsPubKeys[i]) == false, "BLS public key is banned or not a part of LSD network"); 394: require(smartWalletOfKnot[_blsPubKeys[i]] == smartWallet, "BLS public key doesn't belong to the node runner"); 433: require(!Address.isContract(_eoaRepresentative), "Only EOA representative permitted"); 435: require(isNodeRunnerBanned(msg.sender) == false, "Node runner is banned from LSD network"); 453: require(smartWalletRepresentative[smartWallet] == _eoaRepresentative, "Different EOA specified - rotate outside"); 467: require(isBLSPublicKeyPartOfLSDNetwork(_blsPublicKey) == false, "BLS public key is banned or not a part of LSD network"); 532: require(isBLSPublicKeyBanned(blsPubKey) == false, "BLS public key is banned or not a part of LSD network"); 580: require(isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is banned or not a part of LSD network"); 653: require(bytes(_stakehouseTicker).length >= 3, "String must be 3-5 characters long"); 654: require(bytes(_stakehouseTicker).length <= 5, "String must be 3-5 characters long"); 927: require(associatedSmartWallet.balance >= 4 ether, "Smart wallet balance must be at least 4 ether"); 930: require(address(stakingFundsLP) != address(0), "No funds staked in staking funds vault"); 931: require(stakingFundsLP.totalSupply() == 4 ether, "DAO staking funds vault balance must be at least 4 ether"); 935: require(savETHVaultLP.totalSupply() == 24 ether, "KNOT must have 24 ETH in savETH vault");
File: contracts/liquid-staking/SavETHVault.sol 64: require(liquidStakingManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is not part of LSD network"); 84: require(liquidStakingManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key is banned or not a part of LSD network"); 90: require(msg.value == _amount, "Must provide correct amount of ETH"); 204: require(_amount >= 24 ether, "Amount cannot be less than 24 ether");
File: contracts/liquid-staking/StakingFundsVault.sol 79: require(liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is not part of LSD network"); 114: require(liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key is banned or not a part of LSD network"); 120: require(msg.value == _amount, "Must provide correct amount of ETH"); 154: require(address(token) != address(0), "No ETH staked for specified BLS key"); 240: require(_amount >= 4 ether, "Amount cannot be less than 4 ether"); 267: require(syndicate.isNoLongerPartOfSyndicate(_blsPublicKeys[i]), "Knot is still active in syndicate");
File: contracts/smart-wallet/OwnableSmartWallet.sol 33 require( 34 initialOwner != address(0), 35 "OwnableSmartWallet: Attempting to initialize with zero address owner" 36: ); 100 require( 101 isTransferApproved(owner(), msg.sender), 102 "OwnableSmartWallet: Transfer is not allowed" 103: ); // F: [OSW-4] 115 require( 116 to != address(0), 117 "OwnableSmartWallet: Approval cannot be set for zero address" 118: ); // F: [OSW-2A]
public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted
There are 16 instances of this issue:
File: contracts/liquid-staking/ETHPoolLPFactory.sol /// @audit batchRotateLPTokens(), rotateLPTokens() 13: abstract contract ETHPoolLPFactory is StakehouseAPI {
File: contracts/liquid-staking/GiantMevAndFeesPool.sol /// @audit batchDepositETHForStaking(), claimRewards(), previewAccumulatedETH(), batchRotateLPTokens(), bringUnusedETHBackIntoGiantPool(), updateAccumulatedETHPerLP(), beforeTokenTransfer(), afterTokenTransfer() 15: contract GiantMevAndFeesPool is ITransferHookProcessor, GiantPoolBase, SyndicateRewardsProcessor {
File: contracts/liquid-staking/GiantPoolBase.sol /// @audit depositETH(), withdrawETH(), withdrawLPTokens() 10: contract GiantPoolBase is ReentrancyGuard {
File: contracts/liquid-staking/GiantSavETHVaultPool.sol /// @audit batchDepositETHForStaking(), withdrawDETH(), batchRotateLPTokens(), bringUnusedETHBackIntoGiantPool() 13: contract GiantSavETHVaultPool is StakehouseAPI, GiantPoolBase {
File: contracts/liquid-staking/LiquidStakingManager.sol /// @audit executeAsSmartWallet(), deRegisterKnotFromSyndicate(), restoreFreeFloatingSharesToSmartWalletForRageQuit(), updateDAOAddress(), updateDAORevenueCommission(), updateTicker(), updateWhitelisting(), updateNodeRunnerWhitelistStatus(), rotateEOARepresentative(), rotateEOARepresentativeOfNodeRunner(), withdrawETHForKnot(), rotateNodeRunnerOfSmartWallet(), claimRewardsAsNodeRunner(), registerBLSPublicKeys(), isBLSPublicKeyPartOfLSDNetwork(), isBLSPublicKeyBanned(), isNodeRunnerBanned(), stake(), mintDerivatives(), getNetworkFeeRecipient() 33: contract LiquidStakingManager is ILiquidStakingManager, Initializable, ReentrancyGuard, StakehouseAPI {
File: contracts/liquid-staking/LPTokenFactory.sol /// @audit deployLPToken() 9: contract LPTokenFactory {
File: contracts/liquid-staking/LPToken.sol /// @audit liquidStakingManager() 11: contract LPToken is ILPTokenInit, ILiquidStakingManagerChildContract, Initializable, ERC20PermitUpgradeable {
File: contracts/liquid-staking/LSDNFactory.sol /// @audit deployNewLiquidStakingDerivativeNetwork() 9: contract LSDNFactory {
File: contracts/liquid-staking/OptionalGatekeeperFactory.sol /// @audit deploy() 7: contract OptionalGatekeeperFactory {
File: contracts/liquid-staking/SavETHVaultDeployer.sol /// @audit deploySavETHVault() 9: contract SavETHVaultDeployer {
File: contracts/liquid-staking/SavETHVault.sol /// @audit init(), batchDepositETHForStaking(), depositETHForStaking(), burnLPTokensByBLS(), burnLPTokens(), burnLPToken(), withdrawETHForStaking(), isBLSPublicKeyPartOfLSDNetwork(), isBLSPublicKeyBanned(), isDETHReadyForWithdrawal() 16: contract SavETHVault is Initializable, ETHPoolLPFactory, ReentrancyGuard {
File: contracts/liquid-staking/StakingFundsVaultDeployer.sol /// @audit deployStakingFundsVault() 9: contract StakingFundsVaultDeployer {
File: contracts/liquid-staking/StakingFundsVault.sol /// @audit init(), updateDerivativesMinted(), updateAccumulatedETHPerLP(), batchDepositETHForStaking(), depositETHForStaking(), burnLPTokensForETHByBLS(), burnLPTokensForETH(), burnLPForETH(), claimRewards(), withdrawETH(), unstakeSyndicateSharesForRageQuit(), batchPreviewAccumulatedETHByBLSKeys(), batchPreviewAccumulatedETH(), previewAccumulatedETH(), claimFundsFromSyndicateForDistribution() 20: contract StakingFundsVault is
File: contracts/liquid-staking/SyndicateRewardsProcessor.sol /// @audit totalRewardsReceived() 6: abstract contract SyndicateRewardsProcessor {
File: contracts/smart-wallet/OwnableSmartWalletFactory.sol /// @audit createWallet(), createWallet() 11: contract OwnableSmartWalletFactory is IOwnableSmartWalletFactory {
File: contracts/syndicate/Syndicate.sol /// @audit registerKnotsToSyndicate(), deRegisterKnots(), addPriorityStakers(), updatePriorityStakingBlock(), updateAccruedETHPerShares(), stake(), unstake(), claimAsStaker(), claimAsCollateralizedSLOTOwner(), updateCollateralizedSlotOwnersAccruedETH(), batchUpdateCollateralizedSlotOwnersAccruedETH(), calculateUnclaimedFreeFloatingETHShare(), calculateETHForFreeFloatingOrCollateralizedHolders(), previewUnclaimedETHAsFreeFloatingStaker(), previewUnclaimedETHAsCollateralizedSlotOwner(), getUnprocessedETHForAllFreeFloatingSlot(), getUnprocessedETHForAllCollateralizedSlot(), calculateNewAccumulatedETHPerFreeFloatingShare(), calculateNewAccumulatedETHPerCollateralizedSharePerKnot(), totalETHReceived() 36: contract Syndicate is ISyndicateInit, Initializable, Ownable, ReentrancyGuard, StakehouseAPI {
internal functions not called by the contract should be removed to save deployment gasIf the functions are required by an interface, the contract should inherit from that interface and use the override keyword
There are 2 instances of this issue:
File: contracts/syndicate/Syndicate.sol 538: function _calculateCollateralizedETHOwedPerKnot() internal view returns (uint256) { 545: function _calculateNewAccumulatedETHPerCollateralizedShare(uint256 _ethSinceLastUpdate) internal view returns (uint256) {
if (<x> == true) => if (<x>), if (<x> == false) => if (!<x>)
There are 19 instances of this issue:
File: contracts/liquid-staking/GiantSavETHVaultPool.sol 150: vault.isDETHReadyForWithdrawal(address(_lpTokens[i][j])) == false,
File: contracts/liquid-staking/LiquidStakingManager.sol 291: require(isNodeRunnerBanned(msg.sender) == false, "Node runner is banned from LSD network"); 328: require(isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key has already withdrawn or not a part of LSD network"); 332: require(isNodeRunnerBanned(nodeRunnerOfSmartWallet[associatedSmartWallet]) == false, "Node runner is banned from LSD network"); 391: require(isBLSPublicKeyBanned(_blsPubKeys[i]) == false, "BLS public key is banned or not a part of LSD network"); 434: require(_isNodeRunnerValid(msg.sender) == true, "Unrecognised node runner"); 435: require(isNodeRunnerBanned(msg.sender) == false, "Node runner is banned from LSD network"); 467: require(isBLSPublicKeyPartOfLSDNetwork(_blsPublicKey) == false, "BLS public key is banned or not a part of LSD network"); 532: require(isBLSPublicKeyBanned(blsPubKey) == false, "BLS public key is banned or not a part of LSD network"); 580: require(isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is banned or not a part of LSD network"); 679: require(isNodeRunnerWhitelisted[_nodeRunner] == true, "Invalid node runner");
File: contracts/liquid-staking/SavETHVault.sol 64: require(liquidStakingManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is not part of LSD network"); 84: require(liquidStakingManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key is banned or not a part of LSD network");
File: contracts/liquid-staking/StakingFundsVault.sol 79: require(liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is not part of LSD network"); 114: require(liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key is banned or not a part of LSD network"); 205: liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPubKeys[i]) == false,
File: contracts/smart-wallet/OwnableSmartWallet.sol 145: return from == to ? true : _isTransferApproved[from][to]; // F: [OSW-2, 3]
File: contracts/syndicate/Syndicate.sol 611: if (isKnotRegistered[_blsPublicKey] == false) revert KnotIsNotRegisteredWithSyndicate(); 612: if (isNoLongerPartOfSyndicate[_blsPublicKey] == true) revert KnotHasAlreadyBeenDeRegistered();
z = (x == y) ? true : false => z = (x == y)
There is 1 instance of this issue:
File: contracts/smart-wallet/OwnableSmartWallet.sol 145: return from == to ? true : _isTransferApproved[from][to]; // F: [OSW-2, 3]
<x> / 2 is the same as <x> >> 1. While the compiler uses the SHR opcode to accomplish both, the version that uses division incurs an overhead of 20 gas due to JUMPs to and from a compiler utility function that introduces checks which can be avoided by using unchecked {} around the division by two
There is 1 instance of this issue:
File: contracts/syndicate/Syndicate.sol 378: return ethPerKnot / 2;
If the variable is only accessed once, it's cheaper to use the state variable directly that one time, and save the 3 gas the extra stack assignment would spend
There is 1 instance of this issue:
File: contracts/syndicate/Syndicate.sol 388: uint256 currentAccumulatedETHPerFreeFloatingShare = accumulatedETHPerFreeFloatingShare;
The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting. If the contract is meant to be extended, the contract should be abstract and the function signatures be added without any default implementation. If the block is an empty if-statement block to avoid doing subsequent checks in the else-if/else conditions, the else-if/else conditions should be nested under the negation of the if-statement, because they involve different classes of checks, which may lead to the introduction of errors when the code is later modified (if(x){}else if(y){...}else{...} => if(!x){if(y){...}else{...}}). Empty receive()/fallback() payable functions that are not used, can be removed to save deployment gas.
There is 1 instance of this issue:
File: contracts/syndicate/Syndicate.sol 194 } else { 195 // todo - check else case for any ETH lost 196: }
revert()/require() strings to save gasCustom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hit by avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas
There are 21 instances of this issue:
File: contracts/liquid-staking/ETHPoolLPFactory.sol 91 require( 92 getAccountManager().blsPublicKeyToLifecycleStatus(blsPublicKeyOfPreviousKnot) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, 93 "Lifecycle status must be one" 94: ); 96 require( 97 getAccountManager().blsPublicKeyToLifecycleStatus(blsPublicKeyOfNewKnot) ==IDataStructures.LifecycleStatus.INITIALS_REGISTERED, 98 "Lifecycle status must be one" 99: );
File: contracts/liquid-staking/GiantMevAndFeesPool.sol 43 require( 44 liquidStakingDerivativeFactory.isLiquidStakingManager(address(sfv.liquidStakingNetworkManager())), 45 "Invalid liquid staking manager" 46: );
File: contracts/liquid-staking/GiantSavETHVaultPool.sol 49 require( 50 liquidStakingDerivativeFactory.isLiquidStakingManager(address(savETHPool.liquidStakingManager())), 51 "Invalid liquid staking manager" 52: ); 149 require( 150 vault.isDETHReadyForWithdrawal(address(_lpTokens[i][j])) == false, 151 "ETH is either staked or derivatives minted" 152: );
File: contracts/liquid-staking/LiquidStakingManager.sol 334 require( 335 getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKeyOfKnot) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, 336 "Initials not registered" 337: ); 469 require( 470 getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKey) == IDataStructures.LifecycleStatus.UNBEGUN, 471 "Lifecycle status must be zero" 472: ); 536 require( 537 getAccountManager().blsPublicKeyToLifecycleStatus(blsPubKey) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, 538 "Initials not registered" 539: ); 583 require( 584 getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKeyOfKnots[i]) == IDataStructures.LifecycleStatus.DEPOSIT_COMPLETED, 585 "Lifecycle status must be two" 586: );
File: contracts/liquid-staking/SavETHVault.sol 65 require( 66 getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKeyOfKnots[i]) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, 67 "Lifecycle status must be one" 68: ); 85 require( 86 getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKeyOfKnot) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, 87 "Lifecycle status must be one" 88: ); 134 require( 135 validatorStatus == IDataStructures.LifecycleStatus.INITIALS_REGISTERED || 136 validatorStatus == IDataStructures.LifecycleStatus.TOKENS_MINTED, 137 "Cannot burn LP tokens" 138: );
File: contracts/liquid-staking/StakingFundsVault.sol 80 require( 81 getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKeyOfKnots[i]) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, 82 "Lifecycle status must be one" 83: ); 115 require( 116 getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKeyOfKnot) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, 117 "Lifecycle status must be one" 118: ); 180 require( 181 getAccountManager().blsPublicKeyToLifecycleStatus(blsPublicKeyOfKnot) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, 182 "Cannot burn LP tokens" 183: ); 204 require( 205 liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPubKeys[i]) == false, 206 "Unknown BLS public key" 207: ); 210 require( 211 getAccountManager().blsPublicKeyToLifecycleStatus(_blsPubKeys[i]) == IDataStructures.LifecycleStatus.TOKENS_MINTED, 212 "Derivatives not minted" 213: );
File: contracts/smart-wallet/OwnableSmartWalletFactory.sol 37: require(owner != address(0), 'Wallet cannot be address 0');
File: contracts/smart-wallet/OwnableSmartWallet.sol 33 require( 34 initialOwner != address(0), 35 "OwnableSmartWallet: Attempting to initialize with zero address owner" 36: ); 100 require( 101 isTransferApproved(owner(), msg.sender), 102 "OwnableSmartWallet: Transfer is not allowed" 103: ); // F: [OSW-4] 115 require( 116 to != address(0), 117 "OwnableSmartWallet: Approval cannot be set for zero address" 118: ); // F: [OSW-2A]
payableIf a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are
CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost
There are 20 instances of this issue:
File: contracts/liquid-staking/LiquidStakingManager.sol 218: function deRegisterKnotFromSyndicate(bytes[] calldata _blsPublicKeys) external onlyDAO { 226 function restoreFreeFloatingSharesToSmartWalletForRageQuit( 227 address _smartWallet, 228 bytes[] calldata _blsPublicKeys, 229 uint256[] calldata _amounts 230: ) external onlyDAO { 239: function updateDAOAddress(address _newAddress) external onlyDAO { 249: function updateDAORevenueCommission(uint256 _commissionPercentage) external onlyDAO { 255: function updateTicker(string calldata _newTicker) external onlyDAO { 267: function updateWhitelisting(bool _changeWhitelist) external onlyDAO returns (bool) { 278: function updateNodeRunnerWhitelistStatus(address _nodeRunner, bool isWhitelisted) external onlyDAO { 308: function rotateEOARepresentativeOfNodeRunner(address _nodeRunner, address _newRepresentative) external onlyDAO { 357: function rotateNodeRunnerOfSmartWallet(address _current, address _new, bool _wasPreviousNodeRunnerMalicious) external onlyDAO {
File: contracts/liquid-staking/LPToken.sol 46: function mint(address _recipient, uint256 _amount) external onlyDeployer { 51: function burn(address _recipient, uint256 _amount) external onlyDeployer {
File: contracts/liquid-staking/SavETHVault.sol 200 function withdrawETHForStaking( 201 address _smartWallet, 202 uint256 _amount 203: ) public onlyManager nonReentrant returns (uint256) {
File: contracts/liquid-staking/StakingFundsVault.sol 56: function updateDerivativesMinted() external onlyManager { 239: function withdrawETH(address _wallet, uint256 _amount) public onlyManager nonReentrant returns (uint256) { 255 function unstakeSyndicateSharesForRageQuit( 256 address _sETHRecipient, 257 bytes[] calldata _blsPublicKeys, 258 uint256[] calldata _amounts 259: ) external onlyManager nonReentrant {
File: contracts/smart-wallet/OwnableSmartWallet.sol 114: function setApproval(address to, bool status) external onlyOwner override {
File: contracts/syndicate/Syndicate.sol 145 function registerKnotsToSyndicate( 146 bytes[] calldata _newBLSPublicKeyBeingRegistered 147: ) external onlyOwner { 154: function deRegisterKnots(bytes[] calldata _blsPublicKeys) external onlyOwner { 161: function addPriorityStakers(address[] calldata _priorityStakers) external onlyOwner { 168: function updatePriorityStakingBlock(uint256 _endBlock) external onlyOwner {
These findings are excluded from awards calculations because there are publicly-available automated tools that find them. The valid ones appear here for completeness
| Issue | Instances | Total Gas Saved | |
|---|---|---|---|
| [Gā01] | <array>.length should not be looked up in every loop of a for-loop | 16 | 48 |
| [Gā02] | Using bools for storage incurs overhead | 9 | 153900 |
| [Gā03] | ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) | 1 | 5 |
| [Gā04] | Using private rather than public for constants, saves gas | 4 | - |
| [Gā05] | Use custom errors rather than revert()/require() strings to save gas | 198 | - |
Total: 228 instances over 5 issues with 153953 gas saved
Gas totals use lower bounds of ranges and count two iterations of each for-loop. All values above are runtime, not deployment, values; deployment values are listed in the individual issue descriptions. The table above as well as its gas numbers do not include any of the excluded findings.
<array>.length should not be looked up in every loop of a for-loopThe overheads outlined below are PER LOOP, excluding the first loop
MLOAD (3 gas)CALLDATALOAD (3 gas)Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset
There are 16 instances of this issue:
File: contracts/liquid-staking/GiantMevAndFeesPool.sol /// @audit (valid but excluded finding) 90: for (uint256 i; i < _stakingFundsVaults.length; ++i) { /// @audit (valid but excluded finding) 183: for (uint256 i; i < _lpTokens.length; ++i) {
File: contracts/liquid-staking/GiantSavETHVaultPool.sol /// @audit (valid but excluded finding) 82: for (uint256 j; j < _lpTokens[i].length; ++j) { /// @audit (valid but excluded finding) 148: for (uint256 j; j < _lpTokens[i].length; ++j) {
File: contracts/liquid-staking/LiquidStakingManager.sol /// @audit (valid but excluded finding) 390: for(uint256 i; i < _blsPubKeys.length; ++i) {
File: contracts/liquid-staking/StakingFundsVault.sol /// @audit (valid but excluded finding) 203: for (uint256 i; i < _blsPubKeys.length; ++i) { /// @audit (valid but excluded finding) 266: for (uint256 i; i < _blsPublicKeys.length; ++i) { /// @audit (valid but excluded finding) 276: for (uint256 i; i < _blsPubKeys.length; ++i) { /// @audit (valid but excluded finding) 286: for (uint256 i; i < _token.length; ++i) {
File: contracts/syndicate/Syndicate.sol /// @audit (valid but excluded finding) 211: for (uint256 i; i < _blsPubKeys.length; ++i) { /// @audit (valid but excluded finding) 259: for (uint256 i; i < _blsPubKeys.length; ++i) { /// @audit (valid but excluded finding) 301: for (uint256 i; i < _blsPubKeys.length; ++i) { /// @audit (valid but excluded finding) 346: for (uint256 i; i < _blsPubKeys.length; ++i) { /// @audit (valid but excluded finding) 585: for (uint256 i; i < _priorityStakers.length; ++i) { /// @audit (valid but excluded finding) 598: for (uint256 i; i < _blsPublicKeys.length; ++i) { /// @audit (valid but excluded finding) 648: for (uint256 i; i < _blsPubKeys.length; ++i) {
bools for storage incurs overhead// Booleans are more expensive than uint256 or any type that takes up a full // word because each write operation emits an extra SLOAD to first read the // slot's contents, replace the bits taken up by the boolean, and then write // back. This is the compiler's defense against contract upgrades and // pointer aliasing, and it cannot be disabled.
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27
Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from false to true, after having been true in the past
There are 9 instances of this issue:
File: contracts/liquid-staking/LiquidStakingManager.sol /// @audit (valid but excluded finding) 120: bool public enableWhitelisting; /// @audit (valid but excluded finding) 123: mapping(address => bool) public isNodeRunnerWhitelisted; /// @audit (valid but excluded finding) 149: mapping(address => bool) public bannedNodeRunners;
File: contracts/liquid-staking/LSDNFactory.sol /// @audit (valid but excluded finding) 39: mapping(address => bool) public isLiquidStakingManager;
File: contracts/smart-wallet/OwnableSmartWalletFactory.sol /// @audit (valid but excluded finding) 20: mapping(address => bool) public walletExists;
File: contracts/smart-wallet/OwnableSmartWallet.sol /// @audit (valid but excluded finding) 22: mapping(address => mapping(address => bool)) internal _isTransferApproved;
File: contracts/syndicate/Syndicate.sol /// @audit (valid but excluded finding) 90: mapping(bytes => bool) public isKnotRegistered; /// @audit (valid but excluded finding) 96: mapping(address => bool) public isPriorityStaker; /// @audit (valid but excluded finding) 117: mapping(bytes => bool) public isNoLongerPartOfSyndicate;
++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)Saves 5 gas per loop
There is 1 instance of this issue:
File: contracts/liquid-staking/ETHPoolLPFactory.sol /// @audit (valid but excluded finding) 141: numberOfLPTokensIssued++;
private rather than public for constants, saves gasIf needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table
There are 4 instances of this issue:
File: contracts/liquid-staking/ETHPoolLPFactory.sol /// @audit (valid but excluded finding) 38: uint256 public constant MIN_STAKING_AMOUNT = 0.001 ether;
File: contracts/liquid-staking/GiantPoolBase.sol /// @audit (valid but excluded finding) 22: uint256 public constant MIN_STAKING_AMOUNT = 0.001 ether;
File: contracts/liquid-staking/SyndicateRewardsProcessor.sol /// @audit (valid but excluded finding) 15: uint256 public constant PRECISION = 1e24;
File: contracts/syndicate/Syndicate.sol /// @audit (valid but excluded finding) 66: uint256 public constant PRECISION = 1e24;
revert()/require() strings to save gasCustom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hit by avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas
There are 198 instances of this issue:
File: contracts/liquid-staking/ETHPoolLPFactory.sol /// @audit (valid but excluded finding) 59: require(numOfRotations > 0, "Empty arrays"); /// @audit (valid but excluded finding) 60: require(numOfRotations == _newLPTokens.length, "Inconsistent arrays"); /// @audit (valid but excluded finding) 61: require(numOfRotations == _amounts.length, "Inconsistent arrays"); /// @audit (valid but excluded finding) 77: require(address(_oldLPToken) != address(0), "Zero address"); /// @audit (valid but excluded finding) 78: require(address(_newLPToken) != address(0), "Zero address"); /// @audit (valid but excluded finding) 79: require(_oldLPToken != _newLPToken, "Incorrect rotation to same token"); /// @audit (valid but excluded finding) 80: require(_amount >= MIN_STAKING_AMOUNT, "Amount cannot be zero"); /// @audit (valid but excluded finding) 81: require(_amount <= _oldLPToken.balanceOf(msg.sender), "Not enough balance"); /// @audit (valid but excluded finding) 82: require(_oldLPToken.lastInteractedTimestamp(msg.sender) + 30 minutes < block.timestamp, "Liquidity is still fresh"); /// @audit (valid but excluded finding) 83: require(_amount + _newLPToken.totalSupply() <= 24 ether, "Not enough mintable tokens"); /// @audit (valid but excluded finding) 88: require(blsPublicKeyOfPreviousKnot.length == 48, "Incorrect BLS public key"); /// @audit (valid but excluded finding) 89: require(blsPublicKeyOfNewKnot.length == 48, "Incorrect BLS public key"); /// @audit (valid but excluded finding) 111: require(_amount >= MIN_STAKING_AMOUNT, "Min amount not reached"); /// @audit (valid but excluded finding) 112: require(_blsPublicKeyOfKnot.length == 48, "Invalid BLS public key"); /// @audit (valid but excluded finding) 122: require(lpToken.totalSupply() + _amount <= maxStakingAmountPerValidator, "Amount exceeds the staking limit for the validator");
File: contracts/liquid-staking/GiantLP.sol /// @audit (valid but excluded finding) 30: require(msg.sender == pool, "Only pool"); /// @audit (valid but excluded finding) 35: require(msg.sender == pool, "Only pool");
File: contracts/liquid-staking/GiantMevAndFeesPool.sol /// @audit (valid but excluded finding) 35: require(numOfVaults > 0, "Zero vaults"); /// @audit (valid but excluded finding) 36: require(numOfVaults == _blsPublicKeyOfKnots.length, "Inconsistent lengths"); /// @audit (valid but excluded finding) 37: require(numOfVaults == _amounts.length, "Inconsistent lengths"); /// @audit (valid but excluded finding) 62: require(numOfVaults > 0, "Empty array"); /// @audit (valid but excluded finding) 63: require(numOfVaults == _blsPublicKeysForKnots.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 87: require(_stakingFundsVaults.length == _lpTokens.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 112: require(numOfRotations > 0, "Empty arrays"); /// @audit (valid but excluded finding) 113: require(numOfRotations == _oldLPTokens.length, "Inconsistent arrays"); /// @audit (valid but excluded finding) 114: require(numOfRotations == _newLPTokens.length, "Inconsistent arrays"); /// @audit (valid but excluded finding) 115: require(numOfRotations == _amounts.length, "Inconsistent arrays"); /// @audit (valid but excluded finding) 116: require(lpTokenETH.balanceOf(msg.sender) >= 0.5 ether, "No common interest"); /// @audit (valid but excluded finding) 132: require(numOfVaults > 0, "Empty arrays"); /// @audit (valid but excluded finding) 133: require(numOfVaults == _lpTokens.length, "Inconsistent arrays"); /// @audit (valid but excluded finding) 134: require(numOfVaults == _amounts.length, "Inconsistent arrays"); /// @audit (valid but excluded finding) 147: require(msg.sender == address(lpTokenETH), "Caller is not giant LP"); /// @audit (valid but excluded finding) 171: require(msg.sender == address(lpTokenETH), "Caller is not giant LP");
File: contracts/liquid-staking/GiantPoolBase.sol /// @audit (valid but excluded finding) 35: require(msg.value >= MIN_STAKING_AMOUNT, "Minimum not supplied"); /// @audit (valid but excluded finding) 36: require(msg.value == _amount, "Value equal to amount"); /// @audit (valid but excluded finding) 53: require(_amount >= MIN_STAKING_AMOUNT, "Invalid amount"); /// @audit (valid but excluded finding) 54: require(lpTokenETH.balanceOf(msg.sender) >= _amount, "Invalid balance"); /// @audit (valid but excluded finding) 55: require(idleETH >= _amount, "Come back later or withdraw less ETH"); /// @audit (valid but excluded finding) 61: require(success, "Failed to transfer ETH"); /// @audit (valid but excluded finding) 71: require(amountOfTokens > 0, "Empty arrays"); /// @audit (valid but excluded finding) 72: require(amountOfTokens == _amounts.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 94: require(_amount >= MIN_STAKING_AMOUNT, "Invalid amount"); /// @audit (valid but excluded finding) 95: require(_token.balanceOf(address(this)) >= _amount, "Pool does not own specified LP"); /// @audit (valid but excluded finding) 96: require(lpTokenETH.lastInteractedTimestamp(msg.sender) + 1 days < block.timestamp, "Too new");
File: contracts/liquid-staking/GiantSavETHVaultPool.sol /// @audit (valid but excluded finding) 36: require(numOfSavETHVaults > 0, "Empty arrays"); /// @audit (valid but excluded finding) 37: require(numOfSavETHVaults == _ETHTransactionAmounts.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 38: require(numOfSavETHVaults == _blsPublicKeys.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 39: require(numOfSavETHVaults == _stakeAmounts.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 72: require(numOfVaults > 0, "Empty arrays"); /// @audit (valid but excluded finding) 73: require(numOfVaults == _lpTokens.length, "Inconsistent arrays"); /// @audit (valid but excluded finding) 74: require(numOfVaults == _amounts.length, "Inconsistent arrays"); /// @audit (valid but excluded finding) 89: require(vault.isDETHReadyForWithdrawal(address(token)), "dETH is not ready for withdrawal"); /// @audit (valid but excluded finding) 92: require(lpTokenETH.balanceOf(msg.sender) >= amount, "User does not own enough LP"); /// @audit (valid but excluded finding) 123: require(numOfRotations > 0, "Empty arrays"); /// @audit (valid but excluded finding) 124: require(numOfRotations == _oldLPTokens.length, "Inconsistent arrays"); /// @audit (valid but excluded finding) 125: require(numOfRotations == _newLPTokens.length, "Inconsistent arrays"); /// @audit (valid but excluded finding) 126: require(numOfRotations == _amounts.length, "Inconsistent arrays"); /// @audit (valid but excluded finding) 127: require(lpTokenETH.balanceOf(msg.sender) >= 0.5 ether, "No common interest"); /// @audit (valid but excluded finding) 143: require(numOfVaults > 0, "Empty arrays"); /// @audit (valid but excluded finding) 144: require(numOfVaults == _lpTokens.length, "Inconsistent arrays"); /// @audit (valid but excluded finding) 145: require(numOfVaults == _amounts.length, "Inconsistent arrays");
File: contracts/liquid-staking/LiquidStakingManager.sol /// @audit (valid but excluded finding) 161: require(msg.sender == dao, "Must be DAO"); /// @audit (valid but excluded finding) 209: require(smartWallet != address(0), "No wallet found"); /// @audit (valid but excluded finding) 240: require(_newAddress != address(0), "Zero address"); /// @audit (valid but excluded finding) 241: require(_newAddress != dao, "Same address"); /// @audit (valid but excluded finding) 250: require(_commissionPercentage != daoCommissionPercentage, "Same commission percentage"); /// @audit (valid but excluded finding) 256: require(bytes(_newTicker).length >= 3, "String must be 3-5 characters long"); /// @audit (valid but excluded finding) 257: require(bytes(_newTicker).length <= 5, "String must be 3-5 characters long"); /// @audit (valid but excluded finding) 258: require(numberOfKnots == 0, "Cannot change ticker once house is created"); /// @audit (valid but excluded finding) 268: require(_changeWhitelist != enableWhitelisting, "Unnecessary update to same status"); /// @audit (valid but excluded finding) 279: require(_nodeRunner != address(0), "Zero address"); /// @audit (valid but excluded finding) 280: require(isNodeRunnerWhitelisted[_nodeRunner] != isNodeRunnerWhitelisted[_nodeRunner], "Unnecessary update to same status"); /// @audit (valid but excluded finding) 290: require(_newRepresentative != address(0), "Zero address"); /// @audit (valid but excluded finding) 291: require(isNodeRunnerBanned(msg.sender) == false, "Node runner is banned from LSD network"); /// @audit (valid but excluded finding) 294: require(smartWallet != address(0), "No smart wallet"); /// @audit (valid but excluded finding) 295: require(stakedKnotsOfSmartWallet[smartWallet] == 0, "Not all KNOTs are minted"); /// @audit (valid but excluded finding) 296: require(smartWalletRepresentative[smartWallet] != _newRepresentative, "Invalid rotation to same EOA"); /// @audit (valid but excluded finding) 309: require(_newRepresentative != address(0), "Zero address"); /// @audit (valid but excluded finding) 312: require(smartWallet != address(0), "No smart wallet"); /// @audit (valid but excluded finding) 313: require(stakedKnotsOfSmartWallet[smartWallet] == 0, "Not all KNOTs are minted"); /// @audit (valid but excluded finding) 314: require(smartWalletRepresentative[smartWallet] != _newRepresentative, "Invalid rotation to same EOA"); /// @audit (valid but excluded finding) 327: require(_recipient != address(0), "Zero address"); /// @audit (valid but excluded finding) 328: require(isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key has already withdrawn or not a part of LSD network"); /// @audit (valid but excluded finding) 331: require(smartWalletOfNodeRunner[msg.sender] == associatedSmartWallet, "Not the node runner for the smart wallet "); /// @audit (valid but excluded finding) 332: require(isNodeRunnerBanned(nodeRunnerOfSmartWallet[associatedSmartWallet]) == false, "Node runner is banned from LSD network"); /// @audit (valid but excluded finding) 333: require(associatedSmartWallet.balance >= 4 ether, "Insufficient balance"); /// @audit (valid but excluded finding) 359: require(wallet != address(0), "Wallet does not exist"); /// @audit (valid but excluded finding) 362: require(newRunnerCurrentWallet == address(0), "New runner has a wallet"); /// @audit (valid but excluded finding) 384: require(_blsPubKeys.length > 0, "No BLS keys specified"); /// @audit (valid but excluded finding) 385: require(_recipient != address(0), "Zero address"); /// @audit (valid but excluded finding) 388: require(smartWallet != address(0), "Unknown node runner"); /// @audit (valid but excluded finding) 391: require(isBLSPublicKeyBanned(_blsPubKeys[i]) == false, "BLS public key is banned or not a part of LSD network"); /// @audit (valid but excluded finding) 394: require(smartWalletOfKnot[_blsPubKeys[i]] == smartWallet, "BLS public key doesn't belong to the node runner"); /// @audit (valid but excluded finding) 410: require(transferResult, "Failed to transfer"); /// @audit (valid but excluded finding) 414: require(transferResult, "Failed to transfer"); /// @audit (valid but excluded finding) 430: require(len >= 1, "No value provided"); /// @audit (valid but excluded finding) 431: require(len == _blsSignatures.length, "Unequal number of array values"); /// @audit (valid but excluded finding) 432: require(msg.value == len * 4 ether, "Insufficient ether provided"); /// @audit (valid but excluded finding) 433: require(!Address.isContract(_eoaRepresentative), "Only EOA representative permitted"); /// @audit (valid but excluded finding) 434: require(_isNodeRunnerValid(msg.sender) == true, "Unrecognised node runner"); /// @audit (valid but excluded finding) 435: require(isNodeRunnerBanned(msg.sender) == false, "Node runner is banned from LSD network"); /// @audit (valid but excluded finding) 453: require(smartWalletRepresentative[smartWallet] == _eoaRepresentative, "Different EOA specified - rotate outside"); /// @audit (valid but excluded finding) 459: require(result, "Transfer failed"); /// @audit (valid but excluded finding) 467: require(isBLSPublicKeyPartOfLSDNetwork(_blsPublicKey) == false, "BLS public key is banned or not a part of LSD network"); /// @audit (valid but excluded finding) 523: require(numOfValidators > 0, "No data"); /// @audit (valid but excluded finding) 524: require(numOfValidators == _ciphertexts.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 525: require(numOfValidators == _aesEncryptorKeys.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 526: require(numOfValidators == _encryptionSignatures.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 527: require(numOfValidators == _dataRoots.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 532: require(isBLSPublicKeyBanned(blsPubKey) == false, "BLS public key is banned or not a part of LSD network"); /// @audit (valid but excluded finding) 535: require(associatedSmartWallet != address(0), "Unknown BLS public key"); /// @audit (valid but excluded finding) 574: require(numOfKnotsToProcess > 0, "Empty array"); /// @audit (valid but excluded finding) 575: require(numOfKnotsToProcess == _beaconChainBalanceReports.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 576: require(numOfKnotsToProcess == _reportSignatures.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 580: require(isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is banned or not a part of LSD network"); /// @audit (valid but excluded finding) 649: require(_dao != address(0), "Zero address"); /// @audit (valid but excluded finding) 650: require(_syndicateFactory != address(0), "Zero address"); /// @audit (valid but excluded finding) 651: require(_smartWalletFactory != address(0), "Zero address"); /// @audit (valid but excluded finding) 652: require(_brand != address(0), "Zero address"); /// @audit (valid but excluded finding) 653: require(bytes(_stakehouseTicker).length >= 3, "String must be 3-5 characters long"); /// @audit (valid but excluded finding) 654: require(bytes(_stakehouseTicker).length <= 5, "String must be 3-5 characters long"); /// @audit (valid but excluded finding) 676: require(_nodeRunner != address(0), "Zero address"); /// @audit (valid but excluded finding) 679: require(isNodeRunnerWhitelisted[_nodeRunner] == true, "Invalid node runner"); /// @audit (valid but excluded finding) 913: require(_received > 0, "Nothing received"); /// @audit (valid but excluded finding) 927: require(associatedSmartWallet.balance >= 4 ether, "Smart wallet balance must be at least 4 ether"); /// @audit (valid but excluded finding) 930: require(address(stakingFundsLP) != address(0), "No funds staked in staking funds vault"); /// @audit (valid but excluded finding) 931: require(stakingFundsLP.totalSupply() == 4 ether, "DAO staking funds vault balance must be at least 4 ether"); /// @audit (valid but excluded finding) 934: require(address(savETHVaultLP) != address(0), "No funds staked in savETH vault"); /// @audit (valid but excluded finding) 935: require(savETHVaultLP.totalSupply() == 24 ether, "KNOT must have 24 ETH in savETH vault"); /// @audit (valid but excluded finding) 940: require(_commissionPercentage <= MODULO, "Invalid commission");
File: contracts/liquid-staking/LPTokenFactory.sol /// @audit (valid but excluded finding) 19: require(_lpTokenImplementation != address(0), "Address cannot be zero"); /// @audit (valid but excluded finding) 33: require(address(_deployer) != address(0), "Zero address"); /// @audit (valid but excluded finding) 34: require(bytes(_tokenSymbol).length != 0, "Symbol cannot be zero"); /// @audit (valid but excluded finding) 35: require(bytes(_tokenName).length != 0, "Name cannot be zero");
File: contracts/liquid-staking/LPToken.sol /// @audit (valid but excluded finding) 23: require(msg.sender == deployer, "Only savETH vault");
File: contracts/liquid-staking/LSDNFactory.sol /// @audit (valid but excluded finding) 51: require(_liquidStakingManagerImplementation != address(0), "Zero Address"); /// @audit (valid but excluded finding) 52: require(_syndicateFactory != address(0), "Zero Address"); /// @audit (valid but excluded finding) 53: require(_lpTokenFactory != address(0), "Zero Address"); /// @audit (valid but excluded finding) 54: require(_smartWalletFactory != address(0), "Zero Address"); /// @audit (valid but excluded finding) 55: require(_brand != address(0), "Zero Address"); /// @audit (valid but excluded finding) 56: require(_savETHVaultDeployer != address(0), "Zero Address"); /// @audit (valid but excluded finding) 57: require(_stakingFundsVaultDeployer != address(0), "Zero Address"); /// @audit (valid but excluded finding) 58: require(_optionalGatekeeperDeployer != address(0), "Zero Address");
File: contracts/liquid-staking/SavETHVault.sol /// @audit (valid but excluded finding) 50: require(msg.sender == address(liquidStakingManager), "Not the savETH vault manager"); /// @audit (valid but excluded finding) 59: require(numOfValidators > 0, "Empty arrays"); /// @audit (valid but excluded finding) 60: require(numOfValidators == _amounts.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 64: require(liquidStakingManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is not part of LSD network"); /// @audit (valid but excluded finding) 76: require(msg.value == totalAmount, "Invalid ETH amount attached"); /// @audit (valid but excluded finding) 84: require(liquidStakingManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key is banned or not a part of LSD network"); /// @audit (valid but excluded finding) 90: require(msg.value == _amount, "Must provide correct amount of ETH"); /// @audit (valid but excluded finding) 101: require(numOfTokens > 0, "Empty arrays"); /// @audit (valid but excluded finding) 102: require(numOfTokens == _amounts.length, "Inconsistent array length"); /// @audit (valid but excluded finding) 114: require(numOfTokens > 0, "Empty arrays"); /// @audit (valid but excluded finding) 115: require(numOfTokens == _amounts.length, "Inconsisent array length"); /// @audit (valid but excluded finding) 127: require(_amount >= MIN_STAKING_AMOUNT, "Amount cannot be zero"); /// @audit (valid but excluded finding) 128: require(_amount <= _lpToken.balanceOf(msg.sender), "Not enough balance"); /// @audit (valid but excluded finding) 161: require(dETHBalance >= 24 ether, "Nothing to withdraw"); /// @audit (valid but excluded finding) 186: require(isStaleLiquidity, "Liquidity is still fresh"); /// @audit (valid but excluded finding) 190: require(result, "Transfer failed"); /// @audit (valid but excluded finding) 204: require(_amount >= 24 ether, "Amount cannot be less than 24 ether"); /// @audit (valid but excluded finding) 205: require(address(this).balance >= _amount, "Insufficient withdrawal amount"); /// @audit (valid but excluded finding) 206: require(_smartWallet != address(0), "Zero address"); /// @audit (valid but excluded finding) 207: require(_smartWallet != address(this), "This address"); /// @audit (valid but excluded finding) 210: require(result, "Transfer failed"); /// @audit (valid but excluded finding) 236: require(_liquidStakingManagerAddress != address(0), "Zero address"); /// @audit (valid but excluded finding) 237: require(address(_lpTokenFactory) != address(0), "Zero address");
File: contracts/liquid-staking/StakingFundsVault.sol /// @audit (valid but excluded finding) 51: require(msg.sender == address(liquidStakingNetworkManager), "Only network manager"); /// @audit (valid but excluded finding) 71: require(numOfValidators > 0, "Empty arrays"); /// @audit (valid but excluded finding) 72: require(numOfValidators == _amounts.length, "Inconsistent array lengths"); /// @audit (valid but excluded finding) 79: require(liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is not part of LSD network"); /// @audit (valid but excluded finding) 107: require(msg.value == totalAmount, "Invalid ETH amount attached"); /// @audit (valid but excluded finding) 114: require(liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key is banned or not a part of LSD network"); /// @audit (valid but excluded finding) 120: require(msg.value == _amount, "Must provide correct amount of ETH"); /// @audit (valid but excluded finding) 150: require(numOfTokens > 0, "Empty arrays"); /// @audit (valid but excluded finding) 151: require(numOfTokens == _amounts.length, "Inconsistent array length"); /// @audit (valid but excluded finding) 154: require(address(token) != address(0), "No ETH staked for specified BLS key"); /// @audit (valid but excluded finding) 164: require(numOfTokens > 0, "Empty arrays"); /// @audit (valid but excluded finding) 165: require(numOfTokens == _amounts.length, "Inconsistent array length"); /// @audit (valid but excluded finding) 175: require(_amount >= MIN_STAKING_AMOUNT, "Amount cannot be zero"); /// @audit (valid but excluded finding) 176: require(_amount <= _lpToken.balanceOf(msg.sender), "Not enough balance"); /// @audit (valid but excluded finding) 177: require(address(_lpToken) != address(0), "Zero address specified"); /// @audit (valid but excluded finding) 184: require(_lpToken.lastInteractedTimestamp(msg.sender) + 30 minutes < block.timestamp, "Too new"); /// @audit (valid but excluded finding) 191: require(result, "Transfer failed"); /// @audit (valid but excluded finding) 229: require(address(token) != address(0), "Invalid BLS key"); /// @audit (valid but excluded finding) 230: require(token.lastInteractedTimestamp(msg.sender) + 30 minutes < block.timestamp, "Last transfer too recent"); /// @audit (valid but excluded finding) 240: require(_amount >= 4 ether, "Amount cannot be less than 4 ether"); /// @audit (valid but excluded finding) 241: require(_amount <= address(this).balance, "Not enough ETH to withdraw"); /// @audit (valid but excluded finding) 242: require(_wallet != address(0), "Zero address"); /// @audit (valid but excluded finding) 245: require(result, "Transfer failed"); /// @audit (valid but excluded finding) 267: require(syndicate.isNoLongerPartOfSyndicate(_blsPublicKeys[i]), "Knot is still active in syndicate"); /// @audit (valid but excluded finding) 320: require(blsPubKey.length > 0, "Invalid token"); /// @audit (valid but excluded finding) 346: require(KnotAssociatedWithLPToken[token].length > 0, "Invalid token"); /// @audit (valid but excluded finding) 361: require(_syndicate != address(0), "Invalid configuration"); /// @audit (valid but excluded finding) 372: require(address(_liquidStakingNetworkManager) != address(0), "Zero Address"); /// @audit (valid but excluded finding) 373: require(address(_lpTokenFactory) != address(0), "Zero Address");
File: contracts/liquid-staking/SyndicateRewardsProcessor.sol /// @audit (valid but excluded finding) 57: require(_recipient != address(0), "Zero address"); /// @audit (valid but excluded finding) 68: require(success, "Failed to transfer");
File: contracts/smart-wallet/OwnableSmartWallet.sol /// @audit (valid but excluded finding) 79: require(result, "Failed to execute");
#0 - c4-judge
2022-12-02T20:03:22Z
dmvt marked the issue as grade-b
#1 - c4-judge
2022-12-03T12:19:56Z
dmvt marked the issue as grade-a
#2 - c4-judge
2022-12-03T12:21:15Z
dmvt marked the issue as selected for report