From 664f8b23be6e990a9b55de2269507aa892585384 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Mon, 16 Sep 2024 12:03:49 +0100 Subject: [PATCH] feat(engine): compare invalid block witness against a healthy node (#10844) --- Cargo.lock | 5 ++ book/cli/reth/node.md | 5 +- crates/engine/invalid-block-hooks/Cargo.toml | 5 ++ .../engine/invalid-block-hooks/src/witness.rs | 62 ++++++++++---- crates/engine/tree/src/tree/mod.rs | 2 +- crates/node/builder/Cargo.toml | 2 + crates/node/builder/src/launch/common.rs | 85 +++++++++++++------ crates/node/core/src/args/debug.rs | 81 ++++++++++-------- crates/node/core/src/args/mod.rs | 2 +- 9 files changed, 167 insertions(+), 82 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9ba97195e..16edfba2d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7367,6 +7367,8 @@ dependencies = [ "alloy-rlp", "alloy-rpc-types-debug", "eyre", + "futures", + "jsonrpsee", "pretty_assertions", "reth-chainspec", "reth-engine-primitives", @@ -7374,6 +7376,7 @@ dependencies = [ "reth-primitives", "reth-provider", "reth-revm", + "reth-rpc-api", "reth-tracing", "reth-trie", "serde_json", @@ -7643,6 +7646,7 @@ dependencies = [ "eyre", "fdlimit", "futures", + "jsonrpsee", "rayon", "reth-auto-seal-consensus", "reth-beacon-consensus", @@ -7676,6 +7680,7 @@ dependencies = [ "reth-provider", "reth-prune", "reth-rpc", + "reth-rpc-api", "reth-rpc-builder", "reth-rpc-engine-api", "reth-rpc-eth-types", diff --git a/book/cli/reth/node.md b/book/cli/reth/node.md index 722b0ec07..b73b1319b 100644 --- a/book/cli/reth/node.md +++ b/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 - 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 + The RPC URL of a healthy node to use for comparing invalid block hook results against. + Database: --db.log-level Database logging level. Levels higher than "notice" require a debug build diff --git a/crates/engine/invalid-block-hooks/Cargo.toml b/crates/engine/invalid-block-hooks/Cargo.toml index 17ac3cec5..b0eaefce0 100644 --- a/crates/engine/invalid-block-hooks/Cargo.toml +++ b/crates/engine/invalid-block-hooks/Cargo.toml @@ -18,6 +18,7 @@ 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"] } @@ -25,7 +26,11 @@ reth-trie = { workspace = true, features = ["serde"] } 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 diff --git a/crates/engine/invalid-block-hooks/src/witness.rs b/crates/engine/invalid-block-hooks/src/witness.rs index c3829e94e..eb703fb33 100644 --- a/crates/engine/invalid-block-hooks/src/witness.rs +++ b/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 { - /// 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, } impl InvalidBlockWitnessHook { /// 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, + ) -> 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 { 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) } diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index f095098ac..ef7ce66c7 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/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) { self.invalid_block_hook = invalid_block_hook; } diff --git a/crates/node/builder/Cargo.toml b/crates/node/builder/Cargo.toml index 2b3f1faec..57dfd882c 100644 --- a/crates/node/builder/Cargo.toml +++ b/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 diff --git a/crates/node/builder/src/launch/common.rs b/crates/node/builder/src/launch/common.rs index f6c488a00..1fc0d8cae 100644 --- a/crates/node/builder/src/launch/common.rs +++ b/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> { - 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)?; + 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()?; - 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 - } - reth_node_core::args::InvalidBlockHook::PreState | - reth_node_core::args::InvalidBlockHook::Opcode => { - eyre::bail!("invalid block hook {hook:?} is not implemented yet") - } - }) - }) - .collect::>()?; + 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)?; - Box::new(InvalidBlockHooks(hooks)) - } else { - Box::new(NoopInvalidBlockHook::default()) - }) + 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) + }) + .collect::>()?; + + 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> { + 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::() != self.chain_id().id() { + eyre::bail!("invalid chain id for healthy node: {chain_id}") + } + + Ok(client) + }) + .transpose() } } diff --git a/crates/node/core/src/args/debug.rs b/crates/node/core/src/args/debug.rs index 175e2e235..5ed882c92 100644 --- a/crates/node/core/src/args/debug.rs +++ b/crates/node/core/src/args/debug.rs @@ -68,11 +68,20 @@ pub struct DebugArgs { #[arg(long = "debug.engine-api-store", help_heading = "Debug", value_name = "PATH")] pub engine_api_store: Option, - /// Determines which type of bad block hook to install + /// Determines which type of invalid block hook to install /// /// Example: `witness,prestate` #[arg(long = "debug.invalid-block-hook", help_heading = "Debug", value_parser = InvalidBlockSelectionValueParser::default())] pub invalid_block_hook: Option, + + /// The RPC URL of a healthy node to use for comparing invalid block hook results against. + #[arg( + long = "debug.healthy-node-rpc-url", + help_heading = "Debug", + value_name = "URL", + verbatim_doc_comment + )] + pub healthy_node_rpc_url: Option, } /// Describes the invalid block hooks that should be installed. @@ -82,11 +91,11 @@ pub struct DebugArgs { /// Create a [`InvalidBlockSelection`] from a selection. /// /// ``` -/// use reth_node_core::args::{InvalidBlockHook, InvalidBlockSelection}; -/// let config: InvalidBlockSelection = vec![InvalidBlockHook::Witness].into(); +/// use reth_node_core::args::{InvalidBlockHookType, InvalidBlockSelection}; +/// let config: InvalidBlockSelection = vec![InvalidBlockHookType::Witness].into(); /// ``` #[derive(Debug, Clone, PartialEq, Eq, derive_more::Deref)] -pub struct InvalidBlockSelection(HashSet); +pub struct InvalidBlockSelection(HashSet); impl InvalidBlockSelection { /// Creates a new _unique_ [`InvalidBlockSelection`] from the given items. @@ -97,73 +106,73 @@ impl InvalidBlockSelection { /// /// # Example /// - /// Create a selection from the [`InvalidBlockHook`] string identifiers + /// Create a selection from the [`InvalidBlockHookType`] string identifiers /// /// ``` - /// use reth_node_core::args::{InvalidBlockHook, InvalidBlockSelection}; + /// use reth_node_core::args::{InvalidBlockHookType, InvalidBlockSelection}; /// let selection = vec!["witness", "prestate", "opcode"]; /// let config = InvalidBlockSelection::try_from_selection(selection).unwrap(); /// assert_eq!( /// config, /// InvalidBlockSelection::from([ - /// InvalidBlockHook::Witness, - /// InvalidBlockHook::PreState, - /// InvalidBlockHook::Opcode + /// InvalidBlockHookType::Witness, + /// InvalidBlockHookType::PreState, + /// InvalidBlockHookType::Opcode /// ]) /// ); /// ``` /// - /// Create a unique selection from the [`InvalidBlockHook`] string identifiers + /// Create a unique selection from the [`InvalidBlockHookType`] string identifiers /// /// ``` - /// use reth_node_core::args::{InvalidBlockHook, InvalidBlockSelection}; + /// use reth_node_core::args::{InvalidBlockHookType, InvalidBlockSelection}; /// let selection = vec!["witness", "prestate", "opcode", "witness", "prestate"]; /// let config = InvalidBlockSelection::try_from_selection(selection).unwrap(); /// assert_eq!( /// config, /// InvalidBlockSelection::from([ - /// InvalidBlockHook::Witness, - /// InvalidBlockHook::PreState, - /// InvalidBlockHook::Opcode + /// InvalidBlockHookType::Witness, + /// InvalidBlockHookType::PreState, + /// InvalidBlockHookType::Opcode /// ]) /// ); /// ``` pub fn try_from_selection(selection: I) -> Result where I: IntoIterator, - T: TryInto, + T: TryInto, { selection.into_iter().map(TryInto::try_into).collect() } - /// Clones the set of configured [`InvalidBlockHook`]. - pub fn to_selection(&self) -> HashSet { + /// Clones the set of configured [`InvalidBlockHookType`]. + pub fn to_selection(&self) -> HashSet { self.0.clone() } } -impl From<&[InvalidBlockHook]> for InvalidBlockSelection { - fn from(s: &[InvalidBlockHook]) -> Self { +impl From<&[InvalidBlockHookType]> for InvalidBlockSelection { + fn from(s: &[InvalidBlockHookType]) -> Self { Self(s.iter().copied().collect()) } } -impl From> for InvalidBlockSelection { - fn from(s: Vec) -> Self { +impl From> for InvalidBlockSelection { + fn from(s: Vec) -> Self { Self(s.into_iter().collect()) } } -impl From<[InvalidBlockHook; N]> for InvalidBlockSelection { - fn from(s: [InvalidBlockHook; N]) -> Self { +impl From<[InvalidBlockHookType; N]> for InvalidBlockSelection { + fn from(s: [InvalidBlockHookType; N]) -> Self { Self(s.iter().copied().collect()) } } -impl FromIterator for InvalidBlockSelection { +impl FromIterator for InvalidBlockSelection { fn from_iter(iter: I) -> Self where - I: IntoIterator, + I: IntoIterator, { Self(iter.into_iter().collect()) } @@ -205,7 +214,7 @@ impl TypedValueParser for InvalidBlockSelectionValueParser { value.to_str().ok_or_else(|| clap::Error::new(clap::error::ErrorKind::InvalidUtf8))?; val.parse::().map_err(|err| { let arg = arg.map(|a| a.to_string()).unwrap_or_else(|| "...".to_owned()); - let possible_values = InvalidBlockHook::all_variant_names().to_vec().join(","); + let possible_values = InvalidBlockHookType::all_variant_names().to_vec().join(","); let msg = format!( "Invalid value '{val}' for {arg}: {err}.\n [possible values: {possible_values}]" ); @@ -214,12 +223,12 @@ impl TypedValueParser for InvalidBlockSelectionValueParser { } fn possible_values(&self) -> Option + '_>> { - let values = InvalidBlockHook::all_variant_names().iter().map(PossibleValue::new); + let values = InvalidBlockHookType::all_variant_names().iter().map(PossibleValue::new); Some(Box::new(values)) } } -/// The type of bad block hook to install +/// The type of invalid block hook to install #[derive( Debug, Clone, @@ -234,7 +243,7 @@ impl TypedValueParser for InvalidBlockSelectionValueParser { EnumIter, )] #[strum(serialize_all = "kebab-case")] -pub enum InvalidBlockHook { +pub enum InvalidBlockHookType { /// A witness value enum Witness, /// A prestate trace value enum @@ -243,7 +252,7 @@ pub enum InvalidBlockHook { Opcode, } -impl FromStr for InvalidBlockHook { +impl FromStr for InvalidBlockHookType { type Err = ParseError; fn from_str(s: &str) -> Result { @@ -256,20 +265,20 @@ impl FromStr for InvalidBlockHook { } } -impl TryFrom<&str> for InvalidBlockHook { +impl TryFrom<&str> for InvalidBlockHookType { type Error = ParseError; fn try_from(s: &str) -> Result>::Error> { FromStr::from_str(s) } } -impl fmt::Display for InvalidBlockHook { +impl fmt::Display for InvalidBlockHookType { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.pad(self.as_ref()) } } -impl InvalidBlockHook { +impl InvalidBlockHookType { /// Returns all variant names of the enum pub const fn all_variant_names() -> &'static [&'static str] { ::VARIANTS @@ -298,7 +307,7 @@ mod tests { #[test] fn test_parse_invalid_block_args() { let expected_args = DebugArgs { - invalid_block_hook: Some(InvalidBlockSelection::from([InvalidBlockHook::Witness])), + invalid_block_hook: Some(InvalidBlockSelection::from([InvalidBlockHookType::Witness])), ..Default::default() }; let args = CommandParser::::parse_from([ @@ -311,8 +320,8 @@ mod tests { let expected_args = DebugArgs { invalid_block_hook: Some(InvalidBlockSelection::from([ - InvalidBlockHook::Witness, - InvalidBlockHook::PreState, + InvalidBlockHookType::Witness, + InvalidBlockHookType::PreState, ])), ..Default::default() }; diff --git a/crates/node/core/src/args/mod.rs b/crates/node/core/src/args/mod.rs index 0218abddb..1a647ac65 100644 --- a/crates/node/core/src/args/mod.rs +++ b/crates/node/core/src/args/mod.rs @@ -14,7 +14,7 @@ pub use rpc_state_cache::RpcStateCacheArgs; /// DebugArgs struct for debugging purposes mod debug; -pub use debug::{DebugArgs, InvalidBlockHook, InvalidBlockSelection}; +pub use debug::{DebugArgs, InvalidBlockHookType, InvalidBlockSelection}; /// DatabaseArgs struct for configuring the database mod database;