This repository has been archived by the owner on Nov 27, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mds1
reviewed
Apr 11, 2023
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 know it's still a draft, but figured I'd leave a few small comments
davidlaprade
force-pushed
the
davidlaprade/factory-deploy-script
branch
from
April 11, 2023 19:28
c26934e
to
75b85a5
Compare
davidlaprade
requested review from
AustinGreen,
0xrajath,
efecarranza and
dd0sxx
as code owners
April 11, 2023 19:53
mds1
reviewed
Apr 11, 2023
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.
This looks great, awesome job!
davidlaprade
force-pushed
the
davidlaprade/factory-deploy-script
branch
from
April 12, 2023 13:28
40240b5
to
5002249
Compare
efecarranza
reviewed
Apr 12, 2023
dd0sxx
approved these changes
Apr 12, 2023
davidlaprade
force-pushed
the
davidlaprade/factory-deploy-script
branch
from
April 13, 2023 13:33
11b1337
to
133c1a7
Compare
AustinGreen
reviewed
Apr 13, 2023
AustinGreen
reviewed
Apr 13, 2023
AustinGreen
approved these changes
Apr 13, 2023
0xrajath
reviewed
Apr 13, 2023
0xrajath
reviewed
Apr 13, 2023
0xrajath
reviewed
Apr 13, 2023
0xrajath
reviewed
Apr 13, 2023
0xrajath
reviewed
Apr 13, 2023
0xrajath
reviewed
Apr 13, 2023
0xrajath
reviewed
Apr 13, 2023
0xrajath
reviewed
Apr 13, 2023
0xrajath
reviewed
Apr 13, 2023
0xrajath
reviewed
Apr 13, 2023
0xrajath
reviewed
Apr 13, 2023
0xrajath
reviewed
Apr 13, 2023
0xrajath
approved these changes
Apr 13, 2023
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.
LGTM. Left a couple of clarifying questions for my own learning
0xrajath
reviewed
Apr 13, 2023
davidlaprade
force-pushed
the
davidlaprade/factory-deploy-script
branch
from
April 13, 2023 21:00
8f7da46
to
ef0a8c0
Compare
Coverage after merging davidlaprade/factory-deploy-script into main will be
Coverage Report
|
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
We want an automated way to deploy the Vertex infrastructure on different networks, ensuring that important contracts on each network have the same address. We also want to make sure that this is done safely, ideally without the possibility that anyone could frontrun a transaction, e.g. to intialize an important contract.
Modifications:
This PR:
DeployVertex
script to deploy logic contracts, the lens, and the factory to a new chainDeployVertex
as a part of the test suite setup, so that all tests run against the state prepared by the deploy scriptResult:
We have a script to safely deploy Vertex on multiple chains