chore(engine): tree action on downloaded block (#8409)

This commit is contained in:
Roman Krasiuk
2024-05-27 15:36:56 +02:00
committed by GitHub
parent a14e54922a
commit 07dfb9fdc4

View File

@ -1271,84 +1271,6 @@ where
Ok(PayloadStatus::new(status, latest_valid_hash))
}
/// Invoked if we successfully downloaded a new block from the network.
///
/// This will attempt to insert the block into the tree.
///
/// There are several scenarios:
///
/// ## [BlockStatus::Valid]
///
/// The block is connected to the current canonical chain and is valid.
/// If the block is an ancestor of the current forkchoice head, then we can try again to make
/// the chain canonical.
///
/// ## [BlockStatus::Disconnected]
///
/// The block is not connected to the canonical chain, and we need to download the missing
/// parent first.
///
/// ## Insert Error
///
/// If the insertion into the tree failed, then the block was well-formed (valid hash), but its
/// chain is invalid, which means the FCU that triggered the download is invalid. Here we can
/// stop because there's nothing to do here and the engine needs to wait for another FCU.
fn on_downloaded_block(&mut self, block: SealedBlock) {
let downloaded_num_hash = block.num_hash();
trace!(target: "consensus::engine", hash=?block.hash(), number=%block.number, "Downloaded full block");
// check if the block's parent is already marked as invalid
if self.check_invalid_ancestor_with_head(block.parent_hash, block.hash()).is_some() {
// can skip this invalid block
return
}
match self
.blockchain
.insert_block_without_senders(block, BlockValidationKind::SkipStateRootValidation)
{
Ok(status) => {
match status {
InsertPayloadOk::Inserted(BlockStatus::Valid(_)) => {
// block is connected to the canonical chain and is valid.
// if it's not connected to current canonical head, the state root
// has not been validated.
if let Err((hash, error)) =
self.try_make_sync_target_canonical(downloaded_num_hash)
{
if error.is_fatal() {
error!(target: "consensus::engine", %error, "Encountered fatal error while making sync target canonical: {:?}, {:?}", error, hash);
} else if !error.is_block_hash_not_found() {
debug!(
target: "consensus::engine",
"Unexpected error while making sync target canonical: {:?}, {:?}",
error,
hash
)
}
}
}
InsertPayloadOk::Inserted(BlockStatus::Disconnected {
missing_ancestor: missing_parent,
}) => {
// block is not connected to the canonical head, we need to download its
// missing branch first
self.on_disconnected_block(downloaded_num_hash, missing_parent);
}
_ => (),
}
}
Err(err) => {
warn!(target: "consensus::engine", %err, "Failed to insert downloaded block");
if err.kind().is_invalid_block() {
let (block, err) = err.split();
warn!(target: "consensus::engine", invalid_number=?block.number, invalid_hash=?block.hash(), %err, "Marking block as invalid");
self.invalid_headers.insert(block.header);
}
}
}
}
/// This handles downloaded blocks that are shown to be disconnected from the canonical chain.
///
/// This mainly compares the missing parent of the downloaded block with the current canonical
@ -1476,7 +1398,15 @@ where
) -> Result<EngineEventOutcome, BeaconConsensusEngineError> {
let outcome = match event {
EngineSyncEvent::FetchedFullBlock(block) => {
self.on_downloaded_block(block);
trace!(target: "consensus::engine", hash=?block.hash(), number=%block.number, "Downloaded full block");
// Insert block only if the block's parent is not marked as invalid
if self.check_invalid_ancestor_with_head(block.parent_hash, block.hash()).is_none()
{
let previous_action = self
.blockchain_tree_action
.replace(BlockchainTreeAction::InsertDownloadedPayload { block });
debug_assert!(previous_action.is_none(), "Pre-existing action found");
}
EngineEventOutcome::Processed
}
EngineSyncEvent::PipelineStarted(target) => {
@ -1794,6 +1724,55 @@ where
trace!(target: "consensus::engine", ?status, "Returning payload status");
let _ = tx.send(Ok(status));
}
BlockchainTreeAction::InsertDownloadedPayload { block } => {
let downloaded_num_hash = block.num_hash();
match self.blockchain.insert_block_without_senders(
block,
BlockValidationKind::SkipStateRootValidation,
) {
Ok(status) => {
match status {
InsertPayloadOk::Inserted(BlockStatus::Valid(_)) => {
// block is connected to the canonical chain and is valid.
// if it's not connected to current canonical head, the state root
// has not been validated.
if let Err((hash, error)) =
self.try_make_sync_target_canonical(downloaded_num_hash)
{
if error.is_fatal() {
error!(target: "consensus::engine", %error, "Encountered fatal error while making sync target canonical: {:?}, {:?}", error, hash);
} else if !error.is_block_hash_not_found() {
debug!(
target: "consensus::engine",
"Unexpected error while making sync target canonical: {:?}, {:?}",
error,
hash
)
}
}
}
InsertPayloadOk::Inserted(BlockStatus::Disconnected {
missing_ancestor: missing_parent,
}) => {
// block is not connected to the canonical head, we need to download
// its missing branch first
self.on_disconnected_block(downloaded_num_hash, missing_parent);
}
_ => (),
}
}
Err(err) => {
warn!(target: "consensus::engine", %err, "Failed to insert downloaded block");
if err.kind().is_invalid_block() {
let (block, err) = err.split();
warn!(target: "consensus::engine", invalid_number=?block.number, invalid_hash=?block.hash(), %err, "Marking block as invalid");
self.invalid_headers.insert(block.header);
}
}
}
}
};
Ok(EngineEventOutcome::Processed)
}
@ -1935,6 +1914,27 @@ enum BlockchainTreeAction<EngineT: EngineTypes> {
status: PayloadStatus,
tx: oneshot::Sender<Result<PayloadStatus, BeaconOnNewPayloadError>>,
},
/// Action to insert a new block that we successfully downloaded from the network.
/// There are several outcomes for inserting a downloaded block into the tree:
///
/// ## [BlockStatus::Valid]
///
/// The block is connected to the current canonical chain and is valid.
/// If the block is an ancestor of the current forkchoice head, then we can try again to
/// make the chain canonical.
///
/// ## [BlockStatus::Disconnected]
///
/// The block is not connected to the canonical chain, and we need to download the
/// missing parent first.
///
/// ## Insert Error
///
/// If the insertion into the tree failed, then the block was well-formed (valid hash),
/// but its chain is invalid, which means the FCU that triggered the
/// download is invalid. Here we can stop because there's nothing to do here
/// and the engine needs to wait for another FCU.
InsertDownloadedPayload { block: SealedBlock },
}
/// Represents outcomes of processing an engine event