Implement POST /v1/swap/quote with capacity-aware simulation#45
Implement POST /v1/swap/quote with capacity-aware simulation#45
Conversation
Refactor the flat swap.rs file into a swap/ module directory (mod.rs, quote.rs, calldata.rs) to match the codebase pattern and support the upcoming quote implementation. Adds async-trait, rain-math-float, and rain_orderbook_bindings dependencies.
Implement the swap quote endpoint using the rain.orderbook library's take_orders pipeline. Orders are fetched, candidates built with pair-direction filtering, then simulated via simulate_buy_over_candidates for accurate multi-leg pricing with blended IO ratio. Adds SwapDataSource trait for testability, estimated_output field for partial fills, and common test fixtures (mock_order, mock_candidate) in test_helpers. fix #20
📝 WalkthroughWalkthroughThis PR refactors the swap routes architecture by replacing a monolithic handler with modular quote and calldata endpoints, introduces a SwapDataSource trait for data abstraction, implements the POST /v1/swap/quote handler with comprehensive quote processing logic including order fetching, liquidity validation, candidate building, and swap simulation, and adds supporting test utilities and type definitions alongside four new dependencies. Changes
Sequence DiagramsequenceDiagram
participant Client as Client/HTTP
participant Handler as post_swap_quote
participant DS as SwapDataSource
participant Raindex as RaindexClient
participant Simulator as Swap Simulator
participant Response as Response Formatter
Client->>Handler: POST /v1/swap/quote
Handler->>DS: get_orders_for_pair(inputToken, outputToken)
DS->>Raindex: get_orders(filters)
Raindex-->>DS: orders[]
DS-->>Handler: orders[]
Handler->>Handler: validate_liquidity()
Handler->>DS: build_candidates_for_pair(orders)
DS->>Raindex: build_take_order_candidates_for_pair()
Raindex-->>DS: candidates[]
DS-->>Handler: candidates[]
Handler->>Handler: parse_output_amount()
Handler->>Handler: establish_price_cap()
Handler->>Simulator: run_simulation(candidates, amount, cap)
Simulator->>Simulator: evaluate_feasible_swaps()
Simulator-->>Handler: simulated_legs[]
Handler->>Response: format_output(legs)
Response-->>Handler: formatted_response
Handler-->>Client: SwapQuoteResponse (200)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/routes/swap/quote.rs (2)
53-78: Validateoutput_amountbefore making network calls.
Float::parse(req.output_amount)on Line 75 is cheap user-input validation, but it runs after two potentially expensive async calls (get_orders_for_pair+build_candidates_for_pair). Move the parse up front to fail fast on bad input.Proposed reordering
async fn process_swap_quote( ds: &dyn SwapDataSource, req: SwapQuoteRequest, ) -> Result<SwapQuoteResponse, ApiError> { + let buy_target = Float::parse(req.output_amount.clone()).map_err(|e| { + tracing::error!(error = %e, "failed to parse output_amount"); + ApiError::BadRequest("invalid output_amount".into()) + })?; + let orders = ds .get_orders_for_pair(req.input_token, req.output_token) .await?; if orders.is_empty() { return Err(ApiError::NotFound( "no liquidity found for this pair".into(), )); } let candidates = ds .build_candidates_for_pair(&orders, req.input_token, req.output_token) .await?; if candidates.is_empty() { return Err(ApiError::NotFound("no valid quotes available".into())); } - let buy_target = Float::parse(req.output_amount.clone()).map_err(|e| { - tracing::error!(error = %e, "failed to parse output_amount"); - ApiError::BadRequest("invalid output_amount".into()) - })?; - let price_cap = Float::max_positive_value().map_err(|e| {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/swap/quote.rs` around lines 53 - 78, The input validation for output_amount is performed too late in process_swap_quote: move the Float::parse(req.output_amount.clone()) call (and its map_err producing ApiError::BadRequest) to the very start of process_swap_quote before invoking the async calls get_orders_for_pair and build_candidates_for_pair so we fail fast on invalid user input; keep the same error mapping and variable name (e.g., buy_target) and only proceed to call ds.get_orders_for_pair and ds.build_candidates_for_pair after successful parsing.
90-97: Edge case: division whentotal_outputis zero.If
simulate_buy_over_candidatesreturns legs but with a zerototal_output(e.g., due to floating-point edge cases),sim.total_input.div(sim.total_output)would produce an error or infinity. Themap_erron Line 94 handles this gracefully, but consider adding an explicit guard for zero output with a more descriptive error message.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/swap/quote.rs` around lines 90 - 97, Add an explicit guard before computing blended_ratio: check sim.total_output for zero (or use its is_zero method if it's a floating type) after simulate_buy_over_candidates returns but before calling sim.total_input.div(sim.total_output); if zero, return an ApiError with a clear message like "simulation produced zero total output" (use the same ApiError variant pattern you prefer) instead of attempting the division, and keep the existing map_err fallback for other division errors so blended_ratio computation only runs when sim.total_output is non‑zero.src/routes/swap/mod.rs (1)
91-117:MockSwapDataSourcediscards the original error fromorders.Line 105 always returns a fresh
ApiError::Internal("failed to query orders")regardless of what error variant was stored. This is fine for current tests, but if you later need to assert on specific error messages or variants propagated from the data source, consider cloning or forwarding the stored error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/swap/mod.rs` around lines 91 - 117, The mock currently drops the original error stored in MockSwapDataSource.orders and always returns ApiError::Internal("failed to query orders"); update get_orders_for_pair to propagate the stored error instead of creating a new one — when matching Err(e) return Err(e.clone()) (or otherwise clone/forward the stored ApiError from self.orders) so tests can assert on the original error variant/message; the change targets the get_orders_for_pair method and the orders field of MockSwapDataSource.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/swap/calldata.rs`:
- Around line 35-38: The closure passed to raindex.run_with_client currently
contains todo!() which will panic at runtime; replace the todo!() with a proper
error return instead of panicking (for example return an ApiError representing
501 Not Implemented or another appropriate ApiError variant) so the call to
raindex.run_with_client(...).await.map_err(ApiError::from)? yields a handled
error rather than a panic; alternatively remove the route registration that
references this handler in routes() until you implement the function.
---
Nitpick comments:
In `@src/routes/swap/mod.rs`:
- Around line 91-117: The mock currently drops the original error stored in
MockSwapDataSource.orders and always returns ApiError::Internal("failed to query
orders"); update get_orders_for_pair to propagate the stored error instead of
creating a new one — when matching Err(e) return Err(e.clone()) (or otherwise
clone/forward the stored ApiError from self.orders) so tests can assert on the
original error variant/message; the change targets the get_orders_for_pair
method and the orders field of MockSwapDataSource.
In `@src/routes/swap/quote.rs`:
- Around line 53-78: The input validation for output_amount is performed too
late in process_swap_quote: move the Float::parse(req.output_amount.clone())
call (and its map_err producing ApiError::BadRequest) to the very start of
process_swap_quote before invoking the async calls get_orders_for_pair and
build_candidates_for_pair so we fail fast on invalid user input; keep the same
error mapping and variable name (e.g., buy_target) and only proceed to call
ds.get_orders_for_pair and ds.build_candidates_for_pair after successful
parsing.
- Around line 90-97: Add an explicit guard before computing blended_ratio: check
sim.total_output for zero (or use its is_zero method if it's a floating type)
after simulate_buy_over_candidates returns but before calling
sim.total_input.div(sim.total_output); if zero, return an ApiError with a clear
message like "simulation produced zero total output" (use the same ApiError
variant pattern you prefer) instead of attempting the division, and keep the
existing map_err fallback for other division errors so blended_ratio computation
only runs when sim.total_output is non‑zero.
| raindex | ||
| .run_with_client(move |_client| async move { todo!() }) | ||
| .await | ||
| .map_err(ApiError::from)? |
There was a problem hiding this comment.
todo!() will panic at runtime on any request to this registered endpoint.
This endpoint is wired into routes() in mod.rs (Line 79) and documented in OpenAPI, so authenticated clients can reach it. The todo!() macro causes an unrecoverable panic, which will crash the request handler. Either return a proper ApiError (e.g., 501 Not Implemented) or remove the route from routes() until the implementation is ready.
Proposed fix: return a proper error instead of panicking
raindex
- .run_with_client(move |_client| async move { todo!() })
+ .run_with_client(move |_client| async move {
+ Err::<Json<SwapCalldataResponse>, ApiError>(ApiError::Internal(
+ "swap calldata endpoint not yet implemented".into(),
+ ))
+ })
.await
.map_err(ApiError::from)?Alternatively, remove the route from the router until it's implemented:
pub fn routes() -> Vec<Route> {
- rocket::routes![quote::post_swap_quote, calldata::post_swap_calldata]
+ rocket::routes![quote::post_swap_quote]
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/swap/calldata.rs` around lines 35 - 38, The closure passed to
raindex.run_with_client currently contains todo!() which will panic at runtime;
replace the todo!() with a proper error return instead of panicking (for example
return an ApiError representing 501 Not Implemented or another appropriate
ApiError variant) so the call to
raindex.run_with_client(...).await.map_err(ApiError::from)? yields a handled
error rather than a panic; alternatively remove the route registration that
references this handler in routes() until you implement the function.
There was a problem hiding this comment.
Don't worry we'll be implementing this downstream.
There was a problem hiding this comment.
@findolor, understood! I'll leave this for your future implementation. 👍
🧠 Learnings used
Learnt from: findolor
Repo: ST0x-Technology/st0x.rest.api PR: 38
File: src/routes/swap/mod.rs:115-129
Timestamp: 2026-02-17T09:36:00.616Z
Learning: In the swap calldata endpoint (Rust, `src/routes/swap/mod.rs`), when the response includes a non-empty `approvals` array, the two-phase flow is intentional: the client performs the approval transaction first, then calls the endpoint again to retrieve the actual swap calldata. In the approval response, `to` is set to `approval.spender()` to signal the eventual target, and `data` is `Bytes::new()` (empty) to indicate "no transaction yet." This design is preferred over using `Address::ZERO` for clarity.
Motivation
See issues:
This is a standalone reimplementation of PR #37, which was originally part of a chained PR sequence (dependent on the order deploy PR chain). This PR carries the same functionality but is rebased directly onto
mainso it can be reviewed and merged independently.The swap quote endpoint was stubbed with
todo!(). Users need a way to get a price quote before executing a swap, showing the estimated input cost and IO ratio for a desired output amount. The flatswap.rsfile also needed to be refactored into a module directory to support the upcoming calldata implementation.Solution
Refactored
swap.rsinto aswap/module directory (mod.rs,quote.rs,calldata.rs) and implemented the quote endpoint using the rain.orderbook library's take_orders pipeline:SwapDataSourcetrait withget_orders_for_pairandbuild_candidates_for_pair— the latter delegates to the library'sbuild_take_order_candidates_for_pair, which fetches quotes in parallel, filters by pair direction, and removes failed/zero-capacity quotessimulate_buy_over_candidatesfor capacity-aware multi-leg pricing — sorts candidates by best price, greedily fills across multiple orders, and returns accuratetotal_input/total_outputtotal_input / total_outputacross all simulation legs, giving an accurate average price even when liquidity is split across ordersestimated_outputfield added toSwapQuoteResponseto surface partial fills when available liquidity is less than the requested amountFloat::max_positive_value()as price cap for quotes (no filtering — show all available liquidity)mock_order,mock_candidate) moved totest_helpers.rsfor reuse across modulesChecks
By submitting this for review, I'm confirming I've done the following:
fix #20
Summary by CodeRabbit
Release Notes
New Features
/v1/swap/quoteendpoint to retrieve swap quotes with estimated output and pricing metrics/v1/swap/calldataendpoint to generate swap transaction calldataestimated_outputfieldTests
Chores