code/cl8y-dex-terraclassic

Fork 0

match_asks credits maker zero token1 when fill_t0 * price floors to 0 (ask priced below 1) #470

New issue

Closed

opened 2026-07-01 12:57:08 +00:00 by Brouie · 6 comments

Brouie commented

2026-07-01 12:57:08 +00:00

(Migrated from gitlab.com)

What / where

match_asks in smartcontracts/contracts/pair/src/orderbook.rs. The cost of a fill is computed at line 1608:

let mut cost = fill_t0.checked_mul_floor(order.price)...  // 1608

cost is what the maker gets paid (token1) and what the taker's budget gets debited. There's no cost > 0 guard anywhere before it's used, so a fill where cost == 0 sails straight through.

Why it happens

checked_mul_floor rounds toward zero. When an ask is priced below 1 (order.price = token1-per-token0 < 1, which is realistic when the two tokens have mismatched decimals — both denoms are allowed up to 18), a small fill_t0 makes fill_t0 * price land in the open interval (0,1) and floor to 0.

The one adjustment loop right after only handles the too-expensive direction:

while fill_t0 > Uint128::zero() && cost > token1_left {   // 1613
    fill_t0 = fill_t0.saturating_sub(Uint128::one());
    ...
    cost = fill_t0.checked_mul_floor(order.price)...       // 1618
}

It shrinks fill_t0 when cost exceeds the taker budget. It never touches the case where cost floored to 0, and fill_t0 itself is still non-zero, so the earlier fill_t0.is_zero() continues (1603, 1624) don't catch it either.

With cost == 0 the loop then does, per matched maker:

maker payout credited cost = 0 token1 (line 1639)
maker's ask escrow debited fill_t0 token0 (line 1641, batched into token0_escrow_sub_total)
maker's order.remaining reduced by fill_t0 (line 1654)
taker credited net_to_taker ≈ fill_t0 token0 (line 1656)
token1_left -= cost → token1_left unchanged (line 1655), so the taker's token1 budget isn't even consumed

So the maker gives away up to fill_t0 token0 and receives nothing, and the taker gets that token0 essentially for free.

How to hit it

List an ask on a pair where token1 has fewer effective decimals than token0, so the maker's price is a fraction < 1 (e.g. price = 0.4 token1 per token0). Nothing stops this — decimals up to 18 on both sides.
Send a market/take against the book with a token1 budget small enough that max_fill_token0_from_budget (1588-1600) resolves to a fill_t0 where fill_t0 * 0.4 < 1 — i.e. fill_t0 = 1 or 2 base units.
fill_t0 is non-zero, cost = floor(fill_t0 * 0.4) = 0. The fill executes: maker escrow drops by fill_t0, maker gets 0 token1, taker walks with the token0.

It's dust per fill, but it's repeatable and it's a straight-up broken price guarantee — the maker's limit price says "I want 0.4 token1 per token0" and the book honors it at 0.

Fix direction

Guard cost > 0 before committing the fill. Cleanest options:

After computing cost (and after the too-expensive loop), if cost.is_zero() then continue to the next order instead of filling — the taker's remaining budget just can't afford a price-honoring fill against this maker.
Or enforce a minimum fill_t0 such that fill_t0 * price can't floor to zero (round the fill size up to the smallest quantity that yields cost >= 1, capped by order.remaining and budget).

Rounding the cost up instead of the fill would over-charge the taker vs their stated budget, so I'd lean toward the skip-when-zero path — it keeps both the maker's price and the taker's budget honest. Same pattern should be checked on the bid side of the match (match_bids) for the symmetric floor-to-zero case.

Came out of the security sweep on the pair contract's orderbook matching (fits under the #381 hardening umbrella — value-flow / invariant correctness). This is low severity but it's a real limit-price violation: an ask maker can hand over token0 and get zero token1 back. ## What / where `match_asks` in `smartcontracts/contracts/pair/src/orderbook.rs`. The cost of a fill is computed at line 1608: ```rust let mut cost = fill_t0.checked_mul_floor(order.price)... // 1608 ``` `cost` is what the maker gets paid (token1) and what the taker's budget gets debited. There's no `cost > 0` guard anywhere before it's used, so a fill where `cost == 0` sails straight through. ## Why it happens `checked_mul_floor` rounds toward zero. When an ask is priced below 1 (`order.price` = token1-per-token0 < 1, which is realistic when the two tokens have mismatched decimals — both denoms are allowed up to 18), a small `fill_t0` makes `fill_t0 * price` land in the open interval (0,1) and floor to 0. The one adjustment loop right after only handles the *too-expensive* direction: ```rust while fill_t0 > Uint128::zero() && cost > token1_left { // 1613 fill_t0 = fill_t0.saturating_sub(Uint128::one()); ... cost = fill_t0.checked_mul_floor(order.price)... // 1618 } ``` It shrinks `fill_t0` when cost exceeds the taker budget. It never touches the case where cost floored to 0, and `fill_t0` itself is still non-zero, so the earlier `fill_t0.is_zero()` continues (1603, 1624) don't catch it either. With `cost == 0` the loop then does, per matched maker: - maker payout credited `cost` = 0 token1 (line 1639) - maker's ask escrow debited `fill_t0` token0 (line 1641, batched into `token0_escrow_sub_total`) - maker's `order.remaining` reduced by `fill_t0` (line 1654) - taker credited `net_to_taker` ≈ `fill_t0` token0 (line 1656) - `token1_left -= cost` → `token1_left` unchanged (line 1655), so the taker's token1 budget isn't even consumed So the maker gives away up to `fill_t0` token0 and receives nothing, and the taker gets that token0 essentially for free. ## How to hit it - List an ask on a pair where token1 has fewer effective decimals than token0, so the maker's price is a fraction < 1 (e.g. price = 0.4 token1 per token0). Nothing stops this — decimals up to 18 on both sides. - Send a market/take against the book with a token1 budget small enough that `max_fill_token0_from_budget` (1588-1600) resolves to a `fill_t0` where `fill_t0 * 0.4 < 1` — i.e. `fill_t0` = 1 or 2 base units. - `fill_t0` is non-zero, `cost = floor(fill_t0 * 0.4) = 0`. The fill executes: maker escrow drops by `fill_t0`, maker gets 0 token1, taker walks with the token0. It's dust per fill, but it's repeatable and it's a straight-up broken price guarantee — the maker's limit price says "I want 0.4 token1 per token0" and the book honors it at 0. ## Fix direction Guard `cost > 0` before committing the fill. Cleanest options: - After computing `cost` (and after the too-expensive loop), if `cost.is_zero()` then `continue` to the next order instead of filling — the taker's remaining budget just can't afford a price-honoring fill against this maker. - Or enforce a minimum `fill_t0` such that `fill_t0 * price` can't floor to zero (round the fill size up to the smallest quantity that yields `cost >= 1`, capped by `order.remaining` and budget). Rounding the *cost* up instead of the fill would over-charge the taker vs their stated budget, so I'd lean toward the skip-when-zero path — it keeps both the maker's price and the taker's budget honest. Same pattern should be checked on the bid side of the match (`match_bids`) for the symmetric floor-to-zero case. </body> </invoke>

PlasticDigits commented

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

(Migrated from gitlab.com)

Investigate the Fix with cost.is_zero continue solution and make sure it doesnt create any unexpected behavior or liquidity drains. Also check bid side

PlasticDigits commented

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

(Migrated from gitlab.com)

mentioned in commit 93a8b20096

mentioned in commit 93a8b20096e86b18221e4c4837534ade76954d14

PlasticDigits commented

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

(Migrated from gitlab.com)

mentioned in merge request !1004

PlasticDigits commented

2026-07-02 02:00:53 +00:00

(Migrated from gitlab.com)

mentioned in commit ff18265e77

mentioned in commit ff18265e77df292a3c2c734378b3657d74556aea

PlasticDigits commented

2026-07-07 02:15:57 +00:00

(Migrated from gitlab.com)

Verification complete — PASS

Verified GitLab #470 on main (no repo changes; no MR).

Acceptance criteria

Item	Result	How verified
Ask side: skip fill when `floor(fill_t0 × price) = 0` while `fill_t0 > 0`	PASS	`match_asks` guards `cost.is_zero()` after the too-expensive shrink loop (`orderbook.rs` ~1638). Integration test `match_asks_skips_zero_cost_fill_sub_unity_price`: price=0.4, swap_in=1 → order `remaining` unchanged, maker token1 balance unchanged, `HybridSimulation` return=0.
Bid side: symmetric zero-cost skip	PASS	`match_bids` same guard (~1467). Integration test `match_bids_skips_zero_cost_fill_sub_unity_price`: price=0.4, swap_in=1 → bid `remaining` unchanged, maker token0 balance unchanged.
Simulation parity (`simulate_match_*`)	PASS	`simulate_match_bids` / `simulate_match_asks` both `continue` on `cost.is_zero()` (~1783, ~1892). Ask integration test asserts `sim.return_amount.is_zero()`.
Indexer Postgres mirror (`db_orderbook_sim`)	PASS	`simulate_match_bids` / `simulate_match_asks` skip when `cost == 0` (~316, ~381). `cargo test --lib db_orderbook_sim` — 8/8 pass.
No unexpected liquidity drain / honest skip semantics	PASS	Skip path advances `cur = order.next` without debiting escrow or crediting zero payout; taker budget (`token1_left` / `token0_left`) unchanged on skip — matches issue's recommended `continue` fix (no cost round-up).
Docs / invariants	PASS	L18 in `docs/contracts-security-audit.md`; product note in `docs/limit-orders.md` § Zero-cost fill skip; agent playbook `skills/AGENTS_BOOK_MATCH_HINT_SECURITY.md` § L18 with test commands.

Commands run

cd smartcontracts && cargo test -p cl8y-dex-tests skips_zero_cost_fill   # 2/2 pass
cd smartcontracts && cargo test -p cl8y-dex-pair orderbook               # 38/38 pass
cd indexer && cargo test --lib db_orderbook_sim                          # 8/8 pass
make test-contracts                                                      # pass

Follow-ups

Optional: add a dedicated indexer unit test for sub-unity price zero-cost skip (mirror logic is present; coverage today is contract integration tests + code review).

## Verification complete — PASS Verified GitLab #470 on `main` (no repo changes; no MR). ### Acceptance criteria | Item | Result | How verified | |------|--------|--------------| | Ask side: skip fill when `floor(fill_t0 × price) = 0` while `fill_t0 > 0` | **PASS** | `match_asks` guards `cost.is_zero()` after the too-expensive shrink loop (`orderbook.rs` ~1638). Integration test `match_asks_skips_zero_cost_fill_sub_unity_price`: price=0.4, swap_in=1 → order `remaining` unchanged, maker token1 balance unchanged, `HybridSimulation` return=0. | | Bid side: symmetric zero-cost skip | **PASS** | `match_bids` same guard (~1467). Integration test `match_bids_skips_zero_cost_fill_sub_unity_price`: price=0.4, swap_in=1 → bid `remaining` unchanged, maker token0 balance unchanged. | | Simulation parity (`simulate_match_*`) | **PASS** | `simulate_match_bids` / `simulate_match_asks` both `continue` on `cost.is_zero()` (~1783, ~1892). Ask integration test asserts `sim.return_amount.is_zero()`. | | Indexer Postgres mirror (`db_orderbook_sim`) | **PASS** | `simulate_match_bids` / `simulate_match_asks` skip when `cost == 0` (~316, ~381). `cargo test --lib db_orderbook_sim` — 8/8 pass. | | No unexpected liquidity drain / honest skip semantics | **PASS** | Skip path advances `cur = order.next` without debiting escrow or crediting zero payout; taker budget (`token1_left` / `token0_left`) unchanged on skip — matches issue's recommended `continue` fix (no cost round-up). | | Docs / invariants | **PASS** | L18 in `docs/contracts-security-audit.md`; product note in `docs/limit-orders.md` § Zero-cost fill skip; agent playbook `skills/AGENTS_BOOK_MATCH_HINT_SECURITY.md` § L18 with test commands. | ### Commands run ```bash cd smartcontracts && cargo test -p cl8y-dex-tests skips_zero_cost_fill # 2/2 pass cd smartcontracts && cargo test -p cl8y-dex-pair orderbook # 38/38 pass cd indexer && cargo test --lib db_orderbook_sim # 8/8 pass make test-contracts # pass ``` ### Follow-ups - Optional: add a dedicated indexer unit test for sub-unity price zero-cost skip (mirror logic is present; coverage today is contract integration tests + code review).

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-07-07 02:15:58 +00:00

PlasticDigits commented

2026-07-07 02:21:42 +00:00

(Migrated from gitlab.com)

mentioned in merge request !1010