search

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 682 forks source link

Support Solidity panic codes in the RuntimeError class #758

Open axic opened 3 years ago

axic commented 3 years ago

Here revert reasons are processed. This currently only understands the revert strings (which have a signature of Error(string)). It would be nice to also support panic codes (with a signature of Panic(uint256)).

See https://docs.soliditylang.org/en/v0.8.0/control-structures.html#panic-via-assert-and-error-via-require for more information.

I think ganache should return panic codes as a numeric value for the moment.

axic commented 3 years ago

Since 0.8.4 there are also custom errors (with their own selector), fitting the same architecture. Supporting both would be nice.

frangio commented 3 years ago

One of the issues with custom errors and Ganache is that Ganache is completely agnostic to ABIs at the moment, so it won't be able to parse/decode them. I guess it's Web3.js that should parse them, but I'm not sure if it's possible to implement it at that level right now because I don't know if Ganache forwards the full returndata.

frangio commented 2 years ago

Bumping this. Now that Ganache v7 has been released may be a good time to revisit and think how it could be implemented. We are all eager to use custom errors but Ganache is still lacking support.

davidmurdoch commented 2 years ago

Ganache does forward the whole return data as hex on responses for eth_call.

@haltman-at does Truffle decode these? This seems like a Truffle-side thing.

frangio commented 2 years ago

If so, that's great. For Truffle that would be Web3.js, the new version of which is in development and would include support for custom errors.

haltman-at commented 2 years ago

Truffle Contract currently does not decode panic codes or custom errors; I was thinking that would probably wait until the Contract rewrite, although that seems to have gotten rather delayed. But we could add support for panic codes into it if people think that should happen sooner? Custom errors would be more difficult and would likely have to wait, either until the contract rewrite or until this new web3.js version.

Truffle Decoder and Truffle Debugger can handle panic codes and custom errors, of course, but I'm assuming Truffle Contract was what you were talking about. :)

frangio commented 2 years ago

Yes, I was talking about Truffle Contract, specifically about the Error that is thrown when the contract reverts with a custom error.

davidmurdoch commented 2 years ago

@haltman-at Should we move this issue to the truffle repo? I don't think there is anything useful Ganache can do, right? (since we return the raw revert hex string already)

haltman-at commented 2 years ago

There's actually something of an open issue for this already, actually: https://github.com/trufflesuite/truffle/issues/3873. It got marked not very high priority because custom errors are hard and we'll handle them in the rewrite, right? That said, panic codes aren't that hard, and the rewrite has gotten delayed, so it might be worth re-upping that. You want to move discussion over to that?

davidmurdoch commented 2 years ago

Just realized this may be related to some changes plan to make to our eth_call error message strings that would provide additional information over what we give now. @MicaiahReid, I couldn't find an issue for the new error strings we were talking about adding; they might all map to these specific panic codes.

haltman-at commented 2 years ago

FWIW, here's the Solidity documentation of the panic codes, and here's the table of panic codes that the debugger uses for interpreting them. :) (And which Truffle Contract would presumably also use if we added support for that there...)