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: tracing in Rethnet using JS callbacks #3593

Merged
merged 35 commits into from
Feb 21, 2023
Merged

Conversation

Wodann
Copy link
Member

@Wodann Wodann commented Jan 18, 2023

The stack trace implementation has some caveats:

  • traceTransaction is not implemented for Rethnet. This means that there are still 10 failing tests in HARDHAT_EXPERIMENTAL_VM_MODE=rethnet due to Error: traceTransaction not implemented for Rethnet
  • The "Before byzantium" test is skipped
  • There are 7 failing tests in dual adapter mode: 5 in stack traces, 2 (1x Hardhat Network Provider & 1x JSON-RPC provider) in eth_sendTransaction > when automine is enabled > Shoud throw if the transaction fails; all due to differences in the number of stack trace steps. We agreed that it's a time sync to try and resolve them now

In the future we need to revisit the JsTracer implementation to remove ethereumjs specific fixes. This will be a breaking change for the Hardhat tracer ABI, but makes the implementation more "sane".

@Wodann Wodann self-assigned this Jan 18, 2023
@changeset-bot
Copy link

changeset-bot bot commented Jan 18, 2023

⚠️ No Changeset found

Latest commit: 6ddf132

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 Jan 18, 2023

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

Name Status Preview Comments Updated
hardhat ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 7:14AM (UTC)
hardhat-storybook ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 7:14AM (UTC)

result: rethnet_evm::ExecutionResult {
exit_reason: ret,
out: rethnet_evm::TransactOut::Call(out.clone()),
gas_used: if ret == Return::InvalidOpcode || ret == Return::OpcodeNotFound {
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to do this to match ethereumjs (same pattern below). What do you think, @fvictorio, should this be handled in revm or here?

Copy link
Member

@fvictorio fvictorio Jan 19, 2023

Choose a reason for hiding this comment

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

Ugh, good question, I don't know. It seems to me like something revm should do, but maybe there's a good reason to do it this way. In any case, it's worth opening an issue to discuss it.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Wodann Wodann merged commit ce44184 into rethnet/main Feb 21, 2023
@Wodann Wodann deleted the rethnet/ts-tracing branch February 21, 2023 19:18
@Wodann Wodann restored the rethnet/ts-tracing branch February 21, 2023 19:19
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2023
@fvictorio fvictorio deleted the rethnet/ts-tracing branch March 7, 2024 15:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Archived in project
3 participants