diff --git a/crates/engine/tree/Cargo.toml b/crates/engine/tree/Cargo.toml index e7f3c46911..9ffb3718dc 100644 --- a/crates/engine/tree/Cargo.toml +++ b/crates/engine/tree/Cargo.toml @@ -115,6 +115,10 @@ harness = false name = "state_root_task" harness = false +[[bench]] +name = "state_provider_builder" +harness = false + [features] test-utils = [ "reth-chain-state/test-utils", diff --git a/crates/engine/tree/benches/state_provider_builder.rs b/crates/engine/tree/benches/state_provider_builder.rs new file mode 100644 index 0000000000..8472981c86 --- /dev/null +++ b/crates/engine/tree/benches/state_provider_builder.rs @@ -0,0 +1,49 @@ +//! Benchmark for state provider builder reuse. + +#![allow(missing_docs)] + +use alloy_consensus::Header; +use alloy_primitives::B256; +use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion}; +use reth_engine_tree::tree::StateProviderBuilder; +use reth_ethereum_primitives::EthPrimitives; +use reth_provider::{test_utils::MockEthProvider, HeaderProvider}; +use std::hint::black_box; + +fn build_builder( + provider: &MockEthProvider, + hash: B256, +) -> StateProviderBuilder> { + let header = provider.header(hash).expect("header lookup failed"); + assert!(header.is_some(), "missing header for hash"); + StateProviderBuilder::new(provider.clone(), hash, None) +} + +fn bench_state_provider_builder(c: &mut Criterion) { + let provider = MockEthProvider::::new(); + let hash = B256::from([0x11u8; 32]); + let header = Header { number: 1, ..Header::default() }; + provider.add_header(hash, header); + + let mut group = c.benchmark_group("state_provider_builder_reuse"); + + group.bench_with_input(BenchmarkId::new("single_lookup", 1), &hash, |b, hash| { + b.iter(|| { + let builder = build_builder(&provider, *hash); + black_box(builder); + }); + }); + + group.bench_with_input(BenchmarkId::new("double_lookup", 2), &hash, |b, hash| { + b.iter(|| { + let first = build_builder(&provider, *hash); + let second = build_builder(&provider, *hash); + black_box((first, second)); + }); + }); + + group.finish(); +} + +criterion_group!(benches, bench_state_provider_builder); +criterion_main!(benches); diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index d89dc4a6b9..8472e5c0bb 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -321,7 +321,7 @@ where BlockReader, C: ConfigureEvm + 'static, T: PayloadTypes>, - V: EngineValidator, + V: EngineValidator, { /// Creates a new [`EngineApiTreeHandler`]. #[expect(clippy::too_many_arguments)] @@ -2437,7 +2437,9 @@ where self.insert_block_or_payload( payload.block_with_parent(), payload, - |validator, payload, ctx| validator.validate_payload(payload, ctx), + |validator, payload, ctx, provider_builder| { + validator.validate_payload(payload, ctx, provider_builder) + }, |this, payload| Ok(this.payload_validator.convert_payload_to_block(payload)?), ) } @@ -2449,7 +2451,9 @@ where self.insert_block_or_payload( block.block_with_parent(), block, - |validator, block, ctx| validator.validate_block(block, ctx), + |validator, block, ctx, provider_builder| { + validator.validate_block(block, ctx, provider_builder) + }, |_, block| Ok(block), ) } @@ -2475,7 +2479,12 @@ where &mut self, block_id: BlockWithParent, input: Input, - execute: impl FnOnce(&mut V, Input, TreeCtx<'_, N>) -> Result, Err>, + execute: impl FnOnce( + &mut V, + Input, + TreeCtx<'_, N>, + V::ProviderBuilder, + ) -> Result, Err>, convert_to_block: impl FnOnce(&mut Self, Input) -> Result, Err>, ) -> Result where @@ -2500,7 +2509,7 @@ where }; // Ensure that the parent state is available. - match self.state_provider_builder(block_id.parent) { + let provider_builder = match self.payload_validator.provider_builder(block_id.parent, &self.state) { Err(err) => { let block = convert_to_block(self, input)?; return Err(InsertBlockError::new(block, err.into()).into()); @@ -2524,8 +2533,8 @@ where missing_ancestor, })) } - Ok(Some(_)) => {} - } + Ok(Some(provider_builder)) => provider_builder, + }; // determine whether we are on a fork chain by comparing the block number with the // canonical head. This is a simple check that is sufficient for the event emission below. @@ -2537,7 +2546,7 @@ where let start = Instant::now(); - let executed = execute(&mut self.payload_validator, input, ctx)?; + let executed = execute(&mut self.payload_validator, input, ctx, provider_builder)?; // if the parent is the canonical head, we can insert the block as the pending block if self.state.tree_state.canonical_block_hash() == executed.recovered_block().parent_hash() diff --git a/crates/engine/tree/src/tree/payload_validator.rs b/crates/engine/tree/src/tree/payload_validator.rs index b64611fb5e..de77505668 100644 --- a/crates/engine/tree/src/tree/payload_validator.rs +++ b/crates/engine/tree/src/tree/payload_validator.rs @@ -323,6 +323,7 @@ where &mut self, input: BlockOrPayload, mut ctx: TreeCtx<'_, N>, + provider_builder: StateProviderBuilder, ) -> ValidationOutcome> where V: PayloadValidator, @@ -362,16 +363,6 @@ where trace!(target: "engine::tree::payload_validator", "Fetching block state provider"); let _enter = debug_span!(target: "engine::tree::payload_validator", "state provider").entered(); - let Some(provider_builder) = - ensure_ok!(self.state_provider_builder(parent_hash, ctx.state())) - else { - // this is pre-validated in the tree - return Err(InsertBlockError::new( - self.convert_to_block(input)?, - ProviderError::HeaderNotFound(parent_hash.into()).into(), - ) - .into()) - }; let mut state_provider = ensure_ok!(provider_builder.build()); drop(_enter); @@ -876,37 +867,6 @@ where } } - /// Creates a `StateProviderBuilder` for the given parent hash. - /// - /// This method checks if the parent is in the tree state (in-memory) or persisted to disk, - /// and creates the appropriate provider builder. - fn state_provider_builder( - &self, - hash: B256, - state: &EngineApiTreeState, - ) -> ProviderResult>> { - if let Some((historical, blocks)) = state.tree_state.blocks_by_hash(hash) { - debug!(target: "engine::tree::payload_validator", %hash, %historical, "found canonical state for block in memory, creating provider builder"); - // the block leads back to the canonical chain - return Ok(Some(StateProviderBuilder::new( - self.provider.clone(), - historical, - Some(blocks), - ))) - } - - // Check if the block is persisted - if let Some(header) = self.provider.header(hash)? { - debug!(target: "engine::tree::payload_validator", %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))) - } - - debug!(target: "engine::tree::payload_validator", %hash, "no canonical state found for block"); - Ok(None) - } - /// Determines the state root computation strategy based on configuration. /// /// Note: Use state root task only if prefix sets are empty, otherwise proof generation is @@ -1129,6 +1089,19 @@ pub trait EngineValidator< N: NodePrimitives = <::BuiltPayload as BuiltPayload>::Primitives, >: Send + Sync + 'static { + /// State provider builder type used by the validator. + type ProviderBuilder; + + /// Creates a `StateProviderBuilder` for the given parent hash. + /// + /// This method checks if the parent is in the tree state (in-memory) or persisted to disk, + /// and creates the appropriate provider builder. + fn provider_builder( + &self, + hash: B256, + state: &EngineApiTreeState, + ) -> ProviderResult>; + /// Validates the payload attributes with respect to the header. /// /// By default, this enforces that the payload attributes timestamp is greater than the @@ -1162,6 +1135,7 @@ pub trait EngineValidator< &mut self, payload: Types::ExecutionData, ctx: TreeCtx<'_, N>, + provider_builder: Self::ProviderBuilder, ) -> ValidationOutcome; /// Validates a block downloaded from the network. @@ -1169,6 +1143,7 @@ pub trait EngineValidator< &mut self, block: SealedBlock, ctx: TreeCtx<'_, N>, + provider_builder: Self::ProviderBuilder, ) -> ValidationOutcome; /// Hook called after an executed block is inserted directly into the tree. @@ -1193,6 +1168,35 @@ where Evm: ConfigureEngineEvm + 'static, Types: PayloadTypes>, { + type ProviderBuilder = StateProviderBuilder; + + fn provider_builder( + &self, + hash: B256, + state: &EngineApiTreeState, + ) -> ProviderResult> { + if let Some((historical, blocks)) = state.tree_state.blocks_by_hash(hash) { + debug!(target: "engine::tree::payload_validator", %hash, %historical, "found canonical state for block in memory, creating provider builder"); + // the block leads back to the canonical chain + return Ok(Some(StateProviderBuilder::new( + self.provider.clone(), + historical, + Some(blocks), + ))) + } + + // Check if the block is persisted + if let Some(header) = self.provider.header(hash)? { + debug!(target: "engine::tree::payload_validator", %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))) + } + + debug!(target: "engine::tree::payload_validator", %hash, "no canonical state found for block"); + Ok(None) + } + fn validate_payload_attributes_against_header( &self, attr: &Types::PayloadAttributes, @@ -1213,16 +1217,18 @@ where &mut self, payload: Types::ExecutionData, ctx: TreeCtx<'_, N>, + provider_builder: Self::ProviderBuilder, ) -> ValidationOutcome { - self.validate_block_with_state(BlockOrPayload::Payload(payload), ctx) + self.validate_block_with_state(BlockOrPayload::Payload(payload), ctx, provider_builder) } fn validate_block( &mut self, block: SealedBlock, ctx: TreeCtx<'_, N>, + provider_builder: Self::ProviderBuilder, ) -> ValidationOutcome { - self.validate_block_with_state(BlockOrPayload::Block(block), ctx) + self.validate_block_with_state(BlockOrPayload::Block(block), ctx, provider_builder) } fn on_inserted_executed_block(&self, block: ExecutedBlock) { diff --git a/crates/engine/tree/src/tree/tests.rs b/crates/engine/tree/src/tree/tests.rs index b1725bc36c..7184332d4b 100644 --- a/crates/engine/tree/src/tree/tests.rs +++ b/crates/engine/tree/src/tree/tests.rs @@ -428,7 +428,13 @@ impl ValidatorTestHarness { &mut self.harness.tree.state, &self.harness.tree.canonical_in_memory_state, ); - let result = self.validator.validate_block(block, ctx); + let provider_builder = self + .harness + .tree + .state_provider_builder(block.parent_hash()) + .expect("state provider builder lookup failed") + .expect("missing parent state provider builder"); + let result = self.validator.validate_block(block, ctx, provider_builder); self.metrics.record_validation(result.is_ok()); result }