fix(deploy): treat a failed diamond registration as a network-level deploy failure

[0xDEnYO]

Which Linear task belongs to this PR?

https://linear.app/lifi-linear/issue/EXSC-713/fail-network-deploy-when-diamond-registration-fails

Why did I implement it this way?

Observed 2026-07-28 deploying OutputValidator v1.0.0 to robinhood (production): the CREATE3 deploy and Blockscout verification succeeded, but diamondUpdatePeriphery exhausted all 5 retries because the Safe MongoDB tunnel was down, so no Safe proposal was created. The log said:

[error] failed to register OutputValidator in diamond on network robinhood

…yet the run still ended with robinhood: OK (0x321E…56AB) and DEPLOY_EXIT=0. A production rollout that is on-chain but unregistered was indistinguishable from a complete one for anything driving the script non-interactively (CI, the deploy-contract / multisig-rollout skills).

Three independent gaps had to close for the failure to reach the exit code:

1. script/tasks/diamondUpdatePeriphery.sh never returned its failure. LAST_CALL was tracked but the function fell off the end after an echo, so it always returned 0. It is now returned, and made sticky (LAST_CALL is only ever set to 1) — previously a later contract's success overwrote an earlier contract's failure in the multi-contract loop. register() also got an explicit return 0 on its success path.
2. deployAndAddContractToDiamond (script/helperFunctions.sh) combined the two step results with ||, so a successful deploy alone was enough to report success. Changed to &&. The other || uses of RETURN_CODE1/2 in that file are display-only functions (printDeploymentsStatus*) and were deliberately left alone.
3. deployToNetworkWorker (script/deploy/deployContractToNetworks.sh) was already propagating the return code correctly, so it inherits the fix; it now also writes FAILED to its result file rather than leaving it absent, so the SUMMARY distinguishes a failed network from a worker killed mid-deploy. The network is marked FAILED and the script exits 1.

Facet path check (as asked): diamondUpdateFacet propagates its return code correctly, and deployFacetAndAddToDiamond checks it — so the periphery-style masking does not exist there. It did have the related known bug that it sources diamondUpdatePeriphery.sh but not diamondUpdateFacet.sh, so the cut only worked when the caller happened to source script/tasks/*.sh (which deployContractToNetworks.sh does, and other callers do not). That source is now added.

Adjacent one-liner: deployAllContracts.sh stage 7 discarded the (previously meaningless) return code and printed STAGE 7 completed unconditionally. It now warns instead. Control flow is unchanged — that script has no set -e and the bring-up flow is interactive.

Tests: script/deploy/deployRegistrationPropagation.test.ts (9 cases) sources each real shell function with stubbed dependencies and asserts return-code propagation, plus the result-file contents and that diamondUpdateFacet.sh is sourced. All four fixed behaviours were confirmed to fail against origin/main's versions of these functions before the fix.

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: 48 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: 27233e73-7066-4cbb-a95e-e8b7a21a5543

📥 Commits

Reviewing files that changed from the base of the PR and between 358c2b9 and 65b30a3.

📒 Files selected for processing (6)

• script/deploy/deployAllContracts.sh
• script/deploy/deployContractToNetworks.sh
• script/deploy/deployFacetAndAddToDiamond.sh
• script/deploy/deployRegistrationPropagation.test.ts
• script/helperFunctions.sh
• script/tasks/diamondUpdatePeriphery.sh

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/nervous-kepler-b8cb76

---

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

[lifi-qa-agent]

🔍 QA Review — EXSC-713 (Post-Approval Re-Review — Run #29)

Ticket: EXSC-713 — Fail network deploy when diamond registration fails
PR: #2135 — claude/nervous-kepler-b8cb76 → main
Review type: 🔁 Post-approval re-review (Run #29)
Reviewer: lifi-qa-agent[bot] | Date: 2026-07-31

⚠️ New commit pushed after prior approval (2026-07-28T06:15:47Z) — analysing post-approval commit 65b30a35b942 only. The original approval covering all five fix points (sticky LAST_CALL, && logic, FAILED result file, diamondUpdateFacet.sh sourcing, stage-7 warning) remains valid.

---

Post-Approval Commit Reviewed

Commit	Date	Description
65b30a35b942	2026-07-30T20:19:54Z	test(deploy): cover sticky failure flag on the batch periphery path

---

Code Review

This commit adds exactly one test to script/deploy/deployRegistrationPropagation.test.ts:

it('stays failed when an earlier contract fails and a later one succeeds', () => {
  expect(runHarness(batchHarness)).toBe('rc=1')
})

The batchHarness drives diamondUpdatePeriphery in batch mode (UPDATE_ALL=true) with two contracts (OutputValidator and TokenWrapper), where register() returns non-zero for OutputValidator (first) and zero for TokenWrapper (second). The test asserts the function still returns rc=1 — confirming the sticky LAST_CALL flag prevents a later success from masking an earlier failure.

This is precisely the test gap identified in the original review (Run #22):

🟢 [Low] Sticky LAST_CALL in batch mode has no multi-contract loop test — the primary motivation for making LAST_CALL sticky is the UPDATE_ALL=true batch registration path where a later contract's success was previously masking an earlier contract's failure.

The harness correctly uses getIncludedPeripheryContractsArray() (supplying both contracts) and the register() stub uses the third positional argument ($3, the contract name) to discriminate — matching the real register() function's argument ordering. No production code was changed. ✅

---

🛡️ Audit State

No src/ Solidity contracts were modified. AuditNotRequired is correct. ✅

---

✅ Verdict: Pass (Post-Approval Re-Review)

Commit 65b30a35b942 addresses the sole Low-severity test gap from the original review. No production code was changed. The original five fix points remain in place and the prior approval is confirmed valid at the new HEAD.

QA re-review by lifi-qa-agent[bot] — Run #29 · 2026-07-31

[lifi-qa-agent]

QA Pass — all five fix points correctly implemented and verified. Two low-severity observations noted in the full QA comment (missing batch-path test for sticky LAST_CALL; deployAllContracts stage 7 failure not propagated to function exit code — both intentional by design). No security surface affected.

[0xDEnYO]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

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.

[lifi-qa-agent]

✅ QA approved (Run #29, post-approval re-review). Commit 65b30a3: adds the exact batch-path sticky-LAST_CALL test I flagged as a Low test gap. No production code changed. All five original fix points confirmed valid at new HEAD.