Fix DB-hybrid route/solve 502 when path hits zero-reserve pair (#369) #887

Merged
PlasticDigits merged 3 commits from verify/issue-369-zero-reserve-path-skip into main 2026-06-13 02:37:42 +00:00
PlasticDigits commented 2026-06-12 11:59:43 +00:00 (Migrated from gitlab.com)

Summary

  • Skip path candidates whose DB mirror sim returns InsufficientLiquidity (zero-reserve pool legs) during concurrent global best-execution evaluation, instead of fail-fast 502 "Route mirror simulation failed".
  • Return 404 when every enumerated path candidate is unusable (all hops empty).
  • Add integration test route_solve_db_hybrid_skips_zero_reserve_path_candidate covering GET /route/solve and /route/solve/best.
  • Add make verify-issue-369 and document invariant in docs/indexer-invariants.md + skills/AGENTS_INDEXER_HYBRID_BEST_EXECUTION.md.

Verification checklist

Acceptance item Command / output
Symptom: EMBER↔CORAL 502 when graph has zero-reserve pairs but direct pool viable PASS — route_solve_db_hybrid_skips_zero_reserve_path_candidate: multi-path graph with zero-reserve B↔C leg returns 200, 1-hop direct A↔C wins on both /solve and /solve/best
Root cause: InsufficientLiquidity on zero-reserve leg hard-fails whole request PASS — evaluate_candidate returns Ok(None) for skippable error; run_concurrent_candidate_evaluations continues to next candidate
Viable direct route must not be poisoned PASS — integration test asserts hops.len() == 1 and estimated_amount_out from router mock
/route/solve/best same behavior PASS — same test loops both endpoints
Regression suite PASS — make verify-issue-369 (3/3 steps)
Unit: skippable error detection PASS — cargo test skippable_hybrid_sim_error --lib
Docs / invariants PASS — docs/indexer-invariants.md + agent skill updated

Test plan

  • make verify-issue-369
  • cargo test concurrent_solve --lib
  • cargo test --test api_route_solve_db_hybrid -- --test-threads=1

Note

Low Risk
Mostly documentation and test/verify script changes; route-solver behavior is a targeted error-handling improvement already covered by integration tests.

Overview
DB-hybrid route solving no longer fails the whole GET /route/solve with 502 when one enumerated path hits an unfunded (zero-reserve) pool leg. Those candidates are dropped during concurrent global best-execution evaluation; if every path is unusable, the API returns 404 instead.

Docs and agent guidance now record that behavior (#369), including the 404 failure mode in docs/indexer-invariants.md and a dedicated section in skills/AGENTS_INDEXER_HYBRID_BEST_EXECUTION.md. make verify-issue-369 runs an extra step: the full api_route_solve_db_hybrid integration suite (alongside the zero-reserve skip test).

Separately, the Terra broadcast recovery E2E expects the swap submit button label Checking broadcast… (replacing Broadcast status unknown…) while the recovery status copy still mentions “broadcast status unknown”.

Reviewed by Cursor Bugbot for commit 75c9478f77. Bugbot is set up for automated code reviews on this repo. Configure here.

## Summary - Skip path candidates whose DB mirror sim returns `InsufficientLiquidity` (zero-reserve pool legs) during concurrent global best-execution evaluation, instead of fail-fast **502** "Route mirror simulation failed". - Return **404** when every enumerated path candidate is unusable (all hops empty). - Add integration test `route_solve_db_hybrid_skips_zero_reserve_path_candidate` covering `GET /route/solve` and `/route/solve/best`. - Add `make verify-issue-369` and document invariant in `docs/indexer-invariants.md` + `skills/AGENTS_INDEXER_HYBRID_BEST_EXECUTION.md`. ## Verification checklist | Acceptance item | Command / output | |-----------------|------------------| | Symptom: EMBER↔CORAL 502 when graph has zero-reserve pairs but direct pool viable | **PASS** — `route_solve_db_hybrid_skips_zero_reserve_path_candidate`: multi-path graph with zero-reserve B↔C leg returns **200**, 1-hop direct A↔C wins on both `/solve` and `/solve/best` | | Root cause: `InsufficientLiquidity` on zero-reserve leg hard-fails whole request | **PASS** — `evaluate_candidate` returns `Ok(None)` for skippable error; `run_concurrent_candidate_evaluations` continues to next candidate | | Viable direct route must not be poisoned | **PASS** — integration test asserts `hops.len() == 1` and `estimated_amount_out` from router mock | | `/route/solve/best` same behavior | **PASS** — same test loops both endpoints | | Regression suite | **PASS** — `make verify-issue-369` (3/3 steps) | | Unit: skippable error detection | **PASS** — `cargo test skippable_hybrid_sim_error --lib` | | Docs / invariants | **PASS** — `docs/indexer-invariants.md` + agent skill updated | ## Test plan - [x] `make verify-issue-369` - [x] `cargo test concurrent_solve --lib` - [x] `cargo test --test api_route_solve_db_hybrid -- --test-threads=1` <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Mostly documentation and test/verify script changes; route-solver behavior is a targeted error-handling improvement already covered by integration tests. > > **Overview** > **DB-hybrid route solving** no longer fails the whole `GET /route/solve` with **502** when one enumerated path hits an unfunded (zero-reserve) pool leg. Those candidates are dropped during concurrent global best-execution evaluation; if every path is unusable, the API returns **404** instead. > > Docs and agent guidance now record that behavior (**#369**), including the **404** failure mode in `docs/indexer-invariants.md` and a dedicated section in `skills/AGENTS_INDEXER_HYBRID_BEST_EXECUTION.md`. **`make verify-issue-369`** runs an extra step: the full `api_route_solve_db_hybrid` integration suite (alongside the zero-reserve skip test). > > Separately, the Terra broadcast recovery E2E expects the swap submit button label **`Checking broadcast…`** (replacing **`Broadcast status unknown…`**) while the recovery status copy still mentions “broadcast status unknown”. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 75c9478f774e822e4122fbd8dd1f31e186a9a922. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
ghost1 commented 2026-06-12 11:59:55 +00:00 (Migrated from gitlab.com)

changed the description

changed the description
ghost1 commented 2026-06-12 12:00:02 +00:00 (Migrated from gitlab.com)

changed the description

changed the description
PlasticDigits commented 2026-06-12 12:02:22 +00:00 (Migrated from gitlab.com)

Security review

Commit reviewed: 9e4582b52f82c335497655929c7d8a69229b75d2
Scope: DB-hybrid global best-execution error handling (#369) — is_skippable_hybrid_sim_error, concurrent candidate skip on DbSimError::InsufficientLiquidity, empty-merge → 404, integration test + docs/Makefile.

Method: Traced attacker-controlled inputs (token_in, token_out, amount_in, trader/sender) through evaluate_candidate → optimize_multihop_hybrid_joint → simulate_pool_leg. Confirmed skip is narrowly gated to HybridSimError::Db(InsufficientLiquidity) (zero-reserve pool leg with pool_input > 0); StaleMirror, MissingMirror, InvalidNumeric, LCD errors, and router-sim failures still fail-fast (502/400). Winning candidates unchanged: router maybe_simulate + apply_fidelity_guard still bound quoted output.

Outcome: FINDINGS: 0 medium+

Security review: no medium+ findings on this diff.

No inline threads — nothing to block on.

Prior security-review comments on this MR: none.

## Security review **Commit reviewed:** `9e4582b52f82c335497655929c7d8a69229b75d2` **Scope:** DB-hybrid global best-execution error handling (#369) — `is_skippable_hybrid_sim_error`, concurrent candidate skip on `DbSimError::InsufficientLiquidity`, empty-merge → **404**, integration test + docs/Makefile. **Method:** Traced attacker-controlled inputs (`token_in`, `token_out`, `amount_in`, `trader`/`sender`) through `evaluate_candidate` → `optimize_multihop_hybrid_joint` → `simulate_pool_leg`. Confirmed skip is narrowly gated to `HybridSimError::Db(InsufficientLiquidity)` (zero-reserve pool leg with `pool_input > 0`); `StaleMirror`, `MissingMirror`, `InvalidNumeric`, LCD errors, and router-sim failures still fail-fast (**502**/**400**). Winning candidates unchanged: router `maybe_simulate` + `apply_fidelity_guard` still bound quoted output. **Outcome:** `FINDINGS: 0` medium+ Security review: no medium+ findings on this diff. No inline threads — nothing to block on. Prior security-review comments on this MR: none.
PlasticDigits commented 2026-06-12 12:40:40 +00:00 (Migrated from gitlab.com)

added 74 commits

  • 9e4582b5...0b63e003 - 73 commits from branch main
  • c23b1938 - Merge branch 'main' into verify/issue-369-zero-reserve-path-skip

Compare with previous version

added 74 commits <ul><li>9e4582b5...0b63e003 - 73 commits from branch <code>main</code></li><li>c23b1938 - Merge branch &#39;main&#39; into verify/issue-369-zero-reserve-path-skip</li></ul> [Compare with previous version](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/169/diffs?diff_id=1849055345&start_sha=9e4582b52f82c335497655929c7d8a69229b75d2)
PlasticDigits commented 2026-06-12 12:40:46 +00:00 (Migrated from gitlab.com)

Resolved merge conflicts with main:

  • best_execution.rs: kept main's PathUnusable / InsufficientLiquidity candidate skip, zero grid_out filtering, and continue-on-partial-failure concurrent eval (supersedes MR-only is_skippable_hybrid_sim_error helper).
  • api_route_solve_db_hybrid.rs: single integration test via seed_route_solve_zero_reserve_poison; removed duplicate inline seed block from the MR branch.
  • docs/indexer-invariants.md: merged main's zero-reserve mirror wording with MR's 404 unhappy-path note and make verify-issue-369 reference.
  • verify-issue-369.sh: combined main lib checks with MR full api_route_solve_db_hybrid suite step.

Sanity: lib concurrent_solve / pool_reserves_unusable / concurrent_eval_skips pass; integration route_solve_db_hybrid_skips_zero_reserve_path_candidate pass.

Resolved merge conflicts with `main`: - **`best_execution.rs`**: kept main's `PathUnusable` / `InsufficientLiquidity` candidate skip, zero `grid_out` filtering, and continue-on-partial-failure concurrent eval (supersedes MR-only `is_skippable_hybrid_sim_error` helper). - **`api_route_solve_db_hybrid.rs`**: single integration test via `seed_route_solve_zero_reserve_poison`; removed duplicate inline seed block from the MR branch. - **`docs/indexer-invariants.md`**: merged main's zero-reserve mirror wording with MR's 404 unhappy-path note and `make verify-issue-369` reference. - **`verify-issue-369.sh`**: combined main lib checks with MR full `api_route_solve_db_hybrid` suite step. Sanity: lib `concurrent_solve` / `pool_reserves_unusable` / `concurrent_eval_skips` pass; integration `route_solve_db_hybrid_skips_zero_reserve_path_candidate` pass.
ghost1 commented 2026-06-12 12:41:01 +00:00 (Migrated from gitlab.com)

changed the description

changed the description
ghost1 commented 2026-06-12 12:43:04 +00:00 (Migrated from gitlab.com)

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.

Reviewed by Cursor Bugbot for commit c23b19383d. Configure here.

<!-- BUGBOT_REVIEW --> Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue. <!-- BUGBOT_FIX_ALL --> <a href="https://cursor.com/open?link=eyJ2ZXJzaW9uIjoxLCJ0eXBlIjoiQlVHQk9UX0ZJWF9BTExfSU5fQ1VSU09SIiwiZGF0YSI6eyJyZWRpc0tleSI6ImJ1Z2JvdC1tdWx0aTpkNmQyYzVkNi01NGU1LTRlZTktOTUyZi03ODc2YTM0OTk3MzQiLCJlbmNyeXB0aW9uS2V5IjoibEQ1WFVtMDFWYW54NVgyS3lpQndCbU9RckxhODhkb3JmZjlyOWYxZHVhdyIsImJyYW5jaCI6InZlcmlmeS9pc3N1ZS0zNjktemVyby1yZXNlcnZlLXBhdGgtc2tpcCIsInJlcG9Pd25lciI6IlBsYXN0aWNEaWdpdHMiLCJyZXBvTmFtZSI6ImNsOHktZGV4LXRlcnJhY2xhc3NpYyJ9fQ" target="_blank" rel="noopener noreferrer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/fix-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/fix-in-cursor-light.png"><img alt="Fix All in Cursor" width="115" height="28" src="https://cursor.com/assets/images/fix-in-cursor-dark.png"></picture></a> <!-- /BUGBOT_FIX_ALL --> <!-- BUGBOT_AUTOFIX_REVIEW_FOOTNOTE_BEGIN --> <sup>Bugbot Autofix is [ON](https://www.cursor.com/dashboard/bugbot). A cloud agent has been kicked off to fix the reported issue. <!-- BUGBOT_AUTOFIX_AGENT_LINK --></sup> <!-- BUGBOT_AUTOFIX_REVIEW_FOOTNOTE_END --> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c23b19383d1b681df9268fd7c0c9a78c417acedc. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>
ghost1 commented 2026-06-12 12:43:05 +00:00 (Migrated from gitlab.com)

E2E wrong recovering button label

Medium Severity

The new post-sign recovery E2E spec asserts the submit button’s accessible name is Broadcast status unknown…, but during the recovering phase terraBroadcastPendingButtonLabel sets the button copy to Checking broadcast…. The long “broadcast status unknown” text is rendered on terra-broadcast-recovery-status, not the button.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c23b19383d. Configure here.

### E2E wrong recovering button label **Medium Severity** <!-- DESCRIPTION START --> The new post-sign recovery E2E spec asserts the submit button’s accessible name is `Broadcast status unknown…`, but during the `recovering` phase `terraBroadcastPendingButtonLabel` sets the button copy to `Checking broadcast…`. The long “broadcast status unknown” text is rendered on `terra-broadcast-recovery-status`, not the button. <!-- DESCRIPTION END --> <!-- BUGBOT_BUG_ID: 44e88c9b-f691-4caa-95c6-e6559732a579 --> <!-- LOCATIONS START frontend-dapp/e2e/terra-broadcast-recovery.spec.ts#L80-L85 LOCATIONS END --> <div><a href="https://cursor.com/open?link=eyJ2ZXJzaW9uIjoxLCJ0eXBlIjoiQlVHQk9UX0ZJWF9JTl9DVVJTT1IiLCJkYXRhIjp7InJlZGlzS2V5IjoiYnVnYm90OjA4NGQ0M2VhLTc2ZTUtNDU4MC04ODhlLTIxNjc2ZjJiZTI4NyIsImVuY3J5cHRpb25LZXkiOiJ4LXE2REhjMjFCVm9BR0hsYklEaEl3RUtGbjB2bUp1SWozQ2hfOFVYTkZZIiwiYnJhbmNoIjoidmVyaWZ5L2lzc3VlLTM2OS16ZXJvLXJlc2VydmUtcGF0aC1za2lwIiwicmVwb093bmVyIjoiUGxhc3RpY0RpZ2l0cyIsInJlcG9OYW1lIjoiY2w4eS1kZXgtdGVycmFjbGFzc2ljIn19" target="_blank" rel="noopener noreferrer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/fix-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/fix-in-cursor-light.png"><img alt="Fix in Cursor" width="115" height="28" src="https://cursor.com/assets/images/fix-in-cursor-dark.png"></picture></a>&nbsp;<a href="https://cursor.com/agents?link=eyJ2ZXJzaW9uIjoxLCJ0eXBlIjoiQlVHQk9UX0ZJWF9JTl9XRUIiLCJkYXRhIjp7InJlZGlzS2V5IjoiYnVnYm90OjA4NGQ0M2VhLTc2ZTUtNDU4MC04ODhlLTIxNjc2ZjJiZTI4NyIsImVuY3J5cHRpb25LZXkiOiJ4LXE2REhjMjFCVm9BR0hsYklEaEl3RUtGbjB2bUp1SWozQ2hfOFVYTkZZIiwiYnJhbmNoIjoidmVyaWZ5L2lzc3VlLTM2OS16ZXJvLXJlc2VydmUtcGF0aC1za2lwIiwicmVwb093bmVyIjoiUGxhc3RpY0RpZ2l0cyIsInJlcG9OYW1lIjoiY2w4eS1kZXgtdGVycmFjbGFzc2ljIiwicHJOdW1iZXIiOjE2OSwiY29tbWl0U2hhIjoiYzIzYjE5MzgzZDFiNjgxZGY5MjY4ZmQ3YzBjOWE3OGM0MTdhY2VkYyIsInByb3ZpZGVyIjoiZ2l0bGFiIiwiaG9zdG5hbWUiOiJnaXRsYWIuY29tIn19" target="_blank" rel="noopener noreferrer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/fix-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/fix-in-web-light.png"><img alt="Fix in Web" width="99" height="28" src="https://cursor.com/assets/images/fix-in-web-dark.png"></picture></a></div> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c23b19383d1b681df9268fd7c0c9a78c417acedc. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>
PlasticDigits commented 2026-06-12 12:43:10 +00:00 (Migrated from gitlab.com)

Security review

Commit reviewed: c23b19383d1b681df9268fd7c0c9a78c417acedc
Scope: MR diff vs target (0b63e003…c23b193) — documentation (docs/indexer-invariants.md, skills/AGENTS_INDEXER_HYBRID_BEST_EXECUTION.md), QA harness extension (scripts/qa/verify-issue-369.sh adds full api_route_solve_db_hybrid integration run), trivial test-file whitespace. No runtime/indexer API code changes in this diff (the #369 skip logic is already on the target branch via d269d11).

Method: Reviewed all added/modified lines. Confirmed docs/QA edits do not introduce attacker-controlled sinks, secrets, injection surfaces, or authz changes. Spot-checked the already-merged #369 behavior (evaluate_candidate skips PathUnusable / Db(InsufficientLiquidity); winning quotes still pass router maybe_simulate + apply_fidelity_guard) for regressions relevant to this MR’s stated goal — not re-auditing unrelated pre-merge solver logic outside the diff.

Outcome: FINDINGS: 0 medium+

Security review: no medium+ findings on this diff.

No inline threads — nothing to block on.

Prior security-review on this MR: reviewed 9e4582b (pre-merge commit); still no medium+ findings. This pass covers current HEAD and the narrower docs/QA-only delta against target.

## Security review **Commit reviewed:** `c23b19383d1b681df9268fd7c0c9a78c417acedc` **Scope:** MR diff vs target (`0b63e003…c23b193`) — documentation (`docs/indexer-invariants.md`, `skills/AGENTS_INDEXER_HYBRID_BEST_EXECUTION.md`), QA harness extension (`scripts/qa/verify-issue-369.sh` adds full `api_route_solve_db_hybrid` integration run), trivial test-file whitespace. No runtime/indexer API code changes in this diff (the #369 skip logic is already on the target branch via `d269d11`). **Method:** Reviewed all added/modified lines. Confirmed docs/QA edits do not introduce attacker-controlled sinks, secrets, injection surfaces, or authz changes. Spot-checked the already-merged #369 behavior (`evaluate_candidate` skips `PathUnusable` / `Db(InsufficientLiquidity)`; winning quotes still pass router `maybe_simulate` + `apply_fidelity_guard`) for regressions relevant to this MR’s stated goal — not re-auditing unrelated pre-merge solver logic outside the diff. **Outcome:** `FINDINGS: 0` medium+ Security review: no medium+ findings on this diff. No inline threads — nothing to block on. **Prior security-review on this MR:** reviewed `9e4582b` (pre-merge commit); still no medium+ findings. This pass covers current HEAD and the narrower docs/QA-only delta against target.
ghost1 commented 2026-06-12 12:44:33 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 75c9478f - fix(e2e): assert recovering button label Checking broadcast…

Compare with previous version

added 1 commit <ul><li>75c9478f - fix(e2e): assert recovering button label Checking broadcast…</li></ul> [Compare with previous version](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/169/diffs?diff_id=1849063899&start_sha=c23b19383d1b681df9268fd7c0c9a78c417acedc)
ghost1 commented 2026-06-12 12:44:50 +00:00 (Migrated from gitlab.com)

changed the description

changed the description
PlasticDigits commented 2026-06-13 02:37:42 +00:00 (Migrated from gitlab.com)

mentioned in commit 83ae37217a

mentioned in commit 83ae37217a148467a154b19106ed9208ca088150
PlasticDigits (Migrated from gitlab.com) merged commit 83ae37217a into main 2026-06-13 02:37:42 +00:00
Sign in to join this conversation.
No reviewers
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!887
No description provided.