Skip to content

change ZKP section #1030

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

Merged
merged 2 commits into from
Mar 23, 2023
Merged

change ZKP section #1030

merged 2 commits into from
Mar 23, 2023

Conversation

brentzundel
Copy link
Member

@brentzundel brentzundel commented Feb 8, 2023

This PR:

  • Adds a version of the note initially drafter by Orie in Add issue to zkp section #1026
  • modernizes and generalizes the language in the section in an attempt to make it more applicable to various zero knowledge proof mechanisms.
  • adds a note that the examples will either be re-worked or removed.

I believe this PR prepares this section for for one of these possibilities:

  1. The WG normatively specifies a method for securing VCs that supports ZKPs, in which case the examples and normative language in this section could be easily updated.
  2. The WG does not normatively specify a method for securing VCs that supports ZKPs, in which case the examples and normative language in this section would be removed, and the remaining language could be moved to the implementation guide.

Signed-off-by: Brent Zundel [email protected]


Preview | Diff

@brentzundel brentzundel requested a review from OR13 February 8, 2023 00:33
@brentzundel brentzundel requested a review from msporny as a code owner February 8, 2023 00:33
@iherman
Copy link
Member

iherman commented Feb 9, 2023

The issue was discussed in a meeting on 2023-02-08

  • no resolutions were taken
View the transcript

3.2. change ZKP section (pr vc-data-model#1030)

See github pull request vc-data-model#1030.

Brent Zundel: should be straightforward to adjust further or get rid of the section.

Manu Sporny: orie suggests process in order. Doing that.
… processing #1026, #1028 then need review on #1030.

@iherman iherman mentioned this pull request Feb 9, 2023
Copy link
Contributor

@dlongley dlongley left a comment

Choose a reason for hiding this comment

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

I remain a bit concerned about over promising what ZKP can realistically do in various use cases, but this text hasn't changed that much from what was originally approved to be in the VCDM 1.1 spec and there will be another revision at some point in the future before 2.0 is finished, so +1. Thanks for cleaning this up!

Copy link
Contributor

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

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

Selective disclosure, derived claim and unlinkability are three distinct features. By ZKP do we mean a way to secure VCs that fulfill all three or at least one? This needs to be clear. In the former, I think it is highly unlikely we will have a mature way before VC-data-model v2 CR, if the latter, SD-JWT will fall under this umbrella, but then the section name should be renamed from "ZKP".

@brentzundel
Copy link
Member Author

Selective disclosure, derived claim and unlinkability are three distinct features. By ZKP do we mean a way to secure VCs that fulfill all three or at least one? This needs to be clear. In the former, I think it is highly unlikely we will have a mature way before VC-data-model v2 CR, if the latter, SD-JWT will fall under this umbrella, but then the section name should be renamed from "ZKP".

I agree that if SD-JWT becomes a normative part of the spec, then this section should be expanded to include it.

index.html Outdated
mechanisms with <a>verifiable credentials</a> requires an <a>issuer</a> to
secure the <a>verifiable credential</a> in a manner that supports these
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mechanisms with <a>verifiable credentials</a> requires an <a>issuer</a> to
secure the <a>verifiable credential</a> in a manner that supports these
mechanisms with <a>verifiable credentials</a> requires the <a>issuer</a> to
have secured the <a>verifiable credential</a> in a manner that supports these

Copy link
Contributor

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

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

it should be clear that the properties being discussed are mainly Selective Disclosure and unlinkability (via derivatives and identifier clinding), and one mechanism does not have to achieve all three at the same time.

Copy link
Member

@msporny msporny left a comment

Choose a reason for hiding this comment

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

Approving with the assumption that minor nits and other people's suggestions are integrated.

@brentzundel
Copy link
Member Author

@Sakurann and @TallTed I have made changes to the PR to reflect your comments. Please re-review.

Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Much better than it was.

@msporny msporny requested a review from Sakurann March 4, 2023 22:48
@msporny
Copy link
Member

msporny commented Mar 4, 2023

Merging this is currently blocked by @Sakurann. I've requested an explicit re-review. @Sakurann, please re-review the changes that @brentzundel has made to this PR so we can either merge it, or make further changes to address your concerns.

@msporny
Copy link
Member

msporny commented Mar 13, 2023

Merging this is currently blocked by @Sakurann. I've requested an explicit re-review. @Sakurann, please re-review the changes that @brentzundel has made to this PR so we can either merge it, or make further changes to address your concerns.

Still waiting on this (understanding that @Sakurann was on travel last week and unable to get to a review here).

Copy link
Contributor

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

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

Despite the change in the title, the section is still all about "zero-knowledge proof", which is a specific term and would not include things like SD-JWT. But given there are caveats to rewrite/remove this section written in the text, will keep the reservations until when that PR comes in later.

@iherman
Copy link
Member

iherman commented Mar 15, 2023

The issue was discussed in a meeting on 2023-03-14

  • no resolutions were taken
View the transcript

2.2. change ZKP section (pr vc-data-model#1030)

See github pull request vc-data-model#1030.

Manu Sporny: I think we're ready to merge this.
… There are some changes that Kristina and Ted have suggested, but I think those are good to go unless someone here objects.

Dave Longley: +1 to merge with whatever editorial fixes.

Brent Zundel: ok, I'll apply language and merge.

Signed-off-by: Brent Zundel <[email protected]>
@brentzundel brentzundel requested a review from msporny March 21, 2023 16:59
Co-authored-by: Ted Thibodeau Jr <[email protected]>
@David-Chadwick
Copy link
Contributor

Have you considered the impact of these edits on section 5.4 (data schemas) as this has an example that concerns ZKPs. Are edits also needed to this section?

@msporny
Copy link
Member

msporny commented Mar 23, 2023

Have you considered the impact of these edits on section 5.4 (data schemas) as this has an example that concerns ZKPs. Are edits also needed to this section?

If they are, they'll need to go in another PR -- this one is ready to merge. Please raise an issue if you think this needs to be fixed, @David-Chadwick or @brentzundel.

@msporny
Copy link
Member

msporny commented Mar 23, 2023

Normative, multiple reviews, changes requested and made, no objections, merging.

@msporny msporny merged commit dfa4884 into w3c:main Mar 23, 2023
@Sebastian-Elfors-IDnow
Copy link

Thanks for the re-write of the ZKP section. It's a great improvement.

For completeness, the term "unlinkability" could be introduced. It is related to the bullet about "blinded signatures", which could be extended as follows:

"Blinded signatures also allows for unlinkable proofs, which removes a common source of correlatable properties about a holder during multiple presentations to one or more verifiers."

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.

10 participants