feat(deploy): deterministic-clone toolchain + deterministic V4 authoriser deploy capability (#292 step 1) - #250
feat(deploy): deterministic-clone toolchain + deterministic V4 authoriser deploy capability (#292 step 1)#250thedavidmeister wants to merge 15 commits into
Conversation
…posal, #248) Replace the nondeterministic CloneFactory.clone() V4-authoriser deploy with the deterministic clone primitive (rain-factory 0.1.5 CloneFactory.cloneDeterministic + predictDeterministicAddress). The clone address is now a pure function of (factory, impl, salt, deployer), so: - BuildPointers PINS it up-front: STOX_PROD_AUTHORISER_V4_CLONE is computed via predictDeterministicAddress (was address(0)) instead of backfilled post-deploy. - The deploy script asserts predict == pin BEFORE deploying and that the clone lands exactly there — no post-deploy hydration. Retires #211. - Distinct salts give distinct clones of one impl, so future authorisers get their own salt (why cloneDeterministic, not a salt-free Zoltu-direct clone). The msg.sender-namespaced salt means the pin encodes the deployer (V4_AUTHORISER_CLONE_DEPLOYER = 0x8E4bdeec...), so the prod broadcast must come from that key; the script's predict==pin assert enforces it. Proposal for review (josh + team). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR upgrades Rain dependencies, pins the nonce-based factory, adds deterministic V4 authoriser clone generation and deployment, updates clone fixtures, and adds deploy-key role invariants. ChangesV4 deterministic clone flow
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant DeployScript
participant CloneFactory
participant V4AuthoriserClone
participant Safe
DeployScript->>CloneFactory: validate factory and predict deterministic target
DeployScript->>CloneFactory: deploy clone with pinned implementation and salt
CloneFactory->>V4AuthoriserClone: create clone with Safe as initialAdmin
DeployScript->>V4AuthoriserClone: verify codehash and seven admin roles
V4AuthoriserClone->>Safe: establish automatically granted roles
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
The dep bump removed rain-factory-0.1.1; src contracts + tests still imported ICloneableV2 from it. ICloneableV2/ICLONEABLE_V2_SUCCESS are unchanged across 0.1.1->0.1.5, so this is bytecode-neutral (frozen snapshots unaffected). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The V4 authoriser clone pin is now computed up-front (deterministic), so the comments describing an address(0) placeholder, post-execution pin-PR hydration, and 'placeholder until deployed' no longer describe current behaviour. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thedavidmeister
added
the
ai:needs-work
|
🤖 ai:vetter |
rain-vats 0.1.7 consumes rain-factory 0.1.5, so the whole tree collapses to a single rain-factory version. Drops the rain-factory 0.1.1 pin that rain-vats 0.1.6 dragged in, resolving the two-version conflict that broke the build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
script/BuildPointers.sol (1)
355-359: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCompute the clone codehash instead of hardcoding it.
STOX_PROD_AUTHORISER_V4_CLONE_CODEHASHis hardcoded to0x2089950d3cc1112dd66a58adcfadeadc490b50053ac67be8bc676b4a2dcd1717. This is the EIP-1167 minimal proxy codehash, which is a function of the impl address. IfV4_AUTHORISER_CLONE_IMPLchanges but this codehash isn't updated, the upgrade script'scloneCodehash != LibProdDeployV4.STOX_PROD_AUTHORISER_V4_CLONE_CODEHASHcheck will fail in production.Consider computing it as
keccak256 abi.encodePackedof the EIP-1167 bytecode withV4_AUTHORISER_CLONE_IMPLembedded, so it stays consistent automatically.🤖 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 355 - 359, Replace the hardcoded STOX_PROD_AUTHORISER_V4_CLONE_CODEHASH generation in BuildPointers with a computed keccak256 hash of the EIP-1167 minimal-proxy bytecode embedding V4_AUTHORISER_CLONE_IMPL, using abi.encodePacked so the generated value stays synchronized when the implementation address changes.
🤖 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 `@remappings.txt`:
- Around line 7-14: Remove the unused rain-factory-0.1.1 remapping, and resolve
the rain-vats version mismatch by aligning the remapping and foundry.toml
dependency with the rain-vats-0.1.6 imports still used throughout src, script,
and test; keep the imports and declared dependency consistent.
In `@script/BuildPointers.sol`:
- Line 159: Update V4_AUTHORISER_CLONE_DEPLOYER to reference the existing
GEN_OWNER constant instead of duplicating its address, preserving the current
value while keeping both symbols synchronized.
- Line 157: Update BuildPointers and the buildContractPointers flow so
V4_AUTHORISER_CLONE_IMPL is validated against, or generated from,
LibProdDeployV4.STOX_OFFCHAIN_ASSET_RECEIPT_VAULT_AUTHORIZER_V1_0_1_1, ensuring
the hardcoded clone implementation cannot diverge from the generated pointer
address.
---
Outside diff comments:
In `@script/BuildPointers.sol`:
- Around line 355-359: Replace the hardcoded
STOX_PROD_AUTHORISER_V4_CLONE_CODEHASH generation in BuildPointers with a
computed keccak256 hash of the EIP-1167 minimal-proxy bytecode embedding
V4_AUTHORISER_CLONE_IMPL, using abi.encodePacked so the generated value stays
synchronized when the implementation address changes.
🪄 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: 1e64f5fb-26b0-45ed-a3e6-af6cb7184d0f
⛔ Files ignored due to path filters (2)
soldeer.lockis excluded by!**/*.locksrc/generated/LibProdDeployV4.solis excluded by!**/generated/**
📒 Files selected for processing (17)
foundry.tomlremappings.txtscript/20260619-deploy-v4-authoriser-clone.s.solscript/20260623-upgrade-receipt-vaults-to-v4.s.solscript/BuildPointers.solsrc/concrete/StoxWrappedTokenVault.solsrc/concrete/authorize/StoxOffchainAssetReceiptVaultAuthorizerV1.solsrc/concrete/deploy/StoxWrappedTokenVaultBeaconSetDeployer.soltest/script/20260619-deploy-v4-authoriser-clone.t.soltest/src/concrete/StoxCorporateActionsFacet.authorizerIntegration.t.soltest/src/concrete/StoxReceiptVault.setAuthorizerGuard.t.soltest/src/concrete/StoxWrappedTokenVault.t.soltest/src/concrete/StoxWrappedTokenVaultV1.prod.base.t.soltest/src/concrete/authorize/StoxOffchainAssetReceiptVaultAuthorizerV1.initializeGuard.t.soltest/src/concrete/authorize/StoxOffchainAssetReceiptVaultPaymentMintAuthorizerV1.corporateActionPairingGap.t.soltest/src/concrete/integration/OrchestratorIntegrationTest.soltest/src/concrete/upgrade/V3UpgradeShadowFork.t.sol
… cloneDeterministic rain-vats 0.1.7 (on rain-factory 0.1.5) replaces 0.1.6 across all src/test/script imports. rain-factory 0.1.5 is deterministic-only (no clone()), so the test fixtures that spun up authorizer/vault instances via CloneFactory.clone() migrate to cloneDeterministic(impl, data, bytes32(0)) — each uses a fresh single-use factory, so bytes32(0) cannot collide. V3UpgradeShadowFork clones via the deployed factory cast to ICloneableFactoryV3. Bytecode-neutral: deploy addresses/codehashes unchanged, generated pointer libs regenerate identically. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🤖 ai:vetter |
|
🤖 ai:producer |
|
human:reject on the close-candidate — rework ordered. #292 makes deterministic clones an explicit goal, which un-moots this PR's premise but rescopes it to #292 step 1: rain-factory 0.1.5 bump + clone()→cloneDeterministic fixture migration. Pin stays the live 0x315b16… clone in this PR; the deterministic pin flip lands with the Base migration script (#292 step 3). |
thedavidmeister
removed
ai:needs-work
Conflict resolution takes main wholesale for the deploy surface — BuildPointers.sol, LibProdDeployV4.sol, the 20260619 clone script and its test, V3UpgradeShadowFork — so every deployed pin (Base clone 0x315b16…, Ethereum clone, all impl codehashes) stays exactly as main pinned them. The up-front 0x9746… deterministic pin and the cloneDeterministic rewrite of the executed clone script are dropped; #292 owns that as steps 2-3. What survives on top of main: - rain-factory 0.1.5 + rain-vats 0.1.7, import prefixes repointed repo-wide (including main's post-June scripts/tests still on 0.1.6/0.1.1). - src/lib/LibNonceCloneFactory.sol: pins the nonce-based CloneFactory (0x444acC29…) the live clones were deployed through; rain-factory 0.1.5's LibCloneFactoryDeploy pins the deterministic-only factory which has no clone(address,bytes), so the clone script keeps targeting the factory it always used. - Unit fixtures migrated clone() -> cloneDeterministic (0.1.5 concrete is deterministic-only); fork tests still exercise the live nonce factory. - src/generated/candidate/ regenerated via BuildPointers + forge fmt; frozen 0_1_1/ and LibProdDeployV4 deployed pins byte-identical to main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thedavidmeister
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
script/20260722-deploy-missing-tokens-ethereum.s.sol (2)
129-143: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject partially hydrated token rows.
entryClearis true only when all three addresses are zero. A row with one or two non-zero addresses is skipped. If every row is partial,_selectMissing()revertsNoMissingTokens()even though the table is not fully hydrated.Accept only all-zero or all-non-zero rows. Revert on a partial row.
Proposed guard
+ bool entryHydrated = table[i].receipt != address(0) && table[i].receiptVault != address(0) + && table[i].wrappedTokenVault != address(0); bool entryClear = table[i].receipt == address(0) && table[i].receiptVault == address(0) && table[i].wrappedTokenVault == address(0); - if (!entryClear) { + if (!entryClear && !entryHydrated) { + revert TokenTablePartiallyHydrated(i); + } + if (entryHydrated) { continue; }Declare
TokenTablePartiallyHydrated(uint256 index)with the existing errors.🤖 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/20260722-deploy-missing-tokens-ethereum.s.sol` around lines 129 - 143, Update the row-selection logic in _selectMissing so each token table row is classified as either fully empty or fully hydrated: if any of receipt, receiptVault, or wrappedTokenVault is non-zero while not all are non-zero, revert with a newly declared TokenTablePartiallyHydrated(uint256 index) error; continue selecting only all-zero rows and preserve the existing NoMissingTokens behavior when none qualify.
210-214: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winVerify the authorizer and ownership handoff state before logging success.
The script still needs the same readback check used by
20260706-deploy-tokens-ethereum.s.solaftersetAuthorizerandtransferOwnership. CallassertHandoffLanded(receiptVault, authoriser, safe)before logging the tuple, and add or share the helper and matching errors.🤖 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/20260722-deploy-missing-tokens-ethereum.s.sol` around lines 210 - 214, After the setAuthorizer and transferOwnership calls, invoke assertHandoffLanded(receiptVault, authoriser, safe) before logging the success tuple. Add or reuse the matching helper and validation errors from 20260706-deploy-tokens-ethereum.s.sol so the authorizer and ownership readbacks are verified consistently.script/20260706-deploy-tokens-ethereum.s.sol (1)
140-148: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake the Ethereum deployment scripts resume-safe.
vm.startBroadcast()is not atomic, and broadcast mode does not roll back earlier successful calls when a later external call reverts. If a partial run leaves deployed tokens whileproductionTokensEthereum()is still unhydrated, a rerun can treat them as already deployed or still missing and deploy duplicate production tokens.
script/20260706-deploy-tokens-ethereum.s.sol#L140-L148: use durable resume state or on-chain detection instead of the static input pin table as the run-once guard.script/20260706-deploy-tokens-ethereum.s.sol#L245-L271: make each iteration durable before the next, or require explicit reconciliation/partial-run handling, and add a failure-after-first-iteration test.script/20260722-deploy-missing-tokens-ethereum.s.sol#L115-L118: do not use only the unhydrated pin table to decide whether token configs are missing.script/20260722-deploy-missing-tokens-ethereum.s.sol#L168-L189: skip/reconcile already deployed tokens or abort when partial state is detected, and add a failure-after-first-iteration test.🤖 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/20260706-deploy-tokens-ethereum.s.sol` around lines 140 - 148, Replace the static pin-table-based guard in _assertNotAlreadyDeployed() at script/20260706-deploy-tokens-ethereum.s.sol#L140-L148 with durable resume state or on-chain detection that survives partial runs, so partial deployments cannot be mistaken as completed runs. In the deployment loop at script/20260706-deploy-tokens-ethereum.s.sol#L245-L271, make each iteration durable before advancing to the next iteration, add explicit reconciliation or partial-run handling, and add a test that verifies correct behavior when a failure occurs after the first iteration completes. In script/20260722-deploy-missing-tokens-ethereum.s.sol#L115-L118, replace the check that relies only on the unhydrated pin table with on-chain detection to determine whether token configs are missing. In the loop at script/20260722-deploy-missing-tokens-ethereum.s.sol#L168-L189, skip already-deployed tokens by detecting them on-chain, abort when partial deployment state is detected, and add a test that validates correct behavior when a failure occurs after the first iteration.script/20260623-upgrade-receipt-vaults-to-v4.s.sol (1)
132-161: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftDo not reference
LibProdDeployV1from this production script.
UpgradeReceiptVaultsToV4reads live beacon addresses and rollback implementations fromLibProdDeployV1inBEACON,V1_IMPL,beacons(), andv1Impls(). This couples the migration to an obsolete deployment library and violates the repository versioning rule.Expose the required historical V1 values through the current versioned deployment surface or a current-version migration library. Do not replace them with V4 addresses.
As per coding guidelines, production source contracts must use the current versioned
LibProdDeploy*library and must not reference obsolete deployment versions such asLibProdDeployV1.🤖 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/20260623-upgrade-receipt-vaults-to-v4.s.sol` around lines 132 - 161, Remove all LibProdDeployV1 references from UpgradeReceiptVaultsToV4, including BEACON, V1_IMPL, beacons(), and v1Impls(). Re-source the required historical V1 beacon and implementation addresses through the current versioned deployment surface or a current-version migration library, preserving their existing values and index alignment; do not substitute V4 addresses.Source: Coding guidelines
🤖 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 `@script/20260623-upgrade-receipt-vaults-to-v4.s.sol`:
- Around line 181-183: Update the documentation comment for V4_AUTHORISER_CLONE
to avoid claiming the existing clone was deployed deterministically; describe it
as pinned in LibProdDeployV4 or precomputed by BuildPointers, while preserving
the deployed nonce-based clone reference.
In `@src/lib/LibNonceCloneFactory.sol`:
- Line 3: Update the pragma declaration in LibNonceCloneFactory to pin the
compiler exactly to Solidity 0.8.25 by replacing the caret version constraint
with an exact version constraint. Keep the existing pragma location and
formatting unchanged.
---
Outside diff comments:
In `@script/20260623-upgrade-receipt-vaults-to-v4.s.sol`:
- Around line 132-161: Remove all LibProdDeployV1 references from
UpgradeReceiptVaultsToV4, including BEACON, V1_IMPL, beacons(), and v1Impls().
Re-source the required historical V1 beacon and implementation addresses through
the current versioned deployment surface or a current-version migration library,
preserving their existing values and index alignment; do not substitute V4
addresses.
In `@script/20260706-deploy-tokens-ethereum.s.sol`:
- Around line 140-148: Replace the static pin-table-based guard in
_assertNotAlreadyDeployed() at
script/20260706-deploy-tokens-ethereum.s.sol#L140-L148 with durable resume state
or on-chain detection that survives partial runs, so partial deployments cannot
be mistaken as completed runs. In the deployment loop at
script/20260706-deploy-tokens-ethereum.s.sol#L245-L271, make each iteration
durable before advancing to the next iteration, add explicit reconciliation or
partial-run handling, and add a test that verifies correct behavior when a
failure occurs after the first iteration completes. In
script/20260722-deploy-missing-tokens-ethereum.s.sol#L115-L118, replace the
check that relies only on the unhydrated pin table with on-chain detection to
determine whether token configs are missing. In the loop at
script/20260722-deploy-missing-tokens-ethereum.s.sol#L168-L189, skip
already-deployed tokens by detecting them on-chain, abort when partial
deployment state is detected, and add a test that validates correct behavior
when a failure occurs after the first iteration.
In `@script/20260722-deploy-missing-tokens-ethereum.s.sol`:
- Around line 129-143: Update the row-selection logic in _selectMissing so each
token table row is classified as either fully empty or fully hydrated: if any of
receipt, receiptVault, or wrappedTokenVault is non-zero while not all are
non-zero, revert with a newly declared TokenTablePartiallyHydrated(uint256
index) error; continue selecting only all-zero rows and preserve the existing
NoMissingTokens behavior when none qualify.
- Around line 210-214: After the setAuthorizer and transferOwnership calls,
invoke assertHandoffLanded(receiptVault, authoriser, safe) before logging the
success tuple. Add or reuse the matching helper and validation errors from
20260706-deploy-tokens-ethereum.s.sol so the authorizer and ownership readbacks
are verified consistently.
🪄 Autofix
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 Plus
Run ID: 7ab59688-a6ea-48d3-b8c7-acb75921dd1b
📒 Files selected for processing (47)
foundry.tomlremappings.txtscript/20260619-deploy-v4-authoriser-clone.s.solscript/20260623-upgrade-receipt-vaults-to-v4.s.solscript/20260706-deploy-tokens-ethereum.s.solscript/20260722-deploy-missing-tokens-ethereum.s.solscript/20260722-swap-remaining-vault-authorisers.s.solscript/20260722-swap-rklb-authoriser.s.solsrc/concrete/ST0xOrchestrator.solsrc/concrete/StoxCorporateActionsFacet.solsrc/concrete/StoxReceipt.solsrc/concrete/StoxReceiptVault.solsrc/concrete/authorize/StoxOffchainAssetReceiptVaultAuthorizerV1.solsrc/concrete/authorize/StoxOffchainAssetReceiptVaultPaymentMintAuthorizerV1.solsrc/concrete/deploy/StoxOffchainAssetReceiptVaultBeaconSetDeployer.solsrc/concrete/deploy/StoxUnifiedDeployer.solsrc/interface/IStoxUnifiedDeployerV1.solsrc/lib/LibNonceCloneFactory.soltest/script/20260619-deploy-v4-authoriser-clone.t.soltest/script/20260623-upgrade-receipt-vaults-to-v4.t.soltest/script/20260722-swap-remaining-vault-authorisers.t.soltest/script/20260722-swap-rklb-authoriser.t.soltest/src/concrete/DelegatecallHarness.soltest/src/concrete/MockAuthorizer.soltest/src/concrete/MockVault.soltest/src/concrete/PermissiveAuthorizer.soltest/src/concrete/ST0xOrchestrator.t.soltest/src/concrete/StoxCorporateActionsFacet.authorizerIntegration.t.soltest/src/concrete/StoxCorporateActionsFacet.t.soltest/src/concrete/StoxReceiptRebaseIntegrationTest.t.soltest/src/concrete/StoxReceiptVault.setAuthorizerGuard.t.soltest/src/concrete/StoxReceiptVaultFallbackRouting.t.soltest/src/concrete/StoxWrappedTokenVault.t.soltest/src/concrete/authorize/FailingSuperInitAuthorizer.soltest/src/concrete/authorize/StoxOffchainAssetReceiptVaultAuthorizerV1.initializeGuard.t.soltest/src/concrete/authorize/StoxOffchainAssetReceiptVaultPaymentMintAuthorizerV1.corporateActionPairingGap.t.soltest/src/concrete/deploy/StoxProdV2.t.soltest/src/concrete/deploy/StoxProdV4.t.soltest/src/concrete/deploy/StoxUnifiedDeployer.newTokenAndWrapperVault.t.soltest/src/concrete/deploy/StoxUnifiedDeployer.prod.base.t.soltest/src/concrete/deploy/StoxUnifiedDeployer.t.soltest/src/concrete/integration/BurnInfoAuditTrail.t.soltest/src/concrete/integration/CertificationLapse.t.soltest/src/concrete/integration/MintWithEcdsaSignature.t.soltest/src/concrete/integration/OrchestratorIntegrationTest.soltest/src/concrete/upgrade/V3UpgradeShadowFork.t.soltest/src/lib/LibTokenInvariants.addresses.t.sol
💤 Files with no reviewable changes (1)
- remappings.txt
…ehash A codehash pin alone cannot recreate historical behavior once compiler or optimizer drift makes the source unable to reproduce it; the embedded runtime code can. CLONE_FACTORY_DEPLOYED_CODE is the live Base bytecode of 0x444acC29..., asserted in the 20260619 suite as CLONE_FACTORY_DEPLOYED_CODEHASH == keccak256(CLONE_FACTORY_DEPLOYED_CODE), the same shape the prod pins assert. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both the branch's 'pinned deterministically up-front' and main's 'placeholder until deployed' wording predate the deployed, hydrated pin. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…in + dated script BuildPointers now emits the chain-invariant migration target (#292) into LibProdDeployV4: the CREATE2 prediction against the 0.1.5 CloneFactory (0x9746648e...) with its salt, deployer, EIP-1167 runtime bytecode and codehash (#293 pin shape). Live pins are untouched and stay authoritative until the migration. script/20260804-deploy-deterministic-v4-authoriser-clone.s.sol broadcasts the clone at the target on any configured chain: pre-flights factory/impl codehashes and the prediction, no-ops when the pinned clone already occupies the target, initialises with the chain's token-owner Safe as initialAdmin so the deployer never holds a role, and post-asserts landing address, code shape, and the Safe's seven admin roles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thedavidmeister
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@script/20260804-deploy-deterministic-v4-authoriser-clone.s.sol`:
- Around line 119-129: Validate the idempotent clone’s initialization state by
calling assertPostState(target, safe) after the matching codehash check and
before the no-op return in
script/20260804-deploy-deterministic-v4-authoriser-clone.s.sol (lines 119-129).
Update test/script/20260804-deploy-deterministic-v4-authoriser-clone.t.sol
(lines 132-141) so the no-op idempotency test uses an initialized clone rather
than etching only the runtime bytecode.
🪄 Autofix
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 Plus
Run ID: 74db6ab8-9f11-4787-99ef-1e13c7550454
⛔ Files ignored due to path filters (1)
src/generated/LibProdDeployV4.solis excluded by!**/generated/**
📒 Files selected for processing (6)
script/20260623-upgrade-receipt-vaults-to-v4.s.solscript/20260804-deploy-deterministic-v4-authoriser-clone.s.solscript/BuildPointers.solsrc/lib/LibNonceCloneFactory.soltest/script/20260619-deploy-v4-authoriser-clone.t.soltest/script/20260804-deploy-deterministic-v4-authoriser-clone.t.sol
….skip The codehash cannot see initialization, so a same-address clone could have been initialized with different admin data — the no-op path now runs the same post-state assertion as a fresh deploy. The static gate also bans vm.skip: the fork tests now assert the deployed-world branch instead of skipping it, and an etched-but-uninitialized clone is a typed revert. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e retired EOA 0x8E4bdeec... is the retired beacon-owner EOA. The CI deploy key resolves to 0xE8c6eDE2..., the sender that created the live Base clone via the nonce factory (tx 0x26519d1c9090e6236cbd6e9c7f5d6eee7cf633da3a6653742914 b3c17fe7d236). The factory namespaces CREATE2 salts by sender, so the migration target moves to 0xE322d7D0d366cE0bddfACF6D25d11DdA1B13A86b. A regression guard ties the pin to the creation-tx sender and rejects the retired EOA. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sertion The deploy key is a hot wallet: it deploys and holds no role, ever. New LibAuthoriserInvariants.assertKeysHoldNoRoles iterates the pinned role universe (13 expectedGrants ids + DEFAULT_ADMIN_ROLE) and reverts DeployKeyHoldsRole if the hot deploy key (the generated deterministic deployer pin) or the retired deploy EOA (BEACON_INITIAL_OWNER) holds any of them. Composed into assertExpectedGrants so every per-chain prod fork assertion enforces it, and called from the deterministic clone deploy's post-state so it binds at broadcast time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thedavidmeister
added
the
ai:needs-work
|
🤖 ai:vetter |
|
🤖 ai:producer |
The pre-flight description still said the clone-pin check fails because STOX_PROD_AUTHORISER_V4_CLONE is address(0). The pin names a deployed clone that carries the pinned EIP-1167 codehash and the full grant map, so both the impl- and clone-side pre-flights pass. The sibling blocks in this file already describe that state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two claims in this file described pins as unhydrated. The codehash pin is a real value, as the deploy log's own comment a few lines up already says, and both chains the clone-pin selector serves return a hydrated pin — every other chain reverts. `_assertPostState` still re-derives the expected codehash from the impl address, which is what keeps that check independent of the pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/script/20260804-deploy-deterministic-v4-authoriser-clone.t.sol`:
- Around line 178-193: Update testRunRejectsUninitializedCloneAtTarget so its
etched target cannot inherit forked Safe role state: mock the first Safe
hasRole(...) call to return false, or clear the relevant AccessControl storage
before invoking script.run(). Preserve the expected SafeMissingAdminRole revert
assertion.
🪄 Autofix
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 Plus
Run ID: a484c79f-d1e1-4a08-845e-28875e9d53a0
⛔ Files ignored due to path filters (1)
src/generated/LibProdDeployV4.solis excluded by!**/generated/**
📒 Files selected for processing (7)
script/20260623-upgrade-receipt-vaults-to-v4.s.solscript/20260804-deploy-deterministic-v4-authoriser-clone.s.solscript/BuildPointers.solsrc/lib/LibAuthoriserInvariants.soltest/script/20260804-deploy-deterministic-v4-authoriser-clone.t.soltest/src/lib/LibAuthoriserInvariants.t.soltest/src/lib/LibAuthoriserInvariantsHarness.sol
…hydrated vm.etch replaces code but preserves storage, so after the real broadcast lands the etched clone at the deterministic target inherits the live initialized grants and run() correctly takes the idempotent path — the uninitialized shape this test fabricates only exists while the target is empty. Guard with the same tolerant no-live-code pattern the sibling tests use; the hydrated target's post-state stays covered by testRunNoOpsWhenTargetAlreadyHydrated and testCloneFromPinnedDeployerLandsOnPin. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Reviewed 9f774c8: APPROVE Delta since 99b8760: |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/script/20260804-deploy-deterministic-v4-authoriser-clone.t.sol (1)
272-285: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the factory codehash on Ethereum.
When the factory exists, this branch trusts any contract that implements
predictDeterministicAddress. It does not comparefactory.codehashwithLibCloneFactoryDeploy.CLONE_FACTORY_DEPLOYED_CODEHASH. A compatible but incorrect factory can satisfy the prediction while the deployment script rejects it.Proposed assertion
} else { + assertEq( + factory.codehash, + LibCloneFactoryDeploy.CLONE_FACTORY_DEPLOYED_CODEHASH, + "factory codehash drift on Ethereum" + ); address predicted = ICloneableFactoryV3(factory)🤖 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 `@test/script/20260804-deploy-deterministic-v4-authoriser-clone.t.sol` around lines 272 - 285, In the factory-present branch of the test, add an assertion that factory.codehash equals LibCloneFactoryDeploy.CLONE_FACTORY_DEPLOYED_CODEHASH before trusting predictDeterministicAddress. Keep the existing prediction and pinned-address assertion unchanged.
🤖 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.
Outside diff comments:
In `@test/script/20260804-deploy-deterministic-v4-authoriser-clone.t.sol`:
- Around line 272-285: In the factory-present branch of the test, add an
assertion that factory.codehash equals
LibCloneFactoryDeploy.CLONE_FACTORY_DEPLOYED_CODEHASH before trusting
predictDeterministicAddress. Keep the existing prediction and pinned-address
assertion unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bffb95d4-93d5-4c0a-92fa-39e72fbc0f55
📒 Files selected for processing (2)
script/20260619-deploy-v4-authoriser-clone.s.soltest/script/20260804-deploy-deterministic-v4-authoriser-clone.t.sol
Scope addition: the admin and the salt both need to change here, not afterThis PR pins That is the same failure this PR is already working around — the body notes the pin moved off Why this belongs in this PR rather than a follow-up: the fix changes the address derivation. Land this as-is and deploy The changeTwo halves, and neither is safe alone:
Open salt alone reopens squatting — anyone can land on the address with their own Blocked on
Tracking issue with the full rationale: #295 — this PR should close it. The live pins ( |
|
🤖 ai:vetter |
2 participants
Step 1 of #292 plus the deterministic deploy capability: everything needed to deploy the deterministic V4 authoriser clones on all configured chains post-merge. The only follow-up left is the migration itself (grant mirroring,
setAuthorizer, live-pin flip — #292 step 3).What this PR contains
Toolchain — rain-factory 0.1.1→0.1.5 (deterministic-only
CloneFactory), rain-vats 0.1.6→0.1.7 (0.1.6 pins rain-factory 0.1.1 internally; 0.1.7 keeps a single factory in the tree). Import prefixes repointed repo-wide; unit fixtures migratedclone()→cloneDeterministic().src/lib/LibNonceCloneFactory.sol— pins the nonce-basedCloneFactory(0x444acC29…) every LIVE clone was deployed through: address + codehash + full runtime bytecode (#293 pin shape), keccak-asserted in the 20260619 suite. The executed 20260619 script keeps targeting it unchanged.Migration target pin (
BuildPointers-generated intoLibProdDeployV4):STOX_PROD_AUTHORISER_V4_CLONE_DETERMINISTIC = 0xE322d7D0d366cE0bddfACF6D25d11DdA1B13A86b— CREATE2 prediction against the 0.1.5 factory, chain-invariant (Zoltu factory + Zoltu impl + pinned salt/deployer). Moves off the original proposal's0x9746…, which was predicted for the retired0x8E4bdeec…EOA.…_SALT(bytes32(0)),…_DEPLOYER(0xE8c6eDE2…, the EOA the CI deploy key resolves to — the creator of the live Base clone via the nonce factory, tx0x26519d1c…; the factory namespaces salts by sender),…_CODE(EIP-1167 runtime bytes) and…_CODEHASH(Hash-only and address-only pins across the repo: embed runtime bytecode on every deployed-artifact pin (ruling from #250) #293 shape).STOX_PROD_AUTHORISER_V4_CLONE = 0x315b16…, the Ethereum clone) are byte-identical to main and stay authoritative until the migration.script/20260804-deploy-deterministic-v4-authoriser-clone.s.sol— per-chain broadcast (Base + Ethereum config on main; HyperEVM arrives with its own wave):predictDeterministicAddressequals the pin; idempotent — no-op when the pinned clone already occupies the target, typed revert if foreign code does.cloneDeterministic(impl, abi.encode({initialAdmin: tokenOwnerSafe}), salt)— admin lands directly on the Safe, the deployer never holds a role, no renounce ceremony._ADMINroles.Tests (
test/script/20260804-…t.sol, 11 tests): offline CREATE2 re-derivation of the pin, EIP-1167 bytecode re-derivation, keccak(code)==codehash, live-Base-fork prediction check, a real-factory fork deploy from the pinned deployer landing exactly on the pin, idempotent no-op, and typed-revert coverage for foreign-code/missing-factory/codehash-drift pre-flights — tolerant of the not-yet-deployed target ("no live code EXPECTED" pattern).Dropped from the original diff
The up-front flip of the LIVE pin to the predicted address (contradicts the deployed clone) and the rewrite of the executed 20260619 script — the deterministic deploy now lives in its own dated script above.
🤖 Generated with Claude Code
QA
testTargetPinRederivesOffline,testTargetPinCodeRederives,testTargetPinCodehashMatchesCode,testDeployerPinIsTheLiveCloneCreator,testBaseLiveFactoryPredictsPin,testCloneFromPinnedDeployerLandsOnPin,testRunNoOpsWhenTargetAlreadyHydrated,testRunRejectsUninitializedCloneAtTarget,testRunRejectsForeignCodeAtTarget,testRunRejectsMissingFactory,testRunRejectsFactoryCodehashDrift,testRunRejectsImplCodehashDrift,testEthereumFactoryGate(all 13 intest/script/20260804-deploy-deterministic-v4-authoriser-clone.t.sol), plustestNonceCloneFactoryCodehashMatchesEmbeddedCode,testAssertKeysHoldNoRolesPassesLive,testAssertKeysHoldNoRolesRejectsHotKeyRole,testAssertKeysHoldNoRolesRejectsRetiredKeyDefaultAdmin— each fails on base, verified STRUCTURALLY rather than by running the base suite:git diff --name-status origin/main...HEADshowsscript/20260804-deploy-deterministic-v4-authoriser-clone.s.sol,src/lib/LibNonceCloneFactory.soland the 20260804 test file areAdded by this PR, andLibAuthoriserInvariants.assertKeysHoldNoRolesdoes not exist onorigin/main, so on base none of these tests have a subject to compile against. All 17 pass at the head in CI (rainix-sol / test / test, which runs the Base-fork suite).forgemutation pass was run, and the reason is a toolchain one, not a coverage one —pr-review-report sol-toolchainreturnsmode: conflict(exit 3) for this checkout, naming two gating shells (the rainix reusable@mainandgithub:rainlanguage/rainix#sol-shellviagit-clean.yaml), so no single localforgeis the one CI judges with and picking either would measure the wrong toolchain. In its place the deploy-critical derivation was mutated OFFLINE, one input at a time, against the independent keccak in the Oracle line, and each mutant re-predicted: deployer0xE8c6eDE2…→retired0x8E4bdeec…⇒0x9746648ef5…; saltbytes32(0)→bytes32(1)⇒0x898899433b…; factory0x7053c1D0…(0.1.5)→0x444acC29…(nonce) ⇒0xc3df700b63…; impl0x2ea0d35d…→0x315b16faa6…⇒0xff53cd7a92…; drop the sender-namespacing of the salt ⇒0x0dfaca23c2…. Every mutant moves the address off the pin0xE322d7D0…, sotestTargetPinRederivesOfflineandtestBaseLiveFactoryPredictsPinare discriminating on each of the four inputs and on the namespacing itself. The deployer mutant reproduces0x9746…exactly — the original proposal's address, predicted for the retired EOA — which is the drift the deployer correction in this PR fixes.keccak256("")=0xc5d2460186f7…andkeccak256("abc")=0x4e03657aea45…, re-derived from first principles: the EIP-1167 runtime for impl0x2ea0d35d…(45 bytes, byte-equal to the pinned…_DETERMINISTIC_CODE);keccak256(…_DETERMINISTIC_CODE)=0x2089950d3cc1112dd66a58adcfadeadc490b50053ac67be8bc676b4a2dcd1717=…_DETERMINISTIC_CODEHASH; the effective saltkeccak256(abi.encode(deployer, bytes32(0)))=0x55e90699ce…using the formula read out of the rain-factory 0.1.5 soldeer artifact itself (src/concrete/CloneFactory.sol:48_effectiveSalt, downloaded from the URL insoldeer.lock, not read from this repo); the CREATE2 address0xe322d7d0d366ce0bddfacf6d25d11dda1b13a86b= the pinnedSTOX_PROD_AUTHORISER_V4_CLONE_DETERMINISTIC; andkeccak256(CLONE_FACTORY_DEPLOYED_CODE)over the embedded 1012 bytes =0xf21b813c7075a1621285df3a8369d0652c31ea80cb807be1aaadafeecd134475=CLONE_FACTORY_DEPLOYED_CODEHASH. (b) Live chain — raweth_getCode/eth_callover public RPCs, no fork harness: Base0x444acC29…runtime is BYTE-IDENTICAL to the embeddedCLONE_FACTORY_DEPLOYED_CODE(1012 bytes); Base0x7053c1D0…(0.1.5 factory) is live with codehash0x3ec02e2f27d73f603f5b37a7dc228956004e9154c6d1f9c5335bd60036379d00, equal to that artifact's ownBYTECODE_HASH, and is ABSENT on Ethereum (0xon three independent Ethereum RPCs — publicnode, drpc, mevblocker), so the script's typed Ethereum fail-safe describes a real condition; the deterministic target0xE322d7D0…is empty on Base and on Ethereum; the V4 authoriser impl0x2ea0d35d…is present on BOTH chains at 5958 bytes, which is what makes the predicted address chain-invariant; the liveSTOX_PROD_AUTHORISER_V4_CLONE0x315b16faa6…carries codehash0x2089950d3c…, equal toSTOX_PROD_AUTHORISER_V4_CLONE_CODEHASH; and a batchedhasRolesweep of that live clone shows the token-owner Safe holding all 10 distinct pinned roles (the positive control that proves the call encoding) while the hot deploy key0xE8c6eDE2…and the retired key0x8E4bdeec…hold 0 of 22 (role, key) pairs — the "deploy keys hold NOTHING" invariant, measured rather than asserted.closingIssuesReferencesis EMPTY — this PR closes no issue; it is step 1 of 2 of Deterministic authoriser clones everywhere: chain-invariant address, pinned up-front, migrate Base off the nonce-based clone #292, which stays open for step 2. Deterministic authoriser clones everywhere: chain-invariant address, pinned up-front, migrate Base off the nonce-based clone #292's step-1 list asks for six things and this covers all six: rain-factory 0.1.5 + forced rain-vats 0.1.7 bump (foundry.toml"rain-factory" = "0.1.5","rain-vats" = "0.1.7", prefixes repointed inremappings.txt);clone()→cloneDeterministicfixture migration (58 files changed with 0 file deletions in the diff — fixtures migrated, not dropped); a bytecode-complete nonce-factory pin per Hash-only and address-only pins across the repo: embed runtime bytecode on every deployed-artifact pin (ruling from #250) #293 (src/lib/LibNonceCloneFactory.sol: address + codehash + full 1012-byte runtime, keccak-asserted); the datedcloneDeterministicdeploy script for every chain carrying prod config (script/20260804-deploy-deterministic-v4-authoriser-clone.s.sol, Base + Ethereum); the predicted address pinned in the Hash-only and address-only pins across the repo: embed runtime bytecode on every deployed-artifact pin (ruling from #250) #293 shape with offline predicted==pinned and keccak asserts (the five…_DETERMINISTIC*constants inLibProdDeployV4plus the 13 tests above); and live pins untouched (theLibProdDeployV4diff againstorigin/mainis additions only —STOX_PROD_AUTHORISER_V4_CLONEand…_ETHEREUMare byte-identical to main). Deliberately NOT covered, because Deterministic authoriser clones everywhere: chain-invariant address, pinned up-front, migrate Base off the nonce-based clone #292 scopes them elsewhere: step 2 (grant replay fromRoleGranted/RoleRevokedlogs, balance sweep, per-vaultsetAuthorizerbundles, pin flip) and the HyperEVM wave (feat(multichain): HyperEVM V4 authoriser clone pin plumbing (RAI-1511) #274/ops(script): HyperEVM beacon-owner migration + 0.1.1 beacon lib generalisation (RAI-1511) #276/ops(script): HyperEVM token deploy (RAI-1511) #278), which deploys deterministically from the start and needs no migration.Summary by CodeRabbit