chore(cc30): op-mainnet V5 deploy runbook + G10 config dedup + CC-14 version convention#351
Conversation
…version convention
CC-30 SP-independent prep (no external deps):
- G1 prep: docs/deployment/op-mainnet-v5-runbook.md — ties the EXISTING
`./deploy-core op-mainnet` path (DeployLive is v5.4-aware) to the CC-30
gates: prerequisites (audit/keystore/DVT mainnet nodes), deploy sequence,
EOA→Safe ownership transfer, config record, and the @-notify steps. No new
deploy code (path already exists) — mainnet = config only.
- G10: OP mainnet had two chain-10 configs. Marked config.op-mainnet.json as
AUTHORITATIVE (V5.4.2 target) and config.optimism.json as LEGACY V3 record
(do-not-read for V5 canonical). Resolves SDK's G10.
- CC-14: version-naming convention in CONTRIBUTING.md — human-readable text
carries the product name ("SuperPaymaster v5.4.2"), version strings stay
clean semver.
Claude-Session: https://claude.ai/code/session_01MxPwC3rr4mLTSb3cJtsYMm
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Runbook had non-executable commands:
- env var was OP-MAINNET_RPC_URL (hyphen — invalid shell identifier);
deploy-core reads ${ENV_UPPER//-/_}_RPC_URL = OP_MAINNET_RPC_URL.
- first OP-mainnet V5 deploy needs `--fresh-deploy` (new UUPS proxies; V5 is
not an in-place upgrade of legacy V3), not `--force`.
- version-check-onchain.sh takes an RPC_URL arg, not auto-detect.
Added an env-load pre-flight and clarified the transferOwnership step is
per-Ownable-contract (slashPolicyAdmin uses its own setter).
Claude-Session: https://claude.ai/code/session_01MxPwC3rr4mLTSb3cJtsYMm
…executable) Codex stop-review: OP-mainnet version verification was not executable. CheckVersions.s.sol had no case for block.chainid == 10, so it fell through to network = "10" → deployments/config.10.json (does not exist) → vm.readFile reverts. version-check-onchain.sh could never verify OP mainnet. Map chainid 10 → "op-mainnet" so it reads the authoritative V5 config (config.op-mainnet.json, per CC-30 G10; NOT the legacy V3 config.optimism.json). Now `./version-check-onchain.sh "$OP_MAINNET_RPC_URL"` (runbook step 2) resolves an existing config path. Claude-Session: https://claude.ai/code/session_01MxPwC3rr4mLTSb3cJtsYMm
clestons
left a comment
clestons
left a comment
There was a problem hiding this comment.
#351 — REQUEST CHANGES
runbook + CheckVersions.s.sol 修正正确；一条 Low 需修。
[Confirmed by PK] Low — config.optimism.json 删掉了 aPNTs 真实历史地址
PR 明确声明 config.optimism.json 是「LEGACY V3 historical on-chain record」并「KEPT as the historical on-chain record」，但 diff 在添加 _note 字段的同时，把 "aPNTs":"0x0B41C78081B5A141eb4C3C7E7FD8E58A7Bde553B" 一并删掉了。Codex 在 deployments/verify.optimism.contracts-2-11.md 找到该地址确为链上已 verify 的 aPNTs 合约 — 删掉它与「保留历史记录」的意图直接矛盾。
建议将 aPNTs 恢复到 config.optimism.json，或在 _note 中显式说明为何移除（如「该地址属于 aPNTs 旧版本，V5 重部署后由新地址覆盖，故标注而不保留」）。
其余变更无 finding：
CheckVersions.s.sol:10—block.chainid == 10 → "op-mainnet"正确；修复了旧版 fallthrough 到config.10.json（不存在）导致版本验证不可执行的问题 ✓- runbook commit-2 修正（
OP-MAINNET_RPC_URL→OP_MAINNET_RPC_URL、--force→--fresh-deploy、version-check-onchain.sh加 RPC 参数）均正确 ✓ - G10 双 config 权威/历史定义清晰 ✓
PK Summary | 1 CONFIRM / 1 CHALLENGE(rejected)
Reviewed by PR-Daemon (Claude Sonnet + Codex PK — 1 round).
Adding the _note accidentally dropped "aPNTs":"0x0B41...553B" — a real on-chain verified contract — contradicting the file's "KEPT as historical on-chain record" claim. Restored; the diff vs main is now purely additive (only _note prepended, all 18 address keys intact). Claude-Session: https://claude.ai/code/session_01MxPwC3rr4mLTSb3cJtsYMm
|
Fixed the Low: restored |
clestons
left a comment
clestons
left a comment
There was a problem hiding this comment.
#351 — APPROVE (re-review)
Previous finding resolved: aPNTs: "0x0B41C78081B5A141eb4C3C7E7FD8E58A7Bde553B" has been restored to config.optimism.json.
The incremental commit (8b86d8dc) is a single-field JSON restoration — no code, no logic change. Address was independently verified against deployments/verify.optimism.contracts-2-11.md in the first review round.
All other findings from the original review remain non-blocking (runbook fixes, CheckVersions.s.sol op-mainnet chain ID, G10 dual-config structure) — all correct.
Reviewed by PR-Daemon (Claude Sonnet — incremental re-review, PK skipped: single JSON field restore of previously-verified address).
2 participants
CC-30 SP-independent prep (no external deps) — the three items jason greenlit.
G1 prep —
docs/deployment/op-mainnet-v5-runbook.md: ties the existing./deploy-core op-mainnetpath (DeployLive is v5.4-aware) to the CC-30 gates — prerequisites (audit / keystore / DVT mainnet nodes), deploy sequence, EOA→Safe ownership transfer, config record,@-notify steps. No new deploy code (path already exists; mainnet = config only).G10 — OP mainnet had two chain-10 configs.
config.op-mainnet.jsonmarked AUTHORITATIVE (V5.4.2 target);config.optimism.jsonmarked LEGACY V3 record (do-not-read for V5 canonical). Resolves SDK's G10.CC-14 — version-naming convention in
CONTRIBUTING.md: human-readable text carries the product name (SuperPaymaster v5.4.2); version strings stay clean semver.All config JSON validated. No contract/runtime change.
https://claude.ai/code/session_01MxPwC3rr4mLTSb3cJtsYMm