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

Custom errors not picked up by Hardhat Network when using external Hardhat artifact #1618

Closed
PaulRBerg opened this issue Jun 29, 2021 · 32 comments

Comments

@PaulRBerg
Copy link
Contributor

PaulRBerg commented Jun 29, 2021

Description

If I import an artifact from an external package and use Waffle's "deployContract" to deploy it to my local network, and that contract reverts with a custom error, the custom error won't be picked up by Hardhat when running tests.

How to Reproduce

In the terminal:

Then write a test that imports the FintrollerV1 contract and calls the setMaxBonds function. The execution should revert with the OwnableUpgradeable__NotOwner custom error, but it doesn't. It reverts with the following error:

Error: Transaction reverted without a reason string

Environment

  • hardhat@2.4.1
  • @hifi/protocol@1.1.1
@fvictorio
Copy link
Member

(Pasting my answer from discord here)

It's a known limitation: if a contract is not part of your project, you don't have the information needed to parse the custom error.

I don't think there's an easy way to fix that. We want to, at some point, add the capability of using "external" information (e.g., from etherscan), but it's not something we are working on right now

The easy fix is to add that contract to your project, for example using hardhat-dependency-compiler

@PaulRBerg
Copy link
Contributor Author

Oooh, right. I remember reading about this limitation in the release spec:

Keep in mind that Hardhat can only recognize custom errors that are defined within your project. If you use mainnet forking and call some external contract that reverts with a custom error, Hardhat won't be able to parse it, and it will show an unrecognized error instead.

But I forgot about it.

@PaulRBerg PaulRBerg changed the title Bug report: custom errors not picked up by Hardhat Network when using external Hardhat artifact Custom errors not picked up by Hardhat Network when using external Hardhat artifact Jun 29, 2021
@PaulRBerg
Copy link
Contributor Author

I've just bumped into a case where this happens even when the custom errors is defined locally in my project.

I have a custom error defined in contract B, which contract A interacts with via DELEGATECALL. When contract B reverts with that custom error, I am getting this error in the console when running my test:

AssertionError: Expected transaction to be reverted with TargetError, but other exception was thrown: Error: VM Exception while processing transaction: reverted with an unrecognized custom error

The TargetError is the custom error I defined in contract B. Hardhat is not picking it up because contract A is delegate calling, then is checking if the call succeeded or not and bubbles up the revert:

// Delegate call to the target contract.
(bool success, bytes memory returndata) = target.delegatecall{ gas: stipend }(data);
if (success) {
    return returndata;
} else {
    if (returndata.length > 0) {
        assembly {
            let returndata_size := mload(returndata)
            revert(add(32, returndata), returndata_size)
        }
    } else {
        revert();
    }
}

Therefore I think that adding support for "external" custom errors is more important that we initially thought. Though I am aware that this requires a database like 4byte.directory, which is not easy to generate and maintain.

@PaulRBerg
Copy link
Contributor Author

Side note: what if Hardhat included the error selector in the logged error? That way, those of us who know how to handle selectors could use them. It happens that ethers.js has a getSighash method for grabbing the 4byte selector.

@alcuadrado
Copy link
Member

alcuadrado commented Sep 7, 2021

Is this delegatecalling into an external contract? Or is the target one also yours?

PS: I think this is going to be increasingly more and more important as the custom errors start to get used on mainnet. Otherwise interacting with contracts from mainnet would be painful.

@PaulRBerg
Copy link
Contributor Author

The target contract is also mine. It is defined in the same repo and I can see an artifacts generated for it. See this:

https://github.com/paulrberg/prb-proxy/tree/6f2e4c2da37c3289d85218692d5a0a7f3a0ebb8f/contracts/test

I think this is going to be increasingly more and more important as the custom errors start to get used on mainnet.

Yes! Especially true after OpenZeppelin adds support for them.

@fvictorio
Copy link
Member

Opened #1875 based on this issue. I didn't edit this one because I'm not 100% sure that it's exactly the same problem.

@wei3erHase
Copy link

i understand the technical situation, but is there a way of reading those CustomErrors from the interface?

@fvictorio
Copy link
Member

Some decisions about this (although we don't know when we'll be able to implement them):

  • We should have some global repository with all the "known" custom errors in the project.
  • If we can't infer the error, we use this repository in the fallback function to check if its signature matches a known custom error.
  • If it matches one (or more?) of these known custom errors, we try to parse the error data with it. If that succeeds, we assume that the match is real and show an error message with the decoded data (similar to what we do in the error inferrer). If not, we don't do anything and the fallback logic continues.
  • We didn't make a decision about the error message, but I think it should be at least a bit different to the one shown by the error inferrer, so that we can tell them apart if someone reports an issue around this.

@fvictorio
Copy link
Member

i understand the technical situation, but is there a way of reading those CustomErrors from the interface?

What do you mean?

@wei3erHase
Copy link

wei3erHase commented Oct 12, 2021

@fvictorio

Let me rephrase:
The interfaces already has the CustomErrors
We're fetching a contract with an interface and an address
When something happens in that contract, we receive a revert without revert string

Case scenario:
We're looking to make a deployment for developers without showing the source code (without validating in Etherscan) but if we provide only the interface (which has all the CustomErrors) the functionality does not work.

Workaround:
We've made a find and replace:
revert CustomError() >>> revert('CustomError()')

@PatrickAlphaC
Copy link

@PaulRBerg how did you end up testing for the custom errors in hardhat? Do you have an example?

@PaulRBerg
Copy link
Contributor Author

@PatrickAlphaC I ended up importing the said contract which included the custom error I wanted to test for. I did that by manually importing the contract in my code base, but if you'd like to not write a dummy contract you could instead use hardhat-dependency-compiler.

At any rate, this would be much easier to handle if Hardhat managed somehow to decode custom errors from external contracts.

@fvictorio
Copy link
Member

At any rate, this would be much easier to handle if Hardhat managed somehow to decode custom errors from external contracts.

This would be great although really hard to do in the general case, because you only see a bunch of bytes and you need to guess the signature of the error.

Some things we could do are:

  • Have a list of "well-known" custom errors that comes with Hardhat
  • Fetch the code from etherscan, assuming it's verified

But if you are using some obscure contract that is not verified, I think it's just impossible to guess the error.

@PaulRBerg
Copy link
Contributor Author

Have a list of "well-known" custom errors that comes with Hardhat

This would be epic! Something like 4byte.directory but for custom errors.

Fetch the code from etherscan, assuming it's verified

Also cool.

But if you are using some obscure contract that is not verified, I think it's just impossible to guess the error.

That's fine.

@fvictorio
Copy link
Member

Something like 4byte.directory but for custom errors.

You probably know this already, but the same mechanism is used. That is, custom errors are encoded in the same way that function calls are encoded. So 4byte can include custom errors signatures too.

@PaulRBerg
Copy link
Contributor Author

That is, custom errors are encoded in the same way that function calls are encoded

I actually didn't know, I didn't read the low-level spec. Fantastic.

@IAliceBobI
Copy link

IAliceBobI commented Apr 12, 2022

Hi all, I'm living in the 0.6 version of Solidity.
I'm throwing errors like this. Is there a way to get the raw error bytes?

errorData = abi.encodeWithSelector(
            bytes4(keccak256("ABC(address,address)")),
            maker,
            signer
);

assembly {
            revert(add(errorData, 0x20), mload(errorData))
}

@PaulRBerg
Copy link
Contributor Author

PaulRBerg commented Apr 12, 2022

@IAliceBobI custom errors don't exist in v0.6 of Solidity. They have been introduced in v0.8.4.

There's really no good reason to not upgrade to the latest compiler version.

Update: I now see what you wanted to do. Yes, you can replicate the way custom errors are encoded manually, but it's really a chore. If I were you, I would reach out to the developers of the v0.6 libraries you are depending upon, and kindly ask them to upgrade to v0.8.

@IAliceBobI
Copy link

@IAliceBobI custom errors don't exist in v0.6 of Solidity. They have been introduced in v0.8.4.

There's really no good reason to not upgrade to the latest compiler version.

I want to upgrade, but I depend on many 0.6 Solidity audited libraries right now.
I can handle the error if I can get the raw error bytes.

@fvictorio
Copy link
Member

@IAliceBobI maybe #2553 can help you with that when it's released.

@IAliceBobI
Copy link

@IAliceBobI maybe #2553 can help you with that when it's released.

I depend on this branch locally, and it works fine!
Thank you so much!

@kevincheng96
Copy link

I've just bumped into a case where this happens even when the custom errors is defined locally in my project.

I have a custom error defined in contract B, which contract A interacts with via DELEGATECALL. When contract B reverts with that custom error, I am getting this error in the console when running my test:

AssertionError: Expected transaction to be reverted with TargetError, but other exception was thrown: Error: VM Exception while processing transaction: reverted with an unrecognized custom error

@PaulRBerg Curious if you found a fix or workaround for the delegatecall case? I've also been running into this issue recently 🤔

@PaulRBerg
Copy link
Contributor Author

Hi @kevincheng96, I can't remember the exact issue I described above, but I think I finally made it work at least in my testing environment. Take a look at the execute.ts file in the prb-proxy repository.

@kevincheng96
Copy link

Hi @kevincheng96, I can't remember the exact issue I described above, but I think I finally made it work at least in my testing environment. Take a look at the execute.ts file in the prb-proxy repository.

Great, will take a look at it. Thanks for quick response!

@PaulRBerg
Copy link
Contributor Author

Looks like @samczsun newly released signature database could be relevant here?

sig.eth.samczsun.com

@yanikitat
Copy link

@IAliceBobI maybe #2553 can help you with that when it's released.

Hello! In what hardhat version can I find these changes?

@fvictorio
Copy link
Member

@TsigelnikovNikita I don't remember exactly which version of Hardhat added it, but it should be available in the latest versions.

@GeraldHost
Copy link

Any plans to make it so we can pass in selectors?

I run into this problem with running in a fork:

  • The contract I am calling has an artifact
  • connect with typechain Contract__factory.connect(addr, signer)
  • expect call to error with custom error:
    • Instead get this error message reverted with an unrecognized custom error
    • When I inspect the stack trace I can see that the selector is the custom error selector

It would be good If we could just do something like:

.to.be.revertedWith("0xe2e1f17f");

@fvictorio
Copy link
Member

.to.be.revertedWith("0xe2e1f17f");

That's not a bad idea. The name of a custom error can't start with 0, so there wouldn't be any ambiguity. Feel free to open a new issue with that feature request.

@fvictorio
Copy link
Member

The syntax would be .revertedWithCustomError("0x12345678") though

@fvictorio
Copy link
Member

I'm going to close this in favor of #3486, so that we have a single issue tracking this need.

@fvictorio fvictorio closed this as not planned Won't fix, can't repro, duplicate, stale Dec 29, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

9 participants