From 1ef39a6e30b6cb92935354b7158e87c38a4f46be Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Wed, 25 Jan 2023 09:34:17 +0300 Subject: [PATCH 1/2] cli: properly handle --name and password-related wallet commands Fetch account's name from the CLI argument and do not ask for the user's input if provided, close #2882. Fetch password from config if provided, close #2883. --- cli/wallet/wallet.go | 86 +++++++++++++++++++++++++++------------ cli/wallet/wallet_test.go | 69 ++++++++++++++++++++++++++----- 2 files changed, 119 insertions(+), 36 deletions(-) diff --git a/cli/wallet/wallet.go b/cli/wallet/wallet.go index bc89e05b6..50e852a70 100644 --- a/cli/wallet/wallet.go +++ b/cli/wallet/wallet.go @@ -519,7 +519,7 @@ loop: } func importMultisig(ctx *cli.Context) error { - wall, _, err := openWallet(ctx, true) + wall, pass, err := openWallet(ctx, true) if err != nil { return cli.NewExitError(err, 1) } @@ -540,7 +540,12 @@ func importMultisig(ctx *cli.Context) error { } } - acc, err := newAccountFromWIF(ctx.App.Writer, ctx.String("wif"), wall.Scrypt) + var label *string + if ctx.IsSet("name") { + l := ctx.String("name") + label = &l + } + acc, err := newAccountFromWIF(ctx.App.Writer, ctx.String("wif"), wall.Scrypt, label, pass) if err != nil { return cli.NewExitError(err, 1) } @@ -549,9 +554,6 @@ func importMultisig(ctx *cli.Context) error { return cli.NewExitError(err, 1) } - if acc.Label == "" { - acc.Label = ctx.String("name") - } if err := addAccountAndSave(wall, acc); err != nil { return cli.NewExitError(err, 1) } @@ -563,7 +565,7 @@ func importDeployed(ctx *cli.Context) error { if err := cmdargs.EnsureNone(ctx); err != nil { return err } - wall, _, err := openWallet(ctx, true) + wall, pass, err := openWallet(ctx, true) if err != nil { return cli.NewExitError(err, 1) } @@ -574,7 +576,12 @@ func importDeployed(ctx *cli.Context) error { return cli.NewExitError("contract hash was not provided", 1) } - acc, err := newAccountFromWIF(ctx.App.Writer, ctx.String("wif"), wall.Scrypt) + var label *string + if ctx.IsSet("name") { + l := ctx.String("name") + label = &l + } + acc, err := newAccountFromWIF(ctx.App.Writer, ctx.String("wif"), wall.Scrypt, label, pass) if err != nil { return cli.NewExitError(err, 1) } @@ -606,9 +613,6 @@ func importDeployed(ctx *cli.Context) error { } acc.Contract.Deployed = true - if acc.Label == "" { - acc.Label = ctx.String("name") - } if err := addAccountAndSave(wall, acc); err != nil { return cli.NewExitError(err, 1) } @@ -620,13 +624,19 @@ func importWallet(ctx *cli.Context) error { if err := cmdargs.EnsureNone(ctx); err != nil { return err } - wall, _, err := openWallet(ctx, true) + wall, pass, err := openWallet(ctx, true) if err != nil { return cli.NewExitError(err, 1) } defer wall.Close() - acc, err := newAccountFromWIF(ctx.App.Writer, ctx.String("wif"), wall.Scrypt) + var label *string + if ctx.IsSet("name") { + l := ctx.String("name") + label = &l + } + + acc, err := newAccountFromWIF(ctx.App.Writer, ctx.String("wif"), wall.Scrypt, label, pass) if err != nil { return cli.NewExitError(err, 1) } @@ -639,9 +649,6 @@ func importWallet(ctx *cli.Context) error { acc.Contract.Script = ctr } - if acc.Label == "" { - acc.Label = ctx.String("name") - } if err := addAccountAndSave(wall, acc); err != nil { return cli.NewExitError(err, 1) } @@ -843,7 +850,7 @@ func createWallet(ctx *cli.Context) error { } func readAccountInfo() (string, string, error) { - name, err := input.ReadLine("Enter the name of the account > ") + name, err := readAccountName() if err != nil { return "", "", err } @@ -854,6 +861,10 @@ func readAccountInfo() (string, string, error) { return name, phrase, nil } +func readAccountName() (string, error) { + return input.ReadLine("Enter the name of the account > ") +} + func readNewPassword() (string, error) { phrase, err := input.ReadPassword(EnterNewPasswordPrompt) if err != nil { @@ -966,16 +977,33 @@ func ReadWalletConfig(configPath string) (*config.Wallet, error) { return cfg, nil } -func newAccountFromWIF(w io.Writer, wif string, scrypt keys.ScryptParams) (*wallet.Account, error) { +func newAccountFromWIF(w io.Writer, wif string, scrypt keys.ScryptParams, label *string, pass *string) (*wallet.Account, error) { + var ( + phrase, name string + err error + ) + if pass != nil { + phrase = *pass + } + if label != nil { + name = *label + } // note: NEP2 strings always have length of 58 even though // base58 strings can have different lengths even if slice lengths are equal if len(wif) == 58 { - pass, err := input.ReadPassword(EnterPasswordPrompt) - if err != nil { - return nil, fmt.Errorf("Error reading password: %w", err) + if pass == nil { + phrase, err = input.ReadPassword(EnterPasswordPrompt) + if err != nil { + return nil, fmt.Errorf("error reading password: %w", err) + } } - return wallet.NewAccountFromEncryptedWIF(wif, pass, scrypt) + acc, err := wallet.NewAccountFromEncryptedWIF(wif, phrase, scrypt) + if err != nil { + return nil, err + } + acc.Label = name + return acc, nil } acc, err := wallet.NewAccountFromWIF(wif) @@ -984,13 +1012,21 @@ func newAccountFromWIF(w io.Writer, wif string, scrypt keys.ScryptParams) (*wall } fmt.Fprintln(w, "Provided WIF was unencrypted. Wallet can contain only encrypted keys.") - name, pass, err := readAccountInfo() - if err != nil { - return nil, err + if label == nil { + name, err = readAccountName() + if err != nil { + return nil, fmt.Errorf("failed to read account label: %w", err) + } + } + if pass == nil { + phrase, err = readNewPassword() + if err != nil { + return nil, fmt.Errorf("failed to read new password: %w", err) + } } acc.Label = name - if err := acc.Encrypt(pass, scrypt); err != nil { + if err := acc.Encrypt(phrase, scrypt); err != nil { return nil, err } diff --git a/cli/wallet/wallet_test.go b/cli/wallet/wallet_test.go index 27bb4ec98..36b9c5204 100644 --- a/cli/wallet/wallet_test.go +++ b/cli/wallet/wallet_test.go @@ -318,7 +318,7 @@ func TestWalletInit(t *testing.T) { configPath := filepath.Join(tmp, "config.yaml") cfg := config.Wallet{ Path: walletPath, - Password: "pass", // This pass won't be taken into account. + Password: "qwerty", } res, err := yaml.Marshal(cfg) require.NoError(t, err) @@ -326,12 +326,26 @@ func TestWalletInit(t *testing.T) { priv, err = keys.NewPrivateKey() require.NoError(t, err) e.In.WriteString("test_account_4\r") - e.In.WriteString("qwerty\r") - e.In.WriteString("qwerty\r") e.Run(t, "neo-go", "wallet", "import", "--wallet-config", configPath, "--wif", priv.WIF(), "--contract", "0a0b0c0d") check(t, "test_account_4", "qwerty") }) + t.Run("from wallet config with account name argument", func(t *testing.T) { + tmp := t.TempDir() + configPath := filepath.Join(tmp, "config.yaml") + cfg := config.Wallet{ + Path: walletPath, + Password: "qwerty", + } + res, err := yaml.Marshal(cfg) + require.NoError(t, err) + require.NoError(t, os.WriteFile(configPath, res, 0666)) + priv, err = keys.NewPrivateKey() + require.NoError(t, err) + e.Run(t, "neo-go", "wallet", "import", + "--wallet-config", configPath, "--wif", priv.WIF(), "--contract", "0a0b0c0d", "--name", "test_account_5") + check(t, "test_account_5", "qwerty") + }) }) }) t.Run("EncryptedWIF", func(t *testing.T) { @@ -586,21 +600,25 @@ func TestWalletImportDeployed(t *testing.T) { e.In.WriteString("acc\rpass\rpass\r") e.Run(t, "neo-go", "wallet", "import-deployed", "--rpc-endpoint", "http://"+e.RPC.Addresses()[0], - "--wallet", walletPath, "--wif", priv.WIF(), "--name", "my_acc", + "--wallet", walletPath, "--wif", priv.WIF(), "--contract", h.StringLE()) - w, err := wallet.NewWalletFromFile(walletPath) - require.NoError(t, err) - require.Equal(t, 1, len(w.Accounts)) - contractAddr := w.Accounts[0].Address - require.Equal(t, address.Uint160ToString(h), contractAddr) - require.True(t, w.Accounts[0].Contract.Deployed) + contractAddr := address.Uint160ToString(h) + checkDeployed := func(t *testing.T) { + w, err := wallet.NewWalletFromFile(walletPath) + require.NoError(t, err) + require.Equal(t, 1, len(w.Accounts)) + actualAddr := w.Accounts[0].Address + require.Equal(t, contractAddr, actualAddr) + require.True(t, w.Accounts[0].Contract.Deployed) + } + checkDeployed(t) t.Run("re-importing", func(t *testing.T) { e.In.WriteString("acc\rpass\rpass\r") e.RunWithError(t, "neo-go", "wallet", "import-deployed", "--rpc-endpoint", "http://"+e.RPC.Addresses()[0], - "--wallet", walletPath, "--wif", priv.WIF(), "--name", "my_acc", + "--wallet", walletPath, "--wif", priv.WIF(), "--contract", h.StringLE()) }) @@ -630,6 +648,35 @@ func TestWalletImportDeployed(t *testing.T) { b, _ = e.Chain.GetGoverningTokenBalance(privTo.GetScriptHash()) require.Equal(t, big.NewInt(1), b) }) + + t.Run("import with name argument", func(t *testing.T) { + e.Run(t, "neo-go", "wallet", "remove", + "--wallet", walletPath, "--address", address.Uint160ToString(h), "--force") + e.In.WriteString("pass\rpass\r") + e.Run(t, "neo-go", "wallet", "import-deployed", + "--rpc-endpoint", "http://"+e.RPC.Addresses()[0], + "--wallet", walletPath, "--wif", priv.WIF(), + "--contract", h.StringLE(), "--name", "acc") + checkDeployed(t) + }) + + t.Run("import with name argument and wallet config", func(t *testing.T) { + e.Run(t, "neo-go", "wallet", "remove", + "--wallet", walletPath, "--address", address.Uint160ToString(h), "--force") + configPath := filepath.Join(t.TempDir(), "wallet-config.yaml") + cfg := &config.Wallet{ + Path: walletPath, + Password: "pass", + } + bytes, err := yaml.Marshal(cfg) + require.NoError(t, err) + require.NoError(t, os.WriteFile(configPath, bytes, os.ModePerm)) + e.Run(t, "neo-go", "wallet", "import-deployed", + "--rpc-endpoint", "http://"+e.RPC.Addresses()[0], + "--wallet-config", configPath, "--wif", priv.WIF(), + "--contract", h.StringLE(), "--name", "acc") + checkDeployed(t) + }) } func TestStripKeys(t *testing.T) { From b77c2846ee297c4b2450e89a17a0310df1619814 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Wed, 25 Jan 2023 09:51:54 +0300 Subject: [PATCH 2/2] cli: allow to request NEP2 password from input In case if wrong password was provided via config file we allow the user to provide the password manually. --- cli/wallet/wallet.go | 14 +++++++++++++- cli/wallet/wallet_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/cli/wallet/wallet.go b/cli/wallet/wallet.go index 50e852a70..212092f47 100644 --- a/cli/wallet/wallet.go +++ b/cli/wallet/wallet.go @@ -1000,7 +1000,19 @@ func newAccountFromWIF(w io.Writer, wif string, scrypt keys.ScryptParams, label acc, err := wallet.NewAccountFromEncryptedWIF(wif, phrase, scrypt) if err != nil { - return nil, err + // If password from wallet config wasn't OK then retry with the user input, + // see the https://github.com/nspcc-dev/neo-go/issues/2883#issuecomment-1399923088. + if pass == nil { + return nil, err + } + phrase, err = input.ReadPassword(EnterPasswordPrompt) + if err != nil { + return nil, fmt.Errorf("error reading password: %w", err) + } + acc, err = wallet.NewAccountFromEncryptedWIF(wif, phrase, scrypt) + if err != nil { + return nil, err + } } acc.Label = name return acc, nil diff --git a/cli/wallet/wallet_test.go b/cli/wallet/wallet_test.go index 36b9c5204..7425237e9 100644 --- a/cli/wallet/wallet_test.go +++ b/cli/wallet/wallet_test.go @@ -369,6 +369,41 @@ func TestWalletInit(t *testing.T) { require.NotNil(t, actual) require.NoError(t, actual.Decrypt("somepass", w.Scrypt)) }) + t.Run("EncryptedWIF with wallet config", func(t *testing.T) { + pass := "somepass" + check := func(t *testing.T, configPass string, needUserPass bool) { + acc, err := wallet.NewAccount() + require.NoError(t, acc.Encrypt(pass, keys.NEP2ScryptParams())) + configPath := filepath.Join(t.TempDir(), "wallet-config.yaml") + require.NoError(t, err) + cfg := &config.Wallet{ + Path: walletPath, + Password: configPass, + } + bytes, err := yaml.Marshal(cfg) + require.NoError(t, err) + require.NoError(t, os.WriteFile(configPath, bytes, os.ModePerm)) + + if needUserPass { + e.In.WriteString(pass + "\r") + } + e.Run(t, "neo-go", "wallet", "import", "--wallet-config", configPath, + "--wif", acc.EncryptedWIF) + + w, err := wallet.NewWalletFromFile(walletPath) + require.NoError(t, err) + actual := w.GetAccount(acc.PrivateKey().GetScriptHash()) + require.NotNil(t, actual) + require.NoError(t, actual.Decrypt(pass, w.Scrypt)) + } + t.Run("config password mismatch", func(t *testing.T) { + check(t, pass+"badpass", true) + }) + + t.Run("good config password", func(t *testing.T) { + check(t, pass, false) + }) + }) t.Run("Multisig", func(t *testing.T) { t.Run("missing wallet", func(t *testing.T) { e.RunWithError(t, "neo-go", "wallet", "import-multisig")