gnolang / gno

Gno: An interpreted, stack-based Go virtual machine to build succinct and composable apps + Gno.land: a blockchain for timeless code and fair open-source
https://gno.land/
Other
839 stars 342 forks source link

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

Open mazzy89 opened 2 weeks ago

mazzy89 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

@mazzy89 We have a tracker issue for the json output https://github.com/gnolang/gno/issues/2301

r3v4s commented 2 weeks ago

@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?

albttx commented 1 week ago

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.