refactor: Cleanup HeaderValidator and ValidationManager

- Some simplifications and name changes
- Some unused code
- Dropping the network change parts since the rest of the codebase doesnt support it anyway
- Making the `ValidationMode::Full` mode checking PoW by default, not based on yet another parameter

Author: xdustinface

dash-spv/src/client/lifecycle.rs

@@ -62,7 +62,7 @@ impl<
         .map_err(SpvError::Sync)?;
 
         // Create validation manager
-        let validation = ValidationManager::new(config.validation_mode);
+        let validation = ValidationManager::new(config.validation_mode, config.network);
 
         // Create ChainLock manager
         let chainlock_manager = Arc::new(ChainLockManager::new(true));

dash-spv/src/client/sync_coordinator.rs

@@ -931,7 +931,7 @@ impl<
             // Validate headers before adding to chain state
             {
                 // Validate the batch of headers
-                if let Err(e) = self.validation.validate_header_chain(&headers, false) {
+                if let Err(e) = self.validation.validate_headers(&headers) {
                     tracing::error!(
                         "Header validation failed for range {}..{}: {:?}",
                         current_height,

dash-spv/src/validation/headers.rs

@@ -16,10 +16,10 @@ pub struct HeaderValidator {
 
 impl HeaderValidator {
     /// Create a new header validator.
-    pub fn new(mode: ValidationMode) -> Self {
+    pub fn new(mode: ValidationMode, network: Network) -> Self {
         Self {
             mode,
-            network: Network::Dash, // Default to mainnet
+            network,
         }
     }
 
@@ -28,11 +28,6 @@ impl HeaderValidator {
         self.mode = mode;
     }
 
-    /// Set network.
-    pub fn set_network(&mut self, network: Network) {
-        self.network = network;
-    }
-
     /// Validate a single header.
     pub fn validate(
         &self,
@@ -76,55 +71,23 @@ impl HeaderValidator {
         // 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);
-                }
+            return match e {
+                DashError::BlockBadProofOfWork => Err(ValidationError::InvalidProofOfWork),
                 DashError::BlockBadTarget => {
-                    return Err(ValidationError::InvalidHeaderChain("Invalid target".to_string()));
+                    Err(ValidationError::InvalidHeaderChain("Invalid target".to_string()))
                 }
-                _ => {
-                    return 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))?;
+                _ => Err(ValidationError::InvalidHeaderChain(format!(
+                    "PoW validation error: {:?}",
+                    e
+                ))),
+            };
         }
 
-        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
+    /// Validate a chain of headers considering the validation mode.
+    pub fn validate_headers(&self, headers: &[BlockHeader]) -> ValidationResult<()> {
         if self.mode == ValidationMode::None {
             return Ok(());
         }
@@ -145,14 +108,14 @@ impl HeaderValidator {
                 None
             };
 
-            if validate_pow {
-                self.validate_full(header, prev_header)?;
-            } else {
-                self.validate_basic(header, prev_header)?;
-            }
+            self.validate(header, prev_header)?;
         }
 
-        tracing::debug!("Full header chain validation passed for {} headers", headers.len());
+        tracing::debug!(
+            "Header chain validation passed for {} headers in mode: {:?}",
+            headers.len(),
+            self.mode
+        );
         Ok(())
     }
 
@@ -174,23 +137,6 @@ impl HeaderValidator {
 
         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(())
-    }
 }
 
 #[cfg(test)]

dash-spv/src/validation/headers_edge_test.rs

@@ -33,10 +33,9 @@ mod tests {
 
     #[test]
     fn test_genesis_block_validation() {
-        let mut validator = HeaderValidator::new(ValidationMode::Full);
-
         for network in [Network::Dash, Network::Testnet, Network::Regtest] {
-            validator.set_network(network);
+            let validator = HeaderValidator::new(ValidationMode::Full, network);
+
             let genesis = genesis_block(network).header;
 
             // Genesis block should validate with no previous header
@@ -50,7 +49,7 @@ mod tests {
 
     #[test]
     fn test_maximum_target_validation() {
-        let validator = HeaderValidator::new(ValidationMode::Full);
+        let validator = HeaderValidator::new(ValidationMode::Full, Network::Dash);
 
         // Create header with maximum allowed target (easiest difficulty)
         let max_target_bits = 0x1e0fffff; // Maximum target for testing
@@ -71,7 +70,7 @@ mod tests {
 
     #[test]
     fn test_minimum_target_validation() {
-        let validator = HeaderValidator::new(ValidationMode::Full);
+        let validator = HeaderValidator::new(ValidationMode::Full, Network::Dash);
 
         // Create header with very low target (hardest difficulty)
         let min_target_bits = 0x17000000; // Very difficult target
@@ -93,7 +92,7 @@ mod tests {
 
     #[test]
     fn test_zero_prev_blockhash() {
-        let validator = HeaderValidator::new(ValidationMode::Basic);
+        let validator = HeaderValidator::new(ValidationMode::Basic, Network::Dash);
 
         // First header with zero prev_blockhash (like genesis)
         let header1 = create_test_header_with_params(
@@ -126,7 +125,7 @@ mod tests {
 
     #[test]
     fn test_all_ff_prev_blockhash() {
-        let validator = HeaderValidator::new(ValidationMode::Basic);
+        let validator = HeaderValidator::new(ValidationMode::Basic, Network::Dash);
 
         // Header with all 0xFF prev_blockhash
         let header = create_test_header_with_params(
@@ -162,7 +161,7 @@ mod tests {
 
     #[test]
     fn test_timestamp_boundaries() {
-        let validator = HeaderValidator::new(ValidationMode::Basic);
+        let validator = HeaderValidator::new(ValidationMode::Basic, Network::Dash);
 
         // Test with minimum timestamp (0)
         let header_min_time = create_test_header_with_params(
@@ -193,7 +192,7 @@ mod tests {
 
     #[test]
     fn test_version_edge_cases() {
-        let validator = HeaderValidator::new(ValidationMode::Basic);
+        let validator = HeaderValidator::new(ValidationMode::Basic, Network::Dash);
 
         // Test various version values
         let versions = [0, 1, 0x20000000, 0x20000001, u32::MAX];
@@ -217,7 +216,7 @@ mod tests {
 
     #[test]
     fn test_large_chain_validation() {
-        let validator = HeaderValidator::new(ValidationMode::Basic);
+        let validator = HeaderValidator::new(ValidationMode::Basic, Network::Dash);
 
         // Create a large chain
         let chain_size = 1000;
@@ -240,7 +239,7 @@ mod tests {
         }
 
         // Should validate entire chain
-        assert!(validator.validate_chain_basic(&headers).is_ok());
+        assert!(validator.validate_headers(&headers).is_ok());
 
         // Break the chain in the middle
         let broken_index = chain_size / 2;
@@ -256,14 +255,12 @@ mod tests {
         );
 
         // Should fail validation
-        let result = validator.validate_chain_basic(&headers);
+        let result = validator.validate_headers(&headers);
         assert!(matches!(result, Err(ValidationError::InvalidHeaderChain(_))));
     }
 
     #[test]
     fn test_single_header_chain_validation() {
-        let validator = HeaderValidator::new(ValidationMode::Full);
-
         let header = create_test_header_with_params(
             0x20000000,
             dashcore::BlockHash::from_raw_hash(dashcore_hashes::hash_x11::Hash::from_byte_array(
@@ -278,13 +275,17 @@ mod tests {
         let headers = vec![header];
 
         // Single header chain should validate in both basic and full modes
-        assert!(validator.validate_chain_basic(&headers).is_ok());
-        assert!(validator.validate_chain_full(&headers, false).is_ok());
+        assert!(HeaderValidator::new(ValidationMode::Basic, Network::Dash)
+            .validate_headers(&headers)
+            .is_ok());
+        assert!(HeaderValidator::new(ValidationMode::Full, Network::Dash)
+            .validate_headers(&headers)
+            .is_ok());
     }
 
     #[test]
     fn test_duplicate_headers_in_chain() {
-        let validator = HeaderValidator::new(ValidationMode::Basic);
+        let validator = HeaderValidator::new(ValidationMode::Basic, Network::Dash);
 
         let header = create_test_header_with_params(
             0x20000000,
@@ -301,13 +302,13 @@ mod tests {
         let headers = vec![header, header];
 
         // Should fail because second header's prev_blockhash won't match first header's hash
-        let result = validator.validate_chain_basic(&headers);
+        let result = validator.validate_headers(&headers);
         assert!(matches!(result, Err(ValidationError::InvalidHeaderChain(_))));
     }
 
     #[test]
     fn test_merkle_root_variations() {
-        let validator = HeaderValidator::new(ValidationMode::Basic);
+        let validator = HeaderValidator::new(ValidationMode::Basic, Network::Dash);
 
         // Test various merkle root patterns
         let merkle_patterns = [
@@ -340,7 +341,7 @@ mod tests {
 
     #[test]
     fn test_mode_switching_during_chain_validation() {
-        let mut validator = HeaderValidator::new(ValidationMode::None);
+        let mut validator = HeaderValidator::new(ValidationMode::None, Network::Dash);
 
         // Create headers with invalid PoW
         let mut headers = vec![];
@@ -361,18 +362,16 @@ mod tests {
             headers.push(header);
         }
 
-        // Should pass with None mode (ValidationMode::None always passes)
-        let result = validator.validate_chain_full(&headers, true);
+        let result = validator.validate_headers(&headers);
         assert!(result.is_ok(), "ValidationMode::None should always pass, but got: {:?}", result);
 
-        // Switch to Full mode
         validator.set_mode(ValidationMode::Full);
-
         // Should now fail due to invalid PoW
-        let result = validator.validate_chain_full(&headers, true);
+        let result = validator.validate_headers(&headers);
         assert!(matches!(result, Err(ValidationError::InvalidProofOfWork)));
 
-        // But should pass without PoW check
-        assert!(validator.validate_chain_full(&headers, false).is_ok());
+        validator.set_mode(ValidationMode::None);
+        let result = validator.validate_headers(&headers);
+        assert!(result.is_ok(), "ValidationMode::None should always pass, but got: {:?}", result);
     }
 }