chore: Move header validation into sync::headers::validation (#238)

Author: xdustinface

dash-spv/src/client/sync_coordinator.rs

@@ -17,8 +17,8 @@ use crate::error::{Result, SpvError};
 use crate::network::constants::MESSAGE_RECEIVE_TIMEOUT;
 use crate::network::NetworkManager;
 use crate::storage::StorageManager;
+use crate::sync::headers::validate_headers;
 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};

dash-spv/src/sync/headers/mod.rs

@@ -1,5 +1,7 @@
 //! Header synchronization with fork detection and reorganization handling.
 
 mod manager;
+pub mod validation;
 
 pub use manager::{HeaderSyncManager, ReorgConfig};
+pub use validation::validate_headers;

…-spv/src/validation/headers_edge_test.rs dash-spv/src/sync/headers/validation.rsdash-spv/src/validation/headers_edge_test.rs renamed to dash-spv/src/sync/headers/validation.rs dash-spv/src/validation/headers_edge_test.rs renamed to dash-spv/src/sync/headers/validation.rs

@@ -1,8 +1,68 @@
-//! Edge case tests for header validation.
+//! Header validation functionality.
+
+use dashcore::{block::Header as BlockHeader, error::Error as DashError};
+use std::time::Instant;
+
+use crate::error::{ValidationError, ValidationResult};
+use crate::types::ValidationMode;
+
+/// 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(());
+    }
+
+    if headers.is_empty() {
+        tracing::debug!("Skipping header validation: empty headers");
+        return Ok(());
+    }
+
+    let start = Instant::now();
+
+    let mut prev_header_hash = None;
+    for header in headers {
+        // Check chain continuity if we have previous header
+        if let Some(prev) = prev_header_hash {
+            if header.prev_blockhash != prev {
+                return Err(ValidationError::InvalidHeaderChain(
+                    "Header does not connect to previous header".to_string(),
+                ));
+            }
+        }
+
+        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
+                    ))),
+                };
+            }
+        }
+
+        prev_header_hash = Some(header.block_hash());
+    }
+
+    tracing::debug!(
+        "Header chain validation passed for {} headers in mode: {:?}, duration: {:?}",
+        headers.len(),
+        mode,
+        start.elapsed(),
+    );
+    Ok(())
+}
 
 #[cfg(test)]
 mod tests {
-    use super::super::validate_headers;
+    use super::validate_headers;
     use crate::error::ValidationError;
     use crate::types::ValidationMode;
     use dashcore::{
@@ -12,6 +72,23 @@ mod tests {
     };
     use dashcore_hashes::Hash;
 
+    /// Create a test header with given parameters
+    fn create_test_header(
+        prev_hash: dashcore::BlockHash,
+        nonce: u32,
+        bits: u32,
+        time: u32,
+    ) -> BlockHeader {
+        BlockHeader {
+            version: Version::from_consensus(0x20000000),
+            prev_blockhash: prev_hash,
+            merkle_root: dashcore::TxMerkleNode::from_byte_array([0; 32]),
+            time,
+            bits: dashcore::CompactTarget::from_consensus(bits),
+            nonce,
+        }
+    }
+
     /// Create a test header with specific parameters
     fn create_test_header_with_params(
         version: u32,
@@ -31,6 +108,185 @@ mod tests {
         }
     }
 
+    // ==================== Basic Tests ====================
+
+    #[test]
+    fn test_validation_mode_none_always_passes() {
+        let header = create_test_header(
+            dashcore::BlockHash::from_raw_hash(dashcore_hashes::hash_x11::Hash::from_byte_array(
+                [0; 32],
+            )),
+            0,
+            0x1e0fffff,
+            1234567890,
+        );
+
+        // Should pass with no previous header
+        assert!(validate_headers(&[header], ValidationMode::None).is_ok());
+
+        // Should pass even with invalid chain continuity
+        let prev_header = create_test_header(
+            dashcore::BlockHash::from_raw_hash(dashcore_hashes::hash_x11::Hash::from_byte_array(
+                [1; 32],
+            )),
+            1,
+            0x1e0fffff,
+            1234567890,
+        );
+        assert!(validate_headers(&[prev_header, header], ValidationMode::None).is_ok());
+    }
+
+    #[test]
+    fn test_basic_validation_chain_continuity() {
+        // Create two headers that connect properly
+        let header1 = create_test_header(
+            dashcore::BlockHash::from_raw_hash(dashcore_hashes::hash_x11::Hash::from_byte_array(
+                [0; 32],
+            )),
+            1,
+            0x1e0fffff,
+            1234567890,
+        );
+        let header2 = create_test_header(header1.block_hash(), 2, 0x1e0fffff, 1234567900);
+
+        // Should pass when headers connect
+        assert!(validate_headers(&[header1, header2], ValidationMode::Basic).is_ok());
+
+        // Should fail when headers don't connect
+        let disconnected_header = create_test_header(
+            dashcore::BlockHash::from_raw_hash(dashcore_hashes::hash_x11::Hash::from_byte_array(
+                [99; 32],
+            )),
+            3,
+            0x1e0fffff,
+            1234567910,
+        );
+        let result = validate_headers(&[header1, disconnected_header], ValidationMode::Basic);
+        assert!(matches!(result, Err(ValidationError::InvalidHeaderChain(_))));
+    }
+
+    #[test]
+    fn test_basic_validation_no_pow_check() {
+        // Create header with invalid PoW (would fail full validation)
+        let header = create_test_header(
+            dashcore::BlockHash::from_raw_hash(dashcore_hashes::hash_x11::Hash::from_byte_array(
+                [0; 32],
+            )),
+            0, // Invalid nonce that won't produce valid PoW
+            0x1e0fffff,
+            1234567890,
+        );
+
+        // Should pass basic validation (no PoW check)
+        assert!(validate_headers(&[header], ValidationMode::Basic).is_ok());
+    }
+
+    #[test]
+    fn test_full_validation_includes_pow() {
+        // Create header with invalid PoW
+        let header = create_test_header(
+            dashcore::BlockHash::from_raw_hash(dashcore_hashes::hash_x11::Hash::from_byte_array(
+                [0; 32],
+            )),
+            0,          // Invalid nonce
+            0x1d00ffff, // Difficulty that requires real PoW
+            1234567890,
+        );
+
+        // Should fail full validation due to invalid PoW
+        let result = validate_headers(&[header], ValidationMode::Full);
+        assert!(matches!(result, Err(ValidationError::InvalidProofOfWork)));
+    }
+
+    #[test]
+    fn test_validate_headers_empty() {
+        for mode in [ValidationMode::None, ValidationMode::Basic, ValidationMode::Full] {
+            let headers: Vec<BlockHeader> = vec![];
+            // Empty chain should pass
+            assert!(validate_headers(&headers, mode).is_ok());
+        }
+    }
+
+    #[test]
+    fn test_validate_headers_basic_single_header() {
+        let header = create_test_header(
+            dashcore::BlockHash::from_raw_hash(dashcore_hashes::hash_x11::Hash::from_byte_array(
+                [0; 32],
+            )),
+            1,
+            0x1e0fffff,
+            1234567890,
+        );
+
+        // Single header should pass (no chain validation needed)
+        assert!(validate_headers(&[header], ValidationMode::Basic).is_ok());
+    }
+
+    #[test]
+    fn test_validate_headers_basic_valid_chain() {
+        // Create a valid chain of headers
+        let mut headers = vec![];
+        let mut prev_hash = dashcore::BlockHash::from_raw_hash(
+            dashcore_hashes::hash_x11::Hash::from_byte_array([0; 32]),
+        );
+
+        for i in 0..5 {
+            let header = create_test_header(prev_hash, i, 0x1e0fffff, 1234567890 + i * 600);
+            prev_hash = header.block_hash();
+            headers.push(header);
+        }
+
+        // Valid chain should pass
+        assert!(validate_headers(&headers, ValidationMode::Basic).is_ok());
+    }
+
+    #[test]
+    fn test_validate_headers_basic_broken_chain() {
+        // Create a chain with a break in the middle
+        let header1 = create_test_header(
+            dashcore::BlockHash::from_raw_hash(dashcore_hashes::hash_x11::Hash::from_byte_array(
+                [0; 32],
+            )),
+            1,
+            0x1e0fffff,
+            1234567890,
+        );
+        let header2 = create_test_header(header1.block_hash(), 2, 0x1e0fffff, 1234567900);
+        let header3 = create_test_header(
+            dashcore::BlockHash::from_raw_hash(dashcore_hashes::hash_x11::Hash::from_byte_array(
+                [99; 32],
+            )), // Broken link
+            3,
+            0x1e0fffff,
+            1234567910,
+        );
+
+        let headers = vec![header1, header2, header3];
+
+        // Should fail due to broken chain
+        let result = validate_headers(&headers, ValidationMode::Basic);
+        assert!(matches!(result, Err(ValidationError::InvalidHeaderChain(_))));
+    }
+
+    #[test]
+    fn test_validate_headers_full_with_pow() {
+        // Create headers with invalid PoW
+        let header1 = create_test_header(
+            dashcore::BlockHash::from_raw_hash(dashcore_hashes::hash_x11::Hash::from_byte_array(
+                [0; 32],
+            )),
+            0,          // Invalid nonce
+            0x1d00ffff, // Difficulty that requires real PoW
+            1234567890,
+        );
+
+        // Should fail when PoW validation is enabled
+        let result = validate_headers(&[header1], ValidationMode::Full);
+        assert!(matches!(result, Err(ValidationError::InvalidProofOfWork)));
+    }
+
+    // ==================== Edge Case Tests ====================
+
     #[test]
     fn test_genesis_block_validation() {
         for network in [Network::Dash, Network::Testnet, Network::Regtest] {

dash-spv/src/validation/headers.rs

@@ -1,64 +0,0 @@
-//! Header validation functionality.
-
-use dashcore::{block::Header as BlockHeader, error::Error as DashError};
-use std::time::Instant;
-
-use crate::error::{ValidationError, ValidationResult};
-use crate::types::ValidationMode;
-
-/// 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(());
-    }
-
-    let start = Instant::now();
-
-    let mut prev_block_hash = None;
-    for header in headers {
-        // Check chain continuity if we have previous header
-        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(),
-                ));
-            }
-        }
-
-        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
-                    ))),
-                };
-            }
-        }
-
-        prev_block_hash = Some(header.block_hash());
-    }
-
-    tracing::debug!(
-        "Header chain validation passed for {} headers in mode: {:?}, duration: {:?}",
-        headers.len(),
-        mode,
-        start.elapsed(),
-    );
-    Ok(())
-}
-
-#[cfg(test)]
-#[path = "headers_test.rs"]
-mod headers_test;
-
-#[cfg(test)]
-#[path = "headers_edge_test.rs"]
-mod headers_edge_test;