From 029a6562152fd83d4a4d26ec8e177b443c593872 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <git@kchr.de> Date: Tue, 18 Jun 2024 13:38:04 +0200 Subject: [PATCH] Unify `code_at` logic between `CallExecutor` & `Client` (#4618) This unifies the logic between `CallExecutor` and `Client` when it comes to fetching the `code` for a given block. The actual `code` depends on potential overrides/substitutes. Besides that it changes the logic in the lookahead collator on which `ValidationCodeHash` it sends to the validator alongside the `POV`. We are now sending the code hash as found on the relay chain. This is done as the local node could run with an override which is compatible to the validation code on the relay chain, but has a different hash. --- .../consensus/aura/src/collators/lookahead.rs | 12 +- .../consensus/aura/src/collators/mod.rs | 2 +- prdoc/pr_4618.prdoc | 20 + .../service/src/client/call_executor.rs | 222 +---------- substrate/client/service/src/client/client.rs | 23 +- .../service/src/client/code_provider.rs | 348 ++++++++++++++++++ substrate/client/service/src/client/mod.rs | 8 +- .../service/src/client/wasm_substitutes.rs | 6 +- substrate/test-utils/client/src/client_ext.rs | 2 +- 9 files changed, 407 insertions(+), 236 deletions(-) create mode 100644 prdoc/pr_4618.prdoc create mode 100644 substrate/client/service/src/client/code_provider.rs diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index 09416233ea9..b6f7b07f55d 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -363,13 +363,11 @@ where Ok(x) => x, }; - let validation_code_hash = match params.code_hash_provider.code_hash_at(parent_hash) - { - None => { - tracing::error!(target: crate::LOG_TARGET, ?parent_hash, "Could not fetch validation code hash"); - break - }, - Some(v) => v, + let Some(validation_code_hash) = + params.code_hash_provider.code_hash_at(parent_hash) + else { + tracing::error!(target: crate::LOG_TARGET, ?parent_hash, "Could not fetch validation code hash"); + break }; super::check_validation_code_or_log( diff --git a/cumulus/client/consensus/aura/src/collators/mod.rs b/cumulus/client/consensus/aura/src/collators/mod.rs index 6e0067d0ced..0abc034c1ed 100644 --- a/cumulus/client/consensus/aura/src/collators/mod.rs +++ b/cumulus/client/consensus/aura/src/collators/mod.rs @@ -64,7 +64,7 @@ async fn check_validation_code_or_log( ?relay_parent, ?local_validation_code_hash, relay_validation_code_hash = ?state, - "Parachain code doesn't match validation code stored in the relay chain state", + "Parachain code doesn't match validation code stored in the relay chain state.", ); }, None => { diff --git a/prdoc/pr_4618.prdoc b/prdoc/pr_4618.prdoc new file mode 100644 index 00000000000..3dd0fce81ee --- /dev/null +++ b/prdoc/pr_4618.prdoc @@ -0,0 +1,20 @@ +title: Unify logic for fetching the `:code` of a block + +doc: + - audience: Node Operator + description: | + Fixes an issue on parachains when running with a custom `substitute` of the on chain wasm code + and having replaced the wasm code on the relay chain. The relay chain was rejecting blocks + build this way, because the collator was reporting the actual on chain wasm code hash + to the relay chain. However, the relay chain was expecting the code hash of the wasm code substitute + that was also registered on the relay chain. + - audience: Node Dev + description: | + `Client::code_at` will now use the same `substitute` to determine the code for a given block as it is + done when executing any runtime call. + +crates: + - name: cumulus-client-consensus-aura + bump: minor + - name: sc-service + bump: minor diff --git a/substrate/client/service/src/client/call_executor.rs b/substrate/client/service/src/client/call_executor.rs index 9da4d219257..1341aa0e720 100644 --- a/substrate/client/service/src/client/call_executor.rs +++ b/substrate/client/service/src/client/call_executor.rs @@ -16,19 +16,19 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see <https://www.gnu.org/licenses/>. -use super::{client::ClientConfig, wasm_override::WasmOverride, wasm_substitutes::WasmSubstitutes}; +use super::{code_provider::CodeProvider, ClientConfig}; use sc_client_api::{ backend, call_executor::CallExecutor, execution_extensions::ExecutionExtensions, HeaderBackend, }; use sc_executor::{RuntimeVersion, RuntimeVersionOf}; use sp_api::ProofRecorder; -use sp_core::traits::{CallContext, CodeExecutor, RuntimeCode}; +use sp_core::traits::{CallContext, CodeExecutor}; use sp_externalities::Extensions; use sp_runtime::{ generic::BlockId, traits::{Block as BlockT, HashingFor}, }; -use sp_state_machine::{backend::AsTrieBackend, Ext, OverlayedChanges, StateMachine, StorageProof}; +use sp_state_machine::{backend::AsTrieBackend, OverlayedChanges, StateMachine, StorageProof}; use std::{cell::RefCell, sync::Arc}; /// Call executor that executes methods locally, querying all required @@ -36,8 +36,7 @@ use std::{cell::RefCell, sync::Arc}; pub struct LocalCallExecutor<Block: BlockT, B, E> { backend: Arc<B>, executor: E, - wasm_override: Arc<Option<WasmOverride>>, - wasm_substitutes: WasmSubstitutes<Block, E, B>, + code_provider: CodeProvider<Block, B, E>, execution_extensions: Arc<ExecutionExtensions<Block>>, } @@ -53,81 +52,15 @@ where client_config: ClientConfig<Block>, execution_extensions: ExecutionExtensions<Block>, ) -> sp_blockchain::Result<Self> { - let wasm_override = client_config - .wasm_runtime_overrides - .as_ref() - .map(|p| WasmOverride::new(p.clone(), &executor)) - .transpose()?; - - let wasm_substitutes = WasmSubstitutes::new( - client_config.wasm_runtime_substitutes, - executor.clone(), - backend.clone(), - )?; + let code_provider = CodeProvider::new(&client_config, executor.clone(), backend.clone())?; Ok(LocalCallExecutor { backend, executor, - wasm_override: Arc::new(wasm_override), - wasm_substitutes, + code_provider, execution_extensions: Arc::new(execution_extensions), }) } - - /// Check if local runtime code overrides are enabled and one is available - /// for the given `BlockId`. If yes, return it; otherwise return the same - /// `RuntimeCode` instance that was passed. - fn check_override<'a>( - &'a self, - onchain_code: RuntimeCode<'a>, - state: &B::State, - hash: Block::Hash, - ) -> sp_blockchain::Result<(RuntimeCode<'a>, RuntimeVersion)> - where - Block: BlockT, - B: backend::Backend<Block>, - { - let on_chain_version = self.on_chain_runtime_version(&onchain_code, state)?; - 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 {}", hash); - d - } else if let Some(s) = - self.wasm_substitutes - .get(on_chain_version.spec_version, onchain_code.heap_pages, hash) - { - log::debug!(target: "wasm_substitutes", "Using WASM substitute for block {:?}", hash); - s - } else { - log::debug!( - target: "wasm_overrides", - "Neither WASM override nor substitute available for block {hash}, using onchain code", - ); - (onchain_code, on_chain_version) - }; - - Ok(code_and_version) - } - - /// Returns the on chain runtime version. - fn on_chain_runtime_version( - &self, - code: &RuntimeCode, - state: &B::State, - ) -> sp_blockchain::Result<RuntimeVersion> { - let mut overlay = OverlayedChanges::default(); - - let mut ext = Ext::new(&mut overlay, state, None); - - self.executor - .runtime_version(&mut ext, code) - .map_err(|e| sp_blockchain::Error::VersionInvalid(e.to_string())) - } } impl<Block: BlockT, B, E> Clone for LocalCallExecutor<Block, B, E> @@ -138,8 +71,7 @@ where LocalCallExecutor { backend: self.backend.clone(), executor: self.executor.clone(), - wasm_override: self.wasm_override.clone(), - wasm_substitutes: self.wasm_substitutes.clone(), + code_provider: self.code_provider.clone(), execution_extensions: self.execution_extensions.clone(), } } @@ -175,7 +107,7 @@ where let runtime_code = state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?; - let runtime_code = self.check_override(runtime_code, &state, at_hash)?.0; + let runtime_code = self.code_provider.maybe_override_code(runtime_code, &state, at_hash)?.0; let mut extensions = self.execution_extensions.extensions(at_hash, at_number); @@ -215,7 +147,7 @@ where let runtime_code = state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?; - let runtime_code = self.check_override(runtime_code, &state, at_hash)?.0; + let runtime_code = self.code_provider.maybe_override_code(runtime_code, &state, at_hash)?.0; let mut extensions = extensions.borrow_mut(); match recorder { @@ -263,7 +195,9 @@ where let runtime_code = state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?; - self.check_override(runtime_code, &state, at_hash).map(|(_, v)| v) + self.code_provider + .maybe_override_code(runtime_code, &state, at_hash) + .map(|(_, v)| v) } fn prove_execution( @@ -281,7 +215,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, &state, at_hash)?.0; + let runtime_code = self.code_provider.maybe_override_code(runtime_code, &state, at_hash)?.0; sp_state_machine::prove_execution_on_trie_backend( trie_backend, @@ -331,133 +265,3 @@ where self.executor.native_version() } } - -#[cfg(test)] -mod tests { - use super::*; - use backend::Backend; - use sc_client_api::in_mem; - use sc_executor::WasmExecutor; - use sp_core::{ - testing::TaskExecutor, - traits::{FetchRuntimeCode, WrappedRuntimeCode}, - }; - use std::collections::HashMap; - use substrate_test_runtime_client::{runtime, GenesisInit}; - - #[test] - fn should_get_override_if_exists() { - let executor = WasmExecutor::default(); - - let overrides = crate::client::wasm_override::dummy_overrides(); - let onchain_code = WrappedRuntimeCode(substrate_test_runtime::wasm_binary_unwrap().into()); - let onchain_code = RuntimeCode { - code_fetcher: &onchain_code, - heap_pages: Some(128), - hash: vec![0, 0, 0, 0], - }; - - let backend = Arc::new(in_mem::Backend::<runtime::Block>::new()); - - // wasm_runtime_overrides is `None` here because we construct the - // LocalCallExecutor directly later on - let client_config = ClientConfig::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, - Box::new(TaskExecutor::new()), - None, - None, - client_config, - ) - .expect("Creates a client"); - - let call_executor = LocalCallExecutor { - backend: backend.clone(), - executor: executor.clone(), - wasm_override: Arc::new(Some(overrides)), - wasm_substitutes: WasmSubstitutes::new( - Default::default(), - executor.clone(), - backend.clone(), - ) - .unwrap(), - execution_extensions: Arc::new(ExecutionExtensions::new( - None, - Arc::new(executor.clone()), - )), - }; - - let check = call_executor - .check_override( - onchain_code, - &backend.state_at(backend.blockchain().info().genesis_hash).unwrap(), - backend.blockchain().info().genesis_hash, - ) - .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 = WasmExecutor::default(); - - let backend = Arc::new(in_mem::Backend::<runtime::Block>::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::<HashMap<_, _>>(), - ..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, - Box::new(TaskExecutor::new()), - None, - None, - client_config, - ) - .expect("Creates a client"); - - let version = client.runtime_version_at(client.chain_info().genesis_hash).unwrap(); - - assert_eq!(SUBSTITUTE_SPEC_NAME, &*version.spec_name); - } -} diff --git a/substrate/client/service/src/client/client.rs b/substrate/client/service/src/client/client.rs index 3c25c233775..2fbcc3ba4f7 100644 --- a/substrate/client/service/src/client/client.rs +++ b/substrate/client/service/src/client/client.rs @@ -18,7 +18,10 @@ //! Substrate Client -use super::block_rules::{BlockRules, LookupResult as BlockLookupResult}; +use super::{ + block_rules::{BlockRules, LookupResult as BlockLookupResult}, + CodeProvider, +}; use crate::client::notification_pinning::NotificationPinningWorker; use log::{debug, info, trace, warn}; use parking_lot::{Mutex, RwLock}; @@ -57,10 +60,7 @@ use sp_consensus::{BlockOrigin, BlockStatus, Error as ConsensusError}; use sc_utils::mpsc::{tracing_unbounded, TracingUnboundedSender}; use sp_core::{ - storage::{ - well_known_keys, ChildInfo, ChildType, PrefixedStorageKey, StorageChild, StorageData, - StorageKey, - }, + storage::{ChildInfo, ChildType, PrefixedStorageKey, StorageChild, StorageData, StorageKey}, traits::{CallContext, SpawnNamed}, }; use sp_runtime::{ @@ -115,6 +115,7 @@ where config: ClientConfig<Block>, telemetry: Option<TelemetryHandle>, unpin_worker_sender: TracingUnboundedSender<UnpinWorkerMessage<Block>>, + code_provider: CodeProvider<Block, B, E>, _phantom: PhantomData<RA>, } @@ -410,6 +411,7 @@ where Block, BlockImportOperation = <B as backend::Backend<Block>>::BlockImportOperation, >, + E: Clone, B: 'static, { let info = backend.blockchain().info(); @@ -438,6 +440,7 @@ where ); let unpin_worker = NotificationPinningWorker::new(rx, backend.clone()); spawn_handle.spawn("notification-pinning-worker", None, Box::pin(unpin_worker.run())); + let code_provider = CodeProvider::new(&config, executor.clone(), backend.clone())?; Ok(Client { backend, @@ -453,6 +456,7 @@ where config, telemetry, unpin_worker_sender, + code_provider, _phantom: Default::default(), }) } @@ -475,13 +479,10 @@ where } /// Get the code at a given block. + /// + /// This takes any potential substitutes into account, but ignores overrides. pub fn code_at(&self, hash: Block::Hash) -> sp_blockchain::Result<Vec<u8>> { - Ok(StorageProvider::storage(self, hash, &StorageKey(well_known_keys::CODE.to_vec()))? - .expect( - "None is returned if there's no value stored for the given key;\ - ':code' key is always defined; qed", - ) - .0) + self.code_provider.code_at_ignoring_overrides(hash) } /// Get the RuntimeVersion at a given block. diff --git a/substrate/client/service/src/client/code_provider.rs b/substrate/client/service/src/client/code_provider.rs new file mode 100644 index 00000000000..8ba7766ea65 --- /dev/null +++ b/substrate/client/service/src/client/code_provider.rs @@ -0,0 +1,348 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see <https://www.gnu.org/licenses/>. + +use super::{client::ClientConfig, wasm_override::WasmOverride, wasm_substitutes::WasmSubstitutes}; +use sc_client_api::backend; +use sc_executor::{RuntimeVersion, RuntimeVersionOf}; +use sp_core::traits::{FetchRuntimeCode, RuntimeCode}; +use sp_runtime::traits::Block as BlockT; +use sp_state_machine::{Ext, OverlayedChanges}; +use std::sync::Arc; + +/// Provider for fetching `:code` of a block. +/// +/// As a node can run with code overrides or substitutes, this will ensure that these are taken into +/// account before returning the actual `code` for a block. +pub struct CodeProvider<Block: BlockT, Backend, Executor> { + backend: Arc<Backend>, + executor: Arc<Executor>, + wasm_override: Arc<Option<WasmOverride>>, + wasm_substitutes: WasmSubstitutes<Block, Executor, Backend>, +} + +impl<Block: BlockT, Backend, Executor: Clone> Clone for CodeProvider<Block, Backend, Executor> { + fn clone(&self) -> Self { + Self { + backend: self.backend.clone(), + executor: self.executor.clone(), + wasm_override: self.wasm_override.clone(), + wasm_substitutes: self.wasm_substitutes.clone(), + } + } +} + +impl<Block, Backend, Executor> CodeProvider<Block, Backend, Executor> +where + Block: BlockT, + Backend: backend::Backend<Block>, + Executor: RuntimeVersionOf, +{ + /// Create a new instance. + pub fn new( + client_config: &ClientConfig<Block>, + executor: Executor, + backend: Arc<Backend>, + ) -> sp_blockchain::Result<Self> { + let wasm_override = client_config + .wasm_runtime_overrides + .as_ref() + .map(|p| WasmOverride::new(p.clone(), &executor)) + .transpose()?; + + let executor = Arc::new(executor); + + let wasm_substitutes = WasmSubstitutes::new( + client_config.wasm_runtime_substitutes.clone(), + executor.clone(), + backend.clone(), + )?; + + Ok(Self { backend, executor, wasm_override: Arc::new(wasm_override), wasm_substitutes }) + } + + /// Returns the `:code` for the given `block`. + /// + /// This takes into account potential overrides/substitutes. + pub fn code_at_ignoring_overrides(&self, block: Block::Hash) -> sp_blockchain::Result<Vec<u8>> { + let state = self.backend.state_at(block)?; + + 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.maybe_override_code_internal(runtime_code, &state, block, true) + .and_then(|r| { + r.0.fetch_runtime_code().map(Into::into).ok_or_else(|| { + sp_blockchain::Error::Backend("Could not find `:code` in backend.".into()) + }) + }) + } + + /// Maybe override the given `onchain_code`. + /// + /// This takes into account potential overrides/substitutes. + pub fn maybe_override_code<'a>( + &'a self, + onchain_code: RuntimeCode<'a>, + state: &Backend::State, + hash: Block::Hash, + ) -> sp_blockchain::Result<(RuntimeCode<'a>, RuntimeVersion)> { + self.maybe_override_code_internal(onchain_code, state, hash, false) + } + + /// Maybe override the given `onchain_code`. + /// + /// This takes into account potential overrides(depending on `ignore_overrides`)/substitutes. + fn maybe_override_code_internal<'a>( + &'a self, + onchain_code: RuntimeCode<'a>, + state: &Backend::State, + hash: Block::Hash, + ignore_overrides: bool, + ) -> sp_blockchain::Result<(RuntimeCode<'a>, RuntimeVersion)> { + let on_chain_version = self.on_chain_runtime_version(&onchain_code, state)?; + let code_and_version = if let Some(d) = self.wasm_override.as_ref().as_ref().and_then(|o| { + if ignore_overrides { + return None + } + + o.get( + &on_chain_version.spec_version, + onchain_code.heap_pages, + &on_chain_version.spec_name, + ) + }) { + tracing::debug!(target: "code-provider::overrides", block = ?hash, "using WASM override"); + d + } else if let Some(s) = + self.wasm_substitutes + .get(on_chain_version.spec_version, onchain_code.heap_pages, hash) + { + tracing::debug!(target: "code-provider::substitutes", block = ?hash, "Using WASM substitute"); + s + } else { + tracing::debug!( + target: "code-provider", + block = ?hash, + "Neither WASM override nor substitute available, using onchain code", + ); + (onchain_code, on_chain_version) + }; + + Ok(code_and_version) + } + + /// Returns the on chain runtime version. + fn on_chain_runtime_version( + &self, + code: &RuntimeCode, + state: &Backend::State, + ) -> sp_blockchain::Result<RuntimeVersion> { + let mut overlay = OverlayedChanges::default(); + + let mut ext = Ext::new(&mut overlay, state, None); + + self.executor + .runtime_version(&mut ext, code) + .map_err(|e| sp_blockchain::Error::VersionInvalid(e.to_string())) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use backend::Backend; + use sc_client_api::{in_mem, HeaderBackend}; + use sc_executor::WasmExecutor; + use sp_core::{ + testing::TaskExecutor, + traits::{FetchRuntimeCode, WrappedRuntimeCode}, + }; + use std::collections::HashMap; + use substrate_test_runtime_client::{runtime, GenesisInit}; + + #[test] + fn no_override_no_substitutes_work() { + let executor = WasmExecutor::default(); + + let code_fetcher = WrappedRuntimeCode(substrate_test_runtime::wasm_binary_unwrap().into()); + let onchain_code = RuntimeCode { + code_fetcher: &code_fetcher, + heap_pages: Some(128), + hash: vec![0, 0, 0, 0], + }; + + let backend = Arc::new(in_mem::Backend::<runtime::Block>::new()); + + // wasm_runtime_overrides is `None` here because we construct the + // LocalCallExecutor directly later on + let client_config = ClientConfig::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, + Box::new(TaskExecutor::new()), + None, + None, + client_config.clone(), + ) + .expect("Creates a client"); + + let executor = Arc::new(executor); + + let code_provider = CodeProvider { + backend: backend.clone(), + executor: executor.clone(), + wasm_override: Arc::new(None), + wasm_substitutes: WasmSubstitutes::new(Default::default(), executor, backend.clone()) + .unwrap(), + }; + + let check = code_provider + .maybe_override_code( + onchain_code, + &backend.state_at(backend.blockchain().info().genesis_hash).unwrap(), + backend.blockchain().info().genesis_hash, + ) + .expect("RuntimeCode override") + .0; + + assert_eq!(code_fetcher.fetch_runtime_code(), check.fetch_runtime_code()); + } + + #[test] + fn should_get_override_if_exists() { + let executor = WasmExecutor::default(); + + let overrides = crate::client::wasm_override::dummy_overrides(); + let onchain_code = WrappedRuntimeCode(substrate_test_runtime::wasm_binary_unwrap().into()); + let onchain_code = RuntimeCode { + code_fetcher: &onchain_code, + heap_pages: Some(128), + hash: vec![0, 0, 0, 0], + }; + + let backend = Arc::new(in_mem::Backend::<runtime::Block>::new()); + + // wasm_runtime_overrides is `None` here because we construct the + // LocalCallExecutor directly later on + let client_config = ClientConfig::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, + Box::new(TaskExecutor::new()), + None, + None, + client_config.clone(), + ) + .expect("Creates a client"); + + let executor = Arc::new(executor); + + let code_provider = CodeProvider { + backend: backend.clone(), + executor: executor.clone(), + wasm_override: Arc::new(Some(overrides)), + wasm_substitutes: WasmSubstitutes::new(Default::default(), executor, backend.clone()) + .unwrap(), + }; + + let check = code_provider + .maybe_override_code( + onchain_code, + &backend.state_at(backend.blockchain().info().genesis_hash).unwrap(), + backend.blockchain().info().genesis_hash, + ) + .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 = WasmExecutor::default(); + + let backend = Arc::new(in_mem::Backend::<runtime::Block>::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::<HashMap<_, _>>(), + ..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, + Box::new(TaskExecutor::new()), + None, + None, + client_config, + ) + .expect("Creates a client"); + + let version = client.runtime_version_at(client.chain_info().genesis_hash).unwrap(); + + assert_eq!(SUBSTITUTE_SPEC_NAME, &*version.spec_name); + } +} diff --git a/substrate/client/service/src/client/mod.rs b/substrate/client/service/src/client/mod.rs index 0703cc2b47d..ec77a92f162 100644 --- a/substrate/client/service/src/client/mod.rs +++ b/substrate/client/service/src/client/mod.rs @@ -47,14 +47,14 @@ mod block_rules; mod call_executor; mod client; +mod code_provider; mod notification_pinning; mod wasm_override; mod wasm_substitutes; -pub use self::{ - call_executor::LocalCallExecutor, - client::{Client, ClientConfig}, -}; +pub use call_executor::LocalCallExecutor; +pub use client::{Client, ClientConfig}; +pub(crate) use code_provider::CodeProvider; #[cfg(feature = "test-helpers")] pub use self::client::{new_in_mem, new_with_backend}; diff --git a/substrate/client/service/src/client/wasm_substitutes.rs b/substrate/client/service/src/client/wasm_substitutes.rs index 70db0ef20f5..07ca6c96062 100644 --- a/substrate/client/service/src/client/wasm_substitutes.rs +++ b/substrate/client/service/src/client/wasm_substitutes.rs @@ -94,7 +94,7 @@ impl From<WasmSubstituteError> for sp_blockchain::Error { pub struct WasmSubstitutes<Block: BlockT, Executor, Backend> { /// spec_version -> WasmSubstitute substitutes: Arc<HashMap<u32, WasmSubstitute<Block>>>, - executor: Executor, + executor: Arc<Executor>, backend: Arc<Backend>, } @@ -110,14 +110,14 @@ impl<Block: BlockT, Executor: Clone, Backend> Clone for WasmSubstitutes<Block, E impl<Executor, Backend, Block> WasmSubstitutes<Block, Executor, Backend> where - Executor: RuntimeVersionOf + Clone + 'static, + Executor: RuntimeVersionOf, Backend: backend::Backend<Block>, Block: BlockT, { /// Create a new instance. pub fn new( substitutes: HashMap<NumberFor<Block>, Vec<u8>>, - executor: Executor, + executor: Arc<Executor>, backend: Arc<Backend>, ) -> Result<Self> { let substitutes = substitutes diff --git a/substrate/test-utils/client/src/client_ext.rs b/substrate/test-utils/client/src/client_ext.rs index 73581a4f0ef..9dc4739eb79 100644 --- a/substrate/test-utils/client/src/client_ext.rs +++ b/substrate/test-utils/client/src/client_ext.rs @@ -153,7 +153,7 @@ where Self: BlockImport<Block, Error = ConsensusError>, RA: Send, B: Send + Sync, - E: Send, + E: Send + Sync, { async fn import(&mut self, origin: BlockOrigin, block: Block) -> Result<(), ConsensusError> { let (header, extrinsics) = block.deconstruct(); -- GitLab