accounts: cache key addresses

In order to avoid disk thrashing for Accounts and HasAccount,
address->key file mappings are now cached in memory. This makes it no
longer necessary to keep the key address in the file name. The address
of each key is derived from file content instead.

There are minor user-visible changes:

- "geth account list" now reports key file paths alongside the address.
- If multiple keys are present for an address, unlocking by address is
  not possible. Users are directed to remove the duplicate files
  instead. Unlocking by index is still possible.
- Key files are overwritten written in place when updating the password.

accounts/account_manager.go

@@ -22,8 +22,12 @@ package accounts
 import (
 	"crypto/ecdsa"
 	crand "crypto/rand"
+	"encoding/json"
 	"errors"
 	"fmt"
+	"os"
+	"path/filepath"
+	"runtime"
 	"sync"
 	"time"
 
@@ -32,22 +36,28 @@ import (
 )
 
 var (
-	ErrLocked = errors.New("account is locked")
-	ErrNoKeys = errors.New("no keys in store")
+	ErrLocked  = errors.New("account is locked")
+	ErrNoMatch = errors.New("no key for given address or file")
 )
 
 type Account struct {
 	Address common.Address
+	File    string
 }
 
 func (acc *Account) MarshalJSON() ([]byte, error) {
 	return []byte(`"` + acc.Address.Hex() + `"`), nil
 }
 
+func (acc *Account) UnmarshalJSON(raw []byte) error {
+	return json.Unmarshal(raw, &acc.Address)
+}
+
 type Manager struct {
+	cache    *addrCache
 	keyStore keyStore
+	mu       sync.RWMutex
 	unlocked map[common.Address]*unlocked
-	mutex    sync.RWMutex
 }
 
 type unlocked struct {
@@ -56,36 +66,62 @@ type unlocked struct {
 }
 
 func NewManager(keydir string, scryptN, scryptP int) *Manager {
-	return &Manager{
-		keyStore: newKeyStorePassphrase(keydir, scryptN, scryptP),
-		unlocked: make(map[common.Address]*unlocked),
-	}
+	keydir, _ = filepath.Abs(keydir)
+	am := &Manager{keyStore: &keyStorePassphrase{keydir, scryptN, scryptP}}
+	am.init(keydir)
+	return am
 }
 
 func NewPlaintextManager(keydir string) *Manager {
-	return &Manager{
-		keyStore: newKeyStorePlain(keydir),
-		unlocked: make(map[common.Address]*unlocked),
-	}
+	keydir, _ = filepath.Abs(keydir)
+	am := &Manager{keyStore: &keyStorePlain{keydir}}
+	am.init(keydir)
+	return am
+}
+
+func (am *Manager) init(keydir string) {
+	am.unlocked = make(map[common.Address]*unlocked)
+	am.cache = newAddrCache(keydir)
+	// TODO: In order for this finalizer to work, there must be no references
+	// to am. addrCache doesn't keep a reference but unlocked keys do,
+	// so the finalizer will not trigger until all timed unlocks have expired.
+	runtime.SetFinalizer(am, func(m *Manager) {
+		m.cache.close()
+	})
 }
 
 func (am *Manager) HasAddress(addr common.Address) bool {
-	accounts := am.Accounts()
-	for _, acct := range accounts {
-		if acct.Address == addr {
-			return true
-		}
-	}
-	return false
+	return am.cache.hasAddress(addr)
+}
+
+func (am *Manager) Accounts() []Account {
+	return am.cache.accounts()
 }
 
 func (am *Manager) DeleteAccount(a Account, auth string) error {
-	return am.keyStore.DeleteKey(a.Address, auth)
+	// Decrypting the key isn't really necessary, but we do
+	// it anyway to check the password and zero out the key
+	// immediately afterwards.
+	a, key, err := am.getDecryptedKey(a, auth)
+	if key != nil {
+		zeroKey(key.PrivateKey)
+	}
+	if err != nil {
+		return err
+	}
+	// The order is crucial here. The key is dropped from the
+	// cache after the file is gone so that a reload happening in
+	// between won't insert it into the cache again.
+	err = os.Remove(a.File)
+	if err == nil {
+		am.cache.delete(a)
+	}
+	return err
 }
 
 func (am *Manager) Sign(a Account, toSign []byte) (signature []byte, err error) {
-	am.mutex.RLock()
-	defer am.mutex.RUnlock()
+	am.mu.RLock()
+	defer am.mu.RUnlock()
 	unlockedKey, found := am.unlocked[a.Address]
 	if !found {
 		return nil, ErrLocked
@@ -100,12 +136,12 @@ func (am *Manager) Unlock(a Account, keyAuth string) error {
 }
 
 func (am *Manager) Lock(addr common.Address) error {
-	am.mutex.Lock()
+	am.mu.Lock()
 	if unl, found := am.unlocked[addr]; found {
-		am.mutex.Unlock()
+		am.mu.Unlock()
 		am.expire(addr, unl, time.Duration(0)*time.Nanosecond)
 	} else {
-		am.mutex.Unlock()
+		am.mu.Unlock()
 	}
 	return nil
 }
@@ -117,15 +153,14 @@ func (am *Manager) Lock(addr common.Address) error {
 // If the accout is already unlocked, TimedUnlock extends or shortens
 // the active unlock timeout.
 func (am *Manager) TimedUnlock(a Account, keyAuth string, timeout time.Duration) error {
-	key, err := am.keyStore.GetKey(a.Address, keyAuth)
+	_, key, err := am.getDecryptedKey(a, keyAuth)
 	if err != nil {
 		return err
 	}
-	var u *unlocked
-	am.mutex.Lock()
-	defer am.mutex.Unlock()
-	var found bool
-	u, found = am.unlocked[a.Address]
+
+	am.mu.Lock()
+	defer am.mu.Unlock()
+	u, found := am.unlocked[a.Address]
 	if found {
 		// terminate dropLater for this key to avoid unexpected drops.
 		if u.abort != nil {
@@ -142,6 +177,18 @@ func (am *Manager) TimedUnlock(a Account, keyAuth string, timeout time.Duration)
 	return nil
 }
 
+func (am *Manager) getDecryptedKey(a Account, auth string) (Account, *Key, error) {
+	am.cache.maybeReload()
+	am.cache.mu.Lock()
+	a, err := am.cache.find(a)
+	am.cache.mu.Unlock()
+	if err != nil {
+		return a, nil, err
+	}
+	key, err := am.keyStore.GetKey(a.Address, a.File, auth)
+	return a, key, err
+}
+
 func (am *Manager) expire(addr common.Address, u *unlocked, timeout time.Duration) {
 	t := time.NewTimer(timeout)
 	defer t.Stop()
@@ -149,7 +196,7 @@ func (am *Manager) expire(addr common.Address, u *unlocked, timeout time.Duratio
 	case <-u.abort:
 		// just quit
 	case <-t.C:
-		am.mutex.Lock()
+		am.mu.Lock()
 		// only drop if it's still the same key instance that dropLater
 		// was launched with. we can check that using pointer equality
 		// because the map stores a new pointer every time the key is
@@ -158,52 +205,33 @@ func (am *Manager) expire(addr common.Address, u *unlocked, timeout time.Duratio
 			zeroKey(u.PrivateKey)
 			delete(am.unlocked, addr)
 		}
-		am.mutex.Unlock()
+		am.mu.Unlock()
 	}
 }
 
 func (am *Manager) NewAccount(auth string) (Account, error) {
-	key, err := am.keyStore.GenerateNewKey(crand.Reader, auth)
+	_, account, err := storeNewKey(am.keyStore, crand.Reader, auth)
 	if err != nil {
 		return Account{}, err
 	}
-	return Account{Address: key.Address}, nil
+	// Add the account to the cache immediately rather
+	// than waiting for file system notifications to pick it up.
+	am.cache.add(account)
+	return account, nil
 }
 
 func (am *Manager) AccountByIndex(index int) (Account, error) {
-	addrs, err := am.keyStore.GetKeyAddresses()
-	if err != nil {
-		return Account{}, err
-	}
-	if index < 0 || index >= len(addrs) {
-		return Account{}, fmt.Errorf("account index %d not in range [0, %d]", index, len(addrs)-1)
-	}
-	return Account{Address: addrs[index]}, nil
-}
-
-func (am *Manager) Accounts() []Account {
-	addresses, _ := am.keyStore.GetKeyAddresses()
-	accounts := make([]Account, len(addresses))
-	for i, addr := range addresses {
-		accounts[i] = Account{
-			Address: addr,
-		}
-	}
-	return accounts
-}
-
-// zeroKey zeroes a private key in memory.
-func zeroKey(k *ecdsa.PrivateKey) {
-	b := k.D.Bits()
-	for i := range b {
-		b[i] = 0
+	accounts := am.Accounts()
+	if index < 0 || index >= len(accounts) {
+		return Account{}, fmt.Errorf("account index %d out of range [0, %d]", index, len(accounts)-1)
 	}
+	return accounts[index], nil
 }
 
 // USE WITH CAUTION = this will save an unencrypted private key on disk
 // no cli or js interface
 func (am *Manager) Export(path string, a Account, keyAuth string) error {
-	key, err := am.keyStore.GetKey(a.Address, keyAuth)
+	_, key, err := am.getDecryptedKey(a, keyAuth)
 	if err != nil {
 		return err
 	}
@@ -220,30 +248,35 @@ func (am *Manager) Import(path string, keyAuth string) (Account, error) {
 
 func (am *Manager) ImportECDSA(priv *ecdsa.PrivateKey, keyAuth string) (Account, error) {
 	key := newKeyFromECDSA(priv)
-	if err := am.keyStore.StoreKey(key, keyAuth); err != nil {
+	a := Account{Address: key.Address, File: am.keyStore.JoinPath(keyFileName(key.Address))}
+	if err := am.keyStore.StoreKey(a.File, key, keyAuth); err != nil {
 		return Account{}, err
 	}
-	return Account{Address: key.Address}, nil
+	am.cache.add(a)
+	return a, nil
 }
 
-func (am *Manager) Update(a Account, authFrom, authTo string) (err error) {
-	var key *Key
-	key, err = am.keyStore.GetKey(a.Address, authFrom)
-
-	if err == nil {
-		err = am.keyStore.StoreKey(key, authTo)
-		if err == nil {
-			am.keyStore.Cleanup(a.Address)
-		}
+func (am *Manager) Update(a Account, authFrom, authTo string) error {
+	a, key, err := am.getDecryptedKey(a, authFrom)
+	if err != nil {
+		return err
 	}
-	return
+	return am.keyStore.StoreKey(a.File, key, authTo)
 }
 
-func (am *Manager) ImportPreSaleKey(keyJSON []byte, password string) (acc Account, err error) {
-	var key *Key
-	key, err = importPreSaleKey(am.keyStore, keyJSON, password)
+func (am *Manager) ImportPreSaleKey(keyJSON []byte, password string) (Account, error) {
+	a, _, err := importPreSaleKey(am.keyStore, keyJSON, password)
 	if err != nil {
-		return
+		return a, err
+	}
+	am.cache.add(a)
+	return a, nil
+}
+
+// zeroKey zeroes a private key in memory.
+func zeroKey(k *ecdsa.PrivateKey) {
+	b := k.D.Bits()
+	for i := range b {
+		b[i] = 0
 	}
-	return Account{Address: key.Address}, nil
 }

accounts/accounts_test.go

@@ -19,28 +19,70 @@ package accounts
 import (
 	"io/ioutil"
 	"os"
+	"runtime"
+	"strings"
 	"testing"
 	"time"
+
+	"github.com/ethereum/go-ethereum/common"
 )
 
 var testSigData = make([]byte, 32)
 
+func TestManager(t *testing.T) {
+	dir, am := tmpManager(t, true)
+	defer os.RemoveAll(dir)
+
+	a, err := am.NewAccount("foo")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.HasPrefix(a.File, dir) {
+		t.Errorf("account file %s doesn't have dir prefix", a.File)
+	}
+	stat, err := os.Stat(a.File)
+	if err != nil {
+		t.Fatalf("account file %s doesn't exist (%v)", a.File, err)
+	}
+	if runtime.GOOS != "windows" && stat.Mode() != 0600 {
+		t.Fatalf("account file has wrong mode: got %o, want %o", stat.Mode(), 0600)
+	}
+	if !am.HasAddress(a.Address) {
+		t.Errorf("HasAccount(%x) should've returned true", a.Address)
+	}
+	if err := am.Update(a, "foo", "bar"); err != nil {
+		t.Errorf("Update error: %v", err)
+	}
+	if err := am.DeleteAccount(a, "bar"); err != nil {
+		t.Errorf("DeleteAccount error: %v", err)
+	}
+	if common.FileExist(a.File) {
+		t.Errorf("account file %s should be gone after DeleteAccount", a.File)
+	}
+	if am.HasAddress(a.Address) {
+		t.Errorf("HasAccount(%x) should've returned true after DeleteAccount", a.Address)
+	}
+}
+
 func TestSign(t *testing.T) {
-	dir, am := tmpManager(t, false)
+	dir, am := tmpManager(t, true)
 	defer os.RemoveAll(dir)
 
 	pass := "" // not used but required by API
 	a1, err := am.NewAccount(pass)
-	am.Unlock(a1, "")
-
-	_, err = am.Sign(a1, testSigData)
 	if err != nil {
 		t.Fatal(err)
 	}
+	if err := am.Unlock(a1, ""); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := am.Sign(a1, testSigData); err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestTimedUnlock(t *testing.T) {
-	dir, am := tmpManager(t, false)
+	dir, am := tmpManager(t, true)
 	defer os.RemoveAll(dir)
 
 	pass := "foo"
@@ -142,7 +184,7 @@ func tmpManager(t *testing.T, encrypted bool) (string, *Manager) {
 	}
 	new := NewPlaintextManager
 	if encrypted {
-		new = func(kd string) *Manager { return NewManager(kd, LightScryptN, LightScryptP) }
+		new = func(kd string) *Manager { return NewManager(kd, veryLightScryptN, veryLightScryptP) }
 	}
 	return d, new(d)
 }

accounts/addrcache.go

+// Copyright 2016 The go-ethereum Authors
+// This file is part of the go-ethereum library.
+//
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The go-ethereum library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
+
+package accounts
+
+import (
+	"bufio"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+	"sync"
+	"time"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/logger"
+	"github.com/ethereum/go-ethereum/logger/glog"
+)
+
+// Minimum amount of time between cache reloads. This limit applies if the platform does
+// not support change notifications. It also applies if the keystore directory does not
+// exist yet, the code will attempt to create a watcher at most this often.
+const minReloadInterval = 2 * time.Second
+
+type accountsByFile []Account
+
+func (s accountsByFile) Len() int           { return len(s) }
+func (s accountsByFile) Less(i, j int) bool { return s[i].File < s[j].File }
+func (s accountsByFile) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
+
+// AmbiguousAddrError is returned when attempting to unlock
+// an address for which more than one file exists.
+type AmbiguousAddrError struct {
+	Addr    common.Address
+	Matches []Account
+}
+
+func (err *AmbiguousAddrError) Error() string {
+	files := ""
+	for i, a := range err.Matches {
+		files += a.File
+		if i < len(err.Matches)-1 {
+			files += ", "
+		}
+	}
+	return fmt.Sprintf("multiple keys match address (%s)", files)
+}
+
+// addrCache is a live index of all accounts in the keystore.
+type addrCache struct {
+	keydir   string
+	watcher  *watcher
+	mu       sync.Mutex
+	all      accountsByFile
+	byAddr   map[common.Address][]Account
+	throttle *time.Timer
+}
+
+func newAddrCache(keydir string) *addrCache {
+	ac := &addrCache{
+		keydir: keydir,
+		byAddr: make(map[common.Address][]Account),
+	}
+	ac.watcher = newWatcher(ac)
+	return ac
+}
+
+func (ac *addrCache) accounts() []Account {
+	ac.maybeReload()
+	ac.mu.Lock()
+	defer ac.mu.Unlock()
+	cpy := make([]Account, len(ac.all))
+	copy(cpy, ac.all)
+	return cpy
+}
+
+func (ac *addrCache) hasAddress(addr common.Address) bool {
+	ac.maybeReload()
+	ac.mu.Lock()
+	defer ac.mu.Unlock()
+	return len(ac.byAddr[addr]) > 0
+}
+
+func (ac *addrCache) add(newAccount Account) {
+	ac.mu.Lock()
+	defer ac.mu.Unlock()
+
+	i := sort.Search(len(ac.all), func(i int) bool { return ac.all[i].File >= newAccount.File })
+	if i < len(ac.all) && ac.all[i] == newAccount {
+		return
+	}
+	// newAccount is not in the cache.
+	ac.all = append(ac.all, Account{})
+	copy(ac.all[i+1:], ac.all[i:])
+	ac.all[i] = newAccount
+	ac.byAddr[newAccount.Address] = append(ac.byAddr[newAccount.Address], newAccount)
+}
+
+// note: removed needs to be unique here (i.e. both File and Address must be set).
+func (ac *addrCache) delete(removed Account) {
+	ac.mu.Lock()
+	defer ac.mu.Unlock()
+	ac.all = removeAccount(ac.all, removed)
+	if ba := removeAccount(ac.byAddr[removed.Address], removed); len(ba) == 0 {
+		delete(ac.byAddr, removed.Address)
+	} else {
+		ac.byAddr[removed.Address] = ba
+	}
+}
+
+func removeAccount(slice []Account, elem Account) []Account {
+	for i := range slice {
+		if slice[i] == elem {
+			return append(slice[:i], slice[i+1:]...)
+		}
+	}
+	return slice
+}
+
+// find returns the cached account for address if there is a unique match.
+// The exact matching rules are explained by the documentation of Account.
+// Callers must hold ac.mu.
+func (ac *addrCache) find(a Account) (Account, error) {
+	// Limit search to address candidates if possible.
+	matches := ac.all
+	if (a.Address != common.Address{}) {
+		matches = ac.byAddr[a.Address]
+	}
+	if a.File != "" {
+		// If only the basename is specified, complete the path.
+		if !strings.ContainsRune(a.File, filepath.Separator) {
+			a.File = filepath.Join(ac.keydir, a.File)
+		}
+		for i := range matches {
+			if matches[i].File == a.File {
+				return matches[i], nil
+			}
+		}
+		if (a.Address == common.Address{}) {
+			return Account{}, ErrNoMatch
+		}
+	}
+	switch len(matches) {
+	case 1:
+		return matches[0], nil
+	case 0:
+		return Account{}, ErrNoMatch
+	default:
+		err := &AmbiguousAddrError{Addr: a.Address, Matches: make([]Account, len(matches))}
+		copy(err.Matches, matches)
+		return Account{}, err
+	}
+}
+
+func (ac *addrCache) maybeReload() {
+	ac.mu.Lock()
+	defer ac.mu.Unlock()
+	if ac.watcher.running {
+		return // A watcher is running and will keep the cache up-to-date.
+	}
+	if ac.throttle == nil {
+		ac.throttle = time.NewTimer(0)
+	} else {
+		select {
+		case <-ac.throttle.C:
+		default:
+			return // The cache was reloaded recently.
+		}
+	}
+	ac.watcher.start()
+	ac.reload()
+	ac.throttle.Reset(minReloadInterval)
+}
+
+func (ac *addrCache) close() {
+	ac.mu.Lock()
+	ac.watcher.close()
+	if ac.throttle != nil {
+		ac.throttle.Stop()
+	}
+	ac.mu.Unlock()
+}
+
+// reload caches addresses of existing accounts.
+// Callers must hold ac.mu.
+func (ac *addrCache) reload() {
+	accounts, err := ac.scan()
+	if err != nil && glog.V(logger.Debug) {
+		glog.Errorf("can't load keys: %v", err)
+	}
+	ac.all = accounts
+	sort.Sort(ac.all)
+	for k := range ac.byAddr {
+		delete(ac.byAddr, k)
+	}
+	for _, a := range accounts {
+		ac.byAddr[a.Address] = append(ac.byAddr[a.Address], a)
+	}
+	glog.V(logger.Debug).Infof("reloaded keys, cache has %d accounts", len(ac.all))
+}
+
+func (ac *addrCache) scan() ([]Account, error) {
+	files, err := ioutil.ReadDir(ac.keydir)
+	if err != nil {
+		return nil, err
+	}
+
+	var (
+		buf     = new(bufio.Reader)
+		addrs   []Account
+		keyJSON struct {
+			Address common.Address `json:"address"`
+		}
+	)
+	for _, fi := range files {
+		path := filepath.Join(ac.keydir, fi.Name())
+		if skipKeyFile(fi) {
+			glog.V(logger.Detail).Infof("ignoring file %s", path)
+			continue
+		}
+		fd, err := os.Open(path)
+		if err != nil {
+			glog.V(logger.Detail).Infoln(err)
+			continue
+		}
+		buf.Reset(fd)
+		// Parse the address.
+		keyJSON.Address = common.Address{}
+		err = json.NewDecoder(buf).Decode(&keyJSON)
+		switch {
+		case err != nil:
+			glog.V(logger.Debug).Infof("can't decode key %s: %v", path, err)
+		case (keyJSON.Address == common.Address{}):
+			glog.V(logger.Debug).Infof("can't decode key %s: missing or zero address", path)
+		default:
+			addrs = append(addrs, Account{Address: keyJSON.Address, File: path})
+		}
+		fd.Close()
+	}
+	return addrs, err
+}
+
+func skipKeyFile(fi os.FileInfo) bool {
+	// Skip editor backups and UNIX-style hidden files.
+	if strings.HasSuffix(fi.Name(), "~") || strings.HasPrefix(fi.Name(), ".") {
+		return true
+	}
+	// Skip misc special files, directories (yes, symlinks too).
+	if fi.IsDir() || fi.Mode()&os.ModeType != 0 {
+		return true
+	}
+	return false
+}

accounts/addrcache_test.go

+// Copyright 2016 The go-ethereum Authors
+// This file is part of the go-ethereum library.
+//
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The go-ethereum library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
+
+package accounts
+
+import (
+	"fmt"
+	"math/rand"
+	"os"
+	"path/filepath"
+	"reflect"
+	"sort"
+	"testing"
+	"time"
+
+	"github.com/cespare/cp"
+	"github.com/davecgh/go-spew/spew"
+	"github.com/ethereum/go-ethereum/common"
+)
+
+var (
+	cachetestDir, _   = filepath.Abs(filepath.Join("testdata", "keystore"))
+	cachetestAccounts = []Account{
+		{
+			Address: common.HexToAddress("7ef5a6135f1fd6a02593eedc869c6d41d934aef8"),
+			File:    filepath.Join(cachetestDir, "UTC--2016-03-22T12-57-55.920751759Z--7ef5a6135f1fd6a02593eedc869c6d41d934aef8"),
+		},
+		{
+			Address: common.HexToAddress("f466859ead1932d743d622cb74fc058882e8648a"),
+			File:    filepath.Join(cachetestDir, "aaa"),
+		},
+		{
+			Address: common.HexToAddress("289d485d9771714cce91d3393d764e1311907acc"),
+			File:    filepath.Join(cachetestDir, "zzz"),
+		},
+	}
+)
+
+func TestWatchNewFile(t *testing.T) {
+	t.Parallel()
+
+	dir, am := tmpManager(t, false)
+	defer os.RemoveAll(dir)
+
+	// Ensure the watcher is started before adding any files.
+	am.Accounts()
+	time.Sleep(200 * time.Millisecond)
+
+	// Move in the files.
+	wantAccounts := make([]Account, len(cachetestAccounts))
+	for i := range cachetestAccounts {
+		a := cachetestAccounts[i]
+		a.File = filepath.Join(dir, filepath.Base(a.File))
+		wantAccounts[i] = a
+		if err := cp.CopyFile(a.File, cachetestAccounts[i].File); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	// am should see the accounts.
+	var list []Account
+	for d := 200 * time.Millisecond; d < 5*time.Second; d *= 2 {
+		list = am.Accounts()
+		if reflect.DeepEqual(list, wantAccounts) {
+			return
+		}
+		time.Sleep(d)
+	}
+	t.Errorf("got %s, want %s", spew.Sdump(list), spew.Sdump(wantAccounts))
+}
+
+func TestWatchNoDir(t *testing.T) {
+	t.Parallel()
+
+	// Create am but not the directory that it watches.
+	rand.Seed(time.Now().UnixNano())
+	dir := filepath.Join(os.TempDir(), fmt.Sprintf("eth-keystore-watch-test-%d-%d", os.Getpid(), rand.Int()))
+	am := NewManager(dir, LightScryptN, LightScryptP)
+
+	list := am.Accounts()
+	if len(list) > 0 {
+		t.Error("initial account list not empty:", list)
+	}
+	time.Sleep(100 * time.Millisecond)
+
+	// Create the directory and copy a key file into it.
+	os.MkdirAll(dir, 0700)
+	defer os.RemoveAll(dir)
+	file := filepath.Join(dir, "aaa")
+	if err := cp.CopyFile(file, cachetestAccounts[0].File); err != nil {
+		t.Fatal(err)
+	}
+
+	// am should see the account.
+	wantAccounts := []Account{cachetestAccounts[0]}
+	wantAccounts[0].File = file
+	for d := 200 * time.Millisecond; d < 8*time.Second; d *= 2 {
+		list = am.Accounts()
+		if reflect.DeepEqual(list, wantAccounts) {
+			return
+		}
+		time.Sleep(d)
+	}
+	t.Errorf("\ngot  %v\nwant %v", list, wantAccounts)
+}
+
+func TestCacheInitialReload(t *testing.T) {
+	cache := newAddrCache(cachetestDir)
+	accounts := cache.accounts()
+	if !reflect.DeepEqual(accounts, cachetestAccounts) {
+		t.Fatalf("got initial accounts: %swant %s", spew.Sdump(accounts), spew.Sdump(cachetestAccounts))
+	}
+}
+
+func TestCacheAddDeleteOrder(t *testing.T) {
+	cache := newAddrCache("testdata/no-such-dir")
+	cache.watcher.running = true // prevent unexpected reloads
+
+	accounts := []Account{
+		{
+			Address: common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87"),
+			File:    "-309830980",
+		},
+		{
+			Address: common.HexToAddress("2cac1adea150210703ba75ed097ddfe24e14f213"),
+			File:    "ggg",
+		},
+		{
+			Address: common.HexToAddress("8bda78331c916a08481428e4b07c96d3e916d165"),
+			File:    "zzzzzz-the-very-last-one.keyXXX",
+		},
+		{
+			Address: common.HexToAddress("d49ff4eeb0b2686ed89c0fc0f2b6ea533ddbbd5e"),
+			File:    "SOMETHING.key",
+		},
+		{
+			Address: common.HexToAddress("7ef5a6135f1fd6a02593eedc869c6d41d934aef8"),
+			File:    "UTC--2016-03-22T12-57-55.920751759Z--7ef5a6135f1fd6a02593eedc869c6d41d934aef8",
+		},
+		{
+			Address: common.HexToAddress("f466859ead1932d743d622cb74fc058882e8648a"),
+			File:    "aaa",
+		},
+		{
+			Address: common.HexToAddress("289d485d9771714cce91d3393d764e1311907acc"),
+			File:    "zzz",
+		},
+	}
+	for _, a := range accounts {
+		cache.add(a)
+	}
+	// Add some of them twice to check that they don't get reinserted.
+	cache.add(accounts[0])
+	cache.add(accounts[2])
+
+	// Check that the account list is sorted by filename.
+	wantAccounts := make([]Account, len(accounts))
+	copy(wantAccounts, accounts)
+	sort.Sort(accountsByFile(wantAccounts))
+	list := cache.accounts()
+	if !reflect.DeepEqual(list, wantAccounts) {
+		t.Fatalf("got accounts: %s\nwant %s", spew.Sdump(accounts), spew.Sdump(wantAccounts))
+	}
+	for _, a := range accounts {
+		if !cache.hasAddress(a.Address) {
+			t.Errorf("expected hasAccount(%x) to return true", a.Address)
+		}
+	}
+	if cache.hasAddress(common.HexToAddress("fd9bd350f08ee3c0c19b85a8e16114a11a60aa4e")) {
+		t.Errorf("expected hasAccount(%x) to return false", common.HexToAddress("fd9bd350f08ee3c0c19b85a8e16114a11a60aa4e"))
+	}
+
+	// Delete a few keys from the cache.
+	for i := 0; i < len(accounts); i += 2 {
+		cache.delete(wantAccounts[i])
+	}
+	cache.delete(Account{Address: common.HexToAddress("fd9bd350f08ee3c0c19b85a8e16114a11a60aa4e"), File: "something"})
+
+	// Check content again after deletion.
+	wantAccountsAfterDelete := []Account{
+		wantAccounts[1],
+		wantAccounts[3],
+		wantAccounts[5],
+	}
+	list = cache.accounts()
+	if !reflect.DeepEqual(list, wantAccountsAfterDelete) {
+		t.Fatalf("got accounts after delete: %s\nwant %s", spew.Sdump(list), spew.Sdump(wantAccountsAfterDelete))
+	}
+	for _, a := range wantAccountsAfterDelete {
+		if !cache.hasAddress(a.Address) {
+			t.Errorf("expected hasAccount(%x) to return true", a.Address)
+		}
+	}
+	if cache.hasAddress(wantAccounts[0].Address) {
+		t.Errorf("expected hasAccount(%x) to return false", wantAccounts[0].Address)
+	}
+}
+
+func TestCacheFind(t *testing.T) {
+	dir := filepath.Join("testdata", "dir")
+	cache := newAddrCache(dir)
+	cache.watcher.running = true // prevent unexpected reloads
+
+	accounts := []Account{
+		{
+			Address: common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87"),
+			File:    filepath.Join(dir, "a.key"),
+		},
+		{
+			Address: common.HexToAddress("2cac1adea150210703ba75ed097ddfe24e14f213"),
+			File:    filepath.Join(dir, "b.key"),
+		},
+		{
+			Address: common.HexToAddress("d49ff4eeb0b2686ed89c0fc0f2b6ea533ddbbd5e"),
+			File:    filepath.Join(dir, "c.key"),
+		},
+		{
+			Address: common.HexToAddress("d49ff4eeb0b2686ed89c0fc0f2b6ea533ddbbd5e"),
+			File:    filepath.Join(dir, "c2.key"),
+		},
+	}
+	for _, a := range accounts {
+		cache.add(a)
+	}
+
+	nomatchAccount := Account{
+		Address: common.HexToAddress("f466859ead1932d743d622cb74fc058882e8648a"),
+		File:    filepath.Join(dir, "something"),
+	}
+	tests := []struct {
+		Query      Account
+		WantResult Account
+		WantError  error
+	}{
+		// by address
+		{Query: Account{Address: accounts[0].Address}, WantResult: accounts[0]},
+		// by file
+		{Query: Account{File: accounts[0].File}, WantResult: accounts[0]},
+		// by basename
+		{Query: Account{File: filepath.Base(accounts[0].File)}, WantResult: accounts[0]},
+		// by file and address
+		{Query: accounts[0], WantResult: accounts[0]},
+		// ambiguous address, tie resolved by file
+		{Query: accounts[2], WantResult: accounts[2]},
+		// ambiguous address error
+		{
+			Query: Account{Address: accounts[2].Address},
+			WantError: &AmbiguousAddrError{
+				Addr:    accounts[2].Address,
+				Matches: []Account{accounts[2], accounts[3]},
+			},
+		},
+		// no match error
+		{Query: nomatchAccount, WantError: ErrNoMatch},
+		{Query: Account{File: nomatchAccount.File}, WantError: ErrNoMatch},
+		{Query: Account{File: filepath.Base(nomatchAccount.File)}, WantError: ErrNoMatch},
+		{Query: Account{Address: nomatchAccount.Address}, WantError: ErrNoMatch},
+	}
+	for i, test := range tests {
+		a, err := cache.find(test.Query)
+		if !reflect.DeepEqual(err, test.WantError) {
+			t.Errorf("test %d: error mismatch for query %v\ngot %q\nwant %q", i, test.Query, err, test.WantError)
+			continue
+		}
+		if a != test.WantResult {
+			t.Errorf("test %d: result mismatch for query %v\ngot %v\nwant %v", i, test.Query, a, test.WantResult)
+			continue
+		}
+	}
+}

accounts/key.go

@@ -21,8 +21,13 @@ import (
 	"crypto/ecdsa"
 	"encoding/hex"
 	"encoding/json"
+	"fmt"
 	"io"
+	"io/ioutil"
+	"os"
+	"path/filepath"
 	"strings"
+	"time"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
@@ -44,13 +49,12 @@ type Key struct {
 }
 
 type keyStore interface {
-	// create new key using io.Reader entropy source and optionally using auth string
-	GenerateNewKey(io.Reader, string) (*Key, error)
-	GetKey(common.Address, string) (*Key, error) // get key from addr and auth string
-	GetKeyAddresses() ([]common.Address, error)  // get all addresses
-	StoreKey(*Key, string) error                 // store key optionally using auth string
-	DeleteKey(common.Address, string) error      // delete key by addr and auth string
-	Cleanup(keyAddr common.Address) (err error)
+	// Loads and decrypts the key from disk.
+	GetKey(addr common.Address, filename string, auth string) (*Key, error)
+	// Writes and encrypts the key.
+	StoreKey(filename string, k *Key, auth string) error
+	// Joins filename with the key directory unless it is already absolute.
+	JoinPath(filename string) string
 }
 
 type plainKeyJSON struct {
@@ -142,21 +146,6 @@ func newKeyFromECDSA(privateKeyECDSA *ecdsa.PrivateKey) *Key {
 	return key
 }
 
-func NewKey(rand io.Reader) *Key {
-	randBytes := make([]byte, 64)
-	_, err := rand.Read(randBytes)
-	if err != nil {
-		panic("key generation: could not read from random source: " + err.Error())
-	}
-	reader := bytes.NewReader(randBytes)
-	privateKeyECDSA, err := ecdsa.GenerateKey(secp256k1.S256(), reader)
-	if err != nil {
-		panic("key generation: ecdsa.GenerateKey failed: " + err.Error())
-	}
-
-	return newKeyFromECDSA(privateKeyECDSA)
-}
-
 // generate key whose address fits into < 155 bits so it can fit into
 // the Direct ICAP spec. for simplicity and easier compatibility with
 // other libs, we retry until the first byte is 0.
@@ -177,3 +166,64 @@ func NewKeyForDirectICAP(rand io.Reader) *Key {
 	}
 	return key
 }
+
+func newKey(rand io.Reader) (*Key, error) {
+	privateKeyECDSA, err := ecdsa.GenerateKey(secp256k1.S256(), rand)
+	if err != nil {
+		return nil, err
+	}
+	return newKeyFromECDSA(privateKeyECDSA), nil
+}
+
+func storeNewKey(ks keyStore, rand io.Reader, auth string) (*Key, Account, error) {
+	key, err := newKey(rand)
+	if err != nil {
+		return nil, Account{}, err
+	}
+	a := Account{Address: key.Address, File: ks.JoinPath(keyFileName(key.Address))}
+	if err := ks.StoreKey(a.File, key, auth); err != nil {
+		zeroKey(key.PrivateKey)
+		return nil, a, err
+	}
+	return key, a, err
+}
+
+func writeKeyFile(file string, content []byte) error {
+	// Create the keystore directory with appropriate permissions
+	// in case it is not present yet.
+	const dirPerm = 0700
+	if err := os.MkdirAll(filepath.Dir(file), dirPerm); err != nil {
+		return err
+	}
+	// Atomic write: create a temporary hidden file first
+	// then move it into place. TempFile assigns mode 0600.
+	f, err := ioutil.TempFile(filepath.Dir(file), "."+filepath.Base(file)+".tmp")
+	if err != nil {
+		return err
+	}
+	if _, err := f.Write(content); err != nil {
+		f.Close()
+		os.Remove(f.Name())
+		return err
+	}
+	f.Close()
+	return os.Rename(f.Name(), file)
+}
+
+// keyFileName implements the naming convention for keyfiles:
+// UTC--<created_at UTC ISO8601>-<address hex>
+func keyFileName(keyAddr common.Address) string {
+	ts := time.Now().UTC()
+	return fmt.Sprintf("UTC--%s--%s", toISO8601(ts), hex.EncodeToString(keyAddr[:]))
+}
+
+func toISO8601(t time.Time) string {
+	var tz string
+	name, offset := t.Zone()
+	if name == "UTC" {
+		tz = "Z"
+	} else {
+		tz = fmt.Sprintf("%03d00", offset/3600)
+	}
+	return fmt.Sprintf("%04d-%02d-%02dT%02d-%02d-%02d.%09d%s", t.Year(), t.Month(), t.Day(), t.Hour(), t.Minute(), t.Second(), t.Nanosecond(), tz)
+}

accounts/key_store_passphrase.go

@@ -33,7 +33,8 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io"
+	"io/ioutil"
+	"path/filepath"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
@@ -64,32 +65,37 @@ type keyStorePassphrase struct {
 	scryptP     int
 }
 
-func newKeyStorePassphrase(path string, scryptN int, scryptP int) keyStore {
-	return &keyStorePassphrase{path, scryptN, scryptP}
-}
-
-func (ks keyStorePassphrase) GenerateNewKey(rand io.Reader, auth string) (key *Key, err error) {
-	return generateNewKeyDefault(ks, rand, auth)
-}
-
-func (ks keyStorePassphrase) GetKey(keyAddr common.Address, auth string) (key *Key, err error) {
-	return decryptKeyFromFile(ks.keysDirPath, keyAddr, auth)
-}
-
-func (ks keyStorePassphrase) Cleanup(keyAddr common.Address) (err error) {
-	return cleanup(ks.keysDirPath, keyAddr)
-}
-
-func (ks keyStorePassphrase) GetKeyAddresses() (addresses []common.Address, err error) {
-	return getKeyAddresses(ks.keysDirPath)
+func (ks keyStorePassphrase) GetKey(addr common.Address, filename, auth string) (*Key, error) {
+	// Load the key from the keystore and decrypt its contents
+	keyjson, err := ioutil.ReadFile(filename)
+	if err != nil {
+		return nil, err
+	}
+	key, err := DecryptKey(keyjson, auth)
+	if err != nil {
+		return nil, err
+	}
+	// Make sure we're really operating on the requested key (no swap attacks)
+	if key.Address != addr {
+		return nil, fmt.Errorf("key content mismatch: have account %x, want %x", key.Address, addr)
+	}
+	return key, nil
 }
 
-func (ks keyStorePassphrase) StoreKey(key *Key, auth string) error {
+func (ks keyStorePassphrase) StoreKey(filename string, key *Key, auth string) error {
 	keyjson, err := EncryptKey(key, auth, ks.scryptN, ks.scryptP)
 	if err != nil {
 		return err
 	}
-	return writeKeyFile(key.Address, ks.keysDirPath, keyjson)
+	return writeKeyFile(filename, keyjson)
+}
+
+func (ks keyStorePassphrase) JoinPath(filename string) string {
+	if filepath.IsAbs(filename) {
+		return filename
+	} else {
+		return filepath.Join(ks.keysDirPath, filename)
+	}
 }
 
 // EncryptKey encrypts a key using the specified scrypt parameters into a json
@@ -139,14 +145,6 @@ func EncryptKey(key *Key, auth string, scryptN, scryptP int) ([]byte, error) {
 	return json.Marshal(encryptedKeyJSONV3)
 }
 
-func (ks keyStorePassphrase) DeleteKey(keyAddr common.Address, auth string) error {
-	// only delete if correct passphrase is given
-	if _, err := decryptKeyFromFile(ks.keysDirPath, keyAddr, auth); err != nil {
-		return err
-	}
-	return deleteKey(ks.keysDirPath, keyAddr)
-}
-
 // DecryptKey decrypts a key from a json blob, returning the private key itself.
 func DecryptKey(keyjson []byte, auth string) (*Key, error) {
 	// Parse the json into a simple map to fetch the key version
@@ -184,23 +182,6 @@ func DecryptKey(keyjson []byte, auth string) (*Key, error) {
 	}, nil
 }
 
-func decryptKeyFromFile(keysDirPath string, keyAddr common.Address, auth string) (*Key, error) {
-	// Load the key from the keystore and decrypt its contents
-	keyjson, err := getKeyFile(keysDirPath, keyAddr)
-	if err != nil {
-		return nil, err
-	}
-	key, err := DecryptKey(keyjson, auth)
-	if err != nil {
-		return nil, err
-	}
-	// Make sure we're really operating on the requested key (no swap attacks)
-	if keyAddr != key.Address {
-		return nil, fmt.Errorf("key content mismatch: have account %x, want %x", key.Address, keyAddr)
-	}
-	return key, nil
-}
-
 func decryptKeyV3(keyProtected *encryptedKeyJSONV3, auth string) (keyBytes []byte, keyId []byte, err error) {
 	if keyProtected.Version != version {
 		return nil, nil, fmt.Errorf("Version not supported: %v", keyProtected.Version)

accounts/key_store_passphrase_test.go

@@ -17,16 +17,25 @@
 package accounts
 
 import (
+	"io/ioutil"
 	"testing"
 
 	"github.com/ethereum/go-ethereum/common"
 )
 
+const (
+	veryLightScryptN = 2
+	veryLightScryptP = 1
+)
+
 // Tests that a json key file can be decrypted and encrypted in multiple rounds.
 func TestKeyEncryptDecrypt(t *testing.T) {
-	address := common.HexToAddress("f626acac23772cbe04dd578bee681b06bdefb9fa")
-	keyjson := []byte("{\"address\":\"f626acac23772cbe04dd578bee681b06bdefb9fa\",\"crypto\":{\"cipher\":\"aes-128-ctr\",\"ciphertext\":\"1bcf0ab9b14459795ce59f63e63255ffd84dc38d31614a5a78e37144d7e4a17f\",\"cipherparams\":{\"iv\":\"df4c7e225ee2d81adef522013e3fbe24\"},\"kdf\":\"scrypt\",\"kdfparams\":{\"dklen\":32,\"n\":262144,\"p\":1,\"r\":8,\"salt\":\"2909a99dd2bfa7079a4b40991773b1083f8512c0c55b9b63402ab0e3dc8db8b3\"},\"mac\":\"4ecf6a4ad92ae2c016cb7c44abade74799480c3303eb024661270dfefdbc7510\"},\"id\":\"b4718210-9a30-4883-b8a6-dbdd08bd0ceb\",\"version\":3}")
+	keyjson, err := ioutil.ReadFile("testdata/very-light-scrypt.json")
+	if err != nil {
+		t.Fatal(err)
+	}
 	password := ""
+	address := common.HexToAddress("45dea0fb0bba44f4fcf290bba71fd57d7117cbb8")
 
 	// Do a few rounds of decryption and encryption
 	for i := 0; i < 3; i++ {
@@ -44,7 +53,7 @@ func TestKeyEncryptDecrypt(t *testing.T) {
 		}
 		// Recrypt with a new password and start over
 		password += "new data appended"
-		if keyjson, err = EncryptKey(key, password, LightScryptN, LightScryptP); err != nil {
+		if keyjson, err = EncryptKey(key, password, veryLightScryptN, veryLightScryptP); err != nil {
 			t.Errorf("test %d: failed to recrypt key %v", i, err)
 		}
 	}

accounts/key_store_plain.go

@@ -17,14 +17,10 @@
 package accounts
 
 import (
-	"encoding/hex"
 	"encoding/json"
 	"fmt"
-	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
-	"time"
 
 	"github.com/ethereum/go-ethereum/common"
 )
@@ -33,167 +29,34 @@ type keyStorePlain struct {
 	keysDirPath string
 }
 
-func newKeyStorePlain(path string) keyStore {
-	return &keyStorePlain{path}
-}
-
-func (ks keyStorePlain) GenerateNewKey(rand io.Reader, auth string) (key *Key, err error) {
-	return generateNewKeyDefault(ks, rand, auth)
-}
-
-func generateNewKeyDefault(ks keyStore, rand io.Reader, auth string) (key *Key, err error) {
-	defer func() {
-		if r := recover(); r != nil {
-			err = fmt.Errorf("GenerateNewKey error: %v", r)
-		}
-	}()
-	key = NewKey(rand)
-	err = ks.StoreKey(key, auth)
-	return key, err
-}
-
-func (ks keyStorePlain) GetKey(keyAddr common.Address, auth string) (*Key, error) {
-	keyjson, err := getKeyFile(ks.keysDirPath, keyAddr)
+func (ks keyStorePlain) GetKey(addr common.Address, filename, auth string) (*Key, error) {
+	fd, err := os.Open(filename)
 	if err != nil {
 		return nil, err
 	}
+	defer fd.Close()
 	key := new(Key)
-	if err := json.Unmarshal(keyjson, key); err != nil {
+	if err := json.NewDecoder(fd).Decode(key); err != nil {
 		return nil, err
 	}
-	return key, nil
-}
-
-func (ks keyStorePlain) GetKeyAddresses() (addresses []common.Address, err error) {
-	return getKeyAddresses(ks.keysDirPath)
-}
-
-func (ks keyStorePlain) Cleanup(keyAddr common.Address) (err error) {
-	return cleanup(ks.keysDirPath, keyAddr)
-}
-
-func (ks keyStorePlain) StoreKey(key *Key, auth string) (err error) {
-	keyJSON, err := json.Marshal(key)
-	if err != nil {
-		return
-	}
-	err = writeKeyFile(key.Address, ks.keysDirPath, keyJSON)
-	return
-}
-
-func (ks keyStorePlain) DeleteKey(keyAddr common.Address, auth string) (err error) {
-	return deleteKey(ks.keysDirPath, keyAddr)
-}
-
-func deleteKey(keysDirPath string, keyAddr common.Address) (err error) {
-	var path string
-	path, err = getKeyFilePath(keysDirPath, keyAddr)
-	if err == nil {
-		addrHex := hex.EncodeToString(keyAddr[:])
-		if path == filepath.Join(keysDirPath, addrHex, addrHex) {
-			path = filepath.Join(keysDirPath, addrHex)
-		}
-		err = os.RemoveAll(path)
-	}
-	return
-}
-
-func getKeyFilePath(keysDirPath string, keyAddr common.Address) (keyFilePath string, err error) {
-	addrHex := hex.EncodeToString(keyAddr[:])
-	matches, err := filepath.Glob(filepath.Join(keysDirPath, fmt.Sprintf("*--%s", addrHex)))
-	if len(matches) > 0 {
-		if err == nil {
-			keyFilePath = matches[len(matches)-1]
-		}
-		return
-	}
-	keyFilePath = filepath.Join(keysDirPath, addrHex, addrHex)
-	_, err = os.Stat(keyFilePath)
-	return
-}
-
-func cleanup(keysDirPath string, keyAddr common.Address) (err error) {
-	fileInfos, err := ioutil.ReadDir(keysDirPath)
-	if err != nil {
-		return
-	}
-	var paths []string
-	account := hex.EncodeToString(keyAddr[:])
-	for _, fileInfo := range fileInfos {
-		path := filepath.Join(keysDirPath, fileInfo.Name())
-		if len(path) >= 40 {
-			addr := path[len(path)-40 : len(path)]
-			if addr == account {
-				if path == filepath.Join(keysDirPath, addr, addr) {
-					path = filepath.Join(keysDirPath, addr)
-				}
-				paths = append(paths, path)
-			}
-		}
-	}
-	if len(paths) > 1 {
-		for i := 0; err == nil && i < len(paths)-1; i++ {
-			err = os.RemoveAll(paths[i])
-			if err != nil {
-				break
-			}
-		}
-	}
-	return
-}
-
-func getKeyFile(keysDirPath string, keyAddr common.Address) (fileContent []byte, err error) {
-	var keyFilePath string
-	keyFilePath, err = getKeyFilePath(keysDirPath, keyAddr)
-	if err == nil {
-		fileContent, err = ioutil.ReadFile(keyFilePath)
+	if key.Address != addr {
+		return nil, fmt.Errorf("key content mismatch: have address %x, want %x", key.Address, addr)
 	}
-	return
+	return key, nil
 }
 
-func writeKeyFile(addr common.Address, keysDirPath string, content []byte) (err error) {
-	filename := keyFileName(addr)
-	// read, write and dir search for user
-	err = os.MkdirAll(keysDirPath, 0700)
+func (ks keyStorePlain) StoreKey(filename string, key *Key, auth string) error {
+	content, err := json.Marshal(key)
 	if err != nil {
 		return err
 	}
-	// read, write for user
-	return ioutil.WriteFile(filepath.Join(keysDirPath, filename), content, 0600)
+	return writeKeyFile(filename, content)
 }
 
-// keyFilePath implements the naming convention for keyfiles:
-// UTC--<created_at UTC ISO8601>-<address hex>
-func keyFileName(keyAddr common.Address) string {
-	ts := time.Now().UTC()
-	return fmt.Sprintf("UTC--%s--%s", toISO8601(ts), hex.EncodeToString(keyAddr[:]))
-}
-
-func toISO8601(t time.Time) string {
-	var tz string
-	name, offset := t.Zone()
-	if name == "UTC" {
-		tz = "Z"
+func (ks keyStorePlain) JoinPath(filename string) string {
+	if filepath.IsAbs(filename) {
+		return filename
 	} else {
-		tz = fmt.Sprintf("%03d00", offset/3600)
-	}
-	return fmt.Sprintf("%04d-%02d-%02dT%02d-%02d-%02d.%09d%s", t.Year(), t.Month(), t.Day(), t.Hour(), t.Minute(), t.Second(), t.Nanosecond(), tz)
-}
-
-func getKeyAddresses(keysDirPath string) (addresses []common.Address, err error) {
-	fileInfos, err := ioutil.ReadDir(keysDirPath)
-	if err != nil {
-		return nil, err
-	}
-	for _, fileInfo := range fileInfos {
-		filename := fileInfo.Name()
-		if len(filename) >= 40 {
-			addr := filename[len(filename)-40 : len(filename)]
-			address, err := hex.DecodeString(addr)
-			if err == nil {
-				addresses = append(addresses, common.BytesToAddress(address))
-			}
-		}
+		return filepath.Join(ks.keysDirPath, filename)
 	}
-	return addresses, err
 }

accounts/key_store_test.go

@@ -17,106 +17,107 @@
 package accounts
 
 import (
+	"crypto/rand"
 	"encoding/hex"
 	"fmt"
+	"io/ioutil"
+	"os"
 	"reflect"
 	"strings"
 	"testing"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
-	"github.com/ethereum/go-ethereum/crypto/randentropy"
 )
 
+func tmpKeyStore(t *testing.T, encrypted bool) (dir string, ks keyStore) {
+	d, err := ioutil.TempDir("", "geth-keystore-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if encrypted {
+		ks = &keyStorePassphrase{d, veryLightScryptN, veryLightScryptP}
+	} else {
+		ks = &keyStorePlain{d}
+	}
+	return d, ks
+}
+
 func TestKeyStorePlain(t *testing.T) {
-	ks := newKeyStorePlain(common.DefaultDataDir())
+	dir, ks := tmpKeyStore(t, false)
+	defer os.RemoveAll(dir)
+
 	pass := "" // not used but required by API
-	k1, err := ks.GenerateNewKey(randentropy.Reader, pass)
+	k1, account, err := storeNewKey(ks, rand.Reader, pass)
 	if err != nil {
 		t.Fatal(err)
 	}
-
-	k2 := new(Key)
-	k2, err = ks.GetKey(k1.Address, pass)
+	k2, err := ks.GetKey(k1.Address, account.File, pass)
 	if err != nil {
 		t.Fatal(err)
 	}
-
 	if !reflect.DeepEqual(k1.Address, k2.Address) {
 		t.Fatal(err)
 	}
-
 	if !reflect.DeepEqual(k1.PrivateKey, k2.PrivateKey) {
 		t.Fatal(err)
 	}
-
-	err = ks.DeleteKey(k2.Address, pass)
-	if err != nil {
-		t.Fatal(err)
-	}
 }
 
 func TestKeyStorePassphrase(t *testing.T) {
-	ks := newKeyStorePassphrase(common.DefaultDataDir(), LightScryptN, LightScryptP)
+	dir, ks := tmpKeyStore(t, true)
+	defer os.RemoveAll(dir)
+
 	pass := "foo"
-	k1, err := ks.GenerateNewKey(randentropy.Reader, pass)
+	k1, account, err := storeNewKey(ks, rand.Reader, pass)
 	if err != nil {
 		t.Fatal(err)
 	}
-	k2 := new(Key)
-	k2, err = ks.GetKey(k1.Address, pass)
+	k2, err := ks.GetKey(k1.Address, account.File, pass)
 	if err != nil {
 		t.Fatal(err)
 	}
 	if !reflect.DeepEqual(k1.Address, k2.Address) {
 		t.Fatal(err)
 	}
-
 	if !reflect.DeepEqual(k1.PrivateKey, k2.PrivateKey) {
 		t.Fatal(err)
 	}
-
-	err = ks.DeleteKey(k2.Address, pass) // also to clean up created files
-	if err != nil {
-		t.Fatal(err)
-	}
 }
 
 func TestKeyStorePassphraseDecryptionFail(t *testing.T) {
-	ks := newKeyStorePassphrase(common.DefaultDataDir(), LightScryptN, LightScryptP)
+	dir, ks := tmpKeyStore(t, true)
+	defer os.RemoveAll(dir)
+
 	pass := "foo"
-	k1, err := ks.GenerateNewKey(randentropy.Reader, pass)
+	k1, account, err := storeNewKey(ks, rand.Reader, pass)
 	if err != nil {
 		t.Fatal(err)
 	}
-
-	_, err = ks.GetKey(k1.Address, "bar") // wrong passphrase
-	if err == nil {
-		t.Fatal(err)
-	}
-
-	err = ks.DeleteKey(k1.Address, "bar") // wrong passphrase
-	if err == nil {
-		t.Fatal(err)
-	}
-
-	err = ks.DeleteKey(k1.Address, pass) // to clean up
-	if err != nil {
-		t.Fatal(err)
+	if _, err = ks.GetKey(k1.Address, account.File, "bar"); err == nil {
+		t.Fatal("no error for invalid passphrase")
 	}
 }
 
 func TestImportPreSaleKey(t *testing.T) {
+	dir, ks := tmpKeyStore(t, true)
+	defer os.RemoveAll(dir)
+
 	// file content of a presale key file generated with:
 	// python pyethsaletool.py genwallet
 	// with password "foo"
 	fileContent := "{\"encseed\": \"26d87f5f2bf9835f9a47eefae571bc09f9107bb13d54ff12a4ec095d01f83897494cf34f7bed2ed34126ecba9db7b62de56c9d7cd136520a0427bfb11b8954ba7ac39b90d4650d3448e31185affcd74226a68f1e94b1108e6e0a4a91cdd83eba\", \"ethaddr\": \"d4584b5f6229b7be90727b0fc8c6b91bb427821f\", \"email\": \"gustav.simonsson@gmail.com\", \"btcaddr\": \"1EVknXyFC68kKNLkh6YnKzW41svSRoaAcx\"}"
-	ks := newKeyStorePassphrase(common.DefaultDataDir(), LightScryptN, LightScryptP)
 	pass := "foo"
-	_, err := importPreSaleKey(ks, []byte(fileContent), pass)
+	account, _, err := importPreSaleKey(ks, []byte(fileContent), pass)
 	if err != nil {
 		t.Fatal(err)
 	}
+	if account.Address != common.HexToAddress("d4584b5f6229b7be90727b0fc8c6b91bb427821f") {
+		t.Errorf("imported account has wrong address %x", account.Address)
+	}
+	if !strings.HasPrefix(account.File, dir) {
+		t.Errorf("imported account file not in keystore directory: %q", account.File)
+	}
 }
 
 // Test and utils for the key store tests in the Ethereum JSON tests;
@@ -134,51 +135,56 @@ type KeyStoreTestV1 struct {
 }
 
 func TestV3_PBKDF2_1(t *testing.T) {
+	t.Parallel()
 	tests := loadKeyStoreTestV3("testdata/v3_test_vector.json", t)
 	testDecryptV3(tests["wikipage_test_vector_pbkdf2"], t)
 }
 
 func TestV3_PBKDF2_2(t *testing.T) {
+	t.Parallel()
 	tests := loadKeyStoreTestV3("../tests/files/KeyStoreTests/basic_tests.json", t)
 	testDecryptV3(tests["test1"], t)
 }
 
 func TestV3_PBKDF2_3(t *testing.T) {
+	t.Parallel()
 	tests := loadKeyStoreTestV3("../tests/files/KeyStoreTests/basic_tests.json", t)
 	testDecryptV3(tests["python_generated_test_with_odd_iv"], t)
 }
 
 func TestV3_PBKDF2_4(t *testing.T) {
+	t.Parallel()
 	tests := loadKeyStoreTestV3("../tests/files/KeyStoreTests/basic_tests.json", t)
 	testDecryptV3(tests["evilnonce"], t)
 }
 
 func TestV3_Scrypt_1(t *testing.T) {
+	t.Parallel()
 	tests := loadKeyStoreTestV3("testdata/v3_test_vector.json", t)
 	testDecryptV3(tests["wikipage_test_vector_scrypt"], t)
 }
 
 func TestV3_Scrypt_2(t *testing.T) {
+	t.Parallel()
 	tests := loadKeyStoreTestV3("../tests/files/KeyStoreTests/basic_tests.json", t)
 	testDecryptV3(tests["test2"], t)
 }
 
 func TestV1_1(t *testing.T) {
+	t.Parallel()
 	tests := loadKeyStoreTestV1("testdata/v1_test_vector.json", t)
 	testDecryptV1(tests["test1"], t)
 }
 
 func TestV1_2(t *testing.T) {
-	ks := newKeyStorePassphrase("testdata/v1", LightScryptN, LightScryptP)
+	t.Parallel()
+	ks := &keyStorePassphrase{"testdata/v1", LightScryptN, LightScryptP}
 	addr := common.HexToAddress("cb61d5a9c4896fb9658090b597ef0e7be6f7b67e")
-	k, err := ks.GetKey(addr, "g")
+	file := "testdata/v1/cb61d5a9c4896fb9658090b597ef0e7be6f7b67e/cb61d5a9c4896fb9658090b597ef0e7be6f7b67e"
+	k, err := ks.GetKey(addr, file, "g")
 	if err != nil {
 		t.Fatal(err)
 	}
-	if k.Address != addr {
-		t.Fatal(fmt.Errorf("Unexpected address: %v, expected %v", k.Address, addr))
-	}
-
 	privHex := hex.EncodeToString(crypto.FromECDSA(k.PrivateKey))
 	expectedHex := "d1b1178d3529626a1a93e073f65028370d14c7eb0936eb42abef05db6f37ad7d"
 	if privHex != expectedHex {
@@ -227,7 +233,8 @@ func loadKeyStoreTestV1(file string, t *testing.T) map[string]KeyStoreTestV1 {
 }
 
 func TestKeyForDirectICAP(t *testing.T) {
-	key := NewKeyForDirectICAP(randentropy.Reader)
+	t.Parallel()
+	key := NewKeyForDirectICAP(rand.Reader)
 	if !strings.HasPrefix(key.Address.Hex(), "0x00") {
 		t.Errorf("Expected first address byte to be zero, have: %s", key.Address.Hex())
 	}

accounts/presale.go

@@ -31,14 +31,15 @@ import (
 )
 
 // creates a Key and stores that in the given KeyStore by decrypting a presale key JSON
-func importPreSaleKey(keyStore keyStore, keyJSON []byte, password string) (*Key, error) {
+func importPreSaleKey(keyStore keyStore, keyJSON []byte, password string) (Account, *Key, error) {
 	key, err := decryptPreSaleKey(keyJSON, password)
 	if err != nil {
-		return nil, err
+		return Account{}, nil, err
 	}
 	key.Id = uuid.NewRandom()
-	err = keyStore.StoreKey(key, password)
-	return key, err
+	a := Account{Address: key.Address, File: keyStore.JoinPath(keyFileName(key.Address))}
+	err = keyStore.StoreKey(a.File, key, password)
+	return a, key, err
 }
 
 func decryptPreSaleKey(fileContent []byte, password string) (key *Key, err error) {

accounts/testdata/keystore/README

@@ -5,9 +5,9 @@ The "good" key files which are supposed to be loadable are:
 
 - File: UTC--2016-03-22T12-57-55.920751759Z--7ef5a6135f1fd6a02593eedc869c6d41d934aef8
   Address: 0x7ef5a6135f1fd6a02593eedc869c6d41d934aef8
-- File: UTC--2016-03-23T09-30-22.528630983Z--f466859ead1932d743d622cb74fc058882e8648a
+- File: aaa
   Address: 0xf466859ead1932d743d622cb74fc058882e8648a
-- File: UTC--2016-03-23T09-30-26.532308523Z--289d485d9771714cce91d3393d764e1311907acc
+- File: zzz
   Address: 0x289d485d9771714cce91d3393d764e1311907acc
 
 The other files (including this README) are broken in various ways

accounts/testdata/very-light-scrypt.json

+{"address":"45dea0fb0bba44f4fcf290bba71fd57d7117cbb8","crypto":{"cipher":"aes-128-ctr","ciphertext":"b87781948a1befd247bff51ef4063f716cf6c2d3481163e9a8f42e1f9bb74145","cipherparams":{"iv":"dc4926b48a105133d2f16b96833abf1e"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":2,"p":1,"r":8,"salt":"004244bbdc51cadda545b1cfa43cff9ed2ae88e08c61f1479dbb45410722f8f0"},"mac":"39990c1684557447940d4c69e06b1b82b2aceacb43f284df65c956daf3046b85"},"id":"ce541d8d-c79b-40f8-9f8c-20f59616faba","version":3}

accounts/watch.go

+// Copyright 2016 The go-ethereum Authors
+// This file is part of the go-ethereum library.
+//
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The go-ethereum library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
+
+// +build darwin freebsd linux netbsd solaris windows
+
+package accounts
+
+import (
+	"time"
+
+	"github.com/ethereum/go-ethereum/logger"
+	"github.com/ethereum/go-ethereum/logger/glog"
+	"github.com/rjeczalik/notify"
+)
+
+type watcher struct {
+	ac       *addrCache
+	starting bool
+	running  bool
+	ev       chan notify.EventInfo
+	quit     chan struct{}
+}
+
+func newWatcher(ac *addrCache) *watcher {
+	return &watcher{
+		ac:   ac,
+		ev:   make(chan notify.EventInfo, 10),
+		quit: make(chan struct{}),
+	}
+}
+
+// starts the watcher loop in the background.
+// Start a watcher in the background if that's not already in progress.
+// The caller must hold w.ac.mu.
+func (w *watcher) start() {
+	if w.starting || w.running {
+		return
+	}
+	w.starting = true
+	go w.loop()
+}
+
+func (w *watcher) close() {
+	close(w.quit)
+}
+
+func (w *watcher) loop() {
+	defer func() {
+		w.ac.mu.Lock()
+		w.running = false
+		w.starting = false
+		w.ac.mu.Unlock()
+	}()
+
+	err := notify.Watch(w.ac.keydir, w.ev, notify.All)
+	if err != nil {
+		glog.V(logger.Detail).Infof("can't watch %s: %v", w.ac.keydir, err)
+		return
+	}
+	defer notify.Stop(w.ev)
+	glog.V(logger.Detail).Infof("now watching %s", w.ac.keydir)
+	defer glog.V(logger.Detail).Infof("no longer watching %s", w.ac.keydir)
+
+	w.ac.mu.Lock()
+	w.running = true
+	w.ac.mu.Unlock()
+
+	// Wait for file system events and reload.
+	// When an event occurs, the reload call is delayed a bit so that
+	// multiple events arriving quickly only cause a single reload.
+	var (
+		debounce          = time.NewTimer(0)
+		debounceDuration  = 500 * time.Millisecond
+		inCycle, hadEvent bool
+	)
+	defer debounce.Stop()
+	for {
+		select {
+		case <-w.quit:
+			return
+		case <-w.ev:
+			if !inCycle {
+				debounce.Reset(debounceDuration)
+				inCycle = true
+			} else {
+				hadEvent = true
+			}
+		case <-debounce.C:
+			w.ac.mu.Lock()
+			w.ac.reload()
+			w.ac.mu.Unlock()
+			if hadEvent {
+				debounce.Reset(debounceDuration)
+				inCycle, hadEvent = true, false
+			} else {
+				inCycle, hadEvent = false, false
+			}
+		}
+	}
+}

accounts/watch_fallback.go

+// Copyright 2016 The go-ethereum Authors
+// This file is part of the go-ethereum library.
+//
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The go-ethereum library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
+
+// +build !darwin,!freebsd,!linux,!netbsd,!solaris,!windows
+
+// This is the fallback implementation of directory watching.
+// It is used on unsupported platforms.
+
+package accounts
+
+type watcher struct{ running bool }
+
+func newWatcher(*addrCache) *watcher { return new(watcher) }
+func (*watcher) start()              {}
+func (*watcher) close()              {}

cmd/geth/accountcmd.go

@@ -168,7 +168,7 @@ nodes.
 func accountList(ctx *cli.Context) {
 	accman := utils.MakeAccountManager(ctx)
 	for i, acct := range accman.Accounts() {
-		fmt.Printf("Account #%d: %x\n", i, acct)
+		fmt.Printf("Account #%d: {%x} %s\n", i, acct.Address, acct.File)
 	}
 }
 
@@ -230,7 +230,7 @@ func accountCreate(ctx *cli.Context) {
 	if err != nil {
 		utils.Fatalf("Failed to create account: %v", err)
 	}
-	fmt.Printf("Address: %x\n", account)
+	fmt.Printf("Address: {%x}\n", account.Address)
 }
 
 // accountUpdate transitions an account from a previous format to the current
@@ -265,7 +265,7 @@ func importWallet(ctx *cli.Context) {
 	if err != nil {
 		utils.Fatalf("Could not create the account: %v", err)
 	}
-	fmt.Printf("Address: %x\n", acct)
+	fmt.Printf("Address: {%x}\n", acct.Address)
 }
 
 func accountImport(ctx *cli.Context) {
@@ -279,5 +279,5 @@ func accountImport(ctx *cli.Context) {
 	if err != nil {
 		utils.Fatalf("Could not create the account: %v", err)
 	}
-	fmt.Printf("Address: %x\n", acct)
+	fmt.Printf("Address: {%x}\n", acct.Address)
 }

cmd/geth/accountcmd_test.go

@@ -19,6 +19,7 @@ package main
 import (
 	"io/ioutil"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -50,11 +51,19 @@ func TestAccountList(t *testing.T) {
 	datadir := tmpDatadirWithKeystore(t)
 	geth := runGeth(t, "--datadir", datadir, "account")
 	defer geth.expectExit()
-	geth.expect(`
-Account #0: {7ef5a6135f1fd6a02593eedc869c6d41d934aef8}
-Account #1: {f466859ead1932d743d622cb74fc058882e8648a}
-Account #2: {289d485d9771714cce91d3393d764e1311907acc}
+	if runtime.GOOS == "windows" {
+		geth.expect(`
+Account #0: {7ef5a6135f1fd6a02593eedc869c6d41d934aef8} {{.Datadir}}\keystore\UTC--2016-03-22T12-57-55.920751759Z--7ef5a6135f1fd6a02593eedc869c6d41d934aef8
+Account #1: {f466859ead1932d743d622cb74fc058882e8648a} {{.Datadir}}\keystore\aaa
+Account #2: {289d485d9771714cce91d3393d764e1311907acc} {{.Datadir}}\keystore\zzz
+`)
+	} else {
+		geth.expect(`
+Account #0: {7ef5a6135f1fd6a02593eedc869c6d41d934aef8} {{.Datadir}}/keystore/UTC--2016-03-22T12-57-55.920751759Z--7ef5a6135f1fd6a02593eedc869c6d41d934aef8
+Account #1: {f466859ead1932d743d622cb74fc058882e8648a} {{.Datadir}}/keystore/aaa
+Account #2: {289d485d9771714cce91d3393d764e1311907acc} {{.Datadir}}/keystore/zzz
 `)
+	}
 }
 
 func TestAccountNew(t *testing.T) {