What
CloneFactory's two open-salt entry points — cloneDeterministicOpenSalt and
predictDeterministicAddressOpenSalt — have no test oracle independent of the
library they delegate to.
Their only coverage is CloneFactoryLibEquivalence.t.sol, which states its
open-salt expectations in terms of LibICloneableFactoryV4.effectiveOpenSalt(salt, data):
bytes32 effectiveSalt = LibICloneableFactoryV4.effectiveOpenSalt(salt, data);
assertEq(
I_CLONE_FACTORY.predictDeterministicAddressOpenSalt(implementation, data, salt),
LibICloneableFactoryV4.predictCloneAddress(address(I_CLONE_FACTORY), implementation, effectiveSalt)
);
Both sides of that assertion are the code under test.
The namespaced pair does not have this problem
CloneFactoryCloneDeterministicTest.testCloneDeterministicSaltIsDomainTaggedHash
builds the effective salt from the interface constant and checks the
predicted address against OpenZeppelin Clones, a foreign EIP-1167
implementation. Its own NatSpec states the intent:
Pinned against OZ Clones — a foreign implementation of the same EIP-1167
standard — under an independently constructed salt, so the test does not
restate the library's arithmetic back to itself.
So the repo already holds the standard this issue asks for. It is applied to one
of the two derivations and not the other.
Why it matters
ICloneableFactoryV4 pins the open-salt derivation as a MUST, and spells the
preimage out twice in its own NatSpec:
keccak256(abi.encode(ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN, salt, keccak256(data)))
Nothing in this repo checks the code against that spec. A test that derives its
expectation from the implementation moves with the implementation: if
effectiveOpenSalt changed its domain separator, dropped keccak256(data) from
the preimage, or reordered the abi.encode operands, the equivalence assertions
would still pass, because both sides change together. The interface's stated
guarantee — that the open-salt and namespaced images cannot overlap, so no other
entry point on the factory can be aimed at an open-salt address — rests on that
first word being exactly this domain tag.
Scope — what this is NOT
This is not a live mutation gap, and the issue should not be read as claiming
one. A mutation probe over the concrete at 5c165a8 killed all 12 delegation
mutants with the behaviour-only suite. No surviving mutant motivates this.
The claim is narrower and is exactly this: the namespaced pair is checked against
an independent source of truth and the open-salt pair is not, so the open-salt
tests cannot detect a change to the derivation they are supposed to pin.
Proposed fix
Add the open-salt counterpart to the namespaced suite, built the way the
namespaced test is built:
- the effective salt from
ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAIN and the spec's
preimage, predicted through OZ Clones — never through LibICloneableFactoryV4
- deploy-matches-predict, through
LibExtrospectERC1167Proxy and the initialized
TestCloneable
- the derivation's two defining properties, which the namespaced pair cannot
have: the address is sender independent (that is what "open" means) and it
commits to data
Revert paths and the NewClone event do not need restating; the equivalence
suite already holds them field for field.
Found by adversarial mutation testing (skill 0.35.0), whole-repo scan of
5c165a880ac105d6c038cee096500f76849a2190. Split out of #37, which originally
carried this alongside two unrelated concerns.
What
CloneFactory's two open-salt entry points —cloneDeterministicOpenSaltandpredictDeterministicAddressOpenSalt— have no test oracle independent of thelibrary they delegate to.
Their only coverage is
CloneFactoryLibEquivalence.t.sol, which states itsopen-salt expectations in terms of
LibICloneableFactoryV4.effectiveOpenSalt(salt, data):Both sides of that assertion are the code under test.
The namespaced pair does not have this problem
CloneFactoryCloneDeterministicTest.testCloneDeterministicSaltIsDomainTaggedHashbuilds the effective salt from the interface constant and checks the
predicted address against OpenZeppelin
Clones, a foreign EIP-1167implementation. Its own NatSpec states the intent:
So the repo already holds the standard this issue asks for. It is applied to one
of the two derivations and not the other.
Why it matters
ICloneableFactoryV4pins the open-salt derivation as a MUST, and spells thepreimage out twice in its own NatSpec:
Nothing in this repo checks the code against that spec. A test that derives its
expectation from the implementation moves with the implementation: if
effectiveOpenSaltchanged its domain separator, droppedkeccak256(data)fromthe preimage, or reordered the
abi.encodeoperands, the equivalence assertionswould still pass, because both sides change together. The interface's stated
guarantee — that the open-salt and namespaced images cannot overlap, so no other
entry point on the factory can be aimed at an open-salt address — rests on that
first word being exactly this domain tag.
Scope — what this is NOT
This is not a live mutation gap, and the issue should not be read as claiming
one. A mutation probe over the concrete at
5c165a8killed all 12 delegationmutants with the behaviour-only suite. No surviving mutant motivates this.
The claim is narrower and is exactly this: the namespaced pair is checked against
an independent source of truth and the open-salt pair is not, so the open-salt
tests cannot detect a change to the derivation they are supposed to pin.
Proposed fix
Add the open-salt counterpart to the namespaced suite, built the way the
namespaced test is built:
ICLONEABLE_FACTORY_V4_OPEN_SALT_DOMAINand the spec'spreimage, predicted through OZ
Clones— never throughLibICloneableFactoryV4LibExtrospectERC1167Proxyand the initializedTestCloneablehave: the address is sender independent (that is what "open" means) and it
commits to
dataRevert paths and the
NewCloneevent do not need restating; the equivalencesuite already holds them field for field.
Found by adversarial mutation testing (skill 0.35.0), whole-repo scan of
5c165a880ac105d6c038cee096500f76849a2190. Split out of #37, which originallycarried this alongside two unrelated concerns.