Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support returning multiple presentations for a single dcql credential query when requested using
multiple
#398support returning multiple presentations for a single dcql credential query when requested using
multiple
#398Changes from all commits
860c76f
d912e5d
c585737
389e7de
7ba6150
9ee9b81
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to take a step back and consider how we reference a returned verifiable presentation against not only a credential query but also a credential set otherwise with complex queries it may become increasingly difficult for the verifier to understand how the query was satisfied when a credential set was used. If we accept this proposal as is, without discussing that I think we might make that issue worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tplooker My understanding is these changes shouldn't add complexity to credential set functionality.
The only change is that the RP can now receive more than one presentation if they have specified so using
multiple
in the credential query, so instead of performing a single check on a single presentation the RP would perform the same check on each presentation in the array. Essentially the checks are now always performed in a loop.This complexity is added to the check at the credential query level rather than the credential set level.
Though I do see your point that there is more complexity being added overall, moreso when factoring in other desired changes such as #397.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, have made a comment with regard to that issue, another example I was thinking of here though is how will this feature interact with the
credential_sets
feature more generally? For example.Basically in response to the credential_sets query I might get one credential in response to the underlying "bar" credential_query OR N credentials in response to the underlying "foo" query.
Again maybe we view this as an edge case and therefore are not that concerned but its worth considering how the complexity might compound here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are the implications? reconsidering a change introduced in this PR? adding a note in this PR? opening an issue? no action?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tplooker i think this is an edge case but also that the spec makes sense for the use cases I'm dealing with. To make foo and bar concrete an RP could ask for either my transcript OR multiple course badges
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i created this issue #439 to continue the discussion if needed.