-
Notifications
You must be signed in to change notification settings - Fork 40
Add multiple networks #239
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
Conversation
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
How was |
I run the script regardless (I didn't know they were deployed), and it detected it onchain and created the entries. I didn't do it manually. |
FYI, polygon was actually deployed by @nlordell back when some experiments / testing was being run. https://polygonscan.com/tx/0x0e24d3a2a8530eaad5ae62e54e64d57665a77ce3970227d20c1b77da315cbbf6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general it looks good, including Polygon (already the fact that the addresses match is proof enough for the contracts here anyway), but I didn't have time to look into the details, so this is a soft approve only. 🙂
I could only verify BSC contracts. @fedgiac please could you take a look into it? 🙏
Unfortunately I won't be able to do this soon, my recommendations would be to try the following two things in order if the scripts don't work:
- Use the single-deployment verification script for each contract on each chain, or
- Use standard-json verification: the same command as above, but also appending
--show-standard-json-input
to the command and using the resulting file to verify the contract on the web interface directly (without scripts, some more details here for the general concept).
package.json
Outdated
"version": "2.0.0-alpha", | ||
"version": "2.1.0-alpha", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to bump version here since it won't be released as a package. But there's the v1
branch for that, so once deployed and merged we should cherry-pick to that (like here) and bump the version. Should we also update the new network guide?
Contracts were verified for the remaining networks, see #240. I didn't encounter any particular issue when verifying, the script worked out of the box except for Avalanche. Was there some specific error? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I verified all remaining contracts and I checked each entry in networks.json
and it looks good. As long as the version number is restored to the current one, this PR is ready to merge.
bc6649b
to
c6bac3f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Description
Add support for the following networks: BSC, Avalanche, Polygon and Optimism.
Related Issues
I could only verify BSC contracts. @fedgiac please could you take a look into it? 🙏