decentralized-identity / presentation-exchange

Specification that codifies an inter-related pair of data formats for defining proof presentations (Presentation Definition) and subsequent proof submissions (Presentation Submission)
https://identity.foundation/presentation-exchange
Apache License 2.0
82 stars 37 forks source link

Add JMandel's new example without overwriting simpler one #423

Closed bumblefudge closed 1 year ago

bumblefudge commented 1 year ago

Thank you for the swift PR, @jmandel ! We discussed #421 in today's meeting and were excited to see the array bug found and fixed, since many VCs in the realworld would fail this filter just for typing type as an array. We also thought the implicit conflict here between very simplified examples and the significant complexity needed to craft JSON Schema for real-world inputs IS something we should work on in future iterations of the protocol and implementation guide, so thanks for nudging us in that direction.

One nit we identified is that the examples are also test vectors, so rather than overwriting the pd_filter2.json, I created an alternate PR that preserved the simplified version for testing purposes, and also for illustrative purposes. This way, someone reading the examples from left to right will still see the high-level structure first, before being plunged into the brutality of real-world use-cases being encoded as JSON Schema :D

If you feel this addresses #420 to your satisfaction give it a review and we'll merge this one; or, if you want to argue for further work on either PR, we can try that instead.

jmandel commented 1 year ago

Thanks for putting this together. My sense is that having a mix of simple and complex examples (per se) is a good thing, but without any commentary it might be quite confusing for a reader to see a simple (but unrealistic / likely to fail in real life) and more elaborate (but robust in real life) example of the same goal, accomplished via the same underlying properties.

I would suggest the following principle for examples:

If examples use real properties from the VC 1.1 Data Model, they should aim to be compatible with real instances that leverage those properties

... and with this principle, maybe it'd be good to have a "Simple: Filter VC by university and grade level" (assuming these would be made-up properties, with an implied cardinality of 1), and "Complex: Filter VC by Trust Framework Membership" (using the real properties and therefor a filter capable of matching them).

I recognize this may be more work than it's worth. If you want the examples to stay as they are in this PR, adding commentary would be good. But this is... not easy to explain succinctly and well, and maybe deserves treatment outside of the spec.

bumblefudge commented 1 year ago

OK we'll discuss tweaking this next week before merging and tag you again! We've decided to do a "minor version" for errata (v2.1) and an implementation guide in tandem, so maybe it will be explained succinctly (but not well) in v2.1 and well (but not succinctly) in the Imp Guide :D

bumblefudge commented 1 year ago

Discussed on today's call. THe group takes the point, but is worried about overhauling all the examples in the spec. We'll come back to this in the context of adding e2e realworld examples to the implementation guide - tracking issue linked above