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

More binding vote on PR309 #323

Closed bumblefudge closed 2 years ago

bumblefudge commented 2 years ago

There was a vote on the call two weeks ago not to merge #309 , but in the interest of a more broader and better-documented consensus, we are asking implementers and spec editors to vote yay or nay and, if they like, add suggestions for how to [else] to move forward/address the use-case of PS-ignoring implementations if not merging.

Please vote by next week so we can move forward either way and think about next steps.

@csuwildcat @brentzundel @rado0x54 @kimdhamilton @dtmcg @dwaite @decentralgabe @OR13 @wyc @David-Chadwick @nklomp

OR13 commented 2 years ago

The pull request in question: https://github.com/decentralized-identity/presentation-exchange/pull/309

It would be smart to get arguments documented in github before attempting to resolve this further.

OR13 commented 2 years ago

The thread on the PR is dominated by @csuwildcat and @David-Chadwick would you mind doing your best to give the "most generous" interpretations of each-others arguments?

David-Chadwick commented 2 years ago

In a nutshell my position is one of simplification of PE. Don't make it mandatory to implement Presentation Submissions. This is because PSs are not needed for the vast majority of transactions as the RP can determine which VC matches which input descriptor. For those rare transactions where the returned VCs cannot deterministically be matched with the input descriptors, the RP can always send multiple requests so that each one is deterministic.

dwaite commented 2 years ago

My perception is that PEX defines a data model as well as rules based on the data model for processing requests and responses. But as it does not define a transport, it does not get to define the rules by which a transport sends (or generates) those messages.

This is similar to how it uses JSON Path to specify the requirements for verifiable credentials, even when formats such as JSON-LD, CBOR-LD, VC-JWT, mDL, etc. do not actually work reliably with JSON Path. You have to get to predictably structured JSON for software to evaluate requirements, even if that JSON is nothing like what will be ultimately presented.

As such, it is inappropriate in PEX level to say that Presentation Submission is optional - it is required. On the flip side, PEX doesn't get to dictate that its Presentation Submission format is sent over the transport in a particular form. For example, presentation submission information may be added to the actual presentation, or may be omitted and generated dynamically if only one thing is being presented.

That latter example would be more interesting as a PR for PEX - what does a presentation submission need if only one credential was requested? Then transports can say they default to that (without sending bytes on wire) in that case.

kimdhamilton commented 2 years ago

My answer is a belabored, guilt-ridden "do not merge" but with much ♥️

I believe I understand the intent behind #309, and agree with that intent, but not with the fix.

We've previously made progress towards layering/detangling this spec, and we knew it would be a windy, tortured path. I think there's another way to achieve this goal, but need to mull it over.

Unfortunately, the only way to describe my hesitations right now are from an imprecise, touchy-feely, design-aesthetic angle: "feels like a blunt hammer". I hope to articulate this more constructively soon.

(sorry to be late to this party. I will be able to attend meetings regulatory next week -- have had an ongoing conflict the last 2 months)

decentralgabe commented 2 years ago

Do not merge. To me it is quite clear that the only valid response to a Presentation Definition wrapped in a request is a Presentation Submission.

I can see why one might want to implement half the spec, but it leaves open a great deal of uncertainty and puts an onus of processing difficulty on the verifier that is wholly unnecessary.

dtmcg commented 2 years ago

Apologies for the long response. Ive tried to keep it concise. (TLDR dont merge)

Why I want the spec to specify the a required response. It defines an output for the processing of a given input. Both of which are defined.

It empowers the holder/wallet (SSI FTW)

Simplifies Verifier/RP logic

I would contest that it simplifies the spec or the implementation. Having one way to do something is more simple multiple ways with implicit optimality, even if that one way is more verbose. I think the simplicity argument might come from the appearance that PE is a data structure definition.

I would put also suggest that a couple of the arguments for making it optional are use case specific vs generally applicable.

bumblefudge commented 2 years ago

That latter example would be more interesting as a PR for PEX - what does a presentation submission need if only one credential was requested? Then transports can say they default to that (without sending bytes on wire) in that case.

This is a really interesting way of framing a PR that might be easier to merge with consensus-- a section (whether normative or non-) that spells out a "default"/generate-PS-on-the-fly logic that would make a PS-free, non-inferential implementation still conform to the spec as currently written. Thanks, @dwaite, I was hoping someone would suggest something this concrete as a next step!

nklomp commented 2 years ago

I think most has been said. Especially the two first points from @dtmcg sum it up quite nicely for me:

The wallet gets to declare what it is submitting for what reason. It strips ambiguity & doest leave grey areas for verifiers to hold onto more data than they need to. The reason for response descriptor map is to handle query responses of heterogeneous data types, which allows discrimination across the typing and instance data dimensions.

Although our library with some modifications would be able to do without the submission data and I also mentioned the current approach proposed by @David-Chadwick in the past, I do believe that it will bite people in the end. I think it is important that the holder in all cases makes the mapping, as it is a straightforward process, and removes any potential for ambiguity.

I also agree that on some scenario's one could get around it, by allowing to generate it on the fly. Probably doesn't even have to be a normative section, because current spec doesn't dictate where/how the PS is transported to begin with AFAIK.

JaceHensley commented 2 years ago

Do not merge

can't add anything else that other's haven't already said


The wallet gets to declare what it is submitting for what reason. It strips ambiguity & doest leave grey areas for verifiers to hold onto more data than they need to.

:100:

csuwildcat commented 2 years ago

I concur with the rest of the majority here in opposing this PR/flag, which basically creates a bifurcation in processing expectations across the ecosystem of implementations. The tiny amount of client code needed to generate a unified Submission and avoid having any of these issues is just not enough to justify dealing with the interop issues and complexity it introduces.

bumblefudge commented 2 years ago

Discussing on the call now how we can detail/support a "just-in-time" deterministic PS obj and security model/spec language tweaks.

JaceHensley commented 2 years ago

Based on the discussion and the votes in this issue were closing this as resolved in favor of not merging #309