Skip to content
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 V2: Fix issues reported by rkapka #6725

Merged
merged 25 commits into from
Jul 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
8371721
Fix accounts v2 runtime
0xKiwi Jul 26, 2020
00ddf7a
Accounts V2: Fix issues found by rkapka
0xKiwi Jul 26, 2020
89d1e45
Add to help
0xKiwi Jul 26, 2020
fc5b18a
Merge refs/heads/master into fix-v2-runtime
prylabs-bulldozer[bot] Jul 27, 2020
f48a3a8
Merge refs/heads/master into fix-v2-runtime
prylabs-bulldozer[bot] Jul 27, 2020
9d0badc
Merge refs/heads/master into fix-v2-runtime
prylabs-bulldozer[bot] Jul 27, 2020
28bf3a8
Merge refs/heads/master into fix-v2-runtime
prylabs-bulldozer[bot] Jul 27, 2020
0ce1fa8
Fixes
0xKiwi Jul 27, 2020
2c34570
add back error
0xKiwi Jul 27, 2020
3d90d2a
Make new wallet error cleaner
0xKiwi Jul 27, 2020
1d9841d
Fix text
0xKiwi Jul 27, 2020
a9c05f6
Fix log
0xKiwi Jul 27, 2020
55c6a9f
Add case for normal error
0xKiwi Jul 27, 2020
3f80b65
Update validator/flags/flags.go
0xKiwi Jul 27, 2020
7de0d0d
Merge branch 'master' of github.com:prysmaticlabs/prysm into fix-v2-r…
0xKiwi Jul 27, 2020
3d414e0
Merge refs/heads/master into fix-v2-runtime
prylabs-bulldozer[bot] Jul 27, 2020
1c01a86
Fix comment
0xKiwi Jul 27, 2020
6aac7db
Merge refs/heads/master into fix-v2-runtime
prylabs-bulldozer[bot] Jul 27, 2020
7765165
fix comment
0xKiwi Jul 27, 2020
501ea39
Fix test
0xKiwi Jul 27, 2020
5c6b007
Merge branch 'master' into fix-v2-runtime
rauljordan Jul 28, 2020
e2de281
Merge refs/heads/master into fix-v2-runtime
prylabs-bulldozer[bot] Jul 28, 2020
19db83c
Merge refs/heads/master into fix-v2-runtime
prylabs-bulldozer[bot] Jul 28, 2020
1b61794
Merge refs/heads/master into fix-v2-runtime
prylabs-bulldozer[bot] Jul 28, 2020
d08ff87
Merge refs/heads/master into fix-v2-runtime
prylabs-bulldozer[bot] Jul 28, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions validator/accounts/v2/accounts_import.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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
Expand Down
5 changes: 2 additions & 3 deletions validator/accounts/v2/accounts_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
keystoreFileName, err := wallet.FileNameAtPath(ctx, accountNames[i], direct.KeystoreFileName)
Expand All @@ -105,7 +103,8 @@ func listDirectKeymanagerAccounts(
if err != nil {
return errors.Wrap(err, "could not get timestamp from keystore file name")
}
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])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should abstract this logging into a shared function if possible

if !showDepositData {
continue
}
Expand Down
3 changes: 1 addition & 2 deletions validator/accounts/v2/cmd_accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@ 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{
flags.WalletDirFlag,
flags.WalletPasswordsDirFlag,
flags.PasswordFileFlag,
flags.SkipMnemonicConfirmFlag,
flags.NumAccountsFlag,
featureconfig.AltonaTestnet,
featureconfig.MedallaTestnet,
Expand Down
4 changes: 2 additions & 2 deletions validator/accounts/v2/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,11 @@ func inputPassword(cliCtx *cli.Context, promptText string, confirmPassword passw
if err != nil {
return "", errors.Wrap(err, "could not read password file")
}
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
var walletPassword string
Expand Down
12 changes: 8 additions & 4 deletions validator/accounts/v2/wallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ const (
DirectoryPermissions = os.ModePerm
)

var (
// 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",
)
)

// 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
Expand Down Expand Up @@ -81,10 +88,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",
)
return nil, ErrWalletExists
}
accountsPath := filepath.Join(walletDir, keymanagerKind.String())
w := &Wallet{
Expand Down
7 changes: 5 additions & 2 deletions validator/accounts/v2/wallet_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ func CreateWallet(cliCtx *cli.Context) error {
return err
}
w, err := NewWallet(cliCtx, keymanagerKind)
if err != nil {
return errors.Wrap(err, "could not create a new wallet")
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just returning the wallet exists error here so it doesn't pass through could not check if wallet directory exists

}
switch w.KeymanagerKind() {
case v2keymanager.Direct:
Expand Down
2 changes: 1 addition & 1 deletion validator/accounts/v2/wallet_recover.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,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()
Expand Down
2 changes: 1 addition & 1 deletion validator/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 plaintext password.txt file",
}
// MnemonicFileFlag is used to enter a file to mnemonic phrase for new wallet creation, non-interactively.
MnemonicFileFlag = &cli.StringFlag{
Expand Down
7 changes: 6 additions & 1 deletion validator/keymanager/v2/derived/derived.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"io/ioutil"
"path"
"strconv"
"strings"
"sync"

"github.com/google/uuid"
Expand Down Expand Up @@ -208,7 +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 {
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")
} 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()
Expand Down
3 changes: 2 additions & 1 deletion validator/keymanager/v2/derived/mnemonic.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package derived

import (
"fmt"
"strings"

"github.com/manifoldco/promptui"
"github.com/tyler-smith/go-bip39"
Expand Down Expand Up @@ -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")
Expand Down
1 change: 1 addition & 0 deletions validator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ var appFlags = []cli.Flag{
flags.SlasherRPCProviderFlag,
flags.SlasherCertFlag,
flags.WalletPasswordsDirFlag,
flags.PasswordFileFlag,
flags.WalletDirFlag,
cmd.MinimalConfigFlag,
cmd.E2EConfigFlag,
Expand Down
9 changes: 0 additions & 9 deletions validator/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"io/ioutil"
"os"
"os/signal"
"path"
"strings"
"sync"
"syscall"
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions validator/usage.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ var appHelpFlagGroups = []flagGroup{
flags.KeyManagerOpts,
flags.KeystorePathFlag,
flags.PasswordFlag,
flags.PasswordFileFlag,
flags.DisablePenaltyRewardLogFlag,
flags.UnencryptedKeysFlag,
flags.GraffitiFlag,
Expand Down