Skip to content

fix(cluster): exclude removed operators from fee sums [SSV-WEB-001] - #1881

Merged
Chris-ssvlabs merged 3 commits into
stagefrom
fix/ssv-web-001-pr1
Oct 30, 2026
Merged

Chris-ssvlabs merged 3 commits into
stagefrom
fix/ssv-web-001-pr1

Conversation

@Chris-ssvlabs

@Chris-ssvlabs Chris-ssvlabs commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Why

When a cluster contains a removed operator, the API still returns that operator's last fee, and the webapp added it to the cluster's burn rate. On-chain, a removed operator's fee is 0. As a result the webapp showed a shorter runway than the real one (fixture: 75 days instead of 100), asked for funding and reactivation amounts that were too high, and disagreed with the explorer about the same cluster.

The ignore-removed flag fixed this only on the ETH path, only where a caller opted in (the SSV→ETH switch wizard), and never on the SSV path. Product decision (SSV-WEB-001, 2026-10-29): remove the flag. A removed operator now counts as 0 in every fee sum, on both the ETH and SSV paths. A removed operator's last fee may still appear as a per-row display label, but never in a total.

What changed

  • src/lib/utils/operator.ts
    • getOperatorEthFee and the new getOperatorSsvFee return 0n for a removed operator (is_deleted). A missing fee on an operator that isn't removed counts as 0, and logs a console.warn once per operator and field. A 0 here understates the burn rate, so the warning makes the case visible.
    • sumOperatorsFee(operators, by) no longer takes a third parameter.
  • src/hooks/use-compute-funding-cost.ts: the ignoreRemovedOperators option, its default and its queryKey entry are removed. The per-operator is_deleted stays in the queryKey.
  • src/components/wizard/switch-wizard-step-two.tsx: the four ignoreRemovedOperators: true arguments are removed.
  • src/hooks/use-migration-calculation-data.ts: getOperatorEthFee(operator). No visible change.
  • src/components/wizard/operator-fee-comparison.tsx: both yearly totals (SSV and ETH) now go through sumOperatorsFee. The per-row display is unchanged.
  • src/app/routes/create-cluster/select-operators/index.tsx: the "Operators Yearly Fee" total goes through sumOperatorsFee. The reshare flow preselects every current cluster operator, including removed ones.
  • src/lib/utils/__tests__/operator-fee.test.ts (new): 12 tests. They cover helper-level and runway-level checks for both ETH and SSV, and the missing-fee warning: it warns once, never for a removed operator, and never for a valid fee.

src/hooks/cluster/use-cluster-runway.ts needed no change: fixing the helper corrects it.

Visible effect: for a cluster that contains a removed operator, runway goes up and funding and reactivation amounts go down. Clusters without one show the same numbers as before.

Gates (run on 95440c0)

  • pnpm run test:run: Test Files 11 passed (11) / Tests 131 passed (131)
  • pnpm type-check: exit 0
  • pnpm lint: ✖ 1 problem (0 errors, 1 warning). The warning already exists on stage, in a file this PR doesn't touch (migration-effective-balance-form.tsx:61, react-hooks/exhaustive-deps).
  • The pre-commit hook (tests, type-check, lint-staged) passed on the commit.
  • git diff --check is clean.
  • commitlint passed for all three commits when run by hand (npx commitlint --from f9d7e8e3 --to HEAD).

Tests need VITE_SSV_NETWORKS in the environment. src/config/networks.ts throws at import time without it.

Repro

On stage @ f9d7e8e, with a fixture of 4 operators where operator 4 is removed:

expected on stage
ETH sum 3,000,000,000 4,000,000,000
SSV sum 6,000,000 8,000,000
SSV sum with the flag on 6,000,000 8,000,000
runway 100 days 75 days

Vitest on stage: Tests 4 failed | 1 passed (5). On this branch, all 12 tests in the file pass.

Mutation checks

  • M1: removed the removal check in getOperatorSsvFee (operator.ts:177) → getOperatorSsvFee returns 0n for a removed operator, sumOperatorsFee ssv excludes removed operators and the SSV runway test fail.
  • M2: removed the removal check in getOperatorEthFee (operator.ts:174) → getOperatorEthFee returns 0n for a removed operator, sumOperatorsFee eth excludes removed operators, sumOperatorsFee defaults to eth and the ETH runway test fail.
  • M3 (run at 2c29679): removed the || "0" guard → sumOperatorsFee treats empty or missing fee values as 0n fails. On the head that guard is replaced by the readFee fallback (operator.ts:155-157); M3 was not re-run against it.
  • M4: put the SSV sum back to bare BigInt(operator.fee) → the SSV tests fail.
  • M5: removed the warning call, or the once-per-operator check → the warning tests fail.

Not verified

  • That the API returns is_deleted on each operator in GET clusters/{hash}?operatorDetails=true, and returns the stale last fee for removed operators. Confirmed against the live Hoodi API (operator 182: is_deleted: true, fee 382640000000, eth_fee 1770000000). The getter contract returns a fee of 0 for the removed operators checked.
  • The reshare "Operators Yearly Fee" total (select-operators/index.tsx:112) has no test of its own. It is a component, and its logic is the tested helper. It was checked in the running app (S8 below).
  • In-app check: run, see Smoke below.

Known, not fixed here

  • .husky/commit-msg is committed without the executable bit, so git skips it and commitlint never runs at commit time. Tracked in SSV-CI-001.
  • No repo mechanism provides VITE_SSV_NETWORKS for tests (no setupFiles, no .env.test), and the CI test step is commented out (build_deploy.yml:89-90). Tracked in SSV-CI-001.
  • CLAUDE.md says tests run in Node, but vitest.config.ts uses jsdom.
  • No "Removed" tag on a removed operator's row in the fee comparison or the reshare operator list (SSV-WEB-006). Rows can show a last fee that the total leaves out, which the product decision allows.
  • In the reshare flow, a removed operator's record sometimes arrives as a placeholder with a fee of 0 instead of the API record, so stage sometimes shows the right total there by accident. This behaviour already exists on stage.

Smoke

Run in the app on Hoodi. Fixed = this branch @ 2c29679 (95440c0 only adds the warning) under pnpm dev. Baseline = stage @ f9d7e8e under pnpm dev, plus app.ssv.network. Production already includes the reshare total change but not the runway change, so stage is the baseline for S8. The wallet was a read-only stand-in for each cluster owner's address: no keys, and nothing signed or submitted.

  • S1 repro: pnpm vitest run src/lib/utils/__tests__/operator-fee.test.ts (with VITE_SSV_NETWORKS) → Tests 12 passed (12)
  • S2 My Clusters runway, SSV cluster 0x5146… (removed operator 182): 351 → 394 days. The other rows, including a liquidated cluster, are unchanged.
  • S3 Cluster page › Estimated Operational Runway, 0x5146…: 351 → 394 days.
  • S4 Operational Runway card, migrated ETH cluster 0xd4c7… (removed operators 182 and 184)
    • Runway: 427 → 643 days.
    • Liquidation collateral: 0.0038 → 0.0025 ETH.
    • A second ETH cluster, 0x03ee…: 53 → 82 days.
  • S5 Switch wizard › Operators Fee Comparison, 0x5146…: totals 8.4999 SSV / 0.0185 ETH → 7.4999 SSV / 0.0139 ETH. That equals the three non-removed rows (3 × 2.5 SSV, 3 × 0.0046 ETH).
  • S6 Switch wizard › Step two, 0x5146…
    • 6 months, 1 year and custom (100 days) costs are identical to the baseline.
    • "Use Current Runway": 351 days / 0.0903 ETH → 394 days / 0.1013 ETH.
  • S7 Clusters with no removed operator, 0x090c… (SSV) and 0x02e5… (ETH)
    • Runway, cluster page, switch wizard steps 1–3 and Funding Summary are identical to the baseline.
    • The only exception is the SSV balance, which shifts by about 0.0001 between runs because it accrues every block.
  • S8 Reshare › select operators › "Operators Yearly Fee", migrated ETH cluster 0xd4c7…: 0.0185 ETH on stage → 0.0093 ETH. On SSV cluster 0x5146… both builds show 7.4999 SSV, because the removed operator arrived as a placeholder with a fee of 0 (see "Known, not fixed here").
  • S9 Reactivate, liquidated clusters, nothing submitted
    • ETH 0x3b1a…, 1 year: 0.3671 → 0.306 ETH.
    • 0x7c5b…, 1 year: 0.0288 → 0.0242 ETH.
    • Both: operators fee 0.0186 → 0.0139.

A removed operator's fee is 0 on-chain, but the API keeps its last fee and
sumOperatorsFee added it to the burn rate, so runway was too short and
funding/reactivation amounts too high. Drop the ignore-removed flag: every
fee sum (ETH and SSV) now treats a removed operator as 0. Per-row display
of the last fee is unchanged.
… [SSV-WEB-001]

- select-operators: the reshare flow preselects every current cluster operator, removed ones included; route the Operators Yearly Fee total through sumOperatorsFee so a removed operator adds 0 on both the ETH and SSV paths
nir-ssvlabs
nir-ssvlabs previously approved these changes Oct 30, 2026
@RaekwonIII

Copy link
Copy Markdown

Reviewed against the product decision on SSV-WEB-001. The change does what we decided: the flag is gone, both the ETH and SSV paths return 0 for a removed operator, and every total goes through one helper. I also grepped the PR head for remaining fee / eth_fee uses. All the others are per-row display, not sums, so nothing is missed. Good tests, and good smoke on Hoodi.

Main issue: CI does not run the tests

The test step in build_deploy.yml (lines 89-90) is commented out. You list it under "Known, not fixed here", but I think it is the most important point in this PR. The green check does not include the 127 tests, so the 8 new tests protect nothing after merge. The next change to sumOperatorsFee can bring the bug back, and CI will still pass.

It does not need to block this PR, but it needs its own ticket and an owner, together with the two setup problems it depends on:

  • VITE_SSV_NETWORKS is not provided for tests (no setupFiles, no .env.test), so src/config/networks.ts throws at import time.
  • .husky/commit-msg has no executable bit, so commitlint never runs locally either.

Minor 1: a missing fee now counts as 0 without a warning

BigInt(operator.eth_fee || "0") turns a missing fee into 0. Before, getOperatorEthFee threw. A 0 lowers the burn rate and lengthens the runway, which is the unsafe direction, and the same kind of problem as SSV-WEB-002. The fee comparison already did this before, so the risk is small. Consider logging a warning when an operator that is not removed has no fee, so the case shows up instead of passing silently.

Minor 2: rows and totals can disagree

In the fee comparison and the reshare operator list, a removed operator's row still shows its last fee, while the total leaves it out. That is within the decision ("a label outside the sum"), but a user who adds up the rows gets a different number. On SSV-EXPL-002 we decided the explorer shows "—" for a removed operator's fee. A follow-up should do the same in the webapp, ideally with a "Removed" tag on the row.

Neither minor point blocks the merge.

A missing fee counts as 0, which understates burn rate and lengthens
runway. Warn once per operator and field so the case shows up.
@Chris-ssvlabs

Copy link
Copy Markdown
Contributor Author

Thanks, Massimo. All three points are handled:

  • CI does not run the tests: filed as SSV-CI-001 (P2). It covers re-enabling the test step in build_deploy.yml, providing VITE_SSV_NETWORKS for tests, and making .husky/commit-msg executable. The owner is still to be assigned.
  • Minor 1, missing fee: fixed in 95440c0. A missing fee on an operator that is not removed still counts as 0, but now logs a console.warn once per operator and field. Four new tests cover it. Removing the warning call, or the once-only check, makes the tests fail. Gates at 95440c0: Tests 131 passed (131), type-check exit 0, lint 0 errors (1 warning that already exists, in a file outside this diff).
  • Minor 2, rows and totals disagree: filed as SSV-WEB-006 (P3). It shows "—" and a "Removed" tag for removed operators, matching SSV-EXPL-002, in the fee comparison, the reshare operator list and the other per-row fee displays.

@stefan-ssv-labs stefan-ssv-labs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM at 95440c0. Reviewed against SSV-WEB-001 and the agreed product decision. Removing the flag and treating removed operators as zero in the shared ETH and SSV fee helpers is the right fix. The runway, funding, reactivation, fee-comparison and reshare totals use the corrected helpers; I found no missed fee total or blocking regression.

Verified the premise in the contract and Subsquid code: removal zeroes both on-chain fees, while the indexed record retains the last fees. Independently ran all 131 tests across 11 files, type-check and lint. All passed; lint reports one existing warning outside this diff. I reviewed the recorded Hoodi smoke results but did not independently repeat the full browser/wallet flow.

SSV-CI-001 remains a pre-existing, non-blocking follow-up: CI does not run the tests, and the ticket still needs an owner. The stale per-row fee display is explicitly accepted for this PR and tracked separately in SSV-WEB-006. Neither item blocks this approval.

@sebastian-ssvlabs sebastian-ssvlabs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed against the on-chain behaviour: removeOperator resets both fee and ethFee to zero, and burn rate is a plain sum of current operator fees, so counting a removed operator as 0 in every total matches the contract. All sums now go through sumOperatorsFee; remaining raw fee reads are per-row display only. Reshare keeps Next disabled while a removed operator is selected. No blocking findings.

@Chris-ssvlabs
Chris-ssvlabs merged commit 9f4e040 into stage Oct 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants