Surface fee-discount registry query failures to traders and ops #365

Closed
opened 2026-06-12 05:05:46 +00:00 by PlasticDigits · 20 comments
PlasticDigits commented 2026-06-12 05:05:46 +00:00 (Migrated from gitlab.com)

Parent

Gap analysis follow-up from GitLab #361 — register item M2 (fee-discount registry Err → silent full fee).

Current codebase

  • Pair contract lookup_effective_fee_bps_cached / lookup_effective_fee_bps_readonly in smartcontracts/contracts/pair/src/discount_cache.rs map registry query Err(_) to full pair fee with None discount metadata (lines 145, 173).
  • Integration test documents intent: swap_uses_full_fee_when_discount_registry_query_fails (smartcontracts/tests/src/lib.rs).
  • Indexer route solver depends on synced trader tiers; registry LCD outage causes quote/execute mismatch risk when combined with stale indexer tier (#361 executive summary).

Why needed

Traders with registered discounts may be charged full fee when the registry contract is paused, migrating, or temporarily unreachable — with no on-chain signal and no off-chain health indicator. This is a fairness/UX gap, not a security bug (fail-closed to full fee).

Constraints / guardrails

  • Do not change on-chain fail-open behavior in this issue without an explicit ADR and governance review (revert-on-registry-error is a separate design decision).
  • Scope: observability + off-chain surfacing; optional simulation warning in frontend.
  • Parent #361 rejected governance/timelock and audit items — stay within transparency.

Relevant files

  • smartcontracts/contracts/pair/src/discount_cache.rs
  • smartcontracts/contracts/pair/src/contract.rs
  • smartcontracts/tests/src/lib.rs (swap_uses_full_fee_when_discount_registry_query_fails)
  • indexer/src/indexer/trader_tracker.rs
  • indexer/src/api/route_solver.rs
  • frontend-dapp/src/services/terraclassic/feeDiscount.ts
  • docs/contracts-security-audit.md
  1. Indexer health: optional GET /api/v1/health/fee-discount or extend existing health with fee_discount_registry_ok (LCD probe of config query) — note: deep /health rejected for #361; keep this narrow and opt-in if needed.
  2. Frontend: when trader is registered in indexer but simulation returns full fee / no discount, show non-blocking warning (“Fee discount unavailable; full fee applied”).
  3. Docs: document fail-closed semantics for integrators in docs/contracts-security-audit.md.
  4. Optional follow-up ADR: revert swap when registry configured but query fails (not in initial scope).

Acceptance criteria

  • Documented on-chain behavior when registry query fails (fail-closed full fee).
  • Off-chain signal when registry LCD queries fail repeatedly (indexer log or API field).
  • Frontend or API consumer can distinguish “no registration” vs “registry unreachable” where data allows.
  • Existing contract test swap_uses_full_fee_when_discount_registry_query_fails still passes.

Test plan

Path Command / action
Contracts make test-contracts
Frontend unit make test-frontend (if UI warning added)
Indexer make test-indexer-integration (if health probe added)
Manual Point pair at broken registry mock / pause → verify warning path

Attack / abuse test plan

  • Health endpoint must not leak raw LCD stack traces (follow security.rs sanitization).
  • Do not expose per-trader registry errors in public API in ways that enable enumeration attacks.

Verification criteria

  • Registered trader sees warning (or ops sees alert) when registry is down; swap still succeeds at full fee.
  • No change to commission/treasury accounting on registry Err without explicit ADR.
## Parent Gap analysis follow-up from [GitLab #361](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/361) — register item **M2** (fee-discount registry `Err` → silent full fee). ## Current codebase - Pair contract `lookup_effective_fee_bps_cached` / `lookup_effective_fee_bps_readonly` in `smartcontracts/contracts/pair/src/discount_cache.rs` map registry query `Err(_)` to **full pair fee** with `None` discount metadata (lines 145, 173). - Integration test documents intent: `swap_uses_full_fee_when_discount_registry_query_fails` (`smartcontracts/tests/src/lib.rs`). - Indexer route solver depends on synced trader tiers; registry LCD outage causes quote/execute mismatch risk when combined with stale indexer tier ([#361](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/361) executive summary). ## Why needed Traders with registered discounts may be charged **full fee** when the registry contract is paused, migrating, or temporarily unreachable — with no on-chain signal and no off-chain health indicator. This is a fairness/UX gap, not a security bug (fail-closed to full fee). ## Constraints / guardrails - **Do not change on-chain fail-open behavior** in this issue without an explicit ADR and governance review (revert-on-registry-error is a separate design decision). - Scope: observability + off-chain surfacing; optional simulation warning in frontend. - Parent #361 rejected governance/timelock and audit items — stay within transparency. ## Relevant files - `smartcontracts/contracts/pair/src/discount_cache.rs` - `smartcontracts/contracts/pair/src/contract.rs` - `smartcontracts/tests/src/lib.rs` (`swap_uses_full_fee_when_discount_registry_query_fails`) - `indexer/src/indexer/trader_tracker.rs` - `indexer/src/api/route_solver.rs` - `frontend-dapp/src/services/terraclassic/feeDiscount.ts` - `docs/contracts-security-audit.md` ## Recommended direction 1. **Indexer health**: optional `GET /api/v1/health/fee-discount` or extend existing health with `fee_discount_registry_ok` (LCD probe of `config` query) — note: deep `/health` rejected for #361; keep this **narrow** and opt-in if needed. 2. **Frontend**: when trader is `registered` in indexer but simulation returns full fee / no discount, show non-blocking warning (“Fee discount unavailable; full fee applied”). 3. **Docs**: document fail-closed semantics for integrators in `docs/contracts-security-audit.md`. 4. **Optional follow-up ADR**: revert swap when registry configured but query fails (not in initial scope). ## Acceptance criteria - [ ] Documented on-chain behavior when registry query fails (fail-closed full fee). - [ ] Off-chain signal when registry LCD queries fail repeatedly (indexer log or API field). - [ ] Frontend or API consumer can distinguish “no registration” vs “registry unreachable” where data allows. - [ ] Existing contract test `swap_uses_full_fee_when_discount_registry_query_fails` still passes. ## Test plan | Path | Command / action | |------|------------------| | Contracts | `make test-contracts` | | Frontend unit | `make test-frontend` (if UI warning added) | | Indexer | `make test-indexer-integration` (if health probe added) | | Manual | Point pair at broken registry mock / pause → verify warning path | ## Attack / abuse test plan - Health endpoint must not leak raw LCD stack traces (follow `security.rs` sanitization). - Do not expose per-trader registry errors in public API in ways that enable enumeration attacks. ## Verification criteria - Registered trader sees warning (or ops sees alert) when registry is down; swap still succeeds at full fee. - No change to commission/treasury accounting on registry `Err` without explicit ADR.
PlasticDigits commented 2026-06-12 05:06:04 +00:00 (Migrated from gitlab.com)

mentioned in issue #361

mentioned in issue #361
PlasticDigits commented 2026-06-12 05:19:26 +00:00 (Migrated from gitlab.com)

mentioned in commit 8cc60b4acc

mentioned in commit 8cc60b4acc08a5b30b6a5874b9b18b2ff62a32e3
PlasticDigits commented 2026-06-12 05:19:48 +00:00 (Migrated from gitlab.com)

mentioned in merge request !874

mentioned in merge request !874
PlasticDigits commented 2026-06-12 08:34:20 +00:00 (Migrated from gitlab.com)

mentioned in commit 4d33747d7f

mentioned in commit 4d33747d7f1ccf24239f6e349b2ff394da096ca1
PlasticDigits commented 2026-06-12 11:12:57 +00:00 (Migrated from gitlab.com)

mentioned in commit d0a75849a1

mentioned in commit d0a75849a1f13b55cbd26eedb51093c498212491
PlasticDigits commented 2026-06-12 12:37:44 +00:00 (Migrated from gitlab.com)

Verification report — #365 (main @ 69c0aa1)

Overall: FAIL — observability / off-chain surfacing scope is not implemented on main. On-chain fail-closed behavior and its regression test are present; indexer health probe, frontend warning, and integrator-facing registry-outage docs are missing.

Acceptance criteria

Criterion Result How verified
Documented on-chain behavior when registry query fails (fail-closed full fee) PARTIAL docs/contracts-security-audit.md invariant P5 and residual-risk bullet cover fail-closed semantics. No dedicated integrator section on registry LCD outage vs “no registration” (recommended in issue § Docs).
Off-chain signal when registry LCD queries fail repeatedly FAIL GET /health returns only {"status":"ok"} (indexer/src/api/mod.rs:359-361). No /api/v1/health/fee-discount or fee_discount_registry_ok field. trader_tracker.rs logs per-trader tracing::warn! on tier sync LCD errors but exposes nothing via API.
Frontend/API can distinguish “no registration” vs “registry unreachable” FAIL SwapPage.tsx shows “Hold CL8Y…” only when !registrationQuery.data?.registered (lines 1356–1368). No warning when registered + simulation/full fee. feeDiscount.ts has no registry-health helper.
swap_uses_full_fee_when_discount_registry_query_fails still passes PASS cargo test swap_uses_full_fee_when_discount_registry_query_fails in cl8y-dex-tests — ok (0.04s).

Test plan

Path Command / action Result
Contracts cargo test swap_uses_full_fee_when_discount_registry_query_fails PASS
Frontend unit make test-frontend (156 files, 925 tests) PASS (no #365 UI tests — feature absent)
Indexer integration make test-indexer-integration SKIP — no health-probe code to exercise; existing api_health.rs only asserts static /health.
Manual Broken registry mock → registered trader sees warning; swap at full fee FAIL — warning path not implemented; LocalTerra not required to confirm absence.

Verification criteria (issue body)

Check Result
Registered trader sees warning (or ops alert) when registry down; swap still at full fee FAIL — no UI warning or ops API field
No change to commission/treasury accounting on registry Err without ADR PASS — discount_cache.rs:145,173 still maps Err(_) → full fee_bps; P5 test unchanged

Attack / abuse (spot check)

Check Result
Health endpoint sanitization N/A — probe not implemented
No per-trader registry errors in public API PASS — tier sync failures stay in logs only

Follow-ups

  1. Implement narrow GET /api/v1/health/fee-discount (LCD config probe, sanitized errors per security.rs).
  2. Frontend: non-blocking banner when indexer/indexer-registration says registered but LCD get_discount / sim effective fee shows no discount.
  3. Extend docs/contracts-security-audit.md (or integrators.md) with integrator guidance on fail-closed registry Err vs unregistered trader.
  4. Add make verify-issue-365 script once implemented.

Issue left open — implementation required before re-verify.

## Verification report — #365 (main @ `69c0aa1`) **Overall: FAIL** — observability / off-chain surfacing scope is not implemented on `main`. On-chain fail-closed behavior and its regression test are present; indexer health probe, frontend warning, and integrator-facing registry-outage docs are missing. ### Acceptance criteria | Criterion | Result | How verified | |-----------|--------|--------------| | Documented on-chain behavior when registry query fails (fail-closed full fee) | **PARTIAL** | `docs/contracts-security-audit.md` invariant **P5** and residual-risk bullet cover fail-closed semantics. No dedicated integrator section on registry LCD outage vs “no registration” (recommended in issue § Docs). | | Off-chain signal when registry LCD queries fail repeatedly | **FAIL** | `GET /health` returns only `{"status":"ok"}` (`indexer/src/api/mod.rs:359-361`). No `/api/v1/health/fee-discount` or `fee_discount_registry_ok` field. `trader_tracker.rs` logs per-trader `tracing::warn!` on tier sync LCD errors but exposes nothing via API. | | Frontend/API can distinguish “no registration” vs “registry unreachable” | **FAIL** | `SwapPage.tsx` shows “Hold CL8Y…” only when `!registrationQuery.data?.registered` (lines 1356–1368). No warning when registered + simulation/full fee. `feeDiscount.ts` has no registry-health helper. | | `swap_uses_full_fee_when_discount_registry_query_fails` still passes | **PASS** | `cargo test swap_uses_full_fee_when_discount_registry_query_fails` in `cl8y-dex-tests` — ok (0.04s). | ### Test plan | Path | Command / action | Result | |------|------------------|--------| | Contracts | `cargo test swap_uses_full_fee_when_discount_registry_query_fails` | **PASS** | | Frontend unit | `make test-frontend` (156 files, 925 tests) | **PASS** (no #365 UI tests — feature absent) | | Indexer integration | `make test-indexer-integration` | **SKIP** — no health-probe code to exercise; existing `api_health.rs` only asserts static `/health`. | | Manual | Broken registry mock → registered trader sees warning; swap at full fee | **FAIL** — warning path not implemented; LocalTerra not required to confirm absence. | ### Verification criteria (issue body) | Check | Result | |-------|--------| | Registered trader sees warning (or ops alert) when registry down; swap still at full fee | **FAIL** — no UI warning or ops API field | | No change to commission/treasury accounting on registry `Err` without ADR | **PASS** — `discount_cache.rs:145,173` still maps `Err(_)` → full `fee_bps`; P5 test unchanged | ### Attack / abuse (spot check) | Check | Result | |-------|--------| | Health endpoint sanitization | **N/A** — probe not implemented | | No per-trader registry errors in public API | **PASS** — tier sync failures stay in logs only | ### Follow-ups 1. Implement narrow `GET /api/v1/health/fee-discount` (LCD `config` probe, sanitized errors per `security.rs`). 2. Frontend: non-blocking banner when indexer/indexer-registration says registered but LCD `get_discount` / sim effective fee shows no discount. 3. Extend `docs/contracts-security-audit.md` (or `integrators.md`) with integrator guidance on fail-closed registry `Err` vs unregistered trader. 4. Add `make verify-issue-365` script once implemented. **Issue left open** — implementation required before re-verify.
PlasticDigits commented 2026-06-13 02:46:48 +00:00 (Migrated from gitlab.com)

All 4 followups approved, once issues created, implemented, merged, and closed rerun verification

All 4 followups approved, once issues created, implemented, merged, and closed rerun verification
PlasticDigits commented 2026-06-13 02:49:42 +00:00 (Migrated from gitlab.com)

mentioned in issue #373

mentioned in issue #373
PlasticDigits commented 2026-06-13 02:49:45 +00:00 (Migrated from gitlab.com)

mentioned in issue #374

mentioned in issue #374
PlasticDigits commented 2026-06-13 02:49:46 +00:00 (Migrated from gitlab.com)

mentioned in issue #375

mentioned in issue #375
PlasticDigits commented 2026-06-13 02:49:56 +00:00 (Migrated from gitlab.com)

Child issues opened (agent:open_issues)

Approved verification follow-ups from 2026-06-13 comment — all four items approved; bundled into 3 child issues:

Child Bundle Issue
Indexer health probe + API Follow-up 1 #373 — Indexer: narrow fee-discount registry health API
Frontend trader warning Follow-up 2 #374 — Frontend: fee-discount registry outage warning for traders
Integrator docs + verify ladder Follow-ups 3 + 4 #375 — Docs and verify-issue-365 for registry outage observability

Implementation note (main @ 87b6f9a)

Exploration after sync shows partial landing for follow-ups 1–2 (and doc updates for follow-up 3) already on main:

  • indexer/src/indexer/fee_discount_registry_health.rs, indexer/src/api/fee_discount_health.rs, indexer/tests/api_fee_discount_health.rs
  • frontend-dapp/src/utils/feeDiscountRegistryWarning.ts, SwapPage banner wiring
  • P5 / I10 / docs/indexer-invariants.md / skills/AGENTS_FEE_DISCOUNT_TIERS.md

Still missing: docs/integrators.md registry-outage subsection, scripts/qa/verify-issue-365.sh, make verify-issue-365. Child issues remain open until acceptance criteria are verified and closed; then rerun parent verification per approved workflow.

Skipped (not approved / out of scope)

Item Reason
On-chain revert when registry configured but query fails Parent § Recommended direction item 4 — not in initial scope; requires ADR + governance review
Extend generic GET /health with LCD/DB probes Rejected for #361; narrow /api/v1/health/fee-discount approved instead
Governance / timelock / audit items from #361 Parent constraints — out of scope for #365

Next step

Implement (or confirm) each child → merge → close #373–#375 → run make verify-issue-365 (once #375 lands) → rerun parent verification.

## Child issues opened (agent:open_issues) Approved verification follow-ups from [2026-06-13 comment](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/365#note_*) — all four items approved; bundled into **3** child issues: | Child | Bundle | Issue | |-------|--------|-------| | Indexer health probe + API | Follow-up **1** | [#373](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/373) — Indexer: narrow fee-discount registry health API | | Frontend trader warning | Follow-up **2** | [#374](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/374) — Frontend: fee-discount registry outage warning for traders | | Integrator docs + verify ladder | Follow-ups **3** + **4** | [#375](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/375) — Docs and verify-issue-365 for registry outage observability | ### Implementation note (`main` @ `87b6f9a`) Exploration after sync shows **partial landing** for follow-ups 1–2 (and doc updates for follow-up 3) already on `main`: - `indexer/src/indexer/fee_discount_registry_health.rs`, `indexer/src/api/fee_discount_health.rs`, `indexer/tests/api_fee_discount_health.rs` - `frontend-dapp/src/utils/feeDiscountRegistryWarning.ts`, SwapPage banner wiring - P5 / I10 / `docs/indexer-invariants.md` / `skills/AGENTS_FEE_DISCOUNT_TIERS.md` **Still missing:** `docs/integrators.md` registry-outage subsection, `scripts/qa/verify-issue-365.sh`, `make verify-issue-365`. Child issues remain open until acceptance criteria are verified and closed; then rerun parent verification per approved workflow. ### Skipped (not approved / out of scope) | Item | Reason | |------|--------| | On-chain revert when registry configured but query fails | Parent § Recommended direction item 4 — **not in initial scope**; requires ADR + governance review | | Extend generic `GET /health` with LCD/DB probes | **Rejected** for #361; narrow `/api/v1/health/fee-discount` approved instead | | Governance / timelock / audit items from #361 | Parent constraints — **out of scope** for #365 | ### Next step Implement (or confirm) each child → merge → close #373–#375 → run `make verify-issue-365` (once #375 lands) → rerun parent verification.
PlasticDigits commented 2026-06-13 03:01:45 +00:00 (Migrated from gitlab.com)

mentioned in commit b92a12ced3

mentioned in commit b92a12ced36a5b803b78fa13637005d9f017968c
PlasticDigits commented 2026-06-13 03:02:02 +00:00 (Migrated from gitlab.com)

mentioned in merge request !892

mentioned in merge request !892
PlasticDigits commented 2026-06-13 03:02:56 +00:00 (Migrated from gitlab.com)

mentioned in merge request !893

mentioned in merge request !893
Brouie commented 2026-06-13 03:07:33 +00:00 (Migrated from gitlab.com)

Re-checked on main 87b6f9a — this was a FAIL at 69c0aa1 only because the observability scope wasn't built yet. It's in now and the acceptance criteria map clean:

  • On-chain fail-closed documented: P5 in contracts-security-audit.md spells out registry-query Err → configured pair fee_bps, no discount metadata, swap still succeeds, no accounting change without an ADR — and now cites this issue plus the off-chain signal.
  • Off-chain signal: GET /api/v1/health/fee-discount is live, returns {configured, fee_discount_registry_ok, consecutive_lcd_failures}. The probe loop is actually running (spawned in the poller), 60s LCD config probe, error-logs after 2 consecutive failures. Integration tests pass, including the one asserting no LCD stack traces leak.
  • no-registration vs registry-unreachable: an API consumer can tell them apart — fee_discount_registry_ok is null when unset, false when the LCD probe is failing, true when healthy (registration itself stays a separate per-trader query).
  • Contract regression still green: swap_uses_full_fee_when_discount_registry_query_fails passes.

Sanitization holds: the endpoint only exposes booleans + a counter, no LCD detail and no per-trader data, so no enumeration surface.

The one remaining piece is the dApp non-blocking "fee discount unavailable" banner — that's the UI recommendation, not one of the ACs, and it verifies in the browser. The indexer/contract/docs scope is done.

Good to close from my side on this scope. @PlasticDigits

Re-checked on main 87b6f9a — this was a FAIL at 69c0aa1 only because the observability scope wasn't built yet. It's in now and the acceptance criteria map clean: - On-chain fail-closed documented: P5 in contracts-security-audit.md spells out registry-query Err → configured pair fee_bps, no discount metadata, swap still succeeds, no accounting change without an ADR — and now cites this issue plus the off-chain signal. - Off-chain signal: GET /api/v1/health/fee-discount is live, returns {configured, fee_discount_registry_ok, consecutive_lcd_failures}. The probe loop is actually running (spawned in the poller), 60s LCD config probe, error-logs after 2 consecutive failures. Integration tests pass, including the one asserting no LCD stack traces leak. - no-registration vs registry-unreachable: an API consumer can tell them apart — fee_discount_registry_ok is null when unset, false when the LCD probe is failing, true when healthy (registration itself stays a separate per-trader query). - Contract regression still green: swap_uses_full_fee_when_discount_registry_query_fails passes. Sanitization holds: the endpoint only exposes booleans + a counter, no LCD detail and no per-trader data, so no enumeration surface. The one remaining piece is the dApp non-blocking "fee discount unavailable" banner — that's the UI recommendation, not one of the ACs, and it verifies in the browser. The indexer/contract/docs scope is done. Good to close from my side on this scope. @PlasticDigits
PlasticDigits commented 2026-06-13 03:17:08 +00:00 (Migrated from gitlab.com)

mentioned in commit 85b2af8e5f

mentioned in commit 85b2af8e5ffadddf89594895b1a40ed893904fcc
PlasticDigits commented 2026-06-13 03:17:11 +00:00 (Migrated from gitlab.com)

mentioned in merge request !891

mentioned in merge request !891
PlasticDigits commented 2026-06-13 03:41:45 +00:00 (Migrated from gitlab.com)

mentioned in merge request !896

mentioned in merge request !896
PlasticDigits (Migrated from gitlab.com) closed this issue 2026-06-13 03:50:33 +00:00
PlasticDigits commented 2026-06-13 03:56:19 +00:00 (Migrated from gitlab.com)

mentioned in merge request !898

mentioned in merge request !898
PlasticDigits commented 2026-07-12 07:14:19 +00:00 (Migrated from gitlab.com)

mentioned in issue #476

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