From 7acd0aff03666cd13bf5141ef9fe691f24246a04 Mon Sep 17 00:00:00 2001 From: Timosdev99 Date: Sat, 6 Sep 2025 02:29:02 +0000 Subject: [PATCH 01/17] feat(observability): add phase level observablity to new payload processing --- crates/engine/tree/src/tree/metrics.rs | 26 +++ crates/engine/tree/src/tree/mod.rs | 184 ++++++++++-------- .../engine/tree/src/tree/payload_validator.rs | 111 ++++++++--- 3 files changed, 214 insertions(+), 107 deletions(-) diff --git a/crates/engine/tree/src/tree/metrics.rs b/crates/engine/tree/src/tree/metrics.rs index 60be5c4e044..0bda3401a49 100644 --- a/crates/engine/tree/src/tree/metrics.rs +++ b/crates/engine/tree/src/tree/metrics.rs @@ -26,6 +26,8 @@ pub(crate) struct EngineApiMetrics { pub(crate) executor: ExecutorMetrics, /// Metrics for block validation pub(crate) block_validation: BlockValidationMetrics, + /// Metrics for newPayload processing phases + pub(crate) new_payload_phases: NewPayloadPhaseMetrics, /// A copy of legacy blockchain tree metrics, to be replaced when we replace the old tree pub tree: TreeMetrics, } @@ -192,6 +194,30 @@ impl BlockValidationMetrics { } } +/// Metrics for newPayload processing phases +#[derive(Metrics)] +#[metrics(scope = "engine.new_payload")] +pub(crate) struct NewPayloadPhaseMetrics { + /// Pre-execution phase duration (entry → execution start) + pub(crate) pre_execution_duration: Histogram, + /// Prewarming initialization duration + pub(crate) prewarming_init_duration: Histogram, + /// Block execution duration + pub(crate) execution_duration: Histogram, + /// Post-execution validation duration + pub(crate) post_execution_validation_duration: Histogram, + /// State root computation duration + pub(crate) state_root_duration: Histogram, + /// Post-processing duration + pub(crate) post_processing_duration: Histogram, + /// Parallel state root algorithm duration + pub(crate) parallel_state_root_duration: Histogram, + /// Sparse trie state root algorithm duration + pub(crate) sparse_trie_state_root_duration: Histogram, + /// Regular state root algorithm duration + pub(crate) regular_state_root_duration: Histogram, +} + /// Metrics for the blockchain tree block buffer #[derive(Metrics)] #[metrics(scope = "blockchain_tree.block_buffer")] diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index aad6d6e2742..09e54f0db72 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -432,7 +432,7 @@ where debug!(target: "engine::tree", %msg, "received new engine message"); if let Err(fatal) = self.on_engine_message(msg) { error!(target: "engine::tree", %fatal, "insert block fatal error"); - return + return; } } Ok(None) => { @@ -440,13 +440,13 @@ where } Err(_err) => { error!(target: "engine::tree", "Engine channel disconnected"); - return + return; } } if let Err(err) = self.advance_persistence() { error!(target: "engine::tree", %err, "Advancing persistence failed"); - return + return; } } } @@ -462,7 +462,7 @@ where ) -> Result, InsertBlockFatalError> { if blocks.is_empty() { // nothing to execute - return Ok(None) + return Ok(None); } trace!(target: "engine::tree", block_count = %blocks.len(), "received downloaded blocks"); @@ -473,7 +473,7 @@ where self.on_tree_event(event)?; if needs_backfill { // can exit early if backfill is needed - return Ok(None) + return Ok(None); } } } @@ -508,7 +508,11 @@ where trace!(target: "engine::tree", "invoked new payload"); self.metrics.engine.new_payload_messages.increment(1); - let validation_start = Instant::now(); + // start timing for the entire new payload process + let new_payload_start = Instant::now(); + + // Phase A: Pre-execution (entry → execution start) + let pre_execution_start = Instant::now(); // Ensures that the given payload does not violate any consensus rules that concern the // block's layout, like: @@ -537,9 +541,15 @@ where // This validation **MUST** be instantly run in all cases even during active sync process. let parent_hash = payload.parent_hash(); + // record pre-execution phase duration + self.metrics + .new_payload_phases + .pre_execution_duration + .record(pre_execution_start.elapsed().as_secs_f64()); + self.metrics .block_validation - .record_payload_validation(validation_start.elapsed().as_secs_f64()); + .record_payload_validation(pre_execution_start.elapsed().as_secs_f64()); let num_hash = payload.num_hash(); let engine_event = ConsensusEngineEvent::BlockReceived(num_hash); @@ -562,12 +572,12 @@ where Ok(block) => block, Err(error) => { let status = self.on_new_payload_error(error, parent_hash)?; - return Ok(TreeOutcome::new(status)) + return Ok(TreeOutcome::new(status)); } }; let status = self.on_invalid_new_payload(block.into_sealed_block(), invalid)?; - return Ok(TreeOutcome::new(status)) + return Ok(TreeOutcome::new(status)); } let status = if self.backfill_sync_state.is_idle() { @@ -584,8 +594,8 @@ where latest_valid_hash = Some(block_hash); PayloadStatusEnum::Valid } - InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) | - InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { + InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) + | InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { // not known to be invalid, but we don't know anything else PayloadStatusEnum::Syncing } @@ -614,6 +624,8 @@ where } }; + // Phase F: Post-processing + let post_processing_start = Instant::now(); let mut outcome = TreeOutcome::new(status); // if the block is valid and it is the current sync target head, make it canonical if outcome.outcome.is_valid() && self.is_sync_target_head(block_hash) { @@ -625,6 +637,18 @@ where } } + // record post-processing duration + self.metrics + .new_payload_phases + .post_processing_duration + .record(post_processing_start.elapsed().as_secs_f64()); + + // record total newPayload duration + self.metrics + .new_payload_phases + .pre_execution_duration // will be using this as a proxy for total duration + .record(new_payload_start.elapsed().as_secs_f64()); + Ok(outcome) } @@ -639,7 +663,7 @@ where let Some(new_head_block) = self.state.tree_state.blocks_by_hash.get(&new_head) else { debug!(target: "engine::tree", new_head=?new_head, "New head block not found in inmemory tree state"); self.metrics.engine.executed_new_block_cache_miss.increment(1); - return Ok(None) + return Ok(None); }; let new_head_number = new_head_block.recovered_block().number(); @@ -932,11 +956,11 @@ where let mut current_block = target; loop { if current_block.block.hash == canonical_head.hash { - return Ok(false) + return Ok(false); } // We already passed the canonical head if current_block.block.number <= canonical_head.number { - break + break; } current_hash = current_block.parent; @@ -946,12 +970,12 @@ where // verify that the given hash is not already part of canonical chain stored in memory if self.canonical_in_memory_state.header_by_hash(target_hash).is_some() { - return Ok(false) + return Ok(false); } // verify that the given hash is not already part of persisted canonical chain if self.provider.block_number(target_hash)?.is_some() { - return Ok(false) + return Ok(false); } Ok(true) @@ -961,19 +985,19 @@ where fn persisting_kind_for(&self, block: BlockWithParent) -> PersistingKind { // Check that we're currently persisting. let Some(action) = self.persistence_state.current_action() else { - return PersistingKind::NotPersisting + return PersistingKind::NotPersisting; }; // Check that the persistince action is saving blocks, not removing them. let CurrentPersistenceAction::SavingBlocks { highest } = action else { - return PersistingKind::PersistingNotDescendant + return PersistingKind::PersistingNotDescendant; }; // The block being validated can only be a descendant if its number is higher than // the highest block persisting. Otherwise, it's likely a fork of a lower block. - if block.block.number > highest.number && - self.state.tree_state.is_descendant(*highest, block) + if block.block.number > highest.number + && self.state.tree_state.is_descendant(*highest, block) { - return PersistingKind::PersistingDescendant + return PersistingKind::PersistingDescendant; } // In all other cases, the block is not a descendant. @@ -1003,7 +1027,7 @@ where self.canonical_in_memory_state.on_forkchoice_update_received(); if let Some(on_updated) = self.pre_validate_forkchoice_update(state)? { - return Ok(TreeOutcome::new(on_updated)) + return Ok(TreeOutcome::new(on_updated)); } let valid_outcome = |head| { @@ -1034,7 +1058,7 @@ where // update the safe and finalized blocks and ensure their values are valid if let Err(outcome) = self.ensure_consistent_forkchoice_state(state) { // safe or finalized hashes are invalid - return Ok(TreeOutcome::new(outcome)) + return Ok(TreeOutcome::new(outcome)); } // we still need to process payload attributes if the head is already canonical @@ -1047,11 +1071,11 @@ where ProviderError::HeaderNotFound(state.head_block_hash.into()) })?; let updated = self.process_payload_attributes(attr, &tip, state, version); - return Ok(TreeOutcome::new(updated)) + return Ok(TreeOutcome::new(updated)); } // the head block is already canonical - return Ok(valid_outcome(state.head_block_hash)) + return Ok(valid_outcome(state.head_block_hash)); } // 2. check if the head is already part of the canonical chain @@ -1060,14 +1084,14 @@ where // For OpStack, or if explicitly configured, the proposers are allowed to reorg their // own chain at will, so we need to always trigger a new payload job if requested. - if self.engine_kind.is_opstack() || - self.config.always_process_payload_attributes_on_canonical_head() + if self.engine_kind.is_opstack() + || self.config.always_process_payload_attributes_on_canonical_head() { if let Some(attr) = attrs { debug!(target: "engine::tree", head = canonical_header.number(), "handling payload attributes for canonical head"); let updated = self.process_payload_attributes(attr, &canonical_header, state, version); - return Ok(TreeOutcome::new(updated)) + return Ok(TreeOutcome::new(updated)); } // At this point, no alternative block has been triggered, so we need effectively @@ -1088,7 +1112,7 @@ where // the head block is already canonical, so we're not triggering a payload job and can // return right away - return Ok(valid_outcome(state.head_block_hash)) + return Ok(valid_outcome(state.head_block_hash)); } // 3. ensure we can apply a new chain update for the head block @@ -1099,15 +1123,15 @@ where // update the safe and finalized blocks and ensure their values are valid if let Err(outcome) = self.ensure_consistent_forkchoice_state(state) { // safe or finalized hashes are invalid - return Ok(TreeOutcome::new(outcome)) + return Ok(TreeOutcome::new(outcome)); } if let Some(attr) = attrs { let updated = self.process_payload_attributes(attr, &tip, state, version); - return Ok(TreeOutcome::new(updated)) + return Ok(TreeOutcome::new(updated)); } - return Ok(valid_outcome(state.head_block_hash)) + return Ok(valid_outcome(state.head_block_hash)); } // 4. we don't have the block to perform the update @@ -1179,7 +1203,7 @@ where fn persist_blocks(&mut self, blocks_to_persist: Vec>) { if blocks_to_persist.is_empty() { debug!(target: "engine::tree", "Returned empty set of blocks to persist"); - return + return; } // NOTE: checked non-empty above @@ -1219,7 +1243,7 @@ where // if this happened, then we persisted no blocks because we sent an // empty vec of blocks warn!(target: "engine::tree", "Persistence task completed but did not persist any blocks"); - return Ok(()) + return Ok(()); }; debug!(target: "engine::tree", ?last_persisted_block_hash, ?last_persisted_block_number, "Finished persisting, calling finish"); @@ -1267,7 +1291,7 @@ where let block_num_hash = block.recovered_block().num_hash(); if block_num_hash.number <= self.state.tree_state.canonical_block_number() { // outdated block that can be skipped - return Ok(()) + return Ok(()); } debug!(target: "engine::tree", block=?block_num_hash, "inserting already executed block"); @@ -1275,8 +1299,8 @@ where // if the parent is the canonical head, we can insert the block as the // pending block - if self.state.tree_state.canonical_block_hash() == - block.recovered_block().parent_hash() + if self.state.tree_state.canonical_block_hash() + == block.recovered_block().parent_hash() { debug!(target: "engine::tree", pending=?block_num_hash, "updating pending block"); self.canonical_in_memory_state.set_pending_block(block.clone()); @@ -1407,7 +1431,7 @@ where .map(|hash| BlockNumHash { hash, number: backfill_height }) else { debug!(target: "engine::tree", ?ctrl, "Backfill block not found"); - return Ok(()) + return Ok(()); }; if ctrl.is_unwind() { @@ -1445,11 +1469,11 @@ where // the backfill height let Some(sync_target_state) = self.state.forkchoice_state_tracker.sync_target_state() else { - return Ok(()) + return Ok(()); }; if sync_target_state.finalized_block_hash.is_zero() { // no finalized block, can't check distance - return Ok(()) + return Ok(()); } // get the block number of the finalized block, if we have it let newest_finalized = self @@ -1474,7 +1498,7 @@ where self.emit_event(EngineApiEvent::BackfillAction(BackfillAction::Start( backfill_target.into(), ))); - return Ok(()) + return Ok(()); }; // try to close the gap by executing buffered blocks that are child blocks of the new head @@ -1537,7 +1561,7 @@ where // backfill sync and persisting data are mutually exclusive, so we can't start // backfill while we're still persisting debug!(target: "engine::tree", "skipping backfill file while persistence task is active"); - return + return; } self.backfill_sync_state = BackfillSyncState::Pending; @@ -1556,12 +1580,12 @@ where pub const fn should_persist(&self) -> bool { if !self.backfill_sync_state.is_idle() { // can't persist if backfill is running - return false + return false; } let min_block = self.persistence_state.last_persisted_block.number; - self.state.tree_state.canonical_block_number().saturating_sub(min_block) > - self.config.persistence_threshold() + self.state.tree_state.canonical_block_number().saturating_sub(min_block) + > self.config.persistence_threshold() } /// Returns a batch of consecutive canonical blocks to persist in the range @@ -1607,7 +1631,7 @@ where // Calculate missing trie updates for block in &mut blocks_to_persist { if block.trie.is_present() { - continue + continue; } debug!( @@ -1661,7 +1685,7 @@ where // state. if let Some(remove_above) = self.find_disk_reorg()? { self.remove_blocks(remove_above); - return Ok(()) + return Ok(()); } let finalized = self.state.forkchoice_state_tracker.last_valid_finalized(); @@ -1684,7 +1708,7 @@ where trace!(target: "engine::tree", ?hash, "Fetching executed block by hash"); // check memory first if let Some(block) = self.state.tree_state.executed_block_by_hash(hash) { - return Ok(Some(block.block.clone())) + return Ok(Some(block.block.clone())); } let (block, senders) = self @@ -1750,7 +1774,7 @@ where ) -> ProviderResult> { // Check if parent exists in side chain or in canonical chain. if self.sealed_header_by_hash(parent_hash)?.is_some() { - return Ok(Some(parent_hash)) + return Ok(Some(parent_hash)); } // iterate over ancestors in the invalid cache @@ -1764,7 +1788,7 @@ where // If current_header is None, then the current_hash does not have an invalid // ancestor in the cache, check its presence in blockchain tree if current_block.is_none() && self.sealed_header_by_hash(current_hash)?.is_some() { - return Ok(Some(current_hash)) + return Ok(Some(current_hash)); } } Ok(None) @@ -1794,7 +1818,7 @@ where /// See [`ForkchoiceStateTracker::sync_target_state`] fn is_sync_target_head(&self, block_hash: B256) -> bool { if let Some(target) = self.state.forkchoice_state_tracker.sync_target_state() { - return target.head_block_hash == block_hash + return target.head_block_hash == block_hash; } false } @@ -1845,12 +1869,12 @@ where fn validate_block(&self, block: &RecoveredBlock) -> Result<(), ConsensusError> { if let Err(e) = self.consensus.validate_header(block.sealed_header()) { error!(target: "engine::tree", ?block, "Failed to validate header {}: {e}", block.hash()); - return Err(e) + return Err(e); } if let Err(e) = self.consensus.validate_block_pre_execution(block.sealed_block()) { error!(target: "engine::tree", ?block, "Failed to validate block {}: {e}", block.hash()); - return Err(e) + return Err(e); } Ok(()) @@ -1866,7 +1890,7 @@ where if blocks.is_empty() { // nothing to append - return Ok(()) + return Ok(()); } let now = Instant::now(); @@ -1876,8 +1900,8 @@ where match self.insert_block(child) { Ok(res) => { debug!(target: "engine::tree", child =?child_num_hash, ?res, "connected buffered block"); - if self.is_sync_target_head(child_num_hash.hash) && - matches!(res, InsertPayloadOk::Inserted(BlockStatus::Valid)) + if self.is_sync_target_head(child_num_hash.hash) + && matches!(res, InsertPayloadOk::Inserted(BlockStatus::Valid)) { self.make_canonical(child_num_hash.hash)?; } @@ -1887,7 +1911,7 @@ where debug!(target: "engine::tree", ?err, "failed to connect buffered block to tree"); if let Err(fatal) = self.on_insert_block_error(err) { warn!(target: "engine::tree", %fatal, "fatal error occurred while connecting buffered blocks"); - return Err(fatal) + return Err(fatal); } } } @@ -1904,7 +1928,7 @@ where block: RecoveredBlock, ) -> Result<(), InsertBlockError> { if let Err(err) = self.validate_block(&block) { - return Err(InsertBlockError::consensus_error(err, block.into_sealed_block())) + return Err(InsertBlockError::consensus_error(err, block.into_sealed_block())); } self.state.buffer.insert_block(block); Ok(()) @@ -1980,7 +2004,7 @@ where if !state.finalized_block_hash.is_zero() { // we don't have the block yet and the distance exceeds the allowed // threshold - return Some(state.finalized_block_hash) + return Some(state.finalized_block_hash); } // OPTIMISTIC SYNCING @@ -1996,7 +2020,7 @@ where // However, optimism chains will do this. The risk of a reorg is however // low. debug!(target: "engine::tree", hash=?state.head_block_hash, "Setting head hash as an optimistic backfill target."); - return Some(state.head_block_hash) + return Some(state.head_block_hash); } Ok(Some(_)) => { // we're fully synced to the finalized block @@ -2192,11 +2216,11 @@ where .check_invalid_ancestor_with_head(lowest_buffered_ancestor, block.sealed_block())? .is_some() { - return Ok(None) + return Ok(None); } if !self.backfill_sync_state.is_idle() { - return Ok(None) + return Ok(None); } // try to append the block @@ -2209,7 +2233,7 @@ where // canonical return Ok(Some(TreeEvent::TreeAction(TreeAction::MakeCanonical { sync_target_head: block_num_hash.hash, - }))) + }))); } trace!(target: "engine::tree", "appended downloaded block"); self.try_connect_buffered_blocks(block_num_hash)?; @@ -2221,7 +2245,7 @@ where block_num_hash, missing_ancestor, head, - )) + )); } Ok(InsertPayloadOk::AlreadySeen(_)) => { trace!(target: "engine::tree", "downloaded block already executed"); @@ -2231,7 +2255,7 @@ where debug!(target: "engine::tree", err=%err.kind(), "failed to insert downloaded block"); if let Err(fatal) = self.on_insert_block_error(err) { warn!(target: "engine::tree", %fatal, "fatal error occurred while inserting downloaded block"); - return Err(fatal) + return Err(fatal); } } } @@ -2290,7 +2314,7 @@ where // We now assume that we already have this block in the tree. However, we need to // run the conversion to ensure that the block hash is valid. convert_to_block(self, input)?; - return Ok(InsertPayloadOk::AlreadySeen(BlockStatus::Valid)) + return Ok(InsertPayloadOk::AlreadySeen(BlockStatus::Valid)); } _ => {} }; @@ -2318,7 +2342,7 @@ where return Ok(InsertPayloadOk::Inserted(BlockStatus::Disconnected { head: self.state.tree_state.current_canonical_head, missing_ancestor, - })) + })); } Ok(Some(_)) => {} } @@ -2416,7 +2440,7 @@ where } else { // If the block is higher than the best block number, stop filtering, as it's // the first block that's not in the database. - break + break; } } @@ -2554,18 +2578,18 @@ where finalized_block_hash: B256, ) -> Result<(), OnForkChoiceUpdated> { if finalized_block_hash.is_zero() { - return Ok(()) + return Ok(()); } match self.find_canonical_header(finalized_block_hash) { Ok(None) => { debug!(target: "engine::tree", "Finalized block not found in canonical chain"); // if the finalized block is not known, we can't update the finalized block - return Err(OnForkChoiceUpdated::invalid_state()) + return Err(OnForkChoiceUpdated::invalid_state()); } Ok(Some(finalized)) => { - if Some(finalized.num_hash()) != - self.canonical_in_memory_state.get_finalized_num_hash() + if Some(finalized.num_hash()) + != self.canonical_in_memory_state.get_finalized_num_hash() { // we're also persisting the finalized block on disk so we can reload it on // restart this is required by optimism which queries the finalized block: @@ -2584,14 +2608,14 @@ where /// Updates the tracked safe block if we have it fn update_safe_block(&self, safe_block_hash: B256) -> Result<(), OnForkChoiceUpdated> { if safe_block_hash.is_zero() { - return Ok(()) + return Ok(()); } match self.find_canonical_header(safe_block_hash) { Ok(None) => { debug!(target: "engine::tree", "Safe block not found in canonical chain"); // if the safe block is not known, we can't update the safe block - return Err(OnForkChoiceUpdated::invalid_state()) + return Err(OnForkChoiceUpdated::invalid_state()); } Ok(Some(safe)) => { if Some(safe.num_hash()) != self.canonical_in_memory_state.get_safe_num_hash() { @@ -2645,21 +2669,21 @@ where state: ForkchoiceState, ) -> ProviderResult> { if state.head_block_hash.is_zero() { - return Ok(Some(OnForkChoiceUpdated::invalid_state())) + return Ok(Some(OnForkChoiceUpdated::invalid_state())); } // check if the new head hash is connected to any ancestor that we previously marked as // invalid let lowest_buffered_ancestor_fcu = self.lowest_buffered_ancestor_or(state.head_block_hash); if let Some(status) = self.check_invalid_ancestor(lowest_buffered_ancestor_fcu)? { - return Ok(Some(OnForkChoiceUpdated::with_invalid(status))) + return Ok(Some(OnForkChoiceUpdated::with_invalid(status))); } if !self.backfill_sync_state.is_idle() { // We can only process new forkchoice updates if the pipeline is idle, since it requires // exclusive access to the database trace!(target: "engine::tree", "Pipeline is syncing, skipping forkchoice update"); - return Ok(Some(OnForkChoiceUpdated::syncing())) + return Ok(Some(OnForkChoiceUpdated::syncing())); } Ok(None) @@ -2680,7 +2704,7 @@ where self.payload_validator.validate_payload_attributes_against_header(&attrs, head) { warn!(target: "engine::tree", %err, ?head, "Invalid payload attributes"); - return OnForkChoiceUpdated::invalid_payload_attributes() + return OnForkChoiceUpdated::invalid_payload_attributes(); } // 8. Client software MUST begin a payload build process building on top of @@ -2762,7 +2786,7 @@ where self.provider.clone(), historical, Some(blocks), - ))) + ))); } // Check if the block is persisted @@ -2770,7 +2794,7 @@ where debug!(target: "engine::tree", %hash, number = %header.number(), "found canonical state for block in database, creating provider builder"); // For persisted blocks, we create a builder that will fetch state directly from the // database - return Ok(Some(StateProviderBuilder::new(self.provider.clone(), hash, None))) + return Ok(Some(StateProviderBuilder::new(self.provider.clone(), hash, None))); } debug!(target: "engine::tree", %hash, "no canonical state found for block"); diff --git a/crates/engine/tree/src/tree/payload_validator.rs b/crates/engine/tree/src/tree/payload_validator.rs index 3f66a906f18..9c6312b0534 100644 --- a/crates/engine/tree/src/tree/payload_validator.rs +++ b/crates/engine/tree/src/tree/payload_validator.rs @@ -116,19 +116,19 @@ impl<'a, N: NodePrimitives> TreeCtx<'a, N> { pub fn persisting_kind_for(&self, block: BlockWithParent) -> PersistingKind { // Check that we're currently persisting. let Some(action) = self.persistence().current_action() else { - return PersistingKind::NotPersisting + return PersistingKind::NotPersisting; }; // Check that the persistince action is saving blocks, not removing them. let CurrentPersistenceAction::SavingBlocks { highest } = action else { - return PersistingKind::PersistingNotDescendant + return PersistingKind::PersistingNotDescendant; }; // The block being validated can only be a descendant if its number is higher than // the highest block persisting. Otherwise, it's likely a fork of a lower block. - if block.block.number > highest.number && - self.state().tree_state.is_descendant(*highest, block) + if block.block.number > highest.number + && self.state().tree_state.is_descendant(*highest, block) { - return PersistingKind::PersistingDescendant + return PersistingKind::PersistingDescendant; } // In all other cases, the block is not a descendant. @@ -301,7 +301,9 @@ where Ok(val) => val, Err(e) => { let block = self.convert_to_block(input)?; - return Err(InsertBlockError::new(block.into_sealed_block(), e.into()).into()) + return Err( + InsertBlockError::new(block.into_sealed_block(), e.into()).into() + ); } } }; @@ -319,7 +321,7 @@ where self.convert_to_block(input)?.into_sealed_block(), ProviderError::HeaderNotFound(parent_hash.into()).into(), ) - .into()) + .into()); }; let state_provider = ensure_ok!(provider_builder.build()); @@ -331,7 +333,7 @@ where self.convert_to_block(input)?.into_sealed_block(), ProviderError::HeaderNotFound(parent_hash.into()).into(), ) - .into()) + .into()); }; let evm_env = self.evm_env_for(&input); @@ -361,9 +363,9 @@ where // accounting for the prefix sets. let has_ancestors_with_missing_trie_updates = self.has_ancestors_with_missing_trie_updates(input.block_with_parent(), ctx.state()); - let mut use_state_root_task = run_parallel_state_root && - self.config.use_state_root_task() && - !has_ancestors_with_missing_trie_updates; + let mut use_state_root_task = run_parallel_state_root + && self.config.use_state_root_task() + && !has_ancestors_with_missing_trie_updates; debug!( target: "engine::tree", @@ -403,7 +405,8 @@ where // Use state root task only if prefix sets are empty, otherwise proof generation is too // expensive because it requires walking over the paths in the prefix set in every // proof. - if trie_input.prefix_sets.is_empty() { + let prewarming_start = Instant::now(); + let handle = if trie_input.prefix_sets.is_empty() { self.payload_processor.spawn( env.clone(), txs, @@ -416,9 +419,25 @@ where debug!(target: "engine::tree", block=?block_num_hash, "Disabling state root task due to non-empty prefix sets"); use_state_root_task = false; self.payload_processor.spawn_cache_exclusive(env.clone(), txs, provider_builder) - } + }; + + // record prewarming initialization duration + self.metrics + .new_payload_phases + .prewarming_init_duration + .record(prewarming_start.elapsed().as_secs_f64()); + handle } else { - self.payload_processor.spawn_cache_exclusive(env.clone(), txs, provider_builder) + let prewarming_start = Instant::now(); + let handle = + self.payload_processor.spawn_cache_exclusive(env.clone(), txs, provider_builder); + + // Record prewarming initialization duration + self.metrics + .new_payload_phases + .prewarming_init_duration + .record(prewarming_start.elapsed().as_secs_f64()); + handle }; // Use cached state provider before executing, used in execution after prewarming threads @@ -429,6 +448,8 @@ where handle.cache_metrics(), ); + // Phase C: Block execution + let execution_start = Instant::now(); let output = if self.config.state_provider_metrics() { let state_provider = InstrumentedStateProvider::from_state_provider(&state_provider); let output = ensure_ok!(self.execute_block(&state_provider, env, &input, &mut handle)); @@ -438,6 +459,12 @@ where ensure_ok!(self.execute_block(&state_provider, env, &input, &mut handle)) }; + // record execution duration + self.metrics + .new_payload_phases + .execution_duration + .record(execution_start.elapsed().as_secs_f64()); + // after executing the block we can stop executing transactions handle.stop_prewarming_execution(); @@ -453,6 +480,8 @@ where }; } + // Phase D: Post-execution validation + let post_execution_start = Instant::now(); trace!(target: "engine::tree", block=?block_num_hash, "Validating block consensus"); // validate block consensus rules ensure_ok!(self.validate_block_inner(&block)); @@ -462,13 +491,13 @@ where self.consensus.validate_header_against_parent(block.sealed_header(), &parent_block) { warn!(target: "engine::tree", ?block, "Failed to validate header {} against parent: {e}", block.hash()); - return Err(InsertBlockError::new(block.into_sealed_block(), e.into()).into()) + return Err(InsertBlockError::new(block.into_sealed_block(), e.into()).into()); } if let Err(err) = self.consensus.validate_block_post_execution(&block, &output) { // call post-block hook self.on_invalid_block(&parent_block, &block, &output, None, ctx.state_mut()); - return Err(InsertBlockError::new(block.into_sealed_block(), err.into()).into()) + return Err(InsertBlockError::new(block.into_sealed_block(), err.into()).into()); } let hashed_state = self.provider.hashed_post_state(&output.state); @@ -478,9 +507,15 @@ where { // call post-block hook self.on_invalid_block(&parent_block, &block, &output, None, ctx.state_mut()); - return Err(InsertBlockError::new(block.into_sealed_block(), err.into()).into()) + return Err(InsertBlockError::new(block.into_sealed_block(), err.into()).into()); } + // record post-execution validation duration + self.metrics + .new_payload_phases + .post_execution_validation_duration + .record(post_execution_start.elapsed().as_secs_f64()); + debug!(target: "engine::tree", block=?block_num_hash, "Calculating block state root"); let root_time = Instant::now(); @@ -496,9 +531,14 @@ where Ok(StateRootComputeOutcome { state_root, trie_updates }) => { let elapsed = root_time.elapsed(); info!(target: "engine::tree", ?state_root, ?elapsed, "State root task finished"); + // record sparse trie state root algorithm duration + self.metrics + .new_payload_phases + .sparse_trie_state_root_duration + .record(elapsed.as_secs_f64()); // we double check the state root here for good measure if state_root == block.header().state_root() { - maybe_state_root = Some((state_root, trie_updates, elapsed)) + maybe_state_root = Some((state_root, trie_updates, elapsed)); } else { warn!( target: "engine::tree", @@ -527,7 +567,13 @@ where regular_state_root = ?result.0, "Regular root task finished" ); - maybe_state_root = Some((result.0, result.1, root_time.elapsed())); + let elapsed = root_time.elapsed(); + // record parallel state root algorithm duration + self.metrics + .new_payload_phases + .parallel_state_root_duration + .record(elapsed.as_secs_f64()); + maybe_state_root = Some((result.0, result.1, elapsed)); } Err(ParallelStateRootError::Provider(ProviderError::ConsistentView(error))) => { debug!(target: "engine::tree", %error, "Parallel state root computation failed consistency check, falling back"); @@ -543,10 +589,10 @@ where } } - let (state_root, trie_output, root_elapsed) = if let Some(maybe_state_root) = + let (state_root, trie_output, root_elapsed) = if let Some(ref maybe_state_root) = maybe_state_root { - maybe_state_root + maybe_state_root.clone() } else { // fallback is to compute the state root regularly in sync if self.config.state_root_fallback() { @@ -561,6 +607,17 @@ where (root, updates, root_time.elapsed()) }; + // record state root computation duration (Phase E) + self.metrics.new_payload_phases.state_root_duration.record(root_elapsed.as_secs_f64()); + + // record regular state root algorithm duration if we used the fallback + if maybe_state_root.is_none() { + self.metrics + .new_payload_phases + .regular_state_root_duration + .record(root_elapsed.as_secs_f64()); + } + self.metrics.block_validation.record_state_root(&trie_output, root_elapsed.as_secs_f64()); debug!(target: "engine::tree", ?root_elapsed, block=?block_num_hash, "Calculated state root"); @@ -582,7 +639,7 @@ where ) .into(), ) - .into()) + .into()); } // terminate prewarming task with good state output @@ -627,12 +684,12 @@ where fn validate_block_inner(&self, block: &RecoveredBlock) -> Result<(), ConsensusError> { if let Err(e) = self.consensus.validate_header(block.sealed_header()) { error!(target: "engine::tree", ?block, "Failed to validate header {}: {e}", block.hash()); - return Err(e) + return Err(e); } if let Err(e) = self.consensus.validate_block_pre_execution(block.sealed_block()) { error!(target: "engine::tree", ?block, "Failed to validate block {}: {e}", block.hash()); - return Err(e) + return Err(e); } Ok(()) @@ -766,7 +823,7 @@ where self.provider.clone(), historical, Some(blocks), - ))) + ))); } // Check if the block is persisted @@ -774,7 +831,7 @@ where debug!(target: "engine::tree", %hash, number = %header.number(), "found canonical state for block in database, creating provider builder"); // For persisted blocks, we create a builder that will fetch state directly from the // database - return Ok(Some(StateProviderBuilder::new(self.provider.clone(), hash, None))) + return Ok(Some(StateProviderBuilder::new(self.provider.clone(), hash, None))); } debug!(target: "engine::tree", %hash, "no canonical state found for block"); @@ -843,7 +900,7 @@ where } else { // If the block is higher than the best block number, stop filtering, as it's // the first block that's not in the database. - break + break; } } From 01969882f839210f1f68114ae657585454df152c Mon Sep 17 00:00:00 2001 From: Timosdev99 Date: Sat, 6 Sep 2025 14:45:56 +0000 Subject: [PATCH 02/17] feat(observability): add phase level observablity to new payload processing --build --- crates/engine/tree/src/tree/mod.rs | 28 +++++++++---------- .../engine/tree/src/tree/payload_validator.rs | 10 +++---- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index 09e54f0db72..d875cd711ce 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -594,8 +594,8 @@ where latest_valid_hash = Some(block_hash); PayloadStatusEnum::Valid } - InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) - | InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { + InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) | + InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { // not known to be invalid, but we don't know anything else PayloadStatusEnum::Syncing } @@ -994,8 +994,8 @@ where // The block being validated can only be a descendant if its number is higher than // the highest block persisting. Otherwise, it's likely a fork of a lower block. - if block.block.number > highest.number - && self.state.tree_state.is_descendant(*highest, block) + if block.block.number > highest.number && + self.state.tree_state.is_descendant(*highest, block) { return PersistingKind::PersistingDescendant; } @@ -1084,8 +1084,8 @@ where // For OpStack, or if explicitly configured, the proposers are allowed to reorg their // own chain at will, so we need to always trigger a new payload job if requested. - if self.engine_kind.is_opstack() - || self.config.always_process_payload_attributes_on_canonical_head() + if self.engine_kind.is_opstack() || + self.config.always_process_payload_attributes_on_canonical_head() { if let Some(attr) = attrs { debug!(target: "engine::tree", head = canonical_header.number(), "handling payload attributes for canonical head"); @@ -1299,8 +1299,8 @@ where // if the parent is the canonical head, we can insert the block as the // pending block - if self.state.tree_state.canonical_block_hash() - == block.recovered_block().parent_hash() + if self.state.tree_state.canonical_block_hash() == + block.recovered_block().parent_hash() { debug!(target: "engine::tree", pending=?block_num_hash, "updating pending block"); self.canonical_in_memory_state.set_pending_block(block.clone()); @@ -1584,8 +1584,8 @@ where } let min_block = self.persistence_state.last_persisted_block.number; - self.state.tree_state.canonical_block_number().saturating_sub(min_block) - > self.config.persistence_threshold() + self.state.tree_state.canonical_block_number().saturating_sub(min_block) > + self.config.persistence_threshold() } /// Returns a batch of consecutive canonical blocks to persist in the range @@ -1900,8 +1900,8 @@ where match self.insert_block(child) { Ok(res) => { debug!(target: "engine::tree", child =?child_num_hash, ?res, "connected buffered block"); - if self.is_sync_target_head(child_num_hash.hash) - && matches!(res, InsertPayloadOk::Inserted(BlockStatus::Valid)) + if self.is_sync_target_head(child_num_hash.hash) && + matches!(res, InsertPayloadOk::Inserted(BlockStatus::Valid)) { self.make_canonical(child_num_hash.hash)?; } @@ -2588,8 +2588,8 @@ where return Err(OnForkChoiceUpdated::invalid_state()); } Ok(Some(finalized)) => { - if Some(finalized.num_hash()) - != self.canonical_in_memory_state.get_finalized_num_hash() + if Some(finalized.num_hash()) != + self.canonical_in_memory_state.get_finalized_num_hash() { // we're also persisting the finalized block on disk so we can reload it on // restart this is required by optimism which queries the finalized block: diff --git a/crates/engine/tree/src/tree/payload_validator.rs b/crates/engine/tree/src/tree/payload_validator.rs index 9c6312b0534..40567354ed3 100644 --- a/crates/engine/tree/src/tree/payload_validator.rs +++ b/crates/engine/tree/src/tree/payload_validator.rs @@ -125,8 +125,8 @@ impl<'a, N: NodePrimitives> TreeCtx<'a, N> { // The block being validated can only be a descendant if its number is higher than // the highest block persisting. Otherwise, it's likely a fork of a lower block. - if block.block.number > highest.number - && self.state().tree_state.is_descendant(*highest, block) + if block.block.number > highest.number && + self.state().tree_state.is_descendant(*highest, block) { return PersistingKind::PersistingDescendant; } @@ -363,9 +363,9 @@ where // accounting for the prefix sets. let has_ancestors_with_missing_trie_updates = self.has_ancestors_with_missing_trie_updates(input.block_with_parent(), ctx.state()); - let mut use_state_root_task = run_parallel_state_root - && self.config.use_state_root_task() - && !has_ancestors_with_missing_trie_updates; + let mut use_state_root_task = run_parallel_state_root && + self.config.use_state_root_task() && + !has_ancestors_with_missing_trie_updates; debug!( target: "engine::tree", From 74286a0deab402daab2b3ec599a9cf1395fd49f7 Mon Sep 17 00:00:00 2001 From: Timosdev99 Date: Sun, 7 Sep 2025 11:32:41 +0000 Subject: [PATCH 03/17] feat(observability): add phase level observablity to new payload processing --changes --- crates/engine/tree/src/tree/metrics.rs | 16 +++------- crates/engine/tree/src/tree/mod.rs | 25 ++++----------- .../engine/tree/src/tree/payload_validator.rs | 32 ++----------------- 3 files changed, 13 insertions(+), 60 deletions(-) diff --git a/crates/engine/tree/src/tree/metrics.rs b/crates/engine/tree/src/tree/metrics.rs index 0bda3401a49..96c3e350502 100644 --- a/crates/engine/tree/src/tree/metrics.rs +++ b/crates/engine/tree/src/tree/metrics.rs @@ -200,22 +200,14 @@ impl BlockValidationMetrics { pub(crate) struct NewPayloadPhaseMetrics { /// Pre-execution phase duration (entry → execution start) pub(crate) pre_execution_duration: Histogram, - /// Prewarming initialization duration - pub(crate) prewarming_init_duration: Histogram, - /// Block execution duration - pub(crate) execution_duration: Histogram, + /// Payload processor spawning duration + pub(crate) spawn_payload_processor: Histogram, /// Post-execution validation duration pub(crate) post_execution_validation_duration: Histogram, /// State root computation duration pub(crate) state_root_duration: Histogram, - /// Post-processing duration - pub(crate) post_processing_duration: Histogram, - /// Parallel state root algorithm duration - pub(crate) parallel_state_root_duration: Histogram, - /// Sparse trie state root algorithm duration - pub(crate) sparse_trie_state_root_duration: Histogram, - /// Regular state root algorithm duration - pub(crate) regular_state_root_duration: Histogram, + /// Total duration of the new payload call + pub(crate) total_duration: Histogram, } /// Metrics for the blockchain tree block buffer diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index d875cd711ce..d5bfbee2795 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -541,16 +541,6 @@ where // This validation **MUST** be instantly run in all cases even during active sync process. let parent_hash = payload.parent_hash(); - // record pre-execution phase duration - self.metrics - .new_payload_phases - .pre_execution_duration - .record(pre_execution_start.elapsed().as_secs_f64()); - - self.metrics - .block_validation - .record_payload_validation(pre_execution_start.elapsed().as_secs_f64()); - let num_hash = payload.num_hash(); let engine_event = ConsensusEngineEvent::BlockReceived(num_hash); self.emit_event(EngineApiEvent::BeaconConsensus(engine_event)); @@ -580,6 +570,11 @@ where return Ok(TreeOutcome::new(status)); } + // record pre-execution phase duration + let pre_execution_elapsed = pre_execution_start.elapsed().as_secs_f64(); + self.metrics.new_payload_phases.pre_execution_duration.record(pre_execution_elapsed); + self.metrics.block_validation.record_payload_validation(pre_execution_elapsed); + let status = if self.backfill_sync_state.is_idle() { let mut latest_valid_hash = None; match self.insert_payload(payload) { @@ -624,8 +619,6 @@ where } }; - // Phase F: Post-processing - let post_processing_start = Instant::now(); let mut outcome = TreeOutcome::new(status); // if the block is valid and it is the current sync target head, make it canonical if outcome.outcome.is_valid() && self.is_sync_target_head(block_hash) { @@ -637,16 +630,10 @@ where } } - // record post-processing duration - self.metrics - .new_payload_phases - .post_processing_duration - .record(post_processing_start.elapsed().as_secs_f64()); - // record total newPayload duration self.metrics .new_payload_phases - .pre_execution_duration // will be using this as a proxy for total duration + .total_duration .record(new_payload_start.elapsed().as_secs_f64()); Ok(outcome) diff --git a/crates/engine/tree/src/tree/payload_validator.rs b/crates/engine/tree/src/tree/payload_validator.rs index 40567354ed3..a0eac24cf0a 100644 --- a/crates/engine/tree/src/tree/payload_validator.rs +++ b/crates/engine/tree/src/tree/payload_validator.rs @@ -424,7 +424,7 @@ where // record prewarming initialization duration self.metrics .new_payload_phases - .prewarming_init_duration + .spawn_payload_processor .record(prewarming_start.elapsed().as_secs_f64()); handle } else { @@ -435,7 +435,7 @@ where // Record prewarming initialization duration self.metrics .new_payload_phases - .prewarming_init_duration + .spawn_payload_processor .record(prewarming_start.elapsed().as_secs_f64()); handle }; @@ -448,8 +448,6 @@ where handle.cache_metrics(), ); - // Phase C: Block execution - let execution_start = Instant::now(); let output = if self.config.state_provider_metrics() { let state_provider = InstrumentedStateProvider::from_state_provider(&state_provider); let output = ensure_ok!(self.execute_block(&state_provider, env, &input, &mut handle)); @@ -459,12 +457,6 @@ where ensure_ok!(self.execute_block(&state_provider, env, &input, &mut handle)) }; - // record execution duration - self.metrics - .new_payload_phases - .execution_duration - .record(execution_start.elapsed().as_secs_f64()); - // after executing the block we can stop executing transactions handle.stop_prewarming_execution(); @@ -531,14 +523,9 @@ where Ok(StateRootComputeOutcome { state_root, trie_updates }) => { let elapsed = root_time.elapsed(); info!(target: "engine::tree", ?state_root, ?elapsed, "State root task finished"); - // record sparse trie state root algorithm duration - self.metrics - .new_payload_phases - .sparse_trie_state_root_duration - .record(elapsed.as_secs_f64()); // we double check the state root here for good measure if state_root == block.header().state_root() { - maybe_state_root = Some((state_root, trie_updates, elapsed)); + maybe_state_root = Some((state_root, trie_updates, elapsed)) } else { warn!( target: "engine::tree", @@ -568,11 +555,6 @@ where "Regular root task finished" ); let elapsed = root_time.elapsed(); - // record parallel state root algorithm duration - self.metrics - .new_payload_phases - .parallel_state_root_duration - .record(elapsed.as_secs_f64()); maybe_state_root = Some((result.0, result.1, elapsed)); } Err(ParallelStateRootError::Provider(ProviderError::ConsistentView(error))) => { @@ -610,14 +592,6 @@ where // record state root computation duration (Phase E) self.metrics.new_payload_phases.state_root_duration.record(root_elapsed.as_secs_f64()); - // record regular state root algorithm duration if we used the fallback - if maybe_state_root.is_none() { - self.metrics - .new_payload_phases - .regular_state_root_duration - .record(root_elapsed.as_secs_f64()); - } - self.metrics.block_validation.record_state_root(&trie_output, root_elapsed.as_secs_f64()); debug!(target: "engine::tree", ?root_elapsed, block=?block_num_hash, "Calculated state root"); From d7d4e556b6d2c5930dc2546cd59dce4811d8c4f7 Mon Sep 17 00:00:00 2001 From: Timosdev99 Date: Tue, 9 Sep 2025 09:33:29 +0000 Subject: [PATCH 04/17] changes --- crates/engine/tree/src/tree/metrics.rs | 26 ++++++------------- crates/engine/tree/src/tree/mod.rs | 14 ++++------ .../engine/tree/src/tree/payload_validator.rs | 8 +++--- 3 files changed, 17 insertions(+), 31 deletions(-) diff --git a/crates/engine/tree/src/tree/metrics.rs b/crates/engine/tree/src/tree/metrics.rs index 96c3e350502..d313559778a 100644 --- a/crates/engine/tree/src/tree/metrics.rs +++ b/crates/engine/tree/src/tree/metrics.rs @@ -26,8 +26,6 @@ pub(crate) struct EngineApiMetrics { pub(crate) executor: ExecutorMetrics, /// Metrics for block validation pub(crate) block_validation: BlockValidationMetrics, - /// Metrics for newPayload processing phases - pub(crate) new_payload_phases: NewPayloadPhaseMetrics, /// A copy of legacy blockchain tree metrics, to be replaced when we replace the old tree pub tree: TreeMetrics, } @@ -175,6 +173,14 @@ pub(crate) struct BlockValidationMetrics { pub(crate) payload_validation_duration: Gauge, /// Histogram of payload validation latency pub(crate) payload_validation_histogram: Histogram, + /// Pre-execution phase duration (entry → execution start) + pub(crate) pre_execution_duration: Histogram, + /// Payload processor spawning duration + pub(crate) spawn_payload_processor: Histogram, + /// Post-execution validation duration + pub(crate) post_execution_validation_duration: Histogram, + /// Total duration of the new payload call + pub(crate) total_duration: Histogram, } impl BlockValidationMetrics { @@ -194,22 +200,6 @@ impl BlockValidationMetrics { } } -/// Metrics for newPayload processing phases -#[derive(Metrics)] -#[metrics(scope = "engine.new_payload")] -pub(crate) struct NewPayloadPhaseMetrics { - /// Pre-execution phase duration (entry → execution start) - pub(crate) pre_execution_duration: Histogram, - /// Payload processor spawning duration - pub(crate) spawn_payload_processor: Histogram, - /// Post-execution validation duration - pub(crate) post_execution_validation_duration: Histogram, - /// State root computation duration - pub(crate) state_root_duration: Histogram, - /// Total duration of the new payload call - pub(crate) total_duration: Histogram, -} - /// Metrics for the blockchain tree block buffer #[derive(Metrics)] #[metrics(scope = "blockchain_tree.block_buffer")] diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index d5bfbee2795..d941d92c0fd 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -508,11 +508,10 @@ where trace!(target: "engine::tree", "invoked new payload"); self.metrics.engine.new_payload_messages.increment(1); - // start timing for the entire new payload process - let new_payload_start = Instant::now(); + // start timing for the new payload process + let start = Instant::now(); // Phase A: Pre-execution (entry → execution start) - let pre_execution_start = Instant::now(); // Ensures that the given payload does not violate any consensus rules that concern the // block's layout, like: @@ -571,8 +570,8 @@ where } // record pre-execution phase duration - let pre_execution_elapsed = pre_execution_start.elapsed().as_secs_f64(); - self.metrics.new_payload_phases.pre_execution_duration.record(pre_execution_elapsed); + let pre_execution_elapsed = start.elapsed().as_secs_f64(); + self.metrics.block_validation.pre_execution_duration.record(pre_execution_elapsed); self.metrics.block_validation.record_payload_validation(pre_execution_elapsed); let status = if self.backfill_sync_state.is_idle() { @@ -631,10 +630,7 @@ where } // record total newPayload duration - self.metrics - .new_payload_phases - .total_duration - .record(new_payload_start.elapsed().as_secs_f64()); + self.metrics.block_validation.total_duration.record(start.elapsed().as_secs_f64()); Ok(outcome) } diff --git a/crates/engine/tree/src/tree/payload_validator.rs b/crates/engine/tree/src/tree/payload_validator.rs index a0eac24cf0a..ea4dd09bb5f 100644 --- a/crates/engine/tree/src/tree/payload_validator.rs +++ b/crates/engine/tree/src/tree/payload_validator.rs @@ -423,7 +423,7 @@ where // record prewarming initialization duration self.metrics - .new_payload_phases + .block_validation .spawn_payload_processor .record(prewarming_start.elapsed().as_secs_f64()); handle @@ -434,7 +434,7 @@ where // Record prewarming initialization duration self.metrics - .new_payload_phases + .block_validation .spawn_payload_processor .record(prewarming_start.elapsed().as_secs_f64()); handle @@ -504,7 +504,7 @@ where // record post-execution validation duration self.metrics - .new_payload_phases + .block_validation .post_execution_validation_duration .record(post_execution_start.elapsed().as_secs_f64()); @@ -590,7 +590,7 @@ where }; // record state root computation duration (Phase E) - self.metrics.new_payload_phases.state_root_duration.record(root_elapsed.as_secs_f64()); + self.metrics.block_validation.state_root_histogram.record(root_elapsed.as_secs_f64()); self.metrics.block_validation.record_state_root(&trie_output, root_elapsed.as_secs_f64()); debug!(target: "engine::tree", ?root_elapsed, block=?block_num_hash, "Calculated state root"); From 8bf14dcd0129018683a39f6e61b7808dfbbad30f Mon Sep 17 00:00:00 2001 From: YK Date: Wed, 10 Sep 2025 12:37:00 +0800 Subject: [PATCH 05/17] Apply suggestion from @Rjected Co-authored-by: Dan Cline <6798349+Rjected@users.noreply.github.com> --- crates/engine/tree/src/tree/payload_validator.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/engine/tree/src/tree/payload_validator.rs b/crates/engine/tree/src/tree/payload_validator.rs index ea4dd09bb5f..7aef0b89a1a 100644 --- a/crates/engine/tree/src/tree/payload_validator.rs +++ b/crates/engine/tree/src/tree/payload_validator.rs @@ -472,7 +472,6 @@ where }; } - // Phase D: Post-execution validation let post_execution_start = Instant::now(); trace!(target: "engine::tree", block=?block_num_hash, "Validating block consensus"); // validate block consensus rules From cc79b5586796d3ffc01893388bfdf04c4de0319e Mon Sep 17 00:00:00 2001 From: Timosdev99 Date: Wed, 10 Sep 2025 16:57:34 +0000 Subject: [PATCH 06/17] nits --- crates/engine/tree/src/tree/metrics.rs | 16 ---------------- crates/engine/tree/src/tree/mod.rs | 7 ------- crates/engine/tree/src/tree/payload_validator.rs | 14 +++++--------- 3 files changed, 5 insertions(+), 32 deletions(-) diff --git a/crates/engine/tree/src/tree/metrics.rs b/crates/engine/tree/src/tree/metrics.rs index d313559778a..86366e6e62f 100644 --- a/crates/engine/tree/src/tree/metrics.rs +++ b/crates/engine/tree/src/tree/metrics.rs @@ -163,18 +163,10 @@ pub(crate) struct BlockValidationMetrics { pub(crate) state_root_storage_tries_updated_total: Counter, /// Total number of times the parallel state root computation fell back to regular. pub(crate) state_root_parallel_fallback_total: Counter, - /// Histogram of state root duration, ie the time spent blocked waiting for the state root. - pub(crate) state_root_histogram: Histogram, /// Latest state root duration, ie the time spent blocked waiting for the state root. pub(crate) state_root_duration: Gauge, /// Trie input computation duration pub(crate) trie_input_duration: Histogram, - /// Payload conversion and validation latency - pub(crate) payload_validation_duration: Gauge, - /// Histogram of payload validation latency - pub(crate) payload_validation_histogram: Histogram, - /// Pre-execution phase duration (entry → execution start) - pub(crate) pre_execution_duration: Histogram, /// Payload processor spawning duration pub(crate) spawn_payload_processor: Histogram, /// Post-execution validation duration @@ -189,14 +181,6 @@ impl BlockValidationMetrics { self.state_root_storage_tries_updated_total .increment(trie_output.storage_tries_ref().len() as u64); self.state_root_duration.set(elapsed_as_secs); - self.state_root_histogram.record(elapsed_as_secs); - } - - /// Records a new payload validation time, updating both the histogram and the payload - /// validation gauge - pub(crate) fn record_payload_validation(&self, elapsed_as_secs: f64) { - self.payload_validation_duration.set(elapsed_as_secs); - self.payload_validation_histogram.record(elapsed_as_secs); } } diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index d941d92c0fd..eeb4a69e33a 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -511,8 +511,6 @@ where // start timing for the new payload process let start = Instant::now(); - // Phase A: Pre-execution (entry → execution start) - // Ensures that the given payload does not violate any consensus rules that concern the // block's layout, like: // - missing or invalid base fee @@ -569,11 +567,6 @@ where return Ok(TreeOutcome::new(status)); } - // record pre-execution phase duration - let pre_execution_elapsed = start.elapsed().as_secs_f64(); - self.metrics.block_validation.pre_execution_duration.record(pre_execution_elapsed); - self.metrics.block_validation.record_payload_validation(pre_execution_elapsed); - let status = if self.backfill_sync_state.is_idle() { let mut latest_valid_hash = None; match self.insert_payload(payload) { diff --git a/crates/engine/tree/src/tree/payload_validator.rs b/crates/engine/tree/src/tree/payload_validator.rs index 7aef0b89a1a..f0a3e9b9657 100644 --- a/crates/engine/tree/src/tree/payload_validator.rs +++ b/crates/engine/tree/src/tree/payload_validator.rs @@ -405,7 +405,7 @@ where // Use state root task only if prefix sets are empty, otherwise proof generation is too // expensive because it requires walking over the paths in the prefix set in every // proof. - let prewarming_start = Instant::now(); + let spawn_payload_processor_start = Instant::now(); let handle = if trie_input.prefix_sets.is_empty() { self.payload_processor.spawn( env.clone(), @@ -425,7 +425,7 @@ where self.metrics .block_validation .spawn_payload_processor - .record(prewarming_start.elapsed().as_secs_f64()); + .record(spawn_payload_processor_start.elapsed().as_secs_f64()); handle } else { let prewarming_start = Instant::now(); @@ -553,8 +553,7 @@ where regular_state_root = ?result.0, "Regular root task finished" ); - let elapsed = root_time.elapsed(); - maybe_state_root = Some((result.0, result.1, elapsed)); + maybe_state_root = Some((result.0, result.1, root_time.elapsed())); } Err(ParallelStateRootError::Provider(ProviderError::ConsistentView(error))) => { debug!(target: "engine::tree", %error, "Parallel state root computation failed consistency check, falling back"); @@ -570,10 +569,10 @@ where } } - let (state_root, trie_output, root_elapsed) = if let Some(ref maybe_state_root) = + let (state_root, trie_output, root_elapsed) = if let Some(maybe_state_root) = maybe_state_root { - maybe_state_root.clone() + maybe_state_root } else { // fallback is to compute the state root regularly in sync if self.config.state_root_fallback() { @@ -588,9 +587,6 @@ where (root, updates, root_time.elapsed()) }; - // record state root computation duration (Phase E) - self.metrics.block_validation.state_root_histogram.record(root_elapsed.as_secs_f64()); - self.metrics.block_validation.record_state_root(&trie_output, root_elapsed.as_secs_f64()); debug!(target: "engine::tree", ?root_elapsed, block=?block_num_hash, "Calculated state root"); From 17ca5f7e777ac73c6ce3bda719c2fa38715d685f Mon Sep 17 00:00:00 2001 From: Timosdev99 Date: Wed, 10 Sep 2025 17:13:34 +0000 Subject: [PATCH 07/17] nits --- crates/engine/tree/src/tree/metrics.rs | 11 ++++++++++ crates/engine/tree/src/tree/mod.rs | 30 ++++++++++++++------------ 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/crates/engine/tree/src/tree/metrics.rs b/crates/engine/tree/src/tree/metrics.rs index 86366e6e62f..ea1d8ef80a7 100644 --- a/crates/engine/tree/src/tree/metrics.rs +++ b/crates/engine/tree/src/tree/metrics.rs @@ -167,6 +167,10 @@ pub(crate) struct BlockValidationMetrics { pub(crate) state_root_duration: Gauge, /// Trie input computation duration pub(crate) trie_input_duration: Histogram, + /// Payload conversion and validation latency + pub(crate) payload_validation_duration: Gauge, + /// Histogram of payload validation latency + pub(crate) payload_validation_histogram: Histogram, /// Payload processor spawning duration pub(crate) spawn_payload_processor: Histogram, /// Post-execution validation duration @@ -182,6 +186,13 @@ impl BlockValidationMetrics { .increment(trie_output.storage_tries_ref().len() as u64); self.state_root_duration.set(elapsed_as_secs); } + + /// Records a new payload validation time, updating both the histogram and the payload + /// validation gauge + pub(crate) fn record_payload_validation(&self, elapsed_as_secs: f64) { + self.payload_validation_duration.set(elapsed_as_secs); + self.payload_validation_histogram.record(elapsed_as_secs); + } } /// Metrics for the blockchain tree block buffer diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index eeb4a69e33a..1268e5566f2 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -566,6 +566,8 @@ where let status = self.on_invalid_new_payload(block.into_sealed_block(), invalid)?; return Ok(TreeOutcome::new(status)); } + // record pre-excution phase duration + self.metrics.block_validation.record_payload_validation(start.elapsed().as_secs_f64()); let status = if self.backfill_sync_state.is_idle() { let mut latest_valid_hash = None; @@ -581,8 +583,8 @@ where latest_valid_hash = Some(block_hash); PayloadStatusEnum::Valid } - InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) | - InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { + InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) + | InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { // not known to be invalid, but we don't know anything else PayloadStatusEnum::Syncing } @@ -970,8 +972,8 @@ where // The block being validated can only be a descendant if its number is higher than // the highest block persisting. Otherwise, it's likely a fork of a lower block. - if block.block.number > highest.number && - self.state.tree_state.is_descendant(*highest, block) + if block.block.number > highest.number + && self.state.tree_state.is_descendant(*highest, block) { return PersistingKind::PersistingDescendant; } @@ -1060,8 +1062,8 @@ where // For OpStack, or if explicitly configured, the proposers are allowed to reorg their // own chain at will, so we need to always trigger a new payload job if requested. - if self.engine_kind.is_opstack() || - self.config.always_process_payload_attributes_on_canonical_head() + if self.engine_kind.is_opstack() + || self.config.always_process_payload_attributes_on_canonical_head() { if let Some(attr) = attrs { debug!(target: "engine::tree", head = canonical_header.number(), "handling payload attributes for canonical head"); @@ -1275,8 +1277,8 @@ where // if the parent is the canonical head, we can insert the block as the // pending block - if self.state.tree_state.canonical_block_hash() == - block.recovered_block().parent_hash() + if self.state.tree_state.canonical_block_hash() + == block.recovered_block().parent_hash() { debug!(target: "engine::tree", pending=?block_num_hash, "updating pending block"); self.canonical_in_memory_state.set_pending_block(block.clone()); @@ -1560,8 +1562,8 @@ where } let min_block = self.persistence_state.last_persisted_block.number; - self.state.tree_state.canonical_block_number().saturating_sub(min_block) > - self.config.persistence_threshold() + self.state.tree_state.canonical_block_number().saturating_sub(min_block) + > self.config.persistence_threshold() } /// Returns a batch of consecutive canonical blocks to persist in the range @@ -1876,8 +1878,8 @@ where match self.insert_block(child) { Ok(res) => { debug!(target: "engine::tree", child =?child_num_hash, ?res, "connected buffered block"); - if self.is_sync_target_head(child_num_hash.hash) && - matches!(res, InsertPayloadOk::Inserted(BlockStatus::Valid)) + if self.is_sync_target_head(child_num_hash.hash) + && matches!(res, InsertPayloadOk::Inserted(BlockStatus::Valid)) { self.make_canonical(child_num_hash.hash)?; } @@ -2564,8 +2566,8 @@ where return Err(OnForkChoiceUpdated::invalid_state()); } Ok(Some(finalized)) => { - if Some(finalized.num_hash()) != - self.canonical_in_memory_state.get_finalized_num_hash() + if Some(finalized.num_hash()) + != self.canonical_in_memory_state.get_finalized_num_hash() { // we're also persisting the finalized block on disk so we can reload it on // restart this is required by optimism which queries the finalized block: From d655cf20c377c6d9b256a6e85dbc422779a4553f Mon Sep 17 00:00:00 2001 From: Timosdev99 Date: Thu, 11 Sep 2025 00:20:20 +0000 Subject: [PATCH 08/17] fmt --- crates/engine/tree/src/tree/mod.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index 1268e5566f2..d679374c073 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -583,8 +583,8 @@ where latest_valid_hash = Some(block_hash); PayloadStatusEnum::Valid } - InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) - | InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { + InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) | + InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { // not known to be invalid, but we don't know anything else PayloadStatusEnum::Syncing } @@ -972,8 +972,8 @@ where // The block being validated can only be a descendant if its number is higher than // the highest block persisting. Otherwise, it's likely a fork of a lower block. - if block.block.number > highest.number - && self.state.tree_state.is_descendant(*highest, block) + if block.block.number > highest.number && + self.state.tree_state.is_descendant(*highest, block) { return PersistingKind::PersistingDescendant; } @@ -1062,8 +1062,8 @@ where // For OpStack, or if explicitly configured, the proposers are allowed to reorg their // own chain at will, so we need to always trigger a new payload job if requested. - if self.engine_kind.is_opstack() - || self.config.always_process_payload_attributes_on_canonical_head() + if self.engine_kind.is_opstack() || + self.config.always_process_payload_attributes_on_canonical_head() { if let Some(attr) = attrs { debug!(target: "engine::tree", head = canonical_header.number(), "handling payload attributes for canonical head"); @@ -1277,8 +1277,8 @@ where // if the parent is the canonical head, we can insert the block as the // pending block - if self.state.tree_state.canonical_block_hash() - == block.recovered_block().parent_hash() + if self.state.tree_state.canonical_block_hash() == + block.recovered_block().parent_hash() { debug!(target: "engine::tree", pending=?block_num_hash, "updating pending block"); self.canonical_in_memory_state.set_pending_block(block.clone()); @@ -1562,8 +1562,8 @@ where } let min_block = self.persistence_state.last_persisted_block.number; - self.state.tree_state.canonical_block_number().saturating_sub(min_block) - > self.config.persistence_threshold() + self.state.tree_state.canonical_block_number().saturating_sub(min_block) > + self.config.persistence_threshold() } /// Returns a batch of consecutive canonical blocks to persist in the range @@ -1878,8 +1878,8 @@ where match self.insert_block(child) { Ok(res) => { debug!(target: "engine::tree", child =?child_num_hash, ?res, "connected buffered block"); - if self.is_sync_target_head(child_num_hash.hash) - && matches!(res, InsertPayloadOk::Inserted(BlockStatus::Valid)) + if self.is_sync_target_head(child_num_hash.hash) && + matches!(res, InsertPayloadOk::Inserted(BlockStatus::Valid)) { self.make_canonical(child_num_hash.hash)?; } @@ -2566,8 +2566,8 @@ where return Err(OnForkChoiceUpdated::invalid_state()); } Ok(Some(finalized)) => { - if Some(finalized.num_hash()) - != self.canonical_in_memory_state.get_finalized_num_hash() + if Some(finalized.num_hash()) != + self.canonical_in_memory_state.get_finalized_num_hash() { // we're also persisting the finalized block on disk so we can reload it on // restart this is required by optimism which queries the finalized block: From b8acbc215c8a627dd127ab7ee28173173f3ec574 Mon Sep 17 00:00:00 2001 From: Timosdev99 Date: Thu, 11 Sep 2025 00:30:22 +0000 Subject: [PATCH 09/17] typo --- crates/engine/tree/src/tree/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index d679374c073..d3d5fd756d6 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -566,7 +566,7 @@ where let status = self.on_invalid_new_payload(block.into_sealed_block(), invalid)?; return Ok(TreeOutcome::new(status)); } - // record pre-excution phase duration + // record pre-execution phase duration self.metrics.block_validation.record_payload_validation(start.elapsed().as_secs_f64()); let status = if self.backfill_sync_state.is_idle() { From 629e149c25360661e10db10c2f91686c5db270d6 Mon Sep 17 00:00:00 2001 From: Timosdev99 Date: Thu, 11 Sep 2025 19:40:21 +0000 Subject: [PATCH 10/17] semis --- crates/engine/tree/src/tree/mod.rs | 28 +++++++++---------- .../engine/tree/src/tree/payload_validator.rs | 10 +++---- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index d3d5fd756d6..bd5b6d1a933 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -583,8 +583,8 @@ where latest_valid_hash = Some(block_hash); PayloadStatusEnum::Valid } - InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) | - InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { + InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) + | InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { // not known to be invalid, but we don't know anything else PayloadStatusEnum::Syncing } @@ -972,8 +972,8 @@ where // The block being validated can only be a descendant if its number is higher than // the highest block persisting. Otherwise, it's likely a fork of a lower block. - if block.block.number > highest.number && - self.state.tree_state.is_descendant(*highest, block) + if block.block.number > highest.number + && self.state.tree_state.is_descendant(*highest, block) { return PersistingKind::PersistingDescendant; } @@ -1062,8 +1062,8 @@ where // For OpStack, or if explicitly configured, the proposers are allowed to reorg their // own chain at will, so we need to always trigger a new payload job if requested. - if self.engine_kind.is_opstack() || - self.config.always_process_payload_attributes_on_canonical_head() + if self.engine_kind.is_opstack() + || self.config.always_process_payload_attributes_on_canonical_head() { if let Some(attr) = attrs { debug!(target: "engine::tree", head = canonical_header.number(), "handling payload attributes for canonical head"); @@ -1277,8 +1277,8 @@ where // if the parent is the canonical head, we can insert the block as the // pending block - if self.state.tree_state.canonical_block_hash() == - block.recovered_block().parent_hash() + if self.state.tree_state.canonical_block_hash() + == block.recovered_block().parent_hash() { debug!(target: "engine::tree", pending=?block_num_hash, "updating pending block"); self.canonical_in_memory_state.set_pending_block(block.clone()); @@ -1562,8 +1562,8 @@ where } let min_block = self.persistence_state.last_persisted_block.number; - self.state.tree_state.canonical_block_number().saturating_sub(min_block) > - self.config.persistence_threshold() + self.state.tree_state.canonical_block_number().saturating_sub(min_block) + > self.config.persistence_threshold() } /// Returns a batch of consecutive canonical blocks to persist in the range @@ -1878,8 +1878,8 @@ where match self.insert_block(child) { Ok(res) => { debug!(target: "engine::tree", child =?child_num_hash, ?res, "connected buffered block"); - if self.is_sync_target_head(child_num_hash.hash) && - matches!(res, InsertPayloadOk::Inserted(BlockStatus::Valid)) + if self.is_sync_target_head(child_num_hash.hash) + && matches!(res, InsertPayloadOk::Inserted(BlockStatus::Valid)) { self.make_canonical(child_num_hash.hash)?; } @@ -2566,8 +2566,8 @@ where return Err(OnForkChoiceUpdated::invalid_state()); } Ok(Some(finalized)) => { - if Some(finalized.num_hash()) != - self.canonical_in_memory_state.get_finalized_num_hash() + if Some(finalized.num_hash()) + != self.canonical_in_memory_state.get_finalized_num_hash() { // we're also persisting the finalized block on disk so we can reload it on // restart this is required by optimism which queries the finalized block: diff --git a/crates/engine/tree/src/tree/payload_validator.rs b/crates/engine/tree/src/tree/payload_validator.rs index f0a3e9b9657..5f8f8a40aa1 100644 --- a/crates/engine/tree/src/tree/payload_validator.rs +++ b/crates/engine/tree/src/tree/payload_validator.rs @@ -125,8 +125,8 @@ impl<'a, N: NodePrimitives> TreeCtx<'a, N> { // The block being validated can only be a descendant if its number is higher than // the highest block persisting. Otherwise, it's likely a fork of a lower block. - if block.block.number > highest.number && - self.state().tree_state.is_descendant(*highest, block) + if block.block.number > highest.number + && self.state().tree_state.is_descendant(*highest, block) { return PersistingKind::PersistingDescendant; } @@ -363,9 +363,9 @@ where // accounting for the prefix sets. let has_ancestors_with_missing_trie_updates = self.has_ancestors_with_missing_trie_updates(input.block_with_parent(), ctx.state()); - let mut use_state_root_task = run_parallel_state_root && - self.config.use_state_root_task() && - !has_ancestors_with_missing_trie_updates; + let mut use_state_root_task = run_parallel_state_root + && self.config.use_state_root_task() + && !has_ancestors_with_missing_trie_updates; debug!( target: "engine::tree", From 0de42f08e4538699b90edc9428ee51450bd05731 Mon Sep 17 00:00:00 2001 From: Timosdev99 Date: Thu, 11 Sep 2025 20:40:04 +0000 Subject: [PATCH 11/17] semis --- crates/engine/tree/src/tree/mod.rs | 156 +++++++++--------- .../engine/tree/src/tree/payload_validator.rs | 42 ++--- 2 files changed, 99 insertions(+), 99 deletions(-) diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index bd5b6d1a933..27adb083d74 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -432,7 +432,7 @@ where debug!(target: "engine::tree", %msg, "received new engine message"); if let Err(fatal) = self.on_engine_message(msg) { error!(target: "engine::tree", %fatal, "insert block fatal error"); - return; + return } } Ok(None) => { @@ -440,13 +440,13 @@ where } Err(_err) => { error!(target: "engine::tree", "Engine channel disconnected"); - return; + return } } if let Err(err) = self.advance_persistence() { error!(target: "engine::tree", %err, "Advancing persistence failed"); - return; + return } } } @@ -462,7 +462,7 @@ where ) -> Result, InsertBlockFatalError> { if blocks.is_empty() { // nothing to execute - return Ok(None); + return Ok(None) } trace!(target: "engine::tree", block_count = %blocks.len(), "received downloaded blocks"); @@ -473,7 +473,7 @@ where self.on_tree_event(event)?; if needs_backfill { // can exit early if backfill is needed - return Ok(None); + return Ok(None) } } } @@ -559,12 +559,12 @@ where Ok(block) => block, Err(error) => { let status = self.on_new_payload_error(error, parent_hash)?; - return Ok(TreeOutcome::new(status)); + return Ok(TreeOutcome::new(status)) } }; let status = self.on_invalid_new_payload(block.into_sealed_block(), invalid)?; - return Ok(TreeOutcome::new(status)); + return Ok(TreeOutcome::new(status)) } // record pre-execution phase duration self.metrics.block_validation.record_payload_validation(start.elapsed().as_secs_f64()); @@ -583,8 +583,8 @@ where latest_valid_hash = Some(block_hash); PayloadStatusEnum::Valid } - InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) - | InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { + InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) | + InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { // not known to be invalid, but we don't know anything else PayloadStatusEnum::Syncing } @@ -641,7 +641,7 @@ where let Some(new_head_block) = self.state.tree_state.blocks_by_hash.get(&new_head) else { debug!(target: "engine::tree", new_head=?new_head, "New head block not found in inmemory tree state"); self.metrics.engine.executed_new_block_cache_miss.increment(1); - return Ok(None); + return Ok(None) }; let new_head_number = new_head_block.recovered_block().number(); @@ -665,7 +665,7 @@ where warn!(target: "engine::tree", current_hash=?current_hash, "Sidechain block not found in TreeState"); // This should never happen as we're walking back a chain that should connect to // the canonical chain - return Ok(None); + return Ok(None) } } @@ -675,7 +675,7 @@ where new_chain.reverse(); // Simple extension of the current chain - return Ok(Some(NewCanonicalChain::Commit { new: new_chain })); + return Ok(Some(NewCanonicalChain::Commit { new: new_chain })) } // We have a reorg. Walk back both chains to find the fork point. @@ -692,7 +692,7 @@ where } else { // This shouldn't happen as we're walking back the canonical chain warn!(target: "engine::tree", current_hash=?old_hash, "Canonical block not found in TreeState"); - return Ok(None); + return Ok(None) } } @@ -708,7 +708,7 @@ where } else { // This shouldn't happen as we're walking back the canonical chain warn!(target: "engine::tree", current_hash=?old_hash, "Canonical block not found in TreeState"); - return Ok(None); + return Ok(None) } if let Some(block) = self.state.tree_state.executed_block_by_hash(current_hash).cloned() @@ -718,7 +718,7 @@ where } else { // This shouldn't happen as we've already walked this path warn!(target: "engine::tree", invalid_hash=?current_hash, "New chain block not found in TreeState"); - return Ok(None); + return Ok(None) } } new_chain.reverse(); @@ -934,11 +934,11 @@ where let mut current_block = target; loop { if current_block.block.hash == canonical_head.hash { - return Ok(false); + return Ok(false) } // We already passed the canonical head if current_block.block.number <= canonical_head.number { - break; + break } current_hash = current_block.parent; @@ -948,12 +948,12 @@ where // verify that the given hash is not already part of canonical chain stored in memory if self.canonical_in_memory_state.header_by_hash(target_hash).is_some() { - return Ok(false); + return Ok(false) } // verify that the given hash is not already part of persisted canonical chain if self.provider.block_number(target_hash)?.is_some() { - return Ok(false); + return Ok(false) } Ok(true) @@ -963,19 +963,19 @@ where fn persisting_kind_for(&self, block: BlockWithParent) -> PersistingKind { // Check that we're currently persisting. let Some(action) = self.persistence_state.current_action() else { - return PersistingKind::NotPersisting; + return PersistingKind::NotPersisting }; // Check that the persistince action is saving blocks, not removing them. let CurrentPersistenceAction::SavingBlocks { highest } = action else { - return PersistingKind::PersistingNotDescendant; + return PersistingKind::PersistingNotDescendant }; // The block being validated can only be a descendant if its number is higher than // the highest block persisting. Otherwise, it's likely a fork of a lower block. - if block.block.number > highest.number - && self.state.tree_state.is_descendant(*highest, block) + if block.block.number > highest.number && + self.state.tree_state.is_descendant(*highest, block) { - return PersistingKind::PersistingDescendant; + return PersistingKind::PersistingDescendant } // In all other cases, the block is not a descendant. @@ -1005,7 +1005,7 @@ where self.canonical_in_memory_state.on_forkchoice_update_received(); if let Some(on_updated) = self.pre_validate_forkchoice_update(state)? { - return Ok(TreeOutcome::new(on_updated)); + return Ok(TreeOutcome::new(on_updated)) } let valid_outcome = |head| { @@ -1036,7 +1036,7 @@ where // update the safe and finalized blocks and ensure their values are valid if let Err(outcome) = self.ensure_consistent_forkchoice_state(state) { // safe or finalized hashes are invalid - return Ok(TreeOutcome::new(outcome)); + return Ok(TreeOutcome::new(outcome)) } // we still need to process payload attributes if the head is already canonical @@ -1053,7 +1053,7 @@ where } // the head block is already canonical - return Ok(valid_outcome(state.head_block_hash)); + return Ok(valid_outcome(state.head_block_hash)) } // 2. check if the head is already part of the canonical chain @@ -1062,8 +1062,8 @@ where // For OpStack, or if explicitly configured, the proposers are allowed to reorg their // own chain at will, so we need to always trigger a new payload job if requested. - if self.engine_kind.is_opstack() - || self.config.always_process_payload_attributes_on_canonical_head() + if self.engine_kind.is_opstack() || + self.config.always_process_payload_attributes_on_canonical_head() { if let Some(attr) = attrs { debug!(target: "engine::tree", head = canonical_header.number(), "handling payload attributes for canonical head"); @@ -1090,7 +1090,7 @@ where // the head block is already canonical, so we're not triggering a payload job and can // return right away - return Ok(valid_outcome(state.head_block_hash)); + return Ok(valid_outcome(state.head_block_hash)) } // 3. ensure we can apply a new chain update for the head block @@ -1101,15 +1101,15 @@ where // update the safe and finalized blocks and ensure their values are valid if let Err(outcome) = self.ensure_consistent_forkchoice_state(state) { // safe or finalized hashes are invalid - return Ok(TreeOutcome::new(outcome)); + return Ok(TreeOutcome::new(outcome)) } if let Some(attr) = attrs { let updated = self.process_payload_attributes(attr, &tip, state, version); - return Ok(TreeOutcome::new(updated)); + return Ok(TreeOutcome::new(updated)) } - return Ok(valid_outcome(state.head_block_hash)); + return Ok(valid_outcome(state.head_block_hash)) } // 4. we don't have the block to perform the update @@ -1181,7 +1181,7 @@ where fn persist_blocks(&mut self, blocks_to_persist: Vec>) { if blocks_to_persist.is_empty() { debug!(target: "engine::tree", "Returned empty set of blocks to persist"); - return; + return } // NOTE: checked non-empty above @@ -1221,7 +1221,7 @@ where // if this happened, then we persisted no blocks because we sent an // empty vec of blocks warn!(target: "engine::tree", "Persistence task completed but did not persist any blocks"); - return Ok(()); + return Ok(()) }; debug!(target: "engine::tree", ?last_persisted_block_hash, ?last_persisted_block_number, "Finished persisting, calling finish"); @@ -1269,7 +1269,7 @@ where let block_num_hash = block.recovered_block().num_hash(); if block_num_hash.number <= self.state.tree_state.canonical_block_number() { // outdated block that can be skipped - return Ok(()); + return Ok(()) } debug!(target: "engine::tree", block=?block_num_hash, "inserting already executed block"); @@ -1277,8 +1277,8 @@ where // if the parent is the canonical head, we can insert the block as the // pending block - if self.state.tree_state.canonical_block_hash() - == block.recovered_block().parent_hash() + if self.state.tree_state.canonical_block_hash() == + block.recovered_block().parent_hash() { debug!(target: "engine::tree", pending=?block_num_hash, "updating pending block"); self.canonical_in_memory_state.set_pending_block(block.clone()); @@ -1447,11 +1447,11 @@ where // the backfill height let Some(sync_target_state) = self.state.forkchoice_state_tracker.sync_target_state() else { - return Ok(()); + return Ok(()) }; if sync_target_state.finalized_block_hash.is_zero() { // no finalized block, can't check distance - return Ok(()); + return Ok(()) } // get the block number of the finalized block, if we have it let newest_finalized = self @@ -1476,7 +1476,7 @@ where self.emit_event(EngineApiEvent::BackfillAction(BackfillAction::Start( backfill_target.into(), ))); - return Ok(()); + return Ok(()) }; // try to close the gap by executing buffered blocks that are child blocks of the new head @@ -1539,7 +1539,7 @@ where // backfill sync and persisting data are mutually exclusive, so we can't start // backfill while we're still persisting debug!(target: "engine::tree", "skipping backfill file while persistence task is active"); - return; + return } self.backfill_sync_state = BackfillSyncState::Pending; @@ -1558,12 +1558,12 @@ where pub const fn should_persist(&self) -> bool { if !self.backfill_sync_state.is_idle() { // can't persist if backfill is running - return false; + return false } let min_block = self.persistence_state.last_persisted_block.number; - self.state.tree_state.canonical_block_number().saturating_sub(min_block) - > self.config.persistence_threshold() + self.state.tree_state.canonical_block_number().saturating_sub(min_block) > + self.config.persistence_threshold() } /// Returns a batch of consecutive canonical blocks to persist in the range @@ -1609,7 +1609,7 @@ where // Calculate missing trie updates for block in &mut blocks_to_persist { if block.trie.is_present() { - continue; + continue } debug!( @@ -1663,7 +1663,7 @@ where // state. if let Some(remove_above) = self.find_disk_reorg()? { self.remove_blocks(remove_above); - return Ok(()); + return Ok(()) } let finalized = self.state.forkchoice_state_tracker.last_valid_finalized(); @@ -1686,7 +1686,7 @@ where trace!(target: "engine::tree", ?hash, "Fetching executed block by hash"); // check memory first if let Some(block) = self.state.tree_state.executed_block_by_hash(hash) { - return Ok(Some(block.block.clone())); + return Ok(Some(block.block.clone())) } let (block, senders) = self @@ -1766,7 +1766,7 @@ where // If current_header is None, then the current_hash does not have an invalid // ancestor in the cache, check its presence in blockchain tree if current_block.is_none() && self.sealed_header_by_hash(current_hash)?.is_some() { - return Ok(Some(current_hash)); + return Ok(Some(current_hash)) } } Ok(None) @@ -1796,7 +1796,7 @@ where /// See [`ForkchoiceStateTracker::sync_target_state`] fn is_sync_target_head(&self, block_hash: B256) -> bool { if let Some(target) = self.state.forkchoice_state_tracker.sync_target_state() { - return target.head_block_hash == block_hash; + return target.head_block_hash == block_hash } false } @@ -1847,12 +1847,12 @@ where fn validate_block(&self, block: &RecoveredBlock) -> Result<(), ConsensusError> { if let Err(e) = self.consensus.validate_header(block.sealed_header()) { error!(target: "engine::tree", ?block, "Failed to validate header {}: {e}", block.hash()); - return Err(e); + return Err(e) } if let Err(e) = self.consensus.validate_block_pre_execution(block.sealed_block()) { error!(target: "engine::tree", ?block, "Failed to validate block {}: {e}", block.hash()); - return Err(e); + return Err(e) } Ok(()) @@ -1868,7 +1868,7 @@ where if blocks.is_empty() { // nothing to append - return Ok(()); + return Ok(()) } let now = Instant::now(); @@ -1878,8 +1878,8 @@ where match self.insert_block(child) { Ok(res) => { debug!(target: "engine::tree", child =?child_num_hash, ?res, "connected buffered block"); - if self.is_sync_target_head(child_num_hash.hash) - && matches!(res, InsertPayloadOk::Inserted(BlockStatus::Valid)) + if self.is_sync_target_head(child_num_hash.hash) && + matches!(res, InsertPayloadOk::Inserted(BlockStatus::Valid)) { self.make_canonical(child_num_hash.hash)?; } @@ -1889,7 +1889,7 @@ where debug!(target: "engine::tree", ?err, "failed to connect buffered block to tree"); if let Err(fatal) = self.on_insert_block_error(err) { warn!(target: "engine::tree", %fatal, "fatal error occurred while connecting buffered blocks"); - return Err(fatal); + return Err(fatal) } } } @@ -1906,7 +1906,7 @@ where block: RecoveredBlock, ) -> Result<(), InsertBlockError> { if let Err(err) = self.validate_block(&block) { - return Err(InsertBlockError::consensus_error(err, block.into_sealed_block())); + return Err(InsertBlockError::consensus_error(err, block.into_sealed_block())) } self.state.buffer.insert_block(block); Ok(()) @@ -1982,7 +1982,7 @@ where if !state.finalized_block_hash.is_zero() { // we don't have the block yet and the distance exceeds the allowed // threshold - return Some(state.finalized_block_hash); + return Some(state.finalized_block_hash) } // OPTIMISTIC SYNCING @@ -1998,7 +1998,7 @@ where // However, optimism chains will do this. The risk of a reorg is however // low. debug!(target: "engine::tree", hash=?state.head_block_hash, "Setting head hash as an optimistic backfill target."); - return Some(state.head_block_hash); + return Some(state.head_block_hash) } Ok(Some(_)) => { // we're fully synced to the finalized block @@ -2194,11 +2194,11 @@ where .check_invalid_ancestor_with_head(lowest_buffered_ancestor, block.sealed_block())? .is_some() { - return Ok(None); + return Ok(None) } if !self.backfill_sync_state.is_idle() { - return Ok(None); + return Ok(None) } // try to append the block @@ -2211,7 +2211,7 @@ where // canonical return Ok(Some(TreeEvent::TreeAction(TreeAction::MakeCanonical { sync_target_head: block_num_hash.hash, - }))); + }))) } trace!(target: "engine::tree", "appended downloaded block"); self.try_connect_buffered_blocks(block_num_hash)?; @@ -2223,7 +2223,7 @@ where block_num_hash, missing_ancestor, head, - )); + )) } Ok(InsertPayloadOk::AlreadySeen(_)) => { trace!(target: "engine::tree", "downloaded block already executed"); @@ -2233,7 +2233,7 @@ where debug!(target: "engine::tree", err=%err.kind(), "failed to insert downloaded block"); if let Err(fatal) = self.on_insert_block_error(err) { warn!(target: "engine::tree", %fatal, "fatal error occurred while inserting downloaded block"); - return Err(fatal); + return Err(fatal) } } } @@ -2292,7 +2292,7 @@ where // We now assume that we already have this block in the tree. However, we need to // run the conversion to ensure that the block hash is valid. convert_to_block(self, input)?; - return Ok(InsertPayloadOk::AlreadySeen(BlockStatus::Valid)); + return Ok(InsertPayloadOk::AlreadySeen(BlockStatus::Valid)) } _ => {} }; @@ -2320,7 +2320,7 @@ where return Ok(InsertPayloadOk::Inserted(BlockStatus::Disconnected { head: self.state.tree_state.current_canonical_head, missing_ancestor, - })); + })) } Ok(Some(_)) => {} } @@ -2418,7 +2418,7 @@ where } else { // If the block is higher than the best block number, stop filtering, as it's // the first block that's not in the database. - break; + break } } @@ -2556,18 +2556,18 @@ where finalized_block_hash: B256, ) -> Result<(), OnForkChoiceUpdated> { if finalized_block_hash.is_zero() { - return Ok(()); + return Ok(()) } match self.find_canonical_header(finalized_block_hash) { Ok(None) => { debug!(target: "engine::tree", "Finalized block not found in canonical chain"); // if the finalized block is not known, we can't update the finalized block - return Err(OnForkChoiceUpdated::invalid_state()); + return Err(OnForkChoiceUpdated::invalid_state()) } Ok(Some(finalized)) => { - if Some(finalized.num_hash()) - != self.canonical_in_memory_state.get_finalized_num_hash() + if Some(finalized.num_hash()) != + self.canonical_in_memory_state.get_finalized_num_hash() { // we're also persisting the finalized block on disk so we can reload it on // restart this is required by optimism which queries the finalized block: @@ -2586,14 +2586,14 @@ where /// Updates the tracked safe block if we have it fn update_safe_block(&self, safe_block_hash: B256) -> Result<(), OnForkChoiceUpdated> { if safe_block_hash.is_zero() { - return Ok(()); + return Ok(()) } match self.find_canonical_header(safe_block_hash) { Ok(None) => { debug!(target: "engine::tree", "Safe block not found in canonical chain"); // if the safe block is not known, we can't update the safe block - return Err(OnForkChoiceUpdated::invalid_state()); + return Err(OnForkChoiceUpdated::invalid_state()) } Ok(Some(safe)) => { if Some(safe.num_hash()) != self.canonical_in_memory_state.get_safe_num_hash() { @@ -2647,7 +2647,7 @@ where state: ForkchoiceState, ) -> ProviderResult> { if state.head_block_hash.is_zero() { - return Ok(Some(OnForkChoiceUpdated::invalid_state())); + return Ok(Some(OnForkChoiceUpdated::invalid_state())) } // check if the new head hash is connected to any ancestor that we previously marked as @@ -2661,7 +2661,7 @@ where // We can only process new forkchoice updates if the pipeline is idle, since it requires // exclusive access to the database trace!(target: "engine::tree", "Pipeline is syncing, skipping forkchoice update"); - return Ok(Some(OnForkChoiceUpdated::syncing())); + return Ok(Some(OnForkChoiceUpdated::syncing())) } Ok(None) @@ -2682,7 +2682,7 @@ where self.payload_validator.validate_payload_attributes_against_header(&attrs, head) { warn!(target: "engine::tree", %err, ?head, "Invalid payload attributes"); - return OnForkChoiceUpdated::invalid_payload_attributes(); + return OnForkChoiceUpdated::invalid_payload_attributes() } // 8. Client software MUST begin a payload build process building on top of @@ -2764,7 +2764,7 @@ where self.provider.clone(), historical, Some(blocks), - ))); + ))) } // Check if the block is persisted @@ -2772,7 +2772,7 @@ where debug!(target: "engine::tree", %hash, number = %header.number(), "found canonical state for block in database, creating provider builder"); // For persisted blocks, we create a builder that will fetch state directly from the // database - return Ok(Some(StateProviderBuilder::new(self.provider.clone(), hash, None))); + return Ok(Some(StateProviderBuilder::new(self.provider.clone(), hash, None))) } debug!(target: "engine::tree", %hash, "no canonical state found for block"); diff --git a/crates/engine/tree/src/tree/payload_validator.rs b/crates/engine/tree/src/tree/payload_validator.rs index 5f8f8a40aa1..f0bbdcda0a3 100644 --- a/crates/engine/tree/src/tree/payload_validator.rs +++ b/crates/engine/tree/src/tree/payload_validator.rs @@ -116,19 +116,19 @@ impl<'a, N: NodePrimitives> TreeCtx<'a, N> { pub fn persisting_kind_for(&self, block: BlockWithParent) -> PersistingKind { // Check that we're currently persisting. let Some(action) = self.persistence().current_action() else { - return PersistingKind::NotPersisting; + return PersistingKind::NotPersisting }; // Check that the persistince action is saving blocks, not removing them. let CurrentPersistenceAction::SavingBlocks { highest } = action else { - return PersistingKind::PersistingNotDescendant; + return PersistingKind::PersistingNotDescendant }; // The block being validated can only be a descendant if its number is higher than // the highest block persisting. Otherwise, it's likely a fork of a lower block. - if block.block.number > highest.number - && self.state().tree_state.is_descendant(*highest, block) + if block.block.number > highest.number && + self.state().tree_state.is_descendant(*highest, block) { - return PersistingKind::PersistingDescendant; + return PersistingKind::PersistingDescendant } // In all other cases, the block is not a descendant. @@ -303,7 +303,7 @@ where let block = self.convert_to_block(input)?; return Err( InsertBlockError::new(block.into_sealed_block(), e.into()).into() - ); + ) } } }; @@ -321,7 +321,7 @@ where self.convert_to_block(input)?.into_sealed_block(), ProviderError::HeaderNotFound(parent_hash.into()).into(), ) - .into()); + .into()) }; let state_provider = ensure_ok!(provider_builder.build()); @@ -333,7 +333,7 @@ where self.convert_to_block(input)?.into_sealed_block(), ProviderError::HeaderNotFound(parent_hash.into()).into(), ) - .into()); + .into()) }; let evm_env = self.evm_env_for(&input); @@ -363,9 +363,9 @@ where // accounting for the prefix sets. let has_ancestors_with_missing_trie_updates = self.has_ancestors_with_missing_trie_updates(input.block_with_parent(), ctx.state()); - let mut use_state_root_task = run_parallel_state_root - && self.config.use_state_root_task() - && !has_ancestors_with_missing_trie_updates; + let mut use_state_root_task = run_parallel_state_root && + self.config.use_state_root_task() && + !has_ancestors_with_missing_trie_updates; debug!( target: "engine::tree", @@ -482,13 +482,13 @@ where self.consensus.validate_header_against_parent(block.sealed_header(), &parent_block) { warn!(target: "engine::tree", ?block, "Failed to validate header {} against parent: {e}", block.hash()); - return Err(InsertBlockError::new(block.into_sealed_block(), e.into()).into()); + return Err(InsertBlockError::new(block.into_sealed_block(), e.into()).into()) } if let Err(err) = self.consensus.validate_block_post_execution(&block, &output) { // call post-block hook self.on_invalid_block(&parent_block, &block, &output, None, ctx.state_mut()); - return Err(InsertBlockError::new(block.into_sealed_block(), err.into()).into()); + return Err(InsertBlockError::new(block.into_sealed_block(), err.into()).into()) } let hashed_state = self.provider.hashed_post_state(&output.state); @@ -498,7 +498,7 @@ where { // call post-block hook self.on_invalid_block(&parent_block, &block, &output, None, ctx.state_mut()); - return Err(InsertBlockError::new(block.into_sealed_block(), err.into()).into()); + return Err(InsertBlockError::new(block.into_sealed_block(), err.into()).into()) } // record post-execution validation duration @@ -608,7 +608,7 @@ where ) .into(), ) - .into()); + .into()) } // terminate prewarming task with good state output @@ -653,12 +653,12 @@ where fn validate_block_inner(&self, block: &RecoveredBlock) -> Result<(), ConsensusError> { if let Err(e) = self.consensus.validate_header(block.sealed_header()) { error!(target: "engine::tree", ?block, "Failed to validate header {}: {e}", block.hash()); - return Err(e); + return Err(e) } if let Err(e) = self.consensus.validate_block_pre_execution(block.sealed_block()) { error!(target: "engine::tree", ?block, "Failed to validate block {}: {e}", block.hash()); - return Err(e); + return Err(e) } Ok(()) @@ -792,7 +792,7 @@ where self.provider.clone(), historical, Some(blocks), - ))); + ))) } // Check if the block is persisted @@ -800,7 +800,7 @@ where debug!(target: "engine::tree", %hash, number = %header.number(), "found canonical state for block in database, creating provider builder"); // For persisted blocks, we create a builder that will fetch state directly from the // database - return Ok(Some(StateProviderBuilder::new(self.provider.clone(), hash, None))); + return Ok(Some(StateProviderBuilder::new(self.provider.clone(), hash, None))) } debug!(target: "engine::tree", %hash, "no canonical state found for block"); @@ -818,7 +818,7 @@ where ) { if state.invalid_headers.get(&block.hash()).is_some() { // we already marked this block as invalid - return; + return } self.invalid_block_hook.on_invalid_block(parent_header, block, output, trie_updates); } @@ -869,7 +869,7 @@ where } else { // If the block is higher than the best block number, stop filtering, as it's // the first block that's not in the database. - break; + break } } From 9a696e762e8933cc1cce05bdbf1475176096c319 Mon Sep 17 00:00:00 2001 From: YK Date: Fri, 12 Sep 2025 17:28:35 +0800 Subject: [PATCH 12/17] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- crates/engine/tree/src/tree/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index 27adb083d74..6a8164cbbda 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -1752,7 +1752,7 @@ where ) -> ProviderResult> { // Check if parent exists in side chain or in canonical chain. if self.sealed_header_by_hash(parent_hash)?.is_some() { - return Ok(Some(parent_hash)); + return Ok(Some(parent_hash)) } // iterate over ancestors in the invalid cache From dbaef410a149ce92e71eef301f16e5bf3213ddd1 Mon Sep 17 00:00:00 2001 From: YK Date: Fri, 12 Sep 2025 17:29:36 +0800 Subject: [PATCH 13/17] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- crates/engine/tree/src/tree/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index 6a8164cbbda..d4e15b4623b 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -1409,7 +1409,7 @@ where .map(|hash| BlockNumHash { hash, number: backfill_height }) else { debug!(target: "engine::tree", ?ctrl, "Backfill block not found"); - return Ok(()); + return Ok(()) }; if ctrl.is_unwind() { From bc01dc6ce50e0817c1170afa24d9803f0c14d3c6 Mon Sep 17 00:00:00 2001 From: YK Date: Fri, 12 Sep 2025 17:34:35 +0800 Subject: [PATCH 14/17] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- crates/engine/tree/src/tree/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index d4e15b4623b..a79126948e2 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -2654,7 +2654,7 @@ where // invalid let lowest_buffered_ancestor_fcu = self.lowest_buffered_ancestor_or(state.head_block_hash); if let Some(status) = self.check_invalid_ancestor(lowest_buffered_ancestor_fcu)? { - return Ok(Some(OnForkChoiceUpdated::with_invalid(status))); + return Ok(Some(OnForkChoiceUpdated::with_invalid(status))) } if !self.backfill_sync_state.is_idle() { From 5fe3457648349f5bf2ac28a9ffd7b97fac7a6074 Mon Sep 17 00:00:00 2001 From: YK Date: Fri, 12 Sep 2025 18:17:07 +0800 Subject: [PATCH 15/17] Update crates/engine/tree/src/tree/mod.rs remove semis --- crates/engine/tree/src/tree/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index a79126948e2..33fe77533b7 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -1069,7 +1069,7 @@ where debug!(target: "engine::tree", head = canonical_header.number(), "handling payload attributes for canonical head"); let updated = self.process_payload_attributes(attr, &canonical_header, state, version); - return Ok(TreeOutcome::new(updated)); + return Ok(TreeOutcome::new(updated)) } // At this point, no alternative block has been triggered, so we need effectively From 8268516b746d4bc144976d6e5e16290c1b9bb713 Mon Sep 17 00:00:00 2001 From: YK Date: Fri, 12 Sep 2025 18:17:25 +0800 Subject: [PATCH 16/17] Update crates/engine/tree/src/tree/mod.rs --- crates/engine/tree/src/tree/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index 33fe77533b7..5f79d929e59 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -1049,7 +1049,7 @@ where ProviderError::HeaderNotFound(state.head_block_hash.into()) })?; let updated = self.process_payload_attributes(attr, &tip, state, version); - return Ok(TreeOutcome::new(updated)); + return Ok(TreeOutcome::new(updated)) } // the head block is already canonical From 408f97a183554d9640fc12d96b74ab6dd0383d8e Mon Sep 17 00:00:00 2001 From: Timosdev99 Date: Fri, 12 Sep 2025 20:48:23 +0000 Subject: [PATCH 17/17] changes --- crates/engine/tree/src/tree/metrics.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/engine/tree/src/tree/metrics.rs b/crates/engine/tree/src/tree/metrics.rs index ea1d8ef80a7..3a64a259b86 100644 --- a/crates/engine/tree/src/tree/metrics.rs +++ b/crates/engine/tree/src/tree/metrics.rs @@ -165,6 +165,8 @@ pub(crate) struct BlockValidationMetrics { pub(crate) state_root_parallel_fallback_total: Counter, /// Latest state root duration, ie the time spent blocked waiting for the state root. pub(crate) state_root_duration: Gauge, + /// Histogram for state root duration ie the time spent blocked waiting for the state root + pub(crate) state_root_histogram: Histogram, /// Trie input computation duration pub(crate) trie_input_duration: Histogram, /// Payload conversion and validation latency @@ -185,6 +187,7 @@ impl BlockValidationMetrics { self.state_root_storage_tries_updated_total .increment(trie_output.storage_tries_ref().len() as u64); self.state_root_duration.set(elapsed_as_secs); + self.state_root_histogram.record(elapsed_as_secs); } /// Records a new payload validation time, updating both the histogram and the payload