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

Added reserved confidenceMethod property to the vocabulary #1226

Closed
wants to merge 7 commits into from

Conversation

iherman
Copy link
Member

@iherman iherman commented Aug 3, 2023

This follows the latest update of the VCDM spec. Note that I have also added a ConfidenceMethod class, serving as a common superclass for specific instances.

It was not clear in the defintions whether there is any extra domain information to be added. So I left that open.

msporny and others added 7 commits August 2, 2023 10:21
17 approvals, open for more than a week, no objections during the Special topic call on 08-01-2023

* Add section on Ecosystem Compatibility.

* Fix grammar in Ecosystem Compatibility section.

Co-authored-by: Ted Thibodeau Jr <[email protected]>

* Add references to "digital credentials" specifications.

* Update Ecosystem Compatibility based on VCWG Special Topic call.

* Add clarification around when a transformation becomes a VC.

* Add note about what constitutes a verifiable credential.

* Clarify that a conforming document has two possible media types.

* Clarify that JOSE, COSE, wrapped conforming documents are VCs/VPs.

* Clarify which specs need to follow the ecosystem guidelines.

Co-authored-by: Dave Longley <[email protected]>

* Remove vague statement about "any securing mechanism" per @jandrieu.

Co-authored-by: Joe Andrieu <[email protected]>

* Apply editorial suggestions from @Sakurann to Ecosystem Compatibility.

Co-authored-by: Kristina <[email protected]>

---------

Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Dave Longley <[email protected]>
Co-authored-by: Joe Andrieu <[email protected]>
Co-authored-by: Kristina <[email protected]>
Co-authored-by: Ted Thibodeau Jr <[email protected]>
minor typo fix.
@@ -79,6 +85,12 @@ property:
range: IRI
comment: An entity about which claims are made.

- id: confidenceMethod
label: Confidence method
defined_by: https://w3c-ccg.github.io/confidence-method-spec/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We reserved this in a table... do we expect that term definition to show up here?

https://www.w3.org/2018/credentials#confidenceMethod

https://w3c.github.io/vc-data-model/#reserved-extension-points

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current vocabulary includes entries for the other reserved terms already, and the definition in the spec explicitly refers to a URL in the credentials vocabulary. Ie., consistency requires to put this entry into the table, too.

Copy link
Contributor

@OR13 OR13 Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are saying, we don't expect to see these URLs in the vocabulary.

And defined by URL can change in the future if the working group decides to define reserved terms?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am saying there is no definition as of today. We are not yet at Rec and the reserved terms are somehow at risk. If they become "regular" than a proper URL must come to the fore.

@@ -29,6 +29,12 @@ class:
label: Credential status
comment: A Credential Status provides enough information to determine the current status of the credential (for example, suspended or revoked). This class serves as a supertype for specific status types.

- id: ConfidenceMethod
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be reserving a "class" not a "predicate"... I guess we really need to add Credential or we will have a weird mismatch between how to handle classes in the spec.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand what you are saying, @OR13. The vocabulary includes a number of "Classes" that serve as some sort of an "abstract" superclass. I fail to see how this has any relation to the presence or not of a separate class for credentials.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iherman previously you had raised a PR to define Credential, which failed to gain consensus in relation to VerifiableCredential... and yet, all the other parts of the vocab seem to leverage abstract classes.

As a consumer of this ontology, I am wondering what leads the working group to decide when to use an abstract class and when not too.

If we see abstract classes for ConfidenceMethod as valuable, why did we not see an abstract class for Credential as valuable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer not to discuss this issue here. It is a valid question, but it would turn this PR into an endless discussion thread, going way beyond a simple addition to the vocabulary. Let us avoid that; this is what may lead to problems like #1226 (comment)

Copy link
Member

@msporny msporny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR includes commits on main, needs a rebase.

@iherman
Copy link
Member Author

iherman commented Aug 4, 2023

PR includes commits on main, needs a rebase.

@msporny I do not understand this remark. Rebase on what?

@msporny
Copy link
Member

msporny commented Aug 4, 2023

@msporny I do not understand this remark. Rebase on what?

Look at the file diff... it contains your changes and a bunch of changes on an invalid branch on main. There was a PR that was pulled in a few days ago (the Ecosystem Compatability PR) that destroyed history, so I had to re-create history on main and force push to main, which then caused this PR (and many others) to have issues. To rebase:

git checkout main
git pull
git checkout iherman-add-confidenceMethod-vocabulary
git rebase main

^^ That might do the trick, or you might have conflicts you need to resolve.

I can also squash-merge, but the current diff is probably going to confuse people that are reviewing, and they'll start suggesting changes to things that were not in this PR.

@iherman
Copy link
Member Author

iherman commented Aug 4, 2023

@msporny that set of instructions completely screwed up my local copy :-( Git asks me to push a bunch of things to main (?) which I presume I should not do, and the branch iherman-add-... got also garbled beyond recognition... And, to be honest, I am not sure what I am doing. I should certainly not push anything onto main, I presume...

The only thing I can propose to do is a hard reset (the changes for this PR and for #1225 are minimal, so it may be simpler). Ie, when you think everything is fine on the github side, then I remove my local copies, do a fresh cloning, and then submit new PR with the changes. Otherwise this might become way too complicated.

@OR13
Copy link
Contributor

OR13 commented Aug 4, 2023

@iherman I recently addressed a similar concern with:

You can use rebase, if you are comfortable with it... Or you can look at the diff, and remove the changes that 'teleported' into your PR thanks to the previous merges to main.

I prefer the latter, an extra commit is easier for me to manage than caring about rebase... the resulting diff will be the same at the end of it.

Its tedius, but you can do all this from the github UI if you are patient enough.

@@ -2230,7 +2230,7 @@ <h3>Data Schemas</h3>
</p>
<p>
If multiple schemas are present, validity is determined according to the
processing rules outlined by each associated <code>credentialSchema</code>
processing rules outlined by each associated <code>credentialSchema</code>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
processing rules outlined by each associated <code>credentialSchema</code>
processing rules outlined by each associated <code>credentialSchema</code>

@@ -3588,13 +3608,14 @@ <h3>Reserved Extension Points</h3>
<tr>
<td>`renderMethod`</td>
<td>
A property used for specifying how to render a credential into a visual,
A property used for specifying one or more methods to render a credential into a visual,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A property used for specifying one or more methods to render a credential into a visual,
A property used for specifying how to render a credential into a visual,

auditory, or haptic format. The associated vocabulary URL MUST be
`https://www.w3.org/2018/credentials#renderMethod`.
<p class="issue" title="(AT RISK) Reservation depends on implementations">
This reserved property is at risk and will be removed from the
specification if at least one specification with two independent implementations
are not demonstrated by the end of the Candidate Recommendation Phase.
See <a href="https://w3c-ccg.github.io/vc-render-method/">Verifiable Credential Rendering Methods</a>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
See <a href="https://w3c-ccg.github.io/vc-render-method/">Verifiable Credential Rendering Methods</a>.

Comment on lines +3649 to +3724

<section class="normative">
<h3>Ecosystem Compatibility</h3>

<p>
There are a number of digital credential formats that do not natively use the
data model provided in this document, but are aligned with a number of concepts
in this specification. At the time of publication, examples of these digital
credential formats include
<a href="https://www.rfc-editor.org/rfc/rfc7519.html">
JSON Web Tokens</a> (JWTs),
<a href="https://www.rfc-editor.org/rfc/rfc8392.html">
CBOR Web Tokens</a> (CWTs),
<a href="https://www.iso.org/standard/69084.html">ISO-18013-5:2021</a>
(mDLs),
<a href="https://hyperledger.github.io/anoncreds-spec/">
AnonCreds</a>,
<a href="https://www.ietf.org/archive/id/draft-mcnally-envelope-02.html">
Gordian Envelopes</a>, and
<a href="https://www.ietf.org/archive/id/draft-ssmith-acdc-02.html">
Authentic Chained Data Containers</a> (ACDCs).
</p>

<p>
If conceptually aligned digital credential formats can be transformed into a
<a>conforming document</a> according to the rules provided in this section, they
are considered <em>"compatible with the W3C Verifiable Credentials ecosystem"</em>.
A <a>conforming document</a> is either a <a>verifiable credential</a> serialized
as the `application/vc+ld+json` media type or a <a>verifiable presentation</a>
serialized as the `application/vp+ld+json` media type. Specifications that
describe how to perform transformations that enable compatibility with
the Verifiable Credentials ecosystem:
</p>

<ul>
<li>
MUST identify whether the transformation to this data model is one-way-only or
round-trippable.
</li>
<li>
MUST preserve the `@context` values when performing round-trippable
transformation.
</li>
<li>
MUST result in a <a>conforming document</a> when transforming to the data
model described by this specification.
</li>
<li>
MUST specify a registered media type for the input document.
</li>
<li>
SHOULD provide a test suite that demonstrates that the specified transformation
algorithm to the data model in this specification results in
a <a>conforming document</a>.
</li>
<li>
SHOULD ensure that all semantics utilized in the transformed
<a>conforming document</a> follow best practices for Linked Data. See
Section <a href="#getting-started"></a>, Section
<a href="#extensibility"></a>, and Linked Data Best Practices [[?LD-BP]]
for additional guidance.
</li>
</ul>

<p class="note" title="What constitutes a verifiable credential?">
Readers are advised that a digital credential is only considered
compatible with the W3C Verifiable Credentials ecosystem if it is a
<a>conforming document</a> and it utilizes at least one securing mechanism, as
described by their respective requirements in this specification. While some communities might call some digital
credential formats that are not <a>conforming documents</a>
"verifiable credentials", doing so does NOT make that digital credential
compliant to this specification.
</p>

</section>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<section class="normative">
<h3>Ecosystem Compatibility</h3>
<p>
There are a number of digital credential formats that do not natively use the
data model provided in this document, but are aligned with a number of concepts
in this specification. At the time of publication, examples of these digital
credential formats include
<a href="https://www.rfc-editor.org/rfc/rfc7519.html">
JSON Web Tokens</a> (JWTs),
<a href="https://www.rfc-editor.org/rfc/rfc8392.html">
CBOR Web Tokens</a> (CWTs),
<a href="https://www.iso.org/standard/69084.html">ISO-18013-5:2021</a>
(mDLs),
<a href="https://hyperledger.github.io/anoncreds-spec/">
AnonCreds</a>,
<a href="https://www.ietf.org/archive/id/draft-mcnally-envelope-02.html">
Gordian Envelopes</a>, and
<a href="https://www.ietf.org/archive/id/draft-ssmith-acdc-02.html">
Authentic Chained Data Containers</a> (ACDCs).
</p>
<p>
If conceptually aligned digital credential formats can be transformed into a
<a>conforming document</a> according to the rules provided in this section, they
are considered <em>"compatible with the W3C Verifiable Credentials ecosystem"</em>.
A <a>conforming document</a> is either a <a>verifiable credential</a> serialized
as the `application/vc+ld+json` media type or a <a>verifiable presentation</a>
serialized as the `application/vp+ld+json` media type. Specifications that
describe how to perform transformations that enable compatibility with
the Verifiable Credentials ecosystem:
</p>
<ul>
<li>
MUST identify whether the transformation to this data model is one-way-only or
round-trippable.
</li>
<li>
MUST preserve the `@context` values when performing round-trippable
transformation.
</li>
<li>
MUST result in a <a>conforming document</a> when transforming to the data
model described by this specification.
</li>
<li>
MUST specify a registered media type for the input document.
</li>
<li>
SHOULD provide a test suite that demonstrates that the specified transformation
algorithm to the data model in this specification results in
a <a>conforming document</a>.
</li>
<li>
SHOULD ensure that all semantics utilized in the transformed
<a>conforming document</a> follow best practices for Linked Data. See
Section <a href="#getting-started"></a>, Section
<a href="#extensibility"></a>, and Linked Data Best Practices [[?LD-BP]]
for additional guidance.
</li>
</ul>
<p class="note" title="What constitutes a verifiable credential?">
Readers are advised that a digital credential is only considered
compatible with the W3C Verifiable Credentials ecosystem if it is a
<a>conforming document</a> and it utilizes at least one securing mechanism, as
described by their respective requirements in this specification. While some communities might call some digital
credential formats that are not <a>conforming documents</a>
"verifiable credentials", doing so does NOT make that digital credential
compliant to this specification.
</p>
</section>

@OR13
Copy link
Contributor

OR13 commented Aug 4, 2023

@iherman I suggested the changes I think you need to apply to fix this, in case you just want to accept them instead of muck with rebase.

@iherman
Copy link
Member Author

iherman commented Aug 4, 2023

@iherman I suggested the changes I think you need to apply to fix this, in case you just want to accept them instead of muck with rebase.

Thank you @OR13 but I am uneasy getting into something I do not understand... let alone the fact that, as I said in #1226 (comment), the rebase instructions seem to have completely messed up my local clone, so I would probably be better off starting with a fresh clone anyway.

My only question is (@msporny ?) is the github content in order now, ie, can I clone and submit the two super simple PR-s now, or should I wait?

@iherman
Copy link
Member Author

iherman commented Aug 4, 2023

let alone the fact that, as I said in #1226 (comment), the rebase instructions seem to have completely messed up my local clone, so I would probably be better off starting with a fresh clone anyway.

I usually try to keep the local clones up-to-date by pulling all changes almost every morning. I presume that I pulled down the mess to my local copy at some point...

@msporny
Copy link
Member

msporny commented Aug 4, 2023

I usually try to keep the local clones up-to-date by pulling all changes almost every morning. I presume that I pulled down the mess to my local copy at some point...

Yes, correct, that's what happened. :(

My only question is (@msporny ?) is the github content in order now, ie, can I clone and submit the two super simple PR-s now, or should I wait?

Yes, the main branch is back to a stable state. Feel free to proceed.

@iherman
Copy link
Member Author

iherman commented Aug 4, 2023

Move to #1228, this PR should be closed without merge.

@TallTed
Copy link
Member

TallTed commented Aug 4, 2023

Move to #1228, this PR should be closed without merge.

Maybe change the title, as most of the discussion here is about the glitch, not the intended meat of the PR?

@iherman
Copy link
Member Author

iherman commented Aug 5, 2023

Move to #1228, this PR should be closed without merge.

Maybe change the title, as most of the discussion here is about the glitch, not the intended meat of the PR?

I'd prefer to close this PR right away. But I am not a code owner

@msporny
Copy link
Member

msporny commented Aug 5, 2023

@iherman wrote:

I'd prefer to close this PR right away. But I am not a code owner

Closing.

@msporny msporny closed this Aug 5, 2023
@msporny msporny deleted the iherman-add-confidenceMethod-vocabulary branch November 11, 2023 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants