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

fix(connection-pool): Keep only fluence peers in the connection pool #1440

Merged
merged 11 commits into from
Feb 3, 2023
Merged
4 changes: 2 additions & 2 deletions connection-pool/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,12 +250,12 @@ impl ConnectionPoolBehaviour {
self.subscribers.push(outlet);
}

pub fn add_discovered_addresses(&mut self, peer_id: PeerId, addresses: Vec<Multiaddr>) {
pub fn add_discovered_addresses(&mut self, peer_id: PeerId, addresses: &Vec<Multiaddr>) {
self.contacts
.entry(peer_id)
.or_default()
.discovered
.extend(addresses);
.extend(addresses.to_vec());
}

fn meter<U, F: Fn(&ConnectionPoolMetrics) -> U>(&self, f: F) {
Expand Down
2 changes: 1 addition & 1 deletion crates/kademlia/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ impl Kademlia {
}
}

pub fn add_kad_node(&mut self, peer: PeerId, addresses: Vec<Multiaddr>) {
pub fn add_kad_node(&mut self, peer: PeerId, addresses: &Vec<Multiaddr>) {
Copy link
Member

@folex folex Feb 2, 2023

Choose a reason for hiding this comment

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

&[]? or clippy will complain

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

for addr in addresses {
self.kademlia.add_address(&peer, addr.clone());
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to remove .clone

}
Expand Down
52 changes: 39 additions & 13 deletions particle-node/src/behaviour/identify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use libp2p::{
core::{multiaddr::Protocol, Multiaddr},
identify::Event as IdentifyEvent,
};
use particle_protocol::PROTOCOL_NAME;

use super::FluenceNetworkBehaviour;

Expand All @@ -36,17 +37,42 @@ impl FluenceNetworkBehaviour {
info.listen_addrs
);

let addresses = filter_addresses(info.listen_addrs, allow_local_addresses);
let addresses = filter_addresses(&info.listen_addrs, allow_local_addresses);

// Add addresses to connection pool disregarding whether it supports kademlia or not
// we want to have full info on non-kademlia peers as well
self.connection_pool
.add_discovered_addresses(peer_id, addresses.clone());
let mut supports_kademlia = false;
let mut supports_fluence = false;

let supports_kademlia =
info.protocols.iter().any(|p| p.contains("/ipfs/kad/1.0.0"));
if supports_kademlia {
self.kademlia.add_kad_node(peer_id, addresses);
for protocol in info.protocols.iter() {
if !supports_kademlia && protocol.contains("/ipfs/kad/1.0.0") {
supports_kademlia = true;
}
if !supports_fluence && protocol.contains(PROTOCOL_NAME) {
supports_fluence = true;
}
if supports_fluence && supports_kademlia {
break;
}
}

if supports_fluence {
log::debug!(target: "network", "Found fluence peer {}: : protocols: {:?} version: {} listen addrs {:?}", peer_id, info.protocols,
folex marked this conversation as resolved.
Show resolved Hide resolved
info.protocol_version,
info.listen_addrs);
// Add addresses to connection pool disregarding whether it supports kademlia or not
// we want to have full info on non-kademlia peers as well
self.connection_pool
.add_discovered_addresses(peer_id, &addresses);
if supports_kademlia {
self.kademlia.add_kad_node(peer_id, &addresses);
}
} else {
log::debug!(
folex marked this conversation as resolved.
Show resolved Hide resolved
target: "protocols",
Copy link
Member

Choose a reason for hiding this comment

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

lets change to target: network or target: blocked. protocols is too narrow, don't think we'll have much of these so no sense in grouping them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

"Found peer {} not supported fluence protocol, protocols: {:?} version: {} listen addrs {:?}. skipping...",
peer_id, info.protocols,
info.protocol_version,
info.listen_addrs
);
}
}

Expand All @@ -61,18 +87,18 @@ impl FluenceNetworkBehaviour {
}
}

fn filter_addresses(addresses: Vec<Multiaddr>, allow_local: bool) -> Vec<Multiaddr> {
fn filter_addresses(addresses: &Vec<Multiaddr>, allow_local: bool) -> Vec<Multiaddr> {
// Deduplicate addresses
let addresses: Vec<_> = addresses.into_iter().unique().collect();
let addresses = addresses.into_iter().unique();

if allow_local {
// Return all addresses
addresses
addresses.cloned().collect()
} else {
// Keep only global addresses
addresses
.into_iter()
.filter(|maddr| !is_local_maddr(maddr))
.cloned()
.collect()
}
}
Expand Down