[0xDEnYO]

Which Linear task belongs to this PR?

EXSC-631 — https://linear.app/lifi-linear/issue/EXSC-631/deploy-network-skill-agent-driven-staged-new-network-bring-up

Why did I implement it this way?

Adds a deploy-network skill so an agent can drive a full new-network production bring-up
(deployAllContracts.sh / scriptMaster use case 3) end to end — the gap between add-network
(config) and multisig-rollout (upgrades to existing chains). Built and hardened during the live
Injective (chainId 1776) production deploy, which is its shakedown.

Two things blocked agent-driven use of deployAllContracts: the agent Bash tool hard-kills any
command at ~10 min, and the script is interactive (gum/read). This PR makes it drivable
one stage per fresh shell, non-interactively, relying on the deploy's existing CREATE3 +
diamondCut idempotency for safe per-stage re-runs.

Script changes (additive, backward-compatible — default interactive behavior unchanged)

• script/deploy/deployAllContracts.sh
  • Optional START_STAGE/END_STAGE env bounds the stage range and skips the interactive gum
    stage menu when preset. Each stage guard is bounded to [START_STAGE, END_STAGE]; a bounded
    run ending at 11 stops cleanly without the stage-12 ownership-transfer prompt.
  • NON_INTERACTIVE=true accepts the computed defaults at the stage-9 funding prompts.
  • Sources deploySingleContract.sh itself (was relying on scriptMaster to source it → stage 3
    failed deploySingleContract: command not found under direct invocation). Now self-contained.
• script/tasks/verifyERC20ProxyAuthorization.sh: NON_INTERACTIVE=true auto-confirms owner
  funding with the default (stage 10).

The skill (.agents/commands/deploy-network.md + .cursor/.claude symlinks)

New-network bootstrap only (direct-to-diamond, pre-ownership-transfer); stops after stage 11. The
body encodes every rough edge the Injective deploy surfaced so the next one is turnkey:

• Ordered preflight: submodules → node_modules → typechain (TS deploy helpers crash
  without it) → forge clean && forge build → GLOBAL_FILE_PATH sanity (a stale
  ./../config/global.json silently broke stage 2) → verifier-subdomain probe (blockscout
  verify API is often a separate *-api host; wrong URL → 404s misread as gateway timeouts →
  retry-throttling) → premium RPC (public Injective RPC ≈1.4s/call → 30-60s forking per
  contract; Alchemy ≈2× faster) → corePeriphery ↔ target-state reconciliation (caught
  OutputValidator missing from the sheet and deprecated FeeCollector still expected) → version
  currency (ERC20Proxy must be ≥1.2.0 or the Executor isn't pre-authorized).
• Execution mechanics: run under bash -c (agent shell is zsh; scripts use read -a), which
  env overrides survive the script's .env re-source vs. which need a file edit, idempotent
  per-stage re-run with a cap, huge-output handling.
• Verification: deploy with verification off, then a parallel, rate-limit-aware sweep
  (Blockscout indexes lag inline verification; a post-deploy sweep is faster and more reliable).
  Master log is in MongoDB — bun mongo-logs:sync refreshes the local cache.

Still to land on this branch before marking ready

• Prepare for next audit #2 verifyNetworkContractsParallel helper (parallel, network-scoped, Blockscout
  rate-limit backoff) — referenced by the skill's Phase 2.
• fix: links to docs and api #3 short-circuit already-deployed contracts (skip the forge script fork via getDeployed
  • cast code + version check) — biggest win on re-runs.

Checklist before requesting a review

• I have performed a self-review of my code
• This pull request is as small as possible and only tackles one problem
• I have added tests that cover the functionality / test the bug
• For new facets: I have checked all points from this list: https://www.notion.so/lifi/New-Facet-Contract-Checklist-157f0ff14ac78095a2b8f999d655622e
• I have updated any required documentation

Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)

• I have checked that any arbitrary calls to external contracts are validated and or restricted
• I have checked that any privileged calls (i.e. storage modifications) are validated and or restricted
• I have ensured that any new contracts have had AT A MINIMUM 1 preliminary audit conducted on by <company/auditor>

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 10 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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f5adf809-8465-4646-81e3-c34cbe8c4279

📥 Commits

Reviewing files that changed from the base of the PR and between c8383a3 and 1b6a59b.

📒 Files selected for processing (5)

• .agents/commands/deploy-network.md
• script/deploy/deployAllContracts.sh
• script/deploy/deployCoreFacets.sh
• script/deploy/deploySingleContract.sh
• script/helperFunctions.sh

Walkthrough

Adds a deploy-network runbook and linked command entrypoints, bounded stage execution through health checks, deployment reuse and verification override handling, non-interactive owner funding, and parallel contract verification with rate-limit retries.

Changes

Network deployment workflow

Layer / File(s)	Summary
Deployment runbook and command entrypoints .agents/commands/deploy-network.md, .claude/skills/deploy-network/SKILL.md, .cursor/commands/deploy-network.md	Documents preflight, staged deployment, verification, artifact handling, ownership hand-offs, and links both command entrypoints to the shared runbook.
Bounded staged deployment script/deploy/deployAllContracts.sh	Adds validated start/end stage ranges, stage gating, verification override preservation, non-interactive wallet funding, and early termination before stage 12.
Deployment reuse and verification script/deploy/deploySingleContract.sh, script/deploy/deployCoreFacets.sh, script/deploy/deployFacetAndAddToDiamond.sh	Preserves verification settings across .env reloads, reuses eligible deployments, recovers constructor arguments, and applies the effective verification flag.
Owner funding control script/tasks/verifyERC20ProxyAuthorization.sh	Adds balance-based funding decisions with fail-closed lookup handling and separate interactive and non-interactive paths.
Parallel contract verification script/helperFunctions.sh	Adds rate-limit retry matching and concurrency-limited verification that updates successful deployment log entries.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• lifinance/contracts#2105: Both changes modify verifyContract and MongoDB-driven contract verification behavior.

Suggested labels: NewNetwork

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly names the new deploy-network skill and staged new-network bring-up, matching the main change.
Description check	✅ Passed	The description includes the required task, rationale, and reviewer checklists, and is mostly complete despite some unchecked items.
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch deploy-network-staged

---

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: 5

🧹 Nitpick comments (1)

script/deploy/deploySingleContract.sh (1)

282-282: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use the required Bash conditional form.

-  while [ $attempts -le "$MAX_ATTEMPTS_PER_CONTRACT_DEPLOYMENT" ] && [[ -z "$ADDRESS" ]]; do
+  while [[ "$attempts" -le "$MAX_ATTEMPTS_PER_CONTRACT_DEPLOYMENT" && -z "$ADDRESS" ]]; do

As per coding guidelines, “Use [[ ]] for conditional expressions instead of [ ]” and always quote variable references.

🤖 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 `@script/deploy/deploySingleContract.sh` at line 282, Update the while
condition around the deployment retry loop to use Bash’s [[ ]] conditional
syntax consistently, including the attempts comparison and ADDRESS emptiness
check. Quote variable references within the condition while preserving the
existing retry behavior.

Source: Coding guidelines

🤖 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 @.agents/commands/deploy-network.md:
- Around line 108-121: The Phase 1 deployment instructions do not reliably
disable verification because deployAllContracts sources .env after the inline
environment assignment. Update the command guidance around deployAllContracts so
VERIFY_CONTRACTS=false remains effective, either by explicitly instructing users
to edit and restore .env for the run or by preserving the caller-provided
override in the deployment script; ensure the documented command truly avoids
inline verification.

In `@script/deploy/deploySingleContract.sh`:
- Around line 266-280: Before the short-circuit in deploySingleContract, clear
stale RAW_RETURN_DATA and load the constructor arguments from the deployment-log
entry matching CONTRACT and CONTRACT_ADDRESS. Ensure the reuse path uses these
loaded arguments for CONSTRUCTOR_ARGS, while preserving normal forge execution
and output handling for newly deployed contracts.

In `@script/helperFunctions.sh`:
- Around line 2634-2635: Check the return status of logContractDeploymentInfo
before incrementing OK in the deployment-processing flow. On failure, add the
contract to FAILED and skip the success increment; only increment OK when the
deployment-log update succeeds, preserving the existing summary and verification
behavior.
- Around line 2567-2572: Update the concurrency setup near
VERIFICATION_MAX_CONCURRENT_JOBS to read MAX_CONCURRENT_JOBS with
${MAX_CONCURRENT_JOBS:-}, then validate that any provided value is a positive
integer before dispatch. Reject zero, negative, and non-numeric values so the
throttle cannot wait indefinitely, while preserving the configured verification
concurrency behavior for valid inputs.

In `@script/tasks/verifyERC20ProxyAuthorization.sh`:
- Around line 110-119: Update the funding decision in
verifyERC20ProxyAuthorization around DEFAULT_FUND_AMOUNT and DO_FUND_OWNER so
NON_INTERACTIVE mode automatically funds only when OWNER_BALANCE is below the
required amount. Preserve the existing interactive confirmation behavior, and
avoid sending funds on reruns when the owner already has sufficient balance.

---

Nitpick comments:
In `@script/deploy/deploySingleContract.sh`:
- Line 282: Update the while condition around the deployment retry loop to use
Bash’s [[ ]] conditional syntax consistently, including the attempts comparison
and ADDRESS emptiness check. Quote variable references within the condition
while preserving the existing retry behavior.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ed3423b0-d433-44c2-839a-035449b56546

📥 Commits

Reviewing files that changed from the base of the PR and between 7206885 and 70097a2.

📒 Files selected for processing (7)

• .agents/commands/deploy-network.md
• .claude/skills/deploy-network/SKILL.md
• .cursor/commands/deploy-network.md
• script/deploy/deployAllContracts.sh
• script/deploy/deploySingleContract.sh
• script/helperFunctions.sh
• script/tasks/verifyERC20ProxyAuthorization.sh

[gvladika]

Posted two inline findings from a detailed review (VERIFY_CONTRACTS override scope, and constructor-args recovery in the skip path).

[coderabbitai]

Actionable comments posted: 3

Caution

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

⚠️ Outside diff range comments (2)

script/deploy/deployAllContracts.sh (2)

475-480: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not return before deployment-log synchronization.

For an END_STAGE=11 run, this return bypasses updateDiamondLogForNetwork at lines 558-560. Let execution reach the common tail after suppressing the Stage 12 prompt, so the final bounded deployment still synchronizes its logs.

🤖 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 `@script/deploy/deployAllContracts.sh` around lines 475 - 480, Replace the
early return in the END_STAGE boundary block of deployAllContracts with control
flow that suppresses only the Stage 12 ownership-transfer prompt, then continues
into the common tail. Ensure updateDiamondLogForNetwork executes for
END_STAGE=11 while preserving the bounded-run completion messages.

---

72-73: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep stage bounds local to this invocation.

END_STAGE is assigned globally, and START_STAGE is later assigned globally. A subsequent call in the same shell can inherit the prior range and skip stage selection. Initialize both as locals from the caller environment.

As per coding guidelines, “Use local keyword when declaring variables inside functions.”

🤖 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 `@script/deploy/deployAllContracts.sh` around lines 72 - 73, Update the
stage-selection logic in the deployment function containing END_STAGE and
START_STAGE so both variables are declared local while preserving their
caller-provided defaults. Ensure each invocation initializes its own range and
does not leak or inherit modified stage bounds from previous calls.

Source: Coding guidelines

🤖 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 `@node_modules`:
- Line 1: Remove the machine-local node_modules symbolic link from version
control, leaving the generated dependency directory untracked and managed by the
existing ignore configuration.

In `@script/deploy/deploySingleContract.sh`:
- Around line 292-305: The constructor arguments loaded in the deployment
recovery flow must be accepted only when the matched master-log entry’s ADDRESS
equals CONTRACT_ADDRESS case-insensitively. Update the logic around
findContractInMasterLog and CONSTRUCTOR_ARGS_FROM_LOG to inspect the entry
address before assigning its constructor args; on mismatch, leave
CONSTRUCTOR_ARGS_FROM_LOG empty so the existing warning and placeholder fallback
execute.

In `@script/tasks/verifyERC20ProxyAuthorization.sh`:
- Around line 113-119: Update the balance handling around OWNER_BALANCE and the
funding gate so a failed cast balance lookup returns before any NON_INTERACTIVE
auto-funding occurs. Track the command’s exit status separately from a genuinely
retrieved low balance, and remove the empty OWNER_BALANCE condition from
OWNER_NEEDS_FUNDING so missing data fails closed while valid balances below
DEFAULT_FUND_AMOUNT still trigger funding.

---

Outside diff comments:
In `@script/deploy/deployAllContracts.sh`:
- Around line 475-480: Replace the early return in the END_STAGE boundary block
of deployAllContracts with control flow that suppresses only the Stage 12
ownership-transfer prompt, then continues into the common tail. Ensure
updateDiamondLogForNetwork executes for END_STAGE=11 while preserving the
bounded-run completion messages.
- Around line 72-73: Update the stage-selection logic in the deployment function
containing END_STAGE and START_STAGE so both variables are declared local while
preserving their caller-provided defaults. Ensure each invocation initializes
its own range and does not leak or inherit modified stage bounds from previous
calls.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 46ef0007-a703-4f5d-aa36-5964f5179316

📥 Commits

Reviewing files that changed from the base of the PR and between 70097a2 and 2b99d4a.

📒 Files selected for processing (8)

• .agents/commands/deploy-network.md
• node_modules
• script/deploy/deployAllContracts.sh
• script/deploy/deployCoreFacets.sh
• script/deploy/deployFacetAndAddToDiamond.sh
• script/deploy/deploySingleContract.sh
• script/helperFunctions.sh
• script/tasks/verifyERC20ProxyAuthorization.sh

🚧 Files skipped from review as they are similar to previous changes (2)

• script/helperFunctions.sh
• .agents/commands/deploy-network.md

[0xDEnYO]

Overnight gate-review pass on this PR (commit 1b6a59b). Auto-fixed in that commit:

• VERIFY_CONTRACTS=false was silently ignored in the skill's Phase-1 command: deployAllContracts/deployCoreFacets/deploySingleContract sourced script/helperFunctions.sh (which re-sources .env at file level via set -a) before promoting the caller's value into VERIFY_CONTRACTS_OVERRIDE, so the promotion captured the .env value. Reproduced with a controlled .env (OVERRIDE ended up true); the promotion now runs before any source.
• verifyNetworkContractsParallel now passes the logged solc/EVM/optimizer-runs to verifyContract — keeps the parallel sweep consistent with the sequential one after chore(deploy): targetEvmVersion rename + DB-driven re-verify + cancun flip (EXSC-656) #2119's toolchain pinning (extra positional args are no-ops until this branch picks up main).
• Reset NEW_DEPLOYMENT/CONTRACT_ADDRESS before the CREATE3/zkEVM split so stale globals from a previous same-shell call can't trigger the already-deployed short-circuit with another network's address.
• Skill md: corrected the parallel-verification concurrency cap (VERIFICATION_MAX_CONCURRENT_JOBS, default 4).

Escalated (needs a human decision):

1. Branch is ~68 commits behind main, and main has since changed both overlapping files (deploySingleContract.sh, helperFunctions.sh: chore(deploy): targetEvmVersion rename + DB-driven re-verify + cancun flip (EXSC-656) #2119 toolchain-pinned re-verify + cancun flip, fix(deploy): apply customDeployFlags on the zkEVM deploy path (EXSC-684) #2150 zkEVM customDeployFlags, fix(EXSC-657): derive --legacy from on-chain EIP-1559 support #2080 EIP-1559 legacy-flag derivation). The merge is textually clean; recommend updating the branch with main before final review so deploy-smoke-test runs against the merged state.

Lower-confidence (human judgment): a NON_INTERACTIVE full-stage run could still hit gum choose prompts inside inner task scripts (diamondUpdateFacet/diamondSyncWhitelist/diamondUpdatePeriphery) if their inputs are ever missing — not proven to fire on the deployAllContracts path (the Injective shakedown covered it), so left as-is.

[0xDEnYO]

Closing as stale rather than carrying it into the review queue.

This branch was built and hardened during the Injective (chainId 1776) bring-up, which shipped weeks ago — the value it was written to capture has already been banked in that live deploy. Since then the scripts it documents and modifies have been substantially rewritten underneath it:

• deploySingleContract.sh and helperFunctions.sh — salt derivation reworked in fix(deploy): derive deploy salts from a guaranteed artifact on both deploy paths (EXSC-805, EXSC-808) #2234 (EXSC-805 / EXSC-808)
• scriptMaster.sh target-state generation — fix(deploy): make target-state generation fail loudly instead of silently wiping entries #2148

so the staged-bring-up procedure described here no longer matches the scripts it drives, and validating it would cost more than rewriting it against main at the point we next bring up a network.

EXSC-631 stays open. When the next new-network bring-up comes round, that's the moment to re-derive this from the scripts as they actually are, with a live deploy as the shakedown again.