From 99dfad947025661d2e0d8f5ea0ad2a3b8f834b93 Mon Sep 17 00:00:00 2001 From: "Supernovahs.eth" <91280922+supernovahs@users.noreply.github.com> Date: Thu, 5 Oct 2023 22:10:17 +0530 Subject: [PATCH 01/23] Simplify node components (#4922) Co-authored-by: Matthias Seitz --- bin/reth/src/args/rpc_server_args.rs | 10 +++--- bin/reth/src/cli/components.rs | 26 ++++++++++++++++ bin/reth/src/cli/ext.rs | 31 +++++-------------- .../src/main.rs | 26 +++++----------- 4 files changed, 46 insertions(+), 47 deletions(-) diff --git a/bin/reth/src/args/rpc_server_args.rs b/bin/reth/src/args/rpc_server_args.rs index 5ca3932e7bda..aa12adaacffd 100644 --- a/bin/reth/src/args/rpc_server_args.rs +++ b/bin/reth/src/args/rpc_server_args.rs @@ -2,7 +2,7 @@ use crate::{ args::GasPriceOracleArgs, - cli::{config::RethRpcConfig, ext::RethNodeCommandConfig}, + cli::{components::RethRpcComponents, config::RethRpcConfig, ext::RethNodeCommandConfig}, }; use clap::{ builder::{PossibleValue, RangedU64ValueParser, TypedValueParser}, @@ -193,19 +193,19 @@ impl RpcServerArgs { let module_config = self.transport_rpc_module_config(); debug!(target: "reth::cli", http=?module_config.http(), ws=?module_config.ws(), "Using RPC module config"); - let (mut rpc_modules, auth_module, mut registry) = RpcModuleBuilder::default() + let (mut modules, auth_module, mut registry) = RpcModuleBuilder::default() .with_provider(components.provider()) .with_pool(components.pool()) .with_network(components.network()) .with_events(components.events()) .with_executor(components.task_executor()) .build_with_auth_server(module_config, engine_api); - + let node_modules = RethRpcComponents { registry: &mut registry, modules: &mut modules }; // apply configured customization - conf.extend_rpc_modules(self, components, &mut registry, &mut rpc_modules)?; + conf.extend_rpc_modules(self, components, node_modules)?; let server_config = self.rpc_server_config(); - let launch_rpc = rpc_modules.start_server(server_config).map_ok(|handle| { + let launch_rpc = modules.start_server(server_config).map_ok(|handle| { if let Some(url) = handle.ipc_endpoint() { info!(target: "reth::cli", url=%url, "RPC IPC server started"); } diff --git a/bin/reth/src/cli/components.rs b/bin/reth/src/cli/components.rs index 0db43553eca4..1f51409cbce7 100644 --- a/bin/reth/src/cli/components.rs +++ b/bin/reth/src/cli/components.rs @@ -6,6 +6,7 @@ use reth_provider::{ AccountReader, BlockReaderIdExt, CanonStateSubscriptions, ChainSpecProvider, ChangeSetReader, EvmEnvProvider, StateProviderFactory, }; +use reth_rpc_builder::{RethModuleRegistry, TransportRpcModules}; use reth_tasks::TaskSpawner; use reth_transaction_pool::TransactionPool; use std::sync::Arc; @@ -71,6 +72,31 @@ pub trait RethNodeComponents { } } +/// Helper container to encapsulate [RethModuleRegistry] and [TransportRpcModules]. +/// +/// This can be used to access installed modules, or create commonly used handlers like +/// [reth_rpc::EthApi], and ultimately merge additional rpc handler into the configured transport +/// modules [TransportRpcModules]. +#[derive(Debug)] +#[allow(clippy::type_complexity)] +pub struct RethRpcComponents<'a, Reth: RethNodeComponents> { + /// A Helper type the holds instances of the configured modules. + /// + /// This provides easy access to rpc handlers, such as [RethModuleRegistry::eth_api]. + pub registry: &'a mut RethModuleRegistry< + Reth::Provider, + Reth::Pool, + Reth::Network, + Reth::Tasks, + Reth::Events, + >, + /// Holds installed modules per transport type. + /// + /// This can be used to merge additional modules into the configured transports (http, ipc, + /// ws). See [TransportRpcModules::merge_configured] + pub modules: &'a mut TransportRpcModules, +} + /// A Generic implementation of the RethNodeComponents trait. #[derive(Clone, Debug)] #[allow(missing_docs)] diff --git a/bin/reth/src/cli/ext.rs b/bin/reth/src/cli/ext.rs index f8905d037639..c61ef3a9a181 100644 --- a/bin/reth/src/cli/ext.rs +++ b/bin/reth/src/cli/ext.rs @@ -1,13 +1,12 @@ //! Support for integrating customizations into the CLI. use crate::cli::{ - components::RethNodeComponents, + components::{RethNodeComponents, RethRpcComponents}, config::{PayloadBuilderConfig, RethRpcConfig}, }; use clap::Args; use reth_basic_payload_builder::{BasicPayloadJobGenerator, BasicPayloadJobGeneratorConfig}; use reth_payload_builder::{PayloadBuilderHandle, PayloadBuilderService}; -use reth_rpc_builder::{RethModuleRegistry, TransportRpcModules}; use reth_tasks::TaskSpawner; use std::fmt; @@ -49,21 +48,13 @@ pub trait RethNodeCommandConfig: fmt::Debug { /// Allows for registering additional RPC modules for the transports. /// - /// This is expected to call the merge functions of [TransportRpcModules], for example - /// [TransportRpcModules::merge_configured] - #[allow(clippy::type_complexity)] + /// This is expected to call the merge functions of [reth_rpc_builder::TransportRpcModules], for + /// example [reth_rpc_builder::TransportRpcModules::merge_configured] fn extend_rpc_modules( &mut self, config: &Conf, components: &Reth, - registry: &mut RethModuleRegistry< - Reth::Provider, - Reth::Pool, - Reth::Network, - Reth::Tasks, - Reth::Events, - >, - modules: &mut TransportRpcModules, + rpc_components: RethRpcComponents<'_, Reth>, ) -> eyre::Result<()> where Conf: RethRpcConfig, @@ -71,8 +62,7 @@ pub trait RethNodeCommandConfig: fmt::Debug { { let _ = config; let _ = components; - let _ = registry; - let _ = modules; + let _ = rpc_components; Ok(()) } @@ -196,21 +186,14 @@ impl RethNodeCommandConfig for NoArgs { &mut self, config: &Conf, components: &Reth, - registry: &mut RethModuleRegistry< - Reth::Provider, - Reth::Pool, - Reth::Network, - Reth::Tasks, - Reth::Events, - >, - modules: &mut TransportRpcModules, + rpc_components: RethRpcComponents<'_, Reth>, ) -> eyre::Result<()> where Conf: RethRpcConfig, Reth: RethNodeComponents, { if let Some(conf) = self.inner_mut() { - conf.extend_rpc_modules(config, components, registry, modules) + conf.extend_rpc_modules(config, components, rpc_components) } else { Ok(()) } diff --git a/examples/additional-rpc-namespace-in-cli/src/main.rs b/examples/additional-rpc-namespace-in-cli/src/main.rs index 9823fef03f1a..ce03aa5c8e3e 100644 --- a/examples/additional-rpc-namespace-in-cli/src/main.rs +++ b/examples/additional-rpc-namespace-in-cli/src/main.rs @@ -13,14 +13,11 @@ //! ``` use clap::Parser; use jsonrpsee::{core::RpcResult, proc_macros::rpc}; -use reth::{ - cli::{ - components::RethNodeComponents, - config::RethRpcConfig, - ext::{RethCliExt, RethNodeCommandConfig}, - Cli, - }, - rpc::builder::{RethModuleRegistry, TransportRpcModules}, +use reth::cli::{ + components::{RethNodeComponents, RethRpcComponents}, + config::RethRpcConfig, + ext::{RethCliExt, RethNodeCommandConfig}, + Cli, }; use reth_transaction_pool::TransactionPool; @@ -50,14 +47,7 @@ impl RethNodeCommandConfig for RethCliTxpoolExt { &mut self, _config: &Conf, _components: &Reth, - registry: &mut RethModuleRegistry< - Reth::Provider, - Reth::Pool, - Reth::Network, - Reth::Tasks, - Reth::Events, - >, - modules: &mut TransportRpcModules, + rpc_components: RethRpcComponents<'_, Reth>, ) -> eyre::Result<()> where Conf: RethRpcConfig, @@ -68,11 +58,11 @@ impl RethNodeCommandConfig for RethCliTxpoolExt { } // here we get the configured pool type from the CLI. - let pool = registry.pool().clone(); + let pool = rpc_components.registry.pool().clone(); let ext = TxpoolExt { pool }; // now we merge our extension namespace into all configured transports - modules.merge_configured(ext.into_rpc())?; + rpc_components.modules.merge_configured(ext.into_rpc())?; println!("txpool extension enabled"); Ok(()) From 442f28a8f2c5235ef83c843531d92e397eacbe64 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 5 Oct 2023 19:09:55 +0200 Subject: [PATCH 02/23] chore: rm redundant if empty check (#4923) --- crates/payload/basic/src/lib.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index 7c5e7065f9bb..449d636b50e6 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -861,10 +861,8 @@ where let mut payload = BuiltPayload::new(attributes.id, sealed_block, total_fees); - if !blob_sidecars.is_empty() { - // extend the payload with the blob sidecars from the executed txs - payload.extend_sidecars(blob_sidecars); - } + // extend the payload with the blob sidecars from the executed txs + payload.extend_sidecars(blob_sidecars); Ok(BuildOutcome::Better { payload, cached_reads }) } From a9b09a7552ffa3b6d5df9ba41f34d34f023bdf59 Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Thu, 5 Oct 2023 21:34:02 +0300 Subject: [PATCH 03/23] chore: `BlobsBundle::take` (#4925) --- crates/rpc/rpc-types/src/eth/engine/payload.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/rpc/rpc-types/src/eth/engine/payload.rs b/crates/rpc/rpc-types/src/eth/engine/payload.rs index 316f760fbeeb..98a43d9f6980 100644 --- a/crates/rpc/rpc-types/src/eth/engine/payload.rs +++ b/crates/rpc/rpc-types/src/eth/engine/payload.rs @@ -242,6 +242,21 @@ impl From> for BlobsBundleV1 { } } +impl BlobsBundleV1 { + /// Take `len` blob data from the bundle. + /// + /// # Panics + /// + /// If len is more than the blobs bundle len. + pub fn take(&mut self, len: usize) -> (Vec, Vec, Vec) { + ( + self.commitments.drain(0..len).collect(), + self.proofs.drain(0..len).collect(), + self.blobs.drain(0..len).collect(), + ) + } +} + /// An execution payload, which can be either [ExecutionPayloadV1], [ExecutionPayloadV2], or /// [ExecutionPayloadV3]. #[derive(Clone, Debug, PartialEq, Eq)] From 12c0bde9ecc6a7c388a4482fbeb76d1d091af2b6 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 5 Oct 2023 21:09:30 +0200 Subject: [PATCH 04/23] feat: add ipc path to config (#4920) --- bin/reth/src/args/gas_price_oracle_args.rs | 2 +- bin/reth/src/args/rpc_server_args.rs | 14 ++++++++++---- bin/reth/src/cli/config.rs | 3 +++ crates/rpc/rpc-builder/src/auth.rs | 5 +++++ 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/bin/reth/src/args/gas_price_oracle_args.rs b/bin/reth/src/args/gas_price_oracle_args.rs index d56c8ec8cd7f..9d417f348159 100644 --- a/bin/reth/src/args/gas_price_oracle_args.rs +++ b/bin/reth/src/args/gas_price_oracle_args.rs @@ -1,7 +1,7 @@ use clap::Args; /// Parameters to configure Gas Price Oracle -#[derive(Debug, Args, PartialEq, Eq, Default)] +#[derive(Debug, Clone, Args, PartialEq, Eq, Default)] #[command(next_help_heading = "Gas Price Oracle")] pub struct GasPriceOracleArgs { /// Number of recent blocks to check for gas price diff --git a/bin/reth/src/args/rpc_server_args.rs b/bin/reth/src/args/rpc_server_args.rs index aa12adaacffd..fe936049a68e 100644 --- a/bin/reth/src/args/rpc_server_args.rs +++ b/bin/reth/src/args/rpc_server_args.rs @@ -2,7 +2,11 @@ use crate::{ args::GasPriceOracleArgs, - cli::{components::RethRpcComponents, config::RethRpcConfig, ext::RethNodeCommandConfig}, + cli::{ + components::{RethNodeComponents, RethRpcComponents}, + config::RethRpcConfig, + ext::RethNodeCommandConfig, + }, }; use clap::{ builder::{PossibleValue, RangedU64ValueParser, TypedValueParser}, @@ -24,8 +28,6 @@ use reth_rpc::{ }, JwtError, JwtSecret, }; - -use crate::cli::components::RethNodeComponents; use reth_rpc_builder::{ auth::{AuthServerConfig, AuthServerHandle}, constants, @@ -55,7 +57,7 @@ pub(crate) const RPC_DEFAULT_MAX_RESPONSE_SIZE_MB: u32 = 115; pub(crate) const RPC_DEFAULT_MAX_CONNECTIONS: u32 = 500; /// Parameters for configuring the rpc more granularity via CLI -#[derive(Debug, Args)] +#[derive(Debug, Clone, Args)] #[command(next_help_heading = "RPC")] pub struct RpcServerArgs { /// Enable the HTTP-RPC server @@ -309,6 +311,10 @@ impl RethRpcConfig for RpcServerArgs { !self.ipcdisable } + fn ipc_path(&self) -> &str { + self.ipcpath.as_str() + } + fn eth_config(&self) -> EthConfig { EthConfig::default() .max_tracing_requests(self.rpc_max_tracing_requests) diff --git a/bin/reth/src/cli/config.rs b/bin/reth/src/cli/config.rs index 5c10a001a374..01eb395e9f67 100644 --- a/bin/reth/src/cli/config.rs +++ b/bin/reth/src/cli/config.rs @@ -17,6 +17,9 @@ pub trait RethRpcConfig { /// Returns whether ipc is enabled. fn is_ipc_enabled(&self) -> bool; + /// Returns the path to the target ipc socket if enabled. + fn ipc_path(&self) -> &str; + /// The configured ethereum RPC settings. fn eth_config(&self) -> EthConfig; diff --git a/crates/rpc/rpc-builder/src/auth.rs b/crates/rpc/rpc-builder/src/auth.rs index cfe545918896..ee8d53ff312e 100644 --- a/crates/rpc/rpc-builder/src/auth.rs +++ b/crates/rpc/rpc-builder/src/auth.rs @@ -141,6 +141,11 @@ impl AuthServerConfig { AuthServerConfigBuilder::new(secret) } + /// Returns the address the server will listen on. + pub fn address(&self) -> SocketAddr { + self.socket_addr + } + /// Convenience function to start a server in one step. pub async fn start(self, module: AuthRpcModule) -> Result { let Self { socket_addr, secret, server_config } = self; From ec543ab2b6f2cf8fb5abf019487e2f124c04c320 Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Thu, 5 Oct 2023 22:39:10 +0300 Subject: [PATCH 05/23] chore(payload): additional debug logs (#4926) --- crates/payload/basic/src/lib.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index 449d636b50e6..b82fc960af01 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -314,7 +314,7 @@ where // check if the deadline is reached if this.deadline.as_mut().poll(cx).is_ready() { - trace!("Payload building deadline reached"); + trace!(target: "payload_builder", "payload building deadline reached"); return Poll::Ready(Ok(())) } @@ -322,7 +322,7 @@ where while this.interval.poll_tick(cx).is_ready() { // start a new job if there is no pending block and we haven't reached the deadline if this.pending_block.is_none() { - trace!("spawn new payload build task"); + trace!(target: "payload_builder", "spawn new payload build task"); let (tx, rx) = oneshot::channel(); let client = this.client.clone(); let pool = this.pool.clone(); @@ -360,22 +360,22 @@ where match outcome { BuildOutcome::Better { payload, cached_reads } => { this.cached_reads = Some(cached_reads); - trace!("built better payload"); + trace!(target: "payload_builder", value = %payload.fees(), "built better payload"); let payload = Arc::new(payload); this.best_payload = Some(payload); } BuildOutcome::Aborted { fees, cached_reads } => { this.cached_reads = Some(cached_reads); - trace!(?fees, "skipped payload build of worse block"); + trace!(target: "payload_builder", worse_fees = %fees, "skipped payload build of worse block"); } BuildOutcome::Cancelled => { unreachable!("the cancel signal never fired") } } } - Poll::Ready(Err(err)) => { + Poll::Ready(Err(error)) => { // job failed, but we simply try again next interval - trace!(?err, "payload build attempt failed"); + trace!(target: "payload_builder", ?error, "payload build attempt failed"); this.metrics.inc_failed_payload_builds(); } Poll::Pending => { @@ -653,7 +653,7 @@ where chain_spec, } = config; - debug!(parent_hash=?parent_block.hash, parent_number=parent_block.number, "building new payload"); + debug!(target: "payload_builder", parent_hash = ?parent_block.hash, parent_number = parent_block.number, "building new payload"); let mut cumulative_gas_used = 0; let mut sum_blob_gas_used = 0; let block_gas_limit: u64 = initialized_block_env.gas_limit.try_into().unwrap_or(u64::MAX); @@ -725,11 +725,11 @@ where EVMError::Transaction(err) => { if matches!(err, InvalidTransaction::NonceTooLow { .. }) { // if the nonce is too low, we can skip this transaction - trace!(?err, ?tx, "skipping nonce too low transaction"); + trace!(target: "payload_builder", ?err, ?tx, "skipping nonce too low transaction"); } else { // if the transaction is invalid, we can skip it and all of its // descendants - trace!(?err, ?tx, "skipping invalid transaction and its descendants"); + trace!(target: "payload_builder", ?err, ?tx, "skipping invalid transaction and its descendants"); best_txs.mark_invalid(&pool_tx); } @@ -858,6 +858,7 @@ where let block = Block { header, body: executed_txs, ommers: vec![], withdrawals }; let sealed_block = block.seal_slow(); + debug!(target: "payload_builder", ?sealed_block, "sealed built block"); let mut payload = BuiltPayload::new(attributes.id, sealed_block, total_fees); @@ -884,7 +885,7 @@ where initialized_cfg, } = config; - debug!(parent_hash=?parent_block.hash, parent_number=parent_block.number, "building empty payload"); + debug!(target: "payload_builder", parent_hash = ?parent_block.hash, parent_number = parent_block.number, "building empty payload"); let state = client.state_by_block_hash(parent_block.hash)?; let mut db = State::builder() From 50b5215f4bbeed49682da2cfe0423c965f226acc Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Thu, 5 Oct 2023 20:47:15 +0100 Subject: [PATCH 06/23] fix(engine): always push back hooks, add tests (#4924) --- .../beacon/src/engine/hooks/controller.rs | 279 +++++++++++++++++- .../consensus/beacon/src/engine/hooks/mod.rs | 2 +- 2 files changed, 270 insertions(+), 11 deletions(-) diff --git a/crates/consensus/beacon/src/engine/hooks/controller.rs b/crates/consensus/beacon/src/engine/hooks/controller.rs index 1848211a3d2b..73302e5080b8 100644 --- a/crates/consensus/beacon/src/engine/hooks/controller.rs +++ b/crates/consensus/beacon/src/engine/hooks/controller.rs @@ -10,6 +10,8 @@ use tracing::debug; #[derive(Debug)] pub(crate) struct PolledHook { + #[allow(unused)] + pub(crate) name: &'static str, pub(crate) event: EngineHookEvent, pub(crate) action: Option, pub(crate) db_access_level: EngineHookDBAccessLevel, @@ -56,7 +58,12 @@ impl EngineHooksController { match hook.poll(cx, args)? { Poll::Ready((event, action)) => { - let result = PolledHook { event, action, db_access_level: hook.db_access_level() }; + let result = PolledHook { + name: hook.name(), + event, + action, + db_access_level: hook.db_access_level(), + }; debug!( target: "consensus::engine::hooks", @@ -101,6 +108,33 @@ impl EngineHooksController { ) -> Poll> { let Some(mut hook) = self.hooks.pop_front() else { return Poll::Pending }; + let result = self.poll_next_hook_inner(cx, &mut hook, args, db_write_active); + + if matches!( + result, + Poll::Ready(Ok(PolledHook { + event: EngineHookEvent::Started, + db_access_level: EngineHookDBAccessLevel::ReadWrite, + .. + })) + ) { + // If a read-write hook started, set `running_hook_with_db_write` to it + self.running_hook_with_db_write = Some(hook); + } else { + // Otherwise, push it back to the collection of hooks to poll it next time + self.hooks.push_back(hook); + } + + result + } + + fn poll_next_hook_inner( + &mut self, + cx: &mut Context<'_>, + hook: &mut Box, + args: EngineContext, + db_write_active: bool, + ) -> Poll> { // Hook with DB write access level is not allowed to run due to already running hook with DB // write access level or active DB write according to passed argument if hook.db_access_level().is_read_write() && @@ -110,7 +144,12 @@ impl EngineHooksController { } if let Poll::Ready((event, action)) = hook.poll(cx, args)? { - let result = PolledHook { event, action, db_access_level: hook.db_access_level() }; + let result = PolledHook { + name: hook.name(), + event, + action, + db_access_level: hook.db_access_level(), + }; debug!( target: "consensus::engine::hooks", @@ -119,15 +158,7 @@ impl EngineHooksController { "Polled next hook" ); - if result.event.is_started() && result.db_access_level.is_read_write() { - self.running_hook_with_db_write = Some(hook); - } else { - self.hooks.push_back(hook); - } - return Poll::Ready(Ok(result)) - } else { - self.hooks.push_back(hook); } Poll::Pending @@ -138,3 +169,231 @@ impl EngineHooksController { self.running_hook_with_db_write.is_some() } } + +#[cfg(test)] +mod tests { + use crate::hooks::{ + EngineContext, EngineHook, EngineHookAction, EngineHookDBAccessLevel, EngineHookEvent, + EngineHooks, EngineHooksController, + }; + use futures::poll; + use reth_interfaces::{RethError, RethResult}; + use std::{ + collections::VecDeque, + future::poll_fn, + task::{Context, Poll}, + }; + + struct TestHook { + results: VecDeque)>>, + name: &'static str, + access_level: EngineHookDBAccessLevel, + } + + impl TestHook { + fn new_ro(name: &'static str) -> Self { + Self { + results: Default::default(), + name, + access_level: EngineHookDBAccessLevel::ReadOnly, + } + } + fn new_rw(name: &'static str) -> Self { + Self { + results: Default::default(), + name, + access_level: EngineHookDBAccessLevel::ReadWrite, + } + } + + fn add_result(&mut self, result: RethResult<(EngineHookEvent, Option)>) { + self.results.push_back(result); + } + } + + impl EngineHook for TestHook { + fn name(&self) -> &'static str { + self.name + } + + fn poll( + &mut self, + _cx: &mut Context<'_>, + _ctx: EngineContext, + ) -> Poll)>> { + self.results.pop_front().map_or(Poll::Pending, Poll::Ready) + } + + fn db_access_level(&self) -> EngineHookDBAccessLevel { + self.access_level + } + } + + #[tokio::test] + async fn poll_running_hook_with_db_write() { + let mut controller = EngineHooksController::new(EngineHooks::new()); + + let context = EngineContext { tip_block_number: 2, finalized_block_number: Some(1) }; + + // No currently running hook with DB write access is set + let result = poll!(poll_fn(|cx| controller.poll_running_hook_with_db_write(cx, context))); + assert!(result.is_pending()); + + // Currently running hook with DB write access returned `Pending` on polling + controller.running_hook_with_db_write = Some(Box::new(TestHook::new_rw("read-write"))); + + let result = poll!(poll_fn(|cx| controller.poll_running_hook_with_db_write(cx, context))); + assert!(result.is_pending()); + + // Currently running hook with DB write access returned `Ready` on polling, but didn't + // return `EngineHookEvent::Finished` yet. + // Currently running hooks with DB write should still be set. + let mut hook = TestHook::new_rw("read-write"); + hook.add_result(Ok((EngineHookEvent::Started, None))); + controller.running_hook_with_db_write = Some(Box::new(hook)); + + let result = poll!(poll_fn(|cx| controller.poll_running_hook_with_db_write(cx, context))); + assert_eq!( + result.map(|result| { + let polled_hook = result.unwrap(); + polled_hook.event.is_started() && + polled_hook.action.is_none() && + polled_hook.db_access_level.is_read_write() + }), + Poll::Ready(true) + ); + assert!(controller.running_hook_with_db_write.is_some()); + assert!(controller.hooks.is_empty()); + + // Currently running hook with DB write access returned `Ready` on polling and + // `EngineHookEvent::Finished` inside. + // Currently running hooks with DB write should be moved to collection of hooks. + let mut hook = TestHook::new_rw("read-write"); + hook.add_result(Ok((EngineHookEvent::Finished(Ok(())), None))); + controller.running_hook_with_db_write = Some(Box::new(hook)); + + let result = poll!(poll_fn(|cx| controller.poll_running_hook_with_db_write(cx, context))); + assert_eq!( + result.map(|result| { + let polled_hook = result.unwrap(); + polled_hook.event.is_finished() && + polled_hook.action.is_none() && + polled_hook.db_access_level.is_read_write() + }), + Poll::Ready(true) + ); + assert!(controller.running_hook_with_db_write.is_none()); + assert!(controller.hooks.pop_front().is_some()); + } + + #[tokio::test] + async fn poll_next_hook_db_write_active() { + let context = EngineContext { tip_block_number: 2, finalized_block_number: Some(1) }; + + let mut hook_rw = TestHook::new_rw("read-write"); + hook_rw.add_result(Ok((EngineHookEvent::Started, None))); + + let hook_ro_name = "read-only"; + let mut hook_ro = TestHook::new_ro(hook_ro_name); + hook_ro.add_result(Ok((EngineHookEvent::Started, None))); + + let mut hooks = EngineHooks::new(); + hooks.add(hook_rw); + hooks.add(hook_ro); + let mut controller = EngineHooksController::new(hooks); + + // Read-write hook can't be polled when external DB write is active + let result = poll!(poll_fn(|cx| controller.poll_next_hook(cx, context, true))); + assert!(result.is_pending()); + assert!(controller.running_hook_with_db_write.is_none()); + + // Read-only hook can be polled when external DB write is active + let result = poll!(poll_fn(|cx| controller.poll_next_hook(cx, context, true))); + assert_eq!( + result.map(|result| { + let polled_hook = result.unwrap(); + polled_hook.name == hook_ro_name && + polled_hook.event.is_started() && + polled_hook.action.is_none() && + polled_hook.db_access_level.is_read_only() + }), + Poll::Ready(true) + ); + } + + #[tokio::test] + async fn poll_next_hook_db_write_inactive() { + let context = EngineContext { tip_block_number: 2, finalized_block_number: Some(1) }; + + let hook_rw_1_name = "read-write-1"; + let mut hook_rw_1 = TestHook::new_rw(hook_rw_1_name); + hook_rw_1.add_result(Ok((EngineHookEvent::Started, None))); + + let hook_rw_2_name = "read-write-2"; + let mut hook_rw_2 = TestHook::new_rw(hook_rw_2_name); + hook_rw_2.add_result(Ok((EngineHookEvent::Started, None))); + + let hook_ro_name = "read-only"; + let mut hook_ro = TestHook::new_ro(hook_ro_name); + hook_ro.add_result(Ok((EngineHookEvent::Started, None))); + hook_ro.add_result(Err(RethError::Custom("something went wrong".to_string()))); + + let mut hooks = EngineHooks::new(); + hooks.add(hook_rw_1); + hooks.add(hook_rw_2); + hooks.add(hook_ro); + + let mut controller = EngineHooksController::new(hooks); + let hooks_len = controller.hooks.len(); + + // Read-write hook can be polled because external DB write is not active + assert_eq!(controller.hooks.front().map(|hook| hook.name()), Some(hook_rw_1_name)); + let result = poll!(poll_fn(|cx| controller.poll_next_hook(cx, context, false))); + assert_eq!( + result.map(|result| { + let polled_hook = result.unwrap(); + polled_hook.name == hook_rw_1_name && + polled_hook.event.is_started() && + polled_hook.action.is_none() && + polled_hook.db_access_level.is_read_write() + }), + Poll::Ready(true) + ); + assert_eq!( + controller.running_hook_with_db_write.as_ref().map(|hook| hook.name()), + Some(hook_rw_1_name) + ); + + // Read-write hook cannot be polled because another read-write hook is running + assert_eq!(controller.hooks.front().map(|hook| hook.name()), Some(hook_rw_2_name)); + let result = poll!(poll_fn(|cx| controller.poll_next_hook(cx, context, false))); + assert!(result.is_pending()); + + // Read-only hook can be polled in parallel with already running read-write hook + assert_eq!(controller.hooks.front().map(|hook| hook.name()), Some(hook_ro_name)); + let result = poll!(poll_fn(|cx| controller.poll_next_hook(cx, context, false))); + assert_eq!( + result.map(|result| { + let polled_hook = result.unwrap(); + polled_hook.name == hook_ro_name && + polled_hook.event.is_started() && + polled_hook.action.is_none() && + polled_hook.db_access_level.is_read_only() + }), + Poll::Ready(true) + ); + + // Read-write hook still cannot be polled because another read-write hook is running + assert_eq!(controller.hooks.front().map(|hook| hook.name()), Some(hook_rw_2_name)); + let result = poll!(poll_fn(|cx| controller.poll_next_hook(cx, context, false))); + assert!(result.is_pending()); + + // Read-only hook has finished with error + assert_eq!(controller.hooks.front().map(|hook| hook.name()), Some(hook_ro_name)); + let result = poll!(poll_fn(|cx| controller.poll_next_hook(cx, context, false))); + assert_eq!(result.map(|result| { result.is_err() }), Poll::Ready(true)); + + assert!(controller.running_hook_with_db_write.is_some()); + assert_eq!(controller.hooks.len(), hooks_len - 1) + } +} diff --git a/crates/consensus/beacon/src/engine/hooks/mod.rs b/crates/consensus/beacon/src/engine/hooks/mod.rs index a4e4feab6fea..a619e99900d7 100644 --- a/crates/consensus/beacon/src/engine/hooks/mod.rs +++ b/crates/consensus/beacon/src/engine/hooks/mod.rs @@ -113,7 +113,7 @@ pub enum EngineHookError { } /// Level of database access the hook needs for execution. -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Copy, Clone, Eq, PartialEq)] pub enum EngineHookDBAccessLevel { /// Read-only database access. ReadOnly, From 49ac2f511004dc9ba3614dcb5eb420109eadb37c Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Thu, 5 Oct 2023 15:51:50 -0400 Subject: [PATCH 07/23] feat: add metrics to engine RPC API (#4912) --- Cargo.lock | 2 + crates/rpc/rpc-engine-api/Cargo.toml | 4 ++ crates/rpc/rpc-engine-api/src/engine_api.rs | 74 ++++++++++++++++----- crates/rpc/rpc-engine-api/src/lib.rs | 3 + crates/rpc/rpc-engine-api/src/metrics.rs | 32 +++++++++ 5 files changed, 100 insertions(+), 15 deletions(-) create mode 100644 crates/rpc/rpc-engine-api/src/metrics.rs diff --git a/Cargo.lock b/Cargo.lock index 4b5ed5ac2f89..980b6f16dc79 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6265,8 +6265,10 @@ dependencies = [ "async-trait", "jsonrpsee-core", "jsonrpsee-types", + "metrics", "reth-beacon-consensus", "reth-interfaces", + "reth-metrics", "reth-payload-builder", "reth-primitives", "reth-provider", diff --git a/crates/rpc/rpc-engine-api/Cargo.toml b/crates/rpc/rpc-engine-api/Cargo.toml index ebd0bfeb7854..ad2e876e6b8a 100644 --- a/crates/rpc/rpc-engine-api/Cargo.toml +++ b/crates/rpc/rpc-engine-api/Cargo.toml @@ -22,6 +22,10 @@ reth-rpc-types-compat.workspace = true # async tokio = { workspace = true, features = ["sync"] } +# metrics +reth-metrics.workspace = true +metrics.workspace = true + # misc async-trait.workspace = true thiserror.workspace = true diff --git a/crates/rpc/rpc-engine-api/src/engine_api.rs b/crates/rpc/rpc-engine-api/src/engine_api.rs index 1e7cea097983..5218e1b963cc 100644 --- a/crates/rpc/rpc-engine-api/src/engine_api.rs +++ b/crates/rpc/rpc-engine-api/src/engine_api.rs @@ -1,5 +1,6 @@ use crate::{ - payload::PayloadOrAttributes, EngineApiError, EngineApiMessageVersion, EngineApiResult, + metrics::EngineApiMetrics, payload::PayloadOrAttributes, EngineApiError, + EngineApiMessageVersion, EngineApiResult, }; use async_trait::async_trait; use jsonrpsee_core::RpcResult; @@ -19,7 +20,7 @@ use reth_rpc_types_compat::engine::payload::{ convert_payload_input_v2_to_payload, convert_to_payload_body_v1, }; use reth_tasks::TaskSpawner; -use std::sync::Arc; +use std::{sync::Arc, time::Instant}; use tokio::sync::oneshot; use tracing::trace; @@ -46,6 +47,8 @@ struct EngineApiInner { payload_store: PayloadStore, /// For spawning and executing async tasks task_spawner: Box, + /// The metrics for engine api calls + metrics: EngineApiMetrics, } impl EngineApi @@ -66,6 +69,7 @@ where beacon_consensus, payload_store, task_spawner, + metrics: EngineApiMetrics::default(), }); Self { inner } } @@ -591,14 +595,20 @@ where /// Caution: This should not accept the `withdrawals` field async fn new_payload_v1(&self, payload: ExecutionPayloadV1) -> RpcResult { trace!(target: "rpc::engine", "Serving engine_newPayloadV1"); - Ok(EngineApi::new_payload_v1(self, payload).await?) + let start = Instant::now(); + let res = EngineApi::new_payload_v1(self, payload).await; + self.inner.metrics.new_payload_v1.record(start.elapsed()); + Ok(res?) } /// Handler for `engine_newPayloadV2` /// See also async fn new_payload_v2(&self, payload: ExecutionPayloadInputV2) -> RpcResult { trace!(target: "rpc::engine", "Serving engine_newPayloadV2"); - Ok(EngineApi::new_payload_v2(self, payload).await?) + let start = Instant::now(); + let res = EngineApi::new_payload_v2(self, payload).await; + self.inner.metrics.new_payload_v2.record(start.elapsed()); + Ok(res?) } /// Handler for `engine_newPayloadV3` @@ -610,8 +620,12 @@ where parent_beacon_block_root: B256, ) -> RpcResult { trace!(target: "rpc::engine", "Serving engine_newPayloadV3"); - Ok(EngineApi::new_payload_v3(self, payload, versioned_hashes, parent_beacon_block_root) - .await?) + let start = Instant::now(); + let res = + EngineApi::new_payload_v3(self, payload, versioned_hashes, parent_beacon_block_root) + .await; + self.inner.metrics.new_payload_v3.record(start.elapsed()); + Ok(res?) } /// Handler for `engine_forkchoiceUpdatedV1` @@ -624,7 +638,11 @@ where payload_attributes: Option, ) -> RpcResult { trace!(target: "rpc::engine", "Serving engine_forkchoiceUpdatedV1"); - Ok(EngineApi::fork_choice_updated_v1(self, fork_choice_state, payload_attributes).await?) + let start = Instant::now(); + let res = + EngineApi::fork_choice_updated_v1(self, fork_choice_state, payload_attributes).await; + self.inner.metrics.fork_choice_updated_v1.record(start.elapsed()); + Ok(res?) } /// Handler for `engine_forkchoiceUpdatedV2` @@ -635,7 +653,11 @@ where payload_attributes: Option, ) -> RpcResult { trace!(target: "rpc::engine", "Serving engine_forkchoiceUpdatedV2"); - Ok(EngineApi::fork_choice_updated_v2(self, fork_choice_state, payload_attributes).await?) + let start = Instant::now(); + let res = + EngineApi::fork_choice_updated_v2(self, fork_choice_state, payload_attributes).await; + self.inner.metrics.fork_choice_updated_v2.record(start.elapsed()); + Ok(res?) } /// Handler for `engine_forkchoiceUpdatedV2` @@ -647,7 +669,11 @@ where payload_attributes: Option, ) -> RpcResult { trace!(target: "rpc::engine", "Serving engine_forkchoiceUpdatedV3"); - Ok(EngineApi::fork_choice_updated_v3(self, fork_choice_state, payload_attributes).await?) + let start = Instant::now(); + let res = + EngineApi::fork_choice_updated_v3(self, fork_choice_state, payload_attributes).await; + self.inner.metrics.fork_choice_updated_v3.record(start.elapsed()); + Ok(res?) } /// Handler for `engine_getPayloadV1` @@ -663,7 +689,10 @@ where /// > Provider software MAY stop the corresponding build process after serving this call. async fn get_payload_v1(&self, payload_id: PayloadId) -> RpcResult { trace!(target: "rpc::engine", "Serving engine_getPayloadV1"); - Ok(EngineApi::get_payload_v1(self, payload_id).await?) + let start = Instant::now(); + let res = EngineApi::get_payload_v1(self, payload_id).await; + self.inner.metrics.get_payload_v1.record(start.elapsed()); + Ok(res?) } /// Handler for `engine_getPayloadV2` @@ -677,7 +706,10 @@ where /// > Provider software MAY stop the corresponding build process after serving this call. async fn get_payload_v2(&self, payload_id: PayloadId) -> RpcResult { trace!(target: "rpc::engine", "Serving engine_getPayloadV2"); - Ok(EngineApi::get_payload_v2(self, payload_id).await?) + let start = Instant::now(); + let res = EngineApi::get_payload_v2(self, payload_id).await; + self.inner.metrics.get_payload_v2.record(start.elapsed()); + Ok(res?) } /// Handler for `engine_getPayloadV3` @@ -691,7 +723,10 @@ where /// > Provider software MAY stop the corresponding build process after serving this call. async fn get_payload_v3(&self, payload_id: PayloadId) -> RpcResult { trace!(target: "rpc::engine", "Serving engine_getPayloadV3"); - Ok(EngineApi::get_payload_v3(self, payload_id).await?) + let start = Instant::now(); + let res = EngineApi::get_payload_v3(self, payload_id).await; + self.inner.metrics.get_payload_v3.record(start.elapsed()); + Ok(res?) } /// Handler for `engine_getPayloadBodiesByHashV1` @@ -701,7 +736,10 @@ where block_hashes: Vec, ) -> RpcResult { trace!(target: "rpc::engine", "Serving engine_getPayloadBodiesByHashV1"); - Ok(EngineApi::get_payload_bodies_by_hash(self, block_hashes)?) + let start = Instant::now(); + let res = EngineApi::get_payload_bodies_by_hash(self, block_hashes); + self.inner.metrics.get_payload_bodies_by_hash_v1.record(start.elapsed()); + Ok(res?) } /// Handler for `engine_getPayloadBodiesByRangeV1` @@ -726,7 +764,10 @@ where count: U64, ) -> RpcResult { trace!(target: "rpc::engine", "Serving engine_getPayloadBodiesByRangeV1"); - Ok(EngineApi::get_payload_bodies_by_range(self, start.to(), count.to()).await?) + let start_time = Instant::now(); + let res = EngineApi::get_payload_bodies_by_range(self, start.to(), count.to()).await; + self.inner.metrics.get_payload_bodies_by_range_v1.record(start_time.elapsed()); + Ok(res?) } /// Handler for `engine_exchangeTransitionConfigurationV1` @@ -736,7 +777,10 @@ where config: TransitionConfiguration, ) -> RpcResult { trace!(target: "rpc::engine", "Serving engine_exchangeTransitionConfigurationV1"); - Ok(EngineApi::exchange_transition_configuration(self, config).await?) + let start = Instant::now(); + let res = EngineApi::exchange_transition_configuration(self, config).await; + self.inner.metrics.exchange_transition_configuration.record(start.elapsed()); + Ok(res?) } /// Handler for `engine_exchangeCapabilitiesV1` diff --git a/crates/rpc/rpc-engine-api/src/lib.rs b/crates/rpc/rpc-engine-api/src/lib.rs index c75a34021ab9..d39025d508d0 100644 --- a/crates/rpc/rpc-engine-api/src/lib.rs +++ b/crates/rpc/rpc-engine-api/src/lib.rs @@ -22,6 +22,9 @@ mod payload; /// Engine API error. mod error; +/// Engine API metrics. +mod metrics; + pub use engine_api::{EngineApi, EngineApiSender}; pub use error::*; pub use message::EngineApiMessageVersion; diff --git a/crates/rpc/rpc-engine-api/src/metrics.rs b/crates/rpc/rpc-engine-api/src/metrics.rs new file mode 100644 index 000000000000..9df8ff7900e9 --- /dev/null +++ b/crates/rpc/rpc-engine-api/src/metrics.rs @@ -0,0 +1,32 @@ +use metrics::Histogram; +use reth_metrics::Metrics; + +/// Beacon consensus engine metrics. +#[derive(Metrics)] +#[metrics(scope = "engine.rpc")] +pub(crate) struct EngineApiMetrics { + /// Latency for `engine_newPayloadV1` + pub(crate) new_payload_v1: Histogram, + /// Latency for `engine_newPayloadV2` + pub(crate) new_payload_v2: Histogram, + /// Latency for `engine_newPayloadV3` + pub(crate) new_payload_v3: Histogram, + /// Latency for `engine_forkchoiceUpdatedV1` + pub(crate) fork_choice_updated_v1: Histogram, + /// Latency for `engine_forkchoiceUpdatedV2` + pub(crate) fork_choice_updated_v2: Histogram, + /// Latency for `engine_forkchoiceUpdatedV3` + pub(crate) fork_choice_updated_v3: Histogram, + /// Latency for `engine_getPayloadV1` + pub(crate) get_payload_v1: Histogram, + /// Latency for `engine_getPayloadV2` + pub(crate) get_payload_v2: Histogram, + /// Latency for `engine_getPayloadV3` + pub(crate) get_payload_v3: Histogram, + /// Latency for `engine_getPayloadBodiesByRangeV1` + pub(crate) get_payload_bodies_by_range_v1: Histogram, + /// Latency for `engine_getPayloadBodiesByHashV1` + pub(crate) get_payload_bodies_by_hash_v1: Histogram, + /// Latency for `engine_exchangeTransitionConfigurationV1` + pub(crate) exchange_transition_configuration: Histogram, +} From f18845d2518df8666c2f044547f50c1895b7d5c0 Mon Sep 17 00:00:00 2001 From: DoTheBestToGetTheBest <146037313+DoTheBestToGetTheBest@users.noreply.github.com> Date: Thu, 5 Oct 2023 15:46:01 -0700 Subject: [PATCH 08/23] impl for Eip2930 (#4927) Co-authored-by: Matthias Seitz --- .../transaction-pool/src/test_utils/mock.rs | 68 +++++++++++++++++-- 1 file changed, 63 insertions(+), 5 deletions(-) diff --git a/crates/transaction-pool/src/test_utils/mock.rs b/crates/transaction-pool/src/test_utils/mock.rs index c6e3d01bf5a6..f764fad32b9f 100644 --- a/crates/transaction-pool/src/test_utils/mock.rs +++ b/crates/transaction-pool/src/test_utils/mock.rs @@ -16,8 +16,8 @@ use reth_primitives::{ hex, AccessList, Address, Bytes, FromRecoveredPooledTransaction, FromRecoveredTransaction, IntoRecoveredTransaction, PooledTransactionsElementEcRecovered, Signature, Transaction, TransactionKind, TransactionSigned, TransactionSignedEcRecovered, TxEip1559, TxEip2930, - TxEip4844, TxHash, TxLegacy, TxType, TxValue, B256, EIP1559_TX_TYPE_ID, EIP4844_TX_TYPE_ID, - LEGACY_TX_TYPE_ID, U128, U256, + TxEip4844, TxHash, TxLegacy, TxType, TxValue, B256, EIP1559_TX_TYPE_ID, EIP2930_TX_TYPE_ID, + EIP4844_TX_TYPE_ID, LEGACY_TX_TYPE_ID, U128, U256, }; use std::{ops::Range, sync::Arc, time::Instant}; @@ -44,6 +44,9 @@ macro_rules! set_value { MockTransaction::Eip4844 { ref mut $field, .. } => { *$field = new_value; } + MockTransaction::Eip2930 { ref mut $field, .. } => { + *$field = new_value; + } } }; } @@ -55,6 +58,7 @@ macro_rules! get_value { MockTransaction::Legacy { $field, .. } => $field, MockTransaction::Eip1559 { $field, .. } => $field, MockTransaction::Eip4844 { $field, .. } => $field, + MockTransaction::Eip2930 { $field, .. } => $field, } }; } @@ -118,6 +122,17 @@ pub enum MockTransaction { accesslist: AccessList, input: Bytes, }, + Eip2930 { + hash: B256, + sender: Address, + nonce: u64, + to: TransactionKind, + gas_limit: u64, + input: Bytes, + value: U256, + gas_price: u128, + accesslist: AccessList, + }, } // === impl MockTransaction === @@ -251,6 +266,9 @@ impl MockTransaction { MockTransaction::Eip4844 { accesslist, .. } => { *accesslist = list; } + MockTransaction::Eip2930 { accesslist, .. } => { + *accesslist = list; + } } self } @@ -268,6 +286,7 @@ impl MockTransaction { *max_fee_per_gas = val; *max_priority_fee_per_gas = val; } + MockTransaction::Eip2930 { gas_price, .. } => *gas_price = val, } self } @@ -293,6 +312,9 @@ impl MockTransaction { *max_fee_per_gas = val; *max_priority_fee_per_gas = val; } + MockTransaction::Eip2930 { ref mut gas_price, .. } => { + *gas_price = val; + } } self } @@ -302,6 +324,7 @@ impl MockTransaction { MockTransaction::Legacy { gas_price, .. } => *gas_price, MockTransaction::Eip1559 { max_fee_per_gas, .. } => *max_fee_per_gas, MockTransaction::Eip4844 { max_fee_per_gas, .. } => *max_fee_per_gas, + MockTransaction::Eip2930 { gas_price, .. } => *gas_price, } } @@ -377,6 +400,7 @@ impl MockTransaction { Self::Legacy { .. } => LEGACY_TX_TYPE_ID, Self::Eip1559 { .. } => EIP1559_TX_TYPE_ID, Self::Eip4844 { .. } => EIP4844_TX_TYPE_ID, + Self::Eip2930 { .. } => EIP2930_TX_TYPE_ID, } } @@ -391,6 +415,10 @@ impl MockTransaction { pub fn is_eip4844(&self) -> bool { matches!(self, MockTransaction::Eip4844 { .. }) } + + pub fn is_eip2930(&self) -> bool { + matches!(self, MockTransaction::Eip2930 { .. }) + } } impl PoolTransaction for MockTransaction { @@ -399,6 +427,7 @@ impl PoolTransaction for MockTransaction { MockTransaction::Legacy { hash, .. } => hash, MockTransaction::Eip1559 { hash, .. } => hash, MockTransaction::Eip4844 { hash, .. } => hash, + MockTransaction::Eip2930 { hash, .. } => hash, } } @@ -407,6 +436,7 @@ impl PoolTransaction for MockTransaction { MockTransaction::Legacy { sender, .. } => *sender, MockTransaction::Eip1559 { sender, .. } => *sender, MockTransaction::Eip4844 { sender, .. } => *sender, + MockTransaction::Eip2930 { sender, .. } => *sender, } } @@ -415,6 +445,7 @@ impl PoolTransaction for MockTransaction { MockTransaction::Legacy { nonce, .. } => *nonce, MockTransaction::Eip1559 { nonce, .. } => *nonce, MockTransaction::Eip4844 { nonce, .. } => *nonce, + MockTransaction::Eip2930 { nonce, .. } => *nonce, } } @@ -429,6 +460,9 @@ impl PoolTransaction for MockTransaction { MockTransaction::Eip4844 { max_fee_per_gas, value, gas_limit, .. } => { U256::from(*gas_limit) * U256::from(*max_fee_per_gas) + *value } + MockTransaction::Eip2930 { gas_limit, gas_price, value, .. } => { + U256::from(*gas_limit) * U256::from(*gas_price) + *value + } } } @@ -441,6 +475,7 @@ impl PoolTransaction for MockTransaction { MockTransaction::Legacy { gas_price, .. } => *gas_price, MockTransaction::Eip1559 { max_fee_per_gas, .. } => *max_fee_per_gas, MockTransaction::Eip4844 { max_fee_per_gas, .. } => *max_fee_per_gas, + MockTransaction::Eip2930 { gas_price, .. } => *gas_price, } } @@ -449,6 +484,7 @@ impl PoolTransaction for MockTransaction { MockTransaction::Legacy { .. } => None, MockTransaction::Eip1559 { accesslist: accessslist, .. } => Some(accessslist), MockTransaction::Eip4844 { accesslist: accessslist, .. } => Some(accessslist), + MockTransaction::Eip2930 { accesslist, .. } => Some(accesslist), } } @@ -461,6 +497,7 @@ impl PoolTransaction for MockTransaction { MockTransaction::Eip4844 { max_priority_fee_per_gas, .. } => { Some(*max_priority_fee_per_gas) } + MockTransaction::Eip2930 { .. } => None, } } @@ -491,6 +528,7 @@ impl PoolTransaction for MockTransaction { MockTransaction::Legacy { gas_price, .. } => *gas_price, MockTransaction::Eip1559 { max_priority_fee_per_gas, .. } => *max_priority_fee_per_gas, MockTransaction::Eip4844 { max_priority_fee_per_gas, .. } => *max_priority_fee_per_gas, + MockTransaction::Eip2930 { gas_price, .. } => *gas_price, } } @@ -499,6 +537,7 @@ impl PoolTransaction for MockTransaction { MockTransaction::Legacy { to, .. } => to, MockTransaction::Eip1559 { to, .. } => to, MockTransaction::Eip4844 { to, .. } => to, + MockTransaction::Eip2930 { to, .. } => to, } } @@ -507,6 +546,7 @@ impl PoolTransaction for MockTransaction { MockTransaction::Legacy { .. } => &[], MockTransaction::Eip1559 { input, .. } => input, MockTransaction::Eip4844 { input, .. } => input, + MockTransaction::Eip2930 { input, .. } => input, } } @@ -519,6 +559,7 @@ impl PoolTransaction for MockTransaction { MockTransaction::Legacy { .. } => TxType::Legacy.into(), MockTransaction::Eip1559 { .. } => TxType::EIP1559.into(), MockTransaction::Eip4844 { .. } => TxType::EIP4844.into(), + MockTransaction::Eip2930 { .. } => TxType::EIP2930.into(), } } @@ -602,9 +643,26 @@ impl FromRecoveredTransaction for MockTransaction { input, accesslist: access_list, }, - Transaction::Eip2930 { .. } => { - unimplemented!() - } + Transaction::Eip2930(TxEip2930 { + chain_id: _, + nonce, + gas_price, + gas_limit, + to, + value, + input, + access_list, + }) => MockTransaction::Eip2930 { + hash, + sender, + nonce, + gas_price, + gas_limit, + to, + value: value.into(), + input, + accesslist: access_list, + }, } } } From 50fa71d8d82900fa4010afdae92cd474d11dbe6c Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Fri, 6 Oct 2023 11:34:38 +0300 Subject: [PATCH 09/23] feat(cli): debug build block (#4914) --- bin/reth/src/debug_cmd/build_block.rs | 279 ++++++++++++++++++++++++++ bin/reth/src/debug_cmd/mod.rs | 4 + crates/payload/basic/src/lib.rs | 61 ++++-- 3 files changed, 329 insertions(+), 15 deletions(-) create mode 100644 bin/reth/src/debug_cmd/build_block.rs diff --git a/bin/reth/src/debug_cmd/build_block.rs b/bin/reth/src/debug_cmd/build_block.rs new file mode 100644 index 000000000000..c55b49ea2ac9 --- /dev/null +++ b/bin/reth/src/debug_cmd/build_block.rs @@ -0,0 +1,279 @@ +//! Command for debugging block building. +use crate::{ + args::{utils::genesis_value_parser, DatabaseArgs}, + dirs::{DataDirPath, MaybePlatformPath}, + runner::CliContext, +}; +use alloy_rlp::Decodable; +use clap::Parser; +use eyre::Context; +use reth_basic_payload_builder::{ + default_payload_builder, BuildArguments, BuildOutcome, Cancelled, PayloadConfig, +}; +use reth_beacon_consensus::BeaconConsensus; +use reth_blockchain_tree::{ + BlockchainTree, BlockchainTreeConfig, ShareableBlockchainTree, TreeExternals, +}; +use reth_db::{init_db, DatabaseEnv}; +use reth_interfaces::{consensus::Consensus, RethResult}; +use reth_payload_builder::{database::CachedReads, PayloadBuilderAttributes}; +use reth_primitives::{ + constants::eip4844::{LoadKzgSettingsError, MAINNET_KZG_TRUSTED_SETUP}, + fs, + revm_primitives::KzgSettings, + stage::StageId, + Address, BlobTransactionSidecar, Bytes, ChainSpec, SealedBlock, SealedBlockWithSenders, + Transaction, TransactionSigned, TxEip4844, B256, U256, U64, +}; +use reth_provider::{ + providers::BlockchainProvider, BlockHashReader, BlockReader, BlockWriter, ExecutorFactory, + ProviderFactory, StageCheckpointReader, StateProviderFactory, +}; +use reth_revm::Factory; +use reth_rpc_types::engine::{BlobsBundleV1, PayloadAttributes}; +use reth_transaction_pool::{ + blobstore::InMemoryBlobStore, BlobStore, EthPooledTransaction, PoolConfig, TransactionOrigin, + TransactionPool, TransactionValidationTaskExecutor, +}; +use std::{path::PathBuf, str::FromStr, sync::Arc}; +use tracing::*; + +/// `reth debug build-block` command +/// This debug routine requires that the node is positioned at the block before the target. +/// The script will then parse the block and attempt to build a similar one. +#[derive(Debug, Parser)] +pub struct Command { + /// The path to the data dir for all reth files and subdirectories. + /// + /// Defaults to the OS-specific data directory: + /// + /// - Linux: `$XDG_DATA_HOME/reth/` or `$HOME/.local/share/reth/` + /// - Windows: `{FOLDERID_RoamingAppData}/reth/` + /// - macOS: `$HOME/Library/Application Support/reth/` + #[arg(long, value_name = "DATA_DIR", verbatim_doc_comment, default_value_t)] + datadir: MaybePlatformPath, + + /// The chain this node is running. + /// + /// Possible values are either a built-in chain or the path to a chain specification file. + /// + /// Built-in chains: + /// - mainnet + /// - goerli + /// - sepolia + /// - holesky + #[arg( + long, + value_name = "CHAIN_OR_PATH", + verbatim_doc_comment, + default_value = "mainnet", + value_parser = genesis_value_parser + )] + chain: Arc, + + /// Database arguments. + #[clap(flatten)] + db: DatabaseArgs, + + /// Overrides the KZG trusted setup by reading from the supplied file. + #[arg(long, value_name = "PATH")] + trusted_setup_file: Option, + + #[arg(long)] + parent_beacon_block_root: Option, + + #[arg(long)] + prev_randao: B256, + + #[arg(long)] + timestamp: u64, + + #[arg(long)] + suggested_fee_recipient: Address, + + /// Array of transactions. + /// NOTE: 4844 transactions must be provided in the same order as they appear in the blobs + /// bundle. + #[arg(long, value_delimiter = ',')] + transactions: Vec, + + /// Path to the file that contains a corresponding blobs bundle. + #[arg(long)] + blobs_bundle_path: Option, +} + +impl Command { + /// Fetches the best block block from the database. + /// + /// If the database is empty, returns the genesis block. + fn lookup_best_block(&self, db: Arc) -> RethResult> { + let factory = ProviderFactory::new(db, self.chain.clone()); + let provider = factory.provider()?; + + let best_number = + provider.get_stage_checkpoint(StageId::Finish)?.unwrap_or_default().block_number; + let best_hash = provider + .block_hash(best_number)? + .expect("the hash for the latest block is missing, database is corrupt"); + + Ok(Arc::new( + provider + .block(best_number.into())? + .expect("the header for the latest block is missing, database is corrupt") + .seal(best_hash), + )) + } + + /// Loads the trusted setup params from a given file path or falls back to + /// `MAINNET_KZG_TRUSTED_SETUP`. + fn kzg_settings(&self) -> eyre::Result> { + if let Some(ref trusted_setup_file) = self.trusted_setup_file { + let trusted_setup = KzgSettings::load_trusted_setup_file(trusted_setup_file) + .map_err(LoadKzgSettingsError::KzgError)?; + Ok(Arc::new(trusted_setup)) + } else { + Ok(Arc::clone(&MAINNET_KZG_TRUSTED_SETUP)) + } + } + + /// Execute `debug in-memory-merkle` command + pub async fn execute(self, ctx: CliContext) -> eyre::Result<()> { + // add network name to data dir + let data_dir = self.datadir.unwrap_or_chain_default(self.chain.chain); + let db_path = data_dir.db_path(); + fs::create_dir_all(&db_path)?; + + // initialize the database + let db = Arc::new(init_db(db_path, self.db.log_level)?); + + let consensus: Arc = Arc::new(BeaconConsensus::new(Arc::clone(&self.chain))); + + // configure blockchain tree + let tree_externals = TreeExternals::new( + Arc::clone(&db), + Arc::clone(&consensus), + Factory::new(self.chain.clone()), + Arc::clone(&self.chain), + ); + let _tree_config = BlockchainTreeConfig::default(); + let tree = BlockchainTree::new(tree_externals, BlockchainTreeConfig::default(), None)?; + let blockchain_tree = ShareableBlockchainTree::new(tree); + + // fetch the best block from the database + let best_block = + self.lookup_best_block(Arc::clone(&db)).wrap_err("the head block is missing")?; + + let factory = ProviderFactory::new(Arc::clone(&db), Arc::clone(&self.chain)); + let blockchain_db = BlockchainProvider::new(factory.clone(), blockchain_tree.clone())?; + let blob_store = InMemoryBlobStore::default(); + + let validator = TransactionValidationTaskExecutor::eth_builder(Arc::clone(&self.chain)) + .with_head_timestamp(best_block.timestamp) + .kzg_settings(self.kzg_settings()?) + .with_additional_tasks(1) + .build_with_tasks(blockchain_db.clone(), ctx.task_executor.clone(), blob_store.clone()); + + let transaction_pool = reth_transaction_pool::Pool::eth_pool( + validator, + blob_store.clone(), + PoolConfig::default(), + ); + info!(target: "reth::cli", "Transaction pool initialized"); + + let mut blobs_bundle = self + .blobs_bundle_path + .map(|path| -> eyre::Result { + let contents = std::fs::read_to_string(&path) + .wrap_err(format!("could not read {}", path.display()))?; + serde_json::from_str(&contents).wrap_err("failed to deserialize blobs bundle") + }) + .transpose()?; + + for tx_bytes in self.transactions.iter() { + debug!(target: "reth::cli", bytes = ?tx_bytes, "Decoding transaction"); + let transaction = TransactionSigned::decode(&mut &Bytes::from_str(tx_bytes)?[..])? + .into_ecrecovered() + .ok_or(eyre::eyre!("failed to recover tx"))?; + + if let Transaction::Eip4844(TxEip4844 { blob_versioned_hashes, .. }) = + &transaction.transaction + { + let blobs_bundle = blobs_bundle.as_mut().ok_or(eyre::eyre!( + "encountered a blob tx. `--blobs-bundle-path` must be provided" + ))?; + let (commitments, proofs, blobs) = blobs_bundle.take(blob_versioned_hashes.len()); + blob_store.insert( + transaction.hash, + BlobTransactionSidecar { blobs, commitments, proofs }, + )?; + } + + debug!(target: "reth::cli", ?transaction, "Adding transaction to the pool"); + transaction_pool + .add_transaction( + TransactionOrigin::External, + EthPooledTransaction::new(transaction), + ) + .await?; + } + + let payload_attrs = PayloadAttributes { + parent_beacon_block_root: self.parent_beacon_block_root, + prev_randao: self.prev_randao, + timestamp: U64::from(self.timestamp), + suggested_fee_recipient: self.suggested_fee_recipient, + // TODO: add support for withdrawals + withdrawals: None, + }; + let payload_config = PayloadConfig::new( + Arc::clone(&best_block), + Bytes::default(), + PayloadBuilderAttributes::new(best_block.hash, payload_attrs), + self.chain.clone(), + ); + let args = BuildArguments::new( + blockchain_db.clone(), + transaction_pool, + CachedReads::default(), + payload_config, + Cancelled::default(), + None, + ); + match default_payload_builder(args)? { + BuildOutcome::Better { payload, .. } => { + let block = payload.block(); + debug!(target: "reth::cli", ?block, "Built new payload"); + + consensus.validate_header_with_total_difficulty(block, U256::MAX)?; + consensus.validate_header(block)?; + consensus.validate_block(block)?; + + let senders = block.senders().expect("sender recovery failed"); + let block_with_senders = + SealedBlockWithSenders::new(block.clone(), senders).unwrap(); + + let executor_factory = Factory::new(self.chain.clone()); + let mut executor = executor_factory.with_state(blockchain_db.latest()?); + executor.execute_and_verify_receipt( + &block_with_senders.block.clone().unseal(), + U256::MAX, + None, + )?; + let state = executor.take_output_state(); + debug!(target: "reth::cli", ?state, "Executed block"); + + // Attempt to insert new block without committing + let provider_rw = factory.provider_rw()?; + provider_rw.append_blocks_with_bundle_state( + Vec::from([block_with_senders]), + state, + None, + )?; + info!(target: "reth::cli", "Successfully appended built block"); + } + _ => unreachable!("other outcomes are unreachable"), + }; + + Ok(()) + } +} diff --git a/bin/reth/src/debug_cmd/mod.rs b/bin/reth/src/debug_cmd/mod.rs index 7dfa448db43d..75cc196ed4fe 100644 --- a/bin/reth/src/debug_cmd/mod.rs +++ b/bin/reth/src/debug_cmd/mod.rs @@ -3,6 +3,7 @@ use clap::{Parser, Subcommand}; use crate::runner::CliContext; +mod build_block; mod execution; mod in_memory_merkle; mod merkle; @@ -23,6 +24,8 @@ pub enum Subcommands { Merkle(merkle::Command), /// Debug in-memory state root calculation. InMemoryMerkle(in_memory_merkle::Command), + /// Debug block building. + BuildBlock(build_block::Command), } impl Command { @@ -32,6 +35,7 @@ impl Command { Subcommands::Execution(command) => command.execute(ctx).await, Subcommands::Merkle(command) => command.execute(ctx).await, Subcommands::InMemoryMerkle(command) => command.execute(ctx).await, + Subcommands::BuildBlock(command) => command.execute(ctx).await, } } } diff --git a/crates/payload/basic/src/lib.rs b/crates/payload/basic/src/lib.rs index b82fc960af01..94c80a09e3ed 100644 --- a/crates/payload/basic/src/lib.rs +++ b/crates/payload/basic/src/lib.rs @@ -148,18 +148,12 @@ where block.seal(attributes.parent) }; - // configure evm env based on parent block - let (initialized_cfg, initialized_block_env) = - attributes.cfg_and_block_env(&self.chain_spec, &parent_block); - - let config = PayloadConfig { - initialized_block_env, - initialized_cfg, - parent_block: Arc::new(parent_block), - extra_data: self.config.extradata.clone(), + let config = PayloadConfig::new( + Arc::new(parent_block), + self.config.extradata.clone(), attributes, - chain_spec: Arc::clone(&self.chain_spec), - }; + Arc::clone(&self.chain_spec), + ); let until = tokio::time::Instant::now() + self.config.deadline; let deadline = Box::pin(tokio::time::sleep_until(until)); @@ -514,13 +508,13 @@ impl Future for PendingPayload { /// /// If dropped, it will set the `cancelled` flag to true. #[derive(Default, Clone, Debug)] -struct Cancelled(Arc); +pub struct Cancelled(Arc); // === impl Cancelled === impl Cancelled { /// Returns true if the job was cancelled. - fn is_cancelled(&self) -> bool { + pub fn is_cancelled(&self) -> bool { self.0.load(std::sync::atomic::Ordering::Relaxed) } } @@ -533,7 +527,7 @@ impl Drop for Cancelled { /// Static config for how to build a payload. #[derive(Clone, Debug)] -struct PayloadConfig { +pub struct PayloadConfig { /// Pre-configured block environment. initialized_block_env: BlockEnv, /// Configuration for the environment. @@ -548,6 +542,29 @@ struct PayloadConfig { chain_spec: Arc, } +impl PayloadConfig { + /// Create new payload config. + pub fn new( + parent_block: Arc, + extra_data: Bytes, + attributes: PayloadBuilderAttributes, + chain_spec: Arc, + ) -> Self { + // configure evm env based on parent block + let (initialized_cfg, initialized_block_env) = + attributes.cfg_and_block_env(&chain_spec, &parent_block); + + Self { + initialized_block_env, + initialized_cfg, + parent_block, + extra_data, + attributes, + chain_spec, + } + } +} + /// The possible outcomes of a payload building attempt. #[derive(Debug)] pub enum BuildOutcome { @@ -584,6 +601,20 @@ pub struct BuildArguments { best_payload: Option>, } +impl BuildArguments { + /// Create new build arguments. + pub fn new( + client: Client, + pool: Pool, + cached_reads: CachedReads, + config: PayloadConfig, + cancel: Cancelled, + best_payload: Option>, + ) -> Self { + Self { client, pool, cached_reads, config, cancel, best_payload } + } +} + /// A trait for building payloads that encapsulate Ethereum transactions. /// /// This trait provides the `try_build` method to construct a transaction payload @@ -631,7 +662,7 @@ where /// and configuration, this function creates a transaction payload. Returns /// a result indicating success with the payload or an error in case of failure. #[inline] -fn default_payload_builder( +pub fn default_payload_builder( args: BuildArguments, ) -> Result where From 529635f8d4d627d3652807e3f0377b3c256de4fc Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Fri, 6 Oct 2023 16:58:12 +0200 Subject: [PATCH 10/23] fix: always serialize zero balance (#4929) --- crates/rpc/rpc-types/src/eth/trace/geth/pre_state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rpc/rpc-types/src/eth/trace/geth/pre_state.rs b/crates/rpc/rpc-types/src/eth/trace/geth/pre_state.rs index c0cb3f37d174..5384896cbf12 100644 --- a/crates/rpc/rpc-types/src/eth/trace/geth/pre_state.rs +++ b/crates/rpc/rpc-types/src/eth/trace/geth/pre_state.rs @@ -78,7 +78,7 @@ impl AccountState { /// If code is empty, it will be omitted. pub fn from_account_info(nonce: u64, balance: U256, code: Option) -> Self { Self { - balance: (balance != U256::ZERO).then_some(balance), + balance: Some(balance), code: code.filter(|code| !code.is_empty()), nonce: (nonce != 0).then_some(nonce), storage: Default::default(), From 9c8eca6a499e4d5de6af5255694c9833d9a6cea1 Mon Sep 17 00:00:00 2001 From: joshieDo <93316087+joshieDo@users.noreply.github.com> Date: Fri, 6 Oct 2023 17:33:56 +0100 Subject: [PATCH 11/23] feat: add `reth db snapshot ` command (#4889) --- Cargo.lock | 34 ++- bin/reth/Cargo.toml | 3 + bin/reth/src/db/mod.rs | 6 + bin/reth/src/db/snapshots/bench.rs | 48 ++++ bin/reth/src/db/snapshots/headers.rs | 192 ++++++++++++++++ bin/reth/src/db/snapshots/mod.rs | 216 ++++++++++++++++++ crates/storage/db/Cargo.toml | 1 + crates/storage/db/src/snapshot.rs | 19 +- crates/storage/nippy-jar/Cargo.toml | 24 +- .../storage/nippy-jar/src/compression/lz4.rs | 83 +++++++ .../storage/nippy-jar/src/compression/mod.rs | 53 +++-- .../storage/nippy-jar/src/compression/zstd.rs | 78 ++++--- crates/storage/nippy-jar/src/cursor.rs | 98 +++++--- crates/storage/nippy-jar/src/error.rs | 4 + crates/storage/nippy-jar/src/filter/cuckoo.rs | 4 + crates/storage/nippy-jar/src/lib.rs | 210 ++++++++++++++--- crates/storage/nippy-jar/src/phf/fmph.rs | 1 - crates/storage/nippy-jar/src/phf/go_fmph.rs | 1 - .../provider/src/providers/snapshot.rs | 38 ++- 19 files changed, 954 insertions(+), 159 deletions(-) create mode 100644 bin/reth/src/db/snapshots/bench.rs create mode 100644 bin/reth/src/db/snapshots/headers.rs create mode 100644 bin/reth/src/db/snapshots/mod.rs create mode 100644 crates/storage/nippy-jar/src/compression/lz4.rs diff --git a/Cargo.lock b/Cargo.lock index 980b6f16dc79..4c6f123a7093 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -823,17 +823,6 @@ dependencies = [ "generic-array", ] -[[package]] -name = "bloomfilter" -version = "1.0.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b92db7965d438b8b4b1c1d0aedd188440a1084593c9eb7f6657e3df7e906d934" -dependencies = [ - "bit-vec", - "getrandom 0.2.10", - "siphasher 1.0.0", -] - [[package]] name = "blst" version = "0.3.11" @@ -4033,6 +4022,12 @@ dependencies = [ "linked-hash-map", ] +[[package]] +name = "lz4_flex" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ea9b256699eda7b0387ffbc776dd625e28bde3918446381781245b7a50349d8" + [[package]] name = "mach2" version = "0.4.1" @@ -4779,7 +4774,7 @@ version = "0.11.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "90fcb95eef784c2ac79119d1dd819e162b5da872ce6f3c3abe1e8ca1c082f72b" dependencies = [ - "siphasher 0.3.11", + "siphasher", ] [[package]] @@ -5421,6 +5416,7 @@ dependencies = [ "human_bytes", "humantime", "hyper", + "itertools 0.11.0", "jemalloc-ctl", "jemallocator", "metrics", @@ -5430,6 +5426,7 @@ dependencies = [ "pin-project", "pretty_assertions", "proptest", + "rand 0.8.5", "reth-auto-seal-consensus", "reth-basic-payload-builder", "reth-beacon-consensus", @@ -5444,6 +5441,7 @@ dependencies = [ "reth-net-nat", "reth-network", "reth-network-api", + "reth-nippy-jar", "reth-payload-builder", "reth-primitives", "reth-provider", @@ -5639,6 +5637,7 @@ dependencies = [ "reth-metrics", "reth-nippy-jar", "reth-primitives", + "reth-tracing", "secp256k1", "serde", "serde_json", @@ -5978,9 +5977,10 @@ version = "0.1.0-alpha.10" dependencies = [ "anyhow", "bincode", - "bloomfilter", "bytes", "cuckoofilter", + "hex", + "lz4_flex", "memmap2 0.7.1", "ph", "rand 0.8.5", @@ -5988,6 +5988,8 @@ dependencies = [ "sucds 0.8.1", "tempfile", "thiserror", + "tracing", + "tracing-appender", "zstd", ] @@ -7216,12 +7218,6 @@ version = "0.3.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "38b58827f4464d87d377d175e90bf58eb00fd8716ff0a62f80356b5e61555d0d" -[[package]] -name = "siphasher" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "54ac45299ccbd390721be55b412d41931911f654fa99e2cb8bfb57184b2061fe" - [[package]] name = "sketches-ddsketch" version = "0.2.1" diff --git a/bin/reth/Cargo.toml b/bin/reth/Cargo.toml index cba9d4a896a5..f7be0688bff1 100644 --- a/bin/reth/Cargo.toml +++ b/bin/reth/Cargo.toml @@ -50,6 +50,7 @@ reth-discv4 = { path = "../../crates/net/discv4" } reth-prune = { path = "../../crates/prune" } reth-snapshot = { path = "../../crates/snapshot" } reth-trie = { path = "../../crates/trie" } +reth-nippy-jar = { path = "../../crates/storage/nippy-jar" } # crypto alloy-rlp.workspace = true @@ -76,6 +77,7 @@ metrics.workspace = true # test vectors generation proptest.workspace = true +rand.workspace = true # tui comfy-table = "7.0" @@ -102,6 +104,7 @@ pretty_assertions = "1.3.0" humantime = "2.1.0" const-str = "0.5.6" boyer-moore-magiclen = "0.2.16" +itertools.workspace = true [target.'cfg(not(windows))'.dependencies] jemallocator = { version = "0.5.0", optional = true } diff --git a/bin/reth/src/db/mod.rs b/bin/reth/src/db/mod.rs index 1c9f15bcd61c..4f22064f78c3 100644 --- a/bin/reth/src/db/mod.rs +++ b/bin/reth/src/db/mod.rs @@ -24,6 +24,7 @@ mod clear; mod diff; mod get; mod list; +mod snapshots; /// DB List TUI mod tui; @@ -85,6 +86,8 @@ pub enum Subcommands { }, /// Deletes all table entries Clear(clear::Command), + /// Snapshots tables from database + Snapshot(snapshots::Command), /// Lists current and local database versions Version, /// Returns the full database path @@ -210,6 +213,9 @@ impl Command { let db = open_db(&db_path, self.db.log_level)?; command.execute(&db)?; } + Subcommands::Snapshot(command) => { + command.execute(&db_path, self.db.log_level, self.chain.clone())?; + } Subcommands::Version => { let local_db_version = match get_db_version(&db_path) { Ok(version) => Some(version), diff --git a/bin/reth/src/db/snapshots/bench.rs b/bin/reth/src/db/snapshots/bench.rs new file mode 100644 index 000000000000..8c926d226296 --- /dev/null +++ b/bin/reth/src/db/snapshots/bench.rs @@ -0,0 +1,48 @@ +use super::JarConfig; +use reth_db::DatabaseEnvRO; +use reth_primitives::ChainSpec; +use reth_provider::{DatabaseProviderRO, ProviderFactory}; +use std::{sync::Arc, time::Instant}; + +#[derive(Debug)] +pub(crate) enum BenchKind { + Walk, + RandomAll, + RandomOne, + RandomHash, +} + +pub(crate) fn bench( + bench_kind: BenchKind, + db: (DatabaseEnvRO, Arc), + jar_config: JarConfig, + mut snapshot_method: F1, + database_method: F2, +) -> eyre::Result<()> +where + F1: FnMut() -> eyre::Result<()>, + F2: Fn(DatabaseProviderRO<'_, DatabaseEnvRO>) -> eyre::Result<()>, +{ + let (mode, compression, phf) = jar_config; + let (db, chain) = db; + + println!(); + println!("############"); + println!("## [{mode:?}] [{compression:?}] [{phf:?}] [{bench_kind:?}]"); + { + let start = Instant::now(); + snapshot_method()?; + let end = start.elapsed().as_micros(); + println!("# snapshot {bench_kind:?} | {end} μs"); + } + { + let factory = ProviderFactory::new(db, chain); + let provider = factory.provider()?; + let start = Instant::now(); + database_method(provider)?; + let end = start.elapsed().as_micros(); + println!("# database {bench_kind:?} | {end} μs"); + } + + Ok(()) +} diff --git a/bin/reth/src/db/snapshots/headers.rs b/bin/reth/src/db/snapshots/headers.rs new file mode 100644 index 000000000000..387c1e53d695 --- /dev/null +++ b/bin/reth/src/db/snapshots/headers.rs @@ -0,0 +1,192 @@ +use super::{ + bench::{bench, BenchKind}, + Command, Compression, PerfectHashingFunction, Rows, Snapshots, +}; +use crate::utils::DbTool; +use rand::{seq::SliceRandom, Rng}; +use reth_db::{ + cursor::DbCursorRO, database::Database, open_db_read_only, snapshot::create_snapshot_T1_T2, + table::Decompress, tables, transaction::DbTx, DatabaseEnvRO, +}; +use reth_interfaces::db::LogLevel; +use reth_nippy_jar::NippyJar; +use reth_primitives::{BlockNumber, ChainSpec, Header}; +use reth_provider::{HeaderProvider, ProviderError, ProviderFactory}; +use std::{path::Path, sync::Arc}; +use tables::*; + +impl Command { + pub(crate) fn generate_headers_snapshot( + &self, + tool: &DbTool<'_, DatabaseEnvRO>, + compression: Compression, + phf: PerfectHashingFunction, + ) -> eyre::Result<()> { + let mut jar = self.prepare_jar(2, (Snapshots::Headers, compression, phf), tool, || { + // Generates the dataset to train a zstd dictionary if necessary, with the most recent + // rows (at most 1000). + let dataset = tool.db.view(|tx| { + let mut cursor = tx.cursor_read::>()?; + let v1 = cursor + .walk_back(Some(RawKey::from((self.from + self.block_interval - 1) as u64)))? + .take(self.block_interval.min(1000)) + .map(|row| row.map(|(_key, value)| value.into_value()).expect("should exist")) + .collect::>(); + let mut cursor = tx.cursor_read::>()?; + let v2 = cursor + .walk_back(Some(RawKey::from((self.from + self.block_interval - 1) as u64)))? + .take(self.block_interval.min(1000)) + .map(|row| row.map(|(_key, value)| value.into_value()).expect("should exist")) + .collect::>(); + Ok::(vec![v1, v2]) + })??; + Ok(dataset) + })?; + + tool.db.view(|tx| { + // Hacky type inference. TODO fix + let mut none_vec = Some(vec![vec![vec![0u8]].into_iter()]); + let _ = none_vec.take(); + + // Generate list of hashes for filters & PHF + let mut cursor = tx.cursor_read::>()?; + let mut hashes = None; + if self.with_filters { + hashes = Some( + cursor + .walk(Some(RawKey::from(self.from as u64)))? + .take(self.block_interval) + .map(|row| { + row.map(|(_key, value)| value.into_value()).map_err(|e| e.into()) + }), + ); + } + + create_snapshot_T1_T2::( + tx, + self.from as u64..=(self.from as u64 + self.block_interval as u64), + None, + // We already prepared the dictionary beforehand + none_vec, + hashes, + self.block_interval, + &mut jar, + ) + })??; + + Ok(()) + } + + pub(crate) fn bench_headers_snapshot( + &self, + db_path: &Path, + log_level: Option, + chain: Arc, + compression: Compression, + phf: PerfectHashingFunction, + ) -> eyre::Result<()> { + let mode = Snapshots::Headers; + let jar_config = (mode, compression, phf); + let mut row_indexes = (self.from..(self.from + self.block_interval)).collect::>(); + let mut rng = rand::thread_rng(); + let mut dictionaries = None; + let mut jar = NippyJar::load_without_header(&self.get_file_path(jar_config))?; + + let (provider, decompressors) = self.prepare_jar_provider(&mut jar, &mut dictionaries)?; + let mut cursor = if !decompressors.is_empty() { + provider.cursor_with_decompressors(decompressors) + } else { + provider.cursor() + }; + + for bench_kind in [BenchKind::Walk, BenchKind::RandomAll] { + bench( + bench_kind, + (open_db_read_only(db_path, log_level)?, chain.clone()), + jar_config, + || { + for num in row_indexes.iter() { + Header::decompress( + cursor + .row_by_number_with_cols::<0b01, 2>(num - self.from)? + .ok_or(ProviderError::HeaderNotFound((*num as u64).into()))?[0], + )?; + // TODO: replace with below when eventually SnapshotProvider re-uses cursor + // provider.header_by_number(num as + // u64)?.ok_or(ProviderError::HeaderNotFound((*num as u64).into()))?; + } + Ok(()) + }, + |provider| { + for num in row_indexes.iter() { + provider + .header_by_number(*num as u64)? + .ok_or(ProviderError::HeaderNotFound((*num as u64).into()))?; + } + Ok(()) + }, + )?; + + // For random walk + row_indexes.shuffle(&mut rng); + } + + // BENCHMARK QUERYING A RANDOM HEADER BY NUMBER + { + let num = row_indexes[rng.gen_range(0..row_indexes.len())]; + bench( + BenchKind::RandomOne, + (open_db_read_only(db_path, log_level)?, chain.clone()), + jar_config, + || { + Header::decompress( + cursor + .row_by_number_with_cols::<0b01, 2>((num - self.from) as usize)? + .ok_or(ProviderError::HeaderNotFound((num as u64).into()))?[0], + )?; + Ok(()) + }, + |provider| { + provider + .header_by_number(num as u64)? + .ok_or(ProviderError::HeaderNotFound((num as u64).into()))?; + Ok(()) + }, + )?; + } + + // BENCHMARK QUERYING A RANDOM HEADER BY HASH + { + let num = row_indexes[rng.gen_range(0..row_indexes.len())] as u64; + let header_hash = + ProviderFactory::new(open_db_read_only(db_path, log_level)?, chain.clone()) + .header_by_number(num)? + .ok_or(ProviderError::HeaderNotFound(num.into()))? + .hash_slow(); + + bench( + BenchKind::RandomHash, + (open_db_read_only(db_path, log_level)?, chain.clone()), + jar_config, + || { + let header = Header::decompress( + cursor + .row_by_key_with_cols::<0b01, 2>(header_hash.as_slice())? + .ok_or(ProviderError::HeaderNotFound(header_hash.into()))?[0], + )?; + + // Might be a false positive, so in the real world we have to validate it + assert!(header.hash_slow() == header_hash); + Ok(()) + }, + |provider| { + provider + .header(&header_hash)? + .ok_or(ProviderError::HeaderNotFound(header_hash.into()))?; + Ok(()) + }, + )?; + } + Ok(()) + } +} diff --git a/bin/reth/src/db/snapshots/mod.rs b/bin/reth/src/db/snapshots/mod.rs new file mode 100644 index 000000000000..47b5cc4ea69c --- /dev/null +++ b/bin/reth/src/db/snapshots/mod.rs @@ -0,0 +1,216 @@ +use crate::utils::DbTool; +use clap::{clap_derive::ValueEnum, Parser}; +use eyre::WrapErr; +use itertools::Itertools; +use reth_db::{database::Database, open_db_read_only, table::Table, tables, DatabaseEnvRO}; +use reth_interfaces::db::LogLevel; +use reth_nippy_jar::{ + compression::{DecoderDictionary, Decompressor}, + NippyJar, +}; +use reth_primitives::ChainSpec; +use reth_provider::providers::SnapshotProvider; +use std::{ + path::{Path, PathBuf}, + sync::Arc, +}; + +mod bench; +mod headers; + +pub(crate) type Rows = Vec>>; +pub(crate) type JarConfig = (Snapshots, Compression, PerfectHashingFunction); + +#[derive(Parser, Debug)] +/// Arguments for the `reth db snapshot` command. +pub struct Command { + /// Snapshot categories to generate. + modes: Vec, + + /// Starting block for the snapshot. + #[arg(long, short, default_value = "0")] + from: usize, + + /// Number of blocks in the snapshot. + #[arg(long, short, default_value = "500000")] + block_interval: usize, + + /// Flag to enable database-to-snapshot benchmarking. + #[arg(long, default_value = "false")] + bench: bool, + + /// Flag to skip snapshot creation and only run benchmarks on existing snapshots. + #[arg(long, default_value = "false")] + only_bench: bool, + + /// Compression algorithms to use. + #[arg(long, short, value_delimiter = ',', default_value = "lz4")] + compression: Vec, + + /// Flag to enable inclusion list filters and PHFs. + #[arg(long, default_value = "true")] + with_filters: bool, + + /// Specifies the perfect hashing function to use. + #[arg(long, value_delimiter = ',', default_value_if("with_filters", "true", "mphf"))] + phf: Vec, +} + +impl Command { + /// Execute `db snapshot` command + pub fn execute( + self, + db_path: &Path, + log_level: Option, + chain: Arc, + ) -> eyre::Result<()> { + let all_combinations = self + .modes + .iter() + .cartesian_product(self.compression.iter()) + .cartesian_product(self.phf.iter()); + + { + let db = open_db_read_only(db_path, None)?; + let tool = DbTool::new(&db, chain.clone())?; + + if !self.only_bench { + for ((mode, compression), phf) in all_combinations.clone() { + match mode { + Snapshots::Headers => { + self.generate_headers_snapshot(&tool, *compression, *phf)? + } + Snapshots::Transactions => todo!(), + Snapshots::Receipts => todo!(), + } + } + } + } + + if self.only_bench || self.bench { + for ((mode, compression), phf) in all_combinations { + match mode { + Snapshots::Headers => self.bench_headers_snapshot( + db_path, + log_level, + chain.clone(), + *compression, + *phf, + )?, + Snapshots::Transactions => todo!(), + Snapshots::Receipts => todo!(), + } + } + } + + Ok(()) + } + + /// Returns a [`SnapshotProvider`] of the provided [`NippyJar`], alongside a list of + /// [`DecoderDictionary`] and [`Decompressor`] if necessary. + fn prepare_jar_provider<'a>( + &self, + jar: &'a mut NippyJar, + dictionaries: &'a mut Option>>, + ) -> eyre::Result<(SnapshotProvider<'a>, Vec>)> { + let mut decompressors: Vec> = vec![]; + if let Some(reth_nippy_jar::compression::Compressors::Zstd(zstd)) = jar.compressor_mut() { + if zstd.use_dict { + *dictionaries = zstd.generate_decompress_dictionaries(); + decompressors = zstd.generate_decompressors(dictionaries.as_ref().expect("qed"))?; + } + } + + Ok((SnapshotProvider { jar: &*jar, jar_start_block: self.from as u64 }, decompressors)) + } + + /// Returns a [`NippyJar`] according to the desired configuration. + fn prepare_jar eyre::Result>( + &self, + num_columns: usize, + jar_config: JarConfig, + tool: &DbTool<'_, DatabaseEnvRO>, + prepare_compression: F, + ) -> eyre::Result { + let (mode, compression, phf) = jar_config; + let snap_file = self.get_file_path(jar_config); + let table_name = match mode { + Snapshots::Headers => tables::Headers::NAME, + Snapshots::Transactions | Snapshots::Receipts => tables::Transactions::NAME, + }; + + let total_rows = tool.db.view(|tx| { + let table_db = tx.inner.open_db(Some(table_name)).wrap_err("Could not open db.")?; + let stats = tx + .inner + .db_stat(&table_db) + .wrap_err(format!("Could not find table: {}", table_name))?; + + Ok::((stats.entries() - self.from).min(self.block_interval)) + })??; + + assert!( + total_rows >= self.block_interval, + "Not enough rows on database {} < {}.", + total_rows, + self.block_interval + ); + + let mut nippy_jar = NippyJar::new_without_header(num_columns, snap_file.as_path()); + nippy_jar = match compression { + Compression::Lz4 => nippy_jar.with_lz4(), + Compression::Zstd => nippy_jar.with_zstd(false, 0), + Compression::ZstdWithDictionary => { + let dataset = prepare_compression()?; + + nippy_jar = nippy_jar.with_zstd(true, 5_000_000); + nippy_jar.prepare_compression(dataset)?; + nippy_jar + } + Compression::Uncompressed => nippy_jar, + }; + + if self.with_filters { + nippy_jar = nippy_jar.with_cuckoo_filter(self.block_interval); + nippy_jar = match phf { + PerfectHashingFunction::Mphf => nippy_jar.with_mphf(), + PerfectHashingFunction::GoMphf => nippy_jar.with_gomphf(), + }; + } + + Ok(nippy_jar) + } + + /// Generates a filename according to the desired configuration. + fn get_file_path(&self, jar_config: JarConfig) -> PathBuf { + let (mode, compression, phf) = jar_config; + format!( + "snapshot_{mode:?}_{}_{}_{compression:?}_{phf:?}", + self.from, + self.from + self.block_interval + ) + .into() + } +} + +#[derive(Debug, Copy, Clone, ValueEnum)] +pub(crate) enum Snapshots { + Headers, + Transactions, + Receipts, +} + +#[derive(Debug, Copy, Clone, ValueEnum, Default)] +pub(crate) enum Compression { + Lz4, + Zstd, + ZstdWithDictionary, + #[default] + Uncompressed, +} + +#[derive(Debug, Copy, Clone, ValueEnum)] +pub(crate) enum PerfectHashingFunction { + Mphf, + GoMphf, +} diff --git a/crates/storage/db/Cargo.toml b/crates/storage/db/Cargo.toml index 2c895e018767..069c3737c20b 100644 --- a/crates/storage/db/Cargo.toml +++ b/crates/storage/db/Cargo.toml @@ -15,6 +15,7 @@ reth-interfaces.workspace = true reth-codecs = { path = "../codecs" } reth-libmdbx = { path = "../libmdbx-rs", optional = true, features = ["return-borrowed"] } reth-nippy-jar = { path = "../nippy-jar" } +reth-tracing = { path = "../../tracing" } # codecs serde = { workspace = true, default-features = false } diff --git a/crates/storage/db/src/snapshot.rs b/crates/storage/db/src/snapshot.rs index cea6350d68ca..9736c3e1ff87 100644 --- a/crates/storage/db/src/snapshot.rs +++ b/crates/storage/db/src/snapshot.rs @@ -8,6 +8,7 @@ use crate::{ }; use reth_interfaces::RethResult; use reth_nippy_jar::{ColumnResult, NippyJar, PHFKey}; +use reth_tracing::tracing::*; use std::{error::Error as StdError, ops::RangeInclusive}; /// Macro that generates snapshot creation functions that take an arbitratry number of [`Table`] and @@ -25,6 +26,7 @@ macro_rules! generate_snapshot_func { /// /// * `tx`: Database transaction. /// * `range`: Data range for columns in tables. + /// * `additional`: Additional columns which can't be straight straightforwardly walked on. /// * `keys`: Iterator of keys (eg. `TxHash` or `BlockHash`) with length equal to `row_count` and ordered by future column insertion from `range`. /// * `dict_compression_set`: Sets of column data for compression dictionaries. Max size is 2GB. Row count is independent. /// * `row_count`: Total rows to add to `NippyJar`. Must match row count in `range`. @@ -37,6 +39,7 @@ macro_rules! generate_snapshot_func { ( tx: &impl DbTx<'tx>, range: RangeInclusive, + additional: Option, Box>>>>>, dict_compression_set: Option>>>, keys: Option>>, row_count: usize, @@ -44,16 +47,23 @@ macro_rules! generate_snapshot_func { ) -> RethResult<()> where K: Key + Copy { + let additional = additional.unwrap_or_default(); + debug!(target: "reth::snapshot", ?range, "Creating snapshot {:?} and {} more columns.", vec![$($tbl::NAME,)+], additional.len()); + let range: RangeInclusive> = RawKey::new(*range.start())..=RawKey::new(*range.end()); // Create PHF and Filter if required if let Some(keys) = keys { + debug!(target: "reth::snapshot", "Calculating Filter, PHF and offset index list"); nippy_jar.prepare_index(keys, row_count)?; + debug!(target: "reth::snapshot", "Filter, PHF and offset index list calculated."); } // Create compression dictionaries if required if let Some(data_sets) = dict_compression_set { + debug!(target: "reth::snapshot", "Creating compression dictionaries."); nippy_jar.prepare_compression(data_sets)?; + debug!(target: "reth::snapshot", "Compression dictionaries created."); } // Creates the cursors for the columns @@ -75,7 +85,12 @@ macro_rules! generate_snapshot_func { $(Box::new([< $tbl _iter>]),)+ ]; - nippy_jar.freeze(col_iterators, row_count as u64)?; + + debug!(target: "reth::snapshot", jar=?nippy_jar, "Generating snapshot file."); + + nippy_jar.freeze(col_iterators.into_iter().chain(additional).collect(), row_count as u64)?; + + debug!(target: "reth::snapshot", jar=?nippy_jar, "Snapshot file generated."); Ok(()) } @@ -84,4 +99,4 @@ macro_rules! generate_snapshot_func { }; } -generate_snapshot_func!((T1), (T1, T2), (T1, T2, T3), (T1, T2, T3, T4),); +generate_snapshot_func!((T1), (T1, T2), (T1, T2, T3), (T1, T2, T3, T4), (T1, T2, T3, T4, T5),); diff --git a/crates/storage/nippy-jar/Cargo.toml b/crates/storage/nippy-jar/Cargo.toml index 3aa7f92d7652..9be19824a6be 100644 --- a/crates/storage/nippy-jar/Cargo.toml +++ b/crates/storage/nippy-jar/Cargo.toml @@ -12,20 +12,28 @@ description = "Immutable data store format" name = "reth_nippy_jar" [dependencies] -memmap2 = "0.7.1" -bloomfilter = "1" -zstd = { version = "0.12", features = ["experimental", "zdict_builder"] } + +# filter ph = "0.8.0" -thiserror .workspace = true +cuckoofilter = { version = "0.5.0", features = ["serde_support", "serde_bytes"] } + +# compression +zstd = { version = "0.12", features = ["experimental", "zdict_builder"] } +lz4_flex = { version = "0.11", default-features = false } + +# offsets +sucds = "~0.8" + +memmap2 = "0.7.1" bincode = "1.3" serde = { version = "1.0", features = ["derive"] } bytes.workspace = true -cuckoofilter = { version = "0.5.0", features = ["serde_support", "serde_bytes"] } tempfile.workspace = true -sucds = "~0.8" - +tracing = "0.1.0" +tracing-appender = "0.2" anyhow = "1.0" - +thiserror.workspace = true +hex = "*" [dev-dependencies] rand = { version = "0.8", features = ["small_rng"] } diff --git a/crates/storage/nippy-jar/src/compression/lz4.rs b/crates/storage/nippy-jar/src/compression/lz4.rs new file mode 100644 index 000000000000..be014994a8ed --- /dev/null +++ b/crates/storage/nippy-jar/src/compression/lz4.rs @@ -0,0 +1,83 @@ +use crate::{compression::Compression, NippyJarError}; +use serde::{Deserialize, Serialize}; + +/// Wrapper type for `lz4_flex` that implements [`Compression`]. +#[derive(Debug, PartialEq, Serialize, Deserialize, Default)] +#[non_exhaustive] +pub struct Lz4; + +impl Compression for Lz4 { + fn decompress_to(&self, value: &[u8], dest: &mut Vec) -> Result<(), NippyJarError> { + let previous_length = dest.len(); + + // SAFETY: We're setting len to the existing capacity. + unsafe { + dest.set_len(dest.capacity()); + } + + match lz4_flex::decompress_into(value, &mut dest[previous_length..]) { + Ok(written) => { + // SAFETY: `compress_into` can only write if there's enough capacity. Therefore, it + // shouldn't write more than our capacity. + unsafe { + dest.set_len(previous_length + written); + } + Ok(()) + } + Err(_) => { + // SAFETY: we are resetting it to the previous value. + unsafe { + dest.set_len(previous_length); + } + Err(NippyJarError::OutputTooSmall) + } + } + } + + fn decompress(&self, value: &[u8]) -> Result, NippyJarError> { + let mut multiplier = 1; + + loop { + match lz4_flex::decompress(value, multiplier * value.len()) { + Ok(v) => return Ok(v), + Err(err) => { + multiplier *= 2; + if multiplier == 16 { + return Err(NippyJarError::Custom(err.to_string())) + } + } + } + } + } + + fn compress_to(&self, src: &[u8], dest: &mut Vec) -> Result { + let previous_length = dest.len(); + + // SAFETY: We're setting len to the existing capacity. + unsafe { + dest.set_len(dest.capacity()); + } + + match lz4_flex::compress_into(src, &mut dest[previous_length..]) { + Ok(written) => { + // SAFETY: `compress_into` can only write if there's enough capacity. Therefore, it + // shouldn't write more than our capacity. + unsafe { + dest.set_len(previous_length + written); + } + Ok(written) + } + Err(_) => { + // SAFETY: we are resetting it to the previous value. + unsafe { + dest.set_len(previous_length); + } + Err(NippyJarError::OutputTooSmall) + } + } + } + + fn compress(&self, src: &[u8]) -> Result, NippyJarError> { + Ok(lz4_flex::compress(src)) + } +} diff --git a/crates/storage/nippy-jar/src/compression/mod.rs b/crates/storage/nippy-jar/src/compression/mod.rs index 5008dfdf6347..a8f99fa53924 100644 --- a/crates/storage/nippy-jar/src/compression/mod.rs +++ b/crates/storage/nippy-jar/src/compression/mod.rs @@ -1,17 +1,24 @@ use crate::NippyJarError; use serde::{Deserialize, Serialize}; -use std::io::Write; mod zstd; -pub use self::zstd::{Zstd, ZstdState}; +pub use self::zstd::{DecoderDictionary, Decompressor, Zstd, ZstdState}; +mod lz4; +pub use self::lz4::Lz4; /// Trait that will compress column values pub trait Compression: Serialize + for<'a> Deserialize<'a> { + /// Appends decompressed data to the dest buffer. Requires `dest` to have sufficient capacity. + fn decompress_to(&self, value: &[u8], dest: &mut Vec) -> Result<(), NippyJarError>; + /// Returns decompressed data. fn decompress(&self, value: &[u8]) -> Result, NippyJarError>; - /// Compresses data from `src` to `dest` - fn compress_to(&self, src: &[u8], dest: &mut W) -> Result<(), NippyJarError>; + /// Appends compressed data from `src` to `dest`. `dest`. Requires `dest` to have sufficient + /// capacity. + /// + /// Returns number of bytes written to `dest`. + fn compress_to(&self, src: &[u8], dest: &mut Vec) -> Result; /// Compresses data from `src` fn compress(&self, src: &[u8]) -> Result, NippyJarError>; @@ -37,36 +44,54 @@ pub trait Compression: Serialize + for<'a> Deserialize<'a> { #[cfg_attr(test, derive(PartialEq))] pub enum Compressors { Zstd(Zstd), - // Avoids irrefutable let errors. Remove this after adding another one. - Unused, + Lz4(Lz4), } impl Compression for Compressors { + fn decompress_to(&self, value: &[u8], dest: &mut Vec) -> Result<(), NippyJarError> { + match self { + Compressors::Zstd(zstd) => zstd.decompress_to(value, dest), + Compressors::Lz4(lz4) => lz4.decompress_to(value, dest), + } + } fn decompress(&self, value: &[u8]) -> Result, NippyJarError> { match self { Compressors::Zstd(zstd) => zstd.decompress(value), - Compressors::Unused => unimplemented!(), + Compressors::Lz4(lz4) => lz4.decompress(value), } } - fn compress_to(&self, src: &[u8], dest: &mut W) -> Result<(), NippyJarError> { - match self { - Compressors::Zstd(zstd) => zstd.compress_to(src, dest), - Compressors::Unused => unimplemented!(), + fn compress_to(&self, src: &[u8], dest: &mut Vec) -> Result { + let initial_capacity = dest.capacity(); + loop { + let result = match self { + Compressors::Zstd(zstd) => zstd.compress_to(src, dest), + Compressors::Lz4(lz4) => lz4.compress_to(src, dest), + }; + + match result { + Ok(v) => return Ok(v), + Err(err) => match err { + NippyJarError::OutputTooSmall => { + dest.reserve(initial_capacity); + } + _ => return Err(err), + }, + } } } fn compress(&self, src: &[u8]) -> Result, NippyJarError> { match self { Compressors::Zstd(zstd) => zstd.compress(src), - Compressors::Unused => unimplemented!(), + Compressors::Lz4(lz4) => lz4.compress(src), } } fn is_ready(&self) -> bool { match self { Compressors::Zstd(zstd) => zstd.is_ready(), - Compressors::Unused => unimplemented!(), + Compressors::Lz4(lz4) => lz4.is_ready(), } } @@ -76,7 +101,7 @@ impl Compression for Compressors { ) -> Result<(), NippyJarError> { match self { Compressors::Zstd(zstd) => zstd.prepare_compression(columns), - Compressors::Unused => Ok(()), + Compressors::Lz4(lz4) => lz4.prepare_compression(columns), } } } diff --git a/crates/storage/nippy-jar/src/compression/zstd.rs b/crates/storage/nippy-jar/src/compression/zstd.rs index 494ed6734dbd..df1182f2834f 100644 --- a/crates/storage/nippy-jar/src/compression/zstd.rs +++ b/crates/storage/nippy-jar/src/compression/zstd.rs @@ -4,10 +4,9 @@ use std::{ fs::File, io::{Read, Write}, }; -use zstd::{ - bulk::{Compressor, Decompressor}, - dict::DecoderDictionary, -}; +use tracing::*; +use zstd::bulk::Compressor; +pub use zstd::{bulk::Decompressor, dict::DecoderDictionary}; type RawDictionary = Vec; @@ -26,7 +25,7 @@ pub struct Zstd { /// Compression level. A level of `0` uses zstd's default (currently `3`). pub(crate) level: i32, /// Uses custom dictionaries to compress data. - pub(crate) use_dict: bool, + pub use_dict: bool, /// Max size of a dictionary pub(crate) max_dict_size: usize, /// List of column dictionaries. @@ -87,6 +86,8 @@ impl Zstd { let mut compressors = None; if let Some(dictionaries) = &self.raw_dictionaries { + debug!(target: "nippy-jar", count=?dictionaries.len(), "Generating ZSTD compressor dictionaries."); + let mut cmp = Vec::with_capacity(dictionaries.len()); for dict in dictionaries { @@ -99,10 +100,11 @@ impl Zstd { } } - /// Compresses a value using a dictionary. + /// Compresses a value using a dictionary. Reserves additional capacity for `buffer` if + /// necessary. pub fn compress_with_dictionary( column_value: &[u8], - tmp_buf: &mut Vec, + buffer: &mut Vec, handle: &mut File, compressor: Option<&mut Compressor<'_>>, ) -> Result<(), NippyJarError> { @@ -112,16 +114,16 @@ impl Zstd { // enough, the compressed buffer will actually be larger. We keep retrying. // If we eventually fail, it probably means it's another kind of error. let mut multiplier = 1; - while let Err(err) = compressor.compress_to_buffer(column_value, tmp_buf) { - tmp_buf.reserve(column_value.len() * multiplier); + while let Err(err) = compressor.compress_to_buffer(column_value, buffer) { + buffer.reserve(column_value.len() * multiplier); multiplier += 1; if multiplier == 5 { return Err(NippyJarError::Disconnect(err)) } } - handle.write_all(tmp_buf)?; - tmp_buf.clear(); + handle.write_all(buffer)?; + buffer.clear(); } else { handle.write_all(column_value)?; } @@ -129,38 +131,46 @@ impl Zstd { Ok(()) } - /// Decompresses a value using a dictionary to a user provided buffer. + /// Appends a decompressed value using a dictionary to a user provided buffer. pub fn decompress_with_dictionary( column_value: &[u8], output: &mut Vec, decompressor: &mut Decompressor<'_>, ) -> Result<(), NippyJarError> { - let mut multiplier = 1; - - // Just an estimation. - let required_capacity = column_value.len() * 2; - - output.reserve(required_capacity.saturating_sub(output.capacity())); + let previous_length = output.len(); - // Decompressor requires the destination buffer to be big enough to write to, otherwise it - // fails. However, we don't know how big it will be. We keep retrying. - // If we eventually fail, it probably means it's another kind of error. - while let Err(err) = decompressor.decompress_to_buffer(column_value, output) { - output.reserve( - Decompressor::upper_bound(column_value).unwrap_or(required_capacity) * multiplier, - ); + // SAFETY: We're setting len to the existing capacity. + unsafe { + output.set_len(output.capacity()); + } - multiplier += 1; - if multiplier == 5 { - return Err(NippyJarError::Disconnect(err)) + match decompressor.decompress_to_buffer(column_value, &mut output[previous_length..]) { + Ok(written) => { + // SAFETY: `decompress_to_buffer` can only write if there's enough capacity. + // Therefore, it shouldn't write more than our capacity. + unsafe { + output.set_len(previous_length + written); + } + Ok(()) + } + Err(_) => { + // SAFETY: we are resetting it to the previous value. + unsafe { + output.set_len(previous_length); + } + Err(NippyJarError::OutputTooSmall) } } - - Ok(()) } } impl Compression for Zstd { + fn decompress_to(&self, value: &[u8], dest: &mut Vec) -> Result<(), NippyJarError> { + let mut decoder = zstd::Decoder::with_dictionary(value, &[])?; + decoder.read_to_end(dest)?; + Ok(()) + } + fn decompress(&self, value: &[u8]) -> Result, NippyJarError> { let mut decompressed = Vec::with_capacity(value.len() * 2); let mut decoder = zstd::Decoder::new(value)?; @@ -168,13 +178,15 @@ impl Compression for Zstd { Ok(decompressed) } - fn compress_to(&self, src: &[u8], dest: &mut W) -> Result<(), NippyJarError> { + fn compress_to(&self, src: &[u8], dest: &mut Vec) -> Result { + let before = dest.len(); + let mut encoder = zstd::Encoder::new(dest, self.level)?; encoder.write_all(src)?; - encoder.finish()?; + let dest = encoder.finish()?; - Ok(()) + Ok(dest.len() - before) } fn compress(&self, src: &[u8]) -> Result, NippyJarError> { diff --git a/crates/storage/nippy-jar/src/cursor.rs b/crates/storage/nippy-jar/src/cursor.rs index 1fd7170c9010..19e39fa0cd2b 100644 --- a/crates/storage/nippy-jar/src/cursor.rs +++ b/crates/storage/nippy-jar/src/cursor.rs @@ -1,10 +1,10 @@ use crate::{ compression::{Compression, Zstd}, - InclusionFilter, NippyJar, NippyJarError, PerfectHashingFunction, Row, + InclusionFilter, NippyJar, NippyJarError, PerfectHashingFunction, RefRow, }; use memmap2::Mmap; use serde::{de::Deserialize, ser::Serialize}; -use std::{clone::Clone, fs::File}; +use std::{fs::File, ops::Range}; use sucds::int_vectors::Access; use zstd::bulk::Decompressor; @@ -19,14 +19,13 @@ pub struct NippyJarCursor<'a, H = ()> { file_handle: File, /// Data file. mmap_handle: Mmap, - /// Temporary buffer to unload data to (if necessary), without reallocating memory on each - /// retrieval. - tmp_buf: Vec, + /// Internal buffer to unload data to without reallocating memory on each retrieval. + internal_buffer: Vec, /// Cursor row position. row: u64, } -impl<'a, H> std::fmt::Debug for NippyJarCursor<'a, H> +impl<'a, H: std::fmt::Debug> std::fmt::Debug for NippyJarCursor<'a, H> where H: Send + Sync + Serialize + for<'b> Deserialize<'b> + core::fmt::Debug, { @@ -37,7 +36,7 @@ where impl<'a, H> NippyJarCursor<'a, H> where - H: Send + Sync + Serialize + for<'b> Deserialize<'b>, + H: Send + Sync + Serialize + for<'b> Deserialize<'b> + std::fmt::Debug, { pub fn new( jar: &'a NippyJar, @@ -53,7 +52,8 @@ where zstd_decompressors, file_handle: file, mmap_handle: mmap, - tmp_buf: vec![], + // Makes sure that we have enough buffer capacity to decompress any row of data. + internal_buffer: Vec::with_capacity(jar.max_row_size), row: 0, }) } @@ -69,7 +69,7 @@ where /// /// Example usage would be querying a transactions file with a transaction hash which is **NOT** /// stored in file. - pub fn row_by_key(&mut self, key: &[u8]) -> Result, NippyJarError> { + pub fn row_by_key(&mut self, key: &[u8]) -> Result>, NippyJarError> { if let (Some(filter), Some(phf)) = (&self.jar.filter, &self.jar.phf) { // TODO: is it worth to parallize both? @@ -93,13 +93,15 @@ where } /// Returns a row by its number. - pub fn row_by_number(&mut self, row: usize) -> Result, NippyJarError> { + pub fn row_by_number(&mut self, row: usize) -> Result>, NippyJarError> { self.row = row as u64; self.next_row() } /// Returns the current value and advances the row. - pub fn next_row(&mut self) -> Result, NippyJarError> { + pub fn next_row(&mut self) -> Result>, NippyJarError> { + self.internal_buffer.clear(); + if self.row as usize * self.jar.columns >= self.jar.offsets.len() { // Has reached the end return Ok(None) @@ -114,7 +116,14 @@ where self.row += 1; - Ok(Some(row)) + Ok(Some( + row.into_iter() + .map(|v| match v { + ValueRange::Mmap(range) => &self.mmap_handle[range], + ValueRange::Internal(range) => &self.internal_buffer[range], + }) + .collect(), + )) } /// Returns a row, searching it by a key used during [`NippyJar::prepare_index`] by using a @@ -127,7 +136,7 @@ where pub fn row_by_key_with_cols( &mut self, key: &[u8], - ) -> Result, NippyJarError> { + ) -> Result>, NippyJarError> { if let (Some(filter), Some(phf)) = (&self.jar.filter, &self.jar.phf) { // TODO: is it worth to parallize both? @@ -154,7 +163,7 @@ where pub fn row_by_number_with_cols( &mut self, row: usize, - ) -> Result, NippyJarError> { + ) -> Result>, NippyJarError> { self.row = row as u64; self.next_row_with_cols::() } @@ -164,8 +173,8 @@ where /// Uses a `MASK` to only read certain columns from the row. pub fn next_row_with_cols( &mut self, - ) -> Result, NippyJarError> { - debug_assert!(COLUMNS == self.jar.columns); + ) -> Result>, NippyJarError> { + self.internal_buffer.clear(); if self.row as usize * self.jar.columns >= self.jar.offsets.len() { // Has reached the end @@ -179,45 +188,68 @@ where self.read_value(column, &mut row)? } } - self.row += 1; - Ok(Some(row)) + Ok(Some( + row.into_iter() + .map(|v| match v { + ValueRange::Mmap(range) => &self.mmap_handle[range], + ValueRange::Internal(range) => &self.internal_buffer[range], + }) + .collect(), + )) } - /// Takes the column index and reads the value for the corresponding column. - fn read_value(&mut self, column: usize, row: &mut Row) -> Result<(), NippyJarError> { + /// Takes the column index and reads the range value for the corresponding column. + fn read_value( + &mut self, + column: usize, + row: &mut Vec, + ) -> Result<(), NippyJarError> { // Find out the offset of the column value let offset_pos = self.row as usize * self.jar.columns + column; let value_offset = self.jar.offsets.select(offset_pos).expect("should exist"); - let column_value = if self.jar.offsets.len() == (offset_pos + 1) { + let column_offset_range = if self.jar.offsets.len() == (offset_pos + 1) { // It's the last column of the last row - &self.mmap_handle[value_offset..] + value_offset..self.mmap_handle.len() } else { let next_value_offset = self.jar.offsets.select(offset_pos + 1).expect("should exist"); - &self.mmap_handle[value_offset..next_value_offset] + value_offset..next_value_offset }; if let Some(zstd_dict_decompressors) = self.zstd_decompressors.as_mut() { - self.tmp_buf.clear(); - + let from: usize = self.internal_buffer.len(); if let Some(decompressor) = zstd_dict_decompressors.get_mut(column) { - Zstd::decompress_with_dictionary(column_value, &mut self.tmp_buf, decompressor)?; + Zstd::decompress_with_dictionary( + &self.mmap_handle[column_offset_range], + &mut self.internal_buffer, + decompressor, + )?; } + let to = self.internal_buffer.len(); - debug_assert!(!self.tmp_buf.is_empty()); - - row.push(self.tmp_buf.clone()); - } else if let Some(compression) = &self.jar.compressor { + row.push(ValueRange::Internal(from..to)); + } else if let Some(compression) = self.jar.compressor() { // Uses the chosen default decompressor - row.push(compression.decompress(column_value)?); + let from = self.internal_buffer.len(); + compression + .decompress_to(&self.mmap_handle[column_offset_range], &mut self.internal_buffer)?; + let to = self.internal_buffer.len(); + + row.push(ValueRange::Internal(from..to)); } else { // Not compressed - // TODO: return Cow<&> instead of copying if there's no compression - row.push(column_value.to_vec()) + row.push(ValueRange::Mmap(column_offset_range)); } Ok(()) } } + +/// Helper type that stores the range of the decompressed column value either on a `mmap` slice or +/// on the internal buffer. +enum ValueRange { + Mmap(Range), + Internal(Range), +} diff --git a/crates/storage/nippy-jar/src/error.rs b/crates/storage/nippy-jar/src/error.rs index 1ea1b0f4d094..b17d3d2163a1 100644 --- a/crates/storage/nippy-jar/src/error.rs +++ b/crates/storage/nippy-jar/src/error.rs @@ -7,6 +7,8 @@ pub enum NippyJarError { Internal(#[from] Box), #[error(transparent)] Disconnect(#[from] std::io::Error), + #[error("{0}")] + Custom(String), #[error(transparent)] Bincode(#[from] Box), #[error(transparent)] @@ -33,4 +35,6 @@ pub enum NippyJarError { PHFMissing, #[error("NippyJar was built without an index.")] UnsupportedFilterQuery, + #[error("Compression or decompression requires a bigger destination output.")] + OutputTooSmall, } diff --git a/crates/storage/nippy-jar/src/filter/cuckoo.rs b/crates/storage/nippy-jar/src/filter/cuckoo.rs index 139af65e440a..2e4110e58cc7 100644 --- a/crates/storage/nippy-jar/src/filter/cuckoo.rs +++ b/crates/storage/nippy-jar/src/filter/cuckoo.rs @@ -17,6 +17,10 @@ pub struct Cuckoo { impl Cuckoo { pub fn new(max_capacity: usize) -> Self { + // CuckooFilter might return `NotEnoughSpace` even if they are remaining elements, if it's + // close to capacity. Therefore, we increase it. + let max_capacity = max_capacity + 100 + max_capacity / 3; + Cuckoo { remaining: max_capacity, filter: CuckooFilter::with_capacity(max_capacity) } } } diff --git a/crates/storage/nippy-jar/src/lib.rs b/crates/storage/nippy-jar/src/lib.rs index 1e7bcd44ecb7..bf8d683d0276 100644 --- a/crates/storage/nippy-jar/src/lib.rs +++ b/crates/storage/nippy-jar/src/lib.rs @@ -24,6 +24,7 @@ use sucds::{ mii_sequences::{EliasFano, EliasFanoBuilder}, Serializable, }; +use tracing::*; pub mod filter; use filter::{Cuckoo, InclusionFilter, InclusionFilters}; @@ -43,8 +44,9 @@ pub use cursor::NippyJarCursor; const NIPPY_JAR_VERSION: usize = 1; -/// A [`Row`] is a list of its selected column values. -type Row = Vec>; +/// A [`RefRow`] is a list of column value slices pointing to either an internal buffer or a +/// memory-mapped file. +type RefRow<'a> = Vec<&'a [u8]>; /// Alias type for a column value wrapped in `Result` pub type ColumnResult = Result>; @@ -73,7 +75,7 @@ pub type ColumnResult = Result>; /// /// Ultimately, the `freeze` function yields two files: a data file containing both the data and its /// configuration, and an index file that houses the offsets and offsets_index. -#[derive(Debug, Serialize, Deserialize)] +#[derive(Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq))] pub struct NippyJar { /// The version of the NippyJar format. @@ -95,11 +97,34 @@ pub struct NippyJar { /// Offsets within the file for each column value, arranged by row and column. #[serde(skip)] offsets: EliasFano, + /// Maximum uncompressed row size of the set. This will enable decompression without any + /// resizing of the output buffer. + #[serde(skip)] + max_row_size: usize, /// Data path for file. Index file will be `{path}.idx` #[serde(skip)] path: Option, } +impl std::fmt::Debug for NippyJar { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("NippyJar") + .field("version", &self.version) + .field("user_header", &self.user_header) + .field("columns", &self.columns) + .field("compressor", &self.compressor) + .field("filter", &self.filter) + .field("phf", &self.phf) + .field("offsets_index (len)", &self.offsets_index.len()) + .field("offsets_index (size in bytes)", &self.offsets_index.size_in_bytes()) + .field("offsets (len)", &self.offsets.len()) + .field("offsets (size in bytes)", &self.offsets.size_in_bytes()) + .field("path", &self.path) + .field("max_row_size", &self.max_row_size) + .finish_non_exhaustive() + } +} + impl NippyJar<()> { /// Creates a new [`NippyJar`] without an user-defined header data. pub fn new_without_header(columns: usize, path: &Path) -> Self { @@ -119,7 +144,7 @@ impl NippyJar<()> { impl NippyJar where - H: Send + Sync + Serialize + for<'a> Deserialize<'a>, + H: Send + Sync + Serialize + for<'a> Deserialize<'a> + std::fmt::Debug, { /// Creates a new [`NippyJar`] with a user-defined header data. pub fn new(columns: usize, path: &Path, user_header: H) -> Self { @@ -127,6 +152,7 @@ where version: NIPPY_JAR_VERSION, user_header, columns, + max_row_size: 0, compressor: None, filter: None, phf: None, @@ -143,6 +169,12 @@ where self } + /// Adds [`compression::Lz4`] compression. + pub fn with_lz4(mut self) -> Self { + self.compressor = Some(Compressors::Lz4(compression::Lz4::default())); + self + } + /// Adds [`filter::Cuckoo`] filter. pub fn with_cuckoo_filter(mut self, max_capacity: usize) -> Self { self.filter = Some(InclusionFilters::Cuckoo(Cuckoo::new(max_capacity))); @@ -166,6 +198,16 @@ where &self.user_header } + /// Gets a reference to the compressor. + pub fn compressor(&self) -> Option<&Compressors> { + self.compressor.as_ref() + } + + /// Gets a mutable reference to the compressor. + pub fn compressor_mut(&mut self) -> Option<&mut Compressors> { + self.compressor.as_mut() + } + /// Loads the file configuration and returns [`Self`]. /// /// **The user must ensure the header type matches the one used during the jar's creation.** @@ -185,7 +227,8 @@ where let mmap = unsafe { memmap2::Mmap::map(&offsets_file)? }; let mut offsets_reader = mmap.as_ref(); obj.offsets = EliasFano::deserialize_from(&mut offsets_reader)?; - obj.offsets_index = PrefixSummedEliasFano::deserialize_from(offsets_reader)?; + obj.offsets_index = PrefixSummedEliasFano::deserialize_from(&mut offsets_reader)?; + obj.max_row_size = bincode::deserialize_from(offsets_reader)?; Ok(obj) } @@ -211,6 +254,7 @@ where ) -> Result<(), NippyJarError> { // Makes any necessary preparations for the compressors if let Some(compression) = &mut self.compressor { + debug!(target: "nippy-jar", columns=columns.len(), "Preparing compression."); compression.prepare_compression(columns)?; } Ok(()) @@ -226,15 +270,27 @@ where values: impl IntoIterator>, row_count: usize, ) -> Result<(), NippyJarError> { + debug!(target: "nippy-jar", ?row_count, "Preparing index."); + let values = values.into_iter().collect::, _>>()?; + + debug_assert!( + row_count == values.len(), + "Row count ({row_count}) differs from value list count ({}).", + values.len() + ); + let mut offsets_index = vec![0; row_count]; // Builds perfect hashing function from the values if let Some(phf) = self.phf.as_mut() { + debug!(target: "nippy-jar", ?row_count, values_count = ?values.len(), "Setting keys for perfect hashing function."); phf.set_keys(&values)?; } if self.filter.is_some() || self.phf.is_some() { + debug!(target: "nippy-jar", ?row_count, "Creating filter and offsets_index."); + for (row_num, v) in values.into_iter().enumerate() { if let Some(filter) = self.filter.as_mut() { filter.add(v.as_ref())?; @@ -248,6 +304,7 @@ where } } + debug!(target: "nippy-jar", ?row_count, "Encoding offsets index list."); self.offsets_index = PrefixSummedEliasFano::from_slice(&offsets_index)?; Ok(()) } @@ -271,7 +328,7 @@ where // Temporary buffer to avoid multiple reallocations if compressing to a buffer (eg. zstd w/ // dict) - let mut tmp_buf = Vec::with_capacity(100); + let mut tmp_buf = Vec::with_capacity(1_000_000); // Write all rows while taking all row start offsets let mut row_number = 0u64; @@ -279,16 +336,21 @@ where let mut column_iterators = columns.into_iter().map(|v| v.into_iter()).collect::>().into_iter(); + debug!(target: "nippy-jar", compressor=?self.compressor, "Writing rows."); + loop { let mut iterators = Vec::with_capacity(self.columns); // Write the column value of each row // TODO: iter_mut if we remove the IntoIterator interface. + let mut uncompressed_row_size = 0; for (column_number, mut column_iter) in column_iterators.enumerate() { offsets.push(file.stream_position()? as usize); match column_iter.next() { Some(Ok(value)) => { + uncompressed_row_size += value.len(); + if let Some(compression) = &self.compressor { // Special zstd case with dictionaries if let (Some(dict_compressors), Compressors::Zstd(_)) = @@ -301,7 +363,9 @@ where Some(dict_compressors.get_mut(column_number).expect("exists")), )?; } else { - compression.compress_to(&value, &mut file)?; + let before = tmp_buf.len(); + let len = compression.compress_to(&value, &mut tmp_buf)?; + file.write_all(&tmp_buf[before..before + len])?; } } else { file.write_all(&value)?; @@ -319,7 +383,10 @@ where iterators.push(column_iter); } + tmp_buf.clear(); row_number += 1; + self.max_row_size = self.max_row_size.max(uncompressed_row_size); + if row_number == total_rows { break } @@ -330,12 +397,16 @@ where // Write offsets and offset index to file self.freeze_offsets(offsets)?; + debug!(target: "nippy-jar", jar=?self, "Finished."); + Ok(()) } /// Freezes offsets and its own index. fn freeze_offsets(&mut self, offsets: Vec) -> Result<(), NippyJarError> { if !offsets.is_empty() { + debug!(target: "nippy-jar", "Encoding offsets list."); + let mut builder = EliasFanoBuilder::new(*offsets.last().expect("qed") + 1, offsets.len())?; @@ -344,9 +415,13 @@ where } self.offsets = builder.build().enable_rank(); } + + debug!(target: "nippy-jar", path=?self.index_path(), "Writing offsets and offsets index to file."); + let mut file = File::create(self.index_path())?; self.offsets.serialize_into(&mut file)?; - self.offsets_index.serialize_into(file)?; + self.offsets_index.serialize_into(&mut file)?; + self.max_row_size.serialize_into(file)?; Ok(()) } @@ -370,6 +445,8 @@ where let _ = phf.get_index(&[])?; } + debug!(target: "nippy-jar", path=?self.data_path(), "Opening data file."); + Ok(File::create(self.data_path())?) } @@ -517,10 +594,6 @@ mod tests { // // Add more columns until max_capacity assert!(InclusionFilter::add(&mut nippy, &col1[2]).is_ok()); assert!(InclusionFilter::add(&mut nippy, &col1[3]).is_ok()); - assert!(matches!( - InclusionFilter::add(&mut nippy, &col1[4]), - Err(NippyJarError::FilterMaxCapacity) - )); nippy.freeze(vec![clone_with_result(&col1), clone_with_result(&col2)], num_rows).unwrap(); let loaded_nippy = NippyJar::load_without_header(file_path.path()).unwrap(); @@ -542,13 +615,13 @@ mod tests { let file_path = tempfile::NamedTempFile::new().unwrap(); let nippy = NippyJar::new_without_header(num_columns, file_path.path()); - assert!(nippy.compressor.is_none()); + assert!(nippy.compressor().is_none()); let mut nippy = NippyJar::new_without_header(num_columns, file_path.path()).with_zstd(true, 5000); - assert!(nippy.compressor.is_some()); + assert!(nippy.compressor().is_some()); - if let Some(Compressors::Zstd(zstd)) = &mut nippy.compressor { + if let Some(Compressors::Zstd(zstd)) = &mut nippy.compressor_mut() { assert!(matches!(zstd.generate_compressors(), Err(NippyJarError::CompressorNotReady))); // Make sure the number of column iterators match the initial set up ones. @@ -567,7 +640,7 @@ mod tests { nippy.prepare_compression(vec![col1.clone(), col2.clone()]).unwrap(); - if let Some(Compressors::Zstd(zstd)) = &nippy.compressor { + if let Some(Compressors::Zstd(zstd)) = &nippy.compressor() { assert!(matches!( (&zstd.state, zstd.raw_dictionaries.as_ref().map(|dict| dict.len())), (compression::ZstdState::Ready, Some(columns)) if columns == num_columns @@ -580,11 +653,11 @@ mod tests { assert_eq!(nippy, loaded_nippy); let mut dicts = vec![]; - if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor.as_mut() { + if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor_mut() { dicts = zstd.generate_decompress_dictionaries().unwrap() } - if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor.as_ref() { + if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor() { let mut cursor = NippyJarCursor::new( &loaded_nippy, Some(zstd.generate_decompressors(&dicts).unwrap()), @@ -594,12 +667,50 @@ mod tests { // Iterate over compressed values and compare let mut row_index = 0usize; while let Some(row) = cursor.next_row().unwrap() { - assert_eq!((&row[0], &row[1]), (&col1[row_index], &col2[row_index])); + assert_eq!( + (row[0], row[1]), + (col1[row_index].as_slice(), col2[row_index].as_slice()) + ); row_index += 1; } } } + #[test] + fn test_lz4() { + let (col1, col2) = test_data(None); + let num_rows = col1.len() as u64; + let num_columns = 2; + let file_path = tempfile::NamedTempFile::new().unwrap(); + + let nippy = NippyJar::new_without_header(num_columns, file_path.path()); + assert!(nippy.compressor().is_none()); + + let mut nippy = NippyJar::new_without_header(num_columns, file_path.path()).with_lz4(); + assert!(nippy.compressor().is_some()); + + nippy.freeze(vec![clone_with_result(&col1), clone_with_result(&col2)], num_rows).unwrap(); + + let loaded_nippy = NippyJar::load_without_header(file_path.path()).unwrap(); + assert_eq!(nippy, loaded_nippy); + + if let Some(Compressors::Lz4(_)) = loaded_nippy.compressor() { + let mut cursor = NippyJarCursor::new(&loaded_nippy, None).unwrap(); + + // Iterate over compressed values and compare + let mut row_index = 0usize; + while let Some(row) = cursor.next_row().unwrap() { + assert_eq!( + (row[0], row[1]), + (col1[row_index].as_slice(), col2[row_index].as_slice()) + ); + row_index += 1; + } + } else { + panic!("Expected Lz4 compressor") + } + } + #[test] fn test_zstd_no_dictionaries() { let (col1, col2) = test_data(None); @@ -608,18 +719,18 @@ mod tests { let file_path = tempfile::NamedTempFile::new().unwrap(); let nippy = NippyJar::new_without_header(num_columns, file_path.path()); - assert!(nippy.compressor.is_none()); + assert!(nippy.compressor().is_none()); let mut nippy = NippyJar::new_without_header(num_columns, file_path.path()).with_zstd(false, 5000); - assert!(nippy.compressor.is_some()); + assert!(nippy.compressor().is_some()); nippy.freeze(vec![clone_with_result(&col1), clone_with_result(&col2)], num_rows).unwrap(); let loaded_nippy = NippyJar::load_without_header(file_path.path()).unwrap(); assert_eq!(nippy, loaded_nippy); - if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor.as_ref() { + if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor() { assert!(!zstd.use_dict); let mut cursor = NippyJarCursor::new(&loaded_nippy, None).unwrap(); @@ -627,7 +738,10 @@ mod tests { // Iterate over compressed values and compare let mut row_index = 0usize; while let Some(row) = cursor.next_row().unwrap() { - assert_eq!((&row[0], &row[1]), (&col1[row_index], &col2[row_index])); + assert_eq!( + (row[0], row[1]), + (col1[row_index].as_slice(), col2[row_index].as_slice()) + ); row_index += 1; } } else { @@ -670,16 +784,16 @@ mod tests { { let mut loaded_nippy = NippyJar::::load(file_path.path()).unwrap(); - assert!(loaded_nippy.compressor.is_some()); + assert!(loaded_nippy.compressor().is_some()); assert!(loaded_nippy.filter.is_some()); assert!(loaded_nippy.phf.is_some()); assert_eq!(loaded_nippy.user_header().block_start, block_start); let mut dicts = vec![]; - if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor.as_mut() { + if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor_mut() { dicts = zstd.generate_decompress_dictionaries().unwrap() } - if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor.as_ref() { + if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor() { let mut cursor = NippyJarCursor::new( &loaded_nippy, Some(zstd.generate_decompressors(&dicts).unwrap()), @@ -689,7 +803,10 @@ mod tests { // Iterate over compressed values and compare let mut row_num = 0usize; while let Some(row) = cursor.next_row().unwrap() { - assert_eq!((&row[0], &row[1]), (&data[0][row_num], &data[1][row_num])); + assert_eq!( + (row[0], row[1]), + (data[0][row_num].as_slice(), data[1][row_num].as_slice()) + ); row_num += 1; } @@ -700,12 +817,20 @@ mod tests { for (row_num, (v0, v1)) in data { // Simulates `by_hash` queries by iterating col1 values, which were used to // create the inner index. - let row_by_value = cursor.row_by_key(v0).unwrap().unwrap(); - assert_eq!((&row_by_value[0], &row_by_value[1]), (v0, v1)); - - // Simulates `by_number` queries - let row_by_num = cursor.row_by_number(row_num).unwrap().unwrap(); - assert_eq!(row_by_value, row_by_num); + { + let row_by_value = cursor + .row_by_key(v0) + .unwrap() + .unwrap() + .iter() + .map(|a| a.to_vec()) + .collect::>(); + assert_eq!((&row_by_value[0], &row_by_value[1]), (v0, v1)); + + // Simulates `by_number` queries + let row_by_num = cursor.row_by_number(row_num).unwrap().unwrap(); + assert_eq!(row_by_value, row_by_num); + } } } } @@ -738,10 +863,10 @@ mod tests { let mut loaded_nippy = NippyJar::load_without_header(file_path.path()).unwrap(); let mut dicts = vec![]; - if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor.as_mut() { + if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor_mut() { dicts = zstd.generate_decompress_dictionaries().unwrap() } - if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor.as_ref() { + if let Some(Compressors::Zstd(zstd)) = loaded_nippy.compressor() { let mut cursor = NippyJarCursor::new( &loaded_nippy, Some(zstd.generate_decompressors(&dicts).unwrap()), @@ -763,7 +888,10 @@ mod tests { let row_by_value = cursor .row_by_key_with_cols::(v0) .unwrap() - .unwrap(); + .unwrap() + .iter() + .map(|a| a.to_vec()) + .collect::>(); assert_eq!((&row_by_value[0], &row_by_value[1]), (*v0, *v1)); // Simulates `by_number` queries @@ -782,7 +910,10 @@ mod tests { let row_by_value = cursor .row_by_key_with_cols::(v0) .unwrap() - .unwrap(); + .unwrap() + .iter() + .map(|a| a.to_vec()) + .collect::>(); assert_eq!(row_by_value.len(), 1); assert_eq!(&row_by_value[0], *v0); @@ -803,7 +934,10 @@ mod tests { let row_by_value = cursor .row_by_key_with_cols::(v0) .unwrap() - .unwrap(); + .unwrap() + .iter() + .map(|a| a.to_vec()) + .collect::>(); assert_eq!(row_by_value.len(), 1); assert_eq!(&row_by_value[0], *v1); diff --git a/crates/storage/nippy-jar/src/phf/fmph.rs b/crates/storage/nippy-jar/src/phf/fmph.rs index e540bae64fe5..8753334b5133 100644 --- a/crates/storage/nippy-jar/src/phf/fmph.rs +++ b/crates/storage/nippy-jar/src/phf/fmph.rs @@ -60,7 +60,6 @@ impl PartialEq for Fmph { impl std::fmt::Debug for Fmph { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("Fmph") - .field("level_sizes", &self.function.as_ref().map(|f| f.level_sizes())) .field("bytes_size", &self.function.as_ref().map(|f| f.write_bytes())) .finish_non_exhaustive() } diff --git a/crates/storage/nippy-jar/src/phf/go_fmph.rs b/crates/storage/nippy-jar/src/phf/go_fmph.rs index fd244cd1fc01..b2ed28f685fc 100644 --- a/crates/storage/nippy-jar/src/phf/go_fmph.rs +++ b/crates/storage/nippy-jar/src/phf/go_fmph.rs @@ -60,7 +60,6 @@ impl PartialEq for GoFmph { impl std::fmt::Debug for GoFmph { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("GoFmph") - .field("level_sizes", &self.function.as_ref().map(|f| f.level_sizes())) .field("bytes_size", &self.function.as_ref().map(|f| f.write_bytes())) .finish_non_exhaustive() } diff --git a/crates/storage/provider/src/providers/snapshot.rs b/crates/storage/provider/src/providers/snapshot.rs index e53ebaa93ce5..72aed71d0b81 100644 --- a/crates/storage/provider/src/providers/snapshot.rs +++ b/crates/storage/provider/src/providers/snapshot.rs @@ -3,26 +3,37 @@ use reth_db::{ table::{Decompress, Table}, HeaderTD, }; -use reth_interfaces::RethResult; -use reth_nippy_jar::{NippyJar, NippyJarCursor}; +use reth_interfaces::{provider::ProviderError, RethResult}; +use reth_nippy_jar::{compression::Decompressor, NippyJar, NippyJarCursor}; use reth_primitives::{BlockHash, BlockNumber, Header, SealedHeader, U256}; use std::ops::RangeBounds; /// SnapshotProvider /// -/// WIP Rudimentary impl just for testes +/// WIP Rudimentary impl just for tests /// TODO: should be able to walk through snapshot files/block_ranges /// TODO: Arc over NippyJars and/or NippyJarCursors (LRU) #[derive(Debug)] pub struct SnapshotProvider<'a> { /// NippyJar pub jar: &'a NippyJar, + /// Starting snapshot block + pub jar_start_block: u64, } impl<'a> SnapshotProvider<'a> { - fn cursor(&self) -> NippyJarCursor<'a> { + /// Creates cursor + pub fn cursor(&self) -> NippyJarCursor<'a> { NippyJarCursor::new(self.jar, None).unwrap() } + + /// Creates cursor with zstd decompressors + pub fn cursor_with_decompressors( + &self, + decompressors: Vec>, + ) -> NippyJarCursor<'a> { + NippyJarCursor::new(self.jar, Some(decompressors)).unwrap() + } } impl<'a> HeaderProvider for SnapshotProvider<'a> { @@ -31,7 +42,7 @@ impl<'a> HeaderProvider for SnapshotProvider<'a> { let mut cursor = self.cursor(); let header = Header::decompress( - &cursor.row_by_key_with_cols::<0b01, 2>(&block_hash.0).unwrap().unwrap()[0], + cursor.row_by_key_with_cols::<0b01, 2>(&block_hash.0).unwrap().unwrap()[0], ) .unwrap(); @@ -43,8 +54,14 @@ impl<'a> HeaderProvider for SnapshotProvider<'a> { Ok(None) } - fn header_by_number(&self, _num: BlockNumber) -> RethResult> { - unimplemented!(); + fn header_by_number(&self, num: BlockNumber) -> RethResult> { + Header::decompress( + self.cursor() + .row_by_number_with_cols::<0b01, 2>((num - self.jar_start_block) as usize)? + .ok_or(ProviderError::HeaderNotFound(num.into()))?[0], + ) + .map(Some) + .map_err(Into::into) } fn header_td(&self, block_hash: &BlockHash) -> RethResult> { @@ -53,8 +70,8 @@ impl<'a> HeaderProvider for SnapshotProvider<'a> { let row = cursor.row_by_key_with_cols::<0b11, 2>(&block_hash.0).unwrap().unwrap(); - let header = Header::decompress(&row[0]).unwrap(); - let td = ::Value::decompress(&row[1]).unwrap(); + let header = Header::decompress(row[0]).unwrap(); + let td = ::Value::decompress(row[1]).unwrap(); if &header.hash_slow() == block_hash { return Ok(Some(td.0)) @@ -166,6 +183,7 @@ mod test { create_snapshot_T1_T2::( &tx, range, + None, none_vec, Some(hashes), row_count as usize, @@ -179,7 +197,7 @@ mod test { let jar = NippyJar::load_without_header(snap_file.path()).unwrap(); let db_provider = factory.provider().unwrap(); - let snap_provider = SnapshotProvider { jar: &jar }; + let snap_provider = SnapshotProvider { jar: &jar, jar_start_block: 0 }; assert!(!headers.is_empty()); From c825aafbc8a477d6c4da125a66098f4b3ca685f5 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Fri, 6 Oct 2023 13:40:38 -0400 Subject: [PATCH 12/23] fix: only accept rpc enveloped format for transactions in payload (#4928) --- crates/payload/builder/src/payload.rs | 8 +- crates/primitives/src/transaction/mod.rs | 60 ++++++++-- crates/primitives/src/transaction/pooled.rs | 26 ++++- crates/rpc/rpc-engine-api/tests/it/payload.rs | 4 +- crates/rpc/rpc-types-compat/src/engine/mod.rs | 2 +- .../rpc-types-compat/src/engine/payload.rs | 107 ++++++++++++++++-- 6 files changed, 175 insertions(+), 32 deletions(-) diff --git a/crates/payload/builder/src/payload.rs b/crates/payload/builder/src/payload.rs index 64a25a6ffc39..519dadd71df6 100644 --- a/crates/payload/builder/src/payload.rs +++ b/crates/payload/builder/src/payload.rs @@ -10,8 +10,8 @@ use reth_rpc_types::engine::{ PayloadId, }; use reth_rpc_types_compat::engine::payload::{ - convert_block_to_payload_field_v2, convert_standalonewithdraw_to_withdrawal, - try_block_to_payload_v1, try_block_to_payload_v3, + block_to_payload_v3, convert_block_to_payload_field_v2, + convert_standalone_withdraw_to_withdrawal, try_block_to_payload_v1, }; use revm_primitives::{BlobExcessGasAndPrice, BlockEnv, CfgEnv, SpecId}; /// Contains the built payload. @@ -100,7 +100,7 @@ impl From for ExecutionPayloadEnvelopeV3 { let BuiltPayload { block, fees, sidecars, .. } = value; ExecutionPayloadEnvelopeV3 { - execution_payload: try_block_to_payload_v3(block), + execution_payload: block_to_payload_v3(block), block_value: fees, // From the engine API spec: // @@ -148,7 +148,7 @@ impl PayloadBuilderAttributes { |withdrawals: Vec| { withdrawals .into_iter() - .map(convert_standalonewithdraw_to_withdrawal) // Removed the parentheses here + .map(convert_standalone_withdraw_to_withdrawal) // Removed the parentheses here .collect::>() }, ); diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index d0562dcf305f..65bedeb5232e 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -876,7 +876,7 @@ impl TransactionSigned { /// /// For legacy transactions, it encodes the RLP of the transaction into the buffer: `rlp(tx)` /// For EIP-2718 typed it encodes the type of the transaction followed by the rlp of the - /// transaction: `type` + `rlp(tx)` + /// transaction: `type || rlp(tx)` pub fn encode_enveloped(&self, out: &mut dyn bytes::BufMut) { self.encode_inner(out, false) } @@ -927,6 +927,9 @@ impl TransactionSigned { /// Decodes legacy transaction from the data buffer into a tuple. /// /// This expects `rlp(legacy_tx)` + /// + /// Refer to the docs for [Self::decode_rlp_legacy_transaction] for details on the exact + /// format expected. // TODO: make buf advancement semantics consistent with `decode_enveloped_typed_transaction`, // so decoding methods do not need to manually advance the buffer pub(crate) fn decode_rlp_legacy_transaction_tuple( @@ -956,6 +959,12 @@ impl TransactionSigned { /// Decodes legacy transaction from the data buffer. /// + /// This should be used _only_ be used in general transaction decoding methods, which have + /// already ensured that the input is a legacy transaction with the following format: + /// `rlp(legacy_tx)` + /// + /// Legacy transactions are encoded as lists, so the input should start with a RLP list header. + /// /// This expects `rlp(legacy_tx)` // TODO: make buf advancement semantics consistent with `decode_enveloped_typed_transaction`, // so decoding methods do not need to manually advance the buffer @@ -969,7 +978,14 @@ impl TransactionSigned { /// Decodes en enveloped EIP-2718 typed transaction. /// - /// CAUTION: this expects that `data` is `[id, rlp(tx)]` + /// This should be used _only_ be used internally in general transaction decoding methods, + /// which have already ensured that the input is a typed transaction with the following format: + /// `tx_type || rlp(tx)` + /// + /// Note that this format does not start with any RLP header, and instead starts with a single + /// byte indicating the transaction type. + /// + /// CAUTION: this expects that `data` is `tx_type || rlp(tx)` pub fn decode_enveloped_typed_transaction( data: &mut &[u8], ) -> alloy_rlp::Result { @@ -1003,12 +1019,23 @@ impl TransactionSigned { Ok(signed) } - /// Decodes the "raw" format of transaction (e.g. `eth_sendRawTransaction`). + /// Decodes the "raw" format of transaction (similar to `eth_sendRawTransaction`). + /// + /// This should be used for any RPC method that accepts a raw transaction, **excluding** raw + /// EIP-4844 transactions in `eth_sendRawTransaction`. Currently, this includes: + /// * `eth_sendRawTransaction` for non-EIP-4844 transactions. + /// * All versions of `engine_newPayload`, in the `transactions` field. + /// + /// A raw transaction is either a legacy transaction or EIP-2718 typed transaction. /// - /// The raw transaction is either a legacy transaction or EIP-2718 typed transaction - /// For legacy transactions, the format is encoded as: `rlp(tx)` - /// For EIP-2718 typed transaction, the format is encoded as the type of the transaction - /// followed by the rlp of the transaction: `type` + `rlp(tx)` + /// For legacy transactions, the format is encoded as: `rlp(tx)`. This format will start with a + /// RLP list header. + /// + /// For EIP-2718 typed transactions, the format is encoded as the type of the transaction + /// followed by the rlp of the transaction: `type || rlp(tx)`. + /// + /// To decode EIP-4844 transactions in `eth_sendRawTransaction`, use + /// [PooledTransactionsElement::decode_enveloped]. pub fn decode_enveloped(tx: Bytes) -> alloy_rlp::Result { let mut data = tx.as_ref(); @@ -1045,7 +1072,24 @@ impl Encodable for TransactionSigned { /// This `Decodable` implementation only supports decoding rlp encoded transactions as it's used by /// p2p. /// -/// CAUTION: this expects that the given buf contains rlp +/// The p2p encoding format always includes an RLP header, although the type RLP header depends on +/// whether or not the transaction is a legacy transaction. +/// +/// If the transaction is a legacy transaction, it is just encoded as a RLP list: `rlp(tx)`. +/// +/// If the transaction is a typed transaction, it is encoded as a RLP string: +/// `rlp(type || rlp(tx))` +/// +/// This cannot be used for decoding EIP-4844 transactions in p2p, since the EIP-4844 variant of +/// [TransactionSigned] does not include the blob sidecar. For a general purpose decoding method +/// suitable for decoding transactions from p2p, see [PooledTransactionsElement]. +/// +/// CAUTION: Due to a quirk in [Header::decode], this method will succeed even if a typed +/// transaction is encoded in the RPC format, and does not start with a RLP header. This is because +/// [Header::decode] does not advance the buffer, and returns a length-1 string header if the first +/// byte is less than `0xf7`. This causes this decode implementation to pass unaltered buffer to +/// [TransactionSigned::decode_enveloped_typed_transaction], which expects the RPC format. Despite +/// this quirk, this should **not** be used for RPC methods that accept raw transactions. impl Decodable for TransactionSigned { fn decode(buf: &mut &[u8]) -> alloy_rlp::Result { // decode header diff --git a/crates/primitives/src/transaction/pooled.rs b/crates/primitives/src/transaction/pooled.rs index a2c5a2aa79ec..8f32435efdb7 100644 --- a/crates/primitives/src/transaction/pooled.rs +++ b/crates/primitives/src/transaction/pooled.rs @@ -109,12 +109,28 @@ impl PooledTransactionsElement { /// Decodes the "raw" format of transaction (e.g. `eth_sendRawTransaction`). /// - /// The raw transaction is either a legacy transaction or EIP-2718 typed transaction - /// For legacy transactions, the format is encoded as: `rlp(tx)` - /// For EIP-2718 typed transaction, the format is encoded as the type of the transaction - /// followed by the rlp of the transaction: `type` + `rlp(tx)` + /// This should be used for `eth_sendRawTransaction`, for any transaction type. Blob + /// transactions **must** include the blob sidecar as part of the raw encoding. /// - /// For encoded EIP-4844 transactions, the blob sidecar _must_ be included. + /// This method can not be used for decoding the `transactions` field of `engine_newPayload`, + /// because EIP-4844 transactions for that method do not include the blob sidecar. The blobs + /// are supplied in an argument separate from the payload. + /// + /// A raw transaction is either a legacy transaction or EIP-2718 typed transaction, with a + /// special case for EIP-4844 transactions. + /// + /// For legacy transactions, the format is encoded as: `rlp(tx)`. This format will start with a + /// RLP list header. + /// + /// For EIP-2718 typed transactions, the format is encoded as the type of the transaction + /// followed by the rlp of the transaction: `type || rlp(tx)`. + /// + /// For EIP-4844 transactions, the format includes a blob sidecar (the blobs, commitments, and + /// proofs) after the transaction: + /// `type || rlp([tx_payload_body, blobs, commitments, proofs])` + /// + /// Where `tx_payload_body` is encoded as a RLP list: + /// `[chain_id, nonce, max_priority_fee_per_gas, ..., y_parity, r, s]` pub fn decode_enveloped(tx: Bytes) -> alloy_rlp::Result { let mut data = tx.as_ref(); diff --git a/crates/rpc/rpc-engine-api/tests/it/payload.rs b/crates/rpc/rpc-engine-api/tests/it/payload.rs index f8aa231af73e..5b012726cee7 100644 --- a/crates/rpc/rpc-engine-api/tests/it/payload.rs +++ b/crates/rpc/rpc-engine-api/tests/it/payload.rs @@ -13,7 +13,7 @@ use reth_rpc_types::engine::{ ExecutionPayload, ExecutionPayloadBodyV1, ExecutionPayloadV1, PayloadError, }; use reth_rpc_types_compat::engine::payload::{ - convert_standalonewithdraw_to_withdrawal, convert_to_payload_body_v1, try_block_to_payload, + convert_standalone_withdraw_to_withdrawal, convert_to_payload_body_v1, try_block_to_payload, try_block_to_payload_v1, try_into_sealed_block, try_payload_v1_to_block, }; @@ -49,7 +49,7 @@ fn payload_body_roundtrip() { let withdraw = payload_body.withdrawals.map(|withdrawals| { withdrawals .into_iter() - .map(convert_standalonewithdraw_to_withdrawal) + .map(convert_standalone_withdraw_to_withdrawal) .collect::>() }); assert_eq!(block.withdrawals, withdraw); diff --git a/crates/rpc/rpc-types-compat/src/engine/mod.rs b/crates/rpc/rpc-types-compat/src/engine/mod.rs index 10e60327b871..e03ba6f4cd8d 100644 --- a/crates/rpc/rpc-types-compat/src/engine/mod.rs +++ b/crates/rpc/rpc-types-compat/src/engine/mod.rs @@ -1,6 +1,6 @@ //! Standalone functions for engine specific rpc type conversions pub mod payload; pub use payload::{ - convert_standalonewithdraw_to_withdrawal, convert_withdrawal_to_standalonewithdraw, + convert_standalone_withdraw_to_withdrawal, convert_withdrawal_to_standalone_withdraw, try_block_to_payload_v1, try_into_sealed_block, try_payload_v1_to_block, }; diff --git a/crates/rpc/rpc-types-compat/src/engine/payload.rs b/crates/rpc/rpc-types-compat/src/engine/payload.rs index d69b5af85d08..e0fad50606cb 100644 --- a/crates/rpc/rpc-types-compat/src/engine/payload.rs +++ b/crates/rpc/rpc-types-compat/src/engine/payload.rs @@ -1,6 +1,5 @@ //! Standalone Conversion Functions for Handling Different Versions of Execution Payloads in //! Ethereum's Engine -use alloy_rlp::Decodable; use reth_primitives::{ constants::{MAXIMUM_EXTRA_DATA_SIZE, MIN_PROTOCOL_BASE_FEE_U256}, proofs::{self, EMPTY_LIST_HASH}, @@ -23,8 +22,8 @@ pub fn try_payload_v1_to_block(payload: ExecutionPayloadV1) -> Result, _>>()?; let transactions_root = proofs::calculate_transaction_root(&transactions); @@ -68,7 +67,7 @@ pub fn try_payload_v2_to_block(payload: ExecutionPayloadV2) -> Result = payload .withdrawals .iter() - .map(|w| convert_standalonewithdraw_to_withdrawal(w.clone())) + .map(|w| convert_standalone_withdraw_to_withdrawal(w.clone())) .collect(); let withdrawals_root = proofs::calculate_withdrawals_root(&withdrawals); base_sealed_block.withdrawals = Some(withdrawals); @@ -92,7 +91,7 @@ pub fn try_payload_v3_to_block(payload: ExecutionPayloadV3) -> Result ExecutionPayload { if value.header.parent_beacon_block_root.is_some() { // block with parent beacon block root: V3 - ExecutionPayload::V3(try_block_to_payload_v3(value)) + ExecutionPayload::V3(block_to_payload_v3(value)) } else if value.withdrawals.is_some() { // block with withdrawals: V2 ExecutionPayload::V2(try_block_to_payload_v2(value)) @@ -147,7 +146,7 @@ pub fn try_block_to_payload_v2(value: SealedBlock) -> ExecutionPayloadV2 { .clone() .unwrap_or_default() .into_iter() - .map(convert_withdrawal_to_standalonewithdraw) + .map(convert_withdrawal_to_standalone_withdraw) .collect(); ExecutionPayloadV2 { @@ -172,7 +171,7 @@ pub fn try_block_to_payload_v2(value: SealedBlock) -> ExecutionPayloadV2 { } /// Converts [SealedBlock] to [ExecutionPayloadV3] -pub fn try_block_to_payload_v3(value: SealedBlock) -> ExecutionPayloadV3 { +pub fn block_to_payload_v3(value: SealedBlock) -> ExecutionPayloadV3 { let transactions = value .body .iter() @@ -188,7 +187,7 @@ pub fn try_block_to_payload_v3(value: SealedBlock) -> ExecutionPayloadV3 { .clone() .unwrap_or_default() .into_iter() - .map(convert_withdrawal_to_standalonewithdraw) + .map(convert_withdrawal_to_standalone_withdraw) .collect(); ExecutionPayloadV3 { @@ -249,7 +248,7 @@ pub fn convert_payload_input_v2_to_payload(value: ExecutionPayloadInputV2) -> Ex /// Converts [SealedBlock] to [ExecutionPayloadInputV2] pub fn convert_block_to_payload_input_v2(value: SealedBlock) -> ExecutionPayloadInputV2 { let withdraw = value.withdrawals.clone().map(|withdrawals| { - withdrawals.into_iter().map(convert_withdrawal_to_standalonewithdraw).collect::>() + withdrawals.into_iter().map(convert_withdrawal_to_standalone_withdraw).collect::>() }); ExecutionPayloadInputV2 { withdrawals: withdraw, @@ -321,7 +320,7 @@ pub fn validate_block_hash( } /// Converts [Withdrawal] to [reth_rpc_types::engine::payload::Withdrawal] -pub fn convert_withdrawal_to_standalonewithdraw( +pub fn convert_withdrawal_to_standalone_withdraw( withdrawal: Withdrawal, ) -> reth_rpc_types::engine::payload::Withdrawal { reth_rpc_types::engine::payload::Withdrawal { @@ -333,7 +332,7 @@ pub fn convert_withdrawal_to_standalonewithdraw( } /// Converts [reth_rpc_types::engine::payload::Withdrawal] to [Withdrawal] -pub fn convert_standalonewithdraw_to_withdrawal( +pub fn convert_standalone_withdraw_to_withdrawal( standalone: reth_rpc_types::engine::payload::Withdrawal, ) -> Withdrawal { Withdrawal { @@ -355,8 +354,92 @@ pub fn convert_to_payload_body_v1(value: Block) -> ExecutionPayloadBodyV1 { value.withdrawals.map(|withdrawals| { withdrawals .into_iter() - .map(convert_withdrawal_to_standalonewithdraw) + .map(convert_withdrawal_to_standalone_withdraw) .collect::>() }); ExecutionPayloadBodyV1 { transactions: transactions.collect(), withdrawals: withdraw } } + +#[cfg(test)] +mod tests { + use reth_primitives::{hex, Bytes, U256, U64}; + use reth_rpc_types::{engine::ExecutionPayloadV3, ExecutionPayloadV1, ExecutionPayloadV2}; + + use super::{block_to_payload_v3, try_payload_v3_to_block}; + + #[test] + fn roundtrip_payload_to_block() { + let first_transaction_raw = Bytes::from_static(&hex!("02f9017a8501a1f0ff438211cc85012a05f2008512a05f2000830249f094d5409474fd5a725eab2ac9a8b26ca6fb51af37ef80b901040cc7326300000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000001bdd2ed4b616c800000000000000000000000000001e9ee781dd4b97bdef92e5d1785f73a1f931daa20000000000000000000000007a40026a3b9a41754a95eec8c92c6b99886f440c000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000020000000000000000000000009ae80eb647dd09968488fa1d7e412bf8558a0b7a0000000000000000000000000f9815537d361cb02befd9918c95c97d4d8a4a2bc001a0ba8f1928bb0efc3fcd01524a2039a9a2588fa567cd9a7cc18217e05c615e9d69a0544bfd11425ac7748e76b3795b57a5563e2b0eff47b5428744c62ff19ccfc305")[..]); + let second_transaction_raw = Bytes::from_static(&hex!("03f901388501a1f0ff430c843b9aca00843b9aca0082520894e7249813d8ccf6fa95a2203f46a64166073d58878080c005f8c6a00195f6dff17753fc89b60eac6477026a805116962c9e412de8015c0484e661c1a001aae314061d4f5bbf158f15d9417a238f9589783f58762cd39d05966b3ba2fba0013f5be9b12e7da06f0dd11a7bdc4e0db8ef33832acc23b183bd0a2c1408a757a0019d9ac55ea1a615d92965e04d960cb3be7bff121a381424f1f22865bd582e09a001def04412e76df26fefe7b0ed5e10580918ae4f355b074c0cfe5d0259157869a0011c11a415db57e43db07aef0de9280b591d65ca0cce36c7002507f8191e5d4a80a0c89b59970b119187d97ad70539f1624bbede92648e2dc007890f9658a88756c5a06fb2e3d4ce2c438c0856c2de34948b7032b1aadc4642a9666228ea8cdc7786b7")[..]); + + let new_payload = ExecutionPayloadV3 { + payload_inner: ExecutionPayloadV2 { + payload_inner: ExecutionPayloadV1 { + base_fee_per_gas: U256::from(7u64), + block_number: U64::from(0xa946u64), + block_hash: hex!("a5ddd3f286f429458a39cafc13ffe89295a7efa8eb363cf89a1a4887dbcf272b").into(), + logs_bloom: hex!("00200004000000000000000080000000000200000000000000000000000000000000200000000000000000000000000000000000800000000200000000000000000000000000000000000008000000200000000000000000000001000000000000000000000000000000800000000000000000000100000000000030000000000000000040000000000000000000000000000000000800080080404000000000000008000000000008200000000000200000000000000000000000000000000000000002000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000100000000000000000000").into(), + extra_data: hex!("d883010d03846765746888676f312e32312e31856c696e7578").into(), + gas_limit: U64::from(0x1c9c380), + gas_used: U64::from(0x1f4a9), + timestamp: U64::from(0x651f35b8), + fee_recipient: hex!("f97e180c050e5ab072211ad2c213eb5aee4df134").into(), + parent_hash: hex!("d829192799c73ef28a7332313b3c03af1f2d5da2c36f8ecfafe7a83a3bfb8d1e").into(), + prev_randao: hex!("753888cc4adfbeb9e24e01c84233f9d204f4a9e1273f0e29b43c4c148b2b8b7e").into(), + receipts_root: hex!("4cbc48e87389399a0ea0b382b1c46962c4b8e398014bf0cc610f9c672bee3155").into(), + state_root: hex!("017d7fa2b5adb480f5e05b2c95cb4186e12062eed893fc8822798eed134329d1").into(), + transactions: vec![first_transaction_raw, second_transaction_raw], + }, + withdrawals: vec![], + }, + blob_gas_used: U64::from(0xc0000), + excess_blob_gas: U64::from(0x580000), + }; + + let mut block = try_payload_v3_to_block(new_payload.clone()).unwrap(); + + // this newPayload came with a parent beacon block root, we need to manually insert it + // before hashing + let parent_beacon_block_root = + hex!("531cd53b8e68deef0ea65edfa3cda927a846c307b0907657af34bc3f313b5871"); + block.header.parent_beacon_block_root = Some(parent_beacon_block_root.into()); + + let converted_payload = block_to_payload_v3(block.seal_slow()); + + // ensure the payloads are the same + assert_eq!(new_payload, converted_payload); + } + + #[test] + fn payload_to_block_rejects_network_encoded_tx() { + let first_transaction_raw = Bytes::from_static(&hex!("b9017e02f9017a8501a1f0ff438211cc85012a05f2008512a05f2000830249f094d5409474fd5a725eab2ac9a8b26ca6fb51af37ef80b901040cc7326300000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000001bdd2ed4b616c800000000000000000000000000001e9ee781dd4b97bdef92e5d1785f73a1f931daa20000000000000000000000007a40026a3b9a41754a95eec8c92c6b99886f440c000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000020000000000000000000000009ae80eb647dd09968488fa1d7e412bf8558a0b7a0000000000000000000000000f9815537d361cb02befd9918c95c97d4d8a4a2bc001a0ba8f1928bb0efc3fcd01524a2039a9a2588fa567cd9a7cc18217e05c615e9d69a0544bfd11425ac7748e76b3795b57a5563e2b0eff47b5428744c62ff19ccfc305")[..]); + let second_transaction_raw = Bytes::from_static(&hex!("b9013c03f901388501a1f0ff430c843b9aca00843b9aca0082520894e7249813d8ccf6fa95a2203f46a64166073d58878080c005f8c6a00195f6dff17753fc89b60eac6477026a805116962c9e412de8015c0484e661c1a001aae314061d4f5bbf158f15d9417a238f9589783f58762cd39d05966b3ba2fba0013f5be9b12e7da06f0dd11a7bdc4e0db8ef33832acc23b183bd0a2c1408a757a0019d9ac55ea1a615d92965e04d960cb3be7bff121a381424f1f22865bd582e09a001def04412e76df26fefe7b0ed5e10580918ae4f355b074c0cfe5d0259157869a0011c11a415db57e43db07aef0de9280b591d65ca0cce36c7002507f8191e5d4a80a0c89b59970b119187d97ad70539f1624bbede92648e2dc007890f9658a88756c5a06fb2e3d4ce2c438c0856c2de34948b7032b1aadc4642a9666228ea8cdc7786b7")[..]); + + let new_payload = ExecutionPayloadV3 { + payload_inner: ExecutionPayloadV2 { + payload_inner: ExecutionPayloadV1 { + base_fee_per_gas: U256::from(7u64), + block_number: U64::from(0xa946u64), + block_hash: hex!("a5ddd3f286f429458a39cafc13ffe89295a7efa8eb363cf89a1a4887dbcf272b").into(), + logs_bloom: hex!("00200004000000000000000080000000000200000000000000000000000000000000200000000000000000000000000000000000800000000200000000000000000000000000000000000008000000200000000000000000000001000000000000000000000000000000800000000000000000000100000000000030000000000000000040000000000000000000000000000000000800080080404000000000000008000000000008200000000000200000000000000000000000000000000000000002000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000100000000000000000000").into(), + extra_data: hex!("d883010d03846765746888676f312e32312e31856c696e7578").into(), + gas_limit: U64::from(0x1c9c380), + gas_used: U64::from(0x1f4a9), + timestamp: U64::from(0x651f35b8), + fee_recipient: hex!("f97e180c050e5ab072211ad2c213eb5aee4df134").into(), + parent_hash: hex!("d829192799c73ef28a7332313b3c03af1f2d5da2c36f8ecfafe7a83a3bfb8d1e").into(), + prev_randao: hex!("753888cc4adfbeb9e24e01c84233f9d204f4a9e1273f0e29b43c4c148b2b8b7e").into(), + receipts_root: hex!("4cbc48e87389399a0ea0b382b1c46962c4b8e398014bf0cc610f9c672bee3155").into(), + state_root: hex!("017d7fa2b5adb480f5e05b2c95cb4186e12062eed893fc8822798eed134329d1").into(), + transactions: vec![first_transaction_raw, second_transaction_raw], + }, + withdrawals: vec![], + }, + blob_gas_used: U64::from(0xc0000), + excess_blob_gas: U64::from(0x580000), + }; + + let _block = try_payload_v3_to_block(new_payload.clone()) + .expect_err("execution payload conversion requires typed txs without a rlp header"); + } +} From 36306ce9161bc2ec723a56362b5c91c1e25deb07 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Fri, 6 Oct 2023 20:05:03 +0200 Subject: [PATCH 13/23] feat: add cli ext event hooks example (#4935) --- Cargo.lock | 10 ++++ Cargo.toml | 1 + bin/reth/src/cli/ext.rs | 16 +++++- bin/reth/src/cli/mod.rs | 20 ++++++++ examples/cli-extension-event-hooks/Cargo.toml | 11 ++++ .../cli-extension-event-hooks/src/main.rs | 51 +++++++++++++++++++ 6 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 examples/cli-extension-event-hooks/Cargo.toml create mode 100644 examples/cli-extension-event-hooks/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index 4c6f123a7093..07a93662ffe2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1267,6 +1267,16 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cd7cc57abe963c6d3b9d8be5b06ba7c8957a930305ca90304f24ef040aa6f961" +[[package]] +name = "cli-extension-event-hooks" +version = "0.0.0" +dependencies = [ + "clap", + "eyre", + "reth", + "tokio", +] + [[package]] name = "cobs" version = "0.2.3" diff --git a/Cargo.toml b/Cargo.toml index 07c6eb34fbe0..a7a1f74743b6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ members = [ "crates/rpc/rpc-types-compat", "examples", "examples/additional-rpc-namespace-in-cli", + "examples/cli-extension-event-hooks", "examples/rpc-db", "examples/manual-p2p", ] diff --git a/bin/reth/src/cli/ext.rs b/bin/reth/src/cli/ext.rs index c61ef3a9a181..e264340342d1 100644 --- a/bin/reth/src/cli/ext.rs +++ b/bin/reth/src/cli/ext.rs @@ -8,7 +8,7 @@ use clap::Args; use reth_basic_payload_builder::{BasicPayloadJobGenerator, BasicPayloadJobGeneratorConfig}; use reth_payload_builder::{PayloadBuilderHandle, PayloadBuilderService}; use reth_tasks::TaskSpawner; -use std::fmt; +use std::{fmt, marker::PhantomData}; /// A trait that allows for extending parts of the CLI with additional functionality. /// @@ -119,6 +119,14 @@ impl RethNodeCommandConfig for DefaultRethNodeCommandConfig {} impl RethNodeCommandConfig for () {} +/// A helper type for [RethCliExt] extension that don't require any additional clap Arguments. +#[derive(Debug, Clone, Copy)] +pub struct NoArgsCliExt(PhantomData); + +impl RethCliExt for NoArgsCliExt { + type Node = NoArgs; +} + /// A helper struct that allows for wrapping a [RethNodeCommandConfig] value without providing /// additional CLI arguments. /// @@ -214,6 +222,12 @@ impl RethNodeCommandConfig for NoArgs { } } +impl From for NoArgs { + fn from(value: T) -> Self { + Self::with(value) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/bin/reth/src/cli/mod.rs b/bin/reth/src/cli/mod.rs index 1938e1ef2fd0..5fb1224397a4 100644 --- a/bin/reth/src/cli/mod.rs +++ b/bin/reth/src/cli/mod.rs @@ -113,6 +113,15 @@ impl Cli { reth_tracing::init(layers); Ok(guard.flatten()) } + + /// Configures the given node extension. + pub fn with_node_extension(mut self, conf: C) -> Self + where + C: Into, + { + self.command.set_node_extension(conf.into()); + self + } } /// Convenience function for parsing CLI options, set up logging and run the chosen command. @@ -156,6 +165,17 @@ pub enum Commands { Recover(recover::Command), } +impl Commands { + /// Sets the node extension if it is the [NodeCommand](node::NodeCommand). + /// + /// This is a noop if the command is not the [NodeCommand](node::NodeCommand). + pub fn set_node_extension(&mut self, ext: Ext::Node) { + if let Commands::Node(command) = self { + command.ext = ext + } + } +} + /// The log configuration. #[derive(Debug, Args)] #[command(next_help_heading = "Logging")] diff --git a/examples/cli-extension-event-hooks/Cargo.toml b/examples/cli-extension-event-hooks/Cargo.toml new file mode 100644 index 000000000000..9c9b30b65f52 --- /dev/null +++ b/examples/cli-extension-event-hooks/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "cli-extension-event-hooks" +version = "0.0.0" +publish = false +edition.workspace = true +license.workspace = true + +[dependencies] +reth.workspace = true +clap.workspace = true +eyre = "0.6" diff --git a/examples/cli-extension-event-hooks/src/main.rs b/examples/cli-extension-event-hooks/src/main.rs new file mode 100644 index 000000000000..24d7b5c84ec2 --- /dev/null +++ b/examples/cli-extension-event-hooks/src/main.rs @@ -0,0 +1,51 @@ +//! Example for how hook into the node via the CLI extension mechanism without registering +//! additional arguments +//! +//! Run with +//! +//! ```not_rust +//! cargo run -p cli-extension-event-hooks -- node +//! ``` +//! +//! This launch the regular reth node and also print: +//! +//! > "All components initialized" +//! once all components have been initialized and +//! +//! > "Node started" +//! once the node has been started. +use clap::Parser; +use reth::cli::{ + components::RethNodeComponents, + ext::{NoArgsCliExt, RethNodeCommandConfig}, + Cli, +}; + +fn main() { + Cli::>::parse() + .with_node_extension(MyRethConfig::default()) + .run() + .unwrap(); +} + +/// Our custom cli args extension that adds one flag to reth default CLI. +#[derive(Debug, Clone, Copy, Default)] +#[non_exhaustive] +struct MyRethConfig; + +impl RethNodeCommandConfig for MyRethConfig { + fn on_components_initialized( + &mut self, + _components: &Reth, + ) -> eyre::Result<()> { + println!("All components initialized"); + Ok(()) + } + fn on_node_started( + &mut self, + _components: &Reth, + ) -> eyre::Result<()> { + println!("Node started"); + Ok(()) + } +} From 851831f4b8ecbd0420088b79321a2646c4e69c3d Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Fri, 6 Oct 2023 20:11:00 +0200 Subject: [PATCH 14/23] fix: filter out unchanged account states in diff mode (#4931) --- .../src/tracing/builder/geth.rs | 4 ++ .../rpc-types/src/eth/trace/geth/pre_state.rs | 65 ++++++++++++++++++- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/crates/revm/revm-inspectors/src/tracing/builder/geth.rs b/crates/revm/revm-inspectors/src/tracing/builder/geth.rs index bb730fa42c7e..d1ee600a3652 100644 --- a/crates/revm/revm-inspectors/src/tracing/builder/geth.rs +++ b/crates/revm/revm-inspectors/src/tracing/builder/geth.rs @@ -223,6 +223,10 @@ impl GethTraceBuilder { } self.update_storage_from_trace(&mut state_diff.pre, false); self.update_storage_from_trace(&mut state_diff.post, true); + + // ensure we're only keeping changed entries + state_diff.retain_changed(); + Ok(PreStateFrame::Diff(state_diff)) } } diff --git a/crates/rpc/rpc-types/src/eth/trace/geth/pre_state.rs b/crates/rpc/rpc-types/src/eth/trace/geth/pre_state.rs index 5384896cbf12..d867ebd0b870 100644 --- a/crates/rpc/rpc-types/src/eth/trace/geth/pre_state.rs +++ b/crates/rpc/rpc-types/src/eth/trace/geth/pre_state.rs @@ -1,12 +1,20 @@ use reth_primitives::{serde_helper::num::from_int_or_hex_opt, Address, Bytes, B256, U256}; use serde::{Deserialize, Serialize}; -use std::collections::BTreeMap; +use std::collections::{btree_map, BTreeMap}; +/// A tracer that records [AccountState]s. +/// The prestate tracer has two modes: prestate and diff +/// /// #[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)] #[serde(untagged)] pub enum PreStateFrame { + /// The default mode returns the accounts necessary to execute a given transaction. + /// + /// It re-executes the given transaction and tracks every part of state that is touched. Default(PreStateMode), + /// Diff mode returns the differences between the transaction's pre and post-state (i.e. what + /// changed because the transaction happened). Diff(DiffMode), } @@ -44,6 +52,8 @@ impl PreStateFrame { pub struct PreStateMode(pub BTreeMap); /// Represents the account states before and after the transaction is executed. +/// +/// This corresponds to the [DiffMode] of the [PreStateConfig]. #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct DiffMode { @@ -53,8 +63,31 @@ pub struct DiffMode { pub pre: BTreeMap, } +// === impl DiffMode === + +impl DiffMode { + /// The sets of the [DiffMode] should only contain changed [AccountState]s. + /// + /// This will remove all unchanged [AccountState]s from the sets. + /// + /// In other words it removes entries that are equal (unchanged) in both the pre and post sets. + pub fn retain_changed(&mut self) { + self.pre.retain(|address, pre| { + if let btree_map::Entry::Occupied(entry) = self.post.entry(*address) { + if entry.get() == pre { + // remove unchanged account state from both sets + entry.remove(); + return false + } + } + + true + }); + } +} + /// Represents the state of an account -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Hash, Serialize, Deserialize)] pub struct AccountState { #[serde( default, @@ -194,4 +227,32 @@ mod tests { let pre_state: PreStateFrame = serde_json::from_str(s).unwrap(); assert!(pre_state.is_diff()); } + + #[test] + fn test_retain_changed_accounts() { + let s = r#"{ + "post": { + "0x35a9f94af726f07b5162df7e828cc9dc8439e7d0": { + "nonce": 1135 + } + }, + "pre": { + "0x35a9f94af726f07b5162df7e828cc9dc8439e7d0": { + "balance": "0x7a48429e177130a", + "nonce": 1134 + } + } +} +"#; + let diff: DiffMode = serde_json::from_str(s).unwrap(); + let mut diff_changed = diff.clone(); + diff_changed.retain_changed(); + // different entries + assert_eq!(diff_changed, diff); + + diff_changed.pre = diff_changed.post.clone(); + diff_changed.retain_changed(); + assert!(diff_changed.post.is_empty()); + assert!(diff_changed.pre.is_empty()); + } } From 51ecf3e96d49154d583b265cd0140de9b1abba02 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Fri, 6 Oct 2023 21:23:22 +0200 Subject: [PATCH 15/23] fix: improve account diff populate (#4934) --- .../src/tracing/builder/parity.rs | 56 ++++++++++++------- crates/rpc/rpc/src/trace.rs | 12 +--- 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/crates/revm/revm-inspectors/src/tracing/builder/parity.rs b/crates/revm/revm-inspectors/src/tracing/builder/parity.rs index 87a780191efc..75fee402950e 100644 --- a/crates/revm/revm-inspectors/src/tracing/builder/parity.rs +++ b/crates/revm/revm-inspectors/src/tracing/builder/parity.rs @@ -8,7 +8,7 @@ use reth_rpc_types::{trace::parity::*, TransactionInfo}; use revm::{ db::DatabaseRef, interpreter::opcode::{self, spec_opcode_gas}, - primitives::{AccountInfo, ExecutionResult, ResultAndState, SpecId, KECCAK_EMPTY}, + primitives::{Account, ExecutionResult, ResultAndState, SpecId, KECCAK_EMPTY}, }; use std::collections::{HashSet, VecDeque}; @@ -207,11 +207,7 @@ impl ParityTraceBuilder { // check the state diff case if let Some(ref mut state_diff) = trace_res.state_diff { - populate_account_balance_nonce_diffs( - state_diff, - &db, - state.into_iter().map(|(addr, acc)| (addr, acc.info)), - )?; + populate_account_balance_nonce_diffs(state_diff, &db, state.iter())?; } // check the vm trace case @@ -569,31 +565,49 @@ where /// /// It's expected that `DB` is a revm [Database](revm::db::Database) which at this point already /// contains all the accounts that are in the state map and never has to fetch them from disk. -pub fn populate_account_balance_nonce_diffs( +/// +/// This is intended to be called after inspecting a transaction with the returned state. +pub fn populate_account_balance_nonce_diffs<'a, DB, I>( state_diff: &mut StateDiff, db: DB, account_diffs: I, ) -> Result<(), DB::Error> where - I: IntoIterator, + I: IntoIterator, DB: DatabaseRef, { for (addr, changed_acc) in account_diffs.into_iter() { + let addr = *addr; let entry = state_diff.entry(addr).or_default(); - let db_acc = db.basic(addr)?.unwrap_or_default(); - entry.balance = if db_acc.balance == changed_acc.balance { - Delta::Unchanged - } else { - Delta::Changed(ChangedType { from: db_acc.balance, to: changed_acc.balance }) - }; - entry.nonce = if db_acc.nonce == changed_acc.nonce { - Delta::Unchanged + + // we check if this account was created during the transaction + if changed_acc.is_created() { + entry.balance = Delta::Added(changed_acc.info.balance); + entry.nonce = Delta::Added(U64::from(changed_acc.info.nonce)); + if changed_acc.info.code_hash == KECCAK_EMPTY { + // this is an additional check to ensure new accounts always get the empty code + // marked as added + entry.code = Delta::Added(Default::default()); + } } else { - Delta::Changed(ChangedType { - from: U64::from(db_acc.nonce), - to: U64::from(changed_acc.nonce), - }) - }; + // account already exists, we need to fetch the account from the db + let db_acc = db.basic(addr)?.unwrap_or_default(); + entry.balance = if db_acc.balance == changed_acc.info.balance { + Delta::Unchanged + } else { + Delta::Changed(ChangedType { from: db_acc.balance, to: changed_acc.info.balance }) + }; + + // this is relevant for the caller and contracts + entry.nonce = if db_acc.nonce == changed_acc.info.nonce { + Delta::Unchanged + } else { + Delta::Changed(ChangedType { + from: U64::from(db_acc.nonce), + to: U64::from(changed_acc.info.nonce), + }) + }; + } } Ok(()) diff --git a/crates/rpc/rpc/src/trace.rs b/crates/rpc/rpc/src/trace.rs index f38fc0e57e76..54814bbd943d 100644 --- a/crates/rpc/rpc/src/trace.rs +++ b/crates/rpc/rpc/src/trace.rs @@ -168,11 +168,7 @@ where // If statediffs were requested, populate them with the account balance and // nonce from pre-state if let Some(ref mut state_diff) = trace_res.state_diff { - populate_account_balance_nonce_diffs( - state_diff, - &db, - state.iter().map(|(addr, acc)| (*addr, acc.info.clone())), - )?; + populate_account_balance_nonce_diffs(state_diff, &db, state.iter())?; } results.push(trace_res); @@ -513,11 +509,7 @@ where // If statediffs were requested, populate them with the account balance and nonce // from pre-state if let Some(ref mut state_diff) = full_trace.state_diff { - populate_account_balance_nonce_diffs( - state_diff, - db, - state.iter().map(|(addr, acc)| (*addr, acc.info.clone())), - )?; + populate_account_balance_nonce_diffs(state_diff, db, state.iter())?; } let trace = TraceResultsWithTransactionHash { From 6fd66b3149ec24f44353055df9a7243fa3164730 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Sat, 7 Oct 2023 14:04:02 +0200 Subject: [PATCH 16/23] chore: update lock (#4942) --- Cargo.lock | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 07a93662ffe2..2478cdc88adf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1274,7 +1274,6 @@ dependencies = [ "clap", "eyre", "reth", - "tokio", ] [[package]] From 5de04430fd34c68452ef044dd747cd5d3a49acaf Mon Sep 17 00:00:00 2001 From: prames <134806363+0xprames@users.noreply.github.com> Date: Sat, 7 Oct 2023 17:29:32 +0530 Subject: [PATCH 17/23] fix(disc) Send find_node request only after verifiying our endpoint proof (#4909) --- crates/net/discv4/src/lib.rs | 132 ++++++++++++++++++++++++++++++----- 1 file changed, 113 insertions(+), 19 deletions(-) diff --git a/crates/net/discv4/src/lib.rs b/crates/net/discv4/src/lib.rs index 17bfe2d7e603..d889b900cf04 100644 --- a/crates/net/discv4/src/lib.rs +++ b/crates/net/discv4/src/lib.rs @@ -432,6 +432,11 @@ pub struct Discv4Service { queued_pings: VecDeque<(NodeRecord, PingReason)>, /// Currently active pings to specific nodes. pending_pings: HashMap, + /// Currently active endpoint proof verification lookups to specific nodes. + /// + /// Entries here means we've proven the peer's endpoint but haven't completed our end of the + /// endpoint proof + pending_lookup: HashMap, /// Currently active FindNode requests pending_find_nodes: HashMap, /// Currently active ENR requests @@ -546,6 +551,7 @@ impl Discv4Service { egress: egress_tx, queued_pings: Default::default(), pending_pings: Default::default(), + pending_lookup: Default::default(), pending_find_nodes: Default::default(), pending_enr_requests: Default::default(), commands_rx, @@ -988,10 +994,17 @@ impl Discv4Service { // the ping interval let mut is_new_insert = false; let mut needs_bond = false; + let mut is_proven = false; let old_enr = match self.kbuckets.entry(&key) { - kbucket::Entry::Present(mut entry, _) => entry.value_mut().update_with_enr(ping.enr_sq), - kbucket::Entry::Pending(mut entry, _) => entry.value().update_with_enr(ping.enr_sq), + kbucket::Entry::Present(mut entry, _) => { + is_proven = entry.value().has_endpoint_proof; + entry.value_mut().update_with_enr(ping.enr_sq) + } + kbucket::Entry::Pending(mut entry, _) => { + is_proven = entry.value().has_endpoint_proof; + entry.value().update_with_enr(ping.enr_sq) + } kbucket::Entry::Absent(entry) => { let mut node = NodeEntry::new(record); node.last_enr_seq = ping.enr_sq; @@ -1044,6 +1057,19 @@ impl Discv4Service { self.try_ping(record, PingReason::InitialInsert); } else if needs_bond { self.try_ping(record, PingReason::EstablishBond); + } else if is_proven { + // if node has been proven, this means we've recieved a pong and verified its endpoint + // proof. We've also sent a pong above to verify our endpoint proof, so we can now + // send our find_nodes request if PingReason::Lookup + if let Some((_, ctx)) = self.pending_lookup.remove(&record.id) { + if self.pending_find_nodes.contains_key(&record.id) { + // there's already another pending request, unmark it so the next round can + // try to send it + ctx.unmark_queried(record.id); + } else { + self.find_node(&record, ctx); + } + } } else { // Request ENR if included in the ping match (ping.enr_sq, old_enr) { @@ -1156,13 +1182,11 @@ impl Discv4Service { } PingReason::Lookup(node, ctx) => { self.update_on_pong(node, pong.enr_sq); - if self.pending_find_nodes.contains_key(&node.id) { - // there's already another pending request, unmark it so the next round can try - // to send it - ctx.unmark_queried(node.id); - } else { - self.find_node(&node, ctx); - } + // insert node and assoc. lookup_context into the pending_lookup table to complete + // our side of the endpoint proof verification. + // Start the lookup timer here - and evict accordingly. Note that this is a separate + // timer than the ping_request timer. + self.pending_lookup.insert(node.id, (Instant::now(), ctx)); } } } @@ -1279,7 +1303,7 @@ impl Discv4Service { }; // This is the recursive lookup step where we initiate new FindNode requests for new nodes - // that where discovered. + // that were discovered. for node in msg.nodes.into_iter().map(NodeRecord::into_ipv4_mapped) { // prevent banned peers from being added to the context if self.config.ban_list.is_banned(&node.id, &node.address) { @@ -1299,7 +1323,8 @@ impl Discv4Service { match self.kbuckets.entry(&key) { BucketEntry::Absent(entry) => { // the node's endpoint is not proven yet, so we need to ping it first, on - // success, it will initiate a `FindNode` request. + // success, we will add the node to the pending_lookup table, and wait to send + // back a Pong before initiating a FindNode request. // In order to prevent that this node is selected again on subsequent responses, // while the ping is still active, we always mark it as queried. ctx.mark_queried(closest.id); @@ -1365,6 +1390,21 @@ impl Discv4Service { self.remove_node(node_id); } + let mut failed_lookups = Vec::new(); + self.pending_lookup.retain(|node_id, (lookup_sent_at, _)| { + if now.duration_since(*lookup_sent_at) > self.config.ping_expiration { + failed_lookups.push(*node_id); + return false + } + true + }); + debug!(target: "discv4", num=%failed_lookups.len(), "evicting nodes due to failed lookup"); + + // remove nodes that failed the e2e lookup process, so we can restart it + for node_id in failed_lookups { + self.remove_node(node_id); + } + self.evict_failed_neighbours(now); } @@ -1802,7 +1842,7 @@ impl LookupTargetRotator { /// Tracks lookups across multiple `FindNode` requests. /// /// If this type is dropped by all -#[derive(Clone)] +#[derive(Clone, Debug)] struct LookupContext { inner: Rc, } @@ -1905,7 +1945,7 @@ impl LookupContext { // guaranteed that there's only 1 owner ([`Discv4Service`]) of all possible [`Rc`] clones of // [`LookupContext`]. unsafe impl Send for LookupContext {} - +#[derive(Debug)] struct LookupContextInner { /// The target to lookup. target: discv5::Key, @@ -2272,7 +2312,7 @@ mod tests { reth_tracing::init_test_tracing(); let config = Discv4Config::builder().build(); - let (_discv4, mut service) = create_discv4_with_config(config).await; + let (_discv4, mut service) = create_discv4_with_config(config.clone()).await; let id = PeerId::random(); let key = kad_key(id); @@ -2296,9 +2336,65 @@ mod tests { Poll::Ready(()) }) - .await + .await; } + #[tokio::test] + async fn test_on_neighbours_recursive_lookup() { + reth_tracing::init_test_tracing(); + + let config = Discv4Config::builder().build(); + let (_discv4, mut service) = create_discv4_with_config(config.clone()).await; + let (_discv4, mut service2) = create_discv4_with_config(config).await; + + let id = PeerId::random(); + let key = kad_key(id); + let record = NodeRecord::new("0.0.0.0:0".parse().unwrap(), id); + + let _ = service.kbuckets.insert_or_update( + &key, + NodeEntry::new_proven(record), + NodeStatus { + direction: ConnectionDirection::Incoming, + state: ConnectionState::Connected, + }, + ); + // Needed in this test to populate self.pending_find_nodes for as a prereq to a valid + // on_neighbours request + service.lookup_self(); + assert_eq!(service.pending_find_nodes.len(), 1); + + poll_fn(|cx| { + let _ = service.poll(cx); + assert_eq!(service.pending_find_nodes.len(), 1); + + Poll::Ready(()) + }) + .await; + + let expiry = SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or_default().as_secs() + + 10000000000000; + let msg = Neighbours { nodes: vec![service2.local_node_record], expire: expiry }; + service.on_neighbours(msg, record.tcp_addr(), id); + // wait for the processed ping + let event = poll_fn(|cx| service2.poll(cx)).await; + assert_eq!(event, Discv4Event::Ping); + // assert that no find_node req has been added here on top of the initial one, since both + // sides of the endpoint proof is not completed here + assert_eq!(service.pending_find_nodes.len(), 1); + // we now wait for PONG + let event = poll_fn(|cx| service.poll(cx)).await; + assert_eq!(event, Discv4Event::Pong); + // Ideally we want to assert against service.pending_lookup.len() here - but because the + // service2 sends Pong and Ping consecutivley on_ping(), the pending_lookup table gets + // drained almost immediately - and no way to grab the handle to its intermediary state here + // :( + let event = poll_fn(|cx| service.poll(cx)).await; + assert_eq!(event, Discv4Event::Ping); + // assert that we've added the find_node req here after both sides of the endpoint proof is + // done + assert_eq!(service.pending_find_nodes.len(), 2); + } #[tokio::test] async fn test_no_local_in_closest() { reth_tracing::init_test_tracing(); @@ -2463,14 +2559,12 @@ mod tests { // we now wait for PONG let event = poll_fn(|cx| service_2.poll(cx)).await; - // Since the endpoint was already proven from 1 POV it can already send a FindNode so the - // next event is either the PONG or Find Node match event { - Discv4Event::FindNode | Discv4Event::EnrRequest => { + Discv4Event::EnrRequest => { // since we support enr in the ping it may also request the enr let event = poll_fn(|cx| service_2.poll(cx)).await; match event { - Discv4Event::FindNode | Discv4Event::EnrRequest => { + Discv4Event::EnrRequest => { let event = poll_fn(|cx| service_2.poll(cx)).await; assert_eq!(event, Discv4Event::Pong); } From 537ad17b778ce4c11e33146face76300a7b3935b Mon Sep 17 00:00:00 2001 From: prames <134806363+0xprames@users.noreply.github.com> Date: Sun, 8 Oct 2023 01:15:19 +0530 Subject: [PATCH 18/23] (chore) fix vergen git sha in docker (#4946) Co-authored-by: Matthias Seitz --- .dockerignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.dockerignore b/.dockerignore index 95c7af355cfb..7dfd185e5eae 100644 --- a/.dockerignore +++ b/.dockerignore @@ -11,6 +11,7 @@ !Cross.toml !deny.toml !Makefile +!/.git # include for vergen constants # include dist directory, where the reth binary is located after compilation !/dist From 98bc2b5c841bb835523f32c0c3425bbbd3859093 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 8 Oct 2023 13:53:35 +0200 Subject: [PATCH 19/23] chore(deps): weekly `cargo update` (#4949) Co-authored-by: github-merge-queue --- Cargo.lock | 246 ++++++++++++++++++++++++++++------------------------- 1 file changed, 132 insertions(+), 114 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2478cdc88adf..0da532e46c4e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -221,7 +221,7 @@ checksum = "c0391754c09fab4eae3404d19d0d297aa1c670c1775ab51d8a5312afeca23157" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -235,7 +235,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", "syn-solidity", "tiny-keccak", ] @@ -528,7 +528,7 @@ checksum = "bc00ceb34980c03614e35a3a4e218276a0a824e911d07651cd0d858a51e8c0f0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -713,7 +713,7 @@ dependencies = [ "regex", "rustc-hash", "shlex", - "syn 2.0.37", + "syn 2.0.38", "which", ] @@ -734,7 +734,7 @@ dependencies = [ "regex", "rustc-hash", "shlex", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -938,7 +938,7 @@ checksum = "005fa0c5bd20805466dda55eb34cd709bb31a2592bb26927b47714eeed6914d8" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", "synstructure", ] @@ -1044,9 +1044,9 @@ checksum = "374d28ec25809ee0e23827c2ab573d729e293f281dfe393500e7ad618baa61c6" [[package]] name = "byteorder" -version = "1.4.3" +version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" +checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" [[package]] name = "bytes" @@ -1083,18 +1083,18 @@ dependencies = [ [[package]] name = "cargo-platform" -version = "0.1.3" +version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2cfa25e60aea747ec7e1124f238816749faa93759c6ff5b31f1ccdda137f4479" +checksum = "12024c4645c97566567129c204f65d5815a8c9aecf30fcbe682b2fe034996d36" dependencies = [ "serde", ] [[package]] name = "cargo_metadata" -version = "0.15.4" +version = "0.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eee4243f1f26fc7a42710e7439c149e2b10b05472f88090acce52632f231a73a" +checksum = "e7daec1a2a2129eeba1644b220b4647ec537b0b5d4bfd6876fcc5a540056b592" dependencies = [ "camino", "cargo-platform", @@ -1106,9 +1106,9 @@ dependencies = [ [[package]] name = "cargo_metadata" -version = "0.17.0" +version = "0.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e7daec1a2a2129eeba1644b220b4647ec537b0b5d4bfd6876fcc5a540056b592" +checksum = "fb9ac64500cc83ce4b9f8dafa78186aa008c8dea77a09b94cd307fd0cd5022a8" dependencies = [ "camino", "cargo-platform", @@ -1258,7 +1258,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -1292,7 +1292,7 @@ dependencies = [ "proc-macro2", "quote", "serde", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -1391,9 +1391,9 @@ dependencies = [ [[package]] name = "const-hex" -version = "1.9.0" +version = "1.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa72a10d0e914cad6bcad4e7409e68d230c1c2db67896e19a37f758b1fcbdab5" +checksum = "c37be52ef5e3b394db27a2341010685ad5103c72ac15ce2e9420a7e8f93f342c" dependencies = [ "cfg-if", "cpufeatures", @@ -1714,7 +1714,7 @@ checksum = "83fdaf97f4804dcebfa5862639bc9ce4121e82140bec2a987ac5140294865b5b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -1762,7 +1762,7 @@ dependencies = [ "proc-macro2", "quote", "strsim 0.10.0", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -1784,7 +1784,7 @@ checksum = "836a9bbc7ad63342d6d6e7b815ccab164bc77a2d95d84bc3117a8c0d5c98e2d5" dependencies = [ "darling_core 0.20.3", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -1869,7 +1869,7 @@ checksum = "53e0efad4403bfc52dc201159c4b842a246a14b98c64b55dfd0f2d89729dfeb8" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -2043,7 +2043,7 @@ checksum = "487585f4d0c6655fe74905e2504d8ad6908e4db67f744eb140876906c2f3175d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -2158,9 +2158,9 @@ checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" [[package]] name = "elliptic-curve" -version = "0.13.5" +version = "0.13.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "968405c8fdc9b3bf4df0a6638858cc0b52462836ab6b1c87377785dd09cf1c0b" +checksum = "d97ca172ae9dc9f9b779a6e3a65d308f2af74e5b8c921299075bdb4a0370e914" dependencies = [ "base16ct", "crypto-bigint", @@ -2243,20 +2243,20 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] name = "enum-ordinalize" -version = "3.1.13" +version = "3.1.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e4f76552f53cefc9a7f64987c3701b99d982f7690606fd67de1d09712fbf52f1" +checksum = "1bf1fa3f06bbff1ea5b1a9c7b14aa992a39657db60a2759457328d7e058f49ee" dependencies = [ "num-bigint", "num-traits", "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -2267,7 +2267,7 @@ checksum = "c2ad8cef1d801a4686bfd8919f0b30eac4c8e48968c437a6405ded4fb5272d2b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -2289,9 +2289,9 @@ dependencies = [ [[package]] name = "errno" -version = "0.3.3" +version = "0.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "136526188508e25c6fef639d7927dfb3e0e3084488bf202267829cf7fc23dbdd" +checksum = "add4f07d43996f76ef320709726a556a9d4f965d9410d8d0271132d2f8293480" dependencies = [ "errno-dragonfly", "libc", @@ -2414,7 +2414,7 @@ dependencies = [ "regex", "serde", "serde_json", - "syn 2.0.37", + "syn 2.0.38", "toml 0.7.8", "walkdir", ] @@ -2432,7 +2432,7 @@ dependencies = [ "proc-macro2", "quote", "serde_json", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -2458,7 +2458,7 @@ dependencies = [ "serde", "serde_json", "strum 0.25.0", - "syn 2.0.37", + "syn 2.0.38", "tempfile", "thiserror", "tiny-keccak", @@ -2795,7 +2795,7 @@ checksum = "89ca545a94061b6365f2c7355b4b32bd20df3ff95f02da9329b34ccc3bd6ee72" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -3623,7 +3623,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cb0889898416213fab133e1d33a0e5858a48177452750691bde3666d0fdbaf8b" dependencies = [ "hermit-abi", - "rustix 0.38.15", + "rustix 0.38.17", "windows-sys 0.48.0", ] @@ -3915,9 +3915,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "libc" -version = "0.2.148" +version = "0.2.149" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9cdc71e17332e86d2e1d38c1f99edcb6288ee11b815fb1a4b049eaa2114d369b" +checksum = "a08173bc88b7955d1b3145aa561539096c421ac8debde8cbc3612ec635fee29b" [[package]] name = "libloading" @@ -3931,9 +3931,9 @@ dependencies = [ [[package]] name = "libm" -version = "0.2.7" +version = "0.2.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f7012b1bbb0719e1097c47611d3898568c546d597c2e74d66f6087edd5233ff4" +checksum = "4ec2a862134d2a7d32d7983ddcdd1c4923530833c9f2ea1a44fc5fa473989058" [[package]] name = "libproc" @@ -4085,9 +4085,9 @@ checksum = "2532096657941c2fea9c289d370a250971c689d4f143798ff67113ec042024a5" [[package]] name = "memchr" -version = "2.6.3" +version = "2.6.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f232d6ef707e1956a43342693d2a31e72989554d58299d7a88738cc95b0d35c" +checksum = "f665ee40bc4a3c5590afb1e9677db74a508659dfd71e126420da8274909a0167" [[package]] name = "memmap2" @@ -4153,7 +4153,7 @@ checksum = "ddece26afd34c31585c74a4db0630c376df271c285d682d1e55012197830b6df" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -4413,9 +4413,9 @@ dependencies = [ [[package]] name = "num-traits" -version = "0.2.16" +version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f30b0abd723be7e2ffca1272140fac1a2f084c77ec3e123c192b66af1ee9e6c2" +checksum = "39e3200413f237f41ab11ad6d161bc7239c84dcb631773ccd7de3dfe4b5c267c" dependencies = [ "autocfg", "libm", @@ -4458,7 +4458,7 @@ dependencies = [ "proc-macro-crate", "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -4470,7 +4470,7 @@ dependencies = [ "proc-macro-crate", "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -4774,7 +4774,7 @@ dependencies = [ "phf_shared", "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -4803,7 +4803,7 @@ checksum = "4359fd9c9171ec6e8c62926d6faaf553a8dc3f64e1507e76da7911b4f6a04405" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -4988,7 +4988,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ae005bd773ab59b4725093fd7df83fd7892f7d8eafb48dbd7de6e024e4215f9d" dependencies = [ "proc-macro2", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -5041,9 +5041,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.67" +version = "1.0.68" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3d433d9f1a3e8c1263d9456598b16fec66f4acc9a74dacffd35c7bb09b3a1328" +checksum = "5b1106fec09662ec6dd98ccac0f81cef56984d0b49f75c92d8cbad76e20c005c" dependencies = [ "unicode-ident", ] @@ -5063,9 +5063,9 @@ dependencies = [ [[package]] name = "proptest" -version = "1.3.0" +version = "1.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "59d6ddf4877c954130eb46772d1efba4ec0f5b3ee3189925d6a78ee61c41227f" +checksum = "7c003ac8c77cb07bb74f5f198bce836a689bcd5a42574612bf14d17bfd08c20e" dependencies = [ "bit-set", "bit-vec", @@ -5363,9 +5363,9 @@ dependencies = [ [[package]] name = "reqwest" -version = "0.11.20" +version = "0.11.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3e9ad3fe7488d7e34558a2033d45a0c90b72d97b4f80705666fea71472e2e6a1" +checksum = "046cd98826c46c2ac8ddecae268eb5c2e58628688a5fc7a2643704a73faba95b" dependencies = [ "base64 0.21.4", "bytes", @@ -5386,6 +5386,7 @@ dependencies = [ "serde", "serde_json", "serde_urlencoded", + "system-configuration", "tokio", "tower-service", "url", @@ -5476,7 +5477,7 @@ dependencies = [ "tempfile", "thiserror", "tokio", - "toml 0.8.1", + "toml 0.8.2", "tracing", "tui", "vergen", @@ -5601,7 +5602,7 @@ dependencies = [ "serde", "serde_json", "tempfile", - "toml 0.8.1", + "toml 0.8.2", ] [[package]] @@ -5890,7 +5891,7 @@ dependencies = [ "quote", "regex", "serial_test", - "syn 2.0.37", + "syn 2.0.38", "trybuild", ] @@ -6068,7 +6069,7 @@ dependencies = [ "thiserror", "tokio", "tokio-stream", - "toml 0.8.1", + "toml 0.8.2", "tracing", "triehash", "url", @@ -6675,7 +6676,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c37f1bd5ef1b5422177b7646cba67430579cfe2ace80f284fee876bca52ad941" dependencies = [ "bitflags 1.3.2", - "errno 0.3.3", + "errno 0.3.4", "io-lifetimes", "libc", "linux-raw-sys 0.1.4", @@ -6684,12 +6685,12 @@ dependencies = [ [[package]] name = "rustix" -version = "0.38.15" +version = "0.38.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d2f9da0cbd88f9f09e7814e388301c8414c51c62aa6ce1e4b5c551d49d96e531" +checksum = "f25469e9ae0f3d0047ca8b93fc56843f38e6774f0914a107ff8b41be8be8e0b7" dependencies = [ "bitflags 2.4.0", - "errno 0.3.3", + "errno 0.3.4", "libc", "linux-raw-sys 0.4.8", "windows-sys 0.48.0", @@ -6980,7 +6981,7 @@ checksum = "4eca7ac642d82aa35b60049a6eccb4be6be75e599bd2e9adb5f875a737654af2" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -7041,7 +7042,7 @@ dependencies = [ "darling 0.20.3", "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -7066,7 +7067,7 @@ checksum = "91d129178576168c589c9ec973feedf7d3126c01ac2bf08795109aa35b69fb8f" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -7127,9 +7128,9 @@ dependencies = [ [[package]] name = "sharded-slab" -version = "0.1.6" +version = "0.1.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c1b21f559e07218024e7e9f90f96f601825397de0e25420135f7f952453fed0b" +checksum = "f40ca3c46823713e0d4209592e8d6e826aa57e928f09752619fc696c499637f6" dependencies = [ "lazy_static", ] @@ -7191,9 +7192,9 @@ dependencies = [ [[package]] name = "similar" -version = "2.2.1" +version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "420acb44afdae038210c99e69aae24109f32f15500aa708e81d46c9f29d55fcf" +checksum = "2aeaf503862c419d66959f5d7ca015337d864e9c49485d771b732e2a20453597" dependencies = [ "bstr", "unicode-segmentation", @@ -7398,7 +7399,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -7485,9 +7486,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.37" +version = "2.0.38" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7303ef2c05cd654186cb250d29049a24840ca25d2747c25c0381c8d9e2f582e8" +checksum = "e96b79aaa137db8f61e26363a0c9b47d8b4ec75da28b7d1d614c2303e232408b" dependencies = [ "proc-macro2", "quote", @@ -7503,7 +7504,7 @@ dependencies = [ "paste", "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -7514,10 +7515,31 @@ checksum = "285ba80e733fac80aa4270fbcdf83772a79b80aa35c97075320abfee4a915b06" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", "unicode-xid", ] +[[package]] +name = "system-configuration" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba3a3adc5c275d719af8cb4272ea1c4a6d668a777f37e115f6d11ddbc1c8e0e7" +dependencies = [ + "bitflags 1.3.2", + "core-foundation", + "system-configuration-sys", +] + +[[package]] +name = "system-configuration-sys" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a75fb188eb626b924683e3b95e3a48e63551fcfb51949de2f06a9d91dbee93c9" +dependencies = [ + "core-foundation-sys", + "libc", +] + [[package]] name = "tap" version = "1.0.1" @@ -7533,7 +7555,7 @@ dependencies = [ "cfg-if", "fastrand 2.0.1", "redox_syscall 0.3.5", - "rustix 0.38.15", + "rustix 0.38.17", "windows-sys 0.48.0", ] @@ -7554,9 +7576,9 @@ checksum = "3369f5ac52d5eb6ab48c6b4ffdc8efbcad6b89c765749064ba298f2c68a16a76" [[package]] name = "test-fuzz" -version = "4.0.1" +version = "4.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "954c28be70cafa409ddf615dd3c14a62478d38b44a94ade29469e82258cd5229" +checksum = "e47fe9eb29998b02f6d93627531603da1bc5d7b758e87567af1fafa8c960d876" dependencies = [ "serde", "test-fuzz-internal", @@ -7566,42 +7588,38 @@ dependencies = [ [[package]] name = "test-fuzz-internal" -version = "4.0.1" +version = "4.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0f0528a7ad0bc85f826aa831434a37833aea622a5ae155f5b5dd431b25244213" +checksum = "52d59630b15f1a2f396030f3003d43c30ae9e99998dab992194cb26a85431851" dependencies = [ - "cargo_metadata 0.15.4", - "proc-macro2", - "quote", + "bincode", + "cargo_metadata 0.18.0", "serde", - "strum_macros 0.25.2", ] [[package]] name = "test-fuzz-macro" -version = "4.0.1" +version = "4.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "646c4ee44c47ae7888e88fb8441c857bc21136fa2c7d6a0a766b0caf609b35e8" +checksum = "372a64c51728f98776a624e87b6ed0a8c3e7f2bd18513f56b14281225db513d8" dependencies = [ "darling 0.20.3", "if_chain", - "itertools 0.10.5", - "lazy_static", + "itertools 0.11.0", + "once_cell", "proc-macro2", "quote", "subprocess", - "syn 2.0.37", - "test-fuzz-internal", + "syn 2.0.38", "toolchain_find", ] [[package]] name = "test-fuzz-runtime" -version = "4.0.1" +version = "4.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "491cf4aba9f04a5fa35cd2f69f73e4f390779c864d20a96c357c3be16eb7d501" +checksum = "a16256c08ddf5ec8cf9bcf46512b460a3696f5bdbc9c76a834a5eef5fbada951" dependencies = [ - "bincode", "hex", "num-traits", "serde", @@ -7632,7 +7650,7 @@ checksum = "10712f02019e9288794769fba95cd6847df9874d49d871d062172f9dd41bc4cc" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -7695,9 +7713,9 @@ dependencies = [ [[package]] name = "tinystr" -version = "0.7.1" +version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ac3f5b6856e931e15e07b478e98c8045239829a65f9156d4fa7e7788197a5ef" +checksum = "8faa444297615a4e020acb64146b0603c9c395c03a97c17fd9028816d3b4d63e" dependencies = [ "displaydoc", "serde", @@ -7756,7 +7774,7 @@ checksum = "630bdcf245f78637c13ec01ffae6187cca34625e8c63150d424b59e55af2675e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -7832,14 +7850,14 @@ dependencies = [ [[package]] name = "toml" -version = "0.8.1" +version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1bc1433177506450fe920e46a4f9812d0c211f5dd556da10e731a0a3dfa151f0" +checksum = "185d8ab0dfbb35cf1399a6344d8484209c088f75f8f68230da55d48d95d43e3d" dependencies = [ "serde", "serde_spanned", "toml_datetime", - "toml_edit 0.20.1", + "toml_edit 0.20.2", ] [[package]] @@ -7866,9 +7884,9 @@ dependencies = [ [[package]] name = "toml_edit" -version = "0.20.1" +version = "0.20.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ca676d9ba1a322c1b64eb8045a5ec5c0cfb0c9d08e15e9ff622589ad5221c8fe" +checksum = "396e4d48bbb2b7554c944bde63101b5ae446cff6ec4a24227428f15eb72ef338" dependencies = [ "indexmap 2.0.2", "serde", @@ -7985,7 +8003,7 @@ checksum = "5f4f31f56159e98206da9efd823404b79b6ef3143b4a7ab76e67b1751b25a4ab" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -8436,7 +8454,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", "wasm-bindgen-shared", ] @@ -8470,7 +8488,7 @@ checksum = "54681b18a46765f095758388f2d0cf16eb8d4169b639ab575a8f5693af210c7b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -8506,7 +8524,7 @@ dependencies = [ "either", "home", "once_cell", - "rustix 0.38.15", + "rustix 0.38.17", ] [[package]] @@ -8714,9 +8732,9 @@ checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" [[package]] name = "winnow" -version = "0.5.15" +version = "0.5.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c2e3184b9c4e92ad5167ca73039d0c42476302ab603e2fec4487511f38ccefc" +checksum = "037711d82167854aff2018dfd193aa0fef5370f456732f0d5a0c59b0f1b4b907" dependencies = [ "memchr", ] @@ -8821,7 +8839,7 @@ checksum = "d5e19fb6ed40002bab5403ffa37e53e0e56f914a4450c8765f533018db1db35f" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", "synstructure", ] @@ -8842,7 +8860,7 @@ checksum = "e6a647510471d372f2e6c2e6b7219e44d8c574d24fdc11c610a61455782f18c3" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", "synstructure", ] @@ -8863,7 +8881,7 @@ checksum = "ce36e65b0d2999d2aafac989fb249189a141aee1f53c612c1f37d72631959f69" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] @@ -8886,7 +8904,7 @@ checksum = "7a4a1638a1934450809c2266a70362bfc96cd90550c073f5b8a55014d1010157" dependencies = [ "proc-macro2", "quote", - "syn 2.0.37", + "syn 2.0.38", ] [[package]] From 18c4891238d24583962e4d7a720e57694c1b8778 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Sun, 8 Oct 2023 19:37:16 +0200 Subject: [PATCH 20/23] chore: add some geth tracer ergonomics (#4952) --- .../rpc/rpc-types/src/eth/trace/geth/call.rs | 14 ++++++++ .../rpc/rpc-types/src/eth/trace/geth/mod.rs | 36 ++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/crates/rpc/rpc-types/src/eth/trace/geth/call.rs b/crates/rpc/rpc-types/src/eth/trace/geth/call.rs index 636bc19c58b0..00e605043bd1 100644 --- a/crates/rpc/rpc-types/src/eth/trace/geth/call.rs +++ b/crates/rpc/rpc-types/src/eth/trace/geth/call.rs @@ -51,6 +51,20 @@ pub struct CallConfig { pub with_log: Option, } +impl CallConfig { + /// Sets the only top call flag + pub fn only_top_call(mut self) -> Self { + self.only_top_call = Some(true); + self + } + + /// Sets the with log flag + pub fn with_log(mut self) -> Self { + self.with_log = Some(true); + self + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/rpc/rpc-types/src/eth/trace/geth/mod.rs b/crates/rpc/rpc-types/src/eth/trace/geth/mod.rs index 752a0c5be6ad..ec1139f2eeb3 100644 --- a/crates/rpc/rpc-types/src/eth/trace/geth/mod.rs +++ b/crates/rpc/rpc-types/src/eth/trace/geth/mod.rs @@ -4,7 +4,7 @@ use crate::{state::StateOverride, BlockOverrides}; use reth_primitives::{Bytes, B256, U256}; use serde::{de::DeserializeOwned, ser::SerializeMap, Deserialize, Serialize, Serializer}; -use std::collections::BTreeMap; +use std::{collections::BTreeMap, time::Duration}; // re-exports pub use self::{ @@ -186,6 +186,12 @@ pub enum GethDebugTracerType { JsTracer(String), } +impl From for GethDebugTracerType { + fn from(value: GethDebugBuiltInTracerType) -> Self { + GethDebugTracerType::BuiltInTracer(value) + } +} + /// Configuration of the tracer /// /// This is a simple wrapper around serde_json::Value. @@ -264,6 +270,34 @@ pub struct GethDebugTracingOptions { pub timeout: Option, } +impl GethDebugTracingOptions { + /// Sets the tracer to use + pub fn with_tracer(mut self, tracer: GethDebugTracerType) -> Self { + self.tracer = Some(tracer); + self + } + + /// Sets the timeout to use for tracing + pub fn with_timeout(mut self, duration: Duration) -> Self { + self.timeout = Some(format!("{}ms", duration.as_millis())); + self + } + + /// Configures a [CallConfig] + pub fn call_config(mut self, config: CallConfig) -> Self { + self.tracer_config = + GethDebugTracerConfig(serde_json::to_value(config).expect("is serializable")); + self + } + + /// Configures a [PreStateConfig] + pub fn prestate_config(mut self, config: PreStateConfig) -> Self { + self.tracer_config = + GethDebugTracerConfig(serde_json::to_value(config).expect("is serializable")); + self + } +} + /// Default tracing options for the struct looger. /// /// These are all known general purpose tracer options that may or not be supported by a given From 9561f2860a020bc0f271786bd1bf924f7cf1bbe9 Mon Sep 17 00:00:00 2001 From: Yash Atreya <44857776+yash-atreya@users.noreply.github.com> Date: Mon, 9 Oct 2023 08:01:23 -0400 Subject: [PATCH 21/23] fix: improve eth_createAccessList (#4954) Co-authored-by: Matthias Seitz --- crates/rpc/rpc/src/eth/api/call.rs | 29 +++++++++++++++++++++++----- crates/rpc/rpc/src/eth/api/server.rs | 10 ++++------ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/crates/rpc/rpc/src/eth/api/call.rs b/crates/rpc/rpc/src/eth/api/call.rs index c5c60ac2da1a..4a7445bcfba5 100644 --- a/crates/rpc/rpc/src/eth/api/call.rs +++ b/crates/rpc/rpc/src/eth/api/call.rs @@ -12,7 +12,7 @@ use crate::{ EthApi, }; use reth_network_api::NetworkInfo; -use reth_primitives::{AccessList, BlockId, BlockNumberOrTag, Bytes, U256}; +use reth_primitives::{AccessListWithGasUsed, BlockId, BlockNumberOrTag, Bytes, U256}; use reth_provider::{ BlockReaderIdExt, ChainSpecProvider, EvmEnvProvider, StateProvider, StateProviderFactory, }; @@ -168,7 +168,7 @@ where /// Estimates the gas usage of the `request` with the state. /// /// This will execute the [CallRequest] and find the best gas limit via binary search - fn estimate_gas_with( + pub fn estimate_gas_with( &self, mut cfg: CfgEnv, block: BlockEnv, @@ -337,11 +337,23 @@ where Ok(U256::from(highest_gas_limit)) } + /// Creates the AccessList for the `request` at the [BlockId] or latest. pub(crate) async fn create_access_list_at( + &self, + request: CallRequest, + block_number: Option, + ) -> EthResult { + self.on_blocking_task(|this| async move { + this.create_access_list_with(request, block_number).await + }) + .await + } + + async fn create_access_list_with( &self, mut request: CallRequest, at: Option, - ) -> EthResult { + ) -> EthResult { let block_id = at.unwrap_or(BlockId::Number(BlockNumberOrTag::Latest)); let (cfg, block, at) = self.evm_env_at(block_id).await?; let state = self.state_at(at)?; @@ -377,7 +389,7 @@ where let precompiles = get_precompiles(env.cfg.spec_id); let mut inspector = AccessListInspector::new(initial, from, to, precompiles); - let (result, _env) = inspect(&mut db, env, &mut inspector)?; + let (result, env) = inspect(&mut db, env, &mut inspector)?; match result.result { ExecutionResult::Halt { reason, .. } => Err(match reason { @@ -389,7 +401,14 @@ where } ExecutionResult::Success { .. } => Ok(()), }?; - Ok(inspector.into_access_list()) + + let access_list = inspector.into_access_list(); + + // calculate the gas used using the access list + request.access_list = Some(access_list.clone()); + let gas_used = self.estimate_gas_with(env.cfg, env.block, request, db.db.state())?; + + Ok(AccessListWithGasUsed { access_list, gas_used }) } } diff --git a/crates/rpc/rpc/src/eth/api/server.rs b/crates/rpc/rpc/src/eth/api/server.rs index 4eb45828a794..d4d867619e29 100644 --- a/crates/rpc/rpc/src/eth/api/server.rs +++ b/crates/rpc/rpc/src/eth/api/server.rs @@ -254,15 +254,13 @@ where /// Handler for: `eth_createAccessList` async fn create_access_list( &self, - mut request: CallRequest, + request: CallRequest, block_number: Option, ) -> Result { trace!(target: "rpc::eth", ?request, ?block_number, "Serving eth_createAccessList"); - let block_id = block_number.unwrap_or(BlockId::Number(BlockNumberOrTag::Latest)); - let access_list = self.create_access_list_at(request.clone(), block_number).await?; - request.access_list = Some(access_list.clone()); - let gas_used = self.estimate_gas_at(request, block_id).await?; - Ok(AccessListWithGasUsed { access_list, gas_used }) + let access_list_with_gas_used = self.create_access_list_at(request, block_number).await?; + + Ok(access_list_with_gas_used) } /// Handler for: `eth_estimateGas` From 670d459147c5eba1f33b8511c9f335eccc703917 Mon Sep 17 00:00:00 2001 From: "Supernovahs.eth" <91280922+supernovahs@users.noreply.github.com> Date: Mon, 9 Oct 2023 18:10:33 +0530 Subject: [PATCH 22/23] Better topic0 (#4950) Co-authored-by: Matthias Seitz --- crates/rpc/rpc-types/src/eth/filter.rs | 14 +++++++++++--- examples/db-access.rs | 6 +++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/crates/rpc/rpc-types/src/eth/filter.rs b/crates/rpc/rpc-types/src/eth/filter.rs index 370395f73964..710f53caede7 100644 --- a/crates/rpc/rpc-types/src/eth/filter.rs +++ b/crates/rpc/rpc-types/src/eth/filter.rs @@ -393,18 +393,26 @@ impl Filter { #[must_use] pub fn event(self, event_name: &str) -> Self { let hash = keccak256(event_name.as_bytes()); - self.topic0(hash) + self.event_signature(hash) } - /// Hashes all event signatures and sets them as array to topic0 + /// Hashes all event signatures and sets them as array to event_signature(topic0) #[must_use] pub fn events(self, events: impl IntoIterator>) -> Self { let events = events.into_iter().map(|e| keccak256(e.as_ref())).collect::>(); - self.topic0(events) + self.event_signature(events) + } + + /// Sets event_signature(topic0) (the event name for non-anonymous events) + #[must_use] + pub fn event_signature>(mut self, topic: T) -> Self { + self.topics[0] = topic.into(); + self } /// Sets topic0 (the event name for non-anonymous events) #[must_use] + #[deprecated(note = "use `event_signature` instead")] pub fn topic0>(mut self, topic: T) -> Self { self.topics[0] = topic.into(); self diff --git a/examples/db-access.rs b/examples/db-access.rs index 289f895becee..faed7fc1d4da 100644 --- a/examples/db-access.rs +++ b/examples/db-access.rs @@ -182,9 +182,9 @@ fn receipts_provider_example Date: Mon, 9 Oct 2023 15:25:48 +0100 Subject: [PATCH 23/23] feat(prune): headers segment triggered by snapshots (#4936) --- crates/primitives/src/prune/segment.rs | 6 +- crates/prune/src/pruner.rs | 34 +++- crates/prune/src/segments/headers.rs | 192 ++++++++++++++++++ crates/prune/src/segments/mod.rs | 44 +++- crates/prune/src/segments/receipts.rs | 30 ++- crates/stages/src/test_utils/test_db.rs | 2 +- .../src/providers/database/provider.rs | 39 ++-- 7 files changed, 307 insertions(+), 40 deletions(-) create mode 100644 crates/prune/src/segments/headers.rs diff --git a/crates/primitives/src/prune/segment.rs b/crates/primitives/src/prune/segment.rs index 834d2efce2e3..f6f4d02fddc8 100644 --- a/crates/primitives/src/prune/segment.rs +++ b/crates/primitives/src/prune/segment.rs @@ -10,14 +10,16 @@ pub enum PruneSegment { SenderRecovery, /// Prune segment responsible for the `TxHashNumber` table. TransactionLookup, - /// Prune segment responsible for all `Receipts`. + /// Prune segment responsible for all rows in `Receipts` table. Receipts, - /// Prune segment responsible for some `Receipts` filtered by logs. + /// Prune segment responsible for some rows in `Receipts` table filtered by logs. ContractLogs, /// Prune segment responsible for the `AccountChangeSet` and `AccountHistory` tables. AccountHistory, /// Prune segment responsible for the `StorageChangeSet` and `StorageHistory` tables. StorageHistory, + /// Prune segment responsible for the `CanonicalHeaders`, `Headers` and `HeaderTD` tables. + Headers, } /// PruneSegment error type. diff --git a/crates/prune/src/pruner.rs b/crates/prune/src/pruner.rs index b12cff48f90f..19146c5d0db0 100644 --- a/crates/prune/src/pruner.rs +++ b/crates/prune/src/pruner.rs @@ -103,7 +103,7 @@ impl Pruner { let mut segments = BTreeMap::new(); // TODO(alexey): prune snapshotted segments of data (headers, transactions) - // let highest_snapshots = *self.highest_snapshots_tracker.borrow(); + let highest_snapshots = *self.highest_snapshots_tracker.borrow(); // Multiply `delete_limit` (number of row to delete per block) by number of blocks since // last pruner run. `previous_tip_block_number` is close to `tip_block_number`, usually @@ -282,6 +282,38 @@ impl Pruner { ); } + if let Some(snapshots) = highest_snapshots { + if let (Some(to_block), true) = (snapshots.headers, delete_limit > 0) { + let prune_mode = PruneMode::Before(to_block + 1); + trace!( + target: "pruner", + prune_segment = ?PruneSegment::Headers, + %to_block, + ?prune_mode, + "Got target block to prune" + ); + + let segment_start = Instant::now(); + let segment = segments::Headers::default(); + let output = segment.prune(&provider, PruneInput { to_block, delete_limit })?; + if let Some(checkpoint) = output.checkpoint { + segment + .save_checkpoint(&provider, checkpoint.as_prune_checkpoint(prune_mode))?; + } + self.metrics + .get_prune_segment_metrics(PruneSegment::Headers) + .duration_seconds + .record(segment_start.elapsed()); + + done = done && output.done; + delete_limit = delete_limit.saturating_sub(output.pruned); + segments.insert( + PruneSegment::Headers, + (PruneProgress::from_done(output.done), output.pruned), + ); + } + } + provider.commit()?; self.previous_tip_block_number = Some(tip_block_number); diff --git a/crates/prune/src/segments/headers.rs b/crates/prune/src/segments/headers.rs new file mode 100644 index 000000000000..91ae3fa81d4d --- /dev/null +++ b/crates/prune/src/segments/headers.rs @@ -0,0 +1,192 @@ +use crate::{ + segments::{PruneInput, PruneOutput, PruneOutputCheckpoint, Segment}, + PrunerError, +}; +use itertools::Itertools; +use reth_db::{database::Database, table::Table, tables}; +use reth_interfaces::RethResult; +use reth_primitives::{BlockNumber, PruneSegment}; +use reth_provider::DatabaseProviderRW; +use std::ops::RangeInclusive; +use tracing::{instrument, trace}; + +#[derive(Default)] +#[non_exhaustive] +pub(crate) struct Headers; + +impl Segment for Headers { + const SEGMENT: PruneSegment = PruneSegment::Headers; + + #[instrument(level = "trace", target = "pruner", skip(self, provider), ret)] + fn prune( + &self, + provider: &DatabaseProviderRW<'_, DB>, + input: PruneInput, + ) -> Result { + let block_range = match input.get_next_block_range(provider, Self::SEGMENT)? { + Some(range) => range, + None => { + trace!(target: "pruner", "No headers to prune"); + return Ok(PruneOutput::done()) + } + }; + + let delete_limit = input.delete_limit / 3; + if delete_limit == 0 { + // Nothing to do, `input.delete_limit` is less than 3, so we can't prune all + // headers-related tables up to the same height + return Ok(PruneOutput::not_done()) + } + + let results = [ + self.prune_table::( + provider, + block_range.clone(), + delete_limit, + )?, + self.prune_table::(provider, block_range.clone(), delete_limit)?, + self.prune_table::(provider, block_range, delete_limit)?, + ]; + + if !results.iter().map(|(_, _, last_pruned_block)| last_pruned_block).all_equal() { + return Err(PrunerError::InconsistentData( + "All headers-related tables should be pruned up to the same height", + )) + } + + let (done, pruned, last_pruned_block) = results.into_iter().fold( + (true, 0, 0), + |(total_done, total_pruned, _), (done, pruned, last_pruned_block)| { + (total_done && done, total_pruned + pruned, last_pruned_block) + }, + ); + + Ok(PruneOutput { + done, + pruned, + checkpoint: Some(PruneOutputCheckpoint { + block_number: Some(last_pruned_block), + tx_number: None, + }), + }) + } +} + +impl Headers { + /// Prune one headers-related table. + /// + /// Returns `done`, number of pruned rows and last pruned block number. + fn prune_table>( + &self, + provider: &DatabaseProviderRW<'_, DB>, + range: RangeInclusive, + delete_limit: usize, + ) -> RethResult<(bool, usize, BlockNumber)> { + let mut last_pruned_block = *range.end(); + let (pruned, done) = provider.prune_table_with_range::( + range, + delete_limit, + |_| false, + |row| last_pruned_block = row.0, + )?; + trace!(target: "pruner", %pruned, %done, table = %T::NAME, "Pruned headers"); + + Ok((done, pruned, last_pruned_block)) + } +} + +#[cfg(test)] +mod tests { + use crate::segments::{Headers, PruneInput, PruneOutput, Segment}; + use assert_matches::assert_matches; + use reth_db::tables; + use reth_interfaces::test_utils::{generators, generators::random_header_range}; + use reth_primitives::{BlockNumber, PruneCheckpoint, PruneMode, B256}; + use reth_provider::PruneCheckpointReader; + use reth_stages::test_utils::TestTransaction; + + #[test] + fn prune() { + let tx = TestTransaction::default(); + let mut rng = generators::rng(); + + let headers = random_header_range(&mut rng, 0..100, B256::ZERO); + tx.insert_headers_with_td(headers.iter()).expect("insert headers"); + + assert_eq!(tx.table::().unwrap().len(), headers.len()); + assert_eq!(tx.table::().unwrap().len(), headers.len()); + assert_eq!(tx.table::().unwrap().len(), headers.len()); + + let test_prune = |to_block: BlockNumber, expected_result: (bool, usize)| { + let prune_mode = PruneMode::Before(to_block); + let input = PruneInput { to_block, delete_limit: 10 }; + let segment = Headers::default(); + + let next_block_number_to_prune = tx + .inner() + .get_prune_checkpoint(Headers::SEGMENT) + .unwrap() + .and_then(|checkpoint| checkpoint.block_number) + .map(|block_number| block_number + 1) + .unwrap_or_default(); + + let provider = tx.inner_rw(); + let result = segment.prune(&provider, input).unwrap(); + assert_matches!( + result, + PruneOutput {done, pruned, checkpoint: Some(_)} + if (done, pruned) == expected_result + ); + segment + .save_checkpoint( + &provider, + result.checkpoint.unwrap().as_prune_checkpoint(prune_mode), + ) + .unwrap(); + provider.commit().expect("commit"); + + let last_pruned_block_number = to_block + .min(next_block_number_to_prune + input.delete_limit as BlockNumber / 3 - 1); + + assert_eq!( + tx.table::().unwrap().len(), + headers.len() - (last_pruned_block_number + 1) as usize + ); + assert_eq!( + tx.table::().unwrap().len(), + headers.len() - (last_pruned_block_number + 1) as usize + ); + assert_eq!( + tx.table::().unwrap().len(), + headers.len() - (last_pruned_block_number + 1) as usize + ); + assert_eq!( + tx.inner().get_prune_checkpoint(Headers::SEGMENT).unwrap(), + Some(PruneCheckpoint { + block_number: Some(last_pruned_block_number), + tx_number: None, + prune_mode + }) + ); + }; + + test_prune(3, (false, 9)); + test_prune(3, (true, 3)); + } + + #[test] + fn prune_cannot_be_done() { + let tx = TestTransaction::default(); + + let input = PruneInput { + to_block: 1, + // Less than total number of tables for `Headers` segment + delete_limit: 2, + }; + let segment = Headers::default(); + + let provider = tx.inner_rw(); + let result = segment.prune(&provider, input).unwrap(); + assert_eq!(result, PruneOutput::not_done()); + } +} diff --git a/crates/prune/src/segments/mod.rs b/crates/prune/src/segments/mod.rs index 56e0a12548a0..04f45c6e01bc 100644 --- a/crates/prune/src/segments/mod.rs +++ b/crates/prune/src/segments/mod.rs @@ -1,4 +1,7 @@ +mod headers; mod receipts; + +pub(crate) use headers::Headers; pub(crate) use receipts::Receipts; use crate::PrunerError; @@ -88,10 +91,39 @@ impl PruneInput { Ok(Some(range)) } + + /// Get next inclusive block range to prune according to the checkpoint, `to_block` block + /// number and `limit`. + /// + /// To get the range start (`from_block`): + /// 1. If checkpoint exists, use next block. + /// 2. If checkpoint doesn't exist, use block 0. + /// + /// To get the range end: use block `to_block`. + pub(crate) fn get_next_block_range( + &self, + provider: &DatabaseProviderRW<'_, DB>, + segment: PruneSegment, + ) -> RethResult>> { + let from_block = provider + .get_prune_checkpoint(segment)? + .and_then(|checkpoint| checkpoint.block_number) + // Checkpoint exists, prune from the next block after the highest pruned one + .map(|block_number| block_number + 1) + // No checkpoint exists, prune from genesis + .unwrap_or(0); + + let range = from_block..=self.to_block; + if range.is_empty() { + return Ok(None) + } + + Ok(Some(range)) + } } /// Segment pruning output, see [Segment::prune]. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, Eq, PartialEq)] pub(crate) struct PruneOutput { /// `true` if pruning has been completed up to the target block, and `false` if there's more /// data to prune in further runs. @@ -105,12 +137,18 @@ pub(crate) struct PruneOutput { impl PruneOutput { /// Returns a [PruneOutput] with `done = true`, `pruned = 0` and `checkpoint = None`. /// Use when no pruning is needed. - pub(crate) fn done() -> Self { + pub(crate) const fn done() -> Self { Self { done: true, pruned: 0, checkpoint: None } } + + /// Returns a [PruneOutput] with `done = false`, `pruned = 0` and `checkpoint = None`. + /// Use when pruning is needed but cannot be done. + pub(crate) const fn not_done() -> Self { + Self { done: false, pruned: 0, checkpoint: None } + } } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, Eq, PartialEq)] pub(crate) struct PruneOutputCheckpoint { /// Highest pruned block number. If it's [None], the pruning for block `0` is not finished yet. pub(crate) block_number: Option, diff --git a/crates/prune/src/segments/receipts.rs b/crates/prune/src/segments/receipts.rs index 721a57920ce4..ee63a08c6f72 100644 --- a/crates/prune/src/segments/receipts.rs +++ b/crates/prune/src/segments/receipts.rs @@ -136,20 +136,6 @@ mod tests { .min(next_tx_number_to_prune as usize + input.delete_limit) .sub(1); - let last_pruned_block_number = blocks - .iter() - .fold_while((0, 0), |(_, mut tx_count), block| { - tx_count += block.body.len(); - - if tx_count > last_pruned_tx_number { - Done((block.number, tx_count)) - } else { - Continue((block.number, tx_count)) - } - }) - .into_inner() - .0; - let provider = tx.inner_rw(); let result = segment.prune(&provider, input).unwrap(); assert_matches!( @@ -165,8 +151,20 @@ mod tests { .unwrap(); provider.commit().expect("commit"); - let last_pruned_block_number = - last_pruned_block_number.checked_sub(if result.done { 0 } else { 1 }); + let last_pruned_block_number = blocks + .iter() + .fold_while((0, 0), |(_, mut tx_count), block| { + tx_count += block.body.len(); + + if tx_count > last_pruned_tx_number { + Done((block.number, tx_count)) + } else { + Continue((block.number, tx_count)) + } + }) + .into_inner() + .0 + .checked_sub(if result.done { 0 } else { 1 }); assert_eq!( tx.table::().unwrap().len(), diff --git a/crates/stages/src/test_utils/test_db.rs b/crates/stages/src/test_utils/test_db.rs index b0d14e648350..6c5cc623b668 100644 --- a/crates/stages/src/test_utils/test_db.rs +++ b/crates/stages/src/test_utils/test_db.rs @@ -219,7 +219,7 @@ impl TestTransaction { /// Inserts total difficulty of headers into the corresponding tables. /// /// Superset functionality of [TestTransaction::insert_headers]. - pub(crate) fn insert_headers_with_td<'a, I>(&self, headers: I) -> Result<(), DbError> + pub fn insert_headers_with_td<'a, I>(&self, headers: I) -> Result<(), DbError> where I: Iterator, { diff --git a/crates/storage/provider/src/providers/database/provider.rs b/crates/storage/provider/src/providers/database/provider.rs index 0204dac34fde..7beae9b5d154 100644 --- a/crates/storage/provider/src/providers/database/provider.rs +++ b/crates/storage/provider/src/providers/database/provider.rs @@ -684,16 +684,19 @@ impl<'this, TX: DbTxMut<'this> + DbTx<'this>> DatabaseProvider<'this, TX> { let mut deleted = 0; let mut keys = keys.into_iter(); - for key in &mut keys { - if deleted == limit { - break - } - let row = cursor.seek_exact(key.clone())?; - if let Some(row) = row { - cursor.delete_current()?; - deleted += 1; - delete_callback(row); + if limit != 0 { + for key in &mut keys { + let row = cursor.seek_exact(key.clone())?; + if let Some(row) = row { + cursor.delete_current()?; + deleted += 1; + delete_callback(row); + } + + if deleted == limit { + break + } } } @@ -714,15 +717,17 @@ impl<'this, TX: DbTxMut<'this> + DbTx<'this>> DatabaseProvider<'this, TX> { let mut walker = cursor.walk_range(keys)?; let mut deleted = 0; - while let Some(row) = walker.next().transpose()? { - if deleted == limit { - break - } + if limit != 0 { + while let Some(row) = walker.next().transpose()? { + if !skip_filter(&row) { + walker.delete_current()?; + deleted += 1; + delete_callback(row); + } - if !skip_filter(&row) { - walker.delete_current()?; - deleted += 1; - delete_callback(row); + if deleted == limit { + break + } } }