Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

fix: return error for eth_call when call errors #1589

Merged
merged 3 commits into from Nov 18, 2021
Merged

Conversation

davidmurdoch
Copy link
Member

fixes #1496

@davidmurdoch davidmurdoch marked this pull request as ready for review November 17, 2021 17:48
Copy link
Contributor

@MicaiahReid MicaiahReid left a comment

Choose a reason for hiding this comment

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

I've tested the change and it all looks good.

I do think this change gives further support for the idea of reversing --vmErrorsOnRPCResponse to something more like --suppressVmErrors, where the vm errors are enabled by default and can optionally be ignored.

It seems strange that the default use case would be that most vm errors are disabled, except for those from eth_call which are always enabled.

Then again, if that's what geth does that's what geth does.

@davidmurdoch
Copy link
Member Author

Allowing users to do vmErrorsOnRPCResponse = false was so they can make ganache more like a real node, since when running a transaction real nodes don't throw on errors. This makes sense, because transactions aren't run immediately, but they are prioritized and only run when the miner chooses to put the transaction in a block -- and sometimes that can take days (or never).

eth_call is often used by developers to learn why a previously run transaction reverted. This PR makes that possible by default again.

Still +1 on picking a new name for this flag. Changing the default mode would also require changing the default for legacyInstamine to true, which doesn't make it very legacy any more. haha

@davidmurdoch davidmurdoch changed the title fix return error for eth_call when call errors fix: return error for eth_call when call errors Nov 17, 2021
@davidmurdoch davidmurdoch force-pushed the fix-eth_call branch 2 times, most recently from a956838 to 00cf860 Compare November 18, 2021 23:12
@davidmurdoch davidmurdoch merged commit 667834d into develop Nov 18, 2021
@davidmurdoch davidmurdoch deleted the fix-eth_call branch November 18, 2021 23:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

eth_call should return JSON-RPC error on revert
2 participants