chore(tree): accept owned block hash in make_canonical instead of ref (#7465)

This commit is contained in:
Roman Krasiuk
2024-04-04 21:39:30 +02:00
committed by GitHub
parent 67558db6f3
commit 57e25be058
6 changed files with 40 additions and 40 deletions

View File

@ -926,7 +926,7 @@ where
#[instrument(level = "trace", skip(self), target = "blockchain_tree")]
pub fn make_canonical(
&mut self,
block_hash: &BlockHash,
block_hash: BlockHash,
) -> Result<CanonicalOutcome, CanonicalError> {
let mut durations_recorder = MakeCanonicalDurationsRecorder::default();
@ -935,16 +935,16 @@ where
durations_recorder.record_relative(MakeCanonicalAction::CloneOldBlocks);
// If block is already canonical don't return error.
let canonical_header = self.find_canonical_header(block_hash)?;
let canonical_header = self.find_canonical_header(&block_hash)?;
durations_recorder.record_relative(MakeCanonicalAction::FindCanonicalHeader);
if let Some(header) = canonical_header {
info!(target: "blockchain_tree", ?block_hash, "Block is already canonical, ignoring.");
info!(target: "blockchain_tree", %block_hash, "Block is already canonical, ignoring.");
// TODO: this could be fetched from the chainspec first
let td =
self.externals.provider_factory.provider()?.header_td(block_hash)?.ok_or_else(
self.externals.provider_factory.provider()?.header_td(&block_hash)?.ok_or_else(
|| {
CanonicalError::from(BlockValidationError::MissingTotalDifficulty {
hash: *block_hash,
hash: block_hash,
})
},
)?;
@ -956,16 +956,16 @@ where
.active_at_ttd(td, U256::ZERO)
{
return Err(CanonicalError::from(BlockValidationError::BlockPreMerge {
hash: *block_hash,
hash: block_hash,
}))
}
return Ok(CanonicalOutcome::AlreadyCanonical { header })
}
let Some(chain_id) = self.block_indices().get_blocks_chain_id(block_hash) else {
let Some(chain_id) = self.block_indices().get_blocks_chain_id(&block_hash) else {
debug!(target: "blockchain_tree", ?block_hash, "Block hash not found in block indices");
return Err(CanonicalError::from(BlockchainTreeError::BlockHashNotFoundInChain {
block_hash: *block_hash,
block_hash,
}))
};
let chain = self.state.chains.remove(&chain_id).expect("To be present");
@ -973,7 +973,7 @@ where
trace!(target: "blockchain_tree", ?chain, "Found chain to make canonical");
// we are splitting chain at the block hash that we want to make canonical
let canonical = self.split_chain(chain_id, chain, ChainSplitTarget::Hash(*block_hash));
let canonical = self.split_chain(chain_id, chain, ChainSplitTarget::Hash(block_hash));
durations_recorder.record_relative(MakeCanonicalAction::SplitChain);
let mut fork_block = canonical.fork_block();
@ -1535,7 +1535,7 @@ mod tests {
tree.insert_block(fork_block.clone(), BlockValidationKind::Exhaustive).unwrap();
assert_eq!(
tree.make_canonical(&fork_block.hash()).unwrap(),
tree.make_canonical(fork_block.hash()).unwrap(),
CanonicalOutcome::Committed { head: fork_block.header.clone() }
);
@ -1545,7 +1545,7 @@ mod tests {
);
assert_eq!(
tree.make_canonical(&canonical_block_1.hash()).unwrap(),
tree.make_canonical(canonical_block_1.hash()).unwrap(),
CanonicalOutcome::Committed { head: canonical_block_1.header.clone() }
);
@ -1560,12 +1560,12 @@ mod tests {
);
assert_eq!(
tree.make_canonical(&sidechain_block_1.hash()).unwrap(),
tree.make_canonical(sidechain_block_1.hash()).unwrap(),
CanonicalOutcome::Committed { head: sidechain_block_1.header.clone() }
);
assert_eq!(
tree.make_canonical(&canonical_block_1.hash()).unwrap(),
tree.make_canonical(canonical_block_1.hash()).unwrap(),
CanonicalOutcome::Committed { head: canonical_block_1.header.clone() }
);
@ -1575,7 +1575,7 @@ mod tests {
);
assert_eq!(
tree.make_canonical(&sidechain_block_2.hash()).unwrap(),
tree.make_canonical(sidechain_block_2.hash()).unwrap(),
CanonicalOutcome::Committed { head: sidechain_block_2.header.clone() }
);
@ -1585,7 +1585,7 @@ mod tests {
);
assert_eq!(
tree.make_canonical(&canonical_block_3.hash()).unwrap(),
tree.make_canonical(canonical_block_3.hash()).unwrap(),
CanonicalOutcome::Committed { head: canonical_block_3.header.clone() }
);
}
@ -1610,7 +1610,7 @@ mod tests {
let config = BlockchainTreeConfig::new(1, 2, 3, 2);
let mut tree = BlockchainTree::new(externals, config, None).expect("failed to create tree");
// genesis block 10 is already canonical
tree.make_canonical(&B256::ZERO).unwrap();
tree.make_canonical(B256::ZERO).unwrap();
// make genesis block 10 as finalized
tree.finalize_block(10);
@ -1632,7 +1632,7 @@ mod tests {
assert!(block2_chain.trie_updates().is_some());
assert_eq!(
tree.make_canonical(&block2.hash()).unwrap(),
tree.make_canonical(block2.hash()).unwrap(),
CanonicalOutcome::Committed { head: block2.header.clone() }
);
@ -1645,7 +1645,7 @@ mod tests {
assert!(block3_chain.trie_updates().is_some());
assert_eq!(
tree.make_canonical(&block3.hash()).unwrap(),
tree.make_canonical(block3.hash()).unwrap(),
CanonicalOutcome::Committed { head: block3.header.clone() }
);
@ -1667,7 +1667,7 @@ mod tests {
assert!(block5_chain.trie_updates().is_some());
assert_eq!(
tree.make_canonical(&block5.hash()).unwrap(),
tree.make_canonical(block5.hash()).unwrap(),
CanonicalOutcome::Committed { head: block5.header.clone() }
);
@ -1695,7 +1695,7 @@ mod tests {
let config = BlockchainTreeConfig::new(1, 2, 3, 2);
let mut tree = BlockchainTree::new(externals, config, None).expect("failed to create tree");
// genesis block 10 is already canonical
tree.make_canonical(&B256::ZERO).unwrap();
tree.make_canonical(B256::ZERO).unwrap();
// make genesis block 10 as finalized
tree.finalize_block(10);
@ -1795,7 +1795,7 @@ mod tests {
let mut canon_notif = tree.subscribe_canon_state();
// genesis block 10 is already canonical
tree.make_canonical(&B256::ZERO).unwrap();
tree.make_canonical(B256::ZERO).unwrap();
// make sure is_block_hash_canonical returns true for genesis block
tree.is_block_hash_canonical(&B256::ZERO).unwrap();
@ -1874,12 +1874,12 @@ mod tests {
);
// make block1 canonical
tree.make_canonical(&block1.hash()).unwrap();
tree.make_canonical(block1.hash()).unwrap();
// check notification
assert_matches!(canon_notif.try_recv(), Ok(CanonStateNotification::Commit{ new}) if *new.blocks() == BTreeMap::from([(block1.number,block1.clone())]));
// make block2 canonicals
tree.make_canonical(&block2.hash()).unwrap();
tree.make_canonical(block2.hash()).unwrap();
// check notification.
assert_matches!(canon_notif.try_recv(), Ok(CanonStateNotification::Commit{ new}) if *new.blocks() == BTreeMap::from([(block2.number,block2.clone())]));
@ -1950,7 +1950,7 @@ mod tests {
.assert(&tree);
// make b2a canonical
assert!(tree.make_canonical(&block2a_hash).is_ok());
assert!(tree.make_canonical(block2a_hash).is_ok());
// check notification.
assert_matches!(canon_notif.try_recv(),
Ok(CanonStateNotification::Reorg{ old, new})
@ -1979,7 +1979,7 @@ mod tests {
.with_pending_blocks((block2.number + 1, HashSet::new()))
.assert(&tree);
assert_matches!(tree.make_canonical(&block1a_hash), Ok(_));
assert_matches!(tree.make_canonical(block1a_hash), Ok(_));
// Trie state:
// b2a b2 (side chain)
// | /
@ -2017,7 +2017,7 @@ mod tests {
assert!(tree.is_block_hash_canonical(&block1a.hash()).unwrap());
// make b2 canonical
tree.make_canonical(&block2.hash()).unwrap();
tree.make_canonical(block2.hash()).unwrap();
// Trie state:
// b2 b2a (side chain)
// | /
@ -2092,7 +2092,7 @@ mod tests {
.assert(&tree);
// commit b2a
tree.make_canonical(&block2.hash()).unwrap();
tree.make_canonical(block2.hash()).unwrap();
// Trie state:
// b2 b2a (side chain)

View File

@ -53,8 +53,8 @@ impl BlockchainTreeEngine for NoopBlockchainTree {
Ok(())
}
fn make_canonical(&self, block_hash: &BlockHash) -> Result<CanonicalOutcome, CanonicalError> {
Err(BlockchainTreeError::BlockHashNotFoundInChain { block_hash: *block_hash }.into())
fn make_canonical(&self, block_hash: BlockHash) -> Result<CanonicalOutcome, CanonicalError> {
Err(BlockchainTreeError::BlockHashNotFoundInChain { block_hash }.into())
}
fn unwind(&self, _unwind_to: BlockNumber) -> RethResult<()> {

View File

@ -60,7 +60,7 @@ where
block: SealedBlockWithSenders,
validation_kind: BlockValidationKind,
) -> Result<InsertPayloadOk, InsertBlockError> {
trace!(target: "blockchain_tree", hash=?block.hash(), number=block.number, parent_hash=?block.parent_hash, "Inserting block");
trace!(target: "blockchain_tree", hash = %block.hash(), number = block.number, parent_hash = %block.parent_hash, "Inserting block");
let mut tree = self.tree.write();
let res = tree.insert_block(block, validation_kind);
tree.update_chains_metrics();
@ -68,7 +68,7 @@ where
}
fn finalize_block(&self, finalized_block: BlockNumber) {
trace!(target: "blockchain_tree", ?finalized_block, "Finalizing block");
trace!(target: "blockchain_tree", finalized_block, "Finalizing block");
let mut tree = self.tree.write();
tree.finalize_block(finalized_block);
tree.update_chains_metrics();
@ -78,7 +78,7 @@ where
&self,
last_finalized_block: BlockNumber,
) -> RethResult<()> {
trace!(target: "blockchain_tree", ?last_finalized_block, "Connecting buffered blocks to canonical hashes and finalizing the tree");
trace!(target: "blockchain_tree", last_finalized_block, "Connecting buffered blocks to canonical hashes and finalizing the tree");
let mut tree = self.tree.write();
let res =
tree.connect_buffered_blocks_to_canonical_hashes_and_finalize(last_finalized_block);
@ -94,8 +94,8 @@ where
res
}
fn make_canonical(&self, block_hash: &BlockHash) -> Result<CanonicalOutcome, CanonicalError> {
trace!(target: "blockchain_tree", ?block_hash, "Making block canonical");
fn make_canonical(&self, block_hash: BlockHash) -> Result<CanonicalOutcome, CanonicalError> {
trace!(target: "blockchain_tree", %block_hash, "Making block canonical");
let mut tree = self.tree.write();
let res = tree.make_canonical(block_hash);
tree.update_chains_metrics();
@ -103,7 +103,7 @@ where
}
fn unwind(&self, unwind_to: BlockNumber) -> RethResult<()> {
trace!(target: "blockchain_tree", ?unwind_to, "Unwinding to block number");
trace!(target: "blockchain_tree", unwind_to, "Unwinding to block number");
let mut tree = self.tree.write();
let res = tree.unwind(unwind_to);
tree.update_chains_metrics();

View File

@ -373,7 +373,7 @@ where
}
let start = Instant::now();
let make_canonical_result = self.blockchain.make_canonical(&state.head_block_hash);
let make_canonical_result = self.blockchain.make_canonical(state.head_block_hash);
let elapsed = self.record_make_canonical_latency(start, &make_canonical_result);
let status = match make_canonical_result {
@ -1542,7 +1542,7 @@ where
// target might have changed since the block download request was issued
// (new FCU received)
let start = Instant::now();
let make_canonical_result = self.blockchain.make_canonical(&target.head_block_hash);
let make_canonical_result = self.blockchain.make_canonical(target.head_block_hash);
let elapsed = self.record_make_canonical_latency(start, &make_canonical_result);
match make_canonical_result {
Ok(outcome) => {
@ -1586,7 +1586,7 @@ where
.filter(|h| !h.is_head())
{
// TODO: do not ignore this
let _ = self.blockchain.make_canonical(target_hash.as_ref());
let _ = self.blockchain.make_canonical(*target_hash.as_ref());
}
}

View File

@ -96,7 +96,7 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
/// # Returns
///
/// Returns `Ok` if the blocks were canonicalized, or if the blocks were already canonical.
fn make_canonical(&self, block_hash: &BlockHash) -> Result<CanonicalOutcome, CanonicalError>;
fn make_canonical(&self, block_hash: BlockHash) -> Result<CanonicalOutcome, CanonicalError>;
/// Unwind tables and put it inside state
fn unwind(&self, unwind_to: BlockNumber) -> RethResult<()>;

View File

@ -669,7 +669,7 @@ where
self.tree.connect_buffered_blocks_to_canonical_hashes()
}
fn make_canonical(&self, block_hash: &BlockHash) -> Result<CanonicalOutcome, CanonicalError> {
fn make_canonical(&self, block_hash: BlockHash) -> Result<CanonicalOutcome, CanonicalError> {
self.tree.make_canonical(block_hash)
}