w3c / vc-data-model

W3C Verifiable Credentials v2.0 Specification
https://w3c.github.io/vc-data-model/
Other
287 stars 106 forks source link

Clarify the domain and range of the `verifiablePresentation.verifiableCredential` property #1366

Closed andresuribe87 closed 9 months ago

andresuribe87 commented 9 months ago

From https://github.com/w3c/vc-data-model/pull/1358#issuecomment-1831580248, it became evident that the name for the property verifiablePresentation.verifiableCredential is misleading, and the definition needs further clarification.

Relevant excerpt from the conversation below:

In an ideal world, I would agree with you. Yes, the term verifiableCredential is misleading insofar as the graph naming aspect is hidden (and this misconception actually may have led to a bug in v1.0). The problem I see is that renaming a term that is already widely used in implementations out there might be the source of problems. I leave the judgement to others, but, pragmatically, we may have to live with the current term.

I would argue that if renaming fixes these sort of bugs, that's exactly what we should do in 2.0 :)

As I said: in theory yes, pragmatically probably not. But that is a really separate issue, too, I believe.

msporny commented 9 months ago

-1 for at least these reasons:

We don't embed the data structure name in the property names for most (if not all?) other properties. We don't use "mediaTypeString", we use "mediaType", we don't use "typeArray", we use "type". So, listing the data structure in the name feels like the wrong thing to do here.

Everything we do in the spec is a part of a graph, so injecting the word "graph" here and not elsewhere doesn't seem to make sense either... and injecting it everywhere feels unnecessary.

I'm not convinced that adding this word is going to "fix" any sort of bug, as the case for why it would prevent bugs hasn't been made.

Finally, implementations have been using verifiableCredential for years now w/o major issues. We're past what would be theoretically nice to do what's the practical ROI on this change? Presently, it feels like the ROI would be negative on this change: it wouldn't fix any bugs, it would look weird because it's not how we (generally) name properties, and it would be incompatible with all current implementations out there.

It falls into the same category as renaming "credentialSubject" to "subject" -- it would've been nice to do that in v1.0, but given that this is the 3rd version of the specification, changing it now doesn't feel like it would add much value while further complicating processors.

andresuribe87 commented 9 months ago

The definition was changed from what VDCM 1.1 stated to say that only verifiable credential graphs are acceptable as the value.

Previously, implementations would use verifiableCredential to put in externally secured verifiable credentials. Now, the WG is stating that it isn't an acceptable use. This means that implementations are already being asked to update the semantics of the field. The decisions of the WG require implementations to update the handling of the logic of the verifiableCredential property.

Without renaming, the property is likely to be used in the same way as it was used in VCDM 1.1. Renaming that property prevents implementations from misusing the term.

dlongley commented 9 months ago

@andresuribe87,

The definition was changed from what VDCM 1.1 stated to say that only verifiable credential graphs are acceptable as the value.

How this is not a breaking change was discussed here: https://github.com/w3c/vc-data-model/pull/1259#discussion_r1311879185

OR13 commented 9 months ago

basically a dupe of https://github.com/w3c/vc-data-model/issues/1365

still basically blocked by

iherman commented 9 months ago

The issue was discussed in a meeting on 2023-12-06

View the transcript #### 2.6. Clarify the domain and range of the `verifiablePresentation.verifiableCredential` property (issue vc-data-model#1366) _See github issue [vc-data-model#1366](https://github.com/w3c/vc-data-model/issues/1366)._ **Andres Uribe:** I think that if the clarifications are made in #1365, then I think the point made in this issue is moot. It will also be addressed by the solution that was suggested yesterday of adding a type property. > *Manu Sporny:* +1 to what andres just said, agree with that direction. **Brent Zundel:** Do we need both issues to track the concern? **Andres Uribe:** I think one builds upon the other. One would be automatically closed. **Manu Sporny:** Maybe changing the title would help. You're not asking for a rename of the property now, only asking for clarification. So we can change the title of the issue. **Orie Steele:** People have been putting Verifiable Credentials in the verifiableCredential property of Verifiable Presentations already. … Changing this now will be unacceptable to Transmute. … I suggest allowing either using IRIs or the VCs directly. That's also consistent with the RDF here. > *Orie Steele:* I read the minutes. **Brent Zundel:** I'll make this before CR. Who's willing to be assigned? **Dave Longley:** I think Orie is referring to a specific JSON-LD property that has a bug, and once this has been fixed there won't be an issue. > *Orie Steele:* dlongley, confirming you are disclosing that [https://json-ld.org/playground/](https://json-ld.org/playground/) produces invalid N-Quads? **Sebastian Crane:** Personally, is this a clarification on whether you can embed the definition of a VC within a VP? Or is this something different? **Brent Zundel:** It is related to that. There are RDF concerns about whether ... you put the VC thing instead VP.verifiableCredential and that's ok, a VC that's been externally secured using a mechanism that uses an envelope, it creates a new data structure so it no longer fits within that property. **Sebastian Crane:** So this is an uncontroversial change in the mechanics to our RDF data model. **Brent Zundel:** I don't think I'm comfortable with saying uncontroversial here. **Sebastian Crane:** Thank you for the clarification.
msporny commented 9 months ago

@andresuribe87, I read back over the current specification text and think it does state the domain and range pretty clearly:

image

What concrete changes are you requesting to this text?

IOW, I agree with @OR13, this seems to be a dupe of #1365 (based on the way the discussions are going in the WG). Marking this as pending close. @andresuribe87 feel free to object if you disagree w/ the above and we're not seeing the nuance between the two issues.

brentzundel commented 9 months ago

Closing this as a duplicate of #1365