chore: Some restructuring in `dash-spv::sync` #236

xdustinface · 2025-12-04T01:02:39Z

This is some restructuring in the sync folder.

Based on:

refactor: Rename SequentialSyncManager to SyncManager #235

Structure before:

  sync/
  ├── embedded_data.rs
  ├── filters/
  │   └── (10 files)
  ├── headers.rs
  ├── headers2_state.rs
  ├── masternodes.rs
  ├── mod.rs
  └── sequential/
      ├── lifecycle.rs
      ├── manager.rs
      ├── message_handlers.rs
      ├── mod.rs
      ├── phase_execution.rs
      ├── phases.rs
      ├── post_sync.rs
      └── transitions.rs

Structure with this PR:

 sync/
  ├── filters/
  │   └── (10 files)
  ├── headers/
  │   ├── manager.rs
  │   └── mod.rs
  ├── headers2/
  │   ├── manager.rs
  │   └── mod.rs
  ├── masternodes/
  │   ├── embedded_data.rs
  │   ├── manager.rs
  │   └── mod.rs
  ├── manager.rs (with lifecycle.rs merged in)
  ├── message_handlers.rs
  ├── mod.rs
  ├── phase_execution.rs
  ├── phases.rs
  ├── post_sync.rs
  └── transitions.rs

Summary by CodeRabbit

Refactor
- Reorganized internal synchronization module structure, consolidating header and masternode synchronization managers under a cleaner architecture.
- Restructured sync pipeline components for improved maintainability and future extensibility.
- Updated import paths to reflect the new module hierarchy.

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

coderabbitai · 2025-12-04T01:02:45Z

Warning

Rate limit exceeded

@xdustinface has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 9 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 29cb8f5 and ed72f5b.

📒 Files selected for processing (16)

dash-spv/src/client/core.rs (1 hunks)
dash-spv/src/client/filter_sync.rs (1 hunks)
dash-spv/src/client/lifecycle.rs (1 hunks)
dash-spv/src/client/message_handler.rs (1 hunks)
dash-spv/src/client/message_handler_test.rs (1 hunks)
dash-spv/src/client/progress.rs (1 hunks)
dash-spv/src/sync/headers/manager.rs (2 hunks)
dash-spv/src/sync/headers/mod.rs (1 hunks)
dash-spv/src/sync/headers2/mod.rs (1 hunks)
dash-spv/src/sync/manager.rs (2 hunks)
dash-spv/src/sync/masternodes/embedded_data.rs (1 hunks)
dash-spv/src/sync/masternodes/mod.rs (1 hunks)
dash-spv/src/sync/mod.rs (1 hunks)
dash-spv/src/sync/sequential/lifecycle.rs (0 hunks)
dash-spv/src/sync/sequential/mod.rs (0 hunks)
dash-spv/tests/chainlock_validation_test.rs (1 hunks)

Walkthrough

Module reorganization: relocating SyncManager from sync::sequential to direct sync hierarchy; consolidating headers2_state into headers2; establishing new sync pipeline module structure; flattening public API re-exports.

Changes

Cohort / File(s)	Summary
Client module import updates `dash-spv/src/client/core.rs`, `dash-spv/src/client/filter_sync.rs`, `dash-spv/src/client/lifecycle.rs`, `dash-spv/src/client/message_handler.rs`, `dash-spv/src/client/message_handler_test.rs`	Updated SyncManager import paths from `crate::sync::sequential::SyncManager` to `crate::sync::SyncManager`
Client progress module `dash-spv/src/client/progress.rs`	Updated SyncPhase import path from `crate::sync::sequential::phases::SyncPhase` to `crate::sync::SyncPhase`
Sync module hierarchy `dash-spv/src/sync/mod.rs`	Removed `pub mod sequential` and `pub mod headers2_state`; added `pub mod headers2`, `pub mod manager`, `pub mod message_handlers`, `pub mod phase_execution`, `pub mod phases`, `pub mod post_sync`, `pub mod transitions`; updated re-exports for Headers2StateManager, Headers2Stats, ProcessError, PhaseTransition, SyncPhase, TransitionManager
Headers module `dash-spv/src/sync/headers/mod.rs`, `dash-spv/src/sync/headers/manager.rs`	Created new headers module with public re-exports of HeaderSyncManager and ReorgConfig; updated Headers2StateManager import path from `crate::sync::headers2_state` to `crate::sync::headers2`
Headers2 module `dash-spv/src/sync/headers2/mod.rs`	Created new headers2 module with public re-exports of Headers2StateManager, Headers2Stats, and ProcessError
Masternodes module `dash-spv/src/sync/masternodes/mod.rs`	Added public module `embedded_data` and re-export of MasternodeSyncManager
SyncManager implementation `dash-spv/src/sync/manager.rs`	Added public methods: `new()`, `load_headers_from_storage()`, `wallet_birth_height_hint()`, `config_start_height()`, `start_sync()`, `send_initial_requests()`, `reset_pending_requests()`, and `get_base_hash_from_storage()`
Sequential module removal `dash-spv/src/sync/sequential/mod.rs`, `dash-spv/src/sync/sequential/lifecycle.rs`	Removed entire sequential module including all submodule declarations and re-exports
Embedded data path update `dash-spv/src/sync/masternodes/embedded_data.rs`	Updated include_bytes relative paths from `../../../dash/artifacts/` to `../../../../dash/artifacts/`
Test import update `dash-spv/tests/chainlock_validation_test.rs`	Updated SyncPhase import path from `dash_spv::sync::sequential::phases::SyncPhase` to `dash_spv::sync::SyncPhase`

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

SyncManager implementation (dash-spv/src/sync/manager.rs): Eight new public methods with non-trivial logic including header loading, wallet integration, network/storage interactions, and sync state transitions require careful review of error handling and initialization order
Module hierarchy restructuring (dash-spv/src/sync/mod.rs): Verify all re-exports are correctly wired and all previous entry points are still accessible through the new structure
Sequential module removal (dash-spv/src/sync/sequential/): Confirm all functionality has been properly moved to the new manager location and no code paths are orphaned
Import path consistency: Verify all import updates across client modules and tests are correct and no dangling references remain

Possibly related PRs

chore: Drop unused sync code #208: Both PRs modify the sync module's structure and public re-exports, including SyncManager import path changes and module organization updates.
refactor: Rename SequentialSyncManager to SyncManager #235: Related through renaming and restructuring SyncManager (SequentialSyncManager → SyncManager) with corresponding import updates across the codebase.

Poem

🐰 Modules hop to flatter ground,
Sequential paths no longer bound,
Headers sync and managers aligned,
New structure leaves old ways behind,
Organization—tidy and refined! ✨

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 reflects the main change: a filesystem-level restructuring of the sync module with no functional changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

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

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (3)

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

3-9: Message handler now uses top-level SyncManager import

Switching to crate::sync::SyncManager keeps the handler’s interactions with the sync layer intact and consistent with the rest of the client modules.

You may later want to drop “Sequential” from log messages now that the module path is just sync, but this is non‑blocking.

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

3-7: FilterSyncCoordinator now depends on crate::sync::SyncManager

Importing SyncManager from crate::sync keeps the coordinated filter sync logic intact and consistent with the refactored sync module.

Non‑blocking: some comments still say “sequential sync”; you might later refresh wording to reflect the generalized sync module.
dash-spv/src/sync/manager.rs (1)
250-261: Consider logging or propagating the error from reset_pending_requests.

The result of header_sync.reset_pending_requests() is silently discarded with let _ =. Per coding guidelines, library code should use proper error handling. Consider logging the error at minimum, or changing the method signature if errors during reset are expected and non-fatal.
     pub fn reset_pending_requests(&mut self) {
         // Reset all sync manager states
-        let _ = self.header_sync.reset_pending_requests();
+        if let Err(e) = self.header_sync.reset_pending_requests() {
+            tracing::warn!("Failed to reset header sync pending requests: {}", e);
+        }
         self.filter_sync.reset_pending_requests();
         // Masternode sync doesn't have pending requests to reset

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1035c62 and 29cb8f5.

📒 Files selected for processing (16)

dash-spv/src/client/core.rs (1 hunks)
dash-spv/src/client/filter_sync.rs (1 hunks)
dash-spv/src/client/lifecycle.rs (1 hunks)
dash-spv/src/client/message_handler.rs (1 hunks)
dash-spv/src/client/message_handler_test.rs (1 hunks)
dash-spv/src/client/progress.rs (1 hunks)
dash-spv/src/sync/headers/manager.rs (2 hunks)
dash-spv/src/sync/headers/mod.rs (1 hunks)
dash-spv/src/sync/headers2/mod.rs (1 hunks)
dash-spv/src/sync/manager.rs (2 hunks)
dash-spv/src/sync/masternodes/embedded_data.rs (1 hunks)
dash-spv/src/sync/masternodes/mod.rs (1 hunks)
dash-spv/src/sync/mod.rs (1 hunks)
dash-spv/src/sync/sequential/lifecycle.rs (0 hunks)
dash-spv/src/sync/sequential/mod.rs (0 hunks)
dash-spv/tests/chainlock_validation_test.rs (1 hunks)

💤 Files with no reviewable changes (2)

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

🧰 Additional context used

📓 Path-based instructions (6)

**/*.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/client/filter_sync.rs
dash-spv/src/sync/masternodes/embedded_data.rs
dash-spv/src/client/lifecycle.rs
dash-spv/src/client/message_handler_test.rs
dash-spv/src/client/progress.rs
dash-spv/src/sync/headers2/mod.rs
dash-spv/src/client/message_handler.rs
dash-spv/src/sync/headers/mod.rs
dash-spv/src/sync/headers/manager.rs
dash-spv/tests/chainlock_validation_test.rs
dash-spv/src/sync/masternodes/mod.rs
dash-spv/src/client/core.rs
dash-spv/src/sync/mod.rs
dash-spv/src/sync/manager.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/client/filter_sync.rs
dash-spv/src/sync/masternodes/embedded_data.rs
dash-spv/src/client/lifecycle.rs
dash-spv/src/client/message_handler_test.rs
dash-spv/src/client/progress.rs
dash-spv/src/sync/headers2/mod.rs
dash-spv/src/client/message_handler.rs
dash-spv/src/sync/headers/mod.rs
dash-spv/src/sync/headers/manager.rs
dash-spv/tests/chainlock_validation_test.rs
dash-spv/src/sync/masternodes/mod.rs
dash-spv/src/client/core.rs
dash-spv/src/sync/mod.rs
dash-spv/src/sync/manager.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/filter_sync.rs
dash-spv/src/client/lifecycle.rs
dash-spv/src/client/message_handler_test.rs
dash-spv/src/client/progress.rs
dash-spv/src/client/message_handler.rs
dash-spv/src/client/core.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/masternodes/embedded_data.rs
dash-spv/src/sync/headers2/mod.rs
dash-spv/src/sync/headers/mod.rs
dash-spv/src/sync/headers/manager.rs
dash-spv/src/sync/masternodes/mod.rs
dash-spv/src/sync/mod.rs
dash-spv/src/sync/manager.rs

**/tests/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/tests/**/*.rs: Write integration tests for network operations in Rust
Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate in Rust

Integration tests use tests/ directory (e.g., rpc-integration-test)

Files:

dash-spv/tests/chainlock_validation_test.rs

dash-spv/tests/**/*.rs

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

Integration tests should be organized in tests/ directory with comprehensive test suites for network layer, storage layer, header sync, and real network integration

Files:

dash-spv/tests/chainlock_validation_test.rs

🧠 Learnings (30)

📓 Common learnings

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.617Z
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)

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`)

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.617Z
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

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

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.617Z
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/client/filter_sync.rs
dash-spv/src/sync/masternodes/embedded_data.rs
dash-spv/src/client/lifecycle.rs
dash-spv/src/client/message_handler_test.rs
dash-spv/src/client/progress.rs
dash-spv/src/sync/headers2/mod.rs
dash-spv/src/client/message_handler.rs
dash-spv/src/sync/headers/mod.rs
dash-spv/src/sync/headers/manager.rs
dash-spv/tests/chainlock_validation_test.rs
dash-spv/src/sync/masternodes/mod.rs
dash-spv/src/client/core.rs
dash-spv/src/sync/mod.rs
dash-spv/src/sync/manager.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.617Z
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/filter_sync.rs
dash-spv/src/sync/masternodes/embedded_data.rs
dash-spv/src/client/lifecycle.rs
dash-spv/src/client/message_handler_test.rs
dash-spv/src/client/progress.rs
dash-spv/src/sync/headers2/mod.rs
dash-spv/src/client/message_handler.rs
dash-spv/src/sync/headers/mod.rs
dash-spv/src/sync/headers/manager.rs
dash-spv/src/sync/masternodes/mod.rs
dash-spv/src/client/core.rs
dash-spv/src/sync/mod.rs
dash-spv/src/sync/manager.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.617Z
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/filter_sync.rs
dash-spv/src/client/lifecycle.rs
dash-spv/src/client/message_handler_test.rs
dash-spv/src/client/progress.rs
dash-spv/src/client/message_handler.rs
dash-spv/src/sync/headers/manager.rs
dash-spv/tests/chainlock_validation_test.rs
dash-spv/src/client/core.rs
dash-spv/src/sync/mod.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.617Z
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/client/filter_sync.rs
dash-spv/src/client/lifecycle.rs
dash-spv/src/client/message_handler_test.rs
dash-spv/src/client/progress.rs
dash-spv/src/sync/headers2/mod.rs
dash-spv/src/client/message_handler.rs
dash-spv/src/sync/headers/mod.rs
dash-spv/src/client/core.rs
dash-spv/src/sync/mod.rs
dash-spv/src/sync/manager.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/client/filter_sync.rs
dash-spv/src/sync/masternodes/embedded_data.rs
dash-spv/src/client/lifecycle.rs
dash-spv/src/client/progress.rs
dash-spv/src/sync/headers2/mod.rs
dash-spv/src/client/message_handler.rs
dash-spv/src/sync/headers/mod.rs
dash-spv/src/sync/masternodes/mod.rs
dash-spv/src/client/core.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.617Z
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/client/filter_sync.rs
dash-spv/src/client/lifecycle.rs
dash-spv/src/client/progress.rs
dash-spv/src/client/message_handler.rs
dash-spv/src/sync/headers/manager.rs
dash-spv/tests/chainlock_validation_test.rs
dash-spv/src/sync/masternodes/mod.rs
dash-spv/src/client/core.rs
dash-spv/src/sync/manager.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.617Z
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/client/filter_sync.rs
dash-spv/src/sync/masternodes/embedded_data.rs
dash-spv/src/client/lifecycle.rs
dash-spv/src/client/progress.rs
dash-spv/src/client/message_handler.rs
dash-spv/tests/chainlock_validation_test.rs
dash-spv/src/client/core.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/client/filter_sync.rs
dash-spv/src/client/lifecycle.rs
dash-spv/src/client/message_handler_test.rs
dash-spv/src/client/message_handler.rs
dash-spv/src/sync/mod.rs
dash-spv/src/sync/manager.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.617Z
Learning: Applies to dash-spv/src/storage/**/*.rs : Storage module should provide abstraction via `StorageManager` trait with concrete implementations for `MemoryStorageManager` and `DiskStorageManager`

Applied to files:

dash-spv/src/client/filter_sync.rs
dash-spv/src/client/lifecycle.rs
dash-spv/src/client/message_handler_test.rs
dash-spv/src/client/progress.rs
dash-spv/src/sync/headers2/mod.rs
dash-spv/src/client/message_handler.rs
dash-spv/src/sync/headers/mod.rs
dash-spv/src/sync/headers/manager.rs
dash-spv/src/sync/masternodes/mod.rs
dash-spv/src/client/core.rs
dash-spv/src/sync/mod.rs
dash-spv/src/sync/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/client/filter_sync.rs
dash-spv/src/client/lifecycle.rs
dash-spv/src/client/message_handler_test.rs
dash-spv/src/client/progress.rs
dash-spv/src/sync/headers/manager.rs
dash-spv/src/client/core.rs
dash-spv/src/sync/manager.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.617Z
Learning: Applies to dash-spv/src/wallet/**/*.rs : Wallet module should provide UTXO tracking via `Utxo` struct, balance calculation with confirmation states, and transaction processing via `TransactionProcessor`

Applied to files:

dash-spv/src/client/filter_sync.rs
dash-spv/src/sync/headers2/mod.rs
dash-spv/src/sync/mod.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/filter_sync.rs
dash-spv/src/client/progress.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.617Z
Learning: Applies to dash-spv/src/network/**/*.rs : Network module should handle TCP connections, handshake management, message routing, and peer management following Dash protocol implementation

Applied to files:

dash-spv/src/client/filter_sync.rs
dash-spv/src/client/message_handler_test.rs
dash-spv/src/sync/headers2/mod.rs
dash-spv/src/client/message_handler.rs
dash-spv/src/sync/headers/mod.rs
dash-spv/src/sync/masternodes/mod.rs
dash-spv/src/client/core.rs
dash-spv/src/sync/mod.rs
dash-spv/src/sync/manager.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/**/*.rs : Never panic in library code; use `Result<T>` for all fallible operations with custom `Error` type variants and provide context in error messages

Applied to files:

dash-spv/src/client/filter_sync.rs
dash-spv/src/client/progress.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/src/**/*.rs : Add cbindgen annotations for complex types in FFI functions

Applied to files:

dash-spv/src/sync/masternodes/embedded_data.rs
dash-spv/src/client/progress.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: Keep test vectors updated with Dash Core, monitor for new DIPs affecting wallet structure, maintain backward compatibility for serialized wallets, and update derivation paths for Platform features

Applied to files:

dash-spv/src/sync/masternodes/embedded_data.rs
dash-spv/tests/chainlock_validation_test.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.617Z
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/masternodes/embedded_data.rs
dash-spv/src/client/message_handler_test.rs
dash-spv/src/client/progress.rs
dash-spv/src/sync/headers/mod.rs
dash-spv/tests/chainlock_validation_test.rs
dash-spv/src/sync/masternodes/mod.rs
dash-spv/src/client/core.rs
dash-spv/src/sync/mod.rs
dash-spv/src/sync/manager.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/**/*.swift : Use actors for state management in SPVClient to ensure thread safety

Applied to files:

dash-spv/src/client/lifecycle.rs
dash-spv/src/sync/mod.rs

📚 Learning: 2025-06-26T16:01:37.609Z

Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T16:01:37.609Z
Learning: The mempool tracking infrastructure (UnconfirmedTransaction, MempoolState, configuration, and mempool_filter.rs) is fully implemented and integrated in the Dash SPV client as of this PR, including client logic, FFI APIs, and tests.

Applied to files:

dash-spv/src/client/lifecycle.rs
dash-spv/src/client/message_handler.rs
dash-spv/src/sync/manager.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:01:00.652Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : Enable mempool tracking on SPV connection using the appropriate strategy (.fetchAll, .bloomFilter, .selective)

Applied to files:

dash-spv/src/client/lifecycle.rs
dash-spv/src/client/message_handler.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/message_handler_test.rs
dash-spv/src/client/progress.rs
dash-spv/tests/chainlock_validation_test.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/client/message_handler_test.rs
dash-spv/tests/chainlock_validation_test.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/**/address_pool/**/*.rs : Implement staged gap limit management using `GapLimitStage` structure with tracking of last_used_index and used_indices HashSet to enable efficient address discovery without loading entire address chains into memory

Applied to files:

dash-spv/src/client/progress.rs

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

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.617Z
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/headers2/mod.rs
dash-spv/src/sync/headers/mod.rs
dash-spv/tests/chainlock_validation_test.rs
dash-spv/src/sync/mod.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

📚 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
dash-spv/src/sync/masternodes/mod.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/**/tests/**/*.rs : Organize tests by functionality: BIP32 key derivation, mnemonic generation/validation, address generation/validation, path derivation, and PSBT serialization in separate test files

Applied to files:

dash-spv/tests/chainlock_validation_test.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/src/**/*.rs : Use `#[no_mangle] extern "C"` attribute when implementing new FFI functions in Rust

Applied to files:

dash-spv/src/sync/masternodes/mod.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: Keep Rust modules focused and organized

Applied to files:

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

🧬 Code graph analysis (1)

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

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

storage (346-346)

new (68-102)

load_headers_from_storage (105-225)

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

new (19-23)

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

name (121-143)

⏰ 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). (19)

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

🔇 Additional comments (17)

dash-spv/src/sync/masternodes/embedded_data.rs (1)

8-15: include_bytes! paths correctly adjusted for new module location

The extra ../ in both include paths matches moving this file one directory deeper under sync/masternodes/; no behavioral change beyond fixing the artifact location.

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

21-25: SyncManager import re-pointed to new sync module surface

Using crate::sync::SyncManager instead of the old sequential path keeps all existing usages intact and aligns with the new sync module layout.

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

8-13: SyncPhase import updated to top-level sync namespace

Switching to crate::sync::SyncPhase keeps map_phase_to_stage semantics unchanged while matching the new sync module structure (phase‑based coordination).

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

15-21: SyncManager import aligned with flattened sync module

Using crate::sync::SyncManager matches the new organization while preserving constructor usage and client lifecycle behavior.

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

1-18: Commented-out tests updated to new SyncManager path

The import in the disabled test module now points at crate::sync::SyncManager, matching the new layout. Since the block is commented out, this is purely preparatory and harmless.

dash-spv/tests/chainlock_validation_test.rs (1)

355-364: SyncPhase reference in ignored test updated to new public path

Even though this section is commented and the test is ignored, switching to dash_spv::sync::SyncPhase::FullySynced matches the refactored sync API and will be correct when these tests are eventually revived.

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

15-15: Import path correctly updated to match new module structure.

The import path change from crate::sync::headers2_state to crate::sync::headers2 is consistent with the restructuring where headers2_state.rs was moved into the headers2/ subdirectory with its contents now accessible via headers2::Headers2StateManager.

614-616: Pattern path correctly updated for module restructuring.

The ProcessError path update aligns with the new headers2 module structure.

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

1-6: Clean module structure following Rust best practices.

The module correctly encapsulates implementation details in the private manager module while exposing the public MasternodeSyncManager API. The embedded_data module is appropriately public to allow access to embedded masternode data.

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

1-5: Well-structured module with appropriate public API surface.

The module encapsulates implementation in the private manager module while re-exporting the three public types needed by consumers: Headers2StateManager for state management, Headers2Stats for metrics, and ProcessError for error handling.

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

1-5: Consistent module structure with appropriate exports.

The module follows the same clean pattern as the other restructured modules, keeping implementation private while exposing the public HeaderSyncManager and its configuration type ReorgConfig.

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

12-16: Excellent lock ordering documentation for deadlock prevention.

The documented lock acquisition order (state → storage → network) is crucial for preventing deadlocks in async code. This explicit documentation helps future contributors maintain thread-safety.

31-52: Clean module organization with comprehensive public API.

The new modular structure with separate modules for headers, headers2, masternodes, and the sync pipeline components (phases, transitions, etc.) provides good separation of concerns. The re-exports create a flat public API surface making it easier for consumers.

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

23-62: Excellent documentation on generic design rationale.

The detailed explanation of why generics are used (testing, performance, delegation, zero runtime cost) provides valuable context for contributors. This documentation clearly justifies the complexity while explaining the practical benefits.

108-138: Well-structured constructor with appropriate defaults.

The constructor properly initializes all sub-managers and provides sensible defaults for phase timeout (60s) and retry count (3). Error mapping to SyncError::InvalidState is appropriate for initialization failures.

160-176: Good defensive handling for unknown network variants.

The explicit handling of unknown network variants by returning None (line 168) is preferable to defaulting to a potentially incorrect network. The explicit drop(wallet_guard) on line 174 is good practice for clarity, though the guard would drop naturally at function end.

3-16: Updated imports align with new module structure.

The imports correctly use super::phases and super::transitions for sibling modules, and crate::sync for the re-exported sub-managers. The new dependencies (SharedFilterHeights, SpvStats, WalletInterface, etc.) are appropriately imported for the expanded constructor.

dash-spv/src/sync/mod.rs

ZocoLini

Love this new structure

xdustinface force-pushed the chore/sync-restructuring branch from 8172ac9 to c495f85 Compare December 4, 2025 01:08

xdustinface mentioned this pull request Dec 4, 2025

refactor: Cleanup SPV validation #237

Merged

xdustinface force-pushed the chore/sync-restructuring branch from c495f85 to 29cb8f5 Compare December 4, 2025 16:28

xdustinface marked this pull request as ready for review December 4, 2025 16:28

coderabbitai bot reviewed Dec 4, 2025

View reviewed changes

dash-spv/src/sync/mod.rs Outdated Show resolved Hide resolved

xdustinface force-pushed the chore/sync-restructuring branch from 29cb8f5 to b93ff0e Compare December 4, 2025 16:34

chore: Some restructuring in dash-spv::sync

ed72f5b

xdustinface force-pushed the chore/sync-restructuring branch from b93ff0e to ed72f5b Compare December 4, 2025 16:35

ZocoLini approved these changes Dec 4, 2025

View reviewed changes

xdustinface merged commit c76ddbb into v0.41-dev Dec 4, 2025
24 checks passed

pauldelucia pushed a commit that referenced this pull request Dec 12, 2025

chore: Some restructuring in dash-spv::sync (#236)

c2177fc

coderabbitai bot mentioned this pull request Dec 12, 2025

feat(key-wallet): Add DIP-17 Platform Payment account support #229

Merged

3 tasks

pauldelucia pushed a commit that referenced this pull request Dec 12, 2025

chore: Some restructuring in dash-spv::sync (#236)

edeedac

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: Some restructuring in `dash-spv::sync` #236

chore: Some restructuring in `dash-spv::sync` #236

Uh oh!

xdustinface commented Dec 4, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 4, 2025 •

edited

Loading

Rate limit exceeded

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

ZocoLini left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chore: Some restructuring in dash-spv::sync #236

chore: Some restructuring in dash-spv::sync #236

Uh oh!

Conversation

xdustinface commented Dec 4, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Structure before:

Structure with this PR:

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ZocoLini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chore: Some restructuring in `dash-spv::sync` #236

chore: Some restructuring in `dash-spv::sync` #236

xdustinface commented Dec 4, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 4, 2025 •

edited

Loading