fix(deploy): correct LiFiIntentEscrowFacetV2 target state (EXSC-744) - #2181
Conversation
Version was pinned at 1.1.2 (leftover from the V1->V2 rename) on all production networks while src and every deployment are 1.0.0. Correct the version and scope the entry to the 12 networks where the facet is actually registered in the diamond; add the missing jovay entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
WalkthroughUpdated production deployment state for ChangesProduction deployment state
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
🔍 QA Review — EXSC-744 —
|
| # | Criterion | Status |
|---|---|---|
| AC-1 | Version corrected to 1.0.0 |
✅ Done — 12 entries updated from 1.1.2 → 1.0.0 |
| AC-2 | Removed from networks where facet is not deployed | ✅ Done — 69 removals; reduced from 68 to 12 networks |
| AC-3 | Intended network list confirmed before pruning | ✅ Done — PR body explicitly documents cross-check against .diamond.json deployment logs |
Code review
script/deploy/_targetState.json (+12 / −69)
Version correction (verified):
@custom:version 1.0.0 confirmed in src/Facets/LiFiIntentEscrowFacetV2.sol HEAD. The fix from 1.1.2 to 1.0.0 across all 12 retained networks is correct.
Network set (verified):
12 networks retained: arbitrum, arc, base, bsc, jovay, katana, mainnet, megaeth, optimism, pharos, polygon, robinhood. The 69 removals correctly remove entries from all networks where the facet is not registered. jovay was correctly added — it was deployed and registered in the diamond but absent from the old (already-wrong) 1.1.2 list, so adding it here is a net accuracy improvement.
Technique — line-level transform instead of JSON round-trip (correct):
The PR body explains why JSON parse+serialize was deliberately avoided: _targetState.json contains a pre-existing duplicate injective key that a round-trip would silently collapse. The line-level edit correctly avoids mutating unrelated content. ✅
Staging gap (noted, out of scope):
Developer notes that arbitrum and base staging diamonds have the facet cut in but no staging target-state block exists at all — a broader all-facets gap tracked separately. This is correct to flag but correct to leave for a separate issue. Not a blocker here.
AuditNotRequired label: Appropriate — this is a pure configuration JSON change with no contract code or ABI modifications. CI correctly applied this label.
Ticket coverage
| Category | Assessment |
|---|---|
| Version fix | ✅ Fully addressed |
| Network set correction | ✅ Fully addressed |
| Test coverage | N/A — config JSON; no unit tests required |
| Documentation/PR description | ✅ Thorough; lists all 12 networks, explains jovay addition, notes staging gap and duplicate-key concern |
Overall ticket coverage: Complete
Downstream impact
_targetState.json is read by drift-check tooling to determine "what should be deployed where." Before this fix, any drift check for LiFiIntentEscrowFacetV2 was meaningless: it would report false alarms on 56 networks (showing facet "missing" where it was never intended) and would not catch a real version drift on the 12 real networks. After this fix, drift checks for this facet are actionable again. No impact on the EXSC-736 rollout — as the developer notes, the deploy framework reads @custom:version from source, not from target state.
✅ Verdict: Pass
This is a precise, well-reasoned correction that fully addresses both ticket conditions. The implementation is minimal, non-destructive, and the approach (line-level transform, avoiding JSON round-trip) is correct given the pre-existing data quality issue in the file. The network list was cross-checked against deployment logs before pruning, satisfying the ticket's pre-condition for pruning.
🤖 Automated QA review by lifi-qa-agent[bot] — Zeus QA System
![lifi-qa-agent[bot]](https://avatars.githubusercontent.com/in/3411599?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA Pass — config correction is correct, version verified against @Custom:version 1.0.0, network list cross-checked against deployment logs. ✅
3 participants
Which Linear task belongs to this PR?
Fixes EXSC-744
Why did I implement it this way?
script/deploy/_targetState.jsonpinnedLiFiIntentEscrowFacetV2at1.1.2on every production network — a leftover from the V1→V2 rename (#2164) that carried over the oldLiFiIntentEscrowFacetversion, even thoughsrc/Facets/LiFiIntentEscrowFacetV2.soldeclares1.0.0and every deployment is1.0.0. Target state is meant to answer "what should be deployed where", so both the wrong version and the wrong network set (implying the facet runs everywhere) made drift checks meaningless for this facet. I corrected the version to1.0.0and scoped the entry to the exact set of networks where the facet is registered in the diamond, cross-checked against the.diamond.jsondeployment logs: arbitrum, arc, base, bsc, jovay, katana, mainnet, megaeth, optimism, pharos, polygon, robinhood (12). jovay was registered on-chain but missing from target state, so it was added; the entry was removed from the 58 production networks where the facet is not deployed. Staging was left untouched — arbitrum/base have the facet cut into their staging diamonds but nostagingtarget-state block exists at all, which is a broader all-facets gap tracked separately. The edit was done as a line-level transform rather than a JSON round-trip on purpose: the file contains a pre-existing duplicateinjectivekey that a parse/serialize cycle would silently collapse. No impact on the EXSC-736 rollout — the deploy framework reads the version from the source@custom:version, not from target state.Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)