fix: non consistent keyring (#10844)
## Description Normally keyring module creates two record for each public key created in keyringdb. The first one with an address as a key witch contains only name of the second key, wich actually contains a public key  But a couple of times we have faced an issue, when the first record exists, and the second for some reason does not.  In such case you are unable to import public key due to error ```shell $ go run ./cmd/terrad/ keys --keyring-backend kwallet add swelf --pubkey '{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"Ap1W7ww/FaZVpAd487QUVXh7Nmxk4FlREr5IGPuzEnJu"}' Error: public key already exists in keybase ``` in the same time terrad cli do not see any keys in the keyring ```shell $ go run ./cmd/terrad/ keys --keyring-backend kwallet list [] ``` The error occurs when the record with address still exists in the keyring db. I would like to resolve the error. I see at least three different ways to do it. 1) Informing the user about situation and recreate public key ```shell $ go run ./cmd/terrad/ keys --keyring-backend kwallet add swelf --pubkey '{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"Ap1W7ww/FaZVpAd487QUVXh7Nmxk4FlREr5IGPuzEnJu"}' **address "7cc4633deb18c0531b382a50275ad94e05f84580" exists but pubkey itself does not recreating pubkey record** - name: swelf type: offline address: terra10nzxx00trrq9xxec9fgzwkkefczls3vqkpkjl4 pubkey: '{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"Ap1W7ww/FaZVpAd487QUVXh7Nmxk4FlREr5IGPuzEnJu"}' mnemonic: "" ``` with notifying user about an issue 2) Asking the user to confirm procedure of restoring public key 3) Just informing user about an issue and do nothing. I prefer the first way, i do not see a reason when user do not want to fix an issue. --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [x] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
This commit is contained in:
parent
e7066c4271
commit
5099c15f73
@ -167,6 +167,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
|
||||
* (store) [\#11117](https://github.com/cosmos/cosmos-sdk/pull/11117) Fix data race in store trace component
|
||||
* (cli) [\#11065](https://github.com/cosmos/cosmos-sdk/pull/11065) Ensure the `tendermint-validator-set` query command respects the `-o` output flag.
|
||||
* (grpc) [\#10985](https://github.com/cosmos/cosmos-sdk/pull/10992) The `/cosmos/tx/v1beta1/txs/{hash}` endpoint returns a 404 when a tx does not exist.
|
||||
|
||||
@ -810,18 +810,29 @@ func (ks keystore) writeRecord(k *Record) error {
|
||||
|
||||
// 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 && !errors.Is(errAddr, keyring.ErrKeyNotFound) {
|
||||
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 !errors.Is(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
|
||||
|
||||
@ -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)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user