fix: ep negative shares bug (#1033)

* fix: ep negative shares bug

* fix: comments

* test: add integration tests for neg shares

* chore: remove logs

* chore: use already calculated delta

* chore: use stable foundry release in CI

.github/workflows/certora-prover.yml

@@ -32,7 +32,7 @@ jobs:
       - name: Install Foundry
         uses: foundry-rs/foundry-toolchain@v1
         with:
-          version: nightly
+          version: stable
       - name: Install forge dependencies
         run: forge install
       - name: Install python

.github/workflows/coverage.yml

@@ -50,7 +50,7 @@ jobs:
     - name: Install Foundry
       uses: foundry-rs/foundry-toolchain@v1
       with:
-        version: nightly
+        version: stable
     - name: Run coverage
       run: forge coverage --report lcov
       env:

.github/workflows/deploy-local.yml

@@ -30,7 +30,7 @@ jobs:
     - name: Install Foundry
       uses: foundry-rs/foundry-toolchain@v1
       with:
-        version: nightly
+        version: stable
     - name: Run forge install
       run: forge install
     - name: Start anvil and deploy

.github/workflows/format.yml

@@ -18,7 +18,7 @@ jobs:
       - name: Install Foundry
         uses: foundry-rs/foundry-toolchain@v1
         with:
-          version: nightly
+          version: stable
       - name: Run forge fmt
         run: |
           forge fmt --check src/contracts

.github/workflows/storage-report.yml

@@ -16,7 +16,7 @@ jobs:
       - name: Install Foundry
         uses: foundry-rs/foundry-toolchain@v1
         with:
-          version: nightly
+          version: stable
 
       - name: "Generate and prepare the storage reports for current branch"
         run: |

.github/workflows/testinparallel.yml

@@ -34,7 +34,7 @@ jobs:
     - name: Install Foundry
       uses: foundry-rs/foundry-toolchain@v1
       with:
-        version: nightly
+        version: stable
 
     - name: Run Forge build
       run: |

src/contracts/pods/EigenPodManager.sol

@@ -262,8 +262,14 @@ contract EigenPodManager is
         if (updatedDepositShares <= 0) {
             return (0, 0);
         }
-
-        return (prevDepositShares < 0 ? 0 : uint256(prevDepositShares), shares);
+        // If we have gone from negative to positive shares, return (0, positive delta)
+        else if (prevDepositShares < 0) {
+            return (0, uint256(updatedDepositShares));
+        }
+        // Else, return true previous shares and added shares
+        else {
+            return (uint256(prevDepositShares), shares);
+        }
     }
 
     /// @dev Calculates the proportion a pod owner's restaked balance has decreased, and

src/test/integration/IntegrationBase.t.sol

@@ -2017,7 +2017,13 @@ abstract contract IntegrationBase is IntegrationDeployer, TypeImporter {
     }
 
     function _calcNativeETHOperatorShareDelta(User staker, int shareDelta) internal view returns (int) {
-        int curPodOwnerShares = eigenPodManager.podOwnerDepositShares(address(staker));
+        // TODO: Maybe we update parent method to have an M2 and Slashing version?
+        int curPodOwnerShares;
+        if (!isUpgraded) {
+            curPodOwnerShares = IEigenPodManager_DeprecatedM2(address(eigenPodManager)).podOwnerShares(address(staker));
+        } else {
+            curPodOwnerShares = eigenPodManager.podOwnerDepositShares(address(staker));
+        }
         int newPodOwnerShares = curPodOwnerShares + shareDelta;
 
         if (curPodOwnerShares <= 0) {

src/test/integration/tests/upgrade/EigenPod_Negative_Shares.t.sol

@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: BUSL-1.1
+pragma solidity ^0.8.27;
+
+import "src/test/integration/UpgradeTest.t.sol";
+
+contract Integration_Upgrade_EigenPod_Negative_Shares is UpgradeTest {
+    function _init() internal override {
+        _configAssetTypes(HOLDS_ETH);
+        _configUserTypes(DEFAULT);
+    }
+
+    function testFuzz_deposit_delegate_updateBalance_upgrade_completeAsShares(
+        uint24 _rand
+    ) public rand(_rand) {
+        /// 0. Create an operator and staker with some underlying assets
+        (User staker, IStrategy[] memory strategies, uint256[] memory tokenBalances) = _newRandomStaker();
+        (User operator,,) = _newRandomOperator();
+        uint256[] memory shares = _calculateExpectedShares(strategies, tokenBalances);
+
+        ///  1. Deposit into strategies
+        staker.depositIntoEigenlayer(strategies, tokenBalances);
+
+        /// 2. Delegate to operator
+        staker.delegateTo(operator);
+
+        /// 3. Queue a withdrawal for all shares
+        IDelegationManagerTypes.Withdrawal[] memory withdrawals = staker.queueWithdrawals(strategies, shares);
+        IDelegationManagerTypes.Withdrawal memory withdrawal = withdrawals[0];
+
+        /// 4. Update balance randomly (can be positive or negative)
+        (int256[] memory tokenDeltas, int256[] memory balanceUpdateShareDelta,) = _randBalanceUpdate(staker, strategies);
+        staker.updateBalances(strategies, tokenDeltas);
+
+        /// 5. Upgrade contracts
+        _upgradeEigenLayerContracts();
+
+        /// 6. Complete the withdrawal as shares
+        _rollBlocksForCompleteWithdrawals(withdrawals);
+        staker.completeWithdrawalAsShares(withdrawal);
+
+        // Manually complete checks since we could still negative shares prior to the upgrade, causing a revert in the share check
+        (uint256[] memory expectedOperatorShareDelta, int256[] memory expectedStakerShareDelta) =
+            _getPostWithdrawalExpectedShareDeltas(balanceUpdateShareDelta[0], withdrawal);
+        assert_WithdrawalNotPending(delegationManager.calculateWithdrawalRoot(withdrawal), "staker withdrawal should no longer be pending");
+        assert_Snap_Unchanged_TokenBalances(staker, "staker should not have any change in underlying token balances");
+        assert_Snap_Added_OperatorShares(operator, withdrawal.strategies, expectedOperatorShareDelta, "operator should have received shares");
+        assert_Snap_Delta_StakerShares(staker, strategies, expectedStakerShareDelta, "staker should have received expected shares");
+    }
+
+    function testFuzz_deposit_delegate_updateBalance_upgrade_completeAsTokens(
+        uint24 _rand
+    ) public rand(_rand) {
+        /// 0. Create an operator and staker with some underlying assets
+        (User staker, IStrategy[] memory strategies, uint256[] memory tokenBalances) = _newRandomStaker();
+        (User operator,,) = _newRandomOperator();
+        uint256[] memory shares = _calculateExpectedShares(strategies, tokenBalances);
+
+        ///  1. Deposit into strategies
+        staker.depositIntoEigenlayer(strategies, tokenBalances);
+
+        /// 2. Delegate to operator
+        staker.delegateTo(operator);
+
+        /// 3. Queue a withdrawal for all shares
+        IDelegationManagerTypes.Withdrawal[] memory withdrawals = staker.queueWithdrawals(strategies, shares);
+        IDelegationManagerTypes.Withdrawal memory withdrawal = withdrawals[0];
+
+        /// 4. Update balance randomly (can be positive or negative)
+        (int256[] memory tokenDeltas, int256[] memory balanceUpdateShareDelta,) = _randBalanceUpdate(staker, strategies);
+        staker.updateBalances(strategies, tokenDeltas);
+
+        /// 5. Upgrade contracts
+        _upgradeEigenLayerContracts();
+
+        /// 6. Complete the withdrawal as shares
+        _rollBlocksForCompleteWithdrawals(withdrawals);
+        IERC20[] memory tokens = staker.completeWithdrawalAsTokens(withdrawal);
+        uint256[] memory expectedTokens = _getPostWithdrawalExpectedTokenDeltas(balanceUpdateShareDelta[0], withdrawal);
+
+        // Manually complete checks since we could still negative shares prior to the upgrade, causing a revert in the share check
+        assert_WithdrawalNotPending(delegationManager.calculateWithdrawalRoot(withdrawal), "staker withdrawal should no longer be pending");
+        assert_Snap_Added_TokenBalances(staker, tokens, expectedTokens, "staker should have received expected tokens");
+        assert_Snap_Unchanged_OperatorShares(operator, "operator shares should not have changed");
+
+        // If we had a positive balance update, then the staker shares should not have changed
+        if (balanceUpdateShareDelta[0] > 0) {
+            assert_Snap_Unchanged_Staker_DepositShares(staker, "staker shares should not have changed");
+        }
+        // Else, the staker shares should have increased by the magnitude of the negative share delta
+        else {
+            int256[] memory expectedStakerShareDelta = new int256[](1);
+            expectedStakerShareDelta[0] = -balanceUpdateShareDelta[0];
+            assert_Snap_Delta_StakerShares(staker, strategies, expectedStakerShareDelta, "staker should have received expected shares");
+        }
+    }
+
+
+
+    function _getPostWithdrawalExpectedShareDeltas(
+        int256 balanceUpdateShareDelta,
+        IDelegationManagerTypes.Withdrawal memory withdrawal
+    ) internal pure returns (uint256[] memory, int256[] memory) {
+        uint256[] memory operatorShareDelta = new uint256[](1);
+        int256[] memory stakerShareDelta = new int256[](1);
+        // The staker share delta is the withdrawal scaled shares since it can go from negative to positive
+        stakerShareDelta[0] = int256(withdrawal.scaledShares[0]);
+
+        if (balanceUpdateShareDelta > 0) {
+            // If balanceUpdateShareDelta is positive, then the operator delta is the withdrawal scaled shares
+            operatorShareDelta[0] = withdrawal.scaledShares[0];
+        } else {
+            // Operator shares never went negative, so we can just add the withdrawal scaled shares and the negative share delta
+            operatorShareDelta[0] = uint256(int256(withdrawal.scaledShares[0]) + balanceUpdateShareDelta);
+        }
+
+        return (operatorShareDelta, stakerShareDelta);
+    }
+
+    function _getPostWithdrawalExpectedTokenDeltas(
+        int256 balanceUpdateShareDelta,
+        IDelegationManagerTypes.Withdrawal memory withdrawal
+    ) internal pure returns (uint256[] memory) {
+        uint256[] memory expectedTokenDeltas = new uint256[](1);
+        if (balanceUpdateShareDelta > 0) {
+            // If we had a positive balance update, then the expected token delta is the withdrawal scaled shares
+            expectedTokenDeltas[0] = withdrawal.scaledShares[0];
+        } else {
+            // If we had a negative balance update, then the expected token delta is the withdrawal scaled shares plus the negative share delta
+            expectedTokenDeltas[0] = uint256(int256(withdrawal.scaledShares[0]) + balanceUpdateShareDelta);
+        }
+        return expectedTokenDeltas;
+    }
+}

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

@@ -458,9 +458,9 @@ contract User is Logger, IDelegationManagerTypes, IAllocationManagerTypes {
             if (strat == BEACONCHAIN_ETH_STRAT) {
                 tokens[i] = NATIVE_ETH;
 
-                // If we're withdrawing native ETH as tokens, stop ALL validators
-                // and complete a checkpoint
-                if (receiveAsTokens) {
+                // If we're withdrawing native ETH as tokens && do not have negative shares
+                // stop ALL validators and complete a checkpoint
+                if (receiveAsTokens && eigenPodManager.podOwnerDepositShares(address(this)) >= 0) {
                     console.log("- exiting all validators and completing checkpoint");
                     _exitValidators(getActiveValidators());
 

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

@@ -137,7 +137,7 @@ contract User_M2 is User {
                 // If any balance update has occured, a checkpoint will pick it up
                 _startCheckpoint();
                 if (pod.activeValidatorCount() != 0) {
-                    _completeCheckpoint();
+                    _completeCheckpoint_M2();
                 }
             } else {
                 uint256 tokens = uint256(delta);

src/test/unit/EigenPodManagerUnit.t.sol

@@ -185,6 +185,22 @@ contract EigenPodManagerUnitTests_StakeTests is EigenPodManagerUnitTests {
 
 contract EigenPodManagerUnitTests_ShareUpdateTests is EigenPodManagerUnitTests {
 
+    // Wrapper contract that exposes the internal `_calculateChangeInDelegatableShares` function
+    EigenPodManagerWrapper public eigenPodManagerWrapper;
+
+    function setUp() virtual override public {
+        super.setUp();
+
+        // Upgrade eigenPodManager to wrapper
+        eigenPodManagerWrapper = new EigenPodManagerWrapper(
+            ethPOSMock,
+            eigenPodBeacon,
+            IDelegationManager(address(delegationManagerMock)),
+            pauserRegistry
+        );
+        eigenLayerProxyAdmin.upgrade(ITransparentUpgradeableProxy(payable(address(eigenPodManager))), address(eigenPodManagerWrapper));
+    }
+
     /*******************************************************************************
                                 Add Shares Tests
     *******************************************************************************/
@@ -223,6 +239,54 @@ contract EigenPodManagerUnitTests_ShareUpdateTests is EigenPodManagerUnitTests {
         assertEq(eigenPodManager.podOwnerDepositShares(defaultStaker), int256(shares), "Incorrect number of shares added");
     }
 
+    function test_addShares_negativeInitial() public {
+        _initializePodWithShares(defaultStaker, -1);
+
+        cheats.prank(address(delegationManagerMock));
+
+        (uint256 prevDepositShares, uint256 addedShares) = eigenPodManager.addShares(
+            defaultStaker,
+            beaconChainETHStrategy,
+            5
+        );
+
+        assertEq(prevDepositShares, 0);
+        assertEq(addedShares, 4);
+    }
+
+    function testFuzz_addShares_negativeSharesInitial(int256 sharesToStart, int256 sharesToAdd) public {
+        cheats.assume(sharesToStart < 0);
+        cheats.assume(sharesToAdd >= 0);
+
+        _initializePodWithShares(defaultStaker, sharesToStart);
+        int256 expectedDepositShares = sharesToStart + sharesToAdd;
+
+        cheats.prank(address(delegationManagerMock));
+
+        cheats.expectEmit(true, true, true, true, address(eigenPodManager));
+        emit PodSharesUpdated(defaultStaker, sharesToAdd);
+        cheats.expectEmit(true, true, true, true, address(eigenPodManager));
+        emit NewTotalShares(defaultStaker, expectedDepositShares);
+
+        (uint256 prevDepositShares, uint256 addedShares) = eigenPodManager.addShares(
+            defaultStaker,
+            beaconChainETHStrategy,
+            uint256(sharesToAdd)
+        );
+
+        // validate that prev shares return 0 since we started from a negative balance
+        assertEq(prevDepositShares, 0);
+
+        // If we now have positive shares, expect return
+        if (expectedDepositShares > 0) {
+            assertEq(addedShares, uint256(expectedDepositShares));
+        } 
+        // We still have negative shares, return 0 
+        else {
+            assertEq(addedShares, 0);
+        }
+    }
+
     /*******************************************************************************
                                 Remove Shares Tests
     ******************************************************************************/