feat(engine): compare invalid block witness against a healthy node (#…

…10844)

book/cli/reth/node.md

@@ -541,12 +541,15 @@ Debug:
           The path to store engine API messages at. If specified, all of the intercepted engine API messages will be written to specified location
 
       --debug.invalid-block-hook <INVALID_BLOCK_HOOK>
-          Determines which type of bad block hook to install
+          Determines which type of invalid block hook to install
 
           Example: `witness,prestate`
 
           [possible values: witness, pre-state, opcode]
 
+      --debug.healthy-node-rpc-url <URL>
+          The RPC URL of a healthy node to use for comparing invalid block hook results against.
+
 Database:
       --db.log-level <LOG_LEVEL>
           Database logging level. Levels higher than "notice" require a debug build

crates/engine/invalid-block-hooks/Cargo.toml

@@ -18,14 +18,19 @@ reth-evm.workspace = true
 reth-primitives.workspace = true
 reth-provider.workspace = true
 reth-revm.workspace = true
+reth-rpc-api = { workspace = true, features = ["client"] }
 reth-tracing.workspace = true
 reth-trie = { workspace = true, features = ["serde"] }
 
 # alloy
 alloy-rlp.workspace = true
 alloy-rpc-types-debug.workspace = true
 
+# async
+futures.workspace = true
+
 # misc
 eyre.workspace = true
+jsonrpsee.workspace = true
 pretty_assertions = "1.4"
 serde_json.workspace = true

crates/engine/invalid-block-hooks/src/witness.rs

@@ -17,25 +17,33 @@ use reth_revm::{
     primitives::{BlockEnv, CfgEnvWithHandlerCfg, EnvWithHandlerCfg},
     DatabaseCommit, StateBuilder,
 };
+use reth_rpc_api::DebugApiClient;
 use reth_tracing::tracing::warn;
 use reth_trie::{updates::TrieUpdates, HashedPostState, HashedStorage};
 
 /// Generates a witness for the given block and saves it to a file.
 #[derive(Debug)]
 pub struct InvalidBlockWitnessHook<P, EvmConfig> {
-    /// The directory to write the witness to. Additionally, diff files will be written to this
-    /// directory in case of failed sanity checks.
-    output_directory: PathBuf,
     /// The provider to read the historical state and do the EVM execution.
     provider: P,
     /// The EVM configuration to use for the execution.
     evm_config: EvmConfig,
+    /// The directory to write the witness to. Additionally, diff files will be written to this
+    /// directory in case of failed sanity checks.
+    output_directory: PathBuf,
+    /// The healthy node client to compare the witness against.
+    healthy_node_client: Option<jsonrpsee::http_client::HttpClient>,
 }
 
 impl<P, EvmConfig> InvalidBlockWitnessHook<P, EvmConfig> {
     /// Creates a new witness hook.
-    pub const fn new(output_directory: PathBuf, provider: P, evm_config: EvmConfig) -> Self {
-        Self { output_directory, provider, evm_config }
+    pub const fn new(
+        provider: P,
+        evm_config: EvmConfig,
+        output_directory: PathBuf,
+        healthy_node_client: Option<jsonrpsee::http_client::HttpClient>,
+    ) -> Self {
+        Self { provider, evm_config, output_directory, healthy_node_client }
     }
 }
 
@@ -148,12 +156,13 @@ where
         let witness = state_provider.witness(Default::default(), hashed_state.clone())?;
 
         // Write the witness to the output directory.
-        let mut file = File::options()
-            .write(true)
-            .create_new(true)
-            .open(self.output_directory.join(format!("{}_{}.json", block.number, block.hash())))?;
         let response = ExecutionWitness { witness, state_preimages: Some(state_preimages) };
-        file.write_all(serde_json::to_string(&response)?.as_bytes())?;
+        File::create_new(self.output_directory.join(format!(
+            "{}_{}.json",
+            block.number,
+            block.hash()
+        )))?
+        .write_all(serde_json::to_string(&response)?.as_bytes())?;
 
         // The bundle state after re-execution should match the original one.
         if bundle_state != output.state {
@@ -179,6 +188,33 @@ where
             }
         }
 
+        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(),
+                    true,
+                )
+                .await
+            })?;
+
+            // Write the healthy node witness to the output directory.
+            File::create_new(self.output_directory.join(format!(
+                "{}_{}.healthy_witness.json",
+                block.number,
+                block.hash()
+            )))?
+            .write_all(serde_json::to_string(&healthy_node_witness)?.as_bytes())?;
+
+            // If the witnesses are different, write the diff to the output directory.
+            if response != healthy_node_witness {
+                let filename = format!("{}_{}.healthy_witness.diff", block.number, block.hash());
+                let path = self.save_diff(filename, &response, &healthy_node_witness)?;
+                warn!(target: "engine::invalid_block_hooks::witness", path = %path.display(), "Witness mismatch against healthy node");
+            }
+        }
+
         Ok(())
     }
 
@@ -191,11 +227,7 @@ where
     ) -> eyre::Result<PathBuf> {
         let path = self.output_directory.join(filename);
         let diff = Comparison::new(original, new);
-        File::options()
-            .write(true)
-            .create_new(true)
-            .open(&path)?
-            .write_all(diff.to_string().as_bytes())?;
+        File::create_new(&path)?.write_all(diff.to_string().as_bytes())?;
 
         Ok(path)
     }

crates/engine/tree/src/tree/mod.rs

@@ -561,7 +561,7 @@ where
         }
     }
 
-    /// Sets the bad block hook.
+    /// Sets the invalid block hook.
     fn set_invalid_block_hook(&mut self, invalid_block_hook: Box<dyn InvalidBlockHook>) {
         self.invalid_block_hook = invalid_block_hook;
     }

crates/node/builder/Cargo.toml

@@ -44,6 +44,7 @@ reth-payload-validator.workspace = true
 reth-primitives.workspace = true
 reth-provider.workspace = true
 reth-prune.workspace = true
+reth-rpc-api.workspace = true
 reth-rpc-builder.workspace = true
 reth-rpc-engine-api.workspace = true
 reth-rpc-eth-types.workspace = true
@@ -82,6 +83,7 @@ secp256k1 = { workspace = true, features = [
 aquamarine.workspace = true
 eyre.workspace = true
 fdlimit.workspace = true
+jsonrpsee.workspace = true
 rayon.workspace = true
 
 # tracing

crates/node/builder/src/launch/common.rs

@@ -3,7 +3,7 @@
 use std::{sync::Arc, thread::available_parallelism};
 
 use alloy_primitives::{BlockNumber, B256};
-use eyre::Context;
+use eyre::{Context, OptionExt};
 use rayon::ThreadPoolBuilder;
 use reth_auto_seal_consensus::MiningMode;
 use reth_beacon_consensus::EthBeaconConsensus;
@@ -23,6 +23,7 @@ use reth_invalid_block_hooks::InvalidBlockWitnessHook;
 use reth_network_p2p::headers::client::HeadersClient;
 use reth_node_api::{FullNodeTypes, NodeTypes, NodeTypesWithDB};
 use reth_node_core::{
+    args::InvalidBlockHookType,
     dirs::{ChainPath, DataDirPath},
     node_config::NodeConfig,
     version::{
@@ -44,6 +45,7 @@ use reth_provider::{
     TreeViewer,
 };
 use reth_prune::{PruneModes, PrunerBuilder};
+use reth_rpc_api::clients::EthApiClient;
 use reth_rpc_builder::config::RethRpcServerConfig;
 use reth_rpc_layer::JwtSecret;
 use reth_stages::{sets::DefaultStages, MetricEvent, Pipeline, PipelineTarget, StageId};
@@ -855,35 +857,62 @@ where
 {
     /// Returns the [`InvalidBlockHook`] to use for the node.
     pub fn invalid_block_hook(&self) -> eyre::Result<Box<dyn InvalidBlockHook>> {
-        Ok(if let Some(ref hook) = self.node_config().debug.invalid_block_hook {
-            let output_directory = self.data_dir().invalid_block_hooks();
-            let hooks = hook
-                .iter()
-                .copied()
-                .map(|hook| {
-                    let output_directory = output_directory.join(hook.to_string());
-                    fs::create_dir_all(&output_directory)?;
-
-                    Ok(match hook {
-                        reth_node_core::args::InvalidBlockHook::Witness => {
-                            Box::new(InvalidBlockWitnessHook::new(
-                                output_directory,
-                                self.blockchain_db().clone(),
-                                self.components().evm_config().clone(),
-                            )) as Box<dyn InvalidBlockHook>
-                        }
-                        reth_node_core::args::InvalidBlockHook::PreState |
-                        reth_node_core::args::InvalidBlockHook::Opcode => {
-                            eyre::bail!("invalid block hook {hook:?} is not implemented yet")
-                        }
-                    })
-                })
-                .collect::<Result<_, _>>()?;
-
-            Box::new(InvalidBlockHooks(hooks))
-        } else {
-            Box::new(NoopInvalidBlockHook::default())
-        })
+        let Some(ref hook) = self.node_config().debug.invalid_block_hook else {
+            return Ok(Box::new(NoopInvalidBlockHook::default()))
+        };
+        let healthy_node_rpc_client = self.get_healthy_node_client()?;
+
+        let output_directory = self.data_dir().invalid_block_hooks();
+        let hooks = hook
+            .iter()
+            .copied()
+            .map(|hook| {
+                let output_directory = output_directory.join(hook.to_string());
+                fs::create_dir_all(&output_directory)?;
+
+                Ok(match hook {
+                    InvalidBlockHookType::Witness => Box::new(InvalidBlockWitnessHook::new(
+                        self.blockchain_db().clone(),
+                        self.components().evm_config().clone(),
+                        output_directory,
+                        healthy_node_rpc_client.clone(),
+                    )),
+                    InvalidBlockHookType::PreState | InvalidBlockHookType::Opcode => {
+                        eyre::bail!("invalid block hook {hook:?} is not implemented yet")
+                    }
+                } as Box<dyn InvalidBlockHook>)
+            })
+            .collect::<Result<_, _>>()?;
+
+        Ok(Box::new(InvalidBlockHooks(hooks)))
+    }
+
+    /// Returns an RPC client for the healthy node, if configured in the node config.
+    fn get_healthy_node_client(&self) -> eyre::Result<Option<jsonrpsee::http_client::HttpClient>> {
+        self.node_config()
+            .debug
+            .healthy_node_rpc_url
+            .as_ref()
+            .map(|url| {
+                let client = jsonrpsee::http_client::HttpClientBuilder::default().build(url)?;
+
+                // Verify that the healthy node is running the same chain as the current node.
+                let chain_id = futures::executor::block_on(async {
+                    EthApiClient::<
+                        reth_rpc_types::Transaction,
+                        reth_rpc_types::Block,
+                        reth_rpc_types::Receipt,
+                    >::chain_id(&client)
+                    .await
+                })?
+                .ok_or_eyre("healthy node rpc client didn't return a chain id")?;
+                if chain_id.to::<u64>() != self.chain_id().id() {
+                    eyre::bail!("invalid chain id for healthy node: {chain_id}")
+                }
+
+                Ok(client)
+            })
+            .transpose()
     }
 }