From e7d339b8fe9381797152c02eb2ae816f7d4f256b Mon Sep 17 00:00:00 2001 From: swelf Date: Mon, 27 Dec 2021 17:33:20 +0300 Subject: [PATCH 1/4] resolved conflists --- crypto/keyring/keyring.go | 31 +++++++++++++++++++--------- crypto/keyring/keyring_test.go | 37 ++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index 4011b2a2847e..4b8e7162fd4f 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -808,20 +808,31 @@ func (ks keystore) writeRecord(k *Record) error { return nil } -// existsInDb returns (true, nil) if either addr or name exist is in keystore DB. -// On the other hand, it returns (false, error) if Get method returns error different from keyring.ErrKeyNotFound -func (ks keystore) existsInDb(addr sdk.Address, name string) (bool, error) { - if _, err := ks.db.Get(addrHexKeyAsString(addr)); err == nil { - return true, nil // address lookup succeeds - info exists - } else if err != keyring.ErrKeyNotFound { - return false, err // received unexpected error - returns error +// existsInDb returns true if key is in DB. Error is returned only when we have error +// different thant ErrKeyNotFound +func (ks keystore) existsInDb(addr sdk.Address, name string) (bool, error) { + _, errAddr := ks.db.Get(addrHexKeyAsString(addr)) + if errAddr != nil && errAddr != keyring.ErrKeyNotFound { + return false, errAddr // received unexpected error - returns error } - if _, err := ks.db.Get(name); err == nil { + _, errInfo := ks.db.Get(infoKey(name)) + if errInfo == nil { return true, nil // uid lookup succeeds - info exists - } else if err != keyring.ErrKeyNotFound { - return false, err // received unexpected error - returns + } else if errInfo != keyring.ErrKeyNotFound { + return false, errInfo // received unexpected error - returns + } + + // looking for an issue, record with meta (getByAddress) exists, but record with public key itself does not + if errAddr == nil && errors.Is(errInfo, keyring.ErrKeyNotFound) { + fmt.Fprintf(os.Stderr, "address \"%s\" exists but pubkey itself does not\n", hex.EncodeToString(addr.Bytes())) + fmt.Fprintln(os.Stderr, "recreating pubkey record") + err := ks.db.Remove(addrHexKeyAsString(addr)) + if err != nil { + return true, err + } + return false, nil } // both lookups failed, info does not exist diff --git a/crypto/keyring/keyring_test.go b/crypto/keyring/keyring_test.go index 3afd35676fb7..ad3e109b74d0 100644 --- a/crypto/keyring/keyring_test.go +++ b/crypto/keyring/keyring_test.go @@ -1064,6 +1064,43 @@ func TestAltKeyring_SaveOfflineKey(t *testing.T) { require.Len(t, list, 1) } +func TestNonConsistentKeyring_SavePubKey(t *testing.T) { + cdc := getCodec() + kr, err := New(t.Name(), BackendTest, t.TempDir(), nil, cdc) + require.NoError(t, err) + + list, err := kr.List() + require.NoError(t, err) + require.Empty(t, list) + + key := someKey + priv := ed25519.GenPrivKey() + pub := priv.PubKey() + + k, err := kr.SaveOfflineKey(key, pub) + require.Nil(t, err) + + // broken keyring state test + unsafeKr, ok := kr.(keystore) + require.True(t, ok) + // we lost public key for some reason, but still have an address record + unsafeKr.db.Remove(infoKey(key)) + list, err = kr.List() + require.NoError(t, err) + require.Equal(t, 0, len(list)) + + k, err = kr.SaveOfflineKey(key, pub) + require.Nil(t, err) + pubKey, err := k.GetPubKey() + require.NoError(t, err) + require.Equal(t, pub, pubKey) + require.Equal(t, key, k.Name) + + list, err = kr.List() + require.NoError(t, err) + require.Equal(t, 1, len(list)) +} + func TestAltKeyring_SaveMultisig(t *testing.T) { cdc := getCodec() kr, err := New(t.Name(), BackendTest, t.TempDir(), nil, cdc) From 1000063baad50bf01cf4e2362a816c4aa34b2e60 Mon Sep 17 00:00:00 2001 From: swelf Date: Mon, 27 Dec 2021 19:00:00 +0300 Subject: [PATCH 2/4] fixed code comment --- crypto/keyring/keyring.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index 4b8e7162fd4f..d2f4d02b343b 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -809,8 +809,9 @@ func (ks keystore) writeRecord(k *Record) error { } -// existsInDb returns true if key is in DB. Error is returned only when we have error -// different thant ErrKeyNotFound +// existsInDb returns (true, nil) if either addr or name exist is in keystore DB. +// On the other hand, it returns (false, error) if Get method returns error different from keyring.ErrKeyNotFound +// In case of inconsistent keyring, it recovers it automatically. func (ks keystore) existsInDb(addr sdk.Address, name string) (bool, error) { _, errAddr := ks.db.Get(addrHexKeyAsString(addr)) if errAddr != nil && errAddr != keyring.ErrKeyNotFound { From d83bdaebf1961c167016915c7515009da47a978a Mon Sep 17 00:00:00 2001 From: swelf Date: Thu, 30 Dec 2021 10:21:29 +0300 Subject: [PATCH 3/4] improved errors processing --- crypto/keyring/keyring.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index d2f4d02b343b..db4eb8fbb9ca 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -808,20 +808,19 @@ func (ks keystore) writeRecord(k *Record) error { return nil } - // existsInDb returns (true, nil) if either addr or name exist is in keystore DB. // On the other hand, it returns (false, error) if Get method returns error different from keyring.ErrKeyNotFound // In case of inconsistent keyring, it recovers it automatically. func (ks keystore) existsInDb(addr sdk.Address, name string) (bool, error) { _, errAddr := ks.db.Get(addrHexKeyAsString(addr)) - if errAddr != nil && errAddr != keyring.ErrKeyNotFound { - return false, errAddr // received unexpected error - returns error + if errAddr != nil && !errors.Is(errAddr, keyring.ErrKeyNotFound) { + return false, errAddr } _, errInfo := ks.db.Get(infoKey(name)) if errInfo == nil { return true, nil // uid lookup succeeds - info exists - } else if errInfo != keyring.ErrKeyNotFound { + } else if !errors.Is(errInfo, keyring.ErrKeyNotFound) { return false, errInfo // received unexpected error - returns } From c8a5ee9c43beff25e6c5e7d67e408a48d5cb5e91 Mon Sep 17 00:00:00 2001 From: swelf Date: Tue, 11 Jan 2022 02:12:55 +0300 Subject: [PATCH 4/4] updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2244de1fadf7..c5e53d305964 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -161,6 +161,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* [\#10844](https://github.com/cosmos/cosmos-sdk/pull/10844) Automatic recovering non-consistent keyring storage during public key import * [\#10414](https://github.com/cosmos/cosmos-sdk/pull/10414) Use `sdk.GetConfig().GetFullBIP44Path()` instead `sdk.FullFundraiserPath` to generate key * (rosetta) [\#10340](https://github.com/cosmos/cosmos-sdk/pull/10340) Use `GenesisChunked(ctx)` instead `Genesis(ctx)` to get genesis block height * [#10180](https://github.com/cosmos/cosmos-sdk/issues/10180) Documentation: make references to Cosmos SDK consistent