Make choosing an executor (native/wasm) an explicit part of service construction (#9525)

* Split native executor stuff from wasm executor stuff

* Remove `native_runtime_version` in places

* Fix warning

* Fix test warning

* Remove redundant NativeRuntimeInfo trait

* Add a warning for use_native

* Run cargo fmt

* Revert "Add a warning for use_native"

This reverts commit 9494f765a06037e991dd60524f2ed1b14649bfd6.

* Make choosing an executor (native/wasm) an explicit part of service construction

* Add Cargo.lock

* Rename Executor to ExecutorDispatch

* Update bin/node/executor/src/lib.rs

Co-authored-by: Squirrel <gilescope@gmail.com>

* Fix tests

* Fix minor node-executor error

* Fix node cli command thing

Co-authored-by: Squirrel <gilescope@gmail.com>

substrate/client/consensus/babe/src/tests.rs

@@ -55,7 +55,7 @@ type Error = sp_blockchain::Error;
 
 type TestClient = substrate_test_runtime_client::client::Client<
 	substrate_test_runtime_client::Backend,
-	substrate_test_runtime_client::Executor,
+	substrate_test_runtime_client::ExecutorDispatch,
 	TestBlock,
 	substrate_test_runtime_client::runtime::RuntimeApi,
 >;

substrate/client/executor/src/lib.rs

@@ -39,7 +39,7 @@ mod wasm_runtime;
 
 pub use codec::Codec;
 pub use native_executor::{
-	with_externalities_safe, NativeExecutionDispatch, NativeExecutor, WasmExecutor,
+	with_externalities_safe, NativeElseWasmExecutor, NativeExecutionDispatch, WasmExecutor,
 };
 #[doc(hidden)]
 pub use sp_core::traits::Externalities;

substrate/client/executor/src/native_executor.rs

@@ -311,7 +311,7 @@ impl RuntimeVersionOf for WasmExecutor {
 
 /// A generic `CodeExecutor` implementation that uses a delegate to determine wasm code equivalence
 /// and dispatch to native code when possible, falling back on `WasmExecutor` when not.
-pub struct NativeExecutor<D> {
+pub struct NativeElseWasmExecutor<D> {
 	/// Dummy field to avoid the compiler complaining about us not using `D`.
 	_dummy: std::marker::PhantomData<D>,
 	/// Native runtime version info.
@@ -320,7 +320,7 @@ pub struct NativeExecutor<D> {
 	wasm: WasmExecutor,
 }
 
-impl<D: NativeExecutionDispatch> NativeExecutor<D> {
+impl<D: NativeExecutionDispatch> NativeElseWasmExecutor<D> {
 	/// Create new instance.
 	///
 	/// # Parameters
@@ -356,7 +356,7 @@ impl<D: NativeExecutionDispatch> NativeExecutor<D> {
 			None,
 		);
 
-		NativeExecutor {
+		NativeElseWasmExecutor {
 			_dummy: Default::default(),
 			native_version: D::native_version(),
 			wasm: wasm_executor,
@@ -364,7 +364,7 @@ impl<D: NativeExecutionDispatch> NativeExecutor<D> {
 	}
 }
 
-impl<D: NativeExecutionDispatch> RuntimeVersionOf for NativeExecutor<D> {
+impl<D: NativeExecutionDispatch> RuntimeVersionOf for NativeElseWasmExecutor<D> {
 	fn runtime_version(
 		&self,
 		ext: &mut dyn Externalities,
@@ -377,7 +377,7 @@ impl<D: NativeExecutionDispatch> RuntimeVersionOf for NativeExecutor<D> {
 	}
 }
 
-impl<D: NativeExecutionDispatch> GetNativeVersion for NativeExecutor<D> {
+impl<D: NativeExecutionDispatch> GetNativeVersion for NativeElseWasmExecutor<D> {
 	fn native_version(&self) -> &NativeVersion {
 		&self.native_version
 	}
@@ -508,7 +508,7 @@ fn preregister_builtin_ext(module: Arc<dyn WasmModule>) {
 	});
 }
 
-impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeExecutor<D> {
+impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeElseWasmExecutor<D> {
 	type Error = Error;
 
 	fn call<
@@ -586,9 +586,9 @@ impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeExecutor<D> {
 	}
 }
 
-impl<D: NativeExecutionDispatch> Clone for NativeExecutor<D> {
+impl<D: NativeExecutionDispatch> Clone for NativeElseWasmExecutor<D> {
 	fn clone(&self) -> Self {
-		NativeExecutor {
+		NativeElseWasmExecutor {
 			_dummy: Default::default(),
 			native_version: D::native_version(),
 			wasm: self.wasm.clone(),
@@ -596,7 +596,7 @@ impl<D: NativeExecutionDispatch> Clone for NativeExecutor<D> {
 	}
 }
 
-impl<D: NativeExecutionDispatch> sp_core::traits::ReadRuntimeVersion for NativeExecutor<D> {
+impl<D: NativeExecutionDispatch> sp_core::traits::ReadRuntimeVersion for NativeElseWasmExecutor<D> {
 	fn read_runtime_version(
 		&self,
 		wasm_code: &[u8],
@@ -618,9 +618,9 @@ mod tests {
 		}
 	}
 
-	pub struct MyExecutor;
+	pub struct MyExecutorDispatch;
 
-	impl NativeExecutionDispatch for MyExecutor {
+	impl NativeExecutionDispatch for MyExecutorDispatch {
 		type ExtendHostFunctions = (my_interface::HostFunctions, my_interface::HostFunctions);
 
 		fn dispatch(method: &str, data: &[u8]) -> Option<Vec<u8>> {
@@ -634,7 +634,11 @@ mod tests {
 
 	#[test]
 	fn native_executor_registers_custom_interface() {
-		let executor = NativeExecutor::<MyExecutor>::new(WasmExecutionMethod::Interpreted, None, 8);
+		let executor = NativeElseWasmExecutor::<MyExecutorDispatch>::new(
+			WasmExecutionMethod::Interpreted,
+			None,
+			8,
+		);
 		my_interface::HostFunctions::host_functions().iter().for_each(|function| {
 			assert_eq!(executor.wasm.host_functions.iter().filter(|f| f == &function).count(), 2);
 		});

substrate/client/network/test/src/lib.rs

@@ -126,7 +126,7 @@ impl<B: BlockT> Verifier<B> for PassThroughVerifier {
 
 pub type PeersFullClient = Client<
 	substrate_test_runtime_client::Backend,
-	substrate_test_runtime_client::Executor,
+	substrate_test_runtime_client::ExecutorDispatch,
 	Block,
 	substrate_test_runtime_client::runtime::RuntimeApi,
 >;

substrate/client/service/src/builder.rs

@@ -37,7 +37,7 @@ use sc_client_api::{
 };
 use sc_client_db::{Backend, DatabaseSettings};
 use sc_consensus::import_queue::ImportQueue;
-use sc_executor::{NativeExecutionDispatch, NativeExecutor, RuntimeVersionOf};
+use sc_executor::RuntimeVersionOf;
 use sc_keystore::LocalKeystore;
 use sc_network::{
 	block_request_handler::{self, BlockRequestHandler},
@@ -128,39 +128,39 @@ where
 }
 
 /// Full client type.
-pub type TFullClient<TBl, TRtApi, TExecDisp> =
-	Client<TFullBackend<TBl>, TFullCallExecutor<TBl, TExecDisp>, TBl, TRtApi>;
+pub type TFullClient<TBl, TRtApi, TExec> =
+	Client<TFullBackend<TBl>, TFullCallExecutor<TBl, TExec>, TBl, TRtApi>;
 
 /// Full client backend type.
 pub type TFullBackend<TBl> = sc_client_db::Backend<TBl>;
 
 /// Full client call executor type.
-pub type TFullCallExecutor<TBl, TExecDisp> =
-	crate::client::LocalCallExecutor<TBl, sc_client_db::Backend<TBl>, NativeExecutor<TExecDisp>>;
+pub type TFullCallExecutor<TBl, TExec> =
+	crate::client::LocalCallExecutor<TBl, sc_client_db::Backend<TBl>, TExec>;
 
 /// Light client type.
-pub type TLightClient<TBl, TRtApi, TExecDisp> =
-	TLightClientWithBackend<TBl, TRtApi, TExecDisp, TLightBackend<TBl>>;
+pub type TLightClient<TBl, TRtApi, TExec> =
+	TLightClientWithBackend<TBl, TRtApi, TExec, TLightBackend<TBl>>;
 
 /// Light client backend type.
 pub type TLightBackend<TBl> =
 	sc_light::Backend<sc_client_db::light::LightStorage<TBl>, HashFor<TBl>>;
 
 /// Light call executor type.
-pub type TLightCallExecutor<TBl, TExecDisp> = sc_light::GenesisCallExecutor<
+pub type TLightCallExecutor<TBl, TExec> = sc_light::GenesisCallExecutor<
 	sc_light::Backend<sc_client_db::light::LightStorage<TBl>, HashFor<TBl>>,
 	crate::client::LocalCallExecutor<
 		TBl,
 		sc_light::Backend<sc_client_db::light::LightStorage<TBl>, HashFor<TBl>>,
-		NativeExecutor<TExecDisp>,
+		TExec,
 	>,
 >;
 
-type TFullParts<TBl, TRtApi, TExecDisp> =
-	(TFullClient<TBl, TRtApi, TExecDisp>, Arc<TFullBackend<TBl>>, KeystoreContainer, TaskManager);
+type TFullParts<TBl, TRtApi, TExec> =
+	(TFullClient<TBl, TRtApi, TExec>, Arc<TFullBackend<TBl>>, KeystoreContainer, TaskManager);
 
-type TLightParts<TBl, TRtApi, TExecDisp> = (
-	Arc<TLightClient<TBl, TRtApi, TExecDisp>>,
+type TLightParts<TBl, TRtApi, TExec> = (
+	Arc<TLightClient<TBl, TRtApi, TExec>>,
 	Arc<TLightBackend<TBl>>,
 	KeystoreContainer,
 	TaskManager,
@@ -172,12 +172,9 @@ pub type TLightBackendWithHash<TBl, THash> =
 	sc_light::Backend<sc_client_db::light::LightStorage<TBl>, THash>;
 
 /// Light client type with a specific backend.
-pub type TLightClientWithBackend<TBl, TRtApi, TExecDisp, TBackend> = Client<
+pub type TLightClientWithBackend<TBl, TRtApi, TExec, TBackend> = Client<
 	TBackend,
-	sc_light::GenesisCallExecutor<
-		TBackend,
-		crate::client::LocalCallExecutor<TBl, TBackend, NativeExecutor<TExecDisp>>,
-	>,
+	sc_light::GenesisCallExecutor<TBackend, crate::client::LocalCallExecutor<TBl, TBackend, TExec>>,
 	TBl,
 	TRtApi,
 >;
@@ -262,26 +259,28 @@ impl KeystoreContainer {
 }
 
 /// Creates a new full client for the given config.
-pub fn new_full_client<TBl, TRtApi, TExecDisp>(
+pub fn new_full_client<TBl, TRtApi, TExec>(
 	config: &Configuration,
 	telemetry: Option<TelemetryHandle>,
-) -> Result<TFullClient<TBl, TRtApi, TExecDisp>, Error>
+	executor: TExec,
+) -> Result<TFullClient<TBl, TRtApi, TExec>, Error>
 where
 	TBl: BlockT,
-	TExecDisp: NativeExecutionDispatch + 'static,
+	TExec: CodeExecutor + RuntimeVersionOf + Clone,
 	TBl::Hash: FromStr,
 {
-	new_full_parts(config, telemetry).map(|parts| parts.0)
+	new_full_parts(config, telemetry, executor).map(|parts| parts.0)
 }
 
 /// Create the initial parts of a full node.
-pub fn new_full_parts<TBl, TRtApi, TExecDisp>(
+pub fn new_full_parts<TBl, TRtApi, TExec>(
 	config: &Configuration,
 	telemetry: Option<TelemetryHandle>,
-) -> Result<TFullParts<TBl, TRtApi, TExecDisp>, Error>
+	executor: TExec,
+) -> Result<TFullParts<TBl, TRtApi, TExec>, Error>
 where
 	TBl: BlockT,
-	TExecDisp: NativeExecutionDispatch + 'static,
+	TExec: CodeExecutor + RuntimeVersionOf + Clone,
 	TBl::Hash: FromStr,
 {
 	let keystore_container = KeystoreContainer::new(&config.keystore)?;
@@ -291,12 +290,6 @@ where
 		TaskManager::new(config.task_executor.clone(), registry)?
 	};
 
-	let executor = NativeExecutor::<TExecDisp>::new(
-		config.wasm_method,
-		config.default_heap_pages,
-		config.max_runtime_instances,
-	);
-
 	let chain_spec = &config.chain_spec;
 	let fork_blocks = get_extension::<ForkBlocks<TBl>>(chain_spec.extensions())
 		.cloned()
@@ -368,13 +361,14 @@ where
 }
 
 /// Create the initial parts of a light node.
-pub fn new_light_parts<TBl, TRtApi, TExecDisp>(
+pub fn new_light_parts<TBl, TRtApi, TExec>(
 	config: &Configuration,
 	telemetry: Option<TelemetryHandle>,
-) -> Result<TLightParts<TBl, TRtApi, TExecDisp>, Error>
+	executor: TExec,
+) -> Result<TLightParts<TBl, TRtApi, TExec>, Error>
 where
 	TBl: BlockT,
-	TExecDisp: NativeExecutionDispatch + 'static,
+	TExec: CodeExecutor + RuntimeVersionOf + Clone,
 {
 	let keystore_container = KeystoreContainer::new(&config.keystore)?;
 	let task_manager = {
@@ -382,12 +376,6 @@ where
 		TaskManager::new(config.task_executor.clone(), registry)?
 	};
 
-	let executor = NativeExecutor::<TExecDisp>::new(
-		config.wasm_method,
-		config.default_heap_pages,
-		config.max_runtime_instances,
-	);
-
 	let db_storage = {
 		let db_settings = sc_client_db::DatabaseSettings {
 			state_cache_size: config.state_cache_size,

substrate/client/service/src/client/call_executor.rs

@@ -360,17 +360,20 @@ where
 mod tests {
 	use super::*;
 	use sc_client_api::in_mem;
-	use sc_executor::{NativeExecutor, WasmExecutionMethod};
+	use sc_executor::{NativeElseWasmExecutor, WasmExecutionMethod};
 	use sp_core::{
 		testing::TaskExecutor,
 		traits::{FetchRuntimeCode, WrappedRuntimeCode},
 	};
-	use substrate_test_runtime_client::{runtime, GenesisInit, LocalExecutor};
+	use substrate_test_runtime_client::{runtime, GenesisInit, LocalExecutorDispatch};
 
 	#[test]
 	fn should_get_override_if_exists() {
-		let executor =
-			NativeExecutor::<LocalExecutor>::new(WasmExecutionMethod::Interpreted, Some(128), 1);
+		let executor = NativeElseWasmExecutor::<LocalExecutorDispatch>::new(
+			WasmExecutionMethod::Interpreted,
+			Some(128),
+			1,
+		);
 
 		let overrides = crate::client::wasm_override::dummy_overrides(&executor);
 		let onchain_code = WrappedRuntimeCode(substrate_test_runtime::wasm_binary_unwrap().into());

substrate/client/service/src/client/wasm_override.rs

@@ -204,19 +204,20 @@ where
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use sc_executor::{NativeExecutor, WasmExecutionMethod};
+	use sc_executor::{NativeElseWasmExecutor, WasmExecutionMethod};
 	use std::fs::{self, File};
-	use substrate_test_runtime_client::LocalExecutor;
+	use substrate_test_runtime_client::LocalExecutorDispatch;
 
 	fn wasm_test<F>(fun: F)
 	where
-		F: Fn(&Path, &[u8], &NativeExecutor<LocalExecutor>),
+		F: Fn(&Path, &[u8], &NativeElseWasmExecutor<LocalExecutorDispatch>),
 	{
-		let exec = NativeExecutor::<substrate_test_runtime_client::LocalExecutor>::new(
-			WasmExecutionMethod::Interpreted,
-			Some(128),
-			1,
-		);
+		let exec =
+			NativeElseWasmExecutor::<substrate_test_runtime_client::LocalExecutorDispatch>::new(
+				WasmExecutionMethod::Interpreted,
+				Some(128),
+				1,
+			);
 		let bytes = substrate_test_runtime::wasm_binary_unwrap();
 		let dir = tempfile::tempdir().expect("Create a temporary directory");
 		fun(dir.path(), bytes, &exec);
@@ -226,8 +227,11 @@ mod tests {
 	#[test]
 	fn should_get_runtime_version() {
 		let wasm = WasmBlob::new(substrate_test_runtime::wasm_binary_unwrap().to_vec());
-		let executor =
-			NativeExecutor::<LocalExecutor>::new(WasmExecutionMethod::Interpreted, Some(128), 1);
+		let executor = NativeElseWasmExecutor::<LocalExecutorDispatch>::new(
+			WasmExecutionMethod::Interpreted,
+			Some(128),
+			1,
+		);
 
 		let version = WasmOverride::runtime_version(&executor, &wasm, Some(128))
 			.expect("should get the `RuntimeVersion` of the test-runtime wasm blob");

substrate/client/service/test/src/client/light.rs

@@ -30,7 +30,7 @@ use sc_client_api::{
 	RemoteBodyRequest, RemoteCallRequest, RemoteChangesRequest, RemoteHeaderRequest,
 	RemoteReadChildRequest, RemoteReadRequest, Storage, StorageProof, StorageProvider,
 };
-use sc_executor::{NativeExecutor, RuntimeVersion, WasmExecutionMethod};
+use sc_executor::{NativeElseWasmExecutor, RuntimeVersion, WasmExecutionMethod};
 use sc_light::{
 	backend::{Backend, GenesisOrUnavailableState},
 	blockchain::{Blockchain, BlockchainCache},
@@ -258,8 +258,9 @@ impl CallExecutor<Block> for DummyCallExecutor {
 	}
 }
 
-fn local_executor() -> NativeExecutor<substrate_test_runtime_client::LocalExecutor> {
-	NativeExecutor::new(WasmExecutionMethod::Interpreted, None, 8)
+fn local_executor() -> NativeElseWasmExecutor<substrate_test_runtime_client::LocalExecutorDispatch>
+{
+	NativeElseWasmExecutor::new(WasmExecutionMethod::Interpreted, None, 8)
 }
 
 #[test]
@@ -446,7 +447,7 @@ fn code_is_executed_at_genesis_only() {
 }
 
 type TestChecker = LightDataChecker<
-	NativeExecutor<substrate_test_runtime_client::LocalExecutor>,
+	NativeElseWasmExecutor<substrate_test_runtime_client::LocalExecutorDispatch>,
 	Block,
 	DummyStorage,
 >;

substrate/client/service/test/src/client/mod.rs

@@ -62,9 +62,9 @@ mod light;
 
 const TEST_ENGINE_ID: ConsensusEngineId = *b"TEST";
 
-pub struct Executor;
+pub struct ExecutorDispatch;
 
-impl sc_executor::NativeExecutionDispatch for Executor {
+impl sc_executor::NativeExecutionDispatch for ExecutorDispatch {
 	type ExtendHostFunctions = ();
 
 	fn dispatch(method: &str, data: &[u8]) -> Option<Vec<u8>> {
@@ -76,14 +76,14 @@ impl sc_executor::NativeExecutionDispatch for Executor {
 	}
 }
 
-fn executor() -> sc_executor::NativeExecutor<Executor> {
-	sc_executor::NativeExecutor::new(sc_executor::WasmExecutionMethod::Interpreted, None, 8)
+fn executor() -> sc_executor::NativeElseWasmExecutor<ExecutorDispatch> {
+	sc_executor::NativeElseWasmExecutor::new(sc_executor::WasmExecutionMethod::Interpreted, None, 8)
 }
 
 pub fn prepare_client_with_key_changes() -> (
 	client::Client<
 		substrate_test_runtime_client::Backend,
-		substrate_test_runtime_client::Executor,
+		substrate_test_runtime_client::ExecutorDispatch,
 		Block,
 		RuntimeApi,
 	>,
@@ -2106,7 +2106,7 @@ fn cleans_up_closed_notification_sinks_on_block_import() {
 		LocalCallExecutor<
 			Block,
 			in_mem::Backend<Block>,
-			sc_executor::NativeExecutor<LocalExecutor>,
+			sc_executor::NativeElseWasmExecutor<LocalExecutorDispatch>,
 		>,
 		substrate_test_runtime_client::runtime::Block,
 		substrate_test_runtime_client::runtime::RuntimeApi,