From 2bcf6efe38a694f60dd4aa6c246e7848f9acb3cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 5 Jan 2023 13:45:40 +0100 Subject: [PATCH] Return `RuntimeVersion` of overwritten/substituted wasm binary (#13066) * Adds test * Ensure we are using the runtime version of the override/substitute wasm * Update client/service/src/client/call_executor.rs Co-authored-by: Anton Co-authored-by: Anton --- client/service/src/client/call_executor.rs | 151 +++++++++++++----- client/service/src/client/wasm_override.rs | 31 ++-- client/service/src/client/wasm_substitutes.rs | 33 ++-- 3 files changed, 145 insertions(+), 70 deletions(-) diff --git a/client/service/src/client/call_executor.rs b/client/service/src/client/call_executor.rs index bf7e8b3a6555e..7ee19671dd688 100644 --- a/client/service/src/client/call_executor.rs +++ b/client/service/src/client/call_executor.rs @@ -37,7 +37,6 @@ pub struct LocalCallExecutor { wasm_override: Arc>, wasm_substitutes: WasmSubstitutes, spawn_handle: Box, - client_config: ClientConfig, execution_extensions: Arc>, } @@ -61,7 +60,7 @@ where .transpose()?; let wasm_substitutes = WasmSubstitutes::new( - client_config.wasm_runtime_substitutes.clone(), + client_config.wasm_runtime_substitutes, executor.clone(), backend.clone(), )?; @@ -71,7 +70,6 @@ where executor, wasm_override: Arc::new(wasm_override), spawn_handle, - client_config, wasm_substitutes, execution_extensions: Arc::new(execution_extensions), }) @@ -84,34 +82,55 @@ where &'a self, onchain_code: RuntimeCode<'a>, id: &BlockId, - ) -> sp_blockchain::Result> + ) -> sp_blockchain::Result<(RuntimeCode<'a>, RuntimeVersion)> where Block: BlockT, B: backend::Backend, { - let spec = CallExecutor::runtime_version(self, id)?; - let code = - if let Some(d) = - self.wasm_override.as_ref().as_ref().and_then(|o| { - o.get(&spec.spec_version, onchain_code.heap_pages, &spec.spec_name) - }) { - log::debug!(target: "wasm_overrides", "using WASM override for block {}", id); - d - } 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 { - log::debug!( - target: "wasm_overrides", - "No WASM override available for block {}, using onchain code", - id - ); - onchain_code - }; - - Ok(code) + let on_chain_version = self.on_chain_runtime_version(id)?; + let code_and_version = if let Some(d) = self.wasm_override.as_ref().as_ref().and_then(|o| { + o.get( + &on_chain_version.spec_version, + onchain_code.heap_pages, + &on_chain_version.spec_name, + ) + }) { + log::debug!(target: "wasm_overrides", "using WASM override for block {}", id); + d + } else if let Some(s) = + self.wasm_substitutes + .get(on_chain_version.spec_version, onchain_code.heap_pages, id) + { + log::debug!(target: "wasm_substitutes", "Using WASM substitute for block {:?}", id); + s + } else { + log::debug!( + target: "wasm_overrides", + "Neither WASM override nor substitute available for block {id}, using onchain code", + ); + (onchain_code, on_chain_version) + }; + + Ok(code_and_version) + } + + /// Returns the on chain runtime version. + fn on_chain_runtime_version( + &self, + id: &BlockId, + ) -> sp_blockchain::Result { + let mut overlay = OverlayedChanges::default(); + + let at_hash = self.backend.blockchain().expect_block_hash_from_id(id)?; + let state = self.backend.state_at(at_hash)?; + let mut cache = StorageTransactionCache::::default(); + let mut ext = Ext::new(&mut overlay, &mut cache, &state, None); + let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state); + let runtime_code = + state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?; + self.executor + .runtime_version(&mut ext, &runtime_code) + .map_err(|e| sp_blockchain::Error::VersionInvalid(e.to_string())) } } @@ -125,7 +144,6 @@ where executor: self.executor.clone(), wasm_override: self.wasm_override.clone(), spawn_handle: self.spawn_handle.clone(), - client_config: self.client_config.clone(), wasm_substitutes: self.wasm_substitutes.clone(), execution_extensions: self.execution_extensions.clone(), } @@ -162,7 +180,7 @@ where let runtime_code = state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?; - let runtime_code = self.check_override(runtime_code, at)?; + let runtime_code = self.check_override(runtime_code, at)?.0; let extensions = self.execution_extensions.extensions( at_hash, @@ -214,7 +232,7 @@ where let runtime_code = state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?; - let runtime_code = self.check_override(runtime_code, at)?; + let runtime_code = self.check_override(runtime_code, at)?.0; match recorder { Some(recorder) => { @@ -258,18 +276,13 @@ where } fn runtime_version(&self, id: &BlockId) -> sp_blockchain::Result { - let mut overlay = OverlayedChanges::default(); - let at_hash = self.backend.blockchain().expect_block_hash_from_id(id)?; let state = self.backend.state_at(at_hash)?; - let mut cache = StorageTransactionCache::::default(); - let mut ext = Ext::new(&mut overlay, &mut cache, &state, None); let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state); + let runtime_code = state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?; - self.executor - .runtime_version(&mut ext, &runtime_code) - .map_err(|e| sp_blockchain::Error::VersionInvalid(e.to_string())) + self.check_override(runtime_code, id).map(|(_, v)| v) } fn prove_execution( @@ -287,7 +300,7 @@ where let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(trie_backend); let runtime_code = state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?; - let runtime_code = self.check_override(runtime_code, at)?; + let runtime_code = self.check_override(runtime_code, at)?.0; sp_state_machine::prove_execution_on_trie_backend( trie_backend, @@ -352,6 +365,7 @@ mod tests { testing::TaskExecutor, traits::{FetchRuntimeCode, WrappedRuntimeCode}, }; + use std::collections::HashMap; use substrate_test_runtime_client::{runtime, GenesisInit, LocalExecutorDispatch}; #[test] @@ -395,7 +409,7 @@ mod tests { Box::new(TaskExecutor::new()), None, None, - Default::default(), + client_config, ) .expect("Creates a client"); @@ -404,7 +418,6 @@ mod tests { executor: executor.clone(), wasm_override: Arc::new(Some(overrides)), spawn_handle: Box::new(TaskExecutor::new()), - client_config, wasm_substitutes: WasmSubstitutes::new( Default::default(), executor.clone(), @@ -420,8 +433,64 @@ mod tests { let check = call_executor .check_override(onchain_code, &BlockId::Number(Default::default())) - .expect("RuntimeCode override"); + .expect("RuntimeCode override") + .0; assert_eq!(Some(vec![2, 2, 2, 2, 2, 2, 2, 2]), check.fetch_runtime_code().map(Into::into)); } + + #[test] + fn returns_runtime_version_from_substitute() { + const SUBSTITUTE_SPEC_NAME: &str = "substitute-spec-name-cool"; + + let executor = NativeElseWasmExecutor::::new( + WasmExecutionMethod::Interpreted, + Some(128), + 1, + 2, + ); + + let backend = Arc::new(in_mem::Backend::::new()); + + // Let's only override the `spec_name` for our testing purposes. + let substitute = sp_version::embed::embed_runtime_version( + &substrate_test_runtime::WASM_BINARY_BLOATY.unwrap(), + sp_version::RuntimeVersion { + spec_name: SUBSTITUTE_SPEC_NAME.into(), + ..substrate_test_runtime::VERSION + }, + ) + .unwrap(); + + let client_config = crate::client::ClientConfig { + wasm_runtime_substitutes: vec![(0, substitute)].into_iter().collect::>(), + ..Default::default() + }; + + let genesis_block_builder = crate::GenesisBlockBuilder::new( + &substrate_test_runtime_client::GenesisParameters::default().genesis_storage(), + !client_config.no_genesis, + backend.clone(), + executor.clone(), + ) + .expect("Creates genesis block builder"); + + // client is used for the convenience of creating and inserting the genesis block. + let client = + crate::client::new_with_backend::<_, _, runtime::Block, _, runtime::RuntimeApi>( + backend.clone(), + executor.clone(), + genesis_block_builder, + None, + Box::new(TaskExecutor::new()), + None, + None, + client_config, + ) + .expect("Creates a client"); + + let version = client.runtime_version_at(&BlockId::Number(0)).unwrap(); + + assert_eq!(SUBSTITUTE_SPEC_NAME, &*version.spec_name); + } } diff --git a/client/service/src/client/wasm_override.rs b/client/service/src/client/wasm_override.rs index 5fc748f3e88b9..03a5704adf8bf 100644 --- a/client/service/src/client/wasm_override.rs +++ b/client/service/src/client/wasm_override.rs @@ -62,16 +62,16 @@ struct WasmBlob { hash: Vec, /// The path where this blob was found. path: PathBuf, - /// The `spec_name` found in the runtime version of this blob. - spec_name: String, + /// The runtime version of this blob. + version: RuntimeVersion, /// When was the last time we have warned about the wasm blob having /// a wrong `spec_name`? last_warn: parking_lot::Mutex>, } impl WasmBlob { - fn new(code: Vec, hash: Vec, path: PathBuf, spec_name: String) -> Self { - Self { code, hash, path, spec_name, last_warn: Default::default() } + fn new(code: Vec, hash: Vec, path: PathBuf, version: RuntimeVersion) -> Self { + Self { code, hash, path, version, last_warn: Default::default() } } fn runtime_code(&self, heap_pages: Option) -> RuntimeCode { @@ -141,10 +141,10 @@ impl WasmOverride { spec: &u32, pages: Option, spec_name: &str, - ) -> Option> { + ) -> Option<(RuntimeCode<'a>, RuntimeVersion)> { self.overrides.get(spec).and_then(|w| { - if spec_name == w.spec_name { - Some(w.runtime_code(pages)) + if spec_name == &*w.version.spec_name { + Some((w.runtime_code(pages), w.version.clone())) } else { let mut last_warn = w.last_warn.lock(); let now = Instant::now(); @@ -155,7 +155,7 @@ impl WasmOverride { tracing::warn!( target = "wasm_overrides", on_chain_spec_name = %spec_name, - override_spec_name = %w.spec_name, + override_spec_name = %w.version, spec_version = %spec, wasm_file = %w.path.display(), "On chain and override `spec_name` do not match! Ignoring override.", @@ -197,8 +197,7 @@ impl WasmOverride { "Found wasm override.", ); - let wasm = - WasmBlob::new(code, code_hash, path.clone(), version.spec_name.to_string()); + let wasm = WasmBlob::new(code, code_hash, path.clone(), version.clone()); if let Some(other) = overrides.insert(version.spec_version, wasm) { tracing::info!( @@ -246,19 +245,19 @@ impl WasmOverride { /// Returns a WasmOverride struct filled with dummy data for testing. #[cfg(test)] pub fn dummy_overrides() -> WasmOverride { + let version = RuntimeVersion { spec_name: "test".into(), ..Default::default() }; let mut overrides = HashMap::new(); overrides.insert( 0, - WasmBlob::new(vec![0, 0, 0, 0, 0, 0, 0, 0], vec![0], PathBuf::new(), "test".into()), + WasmBlob::new(vec![0, 0, 0, 0, 0, 0, 0, 0], vec![0], PathBuf::new(), version.clone()), ); 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()), + WasmBlob::new(vec![1, 1, 1, 1, 1, 1, 1, 1], vec![1], PathBuf::new(), version.clone()), ); + overrides + .insert(2, WasmBlob::new(vec![2, 2, 2, 2, 2, 2, 2, 2], vec![2], PathBuf::new(), version)); + WasmOverride { overrides } } diff --git a/client/service/src/client/wasm_substitutes.rs b/client/service/src/client/wasm_substitutes.rs index f826fa3613c84..a7a3b2cbb2b35 100644 --- a/client/service/src/client/wasm_substitutes.rs +++ b/client/service/src/client/wasm_substitutes.rs @@ -21,7 +21,7 @@ use sc_client_api::backend; use sc_executor::RuntimeVersionOf; use sp_blockchain::{HeaderBackend, Result}; -use sp_core::traits::{FetchRuntimeCode, RuntimeCode}; +use sp_core::traits::{FetchRuntimeCode, RuntimeCode, WrappedRuntimeCode}; use sp_runtime::{ generic::BlockId, traits::{Block as BlockT, NumberFor}, @@ -41,12 +41,13 @@ struct WasmSubstitute { hash: Vec, /// The block number on which we should start using the substitute. block_number: NumberFor, + version: RuntimeVersion, } impl WasmSubstitute { - fn new(code: Vec, block_number: NumberFor) -> Self { + fn new(code: Vec, block_number: NumberFor, version: RuntimeVersion) -> Self { let hash = make_hash(&code); - Self { code, hash, block_number } + Self { code, hash, block_number, version } } fn runtime_code(&self, heap_pages: Option) -> RuntimeCode { @@ -122,9 +123,17 @@ where let substitutes = substitutes .into_iter() .map(|(block_number, code)| { - let substitute = WasmSubstitute::new(code, block_number); - let version = Self::runtime_version(&executor, &substitute)?; - Ok((version.spec_version, substitute)) + let runtime_code = RuntimeCode { + code_fetcher: &WrappedRuntimeCode((&code).into()), + heap_pages: None, + hash: Vec::new(), + }; + let version = Self::runtime_version(&executor, &runtime_code)?; + let spec_version = version.spec_version; + + let substitute = WasmSubstitute::new(code, block_number, version); + + Ok((spec_version, substitute)) }) .collect::>>()?; @@ -139,18 +148,16 @@ where spec: u32, pages: Option, block_id: &BlockId, - ) -> Option> { + ) -> Option<(RuntimeCode<'_>, RuntimeVersion)> { let s = self.substitutes.get(&spec)?; - s.matches(block_id, &*self.backend).then(|| s.runtime_code(pages)) + s.matches(block_id, &*self.backend) + .then(|| (s.runtime_code(pages), s.version.clone())) } - fn runtime_version( - executor: &Executor, - code: &WasmSubstitute, - ) -> Result { + fn runtime_version(executor: &Executor, code: &RuntimeCode) -> Result { let mut ext = BasicExternalities::default(); executor - .runtime_version(&mut ext, &code.runtime_code(None)) + .runtime_version(&mut ext, code) .map_err(|e| WasmSubstituteError::VersionInvalid(e.to_string()).into()) } }