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

Verification method block should be a first citizen like public keys #283

Closed
awoie opened this issue May 13, 2020 · 8 comments
Closed

Verification method block should be a first citizen like public keys #283

awoie opened this issue May 13, 2020 · 8 comments
Assignees
Labels
PR exists There is an open PR to address this issue

Comments

@awoie
Copy link
Contributor

awoie commented May 13, 2020

Public keys in the public key section can be referenced in other sections of the DID Doc, e.g., authentication. That is great because it makes the DID Doc smaller in case the same public key is used for multiple purposes.

Verification methods should have the same option.

As per https://www.w3.org/TR/did-core/#authentication:

Each verification method MAY be embedded or referenced.

ATM, referencing verification methods other than public keys is not possible. IMO, it would make more sense to introduce verificationMethod as a new first citizen block. That block would list the verification methods, so that they can be referenced in other sections of the DID Doc, e.g., authentication, in the same way as public keys.

Another approach would be to just rename the publicKey section in verificationMethod which might be even the better approach.

An example of a verification method that would benefit from this approach is ethereumAddress.

@awoie
Copy link
Contributor Author

awoie commented May 13, 2020

@OR13 @dlongley In case there is already a way to achieve that behaviour, please provide an example and we can close that issue. It might be even worth to provide an example in the specification. I could probably also live with having an ethereumAddress array similar to publicKey and just referring to those elements from the authentication section.

@OR13
Copy link
Contributor

OR13 commented May 13, 2020

This is essentially a request to rename publicKey verificationMethod... which IMO is an excellent idea for a number of reasons...

In an ideal world, we don't reuse keys for things, so there is no size benefit to linking, since each key is declared only in the block for which it is used....

In the real world, its extremely common to declare a key once in publicKey section, and then reference it from all the other locations...

For DID Methods that don't have a default public key... like did:ethr this creates an awkward situation where we are listing ethereumAddress under publicKey.... when it's clearly not one.

IMO, this is what a did:ethr which has not been updated should look like:

{
    "@context": [
        "https://www.w3.org/ns/did/v1",
        "https://identity.foundation/blockchain-identity/v1"
    ],
    "id": "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf",
    "verificationMethod": [
      {
        "id": "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf#0",
        "type": "EcdsaSecp256k1RecoveryMethod2020",
        "controller": "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf",
        "ethereumAddress": "0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf"
      }
    ],
    "authentication": [
      "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf#0"
    ],
    "assertionMethod": [
      "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf#0"
    ],
    "capabilityDelegation": [
      "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf#0"
    ],
    "capabilityInvocation": [
      "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf#0"
    ]
}

@OR13
Copy link
Contributor

OR13 commented May 13, 2020

PS, its totally valid today... https://github.com/w3c/did-core-registries/blob/master/contexts/did-v1.jsonld#L140

all we need is to define https://identity.foundation/blockchain-identity/v1.

@awoie
Copy link
Contributor Author

awoie commented May 13, 2020

@OR13 thanks, sounds pretty good!

@kdenhartog
Copy link
Member

@awoie with PR #284 and the inclusion in this registry. I think this issue is ready to be closed. Are you satisfied with the outcome or is there more actions that need to be take non this?

@kdenhartog kdenhartog added the pending close Issue will be closed shortly if no objections label May 27, 2020
@rhiaro
Copy link
Member

rhiaro commented May 27, 2020

@kdenhartog PR #284 expands the definition in terminology, but doesn't add verificationMethod to the core properties section of the spec. I can take an action to do this (sounds like that's what is needed?). Removing publicKey in favour of verificationMethod may then be a separate followup..

@rhiaro rhiaro self-assigned this May 27, 2020
@awoie
Copy link
Contributor Author

awoie commented May 27, 2020

@rhiaro I was about to add the same note. Thanks for pointing that out. If there is a PR that fixes that, I'm fine with closing this issue.

@rhiaro rhiaro added ready for PR Issue is ready for a PR and removed pending close Issue will be closed shortly if no objections labels May 27, 2020
@kdenhartog
Copy link
Member

Thanks for the clarity! Makes sense to me now that you've pointed it out.

rhiaro added a commit that referenced this issue May 31, 2020
Fixes #283. Also removes undefined terms that are verification relationships - which fixes #272 - replacing them with links to the DID Spec Registries.
@rhiaro rhiaro added PR exists There is an open PR to address this issue and removed ready for PR Issue is ready for a PR labels May 31, 2020
@msporny msporny closed this as completed in 52a4209 Jun 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR exists There is an open PR to address this issue
Projects
None yet
Development

No branches or pull requests

4 participants