fix: store all peers in storage

[xdustinface]

We currently fetch peers to store via get_addresses_for_peer(MAX_ADDR_TO_STORE) which caps the number internally to MAX_ADDR_TO_SEND. Its not a problem really right now since we so far not even close to end up with so many peers because peer discovery isn't functional but its still wrong.

• First commit refactors get_known_addresses to return the full AddrV2Message instead of just the SocketAddr.
• Second commit fixes it by using get_known_addresses for peer saving.

Working towards closing:

• fix: peer discovery and reputation usage #416

Summary by CodeRabbit

Release Notes

• Refactor

  • Updated internal peer address handling and storage mechanisms throughout the networking layer.
  • Enhanced peer selection logic to work with improved address representations.

• Tests

  • Added test utilities for improved peer address testing.
  • Updated test infrastructure to support the refactored networking components.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request modifies peer address handling throughout the network module, changing the internal API to work with AddrV2Message objects instead of raw SocketAddr types. This affects address retrieval, peer selection algorithms, and test infrastructure while maintaining the same underlying functionality.

Changes

Cohort / File(s)	Summary
Core API Updates dash-spv/src/network/addrv2.rs, dash-spv/src/network/reputation.rs	Method signatures updated to accept and return Vec<AddrV2Message> instead of Vec<SocketAddr>. get_known_addresses() now returns full AddrV2Message objects; select_best_peers() accepts AddrV2Message list with internal conversion to SocketAddr for reputation scoring.
Implementation Changes dash-spv/src/network/manager.rs	Updated to call get_known_addresses() for persisting known addresses to disk instead of the previous limited subset approach.
Test Infrastructure dash-spv/src/network/reputation_tests.rs, dash-spv/tests/peer_test.rs	Test setup and assertions updated to construct AddrV2Message objects and extract underlying SocketAddr values via socket_addr().unwrap() for comparisons.
Test Utilities dash/src/test_utils/mod.rs, dash/src/test_utils/network.rs	Added network module with new AddrV2Message::dummy() helper function to simplify construction of test message instances with configurable time, IPv4 address, and port.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

Across the network hops our furry friend,
AddrV2Messages 'round each bend,
Type-safe addresses dance and play,
Testing dummies light the way! 🐰✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: store all peers in storage' directly and clearly describes the main change: fixing the peer storage mechanism to persist all known peers instead of a capped subset.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ 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/store-all-peers

---

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: 0

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)

1252-1258: ⚠️ Potential issue | 🟠 Major

shutdown() still uses get_addresses_for_peer(MAX_ADDR_TO_STORE) — same cap bug persists here.

The maintenance loop (line 889) was fixed to use get_known_addresses(), but shutdown() still calls get_addresses_for_peer(MAX_ADDR_TO_STORE) which internally caps the result to MAX_ADDR_TO_SEND (line 102 in addrv2.rs). Since shutdown is the last chance to persist peers, this is arguably the most important call site to fix.

Proposed fix

-        let addresses = self.addrv2_handler.get_addresses_for_peer(MAX_ADDR_TO_STORE).await;
+        let addresses = self.addrv2_handler.get_known_addresses().await;