diff --git a/client/api/src/client.rs b/client/api/src/client.rs index 05e3163dcc7bd..0d00257fa7b06 100644 --- a/client/api/src/client.rs +++ b/client/api/src/client.rs @@ -21,7 +21,7 @@ use sp_consensus::BlockOrigin; use sp_core::storage::StorageKey; use sp_runtime::{ - generic::{BlockId, SignedBlock}, + generic::SignedBlock, traits::{Block as BlockT, NumberFor}, Justifications, }; @@ -120,14 +120,13 @@ pub trait BlockBackend { /// that are indexed by the runtime with `storage_index_transaction`. fn block_indexed_body(&self, hash: Block::Hash) -> sp_blockchain::Result>>>; - /// Get full block by id. - fn block(&self, id: &BlockId) -> sp_blockchain::Result>>; + /// Get full block by hash. + fn block(&self, hash: Block::Hash) -> sp_blockchain::Result>>; - /// Get block status. - fn block_status(&self, id: &BlockId) - -> sp_blockchain::Result; + /// Get block status by block hash. + fn block_status(&self, hash: Block::Hash) -> sp_blockchain::Result; - /// Get block justifications for the block with the given id. + /// Get block justifications for the block with the given hash. fn justifications(&self, hash: Block::Hash) -> sp_blockchain::Result>; /// Get block hash by number. diff --git a/client/cli/src/commands/export_blocks_cmd.rs b/client/cli/src/commands/export_blocks_cmd.rs index e2f83200e511c..76fedff4d3ad7 100644 --- a/client/cli/src/commands/export_blocks_cmd.rs +++ b/client/cli/src/commands/export_blocks_cmd.rs @@ -23,7 +23,7 @@ use crate::{ }; use clap::Parser; use log::info; -use sc_client_api::{BlockBackend, UsageProvider}; +use sc_client_api::{BlockBackend, HeaderBackend, UsageProvider}; use sc_service::{chain_ops::export_blocks, config::DatabaseSource}; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; use std::{fmt::Debug, fs, io, path::PathBuf, str::FromStr, sync::Arc}; @@ -73,7 +73,7 @@ impl ExportBlocksCmd { ) -> error::Result<()> where B: BlockT, - C: BlockBackend + UsageProvider + 'static, + C: HeaderBackend + BlockBackend + UsageProvider + 'static, <::Number as FromStr>::Err: Debug, { if let Some(path) = database_config.path() { diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index 76f14a721a439..312fc6f5b7947 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -86,7 +86,6 @@ use sp_consensus::{ BlockOrigin, BlockStatus, }; use sp_runtime::{ - generic::BlockId, traits::{ Block as BlockT, CheckedSub, Hash, HashFor, Header as HeaderT, NumberFor, One, SaturatedConversion, Zero, @@ -1865,8 +1864,7 @@ where self.best_queued_number = info.best_number; if self.mode == SyncMode::Full && - self.client.block_status(&BlockId::hash(info.best_hash))? != - BlockStatus::InChainWithState + self.client.block_status(info.best_hash)? != BlockStatus::InChainWithState { self.import_existing = true; // Latest state is missing, start with the last finalized state or genesis instead. @@ -1898,7 +1896,7 @@ where if self.queue_blocks.contains(hash) { return Ok(BlockStatus::Queued) } - self.client.block_status(&BlockId::Hash(*hash)) + self.client.block_status(*hash) } /// Is the block corresponding to the given hash known? @@ -2455,9 +2453,7 @@ where if queue.contains(hash) { BlockStatus::Queued } else { - client - .block_status(&BlockId::Hash(*hash)) - .unwrap_or(BlockStatus::Unknown) + client.block_status(*hash).unwrap_or(BlockStatus::Unknown) } }, ) { @@ -3202,6 +3198,7 @@ mod test { }; use sp_blockchain::HeaderBackend; use sp_consensus::block_validation::DefaultBlockAnnounceValidator; + use sp_runtime::generic::BlockId; use substrate_test_runtime_client::{ runtime::{Block, Hash, Header}, BlockBuilderExt, ClientBlockImportExt, ClientExt, DefaultTestClientBuilderExt, TestClient, 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 c0ba9e165dd04..c63d373e04f16 100644 --- a/client/rpc-spec-v2/src/chain_head/chain_head.rs +++ b/client/rpc-spec-v2/src/chain_head/chain_head.rs @@ -573,7 +573,7 @@ where return } - let event = match client.block(&BlockId::Hash(hash)) { + let event = match client.block(hash) { Ok(Some(signed_block)) => { let extrinsics = signed_block.block.extrinsics(); let result = format!("0x{:?}", HexDisplay::from(&extrinsics.encode())); diff --git a/client/rpc/src/chain/chain_full.rs b/client/rpc/src/chain/chain_full.rs index ee7870e0629e0..6ff544b0deacd 100644 --- a/client/rpc/src/chain/chain_full.rs +++ b/client/rpc/src/chain/chain_full.rs @@ -29,10 +29,7 @@ use futures::{ use jsonrpsee::SubscriptionSink; use sc_client_api::{BlockBackend, BlockchainEvents}; use sp_blockchain::HeaderBackend; -use sp_runtime::{ - generic::{BlockId, SignedBlock}, - traits::Block as BlockT, -}; +use sp_runtime::{generic::SignedBlock, traits::Block as BlockT}; /// Blockchain API backend for full nodes. Reads all the data from local database. pub struct FullChain { @@ -66,7 +63,7 @@ where } fn block(&self, hash: Option) -> Result>, Error> { - self.client.block(&BlockId::Hash(self.unwrap_or_best(hash))).map_err(client_err) + self.client.block(self.unwrap_or_best(hash)).map_err(client_err) } fn subscribe_all_heads(&self, sink: SubscriptionSink) { diff --git a/client/rpc/src/dev/mod.rs b/client/rpc/src/dev/mod.rs index ec9e61678fd71..e48a8ee4e7d5c 100644 --- a/client/rpc/src/dev/mod.rs +++ b/client/rpc/src/dev/mod.rs @@ -69,10 +69,7 @@ where self.deny_unsafe.check_if_safe()?; let block = { - let block = self - .client - .block(&BlockId::Hash(hash)) - .map_err(|e| Error::BlockQueryError(Box::new(e)))?; + let block = self.client.block(hash).map_err(|e| Error::BlockQueryError(Box::new(e)))?; if let Some(block) = block { let (mut header, body) = block.block.deconstruct(); // Remove the `Seal` to ensure we have the number of digests as expected by the diff --git a/client/service/src/chain_ops/check_block.rs b/client/service/src/chain_ops/check_block.rs index 41a6c73c5f473..c3380e5476ab6 100644 --- a/client/service/src/chain_ops/check_block.rs +++ b/client/service/src/chain_ops/check_block.rs @@ -18,34 +18,37 @@ use crate::error::Error; use codec::Encode; -use futures::{future, prelude::*}; use sc_client_api::{BlockBackend, HeaderBackend}; use sc_consensus::import_queue::ImportQueue; use sp_runtime::{generic::BlockId, traits::Block as BlockT}; use crate::chain_ops::import_blocks; -use std::{pin::Pin, sync::Arc}; +use std::sync::Arc; /// Re-validate known block. -pub fn check_block( +pub async fn check_block( client: Arc, import_queue: IQ, block_id: BlockId, -) -> Pin> + Send>> +) -> Result<(), Error> where C: BlockBackend + HeaderBackend + Send + Sync + 'static, B: BlockT + for<'de> serde::Deserialize<'de>, IQ: ImportQueue + 'static, { - match client.block(&block_id) { - Ok(Some(block)) => { + let maybe_block = client + .block_hash_from_id(&block_id)? + .map(|hash| client.block(hash)) + .transpose()? + .flatten(); + match maybe_block { + Some(block) => { let mut buf = Vec::new(); 1u64.encode_to(&mut buf); block.encode_to(&mut buf); let reader = std::io::Cursor::new(buf); - import_blocks(client, import_queue, reader, true, true) + import_blocks(client, import_queue, reader, true, true).await }, - Ok(None) => Box::pin(future::err("Unknown block".into())), - Err(e) => Box::pin(future::err(format!("Error reading block: {}", e).into())), + None => Err("Unknown block")?, } } diff --git a/client/service/src/chain_ops/export_blocks.rs b/client/service/src/chain_ops/export_blocks.rs index d442a11f2c39b..9638722ef6b21 100644 --- a/client/service/src/chain_ops/export_blocks.rs +++ b/client/service/src/chain_ops/export_blocks.rs @@ -25,7 +25,7 @@ use sp_runtime::{ traits::{Block as BlockT, NumberFor, One, SaturatedConversion, Zero}, }; -use sc_client_api::{BlockBackend, UsageProvider}; +use sc_client_api::{BlockBackend, HeaderBackend, UsageProvider}; use std::{io::Write, pin::Pin, sync::Arc, task::Poll}; /// Performs the blocks export. @@ -37,7 +37,7 @@ pub fn export_blocks( binary: bool, ) -> Pin>>> where - C: BlockBackend + UsageProvider + 'static, + C: HeaderBackend + BlockBackend + UsageProvider + 'static, B: BlockT, { let mut block = from; @@ -75,7 +75,12 @@ where wrote_header = true; } - match client.block(&BlockId::number(block))? { + match client + .block_hash_from_id(&BlockId::number(block))? + .map(|hash| client.block(hash)) + .transpose()? + .flatten() + { Some(block) => if binary { output.write_all(&block.encode())?; @@ -83,7 +88,6 @@ where serde_json::to_writer(&mut output, &block) .map_err(|e| format!("Error writing JSON: {}", e))?; }, - // Reached end of the chain. None => return Poll::Ready(Ok(())), } if (block % 10000u32.into()).is_zero() { diff --git a/client/service/src/client/client.rs b/client/service/src/client/client.rs index 78f2174b89eea..18012fc1931fe 100644 --- a/client/service/src/client/client.rs +++ b/client/service/src/client/client.rs @@ -784,7 +784,8 @@ where let parent_hash = import_block.header.parent_hash(); let at = BlockId::Hash(*parent_hash); let state_action = std::mem::replace(&mut import_block.state_action, StateAction::Skip); - let (enact_state, storage_changes) = match (self.block_status(&at)?, state_action) { + let (enact_state, storage_changes) = match (self.block_status(*parent_hash)?, state_action) + { (BlockStatus::KnownBad, _) => return Ok(PrepareStorageChangesResult::Discard(ImportResult::KnownBad)), ( @@ -1025,17 +1026,18 @@ where } /// Get block status. - pub fn block_status(&self, id: &BlockId) -> sp_blockchain::Result { + pub fn block_status(&self, hash: Block::Hash) -> sp_blockchain::Result { // this can probably be implemented more efficiently - if let BlockId::Hash(ref h) = id { - if self.importing_block.read().as_ref().map_or(false, |importing| h == importing) { - return Ok(BlockStatus::Queued) - } + if self + .importing_block + .read() + .as_ref() + .map_or(false, |importing| &hash == importing) + { + return Ok(BlockStatus::Queued) } - let hash_and_number = match *id { - BlockId::Hash(hash) => self.backend.blockchain().number(hash)?.map(|n| (hash, n)), - BlockId::Number(n) => self.backend.blockchain().hash(n)?.map(|hash| (hash, n)), - }; + + let hash_and_number = self.backend.blockchain().number(hash)?.map(|n| (hash, n)); match hash_and_number { Some((hash, number)) => if self.backend.have_state_at(hash, number) { @@ -1779,7 +1781,7 @@ where // Own status must be checked first. If the block and ancestry is pruned // this function must return `AlreadyInChain` rather than `MissingState` match self - .block_status(&BlockId::Hash(hash)) + .block_status(hash) .map_err(|e| ConsensusError::ClientImport(e.to_string()))? { BlockStatus::InChainWithState | BlockStatus::Queued => @@ -1792,7 +1794,7 @@ where } match self - .block_status(&BlockId::Hash(parent_hash)) + .block_status(parent_hash) .map_err(|e| ConsensusError::ClientImport(e.to_string()))? { BlockStatus::InChainWithState | BlockStatus::Queued => {}, @@ -1947,20 +1949,16 @@ where self.body(hash) } - fn block(&self, id: &BlockId) -> sp_blockchain::Result>> { - Ok(match self.backend.blockchain().block_hash_from_id(id)? { - Some(hash) => - match (self.header(hash)?, self.body(hash)?, self.justifications(hash)?) { - (Some(header), Some(extrinsics), justifications) => - Some(SignedBlock { block: Block::new(header, extrinsics), justifications }), - _ => None, - }, - None => None, + fn block(&self, hash: Block::Hash) -> sp_blockchain::Result>> { + Ok(match (self.header(hash)?, self.body(hash)?, self.justifications(hash)?) { + (Some(header), Some(extrinsics), justifications) => + Some(SignedBlock { block: Block::new(header, extrinsics), justifications }), + _ => None, }) } - fn block_status(&self, id: &BlockId) -> sp_blockchain::Result { - Client::block_status(self, id) + fn block_status(&self, hash: Block::Hash) -> sp_blockchain::Result { + Client::block_status(self, hash) } fn justifications(&self, hash: Block::Hash) -> sp_blockchain::Result> { @@ -2055,9 +2053,9 @@ where { fn block_status( &self, - id: &BlockId, + hash: B::Hash, ) -> Result> { - Client::block_status(self, id).map_err(|e| Box::new(e) as Box<_>) + Client::block_status(self, hash).map_err(|e| Box::new(e) as Box<_>) } } diff --git a/client/service/test/src/client/mod.rs b/client/service/test/src/client/mod.rs index 661cf83fc49bf..97c22a1cb509e 100644 --- a/client/service/test/src/client/mod.rs +++ b/client/service/test/src/client/mod.rs @@ -1137,7 +1137,7 @@ fn get_block_by_bad_block_hash_returns_none() { let client = substrate_test_runtime_client::new(); let hash = H256::from_low_u64_be(5); - assert!(client.block(&BlockId::Hash(hash)).unwrap().is_none()); + assert!(client.block(hash).unwrap().is_none()); } #[test] @@ -1497,10 +1497,7 @@ fn returns_status_for_pruned_blocks() { block_on(client.check_block(check_block_a1.clone())).unwrap(), ImportResult::imported(false), ); - assert_eq!( - client.block_status(&BlockId::hash(check_block_a1.hash)).unwrap(), - BlockStatus::Unknown, - ); + assert_eq!(client.block_status(check_block_a1.hash).unwrap(), BlockStatus::Unknown); block_on(client.import_as_final(BlockOrigin::Own, a1.clone())).unwrap(); @@ -1508,10 +1505,7 @@ fn returns_status_for_pruned_blocks() { block_on(client.check_block(check_block_a1.clone())).unwrap(), ImportResult::AlreadyInChain, ); - assert_eq!( - client.block_status(&BlockId::hash(check_block_a1.hash)).unwrap(), - BlockStatus::InChainWithState, - ); + assert_eq!(client.block_status(check_block_a1.hash).unwrap(), BlockStatus::InChainWithState); let a2 = client .new_block_at(&BlockId::Hash(a1.hash()), Default::default(), false) @@ -1534,18 +1528,12 @@ fn returns_status_for_pruned_blocks() { block_on(client.check_block(check_block_a1.clone())).unwrap(), ImportResult::AlreadyInChain, ); - assert_eq!( - client.block_status(&BlockId::hash(check_block_a1.hash)).unwrap(), - BlockStatus::InChainPruned, - ); + assert_eq!(client.block_status(check_block_a1.hash).unwrap(), BlockStatus::InChainPruned); assert_eq!( block_on(client.check_block(check_block_a2.clone())).unwrap(), ImportResult::AlreadyInChain, ); - assert_eq!( - client.block_status(&BlockId::hash(check_block_a2.hash)).unwrap(), - BlockStatus::InChainWithState, - ); + assert_eq!(client.block_status(check_block_a2.hash).unwrap(), BlockStatus::InChainWithState); let a3 = client .new_block_at(&BlockId::Hash(a2.hash()), Default::default(), false) @@ -1569,26 +1557,17 @@ fn returns_status_for_pruned_blocks() { block_on(client.check_block(check_block_a1.clone())).unwrap(), ImportResult::AlreadyInChain, ); - assert_eq!( - client.block_status(&BlockId::hash(check_block_a1.hash)).unwrap(), - BlockStatus::InChainPruned, - ); + assert_eq!(client.block_status(check_block_a1.hash).unwrap(), BlockStatus::InChainPruned); assert_eq!( block_on(client.check_block(check_block_a2.clone())).unwrap(), ImportResult::AlreadyInChain, ); - assert_eq!( - client.block_status(&BlockId::hash(check_block_a2.hash)).unwrap(), - BlockStatus::InChainPruned, - ); + assert_eq!(client.block_status(check_block_a2.hash).unwrap(), BlockStatus::InChainPruned); assert_eq!( block_on(client.check_block(check_block_a3.clone())).unwrap(), ImportResult::AlreadyInChain, ); - assert_eq!( - client.block_status(&BlockId::hash(check_block_a3.hash)).unwrap(), - BlockStatus::InChainWithState, - ); + assert_eq!(client.block_status(check_block_a3.hash).unwrap(), BlockStatus::InChainWithState); let mut check_block_b1 = BlockCheckParams { hash: b1.hash(), diff --git a/primitives/consensus/common/src/block_validation.rs b/primitives/consensus/common/src/block_validation.rs index 71f3a80b27a64..1dc42104913cb 100644 --- a/primitives/consensus/common/src/block_validation.rs +++ b/primitives/consensus/common/src/block_validation.rs @@ -19,18 +19,18 @@ use crate::BlockStatus; use futures::FutureExt as _; -use sp_runtime::{generic::BlockId, traits::Block}; +use sp_runtime::traits::Block; use std::{error::Error, future::Future, pin::Pin, sync::Arc}; /// A type which provides access to chain information. pub trait Chain { - /// Retrieve the status of the block denoted by the given [`BlockId`]. - fn block_status(&self, id: &BlockId) -> Result>; + /// Retrieve the status of the block denoted by the given [`Block::Hash`]. + fn block_status(&self, hash: B::Hash) -> Result>; } impl, B: Block> Chain for Arc { - fn block_status(&self, id: &BlockId) -> Result> { - (&**self).block_status(id) + fn block_status(&self, hash: B::Hash) -> Result> { + (&**self).block_status(hash) } } @@ -60,7 +60,7 @@ pub trait BlockAnnounceValidator { /// Returning [`Validation::Failure`] will lead to a decrease of the /// peers reputation as it sent us invalid data. /// - /// The returned future should only resolve to an error iff there was an internal error + /// The returned future should only resolve to an error if there was an internal error /// validating the block announcement. If the block announcement itself is invalid, this should /// *always* return [`Validation::Failure`]. fn validate( diff --git a/utils/frame/benchmarking-cli/src/block/bench.rs b/utils/frame/benchmarking-cli/src/block/bench.rs index 578158d8a2356..c69d95725eb14 100644 --- a/utils/frame/benchmarking-cli/src/block/bench.rs +++ b/utils/frame/benchmarking-cli/src/block/bench.rs @@ -94,9 +94,9 @@ where let block_num = BlockId::Number(i.into()); let parent_num = BlockId::Number(((i - 1) as u32).into()); let consumed = self.consumed_weight(&block_num)?; + let hash = self.client.expect_block_hash_from_id(&block_num)?; - let block = - self.client.block(&block_num)?.ok_or(format!("Block {} not found", block_num))?; + let block = self.client.block(hash)?.ok_or(format!("Block {} not found", block_num))?; let block = self.unsealed(block.block); let took = self.measure_block(&block, &parent_num)?;