Clean/remove memory storage #275
Conversation
WalkthroughThe PR removes MemoryStorageManager entirely from the codebase and consolidates the storage system to use only DiskStorageManager as the sole storage backend. This includes restructuring the storage module to expose DiskStorageManager directly, updating all examples and tests, and adjusting internal import paths for storage utilities. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes Key areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
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 |
![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 (5)
dash-spv/CLAUDE.md (1)
144-144: Update test category description to reflect removed memory storage.Line 144 describes the storage layer tests as "Memory/disk operations (9/9 passing)", but with
MemoryStorageManagerremoved, the accurate description should reflect only disk-backed operations (or clarify that "memory" now refers to temporary disk storage viaDiskStorageManager::new_tmp()).Apply this diff to clarify the test scope:
-2. **Storage layer**: Memory/disk operations (9/9 passing) +2. **Storage layer**: Disk storage operations (9/9 passing)dash-spv/src/main.rs (2)
332-338: Fix syntax error: missing 't' in await.Line 332 has
.awaiinstead of.await, which is a syntax error that will prevent compilation.Apply this diff:
- let storage_manager = match dash_spv::storage::DiskStorageManager::new_tmp().awai { + let storage_manager = match dash_spv::storage::DiskStorageManager::new_tmp().await {
351-357: Fix syntax error: missing 't' in await.Line 351 has
.awaiinstead of.await, which is a syntax error that will prevent compilation.Apply this diff:
- let storage_manager = match dash_spv::storage::DiskStorageManager::new_tmp().awai { + let storage_manager = match dash_spv::storage::DiskStorageManager::new_tmp().await {dash-spv/tests/reverse_index_test.rs (1)
55-74: Potential test flakiness:TempDirdropped immediately.The
TempDirinstance is created inline and not stored in a variable. SinceTempDirdeletes its directory on drop, the temporary directory may be deleted immediately after.path().into()is called, before the test completes.Apply this diff to ensure the temp directory persists for the test duration:
#[tokio::test] async fn test_clear_clears_index() { + let temp_dir = TempDir::new().expect("Failed to create tmp dir"); let mut storage = - DiskStorageManager::new(TempDir::new().expect("Failed to create tmp dir").path().into()) + DiskStorageManager::new(temp_dir.path().into()) .await .expect("Failed to create tmp storage");dash-spv/tests/wallet_integration_test.rs (1)
15-33: Docstring inconsistency andTempDirlifetime issue.Two issues:
- The docstring on Line 15 says "memory storage" but the implementation uses
DiskStorageManager- Same
TempDirdrop issue - the temp directory may be deleted before the test completesApply this diff to fix both issues:
-/// Create a test SPV client with memory storage for integration testing. +/// Create a test SPV client with temporary disk storage for integration testing. async fn create_test_client( ) -> DashSpvClient<WalletManager<ManagedWalletInfo>, PeerNetworkManager, DiskStorageManager> { let config = ClientConfig::testnet().without_filters().without_masternodes(); // Create network manager let network_manager = PeerNetworkManager::new(&config).await.unwrap(); // Create storage manager + let temp_dir = TempDir::new().expect("Failed to create tmp dir"); let storage_manager = - DiskStorageManager::new(TempDir::new().expect("Failed to create tmp dir").path().into()) + DiskStorageManager::new(temp_dir.path().into()) .await .expect("Failed to create tmp storage"); + // Leak temp_dir to keep directory alive for test duration + std::mem::forget(temp_dir);
🧹 Nitpick comments (1)
dash-spv/examples/filter_sync.rs (1)
31-31: Consider using a temporary directory for the example.The hardcoded path "./.tmp/example-storage" will persist data across runs and may cause issues if the example is run multiple times or in different directories.
Consider using a temporary directory pattern similar to the tests:
let temp_dir = tempfile::TempDir::new()?; let storage_manager = DiskStorageManager::new(temp_dir.path().into()).await?;Or document that this example creates persistent storage at this location.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
dash-spv/CLAUDE.md(1 hunks)dash-spv/CODE_ANALYSIS_SUMMARY.md(1 hunks)dash-spv/examples/filter_sync.rs(2 hunks)dash-spv/examples/simple_sync.rs(2 hunks)dash-spv/src/chain/chainlock_test.rs(3 hunks)dash-spv/src/client/block_processor_test.rs(3 hunks)dash-spv/src/client/core.rs(2 hunks)dash-spv/src/client/message_handler_test.rs(1 hunks)dash-spv/src/client/mod.rs(3 hunks)dash-spv/src/lib.rs(1 hunks)dash-spv/src/main.rs(2 hunks)dash-spv/src/network/persist.rs(1 hunks)dash-spv/src/network/reputation.rs(1 hunks)dash-spv/src/storage/disk/mod.rs(0 hunks)dash-spv/src/storage/headers.rs(1 hunks)dash-spv/src/storage/manager.rs(2 hunks)dash-spv/src/storage/memory.rs(0 hunks)dash-spv/src/storage/mod.rs(2 hunks)dash-spv/src/storage/segments.rs(1 hunks)dash-spv/src/storage/state.rs(1 hunks)dash-spv/src/sync/headers/manager.rs(1 hunks)dash-spv/tests/block_download_test.rs(9 hunks)dash-spv/tests/edge_case_filter_sync_test.rs(3 hunks)dash-spv/tests/filter_header_verification_test.rs(5 hunks)dash-spv/tests/header_sync_test.rs(9 hunks)dash-spv/tests/integration_real_node_test.rs(4 hunks)dash-spv/tests/peer_test.rs(2 hunks)dash-spv/tests/reverse_index_test.rs(2 hunks)dash-spv/tests/simple_header_test.rs(2 hunks)dash-spv/tests/smart_fetch_integration_test.rs(1 hunks)dash-spv/tests/storage_test.rs(1 hunks)dash-spv/tests/wallet_integration_test.rs(1 hunks)docs/implementation-notes/CHAINLOCK_IMPLEMENTATION.md(1 hunks)docs/implementation-notes/MEMPOOL_IMPLEMENTATION_SUMMARY.md(1 hunks)
💤 Files with no reviewable changes (2)
- dash-spv/src/storage/disk/mod.rs
- dash-spv/src/storage/memory.rs
🧰 Additional context used
📓 Path-based instructions (8)
**/*.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/client/core.rsdash-spv/src/chain/chainlock_test.rsdash-spv/src/storage/headers.rsdash-spv/src/main.rsdash-spv/src/client/message_handler_test.rsdash-spv/tests/wallet_integration_test.rsdash-spv/tests/storage_test.rsdash-spv/src/sync/headers/manager.rsdash-spv/src/storage/state.rsdash-spv/examples/filter_sync.rsdash-spv/tests/smart_fetch_integration_test.rsdash-spv/src/lib.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/mod.rsdash-spv/src/network/reputation.rsdash-spv/src/storage/manager.rsdash-spv/tests/header_sync_test.rsdash-spv/src/client/block_processor_test.rsdash-spv/tests/reverse_index_test.rsdash-spv/tests/block_download_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/storage/segments.rsdash-spv/src/storage/mod.rsdash-spv/tests/simple_header_test.rsdash-spv/src/network/persist.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Use trait-based abstractions for core components likeNetworkManagerandStorageManagerto enable swappable implementations
Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles withcargo check --all-features
Files:
dash-spv/src/client/core.rsdash-spv/src/chain/chainlock_test.rsdash-spv/src/storage/headers.rsdash-spv/src/main.rsdash-spv/src/client/message_handler_test.rsdash-spv/tests/wallet_integration_test.rsdash-spv/tests/storage_test.rsdash-spv/src/sync/headers/manager.rsdash-spv/src/storage/state.rsdash-spv/examples/filter_sync.rsdash-spv/tests/smart_fetch_integration_test.rsdash-spv/src/lib.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/mod.rsdash-spv/src/network/reputation.rsdash-spv/src/storage/manager.rsdash-spv/tests/header_sync_test.rsdash-spv/src/client/block_processor_test.rsdash-spv/tests/reverse_index_test.rsdash-spv/tests/block_download_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/storage/segments.rsdash-spv/src/storage/mod.rsdash-spv/tests/simple_header_test.rsdash-spv/src/network/persist.rs
dash-spv/src/client/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Client module should provide high-level API through
DashSpvClientand configuration viaClientConfig
Files:
dash-spv/src/client/core.rsdash-spv/src/client/message_handler_test.rsdash-spv/src/client/mod.rsdash-spv/src/client/block_processor_test.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Storage module should provide abstraction via
StorageManagertrait with concrete implementations forMemoryStorageManagerandDiskStorageManager
Files:
dash-spv/src/storage/headers.rsdash-spv/src/storage/state.rsdash-spv/src/storage/manager.rsdash-spv/src/storage/segments.rsdash-spv/src/storage/mod.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/wallet_integration_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/smart_fetch_integration_test.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/header_sync_test.rsdash-spv/tests/reverse_index_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/simple_header_test.rs
dash-spv/tests/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Integration tests should be organized in
tests/directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration
Files:
dash-spv/tests/wallet_integration_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/smart_fetch_integration_test.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/header_sync_test.rsdash-spv/tests/reverse_index_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/simple_header_test.rs
dash-spv/src/sync/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Synchronization should use
SyncManagerfor phase-based coordination: Phase 1 (Headers), Phase 2 (Masternode List), Phase 3 (Filter Headers), Phase 4 (Filters), Phase 5 (Blocks)
Files:
dash-spv/src/sync/headers/manager.rs
dash-spv/src/network/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Network module should handle TCP connections, handshake management, message routing, and peer management following Dash protocol implementation
Files:
dash-spv/src/network/reputation.rsdash-spv/src/network/persist.rs
🧠 Learnings (46)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/chain/chainlock_test.rsdash-spv/src/storage/headers.rsdash-spv/src/main.rsdash-spv/src/client/message_handler_test.rsdash-spv/tests/wallet_integration_test.rsdash-spv/tests/storage_test.rsdash-spv/src/sync/headers/manager.rsdash-spv/src/storage/state.rsdash-spv/examples/filter_sync.rsdocs/implementation-notes/MEMPOOL_IMPLEMENTATION_SUMMARY.mddash-spv/tests/smart_fetch_integration_test.rsdash-spv/CLAUDE.mddash-spv/src/lib.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/mod.rsdash-spv/src/network/reputation.rsdash-spv/CODE_ANALYSIS_SUMMARY.mddash-spv/src/storage/manager.rsdocs/implementation-notes/CHAINLOCK_IMPLEMENTATION.mddash-spv/tests/header_sync_test.rsdash-spv/src/client/block_processor_test.rsdash-spv/tests/reverse_index_test.rsdash-spv/tests/block_download_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/storage/segments.rsdash-spv/src/storage/mod.rsdash-spv/tests/simple_header_test.rsdash-spv/src/network/persist.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use trait-based abstractions for core components like `NetworkManager` and `StorageManager` to enable swappable implementations
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/chain/chainlock_test.rsdash-spv/src/storage/headers.rsdash-spv/src/main.rsdash-spv/src/client/message_handler_test.rsdash-spv/tests/wallet_integration_test.rsdash-spv/tests/storage_test.rsdash-spv/src/sync/headers/manager.rsdash-spv/src/storage/state.rsdash-spv/examples/filter_sync.rsdocs/implementation-notes/MEMPOOL_IMPLEMENTATION_SUMMARY.mddash-spv/tests/smart_fetch_integration_test.rsdash-spv/CLAUDE.mddash-spv/src/lib.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/mod.rsdash-spv/src/network/reputation.rsdash-spv/CODE_ANALYSIS_SUMMARY.mddash-spv/src/storage/manager.rsdocs/implementation-notes/CHAINLOCK_IMPLEMENTATION.mddash-spv/tests/header_sync_test.rsdash-spv/src/client/block_processor_test.rsdash-spv/tests/reverse_index_test.rsdash-spv/tests/block_download_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/storage/segments.rsdash-spv/src/storage/mod.rsdash-spv/tests/simple_header_test.rsdash-spv/src/network/persist.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/chain/chainlock_test.rsdash-spv/src/storage/headers.rsdash-spv/src/main.rsdash-spv/src/client/message_handler_test.rsdash-spv/tests/wallet_integration_test.rsdash-spv/tests/storage_test.rsdash-spv/src/sync/headers/manager.rsdash-spv/src/storage/state.rsdash-spv/examples/filter_sync.rsdocs/implementation-notes/MEMPOOL_IMPLEMENTATION_SUMMARY.mddash-spv/tests/smart_fetch_integration_test.rsdash-spv/CLAUDE.mddash-spv/src/lib.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/mod.rsdash-spv/src/network/reputation.rsdash-spv/src/storage/manager.rsdocs/implementation-notes/CHAINLOCK_IMPLEMENTATION.mddash-spv/tests/header_sync_test.rsdash-spv/src/client/block_processor_test.rsdash-spv/tests/reverse_index_test.rsdash-spv/tests/block_download_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/storage/segments.rsdash-spv/src/storage/mod.rsdash-spv/tests/simple_header_test.rsdash-spv/src/network/persist.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should be organized in `tests/` directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/chain/chainlock_test.rsdash-spv/src/storage/headers.rsdash-spv/src/client/message_handler_test.rsdash-spv/tests/wallet_integration_test.rsdash-spv/tests/storage_test.rsdash-spv/examples/filter_sync.rsdash-spv/tests/smart_fetch_integration_test.rsdash-spv/CLAUDE.mddash-spv/src/lib.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/mod.rsdash-spv/CODE_ANALYSIS_SUMMARY.mddash-spv/src/storage/manager.rsdash-spv/tests/header_sync_test.rsdash-spv/src/client/block_processor_test.rsdash-spv/tests/reverse_index_test.rsdash-spv/tests/block_download_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/storage/mod.rsdash-spv/tests/simple_header_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/error.rs : Maintain domain-specific error types in `error.rs` with variants for `NetworkError`, `StorageError`, `SyncError`, `ValidationError`, and `SpvError` (top-level wrapper)
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/main.rsdash-spv/src/sync/headers/manager.rsdash-spv/examples/filter_sync.rsdash-spv/tests/smart_fetch_integration_test.rsdash-spv/src/lib.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/mod.rsdash-spv/src/network/reputation.rsdash-spv/tests/header_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/storage/segments.rsdash-spv/src/storage/mod.rsdash-spv/tests/simple_header_test.rsdash-spv/src/network/persist.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/client/**/*.rs : Client module should provide high-level API through `DashSpvClient` and configuration via `ClientConfig`
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/main.rsdash-spv/tests/wallet_integration_test.rsdash-spv/tests/smart_fetch_integration_test.rsdash-spv/src/lib.rsdash-spv/tests/integration_real_node_test.rsdash-spv/src/client/mod.rsdash-spv/tests/header_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/storage/mod.rsdash-spv/tests/simple_header_test.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/client/core.rsdash-spv/src/chain/chainlock_test.rsdash-spv/src/client/message_handler_test.rsdash-spv/src/sync/headers/manager.rsdash-spv/tests/smart_fetch_integration_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/network/reputation.rsdash-spv/tests/header_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/storage/mod.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/chain/chainlock_test.rsdash-spv/src/storage/headers.rsdash-spv/src/main.rsdash-spv/tests/wallet_integration_test.rsdash-spv/examples/filter_sync.rsdash-spv/tests/smart_fetch_integration_test.rsdash-spv/src/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/mod.rsdash-spv/src/network/reputation.rsdash-spv/src/storage/manager.rsdash-spv/tests/header_sync_test.rsdash-spv/src/client/block_processor_test.rsdash-spv/tests/reverse_index_test.rsdash-spv/tests/block_download_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/storage/segments.rsdash-spv/src/storage/mod.rsdash-spv/src/network/persist.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/wallet/**/*.rs : Wallet module should provide UTXO tracking via `Utxo` struct, balance calculation with confirmation states, and transaction processing via `TransactionProcessor`
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/main.rsdash-spv/src/client/message_handler_test.rsdash-spv/tests/wallet_integration_test.rsdocs/implementation-notes/MEMPOOL_IMPLEMENTATION_SUMMARY.mddash-spv/src/lib.rsdash-spv/src/client/mod.rsdash-spv/src/storage/manager.rsdash-spv/src/client/block_processor_test.rsdash-spv/tests/peer_test.rsdash-spv/src/storage/mod.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/network/**/*.rs : Network module should handle TCP connections, handshake management, message routing, and peer management following Dash protocol implementation
Applied to files:
dash-spv/src/client/core.rsdash-spv/examples/filter_sync.rsdash-spv/tests/smart_fetch_integration_test.rsdash-spv/CLAUDE.mddash-spv/src/lib.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/header_sync_test.rsdash-spv/src/client/block_processor_test.rsdash-spv/tests/block_download_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/simple_header_test.rs
📚 Learning: 2025-12-01T07:59:46.015Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:46.015Z
Learning: Applies to **/tests/**/*.rs : Write integration tests for network operations in Rust
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/chain/chainlock_test.rsdash-spv/tests/wallet_integration_test.rsdash-spv/tests/storage_test.rsdash-spv/src/client/mod.rsdash-spv/CODE_ANALYSIS_SUMMARY.mddash-spv/src/client/block_processor_test.rs
📚 Learning: 2025-12-01T07:59:46.015Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:46.015Z
Learning: Applies to **/tests/**/*.rs : Test both mainnet and testnet configurations
Applied to files:
dash-spv/src/client/core.rsdash-spv/src/chain/chainlock_test.rsdash-spv/tests/storage_test.rsdash-spv/src/client/mod.rsdash-spv/CODE_ANALYSIS_SUMMARY.mddash-spv/src/client/block_processor_test.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/**/SPVClient.swift : Type definitions should be in separate files within Models/ or Types/ directories, not in SPVClient.swift
Applied to files:
dash-spv/src/client/core.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/core.rsdash-spv/tests/wallet_integration_test.rsdash-spv/src/lib.rsdash-spv/tests/peer_test.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 : Enable mempool tracking on SPV connection using the appropriate strategy (.fetchAll, .bloomFilter, .selective)
Applied to files:
dash-spv/src/client/core.rsdash-spv/examples/filter_sync.rsdash-spv/tests/block_download_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/peer_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: Applies to swift-dash-core-sdk/**/*.swift : Use actors for state management in SPVClient to ensure thread safety
Applied to files:
dash-spv/src/client/core.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/validation/**/*.rs : Validation module should support configurable validation modes (`ValidationMode::None`, `ValidationMode::Basic`, `ValidationMode::Full`) with header validation, ChainLock, and InstantLock verification
Applied to files:
dash-spv/src/chain/chainlock_test.rsdash-spv/tests/integration_real_node_test.rsdocs/implementation-notes/CHAINLOCK_IMPLEMENTATION.mddash-spv/tests/header_sync_test.rsdash-spv/tests/peer_test.rsdash-spv/tests/simple_header_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/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_test.rsdash-spv/src/storage/headers.rsdash-spv/src/main.rsdash-spv/src/client/message_handler_test.rsdash-spv/tests/wallet_integration_test.rsdash-spv/src/storage/state.rsdash-spv/examples/filter_sync.rsdash-spv/tests/smart_fetch_integration_test.rsdash-spv/src/lib.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/storage/manager.rsdash-spv/tests/header_sync_test.rsdash-spv/tests/reverse_index_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/storage/segments.rsdash-spv/src/storage/mod.rsdash-spv/tests/simple_header_test.rsdash-spv/src/network/persist.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Synchronization should use `SyncManager` for phase-based coordination: Phase 1 (Headers), Phase 2 (Masternode List), Phase 3 (Filter Headers), Phase 4 (Filters), Phase 5 (Blocks)
Applied to files:
dash-spv/src/chain/chainlock_test.rsdash-spv/src/main.rsdash-spv/src/client/message_handler_test.rsdash-spv/src/sync/headers/manager.rsdash-spv/examples/filter_sync.rsdocs/implementation-notes/MEMPOOL_IMPLEMENTATION_SUMMARY.mddash-spv/tests/smart_fetch_integration_test.rsdash-spv/CLAUDE.mddash-spv/src/lib.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/mod.rsdash-spv/src/storage/manager.rsdocs/implementation-notes/CHAINLOCK_IMPLEMENTATION.mddash-spv/tests/header_sync_test.rsdash-spv/src/client/block_processor_test.rsdash-spv/tests/reverse_index_test.rsdash-spv/tests/block_download_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/storage/mod.rsdash-spv/tests/simple_header_test.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize tests by functionality: BIP32 key derivation, mnemonic generation/validation, address generation/validation, path derivation, and PSBT serialization in separate test files
Applied to files:
dash-spv/src/chain/chainlock_test.rsdash-spv/src/client/message_handler_test.rsdash-spv/tests/wallet_integration_test.rsdash-spv/tests/storage_test.rsdash-spv/src/client/mod.rsdash-spv/CODE_ANALYSIS_SUMMARY.mddash-spv/tests/reverse_index_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/src/chain/chainlock_test.rsdash-spv/src/client/message_handler_test.rsdash-spv/tests/storage_test.rsdash-spv/examples/filter_sync.rsdash-spv/tests/smart_fetch_integration_test.rsdash-spv/src/lib.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/mod.rsdash-spv/src/network/reputation.rsdash-spv/src/storage/manager.rsdash-spv/tests/header_sync_test.rsdash-spv/src/client/block_processor_test.rsdash-spv/tests/block_download_test.rsdash-spv/examples/simple_sync.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/simple_header_test.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)
Applied to files:
dash-spv/src/chain/chainlock_test.rsdash-spv/src/storage/state.rsdash-spv/CLAUDE.mddash-spv/src/lib.rsdash-spv/tests/integration_real_node_test.rsdash-spv/src/storage/manager.rsdash-spv/tests/header_sync_test.rsdash-spv/src/client/block_processor_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/src/storage/segments.rsdash-spv/src/storage/mod.rsdash-spv/tests/simple_header_test.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Applies to **/tests/**/*.rs : Integration tests use `tests/` directory (e.g., `rpc-integration-test`)
Applied to files:
dash-spv/src/chain/chainlock_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/integration_real_node_test.rsdash-spv/CODE_ANALYSIS_SUMMARY.mddash-spv/tests/reverse_index_test.rsdash-spv/tests/simple_header_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: 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/headers.rsdash-spv/src/lib.rsdash-spv/src/network/reputation.rsdash-spv/src/storage/manager.rsdash-spv/src/storage/segments.rsdash-spv/src/network/persist.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/src/storage/headers.rsdash-spv/src/storage/segments.rsdash-spv/src/network/persist.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/src/storage/headers.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles with `cargo check --all-features`
Applied to files:
dash-spv/src/storage/headers.rsdash-spv/examples/filter_sync.rsdash-spv/src/lib.rsdash-spv/examples/simple_sync.rsdash-spv/src/storage/segments.rsdash-spv/src/network/persist.rs
📚 Learning: 2025-02-25T06:13:52.858Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 51
File: dash/src/sml/masternode_list/scores_for_quorum.rs:50-73
Timestamp: 2025-02-25T06:13:52.858Z
Learning: ScoreHash is a cryptographic hash in the rust-dashcore library, and therefore does not need collision handling when used as a key in collections like BTreeMap due to the extremely low probability of collisions.
Applied to files:
dash-spv/src/storage/headers.rsdash-spv/src/storage/state.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/main.rsdash-spv/tests/wallet_integration_test.rsdash-spv/examples/filter_sync.rsdocs/implementation-notes/MEMPOOL_IMPLEMENTATION_SUMMARY.mddash-spv/tests/integration_real_node_test.rsdash-spv/src/client/mod.rsdash-spv/src/storage/manager.rsdocs/implementation-notes/CHAINLOCK_IMPLEMENTATION.mddash-spv/src/client/block_processor_test.rsdash-spv/tests/block_download_test.rsdash-spv/src/storage/mod.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/src/client/message_handler_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/smart_fetch_integration_test.rsdash-spv/tests/integration_real_node_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/client/mod.rsdash-spv/CODE_ANALYSIS_SUMMARY.mddash-spv/tests/header_sync_test.rsdash-spv/src/client/block_processor_test.rsdash-spv/tests/reverse_index_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/peer_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/simple_header_test.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Keep test vectors updated with Dash Core, monitor for new DIPs affecting wallet structure, maintain backward compatibility for serialized wallets, and update derivation paths for Platform features
Applied to files:
dash-spv/tests/wallet_integration_test.rsdash-spv/CODE_ANALYSIS_SUMMARY.md
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Cover critical parsing, networking, SPV, and wallet flows with tests; add regression tests for fixes; consider property tests with `proptest`
Applied to files:
dash-spv/tests/wallet_integration_test.rsdash-spv/src/client/mod.rsdash-spv/src/client/block_processor_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Storage should use segmented storage with headers stored in 10,000-header segments with index files, separate storage for filter headers and compact block filters
Applied to files:
dash-spv/tests/storage_test.rsdash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Implement staged gap limit management using `GapLimitStage` structure with tracking of last_used_index and used_indices HashSet to enable efficient address discovery without loading entire address chains into memory
Applied to files:
docs/implementation-notes/MEMPOOL_IMPLEMENTATION_SUMMARY.mddocs/implementation-notes/CHAINLOCK_IMPLEMENTATION.mddash-spv/tests/peer_test.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/managed_account/**/*.rs : Implement atomic state updates in ManagedAccount: when processing transactions, update address usage state, UTXO set, transaction history, and balance calculations together to maintain consistency
Applied to files:
docs/implementation-notes/MEMPOOL_IMPLEMENTATION_SUMMARY.mddocs/implementation-notes/CHAINLOCK_IMPLEMENTATION.mddash-spv/src/network/persist.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{wallet,account,managed_account}/**/*.rs : Use BTreeMap for ordered data structures (accounts, transactions) and HashMap for quick lookups (address to index mapping) to optimize data structure performance
Applied to files:
docs/implementation-notes/MEMPOOL_IMPLEMENTATION_SUMMARY.mddocs/implementation-notes/CHAINLOCK_IMPLEMENTATION.md
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/transaction_checking/**/*.rs : In SPV sync integration, use `wallet_info.check_transaction()` with appropriate `TransactionContext` and update wallet state atomically only when `is_relevant` flag is true
Applied to files:
docs/implementation-notes/MEMPOOL_IMPLEMENTATION_SUMMARY.md
📚 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/src/lib.rs
📚 Learning: 2025-06-26T15:51:03.126Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T15:51:03.126Z
Learning: In Rust network code, using unwrap() on SystemTime::now().duration_since(UNIX_EPOCH) can panic if the system clock is before UNIX_EPOCH. Use unwrap_or with a safe fallback (e.g., Duration::from_secs(0)) to prevent panics in edge cases.
Applied to files:
dash-spv/src/network/reputation.rs
📚 Learning: 2025-12-01T07:59:46.015Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:46.015Z
Learning: Applies to **/*.rs : Write unit tests for new functionality in Rust
Applied to files:
dash-spv/CODE_ANALYSIS_SUMMARY.md
📚 Learning: 2025-12-01T07:59:46.015Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:46.015Z
Learning: Applies to **/tests/**/*.rs : Use proptest for property-based testing where appropriate in Rust
Applied to files:
dash-spv/CODE_ANALYSIS_SUMMARY.md
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Applies to **/*.rs : Each crate keeps sources in `src/`; unit tests live alongside code with `#[cfg(test)]`
Applied to files:
dash-spv/CODE_ANALYSIS_SUMMARY.md
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Applies to **/*.rs : Unit tests should be placed near code with descriptive names (e.g., `test_parse_address_mainnet`)
Applied to files:
dash-spv/CODE_ANALYSIS_SUMMARY.md
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Applies to **/*.rs : Use `#[ignore]` for network-dependent or long-running tests; run with `-- --ignored` flag
Applied to files:
dash-spv/CODE_ANALYSIS_SUMMARY.md
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Use deterministic test vectors with fixed seeds and known test vectors in unit tests for key derivation, mnemonic generation, and address generation to ensure reproducibility
Applied to files:
dash-spv/tests/reverse_index_test.rs
🧬 Code graph analysis (21)
dash-spv/src/chain/chainlock_test.rs (1)
dash-spv/src/storage/manager.rs (1)
new_tmp(153-158)
dash-spv/src/storage/headers.rs (4)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/client/core.rs (1)
storage(169-171)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv/src/storage/io.rs (1)
atomic_write(23-58)
dash-spv/src/main.rs (4)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/client/core.rs (1)
storage(169-171)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv/src/storage/manager.rs (1)
new_tmp(153-158)
dash-spv/tests/wallet_integration_test.rs (1)
dash-spv/tests/integration_real_node_test.rs (1)
create_test_client(28-46)
dash-spv/src/sync/headers/manager.rs (2)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/client/core.rs (1)
storage(169-171)
dash-spv/src/storage/state.rs (2)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)
dash-spv/examples/filter_sync.rs (3)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/client/core.rs (1)
storage(169-171)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)
dash-spv/tests/smart_fetch_integration_test.rs (3)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/client/core.rs (1)
storage(169-171)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)
dash-spv/tests/integration_real_node_test.rs (2)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)
dash-spv/src/client/mod.rs (2)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/storage/manager.rs (1)
new_tmp(153-158)
dash-spv/src/network/reputation.rs (1)
dash-spv/src/storage/io.rs (1)
atomic_write(23-58)
dash-spv/src/storage/manager.rs (2)
dash-spv/src/storage/headers.rs (2)
headers(21-21)load_block_index(55-66)dash-spv/src/storage/segments.rs (1)
new(406-416)
dash-spv/tests/header_sync_test.rs (2)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)
dash-spv/src/client/block_processor_test.rs (1)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)
dash-spv/tests/reverse_index_test.rs (1)
dash-spv/src/storage/manager.rs (1)
new(64-150)
dash-spv/tests/block_download_test.rs (3)
dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv/src/sync/manager.rs (1)
filter_sync(394-396)dash-spv/src/storage/manager.rs (1)
new_tmp(153-158)
dash-spv/examples/simple_sync.rs (3)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv/src/network/persist.rs (1)
new(32-40)
dash-spv/tests/edge_case_filter_sync_test.rs (4)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/client/core.rs (1)
storage(169-171)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv/src/storage/manager.rs (1)
new_tmp(153-158)
dash-spv/src/storage/segments.rs (4)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/client/core.rs (1)
storage(169-171)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv/src/storage/io.rs (1)
atomic_write(23-58)
dash-spv/src/storage/mod.rs (3)
dash-spv/src/storage/headers.rs (1)
headers(21-21)dash-spv/src/client/sync_coordinator.rs (1)
headers(948-948)dash-spv/src/network/handshake.rs (1)
state(297-299)
dash-spv/src/network/persist.rs (4)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/client/core.rs (1)
storage(169-171)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv/src/storage/io.rs (1)
atomic_write(23-58)
⏰ 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). (18)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: Core Components Tests
- GitHub Check: Key Wallet Components Tests
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: Pre-commit (ubuntu-latest)
| #[cfg(test)] | ||
| pub async fn new_tmp() -> StorageResult<Self> { | ||
| use tempfile::TempDir; | ||
|
|
||
| let temp_dir = TempDir::new()?; | ||
| Self::new(temp_dir.path().into()).await | ||
| } |
There was a problem hiding this comment.
Bug: TempDir dropped before storage initialization completes.
The TempDir is created and immediately converted to a PathBuf, causing it to be dropped. Since TempDir deletes its directory on drop, the directory will be deleted before Self::new() can use it, leading to storage initialization failures.
Apply this diff to keep the TempDir alive. One approach is to leak the handle intentionally for tests, or return a wrapper that holds both:
#[cfg(test)]
pub async fn new_tmp() -> StorageResult<Self> {
use tempfile::TempDir;
let temp_dir = TempDir::new()?;
- Self::new(temp_dir.path().into()).await
+ let path = temp_dir.path().to_path_buf();
+ // Keep temp_dir alive by leaking it - the OS will clean up on process exit
+ // This is acceptable for tests where we want the directory to persist
+ std::mem::forget(temp_dir);
+ Self::new(path).await
}Alternatively, consider returning a tuple or wrapper struct that holds both the DiskStorageManager and TempDir to ensure proper cleanup after test completion.
🤖 Prompt for AI Agents
In dash-spv/src/storage/manager.rs around lines 152 to 158, the TempDir is
created and only its PathBuf is passed into Self::new(), causing TempDir to be
dropped (and the directory removed) before storage initialization completes;
keep the TempDir alive for the lifetime of the storage by either: 1) returning a
tuple or small wrapper struct (e.g., (DiskStorageManager, TempDir) or
DiskStorageWithTempDir) from new_tmp so the TempDir is owned by the test and
cleaned up afterwards, or 2) intentionally leaking the TempDir in tests if
cleanup is unnecessary (e.g., Box::leak) — implement the chosen approach so the
TempDir is not dropped until after tests finish and adjust callers accordingly.
| let storage = | ||
| DiskStorageManager::new(TempDir::new().expect("Failed to create tmp dir").path().into()) | ||
| .await | ||
| .expect("Failed to create tmp storage"); |
There was a problem hiding this comment.
Potential TempDir lifetime issue.
The TempDir is created inline and its path is passed to DiskStorageManager::new(). Since TempDir implements Drop to clean up the directory when dropped, and the temporary value isn't bound to a variable, it may be dropped immediately after this expression, potentially removing the directory while the storage manager is still using it.
To fix this, bind the TempDir to a variable that lives for the duration of the test:
- let storage =
- DiskStorageManager::new(TempDir::new().expect("Failed to create tmp dir").path().into())
- .await
- .expect("Failed to create tmp storage");
+ let temp_dir = TempDir::new().expect("Failed to create tmp dir");
+ let storage = DiskStorageManager::new(temp_dir.path().into())
+ .await
+ .expect("Failed to create tmp storage");This ensures the temporary directory remains available throughout the test execution.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let storage = | |
| DiskStorageManager::new(TempDir::new().expect("Failed to create tmp dir").path().into()) | |
| .await | |
| .expect("Failed to create tmp storage"); | |
| let temp_dir = TempDir::new().expect("Failed to create tmp dir"); | |
| let storage = DiskStorageManager::new(temp_dir.path().into()) | |
| .await | |
| .expect("Failed to create tmp storage"); |
🤖 Prompt for AI Agents
In dash-spv/tests/simple_header_test.rs around lines 54 to 57, the TempDir is
created inline and its path passed to DiskStorageManager::new(), which lets the
TempDir be dropped immediately and the directory removed; bind the TempDir to a
local variable with a lifetime that spans the test (e.g., let tmp_dir =
TempDir::new().expect(...);) and then pass tmp_dir.path().into() to
DiskStorageManager::new() so the temporary directory remains alive until the
test completes.
ZocoLini
force-pushed
the
clean/remove-memory-storage
branch
from
5070407 to
31c9b8b
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
dash-spv/tests/block_download_test.rs(10 hunks)dash-spv/tests/edge_case_filter_sync_test.rs(3 hunks)dash-spv/tests/filter_header_verification_test.rs(6 hunks)dash-spv/tests/reverse_index_test.rs(2 hunks)dash-spv/tests/storage_test.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- dash-spv/tests/edge_case_filter_sync_test.rs
- dash-spv/tests/reverse_index_test.rs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/tests/filter_header_verification_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/block_download_test.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/filter_header_verification_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/block_download_test.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Use trait-based abstractions for core components likeNetworkManagerandStorageManagerto enable swappable implementations
Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles withcargo check --all-features
Files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/block_download_test.rs
dash-spv/tests/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Integration tests should be organized in
tests/directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration
Files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/block_download_test.rs
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/block_download_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use trait-based abstractions for core components like `NetworkManager` and `StorageManager` to enable swappable implementations
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/block_download_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should be organized in `tests/` directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/block_download_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Synchronization should use `SyncManager` for phase-based coordination: Phase 1 (Headers), Phase 2 (Masternode List), Phase 3 (Filter Headers), Phase 4 (Filters), Phase 5 (Blocks)
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_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/filter_header_verification_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/block_download_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_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/filter_header_verification_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/block_download_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/error.rs : Maintain domain-specific error types in `error.rs` with variants for `NetworkError`, `StorageError`, `SyncError`, `ValidationError`, and `SpvError` (top-level wrapper)
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_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/src/**/*.rs : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_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/filter_header_verification_test.rsdash-spv/tests/storage_test.rsdash-spv/tests/block_download_test.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/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/network/**/*.rs : Network module should handle TCP connections, handshake management, message routing, and peer management following Dash protocol implementation
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rs
📚 Learning: 2025-02-25T06:13:52.858Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 51
File: dash/src/sml/masternode_list/scores_for_quorum.rs:50-73
Timestamp: 2025-02-25T06:13:52.858Z
Learning: ScoreHash is a cryptographic hash in the rust-dashcore library, and therefore does not need collision handling when used as a key in collections like BTreeMap due to the extremely low probability of collisions.
Applied to files:
dash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Storage should use segmented storage with headers stored in 10,000-header segments with index files, separate storage for filter headers and compact block filters
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/tests/storage_test.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize tests by functionality: BIP32 key derivation, mnemonic generation/validation, address generation/validation, path derivation, and PSBT serialization in separate test files
Applied to files:
dash-spv/tests/storage_test.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Applies to **/tests/**/*.rs : Integration tests use `tests/` directory (e.g., `rpc-integration-test`)
Applied to files:
dash-spv/tests/storage_test.rs
📚 Learning: 2025-12-01T07:59:46.015Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:46.015Z
Learning: Applies to **/tests/**/*.rs : Test both mainnet and testnet configurations
Applied to files:
dash-spv/tests/storage_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/src/**/*.rs : All FFI types must have corresponding `_destroy()` functions for explicit memory management
Applied to files:
dash-spv/tests/storage_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/client/**/*.rs : Client module should provide high-level API through `DashSpvClient` and configuration via `ClientConfig`
Applied to files:
dash-spv/tests/block_download_test.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 : Enable mempool tracking on SPV connection using the appropriate strategy (.fetchAll, .bloomFilter, .selective)
Applied to files:
dash-spv/tests/block_download_test.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/tests/block_download_test.rs
🧬 Code graph analysis (3)
dash-spv/tests/filter_header_verification_test.rs (3)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv/src/sync/manager.rs (1)
filter_sync(394-396)
dash-spv/tests/storage_test.rs (4)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/client/core.rs (1)
storage(169-171)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv-ffi/src/broadcast.rs (1)
dashcore(29-29)
dash-spv/tests/block_download_test.rs (3)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv/src/sync/manager.rs (1)
filter_sync(394-396)
⏰ 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_sha256)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Key Wallet Components Tests
- GitHub Check: Core Components Tests
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: Pre-commit (ubuntu-latest)
🔇 Additional comments (5)
dash-spv/tests/storage_test.rs (3)
40-62: LGTM! Correct TempDir lifetime management.The test correctly stores
TempDirin a variable before passing its path toDiskStorageManager. This ensures the temporary directory remains valid throughout the test execution and is automatically cleaned up when theTempDirdrops at scope end.
64-86: LGTM! Proper concurrent access testing.The test correctly validates that
DiskStorageManagerprevents concurrent access to the same storage directory, which is essential for data integrity.
88-106: LGTM! Lock file lifecycle verified.The test properly validates lock file creation and cleanup behavior.
dash-spv/tests/block_download_test.rs (1)
168-169: LGTM! Type signatures updated consistently.All
FilterSyncManagertype parameters correctly updated fromMemoryStorageManagertoDiskStorageManager.Also applies to: 180-181, 218-219, 242-243, 273-274, 290-291, 348-349, 378-379
dash-spv/tests/filter_header_verification_test.rs (1)
192-193: LGTM! Type signatures updated consistently.All
FilterSyncManagertype parameters correctly updated fromMemoryStorageManagertoDiskStorageManager.Also applies to: 356-357, 532-533, 630-631
| let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) | ||
| .await | ||
| .expect("Failed to create tmp storage"); |
There was a problem hiding this comment.
Critical: TempDir lifetime bug causes premature directory deletion.
The TempDir is created inline and immediately dropped, deleting the directory before DiskStorageManager initializes. This pattern appears in multiple tests in this file (lines 185-187, 349-351, 525-527, 623-625).
Apply this fix pattern to all occurrences:
- let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf())
- .await
- .expect("Failed to create tmp storage");
+ let temp_dir = TempDir::new().expect("Failed to create temp directory");
+ let mut storage = DiskStorageManager::new(temp_dir.path().to_path_buf())
+ .await
+ .expect("Failed to create tmp storage");The temp_dir variable must remain in scope for the test duration. See dash-spv/tests/storage_test.rs:42-47 for the correct pattern.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) | |
| .await | |
| .expect("Failed to create tmp storage"); | |
| let temp_dir = TempDir::new().expect("Failed to create temp directory"); | |
| let mut storage = DiskStorageManager::new(temp_dir.path().to_path_buf()) | |
| .await | |
| .expect("Failed to create tmp storage"); |
🤖 Prompt for AI Agents
In dash-spv/tests/filter_header_verification_test.rs around lines 185-187 (and
similarly at 349-351, 525-527, 623-625) the TempDir is created inline and
immediately dropped, causing the temporary directory to be deleted before
DiskStorageManager initializes; fix by creating a local variable (e.g. let
temp_dir = TempDir::new().unwrap();) that stays in scope for the whole test,
pass temp_dir.path().to_path_buf() into
DiskStorageManager::new(...).await.expect(...), and ensure temp_dir is not
dropped until the test ends (follow the pattern used in
dash-spv/tests/storage_test.rs:42-47).
| let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) | ||
| .await | ||
| .expect("Failed to create tmp storage"); |
There was a problem hiding this comment.
Critical: Same TempDir lifetime bug.
Same issue as line 185-187. Apply the same fix to keep TempDir alive for the test duration.
🤖 Prompt for AI Agents
In dash-spv/tests/filter_header_verification_test.rs around lines 349 to 351,
the TempDir is created and its path used inline which drops the TempDir
immediately (lifetime bug); fix by binding the TempDir to a local variable
(e.g., let tmp_dir = TempDir::new().unwrap();) and then pass
tmp_dir.path().to_path_buf() into DiskStorageManager::new(...), ensuring the
tmp_dir variable remains in scope for the duration of the test so the temporary
directory is not deleted prematurely.
| let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) | ||
| .await | ||
| .expect("Failed to create tmp storage"); |
There was a problem hiding this comment.
Critical: Same TempDir lifetime bug.
Same issue as line 185-187. Apply the same fix to keep TempDir alive for the test duration.
🤖 Prompt for AI Agents
In dash-spv/tests/filter_header_verification_test.rs around lines 525 to 527,
the TempDir is created inline and dropped immediately which invalidates the path
used by DiskStorageManager; instead, bind the TempDir to a local variable (e.g.
let temp_dir = TempDir::new().unwrap();) before calling DiskStorageManager::new,
then pass temp_dir.path().to_path_buf() to DiskStorageManager::new and keep the
temp_dir variable in scope for the duration of the test so the directory is not
removed while the storage is used.
| let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) | ||
| .await | ||
| .expect("Failed to create tmp storage"); |
There was a problem hiding this comment.
Critical: Same TempDir lifetime bug.
Same issue as line 185-187. Apply the same fix to keep TempDir alive for the test duration.
🤖 Prompt for AI Agents
In dash-spv/tests/filter_header_verification_test.rs around lines 623 to 625,
the TempDir is created inline and immediately dropped before DiskStorageManager
uses its path, causing a lifetime bug; fix it by creating and storing the
TempDir in a local variable (e.g., let tmp_dir = TempDir::new().unwrap()), then
pass tmp_dir.path().to_path_buf() into DiskStorageManager::new and keep tmp_dir
in scope for the whole test so the temporary directory remains alive until the
test completes.
31c9b8b to
d44cf8f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
dash-spv/tests/block_download_test.rs (1)
344-346: Critical: TempDir lifetime bug causes premature directory deletion.The
TempDiris created inline and immediately dropped after calling.to_path_buf(), which deletes the temporary directory beforeDiskStorageManagercan initialize. This will cause the test to fail.Apply this fix to keep the
TempDiralive for the test duration:- let _storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) - .await - .expect("Failed to create tmp storage"); + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let _storage = DiskStorageManager::new(temp_dir.path().to_path_buf()) + .await + .expect("Failed to create tmp storage");Note: The correct pattern is demonstrated in
dash-spv/tests/storage_test.rs:42-47.dash-spv/tests/filter_header_verification_test.rs (4)
185-187: Critical: TempDir lifetime bug causes premature directory deletion.The
TempDiris created inline and immediately dropped, deleting the directory beforeDiskStorageManagerinitializes. This pattern appears in multiple tests in this file (lines 185-187, 349-351, 525-527, 623-625).Apply this fix pattern to all occurrences:
- let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) - .await - .expect("Failed to create tmp storage"); + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let mut storage = DiskStorageManager::new(temp_dir.path().to_path_buf()) + .await + .expect("Failed to create tmp storage");The
temp_dirvariable must remain in scope for the test duration. Seedash-spv/tests/storage_test.rs:42-47for the correct pattern.
349-351: Critical: Same TempDir lifetime bug.Same issue as lines 185-187. Apply the same fix to keep
TempDiralive for the test duration.
525-527: Critical: Same TempDir lifetime bug.Same issue as lines 185-187. Apply the same fix to keep
TempDiralive for the test duration.
623-625: Critical: Same TempDir lifetime bug.Same issue as lines 185-187. Apply the same fix to keep
TempDiralive for the test duration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
dash-spv/src/main.rs(1 hunks)dash-spv/tests/block_download_test.rs(10 hunks)dash-spv/tests/edge_case_filter_sync_test.rs(3 hunks)dash-spv/tests/filter_header_verification_test.rs(6 hunks)dash-spv/tests/reverse_index_test.rs(2 hunks)dash-spv/tests/storage_test.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- dash-spv/src/main.rs
- dash-spv/tests/reverse_index_test.rs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/tests/storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.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/storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Use trait-based abstractions for core components likeNetworkManagerandStorageManagerto enable swappable implementations
Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles withcargo check --all-features
Files:
dash-spv/tests/storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.rs
dash-spv/tests/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Integration tests should be organized in
tests/directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration
Files:
dash-spv/tests/storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.rs
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
Applied to files:
dash-spv/tests/storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should be organized in `tests/` directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration
Applied to files:
dash-spv/tests/storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use trait-based abstractions for core components like `NetworkManager` and `StorageManager` to enable swappable implementations
Applied to files:
dash-spv/tests/storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_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/storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_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/storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize tests by functionality: BIP32 key derivation, mnemonic generation/validation, address generation/validation, path derivation, and PSBT serialization in separate test files
Applied to files:
dash-spv/tests/storage_test.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Applies to **/tests/**/*.rs : Integration tests use `tests/` directory (e.g., `rpc-integration-test`)
Applied to files:
dash-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/storage_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_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/src/**/*.rs : All FFI types must have corresponding `_destroy()` functions for explicit memory management
Applied to files:
dash-spv/tests/storage_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Applied to files:
dash-spv/tests/storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Storage should use segmented storage with headers stored in 10,000-header segments with index files, separate storage for filter headers and compact block filters
Applied to files:
dash-spv/tests/storage_test.rsdash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Synchronization should use `SyncManager` for phase-based coordination: Phase 1 (Headers), Phase 2 (Masternode List), Phase 3 (Filter Headers), Phase 4 (Filters), Phase 5 (Blocks)
Applied to files:
dash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/error.rs : Maintain domain-specific error types in `error.rs` with variants for `NetworkError`, `StorageError`, `SyncError`, `ValidationError`, and `SpvError` (top-level wrapper)
Applied to files:
dash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.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/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_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/src/**/*.rs : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function
Applied to files:
dash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/client/**/*.rs : Client module should provide high-level API through `DashSpvClient` and configuration via `ClientConfig`
Applied to files:
dash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)
Applied to files:
dash-spv/tests/edge_case_filter_sync_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.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/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/network/**/*.rs : Network module should handle TCP connections, handshake management, message routing, and peer management following Dash protocol implementation
Applied to files:
dash-spv/tests/block_download_test.rsdash-spv/tests/filter_header_verification_test.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 : Enable mempool tracking on SPV connection using the appropriate strategy (.fetchAll, .bloomFilter, .selective)
Applied to files:
dash-spv/tests/block_download_test.rs
📚 Learning: 2025-02-25T06:13:52.858Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 51
File: dash/src/sml/masternode_list/scores_for_quorum.rs:50-73
Timestamp: 2025-02-25T06:13:52.858Z
Learning: ScoreHash is a cryptographic hash in the rust-dashcore library, and therefore does not need collision handling when used as a key in collections like BTreeMap due to the extremely low probability of collisions.
Applied to files:
dash-spv/tests/filter_header_verification_test.rs
🧬 Code graph analysis (2)
dash-spv/tests/storage_test.rs (4)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/client/core.rs (1)
storage(169-171)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv-ffi/src/broadcast.rs (1)
dashcore(29-29)
dash-spv/tests/filter_header_verification_test.rs (4)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/client/core.rs (2)
storage(169-171)network(164-166)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv/src/sync/manager.rs (1)
filter_sync(394-396)
⏰ 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: Key Wallet Components Tests
- GitHub Check: Core Components Tests
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: Pre-commit (ubuntu-latest)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
🔇 Additional comments (1)
dash-spv/tests/storage_test.rs (1)
1-106: Excellent migration to DiskStorageManager with correct TempDir lifecycle management.All three tests properly store the
TempDirin a variable that remains in scope for the test duration, ensuring the temporary directory isn't deleted prematurely. This is the correct pattern that should be followed throughout the codebase.
| let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) | ||
| .await | ||
| .expect("Failed to create tmp storage"); |
There was a problem hiding this comment.
Critical: TempDir lifetime bug causes premature directory deletion.
The TempDir is created inline and immediately dropped after calling .to_path_buf(), which deletes the temporary directory before DiskStorageManager can initialize.
Apply this fix to keep the TempDir alive for the test duration:
- let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf())
- .await
- .expect("Failed to create tmp storage");
+ let temp_dir = TempDir::new().expect("Failed to create temp directory");
+ let mut storage = DiskStorageManager::new(temp_dir.path().to_path_buf())
+ .await
+ .expect("Failed to create tmp storage");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) | |
| .await | |
| .expect("Failed to create tmp storage"); | |
| let temp_dir = TempDir::new().expect("Failed to create temp directory"); | |
| let mut storage = DiskStorageManager::new(temp_dir.path().to_path_buf()) | |
| .await | |
| .expect("Failed to create tmp storage"); |
🤖 Prompt for AI Agents
In dash-spv/tests/edge_case_filter_sync_test.rs around lines 125 to 127, the
TempDir is created inline and dropped right after calling to_path_buf(), causing
the directory to be deleted before DiskStorageManager initializes; fix by
binding the TempDir to a local variable that lives for the test (e.g., let
temp_dir = TempDir::new().unwrap();), pass temp_dir.path().to_path_buf() into
DiskStorageManager::new(...).await.expect(...), and keep the temp_dir variable
in scope for the remainder of the test (prefix with _temp_dir if unused) so it
is not dropped prematurely.
| let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) | ||
| .await | ||
| .expect("Failed to create tmp storage"); |
There was a problem hiding this comment.
Critical: Same TempDir lifetime bug.
Same issue as lines 125-127. The TempDir must be stored in a variable to prevent premature directory deletion.
Apply the same fix pattern:
- let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf())
- .await
- .expect("Failed to create tmp storage");
+ let temp_dir = TempDir::new().expect("Failed to create temp directory");
+ let mut storage = DiskStorageManager::new(temp_dir.path().to_path_buf())
+ .await
+ .expect("Failed to create tmp storage");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) | |
| .await | |
| .expect("Failed to create tmp storage"); | |
| let temp_dir = TempDir::new().expect("Failed to create temp directory"); | |
| let mut storage = DiskStorageManager::new(temp_dir.path().to_path_buf()) | |
| .await | |
| .expect("Failed to create tmp storage"); |
🤖 Prompt for AI Agents
In dash-spv/tests/edge_case_filter_sync_test.rs around lines 170 to 172, the
TempDir is created inline and dropped immediately causing the directory to be
deleted before DiskStorageManager uses it; create a TempDir and bind it to a
variable (e.g., let tmp_dir = TempDir::new().unwrap();) and pass
tmp_dir.path().to_path_buf() into DiskStorageManager::new, ensuring the tmp_dir
variable stays in scope for the lifetime of the storage so the directory is not
removed prematurely.
d44cf8f to
394572e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
dash-spv/tests/filter_header_verification_test.rs (1)
185-187: Critical: TempDir lifetime bug causes premature directory deletion.The
TempDiris created inline and immediately dropped, deleting the directory beforeDiskStorageManagerinitializes. This issue occurs at lines 185-187, 349-351, 525-527, and 623-625.Apply this fix pattern to all occurrences:
- let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) - .await - .expect("Failed to create tmp storage"); + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let mut storage = DiskStorageManager::new(temp_dir.path().to_path_buf()) + .await + .expect("Failed to create tmp storage");dash-spv/tests/edge_case_filter_sync_test.rs (2)
122-127: Critical: TempDir lifetime bug causes premature directory deletion.The
TempDiris created inline and immediately dropped. Same issue at lines 170-172.Apply this fix:
+ let temp_dir = TempDir::new().expect("Failed to create temp directory"); let mut filter_sync: FilterSyncManager<DiskStorageManager, MockNetworkManager> = FilterSyncManager::new(&config, received_heights); - let mut storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) - .await - .expect("Failed to create tmp storage"); + let mut storage = DiskStorageManager::new(temp_dir.path().to_path_buf()) + .await + .expect("Failed to create tmp storage");
167-172: Critical: Same TempDir lifetime bug.Same issue as lines 125-127. Apply the same fix pattern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
dash-spv/src/main.rs(1 hunks)dash-spv/tests/block_download_test.rs(10 hunks)dash-spv/tests/edge_case_filter_sync_test.rs(3 hunks)dash-spv/tests/filter_header_verification_test.rs(6 hunks)dash-spv/tests/reverse_index_test.rs(2 hunks)dash-spv/tests/storage_test.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- dash-spv/tests/reverse_index_test.rs
- dash-spv/tests/block_download_test.rs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/tests/storage_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/main.rsdash-spv/tests/edge_case_filter_sync_test.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/storage_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/edge_case_filter_sync_test.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Use trait-based abstractions for core components likeNetworkManagerandStorageManagerto enable swappable implementations
Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles withcargo check --all-features
Files:
dash-spv/tests/storage_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/main.rsdash-spv/tests/edge_case_filter_sync_test.rs
dash-spv/tests/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Integration tests should be organized in
tests/directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration
Files:
dash-spv/tests/storage_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/edge_case_filter_sync_test.rs
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
Applied to files:
dash-spv/tests/storage_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/main.rsdash-spv/tests/edge_case_filter_sync_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should be organized in `tests/` directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration
Applied to files:
dash-spv/tests/storage_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/main.rsdash-spv/tests/edge_case_filter_sync_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use trait-based abstractions for core components like `NetworkManager` and `StorageManager` to enable swappable implementations
Applied to files:
dash-spv/tests/storage_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/main.rsdash-spv/tests/edge_case_filter_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/storage_test.rsdash-spv/tests/edge_case_filter_sync_test.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize tests by functionality: BIP32 key derivation, mnemonic generation/validation, address generation/validation, path derivation, and PSBT serialization in separate test files
Applied to files:
dash-spv/tests/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/storage_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/src/main.rsdash-spv/tests/edge_case_filter_sync_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/storage_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/edge_case_filter_sync_test.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/tests/storage_test.rsdash-spv/tests/filter_header_verification_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/src/**/*.rs : All FFI types must have corresponding `_destroy()` functions for explicit memory management
Applied to files:
dash-spv/tests/storage_test.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)
Applied to files:
dash-spv/tests/storage_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/edge_case_filter_sync_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Storage should use segmented storage with headers stored in 10,000-header segments with index files, separate storage for filter headers and compact block filters
Applied to files:
dash-spv/tests/storage_test.rsdash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Synchronization should use `SyncManager` for phase-based coordination: Phase 1 (Headers), Phase 2 (Masternode List), Phase 3 (Filter Headers), Phase 4 (Filters), Phase 5 (Blocks)
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/src/main.rsdash-spv/tests/edge_case_filter_sync_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/src/main.rsdash-spv/tests/edge_case_filter_sync_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/error.rs : Maintain domain-specific error types in `error.rs` with variants for `NetworkError`, `StorageError`, `SyncError`, `ValidationError`, and `SpvError` (top-level wrapper)
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/src/main.rsdash-spv/tests/edge_case_filter_sync_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/validation/**/*.rs : Validation module should support configurable validation modes (`ValidationMode::None`, `ValidationMode::Basic`, `ValidationMode::Full`) with header validation, ChainLock, and InstantLock verification
Applied to files:
dash-spv/tests/filter_header_verification_test.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/tests/filter_header_verification_test.rsdash-spv/tests/edge_case_filter_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/src/**/*.rs : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/src/main.rsdash-spv/tests/edge_case_filter_sync_test.rs
📚 Learning: 2025-02-25T06:13:52.858Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 51
File: dash/src/sml/masternode_list/scores_for_quorum.rs:50-73
Timestamp: 2025-02-25T06:13:52.858Z
Learning: ScoreHash is a cryptographic hash in the rust-dashcore library, and therefore does not need collision handling when used as a key in collections like BTreeMap due to the extremely low probability of collisions.
Applied to files:
dash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/network/**/*.rs : Network module should handle TCP connections, handshake management, message routing, and peer management following Dash protocol implementation
Applied to files:
dash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/client/**/*.rs : Client module should provide high-level API through `DashSpvClient` and configuration via `ClientConfig`
Applied to files:
dash-spv/src/main.rsdash-spv/tests/edge_case_filter_sync_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles with `cargo check --all-features`
Applied to files:
dash-spv/src/main.rs
🧬 Code graph analysis (1)
dash-spv/tests/filter_header_verification_test.rs (3)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv/src/sync/manager.rs (1)
filter_sync(394-396)
⏰ 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: Core Components Tests
- GitHub Check: SPV Components Tests
- GitHub Check: Key Wallet Components Tests
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: Pre-commit (ubuntu-latest)
🔇 Additional comments (7)
dash-spv/tests/filter_header_verification_test.rs (2)
22-36: LGTM: Import updates are correct.The imports correctly reflect the migration from
MemoryStorageManagertoDiskStorageManagerand addTempDirfor temporary storage paths.
192-193: LGTM: Type parameter update is correct.The
FilterSyncManagertype parameter correctly updated fromMemoryStorageManagertoDiskStorageManager.dash-spv/src/main.rs (1)
323-332: LGTM: Client invocation is correct.The
run_clientcall passes all required parameters correctly after storage initialization.dash-spv/tests/storage_test.rs (3)
4-5: LGTM: Import updates are correct.The imports correctly reflect the migration to
DiskStorageManager.
40-62: LGTM: Correct TempDir lifetime pattern.This test correctly stores
TempDirin a variable (temp_dir) that remains in scope, ensuring the directory isn't deleted prematurely. This is the pattern that should be followed in other test files.
64-106: LGTM: Storage locking tests are well-designed.The concurrent access and lock file lifecycle tests properly verify DiskStorageManager's locking behavior and use the correct TempDir pattern.
dash-spv/tests/edge_case_filter_sync_test.rs (1)
12-19: LGTM: Import updates are correct.The imports correctly add
TempDirand replaceMemoryStorageManagerwithDiskStorageManager.
| let path = if let Some(path) = &config.storage_path { | ||
| path.clone() | ||
| } else { | ||
| let storage_manager = match dash_spv::storage::MemoryStorageManager::new().await { | ||
| Ok(sm) => sm, | ||
| Err(e) => { | ||
| eprintln!("Failed to create memory storage manager: {}", e); | ||
| process::exit(1); | ||
| } | ||
| }; | ||
| run_client( | ||
| config, | ||
| network_manager, | ||
| storage_manager, | ||
| wallet, | ||
| enable_terminal_ui, | ||
| &matches, | ||
| wallet_id, | ||
| ) | ||
| .await?; | ||
| } | ||
| "./.tmp/main-exec-storage".into() | ||
| }; | ||
|
|
||
| let storage_manager = match dash_spv::storage::DiskStorageManager::new(path.clone()).await { | ||
| Ok(sm) => sm, | ||
| Err(e) => { | ||
| eprintln!("Failed to create disk storage manager: {}", e); | ||
| process::exit(1); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the config structure and with_storage_path method
rg -n "with_storage_path" dash-spv/src --type rs -A 5 -B 2Repository: dashpay/rust-dashcore
Length of output: 91
🏁 Script executed:
# Check the config.storage_path field definition
rg -n "storage_path" dash-spv/src/client/config.rs -B 2 -A 2Repository: dashpay/rust-dashcore
Length of output: 630
🏁 Script executed:
# Examine the main.rs code flow around lines 183-320 to trace storage_path
sed -n '183,320p' dash-spv/src/main.rs | cat -nRepository: dashpay/rust-dashcore
Length of output: 5974
🏁 Script executed:
# Check DiskStorageManager::new signature
rg -n "impl DiskStorageManager" dash-spv/src/storage -A 10Repository: dashpay/rust-dashcore
Length of output: 3356
🏁 Script executed:
# Find the validate() method implementation in config.rs
rg -n "fn validate" dash-spv/src/client/config.rs -A 20Repository: dashpay/rust-dashcore
Length of output: 886
🏁 Script executed:
# Check if storage_path is modified after being set with_storage_path
rg -n "storage_path" dash-spv/src/main.rs -B 2 -A 2Repository: dashpay/rust-dashcore
Length of output: 377
🏁 Script executed:
# Check the actual line numbers in main.rs to confirm the code snippet
sed -n '230,260p' dash-spv/src/main.rs | cat -nRepository: dashpay/rust-dashcore
Length of output: 1332
Remove the unreachable fallback path that violates coding guidelines.
The config.storage_path is always set to Some(data_dir) via with_storage_path(data_dir.clone()) at line 239, where data_dir is guaranteed to be populated (either from CLI or a unique temp directory at lines 183–194). The fallback to "./.tmp/main-exec-storage" at line 313 is unreachable dead code and violates the coding guideline against hardcoding paths.
- let path = if let Some(path) = &config.storage_path {
- path.clone()
- } else {
- "./.tmp/main-exec-storage".into()
- };
-
- let storage_manager = match dash_spv::storage::DiskStorageManager::new(path.clone()).await {
+ let storage_path = config.storage_path.clone()
+ .expect("storage_path must be set via with_storage_path");
+
+ let storage_manager = match dash_spv::storage::DiskStorageManager::new(storage_path).await {🤖 Prompt for AI Agents
In dash-spv/src/main.rs around lines 310 to 322, remove the unreachable
hardcoded fallback path and directly use the configured storage path; replace
the if-let that provides "./.tmp/main-exec-storage" with a direct unwrap/expect
on config.storage_path (e.g., config.storage_path.clone().expect("storage_path
must be set")) or otherwise use config.storage_path.as_ref().unwrap().clone(),
so the code always uses the guaranteed Some(data_dir) value and avoids hardcoded
paths.
…te the same operations as the diskstoragemanager. next time use generics pls
ZocoLini
force-pushed
the
clean/remove-memory-storage
branch
from
394572e to
8b2f417
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
dash-spv/tests/filter_header_verification_test.rs (4)
185-187: Critical: TempDir lifetime bug in test_filter_header_verification_failure_reproduction.The
TempDiris created inline and immediately dropped, causing premature directory deletion beforeDiskStorageManagerinitializes.This issue was already flagged in a previous review. Apply the fix pattern: store
TempDirin a variable that stays in scope for the test duration.Also applies to: 192-193
349-351: Critical: TempDir lifetime bug in test_overlapping_batches_from_different_peers.Same TempDir lifetime issue as other tests in this file.
Also applies to: 356-357
525-527: Critical: TempDir lifetime bug in test_filter_header_verification_overlapping_batches.Same TempDir lifetime issue as other tests in this file.
Also applies to: 532-533
623-625: Critical: TempDir lifetime bug in test_filter_header_verification_race_condition_simulation.Same TempDir lifetime issue as other tests in this file.
Also applies to: 630-631
dash-spv/tests/block_download_test.rs (1)
344-346: Critical: TempDir lifetime bug causes premature directory deletion.The
TempDiris created inline and immediately dropped after calling.path().to_path_buf(), deleting the temporary directory beforeDiskStorageManagerinitializes. Despite a previous review indicating this was addressed, the current code still exhibits the problematic pattern.Apply this fix to keep the
TempDiralive for the test duration:- let _storage = DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()) - .await - .expect("Failed to create tmp storage"); + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let _storage = DiskStorageManager::new(temp_dir.path().to_path_buf()) + .await + .expect("Failed to create tmp storage");See
dash-spv/tests/storage_test.rs:42-47for the correct pattern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
dash-spv/src/lib.rs(2 hunks)dash-spv/src/main.rs(1 hunks)dash-spv/src/storage/mod.rs(2 hunks)dash-spv/tests/block_download_test.rs(10 hunks)dash-spv/tests/edge_case_filter_sync_test.rs(3 hunks)dash-spv/tests/filter_header_verification_test.rs(6 hunks)dash-spv/tests/reverse_index_test.rs(2 hunks)dash-spv/tests/storage_test.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- dash-spv/tests/edge_case_filter_sync_test.rs
- dash-spv/src/main.rs
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/tests/reverse_index_test.rsdash-spv/src/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/storage_test.rsdash-spv/src/storage/mod.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/reverse_index_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/storage_test.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Use trait-based abstractions for core components likeNetworkManagerandStorageManagerto enable swappable implementations
Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles withcargo check --all-features
Files:
dash-spv/tests/reverse_index_test.rsdash-spv/src/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/storage_test.rsdash-spv/src/storage/mod.rs
dash-spv/tests/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Integration tests should be organized in
tests/directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration
Files:
dash-spv/tests/reverse_index_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/storage_test.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Storage module should provide abstraction via
StorageManagertrait with concrete implementations forMemoryStorageManagerandDiskStorageManager
Files:
dash-spv/src/storage/mod.rs
🧠 Learnings (29)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
Applied to files:
dash-spv/tests/reverse_index_test.rsdash-spv/src/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/storage_test.rsdash-spv/src/storage/mod.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should be organized in `tests/` directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration
Applied to files:
dash-spv/tests/reverse_index_test.rsdash-spv/src/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/storage_test.rsdash-spv/src/storage/mod.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use trait-based abstractions for core components like `NetworkManager` and `StorageManager` to enable swappable implementations
Applied to files:
dash-spv/tests/reverse_index_test.rsdash-spv/src/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/storage_test.rsdash-spv/src/storage/mod.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/reverse_index_test.rsdash-spv/src/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/storage_test.rsdash-spv/src/storage/mod.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/reverse_index_test.rsdash-spv/src/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/src/storage/mod.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/reverse_index_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/storage_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Synchronization should use `SyncManager` for phase-based coordination: Phase 1 (Headers), Phase 2 (Masternode List), Phase 3 (Filter Headers), Phase 4 (Filters), Phase 5 (Blocks)
Applied to files:
dash-spv/tests/reverse_index_test.rsdash-spv/src/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/src/storage/mod.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/reverse_index_test.rsdash-spv/src/lib.rsdash-spv/tests/block_download_test.rsdash-spv/tests/storage_test.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Applies to **/tests/**/*.rs : Integration tests use `tests/` directory (e.g., `rpc-integration-test`)
Applied to files:
dash-spv/tests/reverse_index_test.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/tests/reverse_index_test.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/src/storage/mod.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Applied to files:
dash-spv/tests/reverse_index_test.rsdash-spv/src/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/tests/storage_test.rsdash-spv/src/storage/mod.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles with `cargo check --all-features`
Applied to files:
dash-spv/src/lib.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/lib.rsdash-spv/tests/storage_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/error.rs : Maintain domain-specific error types in `error.rs` with variants for `NetworkError`, `StorageError`, `SyncError`, `ValidationError`, and `SpvError` (top-level wrapper)
Applied to files:
dash-spv/src/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/src/storage/mod.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/*.rs : Use `?` operator for error propagation in Rust code to maintain clean error handling patterns
Applied to files:
dash-spv/src/lib.rsdash-spv/src/storage/mod.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/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/src/storage/mod.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Applies to **/*.rs : Follow mixed editions (2021/2024) and crate-specific idioms; prefer async via `tokio` where applicable
Applied to files:
dash-spv/src/lib.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/client/**/*.rs : Client module should provide high-level API through `DashSpvClient` and configuration via `ClientConfig`
Applied to files:
dash-spv/src/lib.rsdash-spv/tests/block_download_test.rsdash-spv/src/storage/mod.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/network/**/*.rs : Network module should handle TCP connections, handshake management, message routing, and peer management following Dash protocol implementation
Applied to files:
dash-spv/src/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)
Applied to files:
dash-spv/src/lib.rsdash-spv/tests/filter_header_verification_test.rsdash-spv/tests/block_download_test.rsdash-spv/src/storage/mod.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/wallet/**/*.rs : Wallet module should provide UTXO tracking via `Utxo` struct, balance calculation with confirmation states, and transaction processing via `TransactionProcessor`
Applied to files:
dash-spv/src/lib.rsdash-spv/src/storage/mod.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/lib.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/validation/**/*.rs : Validation module should support configurable validation modes (`ValidationMode::None`, `ValidationMode::Basic`, `ValidationMode::Full`) with header validation, ChainLock, and InstantLock verification
Applied to files:
dash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-02-25T06:13:52.858Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 51
File: dash/src/sml/masternode_list/scores_for_quorum.rs:50-73
Timestamp: 2025-02-25T06:13:52.858Z
Learning: ScoreHash is a cryptographic hash in the rust-dashcore library, and therefore does not need collision handling when used as a key in collections like BTreeMap due to the extremely low probability of collisions.
Applied to files:
dash-spv/tests/filter_header_verification_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Storage should use segmented storage with headers stored in 10,000-header segments with index files, separate storage for filter headers and compact block filters
Applied to files:
dash-spv/tests/filter_header_verification_test.rsdash-spv/tests/storage_test.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 : Enable mempool tracking on SPV connection using the appropriate strategy (.fetchAll, .bloomFilter, .selective)
Applied to files:
dash-spv/tests/block_download_test.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize tests by functionality: BIP32 key derivation, mnemonic generation/validation, address generation/validation, path derivation, and PSBT serialization in separate test files
Applied to files:
dash-spv/tests/storage_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/src/**/*.rs : All FFI types must have corresponding `_destroy()` functions for explicit memory management
Applied to files:
dash-spv/tests/storage_test.rs
🧬 Code graph analysis (4)
dash-spv/tests/reverse_index_test.rs (1)
dash-spv/src/storage/manager.rs (1)
new(64-150)
dash-spv/tests/filter_header_verification_test.rs (3)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv/src/sync/manager.rs (1)
filter_sync(394-396)
dash-spv/tests/block_download_test.rs (2)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)
dash-spv/tests/storage_test.rs (4)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/client/core.rs (1)
storage(169-171)dash-spv/src/sync/headers/manager.rs (1)
storage(261-261)dash-spv-ffi/src/broadcast.rs (1)
dashcore(29-29)
⏰ 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: fuzz (hashes_sha1)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: SPV Components Tests
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: Key Wallet Components Tests
- GitHub Check: Core Components Tests
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: Pre-commit (ubuntu-latest)
🔇 Additional comments (9)
dash-spv/src/lib.rs (1)
17-17: Documentation example correctly updated to use DiskStorageManager.The Quick Start example is properly updated to reflect the new disk-backed storage API. The storage initialization path
./.tmp/example-storageis appropriate for an example demonstrating persistent storage usage.Also applies to: 33-33
dash-spv/tests/storage_test.rs (1)
40-106: Well-structured disk storage tests with correct TempDir lifetime handling.All three tests properly maintain
TempDirin scope for the duration of testing, ensuring the temporary directory isn't prematurely deleted. The tests cover important scenarios: persistence across reopen, concurrent access prevention via lock files, and lock file lifecycle management.dash-spv/tests/reverse_index_test.rs (1)
7-53: Well-structured persistence test with proper TempDir handling.The test correctly maintains
TempDirin scope, uses explicit shutdown to ensure data persistence, and verifies data survives storage reopen. Good pattern for testing disk-backed storage lifecycle.dash-spv/tests/filter_header_verification_test.rs (1)
22-22: Import updates correctly reflect the storage migration.The imports are properly updated to use
DiskStorageManagerand includeTempDirfor temporary storage paths in tests.Also applies to: 36-36
dash-spv/tests/block_download_test.rs (2)
12-12: Import updates correctly reflect the storage migration.The imports are properly updated to include
TempDirfor temporary storage andDiskStorageManageras the storage backend.Also applies to: 26-27
168-169: Type annotations correctly updated to use DiskStorageManager.All
FilterSyncManagertype annotations are properly updated fromMemoryStorageManagertoDiskStorageManager, maintaining consistency across the test file.Also applies to: 180-181, 218-219, 242-243, 273-274, 290-291, 348-349, 378-379
dash-spv/src/storage/mod.rs (3)
3-4: Good internal refactoring.Making the
iomodule crate-visible (pub(crate)) rather than public is appropriate for internal utilities. This properly encapsulates implementation details while allowing internal usage across the crate.
9-14: Clean module organization.The new internal module structure (headers, lockfile, manager, segments, state) properly encapsulates storage implementation details. Re-exporting only
DiskStorageManagerfrom themanagermodule on line 16 maintains a clean public API.
83-87: The documentation example is correct.DiskStorageManager::newaccepts aPathBufparameter (via.into()conversion) and returns an asyncStorageResult<Self>. The example properly chains.await?and wraps the manager inArc<Mutex<>>as appropriate for shared, mutable access in async code. Thenew_tmp()alternative method also exists for testing scenarios if needed.
dash-spv/src/storage/mod.rs
Outdated
| mod state; | ||
|
|
||
| use async_trait::async_trait; | ||
| pub use manager::DiskStorageManager; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for any remaining references to MemoryStorageManager
rg -n "MemoryStorageManager" --type rust
# Check the storage module structure
ls -la dash-spv/src/storage/
# Verify the current exports in mod.rs
cat -n dash-spv/src/storage/mod.rsRepository: dashpay/rust-dashcore
Length of output: 11658
🏁 Script executed:
# Check if MemoryStorageManager is defined anywhere
fd -e rs . dash-spv/src/storage | xargs grep -l "struct MemoryStorageManager" 2>/dev/null || echo "No MemoryStorageManager struct found"
# Look for StorageManager trait definition and implementations
rg -n "trait StorageManager" --type rust -A 5Repository: dashpay/rust-dashcore
Length of output: 533
🏁 Script executed:
# Check test files and their storage usage
fd -e rs . dash-spv --path "*/tests/*" -o --path "*test*" | head -20
rg -n "DiskStorageManager::new_tmp" --type rust -B 2 -A 2Repository: dashpay/rust-dashcore
Length of output: 4607
Remove MemoryStorageManager implementation entirely rather than just from exports.
The codebase currently exports only DiskStorageManager as the StorageManager implementation, with MemoryStorageManager completely removed. This contradicts the architectural guidance that the storage module should provide both MemoryStorageManager and DiskStorageManager as swappable trait implementations.
While tests have been successfully updated to use DiskStorageManager::new_tmp(), this architectural change removes the flexibility for true in-memory storage. Consider whether this simplified approach aligns with long-term maintainability and testing requirements, or if a dual-implementation design should be restored to maintain the trait-based abstraction pattern documented in the guidelines.
🤖 Prompt for AI Agents
In dash-spv/src/storage/mod.rs around line 16, the MemoryStorageManager type was
removed from the module exports leaving only DiskStorageManager, which breaks
the intended dual-implementation trait abstraction; restore MemoryStorageManager
by reintroducing its module and pub export (pub use
memory::MemoryStorageManager) and ensure the StorageManager trait is implemented
for both MemoryStorageManager and DiskStorageManager, update mod declarations to
include memory, and run/adjust tests to use the trait abstraction or provide
MemoryStorageManager::new_tmp() for in-memory test usage.
| let mut storage = | ||
| DiskStorageManager::new(TempDir::new().expect("Failed to create tmp dir").path().into()) | ||
| .await | ||
| .expect("Failed to create tmp storage"); |
There was a problem hiding this comment.
Critical: TempDir lifetime bug causes premature directory deletion.
The TempDir is created inline and immediately dropped after calling .path().into(), deleting the temporary directory before DiskStorageManager can use it. This will cause test failures.
Apply this fix to keep the TempDir alive for the test duration:
- let mut storage =
- DiskStorageManager::new(TempDir::new().expect("Failed to create tmp dir").path().into())
- .await
- .expect("Failed to create tmp storage");
+ let temp_dir = TempDir::new().expect("Failed to create tmp dir");
+ let mut storage = DiskStorageManager::new(temp_dir.path().to_path_buf())
+ .await
+ .expect("Failed to create tmp storage");See dash-spv/tests/storage_test.rs:42-47 for the correct pattern.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut storage = | |
| DiskStorageManager::new(TempDir::new().expect("Failed to create tmp dir").path().into()) | |
| .await | |
| .expect("Failed to create tmp storage"); | |
| let temp_dir = TempDir::new().expect("Failed to create tmp dir"); | |
| let mut storage = DiskStorageManager::new(temp_dir.path().to_path_buf()) | |
| .await | |
| .expect("Failed to create tmp storage"); |
🤖 Prompt for AI Agents
In dash-spv/tests/reverse_index_test.rs around lines 57 to 60, the TempDir is
created inline and dropped immediately causing the temp directory to be deleted
before DiskStorageManager uses it; fix by creating a TempDir variable bound to a
local name (e.g., let tmp_dir = TempDir::new().expect(...);), pass
tmp_dir.path().into() to DiskStorageManager::new, and keep tmp_dir in scope for
the duration of the test so it is not dropped prematurely (follow the pattern
used in dash-spv/tests/storage_test.rs:42-47).
dash-spv/examples/filter_sync.rs
Outdated
|
|
||
| // Create storage manager | ||
| let storage_manager = MemoryStorageManager::new().await?; | ||
| let storage_manager = DiskStorageManager::new("./.tmp/example-storage".into()).await?; |
There was a problem hiding this comment.
Should probably use the proper temp dir constructor here and probably good to log the temp dir then.
There was a problem hiding this comment.
The constructor is only available in the unit test since it is marked with cfg(test), tempdir is a dev-dependency and i didnt want to modify it
There was a problem hiding this comment.
Okay then at least have a separate directory for each example. There is also the third "with wallet" example that could be aligned with those changes but thats a separate issue i guess.
dash-spv/src/storage/manager.rs
Outdated
| } | ||
|
|
||
| #[cfg(test)] | ||
| pub async fn new_tmp() -> StorageResult<Self> { |
There was a problem hiding this comment.
| pub async fn new_tmp() -> StorageResult<Self> { | |
| pub async fn with_temp_dir() -> StorageResult<Self> { |
Maybe? Also, shouldn't we also store the actual temp_dir object then within the manager to ensure it stays alive?
From the docs:
A directory in the filesystem that is automatically deleted when it goes out of scope.
The TempDir type creates a directory on the file system that is deleted once it goes out of scope. At
construction, the TempDir creates a new directory with a randomly generated name.
There was a problem hiding this comment.
The TempDir in dropped before the object gets constructed so the directory has not being used yet, no data loss. I can see the name change
There was a problem hiding this comment.
Hm i see, so basically it just re-creates the dir then after it gets dropped.
There was a problem hiding this comment.
my segment struct akways checks that the dir exists and if not creates one. have to refacor the reverse index and do teh same if it is not being done
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
dash-spv/src/storage/manager.rs (1)
152-158: Critical: TempDir dropped before directory is used, causing test failures.This is the same issue already flagged in previous reviews. The
TempDiris dropped immediately after extracting the path, which causes the directory to be deleted beforeSelf::new()attempts to use it. The response that "the directory has not been used yet" misunderstands the problem:Self::new()at line 64 will try to create subdirectories and initialize storage in a directory that no longer exists.Run the following script to verify whether tests using
with_temp_dir()are actually passing:#!/bin/bash # Verify that tests using DiskStorageManager::with_temp_dir() pass # Find all test files that call with_temp_dir rg -n "with_temp_dir\(\)" --type=rust -g '*test*.rs' # Run the specific test file to see if it passes cargo test --package dash-spv --lib client::block_processor_test -- --nocaptureThe fix should either:
- Return
(DiskStorageManager, TempDir)so the caller owns the TempDir- Store the TempDir in DiskStorageManager as a field
- Intentionally leak the TempDir with
std::mem::forget()for test scenariosdash-spv/src/client/block_processor_test.rs (1)
309-311: Same concern: Dependency on buggywith_temp_dir().This test also depends on the buggy
DiskStorageManager::with_temp_dir()implementation. See the review comment on lines 94-106 for details. Once the bug inmanager.rsis fixed, this usage will also be resolved.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
dash-spv/src/chain/chainlock_test.rs(3 hunks)dash-spv/src/client/block_processor_test.rs(3 hunks)dash-spv/src/client/message_handler_test.rs(1 hunks)dash-spv/src/client/mod.rs(3 hunks)dash-spv/src/storage/manager.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- dash-spv/src/chain/chainlock_test.rs
- dash-spv/src/client/message_handler_test.rs
- dash-spv/src/client/mod.rs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/client/block_processor_test.rsdash-spv/src/storage/manager.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Use trait-based abstractions for core components likeNetworkManagerandStorageManagerto enable swappable implementations
Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles withcargo check --all-features
Files:
dash-spv/src/client/block_processor_test.rsdash-spv/src/storage/manager.rs
dash-spv/src/client/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Client module should provide high-level API through
DashSpvClientand configuration viaClientConfig
Files:
dash-spv/src/client/block_processor_test.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Storage module should provide abstraction via
StorageManagertrait with concrete implementations forMemoryStorageManagerandDiskStorageManager
Files:
dash-spv/src/storage/manager.rs
🧠 Learnings (19)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
Applied to files:
dash-spv/src/client/block_processor_test.rsdash-spv/src/storage/manager.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use trait-based abstractions for core components like `NetworkManager` and `StorageManager` to enable swappable implementations
Applied to files:
dash-spv/src/client/block_processor_test.rsdash-spv/src/storage/manager.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should be organized in `tests/` directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration
Applied to files:
dash-spv/src/client/block_processor_test.rsdash-spv/src/storage/manager.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/wallet/**/*.rs : Wallet module should provide UTXO tracking via `Utxo` struct, balance calculation with confirmation states, and transaction processing via `TransactionProcessor`
Applied to files:
dash-spv/src/client/block_processor_test.rsdash-spv/src/storage/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/src/client/block_processor_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/src/client/block_processor_test.rsdash-spv/src/storage/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/block_processor_test.rsdash-spv/src/storage/manager.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Applied to files:
dash-spv/src/client/block_processor_test.rsdash-spv/src/storage/manager.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Synchronization should use `SyncManager` for phase-based coordination: Phase 1 (Headers), Phase 2 (Masternode List), Phase 3 (Filter Headers), Phase 4 (Filters), Phase 5 (Blocks)
Applied to files:
dash-spv/src/client/block_processor_test.rsdash-spv/src/storage/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/block_processor_test.rsdash-spv/src/storage/manager.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Cover critical parsing, networking, SPV, and wallet flows with tests; add regression tests for fixes; consider property tests with `proptest`
Applied to files:
dash-spv/src/client/block_processor_test.rs
📚 Learning: 2025-12-01T07:59:46.015Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:46.015Z
Learning: Applies to **/tests/**/*.rs : Test both mainnet and testnet configurations
Applied to files:
dash-spv/src/client/block_processor_test.rs
📚 Learning: 2025-12-01T07:59:46.015Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:46.015Z
Learning: Applies to **/tests/**/*.rs : Write integration tests for network operations in Rust
Applied to files:
dash-spv/src/client/block_processor_test.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)
Applied to files:
dash-spv/src/client/block_processor_test.rsdash-spv/src/storage/manager.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/network/**/*.rs : Network module should handle TCP connections, handshake management, message routing, and peer management following Dash protocol implementation
Applied to files:
dash-spv/src/client/block_processor_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/src/**/*.rs : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function
Applied to files:
dash-spv/src/storage/manager.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/error.rs : Maintain domain-specific error types in `error.rs` with variants for `NetworkError`, `StorageError`, `SyncError`, `ValidationError`, and `SpvError` (top-level wrapper)
Applied to files:
dash-spv/src/storage/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/storage/manager.rs
🧬 Code graph analysis (1)
dash-spv/src/storage/manager.rs (2)
dash-spv/tests/reverse_index_test.rs (1)
storage(35-35)dash-spv/src/storage/headers.rs (2)
headers(21-21)load_block_index(55-66)
⏰ 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: SPV Components Tests
- GitHub Check: Key Wallet Components Tests
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: Pre-commit (ubuntu-latest)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: fuzz (hashes_json)
🔇 Additional comments (2)
dash-spv/src/storage/manager.rs (1)
12-13: LGTM: Import path refactoring aligns with module restructuring.The import paths have been updated to reflect the removal of the intermediate
diskmodule layer, which is consistent with the PR's goal to clean up storage organization.dash-spv/src/client/block_processor_test.rs (1)
7-7: LGTM: Import updated to use DiskStorageManager.The import change aligns with the PR's objective to remove in-memory storage and standardize on disk-backed storage.
| BlockProcessor<MockWallet, DiskStorageManager>, | ||
| mpsc::UnboundedSender<BlockProcessingTask>, | ||
| mpsc::UnboundedReceiver<SpvEvent>, | ||
| Arc<RwLock<MockWallet>>, | ||
| Arc<Mutex<MemoryStorageManager>>, | ||
| Arc<Mutex<DiskStorageManager>>, | ||
| ) { | ||
| let (task_tx, task_rx) = mpsc::unbounded_channel(); | ||
| let (event_tx, event_rx) = mpsc::unbounded_channel(); | ||
| let stats = Arc::new(RwLock::new(SpvStats::default())); | ||
| let wallet = Arc::new(RwLock::new(MockWallet::new())); | ||
| let storage = Arc::new(Mutex::new(MemoryStorageManager::new().await.unwrap())); | ||
| let storage = Arc::new(Mutex::new( | ||
| DiskStorageManager::with_temp_dir().await.expect("Failed to create tmp storage"), | ||
| )); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "block_processor_test.rs" 2>/dev/null | head -5Repository: dashpay/rust-dashcore
Length of output: 47
🏁 Script executed:
cd /tmp && find . -name "manager.rs" -path "*/storage/*" 2>/dev/null | head -5Repository: dashpay/rust-dashcore
Length of output: 47
🏁 Script executed:
# Search for block_processor_test.rs from repo root
git ls-files | grep -E "block_processor.*test"Repository: dashpay/rust-dashcore
Length of output: 108
🏁 Script executed:
# Search for DiskStorageManager and with_temp_dir implementation
rg "with_temp_dir" --type rust -A 10Repository: dashpay/rust-dashcore
Length of output: 8533
🏁 Script executed:
# Also search for DiskStorageManager struct definition
rg "struct DiskStorageManager" --type rust -A 5Repository: dashpay/rust-dashcore
Length of output: 498
🏁 Script executed:
cat -n dash-spv/src/client/block_processor_test.rs | head -120Repository: dashpay/rust-dashcore
Length of output: 5000
🏁 Script executed:
cat -n dash-spv/src/storage/manager.rs | head -80Repository: dashpay/rust-dashcore
Length of output: 3300
🏁 Script executed:
# Look at the full with_temp_dir implementation and surrounding code
rg "pub async fn with_temp_dir" -A 20 dash-spv/src/storage/manager.rsRepository: dashpay/rust-dashcore
Length of output: 834
🏁 Script executed:
# Check if TempDir is stored anywhere in DiskStorageManager
rg "TempDir" --type rust dash-spv/src/storage/Repository: dashpay/rust-dashcore
Length of output: 2135
🏁 Script executed:
cd dash-spv && cargo test --lib client::block_processor_test::tests --no-fail-fast 2>&1 | head -100Repository: dashpay/rust-dashcore
Length of output: 174
🏁 Script executed:
# Also try running the specific test with more details
cd dash-spv && cargo test --lib client::block_processor_test::tests::test_process_block -- --nocapture 2>&1Repository: dashpay/rust-dashcore
Length of output: 174
🏁 Script executed:
# Check if tests pass overall
cd dash-spv && cargo test --lib client::block_processor_test 2>&1 | tail -50Repository: dashpay/rust-dashcore
Length of output: 174
🏁 Script executed:
# Search for any TODO or comments about the temp_dir bug
rg "TODO|FIXME|BUG" dash-spv/src/storage/ --type rust -B 2 -A 2Repository: dashpay/rust-dashcore
Length of output: 698
🏁 Script executed:
# Look at the complete with_temp_dir implementation and what happens to TempDir
cat -n dash-spv/src/storage/manager.rs | sed -n '100,140p'Repository: dashpay/rust-dashcore
Length of output: 2112
🏁 Script executed:
# Check if TempDir is persisted anywhere or if there's a comment about the bug
rg "TempDir" -B 3 -A 3 dash-spv/src/storage/manager.rsRepository: dashpay/rust-dashcore
Length of output: 267
🏁 Script executed:
# Search for any recent changes or concerns about with_temp_dir
git log --oneline --all -- "dash-spv/src/storage/manager.rs" | head -10Repository: dashpay/rust-dashcore
Length of output: 115
🏁 Script executed:
# Look at DiskStorageManager struct definition completely to see all fields
rg "pub struct DiskStorageManager" -A 25 dash-spv/src/storage/manager.rsRepository: dashpay/rust-dashcore
Length of output: 1023
🏁 Script executed:
# Check if TempDir might be stored in LockFile or any Arc that persists
rg "struct LockFile" -A 10 dash-spv/src/storage/lockfile.rsRepository: dashpay/rust-dashcore
Length of output: 400
🏁 Script executed:
# Verify the complete chain of what happens after with_temp_dir returns
cat -n dash-spv/src/storage/manager.rs | sed -n '127,155p'Repository: dashpay/rust-dashcore
Length of output: 1308
🏁 Script executed:
# Check if there are any comments about this being intentional
rg "with_temp_dir\|TempDir" -B 5 -A 5 dash-spv/src/storage/manager.rs | head -40Repository: dashpay/rust-dashcore
Length of output: 47
🏁 Script executed:
# Look at how storage is used in actual tests after setup_processor
cat -n dash-spv/src/client/block_processor_test.rs | sed -n '119,160p'Repository: dashpay/rust-dashcore
Length of output: 1947
🏁 Script executed:
# Check if storage operations are deferred or happen immediately
rg "load_or_new\|create_dir" dash-spv/src/storage/manager.rs -B 2 -A 2 | head -50Repository: dashpay/rust-dashcore
Length of output: 47
🏁 Script executed:
# Look at what SegmentCache::load_or_new does during initialization
rg "pub async fn load_or_new" dash-spv/src/storage/segments.rs -A 15Repository: dashpay/rust-dashcore
Length of output: 582
🏁 Script executed:
# Check when files are actually accessed - during new() or lazily
cat -n dash-spv/src/storage/manager.rs | sed -n '64,90p'Repository: dashpay/rust-dashcore
Length of output: 1488
🏁 Script executed:
# See if there's documentation about the temp behavior
grep -r "with_temp_dir" dash-spv/ --include="*.rs" -B 2 -A 2 | grep -E "///|//.*temp|//.*dir" | head -20Repository: dashpay/rust-dashcore
Length of output: 47
🏁 Script executed:
# Check the complete implementation of with_temp_dir and what happens to the temp_dir variable
cat -n dash-spv/src/storage/manager.rs | sed -n '152,160p'Repository: dashpay/rust-dashcore
Length of output: 365
🏁 Script executed:
# Verify that DiskStorageManager only stores base_path, not the TempDir
rg "pub struct DiskStorageManager" -A 30 dash-spv/src/storage/manager.rs | grep -E "TempDir|_temp"Repository: dashpay/rust-dashcore
Length of output: 47
🏁 Script executed:
# Check if there's any async behavior that might defer the TempDir drop
rg "with_temp_dir" -B 2 -A 10 dash-spv/src/client/block_processor_test.rs | head -25Repository: dashpay/rust-dashcore
Length of output: 934
🏁 Script executed:
# Look for any comments in tests indicating workarounds or known issues
rg "TODO|FIXME|BUG|HACK" dash-spv/src/client/block_processor_test.rsRepository: dashpay/rust-dashcore
Length of output: 47
🏁 Script executed:
# Check if there's a wrapper or if TempDir is stored elsewhere in Arc
rg "TempDir\|temp_dir" dash-spv/src/storage/ --type rustRepository: dashpay/rust-dashcore
Length of output: 47
🏁 Script executed:
# Look at how tempfile::TempDir behaves - specifically if there's any way it could persist
rg "tempfile" dash-spv/Cargo.toml -A 2Repository: dashpay/rust-dashcore
Length of output: 120
🏁 Script executed:
# Check if there are any other usages of with_temp_dir that might give hints
rg "with_temp_dir" --type rust -B 3 -A 8Repository: dashpay/rust-dashcore
Length of output: 9290
🏁 Script executed:
# See if maybe TempDir is stored in some Arc somewhere up the chain
cat -n dash-spv/src/client/block_processor.rs | head -50Repository: dashpay/rust-dashcore
Length of output: 2028
🏁 Script executed:
# Verify the complete lifecycle - let's trace if TempDir is somehow stored or if the path persists
cat -n dash-spv/src/storage/manager.rs | sed -n '153,158p'Repository: dashpay/rust-dashcore
Length of output: 290
🏁 Script executed:
# Check tempfile documentation behavior
python3 << 'EOF'
# Explain tempfile::TempDir behavior
print("tempfile::TempDir behavior:")
print("- TempDir::new() creates a temporary directory")
print("- When TempDir is dropped, it deletes the directory automatically")
print("- TempDir is NOT automatically persisted")
print("- Passing path via .into() creates a PathBuf (just a path string)")
print("- After with_temp_dir() returns, TempDir goes out of scope and directory is deleted")
EOFRepository: dashpay/rust-dashcore
Length of output: 151
Store TempDir in DiskStorageManager to prevent premature directory cleanup.
The with_temp_dir() implementation creates a tempfile::TempDir, extracts its path, and returns a DiskStorageManager initialized with only the path. The TempDir is immediately dropped after the function returns, which automatically deletes the temporary directory. Since DiskStorageManager only stores the base_path as a PathBuf, not the TempDir handle itself, the directory will be cleaned up while the storage is still in use, causing operations to fail with directory-not-found errors.
Add a _temp_dir: Option<tempfile::TempDir> field to DiskStorageManager to keep the TempDir alive for the storage lifetime.
🤖 Prompt for AI Agents
In dash-spv/src/client/block_processor_test.rs around lines 94 to 106, the test
calls DiskStorageManager::with_temp_dir() which currently creates a
tempfile::TempDir, extracts and stores only its path, then drops the TempDir
causing the temp directory to be deleted while tests still use it; to fix, add a
field _temp_dir: Option<tempfile::TempDir> to the DiskStorageManager struct and
update with_temp_dir() to store the TempDir in that field (Some(tempdir))
instead of dropping it, ensuring the TempDir handle lives as long as the
DiskStorageManager instance; also adjust any constructors, trait impls, or clone
behavior as needed to accommodate the new Option field.
|
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
added
the
merge-conflict
github-actions
bot
removed
the
merge-conflict
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/examples/spv_with_wallet.rs (1)
21-21: Remove the redundantconfig.storage_pathassignment or use it to createDiskStorageManager.Line 21 sets
config.storage_path = Some("/tmp/dash-spv-example".into()), but this field is never used. Instead,DiskStorageManageris initialized on lines 29-30 with a different path"./.tmp/spv-with-wallet-example-storage". This is inconsistent with the main application pattern whereconfig.storage_pathis used to create storage. Either remove line 21 or refactor to useconfig.storage_pathforDiskStorageManagerinitialization.Additionally, the relative path
./.tmp/depends on the current working directory and may fail if the example is run from different locations.
♻️ Duplicate comments (2)
dash-spv/src/storage/mod.rs (1)
24-24: Public API consolidation is appropriate.The
StorageManagertrait is preserved (lines 107-246), maintaining the abstraction layer for potential future storage implementations. The architectural decision to removeMemoryStorageManagerwas already flagged in a previous review—this simplification is intentional per the PR objectives.dash-spv/tests/reverse_index_test.rs (1)
54-57: Critical: TempDir lifetime bug causes immediate directory deletion.The
TempDiris created inline and dropped immediately after.path().into()completes, causing the temporary directory to be deleted beforeDiskStorageManagercan initialize. This will cause test failures becauseDiskStorageManager::new()needs the directory to exist.Apply this fix to keep the
TempDiralive for the test duration:- let mut storage = - DiskStorageManager::new(TempDir::new().expect("Failed to create tmp dir").path().into()) - .await - .expect("Failed to create tmp storage"); + let temp_dir = TempDir::new().expect("Failed to create tmp dir"); + let mut storage = DiskStorageManager::new(temp_dir.path().to_path_buf()) + .await + .expect("Failed to create tmp storage");See the correct pattern at lines 9-13 in the same file.
🧹 Nitpick comments (1)
dash-spv/examples/simple_sync.rs (1)
27-28: Consider adding cleanup or documenting storage persistence.The example uses a fixed path and doesn't clean up the storage directory after completion. This leaves persistent data in
./.tmp/simple-sync-example-storagebetween runs.This may be intentional (to allow inspection of stored data or resume sync), but consider either:
- Adding a comment explaining this behavior, or
- Using
tempfile::TempDirfor automatic cleanup if persistence isn't needed+ // Note: Storage persists in ./.tmp/simple-sync-example-storage between runs. + // Delete this directory to start fresh. let storage_manager = DiskStorageManager::new("./.tmp/simple-sync-example-storage".into()).await?;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
dash-spv/examples/filter_sync.rs(2 hunks)dash-spv/examples/simple_sync.rs(2 hunks)dash-spv/examples/spv_with_wallet.rs(1 hunks)dash-spv/src/storage/mod.rs(3 hunks)dash-spv/src/storage/state.rs(1 hunks)dash-spv/tests/reverse_index_test.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- dash-spv/examples/filter_sync.rs
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/examples/simple_sync.rsdash-spv/examples/spv_with_wallet.rsdash-spv/src/storage/mod.rsdash-spv/tests/reverse_index_test.rsdash-spv/src/storage/state.rs
dash-spv/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
dash-spv/**/*.rs: Use trait-based abstractions for core components likeNetworkManagerandStorageManagerto enable swappable implementations
Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles withcargo check --all-features
Files:
dash-spv/examples/simple_sync.rsdash-spv/examples/spv_with_wallet.rsdash-spv/src/storage/mod.rsdash-spv/tests/reverse_index_test.rsdash-spv/src/storage/state.rs
dash-spv/src/storage/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Storage module should provide abstraction via
StorageManagertrait with concrete implementations forMemoryStorageManagerandDiskStorageManager
Files:
dash-spv/src/storage/mod.rsdash-spv/src/storage/state.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/reverse_index_test.rs
dash-spv/tests/**/*.rs
📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)
Integration tests should be organized in
tests/directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration
Files:
dash-spv/tests/reverse_index_test.rs
🧠 Learnings (27)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`
Applied to files:
dash-spv/examples/simple_sync.rsdash-spv/examples/spv_with_wallet.rsdash-spv/src/storage/mod.rsdash-spv/tests/reverse_index_test.rsdash-spv/src/storage/state.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use trait-based abstractions for core components like `NetworkManager` and `StorageManager` to enable swappable implementations
Applied to files:
dash-spv/examples/simple_sync.rsdash-spv/examples/spv_with_wallet.rsdash-spv/src/storage/mod.rsdash-spv/tests/reverse_index_test.rsdash-spv/src/storage/state.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/examples/simple_sync.rsdash-spv/src/storage/mod.rsdash-spv/tests/reverse_index_test.rsdash-spv/src/storage/state.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Applied to files:
dash-spv/examples/simple_sync.rsdash-spv/examples/spv_with_wallet.rsdash-spv/src/storage/mod.rsdash-spv/tests/reverse_index_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Synchronization should use `SyncManager` for phase-based coordination: Phase 1 (Headers), Phase 2 (Masternode List), Phase 3 (Filter Headers), Phase 4 (Filters), Phase 5 (Blocks)
Applied to files:
dash-spv/examples/simple_sync.rsdash-spv/src/storage/mod.rsdash-spv/tests/reverse_index_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/src/**/*.rs : Use thread-local storage for error propagation via `dash_spv_ffi_get_last_error()` function
Applied to files:
dash-spv/examples/simple_sync.rsdash-spv/examples/spv_with_wallet.rsdash-spv/src/storage/mod.rsdash-spv/tests/reverse_index_test.rsdash-spv/src/storage/state.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/error.rs : Maintain domain-specific error types in `error.rs` with variants for `NetworkError`, `StorageError`, `SyncError`, `ValidationError`, and `SpvError` (top-level wrapper)
Applied to files:
dash-spv/examples/simple_sync.rsdash-spv/examples/spv_with_wallet.rsdash-spv/src/storage/mod.rsdash-spv/tests/reverse_index_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should be organized in `tests/` directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration
Applied to files:
dash-spv/examples/simple_sync.rsdash-spv/examples/spv_with_wallet.rsdash-spv/tests/reverse_index_test.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/examples/simple_sync.rsdash-spv/src/storage/mod.rsdash-spv/tests/reverse_index_test.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles with `cargo check --all-features`
Applied to files:
dash-spv/examples/simple_sync.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/network/**/*.rs : Network module should handle TCP connections, handshake management, message routing, and peer management following Dash protocol implementation
Applied to files:
dash-spv/examples/simple_sync.rsdash-spv/examples/spv_with_wallet.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 : Enable mempool tracking on SPV connection using the appropriate strategy (.fetchAll, .bloomFilter, .selective)
Applied to files:
dash-spv/examples/simple_sync.rsdash-spv/examples/spv_with_wallet.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/client/**/*.rs : Client module should provide high-level API through `DashSpvClient` and configuration via `ClientConfig`
Applied to files:
dash-spv/examples/simple_sync.rsdash-spv/src/storage/mod.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/wallet/**/*.rs : Wallet module should provide UTXO tracking via `Utxo` struct, balance calculation with confirmation states, and transaction processing via `TransactionProcessor`
Applied to files:
dash-spv/examples/spv_with_wallet.rsdash-spv/src/storage/mod.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/examples/spv_with_wallet.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{bip32,derivation,account}/**/*.rs : Use `ExtendedPubKey` and `ExtendedPrivKey` from the BIP32 implementation for key derivation; avoid direct private key exposure or serialization in logs
Applied to files:
dash-spv/examples/spv_with_wallet.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)
Applied to files:
dash-spv/src/storage/mod.rsdash-spv/src/storage/state.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/*.rs : Use `?` operator for error propagation in Rust code to maintain clean error handling patterns
Applied to files:
dash-spv/src/storage/mod.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/src/storage/mod.rsdash-spv/tests/reverse_index_test.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{account,managed_account}/**/*.rs : Separate immutable and mutable concerns: Use `Account` for immutable identity information (keys, derivation paths) and `ManagedAccount` for mutable state (address pools, metadata, balances)
Applied to files:
dash-spv/src/storage/mod.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.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/storage/mod.rsdash-spv/tests/reverse_index_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/reverse_index_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/reverse_index_test.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{bip32,derivation}/**/*.rs : Cache intermediate derivation results and use hardened derivation only when required; batch derive child keys when possible to optimize derivation performance
Applied to files:
dash-spv/tests/reverse_index_test.rs
📚 Learning: 2025-02-25T06:13:52.858Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 51
File: dash/src/sml/masternode_list/scores_for_quorum.rs:50-73
Timestamp: 2025-02-25T06:13:52.858Z
Learning: ScoreHash is a cryptographic hash in the rust-dashcore library, and therefore does not need collision handling when used as a key in collections like BTreeMap due to the extremely low probability of collisions.
Applied to files:
dash-spv/src/storage/state.rs
🧬 Code graph analysis (2)
dash-spv/examples/simple_sync.rs (1)
dash-spv/src/client/core.rs (1)
storage(169-171)
dash-spv/tests/reverse_index_test.rs (1)
dash-spv/src/storage/manager.rs (1)
new(64-150)
⏰ 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: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Core Components Tests
- GitHub Check: Key Wallet Components Tests
- GitHub Check: SPV Components Tests
- GitHub Check: Pre-commit (macos-latest)
- GitHub Check: Pre-commit (ubuntu-latest)
🔇 Additional comments (7)
dash-spv/src/storage/state.rs (1)
11-11: LGTM!The import path update correctly reflects the module restructuring where
WorkerCommandis now exposed fromcrate::storage::managerinstead of the removedcrate::storage::disk::managerpath.dash-spv/src/storage/mod.rs (2)
3-13: Module restructuring is clean and consistent.The internal modules are now properly organized with appropriate visibility:
pub(crate) mod iofor internal I/O utilities- Private modules (
headers,lockfile,manager,segments,state) encapsulate implementation detailsThis structure correctly hides implementation details while exposing only the necessary public interface.
82-86: Documentation example is correct and demonstrates the updated API.The example properly shows the new usage pattern with
DiskStorageManager::new().dash-spv/examples/simple_sync.rs (1)
4-4: LGTM!Import correctly updated to use
DiskStorageManagerfrom the new module path.dash-spv/tests/reverse_index_test.rs (3)
1-5: LGTM: Import changes align with disk-backed storage migration.The imports correctly reflect the removal of
MemoryStorageManagerand the addition ofTempDirfor temporary directory management in disk-backed tests.
7-50: LGTM: Correct TempDir lifetime management and comprehensive persistence testing.The test properly manages the
TempDirlifetime by binding it totemp_dirat line 9, ensuring the temporary directory remains available throughout the test. The two-phase test design (initial operations + reload verification) effectively validates both immediate functionality and persistence across storage restarts.
73-87: LGTM: Well-designed deterministic test helper.The helper function creates unique, deterministic headers by encoding the index into the
prev_blockhashand other fields, ensuring reproducible test behavior.
| @@ -1,34 +1,8 @@ | |||
| use dash_spv::storage::{DiskStorageManager, MemoryStorageManager, StorageManager}; | |||
| use dash_spv::storage::{DiskStorageManager, StorageManager}; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if MemoryStorageManager exists in the codebase
echo "=== Searching for MemoryStorageManager references ==="
rg -n 'MemoryStorageManager' --type rust 2>/dev/null || echo "No MemoryStorageManager found"
echo -e "\n=== Checking StorageManager trait implementations ==="
rg -n 'impl.*StorageManager' --type rust 2>/dev/null | head -20 || echo "No StorageManager impls found"
echo -e "\n=== Searching for DiskStorageManager ==="
rg -n 'DiskStorageManager' --type rust 2>/dev/null | head -20
echo -e "\n=== Checking storage module structure ==="
fd -e rs 'storage' dash-spv/src/ 2>/dev/null | head -20Repository: dashpay/rust-dashcore
Length of output: 4591
Remove MemoryStorageManager dependency from tests—architecture now disk-only.
This PR removes MemoryStorageManager entirely, leaving only DiskStorageManager as the storage backend. This contradicts the documented architectural design: the StorageManager trait should support multiple concrete implementations for swappable storage backends. Removing the in-memory implementation eliminates fast, filesystem-independent testing and reduces trait abstraction value.
This represents a significant architectural shift. If disk-only storage is intentional, document this decision and verify all tests can tolerate filesystem I/O dependencies. Consider whether maintaining MemoryStorageManager for unit tests would improve test performance and flexibility.
🤖 Prompt for AI Agents
In dash-spv/tests/reverse_index_test.rs around lines 1 to 1, the test suite
still assumes a swappable StorageManager implementation but the in-memory
implementation was removed; restore test portability by either (A) reintroducing
a lightweight MemoryStorageManager implementation (in crate tests or a test-only
module) that implements StorageManager and use it in unit tests to avoid
filesystem I/O, or (B) if disk-only is intentional, update all tests to
explicitly construct DiskStorageManager using temporary directories (e.g.,
tempfile) and add a top-level comment/documentation explaining the disk-only
decision; ensure imports and Cargo features are updated accordingly so tests
compile and run without relying on the deleted MemoryStorageManager.
Summary by CodeRabbit
Breaking Changes
MemoryStorageManageris no longer publicly available; replaced withDiskStorageManager.New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.