This repository has been archived by the owner on Oct 29, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 45
Add publicKeyHex as a valid publicKey format #270
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
{ | ||
"@context": { | ||
"@version": 1.1, | ||
"id": "@id", | ||
"type": "@type", | ||
|
||
"dc": "http://purl.org/dc/terms/", | ||
"schema": "http://schema.org/", | ||
"sec": "https://w3id.org/security#", | ||
"didv": "https://w3id.org/did#", | ||
"xsd": "http://www.w3.org/2001/XMLSchema#", | ||
|
||
"EcdsaSecp256k1Signature2019": "sec:EcdsaSecp256k1Signature2019", | ||
"EcdsaSecp256k1VerificationKey2019": "sec:EcdsaSecp256k1VerificationKey2019", | ||
"Ed25519Signature2018": "sec:Ed25519Signature2018", | ||
"Ed25519VerificationKey2018": "sec:Ed25519VerificationKey2018", | ||
"RsaSignature2018": "sec:RsaSignature2018", | ||
"RsaVerificationKey2018": "sec:RsaVerificationKey2018", | ||
"SchnorrSecp256k1Signature2019": "sec:SchnorrSecp256k1Signature2019", | ||
"SchnorrSecp256k1VerificationKey2019": "sec:SchnorrSecp256k1VerificationKey2019", | ||
"ServiceEndpointProxyService": "didv:ServiceEndpointProxyService", | ||
|
||
"allowedAction": "sec:allowedAction", | ||
"assertionMethod": {"@id": "sec:assertionMethod", "@type": "@id", "@container": "@set"}, | ||
"authentication": {"@id": "sec:authenticationMethod", "@type": "@id", "@container": "@set"}, | ||
"capability": {"@id": "sec:capability", "@type": "@id"}, | ||
"capabilityAction": "sec:capabilityAction", | ||
"capabilityChain": {"@id": "sec:capabilityChain", "@type": "@id", "@container": "@list"}, | ||
"capabilityDelegation": {"@id": "sec:capabilityDelegationMethod", "@type": "@id", "@container": "@set"}, | ||
"capabilityInvocation": {"@id": "sec:capabilityInvocationMethod", "@type": "@id", "@container": "@set"}, | ||
"capabilityStatusList": {"@id": "sec:capabilityStatusList", "@type": "@id"}, | ||
"canonicalizationAlgorithm": "sec:canonicalizationAlgorithm", | ||
"caveat": {"@id": "sec:caveat", "@type": "@id", "@container": "@set"}, | ||
"challenge": "sec:challenge", | ||
"controller": {"@id": "sec:controller", "@type": "@id"}, | ||
"created": {"@id": "dc:created", "@type": "xsd:dateTime"}, | ||
"creator": {"@id": "dc:creator", "@type": "@id"}, | ||
"delegator": {"@id": "sec:delegator", "@type": "@id"}, | ||
"domain": "sec:domain", | ||
"expirationDate": {"@id": "sec:expiration", "@type": "xsd:dateTime"}, | ||
"invocationTarget": {"@id": "sec:invocationTarget", "@type": "@id"}, | ||
"invoker": {"@id": "sec:invoker", "@type": "@id"}, | ||
"jws": "sec:jws", | ||
"keyAgreement": {"@id": "sec:keyAgreementMethod", "@type": "@id", "@container": "@set"}, | ||
"nonce": "sec:nonce", | ||
"owner": {"@id": "sec:owner", "@type": "@id"}, | ||
"proof": {"@id": "sec:proof", "@type": "@id", "@container": "@graph"}, | ||
"proofPurpose": {"@id": "sec:proofPurpose", "@type": "@vocab"}, | ||
"proofValue": "sec:proofValue", | ||
"publicKey": {"@id": "sec:publicKey", "@type": "@id", "@container": "@set"}, | ||
"publicKeyBase58": "sec:publicKeyBase58", | ||
"publicKeyPem": "sec:publicKeyPem", | ||
"publicKeyHex": "sec:publicKeyHex", | ||
"revoked": {"@id": "sec:revoked", "@type": "xsd:dateTime"}, | ||
"service": {"@id": "didv:service", "@type": "@id", "@container": "@set"}, | ||
"serviceEndpoint": {"@id": "didv:serviceEndpoint", "@type": "@id"}, | ||
"verificationMethod": {"@id": "sec:verificationMethod", "@type": "@id"} | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We really need to move to publicKeyMultibase ... because that supports base16 (hex), base58, base64url, etc.
Can we do that instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably also switch to Multicodec encoding of public keys... we've been needing to do this for a very long time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed on both counts (multibase and multicodec). Although I wonder if it might cause less confusion to at least have a transition period where we support both a general
publicKeyMultibase
and the specific onespublicKeyBase58
etc, side by side.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add
publicKeyMultibase
but then it wouldn't be consistent to keeppublicKeyBase58
. The problem is removingpublicKeyBase58
would cause retro-compatibility issue because it is already used by did methods (did:sov)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree we could support them all side by side for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we merge this PR as is, and create an issue for
publicKeyMulticodec
that can be discussed and solved a separate PR?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we shouldn't merge. This should be a CCG decision... it has far too much of an impact on DID Method implementers to merge w/o consulting w/ the community first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why we would not add support for the key types listed in the spec:
https://w3c-ccg.github.io/did-spec/#public-keys
I agree, that some are better than others (Personally I'm a fan of publicKeyPem and publicKeyJwk).
We should either remove the key types from the spec or support them. Saying that the community uses a key type, but then having it not be supported seems wrong.
I agree that the CCG should make the decision here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm with @dmitrizagidulin and @OR13 on this.. I think
publicKeyMulticodec
is great. But considering that we just released the Community Group Final Report, we should have a context (either the current v0.11 or a new v0.12, doesn't matter so much I think) that contains all terms used in that released spec, including all the example public key properties (see § 5.3, this was last changed in 8dbfe62).After that we could immediately start working on a new context 0.13 and have the discussion about removing everything except
publicKeyMultibase
/publicKeyMulticodec
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this PR resolves that issue. It need only be approved / merged.