Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Hardhat Unit Tests Coverage SummaryDetailsDiff against masterResults for commit: b0c3278 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
krogla
reviewed
Sep 15, 2025
krogla
reviewed
Sep 15, 2025
krogla
reviewed
Sep 15, 2025
krogla
reviewed
Sep 15, 2025
krogla
reviewed
Sep 15, 2025
krogla
reviewed
Sep 15, 2025
krogla
reviewed
Sep 15, 2025
There was a problem hiding this comment.
Slither found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
eddort
reviewed
Nov 5, 2025
mkurayan
reviewed
Feb 19, 2026
mkurayan
reviewed
Feb 21, 2026
mkurayan
reviewed
Feb 21, 2026
mkurayan
reviewed
Feb 21, 2026
mkurayan
reviewed
Feb 21, 2026
mkurayan
reviewed
Feb 21, 2026
mkurayan
reviewed
Feb 21, 2026
mkurayan
reviewed
Feb 21, 2026
mkurayan
reviewed
Feb 21, 2026
mkurayan
reviewed
Feb 21, 2026
mkurayan
reviewed
Feb 21, 2026
mkurayan
reviewed
Feb 21, 2026
mkurayan
reviewed
Feb 21, 2026
mkurayan
reviewed
Feb 21, 2026
…solidation-improvements Feat/staking router 3.0 consolidation improvements
Comment on lines
+177
to
+188
| function allowPair( | ||
| uint256 sourceOperatorId, | ||
| uint256 targetOperatorId, | ||
| address submitter | ||
| ) external onlyRole(ALLOW_PAIR_ROLE) { | ||
| if (submitter == address(0)) revert ZeroArgument("submitter"); | ||
|
|
||
| _allowedPairs[sourceOperatorId].add(targetOperatorId); | ||
| _submitters[sourceOperatorId][targetOperatorId] = submitter; | ||
|
|
||
| emit ConsolidationPairAllowed(sourceOperatorId, targetOperatorId, submitter); | ||
| } |
Check warning
Code scanning / Slither
Unused return Medium
Comment on lines
+118
to
+122
| modifier preservesEthBalance() { | ||
| uint256 balanceBeforeCall = address(this).balance - msg.value; | ||
| _; | ||
| assert(address(this).balance == balanceBeforeCall); | ||
| } |
Check warning
Code scanning / Slither
Dangerous strict equalities Medium
…solidation-flow-improvments Feat/staking router 3.0 consolidation flow improvments
…fee-invariant feat: enforce consistent modules fee
…ation-flow refactor: sr modules data validation flow
…pen-the-gate feat: new deposit tracker
…king-router-3.0-consolidation-group-requests
Comment on lines
+294
to
+307
| function finalizeUpgrade_v4() external { | ||
| require(hasInitialized(), "NOT_INITIALIZED"); | ||
| _checkContractVersion(2); | ||
| _setContractVersion(3); | ||
|
|
||
| _migrateStorage_v2_to_v3(); | ||
|
|
||
| _migrateBurner_v2_to_v3(_oldBurner, _contractsWithBurnerAllowances); | ||
|
|
||
| _setMaxExternalRatioBP(_initialMaxExternalRatioBP); | ||
| } | ||
|
|
||
| function _migrateStorage_v2_to_v3() internal { | ||
| // migrate storage to packed representation | ||
| bytes32 LIDO_LOCATOR_POSITION = keccak256("lido.Lido.lidoLocator"); | ||
| address locator = LIDO_LOCATOR_POSITION.getStorageAddress(); | ||
| assert(locator != address(0)); // sanity check | ||
|
|
||
| _setLidoLocator(LIDO_LOCATOR_POSITION.getStorageAddress()); | ||
| LIDO_LOCATOR_POSITION.setStorageUint256(0); | ||
|
|
||
| bytes32 BUFFERED_ETHER_POSITION = keccak256("lido.Lido.bufferedEther"); | ||
| _setBufferedEther(BUFFERED_ETHER_POSITION.getStorageUint256()); | ||
| BUFFERED_ETHER_POSITION.setStorageUint256(0); | ||
|
|
||
| bytes32 DEPOSITED_VALIDATORS_POSITION = keccak256("lido.Lido.depositedValidators"); | ||
| _setDepositedValidators(DEPOSITED_VALIDATORS_POSITION.getStorageUint256()); | ||
| DEPOSITED_VALIDATORS_POSITION.setStorageUint256(0); | ||
|
|
||
| bytes32 CL_VALIDATORS_POSITION = keccak256("lido.Lido.beaconValidators"); | ||
| bytes32 CL_BALANCE_POSITION = keccak256("lido.Lido.beaconBalance"); | ||
| _setClBalanceAndClValidators( | ||
| CL_BALANCE_POSITION.getStorageUint256(), | ||
| CL_VALIDATORS_POSITION.getStorageUint256() | ||
| ); | ||
| CL_BALANCE_POSITION.setStorageUint256(0); | ||
| CL_VALIDATORS_POSITION.setStorageUint256(0); | ||
|
|
||
| bytes32 TOTAL_SHARES_POSITION = keccak256("lido.StETH.totalShares"); | ||
| uint256 totalShares = TOTAL_SHARES_POSITION.getStorageUint256(); | ||
| assert(totalShares > 0); // sanity check | ||
| TOTAL_AND_EXTERNAL_SHARES_POSITION.setLowUint128(totalShares); | ||
| TOTAL_SHARES_POSITION.setStorageUint256(0); | ||
| } | ||
|
|
||
| function _migrateBurner_v2_to_v3( | ||
| address _oldBurner, | ||
| address[] _contractsWithBurnerAllowances | ||
| ) internal { | ||
| require(_oldBurner != address(0), "OLD_BURNER_ADDRESS_ZERO"); | ||
| address burner = _burner(); | ||
| require(_oldBurner != burner, "OLD_BURNER_SAME_AS_NEW"); | ||
|
|
||
| // migrate burner stETH balance | ||
| uint256 oldBurnerShares = _sharesOf(_oldBurner); | ||
| if (oldBurnerShares > 0) { | ||
| _transferShares(_oldBurner, burner, oldBurnerShares); | ||
| _emitTransferEvents(_oldBurner, burner, getPooledEthByShares(oldBurnerShares), oldBurnerShares); | ||
| } | ||
|
|
||
| // initialize new burner with state from the old burner | ||
| IBurnerMigration(burner).migrate(_oldBurner); | ||
|
|
||
| // migrating allowances | ||
| for (uint256 i = 0; i < _contractsWithBurnerAllowances.length; i++) { | ||
| uint256 oldAllowance = allowance(_contractsWithBurnerAllowances[i], _oldBurner); | ||
| _approve(_contractsWithBurnerAllowances[i], _oldBurner, 0); | ||
| _approve(_contractsWithBurnerAllowances[i], burner, oldAllowance); | ||
| } | ||
| /// @dev prevent migration if the last oracle report wasn't submitted, otherwise deposits | ||
| /// made after refSlot and before migration (i.e. report's tx) will be lost | ||
| IAccountingOracle oracle = _accountingOracle(); | ||
| (,,, bool mainDataSubmitted,,,,,) = oracle.getProcessingState(); | ||
| /// @dev pass in case of initial deploy | ||
| require(mainDataSubmitted || oracle.getLastProcessingRefSlot() == 0, "NO_REPORT"); | ||
|
|
||
| _checkContractVersion(3); | ||
| _setContractVersion(4); | ||
| _migrateStorage_v3_to_v4(); | ||
| } |
Check warning
Code scanning / Slither
Unused return Medium
…solidation-group-requests feat: wrap consolidation requests to struct
Comment on lines
+185
to
+223
| function addConsolidationRequests( | ||
| ConsolidationWitnessGroup[] calldata groups, | ||
| address refundRecipient | ||
| ) external payable onlyRole(ADD_CONSOLIDATION_REQUEST_ROLE) preservesEthBalance whenResumed { | ||
| if (msg.value == 0) revert ZeroArgument("msg.value"); | ||
| uint256 groupsCount = groups.length; | ||
| if (groupsCount == 0) revert ZeroArgument("groups"); | ||
|
|
||
| // Count total individual requests across all groups | ||
| uint256 requestsCount = 0; | ||
| for (uint256 i = 0; i < groupsCount; ++i) { | ||
| uint256 groupSize = groups[i].sourcePubkeys.length; | ||
| if (groupSize == 0) revert EmptyGroup(i); | ||
| requestsCount += groupSize; | ||
| } | ||
|
|
||
| _checkConsolidationPreconditions(); | ||
|
|
||
| (IWithdrawalVault withdrawalVault, bytes32 withdrawalCredentials) = _getWithdrawalVaultData(); | ||
|
|
||
| for (uint256 i = 0; i < groupsCount; ++i) { | ||
| _validatePubKeyWCProof(groups[i].targetWitness, withdrawalCredentials); | ||
| } | ||
|
|
||
| _consumeConsolidationRequestLimit(requestsCount); | ||
|
|
||
| uint256 fee = withdrawalVault.getConsolidationRequestFee(); | ||
| uint256 totalFee = requestsCount * fee; | ||
| uint256 refund = _checkFee(totalFee); | ||
|
|
||
| // Expand grouped requests into flat pairs for WithdrawalVault | ||
| (bytes[] memory sourcePubkeys, bytes[] memory targetPubkeys) = _prepareConsolidationPairs( | ||
| groups, | ||
| requestsCount | ||
| ); | ||
| withdrawalVault.addConsolidationRequests{value: totalFee}(sourcePubkeys, targetPubkeys); | ||
|
|
||
| _refundFee(refund, refundRecipient); | ||
| } |
Check failure
Code scanning / Slither
Functions that send Ether to arbitrary destinations High
Comment on lines
+413
to
+418
| function _getDepositedValidatorsCount( | ||
| IUnifiedStakingModule module, | ||
| uint256 operatorId | ||
| ) internal view returns (uint256 totalDeposited) { | ||
| (, , , , , , totalDeposited, ) = module.getNodeOperatorSummary(operatorId); | ||
| } |
Check warning
Code scanning / Slither
Unused return Medium
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
❗ This is a draft pull request
Not ready for review