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

add flag to "ipfs key gen" to output keys in b36/CIDv1, also make it … #7531

Merged
merged 7 commits into from
Jul 13, 2020
Merged

add flag to "ipfs key gen" to output keys in b36/CIDv1, also make it … #7531

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e7af20a
add flag to "ipfs key gen" to output keys in b36/CIDv1, also make it …
petar Jul 9, 2020
c423bfe
add cid output format flag to "ipfs key list"
petar Jul 10, 2020
ef38fc5
change flag format
petar Jul 10, 2020
1e029e8
update sharness
petar Jul 10, 2020
e61a263
more sharness
petar Jul 10, 2020
15a90d1
change default to b58mh
petar Jul 10, 2020
b28a7b4
add sharness tests for key gen format
petar Jul 13, 2020
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
22 changes: 18 additions & 4 deletions core/commands/keystore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import (
"io"
"text/tabwriter"

cmdenv "github.com/ipfs/go-ipfs/core/commands/cmdenv"

cmds "github.com/ipfs/go-ipfs-cmds"
cmdenv "github.com/ipfs/go-ipfs/core/commands/cmdenv"
options "github.com/ipfs/interface-go-ipfs-core/options"
peer "github.com/libp2p/go-libp2p-core/peer"
b36 "github.com/multiformats/go-base36"
)

var KeyCmd = &cmds.Command{
Expand Down Expand Up @@ -56,6 +57,7 @@ type KeyRenameOutput struct {
const (
keyStoreTypeOptionName = "type"
keyStoreSizeOptionName = "size"
cidOptionName = "cid"
)

var keyGenCmd = &cmds.Command{
Expand All @@ -65,6 +67,7 @@ var keyGenCmd = &cmds.Command{
Options: []cmds.Option{
cmds.StringOption(keyStoreTypeOptionName, "t", "type of the key to create: rsa, ed25519").WithDefault("rsa"),
cmds.IntOption(keyStoreSizeOptionName, "s", "size of the key to generate"),
cmds.BoolOption(cidOptionName, "c", "return a base-36 CIDv1 encoding of the key").WithDefault(true),
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that we're planning on using b36 as the default the documentation and style of this flag doesn't really make much sense. i.e. what does it mean if "return a base-36 CIDv1 encoding of the key" is false, how is it encoded?

We could do the opposite sort of flag (e.g. "--b58Mh"), but that leaves us slightly exposed in the event we ever want to change the default going forward (hopefully not for a long time 🤞). WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think, there should be explicit flags for every supported variant, because it sounds like the default may flip-flop. So how about a flag with mutually-exclusive variants: --format b58mh | b36cid?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that seems reasonable. If someone wants anything else they can pass the b36cid into the ipfs cid function. I'm not sure whether the cid option should be called b36cid or just cid, with a comment saying it's currently b36, both seem fine so your call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like b58mh and b36cid, because they have the same schema.

},
Arguments: []cmds.Argument{
cmds.StringArg("name", true, false, "name of key to create"),
Expand Down Expand Up @@ -100,7 +103,7 @@ var keyGenCmd = &cmds.Command{

return cmds.EmitOnce(res, &KeyOutput{
Name: name,
Id: key.ID().Pretty(),
Id: formatID(key.ID(), req.Options[cidOptionName].(bool)),
})
},
Encoders: cmds.EncoderMap{
Expand All @@ -112,12 +115,20 @@ var keyGenCmd = &cmds.Command{
Type: KeyOutput{},
}

func formatID(id peer.ID, useCID bool) string {
if useCID {
return b36.EncodeToStringLc(peer.ToCid(id).Bytes())
}
return id.Pretty()
}

var keyListCmd = &cmds.Command{
Helptext: cmds.HelpText{
Tagline: "List all local keypairs",
},
Options: []cmds.Option{
cmds.BoolOption("l", "Show extra information about keys."),
cmds.BoolOption(cidOptionName, "c", "return a base-36 CIDv1 encoding of the keys").WithDefault(true),
},
Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error {
api, err := cmdenv.GetApi(env, req)
Expand All @@ -133,7 +144,10 @@ var keyListCmd = &cmds.Command{
list := make([]KeyOutput, 0, len(keys))

for _, key := range keys {
list = append(list, KeyOutput{Name: key.Name(), Id: key.ID().Pretty()})
list = append(list, KeyOutput{
Name: key.Name(),
Id: formatID(key.ID(), req.Options[cidOptionName].(bool)),
})
}

return cmds.EmitOnce(res, &KeyOutputList{list})
Expand Down
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ require (
github.com/miekg/dns v1.1.29 // indirect
github.com/mitchellh/go-homedir v1.1.0
github.com/mr-tron/base58 v1.1.3
github.com/multiformats/go-base36 v0.1.0
github.com/multiformats/go-multiaddr v0.2.2
github.com/multiformats/go-multiaddr-dns v0.2.0
github.com/multiformats/go-multiaddr-net v0.1.5
Expand Down