chore: Move header validation into sync::headers::validation

[xdustinface]

Merge and move the 3 files related to header validation (actual validation function in headers.rs and the two test files) into sync/headers/validation.rs

Based on:

• refactor: Cleanup SPV validation #237

Summary by CodeRabbit

• Refactor
  • Reorganized internal code structure to improve maintainability and module organization.
  • Restructured validation logic with updated module dependencies.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This pull request reorganizes header validation code by relocating the validate_headers function and its tests from the validation module to a new sync::headers::validation submodule. Import paths are updated in the sync coordinator to reference the new location.

Changes

Cohort / File(s)	Summary
Validation module cleanup dash-spv/src/validation/headers.rs, dash-spv/src/validation/headers_test.rs, dash-spv/src/validation/mod.rs	Removed validate_headers function, associated logic, and test module; eliminated public module and re-export declarations
Sync headers module enhancement dash-spv/src/sync/headers/validation.rs (new), dash-spv/src/sync/headers/mod.rs	Created new validation.rs submodule with validate_headers function supporting None/Basic/Full modes, chain continuity checks, PoW validation, and comprehensive test coverage; exposed via public re-export
Import path update dash-spv/src/client/sync_coordinator.rs	Updated import to resolve validate_headers from crate::sync::headers::validate_headers

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Verify all references to the old crate::validation::headers::validate_headers path have been updated
• Confirm function signature and logic remain identical across the relocation
• Check that test coverage is preserved and equivalent to the original test module

Poem

🐰 Headers hop to sync, validation finds its way,
From validation deep to headers bright new day,
Import paths dance and twirl, refactoring's grand flight,
The rabbit reorganizes—all modules feel so right! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: relocating header validation functionality from the validation module into sync::headers::validation.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chore/move-header-validation

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

dash-spv/src/sync/headers/validation.rs (2)

1-61: validate_headers correctly models ValidationMode semantics; consider a small clarity refactor.

The function cleanly implements the intended behavior:

• ValidationMode::None: fast early-return, skipping all checks.
• Non-empty chains: always validate intra-batch continuity via prev_blockhash.
• ValidationMode::Full: adds PoW verification on each header and maps failures into ValidationError variants.
• Empty headers short-circuits with Ok(()), which is reasonable and explicitly tested.

If you want to make the per-mode behavior even more explicit (and avoid repeated mode == ... checks), you could rewrite the top-level flow as a match mode (None, Basic, Full) and keep the continuity/Pow logic in distinct branches; behavior would stay the same but be slightly clearer. Based on learnings, this keeps ValidationMode-driven header validation centralized and easy to reason about.

---

65-90: Tests give solid coverage of None/Basic/Full modes and core header-chain behavior.

The test module exercises:

• ValidationMode::None skipping both continuity and PoW checks.
• ValidationMode::Basic enforcing chain continuity but not PoW.
• ValidationMode::Full surfacing PoW failures as InvalidProofOfWork.
• Empty chains, single-header chains, valid and broken multi-header chains, and larger chains.

This is a strong suite around the core contract of validate_headers. If you ever want to trim duplication, the simple create_test_header helper could internally call create_test_header_with_params so there’s only one place constructing BlockHeaders, but that’s purely cosmetic and tests are perfectly fine as-is. Based on learnings, the tests accurately reflect the intended ValidationMode behavior for headers.

Also applies to: 111-287

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d702ed and 9472176.

📒 Files selected for processing (6)

• dash-spv/src/client/sync_coordinator.rs (1 hunks)
• dash-spv/src/sync/headers/mod.rs (1 hunks)
• dash-spv/src/sync/headers/validation.rs (3 hunks)
• dash-spv/src/validation/headers.rs (0 hunks)
• dash-spv/src/validation/headers_test.rs (0 hunks)
• dash-spv/src/validation/mod.rs (0 hunks)

💤 Files with no reviewable changes (3)

• dash-spv/src/validation/mod.rs
• dash-spv/src/validation/headers_test.rs
• dash-spv/src/validation/headers.rs

🧰 Additional context used

📓 Path-based instructions (4)

**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types with thiserror crate and propagate errors appropriately in Rust code
Use tokio runtime for async operations in Rust code
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format Rust code with cargo fmt before committing
Ensure clippy passes with all warnings as errors on all Rust code

**/*.rs: Each crate keeps sources in src/; unit tests live alongside code with #[cfg(test)]
Maintain MSRV of 1.89 for Rust code
Format Rust code with rustfmt (see rustfmt.toml); run cargo fmt --all before commits
Lint Rust code with clippy; avoid unwrap()/expect() in library code; use error types (e.g., thiserror)
Use snake_case for function and variable names in Rust
Use UpperCamelCase for types and traits in Rust
Use SCREAMING_SNAKE_CASE for constants in Rust
Follow mixed editions (2021/2024) and crate-specific idioms; prefer async via tokio where applicable
Unit tests should be placed near code with descriptive names (e.g., test_parse_address_mainnet)
Use #[ignore] for network-dependent or long-running tests; run with -- --ignored flag
Never commit secrets or real keys; avoid logging sensitive data in Rust code
Keep test vectors deterministic in Rust test code

Files:

• dash-spv/src/sync/headers/mod.rs
• dash-spv/src/client/sync_coordinator.rs
• dash-spv/src/sync/headers/validation.rs

dash-spv/**/*.rs

📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)

dash-spv/**/*.rs: Use trait-based abstractions for core components like NetworkManager and StorageManager to enable swappable implementations
Use async/await throughout the codebase with tokio runtime for all asynchronous operations
Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles with cargo check --all-features

Files:

• dash-spv/src/sync/headers/mod.rs
• dash-spv/src/client/sync_coordinator.rs
• dash-spv/src/sync/headers/validation.rs

dash-spv/src/sync/**/*.rs

📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)

Synchronization should use SyncManager for phase-based coordination: Phase 1 (Headers), Phase 2 (Masternode List), Phase 3 (Filter Headers), Phase 4 (Filters), Phase 5 (Blocks)

Files:

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

dash-spv/src/client/**/*.rs

📄 CodeRabbit inference engine (dash-spv/CLAUDE.md)

Client module should provide high-level API through DashSpvClient and configuration via ClientConfig

Files:

• dash-spv/src/client/sync_coordinator.rs

🧠 Learnings (16)

📓 Common learnings

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/validation/**/*.rs : Validation module should support configurable validation modes (`ValidationMode::None`, `ValidationMode::Basic`, `ValidationMode::Full`) with header validation, ChainLock, and InstantLock verification

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should be organized in `tests/` directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration

📚 Learning: 2025-12-04T15:51:59.632Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/validation/**/*.rs : Validation module should support configurable validation modes (`ValidationMode::None`, `ValidationMode::Basic`, `ValidationMode::Full`) with header validation, ChainLock, and InstantLock verification

Applied to files:

• dash-spv/src/sync/headers/mod.rs
• dash-spv/src/client/sync_coordinator.rs
• dash-spv/src/sync/headers/validation.rs

📚 Learning: 2025-12-04T15:51:59.632Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Synchronization should use `SyncManager` for phase-based coordination: Phase 1 (Headers), Phase 2 (Masternode List), Phase 3 (Filter Headers), Phase 4 (Filters), Phase 5 (Blocks)

Applied to files:

• dash-spv/src/sync/headers/mod.rs
• dash-spv/src/client/sync_coordinator.rs
• dash-spv/src/sync/headers/validation.rs

📚 Learning: 2025-12-01T08:00:50.618Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: Run `./sync-headers.sh` to copy updated headers to Swift SDK after rebuilding dash-spv-ffi

Applied to files:

• dash-spv/src/sync/headers/mod.rs
• dash-spv/src/client/sync_coordinator.rs

📚 Learning: 2025-12-04T15:51:59.632Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/client/**/*.rs : Client module should provide high-level API through `DashSpvClient` and configuration via `ClientConfig`

Applied to files:

• dash-spv/src/sync/headers/mod.rs
• dash-spv/src/client/sync_coordinator.rs

📚 Learning: 2025-12-01T08:00:50.618Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: Applies to swift-dash-core-sdk/**/*.rs : Implement new FFI functions in Rust with `#[no_mangle] extern "C"` annotation in dash-spv-ffi

Applied to files:

• dash-spv/src/sync/headers/mod.rs
• dash-spv/src/client/sync_coordinator.rs

📚 Learning: 2025-12-04T15:51:59.632Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/tests/**/*.rs : Integration tests should be organized in `tests/` directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration

Applied to files:

• dash-spv/src/sync/headers/mod.rs
• dash-spv/src/client/sync_coordinator.rs
• dash-spv/src/sync/headers/validation.rs

📚 Learning: 2025-12-01T08:01:18.174Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)

Applied to files:

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

📚 Learning: 2025-12-04T15:51:59.632Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/src/error.rs : Maintain domain-specific error types in `error.rs` with variants for `NetworkError`, `StorageError`, `SyncError`, `ValidationError`, and `SpvError` (top-level wrapper)

Applied to files:

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

📚 Learning: 2025-12-04T15:51:59.632Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use Minimum Rust Version (MSRV) 1.89 and ensure all code compiles with `cargo check --all-features`

Applied to files:

• dash-spv/src/sync/headers/mod.rs
• dash-spv/src/client/sync_coordinator.rs

📚 Learning: 2025-12-01T07:59:58.608Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/tests/unit/**/*.rs : Add corresponding unit tests in `tests/unit/` for each new FFI function

Applied to files:

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

📚 Learning: 2025-12-04T15:51:59.632Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use async/await throughout the codebase with tokio runtime for all asynchronous operations

Applied to files:

• dash-spv/src/client/sync_coordinator.rs

📚 Learning: 2025-12-04T15:51:59.632Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use trait-based abstractions for core components like `NetworkManager` and `StorageManager` to enable swappable implementations

Applied to files:

• dash-spv/src/client/sync_coordinator.rs

📚 Learning: 2025-12-01T08:00:50.618Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: When making changes to Rust FFI in dash-spv-ffi, rebuild with `cargo build --release` and run Swift tests to verify integration

Applied to files:

• dash-spv/src/client/sync_coordinator.rs

📚 Learning: 2025-06-26T16:02:42.390Z

Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:02:42.390Z
Learning: Passing exclusive mutable references to network and storage managers into the sync manager in async Rust can lead to borrow checker issues or runtime contention if concurrent access is needed elsewhere. It's advisable to document this architectural tradeoff and consider refactoring to interior mutability or message passing for shared access as the codebase evolves.

Applied to files:

• dash-spv/src/client/sync_coordinator.rs

📚 Learning: 2025-12-01T08:00:37.919Z

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{wallet,account,managed_account,address_pool}/**/*.rs : Always validate network consistency: use the account's network type when creating addresses and assert that operations use the expected network to prevent mainnet/testnet mixing

Applied to files:

• dash-spv/src/client/sync_coordinator.rs

🧬 Code graph analysis (1)

dash-spv/src/client/sync_coordinator.rs (1)

dash-spv/src/sync/headers/validation.rs (1)

• validate_headers (10-61)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)

• GitHub Check: fuzz (hashes_sha1)
• GitHub Check: fuzz (hashes_cbor)
• GitHub Check: fuzz (hashes_ripemd160)
• GitHub Check: fuzz (hashes_sha512_256)
• GitHub Check: fuzz (dash_deserialize_script)
• GitHub Check: fuzz (hashes_sha512)
• GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
• GitHub Check: fuzz (dash_deser_net_msg)
• GitHub Check: fuzz (hashes_sha256)
• GitHub Check: fuzz (hashes_json)
• GitHub Check: fuzz (dash_deserialize_witness)
• GitHub Check: fuzz (dash_deserialize_address)
• GitHub Check: fuzz (dash_deserialize_amount)
• GitHub Check: fuzz (dash_outpoint_string)
• GitHub Check: SPV Components Tests
• GitHub Check: RPC Tests (stable, true)
• GitHub Check: Core Components Tests
• GitHub Check: Pre-commit (ubuntu-latest)
• GitHub Check: Pre-commit (macos-latest)
• GitHub Check: Key Wallet Components Tests

🔇 Additional comments (2)

dash-spv/src/sync/headers/mod.rs (1)

3-7: Good encapsulation of header validation under sync::headers.

Exposing pub mod validation and re-exporting validate_headers keeps the public API simple (crate::sync::headers::validate_headers) while colocating validation with header sync concerns. This aligns well with the ValidationMode-based design mentioned in the learnings.

dash-spv/src/client/sync_coordinator.rs (1)

20-21: Import path update to new validation module looks correct.

Switching to crate::sync::headers::validate_headers and continuing to use ValidationMode::Basic when restoring headers from storage preserves behavior while matching the new module organization. Using Basic here is appropriate since it checks chain continuity without redoing PoW on already-validated stored headers.

Also applies to: 948-961