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

BlockId removal: refactor: HeaderBackend::header #12874

Merged
merged 17 commits into from
Dec 20, 2022
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
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