trufflesuite / ganache

:warning: The Truffle Suite is being sunset. For information on ongoing support, migration options and FAQs, visit the Consensys blog. Thank you for all the support over the years.
https://consensys.io/blog/consensys-announces-the-sunset-of-truffle-and-ganache-and-new-hardhat?utm_source=github&utm_medium=referral&utm_campaign=2023_Sep_truffle-sunset-2023_announcement_
MIT License
2.62k stars 681 forks source link

Ganache v7.0.0: Javascript traces inside revert message #2133

Closed Uxio0 closed 2 years ago

Uxio0 commented 2 years ago

We are using ganache to test Gnosis Safe, and we are trying to update from v6 to v7, and we detected a weird behaviour. Even if --chain.vmErrorsOnRPCResponse false, when having a revert message there are Javascript traces in there that prevent the parsing of the revert error:

{"id":1,"jsonrpc":"2.0","error":{"message":"VM Exception while processing transaction: revert \\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\xef\xbf\xbdu","stack":"RuntimeError: VM Exception while processing transaction: revert \\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\xef\xbf\xbdu\\n    at Blockchain.simulateTransaction (/home/uxio/.config/yarn/global/node_modules/ganache/dist/node/1.js:2:48034)","code":-32000,"name":"RuntimeError","data":{"hash":null,"programCounter":112,"result":"0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000008775","reason":"\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\\u0000\xef\xbf\xbdu","message":"revert"}}}

It's not easily reproducible as it involves deploying the Safe contracts https://github.com/gnosis/gnosis-py/runs/4897023415?check_suite_focus=true#step:10:370

Same behaviour and more easily reproducible when setting a low maxFeePerGas:

{'message': "VM Exception while processing transaction: Transaction's maxFeePerGas (1) is less than the block's baseFeePerGas (7) (vm hf=london -> block -> tx)", 'stack': "RuntimeError: VM Exception while processing transaction: Transaction's maxFeePerGas (1) is less than the block's baseFeePerGas (7) (vm hf=london -> block -> tx)\n    at Miner.<anonymous> (/app/dist/node/1.js:2:118507)\n    at async Miner.<anonymous> (/app/dist/node/1.js:2:116863)\n    at async Miner.<anonymous> (/app/dist/node/1.js:2:115474)\n    at async Miner.mine (/app/dist/node/1.js:2:120062)\n    at async Blockchain.mine (/app/dist/node/1.js:2:36544)\n    at async Promise.all (index 0)\n    at async TransactionPool.emit (/app/dist/node/0.js:2:81959)", 'code': -32000, 'name': 'RuntimeError', 'data': {'hash': '0xd12db7d9c78648b62c1572f9fcbd8197e7fadb910d6703b310a24ae22c7b8a7b', 'programCounter': 0, 'result': '0xd12db7d9c78648b62c1572f9fcbd8197e7fadb910d6703b310a24ae22c7b8a7b', 'reason': None, 'message': "Transaction's maxFeePerGas (1) is less than the block's baseFeePerGas (7) (vm hf=london -> block -> tx)"}}
davidmurdoch commented 2 years ago

Thanks for opening this. We'll look in to a solution for this ASAP.

davidmurdoch commented 2 years ago

My hunch is that this is due to a change in the way Ganache v7 handles eth_call. The change was made in order to align with Geth. The vmErrorsOnRPCResponse setting no longer affects eth_call. This wasn't listed as a breaking change in the release notes or upgrade guide, so i'll go add it there in a bit.

See this issue https://github.com/trufflesuite/ganache/issues/1496 for more why this change was made.

That said, after reviewing the above issue, I think we may need to rethink the data we are return, as we don't currently align with geth here, so you still won't be able to switch your testing infra back and forth between Ganache in geth, which was the main reason for the alignment. Geth returns error: {message: "...", data: <RAW REVERT DATA HEX>} whereas we return error: {message: "...", data: { ...other things..., result: <RAW REVERT DATA HEX> }.

Now there were reasons (mainly legacy reasons) we didn't completely align with Geth, but I'm thinking we should reevaluate those and treat this as a bug, as the intent was to align with Geth. The change we made won't actually help you simplify your testing code at all here, which was really the point!

A fix will likely take some time (more than just a few days) as we'll likely need to fully understand the ramifications of this change within Truffle, and put the proper changes in place there, as well.

Of course, I could be wrong about this hunch and we'll need to investigate further. Though if I am wrong, I think Ganache should still explore the alignment path I described above.

As a temporary workaround you could change:

            if "error" in response_data and "data" in response_data["error"]:
                error_data = response_data["error"]["data"]
            elif "result" in response_data:  # Ganache-cli
                error_data = response_data["result"]

to something like:

            if "error" in response_data and "data" in response_data["error"]:
                if "result" in response_data["error"]["data"]:
                    # TODO: remove this Ganache-specific code once https://github.com/trufflesuite/ganache/issues/2133 is resolved
                    error_data = response_data["error"]["data"]["result"] # Ganache v7.0.0
                else
                    error_data = response_data["error"]["data"]
            elif "error" in response_data:  # Ganache-cli v6
                error_data = response_data["result"]
Uxio0 commented 2 years ago

Hi @davidmurdoch, thanks for the quick answer and for taking the time to go into our code and provide a custom solution! That fixed one of our issues! We will keep working on it as soon as eth_feeHistory is supported

davidmurdoch commented 2 years ago

Here is our eth_FeeHistory issue if you want to follow along: https://github.com/trufflesuite/ganache/issues/1470

It is currently a "Priority 2" issue which will be worked after these "Priority 1" issues are resolved: https://github.com/trufflesuite/ganache/labels/priority1%20%F0%9F%9A%A8

Uxio0 commented 2 years ago

Thank you very much for all the feedback

davidmurdoch commented 2 years ago

@Uxio0 Looks like we'll align fully with geth here, so you can expect 7.0.1 release fixing this issue later this week!

Uxio0 commented 2 years ago

@Uxio0 Looks like we'll align fully with geth here, so you can expect 7.0.1 release fixing this issue later this week!

Awesome, so fast!