Skip to content

fix(test): retire executed-migration simulations, repoint authoriser truth - #268

Merged
thedavidmeister merged 1 commit into
mainfrom
fix/post-migration-test-staleness
Jul 22, 2026
Merged

fix(test): retire executed-migration simulations, repoint authoriser truth#268
thedavidmeister merged 1 commit into
mainfrom
fix/post-migration-test-staleness

Conversation

@hardyjosh

@hardyjosh hardyjosh commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Main went red when the two Base Safe batches executed on-chain (V4 receipt
vault upgrade + authoriser swap): the tests that simulated those migrations
from reconstructed pre-state lost their premise, and the live-state pins
still asserted the pre-swap V3 authoriser.

  • The live production authoriser is the V4 clone since the swap executed —
    verified on-chain across all 22 production vaults. Repoint
    testProdReceiptVaultsShareUniformAuthoriser and
    testAssertProductionStateBasePassesLive to
    STOX_PROD_AUTHORISER_V4_CLONE, deliberately STRICT: a newly-deployed
    token entering the table on a previous authoriser goes red until its swap
    batch executes.
  • Delete MigrateBeaconOwnersTest (+harness): the beacon-owner migration
    executed; walking it from pre-state is dead. Live post-state coverage
    lives in the strict per-chain beacon-ownership pin.
  • Delete the two executed-run simulations in the 20260623 upgrade test
    (testRunCompletesAndWritesArtifact,
    testAssertPostStateRevertsWhenVaultNotSwapped) (+harness); keep the
    migration-window pin and the pre-flight guard tests.
  • Delete testAuthoriserSwapReWiresGatingOnLiveVault from the V3 shadow
    fork: it required the live vault to still be pre-swap.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01VPs1hCTxusmaSeFKvoc4Kr

Summary by CodeRabbit

  • Tests
    • Updated production-state and token invariant checks to validate the V4 authoriser configuration.
    • Removed obsolete migration and upgrade test scenarios, including end-to-end artifact checks and authoriser mismatch cases.
    • Removed legacy shadow-fork coverage for authoriser rewiring and related gating behavior.
    • Removed supporting test harnesses and fork-based migration tests no longer used by the suite.

…truth

Main went red when the two Base Safe batches executed on-chain (V4 receipt
vault upgrade + authoriser swap): the tests that simulated those migrations
from reconstructed pre-state lost their premise, and the live-state pins
still asserted the pre-swap V3 authoriser.

- The live production authoriser is the V4 clone since the swap executed —
  verified on-chain across all 22 production vaults. Repoint
  `testProdReceiptVaultsShareUniformAuthoriser` and
  `testAssertProductionStateBasePassesLive` to
  `STOX_PROD_AUTHORISER_V4_CLONE`, deliberately STRICT: a newly-deployed
  token entering the table on a previous authoriser goes red until its swap
  batch executes.
- Delete `MigrateBeaconOwnersTest` (+harness): the beacon-owner migration
  executed; walking it from pre-state is dead. Live post-state coverage
  lives in the strict per-chain beacon-ownership pin.
- Delete the two executed-run simulations in the 20260623 upgrade test
  (`testRunCompletesAndWritesArtifact`,
  `testAssertPostStateRevertsWhenVaultNotSwapped`) (+harness); keep the
  migration-window pin and the pre-flight guard tests.
- Delete `testAuthoriserSwapReWiresGatingOnLiveVault` from the V3 shadow
  fork: it required the live vault to still be pre-swap.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VPs1hCTxusmaSeFKvoc4Kr
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9afe0178-9a6f-4238-935d-448e7c75e53a

📥 Commits

Reviewing files that changed from the base of the PR and between 1ce0ab8 and b01ed8c.

📒 Files selected for processing (7)
  • test/script/20260623-upgrade-receipt-vaults-to-v4.t.sol
  • test/script/MigrateBeaconOwnersHarness.sol
  • test/script/MigrateBeaconOwnersTest.t.sol
  • test/script/UpgradeReceiptVaultsToV4Harness.sol
  • test/src/concrete/upgrade/V3UpgradeShadowFork.t.sol
  • test/src/lib/LibInvariants.t.sol
  • test/src/lib/LibTokenInvariants.t.sol
💤 Files with no reviewable changes (3)
  • test/script/MigrateBeaconOwnersHarness.sol
  • test/script/UpgradeReceiptVaultsToV4Harness.sol
  • test/script/MigrateBeaconOwnersTest.t.sol

📝 Walkthrough

Walkthrough

The PR removes obsolete beacon migration and receipt-vault upgrade test harnesses and scenarios, deletes shadow-fork authoriser swap coverage, and updates production invariant tests to reference the V4 authoriser clone.

Changes

V4 authoriser test alignment

Layer / File(s) Summary
Receipt upgrade test cleanup
test/script/20260623-upgrade-receipt-vaults-to-v4.t.sol, test/script/UpgradeReceiptVaultsToV4Harness.sol
Removes receipt-vault upgrade artifact and post-state mismatch coverage, along with its harness.
Beacon migration test removal
test/script/MigrateBeaconOwnersHarness.sol, test/script/MigrateBeaconOwnersTest.t.sol
Deletes the beacon ownership migration harness and fork-based migration tests.
Shadow-fork authoriser coverage cleanup
test/src/concrete/upgrade/V3UpgradeShadowFork.t.sol
Removes live-vault authoriser swapping, gating assertions, its helper interface, and unused imports.
Invariant expectations retargeted to V4
test/src/lib/LibInvariants.t.sol, test/src/lib/LibTokenInvariants.t.sol
Updates production and token authoriser invariants to use LibProdDeployV4.STOX_PROD_AUTHORISER_V4_CLONE.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: thedavidmeister, thedavidmeister

Poem

I’m a rabbit with a V4 key,
Trimming old tests beneath the tree.
Beacons rest, swaps hop away,
Invariants point to clones today.
Fluffy checks now match the way!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: removing outdated migration simulations and updating tests to the V4 authoriser.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/post-migration-test-staleness

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thedavidmeister

Copy link
Copy Markdown
Contributor

Reviewed b01ed8c: approved

Human approval given interactively. Merging over two RED checks, so here is the justification for each — both are pre-existing main-red that this PR inherits and neither is introduced by it.

check status why it is expected
rainix-sol / test / test FAILURE testDeployTag() (0_1_8 != 0_1_16) and testProdDeployEthereumV4() (V4 beacon owner mismatch). Main run 29750799200 fails the same two, count 2 vs count 2 here. Neither test lives in a file this PR touches.
git-clean FAILURE uncommitted regenerated src/generated/LibProdDeployCurrent.sol + LibProdDeployV4.sol. Also failing on main; untouched here.

CodeRabbit, rainix-sol/legal, rainix-sol/static and Graphite/mergeability_check all SUCCESS. 0 unresolved review threads.

One correction to the PR description, for the record

The body says main went red because "the live-state pins still asserted the pre-swap V3 authoriser". On main those pins pass:

[PASS] testAssertProductionStateBasePassesLive()
[PASS] testProdReceiptVaultsShareUniformAuthoriser()

They pass because assertUniformAuthoriser checks only that all vaults share one authoriser, which stayed true across the V3→V4 swap — it never asserted V3 identity. So this PR is not a red fix. What it actually does, and what it is being merged for:

  1. retires ~396 lines of migration simulations whose premise died when the two Base Safe batches executed on-chain, and
  2. strengthens those two pins from "uniform" to "uniform and equals LibProdDeployV4.STOX_PROD_AUTHORISER_V4_CLONE".

Both are worth having; the framing was just wrong. The real main-red causes map elsewhere: testProdDeployEthereumV4#266, git-clean → likely #258, and testDeployTag has no open PR fixing it and drifts with every publish.

Known consequence of the new strictness

The strict pin is deliberate — a token entering the table on a previous authoriser goes red until its swap executes. #265 adds six such tokens (MU/AMD/AVGO/AMAT/LRCX/TTWO, all still on V3), so merging #265 before #269's swap bundle executes on-chain will red main again by design. Flagging so that is a chosen step, not a surprise.

Landing with --merge per the org's no-squash convention.

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.

2 participants