refactor: Cleanup SPV validation (#237)

- Some simplifications and name changes
- Some unused code
- Add header validation timing logs
- Dropping `ValidationManager` as its not really used and not needed
- Dropping the `HeaderValidator` making `validate_headers` a standalone function now located in `sync::headers::validation`
- Making the `ValidationMode::Full` mode checking PoW by default, not based on yet another parameter

Author: xdustinface

dash-spv/src/client/core.rs

@@ -22,7 +22,6 @@ use crate::storage::StorageManager;
 use crate::sync::filters::FilterNotificationSender;
 use crate::sync::SyncManager;
 use crate::types::{ChainState, DetailedSyncProgress, MempoolState, SpvEvent, SpvStats};
-use crate::validation::ValidationManager;
 use key_wallet_manager::wallet_interface::WalletInterface;
 
 use super::{BlockProcessingTask, ClientConfig, StatusDisplay};
@@ -133,7 +132,6 @@ pub struct DashSpvClient<W: WalletInterface, N: NetworkManager, S: StorageManage
     ///
     /// The current design prioritizes simplicity and correctness over concurrent access.
     pub(super) sync_manager: SyncManager<S, N, W>,
-    pub(super) validation: ValidationManager,
     pub(super) chainlock_manager: Arc<ChainLockManager>,
     pub(super) running: Arc<RwLock<bool>>,
     #[cfg(feature = "terminal-ui")]

dash-spv/src/client/lifecycle.rs

@@ -19,7 +19,6 @@ use crate::network::NetworkManager;
 use crate::storage::StorageManager;
 use crate::sync::SyncManager;
 use crate::types::{ChainState, MempoolState, SpvStats};
-use crate::validation::ValidationManager;
 use dashcore::network::constants::NetworkExt;
 use dashcore_hashes::Hash;
 use key_wallet_manager::wallet_interface::WalletInterface;
@@ -61,9 +60,6 @@ impl<
         )
         .map_err(SpvError::Sync)?;
 
-        // Create validation manager
-        let validation = ValidationManager::new(config.validation_mode);
-
         // Create ChainLock manager
         let chainlock_manager = Arc::new(ChainLockManager::new(true));
 
@@ -87,7 +83,6 @@ impl<
             storage,
             wallet,
             sync_manager,
-            validation,
             chainlock_manager,
             running: Arc::new(RwLock::new(false)),
             #[cfg(feature = "terminal-ui")]

dash-spv/src/client/sync_coordinator.rs

@@ -18,6 +18,8 @@ use crate::network::constants::MESSAGE_RECEIVE_TIMEOUT;
 use crate::network::NetworkManager;
 use crate::storage::StorageManager;
 use crate::types::{DetailedSyncProgress, SyncProgress};
+use crate::validation::validate_headers;
+use crate::ValidationMode;
 use key_wallet_manager::wallet_interface::WalletInterface;
 use std::time::{Duration, Instant, SystemTime};
 use tokio::sync::mpsc::UnboundedReceiver;
@@ -946,7 +948,9 @@ impl<
             // Validate headers before adding to chain state
             {
                 // Validate the batch of headers
-                if let Err(e) = self.validation.validate_header_chain(&headers, false) {
+                // Use basic validation only, the headers anyway are already validated since they
+                // come from storage.
+                if let Err(e) = validate_headers(&headers, ValidationMode::Basic) {
                     tracing::error!(
                         "Header validation failed for range {}..{}: {:?}",
                         current_height,

dash-spv/src/validation/headers.rs

@@ -1,196 +1,58 @@
 //! Header validation functionality.
 
-use dashcore::{
-    block::Header as BlockHeader, error::Error as DashError, network::constants::NetworkExt,
-    Network,
-};
+use dashcore::{block::Header as BlockHeader, error::Error as DashError};
+use std::time::Instant;
 
 use crate::error::{ValidationError, ValidationResult};
 use crate::types::ValidationMode;
 
-/// Validates block headers.
-pub struct HeaderValidator {
-    mode: ValidationMode,
-    network: Network,
-}
-
-impl HeaderValidator {
-    /// Create a new header validator.
-    pub fn new(mode: ValidationMode) -> Self {
-        Self {
-            mode,
-            network: Network::Dash, // Default to mainnet
-        }
+/// Validate a chain of headers considering the validation mode.
+pub fn validate_headers(headers: &[BlockHeader], mode: ValidationMode) -> ValidationResult<()> {
+    if mode == ValidationMode::None {
+        tracing::debug!("Skipping header validation: disabled");
+        return Ok(());
     }
 
-    /// Set validation mode.
-    pub fn set_mode(&mut self, mode: ValidationMode) {
-        self.mode = mode;
-    }
-
-    /// Set network.
-    pub fn set_network(&mut self, network: Network) {
-        self.network = network;
-    }
+    let start = Instant::now();
 
-    /// Validate a single header.
-    pub fn validate(
-        &self,
-        header: &BlockHeader,
-        prev_header: Option<&BlockHeader>,
-    ) -> ValidationResult<()> {
-        match self.mode {
-            ValidationMode::None => Ok(()),
-            ValidationMode::Basic => self.validate_basic(header, prev_header),
-            ValidationMode::Full => self.validate_full(header, prev_header),
-        }
-    }
-
-    /// Basic header validation (structure and chain continuity).
-    fn validate_basic(
-        &self,
-        header: &BlockHeader,
-        prev_header: Option<&BlockHeader>,
-    ) -> ValidationResult<()> {
+    let mut prev_block_hash = None;
+    for header in headers {
         // Check chain continuity if we have previous header
-        if let Some(prev) = prev_header {
-            if header.prev_blockhash != prev.block_hash() {
+        if let Some(prev) = prev_block_hash {
+            if header.prev_blockhash != prev {
                 return Err(ValidationError::InvalidHeaderChain(
                     "Header does not connect to previous header".to_string(),
                 ));
             }
         }
 
-        Ok(())
-    }
-
-    /// Full header validation (includes PoW verification).
-    fn validate_full(
-        &self,
-        header: &BlockHeader,
-        prev_header: Option<&BlockHeader>,
-    ) -> ValidationResult<()> {
-        // First do basic validation
-        self.validate_basic(header, prev_header)?;
-
-        // Validate proof of work with X11 hashing (now enabled with core-block-hash-use-x11 feature)
-        let target = header.target();
-        if let Err(e) = header.validate_pow(target) {
-            match e {
-                DashError::BlockBadProofOfWork => {
-                    return Err(ValidationError::InvalidProofOfWork);
-                }
-                DashError::BlockBadTarget => {
-                    return Err(ValidationError::InvalidHeaderChain("Invalid target".to_string()));
-                }
-                _ => {
-                    return Err(ValidationError::InvalidHeaderChain(format!(
+        if mode == ValidationMode::Full {
+            // Validate proof of work with X11 hashing
+            let target = header.target();
+            if let Err(e) = header.validate_pow(target) {
+                return match e {
+                    DashError::BlockBadProofOfWork => Err(ValidationError::InvalidProofOfWork),
+                    DashError::BlockBadTarget => {
+                        Err(ValidationError::InvalidHeaderChain("Invalid target".to_string()))
+                    }
+                    _ => Err(ValidationError::InvalidHeaderChain(format!(
                         "PoW validation error: {:?}",
                         e
-                    )));
-                }
-            }
-        }
-
-        Ok(())
-    }
-
-    /// Validate a chain of headers with basic validation.
-    pub fn validate_chain_basic(&self, headers: &[BlockHeader]) -> ValidationResult<()> {
-        // Respect ValidationMode::None
-        if self.mode == ValidationMode::None {
-            return Ok(());
-        }
-
-        if headers.is_empty() {
-            return Ok(());
-        }
-
-        // Validate chain continuity
-        for i in 1..headers.len() {
-            let header = &headers[i];
-            let prev_header = &headers[i - 1];
-
-            self.validate_basic(header, Some(prev_header))?;
-        }
-
-        tracing::debug!("Basic header chain validation passed for {} headers", headers.len());
-        Ok(())
-    }
-
-    /// Validate a chain of headers with full validation.
-    pub fn validate_chain_full(
-        &self,
-        headers: &[BlockHeader],
-        validate_pow: bool,
-    ) -> ValidationResult<()> {
-        // Respect ValidationMode::None
-        if self.mode == ValidationMode::None {
-            return Ok(());
-        }
-
-        if headers.is_empty() {
-            return Ok(());
-        }
-
-        // For the first header, we might need to check it connects to genesis or our existing chain
-        // For now, we'll just validate internal chain continuity
-
-        // Validate each header in the chain
-        for i in 0..headers.len() {
-            let header = &headers[i];
-            let prev_header = if i > 0 {
-                Some(&headers[i - 1])
-            } else {
-                None
-            };
-
-            if validate_pow {
-                self.validate_full(header, prev_header)?;
-            } else {
-                self.validate_basic(header, prev_header)?;
+                    ))),
+                };
             }
         }
 
-        tracing::debug!("Full header chain validation passed for {} headers", headers.len());
-        Ok(())
+        prev_block_hash = Some(header.block_hash());
     }
 
-    /// Validate headers connect to genesis block.
-    pub fn validate_connects_to_genesis(&self, headers: &[BlockHeader]) -> ValidationResult<()> {
-        if headers.is_empty() {
-            return Ok(());
-        }
-
-        let genesis_hash = self.network.known_genesis_block_hash().ok_or_else(|| {
-            ValidationError::Consensus("No known genesis hash for network".to_string())
-        })?;
-
-        if headers[0].prev_blockhash != genesis_hash {
-            return Err(ValidationError::InvalidHeaderChain(
-                "First header doesn't connect to genesis".to_string(),
-            ));
-        }
-
-        Ok(())
-    }
-
-    /// Validate difficulty adjustment (simplified for SPV).
-    pub fn validate_difficulty_adjustment(
-        &self,
-        header: &BlockHeader,
-        prev_header: &BlockHeader,
-    ) -> ValidationResult<()> {
-        // For SPV client, we trust that the network has validated difficulty properly
-        // We only check basic constraints
-
-        // For SPV we trust the network for difficulty validation
-        // TODO: Implement proper difficulty validation if needed
-        let _prev_target = prev_header.target();
-        let _current_target = header.target();
-
-        Ok(())
-    }
+    tracing::debug!(
+        "Header chain validation passed for {} headers in mode: {:?}, duration: {:?}",
+        headers.len(),
+        mode,
+        start.elapsed(),
+    );
+    Ok(())
 }
 
 #[cfg(test)]