From c237b826900f8f1919960d7b31dcae879fb2b224 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Wed, 21 Dec 2022 11:25:16 +0100 Subject: [PATCH] BlockId removal: refactor: HeaderBackend::status (#12981) It changes the arguments of `HeaderBackend::status` method from: `BlockId<Block>` to: `Block::Hash` This PR is part of BlockId::Number refactoring analysis (paritytech/substrate#11292) Co-authored-by: parity-processbot <> --- substrate/client/api/src/in_mem.rs | 4 ++-- .../client/authority-discovery/src/worker/tests.rs | 2 +- substrate/client/consensus/babe/src/lib.rs | 2 +- substrate/client/db/src/lib.rs | 10 +++------- substrate/client/finality-grandpa/src/import.rs | 2 +- .../client/merkle-mountain-range/src/test_utils.rs | 4 ++-- substrate/client/service/src/client/client.rs | 14 +++++++------- substrate/primitives/blockchain/src/backend.rs | 2 +- 8 files changed, 18 insertions(+), 22 deletions(-) diff --git a/substrate/client/api/src/in_mem.rs b/substrate/client/api/src/in_mem.rs index 58add087074..144aa352f55 100644 --- a/substrate/client/api/src/in_mem.rs +++ b/substrate/client/api/src/in_mem.rs @@ -359,8 +359,8 @@ impl<Block: BlockT> HeaderBackend<Block> for Blockchain<Block> { } } - fn status(&self, id: BlockId<Block>) -> sp_blockchain::Result<BlockStatus> { - match self.id(id).map_or(false, |hash| self.storage.read().blocks.contains_key(&hash)) { + fn status(&self, hash: Block::Hash) -> sp_blockchain::Result<BlockStatus> { + match self.storage.read().blocks.contains_key(&hash) { true => Ok(BlockStatus::InChain), false => Ok(BlockStatus::Unknown), } diff --git a/substrate/client/authority-discovery/src/worker/tests.rs b/substrate/client/authority-discovery/src/worker/tests.rs index ea0c16eca80..ce55728a1bf 100644 --- a/substrate/client/authority-discovery/src/worker/tests.rs +++ b/substrate/client/authority-discovery/src/worker/tests.rs @@ -78,7 +78,7 @@ impl<Block: BlockT> HeaderBackend<Block> for TestApi { fn status( &self, - _id: BlockId<Block>, + _hash: Block::Hash, ) -> std::result::Result<sc_client_api::blockchain::BlockStatus, sp_blockchain::Error> { Ok(sc_client_api::blockchain::BlockStatus::Unknown) } diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs index 9134da0b83d..1d5d4b5fe54 100644 --- a/substrate/client/consensus/babe/src/lib.rs +++ b/substrate/client/consensus/babe/src/lib.rs @@ -1402,7 +1402,7 @@ where // early exit if block already in chain, otherwise the check for // epoch changes will error when trying to re-import an epoch change - match self.client.status(BlockId::Hash(hash)) { + match self.client.status(hash) { Ok(sp_blockchain::BlockStatus::InChain) => { // When re-importing existing block strip away intermediates. let _ = block.remove_intermediate::<BabeIntermediate<Block>>(INTERMEDIATE_KEY); diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index b1f5a6edcd5..3e6332732d6 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -553,12 +553,8 @@ impl<Block: BlockT> sc_client_api::blockchain::HeaderBackend<Block> for Blockcha } } - fn status(&self, id: BlockId<Block>) -> ClientResult<sc_client_api::blockchain::BlockStatus> { - let exists = match id { - BlockId::Hash(hash) => self.header(hash)?.is_some(), - BlockId::Number(n) => n <= self.meta.read().best_number, - }; - match exists { + fn status(&self, hash: Block::Hash) -> ClientResult<sc_client_api::blockchain::BlockStatus> { + match self.header(hash)?.is_some() { true => Ok(sc_client_api::blockchain::BlockStatus::InChain), false => Ok(sc_client_api::blockchain::BlockStatus::Unknown), } @@ -1223,7 +1219,7 @@ impl<Block: BlockT> Backend<Block> { } let parent_exists = - self.blockchain.status(BlockId::Hash(route_to))? == sp_blockchain::BlockStatus::InChain; + self.blockchain.status(route_to)? == sp_blockchain::BlockStatus::InChain; // Cannot find tree route with empty DB or when imported a detached block. if meta.best_hash != Default::default() && parent_exists { diff --git a/substrate/client/finality-grandpa/src/import.rs b/substrate/client/finality-grandpa/src/import.rs index 1359110132c..ed6e4a0fa2e 100644 --- a/substrate/client/finality-grandpa/src/import.rs +++ b/substrate/client/finality-grandpa/src/import.rs @@ -534,7 +534,7 @@ where // early exit if block already in chain, otherwise the check for // authority changes will error when trying to re-import a change block - match self.inner.status(BlockId::Hash(hash)) { + match self.inner.status(hash) { Ok(BlockStatus::InChain) => { // Strip justifications when re-importing an existing block. let _justifications = block.justifications.take(); diff --git a/substrate/client/merkle-mountain-range/src/test_utils.rs b/substrate/client/merkle-mountain-range/src/test_utils.rs index 1bc7a9bd5bb..39570e0d238 100644 --- a/substrate/client/merkle-mountain-range/src/test_utils.rs +++ b/substrate/client/merkle-mountain-range/src/test_utils.rs @@ -234,8 +234,8 @@ impl HeaderBackend<Block> for MockClient { self.client.lock().info() } - fn status(&self, id: BlockId<Block>) -> sc_client_api::blockchain::Result<BlockStatus> { - self.client.lock().status(id) + fn status(&self, hash: Hash) -> sc_client_api::blockchain::Result<BlockStatus> { + self.client.lock().status(hash) } fn number(&self, hash: Hash) -> sc_client_api::blockchain::Result<Option<BlockNumber>> { diff --git a/substrate/client/service/src/client/client.rs b/substrate/client/service/src/client/client.rs index bf25e803f5a..493fd320b7b 100644 --- a/substrate/client/service/src/client/client.rs +++ b/substrate/client/service/src/client/client.rs @@ -552,9 +552,9 @@ where CoreApi<Block> + ApiExt<Block, StateBackend = B::State>, { let parent_hash = *import_headers.post().parent_hash(); - let status = self.backend.blockchain().status(BlockId::Hash(hash))?; - let parent_exists = self.backend.blockchain().status(BlockId::Hash(parent_hash))? == - blockchain::BlockStatus::InChain; + let status = self.backend.blockchain().status(hash)?; + let parent_exists = + self.backend.blockchain().status(parent_hash)? == blockchain::BlockStatus::InChain; match (import_existing, status) { (false, blockchain::BlockStatus::InChain) => return Ok(ImportResult::AlreadyInChain), (false, blockchain::BlockStatus::Unknown) => {}, @@ -1572,8 +1572,8 @@ where self.backend.blockchain().info() } - fn status(&self, id: BlockId<Block>) -> sp_blockchain::Result<blockchain::BlockStatus> { - self.backend.blockchain().status(id) + fn status(&self, hash: Block::Hash) -> sp_blockchain::Result<blockchain::BlockStatus> { + self.backend.blockchain().status(hash) } fn number( @@ -1624,8 +1624,8 @@ where self.backend.blockchain().info() } - fn status(&self, id: BlockId<Block>) -> sp_blockchain::Result<blockchain::BlockStatus> { - (**self).status(id) + fn status(&self, hash: Block::Hash) -> sp_blockchain::Result<blockchain::BlockStatus> { + (**self).status(hash) } fn number( diff --git a/substrate/primitives/blockchain/src/backend.rs b/substrate/primitives/blockchain/src/backend.rs index 6199fd25d0f..ec9c8ac0d57 100644 --- a/substrate/primitives/blockchain/src/backend.rs +++ b/substrate/primitives/blockchain/src/backend.rs @@ -37,7 +37,7 @@ pub trait HeaderBackend<Block: BlockT>: Send + Sync { /// Get blockchain info. fn info(&self) -> Info<Block>; /// Get block status. - fn status(&self, id: BlockId<Block>) -> Result<BlockStatus>; + fn status(&self, hash: Block::Hash) -> Result<BlockStatus>; /// Get block number by hash. Returns `None` if the header is not in the chain. fn number( &self, -- GitLab