Simplify NativeExecutionDispatch and remove the native_executor_instance! macro (#9562)

* Remove the `native_executor_instance!` macro

* Add comment to test runner ex

* Fix comments

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

@@ -3,7 +3,6 @@
 use node_template_runtime::{self, opaque::Block, RuntimeApi};
 use sc_client_api::{ExecutorProvider, RemoteBackend};
 use sc_consensus_aura::{ImportQueueParams, SlotProportion, StartAuraParams};
-use sc_executor::native_executor_instance;
 pub use sc_executor::NativeExecutor;
 use sc_finality_grandpa::SharedVoterState;
 use sc_keystore::LocalKeystore;
@@ -14,12 +13,19 @@ use sp_consensus_aura::sr25519::AuthorityPair as AuraPair;
 use std::{sync::Arc, time::Duration};
 
 // Our native executor instance.
-native_executor_instance!(
+pub struct Executor;
-	pub Executor,
+
-	node_template_runtime::api::dispatch,
+impl sc_executor::NativeExecutionDispatch for Executor {
-	node_template_runtime::native_version,
+	type ExtendHostFunctions = frame_benchmarking::benchmarking::HostFunctions;
-	frame_benchmarking::benchmarking::HostFunctions,
+
-);
+	fn dispatch(method: &str, data: &[u8]) -> Option<Vec<u8>> {
+		node_template_runtime::api::dispatch(method, data)
+	}
+
+	fn native_version() -> sc_executor::NativeVersion {
+		node_template_runtime::native_version()
+	}
+}
 
 type FullClient = sc_service::TFullClient<Block, RuntimeApi, Executor>;
 type FullBackend = sc_service::TFullBackend<Block>;

substrate/bin/node/executor/src/lib.rs

@@ -18,14 +18,20 @@
 //! A `CodeExecutor` specialization which uses natively compiled runtime when the wasm to be
 //! executed is equivalent to the natively compiled code.
 
-use sc_executor::native_executor_instance;
 pub use sc_executor::NativeExecutor;
 
 // Declare an instance of the native executor named `Executor`. Include the wasm binary as the
 // equivalent wasm code.
-native_executor_instance!(
+pub struct Executor;
-	pub Executor,
+
-	node_runtime::api::dispatch,
+impl sc_executor::NativeExecutionDispatch for Executor {
-	node_runtime::native_version,
+	type ExtendHostFunctions = frame_benchmarking::benchmarking::HostFunctions;
-	frame_benchmarking::benchmarking::HostFunctions,
+
-);
+	fn dispatch(method: &str, data: &[u8]) -> Option<Vec<u8>> {
+		node_runtime::api::dispatch(method, data)
+	}
+
+	fn native_version() -> sc_executor::NativeVersion {
+		node_runtime::native_version()
+	}
+}

substrate/bin/node/executor/tests/common.rs

@@ -67,8 +67,7 @@ impl AppCrypto<MultiSigner, MultiSignature> for TestAuthorityId {
 ///
 /// `compact` since it is after post-processing with wasm-gc which performs tree-shaking thus
 /// making the binary slimmer. There is a convention to use compact version of the runtime
-/// as canonical. This is why `native_executor_instance` also uses the compact version of the
+/// as canonical.
-/// runtime.
 pub fn compact_code_unwrap() -> &'static [u8] {
 	node_runtime::WASM_BINARY.expect(
 		"Development wasm binary is not available. Testing is only supported with the flag \

substrate/bin/node/test-runner-example/src/lib.rs

@@ -28,15 +28,22 @@ use test_runner::{ChainInfo, SignatureVerificationOverride};
 
 type BlockImport<B, BE, C, SC> = BabeBlockImport<B, C, GrandpaBlockImport<BE, B, C, SC>>;
 
-sc_executor::native_executor_instance!(
+/// A unit struct which implements `NativeExecutionDispatch` feeding in the
-	pub Executor,
+/// hard-coded runtime.
-	node_runtime::api::dispatch,
+pub struct Executor;
-	node_runtime::native_version,
+
-	(
+impl sc_executor::NativeExecutionDispatch for Executor {
-		frame_benchmarking::benchmarking::HostFunctions,
+	type ExtendHostFunctions =
-		SignatureVerificationOverride,
+		(frame_benchmarking::benchmarking::HostFunctions, SignatureVerificationOverride);
-	)
+
-);
+	fn dispatch(method: &str, data: &[u8]) -> Option<Vec<u8>> {
+		node_runtime::api::dispatch(method, data)
+	}
+
+	fn native_version() -> sc_executor::NativeVersion {
+		node_runtime::native_version()
+	}
+}
 
 /// ChainInfo implementation.
 struct NodeTemplateChainInfo;

substrate/client/executor/src/native_executor.rs

@@ -83,9 +83,7 @@ pub trait NativeExecutionDispatch: Send + Sync {
 	type ExtendHostFunctions: HostFunctions;
 
 	/// Dispatch a method in the runtime.
-	///
+	fn dispatch(method: &str, data: &[u8]) -> Option<Vec<u8>>;
-	/// If the method with the specified name doesn't exist then `Err` is returned.
-	fn dispatch(ext: &mut dyn Externalities, method: &str, data: &[u8]) -> Result<Vec<u8>>;
 
 	/// Provide native runtime version.
 	fn native_version() -> NativeVersion;
@@ -577,7 +575,9 @@ impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeExecutor<D> {
 						);
 
 						used_native = true;
-						Ok(D::dispatch(&mut **ext, method, data).map(NativeOrEncoded::Encoded))
+						Ok(with_externalities_safe(&mut **ext, move || D::dispatch(method, data))?
+							.map(NativeOrEncoded::Encoded)
+							.ok_or_else(|| Error::MethodNotFound(method.to_owned())))
 					},
 				}
 			},
@@ -606,79 +606,6 @@ impl<D: NativeExecutionDispatch> sp_core::traits::ReadRuntimeVersion for NativeE
 	}
 }
 
-/// Implements a `NativeExecutionDispatch` for provided parameters.
-///
-/// # Example
-///
-/// ```
-/// sc_executor::native_executor_instance!(
-///     pub MyExecutor,
-///     substrate_test_runtime::api::dispatch,
-///     substrate_test_runtime::native_version,
-/// );
-/// ```
-///
-/// # With custom host functions
-///
-/// When you want to use custom runtime interfaces from within your runtime, you need to make the
-/// executor aware of the host functions for these interfaces.
-///
-/// ```
-/// # use sp_runtime_interface::runtime_interface;
-///
-/// #[runtime_interface]
-/// trait MyInterface {
-///     fn say_hello_world(data: &str) {
-///         println!("Hello world from: {}", data);
-///     }
-/// }
-///
-/// sc_executor::native_executor_instance!(
-///     pub MyExecutor,
-///     substrate_test_runtime::api::dispatch,
-///     substrate_test_runtime::native_version,
-///     my_interface::HostFunctions,
-/// );
-/// ```
-///
-/// When you have multiple interfaces, you can give the host functions as a tuple e.g.:
-/// `(my_interface::HostFunctions, my_interface2::HostFunctions)`
-#[macro_export]
-macro_rules! native_executor_instance {
-	( $pub:vis $name:ident, $dispatcher:path, $version:path $(,)?) => {
-		/// A unit struct which implements `NativeExecutionDispatch` feeding in the
-		/// hard-coded runtime.
-		$pub struct $name;
-		$crate::native_executor_instance!(IMPL $name, $dispatcher, $version, ());
-	};
-	( $pub:vis $name:ident, $dispatcher:path, $version:path, $custom_host_functions:ty $(,)?) => {
-		/// A unit struct which implements `NativeExecutionDispatch` feeding in the
-		/// hard-coded runtime.
-		$pub struct $name;
-		$crate::native_executor_instance!(
-			IMPL $name, $dispatcher, $version, $custom_host_functions
-		);
-	};
-	(IMPL $name:ident, $dispatcher:path, $version:path, $custom_host_functions:ty) => {
-		impl $crate::NativeExecutionDispatch for $name {
-			type ExtendHostFunctions = $custom_host_functions;
-
-			fn dispatch(
-				ext: &mut dyn $crate::Externalities,
-				method: &str,
-				data: &[u8]
-			) -> $crate::error::Result<Vec<u8>> {
-				$crate::with_externalities_safe(ext, move || $dispatcher(method, data))?
-					.ok_or_else(|| $crate::error::Error::MethodNotFound(method.to_owned()))
-			}
-
-			fn native_version() -> $crate::NativeVersion {
-				$version()
-			}
-		}
-	}
-}
-
 #[cfg(test)]
 mod tests {
 	use super::*;
@@ -691,12 +618,19 @@ mod tests {
 		}
 	}
 
-	native_executor_instance!(
+	pub struct MyExecutor;
-		pub MyExecutor,
+
-		substrate_test_runtime::api::dispatch,
+	impl NativeExecutionDispatch for MyExecutor {
-		substrate_test_runtime::native_version,
+		type ExtendHostFunctions = (my_interface::HostFunctions, my_interface::HostFunctions);
-		(my_interface::HostFunctions, my_interface::HostFunctions),
+
-	);
+		fn dispatch(method: &str, data: &[u8]) -> Option<Vec<u8>> {
+			substrate_test_runtime::api::dispatch(method, data)
+		}
+
+		fn native_version() -> NativeVersion {
+			substrate_test_runtime::native_version()
+		}
+	}
 
 	#[test]
 	fn native_executor_registers_custom_interface() {

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

@@ -27,7 +27,6 @@ use sc_client_db::{
 use sc_consensus::{
 	BlockCheckParams, BlockImport, BlockImportParams, ForkChoiceStrategy, ImportResult,
 };
-use sc_executor::native_executor_instance;
 use sc_service::client::{self, new_in_mem, Client, LocalCallExecutor};
 use sp_api::ProvideRuntimeApi;
 use sp_consensus::{BlockOrigin, BlockStatus, Error as ConsensusError, SelectChain};
@@ -63,11 +62,19 @@ mod light;
 
 const TEST_ENGINE_ID: ConsensusEngineId = *b"TEST";
 
-native_executor_instance!(
+pub struct Executor;
-	Executor,
+
-	substrate_test_runtime_client::runtime::api::dispatch,
+impl sc_executor::NativeExecutionDispatch for Executor {
-	substrate_test_runtime_client::runtime::native_version,
+	type ExtendHostFunctions = ();
-);
+
+	fn dispatch(method: &str, data: &[u8]) -> Option<Vec<u8>> {
+		substrate_test_runtime_client::runtime::api::dispatch(method, data)
+	}
+
+	fn native_version() -> sc_executor::NativeVersion {
+		substrate_test_runtime_client::runtime::native_version()
+	}
+}
 
 fn executor() -> sc_executor::NativeExecutor<Executor> {
 	sc_executor::NativeExecutor::new(sc_executor::WasmExecutionMethod::Interpreted, None, 8)

substrate/test-utils/runtime/client/src/lib.rs

@@ -58,10 +58,20 @@ pub mod prelude {
 	pub use super::{AccountKeyring, Sr25519Keyring};
 }
 
-sc_executor::native_executor_instance! {
+/// A unit struct which implements `NativeExecutionDispatch` feeding in the
-	pub LocalExecutor,
+/// hard-coded runtime.
-	substrate_test_runtime::api::dispatch,
+pub struct LocalExecutor;
-	substrate_test_runtime::native_version,
+
+impl sc_executor::NativeExecutionDispatch for LocalExecutor {
+	type ExtendHostFunctions = ();
+
+	fn dispatch(method: &str, data: &[u8]) -> Option<Vec<u8>> {
+		substrate_test_runtime::api::dispatch(method, data)
+	}
+
+	fn native_version() -> sc_executor::NativeVersion {
+		substrate_test_runtime::native_version()
+	}
 }
 
 /// Test client database backend.

substrate/test-utils/runtime/src/system.rs

@@ -349,7 +349,7 @@ mod tests {
 	use super::*;
 
 	use crate::{wasm_binary_unwrap, Header, Transfer};
-	use sc_executor::{native_executor_instance, NativeExecutor, WasmExecutionMethod};
+	use sc_executor::{NativeExecutor, WasmExecutionMethod};
 	use sp_core::{
 		map,
 		traits::{CodeExecutor, RuntimeCode},
@@ -359,7 +359,19 @@ mod tests {
 	use substrate_test_runtime_client::{AccountKeyring, Sr25519Keyring};
 
 	// Declare an instance of the native executor dispatch for the test runtime.
-	native_executor_instance!(NativeDispatch, crate::api::dispatch, crate::native_version);
+	pub struct NativeDispatch;
+
+	impl sc_executor::NativeExecutionDispatch for NativeDispatch {
+		type ExtendHostFunctions = ();
+
+		fn dispatch(method: &str, data: &[u8]) -> Option<Vec<u8>> {
+			crate::api::dispatch(method, data)
+		}
+
+		fn native_version() -> sc_executor::NativeVersion {
+			crate::native_version()
+		}
+	}
 
 	fn executor() -> NativeExecutor<NativeDispatch> {
 		NativeExecutor::new(WasmExecutionMethod::Interpreted, None, 8)

substrate/test-utils/test-runner/src/lib.rs

@@ -62,12 +62,19 @@
 //!
 //! type BlockImport<B, BE, C, SC> = BabeBlockImport<B, C, GrandpaBlockImport<BE, B, C, SC>>;
 //!
-//! sc_executor::native_executor_instance!(
+//! pub struct Executor;
-//! 	pub Executor,
+//!
-//! 	node_runtime::api::dispatch,
+//! impl sc_executor::NativeExecutionDispatch for Executor {
-//! 	node_runtime::native_version,
+//! 	type ExtendHostFunctions = SignatureVerificationOverride;
-//! 	SignatureVerificationOverride,
+//!
-//! );
+//! 	fn dispatch(method: &str, data: &[u8]) -> Option<Vec<u8>> {
+//! 		node_runtime::api::dispatch(method, data)
+//! 	}
+//!
+//! 	fn native_version() -> sc_executor::NativeVersion {
+//! 		node_runtime::native_version()
+//! 	}
+//! }
 //!
 //! struct Requirements;
 //!