Unverified Commit c5940800 authored by Pierre Krieger's avatar Pierre Krieger Committed by GitHub
Browse files

Companion PR for refactoring priority groups (#2095)

* Companion PR for refactoring priority groups

* Fix non reserved node

* Try fix tests

* Missing import

* Fix warning

* Change protocols order

* Fix test

* Renames

* Update syn dependency to make it compile again after merging master

* "Update Substrate"

Co-authored-by: parity-processbot <>
parent 6f221717
Pipeline #119145 passed with stages
in 31 minutes and 38 seconds
This diff is collapsed.
......@@ -80,12 +80,28 @@ pub enum WireMessage<M> {
ViewUpdate(View),
}
/// Information about the notifications protocol. Should be used during network configuration
/// or shortly after startup to register the protocol with the network service.
pub fn notifications_protocol_info() -> Vec<std::borrow::Cow<'static, str>> {
/// 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![
VALIDATION_PROTOCOL_NAME.into(),
COLLATION_PROTOCOL_NAME.into(),
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,
},
}
]
}
......@@ -202,7 +218,7 @@ impl<N, AD> NetworkBridge<N, AD> {
/// Create a new network bridge subsystem with underlying network service and authority discovery service.
///
/// This assumes that the network service has had the notifications protocol for the network
/// bridge already registered. See [`notifications_protocol_info`](notifications_protocol_info).
/// bridge already registered. See [`peers_sets_info`](peers_sets_info).
pub fn new(network_service: N, authority_discovery_service: AD) -> Self {
NetworkBridge {
network_service,
......@@ -300,7 +316,9 @@ fn action_from_network_message(event: Option<NetworkEvent>) -> Action {
tracing::info!(target: LOG_TARGET, "Shutting down Network Bridge: underlying event stream concluded");
Action::Abort
}
Some(NetworkEvent::Dht(_)) => Action::Nop,
Some(NetworkEvent::Dht(_)) |
Some(NetworkEvent::SyncConnected { .. }) |
Some(NetworkEvent::SyncDisconnected { .. }) => Action::Nop,
Some(NetworkEvent::NotificationStreamOpened { remote, protocol, role }) => {
let role = role.into();
match protocol {
......@@ -755,6 +773,7 @@ mod tests {
use futures::channel::mpsc;
use futures::executor;
use std::borrow::Cow;
use std::sync::Arc;
use std::collections::HashSet;
use async_trait::async_trait;
......@@ -829,11 +848,11 @@ mod tests {
#[async_trait]
impl validator_discovery::Network for TestNetwork {
async fn add_to_priority_group(&mut self, _group_id: String, _multiaddresses: HashSet<Multiaddr>) -> Result<(), String> {
async fn add_peers_to_reserved_set(&mut self, _protocol: Cow<'static, str>, _: HashSet<Multiaddr>) -> Result<(), String> {
Ok(())
}
async fn remove_from_priority_group(&mut self, _group_id: String, _multiaddresses: HashSet<Multiaddr>) -> Result<(), String> {
async fn remove_peers_from_reserved_set(&mut self, _protocol: Cow<'static, str>, _: HashSet<Multiaddr>) -> Result<(), String> {
Ok(())
}
}
......
......@@ -17,6 +17,7 @@
//! A validator discovery service for the Network Bridge.
use core::marker::PhantomData;
use std::borrow::Cow;
use std::collections::{HashSet, HashMap, hash_map};
use std::sync::Arc;
......@@ -28,16 +29,15 @@ use sc_authority_discovery::Service as AuthorityDiscoveryService;
use polkadot_node_network_protocol::PeerId;
use polkadot_primitives::v1::{AuthorityDiscoveryId, Block, Hash};
const PRIORITY_GROUP: &'static str = "parachain_validators";
const LOG_TARGET: &str = "validator_discovery";
/// An abstraction over networking for the purposes of validator discovery service.
#[async_trait]
pub trait Network: Send + 'static {
/// Ask the network to connect to these nodes and not disconnect from them until removed from the priority group.
async fn add_to_priority_group(&mut self, group_id: String, multiaddresses: HashSet<Multiaddr>) -> Result<(), String>;
async fn add_peers_to_reserved_set(&mut self, protocol: Cow<'static, str>, multiaddresses: HashSet<Multiaddr>) -> Result<(), String>;
/// Remove the peers from the priority group.
async fn remove_from_priority_group(&mut self, group_id: String, multiaddresses: HashSet<Multiaddr>) -> Result<(), String>;
async fn remove_peers_from_reserved_set(&mut self, protocol: Cow<'static, str>, multiaddresses: HashSet<Multiaddr>) -> Result<(), String>;
}
/// An abstraction over the authority discovery service.
......@@ -51,12 +51,12 @@ pub trait AuthorityDiscovery: Send + 'static {
#[async_trait]
impl Network for Arc<sc_network::NetworkService<Block, Hash>> {
async fn add_to_priority_group(&mut self, group_id: String, multiaddresses: HashSet<Multiaddr>) -> Result<(), String> {
sc_network::NetworkService::add_to_priority_group(&**self, group_id, multiaddresses).await
async fn add_peers_to_reserved_set(&mut self, protocol: Cow<'static, str>, multiaddresses: HashSet<Multiaddr>) -> Result<(), String> {
sc_network::NetworkService::add_peers_to_reserved_set(&**self, protocol, multiaddresses)
}
async fn remove_from_priority_group(&mut self, group_id: String, multiaddresses: HashSet<Multiaddr>) -> Result<(), String> {
sc_network::NetworkService::remove_from_priority_group(&**self, group_id, multiaddresses).await
async fn remove_peers_from_reserved_set(&mut self, protocol: Cow<'static, str>, multiaddresses: HashSet<Multiaddr>) -> Result<(), String> {
sc_network::NetworkService::remove_peers_from_reserved_set(&**self, protocol, multiaddresses)
}
}
......@@ -274,15 +274,28 @@ 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 priority group
if let Err(e) = network_service.add_to_priority_group(
PRIORITY_GROUP.to_owned(),
// from them until removed from the set
if let Err(e) = network_service.add_peers_to_reserved_set(
super::COLLATION_PROTOCOL_NAME.into(),
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(),
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_from_priority_group(PRIORITY_GROUP.to_owned(), multiaddr_to_remove).await;
let _ = network_service.remove_peers_from_reserved_set(
super::COLLATION_PROTOCOL_NAME.into(),
multiaddr_to_remove.clone()
).await;
let _ = network_service.remove_peers_from_reserved_set(
super::VALIDATION_PROTOCOL_NAME.into(),
multiaddr_to_remove
).await;
let pending = validator_ids.iter()
.cloned()
......@@ -339,7 +352,7 @@ mod tests {
#[derive(Default)]
struct TestNetwork {
priority_group: HashSet<Multiaddr>,
peers_set: HashSet<Multiaddr>,
}
#[derive(Default)]
......@@ -367,13 +380,13 @@ mod tests {
#[async_trait]
impl Network for TestNetwork {
async fn add_to_priority_group(&mut self, _group_id: String, multiaddresses: HashSet<Multiaddr>) -> Result<(), String> {
self.priority_group.extend(multiaddresses.into_iter());
async fn add_peers_to_reserved_set(&mut self, _protocol: Cow<'static, str>, multiaddresses: HashSet<Multiaddr>) -> Result<(), String> {
self.peers_set.extend(multiaddresses.into_iter());
Ok(())
}
async fn remove_from_priority_group(&mut self, _group_id: String, multiaddresses: HashSet<Multiaddr>) -> Result<(), String> {
self.priority_group.retain(|elem| !multiaddresses.contains(elem));
async fn remove_peers_from_reserved_set(&mut self, _protocol: Cow<'static, str>, multiaddresses: HashSet<Multiaddr>) -> Result<(), String> {
self.peers_set.retain(|elem| !multiaddresses.contains(elem));
Ok(())
}
}
......@@ -570,7 +583,7 @@ mod tests {
let _ = receiver.next().await.unwrap();
assert_eq!(service.non_revoked_discovery_requests.len(), 1);
assert_eq!(ns.priority_group.len(), 2);
assert_eq!(ns.peers_set.len(), 2);
// revoke the second request
drop(receiver);
......@@ -586,7 +599,7 @@ mod tests {
let _ = receiver.next().await.unwrap();
assert_eq!(service.non_revoked_discovery_requests.len(), 1);
assert_eq!(ns.priority_group.len(), 1);
assert_eq!(ns.peers_set.len(), 1);
});
}
......
......@@ -562,9 +562,12 @@ pub fn new_full<RuntimeApi, Executor>(
let shared_voter_state = rpc_setup;
// Note: GrandPa is pushed before the Polkadot-specific protocols. This doesn't change
// anything in terms of behaviour, but makes the logs more consistent with the other
// Substrate nodes.
config.network.extra_sets.push(grandpa::grandpa_peers_set_config());
#[cfg(feature = "real-overseer")]
config.network.notifications_protocols.extend(polkadot_network_bridge::notifications_protocol_info());
config.network.notifications_protocols.push(grandpa::GRANDPA_PROTOCOL_NAME.into());
config.network.extra_sets.extend(polkadot_network_bridge::peers_sets_info());
let (network, network_status_sinks, system_rpc_tx, network_starter) =
service::build_network(service::BuildNetworkParams {
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment