diff --git a/dash-spv-ffi/FFI_API.md b/dash-spv-ffi/FFI_API.md index 7f9121b23..fc76d4e22 100644 --- a/dash-spv-ffi/FFI_API.md +++ b/dash-spv-ffi/FFI_API.md @@ -303,10 +303,10 @@ dash_spv_ffi_config_get_mempool_strategy(config: *const FFIClientConfig,) -> FFI ``` **Description:** -Gets the mempool synchronization strategy # Safety - `config` must be a valid pointer to an FFIClientConfig or null - If null, returns FFIMempoolStrategy::Selective as default +Gets the mempool synchronization strategy # Safety - `config` must be a valid pointer to an FFIClientConfig or null - If null, returns FFIMempoolStrategy::FetchAll as default **Safety:** -- `config` must be a valid pointer to an FFIClientConfig or null - If null, returns FFIMempoolStrategy::Selective as default +- `config` must be a valid pointer to an FFIClientConfig or null - If null, returns FFIMempoolStrategy::FetchAll as default **Module:** `config` diff --git a/dash-spv-ffi/include/dash_spv_ffi.h b/dash-spv-ffi/include/dash_spv_ffi.h index dae86e8a6..2e0308c9a 100644 --- a/dash-spv-ffi/include/dash_spv_ffi.h +++ b/dash-spv-ffi/include/dash_spv_ffi.h @@ -19,7 +19,6 @@ namespace dash_spv_ffi { typedef enum FFIMempoolStrategy { FetchAll = 0, BloomFilter = 1, - Selective = 2, } FFIMempoolStrategy; typedef enum FFISyncStage { @@ -817,7 +816,7 @@ int32_t dash_spv_ffi_config_set_persist_mempool(struct FFIClientConfig *config, * * # Safety * - `config` must be a valid pointer to an FFIClientConfig or null - * - If null, returns FFIMempoolStrategy::Selective as default + * - If null, returns FFIMempoolStrategy::FetchAll as default */ enum FFIMempoolStrategy dash_spv_ffi_config_get_mempool_strategy(const struct FFIClientConfig *config) diff --git a/dash-spv-ffi/src/client.rs b/dash-spv-ffi/src/client.rs index a6858970a..f45fb1ee5 100644 --- a/dash-spv-ffi/src/client.rs +++ b/dash-spv-ffi/src/client.rs @@ -296,6 +296,12 @@ impl FFIDashSpvClient { dash_spv::types::SpvEvent::ChainLockReceived { .. } => {} + dash_spv::types::SpvEvent::InstantLockReceived { + .. + } => { + // InstantLock received and validated + // TODO: Add FFI callback if needed for instant lock notifications + } dash_spv::types::SpvEvent::MempoolTransactionAdded { ref txid, amount, @@ -1445,7 +1451,7 @@ pub unsafe extern "C" fn dash_spv_ffi_client_record_send( } }; - let txid = match Txid::from_str(txid_str) { + let _txid = match Txid::from_str(txid_str) { Ok(t) => t, Err(e) => { set_last_error(&format!("Invalid txid: {}", e)); @@ -1468,7 +1474,6 @@ pub unsafe extern "C" fn dash_spv_ffi_client_record_send( } } }; - spv_client.record_transaction_send(txid).await; let mut guard = inner.lock().unwrap(); *guard = Some(spv_client); Ok(()) diff --git a/dash-spv-ffi/src/config.rs b/dash-spv-ffi/src/config.rs index f7393b07f..a6e9ccdf3 100644 --- a/dash-spv-ffi/src/config.rs +++ b/dash-spv-ffi/src/config.rs @@ -503,13 +503,13 @@ pub unsafe extern "C" fn dash_spv_ffi_config_get_mempool_tracking( /// /// # Safety /// - `config` must be a valid pointer to an FFIClientConfig or null -/// - If null, returns FFIMempoolStrategy::Selective as default +/// - If null, returns FFIMempoolStrategy::FetchAll as default #[no_mangle] pub unsafe extern "C" fn dash_spv_ffi_config_get_mempool_strategy( config: *const FFIClientConfig, ) -> FFIMempoolStrategy { if config.is_null() { - return FFIMempoolStrategy::Selective; + return FFIMempoolStrategy::FetchAll; } let config = unsafe { &*((*config).inner as *const ClientConfig) }; diff --git a/dash-spv-ffi/src/types.rs b/dash-spv-ffi/src/types.rs index a66a35211..c644c52da 100644 --- a/dash-spv-ffi/src/types.rs +++ b/dash-spv-ffi/src/types.rs @@ -395,7 +395,6 @@ pub unsafe extern "C" fn dash_spv_ffi_string_array_destroy(arr: *mut FFIArray) { pub enum FFIMempoolStrategy { FetchAll = 0, BloomFilter = 1, - Selective = 2, } impl From for FFIMempoolStrategy { @@ -403,7 +402,6 @@ impl From for FFIMempoolStrategy { match strategy { MempoolStrategy::FetchAll => FFIMempoolStrategy::FetchAll, MempoolStrategy::BloomFilter => FFIMempoolStrategy::BloomFilter, - MempoolStrategy::Selective => FFIMempoolStrategy::Selective, } } } @@ -413,7 +411,6 @@ impl From for MempoolStrategy { match strategy { FFIMempoolStrategy::FetchAll => MempoolStrategy::FetchAll, FFIMempoolStrategy::BloomFilter => MempoolStrategy::BloomFilter, - FFIMempoolStrategy::Selective => MempoolStrategy::Selective, } } } diff --git a/dash-spv/src/client/chainlock.rs b/dash-spv/src/client/chainlock.rs index 85f63f42a..75df8a2ec 100644 --- a/dash-spv/src/client/chainlock.rs +++ b/dash-spv/src/client/chainlock.rs @@ -37,10 +37,16 @@ impl< let chain_state = self.state.read().await; { let mut storage = self.storage.lock().await; - self.chainlock_manager + if let Err(e) = self + .chainlock_manager .process_chain_lock(chainlock.clone(), &chain_state, &mut *storage) .await - .map_err(SpvError::Validation)?; + { + // Penalize the peer that relayed the invalid ChainLock + let reason = format!("Invalid ChainLock: {}", e); + let _ = self.network.penalize_last_message_peer_invalid_chainlock(&reason).await; + return Err(SpvError::Validation(e)); + } } drop(chain_state); @@ -89,20 +95,39 @@ impl< ) -> Result<()> { tracing::info!("Processing InstantSendLock for tx {}", islock.txid); - // TODO: Implement InstantSendLock validation - // - Verify BLS signature against known quorum - // - Check if all inputs are locked - // - Mark transaction as instantly confirmed - // - Store InstantSendLock for future reference + // Get the masternode engine from sync manager for proper quorum verification + let masternode_engine = self.sync_manager.get_masternode_engine().ok_or_else(|| { + SpvError::Validation(crate::error::ValidationError::MasternodeVerification( + "Masternode engine not available for InstantLock verification".to_string(), + )) + })?; + + // Validate the InstantLock (structure + BLS signature) + // This is REQUIRED for security - never accept InstantLocks without signature verification + let validator = crate::validation::instantlock::InstantLockValidator::new(); + if let Err(e) = validator.validate(&islock, masternode_engine) { + // Penalize the peer that relayed the invalid InstantLock + let reason = format!("Invalid InstantLock: {}", e); + tracing::warn!("{}", reason); + + // Ban the peer using the reputation system + let _ = self.network.penalize_last_message_peer_invalid_instantlock(&reason).await; + + return Err(SpvError::Validation(e)); + } - // For now, just log the InstantSendLock details tracing::info!( - "InstantSendLock validated: txid={}, inputs={}, signature={:?}", + "✅ InstantSendLock validated successfully: txid={}, inputs={}", islock.txid, - islock.inputs.len(), - islock.signature.to_string().chars().take(20).collect::() + islock.inputs.len() ); + // Emit InstantLock event + self.emit_event(SpvEvent::InstantLockReceived { + txid: islock.txid, + inputs: islock.inputs.clone(), + }); + Ok(()) } diff --git a/dash-spv/src/client/config.rs b/dash-spv/src/client/config.rs index 781d3dabb..afedf3177 100644 --- a/dash-spv/src/client/config.rs +++ b/dash-spv/src/client/config.rs @@ -12,12 +12,10 @@ use crate::types::ValidationMode; /// Strategy for handling mempool (unconfirmed) transactions. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum MempoolStrategy { - /// Fetch all announced transactions (poor privacy, high bandwidth). + /// Fetch all announced transactions (high bandwidth, sees all transactions). FetchAll, /// Use BIP37 bloom filters (moderate privacy, good efficiency). BloomFilter, - /// Only fetch when recently sent or from known addresses (good privacy, default). - Selective, } /// Configuration for the Dash SPV client. @@ -132,9 +130,6 @@ pub struct ClientConfig { /// Time after which unconfirmed transactions are pruned (seconds). pub mempool_timeout_secs: u64, - /// Time window for recent sends in selective mode (seconds). - pub recent_send_window_secs: u64, - /// Whether to fetch transactions from INV messages immediately. pub fetch_mempool_transactions: bool, @@ -232,11 +227,10 @@ impl Default for ClientConfig { max_filter_gap_restart_attempts: 5, max_filter_gap_sync_size: 50000, // Mempool defaults - enable_mempool_tracking: false, - mempool_strategy: MempoolStrategy::Selective, + enable_mempool_tracking: true, + mempool_strategy: MempoolStrategy::FetchAll, max_mempool_transactions: 1000, - mempool_timeout_secs: 3600, // 1 hour - recent_send_window_secs: 300, // 5 minutes + mempool_timeout_secs: 3600, // 1 hour fetch_mempool_transactions: true, persist_mempool: false, // Request control defaults @@ -388,12 +382,6 @@ impl ClientConfig { self } - /// Set recent send window for selective strategy. - pub fn with_recent_send_window(mut self, window_secs: u64) -> Self { - self.recent_send_window_secs = window_secs; - self - } - /// Enable or disable mempool persistence. pub fn with_mempool_persistence(mut self, enabled: bool) -> Self { self.persist_mempool = enabled; @@ -449,13 +437,6 @@ impl ClientConfig { if self.mempool_timeout_secs == 0 { return Err("mempool_timeout_secs must be > 0".to_string()); } - if self.mempool_strategy == MempoolStrategy::Selective - && self.recent_send_window_secs == 0 - { - return Err( - "recent_send_window_secs must be > 0 for Selective strategy".to_string() - ); - } } Ok(()) diff --git a/dash-spv/src/client/config_test.rs b/dash-spv/src/client/config_test.rs index 2b9cbd4cd..d1a1e388f 100644 --- a/dash-spv/src/client/config_test.rs +++ b/dash-spv/src/client/config_test.rs @@ -33,11 +33,10 @@ mod tests { assert_eq!(config.filter_request_delay_ms, 0); // Mempool defaults - assert!(!config.enable_mempool_tracking); - assert_eq!(config.mempool_strategy, MempoolStrategy::Selective); + assert!(config.enable_mempool_tracking); + assert_eq!(config.mempool_strategy, MempoolStrategy::FetchAll); assert_eq!(config.max_mempool_transactions, 1000); assert_eq!(config.mempool_timeout_secs, 3600); - assert_eq!(config.recent_send_window_secs, 300); assert!(config.fetch_mempool_transactions); assert!(!config.persist_mempool); } @@ -74,7 +73,6 @@ mod tests { .with_mempool_tracking(MempoolStrategy::BloomFilter) .with_max_mempool_transactions(500) .with_mempool_timeout(7200) - .with_recent_send_window(600) .with_mempool_persistence(true) .with_start_height(100000); @@ -93,7 +91,6 @@ mod tests { assert_eq!(config.mempool_strategy, MempoolStrategy::BloomFilter); assert_eq!(config.max_mempool_transactions, 500); assert_eq!(config.mempool_timeout_secs, 7200); - assert_eq!(config.recent_send_window_secs, 600); assert!(config.persist_mempool); assert_eq!(config.start_from_height, Some(100000)); } @@ -200,22 +197,7 @@ mod tests { assert_eq!(result.unwrap_err(), "mempool_timeout_secs must be > 0"); } - #[test] - fn test_validation_invalid_selective_strategy() { - let config = ClientConfig { - enable_mempool_tracking: true, - mempool_strategy: MempoolStrategy::Selective, - recent_send_window_secs: 0, - ..Default::default() - }; - - let result = config.validate(); - assert!(result.is_err()); - assert_eq!( - result.unwrap_err(), - "recent_send_window_secs must be > 0 for Selective strategy" - ); - } + // Removed selective strategy validation test; Selective variant no longer exists #[test] fn test_cfheader_gap_settings() { diff --git a/dash-spv/src/client/lifecycle.rs b/dash-spv/src/client/lifecycle.rs index 232f7b6a9..c7c0ae43a 100644 --- a/dash-spv/src/client/lifecycle.rs +++ b/dash-spv/src/client/lifecycle.rs @@ -10,7 +10,6 @@ use std::collections::HashSet; use std::sync::Arc; -use std::time::Duration; use tokio::sync::{mpsc, Mutex, RwLock}; use crate::chain::ChainLockManager; @@ -121,7 +120,6 @@ impl< // TODO: Get monitored addresses from wallet self.mempool_filter = Some(Arc::new(MempoolFilter::new( self.config.mempool_strategy, - Duration::from_secs(self.config.recent_send_window_secs), self.config.max_mempool_transactions, self.mempool_state.clone(), HashSet::new(), // Will be populated from wallet's monitored addresses diff --git a/dash-spv/src/client/mempool.rs b/dash-spv/src/client/mempool.rs index 4c6ab9e24..8dd7ad042 100644 --- a/dash-spv/src/client/mempool.rs +++ b/dash-spv/src/client/mempool.rs @@ -8,7 +8,6 @@ use std::collections::HashSet; use std::sync::Arc; -use std::time::Duration; use crate::error::Result; use crate::mempool_filter::MempoolFilter; @@ -38,7 +37,6 @@ impl< // TODO: Get monitored addresses from wallet self.mempool_filter = Some(Arc::new(MempoolFilter::new( self.config.mempool_strategy, - Duration::from_secs(self.config.recent_send_window_secs), self.config.max_mempool_transactions, self.mempool_state.clone(), HashSet::new(), // Will be populated from wallet's monitored addresses @@ -147,7 +145,6 @@ impl< // For now, create empty filter until wallet integration is complete self.mempool_filter = Some(Arc::new(MempoolFilter::new( self.config.mempool_strategy, - Duration::from_secs(self.config.recent_send_window_secs), self.config.max_mempool_transactions, self.mempool_state.clone(), HashSet::new(), // Will be populated from wallet's monitored addresses @@ -155,11 +152,4 @@ impl< ))); tracing::info!("Updated mempool filter (wallet integration pending)"); } - - /// Record a transaction send for mempool filtering. - pub async fn record_transaction_send(&self, txid: dashcore::Txid) { - if let Some(ref mempool_filter) = self.mempool_filter { - mempool_filter.record_send(txid).await; - } - } } diff --git a/dash-spv/src/client/message_handler.rs b/dash-spv/src/client/message_handler.rs index 75a83fc6d..010dcc049 100644 --- a/dash-spv/src/client/message_handler.rs +++ b/dash-spv/src/client/message_handler.rs @@ -403,8 +403,18 @@ impl< chainlocks_to_request.push(item); } Inventory::InstantSendLock(islock_hash) => { - tracing::info!("⚡ Inventory: New InstantSendLock {}", islock_hash); - islocks_to_request.push(item); + // Only fetch InstantSendLocks when we're fully synced and have masternode data + if self.sync_manager.is_synced() + && self.sync_manager.get_masternode_engine().is_some() + { + tracing::info!("⚡ Inventory: New InstantSendLock {}", islock_hash); + islocks_to_request.push(item); + } else { + tracing::debug!( + "Skipping InstantSendLock {} fetch - not fully synced or masternode engine unavailable", + islock_hash + ); + } } Inventory::Transaction(txid) => { tracing::debug!("💸 Inventory: New transaction {}", txid); @@ -444,7 +454,7 @@ impl< self.network.send_message(getdata).await.map_err(SpvError::Network)?; } - // Auto-request InstantLocks + // Auto-request InstantLocks (only when synced and masternodes available; gated above) if !islocks_to_request.is_empty() { tracing::info!("Requesting {} InstantLocks", islocks_to_request.len()); let getdata = NetworkMessage::GetData(islocks_to_request); diff --git a/dash-spv/src/client/mod.rs b/dash-spv/src/client/mod.rs index 4f42734e0..6cd2f0028 100644 --- a/dash-spv/src/client/mod.rs +++ b/dash-spv/src/client/mod.rs @@ -133,7 +133,7 @@ mod tests { // Enable mempool tracking to initialize mempool_filter client - .enable_mempool_tracking(crate::client::config::MempoolStrategy::Selective) + .enable_mempool_tracking(crate::client::config::MempoolStrategy::BloomFilter) .await .expect("enable mempool tracking must succeed"); diff --git a/dash-spv/src/client/sync_coordinator.rs b/dash-spv/src/client/sync_coordinator.rs index 793ceec66..8f372a0bc 100644 --- a/dash-spv/src/client/sync_coordinator.rs +++ b/dash-spv/src/client/sync_coordinator.rs @@ -53,11 +53,11 @@ impl< self.update_status_display().await; tracing::info!( - "✅ Initial sync requests sent! Current state - Headers: {}, Filter headers: {}", + "✅ Prepared initial sync state - Headers: {}, Filter headers: {}", result.header_height, result.filter_header_height ); - tracing::info!("📊 Actual sync will complete asynchronously through monitoring loop"); + tracing::info!("📊 Sync requests will be sent by the monitoring loop"); Ok(result) } @@ -612,8 +612,16 @@ impl< self.process_chainlock(clsig.clone()).await?; } NetworkMessage::ISLock(islock_msg) => { - // Additional client-level InstantLock processing - self.process_instantsendlock(islock_msg.clone()).await?; + // Only process InstantLocks when fully synced and masternode engine is available + if self.sync_manager.is_synced() + && self.sync_manager.get_masternode_engine().is_some() + { + self.process_instantsendlock(islock_msg.clone()).await?; + } else { + tracing::debug!( + "Skipping InstantLock processing - not fully synced or masternode engine unavailable" + ); + } } _ => {} } diff --git a/dash-spv/src/error.rs b/dash-spv/src/error.rs index e5d8c6710..cd8ca0879 100644 --- a/dash-spv/src/error.rs +++ b/dash-spv/src/error.rs @@ -142,6 +142,9 @@ pub enum ValidationError { #[error("Invalid InstantLock: {0}")] InvalidInstantLock(String), + #[error("Invalid signature: {0}")] + InvalidSignature(String), + #[error("Invalid filter header chain: {0}")] InvalidFilterHeaderChain(String), diff --git a/dash-spv/src/main.rs b/dash-spv/src/main.rs index afe40d4cb..1c17b593e 100644 --- a/dash-spv/src/main.rs +++ b/dash-spv/src/main.rs @@ -85,6 +85,12 @@ async fn run() -> Result<(), Box> { .help("Disable masternode list synchronization") .action(clap::ArgAction::SetTrue), ) + .arg( + Arg::new("no-mempool") + .long("no-mempool") + .help("Disable mempool transaction tracking") + .action(clap::ArgAction::SetTrue), + ) .arg( Arg::new("validation-mode") .long("validation-mode") @@ -183,6 +189,9 @@ async fn run() -> Result<(), Box> { if matches.get_flag("no-masternodes") { config = config.without_masternodes(); } + if matches.get_flag("no-mempool") { + config.enable_mempool_tracking = false; + } // Set start height if specified if let Some(start_height_str) = matches.get_one::("start-height") { diff --git a/dash-spv/src/mempool_filter.rs b/dash-spv/src/mempool_filter.rs index 00191a8f2..e50098c73 100644 --- a/dash-spv/src/mempool_filter.rs +++ b/dash-spv/src/mempool_filter.rs @@ -14,8 +14,6 @@ use crate::types::{MempoolState, UnconfirmedTransaction}; pub struct MempoolFilter { /// Mempool strategy to use. strategy: MempoolStrategy, - /// Recent send window duration. - recent_send_window: Duration, /// Maximum number of transactions to track. max_transactions: usize, /// Mempool state. @@ -30,7 +28,6 @@ impl MempoolFilter { /// Create a new mempool filter. pub fn new( strategy: MempoolStrategy, - recent_send_window: Duration, max_transactions: usize, mempool_state: Arc>, watched_addresses: HashSet
, @@ -38,7 +35,6 @@ impl MempoolFilter { ) -> Self { Self { strategy, - recent_send_window, max_transactions, mempool_state, watched_addresses: watched_addresses.into_iter().collect(), @@ -47,7 +43,7 @@ impl MempoolFilter { } /// Check if we should fetch a transaction based on its txid. - pub async fn should_fetch_transaction(&self, txid: &Txid) -> bool { + pub async fn should_fetch_transaction(&self, _txid: &Txid) -> bool { match self.strategy { MempoolStrategy::FetchAll => { // Check if we're at capacity @@ -59,11 +55,6 @@ impl MempoolFilter { // This is handled by the network layer true } - MempoolStrategy::Selective => { - // Check if this was a recent send - let state = self.mempool_state.read().await; - state.is_recent_send(txid, self.recent_send_window) - } } } @@ -171,12 +162,6 @@ impl MempoolFilter { )) } - /// Record that we sent a transaction. - pub async fn record_send(&self, txid: Txid) { - let mut state = self.mempool_state.write().await; - state.record_send(txid); - } - /// Prune expired transactions. pub async fn prune_expired(&self, timeout: Duration) -> Vec { let mut state = self.mempool_state.write().await; @@ -363,39 +348,11 @@ mod tests { create_test_addresses(network, 2)[1].clone() } - #[tokio::test] - async fn test_selective_strategy() { - let mempool_state = Arc::new(RwLock::new(MempoolState::default())); - let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), - 1000, - mempool_state.clone(), - HashSet::new(), - Network::Dash, - ); - - // Generate a test txid - let txid = - Txid::from_str("0101010101010101010101010101010101010101010101010101010101010101") - .unwrap(); - - // Should not fetch unknown transaction - assert!(!filter.should_fetch_transaction(&txid).await); - - // Record as recent send - filter.record_send(txid).await; - - // Should fetch recent send - assert!(filter.should_fetch_transaction(&txid).await); - } - #[tokio::test] async fn test_fetch_all_strategy() { let mempool_state = Arc::new(RwLock::new(MempoolState::default())); let filter = MempoolFilter::new( MempoolStrategy::FetchAll, - Duration::from_secs(300), 2, // Small limit for testing mempool_state.clone(), HashSet::new(), @@ -461,8 +418,7 @@ mod tests { let watched_addresses = vec![addr1.clone()].into_iter().collect(); let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), + MempoolStrategy::FetchAll, 1000, mempool_state, watched_addresses, @@ -491,8 +447,7 @@ mod tests { let watched_addresses = vec![addr.clone()].into_iter().collect(); let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), + MempoolStrategy::FetchAll, 1000, mempool_state, watched_addresses, @@ -520,8 +475,7 @@ mod tests { let watched_addresses = vec![addr.clone()].into_iter().collect(); let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), + MempoolStrategy::FetchAll, 1000, mempool_state, watched_addresses, @@ -562,7 +516,6 @@ mod tests { let mempool_state = Arc::new(RwLock::new(MempoolState::default())); let filter = MempoolFilter::new( MempoolStrategy::FetchAll, - Duration::from_secs(300), 3, // Very small limit mempool_state.clone(), HashSet::new(), @@ -607,8 +560,7 @@ mod tests { async fn test_prune_expired() { let mempool_state = Arc::new(RwLock::new(MempoolState::default())); let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), + MempoolStrategy::FetchAll, 1000, mempool_state.clone(), HashSet::new(), @@ -681,7 +633,6 @@ mod tests { let mempool_state = Arc::new(RwLock::new(MempoolState::default())); let filter = MempoolFilter::new( MempoolStrategy::BloomFilter, - Duration::from_secs(300), 1000, mempool_state, HashSet::new(), @@ -712,8 +663,7 @@ mod tests { .collect(); let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), + MempoolStrategy::FetchAll, 1000, mempool_state, watched_addresses, @@ -758,8 +708,7 @@ mod tests { .collect(); let filter = MempoolFilter::new( - MempoolStrategy::Selective, - Duration::from_secs(300), + MempoolStrategy::FetchAll, 1000, mempool_state, watched_addresses, diff --git a/dash-spv/src/network/mod.rs b/dash-spv/src/network/mod.rs index 57cb8ec0b..788925fef 100644 --- a/dash-spv/src/network/mod.rs +++ b/dash-spv/src/network/mod.rs @@ -154,6 +154,40 @@ pub trait NetworkManager: Send + Sync { Ok(()) } + + /// Penalize the last peer that sent us a message by adjusting reputation. + /// Default implementation is a no-op for managers without reputation. + async fn penalize_last_message_peer( + &self, + _score_change: i32, + _reason: &str, + ) -> NetworkResult<()> { + Ok(()) + } + + /// Convenience: penalize last peer for an invalid ChainLock. + async fn penalize_last_message_peer_invalid_chainlock( + &self, + reason: &str, + ) -> NetworkResult<()> { + self.penalize_last_message_peer( + crate::network::reputation::misbehavior_scores::INVALID_CHAINLOCK, + reason, + ) + .await + } + + /// Convenience: penalize last peer for an invalid InstantLock. + async fn penalize_last_message_peer_invalid_instantlock( + &self, + reason: &str, + ) -> NetworkResult<()> { + self.penalize_last_message_peer( + crate::network::reputation::misbehavior_scores::INVALID_INSTANTLOCK, + reason, + ) + .await + } } /// TCP-based network manager implementation. diff --git a/dash-spv/src/network/multi_peer.rs b/dash-spv/src/network/multi_peer.rs index 76894c12f..7afac8eb1 100644 --- a/dash-spv/src/network/multi_peer.rs +++ b/dash-spv/src/network/multi_peer.rs @@ -1050,7 +1050,91 @@ impl NetworkManager for MultiPeerNetworkManager { Ok(()) } + } // end match + } // end send_message + + async fn penalize_last_message_peer( + &self, + score_change: i32, + reason: &str, + ) -> NetworkResult<()> { + // Get the last peer that sent us a message + if let Some(addr) = self.get_last_message_peer().await { + self.reputation_manager.update_reputation(addr, score_change, reason).await; } + Ok(()) + } + + async fn penalize_last_message_peer_invalid_chainlock( + &self, + reason: &str, + ) -> NetworkResult<()> { + if let Some(addr) = self.get_last_message_peer().await { + match self.disconnect_peer(&addr, reason).await { + Ok(()) => { + log::warn!( + "Peer {} disconnected for invalid ChainLock enforcement: {}", + addr, + reason + ); + } + Err(err) => { + log::error!( + "Failed to disconnect peer {} after invalid ChainLock enforcement ({}): {}", + addr, + reason, + err + ); + } + } + + // Apply misbehavior score and a short temporary ban + self.reputation_manager + .update_reputation(addr, misbehavior_scores::INVALID_CHAINLOCK, reason) + .await; + + // Short ban: 10 minutes for relaying invalid ChainLock + self.reputation_manager + .temporary_ban_peer(addr, Duration::from_secs(10 * 60), reason) + .await; + } + Ok(()) + } + + async fn penalize_last_message_peer_invalid_instantlock( + &self, + reason: &str, + ) -> NetworkResult<()> { + if let Some(addr) = self.get_last_message_peer().await { + // Apply misbehavior score and a short temporary ban + self.reputation_manager + .update_reputation(addr, misbehavior_scores::INVALID_INSTANTLOCK, reason) + .await; + + // Short ban: 10 minutes for relaying invalid InstantLock + self.reputation_manager + .temporary_ban_peer(addr, Duration::from_secs(10 * 60), reason) + .await; + + match self.disconnect_peer(&addr, reason).await { + Ok(()) => { + log::warn!( + "Peer {} disconnected for invalid InstantLock enforcement: {}", + addr, + reason + ); + } + Err(err) => { + log::error!( + "Failed to disconnect peer {} after invalid InstantLock enforcement ({}): {}", + addr, + reason, + err + ); + } + } + } + Ok(()) } async fn receive_message(&mut self) -> NetworkResult> { diff --git a/dash-spv/src/network/reputation.rs b/dash-spv/src/network/reputation.rs index 705042291..86410d130 100644 --- a/dash-spv/src/network/reputation.rs +++ b/dash-spv/src/network/reputation.rs @@ -41,6 +41,9 @@ pub mod misbehavior_scores { /// Invalid ChainLock pub const INVALID_CHAINLOCK: i32 = 40; + /// Invalid InstantLock + pub const INVALID_INSTANTLOCK: i32 = 35; + /// Duplicate message pub const DUPLICATE_MESSAGE: i32 = 5; @@ -313,6 +316,24 @@ impl PeerReputationManager { } } + /// Temporarily ban a peer for a specified duration, regardless of score. + /// This can be used for critical protocol violations (e.g., invalid ChainLocks). + pub async fn temporary_ban_peer(&self, peer: SocketAddr, duration: Duration, reason: &str) { + let mut reputations = self.reputations.write().await; + let reputation = reputations.entry(peer).or_default(); + + reputation.banned_until = Some(Instant::now() + duration); + reputation.ban_count += 1; + + log::warn!( + "Peer {} temporarily banned for {:?} (ban #{}, reason: {})", + peer, + duration, + reputation.ban_count, + reason + ); + } + /// Record a connection attempt pub async fn record_connection_attempt(&self, peer: SocketAddr) { let mut reputations = self.reputations.write().await; diff --git a/dash-spv/src/network/tests.rs b/dash-spv/src/network/tests.rs index 86e793664..8c88c32de 100644 --- a/dash-spv/src/network/tests.rs +++ b/dash-spv/src/network/tests.rs @@ -162,10 +162,9 @@ mod multi_peer_tests { max_filter_gap_sync_size: 50000, // Mempool fields enable_mempool_tracking: false, - mempool_strategy: crate::client::config::MempoolStrategy::Selective, + mempool_strategy: crate::client::config::MempoolStrategy::BloomFilter, max_mempool_transactions: 1000, mempool_timeout_secs: 3600, - recent_send_window_secs: 300, fetch_mempool_transactions: true, persist_mempool: false, // Request control fields diff --git a/dash-spv/src/storage/disk/state.rs b/dash-spv/src/storage/disk/state.rs index 81d0921a3..49a49ad4b 100644 --- a/dash-spv/src/storage/disk/state.rs +++ b/dash-spv/src/storage/disk/state.rs @@ -332,50 +332,6 @@ impl DiskStorageManager { Ok(chain_locks) } - /// Store an InstantLock. - pub async fn store_instant_lock( - &mut self, - txid: Txid, - instant_lock: &dashcore::InstantLock, - ) -> StorageResult<()> { - let islocks_dir = self.base_path.join("islocks"); - tokio::fs::create_dir_all(&islocks_dir).await?; - - let path = islocks_dir.join(format!("islock_{}.bin", txid)); - let data = bincode::serialize(instant_lock).map_err(|e| { - crate::error::StorageError::WriteFailed(format!( - "Failed to serialize instant lock: {}", - e - )) - })?; - - tokio::fs::write(&path, &data).await?; - tracing::debug!("Stored instant lock for txid {}", txid); - Ok(()) - } - - /// Load an InstantLock. - pub async fn load_instant_lock( - &self, - txid: Txid, - ) -> StorageResult> { - let path = self.base_path.join("islocks").join(format!("islock_{}.bin", txid)); - - if !path.exists() { - return Ok(None); - } - - let data = tokio::fs::read(&path).await?; - let instant_lock = bincode::deserialize(&data).map_err(|e| { - crate::error::StorageError::ReadFailed(format!( - "Failed to deserialize instant lock: {}", - e - )) - })?; - - Ok(Some(instant_lock)) - } - /// Store metadata. pub async fn store_metadata(&mut self, key: &str, value: &[u8]) -> StorageResult<()> { let path = self.base_path.join(format!("state/{}.dat", key)); @@ -697,18 +653,6 @@ impl StorageManager for DiskStorageManager { Self::get_chain_locks(self, start_height, end_height).await } - async fn store_instant_lock( - &mut self, - txid: Txid, - instant_lock: &dashcore::InstantLock, - ) -> StorageResult<()> { - Self::store_instant_lock(self, txid, instant_lock).await - } - - async fn load_instant_lock(&self, txid: Txid) -> StorageResult> { - Self::load_instant_lock(self, txid).await - } - async fn store_mempool_transaction( &mut self, txid: &Txid, diff --git a/dash-spv/src/storage/memory.rs b/dash-spv/src/storage/memory.rs index dd0c8b946..839baf5ba 100644 --- a/dash-spv/src/storage/memory.rs +++ b/dash-spv/src/storage/memory.rs @@ -557,36 +557,6 @@ impl StorageManager for MemoryStorageManager { Ok(chain_locks) } - async fn store_instant_lock( - &mut self, - txid: dashcore::Txid, - instant_lock: &dashcore::InstantLock, - ) -> StorageResult<()> { - let key = format!("islock_{}", txid); - self.metadata.insert( - key, - bincode::serialize(instant_lock).map_err(|e| { - StorageError::WriteFailed(format!("Failed to serialize instant lock: {}", e)) - })?, - ); - Ok(()) - } - - async fn load_instant_lock( - &self, - txid: dashcore::Txid, - ) -> StorageResult> { - let key = format!("islock_{}", txid); - if let Some(data) = self.metadata.get(&key) { - let instant_lock = bincode::deserialize(data).map_err(|e| { - StorageError::ReadFailed(format!("Failed to deserialize instant lock: {}", e)) - })?; - Ok(Some(instant_lock)) - } else { - Ok(None) - } - } - // Mempool storage methods async fn store_mempool_transaction( &mut self, diff --git a/dash-spv/src/storage/mod.rs b/dash-spv/src/storage/mod.rs index 17a6ac7a1..f8d7d795b 100644 --- a/dash-spv/src/storage/mod.rs +++ b/dash-spv/src/storage/mod.rs @@ -216,19 +216,6 @@ pub trait StorageManager: Send + Sync { end_height: u32, ) -> StorageResult>; - /// Store an instant lock. - async fn store_instant_lock( - &mut self, - txid: dashcore::Txid, - instant_lock: &dashcore::InstantLock, - ) -> StorageResult<()>; - - /// Load an instant lock by transaction ID. - async fn load_instant_lock( - &self, - txid: dashcore::Txid, - ) -> StorageResult>; - // Mempool storage methods /// Store an unconfirmed transaction. async fn store_mempool_transaction( diff --git a/dash-spv/src/sync/filters/headers.rs b/dash-spv/src/sync/filters/headers.rs index 87cf0a01d..42992873c 100644 --- a/dash-spv/src/sync/filters/headers.rs +++ b/dash-spv/src/sync/filters/headers.rs @@ -13,11 +13,13 @@ //! - Filter header chain verification //! - Stability checking before declaring sync complete +use dashcore::hash_types::FilterHeader; use dashcore::{ network::message::NetworkMessage, network::message_filter::{CFHeaders, GetCFHeaders}, BlockHash, }; +use dashcore_hashes::{sha256d, Hash}; use super::types::*; use crate::error::{SyncError, SyncResult}; @@ -69,7 +71,9 @@ impl u32 { - stop_height.saturating_sub(cf_headers.filter_hashes.len() as u32 - 1) + let count = cf_headers.filter_hashes.len() as u32; + let offset = count.saturating_sub(1); + stop_height.saturating_sub(offset) } /// Get the height range for a CFHeaders batch. @@ -1120,91 +1124,90 @@ impl SyncResult<(usize, u32)> { - // Get the height range for this batch - let (batch_start_height, stop_height, _header_tip_height) = + // Get the original height range for this CFHeaders batch + let (original_start_height, _stop_height, _header_tip_height) = self.get_batch_height_range(cf_headers, storage).await?; - let skip_count = expected_start_height.saturating_sub(batch_start_height) as usize; + + // Determine how many headers overlap with what we already have + let headers_to_skip = expected_start_height.saturating_sub(original_start_height) as usize; // Complete overlap case - all headers already processed - if skip_count >= cf_headers.filter_hashes.len() { + if headers_to_skip >= cf_headers.filter_hashes.len() { tracing::info!( "✅ All {} headers in batch already processed, skipping", cf_headers.filter_hashes.len() ); return Ok((0, expected_start_height)); } - - // Find connection point in our chain - let current_filter_tip = storage - .get_filter_tip_height() - .await - .map_err(|e| SyncError::Storage(format!("Failed to get filter tip: {}", e)))? - .unwrap_or(0); - - let mut connection_height = None; - for check_height in (0..=current_filter_tip).rev() { - if let Ok(Some(stored_header)) = storage.get_filter_header(check_height).await { - if stored_header == cf_headers.previous_filter_header { - connection_height = Some(check_height); - break; - } - } + // Compute filter headers for the entire batch WITHOUT verifying against local chain yet. + // This lets us compare continuity precisely at the overlap boundary rather than the + // batch's original start (which may precede our local tip). + let mut computed_headers: Vec = + Vec::with_capacity(cf_headers.filter_hashes.len()); + let mut prev_header = cf_headers.previous_filter_header; + for filter_hash in &cf_headers.filter_hashes { + let mut data = [0u8; 64]; + data[..32].copy_from_slice(filter_hash.as_byte_array()); + data[32..].copy_from_slice(prev_header.as_byte_array()); + let header = FilterHeader::from_byte_array(sha256d::Hash::hash(&data).to_byte_array()); + computed_headers.push(header); + prev_header = header; } - let connection_height = match connection_height { - Some(height) => height, + // Verify continuity exactly at the expected overlap boundary: expected_start_height - 1 + let expected_prev_height = match expected_start_height.checked_sub(1) { + Some(h) => h, None => { - // Special-case: checkpoint overlap where peer starts at checkpoint height - // and we expect to start at checkpoint+1. We don't store the checkpoint's - // filter header in storage, but CFHeaders provides previous_filter_header - // for (checkpoint-1), allowing us to compute from checkpoint onward and skip one. - if self.sync_base_height > 0 - && ( - // Case A: peer starts at checkpoint, we expect checkpoint+1 - (batch_start_height == self.sync_base_height - && expected_start_height == self.sync_base_height + 1) - || - // Case B: peer starts one before checkpoint, we expect checkpoint - (batch_start_height + 1 == self.sync_base_height - && expected_start_height == self.sync_base_height) - ) - { - tracing::debug!( - "Overlap at checkpoint: synthesizing connection at height {}", - self.sync_base_height - 1 - ); - self.sync_base_height - 1 - } else { - // No connection found - check if this is overlapping data we can safely ignore - let overlap_end = expected_start_height.saturating_sub(1); - if batch_start_height <= overlap_end && overlap_end <= current_filter_tip { - tracing::warn!( - "📋 Ignoring overlapping headers from different peer view (range {}-{})", - batch_start_height, - stop_height - ); - return Ok((0, expected_start_height)); - } else { - return Err(SyncError::Validation( - "Cannot find connection point for overlapping headers".to_string(), - )); - } - } + // Should not happen since expected_start_height > 0 in overlap handling + return Ok((0, expected_start_height)); } }; - // Process all filter headers from the connection point - let batch_start_height = connection_height + 1; - let all_filter_headers = - self.process_filter_headers(cf_headers, batch_start_height, storage).await?; + // Determine the computed header at expected_prev_height using the batch data + let steps_to_expected_prev = expected_start_height.saturating_sub(original_start_height); + let computed_prev_at_expected = if steps_to_expected_prev == 0 { + cf_headers.previous_filter_header + } else { + // steps_to_expected_prev >= 1 implies index exists + computed_headers[(steps_to_expected_prev - 1) as usize] + }; - // Extract only the new headers we need - let headers_to_skip = expected_start_height.saturating_sub(batch_start_height) as usize; - if headers_to_skip >= all_filter_headers.len() { + // Load our local header at expected_prev_height + let local_prev_at_expected = match storage.get_filter_header(expected_prev_height).await { + Ok(Some(h)) => h, + Ok(None) => { + tracing::warn!( + "Missing local filter header at height {} while handling overlap; skipping batch", + expected_prev_height + ); + return Ok((0, expected_start_height)); + } + Err(e) => { + return Err(SyncError::Storage(format!( + "Failed to read local filter header at height {}: {}", + expected_prev_height, e + ))); + } + }; + + // If continuity at the overlap boundary doesn't match, ignore this overlapping batch + if computed_prev_at_expected != local_prev_at_expected { + tracing::warn!( + "Overlapping CFHeaders batch does not connect at height {}: computed={:?}, local={:?}. Ignoring batch.", + expected_prev_height, + computed_prev_at_expected, + local_prev_at_expected + ); return Ok((0, expected_start_height)); } - let new_filter_headers = all_filter_headers[headers_to_skip..].to_vec(); + // Store only the non-overlapping suffix starting at expected_start_height + let start_index = steps_to_expected_prev as usize; + let new_filter_headers = if start_index < computed_headers.len() { + computed_headers[start_index..].to_vec() + } else { + Vec::new() + }; if !new_filter_headers.is_empty() { storage.store_filter_headers(&new_filter_headers).await.map_err(|e| { diff --git a/dash-spv/src/sync/sequential/manager.rs b/dash-spv/src/sync/sequential/manager.rs index 456af94b3..01247b55f 100644 --- a/dash-spv/src/sync/sequential/manager.rs +++ b/dash-spv/src/sync/sequential/manager.rs @@ -227,6 +227,15 @@ impl< self.masternode_sync.engine() } + /// Get a quorum manager populated from the masternode engine. + /// Returns an empty quorum manager if masternodes are disabled or not initialized. + pub fn get_quorum_manager(&self) -> crate::validation::quorum::QuorumManager { + // TODO: Populate quorum manager from masternode engine + // For now, return an empty manager + // This will be enhanced when quorum extraction from masternode list is implemented + crate::validation::quorum::QuorumManager::new() + } + /// Get a reference to the filter sync manager. pub fn filter_sync(&self) -> &FilterSyncManager { &self.filter_sync diff --git a/dash-spv/src/sync/sequential/post_sync.rs b/dash-spv/src/sync/sequential/post_sync.rs index 3412af111..016d04747 100644 --- a/dash-spv/src/sync/sequential/post_sync.rs +++ b/dash-spv/src/sync/sequential/post_sync.rs @@ -353,12 +353,9 @@ impl< stop_height ); } else { - // Start from the lesser of filter_tip and (stop_height - 1) - let mut start_height = stop_height.saturating_sub(1); - if filter_tip < start_height { - // normal case: request from tip up to stop - start_height = filter_tip; - } + // Request from the first missing height after our current filter tip + // We already verified filter_tip < stop_height above + let start_height = filter_tip.saturating_add(1); tracing::info!( "📋 Requesting filter headers up to height {} (start: {}, stop: {})", diff --git a/dash-spv/src/types.rs b/dash-spv/src/types.rs index 8a54fb6b7..a41e95297 100644 --- a/dash-spv/src/types.rs +++ b/dash-spv/src/types.rs @@ -899,6 +899,14 @@ pub enum SpvEvent { hash: dashcore::BlockHash, }, + /// InstantLock received and validated. + InstantLockReceived { + /// Transaction ID locked by this InstantLock. + txid: Txid, + /// Transaction inputs locked by this InstantLock. + inputs: Vec, + }, + /// Unconfirmed transaction added to mempool. MempoolTransactionAdded { /// Transaction ID. diff --git a/dash-spv/src/validation/instantlock.rs b/dash-spv/src/validation/instantlock.rs index 6919408d2..7e040df79 100644 --- a/dash-spv/src/validation/instantlock.rs +++ b/dash-spv/src/validation/instantlock.rs @@ -1,12 +1,14 @@ //! InstantLock validation functionality. use dashcore::InstantLock; +use dashcore_hashes::Hash; use crate::error::{ValidationError, ValidationResult}; /// Validates InstantLock messages. pub struct InstantLockValidator { - // TODO: Add masternode list for signature verification + // Quorum manager is now passed as parameter to validate_signature + // to avoid circular dependencies and allow flexible usage } impl Default for InstantLockValidator { @@ -21,22 +23,46 @@ impl InstantLockValidator { Self {} } - /// Validate an InstantLock. - pub fn validate(&self, instant_lock: &InstantLock) -> ValidationResult<()> { - // Basic structural validation + /// Validate an InstantLock with full BLS signature verification. + /// + /// This performs complete validation including: + /// - Structural validation (non-zero txid, signature, inputs) + /// - BLS signature verification using cyclehash-based quorum selection (DIP 24) + /// + /// **Security Critical**: This method requires a masternode engine to verify + /// BLS signatures. Never accept InstantLocks from the network without full + /// signature verification. + pub fn validate( + &self, + instant_lock: &InstantLock, + masternode_engine: &dashcore::sml::masternode_list_engine::MasternodeListEngine, + ) -> ValidationResult<()> { + // Perform structural validation self.validate_structure(instant_lock)?; - // TODO: Validate signature using masternode list - // For now, we just do basic validation - tracing::debug!("InstantLock validation passed for txid {}", instant_lock.txid); + // Perform BLS signature verification (REQUIRED for security) + self.validate_signature(instant_lock, masternode_engine)?; + + tracing::debug!( + "InstantLock fully validated (structure + signature) for txid {}", + instant_lock.txid + ); Ok(()) } - /// Validate InstantLock structure. - fn validate_structure(&self, instant_lock: &InstantLock) -> ValidationResult<()> { - // Check transaction ID is not zero (we'll skip this check for now) - // TODO: Implement proper null txid check + /// Validate InstantLock structure (without BLS signature verification). + /// + /// **WARNING**: This is insufficient for accepting network messages. + /// For production use, always call `validate()` with a masternode engine. + /// This method is public only for testing purposes. + pub fn validate_structure(&self, instant_lock: &InstantLock) -> ValidationResult<()> { + // Check transaction ID is not zero (null txid) + if instant_lock.txid == dashcore::Txid::all_zeros() { + return Err(ValidationError::InvalidInstantLock( + "InstantLock transaction ID cannot be zero".to_string(), + )); + } // Check signature is not zero (null signature) if instant_lock.signature.is_zeroed() { @@ -52,27 +78,46 @@ impl InstantLockValidator { )); } - // Validate each input (we'll skip null check for now) - // TODO: Implement proper null input check + // Validate each input - ensure no input has a null txid + for (idx, input) in instant_lock.inputs.iter().enumerate() { + if input.txid == dashcore::Txid::all_zeros() { + return Err(ValidationError::InvalidInstantLock(format!( + "InstantLock input {} has null transaction ID", + idx + ))); + } + } Ok(()) } - /// Validate InstantLock signature (requires masternode quorum info). + /// Validate InstantLock signature using the masternode list engine. + /// + /// This properly uses the cyclehash to select the correct quorum according to DIP 24. + /// The MasternodeListEngine tracks rotated quorums per cycle and selects the specific + /// quorum based on the request_id. pub fn validate_signature( &self, - _instant_lock: &InstantLock, - // TODO: Add masternode list parameter + instant_lock: &InstantLock, + masternode_engine: &dashcore::sml::masternode_list_engine::MasternodeListEngine, ) -> ValidationResult<()> { - // TODO: Implement proper signature validation - // This requires: - // 1. Active quorum information for InstantSend - // 2. BLS signature verification - // 3. Quorum member validation - // 4. Input validation against the transaction - - // For now, we skip signature validation - tracing::warn!("InstantLock signature validation not implemented"); + // Use the proper verification from the masternode engine which: + // 1. Uses cyclehash to get the set of rotated quorums + // 2. Uses request_id to select the specific quorum (DIP 24) + // 3. Verifies the BLS signature with that quorum's public key + masternode_engine.verify_is_lock(instant_lock).map_err(|e| { + ValidationError::InvalidSignature(format!( + "InstantLock BLS signature verification failed: {}", + e + )) + })?; + + tracing::debug!( + "InstantLock signature verified for txid {} using cyclehash {:x}", + instant_lock.txid, + instant_lock.cyclehash + ); + Ok(()) } @@ -180,7 +225,8 @@ mod tests { let tx = create_test_transaction(vec![(sha256d::Hash::hash(&[1, 2, 3]), 0)], COIN_VALUE); let is_lock = create_test_instant_lock(&tx); - assert!(validator.validate(&is_lock).is_ok()); + // Structural validation only (for testing) + assert!(validator.validate_structure(&is_lock).is_ok()); } #[test] @@ -192,7 +238,7 @@ mod tests { ); is_lock.inputs.clear(); - let result = validator.validate(&is_lock); + let result = validator.validate_structure(&is_lock); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("at least one input")); } @@ -207,11 +253,43 @@ mod tests { is_lock.signature = dashcore::bls_sig_utils::BLSSignature::from([0; 96]); // Zero signatures should be rejected as invalid structure - let result = validator.validate(&is_lock); + let result = validator.validate_structure(&is_lock); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("signature cannot be zero")); } + #[test] + fn test_null_txid() { + let validator = InstantLockValidator::new(); + let mut is_lock = create_instant_lock_with_inputs( + sha256d::Hash::hash(&[1, 2, 3]), + vec![(sha256d::Hash::hash(&[4, 5, 6]), 0)], + ); + is_lock.txid = dashcore::Txid::all_zeros(); + + // Null txid should be rejected as invalid structure + let result = validator.validate_structure(&is_lock); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("transaction ID cannot be zero")); + } + + #[test] + fn test_null_input_txid() { + let validator = InstantLockValidator::new(); + let mut is_lock = create_instant_lock_with_inputs( + sha256d::Hash::hash(&[1, 2, 3]), + vec![(sha256d::Hash::hash(&[4, 5, 6]), 0), (sha256d::Hash::hash(&[7, 8, 9]), 1)], + ); + // Set the second input to have a null txid + is_lock.inputs[1].txid = dashcore::Txid::all_zeros(); + + // Null input txid should be rejected as invalid structure + let result = validator.validate_structure(&is_lock); + assert!(result.is_err()); + let err_str = result.unwrap_err().to_string(); + assert!(err_str.contains("input") && err_str.contains("null transaction ID")); + } + #[test] fn test_conflicts_with_same_input() { let validator = InstantLockValidator::new(); @@ -288,14 +366,27 @@ mod tests { assert!(validator.is_still_valid(&is_lock)); } + // Note: test_signature_validation_without_quorum has been removed as BLS signature + // verification now requires MasternodeListEngine, not the simplified QuorumManager. + + // Note: test_signature_validation_with_quorum_invalid_signature has been removed + // as BLS signature verification now requires MasternodeListEngine with properly + // populated rotated quorums implementing DIP 24 quorum selection. + #[test] - fn test_signature_validation_stub() { - let validator = InstantLockValidator::new(); + fn test_request_id_computation() { let tx = create_test_transaction(vec![(sha256d::Hash::hash(&[1, 2, 3]), 0)], COIN_VALUE); let is_lock = create_test_instant_lock(&tx); - // Should pass for now (not implemented) - assert!(validator.validate_signature(&is_lock).is_ok()); + // Verify request ID can be computed + let request_id = is_lock.request_id(); + assert!(request_id.is_ok()); + + // Same inputs should produce same request ID + let is_lock2 = create_test_instant_lock(&tx); + let request_id2 = is_lock2.request_id(); + assert!(request_id2.is_ok()); + assert_eq!(request_id.unwrap(), request_id2.unwrap()); } #[test] @@ -309,7 +400,7 @@ mod tests { let lock = create_instant_lock_with_inputs(sha256d::Hash::hash(&[100, 101, 102]), many_inputs); - assert!(validator.validate(&lock).is_ok()); + assert!(validator.validate_structure(&lock).is_ok()); } #[test] diff --git a/dash-spv/src/validation/mod.rs b/dash-spv/src/validation/mod.rs index 9d7e2035d..57feab2a6 100644 --- a/dash-spv/src/validation/mod.rs +++ b/dash-spv/src/validation/mod.rs @@ -59,12 +59,18 @@ impl ValidationManager { } } - /// Validate an InstantLock. + /// Validate an InstantLock (structural validation only). + /// + /// **WARNING**: This only performs structural validation without BLS signature + /// verification. For network messages, the caller must use the InstantLockValidator + /// directly with a masternode engine to ensure full security. pub fn validate_instantlock(&self, instantlock: &InstantLock) -> ValidationResult<()> { match self.mode { ValidationMode::None => Ok(()), ValidationMode::Basic | ValidationMode::Full => { - self.instantlock_validator.validate(instantlock) + // Only structural validation - signature verification requires masternode engine + // which should be passed by the caller when processing network messages + self.instantlock_validator.validate_structure(instantlock) } } } diff --git a/dash-spv/src/validation/quorum.rs b/dash-spv/src/validation/quorum.rs index 5f63d1bdc..c14f20b2e 100644 --- a/dash-spv/src/validation/quorum.rs +++ b/dash-spv/src/validation/quorum.rs @@ -2,9 +2,11 @@ //! //! This module implements quorum tracking and validation according to DIP6/DIP7. -use dashcore::{bls_sig_utils::BLSSignature, BlockHash}; +use blsful::Bls12381G2Impl; +use dashcore::bls_sig_utils::{BLSPublicKey, BLSSignature}; +use dashcore::BlockHash; use std::collections::HashMap; -use tracing::{debug, info, warn}; +use tracing::{debug, info}; use crate::error::{ValidationError, ValidationResult}; use crate::types::ChainState; @@ -139,8 +141,8 @@ impl QuorumManager { pub fn verify_signature( &self, quorum_type: QuorumType, - _message: &[u8], - _signature: &BLSSignature, + message: &[u8], + signature: &BLSSignature, signing_height: u32, ) -> ValidationResult<()> { // Get the appropriate quorum @@ -152,15 +154,50 @@ impl QuorumManager { )) })?; - debug!("Verifying {:?} signature with quorum from height {}", quorum_type, quorum.height); + debug!( + "Verifying {:?} signature with quorum from height {} (quorum hash: {:x})", + quorum_type, quorum.height, quorum.quorum_hash + ); + + // Convert the quorum public key from Vec to BLSPublicKey + if quorum.public_key.len() != 48 { + return Err(ValidationError::InvalidSignature(format!( + "Invalid quorum public key length: expected 48 bytes, got {}", + quorum.public_key.len() + ))); + } - // TODO: Implement actual BLS signature verification - // This requires: - // 1. Deserializing the quorum public key - // 2. Verifying the signature against the message - // 3. Ensuring the signature is valid + let mut pk_bytes = [0u8; 48]; + pk_bytes.copy_from_slice(&quorum.public_key); + let bls_public_key = BLSPublicKey::from(pk_bytes); + + // Convert BLSPublicKey to blsful::PublicKey + let public_key: blsful::PublicKey = + bls_public_key.try_into().map_err(|e| { + ValidationError::InvalidSignature(format!( + "Failed to convert quorum public key: {}", + e + )) + })?; + + // Convert BLSSignature to blsful::Signature + let bls_signature: blsful::Signature = + signature.try_into().map_err(|e| { + ValidationError::InvalidSignature(format!("Failed to convert BLS signature: {}", e)) + })?; - warn!("BLS signature verification not implemented - accepting signature"); + // Verify the signature + bls_signature.verify(&public_key, message).map_err(|e| { + ValidationError::InvalidSignature(format!( + "BLS signature verification failed for {:?} quorum at height {}: {}", + quorum_type, quorum.height, e + )) + })?; + + debug!( + "Successfully verified {:?} signature with quorum at height {}", + quorum_type, quorum.height + ); Ok(()) } diff --git a/dash-spv/tests/error_handling_test.rs b/dash-spv/tests/error_handling_test.rs index 279a18894..abda7dde2 100644 --- a/dash-spv/tests/error_handling_test.rs +++ b/dash-spv/tests/error_handling_test.rs @@ -495,26 +495,7 @@ impl StorageManager for MockStorageManager { Ok(vec![]) } - async fn store_instant_lock( - &mut self, - _txid: dashcore::Txid, - _instant_lock: &dashcore::InstantLock, - ) -> StorageResult<()> { - if self.fail_on_write { - return Err(StorageError::WriteFailed("Mock write failure".to_string())); - } - Ok(()) - } - - async fn load_instant_lock( - &self, - _txid: dashcore::Txid, - ) -> StorageResult> { - if self.fail_on_read { - return Err(StorageError::ReadFailed("Mock read failure".to_string())); - } - Ok(None) - } + // InstantLock storage methods removed from trait; no-op here async fn store_mempool_transaction( &mut self, diff --git a/dash-spv/tests/error_recovery_integration_test.rs b/dash-spv/tests/error_recovery_integration_test.rs index 218969ac3..e123da9b5 100644 --- a/dash-spv/tests/error_recovery_integration_test.rs +++ b/dash-spv/tests/error_recovery_integration_test.rs @@ -753,20 +753,7 @@ impl StorageManager for MockStorageManager { Ok(vec![]) } - async fn store_instant_lock( - &mut self, - _txid: Txid, - _instant_lock: &dashcore::InstantLock, - ) -> dash_spv::error::StorageResult<()> { - Ok(()) - } - - async fn load_instant_lock( - &self, - _txid: Txid, - ) -> dash_spv::error::StorageResult> { - Ok(None) - } + // InstantLock storage methods removed from trait; no-op here async fn store_mempool_transaction( &mut self, diff --git a/dash-spv/tests/instantsend_integration_test.rs b/dash-spv/tests/instantsend_integration_test.rs deleted file mode 100644 index ef3c7b317..000000000 --- a/dash-spv/tests/instantsend_integration_test.rs +++ /dev/null @@ -1,184 +0,0 @@ -#![cfg(feature = "skip_mock_implementation_incomplete")] - -// This test is currently disabled because the WalletManager API has changed -// and these methods don't exist anymore. The test needs to be rewritten to use -// the new wallet interface. - -// dash-spv/tests/instantsend_integration_test.rs -// -// TODO: These tests need to be updated to work with the new WalletManager API -// The following methods don't exist in WalletManager: -// - add_utxo -// - add_watched_address -// - get_utxos -// - get_balance (should be get_total_balance) -// - process_verified_instantlock -// -// These tests are currently ignored until they can be properly updated. - -// use std::sync::Arc; -// use tokio::sync::RwLock; - -use blsful::{Bls12381G2Impl, SecretKey}; -// keep module path available for validator usage -use dashcore::{ - Address, InstantLock, Network, OutPoint, ScriptBuf, Transaction, TxIn, TxOut, Txid, Witness, -}; -use dashcore_hashes::Hash; -// use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; -// use key_wallet_manager::wallet_manager::WalletManager; -use rand::thread_rng; - -// /// Helper to create a test wallet manager. -// Removed unused helper create_test_wallet (test scaffolding simplified) -/// Create a deterministic test address. -fn create_test_address() -> Address { - let pubkey_hash = dashcore::PubkeyHash::from_byte_array([1; 20]); - let script = ScriptBuf::new_p2pkh(&pubkey_hash); - Address::from_script(&script, Network::Testnet).unwrap() -} - -/// Create a regular transaction. -fn create_regular_transaction( - inputs: Vec, - outputs: Vec<(u64, ScriptBuf)>, -) -> Transaction { - let tx_inputs = inputs - .into_iter() - .map(|outpoint| TxIn { - previous_output: outpoint, - script_sig: ScriptBuf::new(), - sequence: 0xffffffff, - witness: Witness::new(), - }) - .collect(); - - let tx_outputs = outputs - .into_iter() - .map(|(value, script)| TxOut { - value, - script_pubkey: script, - }) - .collect(); - - Transaction { - version: 1, - lock_time: 0, - input: tx_inputs, - output: tx_outputs, - special_transaction_payload: None, - } -} - -/// Create a signed InstantLock for a transaction. -fn create_signed_instantlock(tx: &Transaction, _sk: &SecretKey) -> InstantLock { - let inputs = tx.input.iter().map(|input| input.previous_output).collect(); - - // Create a non-zero dummy signature that will pass basic validation - let mut sig_bytes = [0u8; 96]; - sig_bytes[0] = 0x01; // Set first byte to make it non-zero - sig_bytes[95] = 0x01; // Set last byte too for good measure - - // TODO: Implement proper signing when InstantLockValidator methods are available - InstantLock { - version: 1, - inputs, - txid: tx.txid(), - signature: dashcore::bls_sig_utils::BLSSignature::from(sig_bytes), - cyclehash: dashcore::BlockHash::from_byte_array([0; 32]), - } -} - -#[tokio::test] -#[ignore = "instantsend tests not yet updated"] -async fn test_instantsend_end_to_end() { - // 1. Create a dummy spending transaction (skipped wallet operations due to API changes) - let spend_tx = create_regular_transaction(vec![], vec![(80_000_000, ScriptBuf::new())]); - - // At this point, the transaction is in the mempool (conceptually). - // The wallet balance would show the initial_amount as confirmed. - - // 3. Create a valid InstantLock for the spending transaction. - let sk = SecretKey::::random(&mut thread_rng()); - let instant_lock = create_signed_instantlock(&spend_tx, &sk); - - // 4. Simulate the client receiving and processing the InstantLock. - // We need to mock the quorum lookup. - // For this test, we will directly call the validation and wallet update. - - // First, validate the instantlock. - let validator = dash_spv::validation::InstantLockValidator::new(); - assert!(validator.validate(&instant_lock).is_ok()); - - // Now, process it with the wallet. - // Note: This won't update anything because spend_tx is spending FROM our wallet, - // not creating new UTXOs for us. We'll test InstantLock processing in the next section. - - // 5. Assert the wallet state has been updated correctly. - // TODO: get_utxos() method no longer exists on WalletManager - // Need to access UTXOs through WalletInterface or base WalletManager - // let utxos = wallet.read().await.get_utxos().await; - // let spent_utxo = utxos.iter().find(|u| u.outpoint == initial_outpoint); - - // The original UTXO should now be marked as instant-locked. - // Note: In a real scenario, the UTXO would be *removed* and a new *change* UTXO added. - // For this test, we simplify by just marking the spent UTXO. - // A more realistic test would involve the TransactionProcessor. - // Let's adjust the test to reflect spending and receiving change. - - // Let's refine the test to be more realistic. - // We will process the transaction first, which will remove the old UTXO and add a change UTXO. - // Then we will process the InstantLock. - - // This test setup is getting complicated without the full block processor. - // Let's simplify and focus on the direct impact of the InstantLock on a UTXO. - - // Let's create a new UTXO that represents a payment *to* us, and then InstantLock it. - let address = create_test_address(); - // TODO: add_watched_address() method no longer exists - // wallet.write().await.add_watched_address(address.clone()).await.unwrap(); - - let incoming_amount = 50_000_000; - // Create a transaction with a dummy input (from external source) - let dummy_input = OutPoint { - txid: Txid::from_byte_array([99; 32]), - vout: 0, - }; - let incoming_tx = create_regular_transaction( - vec![dummy_input], - vec![(incoming_amount, address.script_pubkey())], - ); - // Create an outpoint for the received UTXO (skipped due to API changes) - // TODO: add_utxo() method no longer exists - // wallet.write().await.add_utxo(incoming_utxo).await.unwrap(); - - // Balance should be pending. - // TODO: get_balance() method no longer exists - need to use get_total_balance() or similar - // let balance1 = wallet.read().await.get_balance().await.unwrap(); - // assert_eq!(balance1.pending, Amount::from_sat(incoming_amount)); - // assert_eq!(balance1.instantlocked, Amount::ZERO); - - // Create and process the InstantLock. - let sk = SecretKey::::random(&mut thread_rng()); - let instant_lock = create_signed_instantlock(&incoming_tx, &sk); - - let validator = dash_spv::validation::InstantLockValidator::new(); - assert!(validator.validate(&instant_lock).is_ok()); - - // TODO: process_verified_instantlock() method no longer exists - // let updated = - // wallet.write().await.process_verified_instantlock(incoming_tx.txid()).await.unwrap(); - // assert!(updated); - - // Verify the UTXO is now marked as instant-locked. - // TODO: get_utxos() method no longer exists - // let utxos = wallet.read().await.get_utxos().await; - // let locked_utxo = utxos.iter().find(|u| u.outpoint == incoming_outpoint).unwrap(); - // assert!(locked_utxo.is_instantlocked); - - // Verify the balance has moved from pending to instantlocked. - // TODO: get_balance() method no longer exists - // let balance2 = wallet.read().await.get_balance().await.unwrap(); - // assert_eq!(balance2.pending, Amount::ZERO); - // assert_eq!(balance2.instantlocked, Amount::from_sat(incoming_amount)); -} diff --git a/dash-spv/tests/test_handshake_logic.rs b/dash-spv/tests/test_handshake_logic.rs index 6cc89f620..091ace062 100644 --- a/dash-spv/tests/test_handshake_logic.rs +++ b/dash-spv/tests/test_handshake_logic.rs @@ -6,7 +6,7 @@ use dashcore::Network; #[test] fn test_handshake_state_transitions() { - let mut handshake = HandshakeManager::new(Network::Dash, MempoolStrategy::Selective, None); + let mut handshake = HandshakeManager::new(Network::Dash, MempoolStrategy::BloomFilter, None); // Initial state should be Init assert_eq!(*handshake.state(), HandshakeState::Init);