From 43158df91b7897356745831e0e0e3d380d0ec201 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Sun, 20 Sep 2026 10:27:08 +0000 Subject: [PATCH 1/6] test: cover the deploy declaration's metadata and the open-salt entry points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial mutation testing at 5c165a8 found ten surviving mutants. Nine are the same shape: the inherited `rain-deploy` assertions check that a suite's recorded pins agree with its own creation code, and never ask about the declaration's METADATA — the suite key, the artifact path, the dependency list — none of which is derivable from a snapshot's bytes, and each of which steers something real. `CloneFactoryDeploySuitesDeclarationTest` asks: - Every frozen release directory is declared by a released suite under its own KEY. `testEveryFrozenSnapshotIsReleased` matches a record file to a release by the DEPLOYED ADDRESS it declares, so two releases that froze identical creation code are indistinguishable to it: dropping `0_1_10` from the declaration left `0_1_9` matching its record file and the whole suite green (mutant M45 SURVIVED), while dropping `0_1_1`, whose bytecode is unique, was caught (M46 KILLED). Same edit, opposite verdicts. - The candidate is declared under the bare `clone-factory` key. - `[external.package].version` has a frozen snapshot directory, the lockstep CLAUDE.md documents and nothing enforced. - Every declared suite records an EMPTY dependency list. The repo pinned this for the candidate's generated constant but not for the declaration, which is what the broadcast and the chain check actually read. - Every declared suite's `artifactPath` names a file that exists and the contract inside it. Nothing derives it, so a stale path survives every other assertion and fails after the gas is spent. `CloneFactoryCloneDeterministicOpenSaltTest` is the counterpart to `CloneFactoryCloneDeterministicTest`, which covers only the namespaced pair. Every expectation is built from the `ICloneableFactoryV4` spec and OpenZeppelin `Clones`, never from `LibICloneableFactoryV4`: the equivalence suite states its open-salt expectations in terms of the library's own `effectiveOpenSalt`, so it moves with the derivation rather than checking it, where the namespaced pair has had an independent oracle all along. It also covers the derivation's defining property — that the address is sender independent and commits to `data` — which nothing asserted directly. Revert paths and the `NewClone` event are deliberately not restated; the equivalence suite already holds them field for field. 28 tests -> 38. Co-Authored-By: Claude Opus 5 (1M context) --- .../CloneFactoryDeploySuitesDeclaration.t.sol | 146 ++++++++++++++++++ ...oneFactoryCloneDeterministicOpenSalt.t.sol | 127 +++++++++++++++ 2 files changed, 273 insertions(+) create mode 100644 test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol create mode 100644 test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol diff --git a/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol b/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol new file mode 100644 index 0000000..9fafb89 --- /dev/null +++ b/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol @@ -0,0 +1,146 @@ +// 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.2/src/Test.sol"; + +import {LibRainDeploySnapshot} from "rain-deploy-0.1.8/src/lib/LibRainDeploySnapshot.sol"; +import {DeployCandidate, DeploySuite} from "src/abstract/RainDeploySuitesBase.sol"; +import {CloneFactoryDeploySuites} from "src/abstract/CloneFactoryDeploySuites.sol"; + +/// @title CloneFactoryDeploySuitesDeclarationTest +/// @notice The parts of this repo's deploy declaration that the inherited +/// `rain-deploy` assertions do not reach. +/// +/// `RainDeployVerifySnapshot` checks that each suite's recorded pins agree with +/// its own creation code, that the candidate is anchored to source, and that +/// every frozen record file is declared. What it never asks about is the +/// declaration's METADATA — the suite key, the artifact path and the dependency +/// list — none of which is derivable from a snapshot's bytes, and each of which +/// steers something real: the key selects what `DEPLOYMENT_SUITE` broadcasts, +/// the artifact path is the explorer verification target, and the dependency +/// list gates broadcasting on a network. +contract CloneFactoryDeploySuitesDeclarationTest is CloneFactoryDeploySuites, Test { + /// The record root, spelled once here as the directory the release tags live + /// under. + string constant GENERATED_DIR = "src/generated"; + + /// The suite key prefix every `CloneFactory` suite shares: the rolling + /// candidate is exactly this, and a frozen release is this, an at sign, and + /// the release tag. + string constant CLONE_FACTORY_SUITE = "clone-factory"; + + /// Every frozen release directory MUST be declared by a released suite whose + /// KEY names that tag. + /// + /// `testEveryFrozenSnapshotIsReleased` already walks the same record, but it + /// matches a record file to a release by the DEPLOYED ADDRESS that file + /// declares. Two releases that froze identical creation code therefore + /// deploy to one address and are indistinguishable to it, so dropping one of + /// them from the declaration leaves the other matching its record file and + /// the check green — while the dropped release stops being broadcastable by + /// key and stops carrying its own dependency list. `0_1_9` and `0_1_10` in + /// this repo are exactly such a pair. + /// + /// Matching by key instead, so the question asked is "is THIS tag declared" + /// rather than "does some declared suite happen to land on this address". + function testEveryFrozenTagIsDeclaredByKey() external view { + Vm.DirEntry[] memory entries = vm.readDir(GENERATED_DIR, 1); + DeploySuite[] memory released = releasedSuites(); + + uint256 tagsSeen = 0; + for (uint256 i = 0; i < entries.length; i++) { + if (!entries[i].isDir) { + continue; + } + string[] memory components = vm.split(entries[i].path, "/"); + string memory tag = components[components.length - 1]; + if (!LibRainDeploySnapshot.isTag(tag)) { + continue; + } + tagsSeen++; + + string memory expectedKey = string.concat(CLONE_FACTORY_SUITE, "@", tag); + bool declared = false; + for (uint256 j = 0; j < released.length; j++) { + if (keccak256(bytes(released[j].suite)) == keccak256(bytes(expectedKey))) { + declared = true; + break; + } + } + assertTrue(declared, string.concat("frozen release is not declared under its own key: ", expectedKey)); + } + + // A walk that found no tag would pass the loop above with no subject. + // This repo has frozen releases, so finding none means the walk broke. + assertTrue(tagsSeen > 0, "no frozen release directories found under the record root"); + assertEq(released.length, tagsSeen, "declared releases and frozen release directories disagree in number"); + } + + /// The rolling candidate is declared under the bare suite key, which is what + /// `DEPLOYMENT_SUITE` selects to broadcast current source. + function testCandidateDeclaresTheBareSuiteKey() external pure { + assertEq(cloneFactoryCandidate().snapshot.suite, CLONE_FACTORY_SUITE); + } + + /// `[external.package].version` MUST have a frozen snapshot directory. + /// + /// The version is the last RELEASED version and a release freezes the + /// candidate into `src/generated//` in lockstep, so a version naming a + /// tag that does not exist is a release that was published without its + /// record — the record every chain assertion afterwards reads. + function testDeclaredVersionHasAFrozenSnapshot() external view { + string memory version = vm.parseTomlString(vm.readFile("foundry.toml"), ".external.package.version"); + string memory tag = LibRainDeploySnapshot.tagForVersion(version); + + assertTrue( + vm.exists(string.concat(GENERATED_DIR, "/", tag)), + string.concat("released version has no frozen snapshot directory: ", tag) + ); + } + + /// EVERY declared suite — released and candidate — records an EMPTY deploy + /// dependency list. + /// + /// `CloneFactory` reads nothing and calls nothing at construction, and no + /// release of it ever has, so nothing must already be on chain for it to be + /// broadcast anywhere. A phantom dependency blocks a broadcast on every + /// network where that address is codeless; a dropped one lets a genuinely + /// dependent deployment through. The repo already pins this for the + /// candidate's generated DEPENDENCIES constant — this asks the DECLARATION, + /// which is what the broadcast and the chain check actually read. + function testEveryDeclaredSuiteHasNoDeployDependencies() external pure { + DeploySuite[] memory suites = allSuites(); + assertTrue(suites.length > 0, "no declared suites"); + + for (uint256 i = 0; i < suites.length; i++) { + assertEq( + suites[i].dependencies.length, + 0, + string.concat("suite declares unexpected deploy dependencies: ", suites[i].suite) + ); + } + } + + /// EVERY declared suite's `artifactPath` MUST name a source file that exists + /// and the contract inside it. + /// + /// It is the `:` handed to explorer verification after a + /// broadcast. Nothing derives it — `DeploySuite` documents it as declared + /// precisely because no naming convention recovers it — so a stale path + /// survives every other assertion and fails after the gas is spent. + function testEveryDeclaredSuiteArtifactPathResolves() external view { + DeploySuite[] memory suites = allSuites(); + + for (uint256 i = 0; i < suites.length; i++) { + string[] memory parts = vm.split(suites[i].artifactPath, ":"); + assertEq(parts.length, 2, string.concat("artifactPath is not :: ", suites[i].artifactPath)); + + assertTrue(vm.exists(parts[0]), string.concat("artifactPath names no such file: ", parts[0])); + assertTrue( + vm.contains(vm.readFile(parts[0]), string.concat("contract ", parts[1])), + string.concat("artifactPath file declares no such contract: ", suites[i].artifactPath) + ); + } + } +} diff --git a/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol b/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol new file mode 100644 index 0000000..dc60561 --- /dev/null +++ b/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {Test} from "forge-std-1.16.2/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_FACTORY_V4_OPEN_SALT_DOMAIN} from "rain-factory-0.1.9/src/interface/ICloneableFactoryV4.sol"; +import {CloneFactory} from "../../../src/concrete/CloneFactory.sol"; +import {TestCloneable} from "./TestCloneable.sol"; + +/// @title CloneFactoryCloneDeterministicOpenSaltTest +/// @notice A test suite for `CloneFactory`'s `cloneDeterministicOpenSalt` / +/// `predictDeterministicAddressOpenSalt` — the counterpart to +/// `CloneFactoryCloneDeterministicTest`, which covers only the namespaced pair. +/// +/// Every expectation here is built from the `ICloneableFactoryV4` SPEC and from +/// OpenZeppelin `Clones` — a foreign implementation of the same EIP-1167 +/// standard — never from `LibICloneableFactoryV4`. That is the point of the +/// suite. The equivalence suite already holds the concrete to the library, but +/// it states every open-salt expectation in terms of the library's own +/// `effectiveOpenSalt`, so it moves with the derivation rather than checking +/// it; the namespaced pair has had an independent oracle since +/// `testCloneDeterministicSaltIsDomainTaggedHash` and this gives the open-salt +/// pair the same. +/// +/// The revert paths (`ZeroImplementationCodeSize`, `InitializationFailed`) and +/// the `NewClone` event are deliberately NOT restated here: the equivalence +/// suite already asserts them for this entry point, field for field. +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 effective CREATE2 salt is exactly the derivation + /// `ICloneableFactoryV4` pins: + /// `keccak256(abi.encode(ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN, salt, keccak256(data)))`, + /// so an off-chain caller can reproduce the predicted address. Pinned + /// against OZ `Clones` under an independently constructed salt, so the test + /// does not restate the library'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 — the concrete's two + /// open-salt entry points held to each other, with no library in between. + function testCloneDeterministicOpenSaltMatchesPredict(bytes32 salt, bytes memory data) external { + TestCloneable implementation = new TestCloneable(); + + address predicted = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), data, 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 DEFINING property of the open-salt derivation, and the exact opposite + /// of the namespaced one: the address does not depend on the caller, so + /// every account reaches the same address for the same + /// `(implementation, data, salt)`. The spec forbids the factory mixing + /// `msg.sender`, `tx.origin` or any other caller-derived value in. + function testCloneDeterministicOpenSaltIsSenderIndependent( + 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), data, 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, predicted); + assertEq(childBob, predicted); + } + + /// `data` is INSIDE the derivation, which is what makes an open-salt address + /// safe to pin without sender namespacing: a caller passing different `data` + /// lands somewhere else rather than occupying the address somebody pinned. + function testCloneDeterministicOpenSaltCommitsToData( + address implementation, + bytes32 salt, + bytes memory data, + bytes memory dataOther + ) external view { + vm.assume(keccak256(data) != keccak256(dataOther)); + + assertTrue( + I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, data, salt) + != I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, dataOther, salt) + ); + } + + /// Distinct salts yield distinct clones of the same implementation for the + /// same initialization data — many clones per impl. + 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); + } +} From 70d4e18886c0b681e1d632d7153c1642fb3266e3 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Sun, 20 Sep 2026 10:29:54 +0000 Subject: [PATCH 2/6] test: drop the record-size assertion from the frozen-tag check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rain-deploy deliberately refuses to compare the record's size against the declaration's, and says why on testEveryFrozenSnapshotIsReleased: a release deployed before a repo adopted the machinery has no frozen record and never will, so a size check red-lines that state permanently with no way to spell the exemption. Asserting it here reintroduced exactly that, for no gain — the per-tag key lookup is what kills a dropped release. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol b/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol index 9fafb89..8c19996 100644 --- a/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol +++ b/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol @@ -44,6 +44,12 @@ contract CloneFactoryDeploySuitesDeclarationTest is CloneFactoryDeploySuites, Te /// /// Matching by key instead, so the question asked is "is THIS tag declared" /// rather than "does some declared suite happen to land on this address". + /// Deliberately NOT also a size check. `rain-deploy` explains at length on + /// `testEveryFrozenSnapshotIsReleased` why comparing the record's size against + /// the declaration's would red-line permanently with no way to spell the + /// exemption: a release deployed before this repo adopted the machinery has no + /// frozen record and never will. Asking only that each tag present IS declared + /// keeps that state legal while still catching a dropped release. function testEveryFrozenTagIsDeclaredByKey() external view { Vm.DirEntry[] memory entries = vm.readDir(GENERATED_DIR, 1); DeploySuite[] memory released = releasedSuites(); @@ -74,7 +80,6 @@ contract CloneFactoryDeploySuitesDeclarationTest is CloneFactoryDeploySuites, Te // A walk that found no tag would pass the loop above with no subject. // This repo has frozen releases, so finding none means the walk broke. assertTrue(tagsSeen > 0, "no frozen release directories found under the record root"); - assertEq(released.length, tagsSeen, "declared releases and frozen release directories disagree in number"); } /// The rolling candidate is declared under the bare suite key, which is what From 0164435fe11e176aebd80adaeabfe1804d90e49d Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Sun, 20 Sep 2026 10:34:21 +0000 Subject: [PATCH 3/6] audit: record the adversarial mutation-test scan of 5c165a8 59 behaviours probed across 5 passes, 28 tests before and 38 after. Four findings filed: #33 #34 #35 #36. Co-Authored-By: Claude Opus 5 (1M context) --- audit/mutation-test-scans.json | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 audit/mutation-test-scans.json diff --git a/audit/mutation-test-scans.json b/audit/mutation-test-scans.json new file mode 100644 index 0000000..1c00afd --- /dev/null +++ b/audit/mutation-test-scans.json @@ -0,0 +1,20 @@ +[ + { + "timestamp": "2026-09-20T10:34:00Z", + "commit": "5c165a880ac105d6c038cee096500f76849a2190", + "testsAfterCommit": "70d4e18886c0b681e1d632d7153c1642fb3266e3", + "publishedTag": "sol-v0.1.10", + "commitsAheadOfTag": 6, + "scope": "whole repo", + "tool": "adversarial-mutation-test", + "skillVersion": "0.35.0", + "summary": { + "behaviours": 59, + "candidates": 7, + "confirmed": 4, + "testsBefore": 28, + "testsAfter": 38, + "filed": ["#33", "#34", "#35", "#36"] + } + } +] From 9c78c8a3d6495e385b076465eff4a9e0364978b2 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Sun, 20 Sep 2026 13:24:54 +0000 Subject: [PATCH 4/6] docs: cut the rationale doc blocks from the two new test files Co-Authored-By: Claude Opus 5 (1M context) --- .../CloneFactoryDeploySuitesDeclaration.t.sol | 64 ------------------- ...oneFactoryCloneDeterministicOpenSalt.t.sol | 38 ----------- 2 files changed, 102 deletions(-) diff --git a/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol b/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol index 8c19996..2d995ea 100644 --- a/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol +++ b/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol @@ -8,48 +8,11 @@ import {LibRainDeploySnapshot} from "rain-deploy-0.1.8/src/lib/LibRainDeploySnap import {DeployCandidate, DeploySuite} from "src/abstract/RainDeploySuitesBase.sol"; import {CloneFactoryDeploySuites} from "src/abstract/CloneFactoryDeploySuites.sol"; -/// @title CloneFactoryDeploySuitesDeclarationTest -/// @notice The parts of this repo's deploy declaration that the inherited -/// `rain-deploy` assertions do not reach. -/// -/// `RainDeployVerifySnapshot` checks that each suite's recorded pins agree with -/// its own creation code, that the candidate is anchored to source, and that -/// every frozen record file is declared. What it never asks about is the -/// declaration's METADATA — the suite key, the artifact path and the dependency -/// list — none of which is derivable from a snapshot's bytes, and each of which -/// steers something real: the key selects what `DEPLOYMENT_SUITE` broadcasts, -/// the artifact path is the explorer verification target, and the dependency -/// list gates broadcasting on a network. contract CloneFactoryDeploySuitesDeclarationTest is CloneFactoryDeploySuites, Test { - /// The record root, spelled once here as the directory the release tags live - /// under. string constant GENERATED_DIR = "src/generated"; - /// The suite key prefix every `CloneFactory` suite shares: the rolling - /// candidate is exactly this, and a frozen release is this, an at sign, and - /// the release tag. string constant CLONE_FACTORY_SUITE = "clone-factory"; - /// Every frozen release directory MUST be declared by a released suite whose - /// KEY names that tag. - /// - /// `testEveryFrozenSnapshotIsReleased` already walks the same record, but it - /// matches a record file to a release by the DEPLOYED ADDRESS that file - /// declares. Two releases that froze identical creation code therefore - /// deploy to one address and are indistinguishable to it, so dropping one of - /// them from the declaration leaves the other matching its record file and - /// the check green — while the dropped release stops being broadcastable by - /// key and stops carrying its own dependency list. `0_1_9` and `0_1_10` in - /// this repo are exactly such a pair. - /// - /// Matching by key instead, so the question asked is "is THIS tag declared" - /// rather than "does some declared suite happen to land on this address". - /// Deliberately NOT also a size check. `rain-deploy` explains at length on - /// `testEveryFrozenSnapshotIsReleased` why comparing the record's size against - /// the declaration's would red-line permanently with no way to spell the - /// exemption: a release deployed before this repo adopted the machinery has no - /// frozen record and never will. Asking only that each tag present IS declared - /// keeps that state legal while still catching a dropped release. function testEveryFrozenTagIsDeclaredByKey() external view { Vm.DirEntry[] memory entries = vm.readDir(GENERATED_DIR, 1); DeploySuite[] memory released = releasedSuites(); @@ -77,23 +40,13 @@ contract CloneFactoryDeploySuitesDeclarationTest is CloneFactoryDeploySuites, Te assertTrue(declared, string.concat("frozen release is not declared under its own key: ", expectedKey)); } - // A walk that found no tag would pass the loop above with no subject. - // This repo has frozen releases, so finding none means the walk broke. assertTrue(tagsSeen > 0, "no frozen release directories found under the record root"); } - /// The rolling candidate is declared under the bare suite key, which is what - /// `DEPLOYMENT_SUITE` selects to broadcast current source. function testCandidateDeclaresTheBareSuiteKey() external pure { assertEq(cloneFactoryCandidate().snapshot.suite, CLONE_FACTORY_SUITE); } - /// `[external.package].version` MUST have a frozen snapshot directory. - /// - /// The version is the last RELEASED version and a release freezes the - /// candidate into `src/generated//` in lockstep, so a version naming a - /// tag that does not exist is a release that was published without its - /// record — the record every chain assertion afterwards reads. function testDeclaredVersionHasAFrozenSnapshot() external view { string memory version = vm.parseTomlString(vm.readFile("foundry.toml"), ".external.package.version"); string memory tag = LibRainDeploySnapshot.tagForVersion(version); @@ -104,16 +57,6 @@ contract CloneFactoryDeploySuitesDeclarationTest is CloneFactoryDeploySuites, Te ); } - /// EVERY declared suite — released and candidate — records an EMPTY deploy - /// dependency list. - /// - /// `CloneFactory` reads nothing and calls nothing at construction, and no - /// release of it ever has, so nothing must already be on chain for it to be - /// broadcast anywhere. A phantom dependency blocks a broadcast on every - /// network where that address is codeless; a dropped one lets a genuinely - /// dependent deployment through. The repo already pins this for the - /// candidate's generated DEPENDENCIES constant — this asks the DECLARATION, - /// which is what the broadcast and the chain check actually read. function testEveryDeclaredSuiteHasNoDeployDependencies() external pure { DeploySuite[] memory suites = allSuites(); assertTrue(suites.length > 0, "no declared suites"); @@ -127,13 +70,6 @@ contract CloneFactoryDeploySuitesDeclarationTest is CloneFactoryDeploySuites, Te } } - /// EVERY declared suite's `artifactPath` MUST name a source file that exists - /// and the contract inside it. - /// - /// It is the `:` handed to explorer verification after a - /// broadcast. Nothing derives it — `DeploySuite` documents it as declared - /// precisely because no naming convention recovers it — so a stale path - /// survives every other assertion and fails after the gas is spent. function testEveryDeclaredSuiteArtifactPathResolves() external view { DeploySuite[] memory suites = allSuites(); diff --git a/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol b/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol index dc60561..e7aed1a 100644 --- a/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol +++ b/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol @@ -10,38 +10,13 @@ import {ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN} from "rain-factory-0.1.9/src/int import {CloneFactory} from "../../../src/concrete/CloneFactory.sol"; import {TestCloneable} from "./TestCloneable.sol"; -/// @title CloneFactoryCloneDeterministicOpenSaltTest -/// @notice A test suite for `CloneFactory`'s `cloneDeterministicOpenSalt` / -/// `predictDeterministicAddressOpenSalt` — the counterpart to -/// `CloneFactoryCloneDeterministicTest`, which covers only the namespaced pair. -/// -/// Every expectation here is built from the `ICloneableFactoryV4` SPEC and from -/// OpenZeppelin `Clones` — a foreign implementation of the same EIP-1167 -/// standard — never from `LibICloneableFactoryV4`. That is the point of the -/// suite. The equivalence suite already holds the concrete to the library, but -/// it states every open-salt expectation in terms of the library's own -/// `effectiveOpenSalt`, so it moves with the derivation rather than checking -/// it; the namespaced pair has had an independent oracle since -/// `testCloneDeterministicSaltIsDomainTaggedHash` and this gives the open-salt -/// pair the same. -/// -/// The revert paths (`ZeroImplementationCodeSize`, `InitializationFailed`) and -/// the `NewClone` event are deliberately NOT restated here: the equivalence -/// suite already asserts them for this entry point, field for field. 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 effective CREATE2 salt is exactly the derivation - /// `ICloneableFactoryV4` pins: - /// `keccak256(abi.encode(ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN, salt, keccak256(data)))`, - /// so an off-chain caller can reproduce the predicted address. Pinned - /// against OZ `Clones` under an independently constructed salt, so the test - /// does not restate the library's arithmetic back to itself. function testCloneDeterministicOpenSaltIsDomainTaggedHash(address implementation, bytes memory data, bytes32 salt) external view @@ -51,9 +26,6 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { 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 — the concrete's two - /// open-salt entry points held to each other, with no library in between. function testCloneDeterministicOpenSaltMatchesPredict(bytes32 salt, bytes memory data) external { TestCloneable implementation = new TestCloneable(); @@ -67,11 +39,6 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { assertEq(TestCloneable(child).sData(), data); } - /// The DEFINING property of the open-salt derivation, and the exact opposite - /// of the namespaced one: the address does not depend on the caller, so - /// every account reaches the same address for the same - /// `(implementation, data, salt)`. The spec forbids the factory mixing - /// `msg.sender`, `tx.origin` or any other caller-derived value in. function testCloneDeterministicOpenSaltIsSenderIndependent( bytes32 salt, bytes memory data, @@ -97,9 +64,6 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { assertEq(childBob, predicted); } - /// `data` is INSIDE the derivation, which is what makes an open-salt address - /// safe to pin without sender namespacing: a caller passing different `data` - /// lands somewhere else rather than occupying the address somebody pinned. function testCloneDeterministicOpenSaltCommitsToData( address implementation, bytes32 salt, @@ -114,8 +78,6 @@ contract CloneFactoryCloneDeterministicOpenSaltTest is Test { ); } - /// Distinct salts yield distinct clones of the same implementation for the - /// same initialization data — many clones per impl. function testCloneDeterministicOpenSaltManyClonesPerImpl(bytes32 salt1, bytes32 salt2, bytes memory data) external { vm.assume(salt1 != salt2); TestCloneable implementation = new TestCloneable(); From 0ddc02471c7c27bbceee5cacd867691e8afe794a Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Sun, 20 Sep 2026 14:38:02 +0000 Subject: [PATCH 5/6] test: split the open-salt suite and the scan record out of this PR This PR carried three separable things. One PR per issue, so the other two move to their own branches off `main` and their own PRs: - `test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol` closes no mutation gap (every concrete delegation mutant was already killed by the behaviour suite); it is oracle independence for the open-salt entry points and stands on its own issue. - `audit/mutation-test-scans.json` is campaign bookkeeping tied to no issue. What stays here is the one coherent unit: the five declaration-metadata tests, which close the nine Pass-A survivors that the inherited `rain-deploy` assertions cannot see. Neither removal touches the declaration suite; all three parts are independently mergeable. Co-Authored-By: Claude Opus 5 (1M context) --- audit/mutation-test-scans.json | 20 ----- ...oneFactoryCloneDeterministicOpenSalt.t.sol | 89 ------------------- 2 files changed, 109 deletions(-) delete mode 100644 audit/mutation-test-scans.json delete mode 100644 test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol diff --git a/audit/mutation-test-scans.json b/audit/mutation-test-scans.json deleted file mode 100644 index 1c00afd..0000000 --- a/audit/mutation-test-scans.json +++ /dev/null @@ -1,20 +0,0 @@ -[ - { - "timestamp": "2026-09-20T10:34:00Z", - "commit": "5c165a880ac105d6c038cee096500f76849a2190", - "testsAfterCommit": "70d4e18886c0b681e1d632d7153c1642fb3266e3", - "publishedTag": "sol-v0.1.10", - "commitsAheadOfTag": 6, - "scope": "whole repo", - "tool": "adversarial-mutation-test", - "skillVersion": "0.35.0", - "summary": { - "behaviours": 59, - "candidates": 7, - "confirmed": 4, - "testsBefore": 28, - "testsAfter": 38, - "filed": ["#33", "#34", "#35", "#36"] - } - } -] diff --git a/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol b/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol deleted file mode 100644 index e7aed1a..0000000 --- a/test/src/concrete/CloneFactoryCloneDeterministicOpenSalt.t.sol +++ /dev/null @@ -1,89 +0,0 @@ -// SPDX-License-Identifier: LicenseRef-DCL-1.0 -// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd -pragma solidity =0.8.25; - -import {Test} from "forge-std-1.16.2/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_FACTORY_V4_OPEN_SALT_DOMAIN} from "rain-factory-0.1.9/src/interface/ICloneableFactoryV4.sol"; -import {CloneFactory} from "../../../src/concrete/CloneFactory.sol"; -import {TestCloneable} from "./TestCloneable.sol"; - -contract CloneFactoryCloneDeterministicOpenSaltTest is Test { - CloneFactory internal immutable I_CLONE_FACTORY; - - constructor() { - I_CLONE_FACTORY = new CloneFactory(); - } - - 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); - } - - function testCloneDeterministicOpenSaltMatchesPredict(bytes32 salt, bytes memory data) external { - TestCloneable implementation = new TestCloneable(); - - address predicted = I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(address(implementation), data, 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); - } - - function testCloneDeterministicOpenSaltIsSenderIndependent( - 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), data, 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, predicted); - assertEq(childBob, predicted); - } - - function testCloneDeterministicOpenSaltCommitsToData( - address implementation, - bytes32 salt, - bytes memory data, - bytes memory dataOther - ) external view { - vm.assume(keccak256(data) != keccak256(dataOther)); - - assertTrue( - I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, data, salt) - != I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, dataOther, salt) - ); - } - - 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); - } -} From 2328de95d3c1b1569a0bbae26c86a724542a51f5 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Mon, 21 Sep 2026 10:08:39 +0000 Subject: [PATCH 6/6] test: resolve the declared artifact path through forge, not file text `vm.contains(vm.readFile(path), "contract ")` is a substring match over file text: it is satisfied by the name in a comment, and by a longer declaration, so `contract CloneFactoryV2` satisfied it for `CloneFactory`. `vm.getCode` is forge's own resolution of a `:` artifact id, the same form `forge verify-contract` takes. The file check stays because `vm.getCode` matches by path suffix. Co-Authored-By: Claude Opus 5 (1M context) --- test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol b/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol index 2d995ea..0024917 100644 --- a/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol +++ b/test/src/abstract/CloneFactoryDeploySuitesDeclaration.t.sol @@ -77,10 +77,12 @@ contract CloneFactoryDeploySuitesDeclarationTest is CloneFactoryDeploySuites, Te string[] memory parts = vm.split(suites[i].artifactPath, ":"); assertEq(parts.length, 2, string.concat("artifactPath is not :: ", suites[i].artifactPath)); + // `vm.getCode` matches an artifact id by path SUFFIX, so the file + // check stays: a path that resolves can still name no file. assertTrue(vm.exists(parts[0]), string.concat("artifactPath names no such file: ", parts[0])); assertTrue( - vm.contains(vm.readFile(parts[0]), string.concat("contract ", parts[1])), - string.concat("artifactPath file declares no such contract: ", suites[i].artifactPath) + vm.getCode(suites[i].artifactPath).length > 0, + string.concat("artifactPath resolves to no deployable artifact: ", suites[i].artifactPath) ); } }