From 779a95208a1bddf6f46888b469e5772b498843e7 Mon Sep 17 00:00:00 2001 From: skhomuti Date: Fri, 22 Dec 2023 13:26:05 +0500 Subject: [PATCH 1/3] feat: implement updateExitedValidatorsCount --- src/CSModule.sol | 43 ++++++++++++++++++--- test/CSModule.t.sol | 94 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 6 deletions(-) diff --git a/src/CSModule.sol b/src/CSModule.sol index 23f472e2..cae8b89b 100644 --- a/src/CSModule.sol +++ b/src/CSModule.sol @@ -113,6 +113,8 @@ contract CSModuleBase { error UnbondedKeysPresent(); error InvalidTargetLimit(); error StuckKeysHigherThanTotalDeposited(); + error ExitedKeysHigherThanTotalDeposited(); + error ExitedKeysDecrease(); error QueueLookupNoLimit(); error QueueEmptyBatch(); @@ -800,12 +802,41 @@ contract CSModule is ICSModule, CSModuleBase { function updateExitedValidatorsCount( bytes calldata nodeOperatorIds, bytes calldata exitedValidatorsCounts - ) external { - // TODO: implement - // emit ExitedSigningKeysCountChanged( - // nodeOperatorId, - // exitedValidatorsCount - // ); + ) external onlyStakingRouter { + ValidatorCountsReport.validate(nodeOperatorIds, exitedValidatorsCounts); + + for ( + uint256 i = 0; + i < ValidatorCountsReport.count(nodeOperatorIds); + i++ + ) { + ( + uint256 nodeOperatorId, + uint256 exitedValidatorsCount + ) = ValidatorCountsReport.next( + nodeOperatorIds, + exitedValidatorsCounts, + i + ); + if (nodeOperatorId >= _nodeOperatorsCount) + revert NodeOperatorDoesNotExist(); + + NodeOperator storage no = _nodeOperators[nodeOperatorId]; + if (exitedValidatorsCount > no.totalDepositedKeys) + revert ExitedKeysHigherThanTotalDeposited(); + if (exitedValidatorsCount < no.totalExitedKeys) + revert ExitedKeysDecrease(); + if (exitedValidatorsCount == no.totalExitedKeys) continue; + + _totalExitedValidators += + exitedValidatorsCount - + no.totalExitedKeys; + no.totalExitedKeys = exitedValidatorsCount; + emit ExitedSigningKeysCountChanged( + nodeOperatorId, + exitedValidatorsCount + ); + } } /// @notice Reports withdrawn validator for node operator diff --git a/test/CSModule.t.sol b/test/CSModule.t.sol index a0b78ff3..4fff33f8 100644 --- a/test/CSModule.t.sol +++ b/test/CSModule.t.sol @@ -1631,6 +1631,100 @@ contract CsmUpdateStuckValidatorsCount is CSMCommon { } } +contract CsmUpdateExitedValidatorsCount is CSMCommon { + function test_updateExitedValidatorsCount_NonZero() public { + uint256 noId = createNodeOperator(1); + csm.vetKeys(noId, 1); + csm.obtainDepositData(1, ""); + + vm.expectEmit(true, true, false, true, address(csm)); + emit ExitedSigningKeysCountChanged(noId, 1); + csm.updateExitedValidatorsCount( + bytes.concat(bytes8(0x0000000000000000)), + bytes.concat(bytes16(0x00000000000000000000000000000001)) + ); + + NodeOperatorSummary memory noSummary = getNodeOperatorSummary(noId); + assertEq( + noSummary.totalExitedValidators, + 1, + "totalExitedValidators not increased" + ); + + (uint256 totalExitedValidators, , ) = csm.getStakingModuleSummary(); + assertEq( + totalExitedValidators, + 1, + "totalExitedValidators not increased" + ); + } + + function test_updateExitedValidatorsCount_RevertIfNoNodeOperator() public { + vm.expectRevert(NodeOperatorDoesNotExist.selector); + csm.updateExitedValidatorsCount( + bytes.concat(bytes8(0x0000000000000000)), + bytes.concat(bytes16(0x00000000000000000000000000000001)) + ); + } + + function test_updateExitedValidatorsCount_RevertIfNotStakingRouter() + public + { + // TODO implement + vm.skip(true); + } + + function test_updateExitedValidatorsCount_RevertIfCountMoreThanDeposited() + public + { + uint256 noId = createNodeOperator(1); + + vm.expectRevert(ExitedKeysHigherThanTotalDeposited.selector); + csm.updateExitedValidatorsCount( + bytes.concat(bytes8(0x0000000000000000)), + bytes.concat(bytes16(0x00000000000000000000000000000001)) + ); + } + + function test_updateExitedValidatorsCount_RevertIfExitedKeysDecrease() + public + { + uint256 noId = createNodeOperator(1); + csm.vetKeys(noId, 1); + csm.obtainDepositData(1, ""); + + csm.updateExitedValidatorsCount( + bytes.concat(bytes8(0x0000000000000000)), + bytes.concat(bytes16(0x00000000000000000000000000000001)) + ); + + vm.expectRevert(ExitedKeysDecrease.selector); + csm.updateExitedValidatorsCount( + bytes.concat(bytes8(0x0000000000000000)), + bytes.concat(bytes16(0x00000000000000000000000000000000)) + ); + } + + function test_updateExitedValidatorsCount_NoEventIfSameValue() public { + uint256 noId = createNodeOperator(1); + csm.vetKeys(noId, 1); + csm.obtainDepositData(1, ""); + + csm.updateExitedValidatorsCount( + bytes.concat(bytes8(0x0000000000000000)), + bytes.concat(bytes16(0x00000000000000000000000000000001)) + ); + + vm.recordLogs(); + csm.updateExitedValidatorsCount( + bytes.concat(bytes8(0x0000000000000000)), + bytes.concat(bytes16(0x00000000000000000000000000000001)) + ); + Vm.Log[] memory logs = vm.getRecordedLogs(); + assertEq(logs.length, 0); + } +} + contract CsmPenalize is CSMCommon { function test_penalize_NoUnvet() public { uint256 noId = createNodeOperator(); From 090ff97cd98f6158b5f4d54ef4c2f961ff568e77 Mon Sep 17 00:00:00 2001 From: skhomuti Date: Wed, 27 Dec 2023 13:38:35 +0500 Subject: [PATCH 2/3] feat: submit withdrawal base method --- src/CSModule.sol | 72 ++++++++++++++++++++++++--------------------- test/CSModule.t.sol | 35 ++++++++++++++++++++++ 2 files changed, 74 insertions(+), 33 deletions(-) diff --git a/src/CSModule.sol b/src/CSModule.sol index cae8b89b..5ea0764b 100644 --- a/src/CSModule.sol +++ b/src/CSModule.sol @@ -26,13 +26,13 @@ struct NodeOperator { uint256 targetLimit; bool isTargetLimitActive; uint256 stuckPenaltyEndTimestamp; - uint256 totalExitedKeys; - uint256 totalAddedKeys; - uint256 totalWithdrawnKeys; - uint256 totalDepositedKeys; - uint256 totalVettedKeys; - uint256 stuckValidatorsCount; - uint256 refundedValidatorsCount; + uint256 totalExitedKeys; // @dev only increased + uint256 totalAddedKeys; // @dev only increased + uint256 totalWithdrawnKeys; // @dev only increased + uint256 totalDepositedKeys; // @dev only increased + uint256 totalVettedKeys; // @dev both increased and decreased + uint256 stuckValidatorsCount; // @dev both increased and decreased + uint256 refundedValidatorsCount; // @dev only increased uint256 queueNonce; } @@ -82,6 +82,7 @@ contract CSModuleBase { bool isTargetLimitActive, uint256 targetValidatorsCount ); + event WithdrawalSubmitted(uint256 indexed validatorId, uint256 exitBalance); event BatchEnqueued( uint256 indexed nodeOperatorId, @@ -132,6 +133,7 @@ contract CSModule is ICSModule, CSModuleBase { // @dev max number of node operators is limited by uint64 due to Batch serialization in 32 bytes // it seems to be enough uint64 public constant MAX_NODE_OPERATORS_COUNT = type(uint64).max; + uint256 public constant DEPOSIT_SIZE = 32 ether; bytes32 public constant SIGNING_KEYS_POSITION = keccak256("lido.CommunityStakingModule.signingKeysPosition"); @@ -214,6 +216,7 @@ contract CSModule is ICSModule, CSModuleBase { uint256 /* depositableValidatorsCount */ ) { + // TODO: need to be implemented properly return ( _totalExitedValidators, _totalDepositedValidators, @@ -675,13 +678,14 @@ contract CSModule is ICSModule, CSModuleBase { no.totalVettedKeys - totalDepositedValidators; if (no.isTargetLimitActive) { - depositableValidatorsCount = (totalExitedValidators + - targetValidatorsCount) <= no.totalVettedKeys - ? 0 - : Math.min( - totalExitedValidators + targetValidatorsCount, - no.totalVettedKeys - ) - totalDepositedValidators; + uint256 activeValidatorsCount = no.totalDepositedKeys - + no.totalExitedKeys; + depositableValidatorsCount = Math.min( + targetValidatorsCount > activeValidatorsCount + ? targetValidatorsCount - activeValidatorsCount + : 0, + depositableValidatorsCount + ); } } @@ -839,25 +843,6 @@ contract CSModule is ICSModule, CSModuleBase { } } - /// @notice Reports withdrawn validator for node operator - /// @param withdrawProof Withdraw proof - /// @param validatorId ID of the validator - /// @param nodeOperatorId ID of the node operator - /// @param withdrawnBalance Amount of withdrawn balance - function reportWithdrawnValidator( - bytes32[] memory withdrawProof, - uint256 validatorId, - uint256 nodeOperatorId, - uint256 withdrawnBalance - ) external { - // TODO: implement me - } - - /// @notice Triggers the node operator's unbonded validators to exit - function exitUnbondedValidators(uint256 nodeOperatorId) external { - // TODO: implement me - } - /// @notice Triggers the node operator's validator to exit by DAO decision function unsafeExitValidator( uint256 nodeOperatorId, @@ -975,6 +960,7 @@ contract CSModule is ICSModule, CSModuleBase { if ( no.isTargetLimitActive && + // TODO: totalExited or totalWithdrawn? vetKeysPointer > (no.totalExitedKeys + no.targetLimit) ) revert TargetLimitExceeded(); if (no.stuckValidatorsCount > 0) revert StuckKeysPresent(); @@ -1119,6 +1105,26 @@ contract CSModule is ICSModule, CSModuleBase { _checkForOutOfBond(nodeOperatorId); } + function submitWithdrawal( + bytes32 /*withdrawalProof*/, + uint256 nodeOperatorId, + uint256 validatorId, + uint256 exitBalance + ) external onlyExistingNodeOperator(nodeOperatorId) { + // TODO: check for withdrawal proof + // TODO: consider asserting that withdrawn keys count is not higher than exited keys count + NodeOperator storage no = _nodeOperators[nodeOperatorId]; + + no.totalWithdrawnKeys += 1; + + if (exitBalance < DEPOSIT_SIZE) { + accounting.penalize(nodeOperatorId, DEPOSIT_SIZE - exitBalance); + _checkForOutOfBond(nodeOperatorId); + } + + emit WithdrawalSubmitted(validatorId, exitBalance); + } + /// @notice Called when withdrawal credentials changed by DAO function onWithdrawalCredentialsChanged() external { revert("NOT_IMPLEMENTED"); diff --git a/test/CSModule.t.sol b/test/CSModule.t.sol index 4fff33f8..e91625c5 100644 --- a/test/CSModule.t.sol +++ b/test/CSModule.t.sol @@ -1886,3 +1886,38 @@ contract CsmSettleELRewardsStealingPenalty is CSMCommon { assertEq(lock.retentionUntil, 0); } } + +contract CsmSubmitWithdrawal is CSMCommon { + function test_submitWithdrawal() public { + uint256 validatorId = 1; + uint256 noId = createNodeOperator(); + csm.vetKeys(noId, 1); + csm.obtainDepositData(1, ""); + + vm.expectEmit(true, true, true, true, address(csm)); + emit WithdrawalSubmitted(validatorId, csm.DEPOSIT_SIZE()); + csm.submitWithdrawal("", noId, validatorId, csm.DEPOSIT_SIZE()); + + CSModule.NodeOperatorInfo memory no = csm.getNodeOperator(noId); + assertEq(no.totalWithdrawnValidators, 1); + } + + function test_submitWithdrawal_lowExitBalance() public { + uint256 validatorId = 1; + uint256 noId = createNodeOperator(); + uint256 depositSize = csm.DEPOSIT_SIZE(); + csm.vetKeys(noId, 1); + csm.obtainDepositData(1, ""); + + vm.expectCall( + address(accounting), + abi.encodeWithSelector(accounting.penalize.selector, noId, 1 ether) + ); + csm.submitWithdrawal("", noId, validatorId, depositSize - 1 ether); + } + + function test_submitWithdrawal_RevertWhenNoNodeOperator() public { + vm.expectRevert(NodeOperatorDoesNotExist.selector); + csm.submitWithdrawal("", 0, 0, 0); + } +} From 2aa0c1ef1b6d2f3cd73ef29559e897d104687f8c Mon Sep 17 00:00:00 2001 From: skhomuti Date: Thu, 11 Jan 2024 10:18:15 +0500 Subject: [PATCH 3/3] fix for review --- src/CSModule.sol | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/CSModule.sol b/src/CSModule.sol index 5ea0764b..fef24a2e 100644 --- a/src/CSModule.sol +++ b/src/CSModule.sol @@ -82,7 +82,10 @@ contract CSModuleBase { bool isTargetLimitActive, uint256 targetValidatorsCount ); - event WithdrawalSubmitted(uint256 indexed validatorId, uint256 exitBalance); + event WithdrawalSubmitted( + uint256 indexed validatorId, + uint256 withdrawalBalance + ); event BatchEnqueued( uint256 indexed nodeOperatorId, @@ -960,7 +963,6 @@ contract CSModule is ICSModule, CSModuleBase { if ( no.isTargetLimitActive && - // TODO: totalExited or totalWithdrawn? vetKeysPointer > (no.totalExitedKeys + no.targetLimit) ) revert TargetLimitExceeded(); if (no.stuckValidatorsCount > 0) revert StuckKeysPresent(); @@ -1109,7 +1111,7 @@ contract CSModule is ICSModule, CSModuleBase { bytes32 /*withdrawalProof*/, uint256 nodeOperatorId, uint256 validatorId, - uint256 exitBalance + uint256 withdrawalBalance ) external onlyExistingNodeOperator(nodeOperatorId) { // TODO: check for withdrawal proof // TODO: consider asserting that withdrawn keys count is not higher than exited keys count @@ -1117,12 +1119,15 @@ contract CSModule is ICSModule, CSModuleBase { no.totalWithdrawnKeys += 1; - if (exitBalance < DEPOSIT_SIZE) { - accounting.penalize(nodeOperatorId, DEPOSIT_SIZE - exitBalance); + if (withdrawalBalance < DEPOSIT_SIZE) { + accounting.penalize( + nodeOperatorId, + DEPOSIT_SIZE - withdrawalBalance + ); _checkForOutOfBond(nodeOperatorId); } - emit WithdrawalSubmitted(validatorId, exitBalance); + emit WithdrawalSubmitted(validatorId, withdrawalBalance); } /// @notice Called when withdrawal credentials changed by DAO