feat(trades): add token trades endpoint#105
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds GET /v1/trades/token/{address} with pagination and auth, extends TradesDataSource with token-based queries, refactors shared pagination/response helpers used by trades handlers, registers the new route in OpenAPI, updates test mocks, and changes DCA detection to use OrderBuilderStateV1 metadata. ChangesTrades-by-Token Route
Order Type Classification
Sequence Diagram(s)sequenceDiagram
participant Client
participant RocketHandler
participant RaindexTradesDataSource
participant RaindexClient
Client->>RocketHandler: GET /v1/trades/token/{address} + Authorization + pagination
RocketHandler->>RaindexTradesDataSource: get_trades_for_token(token, page, page_size, time_filter)
RaindexTradesDataSource->>RaindexClient: query trades with GetTradesFilters + page/page_size
RaindexClient-->>RaindexTradesDataSource: RaindexTradesListResult
RaindexTradesDataSource-->>RocketHandler: mapped trades list
RocketHandler-->>Client: 200 JSON (TradesByAddressResponse)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 docstrings
🧪 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 |
How to use the Graphite Merge QueueAdd the label add-to-gt-merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/order/get_order.rs (1)
70-83: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd test coverage for DCA order type detection.
The DCA detection logic was modified but lacks test coverage. The only existing test (
test_determine_order_type_solver_default) verifies the fallback case, not the newOrderBuilderStateV1variant matching.📝 Suggested test additions
Add tests to verify DCA detection with various deployment names:
#[rocket::async_test] async fn test_determine_order_type_dca_lowercase() { let mut order = mock_order(); // Add OrderBuilderStateV1 metadata with "dca" in selected_deployment // (This requires extending the mock to include parsed_meta) assert_eq!(determine_order_type(&order), OrderType::Dca); } #[rocket::async_test] async fn test_determine_order_type_dca_uppercase() { let mut order = mock_order(); // Add OrderBuilderStateV1 metadata with "DCA" in selected_deployment assert_eq!(determine_order_type(&order), OrderType::Dca); } #[rocket::async_test] async fn test_determine_order_type_dca_mixed_case() { let mut order = mock_order(); // Add OrderBuilderStateV1 metadata with "my-DCA-strategy" in selected_deployment assert_eq!(determine_order_type(&order), OrderType::Dca); } #[rocket::async_test] async fn test_determine_order_type_non_dca() { let mut order = mock_order(); // Add OrderBuilderStateV1 metadata with "limit-order" in selected_deployment assert_eq!(determine_order_type(&order), OrderType::Solver); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/order/get_order.rs` around lines 70 - 83, Add unit tests for determine_order_type to cover the new OrderBuilderStateV1 branch: create mock orders (using mock_order) whose parsed_meta() returns a ParsedMeta::OrderBuilderStateV1 with builder_state.selected_deployment set to values containing "dca" in lowercase, uppercase, and mixed case (e.g., "dca", "DCA", "my-DCA-strategy") and assert determine_order_type(&order) == OrderType::Dca; also add a negative test where selected_deployment is non‑matching (e.g., "limit-order") and assert OrderType::Solver. Ensure tests reference determine_order_type, RaindexOrder, ParsedMeta::OrderBuilderStateV1, and OrderType so they exercise the new detection logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/routes/order/get_order.rs`:
- Around line 70-83: Add unit tests for determine_order_type to cover the new
OrderBuilderStateV1 branch: create mock orders (using mock_order) whose
parsed_meta() returns a ParsedMeta::OrderBuilderStateV1 with
builder_state.selected_deployment set to values containing "dca" in lowercase,
uppercase, and mixed case (e.g., "dca", "DCA", "my-DCA-strategy") and assert
determine_order_type(&order) == OrderType::Dca; also add a negative test where
selected_deployment is non‑matching (e.g., "limit-order") and assert
OrderType::Solver. Ensure tests reference determine_order_type, RaindexOrder,
ParsedMeta::OrderBuilderStateV1, and OrderType so they exercise the new
detection logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6d605ad3-7b27-4b83-becc-fdfe7237c1af
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
lib/rain.orderbooksrc/main.rssrc/routes/order/get_order.rssrc/routes/trades/get_by_address.rssrc/routes/trades/get_by_token.rssrc/routes/trades/get_by_tx.rssrc/routes/trades/mod.rs
## Chained PRs - Stacked on [#106](#106) for the taker trades endpoint. - Also follows [#105](#105) for the token trades endpoint. ## Dependent PRs - Depends on [rainlanguage/raindex#2572](rainlanguage/raindex#2572) ([Graphite](https://app.graphite.com/github/pr/rainlanguage/raindex/2572)) for SDK `getTradesByOrderHashes` support, local DB order-hash batching, and grouped results. This PR should not merge until that raindex PR lands and the submodule pointer is mergeable. ## Summary - Adds `POST /v1/trades/query` for batch order-hash trade lookup. - Defines request/response DTOs for `orderHashes` input and `tradesByOrderHash` grouped output. - Includes every requested hash in the response through the SDK grouping behavior, including hashes with no trades. - Validates order hashes with existing API error handling and returns `400` for invalid hash input. - Wires the route through the existing trades module, auth, rate limiting, tracing span propagation, and OpenAPI registration. - Uses the raindex SDK batch order-hash API directly; there is no API-side query workaround. - Reuses the shared trade list mapper for each grouped trade entry. - Bumps the `rain.orderbook` submodule to the SDK commit with batch order-hash trade lookup support. Linear: RAI-528 ## Behavior The new endpoint accepts a JSON body: ```json { "orderHashes": [ "0x4a051ad4567935a5a0570b3e2e77714c44405bb58e5b83e3cc484de1cee0747e" ], "startTime": 1718452800, "endTime": 1718539200 } ``` It returns trades grouped by requested order hash: ```json { "tradesByOrderHash": [ { "orderHash": "0x4a051ad4567935a5a0570b3e2e77714c44405bb58e5b83e3cc484de1cee0747e", "trades": [] } ], "totalCount": 0 } ``` ## Local DB Smoke Test After clearing the local raindex DB and waiting for the Base local source to become ready, the endpoint used local DB only: ```text local_chain_ids_count=1 subgraph_chain_ids_count=0 ``` Single-order smoke test: - Request: one real order hash plus one zero hash. - Result: real hash returned `50` trades; zero hash returned an empty `trades` array. - HTTP total: `0.415652s`. - SDK local fetch: `29ms`; SDK total: `161ms`. Heavy response stress test using the highest-trade order hashes in the local DB: - Top 10 order hashes: `3,790` trades, `2.12 MB` response, HTTP total `11.20s`. - Top 20 order hashes: `4,596` trades, `2.57 MB` response, HTTP total `13.50s`. - Top 20 SQL fetch was `930ms`; full SDK grouping/materialization was `13211ms`; full API request was `13495ms`. The heavy case is dominated by materializing, grouping, mapping, and serializing thousands of full trade objects rather than by SQL lookup. ## Notes - The `unimplemented!()` additions are test-only mock methods required by the expanded `TradesDataSource` trait; they are not production route code. - Empty `orderHashes` currently returns an empty grouped result, matching the SDK behavior. ## Verification - `nix develop -c cargo fmt` - `nix develop -c cargo check` - `nix develop -c cargo test routes::trades::get_by_order_hashes` - `nix develop -c cargo test` - `nix develop -c rainix-rs-static` `rainix-rs-static` passes with pre-existing dead-code warnings in `src/cache.rs`.
Merge activity
|
## Dependent PRs - Depends on [rainlanguage/raindex#2570](rainlanguage/raindex#2570) for SDK `getTrades` token filtering support. This PR should not merge until that raindex PR lands and the submodule pointer is mergeable. ## Summary - Adds `GET /v1/trades/token/{address}` for paginated trades involving a token address. - Wires the new route through the existing `TradesDataSource` abstraction and the raindex SDK `get_trades` token filters. - Reuses the shared trades list response shape used by owner-based trades, including pagination metadata and time filtering. - Refactors the owner trades route to share pagination and trade mapping helpers with the new token route. - Updates trade test mocks for the expanded `TradesDataSource` trait. - Updates order type detection for the raindex parsed meta variant rename from `DotrainGuiStateV1` to `OrderBuilderStateV1`. - Bumps the `rain.orderbook` submodule to the SDK commit with token-filtered `getTrades` support. Linear: RAI-526 ## Behavior The new endpoint accepts the same pagination/time query parameters as `GET /v1/trades/{address}`: ```text GET /v1/trades/token/{tokenAddress}?page=1&pageSize=20&startTime=...&endTime=... ``` It returns trades where the token appears on either side of the trade by passing the token as both an input-token and output-token filter to the SDK. ## Notes - The current raindex local DB query path is still the baseline implementation. In local benchmarking, high-volume token queries are around four seconds for `pageSize=50`; materialized trade read tables should be handled as a separate SDK/local DB optimization. - The `unimplemented!()` additions are test-only mock methods required by the expanded `TradesDataSource` trait; they are not production route code. ## Verification - `nix develop -c cargo check` - `nix develop -c cargo test` - `nix develop -c cargo fmt` - `nix develop -c rainix-rs-static` `rainix-rs-static` passes with pre-existing dead-code warnings in `src/cache.rs`.
500f191 to
54e38c6
Compare
## Chained PRs - Stacked on [#105](#105) for the token trades endpoint. ## Dependent PRs - Depends on [rainlanguage/raindex#2571](rainlanguage/raindex#2571) for SDK `getTrades` taker filtering, local DB predicate pushdown, sender-time indexes, and local query tracing. This PR should not merge until that raindex PR lands and the submodule pointer is mergeable. ## Summary - Adds `GET /v1/trades/taker/{address}` for paginated trades by taker address. - Wires the endpoint through the existing trades route module, OpenAPI registration, auth, rate limiting, tracing span propagation, and shared trades response mapping. - Uses the raindex SDK `get_trades` taker filter directly with pagination and time filtering; there is no API-side query workaround. - Clones the shared `RaindexClient` before awaiting the SDK call so the raindex provider read lock is not held during fetch/count work. - Adds focused tests for successful taker queries, empty results, SDK errors, auth failure, invalid address handling, and route registration. - Bumps the `rain.orderbook` submodule to the SDK commit with taker-filtered `getTrades` support and local DB query tracing. Linear: RAI-530 ## Behavior The new endpoint accepts the same pagination/time query parameters as the existing trade list endpoints: ```text GET /v1/trades/taker/{takerAddress}?page=1&pageSize=20&startTime=...&endTime=... ``` It returns trades whose taker/sender matches the path address and includes the shared trade list pagination metadata. ## Local DB Smoke Test After resetting the local raindex DB and waiting for `/health/detailed` to report Base as active and ready, the endpoint used local DB only: ```text local_chain_ids_count=1 subgraph_chain_ids_count=0 ``` Measured locally against taker `0x55f3412d51bbe48255a286189848a151236c0307`: - `pageSize=3`: 3 rows, `totalTrades=1676`, HTTP total `1.596s`; SDK fetch `667ms`, count `662ms`, total `1343ms`. - `pageSize=50`: 50 rows, `totalTrades=1676`, HTTP total `1.726s`; SDK fetch `675ms`, count `651ms`, total `1477ms`. ## Verification - `nix develop -c cargo check` - `nix develop -c cargo test routes::trades::get_by_taker` - `nix develop -c cargo test` - `nix develop -c cargo fmt` - `nix develop -c rainix-rs-static` `rainix-rs-static` passes with pre-existing dead-code warnings in `src/cache.rs`. <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/ST0x-Technology/codesmith/st0x.rest.api/pr/106"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-light.svg"><img alt="View in Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-in-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith</code> with what you need.</sup> - [ ] Let Codesmith autofix CI failures and bot reviews <!-- /codesmith:footer -->
## Chained PRs - Stacked on [#106](#106) for the taker trades endpoint. - Also follows [#105](#105) for the token trades endpoint. ## Dependent PRs - Depends on [rainlanguage/raindex#2572](rainlanguage/raindex#2572) ([Graphite](https://app.graphite.com/github/pr/rainlanguage/raindex/2572)) for SDK `getTradesByOrderHashes` support, local DB order-hash batching, and grouped results. This PR should not merge until that raindex PR lands and the submodule pointer is mergeable. ## Summary - Adds `POST /v1/trades/query` for batch order-hash trade lookup. - Defines request/response DTOs for `orderHashes` input and `tradesByOrderHash` grouped output. - Includes every requested hash in the response through the SDK grouping behavior, including hashes with no trades. - Validates order hashes with existing API error handling and returns `400` for invalid hash input. - Wires the route through the existing trades module, auth, rate limiting, tracing span propagation, and OpenAPI registration. - Uses the raindex SDK batch order-hash API directly; there is no API-side query workaround. - Reuses the shared trade list mapper for each grouped trade entry. - Bumps the `rain.orderbook` submodule to the SDK commit with batch order-hash trade lookup support. Linear: RAI-528 ## Behavior The new endpoint accepts a JSON body: ```json { "orderHashes": [ "0x4a051ad4567935a5a0570b3e2e77714c44405bb58e5b83e3cc484de1cee0747e" ], "startTime": 1718452800, "endTime": 1718539200 } ``` It returns trades grouped by requested order hash: ```json { "tradesByOrderHash": [ { "orderHash": "0x4a051ad4567935a5a0570b3e2e77714c44405bb58e5b83e3cc484de1cee0747e", "trades": [] } ], "totalCount": 0 } ``` ## Local DB Smoke Test After clearing the local raindex DB and waiting for the Base local source to become ready, the endpoint used local DB only: ```text local_chain_ids_count=1 subgraph_chain_ids_count=0 ``` Single-order smoke test: - Request: one real order hash plus one zero hash. - Result: real hash returned `50` trades; zero hash returned an empty `trades` array. - HTTP total: `0.415652s`. - SDK local fetch: `29ms`; SDK total: `161ms`. Heavy response stress test using the highest-trade order hashes in the local DB: - Top 10 order hashes: `3,790` trades, `2.12 MB` response, HTTP total `11.20s`. - Top 20 order hashes: `4,596` trades, `2.57 MB` response, HTTP total `13.50s`. - Top 20 SQL fetch was `930ms`; full SDK grouping/materialization was `13211ms`; full API request was `13495ms`. The heavy case is dominated by materializing, grouping, mapping, and serializing thousands of full trade objects rather than by SQL lookup. ## Notes - The `unimplemented!()` additions are test-only mock methods required by the expanded `TradesDataSource` trait; they are not production route code. - Empty `orderHashes` currently returns an empty grouped result, matching the SDK behavior. ## Verification - `nix develop -c cargo fmt` - `nix develop -c cargo check` - `nix develop -c cargo test routes::trades::get_by_order_hashes` - `nix develop -c cargo test` - `nix develop -c rainix-rs-static` `rainix-rs-static` passes with pre-existing dead-code warnings in `src/cache.rs`.

Dependent PRs
getTradestoken filtering support. This PR should not merge until that raindex PR lands and the submodule pointer is mergeable.Summary
GET /v1/trades/token/{address}for paginated trades involving a token address.TradesDataSourceabstraction and the raindex SDKget_tradestoken filters.TradesDataSourcetrait.DotrainGuiStateV1toOrderBuilderStateV1.rain.orderbooksubmodule to the SDK commit with token-filteredgetTradessupport.Linear: RAI-526
Behavior
The new endpoint accepts the same pagination/time query parameters as
GET /v1/trades/{address}:It returns trades where the token appears on either side of the trade by passing the token as both an input-token and output-token filter to the SDK.
Notes
pageSize=50; materialized trade read tables should be handled as a separate SDK/local DB optimization.unimplemented!()additions are test-only mock methods required by the expandedTradesDataSourcetrait; they are not production route code.Verification
nix develop -c cargo checknix develop -c cargo testnix develop -c cargo fmtnix develop -c rainix-rs-staticrainix-rs-staticpasses with pre-existing dead-code warnings insrc/cache.rs.Summary by CodeRabbit
New Features
Documentation
Refactor
Chores