fix(deploy): report the zero address when the Zoltu factory call fails

[thedavidmeister]

Closes #17

deployZoltu read its call output buffer unconditionally. The EVM copies revert data into the output region for failed calls too, so a factory call that reverted with at least 20 bytes left the first 20 of those bytes at memory [12, 32) — exactly where the factory's raw 20-byte address would be — and they became deployedAddress. All four operator-facing reports on the failure path (the two console2.log lines, the codehash log, and the DeployFailed revert argument) then showed a fabricated address, with a real code length and codehash for that fabricated address.

The buffer is now read only when the call succeeded, so a failed call reports address(0). DeployFailed's NatSpec states that invariant. The success path is untouched — mstore(0, 0) is still what makes a short return read as zero rather than as stale scratch.

Diagnostics-only, as the issue rates it: the guard already short-circuits on !success, so a failed call could never be mistaken for a successful deployment.

The MockAddressRevertingFactory fixture is byte-identical (blob 4b38fea) to the one on #19, so the two branches merge without an add/add conflict in either order.

QA

• Discriminating tests: testDeployZoltuFailedCallReportsZeroAddress — fails on base (mutating the fix back to base's unconditional deployedAddress := mload(0) produces DeployFailed(false, 0x7A0D94F55792C434d74a40883C6ed8545E406D12) != DeployFailed(false, 0x0000000000000000000000000000000000000000), reproducing the issue's exact reported value; whole suite 26 passed / 1 failed, the 1 being this test)
• Mutations applied: if success { deployedAddress := mload(0) } → deployedAddress := mload(0) (base) → killed by testDeployZoltuFailedCallReportsZeroAddress; if success → if iszero(success) → killed by 8 tests including testDeployZoltuFailedCallReportsZeroAddress and testDeployZoltu (pins both directions of the guard); deployedAddress := mload(0) → deployedAddress := 0 → killed by 7 existing happy-path tests; mstore(0, 0) → mstore(0, not(0)) → killed by testDeployZoltuRevertsNoFactory (DeployFailed(true, 0xFfFf…FF)), proving the pre-zero stays load-bearing after the fix; call(…, 12, 20) → call(…, 12, 0) → killed by 7 existing tests; dropping !success || from the guard → SURVIVED and is provably equivalent post-fix, since deployedAddress is assigned only inside if success so !success implies deployedAddress == address(0) and the second term always fires first — no test added for it. Fixture integrity was checked by applying the base-code mutation together with revert(0, 20) → revert(0, 0) in the mock: the test then passes, so its discriminating power comes specifically from the 20-byte address-shaped revert data. Baseline pinned by commit before the first probe; every mutation restored via VCS; tree verified clean; 27 passed / 0 failed on the restored tree.
• Oracle: EVM CALL semantics — return data is copied into [retOffset, retOffset+min(retSize, returndatasize)) for reverted calls as well as successful ones, so bytes in that region after a failure are revert data and not an address. Expected value address(0) is derived from DeployFailed's own contract (a failed deploy produced no address), not from the implementation's output. The failing-base value 0x7A0D…D12 is independently known: it is the etched fixture's own address, which the mock reverts with by construction.
• Category check: issue asks for one thing — stop reporting revert data as deployedAddress on the failure path (it lists the four report sites as symptoms of that single read) — covered by guarding the read, which fixes all four sites at their common source.

Co-Authored-By: Claude noreply@anthropic.com

Summary by CodeRabbit

• Bug Fixes

  • Deployment failures now consistently report a zero deployed address, preventing misleading address values from failed calls.
  • Improved error details clarify the address reported when deployment fails.

• Tests

  • Added coverage for failed deployments that return address-like revert data.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 75c6ccec-4892-4371-84fa-dcb87b98c4b4

📥 Commits

Reviewing files that changed from the base of the PR and between 4422e29 and c0d48cf.

📒 Files selected for processing (3)

• src/lib/LibRainDeploy.sol
• test/src/lib/LibRainDeploy.t.sol
• test/src/lib/MockAddressRevertingFactory.sol

---

Walkthrough

deployZoltu now reports address(0) when the factory call fails instead of interpreting revert data as a deployed address. A reverting mock factory and regression test verify this failure-path behavior.

Changes

Zoltu failure handling

Layer / File(s)	Summary
Guard deployed address extraction src/lib/LibRainDeploy.sol	deployZoltu reads the returned address only when the factory call succeeds, and DeployFailed documentation specifies zero on failure.
Validate failed-call reporting test/src/lib/MockAddressRevertingFactory.sol, test/src/lib/LibRainDeploy.t.sol	A mock factory reverts with 20 address-like bytes, and a fork-based test expects DeployFailed(false, address(0)).

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 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 concisely describes the main fix to report zero address on failed Zoltu deployments.
Linked Issues check	✅ Passed	The code and tests address issue #17 by guarding the output read on success and reporting address(0) on failure.
Out of Scope Changes check	✅ Passed	The added mock and test support the reported-address fix and do not introduce unrelated behavior.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 2026-07-29-issue-17-failed-deploy-address

---

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]

🤖 ai:vetter
Reviewed c0d48cf: ready — Closes #17 — guards the deployZoltu output-buffer read with if success, so a failed factory call reports address(0) instead of revert bytes; fix sits at the common source of all four report sites, discriminating fork test pins DeployFailed(false, 0) against the mock's address-shaped revert data, QA block (fails-on-base + 6 mutations + EVM CALL-semantics oracle) holds against the diff.
cost 355 — assembly read-guard, diagnostics-only path

[thedavidmeister]

🤖 ai:vetter
vet-protocol 1
Reviewed c0d48cf: ready — Closes #17 — guards the deployZoltu output-buffer read with if success so a failed factory call reports address(0) instead of revert bytes, fixing all four report sites at their common source; discriminating fork test pins DeployFailed(false, 0) against address-shaped revert data with specific selector+args expectRevert; QA block (fails-on-base reproducing the exact fabricated address, six mutation kills, one provably-equivalent survivor disclosed, fixture-integrity check) holds against this diff.
cost 349 — assembly read-guard, diagnostics-only path

[thedavidmeister]

👤 human
Ruled c0d48cf: reject — Wrong pragma on the new concrete contract. test/src/lib/MockAddressRevertingFactory.sol is added by this PR with pragma solidity ^0.8.25;, and it is a concrete contract — a test mock. The org convention is ^ for library and abstract files, = for concrete contracts INCLUDING concrete test mocks. Change it to =0.8.25. Scope: only the file this PR adds. The neighbouring mocks MockDeployable.sol and MockReverter.sol carry the same ^0.8.25 and are equally wrong, but they are pre-existing and belong in their own cleanup — do not sweep them here. Everything else in this PR is sound and should survive the rework untouched: the guard sits at the common source of all four report sites, the oracle is EVM CALL semantics rather than the implementation's own output, the fails-on-base value is the etched fixture's own address, the surviving mutant is disclosed with a provable equivalence argument, and the fixture's discriminating power was itself verified by mutating the mock's revert length. Keep all of it.