Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

BlockId removal: refactor: BlockBackend::block|block_status #13014

Merged
merged 5 commits into from
Jan 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions client/api/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -120,14 +120,13 @@ pub trait BlockBackend<Block: BlockT> {
/// that are indexed by the runtime with `storage_index_transaction`.
fn block_indexed_body(&self, hash: Block::Hash) -> sp_blockchain::Result<Option<Vec<Vec<u8>>>>;

/// Get full block by id.
fn block(&self, id: &BlockId<Block>) -> sp_blockchain::Result<Option<SignedBlock<Block>>>;
/// Get full block by hash.
fn block(&self, hash: Block::Hash) -> sp_blockchain::Result<Option<SignedBlock<Block>>>;

/// Get block status.
fn block_status(&self, id: &BlockId<Block>)
-> sp_blockchain::Result<sp_consensus::BlockStatus>;
/// Get block status by block hash.
fn block_status(&self, hash: Block::Hash) -> sp_blockchain::Result<sp_consensus::BlockStatus>;

/// 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<Option<Justifications>>;

/// Get block hash by number.
Expand Down
4 changes: 2 additions & 2 deletions client/cli/src/commands/export_blocks_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -73,7 +73,7 @@ impl ExportBlocksCmd {
) -> error::Result<()>
where
B: BlockT,
C: BlockBackend<B> + UsageProvider<B> + 'static,
C: HeaderBackend<B> + BlockBackend<B> + UsageProvider<B> + 'static,
<<B::Header as HeaderT>::Number as FromStr>::Err: Debug,
{
if let Some(path) = database_config.path() {
Expand Down
11 changes: 4 additions & 7 deletions client/network/sync/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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?
Expand Down Expand Up @@ -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)
}
},
) {
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion client/rpc-spec-v2/src/chain_head/chain_head.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
7 changes: 2 additions & 5 deletions client/rpc/src/chain/chain_full.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Block: BlockT, Client> {
Expand Down Expand Up @@ -66,7 +63,7 @@ where
}

fn block(&self, hash: Option<Block::Hash>) -> Result<Option<SignedBlock<Block>>, 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) {
Expand Down
5 changes: 1 addition & 4 deletions client/rpc/src/dev/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 12 additions & 9 deletions client/service/src/chain_ops/check_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<B, IQ, C>(
pub async fn check_block<B, IQ, C>(
client: Arc<C>,
import_queue: IQ,
block_id: BlockId<B>,
) -> Pin<Box<dyn Future<Output = Result<(), Error>> + Send>>
) -> Result<(), Error>
where
C: BlockBackend<B> + HeaderBackend<B> + Send + Sync + 'static,
B: BlockT + for<'de> serde::Deserialize<'de>,
IQ: ImportQueue<B> + '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")?,
}
}
12 changes: 8 additions & 4 deletions client/service/src/chain_ops/export_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -37,7 +37,7 @@ pub fn export_blocks<B, C>(
binary: bool,
) -> Pin<Box<dyn Future<Output = Result<(), Error>>>>
where
C: BlockBackend<B> + UsageProvider<B> + 'static,
C: HeaderBackend<B> + BlockBackend<B> + UsageProvider<B> + 'static,
B: BlockT,
{
let mut block = from;
Expand Down Expand Up @@ -75,15 +75,19 @@ 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())?;
} else {
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() {
Expand Down
48 changes: 23 additions & 25 deletions client/service/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
(
Expand Down Expand Up @@ -1025,17 +1026,18 @@ where
}

/// Get block status.
pub fn block_status(&self, id: &BlockId<Block>) -> sp_blockchain::Result<BlockStatus> {
pub fn block_status(&self, hash: Block::Hash) -> sp_blockchain::Result<BlockStatus> {
// 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) {
Expand Down Expand Up @@ -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 =>
Expand All @@ -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 => {},
Expand Down Expand Up @@ -1947,20 +1949,16 @@ where
self.body(hash)
}

fn block(&self, id: &BlockId<Block>) -> sp_blockchain::Result<Option<SignedBlock<Block>>> {
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<Option<SignedBlock<Block>>> {
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<Block>) -> sp_blockchain::Result<BlockStatus> {
Client::block_status(self, id)
fn block_status(&self, hash: Block::Hash) -> sp_blockchain::Result<BlockStatus> {
Client::block_status(self, hash)
}

fn justifications(&self, hash: Block::Hash) -> sp_blockchain::Result<Option<Justifications>> {
Expand Down Expand Up @@ -2055,9 +2053,9 @@ where
{
fn block_status(
&self,
id: &BlockId<B>,
hash: B::Hash,
) -> Result<BlockStatus, Box<dyn std::error::Error + Send>> {
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<_>)
}
}

Expand Down
Loading