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

Public key format mismatch between gnoland secrets init and gnoland genesis validator add #2331

Closed
mazzy89 opened this issue Jun 11, 2024 · 7 comments

Comments

@mazzy89
Copy link
Contributor

mazzy89 commented Jun 11, 2024

Description:

The gnoland genesis validator add command requires a public key input in the Bech32 format. However, the current implementation of the gnoland secrets init command generates a public key that is not in the Bech32 format. This discrepancy causes an incompatibility, preventing the successful addition of validators using the gnoland genesis validator add command.

Steps to Reproduce:

  1. Initialize secrets using the command:
    gnoland secrets init
    
  2. Attempt to add a validator using the generated public key:
    gnoland genesis validator add -pub-key [pub_key]
    

Expected Result:
The public key generated by gnoland secrets init should be in Bech32 format, allowing it to be directly used with the gnoland genesis validator add command without any format conversion.

Actual Result:
The public key generated is not in Bech32 format, leading to an error when attempting to use it with the gnoland genesis validator add command.

Suggested Fix:
Update the gnoland secrets init command to generate the public key in Bech32 format or provide a utility to convert the existing format to Bech32.

@mazzy89 mazzy89 changed the title Public Key format mismatch between gnoland secrets init and gnoland genesis validator add` Public key format mismatch between gnoland secrets init and gnoland genesis validator add` Jun 11, 2024
@mazzy89 mazzy89 changed the title Public key format mismatch between gnoland secrets init and gnoland genesis validator add` Public key format mismatch between gnoland secrets init and gnoland genesis validator add Jun 11, 2024
@mazzy89
Copy link
Contributor Author

mazzy89 commented Jun 11, 2024

I spoke already about this with @moul who agreed on the bad UX and agreed on opening the issue. I'm fine to wire a fix.

@zivkovicmilos
Copy link
Member

zivkovicmilos commented Jun 12, 2024

This is one of those "it's not a bug it's a feature" moments in Gno

Ed25519 keys are saved as base64 encoded Amino on disk, ex:
VRFGraumxyIOS6Rij5HHqhN3zOXp6s4K7WJKZy+bDcA==

This is legacy logic that we haven't really touched (there was no need) -- it's fine to keep the key not human readable. Our secrets get commands know how to display this of course in a format you can use for other commands.

gnoland genesis validator add , however, is custom made to take in the bech32 representation of the key, because the crypto lib we use (our own) can easily parse pub keys from this format (that, or either raw bytes).

The flow currently is:
generate key -> fetch the key using secrets get -> use that value to add the validator

I think we could've easily gone the route of just taking in the public key as base64 amino encoded, and decoding it in the validator add command, but at the time this was the quickest route; we figured we'd iron it out later if needed

What do you think @r3v4s, @albttx? Should we standardize this in some way?

@mazzy89
Copy link
Contributor Author

mazzy89 commented Jun 12, 2024

Thanks @zivkovicmilos for providing all the context.

At the moment, secrets get provides Address and Key information in plain text. I propose either outputting them as json by adding a --json flag or saving the key in Bech32 format. This would allow us to easily parse the contents of the secrets JSON with tools such as jq.

@zivkovicmilos
Copy link
Member

@mazzy89
We have a tracker issue for the json output #2301

@r3v4s
Copy link
Contributor

r3v4s commented Jun 12, 2024

@zivkovicmilos
For better UI/UX, yes I think we should standardize it somehow.
Why don't we just add another flag like #2249(#2301) does?

  • --decode or --readable? Looking for some better string 🤣🤣

@albttx
Copy link
Member

albttx commented Jun 13, 2024

I agree with @zivkovicmilos on this, the key must not be stored in bech32 format because the key could be used somewhere else.

See it like the wallet private key, it's for Gno, but you can use the same private key anywhere else.
The Validator key could be use for another chain, ( It's a bad practice of course ) but it's on the validators to decide.

Bonus, if the key leak, it's harder to determine which chains the key belongs.

But i agree, i already discuss with milos on it, the CLI must be improved for gnoland secrets get ... to show simpler results.

@zivkovicmilos
Copy link
Member

Closed by #2393

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

Successfully merging a pull request may close this issue.

5 participants