From 6c623b203e250b45e4d495ffd87d088598e07917 Mon Sep 17 00:00:00 2001 From: Cong Zhao Date: Sat, 13 Oct 2018 10:22:07 +0800 Subject: [PATCH 1/5] #1255 make keybase opened with readonly option to support better parallelization between gaiacli --- Gopkg.lock | 1 + PENDING.md | 1 + client/keys/add.go | 6 +++--- client/keys/delete.go | 4 ++-- client/keys/update.go | 4 ++-- client/keys/utils.go | 31 +++++++++++++++++++++---------- client/lcd/test_helpers.go | 2 +- server/init.go | 2 +- 8 files changed, 32 insertions(+), 19 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index c679450b9c..06a17d015b 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -671,6 +671,7 @@ "github.com/spf13/viper", "github.com/stretchr/testify/assert", "github.com/stretchr/testify/require", + "github.com/syndtr/goleveldb/leveldb/opt", "github.com/tendermint/go-amino", "github.com/tendermint/iavl", "github.com/tendermint/tendermint/abci/server", diff --git a/PENDING.md b/PENDING.md index 3a92041c33..4f3e03b21c 100644 --- a/PENDING.md +++ b/PENDING.md @@ -159,6 +159,7 @@ IMPROVEMENTS * Gaia CLI (`gaiacli`) * [cli] #2060 removed `--select` from `block` command * [cli] #2128 fixed segfault when exporting directly after `gaiad init` + * [cli] [\#1255](https://github.com/cosmos/cosmos-sdk/issues/1255) make keybase opened with readonly option for query-purpose cli commands * Gaia * [x/stake] [#2023](https://github.com/cosmos/cosmos-sdk/pull/2023) Terminate iteration loop in `UpdateBondedValidators` and `UpdateBondedValidatorsFull` when the first revoked validator is encountered and perform a sanity check. diff --git a/client/keys/add.go b/client/keys/add.go index f13fac547b..6b6e381174 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -63,7 +63,7 @@ func runAddCmd(cmd *cobra.Command, args []string) error { return errMissingName() } name = args[0] - kb, err = GetKeyBase() + kb, err = GetKeyBaseWithWritePerm() if err != nil { return err } @@ -174,7 +174,7 @@ func AddNewKeyRequestHandler(indent bool) http.HandlerFunc { var kb keys.Keybase var m NewKeyBody - kb, err := GetKeyBase() + kb, err := GetKeyBaseWithWritePerm() if err != nil { w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) @@ -311,7 +311,7 @@ func RecoverRequestHandler(indent bool) http.HandlerFunc { return } - kb, err := GetKeyBase() + kb, err := GetKeyBaseWithWritePerm() if err != nil { w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) diff --git a/client/keys/delete.go b/client/keys/delete.go index 23fc41ffd6..84adc00204 100644 --- a/client/keys/delete.go +++ b/client/keys/delete.go @@ -25,7 +25,7 @@ func deleteKeyCommand() *cobra.Command { func runDeleteCmd(cmd *cobra.Command, args []string) error { name := args[0] - kb, err := GetKeyBase() + kb, err := GetKeyBaseWithWritePerm() if err != nil { return err } @@ -73,7 +73,7 @@ func DeleteKeyRequestHandler(w http.ResponseWriter, r *http.Request) { return } - kb, err = GetKeyBase() + kb, err = GetKeyBaseWithWritePerm() if err != nil { w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) diff --git a/client/keys/update.go b/client/keys/update.go index 9416e9b0b4..abf616a653 100644 --- a/client/keys/update.go +++ b/client/keys/update.go @@ -26,7 +26,7 @@ func runUpdateCmd(cmd *cobra.Command, args []string) error { name := args[0] buf := client.BufferStdin() - kb, err := GetKeyBase() + kb, err := GetKeyBaseWithWritePerm() if err != nil { return err } @@ -74,7 +74,7 @@ func UpdateKeyRequestHandler(w http.ResponseWriter, r *http.Request) { return } - kb, err = GetKeyBase() + kb, err = GetKeyBaseWithWritePerm() if err != nil { w.WriteHeader(http.StatusInternalServerError) w.Write([]byte(err.Error())) diff --git a/client/keys/utils.go b/client/keys/utils.go index 4ca8fc8f42..f2ea638d9b 100644 --- a/client/keys/utils.go +++ b/client/keys/utils.go @@ -2,6 +2,7 @@ package keys import ( "fmt" + "github.com/syndtr/goleveldb/leveldb/opt" "path/filepath" "github.com/spf13/viper" @@ -25,14 +26,6 @@ var keybase keys.Keybase type bechKeyOutFn func(keyInfo keys.Info) (KeyOutput, error) -// TODO make keybase take a database not load from the directory - -// initialize a keybase based on the configuration -func GetKeyBase() (keys.Keybase, error) { - rootDir := viper.GetString(cli.HomeFlag) - return GetKeyBaseFromDir(rootDir) -} - // GetKeyInfo returns key info for a given name. An error is returned if the // keybase cannot be retrieved or getting the info fails. func GetKeyInfo(name string) (keys.Info, error) { @@ -82,10 +75,28 @@ func ReadPassphraseFromStdin(name string) (string, error) { return passphrase, nil } +// TODO make keybase take a database not load from the directory + // initialize a keybase based on the configuration -func GetKeyBaseFromDir(rootDir string) (keys.Keybase, error) { +func GetKeyBase() (keys.Keybase, error) { + rootDir := viper.GetString(cli.HomeFlag) + return getKeyBaseFromDirWithOpts(rootDir, &opt.Options{ReadOnly: true}) +} + +// initialize a keybase based on the configuration with write permission +func GetKeyBaseWithWritePerm() (keys.Keybase, error) { + rootDir := viper.GetString(cli.HomeFlag) + return GetKeyBaseFromDirWithWritePerm(rootDir) +} + +// initialize a keybase at particular dir with write permission +func GetKeyBaseFromDirWithWritePerm(rootDir string) (keys.Keybase, error) { + return getKeyBaseFromDirWithOpts(rootDir, &opt.Options{ReadOnly: false}) +} + +func getKeyBaseFromDirWithOpts(rootDir string, o *opt.Options) (keys.Keybase, error) { if keybase == nil { - db, err := dbm.NewGoLevelDB(KeyDBName, filepath.Join(rootDir, "keys")) + db, err := dbm.NewGoLevelDBWithOpts(KeyDBName, filepath.Join(rootDir, "keys"), o) if err != nil { return nil, err } diff --git a/client/lcd/test_helpers.go b/client/lcd/test_helpers.go index ee7fdbdef5..2a81239ac6 100644 --- a/client/lcd/test_helpers.go +++ b/client/lcd/test_helpers.go @@ -94,7 +94,7 @@ func GetKeyBase(t *testing.T) crkeys.Keybase { viper.Set(cli.HomeFlag, dir) - keybase, err := keys.GetKeyBase() + keybase, err := keys.GetKeyBaseWithWritePerm() require.NoError(t, err) return keybase diff --git a/server/init.go b/server/init.go index 65a896e2ac..6a5d821c17 100644 --- a/server/init.go +++ b/server/init.go @@ -115,7 +115,7 @@ func GenerateCoinKey() (sdk.AccAddress, string, error) { func GenerateSaveCoinKey(clientRoot, keyName, keyPass string, overwrite bool) (sdk.AccAddress, string, error) { // get the keystore from the client - keybase, err := clkeys.GetKeyBaseFromDir(clientRoot) + keybase, err := clkeys.GetKeyBaseFromDirWithWritePerm(clientRoot) if err != nil { return sdk.AccAddress([]byte{}), "", err } From add15b5b2814fe79c956d4df7df800aa8107a3c9 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Mon, 22 Oct 2018 14:29:27 -0700 Subject: [PATCH 2/5] Merge review comments/changes --- client/keys/utils.go | 15 ++++++++++----- crypto/keys/keybase.go | 5 +++++ crypto/keys/types.go | 3 +++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/client/keys/utils.go b/client/keys/utils.go index f2ea638d9b..b10faf6988 100644 --- a/client/keys/utils.go +++ b/client/keys/utils.go @@ -77,21 +77,26 @@ func ReadPassphraseFromStdin(name string) (string, error) { // TODO make keybase take a database not load from the directory -// initialize a keybase based on the configuration +// GetKeyBase initializes a keybase based on the configuration. func GetKeyBase() (keys.Keybase, error) { rootDir := viper.GetString(cli.HomeFlag) - return getKeyBaseFromDirWithOpts(rootDir, &opt.Options{ReadOnly: true}) + return GetKeyBaseFromDir(rootDir) } -// initialize a keybase based on the configuration with write permission +// GetKeyBaseWithWritePerm initialize a keybase based on the configuration with write permissions. func GetKeyBaseWithWritePerm() (keys.Keybase, error) { rootDir := viper.GetString(cli.HomeFlag) return GetKeyBaseFromDirWithWritePerm(rootDir) } -// initialize a keybase at particular dir with write permission +// GetKeyBaseFromDirWithWritePerm initializes a keybase at a particular dir with write permissions. func GetKeyBaseFromDirWithWritePerm(rootDir string) (keys.Keybase, error) { - return getKeyBaseFromDirWithOpts(rootDir, &opt.Options{ReadOnly: false}) + return getKeyBaseFromDirWithOpts(rootDir, nil) +} + +// GetKeyBaseFromDir initializes a read-only keybase at a particular dir. +func GetKeyBaseFromDir(rootDir string) (keys.Keybase, error) { + return getKeyBaseFromDirWithOpts(rootDir, &opt.Options{ReadOnly: true}) } func getKeyBaseFromDirWithOpts(rootDir string, o *opt.Options) (keys.Keybase, error) { diff --git a/crypto/keys/keybase.go b/crypto/keys/keybase.go index ddcd0357e1..b75c3473ba 100644 --- a/crypto/keys/keybase.go +++ b/crypto/keys/keybase.go @@ -426,6 +426,11 @@ func (kb dbKeybase) Update(name, oldpass string, getNewpass func() (string, erro } } +// CloseDB releases the lock and closes the storage backend. +func (kb dbKeybase) CloseDB() { + kb.db.Close() +} + func (kb dbKeybase) writeLocalKey(priv tmcrypto.PrivKey, name, passphrase string) Info { // encrypt private key using passphrase privArmor := mintkey.EncryptArmorPrivKey(priv, passphrase) diff --git a/crypto/keys/types.go b/crypto/keys/types.go index f5194748a0..ff90c3205d 100644 --- a/crypto/keys/types.go +++ b/crypto/keys/types.go @@ -47,6 +47,9 @@ type Keybase interface { // *only* works on locally-stored keys. Temporary method until we redo the exporting API ExportPrivateKeyObject(name string, passphrase string) (crypto.PrivKey, error) + + // Close closes the database. + CloseDB() } // KeyType reflects a human-readable type for key listing. From 44a6c21ad9b28aea727a2944ecd6393a489c65dc Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Mon, 22 Oct 2018 14:43:57 -0700 Subject: [PATCH 3/5] Add tests --- client/keys/utils_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 client/keys/utils_test.go diff --git a/client/keys/utils_test.go b/client/keys/utils_test.go new file mode 100644 index 0000000000..39ca634289 --- /dev/null +++ b/client/keys/utils_test.go @@ -0,0 +1,39 @@ +package keys + +import ( + "github.com/cosmos/cosmos-sdk/crypto/keys" + "github.com/stretchr/testify/require" + "io/ioutil" + "os" + "testing" +) + +func TestGetKeyBaseLocks(t *testing.T) { + dir, err := ioutil.TempDir("", "cosmos-sdk-keys") + require.Nil(t, err) + defer os.RemoveAll(dir) + + // Acquire db + kb, err := GetKeyBaseFromDirWithWritePerm(dir) + require.Nil(t, err) + _, _, err = kb.CreateMnemonic("foo", keys.English, "12345678", keys.Secp256k1) + require.Nil(t, err) + // Reset global variable + keybase = nil + // Try to acquire another keybase from the same storage + _, err = GetKeyBaseFromDirWithWritePerm(dir) + require.NotNil(t, err) + _, err = GetKeyBaseFromDirWithWritePerm(dir) + require.NotNil(t, err) + + // Close the db and try to acquire the lock + kb.CloseDB() + kb, err = GetKeyBaseFromDirWithWritePerm(dir) + require.Nil(t, err) + + // Try to acquire another read-only keybase from the same storage + _, err = GetKeyBaseFromDir(dir) + require.Nil(t, err) + + kb.CloseDB() +} From 990852a5b2bf9b73e67cd202c2f7992580f3de3b Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Tue, 23 Oct 2018 10:43:45 -0700 Subject: [PATCH 4/5] Update PENDING.md Co-Authored-By: alessio --- PENDING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/PENDING.md b/PENDING.md index 4f3e03b21c..cfadb97cd0 100644 --- a/PENDING.md +++ b/PENDING.md @@ -159,7 +159,8 @@ IMPROVEMENTS * Gaia CLI (`gaiacli`) * [cli] #2060 removed `--select` from `block` command * [cli] #2128 fixed segfault when exporting directly after `gaiad init` - * [cli] [\#1255](https://github.com/cosmos/cosmos-sdk/issues/1255) make keybase opened with readonly option for query-purpose cli commands + * [cli] [\#1255](https://github.com/cosmos/cosmos-sdk/issues/1255) open KeyBase in read-only mode + for query-purpose CLI commands * Gaia * [x/stake] [#2023](https://github.com/cosmos/cosmos-sdk/pull/2023) Terminate iteration loop in `UpdateBondedValidators` and `UpdateBondedValidatorsFull` when the first revoked validator is encountered and perform a sanity check. From 450873d0809ecd6df1fde6cb99d5c517550ea49e Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Tue, 23 Oct 2018 10:47:20 -0700 Subject: [PATCH 5/5] Update client/keys/utils.go Co-Authored-By: alessio --- client/keys/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/keys/utils.go b/client/keys/utils.go index b10faf6988..522015e1b4 100644 --- a/client/keys/utils.go +++ b/client/keys/utils.go @@ -77,7 +77,7 @@ func ReadPassphraseFromStdin(name string) (string, error) { // TODO make keybase take a database not load from the directory -// GetKeyBase initializes a keybase based on the configuration. +// GetKeyBase initializes a read-only KeyBase based on the configuration. func GetKeyBase() (keys.Keybase, error) { rootDir := viper.GetString(cli.HomeFlag) return GetKeyBaseFromDir(rootDir)