Conversation
…berbono3/keyring-migration
| cmd.PrintErrln("Migration complete.") | ||
|
|
||
| return err | ||
| cmd.Println("Keys migration has been successfully executed") |
There was a problem hiding this comment.
This one you can leave. But you should remove most of other printlns.
| } | ||
|
|
||
| return priv, nil | ||
| // case legacyLedgerInfo, legacyOfflineInfo, LegacyMultiInfo: |
There was a problem hiding this comment.
Error is better than panic.
crypto/keyring/legacy_info.go
Outdated
| return priv, nil | ||
| // case legacyLedgerInfo, legacyOfflineInfo, LegacyMultiInfo: | ||
| default: | ||
| return nil, errors.New("only works on local private keys") |
There was a problem hiding this comment.
In the error message, please add the legacyLocalInfo, eg:
fmt.Errorf("only works on local private keys, provided %s", linfo)
There was a problem hiding this comment.
See Comment what other printlns do you mean, @robert-zaremba ?
|
See Comment What other printlns do you mean, @robert-zaremba ? |
| require.Equal(t, newKey.GetType(), oldKey.GetType()) | ||
| } | ||
| func requireEqualRenamedKey(t *testing.T, key *Record, mnemonic *Record, nameMatch bool) { | ||
| if nameMatch { |
There was a problem hiding this comment.
I added 'nameMatch' argument to requireEqualRenamedKey as in some cases i dont need name check ( e.g. at L1319). If it is set to true, it checks if key.Name and mnemonic.Name match as you can see below. I hope it sounds good to you, @robert-zaremba
cyberbono3
force-pushed
the
cyberbono3/keyring-migration
branch
from
a9aa77c to
1b7b8af
Compare
…berbono3/keyring-migration
|
Thanks a lot @cyberbono3 for your work on this PR! |
|
I hope this work was completed putting legacy clients first and ensuring that Cosmos Hub users would not stumble on any disruption whatsoever. Else, you will hear people shouting very, very loud and from all directions with the next release. As a word of advice, I'd test this thoroughly before releasing it. |
5 participants
Description
The draft PR #9222
Closes: #7108
RecordInfo.gotolegacyInfo.gowithinkeyringpackagemigratecommand that migrates all keys from legacyInfo to proto according to @robert-zaremba migration algorithmMigrateandMigrateAllfunctions inkeyring.gofor single key and all keys migrationAuthor 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...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers 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...
!in the type prefix if API or client breaking change