Skip to content

Commit

Permalink
BlockId removal: refactor: HeaderBackend::header (paritytech#12874)
Browse files Browse the repository at this point in the history
* BlockId removal: refactor: HeaderBackend::header

It changes the arguments of:
- `HeaderBackend::header`,
- `Client::header`,
- `PeersClient::header`
- `ChainApi::block_header`

methods from: `BlockId<Block>` to: `Block::Hash`

This PR is part of BlockId::Number refactoring analysis (paritytech/substrate#11292)

* non-trivial usages of haeder(block_id) refactored

This may required introduction of dedicated function:
header_for_block_num

* fmt

* fix

* doc fixed

* ".git/.scripts/fmt.sh"

* BlockId removal: refactor: HeaderBackend::expect_header

It changes the arguments of `HeaderBackend::expect_header` method from: `BlockId<Block>` to: `Block::Hash`

* ".git/.scripts/fmt.sh"

* readme updated

* ".git/.scripts/fmt.sh"

* fix

Co-authored-by: parity-processbot <>
  • Loading branch information
michalkucharczyk authored and ltfschoen committed Feb 22, 2023
1 parent 839c8de commit 4fcb472
Show file tree
Hide file tree
Showing 55 changed files with 307 additions and 360 deletions.
2 changes: 1 addition & 1 deletion bin/node/bench/src/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ impl core::Benchmark for ConstructionBenchmark {
proposer_factory.init(
&context
.client
.header(&BlockId::number(0))
.header(context.client.chain_info().genesis_hash)
.expect("Database error querying block #0")
.expect("Block #0 should exist"),
),
Expand Down
5 changes: 2 additions & 3 deletions bin/node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,9 +640,8 @@ mod tests {
Ok((node, setup_handles.unwrap()))
},
|service, &mut (ref mut block_import, ref babe_link)| {
let parent_id = BlockId::number(service.client().chain_info().best_number);
let parent_header = service.client().header(&parent_id).unwrap().unwrap();
let parent_hash = parent_header.hash();
let parent_hash = service.client().chain_info().best_hash;
let parent_header = service.client().header(parent_hash).unwrap().unwrap();
let parent_number = *parent_header.number();

futures::executor::block_on(service.transaction_pool().maintain(
Expand Down
7 changes: 3 additions & 4 deletions bin/node/inspect/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,18 +147,17 @@ impl<TBlock: Block, TPrinter: PrettyPrinter<TBlock>> Inspector<TBlock, TPrinter>
.block_body(hash)?
.ok_or_else(|| Error::NotFound(not_found.clone()))?;
let header =
self.chain.header(id)?.ok_or_else(|| Error::NotFound(not_found.clone()))?;
self.chain.header(hash)?.ok_or_else(|| Error::NotFound(not_found.clone()))?;
TBlock::new(header, body)
},
BlockAddress::Hash(hash) => {
let id = BlockId::hash(hash);
let not_found = format!("Could not find block {:?}", id);
let not_found = format!("Could not find block {:?}", BlockId::<TBlock>::Hash(hash));
let body = self
.chain
.block_body(hash)?
.ok_or_else(|| Error::NotFound(not_found.clone()))?;
let header =
self.chain.header(id)?.ok_or_else(|| Error::NotFound(not_found.clone()))?;
self.chain.header(hash)?.ok_or_else(|| Error::NotFound(not_found.clone()))?;
TBlock::new(header, body)
},
})
Expand Down
10 changes: 4 additions & 6 deletions client/api/src/in_mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ impl<Block: BlockT> Blockchain<Block> {
/// Set an existing block as head.
pub fn set_head(&self, hash: Block::Hash) -> sp_blockchain::Result<()> {
let header = self
.header(BlockId::Hash(hash))?
.header(hash)?
.ok_or_else(|| sp_blockchain::Error::UnknownBlock(format!("{}", hash)))?;

self.apply_head(&header)
Expand Down Expand Up @@ -336,11 +336,9 @@ impl<Block: BlockT> Blockchain<Block> {
impl<Block: BlockT> HeaderBackend<Block> for Blockchain<Block> {
fn header(
&self,
id: BlockId<Block>,
hash: Block::Hash,
) -> sp_blockchain::Result<Option<<Block as BlockT>::Header>> {
Ok(self
.id(id)
.and_then(|hash| self.storage.read().blocks.get(&hash).map(|b| b.header().clone())))
Ok(self.storage.read().blocks.get(&hash).map(|b| b.header().clone()))
}

fn info(&self) -> blockchain::Info<Block> {
Expand Down Expand Up @@ -387,7 +385,7 @@ impl<Block: BlockT> HeaderMetadata<Block> for Blockchain<Block> {
&self,
hash: Block::Hash,
) -> Result<CachedHeaderMetadata<Block>, Self::Error> {
self.header(BlockId::hash(hash))?
self.header(hash)?
.map(|header| CachedHeaderMetadata::from(&header))
.ok_or_else(|| {
sp_blockchain::Error::UnknownBlock(format!("header not found: {}", hash))
Expand Down
2 changes: 1 addition & 1 deletion client/authority-discovery/src/worker/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl ProvideRuntimeApi<Block> for TestApi {
impl<Block: BlockT> HeaderBackend<Block> for TestApi {
fn header(
&self,
_id: BlockId<Block>,
_hash: Block::Hash,
) -> std::result::Result<Option<Block::Header>, sp_blockchain::Error> {
Ok(None)
}
Expand Down
2 changes: 1 addition & 1 deletion client/basic-authorship/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ let mut proposer_factory = ProposerFactory::new(client.clone(), txpool.clone(),

// From this factory, we create a `Proposer`.
let proposer = proposer_factory.init(
&client.header(&BlockId::number(0)).unwrap().unwrap(),
&client.header(client.chain_info().genesis_hash).unwrap().unwrap(),
);

// The proposer is created asynchronously.
Expand Down
40 changes: 16 additions & 24 deletions client/basic-authorship/src/basic_authorship.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,8 +617,7 @@ mod tests {
block_on(
txpool.maintain(chain_event(
client
.header(&BlockId::Number(0u64))
.expect("header get error")
.expect_header(client.info().genesis_hash)
.expect("there should be header"),
)),
);
Expand All @@ -628,7 +627,7 @@ mod tests {

let cell = Mutex::new((false, time::Instant::now()));
let proposer = proposer_factory.init_with_now(
&client.header(&BlockId::number(0)).unwrap().unwrap(),
&client.expect_header(client.info().genesis_hash).unwrap(),
Box::new(move || {
let mut value = cell.lock();
if !value.0 {
Expand Down Expand Up @@ -672,7 +671,7 @@ mod tests {

let cell = Mutex::new((false, time::Instant::now()));
let proposer = proposer_factory.init_with_now(
&client.header(&BlockId::number(0)).unwrap().unwrap(),
&client.expect_header(client.info().genesis_hash).unwrap(),
Box::new(move || {
let mut value = cell.lock();
if !value.0 {
Expand Down Expand Up @@ -705,15 +704,13 @@ mod tests {
);

let genesis_hash = client.info().best_hash;
let block_id = BlockId::Hash(genesis_hash);

block_on(txpool.submit_at(&BlockId::number(0), SOURCE, vec![extrinsic(0)])).unwrap();

block_on(
txpool.maintain(chain_event(
client
.header(&BlockId::Number(0u64))
.expect("header get error")
.expect_header(client.info().genesis_hash)
.expect("there should be header"),
)),
);
Expand All @@ -722,7 +719,7 @@ mod tests {
ProposerFactory::new(spawner.clone(), client.clone(), txpool.clone(), None, None);

let proposer = proposer_factory.init_with_now(
&client.header(&block_id).unwrap().unwrap(),
&client.header(genesis_hash).unwrap().unwrap(),
Box::new(move || time::Instant::now()),
);

Expand All @@ -734,7 +731,7 @@ mod tests {
assert_eq!(proposal.block.extrinsics().len(), 1);

let api = client.runtime_api();
api.execute_block(&block_id, proposal.block).unwrap();
api.execute_block(&BlockId::Hash(genesis_hash), proposal.block).unwrap();

let state = backend.state_at(genesis_hash).unwrap();

Expand Down Expand Up @@ -790,8 +787,9 @@ mod tests {
number,
expected_block_extrinsics,
expected_pool_transactions| {
let hash = client.expect_block_hash_from_id(&BlockId::Number(number)).unwrap();
let proposer = proposer_factory.init_with_now(
&client.header(&BlockId::number(number)).unwrap().unwrap(),
&client.expect_header(hash).unwrap(),
Box::new(move || time::Instant::now()),
);

Expand All @@ -813,23 +811,20 @@ mod tests {
block_on(
txpool.maintain(chain_event(
client
.header(&BlockId::Number(0u64))
.expect("header get error")
.expect_header(client.info().genesis_hash)
.expect("there should be header"),
)),
);
assert_eq!(txpool.ready().count(), 7);

// let's create one block and import it
let block = propose_block(&client, 0, 2, 7);
let hashof1 = block.hash();
block_on(client.import(BlockOrigin::Own, block)).unwrap();

block_on(
txpool.maintain(chain_event(
client
.header(&BlockId::Number(1))
.expect("header get error")
.expect("there should be header"),
client.expect_header(hashof1).expect("there should be header"),
)),
);
assert_eq!(txpool.ready().count(), 5);
Expand All @@ -851,8 +846,7 @@ mod tests {
client.clone(),
);
let genesis_header = client
.header(&BlockId::Number(0u64))
.expect("header get error")
.expect_header(client.info().genesis_hash)
.expect("there should be header");

let extrinsics_num = 5;
Expand Down Expand Up @@ -966,8 +960,7 @@ mod tests {
block_on(
txpool.maintain(chain_event(
client
.header(&BlockId::Number(0u64))
.expect("header get error")
.expect_header(client.info().genesis_hash)
.expect("there should be header"),
)),
);
Expand All @@ -978,7 +971,7 @@ mod tests {

let cell = Mutex::new(time::Instant::now());
let proposer = proposer_factory.init_with_now(
&client.header(&BlockId::number(0)).unwrap().unwrap(),
&client.expect_header(client.info().genesis_hash).unwrap(),
Box::new(move || {
let mut value = cell.lock();
let old = *value;
Expand Down Expand Up @@ -1029,8 +1022,7 @@ mod tests {
block_on(
txpool.maintain(chain_event(
client
.header(&BlockId::Number(0u64))
.expect("header get error")
.expect_header(client.info().genesis_hash)
.expect("there should be header"),
)),
);
Expand All @@ -1043,7 +1035,7 @@ mod tests {
let cell = Arc::new(Mutex::new((0, time::Instant::now())));
let cell2 = cell.clone();
let proposer = proposer_factory.init_with_now(
&client.header(&BlockId::number(0)).unwrap().unwrap(),
&client.expect_header(client.info().genesis_hash).unwrap(),
Box::new(move || {
let mut value = cell.lock();
let (called, old) = *value;
Expand Down
2 changes: 1 addition & 1 deletion client/basic-authorship/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
//!
//! // From this factory, we create a `Proposer`.
//! let proposer = proposer_factory.init(
//! &client.header(&BlockId::number(0)).unwrap().unwrap(),
//! &client.header(client.chain_info().genesis_hash).unwrap().unwrap(),
//! );
//!
//! // The proposer is created asynchronously.
Expand Down
2 changes: 1 addition & 1 deletion client/beefy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ where
})?;

// Move up the chain.
header = blockchain.expect_header(BlockId::Hash(parent_hash))?;
header = blockchain.expect_header(parent_hash)?;
};

aux_schema::write_current_version(backend)?;
Expand Down
3 changes: 1 addition & 2 deletions client/beefy/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,9 +448,8 @@ async fn wait_for_best_beefy_blocks(
wait_for.push(Box::pin(stream.take(len).for_each(move |best_beefy_hash| {
let expected = expected.next();
async move {
let block_id = BlockId::hash(best_beefy_hash);
let header =
net.lock().peer(i).client().as_client().expect_header(block_id).unwrap();
net.lock().peer(i).client().as_client().expect_header(best_beefy_hash).unwrap();
let best_beefy = *header.number();

assert_eq!(expected, Some(best_beefy).as_ref());
Expand Down
25 changes: 18 additions & 7 deletions client/beefy/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ where
.map(|hash| {
backend
.blockchain()
.expect_header(BlockId::hash(*hash))
.expect_header(*hash)
.expect("just finalized block should be available; qed.")
})
.chain(std::iter::once(header.clone()))
Expand Down Expand Up @@ -718,16 +718,25 @@ where
let target_header = if target_number == self.best_grandpa_block() {
self.persisted_state.best_grandpa_block_header.clone()
} else {
self.backend
let hash = self
.backend
.blockchain()
.expect_header(BlockId::Number(target_number))
.expect_block_hash_from_id(&BlockId::Number(target_number))
.map_err(|err| {
let err_msg = format!(
"Couldn't get header for block #{:?} (error: {:?}), skipping vote..",
"Couldn't get hash for block #{:?} (error: {:?}), skipping vote..",
target_number, err
);
Error::Backend(err_msg)
})?
})?;

self.backend.blockchain().expect_header(hash).map_err(|err| {
let err_msg = format!(
"Couldn't get header for block #{:?} ({:?}) (error: {:?}), skipping vote..",
target_number, hash, err
);
Error::Backend(err_msg)
})?
};
let target_hash = target_header.hash();

Expand Down Expand Up @@ -1050,8 +1059,10 @@ pub(crate) mod tests {
"/beefy/justifs/1".into(),
known_peers,
);
let at = BlockId::number(Zero::zero());
let genesis_header = backend.blockchain().expect_header(at).unwrap();
let genesis_header = backend
.blockchain()
.expect_header(backend.blockchain().info().genesis_hash)
.unwrap();
let persisted_state = PersistedState::checked_new(
genesis_header,
Zero::zero(),
Expand Down
4 changes: 2 additions & 2 deletions client/consensus/aura/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -957,7 +957,7 @@ mod tests {
compatibility_mode: Default::default(),
};

let head = client.header(&BlockId::Number(0)).unwrap().unwrap();
let head = client.expect_header(client.info().genesis_hash).unwrap();

let res = worker
.on_slot(SlotInfo {
Expand All @@ -972,6 +972,6 @@ mod tests {
.unwrap();

// The returned block should be imported and we should be able to get its header by now.
assert!(client.header(&BlockId::Hash(res.block.hash())).unwrap().is_some());
assert!(client.header(res.block.hash()).unwrap().is_some());
}
}
4 changes: 2 additions & 2 deletions client/consensus/babe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1425,7 +1425,7 @@ where
let parent_hash = *block.header.parent_hash();
let parent_header = self
.client
.header(BlockId::Hash(parent_hash))
.header(parent_hash)
.map_err(|e| ConsensusError::ChainLookup(e.to_string()))?
.ok_or_else(|| {
ConsensusError::ChainLookup(
Expand Down Expand Up @@ -1664,7 +1664,7 @@ where

let finalized_slot = {
let finalized_header = client
.header(BlockId::Hash(info.finalized_hash))
.header(info.finalized_hash)
.map_err(|e| ConsensusError::ClientImport(e.to_string()))?
.expect(
"best finalized hash was given by client; finalized headers must exist in db; qed",
Expand Down
Loading

0 comments on commit 4fcb472

Please sign in to comment.