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

Auto Verify After Deployment #895

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

passabilities
Copy link
Contributor

  • runs verify:verify hardhat script after deployments

@ericglau
Copy link
Member

@passabilities Thanks for suggesting this. This seems like a useful feature to have, however I'm not sure whether this would work well in practice.

My main concerns are regarding latency, both for the plugin and from Etherscan.

First, depending on the scenario, we don't always wait for a deployment to be confirmed but often just return an ethers contract instance which is still pending deployment. For example, after running deployProxy, users can call contract.waitForDeployment() on the returned contract instance to wait for the deployment. For these cases, we would want to avoid having an internal waiting period (which would be needed before running Etherscan verify) and also avoid waiting for the verification itself. Otherwise, the function would take too long to return.

Secondly, Etherscan does not immediately detect bytecode that was deployed to an address. We can see this error message that could be thrown by the hardhat-verify plugin https://github.com/NomicFoundation/hardhat/blob/b06601b9d67bd1f33f5217e215d26eecde6ea86e/packages/hardhat-verify/src/internal/errors.ts#L154, where it suggests to wait perhaps 5 confirmations before running verification. If we run verification immediately after a deployment, this kind of error could possibly occur quite frequently, whereas adding a delay would result in the first issue mentioned above.

Due to these reasons, it could be more reasonable to allow the user to just run verification in their own scripts as needed, as it is currently. Let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants