Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Multiaddr & related types with substrate-specific types #4198

Merged
merged 26 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
87be8e5
Introduce network backend agnostic `Multiaddr` & `Multihash` types
dmitry-markin Apr 16, 2024
584038e
Extend substrate's `Multihash`, make substrate's `PeerId` use it
dmitry-markin Apr 19, 2024
8100f7f
Allow constructing `Multiaddr` from iterator of protocols
dmitry-markin Apr 19, 2024
ea85860
Convert network config to use substrate's `Multiaddr`
dmitry-markin Apr 19, 2024
94af781
Add `ed25519` types to `sc-network-types`
dmitry-markin Apr 23, 2024
aa4d28d
cargo update litep2p
dmitry-markin Apr 23, 2024
da59b29
Make `Check licenses` check happy
dmitry-markin Apr 24, 2024
f614106
Make network config use substrate-specific types
dmitry-markin Apr 24, 2024
ce321a9
Convert substrate-specific types for libp2p backend
dmitry-markin Apr 24, 2024
a0373ae
Convert substrate-specific types for litep2p backend
dmitry-markin Apr 24, 2024
2bb5448
Cleaner separation between backend-specific types
dmitry-markin Apr 24, 2024
bd7df2e
Make authority-discovery use substrate-specific types
dmitry-markin Apr 24, 2024
7c29f74
Make telemetry use libp2p-only `Multiaddr`
dmitry-markin May 16, 2024
2162dac
Make `cli` use `sc_network::config::ed25519`
dmitry-markin May 16, 2024
bb6f58f
Make authority-discovery tests use proper types
dmitry-markin May 16, 2024
9fc3f80
Fix tests
dmitry-markin May 16, 2024
04a4096
Merge remote-tracking branch 'origin/master' into dm-network-types-mu…
dmitry-markin May 16, 2024
036aac7
Make clippy happy
dmitry-markin May 17, 2024
685de87
Add tests for ed25519 types conversion between libs
dmitry-markin May 17, 2024
dabbd96
Make clippy happy again
dmitry-markin May 17, 2024
a66016c
Add PRDoc
dmitry-markin May 17, 2024
41ffe22
Fix doctest
dmitry-markin May 17, 2024
086f389
Apply review suggestions
dmitry-markin May 20, 2024
7547c49
Update semver in PRDoc
dmitry-markin May 20, 2024
b1bea17
Fix CI
dmitry-markin May 20, 2024
a87a92d
Merge remote-tracking branch 'origin/master' into dm-network-types-mu…
dmitry-markin May 20, 2024
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
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.

29 changes: 29 additions & 0 deletions prdoc/pr_4198.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# 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-authority-discovery
bump: patch
- 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
34 changes: 13 additions & 21 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 @@ -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
Loading