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: debug trace transaction #4398

Merged
merged 26 commits into from
Sep 28, 2023
Merged

Conversation

agostbiro
Copy link
Member

WIP to add debug_traceTransaction support for EDR.

I verified that the simple tracer API and the EIP-3155 API are mutually exclusive, so we don’t have support them both for the same transaction execution and we don’t have to change the with_trace logic in the run , dry_run and guaranteed_dry_run methods.

We initially thought the quickest way to add debug_traceTransaction was to keep most of the debug_traceTransaction implementation in JS for now and add a rethnet variant for VMDebugTracer of Hardhat Network. This has proven difficult, because VMDebugTracer is tightly coupled with the ethereum-js vm.

After studying the code and experimenting a bit, I decided instead to expose a debug_trace_transaction function from rethnet_evm_napi that delegates to a corresponding function in rethnet_evm. This function makes a clone of the state, and executes and commits all the prior transactions in the block to the cloned state and then executes the transaction with the matching hash with TracerEip3155 and returns the debug trace output. The transactions from the block are taken as Vec<rethnet_evm_napi::TransactionRequest> as a conversion method for this already exists on the JS side. For the other napi arguments I follow the pattern in runtime.rs. For executing the transactions in rethnet_evm::debug_trace_transaction I follow the pattern in BlockBuilder::add_transaction.

I decided to adapt the implementation of revm::TracerEip3155 as it has an awkward interface to get the logs out and it doesn't follow the EIP-3155 spec. The biggest todo left is adding the fields that are obligatory in EIP-3155, but missing from revm::TracerEip3155 and adding the optional fields that Hardhat currently supports.

@agostbiro agostbiro self-assigned this Sep 18, 2023
@changeset-bot
Copy link

changeset-bot bot commented Sep 18, 2023

⚠️ No Changeset found

Latest commit: a476fa8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Sep 18, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hardhat ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 27, 2023 9:52am
hardhat-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 27, 2023 9:52am

@agostbiro agostbiro marked this pull request as draft September 18, 2023 17:27
@agostbiro agostbiro force-pushed the rethnet/debug-trace-transaction branch from f1d9261 to f072615 Compare September 19, 2023 19:36
@agostbiro agostbiro changed the base branch from rethnet/main to rethnet/napi-b256-try-cast September 19, 2023 19:37
Base automatically changed from rethnet/napi-b256-try-cast to rethnet/main September 19, 2023 21:17
@Wodann
Copy link
Member

Wodann commented Sep 26, 2023

Were you able to get the dual mode adapter working properly, @agostbiro?

I'd also appreciate if you could have a look at the TS code, @fvictorio 🙏

@agostbiro
Copy link
Member Author

Were you able to get the dual mode adapter working properly, @agostbiro?

No, unfortunately not. Looks like my comments got lost: #4398 (comment)

@agostbiro
Copy link
Member Author

Can't seem to reply to these comments for some reason:

#4398 (comment)

Done in c9457d4

#4398 (comment)

Done in 9af7ec2

@Wodann
Copy link
Member

Wodann commented Sep 28, 2023

Merging this as it all tests are succeeding and I don't see any issues with the TS.

When you have time to look at the TS, could you please let us know if there are any issues, @fvictorio ?

@Wodann Wodann merged commit df199e5 into rethnet/main Sep 28, 2023
@Wodann Wodann deleted the rethnet/debug-trace-transaction branch September 28, 2023 12:18
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Archived in project
2 participants