Contract signatures checking (#478)

* Convert T in vm::Ext to a associated type

* WIP

* Fix BTreeMap

* Extract prepare module from vm.

* Move tests as well.

* Fix doc comment.

* macro for env defintion

* Fix prepare tests.

* Clean up

* Renamings

* Refactor scan_imports

* Improve docs

* Docs.

* Add some tests for sandbox module

* Clean up

* Use Error::Instantiate instead of ::Deserialize

* Add test for imports

* Add wrong sig import

* Clean up

* Rebuild binaries.

* Use "another_module" instead of obscure "vne"

Since "vne" looks like an actual typo

Author: pepyakin

Cargo.lock

@@ -16,6 +16,7 @@ substrate-codec = { path = "../codec", default_features = false }
 
 [dev-dependencies]
 wabt = "0.4"
+assert_matches = "1.1"
 
 [features]
 default = ["std"]

demo/runtime/wasm/target/wasm32-unknown-unknown/release/demo_runtime.compact.wasm

@@ -29,8 +29,11 @@
 //! When this crate is used in `std` environment all these functions are implemented by directly
 //! calling wasm VM.
 //!
-//! Typical use-case for this library might be used for implementing smart-contract runtimes
-//! which uses wasm for contract code.
+//! Example of possible use-cases for this library are following:
+//!
+//! - implementing smart-contract runtimes which uses wasm for contract code
+//! - executing wasm substrate runtime inside of a wasm parachain
+//! - etc
 
 #![warn(missing_docs)]
 #![cfg_attr(not(feature = "std"), no_std)]
@@ -47,6 +50,10 @@ extern crate substrate_primitives as primitives;
 #[cfg(test)]
 extern crate wabt;
 
+#[cfg(test)]
+#[macro_use]
+extern crate assert_matches;
+
 use rstd::prelude::*;
 
 pub use primitives::sandbox::{TypedValue, ReturnValue, HostError};
@@ -149,6 +156,11 @@ impl<T> EnvironmentDefinitionBuilder<T> {
 	}
 
 	/// Register a host function in this environment defintion.
+	///
+	/// NOTE that there is no constraints on type of this function. An instance
+	/// can import function passed here with any signature it wants. It can even import
+	/// the same function (i.e. with same `module` and `field`) several times. It's up to
+	/// the user code to check or constrain the types of signatures.
 	pub fn add_host_func<N1, N2>(&mut self, module: N1, field: N2, f: HostFuncType<T>)
 	where
 		N1: Into<Vec<u8>>,

demo/runtime/wasm/target/wasm32-unknown-unknown/release/demo_runtime.wasm

@@ -309,7 +309,7 @@ impl<T> Instance<T> {
 #[cfg(test)]
 mod tests {
 	use wabt;
-	use ::{TypedValue, ReturnValue, HostError, EnvironmentDefinitionBuilder, Instance};
+	use ::{Error, TypedValue, ReturnValue, HostError, EnvironmentDefinitionBuilder, Instance};
 
 	fn execute_sandboxed(code: &[u8], args: &[TypedValue]) -> Result<ReturnValue, HostError> {
 		struct State {
@@ -335,12 +335,20 @@ mod tests {
 			e.counter += inc_by as u32;
 			Ok(ReturnValue::Value(TypedValue::I32(e.counter as i32)))
 		}
+		/// Function that takes one argument of any type and returns that value.
+		fn env_polymorphic_id(_e: &mut State, args: &[TypedValue]) -> Result<ReturnValue, HostError> {
+			if args.len() != 1 {
+				return Err(HostError);
+			}
+			Ok(ReturnValue::Value(args[0]))
+		}
 
 		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);
+		env_builder.add_host_func("env", "polymorphic_id", env_polymorphic_id);
 
 		let mut instance = Instance::new(code, &env_builder, &mut state)?;
 		let result = instance.invoke(b"call", args, &mut state);
@@ -404,4 +412,71 @@ mod tests {
 		).unwrap();
 		assert_eq!(return_val, ReturnValue::Value(TypedValue::I32(0x1337)));
 	}
+
+	#[test]
+	fn signatures_dont_matter() {
+		let code = wabt::wat2wasm(r#"
+		(module
+			(import "env" "polymorphic_id" (func $id_i32 (param i32) (result i32)))
+			(import "env" "polymorphic_id" (func $id_i64 (param i64) (result i64)))
+			(import "env" "assert" (func $assert (param i32)))
+
+			(func (export "call")
+				;; assert that we can actually call the "same" function with different
+				;; signatures.
+				(call $assert
+					(i32.eq
+						(call $id_i32
+							(i32.const 0x012345678)
+						)
+						(i32.const 0x012345678)
+					)
+				)
+				(call $assert
+					(i64.eq
+						(call $id_i64
+							(i64.const 0x0123456789abcdef)
+						)
+						(i64.const 0x0123456789abcdef)
+					)
+				)
+			)
+		)
+		"#).unwrap();
+
+		let return_val = execute_sandboxed(&code, &[]).unwrap();
+		assert_eq!(return_val, ReturnValue::Unit);
+	}
+
+	#[test]
+	fn cant_return_unmatching_type() {
+		fn env_returns_i32(_e: &mut (), _args: &[TypedValue]) -> Result<ReturnValue, HostError> {
+			Ok(ReturnValue::Value(TypedValue::I32(42)))
+		}
+
+		let mut env_builder = EnvironmentDefinitionBuilder::new();
+		env_builder.add_host_func("env", "returns_i32", env_returns_i32);
+
+		let code = wabt::wat2wasm(r#"
+		(module
+			;; It's actually returns i32, but imported as if it returned i64
+			(import "env" "returns_i32" (func $returns_i32 (result i64)))
+
+			(func (export "call")
+				(drop
+					(call $returns_i32)
+				)
+			)
+		)
+		"#).unwrap();
+
+		// It succeeds since we are able to import functions with types we want.
+		let mut instance = Instance::new(&code, &env_builder, &mut ()).unwrap();
+
+		// But this fails since we imported a function that returns i32 as if it returned i64.
+		assert_matches!(
+			instance.invoke(b"call", &[], &mut ()),
+			Err(Error::Execution)
+		);
+	}
 }

substrate/runtime-sandbox/Cargo.toml

@@ -226,7 +226,9 @@ struct CallContext<'a, 'b: 'a, T: Trait + 'b> {
 	_caller: T::AccountId,
 }
 
-impl<'a, 'b: 'a, T: Trait + 'b> vm::Ext<T> for CallContext<'a, 'b, T> {
+impl<'a, 'b: 'a, T: Trait + 'b> vm::Ext for CallContext<'a, 'b, T> {
+	type T = T;
+
 	fn get_storage(&self, key: &[u8]) -> Option<Vec<u8>> {
 		self.ctx.overlay.get_storage(&self.ctx.self_account, key)
 	}