Skip to content

feat(deploy): Ethereum V4 authoriser clone — chain-parametric deploy script + per-chain pin selector - #229

Merged
thedavidmeister merged 10 commits into
mainfrom
feat/rai-1095-ethereum-bootstrap
Jul 22, 2026
Merged

feat(deploy): Ethereum V4 authoriser clone — chain-parametric deploy script + per-chain pin selector#229
thedavidmeister merged 10 commits into
mainfrom
feat/rai-1095-ethereum-bootstrap

Conversation

@hardyjosh

@hardyjosh hardyjosh commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Deploys the V4 authoriser clone on Ethereum by running the same script
that deployed it on Base, adapted only to pick the active chain's pin.

  • Deploy script (20260619-deploy-v4-authoriser-clone) — already
    chain-agnostic (it reads the impl, Safe, and role grants from libs), so the
    only change is swapping the single-chain clone lookup for a per-chain one
    (LibProdAuthoriserClones.cloneForChainId(block.chainid)). Deploy on
    Ethereum by running it with --rpc-url ethereum; on Base it self-guards
    (V4AuthoriserClonePinAlreadyHydrated) because the Base pin is set.

  • Per-chain clone selectionactiveChainClonePin() in the deploy script
    dispatches on block.chainid to the active chain's LibProdDeployV4 pin.
    It is a SELECTOR, not a second copy of the addresses: Base resolves the
    generated STOX_PROD_AUTHORISER_V4_CLONE pin (0x315b…) and Ethereum
    resolves STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM, an address(0)
    placeholder until the post-deploy hydrate PR fills it (the clone is a
    nonce-based CloneFactory.clone, so its address cannot be known ahead of
    the broadcast). An unpinned chain reverts
    V4AuthoriserCloneUnsupportedChain rather than falling back to another
    chain's clone. The chain ids come from LibSafeInvariants.BASE_CHAIN_ID /
    ETHEREUM_CHAIN_ID, the same constants the sibling per-chain selectors use.
    The EIP-1167 codehash is deterministic and shared, so it stays on the
    existing generated pin. No hand-written change to src/generated — the
    new Ethereum constant is emitted by BuildPointers, so the file still
    regenerates byte-for-byte.

  • run-script workflow — drops the clone deploy from the Safe-artifact
    pipeline. It's a deploy-key broadcast (the key deploys the clone, assigns
    the Safe's role grants, and self-revokes admin), not a Safe-signed artifact,
    so it belongs in the manual deploy-key flow.

  • Runbook (docs/ETHEREUM_BOOTSTRAP.md) — operator steps for the Ethereum
    bootstrap.

Summary by CodeRabbit

  • New Features

    • Deployment workflows now support selecting Base or Ethereum as the target network.
    • Network-specific deployment verification and authoriser clone configuration are supported.
    • Generated deployment pointers now include an Ethereum V4 authoriser clone placeholder.
  • Documentation

    • Clarified receipt vault upgrade steps and deployment test comments.

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The workflow now targets Base or Ethereum. V4 authoriser clone deployment selects chain-specific pins and Safe grant expectations, rejects unsupported chains, and verifies the active-chain post-state. Generated constants and related documentation comments are updated.

Changes

Network-aware V4 deployment

Layer / File(s) Summary
Workflow network selection
.github/workflows/manual-broadcast.yaml
Manual broadcasts accept Base or Ethereum, isolate concurrency by network, and pass the selected RPC endpoint to Forge.
Chain-specific clone pinning and verification
script/20260619-deploy-v4-authoriser-clone.s.sol, script/BuildPointers.sol, script/20260623-upgrade-receipt-vaults-to-v4.s.sol, test/src/concrete/deploy/StoxProdV4PostSwap.t.sol
Generated V4 deployment pointers include an Ethereum clone placeholder; deployment and post-state checks resolve active-chain pins, Safes, and expected grants, with related comments updated.

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

Sequence Diagram(s)

sequenceDiagram
  participant Operator
  participant GitHubActions
  participant forge
  participant SelectedRPC
  participant DeployV4AuthoriserClone
  participant LibSafeInvariants
  Operator->>GitHubActions: Select script and network
  GitHubActions->>forge: Pass selected RPC alias and secrets
  forge->>SelectedRPC: Execute broadcast
  SelectedRPC-->>DeployV4AuthoriserClone: Run on active chain
  DeployV4AuthoriserClone->>LibSafeInvariants: Resolve clone pin and Safe
  DeployV4AuthoriserClone->>DeployV4AuthoriserClone: Verify chain-specific post-state
Loading

Possibly related PRs

Suggested reviewers: thedavidmeister

Poem

I’m a rabbit hopping chain to chain,
Base and Ethereum through the rain.
Pins now follow where we go,
Safes grant the roles they know.
Clone checks bloom—no guesswork remains!

🚥 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 clearly summarizes the main change: chain-parametric V4 authoriser clone deployment with per-chain pin selection.
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 feat/rai-1095-ethereum-bootstrap

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Checkov (3.3.8)
.github/workflows/manual-broadcast.yaml

Traceback (most recent call last):
File "/usr/local/bin/checkov", line 2, in
from checkov.main import Checkov
ModuleNotFoundError: No module named 'checkov'


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.

@hardyjosh
hardyjosh force-pushed the feat/rai-1095-ethereum-bootstrap branch from 7f66d31 to 5769d2e Compare July 7, 2026 10:55
@hardyjosh
hardyjosh force-pushed the feat/rai-1096-chain-principals branch from 83c4fc8 to 5043963 Compare July 7, 2026 11:03
@hardyjosh
hardyjosh force-pushed the feat/rai-1095-ethereum-bootstrap branch from 5769d2e to 2320571 Compare July 7, 2026 11:04
@hardyjosh
hardyjosh force-pushed the feat/rai-1096-chain-principals branch from 5043963 to 693fc62 Compare July 15, 2026 13:48
@hardyjosh
hardyjosh force-pushed the feat/rai-1095-ethereum-bootstrap branch from 2320571 to 94b4454 Compare July 15, 2026 13:48
@hardyjosh
hardyjosh force-pushed the feat/rai-1096-chain-principals branch from 693fc62 to 86a6cd3 Compare July 15, 2026 15:04
@hardyjosh
hardyjosh force-pushed the feat/rai-1095-ethereum-bootstrap branch 2 times, most recently from 458ecd9 to f030d37 Compare July 15, 2026 15:30
@hardyjosh
hardyjosh force-pushed the feat/rai-1096-chain-principals branch from 86a6cd3 to 0e0a058 Compare July 15, 2026 15:30
Josh Hardy and others added 9 commits July 22, 2026 09:57
…ne script + runbook

RAI-1095 / RAI-1096. The Base clone-deploy + grants-mirror script
becomes chain-parametric and Ethereum gets a thin subclass run of it,
so Ethereum's permission structure is Base's by construction.

- 20260619-deploy-v4-authoriser-clone.s.sol: every chain-dependent read
  now flows through virtual getters with Base defaults — _principals()
  (LibChainPrincipals.base()), _grants() (the chain-parametric
  expectedGrants filled with _principals()), _safePreflight() (full
  LibSafeInvariants.assertAll on Base), artifact paths + bundle names.
  Bundle logic, pre-flights and verify() are unchanged; on Base the
  authored bundles are byte-identical to before (grant map pinned equal
  by LibChainPrincipalsTest).

- New 20260706-deploy-v4-authoriser-clone-ethereum.s.sol: subclass
  overriding principals (Ethereum, typed-revert until hydrated), the
  clone pin (STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM), Safe pre-flight
  (assertImmutableInvariants until the Ethereum Safe owner/threshold
  pins exist — upgraded by the principals pin PR), and
  Ethereum-suffixed artifacts/names.

- LibProdDeployV4: STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM placeholder
  pin. No separate codehash pin — the EIP-1167 runtime embeds only the
  impl address, which is the same deterministic Zoltu address on every
  chain, so the Base codehash pin covers both clones.

- docs/ETHEREUM_BOOTSTRAP.md: full ordered bootstrap runbook (suites →
  CloneFactory → principals+Safe pins → Fireblocks → clone bundle →
  clone pin → grants mirror → token deploys → parity green). Notes the
  canonical Rain CloneFactory is NOT yet deployed on Ethereum
  (verified 2026-07-06) — permissionless Zoltu deploy, gated by the
  existing CloneFactoryNotDeployed pre-flight.

Pre-existing red (inherited from the draft stack, untouched):
test/script/20260623-upgrade-receipt-vaults-to-v4.t.sol has two
deliberate hydration tripwires failing on the stack tip.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VPs1hCTxusmaSeFKvoc4Kr
…rodAuthoriserClones

BuildPointers no longer emits a clone address. The clone ADDRESS is
non-deterministic (a nonce-based CloneFactory clone), so it cannot be
generated — only the deterministic clone CODEHASH (derived from the pinned
V4 impl) is generated in LibProdDeployV4 and re-exported. Every chain's
clone ADDRESS now lives in one hand-maintained home,
`src/lib/LibProdAuthoriserClones.sol`, instead of the Ethereum pin sitting
there while the Base pin sat in the generated file (Josh: "otherwise we are
spreading it around in a messy way").

- `LibProdAuthoriserClones` gains `STOX_PROD_AUTHORISER_V4_CLONE_BASE`
  alongside `_ETHEREUM` (both `address(0)` placeholders until hydrated), plus
  a `cloneForChainId(block.chainid)` selector (typed revert on unsupported
  chains) for the chain-agnostic clone-deploy script.
- `BuildPointers` + generated `LibProdDeployV4` / `LibProdDeployCurrent` drop
  the `STOX_PROD_AUTHORISER_V4_CLONE` address constant (codehash stays).
- Repoint every consumer: the chain-agnostic `20260619` clone-deploy script
  uses `cloneForChainId`; the Base-specific consumers (`20260623` upgrade
  script + test, `StoxProdV4PostSwap`, `LibInvariants` Base overloads) use
  `STOX_PROD_AUTHORISER_V4_CLONE_BASE`.
- Move the address-placeholder guard into a new `LibProdAuthoriserClones`
  test; `LibProdDeployV4`'s test keeps the codehash-derivation assertion.

Regenerated with BuildPointers + forge fmt (git-clean clean). Verified on
live Base: LibProdAuthoriserClonesTest, the 20260619 clone-deploy suite,
LibInvariants + LibAuthoriserInvariants pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VPs1hCTxusmaSeFKvoc4Kr
Reflect the LibChainPrincipals removal and the clone-pin consolidation:
- Principals are shared, not per-chain: no LibChainPrincipals pins to
  hydrate; the Safe (LibSafeInvariants.STOX_TOKEN_OWNER_SAFE) and signer
  (LibAuthoriserInvariants.GRANTEE_SERVICE_1C66) are the same on every chain.
  The Safe policy gate is the DeploySafeEthereumTest forcing-function test.
- The clone-script "already hydrated" guard is now network-aware
  (cloneForChainId(block.chainid)), so the earlier follow-up is done.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VPs1hCTxusmaSeFKvoc4Kr
Matched/deterministic Safe address abandoned (Josh, 2026-07-15). Rewrite §3:
deploy the Ethereum token-owner Safe out-of-band (clean v1.4.1, Base's owners
+ threshold), pin STOX_TOKEN_OWNER_SAFE_ETHEREUM, and let
EthereumTokenOwnerSafeParityTest assert policy parity (order-insensitive).
Remove the genesis-replay §3a and its script/LibStoxSafeGenesis references.

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

run-script.yaml dispatches only Safe Tx Builder JSON emitting scripts (a
signer reviews + signs the artifact; it never --broadcasts). The V4
authoriser clone deploy (20260619-deploy-v4-authoriser-clone) deploys the
clone and self-revokes the deployer's roles — nothing for the Safe to sign —
so it's a deploy-key broadcast that belongs only in manual-broadcast.yaml.
Remove it from this dispatcher.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VPs1hCTxusmaSeFKvoc4Kr
STOX_PROD_AUTHORISER_V4_CLONE_BASE = 0x315b16faa6eE413faBCa877d3851B3818369f0cD
— the V4 authoriser clone deployed on Base (was address(0); on main in the
generated LibProdDeployV4). The Base vaults are swapped onto it by the
separate V4 receipt-vault upgrade, which hasn't run yet, so any strict
"vault authoriser == clone" assertion (the cross-chain parity token leg) is
RED by design until the Base migration completes — it goes green when the
swap lands. Placeholder guard test -> real-address assertion.

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

The rebase onto main (0.1.11) garbled the generated-file 3-way merge, leaving
LibProdDeployV4/Current missing the 0_1_11 tag constants. Regenerate from
BuildPointers (which on this branch omits the clone ADDRESS — PICKUP 1 keeps
it hand-maintained in LibProdAuthoriserClones). Deterministic output; git-clean
now passes. No semantic change beyond the version-tag refresh.

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

The broadcast hardcoded `LibSafeInvariants.STOX_TOKEN_OWNER_SAFE` (Base) for
its pre-flight and post-state Safe checks, so dispatching it on Ethereum
reverted `SafeProxyCodehashMismatch` — the Base Safe address has no code on
Ethereum. Two-part fix, matching the "same assertion in the script and CI"
structure:

1. New `LibSafeInvariants.assertActiveChainTokenOwnerSafe(chainId)` — the single
   entry point that resolves the per-chain Safe and asserts it chain-
   appropriately: Base (reference chain) order-sensitively via `assertAll`
   against the canonical roster; every other chain order-insensitively via
   `assertPolicyMatchesBase` (a fresh per-chain Safe shares the owner SET +
   threshold + v1.4.1 identity but has an incidental getOwners() order). The
   script's run() pre-flight and post-state now call this / `safeForChainId`
   instead of the Base constant.

2. Test gap closed: every existing test in the script's suite forked Base only,
   so the Safe leg was never exercised on Ethereum and CI stayed green while
   the real broadcast reverted. Add `testSafePreflightPassesOnEthereum` (and a
   Base counterpart) that run the exact pre-flight against the live per-chain
   Safe.

Verified against live Base + Ethereum: both pre-flights pass; the Ethereum
Safe (0x3840…) matches Base's policy.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VPs1hCTxusmaSeFKvoc4Kr
…hain's Safe

The Ethereum dry run (with the CloneFactory now live) cleared every
pre-flight and then tripped its own post-state gate:
ExpectedGrantMissing(<admin role>, <BASE Safe>). Both grant call sites used
the no-arg `expectedGrants()` — pinned to Base's Safe — so on Ethereum
step 2 would have provisioned Base's Safe with the three direct action
roles, and the post-state sweep demanded admin roles Base's Safe never
received. Use `expectedGrants(safe)` with the chain-resolved Safe in
`run()` and `_assertPostState`. On Base the parametric map equals the
no-arg map, so the Base suite is unchanged (11/11 green). Full Ethereum
simulation now completes end to end.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VPs1hCTxusmaSeFKvoc4Kr
The two sibling per-chain selectors resolve their chains through
LibSafeInvariants.BASE_CHAIN_ID / ETHEREUM_CHAIN_ID; this one used the raw
literals, leaving one dispatch in the subsystem where the numbers stand alone.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/manual-broadcast.yaml:
- Around line 73-89: Update the workflow step using BASE_RPC_URL,
ETHEREUM_RPC_URL, and NETWORK so only the RPC secret corresponding to the
selected network is exposed to the Forge invocation. Remove the unconditional
injection of both RPC secrets and derive or conditionally assign the selected
value before running the command, while preserving NETWORK for --rpc-url alias
resolution.
- Around line 22-34: Validate the selected network against the chosen script
before invoking Forge in the workflow. Add an explicit script-to-network
allowlist or reject incompatible pairs so Base-only scripts such as
20260623-upgrade-receipt-vaults-to-v4.s.sol cannot run with ethereum, while
preserving valid base and Ethereum bootstrap combinations.

In `@script/20260619-deploy-v4-authoriser-clone.s.sol`:
- Around line 140-155: Add focused tests covering activeChainClonePin for Base
and Ethereum, asserting each returns its corresponding LibProdDeployV4 pin,
including the zero Ethereum placeholder when applicable. Add an
unsupported-chain test that expects V4AuthoriserCloneUnsupportedChain with the
chain ID, and ensure all selector branches are exercised.

In `@script/BuildPointers.sol`:
- Around line 334-339: Update genCurrent() to expose
STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM alongside the existing Base clone alias,
reusing the Ethereum constant emitted by genV4() so LibProdDeployCurrent stays
synchronized with the new pin.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 51f1a3ab-f26b-4921-a671-63d863723260

📥 Commits

Reviewing files that changed from the base of the PR and between d885b92 and 470e8ab.

⛔ Files ignored due to path filters (1)
  • src/generated/LibProdDeployV4.sol is excluded by !**/generated/**
📒 Files selected for processing (5)
  • .github/workflows/manual-broadcast.yaml
  • script/20260619-deploy-v4-authoriser-clone.s.sol
  • script/20260623-upgrade-receipt-vaults-to-v4.s.sol
  • script/BuildPointers.sol
  • test/src/concrete/deploy/StoxProdV4PostSwap.t.sol

Comment on lines +22 to +34
network:
description: 'Network to broadcast against (default: base)'
required: true
type: choice
default: 'base'
options:
# The chain to broadcast against — the foundry.toml `[rpc_endpoints]`
# alias (`--rpc-url <network>`) and the key selecting which RPC
# secret to expose. The Ethereum bootstrap broadcasts (authoriser
# clone, token deploy) use `ethereum`; the original Base deploys use
# `base`.
- 'base'
- 'ethereum'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Constrain network choices to compatible scripts.

network is independent from script, so a Base-only script such as script/20260623-upgrade-receipt-vaults-to-v4.s.sol can be run with ethereum while the workflow still passes --broadcast. Add a script-to-network allowlist or reject incompatible pairs before invoking Forge.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/manual-broadcast.yaml around lines 22 - 34, Validate the
selected network against the chosen script before invoking Forge in the
workflow. Add an explicit script-to-network allowlist or reject incompatible
pairs so Base-only scripts such as 20260623-upgrade-receipt-vaults-to-v4.s.sol
cannot run with ethereum, while preserving valid base and Ethereum bootstrap
combinations.

Comment on lines +73 to +89
# Expose every network's RPC; `--rpc-url ${NETWORK}` selects which
# one foundry.toml resolves via its `[rpc_endpoints]` aliases.
BASE_RPC_URL: ${{ secrets.RPC_URL_BASE_FORK }}
# Pass the choice input via env rather than template-expanding it
# into the command, so the dispatched script name is used as a
# literal argument and cannot inject shell.
ETHEREUM_RPC_URL: ${{ secrets.RPC_URL_ETHEREUM_FORK }}
# Pass the choice inputs via env rather than template-expanding them
# into the command, so the dispatched values are used as literal
# arguments and cannot inject shell.
SCRIPT: ${{ inputs.script }}
NETWORK: ${{ inputs.network }}
# PRIVATE_KEY is only available inside this step — matching
# `manual-sol-artifacts.yaml`. The workflow file itself does not
# persist it anywhere else.
PRIVATE_KEY: ${{ secrets.PRIVATE_KEY }}
run: |
nix develop --command forge script "script/${SCRIPT}.s.sol" \
--sig 'run()' \
--rpc-url base \
--rpc-url "${NETWORK}" \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Expose only the selected RPC secret.

Both BASE_RPC_URL and ETHEREUM_RPC_URL are injected into every Forge invocation. Any dispatched script can read the non-selected secret, and provider credentials may be embedded in these URLs. Pass only the RPC secret for the selected network.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/manual-broadcast.yaml around lines 73 - 89, Update the
workflow step using BASE_RPC_URL, ETHEREUM_RPC_URL, and NETWORK so only the RPC
secret corresponding to the selected network is exposed to the Forge invocation.
Remove the unconditional injection of both RPC secrets and derive or
conditionally assign the selected value before running the command, while
preserving NETWORK for --rpc-url alias resolution.

Comment on lines +140 to +155
/// @notice The V4 authoriser clone pin for the active chain, selected by
/// `block.chainid` from `LibProdDeployV4` — `address(0)` until that chain's
/// clone is deployed and the pin hydrated. Reverts for any chain without a
/// pin rather than falling back to another chain's clone (reading the wrong
/// chain's clone is the catastrophic failure this guard exists to prevent).
/// @return The active chain's clone pin.
function activeChainClonePin() internal view returns (address) {
if (block.chainid == LibSafeInvariants.ETHEREUM_CHAIN_ID) {
return LibProdDeployV4.STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM;
}
if (block.chainid == LibSafeInvariants.BASE_CHAIN_ID) {
return LibProdDeployV4.STOX_PROD_AUTHORISER_V4_CLONE;
}
revert V4AuthoriserCloneUnsupportedChain(block.chainid);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add tests for every chain-selection branch.

This selector is the deployment safety boundary, but the supplied tests do not cover Base, Ethereum, or unsupported-chain behavior. Add focused tests for both pins, the zero Ethereum placeholder, and V4AuthoriserCloneUnsupportedChain.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@script/20260619-deploy-v4-authoriser-clone.s.sol` around lines 140 - 155, Add
focused tests covering activeChainClonePin for Base and Ethereum, asserting each
returns its corresponding LibProdDeployV4 pin, including the zero Ethereum
placeholder when applicable. Add an unsupported-chain test that expects
V4AuthoriserCloneUnsupportedChain with the chain ID, and ensure all selector
branches are exercised.

Comment thread script/BuildPointers.sol
Comment on lines +334 to +339
// Ethereum V4 authoriser clone — same nonce-based `CloneFactory.clone`
// deploy as Base, so its address is non-deterministic and hand-provided
// here (placeholder `address(0)` until the Ethereum bootstrap deploys it
// and the hydrate PR replaces this literal). Shares the deterministic
// EIP-1167 codehash above (same impl on every chain).
vm.writeLine(GEN_V4_PATH, "address constant STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM = address(0);");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Keep LibProdDeployCurrent synchronized with the new pin.

genV4() now emits STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM, but genCurrent() only aliases the Base clone at Lines [364-371]. Consumers importing the current generated library cannot access the Ethereum pin.

Proposed fix
+        vm.writeLine(
+            GEN_CURRENT_PATH,
+            "address constant STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM = LibProdDeployV4.STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM;"
+        );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Ethereum V4 authoriser clone — same nonce-based `CloneFactory.clone`
// deploy as Base, so its address is non-deterministic and hand-provided
// here (placeholder `address(0)` until the Ethereum bootstrap deploys it
// and the hydrate PR replaces this literal). Shares the deterministic
// EIP-1167 codehash above (same impl on every chain).
vm.writeLine(GEN_V4_PATH, "address constant STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM = address(0);");
// Ethereum V4 authoriser clone — same nonce-based `CloneFactory.clone`
// deploy as Base, so its address is non-deterministic and hand-provided
// here (placeholder `address(0)` until the Ethereum bootstrap deploys it
// and the hydrate PR replaces this literal). Shares the deterministic
// EIP-1167 codehash above (same impl on every chain).
vm.writeLine(GEN_V4_PATH, "address constant STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM = address(0);");
vm.writeLine(
GEN_CURRENT_PATH,
"address constant STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM = LibProdDeployV4.STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM;"
);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@script/BuildPointers.sol` around lines 334 - 339, Update genCurrent() to
expose STOX_PROD_AUTHORISER_V4_CLONE_ETHEREUM alongside the existing Base clone
alias, reusing the Ethereum constant emitted by genV4() so LibProdDeployCurrent
stays synchronized with the new pin.

@thedavidmeister

Copy link
Copy Markdown
Contributor

Reviewed 470e8ab: approve

Fresh run 29955854806 leaves one test failure — testDeployTag (0_1_8 != 0_1_16) — plus git-clean. Both inherited main-red; the MU ReceiptVaultAuthoriserMismatch pair from the 09:58 run is gone now that #265 has landed.

The chain-parametric conversion is sound. The script self-guards on Base via V4AuthoriserClonePinAlreadyHydrated, reverts with a typed error on any unpinned chain rather than borrowing another chain's clone, resolves the active chain's Safe through assertActiveChainTokenOwnerSafe, and parameterises the grant map on that Safe so Ethereum cannot be provisioned with Base's Safe in the grantee slots. The workflow passes both choice inputs via env rather than template expansion, and keys concurrency per network so a Base and an Ethereum broadcast cannot race on nonce state.

That "reverts rather than falls back" decision is load-bearing in a way worth recording. Reviewing #258 I found the Ethereum clone address 0x66566cc9… also has code on Base — a different clone, embedding the V3-era authoriser impl 0x2b4a510c…. So a fallback here would not resolve to nothing; it would resolve to a live, wrong authoriser that passes any code.length > 0 check. Only a codehash comparison separates them.

Three review findings, all disposed:

  1. The body documented a LibProdAuthoriserClones that does not exist — rewritten to describe the real activeChainClonePin().
  2. Raw block.chainid == 1 / == 8453 while both sibling selectors use the named constants — fixed in 470e8ab (LibSafeInvariants.ETHEREUM_CHAIN_ID / BASE_CHAIN_ID).
  3. Four independent per-chain dispatch sites with nothing enforcing they agree on the supported-chain set — filed as Four independent per-chain dispatch sites — adding a chain fails at broadcast time, not compile time #272, since it is the subsystem's shape rather than this PR's defect.

Checked and fine: the leftover no-arg expectedGrants() in assertGrantsSliceInvariant only reads .length, and the map's structure is chain-agnostic; BuildPointers emitting ..._V4_CLONE_ETHEREUM keeps src/generated regenerating byte-for-byte.

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