Skip to content

Commit

Permalink
Replace Multiaddr & related types with substrate-specific types (#4198
Browse files Browse the repository at this point in the history
)

This PR introduces custom types / substrate wrappers for `Multiaddr`,
`multiaddr::Protocol`, `Multihash`, `ed25519::*` and supplementary types
like errors and iterators.

This is needed to unblock `libp2p` upgrade PR
#1631 after
#2944 was merged.
`libp2p` and `litep2p` currently depend on different versions of
`multiaddr` crate, and introduction of this "common ground" types is
needed to support independent version upgrades of `multiaddr` and
dependent crates in `libp2p` & `litep2p`.

While being just convenient to not tie versions of `libp2p` & `litep2p`
dependencies together, it's currently not even possible to keep `libp2p`
& `litep2p` dependencies updated to the same versions as `multiaddr` in
`libp2p` depends on `libp2p-identity` that we can't include as a
dependency of `litep2p`, which has it's own `PeerId` type. In the
future, to keep things updated on `litep2p` side, we will likely need to
fork `multiaddr` and make it use `litep2p` `PeerId` as a payload of
`/p2p/...` protocol.

With these changes, common code in substrate uses these custom types,
and `litep2p` & `libp2p` backends use corresponding libraries types.
  • Loading branch information
dmitry-markin committed May 21, 2024
1 parent e0e1f2d commit d05786f
Show file tree
Hide file tree
Showing 33 changed files with 1,343 additions and 147 deletions.
15 changes: 9 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 31 additions & 0 deletions prdoc/pr_4198.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# 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: Replace `Multiaddr` & related types with substrate-specific types

doc:
- audience: Node Dev
description: |
Introduce custom types / substrate wrappers for `Multiaddr`, `multiaddr::Protocol`,
`Multihash`, `ed25519::*` and supplementary types like errors and iterators.

Common code in substrate uses these custom types, while `libp2p` & `litep2p` network
backends use their corresponding libraries types.

This is needed to independently upgrade `libp2p` & `litep2p` dependencies.

crates:
- name: sc-network-types
bump: minor
- name: sc-network
bump: minor
- name: sc-network-sync
bump: minor
- name: sc-authority-discovery
bump: minor
- name: sc-cli
bump: patch
- name: sc-mixnet
bump: patch
- name: sc-telemetry
bump: patch
4 changes: 2 additions & 2 deletions substrate/client/authority-discovery/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub enum Error {
VerifyingDhtPayload,

#[error("Failed to hash the authority id to be used as a dht key.")]
HashingAuthorityId(#[from] sc_network::multiaddr::multihash::Error),
HashingAuthorityId(#[from] sc_network_types::multihash::Error),

#[error("Failed calling into the Substrate runtime: {0}")]
CallingRuntime(#[from] sp_blockchain::Error),
Expand All @@ -53,7 +53,7 @@ pub enum Error {
EncodingDecodingScale(#[from] codec::Error),

#[error("Failed to parse a libp2p multi address.")]
ParsingMultiaddress(#[from] sc_network::multiaddr::Error),
ParsingMultiaddress(#[from] sc_network::multiaddr::ParseError),

#[error("Failed to parse a libp2p key: {0}")]
ParsingLibp2pIdentity(String),
Expand Down
6 changes: 4 additions & 2 deletions substrate/client/authority-discovery/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ use addr_cache::AddrCache;
use codec::{Decode, Encode};
use ip_network::IpNetwork;
use linked_hash_set::LinkedHashSet;
use multihash::{Code, Multihash, MultihashDigest};

use log::{debug, error, log_enabled};
use prometheus_endpoint::{register, Counter, CounterVec, Gauge, Opts, U64};
Expand All @@ -46,7 +45,10 @@ use sc_network::{
event::DhtEvent, multiaddr, KademliaKey, Multiaddr, NetworkDHTProvider, NetworkSigner,
NetworkStateInfo,
};
use sc_network_types::PeerId;
use sc_network_types::{
multihash::{Code, Multihash},
PeerId,
};
use sp_api::{ApiError, ProvideRuntimeApi};
use sp_authority_discovery::{
AuthorityDiscoveryApi, AuthorityId, AuthorityPair, AuthoritySignature,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ fn addresses_to_peer_ids(addresses: &HashSet<Multiaddr>) -> HashSet<PeerId> {
mod tests {
use super::*;

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

use sp_authority_discovery::{AuthorityId, AuthorityPair};
use sp_core::crypto::Pair;
Expand Down
10 changes: 7 additions & 3 deletions substrate/client/authority-discovery/src/worker/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,15 @@ use futures::{
sink::SinkExt,
task::LocalSpawn,
};
use libp2p::{core::multiaddr, identity::SigningError, kad::record::Key as KademliaKey, PeerId};
use libp2p::{identity::SigningError, kad::record::Key as KademliaKey};
use prometheus_endpoint::prometheus::default_registry;

use sc_client_api::HeaderBackend;
use sc_network::{service::signature::Keypair, Signature};
use sc_network_types::{
multiaddr::{Multiaddr, Protocol},
PeerId,
};
use sp_api::{ApiRef, ProvideRuntimeApi};
use sp_keystore::{testing::MemoryKeystore, Keystore};
use sp_runtime::traits::{Block as BlockT, NumberFor, Zero};
Expand Down Expand Up @@ -168,7 +172,7 @@ impl NetworkSigner for TestNetwork {
let public_key = libp2p::identity::PublicKey::try_decode_protobuf(&public_key)
.map_err(|error| error.to_string())?;
let peer_id: PeerId = peer_id.into();
let remote: libp2p::PeerId = public_key.to_peer_id();
let remote: PeerId = public_key.to_peer_id().into();

Ok(peer_id == remote && public_key.verify(message, signature))
}
Expand Down Expand Up @@ -435,7 +439,7 @@ fn dont_stop_polling_dht_event_stream_after_bogus_event() {
let peer_id = PeerId::random();
let address: Multiaddr = "/ip6/2001:db8:0:0:0:0:0:1/tcp/30333".parse().unwrap();

address.with(multiaddr::Protocol::P2p(peer_id.into()))
address.with(Protocol::P2p(peer_id.into()))
};
let remote_key_store = MemoryKeystore::new();
let remote_public_key: AuthorityId = remote_key_store
Expand Down
10 changes: 3 additions & 7 deletions substrate/client/cli/src/params/node_key_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use clap::Args;
use sc_network::config::{identity::ed25519, NodeKeyConfig};
use sc_network::config::{ed25519, NodeKeyConfig};
use sc_service::Role;
use sp_core::H256;
use std::{path::PathBuf, str::FromStr};
Expand Down Expand Up @@ -148,7 +148,7 @@ fn parse_ed25519_secret(hex: &str) -> error::Result<sc_network::config::Ed25519S
mod tests {
use super::*;
use clap::ValueEnum;
use libp2p_identity::ed25519;
use sc_network::config::ed25519;
use std::fs::{self, File};
use tempfile::TempDir;

Expand Down Expand Up @@ -194,11 +194,7 @@ mod tests {
.into_keypair()
.expect("Creates node key pair");

if let Ok(pair) = node_key.try_into_ed25519() {
if pair.secret().as_ref() != key.as_ref() {
panic!("Invalid key")
}
} else {
if node_key.secret().as_ref() != key.as_ref() {
panic!("Invalid key")
}
}
Expand Down
7 changes: 4 additions & 3 deletions substrate/client/mixnet/src/sync_with_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ use mixnet::core::{
Mixnet, Mixnode as CoreMixnode, MixnodesErr as CoreMixnodesErr, RelSessionIndex,
SessionPhase as CoreSessionPhase, SessionStatus as CoreSessionStatus,
};
use multiaddr::{multiaddr, Multiaddr, Protocol};
use sc_network_types::PeerId;
use sc_network_types::{
multiaddr::{multiaddr, Multiaddr, Protocol},
PeerId,
};
use sp_api::{ApiError, ApiRef};
use sp_mixnet::{
runtime_api::MixnetApi,
Expand Down Expand Up @@ -196,7 +198,6 @@ where
#[cfg(test)]
mod tests {
use super::*;
use multiaddr::multiaddr;

#[test]
fn fixup_empty_external_addresses() {
Expand Down
38 changes: 15 additions & 23 deletions substrate/client/network/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,11 @@ pub use crate::{
types::ProtocolName,
};

pub use libp2p::{
build_multiaddr,
identity::{self, ed25519, Keypair},
multiaddr, Multiaddr,
pub use sc_network_types::{build_multiaddr, ed25519};
use sc_network_types::{
multiaddr::{self, Multiaddr},
PeerId,
};
use sc_network_types::PeerId;

use crate::service::{ensure_addresses_consistent_with_transport, traits::NetworkBackend};
use codec::Encode;
Expand Down Expand Up @@ -100,7 +99,7 @@ impl fmt::Debug for ProtocolId {
/// # Example
///
/// ```
/// # use libp2p::{Multiaddr, PeerId};
/// # use sc_network_types::{multiaddr::Multiaddr, PeerId};
/// use sc_network::config::parse_str_addr;
/// let (peer_id, addr) = parse_str_addr(
/// "/ip4/198.51.100.19/tcp/30333/p2p/QmSk5HQbn6LhUwDiNMseVUjuRYhEtYj4aUZ6WfWoGURpdV"
Expand Down Expand Up @@ -131,7 +130,7 @@ pub fn parse_addr(mut addr: Multiaddr) -> Result<(PeerId, Multiaddr), ParseErr>
/// # Example
///
/// ```
/// # use libp2p::{Multiaddr, PeerId};
/// # use sc_network_types::{multiaddr::Multiaddr, PeerId};
/// use sc_network::config::MultiaddrWithPeerId;
/// let addr: MultiaddrWithPeerId =
/// "/ip4/198.51.100.19/tcp/30333/p2p/QmSk5HQbn6LhUwDiNMseVUjuRYhEtYj4aUZ6WfWoGURpdV".parse().unwrap();
Expand Down Expand Up @@ -187,7 +186,7 @@ impl TryFrom<String> for MultiaddrWithPeerId {
#[derive(Debug)]
pub enum ParseErr {
/// Error while parsing the multiaddress.
MultiaddrParse(multiaddr::Error),
MultiaddrParse(multiaddr::ParseError),
/// Multihash of the peer ID is invalid.
InvalidPeerId,
/// The peer ID is missing from the address.
Expand All @@ -214,8 +213,8 @@ impl std::error::Error for ParseErr {
}
}

impl From<multiaddr::Error> for ParseErr {
fn from(err: multiaddr::Error) -> ParseErr {
impl From<multiaddr::ParseError> for ParseErr {
fn from(err: multiaddr::ParseError) -> ParseErr {
Self::MultiaddrParse(err)
}
}
Expand Down Expand Up @@ -343,10 +342,10 @@ impl NodeKeyConfig {
///
/// * If the secret is configured to be new, it is generated and the corresponding keypair is
/// returned.
pub fn into_keypair(self) -> io::Result<Keypair> {
pub fn into_keypair(self) -> io::Result<ed25519::Keypair> {
use NodeKeyConfig::*;
match self {
Ed25519(Secret::New) => Ok(Keypair::generate_ed25519()),
Ed25519(Secret::New) => Ok(ed25519::Keypair::generate()),

Ed25519(Secret::Input(k)) => Ok(ed25519::Keypair::from(k).into()),

Expand All @@ -365,8 +364,7 @@ impl NodeKeyConfig {
ed25519::SecretKey::generate,
|b| b.as_ref().to_vec(),
)
.map(ed25519::Keypair::from)
.map(Keypair::from),
.map(ed25519::Keypair::from),
}
}
}
Expand Down Expand Up @@ -887,7 +885,7 @@ impl<B: BlockT + 'static, H: ExHashT, N: NetworkBackend<B, H>> FullNetworkConfig
.find(|o| o.peer_id != bootnode.peer_id)
{
Err(crate::error::Error::DuplicateBootnode {
address: bootnode.multiaddr.clone(),
address: bootnode.multiaddr.clone().into(),
first_id: bootnode.peer_id.into(),
second_id: other.peer_id.into(),
})
Expand Down Expand Up @@ -947,14 +945,8 @@ mod tests {
tempfile::Builder::new().prefix(prefix).tempdir().unwrap()
}

fn secret_bytes(kp: Keypair) -> Vec<u8> {
kp.try_into_ed25519()
.expect("ed25519 keypair")
.secret()
.as_ref()
.iter()
.cloned()
.collect()
fn secret_bytes(kp: ed25519::Keypair) -> Vec<u8> {
kp.secret().to_bytes().into()
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion substrate/client/network/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

use crate::{config::TransportConfig, types::ProtocolName};

use libp2p::{Multiaddr, PeerId};
use sc_network_types::{multiaddr::Multiaddr, PeerId};

use std::fmt;

Expand Down
6 changes: 5 additions & 1 deletion substrate/client/network/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ pub use sc_network_common::{
role::{ObservedRole, Roles},
types::ReputationChange,
};
pub use sc_network_types::{
multiaddr::{self, Multiaddr},
PeerId,
};
pub use service::{
metrics::NotificationMetrics,
signature::Signature,
Expand All @@ -285,7 +289,7 @@ pub use service::{
DecodingError, Keypair, NetworkService, NetworkWorker, NotificationSender, OutboundFailure,
PublicKey,
};
pub use types::{multiaddr, Multiaddr, PeerId, ProtocolName};
pub use types::ProtocolName;

/// The maximum allowed number of established connections per peer.
///
Expand Down
7 changes: 3 additions & 4 deletions substrate/client/network/src/litep2p/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@

use crate::{
config::{NetworkConfiguration, ProtocolId},
multiaddr::Protocol,
peer_store::PeerStoreProvider,
Multiaddr,
};

use array_bytes::bytes2hex;
Expand All @@ -42,6 +40,7 @@ use litep2p::{
},
mdns::{Config as MdnsConfig, MdnsEvent},
},
types::multiaddr::{Multiaddr, Protocol},
PeerId, ProtocolName,
};
use parking_lot::RwLock;
Expand Down Expand Up @@ -227,7 +226,7 @@ impl Discovery {
let (identify_config, identify_event_stream) = IdentifyConfig::new(
"/substrate/1.0".to_string(),
Some(user_agent),
config.public_addresses.clone(),
config.public_addresses.clone().into_iter().map(Into::into).collect(),
);

let (mdns_config, mdns_event_stream) = match config.transport {
Expand Down Expand Up @@ -266,7 +265,7 @@ impl Discovery {
duration_to_next_find_query: Duration::from_secs(1),
address_confirmations: LruMap::new(ByLength::new(8)),
allow_non_global_addresses: config.allow_non_globals_in_dht,
public_addresses: config.public_addresses.iter().cloned().collect(),
public_addresses: config.public_addresses.iter().cloned().map(Into::into).collect(),
next_kad_query: Some(Delay::new(KADEMLIA_QUERY_INTERVAL)),
local_protocols: HashSet::from_iter([kademlia_protocol_name(
genesis_hash,
Expand Down
Loading

0 comments on commit d05786f

Please sign in to comment.