Skip to content

feat: react to network events in maintenance loop#433

Merged
xdustinface merged 1 commit intov0.42-devfrom
feat/reactive-peer-recovery
Feb 17, 2026
Merged

feat: react to network events in maintenance loop#433
xdustinface merged 1 commit intov0.42-devfrom
feat/reactive-peer-recovery

Conversation

@xdustinface
Copy link
Collaborator

@xdustinface xdustinface commented Feb 13, 2026

Subscribe to network events so the maintenance loop wakes immediately on peer connect/disconnect to reset the DNS
timer and to try to connect to known addresses.

Based on:

Summary by CodeRabbit

  • Refactor
    • Enhanced network event handling in the maintenance system to respond more promptly to network condition changes, improving system reactivity and responsiveness to network state transitions.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 13, 2026

📝 Walkthrough

Walkthrough

The maintenance loop now subscribes to network events, logging received events, resetting DNS discovery timers, and triggering immediate maintenance checks. This adds a reactive event-driven pathway to the previously timer-based maintenance loop.

Changes

Cohort / File(s) Summary
Network Event Listener
dash-spv/src/network/manager.rs
Added event listener in maintenance loop to handle incoming NetworkEvent messages, log events, reset DNS discovery timers, and trigger maintenance_tick on event reception; loop breaks on stream error.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A listener awakes with each network chime,
Events cascade through the maintenance line,
DNS timers reset in a synchronized dance,
The manager's loop finds its reactive chance,
Where events and tick marks now intertwine! ✨

🚥 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 pull request title 'feat: react to network events in maintenance loop' clearly and accurately summarizes the main change - adding network event subscription to the maintenance loop to enable reactive behavior on connectivity changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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 feat/reactive-peer-recovery

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the merge-conflict The PR conflicts with the target branch. label Feb 16, 2026
@github-actions
Copy link

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

Base automatically changed from refactor/split-loop to v0.42-dev February 17, 2026 00:48
Subscribe to network events so the maintenance loop wakes
immediately on peer connect/disconnect to reset the DNS
 timer and to try to connect to known addresses.
@xdustinface xdustinface force-pushed the feat/reactive-peer-recovery branch from 3afc1a5 to ec3ab31 Compare February 17, 2026 09:44
@github-actions github-actions bot removed the merge-conflict The PR conflicts with the target branch. label Feb 17, 2026
@xdustinface xdustinface marked this pull request as ready for review February 17, 2026 09:54
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@dash-spv/src/network/manager.rs`:
- Line 887: Replace the inconsistent tracing::error! call with log::error! to
match the rest of the file's logging macros; update the occurrence in the
network event handling (the line currently emitting "Network event error: {}"
using tracing::error!) to use log::error! instead so it aligns with other calls
like log::debug!, log::info!, log::warn!, and log::error! throughout manager.rs.
- Around line 879-891: The maintenance loop currently treats any
broadcast::RecvError as fatal; instead, in the event handling branch where
network_events.recv() returns Err(error) match on the RecvError: if it's
RecvError::Lagged(n) log a warning/debug including n, skip breaking (continue
the loop) and allow dns_interval.reset() and this.maintenance_tick() to keep
running; if it's RecvError::Closed then log an error and break the loop. Update
the Err(error) arm to pattern-match RecvError::Lagged and RecvError::Closed
accordingly around network_events.recv(), keeping calls to dns_interval.reset()
and this.maintenance_tick() in the Ok(event) path.

@thepastaclaw
Copy link

Addressed the two review items from CodeRabbit in #448:

  1. Logging consistency — replaced tracing::error! with log::error! to match the rest of manager.rs
  2. Graceful Lagged handlingRecvError::Lagged now logs a warning and continues (missed events aren't fatal), while RecvError::Closed logs an error and breaks the loop

@xdustinface xdustinface merged commit 79233c5 into v0.42-dev Feb 17, 2026
53 checks passed
@xdustinface xdustinface deleted the feat/reactive-peer-recovery branch February 17, 2026 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants