Use custom type for ProtocolName (#12172)

* Add ProtocolName custom type

* Use new ProtocolName in sc_network_common

* Use new ProtocolName in sc_network

* Use new ProtocolName for BEEFY and GRANDPA

* Use new ProtocolName for notifications

* Use new ProtocolName in sc_network (part 2)

* Use new ProtocolName in sc_network_gossip

* Use new ProtocolName in sc_offchain

* Remove unused imports

* Some more fixes

* Add tests

* Fix minor import issues

* Re-export ProtocolName in sc_network

* Revert "Re-export ProtocolName in sc_network"

This reverts commit 8d8ff71927e7750757f29c9bbd88dc0ba181d214.

* Re-export ProtocolName in sc_network

* Remove dependency on sc-network-common from beefy-gadget

substrate/client/network/common/src/config.rs

@@ -21,7 +21,9 @@
 use libp2p::{multiaddr, Multiaddr, PeerId};
 use std::{fmt, str, str::FromStr};
 
-/// Name of a protocol, transmitted on the wire. Should be unique for each chain. Always UTF-8.
+/// Protocol name prefix, transmitted on the wire for legacy protocol names.
+/// I.e., `dot` in `/dot/sync/2`. Should be unique for each chain. Always UTF-8.
+/// Deprecated in favour of genesis hash & fork ID based protocol names.
 #[derive(Clone, PartialEq, Eq, Hash)]
 pub struct ProtocolId(smallvec::SmallVec<[u8; 6]>);
 

substrate/client/network/common/src/protocol.rs

@@ -16,4 +16,131 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
+use std::{
+	borrow::Borrow,
+	fmt,
+	hash::{Hash, Hasher},
+	ops::Deref,
+	sync::Arc,
+};
+
+use libp2p::core::upgrade;
+
 pub mod event;
+
+/// The protocol name transmitted on the wire.
+#[derive(Debug, Clone)]
+pub enum ProtocolName {
+	/// The protocol name as a static string.
+	Static(&'static str),
+	/// The protocol name as a dynamically allocated string.
+	OnHeap(Arc<str>),
+}
+
+impl From<&'static str> for ProtocolName {
+	fn from(name: &'static str) -> Self {
+		Self::Static(name)
+	}
+}
+
+impl From<Arc<str>> for ProtocolName {
+	fn from(name: Arc<str>) -> Self {
+		Self::OnHeap(name)
+	}
+}
+
+impl From<String> for ProtocolName {
+	fn from(name: String) -> Self {
+		Self::OnHeap(Arc::from(name))
+	}
+}
+
+impl Deref for ProtocolName {
+	type Target = str;
+
+	fn deref(&self) -> &str {
+		match self {
+			Self::Static(name) => name,
+			Self::OnHeap(name) => &name,
+		}
+	}
+}
+
+impl Borrow<str> for ProtocolName {
+	fn borrow(&self) -> &str {
+		self
+	}
+}
+
+impl PartialEq for ProtocolName {
+	fn eq(&self, other: &Self) -> bool {
+		(self as &str) == (other as &str)
+	}
+}
+
+impl Eq for ProtocolName {}
+
+impl Hash for ProtocolName {
+	fn hash<H: Hasher>(&self, state: &mut H) {
+		(self as &str).hash(state)
+	}
+}
+
+impl fmt::Display for ProtocolName {
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		f.write_str(self)
+	}
+}
+
+impl upgrade::ProtocolName for ProtocolName {
+	fn protocol_name(&self) -> &[u8] {
+		(self as &str).as_bytes()
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	use super::ProtocolName;
+	use std::{
+		borrow::Borrow,
+		collections::hash_map::DefaultHasher,
+		hash::{Hash, Hasher},
+	};
+
+	#[test]
+	fn protocol_name_keys_are_equivalent_to_str_keys() {
+		const PROTOCOL: &'static str = "/some/protocol/1";
+		let static_protocol_name = ProtocolName::from(PROTOCOL);
+		let on_heap_protocol_name = ProtocolName::from(String::from(PROTOCOL));
+
+		assert_eq!(<ProtocolName as Borrow<str>>::borrow(&static_protocol_name), PROTOCOL);
+		assert_eq!(<ProtocolName as Borrow<str>>::borrow(&on_heap_protocol_name), PROTOCOL);
+		assert_eq!(static_protocol_name, on_heap_protocol_name);
+
+		assert_eq!(hash(static_protocol_name), hash(PROTOCOL));
+		assert_eq!(hash(on_heap_protocol_name), hash(PROTOCOL));
+	}
+
+	#[test]
+	fn different_protocol_names_do_not_compare_equal() {
+		const PROTOCOL1: &'static str = "/some/protocol/1";
+		let static_protocol_name1 = ProtocolName::from(PROTOCOL1);
+		let on_heap_protocol_name1 = ProtocolName::from(String::from(PROTOCOL1));
+
+		const PROTOCOL2: &'static str = "/some/protocol/2";
+		let static_protocol_name2 = ProtocolName::from(PROTOCOL2);
+		let on_heap_protocol_name2 = ProtocolName::from(String::from(PROTOCOL2));
+
+		assert_ne!(<ProtocolName as Borrow<str>>::borrow(&static_protocol_name1), PROTOCOL2);
+		assert_ne!(<ProtocolName as Borrow<str>>::borrow(&on_heap_protocol_name1), PROTOCOL2);
+		assert_ne!(static_protocol_name1, static_protocol_name2);
+		assert_ne!(static_protocol_name1, on_heap_protocol_name2);
+		assert_ne!(on_heap_protocol_name1, on_heap_protocol_name2);
+	}
+
+	fn hash<T: Hash>(x: T) -> u64 {
+		let mut hasher = DefaultHasher::new();
+		x.hash(&mut hasher);
+		hasher.finish()
+	}
+}

substrate/client/network/common/src/protocol/event.rs

@@ -19,9 +19,9 @@
 //! Network event types. These are are not the part of the protocol, but rather
 //! events that happen on the network like DHT get/put results received.
 
+use super::ProtocolName;
 use bytes::Bytes;
 use libp2p::{core::PeerId, kad::record::Key};
-use std::borrow::Cow;
 
 /// Events generated by DHT as a response to get_value and put_value requests.
 #[derive(Debug, Clone)]
@@ -69,13 +69,13 @@ pub enum Event {
 		/// This is always equal to the value of
 		/// `sc_network::config::NonDefaultSetConfig::notifications_protocol` of one of the
 		/// configured sets.
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 		/// If the negotiation didn't use the main name of the protocol (the one in
 		/// `notifications_protocol`), then this field contains which name has actually been
 		/// used.
 		/// Always contains a value equal to the value in
 		/// `sc_network::config::NonDefaultSetConfig::fallback_names`.
-		negotiated_fallback: Option<Cow<'static, str>>,
+		negotiated_fallback: Option<ProtocolName>,
 		/// Role of the remote.
 		role: ObservedRole,
 	},
@@ -86,7 +86,7 @@ pub enum Event {
 		/// Node we closed the substream with.
 		remote: PeerId,
 		/// The concerned protocol. Each protocol uses a different substream.
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 	},
 
 	/// Received one or more messages from the given node using the given protocol.
@@ -94,7 +94,7 @@ pub enum Event {
 		/// Node we received the message from.
 		remote: PeerId,
 		/// Concerned protocol and associated message.
-		messages: Vec<(Cow<'static, str>, Bytes)>,
+		messages: Vec<(ProtocolName, Bytes)>,
 	},
 }
 

substrate/client/network/common/src/request_responses.rs

@@ -18,19 +18,20 @@
 
 //! Collection of generic data structures for request-response protocols.
 
+use crate::protocol::ProtocolName;
 use futures::channel::{mpsc, oneshot};
 use libp2p::{request_response::OutboundFailure, PeerId};
 use sc_peerset::ReputationChange;
-use std::{borrow::Cow, time::Duration};
+use std::time::Duration;
 
 /// Configuration for a single request-response protocol.
 #[derive(Debug, Clone)]
 pub struct ProtocolConfig {
 	/// Name of the protocol on the wire. Should be something like `/foo/bar`.
-	pub name: Cow<'static, str>,
+	pub name: ProtocolName,
 
 	/// Fallback on the wire protocol names to support.
-	pub fallback_names: Vec<Cow<'static, str>>,
+	pub fallback_names: Vec<ProtocolName>,
 
 	/// Maximum allowed size, in bytes, of a request.
 	///

substrate/client/network/common/src/service.rs

@@ -20,7 +20,7 @@
 
 use crate::{
 	config::MultiaddrWithPeerId,
-	protocol::event::Event,
+	protocol::{event::Event, ProtocolName},
 	request_responses::{IfDisconnected, RequestFailure},
 	sync::{warp::WarpSyncProgress, StateDownloadProgress, SyncState},
 };
@@ -30,7 +30,7 @@ use libp2p::{Multiaddr, PeerId};
 use sc_peerset::ReputationChange;
 pub use signature::Signature;
 use sp_runtime::traits::{Block as BlockT, NumberFor};
-use std::{borrow::Cow, collections::HashSet, future::Future, pin::Pin, sync::Arc};
+use std::{collections::HashSet, future::Future, pin::Pin, sync::Arc};
 
 mod signature;
 
@@ -171,7 +171,7 @@ pub trait NetworkPeers {
 	/// See also [`NetworkPeers::remove_from_peers_set`], which has the same effect but also
 	/// prevents the local node from re-establishing an outgoing substream to this peer until it
 	/// is added again.
-	fn disconnect_peer(&self, who: PeerId, protocol: Cow<'static, str>);
+	fn disconnect_peer(&self, who: PeerId, protocol: ProtocolName);
 
 	/// Connect to unreserved peers and allow unreserved peers to connect for syncing purposes.
 	fn accept_unreserved_peers(&self);
@@ -207,7 +207,7 @@ pub trait NetworkPeers {
 	/// invalid peer ID (which includes the local peer ID).
 	fn set_reserved_peers(
 		&self,
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 		peers: HashSet<Multiaddr>,
 	) -> Result<(), String>;
 
@@ -220,12 +220,12 @@ pub trait NetworkPeers {
 	/// invalid peer ID (which includes the local peer ID).
 	fn add_peers_to_reserved_set(
 		&self,
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 		peers: HashSet<Multiaddr>,
 	) -> Result<(), String>;
 
 	/// Remove peers from a peer set.
-	fn remove_peers_from_reserved_set(&self, protocol: Cow<'static, str>, peers: Vec<PeerId>);
+	fn remove_peers_from_reserved_set(&self, protocol: ProtocolName, peers: Vec<PeerId>);
 
 	/// Add a peer to a set of peers.
 	///
@@ -238,14 +238,14 @@ pub trait NetworkPeers {
 	/// invalid peer ID (which includes the local peer ID).
 	fn add_to_peers_set(
 		&self,
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 		peers: HashSet<Multiaddr>,
 	) -> Result<(), String>;
 
 	/// Remove peers from a peer set.
 	///
 	/// If we currently have an open substream with this peer, it will soon be closed.
-	fn remove_from_peers_set(&self, protocol: Cow<'static, str>, peers: Vec<PeerId>);
+	fn remove_from_peers_set(&self, protocol: ProtocolName, peers: Vec<PeerId>);
 
 	/// Returns the number of peers in the sync peer set we're connected to.
 	fn sync_num_connected(&self) -> usize;
@@ -273,7 +273,7 @@ where
 		T::report_peer(self, who, cost_benefit)
 	}
 
-	fn disconnect_peer(&self, who: PeerId, protocol: Cow<'static, str>) {
+	fn disconnect_peer(&self, who: PeerId, protocol: ProtocolName) {
 		T::disconnect_peer(self, who, protocol)
 	}
 
@@ -295,7 +295,7 @@ where
 
 	fn set_reserved_peers(
 		&self,
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 		peers: HashSet<Multiaddr>,
 	) -> Result<(), String> {
 		T::set_reserved_peers(self, protocol, peers)
@@ -303,25 +303,25 @@ where
 
 	fn add_peers_to_reserved_set(
 		&self,
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 		peers: HashSet<Multiaddr>,
 	) -> Result<(), String> {
 		T::add_peers_to_reserved_set(self, protocol, peers)
 	}
 
-	fn remove_peers_from_reserved_set(&self, protocol: Cow<'static, str>, peers: Vec<PeerId>) {
+	fn remove_peers_from_reserved_set(&self, protocol: ProtocolName, peers: Vec<PeerId>) {
 		T::remove_peers_from_reserved_set(self, protocol, peers)
 	}
 
 	fn add_to_peers_set(
 		&self,
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 		peers: HashSet<Multiaddr>,
 	) -> Result<(), String> {
 		T::add_to_peers_set(self, protocol, peers)
 	}
 
-	fn remove_from_peers_set(&self, protocol: Cow<'static, str>, peers: Vec<PeerId>) {
+	fn remove_from_peers_set(&self, protocol: ProtocolName, peers: Vec<PeerId>) {
 		T::remove_from_peers_set(self, protocol, peers)
 	}
 
@@ -431,7 +431,7 @@ pub trait NetworkNotification {
 	///
 	/// The protocol must have been registered with
 	/// `crate::config::NetworkConfiguration::notifications_protocols`.
-	fn write_notification(&self, target: PeerId, protocol: Cow<'static, str>, message: Vec<u8>);
+	fn write_notification(&self, target: PeerId, protocol: ProtocolName, message: Vec<u8>);
 
 	/// Obtains a [`NotificationSender`] for a connected peer, if it exists.
 	///
@@ -502,7 +502,7 @@ pub trait NetworkNotification {
 	fn notification_sender(
 		&self,
 		target: PeerId,
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 	) -> Result<Box<dyn NotificationSender>, NotificationSenderError>;
 }
 
@@ -511,14 +511,14 @@ where
 	T: ?Sized,
 	T: NetworkNotification,
 {
-	fn write_notification(&self, target: PeerId, protocol: Cow<'static, str>, message: Vec<u8>) {
+	fn write_notification(&self, target: PeerId, protocol: ProtocolName, message: Vec<u8>) {
 		T::write_notification(self, target, protocol, message)
 	}
 
 	fn notification_sender(
 		&self,
 		target: PeerId,
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 	) -> Result<Box<dyn NotificationSender>, NotificationSenderError> {
 		T::notification_sender(self, target, protocol)
 	}
@@ -547,7 +547,7 @@ pub trait NetworkRequest {
 	async fn request(
 		&self,
 		target: PeerId,
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 		request: Vec<u8>,
 		connect: IfDisconnected,
 	) -> Result<Vec<u8>, RequestFailure>;
@@ -565,7 +565,7 @@ pub trait NetworkRequest {
 	fn start_request(
 		&self,
 		target: PeerId,
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 		request: Vec<u8>,
 		tx: oneshot::Sender<Result<Vec<u8>, RequestFailure>>,
 		connect: IfDisconnected,
@@ -581,7 +581,7 @@ where
 	fn request<'life0, 'async_trait>(
 		&'life0 self,
 		target: PeerId,
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 		request: Vec<u8>,
 		connect: IfDisconnected,
 	) -> Pin<Box<dyn Future<Output = Result<Vec<u8>, RequestFailure>> + Send + 'async_trait>>
@@ -595,7 +595,7 @@ where
 	fn start_request(
 		&self,
 		target: PeerId,
-		protocol: Cow<'static, str>,
+		protocol: ProtocolName,
 		request: Vec<u8>,
 		tx: oneshot::Sender<Result<Vec<u8>, RequestFailure>>,
 		connect: IfDisconnected,