Limit book match: batch pending-escrow storage updates per swap #255
Labels
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
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
code/cl8y-dex-terraclassic#255
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Reduce swap gas by accumulating pending-escrow deltas during limit-book matching and persisting
PENDING_ESCROW_TOKEN0/PENDING_ESCROW_TOKEN1once per side per match, instead of load+save on every fill.Current codebase
PENDING_ESCROW_TOKEN0/PENDING_ESCROW_TOKEN1(smartcontracts/contracts/pair/src/state.rs) track CW20 held for resting limits, excluded from AMM reserves.escrow_sub_pending_token1subtractscost(token1) — load, subtract, save per fill (orderbook.rs).escrow_sub_pending_token0subtractsfill_t0— same pattern.limit_placement.rs).max_maker_fills(cap 256) fills per swap.Why this is needed
Item(read + write). At highmax_maker_fills, this is pure overhead: the match loop is atomic; nothing reads pending escrow mid-loop.Constraints and guardrails
InvariantViolation/ escrow underflow messages); batch subtract must fail identically if total consumption exceeds pending.match_bids/match_asksexecute paths;simulate_match_*does not touch escrow (no change).Relevant files
smartcontracts/contracts/pair/src/orderbook.rs(escrow_sub_pending_token0/1,match_bids,match_asks)smartcontracts/contracts/pair/src/state.rssmartcontracts/contracts/pair/src/orderbook.rs(aggregation_tests),smartcontracts/tests/src/limit_order_tests.rsRecommended direction
match_bids/match_asks, accumulatetoken1_escrow_delta/token0_escrow_deltain locals during the loop.apply_escrow_delta_token1(-delta)helper that loads once, subtracts total, saves once (or skip save if delta zero).escrow_sub_pending_*for cancel/placement consistency.Acceptance criteria
match_bids/match_asksperform at most one pending-escrow write per token side per invocation.Test plan (functional paths)
Test plan (attack / abuse / hack vectors)
checked_sub; no wrapVerification criteria
cargo testgreen for pair + limit order tests.gas_usedon LocalTerra multi-maker fill swap before/after (expect decrease proportional to fill count).mentioned in commit
ff5968ac09Implemented (main @
ff5968a)Book match (
match_bids/match_asks) now accumulates pending-escrow deltas in locals during the fill loop and applies onePENDING_ESCROW_TOKEN1orPENDING_ESCROW_TOKEN0load /checked_sub/ save per token side per invocation (zero delta skips the write). Placement, cancel, claim, park, andsimulate_match_*are unchanged.Code
smartcontracts/contracts/pair/src/orderbook.rs— batched subtract + unit tests inaggregation_testsdocs/contracts-security-audit.mddocs/limit-orders.md§ Execution order — cross-linkskills/AGENTS_TERRACLASSIC_GAS.md— rule 17 + canonical table rowTests run
cargo test -p cl8y-dex-pair orderbook::aggregation_testscargo test -p cl8y-dex-pair orderbook::(17 tests)cargo test -p cl8y-dex-tests limit_order(53 tests)fmt,clippy,gitleaksVerification checklist
HybridSimulationquotes unchanged for same chain snapshot (no escrow writes in sim)InvariantViolationwithpending escrow token1/token0 underfloworderbook::proptest_limits, fulllimit_order_testsgas_usedbefore/after (expect decrease ~ proportional to fill count)Follow-up (optional)
gas_usedbenchmark 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.
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):
2,279,300,000 − 1,783,800,005 = 495,499,995book_return_amount (495,252,250) + book_commission_amount (247,745) = 495,499,995→ match, to the unit471,904,760==limit_book_offer_consumed 471,904,760→ matchL1 — pair balance = reserves + pending escrow, both tokens, before and after:
98,914,591,705 == 98,914,591,705 + 0✓ | T1:93,519,630,313 == 91,240,330,313 + 2,279,300,000✓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 rawreserves; 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
match_bids/match_asksaccumulatetoken{1,0}_escrow_sub_total(checked_add) and callescrow_sub_pending_token{1,0}once after the loop; the helper early-returns on zero, elsemay_load → checked_sub → save.simulate_match_*touch no escrow. Testsaggregation_tests::{match_bids_batches_pending_escrow_token1_subtract, match_asks_batches_pending_escrow_token0_subtract}. Layer note below.orderbook24/0 and integrationlimit_order55/0 green;match_bids_pending_escrow_underflow_on_excessive_batch_subtractreturnsInvariantViolation { "pending escrow token1 underflow" }(samechecked_sub+ message as the per-fill path). Live L13 arithmetic exact.simulate_match_bids/simulate_match_askshave noPENDING_ESCROWload/save at all; HybridSimulation tests pass.Functional + attack plans
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.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
aggregation_tests+ live per-fill sum exactInvariantViolation"pending escrow token1/token0 underflow" — test + sourceproptest_limits+ fulllimit_order_tests— greenLayer 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_testscarry 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 withremaining=1rather than fully unlinking. Harmless dust — and notably the escrow accounting stays exact through it (released total matchesbook_return + book_commissionto 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.
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.
mentioned in issue #264
marked as related to #264
Follow-up opened
Implementation verified on
main(see QA sign-off above). The live multi-fill run surfaced sub-10-unit dust remainders (remaining = 1token1 per bid frommul_floorrounding) that stay inORDERSand 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
CleanLimitBookin #263).Closing this issue; escrow batching (L13) is complete.
mentioned in issue #256
mentioned in issue #257
mentioned in issue #258