Raise InitializationFailed for every non-conforming initialize answer

[thedavidmeister]

cloneAndInitialize raises InitializationFailed when the clone's initialize answers anything but the 32-byte ICLONEABLE_V2_SUCCESS. A revert with data still bubbles verbatim.

Before, a high-level call ran the ABI decoder before the sentinel check. So a no-data revert (no initialize and no fallback, or a bare revert()), an empty return (an ICloneableV1, a silent fallback) or a short return came out as bare 0x, and an over-long return starting with the sentinel was accepted. Now it is a low-level call. A revert with data bubbles. Every other answer that is not exactly the 32-byte sentinel reverts InitializationFailed. A failed call that reaches the final check has no return data, so the length check rejects it without a separate !success.

Migration

• A clone of a non-ICloneableV2 implementation (ICloneableV1 included) now reverts InitializationFailed (0x19b991a8) instead of empty data. So does an out-of-gas inside initialize: it is also a no-data revert.
• An initialize that returns more than 32 bytes starting with the sentinel no longer clones.

Closes #61

QA

• Head 4ca99fe is the trim plus main 79130af, merged cleanly. The full suite passes locally (54 passed, 0 failed). CI is green on 4ca99fe (test, legal, static).
• One fixture, TestCloneableRawAnswer, reverts with or returns whatever raw bytes the test chooses. Each test runs through both cloneDeterministic and cloneDeterministicOpenSalt.
• Mutation probe on 4ca99fe, full suite per mutant (baseline 54 passed): 14 mutants: 13 killed, 1 survived, 0 no-run. M00 is main's high-level call. M01: != 32 → < 32. M02: length check dropped. M03: empty return exempt from the length check. M04: never bubble. M05: bubble the no-data revert too. M06: bubbled data truncated. M07: sentinel comparison inverted. M08: sentinel constant changed. M09: data not forwarded. M10: EIP-1167 suffix changed. M11: !success || put back. M12: bubble guard > 0 → > 32, so a revert of up to 32 bytes reaches the final check. M13: a successful empty return accepted. M11 survives because it is equivalent: a failed call that reaches the final check has empty return data, so the length check rejects it anyway. Main's clone-flow tests kill M07–M10.
• Each kept test, probed alone against the same 14 mutants:
  • testInitializeReturnsNothingRevertsInitializationFailed kills M00, M02, M03 and M13. It is the only test that kills M13.
  • testInitializeReturnsSentinelWithTrailingBytesRevertsInitializationFailed kills M00, M01 and M02. It is the only test that kills M01.
  • testInitializeRevertsWithoutDataRevertsInitializationFailed kills M00, M02, M03 and M05. It is the only test that kills M05.
  • testInitializeRevertsWithSentinelBubblesVerbatim kills M04, M06, M09 and M12. It is the only test that kills M12.
• Oracle: ICloneableV2.initialize NatSpec (MUST return keccak256("ICloneableV2.initialize"), which exists so that non-support is caught), ICloneableFactoryV3.cloneDeterministic (the clone is created ONLY if that hash is returned), a bytes32 return being exactly 32 bytes, and ICloneableV1.initialize (same selector, no return value).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Clone initialization now consistently validates successful responses using the expected success value and exact response length.
  • Initialization failures with revert data preserve and propagate the original data unchanged.
  • Empty, malformed, or unsuccessful initialization responses now produce a consistent initialization failure.
  • Deterministic and open-salt clone creation paths now follow the same initialization error-handling behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 75fcbd1a-af2b-4298-a7f5-7a8fe7f39864

📥 Commits

Reviewing files that changed from the base of the PR and between 0c4537a and 34dc279.

📒 Files selected for processing (3)

• src/lib/LibICloneableFactoryV4.sol
• test/concrete/TestCloneableRawAnswer.sol
• test/src/lib/LibICloneableFactoryV4.cloneAndInitialize.t.sol

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Walkthrough

The factory now performs initialization with a low-level call. It propagates non-empty revert data and raises InitializationFailed for empty or invalid responses. New tests cover both clone entry points and multiple response formats.

Changes

Initialization response handling

Layer / File(s)	Summary
Factory response validation src/lib/LibICloneableFactoryV4.sol	cloneAndInitialize now propagates non-empty revert data and accepts only the exact 32-byte ICLONEABLE_V2_SUCCESS response. Other responses raise InitializationFailed.
Initialization response tests test/concrete/TestCloneableRawAnswer.sol, test/src/lib/LibICloneableFactoryV4.cloneAndInitialize.t.sol	Tests cover empty returns, trailing data, empty reverts, and sentinel revert data through both clone entry points.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: non-conforming initialize responses now raise InitializationFailed.
Linked Issues check	✅ Passed	Issue #61 requires both deterministic clone entry points to accept only an exact 32-byte ICLONEABLE_V2_SUCCESS response. cloneAndInitialize now uses a low-level call, bubbles non-empty revert data…
Out of Scope Changes check	✅ Passed	The source change updates the initialization response check. The added raw-answer fixture and tests directly verify the behavior required by issue #61. The documented gas and mutation-test updates sup…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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.}

[thedavidmeister]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[thedavidmeister]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.