Update order data source trait with additional methods #2351
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
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 You can disable this status message by setting the WalkthroughThe changes refactor the raindex_client module to implement a local-database-first polling strategy for orders and trades. New OrdersDataSource trait methods (get_added_by_tx_hash, get_removed_by_tx_hash, trades_list, trades_count) are implemented for both LocalDbOrders and SubgraphOrders, with add_orders.rs and remove_orders.rs updated to use the new methods instead of manual subgraph queries. RaindexTrade is now publicly exported. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as add_orders/remove_orders<br/>Caller
participant Poller as Polling Loop
participant LocalDB as LocalDbOrders
participant Subgraph as SubgraphOrders
Client->>Poller: tx_hash, chain_id, orderbook
rect rgb(220, 240, 255)
Note over Poller,LocalDB: Phase 1: Local DB Attempts
loop Retry until timeout or results
Poller->>LocalDB: get_added_by_tx_hash<br/>(or get_removed_by_tx_hash)
LocalDB-->>Poller: Vec<RaindexOrder>
alt Results found
Poller-->>Client: Return orders
else Empty results
Poller->>Poller: Sleep & retry
end
end
end
rect rgb(240, 220, 255)
Note over Poller,Subgraph: Phase 2: Subgraph Fallback
Poller->>Subgraph: get_added_by_tx_hash<br/>(or get_removed_by_tx_hash)
Subgraph->>Subgraph: Query transaction_add_orders<br/>(or transaction_remove_orders)
Subgraph-->>Poller: Vec<RaindexOrder>
Poller-->>Client: Return orders or timeout error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
tauri-app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
crates/common/src/raindex_client/add_orders.rs(3 hunks)crates/common/src/raindex_client/local_db/orders.rs(3 hunks)crates/common/src/raindex_client/orders.rs(4 hunks)crates/common/src/raindex_client/remove_orders.rs(3 hunks)crates/common/src/raindex_client/trades.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
crates/**/*.rs: For Rust crates incrates/*, run lints usingnix develop -c cargo clippy --workspace --all-targets --all-features -D warnings
For Rust crates incrates/*, run tests usingnix develop -c cargo test --workspaceor--package <crate>
Files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
**/crates/**
📄 CodeRabbit inference engine (AGENTS.md)
Rust workspace organized as
crates/*with subdirectories: cli, common, bindings, js_api, quote, subgraph, settings, math, integration_tests
Files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Rust: format code withnix develop -c cargo fmt --all
Rust: lint withnix develop -c rainix-rs-static(preconfigured flags included)
Rust: crates and modules usesnake_case; types usePascalCase
Files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
🧠 Learnings (30)
📓 Common learnings
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2117
File: crates/js_api/src/gui/select_tokens.rs:421-424
Timestamp: 2025-09-02T08:04:34.729Z
Learning: User findolor in the rainlanguage/rain.orderbook codebase prefers not to change test code when tests are already passing, even if there appear to be type mismatches or potential issues. Working tests should be left alone unless there are actual errors.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/fuzz/mod.rs:64-64
Timestamp: 2025-07-04T09:02:57.301Z
Learning: In rainlanguage/rain.orderbook, user findolor prefers to limit type consistency changes to only the parts directly related to the current work scope. For example, when updating chain_id fields from u64 to u32 in fuzz-related code, unrelated files like tauri-app wallet commands can remain as u64 if they serve different purposes and aren't part of the current changes.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2155
File: crates/common/src/raindex_client/trades.rs:133-152
Timestamp: 2025-10-06T14:13:18.531Z
Learning: In the rain.orderbook codebase, the `page` parameter in `RaindexOrder::get_trades_list` method (in crates/common/src/raindex_client/trades.rs) is kept for backwards compatibility with subgraph logic, but the LocalDb fast-path intentionally returns all trades without implementing pagination.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/create_tables/query.sql:71-72
Timestamp: 2025-10-06T11:44:07.888Z
Learning: The local DB feature in the rain.orderbook codebase is not live yet (as of PR #2145), so schema migrations for existing databases are not required when modifying table structures in `crates/common/src/raindex_client/local_db/query/create_tables/query.sql`.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/raindex_client/trades.rs:215-223
Timestamp: 2025-07-04T10:24:56.163Z
Learning: In the rain.orderbook codebase, the get_trade_count method implementation that fetches all trades to count them is intentionally consistent with previous implementations and not considered a performance issue, as indicated by findolor for the trades counting functionality in crates/common/src/raindex_client/trades.rs.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2237
File: crates/common/src/raindex_client/local_db/sync.rs:79-89
Timestamp: 2025-10-18T10:38:41.273Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the sync_database method currently only supports indexing a single orderbook per chain ID, which is why `.first()` is used to select the orderbook configuration. Multi-orderbook support per chain ID is planned for future PRs.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/common/src/raindex_client/orders.rs:738-741
Timestamp: 2025-10-02T19:17:20.332Z
Learning: In crates/common/src/raindex_client/orders.rs, fetch_dotrain_source() is intentionally called in try_from_sg_order for every order conversion because the dotrain source information is needed immediately. A future optimization with local DB logic is planned to eliminate the network round-trip concern.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2202
File: crates/common/src/raindex_client/local_db/sync.rs:33-34
Timestamp: 2025-10-14T07:51:55.148Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the hard-coded `DEFAULT_SYNC_CHAIN_ID` constant (set to `SUPPORTED_LOCAL_DB_CHAINS[0]`) will be replaced with proper chain ID handling in downstream PRs as part of the multi-network/orderbook implementation.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2036
File: crates/js_api/src/filters/raindex_filter_store.rs:319-336
Timestamp: 2025-08-01T07:44:53.910Z
Learning: In the rainlanguage/rain.orderbook project's RaindexFilterStore (crates/js_api/src/filters/raindex_filter_store.rs), the team chose a simplified monolithic approach with hard-coded keys and default auto-save behavior over configurable stores. The update_vaults method intentionally auto-saves to both localStorage and URL after each update as the default behavior, following a design evolution from a previous configurable approach.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1950
File: crates/common/src/raindex_client/orders.rs:301-301
Timestamp: 2025-06-24T13:36:28.797Z
Learning: In the RaindexClient codebase, when Arc::new(RwLock::new(self.clone())) is used (e.g., in get_orders and get_order_by_hash methods), this creates new Arc/RwLock wrappers around a cloned RaindexClient, but the underlying client data is functionally the same. This pattern is used to provide the correct Arc<RwLock<RaindexClient>> type expected by RaindexOrder::try_from_sg_order() method.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql:6-7
Timestamp: 2025-10-06T11:28:30.692Z
Learning: In `crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql`, the orderbook_address is currently hardcoded to '0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB' because the system only supports a single orderbook at the moment. Multiorderbook logic is not yet implemented and will be added in the future.
📚 Learning: 2025-10-06T14:13:18.531Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2155
File: crates/common/src/raindex_client/trades.rs:133-152
Timestamp: 2025-10-06T14:13:18.531Z
Learning: In the rain.orderbook codebase, the `page` parameter in `RaindexOrder::get_trades_list` method (in crates/common/src/raindex_client/trades.rs) is kept for backwards compatibility with subgraph logic, but the LocalDb fast-path intentionally returns all trades without implementing pagination.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-07-04T10:24:56.163Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/raindex_client/trades.rs:215-223
Timestamp: 2025-07-04T10:24:56.163Z
Learning: In the rain.orderbook codebase, the get_trade_count method implementation that fetches all trades to count them is intentionally consistent with previous implementations and not considered a performance issue, as indicated by findolor for the trades counting functionality in crates/common/src/raindex_client/trades.rs.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-10-14T07:51:55.148Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2202
File: crates/common/src/raindex_client/local_db/sync.rs:33-34
Timestamp: 2025-10-14T07:51:55.148Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the hard-coded `DEFAULT_SYNC_CHAIN_ID` constant (set to `SUPPORTED_LOCAL_DB_CHAINS[0]`) will be replaced with proper chain ID handling in downstream PRs as part of the multi-network/orderbook implementation.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-07-21T16:34:31.193Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1911
File: crates/subgraph/src/types/impls.rs:7-15
Timestamp: 2025-07-21T16:34:31.193Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb considers breaking changes that remove unsafe default behaviors to be intentional and acceptable. Specifically, the get_decimals() method in crates/subgraph/src/types/impls.rs was intentionally changed to return MissingDecimals error instead of defaulting to 18 decimals, as defaulting to 18 is considered unsafe and should never have been done.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-07-16T10:40:05.717Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/orders.rs
📚 Learning: 2025-10-06T11:44:07.888Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/create_tables/query.sql:71-72
Timestamp: 2025-10-06T11:44:07.888Z
Learning: The local DB feature in the rain.orderbook codebase is not live yet (as of PR #2145), so schema migrations for existing databases are not required when modifying table structures in `crates/common/src/raindex_client/local_db/query/create_tables/query.sql`.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-10-18T10:38:41.273Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2237
File: crates/common/src/raindex_client/local_db/sync.rs:79-89
Timestamp: 2025-10-18T10:38:41.273Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the sync_database method currently only supports indexing a single orderbook per chain ID, which is why `.first()` is used to select the orderbook configuration. Multi-orderbook support per chain ID is planned for future PRs.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-10-02T19:17:20.332Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/common/src/raindex_client/orders.rs:738-741
Timestamp: 2025-10-02T19:17:20.332Z
Learning: In crates/common/src/raindex_client/orders.rs, fetch_dotrain_source() is intentionally called in try_from_sg_order for every order conversion because the dotrain source information is needed immediately. A future optimization with local DB logic is planned to eliminate the network round-trip concern.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-10-06T11:13:29.956Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2123
File: crates/common/src/raindex_client/local_db/mod.rs:23-29
Timestamp: 2025-10-06T11:13:29.956Z
Learning: In `crates/common/src/raindex_client/local_db/mod.rs`, the `Default` implementation for `LocalDb` that creates an RPC client pointing to `http://localhost:4444` is acceptable because the RPC client must be explicitly configured before actual usage in production scenarios.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rs
📚 Learning: 2025-07-04T10:27:22.544Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/raindex_client/orders.rs:609-609
Timestamp: 2025-07-04T10:27:22.544Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor prefers not to implement overflow protection for trades count casting (usize to u16) at this time, considering it unnecessary for the current scope since the practical risk of orders having 65,535+ trades is extremely low.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rs
📚 Learning: 2025-10-06T11:28:30.692Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql:6-7
Timestamp: 2025-10-06T11:28:30.692Z
Learning: In `crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql`, the orderbook_address is currently hardcoded to '0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB' because the system only supports a single orderbook at the moment. Multiorderbook logic is not yet implemented and will be added in the future.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-05-09T05:28:22.089Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1744
File: crates/js_api/src/subgraph/order.rs:109-114
Timestamp: 2025-05-09T05:28:22.089Z
Learning: In the rain.orderbook project, SubgraphError contains or converts from OrderbookSubgraphClientError, so using OrderbookSubgraphClientError in error creation followed by a conversion to SubgraphError (via ? operator) is acceptable.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-06-24T13:36:28.797Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1950
File: crates/common/src/raindex_client/orders.rs:301-301
Timestamp: 2025-06-24T13:36:28.797Z
Learning: In the RaindexClient codebase, when Arc::new(RwLock::new(self.clone())) is used (e.g., in get_orders and get_order_by_hash methods), this creates new Arc/RwLock wrappers around a cloned RaindexClient, but the underlying client data is functionally the same. This pattern is used to provide the correct Arc<RwLock<RaindexClient>> type expected by RaindexOrder::try_from_sg_order() method.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-06-24T08:46:03.368Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1947
File: crates/common/src/raindex_client/orders.rs:98-125
Timestamp: 2025-06-24T08:46:03.368Z
Learning: In the vault merging logic in crates/common/src/raindex_client/orders.rs, optimization isn't necessary because the maximum list items are usually around 5 items. For such small datasets, the simple three-loop approach is preferred over HashMap-based optimization due to clarity and minimal performance impact.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rs
📚 Learning: 2025-07-15T08:01:38.534Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1994
File: crates/common/src/raindex_client/vaults.rs:282-292
Timestamp: 2025-07-15T08:01:38.534Z
Learning: In the rainlanguage/rain.orderbook codebase, findolor prefers to avoid concurrency optimizations like using `futures::future::try_join_all` for parallel processing of balance changes, considering such optimizations "not that critical at the moment" when the performance impact is minimal.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-06-17T16:32:04.554Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: crates/js_api/src/gui/order_operations.rs:124-134
Timestamp: 2025-06-17T16:32:04.554Z
Learning: In the rain.orderbook codebase, RPC lists are typically small (2 items maximum), so performance optimizations around cloning and converting small Vec<Url> collections are generally unnecessary.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-08-01T09:07:20.383Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2044
File: packages/orderbook/README.md:186-189
Timestamp: 2025-08-01T09:07:20.383Z
Learning: In the rainlanguage/rain.orderbook project, Rust methods on structs like RaindexVaultsList are exported as JavaScript getters in WASM bindings using #[wasm_bindgen(getter)]. This means while the Rust code uses method calls like items(), the JavaScript/WASM API exposes them as property access like .items. The README.md correctly documents the JavaScript API surface, not the Rust implementation details.
Applied to files:
crates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/orders.rs
📚 Learning: 2025-12-03T10:40:25.429Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2344
File: crates/common/src/local_db/pipeline/runner/mod.rs:18-31
Timestamp: 2025-12-03T10:40:25.429Z
Learning: In `crates/common/src/local_db/pipeline/runner/mod.rs`, the `TargetSuccess` struct does not need separate `ob_id` or `orderbook_key` fields because the contained `SyncOutcome` already includes orderbook identification information such as chain_id and orderbook_address. This avoids redundant data duplication.
Applied to files:
crates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-07-04T10:26:24.289Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/raindex_client/mod.rs:116-135
Timestamp: 2025-07-04T10:26:24.289Z
Learning: In crates/common/src/raindex_client/mod.rs, the get_multi_subgraph_args method intentionally treats Some(empty vector) the same as None for chain_ids parameter. Both cases should return all networks to support UI behavior where no selection or empty selection means "show all networks". Only when specific chain IDs are provided should the results be filtered.
Applied to files:
crates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-07-18T10:31:05.498Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like `#[cfg(not(target_family = "wasm"))] use super::*;` in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.
Applied to files:
crates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-07-04T09:02:57.301Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/fuzz/mod.rs:64-64
Timestamp: 2025-07-04T09:02:57.301Z
Learning: In rainlanguage/rain.orderbook, user findolor prefers to limit type consistency changes to only the parts directly related to the current work scope. For example, when updating chain_id fields from u64 to u32 in fuzz-related code, unrelated files like tauri-app wallet commands can remain as u64 if they serve different purposes and aren't part of the current changes.
Applied to files:
crates/common/src/raindex_client/local_db/orders.rs
📚 Learning: 2025-08-01T07:44:53.910Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2036
File: crates/js_api/src/filters/raindex_filter_store.rs:319-336
Timestamp: 2025-08-01T07:44:53.910Z
Learning: In the rainlanguage/rain.orderbook project's RaindexFilterStore (crates/js_api/src/filters/raindex_filter_store.rs), the team chose a simplified monolithic approach with hard-coded keys and default auto-save behavior over configurable stores. The update_vaults method intentionally auto-saves to both localStorage and URL after each update as the default behavior, following a design evolution from a previous configurable approach.
Applied to files:
crates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rs
📚 Learning: 2025-06-20T07:26:50.488Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1941
File: crates/settings/src/yaml/orderbook.rs:112-123
Timestamp: 2025-06-20T07:26:50.488Z
Learning: In crates/settings/src/yaml/orderbook.rs, the user prefers to keep the get_network_by_chain_id implementation simple with O(n) lookup rather than adding caching complexity for performance optimization, even if the method might be called frequently.
Applied to files:
crates/common/src/raindex_client/local_db/orders.rs
📚 Learning: 2025-06-18T18:24:32.049Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1938
File: crates/settings/src/yaml/orderbook.rs:180-199
Timestamp: 2025-06-18T18:24:32.049Z
Learning: In crates/settings/src/yaml/orderbook.rs, the user prefers to avoid refactoring duplicate search logic between get_orderbook_by_address and get_orderbook_by_network_key when there are only 2 functions, indicating they would consider it if more similar functions are added in the future.
Applied to files:
crates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rs
📚 Learning: 2025-08-02T02:34:32.237Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2036
File: crates/js_api/src/filters/raindex_filter_store.rs:446-533
Timestamp: 2025-08-02T02:34:32.237Z
Learning: In the rainlanguage/rain.orderbook project's RaindexFilterStore WASM tests (crates/js_api/src/filters/raindex_filter_store.rs), brusherru decided to focus on testing only methods without side effects (that don't use web_sys) due to difficulties with mocking localStorage and window APIs in the WASM test environment. This pragmatic approach tests pure logic separately from browser integration.
Applied to files:
crates/common/src/raindex_client/local_db/orders.rs
📚 Learning: 2025-04-29T06:19:15.446Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1706
File: crates/quote/src/js_api/mod.rs:57-61
Timestamp: 2025-04-29T06:19:15.446Z
Learning: In the rain.orderbook codebase, the `get_id` function is designed to use `U256::from_str()` rather than `U256::from_hex()` for parsing order hashes, despite the potential for hex string inputs.
Applied to files:
crates/common/src/raindex_client/local_db/orders.rscrates/common/src/raindex_client/orders.rs
📚 Learning: 2025-10-13T12:57:05.900Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2198
File: crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs:7-8
Timestamp: 2025-10-13T12:57:05.900Z
Learning: In the rain.orderbook repository, using unqualified serde helper names (e.g., `#[serde(with = "serde_address")]`) that rely on glob imports is acceptable and does not need to be changed to fully qualified paths.
Applied to files:
crates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/remove_orders.rs
📚 Learning: 2025-08-26T14:52:37.000Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2099
File: crates/common/src/hyper_rpc.rs:3-7
Timestamp: 2025-08-26T14:52:37.000Z
Learning: In the rain.orderbook codebase, creating new reqwest::Client instances per request in HyperRpcClient is not considered an issue by the maintainers, despite potential performance benefits of client reuse.
Applied to files:
crates/common/src/raindex_client/orders.rs
📚 Learning: 2025-06-17T16:54:43.176Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: crates/common/src/deposit.rs:63-64
Timestamp: 2025-06-17T16:54:43.176Z
Learning: In the rain.orderbook codebase, the maintainer findolor prefers to keep the current implementation that clones the rpcs vector when calling ReadableClient::new_from_urls rather than optimizing it to avoid the clone or adding early validation for empty rpcs. Performance micro-optimizations like avoiding vector clones in this context are not a priority.
Applied to files:
crates/common/src/raindex_client/orders.rs
🧬 Code graph analysis (5)
crates/common/src/raindex_client/trades.rs (4)
crates/common/src/raindex_client/mod.rs (2)
local_db(228-230)new(115-137)crates/common/src/raindex_client/local_db/query/fetch_order_trades.rs (1)
fetch_order_trades(8-17)crates/common/src/local_db/mod.rs (2)
is_chain_supported_local_db(352-354)new(344-349)crates/common/src/raindex_client/local_db/orders.rs (1)
new(23-25)
crates/common/src/raindex_client/add_orders.rs (2)
crates/common/src/local_db/mod.rs (1)
is_chain_supported_local_db(352-354)crates/common/src/raindex_client/local_db/orders.rs (1)
new(23-25)
crates/common/src/raindex_client/local_db/orders.rs (2)
crates/common/src/raindex_client/local_db/query/fetch_order_trades.rs (1)
fetch_order_trades(8-17)crates/common/src/raindex_client/local_db/query/fetch_order_trades_count.rs (1)
fetch_order_trades_count(9-19)
crates/common/src/raindex_client/orders.rs (2)
crates/common/src/raindex_client/trades.rs (5)
orderbook(59-61)orderbook(83-85)order_hash(38-40)order_hash(68-70)try_from_sg_trade(268-286)crates/common/src/raindex_client/local_db/query/fetch_orders.rs (1)
orderbook(84-86)
crates/common/src/raindex_client/remove_orders.rs (4)
crates/common/src/raindex_client/orders.rs (4)
orders(754-772)chain_id(193-195)chain_id(264-266)new(40-42)crates/common/src/raindex_client/mod.rs (2)
local_db(228-230)new(115-137)crates/common/src/local_db/mod.rs (1)
is_chain_supported_local_db(352-354)crates/common/src/raindex_client/local_db/orders.rs (1)
new(23-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-browser-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: git-clean
- GitHub Check: test
- GitHub Check: Deploy-Docs-Preview
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: Deploy-Preview-Push
🔇 Additional comments (9)
crates/common/src/raindex_client/local_db/orders.rs (2)
27-57: Tx-hash helper and added/removed methods are consistent and reuse existing conversion logic
fetch_orders_by_tx_hashcorrectly mirrors the existing list/get_by_hash path (sameFetchOrdersArgsfields andfrom_local_db_orderpipeline) and the newget_added_by_tx_hash/get_removed_by_tx_hashsimply delegate to it, ensuring added/removed tx-hash lookups from Local DB share identical behavior.Also applies to: 152-170
2-4: Local DB trades_list/trades_count wiring looks correct and matches intended semanticsThe new imports plus
trades_list/trades_countmethods correctly delegate tofetch_order_trades/fetch_order_trades_countand map rows viaRaindexTrade::try_from_local_db_trade(ob_id.chain_id, trade), and the unused_pageargument ontrades_listis acceptable because the Local DB fast-path intentionally returns all trades without pagination for this API.Also applies to: 172-200
crates/common/src/raindex_client/add_orders.rs (1)
1-4: Local‑DB‑first add‑orders polling is correctly integrated and well covered by testsThe new
LocalDbOrders/SubgraphOrderswiring inget_add_orders_for_transactioncleanly implements a local‑DB‑first polling loop gated byis_chain_supported_local_db, then falls back to subgraphget_added_by_tx_hashwith the same attempt/interval semantics; the non‑wasm tests validate both “prefer local” and “exhaust local before subgraph” behaviors.Also applies to: 92-141, 560-673
crates/common/src/raindex_client/trades.rs (2)
135-156: Local‑DB‑first trades_list path is correct and preserves existing API semantics
get_trades_listnow sensibly derivesOrderbookIdentifierand a B256order_hash, preferringLocalDbOrders::trades_listwhen a supported local DB is present and otherwise delegating toSubgraphOrders::trades_list; forwarding thepageparameter while the Local DB path ignores it is aligned with the documented design where Local DB returns all trades without pagination.
1-2: Local‑DB trade mapping into RaindexTrade is coherent with subgraph representationThe new
RaindexTradeimports plustry_from_local_db_tradecorrectly reconstructRaindexTransactionfrom local fields and build input/outputRaindexVaultBalanceChangeinstances usingtry_from_local_trade_side, then populateRaindexTrade(id fromtrade.trade_id, order_hash fromtrade.order_hash, timestamp fromblock_timestamp, orderbook fromtrade.orderbook), keeping the Local DB-backed trades structurally consistent withtry_from_sg_trade.Also applies to: 12-13, 268-345
crates/common/src/raindex_client/remove_orders.rs (1)
1-5: Removed‑orders polling cleanly reuses LocalDbOrders/SubgraphOrders with correct precedenceThe updated
get_remove_orders_for_transactionmirrors the add‑orders path: it first pollsLocalDbOrders::get_removed_by_tx_hashwhen a supported local DB is available, then falls back toSubgraphOrders::get_removed_by_tx_hashwith identical attempt/interval behavior and aSubgraphIndexingTimeouton exhaustion; the accompanying tests exercise pure subgraph, local‑preferred, and local‑exhaustion scenarios, so the new wiring looks solid.Also applies to: 97-145, 666-792
crates/common/src/raindex_client/orders.rs (3)
2-2: LGTM! Imports properly support new functionality.The added imports (RaindexTrade, Id, and subgraph client types) are correctly scoped for the new trait methods and their implementations.
Also applies to: 23-25
60-89: LGTM! Well-designed trait method signatures.The four new async trait methods follow consistent patterns with appropriate parameter types and error handling. The use of optional parameters for timestamps and pagination provides good flexibility.
846-896: LGTM! Proper error handling in trades methods.Both
trades_listandtrades_countcorrectly propagate errors using the?operator. Thetrades_countimplementation fetches all trades to count them, which is consistent with the project's established patterns.Based on learnings, the trades counting approach is intentionally consistent with previous implementations.
Comment on lines
796
to
819
| async fn get_added_by_tx_hash( | ||
| &self, | ||
| chain_id: u32, | ||
| orderbook: Address, | ||
| tx_hash: B256, | ||
| ) -> Result<Vec<RaindexOrder>, RaindexError> { | ||
| let raindex_client = Rc::new(self.client.clone()); | ||
| let client = self.client.get_orderbook_client(orderbook)?; | ||
| let sg_orders = client | ||
| .transaction_add_orders(Id::new(tx_hash.to_string())) | ||
| .await | ||
| .unwrap_or_default(); | ||
| sg_orders | ||
| .into_iter() | ||
| .map(|value| { | ||
| RaindexOrder::try_from_sg_order( | ||
| raindex_client.clone(), | ||
| chain_id, | ||
| value.order, | ||
| value.transaction.try_into().ok(), | ||
| ) | ||
| }) | ||
| .collect() | ||
| } |
Contributor
There was a problem hiding this comment.
Error swallowing with unwrap_or_default() hides failures.
The unwrap_or_default() on line 807 silently converts subgraph query errors (network failures, malformed responses, etc.) into an empty result set. This prevents callers from distinguishing between "no orders found" and "query failed."
Apply this diff to propagate errors properly:
- let sg_orders = client
- .transaction_add_orders(Id::new(tx_hash.to_string()))
- .await
- .unwrap_or_default();
+ let sg_orders = client
+ .transaction_add_orders(Id::new(tx_hash.to_string()))
+ .await?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async fn get_added_by_tx_hash( | |
| &self, | |
| chain_id: u32, | |
| orderbook: Address, | |
| tx_hash: B256, | |
| ) -> Result<Vec<RaindexOrder>, RaindexError> { | |
| let raindex_client = Rc::new(self.client.clone()); | |
| let client = self.client.get_orderbook_client(orderbook)?; | |
| let sg_orders = client | |
| .transaction_add_orders(Id::new(tx_hash.to_string())) | |
| .await | |
| .unwrap_or_default(); | |
| sg_orders | |
| .into_iter() | |
| .map(|value| { | |
| RaindexOrder::try_from_sg_order( | |
| raindex_client.clone(), | |
| chain_id, | |
| value.order, | |
| value.transaction.try_into().ok(), | |
| ) | |
| }) | |
| .collect() | |
| } | |
| async fn get_added_by_tx_hash( | |
| &self, | |
| chain_id: u32, | |
| orderbook: Address, | |
| tx_hash: B256, | |
| ) -> Result<Vec<RaindexOrder>, RaindexError> { | |
| let raindex_client = Rc::new(self.client.clone()); | |
| let client = self.client.get_orderbook_client(orderbook)?; | |
| let sg_orders = client | |
| .transaction_add_orders(Id::new(tx_hash.to_string())) | |
| .await?; | |
| sg_orders | |
| .into_iter() | |
| .map(|value| { | |
| RaindexOrder::try_from_sg_order( | |
| raindex_client.clone(), | |
| chain_id, | |
| value.order, | |
| value.transaction.try_into().ok(), | |
| ) | |
| }) | |
| .collect() | |
| } |
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/orders.rs around lines 796 to 819, the call
to client.transaction_add_orders(...).await.unwrap_or_default() swallows errors;
instead await the Result and propagate failures to the caller. Replace
unwrap_or_default with handling that maps the client error into the function's
RaindexError (or use the ? operator after converting/mapping the error
accordingly) so the function returns Err on subgraph/network/query failures
rather than silently returning an empty Vec; leave the subsequent
mapping/collect logic unchanged.
Comment on lines
821
to
844
| async fn get_removed_by_tx_hash( | ||
| &self, | ||
| chain_id: u32, | ||
| orderbook: Address, | ||
| tx_hash: B256, | ||
| ) -> Result<Vec<RaindexOrder>, RaindexError> { | ||
| let raindex_client = Rc::new(self.client.clone()); | ||
| let client = self.client.get_orderbook_client(orderbook)?; | ||
| let sg_orders = client | ||
| .transaction_remove_orders(Id::new(tx_hash.to_string())) | ||
| .await | ||
| .unwrap_or_default(); | ||
| sg_orders | ||
| .into_iter() | ||
| .map(|value| { | ||
| RaindexOrder::try_from_sg_order( | ||
| raindex_client.clone(), | ||
| chain_id, | ||
| value.order, | ||
| value.transaction.try_into().ok(), | ||
| ) | ||
| }) | ||
| .collect() | ||
| } |
Contributor
There was a problem hiding this comment.
Error swallowing with unwrap_or_default() hides failures.
Same issue as get_added_by_tx_hash: line 832 silently converts subgraph query errors into an empty result set, preventing proper error propagation.
Apply this diff:
- let sg_orders = client
- .transaction_remove_orders(Id::new(tx_hash.to_string()))
- .await
- .unwrap_or_default();
+ let sg_orders = client
+ .transaction_remove_orders(Id::new(tx_hash.to_string()))
+ .await?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async fn get_removed_by_tx_hash( | |
| &self, | |
| chain_id: u32, | |
| orderbook: Address, | |
| tx_hash: B256, | |
| ) -> Result<Vec<RaindexOrder>, RaindexError> { | |
| let raindex_client = Rc::new(self.client.clone()); | |
| let client = self.client.get_orderbook_client(orderbook)?; | |
| let sg_orders = client | |
| .transaction_remove_orders(Id::new(tx_hash.to_string())) | |
| .await | |
| .unwrap_or_default(); | |
| sg_orders | |
| .into_iter() | |
| .map(|value| { | |
| RaindexOrder::try_from_sg_order( | |
| raindex_client.clone(), | |
| chain_id, | |
| value.order, | |
| value.transaction.try_into().ok(), | |
| ) | |
| }) | |
| .collect() | |
| } | |
| async fn get_removed_by_tx_hash( | |
| &self, | |
| chain_id: u32, | |
| orderbook: Address, | |
| tx_hash: B256, | |
| ) -> Result<Vec<RaindexOrder>, RaindexError> { | |
| let raindex_client = Rc::new(self.client.clone()); | |
| let client = self.client.get_orderbook_client(orderbook)?; | |
| let sg_orders = client | |
| .transaction_remove_orders(Id::new(tx_hash.to_string())) | |
| .await?; | |
| sg_orders | |
| .into_iter() | |
| .map(|value| { | |
| RaindexOrder::try_from_sg_order( | |
| raindex_client.clone(), | |
| chain_id, | |
| value.order, | |
| value.transaction.try_into().ok(), | |
| ) | |
| }) | |
| .collect() | |
| } |
🤖 Prompt for AI Agents
crates/common/src/raindex_client/orders.rs around lines 821 to 844: the code
currently swallows subgraph query errors via .await.unwrap_or_default(),
returning an empty list on failure; replace that unwrap_or_default with proper
error propagation (use .await? and/or .await.map_err(...) to convert the
subgraph client error into RaindexError) so the function returns Err on query
failure and keeps the existing conversion of each sg order into RaindexOrder;
ensure the propagated error type matches the function signature
Result<Vec<RaindexOrder>, RaindexError>.
Comment on lines
+232
to
253
| #[cfg(target_family = "wasm")] | ||
| let order_hash = B256::from_str(&self.order_hash())?; | ||
| #[cfg(not(target_family = "wasm"))] | ||
| let order_hash = self.order_hash(); | ||
|
|
||
| let ob_id = OrderbookIdentifier::new(chain_id, orderbook); | ||
| let raindex_client = self.get_raindex_client(); | ||
|
|
||
| if is_chain_supported_local_db(chain_id) { | ||
| let raindex_client = self.get_raindex_client(); | ||
| if let Some(local_db) = raindex_client.local_db() { | ||
| #[cfg(target_family = "wasm")] | ||
| let order_hash = B256::from_str(&self.order_hash())?; | ||
| #[cfg(not(target_family = "wasm"))] | ||
| let order_hash = self.order_hash(); | ||
|
|
||
| let count = fetch_order_trades_count( | ||
| &local_db, | ||
| &OrderbookIdentifier::new(chain_id, orderbook), | ||
| order_hash, | ||
| start_timestamp, | ||
| end_timestamp, | ||
| ) | ||
| .await?; | ||
| return Ok(count); | ||
| let local_source = LocalDbOrders::new(&local_db, Rc::clone(&raindex_client)); | ||
| return local_source | ||
| .trades_count(&ob_id, &order_hash, start_timestamp, end_timestamp) | ||
| .await; | ||
| } | ||
| } | ||
|
|
||
| let client = self.get_orderbook_client()?; | ||
| let trades_count = client | ||
| .order_trades_list_all( | ||
| Id::new(self.id().to_string()), | ||
| start_timestamp, | ||
| end_timestamp, | ||
| ) | ||
| .await?; | ||
| Ok(trades_count.len() as u64) | ||
| let subgraph_source = SubgraphOrders::new(&raindex_client); | ||
| subgraph_source | ||
| .trades_count(&ob_id, &order_hash, start_timestamp, end_timestamp) | ||
| .await | ||
| } |
Contributor
There was a problem hiding this comment.
Fix non‑WASM get_trade_count: order_hash must be B256, not Bytes
On non‑WASM targets order_hash is currently taken as self.order_hash() (a Bytes value) and then passed by reference into trades_count, which expects a &B256 (matching the Local DB fetch_order_trades_count signature). This will not type‑check and should mirror the B256 parsing used in get_trades_list.
Apply this diff to make the non‑WASM path consistent:
- #[cfg(target_family = "wasm")]
- let order_hash = B256::from_str(&self.order_hash())?;
- #[cfg(not(target_family = "wasm"))]
- let order_hash = self.order_hash();
+ #[cfg(target_family = "wasm")]
+ let order_hash = B256::from_str(&self.order_hash())?;
+ #[cfg(not(target_family = "wasm"))]
+ let order_hash = B256::from_str(&self.order_hash().to_string())?;This matches the approach in get_trades_list, ensuring both Local DB and subgraph trades_count calls receive a proper B256.
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/trades.rs around lines 232 to 253, the
non‑WASM branch currently uses self.order_hash() (Bytes) but trades_count
expects a &B256; change the non‑WASM path to parse B256 the same way as the WASM
path (e.g. let order_hash = B256::from_str(&self.order_hash())?), then pass
&order_hash into both the LocalDbOrders.trades_count and
SubgraphOrders.trades_count calls so both code paths receive a &B256.
Collaborator
Author
There was a problem hiding this comment.
the type is not Bytes. it's B256
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the implementation of RaindexOrder::order_hash() method to verify return type
echo "=== Searching for RaindexOrder struct definition ==="
rg -n "pub struct RaindexOrder" --type=rust -A 20
echo -e "\n=== Searching for order_hash method in RaindexOrder implementations ==="
rg -n "impl RaindexOrder" --type=rust -A 50 | grep -A 10 "order_hash"
echo -e "\n=== Searching for order_hash getter/accessor methods ==="
rg -n "fn order_hash\(&self\)" --type=rust -C 5
Length of output: 17562
@findolor You're absolutely right, my apologies for the incorrect review comment!
I verified in crates/common/src/raindex_client/orders.rs that:
- For WASM targets:
order_hash()returnsString(line 205) - For non-WASM targets:
order_hash()returnsB256(line 273)
So the current code in get_trade_count is correct:
#[cfg(not(target_family = "wasm"))]
let order_hash = self.order_hash(); // ✅ Directly assigns B256In fact, get_trade_count has the cleaner approach. Looking at get_trades_list (lines 135-138), it unnecessarily converts B256 → String → B256 for non-WASM targets, which is less efficient than your approach here.
Thank you for the correction!
🧠 Learnings used
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/raindex_client/trades.rs:215-223
Timestamp: 2025-07-04T10:24:56.163Z
Learning: In the rain.orderbook codebase, the get_trade_count method implementation that fetches all trades to count them is intentionally consistent with previous implementations and not considered a performance issue, as indicated by findolor for the trades counting functionality in crates/common/src/raindex_client/trades.rs.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2155
File: crates/common/src/raindex_client/trades.rs:133-152
Timestamp: 2025-10-06T14:13:18.531Z
Learning: In the rain.orderbook codebase, the `page` parameter in `RaindexOrder::get_trades_list` method (in crates/common/src/raindex_client/trades.rs) is kept for backwards compatibility with subgraph logic, but the LocalDb fast-path intentionally returns all trades without implementing pagination.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/raindex_client/orders.rs:609-609
Timestamp: 2025-07-04T10:27:22.544Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor prefers not to implement overflow protection for trades count casting (usize to u16) at this time, considering it unnecessary for the current scope since the practical risk of orders having 65,535+ trades is extremely low.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/common/src/raindex_client/orders.rs:738-741
Timestamp: 2025-10-02T19:17:20.332Z
Learning: In crates/common/src/raindex_client/orders.rs, fetch_dotrain_source() is intentionally called in try_from_sg_order for every order conversion because the dotrain source information is needed immediately. A future optimization with local DB logic is planned to eliminate the network round-trip concern.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2237
File: crates/common/src/raindex_client/local_db/sync.rs:79-89
Timestamp: 2025-10-18T10:38:41.273Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the sync_database method currently only supports indexing a single orderbook per chain ID, which is why `.first()` is used to select the orderbook configuration. Multi-orderbook support per chain ID is planned for future PRs.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1994
File: crates/common/src/raindex_client/vaults.rs:282-292
Timestamp: 2025-07-15T08:01:38.534Z
Learning: In the rainlanguage/rain.orderbook codebase, findolor prefers to avoid concurrency optimizations like using `futures::future::try_join_all` for parallel processing of balance changes, considering such optimizations "not that critical at the moment" when the performance impact is minimal.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1947
File: crates/common/src/raindex_client/orders.rs:98-125
Timestamp: 2025-06-24T08:46:03.368Z
Learning: In the vault merging logic in crates/common/src/raindex_client/orders.rs, optimization isn't necessary because the maximum list items are usually around 5 items. For such small datasets, the simple three-loop approach is preferred over HashMap-based optimization due to clarity and minimal performance impact.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: crates/js_api/src/gui/order_operations.rs:124-134
Timestamp: 2025-06-17T16:32:04.554Z
Learning: In the rain.orderbook codebase, RPC lists are typically small (2 items maximum), so performance optimizations around cloning and converting small Vec<Url> collections are generally unnecessary.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql:6-7
Timestamp: 2025-10-06T11:28:30.692Z
Learning: In `crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql`, the orderbook_address is currently hardcoded to '0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB' because the system only supports a single orderbook at the moment. Multiorderbook logic is not yet implemented and will be added in the future.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1744
File: crates/js_api/src/subgraph/order.rs:109-114
Timestamp: 2025-05-09T05:28:22.089Z
Learning: In the rain.orderbook project, SubgraphError contains or converts from OrderbookSubgraphClientError, so using OrderbookSubgraphClientError in error creation followed by a conversion to SubgraphError (via ? operator) is acceptable.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1706
File: crates/quote/src/js_api/mod.rs:57-61
Timestamp: 2025-04-29T06:19:15.446Z
Learning: In the rain.orderbook codebase, the `get_id` function is designed to use `U256::from_str()` rather than `U256::from_hex()` for parsing order hashes, despite the potential for hex string inputs.
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1831
File: packages/ui-components/src/lib/types/transaction.ts:71-77
Timestamp: 2025-05-19T10:39:19.157Z
Learning: In the Rain Orderbook project, `orderHash` is typed as `string` (not `Hex`) in the `InternalTransactionArgs` interface because it comes from the WASM function `getRemoveOrderCalldata` which returns a string value. Meanwhile, `txHash` is typed as `Hex` since it represents an actual blockchain transaction hash that requires the stricter type.
Learnt from: hardingjam
Repo: rainlanguage/rain.orderbook PR: 1831
File: packages/ui-components/src/lib/types/transaction.ts:71-77
Timestamp: 2025-05-19T10:39:19.157Z
Learning: In the Rain Orderbook project, `orderHash` is intentionally typed as `string` (not `Hex`) in the `InternalTransactionArgs` interface because it matches the return type of the `getRemoveOrderCalldata` WASM function which returns `Promise<string>`. String operations like `.slice()` are performed on `orderHash` in the code. Meanwhile, `txHash` is typed as `Hex` for blockchain transaction hashes.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1911
File: crates/subgraph/src/types/impls.rs:7-15
Timestamp: 2025-07-21T16:34:31.193Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb considers breaking changes that remove unsafe default behaviors to be intentional and acceptable. Specifically, the get_decimals() method in crates/subgraph/src/types/impls.rs was intentionally changed to return MissingDecimals error instead of defaulting to 18 decimals, as defaulting to 18 is considered unsafe and should never have been done.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/fuzz/mod.rs:64-64
Timestamp: 2025-07-04T09:02:57.301Z
Learning: In rainlanguage/rain.orderbook, user findolor prefers to limit type consistency changes to only the parts directly related to the current work scope. For example, when updating chain_id fields from u64 to u32 in fuzz-related code, unrelated files like tauri-app wallet commands can remain as u64 if they serve different purposes and aren't part of the current changes.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/js_api/src/gui/state_management.rs:196-203
Timestamp: 2025-09-24T10:59:25.666Z
Learning: In crates/js_api/src/gui/state_management.rs, the RainMetaDocumentV1Item::hash(false) method returns a slice, so wrapping the result with FixedBytes() is necessary to convert it to the expected FixedBytes type for DotrainGuiStateV1.dotrain_hash field.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1715
File: crates/js_api/src/common/mod.rs:15-22
Timestamp: 2025-05-14T05:52:04.270Z
Learning: The project doesn't require `#[repr(transparent)]` for newtype wrappers in WASM contexts such as `AddOrderCalldata` and `RemoveOrderCalldata` as the current implementation is working as expected without it.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like `#[cfg(not(target_family = "wasm"))] use super::*;` in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2202
File: crates/common/src/raindex_client/local_db/sync.rs:33-34
Timestamp: 2025-10-14T07:51:55.148Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the hard-coded `DEFAULT_SYNC_CHAIN_ID` constant (set to `SUPPORTED_LOCAL_DB_CHAINS[0]`) will be replaced with proper chain ID handling in downstream PRs as part of the multi-network/orderbook implementation.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/create_tables/query.sql:71-72
Timestamp: 2025-10-06T11:44:07.888Z
Learning: The local DB feature in the rain.orderbook codebase is not live yet (as of PR #2145), so schema migrations for existing databases are not required when modifying table structures in `crates/common/src/raindex_client/local_db/query/create_tables/query.sql`.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2117
File: crates/js_api/src/gui/select_tokens.rs:421-424
Timestamp: 2025-09-02T08:04:34.729Z
Learning: User findolor in the rainlanguage/rain.orderbook codebase prefers not to change test code when tests are already passing, even if there appear to be type mismatches or potential issues. Working tests should be left alone unless there are actual errors.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Chained PR
Motivation
We are using orders data source trade for fetching the orders list and getting an order by hash but we haven't really used it for the other functionality related to orders. This PR aims to expand the trait and implement the other queries and functions.
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.