Skip to content

Commit 02b13f0

Browse files
authored
test: improve slashing invariants and refactor user generation logic (#1149)
**Motivation:** Improve slashing invariants, fix existing DM/ALM invariants, and refactor user generation logic **Modifications:** * Improve `IntegrationDeployer._randUser` generation logic so that additional helpers can be implemented to generate users with specific assets * Update some withdrawal invariants to distinguish between deposit and withdrawable shares. More work to be done here. * Implement several new tests in `SlashingWithdrawals` * Removes the vscode settings file since it was autoformatting tests. Spoke with Rajath about this - i'm fine to revisit this after slashing, but the current impact of this file is that it will create massive diffs across the most active part of the codebase while we're trying to get critical last-mile testing finished. We'll readd this after slashing :+1: **Followup**: * Additional cleanup for queue/complete withdrawal invariants and calling conventions * Additional tests needed in `SlashingWithdrawals` * Additional staker-level invariants needed when slashing an operator
1 parent ee0e51a commit 02b13f0

12 files changed

+450
-562
lines changed

.vscode/settings.json

Lines changed: 0 additions & 10 deletions
This file was deleted.

src/test/integration/IntegrationBase.t.sol

Lines changed: 105 additions & 145 deletions
Large diffs are not rendered by default.

src/test/integration/IntegrationChecks.t.sol

Lines changed: 27 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,8 @@ contract IntegrationCheckUtils is IntegrationBase {
210210
User staker,
211211
User operator,
212212
IStrategy[] memory strategies,
213-
uint[] memory shares,
213+
uint[] memory depositShares,
214+
uint[] memory withdrawableShares,
214215
Withdrawal[] memory withdrawals,
215216
bytes32[] memory withdrawalRoots
216217
) internal {
@@ -226,11 +227,11 @@ contract IntegrationCheckUtils is IntegrationBase {
226227
"check_QueuedWithdrawal_State: calculated withdrawals should match returned roots");
227228
assert_Snap_Added_QueuedWithdrawals(staker, withdrawals,
228229
"check_QueuedWithdrawal_State: staker should have increased nonce by withdrawals.length");
229-
assert_Snap_Removed_OperatorShares(operator, strategies, shares,
230+
assert_Snap_Removed_OperatorShares(operator, strategies, withdrawableShares,
230231
"check_QueuedWithdrawal_State: failed to remove operator shares");
231-
assert_Snap_Removed_Staker_DepositShares(staker, strategies, shares,
232+
assert_Snap_Removed_Staker_DepositShares(staker, strategies, depositShares,
232233
"check_QueuedWithdrawal_State: failed to remove staker shares");
233-
assert_Snap_Removed_Staker_WithdrawableShares(staker, strategies, shares,
234+
assert_Snap_Removed_Staker_WithdrawableShares(staker, strategies, withdrawableShares,
234235
"check_QueuedWithdrawal_State: failed to remove staker withdrawable shares");
235236
}
236237

@@ -248,6 +249,7 @@ contract IntegrationCheckUtils is IntegrationBase {
248249
// ... check that the staker is undelegated, all strategies from which the staker is deposited are unqueued,
249250
// that the returned root matches the hashes for each strategy and share amounts, and that the staker
250251
// and operator have reduced shares
252+
assert_HasNoDelegatableShares(staker, "staker should have withdrawn all shares");
251253
assertFalse(delegationManager.isDelegated(address(staker)),
252254
"check_Undelegate_State: staker should not be delegated");
253255
assert_ValidWithdrawalHashes(withdrawals, withdrawalRoots,
@@ -268,7 +270,8 @@ contract IntegrationCheckUtils is IntegrationBase {
268270

269271
function check_Redelegate_State(
270272
User staker,
271-
User operator,
273+
User prevOperator,
274+
User newOperator,
272275
IDelegationManagerTypes.Withdrawal[] memory withdrawals,
273276
bytes32[] memory withdrawalRoots,
274277
IStrategy[] memory strategies,
@@ -282,13 +285,16 @@ contract IntegrationCheckUtils is IntegrationBase {
282285
// and operator have reduced shares
283286
assertTrue(delegationManager.isDelegated(address(staker)),
284287
"check_Redelegate_State: staker should not be delegated");
288+
assertEq(address(newOperator), delegationManager.delegatedTo(address(staker)), "staker should be delegated to operator");
289+
assert_HasExpectedShares(staker, strategies, new uint[](strategies.length), "staker should not have deposit shares after redelegation");
290+
assert_Snap_Unchanged_OperatorShares(newOperator, "new operator should not have received any shares");
285291
assert_ValidWithdrawalHashes(withdrawals, withdrawalRoots,
286292
"check_Redelegate_State: calculated withdrawal should match returned root");
287293
assert_AllWithdrawalsPending(withdrawalRoots,
288294
"check_Redelegate_State: stakers withdrawal should now be pending");
289295
assert_Snap_Added_QueuedWithdrawals(staker, withdrawals,
290296
"check_Redelegate_State: staker should have increased nonce by withdrawals.length");
291-
assert_Snap_Removed_OperatorShares(operator, strategies, stakerDelegatedShares,
297+
assert_Snap_Removed_OperatorShares(prevOperator, strategies, stakerDelegatedShares,
292298
"check_Redelegate_State: failed to remove operator shares");
293299
assert_Snap_Removed_Staker_DepositShares(staker, strategies, stakerDepositShares,
294300
"check_Redelegate_State: failed to remove staker shares");
@@ -349,7 +355,7 @@ contract IntegrationCheckUtils is IntegrationBase {
349355
if (operator != staker) {
350356
assert_Snap_Unchanged_TokenBalances(operator, "operator should not have any change in underlying token balances");
351357
}
352-
assert_Snap_Added_OperatorShares(operator, withdrawal.strategies, withdrawal.scaledShares, "operator should have received shares");
358+
assert_Snap_Added_OperatorShares(operator, strategies, shares, "operator should have received shares");
353359
}
354360
}
355361

@@ -438,7 +444,7 @@ contract IntegrationCheckUtils is IntegrationBase {
438444
/// @dev Check global max magnitude invariants - these should ALWAYS hold
439445
function check_MaxMag_Invariants(
440446
User operator
441-
) internal {
447+
) internal view {
442448
assert_MaxMagsEqualMaxMagsAtCurrentBlock(operator, allStrats, "max magnitudes should equal upperlookup at current block");
443449
assert_MaxEqualsAllocatablePlusEncumbered(operator, "max magnitude should equal encumbered plus allocatable");
444450
}
@@ -447,7 +453,7 @@ contract IntegrationCheckUtils is IntegrationBase {
447453
function check_ActiveModification_State(
448454
User operator,
449455
AllocateParams memory params
450-
) internal {
456+
) internal view {
451457
OperatorSet memory operatorSet = params.operatorSet;
452458
IStrategy[] memory strategies = params.strategies;
453459

@@ -459,15 +465,15 @@ contract IntegrationCheckUtils is IntegrationBase {
459465
User operator,
460466
OperatorSet memory operatorSet,
461467
IStrategy[] memory strategies
462-
) internal {
468+
) internal view {
463469
assert_IsSlashable(operator, operatorSet, "operator should be slashable for operator set");
464470
assert_CurMinSlashableEqualsMinAllocated(operator, operatorSet, strategies, "minimum slashable stake should equal allocated stake at current block");
465471
}
466472

467473
function check_NotSlashable_State(
468474
User operator,
469475
OperatorSet memory operatorSet
470-
) internal {
476+
) internal view {
471477
assert_NotSlashable(operator, operatorSet, "operator should not be slashable for operator set");
472478
assert_NoSlashableStake(operator, operatorSet, "operator should not have any slashable stake");
473479
}
@@ -893,8 +899,8 @@ contract IntegrationCheckUtils is IntegrationBase {
893899
check_IsSlashable_State(operator, operatorSet, allocateParams.strategies);
894900

895901
// Slashing SHOULD change max magnitude and current allocation
896-
assert_Snap_Slashed_MaxMagnitude(operator, slashParams, "slash should lower max magnitude");
897-
assert_Snap_Slashed_EncumberedMagnitude(operator, slashParams, "slash should lower max magnitude");
902+
assert_Snap_Slashed_MaxMagnitude(operator, operatorSet, slashParams, "slash should lower max magnitude");
903+
assert_Snap_Slashed_EncumberedMagnitude(operator, slashParams, "slash should lower encumbered magnitude");
898904
assert_Snap_Slashed_AllocatedStake(operator, operatorSet, slashParams, "slash should lower allocated stake");
899905
assert_Snap_Slashed_SlashableStake(operator, operatorSet, slashParams, "slash should lower slashable stake");
900906
assert_Snap_Slashed_OperatorShares(operator, slashParams, "slash should remove operator shares");
@@ -905,97 +911,19 @@ contract IntegrationCheckUtils is IntegrationBase {
905911
assert_Snap_Unchanged_AllocatableMagnitude(operator, allStrats, "slashing should not change allocatable magnitude");
906912
assert_Snap_Unchanged_Registration(operator, operatorSet, "slash should not change registration status");
907913
assert_Snap_Unchanged_Slashability(operator, operatorSet, "slash should not change slashability status");
908-
assert_Snap_Unchanged_AllocatedSets(operator, "should not have updated allocated sets");
909-
assert_Snap_Unchanged_AllocatedStrats(operator, operatorSet, "should not have updated allocated strategies");
914+
// assert_Snap_Unchanged_AllocatedSets(operator, "should not have updated allocated sets");
915+
// assert_Snap_Unchanged_AllocatedStrats(operator, operatorSet, "should not have updated allocated strategies");
910916
}
911917

912-
// TODO: improvement needed
913-
914-
function check_Withdrawal_AsTokens_State_AfterSlash(
915-
User staker,
918+
/// Slashing invariants when the operator has been fully slashed for every strategy in the operator set
919+
function check_FullySlashed_State(
916920
User operator,
917-
Withdrawal memory withdrawal,
918921
AllocateParams memory allocateParams,
919-
SlashingParams memory slashingParams,
920-
uint[] memory expectedTokens
921-
) internal {
922-
IERC20[] memory tokens = new IERC20[](withdrawal.strategies.length);
923-
924-
for (uint i; i < withdrawal.strategies.length; i++) {
925-
IStrategy strat = withdrawal.strategies[i];
926-
927-
bool isBeaconChainETHStrategy = strat == beaconChainETHStrategy;
928-
929-
tokens[i] = isBeaconChainETHStrategy ? NATIVE_ETH : withdrawal.strategies[i].underlyingToken();
930-
931-
if (slashingParams.strategies.contains(strat)) {
932-
uint wadToSlash = slashingParams.wadsToSlash[slashingParams.strategies.indexOf(strat)];
933-
934-
expectedTokens[i] -= expectedTokens[i]
935-
.mulWadRoundUp(allocateParams.newMagnitudes[i].mulWadRoundUp(wadToSlash));
936-
937-
uint256 max = allocationManager.getMaxMagnitude(address(operator), strat);
938-
939-
withdrawal.scaledShares[i] -= withdrawal.scaledShares[i].calcSlashedAmount(WAD, max);
940-
941-
// Round down to the nearest gwei for beaconchain ETH strategy.
942-
if (isBeaconChainETHStrategy) {
943-
expectedTokens[i] -= expectedTokens[i] % 1 gwei;
944-
}
945-
}
946-
}
947-
948-
// Common checks
949-
assert_WithdrawalNotPending(delegationManager.calculateWithdrawalRoot(withdrawal), "staker withdrawal should no longer be pending");
950-
951-
// TODO FIXME
952-
// assert_Snap_Added_TokenBalances(staker, tokens, expectedTokens, "staker should have received expected tokens");
953-
assert_Snap_Unchanged_Staker_DepositShares(staker, "staker shares should not have changed");
954-
assert_Snap_Removed_StrategyShares(withdrawal.strategies, withdrawal.scaledShares, "strategies should have total shares decremented");
955-
956-
// Checks specific to an operator that the Staker has delegated to
957-
if (operator != User(payable(0))) {
958-
if (operator != staker) {
959-
assert_Snap_Unchanged_TokenBalances(operator, "operator token balances should not have changed");
960-
}
961-
assert_Snap_Unchanged_OperatorShares(operator, "operator shares should not have changed");
962-
}
963-
}
964-
965-
function check_Withdrawal_AsShares_State_AfterSlash(
966-
User staker,
967-
User operator,
968-
Withdrawal memory withdrawal,
969-
AllocateParams memory allocateParams, // TODO - was this needed?
970-
SlashingParams memory slashingParams
922+
SlashingParams memory slashParams
971923
) internal {
972-
IERC20[] memory tokens = new IERC20[](withdrawal.strategies.length);
973-
974-
for (uint i; i < withdrawal.strategies.length; i++) {
975-
IStrategy strat = withdrawal.strategies[i];
976-
977-
bool isBeaconChainETHStrategy = strat == beaconChainETHStrategy;
924+
check_Base_Slashing_State(operator, allocateParams, slashParams);
978925

979-
tokens[i] = isBeaconChainETHStrategy ? NATIVE_ETH : withdrawal.strategies[i].underlyingToken();
980-
981-
if (slashingParams.strategies.contains(strat)) {
982-
uint256 max = allocationManager.getMaxMagnitude(address(operator), strat);
983-
984-
withdrawal.scaledShares[i] -= withdrawal.scaledShares[i].calcSlashedAmount(WAD, max);
985-
}
986-
}
987-
988-
// Common checks applicable to both user and non-user operator types
989-
assert_WithdrawalNotPending(delegationManager.calculateWithdrawalRoot(withdrawal), "staker withdrawal should no longer be pending");
990-
assert_Snap_Unchanged_TokenBalances(staker, "staker should not have any change in underlying token balances");
991-
assert_Snap_Added_Staker_DepositShares(staker, withdrawal.strategies, withdrawal.scaledShares, "staker should have received expected shares");
992-
assert_Snap_Unchanged_StrategyShares(withdrawal.strategies, "strategies should have total shares unchanged");
993-
994-
// Additional checks or handling for the non-user operator scenario
995-
if (operator != User(User(payable(0)))) {
996-
if (operator != staker) {
997-
assert_Snap_Unchanged_TokenBalances(operator, "operator should not have any change in underlying token balances");
998-
}
999-
}
926+
assert_Snap_Removed_AllocatedSet(operator, allocateParams.operatorSet, "should not have updated allocated sets");
927+
assert_Snap_Removed_AllocatedStrats(operator, allocateParams.operatorSet, slashParams.strategies, "should not have updated allocated strategies");
1000928
}
1001929
}

0 commit comments

Comments
 (0)