Error string in require is unexpectedly failing the test cases

[satyamakgec]

I am updating the contracts for solidity version v0.4.23. Cause of updating is to write the error strings in the require and revert for error handling. But somehow when I am adding the error string in some of require expression, It gets failed in test cases.

function changePermission(
        address _delegate,
        address _module,
        bytes32 _perm,
        bool _valid
    )
    public
    withPerm(CHANGE_PERMISSION)
    returns(bool)
    {
        require(delegateDetails[_delegate] != bytes32(0), "Delegate should be pre-registered");
        perms[_module][_delegate][_perm] = _valid;
        emit LogChangePermission(_delegate, _module, _perm, _valid, now);
        return true;
    }

require function in the above function cause a failure because of error string, If i remove that error string then it will run fine.

Any help is very appreciable

[chriseth]

Sorry, but this is very hard to debug without the full context. Can you try to slim down the contract to something you can post in full here?

[chriseth]

Also, did you try to run a debugger and see where exactly it fails?

[kingjerod]

@chriseth I think the require error messages are getting returned as values. I've ran into the same problem. I created a token where you can map a string to token id. Most of the code is standard but here is the important part:

mapping (string => uint256) private uuidToTokenId;  // Maps UUID to token id
mapping (string => bool) private uuidExists;        // Checking if UUID exists

// This one returns a long integer if uuid does not exist
function tokenIdOfUUID(string _uuid) public view returns (uint256) {
    require(uuidExists[_uuid] == true, "UUID does not exist.");
    return uuidToTokenId[_uuid];
}

// This one returns 0 if uuid does not exist
function tokenIdOfUUIDNoError(string _uuid) public view returns (uint256) {
    require(uuidExists[_uuid] == true);
    return uuidToTokenId[_uuid];
}

You can test it live here: https://rinkeby.etherscan.io/address/0xae85aeb0c17abf5df49fe7538ffb275c8804f742#readContract

I have created 0 tokens so all UUIDs should not exist. If I try the first function tokenIdOfUUID that returns an error message, I get back:
uint256 : 3963877391197344453575983046348115674221700746820753546331534351508065746944

If I try tokenIdOfUUIDNoError I get back 0.

[axic]

Are you sure this is a compiler error and not an error in Etherscans VM implementation? Those view functions I imagine Etherscan runs on their own VM (potentially ethereumjs-vm in the browser) and not as a transcation.

They may not check for failed transactions properly and just take the returndata in every case.

[kingjerod]

I had to update my backend code to accommodate for this new behavior. Before web3 would throw an error on revert, now it just returns the value. Here is some code that I just ran with the output from my personal Rinkeby geth node:

  instance = new web3.eth.Contract(
    abi,
    "0xae85aeb0c17abf5df49fe7538ffb275c8804f742",
    {from: account.address, gasPrice: "1000000000", gas: "500000"}
  );

  try {
    const id = await instance.methods.tokenIdOfUUIDNoError("abc").call();
    console.log("Id 1: " +  + JSON.stringify(id));
  } catch (ex) {
    console.log("Error 1: " + ex);
  }

  try {
    const id = await instance.methods.tokenIdOfUUID("abc").call();
    console.log("Id 2: " + JSON.stringify(id));
  } catch (ex) {
    console.log("Error 2: " + ex);
  }

Output:

Error 1: Error: Couldn't decode uint256 from ABI: 0x
Id 2: "3963877391197344453575983046348115674221700746820753546331534351508065746944"

[axic]

That really feels like a limitation in web3.js. Can you try it in Remix?

[kingjerod]

[satyamakgec]

@chriseth debugger is not working with truffle v4.1.7. It always gives some contractName error. I talked with one of the truffle-debugger developer. He said he fix those error but I think you need to release patch version for the truffle, So we can use it.

This is a very random error. It comes to a lot of the places in my contracts repo even when I declare one more variable which has no use still cause the test case failure. One of my colleague comes with the understanding that using the constructor keyword or adding the Error messages cause the high gas usage that may lead the test case failures.

Just I want to confirm with you that this may be the reason, or something else. If yes then I think need to optimize the gas usage because It increases the tx cost too much.

If you still think that you need to look up the contract as well please let me know I will send you the contract.

[kingjerod]

Any update on this? This is still an issue.

[chriseth]

@kingjerod this really looks like an issue in web3.js. Could you please provide a minimal example to reproduce this issue?

[kingjerod]

It is possible, I don't know what library Remix/Etherscan are using. I created this contract for you:

https://rinkeby.etherscan.io/address/0xc431b94051b1ec1a454bc427ddc5ff87fd6a69d1#code

I added two people, with names "test" and "earth". If I try to look up a name that does not exist like "bob" it returns this
"0": "uint256: personId 3963877391197344453575983046348115674221700746820753546331534351508065746944"

If I deploy this contract in Remix on the Javascript VM, it returns the error message properly. Seems to only happen when I deploy the contract to a real network (it also works fine in Truffle tests).

[axic]

Note, that Remix uses a browser-based VM which makes it possible to exactly tell if a transaction resulted in a revert or not. On a real chain, getTransactionReceipt can only tell if it is was a successful or a failing transaction.

The client then needs to figure out if it was a revert (one heuristic is to check if not all gas was used on a failing transaction) and decode the revert data appropriate.

I do not think your client does that and tries to decode the revert output as a regular output data of the contract.

[kingjerod]

That makes sense. Bummer the clients seem slow to adopt the new features.

[axic]

@ChrisChinchilla maybe you want to document this based on my last comment.

[chriseth]

I don't see anything we can do here.