Skip to content

Commit

Permalink
Fix accounts-v2 deposit and related bugs (#7219)
Browse files Browse the repository at this point in the history
  • Loading branch information
dv8silencer authored Sep 14, 2020
1 parent cebb629 commit f31f495
Show file tree
Hide file tree
Showing 5 changed files with 226 additions and 11 deletions.
1 change: 1 addition & 0 deletions validator/accounts/v2/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ go_test(
"accounts_backup_test.go",
"accounts_create_test.go",
"accounts_delete_test.go",
"accounts_deposit_test.go",
"accounts_exit_test.go",
"accounts_import_test.go",
"accounts_list_test.go",
Expand Down
20 changes: 17 additions & 3 deletions validator/accounts/v2/accounts_deposit.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ func createDepositConfig(cliCtx *cli.Context, km *derived.Keymanager) (*derived.
return nil, errors.Wrap(err, "could not parse BLS public key")
}
}
// Allow the user to interactively select the accounts to backup or optionally
// Allow the user to interactively select the accounts to deposit or optionally
// provide them via cli flags as a string of comma-separated, hex strings. If the user has
// selected to deposit all accounts, we skip this part.
if !cliCtx.IsSet(flags.DepositPublicKeysFlag.Name) {
if !cliCtx.IsSet(flags.DepositAllAccountsFlag.Name) {
pubKeys, err = filterPublicKeysFromUserInput(
cliCtx,
flags.DepositPublicKeysFlag,
Expand Down Expand Up @@ -137,6 +137,20 @@ func createDepositConfig(cliCtx *cli.Context, km *derived.Keymanager) (*derived.
return nil, err
}
config.Eth1PrivateKey = strings.TrimRight(string(fileBytes), "\r\n")
} else {
config.Eth1KeystoreUTCFile = cliCtx.String(flags.Eth1KeystoreUTCPathFlag.Name)
if cliCtx.IsSet(flags.Eth1KeystorePasswordFileFlag.Name) {
config.Eth1KeystorePasswordFile = cliCtx.String(flags.Eth1KeystorePasswordFileFlag.Name)
} else {
config.Eth1KeystorePasswordFile, err = inputWeakPassword(
cliCtx,
flags.Eth1KeystorePasswordFileFlag,
"Enter the file path of a text file containing your eth1 keystore password",
)
if err != nil {
return nil, errors.Wrap(err, "could not read eth1 keystore password file path")
}
}
}
return config, nil
}
Expand Down Expand Up @@ -179,7 +193,7 @@ func createDepositConfig(cliCtx *cli.Context, km *derived.Keymanager) (*derived.
eth1KeystorePasswordFile, err := inputWeakPassword(
cliCtx,
flags.Eth1KeystorePasswordFileFlag,
"Enter the file path a .txt file containing your eth1 keystore password",
"Enter the file path to a text file containing your eth1 keystore password",
)
if err != nil {
return nil, errors.Wrap(err, "could not read eth1 keystore password file path")
Expand Down
201 changes: 201 additions & 0 deletions validator/accounts/v2/accounts_deposit_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
package v2

import (
"encoding/hex"
"errors"
"flag"
"io/ioutil"
"os"
"strconv"
"strings"
"testing"

"github.com/prysmaticlabs/prysm/shared/bytesutil"
"github.com/prysmaticlabs/prysm/shared/testutil/assert"
"github.com/prysmaticlabs/prysm/shared/testutil/require"
"github.com/prysmaticlabs/prysm/validator/flags"
v2keymanager "github.com/prysmaticlabs/prysm/validator/keymanager/v2"
"github.com/prysmaticlabs/prysm/validator/keymanager/v2/derived"
"github.com/urfave/cli/v2"
)

type depositTestWalletConfig struct {
walletDir string
walletPasswordFile string
eth1KeystoreFile string
eth1KeystorePasswordFile string
eth1PrivateKeyFile string
httpWeb3ProviderFlag string
publicKeysFlag string
depositAllAccountsFlag bool
skipDepositConfirmationFlag bool
keymanagerKind v2keymanager.Kind
}

func setupWalletCtxforDeposits(
t *testing.T,
cfg *depositTestWalletConfig,
) *cli.Context {
app := cli.App{}
set := flag.NewFlagSet("test", 0)
set.String(flags.WalletDirFlag.Name, cfg.walletDir, "")
set.String(flags.KeymanagerKindFlag.Name, cfg.keymanagerKind.String(), "")
set.String(flags.WalletPasswordFileFlag.Name, cfg.walletPasswordFile, "")
set.String(flags.HTTPWeb3ProviderFlag.Name, cfg.httpWeb3ProviderFlag, "")
set.String(flags.Eth1KeystoreUTCPathFlag.Name, cfg.eth1KeystoreFile, "")
set.String(flags.Eth1KeystorePasswordFileFlag.Name, cfg.eth1KeystorePasswordFile, "")
set.String(flags.Eth1PrivateKeyFileFlag.Name, cfg.eth1PrivateKeyFile, "")
set.String(flags.DepositPublicKeysFlag.Name, cfg.publicKeysFlag, "")
set.Bool(flags.DepositAllAccountsFlag.Name, cfg.depositAllAccountsFlag, "")
set.Bool(flags.SkipDepositConfirmationFlag.Name, cfg.skipDepositConfirmationFlag, "")
assert.NoError(t, set.Set(flags.WalletDirFlag.Name, cfg.walletDir))
assert.NoError(t, set.Set(flags.KeymanagerKindFlag.Name, cfg.keymanagerKind.String()))
assert.NoError(t, set.Set(flags.WalletPasswordFileFlag.Name, cfg.walletPasswordFile))
assert.NoError(t, set.Set(flags.HTTPWeb3ProviderFlag.Name, cfg.httpWeb3ProviderFlag))
if cfg.eth1KeystoreFile != "" {
assert.NoError(t, set.Set(flags.Eth1KeystoreUTCPathFlag.Name, cfg.eth1KeystoreFile))
assert.NoError(t, set.Set(flags.Eth1KeystorePasswordFileFlag.Name, cfg.eth1KeystorePasswordFile))
}
if cfg.eth1PrivateKeyFile != "" {
assert.NoError(t, set.Set(flags.Eth1PrivateKeyFileFlag.Name, cfg.eth1PrivateKeyFile))
}
if cfg.publicKeysFlag != "" {
assert.NoError(t, set.Set(flags.DepositPublicKeysFlag.Name, cfg.publicKeysFlag))
}
if cfg.depositAllAccountsFlag == true {
assert.NoError(t, set.Set(flags.DepositAllAccountsFlag.Name, strconv.FormatBool(cfg.depositAllAccountsFlag)))
}
assert.NoError(t, set.Set(flags.SkipDepositConfirmationFlag.Name, strconv.FormatBool(cfg.skipDepositConfirmationFlag)))
return cli.NewContext(&app, set, nil)
}

func TestCreateDepositConfig(t *testing.T) {
walletDir, _, passwordFilePath := setupWalletAndPasswordsDir(t)

// First, create the wallet and several accounts
cliCtx := setupWalletCtx(t, &testWalletConfig{
keymanagerKind: v2keymanager.Derived,
walletDir: walletDir,
walletPasswordFile: passwordFilePath,
skipDepositConfirm: true,
})
wallet, err := CreateAndSaveWalletCli(cliCtx)
require.NoError(t, err)

err = CreateAccount(cliCtx.Context, &CreateAccountConfig{
Wallet: wallet,
NumAccounts: 3,
})
require.NoError(t, err)
keymanager, err := wallet.InitializeKeymanager(
cliCtx.Context,
true, /* skip mnemonic confirm */
)
require.NoError(t, err)

// Save public keys for comparison and selection purposes later
pubkeys, err := keymanager.FetchValidatingPublicKeys(cliCtx.Context)
require.NoError(t, err)
var hexPubkeys []string
for _, pubkey := range pubkeys {
encoded := make([]byte, hex.EncodedLen(len(pubkey)))
hex.Encode(encoded, pubkey[:])
hexPubkeys = append(hexPubkeys, string(encoded))
}
// Remove the last key so that we can select a portion and not all of the accounts later
hexPubkeys = hexPubkeys[:len(hexPubkeys)-1]
hexPubkeysString := strings.Join(hexPubkeys, ",")

// Create a file holding the test ETH1 private key
eth1PrivateKeyFile, err := ioutil.TempFile("", "testing")
require.NoError(t, err)
defer func() {
err = eth1PrivateKeyFile.Close()
require.NoError(t, err)
err = os.Remove(eth1PrivateKeyFile.Name())
require.NoError(t, err)
}()
_, err = eth1PrivateKeyFile.WriteString("This should be an ETH1 private key")
require.NoError(t, err)

// First we test the behavior when depositAllAccountsFlag is set to true
depositConfig := createDepositConfigHelper(t, &depositTestWalletConfig{
keymanagerKind: v2keymanager.Derived,
walletDir: walletDir,
walletPasswordFile: passwordFilePath,
skipDepositConfirmationFlag: true,
depositAllAccountsFlag: true,
httpWeb3ProviderFlag: "http://localhost:8545",
eth1PrivateKeyFile: eth1PrivateKeyFile.Name(),
})

require.Equal(t, 3, len(depositConfig.DepositPublicKeys), "wrong number of public keys")
require.Equal(t, "This should be an ETH1 private key", depositConfig.Eth1PrivateKey, "eth1 private key does not match")
require.Equal(t, "http://localhost:8545", depositConfig.Web3Provider, "web3 provider does not match")
require.Equal(t, "", depositConfig.Eth1KeystoreUTCFile, "keystore file path should be empty")
require.Equal(t, "", depositConfig.Eth1KeystorePasswordFile, "keystore password file path should be empty")

// Test the case of providing the public keys via command-line. We also pass in the test eth1 private key file.
// hexPubkeysString holds 1 less than all the accounts.
depositConfig = createDepositConfigHelper(t, &depositTestWalletConfig{
keymanagerKind: v2keymanager.Derived,
walletDir: walletDir,
walletPasswordFile: passwordFilePath,
skipDepositConfirmationFlag: true,
publicKeysFlag: hexPubkeysString,
httpWeb3ProviderFlag: "http://localhost:8545",
eth1PrivateKeyFile: eth1PrivateKeyFile.Name(),
})
require.Equal(t, 2, len(depositConfig.DepositPublicKeys), "wrong number of public keys")

// Compare the keys in the config object with the keys we obtained earlier from the keymanager
for keyNum, configPubKey := range depositConfig.DepositPublicKeys {
for index, eachByte := range bytesutil.ToBytes48(configPubKey.Marshal()) {
if eachByte != pubkeys[keyNum][index] {
require.NoError(t, errors.New("public keys do not match"))
}
}
}

// Now we test when private key file is not provided but rather the keystore and keystore password files
depositConfig = createDepositConfigHelper(t, &depositTestWalletConfig{
keymanagerKind: v2keymanager.Derived,
walletDir: walletDir,
walletPasswordFile: passwordFilePath,
skipDepositConfirmationFlag: true,
depositAllAccountsFlag: true,
httpWeb3ProviderFlag: "http://localhost:8545",
eth1KeystoreFile: "This would be eth1 keystore file path",
eth1KeystorePasswordFile: "This would be eth1 keystore password file path",
})
require.Equal(t, 3, len(depositConfig.DepositPublicKeys), "wrong number of public keys")
require.Equal(t, "This would be eth1 keystore file path", depositConfig.Eth1KeystoreUTCFile,
"eth1 keystore file path incorrect")
require.Equal(t, "This would be eth1 keystore password file path", depositConfig.Eth1KeystorePasswordFile,
"eth1 keystore password file path incorrect")
require.Equal(t, "", depositConfig.Eth1PrivateKey, "eth1 private key should be empty string")
}

// createDepositConfigHelper returns a SendDepositConfig when given a particular wallet configuration.
func createDepositConfigHelper(t *testing.T, config *depositTestWalletConfig) *derived.SendDepositConfig {
cliCtx := setupWalletCtxforDeposits(t, config)
wallet, err := OpenWalletOrElseCli(cliCtx, func(cliCtx *cli.Context) (*Wallet, error) {
err := errors.New("could not open wallet")
require.NoError(t, err)
return nil, err
})
require.NoError(t, err)

keymanager, err := wallet.InitializeKeymanager(
cliCtx.Context,
true, /* skip mnemonic confirm */
)
require.NoError(t, err)
km, ok := keymanager.(*derived.Keymanager)
require.Equal(t, true, ok, "keymanager must be derived type")

// Now we finally call the function we are testing.
depositConfig, err := createDepositConfig(cliCtx, km)
require.NoError(t, err)
return depositConfig
}
11 changes: 3 additions & 8 deletions validator/accounts/v2/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package v2

import (
"fmt"
"io/ioutil"
"os"
"strings"

Expand Down Expand Up @@ -61,17 +60,13 @@ func inputWeakPassword(cliCtx *cli.Context, passwordFileFlag *cli.StringFlag, pr
if err != nil {
return "", errors.Wrap(err, "could not determine absolute path of password file")
}
data, err := ioutil.ReadFile(passwordFilePath)
if err != nil {
return "", errors.Wrap(err, "could not read password file")
}
return strings.TrimRight(string(data), "\r\n"), nil
return passwordFilePath, nil
}
walletPassword, err := promptutil.PasswordPrompt(promptText, promptutil.NotEmpty)
walletPasswordFilePath, err := promptutil.PasswordPrompt(promptText, promptutil.NotEmpty)
if err != nil {
return "", fmt.Errorf("could not read account password: %v", err)
}
return walletPassword, nil
return walletPasswordFilePath, nil
}

func inputRemoteKeymanagerConfig(cliCtx *cli.Context) (*remote.KeymanagerOpts, error) {
Expand Down
4 changes: 4 additions & 0 deletions validator/accounts/v2/wallet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type testWalletConfig struct {
walletPasswordFile string
accountPasswordFile string
privateKeyFile string
skipDepositConfirm bool
numAccounts int64
keymanagerKind v2keymanager.Kind
}
Expand All @@ -71,6 +72,8 @@ func setupWalletCtx(
set.String(flags.WalletPasswordFileFlag.Name, cfg.walletPasswordFile, "")
set.String(flags.AccountPasswordFileFlag.Name, cfg.accountPasswordFile, "")
set.Int64(flags.NumAccountsFlag.Name, cfg.numAccounts, "")
set.Bool(flags.SkipDepositConfirmationFlag.Name, cfg.skipDepositConfirm, "")

if cfg.privateKeyFile != "" {
set.String(flags.ImportPrivateKeyFileFlag.Name, cfg.privateKeyFile, "")
assert.NoError(tb, set.Set(flags.ImportPrivateKeyFileFlag.Name, cfg.privateKeyFile))
Expand All @@ -87,6 +90,7 @@ func setupWalletCtx(
assert.NoError(tb, set.Set(flags.WalletPasswordFileFlag.Name, cfg.walletPasswordFile))
assert.NoError(tb, set.Set(flags.AccountPasswordFileFlag.Name, cfg.accountPasswordFile))
assert.NoError(tb, set.Set(flags.NumAccountsFlag.Name, strconv.Itoa(int(cfg.numAccounts))))
assert.NoError(tb, set.Set(flags.SkipDepositConfirmationFlag.Name, strconv.FormatBool(cfg.skipDepositConfirm)))
return cli.NewContext(&app, set, nil)
}

Expand Down

0 comments on commit f31f495

Please sign in to comment.