feat: add taker filters to getTrades#2571
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
How to use the Graphite Merge QueueAdd the label Raindex-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. |
c621884 to
7d24eb3
Compare
Merge activity
|
## Related Issue - [RAI-529: Add taker address filters to raindex getTrades SDK](https://linear.app/makeitrain/issue/RAI-529/add-taker-address-filters-to-raindex-gettrades-sdk) ## Dependent PRs - Depends on #2570 ## Motivation Consumers need the SDK-level `getTrades` API to support filtering by taker address. The REST API work identifies the taker address as the trade event sender, which is available in the raindex local DB and subgraph trade models. For the local DB path, the taker predicate also needs to be applied before the trade reconstruction CTEs expand and hydrate rows. Filtering only after the normalized trade result forces SQLite to build a much larger intermediate result set before returning the requested page. ## Solution - Add `takers` to the existing `GetTradesFilters` SDK filter shape. - Map takers into local `FetchTradesArgs` and push the SQL predicate into the source CTEs: `take_orders.sender IN (...)` and `clear_v3_events.sender IN (...)`. - Add sender/time indexes for `take_orders` and `clear_v3_events` to support the new taker lookup path. - Add local DB `getTrades` fetch/count tracing with filter counts, parameter count, row/count results, and duration. - Add a subgraph `TradeEvent_filter` binding and map takers to `tradeEvent_: { sender_in: [...] }`. - Preserve the existing `getTrades` pagination, time, owner, order hash, orderbook, and token filter behavior. - Add focused tests for local SQL generation and subgraph filter mapping. ## Checks By submitting this for review, I confirm I have done the following: - [x] made this PR as small as possible - [x] unit-tested any new functionality - [x] linked any relevant issues or PRs - [ ] included screenshots (if this involves a front-end change) Additional validation run: - `cargo fmt --all` - `nix develop -c cargo test -p rain_orderbook_common fetch_trades` - `nix develop -c cargo test -p rain_orderbook_common -p rain_orderbook_subgraph_client` - `nix develop -c cargo test --workspace`
0c4d8ec to
1b82157
Compare
7d24eb3 to
90d4839
Compare
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
## Related Issue - [RAI-527: Add order hash batch query support to raindex trades SDK](https://linear.app/makeitrain/issue/RAI-527/add-order-hash-batch-query-support-to-raindex-trades-sdk) ## Dependent PRs - Depends on #2571 ## Motivation Consumers need to fetch trades for multiple order hashes without issuing one SDK query per hash. Running many independent trade lookups would duplicate local DB reconstruction work and subgraph pagination work, especially when callers need results for dozens of orders. ## Solution - Add `RaindexClient.getTradesByOrderHashes`, exposed to wasm as `getTradesByOrderHashes`, returning trades grouped by requested order hash. - Preserve requested order-hash ordering, dedupe duplicate requested hashes, and return empty buckets for hashes with no matching trades. - Add optional owner, taker, token, orderbook, and time filters to the batch API. - Extend local DB `FetchTradesArgs` with `order_hashes` and push the batch predicates into the trade source CTEs, including `matching_clears`, `take_trades`, `clear_alice`, and `clear_bob`. - Add subgraph `orderHash_in` support and issue one `trades_list_all` request for all requested hashes. - Reuse the existing token-filter normalization/matching logic for subgraph results. - Add debug tracing for local DB, subgraph, and total batch lookup timing and row counts. - Add focused tests for SQL generation, subgraph filter construction, hash deduping, grouping, empty buckets, and result ordering. ## Checks By submitting this for review, I confirm I have done the following: - [x] made this PR as small as possible - [x] unit-tested any new functionality - [x] linked any relevant issues or PRs - [ ] included screenshots (if this involves a front-end change) Additional validation run: - `cargo fmt --all` - `nix develop -c cargo test -p rain_orderbook_common fetch_trades` - `nix develop -c cargo test -p rain_orderbook_common get_by_order_hashes` - `nix develop -c cargo test -p rain_orderbook_common` - `nix develop -c cargo test -p rain_orderbook_subgraph_client` - `nix develop -c rainix-wasm-test` - `nix develop -c rainix-rs-static` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added batch filtering by multiple order hashes in trade queries. * Introduced API to fetch trades grouped by order hashes, with optional filtering by owners, takers, tokens, orderbook addresses, and time range. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/rainlanguage/raindex/pull/2572) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## 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 -->

Related Issue
Dependent PRs
Motivation
Consumers need the SDK-level
getTradesAPI to support filtering by taker address. The REST API work identifies the taker address as the trade event sender, which is available in the raindex local DB and subgraph trade models.For the local DB path, the taker predicate also needs to be applied before the trade reconstruction CTEs expand and hydrate rows. Filtering only after the normalized trade result forces SQLite to build a much larger intermediate result set before returning the requested page.
Solution
takersto the existingGetTradesFiltersSDK filter shape.FetchTradesArgsand push the SQL predicate into the source CTEs:take_orders.sender IN (...)andclear_v3_events.sender IN (...).take_ordersandclear_v3_eventsto support the new taker lookup path.getTradesfetch/count tracing with filter counts, parameter count, row/count results, and duration.TradeEvent_filterbinding and map takers totradeEvent_: { sender_in: [...] }.getTradespagination, time, owner, order hash, orderbook, and token filter behavior.Checks
By submitting this for review, I confirm I have done the following:
Additional validation run:
cargo fmt --allnix develop -c cargo test -p rain_orderbook_common fetch_tradesnix develop -c cargo test -p rain_orderbook_common -p rain_orderbook_subgraph_clientnix develop -c cargo test --workspace