-
Notifications
You must be signed in to change notification settings - Fork 0
docs(ops): fork-aware rewrite of fee-recipient rotation runbook (Codex #572 follow-up) #574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,27 +4,48 @@ How to change the Ophis protocol fee-recipient address if the Safe that collects | |
| fees is ever compromised (or for a planned treasury move). | ||
|
|
||
| The recipient stays a **hardcoded, auditable constant** by design. Rotation is a | ||
| reviewed code change plus a redeploy, not an env flip and not a contract upgrade. | ||
| See [Why the constant is hardcoded](#why-the-constant-is-hardcoded) for the | ||
| rationale. | ||
| reviewed code change plus a coordinated redeploy across the frontend, the SDK, | ||
| **the OP backend allowlist**, and the settlement-sweep / monitoring config, not | ||
| an env flip and not a contract upgrade. See | ||
| [Why the constant is hardcoded](#why-the-constant-is-hardcoded) for the rationale. | ||
|
|
||
| Canonical address today: `0x858f0F5eE954846D47155F5203c04aF1819eCeF8` | ||
| (2-of-3 Safe, CREATE2-deterministic, same address on every CoW chain). | ||
|
|
||
| ## The one fact that makes this simple | ||
|
|
||
| The recipient is **not** a contract parameter. CoW's Settlement contract is | ||
| immutable and pays whatever each order's signed `appData` says. The frontend | ||
| injects `partnerFee.recipient` into that appData when it builds an order. So: | ||
|
|
||
| - Rotation is a **config + redeploy** change. There is no Settlement/Vault | ||
| upgrade, no on-chain migration, no governance transaction. | ||
| - **Settled orders already paid the old Safe.** There is no clawback. If the old | ||
| Safe is compromised, moving its existing balance to safety is a separate Safe | ||
| transaction, handled by the signers, out of scope of this code rotation. | ||
| - **In-flight orders** signed before the redeploy still carry the old recipient | ||
| (they were signed against the old appData). Only orders signed **after** the | ||
| redeploy use the new recipient. | ||
| (Safe v1.4.1, 2-of-3, CREATE2-deterministic, same address on Optimism + Gnosis + | ||
| Ethereum). | ||
|
|
||
| > This is a fork-aware procedure. Read [The model](#the-model-two-enforcement-paths) | ||
| > first: the OP backend **rejects** orders whose partner-fee recipient is not in | ||
| > its allowlist, and OP fees sit in the Settlement contract until swept. A | ||
| > frontend-only change will break fee collection, not silently misroute it. | ||
|
|
||
| ## The model (two enforcement paths) | ||
|
|
||
| 1. **Recipient injection (frontend / SDK).** CoW's Settlement is immutable and | ||
| pays whatever each order's signed `appData` says. The frontend injects | ||
| `partnerFee.recipient` into appData at order-build time. So the *destination* | ||
| is a config value, not a contract parameter. | ||
| 2. **Recipient enforcement (OP backend).** The Ophis OP fork does **not** accept | ||
| an arbitrary recipient. `PARTNER_FEE_RECIPIENT_ALLOWLIST` | ||
| (`apps/backend/crates/app-data/src/app_data.rs`) is checked both at app-data | ||
| validation (`app_data.rs`, line ~335) and in the autopilot fee domain | ||
| (`apps/backend/crates/autopilot/src/domain/fee/mod.rs`, line ~253). An order | ||
| carrying a non-allowlisted recipient is **rejected / its partner-fee policy | ||
| dropped**, not paid to the new address. | ||
| 3. **Custody (OP sweep).** On OP, CIP-75 fees accumulate in the Settlement | ||
| contract (`0x310784c7…`) and are moved to the Safe only by a sweep | ||
| (`infra/optimism-mainnet/scripts/sweep-to-safe.sh` / | ||
| `contracts/script/SweepSettlementBuffer.s.sol`). "Settled" does not mean | ||
| "already in the Safe." | ||
|
|
||
| Consequences for rotation: | ||
|
|
||
| - You must add the new recipient to the **backend allowlist and redeploy the OP | ||
| backend before** (or together with) the frontend, or new swap.ophis.fi orders | ||
| get rejected. | ||
| - The replacement must be controllable at the **same address on every fee | ||
| chain** (the constant is one address used on all of them). | ||
| - **Sweep the Settlement buffer** as part of rotation; do not assume settled | ||
| fees are already in the old Safe. | ||
|
|
||
| ## When to rotate | ||
|
|
||
|
|
@@ -35,103 +56,142 @@ injects `partnerFee.recipient` into that appData when it builds an order. So: | |
|
|
||
| ## Pre-flight | ||
|
|
||
| 1. Stand up the **new** 2-of-3 Safe (or new EOA, though a Safe is the standard) | ||
| and record its address. | ||
| 2. Canonicalize it to EIP-55 mixed case before touching any file. Viem's strict | ||
| EIP-55 rejects a bad-case literal and crashes the frontend at init (the | ||
| 2026-05-17 incident): | ||
| 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. | ||
| 2. **EIP-55 checksum** the new address before touching any file (strict EIP-55 | ||
| crashes the frontend at init, the 2026-05-17 incident): | ||
|
|
||
| ```bash | ||
| cast to-check-sum-address 0x<new-address-lowercased> | ||
| ``` | ||
|
|
||
| Use that exact mixed-case string everywhere below. | ||
| 3. **Raw-byte form for Rust.** `PARTNER_FEE_RECIPIENT_ALLOWLIST` stores the | ||
| address as 20 raw bytes (`Address::new([0x85, 0x8f, ...])`), not a hex string. | ||
| Pre-compute the byte array: | ||
|
|
||
| ## Rotation surface (every place the address is hardcoded) | ||
| ```bash | ||
| python3 -c "a='<new-address-no-0x>'; print(', '.join('0x'+a[i:i+2] for i in range(0,40,2)))" | ||
| ``` | ||
|
|
||
| The address is declared in separate pnpm workspaces, so a single shared import is | ||
| not possible. A cross-workspace CI invariant | ||
| (`scripts/check-partner-fee-invariant.sh`) keeps three of them byte-identical and | ||
| fails any PR that drifts. **But two production references live outside that | ||
| invariant** (rebate-indexer + test fixtures), so always drive the rotation from a | ||
| repo-wide grep, not from the invariant's list alone. | ||
| ## Rotation surface | ||
|
|
||
| Discover the full set: | ||
| The address is declared across several pnpm/cargo workspaces and infra, so a | ||
| single shared import is impossible. A cross-workspace CI invariant | ||
| (`scripts/check-partner-fee-invariant.sh`) keeps three frontend/SDK files | ||
| byte-identical, **but most of the surface is outside that invariant.** Always | ||
| drive the rotation from a **repo-wide** search, not the invariant's list: | ||
|
|
||
| ```bash | ||
| grep -rln '0x858f0F5eE954846D47155F5203c04aF1819eCeF8' \ | ||
| --include='*.ts' --include='*.tsx' --include='*.sh' . \ | ||
| | grep -v node_modules | grep -vE '/(dist|build)/' | ||
| grep -rln '0x858f0F5eE954846D47155F5203c04aF1819eCeF8' . \ | ||
| | grep -v node_modules | grep -vE '/(dist|build|target)/' | ||
| ``` | ||
|
|
||
| Known references (update **all** of them): | ||
| Known production references (update **all**; the byte-array one is the backend): | ||
|
|
||
| **Recipient enforcement (OP backend), do this FIRST:** | ||
| - `apps/backend/crates/app-data/src/app_data.rs`: `PARTNER_FEE_RECIPIENT_ALLOWLIST` | ||
| (raw-byte `Address::new([...])`). | ||
|
|
||
| **Invariant-enforced source of truth (3 files):** | ||
| **Recipient injection (frontend / SDK), invariant-enforced trio + the check:** | ||
| - `apps/frontend/libs/common-const/src/feeRecipient.ts` | ||
| - `apps/frontend/apps/cowswap-frontend/src/ophis/partnerFeeDefault.ts` | ||
| - `packages/sdk/src/partner-fee.ts` | ||
|
|
||
| **The invariant's own canonical literal:** | ||
| - `scripts/check-partner-fee-invariant.sh` (the `CANONICAL=` line) | ||
|
|
||
| **Production references the invariant does NOT cover (do not forget):** | ||
| - `apps/rebate-indexer/src/safe/addresses.ts` (`OPHIS_SAFE_ADDRESS`) and any | ||
| `apps/rebate-indexer/src/cron.ts` use of it. The indexer attributes incoming | ||
| fees to this Safe; if it lags, rebate accounting points at the wrong wallet. | ||
|
|
||
| **Test fixtures** that assert the address (the grep above lists them, e.g. | ||
| `*.test.ts` under `packages/sdk`, `apps/frontend/libs/common-const`, | ||
| `apps/rebate-indexer`, `apps/mcp-server`). These must move too or CI fails. | ||
|
|
||
| ## Procedure | ||
|
|
||
| 1. Replace the old address with the new EIP-55 string in every file from the | ||
| grep above (production + the invariant literal + tests). Byte-exact, same | ||
| case in all of them. | ||
| 2. Run the invariant locally, expect exit 0: | ||
|
|
||
| ```bash | ||
| bash scripts/check-partner-fee-invariant.sh | ||
| ``` | ||
|
|
||
| 3. Typecheck + test the touched workspaces: | ||
| - `scripts/check-partner-fee-invariant.sh` (the `CANONICAL=` literal) | ||
|
|
||
| **Custody / monitoring (sweep + drift), or fees keep flowing to the old Safe:** | ||
| - `contracts/script/SweepSettlementBuffer.s.sol` (`DEFAULT_SAFE`) | ||
| - `infra/optimism-mainnet/scripts/sweep-to-safe.sh`, | ||
| `infra/optimism-mainnet/scripts/check-settlement-buffer.sh`, | ||
| `infra/optimism-mainnet/scripts/verify-e2e-swap.sh`, | ||
| `infra/optimism-mainnet/README.md` | ||
| - `infra/shared/cron/safe-drift-check.sh.tmpl` (the weekly Safe drift monitor; it | ||
| alerts on drift from this Safe, so it must track the new one) | ||
|
|
||
| **Rebate accounting:** | ||
| - `apps/rebate-indexer/src/safe/addresses.ts` (`OPHIS_SAFE_ADDRESS`), | ||
| `apps/rebate-indexer/src/cron.ts`, `apps/rebate-indexer/RUNBOOK.md` | ||
|
|
||
| **Deployed env (read at runtime, not just source):** | ||
| - Any live `.env` that sets `OPHIS_PARTNER_FEE_RECIPIENT` | ||
| (`infra/megaeth/.env.example` is the template; update the real `.env` on each | ||
| backend host). | ||
|
|
||
| **User-facing docs (accuracy):** | ||
| - `apps/docs-ophis/docs/audits.md`, `docs/operations/e2e-swap-verification.md`. | ||
|
|
||
| **Leave (dated historical records):** `docs/audits/2026-05-*`, | ||
| `docs/development/phase-*-validation.md`, `docs/development/plans/2026-05-*`, and | ||
| the already-applied migration comment in | ||
| `apps/rebate-indexer/migrations/0005_affiliate.sql` (do not rewrite an applied | ||
| migration; it documents the Safe at seed time). | ||
|
|
||
| ## Procedure (ordered to avoid rejected orders or misrouted fees) | ||
|
|
||
| 1. **Sweep first.** Run the OP settlement-buffer sweep so accrued fees land in | ||
| the **currently-controlled** Safe before anything changes | ||
| (`infra/optimism-mainnet/scripts/sweep-to-safe.sh`). If the old Safe is | ||
| compromised, sweep with `SAFE=<new-address>` instead so the buffer goes | ||
| straight to the replacement. | ||
| 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. | ||
|
Comment on lines
+140
to
+144
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 Useful? React with 👍 / 👎. |
||
| 3. **Update injection + custody + accounting + env + docs** to the new address | ||
| in every file from the grep above (frontend/SDK trio, invariant `CANONICAL`, | ||
| sweep config, drift monitor, rebate, live `.env`, docs). Byte-exact EIP-55 in | ||
| the TS/JSON/sh/sol files; the raw-byte form in Rust. | ||
| 4. **Local gates:** | ||
|
|
||
| ```bash | ||
| bash scripts/check-partner-fee-invariant.sh # exit 0 | ||
| cargo test -p app-data -p autopilot # backend allowlist tests | ||
| pnpm -C apps/rebate-indexer exec tsc --noEmit && pnpm -C apps/rebate-indexer exec vitest run | ||
| pnpm -C packages/sdk test | ||
| # frontend: rely on CI typecheck, or run the common-const + partnerFeeDefault tests | ||
| ``` | ||
|
|
||
| 4. Open a PR. Treat this as an external-API-config change: pre-merge Codex review | ||
| plus all security tools. The `Partner-fee cross-workspace invariant` gate runs | ||
| in CI and blocks merge on any drift. | ||
| 5. Merge. The **Deploy to Cloudflare Pages** workflow rebuilds swap.ophis.fi; | ||
| from that point new orders inject the new recipient into appData. | ||
| 6. **Republish `@ophis/sdk`** (bump the patch version) so external integrators | ||
| pick up the new recipient. See `docs/operations/` SDK publish notes / the | ||
| `npm-ophis-token` Keychain entry. Until they upgrade, integrators on the old | ||
| SDK still inject the old recipient. | ||
| 7. **Redeploy / restart the rebate-indexer** (`ophis-rebates-vm`) so | ||
| `OPHIS_SAFE_ADDRESS` reflects the new Safe and fee attribution follows. | ||
| 5. **PR** with pre-merge Codex + all security tools (treat as an external-API / | ||
| on-chain-config change). The `Partner-fee cross-workspace invariant` gate runs | ||
| in CI. | ||
| 6. **Merge, then deploy in order:** OP backend (if not already from step 2) → | ||
| frontend (swap.ophis.fi) → republish `@ophis/sdk` (bump patch; | ||
| `npm-ophis-token` Keychain) → redeploy/restart the rebate-indexer | ||
| (`ophis-rebates-vm`) → update the deployed sweep config + the cron drift | ||
| monitor. | ||
| 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. | ||
|
Comment on lines
+166
to
+168
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This drain condition only waits for orders signed before the frontend deploy, but the repo documents Useful? React with 👍 / 👎. |
||
| 8. **Secure the old Safe.** If it was compromised, its remaining balance is a | ||
| separate Safe transaction handled by the signers, out of scope of this code | ||
| rotation. | ||
|
|
||
| ## Verify | ||
|
|
||
| - Build a fresh order in the swap UI and inspect its appData: `partnerFee.recipient` | ||
| equals the new address (or decode the appData hash of a just-placed order). | ||
| - The `Partner-fee cross-workspace invariant` check is green on the merged PR. | ||
| - `grep -rn '<old-address>' --include='*.ts' .` returns only intentional | ||
| historical/archival references (if any), no live production code. | ||
| - The rebate-indexer's `OPHIS_SAFE_ADDRESS` is the new Safe and a post-rotation | ||
| fee settles to it. | ||
| - A fresh swap.ophis.fi order is **accepted** (not rejected) and its appData | ||
| `partnerFee.recipient` equals the new address. (A rejected order means the | ||
| backend allowlist step was missed.) | ||
| - `bash scripts/check-partner-fee-invariant.sh` is green; the CI invariant passed. | ||
| - `grep -rn '<old-address>' . | grep -vE '/(node_modules|dist|build|target)/'` | ||
| returns only the intentional dated-historical references. | ||
| - The sweep targets the new Safe, the drift monitor watches the new Safe, and the | ||
| rebate-indexer's `OPHIS_SAFE_ADDRESS` is the new Safe. | ||
| - A post-rotation fee sweeps to the new Safe on every fee chain. | ||
|
|
||
| ## Why the constant is hardcoded | ||
|
|
||
| Making the recipient env-configurable would let whoever controls the deploy | ||
| environment redirect every protocol fee with no code change and no review. The | ||
| hardcoded constant plus the cross-workspace CI invariant means: | ||
| hardcoded constant, the cross-workspace CI invariant, and the on-chain backend | ||
| allowlist together mean: | ||
|
|
||
| - The fee destination is **auditable** from source at any commit. | ||
| - It **cannot drift silently** (the invariant fails the PR). | ||
| - It **cannot drift silently** (the invariant fails the PR; the backend rejects | ||
| an unlisted recipient). | ||
| - A rotation is always an **on-the-record, reviewed change**, never a hidden env | ||
| edit. | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.tsandpackages/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 👍 / 👎.