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

Version 0.7.0 no longer propagate contract errors for .calls #222

Closed
josojo opened this issue Jun 20, 2019 · 5 comments
Closed

Version 0.7.0 no longer propagate contract errors for .calls #222

josojo opened this issue Jun 20, 2019 · 5 comments

Comments

@josojo
Copy link

josojo commented Jun 20, 2019

Hi,

Since the new version of 0.7.0, a contract call:

contract.call()

does no longer throws an error, if the contract call failed. It used to be like this and it was a highly appreciated feature. Why was it changed? Was this intentional?

I wrote a minimal example - a fork of the simpleStorage - in order to explain how the unwrap no longer fails. Checkout the code here: https://github.com/josojo/rust-web3/tree/errorDescription

The function call here:
https://github.com/josojo/rust-web3/blob/errorDescription/examples/simple_storage.rs#L37
used to return an error. But it does not, it just returns the tx hash.

@tomusdrw
Copy link
Owner

@josojo I don't think contract.call() did ever return errors from transaction execution - this API only schedules a transaction to be executed and gives you the transaction hash. You can check the result only after it's included in the chain.

Querying the contract should however return errors.

@josojo
Copy link
Author

josojo commented Jun 26, 2019

@tomusdrw
contract.call() was behaving like that. In order to convince you, here is a minimal example:
Minimal example
Check out the readme to see how it can be reproduced.

@tomusdrw
Copy link
Owner

tomusdrw commented Jul 9, 2019

Hi @josojo!
Just found some time to finally debug the issue, and suprisingly the behaviour is indeed different across versions.

So firstly - this only affects ganache - note that eth_sendTransaction will never return contract call errors and you should not expect it to do so when connected to a real node, since the transaction is executed asynchronously.

However ganache seems to invoke transaction right away and return an error when that happens, but at the same time it returns the transaction hash as well inside result!

See the payload:

eth_sendTransaction
   > {
   >   "jsonrpc": "2.0",
   >   "method": "eth_sendTransaction",
   >   "params": [
   >     {
   >       "data": "0x60fe47b1000000000000000000000000000000000000000000000000000000000000002a",
   >       "from": "0xd028d24f16a8893bd078259d413372ac01580769",
   >       "to": "0xec0b9ed45c0357a4539df79ba7cf1259a2cf4add"
   >     }
   >   ],
   >   "id": 2
   > }
 <   {
 <     "id": 2,
 <     "jsonrpc": "2.0",
 <     "result": "0x62d3776be72cc7fa62cad6fe8ed873d9bc7ca2ee576e400d987419a3f21079d5",
 <     "error": {
 <       "message": "VM Exception while processing transaction: revert",
 <       "code": -32000,
 <       "data": {
 <         "0x62d3776be72cc7fa62cad6fe8ed873d9bc7ca2ee576e400d987419a3f21079d5": {
 <           "error": "revert",
 <           "program_counter": 81,
 <           "return": "0x"
 <         },
            ...

Note that such response is an INVALID JSON-RPC response. Since the response is invalid we can't guarantee what will be decoded. And in between 0.6.0 and 0.7.0 the way we decode this invalid payload was actually changed so that we favour result above error.

Actually I've made a PR to jsonrpc library, to reject such response completely: paritytech/jsonrpc#448

So I think the issue is more related to ganache and what happens now is actually more inline with what the actual ethereum node would return - so you need to handle the error further down the line (by requesting a transaction receipt).

@tomusdrw
Copy link
Owner

tomusdrw commented Jul 9, 2019

Closing since I believe it's not really an issue with rust-web3 and current behavior of parsing result does not break compatibility with ganache.

@tomusdrw tomusdrw closed this as completed Jul 9, 2019
@josojo
Copy link
Author

josojo commented Jul 17, 2019

@tomusdrw

Thanks a lot for digging down this rabbit hole! This is indeed a strange behavior from ganache.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants