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

Implement debug_traceCall #192

Closed
Wodann opened this issue Sep 27, 2023 · 3 comments · Fixed by NomicFoundation/hardhat#4433
Closed

Implement debug_traceCall #192

Wodann opened this issue Sep 27, 2023 · 3 comments · Fixed by NomicFoundation/hardhat#4433
Assignees
Milestone

Comments

@Wodann
Copy link
Member

Wodann commented Sep 27, 2023

Building on the work done in #65, add support for the debug_traceCall API by using the same tracer to operate on a single transaction

Definition of Done

  • Implement the debug_traceCall functionality in rethnet_evm
  • Expose the trace_call function through the NAPI and use it in the stubbed version of the EDR VM adapter
@Wodann Wodann added this to EDR Sep 27, 2023
@Wodann Wodann converted this from a draft issue Sep 27, 2023
@Wodann Wodann added this to the EDR v0.1.0 milestone Sep 27, 2023
@agostbiro
Copy link
Member

Questions:

  • debug_traceCall is implemented in main, but not in rethnet/main. Mergin debug_traceCall to rethnet/main is a blocker.
  • Assuming we want to test against the fixtures in main. It's unclear why some of these have stack, memory and storage fields and others don't.
  • I'm assuming we want EIP-3155 compatible output in EDR for this feature?
  • I noticed that there are a lot of adjustments made in _runTxAndRevertMutations in main that were moved into the EthereumJS VM in rethnet/main. It's possible that there will be unexpected issues due to this when exposing debug_traceCall support in EDR.
  • The way debug_traceTransaction is implemented currently is not ideal and doesn't have support for gas cost of calls and creates. The problem is that the current implementation just adopts the REVM TracerEIp3155 implementation that operates conceptually on the opcode level. It already requires some hackery to make this approach work for calls/creates, and it breaks down for call gas cost calculation. A better approach would be to start a new trace for each call/create and then flatten the results to the format that EIP-3155 prescribes. To achieve this, I’d do a refactoring of rethnet_evm::trace & rethnet_evm::debug_trace to have a single trace collector that collects all the info needed for trace and debug_trace and then have methods that output it in the desired formats. Do we want to rectify that as part of this PR?

@Wodann
Copy link
Member Author

Wodann commented Sep 27, 2023

  1. The merge is almost done. It's in the final stage of review
  2. It might be related to the "disable*" booleans. If it doesn't comply with the spec, Franco Victorio is happy to fix the Hardhat side
  3. Yes, that'd be great.
  4. This should have been resolved in the aforementioned merge that Franco & I did
  5. As we need debug_trace for the release, I'd opt for the shortest path to completion and do the refactor afterwards

@agostbiro agostbiro linked a pull request Oct 3, 2023 that will close this issue
@agostbiro agostbiro moved this from Todo to Under Review in EDR Oct 3, 2023
@agostbiro
Copy link
Member

Done in NomicFoundation/hardhat#4433

@github-project-automation github-project-automation bot moved this from Under Review to Done in EDR Oct 5, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants