expectRevert should be able to check for custom errors

[Rav3nPL]

Since 0.8.4 we have use of new and cheap custom errors https://blog.soliditylang.org/2021/04/21/custom-errors/

Using them in contract code, but in test I need to use expectRevert.uspecified to handle them in any way.

Reproduce:

pragma solidity ^0.8.4;

contract Test {
    uint256 public num;
    error CustomErrorMessage();

    function willThrowOnZero(uint256 param) external {
        if (param == 0) revert CustomErrorMessage();
        num = param;
    }
}

const { contract } = require('@openzeppelin/test-environment');

const { expectRevert } = require('@openzeppelin/test-helpers');

const Test = contract.fromArtifact('Test');

describe('Custom test', function () {
    let test;
    before(async function () {
        test = await Test.new();
    })
    describe('Test check', function () {
        it('throws on zero', async function () {
            await expectRevert.unspecified(test.willThrowOnZero('0'))
        })
        it('passes on non-zero', async function () {
            await test.willThrowOnZero('1');
        })
        it('can not catch error name', async function () {
            await expectRevert(test.willThrowOnZero('0'), "CustomErrorMessage")
        })
    })
})

Test running throws:

[Rav3nPL]

Seems like #152 is fixing this?

[frangio]

Thank you for raising this @Rav3nPL.

Are you running against Ganache or Hardhat?

[Rav3nPL]

Using Truffle, OpenZepplien test suite and Chai.
So Ganache.
Not tried on Hardhat.

[Amxx]

I've tried using the code in #152, and I couldn't make it work

When calling a function that throws a custom error, I'm getting

Error: Unrecognized revert error string "revert"

I personally believe we need a system that allows to check custom errors arguments just like what we for events, and we are very far from it :/

[frangio]

@Rav3nPL I had read your issue wrong.

This is a limitation of Ganache. Ganache simply returns revert, so our helpers don't have any way to match against the custom error.

[Rav3nPL]

Not necessarily.
Try:

        it('see promise', async function () {
            try {
                await test.willThrowOnZero('0')
            }
            catch (e) {
                console.log(e)
            }
        })

And you find out that:

  data: {
    '0x1aeaa78cb83cb89a49bc6df096724ca05fd95a2a378b5e6d90eb7475d0e97a6e': { error: 'revert', program_counter: 182, return: '0x50e7caa7' },
    stack: 'RuntimeError: VM Exception while processing transaction: revert\n' +
      '    at Function.RuntimeError.fromResults (node_modules/.pnpm/ganache-core@2.13.2/node_modules/ganache-core/lib/utils/runtimeerror.js:94:13)\n' +
      '    at BlockchainDouble.processBlock (node_modules/.pnpm/ganache-core@2.13.2/node_modules/ganache-core/lib/blockchain_double.js:627:24)\n' +
      '    at processTicksAndRejections (node:internal/process/task_queues:96:5)',
    name: 'RuntimeError'
  },

So looks like hash of revert return code IS known.
We "just" need to know matching 'error'.

[frangio]

No I think that's the transaction hash. The custom error is encoded as a 4-byte selector.

[Rav3nPL]

That's the

 return: '0x50e7caa7'

part of error message

[frangio]

Oh I see... I wasn't able to reproduce it when I tried yesterday because I was using a non-view function that reverts.
What you're pointing out only seems to work for view functions, otherwise the return data is not included.

We may have the error selector but do we have the ABI so we can decode it?

[rsodre]

When supporting custom errors, it's also essential to deal with error messages when the error occurs and we're not expecting it, just like an unexpected revert. Today it prints this, while we could be seeing the actual error:

Error: Returned error: VM Exception while processing transaction: revert -- Reason given: Custom error (could not decode).

This is how I'm testing custom errors...

const { web3 } = require('@openzeppelin/test-environment');
async function _expectRevertCustomError(promise, reason) {
	try {
		await promise;
		expect.fail('Expected promise to throw but it didn\'t');
	} catch (revert) {
		if (reason) {
			// expect(revert.message).to.include(reason);
			const reasonId = web3.utils.keccak256(reason + '()').substr(0, 10);
			expect(JSON.stringify(revert), `Expected custom error ${reason} (${reasonId})`).to.include(reasonId);
		}
	}
};

await _expectRevertCustomError(_instance.someFunction({ from: owner }), 'MyCustomErrorName');

[TomiOhl]

@rsodre I tried your code in agoraxyz/token-xyz-contracts. I have installed ganache 7.3.2 and truffle 5.5.20 globally.
It still doesn't work:

I logged the output of JSON.stringify(revert). This is how it looks like for me:

{
  "data": {
    "hash": null,
    "programCounter": 568,
    "result": "0x1bbcd9e50000000000000000000000000000000000000000000000000000000062ffbf4e0000000000000000000000000000000000000000000000000000000062ffbed6",
    "reason": null,
    "message": "revert"
  },
  "reason": "Custom error (could not decode)",
  "hijackedStack": "Error: Returned error: VM Exception while processing transaction: revert -- Reason given: Custom error (could not decode).\n    at Object.ErrorResponse (/home/tomi_ohl/.nvm/versions/node/v16.15.1/lib/node_modules/truffle/build/webpack:/node_modules/web3-core-helpers/lib/errors.js:28:1)\n    at /home/tomi_ohl/.nvm/versions/node/v16.15.1/lib/node_modules/truffle/build/webpack:/node_modules/web3-core-requestmanager/lib/index.js:300:1\n    at /home/tomi_ohl/.nvm/versions/node/v16.15.1/lib/node_modules/truffle/build/webpack:/packages/provider/wrapper.js:119:1\n    at XMLHttpRequest.request.onreadystatechange (/home/tomi_ohl/.nvm/versions/node/v16.15.1/lib/node_modules/truffle/build/webpack:/node_modules/web3-providers-http/lib/index.js:98:1)\n    at XMLHttpRequestEventTarget.dispatchEvent (/home/tomi_ohl/.nvm/versions/node/v16.15.1/lib/node_modules/truffle/build/webpack:/node_modules/xhr2-cookies/dist/xml-http-request-event-target.js:34:1)\n    at XMLHttpRequest.exports.modules.996763.XMLHttpRequest._setReadyState (/home/tomi_ohl/.nvm/versions/node/v16.15.1/lib/node_modules/truffle/build/webpack:/node_modules/xhr2-cookies/dist/xml-http-request.js:208:1)\n    at XMLHttpRequest.exports.modules.996763.XMLHttpRequest._onHttpResponseEnd (/home/tomi_ohl/.nvm/versions/node/v16.15.1/lib/node_modules/truffle/build/webpack:/node_modules/xhr2-cookies/dist/xml-http-request.js:318:1)\n    at IncomingMessage.<anonymous> (/home/tomi_ohl/.nvm/versions/node/v16.15.1/lib/node_modules/truffle/build/webpack:/node_modules/xhr2-cookies/dist/xml-http-request.js:289:48)\n    at IncomingMessage.emit (node:events:539:35)\n    at endReadableNT (node:internal/streams/readable:1345:12)\n    at processTicksAndRejections (node:internal/process/task_queues:83:21)"
}

As you see it doesn't include the reasonId (0x9856227d in my case).
What else do you do differently?

[rsodre]

my receipt is very different than yours, and I use ganache-ui, which is very outdated, from before custom errors were added to Solidity.
I had to add reason + '()' to generate the ID I found on my message, what if you remove it?

[TomiOhl]

Nope, but thanks, your question helped!
I was testing with an error that has parameters:

error DistributionEnded(uint256 current, uint256 end);

May I guess none of your custom errors have parameters?
Custom errors (just like functions) are encoded in Solidity like this: func/error name + ( + parameter types + )
So, to encode an error with no parameters, you have to append only the opening and closing bracket.
To encode the error above, I had to add the types, too, like this: DistributionEnded(uint256,uint256)
Now the code works like charm!
ReasonId of the above: 0x1bbcd9e5
The result from the revert: 0x1bbcd9e5000000000000000000000000000000000000000000000000000000006300e090000000000000000000000000000000000000000000000000000000006300e08f
So, apparently I could decode and test the parameters, too.
Upgraded to truffle 5.5.27 and ganache 7.4.1 in the meantime, and yes, the above works with the latest versions, too.

@frangio I tested the above with non-view functions. We have the selector and the params. The abi could be fetched from the contract artifact that truffle generates, couldn't it? Alternatively, the user could specify the parameter types in a way. I feel like testing for custom errors should be possible with truffle either way.

[rsodre]

Correct, my error had no arguments.
Better to send the full function selector to the test funciton.

[frangio]

The abi could be fetched from the contract artifact that truffle generates, couldn't it?

This library is not who should be doing that. The error needs to be decoded at the level of web3.js or Truffle.

[TomiOhl]

Well, alright. Anyways, I created a separate library for this: https://www.npmjs.com/package/custom-error-test-helper
It's based on rsodre's code, but supports decoding errors with parameters, too.

[advra]

@TomiOhl I am trying to use your library with hardhat like so:

const {
    balance,
    ether,
    expectEvent, 
    expectRevert,
    BN, 
} = require('@openzeppelin/test-helpers');
const { expectRevertCustomError } = require("custom-error-test-helper");
const Marketplace = artifacts.require("Marketplace");

describe("Marketplace contract", function () {
  let marketplace;
  beforeEach(async function () {
      marketplace = await Marketplace.new();
  });
    it("Should validate listed 721 errors", async function () {
        await expectRevertCustomError(Marketplace, marketplace.listNFT(marketplace.address, BigNumber.from("1"), BigNumber.from("1"), BigNumber.from(TOMORROW), BigNumber.from(IN_FIVE_DAYS), 
        { from: TOKEN_OWNER, value: listingFee}), "UnsupportedNftInterface");
    });
});

I get the following error. Do you know what is causing this and how to fix the error?

 1) Marketplace contract
       Validate Listings
         Should validate listed 721 errors:
     TypeError: Cannot read properties of undefined (reading 'result')
      at expectRevertCustomError (node_modules/custom-error-test-helper/build/index.js:17:88)
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at runNextTicks (node:internal/process/task_queues:64:3)
      at listOnTimeout (node:internal/timers:533:9)
      at processTimers (node:internal/timers:507:7)
      at async Context.<anonymous> (test/Marketplace.js:374:9)

[TomiOhl]

Note to others: the above question was answered in TomiOhl/custom-error-test-helper#1