feat: add batch order hash trade lookup#2572
Conversation
📝 WalkthroughWalkthroughAdds batched order-hash support end-to-end: GraphQL filter list, local DB SQL list binding and query builder wiring, existing get_trades integration, and a new WASM-exported RaindexClient API that groups and returns trades per requested order hash. ChangesOrder-hash batch trade fetching
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 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. |
## 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
|
graphite-app
Bot
changed the base branch from
feature/rai-529-add-taker-address-filters-to-raindex-gettrades-sdk
to
graphite-base/2572
findolor
force-pushed
the
feature/rai-527-add-order-hash-batch-query-support-to-raindex-trades-sdk
branch
from
6c52dff to
6dbe929
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/common/src/local_db/query/fetch_trades/mod.rs (1)
129-157: 💤 Low valueOptional: collapse the five sequential
bind_list_clausecalls.Five back-to-back calls with the same iterator factory could be expressed as a single loop over
(placeholder, body)tuples, which makes adding/removing CTE bindings cleaner without changing behavior or param ordering.♻️ Possible shape
for (placeholder, body) in [ (CLEAR_EVENTS_ORDER_HASHES_CLAUSE, CLEAR_EVENTS_ORDER_HASHES_CLAUSE_BODY), (TAKE_TRADES_ORDER_HASHES_CLAUSE, TAKE_TRADES_ORDER_HASHES_CLAUSE_BODY), (CLEAR_ALICE_ORDER_HASHES_CLAUSE, CLEAR_ALICE_ORDER_HASHES_CLAUSE_BODY), (CLEAR_BOB_ORDER_HASHES_CLAUSE, CLEAR_BOB_ORDER_HASHES_CLAUSE_BODY), (ORDER_HASHES_CLAUSE, ORDER_HASHES_CLAUSE_BODY), ] { stmt.bind_list_clause(placeholder, body, order_hashes_iter())?; }🤖 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 `@crates/common/src/local_db/query/fetch_trades/mod.rs` around lines 129 - 157, The five repeated stmt.bind_list_clause calls that use the same iterator factory can be collapsed into a single loop: build a list/array of the (CLEAR_EVENTS_ORDER_HASHES_CLAUSE, CLEAR_EVENTS_ORDER_HASHES_CLAUSE_BODY), (TAKE_TRADES_ORDER_HASHES_CLAUSE, TAKE_TRADES_ORDER_HASHES_CLAUSE_BODY), (CLEAR_ALICE_ORDER_HASHES_CLAUSE, CLEAR_ALICE_ORDER_HASHES_CLAUSE_BODY), (CLEAR_BOB_ORDER_HASHES_CLAUSE, CLEAR_BOB_ORDER_HASHES_CLAUSE_BODY), (ORDER_HASHES_CLAUSE, ORDER_HASHES_CLAUSE_BODY) tuples and iterate over them calling stmt.bind_list_clause(placeholder, body, order_hashes_iter()) for each; keep the order_hashes preparation (clone, sort, dedup) and the order_hashes_iter() closure as-is to preserve parameter ordering and semantics.crates/common/src/raindex_client/trades/get_by_order_hashes.rs (1)
249-336: ⚡ Quick winAlign error handling with
get_tradesto preserve phase-level observability.
get_tradesinget_all.rswraps both the local DB fetch (L247–273) and the subgraph fetch (L340–351) in explicitmatcharms that emittracing::error!with phase context (e.g.,"getTrades local DB fetch failed") before returning. Here, bothfetch_trades(...).await?(L259–273) andclient.trades_list_all(sg_filters).await?(L305) propagate failures silently — the success-pathtracing::debug!lines never fire, and you lose the elapsed time and counts at the point of failure.Mirroring the pattern in
get_all.rsis a small, mechanical change and materially improves debugging when a single layer (local vs subgraph) misbehaves in production.♻️ Suggested shape (sketch)
let local_trades = match fetch_trades(&db, FetchTradesArgs { /* ... */ }).await { Ok(trades) => trades, Err(err) => { tracing::error!( elapsed_ms = local_started.elapsed_ms(), local_chain_ids_count, order_hashes_count = order_hashes.len(), error = %err, "getTradesByOrderHashes local DB fetch failed" ); return Err(err.into()); } }; // ... analogous match around `client.trades_list_all(sg_filters).await` ...🤖 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 `@crates/common/src/raindex_client/trades/get_by_order_hashes.rs` around lines 249 - 336, Wrap the two await calls that can fail — fetch_trades(...) used in the local DB branch and client.trades_list_all(sg_filters).await in the subgraph branch — in explicit match arms (or map_err) so failures are logged with phase context before returning; for fetch_trades (inside the if let Some(db) block, around local_started) emit a tracing::error! including elapsed_ms = local_started.elapsed_ms(), order_hashes.len(), local chain id/count context and the error, then return Err(err.into()); do the same around client.trades_list_all(sg_filters).await (using subgraph_started.elapsed_ms(), order_hashes.len(), rows counts if available) so that failures mirror get_trades in get_all.rs and preserve phase-level observability while keeping the existing conversion/processing (e.g., RaindexTrade::try_from_local_db_trade and RaindexTrade::try_from_sg_trade) on the success path.
🤖 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.
Inline comments:
In `@crates/common/src/raindex_client/trades/get_by_order_hashes.rs`:
- Around line 213-222: After deduplicating hashes (after calling
unique_order_hashes_preserving_order and assigning to order_hashes) validate the
number of order_hashes against a sensible upper bound (e.g., 100–200 depending
on your SQLite build) and return an early Err with a clear message if exceeded;
update the function that returns Result<RaindexTradesByOrderHashResult,
RaindexError> to produce a descriptive RaindexError (or new
BadRequest/Validation variant) stating the max allowed hashes and actual count
to avoid opaque SQLITE_LIMIT_VARIABLE_NUMBER failures when the query constructs
~5N parameters.
---
Nitpick comments:
In `@crates/common/src/local_db/query/fetch_trades/mod.rs`:
- Around line 129-157: The five repeated stmt.bind_list_clause calls that use
the same iterator factory can be collapsed into a single loop: build a
list/array of the (CLEAR_EVENTS_ORDER_HASHES_CLAUSE,
CLEAR_EVENTS_ORDER_HASHES_CLAUSE_BODY), (TAKE_TRADES_ORDER_HASHES_CLAUSE,
TAKE_TRADES_ORDER_HASHES_CLAUSE_BODY), (CLEAR_ALICE_ORDER_HASHES_CLAUSE,
CLEAR_ALICE_ORDER_HASHES_CLAUSE_BODY), (CLEAR_BOB_ORDER_HASHES_CLAUSE,
CLEAR_BOB_ORDER_HASHES_CLAUSE_BODY), (ORDER_HASHES_CLAUSE,
ORDER_HASHES_CLAUSE_BODY) tuples and iterate over them calling
stmt.bind_list_clause(placeholder, body, order_hashes_iter()) for each; keep the
order_hashes preparation (clone, sort, dedup) and the order_hashes_iter()
closure as-is to preserve parameter ordering and semantics.
In `@crates/common/src/raindex_client/trades/get_by_order_hashes.rs`:
- Around line 249-336: Wrap the two await calls that can fail —
fetch_trades(...) used in the local DB branch and
client.trades_list_all(sg_filters).await in the subgraph branch — in explicit
match arms (or map_err) so failures are logged with phase context before
returning; for fetch_trades (inside the if let Some(db) block, around
local_started) emit a tracing::error! including elapsed_ms =
local_started.elapsed_ms(), order_hashes.len(), local chain id/count context and
the error, then return Err(err.into()); do the same around
client.trades_list_all(sg_filters).await (using subgraph_started.elapsed_ms(),
order_hashes.len(), rows counts if available) so that failures mirror get_trades
in get_all.rs and preserve phase-level observability while keeping the existing
conversion/processing (e.g., RaindexTrade::try_from_local_db_trade and
RaindexTrade::try_from_sg_trade) on the success path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 37fe2f01-b818-4540-855e-8fcae4998a05
📒 Files selected for processing (6)
crates/common/src/local_db/query/fetch_trades/mod.rscrates/common/src/local_db/query/fetch_trades/query.sqlcrates/common/src/raindex_client/trades/get_all.rscrates/common/src/raindex_client/trades/get_by_order_hashes.rscrates/common/src/raindex_client/trades/mod.rscrates/subgraph/src/types/common.rs
| ) -> Result<RaindexTradesByOrderHashResult, RaindexError> { | ||
| let started = Timing::now(); | ||
| let requested_order_hashes_count = order_hashes.0.len(); | ||
| let order_hashes = unique_order_hashes_preserving_order(order_hashes.0); | ||
| if order_hashes.is_empty() { | ||
| return Ok(RaindexTradesByOrderHashResult { | ||
| trades_by_order_hash: Vec::new(), | ||
| total_count: 0, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the bundled SQLite build / runtime to confirm the variable-number limit assumption.
rg -nP --type=rust -C3 '\b(SQLITE_LIMIT_VARIABLE_NUMBER|limit_variable_number|sqlite_limit)\b'
rg -nP --type=toml -C2 '^(rusqlite|libsqlite3-sys)\b'Repository: rainlanguage/raindex
Length of output: 726
🏁 Script executed:
# Read the full function to understand parameter binding
wc -l crates/common/src/raindex_client/trades/get_by_order_hashes.rsRepository: rainlanguage/raindex
Length of output: 131
🏁 Script executed:
# Read the relevant function from the file
cat -n crates/common/src/raindex_client/trades/get_by_order_hashes.rs | head -250 | tail -50Repository: rainlanguage/raindex
Length of output: 2512
🏁 Script executed:
# Look for any existing input validation in this file or related files
rg -n "order_hashes|len\(\)|ORDER_HASHES_LIMIT|MAX_" crates/common/src/raindex_client/trades/get_by_order_hashes.rsRepository: rainlanguage/raindex
Length of output: 1829
🏁 Script executed:
# Look for ORDER_HASHES_CLAUSE and SQL query construction
rg -n "ORDER_HASHES_CLAUSE\|order_hash_in\|sql\|query" crates/common/src/raindex_client/trades/get_by_order_hashes.rs | head -30Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Read the section where the local DB query is called
sed -n '249,280p' crates/common/src/raindex_client/trades/get_by_order_hashes.rsRepository: rainlanguage/raindex
Length of output: 1425
🏁 Script executed:
# Find the get_trades_local function to see the actual SQL
rg -n "fn get_trades_local\|ORDER_HASHES_CLAUSE" crates/common/src/raindex_client/trades/ -A 15 | head -60Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Find the fetch_trades function
rg -n "fn fetch_trades" crates/common/src/ -A 20 | head -80Repository: rainlanguage/raindex
Length of output: 7047
🏁 Script executed:
# Search more broadly for SQL related code in raindex_client
find crates/common/src/raindex_client -type f -name "*.rs" | head -20Repository: rainlanguage/raindex
Length of output: 1374
🏁 Script executed:
# Look for where SQL queries are constructed
rg -n "SELECT.*FROM\|INSERT\|WHERE.*ORDER_HASH" crates/common/src/raindex_client/ -B2 -A2 | head -100Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Find build_fetch_trades_stmt function
rg -n "fn build_fetch_trades_stmt\|build_fetch_trades_count_stmt" crates/common/src/raindex_client/local_db/ -A 50 | head -150Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Look for where order_hashes are used in SQL query building
rg -n "order_hashes\|IN.*\?" crates/common/src/raindex_client/local_db/query/ | grep -v testRepository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Search for build_fetch_trades_stmt more broadly
rg -n "build_fetch_trades" crates/common/src/ -B2 -A15Repository: rainlanguage/raindex
Length of output: 38445
🏁 Script executed:
# Check if there's a query builder module
find crates/common/src/raindex_client/local_db -name "*.rs" -type f | xargs grep -l "order_hash"Repository: rainlanguage/raindex
Length of output: 467
🏁 Script executed:
# Look for any CTE or query string construction
rg -n "WITH.*AS\|CREATE.*TEMP\|SELECT.*FROM" crates/common/src/raindex_client/local_db/query/ | head -40Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Read the full build_fetch_trades_stmt function to understand parameter binding
sed -n '68,202p' crates/common/src/local_db/query/fetch_trades/mod.rsRepository: rainlanguage/raindex
Length of output: 4564
🏁 Script executed:
# Look for the order_hashes binding
rg -n "order_hashes\|bind_list_clause\|ORDER_HASHES" crates/common/src/local_db/query/fetch_trades/mod.rs -B2 -A2Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Look for any existing limits or validations on order_hashes in the entire codebase
rg -n "MAX.*ORDER_HASH\|order_hash.*limit\|order_hash.*bound\|order_hash.*size\|order_hashes.*len" crates/common/src/ --type=rust | head -40Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Check if there are any error handlers or parameter limit checks
rg -n "SQLITE_LIMIT\|variable.*number\|too.*many\|parameter.*limit\|max.*param" crates/common/src/ --type=rustRepository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Look at the test file to see what parameter counts are considered acceptable
sed -n '380,410p' crates/common/src/local_db/query/fetch_trades/mod.rsRepository: rainlanguage/raindex
Length of output: 1750
🏁 Script executed:
# Check the QUERY_TEMPLATE to understand all the order_hash bindings
rg -n "QUERY_TEMPLATE\|ORDER_HASHES_CLAUSE\|CLEAR_EVENTS_ORDER_HASHES_CLAUSE\|TAKE_TRADES_ORDER_HASHES_CLAUSE\|CLEAR_ALICE_ORDER_HASHES_CLAUSE\|CLEAR_BOB_ORDER_HASHES_CLAUSE" crates/common/src/local_db/query/fetch_trades/ -B2 -A2 | head -80Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Find the file that defines these constants
find crates/common/src/local_db/query/fetch_trades -name "*.rs" -type fRepository: rainlanguage/raindex
Length of output: 117
🏁 Script executed:
# Read the constants file
cat -n crates/common/src/local_db/query/fetch_trades/constants.rs 2>/dev/null || cat -n crates/common/src/local_db/query/fetch_trades/mod.rs | head -60Repository: rainlanguage/raindex
Length of output: 3890
🏁 Script executed:
# Count how many times order_hashes are bound - check the structure more carefully
grep -n "order_hashes_iter\|ORDER_HASHES" crates/common/src/local_db/query/fetch_trades/mod.rs | head -20Repository: rainlanguage/raindex
Length of output: 1349
🏁 Script executed:
# Check if there are any API limits or guards in get_trades_by_order_hashes
cat -n crates/common/src/raindex_client/trades/get_by_order_hashes.rs | head -250Repository: rainlanguage/raindex
Length of output: 10084
🏁 Script executed:
# Look for any discussion or documentation about limits
rg -n "limit\|bound\|maximum\|cap" crates/common/src/raindex_client/trades/get_by_order_hashes.rs -iRepository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Find what the default SQLite parameter limit is in rusqlite 0.32
web_search "rusqlite 0.32 SQLITE_LIMIT_VARIABLE_NUMBER parameter limit"Repository: rainlanguage/raindex
Length of output: 115
🏁 Script executed:
# Check if there are any recent issues or PRs addressing this
rg -n "order_hash.*limit\|parameter.*limit\|variable.*number" crates/ --type=rust -B3 -A3Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Check for any existing validation in related functions
rg -n "is_empty\|len\(\)\|validate" crates/common/src/raindex_client/trades/ --type=rust | head -20Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
web_search
rusqlite 0.32 SQLite parameter limit SQLITE_LIMIT_VARIABLE_NUMBER defaultRepository: rainlanguage/raindex
Length of output: 162
🏁 Script executed:
# Look at the test more carefully - with 2 hashes it shows 10 params for order_hashes clauses
# Let's calculate: CLEAR_EVENTS binds twice, plus 4 other clauses = 2*2 + 4*2 = 12 for 2 hashes
# But test shows params[0-9] used for order hashes, so 10 total. Let me recount.
# Actually looking at test: (?1, ?2) for alice, (?1, ?2) for bob (reused), (?3, ?4) for oe, (?5, ?6) for mc.alice,
# (?7, ?8) for mc.bob, (?9, ?10) for tws. That's placeholders 1-10, which is 5 pairs = 5 * 2 = 10 for N=2
# So it's approximately 5N parameters from order_hashes alone
# Also check if there's any documentation about expected limits
rg -n "documentation\|doc\|comment" crates/common/src/raindex_client/trades/get_by_order_hashes.rs | head -20Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Check the wasm_export attribute to see if there's any documented limit
sed -n '188,213p' crates/common/src/raindex_client/trades/get_by_order_hashes.rsRepository: rainlanguage/raindex
Length of output: 1121
🌐 Web query:
rusqlite 0.32 SQLite SQLITE_LIMIT_VARIABLE_NUMBER default parameter limit
💡 Result:
In rusqlite 0.32, there is no single hard-coded “default parameter limit” exposed by rusqlite itself; instead, rusqlite can query/set SQLite’s per-connection runtime limit SQLITE_LIMIT_VARIABLE_NUMBER via the limits feature. The relevant rusqlite enum value is Limit::SQLITE_LIMIT_VARIABLE_NUMBER (value 9). [1][2] What the “default” value is depends on how SQLite was compiled (and/or what your SQLite runtime reports). SQLite documents that the compile-time macro SQLITE_MAX_VARIABLE_NUMBER defaults to 999 for SQLite versions prior to 3.32.0 and to 32766 for SQLite versions after 3.32.0, and that the maximum parameter index can also be changed at runtime with sqlite3_limit(db, SQLITE_LIMIT_VARIABLE_NUMBER, size). [3] Practical implications for rusqlite users: - If you have rusqlite built against SQLite default settings, the value reported by SQLite for SQLITE_LIMIT_VARIABLE_NUMBER will typically be 999 (older SQLite) or 32766 (SQLite >= 3.32.0). [3] - You can read/change the value per connection using Connection::limit and Connection::set_limit (feature "limits"). [4][5][6] Example (how to read the limit in rusqlite): - Enable the "limits" feature and then call db.limit(Limit::SQLITE_LIMIT_VARIABLE_NUMBER) on a Connection. [4][1] Sources: rusqlite Connection::limit/set_limit API and Limit enum; SQLite documentation on SQLITE_MAX_VARIABLE_NUMBER defaults and sqlite3_limit runtime adjustment. [4][1][3]
Citations:
- 1: https://docs.rs/rusqlite/latest/rusqlite/limits/enum.Limit.html
- 2: http://sqlite.org/c3ref/c_limit_attached.html
- 3: https://sqlite.org/limits.html
- 4: https://docs.rs/rusqlite-le/latest/rusqlite/struct.Connection.html
- 5: https://docs.rs/rusqlite/latest/rusqlite/limits/index.html
- 6: https://android.googlesource.com/platform/external/rust/crates/rusqlite/+/8a0e0cefcdd7fd8f911691e4422d8e58311cd132/src/limits.rs
🏁 Script executed:
# Check Cargo.lock or dependencies to confirm rusqlite version
rg -n "rusqlite" crates/common/Cargo.tomlRepository: rainlanguage/raindex
Length of output: 125
🏁 Script executed:
# Look for any comments or documentation in the query builder about parameter limits
rg -n "parameter\|limit\|bound\|cap\|maximum" crates/common/src/local_db/query/ --type=rust -i | head -30Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Check if there are any build configurations or features that specify SQLite version
rg -n "libsqlite3\|sqlite.*version\|SQLITE_MAX_VARIABLE" crates/ --type=toml --type=rust | head -20Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Look for error handling around query execution
rg -n "query_json\|Error\|Result\|parameter.*limit\|variable.*number" crates/common/src/local_db/executor.rs -B2 -A2 | head -60Repository: rainlanguage/raindex
Length of output: 46
🏁 Script executed:
# Calculate the actual parameter count for a concrete example
# If we have 100 order_hashes and some basic filters, how many params would that be?
# Let's check by examining what other filters contribute
cat -n crates/common/src/local_db/query/fetch_trades/mod.rs | sed -n '56,66p'Repository: rainlanguage/raindex
Length of output: 483
Add input validation for order_hashes batch size.
The function binds order_hashes to approximately 5-6 different SQL clauses, creating ~5N parameters from hashes alone. On SQLite builds with the default SQLITE_LIMIT_VARIABLE_NUMBER of 999 (pre-3.32.0), a few hundred hashes combined with additional filters (chain IDs, takers, owners, tokens, timestamps) can exceed the parameter limit at runtime, resulting in an opaque SQLite error. Document an expected upper bound on order_hashes (e.g., 100-200 based on your deployment's SQLite version), and validate inputs early with a clear error message. This is low-cost and prevents cryptic runtime failures.
🤖 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 `@crates/common/src/raindex_client/trades/get_by_order_hashes.rs` around lines
213 - 222, After deduplicating hashes (after calling
unique_order_hashes_preserving_order and assigning to order_hashes) validate the
number of order_hashes against a sensible upper bound (e.g., 100–200 depending
on your SQLite build) and return an early Err with a clear message if exceeded;
update the function that returns Result<RaindexTradesByOrderHashResult,
RaindexError> to produce a descriptive RaindexError (or new
BadRequest/Validation variant) stating the max allowed hashes and actual count
to avoid opaque SQLITE_LIMIT_VARIABLE_NUMBER failures when the query constructs
~5N parameters.
## 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 -->
](https://app.coderabbit.ai/change-stack/rainlanguage/raindex/pull/2572){kind=link}
graphite-app
Bot
force-pushed
the
feature/rai-527-add-order-hash-batch-query-support-to-raindex-trades-sdk
branch
from
6dbe929 to
957f5f0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@crates/common/src/raindex_client/trades/get_by_order_hashes.rs`:
- Around line 249-273: The local DB branch currently runs whenever local_db is
Some, which calls fetch_trades with local_ids possibly empty and removes the
chain filter; change the condition to only run the local query when both
local_db.is_some() and !local_ids.is_empty() (mirror the sg_ids check) so
fetch_trades is invoked only for selected local chains; update the branch that
constructs local_tokens/local_started/local_trades to be guarded by
!local_ids.is_empty() and leave behavior unchanged otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d31bb83e-c2dc-414e-9cc6-ff928f871a4c
📒 Files selected for processing (6)
crates/common/src/local_db/query/fetch_trades/mod.rscrates/common/src/local_db/query/fetch_trades/query.sqlcrates/common/src/raindex_client/trades/get_all.rscrates/common/src/raindex_client/trades/get_by_order_hashes.rscrates/common/src/raindex_client/trades/mod.rscrates/subgraph/src/types/common.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/common/src/raindex_client/trades/mod.rs
- crates/common/src/local_db/query/fetch_trades/mod.rs
- crates/common/src/raindex_client/trades/get_all.rs
| if let Some(db) = local_db { | ||
| let local_tokens = filters | ||
| .tokens | ||
| .clone() | ||
| .map(|tokens| FetchTradesTokensFilter { | ||
| inputs: tokens.inputs.unwrap_or_default(), | ||
| outputs: tokens.outputs.unwrap_or_default(), | ||
| }) | ||
| .unwrap_or_default(); | ||
| let local_started = Timing::now(); | ||
| let local_trades = fetch_trades( | ||
| &db, | ||
| FetchTradesArgs { | ||
| chain_ids: local_ids, | ||
| orderbook_addresses: filters.orderbook_addresses.clone().unwrap_or_default(), | ||
| owners: filters.owners.clone(), | ||
| takers: filters.takers.clone(), | ||
| order_hash: None, | ||
| order_hashes: order_hashes.clone(), | ||
| tokens: local_tokens, | ||
| time_filter: filters.time_filter.clone().unwrap_or_default(), | ||
| pagination: PaginationParams::default(), | ||
| }, | ||
| ) | ||
| .await?; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
mod_file=$(fd -p 'mod.rs' crates/common/src/local_db/query/fetch_trades | head -n1)
sql_file=$(fd -p 'query.sql' crates/common/src/local_db/query/fetch_trades | head -n1)
echo "== Inspect fetch_trades builder =="
nl -ba "$mod_file" | sed -n '1,260p' | rg -n -C3 'chain_ids|CHAIN_IDS_CLAUSE|is_empty|bind_list_clause|replace'
echo
echo "== Inspect SQL chain placeholder =="
nl -ba "$sql_file" | rg -n -C2 'CHAIN_IDS_CLAUSE'Repository: rainlanguage/raindex
Length of output: 158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
mod_file=$(fd -p 'mod.rs' crates/common/src/local_db/query/fetch_trades | head -n1)
sql_file=$(fd -p 'query.sql' crates/common/src/local_db/query/fetch_trades | head -n1)
echo "== Inspect fetch_trades builder =="
cat -n "$mod_file" | sed -n '1,260p' | rg -n -C3 'chain_ids|CHAIN_IDS_CLAUSE|is_empty|bind_list_clause|replace'
echo
echo "== Inspect SQL chain placeholder =="
cat -n "$sql_file" | rg -n -C2 'CHAIN_IDS_CLAUSE'Repository: rainlanguage/raindex
Length of output: 6223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find SqlStatement definition
echo "== Search for SqlStatement struct and bind_list_clause =="
fd -p '\.rs$' crates/common/src/local_db | xargs rg -l 'fn bind_list_clause|impl SqlStatement' | head -5
# Also check what's in the sql module
fd -p 'sql' crates/common/src/local_db -t dRepository: rainlanguage/raindex
Length of output: 228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Examine the bind_list_clause implementation
echo "== SqlStatement bind_list_clause implementation =="
cat -n "crates/common/src/local_db/query/sql_statement.rs" | grep -A 30 'fn bind_list_clause'Repository: rainlanguage/raindex
Length of output: 7472
Guard the local query when no local chains were selected.
The subgraph path is gated on !sg_ids.is_empty(), but the local path runs whenever local_db exists. When fetch_trades is called with an empty chain_ids vector, the bind_list_clause method removes the entire chain filter clause from the SQL query. This means a request targeting only subgraph-backed chains will still read from the local DB without any chain filter, returning trades from all chains instead of only the requested ones.
Add an !local_ids.is_empty() guard to the local DB branch to match the subgraph branch's behavior.
🤖 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 `@crates/common/src/raindex_client/trades/get_by_order_hashes.rs` around lines
249 - 273, The local DB branch currently runs whenever local_db is Some, which
calls fetch_trades with local_ids possibly empty and removes the chain filter;
change the condition to only run the local query when both local_db.is_some()
and !local_ids.is_empty() (mirror the sg_ids check) so fetch_trades is invoked
only for selected local chains; update the branch that constructs
local_tokens/local_started/local_trades to be guarded by !local_ids.is_empty()
and leave behavior unchanged otherwise.
|
@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:
|
|
Tip For best results, initiate chat on the files or code changes. SIZE=L |
## 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`.
3 participants
Related Issue
Dependent PRs
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
RaindexClient.getTradesByOrderHashes, exposed to wasm asgetTradesByOrderHashes, returning trades grouped by requested order hash.FetchTradesArgswithorder_hashesand push the batch predicates into the trade source CTEs, includingmatching_clears,take_trades,clear_alice, andclear_bob.orderHash_insupport and issue onetrades_list_allrequest for all requested hashes.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 get_by_order_hashesnix develop -c cargo test -p rain_orderbook_commonnix develop -c cargo test -p rain_orderbook_subgraph_clientnix develop -c rainix-wasm-testnix develop -c rainix-rs-staticSummary by CodeRabbit
New Features
Bug Fixes / Improvements