Skip to content

Bounds check Solver::swap return in trade verifier#4432

Merged
squadgazzz merged 2 commits into
mainfrom
fix/trade-verifier-bounds-check
May 22, 2026
Merged

Bounds check Solver::swap return in trade verifier#4432
squadgazzz merged 2 commits into
mainfrom
fix/trade-verifier-bounds-check

Conversation

@squadgazzz
Copy link
Copy Markdown
Contributor

Description

TradeVerifier::SettleOutput::from_swap indexes the queriedBalances array returned from the on-chain Solver helper without checking its length. The function expects 2 * tokens.len() + 2 entries laid out as [...tokens_before, user_before, user_after, ...tokens_after]. When the helper returns fewer entries the indexing panics with index out of bounds: the len is 4 but the index is 4, killing the orderbook tokio worker.

The panic was observed on barn while submitting a wrapper-bearing order: POST /api/v1/orders re-runs the quoter via get_quote_and_check_fee, the verified estimator drives TradeVerifier::verify_inner and crashes inside from_swap. The container restart loop surfaces as 502s and looks like OOM in metrics.

A separate investigation is needed for why the helper returns a short response for wrapper-bearing orders (Bug B). This PR addresses only the panic so the verifier fails soft.

Changes

  • Validate queriedBalances.len() against the expected 2 * tokens.len() + 2 at the top of SettleOutput::from_swap and return a descriptive anyhow error if the shape is unexpected. Callers up the stack already surface this as Error::SimulationFailed (unverified quote) instead of panicking.

How to test

New unit tests cover both the error path (short response) and the happy path.

@squadgazzz squadgazzz added the hotfix Labels PRs that should be applied into production right away label May 22, 2026
@squadgazzz squadgazzz changed the title Bounds check Solver::swap return in trade verifier Bounds check Solver::swap return in trade verifier May 22, 2026
@squadgazzz squadgazzz marked this pull request as ready for review May 22, 2026 12:12
@squadgazzz squadgazzz requested a review from a team as a code owner May 22, 2026 12:12
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a length validation check for queriedBalances in SettleOutput::from_swap to prevent out-of-bounds panics when the solver returns an unexpected number of balances. Regression and success unit tests have been added to ensure correct error handling and normal operation. No critical issues found; I have no feedback to provide.

@squadgazzz squadgazzz added this pull request to the merge queue May 22, 2026
Merged via the queue into main with commit 9af0ec2 May 22, 2026
20 checks passed
@squadgazzz squadgazzz deleted the fix/trade-verifier-bounds-check branch May 22, 2026 14:03
@github-actions github-actions Bot locked and limited conversation to collaborators May 22, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

hotfix Labels PRs that should be applied into production right away

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants