New error reporting in beta causes undefined results in numerous cases

[benjamincburns]

Expected Behavior

When the vmErrorsOnRPCResponse option is set to true in the provider, submitting a transaction via eth_sendTransaction, eth_sendRawTransaction, or personal_sendTransaction, always returns a response with a transaction hash in the results field.

Current Behavior

In some cases such as out of gas, or gas exceeds block gas limit, the result field on the JSON RPC response is undefined.

Your Environment

• Versions impacted: anything that depends on ganache-core v2.1.0-beta.0, including ganache-cli v6.1.0-beta.0 and ganache v1.1.0-beta.0

[benjamincburns]

Thanks to @cgewecke for pointing this out to me!

[nickjm]

Thanks for looking into this! I have reason to believe that the issue isn't just out of gas, but also in case of revert being thrown (which doesn't burn up the rest of your gas). Just wanted to clarify.

[benjamincburns]

Hi @nickjm. Revert, stack underflow, invalid jump, invalid opcode, out of gas, etc, are all cases of VM runtime errors. Ethereum nodes like geth/Parity don't report these errors when you call eth_sendTransaction and similar because the transaction hasn't been run through the EVM yet. Instead, you're meant to fetch the transaction receipt and check the value of the status field.

This case deals specifically with transaction rejection, which triggers an immediate error response from eth_sendTransaction. This happens during initial transaction validation, and can be caused by things like a bad nonce, transaction gas limit exceeding block gas limit, failure to unlock the sender account, etc.

Per our recent release notes, whether or not runtime errors are reported as RPC errors is controlled by the vmErrorsOnRPCResponse flag. In the latest beta of ganache-cli this flag defaults to true (though in an earlier beta it was hardcoded to false. Setting this flag to false will cause ganache-cli to behave like geth and Parity.

[nickjm]

@benjamincburns I'm a little confused. When I run a transaction that fails because of a revert, I don't have the opportunity to check the receipt with this version of ganache because an error is thrown and that error has an undefined data field and the message field doesn't include any additional information, just says "invalid json rpc response." Is this expected behavior? I was under the impression that a revert should not throw an error in javascript, and I could check the transaction receipt like you said.

[benjamincburns]

I don't have the opportunity to check the receipt with this version of ganache because an error is thrown and that error has an undefined data field and the message field doesn't include any additional information, just says "invalid json rpc response."

Sorry for the confusion - what you describe is not expected behavior, and it's what this issue, plus trufflesuite/ganache-core#62 are meant to address.

[benjamincburns]

Ugh, I just reread and it seems that I've been missing what you're saying all along! So sorry about that...

When I run a transaction that fails because of a revert

Here's the intended behavior on a REVERT tx runtime error:

if vmErrorsOnRPCResponse is true, you should get back a response from eth_sendTransaction which contains both a result field and an error field. The result field will contain the transaction hash.

if vmErrorsOnRPCResponse is false, you should get back a response from eth_sendTransaction which contains only a result field, which contains the transaction hash.

If anything else happens when REVERT is triggered (such as a response object with neither error nor result fields), it's a bug, and not addressed by this issue or the PR I linked to before.

Edit: Actually, let's just expand the scope of this issue to handle that case. I'll add a test to the PR and we'll take it from there.

[nickjm]

Ok great. As a reminder, my basic flow is

try {
  await promise; // some generic transaction with a contract that should fail via revert or require.
} catch (error) {
  const revertEvent = error.message.search('revert') >= 0; // doesn't work
  const revertMessage = error.data; // undefined as well
  // actual message is Invalid JSON RPC response: {"id":202,"jsonrpc":"2.0"}
}

And full stack trace:

Error: Invalid JSON RPC response: {"id":202,"jsonrpc":"2.0"}
    at Object.InvalidResponse (/usr/local/lib/node_modules/truffle/build/cli.bundled.js:41483:16)
    at /usr/local/lib/node_modules/truffle/build/cli.bundled.js:330353:36
    at /usr/local/lib/node_modules/truffle/build/cli.bundled.js:326008:9
    at XMLHttpRequest.request.onreadystatechange (/usr/local/lib/node_modules/truffle/build/cli.bundled.js:329052:7)
    at XMLHttpRequestEventTarget.dispatchEvent (/usr/local/lib/node_modules/truffle/build/cli.bundled.js:176427:18)
    at XMLHttpRequest._setReadyState (/usr/local/lib/node_modules/truffle/build/cli.bundled.js:176717:12)
    at XMLHttpRequest._onHttpResponseEnd (/usr/local/lib/node_modules/truffle/build/cli.bundled.js:176872:12)
    at IncomingMessage.<anonymous> (/usr/local/lib/node_modules/truffle/build/cli.bundled.js:176832:24)
    at emitNone (events.js:110:20)
    at IncomingMessage.emit (events.js:207:7)
    at endReadableNT (_stream_readable.js:1047:12)
    at _combinedTickCallback (internal/process/next_tick.js:102:11)
    at process._tickDomainCallback (internal/process/next_tick.js:198:9)

And if you can't reproduce I will try to identify if there's anything extra I'm doing to cause issue

[almindor]

We're getting exactly what nickjm with Metamask only. Everything works on tests with ganache-cli but if we "plug in" metamask and do the same function call (it's a call, not a tx) we get an error-less response as well.

In latest ganache-cli the error is completely undefined. In 1.0.2 it was:

Invalid JSON RPC response: {"id":70,"jsonrpc":"2.0","error":{"code":-32603}}

I wonder if it's something to do with the recent nonce reset on accounts in metamask or something completely unrelated like a bad gas estimate.

[almindor]

Turns out in our case it was just the error reporting change causing issues.

We had a piece of code do a call on every "element" which was expected to fail if such element was not present in a specific state in another contract.

This was expected and the error message was supposed to contain "revert" in that case. Since this changed we bubbled up the original error back (the "erorrless RPC error").

Seems it really is just the case of not reporting the error properly.

[kmturley]

i'm having the same issue on Ganache 1.1.0-beta.0 for Mac owner.transfer(price);

It throws the error:

Error: Invalid JSON RPC response: {"id":55,"jsonrpc":"2.0"}

However when I download the previous release v1.0.2 from here: https://github.com/trufflesuite/ganache/releases

and run the same code I get the message:

Error: VM Exception while processing transaction: revert

[benjamincburns]

Fixed in current develop branch. Will go out as part of a new beta release in the next few days.

[kmturley]

Great, are you able to also change the default version delivered to github, npm and your site to be the official major/minor releases? Beta versions should not be the default, as they are not production ready!

If you go here: http://truffleframework.com/ganache/

The download on a mac links to: https://github.com/trufflesuite/ganache/releases/download/v1.1.0-beta.0/Ganache-1.1.0-beta.0.dmg

On Github the default branch is develop: https://github.com/trufflesuite/ganache-cli

Which should actually be master: https://github.com/trufflesuite/ganache-cli/blob/master/package.json https://github.com/trufflesuite/ganache-cli/blob/develop/package.json

Users should be able to opt-into betas, but not have them by default :)

[benjamincburns]

Thanks for pointing that out, @kmturley.

On the download link, it seems I forgot to tick the "prerelease" box on the "releases" page. I've fixed that, and the download link should be pointing at v1.0.2 now as a result.

On develop vs master, we've made that change intentionally in some (most?) of our repositories so that PRs automatically target the correct branch. If we don't do this, then people often find out they have merge conflicts with the current line of development rather late in their coding process. We do still use master to track stable releases, it just isn't set as the default branch.

[kmturley]

cool, I have some other suggestions for user onboarding/tutorials, will make a separate ticket for those!

[jtakalai]

What's the status of getting error messages from reverts? The commit 78a4572 is from Jan 31...