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

Remove ZKP Section #999

Closed
wants to merge 1 commit into from
Closed

Remove ZKP Section #999

wants to merge 1 commit into from

Conversation

OR13
Copy link
Contributor

@OR13 OR13 commented Dec 14, 2022

See #863

In the future, if there is a ZKP format that the working group can refer to, such as:

  1. A future work item for BBS selective disclosure
  2. A future work item for JWP
  3. A future work item for SD-JWT

We might define this section and refer to those items, if they can be referred to.

It was also noted, that the original text is preserved for all time in the v1 and v1.1 TRs.


Preview | Diff

@OR13 OR13 requested a review from msporny as a code owner December 14, 2022 18:37
@OR13
Copy link
Contributor Author

OR13 commented Dec 14, 2022

@tplooker I think we intend to refer to BBS eventually, assuming that is possible to do before the charter for the current WG ends, happy to help update this section in the future.

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.

I don't believe this is where we had consensus on the Editor's call. I thought we had agreed to place a big warning at the top of the section noting that the section would be deleted/rewritten before entering Candidate Recommendation if there was no standardized and implemented ZKP mechanism (such as BBS) provided.

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.

I'm not an Editor, and wasn't on that call, but I concur with the handling described by @msporny as his recollection of that call; i.e., the giant deletion is premature.

@jandrieu
Copy link
Contributor

Agreed with @msporny and @TallTed . Seems more appropriate to put a warning in at this time.

@iherman
Copy link
Member

iherman commented Jan 11, 2023

The issue was discussed in a meeting on 2023-01-11

  • no resolutions were taken
View the transcript

2.2. Remove ZKP Section (pr vc-data-model#999)

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

Manu Sporny: Next PR is to remove the ZKP section..
… PR 999 is the ZKP section. We need guidance what to do with the ZKP section..
… Brent has written most of the ZKP section..
… We need more commentary on this PR..

@brentzundel
Copy link
Member

At this stage I would much prefer a warning being added over removing the section entirely.

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.

Having a clean, easy-to-read specification that explains only the minimum of what implementers need to know to get started with Verifiable Credentials would greatly benefit in expanding and maturing the VC ecosystem at this stage.

There are enough documents surrounding vc-data-model that explain various adjacent concepts that are useful - e.g. use-cases, implementation guidelines, etc. - to where we can move current ZKP section (and maybe few other).

I understand why adding a warning might be an alternative, but I believe that moving this section to another related document would benefit the spec more.

We could say we will merge this PR only after this section is merged into another document (implementation considerations).

@TallTed
Copy link
Member

TallTed commented Jan 12, 2023

I understand why adding a warning might be an alternative, but I believe that moving this section to another related document would benefit the spec more.

Moving the section to another document would probably be fine with me. When such a PR is active or merged, I will reconsider this PR, but lacking that relocation, I still want the warning instead of the deletion.

@msporny
Copy link
Member

msporny commented Jan 15, 2023

Yes, we can merge this after this section is moved to the implementation guide. Waiting for a PR that does that before merging this one.

@brentzundel
Copy link
Member

The challenge with "moving this section to the implementation guide" is that this section includes normative requirements.
Until there exists a normative specification that provides equivalent requirements, I am opposed to deleting the zkp section.

@msporny
Copy link
Member

msporny commented Jan 15, 2023

@brentzundel wrote:

Until there exists a normative specification that provides equivalent requirements, I am opposed to deleting the zkp section.

Ok, in that case, it seems like a warning "that we might delete this section if we don't have a ZKP mechanism by CR" would be what could achieve consensus, then? Either that, or we just hold this PR until such a normative spec that provides equivalent requirements (like the BBS cryptosuite) appears.

I'm fine w/ doing either, though feel like a warning might be the best way to resolve this PR at the present.

@OR13, @Sakurann, thoughts?

@OR13
Copy link
Contributor Author

OR13 commented Jan 15, 2023

Warning issue works.

@OR13
Copy link
Contributor Author

OR13 commented Jan 16, 2023

I'd prefer to have the text of the issue approved by the working group before attempting to open another PR.

Some options for the call:

PROPOSAL: This section will be removed prior to CR if there are no active work items that are making use of its normative statements.

PROPOSAL: This section might be removed prior to CR.

The more I write these, the less I agree this is a good move.... the working group should not be holding onto 1.1 text in the hope that it will "one day" become relevant... especially if it contains normative statemetns that are untestable, and for which there are no active work items.

@Sebastian-Elfors-IDnow
Copy link

How about re-writing the section and changing the title to "Selective disclosure and unlikability"? That would make the section more generic and broaden the scope from Zero Knowledge Proof schemes to other techniques. For example, signed objects with salted hashed claims could be described as an alternative for selective disclosure. There is a proposal on this in issue 939.

@decentralgabe
Copy link
Contributor

Can we add a TODO in place?

@iherman
Copy link
Member

iherman commented Jan 25, 2023

The issue was discussed in a meeting on 2023-01-25

  • no resolutions were taken
View the transcript

4.2. Remove ZKP Section (pr vc-data-model#999)

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

Manu Sporny: looking for other folks to weigh in, otherwise just the folks conversing on the pr will contribute.
… location or removal of ZKP section - might be some issues with moving to impl guide given normative language.
… don't know what to do with the PR yet.

Ted Thibodeau Jr.: don't think normative language should be a blocker - could move and change to informative language.
… shouldn't necessarily throw away work.

Orie Steele: notes that the work done is preserved permenantly in 1.1.
… does believe that some of these items may get added back in to core data model at a later date.
… supportive of documenting support.
… concerned that we may be holding onto something that may prevent clean definition of ZKPs in 2.0+ based on work in 1.1.
… also concerned on testing / interop.

Ivan Herman: the fact that it is part of 1.1 will be "hidden" when 2.0 gets published.

Ted Thibodeau Jr.: +1 about the hiddenness (and forgettability) of 1.1 once 2.0 is out.

Ivan Herman: question without knowing all tech details, is this urgent to do now? Not at CR point, maybe a flag is better? other options noting potential removal?.
… not sure on urgency of removal.

Orie Steele: The text adds burden to reviews.....

Kristina Yasuda: no urgency does not mean not doing it... this topic has been up for discussion for a while..

Brent Zundel: as author (not chair) - the section needs significant reworking, removal likely, but removal at this time may be premature.
… don't think axing now is way to go, even if we are going to axe or rework later.

Kristina Yasuda: q.

Manu Sporny: can try to put a warning in.
… clearly communicate that section is at risk.
… does want to audit normative statements that should probably be removed or modified.
… rework this section with a mind for future inclusion of BBS.
… get things set up so that BBS can integrate when it is ready.
… is that a workable PR?.

Brent Zundel: +1 Manu.

Ted Thibodeau Jr.: +1 on manu's suggested plan.

Kristina Yasuda: not a big fan of just a warning.
… may be an issue suggesting shorter text.
… wants to avoid postponing problem.

Kristina Yasuda: +1 manu.

Kerri Lemoie: +1.

Brent Zundel: +1.

Manu Sporny: new PR for a new section intending to replace this section, that is shorter and attempts to capture the "spirit" of things in advance of BBS, non-normative for now, that can become normative later if appropriate.

Orie Steele: That suggestion seems good to me..

Phillip Long: +1.

Dave Longley: +1.

Michael Prorock: +1.

Ted Thibodeau Jr.: +1 works for me.

Ivan Herman: +1.

Orie Steele: If you request changes, clearly, i will modify my PR..

Manu Sporny: begs for other folks to help out because his stack is overflowing, otherwise will eventually get to it.

@OR13
Copy link
Contributor Author

OR13 commented Feb 2, 2023

Closing in favor of #1026

@OR13 OR13 closed this Feb 2, 2023
@iherman
Copy link
Member

iherman commented Feb 6, 2023

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

  • no resolutions were taken
View the transcript

3.2. (pr vc-data-model#999)

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

Manu Sporny: PR 999 to remove ZKP section, there is another PR in the works that is more modern, we're waiting for that to be submitted, then we will look at both PRs side by side to determine which one we want to take..

@msporny msporny deleted the fix/863-remove-zkp-section branch July 27, 2023 21:06
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.

9 participants