This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
*: Update to libp2p v0.21.1 #6559
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
e6d0926
*Cargo.toml: Update versions
mxinden 1956d86
client/network/src/discovery: Adjust to Kademlia API changes
mxinden 1e2cc6b
client/network: Adjust to one_shot.rs changes
mxinden d9373b1
client/network/discovery: Log address list on trace level
mxinden 6ff2637
client/network/discovery: Ignore RoutablePeer and PendingRoutablePeer
mxinden a19dec6
Merge branch 'paritytech/master' into libp2p-0.21
mxinden c54796b
Commit Cargo.lock
tomaka 6de636a
Finish update
tomaka ef4ae28
Merge remote-tracking branch 'upstream/master' into HEAD
tomaka File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,7 +108,7 @@ impl DiscoveryConfig { | |
{ | ||
for (peer_id, addr) in user_defined { | ||
for kad in self.kademlias.values_mut() { | ||
kad.add_address(&peer_id, addr.clone()) | ||
kad.add_address(&peer_id, addr.clone()); | ||
} | ||
self.user_defined.push((peer_id, addr)) | ||
} | ||
|
@@ -230,12 +230,18 @@ pub struct DiscoveryBehaviour { | |
|
||
impl DiscoveryBehaviour { | ||
/// Returns the list of nodes that we know exist in the network. | ||
pub fn known_peers(&mut self) -> impl Iterator<Item = &PeerId> { | ||
let mut set = HashSet::new(); | ||
for p in self.kademlias.values_mut().map(|k| k.kbuckets_entries()).flatten() { | ||
set.insert(p); | ||
pub fn known_peers(&mut self) -> HashSet<PeerId> { | ||
let mut peers = HashSet::new(); | ||
for k in self.kademlias.values_mut() { | ||
for b in k.kbuckets() { | ||
for e in b.iter() { | ||
if !peers.contains(e.node.key.preimage()) { | ||
peers.insert(e.node.key.preimage().clone()); | ||
} | ||
} | ||
} | ||
} | ||
set.into_iter() | ||
peers | ||
} | ||
|
||
/// Adds a hard-coded address for the given peer, that never expires. | ||
|
@@ -246,7 +252,7 @@ impl DiscoveryBehaviour { | |
pub fn add_known_address(&mut self, peer_id: PeerId, addr: Multiaddr) { | ||
if self.user_defined.iter().all(|(p, a)| *p != peer_id && *a != addr) { | ||
for k in self.kademlias.values_mut() { | ||
k.add_address(&peer_id, addr.clone()) | ||
k.add_address(&peer_id, addr.clone()); | ||
} | ||
self.pending_events.push_back(DiscoveryOut::Discovered(peer_id.clone())); | ||
self.user_defined.push((peer_id, addr)); | ||
|
@@ -260,7 +266,7 @@ impl DiscoveryBehaviour { | |
pub fn add_self_reported_address(&mut self, peer_id: &PeerId, addr: Multiaddr) { | ||
if self.allow_non_globals_in_dht || self.can_add_to_dht(&addr) { | ||
for k in self.kademlias.values_mut() { | ||
k.add_address(peer_id, addr.clone()) | ||
k.add_address(peer_id, addr.clone()); | ||
} | ||
} else { | ||
log::trace!(target: "sub-libp2p", "Ignoring self-reported address {} from {}", addr, peer_id); | ||
|
@@ -291,7 +297,8 @@ impl DiscoveryBehaviour { | |
|
||
/// Returns the number of nodes that are in the Kademlia k-buckets. | ||
pub fn num_kbuckets_entries(&mut self) -> impl ExactSizeIterator<Item = (&ProtocolId, usize)> { | ||
self.kademlias.iter_mut().map(|(id, kad)| (id, kad.kbuckets_entries().count())) | ||
self.kademlias.iter_mut() | ||
.map(|(id, kad)| (id, kad.kbuckets().map(|bucket| bucket.iter().count()).sum())) | ||
} | ||
|
||
/// Returns the number of records in the Kademlia record stores. | ||
|
@@ -407,23 +414,7 @@ impl NetworkBehaviour for DiscoveryBehaviour { | |
list.extend(list_to_filter); | ||
} | ||
|
||
if !list.is_empty() { | ||
trace!(target: "sub-libp2p", "Addresses of {:?}: {:?}", peer_id, list); | ||
|
||
} else { | ||
let mut has_entry = false; | ||
for k in self.kademlias.values_mut() { | ||
if k.kbuckets_entries().any(|p| p == peer_id) { | ||
has_entry = true; | ||
break | ||
} | ||
} | ||
if has_entry { | ||
trace!(target: "sub-libp2p", "Addresses of {:?}: none (peer in k-buckets)", peer_id); | ||
} else { | ||
trace!(target: "sub-libp2p", "Addresses of {:?}: none (peer not in k-buckets)", peer_id); | ||
} | ||
} | ||
trace!(target: "sub-libp2p", "Addresses of {:?}: {:?}", peer_id, list); | ||
|
||
list | ||
} | ||
|
@@ -570,13 +561,16 @@ impl NetworkBehaviour for DiscoveryBehaviour { | |
while let Poll::Ready(ev) = kademlia.poll(cx, params) { | ||
match ev { | ||
NetworkBehaviourAction::GenerateEvent(ev) => match ev { | ||
KademliaEvent::RoutingUpdated { peer, .. } => { | ||
let ev = DiscoveryOut::Discovered(peer); | ||
return Poll::Ready(NetworkBehaviourAction::GenerateEvent(ev)); | ||
} | ||
KademliaEvent::UnroutablePeer { peer, .. } => { | ||
let ev = DiscoveryOut::UnroutablePeer(peer); | ||
return Poll::Ready(NetworkBehaviourAction::GenerateEvent(ev)); | ||
} | ||
KademliaEvent::RoutingUpdated { peer, .. } => { | ||
let ev = DiscoveryOut::Discovered(peer); | ||
return Poll::Ready(NetworkBehaviourAction::GenerateEvent(ev)); | ||
KademliaEvent::RoutablePeer { .. } | KademliaEvent::PendingRoutablePeer { .. } => { | ||
// We are not interested in these events at the moment. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These events will be used within #6549. |
||
} | ||
KademliaEvent::QueryResult { result: QueryResult::GetClosestPeers(res), .. } => { | ||
match res { | ||
|
@@ -640,9 +634,6 @@ impl NetworkBehaviour for DiscoveryBehaviour { | |
e.key(), e) | ||
} | ||
} | ||
KademliaEvent::Discovered { .. } => { | ||
// We are not interested in these events at the moment. | ||
} | ||
// We never start any other type of query. | ||
e => { | ||
warn!(target: "sub-libp2p", "Libp2p => Unhandled Kademlia event: {:?}", e) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the motivation for the switch to
HashSet
here, instead of just switching fromto
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning a
HashSet
implies that the collection returned by the method does not contain any duplication whereas anIterator
does not give such guarantee.I don't think it has any performance implications whether the method calls
into_iter
or the consumer callsinto_iter
.That said I am fine changing this to return an
Iterator
instead. What would you prefer @twittner?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to commit to set semantics feel free to change it. I was just curious if there was a specific reason for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the suggestion came from me, I am curious. I tend to think that when a function builds an owned collection and then only returns an
impl Iterator
, there is an unfortunate loss of structure and very often consuming code then eventuallycollect()
s again. But maybe if someone were to callknown_peers().collect::<HashSet<_>>()
withknown_peers
returnimpl Iterator
the compiler would optimise theHashSet::into_iter() -> collect::<HashSet<_>>
away?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume you are worried about performance, not so much about returning an existential type. I do not know though what — if any — compiler optimisations apply here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell
known_peers
is only used withinNetworkWorker::network_state
which specifically states:That said I am still curious and thus wrote a small benchmark. As far as I can tell Rustc or LLVM can not optimize the
HashSet::into_iter() -> collect::<HashSet<_>>
case. @romanb @twittner I would be curious if you can spot something I am missing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mxinden I don't think there are any such optimisations one could rely upon. It just seems that there are some such "in-place" optimisations for specific collections and iterator chains (see e.g. rust-lang/rust#70793).