feat(cast): wallet new - enable default keystore #9201
Conversation
kien6034
requested review from
DaniPopes,
grandizzy,
klkvr,
mattsse,
yash-atreya and
zerosnacks
as code owners
kien6034
changed the title
kien6034
changed the title
|
output locally: |
|
Hi @kien6034 thanks for your PR, a few notes |
kien6034
force-pushed
the
feat/cast-wallet-default-keystore-path
branch
from
31537dc to
f28f6af
Compare
|
Hi @zerosnacks , i updated code and writing tests following your comments. |
There was a problem hiding this comment.
Thanks @kien6034, looks great
Pending another review cc @grandizzy / @yash-atreya
|
@grandizzy @yash-atreya can you help me review this PR? |
There was a problem hiding this comment.
left a comment re newly introduced default_keystore, @zerosnacks pls chime in. thanks!
crates/cast/bin/cmd/wallet/mod.rs
Outdated
| json: bool, | ||
|
|
||
| /// Use default keystore location (~/.foundry/keystores). | ||
| #[arg(long, short, conflicts_with = "path", default_value = "false")] |
There was a problem hiding this comment.
If I get it right, the default keystore is used if no path provided but only with unsafe_password option (any reason why this is not available for password too?).
If so, do we want to make this explicit by adding the new default_keystore arg or can we just use default keystore if path is None?
That is the
} else if default_keystore {below to be
} else if unsafe_password {@zerosnacks wdyt?
There was a problem hiding this comment.
the removal of requires = "path should allow it to use the default path, not place a conditional on it
I would prefer making the default path implicit as well over having a new flag be introduced
There was a problem hiding this comment.
@zerosnacks @grandizzy tks for reviewing
so as my understanding, we should use default_key_store when path is none + unsafe password or password is provided.
if password fields not provided, we just printout the keys for user?
There was a problem hiding this comment.
Correct, a keystore implies having a password whereas a keypair does not
|
@zerosnacks updated sir |
|
@zerosnacks @grandizzy hello sirs, can you help me finalize this pr? |
|
Hi @kien6034 My apology for missing your pings and losing track of your PR; this is definitely still something we want to merge! I've updated the PR to reflect the changes that happened since your proposal |
…ien6034/foundry into feat/cast-wallet-default-keystore-path
|
Thanks @kien6034 for your PR! |
* feat: use default keystore flag * bet * refactor: wallet new function * feat: implicitly defer default keystore * fix nit --------- Co-authored-by: zerosnacks <zerosnacks@protonmail.com> Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com>
3 participants
Close #9166
Motivation
Solution
-default-keystoreto allow user to generate wallet if no path is given