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?