From bb22dfe02fe3ea5bda44353b87b1777d68d0da45 Mon Sep 17 00:00:00 2001
From: Martin Holst Swende <martin@swende.se>
Date: Mon, 20 Nov 2017 15:42:49 +0100
Subject: [PATCH] accounts/keystore: address review-feedback

---
 accounts/keystore/keystore_passphrase.go | 19 +++----------
 cmd/geth/accountcmd.go                   |  6 ++--
 node/config.go                           | 35 +++++++++++++-----------
 3 files changed, 25 insertions(+), 35 deletions(-)

diff --git a/accounts/keystore/keystore_passphrase.go b/accounts/keystore/keystore_passphrase.go
index 164da653bc..eaec39f7df 100644
--- a/accounts/keystore/keystore_passphrase.go
+++ b/accounts/keystore/keystore_passphrase.go
@@ -91,21 +91,10 @@ func (ks keyStorePassphrase) GetKey(addr common.Address, filename, auth string)
 	return key, nil
 }
 
-func StoreKey(dir, auth string, scryptN, scryptP int) (error, common.Address) {
-	key, err := newKey(crand.Reader)
-	if err != nil {
-		return err, common.Address{}
-	}
-	keyjson, err := EncryptKey(key, auth, scryptN, scryptP)
-	if err != nil {
-		return err, common.Address{}
-	}
-	fullpath := filepath.Join(dir, keyFileName(key.Address))
-	err = writeKeyFile(fullpath, keyjson)
-	if err != nil {
-		return err, common.Address{}
-	}
-	return nil, key.Address
+// StoreKey generates a key, encrypts with 'auth' and stores in the given directory
+func StoreKey(dir, auth string, scryptN, scryptP int) (common.Address, error) {
+	_, a, err := storeNewKey(&keyStorePassphrase{dir, scryptN, scryptP}, crand.Reader, auth)
+	return a.Address, err
 }
 
 func (ks keyStorePassphrase) StoreKey(filename string, key *Key, auth string) error {
diff --git a/cmd/geth/accountcmd.go b/cmd/geth/accountcmd.go
index 30e50a520f..846c679a4a 100644
--- a/cmd/geth/accountcmd.go
+++ b/cmd/geth/accountcmd.go
@@ -26,7 +26,6 @@ import (
 	"github.com/ethereum/go-ethereum/console"
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/log"
-	"github.com/ethereum/go-ethereum/node"
 	"gopkg.in/urfave/cli.v1"
 )
 
@@ -292,7 +291,6 @@ func ambiguousAddrRecovery(ks *keystore.KeyStore, err *keystore.AmbiguousAddrErr
 
 // accountCreate creates a new account into the keystore defined by the CLI flags.
 func accountCreate(ctx *cli.Context) error {
-
 	cfg := gethConfig{}
 	// Load config file.
 	if file := ctx.GlobalString(configFileFlag.Name); file != "" {
@@ -301,7 +299,7 @@ func accountCreate(ctx *cli.Context) error {
 		}
 	}
 	utils.SetNodeConfig(ctx, &cfg.Node)
-	scryptN, scryptP, keydir, err := node.ResolveAccountConfig(&cfg.Node)
+	scryptN, scryptP, keydir, err := cfg.Node.AccountConfig()
 
 	if err != nil {
 		utils.Fatalf("Failed to create account: %v", err)
@@ -309,7 +307,7 @@ func accountCreate(ctx *cli.Context) error {
 
 	password := getPassPhrase("Your new account is locked with a password. Please give a password. Do not forget this password.", true, 0, utils.MakePasswordList(ctx))
 
-	err, address := keystore.StoreKey(keydir, password, scryptN, scryptP)
+	address, err := keystore.StoreKey(keydir, password, scryptN, scryptP)
 
 	if err != nil {
 		utils.Fatalf("Failed to create account: %v", err)
diff --git a/node/config.go b/node/config.go
index 89efe1911c..1ee02d8963 100644
--- a/node/config.go
+++ b/node/config.go
@@ -360,11 +360,11 @@ func (c *Config) parsePersistentNodes(path string) []*discover.Node {
 	return nodes
 }
 
-// ResolveAccountConfig determines the settings for scrypt and keydirectory
-func ResolveAccountConfig(conf *Config) (int, int, string, error) {
+// AccountConfig determines the settings for scrypt and keydirectory
+func (c *Config) AccountConfig() (int, int, string, error) {
 	scryptN := keystore.StandardScryptN
 	scryptP := keystore.StandardScryptP
-	if conf.UseLightweightKDF {
+	if c.UseLightweightKDF {
 		scryptN = keystore.LightScryptN
 		scryptP = keystore.LightScryptP
 	}
@@ -374,25 +374,28 @@ func ResolveAccountConfig(conf *Config) (int, int, string, error) {
 		err    error
 	)
 	switch {
-	case filepath.IsAbs(conf.KeyStoreDir):
-		keydir = conf.KeyStoreDir
-	case conf.DataDir != "":
-		if conf.KeyStoreDir == "" {
-			keydir = filepath.Join(conf.DataDir, datadirDefaultKeyStore)
+	case filepath.IsAbs(c.KeyStoreDir):
+		keydir = c.KeyStoreDir
+	case c.DataDir != "":
+		if c.KeyStoreDir == "" {
+			keydir = filepath.Join(c.DataDir, datadirDefaultKeyStore)
 		} else {
-			keydir, err = filepath.Abs(conf.KeyStoreDir)
+			keydir, err = filepath.Abs(c.KeyStoreDir)
 		}
-	case conf.KeyStoreDir != "":
-		keydir, err = filepath.Abs(conf.KeyStoreDir)
-	default:
-		// There is no datadir.
-		keydir, err = ioutil.TempDir("", "go-ethereum-keystore")
+	case c.KeyStoreDir != "":
+		keydir, err = filepath.Abs(c.KeyStoreDir)
 	}
 	return scryptN, scryptP, keydir, err
 }
 
 func makeAccountManager(conf *Config) (*accounts.Manager, string, error) {
-	scryptN, scryptP, keydir, err := ResolveAccountConfig(conf)
+	scryptN, scryptP, keydir, err := conf.AccountConfig()
+	var ephemeral string
+	if keydir == "" {
+		// There is no datadir.
+		keydir, err = ioutil.TempDir("", "go-ethereum-keystore")
+		ephemeral = keydir
+	}
 
 	if err != nil {
 		return nil, "", err
@@ -418,5 +421,5 @@ func makeAccountManager(conf *Config) (*accounts.Manager, string, error) {
 			backends = append(backends, trezorhub)
 		}
 	}
-	return accounts.NewManager(backends...), keydir, nil
+	return accounts.NewManager(backends...), ephemeral, nil
 }