From 55b116461274447db0b1eeaeffa4954c72637a5c Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Wed, 11 Sep 2024 15:37:54 +0100 Subject: [PATCH 1/9] feat(engine): compare invalid block witness against a healthy node --- Cargo.lock | 4 ++ crates/engine/invalid-block-hooks/Cargo.toml | 5 ++ .../engine/invalid-block-hooks/src/witness.rs | 35 +++++++++-- crates/node/builder/Cargo.toml | 1 + crates/node/builder/src/launch/common.rs | 62 +++++++++++-------- crates/node/core/src/args/debug.rs | 9 +++ 6 files changed, 84 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fceb2c06ac7b..073a6e1f70f2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7347,6 +7347,7 @@ dependencies = [ "alloy-rlp", "alloy-rpc-types-debug", "eyre", + "jsonrpsee", "pretty_assertions", "reth-chainspec", "reth-engine-primitives", @@ -7354,9 +7355,11 @@ dependencies = [ "reth-primitives", "reth-provider", "reth-revm", + "reth-rpc-api", "reth-tracing", "reth-trie", "serde_json", + "tokio", ] [[package]] @@ -7623,6 +7626,7 @@ dependencies = [ "eyre", "fdlimit", "futures", + "jsonrpsee", "rayon", "reth-auto-seal-consensus", "reth-beacon-consensus", diff --git a/crates/engine/invalid-block-hooks/Cargo.toml b/crates/engine/invalid-block-hooks/Cargo.toml index 17ac3cec5523..ab90d73f9826 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 +tokio.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 d22dc2b21de4..8c9c798b66bf 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 } } } @@ -185,6 +193,23 @@ where } } + if let Some(healthy_node_client) = &self.healthy_node_client { + let healthy_node_witness = tokio::runtime::Handle::current().block_on(async move { + DebugApiClient::debug_execution_witness( + healthy_node_client, + block.number.into(), + true, + ) + .await + })?; + + if response != healthy_node_witness { + let filename = format!("{}_{}.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(()) } diff --git a/crates/node/builder/Cargo.toml b/crates/node/builder/Cargo.toml index 2b3f1faec364..8b3089de739d 100644 --- a/crates/node/builder/Cargo.toml +++ b/crates/node/builder/Cargo.toml @@ -82,6 +82,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 e15a852e7d62..647a3971f7de 100644 --- a/crates/node/builder/src/launch/common.rs +++ b/crates/node/builder/src/launch/common.rs @@ -855,35 +855,43 @@ 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)?; - - 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") - } - }) + let Some(ref hook) = self.node_config().debug.invalid_block_hook else { + return Ok(Box::new(NoopInvalidBlockHook::default())) + }; + let healthy_node_rpc_client = self + .node_config() + .debug + .healthy_node_rpc_url + .as_ref() + .map(|url| jsonrpsee::http_client::HttpClientBuilder::default().build(url)) + .transpose()?; + + 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( + self.blockchain_db().clone(), + self.components().evm_config().clone(), + output_directory, + healthy_node_rpc_client.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::>()?; + }) + .collect::>()?; - Box::new(InvalidBlockHooks(hooks)) - } else { - Box::new(NoopInvalidBlockHook::default()) - }) + Ok(Box::new(InvalidBlockHooks(hooks))) } } diff --git a/crates/node/core/src/args/debug.rs b/crates/node/core/src/args/debug.rs index 175e2e235d7f..1e635c8627e8 100644 --- a/crates/node/core/src/args/debug.rs +++ b/crates/node/core/src/args/debug.rs @@ -73,6 +73,15 @@ pub struct DebugArgs { /// Example: `witness,prestate` #[arg(long = "debug.invalid-block-hook", help_heading = "Debug", value_parser = InvalidBlockSelectionValueParser::default())] pub invalid_block_hook: Option, + + /// The URL of the healthy node RPC. + #[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. From f2d12f7ffdaf95f5cd9c99c88ad13e9fd386ef2a Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Wed, 11 Sep 2024 15:45:20 +0100 Subject: [PATCH 2/9] update book --- book/cli/reth/node.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/book/cli/reth/node.md b/book/cli/reth/node.md index 0438f67ee7e1..60d80d8356d5 100644 --- a/book/cli/reth/node.md +++ b/book/cli/reth/node.md @@ -547,6 +547,9 @@ Debug: [possible values: witness, pre-state, opcode] + --debug.healthy-node-rpc-url + The URL of the healthy node RPC. + Database: --db.log-level Database logging level. Levels higher than "notice" require a debug build From e2fba8b509be411ea47f8ec6bde4560dce3bf3d8 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Wed, 11 Sep 2024 15:58:44 +0100 Subject: [PATCH 3/9] write healthy witness --- book/cli/reth/node.md | 2 +- .../engine/invalid-block-hooks/src/witness.rs | 23 +++++++++++++++---- crates/node/core/src/args/debug.rs | 2 +- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/book/cli/reth/node.md b/book/cli/reth/node.md index 60d80d8356d5..dad8e07de7f4 100644 --- a/book/cli/reth/node.md +++ b/book/cli/reth/node.md @@ -548,7 +548,7 @@ Debug: [possible values: witness, pre-state, opcode] --debug.healthy-node-rpc-url - The URL of the healthy node RPC. + The RPC URL of a healthy node. Database: --db.log-level diff --git a/crates/engine/invalid-block-hooks/src/witness.rs b/crates/engine/invalid-block-hooks/src/witness.rs index 8c9c798b66bf..968ad501e6da 100644 --- a/crates/engine/invalid-block-hooks/src/witness.rs +++ b/crates/engine/invalid-block-hooks/src/witness.rs @@ -162,12 +162,12 @@ where let witness = state_provider.witness(HashedPostState::default(), hashed_state.clone())?; // Write the witness to the output directory. - let mut file = File::options() + let response = ExecutionWitness { witness, state_preimages: Some(state_preimages) }; + 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())?; + .open(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 { @@ -194,6 +194,7 @@ where } if let Some(healthy_node_client) = &self.healthy_node_client { + // Compare the witness against the healthy node. let healthy_node_witness = tokio::runtime::Handle::current().block_on(async move { DebugApiClient::debug_execution_witness( healthy_node_client, @@ -203,8 +204,20 @@ where .await })?; + // Write the healthy node witness to the output directory. + File::options() + .write(true) + .create_new(true) + .open(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!("{}_{}.witness.diff", block.number, block.hash()); + 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"); } diff --git a/crates/node/core/src/args/debug.rs b/crates/node/core/src/args/debug.rs index 1e635c8627e8..4894fde4001b 100644 --- a/crates/node/core/src/args/debug.rs +++ b/crates/node/core/src/args/debug.rs @@ -74,7 +74,7 @@ pub struct DebugArgs { #[arg(long = "debug.invalid-block-hook", help_heading = "Debug", value_parser = InvalidBlockSelectionValueParser::default())] pub invalid_block_hook: Option, - /// The URL of the healthy node RPC. + /// The RPC URL of a healthy node. #[arg( long = "debug.healthy-node-rpc-url", help_heading = "Debug", From e1a701204d76b3e151d512f5eedd8c678a3fa9b4 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Wed, 11 Sep 2024 16:41:19 +0100 Subject: [PATCH 4/9] check chain id of a healthy node --- Cargo.lock | 1 + crates/node/builder/Cargo.toml | 1 + crates/node/builder/src/launch/common.rs | 24 ++++++++++++++++++++++-- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 073a6e1f70f2..f7b0b06ac971 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7660,6 +7660,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/crates/node/builder/Cargo.toml b/crates/node/builder/Cargo.toml index 8b3089de739d..57dfd882c5f7 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 diff --git a/crates/node/builder/src/launch/common.rs b/crates/node/builder/src/launch/common.rs index 647a3971f7de..67e4050c1949 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; @@ -44,6 +44,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}; @@ -863,7 +864,26 @@ where .debug .healthy_node_rpc_url .as_ref() - .map(|url| jsonrpsee::http_client::HttpClientBuilder::default().build(url)) + .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 = tokio::runtime::Handle::current() + .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()?; let output_directory = self.data_dir().invalid_block_hooks(); From 21f5f289203046f117e2ea232b6fcf428b2dc783 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Thu, 12 Sep 2024 11:22:23 +0100 Subject: [PATCH 5/9] use futures::executor::block_on --- Cargo.lock | 2 +- crates/engine/invalid-block-hooks/Cargo.toml | 2 +- .../engine/invalid-block-hooks/src/witness.rs | 2 +- crates/node/builder/src/launch/common.rs | 19 +++++++++---------- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f7b0b06ac971..eeaa00152408 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7347,6 +7347,7 @@ dependencies = [ "alloy-rlp", "alloy-rpc-types-debug", "eyre", + "futures", "jsonrpsee", "pretty_assertions", "reth-chainspec", @@ -7359,7 +7360,6 @@ dependencies = [ "reth-tracing", "reth-trie", "serde_json", - "tokio", ] [[package]] diff --git a/crates/engine/invalid-block-hooks/Cargo.toml b/crates/engine/invalid-block-hooks/Cargo.toml index ab90d73f9826..b0eaefce07fe 100644 --- a/crates/engine/invalid-block-hooks/Cargo.toml +++ b/crates/engine/invalid-block-hooks/Cargo.toml @@ -27,7 +27,7 @@ alloy-rlp.workspace = true alloy-rpc-types-debug.workspace = true # async -tokio.workspace = true +futures.workspace = true # misc eyre.workspace = true diff --git a/crates/engine/invalid-block-hooks/src/witness.rs b/crates/engine/invalid-block-hooks/src/witness.rs index 968ad501e6da..2cce2028d9a5 100644 --- a/crates/engine/invalid-block-hooks/src/witness.rs +++ b/crates/engine/invalid-block-hooks/src/witness.rs @@ -195,7 +195,7 @@ where if let Some(healthy_node_client) = &self.healthy_node_client { // Compare the witness against the healthy node. - let healthy_node_witness = tokio::runtime::Handle::current().block_on(async move { + let healthy_node_witness = futures::executor::block_on(async move { DebugApiClient::debug_execution_witness( healthy_node_client, block.number.into(), diff --git a/crates/node/builder/src/launch/common.rs b/crates/node/builder/src/launch/common.rs index 67e4050c1949..50478fd0e786 100644 --- a/crates/node/builder/src/launch/common.rs +++ b/crates/node/builder/src/launch/common.rs @@ -868,16 +868,15 @@ where 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 = tokio::runtime::Handle::current() - .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")?; + 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}") } From 1a7ed2c4d7900d42ebc0bb09ce9b57be7b582555 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Thu, 12 Sep 2024 15:39:03 +0100 Subject: [PATCH 6/9] clarify the cli arg doc --- book/cli/reth/node.md | 2 +- crates/node/core/src/args/debug.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/book/cli/reth/node.md b/book/cli/reth/node.md index dad8e07de7f4..f1156eb8cc6d 100644 --- a/book/cli/reth/node.md +++ b/book/cli/reth/node.md @@ -548,7 +548,7 @@ Debug: [possible values: witness, pre-state, opcode] --debug.healthy-node-rpc-url - The RPC URL of a healthy node. + The RPC URL of a healthy node to use for comparing invalid block hook results against. Database: --db.log-level diff --git a/crates/node/core/src/args/debug.rs b/crates/node/core/src/args/debug.rs index 4894fde4001b..47f8475a7fd7 100644 --- a/crates/node/core/src/args/debug.rs +++ b/crates/node/core/src/args/debug.rs @@ -74,7 +74,7 @@ pub struct DebugArgs { #[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. + /// 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", From ab28ceb8f564a0a0c6337d80fd39f09cc3a8620e Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Fri, 13 Sep 2024 13:37:27 +0100 Subject: [PATCH 7/9] use File::create_new --- .../engine/invalid-block-hooks/src/witness.rs | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/crates/engine/invalid-block-hooks/src/witness.rs b/crates/engine/invalid-block-hooks/src/witness.rs index 2cce2028d9a5..e0e84dcf5f82 100644 --- a/crates/engine/invalid-block-hooks/src/witness.rs +++ b/crates/engine/invalid-block-hooks/src/witness.rs @@ -163,11 +163,12 @@ where // Write the witness to the output directory. let response = ExecutionWitness { witness, state_preimages: Some(state_preimages) }; - File::options() - .write(true) - .create_new(true) - .open(self.output_directory.join(format!("{}_{}.json", block.number, block.hash())))? - .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 { @@ -205,15 +206,12 @@ where })?; // Write the healthy node witness to the output directory. - File::options() - .write(true) - .create_new(true) - .open(self.output_directory.join(format!( - "{}_{}.healthy_witness.json", - block.number, - block.hash() - )))? - .write_all(serde_json::to_string(&healthy_node_witness)?.as_bytes())?; + 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 { @@ -235,11 +233,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) } From 4fd65096b389714448bd01a1a32dad9b336b74c3 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Fri, 13 Sep 2024 14:40:22 +0100 Subject: [PATCH 8/9] fixes after review --- crates/engine/tree/src/tree/mod.rs | 2 +- crates/node/builder/src/launch/common.rs | 62 ++++++++++++------------ crates/node/core/src/args/debug.rs | 46 +++++++++--------- crates/node/core/src/args/mod.rs | 2 +- 4 files changed, 57 insertions(+), 55 deletions(-) diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index 9c8377596ba6..738936cc0d9a 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -558,7 +558,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/src/launch/common.rs b/crates/node/builder/src/launch/common.rs index 50478fd0e786..75e4dd68fb7b 100644 --- a/crates/node/builder/src/launch/common.rs +++ b/crates/node/builder/src/launch/common.rs @@ -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::{ @@ -859,8 +860,36 @@ where let Some(ref hook) = self.node_config().debug.invalid_block_hook else { return Ok(Box::new(NoopInvalidBlockHook::default())) }; - let healthy_node_rpc_client = self - .node_config() + 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) + }) + .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() @@ -883,34 +912,7 @@ where Ok(client) }) - .transpose()?; - - 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( - self.blockchain_db().clone(), - self.components().evm_config().clone(), - output_directory, - healthy_node_rpc_client.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::>()?; - - Ok(Box::new(InvalidBlockHooks(hooks))) + .transpose() } } diff --git a/crates/node/core/src/args/debug.rs b/crates/node/core/src/args/debug.rs index 47f8475a7fd7..8b74b22e7423 100644 --- a/crates/node/core/src/args/debug.rs +++ b/crates/node/core/src/args/debug.rs @@ -68,7 +68,7 @@ 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())] @@ -95,7 +95,7 @@ pub struct DebugArgs { /// let config: InvalidBlockSelection = vec![InvalidBlockHook::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. @@ -140,39 +140,39 @@ impl InvalidBlockSelection { 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 { + 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()) } @@ -214,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}]" ); @@ -223,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, @@ -243,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 @@ -252,7 +252,7 @@ pub enum InvalidBlockHook { Opcode, } -impl FromStr for InvalidBlockHook { +impl FromStr for InvalidBlockHookType { type Err = ParseError; fn from_str(s: &str) -> Result { @@ -265,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 @@ -307,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([ @@ -320,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 0218abddb08c..1a647ac65b03 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; From b980b8cdb636d73bf8a0fde4c23497e05196a19a Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Fri, 13 Sep 2024 17:26:15 +0100 Subject: [PATCH 9/9] update doc comments --- crates/node/core/src/args/debug.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/node/core/src/args/debug.rs b/crates/node/core/src/args/debug.rs index 8b74b22e7423..5ed882c92144 100644 --- a/crates/node/core/src/args/debug.rs +++ b/crates/node/core/src/args/debug.rs @@ -91,8 +91,8 @@ 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); @@ -106,34 +106,34 @@ 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 /// ]) /// ); /// ``` @@ -145,7 +145,7 @@ impl InvalidBlockSelection { selection.into_iter().map(TryInto::try_into).collect() } - /// Clones the set of configured [`InvalidBlockHook`]. + /// Clones the set of configured [`InvalidBlockHookType`]. pub fn to_selection(&self) -> HashSet { self.0.clone() }