-
Notifications
You must be signed in to change notification settings - Fork 975
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
Accounts Revamp Fixes: "Overall" Wallet Improvements #6736
Merged
Merged
Changes from 21 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
bf37112
change default wallet dir path to not be hidden
rauljordan 25a2463
Merge branch 'master' of github.com:prysmaticlabs/prysm into wallet-i…
shayzluf dd4b683
gaz + pass wallet dir
shayzluf d485e09
gaz + move const to flags
shayzluf 631e9b0
move to flags
shayzluf 15ada8f
move to flags
shayzluf 092b43a
use filepath join in order to create a valid dir name
shayzluf b73fe3f
add wallet dir
rauljordan 55d31b7
fix confs
rauljordan c53848e
return err no wallet found issues
rauljordan 2010530
Merge branch 'master' into wallet-improvements-overall
rauljordan cc37d1e
fix up edit remote
rauljordan c2a0ec8
Merge branch 'wallet-improvements-overall' of github.com:prysmaticlab…
rauljordan 1886052
all tests passing
rauljordan 232bcf5
fix test
rauljordan 758d72a
create or open wallet
rauljordan 9a71304
ivan feedback
rauljordan 308e91e
Merge refs/heads/master into wallet-improvements-overall
prylabs-bulldozer[bot] 17fa95e
enter password for account with pubkey
rauljordan de054c8
Merge branch 'wallet-improvements-overall' of github.com:prysmaticlab…
rauljordan cfc7a90
Update validator/accounts/v2/accounts_create.go
rauljordan 0c05bdb
works
rauljordan 71093ed
Merge branch 'wallet-improvements-overall' of github.com:prysmaticlab…
rauljordan 25563eb
resolve confs
rauljordan f4c6765
preston feedback
rauljordan 8665bbf
nothing to export
rauljordan 7d38761
fmt
rauljordan 4b237aa
test for create or open
rauljordan 78549d1
gaz
rauljordan 205cbe8
Merge refs/heads/master into wallet-improvements-overall
prylabs-bulldozer[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,53 +21,40 @@ import ( | |
// ImportAccount uses the archived account made from ExportAccount to import an account and | ||
// asks the users for account passwords. | ||
func ImportAccount(cliCtx *cli.Context) error { | ||
walletDir, err := inputDirectory(cliCtx, walletDirPromptText, flags.WalletDirFlag) | ||
if err != nil && !errors.Is(err, ErrNoWalletFound) { | ||
return errors.Wrap(err, "could not parse wallet directory") | ||
} | ||
// Check if the user has a wallet at the specified path. If so, only let them continue if it is a non-HD wallet. | ||
walletExists, err := hasDir(walletDir) | ||
if err != nil { | ||
return errors.Wrap(err, "could not check if wallet exists") | ||
} | ||
if walletExists { | ||
keymanagerKind, err := readKeymanagerKindFromWalletPath(walletDir) | ||
if err != nil { | ||
return errors.Wrap(err, "could not read keymanager kind for existing wallet") | ||
ctx := context.Background() | ||
wallet, err := createOrOpenWallet(cliCtx, func(cliCtx *cli.Context) (*Wallet, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you add a test case with where no wallet exists? |
||
w, err := NewWallet(cliCtx, v2keymanager.Direct) | ||
if err != nil && !errors.Is(err, ErrWalletExists) { | ||
return nil, errors.Wrap(err, "could not create new wallet") | ||
} | ||
if keymanagerKind != v2keymanager.Direct { | ||
return fmt.Errorf( | ||
"importing non-HD accounts into a non-direct wallet is not allowed, given wallet path contains a %s wallet", | ||
keymanagerKind.String(), | ||
) | ||
if err = createDirectKeymanagerWallet(cliCtx, w); err != nil { | ||
return nil, errors.Wrap(err, "could not initialize wallet") | ||
} | ||
} | ||
passwordsDir, err := inputDirectory(cliCtx, passwordsDirPromptText, flags.WalletPasswordsDirFlag) | ||
log.WithField("wallet-path", w.walletDir).Info( | ||
"Successfully created new wallet", | ||
) | ||
return w, err | ||
}) | ||
if err != nil { | ||
return err | ||
return errors.Wrap(err, "could not initialize wallet") | ||
} | ||
if wallet.KeymanagerKind() != v2keymanager.Direct { | ||
return errors.New( | ||
"only non-HD wallets can import accounts, try creating a new wallet with wallet-v2 create", | ||
) | ||
} | ||
keysDir, err := inputDirectory(cliCtx, importKeysDirPromptText, flags.KeysDirFlag) | ||
if err != nil { | ||
return errors.Wrap(err, "could not parse keys directory") | ||
} | ||
|
||
accountsPath := filepath.Join(walletDir, v2keymanager.Direct.String()) | ||
if err := os.MkdirAll(accountsPath, DirectoryPermissions); err != nil { | ||
return errors.Wrap(err, "could not create wallet directory") | ||
} | ||
if err := os.MkdirAll(passwordsDir, DirectoryPermissions); err != nil { | ||
return errors.Wrap(err, "could not create passwords directory") | ||
} | ||
|
||
wallet := &Wallet{ | ||
accountsPath: accountsPath, | ||
passwordsDir: passwordsDir, | ||
keymanagerKind: v2keymanager.Direct, | ||
if err := wallet.SaveWallet(); err != nil { | ||
return errors.Wrap(err, "could not save wallet") | ||
} | ||
|
||
var accountsImported []string | ||
ctx := context.Background() | ||
if err := filepath.Walk(keysDir, func(path string, info os.FileInfo, err error) error { | ||
if err != nil { | ||
return err | ||
} | ||
if info.IsDir() { | ||
return nil | ||
} | ||
|
@@ -88,11 +75,11 @@ func ImportAccount(cliCtx *cli.Context) error { | |
return nil | ||
} | ||
|
||
accountName, err := wallet.importKeystore(ctx, path) | ||
accountName, pubKey, err := wallet.importKeystore(ctx, path) | ||
if err != nil { | ||
return errors.Wrap(err, "could not import keystore") | ||
} | ||
if err := wallet.enterPasswordForAccount(cliCtx, accountName); err != nil { | ||
if err := wallet.enterPasswordForAccount(cliCtx, accountName, pubKey); err != nil { | ||
return errors.Wrap(err, "could not verify password for keystore") | ||
} | ||
accountsImported = append(accountsImported, accountName) | ||
|
@@ -116,25 +103,25 @@ func ImportAccount(cliCtx *cli.Context) error { | |
return nil | ||
} | ||
|
||
func (w *Wallet) importKeystore(ctx context.Context, keystoreFilePath string) (string, error) { | ||
func (w *Wallet) importKeystore(ctx context.Context, keystoreFilePath string) (string, []byte, error) { | ||
keystoreBytes, err := ioutil.ReadFile(keystoreFilePath) | ||
if err != nil { | ||
return "", errors.Wrap(err, "could not read keystore file") | ||
return "", nil, errors.Wrap(err, "could not read keystore file") | ||
} | ||
keystoreFile := &v2keymanager.Keystore{} | ||
if err := json.Unmarshal(keystoreBytes, keystoreFile); err != nil { | ||
return "", errors.Wrap(err, "could not decode keystore json") | ||
return "", nil, errors.Wrap(err, "could not decode keystore json") | ||
} | ||
pubKeyBytes, err := hex.DecodeString(keystoreFile.Pubkey) | ||
if err != nil { | ||
return "", errors.Wrap(err, "could not decode public key string in keystore") | ||
return "", nil, errors.Wrap(err, "could not decode public key string in keystore") | ||
} | ||
accountName := petnames.DeterministicName(pubKeyBytes, "-") | ||
keystoreFileName := filepath.Base(keystoreFilePath) | ||
if err := w.WriteFileAtPath(ctx, accountName, keystoreFileName, keystoreBytes); err != nil { | ||
return "", errors.Wrap(err, "could not write keystore to account dir") | ||
return "", nil, errors.Wrap(err, "could not write keystore to account dir") | ||
} | ||
return accountName, nil | ||
return accountName, pubKeyBytes, nil | ||
} | ||
|
||
func logAccountsImported(wallet *Wallet, keymanager *direct.Keymanager, accountNames []string) error { | ||
|
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
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
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
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
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
Oops, something went wrong.
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.
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.
It looks like this is the export code. Instead of directing the user to create a wallet, maybe we could just inform them there is nothing to export?