R4R: Refactor Bech32 Prefixes and Nomenclature of Validator Pubkey and Operator #2251
Conversation
alexanderbez
requested review from
cwgoes,
ebuchman,
rigelrozanski and
zramsay
as code owners
alexanderbez
changed the title
alexanderbez
added
T: API Breaking
Codecov Report
@@ Coverage Diff @@
## develop #2251 +/- ##
========================================
Coverage 63.73% 63.73%
========================================
Files 140 140
Lines 8642 8642
========================================
Hits 5508 5508
Misses 2755 2755
Partials 379 379 |
|
This is going to break all existing pubkeys / have impacts on people who've setup their accounts on their ledger right? Can we make a script to update your pubkey and address from the different bech32 formats as well. |
|
@ValarDragon I'm not totally sure what the Ledger stores. It might be the case this does not break the Ledger support as the underlying bytes of the address and key pairs are still the same. But let me test this. |
|
Confirmed |
|
@cosmos/cosmos-ui FYI breaking changes |
Any changes to bech32 encoding will not break any underlying private/public keys (although it might break databases which store a bech32ified string and expect a particular bech32 prefix). |
client/lcd/lcd_test.go
Outdated
| foundVal = true | ||
| } | ||
| require.True(t, foundVal, "pkBech %v, operator %v", pkBech, validators[0].Operator) | ||
| require.True(t, foundVal, "pkBech %v, operator %v", pkBech, validators[0].ConsPubKey) |
There was a problem hiding this comment.
Why does this change the printf? The error is wrong now
There was a problem hiding this comment.
Bad copy-pasta job -- good catch.
docs/clients/lcd-rest-api.yaml
Outdated
| PubKey: | ||
| type: string | ||
| description: bech32 encoded public key | ||
| example: cosmospub:zgnkwr7eyyv643dllwfpdwensmgdtz89yu73zq | ||
| ValidatorPubKey: | ||
| type: string | ||
| description: bech32 encoded public key | ||
| example: cosmosvalpub:zgnkwr7eyyv643dllwfpdwensmgdtz89yu73zq | ||
| example: cosmosvaloperpub:zgnkwr7eyyv643dllwfpdwensmgdtz89yu73zq |
There was a problem hiding this comment.
Wait, is this not the consensus pubkey? If not, why does Voyager need the operator's public key?
|
@cwgoes addressed your comments + merged in latest |
closes: #2221
Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (
docs/)Added entries in
PENDING.mdwith issue #rereviewed
Files changedin the github PR explorerFor Admin Use: