test: pin atomicity by redeploying at the address, not by reading code length

[thedavidmeister]

Closes #166

Three tests ended with assertEq(predicted.code.length, 0) under NatSpec that
read it as proof of atomicity. It proves nothing. The call above it is wrapped
in vm.expectRevert, so the EVM has already discarded the reverted frame
including the CREATE2, and predicted held no code before the call — the
assertion is true for every possible library, including one that deployed an
uninitialized clone or deployed at a different salt entirely. A library that
does not revert fails the expectRevert first. The assertion could not change
value if atomicity broke.

Each site now asserts the claim its NatSpec makes — after the failure the
effective salt is still free and the intended clone still deploys at the address
that was predicted:

• testCloneDeterministicInitializeFailureFails — data is outside the
  namespaced derivation, so the retry simply passes
  abi.encode(ICLONEABLE_V2_SUCCESS).
• testCloneDeterministicInitializeRevertBubbles and
  testCloneDeterministicOpenSaltInitializeFailureFails — data is fixed (or
  inside the derivation), so instead the implementation's code is swapped with
  vm.etch. The clone address commits to the implementation's ADDRESS and not
  to its code, which is exactly why the prediction survives the swap; the retry
  then lands at the same predicted and the clone carries the right sData.

No test gained or lost: three were rewritten.

QA

• Discriminating tests: the three rewritten tests
  (test/src/lib/LibICloneableFactoryV4.cloneDeterministic.t.sol,
  test/src/lib/LibICloneableFactoryV4.cloneDeterministicOpenSalt.t.sol).
• Mutations applied: 1 — swap the two derivations in the entry points, so
  cloneDeterministic deploys at the open-salt address and
  cloneDeterministicOpenSalt at the namespaced one. This is the atomicity-ish
  breakage the old assertions claimed to catch: the salt the test predicted is
  NOT the salt the library consumes. On main all three OLD tests PASS under it
  at 2048 runs each — the assertions cannot fail. On this branch all three NEW
  ones FAIL. Unmutated, the whole suite is 78 passed / 0 failed.
• Oracle: EVM revert semantics. A reverted frame's state changes are
  discarded, so predicted.code.length is 0 after any reverting library and the
  old assertion carries no information; the redeploy is the only observation
  that distinguishes "the salt is still free" from "it is not".
• Category check: the category is "an assertion placed after a
  vm.expectRevert that the revert itself makes true". grep -rn "code.length" test/
  over the whole tree leaves two other sites, neither in that category:
  …OpenSaltDisjointTagsCloseTheSquat asserts open.code.length == 0 after a
  SUCCESSFUL attacker deploy — it could fail if the two domain tags collided,
  and the next lines deploy there for real — and
  …CodeGuardRunsBeforeCreate2 reads the implementation's own code
  after vm.etch. The remaining hits are vm.assume guards and
  assertTrue(child.code.length > 0) on successful deploys.

Touches test/src/lib/LibICloneableFactoryV4.cloneDeterministic.t.sol (also in
the PR for #122, about 25 lines apart) and
test/src/lib/LibICloneableFactoryV4.cloneDeterministicOpenSalt.t.sol (also in
the PR for #123, which changes the imports and the tail of the file). All branch
off main.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

Next included review available in 28 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 28765720-cced-4c56-8ef4-882b3e879429

📥 Commits

Reviewing files that changed from the base of the PR and between f2d9e5a and 366cab7.

📒 Files selected for processing (2)

• test/src/lib/LibICloneableFactoryV4.cloneDeterministic.t.sol
• test/src/lib/LibICloneableFactoryV4.cloneDeterministicOpenSalt.t.sol

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}