Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: non consistent keyring #10844

Merged
merged 11 commits into from Feb 15, 2022
28 changes: 20 additions & 8 deletions crypto/keyring/keyring.go
Expand Up @@ -808,20 +808,32 @@ 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) {

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
_, errAddr := ks.db.Get(addrHexKeyAsString(addr))
if errAddr != nil && errAddr != keyring.ErrKeyNotFound {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we should use the errors.Is call here.

return false, errAddr // received unexpected error - returns error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return false, errAddr // received unexpected error - returns error
return false, errAddr

}

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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto: errors.Is

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
Expand Down
37 changes: 37 additions & 0 deletions crypto/keyring/keyring_test.go
Expand Up @@ -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)
Expand Down