Implement POST /v1/swap/quote#37
Implement POST /v1/swap/quote#37findolor wants to merge 2 commits into2026-02-16-implement-order-deployfrom
Conversation
Refactor swap from flat file to module directory and implement the quote endpoint using the 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.
📝 WalkthroughWalkthroughThe PR restructures the swap routes module from a single file into a modular architecture, introduces a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as API Handler<br/>(post_swap_quote)
participant DS as SwapDataSource
participant RaindexClient
participant FloatOps as Float Operations
Client->>API: POST /v1/swap/quote
activate API
API->>DS: get_orders_for_pair(input, output)
activate DS
DS->>RaindexClient: fetch orders
RaindexClient-->>DS: Vec<RaindexOrder>
DS-->>API: Result<Vec<Order>>
deactivate DS
API->>DS: build_candidates_for_pair(orders, input, output)
activate DS
DS->>RaindexClient: build candidates from orders
RaindexClient-->>DS: Vec<TakeOrderCandidate>
DS-->>API: Result<Vec<Candidate>>
deactivate DS
API->>FloatOps: parse output_amount
FloatOps-->>API: buy_target (float)
API->>FloatOps: simulate buys over candidates
FloatOps-->>API: total_input, total_output
API->>FloatOps: compute blended ratio
FloatOps-->>API: estimated_io_ratio
API-->>Client: Json<SwapQuoteResponse>
deactivate API
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/routes/swap/calldata.rs`:
- Around line 34-37: The current handler using raindex.run_with_client(move
|_client| async move { todo!() }) will panic at runtime; either remove this
route registration from routes() until implemented, or replace the todo!() with
an async branch that returns a proper ApiError mapped to HTTP 501 Not
Implemented (so the .map_err(ApiError::from)? path still composes). Locate the
call inside calldata.rs (the raindex.run_with_client closure) and either delete
the route registration in routes() (so it is not reachable) or change the
closure to return an Err(ApiError) that represents "Not Implemented" instead of
calling todo!(). Ensure you reference raindex.run_with_client and the
surrounding .map_err(ApiError::from)? when implementing the fix.
In `@src/routes/swap/quote.rs`:
- Around line 108-114: The response currently returns the requested
output_amount instead of the actual filled amount; update SwapQuoteResponse to
include a new field (e.g., filled_amount or actual_output) and populate it where
the quote is constructed so callers can distinguish requested vs filled units
(see SwapQuoteResponse and the quote construction that returns
formatted_input/formatted_ratio); update any serializers/tests (notably
test_process_swap_quote_partial_fill) and API docs to ensure the new field is
returned and clearly documented as the actual filled amount when liquidity was
insufficient.
- Line 10: Remove the unused import of the std::ops::Div trait at the top of the
file; it is not used anywhere (the code calls the inherent method Float::div(),
which returns a Result), so delete the line `use std::ops::Div;` to clean up the
import list and fix the unused-import warning.
🧹 Nitpick comments (5)
src/routes/swap/quote.rs (2)
89-96: Consider guarding against zerototal_outputbefore division.While
sim.legs.is_empty()being false should implytotal_output > 0, a defensive check would prevent a division-by-zero error ifsimulate_buy_over_candidatesever returns legs with zero output due to a bug or edge case upstream.Suggested defensive check
if sim.legs.is_empty() { return Err(ApiError::NotFound("no valid quotes available".into())); } + // Defensive: ensure total_output is non-zero before division + let zero = Float::parse("0".to_string()).map_err(|e| { + tracing::error!(error = %e, "failed to parse zero"); + ApiError::Internal("internal error".into()) + })?; + if sim.total_output == zero { + return Err(ApiError::NotFound("no valid quotes available".into())); + } + let blended_ratio = sim.total_input.div(sim.total_output).map_err(|e| {
139-233: Solid unit test suite covering key scenarios.Consider adding edge-case tests for
output_amountvalues of"0"and negative numbers (e.g.,"-100") to verify behavior at the boundaries.src/routes/swap/mod.rs (3)
36-73: Implementation looks correct.The order filter correctly scopes to
active: truewith matching input/output tokens. Error handling consistently maps toApiError::Internalwith structured logging. The delegation tobuild_take_order_candidates_for_pairwithNone, Nonefor the optional params is clean.One note: the
Nonearguments passed tobuild_take_order_candidates_for_pair(line 66) andget_orders(line 52) are opaque without context. Brief inline comments indicating what these represent would aid readability.
93-118: Mockget_orders_for_pairdiscards the original error variant.Line 107 always returns
ApiError::Internal("failed to query orders")regardless of what error variantself.orderscontains. This means tests usingMockSwapDataSourcecan't distinguish between different error types propagated fromget_orders_for_pair.This is minor for current tests but could become a gap if you later add tests that need to verify specific error propagation (e.g.,
NotFoundvsInternal).Suggestion: propagate the actual error
async fn get_orders_for_pair( &self, _input_token: Address, _output_token: Address, ) -> Result<Vec<RaindexOrder>, ApiError> { match &self.orders { Ok(orders) => Ok(orders.clone()), - Err(_) => Err(ApiError::Internal("failed to query orders".into())), + Err(e) => Err(e.clone()), } }This requires
ApiErrorto implementClone. If that's not feasible, you could store a closure or a factory function instead.
75-76: Wildcard re-exports may leak internal items.
pub use calldata::*andpub use quote::*re-export everything public from those submodules. Currently this is fine since only the handler functions arepub, but if someone later adds apubhelper toquote.rsorcalldata.rs, it will be unintentionally exported from theswapmodule.Consider explicit re-exports for tighter API control:
-pub use calldata::*; -pub use quote::*; +pub use calldata::post_swap_calldata; +pub use quote::post_swap_quote;
| 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 a registered route.
This route is registered in routes() (see src/routes/swap/mod.rs line 79) and is reachable by any authenticated user. The todo!() macro causes an unrecoverable panic, which will crash the request handler (and potentially the worker thread depending on Rocket's configuration).
Either:
- Don't register the route until it's implemented, or
- Return a proper error response (e.g.,
501 Not Implemented).
Option 1: Remove from routes until ready
In src/routes/swap/mod.rs:
pub fn routes() -> Vec<Route> {
- rocket::routes![quote::post_swap_quote, calldata::post_swap_calldata]
+ rocket::routes![quote::post_swap_quote]
}Option 2: Return a proper error instead of panicking
raindex
- .run_with_client(move |_client| async move { todo!() })
- .await
- .map_err(ApiError::from)?
+ .run_with_client(move |_client| async move {
+ Err::<Json<SwapCalldataResponse>, ApiError>(
+ ApiError::Internal("calldata endpoint not yet implemented".into()),
+ )
+ })
+ .await
+ .map_err(ApiError::from)?🤖 Prompt for AI Agents
In `@src/routes/swap/calldata.rs` around lines 34 - 37, The current handler using
raindex.run_with_client(move |_client| async move { todo!() }) will panic at
runtime; either remove this route registration from routes() until implemented,
or replace the todo!() with an async branch that returns a proper ApiError
mapped to HTTP 501 Not Implemented (so the .map_err(ApiError::from)? path still
composes). Locate the call inside calldata.rs (the raindex.run_with_client
closure) and either delete the route registration in routes() (so it is not
reachable) or change the closure to return an Err(ApiError) that represents "Not
Implemented" instead of calling todo!(). Ensure you reference
raindex.run_with_client and the surrounding .map_err(ApiError::from)? when
implementing the fix.
There was a problem hiding this comment.
This is fine for now as we are not live yet. We are doing these one by one.
There was a problem hiding this comment.
@findolor, understood! That makes sense for incremental development. I'll note that the endpoint is pending implementation and won't flag similar patterns while you're working through the endpoints one by one.
(•ᴗ•)
| use rain_orderbook_common::take_orders::simulate_buy_over_candidates; | ||
| use rocket::serde::json::Json; | ||
| use rocket::State; | ||
| use std::ops::Div; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the quote.rs file to see the import and any usage
cat -n src/routes/swap/quote.rs | head -100Repository: ST0x-Technology/st0x.rest.api
Length of output: 4411
🏁 Script executed:
# Search for any usage of Div trait or the Div operator in quote.rs
rg -n "Div|/" src/routes/swap/quote.rs -A 2Repository: ST0x-Technology/st0x.rest.api
Length of output: 731
🏁 Script executed:
# Find the Float type definition to understand its div method
rg -rn "type Float|struct Float|impl.*Float" --type rs | head -20Repository: ST0x-Technology/st0x.rest.api
Length of output: 99
🏁 Script executed:
# Check if std::ops::Div is used anywhere in the codebase
rg -n "std::ops::Div|<.*as.*Div>" --type rsRepository: ST0x-Technology/st0x.rest.api
Length of output: 99
🏁 Script executed:
# Get the full quote.rs file
wc -l src/routes/swap/quote.rsRepository: ST0x-Technology/st0x.rest.api
Length of output: 101
🏁 Script executed:
# Read the complete file
cat src/routes/swap/quote.rsRepository: ST0x-Technology/st0x.rest.api
Length of output: 10200
🏁 Script executed:
# Search for Float type and its div method in all rust files
rg -n "impl.*Float|fn div" --type-listRepository: ST0x-Technology/st0x.rest.api
Length of output: 462
🏁 Script executed:
# Search more broadly for Float and div
rg -n "Float.*div|div.*Float"Repository: ST0x-Technology/st0x.rest.api
Length of output: 55
Remove unused std::ops::Div import.
The std::ops::Div trait is never used in this file. Line 93 calls Float::div(), which is an inherent method returning Result, not the trait method. The import can be safely removed.
🤖 Prompt for AI Agents
In `@src/routes/swap/quote.rs` at line 10, Remove the unused import of the
std::ops::Div trait at the top of the file; it is not used anywhere (the code
calls the inherent method Float::div(), which returns a Result), so delete the
line `use std::ops::Div;` to clean up the import list and fix the unused-import
warning.
| Ok(SwapQuoteResponse { | ||
| input_token: req.input_token, | ||
| output_token: req.output_token, | ||
| output_amount: req.output_amount, | ||
| estimated_input: formatted_input, | ||
| estimated_io_ratio: formatted_ratio, | ||
| }) |
There was a problem hiding this comment.
Partial fill response echoes the requested output_amount, not the actual filled amount.
When liquidity is insufficient (as shown in test_process_swap_quote_partial_fill), the response returns the requested output_amount ("100") even though only 30 units can actually be filled. The caller has no way to know that the quote only covers a fraction of the requested amount.
Consider adding a field like actual_output or filled_amount to SwapQuoteResponse, or at minimum document that output_amount is the requested amount, not a guaranteed fill.
🤖 Prompt for AI Agents
In `@src/routes/swap/quote.rs` around lines 108 - 114, The response currently
returns the requested output_amount instead of the actual filled amount; update
SwapQuoteResponse to include a new field (e.g., filled_amount or actual_output)
and populate it where the quote is constructed so callers can distinguish
requested vs filled units (see SwapQuoteResponse and the quote construction that
returns formatted_input/formatted_ratio); update any serializers/tests (notably
test_process_swap_quote_partial_fill) and API docs to ensure the new field is
returned and clearly documented as the actual filled amount when liquidity was
insufficient.
findolor
changed the title
|
Closing - already implemented in main branch. |
1 participant
Chained PRs
Motivation
See issues:
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 match the order module pattern and 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, 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 ordersFloat::max_positive_value()as price cap for quotes (no filtering — show all available liquidity)Checks
By submitting this for review, I'm confirming I've done the following:
fix #20
Summary by CodeRabbit
Release Notes
New Features
Refactor
Chores