Add local database command with event fetching functionality#2102
Add local database command with event fetching functionality#2102
Conversation
WalkthroughAdds a LocalDb CLI group with FetchEvents and DecodeEvents commands, an EventClient trait plus SqliteWeb adapter, moves/updates CLI dependencies and .gitignore, and adjusts HyperRpcClient URL selection and a SqliteWeb accessor. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant CLI as CLI (Orderbook)
participant LDB as LocalDb::FetchEvents
participant SW as SqliteWeb (EventClient)
participant RPC as HyperRpcClient
participant FS as FileSystem
U->>CLI: orderbook local-db fetch-events [args]
CLI->>LDB: execute()
alt end_block provided
Note over LDB: use provided end_block
else end_block omitted
LDB->>SW: get_latest_block_number()
SW->>RPC: latestBlock()
RPC-->>SW: latest_block
SW-->>LDB: latest_block
end
LDB->>SW: fetch(address, start_block, end_block)
SW->>RPC: getLogs(address, range)
RPC-->>SW: events[]
SW-->>LDB: events[]
LDB->>LDB: serialize to pretty JSON
LDB->>FS: write output file (explicit or default)
FS-->>LDB: ok
LDB-->>CLI: Result<()>
CLI-->>U: Done
rect rgba(200,230,255,0.12)
Note over LDB,SW: New EventClient abstraction and SqliteWeb usage
end
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
crates/cli/.gitignore(1 hunks)crates/cli/Cargo.toml(2 hunks)crates/cli/src/commands/local_db/fetch_events.rs(1 hunks)crates/cli/src/commands/local_db/mod.rs(1 hunks)crates/cli/src/commands/mod.rs(1 hunks)crates/cli/src/lib.rs(3 hunks)crates/common/src/hyper_rpc.rs(1 hunks)crates/common/src/raindex_client/sqlite_web/mod.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
crates/**/{src,tests,benches,examples}/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
crates/**/{src,tests,benches,examples}/**/*.rs: Rust crates: lint withcargo clippy --workspace --all-targets --all-features -D warnings
Rust crates: ensurecargo test --workspacepasses
Rust crates: build withcargo buildduring development/readiness checks
Files:
crates/cli/src/lib.rscrates/cli/src/commands/mod.rscrates/common/src/hyper_rpc.rscrates/cli/src/commands/local_db/mod.rscrates/common/src/raindex_client/sqlite_web/mod.rscrates/cli/src/commands/local_db/fetch_events.rs
🧬 Code graph analysis (2)
crates/common/src/hyper_rpc.rs (1)
crates/common/src/raindex_client/sqlite_web/mod.rs (1)
new(64-67)
crates/cli/src/commands/local_db/mod.rs (1)
crates/cli/src/commands/local_db/fetch_events.rs (3)
fetch_events(12-17)fetch_events(26-33)fetch_events(185-198)
⏰ 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). (4)
- GitHub Check: git-clean
- GitHub Check: test
- GitHub Check: test
- GitHub Check: copilot-setup-steps
| let output_filename = self | ||
| .output_file | ||
| .map(PathBuf::from) | ||
| .unwrap_or_else(|| Self::default_output_path(end_block)); | ||
| let mut file = File::create(&output_filename)?; | ||
| file.write_all(serde_json::to_string_pretty(&all_events)?.as_bytes())?; | ||
|
|
There was a problem hiding this comment.
Ensure the output directory exists before writing files.
The default path (src/commands/local_db/events_*.json) is unlikely to exist for end users, and even custom paths fail if their parent directories are missing. As written, File::create will error with No such file or directory, breaking the command’s default flow. Create the parent directory before opening the file (or pick a default path that already exists).
let output_filename = self
.output_file
.map(PathBuf::from)
.unwrap_or_else(|| Self::default_output_path(end_block));
+ if let Some(parent) = output_filename.parent() {
+ if !parent.is_dir() {
+ std::fs::create_dir_all(parent)?;
+ }
+ }
let mut file = File::create(&output_filename)?;Also applies to: 87-93
🤖 Prompt for AI Agents
In crates/cli/src/commands/local_db/fetch_events.rs around lines 71-77 (and also
apply the same change for lines 87-93), the code calls File::create on an output
path whose parent directory may not exist; this will fail with "No such file or
directory." Before creating/writing the file, ensure the parent directory exists
by taking the output path (or default_output_path), get its parent directory,
and if Some(parent) call std::fs::create_dir_all(parent) propagating errors if
creation fails; then proceed to create and write the file as before.
There was a problem hiding this comment.
it's fine because either way we should be returning an error to the user. it's best to let the error come from io operations at this point
There was a problem hiding this comment.
@findolor, understood. The I/O error message will clearly indicate the issue to users. I'll skip this suggestion.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Add decode-events CLI command for processing JSON event files
| 42161 => "https://arbitrum.rpc.hypersync.xyz".to_string(), | ||
| _ => return Err(HyperRpcError::UnsupportedChainId { chain_id }), | ||
| }; | ||
| let rpc_url = format!("{}/{}", base_url, api_token); |
There was a problem hiding this comment.
| let rpc_url = format!("{}/{}", base_url, api_token); | |
| let rpc_url = format!("{base_url}/{api_token}"); |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
crates/cli/src/commands/local_db/fetch_events.rs(1 hunks)crates/common/src/hyper_rpc.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
crates/**/{src,tests,benches,examples}/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
crates/**/{src,tests,benches,examples}/**/*.rs: Rust crates: lint withcargo clippy --workspace --all-targets --all-features -D warnings
Rust crates: ensurecargo test --workspacepasses
Rust crates: build withcargo buildduring development/readiness checks
Files:
crates/common/src/hyper_rpc.rscrates/cli/src/commands/local_db/fetch_events.rs
🧬 Code graph analysis (2)
crates/common/src/hyper_rpc.rs (1)
crates/common/src/raindex_client/sqlite_web/mod.rs (1)
new(64-67)
crates/cli/src/commands/local_db/fetch_events.rs (1)
crates/common/src/hyper_rpc.rs (3)
get_latest_block_number(100-129)from(241-243)new(80-94)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Deploy-Docs-Preview
🔇 Additional comments (8)
crates/cli/src/commands/local_db/fetch_events.rs (7)
1-8: LGTM!All imports are relevant and necessary for the file's functionality. The use of
anyhow::Resultfor application-level error handling aligns with the learnings about anyhow usage.
9-18: LGTM!The
EventClienttrait provides a clean abstraction for dependency injection, enabling unit testing with mock implementations. Method signatures and error types are appropriate.
20-34: LGTM!The trait implementation correctly delegates to the underlying
SqliteWebmethods. Theself.fetch_events(...)call at line 32 resolves to the inherent method implementation (not the trait method), avoiding recursion. Based on learnings.
36-51: LGTM!The
FetchEventsstruct is well-designed for CLI usage with appropriate optional fields. Runtime validation in the execute methods is acceptable, asHyperRpcClient::newwill fail for unsupported chain IDs.
54-80: LGTM!The
execute_with_clientmethod correctly handles end block determination, event fetching, and file writing. Error messages provide appropriate context, and the generic design enables testability.
82-85: LGTM!The
executemethod correctly instantiatesSqliteWeband delegates to the generic implementation. Error propagation is appropriate.
97-199: LGTM!The test helpers are well-designed:
sample_eventprovides consistent test dataEnvVarGuardensures proper cleanup and prevents test pollutionMockEventClientuses a builder pattern for flexible test scenarioscrates/common/src/hyper_rpc.rs (1)
89-94: LGTM!The constructor return statement is correct, and the struct fields are properly initialized.
| fn default_output_path(end_block: u64) -> PathBuf { | ||
| let filename = format!("events_{}.json", end_block); | ||
| if let Ok(dir) = std::env::var("RAIN_ORDERBOOK_EVENTS_DIR") { | ||
| Path::new(&dir).join(filename) | ||
| } else { | ||
| Path::new("src/commands/local_db").join(filename) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Reconsider the fallback output path.
The default path src/commands/local_db/events_*.json is relative to the current working directory, not the crate root. This means:
- Running the CLI from different directories creates
src/commands/local_db/in the user's working directory - The path mimics source code structure, which is confusing for end users
- Output ends up in unexpected locations unless
RAIN_ORDERBOOK_EVENTS_DIRis set
Consider defaulting to the current directory for clarity:
fn default_output_path(end_block: u64) -> PathBuf {
let filename = format!("events_{}.json", end_block);
if let Ok(dir) = std::env::var("RAIN_ORDERBOOK_EVENTS_DIR") {
Path::new(&dir).join(filename)
} else {
- Path::new("src/commands/local_db").join(filename)
+ PathBuf::from(filename)
}
}Or document that users should set RAIN_ORDERBOOK_EVENTS_DIR and error if not set.
📝 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.
| fn default_output_path(end_block: u64) -> PathBuf { | |
| let filename = format!("events_{}.json", end_block); | |
| if let Ok(dir) = std::env::var("RAIN_ORDERBOOK_EVENTS_DIR") { | |
| Path::new(&dir).join(filename) | |
| } else { | |
| Path::new("src/commands/local_db").join(filename) | |
| } | |
| } | |
| fn default_output_path(end_block: u64) -> PathBuf { | |
| let filename = format!("events_{}.json", end_block); | |
| if let Ok(dir) = std::env::var("RAIN_ORDERBOOK_EVENTS_DIR") { | |
| Path::new(&dir).join(filename) | |
| } else { | |
| PathBuf::from(filename) | |
| } | |
| } |
🤖 Prompt for AI Agents
In crates/cli/src/commands/local_db/fetch_events.rs around lines 87-94, the
fallback output path currently uses "src/commands/local_db" which creates
source-like directories in whatever CWD the user runs the CLI from; change the
fallback to the current working directory instead (use std::env::current_dir()
or Path::new(".") and join the filename) so outputs land in the user's CWD, and
optionally add a short comment or doc note explaining that
RAIN_ORDERBOOK_EVENTS_DIR can be set to override the default or decide to return
an error if the env var is required.
| let base_url = match chain_id { | ||
| 8453 => "https://base.rpc.hypersync.xyz".to_string(), | ||
| 42161 => "https://arbitrum.rpc.hypersync.xyz".to_string(), | ||
| _ => return Err(HyperRpcError::UnsupportedChainId { chain_id }), | ||
| }; | ||
| let rpc_url = format!("{base_url}/{api_token}"); | ||
| let provider = Arc::new(Self::build_provider(&rpc_url)?); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
❓ Verification inconclusive
LGTM! Arbitrum support added correctly.
The URL construction now supports both Base (8453) and Arbitrum (42161) with consistent base URL patterns. The format string correctly appends the API token as a path segment. As per past review comment.
Verify that test coverage includes the new Arbitrum chain ID:
🏁 Script executed:
#!/bin/bash
# Description: Check if tests cover chain_id 42161 (Arbitrum)
# Expected: Find test cases that construct HyperRpcClient with chain_id 42161
rg -n "HyperRpcClient::new\(42161" crates/common/src/hyper_rpc.rsLength of output: 0
🏁 Script executed:
#!/bin/bash
# Search entire repository for HyperRpcClient::new with 42161 (Arbitrum) to find tests
rg -nP 'HyperRpcClient::new\s*\(\s*42161' -C3Length of output: 0
Add test for Arbitrum chain_id (42161)
- Test coverage currently lacks a case for
chain_id = 42161. Add a unit test invokingHyperRpcClient::new(42161, api_token)and asserting the constructed URL matcheshttps://arbitrum.rpc.hypersync.xyz/{api_token}.
🤖 Prompt for AI Agents
In crates/common/src/hyper_rpc.rs around lines 81 to 87, add a unit test that
covers chain_id = 42161 by calling HyperRpcClient::new(42161, api_token) and
asserting the constructed RPC URL equals
"https://arbitrum.rpc.hypersync.xyz/{api_token}"; if the constructed URL is not
accessible from the test, add a small test-only getter or make the URL-building
function pub(crate) so the test can verify the exact rpc_url string, then add
the test in the file's #[cfg(test)] module to assert equality with the expected
URL.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
crates/cli/src/commands/local_db/fetch_events.rs (2)
87-93: Reconsider the fallback output path for better user experience.The default path
src/commands/local_db/events_*.jsonis relative to the current working directory, not the crate root. This means:
- Running the CLI from different directories creates
src/commands/local_db/in the user's working directory- The path mimics source code structure, which is confusing for end users
- Output ends up in unexpected locations unless
RAIN_ORDERBOOK_EVENTS_DIRis setThis concern was previously raised but remains unresolved. Consider one of these alternatives:
- Default to the current directory (
.)- Require
RAIN_ORDERBOOK_EVENTS_DIRand error if not set- Use a well-known directory like
~/.rain_orderbook/events/Option 1 (simplest): Default to current directory:
fn default_output_path(end_block: u64) -> PathBuf { let filename = format!("events_{}.json", end_block); if let Ok(dir) = std::env::var("RAIN_ORDERBOOK_EVENTS_DIR") { Path::new(&dir).join(filename) } else { - Path::new("src/commands/local_db").join(filename) + PathBuf::from(filename) } }Option 2: Require the environment variable:
fn default_output_path(end_block: u64) -> PathBuf { let filename = format!("events_{}.json", end_block); - if let Ok(dir) = std::env::var("RAIN_ORDERBOOK_EVENTS_DIR") { - Path::new(&dir).join(filename) - } else { - Path::new("src/commands/local_db").join(filename) - } + let dir = std::env::var("RAIN_ORDERBOOK_EVENTS_DIR") + .expect("RAIN_ORDERBOOK_EVENTS_DIR environment variable must be set"); + Path::new(&dir).join(filename) }
12-17: Clarify thefetchmethod name to avoid ambiguity.The trait method
fetchis generic and could be confused with other fetch operations in the codebase. Past discussion (lines 87-94 comment thread) raised concerns about naming confusion. Consider renaming tofetch_eventsto match the underlying implementation and improve clarity.Apply this diff to rename the method:
- async fn fetch( + async fn fetch_events( &self, address: &str, start_block: u64, end_block: u64, ) -> Result<Vec<LogEntryResponse>, SqliteWebError>;And update the call site at line 67:
- let all_events = client - .fetch(&self.orderbook_address, self.start_block, end_block) + let all_events = client + .fetch_events(&self.orderbook_address, self.start_block, end_block) .await .map_err(|e| anyhow::anyhow!("Failed to fetch events: {}", e))?;And update the mock implementation at lines 185-190:
- async fn fetch( + async fn fetch_events( &self, _address: &str, _start_block: u64, _end_block: u64, ) -> Result<Vec<LogEntryResponse>, SqliteWebError> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
crates/cli/src/commands/local_db/fetch_events.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
crates/**/{src,tests,benches,examples}/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
crates/**/{src,tests,benches,examples}/**/*.rs: Rust crates: lint withcargo clippy --workspace --all-targets --all-features -D warnings
Rust crates: ensurecargo test --workspacepasses
Rust crates: build withcargo buildduring development/readiness checks
Files:
crates/cli/src/commands/local_db/fetch_events.rs
🧬 Code graph analysis (1)
crates/cli/src/commands/local_db/fetch_events.rs (2)
crates/common/src/hyper_rpc.rs (3)
get_latest_block_number(100-129)from(241-243)new(80-94)crates/common/src/raindex_client/sqlite_web/fetch.rs (1)
fetch_events(37-50)
🔇 Additional comments (4)
crates/cli/src/commands/local_db/fetch_events.rs (4)
20-34: LGTM! EventClient implementation delegates correctly.The trait implementation properly delegates to the underlying
SqliteWebmethods without introducing recursion, as confirmed by previous review learnings.
36-51: LGTM! FetchEvents struct is well-designed for CLI usage.The struct appropriately uses optional fields for
end_blockandoutput_file, enabling flexible command invocation. Runtime validation of block ranges occurs inexecute_with_client.
54-80: LGTM! Core execution logic is well-structured.The method correctly:
- Resolves
end_blockfrom the latest block when not provided- Fetches events with appropriate error context
- Writes formatted JSON output
Error handling properly wraps underlying errors with user-friendly messages.
97-320: Excellent test coverage with well-structured mocks.The test suite thoroughly covers:
- ✅ Success cases with explicit and default end blocks
- ✅ Error handling for RPC failures
- ✅ Error handling for event fetching failures
- ✅ Environment variable integration
- ✅ Default output path behavior
The
MockEventClientandEnvVarGuardare well-designed test utilities that enable clear, isolated testing.
| pub async fn execute_with_client<C: EventClient>(self, client: C) -> Result<()> { | ||
| println!("Starting event fetch..."); | ||
|
|
||
| let end_block = if let Some(end_block) = self.end_block { | ||
| end_block | ||
| } else { | ||
| client | ||
| .get_latest_block_number() | ||
| .await | ||
| .map_err(|e| anyhow::anyhow!("Failed to get latest block number: {}", e))? | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider validating block range ordering.
When both start_block and end_block are explicitly provided, there's no validation that start_block <= end_block. This could lead to unexpected behavior or confusing error messages from the underlying client.
Add validation before fetching events:
let end_block = if let Some(end_block) = self.end_block {
+ if self.start_block > end_block {
+ return Err(anyhow::anyhow!(
+ "start_block ({}) must be less than or equal to end_block ({})",
+ self.start_block,
+ end_block
+ ));
+ }
end_block
} else {
client
.get_latest_block_number()
.await
.map_err(|e| anyhow::anyhow!("Failed to get latest block number: {}", e))?
};📝 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.
| pub async fn execute_with_client<C: EventClient>(self, client: C) -> Result<()> { | |
| println!("Starting event fetch..."); | |
| let end_block = if let Some(end_block) = self.end_block { | |
| end_block | |
| } else { | |
| client | |
| .get_latest_block_number() | |
| .await | |
| .map_err(|e| anyhow::anyhow!("Failed to get latest block number: {}", e))? | |
| }; | |
| pub async fn execute_with_client<C: EventClient>(self, client: C) -> Result<()> { | |
| println!("Starting event fetch..."); | |
| let end_block = if let Some(end_block) = self.end_block { | |
| // Validate that the explicitly provided range is in the correct order | |
| if self.start_block > end_block { | |
| return Err(anyhow::anyhow!( | |
| "start_block ({}) must be less than or equal to end_block ({})", | |
| self.start_block, | |
| end_block | |
| )); | |
| } | |
| end_block | |
| } else { | |
| client | |
| .get_latest_block_number() | |
| .await | |
| .map_err(|e| anyhow::anyhow!("Failed to get latest block number: {}", e))? | |
| }; |
🤖 Prompt for AI Agents
In crates/cli/src/commands/local_db/fetch_events.rs around lines 54 to 64, add
validation that when both self.start_block and end_block are provided the start
is <= end; after resolving end_block (but before calling client to fetch events)
check if let Some(start) = self.start_block and if start > end_block then return
an Err(anyhow::anyhow!("Invalid block range: start_block ({}) is greater than
end_block ({})", start, end_block)); ensure the check uses the same numeric
types as the block values and short-circuits execution before any client calls.
| #[tokio::test] | ||
| async fn test_execute_with_client_success_with_explicit_end_block() { | ||
| let temp_file = NamedTempFile::new().unwrap(); | ||
| let temp_path = temp_file.path().to_str().unwrap().to_string(); | ||
|
|
||
| let fetch_events = FetchEvents { | ||
| api_token: "test_token".to_string(), | ||
| chain_id: 1, | ||
| start_block: 100, | ||
| end_block: Some(200), | ||
| orderbook_address: "0x123".to_string(), | ||
| output_file: Some(temp_path.clone()), | ||
| }; | ||
|
|
||
| let mock_client = MockEventClient::new().with_events(vec![sample_event("0x64")]); | ||
|
|
||
| let result = fetch_events.execute_with_client(mock_client).await; | ||
| assert!(result.is_ok()); | ||
|
|
||
| let content = std::fs::read_to_string(&temp_path).unwrap(); | ||
| let parsed: serde_json::Value = serde_json::from_str(&content).unwrap(); | ||
| assert_eq!(parsed[0]["blockNumber"], "0x64"); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_execute_with_client_success_with_latest_block() { | ||
| let temp_file = NamedTempFile::new().unwrap(); | ||
| let temp_path = temp_file.path().to_str().unwrap().to_string(); | ||
|
|
||
| let fetch_events = FetchEvents { | ||
| api_token: "test_token".to_string(), | ||
| chain_id: 1, | ||
| start_block: 100, | ||
| end_block: None, | ||
| orderbook_address: "0x123".to_string(), | ||
| output_file: Some(temp_path.clone()), | ||
| }; | ||
|
|
||
| let mock_client = MockEventClient::new() | ||
| .with_latest_block(500) | ||
| .with_events(vec![sample_event("0x1f4")]); | ||
|
|
||
| let result = fetch_events.execute_with_client(mock_client).await; | ||
| assert!(result.is_ok()); | ||
|
|
||
| let content = std::fs::read_to_string(&temp_path).unwrap(); | ||
| let parsed: serde_json::Value = serde_json::from_str(&content).unwrap(); | ||
| assert_eq!(parsed[0]["blockNumber"], "0x1f4"); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_execute_with_client_latest_block_error() { | ||
| let fetch_events = FetchEvents { | ||
| api_token: "test_token".to_string(), | ||
| chain_id: 1, | ||
| start_block: 100, | ||
| end_block: None, | ||
| orderbook_address: "0x123".to_string(), | ||
| output_file: Some("test_output.json".to_string()), | ||
| }; | ||
|
|
||
| let mock_client = | ||
| MockEventClient::new().with_latest_block_error("RPC connection failed".to_string()); | ||
|
|
||
| let result = fetch_events.execute_with_client(mock_client).await; | ||
| assert!(result.is_err()); | ||
| assert!(result | ||
| .unwrap_err() | ||
| .to_string() | ||
| .contains("Failed to get latest block number")); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_execute_with_client_fetch_events_error() { | ||
| let fetch_events = FetchEvents { | ||
| api_token: "test_token".to_string(), | ||
| chain_id: 1, | ||
| start_block: 100, | ||
| end_block: Some(200), | ||
| orderbook_address: "0x123".to_string(), | ||
| output_file: Some("test_output.json".to_string()), | ||
| }; | ||
|
|
||
| let mock_client = | ||
| MockEventClient::new().with_events_error("Network connection failed".to_string()); | ||
|
|
||
| let result = fetch_events.execute_with_client(mock_client).await; | ||
| assert!(result.is_err()); | ||
| assert!(result | ||
| .unwrap_err() | ||
| .to_string() | ||
| .contains("Failed to fetch events")); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_execute_with_client_default_output_filename() { | ||
| let temp_dir = TempDir::new().unwrap(); | ||
| let _env_guard = EnvVarGuard::set( | ||
| "RAIN_ORDERBOOK_EVENTS_DIR", | ||
| temp_dir.path().to_str().unwrap(), | ||
| ); | ||
|
|
||
| let fetch_events = FetchEvents { | ||
| api_token: "test_token".to_string(), | ||
| chain_id: 1, | ||
| start_block: 100, | ||
| end_block: Some(200), | ||
| orderbook_address: "0x123".to_string(), | ||
| output_file: None, | ||
| }; | ||
|
|
||
| let mock_client = MockEventClient::new().with_events(vec![]); | ||
|
|
||
| let result = fetch_events.execute_with_client(mock_client).await; | ||
| assert!(result.is_ok()); | ||
|
|
||
| let expected_filename = temp_dir.path().join("events_200.json"); | ||
| assert!(expected_filename.exists()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional: Consider additional edge case tests.
The current test suite is comprehensive. For even more thorough coverage, you could add:
- Invalid block range test: Verify behavior when
start_block > end_block(both provided) - File write failure test: Simulate filesystem errors (e.g., read-only directory)
These are nice-to-have improvements rather than requirements, as the core functionality is already well-tested.
Example test for invalid block range:
#[tokio::test]
async fn test_execute_with_client_invalid_block_range() {
let fetch_events = FetchEvents {
api_token: "test_token".to_string(),
chain_id: 1,
start_block: 200,
end_block: Some(100), // end < start
orderbook_address: "0x123".to_string(),
output_file: Some("test_output.json".to_string()),
};
let mock_client = MockEventClient::new().with_events(vec![]);
let result = fetch_events.execute_with_client(mock_client).await;
assert!(result.is_err());
// If validation is added, check error message
}🤖 Prompt for AI Agents
crates/cli/src/commands/local_db/fetch_events.rs lines 201-319: add two
additional tests to cover the edge cases suggested—(1) an invalid block range
where start_block > end_block: construct a FetchEvents with start_block >
end_block, call execute_with_client with a MockEventClient and assert the call
returns Err (and optionally assert a specific validation error message once
validation is implemented), and (2) a file write failure: create a temporary
directory or path that will fail on write (e.g., create a temp dir, change its
permissions to read-only or point RAIN_ORDERBOOK_EVENTS_DIR to a directory
without write permissions, then call execute_with_client with output_file None
or a filename and assert it returns Err and contains a filesystem/write error);
ensure tests clean up/reset env vars and permissions after running.
|
@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. 🧩 Analysis chain🏁 Script executed: Length of output: 455 🏁 Script executed: Length of output: 177 🏁 Script executed: Length of output: 671 SIZE=L 🧠 Learnings used |
3 participants
Caution
Chained PR - Do not merge before #2101
Motivation
This change introduces local database operations to the CLI, specifically focusing on
blockchain event fetching capabilities.
Solution
Added a new local-db command module with the following components:
to fetch events from blockchain networks and save them to JSON files
code with mock clients for unit testing
ID 42161) in addition to Base, and exposed the client through SqliteWeb for dependency
injection
handling, and edge cases using mock implementations
output files from version control
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Chores