From c875dd458bd0411a07e67204bbf302b6c1dc46f6 Mon Sep 17 00:00:00 2001
From: Victor Farazdagi <simple.square@gmail.com>
Date: Fri, 22 May 2020 14:00:41 +0300
Subject: [PATCH] Handles issue with empty dirs on validator accounts create
 (#5940)

* handles issue with empty dirs on validator accounts create
* better comments
* Merge refs/heads/master into fix-validator-accounts-create
* Merge refs/heads/master into fix-validator-accounts-create
* Terence's review
---
 validator/accounts/account.go           | 15 ++++++++++-----
 validator/accounts/account_test.go      | 20 +++++++++++++++++---
 validator/keymanager/direct_keystore.go |  2 +-
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/validator/accounts/account.go b/validator/accounts/account.go
index 375e2bc08..12246523f 100644
--- a/validator/accounts/account.go
+++ b/validator/accounts/account.go
@@ -118,7 +118,9 @@ func NewValidatorAccount(directory string, password string) error {
 }
 
 // Exists checks if a validator account at a given keystore path exists.
-func Exists(keystorePath string) (bool, error) {
+// assertNonEmpty is a boolean used to determine whether to check that 
+// the provided directory exists.
+func Exists(keystorePath string, assertNonEmpty bool) (bool, error) {
 	/* #nosec */
 	f, err := os.Open(keystorePath)
 	if err != nil {
@@ -130,10 +132,13 @@ func Exists(keystorePath string) (bool, error) {
 		}
 	}()
 
-	_, err = f.Readdirnames(1) // Or f.Readdir(1)
-	if err == io.EOF {
-		return false, nil
+	if assertNonEmpty {
+		_, err = f.Readdirnames(1) // Or f.Readdir(1)
+		if err == io.EOF {
+			return false, nil
+		}
 	}
+
 	return true, err
 }
 
@@ -141,7 +146,7 @@ func Exists(keystorePath string) (bool, error) {
 func CreateValidatorAccount(path string, passphrase string) (string, string, error) {
 	// Forces user to create directory if using non-default path.
 	if path != DefaultValidatorDir() {
-		exists, err := Exists(path)
+		exists, err := Exists(path, false /* assertNonEmpty */)
 		if err != nil {
 			return path, passphrase, err
 		}
diff --git a/validator/accounts/account_test.go b/validator/accounts/account_test.go
index da98fc6a6..916f79b62 100644
--- a/validator/accounts/account_test.go
+++ b/validator/accounts/account_test.go
@@ -46,12 +46,26 @@ func TestNewValidatorAccount_AccountExists(t *testing.T) {
 }
 
 func TestNewValidatorAccount_CreateValidatorAccount(t *testing.T) {
-	directory := "foobar"
-	_, _, err := CreateValidatorAccount(directory, "foobar")
-	wantErrString := fmt.Sprintf("path %q does not exist", directory)
+	directory := testutil.TempDir() + "/testkeystore"
+	defer func() {
+		if err := os.RemoveAll(directory); err != nil {
+			t.Logf("Could not remove directory: %v", err)
+		}
+	}()
+	_, _, err := CreateValidatorAccount("foobar", "foobar")
+	wantErrString := fmt.Sprintf("path %q does not exist", "foobar")
 	if err == nil || err.Error() != wantErrString {
 		t.Errorf("expected error not thrown, want: %v, got: %v", wantErrString, err)
 	}
+
+	// Make sure that empty existing directory doesn't trigger any errors.
+	if err := os.Mkdir(directory, 0777); err != nil {
+		t.Fatal(err)
+	}
+	_, _, err = CreateValidatorAccount(directory, "foobar")
+	if err != nil {
+		t.Error(err)
+	}
 }
 
 func TestHandleEmptyFlags_FlagsSet(t *testing.T) {
diff --git a/validator/keymanager/direct_keystore.go b/validator/keymanager/direct_keystore.go
index 1115fd756..3d8b81506 100644
--- a/validator/keymanager/direct_keystore.go
+++ b/validator/keymanager/direct_keystore.go
@@ -49,7 +49,7 @@ func NewKeystore(input string) (KeyManager, string, error) {
 	}
 	log.WithField("keystorePath", opts.Path).Info("Checking validator keys")
 
-	exists, err := accounts.Exists(opts.Path)
+	exists, err := accounts.Exists(opts.Path, true /* assertNonEmpty */)
 	if err != nil {
 		return nil, keystoreOptsHelp, err
 	}