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

Bug in the context file? #1373

Closed
iherman opened this issue Dec 5, 2023 · 7 comments
Closed

Bug in the context file? #1373

iherman opened this issue Dec 5, 2023 · 7 comments
Labels
pending close Close if no objection within 7 days

Comments

@iherman
Copy link
Member

iherman commented Dec 5, 2023

Consider a skeleton VP content:

{
  "@context": [
    "https://www.w3.org/ns/credentials/v2"
  ],
  "type": "VerifiablePresentation",
  "verifiableCredential": [{
    "@context": [
      "https://www.w3.org/ns/credentials/v2"
    ],
    "id": "http://university.example/credentials/1872",
    "type": "VerifiableCredential",
    "issuer": "https://university.example/issuers/565049"
  }]
}

per specification, the N-quads equivalent should be something like (I use CURIEs to make it more readable):

_:vp rdf:type cred:VerifiablePresentation .
_:vp cred:verifiableCredential _:credGraph .
<http://university.example/credentials/1872> rdf:type cred:VerifiableCredential _:credGraph .
<http://university.example/credentials/1872> cred:issuer <https://university.example/issuers/565049>  _:credGraph.

However, what I get on the playground:

<http://university.example/credentials/1872> <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <https://www.w3.org/2018/credentials#VerifiableCredential> .
<http://university.example/credentials/1872> <https://www.w3.org/2018/credentials#issuer> <https://university.example/issuers/565049> .
_:b0 <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <https://www.w3.org/2018/credentials#VerifiablePresentation> .
_:b0 <https://www.w3.org/2018/credentials#verifiableCredential> <http://university.example/credentials/1872> .

i.e., the "enclosing" graph does not appear.

This is either a bug in the @context file or the playground. If the former, it is the WG's job to solve it...

Do I miss something?

@dlongley @BigBlueHat @davidlehn @pchampin

@BigBlueHat
Copy link
Member

@iherman fwiw, @gkellogg's Distiller creates these n-quads from the same JSON-LD...so there may indeed be a bug in the playground.

Here are the n-quads from the Distiller--which match what you were after:

_:g524580 <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <https://www.w3.org/2018/credentials#VerifiablePresentation> .
_:g524580 <https://www.w3.org/2018/credentials#verifiableCredential> _:g524600 .
<http://university.example/credentials/1872> <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <https://www.w3.org/2018/credentials#VerifiableCredential> _:g524600 .
<http://university.example/credentials/1872> <https://www.w3.org/2018/credentials#issuer> <https://university.example/issuers/565049> _:g524600 .

I'll do some more direct testing with jsonld.js (which is used on the Playground), and see how deep the rabbit hole goes. @davidlehn may have other thoughts.

@iherman iherman removed the before-CR label Dec 5, 2023
@iherman
Copy link
Member Author

iherman commented Dec 5, 2023

@iherman fwiw, @gkellogg's Distiller creates these n-quads from the same JSON-LD...so there may indeed be a bug in the playground.

Here are the n-quads from the Distiller--which match what you were after:

_:g524580 <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <https://www.w3.org/2018/credentials#VerifiablePresentation> .
_:g524580 <https://www.w3.org/2018/credentials#verifiableCredential> _:g524600 .
<http://university.example/credentials/1872> <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <https://www.w3.org/2018/credentials#VerifiableCredential> _:g524600 .
<http://university.example/credentials/1872> <https://www.w3.org/2018/credentials#issuer> <https://university.example/issuers/565049> _:g524600 .

I'll do some more direct testing with jsonld.js (which is used on the Playground), and see how deep the rabbit hole goes. @davidlehn may have other thoughts.

Phew. From the point of view of VCDM I am relieved (and also from the point of view of my own understanding...).

@msporny @brentzundel I am not sure whether we want to close this issue (being irrelevant for the VCDM spec) or leave it open until is handled, to avoid errors in discussions like in #1358 (comment) or #1358 (comment). I leave this decision to you. In any case, I removed the before-CR label.

@gkellogg
Copy link
Member

gkellogg commented Dec 5, 2023

I suspect it's a bug in handling scoped context in jsonld.js. Looking at the credentials/v2 context definition:

  • VerifiablePresentation is a term which is used as a type in the outermost object, thus the type-scoped context it defines is used for expanding the rest of that object, which includes the term definition for verifiableCredential
  • verfiableCredential which is in scope has a @container: @graph, meaning that its value is taken to be a named graph, which at least in my implementation, gets a blank node graph label. It also sets @context: null, but establishes a new @context which is used to expand the values of that graph object.

For some reason, jsonld.js does not seem to interpret the value of verifiableCredential as being a blank graph. I would have thought that some test would catch this case, though.

@pchampin
Copy link
Contributor

pchampin commented Dec 5, 2023

FWIW, Sophia, which is based on @timothee-haudebourg's JSON-LD library, also parses this correctly: example.

@davidlehn
Copy link
Contributor

There was missing JSON-LD test coverage for this use case and a jsonld.js bug. A JSON-LD test suite test was submitted, jsonld.js patched, and the json-ld.org playground was updated. It should work as expected now.

@iherman
Copy link
Member Author

iherman commented Dec 7, 2023

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

  • no resolutions were taken
View the transcript

2.8. Bug in the context file? (issue vc-data-model#1373)

See github issue vc-data-model#1373.

Brent Zundel: I think this one is before CR.

Manu Sporny: It's before CR.

Dave Longley: +1 to before CR.

Orie Steele: I would like some elaboration to be in the minutes: could someone explain the essence of the bug?

Dave Longley: It's not that we have a bug in our context file. It's that we added terms to make v1 VCs compatible with v2 Verifiable Presentations.
… We needed to make sure that context files for v1 VCs didn't affect the context files for v2 VPs.
… Some implementations had bugs, some didn't.
… We should be able to resolve this by nullifying the context as part of these implementations.
… We don't need to do anything in this WG.

Orie Steele: Sounds like there is no bug in our context, and this issue should be closed. It also sounds like there are buggy JSON-LD processors, that produce inconsistencies, that cause credentials to not verify.

Dave Longley: Orie: It is a reasonable expectation for any software is that there will be bugs from time to time.

Orie Steele: the complicated the software, the more reasonable it is to expect bugs.

Dave Longley: Orie: sometimes! :) ... i would say that it's not a good idea to assume simple software is less likely to be buggy (as this may lead to more likely bugs in such software).

Manu Sporny: You're coming across as trolling, Orie. Please stop. You asked us to explain something. We did, and now you're expressing what comes across as mock surprise.

@iherman
Copy link
Member Author

iherman commented Dec 7, 2023

The bug has indeed been handled and the example works as expected.

This means that the issue turned out not to be in our @context file, so there is no action for this WG.

@iherman iherman closed this as completed Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending close Close if no objection within 7 days
Projects
None yet
Development

No branches or pull requests

6 participants