Improper custom error handling in `expectRevertCustomError` when using `via-ir`

[r0qs]

This line will return null and not an Array object if there is no match for a custom error message (see String.prototype.match() documentation). Thus, it will cause const match = error.match(/some-regex/) on line 20 to attempt to access a member of null and throw a type error.

💻 Environment
OpenZeppelin commit: b425a72
Solidity version: 0.8.19 (0.8.19+commit.7dd6d404.Linux.g++)
Relevant Solidity compiler settings: {viaIR: true, optimizer: {enabled: true}}
Hardhat version: 2.14.1
NodeJS version: v18.16.0

📝 Details

You can see an example of such a case here

In this example the error message is: Returned error: VM Exception while processing transaction: revert with unrecognized return data or custom error and there is nothing inside single quotes, so the current regex /'(.*)'/ will not capture anything.

It seems like Hardhat changes the revert messages of errors or it is not handling custom errors properly. Because this comment suggests that you are expecting an error format that is not the one returned when using via-ir:

// The revert message for custom errors looks like:
// VM Exception while processing transaction:
// reverted with custom error 'InvalidAccountNonce("0x70997970C51812dc3A010C7d01b50e0d17dc79C8", 0)

For instance, this test https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/test/proxy/utils/Initializable.test.js#L131, reverts without via-ir with: Error: VM Exception while processing transaction: reverted with custom error 'AlreadyInitialized()', while enabling via-ir, it reverts with: Error: Returned error: VM Exception while processing transaction: revert with unrecognized return data or custom error

🔢 Code to reproduce bug

git clone https://github.com/OpenZeppelin/openzeppelin-contracts.git
cd openzeppelin-contracts
git reset --hard b425a722409fdf4f3e0ec8f42d686f3c0522af19
npm install
sed -i "s|it(\('can lock contract after initialization'\)|it.only(\1|g" test/proxy/utils/Initializable.test.js
# The options COMPILE_MODE=production and IR=true are required to set the compiler settings as described above in the `Environment` section.
COMPILE_VERSION=0.8.19 COMPILE_MODE=production IR=true npm run test

Honestly, I think Hardhat shouldn't change the revert messages but just return them as they are, since they already have it here as ReturnData, but yeah...

[Amxx]

Thank you @r0qs For raising that.

We can try to find a workaround, but the truth is we should be able to reliably parse the custom error from the revert messages, regardless of the compiler settings. I think its a hardhat/web3.js issue that we are not able to do that consistently.

As you pointed out, when enabling via-ir, we get
Error: Returned error: VM Exception while processing transaction: revert with unrecognized return data or custom error
How are we able to check the correctness of the emitted errors from that ?

Would it work better with ethers.js v6?

[r0qs]

As you pointed out, when enabling via-ir, we get Error: Returned error: VM Exception while processing transaction: revert with unrecognized return data or custom error How are we able to check the correctness of the emitted errors from that ?

I don't know if you will be able to, at least not in the way that Hardhat seems to return them. But I may be wrong, it would be better to discuss with Hardhat how to reliably parse their errors.

Would it work better with ethers.js v6?

It sounds like a viable option, but I didn't check how ethers.js detect or returns the revert messages reported by the vm execution. So, depending on how they do it, you may have the same problem. Also, I'm not sure how much effort it would require from you to refactor that.

[frangio]

Does hardhat-chai-matchers's revertedWithCustomError work with contracts compiled with via-ir?

It's not that Hardhat changes the revert messages. It's that they use heuristics to parse custom errors from returndata, and with optimizations those heuristics can fail.

This is not something we can fix by modifying our test helper.