Limit book match: bound expired-order scan (MAX_SCAN_STEPS) and tune park cap #254

New issue

Closed

opened 2026-05-31 13:45:26 +00:00 by PlasticDigits · 22 comments

PlasticDigits commented 2026-05-31 13:45:26 +00:00 (Migrated from gitlab.com)

Copy link

Summary

Bound the limit-book match walk so expired/skipped head orders cannot force unbounded ORDERS reads per taker swap, and raise MAX_EXPIRED_PARKS_PER_SWAP modestly so cleanup keeps pace within that budget.

Current codebase

• Book match loops (match_bids / match_asks and read-only simulate_match_*) in smartcontracts/contracts/pair/src/orderbook.rs terminate on:
  • makers_used >= max_maker_fills (clamped to MAX_MAKER_FILLS_HARD_CAP = 256),
  • end of doubly-linked list, or
  • taker budget exhausted.
• Expired orders at the book head: each iteration loads the order; if now >= expires_at:
  • If expired_parks < MAX_EXPIRED_PARKS_PER_SWAP (constant 5 in dex-common::pair), park_expired_limit_order_for_claim runs (unlink + EXPIRED_LIMIT_CLAIMS write + event).
  • Otherwise the order is skipped with a read only (expired_parks_skipped++) and the loop continues.
• Critical gap: expired parks/skips do not increment makers_used. There is no cap on total loop iterations. A long expired prefix at the head can make every hybrid swap load the entire prefix until live orders are reached; only 5 parks clear per tx, so cleanup can lag far behind scan cost (roughly O(N²) reads across swaps draining a backlog).
• Safety valve already exists: unfilled book budget spills to the pool in execute_swap (pool_input_amount = pool_leg + (book_leg - offer_consumed_by_book)), then assert_max_spread — stopping the book walk early does not require revert.
• Observability: swap wasm attrs expired_parks_used, expired_parks_capped, expired_parks_skipped (see contract.rs ~1058–1067).
• Tests: smartcontracts/tests/src/limit_order_tests.rs (expired park cap behavior); orderbook.rs aggregation tests.

Why this is needed

• DoS / gas unpredictability: A taker can pay gas proportional to the number of expired orders at the book head, unbounded by max_maker_fills or frontend hybrid gas estimates (hybridSwapGas.ts keys off maker fills, not skip count).
• Operational: After mass expiry (or indexer downtime), the book leg may become economically unusable until many swaps grind through reads.
• Park cap = 5 was a deliberate write-cost limit (GitLab #250) but does not bound reads; pairing a scan step budget with a moderately higher park cap balances heal rate vs per-tx write/event cost.

Constraints and guardrails

• Do not raise park cap to MAX_MAKER_FILLS_HARD_CAP (256): parking is write-heavy (unlink + claim row + event per order) and would make worst-case swap gas worse, not better.
• Recommended park cap: modest increase (e.g. 10–20) — tune with LocalTerra benchmarks; document in dex-common and docs/limit-orders.md.
• Scan budget: introduce MAX_SCAN_STEPS (or equivalent) in dex-common::pair with a hard ceiling; count every list iteration (fills + parks + skips + zero-remaining continues). Stop when budget exhausted; return partial book consumption (existing pool spillover handles remainder).
• Simulation parity: simulate_match_bids / simulate_match_asks must apply the same step budget so quotes match execute behavior.
• Attributes: extend or reuse attrs so indexers/dApp detect truncation (expired_parks_capped, new scan_steps_capped if needed).
• Paused pairs / claims: unchanged — ClaimExpiredLimitOrder remains separate; parked rows still require maker claim.
• Migration: constant-only change preferred (no storage schema change). If new constant is governance-tunable, follow existing pair config patterns.

Relevant files

Area	Path
Match / park	smartcontracts/contracts/pair/src/orderbook.rs
Swap integration	smartcontracts/contracts/pair/src/contract.rs
Constants	smartcontracts/packages/dex-common/src/pair.rs
Docs	docs/limit-orders.md, docs/contracts-security-audit.md (invariant L8/L10 area)
Agent gas	skills/AGENTS_TERRACLASSIC_GAS.md
Tests	smartcontracts/tests/src/limit_order_tests.rs, smartcontracts/contracts/pair/src/orderbook.rs (aggregation_tests, expiry tests)
Frontend gas (docs only)	frontend-dapp/src/services/terraclassic/hybridSwapGas.ts — note if estimates should include scan cap

Recommended direction

1. Add MAX_SCAN_STEPS (suggest: max_maker_fills + K with fixed K, or min(MAX_MAKER_FILLS_HARD_CAP + K, HARD_CAP_SCAN) — pick one formula, document rationale).
2. Increment scan_steps on every while iteration in match + simulate paths; break when scan_steps >= MAX_SCAN_STEPS (same break conditions as budget exhausted).
3. Raise MAX_EXPIRED_PARKS_PER_SWAP to a reasonable value ≤ scan budget so parks can clear expired prefix within one bounded walk when possible.
4. Ensure BookMatchResult / swap attrs expose when scan cap hit (integrators: book may be partially cleared).
5. Update docs and gas playbook.

Acceptance criteria

• Match and simulate loops cannot iterate more than MAX_SCAN_STEPS times per book side per call.
• With only expired orders at head and max_maker_fills > 0, swap completes without OOG; book consumption ≤ budget; pool leg absorbs remainder under existing spread rules.
• Park cap increased and documented; worst-case parks per swap ≤ new cap.
• Existing expired-park tests updated; new test: N expired > scan budget proves bounded iterations and attrs.
• HybridSimulation book leg matches execute for same params when scan cap binds.

Test plan (functional paths)

Path	Expectation
Empty book	No regression
Live orders only, within max_maker_fills	Same fills as today
Expired prefix ≤ park cap	All parked, head clean
Expired prefix > park cap, within scan budget	Parks capped, skips counted, attrs set
Expired prefix > scan budget	Walk stops; partial park/skip; pool spillover fills remainder
book_input = 0	No book walk
Simulation vs execute	Identical makers_used, offer_consumed, cap flags

Test plan (attack / abuse / hack vectors)

Vector	Mitigation to verify
Griefing: mass cheap expired limits at best price	Scan cap bounds taker gas; maker must still claim/cancel
Taker expects full book liquidity	Quote/sim must reflect scan cap; document partial book
Indexer relies on expired_parks_skipped	Attrs still emitted when scan stops early
Repeated swaps to drain backlog	Gas per swap bounded; total chain work to clear N expired is O(N) not O(N²) reads
Paused pair + expired	No change to pause gates

Verification criteria

• cargo test in smartcontracts/ (limit order + hybrid suites) green.
• Manual or scripted LocalTerra: hybrid swap through expired prefix reports capped attrs and bounded gas_used vs unbounded baseline (benchmark note in issue comment optional).
• Docs/AGENTS_TERRACLASSIC_GAS.md mention MAX_SCAN_STEPS and new park cap.

## Summary Bound the limit-book match walk so expired/skipped head orders cannot force unbounded `ORDERS` reads per taker swap, and raise `MAX_EXPIRED_PARKS_PER_SWAP` modestly so cleanup keeps pace within that budget. ## Current codebase - **Book match loops** (`match_bids` / `match_asks` and read-only `simulate_match_*`) in `smartcontracts/contracts/pair/src/orderbook.rs` terminate on: - `makers_used >= max_maker_fills` (clamped to `MAX_MAKER_FILLS_HARD_CAP` = 256), - end of doubly-linked list, or - taker budget exhausted. - **Expired orders** at the book head: each iteration loads the order; if `now >= expires_at`: - If `expired_parks < MAX_EXPIRED_PARKS_PER_SWAP` (constant **5** in `dex-common::pair`), `park_expired_limit_order_for_claim` runs (unlink + `EXPIRED_LIMIT_CLAIMS` write + event). - Otherwise the order is **skipped with a read only** (`expired_parks_skipped++`) and the loop continues. - **Critical gap:** expired parks/skips do **not** increment `makers_used`. There is **no** cap on total loop iterations. A long expired prefix at the head can make every hybrid swap load the entire prefix until live orders are reached; only 5 parks clear per tx, so cleanup can lag far behind scan cost (roughly O(N²) reads across swaps draining a backlog). - **Safety valve already exists:** unfilled book budget spills to the pool in `execute_swap` (`pool_input_amount = pool_leg + (book_leg - offer_consumed_by_book)`), then `assert_max_spread` — stopping the book walk early does not require revert. - **Observability:** swap wasm attrs `expired_parks_used`, `expired_parks_capped`, `expired_parks_skipped` (see `contract.rs` ~1058–1067). - **Tests:** `smartcontracts/tests/src/limit_order_tests.rs` (expired park cap behavior); `orderbook.rs` aggregation tests. ## Why this is needed - **DoS / gas unpredictability:** A taker can pay gas proportional to the number of expired orders at the book head, unbounded by `max_maker_fills` or frontend hybrid gas estimates (`hybridSwapGas.ts` keys off maker fills, not skip count). - **Operational:** After mass expiry (or indexer downtime), the book leg may become economically unusable until many swaps grind through reads. - **Park cap = 5** was a deliberate write-cost limit (GitLab #250) but does not bound reads; pairing a **scan step budget** with a **moderately higher park cap** balances heal rate vs per-tx write/event cost. ## Constraints and guardrails - **Do not** raise park cap to `MAX_MAKER_FILLS_HARD_CAP` (256): parking is write-heavy (unlink + claim row + event per order) and would make worst-case swap gas worse, not better. - **Recommended park cap:** modest increase (e.g. 10–20) — tune with LocalTerra benchmarks; document in `dex-common` and `docs/limit-orders.md`. - **Scan budget:** introduce `MAX_SCAN_STEPS` (or equivalent) in `dex-common::pair` with a hard ceiling; count **every** list iteration (fills + parks + skips + zero-remaining continues). Stop when budget exhausted; return partial book consumption (existing pool spillover handles remainder). - **Simulation parity:** `simulate_match_bids` / `simulate_match_asks` must apply the **same** step budget so quotes match execute behavior. - **Attributes:** extend or reuse attrs so indexers/dApp detect truncation (`expired_parks_capped`, new `scan_steps_capped` if needed). - **Paused pairs / claims:** unchanged — `ClaimExpiredLimitOrder` remains separate; parked rows still require maker claim. - **Migration:** constant-only change preferred (no storage schema change). If new constant is governance-tunable, follow existing pair config patterns. ## Relevant files | Area | Path | |------|------| | Match / park | `smartcontracts/contracts/pair/src/orderbook.rs` | | Swap integration | `smartcontracts/contracts/pair/src/contract.rs` | | Constants | `smartcontracts/packages/dex-common/src/pair.rs` | | Docs | `docs/limit-orders.md`, `docs/contracts-security-audit.md` (invariant L8/L10 area) | | Agent gas | `skills/AGENTS_TERRACLASSIC_GAS.md` | | Tests | `smartcontracts/tests/src/limit_order_tests.rs`, `smartcontracts/contracts/pair/src/orderbook.rs` (`aggregation_tests`, expiry tests) | | Frontend gas (docs only) | `frontend-dapp/src/services/terraclassic/hybridSwapGas.ts` — note if estimates should include scan cap | ## Recommended direction 1. Add `MAX_SCAN_STEPS` (suggest: `max_maker_fills + K` with fixed `K`, or `min(MAX_MAKER_FILLS_HARD_CAP + K, HARD_CAP_SCAN)` — pick one formula, document rationale). 2. Increment `scan_steps` on every `while` iteration in match + simulate paths; break when `scan_steps >= MAX_SCAN_STEPS` (same break conditions as budget exhausted). 3. Raise `MAX_EXPIRED_PARKS_PER_SWAP` to a **reasonable** value ≤ scan budget so parks can clear expired prefix within one bounded walk when possible. 4. Ensure `BookMatchResult` / swap attrs expose when scan cap hit (integrators: book may be partially cleared). 5. Update docs and gas playbook. ## Acceptance criteria - [ ] Match and simulate loops cannot iterate more than `MAX_SCAN_STEPS` times per book side per call. - [ ] With only expired orders at head and `max_maker_fills > 0`, swap completes without OOG; book consumption ≤ budget; pool leg absorbs remainder under existing spread rules. - [ ] Park cap increased and documented; worst-case parks per swap ≤ new cap. - [ ] Existing expired-park tests updated; new test: N expired > scan budget proves bounded iterations and attrs. - [ ] `HybridSimulation` book leg matches execute for same params when scan cap binds. ## Test plan (functional paths) | Path | Expectation | |------|-------------| | Empty book | No regression | | Live orders only, within `max_maker_fills` | Same fills as today | | Expired prefix ≤ park cap | All parked, head clean | | Expired prefix > park cap, within scan budget | Parks capped, skips counted, attrs set | | Expired prefix > scan budget | Walk stops; partial park/skip; pool spillover fills remainder | | `book_input = 0` | No book walk | | Simulation vs execute | Identical `makers_used`, `offer_consumed`, cap flags | ## Test plan (attack / abuse / hack vectors) | Vector | Mitigation to verify | |--------|---------------------| | Griefing: mass cheap expired limits at best price | Scan cap bounds taker gas; maker must still claim/cancel | | Taker expects full book liquidity | Quote/sim must reflect scan cap; document partial book | | Indexer relies on `expired_parks_skipped` | Attrs still emitted when scan stops early | | Repeated swaps to drain backlog | Gas per swap bounded; total chain work to clear N expired is O(N) not O(N²) reads | | Paused pair + expired | No change to pause gates | ## Verification criteria - `cargo test` in `smartcontracts/` (limit order + hybrid suites) green. - Manual or scripted LocalTerra: hybrid swap through expired prefix reports capped attrs and bounded `gas_used` vs unbounded baseline (benchmark note in issue comment optional). - Docs/`AGENTS_TERRACLASSIC_GAS.md` mention `MAX_SCAN_STEPS` and new park cap.

PlasticDigits commented 2026-05-31 13:45:55 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in issue #257

mentioned in issue #257

PlasticDigits commented 2026-05-31 13:54:54 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in commit 22e825fe22

mentioned in commit 22e825fe22802fb744fb30980e95fe0029d01134

PlasticDigits commented 2026-05-31 13:54:59 +00:00 (Migrated from gitlab.com)

Copy link

Implementation summary (GitLab #254)

Bound hybrid limit-book match walks with a scan-step budget and modestly raised the expired-order park cap.

Changes

• MAX_SCAN_STEPS = 288 (MAX_MAKER_FILLS_HARD_CAP + 32) in dex-common::pair — every book-walk iteration (fills, parks, skips, zero-remaining continues) counts toward this hard ceiling.
• MAX_EXPIRED_PARKS_PER_SWAP raised from 5 → 15 — faster expired-prefix cleanup within the scan budget without write-heavy worst case (still well below 256).
• match_bids / match_asks / simulate_match_* share the same step budget via book_walk_step(); early stop sets scan_steps_capped on BookMatchResult / BookSimulateResult.
• Swap wasm attr: scan_steps_capped=true when the walk truncates (alongside existing expired_parks_* attrs).
• Pool spillover unchanged: unfilled book budget still rolls to the AMM leg; swap completes under existing spread rules.
• Docs / invariants: updated docs/limit-orders.md, docs/contracts-security-audit.md (L5), skills/AGENTS_TERRACLASSIC_GAS.md, skills/AGENTS_FRONTEND_LIMIT_PARKED_EXPIRED.md.

Commit: 22e825f on main.

Verification checklist

• cd smartcontracts && cargo test expired_park — unit tests for park cap + scan cap pass
• cd smartcontracts && cargo test scan_steps — scan-cap + sim/execute parity pass
• cd smartcontracts && cargo test -p cl8y-dex-tests limit_order — full limit-order integration suite green (53 tests)
• Hybrid swap through a long expired prefix emits scan_steps_capped=true and non-zero pool_return_amount (book remainder spilled)
• HybridSimulation book leg matches execute for same params when scan cap binds (simulate_match_bids_scan_steps_cap_matches_execute)
• Indexers/dApps parse new scan_steps_capped swap attr; expired_parks_* still emitted when park cap bites
• Docs cross-links: limit-orders.md § Expiry, contracts-security-audit L5

Follow-ups (optional)

• hybridSwapGas.ts still keys off max_maker_fills, not scan-step count — worst-case expired-prefix taker gas may exceed the dApp quote envelope (documented in AGENTS_TERRACLASSIC_GAS.md rule 16). Tune after LocalTerra benchmark if needed (#252).

---

@qa-agent-team — please verify the checklist above on main @ 22e825f (contracts-only change; pair wasm redeploy required for on-chain effect). Issue left open pending QA sign-off.

## Implementation summary (GitLab #254) Bound hybrid limit-book match walks with a scan-step budget and modestly raised the expired-order park cap. ### Changes - **`MAX_SCAN_STEPS = 288`** (`MAX_MAKER_FILLS_HARD_CAP` + 32) in `dex-common::pair` — every book-walk iteration (fills, parks, skips, zero-remaining continues) counts toward this hard ceiling. - **`MAX_EXPIRED_PARKS_PER_SWAP` raised from 5 → 15** — faster expired-prefix cleanup within the scan budget without write-heavy worst case (still well below 256). - **`match_bids` / `match_asks` / `simulate_match_*`** share the same step budget via `book_walk_step()`; early stop sets `scan_steps_capped` on `BookMatchResult` / `BookSimulateResult`. - **Swap wasm attr:** `scan_steps_capped=true` when the walk truncates (alongside existing `expired_parks_*` attrs). - **Pool spillover unchanged:** unfilled book budget still rolls to the AMM leg; swap completes under existing spread rules. - **Docs / invariants:** updated `docs/limit-orders.md`, `docs/contracts-security-audit.md` (L5), `skills/AGENTS_TERRACLASSIC_GAS.md`, `skills/AGENTS_FRONTEND_LIMIT_PARKED_EXPIRED.md`. **Commit:** `22e825f` on `main`. ### Verification checklist - [ ] `cd smartcontracts && cargo test expired_park` — unit tests for park cap + scan cap pass - [ ] `cd smartcontracts && cargo test scan_steps` — scan-cap + sim/execute parity pass - [ ] `cd smartcontracts && cargo test -p cl8y-dex-tests limit_order` — full limit-order integration suite green (53 tests) - [ ] Hybrid swap through a long expired prefix emits `scan_steps_capped=true` and non-zero `pool_return_amount` (book remainder spilled) - [ ] `HybridSimulation` book leg matches execute for same params when scan cap binds (`simulate_match_bids_scan_steps_cap_matches_execute`) - [ ] Indexers/dApps parse new `scan_steps_capped` swap attr; `expired_parks_*` still emitted when park cap bites - [ ] Docs cross-links: [limit-orders.md § Expiry](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/blob/main/docs/limit-orders.md#expiry-expires_at), [contracts-security-audit L5](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/blob/main/docs/contracts-security-audit.md) ### Follow-ups (optional) - **`hybridSwapGas.ts`** still keys off `max_maker_fills`, not scan-step count — worst-case expired-prefix taker gas may exceed the dApp quote envelope (documented in `AGENTS_TERRACLASSIC_GAS.md` rule 16). Tune after LocalTerra benchmark if needed ([#252](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/252)). --- **@qa-agent-team** — please verify the checklist above on `main` @ `22e825f` (contracts-only change; pair wasm redeploy required for on-chain effect). Issue left open pending QA sign-off.

PlasticDigits commented 2026-05-31 13:55:46 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in issue #260

mentioned in issue #260

PlasticDigits commented 2026-05-31 13:55:47 +00:00 (Migrated from gitlab.com)

Copy link

marked as related to #260

marked as related to #260

PlasticDigits commented 2026-05-31 14:36:34 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in commit 844f27506e

mentioned in commit 844f27506e5880edaa31c3eaadf1957b4b4e8993

Brouie commented 2026-06-01 01:57:55 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in issue #252

mentioned in issue #252

PlasticDigits commented 2026-06-01 02:30:49 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in issue #262

mentioned in issue #262

PlasticDigits commented 2026-06-01 02:30:50 +00:00 (Migrated from gitlab.com)

Copy link

marked as related to #262

marked as related to #262

PlasticDigits commented 2026-06-01 02:30:56 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in issue #263

mentioned in issue #263

PlasticDigits commented 2026-06-01 02:30:59 +00:00 (Migrated from gitlab.com)

Copy link

marked as related to #263

marked as related to #263

Brouie commented 2026-06-01 02:36:24 +00:00 (Migrated from gitlab.com)

Copy link

Verified #254 on main @ 6b22feb (redeployed + pair wasm live on LocalTerra, indexer restarted). Source + named tests + two live on-chain runs. All acceptance criteria covered.

AC1 — match + simulate loops bounded by MAX_SCAN_STEPS/side/call:
Source: book_walk_step is called at the top of every iteration in all four loops (match_bids/match_asks + both simulate_match_*), counting fills, parks, skips, and zero-remaining continues alike; returns false at MAX_SCAN_STEPS=288 (256+32) → sets scan_steps_capped and breaks. Tests match_bids_scan_steps_cap_bounds_expired_prefix_walk + hybrid_walk_scan_steps_cap_bounds_expired_prefix_and_spills_to_pool. Live: 300 expired bids in one prefix → taker swap walked exactly 288 (15 parked + 273 skipped), scan_steps_capped=true, code=0, gas_used=1,427,267 (bounded), 12 orders never reached.

AC2 — expired-only head completes, no OOG, book ≤ budget, pool absorbs remainder:
Both live runs: swap code=0, limit_book_offer_consumed=0, book_return_amount=0, and the offer spilled to the pool leg (pool_return_amount 279,009,033 in the 20-order run / 277,316,610 in the 300-order run) under existing spread rules. Test hybrid_walk_scan_steps_cap_..._spills_to_pool.

AC3 — park cap raised + documented, worst-case parks ≤ cap:
Source: MAX_EXPIRED_PARKS_PER_SWAP 5 → 15, park-then-skip logic. Docs: invariant L5 (contracts-security-audit.md), AGENTS_TERRACLASSIC_GAS.md rule 16, limit-orders.md (park cap / scan budget / attrs). Tests {match_bids,match_asks}_parks_at_most_max_expired_parks_per_swap, hybrid_walk_twenty_expired_asks_parks_cap_skips_remainder. Live: 20-order run parked exactly 15, expired_parks_capped=true, expired_parks_skipped=5.

AC4 — existing park tests updated + new N>scan-budget test proving bounded iterations + attrs:
match_bids_scan_steps_cap_bounds_expired_prefix_walk (N = MAX_SCAN_STEPS+50, asserts the iteration bound) + the integration spill test. Green on 6b22feb.

AC5 — HybridSimulation book leg matches execute when cap binds:
Source: simulate paths use the same book_walk_step budget. Tests simulate_match_bids_scan_steps_cap_matches_execute + hybrid_simulation_matches_execute_with_expired_park_cap. (Test-verified; not separately exercised as a live sim-vs-execute query.)

cargo test (6b22feb): the #254 suite is green — orderbook::expired_park_cap_tests::{match_bids_parks_at_most…, match_asks_parks_at_most…, match_bids_parks_three_expired_then_fills_behind, match_bids_scan_steps_cap_bounds_expired_prefix_walk, simulate_match_bids_scan_steps_cap_matches_execute} and integration hybrid_walk_*, hybrid_simulation_matches_execute_with_expired_park_cap, skipped_expired_bid_cancelable_by_maker, expired_bid_parked_on_hybrid_walk_claim_refunds_maker. Full contract suite 378 passed / 0 failed.

Two layer items flagged (not blockers):

• No before/after gas baseline: there's no pre-#254 (unbounded) build deployed, so I can't show a numeric before→after delta — same structural gap as #252. The bound is proven structurally + by the iteration-asserting unit test + the live 288-cap run; the live worst-case gas (1.43M for a full 288-step walk) is bounded but not contrasted against an unbounded build.
• Attr consumption: swap attrs (scan_steps_capped, expired_parks_*) are confirmed emitted live, but whether the indexer/dApp parse them is a separate layer (indexer source / frontend on laptop) — not checked here.

@PlasticDigits — verified and signed off from my side, no issues found; over to you to close.

Verified #254 on `main` @ `6b22feb` (redeployed + pair wasm live on LocalTerra, indexer restarted). Source + named tests + two live on-chain runs. All acceptance criteria covered. **AC1 — match + simulate loops bounded by MAX_SCAN_STEPS/side/call:** Source: `book_walk_step` is called at the top of every iteration in all four loops (`match_bids`/`match_asks` + both `simulate_match_*`), counting fills, parks, skips, and zero-remaining continues alike; returns false at `MAX_SCAN_STEPS=288` (256+32) → sets `scan_steps_capped` and breaks. Tests `match_bids_scan_steps_cap_bounds_expired_prefix_walk` + `hybrid_walk_scan_steps_cap_bounds_expired_prefix_and_spills_to_pool`. **Live:** 300 expired bids in one prefix → taker swap walked exactly 288 (15 parked + 273 skipped), `scan_steps_capped=true`, code=0, `gas_used=1,427,267` (bounded), 12 orders never reached. **AC2 — expired-only head completes, no OOG, book ≤ budget, pool absorbs remainder:** Both live runs: swap `code=0`, `limit_book_offer_consumed=0`, `book_return_amount=0`, and the offer spilled to the pool leg (`pool_return_amount` 279,009,033 in the 20-order run / 277,316,610 in the 300-order run) under existing spread rules. Test `hybrid_walk_scan_steps_cap_..._spills_to_pool`. **AC3 — park cap raised + documented, worst-case parks ≤ cap:** Source: `MAX_EXPIRED_PARKS_PER_SWAP` 5 → 15, park-then-skip logic. Docs: invariant L5 (`contracts-security-audit.md`), `AGENTS_TERRACLASSIC_GAS.md` rule 16, `limit-orders.md` (park cap / scan budget / attrs). Tests `{match_bids,match_asks}_parks_at_most_max_expired_parks_per_swap`, `hybrid_walk_twenty_expired_asks_parks_cap_skips_remainder`. **Live:** 20-order run parked exactly 15, `expired_parks_capped=true`, `expired_parks_skipped=5`. **AC4 — existing park tests updated + new N>scan-budget test proving bounded iterations + attrs:** `match_bids_scan_steps_cap_bounds_expired_prefix_walk` (N = MAX_SCAN_STEPS+50, asserts the iteration bound) + the integration spill test. Green on 6b22feb. **AC5 — HybridSimulation book leg matches execute when cap binds:** Source: simulate paths use the same `book_walk_step` budget. Tests `simulate_match_bids_scan_steps_cap_matches_execute` + `hybrid_simulation_matches_execute_with_expired_park_cap`. (Test-verified; not separately exercised as a live sim-vs-execute query.) **cargo test (6b22feb):** the #254 suite is green — `orderbook::expired_park_cap_tests::{match_bids_parks_at_most…, match_asks_parks_at_most…, match_bids_parks_three_expired_then_fills_behind, match_bids_scan_steps_cap_bounds_expired_prefix_walk, simulate_match_bids_scan_steps_cap_matches_execute}` and integration `hybrid_walk_*`, `hybrid_simulation_matches_execute_with_expired_park_cap`, `skipped_expired_bid_cancelable_by_maker`, `expired_bid_parked_on_hybrid_walk_claim_refunds_maker`. Full contract suite 378 passed / 0 failed. **Two layer items flagged (not blockers):** - No before/after gas baseline: there's no pre-#254 (unbounded) build deployed, so I can't show a numeric before→after delta — same structural gap as #252. The bound is proven structurally + by the iteration-asserting unit test + the live 288-cap run; the live worst-case gas (1.43M for a full 288-step walk) is bounded but not contrasted against an unbounded build. - Attr consumption: swap attrs (`scan_steps_capped`, `expired_parks_*`) are confirmed emitted live, but whether the indexer/dApp parse them is a separate layer (indexer source / frontend on laptop) — not checked here. @PlasticDigits — verified and signed off from my side, no issues found; over to you to close.

PlasticDigits commented 2026-06-01 02:56:19 +00:00 (Migrated from gitlab.com)

Copy link

@Brouie Must check indexer parsing of swap attrs - way to verify is sql query on db

@Brouie Must check indexer parsing of swap attrs - way to verify is sql query on db

Brouie commented 2026-06-01 03:20:46 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in issue #255

mentioned in issue #255

Brouie commented 2026-06-01 03:44:37 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in issue #256

mentioned in issue #256

Brouie commented 2026-06-01 05:03:32 +00:00 (Migrated from gitlab.com)

Copy link

Checked the indexer parsing of the swap attrs via SQL on dex_indexer, per your ask. Answer to the question first:

PASS — a #254 cap-path swap parses correctly into swap_events.

Used the scan-cap swap from my #254 run (swap_events id 322 — on-chain it carries scan_steps_capped=true, expired_parks_used=15, expired_parks_capped=true, expired_parks_skipped=273). DB row vs on-chain, exact across the board:

field	on-chain	DB
offer_amount	300,000,000	300,000,000
return_amount	277,316,610	277,316,610
spread_amount	844,397	844,397
commission_amount	249,809	249,809
effective_fee_bps	9	9
pool_return_amount	277,316,610	277,316,610
book_return_amount	0	0
limit_book_offer_consumed	0	0
sender / receiver	test1 / test1	test1 / test1
offer / ask asset	T0 / T1	T0 / T1

The book-fill fields populate too — a fill-bearing hybrid swap (id 324) lands book_return_amount=19,810,080, limit_book_offer_consumed=11,658,820, matching on-chain.

Indexer was caught up when I checked: indexer_state.last_indexed_height=3,628,720 (≥ node tip), indexer_failed_blocks=0.

On the cap attrs themselves: the indexer doesn't track them — there are no swap_events columns for expired_parks_* / scan_steps_capped, and the parser reads only the standard keys, each looked up by name (wasm_attr_last(attrs, key)) and parsed independently. So the cap attrs are simply never read, and because parsing is key-based they can't drop or corrupt the swap row. By design for the swap row, and harmless — confirmed on the heaviest cap path (273 skipped) above.

---

Separate flag — NOT a #254 blocker, pre-existing, found while doing this:

limit_order_fills (the per-maker fill rows) comes up empty for hybrid swaps that fill multiple makers — even though the fills happen on-chain (id 323 = 5 limit_order_fill events, id 324 = 20). The parser + dispatch + insert are all wired and no blocks failed, so it's silently parsing zero fills.

Root cause (confirmed on id 324): the chain merges the contract's per-fill wasm events — the 20 limit_order_fill attributes collapse into ~5 grouped wasm events, and the last action in each merged group isn't limit_order_fill (it's swap / transfer). parse_limit_order_fills gates per-event on wasm_attr_last(attrs,"action") == "limit_order_fill", so it never matches → 0 fills parsed. (Even if a group did end in limit_order_fill, wasm_attr_last on order_id/maker would only capture the last fill, not all of them.)

The swap row is unaffected — its action=swap lands last in its own group, so swap_events parses fine (hence the PASS above). This looks like it predates the gas wave (the fill parser/event aren't new), so it's its own indexer issue, not part of #254.

@PlasticDigits — #254 indexer parsing checks out, good to close from my side. Want me to file the limit_order_fills per-maker gap as its own indexer issue with this root-cause, or will you take it?

Checked the indexer parsing of the swap attrs via SQL on `dex_indexer`, per your ask. Answer to the question first: **PASS — a #254 cap-path swap parses correctly into `swap_events`.** Used the scan-cap swap from my #254 run (`swap_events` id 322 — on-chain it carries `scan_steps_capped=true`, `expired_parks_used=15`, `expired_parks_capped=true`, `expired_parks_skipped=273`). DB row vs on-chain, exact across the board: | field | on-chain | DB | |---|---|---| | offer_amount | 300,000,000 | 300,000,000 | | return_amount | 277,316,610 | 277,316,610 | | spread_amount | 844,397 | 844,397 | | commission_amount | 249,809 | 249,809 | | effective_fee_bps | 9 | 9 | | pool_return_amount | 277,316,610 | 277,316,610 | | book_return_amount | 0 | 0 | | limit_book_offer_consumed | 0 | 0 | | sender / receiver | test1 / test1 | test1 / test1 | | offer / ask asset | T0 / T1 | T0 / T1 | The book-fill fields populate too — a fill-bearing hybrid swap (id 324) lands `book_return_amount=19,810,080`, `limit_book_offer_consumed=11,658,820`, matching on-chain. Indexer was caught up when I checked: `indexer_state.last_indexed_height=3,628,720` (≥ node tip), `indexer_failed_blocks=0`. On the cap attrs themselves: the indexer doesn't track them — there are no `swap_events` columns for `expired_parks_*` / `scan_steps_capped`, and the parser reads only the standard keys, each looked up **by name** (`wasm_attr_last(attrs, key)`) and parsed independently. So the cap attrs are simply never read, and because parsing is key-based they can't drop or corrupt the swap row. By design for the swap row, and harmless — confirmed on the heaviest cap path (273 skipped) above. --- **Separate flag — NOT a #254 blocker, pre-existing, found while doing this:** `limit_order_fills` (the per-maker fill rows) comes up **empty** for hybrid swaps that fill multiple makers — even though the fills happen on-chain (id 323 = 5 `limit_order_fill` events, id 324 = 20). The parser + dispatch + insert are all wired and no blocks failed, so it's silently parsing zero fills. Root cause (confirmed on id 324): the chain **merges** the contract's per-fill `wasm` events — the 20 `limit_order_fill` attributes collapse into ~5 grouped `wasm` events, and the *last* `action` in each merged group isn't `limit_order_fill` (it's `swap` / `transfer`). `parse_limit_order_fills` gates per-event on `wasm_attr_last(attrs,"action") == "limit_order_fill"`, so it never matches → 0 fills parsed. (Even if a group did end in `limit_order_fill`, `wasm_attr_last` on `order_id`/`maker` would only capture the *last* fill, not all of them.) The swap row is unaffected — its `action=swap` lands last in its own group, so `swap_events` parses fine (hence the PASS above). This looks like it predates the gas wave (the fill parser/event aren't new), so it's its own indexer issue, not part of #254. @PlasticDigits — #254 indexer parsing checks out, good to close from my side. Want me to file the `limit_order_fills` per-maker gap as its own indexer issue with this root-cause, or will you take it?

Brouie commented 2026-06-01 05:25:47 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in issue #258

mentioned in issue #258

PlasticDigits commented 2026-06-01 05:31:24 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in issue #269

mentioned in issue #269

PlasticDigits commented 2026-06-01 05:31:24 +00:00 (Migrated from gitlab.com)

Copy link

Closing #254 — contract scan-cap work verified on main. Per-maker limit_order_fills indexing gap tracked separately: #269.

Closing #254 — contract scan-cap work verified on main. Per-maker `limit_order_fills` indexing gap tracked separately: #269.

PlasticDigits (Migrated from gitlab.com) closed this issue 2026-06-01 05:31:25 +00:00

Brouie commented 2026-06-04 06:30:07 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in issue #289

mentioned in issue #289

PlasticDigits commented 2026-06-05 04:08:27 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in issue #309

mentioned in issue #309

PlasticDigits commented 2026-08-30 05:24:17 +00:00 (Migrated from gitlab.com)

Copy link

mentioned in issue #708

mentioned in issue #708

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

main

renovate/tokio-util-0.x-lockfile

renovate/thiserror-2.x-lockfile

renovate/serde_json-1.x-lockfile

renovate/serde-monorepo

renovate/libc-0.x-lockfile

renovate/chrono-0.x-lockfile

cac-design-issue-1206

cac-design-issue-1317

cac-design-issue-1316

cac-design-issue-1311

cac-design-issue-1300

cac-design-issue-1306

cac-design-issue-1305

cac-design-issue-1302

cac-design-issue-1277

No results found.

Labels

Clear labels   

agent:fix_bugfix

  

agent:fix_conflicts

  

agent:fix_security

  

agent:gap_analysis

  

agent:implement

  

agent:implement

Queue implement gate

  

agent:implement

Queue implement gate

  

agent:open_issues

  

agent:ready

  

agent:research

  

agent:security_audit

  

agent:verify

  

architecture

Architecture

  

backend

API and services

  

blocker:hybrid

Blocks hybrid launch

  

blocker:launch

Blocks launch

  

blocker:limit-orders

Blocks limits launch

  

blocker:v2

Blocks v2

  

block:log_only

  

block:security

  

bug

  

ci

  

contracts

Smart contracts

  

correctness

  

deploy

  

dev

  

devops

  

docs

  

documentation

  

duplicate

  

e2e

  

enhancement

  

epic

Epic

  

feature

  

frontend

  

functional-completion

Functional

  

gas

  

good first issue

  

governance

  

help wanted

  

high-risk

  

hooks

  

hybrid

Hybrid swaps

  

indexer

Indexer

  

infra

CI/CD

  

infrastructure

  

integrators

  

invalid

  

launch-blocker

  

limit-orders

Limit orders

  

localnet

  

localterra

  

low priority

  

missing-implementation

Implementation

  

needs-design

Design decision required

  

ops

  

performance

  

priority

high

  

priority

medium

  

product

  

qa

  

QA

  

question

  

ready

Definition of Ready

  

ready

Definition of Ready

  

research

  

scripts

  

security

Security

  

security-hardening

Security hardening

  

smartcontracts

  

tech-debt

  

testing

Testing

  

ux

  

UX

  

v2

Pool-only swaps

  

verification

  

wontfix

No labels

agent:fix_bugfix

agent:fix_conflicts

agent:fix_security

agent:gap_analysis

agent:implement

agent:implement

agent:implement

agent:open_issues

agent:ready

agent:research

agent:security_audit

agent:verify

architecture

backend

blocker:hybrid

blocker:launch

blocker:limit-orders

blocker:v2

block:log_only

block:security

bug

ci

contracts

correctness

deploy

dev

devops

docs

documentation

duplicate

e2e

enhancement

epic

feature

frontend

functional-completion

gas

good first issue

governance

help wanted

high-risk

hooks

hybrid

indexer

infra

infrastructure

integrators

invalid

launch-blocker

limit-orders

localnet

localterra

low priority

missing-implementation

needs-design

ops

performance

priority

high

priority

medium

product

qa

QA

question

ready

ready

research

scripts

security

security-hardening

smartcontracts

tech-debt

testing

ux

UX

v2

verification

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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#254

No description provided.