From 9a14ba0555bd4ec35f84fbc1fea9a703527ae46d Mon Sep 17 00:00:00 2001
From: Svyatoslav Nikolsky <svyatonik@gmail.com>
Date: Tue, 14 May 2019 14:38:30 +0300
Subject: [PATCH] Fix light client startup (build_select_chain returns
 Result<Option>) (#2574)

* fix light client strtup (build_select_chain -> Option)

* fixed node template compilation

* Update core/service/src/lib.rs

Co-Authored-By: Benjamin Kampmann <ben.kampmann@googlemail.com>
---
 substrate/core/service/src/components.rs | 20 +++++++++++---------
 substrate/core/service/src/error.rs      |  4 ++++
 substrate/core/service/src/lib.rs        | 13 ++++++-------
 substrate/node-template/src/service.rs   |  5 ++++-
 substrate/node/cli/src/service.rs        | 23 +++++++++++++----------
 5 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/substrate/core/service/src/components.rs b/substrate/core/service/src/components.rs
index 7bca1cc3e5..48ea9777bd 100644
--- a/substrate/core/service/src/components.rs
+++ b/substrate/core/service/src/components.rs
@@ -344,7 +344,7 @@ pub trait ServiceFactory: 'static + Sized {
 	fn build_full_import_queue(
 		config: &mut FactoryFullConfiguration<Self>,
 		_client: Arc<FullClient<Self>>,
-		_select_chain: Self::SelectChain
+		_select_chain: Self::SelectChain,
 	) -> Result<Self::FullImportQueue, error::Error> {
 		if let Some(name) = config.chain_spec.consensus_engine() {
 			match name {
@@ -415,7 +415,7 @@ pub trait Components: Sized + 'static {
 	fn build_import_queue(
 		config: &mut FactoryFullConfiguration<Self::Factory>,
 		client: Arc<ComponentClient<Self>>,
-		select_chain: Self::SelectChain,
+		select_chain: Option<Self::SelectChain>,
 	) -> Result<Self::ImportQueue, error::Error>;
 
 	/// Finality proof provider for serving network requests.
@@ -427,7 +427,7 @@ pub trait Components: Sized + 'static {
 	fn build_select_chain(
 		config: &mut FactoryFullConfiguration<Self::Factory>,
 		client: Arc<ComponentClient<Self>>
-	) -> Result<Self::SelectChain, error::Error>;
+	) -> Result<Option<Self::SelectChain>, error::Error>;
 }
 
 /// A struct that implement `Components` for the full client.
@@ -506,16 +506,18 @@ impl<Factory: ServiceFactory> Components for FullComponents<Factory> {
 	fn build_import_queue(
 		config: &mut FactoryFullConfiguration<Self::Factory>,
 		client: Arc<ComponentClient<Self>>,
-		select_chain: Self::SelectChain,
+		select_chain: Option<Self::SelectChain>,
 	) -> Result<Self::ImportQueue, error::Error> {
+		let select_chain = select_chain
+			.ok_or_else(|| error::Error::from(error::ErrorKind::SelectChainRequired))?;
 		Factory::build_full_import_queue(config, client, select_chain)
 	}
 
 	fn build_select_chain(
 		config: &mut FactoryFullConfiguration<Self::Factory>,
 		client: Arc<ComponentClient<Self>>
-	) -> Result<Self::SelectChain, error::Error> {
-		Self::Factory::build_select_chain(config, client)
+	) -> Result<Option<Self::SelectChain>, error::Error> {
+		Self::Factory::build_select_chain(config, client).map(Some)
 	}
 
 	fn build_finality_proof_provider(
@@ -596,7 +598,7 @@ impl<Factory: ServiceFactory> Components for LightComponents<Factory> {
 	fn build_import_queue(
 		config: &mut FactoryFullConfiguration<Self::Factory>,
 		client: Arc<ComponentClient<Self>>,
-		_select_chain: Self::SelectChain,
+		_select_chain: Option<Self::SelectChain>,
 	) -> Result<Self::ImportQueue, error::Error> {
 		Factory::build_light_import_queue(config, client)
 	}
@@ -609,8 +611,8 @@ impl<Factory: ServiceFactory> Components for LightComponents<Factory> {
 	fn build_select_chain(
 		_config: &mut FactoryFullConfiguration<Self::Factory>,
 		_client: Arc<ComponentClient<Self>>
-	) -> Result<Self::SelectChain, error::Error> {
-		Err("Fork choice doesn't happen on light clients.".into())
+	) -> Result<Option<Self::SelectChain>, error::Error> {
+		Ok(None)
 	}
 }
 
diff --git a/substrate/core/service/src/error.rs b/substrate/core/service/src/error.rs
index 3155d781ad..c5bcd61a3c 100644
--- a/substrate/core/service/src/error.rs
+++ b/substrate/core/service/src/error.rs
@@ -39,5 +39,9 @@ error_chain! {
 	}
 
 	errors {
+		SelectChainRequired {
+			description("Best chain selection strategy (SelectChain) must be provided when starting full node or authority."),
+			display("Best chain selection strategy (SelectChain) is not provided."),
+		}
 	}
 }
diff --git a/substrate/core/service/src/lib.rs b/substrate/core/service/src/lib.rs
index e99eeade7c..70b2a435a8 100644
--- a/substrate/core/service/src/lib.rs
+++ b/substrate/core/service/src/lib.rs
@@ -20,10 +20,10 @@
 #![warn(missing_docs)]
 
 mod components;
-mod error;
 mod chain_spec;
 pub mod config;
 pub mod chain_ops;
+pub mod error;
 
 use std::io;
 use std::net::SocketAddr;
@@ -42,7 +42,6 @@ use primitives::Pair;
 use runtime_primitives::generic::BlockId;
 use runtime_primitives::traits::{Header, As};
 use substrate_executor::NativeExecutor;
-use consensus_common::SelectChain;
 use tel::{telemetry, SUBSTRATE_INFO};
 
 pub use self::error::{ErrorKind, Error};
@@ -74,7 +73,7 @@ const DEFAULT_PROTOCOL_ID: &str = "sup";
 /// Substrate service.
 pub struct Service<Components: components::Components> {
 	client: Arc<ComponentClient<Components>>,
-	select_chain: <Components as components::Components>::SelectChain,
+	select_chain: Option<<Components as components::Components>::SelectChain>,
 	network: Option<Arc<components::NetworkService<Components::Factory>>>,
 	transaction_pool: Arc<TransactionPool<Components::TransactionPoolApi>>,
 	inherents_pool: Arc<InherentsPool<ComponentExtrinsic<Components>>>,
@@ -159,11 +158,11 @@ impl<Components: components::Components> Service<Components> {
 			select_chain.clone(),
 		)?);
 		let finality_proof_provider = Components::build_finality_proof_provider(client.clone())?;
-		let best_header = select_chain.best_chain()?;
+		let chain_info = client.info()?.chain;
 
 		let version = config.full_version();
-		info!("Best block: #{}", best_header.number());
-		telemetry!(SUBSTRATE_INFO; "node.start"; "height" => best_header.number().as_(), "best" => ?best_header.hash());
+		info!("Highest known block at #{}", chain_info.best_number);
+		telemetry!(SUBSTRATE_INFO; "node.start"; "height" => chain_info.best_number.as_(), "best" => ?chain_info.best_hash);
 
 		let network_protocol = <Components::Factory>::build_network_protocol(&config)?;
 		let transaction_pool = Arc::new(
@@ -404,7 +403,7 @@ impl<Components> Service<Components> where Components: components::Components {
 	}
 
 	/// Get clone of select chain.
-	pub fn select_chain(&self) -> <Components as components::Components>::SelectChain {
+	pub fn select_chain(&self) -> Option<<Components as components::Components>::SelectChain> {
 		self.select_chain.clone()
 	}
 
diff --git a/substrate/node-template/src/service.rs b/substrate/node-template/src/service.rs
index f964e899d2..7a6a3f0c27 100644
--- a/substrate/node-template/src/service.rs
+++ b/substrate/node-template/src/service.rs
@@ -10,6 +10,7 @@ use substrate_service::{
 	FactoryFullConfiguration, LightComponents, FullComponents, FullBackend,
 	FullClient, LightClient, LightBackend, FullExecutor, LightExecutor,
 	TaskExecutor,
+	error::{Error as ServiceError, ErrorKind as ServiceErrorKind},
 };
 use basic_authorship::ProposerFactory;
 use consensus::{import_queue, start_aura, AuraImportQueue, SlotDuration, NothingExtra};
@@ -65,11 +66,13 @@ construct_service_factory! {
 						inherents_pool: service.inherents_pool(),
 					});
 					let client = service.client();
+					let select_chain = service.select_chain()
+						.ok_or_else(|| ServiceError::from(ServiceErrorKind::SelectChainRequired))?;
 					executor.spawn(start_aura(
 						SlotDuration::get_or_compute(&*client)?,
 						key.clone(),
 						client.clone(),
-						service.select_chain(),
+						select_chain,
 						client,
 						proposer,
 						service.network(),
diff --git a/substrate/node/cli/src/service.rs b/substrate/node/cli/src/service.rs
index 42b3faf236..90041b4cbb 100644
--- a/substrate/node/cli/src/service.rs
+++ b/substrate/node/cli/src/service.rs
@@ -31,6 +31,7 @@ use node_runtime::{GenesisConfig, RuntimeApi};
 use substrate_service::{
 	FactoryFullConfiguration, LightComponents, FullComponents, FullBackend,
 	FullClient, LightClient, LightBackend, FullExecutor, LightExecutor, TaskExecutor,
+	error::{Error as ServiceError, ErrorKind as ServiceErrorKind},
 };
 use transaction_pool::{self, txpool::{Pool as TransactionPool}};
 use inherents::InherentDataProviders;
@@ -90,11 +91,13 @@ construct_service_factory! {
 					});
 
 					let client = service.client();
+					let select_chain = service.select_chain()
+						.ok_or_else(|| ServiceError::from(ServiceErrorKind::SelectChainRequired))?;
 					executor.spawn(start_aura(
 						SlotDuration::get_or_compute(&*client)?,
 						key.clone(),
 						client,
-						service.select_chain(),
+						select_chain,
 						block_import.clone(),
 						proposer,
 						service.network(),
@@ -131,17 +134,17 @@ construct_service_factory! {
 					},
 					Some(_) => {
 						let telemetry_on_connect = TelemetryOnConnect {
-						  on_exit: Box::new(service.on_exit()),
-						  telemetry_connection_sinks: service.telemetry_on_connect_stream(),
-						  executor: &executor,
+							on_exit: Box::new(service.on_exit()),
+							telemetry_connection_sinks: service.telemetry_on_connect_stream(),
+							executor: &executor,
 						};
 						let grandpa_config = grandpa::GrandpaParams {
-						  config: config,
-						  link: link_half,
-						  network: service.network(),
-						  inherent_data_providers: service.config.custom.inherent_data_providers.clone(),
-						  on_exit: service.on_exit(),
-						  telemetry_on_connect: Some(telemetry_on_connect),
+							config: config,
+							link: link_half,
+							network: service.network(),
+							inherent_data_providers: service.config.custom.inherent_data_providers.clone(),
+							on_exit: service.on_exit(),
+							telemetry_on_connect: Some(telemetry_on_connect),
 						};
 						executor.spawn(grandpa::run_grandpa_voter(grandpa_config)?);
 					},