code/cl8y-dex-terraclassic

Fork 0

300s fee-discount cache lets a wallet hold the tier balance one block per 5 minutes #275

New issue

Closed

opened 2026-06-03 07:11:52 +00:00 by Brouie · 8 comments

Brouie commented

2026-06-03 07:11:52 +00:00

(Migrated from gitlab.com)

Severity: Medium
Reachability: Any registered wallet that wants the discount without holding the CL8Y.
Affected: pair fee-discount cache (smartcontracts/contracts/pair/src/discount_cache.rs), 300s TTL.
Root cause: the pair caches the registry's discount answer for 300s and serves it without re-checking live balance, so the "hold CL8Y to get the discount" requirement only has to be true for one block every 5 minutes.

Summary

The discount registry reads live CL8Y balance at query time and returns needs_deregister: true once a wallet drops below its tier. Good. But the pair memoizes that answer for DISCOUNT_CACHE_TTL_SECONDS = 300, keyed on (trader, sender), and during that window it returns the cached discount without going back to the registry.

So the intended invariant ("you must hold the tier balance while you trade") degrades to "you must hold it for one block per 300 seconds":

Acquire enough CL8Y to clear the tier.
Do one tiny swap → pair queries the registry → sees the balance → caches the discount for 300s.
Move the CL8Y away.
For the next 5 minutes every real swap serves the cached discount with zero CL8Y held.
Refresh once per 300s.

It's fee leakage, not theft, but it undercuts the CL8Y demand sink the discount is supposed to create. Calling it Medium.

Current codebase

discount_cache.rs is_fresh (block_time - cached_at < 300), try_discount_cache_hit returns the cached (effective_fee_bps, discount) without re-querying.
smartcontracts/contracts/fee-discount/src/contract.rs query_discount does query live balance — so the hole is the pair-side cache window, not the registry.

Recommended direction

Shorten the TTL hard, or scope the cache so it can't span a balance-changing window cheaply.
Better: invalidate / bypass the cache for the discount decision and only cache the expensive lookup plumbing, so the balance gate stays per-trade. (The cache was added for gas in #251 — there's a real trade-off to weigh here.)
At minimum, document that the discount is a 5-minute snapshot, not a live check, so it's a conscious choice.

Acceptance criteria

A wallet that drops below its tier cannot keep claiming the discount for a meaningful window without holding the balance.
The economic cost of farming the discount with brief balance flashes is no better than holding the balance.

Test plan (attack / abuse)

case	expect
register, tiny swap, drain CL8Y, large swap within 300s	large swap does NOT get the discount
flash balance once per window across many swaps	no sustained discount without sustained balance

**Severity:** Medium **Reachability:** Any registered wallet that wants the discount without holding the CL8Y. **Affected:** pair fee-discount cache (`smartcontracts/contracts/pair/src/discount_cache.rs`), 300s TTL. **Root cause:** the pair caches the registry's discount answer for 300s and serves it without re-checking live balance, so the "hold CL8Y to get the discount" requirement only has to be true for one block every 5 minutes. ## Summary The discount registry reads **live** CL8Y balance at query time and returns `needs_deregister: true` once a wallet drops below its tier. Good. But the pair memoizes that answer for `DISCOUNT_CACHE_TTL_SECONDS = 300`, keyed on `(trader, sender)`, and during that window it returns the cached discount without going back to the registry. So the intended invariant ("you must hold the tier balance while you trade") degrades to "you must hold it for one block per 300 seconds": 1. Acquire enough CL8Y to clear the tier. 2. Do one tiny swap → pair queries the registry → sees the balance → caches the discount for 300s. 3. Move the CL8Y away. 4. For the next 5 minutes every real swap serves the cached discount with zero CL8Y held. 5. Refresh once per 300s. It's fee leakage, not theft, but it undercuts the CL8Y demand sink the discount is supposed to create. Calling it Medium. ## Current codebase - `discount_cache.rs` `is_fresh` (`block_time - cached_at < 300`), `try_discount_cache_hit` returns the cached `(effective_fee_bps, discount)` without re-querying. - `smartcontracts/contracts/fee-discount/src/contract.rs` `query_discount` does query live balance — so the hole is the pair-side cache window, not the registry. ## Recommended direction 1. Shorten the TTL hard, or scope the cache so it can't span a balance-changing window cheaply. 2. Better: invalidate / bypass the cache for the discount decision and only cache the expensive lookup plumbing, so the balance gate stays per-trade. (The cache was added for gas in #251 — there's a real trade-off to weigh here.) 3. At minimum, document that the discount is a 5-minute snapshot, not a live check, so it's a conscious choice. ## Acceptance criteria - [ ] A wallet that drops below its tier cannot keep claiming the discount for a meaningful window without holding the balance. - [ ] The economic cost of farming the discount with brief balance flashes is no better than holding the balance. ## Test plan (attack / abuse) | case | expect | |---|---| | register, tiny swap, drain CL8Y, large swap within 300s | large swap does NOT get the discount | | flash balance once per window across many swaps | no sustained discount without sustained balance |

PlasticDigits commented

2026-06-03 10:32:03 +00:00

(Migrated from gitlab.com)

The invariant needs to be updated. The reason for the 300s cache is to reduce gas costs for frequent traders. This is a necessary gas optimization, and requiring a trader to buy/sell every 300s to maintain their status still provides sufficient benefit given the gas optimization advantages.

Brouie commented

2026-06-04 06:18:34 +00:00

(Migrated from gitlab.com)

Took your call — kept the 300s cache as-is (gas optimization, #251) and updated the invariant to the snapshot model.

Documented in three places:

discount_cache.rs module doc — the cached (effective_fee_bps, discount) is a 300s snapshot, not a live per-trade check; tier holder must transact ≥ once per 300s to keep it; a wallet dropping below tier keeps the cached discount until the entry expires; deliberate gas optimization vs a per-swap registry query; the registry itself still reads live balance.
DISCOUNT_CACHE_TTL_SECONDS const doc (dex-common/pair.rs) — same snapshot note + #275 ref.
New P9 row in the security-audit invariant matrix (docs/contracts-security-audit.md) stating the 300s snapshot is accepted design, fee leakage bounded to one window, the once-per-window refresh deemed sufficient CL8Y-sink demand.

Doc-only, no logic change. Branch qa/275-fee-discount-cache-invariant-doc, MR fork→main (no closing keyword).

One thing for you: the issue's acceptance criteria as written ("a wallet that drops below its tier cannot keep claiming the discount", "no better than holding") can't be satisfied under this decision — they describe the live-check model you've declined. They should be rewritten to the snapshot model (e.g. "discount documented as a 300s snapshot; refresh requirement is once per window") before this closes. Left that for you since it's your AC. @PlasticDigits

Took your call — kept the 300s cache as-is (gas optimization, #251) and updated the invariant to the snapshot model. Documented in three places: - `discount_cache.rs` module doc — the cached `(effective_fee_bps, discount)` is a **300s snapshot**, not a live per-trade check; tier holder must transact ≥ once per 300s to keep it; a wallet dropping below tier keeps the cached discount until the entry expires; deliberate gas optimization vs a per-swap registry query; the registry itself still reads live balance. - `DISCOUNT_CACHE_TTL_SECONDS` const doc (dex-common/pair.rs) — same snapshot note + #275 ref. - New **P9** row in the security-audit invariant matrix (`docs/contracts-security-audit.md`) stating the 300s snapshot is accepted design, fee leakage bounded to one window, the once-per-window refresh deemed sufficient CL8Y-sink demand. Doc-only, no logic change. Branch `qa/275-fee-discount-cache-invariant-doc`, MR fork→main (no closing keyword). One thing for you: the issue's **acceptance criteria** as written ("a wallet that drops below its tier cannot keep claiming the discount", "no better than holding") can't be satisfied under this decision — they describe the live-check model you've declined. They should be rewritten to the snapshot model (e.g. "discount documented as a 300s snapshot; refresh requirement is once per window") before this closes. Left that for you since it's your AC. @PlasticDigits

Brouie commented

2026-06-04 06:18:37 +00:00

(Migrated from gitlab.com)

mentioned in merge request !747

PlasticDigits commented

2026-06-04 08:14:40 +00:00

(Migrated from gitlab.com)

mentioned in commit 38bd253138

mentioned in commit 38bd253138c5cca532f5cef771487ed7b38e7da7

Brouie commented

2026-06-05 01:21:09 +00:00

(Migrated from gitlab.com)

One thing that's yours before this closes: the original acceptance criteria ("a wallet that drops below its tier cannot keep claiming the discount", "no better than holding") describe the live-check model you declined — they can't be met under the snapshot decision, so they read as failing even though the resolution is exactly what you asked for. They need rewriting to the snapshot model so the issue closes clean against its own AC. @PlasticDigits

Took your call — kept the 300s cache (the #251 gas optimization) and moved the invariant to the snapshot model, doc-only, no logic change. Verified the docs landed in all three spots: the `discount_cache.rs` module doc, the `DISCOUNT_CACHE_TTL_SECONDS` const doc, and the P9 row in the security-audit matrix — all stating the cached discount is a 300s snapshot, the tier holder must transact at least once per window, a wallet dropping below tier keeps the cached discount until expiry, and the registry still reads live balance. The cache mechanism matches the doc (the discount_cache TTL-boundary and post-registry-upgrade tests pass). One thing that's yours before this closes: the original acceptance criteria ("a wallet that drops below its tier cannot keep claiming the discount", "no better than holding") describe the live-check model you declined — they can't be met under the snapshot decision, so they read as failing even though the resolution is exactly what you asked for. They need rewriting to the snapshot model so the issue closes clean against its own AC. @PlasticDigits

PlasticDigits commented

2026-06-05 03:46:40 +00:00

(Migrated from gitlab.com)

Approved to ovverride acceptance criteria

PlasticDigits commented

2026-06-05 03:49:58 +00:00

(Migrated from gitlab.com)

Verification summary (GitLab #275)

Verified on main @ 9f0babe (includes merge qa/275-fee-discount-cache-invariant-doc / 942bae9). No additional code or doc changes were required in this pass.

Decision / overridden acceptance criteria

Per @PlasticDigits (2026-06-05): keep the 300s on-pair cache (#251 gas optimization) and treat the discount as a 300s snapshot, not a live per-trade balance gate. Original issue AC (live-check / “no better than holding”) are superseded by this decision.

Criterion	Result	How verified
Discount documented as a 300s snapshot (not live per-trade check)	PASS	`discount_cache.rs` module doc; `DISCOUNT_CACHE_TTL_SECONDS` doc in `dex-common/src/pair.rs`; P9 row in `docs/contracts-security-audit.md` — all cite #275 and describe bounded fee leakage + registry still reads live balance
Tier holder must transact ≥ once per 300s to refresh cache	PASS	Same three docs; code `is_fresh`: `block_time - cached_at < DISCOUNT_CACHE_TTL_SECONDS`
No logic change required for resolution	PASS	`git log --grep=275` shows doc-only merge; working tree clean
Cache behavior matches documentation	PASS	`cargo test discount_cache` (3 tests): TTL boundary, cache hit after registry upgrade within TTL, hybrid sim matches execute
Security matrix / coverage	PASS	P9 links `discount_cache.rs` + TTL const; `cargo test fee_discount_coverage` (11 tests)
Tier table drift guard	PASS	`make check-fee-discount-tier-docs`
Original AC: drop tier → no discount within 300s	SKIP	Superseded by snapshot model (intentional bounded leakage)
Original AC: flash-farming no better than holding	SKIP	Superseded; once-per-window refresh is accepted CL8Y-sink trade-off
Original attack table: drain CL8Y, large swap within 300s → no discount	SKIP	Under accepted design, cached discount does apply until expiry — matches docs/tests
Full stack (postgres / anvil / indexer / frontend / bot swarm)	SKIP	Out of scope for doc-only invariant closure; pair cache is on-chain only

Commands run

glab issue view 275 -R PlasticDigits/cl8y-dex-terraclassic --comments
rg '#275|DISCOUNT_CACHE_TTL|snapshot' smartcontracts docs skills
cargo test discount_cache          # smartcontracts/
cargo test fee_discount_coverage   # smartcontracts/
make check-fee-discount-tier-docs

Cross-links

I9 (docs/reference/fee-discount-tiers.md) and agent skills (AGENTS_FEE_DISCOUNT_TIERS.md, AGENTS_HYBRID_QUOTING.md, AGENTS_TERRACLASSIC_GAS.md) already describe the 300s cache via #251 / I9; authoritative accepted-design statement is P9 + pair source docs above.
.cursor/skills/ and skills/README.md are not present in this repo (N/A).

Closing as verified — documentation and tests align with the approved snapshot invariant.

## Verification summary (GitLab #275) Verified on `main` @ `9f0babe` (includes merge `qa/275-fee-discount-cache-invariant-doc` / `942bae9`). No additional code or doc changes were required in this pass. ### Decision / overridden acceptance criteria Per [@PlasticDigits](https://gitlab.com/PlasticDigits) (2026-06-05): **keep the 300s on-pair cache** (#251 gas optimization) and treat the discount as a **300s snapshot**, not a live per-trade balance gate. Original issue AC (live-check / “no better than holding”) are **superseded** by this decision. | Criterion | Result | How verified | |-----------|--------|------------| | Discount documented as a **300s snapshot** (not live per-trade check) | **PASS** | `discount_cache.rs` module doc; `DISCOUNT_CACHE_TTL_SECONDS` doc in `dex-common/src/pair.rs`; **P9** row in `docs/contracts-security-audit.md` — all cite #275 and describe bounded fee leakage + registry still reads live balance | | Tier holder must **transact ≥ once per 300s** to refresh cache | **PASS** | Same three docs; code `is_fresh`: `block_time - cached_at < DISCOUNT_CACHE_TTL_SECONDS` | | **No logic change** required for resolution | **PASS** | `git log --grep=275` shows doc-only merge; working tree clean | | Cache behavior matches documentation | **PASS** | `cargo test discount_cache` (3 tests): TTL boundary, cache hit after registry upgrade within TTL, hybrid sim matches execute | | Security matrix / coverage | **PASS** | P9 links `discount_cache.rs` + TTL const; `cargo test fee_discount_coverage` (11 tests) | | Tier table drift guard | **PASS** | `make check-fee-discount-tier-docs` | | Original AC: drop tier → no discount within 300s | **SKIP** | Superseded by snapshot model (intentional bounded leakage) | | Original AC: flash-farming no better than holding | **SKIP** | Superseded; once-per-window refresh is accepted CL8Y-sink trade-off | | Original attack table: drain CL8Y, large swap within 300s → no discount | **SKIP** | Under accepted design, cached discount **does** apply until expiry — matches docs/tests | | Full stack (postgres / anvil / indexer / frontend / bot swarm) | **SKIP** | Out of scope for doc-only invariant closure; pair cache is on-chain only | ### Commands run ```bash glab issue view 275 -R PlasticDigits/cl8y-dex-terraclassic --comments rg '#275|DISCOUNT_CACHE_TTL|snapshot' smartcontracts docs skills cargo test discount_cache # smartcontracts/ cargo test fee_discount_coverage # smartcontracts/ make check-fee-discount-tier-docs ``` ### Cross-links - **I9** (`docs/reference/fee-discount-tiers.md`) and agent skills (`AGENTS_FEE_DISCOUNT_TIERS.md`, `AGENTS_HYBRID_QUOTING.md`, `AGENTS_TERRACLASSIC_GAS.md`) already describe the 300s cache via #251 / I9; authoritative accepted-design statement is **P9** + pair source docs above. - `.cursor/skills/` and `skills/README.md` are not present in this repo (N/A). Closing as verified — documentation and tests align with the approved snapshot invariant.

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-06-05 03:50:02 +00:00

PlasticDigits commented

2026-06-29 00:21:34 +00:00

(Migrated from gitlab.com)

mentioned in issue #424