diff --git a/substrate/client/finality-grandpa/src/communication/tests.rs b/substrate/client/finality-grandpa/src/communication/tests.rs index 5b2436b233..4d709f56f3 100644 --- a/substrate/client/finality-grandpa/src/communication/tests.rs +++ b/substrate/client/finality-grandpa/src/communication/tests.rs @@ -27,6 +27,7 @@ use futures::prelude::*; use parity_scale_codec::Encode; use sc_network::{config::Role, Multiaddr, PeerId, ReputationChange}; use sc_network_common::{ + config::MultiaddrWithPeerId, protocol::event::{Event as NetworkEvent, ObservedRole}, service::{ NetworkBlock, NetworkEventStream, NetworkNotification, NetworkPeers, @@ -87,7 +88,7 @@ impl NetworkPeers for TestNetwork { unimplemented!(); } - fn add_reserved_peer(&self, _peer: String) -> Result<(), String> { + fn add_reserved_peer(&self, _peer: MultiaddrWithPeerId) -> Result<(), String> { unimplemented!(); } diff --git a/substrate/client/network-gossip/src/bridge.rs b/substrate/client/network-gossip/src/bridge.rs index b63e3f64a1..45eff09332 100644 --- a/substrate/client/network-gossip/src/bridge.rs +++ b/substrate/client/network-gossip/src/bridge.rs @@ -317,6 +317,7 @@ mod tests { }; use quickcheck::{Arbitrary, Gen, QuickCheck}; use sc_network_common::{ + config::MultiaddrWithPeerId, protocol::event::ObservedRole, service::{ NetworkBlock, NetworkEventStream, NetworkNotification, NetworkPeers, @@ -371,7 +372,7 @@ mod tests { unimplemented!(); } - fn add_reserved_peer(&self, _peer: String) -> Result<(), String> { + fn add_reserved_peer(&self, _peer: MultiaddrWithPeerId) -> Result<(), String> { unimplemented!(); } diff --git a/substrate/client/network-gossip/src/state_machine.rs b/substrate/client/network-gossip/src/state_machine.rs index a870ca6a3a..ff75b4520b 100644 --- a/substrate/client/network-gossip/src/state_machine.rs +++ b/substrate/client/network-gossip/src/state_machine.rs @@ -514,6 +514,7 @@ mod tests { use crate::multiaddr::Multiaddr; use futures::prelude::*; use sc_network_common::{ + config::MultiaddrWithPeerId, protocol::event::Event, service::{ NetworkBlock, NetworkEventStream, NetworkNotification, NetworkPeers, @@ -610,7 +611,7 @@ mod tests { unimplemented!(); } - fn add_reserved_peer(&self, _peer: String) -> Result<(), String> { + fn add_reserved_peer(&self, _peer: MultiaddrWithPeerId) -> Result<(), String> { unimplemented!(); } diff --git a/substrate/client/network/common/src/service.rs b/substrate/client/network/common/src/service.rs index 6df99acf0b..9544cddf01 100644 --- a/substrate/client/network/common/src/service.rs +++ b/substrate/client/network/common/src/service.rs @@ -19,6 +19,7 @@ // If you read this, you are very thorough, congratulations. use crate::{ + config::MultiaddrWithPeerId, protocol::event::Event, request_responses::{IfDisconnected, RequestFailure}, sync::{warp::WarpSyncProgress, StateDownloadProgress, SyncState}, @@ -179,12 +180,11 @@ pub trait NetworkPeers { /// purposes. fn deny_unreserved_peers(&self); - /// Adds a `PeerId` and its address as reserved. The string should encode the address - /// and peer ID of the remote node. + /// Adds a `PeerId` and its `Multiaddr` as reserved. /// /// Returns an `Err` if the given string is not a valid multiaddress /// or contains an invalid peer ID (which includes the local peer ID). - fn add_reserved_peer(&self, peer: String) -> Result<(), String>; + fn add_reserved_peer(&self, peer: MultiaddrWithPeerId) -> Result<(), String>; /// Removes a `PeerId` from the list of reserved peers. fn remove_reserved_peer(&self, peer_id: PeerId); @@ -285,7 +285,7 @@ where T::deny_unreserved_peers(self) } - fn add_reserved_peer(&self, peer: String) -> Result<(), String> { + fn add_reserved_peer(&self, peer: MultiaddrWithPeerId) -> Result<(), String> { T::add_reserved_peer(self, peer) } diff --git a/substrate/client/network/src/service.rs b/substrate/client/network/src/service.rs index 162b9c6a30..68ac4b8db8 100644 --- a/substrate/client/network/src/service.rs +++ b/substrate/client/network/src/service.rs @@ -62,7 +62,7 @@ use parking_lot::Mutex; use sc_client_api::{BlockBackend, ProofProvider}; use sc_consensus::{BlockImportError, BlockImportStatus, ImportQueue, Link}; use sc_network_common::{ - config::parse_str_addr, + config::MultiaddrWithPeerId, protocol::event::{DhtEvent, Event}, request_responses::{IfDisconnected, RequestFailure}, service::{ @@ -702,9 +702,8 @@ where self.service.remove_reserved_peer(peer); } - /// Adds a `PeerId` and its address as reserved. The string should encode the address - /// and peer ID of the remote node. - pub fn add_reserved_peer(&self, peer: String) -> Result<(), String> { + /// Adds a `PeerId` and its `Multiaddr` as reserved. + pub fn add_reserved_peer(&self, peer: MultiaddrWithPeerId) -> Result<(), String> { self.service.add_reserved_peer(peer) } @@ -912,17 +911,16 @@ where let _ = self.to_worker.unbounded_send(ServiceToWorkerMsg::SetReservedOnly(true)); } - fn add_reserved_peer(&self, peer: String) -> Result<(), String> { - let (peer_id, addr) = parse_str_addr(&peer).map_err(|e| format!("{:?}", e))?; + fn add_reserved_peer(&self, peer: MultiaddrWithPeerId) -> Result<(), String> { // Make sure the local peer ID is never added to the PSM. - if peer_id == self.local_peer_id { + if peer.peer_id == self.local_peer_id { return Err("Local peer ID cannot be added as a reserved peer.".to_string()) } let _ = self .to_worker - .unbounded_send(ServiceToWorkerMsg::AddKnownAddress(peer_id, addr)); - let _ = self.to_worker.unbounded_send(ServiceToWorkerMsg::AddReserved(peer_id)); + .unbounded_send(ServiceToWorkerMsg::AddKnownAddress(peer.peer_id, peer.multiaddr)); + let _ = self.to_worker.unbounded_send(ServiceToWorkerMsg::AddReserved(peer.peer_id)); Ok(()) } diff --git a/substrate/client/offchain/src/api.rs b/substrate/client/offchain/src/api.rs index 5c8139d450..f40bceb615 100644 --- a/substrate/client/offchain/src/api.rs +++ b/substrate/client/offchain/src/api.rs @@ -326,7 +326,10 @@ mod tests { use super::*; use libp2p::PeerId; use sc_client_db::offchain::LocalStorage; - use sc_network_common::service::{NetworkPeers, NetworkStateInfo}; + use sc_network_common::{ + config::MultiaddrWithPeerId, + service::{NetworkPeers, NetworkStateInfo}, + }; use sc_peerset::ReputationChange; use sp_core::offchain::{DbExternalities, Externalities}; use std::{borrow::Cow, time::SystemTime}; @@ -362,7 +365,7 @@ mod tests { unimplemented!(); } - fn add_reserved_peer(&self, _peer: String) -> Result<(), String> { + fn add_reserved_peer(&self, _peer: MultiaddrWithPeerId) -> Result<(), String> { unimplemented!(); } diff --git a/substrate/client/offchain/src/lib.rs b/substrate/client/offchain/src/lib.rs index 5eed142ff3..e215a01687 100644 --- a/substrate/client/offchain/src/lib.rs +++ b/substrate/client/offchain/src/lib.rs @@ -249,6 +249,7 @@ mod tests { use libp2p::{Multiaddr, PeerId}; use sc_block_builder::BlockBuilderProvider as _; use sc_client_api::Backend as _; + use sc_network_common::config::MultiaddrWithPeerId; use sc_peerset::ReputationChange; use sc_transaction_pool::{BasicPool, FullChainApi}; use sc_transaction_pool_api::{InPoolTransaction, TransactionPool}; @@ -300,7 +301,7 @@ mod tests { unimplemented!(); } - fn add_reserved_peer(&self, _peer: String) -> Result<(), String> { + fn add_reserved_peer(&self, _peer: MultiaddrWithPeerId) -> Result<(), String> { unimplemented!(); } diff --git a/substrate/client/service/src/lib.rs b/substrate/client/service/src/lib.rs index 2f35a04451..19358c1e5b 100644 --- a/substrate/client/service/src/lib.rs +++ b/substrate/client/service/src/lib.rs @@ -42,7 +42,7 @@ use jsonrpsee::{core::Error as JsonRpseeError, RpcModule}; use log::{debug, error, warn}; use sc_client_api::{blockchain::HeaderBackend, BlockBackend, BlockchainEvents, ProofProvider}; use sc_network::PeerId; -use sc_network_common::service::NetworkBlock; +use sc_network_common::{config::MultiaddrWithPeerId, service::NetworkBlock}; use sc_rpc_server::WsConfig; use sc_utils::mpsc::TracingUnboundedReceiver; use sp_blockchain::HeaderMetadata; @@ -230,8 +230,15 @@ async fn build_network_future< } } sc_rpc::system::Request::NetworkAddReservedPeer(peer_addr, sender) => { - let x = network.add_reserved_peer(peer_addr) - .map_err(sc_rpc::system::error::Error::MalformattedPeerArg); + let result = match MultiaddrWithPeerId::try_from(peer_addr) { + Ok(peer) => { + network.add_reserved_peer(peer) + }, + Err(err) => { + Err(err.to_string()) + }, + }; + let x = result.map_err(sc_rpc::system::error::Error::MalformattedPeerArg); let _ = sender.send(x); } sc_rpc::system::Request::NetworkRemoveReservedPeer(peer_id, sender) => { diff --git a/substrate/client/service/test/src/lib.rs b/substrate/client/service/test/src/lib.rs index 919c90400e..11c1cbaf7a 100644 --- a/substrate/client/service/test/src/lib.rs +++ b/substrate/client/service/test/src/lib.rs @@ -24,9 +24,12 @@ use parking_lot::Mutex; use sc_client_api::{Backend, CallExecutor}; use sc_network::{ config::{NetworkConfiguration, TransportConfig}, - multiaddr, Multiaddr, + multiaddr, +}; +use sc_network_common::{ + config::MultiaddrWithPeerId, + service::{NetworkBlock, NetworkPeers, NetworkStateInfo}, }; -use sc_network_common::service::{NetworkBlock, NetworkPeers, NetworkStateInfo}; use sc_service::{ client::Client, config::{BasePath, DatabaseSource, KeystoreConfig}, @@ -49,8 +52,8 @@ const MAX_WAIT_TIME: Duration = Duration::from_secs(60 * 3); struct TestNet { runtime: Runtime, - authority_nodes: Vec<(usize, F, U, Multiaddr)>, - full_nodes: Vec<(usize, F, U, Multiaddr)>, + authority_nodes: Vec<(usize, F, U, MultiaddrWithPeerId)>, + full_nodes: Vec<(usize, F, U, MultiaddrWithPeerId)>, chain_spec: GenericChainSpec, base_port: u16, nodes: usize, @@ -320,7 +323,7 @@ where handle.spawn(service.clone().map_err(|_| ())); let addr = - addr.with(multiaddr::Protocol::P2p((service.network().local_peer_id()).into())); + MultiaddrWithPeerId { multiaddr: addr, peer_id: service.network().local_peer_id() }; self.authority_nodes.push((self.nodes, service, user_data, addr)); self.nodes += 1; } @@ -340,7 +343,7 @@ where handle.spawn(service.clone().map_err(|_| ())); let addr = - addr.with(multiaddr::Protocol::P2p((service.network().local_peer_id()).into())); + MultiaddrWithPeerId { multiaddr: addr, peer_id: service.network().local_peer_id() }; self.full_nodes.push((self.nodes, service, user_data, addr)); self.nodes += 1; } @@ -382,7 +385,7 @@ where for (_, service, _, _) in network.full_nodes.iter().skip(1) { service .network() - .add_reserved_peer(first_address.to_string()) + .add_reserved_peer(first_address.clone()) .expect("Error adding reserved peer"); } @@ -414,7 +417,7 @@ where if let Some((_, service, _, node_id)) = network.full_nodes.get(i) { service .network() - .add_reserved_peer(address.to_string()) + .add_reserved_peer(address) .expect("Error adding reserved peer"); address = node_id.clone(); } @@ -479,7 +482,7 @@ pub fn sync( for (_, service, _, _) in network.full_nodes.iter().skip(1) { service .network() - .add_reserved_peer(first_address.to_string()) + .add_reserved_peer(first_address.clone()) .expect("Error adding reserved peer"); } @@ -532,13 +535,13 @@ pub fn consensus( for (_, service, _, _) in network.full_nodes.iter() { service .network() - .add_reserved_peer(first_address.to_string()) + .add_reserved_peer(first_address.clone()) .expect("Error adding reserved peer"); } for (_, service, _, _) in network.authority_nodes.iter().skip(1) { service .network() - .add_reserved_peer(first_address.to_string()) + .add_reserved_peer(first_address.clone()) .expect("Error adding reserved peer"); } network.run_until_all_full(|_index, service| { @@ -556,7 +559,7 @@ pub fn consensus( for (_, service, _, _) in network.full_nodes.iter() { service .network() - .add_reserved_peer(first_address.to_string()) + .add_reserved_peer(first_address.clone()) .expect("Error adding reserved peer"); }