-
Notifications
You must be signed in to change notification settings - Fork 0
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
Move component count test to verifier suite & update to 7 components #63
base: main
Are you sure you want to change the base?
Conversation
aljones15
commented
Oct 17, 2024
•
edited
Loading
edited
- moves a verification statement to verifier suite
- update test title and link
- adds a generator that creates a derived proof with only 2 components in it
- updates the issuer assertions on components to allow for 7 components
Co-authored-by: Ted Thibodeau Jr <[email protected]>
it('If the result is not an array of five, six, or seven elements ' + | ||
'— a byte array, a map of integers to integers, two arrays of ' + | ||
'integers, and one or two byte arrays — an error MUST be raised ' + |
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.
1 byte array
+ 1 map of integers to integers
+ 2 arrays of integers
That's 4 elements.
Add 1 or 2 byte arrays
, and that's 5 or 6 elements, total.
What is the possible 7th element?
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.
@TallTed this is direct from the latest spec here: https://w3c.github.io/vc-di-bbs/#parsederivedproofvalue
I was under the impression that the 7th element was due to changes in related bbs specs WRT to the key, blind signing, and pseudonym signing, but I'm not aware of exactly why this came in. It looks like those changes were made here: w3c/vc-di-bbs#181
@Wind4Greg do you know what the 7th element refers to in this statement? I believe this statement was merged in because the blind and pseudonym specs changed.
It sounds like this might be an issue for the spec itself though and not the test suite.
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.
@TallTed fairly confident the 7th element is: https://github.com/w3c/vc-di-bbs/blob/ad66d386015efc775a0320676381d0af951d6af2/index.html#L679-L686
So related to blind and pseudonym algorithms.
Draft PR here: https://github.com/w3c/vc-di-bbs/pull/188/files
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.
The first paragraph of 3.3.7 parseDerivedProofValue —
A single derived proof value object is produced as output, which contains a set of six or seven elements, having the names "bbsProof", "labelMap", "mandatoryIndexes", "selectiveIndexes", "presentationHeader", "featureOption", and, depending on the value of the featureOption parameter, "pseudonym" and/or "lengthBBSMessages".
— is internally inconsistent (the set may have six, seven, or eight elements) and it disagrees with the last paragraph of that algorithm (again, should say "six, seven, or eight elements" including "featureOption", "pseudonym" and/or "lengthBBSMessages") —
Return derived proof value as an object with properties set to the five, six, or seven elements, using the names "bbsProof", "labelMap", "mandatoryIndexes", "selectiveIndexes", "presentationHeader", and optional "pseudonym" and/or "lengthBBSMessages", respectively. In addition, add featureOption and its value to the object.
These should be brought into agreement. Whatever the result is, it should then be applied to this part of the test suite.
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.
Moved to #64 which will probably move to the https://github.com/w3c/vc-di-bbs/ repo.
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.
#64 is untouched. The enumeration of the elements (a byte array, a map of integers to integers, two arrays of integers, and one or two byte arrays
) here does not agree with the possible counts indicated (five, six, or seven elements
). The enumeration only leads to five or six elements
; there is no indication of a possible seventh.