Skip to content

refactor: Cleanup HeaderValidator and ValidationManager #234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
xdustinface wants to merge 1 commit into from
Closed

refactor: Cleanup HeaderValidator and ValidationManager #234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dash-spv/src/client/lifecycle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion dash-spv/src/client/sync_coordinator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
90 changes: 18 additions & 72 deletions dash-spv/src/validation/headers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand All @@ -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,
Expand Down Expand Up @@ -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(());
}
Expand All @@ -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(())
}

Expand All @@ -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)]
Expand Down
52 changes: 24 additions & 28 deletions dash-spv/src/validation/headers_edge_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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];
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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(
Expand All @@ -277,14 +274,15 @@ 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());
// Single header chain should validate
assert!(HeaderValidator::new(ValidationMode::Basic, 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,
Expand All @@ -301,13 +299,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 = [
Expand Down Expand Up @@ -340,7 +338,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![];
Expand All @@ -361,18 +359,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);
}
}
Loading