Skip to content

Commit

Permalink
feat: make InvalidBlockHook generic over NodePrimitives (paradigmxyz#…
Browse files Browse the repository at this point in the history
  • Loading branch information
klkvr authored Dec 3, 2024
1 parent 7008ac2 commit 9d5e159
Show file tree
Hide file tree
Showing 21 changed files with 133 additions and 115 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

67 changes: 36 additions & 31 deletions crates/engine/invalid-block-hooks/src/witness.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use alloy_consensus::Header;
use alloy_consensus::BlockHeader;
use alloy_primitives::{keccak256, B256, U256};
use alloy_rpc_types_debug::ExecutionWitness;
use eyre::OptionExt;
Expand All @@ -8,8 +8,8 @@ use reth_engine_primitives::InvalidBlockHook;
use reth_evm::{
state_change::post_block_balance_increments, system_calls::SystemCaller, ConfigureEvm,
};
use reth_primitives::{Receipt, SealedBlockWithSenders, SealedHeader};
use reth_primitives_traits::SignedTransaction;
use reth_primitives::{NodePrimitives, SealedBlockWithSenders, SealedHeader, TransactionSigned};
use reth_primitives_traits::{HeaderTy, SignedTransaction};
use reth_provider::{BlockExecutionOutput, ChainSpecProvider, StateProviderFactory};
use reth_revm::{
database::StateProviderDatabase, db::states::bundle_state::BundleRetention,
Expand Down Expand Up @@ -54,15 +54,18 @@ where
+ Send
+ Sync
+ 'static,
EvmConfig: ConfigureEvm<Header = Header>,
{
fn on_invalid_block(
fn on_invalid_block<N>(
&self,
parent_header: &SealedHeader,
block: &SealedBlockWithSenders,
output: &BlockExecutionOutput<Receipt>,
parent_header: &SealedHeader<N::BlockHeader>,
block: &SealedBlockWithSenders<N::Block>,
output: &BlockExecutionOutput<N::Receipt>,
trie_updates: Option<(&TrieUpdates, B256)>,
) -> eyre::Result<()> {
) -> eyre::Result<()>
where
N: NodePrimitives<SignedTx = TransactionSigned>,
EvmConfig: ConfigureEvm<Header = N::BlockHeader>,
{
// TODO(alexey): unify with `DebugApi::debug_execution_witness`

// Setup database.
Expand All @@ -86,7 +89,7 @@ where
SystemCaller::new(self.evm_config.clone(), self.provider.chain_spec());

// Apply pre-block system contract calls.
system_caller.apply_pre_execution_changes(&block.clone().unseal(), &mut evm)?;
system_caller.apply_pre_execution_changes(&block.clone().unseal().block, &mut evm)?;

// Re-execute all of the transactions in the block to load all touched accounts into
// the cache DB.
Expand All @@ -106,7 +109,7 @@ where
// NOTE: This is not mut because we are not doing the DAO irregular state change here
let balance_increments = post_block_balance_increments(
self.provider.chain_spec().as_ref(),
&block.block.clone().unseal(),
&block.clone().unseal().block,
U256::MAX,
);

Expand Down Expand Up @@ -163,24 +166,24 @@ where
keys: state_preimages,
};
let re_executed_witness_path = self.save_file(
format!("{}_{}.witness.re_executed.json", block.number, block.hash()),
format!("{}_{}.witness.re_executed.json", block.number(), block.hash()),
&response,
)?;
if let Some(healthy_node_client) = &self.healthy_node_client {
// Compare the witness against the healthy node.
let healthy_node_witness = futures::executor::block_on(async move {
DebugApiClient::debug_execution_witness(healthy_node_client, block.number.into())
DebugApiClient::debug_execution_witness(healthy_node_client, block.number().into())
.await
})?;

let healthy_path = self.save_file(
format!("{}_{}.witness.healthy.json", block.number, block.hash()),
format!("{}_{}.witness.healthy.json", block.number(), block.hash()),
&healthy_node_witness,
)?;

// If the witnesses are different, write the diff to the output directory.
if response != healthy_node_witness {
let filename = format!("{}_{}.witness.diff", block.number, block.hash());
let filename = format!("{}_{}.witness.diff", block.number(), block.hash());
let diff_path = self.save_diff(filename, &response, &healthy_node_witness)?;
warn!(
target: "engine::invalid_block_hooks::witness",
Expand Down Expand Up @@ -210,15 +213,15 @@ where

if bundle_state != output.state {
let original_path = self.save_file(
format!("{}_{}.bundle_state.original.json", block.number, block.hash()),
format!("{}_{}.bundle_state.original.json", block.number(), block.hash()),
&output.state,
)?;
let re_executed_path = self.save_file(
format!("{}_{}.bundle_state.re_executed.json", block.number, block.hash()),
format!("{}_{}.bundle_state.re_executed.json", block.number(), block.hash()),
&bundle_state,
)?;

let filename = format!("{}_{}.bundle_state.diff", block.number, block.hash());
let filename = format!("{}_{}.bundle_state.diff", block.number(), block.hash());
let diff_path = self.save_diff(filename, &bundle_state, &output.state)?;

warn!(
Expand All @@ -236,26 +239,27 @@ where
state_provider.state_root_with_updates(hashed_state)?;
if let Some((original_updates, original_root)) = trie_updates {
if re_executed_root != original_root {
let filename = format!("{}_{}.state_root.diff", block.number, block.hash());
let filename = format!("{}_{}.state_root.diff", block.number(), block.hash());
let diff_path = self.save_diff(filename, &re_executed_root, &original_root)?;
warn!(target: "engine::invalid_block_hooks::witness", ?original_root, ?re_executed_root, diff_path = %diff_path.display(), "State root mismatch after re-execution");
}

// If the re-executed state root does not match the _header_ state root, also log that.
if re_executed_root != block.state_root {
let filename = format!("{}_{}.header_state_root.diff", block.number, block.hash());
let diff_path = self.save_diff(filename, &re_executed_root, &block.state_root)?;
warn!(target: "engine::invalid_block_hooks::witness", header_state_root=?block.state_root, ?re_executed_root, diff_path = %diff_path.display(), "Re-executed state root does not match block state root");
if re_executed_root != block.state_root() {
let filename =
format!("{}_{}.header_state_root.diff", block.number(), block.hash());
let diff_path = self.save_diff(filename, &re_executed_root, &block.state_root())?;
warn!(target: "engine::invalid_block_hooks::witness", header_state_root=?block.state_root(), ?re_executed_root, diff_path = %diff_path.display(), "Re-executed state root does not match block state root");
}

if &trie_output != original_updates {
// Trie updates are too big to diff, so we just save the original and re-executed
let original_path = self.save_file(
format!("{}_{}.trie_updates.original.json", block.number, block.hash()),
format!("{}_{}.trie_updates.original.json", block.number(), block.hash()),
original_updates,
)?;
let re_executed_path = self.save_file(
format!("{}_{}.trie_updates.re_executed.json", block.number, block.hash()),
format!("{}_{}.trie_updates.re_executed.json", block.number(), block.hash()),
&trie_output,
)?;
warn!(
Expand Down Expand Up @@ -292,23 +296,24 @@ where
}
}

impl<P, EvmConfig> InvalidBlockHook for InvalidBlockWitnessHook<P, EvmConfig>
impl<P, EvmConfig, N> InvalidBlockHook<N> for InvalidBlockWitnessHook<P, EvmConfig>
where
N: NodePrimitives<SignedTx = TransactionSigned>,
P: StateProviderFactory
+ ChainSpecProvider<ChainSpec: EthChainSpec + EthereumHardforks>
+ Send
+ Sync
+ 'static,
EvmConfig: ConfigureEvm<Header = Header>,
EvmConfig: ConfigureEvm<Header = HeaderTy<N>>,
{
fn on_invalid_block(
&self,
parent_header: &SealedHeader,
block: &SealedBlockWithSenders,
output: &BlockExecutionOutput<Receipt>,
parent_header: &SealedHeader<N::BlockHeader>,
block: &SealedBlockWithSenders<N::Block>,
output: &BlockExecutionOutput<N::Receipt>,
trie_updates: Option<(&TrieUpdates, B256)>,
) {
if let Err(err) = self.on_invalid_block(parent_header, block, output, trie_updates) {
if let Err(err) = self.on_invalid_block::<N>(parent_header, block, output, trie_updates) {
warn!(target: "engine::invalid_block_hooks::witness", %err, "Failed to invoke hook");
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/engine/local/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ where
payload_builder: PayloadBuilderHandle<N::Engine>,
payload_validator: V,
tree_config: TreeConfig,
invalid_block_hook: Box<dyn InvalidBlockHook>,
invalid_block_hook: Box<dyn InvalidBlockHook<N::Primitives>>,
sync_metrics_tx: MetricEventsSender,
to_engine: UnboundedSender<BeaconEngineMessage<N::Engine>>,
from_engine: EngineMessageStream<N::Engine>,
Expand Down
25 changes: 13 additions & 12 deletions crates/engine/primitives/src/invalid_block_hook.rs
Original file line number Diff line number Diff line change
@@ -1,35 +1,36 @@
use alloy_primitives::B256;
use reth_execution_types::BlockExecutionOutput;
use reth_primitives::{Receipt, SealedBlockWithSenders, SealedHeader};
use reth_primitives::{NodePrimitives, SealedBlockWithSenders, SealedHeader};
use reth_trie::updates::TrieUpdates;

/// An invalid block hook.
pub trait InvalidBlockHook: Send + Sync {
pub trait InvalidBlockHook<N: NodePrimitives>: Send + Sync {
/// Invoked when an invalid block is encountered.
fn on_invalid_block(
&self,
parent_header: &SealedHeader,
block: &SealedBlockWithSenders,
output: &BlockExecutionOutput<Receipt>,
parent_header: &SealedHeader<N::BlockHeader>,
block: &SealedBlockWithSenders<N::Block>,
output: &BlockExecutionOutput<N::Receipt>,
trie_updates: Option<(&TrieUpdates, B256)>,
);
}

impl<F> InvalidBlockHook for F
impl<F, N> InvalidBlockHook<N> for F
where
N: NodePrimitives,
F: Fn(
&SealedHeader,
&SealedBlockWithSenders,
&BlockExecutionOutput<Receipt>,
&SealedHeader<N::BlockHeader>,
&SealedBlockWithSenders<N::Block>,
&BlockExecutionOutput<N::Receipt>,
Option<(&TrieUpdates, B256)>,
) + Send
+ Sync,
{
fn on_invalid_block(
&self,
parent_header: &SealedHeader,
block: &SealedBlockWithSenders,
output: &BlockExecutionOutput<Receipt>,
parent_header: &SealedHeader<N::BlockHeader>,
block: &SealedBlockWithSenders<N::Block>,
output: &BlockExecutionOutput<N::Receipt>,
trie_updates: Option<(&TrieUpdates, B256)>,
) {
self(parent_header, block, output, trie_updates)
Expand Down
2 changes: 1 addition & 1 deletion crates/engine/service/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ where
payload_builder: PayloadBuilderHandle<N::Engine>,
payload_validator: V,
tree_config: TreeConfig,
invalid_block_hook: Box<dyn InvalidBlockHook>,
invalid_block_hook: Box<dyn InvalidBlockHook<N::Primitives>>,
sync_metrics_tx: MetricEventsSender,
) -> Self
where
Expand Down
22 changes: 11 additions & 11 deletions crates/engine/tree/src/tree/invalid_block_hook.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use alloy_primitives::B256;
use reth_engine_primitives::InvalidBlockHook;
use reth_primitives::{Receipt, SealedBlockWithSenders, SealedHeader};
use reth_primitives::{NodePrimitives, SealedBlockWithSenders, SealedHeader};
use reth_provider::BlockExecutionOutput;
use reth_trie::updates::TrieUpdates;

Expand All @@ -9,32 +9,32 @@ use reth_trie::updates::TrieUpdates;
#[non_exhaustive]
pub struct NoopInvalidBlockHook;

impl InvalidBlockHook for NoopInvalidBlockHook {
impl<N: NodePrimitives> InvalidBlockHook<N> for NoopInvalidBlockHook {
fn on_invalid_block(
&self,
_parent_header: &SealedHeader,
_block: &SealedBlockWithSenders,
_output: &BlockExecutionOutput<Receipt>,
_parent_header: &SealedHeader<N::BlockHeader>,
_block: &SealedBlockWithSenders<N::Block>,
_output: &BlockExecutionOutput<N::Receipt>,
_trie_updates: Option<(&TrieUpdates, B256)>,
) {
}
}

/// Multiple [`InvalidBlockHook`]s that are executed in order.
pub struct InvalidBlockHooks(pub Vec<Box<dyn InvalidBlockHook>>);
pub struct InvalidBlockHooks<N: NodePrimitives>(pub Vec<Box<dyn InvalidBlockHook<N>>>);

impl std::fmt::Debug for InvalidBlockHooks {
impl<N: NodePrimitives> std::fmt::Debug for InvalidBlockHooks<N> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("InvalidBlockHooks").field("len", &self.0.len()).finish()
}
}

impl InvalidBlockHook for InvalidBlockHooks {
impl<N: NodePrimitives> InvalidBlockHook<N> for InvalidBlockHooks<N> {
fn on_invalid_block(
&self,
parent_header: &SealedHeader,
block: &SealedBlockWithSenders,
output: &BlockExecutionOutput<Receipt>,
parent_header: &SealedHeader<N::BlockHeader>,
block: &SealedBlockWithSenders<N::Block>,
output: &BlockExecutionOutput<N::Receipt>,
trie_updates: Option<(&TrieUpdates, B256)>,
) {
for hook in &self.0 {
Expand Down
9 changes: 6 additions & 3 deletions crates/engine/tree/src/tree/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ pub enum TreeAction {
/// emitting events.
pub struct EngineApiTreeHandler<N, P, E, T, V>
where
N: NodePrimitives,
T: EngineTypes,
{
provider: P,
Expand Down Expand Up @@ -507,7 +508,7 @@ where
/// Metrics for the engine api.
metrics: EngineApiMetrics,
/// An invalid block hook.
invalid_block_hook: Box<dyn InvalidBlockHook>,
invalid_block_hook: Box<dyn InvalidBlockHook<N>>,
/// The engine API variant of this handler
engine_kind: EngineApiKind,
/// Captures the types the engine operates on
Expand All @@ -516,6 +517,8 @@ where

impl<N, P: Debug, E: Debug, T: EngineTypes + Debug, V: Debug> std::fmt::Debug
for EngineApiTreeHandler<N, P, E, T, V>
where
N: NodePrimitives,
{
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("EngineApiTreeHandler")
Expand Down Expand Up @@ -597,7 +600,7 @@ where
}

/// Sets the invalid block hook.
fn set_invalid_block_hook(&mut self, invalid_block_hook: Box<dyn InvalidBlockHook>) {
fn set_invalid_block_hook(&mut self, invalid_block_hook: Box<dyn InvalidBlockHook<N>>) {
self.invalid_block_hook = invalid_block_hook;
}

Expand All @@ -616,7 +619,7 @@ where
payload_builder: PayloadBuilderHandle<T>,
canonical_in_memory_state: CanonicalInMemoryState,
config: TreeConfig,
invalid_block_hook: Box<dyn InvalidBlockHook>,
invalid_block_hook: Box<dyn InvalidBlockHook<N>>,
kind: EngineApiKind,
) -> (Sender<FromEngine<EngineApiRequest<T>>>, UnboundedReceiver<EngineApiEvent>) {
let best_block_number = provider.best_block_number().unwrap_or(0);
Expand Down
4 changes: 2 additions & 2 deletions crates/ethereum/evm/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ where
let env = self.evm_env_for_block(&block.header, total_difficulty);
let mut evm = self.evm_config.evm_with_env(&mut self.state, env);

self.system_caller.apply_pre_execution_changes(block, &mut evm)?;
self.system_caller.apply_pre_execution_changes(&block.block, &mut evm)?;

Ok(())
}
Expand Down Expand Up @@ -247,7 +247,7 @@ where
drop(evm);

let mut balance_increments =
post_block_balance_increments(&self.chain_spec, block, total_difficulty);
post_block_balance_increments(&self.chain_spec, &block.block, total_difficulty);

// Irregular state change at Ethereum DAO hardfork
if self.chain_spec.fork(EthereumHardfork::Dao).transitions_at_block(block.number) {
Expand Down
Loading

0 comments on commit 9d5e159

Please sign in to comment.