Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

supportRevertedWithCustomError doesn't check if revert happened from the same contract address #6148

Closed
novaknole opened this issue Jan 16, 2025 · 1 comment · Fixed by #6156
Assignees

Comments

@novaknole
Copy link

novaknole commented Jan 16, 2025

Version of Hardhat

2.22.15

Version of hardhat chai matchers

@nomicfoundation/[email protected]

What happened?

Assume I have the following:

const multisig = Multisig__factory.connect(multigAddress, deployer); // Note that 
const blaxblux = Multisig__factory.connect("somerandomAddress", deployer); // Note that 

await expect(
  multisig.doSomething()
)
  .to.be.revertedWithCustomError(blaxblux, 'MinApprovalsOutOfBounds')
  .withArgs(0, 1);

The above will succeed even though the error is emitted from multisig instead of blaxblux. After looking the src code here, I realize that you don't check if the contract addresses match. The code comes to this but nowhere, you assert if the contract address from which revert happened matches the expected one.

What's the rationale for this ? this seems dangerous as tests could silently succeed even though they shouldn't.

Minimal reproduction steps

I didn't want to yet produce the example as src code I provided for hardhat-chai-matchers clearly doesn't have what I am asking. If necessary, I will provide the example code.

Search terms

No response

@galargh
Copy link
Member

galargh commented Jan 17, 2025

Hi! Thank you for raising the issue 🙏 I can certainly see how the behaviour of revertedWithCustomError can be misleading.

Currently, we only use the contract information passed to this function to retrieve the full signature of the expected error based on its' name. Thanks to this, the users don't have to provide full signatures on input (like customError(uint256, uint256) for example).

In order to assert the origin of the raised error, we would have to trace the entire transaction which would be complex and time consuming. We'll definitely explore the opportunities to do so in v3 in the future.

For now, I have created a PR in which I tried to provide a little bit more context on the matter in our documentation - #6156 - I hope that helps 😄

@github-project-automation github-project-automation bot moved this from Backlog to Done in Hardhat Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants