feat: open-salt deterministic clone variant (ICloneableFactoryV4) - #8
Open
thedavidmeister wants to merge 3 commits into
Open
feat: open-salt deterministic clone variant (ICloneableFactoryV4)#8thedavidmeister wants to merge 3 commits into
thedavidmeister wants to merge 3 commits into
Conversation
The deploy half of rainlanguage/rain.factory#51, which was written against the pre-split rain.factory tree. Five of that PR's eight files targeted paths rain.factory#47 deleted; they land here instead. 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)) with 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. The rain-factory Soldeer pin moves 0.1.5 -> 0.1.7 and every versioned import prefix moves with it, because ICloneableFactoryV4 only exists from 0.1.7. That version is not published until rain.factory#51 merges, so this branch cannot resolve its dependencies until then. No hand-authored src/generated/0_1_6/ dir is carried over. Under the rolling candidate model this branch is stacked on, the snapshot that describes current source is src/generated/candidate/, regenerated by BuildPointers; a numbered dir is frozen only by script/cut-release.sh at tag time. candidate/ is regenerated here and lands on the same address 0x19272bCcFcb032eaC545E74ADFa168fDeD3e8d83 and codehash 0x1a16009998834f07d5ccab032c39377f6528870eec5abd47817f9467187b4012 that #51's hand-authored 0_1_6 dir recorded. The frozen-snapshot dirs 0_1_3, 0_1_4 and 0_1_5 are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Tracks the redesign of `ICloneableFactoryV4` at rainlanguage/rain.factory#51. The open-salt `CREATE2` salt is no longer the caller-supplied salt verbatim; it is keccak256(abi.encode(ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN, salt, keccak256(data))) so the clone address commits to `data` and `predictDeterministicAddressOpenSalt` takes `data` as an input. The domain word holds `ICloneableFactoryV4`'s MUST NOT on the factory: without it both derivations would be 64-byte preimages led by a caller-chosen word, and any account `A` could squat every open salt equal to `bytes32(uint256(uint160(A)))` through the inherited `cloneDeterministic`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…apshot The reachable overlap between the two derivations is one equation, so the mirror framing (a victim whose namespaced salt is `keccak256(P)`, reachable untagged by an open call passing `P` as `data`) is stated on the existing test rather than duplicated as a second one. `…DiffersFromSenderNamespaced` kills no mutant — a collision it could catch needs a keccak256 collision — and its NatSpec now says so, so the reader is not misled about which test carries the discrimination. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The deploy half of rainlanguage/rain.factory#51
(which closes rainlanguage/rain.factory#50). That PR was written against the
pre-split
rain.factorytree; rainlanguage/rain.factory#47 then slimmed thatrepo to interfaces only, so five of its eight files targeted paths that no
longer exist there. They land here.
Redesigned in place, 2026-08-13, tracking #51's own in-place redesign at its
head
b6e34ad.What this branch carried before — a
CREATE2salt equal to the caller-suppliedsalt verbatim, with
dataoutside the derivation — no longer implements theinterface it claims to. Everything below describes what is on the branch now;
the old shape and what it cost are under "What changed and why".
The two entry points
CloneFactorygains a second deterministic entry point.cloneDeterministicand
predictDeterministicAddressare untouched — theirmsg.sendernamespacing is a guarantee other consumers rely on — so this is purely additive.
The effective
CREATE2salt is the derivationICloneableFactoryV4fixes:so the clone address is a pure function of
(factory, implementation, salt, data). The difference between the two derivations is not "sender or no sender",it is what the address commits to:
cloneDeterministiccommits to WHOdeployed and not to WHAT;
cloneDeterministicOpenSaltcommits to WHAT and notto WHO.
predictDeterministicAddressOpenSalttherefore takesdata— V3's ruleis that predict takes exactly the inputs of the derivation, and
datais now oneof them.
_requireImplementationCodeand_initializeCloneare reused as-is, soclone-and-initialize stays atomic and the open variant's failure modes are
identical to the existing one.
NewCloneis emitted with the caller-suppliedsalt, not the effective one.The interface, and the NatSpec that states the residual
tx.originMUST NOT onimplementations, live in
rain.factoryand are not duplicated here.The domain separator, and why this repo is where it has to hold
ICloneableFactoryV4states the disjointness of the two derivations as a MUSTNOT on the factory: no other entry point may
CREATE2at an effective salt inthe open-salt image with caller-supplied
data. The factory inherits exactlysuch an entry point —
cloneDeterministictakes arbitrarydataand itseffective salt is
keccak256(abi.encode(deployer, salt)). A rule stated on aninterface is held or broken by the concrete, so it is held here or nowhere.
CloneFactoryholds it structurally:_effectiveSalthashes 64 bytes led by aleft-padded address,
_effectiveOpenSalthashes 96 bytes led by the domainconstant. Without the domain word both preimages would be 64 bytes led by a word
the caller chooses, and
abi.encodeleft-pads an address into exactly the word abytes32salt already is — so any accountAwould reach every open-saltaddress whose
saltequalsbytes32(uint256(uint160(A)))by callingcloneDeterministic(implementation, evilData, keccak256(data)). A choice ofsalt, not a preimage search.
testCloneDeterministicOpenSaltDisjointFromNamespacedAtLeftPaddedAddressSaltisthe test that fails if that stops holding. It does not assert an absence: it
builds the squat, asserts against the factory's own namespaced prediction
that an untagged derivation lands exactly on the honest party's address, then
asserts the real one does not, then deploys both for real and checks the honest
address is still free and still initializes to the honest
data. Removing thedomain word from
_effectiveOpenSaltfails it (mutant 1 below).One equation is the whole of the reachable overlap —
keccak256(abi.encode(deployer, nsSalt)) == keccak256(abi.encode(openSalt, keccak256(data)))— so closing it closes the mirror framing too (a victim whose namespaced salt is
keccak256(P)for reproducibleP, reachable untagged by an open call passingPasdata). Same two unknowns from the other side, same word closes it; thatis stated on the test rather than duplicated as a second one.
Blocked-by, and why CI is red
blocked-by rainlanguage/rain.factory#51.
ICloneableFactoryV4does not exist in any publishedrain-factoryrevision.0.1.6 is the latest on the registry (published 2026-08-13, interfaces-only, and
predating the V4 file). So this branch pins
rain-factory0.1.7and movesevery versioned import prefix with it (
rain-factory-0.1.7/src/interface/…).0.1.7 is published by autopublish when #51 merges.
Until then
forge soldeer installcannot resolve this branch. Every rainixjob runs that install as its first real step, so all three checks are red for
the same single reason —
legalincluded, which is easy to mispredict sincereuse lintitself has nothing to do with Soldeer:Nothing in this repo's own source has been evaluated by CI yet, and that red is
not to be chased. The alternative — vendoring the interface here, or aliasing
rain-factory-0.1.7/at 0.1.6 bytes — would make CI green by making the pin alie.
soldeer.lockstill carries the 0.1.5 entry for the same reason: there isno 0.1.7 revision to lock, and the first successful install rewrites it. Merge
order is #51 first, then re-run CI here.
CodeRabbit did not review this PR:
Review skipped: reviews are disabled for this base branch, because the base is #5's branch rather thanmain. There arezero review threads, resolved or otherwise. Retarget to
mainonce #5 lands andit will review then.
And after 0.1.7 publishes, five tests are still red
The five
LibCloneFactoryDeployProdTestfork tests assert the pinned address hascode on Arbitrum, Base, Base Sepolia, Flare and Polygon. Under the
rolling-candidate model this branch is stacked on, that pin tracks HEAD, and
this PR changes
CloneFactory's bytecode, which is not deployed anywhere. Theywill fail
CloneFactory not deployeduntil a deploy is dispatched. That is thecoupling #5
documents under "Behaviour change to be aware of" and that
CLAUDE.mdalreadystates; there is no code fix for it on the branch. I have deliberately not
dispatched
Manual sol artifacts.Stacked on #5 — the
0_1_6snapshot dir is NOT portedbaseis2026-07-25-rolling-candidate-model, the branch of#5, because #5 has
not merged yet. Merge #5 first; this must not be squash-merged. If #5 lands
by another route, retarget this to
main.#51 originally hand-authored
src/generated/0_1_6/CloneFactory.pointers.sol.That artefact is wrong twice over and is not carried across:
rain-factory0.1.6 haspublished from the library repo, as interfaces-only. A
0_1_6dir here wouldname a version that contains no
CloneFactoryat all.frozen-numbered-dirs-at-author-time with a rolling
src/generated/candidate/regenerated from source, frozen into a numbered dir only at tag time by
script/cut-release.sh.So
src/generated/candidate/CloneFactory.pointers.solis regenerated byforge script ./script/BuildPointers.sol, andsrc/lib/LibCloneFactoryDeploy.solneeds no edit — it already aliases
candidate/, so the pin follows the source.The frozen
0_1_3,0_1_4and0_1_5dirs are untouched, so the append-onlygate has nothing to flag (
is_tag()requires three_-separated all-numericparts, and
"candidate".split('_')has one; againstmainthe file does noteven register as a modification, because
candidate/is itself added by #5).The pin moved, because the bytecode moved. The regenerated candidate is now
main(V3 only)0x7053c1D000192f1FeC1fbe20350139F3300dBBaD0x3ec02e2f…36379d000x19272bCcFcb032eaC545E74ADFa168fDeD3e8d830x1a160099…187b40120xe03b178055685EAC2Fd8F012B444E379dE70576a0xcdb039a288346702df9136f924d5b239d6f5fa01fef3273d76df928738c01cbd.gas-snapshotis regenerated, and re-runningBuildPointerson the committedtree produces no diff.
Reproducing this locally before 0.1.7 exists
forgecannot fetchrain-factory0.1.7 yet. To build this branch today, putthe unpublished package under the gitignored
dependencies/dir:dependencies/rain-factory-0.1.7/.src/interface/ICloneableFactoryV4.solfrom #51 headb6e34adinto itssrc/interface/.remappings.txtat it:rain-factory-0.1.7/=dependencies/rain-factory-0.1.7/.Its
src/interface/is then byte-identical to #51's head — verified here withdiff -ragainst a clone checked out atb6e34ad, which reports no difference —so the bytecode produced is what CI will produce once 0.1.7 publishes. Nothing
from that dir is committed.
Files
src/concrete/CloneFactory.sol_effectiveOpenSalt, the two entry points,is ICloneableFactoryV4, and the disjointness notesrc/generated/candidate/CloneFactory.pointers.soltest/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.soltest/src/lib/LibCloneFactoryDeployCandidate.t.solfoundry.tomlrain-factory0.1.5 → 0.1.7test/src/concrete/{CloneFactoryCloneDeterministic.t.sol,TestCloneable.sol,TestCloneableFailure.sol}README.md,CLAUDE.md.gas-snapshotQA
Category check: Solidity contract change — additive external functions on a
deployed-and-pinned concrete, plus a Soldeer major-consumer pin bump.
Categories that apply: address-derivation correctness, cross-derivation
image disjointness (new, and the substance of the redesign), atomicity of
clone+initialize, non-regression of the existing sender-namespacing guarantee,
deploy-pin/snapshot consistency under the rolling-candidate model, and
versioned-import consistency across the pin bump (no unversioned remapping, no
alias pointing a versioned path at different-version bytes — the local
0.1.7dir above is gitignored and is a reproduction aid, not a committed alias). 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 rather than an
omission).
Oracle: Open-salt deterministic clone variant: keep the deployer out of the address derivation rain.factory#50 and #51's interface read at
b6e34ad,plus EIP-1167 and
CREATE2— not the implementation. Predicted addresses areasserted against OpenZeppelin's own
Clones.predictDeterministicAddressunderan independently constructed salt (the domain-tagged hash for the open
variant,
keccak256(abi.encode(deployer, salt))for the namespaced one), sothe tests do not restate
CloneFactory's arithmetic back to itself.Caller-independence is proven by actually deploying twice from two senders with
vm.snapshotState()/vm.revertToState()between them and comparing the twodeployed addresses — not two predictions, which would only test the
prediction function. The disjointness adversary is an account that can call
every function on the factory with any arguments and land first.
Discriminating tests:
test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol,13 tests at 2048 fuzz runs each.
…IsDomainTaggedHash…SaltIsVerbatimand inverted by the redesign — the CREATE2 salt iskeccak256(abi.encode(DOMAIN, salt, keccak256(data)))…DataInDerivationdataat the samesaltis a different address, and both clones exist with their own initialization…DisjointFromNamespacedAtLeftPaddedAddressSalt…DiffersFromSenderNamespaced…MatchesPredictdata…CallerIndependent(impl, data, salt), same state → the SAME address…PredictCallerIndependent…DoesNotConsumeNamespacedSalt…ManyClonesPerImpl…SecondDeployReverts(impl, data, salt)— now the only way to aim at a taken address — revertsErrors.FailedDeploymentrather than silently returning the existing clone, and the first clone's state is untouched…EventNewClone(sender, impl, child, salt, data)with the raw salt…InitializeFailureFailsInitializationFailed, and the address is left with zero code (atomicity)…ZeroImplementationCodeSizeZeroImplementationCodeSizeStated honestly:
…DiffersFromSenderNamespacedkills no mutant. Acollision it could catch needs a keccak256 collision, so no realistic mutation
makes it fail. It is kept as the plain form of the interface's claim and its
NatSpec says so; the discrimination is carried by
…DisjointFromNamespacedAtLeftPaddedAddressSalt, which is exactly why thattest is constructed rather than fuzzed.
Plus
testCandidateDeployedBytecodeServesBothEntryPointsinLibCloneFactoryDeployCandidate.t.sol(updated for the newpredictsignature). It Zoltu-deploys the recorded
CREATION_CODE(deliberately notnew CloneFactory(), so the assertion is about the snapshot rather than thesource) and calls all four entry points through the
ICloneableFactoryV4ABI,so a pin recording an address for bytecode that does not dispatch an entry
point cannot pass silently. A byte scan of the runtime code would NOT prove
this — a selector can sit in constant data without being dispatchable.
The existing
CloneFactoryCloneDeterministic.t.solalready pins the namespacedderivation (
testCloneDeterministicSaltIsAbiEncodeHash) and its sender-scoping(
testCloneDeterministicSenderScoped); the mutation run confirms both stillkill, so they are left as-is rather than duplicated.
Mutations applied: 11, each applied to the committed tree, whole suite run,
tree restored, baseline re-verified green. All 11 killed. Two liveness
guards on the harness, because a mutation that silently fails to apply reads as
a surviving mutant:
CODE-CHANGED— comments are stripped fromCloneFactory.solbefore andafter each edit and the two must differ. The first run of this pass produced
a false survivor for mutant 3 exactly this way: the regex matched the
docstring that quotes the derivation before it reached the
return. Anchoredand re-run; every mutant below reports
CODE-CHANGED: yes.TOTALS— passed+failed equals the baseline's 33 on every single run, so thetests demonstrably ran rather than being filtered out or failing to compile.
Killers listed are behavioural: the three codehash pins
(
testCandidateSelfConsistent,testDeployAddress,testExpectedCodeHash)trip on all eleven, but that is a bytecode oracle rather than evidence of
behavioural coverage, so they are excluded from attribution.
…IsDomainTaggedHash,…DisjointFromNamespacedAtLeftPaddedAddressSaltdata(the verbatim-salt design this PR replaces)…IsDomainTaggedHash,…DataInDerivationsaltandkeccak256(data)in the preimage…IsDomainTaggedHashmsg.sender…CallerIndependent,…DataInDerivation,…DisjointFrom…,…MatchesPredict,…SecondDeployReverts…CallerIndependent,…DataInDerivation,…DisjointFrom…,…IsDomainTaggedHash,…MatchesPredict,…PredictCallerIndependent_requireImplementationCode…ZeroImplementationCodeSize_initializeClone…DataInDerivation,…DisjointFrom…,…Event,…InitializeFailureFails,…MatchesPredict,…SecondDeployRevertsinitializereturn code left unchecked…InitializeFailureFails+ existingtestCloneDeterministicInitializeFailureFailscloneDeterministicdrops its sender namespacingtestCloneDeterministicSenderScoped,testCloneDeterministicMatchesPredict+…DisjointFrom…,…DoesNotConsumeNamespacedSaltpredictDeterministicAddressdrops its namespacingtestCloneDeterministicSaltIsAbiEncodeHash,testCloneDeterministicSenderScoped,testCloneDeterministicMatchesPredict+…DisjointFrom…,…DoesNotConsumeNamespacedSaltNewCloneemits the effective salt instead of the raw salt…Event+ existingtestCloneDeterministicEventMutant 1 is the one this redesign exists for, and note which test catches it:
…DiffersFromSenderNamespaced— the general fuzz over both derivations —survives it. Only the constructed left-padded-address case fails, which is
the empirical form of "reachable, not merely improbable". Mutants 9 and 10 are
the ones that matter for "do not break the existing guarantee", and the
disjointness test guards that side too.
Local run (rainix
sol-shell, with the reproduction dir above):forge testexcluding the five fork tests — 33 passed, 0 failed.
forge buildclean,forge fmt --checkclean,slither .reports0 result(s) found,reuse lintcompliant, one contract per
.solfile.What changed and why
The branch previously specified
saltused verbatim as theCREATE2salt,with
dataoutside the derivation, andpredictDeterministicAddressOpenSalt(address implementation, bytes32 salt).Under that shape the address did not encode
data, so the first caller'sdatawas baked in permanently at an address that said nothing about it. Deployer
irrelevance therefore depended on every consumer choosing to pass empty
dataand on every implementation's
initializebeing audited for whether it takesauthority from it — a convention to audit, not a property the signature held.
Hashing
datainto the salt collapses that: a front-runner passing differentdataderives a different address and has deployed their own contract at theirown expense; one passing the same
datahas deployed exactly what was intended.What is left to state is the
tx.originMUST NOT and the timing residual, and#51 states those as rules on the interface rather than as an audit. What is left
to hold is the image disjointness, and that is this repo's job.
Three consequences reached this branch and are the substance of this update:
_effectiveOpenSaltreplaces the verbatim pass-through in both open entrypoints, and
predictDeterministicAddressOpenSaltgains itsdataparameter.src/generated/candidate/and.gas-snapshotareregenerated and the pinned address and codehash both move (table above).
…SaltIsVerbatimasserted the exact property theredesign deletes and inverts into
…IsDomainTaggedHash;…SecondDeployRevertspreviously deployed with one
dataand re-deployed with another at the samesalt, which under the new derivation is two different addresses and no longer
reverts, so it now repeats the whole triple;
…MatchesPredict,…CallerIndependent,…PredictCallerIndependent,…InitializeFailureFails,…DoesNotConsumeNamespacedSaltandtestCandidateDeployedBytecodeServesBothEntryPointsall move to the three-argument
predict. Two properties that did not existbefore are owed and added:
…DataInDerivationand…DisjointFromNamespacedAtLeftPaddedAddressSalt. The mutation pass recorded onthis PR previously targeted the verbatim-salt code and no longer applies; the
11 above replace it.
🤖 Generated with Claude Code