Governance rotation fans SetLpAdmin to every pair in one unbatched message

Brouie commented

2026-06-03 07:11:55 +00:00

(Migrated from gitlab.com)

Severity: Informational
Reachability: Governance-only, and only on an actual governance change — not attacker-reachable. Latent scaling cliff.
Affected: execute_update_config SetLpAdmin fanout (smartcontracts/contracts/factory/src/contract.rs).

Summary

When governance rotates, execute_update_config loops for idx in 0..pair_count and pushes one SetLpAdmin message per pair into a single response. With enough pairs that one message batch can exceed gas / message-size limits, and the rotation tx fails — meaning governance can't actually be rotated once the DEX has many pairs.

Gated by ensure_governance and only fires when new_governance != old_governance, so there's no attacker angle. But it's an unbounded fanout on a critical, rare operation, so it should be paginated before pair count grows.

Current codebase

execute_update_config: if new_governance != old_governance { for idx in 0..pair_count { messages.push(SetLpAdmin{...}) } } — no batching, no continuation.

Recommended direction

Paginate the rotation: process pairs in bounded batches with a stored cursor and a follow-up message, or a "continue rotation" entrypoint.
Or store governance centrally and have pairs read it on demand, so rotation is O(1).

Acceptance criteria

Governance rotation succeeds regardless of pair count.
No single tx builds an unbounded message vector over all pairs.

**Severity:** Informational **Reachability:** Governance-only, and only on an actual governance change — not attacker-reachable. Latent scaling cliff. **Affected:** `execute_update_config` SetLpAdmin fanout (`smartcontracts/contracts/factory/src/contract.rs`). ## Summary When governance rotates, `execute_update_config` loops `for idx in 0..pair_count` and pushes one `SetLpAdmin` message per pair into a single response. With enough pairs that one message batch can exceed gas / message-size limits, and the rotation tx fails — meaning governance can't actually be rotated once the DEX has many pairs. Gated by `ensure_governance` and only fires when `new_governance != old_governance`, so there's no attacker angle. But it's an unbounded fanout on a critical, rare operation, so it should be paginated before pair count grows. ## Current codebase - `execute_update_config`: `if new_governance != old_governance { for idx in 0..pair_count { messages.push(SetLpAdmin{...}) } }` — no batching, no continuation. ## Recommended direction 1. Paginate the rotation: process pairs in bounded batches with a stored cursor and a follow-up message, or a "continue rotation" entrypoint. 2. Or store governance centrally and have pairs read it on demand, so rotation is O(1). ## Acceptance criteria - [ ] Governance rotation succeeds regardless of pair count. - [ ] No single tx builds an unbounded message vector over all pairs.

PlasticDigits commented

2026-06-03 10:37:09 +00:00

(Migrated from gitlab.com)

Governance should be stored centrally and read on demand.

Brouie commented

2026-06-04 06:28:52 +00:00

(Migrated from gitlab.com)

Implementation plan — and a real caveat on the "read on demand" framing.

Reading the code: SetLpAdmin does NOT write a pair-stored lp_admin value you can read on demand — it emits WasmMsg::UpdateAdmin, which rewrites each LP token's chain-level x/wasm migration admin. So the only way to make rotation literally O(1) (your intent) is:

PRIMARY (O(1)): at LP-token instantiate set admin: Some(<factory>) (pair/contract.rs ~523) instead of the pair, and delete the per-pair rotation fanout entirely. The factory's CONFIG.governance becomes the single source of truth; rotation touches no pairs. Caveat: this only takes effect for LP tokens instantiated AFTER redeploy — existing pairs need a one-time update-admin (pair→factory) op (runbook).

FALLBACK (lower-risk, admin stays literally == governance): #242-style SetLpAdminAll (capped via calc_limit) + SetLpAdminBatch (cursor) — the exact template is execute_set_discount_registry_all/_batch (factory/contract.rs:386-487) + DiscountRegistryAllTooManyPairs error. Operators loop until has_more=false. Keeps it a fanout but bounded + resumable.

Files: factory/contract.rs (delete fanout for PRIMARY, or add batch arms for FALLBACK), pair/contract.rs (LP instantiate admin, PRIMARY), dex-common/factory.rs + error.rs (FALLBACK variants). Test gap that let this ship: test_update_config creates ZERO pairs, so the rotation fanout is completely untested — add a many-pairs rotation test either way. Your call on PRIMARY vs FALLBACK; PRIMARY matches your O(1) intent, FALLBACK is the proven in-repo pattern. @PlasticDigits

Implementation plan — and a real caveat on the "read on demand" framing. Reading the code: `SetLpAdmin` does NOT write a pair-stored `lp_admin` value you can read on demand — it emits `WasmMsg::UpdateAdmin`, which rewrites each LP token's **chain-level x/wasm migration admin**. So the only way to make rotation literally O(1) (your intent) is: **PRIMARY (O(1)):** at LP-token instantiate set `admin: Some(<factory>)` (pair/contract.rs ~523) instead of the pair, and delete the per-pair rotation fanout entirely. The factory's `CONFIG.governance` becomes the single source of truth; rotation touches no pairs. Caveat: this only takes effect for LP tokens instantiated AFTER redeploy — existing pairs need a one-time `update-admin` (pair→factory) op (runbook). **FALLBACK (lower-risk, admin stays literally == governance):** #242-style `SetLpAdminAll` (capped via calc_limit) + `SetLpAdminBatch` (cursor) — the exact template is `execute_set_discount_registry_all`/`_batch` (factory/contract.rs:386-487) + `DiscountRegistryAllTooManyPairs` error. Operators loop until `has_more=false`. Keeps it a fanout but bounded + resumable. Files: factory/contract.rs (delete fanout for PRIMARY, or add batch arms for FALLBACK), pair/contract.rs (LP instantiate admin, PRIMARY), dex-common/factory.rs + error.rs (FALLBACK variants). **Test gap that let this ship:** `test_update_config` creates ZERO pairs, so the rotation fanout is completely untested — add a many-pairs rotation test either way. Your call on PRIMARY vs FALLBACK; PRIMARY matches your O(1) intent, FALLBACK is the proven in-repo pattern. @PlasticDigits

Brouie commented

2026-06-05 03:23:16 +00:00

(Migrated from gitlab.com)

Re-confirmed the root cause for the record: execute_update_config fans a SetLpAdmin message to every pair in one tx when governance changes (contract.rs ~631), unbounded — that's the issue. The fix is the PRIMARY-vs-FALLBACK call from my earlier note: PRIMARY = set the LP token's admin to the factory at instantiate and delete the rotation fanout entirely (only affects pairs created after redeploy; existing need a one-time update-admin); FALLBACK = #242-style bounded SetLpAdminAll + SetLpAdminBatch cursor. Both change rotation behavior, so I didn't want to pick for you. Say which and I'll ship it — the FALLBACK is the exact same shape as the SetDiscountRegistryAll/Batch already in the factory. @PlasticDigits

Brouie commented

2026-06-05 03:34:53 +00:00

(Migrated from gitlab.com)

Went ahead and implemented the FALLBACK — MR above. Removed the unbounded SetLpAdmin fanout from execute_update_config and added bounded SetLpAdminAll (one tx, fails over the pagination cap) + SetLpAdminBatch (cursor, rerun until has_more=false), the exact shape of the SetDiscountRegistryAll/Batch already in the factory.

The tradeoff to sign off on: governance rotation no longer auto-rotates the LP-token admins — the operator runs SetLpAdminAll/Batch after the UpdateConfig, same as the discount registry works. So between the rotation and the batch, LP tokens keep their old admin; that goes in the rotation runbook. Rewrote the rotation test accordingly; suite 415/0.

If you'd rather go PRIMARY (set LP admin = factory at instantiate and drop the per-pair rotation entirely — existing pairs need a one-time update-admin), say so and I'll swap this for it. @PlasticDigits

Went ahead and implemented the FALLBACK — MR above. Removed the unbounded SetLpAdmin fanout from execute_update_config and added bounded SetLpAdminAll (one tx, fails over the pagination cap) + SetLpAdminBatch (cursor, rerun until has_more=false), the exact shape of the SetDiscountRegistryAll/Batch already in the factory. The tradeoff to sign off on: governance rotation no longer auto-rotates the LP-token admins — the operator runs SetLpAdminAll/Batch after the UpdateConfig, same as the discount registry works. So between the rotation and the batch, LP tokens keep their old admin; that goes in the rotation runbook. Rewrote the rotation test accordingly; suite 415/0. If you'd rather go PRIMARY (set LP admin = factory at instantiate and drop the per-pair rotation entirely — existing pairs need a one-time update-admin), say so and I'll swap this for it. @PlasticDigits

Brouie commented

2026-06-05 03:34:54 +00:00

(Migrated from gitlab.com)

mentioned in merge request !760