Unit
LibICloneableFactoryV4.cloneAndInitialize — the mandatory ICloneableV2.initialize return check — reached through both cloneDeterministic and cloneDeterministicOpenSalt (src/lib/LibICloneableFactoryV4.sol:150-170, check at line 166).
Intent oracle
src/interface/ICloneableV2.sol:38-42, on the success sentinel:
If initialization is successful the ICloneableV2 MUST return the keccak256 hash of the string "ICloneableV2.initialize". This avoids false positives where a contract building a proxy, such as an ICloneableFactoryV2, may incorrectly believe that the clone has been initialized but the implementation doesn't support ICloneableV2.
src/lib/LibICloneableFactoryV4.sol:24-26, on the typed error:
Thrown when initialization fails: ICloneableV2.initialize on the fresh clone returned something other than ICLONEABLE_V2_SUCCESS.
src/interface/ICloneableFactoryV3.sol:41-43:
The factory MUST ONLY consider the clone successfully created if initialize returns the keccak256 hash of the string "ICloneableV2.initialize".
Violated property
The stated purpose of the sentinel is to catch "the implementation doesn't support ICloneableV2". For the ordinary shapes of exactly that case, the sentinel comparison at line 166 is never reached: the ICloneableV2(child).initialize(data) call itself fails first, and the caller receives empty revert data (0x) rather than the typed InitializationFailed (0x19b991a8) that the error's own NatSpec describes.
The gap is between "returned something other than ICLONEABLE_V2_SUCCESS" (which reads as covering any non-success outcome, and is the case the sentinel exists for) and what the code actually distinguishes: only a well-formed 32-byte return that is not the sentinel.
Practical consequence: an integrator that cloned a contract which is not an ICloneableV2 cannot tell that apart from an out-of-gas, an unrelated revert inside a legitimate initialize, or any other bare failure. The typed error exists to make that distinction and does not fire.
Verified repro
Run against TestCloneFactory (the pure-delegation concrete already in test/src/concrete/) at c1c2afd, three implementations that all have non-zero code so checkImplementationCode passes:
/// Has code, but no `initialize(bytes)` and no fallback.
contract NoInitialize { uint256 public x; }
/// Has code and a fallback that returns nothing at all.
contract SilentFallback {
fallback() external { assembly { return(0, 0) } }
}
/// Returns 8 bytes instead of 32.
contract ShortReturn {
fallback() external { assembly { mstore(0, 1) return(0, 8) } }
}
For each, a raw call so the revert data is observable rather than swallowed:
(bool ok, bytes memory ret) = address(I_FACTORY).call(
abi.encodeWithSignature(
"cloneDeterministicOpenSalt(address,bytes,bytes32)",
address(impl), hex"", bytes32(0)
)
);
assertFalse(ok);
emit log_named_bytes("revert data", ret);
Observed on all three:
NoInitialize revert data: 0x
SilentFallback revert data: 0x
ShortReturn revert data: 0x
InitializationFailed selector: 0x19b991a8
cloneDeterministic behaves identically — the check is in the shared tail.
Mechanism: SilentFallback/ShortReturn return fewer than 32 bytes, so solc's own returndatasize() < 32 guard reverts with no data before the comparison; NoInitialize has no matching selector and no fallback, so the delegatecall reverts and the EIP-1167 runtime bubbles the empty revert through.
Note the sentinel does still work for the case the existing tests cover (TestCloneableFailure returns a well-formed 32-byte non-sentinel word → InitializationFailed). The uncovered shapes are the ones above.
Triage framing
Flagging, not adjudicating. Both readings seem available and I am not the one to pick:
- By design. The library is a thin factory; bubbling whatever the implementation did is the honest thing, and a bare revert still satisfies "MUST ONLY consider the clone created if
initialize returns the hash" because nothing is created. Under this reading the NatSpec on InitializationFailed and on ICLONEABLE_V2_SUCCESS is what wants narrowing, not the code.
- A defect. The sentinel's documented purpose is specifically "the implementation doesn't support
ICloneableV2", which is precisely what produces 0x here, so the typed error should cover it — e.g. a low-level call with an explicit returndatasize() == 32 && word == ICLONEABLE_V2_SUCCESS test, reverting InitializationFailed otherwise.
Either way there is currently no test over any of these three shapes, so whichever behaviour is intended is unpinned.
Found by adversarial review during AMT group g3-libicloneablefactoryv4-clone.
Unit
LibICloneableFactoryV4.cloneAndInitialize— the mandatoryICloneableV2.initializereturn check — reached through bothcloneDeterministicandcloneDeterministicOpenSalt(src/lib/LibICloneableFactoryV4.sol:150-170, check at line 166).Intent oracle
src/interface/ICloneableV2.sol:38-42, on the success sentinel:src/lib/LibICloneableFactoryV4.sol:24-26, on the typed error:src/interface/ICloneableFactoryV3.sol:41-43:Violated property
The stated purpose of the sentinel is to catch "the implementation doesn't support
ICloneableV2". For the ordinary shapes of exactly that case, the sentinel comparison at line 166 is never reached: theICloneableV2(child).initialize(data)call itself fails first, and the caller receives empty revert data (0x) rather than the typedInitializationFailed(0x19b991a8) that the error's own NatSpec describes.The gap is between "returned something other than
ICLONEABLE_V2_SUCCESS" (which reads as covering any non-success outcome, and is the case the sentinel exists for) and what the code actually distinguishes: only a well-formed 32-byte return that is not the sentinel.Practical consequence: an integrator that cloned a contract which is not an
ICloneableV2cannot tell that apart from an out-of-gas, an unrelated revert inside a legitimateinitialize, or any other bare failure. The typed error exists to make that distinction and does not fire.Verified repro
Run against
TestCloneFactory(the pure-delegation concrete already intest/src/concrete/) atc1c2afd, three implementations that all have non-zero code socheckImplementationCodepasses:For each, a raw call so the revert data is observable rather than swallowed:
Observed on all three:
cloneDeterministicbehaves identically — the check is in the shared tail.Mechanism:
SilentFallback/ShortReturnreturn fewer than 32 bytes, so solc's ownreturndatasize() < 32guard reverts with no data before the comparison;NoInitializehas no matching selector and no fallback, so the delegatecall reverts and the EIP-1167 runtime bubbles the empty revert through.Note the sentinel does still work for the case the existing tests cover (
TestCloneableFailurereturns a well-formed 32-byte non-sentinel word →InitializationFailed). The uncovered shapes are the ones above.Triage framing
Flagging, not adjudicating. Both readings seem available and I am not the one to pick:
initializereturns the hash" because nothing is created. Under this reading the NatSpec onInitializationFailedand onICLONEABLE_V2_SUCCESSis what wants narrowing, not the code.ICloneableV2", which is precisely what produces0xhere, so the typed error should cover it — e.g. a low-levelcallwith an explicitreturndatasize() == 32 && word == ICLONEABLE_V2_SUCCESStest, revertingInitializationFailedotherwise.Either way there is currently no test over any of these three shapes, so whichever behaviour is intended is unpinned.
Found by adversarial review during AMT group
g3-libicloneablefactoryv4-clone.