security(pair): UpdateLimitOrderPrice skips F6 code-id gate #1234

Closed
opened 2026-09-11 08:07:42 +00:00 by PlasticDigits · 4 comments

Summary

Closed #582 shipped invariant F6: pair write paths call gate_asset_code_ids → asset_code_id_guard::assert_asset_code_ids so a listed CW20 that migrates off the factory whitelist (or off the listing-time pin) fail-closes. Live check is pin equality plus factory IsCodeIdWhitelisted.

ExecuteMsg::UpdateLimitOrderPrice is the maker limit execute arm that still skips that gate. In smartcontracts/contracts/pair/src/contract.rs it runs pause + wallet blacklist, then execute_update_limit_order_price / relink_limit_order_price. Sibling limit arms (CancelLimitOrder(s), ClaimExpiredLimitOrder(s)) call gate_asset_code_ids. CW20 Receive (swap / withdraw / place / batch / ladder) gates once before the hook match.

This is not a reopen of #582. That ticket’s ACs (pin + whitelist re-check on swap / provide / withdraw / place+fill / cancel / claim, factory-then-pair migrate) are done. The merge note treated UpdateLimitOrderPrice as ungated because the tx does not move CW20. That rationale does not match F6 as written: the playbook (skills/AGENTS_CW20_CODE_ID_PIN.md) requires a write-path re-check. Relink mutates the active bid/ask DLL (price, FIFO slot) while escrow remaining stays put. After code-id drift or a governance freeze (RemoveWhitelistedCodeId), makers can still reprice resting orders on a pair that must not accept book writes.

Do not retarget #1227 (same-id FIFO leapfrog) or closed #393 (wallet-blacklist test coverage). Those share the message name only.

Bundle (same ticket, do not split): CleanLimitBook is the other book-mutation execute that skips gate_asset_code_ids (pause only). Decide whether keeper unlink during F6 freeze is allowed; UpdateLimitOrderPrice must be gated regardless. Admin-only config msgs (UpdateLimitOrderConfig, UpdateLimitCleanConfig) stay out of scope.

Impact (Immunefi-style)

Question Answer
Funds at risk today? No new steal-from-pool / cross-escrow drain on the update tx itself (L1: no CW20, remaining / PENDING_ESCROW_* unchanged). The hole is freeze bypass: after pin drift or whitelist removal, swap / place / cancel / claim fail closed while the owner can still relink. Combined with #1227, a maker can also jump FIFO at the destination price on a pair that F6 says is frozen.
Auth / admin required? Maker owner of a resting order. No factory/pair admin. Token wasm admin of a listed instance is the usual #582 migrate actor; this path does not require that key once the order exists.
Theft vs lock? Control-gap / freeze bypass, not a direct drain. Hostile wasm after migrate still cannot settle through gated fill/cancel/claim until unfrozen; the book shape can change under that freeze.
Sticky once armed? Until pair wasm includes the gate (or operators pause the pair). Pause already blocks this arm; F6 freeze without pause does not.
Scope Pair execute arm for UpdateLimitOrderPrice (+ decide CleanLimitBook). Docs/playbook F6 path list. Pin tests (asset_code_id_pin_tests). No indexer/frontend execute change unless they assume reprice works during code_id_frozen.

Invariant that is broken: F6 write-path re-check. A pair whose live asset code_id drifted from ASSET_CODE_IDS or is no longer factory-whitelisted must not accept maker book mutation. “No CW20 in this tx” is not an exemption from pin/whitelist.

Constraints / guardrails

  • Do not reopen #582, #584, or #585. Do not reopen #247 (price-only Edit identity) or #1227 (FIFO at the new price). Keep #247 gas properties: owner-only, no maker fee, no token movement on success, LimitInsertStepsExceeded still all-or-nothing.
  • Call the existing helper (gate_asset_code_ids / assert_asset_code_ids). Do not fork a second pin check. Fail closed on query errors (AssetCodeIdGuardUnavailable), same as other arms.
  • Preserve pause and wallet blacklist order already on this arm. Gate after those (same pattern as cancel/claim).
  • L1 escrow: success still must not change remaining or PENDING_ESCROW_*. F6 reject must not half-detach the node.
  • Simulation / indexer: quotes already fail closed on freeze (#585). Do not teach the dApp to reprice through a frozen pair. Edit UX should surface the same humanized F6 error as place/cancel.
  • CleanLimitBook: if keepers must still park expired/dust during freeze, document that exception in F6 item 2 / cw20-code-id-ops.md. If freeze means no DLL writes, gate it too. Do not silently leave it as the only other hole without a written decision.
  • No FoT / rebase swap math (H-01). No public migrate+reprice sequence on a live pair.
  • Founder-required CosmWasm pair / wasm. No community autoland. Do not add ready.

Relevant files

Path Why
smartcontracts/contracts/pair/src/contract.rs ExecuteMsg::UpdateLimitOrderPrice omits gate_asset_code_ids; cancel/claim/receive already call it
smartcontracts/contracts/pair/src/asset_code_id_guard.rs assert_asset_code_ids — reuse, do not duplicate
smartcontracts/contracts/pair/src/orderbook.rs relink_limit_order_price — mutation that must not run after a failed gate
smartcontracts/contracts/pair/src/limit_book_clean.rs CleanLimitBook sibling ungated writer
smartcontracts/tests/src/asset_code_id_pin_tests.rs (or current pin-test module) No reprice-after-drift / freeze case today
skills/AGENTS_CW20_CODE_ID_PIN.md F6 item 2 list omits price update
docs/security-model.md, docs/runbooks/cw20-whitelist-policy.md, docs/runbooks/cw20-code-id-ops.md Path matrix + exit-path policy
  1. In the UpdateLimitOrderPrice execute arm, call gate_asset_code_ids(deps.as_ref())? immediately after the existing pause + blacklist gates, before execute_update_limit_order_price.
  2. Add pin tests: honest place → migrate asset wasm off pin / remove whitelist → UpdateLimitOrderPrice returns AssetCodeIdDrift or AssetCodeIdNotWhitelisted (same error as swap/place). Order still linked at the old price; escrow unchanged. After RefreshAssetCodeIds on a still-whitelisted honest id (or restore whitelist + refresh per existing tests), reprice succeeds.
  3. Update F6 item 2 and the security/runbook path lists: limit place + fill + cancel + claim + price update. Record the CleanLimitBook decision in the same docs edit.
  4. Keep make verify-issue-582 green; extend it or the pin suite so a missing UpdateLimitOrderPrice gate fails CI.

Acceptance criteria

  • AC1. After F6 drift or whitelist freeze, UpdateLimitOrderPrice aborts with the existing asset-code-id errors. The resting order is not unlinked or repriced. remaining / PENDING_ESCROW_* unchanged.
  • AC2. On an honest pinned pair, owner reprice still succeeds (pause off, not blacklisted, in-band price, step budget). #247 no-CW20 / no-fee properties stay.
  • AC3. Cancel / claim / place / swap / provide / withdraw gates are unchanged (still fail closed). Do not loosen exit-path policy (#582 F6 item 7) as a side effect.
  • AC4. CleanLimitBook either also calls gate_asset_code_ids, or docs explicitly keep it as a freeze-safe keeper exception. One written choice.
  • AC5. Playbook F6 item 2 and security-model path matrix name UpdateLimitOrderPrice.
  • AC6. Focused pin tests + make verify-issue-582 (or the repo’s documented contract suite) pass.

Verification (non-exploitative)

Do not publish a mainnet migrate + reprice recipe. Verify in multi-test only:

  1. Existing pin suite: honest CreatePair → FoT/unlisted migrate → swap still fails; add UpdateLimitOrderPrice on a resting order in the same setup and assert the F6 error, book price unchanged.
  2. Whitelist-removal freeze: RemoveWhitelistedCodeId of the pin → reprice fails; swap/place still fail.
  3. Happy path: no drift → reprice still updates price (and does not move CW20).
  4. Reject-then-state: after F6 error, GetLimitOrder / book page still shows the old price and id.
  5. CleanLimitBook coverage matches whatever AC4 chose.

First-pass model recommendation

Recommendation: grok-high

Rationale: Founder-required CosmWasm pair / wasm and an F6 security control (code-id pin + factory whitelist). Even a one-arm gate_asset_code_ids call still changes freeze semantics for Edit, must not half-detach on reject, and needs pin-test plus playbook/path-matrix updates. Composer is disallowed for security and for contracts/wasm. Verify with the pin tests above and make verify-issue-582, not a live-chain migrate.

## Summary Closed [#582](https://git.cl8y.com/code/cl8y-dex-terraclassic/issues/582) shipped invariant **F6**: pair write paths call `gate_asset_code_ids` → `asset_code_id_guard::assert_asset_code_ids` so a listed CW20 that migrates off the factory whitelist (or off the listing-time pin) fail-closes. Live check is pin equality plus factory `IsCodeIdWhitelisted`. `ExecuteMsg::UpdateLimitOrderPrice` is the maker limit execute arm that still skips that gate. In `smartcontracts/contracts/pair/src/contract.rs` it runs pause + wallet blacklist, then `execute_update_limit_order_price` / `relink_limit_order_price`. Sibling limit arms (`CancelLimitOrder(s)`, `ClaimExpiredLimitOrder(s)`) call `gate_asset_code_ids`. CW20 `Receive` (swap / withdraw / place / batch / ladder) gates once before the hook match. This is not a reopen of #582. That ticket’s ACs (pin + whitelist re-check on swap / provide / withdraw / place+fill / cancel / claim, factory-then-pair migrate) are done. The merge note treated `UpdateLimitOrderPrice` as ungated because the tx does not move CW20. That rationale does not match F6 as written: the playbook (`skills/AGENTS_CW20_CODE_ID_PIN.md`) requires a **write-path** re-check. Relink mutates the active bid/ask DLL (price, FIFO slot) while escrow `remaining` stays put. After code-id drift or a governance freeze (`RemoveWhitelistedCodeId`), makers can still reprice resting orders on a pair that must not accept book writes. Do not retarget [#1227](https://git.cl8y.com/code/cl8y-dex-terraclassic/issues/1227) (same-id FIFO leapfrog) or closed [#393](https://git.cl8y.com/code/cl8y-dex-terraclassic/issues/393) (wallet-blacklist test coverage). Those share the message name only. Bundle (same ticket, do not split): `CleanLimitBook` is the other book-mutation execute that skips `gate_asset_code_ids` (pause only). Decide whether keeper unlink during F6 freeze is allowed; `UpdateLimitOrderPrice` must be gated regardless. Admin-only config msgs (`UpdateLimitOrderConfig`, `UpdateLimitCleanConfig`) stay out of scope. ## Impact (Immunefi-style) | Question | Answer | | --- | --- | | Funds at risk today? | No new steal-from-pool / cross-escrow drain on the update tx itself (L1: no CW20, `remaining` / `PENDING_ESCROW_*` unchanged). The hole is **freeze bypass**: after pin drift or whitelist removal, swap / place / cancel / claim fail closed while the owner can still relink. Combined with #1227, a maker can also jump FIFO at the destination price on a pair that F6 says is frozen. | | Auth / admin required? | Maker owner of a resting order. No factory/pair admin. Token wasm admin of a listed instance is the usual #582 migrate actor; this path does not require that key once the order exists. | | Theft vs lock? | Control-gap / freeze bypass, not a direct drain. Hostile wasm after migrate still cannot settle through gated fill/cancel/claim until unfrozen; the book shape can change under that freeze. | | Sticky once armed? | Until pair wasm includes the gate (or operators pause the pair). Pause already blocks this arm; F6 freeze without pause does not. | | Scope | Pair `execute` arm for `UpdateLimitOrderPrice` (+ decide `CleanLimitBook`). Docs/playbook F6 path list. Pin tests (`asset_code_id_pin_tests`). No indexer/frontend execute change unless they assume reprice works during `code_id_frozen`. | Invariant that is broken: **F6 write-path re-check**. A pair whose live asset `code_id` drifted from `ASSET_CODE_IDS` or is no longer factory-whitelisted must not accept maker book mutation. “No CW20 in this tx” is not an exemption from pin/whitelist. ## Constraints / guardrails - Do not reopen #582, #584, or #585. Do not reopen #247 (price-only Edit identity) or #1227 (FIFO at the new price). Keep #247 gas properties: owner-only, no maker fee, no token movement on success, `LimitInsertStepsExceeded` still all-or-nothing. - Call the existing helper (`gate_asset_code_ids` / `assert_asset_code_ids`). Do not fork a second pin check. Fail closed on query errors (`AssetCodeIdGuardUnavailable`), same as other arms. - Preserve pause and wallet blacklist order already on this arm. Gate after those (same pattern as cancel/claim). - L1 escrow: success still must not change `remaining` or `PENDING_ESCROW_*`. F6 reject must not half-detach the node. - Simulation / indexer: quotes already fail closed on freeze (#585). Do not teach the dApp to reprice through a frozen pair. Edit UX should surface the same humanized F6 error as place/cancel. - `CleanLimitBook`: if keepers must still park expired/dust during freeze, document that exception in F6 item 2 / `cw20-code-id-ops.md`. If freeze means no DLL writes, gate it too. Do not silently leave it as the only other hole without a written decision. - No FoT / rebase swap math (H-01). No public migrate+reprice sequence on a live pair. - Founder-required CosmWasm pair / wasm. No community autoland. Do not add `ready`. ## Relevant files | Path | Why | | --- | --- | | `smartcontracts/contracts/pair/src/contract.rs` | `ExecuteMsg::UpdateLimitOrderPrice` omits `gate_asset_code_ids`; cancel/claim/receive already call it | | `smartcontracts/contracts/pair/src/asset_code_id_guard.rs` | `assert_asset_code_ids` — reuse, do not duplicate | | `smartcontracts/contracts/pair/src/orderbook.rs` | `relink_limit_order_price` — mutation that must not run after a failed gate | | `smartcontracts/contracts/pair/src/limit_book_clean.rs` | `CleanLimitBook` sibling ungated writer | | `smartcontracts/tests/src/asset_code_id_pin_tests.rs` (or current pin-test module) | No reprice-after-drift / freeze case today | | `skills/AGENTS_CW20_CODE_ID_PIN.md` | F6 item 2 list omits price update | | `docs/security-model.md`, `docs/runbooks/cw20-whitelist-policy.md`, `docs/runbooks/cw20-code-id-ops.md` | Path matrix + exit-path policy | ## Recommended direction 1. In the `UpdateLimitOrderPrice` execute arm, call `gate_asset_code_ids(deps.as_ref())?` immediately after the existing pause + blacklist gates, before `execute_update_limit_order_price`. 2. Add pin tests: honest place → migrate asset wasm off pin / remove whitelist → `UpdateLimitOrderPrice` returns `AssetCodeIdDrift` or `AssetCodeIdNotWhitelisted` (same error as swap/place). Order still linked at the old price; escrow unchanged. After `RefreshAssetCodeIds` on a still-whitelisted honest id (or restore whitelist + refresh per existing tests), reprice succeeds. 3. Update F6 item 2 and the security/runbook path lists: limit **place + fill + cancel + claim + price update**. Record the `CleanLimitBook` decision in the same docs edit. 4. Keep `make verify-issue-582` green; extend it or the pin suite so a missing `UpdateLimitOrderPrice` gate fails CI. ## Acceptance criteria - AC1. After F6 drift or whitelist freeze, `UpdateLimitOrderPrice` aborts with the existing asset-code-id errors. The resting order is not unlinked or repriced. `remaining` / `PENDING_ESCROW_*` unchanged. - AC2. On an honest pinned pair, owner reprice still succeeds (pause off, not blacklisted, in-band price, step budget). #247 no-CW20 / no-fee properties stay. - AC3. Cancel / claim / place / swap / provide / withdraw gates are unchanged (still fail closed). Do not loosen exit-path policy (#582 F6 item 7) as a side effect. - AC4. `CleanLimitBook` either also calls `gate_asset_code_ids`, or docs explicitly keep it as a freeze-safe keeper exception. One written choice. - AC5. Playbook F6 item 2 and security-model path matrix name `UpdateLimitOrderPrice`. - AC6. Focused pin tests + `make verify-issue-582` (or the repo’s documented contract suite) pass. ## Verification (non-exploitative) Do not publish a mainnet migrate + reprice recipe. Verify in multi-test only: 1. Existing pin suite: honest CreatePair → FoT/unlisted migrate → swap still fails; add `UpdateLimitOrderPrice` on a resting order in the same setup and assert the F6 error, book price unchanged. 2. Whitelist-removal freeze: `RemoveWhitelistedCodeId` of the pin → reprice fails; swap/place still fail. 3. Happy path: no drift → reprice still updates price (and does not move CW20). 4. Reject-then-state: after F6 error, `GetLimitOrder` / book page still shows the old price and id. 5. `CleanLimitBook` coverage matches whatever AC4 chose. ## First-pass model recommendation Recommendation: grok-high Rationale: Founder-required CosmWasm pair / wasm and an F6 security control (code-id pin + factory whitelist). Even a one-arm `gate_asset_code_ids` call still changes freeze semantics for Edit, must not half-detach on reject, and needs pin-test plus playbook/path-matrix updates. Composer is disallowed for security and for contracts/wasm. Verify with the pin tests above and `make verify-issue-582`, not a live-chain migrate.
Author
Owner

Merged as PR #1288. AC1–AC6 are in tree (F6 gates UpdateLimitOrderPrice and CleanLimitBook).

Leftover: columbus-5 listed pairs still need a pair wasm store+migrate before the gate is live (cw2 stays 1.17.0). Tracked on #1300.

Merged as PR #1288. AC1–AC6 are in tree (F6 gates `UpdateLimitOrderPrice` and `CleanLimitBook`). Leftover: columbus-5 listed pairs still need a pair wasm store+migrate before the gate is live (cw2 stays 1.17.0). Tracked on #1300.
Author
Owner

scripts/qa/verify-issue-1234.sh header still lists make verify-issue-582 as step 4, but the script does not invoke it. Pin tests for #1234 run; shared #582 green is a separate make verify-issue-582. Optional leftover on #1300 if an implement wants the alias to actually call 582.

`scripts/qa/verify-issue-1234.sh` header still lists `make verify-issue-582` as step 4, but the script does not invoke it. Pin tests for #1234 run; shared #582 green is a separate `make verify-issue-582`. Optional leftover on #1300 if an implement wants the alias to actually call 582.
Author
Owner

Live pairs are still code 11664 (1.16.0). Factory is already 11629 / 1.10.0.

Inbox card filed (desktop): store 1.17.0 pair wasm and migrate. Use UPGRADE582_SKIP_FACTORY_MIGRATE=1. Do not SKIP_STORE with 11664.

Leaving this issue open until the migrate tx lands.

Live pairs are still code 11664 (1.16.0). Factory is already 11629 / 1.10.0. Inbox card filed (desktop): store 1.17.0 pair wasm and migrate. Use `UPGRADE582_SKIP_FACTORY_MIGRATE=1`. Do not `SKIP_STORE` with 11664. Leaving this issue open until the migrate tx lands.
Author
Owner

On-chain leftover for F6 + named min remaining is done.

Live columbus-5 LCD:

  • Factory terra1ejpg…chsea code 11629 / cw2 1.10.0 (unchanged)
  • config.pair_code_id 11672
  • Sampled listed pairs (UST1/cUSTC, UST1/USTR, cLUNC/cUSTC, CL8Y-cb/cUSTC, CL8Y-cb/cLUNC, cLUNC/UST1) code 11672 / cw2 1.17.0 (was 11664 / 1.16.0)

Operator inbox card Store and migrate pair wasm completed. Host make verify-issue-1234 already passed. Closing.

On-chain leftover for F6 + named min remaining is done. Live columbus-5 LCD: - Factory `terra1ejpg…chsea` code **11629** / cw2 **1.10.0** (unchanged) - `config.pair_code_id` **11672** - Sampled listed pairs (UST1/cUSTC, UST1/USTR, cLUNC/cUSTC, CL8Y-cb/cUSTC, CL8Y-cb/cLUNC, cLUNC/UST1) code **11672** / cw2 **1.17.0** (was 11664 / 1.16.0) Operator inbox card **Store and migrate pair wasm** completed. Host `make verify-issue-1234` already passed. Closing.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
code/cl8y-dex-terraclassic#1234
No description provided.