Conversation
WalkthroughAdds interpreter_store_sets table and related schema/indexes; extends order_events. Exposes SQL constants and new fetch_store_addresses query. Introduces decoding, fetching, and inserting of InterpreterStoreSet events. Updates CLI sync to collect store addresses, fetch/decode/merge store-set events, and refactors sync via a DatabaseBridge abstraction. Adds binding for IInterpreterStoreV3. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant RaindexClient
participant Sync as sync_database_with_services
participant Bridge as DatabaseBridge
participant API as LocalDbApi
Client->>RaindexClient: sync_database(...)
RaindexClient->>Sync: start
Sync->>Bridge: check_required_tables()
Sync->>Bridge: get_last_synced_block()
alt has orderbook config dump
Sync->>Bridge: initialize_dump_if_needed()
end
Sync->>API: fetch_events(start..end)
API-->>Sync: logs
Sync->>API: decode_events(logs)
API-->>Sync: decoded_order_events
Sync->>Sync: collect_store_addresses(decoded_order_events)
Sync->>Bridge: fetch_existing_store_addresses()
Bridge-->>Sync: existing_store_addresses
Sync->>API: fetch_store_set_events(all_store_addresses, range)
API-->>Sync: store_set_logs
Sync->>API: decode_events(store_set_logs)
API-->>Sync: decoded_store_set_events
Sync->>Sync: merge + sort by block/log
Sync->>API: fetch_token_metadata(if needed)
API-->>Sync: token metadata
Sync->>API: generate_sql(all_decoded, metadata)
API-->>Sync: sql_statements
Sync->>Bridge: execute_sql(sql_statements)
Bridge-->>Sync: result
Sync-->>RaindexClient: done
RaindexClient-->>Client: success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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 |
ebe589e to
53285ed
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
crates/common/src/raindex_client/local_db/mod.rs(1 hunks)crates/common/src/raindex_client/local_db/query/clear_tables/query.sql(1 hunks)crates/common/src/raindex_client/local_db/query/create_tables/mod.rs(1 hunks)crates/common/src/raindex_client/local_db/query/create_tables/query.sql(3 hunks)crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs(1 hunks)crates/common/src/raindex_client/local_db/query/fetch_store_addresses/query.sql(1 hunks)crates/common/src/raindex_client/local_db/query/mod.rs(1 hunks)
🔇 Additional comments (7)
crates/common/src/raindex_client/local_db/query/create_tables/mod.rs (1)
18-18: LGTM!The addition of
interpreter_store_setstoREQUIRED_TABLEScorrectly reflects the new table being created.crates/common/src/raindex_client/local_db/query/clear_tables/query.sql (1)
14-14: LGTM!The
DROP TABLE IF EXISTS interpreter_store_setsstatement correctly handles cleanup of the new table within the transaction.crates/common/src/raindex_client/local_db/query/mod.rs (1)
8-8: LGTM!The public module export for
fetch_store_addressesfollows the established pattern and correctly exposes the new query functionality.crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs (1)
1-54: LGTM!The implementation follows established patterns in the codebase:
- Clean struct definition with appropriate derives
- Proper async function signature and error handling
- Good test coverage for both non-empty and empty result scenarios
crates/common/src/raindex_client/local_db/query/create_tables/query.sql (3)
43-44: LGTM! Schema changes are well-designed.The addition of
interpreter_addressandstore_addressas NOT NULL columns is appropriate for the CREATE TABLE statement. Since theclear_tablesfunctionality drops and recreates tables, this avoids migration issues with existing data.
155-155: LGTM! Index supports query performance.The index on
store_addressappropriately supports thefetch_store_addressesquery by enabling efficient lookups of store addresses in theorder_eventstable.
181-194: LGTM! Well-designed table and indexes.The
interpreter_store_setstable schema and associated indexes are well-designed:
- Primary key on
(transaction_hash, log_index)follows the standard pattern for event tables- Index on
store_addresssupports efficient store lookups (used byfetch_store_addresses)- Index on
block_numberenables efficient block-based queries- Index on
namespaceallows efficient namespace filteringAll columns marked NOT NULL is appropriate for immutable event data.
| pub const RAINTERPRETER_STORE_SET_TOPIC: &str = | ||
| "0xb0f9cbf5897504bdbdda49a4011cf35a8b4601d541d310d044e9001328f4befc"; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
Verify usage of RAINTERPRETER_STORE_SET_TOPIC constant.
The constant is defined but not referenced in any of the provided files. Ensure it's actually used in event decoding, filtering, or elsewhere in the codebase.
Run the following script to verify usage:
🏁 Script executed:
#!/bin/bash
# Search for usage of RAINTERPRETER_STORE_SET_TOPIC constant
rg -n 'RAINTERPRETER_STORE_SET_TOPIC' --type=rust -C 3Length of output: 577
Remove unused constant RAINTERPRETER_STORE_SET_TOPIC.
The constant is defined but never referenced; remove it if not needed or implement its intended event‐filtering usage.
🤖 Prompt for AI Agents
crates/common/src/raindex_client/local_db/mod.rs lines 21-22: the constant
RAINTERPRETER_STORE_SET_TOPIC is declared but unused; either remove the constant
declaration entirely or implement its intended use in event-filtering code where
topic constants are referenced (e.g., replace hard-coded topic strings with this
constant or add filtering logic that uses it). Update imports/refs accordingly
and run cargo build to ensure no compile errors remain.
There was a problem hiding this comment.
@findolor, thank you for confirming! Removing the unused constant is the right approach. 👍
🧠 Learnings used
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2123
File: crates/common/src/raindex_client/local_db/mod.rs:23-29
Timestamp: 2025-10-06T11:13:29.956Z
Learning: In `crates/common/src/raindex_client/local_db/mod.rs`, the `Default` implementation for `LocalDb` that creates an RPC client pointing to `http://localhost:4444` is acceptable because the RPC client must be explicitly configured before actual usage in production scenarios.
6f67408 to
b2d407a
Compare
| pub(crate) const ERC20_QUERY_TEMPLATE: &str = include_str!( | ||
| "../../../../../common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/query.sql" | ||
| ); | ||
| pub(crate) const STORE_ADDRESSES_QUERY: &str = include_str!( | ||
| "../../../../../common/src/raindex_client/local_db/query/fetch_store_addresses/query.sql" | ||
| ); |
There was a problem hiding this comment.
If we need these queries outside of common crate, should the common crate just expose those constants for import here?
| pub const RAINTERPRETER_STORE_SET_TOPIC: &str = | ||
| "0xb0f9cbf5897504bdbdda49a4011cf35a8b4601d541d310d044e9001328f4befc"; |
There was a problem hiding this comment.
Why are we using a hardcoded topic instead of one from alloy sol! bindings?
There was a problem hiding this comment.
replaced it with proper type
| let unique_addresses: Vec<String> = { | ||
| let mut dedup = HashSet::new(); | ||
| store_addresses | ||
| .iter() | ||
| .filter_map(|addr| { | ||
| let lower = addr.to_ascii_lowercase(); | ||
| if dedup.insert(lower.clone()) { | ||
| Some(lower) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| .collect() | ||
| }; |
There was a problem hiding this comment.
consider itertool's .unique (NOTE: the following will make all addresses lowercase. this becomes even simpler when using Address over String)
| let unique_addresses: Vec<String> = { | |
| let mut dedup = HashSet::new(); | |
| store_addresses | |
| .iter() | |
| .filter_map(|addr| { | |
| let lower = addr.to_ascii_lowercase(); | |
| if dedup.insert(lower.clone()) { | |
| Some(lower) | |
| } else { | |
| None | |
| } | |
| }) | |
| .collect() | |
| }; | |
| let unique_addresses: Vec<String> = store_addresses | |
| .iter() | |
| .map(|addr| addr.to_ascii_lowercase()) | |
| .unique() | |
| .collect(); |
There was a problem hiding this comment.
added itertools in upstream PR. i will add this comment to the type issue #2182 to fix this later on
| let mut jobs = Vec::new(); | ||
| let chunk_size = config.chunk_size.max(1); | ||
| let chunk_span = chunk_size.saturating_sub(1); | ||
| for address in unique_addresses.into_iter() { | ||
| let mut current_block = start_block; | ||
| while current_block <= end_block { | ||
| let to_block = current_block.saturating_add(chunk_span).min(end_block); | ||
| jobs.push((address.clone(), current_block, to_block)); | ||
| current_block = to_block.saturating_add(1); | ||
| if to_block == u64::MAX { | ||
| break; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
this doesn't really make it more concise but also an option if you like it
| let mut jobs = Vec::new(); | |
| let chunk_size = config.chunk_size.max(1); | |
| let chunk_span = chunk_size.saturating_sub(1); | |
| for address in unique_addresses.into_iter() { | |
| let mut current_block = start_block; | |
| while current_block <= end_block { | |
| let to_block = current_block.saturating_add(chunk_span).min(end_block); | |
| jobs.push((address.clone(), current_block, to_block)); | |
| current_block = to_block.saturating_add(1); | |
| if to_block == u64::MAX { | |
| break; | |
| } | |
| } | |
| } | |
| let chunk_size = config.chunk_size.max(1); | |
| let chunk_span = chunk_size.saturating_sub(1); | |
| let jobs: Vec<_> = unique_addresses | |
| .into_iter() | |
| .flat_map(|address| { | |
| std::iter::successors(Some(start_block), |¤t| { | |
| if current >= end_block || current == u64::MAX { | |
| None | |
| } else { | |
| Some(current.saturating_add(chunk_span).min(end_block).saturating_add(1)) | |
| } | |
| }) | |
| .map(move |current_block| { | |
| let to_block = current_block.saturating_add(chunk_span).min(end_block); | |
| (address.clone(), current_block, to_block) | |
| }) | |
| }) | |
| .collect(); |
There was a problem hiding this comment.
looks good can implement this
| Ok(all_events) | ||
| } | ||
|
|
||
| pub async fn fetch_store_set_events( |
There was a problem hiding this comment.
nitpick: consider breaking down this function into multiple ones to reduce complexity
| let mut store_addresses: HashSet<String> = collect_store_addresses(&decoded_events) | ||
| .into_iter() | ||
| .collect(); |
There was a problem hiding this comment.
nitpick: collect_store_addresses already returns BTreeSet, consider using a single set type
| let existing_stores: Vec<StoreAddressRow> = | ||
| LocalDbQuery::fetch_store_addresses(&db_callback).await?; | ||
| for row in existing_stores { | ||
| if !row.store_address.is_empty() { | ||
| store_addresses.insert(row.store_address.to_ascii_lowercase()); | ||
| } | ||
| } |
There was a problem hiding this comment.
nitpick: consider the following (once again, would be even simpler with Address as neither the filtering nor the mapping step would be needed)
| let existing_stores: Vec<StoreAddressRow> = | |
| LocalDbQuery::fetch_store_addresses(&db_callback).await?; | |
| for row in existing_stores { | |
| if !row.store_address.is_empty() { | |
| store_addresses.insert(row.store_address.to_ascii_lowercase()); | |
| } | |
| } | |
| let existing_stores: Vec<StoreAddressRow> = | |
| LocalDbQuery::fetch_store_addresses(&db_callback).await?; | |
| store_addresses.extend( | |
| existing_stores | |
| .into_iter() | |
| .filter(|row| !row.store_address.is_empty()) | |
| .map(|row| row.store_address.to_ascii_lowercase()) | |
| ); |
There was a problem hiding this comment.
adding this comment to the type issue #2182
| fn parse_block_number(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) | ||
| } else { | ||
| trimmed.parse::<u64>().unwrap_or(0) | ||
| } | ||
| } |
There was a problem hiding this comment.
nitpick: There is probably something for this in alloy
There was a problem hiding this comment.
i think we still need to check for the prefix so i couldn't find anything for that
There was a problem hiding this comment.
should we add test coverage for the new logic?
There was a problem hiding this comment.
i've updated the file to have trait so it can be easily tested for different scenarios
| let mut out = BTreeSet::new(); | ||
|
|
||
| for event in decoded_events { | ||
| match &event.decoded_data { | ||
| DecodedEvent::AddOrderV3(add) => { | ||
| out.insert(format!("0x{:x}", add.order.evaluable.store)); | ||
| } | ||
| DecodedEvent::RemoveOrderV3(remove) => { | ||
| out.insert(format!("0x{:x}", remove.order.evaluable.store)); | ||
| } | ||
| DecodedEvent::TakeOrderV3(take) => { | ||
| out.insert(format!("0x{:x}", take.config.order.evaluable.store)); | ||
| } | ||
| DecodedEvent::InterpreterStoreSet(set) => { | ||
| out.insert(format!("0x{:x}", set.store_address)); | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
|
|
||
| out |
There was a problem hiding this comment.
| let mut out = BTreeSet::new(); | |
| for event in decoded_events { | |
| match &event.decoded_data { | |
| DecodedEvent::AddOrderV3(add) => { | |
| out.insert(format!("0x{:x}", add.order.evaluable.store)); | |
| } | |
| DecodedEvent::RemoveOrderV3(remove) => { | |
| out.insert(format!("0x{:x}", remove.order.evaluable.store)); | |
| } | |
| DecodedEvent::TakeOrderV3(take) => { | |
| out.insert(format!("0x{:x}", take.config.order.evaluable.store)); | |
| } | |
| DecodedEvent::InterpreterStoreSet(set) => { | |
| out.insert(format!("0x{:x}", set.store_address)); | |
| } | |
| _ => {} | |
| } | |
| } | |
| out | |
| decoded_events | |
| .into_iter() | |
| .filter_map(|event| match event.decoded_data { | |
| DecodedEvent::AddOrderV3(add) => | |
| Some(encode_prefixed(add.order.evaluable.store)), | |
| DecodedEvent::RemoveOrderV3(remove) => | |
| Some(encode_prefixed(remove.order.evaluable.store)), | |
| DecodedEvent::TakeOrderV3(take) => | |
| Some(encode_prefixed(take.config.order.evaluable.store)), | |
| DecodedEvent::InterpreterStoreSet(set) => | |
| Some(encode_prefixed(set.store_address)), | |
| _ => None, | |
| }) | |
| .collect() |
NOTE: assumes alloy::hex::encode_prefix is imported
| pub(crate) const DEFAULT_SCHEMA_SQL: &str = CREATE_TABLES_SQL; | ||
| pub(crate) const SYNC_STATUS_QUERY: &str = FETCH_LAST_SYNCED_BLOCK_SQL; | ||
| pub(crate) const ERC20_QUERY_TEMPLATE: &str = FETCH_ERC20_TOKENS_BY_ADDRESSES_SQL; | ||
| pub(crate) const STORE_ADDRESSES_QUERY: &str = FETCH_STORE_ADDRESSES_SQL; |
There was a problem hiding this comment.
idk why we'd re-assign these but at least things can't diverge in values
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/common/src/raindex_client/local_db/query/fetch_store_addresses/query.sql (1)
1-7: Previous optimization suggestion remains valid.A previous review comment noted that the
DISTINCTclauses in each subquery are technically redundant sinceUNIONalready deduplicates the combined result. The suggestion was to either remove the per-queryDISTINCTclauses or useUNION ALLwith an outerSELECT DISTINCTif internal-table deduplication is desired for performance optimization.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (19)
crates/bindings/src/lib.rs(1 hunks)crates/cli/src/commands/local_db/sync/data_source.rs(3 hunks)crates/cli/src/commands/local_db/sync/runner/apply.rs(1 hunks)crates/cli/src/commands/local_db/sync/runner/mod.rs(9 hunks)crates/cli/src/commands/local_db/sync/runner/window.rs(1 hunks)crates/cli/src/commands/local_db/sync/storage.rs(4 hunks)crates/common/src/raindex_client/local_db/decode.rs(9 hunks)crates/common/src/raindex_client/local_db/fetch.rs(5 hunks)crates/common/src/raindex_client/local_db/insert.rs(15 hunks)crates/common/src/raindex_client/local_db/query/clear_tables/query.sql(1 hunks)crates/common/src/raindex_client/local_db/query/create_tables/mod.rs(2 hunks)crates/common/src/raindex_client/local_db/query/create_tables/query.sql(3 hunks)crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rs(2 hunks)crates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rs(2 hunks)crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs(1 hunks)crates/common/src/raindex_client/local_db/query/fetch_store_addresses/query.sql(1 hunks)crates/common/src/raindex_client/local_db/query/mod.rs(1 hunks)crates/common/src/raindex_client/local_db/sync.rs(19 hunks)crates/common/src/raindex_client/local_db/tokens.rs(3 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/mod.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/bindings/src/lib.rscrates/cli/src/commands/local_db/sync/data_source.rscrates/cli/src/commands/local_db/sync/storage.rscrates/cli/src/commands/local_db/sync/runner/window.rscrates/common/src/raindex_client/local_db/insert.rscrates/common/src/raindex_client/local_db/query/fetch_store_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/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/decode.rscrates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/tokens.rscrates/common/src/raindex_client/local_db/fetch.rscrates/common/src/raindex_client/local_db/sync.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/mod.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/bindings/src/lib.rscrates/cli/src/commands/local_db/sync/data_source.rscrates/cli/src/commands/local_db/sync/storage.rscrates/cli/src/commands/local_db/sync/runner/window.rscrates/common/src/raindex_client/local_db/insert.rscrates/common/src/raindex_client/local_db/query/fetch_store_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/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/decode.rscrates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/tokens.rscrates/common/src/raindex_client/local_db/fetch.rscrates/common/src/raindex_client/local_db/sync.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/mod.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/bindings/src/lib.rscrates/cli/src/commands/local_db/sync/data_source.rscrates/cli/src/commands/local_db/sync/storage.rscrates/cli/src/commands/local_db/sync/runner/window.rscrates/common/src/raindex_client/local_db/insert.rscrates/common/src/raindex_client/local_db/query/fetch_store_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/cli/src/commands/local_db/sync/runner/mod.rscrates/common/src/raindex_client/local_db/decode.rscrates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rscrates/common/src/raindex_client/local_db/tokens.rscrates/common/src/raindex_client/local_db/fetch.rscrates/common/src/raindex_client/local_db/sync.rs
🧠 Learnings (4)
📚 Learning: 2025-10-06T11:13:29.956Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2123
File: crates/common/src/raindex_client/local_db/mod.rs:23-29
Timestamp: 2025-10-06T11:13:29.956Z
Learning: In `crates/common/src/raindex_client/local_db/mod.rs`, the `Default` implementation for `LocalDb` that creates an RPC client pointing to `http://localhost:4444` is acceptable because the RPC client must be explicitly configured before actual usage in production scenarios.
Applied to files:
crates/cli/src/commands/local_db/sync/data_source.rs
📚 Learning: 2025-10-06T11:44:07.888Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2145
File: crates/common/src/raindex_client/local_db/query/create_tables/query.sql:71-72
Timestamp: 2025-10-06T11:44:07.888Z
Learning: The local DB feature in the rain.orderbook codebase is not live yet (as of PR #2145), so schema migrations for existing databases are not required when modifying table structures in `crates/common/src/raindex_client/local_db/query/create_tables/query.sql`.
Applied to files:
crates/cli/src/commands/local_db/sync/storage.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/fetch.rscrates/common/src/raindex_client/local_db/sync.rs
📚 Learning: 2025-04-07T07:50:17.023Z
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1575
File: packages/webapp/src/routes/deploy/layout.test.ts:28-37
Timestamp: 2025-04-07T07:50:17.023Z
Learning: In the rain.orderbook repository, the team considers it acceptable to use type assertions (like `as any`) for complex types in test files. This is preferred over creating detailed type definitions that would only be used for testing.
Applied to files:
crates/common/src/raindex_client/local_db/sync.rs
🧬 Code graph analysis (14)
crates/common/src/raindex_client/local_db/query/mod.rs (2)
crates/common/src/raindex_client/local_db/sync.rs (3)
fetch_store_addresses(35-35)fetch_store_addresses(98-100)fetch_store_addresses(1203-1206)crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs (1)
fetch_store_addresses(11-19)
crates/cli/src/commands/local_db/sync/runner/apply.rs (4)
crates/common/src/raindex_client/local_db/fetch.rs (1)
fetch_store_set_events(133-161)crates/cli/src/commands/local_db/sync/data_source.rs (2)
fetch_store_set_events(23-28)fetch_store_set_events(89-104)crates/cli/src/commands/local_db/sync/runner/mod.rs (1)
fetch_store_set_events(249-261)crates/cli/src/commands/local_db/sync/runner/window.rs (1)
fetch_store_set_events(164-171)
crates/cli/src/commands/local_db/sync/data_source.rs (4)
crates/common/src/raindex_client/local_db/fetch.rs (2)
fetch_store_set_events(133-161)default(27-34)crates/cli/src/commands/local_db/sync/runner/apply.rs (1)
fetch_store_set_events(128-135)crates/cli/src/commands/local_db/sync/runner/mod.rs (1)
fetch_store_set_events(249-261)crates/cli/src/commands/local_db/sync/runner/window.rs (1)
fetch_store_set_events(164-171)
crates/cli/src/commands/local_db/sync/storage.rs (5)
crates/common/src/raindex_client/local_db/query/create_tables/mod.rs (1)
create_tables(22-26)crates/common/src/raindex_client/local_db/query/fetch_erc20_tokens_by_addresses/mod.rs (1)
fetch_erc20_tokens_by_addresses(15-35)crates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rs (1)
fetch_last_synced_block(18-26)crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs (1)
fetch_store_addresses(11-19)crates/cli/src/commands/local_db/sqlite.rs (3)
sqlite_execute(8-41)sqlite_has_required_tables(90-95)sqlite_query_json(44-71)
crates/cli/src/commands/local_db/sync/runner/window.rs (4)
crates/common/src/raindex_client/local_db/fetch.rs (1)
fetch_store_set_events(133-161)crates/cli/src/commands/local_db/sync/data_source.rs (2)
fetch_store_set_events(23-28)fetch_store_set_events(89-104)crates/cli/src/commands/local_db/sync/runner/apply.rs (1)
fetch_store_set_events(128-135)crates/cli/src/commands/local_db/sync/runner/mod.rs (1)
fetch_store_set_events(249-261)
crates/common/src/raindex_client/local_db/insert.rs (1)
crates/common/src/raindex_client/local_db/tokens.rs (1)
build_event(73-82)
crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs (2)
crates/common/src/raindex_client/local_db/sync.rs (3)
fetch_store_addresses(35-35)fetch_store_addresses(98-100)fetch_store_addresses(1203-1206)crates/common/src/raindex_client/local_db/query/mod.rs (2)
execute_query_json(68-78)create_success_callback(323-337)
crates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rs (1)
crates/common/src/raindex_client/local_db/query/mod.rs (1)
execute_query_json(68-78)
crates/common/src/raindex_client/local_db/query/create_tables/mod.rs (2)
crates/common/src/raindex_client/local_db/sync.rs (3)
execute_query_text(41-41)execute_query_text(112-114)execute_query_text(1228-1238)crates/common/src/raindex_client/local_db/query/mod.rs (1)
execute_query_text(80-85)
crates/cli/src/commands/local_db/sync/runner/mod.rs (3)
crates/cli/src/commands/local_db/sync/storage.rs (1)
fetch_existing_store_addresses(36-42)crates/cli/src/commands/local_db/sync/data_source.rs (8)
decode_events(29-32)decode_events(106-111)fetch_events(17-22)fetch_events(78-87)fetch(45-46)fetch(53-66)fetch_store_set_events(23-28)fetch_store_set_events(89-104)crates/common/src/raindex_client/local_db/tokens.rs (1)
collect_store_addresses(28-52)
crates/common/src/raindex_client/local_db/decode.rs (2)
crates/common/src/raindex_client/local_db/sync.rs (4)
decode_events(56-59)decode_events(155-160)decode_events(1288-1293)decode_events(1438-1446)crates/common/src/raindex_client/local_db/mod.rs (1)
decode_events(213-220)
crates/common/src/raindex_client/local_db/tokens.rs (1)
crates/common/src/raindex_client/local_db/insert.rs (2)
sample_order(812-836)build_event(794-810)
crates/common/src/raindex_client/local_db/fetch.rs (2)
crates/common/src/raindex_client/local_db/sync.rs (10)
fetch_store_set_events(60-65)fetch_store_set_events(162-178)fetch_store_set_events(1295-1303)fetch_store_set_events(1448-1465)new(84-86)new(122-124)new(1139-1143)new(1167-1181)new(1251-1265)new(1357-1368)crates/common/src/rpc_client.rs (1)
sample_block_response(260-288)
crates/common/src/raindex_client/local_db/sync.rs (10)
crates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rs (2)
fetch_last_synced_block(18-26)LocalDbQuery(21-21)crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs (2)
fetch_store_addresses(11-19)LocalDbQuery(14-14)crates/common/src/raindex_client/local_db/query/fetch_tables/mod.rs (2)
LocalDbQuery(14-14)fetch_all_tables(11-15)crates/common/src/raindex_client/local_db/token_fetch.rs (1)
fetch_erc20_metadata_concurrent(7-55)crates/common/src/raindex_client/local_db/tokens.rs (2)
collect_store_addresses(28-52)collect_token_addresses(6-26)crates/common/src/raindex_client/local_db/mod.rs (6)
rpc_client(194-196)decode_events(213-220)decoded_events_to_sql(222-234)new(190-192)from(158-163)default(29-33)crates/common/src/raindex_client/local_db/query/mod.rs (1)
execute_query_text(80-85)crates/common/src/raindex_client/local_db/fetch.rs (4)
fetch_events(38-51)fetch_store_set_events(133-161)default(27-34)sort_events_by_block_and_log(341-349)crates/common/src/raindex_client/local_db/decode.rs (2)
decode_events(102-170)event(187-196)crates/common/src/raindex_client/local_db/insert.rs (1)
decoded_events_to_sql(91-168)
⏰ 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-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- 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-sol-artifacts)
- 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-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: test
- GitHub Check: git-clean
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Deploy-Preview-Push
- GitHub Check: Deploy-Docs-Preview
🔇 Additional comments (10)
crates/common/src/raindex_client/local_db/query/fetch_last_synced_block/mod.rs (2)
3-3: LGTM! SQL constant visibility change is appropriate.Making the SQL constant public aligns with the PR's pattern of centralizing and exposing SQL strings for reuse across modules.
21-25: LGTM! Call site updated correctly.The function now uses the public constant
FETCH_LAST_SYNCED_BLOCK_SQLinstead of the previous privateQUERY, maintaining the same functionality.crates/bindings/src/lib.rs (1)
25-28: Verify whether therpcflag is needed.The new
IInterpreterStoreV3binding uses#![sol(all_derives = true)]without therpcflag, unlikeIOrderBookV5(line 4) andIERC20(line 16). Confirm whether this omission is intentional based on how the binding will be used.crates/common/src/raindex_client/local_db/query/create_tables/mod.rs (3)
3-3: LGTM! SQL constant visibility change is appropriate.Making
CREATE_TABLES_SQLpublic aligns with the PR's pattern of exposing SQL constants for centralized access.
18-18: LGTM! New table added to required tables.The
interpreter_store_setstable is correctly added to theREQUIRED_TABLESlist, ensuring it's included in table existence checks and schema validation.
23-23: LGTM! Call site updated correctly.The function now uses the public constant
CREATE_TABLES_SQLinstead of the previous privateQUERY.crates/common/src/raindex_client/local_db/query/clear_tables/query.sql (1)
14-14: LGTM! Table drop statement added correctly.The
DROP TABLE IF EXISTS interpreter_store_sets;statement is correctly placed within the transaction block and uses the safeIF EXISTSclause, consistent with the other drop statements.crates/common/src/raindex_client/local_db/query/mod.rs (1)
8-8: LGTM! New module export added correctly.The
fetch_store_addressesmodule is correctly added to the public module list, following the established pattern.crates/cli/src/commands/local_db/sync/runner/apply.rs (1)
128-135: LGTM! Mock method implementation is appropriate.The
fetch_store_set_eventsmethod is correctly added to theMockDataSourcetest implementation. Returning an empty vector is appropriate for this test mock.crates/cli/src/commands/local_db/sync/runner/window.rs (1)
164-171: LGTM! Mock method implementation is appropriate.The
fetch_store_set_eventsmethod is correctly added to theMockDataSourcetest implementation. Returning an empty vector is appropriate for this test mock.
| let start_block = if last_synced_block == 0 { | ||
| orderbook_cfg.deployment_block | ||
| } else { | ||
| last_synced_block + 1 | ||
| }; | ||
|
|
||
| status.send("Fetching latest onchain events...".to_string())?; | ||
| let events = local_db | ||
| .fetch_events(orderbook_cfg.address.to_string(), start_block, latest_block) | ||
| .await | ||
| .map_err(|e| LocalDbError::FetchEventsFailed(Box::new(e)))?; |
There was a problem hiding this comment.
Guard sync when latest block is already processed
On Lines 308-318, once the database is caught up (last_synced_block == latest_block), start_block becomes last_synced_block + 1, so we call fetch_events with fromBlock > toBlock. Every RPC we target rejects that range, meaning the steady-state sync now fails with FetchEventsFailed. Please short-circuit before issuing the fetch (or otherwise clamp the range) so we skip the log queries when there is nothing new to process.
let start_block = if last_synced_block == 0 {
orderbook_cfg.deployment_block
} else {
last_synced_block + 1
};
+ if start_block > latest_block {
+ status.send(format!(
+ "No new blocks to sync (last_synced={}, latest={})",
+ last_synced_block, latest_block
+ ))?;
+ status.send("Database sync complete.".to_string())?;
+ return Ok(());
+ }
+
status.send("Fetching latest onchain events...".to_string())?;📝 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 start_block = if last_synced_block == 0 { | |
| orderbook_cfg.deployment_block | |
| } else { | |
| last_synced_block + 1 | |
| }; | |
| status.send("Fetching latest onchain events...".to_string())?; | |
| let events = local_db | |
| .fetch_events(orderbook_cfg.address.to_string(), start_block, latest_block) | |
| .await | |
| .map_err(|e| LocalDbError::FetchEventsFailed(Box::new(e)))?; | |
| let start_block = if last_synced_block == 0 { | |
| orderbook_cfg.deployment_block | |
| } else { | |
| last_synced_block + 1 | |
| }; | |
| if start_block > latest_block { | |
| status.send(format!( | |
| "No new blocks to sync (last_synced={}, latest={})", | |
| last_synced_block, latest_block | |
| ))?; | |
| status.send("Database sync complete.".to_string())?; | |
| return Ok(()); | |
| } | |
| status.send("Fetching latest onchain events...".to_string())?; | |
| let events = local_db | |
| .fetch_events(orderbook_cfg.address.to_string(), start_block, latest_block) | |
| .await | |
| .map_err(|e| LocalDbError::FetchEventsFailed(Box::new(e)))?; |
🤖 Prompt for AI Agents
In crates/common/src/raindex_client/local_db/sync.rs around lines 308 to 318,
the code can call fetch_events with fromBlock > toBlock when last_synced_block
== latest_block, causing RPC rejection; fix by short-circuiting before calling
fetch_events (e.g., if last_synced_block >= latest_block { optionally send a
status like "Already up-to-date" and skip/return early }) or clamp the range so
start_block <= latest_block; ensure no fetch_events call happens when there's
nothing new to process.
There was a problem hiding this comment.
that does not happen as the system is not running realtime
There was a problem hiding this comment.
@findolor, understood. If the system is not running continuously and you've confirmed this edge case won't occur in your deployment scenario, then no guard is needed. Thank you for the clarification!
🧠 Learnings used
Learnt from: findolor
PR: rainlanguage/rain.orderbook#2159
File: crates/cli/src/commands/local_db/sync/cli.rs:37-78
Timestamp: 2025-10-06T14:39:36.425Z
Learning: In the SyncLocalDb CLI command (crates/cli/src/commands/local_db/sync/cli.rs), block range validation (e.g., start_block <= end_block) is handled internally by the sync runner, not at the CLI entry point.
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
|
Tip For best results, initiate chat on the files or code changes. SIZE=L 🧠 Learnings used |
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Improvements
Refactor