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

Error "Returned values aren't valid, did it run Out of Gas" not fixed in v1.2.1 #3134

Closed
barakman opened this issue Oct 16, 2019 · 16 comments
Closed
Assignees
Labels
1.x 1.0 related issues Bug Addressing a bug

Comments

@barakman
Copy link
Contributor

Description

This error is reported in details at issue #1916.

Users mention the fact that it dates back as far as v1.0.0-beta.35 or v1.0.0-beta.36.

I know for a fact that it doesn't occur at v1.0.0-beta.34.

It was supposedly fixed at PR #2608 for v2.0.0-alpha.1 and v1.0.0-beta.52.

Unfortunately, it doesn't seem to be fixed on v1.2.1.

Expected behavior

Calling a method which is viable in the ABI but not in the bytecode, should result with an error-message starting with Couldn't decode.

For example: Couldn't decode uint16 from ABI: 0x.

AFAIK, this error is routed all the way from the provider itself (rather than being generated by the web3 client code).

Actual behavior

Calling a method which is viable in the ABI but not in the bytecode, results with an error-message Returned values aren't valid, did it run Out of Gas?, which is implemented in function ABICoder.prototype.decodeParameters (as part of the web3 client code).

Steps to reproduce the behavior

Install either web3 1.0.0-beta.34 or web3 1.2.1, and run the following script via NodeJS:

const Web3 = require("web3");

async function test() {
    const web3 = new Web3("https://mainnet.infura.io");
    const abi = JSON.parse('[{"constant":true,"inputs":[],"name":"token","outputs":[{"name":"","type":"address"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[],"name":"connectorTokenCount","outputs":[{"name":"","type":"uint16"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[{"name":"_index","type":"uint256"}],"name":"connectorTokens","outputs":[{"name":"","type":"address"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[],"name":"reserveTokenCount","outputs":[{"name":"","type":"uint16"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":true,"inputs":[{"name":"_index","type":"uint256"}],"name":"reserveTokens","outputs":[{"name":"","type":"address"}],"payable":false,"stateMutability":"view","type":"function"}]');
    const address = "0x5894110995b8c8401bd38262ba0c8ee41d4e4658";
    const contract = new web3.eth.Contract(abi, address);
    try {
        await contract.methods.connectorTokenCount().call();
    }
    catch (error) {
        console.log(error.message);
    }
}

test();

With web3 1.0.0-beta.34, the error message is Couldn't decode uint16 from ABI: 0x.

With web3 1.2.1, the error message is Returned values aren't valid, did it run Out of Gas?.

Again, I think that the former is generated on the Web3 provider side (i.e., by the node), while the latter is generated by the Web3 client (i.e., in web3.js code).

Hence I believe that the former is desirable while the latter isn't.

Thanks

Versions

@nivida nivida added 1.x 1.0 related issues Bug Addressing a bug labels Oct 16, 2019
@cgewecke cgewecke self-assigned this Oct 25, 2019
@cgewecke
Copy link
Collaborator

@barakman Hi.

The JSON-RPC hex encoding section describes a set of valid and invalid quantity values.

If you:

  • construct a contract object with an ABI which does not match the bytecode,
  • call an ABI method which specifies a uint256 return value

The return values from the client are:

Client Response
geth (stable 1.9) ""
ganache-cli (6.7.0) "0x"

Those trigger the error because they don't meet the quantity spec.

The message wording makes some sense as a design decision because the return value is "invalid". Additionally the ABI's return values description (e.g uint16) is arbitrarily complex. Does displaying all those types make the underlying problem clearer?

(There's no message from the node, unfortunately.)

I was not able to reproduce a case where under-funding a call was handled by this error logic though, so I'm not sure the gas part makes sense. But maybe there is one....

@barakman What is your view of this?

@barakman
Copy link
Contributor Author

barakman commented Oct 25, 2019

@cgewecke : Hi, thank you for your response.

Here are my main points:

First, with regards to There's no message from the node, unfortunately - actually, there is.

Searching my local node_modules folder, in file ganache-core.node.cli.js, I see:

if(!t&&!e.rawValue)throw new Error("Couldn't decode "+name+" from ABI: 0x"+e.rawValue);

Second, with regards to The message wording makes some sense as a design decision because the return value is "invalid" - I can't argue about design decisions because I did not design this system.

But in terms of user-perspective, I believe that the message makes no sense at all and is even misleading, as this is NOT a gas issue.

If anything, it is the previous message which makes sense because the error is clearly the result of a failure to decode the ABI.

To prove this to you, I will refer once again to the Steps to reproduce the behavior part in my post:

On mainnet, the contract at address 0x5894110995b8c8401bd38262ba0c8ee41d4e4658 does not implement function connectorTokenCount() public view returns (uint16).
Nevertheless, we create a new web3.eth.Contract with an ABI object which declares this function.
We then attempt to call this function, which of course throws an exception.

This is clearly:

  1. Not a gas issue
  2. An ABI decoding issue

Which is why I believe that the error message on web3 1.0.0-beta.34 is correct and the error message on web3 1.2.1 is wrong.

In addition to that, at least according to my own investigation, the Couldn't decode uint16 from ABI: 0x comes directly from the provider (Ganache), while the Returned values aren't valid, did it run Out of Gas? comes from the client (Web3 code). That by itself is another reason IMO to maintain the previous message.

Thanks :)

Update: for all it matters, I am using "ganache-cli": "6.5.1".

Update #2: perhaps the Couldn't decode uint16 from ABI: 0x error message comes from the client also in the case of web3 1.0.0-beta.34; I've found it in node_modules/web3-eth-abi/src/formatters.js:

throw new Error('Couldn\'t decode '+ name +' from ABI: 0x'+ param.rawValue);

(I missed it earlier due to the extra backslash).

So I suppose that it could be coming either from the provider or from the client.
Other than that, my previous argument stands as is (the error is clearly the result of a failure to decode the ABI rather than a gas issue).

Thanks again, and apologies for the confusion.

@cgewecke
Copy link
Collaborator

@barakman Have opened #3161 as possible middle-ground solution. Please feel free to advise there.

I suppose that it could be coming either from the provider or from the client.

Yes I believe the error handling logic in the ganache pack is from an older version of Web3 being included there as a dev dependency, caught up in the bundle because they're using npm shrinkwrap. So it points back here...

@nivida nivida mentioned this issue Nov 14, 2019
10 tasks
@cgewecke
Copy link
Collaborator

Addressed by #3161, closing.

@manuel114
Copy link

I'm getting this error when trying to retrieve a value from a mapping(byte32 => string) (using the default get function) from a CircleCI test. All tests work on local and in Remix.

I've checked and the contract address and contract instance are correct, any clue as to why this might be happening?

@barakman
Copy link
Contributor Author

barakman commented Dec 2, 2019

@manuel114 : What is the contract address, what is the function name and what network are you on?

@manuel114
Copy link

manuel114 commented Dec 2, 2019

@barakman

The contract address is 0x9d745FfDb0520242Edb2e5ccCB2b76641A6db145
The function name is registry (its a public mapping) and the network is Ropsten

The mapping key I've been using for the call is 0xdb25dfeb9fc9ced411d0f50165943d3d62d75cc9ba2dac14a2ef9ba950e40df5

@barakman
Copy link
Contributor Author

barakman commented Dec 2, 2019

@manuel114:
I call it from the following node script and I get 0.7554554072934947:

const Web3 = require("web3");

async function rpc(func) {
    while (true) {
        try {
            return await func.call();
        }
        catch (error) {
            if (!error.message.startsWith("Invalid JSON RPC response"))
                throw error;
        }
    }
}

async function run() {
    const web3 = new Web3("https://ropsten.infura.io");
    const abi = [{"constant":true,"inputs":[{"name":"","type":"bytes32"}],"name":"registry","outputs":[{"name":"","type":"string"}],"payable":false,"stateMutability":"view","type":"function"}];
    const contract = new web3.eth.Contract(abi, "0x9d745FfDb0520242Edb2e5ccCB2b76641A6db145");
    const result = await rpc(contract.methods.registry("0xdb25dfeb9fc9ced411d0f50165943d3d62d75cc9ba2dac14a2ef9ba950e40df5"));
    console.log(result);
}

run();

For all it matters, I am using web3.js v1.2.1.

@manuel114
Copy link

Hey @barakman,

Thanks for having a look at it! The weirdest thing about it is that I only get the error when running the code in CircleCi.

I've tried the same test on both local and remix and it always works fine, but using the same code in CircleCi seems to break it.

@DmitryPogodaev
Copy link

DmitryPogodaev commented Dec 18, 2019

@barakman hi!
Can you please give and advise?

I'm wondering, why I'm getting this error when making call of getConvertibleTokens() on 0x85e27A5718382F32238497e78b4A40DD778ab847 contract? I'm using ABI from
https://github.com/bancorprotocol/contracts/blob/master/solidity/build/BancorConverterRegistry.abi.
Meanwhile, getConvertibleToken(0) works just fine.

Web3 1.2.2

@barakman
Copy link
Contributor Author

@DmitryPogodaev:
Funny, I'm the one who wrote this contract just a few days ago!
The abi is correct, and so is the address, so I don't see why you should be getting this error.
I'll give it a try...

@barakman
Copy link
Contributor Author

barakman commented Dec 18, 2019

@DmitryPogodaev:

Works fine for me:

const fs   = require("fs");
const Web3 = require("web3");

const NODE_ADDRESS     = "https://mainnet.infura.io";
const REGISTRY_ADDRESS = "0x85e27A5718382F32238497e78b4A40DD778ab847";
const REGISTRY_ABI     = JSON.parse(fs.readFileSync("BancorConverterRegistry.abi"));

async function run() {
    const web3     = new Web3(NODE_ADDRESS);
    const registry = new web3.eth.Contract(REGISTRY_ABI, REGISTRY_ADDRESS);
    const tokens   = await registry.methods.getConvertibleTokens().call();
    console.log(tokens);

    if (web3.currentProvider.constructor.name == "WebsocketProvider")
        web3.currentProvider.connection.close();
}

run();

I'm on web3.js v1.2.1, so maybe the problem is in web3.js v1.2.2.

Any chance you're using 0x85e27A5718382F32238497e78b4A40DD778ab847 not as a string?

@DmitryPogodaev
Copy link

@barakman,
Thanks for your response!
Yea, I know that you are the author, that's why I decided to ask :)

You are right. I've tried infura node and got results.
So seems that trouble in my light-client node.

Parity returning this "out of gas" error.
Geth returning timeout (5s) errors and after some count of retries it's getting a result.

@barakman
Copy link
Contributor Author

barakman commented Dec 18, 2019

@DmitryPogodaev:
My first thought would be that either your node or your client is unable to handle that amount of data.
The "out of gas" error is obviously misleading, since there's no transaction here (hence no gas required).
We've introduced getConvertibleTokens() just as an optimized alternative for the combination of getConvertibleTokenCount() and getConvertibleToken(uint _index) (i.e., a single web3 call instead of multiple web3 calls).
Of course, the more convertible tokens we have in the system, the higher are the chances that a client would throw an exception when attempting to fetch all of them "in a single shot".
May I suggest that you first call getConvertibleTokenCount, then - according to a predefined threshold - decide whether to call getConvertibleTokens or getConvertibleToken?

@DmitryPogodaev
Copy link

@barakman,
Thank for your advice!

I was trying to make less calls, but if this won't work, I will try to fetch it in series.
Thanks, and have a good day!

@barakman
Copy link
Contributor Author

@DmitryPogodaev:
You're welcome. We also have a Bancor Developers telegram group, where you can ask me directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues Bug Addressing a bug
Projects
None yet
Development

No branches or pull requests

5 participants