From 8e0d77f35b174558e2975bdcacf76485e3736882 Mon Sep 17 00:00:00 2001 From: dkathiriya <8559992+lakshya-sky@users.noreply.github.com> Date: Tue, 22 Oct 2024 15:17:28 -0400 Subject: [PATCH 1/5] reuse inspector --- crates/rpc/rpc/src/debug.rs | 51 ++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/crates/rpc/rpc/src/debug.rs b/crates/rpc/rpc/src/debug.rs index 2d9d6f7822e1..d5153dbc9c36 100644 --- a/crates/rpc/rpc/src/debug.rs +++ b/crates/rpc/rpc/src/debug.rs @@ -112,6 +112,7 @@ where let mut results = Vec::with_capacity(transactions.len()); let mut db = CacheDB::new(StateProviderDatabase::new(state)); let mut transactions = transactions.into_iter().enumerate().peekable(); + let mut shared_inspector = None; while let Some((index, tx)) = transactions.next() { let tx_hash = tx.hash; @@ -124,7 +125,7 @@ where handler_cfg: cfg.handler_cfg, }; let (result, state_changes) = this.trace_transaction( - opts.clone(), + &opts, env, &mut db, Some(TransactionContext { @@ -132,7 +133,9 @@ where tx_hash: Some(tx_hash), tx_index: Some(index), }), + &mut shared_inspector, )?; + shared_inspector.as_mut().map(|i| i.fuse()); results.push(TraceResult::Success { result, tx_hash: Some(tx_hash) }); if transactions.peek().is_some() { @@ -295,7 +298,7 @@ where }; this.trace_transaction( - opts, + &opts, env, &mut db, Some(TransactionContext { @@ -303,6 +306,7 @@ where tx_index: Some(index), tx_hash: Some(tx.hash), }), + &mut None, ) .map(|(trace, _)| trace) }) @@ -573,6 +577,7 @@ where let Bundle { transactions, block_override } = bundle; let block_overrides = block_override.map(Box::new); + let mut shared_inspector = None; let mut transactions = transactions.into_iter().peekable(); while let Some(tx) = transactions.next() { @@ -588,8 +593,15 @@ where overrides, )?; - let (trace, state) = - this.trace_transaction(tracing_options.clone(), env, &mut db, None)?; + let (trace, state) = this.trace_transaction( + &tracing_options, + env, + &mut db, + None, + &mut shared_inspector, + )?; + + shared_inspector.as_mut().map(|i| i.fuse()); // If there is more transactions, commit the database // If there is no transactions, but more bundles, commit to the database too @@ -699,10 +711,11 @@ where /// Caution: this is blocking and should be performed on a blocking task. fn trace_transaction( &self, - opts: GethDebugTracingOptions, + opts: &GethDebugTracingOptions, env: EnvWithHandlerCfg, db: &mut StateCacheDb<'_>, transaction_context: Option, + shared_inspector: &mut Option, ) -> Result<(GethTrace, revm_primitives::EvmState), Eth::Error> { let GethDebugTracingOptions { config, tracer, tracer_config, .. } = opts; @@ -716,24 +729,27 @@ where } GethDebugBuiltInTracerType::CallTracer => { let call_config = tracer_config + .clone() .into_call_config() .map_err(|_| EthApiError::InvalidTracerConfig)?; - let mut inspector = TracingInspector::new( + let mut inspector = shared_inspector.get_or_insert(TracingInspector::new( TracingInspectorConfig::from_geth_call_config(&call_config), - ); + )); let (res, env) = self.eth_api().inspect(db, env, &mut inspector)?; + inspector.set_transaction_gas_limit(env.tx.gas_limit); + let frame = inspector - .with_transaction_gas_limit(env.tx.gas_limit) - .into_geth_builder() + .geth_builder() .geth_call_traces(call_config, res.result.gas_used()); return Ok((frame.into(), res.state)) } GethDebugBuiltInTracerType::PreStateTracer => { let prestate_config = tracer_config + .clone() .into_pre_state_config() .map_err(|_| EthApiError::InvalidTracerConfig)?; @@ -755,6 +771,7 @@ where } GethDebugBuiltInTracerType::MuxTracer => { let mux_config = tracer_config + .clone() .into_mux_config() .map_err(|_| EthApiError::InvalidTracerConfig)?; @@ -769,6 +786,7 @@ where } GethDebugBuiltInTracerType::FlatCallTracer => { let flat_call_config = tracer_config + .clone() .into_flat_call_config() .map_err(|_| EthApiError::InvalidTracerConfig)?; @@ -799,10 +817,10 @@ where } #[cfg(feature = "js-tracer")] GethDebugTracerType::JsTracer(code) => { - let config = tracer_config.into_json(); + let config = tracer_config.clone().into_json(); let mut inspector = revm_inspectors::tracing::js::JsInspector::with_transaction_context( - code, + code.clone(), config, transaction_context.unwrap_or_default(), ) @@ -818,17 +836,16 @@ where } // default structlog tracer - let inspector_config = TracingInspectorConfig::from_geth_config(&config); + let inspector_config = TracingInspectorConfig::from_geth_config(config); - let mut inspector = TracingInspector::new(inspector_config); + //let mut inspector = TracingInspector::new(inspector_config); + let mut inspector = shared_inspector.get_or_insert(TracingInspector::new(inspector_config)); let (res, env) = self.eth_api().inspect(db, env, &mut inspector)?; let gas_used = res.result.gas_used(); let return_value = res.result.into_output().unwrap_or_default(); - let frame = inspector - .with_transaction_gas_limit(env.tx.gas_limit) - .into_geth_builder() - .geth_traces(gas_used, return_value, config); + inspector.set_transaction_gas_limit(env.tx.gas_limit); + let frame = inspector.geth_builder().geth_traces(gas_used, return_value, config.clone()); Ok((frame.into(), res.state)) } From 5285b84c88ea61a7f3977968f24cf8d4c6b844ed Mon Sep 17 00:00:00 2001 From: dkathiriya <8559992+lakshya-sky@users.noreply.github.com> Date: Tue, 22 Oct 2024 15:31:09 -0400 Subject: [PATCH 2/5] fix lints --- crates/rpc/rpc/src/debug.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/rpc/rpc/src/debug.rs b/crates/rpc/rpc/src/debug.rs index d5153dbc9c36..6bb386602a6d 100644 --- a/crates/rpc/rpc/src/debug.rs +++ b/crates/rpc/rpc/src/debug.rs @@ -135,7 +135,10 @@ where }), &mut shared_inspector, )?; - shared_inspector.as_mut().map(|i| i.fuse()); + + if let Some(i) = shared_inspector.as_mut() { + i.fuse(); + } results.push(TraceResult::Success { result, tx_hash: Some(tx_hash) }); if transactions.peek().is_some() { @@ -601,7 +604,9 @@ where &mut shared_inspector, )?; - shared_inspector.as_mut().map(|i| i.fuse()); + if let Some(i) = shared_inspector.as_mut() { + i.fuse(); + } // If there is more transactions, commit the database // If there is no transactions, but more bundles, commit to the database too @@ -845,7 +850,7 @@ where let gas_used = res.result.gas_used(); let return_value = res.result.into_output().unwrap_or_default(); inspector.set_transaction_gas_limit(env.tx.gas_limit); - let frame = inspector.geth_builder().geth_traces(gas_used, return_value, config.clone()); + let frame = inspector.geth_builder().geth_traces(gas_used, return_value, *config); Ok((frame.into(), res.state)) } From ebbc8e11466fce3a407ff668e1e3de93b2b6ecd1 Mon Sep 17 00:00:00 2001 From: dkathiriya <8559992+lakshya-sky@users.noreply.github.com> Date: Tue, 22 Oct 2024 15:37:42 -0400 Subject: [PATCH 3/5] change rest of the geth style traces --- crates/rpc/rpc/src/debug.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/crates/rpc/rpc/src/debug.rs b/crates/rpc/rpc/src/debug.rs index 6bb386602a6d..013518db5fcd 100644 --- a/crates/rpc/rpc/src/debug.rs +++ b/crates/rpc/rpc/src/debug.rs @@ -758,14 +758,14 @@ where .into_pre_state_config() .map_err(|_| EthApiError::InvalidTracerConfig)?; - let mut inspector = TracingInspector::new( + let mut inspector = shared_inspector.get_or_insert(TracingInspector::new( TracingInspectorConfig::from_geth_prestate_config(&prestate_config), - ); + )); let (res, env) = self.eth_api().inspect(&mut *db, env, &mut inspector)?; + inspector.set_transaction_gas_limit(env.tx.gas_limit); let frame = inspector - .with_transaction_gas_limit(env.tx.gas_limit) - .into_geth_builder() + .geth_builder() .geth_prestate_traces(&res, &prestate_config, db) .map_err(Eth::Error::from_eth_err)?; @@ -842,10 +842,7 @@ where // default structlog tracer let inspector_config = TracingInspectorConfig::from_geth_config(config); - - //let mut inspector = TracingInspector::new(inspector_config); let mut inspector = shared_inspector.get_or_insert(TracingInspector::new(inspector_config)); - let (res, env) = self.eth_api().inspect(db, env, &mut inspector)?; let gas_used = res.result.gas_used(); let return_value = res.result.into_output().unwrap_or_default(); From 6152647499aa4bd2fe935d4fbc2d5a5b62b0a389 Mon Sep 17 00:00:00 2001 From: dkathiriya <8559992+lakshya-sky@users.noreply.github.com> Date: Thu, 24 Oct 2024 12:01:20 -0400 Subject: [PATCH 4/5] fix suggestions --- crates/rpc/rpc/src/debug.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/crates/rpc/rpc/src/debug.rs b/crates/rpc/rpc/src/debug.rs index 013518db5fcd..03c371e75cab 100644 --- a/crates/rpc/rpc/src/debug.rs +++ b/crates/rpc/rpc/src/debug.rs @@ -112,7 +112,7 @@ where let mut results = Vec::with_capacity(transactions.len()); let mut db = CacheDB::new(StateProviderDatabase::new(state)); let mut transactions = transactions.into_iter().enumerate().peekable(); - let mut shared_inspector = None; + let mut inspector = None; while let Some((index, tx)) = transactions.next() { let tx_hash = tx.hash; @@ -133,12 +133,10 @@ where tx_hash: Some(tx_hash), tx_index: Some(index), }), - &mut shared_inspector, + &mut inspector, )?; - if let Some(i) = shared_inspector.as_mut() { - i.fuse(); - } + inspector = inspector.map(|insp| insp.fused()); results.push(TraceResult::Success { result, tx_hash: Some(tx_hash) }); if transactions.peek().is_some() { @@ -580,7 +578,7 @@ where let Bundle { transactions, block_override } = bundle; let block_overrides = block_override.map(Box::new); - let mut shared_inspector = None; + let mut inspector = None; let mut transactions = transactions.into_iter().peekable(); while let Some(tx) = transactions.next() { @@ -601,12 +599,10 @@ where env, &mut db, None, - &mut shared_inspector, + &mut inspector, )?; - if let Some(i) = shared_inspector.as_mut() { - i.fuse(); - } + inspector = inspector.map(|insp| insp.fused()); // If there is more transactions, commit the database // If there is no transactions, but more bundles, commit to the database too @@ -709,6 +705,12 @@ where /// Executes the configured transaction with the environment on the given database. /// + /// It optionally takes shared inspector to avoid re-creating the inspector for each + /// + /// transaction. This is useful when tracing multiple transactions in a block. + /// + /// If the inspector is provided then `opts.tracer_config` is ignored. + /// /// Returns the trace frame and the state that got updated after executing the transaction. /// /// Note: this does not apply any state overrides if they're configured in the `opts`. @@ -841,8 +843,10 @@ where } // default structlog tracer - let inspector_config = TracingInspectorConfig::from_geth_config(config); - let mut inspector = shared_inspector.get_or_insert(TracingInspector::new(inspector_config)); + let mut inspector = shared_inspector.get_or_insert_with(|| { + let inspector_config = TracingInspectorConfig::from_geth_config(config); + TracingInspector::new(inspector_config) + }); let (res, env) = self.eth_api().inspect(db, env, &mut inspector)?; let gas_used = res.result.gas_used(); let return_value = res.result.into_output().unwrap_or_default(); From fc563cb178a36dff146c4fd78e51796b5399c019 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Sat, 26 Oct 2024 09:29:45 +0200 Subject: [PATCH 5/5] docs: clarify fused inspector --- crates/rpc/rpc/src/debug.rs | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/crates/rpc/rpc/src/debug.rs b/crates/rpc/rpc/src/debug.rs index 03c371e75cab..5a20bee975ff 100644 --- a/crates/rpc/rpc/src/debug.rs +++ b/crates/rpc/rpc/src/debug.rs @@ -705,11 +705,12 @@ where /// Executes the configured transaction with the environment on the given database. /// - /// It optionally takes shared inspector to avoid re-creating the inspector for each + /// It optionally takes fused inspector ([`TracingInspector::fused`]) to avoid re-creating the + /// inspector for each transaction. This is useful when tracing multiple transactions in a + /// block. This is only useful for block tracing which uses the same tracer for all transactions + /// in the block. /// - /// transaction. This is useful when tracing multiple transactions in a block. - /// - /// If the inspector is provided then `opts.tracer_config` is ignored. + /// Caution: If the inspector is provided then `opts.tracer_config` is ignored. /// /// Returns the trace frame and the state that got updated after executing the transaction. /// @@ -722,7 +723,7 @@ where env: EnvWithHandlerCfg, db: &mut StateCacheDb<'_>, transaction_context: Option, - shared_inspector: &mut Option, + fused_inspector: &mut Option, ) -> Result<(GethTrace, revm_primitives::EvmState), Eth::Error> { let GethDebugTracingOptions { config, tracer, tracer_config, .. } = opts; @@ -740,9 +741,11 @@ where .into_call_config() .map_err(|_| EthApiError::InvalidTracerConfig)?; - let mut inspector = shared_inspector.get_or_insert(TracingInspector::new( - TracingInspectorConfig::from_geth_call_config(&call_config), - )); + let mut inspector = fused_inspector.get_or_insert_with(|| { + TracingInspector::new(TracingInspectorConfig::from_geth_call_config( + &call_config, + )) + }); let (res, env) = self.eth_api().inspect(db, env, &mut inspector)?; @@ -760,9 +763,11 @@ where .into_pre_state_config() .map_err(|_| EthApiError::InvalidTracerConfig)?; - let mut inspector = shared_inspector.get_or_insert(TracingInspector::new( - TracingInspectorConfig::from_geth_prestate_config(&prestate_config), - )); + let mut inspector = fused_inspector.get_or_insert_with(|| { + TracingInspector::new( + TracingInspectorConfig::from_geth_prestate_config(&prestate_config), + ) + }); let (res, env) = self.eth_api().inspect(&mut *db, env, &mut inspector)?; inspector.set_transaction_gas_limit(env.tx.gas_limit); @@ -843,7 +848,7 @@ where } // default structlog tracer - let mut inspector = shared_inspector.get_or_insert_with(|| { + let mut inspector = fused_inspector.get_or_insert_with(|| { let inspector_config = TracingInspectorConfig::from_geth_config(config); TracingInspector::new(inspector_config) });