Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
27 changes: 2 additions & 25 deletions dash-spv-ffi/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,35 +147,12 @@ pub unsafe extern "C" fn dash_spv_ffi_client_new(
}
};

let mut client_config = config.clone_inner();

let storage_path = client_config.storage_path.clone().unwrap_or_else(|| {
// Create a unique temporary directory if none was provided
static PATH_COUNTER: AtomicU64 = AtomicU64::new(0);

let mut path = std::env::temp_dir();
path.push("dash-spv");
path.push(
format!(
"{:?}-{}-{}",
client_config.network,
std::process::id(),
PATH_COUNTER.fetch_add(1, Ordering::Relaxed)
)
.to_lowercase(),
);
tracing::warn!(
"dash-spv FFI config missing storage path, falling back to temp dir {:?}",
path
);
path
});
client_config.storage_path = Some(storage_path.clone());
let client_config = config.clone_inner();

let client_result = runtime.block_on(async move {
// Construct concrete implementations for generics
let network = dash_spv::network::PeerNetworkManager::new(&client_config).await;
let storage = DiskStorageManager::new(storage_path.clone()).await;
let storage = DiskStorageManager::new(&client_config).await;
let wallet = key_wallet_manager::wallet_manager::WalletManager::<
key_wallet::wallet::managed_wallet_info::ManagedWalletInfo,
>::new(client_config.network);
Expand Down
10 changes: 2 additions & 8 deletions dash-spv-ffi/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub unsafe extern "C" fn dash_spv_ffi_config_set_data_dir(
let config = unsafe { &mut *((*config).inner as *mut ClientConfig) };
match CStr::from_ptr(path).to_str() {
Ok(path_str) => {
config.storage_path = Some(path_str.into());
config.storage_path = path_str.into();
FFIErrorCode::Success as i32
}
Err(e) => {
Expand Down Expand Up @@ -331,13 +331,7 @@ pub unsafe extern "C" fn dash_spv_ffi_config_get_data_dir(
}

let config = unsafe { &*((*config).inner as *const ClientConfig) };
match &config.storage_path {
Some(dir) => FFIString::new(&dir.to_string_lossy()),
None => FFIString {
ptr: std::ptr::null_mut(),
length: 0,
},
}
FFIString::new(&config.storage_path.to_string_lossy())
}

/// Destroys an FFIClientConfig and frees its memory
Expand Down
15 changes: 14 additions & 1 deletion dash-spv-ffi/tests/test_wallet_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ mod tests {
FFIError, FFINetwork, FFIWalletManager,
};
use key_wallet_manager::wallet_manager::WalletManager;
use std::ffi::CStr;
use std::ffi::{CStr, CString};
use tempfile::TempDir;

#[test]
fn test_get_wallet_manager() {
Expand All @@ -20,6 +21,12 @@ mod tests {
let config = dash_spv_ffi_config_testnet();
assert!(!config.is_null());

let temp_dir = TempDir::new().unwrap();
dash_spv_ffi_config_set_data_dir(
config,
CString::new(temp_dir.path().to_str().unwrap()).unwrap().as_ptr(),
);

// Create a client
let client = dash_spv_ffi_client_new(config);
assert!(!client.is_null());
Expand Down Expand Up @@ -51,6 +58,12 @@ mod tests {
let config = dash_spv_ffi_config_testnet();
assert!(!config.is_null());

let temp_dir = TempDir::new().unwrap();
dash_spv_ffi_config_set_data_dir(
config,
CString::new(temp_dir.path().to_str().unwrap()).unwrap().as_ptr(),
);

let client = dash_spv_ffi_client_new(config);
assert!(!client.is_null());

Expand Down
10 changes: 6 additions & 4 deletions dash-spv/benches/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::time::Duration;
use criterion::{criterion_group, criterion_main, BatchSize, Criterion};
use dash_spv::{
storage::{BlockHeaderStorage, DiskStorageManager, StorageManager},
Hash,
ClientConfig, Hash,
};
use dashcore::{block::Version, BlockHash, CompactTarget, Header};
use rand::{rngs::StdRng, Rng, SeedableRng};
Expand Down Expand Up @@ -34,7 +34,9 @@ fn bench_disk_storage(c: &mut Criterion) {
c.bench_function("storage/disk/store", |b| {
b.to_async(&rt).iter_batched(
|| async {
DiskStorageManager::new(TempDir::new().unwrap().path().to_path_buf()).await.unwrap()
let config =
ClientConfig::testnet().with_storage_path(TempDir::new().unwrap().path());
DiskStorageManager::new(&config).await.unwrap()
},
|a| async {
let mut storage = a.await;
Expand All @@ -47,10 +49,10 @@ fn bench_disk_storage(c: &mut Criterion) {
)
});

let temp_dir = TempDir::new().unwrap();
let config = ClientConfig::testnet().with_storage_path(TempDir::new().unwrap().path());

let mut storage = rt.block_on(async {
let mut storage = DiskStorageManager::new(temp_dir.path().to_path_buf()).await.unwrap();
let mut storage = DiskStorageManager::new(&config).await.unwrap();

for chunk in headers.chunks(CHUNK_SIZE as usize) {
storage.store_headers(chunk).await.unwrap();
Expand Down
6 changes: 4 additions & 2 deletions dash-spv/examples/filter_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
)?;

// Create configuration with filter support
let config = ClientConfig::mainnet().without_masternodes(); // Skip masternode sync for this example
let config = ClientConfig::mainnet()
.with_storage_path("./.tmp/filter-sync-example-storage")
.without_masternodes(); // Skip masternode sync for this example

// Create network manager
let network_manager = PeerNetworkManager::new(&config).await?;

// Create storage manager
let storage_manager = DiskStorageManager::new("./.tmp/filter-sync-example-storage").await?;
let storage_manager = DiskStorageManager::new(&config).await?;

// Create wallet manager
let wallet = Arc::new(RwLock::new(WalletManager::<ManagedWalletInfo>::new(config.network)));
Expand Down
3 changes: 2 additions & 1 deletion dash-spv/examples/simple_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {

// Create a simple configuration
let config = ClientConfig::mainnet()
.with_storage_path("./.tmp/simple-sync-example-storage")
.without_filters() // Skip filter sync for this example
.without_masternodes(); // Skip masternode sync for this example

// Create network manager
let network_manager = PeerNetworkManager::new(&config).await?;

// Create storage manager
let storage_manager = DiskStorageManager::new("./.tmp/simple-sync-example-storage").await?;
let storage_manager = DiskStorageManager::new(&config).await?;

// Create wallet manager
let wallet = Arc::new(RwLock::new(WalletManager::<ManagedWalletInfo>::new(config.network)));
Expand Down
9 changes: 4 additions & 5 deletions dash-spv/examples/spv_with_wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,15 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
let _logging_guard = dash_spv::init_console_logging(LevelFilter::INFO)?;

// Create SPV client configuration
let mut config = ClientConfig::testnet();
config.storage_path = Some("/tmp/dash-spv-example".into());
config.validation_mode = dash_spv::types::ValidationMode::Full;
config.enable_filters = true;
let config = ClientConfig::testnet()
.with_storage_path("./.tmp/spv-with-wallet-example-storage")
.with_validation_mode(dash_spv::ValidationMode::Full);

// Create network manager
let network_manager = PeerNetworkManager::new(&config).await?;

// Create storage manager - use disk storage for persistence
let storage_manager = DiskStorageManager::new("./.tmp/spv-with-wallet-example-storage").await?;
let storage_manager = DiskStorageManager::new(&config).await?;

// Create wallet manager
let wallet = Arc::new(RwLock::new(WalletManager::<ManagedWalletInfo>::new(config.network)));
Expand Down
17 changes: 12 additions & 5 deletions dash-spv/src/client/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ pub struct ClientConfig {
/// If no peers are configured, no outbound connections will be made.
pub restrict_to_configured_peers: bool,

/// Optional path for persistent storage.
pub storage_path: Option<PathBuf>,
/// Path for persistent storage. Defaults to ./dash-spv-storage
pub storage_path: PathBuf,

/// Validation mode.
pub validation_mode: ValidationMode,
Expand Down Expand Up @@ -80,7 +80,7 @@ impl Default for ClientConfig {
network: Network::Dash,
peers: vec![],
restrict_to_configured_peers: false,
storage_path: None,
storage_path: PathBuf::from("./dash-spv-storage"),
validation_mode: ValidationMode::Full,
Comment on lines +83 to 84
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 | 🟠 Major

Default storage_path is network-agnostic; risk of cross-network data mixing.

If callers use ClientConfig::testnet()/regtest() without overriding storage_path, multiple networks will share the same on-disk state directory. That can corrupt or misinterpret chain data. Consider deriving a network-specific default (e.g., ./dash-spv-storage/{network}) in ClientConfig::new or when constructing defaults for non-mainnet.

🤖 Prompt for AI Agents
In `@dash-spv/src/client/config.rs` around lines 83 - 84, The default storage_path
is network-agnostic and can cause cross-network data mixing; update ClientConfig
construction so the default storage_path includes the network name (e.g.,
"./dash-spv-storage/{network}") when building non-mainnet configs. Modify
ClientConfig::new (and/or the ClientConfig::testnet() and
ClientConfig::regtest() helpers) to derive storage_path from the chosen
network/ValidationMode and set the storage_path field accordingly instead of
PathBuf::from("./dash-spv-storage"), ensuring each network gets a unique
subdirectory.

enable_filters: true,
enable_masternodes: true,
Expand Down Expand Up @@ -136,8 +136,8 @@ impl ClientConfig {
}

/// Set storage path.
pub fn with_storage_path(mut self, path: PathBuf) -> Self {
self.storage_path = Some(path);
pub fn with_storage_path(mut self, path: impl Into<PathBuf>) -> Self {
self.storage_path = path.into();
self
}

Expand Down Expand Up @@ -206,6 +206,13 @@ impl ClientConfig {
);
}

std::fs::create_dir_all(&self.storage_path).map_err(|e| {
format!(
"A valid storage path must be provided to the ClientConfig {:?}: {e}",
self.storage_path
)
})?;

Ok(())
}

Expand Down
8 changes: 1 addition & 7 deletions dash-spv/src/client/config_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ mod tests {
.with_mempool_persistence(true)
.with_start_height(100000);

assert_eq!(config.storage_path, Some(path));
assert_eq!(config.storage_path, path);
assert_eq!(config.validation_mode, ValidationMode::Basic);

// Mempool settings
Expand Down Expand Up @@ -88,12 +88,6 @@ mod tests {
assert!(!config.enable_masternodes);
}

#[test]
fn test_validation_valid_config() {
let config = ClientConfig::default();
assert!(config.validate().is_ok());
}

#[test]
fn test_validation_invalid_max_peers() {
let config = ClientConfig {
Expand Down
31 changes: 14 additions & 17 deletions dash-spv/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,14 @@ mod message_handler_test;
#[cfg(test)]
mod tests {
use super::{ClientConfig, DashSpvClient};
use crate::client::config::MempoolStrategy;
use crate::storage::DiskStorageManager;
use crate::{test_utils::MockNetworkManager, types::UnconfirmedTransaction};
use dashcore::{Address, Amount, Network, Transaction, TxOut};
use dashcore::{Address, Amount, Transaction, TxOut};
use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo;
use key_wallet_manager::wallet_manager::WalletManager;
use std::sync::Arc;
use tempfile::TempDir;
use tokio::sync::RwLock;

// Tests for get_mempool_balance function
Expand All @@ -80,13 +82,11 @@ mod tests {

#[tokio::test]
async fn client_exposes_shared_wallet_manager() {
let config = ClientConfig {
network: Network::Dash,
enable_filters: false,
enable_masternodes: false,
enable_mempool_tracking: false,
..Default::default()
};
let config = ClientConfig::mainnet()
.without_filters()
.without_masternodes()
.with_mempool_tracking(MempoolStrategy::FetchAll)
.with_storage_path(TempDir::new().unwrap().path());

let network_manager = MockNetworkManager::new();
let storage =
Expand All @@ -108,17 +108,14 @@ mod tests {
// This test validates the get_mempool_balance logic by directly testing
// the balance calculation code using a mocked mempool state.

let config = ClientConfig {
network: Network::Testnet,
enable_filters: false,
enable_masternodes: false,
enable_mempool_tracking: true,
..Default::default()
};
let config = ClientConfig::testnet()
.without_filters()
.without_masternodes()
.with_mempool_tracking(MempoolStrategy::FetchAll)
.with_storage_path(TempDir::new().unwrap().path());

let network_manager = MockNetworkManager::new();
let storage =
DiskStorageManager::with_temp_dir().await.expect("Failed to create tmp storage");
let storage = DiskStorageManager::new(&config).await.expect("Failed to create tmp storage");
let wallet = Arc::new(RwLock::new(WalletManager::<ManagedWalletInfo>::new(config.network)));

let test_address = Address::dummy(config.network, 0);
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 @@ -26,11 +26,11 @@
//! async fn main() -> Result<(), Box<dyn std::error::Error>> {
//! // Create configuration for mainnet
//! let config = ClientConfig::mainnet()
//! .with_storage_path("/path/to/data".into());
//! .with_storage_path("./.tmp/example-storage");
//!
//! // Create the required components
//! let network = PeerNetworkManager::new(&config).await?;
//! let storage = DiskStorageManager::new("./.tmp/example-storage").await?;
//! let storage = DiskStorageManager::new(&config).await?;
//! let wallet = Arc::new(RwLock::new(WalletManager::<ManagedWalletInfo>::new(config.network)));
//!
//! // Create and start the client
Expand Down
8 changes: 1 addition & 7 deletions dash-spv/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,13 +320,7 @@ async fn run() -> Result<(), Box<dyn std::error::Error>> {
}
};

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_manager = match dash_spv::storage::DiskStorageManager::new(&config).await {
Ok(sm) => sm,
Err(e) => {
eprintln!("Failed to create disk storage manager: {}", e);
Expand Down
2 changes: 1 addition & 1 deletion dash-spv/src/network/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl PeerNetworkManager {
/// Create a new peer network manager
pub async fn new(config: &ClientConfig) -> Result<Self, Error> {
let discovery = DnsDiscovery::new().await?;
let data_dir = config.storage_path.clone().unwrap_or_else(|| PathBuf::from("."));
let data_dir = config.storage_path.clone();

let peer_store = PersistentPeerStorage::open(data_dir.clone()).await?;

Expand Down
Loading