code/cl8y-dex-terraclassic

Fork 0

fix(indexer): skip zero-reserve paths in DB-hybrid route/solve (#369) #880

Merged

PlasticDigits merged 2 commits from verify/issue-369-zero-reserve-route-solve into main

2026-06-12 11:01:44 +00:00

PlasticDigits commented

2026-06-12 08:06:28 +00:00

(Migrated from gitlab.com)

Summary

DB-hybrid mirror sim treats zero-reserve pool legs as unusable (returns 0 output, counts mirror_missing) instead of propagating InsufficientLiquidity as a 502 for the whole request.
Route solve skips failed or zero-output path candidates and returns the best viable path; responds 404 only when no candidate survives.
Regression integration test, lib tests, make verify-issue-369, and skills/AGENTS_HYBRID_QUOTING.md cross-link.

Verification checklist

Acceptance item	Command / result
`GET /api/v1/route/solve` no longer 502 when a longer candidate touches a zero-reserve pair while a direct pool is healthy	`make verify-issue-369` → integration `route_solve_db_hybrid_skips_zero_reserve_candidate_path` PASS (200, 1-hop direct path, `estimated_amount_out > 0`)
Both swap directions (token_in/token_out reversed)	Same test loops A→C and C→A PASS
`GET /api/v1/route/solve/best` same behavior	Same test hits `/route/solve/best` both directions PASS
Zero-reserve hop does not trigger per-pair LCD hybrid fallback	`hybrid_hits == 0` in test PASS
Empty pool reserve detection (unit)	`cargo test pool_reserves_unusable --lib` PASS
Concurrent solve skips poisoned candidates instead of fail-fast 502	`cargo test concurrent_eval_skips --lib` PASS
Existing DB-hybrid suite unaffected	`cargo test --test api_route_solve_db_hybrid -- --test-threads=1` PASS (4/4)

Follow-ups

Confirm whether production/QA indexer should redeploy after merge; pairs 152/205 on QA may still have stale zero rows until the next successful book_snapshot.
Consider a metric or log counter for skipped zero-reserve candidates to spot unfunded pairs in the graph.

Note

Medium Risk
Changes core DB-hybrid route solver behavior (candidate skipping and error handling); regression coverage is strong but wrong filtering could hide valid multihop routes or return suboptimal paths.

Overview
Fixes GitLab #369: DB-hybrid GET /route/solve and /route/solve/best no longer fail the whole request with 502 when an alternate candidate path hits a zero-reserve pair while a healthy direct route exists.

Hybrid simulation treats empty pool mirrors as unusable: pool_reserves_unusable gates mirror legs (counts mirror_missing, returns 0 output) instead of bubbling InsufficientLiquidity. Pool-only fallbacks use pool_only_or_zero so empty pools yield zero rather than hard errors.

Concurrent candidate evaluation skips failed evaluations and DB-mode candidates with grid_out == 0, then continues with remaining paths (replacing fail-fast abort_all). 404 copy becomes "no viable route" when every candidate is dropped.

Adds integration test route_solve_db_hybrid_skips_zero_reserve_candidate_path, make verify-issue-369, and hybrid quoting doc cross-link.

^{Reviewed by Cursor Bugbot for commit faf127ce0a. Bugbot is set up for automated code reviews on this repo. Configure here.}

## Summary - DB-hybrid mirror sim treats zero-reserve pool legs as unusable (returns 0 output, counts `mirror_missing`) instead of propagating `InsufficientLiquidity` as a 502 for the whole request. - Route solve skips failed or zero-output path candidates and returns the best viable path; responds 404 only when no candidate survives. - Regression integration test, lib tests, `make verify-issue-369`, and `skills/AGENTS_HYBRID_QUOTING.md` cross-link. ## Verification checklist | Acceptance item | Command / result | |-----------------|------------------| | `GET /api/v1/route/solve` no longer 502 when a longer candidate touches a zero-reserve pair while a direct pool is healthy | `make verify-issue-369` → integration `route_solve_db_hybrid_skips_zero_reserve_candidate_path` **PASS** (200, 1-hop direct path, `estimated_amount_out > 0`) | | Both swap directions (token_in/token_out reversed) | Same test loops A→C and C→A **PASS** | | `GET /api/v1/route/solve/best` same behavior | Same test hits `/route/solve/best` both directions **PASS** | | Zero-reserve hop does not trigger per-pair LCD hybrid fallback | `hybrid_hits == 0` in test **PASS** | | Empty pool reserve detection (unit) | `cargo test pool_reserves_unusable --lib` **PASS** | | Concurrent solve skips poisoned candidates instead of fail-fast 502 | `cargo test concurrent_eval_skips --lib` **PASS** | | Existing DB-hybrid suite unaffected | `cargo test --test api_route_solve_db_hybrid -- --test-threads=1` **PASS** (4/4) | ## Follow-ups - Confirm whether production/QA indexer should redeploy after merge; pairs 152/205 on QA may still have stale zero rows until the next successful `book_snapshot`. - Consider a metric or log counter for skipped zero-reserve candidates to spot unfunded pairs in the graph.  --- > [!NOTE] > **Medium Risk** > Changes core DB-hybrid route solver behavior (candidate skipping and error handling); regression coverage is strong but wrong filtering could hide valid multihop routes or return suboptimal paths. > > **Overview** > Fixes **GitLab #369**: DB-hybrid **`GET /route/solve`** and **`/route/solve/best`** no longer fail the whole request with **502** when an alternate candidate path hits a **zero-reserve** pair while a healthy direct route exists. > > **Hybrid simulation** treats empty pool mirrors as unusable: **`pool_reserves_unusable`** gates mirror legs (counts **`mirror_missing`**, returns **0** output) instead of bubbling **`InsufficientLiquidity`**. Pool-only fallbacks use **`pool_only_or_zero`** so empty pools yield zero rather than hard errors. > > **Concurrent candidate evaluation** skips failed evaluations and **DB-mode** candidates with **`grid_out == 0`**, then continues with remaining paths (replacing fail-fast **`abort_all`**). **404** copy becomes **"no viable route"** when every candidate is dropped. > > Adds integration test **`route_solve_db_hybrid_skips_zero_reserve_candidate_path`**, **`make verify-issue-369`**, and hybrid quoting doc cross-link. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit faf127ce0ad7addc71e225ca8b3adad7a15d380b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>

ghost1 commented

2026-06-12 08:06:37 +00:00

(Migrated from gitlab.com)

changed the description

ghost1 commented

2026-06-12 08:06:44 +00:00

(Migrated from gitlab.com)

changed the description

PlasticDigits commented

2026-06-12 08:09:32 +00:00

(Migrated from gitlab.com)

Security review — MR !880

Commit reviewed: 2d46702d237baea7ae0d36f180206c33d101e912

Scope: DB-hybrid route/solve error handling — zero-reserve mirror detection (pool_reserves_unusable, MissingReserves), pool_only_or_zero hop simulation, concurrent candidate skip-and-continue (best_execution.rs), integration tests and QA script.

Method: Traced attacker-controlled inputs (token_in, token_out, amount_in, optional trader/sender) through GET /api/v1/route/solve and /route/solve/best into hybrid grid simulation, router re-simulation (maybe_simulate), and apply_fidelity_guard. Checked for injection, authz bypass, quote inflation, SSRF/LCD abuse, and secret leakage in new paths.

Outcome: FINDINGS: 0 medium+

Inline threads: None (no medium+ issues on this diff).

Notes (informational, below reporting threshold)

Zero-reserve hops now return Ok(0) and skip LCD fallback by design; this is conservative (underestimates, does not inflate quotes) and is covered by the integration test asserting hybrid_hits == 0.
Failed per-candidate evaluations are skipped instead of fail-fast 502; surviving candidates still pass router simulation and the existing poisoned-mirror fidelity guard (db_out vs sim_u drift clamp).
db_mode candidates with grid_out == 0 are excluded before winner selection, preventing zero-output paths from being returned as viable quotes.
No new auth surface, SQL/string concatenation, deserialization of untrusted types, or logging of secrets introduced by this diff.

Prior review comments: None on this MR; nothing to re-report.

## Security review — MR !880 **Commit reviewed:** `2d46702d237baea7ae0d36f180206c33d101e912` **Scope:** DB-hybrid route/solve error handling — zero-reserve mirror detection (`pool_reserves_unusable`, `MissingReserves`), `pool_only_or_zero` hop simulation, concurrent candidate skip-and-continue (`best_execution.rs`), integration tests and QA script. **Method:** Traced attacker-controlled inputs (`token_in`, `token_out`, `amount_in`, optional `trader`/`sender`) through `GET /api/v1/route/solve` and `/route/solve/best` into hybrid grid simulation, router re-simulation (`maybe_simulate`), and `apply_fidelity_guard`. Checked for injection, authz bypass, quote inflation, SSRF/LCD abuse, and secret leakage in new paths. **Outcome:** `FINDINGS: 0` medium+ **Inline threads:** None (no medium+ issues on this diff). ### Notes (informational, below reporting threshold) - Zero-reserve hops now return `Ok(0)` and skip LCD fallback by design; this is conservative (underestimates, does not inflate quotes) and is covered by the integration test asserting `hybrid_hits == 0`. - Failed per-candidate evaluations are skipped instead of fail-fast 502; surviving candidates still pass router simulation and the existing poisoned-mirror fidelity guard (`db_out` vs `sim_u` drift clamp). - `db_mode` candidates with `grid_out == 0` are excluded before winner selection, preventing zero-output paths from being returned as viable quotes. - No new auth surface, SQL/string concatenation, deserialization of untrusted types, or logging of secrets introduced by this diff. **Prior review comments:** None on this MR; nothing to re-report.

PlasticDigits commented

2026-06-12 08:33:53 +00:00

(Migrated from gitlab.com)

added 32 commits

2d46702d...62050542 - 31 commits from branch main
faf127ce - Merge branch 'main' into verify/issue-369-zero-reserve-route-solve

Compare with previous version

added 32 commits <ul><li>2d46702d...62050542 - 31 commits from branch <code>main</code></li><li>faf127ce - Merge branch 'main' into verify/issue-369-zero-reserve-route-solve</li></ul> [Compare with previous version](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/162/diffs?diff_id=1848545109&start_sha=2d46702d237baea7ae0d36f180206c33d101e912)

PlasticDigits commented

2026-06-12 08:33:58 +00:00

(Migrated from gitlab.com)

Resolved merge conflicts with main in four indexer files:

hybrid_route_opt.rs — kept pool_only_or_zero (#369) with main's infra-LCD guards and PathUnusable match arms
best_execution.rs — merged main's Option<CandidateEval> skip path with #369 zero grid_out filtering and "no viable route" 404 text
db_orderbook_sim.rs — kept both pool_reserves_unusable and zero-reserve mirror unit tests
api_route_solve_db_hybrid.rs — doc comment only

Sanity: cargo check, pool_reserves_unusable lib test, concurrent_eval_skips lib test — all pass.

Resolved merge conflicts with `main` in four indexer files: - **hybrid_route_opt.rs** — kept `pool_only_or_zero` (#369) with main's infra-LCD guards and `PathUnusable` match arms - **best_execution.rs** — merged main's `Option<CandidateEval>` skip path with #369 zero `grid_out` filtering and "no viable route" 404 text - **db_orderbook_sim.rs** — kept both `pool_reserves_unusable` and zero-reserve mirror unit tests - **api_route_solve_db_hybrid.rs** — doc comment only Sanity: `cargo check`, `pool_reserves_unusable` lib test, `concurrent_eval_skips` lib test — all pass.

ghost1 commented

2026-06-12 08:34:08 +00:00

(Migrated from gitlab.com)

changed the description

PlasticDigits commented

2026-06-12 08:36:38 +00:00

(Migrated from gitlab.com)

Security review — MR !880

Commit reviewed: faf127ce0ad7addc71e225ca8b3adad7a15d380b

Scope: DB-hybrid route/solve error handling (#369) after merge with main — pool_reserves_unusable / EmptyPool mirror freshness, pool_only_or_zero hop simulation, zero grid_out candidate filtering, concurrent skip-and-continue evaluation (best_execution.rs), PathUnusable candidate skip from main, integration tests and QA script.

Method: Re-traced attacker-controlled inputs (token_in, token_out, amount_in, optional trader/sender) through GET /api/v1/route/solve and /route/solve/best into hybrid grid simulation, router re-simulation (maybe_simulate), and apply_fidelity_guard. Compared post-merge diff (2d46702d → faf127ce) for regressions from conflict resolution. Checked injection, authz bypass, quote inflation, SSRF/LCD abuse, and secret leakage.

Outcome: FINDINGS: 0 medium+

Inline threads: None (no medium+ issues on this diff).

Security review: no medium+ findings on this diff.

Notes (informational, below reporting threshold)

Zero-reserve hops return Ok(0) and skip LCD fallback by design; conservative (underestimates, does not inflate quotes). Integration test asserts hybrid_hits == 0.
Failed per-candidate evaluations are skipped instead of fail-fast 502; surviving candidates still pass router simulation and the poisoned-mirror fidelity guard (db_out vs sim_u drift clamp).
db_mode candidates with grid_out == 0 are excluded before winner selection.
Merge with main added PathUnusable → Ok(None) skip and infra-LCD error guards; no new attack surface identified.
No new auth surface, SQL/string concatenation, deserialization of untrusted types, or logging of secrets in this diff.

Prior review: 2d46702d (2026-06-12) reached the same conclusion; re-reviewed after merge-conflict resolution — findings unchanged.

## Security review — MR !880 **Commit reviewed:** `faf127ce0ad7addc71e225ca8b3adad7a15d380b` **Scope:** DB-hybrid route/solve error handling (#369) after merge with `main` — `pool_reserves_unusable` / `EmptyPool` mirror freshness, `pool_only_or_zero` hop simulation, zero `grid_out` candidate filtering, concurrent skip-and-continue evaluation (`best_execution.rs`), `PathUnusable` candidate skip from `main`, integration tests and QA script. **Method:** Re-traced attacker-controlled inputs (`token_in`, `token_out`, `amount_in`, optional `trader`/`sender`) through `GET /api/v1/route/solve` and `/route/solve/best` into hybrid grid simulation, router re-simulation (`maybe_simulate`), and `apply_fidelity_guard`. Compared post-merge diff (`2d46702d` → `faf127ce`) for regressions from conflict resolution. Checked injection, authz bypass, quote inflation, SSRF/LCD abuse, and secret leakage. **Outcome:** `FINDINGS: 0` medium+ **Inline threads:** None (no medium+ issues on this diff). Security review: no medium+ findings on this diff. ### Notes (informational, below reporting threshold) - Zero-reserve hops return `Ok(0)` and skip LCD fallback by design; conservative (underestimates, does not inflate quotes). Integration test asserts `hybrid_hits == 0`. - Failed per-candidate evaluations are skipped instead of fail-fast 502; surviving candidates still pass router simulation and the poisoned-mirror fidelity guard (`db_out` vs `sim_u` drift clamp). - `db_mode` candidates with `grid_out == 0` are excluded before winner selection. - Merge with `main` added `PathUnusable` → `Ok(None)` skip and infra-LCD error guards; no new attack surface identified. - No new auth surface, SQL/string concatenation, deserialization of untrusted types, or logging of secrets in this diff. **Prior review:** `2d46702d` (2026-06-12) reached the same conclusion; re-reviewed after merge-conflict resolution — findings unchanged.