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

Create a seperate top level block for defining proof purposes #294

Closed
tplooker opened this issue May 27, 2020 · 21 comments
Closed

Create a seperate top level block for defining proof purposes #294

tplooker opened this issue May 27, 2020 · 21 comments
Assignees
Labels
pending close Issue will be closed shortly if no objections

Comments

@tplooker
Copy link
Contributor

Given the conversation and consensus emerging in #283 around creating a top level block for storing verification methods the same argument could also be made for organising proof purposes, this might look something like the following.

{
    "@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"
      }
    ],
    "proofPurpose": {
        "authentication": [
            "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf#0"
        ],
        "assertionMethod": [
            "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf#0"
        ],
        "capabilityDelegation": [
            "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf#0"
        ],
        "capabilityInvocation": [
            "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf#0"
        ]
   }
}

The benefit of an approach like the above makes the concept of a proof purpose more obvious to those trying to understand the data model and perhaps makes a variety of topics within the specification easier to define such as the definition of the core data model and how it should be extended.

Interesting conversation on this related topic has also been had in #273

@tplooker tplooker changed the title Create a seperate top level block for storing proofPurposes Create a seperate top level block for defining proof purposes May 27, 2020
@OR13
Copy link
Contributor

OR13 commented Jun 2, 2020

Looks interesting... but its pretty much equivalent to this totally valid representation today:

{
    "@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"
        ]
}

So i'm not really a fan of it.

@OR13
Copy link
Contributor

OR13 commented Jun 2, 2020

technically, it can even be compressed further...

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

@tplooker
Copy link
Contributor Author

tplooker commented Jun 2, 2020

Yes thats correct however the real intent of this issue is to tease out what the core terms at the base of the data model describes, at the moment it is a bucket for verification methods a flat map of verification relationships and a bucket for service endpoints?

@OR13
Copy link
Contributor

OR13 commented Jun 2, 2020

I'd be a fan of maybe grouping verificationMethod / proofPurposes in the spec / adding additional language about them to make the relationship clear... but I think we should strive for as flat representations as possible.

@tplooker
Copy link
Contributor Author

tplooker commented Jun 2, 2020

To aligned with @dlongley 's terminology it would be represented as

{
    "@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"
      }
    ],
    "verificationRelationships": {
        "authentication": [
            "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf#0"
        ],
        "assertionMethod": [
            "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf#0"
        ],
        "capabilityDelegation": [
            "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf#0"
        ],
        "capabilityInvocation": [
            "did:ethr:0x74fd35392f1c2a97c6080e0394b1995b0e63c8bf#0"
        ]
   }
}

@tplooker
Copy link
Contributor Author

tplooker commented Jun 2, 2020

+1 I'm not advocating for increased nesting just trying to tease out how we describe the core data model as that sets the behaviour for implementers and extenders of it

@OR13
Copy link
Contributor

OR13 commented Jun 2, 2020

The nesting is not necessary... But I do think that its doing a great job of highlighting that verificationMethods (catchall, or purpose specific) and service endpoints are the only things we are currently putting in the root of the document.

@OR13
Copy link
Contributor

OR13 commented Jun 2, 2020

nesting becomes most useful when your categories of items are many, and your number of items per category are many...

we have 2 categories, and good reason to believe that neither will become large...

@tplooker
Copy link
Contributor Author

tplooker commented Jun 2, 2020

Can you clarify how you do not see verificationRelationships as a third bucket?

@OR13
Copy link
Contributor

OR13 commented Jun 2, 2020

This works today:

{
    "@context": [
      "https://www.w3.org/2018/credentials/v1",
      "https://www.w3.org/2018/credentials/examples/v1"
    ],
    "id": "http://example.gov/credentials/3732",
    "type": [
      "VerifiableCredential",
      "UniversityDegreeCredential"
    ],
    "issuer": {
      "id": "did:web:vc.transmute.world"
    },
    "issuanceDate": "2020-03-10T04:24:12.164Z",
    "credentialSubject": {
      "id": "did:example:ebfeb1f712ebc6f1c276e12ec21",
      "degree": {
        "type": "BachelorDegree",
        "name": "Bachelor of Science and Arts"
      }
    },
    "proof": {
      "type": "JsonWebSignature2020",
      "created": "2020-03-21T17:51:48Z",
      "verificationMethod": "did:web:vc.transmute.world#_Qq0UL2Fq651Q0Fjd6TvnYE-faHiOpRlPVQcY_-tA4A",
      "proofPurpose": "assertionMethod",
      "jws": "eyJiNjQiOmZhbHNlLCJjcml0IjpbImI2NCJdLCJhbGciOiJFZERTQSJ9..OPxskX37SK0FhmYygDk-S4csY_gNhCUgSOAaXFXDTZx86CmI5nU9xkqtLWg-f4cqkigKDdMVdtIqWAvaYx2JBA"
    }
  }

I guess I just don't see the point in changing the structure of controllers, given that the VC Data Model works fine with things as they are, and the extra nesting does not add anything but bytes.

https://w3c.github.io/vc-data-model/#proofs-signatures-0

The vocabulary and spec text should be where we communicate relationships, not object hierarchy.

@kdenhartog
Copy link
Member

Seems like there's some disagreement on this subject. I'll plan on writing the PR for #268 still, but will note it as blocked in the PR until we get further clarity on this topic about if core properties should be bucketed.

@kdenhartog kdenhartog added the discuss Needs further discussion before a pull request can be created label Jun 2, 2020
@rhiaro
Copy link
Member

rhiaro commented Jun 3, 2020

-1 on the nesting, it seems like extra parsing for little extra payoff. I think having only authentication, keyAgreement, assertionMethod, and possibly capabilityInvocation, capabilityDelegation as the properties defined in DID Core Spec means it shouldn't get too messy. With others potentially in the registries as they come up - even then, it's not likely to have loads of these in a single DID document, right? (or not right?)

But arranging them in the spec itself carefully is important. PR #304 creates a 'verification relationships' subsection under 'verification methods', so perhaps defining the terms there would be helpful (including moving authentication). (@kdenhartog)

@kdenhartog
Copy link
Member

Since I think it's pretty conclusive that moving things isn't going to make sense, is there any general reasoning that we might be able to provide in non-normative language to explain which aspects should be bucketed and which should exist at the top level of the document? This would be useful for authors of extensions.

@OR13
Copy link
Contributor

OR13 commented Jun 15, 2020

@kdenhartog Make this definition longer: https://w3c.github.io/did-core/#dfn-verification-relationship ?

I'm not sure what you mean by bucketed... nesting JSON is generally to be avoided, especially when the document size must remain small.

Can you propose language on this issue that you feel would address your concern?

@kdenhartog
Copy link
Member

Can you propose language on this issue that you feel would address your concern?
Based on the consensus I think we're heading towards something like this is what I'm thinking.

"In general, extensible attributes should be defined at the top level when _____ and should avoid defining nested JSON extension attributes."

The blank is the part I'm not certain about though.

@dlongley
Copy link
Contributor

A top level property relates the DID subject to something ... it establishes a relationship (defined by the property) between the DID subject and some other thing. Since a DID Document expresses information primarily about a DID subject, that's where most properties should be. A more deeply nested structure would involve expressing properties about that other thing (and its relationships to yet more things) that is related to the DID subject by the top-level property.

@kdenhartog
Copy link
Member

I'm thinking the proposed "bucketing" techinque here won't be done, but I think the action item from this PR would be to add some text about how extensions should be defined to align with what has been discussed in this thread.

@dlongley
Copy link
Contributor

dlongley commented Aug 18, 2020

@kdenhartog could you propose closing or create a PR for the text you'd like to see or something along those lines?

@tplooker
Copy link
Contributor Author

I'm satisfied that based on #304 the specific verificationRelationships defined in did core effectively addresses the expression I was after without having to reflect this via nesting in the resulting data structure. Therefore I'm happy to close this issue if there are no objections. I like the sentiment that @dlongley summarized above here and I think the combination of the first paragraph in core properties along with the definition offered for did document captures enough of the rationale w.r.t the did documents data structure.

@kdenhartog kdenhartog added pending close Issue will be closed shortly if no objections and removed discuss Needs further discussion before a pull request can be created labels Aug 21, 2020
@kdenhartog
Copy link
Member

Yup, I'm aligned with Tobias here as well that we've covered these details via other PRs and this topic is sufficiently covered in the spec now. Marked pending closed now as well.

@brentzundel
Copy link
Member

No objections raised since marked pending close. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending close Issue will be closed shortly if no objections
Projects
None yet
Development

No branches or pull requests

6 participants