feat: add canonical outcome type (#2765)

This commit is contained in:
Matthias Seitz
2023-05-22 15:30:13 +02:00
committed by GitHub
parent 337579176d
commit 44348b4e48
8 changed files with 127 additions and 46 deletions

View File

@@ -93,9 +93,17 @@ impl BlockIndices {
(canonical_tip.number + 1, pending_blocks)
}
/// Returns the block number of the canonical block with the given hash.
///
/// Returns `None` if no block could be found in the canonical chain.
#[inline]
pub(crate) fn get_canonical_block_number(&self, block_hash: &BlockHash) -> Option<BlockNumber> {
self.canonical_chain.get_canonical_block_number(self.last_finalized_block, block_hash)
}
/// Check if block hash belongs to canonical chain.
pub fn is_block_hash_canonical(&self, block_hash: &BlockHash) -> bool {
self.canonical_chain.is_block_hash_canonical(self.last_finalized_block, block_hash)
pub(crate) fn is_block_hash_canonical(&self, block_hash: &BlockHash) -> bool {
self.get_canonical_block_number(block_hash).is_some()
}
/// Last finalized block

View File

@@ -7,7 +7,7 @@ use reth_db::{cursor::DbCursorRO, database::Database, tables, transaction::DbTx}
use reth_interfaces::{
blockchain_tree::{
error::{BlockchainTreeError, InsertBlockError, InsertBlockErrorKind},
BlockStatus,
BlockStatus, CanonicalOutcome,
},
consensus::{Consensus, ConsensusError},
executor::BlockExecutionError,
@@ -15,7 +15,7 @@ use reth_interfaces::{
};
use reth_primitives::{
BlockHash, BlockNumHash, BlockNumber, ForkBlock, Hardfork, SealedBlock, SealedBlockWithSenders,
U256,
SealedHeader, U256,
};
use reth_provider::{
chain::{ChainSplit, SplitAt},
@@ -211,6 +211,11 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
chain.block(block_hash)
}
/// Returns true if the block is included in a side-chain.
fn is_block_hash_inside_chain(&self, block_hash: BlockHash) -> bool {
self.block_by_hash(block_hash).is_some()
}
/// Returns the block that's considered the `Pending` block, if it exists.
pub fn pending_block(&self) -> Option<&SealedBlock> {
let b = self.block_indices.pending_block_num_hash()?;
@@ -775,19 +780,34 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
}
}
/// Determines whether or not a block is canonical, checking the db if necessary.
pub fn is_block_hash_canonical(&self, hash: &BlockHash) -> Result<bool, Error> {
/// Attempts to find the header for the given block hash if it is canonical.
///
/// Returns `Ok(None)` if the block hash is not canonical (block hash does not exist, or is
/// included in a sidechain).
pub fn find_canonical_header(&self, hash: &BlockHash) -> Result<Option<SealedHeader>, Error> {
// if the indices show that the block hash is not canonical, it's either in a sidechain or
// canonical, but in the db. If it is in a sidechain, it is not canonical. If it is not in
// the db, then it is not canonical.
if !self.block_indices.is_block_hash_canonical(hash) &&
(self.block_by_hash(*hash).is_some() ||
self.externals.database().header(hash)?.is_none())
{
return Ok(false)
let mut header = None;
if let Some(num) = self.block_indices.get_canonical_block_number(hash) {
header = self.externals.database().header_by_number(num)?;
}
Ok(true)
if header.is_none() && self.is_block_hash_inside_chain(*hash) {
return Ok(None)
}
if header.is_none() {
header = self.externals.database().header(hash)?
}
Ok(header.map(|header| header.seal(*hash)))
}
/// Determines whether or not a block is canonical, checking the db if necessary.
pub fn is_block_hash_canonical(&self, hash: &BlockHash) -> Result<bool, Error> {
self.find_canonical_header(hash).map(|header| header.is_some())
}
/// Make a block and its parent(s) part of the canonical chain.
@@ -802,12 +822,12 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
/// Returns `Ok` if the blocks were canonicalized, or if the blocks were already canonical.
#[track_caller]
#[instrument(skip(self), target = "blockchain_tree")]
pub fn make_canonical(&mut self, block_hash: &BlockHash) -> Result<(), Error> {
pub fn make_canonical(&mut self, block_hash: &BlockHash) -> Result<CanonicalOutcome, Error> {
let old_block_indices = self.block_indices.clone();
let old_buffered_blocks = self.buffered_blocks.parent_to_child.clone();
// If block is already canonical don't return error.
if self.is_block_hash_canonical(block_hash)? {
if let Some(header) = self.find_canonical_header(block_hash)? {
info!(target: "blockchain_tree", ?block_hash, "Block is already canonical");
let td = self
.externals
@@ -817,7 +837,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
if !self.externals.chain_spec.fork(Hardfork::Paris).active_at_ttd(td, U256::ZERO) {
return Err(BlockExecutionError::BlockPreMerge { hash: *block_hash }.into())
}
return Ok(())
return Ok(CanonicalOutcome::AlreadyCanonical { header })
}
let Some(chain_id) = self.block_indices.get_blocks_chain_id(block_hash) else {
@@ -912,10 +932,13 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
}
}
//
let head = chain_notification.tip().header.clone();
// send notification about new canonical chain.
let _ = self.canon_state_notification_sender.send(chain_notification);
Ok(())
Ok(CanonicalOutcome::Committed { head })
}
/// Subscribe to new blocks events.
@@ -1127,7 +1150,7 @@ mod tests {
BlockchainTree::new(externals, sender, config).expect("failed to create tree");
// genesis block 10 is already canonical
assert_eq!(tree.make_canonical(&H256::zero()), Ok(()));
assert!(tree.make_canonical(&H256::zero()).is_ok());
// make sure is_block_hash_canonical returns true for genesis block
assert!(tree.is_block_hash_canonical(&H256::zero()).unwrap());
@@ -1189,12 +1212,12 @@ mod tests {
assert_eq!(tree.insert_block(block2.clone()).unwrap(), BlockStatus::Valid);
// make block1 canonical
assert_eq!(tree.make_canonical(&block1.hash()), Ok(()));
assert!(tree.make_canonical(&block1.hash()).is_ok());
// check notification
assert_matches!(canon_notif.try_recv(), Ok(CanonStateNotification::Commit{ new}) if *new.blocks() == BTreeMap::from([(block1.number,block1.clone())]));
// make block2 canonicals
assert_eq!(tree.make_canonical(&block2.hash()), Ok(()));
assert!(tree.make_canonical(&block2.hash()).is_ok());
// check notification.
assert_matches!(canon_notif.try_recv(), Ok(CanonStateNotification::Commit{ new}) if *new.blocks() == BTreeMap::from([(block2.number,block2.clone())]));
@@ -1256,7 +1279,7 @@ mod tests {
.assert(&tree);
// make b2a canonical
assert_eq!(tree.make_canonical(&block2a_hash), Ok(()));
assert!(tree.make_canonical(&block2a_hash).is_ok());
// check notification.
assert_matches!(canon_notif.try_recv(),
Ok(CanonStateNotification::Reorg{ old, new})
@@ -1282,7 +1305,7 @@ mod tests {
.with_pending_blocks((block2.number + 1, HashSet::new()))
.assert(&tree);
assert_eq!(tree.make_canonical(&block1a_hash), Ok(()));
assert!(tree.make_canonical(&block1a_hash).is_ok());
// Trie state:
// b2a b2 (side chain)
// | /
@@ -1320,7 +1343,7 @@ mod tests {
assert!(tree.is_block_hash_canonical(&block1a.hash).unwrap());
// make b2 canonical
assert_eq!(tree.make_canonical(&block2.hash()), Ok(()));
assert!(tree.make_canonical(&block2.hash()).is_ok());
// Trie state:
// b2 b2a (side chain)
@@ -1390,7 +1413,7 @@ mod tests {
.assert(&tree);
// commit b2a
assert_eq!(tree.make_canonical(&block2.hash), Ok(()));
assert!(tree.make_canonical(&block2.hash).is_ok());
// Trie state:
// b2 b2a (side chain)

View File

@@ -42,14 +42,18 @@ impl CanonicalChain {
)
}
/// Check if block hash belongs to canonical chain.
/// Returns the block number of the canonical block with the given hash.
///
/// Returns `None` if no block could be found in the canonical chain.
#[inline]
pub(crate) fn is_block_hash_canonical(
pub(crate) fn get_canonical_block_number(
&self,
last_finalized_block: BlockNumber,
block_hash: &BlockHash,
) -> bool {
self.chain.range(last_finalized_block..).any(|(_, &h)| h == *block_hash)
) -> Option<BlockNumber> {
self.chain
.range(last_finalized_block..)
.find_map(|(num, &h)| (h == *block_hash).then_some(*num))
}
/// Extends all items from the given iterator to the chain.

View File

@@ -5,6 +5,7 @@ use reth_db::database::Database;
use reth_interfaces::{
blockchain_tree::{
error::InsertBlockError, BlockStatus, BlockchainTreeEngine, BlockchainTreeViewer,
CanonicalOutcome,
},
consensus::Consensus,
Error,
@@ -68,7 +69,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTreeEngine
self.tree.write().restore_canonical_hashes(last_finalized_block)
}
fn make_canonical(&self, block_hash: &BlockHash) -> Result<(), Error> {
fn make_canonical(&self, block_hash: &BlockHash) -> Result<CanonicalOutcome, Error> {
trace!(target: "blockchain_tree", ?block_hash, "Making block canonical");
self.tree.write().make_canonical(block_hash)
}

View File

@@ -389,30 +389,23 @@ where
// We can only process new forkchoice updates if the pipeline is idle, since it requires
// exclusive access to the database
match self.blockchain.make_canonical(&state.head_block_hash) {
Ok(_) => {
let head_block_number = self
.get_block_number(state.head_block_hash)?
.expect("was canonicalized, so it exists");
debug!(target: "consensus::engine", hash=?state.head_block_hash, number=head_block_number, "canonicalized new head");
Ok(outcome) => {
let header = outcome.into_header();
debug!(target: "consensus::engine", hash=?state.head_block_hash, number=header.number, "canonicalized new head");
let pipeline_min_progress =
FINISH.get_checkpoint(&self.db.tx()?)?.unwrap_or_default().block_number;
if pipeline_min_progress < head_block_number {
debug!(target: "consensus::engine", last_finished=pipeline_min_progress, head_number=head_block_number, "pipeline run to head required");
if pipeline_min_progress < header.number {
debug!(target: "consensus::engine", last_finished=pipeline_min_progress, head_number=header.number, "pipeline run to head required");
// TODO(mattsse) ideally sync blockwise
self.sync.set_pipeline_sync_target(state.head_block_hash);
}
if let Some(attrs) = attrs {
// get header for further validation
let header = self
.load_header(head_block_number)?
.expect("was canonicalized, so it exists");
let payload_response =
self.process_payload_attributes(attrs, header, state);
self.process_payload_attributes(attrs, header.unseal(), state);
if payload_response.is_valid_update() {
// we will return VALID, so let's make sure the info tracker is
// properly updated

View File

@@ -60,7 +60,8 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
/// [`BlockchainTreeEngine::finalize_block`]).
fn restore_canonical_hashes(&self, last_finalized_block: BlockNumber) -> Result<(), Error>;
/// Make a block and its parent part of the canonical chain by committing it to the database.
/// Make a block and its parent chain part of the canonical chain by committing it to the
/// database.
///
/// # Note
///
@@ -70,12 +71,45 @@ 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<(), Error>;
fn make_canonical(&self, block_hash: &BlockHash) -> Result<CanonicalOutcome, Error>;
/// Unwind tables and put it inside state
fn unwind(&self, unwind_to: BlockNumber) -> Result<(), Error>;
}
/// All possible outcomes of a canonicalization attempt of [BlockchainTreeEngine::make_canonical].
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum CanonicalOutcome {
/// The block is already canonical.
AlreadyCanonical {
/// The corresponding [SealedHeader] that is already canonical.
header: SealedHeader,
},
/// Committed the block to the database.
Committed {
/// The new corresponding canonical head
head: SealedHeader,
},
}
impl CanonicalOutcome {
/// Returns the header of the block that was made canonical.
pub fn header(&self) -> &SealedHeader {
match self {
CanonicalOutcome::AlreadyCanonical { header } => header,
CanonicalOutcome::Committed { head } => head,
}
}
/// Consumes the outcome and returns the header of the block that was made canonical.
pub fn into_header(self) -> SealedHeader {
match self {
CanonicalOutcome::AlreadyCanonical { header } => header,
CanonicalOutcome::Committed { head } => head,
}
}
}
/// From Engine API spec, block inclusion can be valid, accepted or invalid.
/// Invalid case is already covered by error, but we need to make distinction
/// between if it is valid (extends canonical chain) or just accepted (is side chain).

View File

@@ -35,7 +35,7 @@ mod state;
use crate::{providers::chain_info::ChainInfoTracker, traits::BlockSource};
pub use database::*;
pub use post_state_provider::PostStateProvider;
use reth_interfaces::blockchain_tree::error::InsertBlockError;
use reth_interfaces::blockchain_tree::{error::InsertBlockError, CanonicalOutcome};
/// The main type for interacting with the blockchain.
///
@@ -401,7 +401,7 @@ where
self.tree.restore_canonical_hashes(last_finalized_block)
}
fn make_canonical(&self, block_hash: &BlockHash) -> Result<()> {
fn make_canonical(&self, block_hash: &BlockHash) -> Result<CanonicalOutcome> {
self.tree.make_canonical(block_hash)
}

View File

@@ -1,6 +1,7 @@
//! Canonical chain state notification trait and types.
use crate::{chain::BlockReceipts, Chain};
use auto_impl::auto_impl;
use reth_primitives::SealedBlockWithSenders;
use std::{
pin::Pin,
sync::Arc,
@@ -98,7 +99,11 @@ impl CanonStateNotification {
}
}
/// Get new chain if any.
/// Get the new chain if any.
///
/// Returns the new committed [Chain] for [Self::Reorg] and [Self::Commit] variants.
///
/// Returns None for [Self::Revert] variant.
pub fn committed(&self) -> Option<Arc<Chain>> {
match self {
Self::Reorg { new, .. } => Some(new.clone()),
@@ -107,6 +112,19 @@ impl CanonStateNotification {
}
}
/// Returns the new tip of the chain.
///
/// Returns the new tip for [Self::Reorg] and [Self::Commit] variants which commit at least 1
/// new block. Returns the first block of the chain for [Self::Revert] variant, which is the
/// block that the chain reverted to.
pub fn tip(&self) -> &SealedBlockWithSenders {
match self {
Self::Reorg { new, .. } => new.tip(),
Self::Revert { old } => old.first(),
Self::Commit { new } => new.tip(),
}
}
/// Return receipt with its block number and transaction hash.
///
/// Last boolean is true if receipt is from reverted block.