diff --git a/accounts/keystore/account_cache.go b/accounts/keystore/account_cache.go index 82a61b75a6..fb7c6ec7ca 100644 --- a/accounts/keystore/account_cache.go +++ b/accounts/keystore/account_cache.go @@ -27,6 +27,8 @@ import ( "sync" "time" + "io/ioutil" + "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/log" @@ -76,7 +78,8 @@ type accountCache struct { // fileCache is a cache of files seen during scan of keystore type fileCache struct { - files *set.SetNonTS //We maintain sync ourselves + all *set.SetNonTS //List of all files + mtime *time.Time //Latest mtime seen mu sync.RWMutex } @@ -86,7 +89,8 @@ func newAccountCache(keydir string) (*accountCache, chan struct{}) { byAddr: make(map[common.Address][]accounts.Account), notify: make(chan struct{}, 1), fileC: fileCache{ - files: set.NewNonTS(), + all: set.NewNonTS(), + mtime: new(time.Time), }, } ac.watcher = newWatcher(ac) @@ -235,53 +239,59 @@ func (ac *accountCache) close() { ac.mu.Unlock() } -// listDir reads the directory named by dirname and returns -// a list of directory sorted by filename. -// This code copies ioutil.ReadDir, but does not perform 'lstat' on each file -// since it uses Readdirnames instead of Readdir -func listDir(dirname string) ([]string, error) { - f, err := os.Open(dirname) - if err != nil { - return nil, err - } - defer f.Close() - return f.Readdirnames(-1) -} - // scanFiles performs a new scan on the given directory, compares against the already -// cached filenames, and returns new and missing file lists. -func (fc *fileCache) scanFiles(keyDir string) (set.Interface, set.Interface, error) { +// cached filenames, and returns file sets: new, missing , modified +func (fc *fileCache) scanFiles(keyDir string) (set.Interface, set.Interface, set.Interface, error) { t0 := time.Now() - files, err := listDir(keyDir) + files, err := ioutil.ReadDir(keyDir) t1 := time.Now() if err != nil { - return nil, nil, err + return nil, nil, nil, err } + fc.mu.RLock() + previous_mtime := fc.mtime + fc.mu.RUnlock() + filesNow := set.NewNonTS() - for _, fname := range files { - path := filepath.Join(keyDir, fname) - if skipKeyFile(fname) { + moddedFiles := set.NewNonTS() + newMtime := new(time.Time) + for _, fi := range files { + modTime := fi.ModTime() + path := filepath.Join(keyDir, fi.Name()) + if skipKeyFile(fi) { log.Trace("Ignoring file on account scan", "path", path) continue } filesNow.Add(path) + if modTime.After(*previous_mtime) { + moddedFiles.Add(path) + } + if modTime.After(*newMtime) { + newMtime = &modTime + } } t2 := time.Now() + fc.mu.Lock() - missingFiles := set.Difference(fc.files, filesNow) - newFiles := set.Difference(filesNow, fc.files) - fc.files = filesNow + // Missing = previous - current + missing := set.Difference(fc.all, filesNow) + // New = current - previous + newFiles := set.Difference(filesNow, fc.all) + // Modified = modified - new + modified := set.Difference(moddedFiles, newFiles) + fc.all = filesNow + fc.mtime = newMtime fc.mu.Unlock() t3 := time.Now() log.Debug("FS scan times", "list", t1.Sub(t0), "set", t2.Sub(t1), "diff", t3.Sub(t2)) - return newFiles, missingFiles, nil + return newFiles, missing, modified, nil } // scanAccounts checks if any changes have occurred on the filesystem, and // updates the account cache accordingly func (ac *accountCache) scanAccounts() error { - newFiles, missingFiles, err := ac.fileC.scanFiles(ac.keydir) + newFiles, missingFiles, modified, err := ac.fileC.scanFiles(ac.keydir) t1 := time.Now() if err != nil { log.Debug("Failed to reload keystore contents", "err", err) @@ -293,20 +303,13 @@ func (ac *accountCache) scanAccounts() error { Address string `json:"address"` } ) - - for _, p := range newFiles.List() { - path, _ := p.(string) - + readAccount := func(path string) *accounts.Account { fd, err := os.Open(path) - // Skip misc special files, directories (yes, symlinks too). - //if lerr != nil || fi.IsDir() || fi.Mode()&os.ModeType != 0 { - // continue - //} - if err != nil { log.Trace("Failed to open keystore file", "path", path, "err", err) - continue + return nil } + defer fd.Close() buf.Reset(fd) // Parse the address. keyJSON.Address = "" @@ -318,15 +321,32 @@ func (ac *accountCache) scanAccounts() error { case (addr == common.Address{}): log.Debug("Failed to decode keystore key", "path", path, "err", "missing or zero address") default: - a := accounts.Account{Address: addr, URL: accounts.URL{Scheme: KeyStoreScheme, Path: path}} - ac.add(a) + return &accounts.Account{Address: addr, URL: accounts.URL{Scheme: KeyStoreScheme, Path: path}} + } + return nil + } + + for _, p := range newFiles.List() { + path, _ := p.(string) + a := readAccount(path) + if a != nil { + ac.add(*a) } - fd.Close() } for _, p := range missingFiles.List() { path, _ := p.(string) ac.deleteByFile(path) } + + for _, p := range modified.List() { + path, _ := p.(string) + a := readAccount(path) + ac.deleteByFile(path) + if a != nil { + ac.add(*a) + } + } + t2 := time.Now() select { @@ -338,9 +358,13 @@ func (ac *accountCache) scanAccounts() error { return nil } -func skipKeyFile(fn string) bool { +func skipKeyFile(fi os.FileInfo) bool { // Skip editor backups and UNIX-style hidden files. - if strings.HasSuffix(fn, "~") || strings.HasPrefix(fn, ".") { + 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 diff --git a/accounts/keystore/account_cache_test.go b/accounts/keystore/account_cache_test.go index ab8aa9e6c3..134ae6230f 100644 --- a/accounts/keystore/account_cache_test.go +++ b/accounts/keystore/account_cache_test.go @@ -30,6 +30,7 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/common" + "io/ioutil" ) var ( @@ -56,11 +57,11 @@ func TestWatchNewFile(t *testing.T) { dir, ks := tmpKeyStore(t, false) defer os.RemoveAll(dir) - // Ensure the watcher is started before adding any files. + // Ensure the watcher is started before adding any all. ks.Accounts() time.Sleep(200 * time.Millisecond) - // Move in the files. + // Move in the all. wantAccounts := make([]accounts.Account, len(cachetestAccounts)) for i := range cachetestAccounts { wantAccounts[i] = accounts.Account{ @@ -295,3 +296,101 @@ func TestCacheFind(t *testing.T) { } } } + +func waitForAccounts(wantAccounts []accounts.Account, ks *KeyStore) error { + var list []accounts.Account + for d := 200 * time.Millisecond; d < 8*time.Second; d *= 2 { + list = ks.Accounts() + if reflect.DeepEqual(list, wantAccounts) { + // ks should have also received change notifications + select { + case <-ks.changes: + default: + return fmt.Errorf("wasn't notified of new accounts") + } + return nil + } + time.Sleep(d) + } + return fmt.Errorf("\ngot %v\nwant %v", list, wantAccounts) +} + +// TestUpdatedKeyfileContents tests that updating the contents of a keystore file +// is noticed by the watcher, and the account cache is updated accordingly +func TestUpdatedKeyfileContents(t *testing.T) { + // Create a temporary kesytore to test with + rand.Seed(time.Now().UnixNano()) + dir := filepath.Join(os.TempDir(), fmt.Sprintf("eth-keystore-watch-test-%d-%d", os.Getpid(), rand.Int())) + ks := NewKeyStore(dir, LightScryptN, LightScryptP) + + list := ks.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") + + // Place one of our testfiles in there + if err := cp.CopyFile(file, cachetestAccounts[0].URL.Path); err != nil { + t.Fatal(err) + } + + // ks should see the account. + wantAccounts := []accounts.Account{cachetestAccounts[0]} + wantAccounts[0].URL = accounts.URL{Scheme: KeyStoreScheme, Path: file} + if err := waitForAccounts(wantAccounts, ks); err != nil { + t.Error(err) + return + } + + copyContents := func(dst, src string) error { + data, err := ioutil.ReadFile(src) + if err != nil { + return err + } + err = ioutil.WriteFile(dst, data, 0644) + if err != nil { + return err + } + return nil + } + // Now replace file contents + if err := copyContents(file, cachetestAccounts[1].URL.Path); err != nil { + t.Fatal(err) + return + } + wantAccounts = []accounts.Account{cachetestAccounts[1]} + wantAccounts[0].URL = accounts.URL{Scheme: KeyStoreScheme, Path: file} + if err := waitForAccounts(wantAccounts, ks); err != nil { + t.Errorf("First replacement failed") + t.Error(err) + return + } + + // Now replace file contents again + if err := copyContents(file, cachetestAccounts[2].URL.Path); err != nil { + t.Fatal(err) + return + } + wantAccounts = []accounts.Account{cachetestAccounts[2]} + wantAccounts[0].URL = accounts.URL{Scheme: KeyStoreScheme, Path: file} + if err := waitForAccounts(wantAccounts, ks); err != nil { + t.Errorf("Second replacement failed") + t.Error(err) + return + } + // Now replace file contents with crap + if err := ioutil.WriteFile(file, []byte("foo"), 0644); err != nil { + t.Fatal(err) + return + } + if err := waitForAccounts([]accounts.Account{}, ks); err != nil { + t.Errorf("Emptying account file failed") + t.Error(err) + return + } +} diff --git a/accounts/keystore/watch.go b/accounts/keystore/watch.go index c7f76850d5..602300b107 100644 --- a/accounts/keystore/watch.go +++ b/accounts/keystore/watch.go @@ -20,7 +20,7 @@ package keystore import ( "time" - + "github.com/ethereum/go-ethereum/log" "github.com/rjeczalik/notify" ) @@ -92,7 +92,7 @@ func (w *watcher) loop() { return case <-w.ev: // Trigger the scan (with delay), if not already triggered - if !rescanTriggered{ + if !rescanTriggered { debounce.Reset(debounceDuration) rescanTriggered = true }