Skip to content

Commit

Permalink
Use custom type for ProtocolName (#12172)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
dmitry-markin authored Sep 3, 2022
1 parent 4b79116 commit 78bff0e
Show file tree
Hide file tree
Showing 27 changed files with 381 additions and 280 deletions.
8 changes: 5 additions & 3 deletions client/beefy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use beefy_primitives::{BeefyApi, MmrRootHash};
use prometheus::Registry;
use sc_client_api::{Backend, BlockchainEvents, Finalizer};
use sc_consensus::BlockImport;
use sc_network::ProtocolName;
use sc_network_gossip::Network as GossipNetwork;
use sp_api::ProvideRuntimeApi;
use sp_blockchain::HeaderBackend;
Expand Down Expand Up @@ -55,6 +56,7 @@ pub use beefy_protocol_name::standard_name as protocol_standard_name;

pub(crate) mod beefy_protocol_name {
use sc_chain_spec::ChainSpec;
use sc_network::ProtocolName;

const NAME: &str = "/beefy/1";
/// Old names for the notifications protocol, used for backward compatibility.
Expand All @@ -66,7 +68,7 @@ pub(crate) mod beefy_protocol_name {
pub fn standard_name<Hash: AsRef<[u8]>>(
genesis_hash: &Hash,
chain_spec: &Box<dyn ChainSpec>,
) -> std::borrow::Cow<'static, str> {
) -> ProtocolName {
let chain_prefix = match chain_spec.fork_id() {
Some(fork_id) => format!("/{}/{}", hex::encode(genesis_hash), fork_id),
None => format!("/{}", hex::encode(genesis_hash)),
Expand All @@ -79,7 +81,7 @@ pub(crate) mod beefy_protocol_name {
/// [`sc_network::config::NetworkConfiguration::extra_sets`].
/// For standard protocol name see [`beefy_protocol_name::standard_name`].
pub fn beefy_peers_set_config(
protocol_name: std::borrow::Cow<'static, str>,
protocol_name: ProtocolName,
) -> sc_network::config::NonDefaultSetConfig {
let mut cfg = sc_network::config::NonDefaultSetConfig::new(protocol_name, 1024 * 1024);

Expand Down Expand Up @@ -202,7 +204,7 @@ where
/// Prometheus metric registry
pub prometheus_registry: Option<Registry>,
/// Chain specific GRANDPA protocol name. See [`beefy_protocol_name::standard_name`].
pub protocol_name: std::borrow::Cow<'static, str>,
pub protocol_name: ProtocolName,
/// Links between the block importer, the background voter and the RPC layer.
pub links: BeefyVoterLinks<B>,
}
Expand Down
3 changes: 2 additions & 1 deletion client/finality-grandpa/src/communication/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ pub(crate) mod tests;

pub mod grandpa_protocol_name {
use sc_chain_spec::ChainSpec;
use sc_network_common::protocol::ProtocolName;

pub(crate) const NAME: &str = "/grandpa/1";
/// Old names for the notifications protocol, used for backward compatibility.
Expand All @@ -81,7 +82,7 @@ pub mod grandpa_protocol_name {
pub fn standard_name<Hash: AsRef<[u8]>>(
genesis_hash: &Hash,
chain_spec: &Box<dyn ChainSpec>,
) -> std::borrow::Cow<'static, str> {
) -> ProtocolName {
let chain_prefix = match chain_spec.fork_id() {
Some(fork_id) => format!("/{}/{}", hex::encode(genesis_hash), fork_id),
None => format!("/{}", hex::encode(genesis_hash)),
Expand Down
22 changes: 12 additions & 10 deletions client/finality-grandpa/src/communication/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ 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},
protocol::{
event::{Event as NetworkEvent, ObservedRole},
ProtocolName,
},
service::{
NetworkBlock, NetworkEventStream, NetworkNotification, NetworkPeers,
NetworkSyncForkRequest, NotificationSender, NotificationSenderError,
Expand All @@ -41,7 +44,6 @@ use sp_finality_grandpa::AuthorityList;
use sp_keyring::Ed25519Keyring;
use sp_runtime::traits::NumberFor;
use std::{
borrow::Cow,
collections::HashSet,
pin::Pin,
sync::Arc,
Expand Down Expand Up @@ -78,7 +80,7 @@ impl NetworkPeers for TestNetwork {
let _ = self.sender.unbounded_send(Event::Report(who, cost_benefit));
}

fn disconnect_peer(&self, _who: PeerId, _protocol: Cow<'static, str>) {}
fn disconnect_peer(&self, _who: PeerId, _protocol: ProtocolName) {}

fn accept_unreserved_peers(&self) {
unimplemented!();
Expand All @@ -98,31 +100,31 @@ impl NetworkPeers for TestNetwork {

fn set_reserved_peers(
&self,
_protocol: Cow<'static, str>,
_protocol: ProtocolName,
_peers: HashSet<Multiaddr>,
) -> Result<(), String> {
unimplemented!();
}

fn add_peers_to_reserved_set(
&self,
_protocol: Cow<'static, str>,
_protocol: ProtocolName,
_peers: HashSet<Multiaddr>,
) -> Result<(), String> {
unimplemented!();
}

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>) {}

fn add_to_peers_set(
&self,
_protocol: Cow<'static, str>,
_protocol: ProtocolName,
_peers: HashSet<Multiaddr>,
) -> Result<(), String> {
unimplemented!();
}

fn remove_from_peers_set(&self, _protocol: Cow<'static, str>, _peers: Vec<PeerId>) {
fn remove_from_peers_set(&self, _protocol: ProtocolName, _peers: Vec<PeerId>) {
unimplemented!();
}

Expand All @@ -143,14 +145,14 @@ impl NetworkEventStream for TestNetwork {
}

impl NetworkNotification for TestNetwork {
fn write_notification(&self, target: PeerId, _protocol: Cow<'static, str>, message: Vec<u8>) {
fn write_notification(&self, target: PeerId, _protocol: ProtocolName, message: Vec<u8>) {
let _ = self.sender.unbounded_send(Event::WriteNotification(target, message));
}

fn notification_sender(
&self,
_target: PeerId,
_protocol: Cow<'static, str>,
_protocol: ProtocolName,
) -> Result<Box<dyn NotificationSender>, NotificationSenderError> {
unimplemented!();
}
Expand Down
5 changes: 3 additions & 2 deletions client/finality-grandpa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ use sc_client_api::{
StorageProvider, TransactionFor,
};
use sc_consensus::BlockImport;
use sc_network_common::protocol::ProtocolName;
use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_INFO};
use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedReceiver};
use sp_api::ProvideRuntimeApi;
Expand Down Expand Up @@ -266,7 +267,7 @@ pub struct Config {
/// TelemetryHandle instance.
pub telemetry: Option<TelemetryHandle>,
/// Chain specific GRANDPA protocol name. See [`crate::protocol_standard_name`].
pub protocol_name: std::borrow::Cow<'static, str>,
pub protocol_name: ProtocolName,
}

impl Config {
Expand Down Expand Up @@ -723,7 +724,7 @@ pub struct GrandpaParams<Block: BlockT, C, N, SC, VR> {
/// [`sc_network::config::NetworkConfiguration::extra_sets`].
/// For standard protocol name see [`crate::protocol_standard_name`].
pub fn grandpa_peers_set_config(
protocol_name: std::borrow::Cow<'static, str>,
protocol_name: ProtocolName,
) -> sc_network::config::NonDefaultSetConfig {
use communication::grandpa_protocol_name;
sc_network::config::NonDefaultSetConfig {
Expand Down
38 changes: 13 additions & 25 deletions client/network-gossip/src/bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{
Network, Validator,
};

use sc_network_common::protocol::event::Event;
use sc_network_common::protocol::{event::Event, ProtocolName};
use sc_peerset::ReputationChange;

use futures::{
Expand All @@ -33,7 +33,6 @@ use log::trace;
use prometheus_endpoint::Registry;
use sp_runtime::traits::Block as BlockT;
use std::{
borrow::Cow,
collections::{HashMap, VecDeque},
pin::Pin,
sync::Arc,
Expand All @@ -46,7 +45,7 @@ pub struct GossipEngine<B: BlockT> {
state_machine: ConsensusGossip<B>,
network: Box<dyn Network<B> + Send>,
periodic_maintenance_interval: futures_timer::Delay,
protocol: Cow<'static, str>,
protocol: ProtocolName,

/// Incoming events from the network.
network_event_stream: Pin<Box<dyn Stream<Item = Event> + Send>>,
Expand Down Expand Up @@ -78,7 +77,7 @@ impl<B: BlockT> GossipEngine<B> {
/// Create a new instance.
pub fn new<N: Network<B> + Send + Clone + 'static>(
network: N,
protocol: impl Into<Cow<'static, str>>,
protocol: impl Into<ProtocolName>,
validator: Arc<dyn Validator<B>>,
metrics_registry: Option<&Registry>,
) -> Self
Expand Down Expand Up @@ -329,7 +328,6 @@ mod tests {
traits::{Block as BlockT, NumberFor},
};
use std::{
borrow::Cow,
collections::HashSet,
sync::{Arc, Mutex},
};
Expand Down Expand Up @@ -360,7 +358,7 @@ mod tests {

fn report_peer(&self, _who: PeerId, _cost_benefit: ReputationChange) {}

fn disconnect_peer(&self, _who: PeerId, _protocol: Cow<'static, str>) {
fn disconnect_peer(&self, _who: PeerId, _protocol: ProtocolName) {
unimplemented!();
}

Expand All @@ -382,36 +380,31 @@ mod tests {

fn set_reserved_peers(
&self,
_protocol: Cow<'static, str>,
_protocol: ProtocolName,
_peers: HashSet<Multiaddr>,
) -> Result<(), String> {
unimplemented!();
}

fn add_peers_to_reserved_set(
&self,
_protocol: Cow<'static, str>,
_protocol: ProtocolName,
_peers: HashSet<Multiaddr>,
) -> Result<(), String> {
unimplemented!();
}

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>) {}

fn add_to_peers_set(
&self,
_protocol: Cow<'static, str>,
_protocol: ProtocolName,
_peers: HashSet<Multiaddr>,
) -> Result<(), String> {
unimplemented!();
}

fn remove_from_peers_set(&self, _protocol: Cow<'static, str>, _peers: Vec<PeerId>) {
fn remove_from_peers_set(&self, _protocol: ProtocolName, _peers: Vec<PeerId>) {
unimplemented!();
}

Expand All @@ -430,19 +423,14 @@ mod tests {
}

impl NetworkNotification for TestNetwork {
fn write_notification(
&self,
_target: PeerId,
_protocol: Cow<'static, str>,
_message: Vec<u8>,
) {
fn write_notification(&self, _target: PeerId, _protocol: ProtocolName, _message: Vec<u8>) {
unimplemented!();
}

fn notification_sender(
&self,
_target: PeerId,
_protocol: Cow<'static, str>,
_protocol: ProtocolName,
) -> Result<Box<dyn NotificationSender>, NotificationSenderError> {
unimplemented!();
}
Expand Down Expand Up @@ -505,7 +493,7 @@ mod tests {
#[test]
fn keeps_multiple_subscribers_per_topic_updated_with_both_old_and_new_messages() {
let topic = H256::default();
let protocol = Cow::Borrowed("/my_protocol");
let protocol = ProtocolName::from("/my_protocol");
let remote_peer = PeerId::random();
let network = TestNetwork::default();

Expand Down Expand Up @@ -622,7 +610,7 @@ mod tests {
}

fn prop(channels: Vec<ChannelLengthAndTopic>, notifications: Vec<Vec<Message>>) {
let protocol = Cow::Borrowed("/my_protocol");
let protocol = ProtocolName::from("/my_protocol");
let remote_peer = PeerId::random();
let network = TestNetwork::default();

Expand Down
9 changes: 5 additions & 4 deletions client/network-gossip/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,12 @@ pub use self::{
};

use libp2p::{multiaddr, PeerId};
use sc_network_common::service::{
NetworkBlock, NetworkEventStream, NetworkNotification, NetworkPeers,
use sc_network_common::{
protocol::ProtocolName,
service::{NetworkBlock, NetworkEventStream, NetworkNotification, NetworkPeers},
};
use sp_runtime::traits::{Block as BlockT, NumberFor};
use std::{borrow::Cow, iter};
use std::iter;

mod bridge;
mod state_machine;
Expand All @@ -82,7 +83,7 @@ mod validator;
pub trait Network<B: BlockT>:
NetworkPeers + NetworkEventStream + NetworkNotification + NetworkBlock<B::Hash, NumberFor<B>>
{
fn add_set_reserved(&self, who: PeerId, protocol: Cow<'static, str>) {
fn add_set_reserved(&self, who: PeerId, protocol: ProtocolName) {
let addr =
iter::once(multiaddr::Protocol::P2p(who.into())).collect::<multiaddr::Multiaddr>();
let result = self.add_peers_to_reserved_set(protocol, iter::once(addr).collect());
Expand Down
Loading

0 comments on commit 78bff0e

Please sign in to comment.