Skip to content

Commit

Permalink
Upgrade libp2p to 0.52.4 (paritytech#1631)
Browse files Browse the repository at this point in the history
Upgrade libp2p to 0.52.4, including a fix: 

* Set Kademlia to server mode
(paritytech/substrate#14703)

### TODO
- [x] Fix 3 zombienet tests failing:
  - [x] `zombienet-substrate-0002-validators-warp-sync`
- [ ]
~`zombienet-polkadot-functional-0005-parachains-disputes-past-session`~
The test is also flaky in other PRs and is not required for CI to
succeed.
  - [x] `zombienet-polkadot-functional-0009-approval-voting-coalescing`
- [x] Uncomment and update to the actual libp2p API tests in
[`substrate/client/network/src/protocol/notifications/handler.rs`](https://github.com/paritytech/polkadot-sdk/blob/7331f1796f1a0b0e9fb0cd7bf441239ad9664595/substrate/client/network/src/protocol/notifications/handler.rs#L1009).
- [x] When upgrading `multihash` crate as part of libp2p upgrade to
version v0.19.1, uncomment the conversion code at
https://github.com/paritytech/polkadot-sdk/blob/7547c4942a887029c11cbcfd5103f6d8315ab95c/substrate/client/network/types/src/multihash.rs#L159
- [x] Perform a burn-in.

---------

Co-authored-by: Anton <anton.kalyaev@gmail.com>
Co-authored-by: command-bot <>
Co-authored-by: Dmitry Markin <dmitry@markin.tech>
Co-authored-by: Bastian Köcher <git@kchr.de>
  • Loading branch information
4 people authored Jun 25, 2024
1 parent 3c21372 commit 414a8fc
Show file tree
Hide file tree
Showing 35 changed files with 1,128 additions and 791 deletions.
766 changes: 429 additions & 337 deletions Cargo.lock

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -805,8 +805,8 @@ landlock = { version = "0.3.0" }
lazy_static = { version = "1.4.0" }
libc = { version = "0.2.153" }
libfuzzer-sys = { version = "0.4" }
libp2p = { version = "0.51.4" }
libp2p-identity = { version = "0.1.3" }
libp2p = { version = "0.52.4" }
libp2p-identity = { version = "0.2.3" }
libsecp256k1 = { version = "0.7.0", default-features = false }
linked-hash-map = { version = "0.5.4" }
linked_hash_set = { version = "0.1.4" }
Expand All @@ -831,10 +831,10 @@ mmr-gadget = { path = "substrate/client/merkle-mountain-range", default-features
mmr-lib = { version = "0.5.2", package = "ckb-merkle-mountain-range" }
mmr-rpc = { path = "substrate/client/merkle-mountain-range/rpc", default-features = false }
mockall = { version = "0.11.3" }
multiaddr = { version = "0.17.1" }
multihash = { version = "0.17.0", default-features = false }
multiaddr = { version = "0.18.1" }
multihash = { version = "0.19.1", default-features = false }
multihash-codetable = { version = "0.1.1" }
multistream-select = { version = "0.12.1" }
multistream-select = { version = "0.13.0" }
names = { version = "0.14.0", default-features = false }
nix = { version = "0.28.0" }
node-cli = { path = "substrate/bin/node/cli", package = "staging-node-cli" }
Expand Down
39 changes: 39 additions & 0 deletions prdoc/pr_1631.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Upgrade libp2p to 0.52.4

doc:
- audience: [Node Dev, Node Operator]
description: |
Upgrade libp2p from 0.51.4 to 0.52.4

crates:
- name: sc-authority-discovery
bump: minor
- name: sc-cli
bump: minor
- name: sc-mixnet
bump: minor
- name: sc-network
bump: minor
- name: sc-network-gossip
bump: minor
- name: sc-network-common
bump: minor
- name: sc-network-light
bump: minor
- name: sc-network-statement
bump: minor
- name: sc-network-sync
bump: minor
- name: sc-network-test
bump: minor
- name: sc-network-transactions
bump: minor
- name: sc-network-types
bump: minor
- name: sc-offchain
bump: major
- name: sc-telemetry
bump: major
7 changes: 1 addition & 6 deletions substrate/client/authority-discovery/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ futures = { workspace = true }
futures-timer = { workspace = true }
ip_network = { workspace = true }
libp2p = { features = ["ed25519", "kad"], workspace = true }
multihash = { features = ["sha2", "std"], workspace = true }
multihash = { workspace = true }
linked_hash_set = { workspace = true }
log = { workspace = true, default-features = true }
prost = { workspace = true }
Expand All @@ -42,11 +42,6 @@ sp-core = { workspace = true, default-features = true }
sp-keystore = { workspace = true, default-features = true }
sp-runtime = { workspace = true, default-features = true }
async-trait = { workspace = true }
multihash-codetable = { features = [
"digest",
"serde",
"sha2",
], workspace = true }

[dev-dependencies]
quickcheck = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion substrate/client/authority-discovery/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl Service {
/// [`crate::Worker`] failed.
///
/// Note: [`Multiaddr`]s returned always include a [`PeerId`] via a
/// [`libp2p::core::multiaddr::Protocol::P2p`] component. Equality of
/// [`sc_network_types::multiaddr::Protocol::P2p`] component. Equality of
/// [`PeerId`]s across [`Multiaddr`]s returned by a single call is not
/// enforced today, given that there are still authorities out there
/// publishing the addresses of their sentry nodes on the DHT. In the future
Expand Down
12 changes: 4 additions & 8 deletions substrate/client/authority-discovery/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,7 @@ use sc_network::{
event::DhtEvent, multiaddr, KademliaKey, Multiaddr, NetworkDHTProvider, NetworkSigner,
NetworkStateInfo,
};
use sc_network_types::{
multihash::{Code, Multihash},
PeerId,
};
use sc_network_types::{multihash::Code, PeerId};
use schema::PeerSignature;
use sp_api::{ApiError, ProvideRuntimeApi};
use sp_authority_discovery::{
Expand Down Expand Up @@ -247,14 +244,14 @@ where
};

let public_addresses = {
let local_peer_id: Multihash = network.local_peer_id().into();
let local_peer_id = network.local_peer_id();

config
.public_addresses
.into_iter()
.map(|mut address| {
if let Some(multiaddr::Protocol::P2p(peer_id)) = address.iter().last() {
if peer_id != local_peer_id {
if peer_id != *local_peer_id.as_ref() {
error!(
target: LOG_TARGET,
"Discarding invalid local peer ID in public address {address}.",
Expand Down Expand Up @@ -401,10 +398,9 @@ where
);

// The address must include the local peer id.
let local_peer_id: Multihash = local_peer_id.into();
addresses
.into_iter()
.map(move |a| a.with(multiaddr::Protocol::P2p(local_peer_id)))
.map(move |a| a.with(multiaddr::Protocol::P2p(*local_peer_id.as_ref())))
}

/// Publish own public addresses.
Expand Down
16 changes: 7 additions & 9 deletions substrate/client/authority-discovery/src/worker/addr_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ mod tests {
use super::*;

use quickcheck::{Arbitrary, Gen, QuickCheck, TestResult};
use sc_network_types::multihash::Multihash;
use sc_network_types::multihash::{Code, Multihash};

use sp_authority_discovery::{AuthorityId, AuthorityPair};
use sp_core::crypto::Pair;
Expand All @@ -198,10 +198,9 @@ mod tests {
impl Arbitrary for TestMultiaddr {
fn arbitrary(g: &mut Gen) -> Self {
let seed = (0..32).map(|_| u8::arbitrary(g)).collect::<Vec<_>>();
let peer_id = PeerId::from_multihash(
Multihash::wrap(multihash::Code::Sha2_256.into(), &seed).unwrap(),
)
.unwrap();
let peer_id =
PeerId::from_multihash(Multihash::wrap(Code::Sha2_256.into(), &seed).unwrap())
.unwrap();
let multiaddr = "/ip6/2001:db8:0:0:0:0:0:2/tcp/30333"
.parse::<Multiaddr>()
.unwrap()
Expand All @@ -217,10 +216,9 @@ mod tests {
impl Arbitrary for TestMultiaddrsSamePeerCombo {
fn arbitrary(g: &mut Gen) -> Self {
let seed = (0..32).map(|_| u8::arbitrary(g)).collect::<Vec<_>>();
let peer_id = PeerId::from_multihash(
Multihash::wrap(multihash::Code::Sha2_256.into(), &seed).unwrap(),
)
.unwrap();
let peer_id =
PeerId::from_multihash(Multihash::wrap(Code::Sha2_256.into(), &seed).unwrap())
.unwrap();
let multiaddr1 = "/ip6/2001:db8:0:0:0:0:0:2/tcp/30333"
.parse::<Multiaddr>()
.unwrap()
Expand Down
1 change: 0 additions & 1 deletion substrate/client/network-gossip/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ targets = ["x86_64-unknown-linux-gnu"]
ahash = { workspace = true }
futures = { workspace = true }
futures-timer = { workspace = true }
libp2p = { workspace = true }
log = { workspace = true, default-features = true }
schnellru = { workspace = true }
tracing = { workspace = true, default-features = true }
Expand Down
3 changes: 2 additions & 1 deletion substrate/client/network-gossip/src/bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ impl<B: BlockT> futures::future::FusedFuture for GossipEngine<B> {
#[cfg(test)]
mod tests {
use super::*;
use crate::{multiaddr::Multiaddr, ValidationResult, ValidatorContext};
use crate::{ValidationResult, ValidatorContext};
use codec::{DecodeAll, Encode};
use futures::{
channel::mpsc::{unbounded, UnboundedReceiver, UnboundedSender},
Expand All @@ -363,6 +363,7 @@ mod tests {
};
use sc_network_common::role::ObservedRole;
use sc_network_sync::SyncEventStream;
use sc_network_types::multiaddr::Multiaddr;
use sp_runtime::{
testing::H256,
traits::{Block as BlockT, NumberFor},
Expand Down
10 changes: 6 additions & 4 deletions substrate/client/network-gossip/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,12 @@ pub use self::{
validator::{DiscardAll, MessageIntent, ValidationResult, Validator, ValidatorContext},
};

use sc_network::{multiaddr, types::ProtocolName, NetworkBlock, NetworkEventStream, NetworkPeers};
use sc_network::{types::ProtocolName, NetworkBlock, NetworkEventStream, NetworkPeers};
use sc_network_sync::SyncEventStream;
use sc_network_types::PeerId;
use sc_network_types::{
multiaddr::{Multiaddr, Protocol},
PeerId,
};
use sp_runtime::traits::{Block as BlockT, NumberFor};
use std::iter;

Expand All @@ -80,8 +83,7 @@ mod validator;
/// Abstraction over a network.
pub trait Network<B: BlockT>: NetworkPeers + NetworkEventStream {
fn add_set_reserved(&self, who: PeerId, protocol: ProtocolName) {
let addr =
iter::once(multiaddr::Protocol::P2p(who.into())).collect::<multiaddr::Multiaddr>();
let addr = Multiaddr::empty().with(Protocol::P2p(*who.as_ref()));
let result = self.add_peers_to_reserved_set(protocol, iter::once(addr).collect());
if let Err(err) = result {
log::error!(target: "gossip", "add_set_reserved failed: {}", err);
Expand Down
2 changes: 1 addition & 1 deletion substrate/client/network-gossip/src/state_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,12 +542,12 @@ impl Metrics {
#[cfg(test)]
mod tests {
use super::*;
use crate::multiaddr::Multiaddr;
use futures::prelude::*;
use sc_network::{
config::MultiaddrWithPeerId, event::Event, service::traits::NotificationEvent, MessageSink,
NetworkBlock, NetworkEventStream, NetworkPeers, ReputationChange,
};
use sc_network_types::multiaddr::Multiaddr;
use sp_runtime::{
testing::{Block as RawBlock, ExtrinsicWrapper, H256},
traits::NumberFor,
Expand Down
18 changes: 14 additions & 4 deletions substrate/client/network/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ use crate::{

use futures::channel::oneshot;
use libp2p::{
core::Multiaddr, identify::Info as IdentifyInfo, identity::PublicKey, kad::RecordKey,
swarm::NetworkBehaviour, PeerId,
connection_limits::ConnectionLimits, core::Multiaddr, identify::Info as IdentifyInfo,
identity::PublicKey, kad::RecordKey, swarm::NetworkBehaviour, PeerId, StreamProtocol,
};

use parking_lot::Mutex;
Expand All @@ -47,8 +47,10 @@ pub use crate::request_responses::{InboundFailure, OutboundFailure, ResponseFail

/// General behaviour of the network. Combines all protocols together.
#[derive(NetworkBehaviour)]
#[behaviour(out_event = "BehaviourOut")]
#[behaviour(to_swarm = "BehaviourOut")]
pub struct Behaviour<B: BlockT> {
/// Connection limits.
connection_limits: libp2p::connection_limits::Behaviour,
/// All the substrate-specific protocols.
substrate: Protocol<B>,
/// Periodically pings and identifies the nodes we are connected to, and store information in a
Expand Down Expand Up @@ -180,6 +182,7 @@ impl<B: BlockT> Behaviour<B> {
request_response_protocols: Vec<ProtocolConfig>,
peer_store_handle: Arc<dyn PeerStoreProvider>,
external_addresses: Arc<Mutex<HashSet<Multiaddr>>>,
connection_limits: ConnectionLimits,
) -> Result<Self, request_responses::RegisterError> {
Ok(Self {
substrate,
Expand All @@ -193,6 +196,7 @@ impl<B: BlockT> Behaviour<B> {
request_response_protocols.into_iter(),
peer_store_handle,
)?,
connection_limits: libp2p::connection_limits::Behaviour::new(connection_limits),
})
}

Expand Down Expand Up @@ -267,7 +271,7 @@ impl<B: BlockT> Behaviour<B> {
pub fn add_self_reported_address_to_dht(
&mut self,
peer_id: &PeerId,
supported_protocols: &[impl AsRef<[u8]>],
supported_protocols: &[StreamProtocol],
addr: Multiaddr,
) {
self.discovery.add_self_reported_address(peer_id, supported_protocols, addr);
Expand Down Expand Up @@ -376,3 +380,9 @@ impl From<DiscoveryOut> for BehaviourOut {
}
}
}

impl From<void::Void> for BehaviourOut {
fn from(e: void::Void) -> Self {
void::unreachable(e)
}
}
8 changes: 4 additions & 4 deletions substrate/client/network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,13 @@ pub fn parse_str_addr(addr_str: &str) -> Result<(PeerId, Multiaddr), ParseErr> {

/// Splits a Multiaddress into a Multiaddress and PeerId.
pub fn parse_addr(mut addr: Multiaddr) -> Result<(PeerId, Multiaddr), ParseErr> {
let who = match addr.pop() {
Some(multiaddr::Protocol::P2p(key)) =>
PeerId::from_multihash(key).map_err(|_| ParseErr::InvalidPeerId)?,
let multihash = match addr.pop() {
Some(multiaddr::Protocol::P2p(multihash)) => multihash,
_ => return Err(ParseErr::PeerIdMissing),
};
let peer_id = PeerId::from_multihash(multihash).map_err(|_| ParseErr::InvalidPeerId)?;

Ok((who, addr))
Ok((peer_id, addr))
}

/// Address of a node, including its identity.
Expand Down
Loading

0 comments on commit 414a8fc

Please sign in to comment.