Incorrect handling of revert with a message

[barakman]

Expected behavior

Function call should revert

Actual behavior

Function call completes successfully and returns an impossible value

Steps to reproduce the behavior

I have deployed the following contract to Ropsten, and verified its source-code on Etherscan:

pragma solidity 0.4.26;

contract MyContract {
    bool public x;
    function func() view public returns (bool) {
        require(x);
        return false;
    }
}

It is at address 0x23335ED6E550726D0bA881a3D1f5b5B1eF411A97.

I then deployed and verified an identical contract with one difference - require(x, "error");.
It is at address 0xc8A9f997098c81378A000bBa293567b20645EFD9.

Note that in both cases, calling function func should revert.

Here is a testing script (you'll need an Infura Project-ID):

const Web3 = require("web3");

const ADDR = "0x23335ED6E550726D0bA881a3D1f5b5B1eF411A97";

const ABI = [{
    "constant":true,
    "inputs":[],
    "name":"func",
    "outputs":[{"name":"","type":"bool"}],
    "payable":false,
    "stateMutability":"view",
    "type":"function"
}];

async function test() {
    const web3 = new Web3("https://ropsten.infura.io/v3/<Project-ID>");
    const contract = new web3.eth.Contract(ABI, ADDR);
    try {
        const retval = await contract.methods.func().call();
        console.log(retval);
    }
    catch (error) {
        console.log(error.message);
    }
}

test();

As expected, the result of running the above is an exception with the following error-message:

Returned values aren't valid, did it run Out of Gas?

As I've mentioned here in the past, you probably want to refine this message, but I've tested the above with web3.js v1.2.1, so perhaps you already have. In either case, that's not the main issue here.

The main issue is this:

When changing the value of ADDR in the script above from the address of the first contract to the address of the second contract, the (extremely odd) result is:

1. The function-call completes successfully
2. The returned value of it is true

And obviously, neither one of these aspects by itself could not have possibly taken place.

My analysis is that this is a bug in either Web3 or Infura.

But I tested the second contract also with ganache-cli (after deploying that contract on the local network of course), and I got:

Returned error: VM Exception while processing transaction: revert error

Which makes me more inclined to the "Infura option" (since web3.js seems to do the job well when using a different node).

Nevertheless, I am opening this issue here with a hope that you will look into it.

I have a wild speculation that while fetching the return-value of the function-call from the stack, you retrieve the first character of the message string ('e'), or perhaps the length of the message string, which you then cast to true. Perhaps the appearance of that string has "pushed" the REVERT opcode (0xfd if I remember correctly) deeper into the stack or something like that?

Thanks :)

Versions

Operating System: Windows 10:
node version: 10.16.0
npm version: 6.9.0
web3.js version: 1.2.1
ganache-cli version: 6.7.0

[barakman]

UPDATE:

I've deployed a pair of slightly different contracts, where function func takes uint x as input and returns x > 0 as output.

These two contracts are deployed at:

1. Address 0xD4D8bB5A6Eba78844fB6345dce35725Ec3E62ef8
2. Address 0x13566A62a0Aa3D4BE5aaE979a47cB2F69048977B

As before, we have require(ok); in the 1st contract and require(ok, "error"); in the 2nd contract.

This time, you can reproduce the problem also on Etherscan's read-contract tab for contract #2.

Pass 0 to the function, and you'll see that the result is true.

I doubt that Etherscan uses Infura as its Web3 provider, so I am once again inclined to think that the problem is in web3.js.

That said, I do understand that Infura is not the actual node type, which is typically either Geth or Parity, so perhaps the problem is in the type or types of nodes used by both Infura and Etherscan.

[Kapn]

I am facing a similar issue as well. The below function doesn't raise an error when passing x=0 and returns true.

function exists(uint8 x) public view returns (bool) {
  require(x > 0, "invalid x");
  return false;
}

[cgewecke]

@barakman @Kapn Thanks for raising this.

Revert with reason returns an encoded reason string as data. I'm unsure if using it in Solidity view methods is unproblematic - would like to hear your opinion though. Another approach would be to have defined null values for your view methods which match the function's return data type and document them as such.

If you upgrade to 1.2.6 and set the web3.eth.handleRevert property you should see .calls with failing requires throw and you'll also have access to the reason string.

The implementation details are in #3248 and there are usage examples in the unit tests here.

[barakman]

@cgewecke:
It is non-conventional for sure, and even seems senseless to a certain extent, i.e., what's the point in reverting a (non-transaction) call?
Nevertheless, nowhere in the official documentation of the language is this scenario prohibited.
Does web3 access the call-stack in order to retrieve the returned value?
Or how else does it obtain the return value back from the provider (i.e., the node)?
Thanks

[cgewecke]

@barakman

The node just returns data that has a special signature and form for revert reasons. So it is detectable and Web3 1.2.6 (optionally) implements this. Am pretty sure Ethers does as well.

On the other hand, as a contract developer you might worry that there aren't strong assurances the revert return data will be interpreted correctly for view methods since it's not specified by their ABI.
Am not completely sure what the norms are about this tbh.

[barakman]

@cgewecke:
That's a very interesting notion, so I decided to test it.

I deployed the following contract to Ropsten address 0x2A15AF7156d10F891Bc18284AC68fEAf385dFa5D:

pragma solidity 0.4.26;

contract MyContract {
    function func(uint x) view public returns (bool);
}

contract MyContractUser {
    bool public ok;
    MyContract public c;

    constructor(MyContract myContract) public {
        c = myContract;
    }

    function set(uint x) public {
        ok = c.func(x);
    }
}

I then executed a MyContractUser.set transaction using Web3.js and Infura:

const Web3 = require("web3");

const ADDR = "0x2A15AF7156d10F891Bc18284AC68fEAf385dFa5D";

const ABI = [
    {"constant":true,"inputs":[],"name":"ok","outputs":[{"name":"","type":"bool"}],"payable":false,"stateMutability":"view","type":"function"},
    {"constant":false,"inputs":[{"name":"x","type":"uint256"}],"name":"set","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"}
];

async function send(web3, account, transaction) {
    const options = {
        to      : transaction._parent._address,
        data    : transaction.encodeABI(),
        gas     : 200000, // because await transaction.estimateGas({from: account.address}) reverts
        gasPrice: "3e9", // 3 gwei
    };
    const signed  = await web3.eth.accounts.signTransaction(options, account.privateKey);
    const receipt = await web3.eth.sendSignedTransaction(signed.rawTransaction);
    return receipt;
}

async function test() {
    const web3 = new Web3("https://ropsten.infura.io/v3/<Project-ID>");
    const contract = new web3.eth.Contract(ABI, ADDR);
    const account = web3.eth.accounts.privateKeyToAccount("<Private-Key>");
    console.log("ok =", await contract.methods.ok().call());
    try {
        const receipt = await send(web3, account, contract.methods.set(0));
        console.log(JSON.stringify(receipt, null, 4));
    }
    catch (error) {
        console.log(error.message);
    }
    console.log("ok =", await contract.methods.ok().call());
}

test();

The result implies that the transaction reverts as expected:

ok = false
Transaction has been reverted by the EVM:
{
  ...
  "status": false,
  "to": "0x2a15af7156d10f891bc18284ac68feaf385dfa5d",
  "transactionHash": "0x9334707db23d193c23c8c59a284a1198f0e2a4760e250079ebbcb9456da32869",
  "transactionIndex": 17
}
ok = false

I believe that this rules out your speculation of an incorrect value possibly returned when calling the "dubious" function on-chain (more specifically, from a non-constant function).

Thanks

[cgewecke]

@barakman Going to close because latest version supports revert with reason as an option. Please just ping if you feel like this should stay open and concrete steps to change default behavior should be taken here.

[barakman]

@cgewecke:
This issue persists on Etherscan.
Do you think that this is because they are not setting web3.eth.handleRevert = true;?

[chriseth]

@cgewecke I'm a bit confused by this issue. Whether or not a smart contract is executed via eth_call or eth_sendTransaction is not detectable from inside the smart contract. Both view and even pure functions can revert, because at the EVM level, any call can cause a failure.

The node just returns data that has a special signature and form

This worries me that web3.js might detect failure from the return data and not from a special flag returned by the node. Does the node not return whether or not the call was successful or not?

[cgewecke]

@chriseth @barakman.

This looks client dependent and things are in flux. Until geth 1.9.14, executing eth_call on a method which reverts with reason or returns uint would get these responses (for example):

Revert

 {
 "jsonrpc": "2.0",
 "id": 1000,
 "result": "0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001a7468697320697320612072657665727420726561736f6e2e2e2e000000000000"
}

Success

{
 "jsonrpc": "2.0",
 "id": 1001,
 "result": "0x0000000000000000000000000000000000000000000000000000000000000001"
}

In geth 1.9.14, the revert case returns

 {
 "jsonrpc": "2.0",
 "id": 1000,
 "result": "0x" // Web3 throws by default for this...
}

And there is an open PR at geth 21083 to make the error explicit.

The issue is reported and discussed in geth 19027

There's a reproduction case for this at: https://github.com/cgewecke/geth-revert-reason-repro

[barakman]

@cgewecke : Thank you for your quick response.

1. Does this mean that even v1.2.6 users are expected to see this (i.e., setting the new property web3.eth.handleRevert is not going to resolve the problem for them)?

2. Does it also explain why I've managed to reproduce this problem on Etherscan, as well as directly via a public Infura node, but not on Ganache?

[cgewecke]

@barakman I believe the following holds:

handleRevert	geth <= 1.9.13	geth 1.9.14
true	revert error + reason	revert error
false	"value"	abi encoding error

So it will depend on what nodes the remote services are running. Parity has always reported this case as an error fwiw... see geth 19027

(Related: #3520)

[barakman]

@cgewecke:

In that case, I am rather puzzled by how come it works correctly when using:

• Web3.js 0.20.6 (indirectly via Truffle 4.1.6)
• Ganache-cli 6.7.0

And by correctly I mean, not only does it revert, but it also shows the revert-message.
Perhaps Truffle is somehow in charge of that, but I don't quite see how...

[cgewecke]

@barakman

Yes, ganache-cli has it's own implementation.

If you run with this option it may emulate Geth more closely.

--noVMErrorsOnRPCResponse: 

Do not transmit transaction failures as RPC errors. 
Enable this flag for error reporting behaviour which is compatible with other clients 
such as geth and Parity

[barakman]

@cgewecke:

And with regards to:

@barakman I believe the following holds:

handleRevert geth <= 1.9.13 geth 1.9.14
true revert error + reason revert error
false "value" abi encoding error
So it will depend on what nodes the remote services are running. Parity has always reported this case as an error fwiw... see geth 19027

(Related: #3520)

Does that imply that my initial analysis from the top of this thread ("My analysis is that this is a bug in either Web3 or Infura") is more or less accurate, and this is indeed a problem which must be resolved in both places?

Thanks

BTW, as per your noVMErrorsOnRPCResponse suggestion, the function execution has completed successfully (even though it should have reverted), and the returned-value is false.
So it's kind of a mixed behavior of what we've observed so far (i.e., not reverting and returning true).

[cgewecke]

@barakman

Yes, it's a little open to interpretation. The client (or Infura) should error and it looks like that's being addressed.

We're waiting for the additional changes in geth 21083 to finalize handling at Web3.

[albertov19]

Hi guys, I'm too having a similar problem, it is described in: https://ethereum.stackexchange.com/questions/83714/contract-not-returning-error-to-react-interface?noredirect=1#comment104346_83714

I'm too using web3 and Infura. I have a require option and instead of reverting the call or returning an error I get an impossible number return. I say impossible because the return should be the amount of ether a certain address has on the contract and the waiting time it need to pass to be able to take it out ( a timelock contract). But if the person has no account in the contract it returns a huge weird number for both variables.

Guess is a similar error then.

[cgewecke]

Merged a fix for this on Web3's side with #3571. Will be available in 1.2.10 (might be a little bit)

[AdamMiltonBarker]

I am facing no reason issue, v 1.9.20, no reason message.