From 1eba5b79081f8f61f41698c6676477fb28394e11 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Tue, 24 Jan 2023 13:52:01 +0100 Subject: [PATCH] BlockId removal: refactor: CallExecutor trait (#13173) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * BlockId removal: refactor: CallExecutor trait It changes the arguments of CallExecutor methods: - `call`, 'contextual_call', 'runtime_version', 'prove_execution' from: `BlockId` to: `Block::Hash` This PR is part of BlockId::Number refactoring analysis (paritytech/substrate#11292) * Apply suggestions from code review Co-authored-by: Bastian Köcher * ".git/.scripts/commands/fmt/fmt.sh" Co-authored-by: Bastian Köcher Co-authored-by: command-bot <> --- client/api/src/call_executor.rs | 10 ++-- client/finality-grandpa/src/lib.rs | 6 +-- .../rpc-spec-v2/src/chain_head/chain_head.rs | 2 +- client/rpc/src/state/state_full.rs | 2 +- client/service/src/client/call_executor.rs | 54 +++++++++---------- client/service/src/client/client.rs | 11 ++-- client/service/src/client/wasm_substitutes.rs | 20 +++---- primitives/version/src/lib.rs | 6 +-- 8 files changed, 55 insertions(+), 56 deletions(-) diff --git a/client/api/src/call_executor.rs b/client/api/src/call_executor.rs index 7a42385010c68..74a2b8b8557c6 100644 --- a/client/api/src/call_executor.rs +++ b/client/api/src/call_executor.rs @@ -19,7 +19,7 @@ //! A method call executor interface. use sc_executor::{RuntimeVersion, RuntimeVersionOf}; -use sp_runtime::{generic::BlockId, traits::Block as BlockT}; +use sp_runtime::traits::Block as BlockT; use sp_state_machine::{ExecutionStrategy, OverlayedChanges, StorageProof}; use std::cell::RefCell; @@ -54,7 +54,7 @@ pub trait CallExecutor: RuntimeVersionOf { /// No changes are made. fn call( &self, - id: &BlockId, + at_hash: B::Hash, method: &str, call_data: &[u8], strategy: ExecutionStrategy, @@ -67,7 +67,7 @@ pub trait CallExecutor: RuntimeVersionOf { /// of the execution context. fn contextual_call( &self, - at: &BlockId, + at_hash: B::Hash, method: &str, call_data: &[u8], changes: &RefCell, @@ -83,14 +83,14 @@ pub trait CallExecutor: RuntimeVersionOf { /// Extract RuntimeVersion of given block /// /// No changes are made. - fn runtime_version(&self, id: &BlockId) -> Result; + fn runtime_version(&self, at_hash: B::Hash) -> Result; /// Prove the execution of the given `method`. /// /// No changes are made. fn prove_execution( &self, - at: &BlockId, + at_hash: B::Hash, method: &str, call_data: &[u8], ) -> Result<(Vec, StorageProof), sp_blockchain::Error>; diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index 8758efef6037f..15d689e917d20 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -464,10 +464,10 @@ pub trait GenesisAuthoritySetProvider { fn get(&self) -> Result; } -impl GenesisAuthoritySetProvider - for Arc> +impl GenesisAuthoritySetProvider for Arc where E: CallExecutor, + Client: ExecutorProvider + HeaderBackend, { fn get(&self) -> Result { // This implementation uses the Grandpa runtime API instead of reading directly from the @@ -475,7 +475,7 @@ where // the chain, whereas the runtime API is backwards compatible. self.executor() .call( - &BlockId::Number(Zero::zero()), + self.expect_block_hash_from_id(&BlockId::Number(Zero::zero()))?, "GrandpaApi_grandpa_authorities", &[], ExecutionStrategy::NativeElseWasm, diff --git a/client/rpc-spec-v2/src/chain_head/chain_head.rs b/client/rpc-spec-v2/src/chain_head/chain_head.rs index c63d373e04f16..2a9cabaf2b310 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -744,7 +744,7 @@ where let res = client .executor() .call( - &BlockId::Hash(hash), + hash, &function, &call_parameters, client.execution_extensions().strategies().other, diff --git a/client/rpc/src/state/state_full.rs b/client/rpc/src/state/state_full.rs index d8fe39030aa82..58dfd9ea876a1 100644 --- a/client/rpc/src/state/state_full.rs +++ b/client/rpc/src/state/state_full.rs @@ -203,7 +203,7 @@ where self.client .executor() .call( - &BlockId::Hash(block), + block, &method, &call_data, self.client.execution_extensions().strategies().other, diff --git a/client/service/src/client/call_executor.rs b/client/service/src/client/call_executor.rs index 7ee19671dd688..7906d62240e0b 100644 --- a/client/service/src/client/call_executor.rs +++ b/client/service/src/client/call_executor.rs @@ -81,13 +81,13 @@ where fn check_override<'a>( &'a self, onchain_code: RuntimeCode<'a>, - id: &BlockId, + hash: ::Hash, ) -> sp_blockchain::Result<(RuntimeCode<'a>, RuntimeVersion)> where Block: BlockT, B: backend::Backend, { - let on_chain_version = self.on_chain_runtime_version(id)?; + let on_chain_version = self.on_chain_runtime_version(hash)?; 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, @@ -95,18 +95,18 @@ where &on_chain_version.spec_name, ) }) { - log::debug!(target: "wasm_overrides", "using WASM override for block {}", id); + 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, id) + .get(on_chain_version.spec_version, onchain_code.heap_pages, hash) { - log::debug!(target: "wasm_substitutes", "Using WASM substitute for block {:?}", id); + 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 {id}, using onchain code", + "Neither WASM override nor substitute available for block {hash}, using onchain code", ); (onchain_code, on_chain_version) }; @@ -115,14 +115,10 @@ where } /// Returns the on chain runtime version. - fn on_chain_runtime_version( - &self, - id: &BlockId, - ) -> sp_blockchain::Result { + fn on_chain_runtime_version(&self, hash: Block::Hash) -> 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 state = self.backend.state_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); @@ -166,21 +162,21 @@ where fn call( &self, - at: &BlockId, + at_hash: Block::Hash, method: &str, call_data: &[u8], strategy: ExecutionStrategy, ) -> sp_blockchain::Result> { let mut changes = OverlayedChanges::default(); - let at_hash = self.backend.blockchain().expect_block_hash_from_id(at)?; - let at_number = self.backend.blockchain().expect_block_number_from_id(at)?; + let at_number = + self.backend.blockchain().expect_block_number_from_id(&BlockId::Hash(at_hash))?; let state = self.backend.state_at(at_hash)?; 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)?; - let runtime_code = self.check_override(runtime_code, at)?.0; + let runtime_code = self.check_override(runtime_code, at_hash)?.0; let extensions = self.execution_extensions.extensions( at_hash, @@ -206,7 +202,7 @@ where fn contextual_call( &self, - at: &BlockId, + at_hash: Block::Hash, method: &str, call_data: &[u8], changes: &RefCell, @@ -216,8 +212,8 @@ where ) -> Result, sp_blockchain::Error> { let mut storage_transaction_cache = storage_transaction_cache.map(|c| c.borrow_mut()); - let at_hash = self.backend.blockchain().expect_block_hash_from_id(at)?; - let at_number = self.backend.blockchain().expect_block_number_from_id(at)?; + let at_number = + self.backend.blockchain().expect_block_number_from_id(&BlockId::Hash(at_hash))?; let state = self.backend.state_at(at_hash)?; let (execution_manager, extensions) = @@ -232,7 +228,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)?.0; + let runtime_code = self.check_override(runtime_code, at_hash)?.0; match recorder { Some(recorder) => { @@ -275,24 +271,23 @@ where .map_err(Into::into) } - fn runtime_version(&self, id: &BlockId) -> sp_blockchain::Result { - let at_hash = self.backend.blockchain().expect_block_hash_from_id(id)?; + fn runtime_version(&self, at_hash: Block::Hash) -> sp_blockchain::Result { let state = self.backend.state_at(at_hash)?; 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.check_override(runtime_code, id).map(|(_, v)| v) + self.check_override(runtime_code, at_hash).map(|(_, v)| v) } fn prove_execution( &self, - at: &BlockId, + at_hash: Block::Hash, method: &str, call_data: &[u8], ) -> sp_blockchain::Result<(Vec, StorageProof)> { - let at_hash = self.backend.blockchain().expect_block_hash_from_id(at)?; - let at_number = self.backend.blockchain().expect_block_number_from_id(at)?; + let at_number = + self.backend.blockchain().expect_block_number_from_id(&BlockId::Hash(at_hash))?; let state = self.backend.state_at(at_hash)?; let trie_backend = state.as_trie_backend(); @@ -300,7 +295,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)?.0; + let runtime_code = self.check_override(runtime_code, at_hash)?.0; sp_state_machine::prove_execution_on_trie_backend( trie_backend, @@ -340,7 +335,7 @@ where E: CodeExecutor + RuntimeVersionOf + Clone + 'static, Block: BlockT, { - fn runtime_version(&self, at: &BlockId) -> Result { + fn runtime_version(&self, at: Block::Hash) -> Result { CallExecutor::runtime_version(self, at).map_err(|e| e.to_string()) } } @@ -359,6 +354,7 @@ where #[cfg(test)] mod tests { use super::*; + use backend::Backend; use sc_client_api::in_mem; use sc_executor::{NativeElseWasmExecutor, WasmExecutionMethod}; use sp_core::{ @@ -432,7 +428,7 @@ mod tests { }; let check = call_executor - .check_override(onchain_code, &BlockId::Number(Default::default())) + .check_override(onchain_code, backend.blockchain().info().genesis_hash) .expect("RuntimeCode override") .0; diff --git a/client/service/src/client/client.rs b/client/service/src/client/client.rs index d32baa671f54c..6a75fad628681 100644 --- a/client/service/src/client/client.rs +++ b/client/service/src/client/client.rs @@ -493,7 +493,8 @@ where /// Get the RuntimeVersion at a given block. pub fn runtime_version_at(&self, id: &BlockId) -> sp_blockchain::Result { - CallExecutor::runtime_version(&self.executor, id) + let hash = self.backend.blockchain().expect_block_hash_from_id(id)?; + CallExecutor::runtime_version(&self.executor, hash) } /// Apply a checked and validated block to an operation. If a justification is provided @@ -1241,7 +1242,7 @@ where method: &str, call_data: &[u8], ) -> sp_blockchain::Result<(Vec, StorageProof)> { - self.executor.prove_execution(&BlockId::Hash(hash), method, call_data) + self.executor.prove_execution(hash, method, call_data) } fn read_proof_collection( @@ -1726,9 +1727,10 @@ where &self, params: CallApiAtParams, ) -> Result, sp_api::ApiError> { + let at_hash = self.expect_block_hash_from_id(params.at)?; self.executor .contextual_call( - params.at, + at_hash, params.function, ¶ms.arguments, params.overlayed_changes, @@ -1740,7 +1742,8 @@ where } fn runtime_version_at(&self, at: &BlockId) -> Result { - CallExecutor::runtime_version(&self.executor, at).map_err(Into::into) + let hash = self.backend.blockchain().expect_block_hash_from_id(at)?; + CallExecutor::runtime_version(&self.executor, hash).map_err(Into::into) } fn state_at(&self, at: &BlockId) -> Result { diff --git a/client/service/src/client/wasm_substitutes.rs b/client/service/src/client/wasm_substitutes.rs index a7a3b2cbb2b35..ed931d3dae213 100644 --- a/client/service/src/client/wasm_substitutes.rs +++ b/client/service/src/client/wasm_substitutes.rs @@ -22,10 +22,7 @@ use sc_client_api::backend; use sc_executor::RuntimeVersionOf; use sp_blockchain::{HeaderBackend, Result}; use sp_core::traits::{FetchRuntimeCode, RuntimeCode, WrappedRuntimeCode}; -use sp_runtime::{ - generic::BlockId, - traits::{Block as BlockT, NumberFor}, -}; +use sp_runtime::traits::{Block as BlockT, NumberFor}; use sp_state_machine::BasicExternalities; use sp_version::RuntimeVersion; use std::{ @@ -54,10 +51,13 @@ impl WasmSubstitute { RuntimeCode { code_fetcher: self, hash: self.hash.clone(), heap_pages } } - /// Returns `true` when the substitute matches for the given `block_id`. - fn matches(&self, block_id: &BlockId, backend: &impl backend::Backend) -> bool { - let requested_block_number = - backend.blockchain().block_number_from_id(block_id).ok().flatten(); + /// Returns `true` when the substitute matches for the given `hash`. + fn matches( + &self, + hash: ::Hash, + backend: &impl backend::Backend, + ) -> bool { + let requested_block_number = backend.blockchain().number(hash).ok().flatten(); Some(self.block_number) <= requested_block_number } @@ -147,10 +147,10 @@ where &self, spec: u32, pages: Option, - block_id: &BlockId, + hash: Block::Hash, ) -> Option<(RuntimeCode<'_>, RuntimeVersion)> { let s = self.substitutes.get(&spec)?; - s.matches(block_id, &*self.backend) + s.matches(hash, &*self.backend) .then(|| (s.runtime_code(pages), s.version.clone())) } diff --git a/primitives/version/src/lib.rs b/primitives/version/src/lib.rs index 0bd62f0bac5aa..37bb15afb29e6 100644 --- a/primitives/version/src/lib.rs +++ b/primitives/version/src/lib.rs @@ -48,7 +48,7 @@ pub use sp_runtime::{create_runtime_str, StateVersion}; pub use sp_std; #[cfg(feature = "std")] -use sp_runtime::{generic::BlockId, traits::Block as BlockT}; +use sp_runtime::traits::Block as BlockT; #[cfg(feature = "std")] pub mod embed; @@ -370,14 +370,14 @@ pub trait GetNativeVersion { #[cfg(feature = "std")] pub trait GetRuntimeVersionAt { /// Returns the version of runtime at the given block. - fn runtime_version(&self, at: &BlockId) -> Result; + fn runtime_version(&self, at: ::Hash) -> Result; } #[cfg(feature = "std")] impl, Block: BlockT> GetRuntimeVersionAt for std::sync::Arc { - fn runtime_version(&self, at: &BlockId) -> Result { + fn runtime_version(&self, at: ::Hash) -> Result { (&**self).runtime_version(at) } }