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

*: Update to libp2p v0.21.1 #6559

Merged
9 commits merged into from
Jul 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
176 changes: 111 additions & 65 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion bin/node/browser-testing/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ license = "Apache-2.0"

[dependencies]
futures-timer = "3.0.2"
libp2p = { version = "0.20.1", default-features = false }
libp2p = { version = "0.21.1", default-features = false }
jsonrpc-core = "14.2.0"
serde = "1.0.106"
serde_json = "1.0.48"
Expand Down
2 changes: 1 addition & 1 deletion bin/utils/subkey/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ derive_more = { version = "0.99.2" }
sc-rpc = { version = "2.0.0-rc4", path = "../../../client/rpc" }
jsonrpc-core-client = { version = "14.2.0", features = ["http"] }
hyper = "0.12.35"
libp2p = { version = "0.20.1", default-features = false }
libp2p = { version = "0.21.1", default-features = false }
serde_json = "1.0"

[features]
Expand Down
2 changes: 1 addition & 1 deletion client/authority-discovery/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ codec = { package = "parity-scale-codec", default-features = false, version = "1
derive_more = "0.99.2"
futures = "0.3.4"
futures-timer = "3.0.1"
libp2p = { version = "0.20.1", default-features = false, features = ["kad"] }
libp2p = { version = "0.21.1", default-features = false, features = ["kad"] }
log = "0.4.8"
prometheus-endpoint = { package = "substrate-prometheus-endpoint", path = "../../utils/prometheus", version = "0.8.0-rc4"}
prost = "0.6.1"
Expand Down
4 changes: 2 additions & 2 deletions client/network-gossip/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ targets = ["x86_64-unknown-linux-gnu"]
[dependencies]
futures = "0.3.4"
futures-timer = "3.0.1"
libp2p = { version = "0.20.1", default-features = false }
libp2p = { version = "0.21.1", default-features = false }
log = "0.4.8"
lru = "0.4.3"
sc-network = { version = "0.8.0-rc4", path = "../network" }
sp-runtime = { version = "2.0.0-rc4", path = "../../primitives/runtime" }
wasm-timer = "0.2"

[dev-dependencies]
async-std = "1.5"
async-std = "1.6.2"
quickcheck = "0.9.0"
rand = "0.7.2"
substrate-test-runtime-client = { version = "2.0.0-rc4", path = "../../test-utils/runtime/client" }
6 changes: 3 additions & 3 deletions client/network/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,15 @@ wasm-timer = "0.2"
zeroize = "1.0.0"

[dependencies.libp2p]
version = "0.20.1"
version = "0.21.1"
default-features = false
features = ["identify", "kad", "mdns", "mplex", "noise", "ping", "tcp-async-std", "websocket", "yamux"]

[dev-dependencies]
async-std = "1.5"
async-std = "1.6.2"
assert_matches = "1.3"
env_logger = "0.7.0"
libp2p = { version = "0.20.1", default-features = false, features = ["secio"] }
libp2p = { version = "0.21.1", default-features = false, features = ["secio"] }
quickcheck = "0.9.0"
rand = "0.7.2"
sp-keyring = { version = "2.0.0-rc4", path = "../../primitives/keyring" }
Expand Down
10 changes: 8 additions & 2 deletions client/network/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@ use libp2p::swarm::{NetworkBehaviourAction, NetworkBehaviourEventProcess, PollPa
use log::debug;
use sp_consensus::{BlockOrigin, import_queue::{IncomingBlock, Origin}};
use sp_runtime::{traits::{Block as BlockT, NumberFor}, ConsensusEngineId, Justification};
use std::{borrow::Cow, collections::VecDeque, iter, task::{Context, Poll}, time::Duration};
use std::{
borrow::Cow,
collections::{HashSet, VecDeque},
iter,
task::{Context, Poll},
time::Duration,
};

/// General behaviour of the network. Combines all protocols together.
#[derive(NetworkBehaviour)]
Expand Down Expand Up @@ -124,7 +130,7 @@ impl<B: BlockT, H: ExHashT> Behaviour<B, H> {
}

/// Returns the list of nodes that we know exist in the network.
pub fn known_peers(&mut self) -> impl Iterator<Item = &PeerId> {
pub fn known_peers(&mut self) -> HashSet<PeerId> {
self.discovery.known_peers()
}

Expand Down
4 changes: 2 additions & 2 deletions client/network/src/block_requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,8 @@ where
marker: PhantomData,
};
let mut cfg = OneShotHandlerConfig::default();
cfg.inactive_timeout = self.config.inactivity_timeout;
cfg.substream_timeout = self.config.request_timeout;
cfg.keep_alive_timeout = self.config.inactivity_timeout;
cfg.outbound_substream_timeout = self.config.request_timeout;
OneShotHandler::new(SubstreamProtocol::new(p), cfg)
}

Expand Down
55 changes: 23 additions & 32 deletions client/network/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down Expand Up @@ -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> {
Copy link
Contributor

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 from

impl Iterator<Item = &PeerId>

to

impl Iterator<Item = PeerId>

?

Copy link
Contributor Author

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 an Iterator does not give such guarantee.

I don't think it has any performance implications whether the method calls into_iter or the consumer calls into_iter.

That said I am fine changing this to return an Iterator instead. What would you prefer @twittner?

Copy link
Contributor

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.

Copy link
Contributor

@romanb romanb Jul 2, 2020

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 eventually collect()s again. But maybe if someone were to call known_peers().collect::<HashSet<_>>() with known_peers return impl Iterator the compiler would optimise the HashSet::into_iter() -> collect::<HashSet<_>> away?

Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not know though what — if any — compiler optimisations apply here.

As far as I can tell known_peers is only used within NetworkWorker::network_state which specifically states:

Use this only for debugging.

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.

Copy link
Contributor

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).

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.
Expand All @@ -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));
Expand All @@ -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);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 {
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion client/network/src/finality_requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ where
marker: PhantomData,
};
let mut cfg = OneShotHandlerConfig::default();
cfg.inactive_timeout = self.config.inactivity_timeout;
cfg.keep_alive_timeout = self.config.inactivity_timeout;
OneShotHandler::new(SubstreamProtocol::new(p), cfg)
}

Expand Down
2 changes: 1 addition & 1 deletion client/network/src/light_client_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ where
protocol: self.config.light_protocol.clone(),
};
let mut cfg = OneShotHandlerConfig::default();
cfg.inactive_timeout = self.config.inactivity_timeout;
cfg.keep_alive_timeout = self.config.inactivity_timeout;
OneShotHandler::new(SubstreamProtocol::new(p), cfg)
}

Expand Down
21 changes: 11 additions & 10 deletions client/network/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,17 +490,18 @@ impl<B: BlockT + 'static, H: ExHashT> NetworkWorker<B, H> {

let not_connected_peers = {
let swarm = &mut *swarm;
let list = swarm.known_peers().filter(|p| open.iter().all(|n| n != *p))
.cloned().collect::<Vec<_>>();
list.into_iter().map(move |peer_id| {
(peer_id.to_base58(), NetworkStateNotConnectedPeer {
version_string: swarm.node(&peer_id)
.and_then(|i| i.client_version().map(|s| s.to_owned())),
latest_ping_time: swarm.node(&peer_id).and_then(|i| i.latest_ping()),
known_addresses: NetworkBehaviour::addresses_of_peer(&mut **swarm, &peer_id)
.into_iter().collect(),
swarm.known_peers().into_iter()
.filter(|p| open.iter().all(|n| n != p))
.map(move |peer_id| {
(peer_id.to_base58(), NetworkStateNotConnectedPeer {
version_string: swarm.node(&peer_id)
.and_then(|i| i.client_version().map(|s| s.to_owned())),
latest_ping_time: swarm.node(&peer_id).and_then(|i| i.latest_ping()),
known_addresses: NetworkBehaviour::addresses_of_peer(&mut **swarm, &peer_id)
.into_iter().collect(),
})
})
}).collect()
.collect()
};

NetworkState {
Expand Down
2 changes: 1 addition & 1 deletion client/network/test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ parking_lot = "0.10.0"
futures = "0.3.4"
futures-timer = "3.0.1"
rand = "0.7.2"
libp2p = { version = "0.20.1", default-features = false }
libp2p = { version = "0.21.1", default-features = false }
sp-consensus = { version = "0.8.0-rc4", path = "../../../primitives/consensus/common" }
sc-consensus = { version = "0.8.0-rc4", path = "../../../client/consensus/common" }
sc-client-api = { version = "2.0.0-rc4", path = "../../api" }
Expand Down
2 changes: 1 addition & 1 deletion client/peerset/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
futures = "0.3.4"
libp2p = { version = "0.20.1", default-features = false }
libp2p = { version = "0.21.1", default-features = false }
sp-utils = { version = "2.0.0-rc4", path = "../../primitives/utils"}
log = "0.4.8"
serde_json = "1.0.41"
Expand Down
2 changes: 1 addition & 1 deletion client/telemetry/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ parking_lot = "0.10.0"
futures = "0.3.4"
futures-timer = "3.0.1"
wasm-timer = "0.2.0"
libp2p = { version = "0.20.1", default-features = false, features = ["dns", "tcp-async-std", "wasm-ext", "websocket"] }
libp2p = { version = "0.21.1", default-features = false, features = ["dns", "tcp-async-std", "wasm-ext", "websocket"] }
log = "0.4.8"
pin-project = "0.4.6"
rand = "0.7.2"
Expand Down
2 changes: 1 addition & 1 deletion primitives/consensus/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
derive_more = "0.99.2"
libp2p = { version = "0.20.1", default-features = false }
libp2p = { version = "0.21.1", default-features = false }
log = "0.4.8"
sp-core = { path= "../../core", version = "2.0.0-rc4"}
sp-inherents = { version = "2.0.0-rc4", path = "../../inherents" }
Expand Down
2 changes: 1 addition & 1 deletion utils/browser/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ targets = ["x86_64-unknown-linux-gnu"]
futures = { version = "0.3", features = ["compat"] }
futures01 = { package = "futures", version = "0.1.29" }
log = "0.4.8"
libp2p-wasm-ext = { version = "0.19.0", features = ["websocket"] }
libp2p-wasm-ext = { version = "0.20", features = ["websocket"] }
console_error_panic_hook = "0.1.6"
console_log = "0.1.2"
js-sys = "0.3.34"
Expand Down
3 changes: 1 addition & 2 deletions utils/prometheus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ futures-util = { version = "0.3.1", default-features = false, features = ["io"]
derive_more = "0.99"

[target.'cfg(not(target_os = "unknown"))'.dependencies]
# async-std is temporarily pinned to <1.6 because version 1.6.0 is buggy
async-std = { version = "1.0.1, <1.6", features = ["unstable"] }
async-std = { version = "1.6.2", features = ["unstable"] }
hyper = { version = "0.13.1", default-features = false, features = ["stream"] }
tokio = "0.2"