jleony/nanoreth
1
0
Fork 0
mirror of https://github.com/hl-archive-node/nanoreth.git synced 2025-12-06 10:59:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: propagate try_make_sync_target_canonical error (#7164)

Browse Source
This commit is contained in:
Dan Cline
2024-03-18 14:32:36 -04:00
committed by GitHub
parent 273f3c734c
commit 9962c39492
3 changed files with 72 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
crates/blockchain-tree/src/blockchain_tree.rs
View File

@ -1013,7 +1013,6 @@ impl<DB: Database, EVM: ExecutorFactory> BlockchainTree<DB, EVM> {
self.commit_canonical_to_database(new_canon_chain, &mut durations_recorder)?;
} else {
// it forks to canonical block that is not the tip.
let canon_fork: BlockNumHash = new_canon_chain.fork_block();
// sanity check
if self.block_indices().canonical_hash(&canon_fork.number) != Some(canon_fork.hash) {

83
crates/consensus/beacon/src/engine/mod.rs
View File

@ -430,7 +430,7 @@ where
Err(error) => {
if let RethError::Canonical(ref err) = error {
if err.is_fatal() {
tracing::error!(target: "consensus::engine", %err, "Encountered fatal error");
error!(target: "consensus::engine", %err, "Encountered fatal error");
return Err(error)
}
}
@ -1080,7 +1080,7 @@ where
///
/// This returns a [`PayloadStatus`] that represents the outcome of a processed new payload and
/// returns an error if an internal error occurred.
#[instrument(level = "trace", skip(self, payload, cancun_fields), fields(block_hash= ?payload.block_hash(), block_number = %payload.block_number(), is_pipeline_idle = %self.sync.is_pipeline_idle()), target = "consensus::engine")]
#[instrument(level = "trace", skip(self, payload, cancun_fields), fields(block_hash = ?payload.block_hash(), block_number = %payload.block_number(), is_pipeline_idle = %self.sync.is_pipeline_idle()), target = "consensus::engine")]
fn on_new_payload(
&mut self,
payload: ExecutionPayload,
@ -1120,7 +1120,30 @@ where
// if we're currently syncing and the inserted block is the targeted FCU
// head block, we can try to make it canonical.
if block_hash == target.head_block_hash {
self.try_make_sync_target_canonical(block_num_hash);
if let Err((_hash, error)) =
self.try_make_sync_target_canonical(block_num_hash)
{
if let RethError::Canonical(ref err) = error {
if err.is_fatal() {
error!(target: "consensus::engine", %err, "Encountered fatal error");
return Err(BeaconOnNewPayloadError::Internal(Box::new(
error,
)))
}
} else {
// If we could not make the sync target block canonical, we
// should return the error as an invalid payload status.
return Ok(PayloadStatus::new(
PayloadStatusEnum::Invalid {
validation_error: error.to_string(),
},
// TODO: return a proper latest valid hash
//
// See: <https://github.com/paradigmxyz/reth/issues/7146>
self.forkchoice_state_tracker.last_valid_head(),
))
}
}
}
}
// block was successfully inserted, so we can cancel the full block request, if
@ -1148,21 +1171,22 @@ where
/// - the versioned hashes passed with the payload do not exactly match transaction
/// versioned hashes
/// - the block does not contain blob transactions if it is pre-cancun
// This validates the following engine API rule:
//
// 3. Given the expected array of blob versioned hashes client software **MUST** run its
// validation by taking the following steps:
//
// 1. Obtain the actual array by concatenating blob versioned hashes lists
// (`tx.blob_versioned_hashes`) of each [blob
// transaction](https://eips.ethereum.org/EIPS/eip-4844#new-transaction-type) included
// in the payload, respecting the order of inclusion. If the payload has no blob
// transactions the expected array **MUST** be `[]`.
//
// 2. Return `{status: INVALID, latestValidHash: null, validationError: errorMessage | null}`
// if the expected and the actual arrays don't match.
//
// This validation **MUST** be instantly run in all cases even during active sync process.
///
/// This validates the following engine API rule:
///
/// 3. Given the expected array of blob versioned hashes client software **MUST** run its
/// validation by taking the following steps:
///
/// 1. Obtain the actual array by concatenating blob versioned hashes lists
/// (`tx.blob_versioned_hashes`) of each [blob
/// transaction](https://eips.ethereum.org/EIPS/eip-4844#new-transaction-type) included
/// in the payload, respecting the order of inclusion. If the payload has no blob
/// transactions the expected array **MUST** be `[]`.
///
/// 2. Return `{status: INVALID, latestValidHash: null, validationError: errorMessage | null}`
/// if the expected and the actual arrays don't match.
///
/// This validation **MUST** be instantly run in all cases even during active sync process.
fn ensure_well_formed_payload(
&self,
payload: ExecutionPayload,
@ -1277,6 +1301,7 @@ where
let status = self
.blockchain
.insert_block_without_senders(block.clone(), BlockValidationKind::Exhaustive)?;
let elapsed = start.elapsed();
let mut latest_valid_hash = None;
let block = Arc::new(block);
@ -1402,7 +1427,16 @@ where
// 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.
self.try_make_sync_target_canonical(downloaded_num_hash);
if let Err((hash, error)) =
self.try_make_sync_target_canonical(downloaded_num_hash)
{
tracing::error!(
target: "consensus::engine",
"Unexpected error while making sync target canonical: {:?}, {:?}",
error,
hash
)
}
}
InsertPayloadOk::Inserted(BlockStatus::Disconnected {
missing_ancestor: missing_parent,
@ -1484,7 +1518,10 @@ where
/// Note: This will not succeed if the sync target has changed since the block download request
/// was issued and the new target is still disconnected and additional missing blocks are
/// downloaded
fn try_make_sync_target_canonical(&mut self, inserted: BlockNumHash) {
fn try_make_sync_target_canonical(
&mut self,
inserted: BlockNumHash,
) -> Result<(), (B256, RethError)> {
if let Some(target) = self.forkchoice_state_tracker.sync_target_state() {
// optimistically try to make the head of the current FCU target canonical, the sync
// target might have changed since the block download request was issued
@ -1514,6 +1551,7 @@ where
// clear any active block requests
self.sync.clear_block_download_requests();
Ok(())
}
Err(err) => {
// if we failed to make the FCU's head canonical, because we don't have that
@ -1532,11 +1570,16 @@ where
if let Some(target_hash) = ForkchoiceStateHash::find(&target, inserted.hash)
.filter(|h| !h.is_head())
{
// TODO: do not ignore this
let _ = self.blockchain.make_canonical(target_hash.as_ref());
}
}
Err((target.head_block_hash, err))
}
}
} else {
Ok(())
}
}

14
crates/interfaces/src/blockchain_tree/error.rs
View File

@ -4,6 +4,7 @@ use crate::{
consensus::ConsensusError,
executor::{BlockExecutionError, BlockValidationError},
provider::ProviderError,
RethError,
};
use reth_primitives::{BlockHash, BlockNumber, SealedBlock};
@ -110,6 +111,11 @@ impl InsertBlockError {
Self::new(block, InsertBlockErrorKind::Execution(error))
}
/// Create a new InsertBlockError from a RethError and block.
pub fn from_reth_error(error: RethError, block: SealedBlock) -> Self {
Self::new(block, error.into())
}
/// Consumes the error and returns the block that resulted in the error
#[inline]
pub fn into_block(self) -> SealedBlock {
@ -324,11 +330,9 @@ impl InsertBlockErrorKind {
}
}
// This is a convenience impl to convert from crate::Error to InsertBlockErrorKind, most
impl From<crate::RethError> for InsertBlockErrorKind {
fn from(err: crate::RethError) -> Self {
use crate::RethError;
// This is a convenience impl to convert from crate::Error to InsertBlockErrorKind
impl From<RethError> for InsertBlockErrorKind {
fn from(err: RethError) -> Self {
match err {
RethError::Execution(err) => InsertBlockErrorKind::Execution(err),
RethError::Consensus(err) => InsertBlockErrorKind::Consensus(err),