docs(ops): fork-aware rewrite of fee-recipient rotation runbook (Codex #572 follow-up)#574
Conversation
Post-merge Codex review of #572 found the first draft missed the OP fork's server-side enforcement and custody model. Rewritten to cover: - P1: the OP backend ALLOWLIST. PARTNER_FEE_RECIPIENT_ALLOWLIST (apps/backend/crates/app-data/src/app_data.rs, enforced there + in autopilot fee/mod.rs) REJECTS any non-allowlisted recipient. Rotating only the FE/SDK makes the backend reject every new order. New ordered step: add the new recipient to the allowlist (raw-byte Address form) and redeploy the OP backend BEFORE/with the frontend; keep the old entry until in-flight orders drain, then remove it. - P2 sweep: OP fees accrue in the Settlement contract until swept, so "already in the old Safe" was wrong. Added a sweep-first step and the sweep config files (SweepSettlementBuffer.s.sol DEFAULT_SAFE, sweep/check scripts). - P2 cross-chain: the constant is one address on every chain; pre-flight now requires the replacement be controllable at the SAME address on OP+Gnosis+ETH (CREATE2 plan or EOA), not a single-chain Safe. - P2 grep: broadened discovery to a repo-wide search and enumerated the full surface (backend allowlist, sweep contract, cron drift monitor, infra scripts, live .env, rebate, docs); dated historical records left intact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccdf0e798f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 7. **Drain the transition.** Once orders signed before the FE redeploy have all | ||
| settled, **remove the old recipient** from `PARTNER_FEE_RECIPIENT_ALLOWLIST` | ||
| and redeploy the OP backend, so only the new Safe is accepted. |
There was a problem hiding this comment.
Keep old recipient allowlisted until SDK clients migrate
This drain condition only waits for orders signed before the frontend deploy, but the repo documents @ophis/sdk as a public npm package for programmatic order submission and tells integrators to use buildOphisAppDataPartnerFee (apps/docs-ophis/docs/ai-agents.md:207 and :250). Any client that has not upgraded after the republish can continue creating OP orders with the old recipient indefinitely, so removing it from PARTNER_FEE_RECIPIENT_ALLOWLIST at this point would make those external submissions get rejected or have their fee policy dropped. The runbook should require an SDK/integrator migration window or explicit cutoff before removing the old recipient.
Useful? React with 👍 / 👎.
| 1. **Same address on every fee chain.** The app/SDK/backend use one recipient on | ||
| all chains. Stand up the replacement so the **same address is controllable on | ||
| Optimism + Gnosis + Ethereum** (and any other fee chain): either a | ||
| CREATE2-deterministic Safe with a deployment plan proving the address resolves | ||
| and is owned on each chain, or an EOA you control everywhere. Do **not** reuse | ||
| a single-chain Safe address globally, fees on the other chains would go to an | ||
| address with no Safe deployed. |
There was a problem hiding this comment.
Enumerate every emitted fee chain before approving the Safe
The pre-flight checklist names Optimism/Gnosis/Ethereum but does not identify the current "other fee chains" that must be verified. The frontend gate and SDK currently emit partner fees on all CoW-supported chains plus 10/4326/999 (for example Base, Arbitrum, Polygon, BNB, Avalanche, Linea, etc.; see shouldEmitOphisPartnerFee.test.ts and packages/sdk/src/partner-fee.ts), so following this as written can approve a replacement Safe only on the three named chains while orders on the remaining fee chains still route to the same address without verified deployment/ownership there. The runbook should derive or enumerate the full current fee-chain set before the replacement is accepted.
Useful? React with 👍 / 👎.
| 2. **Allowlist the new recipient + redeploy the OP backend.** Add the new | ||
| address to `PARTNER_FEE_RECIPIENT_ALLOWLIST` (raw bytes). Keep the old entry | ||
| for now so in-flight orders are not rejected mid-rotation. Redeploy the OP | ||
| backend (app-data validation + autopilot). New orders carrying the new | ||
| recipient are now accepted. |
There was a problem hiding this comment.
Don't redeploy the allowlist before PR review
In planned rotations this step tells operators to add the new recipient to the production OP backend and redeploy before the later PR/security-review step, even though the runbook's trust model depends on recipient changes being reviewed code changes. Because PARTNER_FEE_RECIPIENT_ALLOWLIST is the backend enforcement boundary, deploying it before CI/review broadens the set of addresses that can receive accepted partner-fee orders without the auditability this procedure is meant to preserve. The allowlist code change should be reviewed and merged first, then deployed before the frontend/SDK start emitting the new recipient.
Useful? React with 👍 / 👎.
Post-merge Codex review of #572 found the first runbook draft missed how the OP fork actually enforces and custodies the partner fee. A flawed rotation runbook is a real incident risk, so this rewrites it to be fork-accurate. All four findings verified against the code before rewriting.
Findings addressed
P1 — OP backend allowlist.
PARTNER_FEE_RECIPIENT_ALLOWLIST(apps/backend/crates/app-data/src/app_data.rs, enforced there at app-data validation and inautopilot/.../fee/mod.rs) rejects any non-allowlisted recipient. Rotating only the FE/SDK would make the OP backend reject every new order. New ordered step: add the new recipient to the allowlist (it's a raw-byteAddress::new([...]), not a hex string) and redeploy the OP backend before/with the frontend; keep the old entry until in-flight orders drain, then remove it.P2 — sweep unswept settlement buffers. OP CIP-75 fees accrue in the Settlement contract until swept (
sweep-to-safe.sh/SweepSettlementBuffer.s.solDEFAULT_SAFE), so "settled = already in the old Safe" was wrong. Added a sweep-first step and the sweep/monitor files to the surface.P2 — cross-chain control. The constant is one address used on every chain (the current Safe is CREATE2-deterministic across OP+Gnosis+ETH). Pre-flight now requires the replacement be controllable at the same address on every fee chain (CREATE2 plan or EOA), not a single-chain Safe.
P2 — broaden the grep. Discovery is now a repo-wide search; the surface enumerates the backend allowlist, sweep contract, cron drift monitor, infra scripts, live
.env, rebate accounting, and docs. Dated historical records (audits/validation/plans, the applied migration comment) are explicitly left intact.Also added the EIP-55 + raw-byte pre-compute steps and a verify step that checks a fresh order is accepted (catches a missed allowlist update).
Docs-only, internal
docs/operations/runbook (not served). 0 em-dash.