Skip to content

feat(lib): chain-aware token-owner Safe assertion - #267

Merged
thedavidmeister merged 2 commits into
mainfrom
lib/chain-aware-safe-preflight
Jul 22, 2026
Merged

feat(lib): chain-aware token-owner Safe assertion#267
thedavidmeister merged 2 commits into
mainfrom
lib/chain-aware-safe-preflight

Conversation

@hardyjosh

Copy link
Copy Markdown
Contributor

Add LibSafeInvariants.assertActiveChainTokenOwnerSafe(chainId) — the single
entry point that resolves the active chain's token-owner Safe and asserts it
chain-appropriately: Base (the reference chain) order-sensitively via
assertAll against the canonical pinned roster; every other chain order-
insensitively via assertPolicyMatchesBase (a per-chain Safe shares the owner
SET + threshold + v1.4.1 identity, but its getOwners() order is an incidental
deploy artifact).

Fork-tested against the LIVE Base and Ethereum Safes, so any consumer (e.g. a
broadcast script's pre-flight) is proven against every pinned chain's Safe on
every CI run — a dispatch can never revert on a Safe check CI hasn't already
exercised on that chain.

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

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e85b512b-c1db-4713-84d0-0dc64bc7fa37

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch lib/chain-aware-safe-preflight

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.

Josh Hardy and others added 2 commits July 21, 2026 22:48
Add `LibSafeInvariants.assertActiveChainTokenOwnerSafe(chainId)` — the single
entry point that resolves the active chain's token-owner Safe and asserts it
chain-appropriately: Base (the reference chain) order-sensitively via
`assertAll` against the canonical pinned roster; every other chain order-
insensitively via `assertPolicyMatchesBase` (a per-chain Safe shares the owner
SET + threshold + v1.4.1 identity, but its getOwners() order is an incidental
deploy artifact).

Fork-tested against the LIVE Base and Ethereum Safes, so any consumer (e.g. a
broadcast script's pre-flight) is proven against every pinned chain's Safe on
every CI run — a dispatch can never revert on a Safe check CI hasn't already
exercised on that chain.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VPs1hCTxusmaSeFKvoc4Kr
Rename `assertPolicyMatchesBase` -> `assertTokenOwnerSafePolicy` and reframe:
the pinned owner SET + threshold + v1.4.1 identity are properties of the
ORGANISATION, not of Base — every chain's token-owner Safe (Base included) is
asserted against the same pins. `assertActiveChainTokenOwnerSafe` drops its
Base special-case: resolve the chain's Safe address (the only per-chain
artifact), assert the shared policy, identically everywhere.

`LibInvariants.assertProductionState` now composes the resolve+assert entry
point instead of hand-pairing `safeForChainId` with the policy check.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VPs1hCTxusmaSeFKvoc4Kr
@hardyjosh
hardyjosh changed the base branch from main to graphite-base/267 July 21, 2026 22:52
@hardyjosh
hardyjosh force-pushed the lib/chain-aware-safe-preflight branch from 18acfcf to 4d875cc Compare July 21, 2026 22:52
@hardyjosh
hardyjosh changed the base branch from graphite-base/267 to fix/post-migration-test-staleness July 21, 2026 22:52
@thedavidmeister
thedavidmeister changed the base branch from fix/post-migration-test-staleness to main July 22, 2026 15:16
@thedavidmeister

Copy link
Copy Markdown
Contributor

Reviewed 4d875cc: approved

Human approval given interactively. MERGEABLE, base retargeted to main, 0 unresolved review threads.

Merging over two reds — both inherited

check status why expected
rainix-sol / test / test FAILURE testDeployTag (0_1_8 != 0_1_16) + testProdDeployEthereumV4 (V4 beacon owner mismatch). Identical text and same count (2) as main and as #268. Neither test is in a file this PR touches.
git-clean FAILURE uncommitted regenerated src/generated/*; failing on main, untouched here.

CodeRabbit, legal, static, build-artifact and Graphite/mergeability_check all SUCCESS.

Caveat recorded: the CI is stale

Those results are from 2026-07-21T22:5x — before #268 merged (07-22) and before I retargeted this PR from fix/post-migration-test-staleness to main. So they do not reflect the state being merged. A fresh run against current main would have been the honest basis; merging on the maintainer's explicit word instead, with that limitation stated rather than papered over. Main's own CI runs immediately after this merge and will surface anything this misses — an acceptable net given main is already red on two known causes.

Why the change is sound

Adds LibSafeInvariants.assertActiveChainTokenOwnerSafe(chainId): resolve the active chain's token-owner Safe and assert the chain-agnostic policy (v1.4.1 identity, owner set, threshold) in one call. Base keeps its order-sensitive roster pin; other chains use order-insensitive owners, justified because getOwners() order is a Safe linked-list insertion artifact rather than policy.

I probed the obvious fail-open in that weakening — can an unordered set check miss an extra owner? It cannot. assertOwnerSetUnordered (pre-existing on main, not introduced here) tests actual.length != expected.length first, then membership: an extra owner trips the count, a swapped owner trips membership, and Safe forbids duplicate owners. That is genuine set equality.

An unpinned chain reverts UnsupportedChainForTokenOwnerSafe rather than silently asserting another chain's Safe — the right fail-safe direction.

This is also the prerequisite for #266, which fixes testProdDeployEthereumV4 — one of the two live main-red causes.

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