code/cl8y-dex-terraclassic

Fork 0

feat(factory): bounded SetLpAdminAll/Batch; drop unbounded rotation fanout (#277) #760

Merged

Brouie merged 2 commits from qa/277-bounded-lp-admin-rotation into main

2026-06-05 04:10:18 +00:00

Brouie commented

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

(Migrated from gitlab.com)

Follow-up on #277 — bounded FALLBACK (SetLpAdminAll / SetLpAdminBatch); PRIMARY (LP admin=factory at instantiate, no fanout) remains an alternative.

Summary

execute_update_config no longer fans SetLpAdmin to every pair (unbounded gas DoS as the factory grows). Governance rotates factory config in one tx; operators propagate LP CW20 admin explicitly via SetLpAdminAll (≤10 pairs) or paginated SetLpAdminBatch, mirroring SetDiscountRegistryAll/Batch.

Merge fix (this push): Rebased onto current main — resolved import conflict with pair-creation fee paths (BankMsg, CosmosMsg, Uint128). Documented LP admin rollout in docs/contracts-terraclassic.md and skills/AGENTS_TERRACLASSIC_GAS.md.

Verification checklist

Item	Command	Result
Merge conflict resolved (`factory/contract.rs` imports)	`git merge origin/main`	PASS
Factory unit + integration tests	`cd smartcontracts && cargo test -p cl8y-dex-tests`	PASS (361/0)
Contract workspace tests	`make test-contracts`	PASS
Contract lint (fmt + clippy)	`make lint-contracts`	PASS
LP admin rotation test	`cargo test factory_lp_admin`	PASS
Fee tier docs drift	`python3 scripts/check_fee_discount_tier_docs.py`	PASS
Security review (automated)	MR thread 2026-06-05	PASS (`FINDINGS: 0` medium+)

Issue #277 acceptance

No single tx builds an unbounded message vector over all pairs (UpdateConfig fanout removed; All/Batch capped).
Governance rotation succeeds regardless of pair count (UpdateConfig O(1); LP admin via SetLpAdminBatch pagination).

Closes #277

Follow-up on #277 — bounded **FALLBACK** (`SetLpAdminAll` / `SetLpAdminBatch`); **PRIMARY** (LP admin=factory at instantiate, no fanout) remains an alternative. ## Summary `execute_update_config` no longer fans `SetLpAdmin` to every pair (unbounded gas DoS as the factory grows). Governance rotates factory config in one tx; operators propagate LP CW20 `admin` explicitly via **`SetLpAdminAll`** (≤10 pairs) or paginated **`SetLpAdminBatch`**, mirroring `SetDiscountRegistryAll/Batch`. **Merge fix (this push):** Rebased onto current `main` — resolved import conflict with pair-creation fee paths (`BankMsg`, `CosmosMsg`, `Uint128`). Documented LP admin rollout in `docs/contracts-terraclassic.md` and `skills/AGENTS_TERRACLASSIC_GAS.md`. ## Verification checklist | Item | Command | Result | |------|---------|--------| | Merge conflict resolved (`factory/contract.rs` imports) | `git merge origin/main` | PASS | | Factory unit + integration tests | `cd smartcontracts && cargo test -p cl8y-dex-tests` | PASS (361/0) | | Contract workspace tests | `make test-contracts` | PASS | | Contract lint (fmt + clippy) | `make lint-contracts` | PASS | | LP admin rotation test | `cargo test factory_lp_admin` | PASS | | Fee tier docs drift | `python3 scripts/check_fee_discount_tier_docs.py` | PASS | | Security review (automated) | MR thread 2026-06-05 | PASS (`FINDINGS: 0` medium+) | ## Issue #277 acceptance - [x] No single tx builds an unbounded message vector over all pairs (`UpdateConfig` fanout removed; All/Batch capped). - [x] Governance rotation succeeds regardless of pair count (`UpdateConfig` O(1); LP admin via `SetLpAdminBatch` pagination). Closes #277

PlasticDigits commented

2026-06-05 03:36:33 +00:00

(Migrated from gitlab.com)

Security review (automated)

Commit reviewed: 1b2e6f6758e2857cbd2ad94d2e394574d3cfc8fc
Scope: Factory SetLpAdminAll / SetLpAdminBatch; removal of unbounded UpdateConfig → SetLpAdmin fanout (smartcontracts/contracts/factory/src/contract.rs, error.rs, dex-common factory msgs, rotation test).

Outcome: FINDINGS: 0 medium+

What was checked

Authn/authz: Both new executes call ensure_governance before work; pair-side SetLpAdmin still requires info.sender == pair_info.factory, so only the factory can rotate LP-token admin on a pair.
Input validation: admin is addr_validated on the factory; pair re-validates before WasmMsg::UpdateAdmin.
Gas / DoS: Unbounded fanout on governance change is removed; All/Batch mirror the existing bounded SetDiscountRegistryAll/Batch pattern (calc_limit, cap error, paginated cursor).
Attacker-controlled paths: No new unprivileged entrypoints; non-governance SetLpAdminAll rejection is covered in tests.

Notes (not reported as findings)

Post-rotation LP admin may lag factory governance until operators run All/Batch — operational sequencing / trusted-governance concern, same class as the discount-registry rollout model.
SetLpAdminAll allows any validated admin, not only config.governance; that is explicit governance power, not an external bypass.

Inline threads: none (no medium+ issues on this diff).

## Security review (automated) **Commit reviewed:** `1b2e6f6758e2857cbd2ad94d2e394574d3cfc8fc` **Scope:** Factory `SetLpAdminAll` / `SetLpAdminBatch`; removal of unbounded `UpdateConfig` → `SetLpAdmin` fanout (`smartcontracts/contracts/factory/src/contract.rs`, `error.rs`, `dex-common` factory msgs, rotation test). **Outcome:** `FINDINGS: 0` medium+ ### What was checked - **Authn/authz:** Both new executes call `ensure_governance` before work; pair-side `SetLpAdmin` still requires `info.sender == pair_info.factory`, so only the factory can rotate LP-token admin on a pair. - **Input validation:** `admin` is `addr_validate`d on the factory; pair re-validates before `WasmMsg::UpdateAdmin`. - **Gas / DoS:** Unbounded fanout on governance change is removed; All/Batch mirror the existing bounded `SetDiscountRegistryAll/Batch` pattern (`calc_limit`, cap error, paginated cursor). - **Attacker-controlled paths:** No new unprivileged entrypoints; non-governance `SetLpAdminAll` rejection is covered in tests. ### Notes (not reported as findings) - Post-rotation LP admin may lag factory governance until operators run All/Batch — operational sequencing / trusted-governance concern, same class as the discount-registry rollout model. - `SetLpAdminAll` allows any validated `admin`, not only `config.governance`; that is explicit governance power, not an external bypass. **Inline threads:** none (no medium+ issues on this diff).

PlasticDigits commented

2026-06-05 03:45:17 +00:00

(Migrated from gitlab.com)

added 24 commits

1b2e6f67...9f0babe5 - 23 commits from branch PlasticDigits:main
5b4845d2 - merge(main): resolve import conflict; document bounded LP admin rotation (#277)

Compare with previous version

added 24 commits <ul><li>1b2e6f67...9f0babe5 - 23 commits from branch <code>PlasticDigits:main</code></li><li>5b4845d2 - merge(main): resolve import conflict; document bounded LP admin rotation (#277)</li></ul> [Compare with previous version](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/42/diffs?diff_id=1835866671&start_sha=1b2e6f6758e2857cbd2ad94d2e394574d3cfc8fc)

PlasticDigits commented

2026-06-05 03:45:38 +00:00

(Migrated from gitlab.com)

changed the description

PlasticDigits commented

2026-06-05 03:48:21 +00:00

(Migrated from gitlab.com)

Security review (automated)

Commit reviewed: 5b4845d21cd38fe9b1dd6d7eada98253cef1e931
Scope: Factory bounded SetLpAdminAll / SetLpAdminBatch; removal of unbounded UpdateConfig → SetLpAdmin fanout (smartcontracts/contracts/factory, dex-common factory msgs, rotation test); docs/skills (#277); cosmetic LimitOrderLadderPanel formatting.

Outcome: FINDINGS: 0 medium+

What was checked

Authn/authz: SetLpAdminAll and SetLpAdminBatch call ensure_governance before any work. Pair SetLpAdmin still requires info.sender == pair_info.factory, so only the factory can rotate LP-token admin on a pair.
Input validation: admin is addr_validated on the factory; the pair re-validates before WasmMsg::UpdateAdmin.
Gas / DoS: Unbounded fanout on governance change is removed; All/Batch mirror the existing bounded SetDiscountRegistryAll/Batch pattern (calc_limit, LpAdminAllTooManyPairs, paginated next_start_after).
Attacker-controlled paths: No new unprivileged entrypoints; non-governance SetLpAdminAll rejection is covered in tests.

Notes (not reported as findings)

LP-token admin may lag factory governance until operators run All/Batch — operational sequencing / trusted-governance concern, same class as the discount-registry rollout model (documented in MR and docs/contracts-terraclassic.md).
SetLpAdminAll allows any validated admin, not only config.governance; that is explicit governance power, not an external bypass.
max_spread.rs diff is formatting-only; no logic change.

Inline threads: none (no medium+ issues on this diff).

Security review: no medium+ findings on this diff.

## Security review (automated) **Commit reviewed:** `5b4845d21cd38fe9b1dd6d7eada98253cef1e931` **Scope:** Factory bounded `SetLpAdminAll` / `SetLpAdminBatch`; removal of unbounded `UpdateConfig` → `SetLpAdmin` fanout (`smartcontracts/contracts/factory`, `dex-common` factory msgs, rotation test); docs/skills (#277); cosmetic `LimitOrderLadderPanel` formatting. **Outcome:** `FINDINGS: 0` medium+ ### What was checked - **Authn/authz:** `SetLpAdminAll` and `SetLpAdminBatch` call `ensure_governance` before any work. Pair `SetLpAdmin` still requires `info.sender == pair_info.factory`, so only the factory can rotate LP-token admin on a pair. - **Input validation:** `admin` is `addr_validate`d on the factory; the pair re-validates before `WasmMsg::UpdateAdmin`. - **Gas / DoS:** Unbounded fanout on governance change is removed; All/Batch mirror the existing bounded `SetDiscountRegistryAll/Batch` pattern (`calc_limit`, `LpAdminAllTooManyPairs`, paginated `next_start_after`). - **Attacker-controlled paths:** No new unprivileged entrypoints; non-governance `SetLpAdminAll` rejection is covered in tests. ### Notes (not reported as findings) - LP-token admin may lag factory governance until operators run All/Batch — operational sequencing / trusted-governance concern, same class as the discount-registry rollout model (documented in MR and `docs/contracts-terraclassic.md`). - `SetLpAdminAll` allows any validated `admin`, not only `config.governance`; that is explicit governance power, not an external bypass. - `max_spread.rs` diff is formatting-only; no logic change. **Inline threads:** none (no medium+ issues on this diff). Security review: no medium+ findings on this diff.

PlasticDigits commented

2026-06-05 04:10:19 +00:00

(Migrated from gitlab.com)

mentioned in commit 091ca26061

mentioned in commit 091ca260611f9b65ae70f5b20d593f20f169dc3f