Cap expired limit order parking during hybrid match walks (max 5 per swap) #250

New issue

Closed

opened 2026-05-31 12:21:52 +00:00 by PlasticDigits · 8 comments

PlasticDigits commented

2026-05-31 12:21:52 +00:00

(Migrated from gitlab.com)

Summary

Stop takers from unbounded gas spend cleaning up expired resting orders during match_bids / match_asks. Cap park_expired_limit_order_for_claim calls to 5 per swap; beyond the cap, skip expired orders without parking (leave for maker claim via separate path or future keeper).

Current codebase

Hybrid swap book walk (orderbook.rs ~632–645 bids, ~811–824 asks): when order.expires_at <= now, calls park_expired_limit_order_for_claim:
- unlink_order (DLL repair + storage remove)
- EXPIRED_LIMIT_CLAIMS.save (new map row)
- PENDING_ESCROW_* unchanged until maker claims
- Emits limit_order_expired_parked event
Taker pays gas for each expired order encountered at the top of the book before reaching fillable liquidity.
No cap: A book head stacked with expired orders can make hybrid swaps prohibitively expensive (griefing / neglect vector).
max_maker_fills caps profitable fills but does not cap expiry parks (expired orders do not increment makers_used).

Why this is needed

Expired orders are the maker’s responsibility to cancel/claim. Requiring every taker to subsidize unbounded expiry cleanup is a gas griefing surface: attacker (or inactive MM) leaves many expired limits at best prices → every hybrid swap pays linear park cost before matching real liquidity.

Constraints / guardrails

Cap value: 5 parks per match_bids / match_asks invocation (constant in dex-common or orderbook.rs, e.g. MAX_EXPIRED_PARKS_PER_SWAP = 5).
Behavior after cap: Stop parking; advance cur = next_ptr without storage write for additional expired orders (they remain on book until another tx parks them or maker cancels). Document this explicitly — trade-off vs leaving stale head.
- Alternative (if product rejects skip): revert when cap exceeded — stricter but bad UX; recommended: skip without park per feature request.
Fill cap unchanged: max_maker_fills still limits profitable matches only.
Indexer: Skipped expired orders may remain visible until parked elsewhere; document lifecycle edge case.
Pause / claim paths: Unaffected; makers can still CancelLimitOrder or get parked on a later swap.
Simulation parity: Update simulate_match_bids / simulate_match_asks to mirror skip logic (today sim skips expired without parking already — align execute with sim where possible).

Relevant files

File	Role
`smartcontracts/contracts/pair/src/orderbook.rs`	`match_bids`, `match_asks`, `park_expired_limit_order_for_claim`
`smartcontracts/packages/dex-common/src/pair.rs`	Export cap constant
`smartcontracts/contracts/pair/src/error.rs`	Optional new error if revert-on-cap chosen
`smartcontracts/tests/src/limit_order_tests.rs`	Hybrid + expiry tests
`indexer/src/indexer/parser.rs`	Expired park events
`docs/limit-orders.md`, `docs/contracts-security-audit.md`	Expiry semantics

Acceptance criteria

Single swap parks at most 5 expired orders per book side walked.
6th+ expired order at head is skipped without EXPIRED_LIMIT_CLAIMS write (per skip design).
Fillable orders behind expired stack still reachable within same swap (after skips/parks).
Gas for swap with 10 expired head orders bounded (document vs uncapped baseline).
No escrow invariant violation (skipped orders still counted in PENDING_ESCROW_*).

Test plan — functional paths

0 expired: unchanged behavior.
3 expired then fillable: all 3 parked, fill proceeds.
5 expired: all parked.
10 expired, no fillable behind: parks 5, skips 5, tx succeeds (may consume less offer budget — document).
10 expired, fillable on 11th: parks 5, skips until fillable or budget exhausted.
Ask side symmetric tests.
Simulation query consistent with execute for return amounts when skips occur.

Test plan — attack / abuse vectors

Griefing stack: Attacker places many expired limits at best price → taker swap gas bounded; attacker escrow locked until self-cancel (no free drain).
Skip stale head: Verify skipped orders still owned by maker; cannot double-claim without park row.
Maker neglect: Maker can still CancelLimitOrder on skipped expired order on book.
DoS book head: After cap, book still traversable; not infinite loop on same expired node.

Verification criteria

make test-contracts with new expiry-cap tests green.
Gas benchmark: 10-expired-head swap gas ≤ cap × (park cost) + fill cost.
docs/limit-orders.md § Expiry updated with cap + skip semantics.

## Summary Stop takers from unbounded gas spend **cleaning up expired resting orders** during `match_bids` / `match_asks`. Cap **`park_expired_limit_order_for_claim`** calls to **5 per swap**; beyond the cap, **skip** expired orders without parking (leave for maker claim via separate path or future keeper). ## Current codebase - Hybrid swap book walk (`orderbook.rs` ~632–645 bids, ~811–824 asks): when `order.expires_at <= now`, calls **`park_expired_limit_order_for_claim`**: - `unlink_order` (DLL repair + storage remove) - **`EXPIRED_LIMIT_CLAIMS.save`** (new map row) - **`PENDING_ESCROW_*` unchanged** until maker claims - Emits `limit_order_expired_parked` event - Taker pays gas for **each** expired order encountered at the top of the book before reaching fillable liquidity. - **No cap:** A book head stacked with expired orders can make hybrid swaps prohibitively expensive (griefing / neglect vector). - **`max_maker_fills`** caps profitable fills but **does not cap** expiry parks (expired orders do not increment `makers_used`). ## Why this is needed Expired orders are the **maker’s responsibility** to cancel/claim. Requiring every taker to subsidize unbounded expiry cleanup is a **gas griefing surface**: attacker (or inactive MM) leaves many expired limits at best prices → every hybrid swap pays linear park cost before matching real liquidity. ## Constraints / guardrails - **Cap value:** **5** parks per `match_bids` / `match_asks` invocation (constant in `dex-common` or `orderbook.rs`, e.g. `MAX_EXPIRED_PARKS_PER_SWAP = 5`). - **Behavior after cap:** Stop parking; **advance** `cur = next_ptr` without storage write for additional expired orders (they remain on book until another tx parks them or maker cancels). Document this explicitly — trade-off vs leaving stale head. - *Alternative (if product rejects skip):* revert when cap exceeded — stricter but bad UX; **recommended: skip without park** per feature request. - **Fill cap unchanged:** `max_maker_fills` still limits profitable matches only. - **Indexer:** Skipped expired orders may remain visible until parked elsewhere; document lifecycle edge case. - **Pause / claim paths:** Unaffected; makers can still `CancelLimitOrder` or get parked on a later swap. - **Simulation parity:** Update `simulate_match_bids` / `simulate_match_asks` to mirror skip logic (today sim skips expired without parking already — align execute with sim where possible). ## Relevant files | File | Role | |------|------| | `smartcontracts/contracts/pair/src/orderbook.rs` | `match_bids`, `match_asks`, `park_expired_limit_order_for_claim` | | `smartcontracts/packages/dex-common/src/pair.rs` | Export cap constant | | `smartcontracts/contracts/pair/src/error.rs` | Optional new error if revert-on-cap chosen | | `smartcontracts/tests/src/limit_order_tests.rs` | Hybrid + expiry tests | | `indexer/src/indexer/parser.rs` | Expired park events | | `docs/limit-orders.md`, `docs/contracts-security-audit.md` | Expiry semantics | ## Recommended solution direction 1. Add `MAX_EXPIRED_PARKS_PER_SWAP: u32 = 5` in `dex-common`. 2. In `match_bids` / `match_asks`, track `expired_parks: u32`; on expired order: - if `expired_parks < MAX`: park as today, increment counter. - else: `cur = next_ptr; continue` (no storage mutation). 3. Add wasm attr on swap when cap hit: `expired_parks_capped=true`, `expired_parks_skipped=N` (optional, for indexers). 4. Integration test: 10 expired bids at head; swap parks ≤5, matches behind them if any, completes without OOG. ## Acceptance criteria - [ ] Single swap parks at most **5** expired orders per book side walked. - [ ] 6th+ expired order at head is skipped without `EXPIRED_LIMIT_CLAIMS` write (per skip design). - [ ] Fillable orders behind expired stack still reachable within same swap (after skips/parks). - [ ] Gas for swap with 10 expired head orders bounded (document vs uncapped baseline). - [ ] No escrow invariant violation (skipped orders still counted in `PENDING_ESCROW_*`). ## Test plan — functional paths - [ ] 0 expired: unchanged behavior. - [ ] 3 expired then fillable: all 3 parked, fill proceeds. - [ ] 5 expired: all parked. - [ ] 10 expired, no fillable behind: parks 5, skips 5, tx succeeds (may consume less offer budget — document). - [ ] 10 expired, fillable on 11th: parks 5, skips until fillable or budget exhausted. - [ ] Ask side symmetric tests. - [ ] Simulation query consistent with execute for return amounts when skips occur. ## Test plan — attack / abuse vectors - [ ] **Griefing stack:** Attacker places many expired limits at best price → taker swap gas bounded; attacker escrow locked until self-cancel (no free drain). - [ ] **Skip stale head:** Verify skipped orders still owned by maker; cannot double-claim without park row. - [ ] **Maker neglect:** Maker can still `CancelLimitOrder` on skipped expired order on book. - [ ] **DoS book head:** After cap, book still traversable; not infinite loop on same expired node. ## Verification criteria - [ ] `make test-contracts` with new expiry-cap tests green. - [ ] Gas benchmark: 10-expired-head swap gas ≤ cap × (park cost) + fill cost. - [ ] `docs/limit-orders.md` § Expiry updated with cap + skip semantics.

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-05-31 13:19:19 +00:00

PlasticDigits commented

2026-05-31 13:19:23 +00:00

(Migrated from gitlab.com)

Implementation summary (merged to `main` @ `ff7a680`)

Capped expired limit order parking during hybrid match_bids / match_asks walks to bound taker gas when many expired limits stack at the book head.

Contract changes

Added MAX_EXPIRED_PARKS_PER_SWAP = 5 in dex-common.
match_bids / match_asks track expired_parks / expired_parks_skipped:
- ≤ 5 expired head orders: park via existing park_expired_limit_order_for_claim (unchanged semantics).
- 6th+ expired head order: skip without storage write (cur = next_ptr; continue).
Swap attrs when book leg runs: expired_parks_used, expired_parks_capped=true, expired_parks_skipped (when applicable).
max_maker_fills unchanged; expired parks do not increment makers_used.
PENDING_ESCROW_* unchanged for skipped orders; makers can still CancelLimitOrder on skipped expired rows.

Docs / invariants

Updated docs/limit-orders.md § Expiry + tx attrs table.
Extended invariant L5 in docs/contracts-security-audit.md.
Cross-linked in skills/AGENTS_TERRACLASSIC_GAS.md and skills/AGENTS_FRONTEND_LIMIT_PARKED_EXPIRED.md.

Tests added

Unit: orderbook::expired_park_cap_tests::*
Integration: hybrid_walk_parks_at_most_five_expired_bids_per_swap, hybrid_walk_three_expired_bids_all_parked_then_fills_live_bid, hybrid_walk_ten_expired_asks_parks_five_skips_five, skipped_expired_bid_cancelable_by_maker, hybrid_simulation_matches_execute_with_expired_park_cap

Verification checklist

make test-contracts green (or cargo test -p cl8y-dex-pair expired_park_cap + cargo test -p cl8y-dex-tests hybrid_walk skipped_expired hybrid_simulation_matches_execute_with_expired)
Single hybrid swap parks ≤ 5 limit_order_expired_parked events per book side walked
6th+ expired head order has no ExpiredLimitRefund row until a later park/claim path
Fillable order behind a 10-expired stack still matches in the same swap
Skipped expired order remains on book; maker CancelLimitOrder succeeds
HybridSimulation return amount matches execute when fillable liquidity exists behind expired stack
Swap attrs expired_parks_used / expired_parks_skipped present when cap bites
Indexer book APIs may still show skipped expired head until parked — expected per docs

Follow-ups (optional)

LocalTerra gas benchmark for 10-expired-head swap vs uncapped baseline (#252 family) if tuning HYBRID_SWAP_* constants is desired.

@qa-agent-team — please verify on LocalTerra or CI: run the checklist above against main after pair wasm rebuild/deploy. Leave this issue open until QA signs off.

## Implementation summary (merged to `main` @ ff7a680) Capped expired limit order parking during hybrid `match_bids` / `match_asks` walks to bound taker gas when many expired limits stack at the book head. ### Contract changes - Added `MAX_EXPIRED_PARKS_PER_SWAP = 5` in `dex-common`. - `match_bids` / `match_asks` track `expired_parks` / `expired_parks_skipped`: - **≤ 5 expired head orders:** park via existing `park_expired_limit_order_for_claim` (unchanged semantics). - **6th+ expired head order:** skip without storage write (`cur = next_ptr; continue`). - Swap attrs when book leg runs: `expired_parks_used`, `expired_parks_capped=true`, `expired_parks_skipped` (when applicable). - `max_maker_fills` unchanged; expired parks do not increment `makers_used`. - `PENDING_ESCROW_*` unchanged for skipped orders; makers can still `CancelLimitOrder` on skipped expired rows. ### Docs / invariants - Updated `docs/limit-orders.md` § Expiry + tx attrs table. - Extended invariant **L5** in `docs/contracts-security-audit.md`. - Cross-linked in `skills/AGENTS_TERRACLASSIC_GAS.md` and `skills/AGENTS_FRONTEND_LIMIT_PARKED_EXPIRED.md`. ### Tests added - Unit: `orderbook::expired_park_cap_tests::*` - Integration: `hybrid_walk_parks_at_most_five_expired_bids_per_swap`, `hybrid_walk_three_expired_bids_all_parked_then_fills_live_bid`, `hybrid_walk_ten_expired_asks_parks_five_skips_five`, `skipped_expired_bid_cancelable_by_maker`, `hybrid_simulation_matches_execute_with_expired_park_cap` --- ## Verification checklist - [ ] `make test-contracts` green (or `cargo test -p cl8y-dex-pair expired_park_cap` + `cargo test -p cl8y-dex-tests hybrid_walk skipped_expired hybrid_simulation_matches_execute_with_expired`) - [ ] Single hybrid swap parks **≤ 5** `limit_order_expired_parked` events per book side walked - [ ] 6th+ expired head order has **no** `ExpiredLimitRefund` row until a later park/claim path - [ ] Fillable order behind a 10-expired stack still matches in the same swap - [ ] Skipped expired order remains on book; maker `CancelLimitOrder` succeeds - [ ] `HybridSimulation` return amount matches execute when fillable liquidity exists behind expired stack - [ ] Swap attrs `expired_parks_used` / `expired_parks_skipped` present when cap bites - [ ] Indexer book APIs may still show skipped expired head until parked — expected per docs ### Follow-ups (optional) - LocalTerra gas benchmark for 10-expired-head swap vs uncapped baseline ([#252](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/252) family) if tuning `HYBRID_SWAP_*` constants is desired. --- **@qa-agent-team** — please verify on LocalTerra or CI: run the checklist above against `main` after pair wasm rebuild/deploy. Leave this issue open until QA signs off.

PlasticDigits commented

2026-05-31 13:26:13 +00:00

(Migrated from gitlab.com)

mentioned in issue #252