refactor(dash-spv): reputation module cleanup

[ZocoLini]

Refactors the reputation system from manual (score: i32, reason: &str) calls to a typed ChangeReason enum, and removes dead code surfaced along the way. No behavior change

• Replace ad-hoc score/string arguments with a ChangeReason enum that owns each reason's score delta and label; all update_reputation call sites now pass a typed variant.
• Drop the misbehavior_scores / positive_scores constant modules (folded into ChangeReason::score()).
• Remove the write-only event subsystem (ReputationEvent, recent_events, record_event, get_recent_events) — nothing ever read it.
• Remove unused methods: reset_reputation, get_peers_by_reputation, get_score, temporary_ban_peer.
• Make the reputation module private again.

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of peers that send invalid ChainLock or InstantLock messages.
  • Invalid peers are now penalized, temporarily banned, and disconnected more consistently.
  • Added stronger validation before accepting InstantSend/ChainLock messages, helping reduce bad network behavior and improve sync reliability.

[coderabbitai]

📝 Walkthrough

Walkthrough

A new ChangeReason enum (InvalidChainLock, InvalidInstantLock) is added to the reputation module with score() and Display. The NetworkRequest::PenalizePeer variant, RequestSender helpers, and NetworkManager trait methods are updated to use ChangeReason instead of string-based reasons. The ChainLock and InstantLock sync managers are updated to penalize peers on invalid messages using these typed reasons.

Changes

Typed peer penalization

Layer / File(s)	Summary
ChangeReason enum dash-spv/src/network/reputation.rs	Adds ChangeReason enum with InvalidChainLock/InvalidInstantLock variants, score() method, and Display impl.
NetworkRequest variant and NetworkManager trait dash-spv/src/network/mod.rs	Adds PenalizePeer(SocketAddr, ChangeReason) to NetworkRequest; updates RequestSender helpers to drop the reason-string parameter; updates NetworkManager trait penalty signatures to accept ChangeReason; sets PeerReputation re-export to pub(crate).
NetworkManager implementation dash-spv/src/network/manager.rs	Dispatches PenalizePeer requests by ChangeReason variant; updates penalize_peer to call update_reputation(reason.score()), temporary_ban_peer (10 min), and disconnect_peer with reason.to_string().
ChainLock and InstantLock sync managers dash-spv/src/sync/chainlock/sync_manager.rs, dash-spv/src/sync/instantsend/sync_manager.rs, dash-spv/src/sync/instantsend/manager.rs	CLSig handler penalizes peer when a ChainLockReceived { validated: false } event is produced; ISLock handler calls validate_structure (now pub(super)) and penalizes peer before processing on structural failure.
Storage peers import cleanup dash-spv/src/storage/peers.rs	Updates PeerReputation import path; moves peers vector declaration inside spawn_blocking closure.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 A reason once was just a string,
Now ChangeReason does the naming thing.
Invalid locks get scored and banned,
Peers disconnected, reputation scanned.
Type-safe penalties hop along —
The rabbit's network growing strong! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately reflects the reputation-focused cleanup across dash-spv.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/reputation-cleanup

---

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

[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]

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/reputation.rs (1)

117-187: Decay moves scores away from neutral; should trend toward 0.

apply_decay subtracts DECAY_AMOUNT regardless of sign, which makes negative (good) scores grow more negative over time without new positive behavior. If decay is meant to normalize toward neutral, adjust based on the sign.

🐛 Suggested fix

-            let decay = intervals_i32.saturating_mul(DECAY_AMOUNT);
-            self.score = (self.score - decay).max(MIN_MISBEHAVIOR_SCORE);
+            let decay = intervals_i32.saturating_mul(DECAY_AMOUNT);
+            if self.score > 0 {
+                self.score = (self.score - decay).max(0);
+            } else if self.score < 0 {
+                self.score = (self.score + decay).min(0);
+            }

🧹 Nitpick comments (2)

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

200-215: Avoid silently discarding reputation load failures.

Swallowing the storage error hides corruption and makes diagnostics harder; consider logging it before falling back to an empty map.

♻️ Suggested tweak

-        let mut reputations =
-            storage.load_peers_reputation().await.unwrap_or_else(|_| HashMap::new());
+        let mut reputations = match storage.load_peers_reputation().await {
+            Ok(reputations) => reputations,
+            Err(e) => {
+                log::warn!("Failed to load peer reputations: {e}");
+                HashMap::new()
+            }
+        };

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

170-173: Avoid awaiting while holding the reputation mutex (possible contention / clippy::await_holding_lock).

Patterns like lock().await.*(...).await hold the mutex across .await, and save_to_storage also awaits IO. Consider making PeerReputationManager methods synchronous or snapshotting reputations before async IO so the lock can be released early. As per coding guidelines, please ensure clippy passes with warnings-as-errors.

Also applies to: 758-759, 1023-1025

[github-actions]

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

[codecov]

Codecov Report

❌ Patch coverage is 28.76712% with 52 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.14%. Comparing base (03096dd) to head (bf25b4c).

Files with missing lines	Patch %	Lines
dash-spv/src/network/manager.rs	8.69%	21 Missing ⚠️
dash-spv/src/network/reputation.rs	44.82%	16 Missing ⚠️
dash-spv/src/network/mod.rs	0.00%	15 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #328      +/-   ##
==========================================
- Coverage   73.16%   73.14%   -0.02%     
==========================================
  Files         323      323              
  Lines       72438    72433       -5     
==========================================
- Hits        52999    52981      -18     
- Misses      19439    19452      +13     

Flag	Coverage Δ
core	76.94% <ø> (ø)
ffi	47.70% <ø> (ø)
rpc	20.00% <ø> (ø)
spv	90.25% <28.76%> (-0.09%)	⬇️
wallet	71.77% <ø> (ø)

Files with missing lines	Coverage Δ
dash-spv/src/storage/peers.rs	82.08% <100.00%> (+0.27%)	⬆️
dash-spv/src/sync/chainlock/sync_manager.rs	88.57% <100.00%> (+1.47%)	⬆️
dash-spv/src/sync/instantsend/manager.rs	84.40% <100.00%> (-8.80%)	⬇️
dash-spv/src/sync/instantsend/sync_manager.rs	88.46% <ø> (ø)
dash-spv/src/network/mod.rs	85.83% <0.00%> (-12.27%)	⬇️
dash-spv/src/network/reputation.rs	75.89% <44.82%> (+1.30%)	⬆️
dash-spv/src/network/manager.rs	71.07% <8.69%> (+1.20%)	⬆️

... and 1 file with indirect coverage changes