Merge pull request #94 from lidofinance/reset-benefits

feat: outOfBond => resetBenefits
lidofinance · Mar 19, 2024 · 89fc8c4 · 89fc8c4
2 parents 1220033 + 8819a24
commit 89fc8c4
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 57 deletions.
diff --git a/src/CSAccounting.sol b/src/CSAccounting.sol
@@ -669,13 +669,16 @@ contract CSAccounting is
 
     /// @notice Settles locked bond ETH for the given node operator.
     /// @param nodeOperatorId id of the node operator to settle locked bond for.
-    function settleLockedBondETH(uint256 nodeOperatorId) external onlyCSM {
-        uint256 lockedAmount = CSBondLock.getActualLockedBond(nodeOperatorId);
+    function settleLockedBondETH(
+        uint256 nodeOperatorId
+    ) external onlyCSM returns (uint256 lockedAmount) {
+        lockedAmount = CSBondLock.getActualLockedBond(nodeOperatorId);
         if (lockedAmount > 0) {
             CSBondCore._burn(nodeOperatorId, lockedAmount);
         }
         // reduce all locked bond even if bond isn't covered lock fully
         CSBondLock._remove(nodeOperatorId);
+        return lockedAmount;
     }
 
     /// @notice Penalize bond by burning shares of the given node operator.

diff --git a/src/CSModule.sol b/src/CSModule.sol
@@ -1047,20 +1047,16 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil {
         }
     }
 
-    /// @notice any penalty might cause bond out, so we need to clear any benefits from the node operator
+    /// @notice reset benefits for the Node Operator
     /// @param nodeOperatorId ID of the node operator
-    function _checkForOutOfBond(uint256 nodeOperatorId) internal {
-        // TODO: Should be done manually or automatically? Any penalty should reset bond curve or not?
-        if (accounting.getBondShares(nodeOperatorId) == 0) {
-            accounting.resetBondCurve(nodeOperatorId);
-        }
+    function _resetBenefits(uint256 nodeOperatorId) internal {
+        accounting.resetBondCurve(nodeOperatorId);
+        _updateDepositableValidatorsCount(nodeOperatorId);
     }
 
     function _applyRemovalCharge(uint256 nodeOperatorId) internal {
         accounting.chargeFee(nodeOperatorId, removalCharge);
         emit RemovalChargeApplied(nodeOperatorId);
-
-        _checkForOutOfBond(nodeOperatorId);
     }
 
     /// @notice Reports EL rewards stealing for the given node operator.
@@ -1101,8 +1097,10 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil {
             uint256 nodeOperatorId = nodeOperatorIds[i];
             if (nodeOperatorId >= _nodeOperatorsCount)
                 revert NodeOperatorDoesNotExist();
-            accounting.settleLockedBondETH(nodeOperatorId);
-            _checkForOutOfBond(nodeOperatorId);
+            uint256 settled = accounting.settleLockedBondETH(nodeOperatorId);
+            if (settled > 0) {
+                _resetBenefits(nodeOperatorId);
+            }
         }
     }
 
@@ -1118,8 +1116,7 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil {
             revert Expired();
         }
         accounting.penalize(nodeOperatorId, amount);
-        _updateDepositableValidatorsCount(nodeOperatorId);
-        _checkForOutOfBond(nodeOperatorId);
+        _resetBenefits(nodeOperatorId);
     }
 
     /// @notice Report node operator's key as withdrawn and settle withdrawn amount.
@@ -1154,7 +1151,6 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil {
         if (amount < DEPOSIT_SIZE) {
             accounting.penalize(nodeOperatorId, DEPOSIT_SIZE - amount);
             _updateDepositableValidatorsCount(nodeOperatorId); // FIXME: normalizeQueue?
-            _checkForOutOfBond(nodeOperatorId);
         }
 
         _incrementModuleNonce();
@@ -1185,8 +1181,7 @@ contract CSModule is ICSModule, CSModuleBase, AccessControl, PausableUntil {
         emit InitialSlashingSubmitted(nodeOperatorId, keyIndex);
 
         accounting.penalize(nodeOperatorId, INITIAL_SLASHING_PENALTY);
-        _updateDepositableValidatorsCount(nodeOperatorId);
-        _checkForOutOfBond(nodeOperatorId);
+        _resetBenefits(nodeOperatorId);
         _incrementModuleNonce();
     }
 

diff --git a/src/interfaces/ICSAccounting.sol b/src/interfaces/ICSAccounting.sol
@@ -72,7 +72,9 @@ interface ICSAccounting is ICSBondCore, ICSBondCurve, ICSBondLock {
 
     function lockBondETH(uint256 nodeOperatorId, uint256 amount) external;
 
-    function settleLockedBondETH(uint256 nodeOperatorId) external;
+    function settleLockedBondETH(
+        uint256 nodeOperatorId
+    ) external returns (uint256);
 
     function setBondCurve(uint256 nodeOperatorId, uint256 curveId) external;
 

diff --git a/test/CSModule.t.sol b/test/CSModule.t.sol
@@ -1928,6 +1928,22 @@ contract CsmSettleELRewardsStealingPenalty is CSMCommon {
         idsToSettle[0] = noId;
         csm.reportELRewardsStealingPenalty(noId, block.number, amount);
 
+        vm.expectCall(
+            address(accounting),
+            abi.encodeWithSelector(accounting.resetBondCurve.selector, noId)
+        );
+        csm.settleELRewardsStealingPenalty(idsToSettle);
+
+        CSBondLock.BondLock memory lock = accounting.getLockedBondInfo(noId);
+        assertEq(lock.amount, 0 ether);
+        assertEq(lock.retentionUntil, 0);
+    }
+
+    function test_settleELRewardsStealingPenalty_noLocked() public {
+        uint256 noId = createNodeOperator();
+        uint256[] memory idsToSettle = new uint256[](1);
+        idsToSettle[0] = noId;
+
         expectNoCall(
             address(accounting),
             abi.encodeWithSelector(accounting.resetBondCurve.selector, noId)
@@ -1971,20 +1987,6 @@ contract CsmSettleELRewardsStealingPenalty is CSMCommon {
         assertEq(lock.retentionUntil, 0);
     }
 
-    function test_settleELRewardsStealingPenalty_penalizeEntireBond() public {
-        uint256 noId = createNodeOperator();
-        uint256 amount = BOND_SIZE;
-        uint256[] memory idsToSettle = new uint256[](1);
-        idsToSettle[0] = noId;
-        csm.reportELRewardsStealingPenalty(noId, block.number, amount);
-
-        vm.expectCall(
-            address(accounting),
-            abi.encodeWithSelector(accounting.resetBondCurve.selector, noId)
-        );
-        csm.settleELRewardsStealingPenalty(idsToSettle);
-    }
-
     function test_settleELRewardsStealingPenalty_WhenRetentionPeriodIsExpired()
         public
     {
@@ -2068,18 +2070,6 @@ contract CsmSubmitWithdrawal is CSMCommon {
         assertEq(csm.getNonce(), nonce + 1);
     }
 
-    function test_submitWithdrawal_outOfBond() public {
-        uint256 keyIndex = 0;
-        uint256 noId = createNodeOperator();
-        csm.obtainDepositData(1, "");
-
-        vm.expectCall(
-            address(accounting),
-            abi.encodeWithSelector(accounting.resetBondCurve.selector, noId)
-        );
-        csm.submitWithdrawal(noId, keyIndex, 0 ether);
-    }
-
     function test_submitWithdrawal_RevertWhenNoNodeOperator() public {
         vm.expectRevert(NodeOperatorDoesNotExist.selector);
         csm.submitWithdrawal(0, 0, 0);
@@ -2118,6 +2108,10 @@ contract CsmSubmitInitialSlashing is CSMCommon {
                 penaltyAmount
             )
         );
+        vm.expectCall(
+            address(accounting),
+            abi.encodeWithSelector(accounting.resetBondCurve.selector, noId)
+        );
         csm.submitInitialSlashing(noId, 0);
     }
 
@@ -2134,19 +2128,6 @@ contract CsmSubmitInitialSlashing is CSMCommon {
         csm.submitInitialSlashing(noId, 1);
     }
 
-    function test_submitInitialSlashing_outOfBond() public {
-        uint256 keyIndex = 0;
-        uint256 noId = createNodeOperator();
-        csm.obtainDepositData(1, "");
-
-        csm.penalize(noId, csm.DEPOSIT_SIZE() - csm.INITIAL_SLASHING_PENALTY());
-        vm.expectCall(
-            address(accounting),
-            abi.encodeWithSelector(accounting.resetBondCurve.selector, noId)
-        );
-        csm.submitInitialSlashing(noId, keyIndex);
-    }
-
     function test_submitInitialSlashing_RevertWhenNoNodeOperator() public {
         vm.expectRevert(NodeOperatorDoesNotExist.selector);
         csm.submitInitialSlashing(0, 0);