Implement runtime version checks in set_code (#4548)

* Implement runtime version checks in `set_code`

Check that the new runtime code given to `set_code` fullfills some
requirements:

- `spec_name` matches
- `spec_version` does not decreases
- `impl_version` does not decreases
- Either `spec_version` and `impl_version` increase

* Make tests almost work

* Some fixes after master merge

* Fix tests

* Add missed file

* Make depedency check happy?

* Remove leftover `sc-executor`

* AHHHHH

* Reset debug stuff

* Remove some 'static

* More 'static

* Some docs

* Update `Cargo.lock`

client/executor/common/src/error.rs

@@ -57,6 +57,10 @@ pub enum Error {
 	/// Runtime failed.
 	#[display(fmt="Runtime error")]
 	Runtime,
+	/// Runtime panicked.
+	#[display(fmt="Runtime panicked: {}", _0)]
+	#[from(ignore)]
+	RuntimePanicked(String),
 	/// Invalid memory reference.
 	#[display(fmt="Invalid memory reference")]
 	InvalidMemoryReference,

client/executor/common/src/wasm_runtime.rs

@@ -17,7 +17,6 @@
 //! Definitions for a wasm runtime.
 
 use crate::error::Error;
-use sp_core::traits::Externalities;
 use sp_wasm_interface::Function;
 
 /// A trait that defines an abstract wasm runtime.
@@ -34,6 +33,5 @@ pub trait WasmRuntime {
 	fn host_functions(&self) -> &[&'static dyn Function];
 
 	/// Call a method in the Substrate runtime by name. Returns the encoded result on success.
-	fn call(&mut self, ext: &mut dyn Externalities, method: &str, data: &[u8])
-		-> Result<Vec<u8>, Error>;
+	fn call(&mut self, method: &str, data: &[u8]) -> Result<Vec<u8>, Error>;
 }

client/executor/src/integration_tests/mod.rs

@@ -83,7 +83,7 @@ fn call_in_wasm<E: Externalities>(
 	code: &[u8],
 	heap_pages: u64,
 ) -> crate::error::Result<Vec<u8>> {
-	crate::call_in_wasm::<E, HostFunctions>(
+	crate::call_in_wasm::<HostFunctions>(
 		function,
 		call_data,
 		execution_method,

client/executor/src/lib.rs

@@ -39,7 +39,7 @@ mod wasm_runtime;
 mod integration_tests;
 
 pub use wasmi;
-pub use native_executor::{with_native_environment, NativeExecutor, NativeExecutionDispatch};
+pub use native_executor::{with_externalities_safe, NativeExecutor, NativeExecutionDispatch};
 pub use sp_version::{RuntimeVersion, NativeVersion};
 pub use codec::Codec;
 #[doc(hidden)]
@@ -57,26 +57,55 @@ pub use sc_executor_common::{error, allocator, sandbox};
 /// - `call_data`: Will be given as input parameters to `function`
 /// - `execution_method`: The execution method to use.
 /// - `ext`: The externalities that should be set while executing the wasm function.
+///          If `None` is given, no externalities will be set.
 /// - `heap_pages`: The number of heap pages to allocate.
 ///
 /// Returns the `Vec<u8>` that contains the return value of the function.
-pub fn call_in_wasm<E: Externalities, HF: sp_wasm_interface::HostFunctions>(
+pub fn call_in_wasm<HF: sp_wasm_interface::HostFunctions>(
 	function: &str,
 	call_data: &[u8],
 	execution_method: WasmExecutionMethod,
-	ext: &mut E,
+	ext: &mut dyn Externalities,
 	code: &[u8],
 	heap_pages: u64,
 	allow_missing_imports: bool,
 ) -> error::Result<Vec<u8>> {
-	let mut instance = wasm_runtime::create_wasm_runtime_with_code(
+	call_in_wasm_with_host_functions(
+		function,
+		call_data,
 		execution_method,
-		heap_pages,
+		ext,
 		code,
+		heap_pages,
 		HF::host_functions(),
 		allow_missing_imports,
+	)
+}
+
+/// Non-generic version of [`call_in_wasm`] that takes the `host_functions` as parameter.
+/// For more information please see [`call_in_wasm`].
+pub fn call_in_wasm_with_host_functions(
+	function: &str,
+	call_data: &[u8],
+	execution_method: WasmExecutionMethod,
+	ext: &mut dyn Externalities,
+	code: &[u8],
+	heap_pages: u64,
+	host_functions: Vec<&'static dyn sp_wasm_interface::Function>,
+	allow_missing_imports: bool,
+) -> error::Result<Vec<u8>> {
+	let instance = wasm_runtime::create_wasm_runtime_with_code(
+		execution_method,
+		heap_pages,
+		code,
+		host_functions,
+		allow_missing_imports,
 	)?;
-	instance.call(ext, function, call_data)
+
+	// It is safe, as we delete the instance afterwards.
+	let mut instance = std::panic::AssertUnwindSafe(instance);
+
+	with_externalities_safe(ext, move || instance.call(function, call_data)).and_then(|r| r)
 }
 
 /// Provides runtime information.
@@ -98,7 +127,7 @@ mod tests {
 	fn call_in_interpreted_wasm_works() {
 		let mut ext = TestExternalities::default();
 		let mut ext = ext.ext();
-		let res = call_in_wasm::<_, sp_io::SubstrateHostFunctions>(
+		let res = call_in_wasm::<sp_io::SubstrateHostFunctions>(
 			"test_empty_return",
 			&[],
 			WasmExecutionMethod::Interpreted,

client/executor/src/native_executor.rs

@@ -22,7 +22,7 @@ use sp_version::{NativeVersion, RuntimeVersion};
 use codec::{Decode, Encode};
 use sp_core::{NativeOrEncoded, traits::{CodeExecutor, Externalities}};
 use log::trace;
-use std::{result, cell::RefCell, panic::{UnwindSafe, AssertUnwindSafe}};
+use std::{result, cell::RefCell, panic::{UnwindSafe, AssertUnwindSafe}, sync::Arc};
 use sp_wasm_interface::{HostFunctions, Function};
 use sc_executor_common::wasm_runtime::WasmRuntime;
 
@@ -33,22 +33,29 @@ thread_local! {
 /// Default num of pages for the heap
 const DEFAULT_HEAP_PAGES: u64 = 1024;
 
-pub(crate) fn safe_call<F, U>(f: F) -> Result<U>
-	where F: UnwindSafe + FnOnce() -> U
-{
-	// Substrate uses custom panic hook that terminates process on panic. Disable
-	// termination for the native call.
-	let _guard = sp_panic_handler::AbortGuard::force_unwind();
-	std::panic::catch_unwind(f).map_err(|_| Error::Runtime)
-}
-
-/// Set up the externalities and safe calling environment to execute calls to a native runtime.
+/// Set up the externalities and safe calling environment to execute runtime calls.
 ///
 /// If the inner closure panics, it will be caught and return an error.
-pub fn with_native_environment<F, U>(ext: &mut dyn Externalities, f: F) -> Result<U>
+pub fn with_externalities_safe<F, U>(ext: &mut dyn Externalities, f: F) -> Result<U>
 	where F: UnwindSafe + FnOnce() -> U
 {
-	sp_externalities::set_and_run_with_externalities(ext, move || safe_call(f))
+	sp_externalities::set_and_run_with_externalities(
+		ext,
+		move || {
+			// Substrate uses custom panic hook that terminates process on panic. Disable
+			// termination for the native call.
+			let _guard = sp_panic_handler::AbortGuard::force_unwind();
+			std::panic::catch_unwind(f).map_err(|e| {
+				if let Some(err) = e.downcast_ref::<String>() {
+					Error::RuntimePanicked(err.clone())
+				} else if let Some(err) = e.downcast_ref::<&'static str>() {
+					Error::RuntimePanicked(err.to_string())
+				} else {
+					Error::RuntimePanicked("Unknown panic".into())
+				}
+			})
+		},
+	)
 }
 
 /// Delegate for dispatching a CodeExecutor call.
@@ -80,7 +87,7 @@ pub struct NativeExecutor<D> {
 	/// The number of 64KB pages to allocate for Wasm execution.
 	default_heap_pages: u64,
 	/// The host functions registered with this instance.
-	host_functions: Vec<&'static dyn Function>,
+	host_functions: Arc<Vec<&'static dyn Function>>,
 }
 
 impl<D: NativeExecutionDispatch> NativeExecutor<D> {
@@ -107,7 +114,7 @@ impl<D: NativeExecutionDispatch> NativeExecutor<D> {
 			fallback_method,
 			native_version: D::native_version(),
 			default_heap_pages: default_heap_pages.unwrap_or(DEFAULT_HEAP_PAGES),
-			host_functions,
+			host_functions: Arc::new(host_functions),
 		}
 	}
 
@@ -139,7 +146,7 @@ impl<D: NativeExecutionDispatch> NativeExecutor<D> {
 				ext,
 				self.fallback_method,
 				self.default_heap_pages,
-				&self.host_functions,
+				&*self.host_functions,
 			)?;
 
 			let runtime = AssertUnwindSafe(runtime);
@@ -181,7 +188,7 @@ impl<D: NativeExecutionDispatch> RuntimeInfo for NativeExecutor<D> {
 	}
 }
 
-impl<D: NativeExecutionDispatch> CodeExecutor for NativeExecutor<D> {
+impl<D: NativeExecutionDispatch + 'static> CodeExecutor for NativeExecutor<D> {
 	type Error = Error;
 
 	fn call
@@ -212,13 +219,15 @@ impl<D: NativeExecutionDispatch> CodeExecutor for NativeExecutor<D> {
 						onchain_version,
 					);
 
-					safe_call(
-						move || runtime.call(&mut **ext, method, data).map(NativeOrEncoded::Encoded)
+					with_externalities_safe(
+						&mut **ext,
+						move || runtime.call(method, data).map(NativeOrEncoded::Encoded)
 					)
 				}
 				(false, _, _) => {
-					safe_call(
-						move || runtime.call(&mut **ext, method, data).map(NativeOrEncoded::Encoded)
+					with_externalities_safe(
+						&mut **ext,
+						move || runtime.call(method, data).map(NativeOrEncoded::Encoded)
 					)
 				},
 				(true, true, Some(call)) => {
@@ -230,7 +239,7 @@ impl<D: NativeExecutionDispatch> CodeExecutor for NativeExecutor<D> {
 					);
 
 					used_native = true;
-					let res = with_native_environment(&mut **ext, move || (call)())
+					let res = with_externalities_safe(&mut **ext, move || (call)())
 						.and_then(|r| r
 							.map(NativeOrEncoded::Native)
 							.map_err(|s| Error::ApiError(s.to_string()))
@@ -255,6 +264,27 @@ impl<D: NativeExecutionDispatch> CodeExecutor for NativeExecutor<D> {
 	}
 }
 
+impl<D: NativeExecutionDispatch> sp_core::traits::CallInWasm for NativeExecutor<D> {
+	fn call_in_wasm(
+		&self,
+		wasm_blob: &[u8],
+		method: &str,
+		call_data: &[u8],
+		ext: &mut dyn Externalities,
+	) -> std::result::Result<Vec<u8>, String> {
+		crate::call_in_wasm_with_host_functions(
+			method,
+			call_data,
+			self.fallback_method,
+			ext,
+			wasm_blob,
+			self.default_heap_pages,
+			(*self.host_functions).clone(),
+			false,
+		).map_err(|e| e.to_string())
+	}
+}
+
 /// Implements a `NativeExecutionDispatch` for provided parameters.
 ///
 /// # Example
@@ -318,7 +348,7 @@ macro_rules! native_executor_instance {
 				method: &str,
 				data: &[u8]
 			) -> $crate::error::Result<Vec<u8>> {
-				$crate::with_native_environment(ext, move || $dispatcher(method, data))?
+				$crate::with_externalities_safe(ext, move || $dispatcher(method, data))?
 					.ok_or_else(|| $crate::error::Error::MethodNotFound(method.to_owned()))
 			}
 

client/executor/src/wasm_runtime.rs

@@ -223,8 +223,9 @@ fn create_versioned_wasm_runtime<E: Externalities>(
 		// The following unwind safety assertion is OK because if the method call panics, the
 		// runtime will be dropped.
 		let mut runtime = AssertUnwindSafe(runtime.as_mut());
-		crate::native_executor::safe_call(
-			move || runtime.call(&mut **ext, "Core_version", &[])
+		crate::native_executor::with_externalities_safe(
+			&mut **ext,
+			move || runtime.call("Core_version", &[])
 		).map_err(|_| WasmError::Instantiation("panic in call to get runtime version".into()))?
 	};
 	let encoded_version = version_result

client/executor/wasmi/Cargo.toml

@@ -13,4 +13,3 @@ sc-executor-common = { version = "0.8", path = "../common" }
 sp-wasm-interface = { version = "2.0.0", path = "../../../primitives/wasm-interface" }
 sp-runtime-interface = { version = "2.0.0", path = "../../../primitives/runtime-interface" }
 sp-core = { version = "2.0.0", path = "../../../primitives/core" }
-sp-externalities = { version = "0.8.0", path = "../../../primitives/externalities" }

client/executor/wasmi/src/lib.rs

@@ -27,7 +27,7 @@ use wasmi::{
 	memory_units::Pages, RuntimeValue::{I32, I64, self},
 };
 use codec::{Encode, Decode};
-use sp_core::{sandbox as sandbox_primitives, traits::Externalities};
+use sp_core::sandbox as sandbox_primitives;
 use log::{error, trace};
 use parity_wasm::elements::{deserialize_buffer, DataSegment, Instruction, Module as RawModule};
 use sp_wasm_interface::{
@@ -381,7 +381,6 @@ fn get_heap_base(module: &ModuleRef) -> Result<u32, Error> {
 
 /// Call a given method in the given wasm-module runtime.
 fn call_in_wasm_module(
-	ext: &mut dyn Externalities,
 	module_instance: &ModuleRef,
 	method: &str,
 	data: &[u8],
@@ -410,13 +409,10 @@ fn call_in_wasm_module(
 	let offset = fec.allocate_memory(data.len() as u32)?;
 	fec.write_memory(offset, data)?;
 
-	let result = sp_externalities::set_and_run_with_externalities(
-		ext,
-		|| module_instance.invoke_export(
-			method,
-			&[I32(u32::from(offset) as i32), I32(data.len() as i32)],
-			&mut fec,
-		),
+	let result = module_instance.invoke_export(
+		method,
+		&[I32(u32::from(offset) as i32), I32(data.len() as i32)],
+		&mut fec,
 	);
 
 	match result {
@@ -599,7 +595,6 @@ impl WasmRuntime for WasmiRuntime {
 
 	fn call(
 		&mut self,
-		ext: &mut dyn Externalities,
 		method: &str,
 		data: &[u8],
 	) -> Result<Vec<u8>, Error> {
@@ -612,7 +607,6 @@ impl WasmRuntime for WasmiRuntime {
 				e
 			})?;
 		call_in_wasm_module(
-			ext,
 			&self.instance,
 			method,
 			data,

client/executor/wasmtime/Cargo.toml

@@ -13,7 +13,6 @@ sc-executor-common = { version = "0.8", path = "../common" }
 sp-wasm-interface = { version = "2.0.0", path = "../../../primitives/wasm-interface" }
 sp-runtime-interface = { version = "2.0.0", path = "../../../primitives/runtime-interface" }
 sp-core = { version = "2.0.0", path = "../../../primitives/core" }
-sp-externalities = { version = "0.8.0", path = "../../../primitives/externalities" }
 
 cranelift-codegen = "0.50"
 cranelift-entity = "0.50"

client/executor/wasmtime/src/runtime.rs

@@ -24,7 +24,6 @@ use sc_executor_common::{
 	error::{Error, Result, WasmError},
 	wasm_runtime::WasmRuntime,
 };
-use sp_core::traits::Externalities;
 use sp_wasm_interface::{Pointer, WordSize, Function};
 use sp_runtime_interface::unpack_ptr_and_len;
 
@@ -70,11 +69,10 @@ impl WasmRuntime for WasmtimeRuntime {
 		&self.host_functions
 	}
 
-	fn call(&mut self, ext: &mut dyn Externalities, method: &str, data: &[u8]) -> Result<Vec<u8>> {
+	fn call(&mut self, method: &str, data: &[u8]) -> Result<Vec<u8>> {
 		call_method(
 			&mut self.context,
 			&mut self.module,
-			ext,
 			method,
 			data,
 			self.heap_pages,
@@ -146,7 +144,6 @@ fn create_compiled_unit(
 fn call_method(
 	context: &mut Context,
 	module: &mut CompiledModule,
-	ext: &mut dyn Externalities,
 	method: &str,
 	data: &[u8],
 	heap_pages: u32,
@@ -176,11 +173,9 @@ fn call_method(
 	let args = [RuntimeValue::I32(u32::from(data_ptr) as i32), RuntimeValue::I32(data_len as i32)];
 
 	// Invoke the function in the runtime.
-	let outcome = sp_externalities::set_and_run_with_externalities(ext, || {
-		context
-			.invoke(&mut instance, method, &args[..])
-			.map_err(|e| Error::Other(format!("error calling runtime: {}", e)))
-	})?;
+	let outcome = context
+		.invoke(&mut instance, method, &args[..])
+		.map_err(|e| Error::Other(format!("error calling runtime: {}", e)))?;
 	let trap_error = reset_env_state_and_take_trap(context, None)?;
 	let (output_ptr, output_len) = match outcome {
 		ActionOutcome::Returned { values } => match values.as_slice() {

client/finality-grandpa/src/environment.rs

@@ -412,7 +412,7 @@ for Environment<B, E, Block, N, RA, SC, VR>
 where
 	Block: 'static,
 	B: Backend<Block> + 'static,
-	E: CallExecutor<Block> + Send + Sync + 'static,
+	E: CallExecutor<Block> + Send + Sync,
  	N: NetworkT<Block> + 'static + Send,
 	SC: SelectChain<Block> + 'static,
 	VR: VotingRule<Block, Client<B, E, Block, RA>>,