Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(anvil): --disable-tracing #7445

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

yash-atreya
Copy link
Collaborator

Motivation

#7442

Solution

Add CLI arg --disable-tracing to disable all tx tracing in anvil. Default value is false.

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this now needs to be propagated to

pub async fn with_genesis(

and then eventually to the executor:

let executor = TransactionExecutor {

crates/anvil/src/config.rs Outdated Show resolved Hide resolved
@yash-atreya yash-atreya marked this pull request as ready for review March 20, 2024 15:23
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we disable tracing we should skip the inspector entirely

Comment on lines +288 to +293
let mut inspector = Inspector::default();
if !self.disable_tracing {
inspector = inspector.with_tracing();
if self.enable_steps_tracing {
inspector = inspector.with_steps_tracing();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can leave the setup as is, but if tracing is disabled we need to change this:

foundry_evm::utils::new_evm_with_inspector(&mut *self.db, env, &mut inspector);

to evm setup without inspector, which is always faster than with any inspector

Comment on lines +303 to +311
match evm.transact_commit() {
Ok(exec_result) => exec_result,
Err(err) => {
warn!(target: "backend", "[{:?}] failed to execute: {:?}", transaction.hash(), err);
match err {
EVMError::Database(err) => {
return Some(TransactionExecutionOutcome::DatabaseError(
transaction,
err,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need the duplication here if we make the if else block return the result?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue was building the evm with different types, one with an inspector and one without.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need the duplication here if we make the if else block return the result?

@mattsse The if else block already returns the exec_result.

let exec_result = if !self.disable_tracing {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants