wasm-override: Support checking spec_name (paritytech#10380)

This adds support to the wasm-override feature to compare the spec_name. If the spec_name doesn't
match, a warning will be printed and the override will be ignored.

client/service/src/client/call_executor.rs

@@ -38,7 +38,7 @@ use std::{cell::RefCell, panic::UnwindSafe, result, sync::Arc};
 pub struct LocalCallExecutor<Block: BlockT, B, E> {
 	backend: Arc<B>,
 	executor: E,
-	wasm_override: Option<WasmOverride>,
+	wasm_override: Arc<Option<WasmOverride>>,
 	wasm_substitutes: WasmSubstitutes<Block, E, B>,
 	spawn_handle: Box<dyn SpawnNamed>,
 	client_config: ClientConfig<Block>,
@@ -71,7 +71,7 @@ where
 		Ok(LocalCallExecutor {
 			backend,
 			executor,
-			wasm_override,
+			wasm_override: Arc::new(wasm_override),
 			spawn_handle,
 			client_config,
 			wasm_substitutes,
@@ -90,16 +90,19 @@ where
 		Block: BlockT,
 		B: backend::Backend<Block>,
 	{
-		let spec = self.runtime_version(id)?.spec_version;
+		let spec = self.runtime_version(id)?;
 		let code = if let Some(d) = self
 			.wasm_override
 			.as_ref()
-			.map(|o| o.get(&spec, onchain_code.heap_pages))
+			.as_ref()
+			.map(|o| o.get(&spec.spec_version, onchain_code.heap_pages, &spec.spec_name))
 			.flatten()
 		{
 			log::debug!(target: "wasm_overrides", "using WASM override for block {}", id);
 			d
-		} else if let Some(s) = self.wasm_substitutes.get(spec, onchain_code.heap_pages, id) {
+		} else if let Some(s) =
+			self.wasm_substitutes.get(spec.spec_version, onchain_code.heap_pages, id)
+		{
 			log::debug!(target: "wasm_substitutes", "Using WASM substitute for block {:?}", id);
 			s
 		} else {
@@ -395,7 +398,7 @@ mod tests {
 		let call_executor = LocalCallExecutor {
 			backend: backend.clone(),
 			executor: executor.clone(),
-			wasm_override: Some(overrides),
+			wasm_override: Arc::new(Some(overrides)),
 			spawn_handle: Box::new(TaskExecutor::new()),
 			client_config,
 			wasm_substitutes: WasmSubstitutes::new(

client/service/src/client/wasm_override.rs

@@ -35,29 +35,43 @@
 //! A custom WASM blob will override on-chain WASM if the spec version matches. If it is
 //! required to overrides multiple runtimes, multiple WASM blobs matching each of the spec versions
 //! needed must be provided in the given directory.
+
 use sc_executor::RuntimeVersionOf;
 use sp_blockchain::Result;
-use sp_core::traits::{FetchRuntimeCode, RuntimeCode};
+use sp_core::traits::{FetchRuntimeCode, RuntimeCode, WrappedRuntimeCode};
 use sp_state_machine::BasicExternalities;
 use sp_version::RuntimeVersion;
 use std::{
 	collections::{hash_map::DefaultHasher, HashMap},
 	fs,
 	hash::Hasher as _,
 	path::{Path, PathBuf},
+	time::{Duration, Instant},
 };
 
-#[derive(Clone, Debug, PartialEq)]
+/// The interval in that we will print a warning when a wasm blob `spec_name`
+/// doesn't match with the on-chain `spec_name`.
+const WARN_INTERVAL: Duration = Duration::from_secs(30);
+
 /// Auxiliary structure that holds a wasm blob and its hash.
+#[derive(Debug)]
 struct WasmBlob {
+	/// The actual wasm blob, aka the code.
 	code: Vec<u8>,
+	/// The hash of [`Self::code`].
 	hash: Vec<u8>,
+	/// The path where this blob was found.
+	path: PathBuf,
+	/// The `spec_name` found in the runtime version of this blob.
+	spec_name: String,
+	/// When was the last time we have warned about the wasm blob having
+	/// a wrong `spec_name`?
+	last_warn: parking_lot::Mutex<Option<Instant>>,
 }
 
 impl WasmBlob {
-	fn new(code: Vec<u8>) -> Self {
-		let hash = make_hash(&code);
-		Self { code, hash }
+	fn new(code: Vec<u8>, hash: Vec<u8>, path: PathBuf, spec_name: String) -> Self {
+		Self { code, hash, path, spec_name, last_warn: Default::default() }
 	}
 
 	fn runtime_code(&self, heap_pages: Option<u64>) -> RuntimeCode {
@@ -103,7 +117,7 @@ impl From<WasmOverrideError> for sp_blockchain::Error {
 
 /// Scrapes WASM from a folder and returns WASM from that folder
 /// if the runtime spec version matches.
-#[derive(Clone, Debug)]
+#[derive(Debug)]
 pub struct WasmOverride {
 	// Map of runtime spec version -> Wasm Blob
 	overrides: HashMap<u32, WasmBlob>,
@@ -122,8 +136,35 @@ impl WasmOverride {
 	/// Gets an override by it's runtime spec version.
 	///
 	/// Returns `None` if an override for a spec version does not exist.
-	pub fn get<'a, 'b: 'a>(&'b self, spec: &u32, pages: Option<u64>) -> Option<RuntimeCode<'a>> {
-		self.overrides.get(spec).map(|w| w.runtime_code(pages))
+	pub fn get<'a, 'b: 'a>(
+		&'b self,
+		spec: &u32,
+		pages: Option<u64>,
+		spec_name: &str,
+	) -> Option<RuntimeCode<'a>> {
+		self.overrides.get(spec).and_then(|w| {
+			if spec_name == w.spec_name {
+				Some(w.runtime_code(pages))
+			} else {
+				let mut last_warn = w.last_warn.lock();
+				let now = Instant::now();
+
+				if last_warn.map_or(true, |l| l + WARN_INTERVAL <= now) {
+					*last_warn = Some(now);
+
+					tracing::warn!(
+						target = "wasm_overrides",
+						on_chain_spec_name = %spec_name,
+						override_spec_name = %w.spec_name,
+						spec_version = %spec,
+						wasm_file = %w.path.display(),
+						"On chain and override `spec_name` do not match! Ignoring override.",
+					);
+				}
+
+				None
+			}
+		})
 	}
 
 	/// Scrapes a folder for WASM runtimes.
@@ -147,22 +188,29 @@ impl WasmOverride {
 			let path = entry.path();
 			match path.extension().map(|e| e.to_str()).flatten() {
 				Some("wasm") => {
-					let wasm = WasmBlob::new(fs::read(&path).map_err(handle_err)?);
-					let version = Self::runtime_version(executor, &wasm, Some(128))?;
-					log::info!(
+					let code = fs::read(&path).map_err(handle_err)?;
+					let code_hash = make_hash(&code);
+					let version = Self::runtime_version(executor, &code, &code_hash, Some(128))?;
+
+					tracing::info!(
 						target: "wasm_overrides",
-						"Found wasm override in file: `{:?}`, version: {}",
-						path.to_str(),
-						version,
+						version = %version,
+						file = %path.display(),
+						"Found wasm override.",
 					);
-					if let Some(_duplicate) = overrides.insert(version.spec_version, wasm) {
-						log::info!(
+
+					let wasm =
+						WasmBlob::new(code, code_hash, path.clone(), version.spec_name.to_string());
+
+					if let Some(other) = overrides.insert(version.spec_version, wasm) {
+						tracing::info!(
 							target: "wasm_overrides",
-							"Found duplicate spec version for runtime in file: `{:?}`, version: {}",
-							path.to_str(),
-							version,
+							first = %other.path.display(),
+							second = %path.display(),
+							%version,
+							"Found duplicate spec version for runtime.",
 						);
-						duplicates.push(format!("{}", path.display()));
+						duplicates.push(path.display().to_string());
 					}
 				},
 				_ => (),
@@ -178,15 +226,23 @@ impl WasmOverride {
 
 	fn runtime_version<E>(
 		executor: &E,
-		code: &WasmBlob,
+		code: &[u8],
+		code_hash: &[u8],
 		heap_pages: Option<u64>,
 	) -> Result<RuntimeVersion>
 	where
 		E: RuntimeVersionOf,
 	{
 		let mut ext = BasicExternalities::default();
 		executor
-			.runtime_version(&mut ext, &code.runtime_code(heap_pages))
+			.runtime_version(
+				&mut ext,
+				&RuntimeCode {
+					code_fetcher: &WrappedRuntimeCode(code.into()),
+					heap_pages,
+					hash: code_hash.into(),
+				},
+			)
 			.map_err(|e| WasmOverrideError::VersionInvalid(format!("{:?}", e)).into())
 	}
 }
@@ -195,9 +251,18 @@ impl WasmOverride {
 #[cfg(test)]
 pub fn dummy_overrides() -> WasmOverride {
 	let mut overrides = HashMap::new();
-	overrides.insert(0, WasmBlob::new(vec![0, 0, 0, 0, 0, 0, 0, 0]));
-	overrides.insert(1, WasmBlob::new(vec![1, 1, 1, 1, 1, 1, 1, 1]));
-	overrides.insert(2, WasmBlob::new(vec![2, 2, 2, 2, 2, 2, 2, 2]));
+	overrides.insert(
+		0,
+		WasmBlob::new(vec![0, 0, 0, 0, 0, 0, 0, 0], vec![0], PathBuf::new(), "test".into()),
+	);
+	overrides.insert(
+		1,
+		WasmBlob::new(vec![1, 1, 1, 1, 1, 1, 1, 1], vec![1], PathBuf::new(), "test".into()),
+	);
+	overrides.insert(
+		2,
+		WasmBlob::new(vec![2, 2, 2, 2, 2, 2, 2, 2], vec![2], PathBuf::new(), "test".into()),
+	);
 	WasmOverride { overrides }
 }
 
@@ -226,15 +291,19 @@ mod tests {
 
 	#[test]
 	fn should_get_runtime_version() {
-		let wasm = WasmBlob::new(substrate_test_runtime::wasm_binary_unwrap().to_vec());
 		let executor = NativeElseWasmExecutor::<LocalExecutorDispatch>::new(
 			WasmExecutionMethod::Interpreted,
 			Some(128),
 			1,
 		);
 
-		let version = WasmOverride::runtime_version(&executor, &wasm, Some(128))
-			.expect("should get the `RuntimeVersion` of the test-runtime wasm blob");
+		let version = WasmOverride::runtime_version(
+			&executor,
+			substrate_test_runtime::wasm_binary_unwrap(),
+			&[1],
+			Some(128),
+		)
+		.expect("should get the `RuntimeVersion` of the test-runtime wasm blob");
 		assert_eq!(version.spec_version, 2);
 	}
 

primitives/runtime/src/runtime_string.rs

@@ -92,6 +92,18 @@ impl AsRef<[u8]> for RuntimeString {
 	}
 }
 
+#[cfg(feature = "std")]
+impl std::ops::Deref for RuntimeString {
+	type Target = str;
+
+	fn deref(&self) -> &str {
+		match self {
+			Self::Borrowed(val) => &val,
+			Self::Owned(val) => &val,
+		}
+	}
+}
+
 impl Encode for RuntimeString {
 	fn encode(&self) -> Vec<u8> {
 		match self {