refactor(spv): unify blockchain height handling in storage and sync layers #153
Conversation
…ayers - Updated storage and sync components to treat heights as blockchain heights instead of storage indices, simplifying the API and improving clarity. - Refactored methods in `StorageManager` to accept blockchain heights directly, eliminating the need for manual conversions. - Adjusted header retrieval and loading logic across various modules to align with the new height handling, enhancing consistency and reducing potential errors. - Improved logging to reflect the changes in height handling, providing clearer insights during synchronization processes.
|
Warning Rate limit exceeded@QuantumExplorer has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughRefactors SPV client, sync, and storage to use absolute blockchain heights everywhere instead of storage-relative indices; storage backends map between blockchain heights and storage indices when a sync base (checkpoint) exists; adds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Sync as SyncEngine
participant Storage
participant Peer
rect rgba(220,235,255,0.6)
note right of Sync: Header sync and progress use absolute blockchain heights
Client->>Sync: start sync / announce header
Sync->>Storage: get_tip_height() %% absolute height
Storage-->>Sync: tip_height
loop current_height <= tip_height
Sync->>Storage: load_headers(current_height..end_height)
Storage-->>Sync: headers[]
alt headers non-empty
Sync->>Storage: store_headers_from_height(headers, current_height)
Sync-->>Client: emit Progress(current_height=tip_height)
Sync->>Sync: current_height = end_height + 1
else no headers
Sync-->>Sync: warn and break
end
Sync->>Peer: request more (if needed)
Peer-->>Sync: headers / cfheaders
end
end
rect rgba(230,255,230,0.6)
note over Sync,Storage: Filter/CFHeaders use absolute heights
Sync->>Storage: get_header(height)
Storage-->>Sync: header | None
Sync->>Peer: getcfheaders(from..to)
Peer-->>Sync: cfheaders
Sync->>Storage: store filter headers (mapped via base if present)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
dash-spv/src/storage/memory.rs (1)
236-247: Bug: get_filter_header uses saturating_sub, returning index 0 for heights < base.This can return an unrelated filter header instead of None.
Apply:
- let idx = height.saturating_sub(base) as usize; - Ok(self.filter_headers.get(idx).copied()) + if base > 0 && height < base { + return Ok(None); + } + let idx = if base > 0 { + (height - base) as usize + } else { + height as usize + }; + Ok(self.filter_headers.get(idx).copied())dash-spv/src/storage/disk.rs (4)
942-945: Compile error: u32::is_multiple_of doesn’t exist (MSRV 1.89).This won’t compile with stable Rust; use modulo.
- if headers.len() >= 1000 || blockchain_height.is_multiple_of(1000) { + if headers.len() >= 1000 || blockchain_height % 1000 == 0 { self.save_dirty_segments().await?; }
1683-1693: Wrong heights when start_height < base in get_headers_batch.Mapping results as start_height + idx is incorrect if the loaded slice began at base > start_height.
- let headers = self.load_headers(start_height..range_end).await?; - let mut results = Vec::with_capacity(headers.len()); - for (idx, header) in headers.into_iter().enumerate() { - results.push((start_height + idx as u32, header)); - } + let base = *self.sync_base_height.read().await; + let effective_start = if base > 0 { start_height.max(base) } else { start_height }; + let headers = self.load_headers(effective_start..range_end).await?; + let mut results = Vec::with_capacity(headers.len()); + for (idx, header) in headers.into_iter().enumerate() { + results.push((effective_start + idx as u32, header)); + }
1518-1533: load_chain_state populates headers using default (empty) state; base flags parsed too late.You compute range_start using state.synced_from_checkpoint and state.sync_base_height before loading these from JSON, so headers are always loaded from 0. Reorder to parse JSON fields first.
- let content = tokio::fs::read_to_string(path).await?; - let value: serde_json::Value = serde_json::from_str(&content).map_err(|e| { + let content = tokio::fs::read_to_string(path).await?; + let value: serde_json::Value = serde_json::from_str(&content).map_err(|e| { StorageError::Serialization(format!("Failed to parse chain state: {}", e)) })?; - let mut state = ChainState::default(); - - // Load all headers - if let Some(tip_height) = self.get_tip_height().await? { - let range_start = if state.synced_from_checkpoint && state.sync_base_height > 0 { - state.sync_base_height - } else { - 0 - }; - state.headers = self.load_headers(range_start..tip_height + 1).await?; - } + let mut state = ChainState::default(); + // Load base flags first + state.sync_base_height = + value.get("sync_base_height").and_then(|v| v.as_u64()).map(|h| h as u32).unwrap_or(0); + state.synced_from_checkpoint = + value.get("synced_from_checkpoint").and_then(|v| v.as_bool()).unwrap_or(false); + // Load headers using correct base + if let Some(tip_height) = self.get_tip_height().await? { + let range_start = if state.synced_from_checkpoint && state.sync_base_height > 0 { + state.sync_base_height + } else { + 0 + }; + state.headers = self.load_headers(range_start..tip_height + 1).await?; + } @@ - // Load checkpoint sync fields - state.sync_base_height = - value.get("sync_base_height").and_then(|v| v.as_u64()).map(|h| h as u32).unwrap_or(0); - state.synced_from_checkpoint = - value.get("synced_from_checkpoint").and_then(|v| v.as_bool()).unwrap_or(false); + // (Already loaded above) Keep remaining fields belowAlso applies to: 1549-1556
1044-1109: Avoid awaiting with locks held (Clippy: await_holding_lock).Multiple paths hold RwLocks (cached_tip_height, header_hash_index, active_segments) across awaits (e.g., ensure_segment_loaded, save_segment_to_disk via eviction). This can stall other tasks and may trigger clippy -D warnings.
Refactor to:
- Call ensure_segment_loaded before acquiring segment map write.
- Scope each lock to the minimal region and drop before any await.
- For eviction/saves, extract data, drop the map lock, then await the disk write.
I can provide a targeted patch if you confirm preferred locking strategy. Based on coding guidelines.
Also applies to: 848-909, 515-547, 592-621
dash-spv/src/sync/sequential/mod.rs (3)
2145-2162: Bug: blockchain height double-counted when syncing from checkpointStorage now returns absolute blockchain heights. Adding
sync_base_heighthere inflates heights and can cause premature phase transitions and misleading logs in header handlers that rely on this helper.As per coding guidelines
Apply this diff:
pub(crate) async fn get_blockchain_height_from_storage(&self, storage: &S) -> SyncResult<u32> { let storage_height = storage .get_tip_height() .await .map_err(|e| SyncError::Storage(format!("Failed to get tip height: {}", e)))? .unwrap_or(0); - - // Check if we're syncing from a checkpoint - if self.header_sync.is_synced_from_checkpoint() - && self.header_sync.get_sync_base_height() > 0 - { - // For checkpoint sync, blockchain height = sync_base_height + storage_height - Ok(self.header_sync.get_sync_base_height() + storage_height) - } else { - // Normal sync: storage height IS the blockchain height - Ok(storage_height) - } + // Storage height is already the absolute blockchain height + Ok(storage_height) }
2050-2061: Post-sync MN state height still adds sync_base_heightMN state height is already absolute. Adding
sync_baseresults in inflated numbers and inconsistent logs vs the rest of the system.As per coding guidelines
- let mn_blockchain_height = if is_ckpt && sync_base > 0 { - sync_base + mn_state.last_height - } else { - mn_state.last_height - }; + let mn_blockchain_height = mn_state.last_height;
1094-1116: Centralize checkpoint height logic in headers_with_reorg.rs
Replace the manual sum on line 648 of dash-spv/src/sync/headers_with_reorg.rs—let actual_height = self.get_sync_base_height() + stored_headers;—with a call to the fixed helper (e.g.
self.get_blockchain_height_from_storage(storage).await?) so all checkpoint adjustments use the same logic.dash-spv/src/sync/masternodes.rs (1)
542-547: Fix clippy unused parameter warnings
sync_base_heightisn’t used anymore in these functions. Rename to_sync_base_height(or remove) to keep CI clippy-clean.As per coding guidelines
- async fn feed_qrinfo_and_get_additional_diffs( + async fn feed_qrinfo_and_get_additional_diffs( &mut self, qr_info: &QRInfo, storage: &mut S, network: &mut dyn NetworkManager, - sync_base_height: u32, + _sync_base_height: u32, ) -> Result<(), String> { ... - async fn fetch_diffs_with_hashes( + async fn fetch_diffs_with_hashes( &mut self, quorum_hashes: &std::collections::BTreeSet<QuorumHash>, storage: &mut S, network: &mut dyn NetworkManager, - sync_base_height: u32, + _sync_base_height: u32, ) -> Result<(), String> {Also applies to: 626-633
dash-spv/src/sync/filters.rs (2)
1328-1376: Clamp filter sync range to the checkpoint base.When
sync_base_height > 0(checkpoint restore), the default range computation drivesstartbelow the base height (e.g., base = 400_000, tip = 400_010 →startbecomes 399_910). That makesheader_abs_to_storage_index(batch_end)returnNone, and this branch immediately returns aSyncError::Storage. The sequential sync path therefore aborts as soon as it tries to prefetch filters after a checkpoint restore.Please clamp the effective start height to
sync_base_heightbefore issuing requests (and keep the caller-suppliedstart_heightuntouched when it is already ≥ base). One way to address it:- let start = start_height.unwrap_or_else(|| { - // Default: sync last blocks for recent transaction discovery - filter_tip_height.saturating_sub(DEFAULT_FILTER_SYNC_RANGE) - }); + let requested_start = start_height.unwrap_or_else(|| { + // Default: sync last blocks for recent transaction discovery + filter_tip_height.saturating_sub(DEFAULT_FILTER_SYNC_RANGE) + }); + let start = if self.sync_base_height > 0 { + requested_start.max(self.sync_base_height) + } else { + requested_start + };Apply the same guarding logic wherever we derive batch starts from absolute heights in this module.
1445-1536: Mirror the base-height clamp in the flow-control queue builder.
build_filter_request_queuehits the same regression assync_filters: with a checkpoint base,current_heightstarts belowsync_base_height, so the very firstheader_abs_to_storage_index(batch_end)returnsNoneand the method fails. That prevents flow-controlled filter sync from ever queuing requests after a checkpoint restore.Please reuse the clamping from the fix above before assigning
current_height:- let start = start_height - .unwrap_or_else(|| filter_header_tip_height.saturating_sub(DEFAULT_FILTER_SYNC_RANGE)); + let requested_start = start_height + .unwrap_or_else(|| filter_header_tip_height.saturating_sub(DEFAULT_FILTER_SYNC_RANGE)); + let start = if self.sync_base_height > 0 { + requested_start.max(self.sync_base_height) + } else { + requested_start + }; // Calculate the end height based on the requested countThis keeps flow-control working for checkpoint-based resumes and avoids the early
Validationerror.
🧹 Nitpick comments (6)
dash-spv/src/storage/disk.rs (2)
1162-1178: Clamp absolute→storage index with saturating_sub to avoid pre-base segment scans.Current mapping uses a conditional that can point before the checkpoint and cause unnecessary segment loads. Saturate to 0 like the memory backend.
- let sync_base_height = *self.sync_base_height.read().await; - let storage_start = if sync_base_height > 0 && range.start >= sync_base_height { - range.start - sync_base_height - } else { - range.start - }; - let storage_end = if sync_base_height > 0 && range.end > sync_base_height { - range.end - sync_base_height - } else { - range.end - }; + let sync_base_height = *self.sync_base_height.read().await; + let storage_start = range.start.saturating_sub(sync_base_height); + let storage_end = range.end.saturating_sub(sync_base_height);Also applies to: 1185-1192
1366-1383: Mirror saturating_sub mapping for filter headers load.Same rationale as headers path; reduces wasted I/O and keeps semantics symmetric.
- let storage_start = if sync_base_height > 0 && range.start >= sync_base_height { - range.start - sync_base_height - } else { - range.start - }; - let storage_end = if sync_base_height > 0 && range.end > sync_base_height { - range.end - sync_base_height - } else { - range.end - }; + let storage_start = range.start.saturating_sub(sync_base_height); + let storage_end = range.end.saturating_sub(sync_base_height);dash-spv/src/sync/sequential/mod.rs (1)
274-281: Rename misleading variable and trim stale log field
storage_based_heightnow holds an absolute tip height. Rename totip_heightand dropexpected_storage_indexfrom the log to avoid confusion with old storage-index semantics.As per coding guidelines
- tracing::info!( - "Starting masternode sync: effective_height={}, sync_base={}, storage_tip={:?}, chain_state_height={}, expected_storage_index={}", - effective_height, - sync_base_height, - storage_tip, - chain_state_height, - if sync_base_height > 0 { effective_height.saturating_sub(sync_base_height) } else { effective_height } - ); + tracing::info!( + "Starting masternode sync: effective_height={}, sync_base={}, storage_tip={:?}, chain_state_height={}", + effective_height, + sync_base_height, + storage_tip, + chain_state_height + ); ... - let _safe_height = if let Some(tip) = storage_tip { - let storage_based_height = tip; - if storage_based_height < effective_height { + let _safe_height = if let Some(tip) = storage_tip { + let tip_height = tip; + if tip_height < effective_height { tracing::warn!( "Chain state height {} exceeds storage height {}, using storage height", effective_height, - storage_based_height + tip_height ); - storage_based_height + tip_height } else { effective_height } } else { effective_height };Also applies to: 283-296
dash-spv/src/client/status_display.rs (1)
46-54: Comment and fallback mismatch with absolute-height semantics
- The “Unified formula … checkpoint” comment is stale; we now treat
storage_tipas the blockchain height.- Consider returning
0when no headers are present instead ofstate.sync_base_heightto avoid implying progress before any header is stored.As per coding guidelines
- // Unified formula for both checkpoint and genesis sync: - // For genesis sync: sync_base_height = 0, so height = 0 + storage_count - // For checkpoint sync: height = checkpoint_height + storage_count + // Heights are absolute: storage_tip is the blockchain tip height. let storage = self.storage.lock().await; if let Ok(Some(storage_tip)) = storage.get_tip_height().await { let blockchain_height = storage_tip; if with_logging { tracing::debug!( - "Status display: reported tip height={}, sync_base={}, raw_storage_tip={}", + "Status display: reported tip height={}, raw_storage_tip={}", blockchain_height, - state.sync_base_height, storage_tip ); } blockchain_height } else { // No headers in storage yet - state.sync_base_height + 0 }Also applies to: 55-58, 61-64
dash-spv/src/sync/masternodes.rs (2)
548-566: Blocking callback: acceptable, but watch for I/O latency
block_in_place + block_onis a pragmatic bridge for the engine’s sync callback. If storage backends perform heavier I/O, consider a small cache map of hash→height pre-filled from QRInfo to reduce callback roundtrips.
383-395: Edge case: empty storage causes ‘Tip header not found’When no headers are stored yet,
tip_height.unwrap_or(0)followed byget_header(0)may fail and abort MN sync. Guard this by early‑returningOk(false)until headers exist.- let tip_header = storage + if tip_height == 0 { + tracing::info!("Masternode sync: no headers in storage yet, deferring"); + self.sync_in_progress = false; + return Ok(false); + } + let tip_header = storage .get_header(tip_height)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
dash-spv/src/client/mod.rs(5 hunks)dash-spv/src/client/status_display.rs(1 hunks)dash-spv/src/storage/disk.rs(7 hunks)dash-spv/src/storage/memory.rs(2 hunks)dash-spv/src/storage/mod.rs(3 hunks)dash-spv/src/sync/filters.rs(20 hunks)dash-spv/src/sync/headers_with_reorg.rs(3 hunks)dash-spv/src/sync/masternodes.rs(1 hunks)dash-spv/src/sync/sequential/mod.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/client/mod.rsdash-spv/src/client/status_display.rsdash-spv/src/sync/sequential/mod.rsdash-spv/src/sync/headers_with_reorg.rsdash-spv/src/sync/filters.rsdash-spv/src/storage/mod.rsdash-spv/src/storage/memory.rsdash-spv/src/sync/masternodes.rsdash-spv/src/storage/disk.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv/src/client/mod.rsdash-spv/src/client/status_display.rsdash-spv/src/sync/sequential/mod.rsdash-spv/src/sync/headers_with_reorg.rsdash-spv/src/sync/filters.rsdash-spv/src/storage/mod.rsdash-spv/src/storage/memory.rsdash-spv/src/sync/masternodes.rsdash-spv/src/storage/disk.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv/src/client/mod.rsdash-spv/src/client/status_display.rsdash-spv/src/sync/sequential/mod.rsdash-spv/src/sync/headers_with_reorg.rsdash-spv/src/sync/filters.rsdash-spv/src/storage/mod.rsdash-spv/src/storage/memory.rsdash-spv/src/sync/masternodes.rsdash-spv/src/storage/disk.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Provide both
MemoryStorageManagerandDiskStorageManagerbehind theStorageManagertrait
Files:
dash-spv/src/storage/mod.rsdash-spv/src/storage/memory.rsdash-spv/src/storage/disk.rs
🧠 Learnings (7)
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : All code must be clippy-clean: run `cargo clippy --all-targets --all-features -- -D warnings`
Applied to files:
dash-spv/src/client/mod.rsdash-spv/src/client/status_display.rsdash-spv/src/storage/mod.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/src/error.rs : Define and maintain domain-specific error types in `src/error.rs` (NetworkError, StorageError, SyncError, ValidationError, SpvError) and update them when adding new failure modes
Applied to files:
dash-spv/src/client/mod.rs
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
Applied to files:
dash-spv/src/client/mod.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
Applied to files:
dash-spv/src/client/mod.rs
📚 Learning: 2025-06-26T15:52:45.327Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T15:52:45.327Z
Learning: In Rust, calling `headers.last().unwrap()` or `headers[headers.len() - 1]` is only safe if the vector is guaranteed non-empty; placing such code outside a tightly-coupled empty check is fragile and risks panics if future refactoring bypasses the check. Prefer pattern matching or moving logic inside the non-empty branch for robustness.
Applied to files:
dash-spv/src/sync/headers_with_reorg.rs
📚 Learning: 2025-06-26T15:54:02.509Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T15:54:02.509Z
Learning: The `StorageManager` trait in `dash-spv/src/storage/mod.rs` uses `&mut self` methods but is also `Send + Sync`, and implementations often use interior mutability for concurrency. This can be confusing, so explicit documentation should clarify thread-safety expectations and the rationale for the API design.
Applied to files:
dash-spv/src/storage/mod.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Provide both `MemoryStorageManager` and `DiskStorageManager` behind the `StorageManager` trait
Applied to files:
dash-spv/src/storage/mod.rsdash-spv/src/storage/disk.rs
🧬 Code graph analysis (5)
dash-spv/src/sync/headers_with_reorg.rs (3)
key-wallet-manager/src/wallet_manager/mod.rs (1)
current_height(1035-1037)dash-spv/src/client/mod.rs (2)
tip_height(104-107)storage(2400-2402)dash-spv/src/types.rs (1)
tip_height(229-238)
dash-spv/src/storage/mod.rs (3)
dash-spv/src/storage/disk.rs (6)
get_header(1211-1273)get_tip_height(1275-1287)store_filter_headers(1289-1365)load_filter_headers(1367-1413)get_filter_header(1415-1450)store_filter(1558-1562)dash-spv/src/storage/memory.rs (6)
get_header(154-171)get_tip_height(173-184)store_filter_headers(186-191)load_filter_headers(193-234)get_filter_header(236-247)store_filter(282-285)dash-spv/src/sync/filters.rs (1)
store_filter_headers(2162-2309)
dash-spv/src/storage/memory.rs (2)
dash-spv/src/storage/disk.rs (3)
new(210-324)get_header(1211-1273)get_tip_height(1275-1287)dash-spv/src/storage/mod.rs (3)
get_header(31-31)get_header(113-113)get_tip_height(116-116)
dash-spv/src/sync/masternodes.rs (1)
dash-spv/src/client/mod.rs (1)
storage(2400-2402)
dash-spv/src/storage/disk.rs (3)
dash-spv/src/storage/memory.rs (1)
get_tip_height(173-184)dash-spv/src/storage/mod.rs (1)
get_tip_height(116-116)dash-spv/src/types.rs (1)
tip_height(229-238)
⏰ 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). (15)
- GitHub Check: Format
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_outpoint_string)
🔇 Additional comments (4)
dash-spv/src/storage/memory.rs (2)
173-184: LGTM: get_tip_height now reports blockchain height.Consistent with disk backend and PR intent.
402-421: LGTM: get_headers_batch maps absolute heights correctly.Iterating abs_h and translating via base avoids off-by-base mistakes.
dash-spv/src/storage/mod.rs (1)
112-116: Verify callers use absolute blockchain heights
Grep results confirmget_header,load_headers, and related methods are invoked throughout tests and sync modules; manually ensure each call’sheight(or range) argument reflects the blockchain’s absolute height, not a storage-relative index.dash-spv/src/sync/masternodes.rs (1)
679-684: Ensure Clippy is clean
Unable to runcargo clippy --all-targets --all-features -- -D warningsin this environment (proc not mounted). Please verify locally that Clippy emits no warnings for the changes in dash-spv/src/sync/masternodes.rs (lines 679–721).
… request full blocks for privacy
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dash-spv/src/client/message_handler.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/client/message_handler.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv/src/client/message_handler.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv/src/client/message_handler.rs
🧠 Learnings (2)
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
PR: dashpay/rust-dashcore#0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.
Applied to files:
dash-spv/src/client/message_handler.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Provide both `MemoryStorageManager` and `DiskStorageManager` behind the `StorageManager` trait
Applied to files:
dash-spv/src/client/message_handler.rs
🧬 Code graph analysis (1)
dash-spv/src/client/message_handler.rs (2)
dash-spv/src/client/mod.rs (1)
new(198-265)dash-spv/src/client/block_processor.rs (1)
new(46-64)
⏰ 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: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Core Components Tests
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: SPV Components Tests
… base > 0 in MemoryStorageManager::get_header\n\nPrevents incorrect header lookup by avoiding implicit storage-index interpretation for absolute heights below the checkpoint base; aligns in-memory behavior with absolute height semantics.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dash-spv/src/storage/memory.rs (1)
239-249: Reject pre-base heights in filter header lookup.
height.saturating_sub(base)turns any request belowsync_base_heightinto index0, so callers asking for a height that predates the checkpoint will silently receive the header at the checkpoint height. That defeats the absolute-height contract and can return incorrect filter headers.- let idx = height.saturating_sub(base) as usize; - Ok(self.filter_headers.get(idx).copied()) + if base > 0 && height < base { + return Ok(None); + } + let idx = if base > 0 { + (height - base) as usize + } else { + height as usize + }; + Ok(self.filter_headers.get(idx).copied())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dash-spv/src/storage/memory.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/storage/memory.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Provide both
MemoryStorageManagerandDiskStorageManagerbehind theStorageManagertrait
Files:
dash-spv/src/storage/memory.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv/src/storage/memory.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv/src/storage/memory.rs
🧬 Code graph analysis (1)
dash-spv/src/storage/memory.rs (2)
dash-spv/src/storage/disk.rs (3)
new(210-324)get_header(1211-1273)get_tip_height(1275-1287)dash-spv/src/storage/mod.rs (3)
get_header(31-31)get_header(113-113)get_tip_height(116-116)
⏰ 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). (20)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Key Wallet Components Tests
- GitHub Check: SPV Components Tests
- GitHub Check: Core Components Tests
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_outpoint_string)
…e ad-hoc local cfilter gating
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dash-spv/src/client/message_handler.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/client/message_handler.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv/src/client/message_handler.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv/src/client/message_handler.rs
🧠 Learnings (2)
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Provide both `MemoryStorageManager` and `DiskStorageManager` behind the `StorageManager` trait
Applied to files:
dash-spv/src/client/message_handler.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{managed_account,transaction_checking}/**/*.rs : Update wallet state atomically: add transaction, update UTXOs, recalculate balance, and mark addresses used in a single operation
Applied to files:
dash-spv/src/client/message_handler.rs
⏰ 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). (16)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: Core Components Tests
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: SPV Components Tests
🔇 Additional comments (1)
dash-spv/src/client/message_handler.rs (1)
455-465: LGTM: proactive block fetch keeps privacy intactGreat to see the inventory path now always issuing a
getdatafor announced blocks—this keeps the old privacy guarantees intact while the new absolute-height handling kicks in.
…rageManager::get_header Updated the logic in MemoryStorageManager::get_header to skip heights below the base when base > 0, preventing unnecessary iterations and aligning behavior with expected height semantics.
…_handler Updated the message_handler to clone the block header when constructing the headers message. This change ensures that the original block header remains intact and avoids potential issues with ownership and borrowing in the processing flow.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dash-spv/src/storage/memory.rs (1)
239-250: Bug: get_filter_header wrongly returns index 0 for heights below base.saturating_sub maps abs heights < base to 0, returning an incorrect header. Must return None like get_header.
Apply:
@@ - let idx = height.saturating_sub(base) as usize; - Ok(self.filter_headers.get(idx).copied()) + if base > 0 && height < base { + return Ok(None); + } + let idx = if base > 0 { + (height - base) as usize + } else { + height as usize + }; + Ok(self.filter_headers.get(idx).copied())
🧹 Nitpick comments (2)
dash-spv/src/storage/memory.rs (2)
114-151: Absolute-height range mapping is correct; minor cleanup optional.Logic handles base-inclusive mapping and bounds correctly. Optionally, add an early return to short-circuit empty ranges and reduce branches.
Example minimal tweak within this hunk:
@@ - let start_idx = if has_base { + // Fast‑path: entire range ends before base => empty + if has_base && range.end <= base { + return Ok(Vec::new()); + } + let start_idx = if has_base { if range.start < base { 0usize } else { (range.start - base) as usize } } else { range.start as usize };
196-237: Filter header range mapping mirrors header logic.Semantics are consistent. Consider extracting a small helper to dedupe range-to-index mapping used here and in load_headers.
Helper idea (outside this hunk):
fn map_abs_range_to_indices(base: u32, len: usize, range: Range<u32>) -> (usize, usize) { if base > 0 { let start_idx = if range.start < base { 0 } else { (range.start - base) as usize }; let end_abs = range.end.min(base + len as u32); let end_idx = if end_abs <= base { 0 } else { (end_abs - base) as usize }; (start_idx.min(len), end_idx.min(len)) } else { (range.start as usize, range.end.min(len as u32) as usize) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
dash-spv/src/client/message_handler.rs(3 hunks)dash-spv/src/storage/memory.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- dash-spv/src/client/message_handler.rs
🧰 Additional context used
📓 Path-based instructions (4)
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/storage/memory.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Provide both
MemoryStorageManagerandDiskStorageManagerbehind theStorageManagertrait
Files:
dash-spv/src/storage/memory.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv/src/storage/memory.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv/src/storage/memory.rs
🧬 Code graph analysis (1)
dash-spv/src/storage/memory.rs (2)
dash-spv/src/storage/disk.rs (3)
new(210-324)get_header(1211-1273)get_tip_height(1275-1287)dash-spv/src/storage/mod.rs (3)
get_header(31-31)get_header(113-113)get_tip_height(116-116)
⏰ 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). (19)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: Core Components Tests
- GitHub Check: SPV Components Tests
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_amount)
🔇 Additional comments (2)
dash-spv/src/storage/memory.rs (2)
177-187: Tip height calculation matches absolute semantics.Returns None when empty; otherwise base + len - 1. Good.
405-425: Batch retrieval correctly skips pre-base heights.This addresses the earlier “skip pre-base” issue; logic now continues when abs_h < base.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
dash-spv/src/sync/filters.rs(20 hunks)dash-spv/src/sync/masternodes.rs(3 hunks)dash-spv/src/sync/sequential/mod.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/sync/filters.rsdash-spv/src/sync/masternodes.rsdash-spv/src/sync/sequential/mod.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv/src/sync/filters.rsdash-spv/src/sync/masternodes.rsdash-spv/src/sync/sequential/mod.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv/src/sync/filters.rsdash-spv/src/sync/masternodes.rsdash-spv/src/sync/sequential/mod.rs
🧠 Learnings (1)
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/src/error.rs : Define and maintain domain-specific error types in `src/error.rs` (NetworkError, StorageError, SyncError, ValidationError, SpvError) and update them when adding new failure modes
Applied to files:
dash-spv/src/sync/sequential/mod.rs
🧬 Code graph analysis (1)
dash-spv/src/sync/sequential/mod.rs (1)
dash-spv/src/client/mod.rs (3)
storage(2400-2402)network(432-434)network(1297-1300)
⏰ 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). (19)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Core Components Tests
- GitHub Check: SPV Components Tests
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dash-spv/src/storage/memory.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Enforce Rust formatting viacargo fmt --checkon all Rust source files
All code must be clippy-clean: runcargo clippy --all-targets --all-features -- -D warnings
Files:
dash-spv/src/storage/memory.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Provide both
MemoryStorageManagerandDiskStorageManagerbehind theStorageManagertrait
Files:
dash-spv/src/storage/memory.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
dash-spv/src/storage/memory.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
dash-spv/src/storage/memory.rs
🧬 Code graph analysis (1)
dash-spv/src/storage/memory.rs (2)
dash-spv/src/storage/disk.rs (3)
new(210-324)get_header(1211-1273)get_tip_height(1275-1287)dash-spv/src/storage/mod.rs (3)
get_header(31-31)get_header(113-113)get_tip_height(116-116)
StorageManagerto accept blockchain heights directly, eliminating the need for manual conversions.Summary by CodeRabbit
New Features
Bug Fixes
Refactor