feat: symbiotic setup and slash tests#1507
feat: symbiotic setup and slash tests#1507rackstar wants to merge 7 commits intorelease-candidatefrom
Conversation
7702ca4 to
12d256a
Compare
12d256a to
2186a02
Compare
ea7848a to
d937277
Compare
2186a02 to
8c00c3a
Compare
b582e1f to
8046746
Compare
8046746 to
ee83c50
Compare
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds three comprehensive test files for Symbiotic protocol integration. Includes setup orchestration for multi-vault deployments, registrations, staking, and subnetwork allocations; a Safe multisend workflow test; and extensive slashing scenario tests with state validation and token flow verification. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/fork/symbiotic/symbiotic-setup-safe-tx.js`:
- Around line 75-77: The test currently asserts safeReceipt.status equals BigInt
by comparing to 1n; update the assertion to compare the numeric
TransactionReceipt.status to 1 (not 1n). Locate the code using
executeSafeTransaction, safeTx, and safeReceipt and change the expectation to
expect(safeReceipt.status).to.equal(1) so it matches ethers v6's number-typed
receipt.status.
In `@test/fork/symbiotic/symbiotic-setup.js`:
- Around line 579-582: The opt-in assertions use undefined properties
this.vault1Addr..vault4Addr; update each call to use the deployed vault target
stored on the vault objects (e.g. replace this.vault1Addr with
this.vault1.vault.target) so that
operatorVaultOptIn.isOptedIn(this.operator1.address, <vault>.vault.target) is
asserted for vault1..vault4.
In `@test/fork/symbiotic/symbiotic-tests.js`:
- Around line 1242-1263: The test relies on state mutated by a previous test
(operatorNetworkReceiver) so make the receiver override part of this test’s
setup: after setting middleware
(this.middlewareService.connect(...).setMiddleware(...)) and before calling
this.burnerRouter.triggerTransfer(this.newMiddleware.address), explicitly set
the operator/ network receiver to this.newMiddleware (the same override that
previous tests set, i.e. ensure
SAFE_MULTISIG_SLASH_RECEIVER/operatorNetworkReceiver is updated for this.network
and this.operator1) so the triggered transfer actually routes slashed funds to
this.newMiddleware; update the test to perform that receiver override as part of
its setup and then proceed with slash and triggerTransfer.
- Around line 1212-1239: The delegated-stake assertion assumes a 1:1 reduction
but delegated stake is bounded by min(activeStake, subnetworkLimit); replace the
calculation for expectedAfterDelegatedStake so it uses
Math.min(beforeVaultStake, subnetworkLimit) minus amountToSlash (obtain
subnetworkLimit from the test fixture object for subnetwork1), then compare that
to afterDelegatedStake; apply the same adjustment to the analogous assertion
block further down.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 66442732-438d-4932-9f51-05dbfa112da8
📒 Files selected for processing (3)
test/fork/symbiotic/symbiotic-setup-safe-tx.jstest/fork/symbiotic/symbiotic-setup.jstest/fork/symbiotic/symbiotic-tests.js
Description
Testing
Explain how you tested your changes to ensure they work as expected.
Checklist
Summary by CodeRabbit
Release Notes