From 9a73b93c818b9cd7ef44fdfab5937871721e9e19 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 11:23:37 +0000 Subject: [PATCH 1/3] feat: open-salt deterministic clone variant (ICloneableFactoryV4) 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) --- .gas-snapshot | 36 ++- CLAUDE.md | 19 +- README.md | 25 ++ foundry.toml | 2 +- src/concrete/CloneFactory.sol | 46 +++- .../candidate/CloneFactory.pointers.sol | 8 +- .../CloneFactoryCloneDeterministic.t.sol | 2 +- ...oneFactoryCloneDeterministicOpenSalt.t.sol | 219 ++++++++++++++++++ test/src/concrete/TestCloneable.sol | 2 +- test/src/concrete/TestCloneableFailure.sol | 2 +- .../lib/LibCloneFactoryDeployCandidate.t.sol | 32 +++ 11 files changed, 362 insertions(+), 31 deletions(-) create mode 100644 test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol diff --git a/.gas-snapshot b/.gas-snapshot index 9fe3697..7f83fb9 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,19 +1,31 @@ -CloneFactoryCloneDeterministicTest:testCloneDeterministicEvent(bytes32,bytes) (runs: 2048, μ: 421457, ~: 371564) -CloneFactoryCloneDeterministicTest:testCloneDeterministicInitializeFailureFails(bytes32,bytes32) (runs: 2048, μ: 161266, ~: 161266) -CloneFactoryCloneDeterministicTest:testCloneDeterministicManyClonesPerImpl(bytes32,bytes32,bytes) (runs: 2048, μ: 569724, ~: 482275) -CloneFactoryCloneDeterministicTest:testCloneDeterministicMatchesPredict(bytes32,bytes) (runs: 2048, μ: 423089, ~: 372826) -CloneFactoryCloneDeterministicTest:testCloneDeterministicSaltIsAbiEncodeHash(address,bytes32,address) (runs: 2048, μ: 4686, ~: 4686) -CloneFactoryCloneDeterministicTest:testCloneDeterministicSenderScoped(bytes32,bytes,address,address) (runs: 2048, μ: 585442, ~: 485815) -CloneFactoryCloneDeterministicTest:testCloneDeterministicZeroImplementationCodeSize(address,bytes,bytes32) (runs: 2048, μ: 10699, ~: 10678) -LibCloneFactoryDeployCandidateTest:testCandidateCreationDeploysToPinnedAddress() (gas: 316633) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltCallerIndependent(bytes32,bytes,address,address) (runs: 2048, μ: 582462, ~: 439968) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDiffersFromSenderNamespaced(address,bytes32,address) (runs: 2048, μ: 5400, ~: 5400) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDoesNotConsumeNamespacedSalt(bytes32,bytes) (runs: 2048, μ: 574640, ~: 436159) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltEvent(bytes32,bytes) (runs: 2048, μ: 418423, ~: 349063) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltInitializeFailureFails(bytes32,bytes32) (runs: 2048, μ: 164888, ~: 164888) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltManyClonesPerImpl(bytes32,bytes32,bytes) (runs: 2048, μ: 568887, ~: 437277) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltMatchesPredict(bytes32,bytes) (runs: 2048, μ: 419757, ~: 349902) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltPredictCallerIndependent(address,bytes32,address,address) (runs: 2048, μ: 8766, ~: 8766) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltSaltIsVerbatim(address,bytes32) (runs: 2048, μ: 4262, ~: 4262) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltSecondDeployReverts(bytes32,bytes,bytes,address,address) (runs: 2048, μ: 1040442091, ~: 1040442127) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltZeroImplementationCodeSize(address,bytes,bytes32) (runs: 2048, μ: 10701, ~: 10678) +CloneFactoryCloneDeterministicTest:testCloneDeterministicEvent(bytes32,bytes) (runs: 2048, μ: 418483, ~: 349123) +CloneFactoryCloneDeterministicTest:testCloneDeterministicInitializeFailureFails(bytes32,bytes32) (runs: 2048, μ: 161288, ~: 161288) +CloneFactoryCloneDeterministicTest:testCloneDeterministicManyClonesPerImpl(bytes32,bytes32,bytes) (runs: 2048, μ: 569073, ~: 437463) +CloneFactoryCloneDeterministicTest:testCloneDeterministicMatchesPredict(bytes32,bytes) (runs: 2048, μ: 420117, ~: 350262) +CloneFactoryCloneDeterministicTest:testCloneDeterministicSaltIsAbiEncodeHash(address,bytes32,address) (runs: 2048, μ: 4708, ~: 4708) +CloneFactoryCloneDeterministicTest:testCloneDeterministicSenderScoped(bytes32,bytes,address,address) (runs: 2048, μ: 583541, ~: 441047) +CloneFactoryCloneDeterministicTest:testCloneDeterministicZeroImplementationCodeSize(address,bytes,bytes32) (runs: 2048, μ: 10717, ~: 10700) +LibCloneFactoryDeployCandidateTest:testCandidateCreationDeploysToPinnedAddress() (gas: 339992) +LibCloneFactoryDeployCandidateTest:testCandidateDeployedBytecodeServesBothEntryPoints() (gas: 733209) LibCloneFactoryDeployCandidateTest:testCandidateIsTheAliasedSnapshot() (gas: 3708) -LibCloneFactoryDeployCandidateTest:testCandidateRuntimeHashesToBytecodeHash() (gas: 879) -LibCloneFactoryDeployCandidateTest:testCandidateSelfConsistent() (gas: 4706) +LibCloneFactoryDeployCandidateTest:testCandidateRuntimeHashesToBytecodeHash() (gas: 928) +LibCloneFactoryDeployCandidateTest:testCandidateSelfConsistent() (gas: 4791) LibCloneFactoryDeployTaggedConstantsTest:testCloneFactory_0_1_3_CreationDeploysToPinnedAddress() (gas: 240145) LibCloneFactoryDeployTaggedConstantsTest:testCloneFactory_0_1_3_RuntimeHashesToBytecodeHash() (gas: 755) LibCloneFactoryDeployTaggedConstantsTest:testCloneFactory_0_1_4_CreationDeploysToPinnedAddress() (gas: 240146) LibCloneFactoryDeployTaggedConstantsTest:testCloneFactory_0_1_4_RuntimeHashesToBytecodeHash() (gas: 755) LibCloneFactoryDeployTaggedConstantsTest:testCloneFactory_0_1_5_CreationDeploysToPinnedAddress() (gas: 316590) LibCloneFactoryDeployTaggedConstantsTest:testCloneFactory_0_1_5_RuntimeHashesToBytecodeHash() (gas: 922) -LibCloneFactoryDeployTest:testDeployAddress() (gas: 315988) -LibCloneFactoryDeployTest:testExpectedCodeHash() (gas: 311566) \ No newline at end of file +LibCloneFactoryDeployTest:testDeployAddress() (gas: 339275) +LibCloneFactoryDeployTest:testExpectedCodeHash() (gas: 334816) \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md index f914688..bff5632 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -68,8 +68,23 @@ as the `rain-factory` Soldeer dependency, so they are read under `dependencies/rain-factory-/src/interface/`. - `src/concrete/CloneFactory.sol` — The single concrete implementation of - `ICloneableFactoryV3`. Uses OpenZeppelin `Clones.cloneDeterministic()`; there - is no plain `clone()`. + `ICloneableFactoryV4`. Uses OpenZeppelin `Clones.cloneDeterministic()` for + both deterministic entry points; there is no plain `clone()`. The two entry + points differ ONLY in the salt they pass to `Clones`: + - `cloneDeterministic` / `predictDeterministicAddress` (declared on + `ICloneableFactoryV3`, which V4 extends) namespace the caller-supplied salt + by `msg.sender` via `_effectiveSalt`, so a caller's `(implementation, salt)` + address cannot be reached by another account. + - `cloneDeterministicOpenSalt` / `predictDeterministicAddressOpenSalt` pass + the caller-supplied salt verbatim, so the address carries no identity and + anyone can deploy it. Only safe for implementations whose `initialize` + takes no caller-controlled authority — the NatSpec on + `ICloneableFactoryV4.cloneDeterministicOpenSalt` is the spec for that + condition, and it lives in rain.factory, not here. + + Both share `_requireImplementationCode` and `_initializeClone`, so + clone-and-initialize is atomic and the failure modes are identical across the + two. - `src/lib/LibCloneFactoryDeploy.sol` — Deterministic deployment address and codehash constants (generated; aliases the rolling `src/generated/candidate/` snapshot). diff --git a/README.md b/README.md index 4b7301c..b9ed8c6 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,31 @@ here as the `rain-factory` Soldeer package. Consumers that need only the interfaces depend on `rain-factory`; consumers that need the deployed address/codehash pins depend on `rain-factory-deploy`. +## Entry points + +`CloneFactory` implements `ICloneableFactoryV4`, letting any compatible +`ICloneableV2` contract be cloned as an EIP1167 proxy and initialized +atomically. It offers two deterministic (`CREATE2`) entry points that differ +only in how the salt is derived: + +- `cloneDeterministic` namespaces the caller-supplied salt by `msg.sender`, so + nobody else can reach the caller's address — but the deploying account is + baked into that address forever. +- `cloneDeterministicOpenSalt` uses the caller-supplied salt verbatim, so the + address is a function of `(implementation, salt)` and the factory alone: every + account reaches the same address, and so can anyone. That also makes it the + same address across chains, but only where both the factory and the + implementation are themselves at the same address on each chain — `CREATE2` + hashes the factory, and the EIP1167 creation code it hashes contains the + implementation. + +Open-salt is ONLY safe for implementations whose `initialize` takes no +caller-controlled authority: clone-and-initialize is atomic and runs once, so +the first deployer's `data` sets the clone's authority permanently, with no +recovery. Read the NatSpec on +`ICloneableFactoryV4.cloneDeterministicOpenSalt` (in `rain.factory`) before +using it — the qualifying condition is stated there, not here. + ## Snapshots `src/generated/` holds two kinds of deploy-pin snapshot, both with the same file diff --git a/foundry.toml b/foundry.toml index 0a3a1c3..493677c 100644 --- a/foundry.toml +++ b/foundry.toml @@ -40,7 +40,7 @@ forge-std = "1.16.1" "rain-extrospection" = "0.1.1" "rain-deploy" = "0.1.3" "rain-sol-codegen" = "0.1.0" -"rain-factory" = "0.1.5" +"rain-factory" = "0.1.7" [soldeer] recursive_deps = false diff --git a/src/concrete/CloneFactory.sol b/src/concrete/CloneFactory.sol index 0d6f16c..9845644 100644 --- a/src/concrete/CloneFactory.sol +++ b/src/concrete/CloneFactory.sol @@ -2,8 +2,12 @@ // SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd pragma solidity =0.8.25; -import {ICloneableV2, ICLONEABLE_V2_SUCCESS} from "rain-factory-0.1.5/src/interface/ICloneableV2.sol"; -import {ICloneableFactoryV3} from "rain-factory-0.1.5/src/interface/ICloneableFactoryV3.sol"; +import {ICloneableV2, ICLONEABLE_V2_SUCCESS} from "rain-factory-0.1.7/src/interface/ICloneableV2.sol"; +// `ICloneableFactoryV3` is imported for the `@inheritdoc` references on the +// functions it declares; `ICloneableFactoryV4` inherits rather than redeclares +// them, so the tag must name V3 and V3 must be in scope here. +import {ICloneableFactoryV3} from "rain-factory-0.1.7/src/interface/ICloneableFactoryV3.sol"; +import {ICloneableFactoryV4} from "rain-factory-0.1.7/src/interface/ICloneableFactoryV4.sol"; import {Clones} from "@openzeppelin-contracts-5.6.1/proxy/Clones.sol"; /// Thrown when an implementation has zero code size which is always a mistake. @@ -13,15 +17,21 @@ error ZeroImplementationCodeSize(); error InitializationFailed(); /// @title CloneFactory -/// @notice A fairly minimal implementation of `ICloneableFactoryV3` that uses +/// @notice A fairly minimal implementation of `ICloneableFactoryV4` that uses /// Open Zeppelin `Clones` to create EIP1167 clones of a reference bytecode. The /// reference bytecode MUST implement `ICloneableV2`. /// -/// `cloneDeterministic` deploys via `CREATE2` at a pre-computable address -/// (`predictDeterministicAddress`), namespacing the caller-supplied salt by -/// `msg.sender` so a caller's `(implementation, salt)` address cannot be squatted -/// by another account. -contract CloneFactory is ICloneableFactoryV3 { +/// Two deterministic entry points, both `CREATE2`, differing only in the salt: +/// +/// - `cloneDeterministic` / `predictDeterministicAddress` namespace the +/// caller-supplied salt by `msg.sender` (see `_effectiveSalt`) so a caller's +/// `(implementation, salt)` address cannot be squatted by another account. +/// - `cloneDeterministicOpenSalt` / `predictDeterministicAddressOpenSalt` use +/// the caller-supplied salt verbatim, so the address carries no identity and +/// anyone can deploy it. This is only safe for implementations whose +/// `initialize` takes no caller-controlled authority — read the warning on +/// `ICloneableFactoryV4.cloneDeterministicOpenSalt` before using it. +contract CloneFactory is ICloneableFactoryV4 { /// @inheritdoc ICloneableFactoryV3 function cloneDeterministic(address implementation, bytes calldata data, bytes32 salt) external returns (address) { _requireImplementationCode(implementation); @@ -39,6 +49,24 @@ contract CloneFactory is ICloneableFactoryV3 { return Clones.predictDeterministicAddress(implementation, _effectiveSalt(deployer, salt), address(this)); } + /// @inheritdoc ICloneableFactoryV4 + function cloneDeterministicOpenSalt(address implementation, bytes calldata data, bytes32 salt) + external + returns (address) + { + _requireImplementationCode(implementation); + // CREATE2 clone at the caller-supplied salt verbatim: no `_effectiveSalt` + // namespacing, so the address is the same for every caller and there is + // no identity in the derivation. + address child = Clones.cloneDeterministic(implementation, salt); + return _initializeClone(implementation, child, data, salt); + } + + /// @inheritdoc ICloneableFactoryV4 + function predictDeterministicAddressOpenSalt(address implementation, bytes32 salt) external view returns (address) { + return Clones.predictDeterministicAddress(implementation, salt, address(this)); + } + /// @dev The CREATE2 salt actually used: the caller-supplied `salt` namespaced /// by the deploying account. Prevents a caller's `(implementation, salt)` /// address being front-run/squatted by another account, while still letting a @@ -69,7 +97,7 @@ contract CloneFactory is ICloneableFactoryV3 { { emit NewClone(msg.sender, implementation, child, salt, data); // Checking the return value of initialize is mandatory as per - // ICloneableFactoryV3. + // ICloneableFactoryV3 and ICloneableFactoryV4. if (ICloneableV2(child).initialize(data) != ICLONEABLE_V2_SUCCESS) { revert InitializationFailed(); } diff --git a/src/generated/candidate/CloneFactory.pointers.sol b/src/generated/candidate/CloneFactory.pointers.sol index f0b9c14..886a926 100644 --- a/src/generated/candidate/CloneFactory.pointers.sol +++ b/src/generated/candidate/CloneFactory.pointers.sol @@ -10,16 +10,16 @@ pragma solidity ^0.8.25; // file needs the contract to exist so that it can be compiled. /// @dev Hash of the known bytecode. -bytes32 constant BYTECODE_HASH = bytes32(0x3ec02e2f27d73f603f5b37a7dc228956004e9154c6d1f9c5335bd60036379d00); +bytes32 constant BYTECODE_HASH = bytes32(0x1a16009998834f07d5ccab032c39377f6528870eec5abd47817f9467187b4012); /// @dev The deterministic deploy address of the contract when deployed via /// the Zoltu factory. -address constant DEPLOYED_ADDRESS = address(0x7053c1D000192f1FeC1fbe20350139F3300dBBaD); +address constant DEPLOYED_ADDRESS = address(0x19272bCcFcb032eaC545E74ADFa168fDeD3e8d83); /// @dev The creation bytecode of the contract. bytes constant CREATION_CODE = - hex"6080604052348015600e575f80fd5b506105708061001c5f395ff3fe608060405234801561000f575f80fd5b5060043610610034575f3560e01c806340419eec1461003857806393a7e71114610074575b5f80fd5b61004b6100463660046103f1565b610087565b60405173ffffffffffffffffffffffffffffffffffffffff909116815260200160405180910390f35b61004b610082366004610473565b6100c7565b5f61009185610159565b5f6100ae866100a933865f9182526020526040902090565b6101ac565b90506100bd86828787876101b8565b9695505050505050565b5f61014f846100df84865f9182526020526040902090565b6040513060388201526f5af43d82803e903d91602b57fd5bf3ff60248201526014810192909252733d602d80600a3d3981f3363d3d373d3d3d363d73825260588201526037600c8201206078820152605560439091012073ffffffffffffffffffffffffffffffffffffffff1690565b90505b9392505050565b8073ffffffffffffffffffffffffffffffffffffffff163b5f036101a9576040517ff432283200000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b50565b5f61015283835f6102f1565b5f7f3b3e5b48cfaf4a4b3b2b36425ddec21c2c507fd502d2aea55eb7ffa36b0ca2053387878588886040516101f2969594939291906104f3565b60405180910390a16040517f439fab910000000000000000000000000000000000000000000000000000000081527fe0e57eda3f08f2a93bbe980d3df7f9c315eac41181f58b865a13d917fe769fc39073ffffffffffffffffffffffffffffffffffffffff87169063439fab91906102709088908890600401610546565b6020604051808303815f875af115801561028c573d5f803e3d5ffd5b505050506040513d601f19601f820116820180604052508101906102b09190610559565b146102e7576040517f19b991a800000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b5092949350505050565b5f81471015610339576040517fcf4791810000000000000000000000000000000000000000000000000000000081524760048201526024810183905260440160405180910390fd5b763d602d80600a3d3981f3363d3d373d3d3d363d730000008460601b60e81c175f526e5af43d82803e903d91602b57fd5bf38460781b17602052826037600984f5905073ffffffffffffffffffffffffffffffffffffffff8116610152576040517fb06ebf3d00000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b803573ffffffffffffffffffffffffffffffffffffffff811681146103ec575f80fd5b919050565b5f805f8060608587031215610404575f80fd5b61040d856103c9565b9350602085013567ffffffffffffffff80821115610429575f80fd5b818701915087601f83011261043c575f80fd5b81358181111561044a575f80fd5b88602082850101111561045b575f80fd5b95986020929092019750949560400135945092505050565b5f805f60608486031215610485575f80fd5b61048e846103c9565b9250602084013591506104a3604085016103c9565b90509250925092565b81835281816020850137505f602082840101525f60207fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0601f840116840101905092915050565b5f73ffffffffffffffffffffffffffffffffffffffff8089168352808816602084015280871660408401525084606083015260a0608083015261053a60a0830184866104ac565b98975050505050505050565b602081525f61014f6020830184866104ac565b5f60208284031215610569575f80fd5b505191905056"; + hex"6080604052348015600e575f80fd5b506105e48061001c5f395ff3fe608060405234801561000f575f80fd5b506004361061004a575f3560e01c806340375cb71461004e57806340419eec1461008a57806393a7e7111461009d578063fc90f455146100b0575b5f80fd5b61006161005c36600461043d565b61012e565b60405173ffffffffffffffffffffffffffffffffffffffff909116815260200160405180910390f35b61006161009836600461043d565b61015c565b6100616100ab3660046104bf565b610183565b6100616100be3660046104f8565b6040513060388201526f5af43d82803e903d91602b57fd5bf3ff60248201526014810192909252733d602d80600a3d3981f3363d3d373d3d3d363d73825260588201526037600c8201206078820152605560439091012073ffffffffffffffffffffffffffffffffffffffff1690565b5f610138856101a5565b5f61014386846101f8565b90506101528682878787610204565b9695505050505050565b5f610166856101a5565b5f6101438661017e33865f9182526020526040902090565b6101f8565b5f61019b846100be84865f9182526020526040902090565b90505b9392505050565b8073ffffffffffffffffffffffffffffffffffffffff163b5f036101f5576040517ff432283200000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b50565b5f61019e83835f61033d565b5f7f3b3e5b48cfaf4a4b3b2b36425ddec21c2c507fd502d2aea55eb7ffa36b0ca20533878785888860405161023e96959493929190610567565b60405180910390a16040517f439fab910000000000000000000000000000000000000000000000000000000081527fe0e57eda3f08f2a93bbe980d3df7f9c315eac41181f58b865a13d917fe769fc39073ffffffffffffffffffffffffffffffffffffffff87169063439fab91906102bc90889088906004016105ba565b6020604051808303815f875af11580156102d8573d5f803e3d5ffd5b505050506040513d601f19601f820116820180604052508101906102fc91906105cd565b14610333576040517f19b991a800000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b5092949350505050565b5f81471015610385576040517fcf4791810000000000000000000000000000000000000000000000000000000081524760048201526024810183905260440160405180910390fd5b763d602d80600a3d3981f3363d3d373d3d3d363d730000008460601b60e81c175f526e5af43d82803e903d91602b57fd5bf38460781b17602052826037600984f5905073ffffffffffffffffffffffffffffffffffffffff811661019e576040517fb06ebf3d00000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b803573ffffffffffffffffffffffffffffffffffffffff81168114610438575f80fd5b919050565b5f805f8060608587031215610450575f80fd5b61045985610415565b9350602085013567ffffffffffffffff80821115610475575f80fd5b818701915087601f830112610488575f80fd5b813581811115610496575f80fd5b8860208285010111156104a7575f80fd5b95986020929092019750949560400135945092505050565b5f805f606084860312156104d1575f80fd5b6104da84610415565b9250602084013591506104ef60408501610415565b90509250925092565b5f8060408385031215610509575f80fd5b61051283610415565b946020939093013593505050565b81835281816020850137505f602082840101525f60207fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0601f840116840101905092915050565b5f73ffffffffffffffffffffffffffffffffffffffff8089168352808816602084015280871660408401525084606083015260a060808301526105ae60a083018486610520565b98975050505050505050565b602081525f61019b602083018486610520565b5f602082840312156105dd575f80fd5b505191905056"; /// @dev The runtime bytecode of the contract. bytes constant RUNTIME_CODE = - hex"608060405234801561000f575f80fd5b5060043610610034575f3560e01c806340419eec1461003857806393a7e71114610074575b5f80fd5b61004b6100463660046103f1565b610087565b60405173ffffffffffffffffffffffffffffffffffffffff909116815260200160405180910390f35b61004b610082366004610473565b6100c7565b5f61009185610159565b5f6100ae866100a933865f9182526020526040902090565b6101ac565b90506100bd86828787876101b8565b9695505050505050565b5f61014f846100df84865f9182526020526040902090565b6040513060388201526f5af43d82803e903d91602b57fd5bf3ff60248201526014810192909252733d602d80600a3d3981f3363d3d373d3d3d363d73825260588201526037600c8201206078820152605560439091012073ffffffffffffffffffffffffffffffffffffffff1690565b90505b9392505050565b8073ffffffffffffffffffffffffffffffffffffffff163b5f036101a9576040517ff432283200000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b50565b5f61015283835f6102f1565b5f7f3b3e5b48cfaf4a4b3b2b36425ddec21c2c507fd502d2aea55eb7ffa36b0ca2053387878588886040516101f2969594939291906104f3565b60405180910390a16040517f439fab910000000000000000000000000000000000000000000000000000000081527fe0e57eda3f08f2a93bbe980d3df7f9c315eac41181f58b865a13d917fe769fc39073ffffffffffffffffffffffffffffffffffffffff87169063439fab91906102709088908890600401610546565b6020604051808303815f875af115801561028c573d5f803e3d5ffd5b505050506040513d601f19601f820116820180604052508101906102b09190610559565b146102e7576040517f19b991a800000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b5092949350505050565b5f81471015610339576040517fcf4791810000000000000000000000000000000000000000000000000000000081524760048201526024810183905260440160405180910390fd5b763d602d80600a3d3981f3363d3d373d3d3d363d730000008460601b60e81c175f526e5af43d82803e903d91602b57fd5bf38460781b17602052826037600984f5905073ffffffffffffffffffffffffffffffffffffffff8116610152576040517fb06ebf3d00000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b803573ffffffffffffffffffffffffffffffffffffffff811681146103ec575f80fd5b919050565b5f805f8060608587031215610404575f80fd5b61040d856103c9565b9350602085013567ffffffffffffffff80821115610429575f80fd5b818701915087601f83011261043c575f80fd5b81358181111561044a575f80fd5b88602082850101111561045b575f80fd5b95986020929092019750949560400135945092505050565b5f805f60608486031215610485575f80fd5b61048e846103c9565b9250602084013591506104a3604085016103c9565b90509250925092565b81835281816020850137505f602082840101525f60207fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0601f840116840101905092915050565b5f73ffffffffffffffffffffffffffffffffffffffff8089168352808816602084015280871660408401525084606083015260a0608083015261053a60a0830184866104ac565b98975050505050505050565b602081525f61014f6020830184866104ac565b5f60208284031215610569575f80fd5b505191905056"; + hex"608060405234801561000f575f80fd5b506004361061004a575f3560e01c806340375cb71461004e57806340419eec1461008a57806393a7e7111461009d578063fc90f455146100b0575b5f80fd5b61006161005c36600461043d565b61012e565b60405173ffffffffffffffffffffffffffffffffffffffff909116815260200160405180910390f35b61006161009836600461043d565b61015c565b6100616100ab3660046104bf565b610183565b6100616100be3660046104f8565b6040513060388201526f5af43d82803e903d91602b57fd5bf3ff60248201526014810192909252733d602d80600a3d3981f3363d3d373d3d3d363d73825260588201526037600c8201206078820152605560439091012073ffffffffffffffffffffffffffffffffffffffff1690565b5f610138856101a5565b5f61014386846101f8565b90506101528682878787610204565b9695505050505050565b5f610166856101a5565b5f6101438661017e33865f9182526020526040902090565b6101f8565b5f61019b846100be84865f9182526020526040902090565b90505b9392505050565b8073ffffffffffffffffffffffffffffffffffffffff163b5f036101f5576040517ff432283200000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b50565b5f61019e83835f61033d565b5f7f3b3e5b48cfaf4a4b3b2b36425ddec21c2c507fd502d2aea55eb7ffa36b0ca20533878785888860405161023e96959493929190610567565b60405180910390a16040517f439fab910000000000000000000000000000000000000000000000000000000081527fe0e57eda3f08f2a93bbe980d3df7f9c315eac41181f58b865a13d917fe769fc39073ffffffffffffffffffffffffffffffffffffffff87169063439fab91906102bc90889088906004016105ba565b6020604051808303815f875af11580156102d8573d5f803e3d5ffd5b505050506040513d601f19601f820116820180604052508101906102fc91906105cd565b14610333576040517f19b991a800000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b5092949350505050565b5f81471015610385576040517fcf4791810000000000000000000000000000000000000000000000000000000081524760048201526024810183905260440160405180910390fd5b763d602d80600a3d3981f3363d3d373d3d3d363d730000008460601b60e81c175f526e5af43d82803e903d91602b57fd5bf38460781b17602052826037600984f5905073ffffffffffffffffffffffffffffffffffffffff811661019e576040517fb06ebf3d00000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b803573ffffffffffffffffffffffffffffffffffffffff81168114610438575f80fd5b919050565b5f805f8060608587031215610450575f80fd5b61045985610415565b9350602085013567ffffffffffffffff80821115610475575f80fd5b818701915087601f830112610488575f80fd5b813581811115610496575f80fd5b8860208285010111156104a7575f80fd5b95986020929092019750949560400135945092505050565b5f805f606084860312156104d1575f80fd5b6104da84610415565b9250602084013591506104ef60408501610415565b90509250925092565b5f8060408385031215610509575f80fd5b61051283610415565b946020939093013593505050565b81835281816020850137505f602082840101525f60207fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0601f840116840101905092915050565b5f73ffffffffffffffffffffffffffffffffffffffff8089168352808816602084015280871660408401525084606083015260a060808301526105ae60a083018486610520565b98975050505050505050565b602081525f61019b602083018486610520565b5f602082840312156105dd575f80fd5b505191905056"; diff --git a/test/src/concrete/CloneFactoryCloneDeterministic.t.sol b/test/src/concrete/CloneFactoryCloneDeterministic.t.sol index bf8572e..0bf4be9 100644 --- a/test/src/concrete/CloneFactoryCloneDeterministic.t.sol +++ b/test/src/concrete/CloneFactoryCloneDeterministic.t.sol @@ -6,7 +6,7 @@ import {Test, Vm} from "forge-std-1.16.1/src/Test.sol"; import {Clones} from "@openzeppelin-contracts-5.6.1/proxy/Clones.sol"; import {LibExtrospectERC1167Proxy} from "rain-extrospection-0.1.1/src/lib/LibExtrospectERC1167Proxy.sol"; -import {ICLONEABLE_V2_SUCCESS} from "rain-factory-0.1.5/src/interface/ICloneableV2.sol"; +import {ICLONEABLE_V2_SUCCESS} from "rain-factory-0.1.7/src/interface/ICloneableV2.sol"; import {CloneFactory, ZeroImplementationCodeSize, InitializationFailed} from "../../../src/concrete/CloneFactory.sol"; import {TestCloneable} from "./TestCloneable.sol"; import {TestCloneableFailure} from "./TestCloneableFailure.sol"; diff --git a/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol b/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol new file mode 100644 index 0000000..a38afa1 --- /dev/null +++ b/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol @@ -0,0 +1,219 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {Test, Vm} from "forge-std-1.16.1/src/Test.sol"; + +import {Clones} from "@openzeppelin-contracts-5.6.1/proxy/Clones.sol"; +import {Errors} from "@openzeppelin-contracts-5.6.1/utils/Errors.sol"; +import {LibExtrospectERC1167Proxy} from "rain-extrospection-0.1.1/src/lib/LibExtrospectERC1167Proxy.sol"; +import {ICLONEABLE_V2_SUCCESS} from "rain-factory-0.1.7/src/interface/ICloneableV2.sol"; +import {CloneFactory, ZeroImplementationCodeSize, InitializationFailed} from "../../../src/concrete/CloneFactory.sol"; +import {TestCloneable} from "./TestCloneable.sol"; +import {TestCloneableFailure} from "./TestCloneableFailure.sol"; + +/// @title CloneFactoryCloneDeterministicOpenSaltTest +/// @notice A test suite for `CloneFactory`'s `cloneDeterministicOpenSalt` / +/// `predictDeterministicAddressOpenSalt` functions. The defining property is +/// that the deployer is NOT in the address derivation, which is the exact +/// opposite of what `cloneDeterministic` guarantees, so the two derivations are +/// also tested against each other here. +contract CloneFactoryCloneDeterministicOpenSaltTest is Test { + /// The `CloneFactory` instance under test. Stateless, so reused everywhere. + CloneFactory internal immutable I_CLONE_FACTORY; + + constructor() { + I_CLONE_FACTORY = new CloneFactory(); + } + + /// The `CREATE2` salt is the caller-supplied salt VERBATIM — no hashing, no + /// namespacing, nothing mixed in. Pins the derivation against OZ's own + /// prediction under the raw salt, so an off-chain caller can reproduce the + /// address from `(implementation, salt, factory)` alone. + function testCloneDeterministicOpenSaltSaltIsVerbatim(address implementation, bytes32 salt) external view { + address expected = Clones.predictDeterministicAddress(implementation, salt, address(I_CLONE_FACTORY)); + assertEq(I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, salt), expected); + } + + /// The deployed clone lands at the predicted address, is an EIP1167 proxy of + /// the implementation, and is initialized with the data. `predict` therefore + /// lets a caller pin the address before deploying. + function testCloneDeterministicOpenSaltMatchesPredict(bytes32 salt, bytes memory data) external { + TestCloneable implementation = new TestCloneable(); + + address predicted = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), salt); + address child = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), data, salt); + + assertEq(child, predicted); + (bool isProxy, address proxyImplementation) = LibExtrospectERC1167Proxy.isERC1167Proxy(child.code); + assertEq(isProxy, true); + assertEq(proxyImplementation, address(implementation)); + assertEq(TestCloneable(child).sData(), data); + } + + /// THE POINT OF THIS VARIANT. The same `(implementation, salt)` from two + /// different callers lands on the SAME address. State is snapshotted and + /// rolled back between the two deploys so both callers genuinely deploy from + /// the same starting state — the addresses are compared, not merely + /// predicted. This is exactly what `cloneDeterministic` forbids, so an + /// address deployed here survives its original deployer being retired: any + /// other account can re-establish it on another chain. + function testCloneDeterministicOpenSaltCallerIndependent( + bytes32 salt, + bytes memory data, + address alice, + address bob + ) external { + vm.assume(alice != bob); + TestCloneable implementation = new TestCloneable(); + + address predicted = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), salt); + + uint256 snapshot = vm.snapshotState(); + + vm.prank(alice); + address childAlice = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), data, salt); + + vm.revertToState(snapshot); + + vm.prank(bob); + address childBob = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), data, salt); + + assertEq(childAlice, childBob); + assertEq(childAlice, predicted); + } + + /// The prediction takes no deployer, so it cannot vary with one. Predicting + /// the same `(implementation, salt)` from two different callers returns the + /// same address — a caller pinning an address offchain does not need to know + /// who will deploy it. + function testCloneDeterministicOpenSaltPredictCallerIndependent( + address implementation, + bytes32 salt, + address alice, + address bob + ) external { + vm.assume(alice != bob); + + vm.prank(alice); + address predictedAlice = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, salt); + + vm.prank(bob); + address predictedBob = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, salt); + + assertEq(predictedAlice, predictedBob); + } + + /// The two derivations are disjoint: for any `(implementation, salt, + /// deployer)` the open-salt address is not the sender-namespaced address. + /// So adding the open variant cannot reach, block or collide with an address + /// that `cloneDeterministic` promised to a specific caller. + function testCloneDeterministicOpenSaltDiffersFromSenderNamespaced( + address implementation, + bytes32 salt, + address deployer + ) external view { + address open = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, salt); + address namespaced = I_CLONE_FACTORY.predictDeterministicAddress(implementation, salt, deployer); + assertTrue(open != namespaced); + } + + /// REGRESSION GUARD on the guarantee that must not break. Taking a salt via + /// the open variant does not consume it for `cloneDeterministic`: the same + /// caller can still deploy at the same `salt` through the namespaced + /// derivation, at the address it always predicted, and both clones exist + /// independently. + function testCloneDeterministicOpenSaltDoesNotConsumeNamespacedSalt(bytes32 salt, bytes memory data) external { + TestCloneable implementation = new TestCloneable(); + + address predictedNamespaced = + I_CLONE_FACTORY.predictDeterministicAddress(address(implementation), salt, address(this)); + + address childOpen = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), data, salt); + address childNamespaced = I_CLONE_FACTORY.cloneDeterministic(address(implementation), data, salt); + + assertEq(childNamespaced, predictedNamespaced); + assertTrue(childOpen != childNamespaced); + assertTrue(childOpen.code.length > 0); + assertTrue(childNamespaced.code.length > 0); + } + + /// Distinct salts yield distinct clones of the same implementation — many + /// clones per impl, as with the namespaced variant. + function testCloneDeterministicOpenSaltManyClonesPerImpl(bytes32 salt1, bytes32 salt2, bytes memory data) external { + vm.assume(salt1 != salt2); + TestCloneable implementation = new TestCloneable(); + + address child1 = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), data, salt1); + address child2 = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), data, salt2); + assertTrue(child1 != child2); + } + + /// A second deploy at an already-taken open salt REVERTS. It does not + /// silently return the existing clone, so a caller can never mistake + /// somebody else's already-initialized contract for their own fresh deploy. + /// Squatting is therefore loud at the point of deploy, even though it is + /// unrecoverable after it. + function testCloneDeterministicOpenSaltSecondDeployReverts( + bytes32 salt, + bytes memory dataFirst, + bytes memory dataSecond, + address alice, + address bob + ) external { + vm.assume(alice != bob); + TestCloneable implementation = new TestCloneable(); + + vm.prank(alice); + address child = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), dataFirst, salt); + + vm.prank(bob); + vm.expectRevert(abi.encodeWithSelector(Errors.FailedDeployment.selector)); + I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), dataSecond, salt); + + // The first deploy's state is untouched by the failed second one. + assertEq(TestCloneable(child).sData(), dataFirst); + } + + /// `NewClone` is emitted with the caller, implementation, child, salt and + /// data. The event is shared with `cloneDeterministic` and carries the RAW + /// salt in both cases, so an indexer must know which entry point was called + /// to recompute the address — the `clone` field is the authoritative address. + function testCloneDeterministicOpenSaltEvent(bytes32 salt, bytes memory data) external { + TestCloneable implementation = new TestCloneable(); + + vm.recordLogs(); + address child = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), data, salt); + Vm.Log[] memory entries = vm.getRecordedLogs(); + + assertEq(entries.length, 1); + assertEq(entries[0].topics[0], bytes32(uint256(keccak256("NewClone(address,address,address,bytes32,bytes)")))); + assertEq(entries[0].data, abi.encode(address(this), address(implementation), child, salt, data)); + } + + /// An implementation that initializes to a non-success code reverts + /// `InitializationFailed`, so clone-and-initialize stays atomic and the + /// address is left free rather than occupied by an uninitialized clone. + function testCloneDeterministicOpenSaltInitializeFailureFails(bytes32 notSuccess, bytes32 salt) external { + vm.assume(notSuccess != ICLONEABLE_V2_SUCCESS); + TestCloneableFailure implementation = new TestCloneableFailure(); + + address predicted = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), salt); + + vm.expectRevert(abi.encodeWithSelector(InitializationFailed.selector)); + I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), abi.encode(notSuccess), salt); + + assertEq(predicted.code.length, 0); + } + + /// A zero-code implementation reverts `ZeroImplementationCodeSize`. + function testCloneDeterministicOpenSaltZeroImplementationCodeSize( + address implementation, + bytes memory data, + bytes32 salt + ) external { + vm.assume(implementation.code.length == 0); + vm.expectRevert(abi.encodeWithSelector(ZeroImplementationCodeSize.selector)); + I_CLONE_FACTORY.cloneDeterministicOpenSalt(implementation, data, salt); + } +} diff --git a/test/src/concrete/TestCloneable.sol b/test/src/concrete/TestCloneable.sol index b5cc3bb..7262310 100644 --- a/test/src/concrete/TestCloneable.sol +++ b/test/src/concrete/TestCloneable.sol @@ -2,7 +2,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd pragma solidity =0.8.25; -import {ICloneableV2, ICLONEABLE_V2_SUCCESS} from "rain-factory-0.1.5/src/interface/ICloneableV2.sol"; +import {ICloneableV2, ICLONEABLE_V2_SUCCESS} from "rain-factory-0.1.7/src/interface/ICloneableV2.sol"; /// @title TestCloneable /// @notice A cloneable contract that implements `ICloneableV2`. Initializes diff --git a/test/src/concrete/TestCloneableFailure.sol b/test/src/concrete/TestCloneableFailure.sol index 35c2398..7795b02 100644 --- a/test/src/concrete/TestCloneableFailure.sol +++ b/test/src/concrete/TestCloneableFailure.sol @@ -2,7 +2,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd pragma solidity =0.8.25; -import {ICloneableV2} from "rain-factory-0.1.5/src/interface/ICloneableV2.sol"; +import {ICloneableV2} from "rain-factory-0.1.7/src/interface/ICloneableV2.sol"; /// @title TestCloneableFailure /// @notice A cloneable contract that implements `ICloneableV2` but always diff --git a/test/src/lib/LibCloneFactoryDeployCandidate.t.sol b/test/src/lib/LibCloneFactoryDeployCandidate.t.sol index e41d4dd..43a08a5 100644 --- a/test/src/lib/LibCloneFactoryDeployCandidate.t.sol +++ b/test/src/lib/LibCloneFactoryDeployCandidate.t.sol @@ -6,6 +6,7 @@ import {Test} from "forge-std-1.16.1/src/Test.sol"; import {LibRainDeploy} from "rain-deploy-0.1.3/src/lib/LibRainDeploy.sol"; import {LibCloneFactoryDeploy} from "../../../src/lib/LibCloneFactoryDeploy.sol"; import {CloneFactory} from "../../../src/concrete/CloneFactory.sol"; +import {TestCloneable} from "../concrete/TestCloneable.sol"; import { BYTECODE_HASH as CLONE_FACTORY_BYTECODE_HASH_CANDIDATE, DEPLOYED_ADDRESS as CLONE_FACTORY_DEPLOYED_ADDRESS_CANDIDATE, @@ -61,4 +62,35 @@ contract LibCloneFactoryDeployCandidateTest is Test { assertEq(deployed.codehash, CLONE_FACTORY_BYTECODE_HASH_CANDIDATE); assertEq(keccak256(deployed.code), CLONE_FACTORY_BYTECODE_HASH_CANDIDATE); } + + /// The candidate's recorded bytecode must actually SERVE all four + /// deterministic entry points, so the pin cannot record an address for + /// bytecode that is missing one — which is what a frozen release copied from + /// this candidate would then publish. Proved by Zoltu-deploying the recorded + /// `CREATION_CODE` (NOT `new CloneFactory()`, so the assertion is about the + /// snapshot rather than the source) and calling every entry point on the + /// result through the `ICloneableFactoryV4` ABI: an entry point the + /// dispatcher does not expose falls through to the (absent) fallback and + /// reverts here. A byte scan of the runtime code would NOT prove this — a + /// selector can sit in constant data without being dispatchable. + function testCandidateDeployedBytecodeServesBothEntryPoints() external { + LibRainDeploy.etchZoltuFactory(vm); + CloneFactory factory = CloneFactory(LibRainDeploy.deployZoltu(CLONE_FACTORY_CREATION_CODE_CANDIDATE)); + TestCloneable implementation = new TestCloneable(); + + bytes32 salt = keccak256("rain.factory.deploy.candidate.entry.points"); + bytes memory data = hex"f100dedb0a75"; + + address predictedNamespaced = factory.predictDeterministicAddress(address(implementation), salt, address(this)); + address predictedOpen = factory.predictDeterministicAddressOpenSalt(address(implementation), salt); + assertTrue(predictedNamespaced != predictedOpen); + + address childNamespaced = factory.cloneDeterministic(address(implementation), data, salt); + address childOpen = factory.cloneDeterministicOpenSalt(address(implementation), data, salt); + + assertEq(childNamespaced, predictedNamespaced); + assertEq(childOpen, predictedOpen); + assertEq(TestCloneable(childNamespaced).sData(), data); + assertEq(TestCloneable(childOpen).sData(), data); + } } From d9a09f19d338d27fbd73f1f86c54c1beac7465d7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 22:11:44 +0000 Subject: [PATCH 2/3] feat: open-salt derivation hashes data under a domain separator 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) --- .gas-snapshot | 46 ++-- CLAUDE.md | 29 ++- README.md | 75 ++++-- src/concrete/CloneFactory.sol | 79 +++++-- .../candidate/CloneFactory.pointers.sol | 8 +- ...oneFactoryCloneDeterministicOpenSalt.t.sol | 215 ++++++++++++++---- .../lib/LibCloneFactoryDeployCandidate.t.sol | 2 +- 7 files changed, 334 insertions(+), 120 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index 7f83fb9..d947b7f 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,31 +1,33 @@ -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltCallerIndependent(bytes32,bytes,address,address) (runs: 2048, μ: 582462, ~: 439968) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDiffersFromSenderNamespaced(address,bytes32,address) (runs: 2048, μ: 5400, ~: 5400) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDoesNotConsumeNamespacedSalt(bytes32,bytes) (runs: 2048, μ: 574640, ~: 436159) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltEvent(bytes32,bytes) (runs: 2048, μ: 418423, ~: 349063) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltInitializeFailureFails(bytes32,bytes32) (runs: 2048, μ: 164888, ~: 164888) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltManyClonesPerImpl(bytes32,bytes32,bytes) (runs: 2048, μ: 568887, ~: 437277) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltMatchesPredict(bytes32,bytes) (runs: 2048, μ: 419757, ~: 349902) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltPredictCallerIndependent(address,bytes32,address,address) (runs: 2048, μ: 8766, ~: 8766) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltSaltIsVerbatim(address,bytes32) (runs: 2048, μ: 4262, ~: 4262) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltSecondDeployReverts(bytes32,bytes,bytes,address,address) (runs: 2048, μ: 1040442091, ~: 1040442127) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltZeroImplementationCodeSize(address,bytes,bytes32) (runs: 2048, μ: 10701, ~: 10678) -CloneFactoryCloneDeterministicTest:testCloneDeterministicEvent(bytes32,bytes) (runs: 2048, μ: 418483, ~: 349123) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltCallerIndependent(bytes32,bytes,address,address) (runs: 2048, μ: 589533, ~: 486442) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDataInDerivation(bytes32,bytes,bytes) (runs: 2048, μ: 572082, ~: 471101) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDiffersFromSenderNamespaced(address,bytes,bytes32,bytes32,address) (runs: 2048, μ: 6747, ~: 6672) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDisjointFromNamespacedAtLeftPaddedAddressSalt(address,bytes,bytes) (runs: 2048, μ: 568064, ~: 470240) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDoesNotConsumeNamespacedSalt(bytes32,bytes) (runs: 2048, μ: 579815, ~: 436545) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltEvent(bytes32,bytes) (runs: 2048, μ: 421178, ~: 349404) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltInitializeFailureFails(bytes32,bytes32) (runs: 2048, μ: 166138, ~: 166138) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltIsDomainTaggedHash(address,bytes,bytes32) (runs: 2048, μ: 5884, ~: 5792) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltManyClonesPerImpl(bytes32,bytes32,bytes) (runs: 2048, μ: 575136, ~: 438028) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltMatchesPredict(bytes32,bytes) (runs: 2048, μ: 423460, ~: 351127) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltPredictCallerIndependent(address,bytes,bytes32,address,address) (runs: 2048, μ: 10987, ~: 10918) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltSecondDeployReverts(bytes32,bytes,address,address) (runs: 2048, μ: 1040444148, ~: 1040443103) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltZeroImplementationCodeSize(address,bytes,bytes32) (runs: 2048, μ: 10659, ~: 10634) +CloneFactoryCloneDeterministicTest:testCloneDeterministicEvent(bytes32,bytes) (runs: 2048, μ: 420866, ~: 349123) CloneFactoryCloneDeterministicTest:testCloneDeterministicInitializeFailureFails(bytes32,bytes32) (runs: 2048, μ: 161288, ~: 161288) -CloneFactoryCloneDeterministicTest:testCloneDeterministicManyClonesPerImpl(bytes32,bytes32,bytes) (runs: 2048, μ: 569073, ~: 437463) -CloneFactoryCloneDeterministicTest:testCloneDeterministicMatchesPredict(bytes32,bytes) (runs: 2048, μ: 420117, ~: 350262) +CloneFactoryCloneDeterministicTest:testCloneDeterministicManyClonesPerImpl(bytes32,bytes32,bytes) (runs: 2048, μ: 574513, ~: 437463) +CloneFactoryCloneDeterministicTest:testCloneDeterministicMatchesPredict(bytes32,bytes) (runs: 2048, μ: 422516, ~: 350262) CloneFactoryCloneDeterministicTest:testCloneDeterministicSaltIsAbiEncodeHash(address,bytes32,address) (runs: 2048, μ: 4708, ~: 4708) -CloneFactoryCloneDeterministicTest:testCloneDeterministicSenderScoped(bytes32,bytes,address,address) (runs: 2048, μ: 583541, ~: 441047) -CloneFactoryCloneDeterministicTest:testCloneDeterministicZeroImplementationCodeSize(address,bytes,bytes32) (runs: 2048, μ: 10717, ~: 10700) -LibCloneFactoryDeployCandidateTest:testCandidateCreationDeploysToPinnedAddress() (gas: 339992) -LibCloneFactoryDeployCandidateTest:testCandidateDeployedBytecodeServesBothEntryPoints() (gas: 733209) +CloneFactoryCloneDeterministicTest:testCloneDeterministicSenderScoped(bytes32,bytes,address,address) (runs: 2048, μ: 588912, ~: 485903) +CloneFactoryCloneDeterministicTest:testCloneDeterministicZeroImplementationCodeSize(address,bytes,bytes32) (runs: 2048, μ: 10715, ~: 10700) +LibCloneFactoryDeployCandidateTest:testCandidateCreationDeploysToPinnedAddress() (gas: 366569) +LibCloneFactoryDeployCandidateTest:testCandidateDeployedBytecodeServesBothEntryPoints() (gas: 760962) LibCloneFactoryDeployCandidateTest:testCandidateIsTheAliasedSnapshot() (gas: 3708) -LibCloneFactoryDeployCandidateTest:testCandidateRuntimeHashesToBytecodeHash() (gas: 928) -LibCloneFactoryDeployCandidateTest:testCandidateSelfConsistent() (gas: 4791) +LibCloneFactoryDeployCandidateTest:testCandidateRuntimeHashesToBytecodeHash() (gas: 977) +LibCloneFactoryDeployCandidateTest:testCandidateSelfConsistent() (gas: 4924) LibCloneFactoryDeployTaggedConstantsTest:testCloneFactory_0_1_3_CreationDeploysToPinnedAddress() (gas: 240145) LibCloneFactoryDeployTaggedConstantsTest:testCloneFactory_0_1_3_RuntimeHashesToBytecodeHash() (gas: 755) LibCloneFactoryDeployTaggedConstantsTest:testCloneFactory_0_1_4_CreationDeploysToPinnedAddress() (gas: 240146) LibCloneFactoryDeployTaggedConstantsTest:testCloneFactory_0_1_4_RuntimeHashesToBytecodeHash() (gas: 755) LibCloneFactoryDeployTaggedConstantsTest:testCloneFactory_0_1_5_CreationDeploysToPinnedAddress() (gas: 316590) LibCloneFactoryDeployTaggedConstantsTest:testCloneFactory_0_1_5_RuntimeHashesToBytecodeHash() (gas: 922) -LibCloneFactoryDeployTest:testDeployAddress() (gas: 339275) -LibCloneFactoryDeployTest:testExpectedCodeHash() (gas: 334816) \ No newline at end of file +LibCloneFactoryDeployTest:testDeployAddress() (gas: 365802) +LibCloneFactoryDeployTest:testExpectedCodeHash() (gas: 361282) \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md index bff5632..44b5791 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,17 +74,32 @@ as the `rain-factory` Soldeer dependency, so they are read under - `cloneDeterministic` / `predictDeterministicAddress` (declared on `ICloneableFactoryV3`, which V4 extends) namespace the caller-supplied salt by `msg.sender` via `_effectiveSalt`, so a caller's `(implementation, salt)` - address cannot be reached by another account. - - `cloneDeterministicOpenSalt` / `predictDeterministicAddressOpenSalt` pass - the caller-supplied salt verbatim, so the address carries no identity and - anyone can deploy it. Only safe for implementations whose `initialize` - takes no caller-controlled authority — the NatSpec on - `ICloneableFactoryV4.cloneDeterministicOpenSalt` is the spec for that - condition, and it lives in rain.factory, not here. + address cannot be reached by another account. `data` is outside that + derivation. + - `cloneDeterministicOpenSalt` / `predictDeterministicAddressOpenSalt` derive + the salt via `_effectiveOpenSalt` as + `keccak256(abi.encode(ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN, salt, keccak256(data)))`, + so the address carries no identity and anyone can deploy it, but everyone + who does deploys the same contract initialized with the same bytes. + `predictDeterministicAddressOpenSalt` therefore takes `data` — it is one of + the derivation's inputs. The residual condition on implementations + (`initialize` MUST NOT read `tx.origin`) is specified by the NatSpec on + `ICloneableFactoryV4.cloneDeterministicOpenSalt`, which lives in + rain.factory, not here. Both share `_requireImplementationCode` and `_initializeClone`, so clone-and-initialize is atomic and the failure modes are identical across the two. + + **The two salt derivations MUST have disjoint images**, and `CloneFactory` is + where `ICloneableFactoryV4`'s MUST NOT on the factory is actually held: 96 + bytes led by the domain constant versus 64 bytes led by a left-padded address. + Drop the domain word and any account `A` reaches every open-salt address whose + `salt` equals `bytes32(uint256(uint160(A)))` via `cloneDeterministic` with + arbitrary `data`. Do not add a third entry point that hashes to either shape, + and do not change the shape of either preimage. + `testCloneDeterministicOpenSaltDisjointFromNamespacedAtLeftPaddedAddressSalt` + is the gate. - `src/lib/LibCloneFactoryDeploy.sol` — Deterministic deployment address and codehash constants (generated; aliases the rolling `src/generated/candidate/` snapshot). diff --git a/README.md b/README.md index b9ed8c6..54e49fc 100644 --- a/README.md +++ b/README.md @@ -18,22 +18,50 @@ atomically. It offers two deterministic (`CREATE2`) entry points that differ only in how the salt is derived: - `cloneDeterministic` namespaces the caller-supplied salt by `msg.sender`, so - nobody else can reach the caller's address — but the deploying account is - baked into that address forever. -- `cloneDeterministicOpenSalt` uses the caller-supplied salt verbatim, so the - address is a function of `(implementation, salt)` and the factory alone: every + the address commits to WHO deployed: nobody else can reach the caller's + address, but the deploying account is baked into it forever and `data` is + outside the derivation. +- `cloneDeterministicOpenSalt` derives the salt as + `keccak256(abi.encode(ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN, salt, keccak256(data)))`, + so the address commits to WHAT was deployed and to nothing about who deployed + it: it is a function of `(factory, implementation, salt, data)` alone. Every account reaches the same address, and so can anyone. That also makes it the same address across chains, but only where both the factory and the implementation are themselves at the same address on each chain — `CREATE2` hashes the factory, and the EIP1167 creation code it hashes contains the implementation. -Open-salt is ONLY safe for implementations whose `initialize` takes no -caller-controlled authority: clone-and-initialize is atomic and runs once, so -the first deployer's `data` sets the clone's authority permanently, with no -recovery. Read the NatSpec on -`ICloneableFactoryV4.cloneDeterministicOpenSalt` (in `rain.factory`) before -using it — the qualifying condition is stated there, not here. +Because `data` is in the derivation, open-salt needs no per-implementation audit +of what a squatter could pass. A front-runner who passes different `data` +derives a different address and has deployed their own contract at their own +expense; one who passes the same `data` has deployed exactly the intended +contract with the intended bytes and has paid the gas for it. What is left is +that the address cannot fix what `initialize` reads that is not `data`, so an +implementation used this way MUST NOT read `tx.origin`. The full statement of +that condition and of the residual timing lever is the NatSpec on +`ICloneableFactoryV4.cloneDeterministicOpenSalt` (in `rain.factory`), not here. +The cost open-salt does carry is that the address is not knowable until `data` +is final, and a consumer pinning one must be able to reproduce those bytes +exactly, ABI encoding and all. + +### The domain separator is load-bearing + +`ICloneableFactoryV4` states the disjointness of the two derivations as a MUST +NOT on the **factory**: no other entry point may `CREATE2` in the open-salt +image with caller-supplied `data`. `cloneDeterministic` is exactly such an entry +point, so `CloneFactory` holds the rule structurally — a 96-byte preimage led by +the domain constant against a 64-byte preimage led by a left-padded address. + +Without the domain word both preimages would be 64 bytes led by a word the +caller chooses, and since `abi.encode` left-pads an address into the same word a +`bytes32` salt already is, any account `A` would reach every open-salt address +whose `salt` equals `bytes32(uint256(uint160(A)))` by calling +`cloneDeterministic(implementation, evilData, keccak256(data))` — a choice of +salt, not a preimage search. +`testCloneDeterministicOpenSaltDisjointFromNamespacedAtLeftPaddedAddressSalt` is +the test that fails if that ever stops holding: it builds that exact squat, +asserts against the factory's own namespaced prediction that an untagged +derivation would land on it, and then shows the real one does not. ## Snapshots @@ -72,19 +100,20 @@ revisions. `sol-v0.1.6` exists as a tag on `685bb2ba`. Its `rainix-tag-release` run ([30097157490](https://github.com/rainlanguage/rain.factory.deploy/actions/runs/30097157490)) -got as far as *Verify live chain matches the fresh pins* and died there — all -five fork tests failed with `vm.createSelectFork: environment variable -_RPC_URL not found`. The reusable exported the fork endpoints under the -**secret** names (`RPC_URL__FORK`), while `[rpc_endpoints]` in -`foundry.toml` reads `${_RPC_URL}`, so every endpoint resolved to an -empty string. Publish, commit-back and GitHub Release were all skipped, which is -why the tag exists with no revision, no release and no `0_1_6` snapshot behind -it. +got as far as _Verify live chain matches the fresh pins_ and died there — all +five fork tests failed with +`vm.createSelectFork: environment variable +_RPC_URL not found`. The +reusable exported the fork endpoints under the **secret** names +(`RPC_URL__FORK`), while `[rpc_endpoints]` in `foundry.toml` reads +`${_RPC_URL}`, so every endpoint resolved to an empty string. Publish, +commit-back and GitHub Release were all skipped, which is why the tag exists +with no revision, no release and no `0_1_6` snapshot behind it. That was a defect in `rainix-tag-release`, not in this repo, and it is fixed upstream: `rainix` now runs an `rpc-preflight` step that binds each env name -foundry actually reads to an endpoint probed healthy at that moment. The next tag -does not hit this. +foundry actually reads to an endpoint probed healthy at that moment. The next +tag does not hit this. Two consequences for whoever cuts the first release: @@ -95,8 +124,8 @@ Two consequences for whoever cuts the first release: - **The fork RPCs still gate the release.** The verify step is the repo's own fork suite, so a release only publishes if the pins resolve on every supported chain. Those endpoints are currently intermittent (a free-plan `lb.drpc.live` - returning quota and 408 errors), which reds the same suite on ordinary PRs. Get - them healthy before tagging: a transient failure here fails the release, and - the fix is to tag again, not to retry the run. + returning quota and 408 errors), which reds the same suite on ordinary PRs. + Get them healthy before tagging: a transient failure here fails the release, + and the fix is to tag again, not to retry the run. See rainlanguage/rain.factory#46 for the split rationale. diff --git a/src/concrete/CloneFactory.sol b/src/concrete/CloneFactory.sol index 9845644..b438ac0 100644 --- a/src/concrete/CloneFactory.sol +++ b/src/concrete/CloneFactory.sol @@ -7,7 +7,10 @@ import {ICloneableV2, ICLONEABLE_V2_SUCCESS} from "rain-factory-0.1.7/src/interf // functions it declares; `ICloneableFactoryV4` inherits rather than redeclares // them, so the tag must name V3 and V3 must be in scope here. import {ICloneableFactoryV3} from "rain-factory-0.1.7/src/interface/ICloneableFactoryV3.sol"; -import {ICloneableFactoryV4} from "rain-factory-0.1.7/src/interface/ICloneableFactoryV4.sol"; +import { + ICloneableFactoryV4, + ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN +} from "rain-factory-0.1.7/src/interface/ICloneableFactoryV4.sol"; import {Clones} from "@openzeppelin-contracts-5.6.1/proxy/Clones.sol"; /// Thrown when an implementation has zero code size which is always a mistake. @@ -21,16 +24,38 @@ error InitializationFailed(); /// Open Zeppelin `Clones` to create EIP1167 clones of a reference bytecode. The /// reference bytecode MUST implement `ICloneableV2`. /// -/// Two deterministic entry points, both `CREATE2`, differing only in the salt: +/// Two deterministic entry points, both `CREATE2`, differing only in the salt +/// they derive and therefore in what the clone's address commits to: /// /// - `cloneDeterministic` / `predictDeterministicAddress` namespace the -/// caller-supplied salt by `msg.sender` (see `_effectiveSalt`) so a caller's -/// `(implementation, salt)` address cannot be squatted by another account. -/// - `cloneDeterministicOpenSalt` / `predictDeterministicAddressOpenSalt` use -/// the caller-supplied salt verbatim, so the address carries no identity and -/// anyone can deploy it. This is only safe for implementations whose -/// `initialize` takes no caller-controlled authority — read the warning on -/// `ICloneableFactoryV4.cloneDeterministicOpenSalt` before using it. +/// caller-supplied salt by `msg.sender` (see `_effectiveSalt`), so the +/// address commits to WHO deployed and a caller's `(implementation, salt)` +/// address cannot be squatted by another account. `data` is outside the +/// derivation. +/// - `cloneDeterministicOpenSalt` / `predictDeterministicAddressOpenSalt` hash +/// the caller-supplied salt together with `data` under a fixed domain +/// separator (see `_effectiveOpenSalt`), so the address commits to WHAT was +/// deployed and to nothing caller-derived. Anyone can deploy it, and everyone +/// who does deploys the same contract initialized with the same bytes, +/// because varying either input lands somewhere else. +/// +/// # The two images MUST be disjoint, and this contract is where that holds +/// +/// `ICloneableFactoryV4` states as a MUST NOT on the factory that no other +/// entry point may `CREATE2` at an effective salt in the open-salt derivation's +/// image with caller-supplied `data`. `cloneDeterministic` is exactly such an +/// entry point, so the separation is structural rather than incidental: +/// `_effectiveSalt` hashes 64 bytes whose first word is a left-padded address, +/// `_effectiveOpenSalt` hashes 96 bytes whose first word is +/// `ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN`. +/// +/// Drop the domain word and both preimages become 64 bytes led by a word an +/// attacker controls, at which point an account `A` reaches EVERY open-salt +/// address whose `salt` equals `bytes32(uint256(uint160(A)))` — the abi-encoding +/// of `A` — simply by calling `cloneDeterministic(implementation, evilData, +/// keccak256(data))`. No preimage search, just a choice of salt. That is what +/// the domain word buys, and it is why a third entry point that hashes to +/// either shape MUST NOT be added here. contract CloneFactory is ICloneableFactoryV4 { /// @inheritdoc ICloneableFactoryV3 function cloneDeterministic(address implementation, bytes calldata data, bytes32 salt) external returns (address) { @@ -55,16 +80,20 @@ contract CloneFactory is ICloneableFactoryV4 { returns (address) { _requireImplementationCode(implementation); - // CREATE2 clone at the caller-supplied salt verbatim: no `_effectiveSalt` - // namespacing, so the address is the same for every caller and there is - // no identity in the derivation. - address child = Clones.cloneDeterministic(implementation, salt); + // CREATE2 clone at a salt derived from `(salt, data)` and nothing + // caller-derived (see `_effectiveOpenSalt`), so every caller lands on + // the same address and every address pins its own `data`. + address child = Clones.cloneDeterministic(implementation, _effectiveOpenSalt(data, salt)); return _initializeClone(implementation, child, data, salt); } /// @inheritdoc ICloneableFactoryV4 - function predictDeterministicAddressOpenSalt(address implementation, bytes32 salt) external view returns (address) { - return Clones.predictDeterministicAddress(implementation, salt, address(this)); + function predictDeterministicAddressOpenSalt(address implementation, bytes calldata data, bytes32 salt) + external + view + returns (address) + { + return Clones.predictDeterministicAddress(implementation, _effectiveOpenSalt(data, salt), address(this)); } /// @dev The CREATE2 salt actually used: the caller-supplied `salt` namespaced @@ -73,6 +102,10 @@ contract CloneFactory is ICloneableFactoryV4 { /// single caller mint many clones of one implementation via distinct salts. /// Equal to `keccak256(abi.encode(deployer, salt))`, hashed directly in the /// scratch space; `deployer` is a clean address so it occupies a full word. + /// + /// The 64-byte preimage led by a left-padded address is half of the image + /// disjointness described on this contract — changing its shape is a change + /// to the open-salt guarantee as much as to this one. function _effectiveSalt(address deployer, bytes32 salt) internal pure returns (bytes32 effectiveSalt) { assembly ("memory-safe") { mstore(0, deployer) @@ -81,6 +114,22 @@ contract CloneFactory is ICloneableFactoryV4 { } } + /// @dev The CREATE2 salt actually used by the open-salt entry points, fixed + /// by `ICloneableFactoryV4` as + /// `keccak256(abi.encode(ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN, salt, keccak256(data)))` + /// so third parties can recompute it. Written as that expression rather + /// than as scratch-space assembly because the preimage is three words and + /// does not fit the scratch space. + /// + /// Hashing `data` in is what makes the deployer's absence safe: different + /// `data` is a different address, so a front-runner either deploys exactly + /// what was intended or deploys their own contract at their own expense + /// somewhere else. The domain word is what keeps this image disjoint from + /// `_effectiveSalt`'s — see the note on this contract. + function _effectiveOpenSalt(bytes calldata data, bytes32 salt) internal pure returns (bytes32) { + return keccak256(abi.encode(ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN, salt, keccak256(data))); + } + /// @dev Reverts with a clear error if `implementation` has no code. function _requireImplementationCode(address implementation) internal view { if (implementation.code.length == 0) { diff --git a/src/generated/candidate/CloneFactory.pointers.sol b/src/generated/candidate/CloneFactory.pointers.sol index 886a926..c7d1db0 100644 --- a/src/generated/candidate/CloneFactory.pointers.sol +++ b/src/generated/candidate/CloneFactory.pointers.sol @@ -10,16 +10,16 @@ pragma solidity ^0.8.25; // file needs the contract to exist so that it can be compiled. /// @dev Hash of the known bytecode. -bytes32 constant BYTECODE_HASH = bytes32(0x1a16009998834f07d5ccab032c39377f6528870eec5abd47817f9467187b4012); +bytes32 constant BYTECODE_HASH = bytes32(0xcdb039a288346702df9136f924d5b239d6f5fa01fef3273d76df928738c01cbd); /// @dev The deterministic deploy address of the contract when deployed via /// the Zoltu factory. -address constant DEPLOYED_ADDRESS = address(0x19272bCcFcb032eaC545E74ADFa168fDeD3e8d83); +address constant DEPLOYED_ADDRESS = address(0xe03b178055685EAC2Fd8F012B444E379dE70576a); /// @dev The creation bytecode of the contract. bytes constant CREATION_CODE = - hex"6080604052348015600e575f80fd5b506105e48061001c5f395ff3fe608060405234801561000f575f80fd5b506004361061004a575f3560e01c806340375cb71461004e57806340419eec1461008a57806393a7e7111461009d578063fc90f455146100b0575b5f80fd5b61006161005c36600461043d565b61012e565b60405173ffffffffffffffffffffffffffffffffffffffff909116815260200160405180910390f35b61006161009836600461043d565b61015c565b6100616100ab3660046104bf565b610183565b6100616100be3660046104f8565b6040513060388201526f5af43d82803e903d91602b57fd5bf3ff60248201526014810192909252733d602d80600a3d3981f3363d3d373d3d3d363d73825260588201526037600c8201206078820152605560439091012073ffffffffffffffffffffffffffffffffffffffff1690565b5f610138856101a5565b5f61014386846101f8565b90506101528682878787610204565b9695505050505050565b5f610166856101a5565b5f6101438661017e33865f9182526020526040902090565b6101f8565b5f61019b846100be84865f9182526020526040902090565b90505b9392505050565b8073ffffffffffffffffffffffffffffffffffffffff163b5f036101f5576040517ff432283200000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b50565b5f61019e83835f61033d565b5f7f3b3e5b48cfaf4a4b3b2b36425ddec21c2c507fd502d2aea55eb7ffa36b0ca20533878785888860405161023e96959493929190610567565b60405180910390a16040517f439fab910000000000000000000000000000000000000000000000000000000081527fe0e57eda3f08f2a93bbe980d3df7f9c315eac41181f58b865a13d917fe769fc39073ffffffffffffffffffffffffffffffffffffffff87169063439fab91906102bc90889088906004016105ba565b6020604051808303815f875af11580156102d8573d5f803e3d5ffd5b505050506040513d601f19601f820116820180604052508101906102fc91906105cd565b14610333576040517f19b991a800000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b5092949350505050565b5f81471015610385576040517fcf4791810000000000000000000000000000000000000000000000000000000081524760048201526024810183905260440160405180910390fd5b763d602d80600a3d3981f3363d3d373d3d3d363d730000008460601b60e81c175f526e5af43d82803e903d91602b57fd5bf38460781b17602052826037600984f5905073ffffffffffffffffffffffffffffffffffffffff811661019e576040517fb06ebf3d00000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b803573ffffffffffffffffffffffffffffffffffffffff81168114610438575f80fd5b919050565b5f805f8060608587031215610450575f80fd5b61045985610415565b9350602085013567ffffffffffffffff80821115610475575f80fd5b818701915087601f830112610488575f80fd5b813581811115610496575f80fd5b8860208285010111156104a7575f80fd5b95986020929092019750949560400135945092505050565b5f805f606084860312156104d1575f80fd5b6104da84610415565b9250602084013591506104ef60408501610415565b90509250925092565b5f8060408385031215610509575f80fd5b61051283610415565b946020939093013593505050565b81835281816020850137505f602082840101525f60207fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0601f840116840101905092915050565b5f73ffffffffffffffffffffffffffffffffffffffff8089168352808816602084015280871660408401525084606083015260a060808301526105ae60a083018486610520565b98975050505050505050565b602081525f61019b602083018486610520565b5f602082840312156105dd575f80fd5b505191905056"; + hex"6080604052348015600e575f80fd5b506106688061001c5f395ff3fe608060405234801561000f575f80fd5b506004361061004a575f3560e01c806340375cb71461004e57806340419eec1461008a57806393a7e7111461009d578063e921aceb146100b0575b5f80fd5b61006161005c3660046104da565b6100c3565b60405173ffffffffffffffffffffffffffffffffffffffff909116815260200160405180910390f35b6100616100983660046104da565b6100fb565b6100616100ab36600461055c565b61011d565b6100616100be3660046104da565b6101af565b5f6100cd856101c8565b5f6100e2866100dd87878761021b565b610295565b90506100f186828787876102a1565b9695505050505050565b5f610105856101c8565b5f6100e2866100dd33865f9182526020526040902090565b5f6101a58461013584865f9182526020526040902090565b6040513060388201526f5af43d82803e903d91602b57fd5bf3ff60248201526014810192909252733d602d80600a3d3981f3363d3d373d3d3d363d73825260588201526037600c8201206078820152605560439091012073ffffffffffffffffffffffffffffffffffffffff1690565b90505b9392505050565b5f6101bf8561013586868661021b565b95945050505050565b8073ffffffffffffffffffffffffffffffffffffffff163b5f03610218576040517ff432283200000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b50565b5f7f02d4347d074298c7b3cea4d31c1362e4d9a0a8801a18322f532336bd2c25b45982858560405161024e929190610595565b6040519081900381206102769392916020019283526020830191909152604082015260600190565b6040516020818303038152906040528051906020012090509392505050565b5f6101a883835f6103da565b5f7f3b3e5b48cfaf4a4b3b2b36425ddec21c2c507fd502d2aea55eb7ffa36b0ca2053387878588886040516102db969594939291906105eb565b60405180910390a16040517f439fab910000000000000000000000000000000000000000000000000000000081527fe0e57eda3f08f2a93bbe980d3df7f9c315eac41181f58b865a13d917fe769fc39073ffffffffffffffffffffffffffffffffffffffff87169063439fab9190610359908890889060040161063e565b6020604051808303815f875af1158015610375573d5f803e3d5ffd5b505050506040513d601f19601f820116820180604052508101906103999190610651565b146103d0576040517f19b991a800000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b5092949350505050565b5f81471015610422576040517fcf4791810000000000000000000000000000000000000000000000000000000081524760048201526024810183905260440160405180910390fd5b763d602d80600a3d3981f3363d3d373d3d3d363d730000008460601b60e81c175f526e5af43d82803e903d91602b57fd5bf38460781b17602052826037600984f5905073ffffffffffffffffffffffffffffffffffffffff81166101a8576040517fb06ebf3d00000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b803573ffffffffffffffffffffffffffffffffffffffff811681146104d5575f80fd5b919050565b5f805f80606085870312156104ed575f80fd5b6104f6856104b2565b9350602085013567ffffffffffffffff80821115610512575f80fd5b818701915087601f830112610525575f80fd5b813581811115610533575f80fd5b886020828501011115610544575f80fd5b95986020929092019750949560400135945092505050565b5f805f6060848603121561056e575f80fd5b610577846104b2565b92506020840135915061058c604085016104b2565b90509250925092565b818382375f9101908152919050565b81835281816020850137505f602082840101525f60207fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0601f840116840101905092915050565b5f73ffffffffffffffffffffffffffffffffffffffff8089168352808816602084015280871660408401525084606083015260a0608083015261063260a0830184866105a4565b98975050505050505050565b602081525f6101a56020830184866105a4565b5f60208284031215610661575f80fd5b505191905056"; /// @dev The runtime bytecode of the contract. bytes constant RUNTIME_CODE = - hex"608060405234801561000f575f80fd5b506004361061004a575f3560e01c806340375cb71461004e57806340419eec1461008a57806393a7e7111461009d578063fc90f455146100b0575b5f80fd5b61006161005c36600461043d565b61012e565b60405173ffffffffffffffffffffffffffffffffffffffff909116815260200160405180910390f35b61006161009836600461043d565b61015c565b6100616100ab3660046104bf565b610183565b6100616100be3660046104f8565b6040513060388201526f5af43d82803e903d91602b57fd5bf3ff60248201526014810192909252733d602d80600a3d3981f3363d3d373d3d3d363d73825260588201526037600c8201206078820152605560439091012073ffffffffffffffffffffffffffffffffffffffff1690565b5f610138856101a5565b5f61014386846101f8565b90506101528682878787610204565b9695505050505050565b5f610166856101a5565b5f6101438661017e33865f9182526020526040902090565b6101f8565b5f61019b846100be84865f9182526020526040902090565b90505b9392505050565b8073ffffffffffffffffffffffffffffffffffffffff163b5f036101f5576040517ff432283200000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b50565b5f61019e83835f61033d565b5f7f3b3e5b48cfaf4a4b3b2b36425ddec21c2c507fd502d2aea55eb7ffa36b0ca20533878785888860405161023e96959493929190610567565b60405180910390a16040517f439fab910000000000000000000000000000000000000000000000000000000081527fe0e57eda3f08f2a93bbe980d3df7f9c315eac41181f58b865a13d917fe769fc39073ffffffffffffffffffffffffffffffffffffffff87169063439fab91906102bc90889088906004016105ba565b6020604051808303815f875af11580156102d8573d5f803e3d5ffd5b505050506040513d601f19601f820116820180604052508101906102fc91906105cd565b14610333576040517f19b991a800000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b5092949350505050565b5f81471015610385576040517fcf4791810000000000000000000000000000000000000000000000000000000081524760048201526024810183905260440160405180910390fd5b763d602d80600a3d3981f3363d3d373d3d3d363d730000008460601b60e81c175f526e5af43d82803e903d91602b57fd5bf38460781b17602052826037600984f5905073ffffffffffffffffffffffffffffffffffffffff811661019e576040517fb06ebf3d00000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b803573ffffffffffffffffffffffffffffffffffffffff81168114610438575f80fd5b919050565b5f805f8060608587031215610450575f80fd5b61045985610415565b9350602085013567ffffffffffffffff80821115610475575f80fd5b818701915087601f830112610488575f80fd5b813581811115610496575f80fd5b8860208285010111156104a7575f80fd5b95986020929092019750949560400135945092505050565b5f805f606084860312156104d1575f80fd5b6104da84610415565b9250602084013591506104ef60408501610415565b90509250925092565b5f8060408385031215610509575f80fd5b61051283610415565b946020939093013593505050565b81835281816020850137505f602082840101525f60207fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0601f840116840101905092915050565b5f73ffffffffffffffffffffffffffffffffffffffff8089168352808816602084015280871660408401525084606083015260a060808301526105ae60a083018486610520565b98975050505050505050565b602081525f61019b602083018486610520565b5f602082840312156105dd575f80fd5b505191905056"; + hex"608060405234801561000f575f80fd5b506004361061004a575f3560e01c806340375cb71461004e57806340419eec1461008a57806393a7e7111461009d578063e921aceb146100b0575b5f80fd5b61006161005c3660046104da565b6100c3565b60405173ffffffffffffffffffffffffffffffffffffffff909116815260200160405180910390f35b6100616100983660046104da565b6100fb565b6100616100ab36600461055c565b61011d565b6100616100be3660046104da565b6101af565b5f6100cd856101c8565b5f6100e2866100dd87878761021b565b610295565b90506100f186828787876102a1565b9695505050505050565b5f610105856101c8565b5f6100e2866100dd33865f9182526020526040902090565b5f6101a58461013584865f9182526020526040902090565b6040513060388201526f5af43d82803e903d91602b57fd5bf3ff60248201526014810192909252733d602d80600a3d3981f3363d3d373d3d3d363d73825260588201526037600c8201206078820152605560439091012073ffffffffffffffffffffffffffffffffffffffff1690565b90505b9392505050565b5f6101bf8561013586868661021b565b95945050505050565b8073ffffffffffffffffffffffffffffffffffffffff163b5f03610218576040517ff432283200000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b50565b5f7f02d4347d074298c7b3cea4d31c1362e4d9a0a8801a18322f532336bd2c25b45982858560405161024e929190610595565b6040519081900381206102769392916020019283526020830191909152604082015260600190565b6040516020818303038152906040528051906020012090509392505050565b5f6101a883835f6103da565b5f7f3b3e5b48cfaf4a4b3b2b36425ddec21c2c507fd502d2aea55eb7ffa36b0ca2053387878588886040516102db969594939291906105eb565b60405180910390a16040517f439fab910000000000000000000000000000000000000000000000000000000081527fe0e57eda3f08f2a93bbe980d3df7f9c315eac41181f58b865a13d917fe769fc39073ffffffffffffffffffffffffffffffffffffffff87169063439fab9190610359908890889060040161063e565b6020604051808303815f875af1158015610375573d5f803e3d5ffd5b505050506040513d601f19601f820116820180604052508101906103999190610651565b146103d0576040517f19b991a800000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b5092949350505050565b5f81471015610422576040517fcf4791810000000000000000000000000000000000000000000000000000000081524760048201526024810183905260440160405180910390fd5b763d602d80600a3d3981f3363d3d373d3d3d363d730000008460601b60e81c175f526e5af43d82803e903d91602b57fd5bf38460781b17602052826037600984f5905073ffffffffffffffffffffffffffffffffffffffff81166101a8576040517fb06ebf3d00000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b803573ffffffffffffffffffffffffffffffffffffffff811681146104d5575f80fd5b919050565b5f805f80606085870312156104ed575f80fd5b6104f6856104b2565b9350602085013567ffffffffffffffff80821115610512575f80fd5b818701915087601f830112610525575f80fd5b813581811115610533575f80fd5b886020828501011115610544575f80fd5b95986020929092019750949560400135945092505050565b5f805f6060848603121561056e575f80fd5b610577846104b2565b92506020840135915061058c604085016104b2565b90509250925092565b818382375f9101908152919050565b81835281816020850137505f602082840101525f60207fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0601f840116840101905092915050565b5f73ffffffffffffffffffffffffffffffffffffffff8089168352808816602084015280871660408401525084606083015260a0608083015261063260a0830184866105a4565b98975050505050505050565b602081525f6101a56020830184866105a4565b5f60208284031215610661575f80fd5b505191905056"; diff --git a/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol b/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol index a38afa1..6156b6c 100644 --- a/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol +++ b/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol @@ -8,6 +8,7 @@ import {Clones} from "@openzeppelin-contracts-5.6.1/proxy/Clones.sol"; import {Errors} from "@openzeppelin-contracts-5.6.1/utils/Errors.sol"; import {LibExtrospectERC1167Proxy} from "rain-extrospection-0.1.1/src/lib/LibExtrospectERC1167Proxy.sol"; import {ICLONEABLE_V2_SUCCESS} from "rain-factory-0.1.7/src/interface/ICloneableV2.sol"; +import {ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN} from "rain-factory-0.1.7/src/interface/ICloneableFactoryV4.sol"; import {CloneFactory, ZeroImplementationCodeSize, InitializationFailed} from "../../../src/concrete/CloneFactory.sol"; import {TestCloneable} from "./TestCloneable.sol"; import {TestCloneableFailure} from "./TestCloneableFailure.sol"; @@ -15,9 +16,11 @@ import {TestCloneableFailure} from "./TestCloneableFailure.sol"; /// @title CloneFactoryCloneDeterministicOpenSaltTest /// @notice A test suite for `CloneFactory`'s `cloneDeterministicOpenSalt` / /// `predictDeterministicAddressOpenSalt` functions. The defining property is -/// that the deployer is NOT in the address derivation, which is the exact -/// opposite of what `cloneDeterministic` guarantees, so the two derivations are -/// also tested against each other here. +/// that the address commits to WHAT is deployed — `(implementation, data, +/// salt)` — and to nothing about WHO deploys it, which is the exact opposite of +/// what `cloneDeterministic` guarantees. So the two derivations are also tested +/// against each other here, including the one squat that the domain separator +/// exists to close. contract CloneFactoryCloneDeterministicOpenSaltTest is Test { /// The `CloneFactory` instance under test. Stateless, so reused everywhere. CloneFactory internal immutable I_CLONE_FACTORY; @@ -26,22 +29,30 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { I_CLONE_FACTORY = new CloneFactory(); } - /// The `CREATE2` salt is the caller-supplied salt VERBATIM — no hashing, no - /// namespacing, nothing mixed in. Pins the derivation against OZ's own - /// prediction under the raw salt, so an off-chain caller can reproduce the - /// address from `(implementation, salt, factory)` alone. - function testCloneDeterministicOpenSaltSaltIsVerbatim(address implementation, bytes32 salt) external view { - address expected = Clones.predictDeterministicAddress(implementation, salt, address(I_CLONE_FACTORY)); - assertEq(I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, salt), expected); + /// The effective `CREATE2` salt is exactly the derivation + /// `ICloneableFactoryV4` fixes: + /// `keccak256(abi.encode(ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN, salt, keccak256(data)))`. + /// Pinned against OZ's own prediction under an independently constructed + /// salt, so an off-chain caller can reproduce the address from + /// `(implementation, data, salt, factory)` alone and the test does not + /// restate `CloneFactory`'s arithmetic back to itself. + function testCloneDeterministicOpenSaltIsDomainTaggedHash(address implementation, bytes memory data, bytes32 salt) + external + view + { + bytes32 effectiveSalt = keccak256(abi.encode(ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN, salt, keccak256(data))); + address expected = Clones.predictDeterministicAddress(implementation, effectiveSalt, address(I_CLONE_FACTORY)); + assertEq(I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, data, salt), expected); } /// The deployed clone lands at the predicted address, is an EIP1167 proxy of /// the implementation, and is initialized with the data. `predict` therefore - /// lets a caller pin the address before deploying. + /// lets a caller pin the address before deploying — but only once `data` is + /// final, since `data` is in the derivation. function testCloneDeterministicOpenSaltMatchesPredict(bytes32 salt, bytes memory data) external { TestCloneable implementation = new TestCloneable(); - address predicted = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), salt); + address predicted = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), data, salt); address child = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), data, salt); assertEq(child, predicted); @@ -51,8 +62,8 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { assertEq(TestCloneable(child).sData(), data); } - /// THE POINT OF THIS VARIANT. The same `(implementation, salt)` from two - /// different callers lands on the SAME address. State is snapshotted and + /// THE POINT OF THIS VARIANT. The same `(implementation, data, salt)` from + /// two different callers lands on the SAME address. State is snapshotted and /// rolled back between the two deploys so both callers genuinely deploy from /// the same starting state — the addresses are compared, not merely /// predicted. This is exactly what `cloneDeterministic` forbids, so an @@ -67,7 +78,7 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { vm.assume(alice != bob); TestCloneable implementation = new TestCloneable(); - address predicted = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), salt); + address predicted = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), data, salt); uint256 snapshot = vm.snapshotState(); @@ -84,11 +95,12 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { } /// The prediction takes no deployer, so it cannot vary with one. Predicting - /// the same `(implementation, salt)` from two different callers returns the - /// same address — a caller pinning an address offchain does not need to know - /// who will deploy it. + /// the same `(implementation, data, salt)` from two different callers + /// returns the same address — a caller pinning an address offchain does not + /// need to know who will deploy it. function testCloneDeterministicOpenSaltPredictCallerIndependent( address implementation, + bytes memory data, bytes32 salt, address alice, address bob @@ -96,33 +108,133 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { vm.assume(alice != bob); vm.prank(alice); - address predictedAlice = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, salt); + address predictedAlice = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, data, salt); vm.prank(bob); - address predictedBob = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, salt); + address predictedBob = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, data, salt); assertEq(predictedAlice, predictedBob); } - /// The two derivations are disjoint: for any `(implementation, salt, - /// deployer)` the open-salt address is not the sender-namespaced address. - /// So adding the open variant cannot reach, block or collide with an address - /// that `cloneDeterministic` promised to a specific caller. + /// `data` IS IN THE DERIVATION, which is what makes losing the `msg.sender` + /// namespacing safe. Two different `data` at the SAME `(implementation, + /// salt)` are two different addresses, and both clones exist independently + /// with their own initialization. So a front-runner who passes anything + /// other than the intended bytes deploys their own contract at their own + /// address and at their own expense, leaving the address that was pinned + /// untouched and still deployable. + function testCloneDeterministicOpenSaltDataInDerivation(bytes32 salt, bytes memory dataA, bytes memory dataB) + external + { + vm.assume(keccak256(dataA) != keccak256(dataB)); + TestCloneable implementation = new TestCloneable(); + + address predictedA = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), dataA, salt); + address predictedB = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), dataB, salt); + assertTrue(predictedA != predictedB); + + address childA = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), dataA, salt); + address childB = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), dataB, salt); + + assertEq(childA, predictedA); + assertEq(childB, predictedB); + assertEq(TestCloneable(childA).sData(), dataA); + assertEq(TestCloneable(childB).sData(), dataB); + } + + /// The two derivations are disjoint under freely varying inputs on BOTH + /// sides: no `(data, openSalt)` open-salt address is any `(namespacedSalt, + /// deployer)` sender-namespaced address. Adding the open variant therefore + /// cannot reach, block or collide with an address that `cloneDeterministic` + /// promised to a specific caller, or vice versa. + /// + /// This is the broad statement; the reachable case that actually + /// discriminates the domain separator is the next test. function testCloneDeterministicOpenSaltDiffersFromSenderNamespaced( address implementation, - bytes32 salt, + bytes memory data, + bytes32 openSalt, + bytes32 namespacedSalt, address deployer ) external view { - address open = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, salt); - address namespaced = I_CLONE_FACTORY.predictDeterministicAddress(implementation, salt, deployer); + address open = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, data, openSalt); + address namespaced = I_CLONE_FACTORY.predictDeterministicAddress(implementation, namespacedSalt, deployer); + assertTrue(open != namespaced); + } + + /// THE SQUAT THE DOMAIN SEPARATOR CLOSES, stated as an attack rather than as + /// an absence. + /// + /// `ICloneableFactoryV4` makes it a MUST NOT on the factory that no other + /// entry point can `CREATE2` in the open-salt derivation's image with + /// caller-supplied `data`. `cloneDeterministic` is precisely such an entry + /// point. Had the open derivation been the untagged + /// `keccak256(abi.encode(salt, keccak256(data)))`, both preimages would be + /// 64 bytes led by a caller-chosen word, and `abi.encode` left-pads an + /// address into exactly the word a `bytes32` salt already is. So an account + /// `A` would reach EVERY open-salt address whose `salt` happens to equal + /// `bytes32(uint256(uint160(A)))` — no preimage search, just + /// `cloneDeterministic(implementation, evilData, keccak256(data))` with + /// whatever `evilData` it liked, aimed at an address somebody else pinned. + /// + /// The premise is asserted against the factory's OWN namespaced prediction, + /// so this test proves the attack is live absent the tag rather than merely + /// restating `abi.encode`. Remove + /// `ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN` from `_effectiveOpenSalt` and + /// this test fails. + function testCloneDeterministicOpenSaltDisjointFromNamespacedAtLeftPaddedAddressSalt( + address attacker, + bytes memory data, + bytes memory evilData + ) external { + TestCloneable implementation = new TestCloneable(); + + // The open salt an honest party pinned, which happens to be the + // abi-encoding of the attacker's own address. Nothing stops a salt + // taking this value; the attacker is free to go looking for one that + // does, or to pick the address to suit the salt. + bytes32 openSalt = bytes32(uint256(uint160(attacker))); + address open = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), data, openSalt); + + // The attacker's namespaced salt is just `keccak256(data)`, read off + // the honest deploy they are front-running. `cloneDeterministic` hashes + // `abi.encode(msg.sender, salt)`, so from `attacker` this is the + // effective salt `keccak256(abi.encode(attacker, keccak256(data)))`. + bytes32 attackerSalt = keccak256(data); + address namespaced = + I_CLONE_FACTORY.predictDeterministicAddress(address(implementation), attackerSalt, attacker); + + // ATTACK PREMISE. Those are byte for byte the 64 bytes an untagged open + // derivation would hash for `(data, openSalt)`, so without the domain + // word the squat lands exactly on the address the honest party pinned. + address undomained = Clones.predictDeterministicAddress( + address(implementation), keccak256(abi.encode(openSalt, keccak256(data))), address(I_CLONE_FACTORY) + ); + assertEq(undomained, namespaced, "an untagged open salt IS reachable by cloneDeterministic"); + + // THE GUARANTEE. The domain word moves the real open-salt address off + // the one `cloneDeterministic` can reach. assertTrue(open != namespaced); + + // End to end, not just in prediction: the attacker really deploys, at + // their own address, with their own data, and the honest open-salt + // address is still free afterwards and still deploys the intended + // clone with the intended bytes. + vm.prank(attacker); + address childAttacker = I_CLONE_FACTORY.cloneDeterministic(address(implementation), evilData, attackerSalt); + assertEq(childAttacker, namespaced); + assertEq(open.code.length, 0); + + address childOpen = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), data, openSalt); + assertEq(childOpen, open); + assertEq(TestCloneable(childOpen).sData(), data); } - /// REGRESSION GUARD on the guarantee that must not break. Taking a salt via - /// the open variant does not consume it for `cloneDeterministic`: the same - /// caller can still deploy at the same `salt` through the namespaced - /// derivation, at the address it always predicted, and both clones exist - /// independently. + /// REGRESSION GUARD on the guarantee that must not break, in the other + /// direction. Taking a salt via the open variant does not consume it for + /// `cloneDeterministic`: the same caller can still deploy at the same `salt` + /// through the namespaced derivation, at the address it always predicted, + /// and both clones exist independently. function testCloneDeterministicOpenSaltDoesNotConsumeNamespacedSalt(bytes32 salt, bytes memory data) external { TestCloneable implementation = new TestCloneable(); @@ -138,8 +250,10 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { assertTrue(childNamespaced.code.length > 0); } - /// Distinct salts yield distinct clones of the same implementation — many - /// clones per impl, as with the namespaced variant. + /// Distinct salts yield distinct clones of the same implementation and the + /// same `data` — many clones per impl, as with the namespaced variant. The + /// other half of "distinct `(salt, data)` pairs yield distinct clones"; the + /// `data` half is `…DataInDerivation`. function testCloneDeterministicOpenSaltManyClonesPerImpl(bytes32 salt1, bytes32 salt2, bytes memory data) external { vm.assume(salt1 != salt2); TestCloneable implementation = new TestCloneable(); @@ -149,15 +263,16 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { assertTrue(child1 != child2); } - /// A second deploy at an already-taken open salt REVERTS. It does not - /// silently return the existing clone, so a caller can never mistake - /// somebody else's already-initialized contract for their own fresh deploy. - /// Squatting is therefore loud at the point of deploy, even though it is - /// unrecoverable after it. + /// A second deploy at an already-taken open salt REVERTS. Since `data` is in + /// the derivation, repeating the whole `(implementation, data, salt)` is now + /// the ONLY way to aim at an address somebody else already took, and even + /// that does not silently return the existing clone: a caller can never + /// mistake an already-initialized contract for their own fresh deploy. What + /// the reverting caller would have deployed is byte-identical to what is + /// already there, so the loss is the gas and nothing else. function testCloneDeterministicOpenSaltSecondDeployReverts( bytes32 salt, - bytes memory dataFirst, - bytes memory dataSecond, + bytes memory data, address alice, address bob ) external { @@ -165,20 +280,23 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { TestCloneable implementation = new TestCloneable(); vm.prank(alice); - address child = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), dataFirst, salt); + address child = I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), data, salt); vm.prank(bob); vm.expectRevert(abi.encodeWithSelector(Errors.FailedDeployment.selector)); - I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), dataSecond, salt); + I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), data, salt); // The first deploy's state is untouched by the failed second one. - assertEq(TestCloneable(child).sData(), dataFirst); + assertEq(TestCloneable(child).sData(), data); } /// `NewClone` is emitted with the caller, implementation, child, salt and /// data. The event is shared with `cloneDeterministic` and carries the RAW - /// salt in both cases, so an indexer must know which entry point was called - /// to recompute the address — the `clone` field is the authoritative address. + /// salt in both cases, never the effective one. `salt` and `data` together + /// are the whole of the open derivation, so the event carries enough to + /// recompute the address — an indexer picks the derivation by trying both + /// and keeping the match, which is well defined precisely because the two + /// images are disjoint. function testCloneDeterministicOpenSaltEvent(bytes32 salt, bytes memory data) external { TestCloneable implementation = new TestCloneable(); @@ -198,10 +316,11 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { vm.assume(notSuccess != ICLONEABLE_V2_SUCCESS); TestCloneableFailure implementation = new TestCloneableFailure(); - address predicted = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), salt); + bytes memory data = abi.encode(notSuccess); + address predicted = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), data, salt); vm.expectRevert(abi.encodeWithSelector(InitializationFailed.selector)); - I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), abi.encode(notSuccess), salt); + I_CLONE_FACTORY.cloneDeterministicOpenSalt(address(implementation), data, salt); assertEq(predicted.code.length, 0); } diff --git a/test/src/lib/LibCloneFactoryDeployCandidate.t.sol b/test/src/lib/LibCloneFactoryDeployCandidate.t.sol index 43a08a5..62bdb5b 100644 --- a/test/src/lib/LibCloneFactoryDeployCandidate.t.sol +++ b/test/src/lib/LibCloneFactoryDeployCandidate.t.sol @@ -82,7 +82,7 @@ contract LibCloneFactoryDeployCandidateTest is Test { bytes memory data = hex"f100dedb0a75"; address predictedNamespaced = factory.predictDeterministicAddress(address(implementation), salt, address(this)); - address predictedOpen = factory.predictDeterministicAddressOpenSalt(address(implementation), salt); + address predictedOpen = factory.predictDeterministicAddressOpenSalt(address(implementation), data, salt); assertTrue(predictedNamespaced != predictedOpen); address childNamespaced = factory.cloneDeterministic(address(implementation), data, salt); From 0a47851a74630199121700666f4906ce99f4de9b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 22:19:59 +0000 Subject: [PATCH 3/3] test: widen the disjointness note to the equation, regenerate .gas-snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .gas-snapshot | 30 +++++++++---------- README.md | 5 +++- ...oneFactoryCloneDeterministicOpenSalt.t.sol | 16 ++++++++-- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index d947b7f..b1c369e 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,22 +1,22 @@ -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltCallerIndependent(bytes32,bytes,address,address) (runs: 2048, μ: 589533, ~: 486442) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDataInDerivation(bytes32,bytes,bytes) (runs: 2048, μ: 572082, ~: 471101) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDiffersFromSenderNamespaced(address,bytes,bytes32,bytes32,address) (runs: 2048, μ: 6747, ~: 6672) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDisjointFromNamespacedAtLeftPaddedAddressSalt(address,bytes,bytes) (runs: 2048, μ: 568064, ~: 470240) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDoesNotConsumeNamespacedSalt(bytes32,bytes) (runs: 2048, μ: 579815, ~: 436545) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltEvent(bytes32,bytes) (runs: 2048, μ: 421178, ~: 349404) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltCallerIndependent(bytes32,bytes,address,address) (runs: 2048, μ: 588853, ~: 486442) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDataInDerivation(bytes32,bytes,bytes) (runs: 2048, μ: 578320, ~: 471101) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDiffersFromSenderNamespaced(address,bytes,bytes32,bytes32,address) (runs: 2048, μ: 6748, ~: 6672) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDisjointFromNamespacedAtLeftPaddedAddressSalt(address,bytes,bytes) (runs: 2048, μ: 572194, ~: 470240) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltDoesNotConsumeNamespacedSalt(bytes32,bytes) (runs: 2048, μ: 577796, ~: 436545) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltEvent(bytes32,bytes) (runs: 2048, μ: 420167, ~: 349404) CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltInitializeFailureFails(bytes32,bytes32) (runs: 2048, μ: 166138, ~: 166138) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltIsDomainTaggedHash(address,bytes,bytes32) (runs: 2048, μ: 5884, ~: 5792) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltManyClonesPerImpl(bytes32,bytes32,bytes) (runs: 2048, μ: 575136, ~: 438028) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltMatchesPredict(bytes32,bytes) (runs: 2048, μ: 423460, ~: 351127) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltPredictCallerIndependent(address,bytes,bytes32,address,address) (runs: 2048, μ: 10987, ~: 10918) -CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltSecondDeployReverts(bytes32,bytes,address,address) (runs: 2048, μ: 1040444148, ~: 1040443103) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltIsDomainTaggedHash(address,bytes,bytes32) (runs: 2048, μ: 5883, ~: 5792) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltManyClonesPerImpl(bytes32,bytes32,bytes) (runs: 2048, μ: 564711, ~: 438028) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltMatchesPredict(bytes32,bytes) (runs: 2048, μ: 422441, ~: 351127) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltPredictCallerIndependent(address,bytes,bytes32,address,address) (runs: 2048, μ: 10997, ~: 10918) +CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltSecondDeployReverts(bytes32,bytes,address,address) (runs: 2048, μ: 1040444140, ~: 1040443099) CloneFactoryCloneDeterministicOpenSaltTest:testCloneDeterministicOpenSaltZeroImplementationCodeSize(address,bytes,bytes32) (runs: 2048, μ: 10659, ~: 10634) -CloneFactoryCloneDeterministicTest:testCloneDeterministicEvent(bytes32,bytes) (runs: 2048, μ: 420866, ~: 349123) +CloneFactoryCloneDeterministicTest:testCloneDeterministicEvent(bytes32,bytes) (runs: 2048, μ: 419856, ~: 349123) CloneFactoryCloneDeterministicTest:testCloneDeterministicInitializeFailureFails(bytes32,bytes32) (runs: 2048, μ: 161288, ~: 161288) -CloneFactoryCloneDeterministicTest:testCloneDeterministicManyClonesPerImpl(bytes32,bytes32,bytes) (runs: 2048, μ: 574513, ~: 437463) -CloneFactoryCloneDeterministicTest:testCloneDeterministicMatchesPredict(bytes32,bytes) (runs: 2048, μ: 422516, ~: 350262) +CloneFactoryCloneDeterministicTest:testCloneDeterministicManyClonesPerImpl(bytes32,bytes32,bytes) (runs: 2048, μ: 564092, ~: 437463) +CloneFactoryCloneDeterministicTest:testCloneDeterministicMatchesPredict(bytes32,bytes) (runs: 2048, μ: 421499, ~: 350262) CloneFactoryCloneDeterministicTest:testCloneDeterministicSaltIsAbiEncodeHash(address,bytes32,address) (runs: 2048, μ: 4708, ~: 4708) -CloneFactoryCloneDeterministicTest:testCloneDeterministicSenderScoped(bytes32,bytes,address,address) (runs: 2048, μ: 588912, ~: 485903) +CloneFactoryCloneDeterministicTest:testCloneDeterministicSenderScoped(bytes32,bytes,address,address) (runs: 2048, μ: 588232, ~: 485903) CloneFactoryCloneDeterministicTest:testCloneDeterministicZeroImplementationCodeSize(address,bytes,bytes32) (runs: 2048, μ: 10715, ~: 10700) LibCloneFactoryDeployCandidateTest:testCandidateCreationDeploysToPinnedAddress() (gas: 366569) LibCloneFactoryDeployCandidateTest:testCandidateDeployedBytecodeServesBothEntryPoints() (gas: 760962) diff --git a/README.md b/README.md index 54e49fc..81d7040 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,10 @@ only in how the salt is derived: address, but the deploying account is baked into it forever and `data` is outside the derivation. - `cloneDeterministicOpenSalt` derives the salt as - `keccak256(abi.encode(ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN, salt, keccak256(data)))`, + `keccak256(abi.encode(ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN, salt, keccak256(data)))` + — the domain constant being + `keccak256("ICloneableFactoryV4.cloneDeterministicOpenSalt")`, declared in + `rain.factory` so third parties recompute the address rather than trust it — so the address commits to WHAT was deployed and to nothing about who deployed it: it is a function of `(factory, implementation, salt, data)` alone. Every account reaches the same address, and so can anyone. That also makes it the diff --git a/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol b/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol index 6156b6c..3d3c52e 100644 --- a/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol +++ b/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol @@ -148,8 +148,11 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { /// cannot reach, block or collide with an address that `cloneDeterministic` /// promised to a specific caller, or vice versa. /// - /// This is the broad statement; the reachable case that actually - /// discriminates the domain separator is the next test. + /// This is the broad statement, and on its own it is weak: a collision it + /// could catch needs a keccak256 collision, so no realistic mutation of the + /// derivation makes it fail. It is kept as the plain form of the interface's + /// claim. The reachable case that actually discriminates the domain + /// separator is the next test. function testCloneDeterministicOpenSaltDiffersFromSenderNamespaced( address implementation, bytes memory data, @@ -182,6 +185,15 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { /// restating `abi.encode`. Remove /// `ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN` from `_effectiveOpenSalt` and /// this test fails. + /// + /// The single equation + /// `keccak256(abi.encode(deployer, nsSalt)) == keccak256(abi.encode(openSalt, keccak256(data)))` + /// is the whole of the reachable overlap, and this test closes it, so the + /// mirror framing needs no second test: a victim who picks their namespaced + /// `nsSalt` as `keccak256(P)` for reproducible bytes `P` — an ordinary + /// choice — would, untagged, be reachable by an attacker calling + /// `cloneDeterministicOpenSalt(implementation, P, bytes32(uint256(uint160(victim))))`. + /// Same two unknowns, solved from the other side, closed by the same word. function testCloneDeterministicOpenSaltDisjointFromNamespacedAtLeftPaddedAddressSalt( address attacker, bytes memory data,