refactor: split network maintenance loop into smaller functions

[xdustinface]

Make it better readable and maintainable.

Based on:

• refactor: move manager clone into start_maintenance_loop task #426

Summary by CodeRabbit

• Refactor
  • Reorganized peer maintenance processes by consolidating network management tasks into focused operations for improved code clarity.
  • Enhanced DNS-based peer discovery fallback logic with better separation of concerns and control flow.

[coderabbitai]

📝 Walkthrough

Walkthrough

The peer maintenance loop in the network manager is refactored by extracting per-tick maintenance logic and DNS fallback discovery into separate private methods (maintenance_tick and dns_fallback_tick), improving code organization while preserving existing behavior and public APIs.

Changes

Cohort / File(s)	Summary
Peer Maintenance Refactoring dash-spv/src/network/manager.rs	Extracted maintenance loop logic into dedicated maintenance_tick (handles cleanup, peer counting, reconnection, pings, data persistence) and dns_fallback_tick (handles DNS-based peer discovery). Main loop now delegates to these methods on appropriate intervals. Existing behavior preserved for exclusive mode, reputation updates, and peer persistence.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hoppy refactoring, so neat and so clean,
Ticks split asunder, a system serene,
Peers maintained well in methods so fine,
Logic extracted—each task gets its line!
DNS discovery hops on its own beat,
Making our code more organized—what a treat! 🌟

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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: refactoring the network maintenance loop by extracting logic into smaller functions (maintenance_tick and dns_fallback_tick).
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into v0.42-dev

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

✨ 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/split-loop

---

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.

[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/network/manager.rs (1)

861-885: ⚠️ Potential issue | 🔴 Critical

Replace time::sleep with time::interval to prevent maintenance task starvation.

time::sleep(MAINTENANCE_INTERVAL) creates a fresh future on each tokio::select! loop iteration. Since time::interval().tick() fires immediately on the first poll, the DNS branch completes first and the maintenance sleep is dropped before awaiting. With MAINTENANCE_INTERVAL and DNS_DISCOVERY_DELAY both at 10 seconds, maintenance_tick will never execute in non-exclusive mode.

Use time::interval for both timers to maintain independent periodic cadences:

Proposed fix

     async fn start_maintenance_loop(&self) {
         let this = self.clone();
         let mut tasks = self.tasks.lock().await;
         tasks.spawn(async move {
+            let mut maintenance_interval = time::interval(MAINTENANCE_INTERVAL);
             let mut dns_interval = time::interval(DNS_DISCOVERY_DELAY);
 
             while !this.shutdown_token.is_cancelled() {
                 tokio::select! {
-                    _ = time::sleep(MAINTENANCE_INTERVAL) => {
+                    _ = maintenance_interval.tick() => {
                         log::debug!("Maintenance interval elapsed");
                         this.maintenance_tick().await;
                     }
                     _ = dns_interval.tick(), if !this.exclusive_mode => {
                         this.dns_fallback_tick().await;
                     }
                     _ = this.shutdown_token.cancelled() => {
                         log::info!("Maintenance loop shutting down");
                         break;
                     }
                 }
             }
         });
     }

Note: Both time::interval instances tick immediately on first poll. Use interval.tick().await before the loop or time::interval_at() to skip the initial tick if needed.