-
Notifications
You must be signed in to change notification settings - Fork 115
Clean up use-cases summary #1166
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
Conversation
Signed-off-by: Brent Zundel <[email protected]>
Signed-off-by: Brent Zundel <[email protected]>
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.
lgtm
index.html
Outdated
different <a>issuers</a> into a single artifact, a <a>verifiable presentation</a> | ||
that is tamper-evident. |
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.
different <a>issuers</a> into a single artifact, a <a>verifiable presentation</a> | |
that is tamper-evident. | |
different <a>issuers</a> into a single artifact, a <a>verifiable presentation</a> | |
that might be tamper-evident. |
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.
proof is optional on presentations
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.
Proof is optional on both credentials and presentations. Structure of both make verifiability testable, i.e., enable verification, which process might succeed or fail, depending on (among other things) presence of proof.
different <a>issuers</a> into a single artifact, a <a>verifiable presentation</a> | |
that is tamper-evident. | |
different <a>issuers</a> into a single artifact, a <a>presentation</a> | |
that might be tamper-evident (making it a <a>verifiable presentation</a>). |
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.
Rather than accept your suggestions, I've added the word 'can', which I hope addresses your concerns about the strict accuracy of the sentence.
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.
a verifiable presentation that is tamper-evident.
This remains false, and my change request is focused on this, as @TallTed noted, proof is optional everywhere, this means we cannot assert the "tamper-evident" property.
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.
a verifiable presentation can be tamper evident, which is all the language currently says.
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.
We should not assume presentations are protected or credentials are protected, proof is optional in both.
the data model of this specification in JSON-LD, the context and vocabulary are likely to be normative, it does not make sense to use words other than JSON-LD to describe the data model of the spec.
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Co-authored-by: Orie Steele <[email protected]>
So close! Change one more word, and I'm happy. |
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Done! |
…ion as being possibly tamper-evident Signed-off-by: Brent Zundel <[email protected]>
@OR13 please re-review |
@OR13 I have changed the PR according to your comments, please re-review. |
deterministic, bi-directional, and lossless. Any | ||
<a>verifiable credential</a> or <a>verifiable presentation</a> has to be | ||
transformable to the JSON-LD data model defined in this document via a | ||
deterministic process so that its verification can be processed in an |
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.
@brentzundel I am still struggling with this part. I know you didn't create this text, you are just trying to clean it up.... but the whole paragraph is problematic.
Many digital signatures are not deterministic, some datetime conversions are always lossy....
DataIntegrityProofs can be lossy ( order issues, context integrity issues, etc...)... Even assuming this paragraph was meant to only apply to data integrity proofs, I am not sure it is accurate.
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 think there a numerous problems with the whole use cases section, which is non-normative but feels very official. I've opened #1169 to have a conversation about removing it.
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.
Based on #1166 (comment)
Editorial, multiple reviews, changes requested and made, no objections, merging. |
This PR attempts to clarify the use cases summary and make it easier for newcomers to parse by editorially unifying language around verifiable credentials and verifiable presentations.
It is intended as a first step toward addressing #1126
Preview | Diff