Add more documentation.

substrate/executor/src/sandbox.rs

@@ -14,6 +14,8 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
+//! This module implements sandboxing support in the runtime.
+
 use codec::Slicable;
 use primitives::sandbox as sandbox_primitives;
 use std::collections::HashMap;
@@ -45,9 +47,9 @@ impl GuestToSuperviserFunctionMapping {
 		GuestToSuperviserFunctionMapping { funcs: Vec::new() }
 	}
 
-	fn define(&mut self, host_func: SupervisorFuncIndex) -> GuestFuncIndex {
+	fn define(&mut self, supervisor_func: SupervisorFuncIndex) -> GuestFuncIndex {
 		let idx = self.funcs.len();
-		self.funcs.push(host_func);
+		self.funcs.push(supervisor_func);
 		GuestFuncIndex(idx)
 	}
 
@@ -124,26 +126,60 @@ impl ImportResolver for Imports {
 	}
 }
 
+/// This trait encapsulates sandboxing capabilities.
+///
+/// Note that this functions are only called in the `supervisor` context.
 pub trait SandboxCapabilities {
-	/// Returns associated `Store`.
+	/// Returns associated sandbox `Store`.
 	fn store(&self) -> &Store;
 
+	/// Allocate space of the specified length in the supervisor memory.
+	///
+	/// Returns pointer to the allocated block.
 	fn allocate(&mut self, len: u32) -> u32;
+
+	/// Deallocate space specified by the pointer that was previously returned by [`allocate`].
+	///
+	/// [`allocate`]: #tymethod.allocate
 	fn deallocate(&mut self, ptr: u32);
+
+	/// Write `data` into the supervisor memory at offset specified by `ptr`.
+	///
+	/// # Errors
+	///
+	/// Returns `Err` if `ptr + data.len()` is out of bounds.
 	fn write_memory(&mut self, ptr: u32, data: &[u8]) -> Result<(), DummyUserError>;
+
+	/// Read `len` bytes from the supervisor memory.
+	///
+	/// # Errors
+	///
+	/// Returns `Err` if `ptr + len` is out of bounds.
 	fn read_memory(&self, ptr: u32, len: u32) -> Result<Vec<u8>, DummyUserError>;
 }
 
+/// Implementation of [`Externals`] that allows execution of guest module with
+/// [externals][`Externals`] that might refer functions defined by supervisor.
+///
+/// [`Externals`]: ../../wasmi/trait.Externals.html
 pub struct GuestExternals<'a, FE: SandboxCapabilities + Externals + 'a> {
-	original_externals: &'a mut FE,
+	supervisor_externals: &'a mut FE,
 	instance_idx: u32,
 	state: u32,
 }
 
 impl<'a, FE: SandboxCapabilities + Externals + 'a> GuestExternals<'a, FE> {
-	pub fn new(original_externals: &mut FE, instance_idx: u32, state: u32) -> GuestExternals<FE> {
+	/// Create a new instance of `GuestExternals`.
+	///
+	/// It will use `supervisor_externals` to execute calls from guest to supervisor.
+	/// `instance_idx` required to fetch settings for this particular instance, e.g
+	/// associated dispatch thunk funtion and mapping between externals function ids to
+	/// functions in supervisor module.
+	/// `state` is just an integer that allows supervisor to have arbitrary state associated with the call,
+	/// typically used for implementing runtime functions.
+	pub fn new(supervisor_externals: &mut FE, instance_idx: u32, state: u32) -> GuestExternals<FE> {
 		GuestExternals {
-			original_externals,
+			supervisor_externals,
 			instance_idx,
 			state,
 		}
@@ -175,7 +211,7 @@ impl<'a, FE: SandboxCapabilities + Externals + 'a> Externals for GuestExternals<
 		args: RuntimeArgs,
 	) -> Result<Option<RuntimeValue>, Trap> {
 		let (func_idx, dispatch_thunk) = {
-			let instance = &self.original_externals.store().instances[self.instance_idx as usize];
+			let instance = &self.supervisor_externals.store().instances[self.instance_idx as usize];
 			let dispatch_thunk = instance.dispatch_thunk.clone();
 			let func_idx = instance
 				.guest_to_supervisor_mapping
@@ -203,9 +239,9 @@ impl<'a, FE: SandboxCapabilities + Externals + 'a> Externals for GuestExternals<
 
 		// Move serialized arguments inside the memory and invoke dispatch thunk and
 		// then free allocated memory.
-		let invoke_args_ptr = self.original_externals
+		let invoke_args_ptr = self.supervisor_externals
 			.allocate(invoke_args_data.len() as u32);
-		self.original_externals
+		self.supervisor_externals
 			.write_memory(invoke_args_ptr, &invoke_args_data)?;
 		let result = ::wasmi::FuncInstance::invoke(
 			&dispatch_thunk,
@@ -215,9 +251,9 @@ impl<'a, FE: SandboxCapabilities + Externals + 'a> Externals for GuestExternals<
 				RuntimeValue::I32(state as i32),
 				RuntimeValue::I32(func_idx.0 as i32),
 			],
-			self.original_externals,
+			self.supervisor_externals,
 		);
-		self.original_externals.deallocate(invoke_args_ptr);
+		self.supervisor_externals.deallocate(invoke_args_ptr);
 
 		// dispatch_thunk returns pointer to serialized arguments.
 		let (serialized_result_val_ptr, serialized_result_val_len) = match result {
@@ -232,9 +268,9 @@ impl<'a, FE: SandboxCapabilities + Externals + 'a> Externals for GuestExternals<
 			_ => return Err(trap()),
 		};
 
-		let serialized_result_val = self.original_externals
+		let serialized_result_val = self.supervisor_externals
 			.read_memory(serialized_result_val_ptr, serialized_result_val_len)?;
-		self.original_externals
+		self.supervisor_externals
 			.deallocate(serialized_result_val_ptr);
 
 		// TODO: check the signature?
@@ -294,13 +330,28 @@ pub struct Store {
 }
 
 impl Store {
+	/// Create new empty sandbox store.
 	pub fn new() -> Store {
 		Store {
 			instances: Vec::new(),
 			memories: Vec::new(),
 		}
 	}
 
+	/// Instantiate a guest module and return it's index in the store.
+	///
+	/// The guest module's code is specified in `wasm`. Environment that will be available to
+	/// guest module is specified in `raw_env_def` (serialized version of [`EnvironmentDefinition`]).
+	/// `dispatch_thunk` is used as function that handle calls from guests.
+	///
+	/// # Errors
+	///
+	/// Returns `Err` if any of the following conditions happens:
+	///
+	/// - `raw_env_def` can't be deserialized as a [`EnvironmentDefinition`].
+	/// - Module in `wasm` is invalid or couldn't be instantiated.
+	///
+	/// [`EnvironmentDefinition`]: ../../sandbox/struct.EnvironmentDefinition.html
 	pub fn instantiate(
 		&mut self,
 		dispatch_thunk: FuncRef,
@@ -328,27 +379,46 @@ impl Store {
 		Ok(instance_idx as u32)
 	}
 
-	pub fn new_memory(&mut self, initial: u32, maximum: u32) -> Option<u32> {
+	/// Create a new memory instance and return it's index.
+	///
+	/// # Errors
+	///
+	/// Returns `Err` if the memory couldn't be created.
+	/// Typically happens if `initial` is more than `maximum`.
+	pub fn new_memory(&mut self, initial: u32, maximum: u32) -> Result<u32, DummyUserError> {
 		let maximum = match maximum {
 			sandbox_primitives::MEM_UNLIMITED => None,
 			specified_limit => Some(Pages(specified_limit as usize)),
 		};
 
-		let mem =
-			MemoryInstance::alloc(Pages(initial as usize), maximum).ok()?;
+		let mem = MemoryInstance::alloc(Pages(initial as usize), maximum).map_err(|_| DummyUserError)?;
 		self.memories.push(mem);
 		let mem_idx = self.memories.len() - 1;
-		Some(mem_idx as u32)
+		Ok(mem_idx as u32)
 	}
 
-	pub fn instance(&self, instance_idx: u32) -> Option<ModuleRef> {
+	/// Returns `ModuleRef` by `instance_idx`.
+	///
+	/// # Errors
+	///
+	/// Returns `Err` If `instance_idx` isn't a valid index of an instance.
+	pub fn instance(&self, instance_idx: u32) -> Result<ModuleRef, DummyUserError> {
 		self.instances
 			.get(instance_idx as usize)
 			.map(|i| i.instance.clone())
+			.ok_or_else(|| DummyUserError)
 	}
 
-	pub fn memory(&self, memory_idx: u32) -> Option<MemoryRef> {
-		self.memories.get(memory_idx as usize).cloned()
+	/// Returns reference to a memory instance by `memory_idx`.
+	///
+	/// # Errors
+	///
+	/// Returns `Err` If `memory_idx` isn't a valid index of an instance.
+	pub fn memory(&self, memory_idx: u32) -> Result<MemoryRef, DummyUserError> {
+		self.memories
+			.get(memory_idx as usize)
+			.cloned()
+			.ok_or_else(|| DummyUserError)
 	}
 }
 

substrate/executor/src/wasm_executor.rs

@@ -359,7 +359,7 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
 					.map_err(|_| DummyUserError)
 			)?;
 
-		let instance = this.sandbox_store.instance(instance_idx).ok_or_else(|| DummyUserError)?;
+		let instance = this.sandbox_store.instance(instance_idx)?;
 
 		let mut guest_externals = sandbox::GuestExternals::new(this, instance_idx, state);
 
@@ -372,11 +372,11 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
 		}
 	},
 	ext_sandbox_memory_new(initial: u32, maximum: u32) -> u32 => {
-		let mem_idx = this.sandbox_store.new_memory(initial, maximum).ok_or_else(|| DummyUserError)?;
+		let mem_idx = this.sandbox_store.new_memory(initial, maximum)?;
 		Ok(mem_idx)
 	},
 	ext_sandbox_memory_get(memory_idx: u32, offset: u32, buf_ptr: *mut u8, buf_len: usize) -> u32 => {
-		let dst_memory = this.sandbox_store.memory(memory_idx).ok_or_else(|| DummyUserError)?;
+		let dst_memory = this.sandbox_store.memory(memory_idx)?;
 
 		let data: Vec<u8> = match dst_memory.get(offset, buf_len as usize) {
 			Ok(data) => data,
@@ -390,7 +390,7 @@ impl_function_executor!(this: FunctionExecutor<'e, E>,
 		Ok(sandbox_primitives::ERR_OK)
 	},
 	ext_sandbox_memory_set(memory_idx: u32, offset: u32, val_ptr: *const u8, val_len: usize) -> u32 => {
-		let dst_memory = this.sandbox_store.memory(memory_idx).ok_or_else(|| DummyUserError)?;
+		let dst_memory = this.sandbox_store.memory(memory_idx)?;
 
 		let data = match this.memory.get(offset, val_len as usize) {
 			Ok(data) => data,

substrate/runtime-sandbox/src/lib.rs

@@ -14,11 +14,23 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
-//! This crate provides means for sandboxed execution of wasm modules.
+//! This crate provides means of instantiation and execution of wasm modules.
 //!
-//! In case this crate is used within wasm execution environment
-//! then same VM will be used for execution of sandboxed code without
-//! comrpomising the security of the sandbox owner.
+//! It works even when the user of this library is itself executes
+//! inside the wasm VM. In this case same VM is used for execution
+//! of both the sandbox owner and the sandboxed module, without compromising security
+//! and without performance penalty of full wasm emulation inside wasm.
+//!
+//! This is achieved by using bindings to wasm VM which are published by the host API.
+//! This API is thin and consists of only handful functions. It contains functions for instantiating
+//! modules and executing them and for example doesn't contain functions for inspecting the module
+//! structure. The user of this library is supposed to read wasm module by it's own means.
+//!
+//! 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.
 
 #![warn(missing_docs)]
 #![cfg_attr(not(feature = "std"), no_std)]
@@ -65,29 +77,52 @@ impl From<Error> for HostError {
 	}
 }
 
-/// Callable function pointer.
+/// Function pointer for specifying functions by the
+/// supervisor in [`EnvironmentDefinitionBuilder`].
+///
+/// [`EnvironmentDefinitionBuilder`]: struct.EnvironmentDefinitionBuilder.html
 pub type HostFuncType<T> = fn(&mut T, &[TypedValue]) -> Result<ReturnValue, HostError>;
 
-/// Reference to a sandboxed linear memory.
+/// Reference to a sandboxed linear memory, that
+/// will be used by the guest module.
+///
+/// The memory can't be directly accessed by supervisor, but only
+/// through designated functions [`get`] and [`set`].
+///
+/// [`get`]: #method.get
+/// [`set`]: #method.set
 #[derive(Clone)]
 pub struct Memory {
 	inner: imp::Memory,
 }
 
 impl Memory {
 	/// Construct a new linear memory instance.
+	///
+	/// The memory allocated with initial number of pages specified by `initial`.
+	/// Minimal possible value for `initial` is 0 and maximum possible is `65536`.
+	/// (Since maximum addressible memory is 2<sup>32</sup> = 4GiB = 65536 * 64KiB).
+	///
+	/// It is possible to limit maximum number of pages this memory instance can have by specifying
+	/// `maximum`. If not specified, this memory instance would be able to allocate up to 4GiB.
+	///
+	/// Allocated memory is always zeroed.
 	pub fn new(initial: u32, maximum: Option<u32>) -> Result<Memory, Error> {
 		Ok(Memory {
 			inner: imp::Memory::new(initial, maximum)?,
 		})
 	}
 
 	/// Read a memory area at the address `ptr` with the size of the provided slice `buf`.
+	///
+	/// Returns `Err` if the range is out-of-bounds.
 	pub fn get(&self, ptr: u32, buf: &mut [u8]) -> Result<(), Error> {
 		self.inner.get(ptr, buf)
 	}
 
 	/// Write a memory area at the address `ptr` with contents of the provided slice `buf`.
+	///
+	/// Returns `Err` if the range is out-of-bounds.
 	pub fn set(&self, ptr: u32, value: &[u8]) -> Result<(), Error> {
 		self.inner.set(ptr, value)
 	}