#274 - clean_limit_book caps parks but not traversal — unbounded scan / gas DoS - code/cl8y-dex-terraclassic

Brouie commented

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

(Migrated from gitlab.com)

Severity: Medium
Reachability: Permissionless. Anyone calls CleanLimitBook; the cost scales with the live orders ahead of the first eligible one.
Affected: clean_limit_book (smartcontracts/contracts/pair/src/limit_book_clean.rs).
Root cause: max_orders caps how many orders get parked, not how many get traversed. There is no scan-step cap.

Summary

clean_limit_book walks the book from the head (or a hint) and parks expired / dust orders, up to max_orders. But the loop only breaks on cleaned_count >= cap, and cleaned_count increments only when an order is actually parked. Zero-remaining orders continue without counting, and live, healthy, non-expired, non-dust orders fall straight through and advance cur = order.next without counting either.

So a long run of healthy orders at the head is traversed in full — one ORDERS.load per node — no matter what max_orders is. The matcher has a MAX_SCAN_STEPS cap for exactly this reason; clean has no equivalent.

Impact: clean's gas is O(orders ahead of the first eligible one), not O(max_orders). If a head-run of healthy orders is long enough to blow the gas budget, clean can never make progress past it, so a clogged head of expired/dust orders sitting behind that run can't be cleared.

Current codebase

limit_book_clean.rs clean_limit_book: while let Some(oid) = cur { if cleaned_count >= cap { break } ... } — cleaned_count += 1 only inside the if time_exp || force park branch; healthy orders just do cur = next_ptr.
Compare smartcontracts/contracts/pair/src/orderbook.rs matcher, which bounds traversal with MAX_SCAN_STEPS independent of fills.

Recommended direction

Add a traversal-step cap to clean_limit_book (mirror MAX_SCAN_STEPS): count every node visited, break with a "scan capped" flag when hit, return a resume hint.
Keep the park cap as-is; the two caps are independent (one bounds work, one bounds effect).

Acceptance criteria

clean_limit_book visits at most a bounded number of nodes per call regardless of book length.
A head-run of N healthy orders does not make a clean call cost grow with N.
Caller can resume cleaning past a long healthy run via the returned hint.

Test plan (attack / abuse)

case	expect
book head = long run of healthy orders, expired ones behind	clean caps traversal, returns resume hint, bounded gas
repeated clean with resume hint	eventually reaches and parks the expired tail

**Severity:** Medium **Reachability:** Permissionless. Anyone calls `CleanLimitBook`; the cost scales with the live orders ahead of the first eligible one. **Affected:** `clean_limit_book` (`smartcontracts/contracts/pair/src/limit_book_clean.rs`). **Root cause:** `max_orders` caps how many orders get *parked*, not how many get *traversed*. There is no scan-step cap. ## Summary `clean_limit_book` walks the book from the head (or a hint) and parks expired / dust orders, up to `max_orders`. But the loop only breaks on `cleaned_count >= cap`, and `cleaned_count` increments **only** when an order is actually parked. Zero-remaining orders `continue` without counting, and live, healthy, non-expired, non-dust orders fall straight through and advance `cur = order.next` without counting either. So a long run of healthy orders at the head is traversed in full — one `ORDERS.load` per node — no matter what `max_orders` is. The matcher has a `MAX_SCAN_STEPS` cap for exactly this reason; clean has no equivalent. Impact: clean's gas is O(orders ahead of the first eligible one), not O(max_orders). If a head-run of healthy orders is long enough to blow the gas budget, clean can never make progress past it, so a clogged head of expired/dust orders sitting *behind* that run can't be cleared. ## Current codebase - `limit_book_clean.rs` `clean_limit_book`: `while let Some(oid) = cur { if cleaned_count >= cap { break } ... }` — `cleaned_count += 1` only inside the `if time_exp || force` park branch; healthy orders just do `cur = next_ptr`. - Compare `smartcontracts/contracts/pair/src/orderbook.rs` matcher, which bounds traversal with `MAX_SCAN_STEPS` independent of fills. ## Recommended direction 1. Add a traversal-step cap to `clean_limit_book` (mirror `MAX_SCAN_STEPS`): count every node visited, break with a "scan capped" flag when hit, return a resume hint. 2. Keep the park cap as-is; the two caps are independent (one bounds work, one bounds effect). ## Acceptance criteria - [ ] `clean_limit_book` visits at most a bounded number of nodes per call regardless of book length. - [ ] A head-run of N healthy orders does not make a clean call cost grow with N. - [ ] Caller can resume cleaning past a long healthy run via the returned hint. ## Test plan (attack / abuse) | case | expect | |---|---| | book head = long run of healthy orders, expired ones behind | clean caps traversal, returns resume hint, bounded gas | | repeated clean with resume hint | eventually reaches and parks the expired tail |

PlasticDigits commented

2026-06-03 10:33:56 +00:00

(Migrated from gitlab.com)

First, clean_limit_book should consume a start index for where in the buy/sell it should start then step away from the market price.
Second, clean_limit_book should consume a max steps for how long it will run before exiting (unless it hits the end of the book at farthest price from market price).
This allow callers to decide how far they want to clean.

First, `clean_limit_book` should consume a start index for where in the buy/sell it should start then step away from the market price. Second, `clean_limit_book` should consume a max steps for how long it will run before exiting (unless it hits the end of the book at farthest price from market price). This allow callers to decide how far they want to clean.

Brouie commented

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

(Migrated from gitlab.com)

Implementation plan (your "start index + max steps" direction). Small, contract-only.

Add max_steps: u32 to clean_limit_book + a MAX_CLEAN_SCAN_STEPS=500 cap in dex-common/limit_clean.rs (mirrors the matcher's MAX_SCAN_STEPS, same ~19k-gas/iter sizing). Count EVERY visited node at the top of the walk loop — zero-remaining continue, healthy fall-through, AND parked — and break when the cap is hit. start_hint stays the "start index"; the existing head→tail next walk is already directional (head = nearest market, next = away). Park cap (max_orders) stays independent.
Return scan_capped + resume_cursor on CleanLimitBookResult and emit them as execute attrs, so a keeper resumes by re-submitting with start_hint = resume_cursor until scan_capped=false. Add optional max_steps to the CleanLimitBook ExecuteMsg (None → cap; backward-compatible).
Files: limit_book_clean.rs (loop + result + sig), dex-common/limit_clean.rs (const + clamp), dex-common/pair.rs (msg field), pair/contract.rs (thread + attrs).

The gotcha: resume_cursor must be the FIRST UNVISITED node (the oid the walk broke before), NOT the last processed — clean doesn't consume the healthy head, so re-passing the same start_hint would loop forever on the same prefix. Off-by-one here = no progress or skipped orders. A stale cursor degrades safely (resolve_start validates + falls back to head). No DB/frontend/indexer/router ripple (clean is a keeper action). Precedent: book_walk_step/MAX_SCAN_STEPS + match_bids_scan_steps_cap_bounds_expired_prefix_walk. Tests: traversal-capped, resume-reaches-expired-tail, clamp, park-cap-independent, end-of-book-no-resume. @PlasticDigits

Implementation plan (your "start index + max steps" direction). Small, contract-only. - Add `max_steps: u32` to `clean_limit_book` + a `MAX_CLEAN_SCAN_STEPS=500` cap in dex-common/limit_clean.rs (mirrors the matcher's MAX_SCAN_STEPS, same ~19k-gas/iter sizing). Count EVERY visited node at the top of the walk loop — zero-remaining `continue`, healthy fall-through, AND parked — and break when the cap is hit. `start_hint` stays the "start index"; the existing head→tail `next` walk is already directional (head = nearest market, next = away). Park cap (max_orders) stays independent. - Return `scan_capped` + `resume_cursor` on `CleanLimitBookResult` and emit them as execute attrs, so a keeper resumes by re-submitting with `start_hint = resume_cursor` until `scan_capped=false`. Add optional `max_steps` to the `CleanLimitBook` ExecuteMsg (None → cap; backward-compatible). - Files: `limit_book_clean.rs` (loop + result + sig), `dex-common/limit_clean.rs` (const + clamp), `dex-common/pair.rs` (msg field), `pair/contract.rs` (thread + attrs). **The gotcha:** `resume_cursor` must be the FIRST UNVISITED node (the oid the walk broke *before*), NOT the last processed — clean doesn't consume the healthy head, so re-passing the same `start_hint` would loop forever on the same prefix. Off-by-one here = no progress or skipped orders. A stale cursor degrades safely (resolve_start validates + falls back to head). No DB/frontend/indexer/router ripple (clean is a keeper action). Precedent: `book_walk_step`/MAX_SCAN_STEPS + `match_bids_scan_steps_cap_bounds_expired_prefix_walk`. Tests: traversal-capped, resume-reaches-expired-tail, clamp, park-cap-independent, end-of-book-no-resume. @PlasticDigits

Brouie commented

2026-06-04 06:30:07 +00:00

(Migrated from gitlab.com)

mentioned in issue #289

Brouie commented

2026-06-05 02:17:02 +00:00

(Migrated from gitlab.com)

mentioned in merge request !753

Brouie commented

2026-06-05 02:17:15 +00:00

(Migrated from gitlab.com)

Shipped your "start index + max steps" direction — MR !753.

The hole: clean_limit_book capped parks (max_orders) but not traversal, so the walk visited every node until it parked the cap or reached the end — a book of healthy or zero-remaining orders got walked unboundedly.

Fix: MAX_CLEAN_SCAN_STEPS=500 (mirrors the matcher's MAX_SCAN_STEPS, ~19k gas/iter), counted on EVERY visited node (zero-remaining skip, healthy fall-through, parked alike). CleanLimitBookResult returns scan_capped + resume_cursor = the first UNVISITED order id, emitted as execute attrs; a keeper resumes with start_hint = resume_cursor until it clears. The off-by-one you'd worry about — resume must be the node we broke BEFORE, not the last processed, or re-passing the same start_hint loops on the same prefix; a stale cursor degrades safely (resolve_start falls back to head). New optional max_steps on the msg (#[serde(default)], absent or 0 -> full cap, backward-compatible). Park cap stays independent.

Tests (suite 419/0): traversal-bounded-when-nothing-parked (the DoS itself — 0 parks but still bounded), scan-cap-resume-reaches-tail, zero-max-steps-is-full-cap, park-cap-independent. Live gas re-confirm rides the next deploy. @PlasticDigits

Shipped your "start index + max steps" direction — MR !753. The hole: `clean_limit_book` capped parks (`max_orders`) but not traversal, so the walk visited every node until it parked the cap or reached the end — a book of healthy or zero-remaining orders got walked unboundedly. Fix: `MAX_CLEAN_SCAN_STEPS=500` (mirrors the matcher's `MAX_SCAN_STEPS`, ~19k gas/iter), counted on EVERY visited node (zero-remaining skip, healthy fall-through, parked alike). `CleanLimitBookResult` returns `scan_capped` + `resume_cursor` = the first UNVISITED order id, emitted as execute attrs; a keeper resumes with `start_hint = resume_cursor` until it clears. The off-by-one you'd worry about — resume must be the node we broke BEFORE, not the last processed, or re-passing the same `start_hint` loops on the same prefix; a stale cursor degrades safely (resolve_start falls back to head). New optional `max_steps` on the msg (`#[serde(default)]`, absent or 0 -> full cap, backward-compatible). Park cap stays independent. Tests (suite 419/0): traversal-bounded-when-nothing-parked (the DoS itself — 0 parks but still bounded), scan-cap-resume-reaches-tail, zero-max-steps-is-full-cap, park-cap-independent. Live gas re-confirm rides the next deploy. @PlasticDigits

PlasticDigits commented

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

(Migrated from gitlab.com)

mentioned in commit 974cabb659

mentioned in commit 974cabb659d85dedbb1b4a32a62e4d76f2b35eb0

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-06-05 03:49:27 +00:00

PlasticDigits commented

2026-06-05 03:49:46 +00:00

(Migrated from gitlab.com)

Verification — GitLab #274 (clean_limit_book traversal cap)

Verified on main @ 9f0babe after merge of !753.

Acceptance criteria

Criterion	Result	How verified
`clean_limit_book` visits at most a bounded number of nodes per call regardless of book length	PASS	`MAX_CLEAN_SCAN_STEPS = 500` in `dex-common/limit_clean.rs`; loop breaks when `steps >= step_cap` before loading the next node (`limit_book_clean.rs`).
A head-run of N healthy orders does not make a clean call cost grow with N	PASS	`clean_limit_book_traversal_bounded_when_nothing_parked`: 5 far-future (unparkable) bids, `max_steps=2` → `cleaned_count=0`, `scan_capped=true`, `resume_cursor=ids[2]` (only 2 nodes visited).
Caller can resume cleaning past a long healthy run via the returned hint	PASS	`clean_limit_book_scan_cap_resume_reaches_tail`: first pass parks 2 with `scan_capped`; second pass with `start_hint=resume_cursor` parks remaining 3; `scan_capped=false`, no `resume_cursor`.

Test plan (issue body)

Case	Result	How verified
Book head = long run of healthy orders, expired behind → bounded traversal + resume hint	PASS	Same as `clean_limit_book_traversal_bounded_when_nothing_parked` (healthy prefix, cap before tail).
Repeated clean with resume hint → reaches expired tail	PASS	`clean_limit_book_scan_cap_resume_reaches_tail`.

Additional checks (MR !753 / comments)

Check	Result	How verified
`max_steps` optional on `CleanLimitBook` (`None`/`0` → full cap)	PASS	`clean_limit_book_zero_max_steps_means_full_cap`; `pair.rs` `ExecuteMsg::CleanLimitBook.max_steps`.
Park cap independent of scan cap	PASS	`clean_limit_book_park_cap_independent_of_scan_cap` (`cap_hit=true`, `scan_capped=false`).
`resume_cursor` = first unvisited oid (off-by-one)	PASS	Code: break on `steps >= step_cap` before `steps += 1`; tests assert `resume_cursor=ids[2]` after 2 visits.
Execute attrs `scan_capped`, `resume_cursor`	PASS	`contract.rs` `execute_clean_limit_book`; tests read wasm attrs.
Full `clean_limit_book_*` regression suite	PASS	`cargo test clean_limit_book_` → 10/10 passed.
Live gas re-confirm on LocalTerra	SKIP	Explicitly deferred to next deploy in !753; unit tests bound traversal; no wasm redeploy in this session.

Implementation reference

Merged: !753 (feat(pair): bound CleanLimitBook traversal with max_steps + resume cursor (#274))
Constants mirror matcher: MAX_CLEAN_SCAN_STEPS = MAX_SCAN_STEPS = 500

Conclusion: Issue fixed on main; acceptance criteria and automated abuse tests pass. Closing.

## Verification — GitLab #274 (clean_limit_book traversal cap) Verified on `main` @ `9f0babe` after merge of !753. ### Acceptance criteria | Criterion | Result | How verified | |-----------|--------|----------------| | `clean_limit_book` visits at most a bounded number of nodes per call regardless of book length | **PASS** | `MAX_CLEAN_SCAN_STEPS = 500` in `dex-common/limit_clean.rs`; loop breaks when `steps >= step_cap` before loading the next node (`limit_book_clean.rs`). | | A head-run of N healthy orders does not make a clean call cost grow with N | **PASS** | `clean_limit_book_traversal_bounded_when_nothing_parked`: 5 far-future (unparkable) bids, `max_steps=2` → `cleaned_count=0`, `scan_capped=true`, `resume_cursor=ids[2]` (only 2 nodes visited). | | Caller can resume cleaning past a long healthy run via the returned hint | **PASS** | `clean_limit_book_scan_cap_resume_reaches_tail`: first pass parks 2 with `scan_capped`; second pass with `start_hint=resume_cursor` parks remaining 3; `scan_capped=false`, no `resume_cursor`. | ### Test plan (issue body) | Case | Result | How verified | |------|--------|----------------| | Book head = long run of healthy orders, expired behind → bounded traversal + resume hint | **PASS** | Same as `clean_limit_book_traversal_bounded_when_nothing_parked` (healthy prefix, cap before tail). | | Repeated clean with resume hint → reaches expired tail | **PASS** | `clean_limit_book_scan_cap_resume_reaches_tail`. | ### Additional checks (MR !753 / comments) | Check | Result | How verified | |-------|--------|----------------| | `max_steps` optional on `CleanLimitBook` (`None`/`0` → full cap) | **PASS** | `clean_limit_book_zero_max_steps_means_full_cap`; `pair.rs` `ExecuteMsg::CleanLimitBook.max_steps`. | | Park cap independent of scan cap | **PASS** | `clean_limit_book_park_cap_independent_of_scan_cap` (`cap_hit=true`, `scan_capped=false`). | | `resume_cursor` = first **unvisited** oid (off-by-one) | **PASS** | Code: break on `steps >= step_cap` before `steps += 1`; tests assert `resume_cursor=ids[2]` after 2 visits. | | Execute attrs `scan_capped`, `resume_cursor` | **PASS** | `contract.rs` `execute_clean_limit_book`; tests read wasm attrs. | | Full `clean_limit_book_*` regression suite | **PASS** | `cargo test clean_limit_book_` → **10/10** passed. | | Live gas re-confirm on LocalTerra | **SKIP** | Explicitly deferred to next deploy in !753; unit tests bound traversal; no wasm redeploy in this session. | ### Implementation reference - Merged: !753 (`feat(pair): bound CleanLimitBook traversal with max_steps + resume cursor (#274)`) - Constants mirror matcher: `MAX_CLEAN_SCAN_STEPS` = `MAX_SCAN_STEPS` = 500 **Conclusion:** Issue fixed on `main`; acceptance criteria and automated abuse tests pass. Closing.

PlasticDigits (Migrated from gitlab.com) reopened this issue

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

ghost1 commented

2026-06-05 04:47:54 +00:00

(Migrated from gitlab.com)

mentioned in commit 8e8c1ef41b

mentioned in commit 8e8c1ef41b212bcb73f8550fe1adc445ad537c67

PlasticDigits commented

2026-06-05 04:48:57 +00:00

(Migrated from gitlab.com)

Live gas re-confirm (LocalTerra) — PASS

Reopened per request; fresh wasm via QA_FRESH_VOLUMES=1 make reset-qa (stamp git_sha=1ad11dc), then make verify-issue-274 with VERIFY274_PAIR_INDEX=3 and VERIFY274_HEALTHY_COUNT=0 (book already had 100 healthy far-future bids from seed; 5 expired bids at tail 0.10).

Results

Check	Result	Evidence
Traversal capped (`max_steps=15`)	PASS	`scan_capped=true`, `cleaned_count=0`
Gas bounded vs deep book	PASS	`gas_used=152798` (≪ 800k ceiling; ~105 nodes unbounded would be ~2M+ at ~19k/step)
`resume_cursor` + resume parks tail	PASS	`resume_cursor=16`; resume tx parked 5 expired (`scan_capped=false`)
Contract attrs on live wasm	PASS	tx `E8CE2030…` / `7BA60562…`

How to reproduce

make reset-qa   # or deploy-local after build-optimized
# Seed deep book once (default script seeds 100 healthy on pair index 3):
make verify-issue-274
# Or reuse seeded book:
VERIFY274_PAIR_INDEX=3 VERIFY274_HEALTHY_COUNT=0 make verify-issue-274

MR adds scripts/qa/verify-issue-274.sh, make verify-issue-274, and deploy fix for #276 pair-creation fee attachment.

## Live gas re-confirm (LocalTerra) — PASS Reopened per request; fresh wasm via `QA_FRESH_VOLUMES=1 make reset-qa` (stamp `git_sha=1ad11dc`), then `make verify-issue-274` with `VERIFY274_PAIR_INDEX=3` and `VERIFY274_HEALTHY_COUNT=0` (book already had **100** healthy far-future bids from seed; **5** expired bids at tail `0.10`). ### Results | Check | Result | Evidence | |-------|--------|----------| | Traversal capped (`max_steps=15`) | **PASS** | `scan_capped=true`, `cleaned_count=0` | | Gas bounded vs deep book | **PASS** | `gas_used=152798` (≪ 800k ceiling; ~105 nodes unbounded would be ~2M+ at ~19k/step) | | `resume_cursor` + resume parks tail | **PASS** | `resume_cursor=16`; resume tx parked **5** expired (`scan_capped=false`) | | Contract attrs on live wasm | **PASS** | tx `E8CE2030…` / `7BA60562…` | ### How to reproduce ```bash make reset-qa # or deploy-local after build-optimized # Seed deep book once (default script seeds 100 healthy on pair index 3): make verify-issue-274 # Or reuse seeded book: VERIFY274_PAIR_INDEX=3 VERIFY274_HEALTHY_COUNT=0 make verify-issue-274 ``` MR adds `scripts/qa/verify-issue-274.sh`, `make verify-issue-274`, and deploy fix for #276 pair-creation fee attachment.

PlasticDigits commented

2026-06-05 05:00:05 +00:00

(Migrated from gitlab.com)

mentioned in merge request !768

PlasticDigits commented

2026-06-05 07:02:23 +00:00

(Migrated from gitlab.com)

mentioned in commit 4490ba8d28

mentioned in commit 4490ba8d2883963a2d6755197e083e7904fc14b7

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-06-05 07:02:24 +00:00

ghost1 commented

2026-06-05 11:02:54 +00:00

(Migrated from gitlab.com)

mentioned in commit 4c4c26846b

mentioned in commit 4c4c26846bff977763c2d66c787473c6627bbfc5

PlasticDigits commented

2026-06-05 11:03:25 +00:00

(Migrated from gitlab.com)

mentioned in merge request !795

PlasticDigits commented

2026-08-22 12:26:36 +00:00

(Migrated from gitlab.com)

mentioned in issue #597

Rows
Columns

clean_limit_book caps parks but not traversal — unbounded scan / gas DoS #274

Summary

Current codebase

Recommended direction

Acceptance criteria

Test plan (attack / abuse)

Verification — GitLab #274 (clean_limit_book traversal cap)

Acceptance criteria

Test plan (issue body)

Additional checks (MR !753 / comments)

Implementation reference

Live gas re-confirm (LocalTerra) — PASS

Results

How to reproduce