feat: open-salt deterministic clone variant (ICloneableFactoryV4)

[thedavidmeister]

Closes #50

What

CloneFactory gains a second deterministic entry point whose CREATE2 salt is
the caller-supplied salt verbatim, so the clone address is
CREATE2(factory, salt, EIP1167(impl)) — no identity in the derivation:

• cloneDeterministicOpenSalt(address implementation, bytes data, bytes32 salt)
• predictDeterministicAddressOpenSalt(address implementation, bytes32 salt)

cloneDeterministic and predictDeterministicAddress are untouched. Their
msg.sender namespacing is a guarantee other consumers rely on, so this is
purely additive and the two derivations are provably disjoint (a fuzz test
asserts no (implementation, salt, deployer) maps to the same address under
both).

_requireImplementationCode and _initializeClone are reused as-is, so
clone-and-initialize stays atomic and the open variant's failure modes are
identical to the existing one.

Interface versioning

ICloneableFactoryV3 is published, so the new functions go on a new
ICloneableFactoryV4. It extends ICloneableFactoryV3 rather than
restating it: the repo's stated reason for V3 being standalone is that the
non-deterministic clone() was intentionally dropped from V2. Nothing is
dropped here, so inheriting is the shape that matches the documented rule.
CloneFactory now declares is ICloneableFactoryV4; the @inheritdoc tags on
the two pre-existing functions still name V3, because that is where they are
declared.

The footgun, and the docs that are the deliverable

With no msg.sender in the salt, anyone can land on the address with their own
data. Initialization is atomic and runs once, so the first mover sets the
contract's authority permanently and there is no recovery — the address is
occupied and nobody can redeploy over it.

The NatSpec on cloneDeterministicOpenSalt states the qualifying condition as
something a reader can actually check against their own implementation, not as a
vague warning:

for every data any account could pass at a given salt, the resulting
contract must be the contract that was intended

with the concrete consequence spelled out (initialize must not read any
address, key, role, owner or admin out of data; an initialize taking an
owner argument is disqualified by that argument alone), the reason
permissionless deploys are otherwise harmless (a Zoltu deploy has no arguments,
so a front-runner produces byte-for-byte the intended contract), and the
intended pairing that reaches that same position with arguments — an address
registry (rainlanguage/rain.deploy#25) where initialize resolves the admin by
name and the salt commits to that name, so a squatter can neither
substitute a different admin (the registry decides) nor pass a different name (a
different name is a different address). The NatSpec also notes that either half
alone is insufficient.

README.md and CLAUDE.md are updated for the new interface; both were also
still describing CloneFactory as implementing ICloneableFactoryV2, which was
already stale.

Release / deploy

• Consumers bump to soldeer rain-factory 0.1.6. [package].version in
  foundry.toml is the next in-development version and 0.1.5 is the latest on
  the registry, so autopublish on merge publishes exactly 0.1.6.

• Per the package-release.yaml contract ("a version's deploy-pin snapshot is
  built by the PR that changes the bytecode"), this PR runs
  forge script script/BuildPointers.sol and commits
  src/generated/0_1_6/CloneFactory.pointers.sol plus the regenerated
  LibCloneFactoryDeploy. Existing frozen snapshots are untouched (append-only).

• LibCloneFactoryDeployProd.t.sol is red on all five networks —
  CloneFactory not deployed, because 0.1.6's bytecode is not on chain yet.
  0.1.6 deploys to 0x19272bCcFcb032eaC545E74ADFa168fDeD3e8d83, codehash
  0x1a16009998834f07d5ccab032c39377f6528870eec5abd47817f9467187b4012.
  Everything else in rainix-sol / test passes (29/29), and static and
  legal are green.

  This is not fixable on the branch by code, and it is the exact shape that says
  the repo is still on the legacy single-current-pin lifecycle: autopublish
  on merge, plus one LibCloneFactoryDeploy "current" pin that a bytecode
  change immediately invalidates until a deploy catches up. Under the
  deploy/library split, deploys do not block merges at all — a library repo does
  not carry a live-chain pin, and a concrete's per-tag records are frozen and
  tag-released with the deploy as a decoupled manual dispatch. So the right fix
  is migrating this repo to that split, not branch-deploy choreography to paint
  the pin green. I have deliberately not dispatched Manual sol artifacts here.
  A deploy would go green — CREATE2 is idempotent and permissionless, so it is
  cheap either way — but it would be paying the retired choreography rather than
  the standing one. Say which you want.

QA

Category check. Solidity contract change, additive external functions on a
deployed-and-pinned concrete. Categories that apply: address-derivation
correctness, atomicity of clone+initialize, non-regression of the existing
sender-namespacing guarantee, and deploy-pin consistency. Not applicable:
storage layout (the factory is stateless), upgrade/migration (clones are
immutable EIP1167 proxies), access control (the factory is permissionless by
design and that is the subject of the change, not an omission).

Oracle. The spec is issue #50 plus EIP-1167 + CREATE2, not the
implementation. Predicted addresses are asserted against OpenZeppelin's own
Clones.predictDeterministicAddress under an independently constructed salt
(raw salt for the open variant, keccak256(abi.encode(deployer, salt)) for the
namespaced one), so the tests do not simply restate CloneFactory's arithmetic
back to itself. Caller-independence is proven by actually deploying twice from
two different senders with vm.snapshotState()/vm.revertToState() between
them and comparing the two deployed addresses — not by comparing two
predictions, which would only test the prediction function.

Discriminating tests (new, test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol, 11 tests, 2048 fuzz runs each):

test	property
…SaltIsVerbatim	CREATE2 salt is the raw salt — no hashing, nothing mixed in
…MatchesPredict	deployment lands at the prediction; is an EIP1167 proxy of the impl; initialized with data
…CallerIndependent	the point of the variant — two different senders deploying the same (impl, salt) from the same state land on the SAME address
…PredictCallerIndependent	the prediction cannot vary with the caller
…DiffersFromSenderNamespaced	the two derivations never collide, for any (impl, salt, deployer)
…DoesNotConsumeNamespacedSalt	regression guard: taking a salt openly does not block or move the namespaced address for the same salt
…ManyClonesPerImpl	distinct salts, distinct clones
…SecondDeployReverts	a second deploy at a taken open salt reverts Errors.FailedDeployment — it does not silently return the existing clone — and the first clone's state is untouched
…Event	NewClone(sender, impl, child, salt, data) with the raw salt
…InitializeFailureFails	InitializationFailed, and the address is left with zero code (atomicity)
…ZeroImplementationCodeSize	ZeroImplementationCodeSize

Plus testCloneFactory_0_1_6_DeployedBytecodeServesBothEntryPoints in the
tagged-constants suite: the frozen 0.1.6 CREATION_CODE is Zoltu-deployed and
all four entry points are called on the result through the ICloneableFactoryV4
ABI, so a snapshot pinning an address for bytecode that does not serve
cloneDeterministicOpenSalt cannot pass silently. Discrimination verified by
swapping the pinned constant to CLONE_FACTORY_CREATION_CODE_0_1_5 (which has
no open-salt entry point): EvmError: Revert.

The existing CloneFactoryCloneDeterministic.t.sol already pins the namespaced
derivation (testCloneDeterministicSaltIsAbiEncodeHash) and its sender-scoping
(testCloneDeterministicSenderScoped); the mutation run below confirms both
still kill, so they are left as-is rather than duplicated.

Mutations applied, with the killing test. Each mutation was applied to the
working tree, the whole suite run, then the tree restored. Every mutant is
listed with a behavioural killer — the codehash pin (testDeployAddress /
testExpectedCodeHash) also trips on all eight, but that is a bytecode oracle,
not evidence of behavioural coverage, so it is excluded here.

#	mutation	killed by
1	open clone secretly namespaces the salt by msg.sender	…CallerIndependent, …MatchesPredict, …DoesNotConsumeNamespacedSalt, …SecondDeployReverts
2	open predict namespaces the salt	…SaltIsVerbatim, …MatchesPredict, …PredictCallerIndependent, …CallerIndependent
3	open clone drops _requireImplementationCode	…ZeroImplementationCodeSize (reverts, but with no data — the selector assertion discriminates)
4	open clone skips _initializeClone	…MatchesPredict, …Event, …InitializeFailureFails, …SecondDeployReverts
5	initialize return code left unchecked	…InitializeFailureFails + existing testCloneDeterministicInitializeFailureFails
6	cloneDeterministic drops its sender namespacing	existing testCloneDeterministicSenderScoped + new …DoesNotConsumeNamespacedSalt
7	predictDeterministicAddress drops its namespacing	existing testCloneDeterministicSaltIsAbiEncodeHash, testCloneDeterministicSenderScoped, testCloneDeterministicMatchesPredict + new …DiffersFromSenderNamespaced
8	NewClone emits the effective salt instead of the raw salt	…Event + existing testCloneDeterministicEvent

Mutants 6 and 7 are the ones that matter for "do not break the existing
guarantee": the suite fails loudly if the sender-namespacing is ever removed.

CodeRabbit. Two findings, both correct, both fixed in f4635ce and the
threads replied to and resolved: (1) cross-chain address portability is
conditional on the factory AND the implementation being at the same address on
each chain — my README and ICloneableFactoryV4 NatSpec both claimed it
unconditionally, now corrected in both; (2) the entry-point check was a byte
scan of the runtime code, which a selector in constant data passes without being
dispatchable — replaced with the real deploy-and-call test described above.

Gates run locally (rainix sol-shell, the same commands the CI reusables
invoke): forge test — 29 passed, 0 failed (fork-only
LibCloneFactoryDeployProd.t.sol excluded, see Release/deploy above);
forge fmt --check clean; slither . — 13 contracts, 97 detectors, 0 results;
reuse lint compliant; rainix-sol-single-contract clean.

Underspecified, decided rather than guessed

• The NewClone event does not distinguish the two entry points. Reusing
  _initializeClone as-is (as the issue requires) means both variants emit the
  identical NewClone, so an indexer cannot tell from the event alone which
  derivation produced the address. This is harmless in practice because the
  event carries the clone address explicitly, and a separate event would have
  meant not reusing _initializeClone. Documented in ICloneableFactoryV4 and
  asserted in …Event rather than left implicit. Say the word if a distinct
  event is wanted instead.
• Function naming. cloneDeterministicOpenSalt /
  predictDeterministicAddressOpenSalt — long, but consistent suffixes on the
  existing names, and they read at the call site as the variant they are.

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

CloneFactory now implements ICloneableFactoryV4. The new interface adds deterministic clone deployment and address prediction with caller-independent salts. Documentation, behavioral tests, and 0.1.6 deployment-pin validation were updated.

Changes

Open-salt deterministic cloning

Layer / File(s)	Summary
V4 interface and CloneFactory implementation src/interface/ICloneableFactoryV4.sol, src/concrete/CloneFactory.sol, CLAUDE.md, README.md	Adds open-salt deployment and prediction methods. CloneFactory implements V4 and retains V3 caller-namespaced operations. Documentation describes salt derivation and initialization constraints.
Open-salt behavior validation test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol	Tests address prediction, caller independence, salt reuse, event data, duplicate deployment, initialization failure, and zero-code implementations.
0.1.6 deployment pin validation src/lib/LibCloneFactoryDeploy.sol, test/src/lib/LibCloneFactoryDeployTaggedConstants.t.sol	Updates the deployment snapshot to 0.1.6 and validates its bytecode, pinned address, code hash, and entry-point selectors.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant CloneFactory
  participant Implementation
  Caller->>CloneFactory: cloneDeterministicOpenSalt(implementation, data, salt)
  CloneFactory->>Implementation: Deploy clone with verbatim salt
  CloneFactory->>Implementation: Initialize clone with data
  CloneFactory-->>Caller: Return clone address

Loading

Possibly related PRs

• rainlanguage/rain.factory#41: Adds the V3 deterministic factory functionality extended by this PR.
• rainlanguage/rain.factory#43: Updates deployment pins and validates tagged CloneFactory constants.
• rainlanguage/rain.factory#24: Introduces the deployment-pointer updates used by the pinning changes.

Suggested labels: ai:design

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes implement the raw-salt clone and prediction APIs, preserve existing behavior, document risks, and add comprehensive tests.
Out of Scope Changes check	✅ Passed	The implementation, documentation, tests, and deployment artifact updates directly support the stated objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the addition of the open-salt deterministic clone variant and its new interface.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch factory/open-salt-clone

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@README.md`:
- Around line 16-20: Update the cloneDeterministicOpenSalt documentation to make
cross-chain address portability conditional: the same raw salt yields the same
address only when both the factory and implementation addresses match across
chains, since CREATE2 incorporates the factory and the EIP-1167 initialization
code incorporates the implementation. Preserve the existing security guidance
about caller-controlled authority.

In `@test/src/lib/LibCloneFactoryDeployTaggedConstants.t.sol`:
- Around line 123-135: Update _containsSelector so it verifies the selector is
reachable through executable dispatcher logic rather than merely matching bytes
anywhere in code. Decode the runtime dispatcher, or deploy the frozen runtime
and invoke each expected selector with valid arguments, and preserve the test’s
failure behavior when an entry point is unavailable.

🪄 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: 44e6cc07-f166-4b4d-808b-56418e7422a5

📥 Commits

Reviewing files that changed from the base of the PR and between 7f6e150 and dad2d68.

⛔ Files ignored due to path filters (1)

• src/generated/0_1_6/CloneFactory.pointers.sol is excluded by !**/generated/**

📒 Files selected for processing (7)

• CLAUDE.md
• README.md
• src/concrete/CloneFactory.sol
• src/interface/ICloneableFactoryV4.sol
• src/lib/LibCloneFactoryDeploy.sol
• test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol
• test/src/lib/LibCloneFactoryDeployTaggedConstants.t.sol

[thedavidmeister]

🤖 ai:producer
Blocked-on: repo not migrated to the split release lifecycle: the five testProdDeploy* fork tests in test/src/lib/LibCloneFactoryDeployProd.t.sol assert the single current pin LibCloneFactoryDeploy.CLONE_FACTORY_DEPLOYED_ADDRESS has code on Arbitrum, Base, Base Sepolia, Flare and Polygon. This PR changes CloneFactory bytecode, so it necessarily repoints that pin from src/generated/0_1_5 to src/generated/0_1_6; 0.1.6 is generated by this PR and is not on chain anywhere, so rainix-sol / test / test stays red until an out-of-band deploy. There is no code fix on the branch: not bumping the pin would leave the current pin naming bytecode that lacks the new open-salt entry points. Producer deploys nothing (#162). Verified locally in the repo's own toolchain (rainix sol-shell @53e96a7, forge test --no-match-path test/src/lib/LibCloneFactoryDeployProd.t.sol): 29 passed, 0 failed, so the prod pin is the only red.
blocked-by #46