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.

Sign up for GitHub

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

client/keys: print out pubkeys in JSON format #808

Closed
wants to merge 1 commit into from
Closed

client/keys: print out pubkeys in JSON format #808

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions client/keys/show.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,29 @@ package keys

import (
"encoding/json"
"fmt"
"net/http"

"github.com/gorilla/mux"
"github.com/pkg/errors"
"github.com/spf13/cobra"
"github.com/spf13/viper"
keys "github.com/tendermint/go-crypto/keys"
)

"github.com/spf13/cobra"
const (
flagExportPubKey = "export-pubkey"
)

var showKeysCmd = &cobra.Command{
Use: "show <name>",
Short: "Show key info for the given name",
Long: `Return public details of one local key.`,
RunE: runShowCmd,
Args: cobra.ExactArgs(1),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cwgoes nope, this just does it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it.

}

func init() {
showKeysCmd.Flags().Bool(flagExportPubKey, false, "Export public key.")
}

func getKey(name string) (keys.Info, error) {
Expand All @@ -30,16 +39,20 @@ func getKey(name string) (keys.Info, error) {
// CMD

func runShowCmd(cmd *cobra.Command, args []string) error {
if len(args) != 1 || len(args[0]) == 0 {
return errors.New("You must provide a name for the key")
info, err := getKey(args[0])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should still check the length of args and print a helpful error if it isn't equal to 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See github.com/spf13/cobra.ExactArgs()

if err != nil {
return err
}
name := args[0]

info, err := getKey(name)
if err == nil {
printInfo(info)
if viper.GetBool(flagExportPubKey) {
out, err := info.PubKey.MarshalJSON()
if err != nil {
return err
}
fmt.Println(string(out))
Copy link
Contributor

@rigelrozanski rigelrozanski Apr 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the idea of moving out of the current process train of printing everything through the printInfo function (right below this comment)...
Also don't like introducing another key. Already you can get the pubkey in json by using gaiacli keys show <yourkey> --output json - If you want to also see the pubkey with "text" output (aka --output text aka the default) then we should add that right about here:

cosmos-sdk/client/keys/utils.go

Line 58 in 18ac0f9

fmt.Printf("%s%s%s\n", info.Name, sep, addr)

maybe with a table header so it would look like:

name          address                pubkey 
alessio        197609375...       0x1241412....

just need to make sure we'd get it right for keys list as well as keys show but meh - I'm okay with just using --output json myself

Copy link
Contributor Author

@alessio alessio Apr 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi!

Apologies for the late reply.

Already you can get the pubkey in json by using gaiacli keys show --output json

True, but the following prints the private key out too:

alessio@bizet:~/.../github.com/tendermint/clearchain$ clearchainctl keys show --output json ch_admin_1
{"name":"ch_admin_1","pubkey":[1,"CEB016A0495DEE269D655F12DE14941963214B3BEE034C3DDFDBCECB2627D3C7"],"privkey.armor":"-----BEGIN TENDERMINT PRIVATE KEY-----\nkdf: bcrypt\nsalt: CB69DDE1B83E179E7DED2C9A7FDE1BA8\n\n3Q2YAte6PArOEnMt9YR5CNmAyk7H6cFvZLuibwxoIlf+B7whJ8Xj4TZmBDo/JfEE\nlcdUZTafvGvHp2vxZ7PVg0ria+sJIPqQlgaNI1iGXqZ/kRYHSuRPIKwniRwrk6LE\nXFE5c8DN3Ceu\n=v8dc\n-----END TENDERMINT PRIVATE KEY-----"}

Furthermore, the public key JSON format seems a bit fuzzy: [1,"CEB016A0495DEE269D655F12DE14941963214B3BEE034C3DDFDBCECB2627D3C7"]

If you want to also see the pubkey with "text" output (aka --output text aka the default) then we should add that right about here: [snip]

That's fair, I'd be happy with that too! I just need to allow users to exchange public keys in a portable format, be it a string or a JSON thing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it sounds like we just want to improve the JSON output format, which I could totally get behind - maybe don't print the private key by default too

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay yeah the validator json key is not a HEX string which stinks

return nil
}
return err
printInfo(info)
return nil
}

// REST
Expand Down