-
Notifications
You must be signed in to change notification settings - Fork 16
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
generate presentation submission with path_nested #2563
generate presentation submission with path_nested #2563
Conversation
can you rebase this so its easier to review |
561c238
to
14229ee
Compare
vcr/pe/presentation_definition.go
Outdated
// ErrUnsupportedFilter is returned when a filter uses unsupported features. | ||
// Other errors can be returned for faulty JSON paths or regex patterns. | ||
func (presentationDefinition PresentationDefinition) Match(vcs []vc.VerifiableCredential) (PresentationSubmission, []vc.VerifiableCredential, error) { | ||
func (presentationDefinition PresentationDefinition) Match(context PresentationContext, vcs []vc.VerifiableCredential) ([]vc.VerifiableCredential, error) { |
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.
weird public-facing API; what to does the caller need to do with "Index"? This should probably be an internal shelper struct. And this function should create the submission, in my opinion (or there should be a separate PresentationDefinition.CreateSubmission
which does this, potato/potato). Now callers have to set the right DefinitionId
, while this is part of creating the submission.
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'll create an AddMatch
to a PresentationSubmission. That way the submission can also handle the index and single vs plural paths.
vcr/pe/presentation_definition.go
Outdated
for _, descriptorMap := range descriptorMaps { | ||
nestedDescriptorMap := InputDescriptorMappingObject{ | ||
Id: descriptorMap.Id, | ||
Format: "ldp_vp", // todo support jwt_vp |
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.
easy, just get the format from selectedVCs[i].Format()
(if the slices pair up)
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.
Better, small remarks left
|
||
// AddWallet adds credentials from a wallet that may be used to create the PresentationSubmission. | ||
func (b *PresentationSubmissionBuilder) AddWallet(holder did.DID, vcs []vc.VerifiableCredential) { | ||
b.holders = append(b.holders, holder) |
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.
note: we might have to support multiple holders later (match submission from 2 wallets of different holders).
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.
Which already works for the submission part. Only the VP creation part is missing, therefore the SignInstructions are added.
vcr/pe/presentation_submission.go
Outdated
|
||
// Builder returns a new PresentationSubmissionBuilder. | ||
// A PresentationSubmissionBuilder can be used to create a PresentationSubmission with multiple wallets as input. | ||
func (presentationDefinition PresentationDefinition) Builder() PresentationSubmissionBuilder { |
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.
Builder()
of type X normally builds X (although Go's different since it has no static functions), so maybe rename this to Submission()
?
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.
Named it PresentationSubmissionBuilder
Part of #2553
This PR covers the presentation submission creation with
path_nested
.Requires #2559