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
86 stars 37 forks source link

WIP: editorial pass to rearrange/layer spec #287

Closed kimdhamilton closed 2 years ago

kimdhamilton commented 2 years ago

See preview

Reflects discussion of Jan 20 Discussion.

Notes

  1. limit_disclosure is now entirely moved to a Feature
  2. Rolled logically-similar properties more aggressively into single Features (e.g., Advanced Constraints)
  3. Added language to address MUST/MAY topic, e.g., "When using this Feature, the constraints object MAY contain a limit_disclosure property. If present, its value MUST be one of the following strings"
  4. I think there still may be some lingering weirdness in Submission Requirements, i.e., if a Feature introduces extensions to multiple objects
  5. Tried to add special formatting to Input Evaluation Extension handling. It's awkward and I probably need to discuss with @csuwildcat

Issues

  1. Other similar restructuring improvements: consider presentation submission special case (for 1) to avoid need for descriptor_map? Consider doing this in separate PR
  2. Need language around processing entity handling when Features not supported, reflecting the "naughty behavior" discussion. i feel like I can't capture this as well as @csuwildcat
  3. TODO: I need to ensure David's examples are captured
  4. TODO I manually checked in index.html in my branch; will remove that when merging
  5. TODO: Update authors/editors. Add David Chadwick and get updated list in general.

Feel free to refer to Note # or Issue # when commenting. E.g., "Note 2: I disagree"

Fixes #283; Overrides #275

bumblefudge commented 2 years ago

NB @David-Chadwick

bumblefudge commented 2 years ago

I've been trying to understand Issue # 2, and i am a little stumped. Part of me thinks we should spill a paragraph's worth of free digital ink after the second graf of ## Input Evaluation (L 1191 in the PR), explaining that the choices taken by processors in Input Evaluation may be judged or tracked by Verifiers, and to consider optional features subject to reputation-scoring. Was this what Daniel meant? Would a mentioned in the definition of Features in the ## Terminology section help, since they are free from conformance-stick but still subject to the reputation-carrot?

bumblefudge commented 2 years ago

Sidenote, are these 5 issues being opened as tracking issues if not addressed by this merge? Not sure if that was implied or not but asking for the sake of clear process.

kimdhamilton commented 2 years ago

Sidenote, are these 5 issues being opened as tracking issues if not addressed by this merge? Not sure if that was implied or not but asking for the sake of clear process.

It is a mix of "notes to self" that I will address before merging (e.g. index.html hack), issues we may quickly decide on as a group (and I can address with this change", and (default) issues I will create as separate tracking issues if not address.

kimdhamilton commented 2 years ago

I've been trying to understand Issue # 2, and i am a little stumped. Part of me thinks we should spill a paragraph's worth of free digital ink after the second graf of ## Input Evaluation (L 1191 in the PR), explaining that the choices taken by processors in Input Evaluation may be judged or tracked by Verifiers, and to consider optional features subject to reputation-scoring. Was this what Daniel meant? Would a mentioned in the definition of Features in the ## Terminology section help, since they are free from conformance-stick but still subject to the reputation-carrot?

Definitely that's the right line of thinking -- this is just an area I hadn't already addressed in the PR but something I want tracked. So worst case, if I don't have time to get to it in this already massive change, we can create a tracking issue

JaceHensley commented 2 years ago

Based on discussion on Jan 27th:

Note # 1: limit_disclosure should be moved into the base profile with additional language around "required" ("If you don't support "required" don't return anything" / "Wallets must understand how to at least know when "limit_disclosure" is "required"") Note # 2: Have the "status" constraint be it's own feature, the others are relational within the credential but status is about the credential itself Note # 3: Note # 4: Having a feature impact multiple objects doesn't feel weird to us

kimdhamilton commented 2 years ago

Thanks for the feedback!

PR Updates

See preview

Next steps

I agree with the discussion point that we should land this and follow up with subsequent PRs. Here are next steps I will take

Checkin Prereqs

Create issues

Other Group TODOs