Cleanup light client leftovers (#11865)

* Remove --light cli option

* Cleanup light client leftovers

* Remove commented-out code and clean-up more light client leftovers

* Fix formatting with `cargo +nightly fmt`

* Remove FIXME regarding db directory structure

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

substrate/client/cli/src/commands/run_cmd.rs

@@ -50,10 +50,6 @@ pub struct RunCmd {
 	#[clap(long)]
 	pub no_grandpa: bool,
 
-	/// Experimental: Run in light client mode.
-	#[clap(long)]
-	pub light: bool,
-
 	/// Listen to all RPC interfaces.
 	///
 	/// Default is local. Note: not all RPC methods are safe to be exposed publicly. Use an RPC
@@ -337,7 +333,7 @@ impl CliConfiguration for RunCmd {
 
 	fn dev_key_seed(&self, is_dev: bool) -> Result<Option<String>> {
 		Ok(self.get_keyring().map(|a| format!("//{}", a)).or_else(|| {
-			if is_dev && !self.light {
+			if is_dev {
 				Some("//Alice".into())
 			} else {
 				None
@@ -363,16 +359,9 @@ impl CliConfiguration for RunCmd {
 
 	fn role(&self, is_dev: bool) -> Result<Role> {
 		let keyring = self.get_keyring();
-		let is_light = self.light;
+		let is_authority = self.validator || is_dev || keyring.is_some();
-		let is_authority = (self.validator || is_dev || keyring.is_some()) && !is_light;
 
-		Ok(if is_light {
+		Ok(if is_authority { sc_service::Role::Authority } else { sc_service::Role::Full })
-			sc_service::Role::Light
-		} else if is_authority {
-			sc_service::Role::Authority
-		} else {
-			sc_service::Role::Full
-		})
 	}
 
 	fn force_authoring(&self) -> Result<bool> {

substrate/client/cli/src/config.rs

@@ -211,12 +211,8 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 		base_path: &PathBuf,
 		cache_size: usize,
 		database: Database,
-		role: &Role,
 	) -> Result<DatabaseSource> {
-		let role_dir = match role {
+		let role_dir = "full";
-			Role::Light => "light",
-			Role::Full | Role::Authority => "full",
-		};
 		let rocksdb_path = base_path.join("db").join(role_dir);
 		let paritydb_path = base_path.join("paritydb").join(role_dir);
 		Ok(match database {
@@ -536,7 +532,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
 			)?,
 			keystore_remote,
 			keystore,
-			database: self.database_config(&config_dir, database_cache_size, database, &role)?,
+			database: self.database_config(&config_dir, database_cache_size, database)?,
 			state_cache_size: self.state_cache_size()?,
 			state_cache_child_ratio: self.state_cache_child_ratio()?,
 			state_pruning: self.state_pruning()?,

substrate/client/finality-grandpa/src/communication/gossip.rs

@@ -728,11 +728,7 @@ type MaybeMessage<Block> = Option<(Vec<PeerId>, NeighborPacket<NumberFor<Block>>
 
 impl<Block: BlockT> Inner<Block> {
 	fn new(config: crate::Config) -> Self {
-		let catch_up_config = if config.local_role.is_light() {
+		let catch_up_config = if config.observer_enabled {
-			// if we are a light client we shouldn't be issuing any catch-up requests
-			// as we don't participate in the full GRANDPA protocol
-			CatchUpConfig::disabled()
-		} else if config.observer_enabled {
 			if config.local_role.is_authority() {
 				// since the observer protocol is enabled, we will only issue
 				// catch-up requests if we are an authority (and only to other
@@ -1231,10 +1227,6 @@ impl<Block: BlockT> Inner<Block> {
 			None => return false,
 		};
 
-		if self.config.local_role.is_light() {
-			return false
-		}
-
 		if round_elapsed < round_duration.mul_f32(PROPAGATION_SOME) {
 			self.peers.first_stage_peers.contains(who)
 		} else if round_elapsed < round_duration.mul_f32(PROPAGATION_ALL) {
@@ -1266,10 +1258,6 @@ impl<Block: BlockT> Inner<Block> {
 			None => return false,
 		};
 
-		if self.config.local_role.is_light() {
-			return false
-		}
-
 		if round_elapsed < round_duration.mul_f32(PROPAGATION_ALL) {
 			self.peers.first_stage_peers.contains(who) ||
 				self.peers.second_stage_peers.contains(who) ||

substrate/client/network/src/config.rs

@@ -147,8 +147,6 @@ where
 pub enum Role {
 	/// Regular full node.
 	Full,
-	/// Regular light node.
-	Light,
 	/// Actual authority.
 	Authority,
 }
@@ -158,18 +156,12 @@ impl Role {
 	pub fn is_authority(&self) -> bool {
 		matches!(self, Self::Authority { .. })
 	}
-
-	/// True for [`Role::Light`].
-	pub fn is_light(&self) -> bool {
-		matches!(self, Self::Light { .. })
-	}
 }
 
 impl fmt::Display for Role {
 	fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 		match self {
 			Self::Full => write!(f, "FULL"),
-			Self::Light => write!(f, "LIGHT"),
 			Self::Authority { .. } => write!(f, "AUTHORITY"),
 		}
 	}

substrate/client/network/src/protocol/message.rs

@@ -107,7 +107,6 @@ pub mod generic {
 		fn from(roles: &'a crate::config::Role) -> Self {
 			match roles {
 				crate::config::Role::Full => Self::FULL,
-				crate::config::Role::Light => Self::LIGHT,
 				crate::config::Role::Authority { .. } => Self::AUTHORITY,
 			}
 		}

substrate/client/network/src/service.rs

@@ -245,15 +245,11 @@ where
 		};
 
 		let chain_sync = (params.create_chain_sync)(
-			if params.role.is_light() {
+			match params.network_config.sync_mode {
-				SyncMode::Light
+				config::SyncMode::Full => SyncMode::Full,
-			} else {
+				config::SyncMode::Fast { skip_proofs, storage_chain_mode } =>
-				match params.network_config.sync_mode {
+					SyncMode::LightState { skip_proofs, storage_chain_mode },
-					config::SyncMode::Full => SyncMode::Full,
+				config::SyncMode::Warp => SyncMode::Warp,
-					config::SyncMode::Fast { skip_proofs, storage_chain_mode } =>
-						SyncMode::LightState { skip_proofs, storage_chain_mode },
-					config::SyncMode::Warp => SyncMode::Warp,
-				}
 			},
 			params.chain.clone(),
 			warp_sync_provider,
@@ -489,7 +485,6 @@ where
 
 		let (tx_handler, tx_handler_controller) = transactions_handler_proto.build(
 			service.clone(),
-			params.role,
 			params.transaction_pool,
 			params.metrics_registry.as_ref(),
 		)?;

substrate/client/network/src/transactions.rs

@@ -83,8 +83,6 @@ mod rep {
 	pub const GOOD_TRANSACTION: Rep = Rep::new(1 << 7, "Good transaction");
 	/// Reputation change when a peer sends us a bad transaction.
 	pub const BAD_TRANSACTION: Rep = Rep::new(-(1 << 12), "Bad transaction");
-	/// We received an unexpected transaction packet.
-	pub const UNEXPECTED_TRANSACTIONS: Rep = Rep::new_fatal("Unexpected transactions packet");
 }
 
 struct Metrics {
@@ -160,7 +158,6 @@ impl TransactionsHandlerPrototype {
 	pub fn build<B: BlockT + 'static, H: ExHashT>(
 		self,
 		service: Arc<NetworkService<B, H>>,
-		local_role: config::Role,
 		transaction_pool: Arc<dyn TransactionPool<H, B>>,
 		metrics_registry: Option<&Registry>,
 	) -> error::Result<(TransactionsHandler<B, H>, TransactionsHandlerController<H>)> {
@@ -178,7 +175,6 @@ impl TransactionsHandlerPrototype {
 			event_stream,
 			peers: HashMap::new(),
 			transaction_pool,
-			local_role,
 			from_controller,
 			metrics: if let Some(r) = metrics_registry {
 				Some(Metrics::register(r)?)
@@ -247,7 +243,6 @@ pub struct TransactionsHandler<B: BlockT + 'static, H: ExHashT> {
 	peers: HashMap<PeerId, Peer<H>>,
 	transaction_pool: Arc<dyn TransactionPool<H, B>>,
 	gossip_enabled: Arc<AtomicBool>,
-	local_role: config::Role,
 	from_controller: mpsc::UnboundedReceiver<ToHandler<H>>,
 	/// Prometheus metrics.
 	metrics: Option<Metrics>,
@@ -360,14 +355,6 @@ impl<B: BlockT + 'static, H: ExHashT> TransactionsHandler<B, H> {
 
 	/// Called when peer sends us new transactions
 	fn on_transactions(&mut self, who: PeerId, transactions: message::Transactions<B::Extrinsic>) {
-		// sending transaction to light node is considered a bad behavior
-		if matches!(self.local_role, config::Role::Light) {
-			debug!(target: "sync", "Peer {} is trying to send transactions to the light node", who);
-			self.service.disconnect_peer(who, self.protocol_name.clone());
-			self.service.report_peer(who, rep::UNEXPECTED_TRANSACTIONS);
-			return
-		}
-
 		// Accept transactions only when enabled
 		if !self.gossip_enabled.load(Ordering::Relaxed) {
 			trace!(target: "sync", "{} Ignoring transactions while disabled", who);

substrate/client/rpc-api/src/system/helpers.rs

@@ -76,8 +76,6 @@ pub struct PeerInfo<Hash, Number> {
 pub enum NodeRole {
 	/// The node is a full node
 	Full,
-	/// The node is a light client
-	LightClient,
 	/// The node is an authority
 	Authority,
 }

substrate/client/service/src/builder.rs

@@ -37,17 +37,12 @@ use sc_client_db::{Backend, DatabaseSettings};
 use sc_consensus::import_queue::ImportQueue;
 use sc_executor::RuntimeVersionOf;
 use sc_keystore::LocalKeystore;
-use sc_network::{
+use sc_network::{config::SyncMode, NetworkService};
-	config::{Role, SyncMode},
-	NetworkService,
-};
 use sc_network_common::sync::warp::WarpSyncProvider;
-use sc_network_light::light_client_requests::{self, handler::LightClientRequestHandler};
+use sc_network_light::light_client_requests::handler::LightClientRequestHandler;
 use sc_network_sync::{
-	block_request_handler::{self, BlockRequestHandler},
+	block_request_handler::BlockRequestHandler, state_request_handler::StateRequestHandler,
-	state_request_handler::{self, StateRequestHandler},
+	warp_request_handler::RequestHandler as WarpSyncRequestHandler, ChainSync,
-	warp_request_handler::{self, RequestHandler as WarpSyncRequestHandler},
-	ChainSync,
 };
 use sc_rpc::{
 	author::AuthorApiServer,
@@ -731,11 +726,8 @@ where
 		}
 	}
 
-	let transaction_pool_adapter = Arc::new(TransactionPoolAdapter {
+	let transaction_pool_adapter =
-		imports_external_transactions: !matches!(config.role, Role::Light),
+		Arc::new(TransactionPoolAdapter { pool: transaction_pool, client: client.clone() });
-		pool: transaction_pool,
-		client: client.clone(),
-	});
 
 	let protocol_id = config.protocol_id();
 
@@ -746,63 +738,42 @@ where
 	};
 
 	let block_request_protocol_config = {
-		if matches!(config.role, Role::Light) {
+		// Allow both outgoing and incoming requests.
-			// Allow outgoing requests but deny incoming requests.
+		let (handler, protocol_config) = BlockRequestHandler::new(
-			block_request_handler::generate_protocol_config(&protocol_id)
+			&protocol_id,
-		} else {
+			client.clone(),
-			// Allow both outgoing and incoming requests.
+			config.network.default_peers_set.in_peers as usize +
-			let (handler, protocol_config) = BlockRequestHandler::new(
+				config.network.default_peers_set.out_peers as usize,
-				&protocol_id,
+		);
-				client.clone(),
+		spawn_handle.spawn("block-request-handler", Some("networking"), handler.run());
-				config.network.default_peers_set.in_peers as usize +
+		protocol_config
-					config.network.default_peers_set.out_peers as usize,
-			);
-			spawn_handle.spawn("block-request-handler", Some("networking"), handler.run());
-			protocol_config
-		}
 	};
 
 	let state_request_protocol_config = {
-		if matches!(config.role, Role::Light) {
+		// Allow both outgoing and incoming requests.
-			// Allow outgoing requests but deny incoming requests.
+		let (handler, protocol_config) = StateRequestHandler::new(
-			state_request_handler::generate_protocol_config(&protocol_id)
+			&protocol_id,
-		} else {
+			client.clone(),
-			// Allow both outgoing and incoming requests.
+			config.network.default_peers_set_num_full as usize,
-			let (handler, protocol_config) = StateRequestHandler::new(
+		);
-				&protocol_id,
+		spawn_handle.spawn("state-request-handler", Some("networking"), handler.run());
-				client.clone(),
+		protocol_config
-				config.network.default_peers_set_num_full as usize,
-			);
-			spawn_handle.spawn("state-request-handler", Some("networking"), handler.run());
-			protocol_config
-		}
 	};
 
 	let warp_sync_params = warp_sync.map(|provider| {
-		let protocol_config = if matches!(config.role, Role::Light) {
+		// Allow both outgoing and incoming requests.
-			// Allow outgoing requests but deny incoming requests.
+		let (handler, protocol_config) =
-			warp_request_handler::generate_request_response_config(protocol_id.clone())
+			WarpSyncRequestHandler::new(protocol_id.clone(), provider.clone());
-		} else {
+		spawn_handle.spawn("warp-sync-request-handler", Some("networking"), handler.run());
-			// Allow both outgoing and incoming requests.
-			let (handler, protocol_config) =
-				WarpSyncRequestHandler::new(protocol_id.clone(), provider.clone());
-			spawn_handle.spawn("warp-sync-request-handler", Some("networking"), handler.run());
-			protocol_config
-		};
 		(provider, protocol_config)
 	});
 
 	let light_client_request_protocol_config = {
-		if matches!(config.role, Role::Light) {
+		// Allow both outgoing and incoming requests.
-			// Allow outgoing requests but deny incoming requests.
+		let (handler, protocol_config) =
-			light_client_requests::generate_protocol_config(&protocol_id)
+			LightClientRequestHandler::new(&protocol_id, client.clone());
-		} else {
+		spawn_handle.spawn("light-client-request-handler", Some("networking"), handler.run());
-			// Allow both outgoing and incoming requests.
+		protocol_config
-			let (handler, protocol_config) =
-				LightClientRequestHandler::new(&protocol_id, client.clone());
-			spawn_handle.spawn("light-client-request-handler", Some("networking"), handler.run());
-			protocol_config
-		}
 	};
 
 	let max_parallel_downloads = config.network.max_parallel_downloads;

substrate/client/service/src/lib.rs

@@ -253,7 +253,6 @@ async fn build_network_future<
 
 						let node_role = match role {
 							Role::Authority { .. } => NodeRole::Authority,
-							Role::Light => NodeRole::LightClient,
 							Role::Full => NodeRole::Full,
 						};
 
@@ -377,7 +376,6 @@ where
 
 /// Transaction pool adapter.
 pub struct TransactionPoolAdapter<C, P> {
-	imports_external_transactions: bool,
 	pool: Arc<P>,
 	client: Arc<C>,
 }
@@ -425,11 +423,6 @@ where
 	}
 
 	fn import(&self, transaction: B::Extrinsic) -> TransactionImportFuture {
-		if !self.imports_external_transactions {
-			debug!("Transaction rejected");
-			return Box::pin(futures::future::ready(TransactionImport::None))
-		}
-
 		let encoded = transaction.encode();
 		let uxt = match Decode::decode(&mut &encoded[..]) {
 			Ok(uxt) => uxt,

substrate/client/service/src/metrics.rs

@@ -160,7 +160,7 @@ impl MetricsService {
 	) -> Result<Self, PrometheusError> {
 		let role_bits = match config.role {
 			Role::Full => 1u64,
-			Role::Light => 2u64,
+			// 2u64 used to represent light client role
 			Role::Authority { .. } => 4u64,
 		};