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

Commit

Permalink
BlockId removal: refactor: BlockImportOperation + Backend::finalize_b…
Browse files Browse the repository at this point in the history
…lock

It changes the arguments of methods of `BlockImportOperation` trait
from: block: `BlockId<Block>` to: hash: `&Block::Hash`
`Backend::finalize_block` was also changed.

This PR is part of BlockId::Number refactoring analysis (paritytech/substrate#11292)
  • Loading branch information
michalkucharczyk committed Oct 20, 2022
1 parent be4499b commit 1dc571e
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 75 deletions.
8 changes: 4 additions & 4 deletions client/api/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,13 @@ pub trait BlockImportOperation<Block: BlockT> {
/// Mark a block as finalized.
fn mark_finalized(
&mut self,
id: BlockId<Block>,
hash: &Block::Hash,
justification: Option<Justification>,
) -> sp_blockchain::Result<()>;

/// Mark a block as new head. If both block import and set head are specified, set head
/// overrides block import's best block rule.
fn mark_head(&mut self, id: BlockId<Block>) -> sp_blockchain::Result<()>;
fn mark_head(&mut self, hash: &Block::Hash) -> sp_blockchain::Result<()>;

/// Add a transaction index operation.
fn update_transaction_index(&mut self, index: Vec<IndexOperation>)
Expand Down Expand Up @@ -476,12 +476,12 @@ pub trait Backend<Block: BlockT>: AuxStore + Send + Sync {
transaction: Self::BlockImportOperation,
) -> sp_blockchain::Result<()>;

/// Finalize block with given Id.
/// Finalize block with given `hash`.
///
/// This should only be called if the parent of the given block has been finalized.
fn finalize_block(
&self,
block: BlockId<Block>,
hash: &Block::Hash,
justification: Option<Justification>,
) -> sp_blockchain::Result<()>;

Expand Down
31 changes: 13 additions & 18 deletions client/api/src/in_mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,9 @@ impl<Block: BlockT> Blockchain<Block> {
}

/// Set an existing block as head.
pub fn set_head(&self, id: BlockId<Block>) -> sp_blockchain::Result<()> {
pub fn set_head(&self, id: Block::Hash) -> sp_blockchain::Result<()> {
let header = self
.header(id)?
.header(BlockId::Hash(id))?
.ok_or_else(|| sp_blockchain::Error::UnknownBlock(format!("{}", id)))?;

self.apply_head(&header)
Expand Down Expand Up @@ -271,21 +271,16 @@ impl<Block: BlockT> Blockchain<Block> {

fn finalize_header(
&self,
id: BlockId<Block>,
block: &Block::Hash,
justification: Option<Justification>,
) -> sp_blockchain::Result<()> {
let hash = match self.header(id)? {
Some(h) => h.hash(),
None => return Err(sp_blockchain::Error::UnknownBlock(format!("{}", id))),
};

let mut storage = self.storage.write();
storage.finalized_hash = hash;
storage.finalized_hash = *block;

if justification.is_some() {
let block = storage
.blocks
.get_mut(&hash)
.get_mut(block)
.expect("hash was fetched from a block in the db; qed");

let block_justifications = match block {
Expand Down Expand Up @@ -500,8 +495,8 @@ pub struct BlockImportOperation<Block: BlockT> {
new_state:
Option<<InMemoryBackend<HashFor<Block>> as StateBackend<HashFor<Block>>>::Transaction>,
aux: Vec<(Vec<u8>, Option<Vec<u8>>)>,
finalized_blocks: Vec<(BlockId<Block>, Option<Justification>)>,
set_head: Option<BlockId<Block>>,
finalized_blocks: Vec<(Block::Hash, Option<Justification>)>,
set_head: Option<Block::Hash>,
}

impl<Block: BlockT> BlockImportOperation<Block>
Expand Down Expand Up @@ -605,16 +600,16 @@ where

fn mark_finalized(
&mut self,
block: BlockId<Block>,
hash: &Block::Hash,
justification: Option<Justification>,
) -> sp_blockchain::Result<()> {
self.finalized_blocks.push((block, justification));
self.finalized_blocks.push((*hash, justification));
Ok(())
}

fn mark_head(&mut self, block: BlockId<Block>) -> sp_blockchain::Result<()> {
fn mark_head(&mut self, hash: &Block::Hash) -> sp_blockchain::Result<()> {
assert!(self.pending_block.is_none(), "Only one set block per operation is allowed");
self.set_head = Some(block);
self.set_head = Some(*hash);
Ok(())
}

Expand Down Expand Up @@ -710,7 +705,7 @@ where
fn commit_operation(&self, operation: Self::BlockImportOperation) -> sp_blockchain::Result<()> {
if !operation.finalized_blocks.is_empty() {
for (block, justification) in operation.finalized_blocks {
self.blockchain.finalize_header(block, justification)?;
self.blockchain.finalize_header(&block, justification)?;
}
}

Expand Down Expand Up @@ -743,7 +738,7 @@ where

fn finalize_block(
&self,
block: BlockId<Block>,
block: &Block::Hash,
justification: Option<Justification>,
) -> sp_blockchain::Result<()> {
self.blockchain.finalize_header(block, justification)
Expand Down
6 changes: 4 additions & 2 deletions client/beefy/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1371,8 +1371,10 @@ pub(crate) mod tests {
let mut best_block_stream = best_block_streams.drain(..).next().unwrap();
net.peer(0).push_blocks(2, false);
// finalize 1 and 2 without justifications
backend.finalize_block(BlockId::number(1), None).unwrap();
backend.finalize_block(BlockId::number(2), None).unwrap();
let hashof1 = backend.blockchain().expect_block_hash_from_id(&BlockId::Number(1)).unwrap();
let hashof2 = backend.blockchain().expect_block_hash_from_id(&BlockId::Number(2)).unwrap();
backend.finalize_block(&hashof1, None).unwrap();
backend.finalize_block(&hashof2, None).unwrap();

let justif = create_finality_proof(2);
// create new session at block #2
Expand Down
Loading

0 comments on commit 1dc571e

Please sign in to comment.