refactor: move manager initialization from trait method to constructors

[xdustinface]

• Moves initialization logic out of the SyncManager::initialize() trait method and into each manager's constructor.
• Genesis block is now stored before managers are created, so they can read the tip during construction.
• The SyncCoordinator seeds initial progress from actual manager state instead of default and uses WatchStream::from_changes to avoid re-emitting all initial progresses.

Based on:

• feat: report/propagate client start/run failures #432
• refactor: remove SyncState::Initializing variant #465
• refactor: add ensure_not_started() guard #466

Summary by CodeRabbit

• Refactor
  • Streamlined component initialization by integrating setup logic into the construction phase, eliminating a separate initialization lifecycle step.
  • Updated synchronization managers to initialize asynchronously during creation for improved startup sequencing.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request refactors manager initialization by converting synchronous constructors to asynchronous ones that perform their own setup, and removes the initialize() method from the SyncManager trait. Manager initialization logic moves from a separate lifecycle phase into construction itself. The client's new() method is also made async to orchestrate genesis block setup and async manager initialization.

Changes

Cohort / File(s)	Summary
Trait Definition src/sync/sync_manager.rs, ARCHITECTURE.md	Removed async fn initialize() method from the SyncManager trait and updated architecture documentation. Managers no longer have a separate initialization lifecycle phase.
Manager Async Constructors src/sync/block_headers/manager.rs, src/sync/filter_headers/manager.rs, src/sync/filters/manager.rs, src/sync/blocks/manager.rs, src/sync/masternodes/manager.rs, src/sync/chainlock/manager.rs	Converted synchronous constructors to async functions that read storage/wallet state and initialize progress before returning. Each now performs setup that was previously delegated to separate initialize() calls.
Manager SyncManager Implementations src/sync/block_headers/sync_manager.rs, src/sync/filter_headers/sync_manager.rs, src/sync/filters/sync_manager.rs, src/sync/blocks/sync_manager.rs, src/sync/chainlock/sync_manager.rs	Removed async fn initialize() implementations from each manager's SyncManager trait impl. Initialization logic now occurs in async constructors instead.
Client Lifecycle src/client/lifecycle.rs	Refactored Client::new() to be async and handle genesis/checkpoint initialization via new initialize_genesis_block() associated function. Storage parameter made mutable. All manager construction updated to await async constructors. Initialization of wallet data and mempool tracking integrated into constructor flow.
Sync Coordinator src/sync/sync_coordinator.rs	Made SyncCoordinator::new() async with pre-computed initial progress aggregated from manager states. Updated spawn_manager macro to accept manager expressions as parameters. Introduced try_update_progress() helper for optional progress merging.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Managers as Managers<br/>(BlockHeaders, etc.)
    participant Storage
    participant Coordinator

    rect rgba(200, 150, 255, 0.5)
    Note over Client,Coordinator: Before (Old Flow)
    Client->>Managers: new() → sync
    Client->>Managers: initialize().await
    Managers->>Storage: read state
    Managers->>Managers: set initial progress
    Client->>Coordinator: new()
    Client->>Coordinator: start_sync()
    end

    rect rgba(150, 200, 255, 0.5)
    Note over Client,Coordinator: After (New Flow)
    Client->>Managers: new().await (with init)
    Managers->>Storage: read state
    Managers->>Managers: set initial progress
    Client->>Client: initialize_genesis_block()
    Client->>Coordinator: new().await
    Coordinator->>Managers: aggregate progress
    Client->>Coordinator: start_sync()
    end

Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 A rabbit hops through async code,
Where constructors now take the load,
No more initialize delay,
Just awaken and sync the way,
Building managers bright and bold! ✨

🚥 Pre-merge checks | ✅ 3

✅ 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 and concisely describes the main refactoring change: moving manager initialization from a trait method to constructors.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/move-initialization

---

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

[github-actions]

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

[github-actions]

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

[github-actions]

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

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

175-203: ⚠️ Potential issue | 🟠 Major

Rollback coordinator startup if network connect fails.

After SyncCoordinator::start (Line 187), a connect() failure (Line 194) returns early with running == false, but spawned sync tasks remain active. Since stop() exits early when not running, those tasks may never be shut down.

🔧 Suggested fix

-        // Connect to network
-        self.network.lock().await.connect().await?;
+        // Connect to network
+        if let Err(e) = self.network.lock().await.connect().await {
+            if let Err(shutdown_err) = self.sync_coordinator.lock().await.shutdown().await {
+                tracing::warn!(
+                    "Failed to rollback sync coordinator after connect error: {}",
+                    shutdown_err
+                );
+            }
+            return Err(e);
+        }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@dash-spv/src/client/lifecycle.rs` around lines 175 - 203, When
network.connect() fails after starting the sync coordinator, rollback the
coordinator startup by calling its shutdown/stop method (the same API used to
stop coordinator tasks) before returning the error; specifically, in
Client::start after self.network.lock().await.connect().await? fails, acquire
the sync_coordinator lock and invoke SyncCoordinator::stop (or the appropriate
shutdown method) to terminate spawned tasks, handle/log any error from that stop
call, and then return SpvError::Sync(e) so running remains false; reference the
start method, self.sync_coordinator, SyncCoordinator::start, and the
network.connect call when making this change.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@dash-spv/src/client/lifecycle.rs`:
- Around line 175-203: When network.connect() fails after starting the sync
coordinator, rollback the coordinator startup by calling its shutdown/stop
method (the same API used to stop coordinator tasks) before returning the error;
specifically, in Client::start after self.network.lock().await.connect().await?
fails, acquire the sync_coordinator lock and invoke SyncCoordinator::stop (or
the appropriate shutdown method) to terminate spawned tasks, handle/log any
error from that stop call, and then return SpvError::Sync(e) so running remains
false; reference the start method, self.sync_coordinator,
SyncCoordinator::start, and the network.connect call when making this change.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2fa3224 and 640c1f8.

📒 Files selected for processing (15)

• dash-spv/ARCHITECTURE.md
• dash-spv/src/client/lifecycle.rs
• dash-spv/src/sync/block_headers/manager.rs
• dash-spv/src/sync/block_headers/sync_manager.rs
• dash-spv/src/sync/blocks/manager.rs
• dash-spv/src/sync/blocks/sync_manager.rs
• dash-spv/src/sync/chainlock/manager.rs
• dash-spv/src/sync/chainlock/sync_manager.rs
• dash-spv/src/sync/filter_headers/manager.rs
• dash-spv/src/sync/filter_headers/sync_manager.rs
• dash-spv/src/sync/filters/manager.rs
• dash-spv/src/sync/filters/sync_manager.rs
• dash-spv/src/sync/masternodes/manager.rs
• dash-spv/src/sync/sync_coordinator.rs
• dash-spv/src/sync/sync_manager.rs

💤 Files with no reviewable changes (7)

• dash-spv/ARCHITECTURE.md
• dash-spv/src/sync/sync_manager.rs
• dash-spv/src/sync/chainlock/sync_manager.rs
• dash-spv/src/sync/filter_headers/sync_manager.rs
• dash-spv/src/sync/filters/sync_manager.rs
• dash-spv/src/sync/blocks/sync_manager.rs
• dash-spv/src/sync/block_headers/sync_manager.rs