-
Notifications
You must be signed in to change notification settings - Fork 23
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
feat: replace env private keys with keystores #83
Conversation
@@ -104,7 +104,13 @@ yarn coverage | |||
|
|||
### Setup | |||
|
|||
Configure the `.env` variables. |
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.
Keep this as you still need the rpcs and etherescan api key
README.md
Outdated
@@ -104,7 +104,13 @@ yarn coverage | |||
|
|||
### Setup | |||
|
|||
Configure the `.env` variables. | |||
```bash | |||
cast wallet import MAINNET_DEPLOYER --interactive |
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.
What do you think about giving devs the choice to name the wallet the way they want? We could do it through the .env
file.
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.
hmmm seems unnecessary to build an .env to name the deployer, isn't foundry storing it locally?
is "forcing a dev to choose a name" (setting the env), vs "just using MAINNET_DEPLOYER
" and allowing the dev to change the package.json
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.
Changing the package.json
would require every dev to declare (or redeclare) such account locally in order to be able to run yarn deploy
. However, I agree with that forcing a dev to choose a name is generally undesired. Maybe we could update MAINNET_DEPLOYER
at the beginning of each project, setting it to a more ad hoc account name, or use an organization-wide default name.
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.
👍
6b8b5c2
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.
😘🤌🏻
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.
👍
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.
😍
Closes BES-362