Remove headers from chain state #292
Conversation
WalkthroughMoves header ownership to storage-first access: removes in-memory per-height headers and related ChainState APIs, changes StorageManager get_tip_height to return Option and adds start/stored-length accessors, and updates sync/client/chainlock flows to query storage asynchronously with simplified error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client / SyncManager
participant HdrSync as HeaderSyncManager
participant Storage as StorageLayer
participant Net as Network/Peer
rect rgb(220,255,220)
note left of Client: New storage-first async flow
Client->>Storage: get_tip_height() -> Option<u32>
Storage-->>Client: Option<u32>
Client->>Storage: get_header(height) -> Option<Header>
Storage-->>Client: Option<Header>
Client->>HdrSync: prepare/start sync (passes storage)
HdrSync->>Net: request headers(base_hash)
Net-->>HdrSync: headers message
HdrSync->>Storage: store_headers(...)
Storage-->>HdrSync: ack
end
rect rgb(255,240,200)
note right of Client: Error handling simplified — storage errors often treated as None/defaults (unwrap_or / Option paths)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
ZocoLini
force-pushed
the
remove-headers-from-chain-state
branch
2 times, most recently
from
fb75d92 to
12c9304
Compare
ZocoLini
force-pushed
the
remove-headers-from-chain-state
branch
from
12c9304 to
bc544a5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
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/segments.rs (1)
230-233: Consider clearingstart_heightinclear_in_memory.The
clear_in_memorymethod clearstip_heightbut doesn't clear the newly addedstart_heightfield. This could leave stale data if the cache is cleared and reused.🔎 Proposed fix
pub fn clear_in_memory(&mut self) { self.segments.clear(); self.tip_height = None; + self.start_height = None; }
🧹 Nitpick comments (6)
dash-spv/src/storage/mod.rs (2)
84-88: Approve signature changes; consider adding documentation for new methods.The change to
get_tip_height()returningOption<u32>directly aligns with the PR objective that this operation cannot fail once storage is built. The newget_start_height()andget_stored_headers_len()methods are useful additions for the storage-first architecture.Consider adding doc comments for the new methods to match the documentation style of other trait methods (e.g.,
/// Get the starting height of stored headers.).🔎 Suggested documentation
/// Get the current tip blockchain height. async fn get_tip_height(&self) -> Option<u32>; + /// Get the starting height of stored headers. async fn get_start_height(&self) -> Option<u32>; + /// Get the count of stored headers. async fn get_stored_headers_len(&self) -> u32;
100-100: Consider consistency withget_tip_heightsignature.
get_filter_tip_heightstill returnsStorageResult<Option<u32>>whileget_tip_heightwas changed toOption<u32>. If filter tip height access has similar in-memory characteristics after initialization, consider aligning the signatures for consistency. Otherwise, a brief doc comment explaining why this method can fail would be helpful.dash-spv/tests/storage_consistency_test.rs (1)
409-409: Minor inconsistency:.unwrap()differs from pattern used elsewhere.This line uses
.unwrap()while other parts of this test file useif let Some(...). Since headers were stored before this line, the unwrap is safe, but it's slightly inconsistent with the defensive pattern used throughout the rest of this test file.🔎 Optional: Align with the file's defensive pattern
- let tip_height = storage.get_tip_height().await.unwrap(); + let tip_height = storage.get_tip_height().await.expect("tip should exist after storing headers");Or use the
if let Some(...)pattern consistent with other tests in this file.dash-spv/tests/header_sync_test.rs (1)
341-341: Consider usingexpect()with a descriptive message.While
.unwrap()is acceptable in tests, using.expect("Headers were stored, tip should exist")would provide clearer diagnostics if this ever fails.🔎 Suggested improvement
- let tip_height = storage.get_tip_height().await.unwrap(); + let tip_height = storage.get_tip_height().await.expect("Headers were stored, tip should exist");dash-spv/src/sync/headers/manager.rs (2)
98-112: Consider propagating storage errors.Similar to the issue in
manager.rs, this method changed from returningSyncResult<u32>to returning(). Ifstorage.load_chain_state()fails (theErrcase), the error is silently ignored and the method continues with the existing (potentially stale) chain state.This could lead to sync proceeding with incorrect state if storage is corrupted or unavailable.
Consider at minimum adding a warning log when storage loading fails:
🔎 Proposed improvement
pub async fn load_headers_from_storage(&mut self, storage: &S) { // First, try to load the persisted chain state which may contain sync_base_height if let Ok(Some(stored_chain_state)) = storage.load_chain_state().await { tracing::info!( "Loaded chain state from storage with sync_base_height: {}", stored_chain_state.sync_base_height, ); // Update our chain state with the loaded one { self.cached_sync_base_height = stored_chain_state.sync_base_height; let mut cs = self.chain_state.write().await; *cs = stored_chain_state; } + } else { + tracing::warn!("Failed to load chain state from storage or no state found"); } }
657-665: Consider extracting repeated checkpoint header retrieval.This code segment (lines 657-665) is nearly identical to lines 497-505, retrieving the checkpoint header unconditionally. This duplication:
- Makes the code harder to maintain
- Repeats the same unconditional retrieval issue noted earlier
🔎 Proposed refactor
Consider extracting this into a helper method:
async fn get_checkpoint_header(&self, storage: &S) -> SyncResult<Option<BlockHeader>> { if !self.is_synced_from_checkpoint() { return Ok(None); } let first_height = storage .get_start_height() .await .ok_or(SyncError::Storage("Failed to get start height".to_string()))?; let checkpoint_header = storage .get_header(first_height) .await .map_err(|e| SyncError::Storage(format!("Failed to get checkpoint header: {}", e)))?; Ok(checkpoint_header) }This would centralize the logic and make the conditional behavior explicit.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
dash-spv-ffi/src/types.rs(0 hunks)dash-spv-ffi/tests/unit/test_type_conversions.rs(0 hunks)dash-spv/src/chain/chainlock_manager.rs(1 hunks)dash-spv/src/client/core.rs(1 hunks)dash-spv/src/client/lifecycle.rs(6 hunks)dash-spv/src/client/progress.rs(1 hunks)dash-spv/src/client/status_display.rs(1 hunks)dash-spv/src/client/sync_coordinator.rs(3 hunks)dash-spv/src/storage/mod.rs(1 hunks)dash-spv/src/storage/segments.rs(5 hunks)dash-spv/src/storage/state.rs(2 hunks)dash-spv/src/sync/filters/headers.rs(4 hunks)dash-spv/src/sync/filters/retry.rs(1 hunks)dash-spv/src/sync/headers/manager.rs(20 hunks)dash-spv/src/sync/manager.rs(5 hunks)dash-spv/src/sync/masternodes/manager.rs(7 hunks)dash-spv/src/sync/message_handlers.rs(1 hunks)dash-spv/src/sync/phase_execution.rs(1 hunks)dash-spv/src/sync/transitions.rs(3 hunks)dash-spv/src/types.rs(1 hunks)dash-spv/tests/edge_case_filter_sync_test.rs(1 hunks)dash-spv/tests/filter_header_verification_test.rs(2 hunks)dash-spv/tests/header_sync_test.rs(10 hunks)dash-spv/tests/integration_real_node_test.rs(1 hunks)dash-spv/tests/rollback_test.rs(2 hunks)dash-spv/tests/segmented_storage_debug.rs(1 hunks)dash-spv/tests/segmented_storage_test.rs(6 hunks)dash-spv/tests/simple_header_test.rs(1 hunks)dash-spv/tests/simple_segmented_test.rs(2 hunks)dash-spv/tests/storage_consistency_test.rs(15 hunks)dash-spv/tests/storage_test.rs(1 hunks)
💤 Files with no reviewable changes (2)
- dash-spv-ffi/src/types.rs
- dash-spv-ffi/tests/unit/test_type_conversions.rs
🧰 Additional context used
📓 Path-based instructions (7)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types with thiserror crate and propagate errors appropriately in Rust code
Use tokio runtime for async operations in Rust code
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format Rust code with cargo fmt before committing
Ensure clippy passes with all warnings as errors on all Rust code
**/*.rs: Each crate keeps sources insrc/; unit tests live alongside code with#[cfg(test)]
Maintain MSRV of 1.89 for Rust code
Format Rust code withrustfmt(seerustfmt.toml); runcargo fmt --allbefore commits
Lint Rust code withclippy; avoidunwrap()/expect()in library code; use error types (e.g.,thiserror)
Usesnake_casefor function and variable names in Rust
UseUpperCamelCasefor types and traits in Rust
UseSCREAMING_SNAKE_CASEfor constants in Rust
Follow mixed editions (2021/2024) and crate-specific idioms; prefer async viatokiowhere applicable
Unit tests should be placed near code with descriptive names (e.g.,test_parse_address_mainnet)
Use#[ignore]for network-dependent or long-running tests; run with-- --ignoredflag
Never commit secrets or real keys; avoid logging sensitive data in Rust code
Keep test vectors deterministic in Rust test code
Files:
dash-spv/src/chain/chainlock_manager.rsdash-spv/src/sync/transitions.rsdash-spv/src/storage/mod.rsdash-spv/src/client/core.rsdash-spv/src/sync/message_handlers.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/simple_segmented_test.rsdash-spv/src/client/status_display.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/progress.rsdash-spv/src/sync/filters/retry.rsdash-spv/tests/simple_header_test.rsdash-spv/src/sync/phase_execution.rsdash-spv/tests/segmented_storage_debug.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/segmented_storage_test.rsdash-spv/src/storage/state.rsdash-spv/src/storage/segments.rsdash-spv/src/sync/filters/headers.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/sync/manager.rsdash-spv/src/client/sync_coordinator.rsdash-spv/tests/rollback_test.rsdash-spv/tests/header_sync_test.rsdash-spv/tests/storage_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/sync/masternodes/manager.rsdash-spv/src/types.rsdash-spv/src/sync/headers/manager.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Use async/await throughout the codebase, built on tokio runtime
Use Arc for trait objects to enable runtime polymorphism for NetworkManager and StorageManager
Use Tokio channels for inter-component message passing between async tasks
Maintain minimum Rust version (MSRV) of 1.89 and use only compatible syntax and features
Files:
dash-spv/src/chain/chainlock_manager.rsdash-spv/src/sync/transitions.rsdash-spv/src/storage/mod.rsdash-spv/src/client/core.rsdash-spv/src/sync/message_handlers.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/simple_segmented_test.rsdash-spv/src/client/status_display.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/progress.rsdash-spv/src/sync/filters/retry.rsdash-spv/tests/simple_header_test.rsdash-spv/src/sync/phase_execution.rsdash-spv/tests/segmented_storage_debug.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/segmented_storage_test.rsdash-spv/src/storage/state.rsdash-spv/src/storage/segments.rsdash-spv/src/sync/filters/headers.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/sync/manager.rsdash-spv/src/client/sync_coordinator.rsdash-spv/tests/rollback_test.rsdash-spv/tests/header_sync_test.rsdash-spv/tests/storage_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/sync/masternodes/manager.rsdash-spv/src/types.rsdash-spv/src/sync/headers/manager.rs
dash-spv/src/sync/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Files:
dash-spv/src/sync/transitions.rsdash-spv/src/sync/message_handlers.rsdash-spv/src/sync/filters/retry.rsdash-spv/src/sync/phase_execution.rsdash-spv/src/sync/filters/headers.rsdash-spv/src/sync/manager.rsdash-spv/src/sync/masternodes/manager.rsdash-spv/src/sync/headers/manager.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Store headers in 10,000-header segments with index files in headers/ directory, filter headers and compact block filters in filters/ directory, and state in state/ directory
Files:
dash-spv/src/storage/mod.rsdash-spv/src/storage/state.rsdash-spv/src/storage/segments.rs
dash-spv/src/client/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Use the DashSpvClient high-level API with proper configuration via ClientConfig for client initialization
Files:
dash-spv/src/client/core.rsdash-spv/src/client/status_display.rsdash-spv/src/client/progress.rsdash-spv/src/client/sync_coordinator.rsdash-spv/src/client/lifecycle.rs
**/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/tests/**/*.rs: Write integration tests for network operations in Rust
Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate in RustIntegration tests use
tests/directory (e.g.,rpc-integration-test)
Files:
dash-spv/tests/integration_real_node_test.rsdash-spv/tests/simple_segmented_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/simple_header_test.rsdash-spv/tests/segmented_storage_debug.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/segmented_storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/rollback_test.rsdash-spv/tests/header_sync_test.rsdash-spv/tests/storage_test.rs
dash-spv/tests/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/tests/**/*.rs: Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks
Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite
Files:
dash-spv/tests/integration_real_node_test.rsdash-spv/tests/simple_segmented_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/simple_header_test.rsdash-spv/tests/segmented_storage_debug.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/segmented_storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/rollback_test.rsdash-spv/tests/header_sync_test.rsdash-spv/tests/storage_test.rs
🧠 Learnings (24)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Store headers in 10,000-header segments with index files in headers/ directory, filter headers and compact block filters in filters/ directory, and state in state/ directory
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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.
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Prepare support for future Dash upgrades including Schnorr/Taproot support, descriptor wallets, multi-signature account types, and Lightning Network payment channels
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:01:00.652Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : Enable mempool tracking on SPV connection using the appropriate strategy (.fetchAll, .bloomFilter, .selective)
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: Applies to swift-dash-core-sdk/**/*.swift : Use actors for state management in SPVClient to ensure thread safety
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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.
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase, built on tokio runtime
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Applied to files:
dash-spv/src/chain/chainlock_manager.rsdash-spv/src/sync/transitions.rsdash-spv/src/storage/mod.rsdash-spv/src/client/core.rsdash-spv/src/sync/message_handlers.rsdash-spv/src/client/progress.rsdash-spv/src/sync/filters/retry.rsdash-spv/src/sync/phase_execution.rsdash-spv/src/storage/state.rsdash-spv/src/sync/filters/headers.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/sync/manager.rsdash-spv/src/client/sync_coordinator.rsdash-spv/tests/header_sync_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/sync/masternodes/manager.rsdash-spv/src/types.rsdash-spv/src/sync/headers/manager.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function
Applied to files:
dash-spv/src/chain/chainlock_manager.rsdash-spv/src/sync/transitions.rsdash-spv/src/storage/mod.rsdash-spv/src/client/core.rsdash-spv/src/sync/message_handlers.rsdash-spv/src/client/status_display.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/progress.rsdash-spv/src/sync/filters/retry.rsdash-spv/tests/segmented_storage_debug.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/filters/headers.rsdash-spv/src/client/sync_coordinator.rsdash-spv/tests/rollback_test.rsdash-spv/tests/storage_test.rsdash-spv/src/client/lifecycle.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/validation/**/*.rs : Implement three validation modes: ValidationMode::None (no validation), ValidationMode::Basic (structure and timestamp validation), and ValidationMode::Full (complete PoW and chain validation)
Applied to files:
dash-spv/src/chain/chainlock_manager.rsdash-spv/tests/simple_segmented_test.rsdash-spv/src/client/status_display.rsdash-spv/tests/simple_header_test.rsdash-spv/tests/rollback_test.rsdash-spv/tests/header_sync_test.rsdash-spv/src/types.rs
📚 Learning: 2025-06-26T15:54:02.509Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/chain/chainlock_manager.rsdash-spv/src/storage/mod.rsdash-spv/src/client/core.rsdash-spv/src/sync/message_handlers.rsdash-spv/src/sync/phase_execution.rsdash-spv/tests/segmented_storage_debug.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/segmented_storage_test.rsdash-spv/src/storage/state.rsdash-spv/src/sync/filters/headers.rsdash-spv/src/sync/manager.rsdash-spv/src/client/sync_coordinator.rsdash-spv/tests/storage_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/sync/masternodes/manager.rsdash-spv/src/sync/headers/manager.rs
📚 Learning: 2025-06-26T16:02:42.390Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:02:42.390Z
Learning: Passing exclusive mutable references to network and storage managers into the sync manager in async Rust can lead to borrow checker issues or runtime contention if concurrent access is needed elsewhere. It's advisable to document this architectural tradeoff and consider refactoring to interior mutability or message passing for shared access as the codebase evolves.
Applied to files:
dash-spv/src/chain/chainlock_manager.rsdash-spv/src/sync/manager.rsdash-spv/src/sync/masternodes/manager.rsdash-spv/src/sync/headers/manager.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase, built on tokio runtime
Applied to files:
dash-spv/src/chain/chainlock_manager.rsdash-spv/src/sync/transitions.rsdash-spv/src/storage/mod.rsdash-spv/src/client/core.rsdash-spv/src/sync/message_handlers.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/simple_segmented_test.rsdash-spv/src/client/status_display.rsdash-spv/src/client/progress.rsdash-spv/src/sync/filters/retry.rsdash-spv/tests/simple_header_test.rsdash-spv/tests/segmented_storage_debug.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/filters/headers.rsdash-spv/src/sync/manager.rsdash-spv/src/client/sync_coordinator.rsdash-spv/tests/rollback_test.rsdash-spv/tests/header_sync_test.rsdash-spv/tests/storage_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/sync/masternodes/manager.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/error.rs : Use domain-specific error types (NetworkError, StorageError, SyncError, ValidationError, SpvError) rather than generic error handling
Applied to files:
dash-spv/src/chain/chainlock_manager.rsdash-spv/src/sync/transitions.rsdash-spv/src/storage/mod.rsdash-spv/src/client/core.rsdash-spv/src/sync/message_handlers.rsdash-spv/tests/integration_real_node_test.rsdash-spv/src/client/status_display.rsdash-spv/src/client/progress.rsdash-spv/src/sync/filters/retry.rsdash-spv/tests/segmented_storage_debug.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/filters/headers.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/sync/manager.rsdash-spv/src/client/sync_coordinator.rsdash-spv/tests/rollback_test.rsdash-spv/src/client/lifecycle.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Store headers in 10,000-header segments with index files in headers/ directory, filter headers and compact block filters in filters/ directory, and state in state/ directory
Applied to files:
dash-spv/src/sync/transitions.rsdash-spv/src/storage/mod.rsdash-spv/src/client/core.rsdash-spv/src/sync/message_handlers.rsdash-spv/tests/simple_segmented_test.rsdash-spv/src/client/status_display.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/progress.rsdash-spv/src/sync/filters/retry.rsdash-spv/src/sync/phase_execution.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/segmented_storage_test.rsdash-spv/src/storage/state.rsdash-spv/src/storage/segments.rsdash-spv/src/sync/filters/headers.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/sync/manager.rsdash-spv/src/client/sync_coordinator.rsdash-spv/tests/rollback_test.rsdash-spv/tests/header_sync_test.rsdash-spv/src/client/lifecycle.rsdash-spv/src/types.rsdash-spv/src/sync/headers/manager.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/network/**/*.rs : Implement configurable timeouts with recovery mechanisms for network operations
Applied to files:
dash-spv/src/sync/transitions.rsdash-spv/src/client/progress.rsdash-spv/src/sync/filters/retry.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/network/**/*.rs : Use DNS-first peer discovery with automatic DNS seeds (dnsseed.dash.org, testnet-seed.dashdot.io) when no explicit peers are configured; implement immediate startup with 10-second delay only for subsequent peer searches
Applied to files:
dash-spv/src/sync/transitions.rsdash-spv/src/sync/masternodes/manager.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Use Arc<dyn TraitName> for trait objects to enable runtime polymorphism for NetworkManager and StorageManager
Applied to files:
dash-spv/src/storage/mod.rsdash-spv/src/client/core.rsdash-spv/tests/integration_real_node_test.rsdash-spv/src/client/status_display.rsdash-spv/tests/segmented_storage_debug.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/manager.rsdash-spv/src/client/lifecycle.rsdash-spv/src/sync/masternodes/manager.rs
📚 Learning: 2025-12-01T08:00:50.618Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: Applies to swift-dash-core-sdk/**/*.rs : Implement new FFI functions in Rust with `#[no_mangle] extern "C"` annotation in dash-spv-ffi
Applied to files:
dash-spv/src/storage/mod.rsdash-spv/tests/header_sync_test.rsdash-spv/src/types.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Maintain minimum Rust version (MSRV) of 1.89 and use only compatible syntax and features
Applied to files:
dash-spv/src/storage/mod.rsdash-spv/src/client/core.rsdash-spv/src/sync/message_handlers.rsdash-spv/src/client/status_display.rsdash-spv/src/sync/manager.rsdash-spv/tests/rollback_test.rsdash-spv/src/sync/masternodes/manager.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/client/**/*.rs : Use the DashSpvClient high-level API with proper configuration via ClientConfig for client initialization
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/client/status_display.rsdash-spv/src/client/progress.rsdash-spv/src/client/sync_coordinator.rsdash-spv/src/client/lifecycle.rsdash-spv/src/sync/headers/manager.rs
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/core.rsdash-spv/src/storage/state.rsdash-spv/src/client/lifecycle.rsdash-spv/src/types.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite
Applied to files:
dash-spv/tests/integration_real_node_test.rsdash-spv/tests/simple_segmented_test.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/segmented_storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/rollback_test.rsdash-spv/tests/header_sync_test.rsdash-spv/tests/storage_test.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks
Applied to files:
dash-spv/tests/integration_real_node_test.rsdash-spv/tests/simple_segmented_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/simple_header_test.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/rollback_test.rsdash-spv/tests/header_sync_test.rsdash-spv/tests/storage_test.rs
📚 Learning: 2025-12-01T08:00:50.618Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: When making changes to Rust FFI in dash-spv-ffi, rebuild with `cargo build --release` and run Swift tests to verify integration
Applied to files:
dash-spv/tests/integration_real_node_test.rsdash-spv/tests/simple_header_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/rollback_test.rsdash-spv/tests/header_sync_test.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Applied to files:
dash-spv/tests/simple_segmented_test.rsdash-spv/tests/simple_header_test.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/rollback_test.rsdash-spv/tests/header_sync_test.rs
📚 Learning: 2025-06-26T15:52:45.327Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/tests/simple_segmented_test.rsdash-spv/tests/segmented_storage_debug.rsdash-spv/tests/storage_consistency_test.rsdash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/filters/headers.rsdash-spv/tests/rollback_test.rsdash-spv/tests/header_sync_test.rsdash-spv/src/sync/headers/manager.rs
📚 Learning: 2025-12-01T08:00:50.618Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: Run `./sync-headers.sh` to copy updated headers to Swift SDK after rebuilding dash-spv-ffi
Applied to files:
dash-spv/src/sync/filters/headers.rsdash-spv/tests/header_sync_test.rsdash-spv/src/sync/headers/manager.rs
📚 Learning: 2025-12-01T08:01:00.652Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:01:00.652Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : Use DashSDK's public methods, not direct SPVClient access, when accessing SPV functionality in Swift code
Applied to files:
dash-spv/src/client/sync_coordinator.rsdash-spv/src/client/lifecycle.rsdash-spv/src/types.rs
📚 Learning: 2025-02-25T06:19:32.230Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 51
File: dash/src/sml/masternode_list_entry/hash.rs:7-12
Timestamp: 2025-02-25T06:19:32.230Z
Learning: The `consensus_encode` method on `MasternodeListEntry` writing to a `Vec` buffer cannot fail, so using `.expect()` is appropriate rather than propagating the error with the `?` operator.
Applied to files:
dash-spv/src/sync/masternodes/manager.rs
🧬 Code graph analysis (23)
dash-spv/src/chain/chainlock_manager.rs (2)
dash-spv/src/types.rs (1)
header(67-69)dash-spv/src/client/core.rs (1)
storage(168-170)
dash-spv/src/storage/mod.rs (1)
dash-spv/src/storage/state.rs (3)
get_tip_height(400-402)get_start_height(404-406)get_stored_headers_len(408-423)
dash-spv/src/client/core.rs (2)
dash-spv/src/storage/segments.rs (1)
tip_height(409-411)dash-spv/src/types.rs (1)
header(67-69)
dash-spv/src/sync/message_handlers.rs (1)
dash-spv/src/client/core.rs (1)
storage(168-170)
dash-spv/tests/integration_real_node_test.rs (3)
dash-spv/src/client/core.rs (1)
storage(168-170)dash-spv/src/storage/mod.rs (1)
get_tip_height(84-84)dash-spv/src/storage/state.rs (1)
get_tip_height(400-402)
dash-spv/tests/simple_segmented_test.rs (1)
dash-spv/src/client/core.rs (1)
storage(168-170)
dash-spv/src/client/status_display.rs (1)
dash-spv/src/client/core.rs (1)
storage(168-170)
dash-spv/tests/filter_header_verification_test.rs (2)
dash-spv/src/storage/segments.rs (1)
tip_height(409-411)dash-spv/src/client/core.rs (2)
tip_height(201-203)storage(168-170)
dash-spv/src/client/progress.rs (1)
dash-spv/src/client/core.rs (1)
storage(168-170)
dash-spv/src/sync/filters/retry.rs (1)
dash-spv/src/client/core.rs (1)
storage(168-170)
dash-spv/tests/simple_header_test.rs (3)
dash-spv/src/client/core.rs (1)
storage(168-170)dash-spv/src/storage/mod.rs (1)
get_tip_height(84-84)dash-spv/src/storage/state.rs (1)
get_tip_height(400-402)
dash-spv/src/sync/phase_execution.rs (1)
dash-spv/src/client/core.rs (2)
network(163-165)storage(168-170)
dash-spv/tests/segmented_storage_debug.rs (1)
dash-spv/src/client/core.rs (1)
storage(168-170)
dash-spv/tests/storage_consistency_test.rs (1)
dash-spv/src/client/core.rs (1)
storage(168-170)
dash-spv/tests/segmented_storage_test.rs (2)
dash-spv/src/storage/mod.rs (1)
get_tip_height(84-84)dash-spv/src/storage/state.rs (1)
get_tip_height(400-402)
dash-spv/src/sync/filters/headers.rs (1)
dash-spv/src/client/core.rs (1)
storage(168-170)
dash-spv/tests/edge_case_filter_sync_test.rs (2)
dash-spv/src/storage/segments.rs (1)
tip_height(409-411)dash-spv/src/client/core.rs (2)
tip_height(201-203)storage(168-170)
dash-spv/src/client/sync_coordinator.rs (1)
dash-spv/src/client/core.rs (1)
storage(168-170)
dash-spv/tests/header_sync_test.rs (5)
dash-spv/src/client/core.rs (2)
storage(168-170)tip_height(201-203)dash-spv/src/storage/mod.rs (1)
get_tip_height(84-84)dash-spv/src/storage/state.rs (1)
get_tip_height(400-402)dash-spv/src/storage/segments.rs (1)
tip_height(409-411)dash-spv/src/storage/headers.rs (1)
headers(21-21)
dash-spv/tests/storage_test.rs (1)
dash-spv/src/client/core.rs (1)
storage(168-170)
dash-spv/src/client/lifecycle.rs (2)
dash-spv/src/client/core.rs (2)
storage(168-170)tip_height(201-203)dash-spv/src/storage/segments.rs (1)
tip_height(409-411)
dash-spv/src/sync/masternodes/manager.rs (2)
dash-spv/src/storage/segments.rs (1)
tip_height(409-411)dash-spv/src/client/core.rs (2)
tip_height(201-203)storage(168-170)
dash-spv/src/sync/headers/manager.rs (4)
dash-spv/src/sync/manager.rs (1)
load_headers_from_storage(141-144)dash-spv/src/storage/mod.rs (2)
get_stored_headers_len(88-88)get_tip_height(84-84)dash-spv/src/storage/state.rs (2)
get_stored_headers_len(408-423)get_tip_height(400-402)dash-spv/src/storage/segments.rs (1)
tip_height(409-411)
⏰ 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 (hashes_sha1)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Core Components Tests
- GitHub Check: SPV Components Tests
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: Pre-commit (ubuntu-latest)
| let first_height = storage | ||
| .get_start_height() | ||
| .await | ||
| .ok_or(SyncError::Storage("Failed to get start height".to_string()))?; | ||
| let checkpoint_header = storage | ||
| .get_header(first_height) | ||
| .await | ||
| .map_err(|e| SyncError::Storage(format!("Failed to get first header: {}", e)))? | ||
| .ok_or(SyncError::Storage("Storage didn't return first header".to_string()))?; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the manager.rs file in headers directory
fd "manager.rs" -p "src/sync/headers/" | head -20Repository: dashpay/rust-dashcore
Length of output: 186
🏁 Script executed:
#!/bin/bash
# List the current directory to find repository
pwd
ls -la
# Search for the file in common locations
find . -name "manager.rs" -path "*headers*" 2>/dev/null | head -5
# If found, get line count and view context around lines 497-505
if [ -f ./dash-spv/src/sync/headers/manager.rs ]; then
wc -l ./dash-spv/src/sync/headers/manager.rs
echo "=== Lines 490-515 ==="
sed -n '490,515p' ./dash-spv/src/sync/headers/manager.rs
echo "=== Lines 470-510 for full function context ==="
sed -n '470,510p' ./dash-spv/src/sync/headers/manager.rs
fiRepository: dashpay/rust-dashcore
Length of output: 6155
🏁 Script executed:
#!/bin/bash
# Find StorageManager trait or get_start_height implementation
echo "=== Searching for get_start_height definition ==="
rg "fn get_start_height" dash-spv/ -A 5 -B 2
echo ""
echo "=== Searching for StorageManager trait ==="
rg "trait.*Storage" dash-spv/src/ -A 10 | head -100
echo ""
echo "=== Searching for start_height initialization/setting ==="
rg "start_height" dash-spv/src/sync/ -B 2 -A 2 | head -60Repository: dashpay/rust-dashcore
Length of output: 6288
🏁 Script executed:
#!/bin/bash
# Find where start_height is set/initialized in storage
echo "=== Searching for where start_height is set in block_headers ==="
rg "start_height\(\)" dash-spv/src/ -B 5 -A 2 | head -100
echo ""
echo "=== Searching for BlockHeaderStore or similar to see start_height field ==="
rg "struct.*Header" dash-spv/src/storage/ -A 15 | grep -A 15 "BlockHeader"
echo ""
echo "=== Check how start_height is actually used in prepare_sync ==="
sed -n '495,530p' ./dash-spv/src/sync/headers/manager.rsRepository: dashpay/rust-dashcore
Length of output: 5353
🏁 Script executed:
#!/bin/bash
# Get the full function signature and understand the flow better
echo "=== Function signature and full context ==="
sed -n '430,550p' ./dash-spv/src/sync/headers/manager.rs
echo ""
echo "=== Check BlockHeaderStore to understand start_height ==="
rg "fn start_height" dash-spv/src/ -B 3 -A 8Repository: dashpay/rust-dashcore
Length of output: 6282
🏁 Script executed:
#!/bin/bash
# Check what happens after the match statement - where else are these vars used?
echo "=== Check how checkpoint_header is used throughout the function ==="
sed -n '495,650p' ./dash-spv/src/sync/headers/manager.rs | grep -n "checkpoint_header\|first_height"
echo ""
echo "=== Search for any other unconditional uses of checkpoint_header ==="
sed -n '495,750p' ./dash-spv/src/sync/headers/manager.rsRepository: dashpay/rust-dashcore
Length of output: 12021
🏁 Script executed:
#!/bin/bash
# Check if get_start_height is guaranteed to return Some or can return None
echo "=== How is BlockHeaderStore.start_height initialized? ==="
rg "start_height\s*=" dash-spv/src/storage/ -B 3 -A 3 | head -60
echo ""
echo "=== Check if there's any guarantee that start_height is always Some ==="
rg "start_height" dash-spv/src/storage/segments.rs -B 2 -A 2Repository: dashpay/rust-dashcore
Length of output: 3430
🏁 Script executed:
#!/bin/bash
# One final check - is there any guarantee that by prepare_sync time, start_height is always Some?
echo "=== Check if get_stored_headers_len is used as a guard elsewhere ==="
rg "get_stored_headers_len" dash-spv/src/sync/headers/manager.rs -B 2 -A 2
echo ""
echo "=== Summary of start_height contract ==="
echo "Based on BlockHeaderStore initialization in segments.rs:"
echo "- start_height field is Option<u32>, initialized to None"
echo "- Only set to Some when store_items() is called"
echo "- Can return None if no headers have been stored"Repository: dashpay/rust-dashcore
Length of output: 2839
Move checkpoint header retrieval inside conditional branches where it's used.
The code unconditionally retrieves first_height and checkpoint_header at lines 497-505, but these values are only used when both is_synced_from_checkpoint() returns true AND headers exist in storage. If neither condition holds, the function errors with "Failed to get start height" despite not needing those values for the non-checkpoint sync path.
Move the retrieval into the conditional blocks that actually require it (lines 517, 568) or add explicit guards before the retrieval. This also applies to the same pattern in check_sync_timeout() around line 640.
github-actions
bot
added
the
merge-conflict
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
github-actions
bot
removed
the
merge-conflict
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
dash-spv/src/sync/manager.rs (2)
142-144: Silently discards initialization errors.The method signature changed from returning
SyncResult<u32>to(), removing error propagation. Based on the relevant code snippet, the delegatedheader_sync.load_headers_from_storage(storage)loads chain state from storage and updates internal state, but any errors during this process are now silently ignored. While this may be acceptable for initialization (the sync will proceed from defaults if load fails), consider adding explicit error logging or documenting this behavior.Run the following script to verify error handling in the delegated method:
#!/bin/bash # Description: Check error handling in HeaderSync::load_headers_from_storage # Find the implementation ast-grep --pattern 'impl $_ HeaderSyncManager$_ { $$$ pub async fn load_headers_from_storage($$$) { $$$ } $$$ }'
383-385: Document the 0 return value semantics.The method returns 0 via
unwrap_or(0)whenstorage.get_tip_height()returnsNone. While this is consistent with the codebase and reasonable for sync initialization, the method documentation should clarify that 0 represents both "no headers synced" and "at genesis height" to prevent future confusion.📝 Suggested documentation
- /// Get the actual blockchain height from storage height, accounting for checkpoints + /// Get the actual blockchain height from storage. + /// + /// Returns 0 if no tip height is present in storage (no headers synced) + /// or when the blockchain is at genesis height. This behavior is intentional + /// for initializing sync state. pub(super) async fn get_blockchain_height_from_storage(&self, storage: &S) -> u32 { storage.get_tip_height().await.unwrap_or(0) }
🧹 Nitpick comments (1)
dash-spv/src/storage/state.rs (1)
368-384: Consider simplifying the nested range check logic.The nested
if-letblocks with early returns are correct but verbose. Consider refactoring for improved readability:🔎 Proposed refactor
async fn get_header(&self, height: u32) -> StorageResult<Option<BlockHeader>> { - if let Some(tip_height) = self.get_tip_height().await { - if height > tip_height { - return Ok(None); - } - } else { + let tip_height = match self.get_tip_height().await { + Some(h) => h, + None => return Ok(None), + }; + + let start_height = match self.get_start_height().await { + Some(h) => h, + None => return Ok(None), + }; + + if height > tip_height || height < start_height { return Ok(None); } - - if let Some(start_height) = self.get_start_height().await { - if height < start_height { - return Ok(None); - } - } else { - return Ok(None); - } - + Ok(self.block_headers.write().await.get_items(height..height + 1).await?.first().copied()) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
dash-spv-ffi/src/types.rsdash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/client/core.rsdash-spv/src/storage/state.rsdash-spv/src/sync/manager.rsdash-spv/src/types.rsdash-spv/tests/segmented_storage_test.rs
💤 Files with no reviewable changes (2)
- dash-spv-ffi/src/types.rs
- dash-spv/src/types.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- dash-spv-ffi/tests/unit/test_type_conversions.rs
- dash-spv/src/client/core.rs
🧰 Additional context used
📓 Path-based instructions (8)
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Use async/await throughout the codebase, built on tokio runtime
Use Arc for trait objects to enable runtime polymorphism for NetworkManager and StorageManager
Use Tokio channels for inter-component message passing between async tasks
Maintain minimum Rust version (MSRV) of 1.89 and use only compatible syntax and features
Files:
dash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/manager.rsdash-spv/src/storage/state.rs
dash-spv/tests/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/tests/**/*.rs: Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks
Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite
Files:
dash-spv/tests/segmented_storage_test.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CLAUDE.md)
Never hardcode network parameters, addresses, or keys
Files:
dash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/manager.rsdash-spv/src/storage/state.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types (thiserror) and propagate errors appropriately
Use tokio runtime for async operations
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality
Use secure random number generation for keys
Never log or expose private keys
Code must target Rust 1.89 minimum supported version (MSRV)
Format code using cargo fmt
Pass clippy linting without warnings
**/*.rs: MSRV (Minimum Supported Rust Version) is 1.89; ensure compatibility with this version for all builds
Unit tests should live alongside code with#[cfg(test)]annotation; integration tests use thetests/directory
Usesnake_casefor function and variable names
UseUpperCamelCasefor types and traits
UseSCREAMING_SNAKE_CASEfor constants
Format code withrustfmtbefore commits; ensurecargo fmt --allis run
Runcargo clippy --workspace --all-targets -- -D warningsfor linting; avoid warnings in CI
Preferasync/awaitviatokiofor asynchronous operations
Files:
dash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/manager.rsdash-spv/src/storage/state.rs
**/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/tests/**/*.rs: Create integration tests for network operations
Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate
**/tests/**/*.rs: Use descriptive test names (e.g.,test_parse_address_mainnet)
Mark network-dependent or long-running tests with#[ignore]and run with-- --ignored
Files:
dash-spv/tests/segmented_storage_test.rs
**/{dash-network,dash-spv,key-wallet}/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use async/await for async operations in network and wallet modules
Files:
dash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/manager.rsdash-spv/src/storage/state.rs
dash-spv/src/sync/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Files:
dash-spv/src/sync/manager.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Store headers in 10,000-header segments with index files in headers/ directory, filter headers and compact block filters in filters/ directory, and state in state/ directory
Files:
dash-spv/src/storage/state.rs
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Store headers in 10,000-header segments with index files in headers/ directory, filter headers and compact block filters in filters/ directory, and state in state/ directory
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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.
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks
Applied to files:
dash-spv/tests/segmented_storage_test.rs
📚 Learning: 2025-06-26T15:54:02.509Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/tests/segmented_storage_test.rsdash-spv/src/sync/manager.rsdash-spv/src/storage/state.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase, built on tokio runtime
Applied to files:
dash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/manager.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function
Applied to files:
dash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/manager.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Applied to files:
dash-spv/tests/segmented_storage_test.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Use Arc<dyn TraitName> for trait objects to enable runtime polymorphism for NetworkManager and StorageManager
Applied to files:
dash-spv/tests/segmented_storage_test.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should gracefully handle unavailable Dash Core nodes at 127.0.0.1:9999 without failing the test suite
Applied to files:
dash-spv/tests/segmented_storage_test.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Store headers in 10,000-header segments with index files in headers/ directory, filter headers and compact block filters in filters/ directory, and state in state/ directory
Applied to files:
dash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/manager.rsdash-spv/src/storage/state.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/error.rs : Use domain-specific error types (NetworkError, StorageError, SyncError, ValidationError, SpvError) rather than generic error handling
Applied to files:
dash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/manager.rs
📚 Learning: 2025-12-22T17:59:37.849Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/{dash-network,dash-spv,key-wallet}/**/*.rs : Use async/await for async operations in network and wallet modules
Applied to files:
dash-spv/tests/segmented_storage_test.rsdash-spv/src/sync/manager.rs
📚 Learning: 2025-06-26T15:52:45.327Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/tests/segmented_storage_test.rsdash-spv/src/sync/manager.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Applied to files:
dash-spv/src/sync/manager.rsdash-spv/src/storage/state.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Maintain minimum Rust version (MSRV) of 1.89 and use only compatible syntax and features
Applied to files:
dash-spv/src/sync/manager.rs
📚 Learning: 2025-06-26T16:02:42.390Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:02:42.390Z
Learning: Passing exclusive mutable references to network and storage managers into the sync manager in async Rust can lead to borrow checker issues or runtime contention if concurrent access is needed elsewhere. It's advisable to document this architectural tradeoff and consider refactoring to interior mutability or message passing for shared access as the codebase evolves.
Applied to files:
dash-spv/src/sync/manager.rs
🧬 Code graph analysis (2)
dash-spv/tests/segmented_storage_test.rs (3)
dash-spv/src/client/core.rs (1)
storage(168-170)dash-spv/src/storage/state.rs (1)
get_tip_height(387-389)dash-spv/src/storage/mod.rs (1)
get_tip_height(84-84)
dash-spv/src/sync/manager.rs (1)
dash-spv/src/sync/headers/manager.rs (1)
load_headers_from_storage(98-112)
🔇 Additional comments (3)
dash-spv/src/storage/state.rs (1)
395-410: LGTM!The
get_stored_headers_len()implementation correctly calculates the inclusive range length usingend_height - start_height + 1. The early returns for missing heights are appropriate.dash-spv/src/sync/manager.rs (2)
255-268: LGTM!The simplified implementation correctly handles the new
get_tip_height()API that returnsOption<u32>directly. The pattern matching properly handles bothNoneandSome(height)cases.
360-361: Signature simplified to match storage-first architecture.The removal of the
headers_lenparameter aligns with the PR's goal of removing headers fromChainStateand delegating to storage-based access.
ZocoLini
force-pushed
the
remove-headers-from-chain-state
branch
from
c5679fd to
29654ab
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
dash-spv/src/sync/manager.rs (2)
142-144: Past concern remains: error handling removed.The method signature change from
SyncResult<u32>to()removes error propagation, as previously flagged. If silent failure during initialization is intentional, consider adding explicit logging within the delegatedheader_sync.load_headers_from_storage()to aid debugging.
383-384: Past concern remains: return value semantics unclear.As previously noted, the method returns
0both when storage has no tip (no headers synced) and when the blockchain is at genesis. Consider adding a doc comment to clarify this semantic choice for future maintainers.
🧹 Nitpick comments (1)
dash-spv/src/client/core.rs (1)
192-197: Consider simplifying the double-? pattern for clarity.The implementation correctly fetches the tip hash from storage, but the pattern
.ok()??on line 195 may be hard to read. Consider usingmap_error a more explicit match for clarity:- let tip_height = storage.get_tip_height().await?; - let header = storage.get_header(tip_height).await.ok()??; - - Some(header.block_hash()) + let tip_height = storage.get_tip_height().await?; + let header = storage + .get_header(tip_height) + .await + .map_err(|_| None) + .ok()??; + Some(header.block_hash())Or more explicitly:
let tip_height = storage.get_tip_height().await?; match storage.get_header(tip_height).await { Ok(Some(header)) => Some(header.block_hash()), _ => None, }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
dash-spv-ffi/src/types.rsdash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/client/core.rsdash-spv/src/storage/state.rsdash-spv/src/sync/manager.rsdash-spv/src/types.rsdash-spv/tests/segmented_storage_test.rs
💤 Files with no reviewable changes (2)
- dash-spv-ffi/src/types.rs
- dash-spv/src/types.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- dash-spv/tests/segmented_storage_test.rs
🧰 Additional context used
📓 Path-based instructions (10)
dash-spv-ffi/tests/unit/**/*.rs
📄 CodeRabbit inference engine (dash-spv-ffi/CLAUDE.md)
Add corresponding unit tests in
tests/unit/for each new FFI function
Files:
dash-spv-ffi/tests/unit/test_type_conversions.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CLAUDE.md)
Never hardcode network parameters, addresses, or keys
Files:
dash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/client/core.rsdash-spv/src/storage/state.rsdash-spv/src/sync/manager.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types (thiserror) and propagate errors appropriately
Use tokio runtime for async operations
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality
Use secure random number generation for keys
Never log or expose private keys
Code must target Rust 1.89 minimum supported version (MSRV)
Format code using cargo fmt
Pass clippy linting without warnings
**/*.rs: MSRV (Minimum Supported Rust Version) is 1.89; ensure compatibility with this version for all builds
Unit tests should live alongside code with#[cfg(test)]annotation; integration tests use thetests/directory
Usesnake_casefor function and variable names
UseUpperCamelCasefor types and traits
UseSCREAMING_SNAKE_CASEfor constants
Format code withrustfmtbefore commits; ensurecargo fmt --allis run
Runcargo clippy --workspace --all-targets -- -D warningsfor linting; avoid warnings in CI
Preferasync/awaitviatokiofor asynchronous operations
Files:
dash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/client/core.rsdash-spv/src/storage/state.rsdash-spv/src/sync/manager.rs
**/*-ffi/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*-ffi/**/*.rs: Exercise careful handling at FFI boundaries for memory safety
Be careful with FFI memory management
Files:
dash-spv-ffi/tests/unit/test_type_conversions.rs
**/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/tests/**/*.rs: Create integration tests for network operations
Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate
**/tests/**/*.rs: Use descriptive test names (e.g.,test_parse_address_mainnet)
Mark network-dependent or long-running tests with#[ignore]and run with-- --ignored
Files:
dash-spv-ffi/tests/unit/test_type_conversions.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Use async/await throughout the codebase, built on tokio runtime
Use Arc for trait objects to enable runtime polymorphism for NetworkManager and StorageManager
Use Tokio channels for inter-component message passing between async tasks
Maintain minimum Rust version (MSRV) of 1.89 and use only compatible syntax and features
Files:
dash-spv/src/client/core.rsdash-spv/src/storage/state.rsdash-spv/src/sync/manager.rs
dash-spv/src/client/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Use the DashSpvClient high-level API with proper configuration via ClientConfig for client initialization
Files:
dash-spv/src/client/core.rs
**/{dash-network,dash-spv,key-wallet}/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use async/await for async operations in network and wallet modules
Files:
dash-spv/src/client/core.rsdash-spv/src/storage/state.rsdash-spv/src/sync/manager.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Store headers in 10,000-header segments with index files in headers/ directory, filter headers and compact block filters in filters/ directory, and state in state/ directory
Files:
dash-spv/src/storage/state.rs
dash-spv/src/sync/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Files:
dash-spv/src/sync/manager.rs
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Store headers in 10,000-header segments with index files in headers/ directory, filter headers and compact block filters in filters/ directory, and state in state/ directory
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function
Applied to files:
dash-spv-ffi/tests/unit/test_type_conversions.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/tests/**/*.rs : Organize tests into unit tests (in-module), integration tests (tests/ directory), real network tests (with live Dash Core nodes), and performance benchmarks
Applied to files:
dash-spv-ffi/tests/unit/test_type_conversions.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types in FFI functions
Applied to files:
dash-spv-ffi/tests/unit/test_type_conversions.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/validation/**/*.rs : Implement three validation modes: ValidationMode::None (no validation), ValidationMode::Basic (structure and timestamp validation), and ValidationMode::Full (complete PoW and chain validation)
Applied to files:
dash-spv-ffi/tests/unit/test_type_conversions.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Input strings in FFI functions are `*const c_char` (borrowed, not freed by C caller)
Applied to files:
dash-spv-ffi/tests/unit/test_type_conversions.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : All FFI types must have corresponding `_destroy()` functions for explicit memory management
Applied to files:
dash-spv-ffi/tests/unit/test_type_conversions.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Store headers in 10,000-header segments with index files in headers/ directory, filter headers and compact block filters in filters/ directory, and state in state/ directory
Applied to files:
dash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/client/core.rsdash-spv/src/storage/state.rsdash-spv/src/sync/manager.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Rust strings must be returned as `*const c_char` with caller responsibility to free using `dash_string_free`
Applied to files:
dash-spv-ffi/tests/unit/test_type_conversions.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use `#[no_mangle] extern "C"` attribute when implementing new FFI functions in Rust
Applied to files:
dash-spv-ffi/tests/unit/test_type_conversions.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Implement sequential phase-based synchronization via SyncManager with phases progressing in order: Headers → Masternode List → Filter Headers → Filters → Blocks
Applied to files:
dash-spv-ffi/tests/unit/test_type_conversions.rsdash-spv/src/client/core.rsdash-spv/src/storage/state.rsdash-spv/src/sync/manager.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/sync/manager.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/client/**/*.rs : Use the DashSpvClient high-level API with proper configuration via ClientConfig for client initialization
Applied to files:
dash-spv/src/client/core.rs
📚 Learning: 2025-06-26T16:01:37.609Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/core.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase, built on tokio runtime
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/sync/manager.rs
📚 Learning: 2025-06-26T15:54:02.509Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/client/core.rsdash-spv/src/storage/state.rsdash-spv/src/sync/manager.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Use Arc<dyn TraitName> for trait objects to enable runtime polymorphism for NetworkManager and StorageManager
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/sync/manager.rs
📚 Learning: 2025-12-22T17:59:37.849Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-22T17:59:37.849Z
Learning: Applies to **/{dash-network,dash-spv,key-wallet}/**/*.rs : Use async/await for async operations in network and wallet modules
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/sync/manager.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/**/*.rs : Maintain minimum Rust version (MSRV) of 1.89 and use only compatible syntax and features
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/sync/manager.rs
📚 Learning: 2025-12-16T09:03:55.811Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-16T09:03:55.811Z
Learning: Applies to dash-spv/src/error.rs : Use domain-specific error types (NetworkError, StorageError, SyncError, ValidationError, SpvError) rather than generic error handling
Applied to files:
dash-spv/src/sync/manager.rs
📚 Learning: 2025-06-26T15:52:45.327Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 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/manager.rs
📚 Learning: 2025-06-26T16:02:42.390Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:02:42.390Z
Learning: Passing exclusive mutable references to network and storage managers into the sync manager in async Rust can lead to borrow checker issues or runtime contention if concurrent access is needed elsewhere. It's advisable to document this architectural tradeoff and consider refactoring to interior mutability or message passing for shared access as the codebase evolves.
Applied to files:
dash-spv/src/sync/manager.rs
🧬 Code graph analysis (2)
dash-spv/src/client/core.rs (2)
dash-spv/src/storage/segments.rs (1)
tip_height(409-411)dash-spv/src/types.rs (1)
header(67-69)
dash-spv/src/sync/manager.rs (1)
dash-spv/src/sync/headers/manager.rs (1)
load_headers_from_storage(98-112)
🔇 Additional comments (10)
dash-spv-ffi/tests/unit/test_type_conversions.rs (1)
163-185: LGTM! Test properly updated for ChainState API change.The removal of the
headersfield initialization andheader_heightassertion correctly aligns with the PR objective of moving headers to storage-first access. The test maintains its purpose of validatingNone-value conversions for the remainingChainStatefields.dash-spv/src/storage/state.rs (5)
49-72: LGTM: ChainState deserialization correctly omits headers field.The removal of header loading logic from
load_chain_statealigns with the PR objective to move headers to storage-backed access. The JSON deserialization correctly reconstructsChainStatewith all necessary fields except the removedheadersfield.
367-382: Good defensive boundary checks added.The pre-flight validation against
tip_heightandstart_heightprevents out-of-bounds header requests and provides clear semantics: returningNonewhen the requested height falls outside the stored range or when storage has no headers.
386-389: LGTM: Signature simplified per PR design.The removal of
StorageResultwrapping aligns with the PR objective that this method "cannot fail once the StorageManager is built." The directOption<u32>return simplifies caller error handling.
390-409: LGTM: New storage accessors correctly implemented.Both
get_start_heightandget_stored_headers_lenare correctly implemented:
get_start_heightprovides the storage-backed start heightget_stored_headers_lencorrectly computes the inclusive range length with the formulaend_height - start_height + 1- Early returns for
Nonecases prevent computation on incomplete storage state
640-640: Good test coverage for new API.The test correctly validates that
get_stored_headers_len()returns the expected count after storing headers at a checkpoint height.dash-spv/src/client/core.rs (1)
202-202: LGTM: Correctly defaults to 0 for missing tip height.The
unwrap_or(0)pattern is appropriate here, representing either genesis height or the absence of stored headers. This aligns with the storage-first access pattern of the PR.dash-spv/src/sync/manager.rs (3)
224-224: LGTM: Storage parameter correctly threaded through.The addition of the
storageparameter torequest_headersenables storage-backed header synchronization, consistent with the PR's storage-first architecture.
255-255: LGTM: Simplified tip height retrieval.The direct use of
Option<u32>fromstorage.get_tip_height()correctly aligns with the simplified storage API, removing unnecessary error wrapping.
360-361: LGTM: Signature simplified to remove headers_len.Removing the
headers_lenparameter aligns with moving header ownership to storage. The cache update now only needs thesync_base_height, as header length can be queried from storage when needed.
Removed the headers field from ChainState. Also modified a StorageManager trait method return type removing a the possibility of error (bcs it cannot fail once the StorageManager is built), that added a little bit of noise to this PR, more than expected.
All tests pass and I was able to execute example simple_sync reaching height 2_391_372 in 15 min (more or less).
There is only one thing, one piece of logic that I didn't replace.
In
dash-spv/src/types.rsline 303, inside methodChainState::new_for_networkwe can find this:I just dropped the push of the genesis_block without adding it to the storage expecting a future issue during sync or while executing the tests but that never happened. I guess we are sync the genesis block from the network instead of having it hardcoded there but I am not sure and make me wonder. Since it works and theorically this is somethign we can ask the network for I am not wasting time here unless someone knows something that I don't. I want to continue updating ChainState
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.