fix: DSF increase on checkpointed nonzero shares

docs/core/EigenPodManager.md

@@ -320,14 +320,15 @@ function recordBeaconChainETHBalanceUpdate(
     nonReentrant
 ```
 
-This method is called by an `EigenPod` to report a change in its pod owner's shares. It accepts a positive or negative `balanceDeltaWei`. A positive delta is added to the pod owner's _deposit shares,_ and delegated to their operator if applicable. A negative delta is NOT removed from the pod owner's deposit shares. Instead, the proportion of the balance decrease is used to update the pod owner's beacon chain slashing factor and decrease the number of shares delegated to their operator (if applicable).
+This method is called by an `EigenPod` to report a change in its pod owner's shares. It accepts a positive or negative `balanceDeltaWei`. A positive delta is added to the pod owner's _deposit shares,_ and delegated to their operator if applicable. A negative delta is NOT removed from the pod owner's deposit shares. Instead, the proportion of the balance decrease is used to update the pod owner's beacon chain slashing factor and decrease the number of shares delegated to their operator (if applicable). A zero delta results in no change. 
 
 **Note** that prior to the slashing release, negative balance deltas subtracted from the pod owner's shares, and could, in certain cases, result in a negative share balance. As of the slashing release, negative balance deltas no longer subtract from share balances, updating the beacon chain slashing factor instead. 
 
 If a staker has negative shares as of the slashing release, this method will REVERT, preventing any further balance updates from their pod while the negative share balance persists. In order to fix this and restore the use of their pod, the staker should complete any outstanding withdrawals in the `DelegationManager` "as shares," which will correct the share deficit.
 
 *Effects*:
-* If `balanceDeltaWei` is positive or 0:
+* If `balanceDeltaWei` is zero, do nothing
+* If `balanceDeltaWei` is positive:
   * Adds `shares` to `podOwnerDepositShares` for `podOwner`
   * Emits `PodSharesUpdated` and `NewTotalShares` events
   * Calls [`DelegationManager.increaseDelegatedShares`](./DelegationManager.md#increasedelegatedshares)

src/contracts/core/DelegationManager.sol

@@ -646,6 +646,11 @@ contract DelegationManager is
         // This is to prevent a divWad by 0 when updating the depositScalingFactor
         require(slashingFactor != 0, FullySlashed());
 
+        // If `addedShares` is 0, do nothing
+        if (addedShares == 0) {
+            return;
+        }
+
         // Update the staker's depositScalingFactor. This only results in an update
         // if the slashing factor has changed for this strategy.
         DepositScalingFactor storage dsf = _depositScalingFactor[staker][strategy];

src/contracts/pods/EigenPodManager.sol

@@ -106,10 +106,10 @@ contract EigenPodManager is
         int256 currentDepositShares = podOwnerDepositShares[podOwner];
         require(currentDepositShares >= 0, LegacyWithdrawalsNotCompleted());
 
-        // Shares are only added to the pod owner's balance when `balanceDeltaWei` >= 0. When a pod reports
+        // Shares are only added to the pod owner's balance when `balanceDeltaWei` > 0. When a pod reports
         // a negative balance delta, the pod owner's beacon chain slashing factor is decreased, devaluing
-        // their shares.
-        if (balanceDeltaWei >= 0) {
+        // their shares. If the delta is zero, then no action needs to be taken.
+        if (balanceDeltaWei > 0) {
             (uint256 prevDepositShares, uint256 addedShares) = _addShares(podOwner, uint256(balanceDeltaWei));
 
             // Update operator shares
@@ -119,7 +119,7 @@ contract EigenPodManager is
                 prevDepositShares: prevDepositShares,
                 addedShares: addedShares
             });
-        } else {
+        } else if (balanceDeltaWei < 0) {
             uint64 beaconChainSlashingFactorDecrease = _reduceSlashingFactor({
                 podOwner: podOwner,
                 prevRestakedBalanceWei: prevRestakedBalanceWei,

src/test/integration/IntegrationBase.t.sol

@@ -1541,6 +1541,19 @@ abstract contract IntegrationBase is IntegrationDeployer, TypeImporter {
         assert_Snap_Removed_Staker_WithdrawableShares(staker, strat.toArray(), removedShares.toArrayU256(), err);
     }
 
+    function assert_Snap_Unchanged_Staker_WithdrawableShares(
+        User staker,
+        IStrategy[] memory strategies,
+        string memory err
+    ) internal {
+        uint[] memory curShares = _getStakerWithdrawableShares(staker, strategies);
+        uint[] memory prevShares = _getPrevStakerWithdrawableShares(staker, strategies);
+        // For each strategy, check all shares have been withdrawn
+        for (uint i = 0; i < strategies.length; i++) {
+            assertEq(prevShares[i], curShares[i], err);
+        }
+    }
+
     /// @dev Check that the staker's withdrawable shares have decreased by `removedShares`
     /// FIX THIS WHEN WORKING ON ROUNDING ISSUES
     function assert_Snap_Unchanged_Staker_WithdrawableShares_Delegation(
@@ -1602,6 +1615,19 @@ abstract contract IntegrationBase is IntegrationDeployer, TypeImporter {
         }
     }
 
+    function assert_Snap_Unchanged_DSF(
+        User staker,
+        IStrategy[] memory strategies,
+        string memory err
+    ) internal {
+        uint[] memory curDSFs = _getDepositScalingFactors(staker, strategies);
+        uint[] memory prevDSFs = _getPrevDepositScalingFactors(staker, strategies);
+
+        for (uint i = 0; i < strategies.length; i++) {
+            assertEq(prevDSFs[i], curDSFs[i], err);
+        }
+    }
+
     /*******************************************************************************
                         SNAPSHOT ASSERTIONS: STRATEGY SHARES
     *******************************************************************************/
@@ -2068,7 +2094,7 @@ abstract contract IntegrationBase is IntegrationDeployer, TypeImporter {
         params.wadsToSlash = new uint[](params.strategies.length);
 
         for (uint i = 0; i < params.wadsToSlash.length; i++) {
-            params.wadsToSlash[i] = 1e17;
+            params.wadsToSlash[i] = 5e17;
         }
     }
 
@@ -2709,6 +2735,10 @@ abstract contract IntegrationBase is IntegrationDeployer, TypeImporter {
         return curShares;
     }
 
+    function _getExpectedWithdrawableSharesAfterCompletion(User staker, uint scaledShares, uint depositScalingFactor, uint slashingFactor) internal view returns (uint) {
+        return scaledShares.mulWad(depositScalingFactor).mulWad(slashingFactor);
+    }
+
     function _getPrevStakerWithdrawableShares(User staker, IStrategy[] memory strategies) internal timewarp() returns (uint[] memory) {
         return _getStakerWithdrawableShares(staker, strategies);
     }
@@ -2783,6 +2813,10 @@ abstract contract IntegrationBase is IntegrationDeployer, TypeImporter {
         return delegationManager.depositScalingFactor(address(staker), strategy);
     }
 
+    function _getPrevDepositScalingFactors(User staker, IStrategy[] memory strategies) internal timewarp() returns (uint[] memory) {
+        return _getDepositScalingFactors(staker, strategies);
+    }
+
     function _getExpectedDSFUndelegate(User staker) internal view returns (uint expectedDepositScalingFactor) {
         return WAD.divWad(_getBeaconChainSlashingFactor(staker));
     }
@@ -2875,4 +2909,21 @@ abstract contract IntegrationBase is IntegrationDeployer, TypeImporter {
     function _getPrevCheckpointBalanceExited(User staker, uint64 checkpointTimestamp) internal timewarp() returns (uint64) {
         return _getCheckpointBalanceExited(staker, checkpointTimestamp);
     }
+
+    function _getQueuedWithdrawals(User staker) internal view returns (Withdrawal[] memory) {
+        (Withdrawal[] memory withdrawals,) = delegationManager.getQueuedWithdrawals(address(staker));
+        return withdrawals;
+    }
+
+    function _getSlashingFactor(
+        User staker,
+        IStrategy strategy
+    ) internal view returns (uint256) {
+        address operator = delegationManager.delegatedTo(address(staker));
+        uint64 maxMagnitude = allocationManager.getMaxMagnitudes(operator, strategy.toArray())[0];
+        if (strategy == BEACONCHAIN_ETH_STRAT) {
+            return maxMagnitude.mulWad(eigenPodManager.beaconChainSlashingFactor(address(staker)));
+        }
+        return maxMagnitude;
+    }
 }

src/test/integration/IntegrationChecks.t.sol

@@ -8,7 +8,7 @@ import "src/test/integration/users/User_M2.t.sol";
 
 /// @notice Contract that provides utility functions to reuse common test blocks & checks
 contract IntegrationCheckUtils is IntegrationBase {
-    using ArrayLib for IStrategy[];
+    using ArrayLib for *;
     using SlashingLib for *;
     using StdStyle for *;
 
@@ -23,6 +23,7 @@ contract IntegrationCheckUtils is IntegrationBase {
     ) internal {
         uint beaconBalanceWei = beaconBalanceGwei * GWEI_TO_WEI;
         assert_Snap_Added_Staker_DepositShares(staker, BEACONCHAIN_ETH_STRAT, beaconBalanceWei, "staker should have added deposit shares to beacon chain strat");
+        assert_Snap_Added_Staker_WithdrawableShares(staker, BEACONCHAIN_ETH_STRAT.toArray(), beaconBalanceWei.toArrayU256(), "staker should have added withdrawable shares to beacon chain strat");
         assert_Snap_Added_ActiveValidatorCount(staker, validators.length, "staker should have increased active validator count");
         assert_Snap_Added_ActiveValidators(staker, validators, "validators should each be active");
     }
@@ -95,6 +96,16 @@ contract IntegrationCheckUtils is IntegrationBase {
         assert_Snap_Removed_ActiveValidators(staker, slashedValidators, "exited validators should each be WITHDRAWN");
     }
 
+    function check_CompleteCheckpoint_ZeroBalanceDelta_State(
+        User staker
+    ) internal {
+        check_CompleteCheckpoint_State(staker);
+
+        assert_Snap_Unchanged_Staker_DepositShares(staker, "staker deposit shares should not have decreased");
+        assert_Snap_Unchanged_Staker_WithdrawableShares(staker, BEACONCHAIN_ETH_STRAT.toArray(), "staker withdrawable shares should not have decreased");
+        assert_Snap_Unchanged_DSF(staker, BEACONCHAIN_ETH_STRAT.toArray(), "staker DSF should not have changed");
+    }
+
     function check_CompleteCheckpoint_WithSlashing_HandleRoundDown_State(
         User staker,
         uint40[] memory slashedValidators,

src/test/integration/tests/Slashed_Eigenpod_AVS.t.sol

@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: BUSL-1.1
+pragma solidity ^0.8.27;
+
+import "src/test/integration/IntegrationChecks.t.sol";
+
+contract Integration_SlashedEigenpod_AVS_Base is IntegrationCheckUtils {
+    using ArrayLib for *;
+    using SlashingLib for *;
+    using Math for uint256;
+
+    AVS avs;
+    OperatorSet operatorSet;
+
+    User operator;
+    AllocateParams allocateParams;
+    SlashingParams slashParams;
+
+    User staker;
+    IStrategy[] strategies;
+    uint[] initTokenBalances;
+    uint[] initDepositShares;
+
+    function _init() internal virtual override {
+        _configAssetTypes(HOLDS_ETH);
+        (staker, strategies, initTokenBalances) = _newRandomStaker();
+        (operator,,) = _newRandomOperator();
+        (avs,) = _newRandomAVS();
+
+        cheats.assume(initTokenBalances[0] >= 64 ether);
+
+        // 1. Deposit Into Strategies
+        staker.depositIntoEigenlayer(strategies, initTokenBalances);
+        initDepositShares = _calculateExpectedShares(strategies, initTokenBalances);
+        check_Deposit_State(staker, strategies, initDepositShares);
+
+        // 2. Delegate to an operator
+        staker.delegateTo(operator);
+        check_Delegation_State(staker, operator, strategies, initDepositShares);
+
+        // 3. Create an operator set and register an operator.
+        operatorSet = avs.createOperatorSet(strategies);
+
+        // 4. Register for operator set
+        operator.registerForOperatorSet(operatorSet);
+        check_Registration_State_NoAllocation(operator, operatorSet, allStrats);
+
+        // 5. Allocate to operator set
+        allocateParams = _genAllocation_AllAvailable(operator, operatorSet);
+        operator.modifyAllocations(allocateParams);
+        check_IncrAlloc_State_Slashable(operator, allocateParams);
+        _rollBlocksForCompleteAllocation(operator, operatorSet, strategies);
+    }
+}
+
+contract Integration_SlashedEigenpod_AVS_Checkpoint is Integration_SlashedEigenpod_AVS_Base {
+
+    function _init() internal override {
+        super._init();
+
+        // 6. Slash
+        slashParams = _genSlashing_Rand(operator, operatorSet);
+        avs.slashOperator(slashParams);
+        check_Base_Slashing_State(operator, allocateParams, slashParams);
+
+        beaconChain.advanceEpoch_NoRewards();
+    }
+
+    /// @dev Asserts that the DSF isn't updated after a slash & checkpoint with 0 balance
+    function testFuzz_deposit_delegate_allocate_slash_checkpointZeroBalance(uint24 _rand) public rand(_rand) {
+        // 7. Start & complete checkpoint
+        staker.startCheckpoint();
+        check_StartCheckpoint_State(staker);
+        staker.completeCheckpoint();
+        check_CompleteCheckpoint_ZeroBalanceDelta_State(staker);
+    }
+}
+
+contract Integration_SlashedEigenpod_AVS_Withdraw is Integration_SlashedEigenpod_AVS_Base {
+
+    function _init() internal override {
+        super._init();
+
+        // Slash or queue a withdrawal in a random order
+        if (_randBool()) { // Slash -> Queue Withdrawal
+            // 7. Slash
+            slashParams = _genSlashing_Half(operator, operatorSet);
+            avs.slashOperator(slashParams);
+            check_Base_Slashing_State(operator, allocateParams, slashParams);
+
+            // 8. Queue Withdrawal for all shares. TODO: add proper assertion
+            staker.queueWithdrawals(strategies, initDepositShares);
+        } else { // Queue Withdrawal -> Slash
+            // 7. Queue Withdrawal for all shares
+            Withdrawal[] memory withdrawals = staker.queueWithdrawals(strategies, initDepositShares);
+            bytes32[] memory withdrawalRoots = _getWithdrawalHashes(withdrawals);
+            check_QueuedWithdrawal_State(staker, operator, strategies, initDepositShares, withdrawals, withdrawalRoots);
+
+            // 8. Slash
+            slashParams = _genSlashing_Half(operator, operatorSet);
+            avs.slashOperator(slashParams);
+            check_Base_Slashing_State(operator, allocateParams, slashParams);
+        }
+
+        beaconChain.advanceEpoch_NoRewards();
+    }
+
+    /// @dev Asserts that the DSF isn't updated after a slash/queue and a checkpoint with 0 balance. 
+    /// @dev The staker should subsequently not be able to inflate their withdrawable shares
+    function testFuzz_deposit_delegate_allocate_slashAndQueue_checkpoint_redeposit(uint24 _rand) public rand(_rand) {
+        // 9.  Start & complete checkpoint. 
+        staker.startCheckpoint();
+        check_StartCheckpoint_State(staker);
+        staker.completeCheckpoint();
+        check_CompleteCheckpoint_ZeroBalanceDelta_State(staker);
+
+        // 10. Redeposit
+        cheats.deal(address(staker), 32 ether);
+        (uint40[] memory newValidators, uint64 addedBeaconBalanceGwei) = staker.startValidators();
+        beaconChain.advanceEpoch_NoRewards();
+        staker.verifyWithdrawalCredentials(newValidators);
+        check_VerifyWC_State(staker, newValidators, addedBeaconBalanceGwei);
+    }
+
+    /// @dev Asserts that the staker cannot inflate withdrawable shares after redepositing
+    function testFuzz_deposit_delegate_allocate_slashAndQueue_completeAsTokens_redeposit(uint24 _rand) public rand(_rand) {
+        Withdrawal[] memory withdrawals = _getQueuedWithdrawals(staker);
+        _rollBlocksForCompleteWithdrawals(withdrawals);
+
+        // 9. Complete withdrawal as tokens
+        for (uint256 i = 0; i < withdrawals.length; ++i) {
+            uint256[] memory expectedTokens =
+                _calculateExpectedTokens(withdrawals[i].strategies, withdrawals[i].scaledShares);
+            staker.completeWithdrawalAsTokens(withdrawals[i]);
+            check_Withdrawal_AsTokens_State_AfterSlash(staker, operator, withdrawals[i], allocateParams, slashParams, expectedTokens);
+        }
+
+        // 10. Redeposit
+        cheats.deal(address(staker), 32 ether);
+        (uint40[] memory newValidators, uint64 addedBeaconBalanceGwei) = staker.startValidators();
+        beaconChain.advanceEpoch_NoRewards();
+        staker.verifyWithdrawalCredentials(newValidators);
+        check_VerifyWC_State(staker, newValidators, addedBeaconBalanceGwei);
+    }
+
+    /// @dev Asserts that the staker cannot inflate withdrawable shares after checkpointing & completing as shares
+    function testFuzz_deposit_delegate_allocate_slashAndQueue_checkPoint_completeAsShares(uint24 _rand) public rand(_rand) {
+        Withdrawal[] memory withdrawals = _getQueuedWithdrawals(staker);
+        _rollBlocksForCompleteWithdrawals(withdrawals);
+        uint slashingFactor = _getSlashingFactor(staker, BEACONCHAIN_ETH_STRAT);
+        uint depositScalingFactor = _getDepositScalingFactor(staker, BEACONCHAIN_ETH_STRAT);
+
+        // 9.  Start & complete checkpoint, since the next step does not. 
+        staker.startCheckpoint();
+        check_StartCheckpoint_State(staker);
+        staker.completeCheckpoint();
+        check_CompleteCheckpoint_ZeroBalanceDelta_State(staker);
+
+        // 10. Complete withdrawal as shares. Deposit scaling factor is doubled because operator was slashed by half.
+        staker.completeWithdrawalAsShares(withdrawals[0]);
+        check_Withdrawal_AsShares_State_AfterSlash(staker, operator, withdrawals[0], allocateParams, slashParams);
+        assertEq(
+            _getStakerWithdrawableShares(staker, strategies)[0],
+            _getExpectedWithdrawableSharesAfterCompletion(staker, withdrawals[0].scaledShares[0], depositScalingFactor * 2, slashingFactor),
+            "withdrawable shares not incremented correctly"
+        );
+    }
+}

src/test/integration/tests/Slashed_Eigenpod.t.sol → src/test/integration/tests/Slashed_Eigenpod_BC.t.sol

@@ -3,7 +3,7 @@ pragma solidity ^0.8.27;
 
 import "src/test/integration/IntegrationChecks.t.sol";
 
-contract Integration_SlashedEigenpod is IntegrationCheckUtils {
+contract Integration_SlashedEigenpod_BC is IntegrationCheckUtils {
     using ArrayLib for *;
 
     AVS avs;

src/test/integration/users/User.t.sol

@@ -645,18 +645,6 @@ contract User is Logger, IDelegationManagerTypes, IAllocationManagerTypes {
         return abi.encodePacked(bytes1(uint8(1)), bytes11(0), address(pod));
     }
 
-    function _getSlashingFactor(
-        address staker,
-        IStrategy strategy
-    ) internal view returns (uint256) {
-        address operator = delegationManager.delegatedTo(staker);
-        uint64 maxMagnitude = allocationManager().getMaxMagnitudes(operator, strategy.toArray())[0];
-        if (strategy == beaconChainETHStrategy) {
-            return maxMagnitude.mulWad(eigenPodManager.beaconChainSlashingFactor(staker));
-        }
-        return maxMagnitude;
-    }
-
     /// @notice Gets the expected withdrawals to be created when the staker is undelegated via a call to `DelegationManager.undelegate()`
     /// @notice Assumes staker and withdrawer are the same and that all strategies and shares are withdrawn
     function _getExpectedWithdrawalStructsForStaker(

src/test/unit/DelegationUnit.t.sol

@@ -3075,8 +3075,12 @@ contract DelegationManagerUnitTests_increaseDelegatedShares is DelegationManager
         assertFalse(delegationManager.isDelegated(staker), "bad test setup");
 
         cheats.prank(address(strategyManagerMock));
+        vm.recordLogs();
         delegationManager.increaseDelegatedShares(staker, strategyMock, 0, 0);
         assertEq(delegationManager.operatorShares(defaultOperator, strategyMock), 0, "shares should not have changed");
+
+        Vm.Log[] memory entries = vm.getRecordedLogs();
+        assertEq(0, entries.length, "should not have emitted any events");
     }
 
     /**