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

Return values for eth_call are truncated when using revert #941

Closed
Uxio0 opened this issue Jun 29, 2018 · 5 comments · Fixed by #1585
Closed

Return values for eth_call are truncated when using revert #941

Uxio0 opened this issue Jun 29, 2018 · 5 comments · Fixed by #1585

Comments

@Uxio0
Copy link
Contributor

Uxio0 commented Jun 29, 2018

  • Version: 4.4.0
  • Python: 3.6
  • OS: linux

What was wrong?

When calling a contract with eth_call to a function that has a revert(string(abi.encodePacked("Hello"))); I would expect a return value like this:

0x08c379a0                                                         // Function selector for Error(string)
0000000000000000000000000000000000000000000000000000000000000020 // Data offset
000000000000000000000000000000000000000000000000000000000000001a // String length
4e6f7420656e6f7567682045746865722070726f76696465642e000000000000 // String data

As stated in the solidity documentation: http://solidity.readthedocs.io/en/v0.4.24/control-structures.html

Instead, I get the return value truncated:

0x8c379a000000000000000000000000000000000000000000000000000000000

If I see the RPC Call that web3 does in Wireshark I see this as the result:

0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000001df7

How can it be fixed?

Don't truncate the return value

UPDATE

I can just reproduce the issue calling mycontract.functions.myfunction().call(). If I use eth.call I get the non truncated output (as it should be)

@dylanjw
Copy link
Contributor

dylanjw commented Jun 29, 2018

The contract function call() method has no handling for error reasons strings. It fails to detect the errored return value, and attempts to parse the return value according the the function abi. We need to add a way to detect the error return string, and then parse it according the reason string format.

@carver
Copy link
Collaborator

carver commented Jun 29, 2018

We need to add a way to detect the error return string, and then parse it according the reason string format.

Also, probably add a new exception type to raise. I'm not sure how standard this is (or might become) across languages, so maybe a SolidityError.

@dylanjw
Copy link
Contributor

dylanjw commented Jun 29, 2018

It looks like py-evm/eth-tester will also need to add support for the error return string spec.

@karlb
Copy link
Contributor

karlb commented Feb 24, 2020

I'm interested in writing a PR for this. Is there already a test case including a revert that can be used?

@kclowes
Copy link
Collaborator

kclowes commented Feb 24, 2020

@karlb That would be great! I don't think we have any test cases already. Feel free to ping me if you need anything!

@karlb karlb mentioned this issue Feb 24, 2020
5 tasks
wolovim added a commit that referenced this issue Oct 28, 2020
* Parse revert reason when `call` fails

Solidity allows messages in `require` statements which can help a lot
when debugging. This commit parses the revert reasons from failing
`call`s and raises them as `SolidityError` exceptions.

Closes #941.

* Add test for revert interface

* Convert TransactionFailed into SolidityError

In an effort to be backend-agnostic eth-tester changes errors that are
specific to one EVM (like Revert) to a TransactionFailed error. This is
useful when using only eth-tester. But as web3.py also works directly
with other eth clients, we have to abstract over the different revert
behaviors ourselves.

* Check revert reason in test

* fixup! Add test for revert interface

Fix linting errors

* Revert contract working in geth fixture

* Beginnings of Geth integration test

* Minor refactoring, eth-tester tests passing

* Add parity fixture with revert contract

* address linting issues

* fix eth_module test and naming

* contain eth_tester exceptions

* fix lint

* check eth_estimateGas for revert exception

* wip: new fixture + latest geth revert messages

* update geth fixture

* handle eth-tester revert messages

* wip: standardize client errors

* lints

* upgrade parity fixture + handle parity revert

* dedupe constants, more cleanup

* pr review fixes

* more pr review

Co-authored-by: Keri <kclowes@users.noreply.github.com>
Co-authored-by: Marc Garreau <marcdgarreau@gmail.com>
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

Successfully merging a pull request may close this issue.

5 participants