LSD Network - Stakehouse contest - tnevler's results

Report

Low Risk

[L-1]: Missing checks for address(0x0)

Context:

1. pool = _pool; L25
2. transferHookProcessor = ITransferHookProcessor(_transferHookProcessor); L26 (_transferHookProcessor)

Recommendation:

Add non-zero address checks when set address state variables.

Non-Critical Issues

[N-1]: Use double quotes for string literals

require(owner != address(0), 'Wallet cannot be address 0'); L37

Double quotes are used in all contracts except here.

[N-2]: Use mixedCase

Context:

1. uint256 public MODULO = 100_00000; L158 (variable name must be mixedCase)
2. mapping(LPToken => bytes) public KnotAssociatedWithLPToken; L47 (variable name must be in mixedCase)

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

Context:

1. return (rest, daoAmount); L927
2. return (_received, 0); L930

Recommendation:

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

[N-4]: Wrong order of functions

Context:

1. SyndicateFactory.calculateSyndicateDeploymentAddress (external function can not go after public function)
2. GiantPoolBase.withdrawETH (external function can not go after public function)
3. GiantSavETHVaultPool.withdrawDETH (external function can not go after public function)
4. OwnableSmartWallet.transferOwnership (public function can not go after public view function)
5. OwnableSmartWallet.setApproval (external function can not go after public function)
6. OwnableSmartWallet.isTransferApproved (public function can not go after internal function)
7. OwnableSmartWallet.receive (receive function must go after constructor and before external functions)
8. SavETHVault.burnLPTokensByBLS (external function can not go after public function)
9. SavETHVault.isDETHReadyForWithdrawal (external function can not go after public function)
10. GiantMevAndFeesPool.batchRotateLPTokens (external function can not go after external view function)
11. GiantMevAndFeesPool.beforeTokenTransfer (external function can not go after public function)
12. StakingFundsVault.batchDepositETHForStaking (external function can not go after public function)
13. StakingFundsVault.burnLPTokensForETHByBLS (external function can not go after public function)
14. StakingFundsVault.claimRewards (external function can not go after public function)
15. StakingFundsVault.unstakeSyndicateSharesForRageQuit (external function can not go after public function)
16. StakingFundsVault.beforeTokenTransfer (external function can not go after public function)
17. Syndicate.stake (external function can not go after public function)
18. Syndicate.updateCollateralizedSlotOwnersAccruedETH (external function can not go after public function)
19. Syndicate.receive (receive function must go after constructor and before external functions)
20. Syndicate.previewUnclaimedETHAsFreeFloatingStaker (external function can not go after public function)
21. LiquidStakingManager.stake (external function can not go after public function)
22. LiquidStakingManager.receive (receive function must go after constructor and before external functions)
23. LiquidStakingManager._updateDAORevenueCommission (internal function can not go after internal view function)
24. SyndicateRewardsProcessor.totalRewardsReceived (public function can not go after internal function)
25. SyndicateRewardsProcessor.receive (receive function must go after constructor and before external functions)

Description:

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

• constructor

• receive function (if exists)

• fallback function (if exists)

• external

• public

• internal

• private

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

Recommendation:

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

[N-5]: Public functions can be external

Context:

1. GiantPoolBase.depositETH
2. LSDNFactory.deployNewLiquidStakingDerivativeNetwork
3. GiantSavETHVaultPool.batchDepositETHForStaking
4. SavETHVault.depositETHForStaking
5. SavETHVault.withdrawETHForStaking
6. StakingFundsVault.depositETHForStaking
7. StakingFundsVault.withdrawETH
8. Syndicate.claimAsStaker
9. Syndicate.calculateNewAccumulatedETHPerCollateralizedSharePerKnot
10. LiquidStakingManager.isKnotDeregistered

Description:

Public functions can be declared external if they are not called by the contract.

Recommendation:

Declare these functions as external instead of public.

[N-6]: NatSpec is missing

Context:

1. function createWallet() external returns (address wallet) { L28
2. function createWallet(address owner) external returns (address wallet) { L32
3. function _createWallet(address owner) internal returns (address wallet) { L36
4. function mint(address _recipient, uint256 _amount) external { L29
5. function burn(address _recipient, uint256 _amount) external { L34
6. function _beforeTokenTransfer(address _from, address _to, uint256 _amount) internal override { L39
7. function _afterTokenTransfer(address _from, address _to, uint256 _amount) internal override { L43
8. function init(address _liquidStakingManagerAddress, LPTokenFactory _lpTokenFactory) external virtual initializer { L45
9. function _updateCollateralizedSlotOwnersLiabilitySnapshot(bytes memory _blsPubKey) internal { L491
10. function _calculateCollateralizedETHOwedPerKnot() internal view returns (uint256) { L538
11. function _initStakingFundsVault(address _stakingFundsVaultDeployer, address _tokenFactory) internal virtual { L911

[N-7]: Typos

Context:

1. require(numOfTokens == _amounts.length, "Inconsisent array length"); L115 (Change Inconsisent to Inconsistent)
2. /// @notice Utility function that determins whether an LP can be burned for dETH if the associated derivatives have been minted L227 (Change determins to determines)
3. // register validtor initals for each of the KNOTs L476 (Change validtor to validator)
4. // register validtor initals for each of the KNOTs L476 (Change initals to initials)
5. /// @param _newLPToken Instane of the new LP token (to be minted) L74 (Change Instane to Instance)
6. // mint LP tokens for the depoistor with 1:1 ratio of LP tokens and ETH supplied L124 (Change depoistor to depositor)
7. // mint LP tokens for the depoistor with 1:1 ratio of LP tokens and ETH supplied L150 (Change depoistor to depositor)

[N-8]: TODO

Context:

// todo - check else case for any ETH lost L195

[N-9]: Line is too long

Context:

1. if (address(transferHookProcessor) != address(0)) ITransferHookProcessor(transferHookProcessor).beforeTokenTransfer(_from, _to, _amount); L40
2. if (address(transferHookProcessor) != address(0)) ITransferHookProcessor(transferHookProcessor).afterTokenTransfer(_from, _to, _amount); L46
3. import { ERC20PermitUpgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/draft-ERC20PermitUpgradeable.sol"; L6
4. if (address(transferHookProcessor) != address(0)) transferHookProcessor.beforeTokenTransfer(_from, _to, _amount); L62
5. function batchDepositETHForStaking(bytes[] calldata _blsPublicKeyOfKnots, uint256[] calldata _amounts) external payable { L57
6. require(liquidStakingManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is not part of LSD network"); L64
7. getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKeyOfKnots[i]) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, L66
8. function depositETHForStaking(bytes calldata _blsPublicKeyOfKnot, uint256 _amount) public payable returns (uint256) { L83
9. require(liquidStakingManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key is banned or not a part of LSD network"); L84
10. getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKeyOfKnot) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, L86
11. IDataStructures.LifecycleStatus validatorStatus = getAccountManager().blsPublicKeyToLifecycleStatus(blsPublicKeyOfKnot); L132
12. uint256 dETHBalance = getSavETHRegistry().knotDETHBalanceInIndex(indexOwnedByTheVault, blsPublicKeyOfKnot); L158
13. getSavETHRegistry().addKnotToOpenIndex(liquidStakingManager.stakehouse(), blsPublicKeyOfKnot, address(this)); L165
14. IDataStructures.LifecycleStatus validatorStatus = getAccountManager().blsPublicKeyToLifecycleStatus(blsPublicKeyOfKnot); L230
15. /// @notice Stake ETH against multiple BLS keys within multiple LSDNs and specify the amount of ETH being supplied for each key L22
16. return _previewAccumulatedETH(_user, address(lpTokenETH), lpTokenETH.balanceOf(_user), lpTokenETH.totalSupply(), accumulated); L97
17. function batchDepositETHForStaking(bytes[] calldata _blsPublicKeyOfKnots, uint256[] calldata _amounts) external payable { L69
18. require(liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is not part of LSD network"); L79
19. getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKeyOfKnots[i]) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, L81
20. claimed[msg.sender][address(tokenForKnot)] = (tokenForKnot.balanceOf(msg.sender) * accumulatedETHPerLPShare) / PRECISION; L103
21. function depositETHForStaking(bytes calldata _blsPublicKeyOfKnot, uint256 _amount) public payable returns (uint256) { L113
22. require(liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key is banned or not a part of LSD network"); L114
23. getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKeyOfKnot) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, L116
24. claimed[msg.sender][address(tokenForKnot)] = (tokenForKnot.balanceOf(msg.sender) * accumulatedETHPerLPShare) / PRECISION; L140
25. getAccountManager().blsPublicKeyToLifecycleStatus(blsPublicKeyOfKnot) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, L181
26. /// @notice Any LP tokens for BLS keys that have had their derivatives minted can claim ETH from the syndicate contract L197
27. getAccountManager().blsPublicKeyToLifecycleStatus(_blsPubKeys[i]) == IDataStructures.LifecycleStatus.TOKENS_MINTED, L211
28. if (i == 0 && !Syndicate(payable(liquidStakingNetworkManager.syndicate())).isNoLongerPartOfSyndicate(_blsPubKeys[i])) { L215
29. // If a partial list of BLS keys that have free floating staked are supplied, then partial funds accrued will be fetched L217
30. require(token.lastInteractedTimestamp(msg.sender) + 30 minutes < block.timestamp, "Last transfer too recent"); L230
31. /// @param _blsPublicKeys List of BLS public keys being processed (assuming DAO only has BLS pub keys from correct smart wallet) L253
32. /// @notice Preview total ETH accumulated by a staking funds LP token holder associated with many KNOTs that have minted derivatives L273
33. function batchPreviewAccumulatedETHByBLSKeys(address _user, bytes[] calldata _blsPubKeys) external view returns (uint256) { L274
34. /// @notice Preview total ETH accumulated by a staking funds LP token holder associated with many KNOTs that have minted derivatives L283
35. /// @notice Preview total ETH accumulated by a staking funds LP token holder associated with a KNOT that has minted derivatives L292
36. if (getAccountManager().blsPublicKeyToLifecycleStatus(associatedBLSPublicKeyOfLpToken) != IDataStructures.LifecycleStatus.TOKENS_MINTED) { L296
37. // Looking at this contract balance and the ETH that is yet to be transferred from the syndicate, then tell the user how much ETH they have earned L300
38. if (getAccountManager().blsPublicKeyToLifecycleStatus(blsPubKey) == IDataStructures.LifecycleStatus.TOKENS_MINTED) { L322
39. // in case the new user has existing rewards - give it to them so that the after transfer hook does not wipe pending rewards L336
40. /// @param _blsPubKeys List of BLS public key identifiers of validators that have sETH staked in the syndicate for the vault L354
41. /// @dev This contract can be extended to allow lending and borrowing of time slots for borrower to redeem any revenue generated within the specified window L35
42. event ETHClaimed(bytes BLSPubKey, address indexed user, address recipient, uint256 claim, bool indexed isCollateralizedClaim); L63
43. /// @notice Total accrued ETH for all collateralized SLOT holders per knot which is then distributed based on individual balances L71
44. /// @notice Block number after which if there are sETH staking slots available, it can be supplied by anyone on the market L92
45. /// @notice Syndicate deployer can highlight addresses that get priority for staking free floating house sETH up to a certain block before anyone can do it L95
46. /// @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 L116
47. /// @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 L119
48. /// @param _priorityStakers Optional list of addresses that will have priority for staking sETH against each knot registered L127
49. /// @param _blsPubKeysForSyndicateKnots List of BLS public keys of Stakehouse protocol registered KNOTs participating in syndicate L128
50. /// @notice Make knot shares of a registered list of BLS public keys inactive - the action cannot be undone and no further ETH accrued L153
51. /// @notice Should this block be in the future, it means only those listed in the priority staker list can stake sETH L166
52. /// @notice Update accrued ETH per SLOT share without distributing ETH as users of the syndicate individually pull funds L173
53. // Ensure there are registered KNOTs. Syndicates are deployed with at least 1 registered but this can fall to zero. L175
54. accumulatedETHPerFreeFloatingShare += _calculateNewAccumulatedETHPerFreeFloatingShare(freeFloatingUnprocessed); L185
55. uint256 collateralizedUnprocessed = ((totalEthPerSlotType - lastSeenETHPerCollateralizedSlotPerKnot) / numberOfRegisteredKnots); L189
56. /// @param _sETHAmounts Per BLS public key, the total amount of sETH that will be staked (up to 4 collateralized SLOT per KNOT) L201
57. /// @param _onBehalfOf Allows a caller to specify an address that will be assigned stake ownership and rights to claim L202
58. if (!isKnotRegistered[_blsPubKey] || isNoLongerPartOfSyndicate[_blsPubKey]) revert KnotIsNotRegisteredWithSyndicate(); L216
59. sETHUserClaimForKnot[_blsPubKey][_onBehalfOf] = (_sETHAmount * accumulatedETHPerFreeFloatingShare) / PRECISION; L228
60. // This is designed to cope with falling SLOT balances i.e. when collateralized SLOT is burnt after applying penalties L311
61. /// @notice For any new ETH received by the syndicate, at the knot level allocate ETH owed to each collateralized owner L335
62. /// @notice For any new ETH received by the syndicate, at the knot level allocate ETH owed to each collateralized owner and do it for a batch of knots L341
63. /// @notice Calculate the amount of unclaimed ETH for a given BLS publice key + free floating SLOT staker without factoring in unprocessed rewards L356
64. function calculateUnclaimedFreeFloatingETHShare(bytes memory _blsPubKey, address _user) public view returns (uint256) { L359
65. /// @notice Using `highestSeenBalance`, this is the amount that is separately allocated to either free floating or collateralized SLOT holders L372
66. /// @notice Preview the amount of unclaimed ETH available for an sETH staker against a KNOT which factors in unprocessed rewards from new ETH sent to contract L381
67. currentAccumulatedETHPerFreeFloatingShare + calculateNewAccumulatedETHPerFreeFloatingShare(); L390
68. /// @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 L398
69. + ((calculateETHForFreeFloatingOrCollateralizedHolders() - lastSeenETHPerCollateralizedSlotPerKnot) / numberOfRegisteredKnots); L407
70. uint256 numberOfCollateralisedSlotOwnersForKnot = getSlotRegistry().numberOfCollateralisedSlotOwnersForKnot(_blsPubKey); L416
71. return ((calculateETHForFreeFloatingOrCollateralizedHolders() - lastSeenETHPerCollateralizedSlotPerKnot) / numberOfRegisteredKnots); L447
72. /// 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) L490
73. // Don't allocate ETH when the current slashed amount is four. Syndicate will wait until ETH is topped up to claim revenue L503
74. // This copes with increasing numbers of collateralized slot owners and also copes with SLOT that has been slashed but not topped up L505
75. uint256 numberOfCollateralisedSlotOwnersForKnot = getSlotRegistry().numberOfCollateralisedSlotOwnersForKnot(_blsPubKey); L506
76. accruedEarningPerCollateralizedSlotOwnerOfKnot[_blsPubKey][collateralizedOwnerAtIndex] += unprocessedETHForCurrentKnot; L511
77. // if the knot is no longer active, no further accrual of rewards are possible snapshots are possible but ETH accrued up to that point L531
78. /// @dev Business logic for de-registering a set of knots from the syndicate and doing the required snapshots to ensure historical earnings are preserved L596
79. event ETHWithdrawnFromSmartWallet(address indexed associatedSmartWallet, bytes blsPublicKeyOfKnot, address nodeRunner); L66
80. /// @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 L222
81. /// @param _blsPublicKeys List of BLS public keys being processed (assuming DAO only has BLS pub keys from correct smart wallet) L224
82. require(isNodeRunnerWhitelisted[_nodeRunner] != isNodeRunnerWhitelisted[_nodeRunner], "Unnecessary update to same status"); L280
83. /// @notice Allow node runners to withdraw ETH from their smart wallet. ETH can only be withdrawn until the KNOT has not been staked. L323
84. require(isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key has already withdrawn or not a part of LSD network"); L328
85. require(smartWalletOfNodeRunner[msg.sender] == associatedSmartWallet, "Not the node runner for the smart wallet "); L331
86. require(isNodeRunnerBanned(nodeRunnerOfSmartWallet[associatedSmartWallet]) == false, "Node runner is banned from LSD network"); L332
87. getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKeyOfKnot) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, L335
88. function rotateNodeRunnerOfSmartWallet(address _current, address _new, bool _wasPreviousNodeRunnerMalicious) external { L356
89. require(isBLSPublicKeyBanned(_blsPubKeys[i]) == false, "BLS public key is banned or not a part of LSD network"); L393
90. require(smartWalletOfKnot[_blsPubKeys[i]] == smartWallet, "BLS public key doesn't belong to the node runner"); L396
91. // Ensure that the node runner does not whitelist multiple EOA representatives - they can only have 1 active at a time L453
92. require(smartWalletRepresentative[smartWallet] == _eoaRepresentative, "Different EOA specified - rotate outside"); L455
93. require(isBLSPublicKeyPartOfLSDNetwork(_blsPublicKey) == false, "BLS public key is banned or not a part of LSD network"); L469
94. getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKey) == IDataStructures.LifecycleStatus.UNBEGUN, L472
95. return !isBLSPublicKeyPartOfLSDNetwork(_blsPublicKeyOfKnot) || bannedBLSPublicKeys[_blsPublicKeyOfKnot] != address(0); L501
96. getAccountManager().blsPublicKeyToLifecycleStatus(blsPubKey) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, L546
97. /// @notice Anyone can call this to trigger creating a knot which will mint derivatives once the balance has been reported L573
98. /// @param _blsPublicKeyOfKnots List of BLS public keys registered with the network becoming knots and minting derivatives L574
99. require(isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is banned or not a part of LSD network"); L589
100. getAccountManager().blsPublicKeyToLifecycleStatus(_blsPublicKeyOfKnots[i]) == IDataStructures.LifecycleStatus.DEPOSIT_COMPLETED, L593
101. require(_oldLPToken.lastInteractedTimestamp(msg.sender) + 30 minutes < block.timestamp, "Liquidity is still fresh"); L82
102. getAccountManager().blsPublicKeyToLifecycleStatus(blsPublicKeyOfPreviousKnot) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED, L92
103. getAccountManager().blsPublicKeyToLifecycleStatus(blsPublicKeyOfNewKnot) ==IDataStructures.LifecycleStatus.INITIALS_REGISTERED, L97
104. function _depositETHForStaking(bytes calldata _blsPublicKeyOfKnot, uint256 _amount, bool _enableTransferHook) internal { L110
105. require(lpToken.totalSupply() + _amount <= maxStakingAmountPerValidator, "Amount exceeds the staking limit for the validator"); L122
106. LPToken(lpTokenFactory.deployLPToken(address(this), address(this), tokenSymbol, tokenName)) : L140

Description:

Maximum suggested line length is 120 characters.

Report

Gas Optimizations

[G-1]: The increment in for loop post condition can be made unchecked

Context:

1. for (uint256 i; i < amountOfTokens; ++i) { L76
2. for (uint256 i; i < numOfSavETHVaults; ++i) { L42
3. for (uint256 i; i < numOfVaults; ++i) { L78
4. for (uint256 j; j < _lpTokens[i].length; ++j) { L82
5. for (uint256 i; i < numOfRotations; ++i) { L128
6. for (uint256 i; i < numOfVaults; ++i) { L146
7. for (uint256 j; j < _lpTokens[i].length; ++j) { L148
8. for (uint256 i; i < numOfValidators; ++i) { L63
9. for (uint256 i; i < numOfTokens; ++i) { L103
10. for (uint256 i; i < numOfTokens; ++i) { L116
11. for (uint256 i; i < numOfVaults; ++i) { L38
12. for (uint256 i; i < numOfVaults; ++i) { L64
13. for (uint256 i; i < _stakingFundsVaults.length; ++i) { L90
14. for (uint256 i; i < numOfRotations; ++i) { L117
15. for (uint256 i; i < numOfVaults; ++i) { L135
16. for (uint256 i; i < _lpTokens.length; ++i) { L183
17. for (uint256 i; i < numOfValidators; ++i) { L78
18. for (uint256 i; i < numOfTokens; ++i) { L152
19. for (uint256 i; i < numOfTokens; ++i) { L166
20. for (uint256 i; i < _blsPubKeys.length; ++i) { L203
21. for (uint256 i; i < _blsPublicKeys.length; ++i) { L266
22. for (uint256 i; i < _blsPubKeys.length; ++i) { L276
23. for (uint256 i; i < _token.length; ++i) { L286
24. for (uint256 i; i < _blsPubKeys.length; ++i) { L211
25. for (uint256 i; i < _blsPubKeys.length; ++i) { L259
26. for (uint256 i; i < _blsPubKeys.length; ++i) { L301
27. for (uint256 i; i < _blsPubKeys.length; ++i) { L346
28. for (uint256 i; i < numberOfCollateralisedSlotOwnersForKnot; ++i) { L420
29. for (uint256 i; i < numberOfCollateralisedSlotOwnersForKnot; ++i) { L513
30. for (uint256 i; i < knotsToRegister; ++i) { L560
31. for (uint256 i; i < _priorityStakers.length; ++i) { L585
32. for (uint256 i; i < _blsPublicKeys.length; ++i) { L598
33. for (uint256 i; i < _blsPubKeys.length; ++i) { L648
34. for(uint256 i; i < _blsPubKeys.length; ++i) { L392
35. for(uint256 i; i < len; ++i) { L465
36. for (uint256 i; i < numOfValidators; ++i) { L538
37. for (uint256 i; i < numOfKnotsToProcess; ++i) { L587
38. for (uint256 i; i < numOfRotations; ++i) { L63

Description:

This can save 30-40 gas per loop iteration.

Recommendation:

Example how to fix. Change:

for (uint256 i = 0; i < orders.length; ++i) {
   // Do the thing
}

To:

for (uint256 i = 0; i < orders.length;) {
   // Do the thing
   unchecked { ++i; }
}

[G-2]: Place subtractions where the operands can't underflow in unchecked {} block

Context:

idleETH -= _amount; L57

Description:

Some gas can be saved by using an unchecked {} block if an underflow isn't possible because of a previous require() or if-statement.

[G-3]: If internal function called only once it can be inlined to save gas

Context:

function _assertUserHasEnoughGiantLPToClaimVaultLP(LPToken _token, uint256 _amount) internal view { L93

Description:

If they are not inlined, they cost an additional 20 to 40 gas because of 2 extra jump instructions and additional stack operations needed for function calls.

[G-4]: Cache a value from a mapping/array in local memory

Context:

1. https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/syndicate/Syndicate.sol#L500-L535 (isNoLongerPartOfSyndicate[_blsPubKey])

        if (unprocessedETHForCurrentKnot > 0 && !isNoLongerPartOfSyndicate[_blsPubKey]) {
            ...
        }

        // if the knot is no longer active, no further accrual of rewards are possible snapshots are possible but ETH accrued up to that point
        // Basically, under a rage quit or voluntary withdrawal from the beacon chain, the knot kick is auto-propagated to syndicate
        if (!isActive && !isNoLongerPartOfSyndicate[_blsPubKey]) {
            _deRegisterKnot(_blsPubKey);
        }

2. https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/LiquidStakingManager.sol#L296-L299 (smartWalletRepresentative[smartWallet])

        require(smartWalletRepresentative[smartWallet] != _newRepresentative, "Invalid rotation to same EOA");

        // unauthorize old representative
        _authorizeRepresentative(smartWallet, smartWalletRepresentative[smartWallet], false);

3. https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/LiquidStakingManager.sol#L314-L317 (smartWalletRepresentative[smartWallet])

        require(smartWalletRepresentative[smartWallet] != _newRepresentative, "Invalid rotation to same EOA");

        // unauthorize old representative
        _authorizeRepresentative(smartWallet, smartWalletRepresentative[smartWallet], false);

4. https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/LiquidStakingManager.sol#L454-L455 (smartWalletRepresentative[smartWallet])

        if(smartWalletRepresentative[smartWallet] != address(0)) {
            require(smartWalletRepresentative[smartWallet] == _eoaRepresentative, "Different EOA specified - rotate outside");

Description:

If you read value from mapping/array more than once within a function then it is cheaper to cache it in local memory and then read it from memory wnen it is neaded. This will save about 40 gas.

Recommendation:

Example how to fix. Change:

        require(smartWalletRepresentative[smartWallet] != _newRepresentative, "Invalid rotation to same EOA");

        // unauthorize old representative
        _authorizeRepresentative(smartWallet, smartWalletRepresentative[smartWallet], false);

to:

        address _oldRepresentative = smartWalletRepresentative[smartWallet];
        require(_oldRepresentative != _newRepresentative, "Invalid rotation to same EOA");

        // unauthorize old representative
        _authorizeRepresentative(smartWallet, _oldRepresentative, false);

[G-5]: Cache state variable in memory and not re-read it from the storage

Context:

1. https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/syndicate/Syndicate.sol#L177-L189 (numberOfRegisteredKnots)

        if (numberOfRegisteredKnots > 0) {
            ...
            }
            uint256 collateralizedUnprocessed = ((totalEthPerSlotType - lastSeenETHPerCollateralizedSlotPerKnot) / numberOfRegisteredKnots);

2. https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/LiquidStakingManager.sol#L241-L243 (dao)

        require(_newAddress != dao, "Same address");

        emit UpdateDAOAddress(dao, _newAddress);

3. https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/LiquidStakingManager.sol#L361-L371 (dao)

        require(_current == msg.sender || dao == msg.sender, "Not current owner or DAO");

        ...

        if (msg.sender == dao && _wasPreviousNodeRunnerMalicious) {

Description:

Every reading from storage costs about 100 gas while every reading from memory costs only 3 gas. If state variable referred more than once within a function then it is cheaper to cache it in local memory (100 gas) and then read it from memory wnen it is neaded (3 gas) rather than read state variable from storage everytime (100 gas).

Recommendation:

Example how to fix. Change:

        require(_current == msg.sender || dao == msg.sender, "Not current owner or DAO");

        ...

        if (msg.sender == dao && _wasPreviousNodeRunnerMalicious) {

to:

        address _dao = dao;
        require(_current == msg.sender || _dao == msg.sender, "Not current owner or DAO");

        ...

        if (msg.sender == _dao && _wasPreviousNodeRunnerMalicious) {

[G-6]: Use calldata instead of memory

Context:

1. function execute(address target, bytes memory callData) L41
2. bytes memory callData, L54
3. bytes memory callData, L69
4. function claimFundsFromSyndicateForDistribution(bytes[] memory _blsPubKeys) external { L355
5. function _claimFundsFromSyndicateForDistribution(address _syndicate, bytes[] memory _blsPubKeys) internal { L360
6. function updateCollateralizedSlotOwnersAccruedETH(bytes memory _blsPubKey) external { L337
7. function batchUpdateCollateralizedSlotOwnersAccruedETH(bytes[] memory _blsPubKeys) external { L343
8. function calculateUnclaimedFreeFloatingETHShare(bytes memory _blsPubKey, address _user) public view returns (uint256) { L359
9. address[] memory _priorityStakers, L472
10. bytes[] memory _blsPubKeysForSyndicateKnots L473
11. function _updateCollateralizedSlotOwnersLiabilitySnapshot(bytes memory _blsPubKey) internal { L491
12. function _registerKnotsToSyndicate(bytes[] memory _blsPubKeysForSyndicateKnots) internal { L555
13. function _addPriorityStakers(address[] memory _priorityStakers) internal { L583
14. function _deRegisterKnot(bytes memory _blsPublicKey) internal { L610
15. function _autoStakeWithSyndicate(address _associatedSmartWallet, bytes memory _blsPubKey) internal { L879

Description:

If a reference type function parameter is read-only, it is recommended to use calldata instead of memory because this provides significant gas savings. Since Solidity v0.6.9, memory and calldata are allowed in all functions regardless of their visibility type (ie external, public, etc).

[G-7]: Don't compare boolean expressions to boolean literals

Context:

1. vault.isDETHReadyForWithdrawal(address(_lpTokens[i][j])) == false, L150
2. require(liquidStakingManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is not part of LSD network"); L64
3. require(liquidStakingManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key is banned or not a part of LSD network"); L84
4. require(liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is not part of LSD network"); L79
5. require(liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key is banned or not a part of LSD network"); L114
6. liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPubKeys[i]) == false, L205
7. if (isKnotRegistered[_blsPublicKey] == false) revert KnotIsNotRegisteredWithSyndicate(); L611
8. if (isNoLongerPartOfSyndicate[_blsPublicKey] == true) revert KnotHasAlreadyBeenDeRegistered(); L612
9. require(isNodeRunnerBanned(msg.sender) == false, "Node runner is banned from LSD network"); L291
10. require(isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key has already withdrawn or not a part of LSD network"); L328
11. require(isNodeRunnerBanned(nodeRunnerOfSmartWallet[associatedSmartWallet]) == false, "Node runner is banned from LSD network"); L332
12. require(isBLSPublicKeyBanned(_blsPubKeys[i]) == false, "BLS public key is banned or not a part of LSD network"); L393
13. require(_isNodeRunnerValid(msg.sender) == true, "Unrecognised node runner"); L436
14. require(isNodeRunnerBanned(msg.sender) == false, "Node runner is banned from LSD network"); L437
15. require(isBLSPublicKeyPartOfLSDNetwork(_blsPublicKey) == false, "BLS public key is banned or not a part of LSD network"); L469
16. require(isBLSPublicKeyBanned(blsPubKey) == false, "BLS public key is banned or not a part of LSD network"); L541
17. require(isBLSPublicKeyBanned(_blsPublicKeyOfKnots[i]) == false, "BLS public key is banned or not a part of LSD network"); L589
18. require(isNodeRunnerWhitelisted[_nodeRunner] == true, "Invalid node runner"); L688