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

format property needs more details #412

Closed Sakurann closed 1 year ago

Sakurann commented 1 year ago

I think description of format property in the Presentation Definition object should say that depending on a format, format object must (?) contain alg or proof_type. Otherwise, one needs to go to a claim format registry to find that out (if one can reach the registry).

bumblefudge commented 1 year ago

sounds like a breaking change I'd rather defer to V3 😉

Sakurann commented 1 year ago

it is not breaking. adding text in PE v2.1 saying go to the registry to figure out what objects need to be in formats might do the job.

bumblefudge commented 1 year ago

I was making a joke because the same week you requested this change, you also requested elsewhere that work not begin on V3 for some time.

I'm not exactly sure, but I'm fairly certain the text you're requesting is here: https://identity.foundation/presentation-exchange/#claim-format-designations It was added last July by the illustratively-titled PR #349

If anyone would like to add additional explanatory text to make the situation clearer or the registry easier to reach, such as in the Abstract section or elsewhere, a non-breaking PR is always welcome! This editorial group meets weekly but covers a lot of repos so we have much shorter turnaround for PRs than issues.

Sakurann commented 1 year ago

Please reopen the issue. It has not been completed. I filed an issue because i felt like the text you are referring to should be clarified.

Why is it the expectation that a person who finds an issue has to also do a PR? What is the role of the editing team? All the issues that the editing does not have time for get closed?

nklomp commented 1 year ago

Agreed

csuwildcat commented 1 year ago

The best way forward is to have the parties requesting a change create a PR that adds whatever they think is needed, then we can pull it in.

decentralgabe commented 1 year ago

I would also add the format property is not clear. I see two main uses of format:

So pretty much need a way to say - ok the top level claim may be x, and within that we allow [x, y, z]

Sakurann commented 1 year ago

Is there actually a use case where *_vp can have multiple credential formats inside..?

decentralgabe commented 1 year ago

Is there actually a use case where *_vp can have multiple credential formats inside..?

Yes. A user has a number of credentials from different issuers, of different formats (e.g. an LDP_VC drivers license, a JWT_VC employment verification). They wish to submit them against a Presentation Request in a VP. Probably most of the use cases we intend to support will have this constraint.

bumblefudge commented 1 year ago

Agreed on today's call to make a minor text change to clarify that having to check the registry is a feature not a bug and that different claim formats have different required and valid values for each column.

Before closing we will also check JSON Schema examples for alg/proof filtering that might contradict the new language.

bumblefudge commented 1 year ago

Is there actually a use case where *_vp can have multiple credential formats inside..?

Yes. A user has a number of credentials from different issuers, of different formats (e.g. an LDP_VC drivers license, a JWT_VC employment verification). They wish to submit them against a Presentation Request in a VP. Probably most of the use cases we intend to support will have this constraint.

moved this out to a separate issue (#431) to make sure Kristina's editorial request is treated faster!