Fix DB-hybrid route/solve 502 when path hits zero-reserve pair (#369) #887
No reviewers
Labels
No labels
agent:fix_bugfix
agent:fix_conflicts
agent:fix_security
agent:gap_analysis
agent:implement
agent:implement
agent:implement
agent:open_issues
agent:ready
agent:research
agent:security_audit
agent:verify
architecture
backend
blocker:hybrid
blocker:launch
blocker:limit-orders
blocker:v2
block:log_only
block:security
bug
ci
contracts
correctness
deploy
dev
devops
docs
documentation
duplicate
e2e
enhancement
epic
feature
frontend
functional-completion
gas
good first issue
governance
help wanted
high-risk
hooks
hybrid
indexer
infra
infrastructure
integrators
invalid
launch-blocker
limit-orders
localnet
localterra
low priority
missing-implementation
needs-design
ops
performance
priority
high
priority
medium
product
qa
QA
question
ready
ready
research
scripts
security
security-hardening
smartcontracts
tech-debt
testing
ux
UX
v2
verification
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
code/cl8y-dex-terraclassic!887
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "verify/issue-369-zero-reserve-path-skip"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
InsufficientLiquidity(zero-reserve pool legs) during concurrent global best-execution evaluation, instead of fail-fast 502 "Route mirror simulation failed".route_solve_db_hybrid_skips_zero_reserve_path_candidatecoveringGET /route/solveand/route/solve/best.make verify-issue-369and document invariant indocs/indexer-invariants.md+skills/AGENTS_INDEXER_HYBRID_BEST_EXECUTION.md.Verification checklist
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/solveand/solve/bestInsufficientLiquidityon zero-reserve leg hard-fails whole requestevaluate_candidatereturnsOk(None)for skippable error;run_concurrent_candidate_evaluationscontinues to next candidatehops.len() == 1andestimated_amount_outfrom router mock/route/solve/bestsame behaviormake verify-issue-369(3/3 steps)cargo test skippable_hybrid_sim_error --libdocs/indexer-invariants.md+ agent skill updatedTest plan
make verify-issue-369cargo test concurrent_solve --libcargo test --test api_route_solve_db_hybrid -- --test-threads=1changed the description
changed the description
Security review
Commit reviewed:
9e4582b52f82c335497655929c7d8a69229b75d2Scope: DB-hybrid global best-execution error handling (#369) —
is_skippable_hybrid_sim_error, concurrent candidate skip onDbSimError::InsufficientLiquidity, empty-merge → 404, integration test + docs/Makefile.Method: Traced attacker-controlled inputs (
token_in,token_out,amount_in,trader/sender) throughevaluate_candidate→optimize_multihop_hybrid_joint→simulate_pool_leg. Confirmed skip is narrowly gated toHybridSimError::Db(InsufficientLiquidity)(zero-reserve pool leg withpool_input > 0);StaleMirror,MissingMirror,InvalidNumeric, LCD errors, and router-sim failures still fail-fast (502/400). Winning candidates unchanged: routermaybe_simulate+apply_fidelity_guardstill bound quoted output.Outcome:
FINDINGS: 0medium+Security review: no medium+ findings on this diff.
No inline threads — nothing to block on.
Prior security-review comments on this MR: none.
added 74 commits
mainc23b1938- Merge branch 'main' into verify/issue-369-zero-reserve-path-skipCompare with previous version
Resolved merge conflicts with
main:best_execution.rs: kept main'sPathUnusable/InsufficientLiquiditycandidate skip, zerogrid_outfiltering, and continue-on-partial-failure concurrent eval (supersedes MR-onlyis_skippable_hybrid_sim_errorhelper).api_route_solve_db_hybrid.rs: single integration test viaseed_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 andmake verify-issue-369reference.verify-issue-369.sh: combined main lib checks with MR fullapi_route_solve_db_hybridsuite step.Sanity: lib
concurrent_solve/pool_reserves_unusable/concurrent_eval_skipspass; integrationroute_solve_db_hybrid_skips_zero_reserve_path_candidatepass.changed the description
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
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.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 therecoveringphaseterraBroadcastPendingButtonLabelsets the button copy toChecking broadcast…. The long “broadcast status unknown” text is rendered onterra-broadcast-recovery-status, not the button.Reviewed by Cursor Bugbot for commit
c23b19383d. Configure here.Security review
Commit reviewed:
c23b19383d1b681df9268fd7c0c9a78c417acedcScope: 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.shadds fullapi_route_solve_db_hybridintegration run), trivial test-file whitespace. No runtime/indexer API code changes in this diff (the #369 skip logic is already on the target branch viad269d11).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_candidateskipsPathUnusable/Db(InsufficientLiquidity); winning quotes still pass routermaybe_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: 0medium+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.added 1 commit
75c9478f- fix(e2e): assert recovering button label Checking broadcast…Compare with previous version
changed the description
mentioned in commit
83ae37217a