Skip to content

test: improve slashing invariants and refactor user generation logic #1149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 21, 2025

Conversation

wadealexc
Copy link
Member

@wadealexc wadealexc commented Feb 20, 2025

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 👍

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

@ypatil12 ypatil12 added 🗡️ Slashing Release Changes for the slashing release. 🧪 Test Test-related changes (unit, integration, etc.). labels Feb 20, 2025
@0xClandestine 0xClandestine self-requested a review February 21, 2025 14:22

print.user(name, assetType, userType, strategies, tokenBalances);
return (user, strategies, tokenBalances);
}

function _randUser(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this and _newStaker(strategies) can generate assets for specified strategies vs just random

@@ -349,7 +355,7 @@ contract IntegrationCheckUtils is IntegrationBase {
if (operator != staker) {
assert_Snap_Unchanged_TokenBalances(operator, "operator should not have any change in underlying token balances");
}
assert_Snap_Added_OperatorShares(operator, withdrawal.strategies, withdrawal.scaledShares, "operator should have received shares");
assert_Snap_Added_OperatorShares(operator, strategies, shares, "operator should have received shares");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe in followup PR, but the check_Withdrawal_AsShares_* functions could probably include this call
uint[] memory expectedShares = _calculateExpectedShares(withdrawals[i]);
and not have to pass in shares as input. Since we are already passing in the withdrawal

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, good callout. there's a decent bit of cleanup to be done in these assertions

* also fix a bunch of compiler errors
@wadealexc wadealexc merged commit 02b13f0 into test/slashing-integration-testing Feb 21, 2025
10 checks passed
@wadealexc wadealexc deleted the alex/alm-dm-invariants branch February 21, 2025 15:33
ypatil12 pushed a commit that referenced this pull request Feb 27, 2025
…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
ypatil12 pushed a commit that referenced this pull request Feb 27, 2025
…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
ypatil12 pushed a commit that referenced this pull request Feb 27, 2025
…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
ypatil12 pushed a commit that referenced this pull request Mar 5, 2025
…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
ypatil12 added a commit that referenced this pull request Mar 5, 2025
**Motivation:**

Slashing Integration Testing for Competition Audit

**Modifications:**

***General State Validation***
- #1204
- #1198
- #1169
- #1158

***Upgrade Tests***
- #1187
- #1171
- #1143

***Dual Slash Tests***
- #1195
- #1153

***Rounding Tests***
- #1178

***EigenPod Tests***
- #1191
- #1188
- #1203
- #1194
- #1163

***Invariants***
- #1201
- #1176
- #1192
- #1197
- #1175
- #1189
- #1150
- #1149

**Result:**

Comprehensive Test Coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗡️ Slashing Release Changes for the slashing release. 🧪 Test Test-related changes (unit, integration, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants