Skip to content
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

add basic peer manager #108

Open
wants to merge 6 commits into
base: unstable
Choose a base branch
from

Conversation

diegomrsantos
Copy link

@diegomrsantos diegomrsantos commented Jan 16, 2025

Issue Addressed

Related to #18

Proposed Changes

Dependencies:

Updated Cargo.toml and anchor/network/Cargo.toml to include new dependencies such as smallvec, arbitrary, delay_map, and itertools.

Network Types:

Added peer_manager module to anchor/network/src/lib.rs.

Network Implementation:

Enhanced anchor/network/src/behaviour.rs and anchor/network/src/network.rs to integrate the PeerManager.
The PeerManager is responsible for managing peer connections, including dialing, connecting, and disconnecting peers.
It includes logic to maintain the target peer count, ensuring the network has the desired number of peers connected.

Peer Manager:

Added a new peer_manager directory with modules for managing peers.
Implemented functionality in PeerManager to handle peer connections, maintain the target peer count, and manage peer discovery.

Additional Info

For now, it's mostly a copy from LH with a few modifications. However, there are efforts to make the LH code more modular, generic, and thus easier to reuse. See sigp/lighthouse#6840 and sigp/lighthouse#6757

@diegomrsantos diegomrsantos changed the title add basic peer manage add basic peer manager Jan 16, 2025
@diegomrsantos diegomrsantos force-pushed the peer-manager branch 3 times, most recently from 01ebfd7 to d0970c4 Compare January 22, 2025 13:47
Comment on lines +175 to +176
self.outbound_ping_peers.insert(peer_id);
self.events.push(PeerManagerEvent::Ping(peer_id));
Copy link
Author

Choose a reason for hiding this comment

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

@AgeManning do you know why we need this? I thought it was handled by libp2p's Ping.

@diegomrsantos diegomrsantos marked this pull request as ready for review January 23, 2025 19:48
@diegomrsantos diegomrsantos requested review from dknopik, AgeManning and jking-aus and removed request for dknopik and AgeManning January 23, 2025 19:48
@@ -85,6 +85,7 @@ parking_lot = "0.12"
reqwest = "0.12.12"
rusqlite = "0.28.0"
serde = { version = "1.0.208", features = ["derive"] }
smallvec = "2.0.0-alpha.9"
Copy link
Member

Choose a reason for hiding this comment

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

Any specific reason why 2.0.0-alpha.9? Unless we need it, I suggest we stay with the most recent one from 1.*.

Copy link
Author

Choose a reason for hiding this comment

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

There's no reason, I just chose the latest one.

Comment on lines +379 to +393
// Updates peer's scores and unban any peers if required.
//let actions = self.peers.write().update_scores();
//for (peer_id, action) in actions {
// self.handle_score_action(&peer_id, action, None);
//}

// Update peer score metrics;
//self.update_peer_score_metrics();

// Prune any excess peers back to our target in such a way that incentivises good scores and
// a uniform distribution of subnets.
//self.prune_excess_peers();

// Unban any peers that have served their temporary ban timeout
//self.unban_temporary_banned_peers();
Copy link
Member

Choose a reason for hiding this comment

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

What's the plan for the commented calls here?

Copy link
Author

Choose a reason for hiding this comment

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

We'll need it eventually, but it's not important for now. The plan is to reuse the LH code anyway.

Comment on lines +26 to +37
/// A lighthouse node (the best kind).
Lighthouse,
/// A Nimbus node.
Nimbus,
/// A Teku node.
Teku,
/// A Prysm node.
Prysm,
/// A lodestar node.
Lodestar,
/// A Caplin node.
Caplin,
Copy link
Member

Choose a reason for hiding this comment

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

Need to update this to Anchor and GoSsv

Copy link
Author

Choose a reason for hiding this comment

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

I think this isn't important for now. Wdyt?

Comment on lines +44 to +46
/// The current syncing state of the peer. The state may be determined after it's initial
/// connection.
sync_status: SyncStatus,
Copy link
Member

Choose a reason for hiding this comment

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

SSV has no sync status communicated via network, right?

Copy link
Author

@diegomrsantos diegomrsantos Jan 24, 2025

Choose a reason for hiding this comment

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

I don't know, we need input here. But I'd say it's not important for now.

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.

2 participants