feat(contracts): v0.2.0 zeus upgrade — EigenKMSRegistrar platformRpcUrl

[seanmcgary]

Summary

Adds a Zeus upgrade release that deploys the EigenKMSRegistrar functionality built in #120 onto the live sepolia-dev proxy: the new impl adds AvsConfig.platformRpcUrl + the AvsConfigSet event so KMS operators can discover the ecloud-platform gRPC endpoint on-chain.

What's in the release

contracts/script/releases/v0.2.0-sepolia-platform-rpc-url/:

• upgrade.json — single EOA phase, 0.1.0 → 0.2.0.
• 1-upgradeRegistrar.s.sol — an EOADeployer script that:
  1. deploys the new EigenKMSRegistrar impl (constructor immutables wired from Env: AllocationManager / KeyRegistrar / PermissionController),
  2. repoints the proxy via the EOA-owned ProxyAdmin.upgrade(...),
  3. calls setAvsConfig to populate platformRpcUrl while preserving the existing operatorSetId (read from the live getAvsConfig()), then records the new impl in Zeus state (outside the broadcast segment, per the zeus-templates convention).

Env.sol — adds a platformRpcUrl() accessor (+ _envString helper) so the URL is injected via the Zeus platformRpcUrl env var rather than hardcoded.

Design notes

• EOA upgrade (not multisig): deployment.json shows the ProxyAdmin owner and the registrar's Ownable owner are both the operatorOwner EOA, matching the init phase's type: "eoa".
• Storage-safe: feat: KMS ↔ ecloud-platform integration — on-chain URL discovery, signed release-fetch client, stack_id /secrets auth #120 appended platformRpcUrl to the AvsConfig struct, so the upgrade doesn't shift existing storage.
• sepolia-dev only: base-sepolia's EigenKMSCommitmentRegistry was not changed by feat: KMS ↔ ecloud-platform integration — on-chain URL discovery, signed release-fetch client, stack_id /secrets auth #120, so no upgrade is needed there.
• Verified the vendored OZ ProxyAdmin exposes the v4-style upgrade(proxy, impl).

Testing

• Release compiles cleanly (forge build on the release path; releases are excluded from the normal build via no_match_path).
• Normal forge build / forge test suite unaffected (36/36 passing).
• The release's testScript() and the real dry-run execute through the Zeus CLI against the sepolia-dev env (same convention as the existing init scripts).

Before running the real deployment

Set the Zeus platformRpcUrl env var to the actual ecloud-platform gRPC endpoint (host:port) for sepolia-dev.

[claude]

Claude finished @seanmcgary's task in 2m 19s —— View job

---

Code Review — Round 4

• Gather context
• Read changed files
• Review Env.sol changes
• Review 1-upgradeRegistrar.s.sol
• Review upgrade.json
• Post comprehensive review

---

This is a clean, well-structured upgrade script that has been improved through three rounds of review. Most concerns from prior rounds have been addressed properly. Below are my remaining observations.

---

contracts/script/releases/Env.sol

No issues. The platformRpcUrl() accessor and _envString helper are clean additions that follow the existing _envAddress/_envU256 pattern exactly. The comment on line 61–65 is appropriately scoped.

---

contracts/script/releases/v0.2.0-sepolia-platform-rpc-url/upgrade.json

No issues. Consistent with the v0.1.0 init manifest format. from/to versions and phase type are correct.

---

contracts/script/releases/v0.2.0-sepolia-platform-rpc-url/1-upgradeRegistrar.s.sol

[Positive] ABI-safe pre-upgrade read via IEigenKMSRegistrarV1

The IEigenKMSRegistrarV1 interface (lines 38–44) with its AvsConfigV1 {uint32 operatorSetId} is the correct solution to the ABI incompatibility surfaced in round 3. The analysis in the class-level @dev comment is accurate:

• v0.1.0 AvsConfig = {uint32} → static tuple ABI → return data is a single 32-byte padded word.
• v0.2.0 AvsConfig = {uint32, string} → dynamic tuple ABI → return data starts with an offset pointer.

Decoding the old static return with the new dynamic signature would interpret the uint32 value as a tuple offset and then attempt to read tuple data at that byte position — likely an out-of-bounds revert or corrupted decode. The old-shaped interface correctly decodes as (uint32) → reads the single word directly. This is well-handled and well-documented.

[Positive] Guard ordering

require(bytes(Env.platformRpcUrl()).length > 0, ...) at line 55 is placed before vm.startBroadcast(), which is correct: the deployment fails fast at dry-run without emitting any transactions. The two reads of Env.platformRpcUrl() (guard + setAvsConfig) are idempotent env reads — no concern there.

[Positive] Non-atomic window documentation

The @dev NatSpec at lines 26–32 clearly states the upgrade/setAvsConfig two-tx window and the correct mainnet mitigation path (ProxyAdmin.upgradeAndCall or same-block sequencing). This is the right disposition for a sepolia-dev release.

[Positive] testScript() invariant coverage

testScript() validates:

1. Pre-upgrade operatorSetId != 0 sanity (line 98)
2. Proxy impl pointer updated (lines 103–107)
3. platformRpcUrl set to the injected value (line 111)
4. operatorSetId preserved through upgrade (line 112)

This covers the core correctness invariant of the release.

---

[Nit] assertTrue(preOperatorSetId != 0) relies on deployment convention

Line 98: assertTrue(preOperatorSetId != 0, "sanity: proxy not initialised") uses != 0 as a proxy for the contract being initialized. This is a reasonable heuristic for sepolia-dev (operator set IDs are non-zero by convention), but would fail falsely if operatorSetId was ever legitimately initialized to 0. The existing comment ("sanity: proxy not initialised") captures the intent, but it might be worth noting the assumption explicitly in the comment (e.g. "assumes sepolia-dev init set a non-zero operatorSetId"). Minor — current state is acceptable.

[Nit] testScript() does not assert pre/post impl address differs

Lines 103–107 assert the proxy's implementation equals Env.impl.eigenKMSRegistrar() after the upgrade, which is correct. However, it doesn't capture the pre-upgrade impl address and assert it changed — a no-op upgrade (e.g., if Env.impl.eigenKMSRegistrar() returned the old impl) would still pass the post-state assertion if the Zeus state recorded the same address. This is a minor defensive gap. The existing init scripts don't capture pre-state either, so this is consistent with the pattern. Fix this →

[Observation] ITransparentUpgradeableProxy transitive resolution

ITransparentUpgradeableProxy is used in lines 71 and 104 but not directly imported — it arrives transitively via import "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol". The script compiles cleanly (per PR description), confirming the transitive resolution works in Forge. Worth noting in case a future OZ upgrade changes import structure, but this is idiomatic for the existing codebase pattern.

---

Overall

The release script is correct and well-documented. The three prior review rounds have addressed all substantive concerns. The two nits above are cosmetic and can be addressed at discretion — neither blocks the deployment. The core logic (ABI-safe pre-upgrade read, non-atomic window documentation, fail-fast env var guard, and operatorSetId non-clobber invariant) is sound.

LGTM for the sepolia-dev deployment scope described in the PR.

[seanmcgary]

Round 1 review responses (commit c3a2834):

• [Medium] testScript() doesn't assert operatorSetId preservation — Fixed. testScript() now captures preOperatorSetId from the live proxy before runAsEOA(), and asserts cfg.operatorSetId == preOperatorSetId after — directly validating the PR's core non-clobber invariant (alongside the existing platformRpcUrl + proxy-impl assertions).
• [Low] No empty-string guard on platformRpcUrl — Fixed. Added require(bytes(Env.platformRpcUrl()).length > 0, "platformRpcUrl env var not set") at the top of _runAsEOA() (before vm.startBroadcast()), so an unset/empty env var fails fast at dry-run instead of silently deploying a broken on-chain config.
• [Nit] Unused TransparentUpgradeableProxy concrete import — Fixed. Removed; only ITransparentUpgradeableProxy is used, and it's transitively available via the ProxyAdmin.sol import (whose own signatures reference it). Recompiled clean.
• [Nit] _envString doc — no change needed (consistent with the existing undocumented _envAddress/_envU256 helpers), as you noted.

Release recompiles cleanly (forge build on the release path); normal build/test suite unaffected.

[seanmcgary]

Round 2 review responses (commit follows):

• [Low] testScript() doesn't assert preOperatorSetId != 0 — Fixed. Added assertTrue(preOperatorSetId != 0, "sanity: proxy not initialised") right after the capture, making the test's precondition self-documenting.
• [Nit] PR reference in NatSpec — Fixed. Reworded the @notice to describe the functionality directly ("Upgrades the EigenKMSRegistrar implementation to add AvsConfig.platformRpcUrl + the AvsConfigSet event…") instead of referencing PR feat: KMS ↔ ecloud-platform integration — on-chain URL discovery, signed release-fetch client, stack_id /secrets auth #120, so the release artifact's doc doesn't rot as history moves.
• [Observation] view call inside broadcast — acknowledged; getAvsConfig() is a view read, emits no tx, intentional.
• [Observation] no host:port format validation — acknowledged; platformRpcUrl is an operator-controlled Zeus env var (internal trust boundary). The empty-string guard covers the silent-misconfig case; strict format validation is left to the operator runbook.

Recompiles clean; normal build/test suite unaffected.

[seanmcgary]

Round 3 review responses (commit follows):

• [Low] Move getAvsConfig() read before vm.startBroadcast() — Pushed back (would introduce a bug), but I fixed a related latent issue it surfaced. Reading the config before the upgrade is not ABI-safe: the deployed v0.1.0 impl's getAvsConfig() returns a static (uint32) tuple, whereas the new signature returns a dynamic tuple (it added string platformRpcUrl), ABI-encoded as an offset pointer. Decoding the old static return with the new signature reads operatorSetId as the tuple offset → out-of-bounds → revert/garbage. So the _runAsEOA() config read must stay after upgrade() (it already is — unchanged).
  • This did surface a real bug in my round-2 testScript(): it captured preOperatorSetId via the new signature before runAsEOA(), which in a fork test hits the same mis-decode. Fixed by reading the pre-upgrade value through a minimal old-shaped IEigenKMSRegistrarV1.getAvsConfig() interface (static (uint32)), which decodes the old impl correctly. Added a NatSpec note explaining the static-vs-dynamic ABI hazard. Good catch prompting this.
• [Medium] Non-atomic upgrade window (upgrade tx, then setAvsConfig tx) — Documented. Acknowledged: between the two txs getAvsConfig().platformRpcUrl reads empty. Acceptable for sepolia-dev (no live operators). Added a NatSpec note: for mainnet, land both txs in one block, or use ProxyAdmin.upgradeAndCall once the registrar exposes a suitable initializer (the current contract has none, so the atomic path needs contract scaffolding — out of scope for this sepolia-dev release).
• [Low] testScript pre-condition on empty platformRpcUrl — covered. The require(bytes(...).length > 0, ...) in _runAsEOA() (added round 1) already fails fast with a clear message; testScript exercises it via runAsEOA().
• [Nit] upgrade.json description field — no change (consistent with the v0.1.0 init manifests, which also omit it).

Recompiles clean. This is the 3rd review round (default cap); remaining items are the acknowledged non-atomic-window observation (documented) and nits.