Refactor sp-sandbox; make sure both sandbox executors are always te…

…sted (#10173)

* sp-sandbox: convert executors into normal `mod`s instead of using `include!`

* sp-sandbox: run `cargo fmt` on `host_executor.rs`

* sp-sandbox: abstract away the executors behind traits

* sp_sandbox: always compile both executors when possible

* sc-executor: make sure all sandbox tests run on both sandbox executors

* sc-executor: fix brainfart: actually call into the sandbox through the trait

* sc-runtime-test: fix cargo fmt

* sc-runtime-test: deduplicate executor-specific sandbox test entrypoints

* sc-executor: test each sandbox executor in a separate test

* cargo fmt (Github's conflict resolving thingy broke indentation)

client/executor/runtime-test/Cargo.toml

@@ -19,6 +19,7 @@ sp-runtime = { version = "4.0.0-dev", default-features = false, path = "../../..
 sp-sandbox = { version = "0.10.0-dev", default-features = false, path = "../../../primitives/sandbox" }
 sp-std = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/std" }
 sp-tasks = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/tasks" }
+paste = "1.0.4"
 
 [build-dependencies]
 substrate-wasm-builder = { version = "5.0.0-dev", path = "../../../utils/wasm-builder" }

client/executor/runtime-test/src/lib.rs

@@ -30,7 +30,7 @@ use sp_runtime::{
 	traits::{BlakeTwo256, Hash},
 };
 #[cfg(not(feature = "std"))]
-use sp_sandbox::Value;
+use sp_sandbox::{SandboxEnvironmentBuilder, SandboxInstance, SandboxMemory, Value};
 
 extern "C" {
 	#[allow(dead_code)]
@@ -183,61 +183,6 @@ sp_core::wasm_export_functions! {
 	   ).as_ref().to_vec()
    }
 
-   fn test_sandbox(code: Vec<u8>) -> bool {
-	   execute_sandboxed(&code, &[]).is_ok()
-   }
-
-   fn test_sandbox_args(code: Vec<u8>) -> bool {
-	   execute_sandboxed(
-		   &code,
-		   &[
-			   Value::I32(0x12345678),
-			   Value::I64(0x1234567887654321),
-		   ],
-	   ).is_ok()
-   }
-
-   fn test_sandbox_return_val(code: Vec<u8>) -> bool {
-	   let ok = match execute_sandboxed(
-		   &code,
-		   &[
-			   Value::I32(0x1336),
-		   ]
-	   ) {
-		   Ok(sp_sandbox::ReturnValue::Value(Value::I32(0x1337))) => true,
-		   _ => false,
-	   };
-
-	   ok
-   }
-
-   fn test_sandbox_instantiate(code: Vec<u8>) -> u8 {
-	   let env_builder = sp_sandbox::EnvironmentDefinitionBuilder::new();
-	   let code = match sp_sandbox::Instance::new(&code, &env_builder, &mut ()) {
-		   Ok(_) => 0,
-		   Err(sp_sandbox::Error::Module) => 1,
-		   Err(sp_sandbox::Error::Execution) => 2,
-		   Err(sp_sandbox::Error::OutOfBounds) => 3,
-	   };
-
-	   code
-   }
-
-   fn test_sandbox_get_global_val(code: Vec<u8>) -> i64 {
-	   let env_builder = sp_sandbox::EnvironmentDefinitionBuilder::new();
-	   let instance = if let Ok(i) = sp_sandbox::Instance::new(&code, &env_builder, &mut ()) {
-		   i
-	   } else {
-		   return 20;
-	   };
-
-	   match instance.get_global_val("test_global") {
-		   Some(sp_sandbox::Value::I64(val)) => val,
-		   None => 30,
-		   _ => 40,
-	   }
-   }
-
    fn test_offchain_index_set() {
 	   sp_io::offchain_index::set(b"k", b"v");
    }
@@ -408,15 +353,112 @@ mod tasks {
 	}
 }
 
+/// A macro to define a test entrypoint for each available sandbox executor.
+macro_rules! wasm_export_sandbox_test_functions {
+	(
+		$(
+			fn $name:ident<T>(
+				$( $arg_name:ident: $arg_ty:ty ),* $(,)?
+			) $( -> $ret_ty:ty )? where T: SandboxInstance<$state:ty> $(,)?
+			{ $( $fn_impl:tt )* }
+		)*
+	) => {
+		$(
+				#[cfg(not(feature = "std"))]
+				fn $name<T>( $($arg_name: $arg_ty),* ) $( -> $ret_ty )? where T: SandboxInstance<$state> {
+					$( $fn_impl )*
+				}
+
+				paste::paste! {
+					sp_core::wasm_export_functions! {
+						fn [<$name _host>]( $($arg_name: $arg_ty),* ) $( -> $ret_ty )? {
+							$name::<sp_sandbox::host_executor::Instance<$state>>( $( $arg_name ),* )
+						}
+
+						fn [<$name _embedded>]( $($arg_name: $arg_ty),* ) $( -> $ret_ty )? {
+							$name::<sp_sandbox::embedded_executor::Instance<$state>>( $( $arg_name ),* )
+						}
+					}
+				}
+		)*
+	};
+}
+
+wasm_export_sandbox_test_functions! {
+	fn test_sandbox<T>(code: Vec<u8>) -> bool
+	where
+		T: SandboxInstance<State>,
+	{
+		execute_sandboxed::<T>(&code, &[]).is_ok()
+	}
+
+	fn test_sandbox_args<T>(code: Vec<u8>) -> bool
+	where
+		T: SandboxInstance<State>,
+	{
+		execute_sandboxed::<T>(&code, &[Value::I32(0x12345678), Value::I64(0x1234567887654321)])
+			.is_ok()
+	}
+
+	fn test_sandbox_return_val<T>(code: Vec<u8>) -> bool
+	where
+		T: SandboxInstance<State>,
+	{
+		let ok = match execute_sandboxed::<T>(&code, &[Value::I32(0x1336)]) {
+			Ok(sp_sandbox::ReturnValue::Value(Value::I32(0x1337))) => true,
+			_ => false,
+		};
+
+		ok
+	}
+
+	fn test_sandbox_instantiate<T>(code: Vec<u8>) -> u8
+	where
+		T: SandboxInstance<()>,
+	{
+		let env_builder = T::EnvironmentBuilder::new();
+		let code = match T::new(&code, &env_builder, &mut ()) {
+			Ok(_) => 0,
+			Err(sp_sandbox::Error::Module) => 1,
+			Err(sp_sandbox::Error::Execution) => 2,
+			Err(sp_sandbox::Error::OutOfBounds) => 3,
+		};
+
+		code
+	}
+
+	fn test_sandbox_get_global_val<T>(code: Vec<u8>) -> i64
+	where
+		T: SandboxInstance<()>,
+	{
+		let env_builder = T::EnvironmentBuilder::new();
+		let instance = if let Ok(i) = T::new(&code, &env_builder, &mut ()) {
+			i
+		} else {
+			return 20
+		};
+
+		match instance.get_global_val("test_global") {
+			Some(sp_sandbox::Value::I64(val)) => val,
+			None => 30,
+			_ => 40,
+		}
+	}
+}
+
 #[cfg(not(feature = "std"))]
-fn execute_sandboxed(
+struct State {
+	counter: u32,
+}
+
+#[cfg(not(feature = "std"))]
+fn execute_sandboxed<T>(
 	code: &[u8],
 	args: &[Value],
-) -> Result<sp_sandbox::ReturnValue, sp_sandbox::HostError> {
-	struct State {
-		counter: u32,
-	}
-
+) -> Result<sp_sandbox::ReturnValue, sp_sandbox::HostError>
+where
+	T: sp_sandbox::SandboxInstance<State>,
+{
 	fn env_assert(
 		_e: &mut State,
 		args: &[Value],
@@ -446,10 +488,10 @@ fn execute_sandboxed(
 	let mut state = State { counter: 0 };
 
 	let env_builder = {
-		let mut env_builder = sp_sandbox::EnvironmentDefinitionBuilder::new();
+		let mut env_builder = T::EnvironmentBuilder::new();
 		env_builder.add_host_func("env", "assert", env_assert);
 		env_builder.add_host_func("env", "inc_counter", env_inc_counter);
-		let memory = match sp_sandbox::Memory::new(1, Some(16)) {
+		let memory = match T::Memory::new(1, Some(16)) {
 			Ok(m) => m,
 			Err(_) => unreachable!(
 				"
@@ -462,7 +504,7 @@ fn execute_sandboxed(
 		env_builder
 	};
 
-	let mut instance = sp_sandbox::Instance::new(code, &env_builder, &mut state)?;
+	let mut instance = T::new(code, &env_builder, &mut state)?;
 	let result = instance.invoke("call", args, &mut state);
 
 	result.map_err(|_| sp_sandbox::HostError)

client/executor/src/integration_tests/mod.rs

@@ -71,6 +71,53 @@ macro_rules! test_wasm_execution {
 	};
 }
 
+/// A macro to run a given test for each available WASM execution method *and* for each
+/// sandbox execution method.
+#[macro_export]
+macro_rules! test_wasm_execution_sandbox {
+	($method_name:ident) => {
+		paste::item! {
+			#[test]
+			fn [<$method_name _interpreted_host_executor>]() {
+				$method_name(WasmExecutionMethod::Interpreted, "_host");
+			}
+
+			#[test]
+			fn [<$method_name _interpreted_embedded_executor>]() {
+				$method_name(WasmExecutionMethod::Interpreted, "_embedded");
+			}
+
+			#[test]
+			#[cfg(feature = "wasmtime")]
+			fn [<$method_name _compiled_host_executor>]() {
+				$method_name(WasmExecutionMethod::Compiled, "_host");
+			}
+
+			#[test]
+			#[cfg(feature = "wasmtime")]
+			fn [<$method_name _compiled_embedded_executor>]() {
+				$method_name(WasmExecutionMethod::Compiled, "_embedded");
+			}
+		}
+	};
+
+	(interpreted_only $method_name:ident) => {
+		paste::item! {
+			#[test]
+			fn [<$method_name _interpreted_host_executor>]() {
+				$method_name(WasmExecutionMethod::Interpreted, "_host");
+			}
+		}
+
+		paste::item! {
+			#[test]
+			fn [<$method_name _interpreted_embedded_executor>]() {
+				$method_name(WasmExecutionMethod::Interpreted, "_embedded");
+			}
+		}
+	};
+}
+
 fn call_in_wasm<E: Externalities>(
 	function: &str,
 	call_data: &[u8],