Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Peerset::discovered accepts many peer ids #2213

Merged
bkchr merged 2 commits into from
Apr 5, 2019
Merged

Peerset::discovered accepts many peer ids #2213

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
35be41a
Peerset::discovered accepts many peer ids
debris Apr 4, 2019
e920d3b
Improve tracing in peerset
debris Apr 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions core/network-libp2p/src/behaviour.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ impl<TMessage, TSubstream> NetworkBehaviourEventProcess<IdentifyEvent> for Behav
for addr in &info.listen_addrs {
self.discovery.kademlia.add_connected_address(&peer_id, addr.clone());
}
self.custom_protocols.add_discovered_node(&peer_id);
self.custom_protocols.add_discovered_nodes(Some(peer_id.clone()));
self.events.push(BehaviourOut::Identified { peer_id, info });
}
IdentifyEvent::Error { .. } => {}
Expand All @@ -272,7 +272,7 @@ impl<TMessage, TSubstream> NetworkBehaviourEventProcess<KademliaOut> for Behavio
match out {
KademliaOut::Discovered { .. } => {}
KademliaOut::KBucketAdded { peer_id, .. } => {
self.custom_protocols.add_discovered_node(&peer_id);
self.custom_protocols.add_discovered_nodes(Some(peer_id));
}
KademliaOut::FindNodeResult { key, closer_peers } => {
trace!(target: "sub-libp2p", "Libp2p => Query for {:?} yielded {:?} results",
Expand Down Expand Up @@ -303,9 +303,7 @@ impl<TMessage, TSubstream> NetworkBehaviourEventProcess<MdnsEvent> for Behaviour
fn inject_event(&mut self, event: MdnsEvent) {
match event {
MdnsEvent::Discovered(list) => {
for (peer_id, _) in list {
self.custom_protocols.add_discovered_node(&peer_id);
}
self.custom_protocols.add_discovered_nodes(list.into_iter().map(|(peer_id, _)| peer_id));
},
MdnsEvent::Expired(_) => {}
}
Expand Down
5 changes: 2 additions & 3 deletions core/network-libp2p/src/custom_proto/behaviour.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,8 @@ impl<TMessage, TSubstream> CustomProto<TMessage, TSubstream> {
}

/// Indicates to the peerset that we have discovered new addresses for a given node.
pub fn add_discovered_node(&mut self, peer_id: &PeerId) {
debug!(target: "sub-libp2p", "PSM <= Discovered({:?})", peer_id);

@debris debris Apr 4, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not sure what to do about this debug!. I could move print it in Peerset::discovered if needed

@tomaka tomaka Apr 4, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it would be nice to have the logging both in sub-libp2p and in the peerset.
I guess we could have a custom iterator that does the same as Iterator::map but also implements Drop and prints the elements that are ignored?

@debris debris Apr 4, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for suggestion. I realised that we can just print all items inside .map combinator. I fixed it in the latest commit

self.peerset.discovered(peer_id.clone())
pub fn add_discovered_nodes<I: IntoIterator<Item = PeerId>>(&mut self, peer_ids: I) {
self.peerset.discovered(peer_ids);
}

/// Returns the state of the peerset manager, for debugging purposes.
Expand Down
17 changes: 9 additions & 8 deletions core/peerset/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,16 +434,17 @@ impl Peerset {
self.alloc_slots();
}

/// Adds a discovered peer id to the PSM.
/// Adds discovered peer ids to the PSM.
///
/// > **Note**: There is no equivalent "expired" message, meaning that it is the responsibility
/// > of the PSM to remove `PeerId`s that fail to dial too often.
pub fn discovered(&mut self, peer_id: PeerId) {
if self.data.in_slots.contains(&peer_id) || self.data.out_slots.contains(&peer_id) {
return;
pub fn discovered<I: IntoIterator<Item = PeerId>>(&mut self, peer_ids: I) {
for peer_id in peer_ids {
if !self.data.in_slots.contains(&peer_id) && !self.data.out_slots.contains(&peer_id) {
self.data.discovered.add_peer(peer_id, SlotType::Common);
}
}

self.data.discovered.add_peer(peer_id, SlotType::Common);
self.alloc_slots();
}

Expand Down Expand Up @@ -721,9 +722,9 @@ mod tests {
};

let (mut peerset, _handle) = Peerset::from_config(config);
peerset.discovered(discovered.clone());
peerset.discovered(discovered.clone());
peerset.discovered(discovered2);
peerset.discovered(Some(discovered.clone()));
peerset.discovered(Some(discovered.clone()));
peerset.discovered(Some(discovered2));

assert_messages(peerset, vec![
Message::Connect(bootnode),
Expand Down