executor docs refresh

client/executor/README.md

@@ -1,13 +1,43 @@
-A crate that provides means of executing/dispatching calls into the runtime.
+sp_executor provides means of executing/dispatching calls into the runtime.
+
+The executor is responsible for executing the runtime's state transition function (which could be native or an on-chain wasm blob).
+For wasm, while executing the state transition function the executor facilitates the [thunking](https://en.wikipedia.org/wiki/Thunk) 
+of calls between wasm pallets and native pallets.
+
 
 There are a few responsibilities of this crate at the moment:
 
-- It provides an implementation of a common entrypoint for calling into the runtime, both
-wasm and compiled.
-- It defines the environment for the wasm execution, namely the host functions that are to be
-provided into the wasm runtime module.
-- It also provides the required infrastructure for executing the current wasm runtime (specified
-by the current value of `:code` in the provided externalities), i.e. interfacing with
-wasm engine used, instance cache.
+  - It provides a common entrypoint for calling into the runtime whether the
+    runtime is wasm, native or a mix of wasm and native pallets.
+
+  - It defines the guest environment for the wasm execution ( `EnvironmentDefinition` ), namely the host functions that are to be
+    exposed to the wasm runtime module.
+
+  - It also provides the required infrastructure for executing the current wasm runtime (specified
+    by the current value of `:code` in the provided externalities), i.e. interfacing with
+    wasm engine used, instance cache.
+
+## Execution Strategies
+Several different implementations are available for executing runtimes:
+
+  * [`sc_executor_wasmtime`] - Wasmtime compiles wasm to native machine code before executing the code so is faster,
+    but only certain platforms are supported. (This is enabled by compiling with the `wasmtime` feature)
+
+  * [`sc_executor_wasmi`] - Wasmi is a wasm interpreter and thus slower but it is cross-platform.
+    It supports sandboxing execution of untrusted code.
+
+  * [`native_executor_instance`] - Fastest, but not on-chain so harder to automatically upgrade.
+
+A combination of the above strategies can be used - this is controlled by the `ExecutionStrategy` enum.
+
+## Other
+
+  * [`sc_executor_common`] details sandboxing and how to do wasm instrumentation.
+
+## References
+
+See [`sp_runtime_interface`] for an example of how to export a host function to the runtime.
+
+See also the substrate docs on [executor](https://substrate.dev/docs/en/knowledgebase/advanced/executor)
 
-License: GPL-3.0-or-later WITH Classpath-exception-2.0
+License: GPL-3.0-or-later WITH Classpath-exception-2.0

client/executor/common/README.md

@@ -1,3 +1,11 @@
 A set of common definitions that are needed for defining execution engines.
 
-License: GPL-3.0-or-later WITH Classpath-exception-2.0
+Notably:
+
+  * [`runtime_blob`] module allows for inspection and instrumentation of wasm code.
+
+  * [`sandbox::SandboxInstance`] struct allows safe execution of untrusted wasm.
+
+  * [`wasm_runtime::WasmInstance`] trait for defining the interface of a deserialised wasm module.
+
+  License: GPL-3.0-or-later WITH Classpath-exception-2.0

client/executor/common/src/lib.rs

@@ -16,8 +16,7 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
-//! A set of common definitions that are needed for defining execution engines.
-
+#![doc = include_str!("../README.md")]
 #![warn(missing_docs)]
 #![deny(unused_crate_dependencies)]
 

client/executor/common/src/runtime_blob/data_segments_snapshot.rs

@@ -21,7 +21,7 @@ use crate::error::{self, Error};
 use pwasm_utils::parity_wasm::elements::Instruction;
 use std::mem;
 
-/// This is a snapshot of data segments specialzied for a particular instantiation.
+/// This is a snapshot of data segments specialized for a particular instantiation.
 ///
 /// Note that this assumes that no mutable globals are used.
 #[derive(Clone)]

client/executor/common/src/runtime_blob/globals_snapshot.rs

@@ -31,12 +31,15 @@ pub trait InstanceGlobals {
 	/// A handle to a global which can be used to get or set a global variable. This is supposed to
 	/// be a lightweight handle, like an index or an Rc-like smart-pointer, which is cheap to clone.
 	type Global: Clone;
+
 	/// Get a handle to a global by it's export name.
 	///
 	/// The requested export is must exist in the exported list, and it should be a mutable global.
 	fn get_global(&self, export_name: &str) -> Self::Global;
+
 	/// Get the current value of the global.
 	fn get_global_value(&self, global: &Self::Global) -> sp_wasm_interface::Value;
+
 	/// Update the current value of the global.
 	///
 	/// The global behind the handle is guaranteed to be mutable and the value to be the same type
@@ -49,7 +52,7 @@ pub trait InstanceGlobals {
 /// This is set of globals required to create a [`GlobalsSnapshot`] and that are collected from
 /// a runtime blob that was instrumented by
 /// [`RuntimeBlob::expose_mutable_globals`](super::RuntimeBlob::expose_mutable_globals`).
-
+///
 /// If the code wasn't instrumented then it would be empty and snapshot would do nothing.
 pub struct ExposedMutableGlobalsSet(Vec<String>);
 

client/executor/common/src/sandbox.rs

@@ -71,6 +71,7 @@ impl GuestToSupervisorFunctionMapping {
 	}
 }
 
+/// External functions and memories that a particular wasm module depends upon.
 struct Imports {
 	func_map: HashMap<(Vec<u8>, Vec<u8>), GuestFuncIndex>,
 	memories_map: HashMap<(Vec<u8>, Vec<u8>), MemoryRef>,
@@ -131,7 +132,7 @@ impl ImportResolver for Imports {
 
 /// This trait encapsulates sandboxing capabilities.
 ///
-/// Note that this functions are only called in the `supervisor` context.
+/// Note that these functions are only called in the `supervisor` context.
 pub trait SandboxCapabilities: FunctionContext {
 	/// Represents a function reference into the supervisor environment.
 	type SupervisorFuncRef;
@@ -159,7 +160,7 @@ pub trait SandboxCapabilities: FunctionContext {
 	) -> Result<i64>;
 }
 
-/// Implementation of [`Externals`] that allows execution of guest module with
+/// Implementation of [`Externals`] that allows execution of a guest (wasm) module with
 /// [externals][`Externals`] that might refer functions defined by supervisor.
 ///
 /// [`Externals`]: ../wasmi/trait.Externals.html
@@ -173,6 +174,7 @@ fn trap(msg: &'static str) -> Trap {
 	TrapKind::Host(Box::new(Error::Other(msg.into()))).into()
 }
 
+/// Transforms the result of an external function (from the host) into a format understood by the runtime.
 fn deserialize_result(
 	mut serialized_result: &[u8],
 ) -> std::result::Result<Option<RuntimeValue>, Trap> {
@@ -301,7 +303,8 @@ where
 
 /// Sandboxed instance of a wasm module.
 ///
-/// It's primary purpose is to [`invoke`] exported functions on it.
+/// The supervisor (host) can [`invoke`] wasm-exported functions
+/// and retrieve global variables from the wasm instance.
 ///
 /// All imports of this instance are specified at the creation time and
 /// imports are implemented by the supervisor.
@@ -323,7 +326,7 @@ impl<FR> SandboxInstance<FR> {
 	/// Invoke an exported function by a name.
 	///
 	/// `supervisor_externals` is required to execute the implementations
-	/// of the syscalls that published to a sandboxed module instance.
+	/// of the syscalls that are published to a sandboxed module instance.
 	///
 	/// The `state` parameter can be used to provide custom data for
 	/// these syscall implementations.
@@ -358,6 +361,7 @@ pub enum InstantiationError {
 	ModuleDecoding,
 	/// Module is a well-formed webassembly binary but could not be instantiated. This could
 	/// happen because, e.g. the module imports entries not provided by the environment.
+	/// (A linking error)
 	Instantiation,
 	/// Module is well-formed, instantiated and linked, but while executing the start function
 	/// a trap was generated.
@@ -399,7 +403,7 @@ fn decode_environment_definition(
 	Ok((Imports { func_map, memories_map }, guest_to_supervisor_mapping))
 }
 
-/// An environment in which the guest module is instantiated.
+/// An environment in which the guest (wasm) module is instantiated.
 pub struct GuestEnvironment {
 	imports: Imports,
 	guest_to_supervisor_mapping: GuestToSupervisorFunctionMapping,

client/executor/common/src/wasm_runtime.rs

@@ -65,7 +65,7 @@ impl<'a> From<&'a str> for InvokeMethod<'a> {
 ///
 /// This can be implemented by an execution engine.
 pub trait WasmModule: Sync + Send {
-	/// Create a new instance.
+	/// Create a new instance (or reuses an existing instance if possible).
 	fn new_instance(&self) -> Result<Box<dyn WasmInstance>, Error>;
 }
 

client/executor/src/lib.rs

@@ -16,18 +16,7 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
-//! A crate that provides means of executing/dispatching calls into the runtime.
-//!
-//! There are a few responsibilities of this crate at the moment:
-//!
-//! - It provides an implementation of a common entrypoint for calling into the runtime, both
-//! wasm and compiled.
-//! - It defines the environment for the wasm execution, namely the host functions that are to be
-//! provided into the wasm runtime module.
-//! - It also provides the required infrastructure for executing the current wasm runtime (specified
-//! by the current value of `:code` in the provided externalities), i.e. interfacing with
-//! wasm engine used, instance cache.
-
+#![doc = include_str!("../README.md")]
 #![warn(missing_docs)]
 #![recursion_limit = "128"]
 

client/executor/src/native_executor.rs

@@ -194,7 +194,7 @@ impl WasmExecutor {
 
 	/// Perform a call into the given runtime.
 	///
-	/// The runtime is passed as a [`RuntimeBlob`]. The runtime will be isntantiated with the
+	/// The runtime is passed as a [`RuntimeBlob`]. The runtime will be instantiated with the
 	/// parameters this `WasmExecutor` was initialized with.
 	///
 	/// In case of problems with during creation of the runtime or instantation, a `Err` is returned.
@@ -394,11 +394,7 @@ impl RuntimeSpawn for RuntimeInstanceSpawn {
 				};
 
 				let result = with_externalities_safe(&mut async_ext, move || {
-					// FIXME: Should be refactored to shared "instance factory".
-					// Instantiating wasm here every time is suboptimal at the moment, shared
-					// pool of instances should be used.
-					//
-					// https://github.com/paritytech/substrate/issues/7354
+					// The new_instance() call may return a reused instance.
 					let instance =
 						module.new_instance().expect("Failed to create new instance from module");
 

client/executor/wasmtime/src/runtime.rs

@@ -279,7 +279,7 @@ fn common_config(semantics: &Semantics) -> std::result::Result<wasmtime::Config,
 /// usage in bytes.
 ///
 /// The actual number of bytes consumed by a function is not trivial to compute  without going through
-/// full compilation. Therefore, it's expected that `native_stack_max` is grealy overestimated and
+/// full compilation. Therefore, it's expected that `native_stack_max` is greatly overestimated and
 /// thus never reached in practice. The stack overflow check introduced by the instrumentation and
 /// that relies on the logical item count should be reached first.
 ///
@@ -297,7 +297,7 @@ pub struct DeterministicStackLimit {
 	/// It's not specified how much bytes will be consumed by a stack frame for a given wasm function
 	/// after translation into machine code. It is also not quite trivial.
 	///
-	/// Therefore, this number should be choosen conservatively. It must be so large so that it can
+	/// Therefore, this number should be chosen conservatively. It must be so large so that it can
 	/// fit the [`logical_max`](Self::logical_max) logical values on the stack, according to the current
 	/// instrumentation algorithm.
 	///
@@ -307,7 +307,7 @@ pub struct DeterministicStackLimit {
 
 pub struct Semantics {
 	/// Enabling this will lead to some optimization shenanigans that make calling [`WasmInstance`]
-	/// extermely fast.
+	/// extremely fast.
 	///
 	/// Primarily this is achieved by not recreating the instance for each call and performing a
 	/// bare minimum clean up: reapplying the data segments and restoring the values for global
@@ -322,7 +322,7 @@ pub struct Semantics {
 	// I.e. if [`CodeSupplyMode::Verbatim`] is used.
 	pub fast_instance_reuse: bool,
 
-	/// Specifiying `Some` will enable deterministic stack height. That is, all executor invocations
+	/// Specifying `Some` will enable deterministic stack height. That is, all executor invocations
 	/// will reach stack overflow at the exactly same point across different wasmtime versions and
 	/// architectures.
 	///
@@ -354,8 +354,8 @@ pub struct Config {
 	pub heap_pages: u32,
 
 	/// The WebAssembly standard requires all imports of an instantiated module to be resolved,
-	/// othewise, the instantiation fails. If this option is set to `true`, then this behavior is
-	/// overriden and imports that are requested by the module and not provided by the host functions
+	/// otherwise, the instantiation fails. If this option is set to `true`, then this behavior is
+	/// overridden and imports that are requested by the module and not provided by the host functions
 	/// will be resolved using stubs. These stubs will trap upon a call.
 	pub allow_missing_func_imports: bool,
 
@@ -462,7 +462,7 @@ unsafe fn do_create_runtime(
 			}
 		},
 		CodeSupplyMode::Artifact { compiled_artifact } => {
-			// SAFETY: The unsafity of `deserialize` is covered by this function. The
+			// SAFETY: The unsafety of `deserialize` is covered by this function. The
 			//         responsibilities to maintain the invariants are passed to the caller.
 			let module = wasmtime::Module::deserialize(&engine, compiled_artifact)
 				.map_err(|e| WasmError::Other(format!("cannot deserialize module: {}", e)))?;

client/executor/wasmtime/src/tests.rs

@@ -113,7 +113,7 @@ fn test_nan_canonicalization() {
 
 	/// A NaN with canonical payload bits.
 	const CANONICAL_NAN_BITS: u32 = 0x7fc00000;
-	/// A NaN value with an abitrary payload.
+	/// A NaN value with an arbitrary payload.
 	const ARBITRARY_NAN_BITS: u32 = 0x7f812345;
 
 	// This test works like this: we essentially do
@@ -131,7 +131,7 @@ fn test_nan_canonicalization() {
 	// However, with the `canonicalize_nans` option turned on above, we expect that the output will
 	// be a canonical NaN.
 	//
-	// We exterpolate the results of this tests so that we assume that all intermediate computations
+	// We extrapolate the results of this tests so that we assume that all intermediate computations
 	// that involve floats are sanitized and cannot produce a non-deterministic NaN.
 
 	let params = (u32::to_le_bytes(ARBITRARY_NAN_BITS), u32::to_le_bytes(1)).encode();