From 83717218ba4364017355765df61e80159255d58d Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Sun, 26 Jul 2020 04:12:08 -0400 Subject: [PATCH 01/13] Fix accounts v2 runtime --- validator/accounts/v2/prompt.go | 4 ++-- validator/main.go | 1 + validator/node/node.go | 9 --------- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/validator/accounts/v2/prompt.go b/validator/accounts/v2/prompt.go index 35bf1683dd04..aeade206d869 100644 --- a/validator/accounts/v2/prompt.go +++ b/validator/accounts/v2/prompt.go @@ -110,11 +110,11 @@ func inputPassword(cliCtx *cli.Context, promptText string, confirmPassword passw if err != nil { return "", err } - enteredPassword := string(data) + enteredPassword := strings.TrimRight(string(data), "\r\n") if err := validatePasswordInput(enteredPassword); err != nil { return "", errors.Wrap(err, "password did not pass validation") } - return strings.TrimRight(enteredPassword, "\r\n"), nil + return enteredPassword, nil } var hasValidPassword bool diff --git a/validator/main.go b/validator/main.go index 47d0cb818fe2..ad1017004786 100644 --- a/validator/main.go +++ b/validator/main.go @@ -72,6 +72,7 @@ var appFlags = []cli.Flag{ flags.SlasherRPCProviderFlag, flags.SlasherCertFlag, flags.WalletPasswordsDirFlag, + flags.PasswordFileFlag, flags.WalletDirFlag, cmd.MinimalConfigFlag, cmd.E2EConfigFlag, diff --git a/validator/node/node.go b/validator/node/node.go index 53b3637ababc..8b85ce965fc2 100644 --- a/validator/node/node.go +++ b/validator/node/node.go @@ -9,7 +9,6 @@ import ( "io/ioutil" "os" "os/signal" - "path" "strings" "sync" "syscall" @@ -83,14 +82,6 @@ func NewValidatorClient(cliCtx *cli.Context) (*ValidatorClient, error) { var keyManagerV1 v1.KeyManager var keyManagerV2 v2.IKeymanager if featureconfig.Get().EnableAccountsV2 { - walletDir := cliCtx.String(flags.WalletDirFlag.Name) - if walletDir == flags.DefaultValidatorDir() { - walletDir = path.Join(walletDir, accountsv2.WalletDefaultDirName) - } - passwordsDir := cliCtx.String(flags.WalletPasswordsDirFlag.Name) - if passwordsDir == flags.DefaultValidatorDir() { - passwordsDir = path.Join(passwordsDir, accountsv2.PasswordsDefaultDirName) - } // Read the wallet from the specified path. wallet, err := accountsv2.OpenWallet(cliCtx) if err != nil { From 00ddf7aa75c31cc382a1869e4045c9b9f3ba8509 Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Sun, 26 Jul 2020 04:29:36 -0400 Subject: [PATCH 02/13] Accounts V2: Fix issues found by rkapka --- validator/accounts/v2/accounts_list.go | 6 +++--- validator/accounts/v2/cmd_accounts.go | 1 - validator/keymanager/v2/derived/derived.go | 8 ++++++-- validator/keymanager/v2/derived/mnemonic.go | 3 ++- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/validator/accounts/v2/accounts_list.go b/validator/accounts/v2/accounts_list.go index 862c47d2b39d..93686e62ccbb 100644 --- a/validator/accounts/v2/accounts_list.go +++ b/validator/accounts/v2/accounts_list.go @@ -93,8 +93,6 @@ func listDirectKeymanagerAccounts( } for i := 0; i < len(accountNames); i++ { fmt.Println("") - fmt.Printf("%s\n", au.BrightGreen(accountNames[i]).Bold()) - fmt.Printf("%s %#x\n", au.BrightMagenta("[public key]").Bold(), pubKeys[i]) // Retrieve the account creation timestamp. createdAtBytes, err := wallet.ReadFileAtPath(ctx, accountNames[i], direct.TimestampFileName) @@ -106,7 +104,9 @@ func listDirectKeymanagerAccounts( return errors.Wrapf(err, "could not parse account created at timestamp: %s", createdAtBytes) } unixTimestamp := time.Unix(unixTimestampStr, 0) - fmt.Printf("%s %s\n", au.BrightCyan("[created at]").Bold(), humanize.Time(unixTimestamp)) + + fmt.Printf("%s | %s\n", au.BrightGreen(accountNames[i]).Bold(), humanize.Time(unixTimestamp)) + fmt.Printf("%s %#x\n", au.BrightMagenta("[validating public key]").Bold(), pubKeys[i]) if !showDepositData { continue } diff --git a/validator/accounts/v2/cmd_accounts.go b/validator/accounts/v2/cmd_accounts.go index 0800b2a0cb05..d6a25414cc78 100644 --- a/validator/accounts/v2/cmd_accounts.go +++ b/validator/accounts/v2/cmd_accounts.go @@ -22,7 +22,6 @@ this command outputs a deposit data string which is required to become a validat flags.WalletDirFlag, flags.WalletPasswordsDirFlag, flags.PasswordFileFlag, - flags.SkipMnemonicConfirmFlag, featureconfig.AltonaTestnet, featureconfig.MedallaTestnet, }, diff --git a/validator/keymanager/v2/derived/derived.go b/validator/keymanager/v2/derived/derived.go index 79156286fe99..c6a593fc7314 100644 --- a/validator/keymanager/v2/derived/derived.go +++ b/validator/keymanager/v2/derived/derived.go @@ -9,6 +9,7 @@ import ( "io/ioutil" "path" "strconv" + "strings" "sync" "github.com/google/uuid" @@ -208,8 +209,11 @@ func InitializeWalletSeedFile(ctx context.Context, password string, skipMnemonic // appropriate seed file for recovering a derived wallets. func SeedFileFromMnemonic(ctx context.Context, mnemonic string, password string) (*SeedConfig, error) { walletSeed, err := bip39.EntropyFromMnemonic(mnemonic) - if err != nil { - return nil, errors.Wrap(err, "could not convert mnemonic to wallet seed") + if err != nil && strings.Contains(err.Error(), "not found in reverse map") { + return nil, errors.New("could not convert mnemonic to wallet seed: invalid seed word entered") + } + if errors.Is(err, bip39.ErrInvalidMnemonic) { + return nil, errors.Wrap(bip39.ErrInvalidMnemonic, "could not convert mnemonic to wallet seed") } encryptor := keystorev4.New() cryptoFields, err := encryptor.Encrypt(walletSeed, []byte(password)) diff --git a/validator/keymanager/v2/derived/mnemonic.go b/validator/keymanager/v2/derived/mnemonic.go index 3d9da01a4754..d1a990a7bd15 100644 --- a/validator/keymanager/v2/derived/mnemonic.go +++ b/validator/keymanager/v2/derived/mnemonic.go @@ -2,6 +2,7 @@ package derived import ( "fmt" + "strings" "github.com/manifoldco/promptui" "github.com/tyler-smith/go-bip39" @@ -55,7 +56,7 @@ func (m *EnglishMnemonicGenerator) ConfirmAcknowledgement(phrase string) error { expected := "y" var result string var err error - for result != expected { + for strings.ToLower(result) != expected { result, err = prompt.Run() if err != nil { log.Errorf("Could not confirm acknowledgement of prompt, please enter y") From 89d1e45572eaefe86553b169447b314ecd95f90b Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Sun, 26 Jul 2020 04:38:39 -0400 Subject: [PATCH 03/13] Add to help --- validator/usage.go | 1 + 1 file changed, 1 insertion(+) diff --git a/validator/usage.go b/validator/usage.go index d62b89a9a5ed..025f51980dd7 100644 --- a/validator/usage.go +++ b/validator/usage.go @@ -83,6 +83,7 @@ var appHelpFlagGroups = []flagGroup{ flags.KeyManagerOpts, flags.KeystorePathFlag, flags.PasswordFlag, + flags.PasswordFileFlag, flags.DisablePenaltyRewardLogFlag, flags.UnencryptedKeysFlag, flags.GraffitiFlag, From 0ce1fa859a25b27749cb4b76a86e99f5a68e762e Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 27 Jul 2020 10:03:07 -0400 Subject: [PATCH 04/13] Fixes --- validator/accounts/v2/accounts_import.go | 17 +++++++++++++++++ validator/accounts/v2/wallet.go | 3 +-- validator/accounts/v2/wallet_create.go | 4 ++-- validator/accounts/v2/wallet_recover.go | 2 +- validator/flags/flags.go | 2 +- 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/validator/accounts/v2/accounts_import.go b/validator/accounts/v2/accounts_import.go index aadfccc991bb..057ed7c65856 100644 --- a/validator/accounts/v2/accounts_import.go +++ b/validator/accounts/v2/accounts_import.go @@ -24,6 +24,23 @@ func ImportAccount(cliCtx *cli.Context) error { 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") + } + 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(), + ) + } + } passwordsDir, err := inputDirectory(cliCtx, passwordsDirPromptText, flags.WalletPasswordsDirFlag) if err != nil { return err diff --git a/validator/accounts/v2/wallet.go b/validator/accounts/v2/wallet.go index 3d4e4b52d2cc..aa4141c176cf 100644 --- a/validator/accounts/v2/wallet.go +++ b/validator/accounts/v2/wallet.go @@ -80,8 +80,7 @@ func NewWallet( } if walletExists { return nil, errors.New( - "you already have a wallet at the specified path. You can " + - "edit your wallet configuration by running ./prysm.sh validator wallet-v2 edit", + , ) } accountsPath := filepath.Join(walletDir, keymanagerKind.String()) diff --git a/validator/accounts/v2/wallet_create.go b/validator/accounts/v2/wallet_create.go index 2a5843961561..11a33fdffe75 100644 --- a/validator/accounts/v2/wallet_create.go +++ b/validator/accounts/v2/wallet_create.go @@ -29,7 +29,7 @@ func CreateWallet(cliCtx *cli.Context) error { if err = createDirectKeymanagerWallet(cliCtx, w); err != nil { return errors.Wrap(err, "could not initialize wallet with direct keymanager") } - log.WithField("wallet-path", w.accountsPath).Infof( + log.WithField("wallet-dir", w.accountsPath).Infof( "Successfully created wallet with on-disk keymanager configuration. " + "Make a new validator account with ./prysm.sh validator accounts-v2 create", ) @@ -37,7 +37,7 @@ func CreateWallet(cliCtx *cli.Context) error { if err = createDerivedKeymanagerWallet(cliCtx, w); err != nil { return errors.Wrap(err, "could not initialize wallet with derived keymanager") } - log.WithField("wallet-path", w.accountsPath).Infof( + log.WithField("wallet-dir", w.accountsPath).Infof( "Successfully created HD wallet and saved configuration to disk. " + "Make a new validator account with ./prysm.sh validator accounts-2 create", ) diff --git a/validator/accounts/v2/wallet_recover.go b/validator/accounts/v2/wallet_recover.go index af47dfdc372a..67ebac9d9aaf 100644 --- a/validator/accounts/v2/wallet_recover.go +++ b/validator/accounts/v2/wallet_recover.go @@ -69,7 +69,7 @@ func inputMnemonic(cliCtx *cli.Context) (string, error) { return enteredMnemonic, nil } prompt := promptui.Prompt{ - Label: "Enter the wallet recovery seed phrase you would like to recover", + Label: "Enter the seed phrase for the wallet you would like to recover", Validate: validateMnemonic, } menmonicPhrase, err := prompt.Run() diff --git a/validator/flags/flags.go b/validator/flags/flags.go index c896fe9ce7df..99bfdd50827c 100644 --- a/validator/flags/flags.go +++ b/validator/flags/flags.go @@ -136,7 +136,7 @@ var ( // PasswordFileFlag is used to enter a file to get a password for new account creation, non-interactively. PasswordFileFlag = &cli.StringFlag{ Name: "password-file", - Usage: "Path to a file containing a password to interact with wallets/accounts in a non-interactive way", + Usage: "Path to a file containing a password in text to interact with wallets/accounts in a non-interactive way", } // MnemonicFileFlag is used to enter a file to mnemonic phrase for new wallet creation, non-interactively. MnemonicFileFlag = &cli.StringFlag{ From 2c345703265b1eaea024241800bf3249399b0eea Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 27 Jul 2020 10:03:21 -0400 Subject: [PATCH 05/13] add back error --- validator/accounts/v2/wallet.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/validator/accounts/v2/wallet.go b/validator/accounts/v2/wallet.go index aa4141c176cf..04c774879999 100644 --- a/validator/accounts/v2/wallet.go +++ b/validator/accounts/v2/wallet.go @@ -80,7 +80,8 @@ func NewWallet( } if walletExists { return nil, errors.New( - , + "you already have a wallet at the specified path. You can " + + "edit your wallet configuration by running ./prysm.sh validator wallet-v2 edit-config", ) } accountsPath := filepath.Join(walletDir, keymanagerKind.String()) From 3d90d2a3fd79af6ee6cfe31746441d06efae01df Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 27 Jul 2020 10:12:28 -0400 Subject: [PATCH 06/13] Make new wallet error cleaner --- validator/accounts/v2/wallet.go | 12 ++++++++---- validator/accounts/v2/wallet_create.go | 5 ++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/validator/accounts/v2/wallet.go b/validator/accounts/v2/wallet.go index 04c774879999..9f2c5614f78b 100644 --- a/validator/accounts/v2/wallet.go +++ b/validator/accounts/v2/wallet.go @@ -46,6 +46,13 @@ const ( DirectoryPermissions = os.ModePerm ) +var ( + // Wallet error vars. + ErrWalletExists = errors.New("you already have a wallet at the specified path. You can " + + "edit your wallet configuration by running ./prysm.sh validator wallet-v2 edit-config", + ) +) + // Wallet is a primitive in Prysm's v2 account management which // has the capability of creating new accounts, reading existing accounts, // and providing secure access to eth2 secrets depending on an @@ -79,10 +86,7 @@ func NewWallet( return nil, errors.Wrap(err, "could not check if wallet exists") } if walletExists { - return nil, errors.New( - "you already have a wallet at the specified path. You can " + - "edit your wallet configuration by running ./prysm.sh validator wallet-v2 edit-config", - ) + return nil, ErrWalletExists } accountsPath := filepath.Join(walletDir, keymanagerKind.String()) w := &Wallet{ diff --git a/validator/accounts/v2/wallet_create.go b/validator/accounts/v2/wallet_create.go index 11a33fdffe75..2e14a6cc5e8d 100644 --- a/validator/accounts/v2/wallet_create.go +++ b/validator/accounts/v2/wallet_create.go @@ -21,9 +21,12 @@ func CreateWallet(cliCtx *cli.Context) error { return err } w, err := NewWallet(cliCtx, keymanagerKind) - if err != nil { + if err != nil && !errors.Is(err, ErrWalletExists) { return errors.Wrap(err, "could not check if wallet directory exists") } + if errors.Is(err, ErrWalletExists) { + return ErrWalletExists + } switch w.KeymanagerKind() { case v2keymanager.Direct: if err = createDirectKeymanagerWallet(cliCtx, w); err != nil { From 1d9841dbe049ae171a45804ae124ca8ad29ef28a Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 27 Jul 2020 10:27:14 -0400 Subject: [PATCH 07/13] Fix text --- validator/accounts/v2/cmd_accounts.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validator/accounts/v2/cmd_accounts.go b/validator/accounts/v2/cmd_accounts.go index d6a25414cc78..7b918c894390 100644 --- a/validator/accounts/v2/cmd_accounts.go +++ b/validator/accounts/v2/cmd_accounts.go @@ -15,7 +15,7 @@ var AccountCommands = &cli.Command{ // AccountCommands for accounts-v2 for Prysm validators. { Name: "create", - Description: `creates a new validator account for eth2. If no account exists at the wallet path, creates a new wallet for a user based on + Description: `creates a new validator account for eth2. If no wallet exists at the given wallet path, creates a new wallet for a user based on specified input, capable of creating a direct, derived, or remote wallet. this command outputs a deposit data string which is required to become a validator in eth2.`, Flags: []cli.Flag{ From a9c05f656e819390b1fb84b76dbdaeada9332e68 Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 27 Jul 2020 10:30:07 -0400 Subject: [PATCH 08/13] Fix log --- validator/accounts/v2/wallet_create.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/validator/accounts/v2/wallet_create.go b/validator/accounts/v2/wallet_create.go index 2e14a6cc5e8d..4058b67e2418 100644 --- a/validator/accounts/v2/wallet_create.go +++ b/validator/accounts/v2/wallet_create.go @@ -32,7 +32,7 @@ func CreateWallet(cliCtx *cli.Context) error { if err = createDirectKeymanagerWallet(cliCtx, w); err != nil { return errors.Wrap(err, "could not initialize wallet with direct keymanager") } - log.WithField("wallet-dir", w.accountsPath).Infof( + log.WithField("wallet-path", w.accountsPath).Infof( "Successfully created wallet with on-disk keymanager configuration. " + "Make a new validator account with ./prysm.sh validator accounts-v2 create", ) @@ -40,7 +40,7 @@ func CreateWallet(cliCtx *cli.Context) error { if err = createDerivedKeymanagerWallet(cliCtx, w); err != nil { return errors.Wrap(err, "could not initialize wallet with derived keymanager") } - log.WithField("wallet-dir", w.accountsPath).Infof( + log.WithField("wallet-path", w.accountsPath).Infof( "Successfully created HD wallet and saved configuration to disk. " + "Make a new validator account with ./prysm.sh validator accounts-2 create", ) From 55c6a9f43d710e679195ba6f6b759e7880d6ab16 Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 27 Jul 2020 10:33:12 -0400 Subject: [PATCH 09/13] Add case for normal error --- validator/keymanager/v2/derived/derived.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/validator/keymanager/v2/derived/derived.go b/validator/keymanager/v2/derived/derived.go index c6a593fc7314..2f47a7529b6e 100644 --- a/validator/keymanager/v2/derived/derived.go +++ b/validator/keymanager/v2/derived/derived.go @@ -211,9 +211,10 @@ func SeedFileFromMnemonic(ctx context.Context, mnemonic string, password string) walletSeed, err := bip39.EntropyFromMnemonic(mnemonic) if err != nil && strings.Contains(err.Error(), "not found in reverse map") { return nil, errors.New("could not convert mnemonic to wallet seed: invalid seed word entered") - } - if errors.Is(err, bip39.ErrInvalidMnemonic) { + } else if errors.Is(err, bip39.ErrInvalidMnemonic) { return nil, errors.Wrap(bip39.ErrInvalidMnemonic, "could not convert mnemonic to wallet seed") + } else if err != nil { + return nil, errors.Wrap(err, "could not convert mnemonic to wallet seed") } encryptor := keystorev4.New() cryptoFields, err := encryptor.Encrypt(walletSeed, []byte(password)) From 3f80b65f652e596e23f6bdf3e1978996146fd494 Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 27 Jul 2020 11:02:14 -0400 Subject: [PATCH 10/13] Update validator/flags/flags.go Co-authored-by: Raul Jordan --- validator/flags/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validator/flags/flags.go b/validator/flags/flags.go index 99bfdd50827c..21335d82d9a5 100644 --- a/validator/flags/flags.go +++ b/validator/flags/flags.go @@ -136,7 +136,7 @@ var ( // PasswordFileFlag is used to enter a file to get a password for new account creation, non-interactively. PasswordFileFlag = &cli.StringFlag{ Name: "password-file", - Usage: "Path to a file containing a password in text to interact with wallets/accounts in a non-interactive way", + Usage: "Path to a plaintext password.txt file", } // MnemonicFileFlag is used to enter a file to mnemonic phrase for new wallet creation, non-interactively. MnemonicFileFlag = &cli.StringFlag{ From 1c01a864c9c308f9eaa737a4d8ff26b65a72bdd6 Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 27 Jul 2020 15:41:46 -0400 Subject: [PATCH 11/13] Fix comment --- validator/accounts/v2/wallet.go | 1 + 1 file changed, 1 insertion(+) diff --git a/validator/accounts/v2/wallet.go b/validator/accounts/v2/wallet.go index 18bc8d3f53f7..0a997fe47f59 100644 --- a/validator/accounts/v2/wallet.go +++ b/validator/accounts/v2/wallet.go @@ -48,6 +48,7 @@ const ( var ( // Wallet error vars. + // ErrWalletExists is an error returned when a wallet already exists in the path provided. ErrWalletExists = errors.New("you already have a wallet at the specified path. You can " + "edit your wallet configuration by running ./prysm.sh validator wallet-v2 edit-config", ) From 7765165bc3fa2c25ee012a3d7dc9fbe39c3f3e79 Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 27 Jul 2020 17:27:29 -0400 Subject: [PATCH 12/13] fix comment --- validator/accounts/v2/wallet.go | 1 - 1 file changed, 1 deletion(-) diff --git a/validator/accounts/v2/wallet.go b/validator/accounts/v2/wallet.go index 0a997fe47f59..39646522a601 100644 --- a/validator/accounts/v2/wallet.go +++ b/validator/accounts/v2/wallet.go @@ -47,7 +47,6 @@ const ( ) var ( - // Wallet error vars. // ErrWalletExists is an error returned when a wallet already exists in the path provided. ErrWalletExists = errors.New("you already have a wallet at the specified path. You can " + "edit your wallet configuration by running ./prysm.sh validator wallet-v2 edit-config", From 501ea3922fd8256c9b2cbbd5ed632df7180d7e4f Mon Sep 17 00:00:00 2001 From: Ivan Martinez Date: Mon, 27 Jul 2020 17:40:28 -0400 Subject: [PATCH 13/13] Fix test --- validator/accounts/v2/testing/mock.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/validator/accounts/v2/testing/mock.go b/validator/accounts/v2/testing/mock.go index f0b2f5723c83..7d928d5c3e93 100644 --- a/validator/accounts/v2/testing/mock.go +++ b/validator/accounts/v2/testing/mock.go @@ -6,6 +6,7 @@ import ( "errors" "io" "io/ioutil" + "strings" "sync" ) @@ -77,7 +78,7 @@ func (m *Wallet) ReadFileAtPath(ctx context.Context, pathName string, fileName s m.lock.RLock() defer m.lock.RUnlock() for f, v := range m.Files[pathName] { - if f == fileName { + if strings.Contains(fileName, f) { return v, nil } }