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

Support multiple api keys #2130

Merged
merged 11 commits into from
Dec 21, 2021
Merged

Support multiple api keys #2130

merged 11 commits into from
Dec 21, 2021

Conversation

kanej
Copy link
Member

@kanej kanej commented Dec 6, 2021

Etherscan has expanded to provided block explorers and verifiers for popular sidechains. The config in the hardhat-etherscan plugin assumes there is only one apiKey for use in verification, even though the project may be targeting multiple network/sidechains.

This PR adds support in the hardhat-etherscan configuration for setting an api key for etherscan per network (i.e. a project could support project verification on both mainnet and polygon at once without hacks).

Approach

Extend the current allowed structure of the hardhat-etherscan from:

{
   etherscan: {
      apiKey: string, 
   }
}

to:

{
   etherscan: {
      apiKey: string | EtherscanApiKeys, 
   }
}

Where EtherscanApiKeys is an object with one property for each supported network mapping to the apiKey to used when verifying on that network.

The supported networks are derived from the networkConfig object that combines the chainId and etherscan urls, and is used to define the typesafety of the etherscan config object (only networks present on the config object are allowed in a valid etherscan config).

Relates to #1448

TODO

We'll release a new major version.

  • Add support for EtherscanApiKeys under etherscan config
  • Refactor supported networks so that each one has a name (used in the EtherscanApiKeys type)
  • The relationship between supported networks and EtherscanApiKeys should be typesafe
  • The key to be used when verifying a contract should be the one that maps to the currently selected network if the using EtherscanApiKeys, or the apiKey string otherwise
  • Update the README
  • Update the contribution guide
  • Manual testing against a sample of alternate networks

@kanej kanej self-assigned this Dec 6, 2021
@changeset-bot
Copy link

changeset-bot bot commented Dec 6, 2021

🦋 Changeset detected

Latest commit: 6dcf619

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@nomiclabs/hardhat-etherscan Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

The config has been extended to allow both a string as api key or an object that can contain multiple api keys, keyed by chain.

Relates to #1448.
@kanej kanej marked this pull request as ready for review December 7, 2021 19:49
@kanej kanej requested a review from fvictorio December 7, 2021 19:51
@kanej kanej linked an issue Dec 8, 2021 that may be closed by this pull request
kanej and others added 3 commits December 9, 2021 09:50
Previously the enums where internal, to support passing eslint tests they are being switched to camelcase.
fvictorio and others added 2 commits December 15, 2021 11:08
A previous refactoring was able to invalidate the test when the type of
chainConfig changed.
@fvictorio
Copy link
Member

I modified some error messages. I think they look a bit better now:

image

image

There is only one extra thing I'd like to have here related to error handling: to show a proper error when the user configures an API key under an invalid network name. This is only relevant for javascript projects, since in typescript that will simply not compile.

I think this is important because it's likely that users won't read the docs, and they'll try to use the network name as the key.

fvictorio and others added 2 commits December 15, 2021 18:21
For js projects there is now an explicit check that the api keys
provided are supported.
@kanej
Copy link
Member Author

kanej commented Dec 20, 2021

I have added a check on the etherscan api keys during extendConfig, so it throws if a js project has included an unsupported network as a key:

image

I have repeated the manual test of a verify against ropsten, but that was using the same linking approach as before @fvictorio.

Copy link
Member

@fvictorio fvictorio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comments, pre-approving

Co-authored-by: Franco Victorio <victorio.franco@gmail.com>
@kanej kanej merged commit 456b685 into master Dec 21, 2021
@kanej kanej deleted the support-multiple-api-keys branch December 21, 2021 21:16
@Shelvak
Copy link

Shelvak commented Dec 24, 2021

I pulled master and just use the multi-api-key, amazing feature 👍 Thank you so much 🚀 🚀

@fvictorio
Copy link
Member

@Shelvak oh, thank you for testing it! We'll probably release this feature this week.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants