fix(deploy): check dependencies and deploy on one fork per network

[thedavidmeister]

Summary

deployAndBroadcast validated dependencies and deployed in two passes that each forked every network — checkDependencies (fork, validate) then deployToNetworks (fork again at a newer head, re-read, deploy). That redundant second read on a fresh fork let a rare transient RPC inconsistency report an already-deployed dependency as having no code, falsely reverting MissingDependency and aborting an otherwise-valid deploy.

This folds the two passes into one: deployToNetworks forks each network once, verifies the Zoltu factory and every dependency have code on that fork, then broadcasts the deploy — so each dependency is read exactly once and the second fork is gone.

The separate all-network pre-flight isn't needed, because the Zoltu deploy is idempotent: an already-deployed contract is skipped, each network is independent, and a failure on one network leaves the others intact and is safely re-runnable. The per-network dependency check still runs before that network's deploy, so nothing deploys onto a network that is missing a prerequisite.

Changes

• Remove checkDependencies. deployToNetworks(vm, networks, deployer, creationCode, contractPath, expectedAddress, expectedCodeHash, dependencies) now forks each network, checks the Zoltu factory + dependencies inline, then deploys.
• deployAndBroadcast just calls deployToNetworks — no forkIds, no depCodeHashes.
• Tests: the dependency / Zoltu-factory checks now run through deployToNetworks and are mutation-validated (disabling each check fails its test, restoring it passes); added a two-network deploy test; removed the fork-reuse and checkDependencies tests; the two mocks live in their own files for the one-contract-per-file static check.

⚠️ Audit re-review needed

This is an audited library and the change is structural — please re-review.

• The deploy is now read-once per network: the dependency / Zoltu checks and the deploy run on a single fork, so there is no second, divergent snapshot for a transient RPC glitch to surface on.
• deployAndBroadcast's signature changed vs the released v0.1.3: the depCodeHashes storage-mapping parameter is gone (the codehash record/compare machinery only had meaning across two forks). Consumers must drop that argument — e.g. st0x.deploy's Deploy.sol, which I'll update on the dependency bump.
• No all-network pre-flight: justified by Zoltu idempotency + re-runnability (see Summary).

Testing

forge build + forge test green for the deploy/dependency suite (17 tests). The three inline checks are mutation-validated (each one disabled → its test fails; restored → all pass), and testDeployZoltu still pins the deterministic 0xC24016… address. The archive-history findDeployBlock / isStartBlock tests are untouched and run under CI's archive RPC.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Updated multi-network deployment to validate and deploy on the same fork to avoid inconsistencies.
  • Removed redundant dependency verification during deployment, streamlining the process.
  • Adjusted tests to match the new one-pass, fork-based behavior and ensure error handling remains correct.
• Tests
  • Added new helper contracts to simulate successful deployments and guaranteed deployment failures.

[coderabbitai]

Warning

Review limit reached

@thedavidmeister, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 64f5748d-3486-4cb2-a885-e1de8c386b86

📥 Commits

Reviewing files that changed from the base of the PR and between 503002e and 9156d28.

📒 Files selected for processing (2)

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

Walkthrough

LibRainDeploy removes the dependency-codehash preflight path and now verifies factory and dependency presence per network before deploying. The broadcast entrypoint drops the mapping parameter, and the tests are updated for the new flow and helper contracts.

Changes

Deployment flow simplification

Layer / File(s)	Summary
Network verification and broadcast wiring src/lib/LibRainDeploy.sol	deployToNetworks now checks dependency presence without stored codehashes, and deployAndBroadcast removes the mapping parameter and calls the deployment loop directly. Deployment documentation is updated to match the per-network flow.
Test wrappers and deployment cases test/src/lib/LibRainDeploy.t.sol	The test harness removes sDepCodeHashes, updates wrapper calls, revises deploy-to-network and revert coverage, and deletes the removed dependency-check and codehash-change scenarios.
Deployment helper contracts test/src/lib/MockDeployable.sol, test/src/lib/MockReverter.sol	New helper contracts provide a concrete deployment target and an always-reverting deployment path for test coverage.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 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 accurately summarizes the main change: dependency validation and deployment now happen on one fork per network.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/dep-check-reuse-validated-fork

---

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

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

test/src/lib/LibRainDeploy.t.sol (1)

356-380: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add a regression where skip wins over missing dependencies.

This skip-path test still uses an empty dependencies array, so it will not catch the ordering bug above. Make one dependency undeployed here and keep asserting success once expectedAddress already has code.

Minimal test tweak

-        address[] memory dependencies = new address[](0);
+        address[] memory dependencies = new address[](1);
+        dependencies[0] = address(0xdead);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/src/lib/LibRainDeploy.t.sol` around lines 356 - 380, The regression test
in testDeployToNetworksSkipsWhenAlreadyDeployed still uses an empty dependencies
array, so it does not exercise the skip-over-missing-dependencies path. Update
this test to include at least one undeployed dependency in the dependencies list
while keeping the predeployed expectedAddress setup and the assert that
externalDeployToNetworks returns the existing address. Use the existing test
helpers and identifiers like externalDeployToNetworks, externalDeployZoltu, and
MockDeployable to keep the scenario focused on the skip logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/LibRainDeploy.sol`:
- Around line 221-240: The deployment loop in LibRainDeploy currently validates
ZOLTU_FACTORY and other dependencies before checking whether a network is
already deployed, so idempotent reruns can still revert on MissingDependency or
DependencyChanged. Move the already-deployed skip logic to the start of the
per-network loop in the deployment routine that iterates over networks, before
any dependency validation or logging that assumes deployment is needed. Make
sure the skip path using expectedAddress runs first so previously deployed
networks are bypassed entirely.

---

Outside diff comments:
In `@test/src/lib/LibRainDeploy.t.sol`:
- Around line 356-380: The regression test in
testDeployToNetworksSkipsWhenAlreadyDeployed still uses an empty dependencies
array, so it does not exercise the skip-over-missing-dependencies path. Update
this test to include at least one undeployed dependency in the dependencies list
while keeping the predeployed expectedAddress setup and the assert that
externalDeployToNetworks returns the existing address. Use the existing test
helpers and identifiers like externalDeployToNetworks, externalDeployZoltu, and
MockDeployable to keep the scenario focused on the skip logic.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a36bf56e-a7f9-4d4c-bb95-8a84fa2a840c

📥 Commits

Reviewing files that changed from the base of the PR and between 5bfe2c4 and 503002e.

📒 Files selected for processing (4)

• src/lib/LibRainDeploy.sol
• test/src/lib/LibRainDeploy.t.sol
• test/src/lib/MockDeployable.sol
• test/src/lib/MockReverter.sol

[thedavidmeister]

Reviewed 9156d28

Final merged-design fork-reuse fix: deployToNetworks forks each network once, takes the already-deployed skip path before any dependency validation, and validates the Zoltu factory + dependencies only on the deploy path. Read-once-per-network eliminates the transient-glitch MissingDependency; no all-network pre-flight (Zoltu deploy is idempotent + re-runnable). Dependency checks and the skip-before-validate reorder are mutation-validated; slither 0 results; all CI green; CodeRabbit confirmed its Major finding resolved. Note for the record: this is an audited library — the change is structural and warrants the audit re-review flagged in the PR body. Merging per thedavidmeister's explicit go-ahead.