CloneFactory: concrete only delegates to LibICloneableFactoryV4 - #21
Conversation
0.1.9 is the release that carries LibICloneableFactoryV4 — the whole of an ICloneableFactoryV4 factory as internal library logic. This commit only moves the dependency and every import path; the concrete still carries its own logic, replaced in the next commits. CloneFactory's bytecode is unchanged so the candidate pins hold. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rain-deploy's RainDeployVerifyChain NatSpec scopes chain verification to released suites only: "the candidate is meant to be ahead of the chain", and "Demanding the candidate be live asserts something false by design". This test asserted exactly that over the candidate alias pins, so any bytecode-moving change reds five fork suites until a human broadcasts the new candidate. It predates the migration onto the rain-deploy abstracts and only ever passed because the candidate happened to equal the deployed 0.1.5 bytecode. The model-correct anchors already exist here: CloneFactoryDeployChainTest holds every RELEASED suite live on every supported network, and CloneFactoryDeploySnapshotTest / LibCloneFactoryDeployCandidateTest hold the candidate anchored to source. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The concrete is now nothing but one delegation per entry point into the library, implementing ICloneableFactoryV4 — the namespaced pair inherited from V3 plus the open-salt pair. The local _effectiveSalt (untagged keccak256(abi.encode(deployer, salt))), the local errors, the OZ Clones dependency and the inline initialize check are superseded by the library, not preserved: both derivations are now domain-tagged per the V4 spec, and the typed errors (ZeroImplementationCodeSize, CloneDeploymentFailed, InitializationFailed) live in LibICloneableFactoryV4. The behavior suite pins the new namespaced derivation against OZ Clones as a foreign EIP-1167 oracle and imports the errors from the library. script/Build.sol regenerated the candidate snapshot; the bytecode, address and codehash all move, which is what releasing through the rolling candidate is for. OZ Clones remains a test-only dependency. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TestLibCloneFactory is the library run bare behind an independent delegating surface. CloneFactoryLibEquivalenceTest holds each entry point to the library's behaviour two ways at once: against the library's own pure derivation (effectiveSalt / effectiveOpenSalt / predictCloneAddress) applied at the concrete's address, and against the bare surface exercised from identical chain state via snapshotState/revertToState — child address, deployed code, initialized state and NewClone field for field, plus revert equivalence for ZeroImplementationCodeSize, InitializationFailed and CloneDeploymentFailed (salt reuse with different data on the namespaced path, a different sender on the open-salt path). testEquivalenceRuntimeBytecode pins the strongest form: metadata is stripped, so the concrete and the bare surface compile to byte-identical runtime code — behaviour added to the concrete breaks this before it breaks anything behavioural. .gas-snapshot regenerated; it was stale on main (it still named LibCloneFactoryDeployTaggedConstantsTest, deleted in the abstracts migration). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CLAUDE.md and README each carried a sentence the rework falsifies — the concrete "clones via OpenZeppelin Clones", and the library half being only the ICloneable* interfaces. Both now state the delegation model: LibICloneableFactoryV4 carries the factory logic and the concrete adds no behaviour of its own. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (11)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughCloneFactory now implements the V4 factory interface and delegates deterministic clone operations to LibICloneableFactoryV4. The change adds open-salt methods, updates dependency documentation, and adds equivalence and deployment validation coverage. ChangesV4 clone factory delegation
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The delegation change is merge-ready after normal checks and review; no actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant Caller
participant CloneFactory
participant LibICloneableFactoryV4
Caller->>CloneFactory: Request deterministic clone
CloneFactory->>LibICloneableFactoryV4: Delegate deployment or prediction
LibICloneableFactoryV4-->>CloneFactory: Return clone address or revert
CloneFactory-->>Caller: Return result
🚥 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 |
Reworks the concrete
CloneFactoryto userain.factoryproperly: the concrete is now nothing but one delegation per entry point intoLibICloneableFactoryV4, per the model that the logic lives in the library repo and the deploy repo ships only the concrete and its pins.What changed
rain-factory0.1.5 → 0.1.9, the release that carriessrc/lib/LibICloneableFactoryV4.sol: both V4 domain-tagged salt derivations, EIP-1167 construction and prediction, the implementation-code guard, the atomic clone-initialize-verify flow, four entry points, and the typed errors (ZeroImplementationCodeSize,CloneDeploymentFailed,InitializationFailed).src/concrete/CloneFactory.solnow implementsICloneableFactoryV4as four single-line delegations. The local_effectiveSalt(untaggedkeccak256(abi.encode(deployer, salt))), the local errors, the OZClonesdependency and the inline initialize check are all gone — superseded by the library, not preserved. The namespaced derivation is therefore now the V4 domain-tagged one; the concrete's bytecode, address and codehash all move, which is the point of releasing through the candidate.test/src/concrete/TestLibCloneFactory.sol+test/src/concrete/CloneFactoryLibEquivalence.t.sol: per-function equivalence holding concrete == library (see QA).CloneFactoryCloneDeterministicsuite keeps OZClonesas a foreign EIP-1167 oracle and pins the new domain-tagged derivation.src/generated/candidate/CloneFactory.sol,src/lib/LibCloneFactoryDeploy.solregenerated byscript/Build.sol;.gas-snapshotregenerated (it was stale on main — it still namedLibCloneFactoryDeployTaggedConstantsTest, deleted in the abstracts migration).test/src/lib/LibCloneFactoryDeployProd.t.soldeleted. It asserted the CANDIDATE pins are live on five networks, whichrain-deploy's own model doc calls out as wrong by construction: "the candidate is meant to be ahead of the chain … Demanding the candidate be live asserts something false by design" (RainDeployVerifyChainNatSpec). It only ever passed because the candidate happened to equal the deployed 0.1.5 bytecode; any source change reds it until a human broadcasts. The model-correct chain anchor — every RELEASED suite live on every network — already exists here asCloneFactoryDeployChainTest, and the candidate stays anchored to source byCloneFactoryDeploySnapshotTest+LibCloneFactoryDeployCandidateTest.Clones"; library half is "theICloneable*interfaces") updated to the delegation model.QA
Discriminating tests
CloneFactoryLibEquivalenceTestholds each entry point to the library's behaviour two ways at once: against the library's own pure derivation (effectiveSalt/effectiveOpenSalt/predictCloneAddress) applied at the concrete's address, and againstTestLibCloneFactory— the library run bare behind an independent delegating surface — exercised from identical chain state viavm.snapshotState/vm.revertToState, comparing child address, deployed code, initialized state, andNewClonefield for field. Revert equivalence coversZeroImplementationCodeSize,InitializationFailed, andCloneDeploymentFailed(salt reuse: differentdataon the namespaced path, different sender on the open-salt path) on both surfaces.testEquivalenceRuntimeBytecodepins the strongest form: with metadata stripped, the concrete and the bare-library surface compile to byte-identical runtime code — any behaviour added to the concrete breaks this before it breaks anything behavioural.Mutations applied
nix run github:rainlanguage/adversarial-mutation-test#mutation-probe -- mutants.toml, suite scoped totest/src/concrete/*so kills come from the concrete's own tests, never from the pin-currency tests (which any bytecode change trips trivially). Six mutants over the four delegations insrc/concrete/CloneFactory.sol, baseline green (18 passed):cloneDeterministiccross-wired to the open-salt derivationtestCloneDeterministicMatchesPredict,testCloneDeterministicSenderScoped,testEquivalenceCloneDeterministic,testEquivalenceCloneDeterministicSaltTakencloneDeterministicdrops the caller salt (bytes32(0))testCloneDeterministicEvent,testCloneDeterministicManyClonesPerImpl,testCloneDeterministicMatchesPredict,testCloneDeterministicSenderScoped,testEquivalenceCloneDeterministicpredictDeterministicAddressignores thedeployerargument (msg.sender)testCloneDeterministicSaltIsDomainTaggedHash,testCloneDeterministicSenderScoped,testEquivalencePredictDeterministicAddresspredictDeterministicAddresspredicts from the wrong factory addresstestEquivalencePredictDeterministicAddress,testCloneDeterministicMatchesPredict,testCloneDeterministicSaltIsDomainTaggedHash,testCloneDeterministicSenderScopedcloneDeterministicOpenSaltcross-wired to the namespaced derivationtestEquivalenceCloneDeterministicOpenSalt,testEquivalenceCloneDeterministicOpenSaltSaltTakenpredictDeterministicAddressOpenSaltignores thedataargumenttestEquivalencePredictDeterministicAddressOpenSaltTwo passes, both 6/6 killed, 0 survived, 0 no-run, 0 harness errors: the full concrete suite (where
testEquivalenceRuntimeBytecodeadditionally kills every mutant), and a second pass with that test excluded (--no-match-test) proving the behavioural tests discriminate on their own — the killer names above are from the behavioural pass.Oracle
ICloneableFactoryV4's NatSpec pins both effective-salt derivations to exact bytes; the tests recompute them inline from the pinned formulas and from the library's pure functions, never read back from the contract under test. OZClonesserves as a foreign implementation of the same EIP-1167 standard for address prediction, so the derivation is checked against code that shares none of the library's arithmetic.Category check
address(this)of the concrete.testEquivalenceRuntimeBytecodebefore behaviour is even observed.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation