Skip zero-cost book fills when floor(fill × price) is 0 (#470) #1004

2026-07-01T14:08:08Z

PlasticDigits commented

2026-07-01 14:08:08 +00:00

(Migrated from gitlab.com)

Summary

Fixes GitLab #470: when a resting limit price is below 1 token1 per token0, small fills can make floor(fill × price) = 0 while fill > 0. Without a guard, match_asks debited maker token0 escrow and credited 0 token1 (symmetric bug on match_bids).

Change: after computing cost (and the too-expensive shrink loop), skip the order when cost.is_zero() — same path in simulate_match_* and indexer db_orderbook_sim for L8 parity.

Documented as invariant L18 in docs/contracts-security-audit.md and docs/limit-orders.md.

Acceptance checklist

Criterion	Verification	Result
`match_asks` skips fill when `floor(fill_t0 × price) = 0`	`cd smartcontracts && cargo test -p cl8y-dex-tests match_asks_skips_zero_cost_fill_sub_unity_price`	PASS
`match_bids` symmetric skip	`cd smartcontracts && cargo test -p cl8y-dex-tests match_bids_skips_zero_cost_fill_sub_unity_price`	PASS
`simulate_match_*` parity (L8)	Ask test asserts `HybridSimulation.return_amount == 0`; pair `orderbook::` unit tests	PASS
Indexer Postgres mirror aligned	`cd indexer && cargo test --lib db_orderbook`	PASS
No regression on book matching	`cd smartcontracts && cargo test -p cl8y-dex-pair orderbook::`	PASS
Invariants documented + cross-linked	`docs/contracts-security-audit.md` L18, `docs/limit-orders.md`, `skills/AGENTS_BOOK_MATCH_HINT_SECURITY.md`	PASS
Bid-side checked per issue comment	`match_bids` + `simulate_match_bids` + indexer `simulate_match_bids` all guarded	PASS

Third-party verification

git fetch origin fix/470-match-zero-cost-skip
git checkout fix/470-match-zero-cost-skip
cd smartcontracts && cargo test -p cl8y-dex-tests zero_cost_fill
cd smartcontracts && cargo test -p cl8y-dex-pair orderbook::
cd indexer && cargo test --lib db_orderbook

Manual repro (pre-fix): place ask at price 0.4, hybrid swap token1→token0 with book_input=1; maker lost token0 with zero token1 payout. Post-fix: order remaining unchanged.

Related: #381 hardening umbrella. Issue #470 left open until merge.

## Summary Fixes GitLab #470: when a resting limit price is below 1 token1 per token0, small fills can make `floor(fill × price) = 0` while `fill > 0`. Without a guard, `match_asks` debited maker token0 escrow and credited 0 token1 (symmetric bug on `match_bids`). **Change:** after computing `cost` (and the too-expensive shrink loop), skip the order when `cost.is_zero()` — same path in `simulate_match_*` and indexer `db_orderbook_sim` for L8 parity. Documented as invariant **L18** in `docs/contracts-security-audit.md` and `docs/limit-orders.md`. ## Acceptance checklist | Criterion | Verification | Result | |-----------|--------------|--------| | `match_asks` skips fill when `floor(fill_t0 × price) = 0` | `cd smartcontracts && cargo test -p cl8y-dex-tests match_asks_skips_zero_cost_fill_sub_unity_price` | PASS | | `match_bids` symmetric skip | `cd smartcontracts && cargo test -p cl8y-dex-tests match_bids_skips_zero_cost_fill_sub_unity_price` | PASS | | `simulate_match_*` parity (L8) | Ask test asserts `HybridSimulation.return_amount == 0`; pair `orderbook::` unit tests | PASS | | Indexer Postgres mirror aligned | `cd indexer && cargo test --lib db_orderbook` | PASS | | No regression on book matching | `cd smartcontracts && cargo test -p cl8y-dex-pair orderbook::` | PASS | | Invariants documented + cross-linked | `docs/contracts-security-audit.md` L18, `docs/limit-orders.md`, `skills/AGENTS_BOOK_MATCH_HINT_SECURITY.md` | PASS | | Bid-side checked per issue comment | `match_bids` + `simulate_match_bids` + indexer `simulate_match_bids` all guarded | PASS | ## Third-party verification ```bash git fetch origin fix/470-match-zero-cost-skip git checkout fix/470-match-zero-cost-skip cd smartcontracts && cargo test -p cl8y-dex-tests zero_cost_fill cd smartcontracts && cargo test -p cl8y-dex-pair orderbook:: cd indexer && cargo test --lib db_orderbook ``` Manual repro (pre-fix): place ask at price 0.4, hybrid swap token1→token0 with `book_input=1`; maker lost token0 with zero token1 payout. Post-fix: order `remaining` unchanged. Related: #381 hardening umbrella. Issue #470 left open until merge.

PlasticDigits commented

2026-07-01 14:10:31 +00:00

(Migrated from gitlab.com)

Security review — MR !1004

Commit reviewed: 93a8b20096e86b18221e4c4837534ade76954d14
Scope: L18 zero-cost fill skip in match_bids / match_asks, simulate_match_*, indexer db_orderbook_sim; docs/tests/skills only elsewhere.

Outcome: FINDINGS: 0 medium+

Summary

This MR closes a pre-existing maker fund-loss bug (GitLab #470): without the guard, floor(fill × price) = 0 with fill > 0 debited maker escrow while crediting zero on the counter leg. The fix correctly skips the order (continue) after the shrink loop, before makers_used increment and any escrow/payout mutation — symmetric on bids and asks.

Attack-path review (no new medium+ issues):

Candidate	Trace	Verdict
Maker escrow theft via sub-unity fills	Taker-controlled `book_input` / hybrid split → `match_*` fill math	Fixed by this MR; pre-fix was exploitable, post-fix aborts before state change
Taker fund loss on skip	Skipped orders leave `offer_consumed = 0`; unconsumed `book_leg` rolls to pool per existing hybrid routing (`pool_input_amount = pool_leg + book_leg − offer_consumed`)	Safe — taker input is not burned; slippage floors (`min_return`, `belief_price`, `max_spread`) still apply at execute
Quote/execute (L8) manipulation	Indexer `db_orderbook_sim` guarded same as on-chain `simulate_match_*`	Aligned — no new sandwich/quote drift surface
Head-clog / scan-step griefing	Sub-unity orders consume `MAX_SCAN_STEPS` without filling	Pre-existing class (#289 / `book_start_hint`, `CleanLimitBook`); attacker must post escrow; not introduced by this diff
Authz / injection / SSRF / XSS	Diff is pure matching math + mirror	N/A

Prior security-review threads: none on this MR.

Inline findings: none (no medium+ items to thread).

Security review: no medium+ findings on this diff.

## Security review — MR !1004 **Commit reviewed:** `93a8b20096e86b18221e4c4837534ade76954d14` **Scope:** L18 zero-cost fill skip in `match_bids` / `match_asks`, `simulate_match_*`, indexer `db_orderbook_sim`; docs/tests/skills only elsewhere. **Outcome:** `FINDINGS: 0` medium+ ### Summary This MR closes a **pre-existing maker fund-loss bug** (GitLab #470): without the guard, `floor(fill × price) = 0` with `fill > 0` debited maker escrow while crediting zero on the counter leg. The fix correctly skips the order (`continue`) after the shrink loop, before `makers_used` increment and any escrow/payout mutation — symmetric on bids and asks. **Attack-path review (no new medium+ issues):** | Candidate | Trace | Verdict | |-----------|-------|---------| | Maker escrow theft via sub-unity fills | Taker-controlled `book_input` / hybrid split → `match_*` fill math | **Fixed** by this MR; pre-fix was exploitable, post-fix aborts before state change | | Taker fund loss on skip | Skipped orders leave `offer_consumed = 0`; unconsumed `book_leg` rolls to pool per existing hybrid routing (`pool_input_amount = pool_leg + book_leg − offer_consumed`) | **Safe** — taker input is not burned; slippage floors (`min_return`, `belief_price`, `max_spread`) still apply at execute | | Quote/execute (L8) manipulation | Indexer `db_orderbook_sim` guarded same as on-chain `simulate_match_*` | **Aligned** — no new sandwich/quote drift surface | | Head-clog / scan-step griefing | Sub-unity orders consume `MAX_SCAN_STEPS` without filling | **Pre-existing class** (#289 / `book_start_hint`, `CleanLimitBook`); attacker must post escrow; not introduced by this diff | | Authz / injection / SSRF / XSS | Diff is pure matching math + mirror | **N/A** | Prior security-review threads: none on this MR. **Inline findings:** none (no medium+ items to thread). `Security review: no medium+ findings on this diff.`