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

Is there any way to extract Ethereum error message? #368

Closed
sweetpalma opened this issue Dec 5, 2018 · 9 comments

Comments

@sweetpalma
Copy link

commented Dec 5, 2018

Just a quick question regarding better UX. For example, I have some kind of require in my contract code like that:

function someFunction() public {
  require(someCondition, 'Not registered');
}

When I call that method with ethers.js with a failing precondition, I receive the following error.toString():

Error: VM Exception while processing transaction: revert Not registered.

Is there any way to extract only the precondition error message without any kind of string manipulation, something like that?

Not registered.
@ricmoo

This comment has been minimized.

Copy link
Member

commented Dec 5, 2018

Unfortunately that’s not possible in Ethereum.

The only way to return a result from a state-changing method is to use logs (called events in Solidity), but a revert (which is how require works) also rolls back all logs.

The message you are getting is specifically from Ganache (in non-compliant mode), so in the real blockchain that doesn’t work anyways, even when parsing strings.

:(

@ricmoo ricmoo added the discussion label Dec 5, 2018

@onetom

This comment has been minimized.

Copy link

commented Dec 6, 2018

What you can still do is to eth_call your method first before doing an eth_sendTransaction and try to make sense of the ABI encoded error message returned by those new-style require (note, that assert doesn't support this extra param) as described vaguely at the bottom of https://solidity.readthedocs.io/en/develop/control-structures.html#error-handling-assert-require-revert-and-exceptions

It think it's worth supporting and it would highly speed up debugging.

@ricmoo

This comment has been minimized.

Copy link
Member

commented Dec 6, 2018

A quick note is that eth_call revert reasons are already completely supported:

contract.someMethod(1, 2, 3).then((result) => {
}, (error) => {
    console.log(error);
    // error.reason - The Revert reason; this is what you probably care about. :)
    // Additionally:
    // - error.address - the contract address
    // - error.args - [ BigNumber(1), BigNumber(2), BigNumber(3) ] in this case
    // - error.method - "someMethod()" in this case
    // - error.errorSignature - "Error(string)" (the EIP 838 sighash; supports future custom errors)
    // - error.errorArgs - The arguments passed into the error (more relevant post EIP 838 custom errors)
    // - error.transaction - The call transaction used
});

@onetom That's an interesting idea. I will have to experiment with it a bit. I don't think it makes sense to always use eth_call, since that is an additional call with every transaction that is not necessarily going to return consistent results, but maybe on the error returned from a failed transaction receipt, there can be a checkCall method?

contract.transfer(someAddress, someValue).then((tx) => {
    return tx.wait().then((receipt) => {
        // This is entered if the transaction receipt indicates success
        return true;
    }, (error) => {
        // This is entered if the status of the receipt is failure
        return error.checkCall().then((error) => {
            console.log("Error", error);
            return false;
        });
    }
});

The main issue with consistency, is that things can change on the blockchain between an eth_call and a transaction being mined. Also, a lot of things are not meaningful during an eth_call (e.g. block hash) and the dangers of front-running can crop up.

@kyriediculous

This comment has been minimized.

Copy link

commented Jan 15, 2019

I've been waiting forever for reasoning strings to be added to the return object from RPC.

This is going to do so much for UX/UI !

Logging errors through events wasn't approach to begin with I think, somebody only came up with it out of necessity and not efficiency.
Barely any standardisation and why would someone pay extra gas for a call that returns an error anyway.
At least I can show real errors now returning from contract calls instead of "Oops. Something went wrong" :D .

Forgive me if I'm wrong @ricmoo but a call is never broadcasted to peers so frontrunning would only be problematic when there's a MITM attack between the user and the node, right? I'd say that consistency is indeed the larger issue.

@ricmoo

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

Keep in mind even if the errors are recorded in the JSON-RPC call, they will probably be implemented in Solidity as logs, so there should be zero difference between logging an error using emit and providing an error response; it will just be built into the language and easier to use, but will still cost the exact same gas.

Ideally, you should have some other way of determining what went wrong, for the UX... You could also just return error codes instead of throwing exceptions, if the UX absolutely depends on the internal state.

Indeed, eth_call is not forwarded to other nodes, but that doesn't have any affect on front running. Let's say I want to buy CryptoKitty 1337, and the current price is 1 ether. So, I sign a transaction to buy it, with a gas price of 4 gwei. A front-runner will see on the network a pending transaction for the CK1337, and place their own buy transaction, but with a gas price of 4.0000001 gwei. The miners will mine both of us, but will almost certainly choose the the front-runner's first, so they get the kitty, and I get a failed transaction because the kitty has already been purchased and is not longer for sale.

There are many other sources of consistency issues too, for example if the blockhash, coinbase, or now are used. And if a contract calls into any other contract, anything can happen in those as well.

Turing-Completeness is an amazing tool, but comes with it costs too. :)

@kyriediculous

This comment has been minimized.

Copy link

commented Jan 15, 2019

Hmm interesting. But my gut says they should be in the tx receipt then and they aren't.

I did some looking around at in seems one of the proposals was to return it as return data (uint, uint, string) , so should be in the ABI then?

Can't find it there either, perhaps my truffle and solc needs upgrading. Oh well.

I thought you meant eth_call beforehand would affect frontrunning, we're on the same page! :)

@ricmoo

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

Oh! You are correct, it would absolutely likely be returned as data, not a log, which may make more sense. We would need to solve some sort of method of telling a transaction failed then (since currently there cannot be return data for a revert-ed function.

My preference is that if the return size is congruent to 4 mod 32, it indicates an error (4 byte error selector + ABI data) whereas a success is congruent to 0 mod 32. This way it is backward compatible, completely customizable and has no ambiguities.

Thanks for pointing that out though. :)

@ricmoo

This comment has been minimized.

Copy link
Member

commented Feb 2, 2019

I'm going to close this now, but feel free to re-open, or keep discussing.

In v5, I've added acontract.staticCall.transfer, for example, which allows any call that would otherwise be a transaction to be forced into being a call. That would allow you to manually inspect errors if your situation is likely free of consistency issues. I think that is safer, to force it to be explicit.

I should add a cookbook entry though on this technique suggested above.

Thanks!

@justinjmoses

This comment has been minimized.

Copy link

commented May 20, 2019

It's actually pretty straightforward now to get the revert reason out using ethers-js:
https://gist.github.com/gluk64/fdea559472d957f1138ed93bcbc6f78a#file-reason-js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.