Refactor sp-sandbox; make sure both sandbox executors are always tested

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,12 +183,26 @@ sp_core::wasm_export_functions! {
 	   ).as_ref().to_vec()
    }
 
-   fn test_sandbox(code: Vec<u8>) -> bool {
-	   execute_sandboxed(&code, &[]).is_ok()
+   fn test_sandbox_host(code: Vec<u8>) -> bool {
+	   execute_sandboxed_host(&code, &[]).is_ok()
    }
 
-   fn test_sandbox_args(code: Vec<u8>) -> bool {
-	   execute_sandboxed(
+   fn test_sandbox_embedded(code: Vec<u8>) -> bool {
+	   execute_sandboxed_embedded(&code, &[]).is_ok()
+   }
+
+   fn test_sandbox_args_host(code: Vec<u8>) -> bool {
+	   execute_sandboxed_host(
+		   &code,
+		   &[
+			   Value::I32(0x12345678),
+			   Value::I64(0x1234567887654321),
+		   ],
+	   ).is_ok()
+   }
+
+   fn test_sandbox_args_embedded(code: Vec<u8>) -> bool {
+	   execute_sandboxed_embedded(
 		   &code,
 		   &[
 			   Value::I32(0x12345678),
@@ -197,8 +211,8 @@ sp_core::wasm_export_functions! {
 	   ).is_ok()
    }
 
-   fn test_sandbox_return_val(code: Vec<u8>) -> bool {
-	   let ok = match execute_sandboxed(
+   fn test_sandbox_return_val_host(code: Vec<u8>) -> bool {
+	   let ok = match execute_sandboxed_host(
 		   &code,
 		   &[
 			   Value::I32(0x1336),
@@ -211,9 +225,35 @@ sp_core::wasm_export_functions! {
 	   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 ()) {
+   fn test_sandbox_return_val_embedded(code: Vec<u8>) -> bool {
+	   let ok = match execute_sandboxed_embedded(
+		   &code,
+		   &[
+			   Value::I32(0x1336),
+		   ]
+	   ) {
+		   Ok(sp_sandbox::ReturnValue::Value(Value::I32(0x1337))) => true,
+		   _ => false,
+	   };
+
+	   ok
+   }
+
+   fn test_sandbox_instantiate_host(code: Vec<u8>) -> u8 {
+	   let env_builder = sp_sandbox::host_executor::EnvironmentDefinitionBuilder::new();
+	   let code = match sp_sandbox::host_executor::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_instantiate_embedded(code: Vec<u8>) -> u8 {
+	   let env_builder = sp_sandbox::embedded_executor::EnvironmentDefinitionBuilder::new();
+	   let code = match sp_sandbox::embedded_executor::Instance::new(&code, &env_builder, &mut ()) {
 		   Ok(_) => 0,
 		   Err(sp_sandbox::Error::Module) => 1,
 		   Err(sp_sandbox::Error::Execution) => 2,
@@ -223,9 +263,24 @@ sp_core::wasm_export_functions! {
 	   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 ()) {
+   fn test_sandbox_get_global_val_host(code: Vec<u8>) -> i64 {
+	   let env_builder = sp_sandbox::host_executor::EnvironmentDefinitionBuilder::new();
+	   let instance = if let Ok(i) = sp_sandbox::host_executor::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_sandbox_get_global_val_embedded(code: Vec<u8>) -> i64 {
+	   let env_builder = sp_sandbox::host_executor::EnvironmentDefinitionBuilder::new();
+	   let instance = if let Ok(i) = sp_sandbox::host_executor::Instance::new(&code, &env_builder, &mut ()) {
 		   i
 	   } else {
 		   return 20;
@@ -409,14 +464,18 @@ mod tasks {
 }
 
 #[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 +505,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,8 +521,24 @@ 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)
 }
+
+#[cfg(not(feature = "std"))]
+fn execute_sandboxed_host(
+	code: &[u8],
+	args: &[Value],
+) -> Result<sp_sandbox::ReturnValue, sp_sandbox::HostError> {
+	execute_sandboxed::<sp_sandbox::host_executor::Instance<State>>(code, args)
+}
+
+#[cfg(not(feature = "std"))]
+fn execute_sandboxed_embedded(
+	code: &[u8],
+	args: &[Value],
+) -> Result<sp_sandbox::ReturnValue, sp_sandbox::HostError> {
+	execute_sandboxed::<sp_sandbox::embedded_executor::Instance<State>>(code, args)
+}

client/executor/src/integration_tests/sandbox.rs

@@ -51,7 +51,14 @@ fn sandbox_should_work(wasm_method: WasmExecutionMethod) {
 	.unwrap()
 	.encode();
 
-	assert_eq!(call_in_wasm("test_sandbox", &code, wasm_method, &mut ext).unwrap(), true.encode());
+	assert_eq!(
+		call_in_wasm("test_sandbox_host", &code, wasm_method, &mut ext).unwrap(),
+		true.encode()
+	);
+	assert_eq!(
+		call_in_wasm("test_sandbox_embedded", &code, wasm_method, &mut ext).unwrap(),
+		true.encode()
+	);
 }
 
 test_wasm_execution!(sandbox_trap);
@@ -72,7 +79,11 @@ fn sandbox_trap(wasm_method: WasmExecutionMethod) {
 	)
 	.unwrap();
 
-	assert_eq!(call_in_wasm("test_sandbox", &code, wasm_method, &mut ext).unwrap(), vec![0]);
+	assert_eq!(call_in_wasm("test_sandbox_host", &code, wasm_method, &mut ext).unwrap(), vec![0]);
+	assert_eq!(
+		call_in_wasm("test_sandbox_embedded", &code, wasm_method, &mut ext).unwrap(),
+		vec![0]
+	);
 }
 
 test_wasm_execution!(start_called);
@@ -111,7 +122,14 @@ fn start_called(wasm_method: WasmExecutionMethod) {
 	.unwrap()
 	.encode();
 
-	assert_eq!(call_in_wasm("test_sandbox", &code, wasm_method, &mut ext).unwrap(), true.encode());
+	assert_eq!(
+		call_in_wasm("test_sandbox_host", &code, wasm_method, &mut ext).unwrap(),
+		true.encode()
+	);
+	assert_eq!(
+		call_in_wasm("test_sandbox_embedded", &code, wasm_method, &mut ext).unwrap(),
+		true.encode()
+	);
 }
 
 test_wasm_execution!(invoke_args);
@@ -147,7 +165,11 @@ fn invoke_args(wasm_method: WasmExecutionMethod) {
 	.encode();
 
 	assert_eq!(
-		call_in_wasm("test_sandbox_args", &code, wasm_method, &mut ext,).unwrap(),
+		call_in_wasm("test_sandbox_args_host", &code, wasm_method, &mut ext,).unwrap(),
+		true.encode(),
+	);
+	assert_eq!(
+		call_in_wasm("test_sandbox_args_embedded", &code, wasm_method, &mut ext,).unwrap(),
 		true.encode(),
 	);
 }
@@ -173,7 +195,11 @@ fn return_val(wasm_method: WasmExecutionMethod) {
 	.encode();
 
 	assert_eq!(
-		call_in_wasm("test_sandbox_return_val", &code, wasm_method, &mut ext,).unwrap(),
+		call_in_wasm("test_sandbox_return_val_host", &code, wasm_method, &mut ext,).unwrap(),
+		true.encode(),
+	);
+	assert_eq!(
+		call_in_wasm("test_sandbox_return_val_embedded", &code, wasm_method, &mut ext,).unwrap(),
 		true.encode(),
 	);
 }
@@ -197,7 +223,11 @@ fn unlinkable_module(wasm_method: WasmExecutionMethod) {
 	.encode();
 
 	assert_eq!(
-		call_in_wasm("test_sandbox_instantiate", &code, wasm_method, &mut ext,).unwrap(),
+		call_in_wasm("test_sandbox_instantiate_host", &code, wasm_method, &mut ext,).unwrap(),
+		1u8.encode(),
+	);
+	assert_eq!(
+		call_in_wasm("test_sandbox_instantiate_embedded", &code, wasm_method, &mut ext,).unwrap(),
 		1u8.encode(),
 	);
 }
@@ -211,7 +241,11 @@ fn corrupted_module(wasm_method: WasmExecutionMethod) {
 	let code = vec![0u8, 0, 0, 0, 1, 0, 0, 0].encode();
 
 	assert_eq!(
-		call_in_wasm("test_sandbox_instantiate", &code, wasm_method, &mut ext,).unwrap(),
+		call_in_wasm("test_sandbox_instantiate_host", &code, wasm_method, &mut ext,).unwrap(),
+		1u8.encode(),
+	);
+	assert_eq!(
+		call_in_wasm("test_sandbox_instantiate_embedded", &code, wasm_method, &mut ext,).unwrap(),
 		1u8.encode(),
 	);
 }
@@ -238,7 +272,11 @@ fn start_fn_ok(wasm_method: WasmExecutionMethod) {
 	.encode();
 
 	assert_eq!(
-		call_in_wasm("test_sandbox_instantiate", &code, wasm_method, &mut ext,).unwrap(),
+		call_in_wasm("test_sandbox_instantiate_host", &code, wasm_method, &mut ext,).unwrap(),
+		0u8.encode(),
+	);
+	assert_eq!(
+		call_in_wasm("test_sandbox_instantiate_embedded", &code, wasm_method, &mut ext,).unwrap(),
 		0u8.encode(),
 	);
 }
@@ -266,7 +304,11 @@ fn start_fn_traps(wasm_method: WasmExecutionMethod) {
 	.encode();
 
 	assert_eq!(
-		call_in_wasm("test_sandbox_instantiate", &code, wasm_method, &mut ext,).unwrap(),
+		call_in_wasm("test_sandbox_instantiate_host", &code, wasm_method, &mut ext,).unwrap(),
+		2u8.encode(),
+	);
+	assert_eq!(
+		call_in_wasm("test_sandbox_instantiate_embedded", &code, wasm_method, &mut ext,).unwrap(),
 		2u8.encode(),
 	);
 }
@@ -287,7 +329,12 @@ fn get_global_val_works(wasm_method: WasmExecutionMethod) {
 	.encode();
 
 	assert_eq!(
-		call_in_wasm("test_sandbox_get_global_val", &code, wasm_method, &mut ext,).unwrap(),
+		call_in_wasm("test_sandbox_get_global_val_host", &code, wasm_method, &mut ext,).unwrap(),
+		500i64.encode(),
+	);
+	assert_eq!(
+		call_in_wasm("test_sandbox_get_global_val_embedded", &code, wasm_method, &mut ext,)
+			.unwrap(),
 		500i64.encode(),
 	);
 }

frame/contracts/src/benchmarking/code.rs

@@ -35,7 +35,10 @@ use pwasm_utils::parity_wasm::{
 };
 use sp_core::crypto::UncheckedFrom;
 use sp_runtime::traits::Hash;
-use sp_sandbox::{EnvironmentDefinitionBuilder, Memory};
+use sp_sandbox::{
+	default_executor::{EnvironmentDefinitionBuilder, Memory},
+	SandboxEnvironmentBuilder, SandboxMemory,
+};
 use sp_std::{borrow::ToOwned, convert::TryFrom, prelude::*};
 
 /// Pass to `create_code` in order to create a compiled `WasmModule`.

frame/contracts/src/benchmarking/sandbox.rs

@@ -20,7 +20,10 @@
 /// ! environment that provides the seal interface as imported functions.
 use super::{code::WasmModule, Config};
 use sp_core::crypto::UncheckedFrom;
-use sp_sandbox::{EnvironmentDefinitionBuilder, Instance, Memory};
+use sp_sandbox::{
+	default_executor::{EnvironmentDefinitionBuilder, Instance, Memory},
+	SandboxEnvironmentBuilder, SandboxInstance,
+};
 
 /// Minimal execution environment without any exported functions.
 pub struct Sandbox {

frame/contracts/src/wasm/mod.rs

@@ -36,6 +36,7 @@ use crate::{
 use codec::{Decode, Encode};
 use frame_support::dispatch::DispatchError;
 use sp_core::crypto::UncheckedFrom;
+use sp_sandbox::{SandboxEnvironmentBuilder, SandboxInstance, SandboxMemory};
 use sp_std::prelude::*;
 #[cfg(test)]
 pub use tests::MockExt;
@@ -182,8 +183,8 @@ where
 		function: &ExportedFunction,
 		input_data: Vec<u8>,
 	) -> ExecResult {
-		let memory =
-			sp_sandbox::Memory::new(self.initial, Some(self.maximum)).unwrap_or_else(|_| {
+		let memory = sp_sandbox::default_executor::Memory::new(self.initial, Some(self.maximum))
+			.unwrap_or_else(|_| {
 				// unlike `.expect`, explicit panic preserves the source location.
 				// Needed as we can't use `RUST_BACKTRACE` in here.
 				panic!(
@@ -193,7 +194,7 @@ where
 				)
 			});
 
-		let mut imports = sp_sandbox::EnvironmentDefinitionBuilder::new();
+		let mut imports = sp_sandbox::default_executor::EnvironmentDefinitionBuilder::new();
 		imports.add_memory(self::prepare::IMPORT_MODULE_MEMORY, "memory", memory.clone());
 		runtime::Env::impls(&mut |module, name, func_ptr| {
 			imports.add_host_func(module, name, func_ptr);
@@ -209,7 +210,7 @@ where
 
 		// Instantiate the instance from the instrumented module code and invoke the contract
 		// entrypoint.
-		let result = sp_sandbox::Instance::new(&code, &imports, &mut runtime)
+		let result = sp_sandbox::default_executor::Instance::new(&code, &imports, &mut runtime)
 			.and_then(|mut instance| instance.invoke(function.identifier(), &[], &mut runtime));
 
 		runtime.to_execution_result(result)