Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions dash-spv/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,8 @@ The `SyncManager` coordinates all synchronization through a phase-based approach
Each phase must complete before the next begins, ensuring consistency and simplifying error recovery.

### Storage Backends
Two storage implementations via the `StorageManager` trait:
- `MemoryStorageManager`: In-memory storage for testing
- `DiskStorageManager`: Persistent disk storage for production
One storage implementation via the `StorageManager` trait:
- `DiskStorageManager`: Persistent disk storage

### Network Layer
TCP-based networking with proper Dash protocol implementation:
Expand Down
2 changes: 1 addition & 1 deletion dash-spv/CODE_ANALYSIS_SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@
**Test Types:**
- **Unit Tests**: Individual functions/modules (in-file with `#[cfg(test)]`)
- **Integration Tests**: Cross-module interactions (`tests/` directory)
- **Mock Tests**: Use MockNetworkManager, MemoryStorageManager
- **Mock Tests**: Use MockNetworkManager, DiskStorageManager::new_tmp
- **Property Tests**: Invariant testing (could add more with proptest)

---
Expand Down
4 changes: 2 additions & 2 deletions dash-spv/examples/filter_sync.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! BIP157 filter synchronization example.

use dash_spv::network::PeerNetworkManager;
use dash_spv::storage::MemoryStorageManager;
use dash_spv::storage::DiskStorageManager;
use dash_spv::{init_console_logging, ClientConfig, DashSpvClient, LevelFilter};
use dashcore::Address;
use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo;
Expand All @@ -28,7 +28,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
let network_manager = PeerNetworkManager::new(&config).await?;

// Create storage manager
let storage_manager = MemoryStorageManager::new().await?;
let storage_manager = DiskStorageManager::new("./.tmp/example-storage".into()).await?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably use the proper temp dir constructor here and probably good to log the temp dir then.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// Create wallet manager
let wallet = Arc::new(RwLock::new(WalletManager::<ManagedWalletInfo>::new()));
Expand Down
4 changes: 2 additions & 2 deletions dash-spv/examples/simple_sync.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Simple header synchronization example.

use dash_spv::network::PeerNetworkManager;
use dash_spv::storage::MemoryStorageManager;
use dash_spv::storage::DiskStorageManager;
use dash_spv::{init_console_logging, ClientConfig, DashSpvClient, LevelFilter};
use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo;

Expand All @@ -24,7 +24,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
let network_manager = PeerNetworkManager::new(&config).await?;

// Create storage manager
let storage_manager = MemoryStorageManager::new().await?;
let storage_manager = DiskStorageManager::new("./.tmp/example-storage".into()).await?;

// Create wallet manager
let wallet = Arc::new(RwLock::new(WalletManager::<ManagedWalletInfo>::new()));
Expand Down
9 changes: 4 additions & 5 deletions dash-spv/src/chain/chainlock_test.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
#[cfg(test)]
mod tests {
use super::super::*;
use crate::storage::MemoryStorageManager;
use crate::types::ChainState;
use crate::{storage::DiskStorageManager, types::ChainState};
use dashcore::{BlockHash, ChainLock, Network};
use dashcore_hashes::Hash;

#[tokio::test]
async fn test_chainlock_processing() {
// Create storage and ChainLock manager
let mut storage =
MemoryStorageManager::new().await.expect("Failed to create memory storage");
DiskStorageManager::new_tmp().await.expect("Failed to create tmp storage");
let chainlock_manager = ChainLockManager::new(true);
let chain_state = ChainState::new_for_network(Network::Testnet);

Expand Down Expand Up @@ -43,7 +42,7 @@ mod tests {
#[tokio::test]
async fn test_chainlock_superseding() {
let mut storage =
MemoryStorageManager::new().await.expect("Failed to create memory storage");
DiskStorageManager::new_tmp().await.expect("Failed to create tmp storage");
let chainlock_manager = ChainLockManager::new(true);
let chain_state = ChainState::new_for_network(Network::Testnet);

Expand Down Expand Up @@ -83,7 +82,7 @@ mod tests {
let chainlock_manager = ChainLockManager::new(true);
let chain_state = ChainState::new_for_network(Network::Testnet);
let mut storage =
MemoryStorageManager::new().await.expect("Failed to create memory storage");
DiskStorageManager::new_tmp().await.expect("Failed to create tmp storage");

// Add ChainLocks at heights 1000, 2000, 3000
for height in [1000, 2000, 3000] {
Expand Down
15 changes: 9 additions & 6 deletions dash-spv/src/client/block_processor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
mod tests {
use crate::client::block_processor::{BlockProcessingTask, BlockProcessor};

use crate::storage::memory::MemoryStorageManager;
use crate::storage::StorageManager;
use crate::storage::DiskStorageManager;
use crate::types::{SpvEvent, SpvStats};
use dashcore::{blockdata::constants::genesis_block, Block, Network, Transaction};

Expand Down Expand Up @@ -92,17 +91,19 @@ mod tests {
}

async fn setup_processor() -> (
BlockProcessor<MockWallet, MemoryStorageManager>,
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::new_tmp().await.expect("Failed to create tmp storage"),
));
let processor = BlockProcessor::new(
task_rx,
wallet.clone(),
Expand Down Expand Up @@ -305,7 +306,9 @@ mod tests {
let (event_tx, mut event_rx) = mpsc::unbounded_channel();
let stats = Arc::new(RwLock::new(SpvStats::default()));
let wallet = Arc::new(RwLock::new(NonMatchingWallet {}));
let storage = Arc::new(Mutex::new(MemoryStorageManager::new().await.unwrap()));
let storage = Arc::new(Mutex::new(
DiskStorageManager::new_tmp().await.expect("Failed to create tmp storage"),
));

let processor =
BlockProcessor::new(task_rx, wallet, storage, stats, event_tx, Network::Dash);
Expand Down
4 changes: 2 additions & 2 deletions dash-spv/src/client/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ use super::{BlockProcessingTask, ClientConfig, StatusDisplay};
/// - Essential for a reusable library
///
/// ### 4. **Testing Without Mocks** 🧪
/// - Test implementations (`MockNetworkManager`, `MemoryStorageManager`) are
/// - Test implementations (`MockNetworkManager`) are
/// first-class types, not runtime injections
/// - No conditional compilation or feature flags needed for tests
/// - Type system ensures test and production code are compatible
Expand Down Expand Up @@ -84,7 +84,7 @@ use super::{BlockProcessingTask, ClientConfig, StatusDisplay};
/// type TestSpvClient = DashSpvClient<
/// WalletManager,
/// MockNetworkManager,
/// MemoryStorageManager,
/// DiskStorageManager,
/// >;
/// ```
///
Expand Down
5 changes: 2 additions & 3 deletions dash-spv/src/client/message_handler_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ mod tests {
use crate::mempool_filter::MempoolFilter;
use crate::network::mock::MockNetworkManager;
use crate::network::NetworkManager;
use crate::storage::memory::MemoryStorageManager;
use crate::storage::StorageManager;
use crate::sync::filters::FilterNotificationSender;
use crate::sync::SyncManager;
Expand Down Expand Up @@ -41,11 +40,11 @@ mod tests {
) {
let network = Box::new(MockNetworkManager::new()) as Box<dyn NetworkManager>;
let storage =
Box::new(MemoryStorageManager::new().await.unwrap()) as Box<dyn StorageManager>;
Box::new(DiskStorageManager::new_tmp().await.expect("Failed to create tmp storage")) as Box<dyn StorageManager>;
let config = ClientConfig::default();
let stats = Arc::new(RwLock::new(SpvStats::default()));
let (block_tx, _block_rx) = mpsc::unbounded_channel();
let wallet_storage = Arc::new(RwLock::new(MemoryStorageManager::new().await.unwrap()));
let wallet_storage = Arc::new(RwLock::new(DiskStorageManager::new_tmp().await.expect("Failed to create tmp storage")));
let wallet = Arc::new(RwLock::new(Wallet::new(wallet_storage)));
let mempool_state = Arc::new(RwLock::new(MempoolState::default()));
let (event_tx, _event_rx) = mpsc::unbounded_channel();
Expand Down
7 changes: 3 additions & 4 deletions dash-spv/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,8 @@ mod message_handler_test;
#[cfg(test)]
mod tests {
use super::{ClientConfig, DashSpvClient};
use crate::network::mock::MockNetworkManager;
use crate::storage::MemoryStorageManager;
use crate::types::UnconfirmedTransaction;
use crate::{network::mock::MockNetworkManager, storage::DiskStorageManager};
use dashcore::{Amount, Network, Transaction, TxOut};
use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo;
use key_wallet_manager::wallet_manager::WalletManager;
Expand All @@ -96,7 +95,7 @@ mod tests {
};

let network_manager = MockNetworkManager::new();
let storage = MemoryStorageManager::new().await.expect("memory storage should initialize");
let storage = DiskStorageManager::new_tmp().await.expect("Failed to create tmp storage");
let wallet = Arc::new(RwLock::new(WalletManager::<ManagedWalletInfo>::new()));

let client = DashSpvClient::new(config, network_manager, storage, wallet)
Expand All @@ -123,7 +122,7 @@ mod tests {
};

let network_manager = MockNetworkManager::new();
let storage = MemoryStorageManager::new().await.expect("memory storage should initialize");
let storage = DiskStorageManager::new_tmp().await.expect("Failed to create tmp storage");
let wallet = Arc::new(RwLock::new(WalletManager::<ManagedWalletInfo>::new()));

let mut client = DashSpvClient::new(config, network_manager, storage, wallet)
Expand Down
4 changes: 2 additions & 2 deletions dash-spv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
//! ```no_run
//! use dash_spv::{DashSpvClient, ClientConfig};
//! use dash_spv::network::PeerNetworkManager;
//! use dash_spv::storage::MemoryStorageManager;
//! use dash_spv::storage::DiskStorageManager;
//! use dashcore::Network;
//! use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo;
//! use key_wallet_manager::wallet_manager::WalletManager;
Expand All @@ -30,7 +30,7 @@
//!
//! // Create the required components
//! let network = PeerNetworkManager::new(&config).await?;
//! let storage = MemoryStorageManager::new().await?;
//! let storage = DiskStorageManager::new("./.tmp/example-storage".into()).await?;
//! let wallet = Arc::new(RwLock::new(WalletManager::<ManagedWalletInfo>::new()));
//!
//! // Create and start the client
Expand Down
80 changes: 22 additions & 58 deletions dash-spv/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,65 +307,29 @@ async fn run() -> Result<(), Box<dyn std::error::Error>> {
}
};

// Create and start the client based on storage type
if config.enable_persistence {
if let Some(path) = &config.storage_path {
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);
}
};
run_client(
config,
network_manager,
storage_manager,
wallet,
enable_terminal_ui,
&matches,
wallet_id,
)
.await?;
} 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?;
}
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);
}
};
Comment on lines +310 to +322
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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 2

Repository: 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 2

Repository: 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 -n

Repository: dashpay/rust-dashcore

Length of output: 5974


🏁 Script executed:

# Check DiskStorageManager::new signature
rg -n "impl DiskStorageManager" dash-spv/src/storage -A 10

Repository: 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 20

Repository: 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 2

Repository: 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 -n

Repository: 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.

run_client(
config,
network_manager,
storage_manager,
wallet,
enable_terminal_ui,
&matches,
wallet_id,
)
.await?;

Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion dash-spv/src/network/persist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize};
use std::path::PathBuf;

use crate::error::{SpvError as Error, StorageError};
use crate::storage::disk::io::atomic_write;
use crate::storage::io::atomic_write;

/// Peer persistence for saving and loading known peer addresses
pub struct PeerStore {
Expand Down
2 changes: 1 addition & 1 deletion dash-spv/src/network/reputation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::sync::Arc;
use std::time::{Duration, Instant};
use tokio::sync::RwLock;

use crate::storage::disk::io::atomic_write;
use crate::storage::io::atomic_write;

/// Maximum misbehavior score before a peer is banned
const MAX_MISBEHAVIOR_SCORE: i32 = 100;
Expand Down
29 changes: 0 additions & 29 deletions dash-spv/src/storage/disk/mod.rs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use dashcore::block::Header as BlockHeader;
use dashcore::BlockHash;

use crate::error::StorageResult;
use crate::storage::disk::io::atomic_write;
use crate::storage::io::atomic_write;
use crate::StorageError;

use super::manager::DiskStorageManager;
Expand Down
File renamed without changes.
File renamed without changes.
Loading