diff --git a/contracts/test/harness/SSVClustersHarness.sol b/contracts/test/harness/SSVClustersHarness.sol index d54a2d63..e078fdd6 100644 --- a/contracts/test/harness/SSVClustersHarness.sol +++ b/contracts/test/harness/SSVClustersHarness.sol @@ -339,6 +339,19 @@ contract SSVClustersHarness is SSVClusters, SSVValidators { operator.ethSnapshot.balance = PACKED_ETH_ZERO; } + function mockSetOperatorFeeChangeRequest( + uint64 operatorId, + uint64 fee, + uint64 approvalBeginTime, + uint64 approvalEndTime + ) external { + SSVStorage.load().operatorFeeChangeRequests[operatorId] = ISSVNetworkCore.OperatorFeeChangeRequest( + fee, + approvalBeginTime, + approvalEndTime + ); + } + function mockSetToken(address token) external { SSVStorage.load().token = IERC20(token); } diff --git a/ssv-review/planning/MAINNET-READINESS.md b/ssv-review/planning/MAINNET-READINESS.md index 51873054..c56b8e31 100644 --- a/ssv-review/planning/MAINNET-READINESS.md +++ b/ssv-review/planning/MAINNET-READINESS.md @@ -66,7 +66,7 @@ | TEST-12 | ~~Multi-staker reward fairness~~ | Unit Test Completeness | P1 | ✅ Done | | TEST-13 | ~~Liquidation + reactivation multi-cycle accounting~~ | Unit Test Completeness | P1 | ✅ Done | | TEST-14 | ~~Reactivation with EB deviation solvency check~~ | Unit Test Completeness | P1 | ✅ Done | -| TEST-15 | SSV cluster operations completeness | Unit Test Completeness | P1 | M | +| TEST-15 | ~~SSV cluster operations completeness~~ | Unit Test Completeness | P1 | ✅ Closed (legacy SSV fee settlement covered; direct SSV withdraw is spec-blocked) | | TEST-16 | ~~View function coverage (SSVViews)~~ | Unit Test Completeness | P1 | ✅ Fixed | | TEST-17 | ~~Staking rewards from EB-weighted cluster fees~~ | Unit Test Completeness | P1 | ✅ Closed (Covered in `test/integration/SSVNetwork/staking.test.ts`) | | TEST-18 | `withdrawNetworkETHEarnings` (DAO ETH withdrawal) | Unit Test Completeness | P1 | S | @@ -1923,12 +1923,12 @@ Reactivate tests don't verify that the minimum deposit scales with vUnits. A clu --- -### [TEST-15] SSV cluster operations completeness +### [TEST-15] ~~SSV cluster operations completeness~~ - **Type:** Unit Test Completeness - **Priority:** P1 -- **Status:** Open -- **Owner:** (unassigned) -- **Timeline:** (empty) +- **Status:** ✅ Closed +- **Owner:** (resolved) +- **Timeline:** 2026-03-12 - **Github Link:** (empty) **Requirement:** @@ -1937,10 +1937,21 @@ Add comprehensive tests for SSV-denominated cluster operations. Most tests focus **Context:** The dual cluster system maintains parallel SSV and ETH records. SSV cluster operations should still work correctly during the transition period. +**Resolution:** +Closed with focused legacy SSV accounting coverage across allowed SSV-cluster paths: +- `test/unit/SSVValidator/removeValidator.test.ts` already covers removal from active legacy SSV clusters, including a non-zero-fee balance-deduction check. +- `test/unit/SSVClusters/legacySSVAccounting.test.ts` adds exact settlement checks for: + - `removeValidator` with accrued legacy SSV operator fees + - `removeValidator` with a pending ETH fee change request — proves SSV settlement is isolated from ETH fee state + - `bulkRemoveValidator` with non-zero legacy SSV network fee +- Full verification run: `just test-unit` → `662 passing`. + +The previous "SSV cluster withdrawal" acceptance item was stale relative to the current code/spec. Direct `withdraw()` on an SSV cluster is intentionally blocked and is already covered by `test/unit/SSVClusters/withdraw.test.ts` expecting `IncorrectClusterVersion`. + **Acceptance Criteria:** -- [ ] Test: Register/remove validators in SSV cluster with non-zero SSV fees → verify fee deductions -- [ ] Test: SSV cluster with non-zero network fee → verify fee deductions -- [ ] Test: Withdraw from SSV cluster → verify balance and token transfer +- [x] Test: Register/remove validators in SSV cluster with non-zero SSV fees → verify fee deductions +- [x] Test: SSV cluster with non-zero network fee → verify fee deductions +- [x] Direct SSV cluster `withdraw()` is confirmed spec-blocked and covered as `IncorrectClusterVersion`; no positive-path withdraw test is required **Agent Instructions:** 1. Read existing SSV-related tests: `test/unit/SSVClusters/liquidateSSV.test.ts`, `test/integration/SSVNetwork/legacy-ssv.test.ts`. @@ -1950,9 +1961,9 @@ The dual cluster system maintains parallel SSV and ETH records. SSV cluster oper 5. Run `npm run test:unit`. #### Sub-items: -- [ ] Sub-task 1: SSV validator registration with fees -- [ ] Sub-task 2: SSV cluster network fee deductions -- [ ] Sub-task 3: SSV cluster withdrawal +- [x] Sub-task 1: Legacy SSV validator removal path with fees +- [x] Sub-task 2: SSV cluster network fee deductions +- [x] Sub-task 3: Confirm direct SSV cluster withdrawal is intentionally blocked by spec/code --- diff --git a/test/unit/SSVClusters/legacySSVAccounting.test.ts b/test/unit/SSVClusters/legacySSVAccounting.test.ts new file mode 100644 index 00000000..2ab9c599 --- /dev/null +++ b/test/unit/SSVClusters/legacySSVAccounting.test.ts @@ -0,0 +1,206 @@ +import { expect } from "chai"; +import type { NetworkConnection } from "hardhat/types/network"; +import type { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/types"; +import { ethers } from "ethers"; +import { getTestConnection } from "../../setup/connection.ts"; +import { ssvClustersHarnessFixture } from "../../setup/fixtures.ts"; +import type { Cluster, NetworkHelpersType } from "../../common/types.ts"; +import { createCluster, makePublicKeys, parseClusterFromEvent } from "../../common/helpers.ts"; +import { DEDUCTED_DIGITS } from "../../common/constants.ts"; +import { Events } from "../../common/events.ts"; + +type Snapshot = { + block: bigint; + index: bigint; +}; + +describe("SSVClusters legacy SSV accounting", async () => { + let connection: NetworkConnection<"generic">; + let networkHelpers: NetworkHelpersType; + let clusterOwner: HardhatEthersSigner; + + before(async function () { + ({ connection, networkHelpers } = await getTestConnection()); + [clusterOwner] = await connection.ethers.getSigners(); + }); + + const deployLegacySSVFixture = async (operatorFeeRaw: bigint, networkFeeRaw: bigint) => { + const { clusters, operatorIds } = await ssvClustersHarnessFixture(connection); + const operatorFeeUnpacked = operatorFeeRaw * DEDUCTED_DIGITS; + + for (const operatorId of operatorIds) { + await clusters.mockOperatorSSVFee(operatorId, operatorFeeUnpacked); + } + + await clusters.mockSSVNetworkFee(networkFeeRaw); + const networkFeeIndexTx = await clusters.mockCurrentNetworkFeeIndexSSV(0n); + const networkFeeIndexReceipt = await networkFeeIndexTx.wait(); + + return { + clusters, + operatorIds, + networkFeeIndexBlock: BigInt(networkFeeIndexReceipt!.blockNumber), + }; + }; + + const deployOperatorFeeFixture = async () => deployLegacySSVFixture(2_000n, 0n); + const deployNetworkFeeFixture = async () => deployLegacySSVFixture(0n, 75n); + + const createLegacySSVCluster = (overrides: Partial = {}): Cluster => + createCluster({ + validatorCount: 2n, + index: 0n, + networkFeeIndex: 0n, + balance: ethers.parseEther("100"), + ...overrides, + }); + + const captureSnapshots = async (clusters: any, operatorIds: bigint[]): Promise => + Promise.all( + operatorIds.map(async (operatorId) => { + const [index, blockNumber] = await clusters.getOperatorSnapshot(operatorId); + return { + block: BigInt(blockNumber), + index: BigInt(index), + }; + }) + ); + + const calculateClusterIndex = (snapshots: Snapshot[], currentBlock: bigint, operatorFeeRaw: bigint): bigint => + snapshots.reduce( + (sum, snapshot) => sum + snapshot.index + (currentBlock - snapshot.block) * operatorFeeRaw, + 0n + ); + + const calculateNetworkFeeIndex = ( + currentBlock: bigint, + feeIndexBlock: bigint, + networkFeeRaw: bigint + ): bigint => (currentBlock - feeIndexBlock) * networkFeeRaw; + + const calculateSettledFees = ( + cluster: Cluster, + currentClusterIndex: bigint, + currentNetworkFeeIndex: bigint + ): bigint => + ( + (currentClusterIndex - cluster.index) * BigInt(cluster.validatorCount) + + (currentNetworkFeeIndex - cluster.networkFeeIndex) * BigInt(cluster.validatorCount) + ) * DEDUCTED_DIGITS; + + it("removeValidator settles accrued legacy SSV operator fees before decrementing validator count", async function () { + const operatorFeeRaw = 2_000n; + const { clusters, operatorIds, networkFeeIndexBlock } = + await networkHelpers.loadFixture(deployOperatorFeeFixture); + + const [publicKey1, publicKey2] = makePublicKeys(2); + const cluster = createLegacySSVCluster({ validatorCount: 2n }); + + await clusters.mockRegisterSSVValidator(publicKey1, operatorIds, clusterOwner.address, cluster); + await clusters.mockRegisterSSVValidator(publicKey2, operatorIds, clusterOwner.address, cluster); + + const snapshots = await captureSnapshots(clusters, operatorIds); + + await networkHelpers.mine(25); + + const removeTx = await clusters.connect(clusterOwner).removeValidator(publicKey1, operatorIds, cluster); + const removeReceipt = await removeTx.wait(); + const clusterAfterRemove = parseClusterFromEvent(clusters, removeReceipt, Events.VALIDATOR_REMOVED); + const removeBlock = BigInt(removeReceipt!.blockNumber); + + const expectedClusterIndex = calculateClusterIndex(snapshots, removeBlock, operatorFeeRaw); + const expectedNetworkFeeIndex = calculateNetworkFeeIndex(removeBlock, networkFeeIndexBlock, 0n); + const expectedFees = calculateSettledFees(cluster, expectedClusterIndex, expectedNetworkFeeIndex); + + expect(clusterAfterRemove.validatorCount).to.equal(1n); + expect(clusterAfterRemove.index).to.equal(expectedClusterIndex); + expect(clusterAfterRemove.networkFeeIndex).to.equal(expectedNetworkFeeIndex); + expect(clusterAfterRemove.balance).to.equal(cluster.balance - expectedFees); + expect(expectedFees).to.equal(expectedClusterIndex * BigInt(cluster.validatorCount) * DEDUCTED_DIGITS); + expect(expectedFees % DEDUCTED_DIGITS).to.equal(0n); + }); + + it("removeValidator settles legacy SSV fees identically when a pending ETH fee change request exists", async function () { + const operatorFeeRaw = 2_000n; + const { clusters, operatorIds, networkFeeIndexBlock } = + await networkHelpers.loadFixture(deployOperatorFeeFixture); + + const [publicKey1, publicKey2] = makePublicKeys(2, 21); + const cluster = createLegacySSVCluster({ validatorCount: 2n }); + + await clusters.mockRegisterSSVValidator(publicKey1, operatorIds, clusterOwner.address, cluster); + await clusters.mockRegisterSSVValidator(publicKey2, operatorIds, clusterOwner.address, cluster); + + // Inject a pending ETH fee change request on each operator (declared, within approval window) + const now = BigInt(await networkHelpers.time.latest()); + for (const operatorId of operatorIds) { + await clusters.mockSetOperatorFeeChangeRequest( + operatorId, + 99_999n, // large pending ETH fee — must NOT affect SSV settlement + now + 1n, // approvalBeginTime (in the future, so pending) + now + 86400n, // approvalEndTime + ); + } + + const snapshots = await captureSnapshots(clusters, operatorIds); + + await networkHelpers.mine(30); + + const removeTx = await clusters.connect(clusterOwner).removeValidator(publicKey1, operatorIds, cluster); + const removeReceipt = await removeTx.wait(); + const clusterAfterRemove = parseClusterFromEvent(clusters, removeReceipt, Events.VALIDATOR_REMOVED); + const removeBlock = BigInt(removeReceipt!.blockNumber); + + // Expected values use only the SSV fee — identical formula to the operator-fee-only test + const expectedClusterIndex = calculateClusterIndex(snapshots, removeBlock, operatorFeeRaw); + const expectedNetworkFeeIndex = calculateNetworkFeeIndex(removeBlock, networkFeeIndexBlock, 0n); + const expectedFees = calculateSettledFees(cluster, expectedClusterIndex, expectedNetworkFeeIndex); + + expect(clusterAfterRemove.validatorCount).to.equal(1n); + expect(clusterAfterRemove.index).to.equal(expectedClusterIndex); + expect(clusterAfterRemove.networkFeeIndex).to.equal(expectedNetworkFeeIndex); + expect(clusterAfterRemove.balance).to.equal(cluster.balance - expectedFees); + // The pending ETH fee (99_999) had zero effect — fees match the SSV-only formula exactly + expect(expectedFees).to.equal(expectedClusterIndex * BigInt(cluster.validatorCount) * DEDUCTED_DIGITS); + }); + + it("bulkRemoveValidator settles legacy SSV network fees on active clusters", async function () { + const networkFeeRaw = 75n; + const { clusters, operatorIds, networkFeeIndexBlock } = + await networkHelpers.loadFixture(deployNetworkFeeFixture); + + const publicKeys = makePublicKeys(3, 11); + const cluster = createLegacySSVCluster({ + validatorCount: 3n, + balance: ethers.parseEther("60"), + }); + + for (const publicKey of publicKeys) { + await clusters.mockRegisterSSVValidator(publicKey, operatorIds, clusterOwner.address, cluster); + } + + const snapshots = await captureSnapshots(clusters, operatorIds); + + await networkHelpers.mine(40); + + const removeTx = await clusters.connect(clusterOwner).bulkRemoveValidator( + [publicKeys[0], publicKeys[1]], + operatorIds, + cluster + ); + const removeReceipt = await removeTx.wait(); + const clusterAfterRemove = parseClusterFromEvent(clusters, removeReceipt, Events.VALIDATOR_REMOVED); + const removeBlock = BigInt(removeReceipt!.blockNumber); + + const expectedClusterIndex = calculateClusterIndex(snapshots, removeBlock, 0n); + const expectedNetworkFeeIndex = calculateNetworkFeeIndex(removeBlock, networkFeeIndexBlock, networkFeeRaw); + const expectedFees = calculateSettledFees(cluster, expectedClusterIndex, expectedNetworkFeeIndex); + + expect(expectedClusterIndex).to.equal(0n); + expect(clusterAfterRemove.validatorCount).to.equal(1n); + expect(clusterAfterRemove.index).to.equal(0n); + expect(clusterAfterRemove.networkFeeIndex).to.equal(expectedNetworkFeeIndex); + expect(clusterAfterRemove.balance).to.equal(cluster.balance - expectedFees); + expect(expectedFees).to.equal(expectedNetworkFeeIndex * BigInt(cluster.validatorCount) * DEDUCTED_DIGITS); + }); +});