Skip to content
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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

aljones15
Copy link
Contributor

@aljones15 aljones15 commented Oct 17, 2024

  1. moves a verification statement to verifier suite
  2. update test title and link
  3. adds a generator that creates a derived proof with only 2 components in it
  4. updates the issuer assertions on components to allow for 7 components

@aljones15 aljones15 self-assigned this Oct 17, 2024
tests/suites/verify.js Outdated Show resolved Hide resolved
@aljones15 aljones15 requested a review from TallTed October 20, 2024 14:24
Comment on lines +147 to +149
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 ' +
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

@aljones15 aljones15 Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants