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

Add key generation subcommands #259

Merged
merged 9 commits into from
Feb 16, 2022
Merged

Add key generation subcommands #259

merged 9 commits into from
Feb 16, 2022

Conversation

sbihel
Copy link
Member

@sbihel sbihel commented Feb 9, 2022

Also move to clap v3 (structopt has been integrated in clap).


Here's my proposal for the subcommands.

key
├─ generate
   ├─ secp256k1
   ├─ ...
├─ did
   ├─ web
   ├─ pkh:tz
   ├─ ...

Also move to clap v3.
@sbihel sbihel requested a review from clehner February 9, 2022 10:08
cli/src/main.rs Outdated Show resolved Hide resolved
cli/src/main.rs Outdated Show resolved Hide resolved
@clehner
Copy link
Contributor

clehner commented Feb 10, 2022

key generate command looks good to me.

Suggesting to rename the third key type to Secp256r1: #259 (comment) #259 (comment)

What would the did part be?

├─ did
   ├─ web
   ├─ pkh:tz
   ├─ ...

@sbihel
Copy link
Member Author

sbihel commented Feb 11, 2022

Suggesting to rename the third key type to Secp256r1

Actually, do you think it would be better to name them k256 and p256?

What would the did part be?

It's to replace the key-to-did command, but it's obviously not the greatest name 😅

@clehner
Copy link
Contributor

clehner commented Feb 11, 2022

Actually, do you think it would be better to name them k256 and p256?

"Secp256k1" would be consistent with the usage here:

Usage of the term Secp256r1 seems to be a little less consistent, e.g.:

But there is mention of Secp256r1 in the VC Data model V1 context:

The JOSE table of elliptic curves includes secp256k1 and P-256:
https://www.iana.org/assignments/jose/jose.xhtml#web-key-elliptic-curve

We could also have additional (hidden) aliases like for p256, P-256, k256, etc.

It's to replace the key-to-did command, but it's obviously not the greatest name

OK. Not bad I think. If it was key to-did, that could keep the verb/imperative style. Or it could be moved into a did subcommand (e.g. did from-key, did generate, did create) In #255 I'm adding more DID operations also.

@sbihel sbihel marked this pull request as ready for review February 15, 2022 09:44
@sbihel
Copy link
Member Author

sbihel commented Feb 15, 2022

I didn't find a way to override the -h help flag on a per command basis. Is the did-auth command part of the latest publish version of DIDKit CLI?

@clehner
Copy link
Contributor

clehner commented Feb 15, 2022

I didn't find a way to override the -h help flag on a per command basis.

I see, did-auth's -h previously overrided the default short help command, but on the newer clap version used here, this results in an error (Short option names must be unique for each argument, but '-h' is in use by both 'holder' and 'help'). I don't see a way to override it now either. Probably it's good to just let -h be for help. Although in the newer clap it looks like -h is the same as --help while previously -h showed a shorter help...

Is the did-auth command part of the latest publish version of DIDKit CLI?

Yes (didkit-cli crate v0.1.1, and on ghcr.io).

cli/src/main.rs Outdated Show resolved Hide resolved
@sbihel sbihel merged commit 3730b54 into main Feb 16, 2022
@sbihel sbihel deleted the feat/generate-subcommands branch February 16, 2022 15:27
@clehner clehner mentioned this pull request Mar 1, 2022
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

Successfully merging this pull request may close these issues.

2 participants