Limit book match: batch pending-escrow storage updates per swap #255

Closed
opened 2026-05-31 13:45:39 +00:00 by PlasticDigits · 10 comments
PlasticDigits commented 2026-05-31 13:45:39 +00:00 (Migrated from gitlab.com)

Summary

Reduce swap gas by accumulating pending-escrow deltas during limit-book matching and persisting PENDING_ESCROW_TOKEN0 / PENDING_ESCROW_TOKEN1 once per side per match, instead of load+save on every fill.

Current codebase

  • Escrow totals PENDING_ESCROW_TOKEN0 / PENDING_ESCROW_TOKEN1 (smartcontracts/contracts/pair/src/state.rs) track CW20 held for resting limits, excluded from AMM reserves.
  • On each bid fill (taker sells token0): escrow_sub_pending_token1 subtracts cost (token1) — load, subtract, save per fill (orderbook.rs).
  • On each ask fill: escrow_sub_pending_token0 subtracts fill_t0 — same pattern.
  • Placement/batch placement adds escrow in aggregated writes (already efficient in limit_placement.rs).
  • Match loops can run up to max_maker_fills (cap 256) fills per swap.

Why this is needed

  • Each fill causes 2 storage ops on the pending escrow Item (read + write). At high max_maker_fills, this is pure overhead: the match loop is atomic; nothing reads pending escrow mid-loop.
  • Complements transfer aggregation (GitLab #248); this is in-pair storage optimization with no semantic change.

Constraints and guardrails

  • Invariant: Final pending escrow after match must equal today’s sequential subtracts (same arithmetic, single commit).
  • Underflow: Preserve existing underflow errors (InvariantViolation / escrow underflow messages); batch subtract must fail identically if total consumption exceeds pending.
  • Scope: Only refactor match_bids / match_asks execute paths; simulate_match_* does not touch escrow (no change).
  • No change to placement, cancel, claim, or park paths unless they share helpers — keep diffs minimal.
  • Migration: none.

Relevant files

Area Path
Escrow helpers smartcontracts/contracts/pair/src/orderbook.rs (escrow_sub_pending_token0/1, match_bids, match_asks)
State smartcontracts/contracts/pair/src/state.rs
Tests smartcontracts/contracts/pair/src/orderbook.rs (aggregation_tests), smartcontracts/tests/src/limit_order_tests.rs
  1. In match_bids / match_asks, accumulate token1_escrow_delta / token0_escrow_delta in locals during the loop.
  2. After loop (or on early break), call a single apply_escrow_delta_token1(-delta) helper that loads once, subtracts total, saves once (or skip save if delta zero).
  3. Optionally share helper with existing escrow_sub_pending_* for cancel/placement consistency.
  4. Add unit test: multiple fills in one match → one observable escrow change equal to sum of per-fill subtracts.

Acceptance criteria

  • Multi-fill match_bids / match_asks perform at most one pending-escrow write per token side per invocation.
  • Escrow balances and underflow behavior match pre-change for all existing tests.
  • No change to simulation query results.

Test plan (functional paths)

Path Expectation
Single fill Same escrow as before
Multiple fills same side Escrow -= sum(costs)
Partial fill + unlink Escrow correct
Full consume escrow → underflow on next fill Same error as today
Bid vs ask Correct token0/token1 escrow

Test plan (attack / abuse / hack vectors)

Vector Verification
Rounding / overflow in batched subtract Use checked_sub; no wrap
Reentrancy N/A (no external call in loop)
Reserve vs escrow accounting Invariant tests: reserves + pending = token balances (existing suite)

Verification criteria

  • cargo test green for pair + limit order tests.
  • Optional: compare gas_used on LocalTerra multi-maker fill swap before/after (expect decrease proportional to fill count).
## Summary Reduce swap gas by accumulating pending-escrow deltas during limit-book matching and persisting `PENDING_ESCROW_TOKEN0` / `PENDING_ESCROW_TOKEN1` once per side per match, instead of load+save on every fill. ## Current codebase - Escrow totals `PENDING_ESCROW_TOKEN0` / `PENDING_ESCROW_TOKEN1` (`smartcontracts/contracts/pair/src/state.rs`) track CW20 held for resting limits, excluded from AMM reserves. - On each **bid fill** (taker sells token0): `escrow_sub_pending_token1` subtracts `cost` (token1) — **load, subtract, save** per fill (`orderbook.rs`). - On each **ask fill**: `escrow_sub_pending_token0` subtracts `fill_t0` — same pattern. - Placement/batch placement adds escrow in aggregated writes (already efficient in `limit_placement.rs`). - Match loops can run up to `max_maker_fills` (cap 256) fills per swap. ## Why this is needed - Each fill causes **2 storage ops** on the pending escrow `Item` (read + write). At high `max_maker_fills`, this is pure overhead: the match loop is atomic; nothing reads pending escrow mid-loop. - Complements transfer aggregation (GitLab #248); this is **in-pair storage** optimization with no semantic change. ## Constraints and guardrails - **Invariant:** Final pending escrow after match must equal today’s sequential subtracts (same arithmetic, single commit). - **Underflow:** Preserve existing underflow errors (`InvariantViolation` / escrow underflow messages); batch subtract must fail identically if total consumption exceeds pending. - **Scope:** Only refactor `match_bids` / `match_asks` execute paths; `simulate_match_*` does not touch escrow (no change). - **No** change to placement, cancel, claim, or park paths unless they share helpers — keep diffs minimal. - **Migration:** none. ## Relevant files | Area | Path | |------|------| | Escrow helpers | `smartcontracts/contracts/pair/src/orderbook.rs` (`escrow_sub_pending_token0/1`, `match_bids`, `match_asks`) | | State | `smartcontracts/contracts/pair/src/state.rs` | | Tests | `smartcontracts/contracts/pair/src/orderbook.rs` (`aggregation_tests`), `smartcontracts/tests/src/limit_order_tests.rs` | ## Recommended direction 1. In `match_bids` / `match_asks`, accumulate `token1_escrow_delta` / `token0_escrow_delta` in locals during the loop. 2. After loop (or on early break), call a single `apply_escrow_delta_token1(-delta)` helper that loads once, subtracts total, saves once (or skip save if delta zero). 3. Optionally share helper with existing `escrow_sub_pending_*` for cancel/placement consistency. 4. Add unit test: multiple fills in one match → one observable escrow change equal to sum of per-fill subtracts. ## Acceptance criteria - [ ] Multi-fill `match_bids` / `match_asks` perform at most **one** pending-escrow write per token side per invocation. - [ ] Escrow balances and underflow behavior match pre-change for all existing tests. - [ ] No change to simulation query results. ## Test plan (functional paths) | Path | Expectation | |------|-------------| | Single fill | Same escrow as before | | Multiple fills same side | Escrow -= sum(costs) | | Partial fill + unlink | Escrow correct | | Full consume escrow → underflow on next fill | Same error as today | | Bid vs ask | Correct token0/token1 escrow | ## Test plan (attack / abuse / hack vectors) | Vector | Verification | |--------|----------------| | Rounding / overflow in batched subtract | Use `checked_sub`; no wrap | | Reentrancy | N/A (no external call in loop) | | Reserve vs escrow accounting | Invariant tests: reserves + pending = token balances (existing suite) | ## Verification criteria - `cargo test` green for pair + limit order tests. - Optional: compare `gas_used` on LocalTerra multi-maker fill swap before/after (expect decrease proportional to fill count).
PlasticDigits commented 2026-05-31 13:57:42 +00:00 (Migrated from gitlab.com)

mentioned in commit ff5968ac09

mentioned in commit ff5968ac0927fa6972bde237a2dc4d8cf2948fc4
PlasticDigits commented 2026-05-31 13:57:56 +00:00 (Migrated from gitlab.com)

Implemented (main @ ff5968a)

Book match (match_bids / match_asks) now accumulates pending-escrow deltas in locals during the fill loop and applies one PENDING_ESCROW_TOKEN1 or PENDING_ESCROW_TOKEN0 load / checked_sub / save per token side per invocation (zero delta skips the write). Placement, cancel, claim, park, and simulate_match_* are unchanged.

Code

  • smartcontracts/contracts/pair/src/orderbook.rs — batched subtract + unit tests in aggregation_tests
  • Invariant L13 in docs/contracts-security-audit.md
  • docs/limit-orders.md § Execution order — cross-link
  • skills/AGENTS_TERRACLASSIC_GAS.md — rule 17 + canonical table row

Tests run

  • cargo test -p cl8y-dex-pair orderbook::aggregation_tests
  • cargo test -p cl8y-dex-pair orderbook:: (17 tests)
  • cargo test -p cl8y-dex-tests limit_order (53 tests)
  • Pre-commit: fmt, clippy, gitleaks

Verification checklist

  • Multi-maker hybrid swap: escrow balances and maker payouts match pre-change behavior
  • HybridSimulation quotes unchanged for same chain snapshot (no escrow writes in sim)
  • Single-fill bid/ask match: pending escrow delta equals fill cost
  • Underflow path still returns InvariantViolation with pending escrow token1/token0 underflow
  • Proptest / integration: orderbook::proptest_limits, full limit_order_tests
  • Optional: LocalTerra multi-maker fill — compare gas_used before/after (expect decrease ~ proportional to fill count)

Follow-up (optional)

  • LocalTerra gas_used benchmark on deep book (complements #252 warm-swarm tuning); no contract semantics change expected.

QA agent team: Please verify the checklist on LocalTerra or columbus-5 as appropriate and confirm escrow/reserve invariants (L1, L13) on a multi-fill hybrid swap.

## Implemented (main @ ff5968a) Book match (`match_bids` / `match_asks`) now accumulates pending-escrow deltas in locals during the fill loop and applies **one** `PENDING_ESCROW_TOKEN1` or `PENDING_ESCROW_TOKEN0` load / `checked_sub` / save per token side per invocation (zero delta skips the write). Placement, cancel, claim, park, and `simulate_match_*` are unchanged. ### Code - `smartcontracts/contracts/pair/src/orderbook.rs` — batched subtract + unit tests in `aggregation_tests` - Invariant **L13** in `docs/contracts-security-audit.md` - `docs/limit-orders.md` § Execution order — cross-link - `skills/AGENTS_TERRACLASSIC_GAS.md` — rule 17 + canonical table row ### Tests run - `cargo test -p cl8y-dex-pair orderbook::aggregation_tests` - `cargo test -p cl8y-dex-pair orderbook::` (17 tests) - `cargo test -p cl8y-dex-tests limit_order` (53 tests) - Pre-commit: `fmt`, `clippy`, `gitleaks` ## Verification checklist - [ ] Multi-maker hybrid swap: escrow balances and maker payouts match pre-change behavior - [ ] `HybridSimulation` quotes unchanged for same chain snapshot (no escrow writes in sim) - [ ] Single-fill bid/ask match: pending escrow delta equals fill cost - [ ] Underflow path still returns `InvariantViolation` with `pending escrow token1/token0 underflow` - [ ] Proptest / integration: `orderbook::proptest_limits`, full `limit_order_tests` - [ ] Optional: LocalTerra multi-maker fill — compare `gas_used` before/after (expect decrease ~ proportional to fill count) ## Follow-up (optional) - LocalTerra `gas_used` benchmark on deep book (complements #252 warm-swarm tuning); no contract semantics change expected. --- QA agent team: Please verify the checklist on LocalTerra or columbus-5 as appropriate and confirm escrow/reserve invariants (L1, L13) on a multi-fill hybrid swap.
Brouie commented 2026-06-01 03:20:45 +00:00 (Migrated from gitlab.com)

Verified #255 on main @ 6b22feb (ff5968a, live on LocalTerra). Source + named tests + a live multi-fill hybrid swap confirming invariants L1 and L13. All acceptance criteria covered.

Live multi-fill (the headline — L1 + L13 on a real 5-maker swap)

Placed 5 live bids @1.05 (above the resting book so they sit at the head and fill cleanly), then one taker hybrid swap: code=0, no OOG, gas_used=668,820, 5 fills.

L13 — batched escrow release equals the sum of per-fill costs (exact):

  • pending_T1 released = 2,279,300,000 − 1,783,800,005 = 495,499,995
  • book_return_amount (495,252,250) + book_commission_amount (247,745) = 495,499,995 → match, to the unit
  • maker payout: maker(test2) T0 delta 471,904,760 == limit_book_offer_consumed 471,904,760 → match

L1 — pair balance = reserves + pending escrow, both tokens, before and after:

  • pre-swap T0: 98,914,591,705 == 98,914,591,705 + 0 ✓ | T1: 93,519,630,313 == 91,240,330,313 + 2,279,300,000 ✓
  • post-swap T0: 99,142,686,945 == 99,142,686,945 + 0 ✓ | T1: 92,814,215,844 == 91,030,415,839 + 1,783,800,005 ✓

(pending read from raw state escrow_t0/escrow_t1; reserves from raw reserves; balances from the pair's CW20 balance. I checked the escrow/reserve/maker sides rather than the taker's T1 delta, to sidestep the localnet treasury==dev-wallet overlap.)

Acceptance criteria

  • AC1 — ≤1 pending-escrow write per token side per invocation (zero delta skips): source — match_bids/match_asks accumulate token{1,0}_escrow_sub_total (checked_add) and call escrow_sub_pending_token{1,0} once after the loop; the helper early-returns on zero, else may_load → checked_sub → save. simulate_match_* touch no escrow. Tests aggregation_tests::{match_bids_batches_pending_escrow_token1_subtract, match_asks_batches_pending_escrow_token0_subtract}. Layer note below.
  • AC2 — escrow + underflow match pre-change: pair orderbook 24/0 and integration limit_order 55/0 green; match_bids_pending_escrow_underflow_on_excessive_batch_subtract returns InvariantViolation { "pending escrow token1 underflow" } (same checked_sub + message as the per-fill path). Live L13 arithmetic exact.
  • AC3 — no change to simulation results: source — simulate_match_bids/simulate_match_asks have no PENDING_ESCROW load/save at all; HybridSimulation tests pass.

Functional + attack plans

  • Functional (single / multi same-side / partial+unlink / consume→underflow / bid vs ask): covered by aggregation_tests (batched subtract, match_bids_commission_and_return_net_sum_per_fill, match_bids_dedupes_maker_payouts_by_owner), the underflow test, and the live 5-fill bid run.
  • Attack: rounding/overflow → checked_sub/checked_add, no wrap (source + underflow test); reentrancy N/A — no external call inside the match loop (transfers aggregated after); reserve-vs-escrow accounting → L1 live-confirmed + proptest_limits::{prop_match_bids_maker_cap, prop_match_asks_maker_cap, prop_escrow_dll_after_random_inserts}.

Dev checklist

  • Multi-maker hybrid swap: escrow + maker payouts match — live (above)
  • HybridSimulation quotes unchanged (no escrow writes in sim) — source
  • Single-fill bid/ask: pending delta == fill cost — aggregation_tests + live per-fill sum exact
  • Underflow → InvariantViolation "pending escrow token1/token0 underflow" — test + source
  • proptest_limits + full limit_order_tests — green
  • [~] Optional LocalTerra gas before/after — see flag below

Layer note (being precise)

The "one storage write per side" claim is source/test-level — individual storage writes aren't observable from a tx, so I can't prove the write count on-chain. What's live-verified is the observable consequence: the final escrow equals the sum of per-fill costs exactly (L13), and L1 holds before and after. Source + the aggregation_tests carry the write-count claim.

Transparency observation (benign)

Each of the 5 bids left a 1-unit token1 remainder from integer rounding (floor(94,380,952 × 1.05) cost), so they stayed in the book with remaining=1 rather than fully unlinking. Harmless dust — and notably the escrow accounting stays exact through it (released total matches book_return + book_commission to the unit, and L1 still balances).

One open item (flagged, not chased)

The optional "gas before/after" can't be shown as a numeric delta — there's no pre-#255 (unbatched) build deployed, same structural baseline gap as #252/#254. The live swap gas (668,820 for 5 fills) is a single absolute data point, not a contrast. The gas reduction is structural (one load+save per side instead of per fill) and proven by the source + aggregation_tests, not by an on-chain before/after.

@PlasticDigits — verified and signed off from my side, no issues found (L1 + L13 hold exactly on a live multi-fill); over to you to close.

Verified #255 on `main` @ `6b22feb` (`ff5968a`, live on LocalTerra). Source + named tests + a live multi-fill hybrid swap confirming invariants L1 and L13. All acceptance criteria covered. ## Live multi-fill (the headline — L1 + L13 on a real 5-maker swap) Placed 5 live bids @1.05 (above the resting book so they sit at the head and fill cleanly), then one taker hybrid swap: `code=0`, no OOG, `gas_used=668,820`, **5 fills**. **L13 — batched escrow release equals the sum of per-fill costs (exact):** - pending_T1 released = `2,279,300,000 − 1,783,800,005 = 495,499,995` - `book_return_amount (495,252,250) + book_commission_amount (247,745) = 495,499,995` → **match, to the unit** - maker payout: maker(test2) T0 delta `471,904,760` == `limit_book_offer_consumed 471,904,760` → match **L1 — pair balance = reserves + pending escrow, both tokens, before and after:** - pre-swap T0: `98,914,591,705 == 98,914,591,705 + 0` ✓ | T1: `93,519,630,313 == 91,240,330,313 + 2,279,300,000` ✓ - post-swap T0: `99,142,686,945 == 99,142,686,945 + 0` ✓ | T1: `92,814,215,844 == 91,030,415,839 + 1,783,800,005` ✓ (pending read from raw state `escrow_t0`/`escrow_t1`; reserves from raw `reserves`; balances from the pair's CW20 balance. I checked the escrow/reserve/maker sides rather than the taker's T1 delta, to sidestep the localnet treasury==dev-wallet overlap.) ## Acceptance criteria - **AC1 — ≤1 pending-escrow write per token side per invocation (zero delta skips):** source — `match_bids`/`match_asks` accumulate `token{1,0}_escrow_sub_total` (`checked_add`) and call `escrow_sub_pending_token{1,0}` once after the loop; the helper early-returns on zero, else `may_load → checked_sub → save`. `simulate_match_*` touch no escrow. Tests `aggregation_tests::{match_bids_batches_pending_escrow_token1_subtract, match_asks_batches_pending_escrow_token0_subtract}`. Layer note below. - **AC2 — escrow + underflow match pre-change:** pair `orderbook` 24/0 and integration `limit_order` 55/0 green; `match_bids_pending_escrow_underflow_on_excessive_batch_subtract` returns `InvariantViolation { "pending escrow token1 underflow" }` (same `checked_sub` + message as the per-fill path). Live L13 arithmetic exact. - **AC3 — no change to simulation results:** source — `simulate_match_bids`/`simulate_match_asks` have no `PENDING_ESCROW` load/save at all; HybridSimulation tests pass. ## Functional + attack plans - Functional (single / multi same-side / partial+unlink / consume→underflow / bid vs ask): covered by `aggregation_tests` (batched subtract, `match_bids_commission_and_return_net_sum_per_fill`, `match_bids_dedupes_maker_payouts_by_owner`), the underflow test, and the live 5-fill bid run. - Attack: rounding/overflow → `checked_sub`/`checked_add`, no wrap (source + underflow test); reentrancy N/A — no external call inside the match loop (transfers aggregated after); reserve-vs-escrow accounting → **L1 live-confirmed** + `proptest_limits::{prop_match_bids_maker_cap, prop_match_asks_maker_cap, prop_escrow_dll_after_random_inserts}`. ## Dev checklist - [x] Multi-maker hybrid swap: escrow + maker payouts match — live (above) - [x] HybridSimulation quotes unchanged (no escrow writes in sim) — source - [x] Single-fill bid/ask: pending delta == fill cost — `aggregation_tests` + live per-fill sum exact - [x] Underflow → `InvariantViolation` "pending escrow token1/token0 underflow" — test + source - [x] `proptest_limits` + full `limit_order_tests` — green - [~] Optional LocalTerra gas before/after — see flag below ## Layer note (being precise) The "one storage write per side" claim is **source/test-level** — individual storage writes aren't observable from a tx, so I can't prove the write *count* on-chain. What's **live-verified** is the observable consequence: the final escrow equals the sum of per-fill costs exactly (L13), and L1 holds before and after. Source + the `aggregation_tests` carry the write-count claim. ## Transparency observation (benign) Each of the 5 bids left a **1-unit** token1 remainder from integer rounding (`floor(94,380,952 × 1.05)` cost), so they stayed in the book with `remaining=1` rather than fully unlinking. Harmless dust — and notably the escrow accounting stays exact through it (released total matches `book_return + book_commission` to the unit, and L1 still balances). ## One open item (flagged, not chased) The optional "gas before/after" can't be shown as a numeric delta — there's no pre-#255 (unbatched) build deployed, same structural baseline gap as #252/#254. The live swap gas (668,820 for 5 fills) is a single absolute data point, not a contrast. The gas reduction is structural (one load+save per side instead of per fill) and proven by the source + `aggregation_tests`, not by an on-chain before/after. @PlasticDigits — verified and signed off from my side, no issues found (L1 + L13 hold exactly on a live multi-fill); over to you to close.
PlasticDigits commented 2026-06-01 03:40:11 +00:00 (Migrated from gitlab.com)

Must open an issue to prevent dust (less than 10 unites left) from accumulating in the book and growing chain state - we need remaining to be 0 after execution so state can be deleted.

Must open an issue to prevent dust (less than 10 unites left) from accumulating in the book and growing chain state - we need remaining to be 0 after execution so state can be deleted.
PlasticDigits commented 2026-06-01 03:42:16 +00:00 (Migrated from gitlab.com)

mentioned in issue #264

mentioned in issue #264
PlasticDigits commented 2026-06-01 03:42:29 +00:00 (Migrated from gitlab.com)

marked as related to #264

marked as related to #264
PlasticDigits commented 2026-06-01 03:42:30 +00:00 (Migrated from gitlab.com)

Follow-up opened

Implementation verified on main (see QA sign-off above). The live multi-fill run surfaced sub-10-unit dust remainders (remaining = 1 token1 per bid from mul_floor rounding) that stay in ORDERS and bloat chain state.

Tracked in #264 — Limit book match: auto-flush sub-10-unit dust remainders after fill (proactive flush at match time; complements governance CleanLimitBook in #263).

Closing this issue; escrow batching (L13) is complete.

## Follow-up opened Implementation verified on `main` (see QA sign-off above). The live multi-fill run surfaced **sub-10-unit dust remainders** (`remaining = 1` token1 per bid from `mul_floor` rounding) that stay in `ORDERS` and bloat chain state. Tracked in **#264** — *Limit book match: auto-flush sub-10-unit dust remainders after fill* (proactive flush at match time; complements governance `CleanLimitBook` in **#263**). Closing this issue; escrow batching (**L13**) is complete.
PlasticDigits (Migrated from gitlab.com) closed this issue 2026-06-01 03:42:34 +00:00
Brouie commented 2026-06-01 03:44:37 +00:00 (Migrated from gitlab.com)

mentioned in issue #256

mentioned in issue #256
Brouie commented 2026-06-01 04:11:04 +00:00 (Migrated from gitlab.com)

mentioned in issue #257

mentioned in issue #257
Brouie commented 2026-06-01 05:25:47 +00:00 (Migrated from gitlab.com)

mentioned in issue #258

mentioned in issue #258
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#255
No description provided.