Argument passing and returning values when invoking sandboxed funcs (#189)

substrate/Cargo.lock

@@ -2199,6 +2199,7 @@ dependencies = [
  "substrate-primitives 0.1.0",
  "substrate-runtime-io 0.1.0",
  "substrate-runtime-std 0.1.0",
+ "wabt 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "wasmi 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 

substrate/substrate/executor/src/sandbox.rs

@@ -25,8 +25,10 @@ use primitives::sandbox as sandbox_primitives;
 use wasm_utils::DummyUserError;
 use wasmi;
 use wasmi::memory_units::Pages;
-use wasmi::{Externals, FuncRef, ImportResolver, MemoryInstance, MemoryRef, Module, ModuleInstance,
-            ModuleRef, RuntimeArgs, RuntimeValue, Trap, TrapKind};
+use wasmi::{
+	Externals, FuncRef, ImportResolver, MemoryInstance, MemoryRef, Module, ModuleInstance,
+	ModuleRef, RuntimeArgs, RuntimeValue, Trap, TrapKind
+};
 
 /// Index of a function inside the supervisor.
 ///
@@ -111,22 +113,26 @@ impl ImportResolver for Imports {
 
 	fn resolve_global(
 		&self,
-		_module_name: &str,
-		_field_name: &str,
+		module_name: &str,
+		field_name: &str,
 		_global_type: &::wasmi::GlobalDescriptor,
 	) -> Result<::wasmi::GlobalRef, ::wasmi::Error> {
-		// TODO:
-		unimplemented!()
+		Err(::wasmi::Error::Instantiation(format!(
+			"Export {}:{} not found",
+			module_name, field_name
+		)))
 	}
 
 	fn resolve_table(
 		&self,
-		_module_name: &str,
-		_field_name: &str,
+		module_name: &str,
+		field_name: &str,
 		_table_type: &::wasmi::TableDescriptor,
 	) -> Result<::wasmi::TableRef, ::wasmi::Error> {
-		// TODO:
-		unimplemented!()
+		Err(::wasmi::Error::Instantiation(format!(
+			"Export {}:{} not found",
+			module_name, field_name
+		)))
 	}
 }
 
@@ -259,7 +265,8 @@ impl<'a, FE: SandboxCapabilities + Externals + 'a> Externals for GuestExternals<
 		self.supervisor_externals
 			.deallocate(serialized_result_val_ptr);
 
-		// TODO: check the signature?
+		// We do not have to check the signature here, because it's automatically
+		// checked by wasmi.
 
 		deserialize_result(&serialized_result_val)
 	}
@@ -610,4 +617,60 @@ mod tests {
 			vec![1],
 		);
 	}
+
+	#[test]
+	fn invoke_args() {
+		let mut ext = TestExternalities::default();
+		let test_code = include_bytes!("../wasm/target/wasm32-unknown-unknown/release/runtime_test.compact.wasm");
+
+		let code = wabt::wat2wasm(r#"
+		(module
+			(import "env" "assert" (func $assert (param i32)))
+
+			(func (export "call") (param $x i32) (param $y i64)
+				;; assert that $x = 0x12345678
+				(call $assert
+					(i32.eq
+						(get_local $x)
+						(i32.const 0x12345678)
+					)
+				)
+
+				(call $assert
+					(i64.eq
+						(get_local $y)
+						(i64.const 0x1234567887654321)
+					)
+				)
+			)
+		)
+		"#).unwrap();
+
+		assert_eq!(
+			WasmExecutor.call(&mut ext, &test_code[..], "test_sandbox_args", &code).unwrap(),
+			vec![1],
+		);
+	}
+
+	#[test]
+	fn return_val() {
+		let mut ext = TestExternalities::default();
+		let test_code = include_bytes!("../wasm/target/wasm32-unknown-unknown/release/runtime_test.compact.wasm");
+
+		let code = wabt::wat2wasm(r#"
+		(module
+			(func (export "call") (param $x i32) (result i32)
+				(i32.add
+					(get_local $x)
+					(i32.const 1)
+				)
+			)
+		)
+		"#).unwrap();
+
+		assert_eq!(
+			WasmExecutor.call(&mut ext, &test_code[..], "test_sandbox_return_val", &code).unwrap(),
+			vec![1],
+		);
+	}
 }

substrate/substrate/executor/src/wasm_executor.rs

@@ -360,7 +360,9 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
 		this.sandbox_store.instance_teardown(instance_idx)?;
 		Ok(())
 	},
-	ext_sandbox_invoke(instance_idx: u32, export_ptr: *const u8, export_len: usize, state: usize) -> u32 => {
+	ext_sandbox_invoke(instance_idx: u32, export_ptr: *const u8, export_len: usize, args_ptr: *const u8, args_len: usize, return_val_ptr: *const u8, return_val_len: usize, state: usize) -> u32 => {
+		use codec::Slicable;
+
 		trace!(target: "runtime-sandbox", "invoke, instance_idx={}", instance_idx);
 		let export = this.memory.get(export_ptr, export_len as usize)
 			.map_err(|_| DummyUserError)
@@ -369,12 +371,32 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
 					.map_err(|_| DummyUserError)
 			)?;
 
+		// Deserialize arguments and convert them into wasmi types.
+		let serialized_args = this.memory.get(args_ptr, args_len as usize)
+			.map_err(|_| DummyUserError)?;
+		let args = Vec::<sandbox_primitives::TypedValue>::decode(&mut &serialized_args[..])
+			.ok_or_else(|| DummyUserError)?
+			.into_iter()
+			.map(Into::into)
+			.collect::<Vec<_>>();
+
 		let instance = this.sandbox_store.instance(instance_idx)?;
-		let result = instance.invoke(&export, &[], this, state);
+		let result = instance.invoke(&export, &args, this, state);
+
 		match result {
 			Ok(None) => Ok(sandbox_primitives::ERR_OK),
-			// TODO: Return value
-			Ok(_) => unimplemented!(),
+			Ok(Some(val)) => {
+				// Serialize return value and write it back into the memory.
+				sandbox_primitives::ReturnValue::Value(val.into()).using_encoded(|val| {
+					if val.len() > return_val_len as usize {
+						Err(DummyUserError)?;
+					}
+					this.memory
+						.set(return_val_ptr, val)
+						.map_err(|_| DummyUserError)?;
+					Ok(sandbox_primitives::ERR_OK)
+				})
+			}
 			Err(_) => Ok(sandbox_primitives::ERR_EXECUTION),
 		}
 	},

substrate/substrate/executor/wasm/src/lib.rs

@@ -54,12 +54,32 @@ impl_stubs!(
 		enumerated_trie_root(&[&b"zero"[..], &b"one"[..], &b"two"[..]]).to_vec()
 	},
 	test_sandbox NO_DECODE => |code: &[u8]| {
-		let result = execute_sandboxed(code).is_ok();
-		[result as u8].to_vec()
+		let ok = execute_sandboxed(code, &[]).is_ok();
+		[ok as u8].to_vec()
+	},
+	test_sandbox_args NO_DECODE => |code: &[u8]| {
+		let ok = execute_sandboxed(
+			code,
+			&[
+				sandbox::TypedValue::I32(0x12345678),
+				sandbox::TypedValue::I64(0x1234567887654321),
+			]
+		).is_ok();
+		[ok as u8].to_vec()
+	},
+	test_sandbox_return_val NO_DECODE => |code: &[u8]| {
+		let result = execute_sandboxed(
+			code,
+			&[
+				sandbox::TypedValue::I32(0x1336),
+			]
+		);
+		let ok = if let Ok(sandbox::ReturnValue::Value(sandbox::TypedValue::I32(0x1337))) = result { true } else { false };
+		[ok as u8].to_vec()
 	}
 );
 
-fn execute_sandboxed(code: &[u8]) -> Result<sandbox::ReturnValue, sandbox::HostError> {
+fn execute_sandboxed(code: &[u8], args: &[sandbox::TypedValue]) -> Result<sandbox::ReturnValue, sandbox::HostError> {
 	struct State {
 		counter: u32,
 	}
@@ -91,7 +111,7 @@ fn execute_sandboxed(code: &[u8]) -> Result<sandbox::ReturnValue, sandbox::HostE
 	env_builder.add_host_func("env", "inc_counter", env_inc_counter);
 
 	let mut instance = sandbox::Instance::new(code, &env_builder, &mut state)?;
-	let result = instance.invoke(b"call", &[], &mut state);
+	let result = instance.invoke(b"call", args, &mut state);
 
 	result.map_err(|_| sandbox::HostError)
 }

substrate/substrate/primitives/src/sandbox.rs

@@ -180,6 +180,23 @@ impl Slicable for ReturnValue {
 	}
 }
 
+impl ReturnValue {
+	/// Maximum number of bytes `ReturnValue` might occupy when serialized with
+	/// `Slicable`.
+	///
+	/// Breakdown:
+	///  1 byte for encoding unit/value variant
+	///  1 byte for encoding value type
+	///  8 bytes for encoding the biggest value types available in wasm: f64, i64.
+	pub const ENCODED_MAX_SIZE: usize = 10;
+}
+
+#[test]
+fn return_value_encoded_max_size() {
+	let encoded = ReturnValue::Value(TypedValue::I64(-1)).encode();
+	assert_eq!(encoded.len(), ReturnValue::ENCODED_MAX_SIZE);
+}
+
 #[derive(Clone, Copy, PartialEq, Eq)]
 #[cfg_attr(feature = "std", derive(Debug))]
 #[repr(i8)]

substrate/substrate/runtime-io/without_std.rs

@@ -29,12 +29,12 @@ pub use rstd::{mem, slice};
 
 #[panic_implementation]
 #[no_mangle]
-pub fn panic(_info: &core::panic::PanicInfo) -> ! {
+pub fn panic(info: &::core::panic::PanicInfo) -> ! {
 	unsafe {
-		if let Some(location) = _info.location() {
-			ext_print_utf8(location.file().as_ptr() as *const u8, location.file().len() as u32);
-			ext_print_num(location.line() as u64);
-			ext_print_num(location.column() as u64);
+		if let Some(loc) = info.location() {
+			ext_print_utf8(loc.file().as_ptr() as *const u8, loc.file().len() as u32);
+			ext_print_num(loc.line() as u64);
+			ext_print_num(loc.column() as u64);
 		}
 		intrinsics::abort()
 	}

substrate/substrate/runtime-sandbox/Cargo.toml

@@ -14,6 +14,9 @@ substrate-runtime-std = { path = "../runtime-std", default_features = false }
 substrate-runtime-io = { path = "../runtime-io", default_features = false }
 substrate-codec = { path = "../codec", default_features = false }
 
+[dev-dependencies]
+wabt = "0.1.7"
+
 [features]
 default = ["std"]
 std = [

substrate/substrate/runtime-sandbox/src/lib.rs

@@ -40,9 +40,13 @@
 
 extern crate substrate_codec as codec;
 extern crate substrate_runtime_io as runtime_io;
+#[cfg_attr(not(feature = "std"), macro_use)]
 extern crate substrate_runtime_std as rstd;
 extern crate substrate_primitives as primitives;
 
+#[cfg(test)]
+extern crate wabt;
+
 use rstd::prelude::*;
 
 pub use primitives::sandbox::{TypedValue, ReturnValue, HostError};

substrate/substrate/runtime-sandbox/with_std.rs

@@ -20,9 +20,11 @@ use rstd::collections::btree_map::BTreeMap;
 use rstd::fmt;
 
 
-use self::wasmi::{Externals, FuncInstance, FuncRef, GlobalDescriptor, GlobalRef, ImportResolver,
-                  MemoryDescriptor, MemoryInstance, MemoryRef, Module, ModuleInstance, ModuleRef,
-                  RuntimeArgs, RuntimeValue, Signature, TableDescriptor, TableRef, Trap, TrapKind};
+use self::wasmi::{
+	Externals, FuncInstance, FuncRef, GlobalDescriptor, GlobalRef, ImportResolver,
+	MemoryDescriptor, MemoryInstance, MemoryRef, Module, ModuleInstance, ModuleRef,
+	RuntimeArgs, RuntimeValue, Signature, TableDescriptor, TableRef, Trap, TrapKind
+};
 use self::wasmi::memory_units::Pages;
 use super::{Error, TypedValue, ReturnValue, HostFuncType, HostError};
 
@@ -208,8 +210,9 @@ impl<T> ImportResolver for EnvironmentDefinitionBuilder<T> {
 		_field_name: &str,
 		_global_type: &GlobalDescriptor,
 	) -> Result<GlobalRef, wasmi::Error> {
-		// TODO: Implement sandboxed globals.
-		unimplemented!()
+		Err(wasmi::Error::Instantiation(format!(
+			"Importing globals is not supported yet"
+		)))
 	}
 
 	fn resolve_memory(
@@ -243,8 +246,9 @@ impl<T> ImportResolver for EnvironmentDefinitionBuilder<T> {
 		_field_name: &str,
 		_table_type: &TableDescriptor,
 	) -> Result<TableRef, wasmi::Error> {
-		// TODO: Implement sandboxed tables.
-		unimplemented!()
+		Err(wasmi::Error::Instantiation(format!(
+			"Importing tables is not supported yet"
+		)))
 	}
 }
 
@@ -284,10 +288,7 @@ impl<T> Instance<T> {
 		args: &[TypedValue],
 		state: &mut T,
 	) -> Result<ReturnValue, Error> {
-		if args.len() > 0 {
-			// TODO: Convert args into `RuntimeValue` and use it.
-			unimplemented!();
-		}
+		let args = args.iter().cloned().map(Into::into).collect::<Vec<_>>();
 
 		let name = ::std::str::from_utf8(name).map_err(|_| Error::Execution)?;
 		let mut externals = GuestExternals {
@@ -295,15 +296,112 @@ impl<T> Instance<T> {
 			defined_host_functions: &self.defined_host_functions,
 		};
 		let result = self.instance
-			.invoke_export(&name, &[], &mut externals);
+			.invoke_export(&name, &args, &mut externals);
 
 		match result {
 			Ok(None) => Ok(ReturnValue::Unit),
-			Ok(_val) => {
-				// TODO: Convert result value into `TypedValue` and return it.
-				unimplemented!();
-			}
+			Ok(Some(val)) => Ok(ReturnValue::Value(val.into())),
 			Err(_err) => Err(Error::Execution),
 		}
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use wabt;
+	use ::{TypedValue, ReturnValue, HostError, EnvironmentDefinitionBuilder, Instance};
+
+	fn execute_sandboxed(code: &[u8], args: &[TypedValue]) -> Result<ReturnValue, HostError> {
+		struct State {
+			counter: u32,
+		}
+
+		fn env_assert(_e: &mut State, args: &[TypedValue]) -> Result<ReturnValue, HostError> {
+			if args.len() != 1 {
+				return Err(HostError);
+			}
+			let condition = args[0].as_i32().ok_or_else(|| HostError)?;
+			if condition != 0 {
+				Ok(ReturnValue::Unit)
+			} else {
+				Err(HostError)
+			}
+		}
+		fn env_inc_counter(e: &mut State, args: &[TypedValue]) -> Result<ReturnValue, HostError> {
+			if args.len() != 1 {
+				return Err(HostError);
+			}
+			let inc_by = args[0].as_i32().ok_or_else(|| HostError)?;
+			e.counter += inc_by as u32;
+			Ok(ReturnValue::Value(TypedValue::I32(e.counter as i32)))
+		}
+
+		let mut state = State { counter: 0 };
+
+		let mut env_builder = EnvironmentDefinitionBuilder::new();
+		env_builder.add_host_func("env", "assert", env_assert);
+		env_builder.add_host_func("env", "inc_counter", env_inc_counter);
+
+		let mut instance = Instance::new(code, &env_builder, &mut state)?;
+		let result = instance.invoke(b"call", args, &mut state);
+
+		result.map_err(|_| HostError)
+	}
+
+	#[test]
+	fn invoke_args() {
+		let code = wabt::wat2wasm(r#"
+		(module
+			(import "env" "assert" (func $assert (param i32)))
+
+			(func (export "call") (param $x i32) (param $y i64)
+				;; assert that $x = 0x12345678
+				(call $assert
+					(i32.eq
+						(get_local $x)
+						(i32.const 0x12345678)
+					)
+				)
+
+				(call $assert
+					(i64.eq
+						(get_local $y)
+						(i64.const 0x1234567887654321)
+					)
+				)
+			)
+		)
+		"#).unwrap();
+
+		let result = execute_sandboxed(
+			&code,
+			&[
+				TypedValue::I32(0x12345678),
+				TypedValue::I64(0x1234567887654321),
+			]
+		);
+		assert!(result.is_ok());
+	}
+
+	#[test]
+	fn return_value() {
+		let code = wabt::wat2wasm(r#"
+		(module
+			(func (export "call") (param $x i32) (result i32)
+				(i32.add
+					(get_local $x)
+					(i32.const 1)
+				)
+			)
+		)
+		"#).unwrap();
+
+		let return_val = execute_sandboxed(
+			&code,
+			&[
+				TypedValue::I32(0x1336),
+			]
+		).unwrap();
+		assert_eq!(return_val, ReturnValue::Value(TypedValue::I32(0x1337)));
+	}
+}

substrate/substrate/runtime-sandbox/without_std.rs

@@ -61,6 +61,10 @@ mod ffi {
 			instance_idx: u32,
 			export_ptr: *const u8,
 			export_len: usize,
+			args_ptr: *const u8,
+			args_len: usize,
+			return_val_ptr: *mut u8,
+			return_val_len: usize,
 			state: usize,
 		) -> u32;
 		pub fn ext_sandbox_memory_new(initial: u32, maximum: u32) -> u32;
@@ -260,16 +264,29 @@ impl<T> Instance<T> {
 	pub fn invoke(
 		&mut self,
 		name: &[u8],
-		_args: &[TypedValue],
+		args: &[TypedValue],
 		state: &mut T,
 	) -> Result<ReturnValue, Error> {
-		// TODO: Serialize arguments and pass them thru.
-		let result =
-			unsafe { ffi::ext_sandbox_invoke(self.instance_idx, name.as_ptr(), name.len(), state as *const T as usize) };
+		let serialized_args = args.to_vec().encode();
+		let mut return_val = vec![0u8; sandbox_primitives::ReturnValue::ENCODED_MAX_SIZE];
+
+		let result = unsafe {
+			ffi::ext_sandbox_invoke(
+				self.instance_idx,
+				name.as_ptr(),
+				name.len(),
+				serialized_args.as_ptr(),
+				serialized_args.len(),
+				return_val.as_mut_ptr(),
+				return_val.len(),
+				state as *const T as usize,
+			)
+		};
 		match result {
 			sandbox_primitives::ERR_OK => {
-				// TODO: Fetch the result of the execution.
-				Ok(ReturnValue::Unit)
+				let return_val = sandbox_primitives::ReturnValue::decode(&mut &return_val[..])
+					.ok_or(Error::Execution)?;
+				Ok(return_val)
 			}
 			sandbox_primitives::ERR_EXECUTION => Err(Error::Execution),
 			_ => unreachable!(),