Remove sc_network::NetworkService::register_notifications_protocol and partially refactor Grandpa tests

bin/node-template/node/src/service.rs

@@ -79,13 +79,15 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen
 }
 
 /// Builds a new service for a full client.
-pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
+pub fn new_full(mut config: Configuration) -> Result<TaskManager, ServiceError> {
  let sc_service::PartialComponents {
  client, backend, mut task_manager, import_queue, keystore_container,
  select_chain, transaction_pool, inherent_data_providers,
  other: (block_import, grandpa_link),
  } = new_partial(&config)?;
 
+ config.network.notifications_protocols.push(sc_finality_grandpa::GRANDPA_PROTOCOL_NAME.into());
+
  let (network, network_status_sinks, system_rpc_tx, network_starter) =
  sc_service::build_network(sc_service::BuildNetworkParams {
  config: &config,
@@ -210,19 +212,19 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
  "grandpa-voter",
  sc_finality_grandpa::run_grandpa_voter(grandpa_config)?
  );
- } else {
- sc_finality_grandpa::setup_disabled_grandpa(network)?;
  }
 
  network_starter.start_network();
  Ok(task_manager)
 }
 
 /// Builds a new service for a light client.
-pub fn new_light(config: Configuration) -> Result<TaskManager, ServiceError> {
+pub fn new_light(mut config: Configuration) -> Result<TaskManager, ServiceError> {
  let (client, backend, keystore_container, mut task_manager, on_demand) =
  sc_service::new_light_parts::<Block, RuntimeApi, Executor>(&config)?;
 
+ config.network.notifications_protocols.push(sc_finality_grandpa::GRANDPA_PROTOCOL_NAME.into());
+
  let select_chain = sc_consensus::LongestChain::new(backend.clone());
 
  let transaction_pool = Arc::new(sc_transaction_pool::BasicPool::new_light(

bin/node/cli/src/service.rs

@@ -164,7 +164,7 @@ pub struct NewFullBase {
 
 /// Creates a full service from the configuration.
 pub fn new_full_base(
- config: Configuration,
+ mut config: Configuration,
  with_startup_data: impl FnOnce(
  &sc_consensus_babe::BabeBlockImport<Block, FullClient, FullGrandpaBlockImport>,
  &sc_consensus_babe::BabeLink<Block>,
@@ -178,6 +178,8 @@ pub fn new_full_base(
 
  let shared_voter_state = rpc_setup;
 
+ config.network.notifications_protocols.push(grandpa::GRANDPA_PROTOCOL_NAME.into());
+
  let (network, network_status_sinks, system_rpc_tx, network_starter) =
  sc_service::build_network(sc_service::BuildNetworkParams {
  config: &config,
@@ -315,8 +317,6 @@ pub fn new_full_base(
  "grandpa-voter",
  grandpa::run_grandpa_voter(grandpa_config)?
  );
- } else {
- grandpa::setup_disabled_grandpa(network.clone())?;
  }
 
  network_starter.start_network();
@@ -338,14 +338,16 @@ pub fn new_full(config: Configuration)
  })
 }
 
-pub fn new_light_base(config: Configuration) -> Result<(
+pub fn new_light_base(mut config: Configuration) -> Result<(
  TaskManager, RpcHandlers, Arc<LightClient>,
  Arc<NetworkService<Block, <Block as BlockT>::Hash>>,
  Arc<sc_transaction_pool::LightPool<Block, LightClient, sc_network::config::OnDemand<Block>>>
 ), ServiceError> {
  let (client, backend, keystore_container, mut task_manager, on_demand) =
  sc_service::new_light_parts::<Block, RuntimeApi, Executor>(&config)?;
 
+ config.network.notifications_protocols.push(grandpa::GRANDPA_PROTOCOL_NAME.into());
+
  let select_chain = sc_consensus::LongestChain::new(backend.clone());
 
  let transaction_pool = Arc::new(sc_transaction_pool::BasicPool::new_light(

client/finality-grandpa/src/communication/mod.rs

@@ -68,6 +68,8 @@ mod periodic;
 #[cfg(test)]
 pub(crate) mod tests;
 
+/// Name of the notifications protocol used by Grandpa. Must be registered towards the networking
+/// in order for Grandpa to properly function.
 pub const GRANDPA_PROTOCOL_NAME: &'static str = "/paritytech/grandpa/1";
 
 // cost scalars for reporting peers.

client/finality-grandpa/src/communication/tests.rs

@@ -62,8 +62,6 @@ impl sc_network_gossip::Network<Block> for TestNetwork {
  let _ = self.sender.unbounded_send(Event::WriteNotification(who, message));
  }
 
- fn register_notifications_protocol(&self, _: Cow<'static, str>) {}
-
  fn announce(&self, block: Hash, _associated_data: Vec<u8>) {
  let _ = self.sender.unbounded_send(Event::Announce(block));
  }

client/finality-grandpa/src/lib.rs

@@ -122,6 +122,7 @@ mod until_imported;
 mod voting_rule;
 
 pub use authorities::{SharedAuthoritySet, AuthoritySet};
+pub use communication::GRANDPA_PROTOCOL_NAME;
 pub use finality_proof::{FinalityProofFragment, FinalityProofProvider, StorageAndProofProvider};
 pub use notification::{GrandpaJustificationSender, GrandpaJustificationStream};
 pub use import::GrandpaBlockImport;
@@ -652,6 +653,10 @@ pub struct GrandpaParams<Block: BlockT, C, N, SC, VR> {
  /// A link to the block import worker.
  pub link: LinkHalf<Block, C, SC>,
  /// The Network instance.
+ ///
+ /// It is assumed that this network will feed us Grandpa notifications. When using the
+ /// `sc_network` crate, it is assumed that the Grandpa notifications protocol has been passed
+ /// to the configuration of the networking.
  pub network: N,
  /// If supplied, can be used to hook on telemetry connection established events.
  pub telemetry_on_connect: Option<TracingUnboundedReceiver<()>>,
@@ -1065,26 +1070,6 @@ where
  }
 }
 
-/// When GRANDPA is not initialized we still need to register the finality
-/// tracker inherent provider which might be expected by the runtime for block
-/// authoring. Additionally, we register a gossip message validator that
-/// discards all GRANDPA messages (otherwise, we end up banning nodes that send
-/// us a `Neighbor` message, since there is no registered gossip validator for
-/// the engine id defined in the message.)
-pub fn setup_disabled_grandpa<Block: BlockT, N>(network: N) -> Result<(), sp_consensus::Error>
-where
- N: NetworkT<Block> + Send + Clone + 'static,
-{
- // We register the GRANDPA protocol so that we don't consider it an anomaly
- // to receive GRANDPA messages on the network. We don't process the
- // messages.
- network.register_notifications_protocol(
- From::from(communication::GRANDPA_PROTOCOL_NAME),
- );
-
- Ok(())
-}
-
 /// Checks if this node has any available keys in the keystore for any authority id in the given
 /// voter set. Returns the authority id for which keys are available, or `None` if no keys are
 /// available.