code/cl8y-dex-terraclassic

Fork 0

fix(indexer): skip zero-reserve path candidates in DB hybrid route solve (#369) #890

Merged

PlasticDigits merged 2 commits from fix/indexer-369-skip-zero-reserve-path into main

2026-06-13 03:11:04 +00:00

PlasticDigits commented

2026-06-13 02:52:41 +00:00

(Migrated from gitlab.com)

Summary

Skip path candidates whose Postgres mirror sim returns InsufficientLiquidity (zero pool reserves) instead of failing the whole GET /route/solve request with 502.
Concurrent candidate evaluation continues; the best viable path wins (e.g. direct pool when a multi-hop alternate touches an unfunded pair).
Add regression test route_solve_db_hybrid_skips_zero_reserve_path_candidate and document invariant in docs/indexer-invariants.md + skills/AGENTS_INDEXER_HYBRID_BEST_EXECUTION.md.

Acceptance checklist

Item	Result	How verified
`GET /api/v1/route/solve` no longer 502s when a candidate path touches a zero-reserve pair but a direct route is viable (DB hybrid)	PASS	`cd indexer && cargo test --test api_route_solve_db_hybrid route_solve_db_hybrid_skips_zero_reserve_path_candidate -- --test-threads=1` → 200, 1-hop direct path, non-zero `estimated_amount_out`
`GET /api/v1/route/solve/best` same behavior	PASS	Same test loops both `/route/solve` and `/route/solve/best`
Existing DB hybrid regressions unaffected	PASS	`cd indexer && cargo test --test api_route_solve --test api_route_solve_db_hybrid --test db_orderbook_mirror -- --test-threads=1` → 31 passed
Hard upstream/LCD errors still fail-fast (not silently skipped)	PASS	`hybrid_sim_gateway_err` unchanged for non-`InsufficientLiquidity` errors; existing `concurrent_eval_fail_fast_aborts_remaining_tasks` unit test retained
Docs/skills updated for zero-reserve path skip invariant	PASS	`docs/indexer-invariants.md`, `skills/AGENTS_INDEXER_HYBRID_BEST_EXECUTION.md`

Test plan

route_solve_db_hybrid_skips_zero_reserve_path_candidate
Full route-solve + db-hybrid + mirror integration suite (--test-threads=1)

Note

Low Risk
Narrow indexer routing behavior change with regression tests; worst case is returning a suboptimal path or 404 when all candidates are unfunded, not silent wrong quotes on-chain.

Overview
Fixes 502 on GET /api/v1/route/solve and /route/solve/best when global best execution evaluates a multi-hop candidate that hits an indexed pair with zero pair_reserves, while a shorter funded path (e.g. direct pool) is still viable.

In best_execution.rs, concurrent path evaluation now treats DbSimError::InsufficientLiquidity like existing PathUnusable: the candidate is dropped (Ok(None)) and evaluation continues; other DB/LCD errors still map through hybrid_sim_gateway_err to 502. The winning path is the best among remaining candidates; 404 only when every path is unusable.

Adds integration test route_solve_db_hybrid_skips_zero_reserve_path_candidate, make verify-issue-369, and documents the invariant in docs/indexer-invariants.md and skills/AGENTS_INDEXER_HYBRID_BEST_EXECUTION.md.

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

## Summary - Skip path candidates whose Postgres mirror sim returns `InsufficientLiquidity` (zero pool reserves) instead of failing the whole `GET /route/solve` request with **502**. - Concurrent candidate evaluation continues; the best viable path wins (e.g. direct pool when a multi-hop alternate touches an unfunded pair). - Add regression test `route_solve_db_hybrid_skips_zero_reserve_path_candidate` and document invariant in `docs/indexer-invariants.md` + `skills/AGENTS_INDEXER_HYBRID_BEST_EXECUTION.md`. ## Acceptance checklist | Item | Result | How verified | |------|--------|--------------| | `GET /api/v1/route/solve` no longer 502s when a candidate path touches a zero-reserve pair but a direct route is viable (DB hybrid) | **PASS** | `cd indexer && cargo test --test api_route_solve_db_hybrid route_solve_db_hybrid_skips_zero_reserve_path_candidate -- --test-threads=1` → 200, 1-hop direct path, non-zero `estimated_amount_out` | | `GET /api/v1/route/solve/best` same behavior | **PASS** | Same test loops both `/route/solve` and `/route/solve/best` | | Existing DB hybrid regressions unaffected | **PASS** | `cd indexer && cargo test --test api_route_solve --test api_route_solve_db_hybrid --test db_orderbook_mirror -- --test-threads=1` → 31 passed | | Hard upstream/LCD errors still fail-fast (not silently skipped) | **PASS** | `hybrid_sim_gateway_err` unchanged for non-`InsufficientLiquidity` errors; existing `concurrent_eval_fail_fast_aborts_remaining_tasks` unit test retained | | Docs/skills updated for zero-reserve path skip invariant | **PASS** | `docs/indexer-invariants.md`, `skills/AGENTS_INDEXER_HYBRID_BEST_EXECUTION.md` | ## Test plan - [x] `route_solve_db_hybrid_skips_zero_reserve_path_candidate` - [x] Full route-solve + db-hybrid + mirror integration suite (`--test-threads=1`)  --- > [!NOTE] > **Low Risk** > Narrow indexer routing behavior change with regression tests; worst case is returning a suboptimal path or 404 when all candidates are unfunded, not silent wrong quotes on-chain. > > **Overview** > Fixes **502** on `GET /api/v1/route/solve` and `/route/solve/best` when global best execution evaluates a multi-hop candidate that hits an indexed pair with **zero `pair_reserves`**, while a shorter funded path (e.g. direct pool) is still viable. > > In **`best_execution.rs`**, concurrent path evaluation now treats **`DbSimError::InsufficientLiquidity`** like existing **`PathUnusable`**: the candidate is dropped (`Ok(None)`) and evaluation continues; other DB/LCD errors still map through **`hybrid_sim_gateway_err`** to **502**. The winning path is the best among remaining candidates; **404** only when every path is unusable. > > Adds integration test **`route_solve_db_hybrid_skips_zero_reserve_path_candidate`**, **`make verify-issue-369`**, and documents the invariant in **`docs/indexer-invariants.md`** and **`skills/AGENTS_INDEXER_HYBRID_BEST_EXECUTION.md`**. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 43853ec6a421922a514941af31789f4cce74b8a1. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>

ghost1 commented

2026-06-13 02:52:48 +00:00

(Migrated from gitlab.com)

changed the description

ghost1 commented

2026-06-13 02:52:55 +00:00

(Migrated from gitlab.com)

changed the description

PlasticDigits commented

2026-06-13 02:55:01 +00:00

(Migrated from gitlab.com)

Security review — MR !890

Commit reviewed: 22384600fb238b9ef4a63adfe097163452715ac6

Method: Traced attacker-controlled inputs (token_in, token_out, amount_in, optional trader/sender) through evaluate_candidate → optimize_multihop_hybrid_joint → is_unviable_path_sim_err → run_concurrent_candidate_evaluations → merge_candidate_evaluations / apply_fidelity_guard. Verified mirror write paths (pair_reserves via indexer book_snapshot loop only, not HTTP).

Outcome: FINDINGS: 0 medium+

Notes (not findings):

Skip is narrowly scoped to HybridSimError::Db(InsufficientLiquidity) on a fresh mirror when pool leg input is non-zero; stale/missing mirrors still LCD-fallback; LCD and other DB errors remain fail-fast (502).
Winning path still runs router maybe_simulate and existing apply_fidelity_guard — no new quote-inflation path introduced by this diff.
Suboptimal path selection if mirror reserves are wrong is a pre-existing indexer data-integrity concern, not a new attacker-controlled sink from this change.

Inline threads: none (no medium+ findings).

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

## Security review — MR !890 **Commit reviewed:** `22384600fb238b9ef4a63adfe097163452715ac6` **Scope:** DB hybrid route solve (`global_v4`) — skip path candidates whose Postgres mirror sim returns `InsufficientLiquidity` (zero pool reserves) instead of failing the whole `GET /route/solve` request with 502; map all-unviable candidate sets to 404. **Method:** Traced attacker-controlled inputs (`token_in`, `token_out`, `amount_in`, optional `trader`/`sender`) through `evaluate_candidate` → `optimize_multihop_hybrid_joint` → `is_unviable_path_sim_err` → `run_concurrent_candidate_evaluations` → `merge_candidate_evaluations` / `apply_fidelity_guard`. Verified mirror write paths (`pair_reserves` via indexer `book_snapshot` loop only, not HTTP). **Outcome:** `FINDINGS: 0` medium+ **Notes (not findings):** - Skip is narrowly scoped to `HybridSimError::Db(InsufficientLiquidity)` on a **fresh** mirror when pool leg input is non-zero; stale/missing mirrors still LCD-fallback; LCD and other DB errors remain fail-fast (502). - Winning path still runs router `maybe_simulate` and existing `apply_fidelity_guard` — no new quote-inflation path introduced by this diff. - Suboptimal path selection if mirror reserves are wrong is a pre-existing indexer data-integrity concern, not a new attacker-controlled sink from this change. **Inline threads:** none (no medium+ findings). **Prior review comments:** none on this MR; nothing to re-report.

PlasticDigits commented

2026-06-13 03:01:27 +00:00

(Migrated from gitlab.com)

added 84 commits

22384600...52748e1f - 83 commits from branch main
43853ec6 - Merge origin/main into fix/indexer-369-skip-zero-reserve-path

Compare with previous version

added 84 commits <ul><li>22384600...52748e1f - 83 commits from branch <code>main</code></li><li>43853ec6 - Merge origin/main into fix/indexer-369-skip-zero-reserve-path</li></ul> [Compare with previous version](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/172/diffs?diff_id=1850134835&start_sha=22384600fb238b9ef4a63adfe097163452715ac6)

PlasticDigits commented

2026-06-13 03:01:30 +00:00

(Migrated from gitlab.com)

Resolved merge conflicts with main by adopting main's #369 implementation (PathUnusable, pool_only_or_zero, concurrent candidate skip + fail-fast only when all paths fatal). Dropped duplicate MR-branch regression test helper; kept main's seed_route_solve_zero_reserve_poison test. cargo check passes on indexer.

Resolved merge conflicts with `main` by adopting main's #369 implementation (`PathUnusable`, `pool_only_or_zero`, concurrent candidate skip + fail-fast only when all paths fatal). Dropped duplicate MR-branch regression test helper; kept main's `seed_route_solve_zero_reserve_poison` test. `cargo check` passes on indexer.

ghost1 commented

2026-06-13 03:01:45 +00:00

(Migrated from gitlab.com)

changed the description

PlasticDigits commented

2026-06-13 03:04:46 +00:00

(Migrated from gitlab.com)

Security review — MR !890

Commit reviewed: 43853ec6a421922a514941af31789f4cce74b8a1

Scope: DB hybrid route solve (global_v4) — skip path candidates whose Postgres mirror sim returns InsufficientLiquidity (zero pool reserves) instead of failing the whole GET /route/solve with 502; return 404 when no viable candidate remains. MR head is tree-identical to base 52748e1 (merge-only); substantive logic reviewed at HEAD in best_execution.rs / hybrid_route_opt.rs (commit 2238460).

Method: Re-traced attacker-controlled inputs (token_in, token_out, amount_in, optional trader/sender) through evaluate_candidate → optimize_multihop_hybrid_joint → run_concurrent_candidate_evaluations → merge_candidate_evaluations / apply_fidelity_guard / maybe_simulate. Confirmed pair_reserves mirror writes remain indexer-only (book_snapshot loop), not HTTP-writable.

Outcome: FINDINGS: 0 medium+

Notes (not findings):

Skip is narrowly scoped to per-path HybridSimError::Db(InsufficientLiquidity) and PathUnusable; other DB/LCD errors still surface as 502 when every evaluated candidate fails fatally.
Winner selection uses router maybe_simulate out_u; apply_fidelity_guard still caps mirror/grid drift — no new quote-inflation sink.
Bounded work (MAX_PATH_CANDIDATES = 5, rate-limited /route/solve); continuing past skipped candidates is not an unbounded DoS vector.
Suboptimal path if mirror reserves are stale/wrong is a pre-existing indexer integrity concern, not attacker-controlled via this diff.

Inline threads: none (no medium+ findings).

Prior review: commit 2238460 review (2026-06-13) — findings still do not apply on current SHA; no new issues introduced.

## Security review — MR !890 **Commit reviewed:** `43853ec6a421922a514941af31789f4cce74b8a1` **Scope:** DB hybrid route solve (`global_v4`) — skip path candidates whose Postgres mirror sim returns `InsufficientLiquidity` (zero pool reserves) instead of failing the whole `GET /route/solve` with 502; return 404 when no viable candidate remains. MR head is tree-identical to base `52748e1` (merge-only); substantive logic reviewed at HEAD in `best_execution.rs` / `hybrid_route_opt.rs` (commit `2238460`). **Method:** Re-traced attacker-controlled inputs (`token_in`, `token_out`, `amount_in`, optional `trader`/`sender`) through `evaluate_candidate` → `optimize_multihop_hybrid_joint` → `run_concurrent_candidate_evaluations` → `merge_candidate_evaluations` / `apply_fidelity_guard` / `maybe_simulate`. Confirmed `pair_reserves` mirror writes remain indexer-only (`book_snapshot` loop), not HTTP-writable. **Outcome:** `FINDINGS: 0` medium+ **Notes (not findings):** - Skip is narrowly scoped to per-path `HybridSimError::Db(InsufficientLiquidity)` and `PathUnusable`; other DB/LCD errors still surface as 502 when every evaluated candidate fails fatally. - Winner selection uses router `maybe_simulate` `out_u`; `apply_fidelity_guard` still caps mirror/grid drift — no new quote-inflation sink. - Bounded work (`MAX_PATH_CANDIDATES` = 5, rate-limited `/route/solve`); continuing past skipped candidates is not an unbounded DoS vector. - Suboptimal path if mirror reserves are stale/wrong is a pre-existing indexer integrity concern, not attacker-controlled via this diff. **Inline threads:** none (no medium+ findings). **Prior review:** commit `2238460` review (2026-06-13) — findings still do not apply on current SHA; no new issues introduced.

PlasticDigits commented

2026-06-13 03:11:05 +00:00

(Migrated from gitlab.com)

mentioned in commit bfd30abba9

mentioned in commit bfd30abba9cbc3e6bc7f270387383f03182b8c7e