Implement POST /v1/swap/calldata#38
Implement POST /v1/swap/calldata#38findolor wants to merge 1 commit into2026-02-16-implement-swap-quotefrom
Conversation
📝 WalkthroughWalkthroughThis PR implements the POST /v1/swap/calldata endpoint by introducing a trait-based abstraction layer for calldata generation. The changes include a new SwapCalldataDataSource trait with RaindexSwapDataSource implementation, request/response mapping logic, comprehensive test coverage, and an updated SwapCalldataRequest type to include the taker address field. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as HTTP Client
participant Handler as Route Handler
participant DS as SwapCalldataDataSource<br/>(Trait)
participant RaindexDS as RaindexSwapDataSource
participant Raindex as Raindex Service
Client->>Handler: POST /v1/swap/calldata<br/>SwapCalldataRequest
Handler->>Handler: Map to TakeOrdersRequest<br/>(chain_id, taker, mode, price_cap)
Handler->>DS: get_calldata(take_req)
DS->>RaindexDS: get_calldata(take_req)
RaindexDS->>Raindex: get_take_orders_calldata()
Raindex-->>RaindexDS: RaindexResponse or Error
RaindexDS->>RaindexDS: Map error to ApiError<br/>or construct response
RaindexDS-->>DS: SwapCalldataResponse or ApiError
DS-->>Handler: SwapCalldataResponse or ApiError
Handler-->>Client: 200 + SwapCalldataResponse<br/>or 4xx/5xx + Error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ 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: 2
🧹 Nitpick comments (1)
src/routes/swap/calldata.rs (1)
119-133: Unit tests don't verifyTakeOrdersRequestfield mapping.The
MockSwapCalldataDataSourceignores the_requestparameter, so none of the tests validate thatprocess_swap_calldatacorrectly mapstaker,chain_id,sell_token,buy_token,mode,amount, orprice_cap. The core logic of this function is the field mapping, and it's currently untested.Consider capturing the request in the mock (e.g., via
Arc<Mutex<Option<TakeOrdersRequest>>>) and asserting the mapped fields in at least one test.🤖 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 119 - 133, Update the test to capture and assert that process_swap_calldata maps fields into a TakeOrdersRequest: modify MockSwapCalldataDataSource to store the incoming request (e.g., an Arc<Mutex<Option<TakeOrdersRequest>>>), call process_swap_calldata from test_process_swap_calldata_ready as before, then lock and unwrap the stored request and assert its taker, chain_id, sell_token, buy_token, mode, amount, and price_cap match the values returned by calldata_request(); keep the existing assertions for result unchanged and reference MockSwapCalldataDataSource, process_swap_calldata, TakeOrdersRequest, test_process_swap_calldata_ready, and calldata_request when locating the code to change.
🤖 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 55-63: Replace the hardcoded chain_id value in the
TakeOrdersRequest initializer with the existing TARGET_CHAIN_ID constant: change
chain_id: 8453 to chain_id: TARGET_CHAIN_ID, and ensure TARGET_CHAIN_ID is
brought into scope (add the appropriate use/import for the tokens module where
TARGET_CHAIN_ID is defined). Keep the rest of the TakeOrdersRequest fields
unchanged so behavior remains identical.
In `@src/routes/swap/mod.rs`:
- Around line 115-129: The approval branch currently returns a
SwapCalldataResponse where top-level to is set to approval.spender() and data is
Bytes::new(), which is misleading since the client should use the approvals
array; change the top-level to to a canonical "no-op" address (e.g.,
Address::zero()) or otherwise document the convention, and ensure data remains
empty only when appropriate; additionally, update the Approval type and this
construction to use Option<String> for symbol (emit None/null when upstream
symbol is missing) instead of symbol: String::new() so the JSON response conveys
absence of a symbol.
---
Nitpick comments:
In `@src/routes/swap/calldata.rs`:
- Around line 119-133: Update the test to capture and assert that
process_swap_calldata maps fields into a TakeOrdersRequest: modify
MockSwapCalldataDataSource to store the incoming request (e.g., an
Arc<Mutex<Option<TakeOrdersRequest>>>), call process_swap_calldata from
test_process_swap_calldata_ready as before, then lock and unwrap the stored
request and assert its taker, chain_id, sell_token, buy_token, mode, amount, and
price_cap match the values returned by calldata_request(); keep the existing
assertions for result unchanged and reference MockSwapCalldataDataSource,
process_swap_calldata, TakeOrdersRequest, test_process_swap_calldata_ready, and
calldata_request when locating the code to change.
| let take_req = TakeOrdersRequest { | ||
| taker: req.taker.to_string(), | ||
| chain_id: 8453, | ||
| sell_token: req.input_token.to_string(), | ||
| buy_token: req.output_token.to_string(), | ||
| mode: TakeOrdersMode::BuyUpTo, | ||
| amount: req.output_amount, | ||
| price_cap: req.maximum_io_ratio, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'chain_id' --type rust -C2Repository: ST0x-Technology/st0x.rest.api
Length of output: 2299
🏁 Script executed:
# Check the context of the file and function signature
head -70 src/routes/swap/calldata.rs | tail -30Repository: ST0x-Technology/st0x.rest.api
Length of output: 891
🏁 Script executed:
# Look for config patterns or constants related to chain
rg -n '8453|BASE|chain|config' --type rust src/ | head -40Repository: ST0x-Technology/st0x.rest.api
Length of output: 3282
Use the existing TARGET_CHAIN_ID constant instead of hardcoding 8453.
The hardcoded chain_id: 8453 should be replaced with TARGET_CHAIN_ID (defined in src/routes/tokens.rs), which is already used for the same purpose elsewhere in the codebase. This same hardcoded pattern appears in multiple files (src/routes/order/mod.rs, src/routes/order/helpers.rs) and should be unified for maintainability.
🤖 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 55 - 63, Replace the hardcoded
chain_id value in the TakeOrdersRequest initializer with the existing
TARGET_CHAIN_ID constant: change chain_id: 8453 to chain_id: TARGET_CHAIN_ID,
and ensure TARGET_CHAIN_ID is brought into scope (add the appropriate use/import
for the tokens module where TARGET_CHAIN_ID is defined). Keep the rest of the
TakeOrdersRequest fields unchanged so behavior remains identical.
There was a problem hiding this comment.
This will be improved somewhere else.
| if let Some(approval) = result.approval_info() { | ||
| let formatted = approval.formatted_amount().to_string(); | ||
| Ok(SwapCalldataResponse { | ||
| to: approval.spender(), | ||
| data: Bytes::new(), | ||
| value: U256::ZERO, | ||
| estimated_input: formatted.clone(), | ||
| approvals: vec![crate::types::common::Approval { | ||
| token: approval.token(), | ||
| spender: approval.spender(), | ||
| amount: formatted, | ||
| symbol: String::new(), | ||
| approval_data: approval.calldata().clone(), | ||
| }], | ||
| }) |
There was a problem hiding this comment.
Approval branch: to is set to spender with empty data — verify this is the intended contract.
When an approval is needed, the top-level to is set to approval.spender() while data is empty. Since the client is expected to use the approvals array (which contains token, spender, and approval_data) to construct the approval transaction, the top-level to becomes a misleading vestige. Consider using Address::ZERO or documenting the convention that to/data are only meaningful when approvals is empty.
Also, symbol: String::new() will surface as an empty string in the JSON response — if the token symbol is unavailable from the upstream, consider using Option<String> to emit null instead.
🤖 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 115 - 129, The approval branch currently
returns a SwapCalldataResponse where top-level to is set to approval.spender()
and data is Bytes::new(), which is misleading since the client should use the
approvals array; change the top-level to to a canonical "no-op" address (e.g.,
Address::zero()) or otherwise document the convention, and ensure data remains
empty only when appropriate; additionally, update the Approval type and this
construction to use Option<String> for symbol (emit None/null when upstream
symbol is missing) instead of symbol: String::new() so the JSON response conveys
absence of a symbol.
There was a problem hiding this comment.
Intentional two-phase flow — when approvals is non-empty, the client approves first then calls again for swap calldata. to as spender signals the eventual target; empty data means "no tx yet." Address::ZERO would be less clear.
There was a problem hiding this comment.
@findolor, thank you for clarifying the design—I understand the two-phase flow now. Setting to to the spender makes sense as a signal for the eventual target when approvals are pending. I appreciate the explanation!
✏️ Learnings added
Learnt from: findolor
Repo: ST0x-Technology/st0x.rest.api PR: 38
File: src/routes/swap/mod.rs:115-129
Timestamp: 2026-02-17T09:35:57.431Z
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.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
Closing - already implemented in main branch. |
1 participant
Chained PRs
Motivation
See issues:
The swap/quote endpoint returns pricing estimates, but users need actual transaction calldata to execute swaps on-chain. This implements the follow-up calldata endpoint that delegates to
RaindexClient::get_take_orders_calldatato generate ABI-encodedtakeOrders4calldata, handling both the "ready to execute" and "needs ERC20 approval" cases.Solution
takerfield toSwapCalldataRequest(wallet address executing the tx)SwapCalldataDataSourcetrait withget_calldatamethod, implemented onRaindexSwapDataSource— delegates to the library and mapsRaindexErrorvariants to appropriateApiErrorcategories (bad request, not found, internal)process_swap_calldatahandler that builds aTakeOrdersRequest(hardcoded to chain 8453/Base,BuyUpTomode) and calls the data sourceMockSwapCalldataDataSourcefor unit testingChecks
By submitting this for review, I'm confirming I've done the following:
fix #21
Summary by CodeRabbit
New Features
Tests