Refactor decode logic for Set events for local db#2261
Merged
Conversation
Collaborator
Author
|
@coderabbitai review |
Contributor
WalkthroughAdded serde::Serialize/Deserialize derives to the generated IInterpreterStoreV3 binding. Replaced separate namespace/key/value fields on InterpreterStoreSetEvent with a single Changes
Sequence Diagram(s)sequenceDiagram
participant Event as Event Log
participant Decode as Decode Layer
participant DB as Database Layer
Event->>Decode: raw InterpreterStoreSet log
Decode->>Decode: extract store_address
Decode->>Decode: decode log into `Set` payload (namespace: U256, key, value)
Decode->>Decode: build InterpreterStoreSetEvent { store_address, payload }
Decode->>DB: pass InterpreterStoreSetEvent
DB->>DB: convert payload.namespace (U256) -> 32-byte BE
DB->>DB: INSERT using payload.namespace, payload.key, payload.value
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Contributor
✅ Actions performedReview triggered.
|
hardyjosh
approved these changes
Oct 24, 2025
0xgleb
approved these changes
Oct 27, 2025
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
crates/common/src/local_db/decode.rs(6 hunks)crates/common/src/local_db/sync.rs(2 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/local_db/sync.rscrates/common/src/local_db/decode.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/local_db/sync.rscrates/common/src/local_db/decode.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/local_db/sync.rscrates/common/src/local_db/decode.rs
🧠 Learnings (13)
📓 Common learnings
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/fuzz/mod.rs:64-64
Timestamp: 2025-07-04T09:02:57.301Z
Learning: In rainlanguage/rain.orderbook, user findolor prefers to limit type consistency changes to only the parts directly related to the current work scope. For example, when updating chain_id fields from u64 to u32 in fuzz-related code, unrelated files like tauri-app wallet commands can remain as u64 if they serve different purposes and aren't part of the current changes.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2117
File: crates/js_api/src/gui/select_tokens.rs:421-424
Timestamp: 2025-09-02T08:04:34.729Z
Learning: User findolor in the rainlanguage/rain.orderbook codebase prefers not to change test code when tests are already passing, even if there appear to be type mismatches or potential issues. Working tests should be left alone unless there are actual errors.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
📚 Learning: 2025-10-21T05:15:50.518Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2256
File: crates/common/src/raindex_client/local_db/query/fetch_tables.rs:7-7
Timestamp: 2025-10-21T05:15:50.518Z
Learning: In Rust 2021+ editions, passing a reference to a temporary into an async function within the same expression (e.g., `exec.query_json(&fetch_tables_stmt()).await`) is safe and idiomatic. The temporary lifetime is extended across the await point, so this pattern should not be flagged as an issue.
Applied to files:
crates/common/src/local_db/sync.rs
📚 Learning: 2025-10-06T11:44:07.888Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/create_tables/query.sql:71-72
Timestamp: 2025-10-06T11:44:07.888Z
Learning: The local DB feature in the rain.orderbook codebase is not live yet (as of PR #2145), so schema migrations for existing databases are not required when modifying table structures in `crates/common/src/raindex_client/local_db/query/create_tables/query.sql`.
Applied to files:
crates/common/src/local_db/sync.rs
📚 Learning: 2025-10-06T11:13:29.956Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2123
File: crates/common/src/raindex_client/local_db/mod.rs:23-29
Timestamp: 2025-10-06T11:13:29.956Z
Learning: In `crates/common/src/raindex_client/local_db/mod.rs`, the `Default` implementation for `LocalDb` that creates an RPC client pointing to `http://localhost:4444` is acceptable because the RPC client must be explicitly configured before actual usage in production scenarios.
Applied to files:
crates/common/src/local_db/sync.rs
📚 Learning: 2025-10-18T10:38:41.273Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2237
File: crates/common/src/raindex_client/local_db/sync.rs:79-89
Timestamp: 2025-10-18T10:38:41.273Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the sync_database method currently only supports indexing a single orderbook per chain ID, which is why `.first()` is used to select the orderbook configuration. Multi-orderbook support per chain ID is planned for future PRs.
Applied to files:
crates/common/src/local_db/sync.rs
📚 Learning: 2025-10-06T11:28:30.692Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql:6-7
Timestamp: 2025-10-06T11:28:30.692Z
Learning: In `crates/common/src/raindex_client/local_db/query/fetch_orders/query.sql`, the orderbook_address is currently hardcoded to '0x2f209e5b67A33B8fE96E28f24628dF6Da301c8eB' because the system only supports a single orderbook at the moment. Multiorderbook logic is not yet implemented and will be added in the future.
Applied to files:
crates/common/src/local_db/sync.rs
📚 Learning: 2025-05-20T10:20:08.206Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 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/common/src/local_db/sync.rs
📚 Learning: 2025-10-13T12:57:05.900Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2198
File: crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs:7-8
Timestamp: 2025-10-13T12:57:05.900Z
Learning: In the rain.orderbook repository, using unqualified serde helper names (e.g., `#[serde(with = "serde_address")]`) that rely on glob imports is acceptable and does not need to be changed to fully qualified paths.
Applied to files:
crates/common/src/local_db/sync.rs
📚 Learning: 2025-06-17T16:32:04.554Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: crates/js_api/src/gui/order_operations.rs:124-134
Timestamp: 2025-06-17T16:32:04.554Z
Learning: In the rain.orderbook codebase, RPC lists are typically small (2 items maximum), so performance optimizations around cloning and converting small Vec<Url> collections are generally unnecessary.
Applied to files:
crates/common/src/local_db/sync.rs
📚 Learning: 2025-07-16T10:40:05.717Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Applied to files:
crates/common/src/local_db/sync.rs
📚 Learning: 2025-06-18T18:24:32.049Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1938
File: crates/settings/src/yaml/orderbook.rs:180-199
Timestamp: 2025-06-18T18:24:32.049Z
Learning: In crates/settings/src/yaml/orderbook.rs, the user prefers to avoid refactoring duplicate search logic between get_orderbook_by_address and get_orderbook_by_network_key when there are only 2 functions, indicating they would consider it if more similar functions are added in the future.
Applied to files:
crates/common/src/local_db/sync.rs
📚 Learning: 2025-10-02T19:17:20.332Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/common/src/raindex_client/orders.rs:738-741
Timestamp: 2025-10-02T19:17:20.332Z
Learning: In crates/common/src/raindex_client/orders.rs, fetch_dotrain_source() is intentionally called in try_from_sg_order for every order conversion because the dotrain source information is needed immediately. A future optimization with local DB logic is planned to eliminate the network round-trip concern.
Applied to files:
crates/common/src/local_db/sync.rs
📚 Learning: 2025-06-17T16:46:19.035Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: crates/cli/src/commands/order/calldata.rs:47-57
Timestamp: 2025-06-17T16:46:19.035Z
Learning: In the CLI command `crates/cli/src/commands/order/calldata.rs`, the user prefers to let lower-level errors from `try_into_call()` bubble up when the RPCs list is empty, rather than adding early validation checks with custom error messages.
Applied to files:
crates/common/src/local_db/decode.rs
🧬 Code graph analysis (1)
crates/common/src/local_db/sync.rs (2)
crates/common/src/local_db/decode.rs (1)
U256(261-261)crates/common/src/local_db/insert.rs (1)
value(41-41)
⏰ 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: build-tauri (ubuntu-22.04, true)
- 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-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: git-clean
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: test
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Deploy-Docs-Preview
- GitHub Check: Deploy-Preview-Push
🔇 Additional comments (7)
crates/common/src/local_db/sync.rs (2)
346-348: LGTM! Import additions support the new payload structure.The imports for
Address,FixedBytes,U256, andSetare appropriately added to support the refactored event construction in tests.
712-716: LGTM! Test correctly uses the new payload structure.The test properly constructs
InterpreterStoreSetEventwith the newpayload: Setfield, aligning with the refactored event structure.crates/common/src/local_db/decode.rs (5)
104-114: LGTM! Struct definitions are clean and appropriate.The
InterpreterStoreSetEventstruct properly encapsulates theSetpayload with the store address, andUnknownEventDecodedprovides a clear structure for unknown events. Both derive appropriate traits for serialization and debugging.
444-448: LGTM! Test helper correctly constructs Set payload.The
create_store_set_logfunction properly creates aSetstruct with namespace, key, and value fields, matching the new event structure.
691-693: LGTM! Test assertions correctly access payload fields.The test properly accesses the Set fields through the
payloadfield, matching the new event structure.
699-720: LGTM! Serialization test ensures correct JSON format.The new test verifies that the
namespacefield serializes as a hex quantity string (e.g., "0x1234"), which is important for ensuring consistent JSON output format.
232-232: LGTM! Import needed for new test.The
serde_json::Valueimport is properly used in the new serialization test starting at line 711.
| sol_types::{abi::token::WordToken, SolEvent}, | ||
| }; | ||
| use core::convert::{TryFrom, TryInto}; | ||
| use core::convert::TryFrom; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Potentially unused import.
The core::convert::TryFrom import doesn't appear to be used in this file. The B256::try_from at line 209 uses the trait from the alloy crate's prelude. Consider removing this import to keep the code clean.
Apply this diff to remove the unused import:
-use core::convert::TryFrom;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| use core::convert::TryFrom; |
🤖 Prompt for AI Agents
In crates/common/src/local_db/decode.rs around line 7, the import "use
core::convert::TryFrom;" is unused; remove that import line and ensure there are
no remaining references that require it (the B256::try_from call uses the alloy
prelude trait), then run cargo check/build to confirm no compilation errors.
Comment on lines
+169
to
+172
| DecodedEvent::InterpreterStoreSet(Box::new(InterpreterStoreSetEvent { | ||
| store_address: Address::from_str(&event.address)?, | ||
| payload: decode_event::<Set>(event)?, | ||
| })) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
Verify error handling for Address::from_str.
The Address::from_str(&event.address)? call returns a different error type than DecodeError. Ensure that the appropriate From implementation exists to convert the address parsing error into DecodeError, or this will fail to compile.
Run the following script to verify compilation:
🏁 Script executed:
#!/bin/bash
# Verify that the code compiles successfully
cd crates/common
cargo check 2>&1 | grep -A 5 "error\|warning" || echo "No compilation errors found"Length of output: 246
🏁 Script executed:
#!/bin/bash
# Find DecodeError definition and check error handling
rg -n "enum DecodeError|struct DecodeError" crates/common/src/local_db/decode.rsLength of output: 25
🏁 Script executed:
#!/bin/bash
# Look for From implementations related to AddressError
rg -n "From.*Address" crates/common/src/local_db/ -A 2Length of output: 287
🏁 Script executed:
#!/bin/bash
# Get full context of the function containing lines 169-172
sed -n '150,185p' crates/common/src/local_db/decode.rsLength of output: 1713
🏁 Script executed:
#!/bin/bash
# Get DecodeError enum definition and surrounding context
sed -n '1,40p' crates/common/src/local_db/decode.rsLength of output: 1045
🏁 Script executed:
#!/bin/bash
# Search for all From implementations in the decode.rs file
rg -n "impl From" crates/common/src/local_db/decode.rs -A 3Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for DecodeError variants and see full definition
sed -n '19,35p' crates/common/src/local_db/decode.rsLength of output: 490
🏁 Script executed:
#!/bin/bash
# Check if there are any other error handling mechanisms or FromStr implementations for Address
rg -n "FromStr|from_str" crates/common/src/local_db/decode.rs -B 2 -A 2Length of output: 729
🏁 Script executed:
#!/bin/bash
# Search for AddressError handling anywhere in common crate
rg -n "AddressError" crates/common/src/Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check git log for this specific file to see if there are recent changes
git log --oneline -20 -- crates/common/src/local_db/decode.rs 2>/dev/null | head -10Length of output: 277
Fix Address error handling in InterpreterStoreSet decoding.
Line 170 uses Address::from_str(&event.address)? but DecodeError has no From implementation for address parsing errors. Add an error variant to DecodeError with #[from] for the address error type, or wrap the call with .map_err() to convert the error to DecodeError.
store_address: Address::from_str(&event.address)
.map_err(|e| DecodeError::AbiDecode(e.to_string()))?,Alternatively, add to DecodeError enum:
#[error("Address parse error: {0}")]
AddressParse(#[from] alloy::primitives::address::ParseError),🤖 Prompt for AI Agents
In crates/common/src/local_db/decode.rs around lines 169 to 172, the call to
Address::from_str(&event.address)? will not convert its parse error into the
DecodeError type; update the decoding to either map the parse error into
DecodeError (e.g. replace the ? with .map_err(|e|
DecodeError::AbiDecode(e.to_string()))?) or add a new DecodeError variant with
#[from] for alloy::primitives::address::ParseError (e.g. AddressParse(#[from]
alloy::primitives::address::ParseError)) so Address::from_str(...) can still use
?; pick one approach and apply it consistently where address parsing occurs.
Contributor
|
@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:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. SIZE=M 🧠 Learnings used |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
See issue: #2224
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Refactor