fix: maintenance loop stops connecting at 1 peer

[xdustinface]

With MIN_PEERS=1 used as a "should we connect more peers" check, we ended up not trying to connect more peers in the maintenance loop which leads to getting stuck with 1 peer only after initial peers are gone.

Remove MIN_PEERS and MAX_PEERS and use TARGET_PEERS as the only peer count constant.

Summary by CodeRabbit

• Refactor

  • Consolidated multiple peer-count thresholds into a single unified threshold (TARGET_PEERS), simplifying peer connection logic and changing when the system adds peers or triggers discovery.

• Behavioral change

  • DNS discovery and auto-adding peers now wait until the unified TARGET_PEERS threshold is reached, raising the effective minimum connected peer target.

• Tests

  • Updated and removed network tests to match the simplified peer management.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR removes MIN_PEERS and MAX_PEERS and replaces their uses with TARGET_PEERS across the network module, updating peer maintenance, capacity checks, DNS discovery conditions, and related tests.

Changes

Cohort / File(s)	Summary
Constants dash-spv/src/network/constants.rs	Removed pub const MIN_PEERS and pub const MAX_PEERS and two compile-time assert!s referencing them; TARGET_PEERS remains.
Peer management logic dash-spv/src/network/manager.rs, dash-spv/src/network/pool.rs	Replaced checks and thresholds that referenced MIN_PEERS/MAX_PEERS with TARGET_PEERS; capacity, needs-more-peers, and DNS-discovery conditions now use TARGET_PEERS. Error/message text updated accordingly.
Tests dash-spv/src/network/tests.rs, dash-spv/tests/peer_test.rs	Updated test comment to reference TARGET_PEERS; removed the test_max_peer_limit unit test and imports/usages of MAX_PEERS. Assertions updated to align with TARGET_PEERS where applicable.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through constants, tidy and neat,
I nudged out MIN and MAX with quick feet,
TARGET_PEERS now stands — one hill to seek,
Peer fields simplified, my whiskers twitch cheek to cheek! 🥕

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: changing the maintenance loop threshold from MIN_PEERS (1) to TARGET_PEERS, which prevents the loop from stopping prematurely when exactly 1 peer is connected.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 fix/peer-maintenance-threshold

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

dash-spv/src/network/pool.rs (1)

152-160: needs_more_peers and can_accept_peers are now identical.

Both methods return self.peer_count().await < TARGET_PEERS. Previously they checked against MIN_PEERS and MAX_PEERS respectively, giving them distinct behavior. With a single threshold they're fully redundant.

Consider either (a) consolidating into one method, or (b) adding doc comments clarifying the intended semantic distinction so a future maintainer knows whether they should diverge again.

#!/bin/bash
# Check all call sites to understand if the two methods are used in different contexts
rg -n --type=rs '\b(needs_more_peers|can_accept_peers)\b' -C2

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@dash-spv/tests/peer_test.rs`:
- Line 185: The assertion in peer_test.rs uses TARGET_PEERS (constant defined as
3) but the failure message says "Default max peers should be 12"; update the
assertion message to reflect the actual expected value by changing the message
to "Default max peers should be 3" (locate the assert_eq!(TARGET_PEERS, 3, ...)
in the test and replace the string), ensuring consistency between TARGET_PEERS
and the human-readable assertion text.

🧹 Nitpick comments (1)

dash-spv/src/network/pool.rs (1)

152-160: needs_more_peers and can_accept_peers are now identical.

Both methods return self.peer_count().await < TARGET_PEERS. Previously they used different thresholds (MIN_PEERS vs MAX_PEERS), giving the system hysteresis. With a single constant, they are functionally duplicates. This is fine if you want distinct semantic names at call sites, but worth noting that there's no longer any gap between "we need more" and "we're at capacity."