refactor: Rename HeaderSyncManagerWithReorg -> HeaderSyncManager

[xdustinface]

Just renaming the "with reorg" away in all places after dropping the old implementation of HeaderSyncManager in #208.

Based on:

• chore: Drop unused sync code #208

Summary by CodeRabbit

• Refactor
  • Simplified the header synchronization API by consolidating module structure and updating public type naming for improved consistency and clarity.
  • Updated documentation to reflect streamlined header synchronization functionality.

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

[coderabbitai]

Walkthrough

The changes consolidate the header synchronization API by renaming HeaderSyncManagerWithReorg<S, N> to HeaderSyncManager<S, N>, removing reorg-specific naming. Module exports are reorganized to promote the simplified headers module while deprecating headers_with_reorg, and all dependent code is updated to use the new type name.

Changes

Cohort / File(s)	Summary
Core struct rename dash-spv/src/sync/headers.rs	Renamed public struct from HeaderSyncManagerWithReorg<S, N> to HeaderSyncManager<S, N> and updated impl block signature. Updated constructor documentation and removed reorg-specific wording from method docs and log messages.
Module consolidation dash-spv/src/sync/mod.rs	Removed pub mod headers_with_reorg and replaced it with pub mod headers. Updated public re-exports from headers_with_reorg::{HeaderSyncManagerWithReorg, ReorgConfig} to headers::{HeaderSyncManager, ReorgConfig}.
Usage site updates dash-spv/src/sync/sequential/lifecycle.rs, dash-spv/src/sync/sequential/manager.rs	Updated type references from HeaderSyncManagerWithReorg<S, N> to HeaderSyncManager<S, N> in struct field declarations, imports, and constructor calls.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Straightforward rename refactor with consistent pattern across all files
• Low behavioral complexity; primarily mechanical updates to type names and imports
• Module consolidation is straightforward with clear intent
• Single naming convention change with no logic modifications

Poem

🐰 A type once named with reorg's weight,
Now HeaderSyncManager, clean and straight!
Modules aligned, no fuss or fray,
Simpler names lead the way! ✨

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 clearly and concisely summarizes the main change: renaming HeaderSyncManagerWithReorg to HeaderSyncManager across the codebase, which aligns with all file changes shown in the summary.
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 refactor/rename-header-sync-manager

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c59518d and 4534d4b.

📒 Files selected for processing (4)

• dash-spv/src/sync/headers.rs (7 hunks)
• dash-spv/src/sync/mod.rs (1 hunks)
• dash-spv/src/sync/sequential/lifecycle.rs (2 hunks)
• dash-spv/src/sync/sequential/manager.rs (3 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.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/sequential/lifecycle.rs
• dash-spv/src/sync/mod.rs
• dash-spv/src/sync/sequential/manager.rs
• dash-spv/src/sync/headers.rs

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

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

dash-spv/src/**/*.rs: Use trait-based abstractions for swappable implementations (e.g., NetworkManager, StorageManager)
Use async/await throughout the codebase with tokio runtime
Use domain-specific error types (NetworkError, StorageError, SyncError, ValidationError, SpvError) for error handling
Use trait objects (Arc<dyn StorageManager>, Arc<dyn NetworkManager>) for runtime polymorphism
Use Tokio channels for inter-component message passing
Add comprehensive unit tests in-module for individual components

Files:

• dash-spv/src/sync/sequential/lifecycle.rs
• dash-spv/src/sync/mod.rs
• dash-spv/src/sync/sequential/manager.rs
• dash-spv/src/sync/headers.rs

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

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

dash-spv/src/sync/**/*.rs: Use sequential phase-based synchronization with SequentialSyncManager for organized sync phases
Use state machines with SyncState enum to drive synchronization flow and state transitions

Files:

• dash-spv/src/sync/sequential/lifecycle.rs
• dash-spv/src/sync/mod.rs
• dash-spv/src/sync/sequential/manager.rs
• dash-spv/src/sync/headers.rs

🧠 Learnings (18)

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.310Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Use sequential phase-based synchronization with `SequentialSyncManager` for organized sync phases

Applied to files:

• dash-spv/src/sync/sequential/lifecycle.rs
• dash-spv/src/sync/mod.rs
• dash-spv/src/sync/sequential/manager.rs
• dash-spv/src/sync/headers.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.311Z
Learning: Applies to dash-spv/src/sync/**/*.rs : Use state machines with `SyncState` enum to drive synchronization flow and state transitions

Applied to files:

• dash-spv/src/sync/sequential/lifecycle.rs
• dash-spv/src/sync/mod.rs
• dash-spv/src/sync/sequential/manager.rs
• dash-spv/src/sync/headers.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.310Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Implement `StorageManager` trait with segmented storage: headers in 10,000-header segments with index files, filters in separate storage, and state persistence in dedicated paths

Applied to files:

• dash-spv/src/sync/sequential/lifecycle.rs
• dash-spv/src/sync/mod.rs
• dash-spv/src/sync/sequential/manager.rs
• dash-spv/src/sync/headers.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.310Z
Learning: Applies to dash-spv/src/**/*.rs : Use trait-based abstractions for swappable implementations (e.g., `NetworkManager`, `StorageManager`)

Applied to files:

• dash-spv/src/sync/sequential/lifecycle.rs
• dash-spv/src/sync/mod.rs
• dash-spv/src/sync/sequential/manager.rs
• dash-spv/src/sync/headers.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.310Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Use segmented storage with headers in 10,000-header chunks stored with index files for efficient retrieval

Applied to files:

• dash-spv/src/sync/sequential/lifecycle.rs
• dash-spv/src/sync/mod.rs
• dash-spv/src/sync/sequential/manager.rs
• dash-spv/src/sync/headers.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.310Z
Learning: Applies to dash-spv/src/**/*.rs : Use async/await throughout the codebase with tokio runtime

Applied to files:

• dash-spv/src/sync/sequential/lifecycle.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.310Z
Learning: Applies to dash-spv/src/**/*.rs : Use trait objects (`Arc<dyn StorageManager>`, `Arc<dyn NetworkManager>`) for runtime polymorphism

Applied to files:

• dash-spv/src/sync/sequential/lifecycle.rs
• dash-spv/src/sync/sequential/manager.rs
• dash-spv/src/sync/headers.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.310Z
Learning: Applies to dash-spv/src/types.rs : Define common data structures in `types.rs` including `SyncProgress`, `ValidationMode`, `WatchItem`, etc.

Applied to files:

• dash-spv/src/sync/sequential/lifecycle.rs
• dash-spv/src/sync/mod.rs
• dash-spv/src/sync/sequential/manager.rs
• dash-spv/src/sync/headers.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.311Z
Learning: Follow a layered architecture with clear separation of concerns: `client/`, `network/`, `storage/`, `sync/`, `validation/`, `wallet/`, `types.rs`, and `error.rs`

Applied to files:

• dash-spv/src/sync/sequential/lifecycle.rs
• dash-spv/src/sync/mod.rs
• dash-spv/src/sync/sequential/manager.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/sync/sequential/lifecycle.rs
• dash-spv/src/sync/sequential/manager.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.310Z
Learning: Applies to dash-spv/src/**/*.rs : Use domain-specific error types (`NetworkError`, `StorageError`, `SyncError`, `ValidationError`, `SpvError`) for error handling

Applied to files:

• dash-spv/src/sync/sequential/lifecycle.rs
• dash-spv/src/sync/headers.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.906Z
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/sync/sequential/lifecycle.rs

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

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

Applied to files:

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

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.440Z
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/mod.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.599Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Use `#[no_mangle] extern "C"` attribute when implementing new FFI functions in Rust

Applied to files:

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

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.310Z
Learning: Applies to dash-spv/src/**/*.rs : Add comprehensive unit tests in-module for individual components

Applied to files:

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

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.160Z
Learning: Keep Rust modules focused and organized

Applied to files:

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

📚 Learning: 2025-06-26T15:54:02.509Z

Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T15:54:02.509Z
Learning: The `StorageManager` trait in `dash-spv/src/storage/mod.rs` uses `&mut self` methods but is also `Send + Sync`, and implementations often use interior mutability for concurrency. This can be confusing, so explicit documentation should clarify thread-safety expectations and the rationale for the API design.

Applied to files:

• dash-spv/src/sync/sequential/manager.rs
• dash-spv/src/sync/headers.rs

🧬 Code graph analysis (1)

dash-spv/src/sync/sequential/lifecycle.rs (1)

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

• new (71-105)

🔇 Additional comments (10)

dash-spv/src/sync/sequential/manager.rs (3)

9-9: Import updated to renamed HeaderSyncManager

The SequentialSyncManager now correctly imports HeaderSyncManager from crate::sync, matching the new module re-exports and headers.rs definition.

---

50-52: Delegation docs accurately reference HeaderSyncManager

The delegation description now names HeaderSyncManager alongside FilterSyncManager and MasternodeSyncManager, which matches the actual field types and generics.

---

71-73: header_sync field type aligned with new manager struct

The header_sync field is now HeaderSyncManager<S, N>; existing usages (get_chain_height, update_chain_state_cache, checkpoint helpers) remain valid against the same interface.

dash-spv/src/sync/sequential/lifecycle.rs (2)

11-11: Lifecycle imports now target unified HeaderSyncManager API

Importing HeaderSyncManager and ReorgConfig from crate::sync is consistent with the new sync::headers module and its public re-exports.

---

41-43: HeaderSyncManager construction matches new constructor signature

SequentialSyncManager::new calls HeaderSyncManager::new(config, reorg_config, chain_state) and maps errors into SyncError::InvalidState, which matches the headers.rs constructor and preserves existing initialization semantics.

dash-spv/src/sync/headers.rs (4)

1-75: HeaderSyncManager rename and docs stay aligned with functionality

Renaming HeaderSyncManagerWithReorg to HeaderSyncManager (including the module and struct docs) keeps the type name concise while still accurately describing fork detection and reorg handling via ReorgConfig, ForkDetector, and ChainTipManager; constructor parameters and behavior remain unchanged.

---

230-399: handle_headers_message wording/logging updates are non‑behavioral

The refreshed doc comment and info log around handling headers batches remove the old “with reorg” naming but leave batch validation, checkpoint checks, storage writes, and tip manager updates untouched, so runtime behavior is preserved.

---

652-810: prepare_sync logging clarified without altering checkpoint logic

The additional/updated tracing::info! messages in prepare_sync give clearer insight into checkpoint vs. genesis flows and effective tip height calculation, while the underlying logic for choosing base_hash and initializing sync state is unchanged.

---

813-823: start_sync continues to delegate through prepare_sync and request_headers

The start_sync method now logs “Starting header synchronization” but still calls prepare_sync followed by request_headers with the computed base hash, so its control flow and error behavior are consistent with previous semantics.

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

8-14: Public sync exports correctly switched to headers::HeaderSyncManager

The transition from headers_with_reorg to the unified headers module is complete with no stray references to the old module or type names. The re-exports of HeaderSyncManager and ReorgConfig cleanly replace the prior path.

---

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.}