Update repository to have proper types for local db logic#2198
Update repository to have proper types for local db logic#2198
Conversation
WalkthroughThis PR migrates many local DB, RPC, query, and CLI surfaces from hex/string representations to strongly-typed primitives (Address, Bytes, B256/U256, Float, u64), updates decoding/fetch/insert/SQL builder logic and serde helpers, and adjusts the sync pipeline and tests to use the new types. Several public struct fields and method signatures change. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI
participant Runner as Sync Runner
participant DS as DataSource
participant DB as LocalDb
Note over CLI,Runner: typed primitives (Address, Bytes, B256/U256, u64)
User->>CLI: raindex sync --orderbook <hex>
CLI->>CLI: parse hex -> Address
CLI->>Runner: SyncParams{orderbook_address: Address, ...}
Runner->>DS: fetch_events(Address, start, end)
DS->>DB: fetch_events(Address, start, end)
DB-->>DS: Vec<LogEntryResponse{address:Address, topics:Vec<Bytes>, data:Bytes, block_number:u64, ...}>
DS-->>Runner: events
Runner->>Runner: decode(events) -> DecodedEventData{block_number:u64, transaction_hash:B256, ...}
alt store addresses present
Runner->>DS: fetch_store_set_events(&[Address], start, end)
DS->>DB: fetch_store_set_events(&[Address], start, end)
DB-->>DS: Vec<LogEntryResponse>
DS-->>Runner: store set events
end
Runner->>DB: insert SQL using EventContext{transaction_hash:Bytes, block_number:u64, ...}
DB-->>Runner: ok
Runner-->>CLI: Sync complete
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
findolor
requested review from
0xgleb and
hardyjosh
and removed request for
0xgleb
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
crates/common/src/raindex_client/remove_orders.rs (1)
56-82: Optional: Consider direct byte operations in future refactors.Lines 66 and 120 convert typed
Bytesback to strings for API compatibility. While correct, these could be more efficient with direct byte operations if the underlying APIs (Id::new,hex::decode) were updated to accept byte slices.This is outside the scope of the current PR but worth noting for future optimization.
crates/cli/src/commands/local_db/sync/runner/window.rs (1)
98-104: Prevent potential u64 overflow in default_start_blockIf last_synced_block == u64::MAX, adding 1 overflows (wraps in release). Use saturating_add to avoid UB; compute_sync_window already uses checked_add and will error if needed.
pub(super) fn default_start_block(last_synced_block: u64, deployment_block: u64) -> u64 { if last_synced_block == 0 { deployment_block } else { - last_synced_block + 1 + last_synced_block.saturating_add(1) } }crates/common/src/raindex_client/local_db/query/fetch_vaults/mod.rs (1)
36-47: Optional: use lower-hex formatting directlySame minor cleanup as orders: format with 0x{:x} to avoid extra to_string/to_lowercase.
- .map(|owner| { - let owner_lower = owner.to_string().to_lowercase(); - format!("'{}'", owner_lower) - }) + .map(|owner| format!("'0x{:x}'", owner)) @@ - .map(|token| { - let token_lower = token.to_string().to_lowercase(); - format!("'{}'", token_lower) - }) + .map(|token| format!("'0x{:x}'", token))Also applies to: 49-60
crates/cli/src/commands/local_db/sync/runner/mod.rs (1)
182-200: Helper duplication: consider centralizing hex parsingparse_hex_field duplicates logic used elsewhere in the repo. Consider a shared utility to keep sorting consistent across modules.
crates/common/src/rpc_client.rs (1)
191-203: Optional: Reuse parse_hex_u64 in get_latest_block_number for consistencyYou can centralize hex parsing by using parse_hex_u64 here for consistent behavior and error messaging. Not required, but keeps one parsing path.
- let block_hex = block_hex.strip_prefix("0x").unwrap_or(&block_hex); - let block_number = u64::from_str_radix(block_hex, 16)?; - Ok(block_number) + match parse_hex_u64(&block_hex) { + Ok(n) => Ok(n), + Err(e) => Err(RpcClientError::RpcError { message: e }), + }crates/common/src/raindex_client/local_db/fetch.rs (1)
123-130: Bug: log_index sorted lexicographically, not numerically, in fetch_eventsFor same-block events, "0x10" will sort before "0x2". The store_set path already uses numeric ordering; align fetch_events.
Fix by reusing the existing sorter:
- all_events.sort_by(|a, b| { - let block_a = extract_block_number_from_entry(a).unwrap_or(0); - let block_b = extract_block_number_from_entry(b).unwrap_or(0); - block_a - .cmp(&block_b) - .then_with(|| a.log_index.cmp(&b.log_index)) - }); + Self::sort_events_by_block_and_log(&mut all_events);Optional: add a test asserting correct order for same-block log_index values.
crates/cli/src/commands/local_db/sync/data_source.rs (1)
98-106: Remove unnecessary clone of store_addressesYou can pass the slice directly to LocalDb::fetch_store_set_events.
- let parsed_addresses: Vec<Address> = store_addresses.to_vec(); <LocalDb>::fetch_store_set_events( self, - parsed_addresses.as_slice(), + store_addresses, start_block, end_block, &FetchConfig::default(), )crates/common/src/raindex_client/local_db/insert.rs (1)
841-846: Support uppercase 0X prefix in hex_to_decimalBe lenient to both 0x and 0X.
-fn hex_to_decimal(hex_str: &str) -> Result<u64, InsertError> { - let hex_str_clean = hex_str.strip_prefix("0x").unwrap_or(hex_str); +fn hex_to_decimal(hex_str: &str) -> Result<u64, InsertError> { + let hex_str_clean = hex_str + .strip_prefix("0x") + .or_else(|| hex_str.strip_prefix("0X")) + .unwrap_or(hex_str); u64::from_str_radix(hex_str_clean, 16).map_err(|_| InsertError::HexParseError { hex_str: hex_str.to_string(), }) }crates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs (2)
42-59: Trim parts when parsing IO stringsHandle incidental whitespace for robustness.
- if let Some(s) = io { - for part in s.split(',') { + if let Some(s) = io { + for part in s.split(',').map(|p| p.trim()).filter(|p| !p.is_empty()) { let mut segs = part.split(':'); if let (Some(idx), Some(vault_id), Some(token)) = (segs.next(), segs.next(), segs.next()) { if let Ok(index) = idx.parse::<usize>() { items.push((index, vault_id.to_string(), token.to_string())); } } }
27-41: API consistency with typed paramsOther queries (e.g., fetch_vault_balance_changes) moved to Address/U256 params. Confirm keeping vault_id/token as &str here is intentional; if not, consider migrating for uniformity.
crates/common/src/raindex_client/local_db/sync.rs (2)
212-221: Add HTTP timeout to avoid hangs during dump downloadSet a reasonable timeout on the reqwest client.
- let client = Client::new(); + let client = Client::builder() + .timeout(std::time::Duration::from_secs(30)) + .build()?;
394-404: DRY: consolidate hex parsing helpersparse_hex_field here and hex_to_decimal in insert.rs do similar jobs. Consider a shared helper to reduce duplication and edge-case drift (e.g., uppercase 0X).
Also applies to: 841-846
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (39)
crates/cli/src/commands/local_db/decode_events.rs(1 hunks)crates/cli/src/commands/local_db/decoded_events_to_sql.rs(3 hunks)crates/cli/src/commands/local_db/fetch_events.rs(10 hunks)crates/cli/src/commands/local_db/sync/cli.rs(2 hunks)crates/cli/src/commands/local_db/sync/data_source.rs(3 hunks)crates/cli/src/commands/local_db/sync/runner/apply.rs(9 hunks)crates/cli/src/commands/local_db/sync/runner/mod.rs(17 hunks)crates/cli/src/commands/local_db/sync/runner/window.rs(4 hunks)crates/cli/src/commands/local_db/sync/token.rs(2 hunks)crates/cli/src/commands/local_db/tokens_fetch.rs(2 hunks)crates/common/src/raindex_client/add_orders.rs(2 hunks)crates/common/src/raindex_client/local_db/decode.rs(23 hunks)crates/common/src/raindex_client/local_db/fetch.rs(17 hunks)crates/common/src/raindex_client/local_db/insert.rs(29 hunks)crates/common/src/raindex_client/local_db/mod.rs(4 hunks)crates/common/src/raindex_client/local_db/query/clear_tables/mod.rs(1 hunks)crates/common/src/raindex_client/local_db/query/create_tables/mod.rs(1 hunks)crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rs(4 hunks)crates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rs(1 hunks)crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs(5 hunks)crates/common/src/raindex_client/local_db/query/fetch_order_trades_count/mod.rs(6 hunks)crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rs(10 hunks)crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs(2 hunks)crates/common/src/raindex_client/local_db/query/fetch_tables/mod.rs(1 hunks)crates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs(6 hunks)crates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rs(3 hunks)crates/common/src/raindex_client/local_db/query/fetch_vaults/mod.rs(8 hunks)crates/common/src/raindex_client/local_db/query/mod.rs(1 hunks)crates/common/src/raindex_client/local_db/query/update_last_synced_block/mod.rs(1 hunks)crates/common/src/raindex_client/local_db/sync.rs(31 hunks)crates/common/src/raindex_client/local_db/tokens.rs(1 hunks)crates/common/src/raindex_client/orders.rs(14 hunks)crates/common/src/raindex_client/remove_orders.rs(2 hunks)crates/common/src/raindex_client/trades.rs(13 hunks)crates/common/src/raindex_client/transactions.rs(1 hunks)crates/common/src/raindex_client/vaults.rs(36 hunks)crates/common/src/raindex_client/vaults_list.rs(2 hunks)crates/common/src/rpc_client.rs(4 hunks)packages/orderbook/test/js_api/raindexClient.test.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
crates/*/{src,tests,benches,examples}/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Rust code must pass clippy with all warnings denied (cargo clippy --workspace --all-targets --all-features -D warnings)
Files:
crates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/transactions.rscrates/common/src/raindex_client/local_db/query/clear_tables/mod.rscrates/common/src/raindex_client/vaults_list.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades_count/mod.rscrates/cli/src/commands/local_db/sync/data_source.rscrates/cli/src/commands/local_db/decode_events.rscrates/cli/src/commands/local_db/decoded_events_to_sql.rscrates/common/src/raindex_client/local_db/query/fetch_tables/mod.rscrates/cli/src/commands/local_db/fetch_events.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/common/src/raindex_client/remove_orders.rscrates/cli/src/commands/local_db/sync/token.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/decode.rscrates/cli/src/commands/local_db/sync/runner/window.rscrates/common/src/raindex_client/local_db/query/create_tables/mod.rscrates/common/src/raindex_client/local_db/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rscrates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/local_db/query/fetch_orders/mod.rscrates/cli/src/commands/local_db/sync/cli.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/query/update_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rscrates/common/src/raindex_client/local_db/sync.rscrates/common/src/raindex_client/local_db/tokens.rscrates/cli/src/commands/local_db/tokens_fetch.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vaults/mod.rscrates/common/src/raindex_client/local_db/fetch.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rscrates/common/src/raindex_client/local_db/insert.rscrates/common/src/rpc_client.rscrates/common/src/raindex_client/vaults.rscrates/common/src/raindex_client/local_db/query/mod.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Rust code lives in the workspace under crates/* (e.g., cli, common, bindings, js_api, quote, subgraph, settings, math, integration_tests)
Files:
crates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/transactions.rscrates/common/src/raindex_client/local_db/query/clear_tables/mod.rscrates/common/src/raindex_client/vaults_list.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades_count/mod.rscrates/cli/src/commands/local_db/sync/data_source.rscrates/cli/src/commands/local_db/decode_events.rscrates/cli/src/commands/local_db/decoded_events_to_sql.rscrates/common/src/raindex_client/local_db/query/fetch_tables/mod.rscrates/cli/src/commands/local_db/fetch_events.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/common/src/raindex_client/remove_orders.rscrates/cli/src/commands/local_db/sync/token.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/decode.rscrates/cli/src/commands/local_db/sync/runner/window.rscrates/common/src/raindex_client/local_db/query/create_tables/mod.rscrates/common/src/raindex_client/local_db/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rscrates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/local_db/query/fetch_orders/mod.rscrates/cli/src/commands/local_db/sync/cli.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/query/update_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rscrates/common/src/raindex_client/local_db/sync.rscrates/common/src/raindex_client/local_db/tokens.rscrates/cli/src/commands/local_db/tokens_fetch.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vaults/mod.rscrates/common/src/raindex_client/local_db/fetch.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rscrates/common/src/raindex_client/local_db/insert.rscrates/common/src/rpc_client.rscrates/common/src/raindex_client/vaults.rscrates/common/src/raindex_client/local_db/query/mod.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust with nix develop -c cargo fmt --all
Lint Rust with nix develop -c rainix-rs-static (preconfigured flags)
Use snake_case for Rust crates/modules and PascalCase for types
Files:
crates/common/src/raindex_client/add_orders.rscrates/common/src/raindex_client/transactions.rscrates/common/src/raindex_client/local_db/query/clear_tables/mod.rscrates/common/src/raindex_client/vaults_list.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades_count/mod.rscrates/cli/src/commands/local_db/sync/data_source.rscrates/cli/src/commands/local_db/decode_events.rscrates/cli/src/commands/local_db/decoded_events_to_sql.rscrates/common/src/raindex_client/local_db/query/fetch_tables/mod.rscrates/cli/src/commands/local_db/fetch_events.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/common/src/raindex_client/remove_orders.rscrates/cli/src/commands/local_db/sync/token.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/decode.rscrates/cli/src/commands/local_db/sync/runner/window.rscrates/common/src/raindex_client/local_db/query/create_tables/mod.rscrates/common/src/raindex_client/local_db/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rscrates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/local_db/query/fetch_orders/mod.rscrates/cli/src/commands/local_db/sync/cli.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/query/update_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rscrates/common/src/raindex_client/local_db/sync.rscrates/common/src/raindex_client/local_db/tokens.rscrates/cli/src/commands/local_db/tokens_fetch.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vaults/mod.rscrates/common/src/raindex_client/local_db/fetch.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rscrates/common/src/raindex_client/local_db/insert.rscrates/common/src/rpc_client.rscrates/common/src/raindex_client/vaults.rscrates/common/src/raindex_client/local_db/query/mod.rs
packages/orderbook/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Orderbook TypeScript package must pass type/lint checks (npm run check -w @rainlanguage/orderbook)
Files:
packages/orderbook/test/js_api/raindexClient.test.ts
packages/orderbook/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Orderbook tests must pass (npm run test -w @rainlanguage/orderbook)
Files:
packages/orderbook/test/js_api/raindexClient.test.ts
packages/**
📄 CodeRabbit inference engine (AGENTS.md)
JavaScript/Svelte projects live under packages/* (webapp, ui-components, orderbook)
Files:
packages/orderbook/test/js_api/raindexClient.test.ts
packages/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name TypeScript test files as *.test.ts or *.spec.ts (Vitest)
Files:
packages/orderbook/test/js_api/raindexClient.test.ts
🧠 Learnings (13)
📚 Learning: 2025-04-29T06:19:15.446Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/add_orders.rscrates/common/src/raindex_client/orders.rs
📚 Learning: 2025-07-18T10:31:05.498Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/local_db/query/clear_tables/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades_count/mod.rscrates/common/src/raindex_client/local_db/query/fetch_tables/mod.rscrates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/query/create_tables/mod.rscrates/common/src/raindex_client/local_db/query/update_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vaults/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
📚 Learning: 2025-05-14T05:13:59.713Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1744
File: crates/subgraph/src/orderbook_client.rs:44-46
Timestamp: 2025-05-14T05:13:59.713Z
Learning: In the rain.orderbook project, WASM dependencies are intentionally made available in non-WASM targets to facilitate testing of WASM-related functionality, so conditional compilation guards like `#[cfg(target_family = "wasm")]` should not be added to imports or implementations that may be needed for tests.
Applied to files:
crates/common/src/raindex_client/local_db/query/clear_tables/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades_count/mod.rscrates/common/src/raindex_client/local_db/query/fetch_tables/mod.rscrates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/query/create_tables/mod.rscrates/common/src/raindex_client/local_db/query/update_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vaults/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
📚 Learning: 2025-04-30T09:28:36.960Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1715
File: crates/js_api/src/common/mod.rs:111-118
Timestamp: 2025-04-30T09:28:36.960Z
Learning: In the rain.orderbook repository, the WASM tests are already properly configured with conditional compilation using `#[cfg(target_family = "wasm")]` and `#[cfg(not(target_family = "wasm"))]`, and don't require additional `wasm_bindgen_test_configure!(run_in_browser)` directives.
Applied to files:
crates/common/src/raindex_client/local_db/query/clear_tables/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades_count/mod.rscrates/common/src/raindex_client/local_db/query/fetch_tables/mod.rscrates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/query/create_tables/mod.rscrates/common/src/raindex_client/local_db/query/update_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vaults/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
📚 Learning: 2025-06-24T13:30:02.968Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1950
File: crates/common/src/raindex_client/transactions.rs:34-38
Timestamp: 2025-06-24T13:30:02.968Z
Learning: When using #[wasm_bindgen] on an impl block, all methods within that block must have wasm_bindgen attributes, even if they are conditionally compiled for non-WASM targets using #[cfg(not(target_family = "wasm"))]. Removing these attributes causes compiler errors because the wasm_bindgen macro expansion processes the entire impl block and expects consistent attribute usage across all methods.
Applied to files:
crates/common/src/raindex_client/local_db/query/clear_tables/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades_count/mod.rscrates/common/src/raindex_client/local_db/query/fetch_tables/mod.rscrates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/query/create_tables/mod.rscrates/common/src/raindex_client/local_db/query/update_last_synced_block/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vaults/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
📚 Learning: 2025-05-20T10:20:08.206Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1859
File: crates/quote/src/quote_debug.rs:472-492
Timestamp: 2025-05-20T10:20:08.206Z
Learning: In the Rain Orderbook codebase, the `#[tokio::test(flavor = "multi_thread")]` annotation is specifically needed for tests that use `LocalEvm`, not just for consistency across all async tests.
Applied to files:
crates/cli/src/commands/local_db/decoded_events_to_sql.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/cli/src/commands/local_db/sync/token.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/sync.rscrates/cli/src/commands/local_db/tokens_fetch.rs
📚 Learning: 2025-05-19T10:39:19.157Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#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.
Applied to files:
crates/common/src/raindex_client/orders.rs
📚 Learning: 2025-10-06T11:28:30.692Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/orders.rs
📚 Learning: 2025-10-06T14:13:18.531Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/local_db/query/fetch_order_trades/mod.rs
📚 Learning: 2025-07-16T10:40:05.717Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/local_db/query/fetch_vault/mod.rscrates/common/src/raindex_client/vaults.rs
📚 Learning: 2025-07-04T10:24:56.163Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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.rs
📚 Learning: 2025-08-02T02:34:32.237Z
Learnt from: brusherru
PR: rainlanguage/rain.orderbook#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/query/fetch_store_addresses/mod.rs
📚 Learning: 2025-07-31T19:34:11.716Z
Learnt from: brusherru
PR: rainlanguage/rain.orderbook#2044
File: crates/common/src/raindex_client/vaults_list.rs:363-423
Timestamp: 2025-07-31T19:34:11.716Z
Learning: In the rainlanguage/rain.orderbook project, for WASM-exposed functionality like VaultsList, the team prefers to keep comprehensive tests in the non-WASM environment due to the complexity of recreating objects like RaindexVaults in WASM. WASM tests focus on basic functionality and error cases since the WASM code reuses the already-tested non-WASM implementation.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
🧬 Code graph analysis (25)
crates/common/src/raindex_client/transactions.rs (1)
tauri-app/src-tauri/src/commands/trade_debug.rs (1)
tx_hash(26-26)
crates/common/src/raindex_client/local_db/query/fetch_order_trades_count/mod.rs (2)
crates/common/src/raindex_client/orders.rs (2)
order_hash(88-90)order_hash(156-158)crates/common/src/raindex_client/local_db/query/mod.rs (1)
create_success_callback(439-453)
crates/cli/src/commands/local_db/sync/data_source.rs (5)
crates/cli/src/commands/local_db/sync/runner/apply.rs (1)
fetch_store_set_events(174-181)crates/cli/src/commands/local_db/sync/runner/mod.rs (1)
fetch_store_set_events(294-306)crates/cli/src/commands/local_db/sync/runner/window.rs (1)
fetch_store_set_events(164-171)crates/common/src/raindex_client/local_db/fetch.rs (1)
fetch_store_set_events(136-164)crates/common/src/raindex_client/local_db/sync.rs (4)
fetch_store_set_events(60-65)fetch_store_set_events(162-178)fetch_store_set_events(1306-1314)fetch_store_set_events(1456-1473)
crates/cli/src/commands/local_db/decode_events.rs (1)
crates/common/src/raindex_client/local_db/decode.rs (1)
event(158-166)
crates/cli/src/commands/local_db/decoded_events_to_sql.rs (1)
crates/common/src/raindex_client/local_db/decode.rs (1)
U256(243-243)
crates/cli/src/commands/local_db/fetch_events.rs (3)
crates/cli/src/commands/local_db/sync/data_source.rs (2)
fetch(48-49)fetch(56-69)crates/common/src/raindex_client/vaults.rs (3)
address(235-237)address(260-262)new(176-181)crates/common/src/raindex_client/local_db/sync.rs (6)
new(84-86)new(122-124)new(1150-1154)new(1178-1192)new(1262-1276)new(1368-1379)
crates/cli/src/commands/local_db/sync/runner/apply.rs (4)
crates/cli/src/commands/local_db/sync/data_source.rs (2)
fetch_events(19-24)fetch_events(81-90)crates/cli/src/commands/local_db/sync/runner/mod.rs (1)
fetch_events(280-292)crates/cli/src/commands/local_db/sync/runner/window.rs (1)
fetch_events(155-162)crates/common/src/raindex_client/local_db/fetch.rs (1)
fetch_events(41-54)
crates/common/src/raindex_client/orders.rs (2)
crates/common/src/raindex_client/trades.rs (2)
order_hash(42-44)order_hash(72-74)crates/common/src/raindex_client/mod.rs (2)
new_test_client_with_db_callback(534-541)get_local_db_test_yaml(544-588)
crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rs (1)
crates/common/src/raindex_client/vaults.rs (2)
address(235-237)address(260-262)
crates/common/src/raindex_client/local_db/decode.rs (4)
crates/common/src/raindex_client/local_db/insert.rs (2)
bytes(1284-1286)value(41-41)crates/common/src/raindex_client/local_db/query/mod.rs (10)
serialize(93-98)serialize(114-119)serialize(136-141)serialize(158-169)serialize(187-192)deserialize(100-106)deserialize(121-127)deserialize(143-149)deserialize(171-179)deserialize(194-200)crates/common/src/rpc_client.rs (5)
serialize(88-93)serialize(108-116)deserialize(95-101)deserialize(118-125)from(313-315)crates/common/src/raindex_client/orders.rs (3)
try_from(385-391)try_from(395-401)try_from(800-821)
crates/common/src/raindex_client/local_db/mod.rs (1)
crates/common/src/raindex_client/local_db/decode.rs (2)
U256(243-243)event(158-166)
crates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rs (2)
crates/common/src/raindex_client/local_db/decode.rs (1)
U256(243-243)crates/common/src/raindex_client/local_db/query/mod.rs (2)
execute_query_json(68-78)create_success_callback(439-453)
crates/common/src/raindex_client/trades.rs (4)
crates/common/src/raindex_client/orders.rs (9)
order_hash(88-90)order_hash(156-158)chain_id(76-78)chain_id(147-149)transaction(117-119)transaction(177-179)order_hash_bytes(247-257)order(844-855)order(858-869)crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs (2)
LocalDbQuery(74-74)fetch_order_trades(52-75)crates/common/src/raindex_client/local_db/query/fetch_order_trades_count/mod.rs (2)
LocalDbQuery(33-33)fetch_order_trades_count(13-37)crates/common/src/raindex_client/vaults.rs (7)
chain_id(86-88)chain_id(133-135)chain_id(227-229)chain_id(254-256)try_from_local_trade_side(822-867)transaction(660-662)transaction(700-702)
crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rs (1)
crates/common/src/raindex_client/orders.rs (4)
owner(92-94)owner(159-161)order_hash(88-90)order_hash(156-158)
crates/cli/src/commands/local_db/sync/runner/mod.rs (3)
crates/common/src/raindex_client/local_db/insert.rs (3)
value(41-41)raw_events(181-200)addr(1280-1282)crates/common/src/raindex_client/local_db/fetch.rs (1)
sort_events_by_block_and_log(340-348)crates/common/src/raindex_client/local_db/sync.rs (2)
sort_events_by_block_and_log(386-392)parse_hex_field(394-404)
crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs (1)
crates/common/src/raindex_client/local_db/query/mod.rs (1)
create_success_callback(439-453)
crates/common/src/raindex_client/local_db/sync.rs (3)
crates/cli/src/commands/local_db/sync/data_source.rs (2)
fetch_events(19-24)fetch_events(81-90)crates/cli/src/commands/local_db/sync/runner/mod.rs (2)
fetch_events(280-292)parse_hex_field(190-200)crates/common/src/raindex_client/local_db/fetch.rs (1)
fetch_events(41-54)
crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs (3)
crates/common/src/raindex_client/orders.rs (2)
order_hash(88-90)order_hash(156-158)crates/common/src/raindex_client/trades.rs (2)
order_hash(42-44)order_hash(72-74)crates/common/src/raindex_client/local_db/query/mod.rs (1)
create_success_callback(439-453)
crates/common/src/raindex_client/local_db/query/fetch_vaults/mod.rs (2)
crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rs (1)
from(36-49)crates/common/src/raindex_client/vaults.rs (18)
owner(98-100)owner(142-144)token(114-116)token(154-156)token(627-629)token(676-678)token(887-889)token(900-902)vault_id(102-104)vault_id(145-147)vault_id(622-625)vault_id(673-675)vault_id(1514-1514)balance(106-108)balance(148-150)balance(187-189)balance(197-199)vaults(1143-1164)
crates/common/src/raindex_client/local_db/fetch.rs (3)
crates/common/src/raindex_client/local_db/decode.rs (1)
event(158-166)crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rs (1)
addresses(26-29)crates/cli/src/commands/local_db/sync/storage.rs (1)
addresses(86-89)
crates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs (2)
crates/common/src/raindex_client/local_db/decode.rs (1)
U256(243-243)crates/common/src/raindex_client/vaults.rs (6)
vault(1439-1443)vault(1444-1448)balance(106-108)balance(148-150)balance(187-189)balance(197-199)
crates/common/src/raindex_client/local_db/insert.rs (2)
crates/common/src/raindex_client/local_db/decode.rs (1)
event(158-166)crates/common/src/raindex_client/local_db/tokens.rs (1)
build_event(73-82)
crates/common/src/rpc_client.rs (1)
crates/common/src/raindex_client/local_db/query/mod.rs (10)
serialize(93-98)serialize(114-119)serialize(136-141)serialize(158-169)serialize(187-192)deserialize(100-106)deserialize(121-127)deserialize(143-149)deserialize(171-179)deserialize(194-200)
crates/common/src/raindex_client/vaults.rs (4)
crates/common/src/raindex_client/orders.rs (7)
id(80-82)id(150-152)transaction(117-119)transaction(177-179)chain_id(76-78)chain_id(147-149)try_from_local_db_orders_csv(405-467)crates/common/src/raindex_client/transactions.rs (5)
id(23-25)id(43-45)from_local_parts(58-70)from(27-29)from(46-48)crates/common/src/raindex_client/local_db/decode.rs (1)
U256(243-243)crates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs (1)
fetch_vault(27-40)
crates/common/src/raindex_client/local_db/query/mod.rs (4)
crates/common/src/rpc_client.rs (4)
serialize(88-93)serialize(108-116)deserialize(95-101)deserialize(118-125)crates/common/src/raindex_client/local_db/insert.rs (1)
value(41-41)crates/common/src/withdraw.rs (1)
parse(70-71)crates/common/src/raindex_client/local_db/decode.rs (1)
U256(243-243)
⏰ 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). (17)
- GitHub Check: test
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: git-clean
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: Deploy-Preview-Push
- GitHub Check: Deploy-Docs-Preview
🔇 Additional comments (77)
crates/common/src/raindex_client/local_db/query/create_tables/mod.rs (1)
32-33: LGTM! Prevents unused import warning.The
#[cfg(target_family = "wasm")]guard appropriately restricts the import to wasm builds, preventing an unused import warning in non-wasm test builds since this module only contains wasm-specific tests.Based on learnings.
crates/common/src/raindex_client/local_db/query/clear_tables/mod.rs (1)
25-26: Scoped wasm import avoids native warnings.Conditionally pulling
super::*only for wasm builds keeps the wasm-specific tests compiling cleanly while eliminating unused import noise on native targets—nice tidy-up.crates/common/src/raindex_client/remove_orders.rs (2)
328-331: LGTM!The test assertion correctly compares
token.id()asBytesinstead of string, aligning with the PR's migration to strongly-typed primitives.
382-385: LGTM!Consistent with the output token assertion, this correctly compares
token.id()asBytes.crates/common/src/raindex_client/add_orders.rs (3)
48-52: LGTM: Type-safe API boundary.The conversion from string parameters to typed primitives (Address, Bytes) at the WASM binding boundary provides proper type safety for the public API.
285-288: LGTM: Test assertions correctly updated to use typed Bytes.The test assertions for token IDs now use
Bytes::from_strinstead of string comparisons, which is consistent with the PR's goal of migrating to strongly-typed primitives.Also applies to: 339-342
64-66: Retain the round-trip conversion asId::newcurrently only accepts string inputs; consider extending it to acceptBytesin a future refactor.crates/common/src/raindex_client/local_db/query/fetch_order_trades_count/mod.rs (5)
2-2: LGTM!The
alloy::primitives::Bytesimport is appropriate for the type migration from string-based to typed order hashes.
28-31: LGTM!The SQL query placeholder replacement logic is correct. The
order_hash_literal(which includes quotes) replaces the placeholder'?order_hash'(also with quotes), ensuring proper SQL syntax.
42-43: LGTM!The test updates correctly migrate from string-based to
Bytes-based order hashes:
- Conditional imports align with the codebase's WASM testing patterns (as per learnings)
Bytes::from_str()with.unwrap()is acceptable in tests- Tests verify both the SQL literal format and placeholder replacement logic
The test at line 89 confirms the expected format is lowercase hex with
0xprefix, which matches the earlier concern aboutBytes::Displayformat consistency.Also applies to: 51-51, 58-60, 70-72, 84-93
13-18: Verified all call sites now passBytes. Both the production usage in raindex_client/trades.rs and all tests invokefetch_order_trades_countwith aBytesargument.
19-19: Formatting logic is correct Bytes::Display outputs lowercase hex with a0xprefix, soformat!("'{}'", order_hash)yields the expected SQL literal.crates/cli/src/commands/local_db/sync/token.rs (2)
87-87: Import additions look correctAddress/Bytes/U256 are appropriate for the updated test scaffolding.
125-128: Good type migration in test dataUsing numeric zeroes and a 32-byte Bytes for transaction_hash aligns with the new models.
crates/common/src/raindex_client/orders.rs (2)
717-717: Bytes plumbed through LocalDB and subgraph pathsPassing Bytes to LocalDB and wrapping in SgBytes for subgraph is consistent and correct.
Also applies to: 729-731
740-745: LocalDB fetch by Bytes-hashSignature and FetchOrdersArgs updates to Bytes are coherent with the repo-wide type shift.
crates/common/src/raindex_client/trades.rs (3)
137-142: LocalDB path now uses order_hash_bytes()Correctly retrieves Bytes and forwards to fetch_order_trades; aligns with new query signature.
248-252: Trade count via Bytes order_hashConsistent with fetch_order_trades_count API; looks good.
308-359: Local trade conversion uses typed fields end-to-endTransaction, token info, balances, and orderbook are all typed; construction matches downstream expectations.
crates/common/src/raindex_client/vaults_list.rs (1)
264-269: Test fixtures updated to address-based token IDsSwitch to real addresses for token.id is correct and consistent with the new types.
Also applies to: 286-291
crates/common/src/raindex_client/local_db/query/fetch_tables/mod.rs (1)
20-21: cfg gate on test import: confirm necessityGating
use super::*;to wasm-only is likely fine here, but please ensure it matches patterns across LocalDB query tests and doesn’t hide non-wasm test/build issues. If this was added to appease warnings, keep it; otherwise consider leaving the import ungated for consistency.Based on learnings
crates/common/src/raindex_client/local_db/query/update_last_synced_block/mod.rs (1)
21-21: LGTM!The conditional compilation guard aligns with the project's pattern for WASM test modules.
crates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rs (1)
47-47: LGTM!The conditional compilation guard is consistent with the project's WASM testing pattern.
crates/cli/src/commands/local_db/tokens_fetch.rs (2)
89-89: LGTM!The addition of
Bytesto the imports aligns with the type migration for transaction hashes in test data.
108-110: LGTM!The test data construction now uses proper typed values (
0for block fields andBytesfor transaction hash) instead of string representations, aligning with the broader PR migration.crates/common/src/raindex_client/local_db/tokens.rs (1)
76-78: LGTM!The test helper now constructs event data with typed primitives (
0for numeric fields,Bytesfor transaction hash), consistent with the codebase-wide migration.crates/cli/src/commands/local_db/sync/cli.rs (3)
50-51: LGTM!The two-step approach (storing the typed
Addressand separately computing the hex string for display) is clean and aligns with the migration to strongly-typed addresses.
57-63: LGTM!The display logic correctly uses the hex-encoded string representation of the address.
70-76: LGTM!Passing
orderbook_addressby value (instead of by reference) indicates thatSyncParamshas been updated to take ownership of theAddressvalue, consistent with the type migration.crates/cli/src/commands/local_db/decoded_events_to_sql.rs (3)
115-115: LGTM!The import addition aligns with the use of
Bytesfor transaction hashes in test data.
126-128: LGTM!The test helper now uses numeric literals for block fields and
Bytesfor the transaction hash, consistent with the type migration.
236-238: LGTM!The test data construction correctly uses hex numeric literals and
Bytes, aligning with the typed data model.packages/orderbook/test/js_api/raindexClient.test.ts (3)
1851-1873: LGTM!The test data now uses full Ethereum addresses as token IDs instead of short identifiers, aligning with the migration to proper address types across the codebase.
1888-1906: LGTM!The assertions correctly reference the updated token IDs (full addresses).
1912-1931: LGTM!The filtered test correctly uses the full address format for token IDs.
crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rs (4)
2-2: LGTM!The import addition enables the use of
Addresstype for theaddressfield.
9-10: Public API change: String → Address for address field.The
addressfield type has changed fromStringtoalloy::primitives::Address, with appropriate serde support viaserde_address. This aligns with the PR objective to provide proper type support for local DB logic.
42-42: LGTM!The conditional compilation guard is consistent with the project's WASM testing pattern. Based on learnings.
85-96: LGTM!Test data construction correctly uses
Address::from([0x01; 20])andAddress::from([0x02; 20])to create typed address values, consistent with the migration.crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs (1)
32-46: LGTM: tests now use typed AddressTests correctly construct
AddressviaFromStrand compare round-tripped JSON using the custom serde. Looks good.crates/cli/src/commands/local_db/decode_events.rs (1)
108-119: LGTM: event fixture migrated to typed fields
dataasBytesandtopicsas 32‑byteBytesalign with decoding expectations.address,block_number,block_timestamp,transaction_hash,block_hashnow strongly typed.crates/common/src/raindex_client/local_db/query/mod.rs (4)
88-107: Address serde helper is solidHex with 0x prefix via
encode_prefixedandAddress::from_stris consistent and reversible.
109-128: Bytes serde helper looks correct0x‑prefixed hex for
ByteswithFromStrparsing is appropriate and consistent.
130-150: Float serde helper reads wellFormatting on serialize and strict parse on deserialize with clear error mapping. Good.
152-180: Option serde helper is consistentOptional handling and error mapping are correct.
crates/common/src/raindex_client/transactions.rs (2)
59-69: Constructor signature change is good; removes redundant parsingAccepting
Bytes/Addressdirectly simplifies callers and avoids parse overhead. Implementation is correct.
57-63: All call sites use ownedBytesandAddressVerified that everyfrom_local_partscall suppliesBytesandAddress—no&strusages found.crates/common/src/raindex_client/local_db/mod.rs (2)
272-282: LGTM: typed LogEntryResponse test fixture
address,topics(32‑byte),data, and hashes/timestamps updated to typed fields correctly; compatible with decoding.
311-315: LGTM: decoded event with invalid field for error mappingUsing decimal u64s and a non‑hex
log_indexmaintains the intended failure path for SQL generation.Also applies to: 322-322
crates/cli/src/commands/local_db/sync/runner/window.rs (3)
36-37: Signature update to typed SyncParams looks goodMoving to &SyncParams (no lifetime) aligns with Address-based params and simplifies call sites.
155-169: Typed data source API (Address, &[Address])Switching fetch_events/fetch_store_set_events to Address/addresses matches the repo-wide type migration. Mocks updated correctly.
199-207: Test params updated to AddressUsing Address::from([0x11; 20]) in params() keeps tests consistent with new types.
crates/cli/src/commands/local_db/fetch_events.rs (2)
30-35: LocalDb impl correctly delegates with AddressDirectly forwarding Address to fetch_events is correct.
16-20: Trait fetch(Address, ..) change is sensible
No remaining&strliteral call sites detected across the workspace; all implementors and callers have been updated.crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rs (2)
17-22: Args migrated to typed Address/BytesStrongly-typed owners/tokens/order_hash reduces parsing and injection risks. Good change.
43-48: Direct mapping from filters is correctUsing filters.owners/order_hash/tokens.unwrap_or_default() is appropriate with typed fields.
crates/common/src/raindex_client/local_db/query/fetch_vaults/mod.rs (3)
8-12: Args migrated to typed Address vectorsTyped owners/tokens is consistent with the PR direction. Good.
16-21: From mapping looks correctDirect assignment and unwrap_or_default for tokens is appropriate.
89-100: WASM test gating acknowledgedImport/test gating for wasm-only tests matches repo patterns; no issues.
crates/cli/src/commands/local_db/sync/runner/mod.rs (3)
32-38: SyncParams.orderbook_address: AddressPublic surface change is coherent with the rest of the refactor.
59-66: run(&SyncParams) change OKAPI simplification without lifetime is fine.
282-291: Formatting orderbook address in mock fetch_callsUsing format!("0x{:x}", orderbook_address) ensures consistent lowercase hex; good for assertions.
crates/common/src/rpc_client.rs (2)
53-65: Typed LogEntryResponse (Address/Bytes/u64) is solidSerialization/deserialization choices look correct and align with downstream decode paths. Good move away from strings.
84-126: Serde helpers for hex/u64 look correctHex parsing handles 0x/0X and decimal fallback; serializers emit minimal 0x lowercase. This should interop cleanly with RPC.
crates/common/src/raindex_client/local_db/fetch.rs (2)
241-247: Address dedupe is fineUsing {:#x} and lowercasing ensures case-insensitive dedupe and RPC-friendly strings. LGTM.
170-200: Concurrent timestamp fetch and collection looks goodThe retry/backoff and collection into HashMap via Result iterator is idiomatic and correct.
crates/common/src/raindex_client/local_db/decode.rs (3)
38-50: Topic matching on Bytes is correct and preciseDirect slice equality against SIGNATURE_HASH removes fragile hex conversions. Good change.
162-169: Safe topic conversion and ABI decodeB256::try_from with explicit length check and passing raw data slice is correct. Error mapping is clean.
171-199: Store-set decode is robustBounds checks and direct use of event.address improve safety and clarity. LGTM.
crates/cli/src/commands/local_db/sync/runner/apply.rs (1)
20-34: Address-based fetch_events signature plumbed correctlyType migration is clean; function passes through and tests adjusted appropriately.
crates/common/src/raindex_client/local_db/insert.rs (1)
240-278: Token upsert SQL builder is solidGood escaping via sql_string_literal and ON CONFLICT handling. No issues.
crates/common/src/raindex_client/vaults.rs (6)
216-216: Typed token id upgrade looks goodSwitching RaindexVaultToken.id to Bytes and aligning WASM/non‑WASM getters is correct and consistent with the PR’s objectives.
Also applies to: 230-233, 257-259
795-816: Local DB balance change mapping math LGTMUsing Float and computing old_balance = running_balance − delta is correct for both deposits and withdrawals.
822-867: Local trade side conversion is consistentTyped vault_id (U256), Address-based token metadata, and Float balances are coherent and match tests.
1553-1569: ERC20 mapping: required decimals and typed idRequiring decimals and mapping token.id as Bytes is aligned with stricter typing; conversions to/from SgErc20 look correct.
Based on learnings
Also applies to: 1574-1581
311-313: Local DB API signature & SQL placeholders correctly updated
fetch_vault_balance_changes now takesU256 vault_idandAddress token, and SQL placeholders (?vault_id,?token) are properly replaced (covered by existing tests).
1514-1521: ```bash
#!/bin/bash
sed -n '1280,1360p' crates/common/src/raindex_client/vaults.rs</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/cli/src/commands/local_db/sync/runner/mod.rs (1)
182-200: DRY up event sorting and hex parsing: reuse common crate helpers
Remove the localsort_events_by_block_and_logandparse_hex_fieldin crates/cli/src/commands/local_db/sync/runner/mod.rs (lines 182–200) and import them fromcommon::raindex_client::local_db.crates/common/src/raindex_client/trades.rs (1)
137-145: Remove needless clone on Bytes
order_hash.clone()is unnecessary; the value isn’t reused after the call. This will trip clippy with -D warnings.Apply this diff:
- order_hash.clone(), + order_hash,
♻️ Duplicate comments (4)
crates/cli/src/commands/local_db/sync/runner/mod.rs (1)
116-123: Good fix: log parsed store countLogging parsed_store_addresses.len() accurately reflects usable addresses. Matches prior feedback.
crates/cli/src/commands/local_db/fetch_events.rs (1)
15-19: Address-based API and Clap parsing LGTMShifting to Address in the trait, LocalDb impl, CLI args, and call site removes runtime parsing and matches the broader type refactor.
Also applies to: 27-34, 49-49, 67-69
crates/common/src/raindex_client/local_db/insert.rs (1)
76-81: Good: EventContext now owns Bytes tx hash and drops lifetimesThis matches the move to strong types and simplifies borrow management; Bytes clone is cheap. Solid change.
Also applies to: 83-90
crates/common/src/raindex_client/orders.rs (1)
246-249: Either return Bytes directly or silence clippy for unnecessary ResultThis always returns Ok and will trigger clippy::unnecessary_wraps with -D warnings.
Two options:
- Preferred: change signature to return Bytes and update call sites.
- Minimal: keep Result but add an allow.
Minimal patch:
#[wasm_export(skip)] +#[allow(clippy::unnecessary_wraps)] pub fn order_hash_bytes(&self) -> Result<Bytes, RaindexError> { Ok(self.order_hash.clone()) }If you prefer the signature change, I can generate a follow-up diff across call sites.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
crates/cli/src/commands/local_db/fetch_events.rs(12 hunks)crates/cli/src/commands/local_db/sync/runner/mod.rs(17 hunks)crates/common/src/raindex_client/local_db/insert.rs(32 hunks)crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs(5 hunks)crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rs(10 hunks)crates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs(6 hunks)crates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rs(3 hunks)crates/common/src/raindex_client/local_db/query/mod.rs(1 hunks)crates/common/src/raindex_client/orders.rs(14 hunks)crates/common/src/raindex_client/trades.rs(12 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/*/{src,tests,benches,examples}/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Rust code must pass clippy with all warnings denied (cargo clippy --workspace --all-targets --all-features -D warnings)
Files:
crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/insert.rscrates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/orders.rscrates/cli/src/commands/local_db/fetch_events.rscrates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rscrates/common/src/raindex_client/local_db/query/mod.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Rust code lives in the workspace under crates/* (e.g., cli, common, bindings, js_api, quote, subgraph, settings, math, integration_tests)
Files:
crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/insert.rscrates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/orders.rscrates/cli/src/commands/local_db/fetch_events.rscrates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rscrates/common/src/raindex_client/local_db/query/mod.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust with nix develop -c cargo fmt --all
Lint Rust with nix develop -c rainix-rs-static (preconfigured flags)
Use snake_case for Rust crates/modules and PascalCase for types
Files:
crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/insert.rscrates/common/src/raindex_client/trades.rscrates/common/src/raindex_client/orders.rscrates/cli/src/commands/local_db/fetch_events.rscrates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rscrates/common/src/raindex_client/local_db/query/mod.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
🧠 Learnings (17)
📓 Common learnings
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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.
📚 Learning: 2025-04-29T06:19:15.446Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/query/fetch_orders/mod.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/local_db/query/mod.rs
📚 Learning: 2025-10-13T12:57:05.900Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/local_db/query/fetch_orders/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs
📚 Learning: 2025-07-18T10:31:05.498Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
📚 Learning: 2025-07-21T16:37:20.599Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1911
File: crates/subgraph/src/utils/mod.rs:1-1
Timestamp: 2025-07-21T16:37:20.599Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb prefers explicit module imports over re-exporting symbols at higher levels. Specifically for the float constants in crates/subgraph/src/utils/float.rs, they prefer using `utils::float::*` rather than re-exporting with `pub use float::*` in the utils module, as the explicit import makes it clearer what is being imported and why it's needed.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs
📚 Learning: 2025-07-04T10:27:22.544Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/local_db/query/fetch_order_trades/mod.rs
📚 Learning: 2025-07-16T10:40:05.717Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
📚 Learning: 2025-07-04T09:02:57.301Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/query/fetch_order_trades/mod.rs
📚 Learning: 2025-06-18T12:56:44.290Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1938
File: crates/js_api/src/raindex/mod.rs:102-118
Timestamp: 2025-06-18T12:56:44.290Z
Learning: In the rainlanguage/rain.orderbook codebase, it's acceptable to scaffold unused enum variants in initial implementation PRs when they will be implemented in future PRs, as confirmed by findolor.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs
📚 Learning: 2025-10-06T14:13:18.531Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/trades.rs
📚 Learning: 2025-05-14T05:13:59.713Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1744
File: crates/subgraph/src/orderbook_client.rs:44-46
Timestamp: 2025-05-14T05:13:59.713Z
Learning: In the rain.orderbook project, WASM dependencies are intentionally made available in non-WASM targets to facilitate testing of WASM-related functionality, so conditional compilation guards like `#[cfg(target_family = "wasm")]` should not be added to imports or implementations that may be needed for tests.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
📚 Learning: 2025-04-30T09:28:36.960Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1715
File: crates/js_api/src/common/mod.rs:111-118
Timestamp: 2025-04-30T09:28:36.960Z
Learning: In the rain.orderbook repository, the WASM tests are already properly configured with conditional compilation using `#[cfg(target_family = "wasm")]` and `#[cfg(not(target_family = "wasm"))]`, and don't require additional `wasm_bindgen_test_configure!(run_in_browser)` directives.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
📚 Learning: 2025-06-24T13:30:02.968Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1950
File: crates/common/src/raindex_client/transactions.rs:34-38
Timestamp: 2025-06-24T13:30:02.968Z
Learning: When using #[wasm_bindgen] on an impl block, all methods within that block must have wasm_bindgen attributes, even if they are conditionally compiled for non-WASM targets using #[cfg(not(target_family = "wasm"))]. Removing these attributes causes compiler errors because the wasm_bindgen macro expansion processes the entire impl block and expects consistent attribute usage across all methods.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
📚 Learning: 2025-07-04T10:24:56.163Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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.rs
📚 Learning: 2025-10-14T07:51:55.148Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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.rs
📚 Learning: 2025-05-20T10:20:08.206Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1859
File: crates/quote/src/quote_debug.rs:472-492
Timestamp: 2025-05-20T10:20:08.206Z
Learning: In the Rain Orderbook codebase, the `#[tokio::test(flavor = "multi_thread")]` annotation is specifically needed for tests that use `LocalEvm`, not just for consistency across all async tests.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/mod.rs
📚 Learning: 2025-07-31T19:34:11.716Z
Learnt from: brusherru
PR: rainlanguage/rain.orderbook#2044
File: crates/common/src/raindex_client/vaults_list.rs:363-423
Timestamp: 2025-07-31T19:34:11.716Z
Learning: In the rainlanguage/rain.orderbook project, for WASM-exposed functionality like VaultsList, the team prefers to keep comprehensive tests in the non-WASM environment due to the complexity of recreating objects like RaindexVaults in WASM. WASM tests focus on basic functionality and error cases since the WASM code reuses the already-tested non-WASM implementation.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
🧬 Code graph analysis (10)
crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rs (3)
crates/common/src/raindex_client/orders.rs (9)
owner(92-94)owner(159-161)order_hash(88-90)order_hash(156-158)orders(574-592)order_bytes(84-86)order_bytes(153-155)meta(109-111)meta(171-173)crates/common/src/raindex_client/vaults.rs (2)
owner(98-100)owner(142-144)crates/common/src/raindex_client/trades.rs (2)
order_hash(42-44)order_hash(72-74)
crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs (2)
crates/common/src/raindex_client/local_db/decode.rs (1)
U256(243-243)crates/common/src/raindex_client/local_db/query/mod.rs (1)
create_success_callback(748-762)
crates/common/src/raindex_client/local_db/insert.rs (2)
crates/common/src/raindex_client/local_db/decode.rs (2)
U256(243-243)event(158-166)crates/common/src/raindex_client/local_db/tokens.rs (1)
build_event(73-82)
crates/common/src/raindex_client/trades.rs (4)
crates/common/src/raindex_client/orders.rs (9)
order_hash(88-90)order_hash(156-158)chain_id(76-78)chain_id(147-149)transaction(117-119)transaction(177-179)order_hash_bytes(247-249)order(835-846)order(849-860)crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs (2)
LocalDbQuery(75-75)fetch_order_trades(53-76)crates/common/src/raindex_client/local_db/query/fetch_order_trades_count/mod.rs (2)
LocalDbQuery(33-33)fetch_order_trades_count(13-37)crates/common/src/raindex_client/vaults.rs (7)
chain_id(86-88)chain_id(133-135)chain_id(227-229)chain_id(254-256)try_from_local_trade_side(822-867)transaction(660-662)transaction(700-702)
crates/common/src/raindex_client/orders.rs (3)
crates/common/src/raindex_client/trades.rs (4)
order_hash(42-44)order_hash(72-74)id(38-40)id(69-71)crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rs (3)
default(25-32)LocalDbQuery(141-141)fetch_orders(79-142)crates/common/src/raindex_client/mod.rs (3)
new(111-132)new_test_client_with_db_callback(534-541)get_local_db_test_yaml(544-588)
crates/cli/src/commands/local_db/fetch_events.rs (2)
crates/common/src/raindex_client/vaults.rs (3)
address(235-237)address(260-262)new(176-181)crates/common/src/raindex_client/local_db/sync.rs (6)
new(84-86)new(122-124)new(1150-1154)new(1178-1192)new(1262-1276)new(1368-1379)
crates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rs (1)
crates/common/src/raindex_client/local_db/query/mod.rs (7)
execute_query_json(68-78)serde_json(144-144)serde_json(206-206)serde_json(290-290)serde_json(349-349)serde_json(503-503)create_success_callback(748-762)
crates/common/src/raindex_client/local_db/query/mod.rs (3)
crates/common/src/rpc_client.rs (5)
serialize(88-93)serialize(108-116)deserialize(95-101)deserialize(118-125)from(313-315)crates/common/src/raindex_client/local_db/insert.rs (3)
value(41-41)addr(1282-1284)bytes(1286-1288)crates/common/src/raindex_client/local_db/decode.rs (1)
U256(243-243)
crates/cli/src/commands/local_db/sync/runner/mod.rs (5)
crates/cli/src/commands/local_db/sync/runner/window.rs (1)
params(199-207)crates/common/src/raindex_client/local_db/insert.rs (3)
value(41-41)raw_events(182-201)addr(1282-1284)crates/common/src/raindex_client/local_db/fetch.rs (1)
sort_events_by_block_and_log(340-348)crates/common/src/raindex_client/local_db/sync.rs (2)
sort_events_by_block_and_log(386-392)parse_hex_field(394-404)crates/common/src/raindex_client/local_db/mod.rs (1)
from(161-166)
crates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs (2)
crates/common/src/raindex_client/local_db/decode.rs (1)
U256(243-243)crates/common/src/raindex_client/vaults.rs (6)
vault(1439-1443)vault(1444-1448)balance(106-108)balance(148-150)balance(187-189)balance(197-199)
⏰ 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). (17)
- GitHub Check: test
- GitHub Check: git-clean
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: test
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: test
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: Deploy-Preview-Push
- GitHub Check: Deploy-Docs-Preview
🔇 Additional comments (13)
crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rs (4)
4-4: LGTM! Type safety achieved for FetchOrdersArgs.The import of
Bytesand the migration ofFetchOrdersArgsfields from string-based to strongly-typed primitives (Vec<Address>,Option<Bytes>) aligns with the PR objective. TheFrom<GetOrdersFilters>implementation correctly passes through the typed values.Also applies to: 19-21, 45-47
54-76: LGTM! LocalDbOrder properly typed with serde helpers.The struct now uses
AddressandBytesprimitives with appropriate serde helpers (serde_address,serde_bytes,serde_option_bytes). This achieves the PR goal of proper type support for local DB logic.Based on learnings.
96-99: LGTM! Efficient lowercase hex formatting for addresses.Using
format!("'{owner:#x}'")andformat!("'{token:#x}'")directly produces lowercase hex with 0x prefix, avoiding the intermediateto_string().to_lowercase()allocation mentioned in previous reviews.Also applies to: 122-125
179-193: LGTM! Tests properly updated for typed values.All test cases correctly use
Address::from_str()andBytes::from_str()to create typed test data, and assertions verify that the generated SQL contains the expected lowercase hex strings with 0x prefixes.Also applies to: 220-223, 229-274, 428-430, 453-453, 479-481
crates/cli/src/commands/local_db/sync/runner/mod.rs (2)
32-35: Address plumbing LGTMUsing Address in SyncParams, run, and fetch_events removes runtime parsing and aligns with the typed API.
Also applies to: 59-59, 82-93
136-143: Correctly merging and re-sorting after store eventsExtending raw_events, decoding, re-sorting, and updating decoded_count is correct and preserves order.
crates/common/src/raindex_client/trades.rs (4)
244-256: Local DB fast-path behavior is correctCounting via LocalDB with typed order_hash looks good; matches the intended all-trades fast-path and type changes.
Based on learnings
304-360: Type-safe mapping from LocalDbOrderTrade → RaindexTrade LGTMAll fields use strongly-typed Address/Bytes/U256/Float as expected; timestamp uses U256::from(u64), and vault balance changes are correctly constructed.
381-438: WASM test fixtures migrated to typed primitives correctlyGood construction of Address/Bytes/U256/Float and trade_id. Fixtures and LocalDb rows align with new types.
Also applies to: 447-517, 521-523
654-661: Assertions updated to typed getters look solidParsing back to Bytes/Address for comparisons is consistent and precise across wasm and non-wasm tests.
Also applies to: 665-668, 703-706, 716-720, 1128-1134, 1132-1132, 1192-1192
crates/common/src/raindex_client/orders.rs (3)
709-723: Subgraph call with SgBytes(order_hash.to_string()) looks correctTyped order_hash is serialized once for the subgraph client; no extra parsing.
733-744: Local DB lookup typed arguments look correctPassing Bytes for order_hash and comparing Address directly avoids string round-trips.
978-988: Tests updated for typed Address/Bytes/U256 are consistentGood end-to-end checks across wasm and non-wasm paths; assertions match the new API.
Also applies to: 1011-1011, 1059-1094, 1096-1118, 1122-1131, 1144-1155, 1171-1173, 1181-1182, 1195-1219, 1261-1263, 1277-1280, 1284-1295, 1784-1784, 1811-1811
| let store_list: Vec<String> = store_addresses.into_iter().collect(); | ||
| println!( | ||
| "Fetching interpreter store Set events for {} store(s)", | ||
| store_list.len() | ||
| ); | ||
| let store_events = self | ||
| .data_source | ||
| .fetch_store_set_events(&store_list, window.start_block, window.target_block) | ||
| .await?; | ||
| println!( | ||
| "Fetched {} interpreter store Set events", | ||
| store_events.len() | ||
| ); | ||
|
|
||
| if !store_events.is_empty() { | ||
| raw_events.extend(store_events.iter().cloned()); | ||
| let mut decoded_store = self.data_source.decode_events(&store_events)?; | ||
| decoded_events.append(&mut decoded_store); | ||
| sort_events_by_block_and_log(&mut decoded_events); | ||
| decoded_count = decoded_events.len(); | ||
| println!("Decoded {} total events", decoded_count); | ||
| let parsed_store_addresses: Vec<Address> = store_list | ||
| .iter() | ||
| .filter_map(|value| Address::from_str(value).ok()) | ||
| .collect(); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid intermediate allocation when parsing store addresses
Parse directly from the BTreeSet to Vec
; no need for store_list.- let store_list: Vec<String> = store_addresses.into_iter().collect();
- let parsed_store_addresses: Vec<Address> = store_list
- .iter()
- .filter_map(|value| Address::from_str(value).ok())
- .collect();
+ let parsed_store_addresses: Vec<Address> = store_addresses
+ .iter()
+ .filter_map(|value| Address::from_str(value).ok())
+ .collect();📝 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.
| let store_list: Vec<String> = store_addresses.into_iter().collect(); | |
| println!( | |
| "Fetching interpreter store Set events for {} store(s)", | |
| store_list.len() | |
| ); | |
| let store_events = self | |
| .data_source | |
| .fetch_store_set_events(&store_list, window.start_block, window.target_block) | |
| .await?; | |
| println!( | |
| "Fetched {} interpreter store Set events", | |
| store_events.len() | |
| ); | |
| if !store_events.is_empty() { | |
| raw_events.extend(store_events.iter().cloned()); | |
| let mut decoded_store = self.data_source.decode_events(&store_events)?; | |
| decoded_events.append(&mut decoded_store); | |
| sort_events_by_block_and_log(&mut decoded_events); | |
| decoded_count = decoded_events.len(); | |
| println!("Decoded {} total events", decoded_count); | |
| let parsed_store_addresses: Vec<Address> = store_list | |
| .iter() | |
| .filter_map(|value| Address::from_str(value).ok()) | |
| .collect(); | |
| let parsed_store_addresses: Vec<Address> = store_addresses | |
| .iter() | |
| .filter_map(|value| Address::from_str(value).ok()) | |
| .collect(); |
🤖 Prompt for AI Agents
In crates/cli/src/commands/local_db/sync/runner/mod.rs around lines 110 to 115,
the code creates an intermediate Vec<String> (store_list) before parsing
addresses; replace this with a single pass that iterates the original BTreeSet
(store_addresses), parses each &str -> Address with
filter_map(Address::from_str(...).ok()) and collects directly into a
Vec<Address>, removing the temporary allocation and simplifying the logic.
| let transaction_hash = hex::encode_prefixed(event.transaction_hash.as_ref()); | ||
| eprintln!( | ||
| "Warning: Unknown event type for transaction {}: {}", | ||
| event.transaction_hash, decoded.note | ||
| transaction_hash, decoded.note | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Enrich unknown-event warning with block/log index
Helps triage and cross-reference raw_events quickly.
- eprintln!(
- "Warning: Unknown event type for transaction {}: {}",
- transaction_hash, decoded.note
- );
+ eprintln!(
+ "Warning: Unknown event at block {} log {} (tx {}): {}",
+ event.block_number, event.log_index, transaction_hash, decoded.note
+ );📝 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.
| let transaction_hash = hex::encode_prefixed(event.transaction_hash.as_ref()); | |
| eprintln!( | |
| "Warning: Unknown event type for transaction {}: {}", | |
| event.transaction_hash, decoded.note | |
| transaction_hash, decoded.note | |
| ); | |
| let transaction_hash = hex::encode_prefixed(event.transaction_hash.as_ref()); | |
| eprintln!( | |
| "Warning: Unknown event at block {} log {} (tx {}): {}", | |
| event.block_number, event.log_index, transaction_hash, decoded.note | |
| ); |
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/local_db/insert.rs around lines 153 to 157,
the warning for an unknown event type only prints the transaction hash and note;
update the eprintln! to also include the block index and log index (use the
available fields on the event or decoded struct, e.g., event.block_number or
event.block_index and event.log_index or decoded.block_index/log_index),
formatting them alongside the transaction hash and note so the message reads
like: "Warning: Unknown event type for transaction <tx> (block <block_index>,
log <log_index>): <note>". Ensure you reference the correct field names and
handle Option values (print "unknown" or similar if absent).
| escape_sql_text(&hex::encode_prefixed(row.event.transaction_hash.as_ref())), | ||
| row.log_index, | ||
| escape_sql_text(&row.event.address), | ||
| escape_sql_text(&format!("{:#x}", row.event.address)), | ||
| escape_sql_text(&row.topics_json), | ||
| escape_sql_text(&row.event.data), | ||
| escape_sql_text(&hex::encode_prefixed(row.event.data.as_ref())), | ||
| escape_sql_text(&row.raw_json), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Normalize address hex formatting in raw_events_to_sql
Elsewhere you use hex::encode_prefixed for hex strings. Use the same here for Address to keep consistent casing/zero‑padding and avoid any formatter behavior drift.
Apply:
- escape_sql_text(&format!("{:#x}", row.event.address)),
+ escape_sql_text(&hex::encode_prefixed(row.event.address)),📝 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.
| escape_sql_text(&hex::encode_prefixed(row.event.transaction_hash.as_ref())), | |
| row.log_index, | |
| escape_sql_text(&row.event.address), | |
| escape_sql_text(&format!("{:#x}", row.event.address)), | |
| escape_sql_text(&row.topics_json), | |
| escape_sql_text(&row.event.data), | |
| escape_sql_text(&hex::encode_prefixed(row.event.data.as_ref())), | |
| escape_sql_text(&row.raw_json), | |
| escape_sql_text(&hex::encode_prefixed(row.event.transaction_hash.as_ref())), | |
| row.log_index, | |
| escape_sql_text(&hex::encode_prefixed(row.event.address)), | |
| escape_sql_text(&row.topics_json), | |
| escape_sql_text(&hex::encode_prefixed(row.event.data.as_ref())), | |
| escape_sql_text(&row.raw_json), |
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/local_db/insert.rs around lines 228 to 233,
the address is formatted with format!("{:#x}", row.event.address) which can
produce inconsistent casing/zero-padding; replace that formatting with
hex::encode_prefixed on the address bytes (e.g.
hex::encode_prefixed(row.event.address.as_ref())) and pass the result into
escape_sql_text just like the other hex fields so the address uses the same
canonical hex representation as elsewhere.
| let block_number = context.block_number; | ||
| let block_timestamp = context.block_timestamp; | ||
| let transaction_hash = context.transaction_hash; | ||
| let transaction_hash = hex::encode_prefixed(context.transaction_hash.as_ref()); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
DRY the transaction hash hex rendering
You repeatedly derive the same hex string. Add a small helper on EventContext and call it at use sites.
Add:
impl EventContext {
fn tx_hex(&self) -> String {
alloy::hex::encode_prefixed(self.transaction_hash.as_ref())
}
}Example change (repeat at other sites):
- let transaction_hash = hex::encode_prefixed(context.transaction_hash.as_ref());
+ let transaction_hash = context.tx_hex();Also applies to: 351-351, 396-396, 446-446, 498-498, 621-621, 689-689, 726-726, 760-760, 801-801, 1047-1047
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/local_db/insert.rs around lines 311, 351,
396, 446, 498, 621, 689, 726, 760, 801, and 1047, the same hex rendering of
context.transaction_hash is repeated; add a small helper on EventContext (fn
tx_hex(&self) -> String {
alloy::hex::encode_prefixed(self.transaction_hash.as_ref()) }) and replace each
repeated hex::encode_prefixed(...) call with context.tx_hex() (or
&context.tx_hex() where appropriate) so all sites reuse the single helper
implementation.
| #[serde(with = "serde_b256")] | ||
| pub output_vault_id: U256, | ||
| #[serde(with = "serde_address")] | ||
| pub output_token: Address, | ||
| pub output_token_name: Option<String>, | ||
| #[serde(alias = "output_token_symbol")] | ||
| pub output_token_symbol: Option<String>, |
There was a problem hiding this comment.
Vault ID hex serialization mismatch
serde_b256 serializes U256 as a 32-byte, 0x-prefixed hex string (via encode_prefixed(B256::from(*value))). That format only covers 32 bytes; it silently truncates/zero-pads U256 values > 2²⁵⁶−1 (which can’t exist) but—more importantly—the DB is storing vault IDs as plain decimal strings today. Switching to hex without migrating stored rows or updating SQL queries that compare against decimal literals will break fetches. Confirm DB contents and align serialization before landing.
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs
around lines 37–42, the field output_vault_id is currently annotated with
serde_b256 which serializes U256 as a 0x-prefixed 32-byte hex string, but the DB
stores vault IDs as plain decimal strings; to fix, first confirm the DB format
(decimal strings) and then either change the serde annotation to
serialize/deserialize U256 as decimal strings (implement or use a serde helper
that calls to_string()/from_str() for U256) so JSON/DB comparisons keep working,
or, if you intend to switch to hex, add a coordinated migration and update any
SQL queries that compare vault IDs to use hex literals; apply the chosen
approach consistently across serialization and DB access code.
| let filter_order_hash = order_hash | ||
| .and_then(|hash| { | ||
| let trimmed = hash.trim(); | ||
| if trimmed.is_empty() { | ||
| None | ||
| } else { | ||
| Some(format!( | ||
| "\nAND lower(COALESCE(la.order_hash, l.order_hash)) = lower('{}')\n", | ||
| sanitize_literal(trimmed) | ||
| )) | ||
| if hash.is_empty() { | ||
| return None; | ||
| } | ||
|
|
||
| Some(format!( | ||
| "\nAND lower(COALESCE(la.order_hash, l.order_hash)) = lower('{}')\n", | ||
| hash | ||
| )) | ||
| }) | ||
| .unwrap_or_default(); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider consistent hex formatting for order_hash.
The order_hash formatting uses format!("...'{}'", hash)(Display trait), while owners and tokens use{:#x}` (explicit lowercase hex). Both produce correct output, but for consistency consider:
- Some(format!(
- "\nAND lower(COALESCE(la.order_hash, l.order_hash)) = lower('{}')\n",
- hash
- ))
+ Some(format!(
+ "\nAND lower(COALESCE(la.order_hash, l.order_hash)) = lower('{hash:#x}')\n"
+ ))Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rs around lines
109-120: the order_hash is interpolated with format!("'{}'", hash) while other
hex values use {:#x}; normalize and emit order_hash in the same lowercase
0x-prefixed hex style as owners/tokens—either parse the incoming hash (trim any
"0x"), lowercase it and prepend "0x" before embedding, or convert to the same
numeric/byte representation used elsewhere and format with {:#x} so the SQL
fragment uses a consistent lowercase 0x-prefixed hex representation.
| let mut id = order.orderbook_address.to_vec(); | ||
| id.extend_from_slice(order.order_hash.as_ref()); | ||
|
|
||
| Ok(Self { | ||
| raindex_client: Rc::clone(&raindex_client), | ||
| chain_id, | ||
| id: Bytes::from(keccak256(&id).as_slice().to_vec()), | ||
| order_bytes: Bytes::from_str(&order.order_bytes)?, | ||
| order_hash: Bytes::from_str(&order.order_hash)?, | ||
| owner: Address::from_str(&order.owner)?, | ||
| order_bytes: order.order_bytes.clone(), | ||
| order_hash: order.order_hash.clone(), | ||
| owner: order.owner, | ||
| inputs: inputs |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid extra allocation when deriving id from keccak
Use copy_from_slice instead of allocating a Vec.
- id: Bytes::from(keccak256(&id).as_slice().to_vec()),
+ id: Bytes::copy_from_slice(keccak256(&id).as_slice()),📝 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.
| let mut id = order.orderbook_address.to_vec(); | |
| id.extend_from_slice(order.order_hash.as_ref()); | |
| Ok(Self { | |
| raindex_client: Rc::clone(&raindex_client), | |
| chain_id, | |
| id: Bytes::from(keccak256(&id).as_slice().to_vec()), | |
| order_bytes: Bytes::from_str(&order.order_bytes)?, | |
| order_hash: Bytes::from_str(&order.order_hash)?, | |
| owner: Address::from_str(&order.owner)?, | |
| order_bytes: order.order_bytes.clone(), | |
| order_hash: order.order_hash.clone(), | |
| owner: order.owner, | |
| inputs: inputs | |
| let mut id = order.orderbook_address.to_vec(); | |
| id.extend_from_slice(order.order_hash.as_ref()); | |
| Ok(Self { | |
| raindex_client: Rc::clone(&raindex_client), | |
| chain_id, | |
| id: Bytes::copy_from_slice(keccak256(&id).as_slice()), | |
| order_bytes: order.order_bytes.clone(), | |
| order_hash: order.order_hash.clone(), | |
| owner: order.owner, | |
| inputs: inputs |
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/orders.rs around lines 922 to 932, the code
currently calls keccak256(&id).as_slice().to_vec() which creates an unnecessary
intermediate Vec; instead, allocate a 32-byte fixed array, call copy_from_slice
to copy the keccak output into that array, and then convert that array directly
into the Bytes value (avoiding the intermediate to_vec allocation).
| active: order.active, | ||
| timestamp_added: U256::from_str(&order.block_timestamp.to_string())?, | ||
| meta: order.meta.map(|meta| Bytes::from_str(&meta)).transpose()?, | ||
| meta: order.meta.clone(), | ||
| rainlang, | ||
| transaction: None, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid string round-trip when building U256 from u64
Use From to prevent allocation/parsing and silence clippy.
- timestamp_added: U256::from_str(&order.block_timestamp.to_string())?,
+ timestamp_added: U256::from(order.block_timestamp),📝 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.
| active: order.active, | |
| timestamp_added: U256::from_str(&order.block_timestamp.to_string())?, | |
| meta: order.meta.map(|meta| Bytes::from_str(&meta)).transpose()?, | |
| meta: order.meta.clone(), | |
| rainlang, | |
| transaction: None, | |
| active: order.active, | |
| timestamp_added: U256::from(order.block_timestamp), | |
| meta: order.meta.clone(), | |
| rainlang, | |
| transaction: None, |
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/orders.rs around lines 955 to 959, avoid
converting block_timestamp to a string and parsing back into U256; instead
construct U256 directly from the integer (e.g.,
U256::from(order.block_timestamp) or U256::from(order.block_timestamp as u64)
depending on the field type) and remove the fallible parsing (the trailing ?),
which eliminates allocation/parsing and satisfies Clippy.
| .expect("fetch events"); | ||
| let result = fetch_events( | ||
| &data_source, | ||
| Address::from_str("0x0000000000000000000000000000000000000abc").unwrap(), |
There was a problem hiding this comment.
nitpick: consider using address! macro
| Address::from_str("0x0000000000000000000000000000000000000abc").unwrap(), | |
| address!("0x0000000000000000000000000000000000000abc"), |
| transaction_hash: "0x0".into(), | ||
| block_number: 0, | ||
| block_timestamp: 0, | ||
| transaction_hash: Bytes::from(vec![0u8; 32]), |
There was a problem hiding this comment.
consider B256 (alias of FixedBytes<32>) instead of dynamic size Bytes. with fixed bytes you also can use fixed_bytes! macro
| pub struct Erc20TokenRow { | ||
| pub chain_id: u32, | ||
| pub address: String, | ||
| #[serde(with = "serde_address")] |
| let orderbook_address = | ||
| Address::from_str("0x2f209e5b67a33b8fe96e28f24628df6da301c8eb").unwrap(); | ||
| let order_owner = | ||
| Address::from_str("0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").unwrap(); | ||
| let transaction_sender = | ||
| Address::from_str("0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb").unwrap(); |
There was a problem hiding this comment.
nitpick: consider address!
| "0x0000000000000000000000000000000000000000000000000000000000000001", | ||
| ) | ||
| .unwrap(); | ||
| let input_running_balance = Float::from_hex( |
There was a problem hiding this comment.
not something to do in this PR of course, but I wish we had a float! macro or something. maybe we should create an issue for that in rain.math.float
| token: Address::from_str("0x00000000000000000000000000000000000000aa").unwrap(), | ||
| owner: Address::from_str("0x0000000000000000000000000000000000001111").unwrap(), | ||
| orderbook_address: Address::from_str("0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB") |
There was a problem hiding this comment.
nitpick: consider address!
There was a problem hiding this comment.
I thought alloy already provides ser/de for Address'es and bytes
| match topic.as_ref() { | ||
| bytes if bytes == AddOrderV3::SIGNATURE_HASH.as_slice() => Self::AddOrderV3, | ||
| bytes if bytes == TakeOrderV3::SIGNATURE_HASH.as_slice() => Self::TakeOrderV3, | ||
| bytes if bytes == WithdrawV2::SIGNATURE_HASH.as_slice() => Self::WithdrawV2, | ||
| bytes if bytes == DepositV2::SIGNATURE_HASH.as_slice() => Self::DepositV2, | ||
| bytes if bytes == RemoveOrderV3::SIGNATURE_HASH.as_slice() => Self::RemoveOrderV3, | ||
| bytes if bytes == ClearV3::SIGNATURE_HASH.as_slice() => Self::ClearV3, | ||
| bytes if bytes == AfterClearV2::SIGNATURE_HASH.as_slice() => Self::AfterClearV2, | ||
| bytes if bytes == MetaV1_2::SIGNATURE_HASH.as_slice() => Self::MetaV1_2, | ||
| bytes if bytes == Set::SIGNATURE_HASH.as_slice() => Self::InterpreterStoreSet, | ||
| _ => Self::Unknown, |
There was a problem hiding this comment.
this is so much cleaner. nice refactor
| "0x0000000000000000000000000000000000000000000000000000000000000001"; | ||
|
|
||
| let trade_id = format!( | ||
| let orderbook_address = Address::from_str(ORDERBOOK_ADDRESS).unwrap(); |
There was a problem hiding this comment.
if you use address! macro then you can declare the constant as Address and not need to parse strings at all. same could apply for hashes if they used FixedBytes<32> and fixed_bytes! macro (I don't think alloy has a macro for Bytes if I remember correctly)
| vault_id, | ||
| token, | ||
| owner, | ||
| orderbook_address: Address::from_str("0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB") |
There was a problem hiding this comment.
more address! nitpicks in this file but you get the idea
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/common/src/raindex_client/local_db/tokens.rs (1)
28-52: Use Address type for store addresses
Refactorcollect_store_addressesto returnBTreeSet<Address>(notString) and drop theformat!calls.
Update downstream call sites accordingly:
- Tests in
crates/common/src/raindex_client/local_db/tokens.rs(lines 243–248)crates/common/src/raindex_client/local_db/sync.rs(lines 409–413)crates/cli/src/commands/local_db/sync/runner/mod.rs(lines 105–108)crates/common/src/raindex_client/local_db/sync.rs (2)
394-404: Silent error handling masks invalid log indices.Using
unwrap_or(0)converts parsing failures to0, which is a valid log index. This makes it impossible to distinguish between an actual index of0and corrupted data, potentially causing incorrect sort order.Consider propagating the error or at minimum logging the failure:
fn parse_hex_field(value: &str) -> u64 { let trimmed = value.trim(); if let Some(hex) = trimmed .strip_prefix("0x") .or_else(|| trimmed.strip_prefix("0X")) { - u64::from_str_radix(hex, 16).unwrap_or(0) + u64::from_str_radix(hex, 16).unwrap_or_else(|e| { + eprintln!("Warning: Failed to parse hex log_index '{}': {}", value, e); + 0 + }) } else { - trimmed.parse::<u64>().unwrap_or(0) + trimmed.parse::<u64>().unwrap_or_else(|e| { + eprintln!("Warning: Failed to parse decimal log_index '{}': {}", value, e); + 0 + }) } }Better yet, make this function return
Result<u64, ParseError>and handle errors at the call site.
406-422: Consider using BTreeSet<Address> to avoid string conversions.The function converts
Addressto string and back, which adds unnecessary overhead. SinceAddressimplementsOrd, you can useBTreeSet<Address>directly.-fn collect_all_store_addresses( +fn collect_store_addresses_typed( decoded_events: &[DecodedEventData<DecodedEvent>], existing_stores: &[StoreAddressRow], -) -> Vec<String> { - let mut store_addresses: BTreeSet<String> = collect_store_addresses(decoded_events) - .into_iter() - .collect(); +) -> Vec<Address> { + let mut store_addresses: BTreeSet<Address> = decoded_events + .iter() + .filter_map(|event| match &event.decoded_data { + DecodedEvent::AddOrderV3(order) => Some(order.order.evaluable.store), + _ => None, + }) + .collect(); for row in existing_stores { if row.store_address != Address::ZERO { - let formatted = format!("{:#x}", row.store_address); - store_addresses.insert(formatted.to_ascii_lowercase()); + store_addresses.insert(row.store_address); } } store_addresses.into_iter().collect() }This eliminates the string conversion overhead and makes the type flow more consistent. If the return type must remain
Vec<String>for compatibility, at least document why.Additionally, document why
Address::ZEROis filtered out at line 415.
♻️ Duplicate comments (4)
crates/common/src/raindex_client/local_db/insert.rs (3)
153-158: Improve unknown-event log contextInclude block and log index for faster triage.
Apply:
- eprintln!( - "Warning: Unknown event type for transaction {}: {}", - transaction_hash, decoded.note - ); + eprintln!( + "Warning: Unknown event at block {} log {} (tx {}): {}", + event.block_number, + hex_to_decimal(&event.log_index).unwrap_or_default(), + transaction_hash, + decoded.note + );
229-234: Normalize address hex formatting in raw_events_to_sqlUse the same hex encoder as elsewhere for consistency.
- escape_sql_text(&hex::encode_prefixed(row.event.transaction_hash)), + escape_sql_text(&hex::encode_prefixed(row.event.transaction_hash)), row.log_index, - escape_sql_text(&format!("{:#x}", row.event.address)), + escape_sql_text(&hex::encode_prefixed(row.event.address)), escape_sql_text(&row.topics_json), - escape_sql_text(&hex::encode_prefixed(row.event.data.as_ref())), + escape_sql_text(&hex::encode_prefixed(row.event.data.as_ref())),
77-91: DRY transaction-hash hex formatting via EventContext helperAvoid repeating hex::encode_prefixed(context.transaction_hash).
struct EventContext { block_number: u64, block_timestamp: u64, transaction_hash: B256, log_index: u64, } +impl EventContext { + fn tx_hex(&self) -> String { + alloy::hex::encode_prefixed(self.transaction_hash) + } +} @@ - let transaction_hash = hex::encode_prefixed(context.transaction_hash); + let transaction_hash = context.tx_hex(); @@ - let transaction_hash = hex::encode_prefixed(context.transaction_hash); + let transaction_hash = context.tx_hex(); @@ - let transaction_hash = hex::encode_prefixed(context.transaction_hash); + let transaction_hash = context.tx_hex(); @@ - let transaction_hash = hex::encode_prefixed(context.transaction_hash); + let transaction_hash = context.tx_hex(); @@ - let transaction_hash = hex::encode_prefixed(context.transaction_hash); + let transaction_hash = context.tx_hex(); @@ - let transaction_hash = hex::encode_prefixed(context.transaction_hash); + let transaction_hash = context.tx_hex(); @@ - let transaction_hash = hex::encode_prefixed(context.transaction_hash); + let transaction_hash = context.tx_hex(); @@ - let transaction_hash = hex::encode_prefixed(context.transaction_hash); + let transaction_hash = context.tx_hex(); @@ - let transaction_hash = hex::encode_prefixed(context.transaction_hash); + let transaction_hash = context.tx_hex();Also applies to: 310-318, 350-356, 396-403, 446-454, 498-505, 621-629, 689-697, 726-733, 760-768, 801-809, 1041-1048
crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rs (1)
107-118: Consider consistent hex formatting for order_hash.While the current implementation works correctly (Display trait produces "0x..." format), for consistency with owner/token formatting which uses
{:#x}, consider normalizing the order_hash format as well. This would make the SQL generation more uniform.Apply this diff if you prefer consistent formatting:
- Some(format!( - "\nAND lower(COALESCE(la.order_hash, l.order_hash)) = lower('{}')\n", - hash - )) + Some(format!( + "\nAND lower(COALESCE(la.order_hash, l.order_hash)) = lower('0x{hash:#x}')\n" + ))Note: This suggestion aligns with a previous review comment that was marked as addressed but may not have been fully implemented.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (22)
crates/cli/src/commands/local_db/decode_events.rs(2 hunks)crates/cli/src/commands/local_db/decoded_events_to_sql.rs(3 hunks)crates/cli/src/commands/local_db/fetch_events.rs(12 hunks)crates/cli/src/commands/local_db/sync/runner/apply.rs(8 hunks)crates/cli/src/commands/local_db/sync/runner/mod.rs(17 hunks)crates/cli/src/commands/local_db/sync/token.rs(2 hunks)crates/cli/src/commands/local_db/tokens_fetch.rs(2 hunks)crates/common/src/raindex_client/local_db/decode.rs(24 hunks)crates/common/src/raindex_client/local_db/fetch.rs(17 hunks)crates/common/src/raindex_client/local_db/insert.rs(31 hunks)crates/common/src/raindex_client/local_db/mod.rs(4 hunks)crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rs(4 hunks)crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs(5 hunks)crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rs(10 hunks)crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs(2 hunks)crates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs(6 hunks)crates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rs(3 hunks)crates/common/src/raindex_client/local_db/query/mod.rs(1 hunks)crates/common/src/raindex_client/local_db/sync.rs(31 hunks)crates/common/src/raindex_client/local_db/tokens.rs(2 hunks)crates/common/src/raindex_client/vaults.rs(43 hunks)crates/common/src/rpc_client.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/*/{src,tests,benches,examples}/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Rust code must pass clippy with all warnings denied (cargo clippy --workspace --all-targets --all-features -D warnings)
Files:
crates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rscrates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/cli/src/commands/local_db/tokens_fetch.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/cli/src/commands/local_db/fetch_events.rscrates/common/src/raindex_client/local_db/fetch.rscrates/cli/src/commands/local_db/decoded_events_to_sql.rscrates/common/src/raindex_client/local_db/query/fetch_orders/mod.rscrates/common/src/raindex_client/local_db/insert.rscrates/common/src/raindex_client/local_db/query/mod.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rscrates/common/src/rpc_client.rscrates/common/src/raindex_client/local_db/mod.rscrates/cli/src/commands/local_db/decode_events.rscrates/common/src/raindex_client/local_db/sync.rscrates/common/src/raindex_client/local_db/tokens.rscrates/common/src/raindex_client/local_db/decode.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rscrates/cli/src/commands/local_db/sync/token.rscrates/common/src/raindex_client/vaults.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Rust code lives in the workspace under crates/* (e.g., cli, common, bindings, js_api, quote, subgraph, settings, math, integration_tests)
Files:
crates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rscrates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/cli/src/commands/local_db/tokens_fetch.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/cli/src/commands/local_db/fetch_events.rscrates/common/src/raindex_client/local_db/fetch.rscrates/cli/src/commands/local_db/decoded_events_to_sql.rscrates/common/src/raindex_client/local_db/query/fetch_orders/mod.rscrates/common/src/raindex_client/local_db/insert.rscrates/common/src/raindex_client/local_db/query/mod.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rscrates/common/src/rpc_client.rscrates/common/src/raindex_client/local_db/mod.rscrates/cli/src/commands/local_db/decode_events.rscrates/common/src/raindex_client/local_db/sync.rscrates/common/src/raindex_client/local_db/tokens.rscrates/common/src/raindex_client/local_db/decode.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rscrates/cli/src/commands/local_db/sync/token.rscrates/common/src/raindex_client/vaults.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust with nix develop -c cargo fmt --all
Lint Rust with nix develop -c rainix-rs-static (preconfigured flags)
Use snake_case for Rust crates/modules and PascalCase for types
Files:
crates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rscrates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/cli/src/commands/local_db/tokens_fetch.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/cli/src/commands/local_db/fetch_events.rscrates/common/src/raindex_client/local_db/fetch.rscrates/cli/src/commands/local_db/decoded_events_to_sql.rscrates/common/src/raindex_client/local_db/query/fetch_orders/mod.rscrates/common/src/raindex_client/local_db/insert.rscrates/common/src/raindex_client/local_db/query/mod.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rscrates/common/src/rpc_client.rscrates/common/src/raindex_client/local_db/mod.rscrates/cli/src/commands/local_db/decode_events.rscrates/common/src/raindex_client/local_db/sync.rscrates/common/src/raindex_client/local_db/tokens.rscrates/common/src/raindex_client/local_db/decode.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rscrates/cli/src/commands/local_db/sync/token.rscrates/common/src/raindex_client/vaults.rs
🧠 Learnings (17)
📚 Learning: 2025-05-14T05:13:59.713Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1744
File: crates/subgraph/src/orderbook_client.rs:44-46
Timestamp: 2025-05-14T05:13:59.713Z
Learning: In the rain.orderbook project, WASM dependencies are intentionally made available in non-WASM targets to facilitate testing of WASM-related functionality, so conditional compilation guards like `#[cfg(target_family = "wasm")]` should not be added to imports or implementations that may be needed for tests.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rscrates/common/src/raindex_client/vaults.rs
📚 Learning: 2025-04-30T09:28:36.960Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1715
File: crates/js_api/src/common/mod.rs:111-118
Timestamp: 2025-04-30T09:28:36.960Z
Learning: In the rain.orderbook repository, the WASM tests are already properly configured with conditional compilation using `#[cfg(target_family = "wasm")]` and `#[cfg(not(target_family = "wasm"))]`, and don't require additional `wasm_bindgen_test_configure!(run_in_browser)` directives.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rscrates/common/src/raindex_client/vaults.rs
📚 Learning: 2025-07-18T10:31:05.498Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/cli/src/commands/local_db/decoded_events_to_sql.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rscrates/common/src/raindex_client/vaults.rs
📚 Learning: 2025-06-24T13:30:02.968Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1950
File: crates/common/src/raindex_client/transactions.rs:34-38
Timestamp: 2025-06-24T13:30:02.968Z
Learning: When using #[wasm_bindgen] on an impl block, all methods within that block must have wasm_bindgen attributes, even if they are conditionally compiled for non-WASM targets using #[cfg(not(target_family = "wasm"))]. Removing these attributes causes compiler errors because the wasm_bindgen macro expansion processes the entire impl block and expects consistent attribute usage across all methods.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
📚 Learning: 2025-05-20T10:20:08.206Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1859
File: crates/quote/src/quote_debug.rs:472-492
Timestamp: 2025-05-20T10:20:08.206Z
Learning: In the Rain Orderbook codebase, the `#[tokio::test(flavor = "multi_thread")]` annotation is specifically needed for tests that use `LocalEvm`, not just for consistency across all async tests.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/apply.rscrates/cli/src/commands/local_db/decoded_events_to_sql.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/sync.rscrates/cli/src/commands/local_db/sync/token.rs
📚 Learning: 2025-07-21T16:37:20.599Z
Learnt from: 0xgleb
PR: rainlanguage/rain.orderbook#1911
File: crates/subgraph/src/utils/mod.rs:1-1
Timestamp: 2025-07-21T16:37:20.599Z
Learning: In the rainlanguage/rain.orderbook codebase, user 0xgleb prefers explicit module imports over re-exporting symbols at higher levels. Specifically for the float constants in crates/subgraph/src/utils/float.rs, they prefer using `utils::float::*` rather than re-exporting with `pub use float::*` in the utils module, as the explicit import makes it clearer what is being imported and why it's needed.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs
📚 Learning: 2025-10-13T12:57:05.900Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_orders/mod.rscrates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs
📚 Learning: 2025-07-04T10:27:22.544Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/local_db/query/fetch_order_trades/mod.rs
📚 Learning: 2025-07-16T10:40:05.717Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/mod.rscrates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs
📚 Learning: 2025-07-04T09:02:57.301Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/query/fetch_order_trades/mod.rs
📚 Learning: 2025-06-18T12:56:44.290Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1938
File: crates/js_api/src/raindex/mod.rs:102-118
Timestamp: 2025-06-18T12:56:44.290Z
Learning: In the rainlanguage/rain.orderbook codebase, it's acceptable to scaffold unused enum variants in initial implementation PRs when they will be implemented in future PRs, as confirmed by findolor.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rscrates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs
📚 Learning: 2025-07-04T09:13:39.020Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1956
File: packages/ui-components/src/lib/components/ButtonVaultLink.svelte:18-18
Timestamp: 2025-07-04T09:13:39.020Z
Learning: In the RaindexVault type, the vaultId field is a bigint, so bigintToHex should be used instead of bigintStringToHex when converting vault IDs to hex format in UI components.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs
📚 Learning: 2025-10-06T14:13:18.531Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/local_db/query/fetch_order_trades/mod.rs
📚 Learning: 2025-04-29T06:19:15.446Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/query/fetch_orders/mod.rs
📚 Learning: 2025-07-16T05:52:05.576Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1994
File: crates/common/src/raindex_client/vaults.rs:59-59
Timestamp: 2025-07-16T05:52:05.576Z
Learning: User findolor prefers to handle documentation updates for getter methods in batch via dedicated PRs rather than addressing them individually during feature development, as mentioned for the formatted amount string fields in crates/common/src/raindex_client/vaults.rs.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs
📚 Learning: 2025-06-18T12:54:47.807Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1938
File: crates/js_api/src/raindex/orders.rs:1-11
Timestamp: 2025-06-18T12:54:47.807Z
Learning: In the rain.orderbook repository, findolor prefers to keep unused imports in place when they will be used in upcoming follow-up PRs, rather than removing and re-adding them during incremental development.
Applied to files:
crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs
📚 Learning: 2025-08-02T02:34:32.237Z
Learnt from: brusherru
PR: rainlanguage/rain.orderbook#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/query/fetch_store_addresses/mod.rs
🧬 Code graph analysis (19)
crates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rs (1)
crates/common/src/raindex_client/local_db/query/mod.rs (4)
execute_query_json(68-78)serde_json(138-138)serde_json(292-292)create_success_callback(537-551)
crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rs (1)
crates/common/src/raindex_client/vaults.rs (2)
address(235-237)address(260-262)
crates/cli/src/commands/local_db/sync/runner/apply.rs (4)
crates/cli/src/commands/local_db/sync/runner/mod.rs (1)
fetch_events(280-292)crates/common/src/raindex_client/local_db/fetch.rs (1)
fetch_events(41-54)crates/common/src/raindex_client/local_db/sync.rs (4)
fetch_events(50-55)fetch_events(144-153)fetch_events(1289-1297)fetch_events(1430-1444)crates/cli/src/commands/local_db/sync/data_source.rs (2)
fetch_events(19-24)fetch_events(81-90)
crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs (1)
crates/common/src/raindex_client/local_db/query/mod.rs (1)
create_success_callback(537-551)
crates/cli/src/commands/local_db/fetch_events.rs (2)
crates/common/src/raindex_client/vaults.rs (3)
address(235-237)address(260-262)new(176-181)crates/common/src/raindex_client/local_db/sync.rs (6)
new(84-86)new(122-124)new(1150-1154)new(1178-1192)new(1262-1276)new(1368-1379)
crates/common/src/raindex_client/local_db/fetch.rs (3)
crates/common/src/raindex_client/local_db/insert.rs (1)
addr(1277-1279)crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rs (1)
addresses(25-28)crates/cli/src/commands/local_db/sync/storage.rs (1)
addresses(86-89)
crates/cli/src/commands/local_db/decoded_events_to_sql.rs (1)
crates/common/src/raindex_client/local_db/decode.rs (1)
U256(243-243)
crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rs (2)
crates/common/src/raindex_client/vaults.rs (2)
owner(98-100)owner(142-144)crates/common/src/raindex_client/orders.rs (9)
owner(92-94)owner(159-161)order_hash(88-90)order_hash(156-158)orders(574-592)order_bytes(84-86)order_bytes(153-155)meta(109-111)meta(171-173)
crates/common/src/raindex_client/local_db/insert.rs (2)
crates/common/src/raindex_client/local_db/decode.rs (1)
event(158-166)crates/common/src/raindex_client/local_db/tokens.rs (1)
build_event(73-82)
crates/common/src/raindex_client/local_db/query/mod.rs (1)
crates/common/src/rpc_client.rs (4)
serialize(88-93)serialize(108-116)deserialize(95-101)deserialize(118-125)
crates/cli/src/commands/local_db/sync/runner/mod.rs (4)
crates/cli/src/commands/local_db/sync/runner/window.rs (1)
params(199-207)crates/common/src/raindex_client/local_db/insert.rs (3)
value(42-42)raw_events(183-202)addr(1277-1279)crates/common/src/raindex_client/local_db/fetch.rs (1)
sort_events_by_block_and_log(340-348)crates/common/src/raindex_client/local_db/sync.rs (2)
sort_events_by_block_and_log(386-392)parse_hex_field(394-404)
crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs (1)
crates/common/src/raindex_client/local_db/query/mod.rs (1)
create_success_callback(537-551)
crates/common/src/rpc_client.rs (1)
crates/common/src/raindex_client/local_db/query/mod.rs (6)
serialize(94-99)serialize(153-164)serialize(226-231)deserialize(101-107)deserialize(166-174)deserialize(233-246)
crates/common/src/raindex_client/local_db/mod.rs (1)
crates/common/src/raindex_client/local_db/decode.rs (2)
U256(243-243)event(158-166)
crates/cli/src/commands/local_db/decode_events.rs (1)
crates/common/src/raindex_client/local_db/decode.rs (2)
U256(243-243)event(158-166)
crates/common/src/raindex_client/local_db/sync.rs (5)
crates/cli/src/commands/local_db/sync/runner/apply.rs (2)
fetch_events(20-34)fetch_events(164-171)crates/cli/src/commands/local_db/sync/runner/mod.rs (2)
fetch_events(280-292)parse_hex_field(190-200)crates/common/src/raindex_client/local_db/fetch.rs (1)
fetch_events(41-54)crates/cli/src/commands/local_db/sync/data_source.rs (2)
fetch_events(19-24)fetch_events(81-90)crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rs (1)
addresses(25-28)
crates/common/src/raindex_client/local_db/decode.rs (3)
crates/common/src/raindex_client/local_db/insert.rs (2)
bytes(1281-1283)value(42-42)crates/common/src/raindex_client/local_db/query/mod.rs (6)
serialize(94-99)serialize(153-164)serialize(226-231)deserialize(101-107)deserialize(166-174)deserialize(233-246)crates/common/src/rpc_client.rs (5)
serialize(88-93)serialize(108-116)deserialize(95-101)deserialize(118-125)from(313-315)
crates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs (2)
crates/common/src/raindex_client/local_db/decode.rs (1)
U256(243-243)crates/common/src/raindex_client/vaults.rs (6)
vault(1439-1443)vault(1444-1448)balance(106-108)balance(148-150)balance(187-189)balance(197-199)
crates/common/src/raindex_client/vaults.rs (3)
crates/common/src/raindex_client/transactions.rs (9)
id(23-25)id(43-45)from_local_parts(58-70)from(27-29)from(46-48)block_number(31-34)block_number(49-51)timestamp(36-39)timestamp(52-54)crates/common/src/raindex_client/orders.rs (7)
id(80-82)id(150-152)transaction(117-119)transaction(177-179)chain_id(76-78)chain_id(147-149)try_from_local_db_orders_csv(397-459)crates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs (1)
fetch_vault(24-37)
⏰ 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). (17)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: test
- GitHub Check: git-clean
- GitHub Check: Deploy-Docs-Preview
- GitHub Check: Deploy-Preview-Push
🔇 Additional comments (51)
crates/cli/src/commands/local_db/sync/token.rs (1)
87-87: LGTM! Type migration aligns with PR objectives and past feedback.The changes correctly migrate test data from string/hex representations to strongly-typed primitives:
B256::from([0u8; 32])for transaction hash follows the past review guidance to useB256(fixed-size) instead of dynamicBytes- Numeric zeros for
block_numberandblock_timestampare type-appropriateThe test still validates its intended behavior (reusing existing token metadata from DB), and the specific field values are not critical to this test's purpose.
Also applies to: 125-127
crates/common/src/raindex_client/local_db/tokens.rs (1)
67-67: LGTM! Proper type migration in tests.The test code correctly migrates to strongly-typed primitives (B256 for transaction hash, numeric types for block_number and block_timestamp), aligning with the PR's objective to add proper type support.
Also applies to: 76-78
crates/common/src/raindex_client/local_db/query/fetch_vault_balance_changes/mod.rs (5)
2-6: LGTM! Appropriate imports for type migration.The imports correctly bring in the alloy primitives and Float type needed for the strongly-typed fields.
29-42: LGTM! Type-safe function signature and correct SQL literal construction.The function now accepts strongly-typed parameters (
U256,Address) instead of strings. The SQL literal construction correctly:
- Converts
U256to hex-prefixed string viaB256encoding (line 34)- Formats
Addressusing lower-hex notation (line 35) as per previous review feedbackThe
U256 → B256conversion on line 34 is safe (both are 256-bit) and treats the vault ID as a byte sequence for SQL, which aligns with the test expectations on line 96.
56-61: LGTM! Test imports align with type migration.The test imports correctly bring in the necessary types and traits for constructing test data with the new strongly-typed fields.
64-131: LGTM! Comprehensive test coverage for type migration.Both tests correctly:
- Construct test data using the new strongly-typed APIs
- Verify serialization format (especially the hex-prefixed vault_id on lines 96-100)
- Validate SQL placeholder replacement with proper hex formatting (lines 126-127)
The tests confirm the expected behavior of the type conversions and serde helpers.
20-25: serde helpers for U256 and Float are correctly implementedcrates/common/src/raindex_client/local_db/sync.rs (2)
48-77: LGTM: Trait signatures properly typed.The trait method signatures now use strongly-typed
Addressprimitives instead of strings, improving type safety and reducing conversion overhead.
1019-1049: LGTM: Test helpers properly use typed primitives.The test helper functions correctly construct log entries using strongly-typed
Address,Bytes, andB256primitives. Themake_transaction_hashfunction creates deterministic test hashes, which is good for reproducible tests.crates/common/src/raindex_client/local_db/query/fetch_vault/mod.rs (5)
2-3: LGTM! Proper imports for type migration.The imports of
Address,U256from alloy primitives andFloatfrom rain_math_float support the migration from string-based to strongly-typed fields.
86-94: LGTM! Test imports support the new type system.The test imports correctly include the new types (
Address,U256,B256,Float) and helpers (encode_prefixed,address!macro,FromStr) needed for test construction and assertions.
99-109: LGTM! Test data construction uses appropriate APIs.The vault construction correctly uses:
U256::from(1)for the vault IDaddress!macro for address literals (as previously suggested by reviewer 0xgleb)Float::parse("16".into()).unwrap()for the balance- String format for I/O orders (consistent with struct definition)
111-115: LGTM! Robust verification of JSON serialization format.The test correctly verifies that
vault_idis serialized as a hex-prefixed string in JSON output, which is crucial for API contracts. The detailed assertion message will aid debugging if the format changes.
130-133: Verify Float equality semantics
Confirm whetherrain_math_float::FloatimplementsPartialEq; if so, useassert_eq!(data.balance, vault.balance)for a direct numeric comparison, otherwise document why comparing viaformat()(and unwrapping) is required.crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs (3)
31-32: LGTM: Idiomatic imports for typed address construction.Adding
AddressandFromStrimports is the standard approach for constructing strongly-typed addresses from string literals in tests.
37-46: LGTM: Test data properly migrated to Address type.The test data construction correctly uses
Address::from_str(...).unwrap()to parse valid Ethereum addresses. Using.unwrap()in test context is acceptable since test data is known to be valid, and a panic here would appropriately signal a test setup error.
5-8: Approve Address migration and confirm serialization compatibility.The
store_addresschange toAddressis safe:collect_all_store_addressesusesformat!("{:#x}", …).to_ascii_lowercase()to produce 0x-prefixed lowercase hex, andfetch_existing_store_addressesin the CLI expects and lowercases the same string format. No further changes needed.crates/cli/src/commands/local_db/tokens_fetch.rs (1)
89-111: Tests aligned with typed DecodedEventDataSwitch to B256 and numeric block fields is correct and consistent with the new decode types.
crates/common/src/raindex_client/local_db/query/fetch_order_trades/mod.rs (2)
8-41: Struct typing and serde adapters look correctUsing Address/Bytes and serde adapters for Float and U256 (as hex via serde_b256) is consistent with insert/query expectations.
148-166: Good: JSON shape validated for hex-encoded vault IDsThe test asserting hex-prefixed input/output_vault_id via B256::from(U256) confirms the intended JSON format.
crates/cli/src/commands/local_db/sync/runner/apply.rs (3)
20-34: Typed orderbook_address for fetch pathTaking Address here and threading it through the data source is the right move; matches surrounding fetch and SQL builders.
257-263: Test update LGTMUsing address! macro and typed arguments keeps the mock path coherent.
337-383: Raw events test fixtures updated correctlyAddress/Bytes/B256 usage matches LogEntryResponse’s typed fields.
crates/common/src/raindex_client/vaults.rs (6)
216-233: VaultToken.id and getters moved to Bytes/hexUsing Bytes for id with hex string getters (WASM) and Bytes (non-WASM) is a solid, consistent split.
Also applies to: 257-259
311-314: Local DB balance changes: typed inputsPassing U256 vault_id and Address token into the local DB fetch aligns with the new query interface.
846-856: Trade-side mapping: correct token id/address constructionCopying token address bytes into id and keeping Address separately is correct and matches other types.
1499-1541: Local DB vault conversion and composite idBuilding the composite id from orderbook/owner/token/vault_id bytes is correct and matches retrieval logic and tests.
1562-1566: Subgraph token mapping maintainedConversions between RaindexVaultToken and SgErc20 remain coherent with the new id type.
Also applies to: 1575-1580
1715-1722: Tests: consistent Address/Bytes assertionsUpdated expectations for Address/Bytes fields are correct and strengthen type guarantees.
Also applies to: 1757-1764, 2419-2427, 2967-2969
crates/cli/src/commands/local_db/decoded_events_to_sql.rs (2)
115-129: Tests correctly migrated to B256/numeric fieldsThe fixture updates (B256 tx hash, numeric block fields) align with the insert pipeline.
234-247: Deposit test data: good use of typed primitivesTyped Address/U256 ensure decimals mapping and SQL generation behave deterministically.
crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rs (2)
7-13: Erc20TokenRow.address typed as AddressStrongly-typed address here simplifies downstream usage and serde. Good change.
79-96: Parsing tests for Address fieldsDeserialization to Address validates the schema; looks good.
crates/cli/src/commands/local_db/decode_events.rs (1)
60-66: Event fixture construction updated to typed primitivesUsing Bytes::from(event.encode_data()), derived signature topic, Address::ZERO, and B256 tx hash is correct and matches decoder expectations.
Also applies to: 108-121
crates/common/src/rpc_client.rs (6)
52-65: LGTM! Type migration improves type safety.The LogEntryResponse struct has been successfully migrated from string-based representations to strongly-typed primitives (Address, Bytes, B256, u64). The custom serde helpers ensure proper serialization/deserialization of hex values.
67-82: LGTM! Robust hex parsing implementation.The
parse_hex_u64function correctly handles both hex (0x-prefixed, case-insensitive) and decimal string inputs, with proper validation for empty strings and clear error messages.
84-102: LGTM! Custom serde module for hex u64 serialization.The
serde_hex_u64module correctly implements bidirectional conversion between u64 and lowercase hex strings with 0x prefix, supporting both hex and decimal input during deserialization.
104-126: LGTM! Optional hex u64 serde module is correct.The
serde_opt_hex_u64module properly handles optional u64 values, with correct None handling and idiomatic use of transpose() for error propagation.
363-376: LGTM! Test helper updated for typed fields.The
sample_log_entryfunction correctly constructs a fixture with typed Address, Bytes, and B256 values, ensuring proper test coverage for the new types.
505-505: LGTM! Test assertion verifies numeric block number.The assertion correctly validates that the deserialized block_number is now a u64 (0x64 = 100 decimal), confirming the type migration works as expected.
crates/common/src/raindex_client/local_db/query/fetch_orders/mod.rs (5)
16-22: LGTM! Args struct migrated to typed primitives.The
FetchOrdersArgsstruct fields have been properly migrated from strings to strongly-typed Address and Bytes, improving type safety and consistency with the rest of the codebase.
52-74: LGTM! Order struct properly typed with preserved serde aliases.The
LocalDbOrderstruct has been successfully migrated to typed fields while maintaining backward compatibility through serde aliases. This ensures proper serialization/deserialization with existing data.
94-105: LGTM! Owner filtering uses correct hex formatting.The owner address formatting correctly uses
{owner:#x}to produce lowercase hex addresses without the 0x prefix, wrapped in SQL-compatible single quotes.
120-131: LGTM! Token filtering consistent with owner formatting.The token address formatting correctly mirrors the owner formatting approach, using
{token:#x}for consistent lowercase hex representation in SQL.
143-493: LGTM! Comprehensive test coverage for typed fields.The test suite has been thoroughly updated to work with the new typed fields, covering conversion logic, SQL generation, and filter combinations. Tests properly construct Address and Bytes instances using from_str and verify correct behavior.
crates/common/src/raindex_client/local_db/decode.rs (6)
38-51: LGTM! Event type matching updated for Bytes.The
from_topicmethod correctly uses byte-slice comparison withas_ref()to match event signatures, which is more efficient and type-safe than the previous string-based approach.
54-63: LGTM! Decoded event data uses native types.The
DecodedEventDatastruct fields have been properly migrated from strings to native types (u64 for block numbers/timestamps, B256 for transaction hash), improving type safety and consistency.
80-141: LGTM! Event decoding logic properly handles typed fields.The
decode_eventsfunction has been correctly updated to work with typed LogEntryResponse fields. The handling of empty data, unknown events, and field assignments all properly use the new types with appropriate conversions.
157-169: LGTM! Event decoding helper correctly converts types.The
decode_eventfunction properly converts Bytes topics and data to the format required by alloy'sdecode_raw_log, with appropriate error handling for invalid topic lengths.
171-199: LGTM! Store set decoding simplified with typed address.The
decode_store_set_eventfunction benefits from typed fields, directly usingevent.address(now Address) without string parsing, simplifying the logic and improving type safety.
201-960: LGTM! Comprehensive test helper updates for typed fields.The test helpers have been thoroughly updated to work with typed fields, including new helper functions (
hex_to_u64,bytes_from_hex_str) and updated event constructors. All test cases properly verify the new typed behavior.
| #[serde(with = "serde_b256")] | ||
| pub vault_id: U256, | ||
| pub token: Address, | ||
| pub owner: Address, | ||
| pub orderbook_address: Address, | ||
| pub token_name: String, | ||
| #[serde(alias = "tokenSymbol")] | ||
| pub token_symbol: String, | ||
| #[serde(alias = "tokenDecimals")] | ||
| pub token_decimals: u8, | ||
| pub balance: String, | ||
| #[serde(alias = "inputOrders")] | ||
| #[serde(with = "serde_float")] | ||
| pub balance: Float, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
Verify custom serde helpers exist and handle edge cases.
The migration to strongly-typed fields (U256, Address, Float) with custom serde attributes is a significant improvement over string-based representations. Ensure that serde_b256 and serde_float helpers are implemented correctly and handle edge cases (e.g., invalid hex strings, malformed floats, boundary values).
Run the following script to verify the serde helpers exist and review their implementations:
🏁 Script executed:
#!/bin/bash
# Description: Locate and examine serde_b256 and serde_float helper implementations
# Search for serde_b256 module/function definition
echo "=== Searching for serde_b256 ==="
rg -n -A10 'mod serde_b256|fn serde_b256' --type=rust
# Search for serde_float module/function definition
echo "=== Searching for serde_float ==="
rg -n -A10 'mod serde_float|fn serde_float' --type=rust
# Also check for any tests of these helpers
echo "=== Searching for tests of serde helpers ==="
rg -n -B2 -A5 '#\[test\].*serde_(b256|float)|test.*serde_(b256|float)' --type=rustLength of output: 2084
Add edge-case tests for custom serde helpers
Modules serde_b256 and serde_float are implemented in crates/common/src/raindex_client/local_db/query/mod.rs. Cover invalid hex, malformed floats, and boundary values with unit tests to ensure proper error handling.
| let store_addresses: Vec<Address> = store_addresses_vec | ||
| .iter() | ||
| .filter_map(|addr| Address::from_str(addr).ok()) | ||
| .collect(); |
There was a problem hiding this comment.
Add logging or error handling for failed address parsing.
The filter_map with .ok() silently discards addresses that fail to parse. This could hide data quality issues or cause incomplete synchronization if store addresses are malformed in the database.
Consider one of these approaches:
-let store_addresses: Vec<Address> = store_addresses_vec
- .iter()
- .filter_map(|addr| Address::from_str(addr).ok())
- .collect();
+let mut store_addresses = Vec::new();
+for addr in store_addresses_vec.iter() {
+ match Address::from_str(addr) {
+ Ok(address) => store_addresses.push(address),
+ Err(e) => {
+ // Log or propagate the error
+ return Err(LocalDbError::CustomError(format!(
+ "Invalid store address '{}': {}", addr, e
+ )));
+ }
+ }
+}Or at minimum, log warnings for skipped addresses while continuing:
let store_addresses: Vec<Address> = store_addresses_vec
.iter()
- .filter_map(|addr| Address::from_str(addr).ok())
+ .filter_map(|addr| {
+ Address::from_str(addr).or_else(|e| {
+ eprintln!("Warning: Skipping invalid store address '{}': {}", addr, e);
+ Err(e)
+ }).ok()
+ })
.collect();🤖 Prompt for AI Agents
In crates/common/src/raindex_client/local_db/sync.rs around lines 327 to 330,
the current filter_map(... .ok()) silently drops addresses that fail to parse;
instead, iterate over store_addresses_vec and attempt Address::from_str for each
entry, logging a warning (e.g., use the crate's logger: warn!("failed to parse
store address '{}': {}", addr, err)) when parsing fails and only push successful
parses into store_addresses, or alternately collect parse results and return an
Err with details; ensure the log includes the raw address and the parse error so
malformed DB entries are visible while still continuing to collect valid
addresses.
| fn sort_events_by_block_and_log(events: &mut [DecodedEventData<DecodedEvent>]) { | ||
| events.sort_by(|a, b| { | ||
| let block_a = parse_block_number(&a.block_number); | ||
| let block_b = parse_block_number(&b.block_number); | ||
| block_a | ||
| .cmp(&block_b) | ||
| .then_with(|| parse_block_number(&a.log_index).cmp(&parse_block_number(&b.log_index))) | ||
| a.block_number | ||
| .cmp(&b.block_number) | ||
| .then_with(|| parse_hex_field(&a.log_index).cmp(&parse_hex_field(&b.log_index))) | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider typing log_index as u64 for consistency.
While block_number is now properly typed as u64, log_index remains a string requiring runtime parsing during sorting. This creates inconsistency and impacts sort performance.
If log_index can be typed as u64 in the DecodedEventData structure, sorting would be more efficient:
events.sort_by(|a, b| {
a.block_number
.cmp(&b.block_number)
- .then_with(|| parse_hex_field(&a.log_index).cmp(&parse_hex_field(&b.log_index)))
+ .then_with(|| a.log_index.cmp(&b.log_index))
});If the underlying LogEntryResponse type still provides log_index as a string, consider parsing it once during DecodedEventData construction rather than repeatedly during sorting.
Committable suggestion skipped: line range outside the PR's diff.
2 participants
Motivation
See issue: #2182
This PR refactors the repository to have proper type support for newly added local db logic and some of the old logic.
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Refactor
Tests