Skip to content

Commit bb97aca

Browse files
ypatil12ypatil12
committed
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
1 parent 777ff4a commit bb97aca

File tree

6 files changed

+216
-7
lines changed

6 files changed

+216
-7
lines changed

‎src/contracts/pods/EigenPodManager.sol‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,14 @@ contract EigenPodManager is
262262
if (updatedDepositShares <= 0) {
263263
return (0, 0);
264264
}
265-
266-
return (prevDepositShares < 0 ? 0 : uint256(prevDepositShares), shares);
265+
// If we have gone from negative to positive shares, return (0, positive delta)
266+
else if (prevDepositShares < 0) {
267+
return (0, uint256(updatedDepositShares));
268+
}
269+
// Else, return true previous shares and added shares
270+
else {
271+
return (uint256(prevDepositShares), shares);
272+
}
267273
}
268274

269275
/// @dev Calculates the proportion a pod owner's restaked balance has decreased, and

‎src/test/integration/IntegrationBase.t.sol‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2017,7 +2017,13 @@ abstract contract IntegrationBase is IntegrationDeployer, TypeImporter {
20172017
}
20182018

20192019
function _calcNativeETHOperatorShareDelta(User staker, int shareDelta) internal view returns (int) {
2020-
int curPodOwnerShares = eigenPodManager.podOwnerDepositShares(address(staker));
2020+
// TODO: Maybe we update parent method to have an M2 and Slashing version?
2021+
int curPodOwnerShares;
2022+
if (!isUpgraded) {
2023+
curPodOwnerShares = IEigenPodManager_DeprecatedM2(address(eigenPodManager)).podOwnerShares(address(staker));
2024+
} else {
2025+
curPodOwnerShares = eigenPodManager.podOwnerDepositShares(address(staker));
2026+
}
20212027
int newPodOwnerShares = curPodOwnerShares + shareDelta;
20222028

20232029
if (curPodOwnerShares <= 0) {

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

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
// SPDX-License-Identifier: BUSL-1.1
2+
pragma solidity ^0.8.27;
3+
4+
import "src/test/integration/UpgradeTest.t.sol";
5+
6+
contract Integration_Upgrade_EigenPod_Negative_Shares is UpgradeTest {
7+
function _init() internal override {
8+
_configAssetTypes(HOLDS_ETH);
9+
_configUserTypes(DEFAULT);
10+
}
11+
12+
function testFuzz_deposit_delegate_updateBalance_upgrade_completeAsShares(
13+
uint24 _rand
14+
) public rand(_rand) {
15+
/// 0. Create an operator and staker with some underlying assets
16+
(User staker, IStrategy[] memory strategies, uint256[] memory tokenBalances) = _newRandomStaker();
17+
(User operator,,) = _newRandomOperator();
18+
uint256[] memory shares = _calculateExpectedShares(strategies, tokenBalances);
19+
20+
/// 1. Deposit into strategies
21+
staker.depositIntoEigenlayer(strategies, tokenBalances);
22+
23+
/// 2. Delegate to operator
24+
staker.delegateTo(operator);
25+
26+
/// 3. Queue a withdrawal for all shares
27+
IDelegationManagerTypes.Withdrawal[] memory withdrawals = staker.queueWithdrawals(strategies, shares);
28+
IDelegationManagerTypes.Withdrawal memory withdrawal = withdrawals[0];
29+
30+
/// 4. Update balance randomly (can be positive or negative)
31+
(int256[] memory tokenDeltas, int256[] memory balanceUpdateShareDelta,) = _randBalanceUpdate(staker, strategies);
32+
staker.updateBalances(strategies, tokenDeltas);
33+
34+
/// 5. Upgrade contracts
35+
_upgradeEigenLayerContracts();
36+
37+
/// 6. Complete the withdrawal as shares
38+
_rollBlocksForCompleteWithdrawals(withdrawals);
39+
staker.completeWithdrawalAsShares(withdrawal);
40+
41+
// Manually complete checks since we could still negative shares prior to the upgrade, causing a revert in the share check
42+
(uint256[] memory expectedOperatorShareDelta, int256[] memory expectedStakerShareDelta) =
43+
_getPostWithdrawalExpectedShareDeltas(balanceUpdateShareDelta[0], withdrawal);
44+
assert_WithdrawalNotPending(delegationManager.calculateWithdrawalRoot(withdrawal), "staker withdrawal should no longer be pending");
45+
assert_Snap_Unchanged_TokenBalances(staker, "staker should not have any change in underlying token balances");
46+
assert_Snap_Added_OperatorShares(operator, withdrawal.strategies, expectedOperatorShareDelta, "operator should have received shares");
47+
assert_Snap_Delta_StakerShares(staker, strategies, expectedStakerShareDelta, "staker should have received expected shares");
48+
}
49+
50+
function testFuzz_deposit_delegate_updateBalance_upgrade_completeAsTokens(
51+
uint24 _rand
52+
) public rand(_rand) {
53+
/// 0. Create an operator and staker with some underlying assets
54+
(User staker, IStrategy[] memory strategies, uint256[] memory tokenBalances) = _newRandomStaker();
55+
(User operator,,) = _newRandomOperator();
56+
uint256[] memory shares = _calculateExpectedShares(strategies, tokenBalances);
57+
58+
/// 1. Deposit into strategies
59+
staker.depositIntoEigenlayer(strategies, tokenBalances);
60+
61+
/// 2. Delegate to operator
62+
staker.delegateTo(operator);
63+
64+
/// 3. Queue a withdrawal for all shares
65+
IDelegationManagerTypes.Withdrawal[] memory withdrawals = staker.queueWithdrawals(strategies, shares);
66+
IDelegationManagerTypes.Withdrawal memory withdrawal = withdrawals[0];
67+
68+
/// 4. Update balance randomly (can be positive or negative)
69+
(int256[] memory tokenDeltas, int256[] memory balanceUpdateShareDelta,) = _randBalanceUpdate(staker, strategies);
70+
staker.updateBalances(strategies, tokenDeltas);
71+
72+
/// 5. Upgrade contracts
73+
_upgradeEigenLayerContracts();
74+
75+
/// 6. Complete the withdrawal as shares
76+
_rollBlocksForCompleteWithdrawals(withdrawals);
77+
IERC20[] memory tokens = staker.completeWithdrawalAsTokens(withdrawal);
78+
uint256[] memory expectedTokens = _getPostWithdrawalExpectedTokenDeltas(balanceUpdateShareDelta[0], withdrawal);
79+
80+
// Manually complete checks since we could still negative shares prior to the upgrade, causing a revert in the share check
81+
assert_WithdrawalNotPending(delegationManager.calculateWithdrawalRoot(withdrawal), "staker withdrawal should no longer be pending");
82+
assert_Snap_Added_TokenBalances(staker, tokens, expectedTokens, "staker should have received expected tokens");
83+
assert_Snap_Unchanged_OperatorShares(operator, "operator shares should not have changed");
84+
85+
// If we had a positive balance update, then the staker shares should not have changed
86+
if (balanceUpdateShareDelta[0] > 0) {
87+
assert_Snap_Unchanged_Staker_DepositShares(staker, "staker shares should not have changed");
88+
}
89+
// Else, the staker shares should have increased by the magnitude of the negative share delta
90+
else {
91+
int256[] memory expectedStakerShareDelta = new int256[](1);
92+
expectedStakerShareDelta[0] = -balanceUpdateShareDelta[0];
93+
assert_Snap_Delta_StakerShares(staker, strategies, expectedStakerShareDelta, "staker should have received expected shares");
94+
}
95+
}
96+
97+
98+
99+
function _getPostWithdrawalExpectedShareDeltas(
100+
int256 balanceUpdateShareDelta,
101+
IDelegationManagerTypes.Withdrawal memory withdrawal
102+
) internal pure returns (uint256[] memory, int256[] memory) {
103+
uint256[] memory operatorShareDelta = new uint256[](1);
104+
int256[] memory stakerShareDelta = new int256[](1);
105+
// The staker share delta is the withdrawal scaled shares since it can go from negative to positive
106+
stakerShareDelta[0] = int256(withdrawal.scaledShares[0]);
107+
108+
if (balanceUpdateShareDelta > 0) {
109+
// If balanceUpdateShareDelta is positive, then the operator delta is the withdrawal scaled shares
110+
operatorShareDelta[0] = withdrawal.scaledShares[0];
111+
} else {
112+
// Operator shares never went negative, so we can just add the withdrawal scaled shares and the negative share delta
113+
operatorShareDelta[0] = uint256(int256(withdrawal.scaledShares[0]) + balanceUpdateShareDelta);
114+
}
115+
116+
return (operatorShareDelta, stakerShareDelta);
117+
}
118+
119+
function _getPostWithdrawalExpectedTokenDeltas(
120+
int256 balanceUpdateShareDelta,
121+
IDelegationManagerTypes.Withdrawal memory withdrawal
122+
) internal pure returns (uint256[] memory) {
123+
uint256[] memory expectedTokenDeltas = new uint256[](1);
124+
if (balanceUpdateShareDelta > 0) {
125+
// If we had a positive balance update, then the expected token delta is the withdrawal scaled shares
126+
expectedTokenDeltas[0] = withdrawal.scaledShares[0];
127+
} else {
128+
// If we had a negative balance update, then the expected token delta is the withdrawal scaled shares plus the negative share delta
129+
expectedTokenDeltas[0] = uint256(int256(withdrawal.scaledShares[0]) + balanceUpdateShareDelta);
130+
}
131+
return expectedTokenDeltas;
132+
}
133+
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -458,9 +458,9 @@ contract User is Logger, IDelegationManagerTypes, IAllocationManagerTypes {
458458
if (strat == BEACONCHAIN_ETH_STRAT) {
459459
tokens[i] = NATIVE_ETH;
460460

461-
// If we're withdrawing native ETH as tokens, stop ALL validators
462-
// and complete a checkpoint
463-
if (receiveAsTokens) {
461+
// If we're withdrawing native ETH as tokens && do not have negative shares
462+
// stop ALL validators and complete a checkpoint
463+
if (receiveAsTokens && eigenPodManager.podOwnerDepositShares(address(this)) >= 0) {
464464
console.log("- exiting all validators and completing checkpoint");
465465
_exitValidators(getActiveValidators());
466466

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ contract User_M2 is User {
137137
// If any balance update has occured, a checkpoint will pick it up
138138
_startCheckpoint();
139139
if (pod.activeValidatorCount() != 0) {
140-
_completeCheckpoint();
140+
_completeCheckpoint_M2();
141141
}
142142
} else {
143143
uint256 tokens = uint256(delta);

‎src/test/unit/EigenPodManagerUnit.t.sol‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,22 @@ contract EigenPodManagerUnitTests_StakeTests is EigenPodManagerUnitTests {
185185

186186
contract EigenPodManagerUnitTests_ShareUpdateTests is EigenPodManagerUnitTests {
187187

188+
// Wrapper contract that exposes the internal `_calculateChangeInDelegatableShares` function
189+
EigenPodManagerWrapper public eigenPodManagerWrapper;
190+
191+
function setUp() virtual override public {
192+
super.setUp();
193+
194+
// Upgrade eigenPodManager to wrapper
195+
eigenPodManagerWrapper = new EigenPodManagerWrapper(
196+
ethPOSMock,
197+
eigenPodBeacon,
198+
IDelegationManager(address(delegationManagerMock)),
199+
pauserRegistry
200+
);
201+
eigenLayerProxyAdmin.upgrade(ITransparentUpgradeableProxy(payable(address(eigenPodManager))), address(eigenPodManagerWrapper));
202+
}
203+
188204
/*******************************************************************************
189205
Add Shares Tests
190206
*******************************************************************************/
@@ -223,6 +239,54 @@ contract EigenPodManagerUnitTests_ShareUpdateTests is EigenPodManagerUnitTests {
223239
assertEq(eigenPodManager.podOwnerDepositShares(defaultStaker), int256(shares), "Incorrect number of shares added");
224240
}
225241

242+
function test_addShares_negativeInitial() public {
243+
_initializePodWithShares(defaultStaker, -1);
244+
245+
cheats.prank(address(delegationManagerMock));
246+
247+
(uint256 prevDepositShares, uint256 addedShares) = eigenPodManager.addShares(
248+
defaultStaker,
249+
beaconChainETHStrategy,
250+
5
251+
);
252+
253+
assertEq(prevDepositShares, 0);
254+
assertEq(addedShares, 4);
255+
}
256+
257+
function testFuzz_addShares_negativeSharesInitial(int256 sharesToStart, int256 sharesToAdd) public {
258+
cheats.assume(sharesToStart < 0);
259+
cheats.assume(sharesToAdd >= 0);
260+
261+
_initializePodWithShares(defaultStaker, sharesToStart);
262+
int256 expectedDepositShares = sharesToStart + sharesToAdd;
263+
264+
cheats.prank(address(delegationManagerMock));
265+
266+
cheats.expectEmit(true, true, true, true, address(eigenPodManager));
267+
emit PodSharesUpdated(defaultStaker, sharesToAdd);
268+
cheats.expectEmit(true, true, true, true, address(eigenPodManager));
269+
emit NewTotalShares(defaultStaker, expectedDepositShares);
270+
271+
(uint256 prevDepositShares, uint256 addedShares) = eigenPodManager.addShares(
272+
defaultStaker,
273+
beaconChainETHStrategy,
274+
uint256(sharesToAdd)
275+
);
276+
277+
// validate that prev shares return 0 since we started from a negative balance
278+
assertEq(prevDepositShares, 0);
279+
280+
// If we now have positive shares, expect return
281+
if (expectedDepositShares > 0) {
282+
assertEq(addedShares, uint256(expectedDepositShares));
283+
}
284+
// We still have negative shares, return 0
285+
else {
286+
assertEq(addedShares, 0);
287+
}
288+
}
289+
226290
/*******************************************************************************
227291
Remove Shares Tests
228292
******************************************************************************/

0 commit comments

Comments
 (0)