Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Minor refactorings in the process of #2177
Browse files Browse the repository at this point in the history
By having everything peer set related depend directly on the enum the
code becomes more clear and it is also straight forward to add more
peersets/protocols as the compiler will complain if you forget to
implement parts of it.
  • Loading branch information
eskimor committed Jan 7, 2021
1 parent a9a76be commit 56e1d78
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 83 deletions.
82 changes: 12 additions & 70 deletions node/network/bridge/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use polkadot_subsystem::messages::{
};
use polkadot_primitives::v1::{AuthorityDiscoveryId, Block, Hash, BlockNumber};
use polkadot_node_network_protocol::{
ObservedRole, ReputationChange, PeerId, PeerSet, View, NetworkBridgeEvent, v1 as protocol_v1, OurView,
ObservedRole, ReputationChange, PeerId, peer_set::PeerSet, View, NetworkBridgeEvent, v1 as protocol_v1, OurView,
};

use std::collections::{HashMap, hash_map};
Expand All @@ -54,10 +54,6 @@ mod validator_discovery;
/// We use the same limit to compute the view sent to peers locally.
const MAX_VIEW_HEADS: usize = 5;

/// The protocol name for the validation peer-set.
pub const VALIDATION_PROTOCOL_NAME: &'static str = "/polkadot/validation/1";
/// The protocol name for the collation peer-set.
pub const COLLATION_PROTOCOL_NAME: &'static str = "/polkadot/collation/1";

const MALFORMED_MESSAGE_COST: ReputationChange
= ReputationChange::new(-500, "Malformed Network-bridge message");
Expand All @@ -80,31 +76,6 @@ pub enum WireMessage<M> {
ViewUpdate(View),
}

/// Information about the extra peers set. Should be used during network configuration
/// to register the protocol with the network service.
pub fn peers_sets_info() -> Vec<sc_network::config::NonDefaultSetConfig> {
vec![
sc_network::config::NonDefaultSetConfig {
notifications_protocol: VALIDATION_PROTOCOL_NAME.into(),
set_config: sc_network::config::SetConfig {
in_peers: 25,
out_peers: 0,
reserved_nodes: Vec::new(),
non_reserved_mode: sc_network::config::NonReservedPeerMode::Accept,
},
},
sc_network::config::NonDefaultSetConfig {
notifications_protocol: COLLATION_PROTOCOL_NAME.into(),
set_config: sc_network::config::SetConfig {
in_peers: 25,
out_peers: 0,
reserved_nodes: Vec::new(),
non_reserved_mode: sc_network::config::NonReservedPeerMode::Accept,
},
}
]
}

/// An action to be carried out by the network.
#[derive(Debug, PartialEq)]
pub enum NetworkAction {
Expand Down Expand Up @@ -176,20 +147,8 @@ impl Network for Arc<sc_network::NetworkService<Block, Hash>> {
cost_benefit,
)
}
NetworkAction::WriteNotification(peer, peer_set, message) => {
match peer_set {
PeerSet::Validation => self.0.write_notification(
peer,
VALIDATION_PROTOCOL_NAME.into(),
message,
),
PeerSet::Collation => self.0.write_notification(
peer,
COLLATION_PROTOCOL_NAME.into(),
message,
),
}
}
NetworkAction::WriteNotification(peer, peer_set, message) =>
self.0.write_notification(peer, peer_set.into_protocol_name(), message)
}

Ok(())
Expand Down Expand Up @@ -326,26 +285,16 @@ fn action_from_network_message(event: Option<NetworkEvent>) -> Action {
Some(NetworkEvent::SyncDisconnected { .. }) => Action::Nop,
Some(NetworkEvent::NotificationStreamOpened { remote, protocol, role }) => {
let role = role.into();
match protocol {
x if x == VALIDATION_PROTOCOL_NAME
=> Action::PeerConnected(PeerSet::Validation, remote, role),
x if x == COLLATION_PROTOCOL_NAME
=> Action::PeerConnected(PeerSet::Collation, remote, role),
_ => Action::Nop,
}
PeerSet::try_from_protocol_name(&protocol)
.map_or(Action::Nop, |peer_set| Action::PeerConnected(peer_set, remote, role))
}
Some(NetworkEvent::NotificationStreamClosed { remote, protocol }) => {
match protocol {
x if x == VALIDATION_PROTOCOL_NAME
=> Action::PeerDisconnected(PeerSet::Validation, remote),
x if x == COLLATION_PROTOCOL_NAME
=> Action::PeerDisconnected(PeerSet::Collation, remote),
_ => Action::Nop,
}
PeerSet::try_from_protocol_name(&protocol)
.map_or(Action::Nop, |peer_set| Action::PeerDisconnected(peer_set, remote))
}
Some(NetworkEvent::NotificationsReceived { remote, messages }) => {
let v_messages: Result<Vec<_>, _> = messages.iter()
.filter(|(protocol, _)| protocol == &VALIDATION_PROTOCOL_NAME)
.filter(|(protocol, _)| protocol == &PeerSet::Validation.into_protocol_name())
.map(|(_, msg_bytes)| WireMessage::decode(&mut msg_bytes.as_ref()))
.collect();

Expand All @@ -355,7 +304,7 @@ fn action_from_network_message(event: Option<NetworkEvent>) -> Action {
};

let c_messages: Result<Vec<_>, _> = messages.iter()
.filter(|(protocol, _)| protocol == &COLLATION_PROTOCOL_NAME)
.filter(|(protocol, _)| protocol == &PeerSet::Collation.into_protocol_name())
.map(|(_, msg_bytes)| WireMessage::decode(&mut msg_bytes.as_ref()))
.collect();

Expand Down Expand Up @@ -830,13 +779,6 @@ mod tests {
)
}

fn peer_set_protocol(peer_set: PeerSet) -> std::borrow::Cow<'static, str> {
match peer_set {
PeerSet::Validation => VALIDATION_PROTOCOL_NAME.into(),
PeerSet::Collation => COLLATION_PROTOCOL_NAME.into(),
}
}

impl Network for TestNetwork {
fn event_stream(&mut self) -> BoxStream<'static, NetworkEvent> {
self.net_events.lock()
Expand Down Expand Up @@ -893,22 +835,22 @@ mod tests {
async fn connect_peer(&mut self, peer: PeerId, peer_set: PeerSet, role: ObservedRole) {
self.send_network_event(NetworkEvent::NotificationStreamOpened {
remote: peer,
protocol: peer_set_protocol(peer_set),
protocol: peer_set.into_protocol_name(),
role: role.into(),
}).await;
}

async fn disconnect_peer(&mut self, peer: PeerId, peer_set: PeerSet) {
self.send_network_event(NetworkEvent::NotificationStreamClosed {
remote: peer,
protocol: peer_set_protocol(peer_set),
protocol: peer_set.into_protocol_name(),
}).await;
}

async fn peer_message(&mut self, peer: PeerId, peer_set: PeerSet, message: Vec<u8>) {
self.send_network_event(NetworkEvent::NotificationsReceived {
remote: peer,
messages: vec![(peer_set_protocol(peer_set), message.into())],
messages: vec![(peer_set.into_protocol_name(), message.into())],
}).await;
}

Expand Down
9 changes: 5 additions & 4 deletions node/network/bridge/src/validator_discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use sc_network::multiaddr::{Multiaddr, Protocol};
use sc_authority_discovery::Service as AuthorityDiscoveryService;
use polkadot_node_network_protocol::PeerId;
use polkadot_primitives::v1::{AuthorityDiscoveryId, Block, Hash};
use polkadot_node_network_protocol::peer_set::PeerSet;

const LOG_TARGET: &str = "validator_discovery";

Expand Down Expand Up @@ -276,24 +277,24 @@ impl<N: Network, AD: AuthorityDiscovery> Service<N, AD> {
// ask the network to connect to these nodes and not disconnect
// from them until removed from the set
if let Err(e) = network_service.add_peers_to_reserved_set(
super::COLLATION_PROTOCOL_NAME.into(),
PeerSet::Collation.into_protocol_name(),
multiaddr_to_add.clone(),
).await {
tracing::warn!(target: LOG_TARGET, err = ?e, "AuthorityDiscoveryService returned an invalid multiaddress");
}
if let Err(e) = network_service.add_peers_to_reserved_set(
super::VALIDATION_PROTOCOL_NAME.into(),
PeerSet::Validation.into_protocol_name(),
multiaddr_to_add,
).await {
tracing::warn!(target: LOG_TARGET, err = ?e, "AuthorityDiscoveryService returned an invalid multiaddress");
}
// the addresses are known to be valid
let _ = network_service.remove_peers_from_reserved_set(
super::COLLATION_PROTOCOL_NAME.into(),
PeerSet::Collation.into_protocol_name(),
multiaddr_to_remove.clone()
).await;
let _ = network_service.remove_peers_from_reserved_set(
super::VALIDATION_PROTOCOL_NAME.into(),
PeerSet::Validation.into_protocol_name(),
multiaddr_to_remove
).await;

Expand Down
13 changes: 4 additions & 9 deletions node/network/protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ pub use polkadot_node_jaeger::JaegerSpan;
#[doc(hidden)]
pub use std::sync::Arc;


/// Peer-sets and protocols used for parachains.
pub mod peer_set;

/// A unique identifier of a request.
pub type RequestId = u64;

Expand All @@ -47,15 +51,6 @@ impl fmt::Display for WrongVariant {

impl std::error::Error for WrongVariant {}

/// The peer-sets that the network manages. Different subsystems will use different peer-sets.
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum PeerSet {
/// The validation peer-set is responsible for all messages related to candidate validation and communication among validators.
Validation,
/// The collation peer-set is used for validator<>collator communication.
Collation,
}

/// The advertised role of a node.
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum ObservedRole {
Expand Down
91 changes: 91 additions & 0 deletions node/network/protocol/src/peer_set.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Copyright 2021 Parity Technologies (UK) Ltd.
// This file is part of Polkadot.

// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

//! All peersets and protocols used for parachains.
use sc_network::config::{NonDefaultSetConfig, SetConfig};
use std::borrow::Cow;

/// The peer-sets and thus the protocols which are used for the network.
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum PeerSet {
/// The validation peer-set is responsible for all messages related to candidate validation and communication among validators.
Validation,
/// The collation peer-set is used for validator<>collator communication.
Collation,
}

/// Protocol name as understood in substrate.
///
/// Ideally this would be defined in substrate as a newtype.
type ProtocolName = Cow<'static, str>;

impl PeerSet {
/// Get `sc_network` peer set configurations for each peerset.
///
/// Those should be used in the network configuration to register the protocols with the
/// network service.
pub fn get_info(self) -> NonDefaultSetConfig {
let protocol = self.into_protocol_name();
match self {
PeerSet::Validation => NonDefaultSetConfig {
notifications_protocol: protocol,
set_config: sc_network::config::SetConfig {
in_peers: 25,
out_peers: 0,
reserved_nodes: Vec::new(),
non_reserved_mode: sc_network::config::NonReservedPeerMode::Accept,
},
},
PeerSet::Collation => NonDefaultSetConfig {
notifications_protocol: protocol,
set_config: SetConfig {
in_peers: 25,
out_peers: 0,
reserved_nodes: Vec::new(),
non_reserved_mode: sc_network::config::NonReservedPeerMode::Accept,
},
},
}
}

/// Get the protocol name associated with each peer set as static str.
pub const fn get_protocol_name_static(self) -> &'static str {
match self {
PeerSet::Validation => "/polkadot/validation/1",
PeerSet::Collation => "/polkadot/collation/1",
}
}

/// Convert a peer set into a protocol name as understood by Substrate.
///
/// With `ProtocolName` being a proper newtype we could use the `Into` trait here.
pub fn into_protocol_name(self) -> ProtocolName {
self.get_protocol_name_static().into()
}

/// Try parsing a protocol name into a peer set.
///
/// If ProtocolName was a newtype, this would actually be nice to implement in terms of the
/// standard `TryFrom` trait.
pub fn try_from_protocol_name(name: &ProtocolName) -> Option<PeerSet> {
match name {
n if n == &PeerSet::Validation.into_protocol_name() => Some(PeerSet::Validation),
n if n == &PeerSet::Collation.into_protocol_name() => Some(PeerSet::Collation),
_ => None,
}
}
}

0 comments on commit 56e1d78

Please sign in to comment.