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

Consensus key seems different size when bech32-encoded #228

Closed
greg-szabo opened this issue Nov 19, 2020 · 6 comments
Closed

Consensus key seems different size when bech32-encoded #228

greg-szabo opened this issue Nov 19, 2020 · 6 comments

Comments

@greg-szabo
Copy link

greg-szabo commented Nov 19, 2020

It seems that the consensus key generated on the YubiHSM is different in size than the one generated by a simple gaiad command:
gaiad version: cosmoshub-test-stargate
tmkms version: 0.10.0-alpha7

Compare size:

cosmosvalconspub1zcjduepq4f2zzkjr20v4gt0gy9nmdznxsvmgluyn3vnjmgqq58q9t47uyffqndgy0p
cosmosvalconspub1wzmcs4kpsetyk7fn7czvdljgawa93r6mnzvcwt9a366rep2gcs0q62ce7v
gaiad tendermint show-validator
cosmosvalconspub1zcjduepq4f2zzkjr20v4gt0gy9nmdznxsvmgluyn3vnjmgqq58q9t47uyffqndgy0p
tmkms yubihsm keys list
- 0x0001: [cons] cosmosvalconspub1wzmcs4kpsetyk7fn7czvdljgawa93r6mnzvcwt9a366rep2gcs0q62ce7v
   label: "cosmosvalconspub:2019-03-12T21:03:43Z"
- 0x0002: [cons] FE4AB0236C7B8E3C3687D7703CF58FD16128ABF28A17CF89F585C7D724D1B0E7
   label: "cosmoshub-test-stargate-b"

The first key was generated in the amino times, but the second key showed a same-size consensus key at generation.

Additionally, you can see that if a random label is used, then the key is not printed with the correct bech32 encoding.

@tony-iqlusion
Copy link
Member

@greg-szabo it's using the public key formatting logic from tendermint-rs, so if something is wrong, it's a tendermint-rs bug:

https://docs.rs/tendermint/0.16.0/tendermint/public_key/enum.PublicKey.html#method.to_bech32

It seems like it might be something to do with Amino vs Protobuf framing? I still don't understand the story around public keys and in particular Bech32 public keys going forward:

cosmos/cosmos-sdk#7447

Additionally, you can see that if a random label is used, then the key is not printed with the correct bech32 encoding.

For it to print the key in Bech32, it needs to be able to determine what Bech32 prefix needs to be used. It presently does this in a somewhat janky way:

https://github.com/iqlusioninc/tmkms/blob/develop/src/commands/yubihsm/keys/list.rs#L62-L72

If the key isn't configured for use in any particular chain, it can't determine the Bech32 prefix so it prints it as raw hex.

@greg-szabo
Copy link
Author

Cool! As for the first part of the issue, I'll try to go through the SDK code too and see if I can find the discrepancy in Tendermint-rs.

As for the second part, it would be easier to test for me if I had a way to delete consensus keys so I can just create/delete multiple keys. I couldn't find a tmkms keys delete command, though, do you have any pointers to this?

@tony-iqlusion
Copy link
Member

There presently isn't support in TMKMS for deleting keys. You'd need to use yubihsm-shell to do that for now.

@greg-szabo
Copy link
Author

Thanks for the pointer, yubihsm-shell worked. Here's a complete example of the labeling issue. Note that at generation, the prefix is correctly set and used, but further listing doesn't encode the key.

> tmkms yubihsm keys generate 2 -p cosmosvalconspub -b stargate-b-validator-key.enc
   Generated consensus (ed25519) key 0x0002: cosmosvalconspub10mfxrgty437tnf45xhgxujfq4zudppqs83pkvh4u50x20pe8qjuqsursa8
       Wrote backup of key 2 (encrypted under wrap key 1) to stargate-b-validator-key.enc
> tmkms yubihsm keys list
Listing keys in YubiHSM #0007550403:
- 0x0001: [cons] cosmosvalconspub1wzmcs4kpsetyk7fn7czvdljgawa93r6mnzvcwt9a366rep2gcs0q62ce7v
   label: "cosmosvalconspub:2019-03-12T21:03:43Z"
- 0x0002: [cons] 7ED261A164AC7CB9A6B435D06E4920A8B8D084103C43665EBCA3CCA7872704B8
   label: "cosmosvalconspub:2020-11-19T23:09:24Z

I'll look into the bech encoding in Tendermint next.

@tony-iqlusion
Copy link
Member

The key needs to be in the keyring for one and only one chain, or else it will print hex.

It should probably be changed to print the Bech32 for all registered chains.

@tony-iqlusion
Copy link
Member

I believe this was fixed in informalsystems/tendermint-rs#690

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants