diff --git a/substrate/bin/node-template/node/src/service.rs b/substrate/bin/node-template/node/src/service.rs
index 339aafbefcdf4775a234b21accb8e5feff1b256c..021d0ac8f7d1e74f8d95d200f4b6a672fbdb4d81 100644
--- a/substrate/bin/node-template/node/src/service.rs
+++ b/substrate/bin/node-template/node/src/service.rs
@@ -83,7 +83,7 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
let finality_proof_provider =
GrandpaFinalityProofProvider::new_for_service(backend.clone(), client.clone());
- let (network, network_status_sinks, system_rpc_tx) =
+ let (network, network_status_sinks, system_rpc_tx, network_starter) =
sc_service::build_network(sc_service::BuildNetworkParams {
config: &config,
client: client.clone(),
@@ -215,6 +215,7 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
)?;
}
+ network_starter.start_network();
Ok(task_manager)
}
@@ -253,7 +254,7 @@ pub fn new_light(config: Configuration) -> Result<TaskManager, ServiceError> {
let finality_proof_provider =
GrandpaFinalityProofProvider::new_for_service(backend.clone(), client.clone());
- let (network, network_status_sinks, system_rpc_tx) =
+ let (network, network_status_sinks, system_rpc_tx, network_starter) =
sc_service::build_network(sc_service::BuildNetworkParams {
config: &config,
client: client.clone(),
@@ -288,5 +289,7 @@ pub fn new_light(config: Configuration) -> Result<TaskManager, ServiceError> {
system_rpc_tx,
})?;
+ network_starter.start_network();
+
Ok(task_manager)
}
diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs
index a47869ed83232bed0fb5c51bba0263a5ab90bd96..fdfa6816296aaccfc2676383e250ac4b5aeb60d2 100644
--- a/substrate/bin/node/cli/src/service.rs
+++ b/substrate/bin/node/cli/src/service.rs
@@ -164,8 +164,8 @@ pub fn new_full_base(
let finality_proof_provider =
GrandpaFinalityProofProvider::new_for_service(backend.clone(), client.clone());
-
- let (network, network_status_sinks, system_rpc_tx) =
+
+ let (network, network_status_sinks, system_rpc_tx, network_starter) =
sc_service::build_network(sc_service::BuildNetworkParams {
config: &config,
client: client.clone(),
@@ -206,7 +206,7 @@ pub fn new_full_base(
network_status_sinks,
system_rpc_tx,
})?;
-
+
let (block_import, grandpa_link, babe_link) = import_setup;
let shared_voter_state = rpc_setup;
@@ -322,6 +322,7 @@ pub fn new_full_base(
)?;
}
+ network_starter.start_network();
Ok((task_manager, inherent_data_providers, client, network, transaction_pool))
}
@@ -383,7 +384,7 @@ pub fn new_light_base(config: Configuration) -> Result<(
let finality_proof_provider =
GrandpaFinalityProofProvider::new_for_service(backend.clone(), client.clone());
- let (network, network_status_sinks, system_rpc_tx) =
+ let (network, network_status_sinks, system_rpc_tx, network_starter) =
sc_service::build_network(sc_service::BuildNetworkParams {
config: &config,
client: client.clone(),
@@ -395,7 +396,8 @@ pub fn new_light_base(config: Configuration) -> Result<(
finality_proof_request_builder: Some(finality_proof_request_builder),
finality_proof_provider: Some(finality_proof_provider),
})?;
-
+ network_starter.start_network();
+
if config.offchain_worker.enabled {
sc_service::build_offchain_workers(
&config, backend.clone(), task_manager.spawn_handle(), client.clone(), network.clone(),
@@ -412,7 +414,7 @@ pub fn new_light_base(config: Configuration) -> Result<(
let rpc_extensions = node_rpc::create_light(light_deps);
let rpc_handlers =
- sc_service::spawn_tasks(sc_service::SpawnTasksParams {
+ sc_service::spawn_tasks(sc_service::SpawnTasksParams {
on_demand: Some(on_demand),
remote_blockchain: Some(backend.remote_blockchain()),
rpc_extensions_builder: Box::new(sc_service::NoopRpcExtensionBuilder(rpc_extensions)),
@@ -423,7 +425,7 @@ pub fn new_light_base(config: Configuration) -> Result<(
telemetry_connection_sinks: sc_service::TelemetryConnectionSinks::default(),
task_manager: &mut task_manager,
})?;
-
+
Ok((task_manager, rpc_handlers, client, network, transaction_pool))
}
@@ -498,7 +500,7 @@ mod tests {
setup_handles = Some((block_import.clone(), babe_link.clone()));
}
)?;
-
+
let node = sc_service_test::TestNetComponents::new(
keep_alive, client, network, transaction_pool
);
diff --git a/substrate/client/service/src/builder.rs b/substrate/client/service/src/builder.rs
index 4c7c1f57ee04bbfd3b32e48445bf7a53f85ae153..fc9303498d674edf228e1e554c07347f40618935 100644
--- a/substrate/client/service/src/builder.rs
+++ b/substrate/client/service/src/builder.rs
@@ -33,7 +33,7 @@ use sp_consensus::{
block_validation::{BlockAnnounceValidator, DefaultBlockAnnounceValidator, Chain},
import_queue::ImportQueue,
};
-use futures::{FutureExt, StreamExt, future::ready};
+use futures::{FutureExt, StreamExt, future::ready, channel::oneshot};
use jsonrpc_pubsub::manager::SubscriptionManager;
use sc_keystore::Store as Keystore;
use log::{info, warn, error};
@@ -668,7 +668,7 @@ fn build_telemetry<TBl: BlockT>(
let startup_time = SystemTime::UNIX_EPOCH.elapsed()
.map(|dur| dur.as_millis())
.unwrap_or(0);
-
+
spawn_handle.spawn(
"telemetry-worker",
telemetry.clone()
@@ -822,6 +822,7 @@ pub fn build_network<TBl, TExPool, TImpQu, TCl>(
Arc<NetworkService<TBl, <TBl as BlockT>::Hash>>,
NetworkStatusSinks<TBl>,
TracingUnboundedSender<sc_rpc::system::Request<TBl>>,
+ NetworkStarter,
),
Error
>
@@ -900,6 +901,22 @@ pub fn build_network<TBl, TExPool, TImpQu, TCl>(
config.announce_block,
);
+ // TODO: Normally, one is supposed to pass a list of notifications protocols supported by the
+ // node through the `NetworkConfiguration` struct. But because this function doesn't know in
+ // advance which components, such as GrandPa or Polkadot, will be plugged on top of the
+ // service, it is unfortunately not possible to do so without some deep refactoring. To bypass
+ // this problem, the `NetworkService` provides a `register_notifications_protocol` method that
+ // can be called even after the network has been initialized. However, we want to avoid the
+ // situation where `register_notifications_protocol` is called *after* the network actually
+ // connects to other peers. For this reason, we delay the process of the network future until
+ // the user calls `NetworkStarter::start_network`.
+ //
+ // This entire hack should eventually be removed in favour of passing the list of protocols
+ // through the configuration.
+ //
+ // See also https://github.com/paritytech/substrate/issues/6827
+ let (network_start_tx, network_start_rx) = oneshot::channel();
+
// The network worker is responsible for gathering all network messages and processing
// them. This is quite a heavy task, and at the time of the writing of this comment it
// frequently happens that this future takes several seconds or in some situations
@@ -907,7 +924,32 @@ pub fn build_network<TBl, TExPool, TImpQu, TCl>(
// issue, and ideally we would like to fix the network future to take as little time as
// possible, but we also take the extra harm-prevention measure to execute the networking
// future using `spawn_blocking`.
- spawn_handle.spawn_blocking("network-worker", future);
+ spawn_handle.spawn_blocking("network-worker", async move {
+ if network_start_rx.await.is_err() {
+ debug_assert!(false);
+ log::warn!(
+ "The NetworkStart returned as part of `build_network` has been silently dropped"
+ );
+ // This `return` might seem unnecessary, but we don't want to make it look like
+ // everything is working as normal even though the user is clearly misusing the API.
+ return;
+ }
+
+ future.await
+ });
+
+ Ok((network, network_status_sinks, system_rpc_tx, NetworkStarter(network_start_tx)))
+}
- Ok((network, network_status_sinks, system_rpc_tx))
+/// Object used to start the network.
+#[must_use]
+pub struct NetworkStarter(oneshot::Sender<()>);
+
+impl NetworkStarter {
+ /// Start the network. Call this after all sub-components have been initialized.
+ ///
+ /// > **Note**: If you don't call this function, the networking will not work.
+ pub fn start_network(self) {
+ let _ = self.0.send(());
+ }
}
diff --git a/substrate/client/service/src/lib.rs b/substrate/client/service/src/lib.rs
index 40826a70d45015454e4f1f2bba9dfd8ca4405f3e..415a5de4f930da5cae93156a8bab663636f67bcf 100644
--- a/substrate/client/service/src/lib.rs
+++ b/substrate/client/service/src/lib.rs
@@ -53,7 +53,7 @@ use sp_utils::{status_sinks, mpsc::{tracing_unbounded, TracingUnboundedReceiver,
pub use self::error::Error;
pub use self::builder::{
new_full_client, new_client, new_full_parts, new_light_parts,
- spawn_tasks, build_network, BuildNetworkParams, build_offchain_workers,
+ spawn_tasks, build_network, BuildNetworkParams, NetworkStarter, build_offchain_workers,
SpawnTasksParams, TFullClient, TLightClient, TFullBackend, TLightBackend,
TLightBackendWithHash, TLightClientWithBackend,
TFullCallExecutor, TLightCallExecutor, RpcExtensionBuilder, NoopRpcExtensionBuilder,