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

Add Presentation Submission Required field #309

Closed David-Chadwick closed 2 years ago

David-Chadwick commented 2 years ago

Add a field to presentation definitions to determine if presentation submissions are required or not

csuwildcat commented 2 years ago

This PR doesn't make sense, because it's literally codifying a statement that says "I am breaking compliance with the spec". There's no need to alert the wallet to include/exclude a required element if you don't use it in divergence with the spec. It's like messaging to someone "You don't need to wear your seat belt", meanwhile, wearing it is still the law.

David-Chadwick commented 2 years ago

@csuwildcat You are missing the point. presentation submissions are not needed in the majority of uses. Therefore mandating something that is not needed is like saying you must wear a crash helmet in a car. If you are a formula 1 driver it makes sense, but not for normal car usage. Therefore this PR allows the verifier to say if they require it or not.

csuwildcat commented 2 years ago

It doesn't matter, because this is a no-op. Either the client will be compliant all the time or they won't, there's no rational reason a compliant client would want to put the required pieces in sometime. Instead, they'd always include it, and then some noncompliant server can oddly choose not to use it when it arrives. There's no reason we need such a property in the spec, because nothing has to change for a noncompliant server to simply ignore the included Submission object.

David-Chadwick commented 2 years ago

But you are missing the majority of implementations that will never use it or implement it because they will never need it. Only in those rare cases where a RP might need it (because of the ambiguity of which VC matches which requirement) will it ask for it. But if the wallet does not support it then the RP can always work around this by making several requests to the wallet where the answer is not ambiguous.

csuwildcat commented 2 years ago

I agree with @brentzundel - it would be bizarre to add a flag in the spec that basically equates to: "I don't want to send the deterministic bits required to normalize processing across envelopes - please consider me a one-off you need to do special work to interface with". Flagging to people that you're going to deviate from deterministic processing isn't even an operable flag, all it says is: "I'm going off on my own, I don't adhere to the prescribed processing steps". You don't need a spec flag to do that, just don't touch the thing you don't want if you're not worried about your implementation being interoperable.

David-Chadwick commented 2 years ago

@brentzundel " allow a Verifier to indicate that they do not use parts of the specification." that is precisely what this is doing. The whole point of producing a simplified profile of the original PE (which is what v2 is now doing) is to simplify PE for the majority of use cases and remove unnecessary components. Presentation submission is unnecessary and not needed for the majority of uses, therefore I submit that most implementations wont implement it, in the same way that they wont implement all the optional features of the PD

csuwildcat commented 2 years ago

The difference is that Presentation Submission is part of the core data model that Presentation Definitions and the processing rules around both rely on. This isn't a request to simply forego use of an isolated feature one sprinkles in as desired, it destabilizes the deterministic processing procedure for everyone by creating a condition where you can't rely on the same ingest processing code working across implementations.

David-Chadwick commented 2 years ago

@csuwildcat "is part of the core data model" only because you decided it should be. A different viewpoint is that PS is an optional data structure that may be used in complex (and very rare) use cases where the RP cannot deterministically decide which VC matches which of the PD input descriptors, because more than one of the VCs match an input descriptor. In this case the RP may request the PS to be inserted into the response message to remove the ambiguity. I am suggesting, with this viewpoint in mind, that most implementors will not implement PS.

bumblefudge commented 2 years ago

Discussed on today's call; building on DW's comment in #323 we talked through some possible clarifications to spec text that would offer some clarity, and possible footguns and attack vectors specific to the "just-in-time" approach. @csuwildcat took the action item to work on an editorial pass with this in mind, and we're open to further PRs as needed to support the "single-VC/wallet-won't-generate" use-case.

JaceHensley commented 2 years ago

As per #323 we will not be merging this PR