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.

Sign up for GitHub

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

Jump to bottom

feat: automate etherscan verification #1918

Merged

nicholaspai merged 9 commits into master from feat/etherscan-automate-verification

Sep 3, 2020

Contributor

kendricktan commented Aug 28, 2020 •

edited by mrice32

Loading

Motivation

This PR includes a way to automate the etherscan verification process.

Summary

To automate the verification process on etherscan, we need to do two additional things:

Capture the constructor args during deployment/migrations
Save the constructor args into a file.

Details

This PR adds the feature to save constructor args into a separate file (networkid_args.json) in the networks directory. Upon deployment it'll use that those args to verify the contracts on etherscan.

For example:

truffle migrate --reset --network kovan
# There is now a file `42_args.json` in the `networks` directory

# Gets the files prefixed with `42` in the `networks` directory
# and attempts to verify them
npx buidler etherscan-verification --network-id 42

Known Issues
Unable to verify ExpiringMultiPartyLib, DesignatedVotingFactory, TokenFactory, and Governor via the API, however uploading the solc-input.json via the web interface works. A support ticket with etherscan has been made.

Issue(s)

#1807
Fixes #1854.

kendricktan added 4 commits

August 25, 2020 14:08


          etherscan verification example

7a2a7ed


          fixed data structures

60cd122

Signed-off-by: Kendrick Tan <[email protected]>


          partial etherscan verification

5fac9f6

Signed-off-by: Kendrick Tan <[email protected]>


          Merge branch 'master' of github.com:umaprotocol/protocol into feat/et…

6297faa

…herscan-automate-verification

kendricktan requested review from a team, daywiss and nicholaspai and removed request for a team

August 28, 2020 01:17


          checks

32fcbb2

Signed-off-by: Kendrick Tan <[email protected]>

Member

nicholaspai commented Aug 28, 2020

Drive by comment

Unable to verify ExpiringMultiPartyLib via the API, however uploading the solc-input.json via the web interface works. A support ticket with etherscan has been made.

Does this work without. modifications to solc-input.json?

nicholaspai reviewed

View reviewed changes

packages/common/src/MigrationUtils.js

+                    });
+                  // Save to file
+                  fs.writeFileSync(tvrPath, JSON.stringify({ ...tvrData, [address]: argsFixed }, null, 4));

Member

nicholaspai Aug 28, 2020

Can you explain what the null and 4 args passed to JSON.stringfy do and why they're needed? Seems like a bit of a magic number

Contributor Author

kendricktan Aug 30, 2020

The null and 4 are there to make it human readable, otherwise it'll flatten the whole thing.

The null is the replacer function, which intends to replace key-value pairs.

4 is number of spacing. 4 was chosen as thats the standard number of indented spaces.

nicholaspai reviewed

View reviewed changes

packages/core/networks/42.json Outdated

@@ @@ -1,66 +1,66 @@ @@
               [
                 {
                   "contractName": "Migrations",
-                  "address": "0xB851449bC030F08E0f1Aa57A0a7cAEFE73030De0"

Member

nicholaspai Aug 28, 2020

I think you should remove all changes to 42.json from this PR, as we already support a set of contracts that are integrated into emp-tools for example. However, were these contracts all verified on Etherscan via your script?

Contributor Author

kendricktan Aug 30, 2020

Can do, this is more to show-case the automation side of it. I can revert it back to the original contracts.

Contributor Author

kendricktan Aug 30, 2020

I'll revert the contract addresses once the PR comes to a consensus. I left it here just to showcase how the process works.

Member

mrice32 Aug 31, 2020

SGTM!

nicholaspai reviewed

View reviewed changes

packages/core/networks/42.json Outdated

                 },
                 {
                   "contractName": "Governor",
-                  "address": "0xca4575EE197308c9D2aBF813A5f064f44898b7a4"
+                  "address": "0x94c99c8Ce915ddd77a556eA81caa3A9aB86f5642"

Member

nicholaspai Aug 28, 2020

Governor does not look verified

nicholaspai reviewed

View reviewed changes

packages/core/networks/42.json Outdated

-                {
-                  "contractName": "WETH9",
-                  "address": "0xd0A1E359811322d97991E03f863a0C30C2cF029C"
+                  "address": "0xac065bB2C954dCf2f76Fa0092e2bc997fc261B4f"

Member

nicholaspai Aug 28, 2020

WETH9 is not verified

Member

nicholaspai Aug 28, 2020

Actually this might be DesignatedVotingFactory

nicholaspai reviewed

View reviewed changes

packages/core/networks/42.json Outdated

                 },
                 {
                   "contractName": "TestnetERC20",
                   "address": "0xbF7A7169562078c96f0eC1A8aFD6aE50f12e5A99"
                 },
+                {
+                  "contractName": "TokenFactory",
+                  "address": "0x4bb1B6F3e028195373A85bd30869aEeC8A7C2671"

Member

nicholaspai Aug 28, 2020

TokenFactory not verified

nicholaspai reviewed

View reviewed changes

packages/core/networks/42.json Outdated

                 },
                 {
                   "contractName": "ExpiringMultiPartyLib",
-                  "address": "0x9C53D9f4E959b1cd2eE25C45f50aFf45dca471CE"
+                  "address": "0x46C1790fdE1F44cdf052735a4Fac4152a51Cae04"

Member

nicholaspai Aug 28, 2020

This one not verified, but I see you pointed this out in PR description

nicholaspai reviewed

View reviewed changes

packages/core/buidler.config.js Outdated Show resolved Hide resolved

nicholaspai reviewed

View reviewed changes

Member

nicholaspai left a comment

Looks really useful! A few changes I'd like to see:

Don't commit changes to 42.json as this is the list of supported Kovan contracts which we've already verified. However, please do post as a comment a list of contracts that were verified via this method
I noticed that some contracts still were not verified

Contributor

daywiss commented Aug 28, 2020

im prob not the best to review this, but i will look it over. you might want to get @mrice32 or @chrismaree as a review as well

Contributor Author

kendricktan commented Aug 30, 2020 •

edited

Loading

Drive by comment

Unable to verify ExpiringMultiPartyLib via the API, however uploading the solc-input.json via the web interface works. A support ticket with etherscan has been made.

Does this work without. modifications to solc-input.json?

Yeap ExpiringMultiPartyLib can be verified via the web-interface with 0 edits to solc-input.json. The one that needs modification is the ExpiringMultiPartyCreator, which gets automatically verified for some reason...

I noticed that some contracts still were not verified
Some contracts that are not added by the truffle-registry (e.g. WETH) won't automatically get verified as there is no constructor args.

The API feels a bit flaky, its only able to verify SOME contracts through the API. However if you were to manually upload solc-input.json via the web portal, it verifies successfully.

Some contracts that are affected by this as you've discovered (you'll have to manually upload it via the web portal):

Governor
TokenFactory
DesignatedVotingFactory
ExpiringMultiPartyLib

I didn't mention 1, 2, and 3 as I presumed the ones that were causing the biggest issues on verification were the Voting, EMPCreator, and EMPLib contracts

Member

nicholaspai commented Aug 30, 2020

Drive by comment

Unable to verify ExpiringMultiPartyLib via the API, however uploading the solc-input.json via the web interface works. A support ticket with etherscan has been made.

Does this work without. modifications to solc-input.json?

Yeap ExpiringMultiPartyLib can be verified via the web-interface with 0 edits to solc-input.json. The one that needs modification is the ExpiringMultiPartyCreator, which gets automatically verified for some reason...

I noticed that some contracts still were not verified
Some contracts that are not added by the truffle-registry (e.g. WETH) won't automatically get verified as there is no constructor args.

The API feels a bit flaky, its only able to verify SOME contracts through the API. However if you were to manually upload solc-input.json via the web portal, it verifies successfully.

Some contracts that are affected by this as you've discovered (you'll have to manually upload it via the web portal):

Governor

TokenFactory

DesignatedVotingFactory

ExpiringMultiPartyLib

I didn't mention 1, 2, and 3 as I presumed the ones that were causing the biggest issues on verification were the Voting, EMPCreator, and EMPLib contracts

Why do you think those didn't get auto verified? Is it a buidler issue, since the manual solc.json upload works? Overall I support this approach, my only suggestions are to figure out (if possible) how to auto verify ALL contracts and secondly to auto-detect the user's network so we have fewer .env vars


          coverage to truffle

f442a4c

Contributor Author

kendricktan commented Aug 31, 2020

Why do you think those didn't get auto verified? Is it a buidler issue, since the manual solc.json upload works?

I don't think its a buidler issue, technically speaking, I'm only writing it as a builder plugin because I need buidler to generate solc-input.json. Apart from using builder to generate solc-input.json, everything else is API calls.

It is likely an issue on etherscan's end as:

Files are identical
The only difference is that files are uploaded via the API instead of the web UI
- They're using a different processes for verification via the web ui and verification via the API

kendricktan added 2 commits

August 31, 2020 10:40


          truffle...

08af082


          auto get network api url

5100f88

Contributor Author

kendricktan commented Aug 31, 2020

@mrice32 @nicholaspai @chrismaree @daywiss I'm changing the coverage to use truffle run converage instead of buidler as it keeps hanging, PR can't be merged unless all the checks passes.

coveralls commented Aug 31, 2020 •

edited

Loading

Coverage remained the same at 92.71% when pulling 5f9989e on feat/etherscan-automate-verification into c210f03 on master.

Member

nicholaspai commented Aug 31, 2020

Why do you think those didn't get auto verified? Is it a buidler issue, since the manual solc.json upload works?

I don't think its a buidler issue, technically speaking, I'm only writing it as a builder plugin because I need buidler to generate solc-input.json. Apart from using builder to generate solc-input.json, everything else is API calls.

It is likely an issue on etherscan's end as:

Files are identical

The only difference is that files are uploaded via the API instead of the web UI

They're using a different processes for verification via the web ui and verification via the API

Ok, I think before approving this let's revert the changes in 42.json and 42.args.json, and then can you try to figure out a way to automate the remaining contract verifications? Otherwise you should make a note somewhere which contracts get verified and which do not (or maybe console.log each successful verified contract) so the user knows which ones to manually verify.

Contributor Author

kendricktan commented Aug 31, 2020 •

edited

Loading

Ok, I think before approving this let's revert the changes in 42.json and 42.args.json, and then can you try to figure out a way to automate the remaining contract verifications? Otherwise you should make a note somewhere which contracts get verified and which do not (or maybe console.log each successful verified contract) so the user knows which ones to manually verify.

It console.logs the whole process (success, failed, in-progress), with color coding 😛

Member

nicholaspai commented Aug 31, 2020

Ok, I think before approving this let's revert the changes in 42.json and 42.args.json, and then can you try to figure out a way to automate the remaining contract verifications? Otherwise you should make a note somewhere which contracts get verified and which do not (or maybe console.log each successful verified contract) so the user knows which ones to manually verify.

It console.logs the whole process (success, failed, in-progress), with color coding 😛

Beautiful. Just revert the 42.json files then LGTM


          revert contracts

5f9989e

Signed-off-by: Kendrick Tan <[email protected]>

nicholaspai reviewed

View reviewed changes

packages/core/buidler.config.js

               module.exports = {
                 solc: {
-                  version: "0.6.12",
+                  version: solcVersion,
                   optimizer: {
                     enabled: true,
                     runs: 199

Member

nicholaspai Aug 31, 2020

Should you change the networks config to the user's connected network?

Contributor Author

kendricktan Aug 31, 2020

This networks is a config when buidler runs tests, e.g. npx buidler test --network localhost/buidlerevm.

What do you mean by "user connected network"?

Member

nicholaspai Aug 31, 2020

what if I want to use Kovan or Mainnet network would it work for that? Sorry I don't have much familiarity with buidler

Contributor Author

kendricktan Aug 31, 2020

If you'd like to run tests for the kovan network, you'll need to add the network config to the "networks" section. This PR is about a separate topic - verifying contracts, please keep it at that.

To verify the contracts, you need to files to be saved in the truffle-registry (i.e. in networks/ directory). So for example, if you've deployed the kovan contracts, it will appear as networks/42.json. This PR also captures the constructor args for each deployed contract, and saves them into networks/42_args.json.

To verify the deployed contracts for kovan you'll run npx buidler verify-etherscan --network-id 42. It'll try and find the files 42.json and 42_args.json in the network directory, and combined them with solc-input.json before uploading them to etherscan.

Member

nicholaspai Aug 31, 2020

If you'd like to run tests for the kovan network, you'll need to add the network config to the "networks" section. This PR is about a separate topic - verifying contracts, please keep it at that.

Sorry I think i am confused. You're saying that the networks object has no impact on verification? Only affects the tests?

Contributor Author

kendricktan Aug 31, 2020 •

edited

Loading

Yeap!

For verification, you supply the network-id to the cli here, https://github.com/UMAprotocol/protocol/blob/feat/etherscan-automate-verification/packages/core/buidler.config.js#L48

Member

mrice32 Sep 1, 2020 •

edited

Loading

Just to tie this thread off, I think what you're saying @kendricktan is that the verification does not require buidler to have any non-test networks set up (like we have for truffle). The verification task is independent of (and doesn't rely on) the declared networks in the buidler config. All it needs is to be able to compile (network agnostic) and see the networks/ files.

SGTM!

nicholaspai approved these changes

View reviewed changes

Member

nicholaspai left a comment

Looks great! 2 things:

Would like a second set of eyes from this so I've requested @mrice32 @daywiss and @chrismaree
Is your networks param in the buidler.config.js meant to be pointed at localHost?

nicholaspai requested review from mrice32 and chrismaree

August 31, 2020 18:31

mrice32 approved these changes

View reviewed changes

Member

mrice32 left a comment

This LGTM!

Two PR nits:

Title should be feat: automate etherscan verification to conform to conventional commits.

And I think this PR should fix #1854, so please add Fixes #1854 to the issues section of the description.

packages/core/buidler.config.js

               module.exports = {
                 solc: {
-                  version: "0.6.12",
+                  version: solcVersion,
                   optimizer: {
                     enabled: true,
                     runs: 199

Member

mrice32 Sep 1, 2020 •

edited

Loading

Just to tie this thread off, I think what you're saying @kendricktan is that the verification does not require buidler to have any non-test networks set up (like we have for truffle). The verification task is independent of (and doesn't rely on) the declared networks in the buidler config. All it needs is to be able to compile (network agnostic) and see the networks/ files.

SGTM!

mrice32 changed the title ~~Feat/etherscan automate verification~~ feat: automate etherscan verification

chrismaree approved these changes

View reviewed changes

Member

chrismaree left a comment

This is pretty awesome. pitty it does not work in all cases but seeing we can upload the output file directly to the Etherscan UI it's still a massive step up.

daywiss approved these changes

View reviewed changes

nicholaspai merged commit f2d47a4 into master

mrice32 deleted the feat/etherscan-automate-verification branch

September 3, 2020 12:55

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet