diff --git a/src/CSAccounting.sol b/src/CSAccounting.sol index 77a0089e..e9d7c03d 100644 --- 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 index ae7370bd..e094ddf3 100644 --- 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 index 203154ef..ba4bbd5e 100644 --- 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 index b94b3997..31c1ad42 100644 --- 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);