Skip to content

refactor(peer_manager): modular peer manager #428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

diegomrsantos
Copy link
Contributor

@diegomrsantos diegomrsantos commented Jul 15, 2025

Proposed Changes

This PR refactors the monolithic peer_manager.rs into a modular structure with focused modules for better maintainability and separation of concerns. The refactor maintains all existing functionality while improving code organization.

  • Splits peer manager into specialized modules: connection management, peer discovery, heartbeat events, and common types
  • Introduces module-based architecture similar to existing scoring module pattern
  • Preserves all existing networking behavior and API contracts

@diegomrsantos diegomrsantos requested a review from Copilot July 15, 2025 18:19
@diegomrsantos diegomrsantos self-assigned this Jul 15, 2025
Copilot

This comment was marked as outdated.

- Split monolithic peer_manager.rs into focused modules:
  - connection.rs: Connection management and limits
  - discovery.rs: Peer discovery and subnet-based selection
  - heartbeat.rs: Periodic heartbeat events and status reporting
  - types.rs: Common types and events
  - mod.rs: Main PeerManager coordination

- Improve code organization and separation of concerns
- Maintain existing functionality while improving maintainability
- Each module has a single responsibility
- Easier to test individual components
- Follows same pattern as existing scoring/ module

This refactor prepares the codebase for future peer blocking
functionality while keeping the current implementation clean
and modular.
@diegomrsantos diegomrsantos force-pushed the refactor/modular-peer-manager branch 2 times, most recently from 147d165 to b3ce3f4 Compare July 15, 2025 19:28
Copy link
Member

@dknopik dknopik left a comment

Choose a reason for hiding this comment

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

Have not checked line for line, but I like the idea and the approach!

@diegomrsantos diegomrsantos requested review from Copilot and dknopik July 16, 2025 11:10
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the monolithic peer_manager.rs into a modular structure with focused modules for better maintainability and separation of concerns. The refactor maintains all existing functionality while improving code organization.

  • Splits peer manager into specialized modules: connection management, peer discovery, heartbeat events, and common types
  • Introduces module-based architecture with ConnectionManager, PeerDiscovery, and HeartbeatManager components
  • Updates API method name from discovered_peer to report_discovered_peer for clarity

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
anchor/network/src/peer_manager/mod.rs Main peer manager module that coordinates all sub-components and implements NetworkBehaviour
anchor/network/src/peer_manager/types.rs Defines common types like ConnectActions and Event used across modules
anchor/network/src/peer_manager/connection.rs Manages peer connections and connection limits logic
anchor/network/src/peer_manager/discovery.rs Handles peer discovery and subnet-based peer selection
anchor/network/src/peer_manager/heartbeat.rs Manages periodic heartbeat events and status reporting
anchor/network/src/peer_manager.rs Removes the original monolithic implementation
anchor/network/src/network.rs Updates method call to use the new API name

@diegomrsantos diegomrsantos marked this pull request as ready for review July 16, 2025 11:19
@diegomrsantos diegomrsantos added the ready-for-review This PR is ready to be reviewed label Jul 16, 2025
@diegomrsantos diegomrsantos changed the title Modular peer manager refactor(peer_manager): modular peer manager Jul 16, 2025
Copy link
Member

@dknopik dknopik left a comment

Choose a reason for hiding this comment

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

LGTM, thanks :)

@diegomrsantos diegomrsantos merged commit afb8626 into sigp:unstable Jul 16, 2025
14 checks passed
@diegomrsantos diegomrsantos deleted the refactor/modular-peer-manager branch July 16, 2025 14:26
@dknopik
Copy link
Member

dknopik commented Jul 16, 2025

@diegomrsantos Please merge using the ready-for-merge tag in future to ensure it is tested with latest unstable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review This PR is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants