Skip to content

Comments

Run etherscan validation during the deployment process#1722

Merged
maurelian merged 2 commits intoregenesis/0.5.0from
maurelian/eng-1632-add-etherscan-validation-to
Nov 9, 2021
Merged

Run etherscan validation during the deployment process#1722
maurelian merged 2 commits intoregenesis/0.5.0from
maurelian/eng-1632-add-etherscan-validation-to

Conversation

@maurelian
Copy link
Contributor

@maurelian maurelian commented Nov 9, 2021

Description

Adds a try/catch wrapped attempt to verify each contract on etherscan immediately after deployment.

Testing instructions

  1. Download and apply the patch file attached:
    0001-TEMP-commit-for-testing-eng-1632.patch.txt

  2. Set and source the appropriate values in your .env file.

  3. Open ./scripts/deploy-scripts/kovan.sh and modify according to the instructions therein.

  4. Run that script.

Expected results

I think that on Kovan, all verification attempts will just give an "error", since identical source code has already been verified on Kovan, but that's OK.

Error when verifying bytecode on etherscan:
NomicLabsHardhatPluginError: The Etherscan API responded with a failure status.
The verification may still succeed but should be checked manually.
Reason: Already Verified

You could try testing on another testnet for a more representative simulation of mainnet.

refactor(contracts): Rename deployAndPostDeploy to deployAndVerifyAndThen
@changeset-bot
Copy link

changeset-bot bot commented Nov 9, 2021

🦋 Changeset detected

Latest commit: c55c4cb

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

This PR includes changesets to release 4 packages
Name Type
@eth-optimism/contracts Patch
@eth-optimism/batch-submitter Patch
@eth-optimism/data-transport-layer Patch
@eth-optimism/message-relayer Patch

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

@maurelian maurelian force-pushed the maurelian/eng-1632-add-etherscan-validation-to branch from cac45aa to 31f5c80 Compare November 9, 2021 03:31
@codecov-commenter
Copy link

codecov-commenter commented Nov 9, 2021

Codecov Report

Merging #1722 (c55c4cb) into regenesis/0.5.0 (8282054) will decrease coverage by 0.51%.
The diff coverage is n/a.

Impacted file tree graph

@@                 Coverage Diff                 @@
##           regenesis/0.5.0    #1722      +/-   ##
===================================================
- Coverage            72.64%   72.12%   -0.52%     
===================================================
  Files                   69       67       -2     
  Lines                 2274     2167     -107     
  Branches               337      324      -13     
===================================================
- Hits                  1652     1563      -89     
+ Misses                 622      604      -18     
Flag Coverage Δ
batch-submitter 61.56% <ø> (ø)
contracts 87.96% <ø> (ø)
core-utils 57.72% <ø> (ø)
data-transport-layer 38.23% <ø> (ø)
message-relayer ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/message-relayer/src/relay-tx.ts
packages/message-relayer/hardhat.config.ts

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8282054...c55c4cb. Read the comment docs.

@optimisticben
Copy link
Contributor

Reason: Already Verified

Is it possible to check for an existing verification before performing another one?
Is it possible to capture the "failed" response, parse for this "Reason: Already Verified" message, and provide "contract verified" in the output?

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the error?

Copy link
Contributor Author

@maurelian maurelian Nov 9, 2021

Choose a reason for hiding this comment

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

Usually this, which happens on Kovan because the identical source code has already been verified.

NomicLabsHardhatPluginError: The Etherscan API responded with a failure status.
The verification may still succeed but should be checked manually.
Reason: Already Verified

I could detect that particular error and throw on everything else, but IMO halting the deployment based on an external failure from the Etherscan API would be an overreaction. Catching any errors on this call seems like the best approach, as we can always go back and manually verify a single contract quite easily vs. restarting the process mid-deployment which feels more distuptive to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah that's fine. I'll just request that we open up an issue on GH to make this more robust in the future.

@maurelian maurelian force-pushed the maurelian/eng-1632-add-etherscan-validation-to branch from 31f5c80 to c55c4cb Compare November 9, 2021 18:28
@tynes
Copy link
Contributor

tynes commented Nov 9, 2021

I know that @mslipper is working on replacing hardhat with geth in clique mode, we may need to make additional changes based on that eventually since geth in clique uses a chainid of 1337

@maurelian
Copy link
Contributor Author

@tynes That sounds great. It should be easy to rename and modify isHardHatNode() to handle that.

@maurelian maurelian merged commit c61e070 into regenesis/0.5.0 Nov 9, 2021
@maurelian maurelian deleted the maurelian/eng-1632-add-etherscan-validation-to branch November 9, 2021 21:18
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.

5 participants