refactor: add ensure_not_started() guard

[xdustinface]

Add a state guard that returns an error if a manager's sync has already been started, and use it in start_sync() implementations instead of inline state checks.

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Enhanced error handling for concurrent sync attempts with clearer error messages that identify which specific sync manager is already active.
  • Added centralized guards across sync components to prevent re-entrancy and duplicate sync operations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

📝 Walkthrough

Walkthrough

Changes enhance sync error handling by refactoring SyncError::SyncInProgress to carry a ManagerIdentifier and consolidating duplicate inline state guards across multiple sync manager implementations into a centralized ensure_not_started helper function. This improves error specificity and reduces code duplication.

Changes

Cohort / File(s)	Summary
Error Type Definition dash-spv/src/error.rs	Updated SyncInProgress enum variant from unit variant to data variant carrying ManagerIdentifier payload. Updated error formatting to include manager context in messages like "{0} already started".
Sync Manager Implementations dash-spv/src/sync/block_headers/sync_manager.rs, dash-spv/src/sync/blocks/sync_manager.rs, dash-spv/src/sync/filters/sync_manager.rs	Replaced inline state validation guards with calls to ensure_not_started helper function, removing previous warning logs and early returns in favor of error propagation.
Core Sync Manager dash-spv/src/sync/sync_manager.rs	Added new pub(super) ensure_not_started helper function that validates state is WaitingForConnections, returning SyncError::SyncInProgress with ManagerIdentifier if not. Updated default start_sync to use this helper instead of inline checks.
Sync Coordinator dash-spv/src/sync/sync_coordinator.rs	Changed error return from SyncError::SyncInProgress to SyncError::InvalidState in start method guard.
Test Updates dash-spv/tests/error_types_test.rs	Updated test cases to construct and pattern-match SyncInProgress(ManagerIdentifier) variant. Adjusted error message expectations to reflect manager-specific context in messages.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A scattered warren of guards became one,
ManagerIdentifiers now carry the tale,
Where each sync manager bows to the sun,
Of centralized wisdom that won't ever fail.
Hop, hop! The errors now speak true and clear! 🌟

🚥 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 captures the main refactoring change: introducing a centralized ensure_not_started() guard function used across multiple sync manager implementations.
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/ensure-not-started

---

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

[ZocoLini]

I think this is bad practice, we are checking this in runtime when we can do it compile time using TypeStatePattern, are you familiar with it??

[xdustinface]

This just moves to logic into a function but if you think you can apply the pattern here, feel free to go ahead in a follow up PR :)