w3c / did-core

W3C Decentralized Identifier Specification v1.0
https://www.w3.org/TR/did-core/
Other
398 stars 94 forks source link

Update DID Document examples #748

Closed OR13 closed 3 years ago

OR13 commented 3 years ago

Addresses: #734


Preview | Diff

peacekeeper commented 3 years ago

I'm not sure if I like splitting up the verification methods example into two examples - one that highlights only JsonWebKey2020, and one that actually shows different verification methods.

I don't think the DID Core specification should give the impression that one verification method is more "special" than others.

OR13 commented 3 years ago

@peacekeeper I agree, but it was worse before... it highlighted only publicKeyMultibase.

both publicKeyMultibase and publicKeyJwk are defined by DID Core....

The other examples are not.

I don't see a good way around this issue, given we were unable to agree to a single verification material representation, the best we can do is provide examples of the 2 presentations we did agree to include, and then a 3rd example which shows how bad this can get.

even ordering the examples starting with multibase could be argued as bias... there was clearly no consensus on this topic.

I might argue that publicKeyJwk should be listed first, since it relies on a real standard.

But I am willing to let the informative examples hint that a JSON only verification method format might not be the best foot to lead with for an abstract data model based spec.

peacekeeper commented 3 years ago

it highlighted only publicKeyMultibase

I don't understand. The current example https://www.w3.org/TR/did-core/#example-31-did-document-with-many-different-verification-methods shows 1x publicKeyMultibase, 1x publicKeyBase58, and 7x publicKeyJwk.

So publicKeyJwk is already given much more attention than the others. This PR here seems to highlight publicKeyJwk even more by giving it its own separate example block. Why?

OR13 commented 3 years ago

@peacekeeper the first example uses publicKeyMultibase only:

https://www.w3.org/TR/did-core/#example-30-did-document-with-1-verification-method-type

{
    "@context": [
      "https://www.w3.org/ns/did/v1",
      "https://w3id.org/security/suites/ed25519-2020/v1"
    ],
    "id": "did:example:123",
    "authentication": [
      {
        "id": "did:example:123#z6MkecaLyHuYWkayBDLw5ihndj3T1m6zKTGqau3A51G7RBf3",
        "type": "Ed25519VerificationKey2020", // external (property value)
        "controller": "did:example:123",
        "publicKeyMultibase": "zAKJP3f7BD6W4iWEQ9jwndVTCBq8ua2Utt8EEjJ6Vxsf"
      }
    ],
    "capabilityInvocation": [
      {
        "id": "did:example:123#z6MkhdmzFu659ZJ4XKj31vtEDmjvsi5yDZG5L7Caz63oP39k",
        "type": "Ed25519VerificationKey2020", // external (property value)
        "controller": "did:example:123",
        "publicKeyMultibase": "z4BWwfeqdp1obQptLLMvPNgBw48p7og1ie6Hf9p5nTpNN"
      }
    ],
    "capabilityDelegation": [
      {
        "id": "did:example:123#z6Mkw94ByR26zMSkNdCUi6FNRsWnc2DFEeDXyBGJ5KTzSWyi",
        "type": "Ed25519VerificationKey2020", // external (property value)
        "controller": "did:example:123",
        "publicKeyMultibase": "zHgo9PAmfeoxHG8Mn2XHXamxnnSwPpkyBHAMNF3VyXJCL"
      }
    ],
    "assertionMethod": [
      {
        "id": "did:example:123#z6MkiukuAuQAE8ozxvmahnQGzApvtW7KT5XXKfojjwbdEomY",
        "type": "Ed25519VerificationKey2020", // external (property value)
        "controller": "did:example:123",
        "publicKeyMultibase": "z5TVraf9itbKXrRvt2DSS95Gw4vqU3CHAdetoufdcKazA"
      }
    ]
}

I interpret the first example as being the most recommended way to handle things.

peacekeeper commented 3 years ago

@OR13 but what's the motivation for introducing a new example that focuses only on the JsonWebKey2020 verification method, when there is already an existing example that shows several verification methods including JsonWebKey2020?

Is the idea that you want to highlight how a single verification method can support many different key types? If so, then I think that's useful, but maybe we could swap the two examples, so the one with several different verification methods comes first, and then the one that focuses only on JsonWebKey2020? Would that work for you?

iherman commented 3 years ago

The issue was discussed in a meeting on 2021-05-25

View the transcript #### 3.5. Examples might be out of date _See github issue [#734](https://github.com/w3c/did-core/issues/734)._ _See github pull request [#748](https://github.com/w3c/did-core/pull/748)._ **Drummond Reed:** I submitted PRs on that and the next issue; they just need review **Orie Steele:** I removed the Ethereum address example … there is still a potential issue with having a public key multibase example first … so I hoped that providing several examples would solve the problem **Brent Zundel:** a link to the PR is in IRC so please review
OR13 commented 3 years ago

@peacekeeper

I think the best we can do is:

  1. example using publicKeyMultibase
  2. example using publicKeyJwk
  3. example using many types.

Thats what I did in this PR.

I could also live with:

  1. example using many types.

I think its pretty obvious that:

  1. example using publicKeyMultibase
  2. example using many types.

is using non normative examples to bias against publicKeyJwk which is just as well defined in did core as publicKeyMultibase.

OR13 commented 3 years ago

@peacekeeper another option:

  1. publicKeyMultibase + publicKeyJwk
  2. example using many types

The problem with this is that it sorta sucks to consume keys that are varying types from a did document... part of the reason the "many" option is last... its better to have them all publicKeyMultibase or publicKeyJwk imo... even those a mix is legal according to the spec.

OR13 commented 3 years ago

Another note, publicKeyJwk and publicKeyMultibase are actually key types... they just represent the type information differently... whereas publicKeyJwk and publicKeyBase58 are not both types.... you don't know the crv or kty values from publicKeyBase58 but you do from publicKeyMultibase... imo this is a major reason why publicKeyMultibase is superior to publicKeyBase58

msporny commented 3 years ago

Editorial, multiple reviews, changes requested and made, no objections, merging.

msporny commented 3 years ago

I can confirm that my concerns have been address with this PR for issue #734.