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`

substrate/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,

substrate/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,
+		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,
-		HF::host_functions(),
+		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,

substrate/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()))
 			}
 

substrate/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