-
Notifications
You must be signed in to change notification settings - Fork 3
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
DIP-257 Attribute Sets and Attestation #275
Conversation
8309d04
to
be8b188
Compare
992df28
to
a1a2182
Compare
a1a2182
to
64dab1c
Compare
Other updates, fixes, and clarifications.
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.
All my comments were addressed, looks good to go
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.
Generally we have some consistency issues with Announcement vs announcement, and YES/NO vs YES no. All links have been checked as well
STYLEGUIDE.md
Outdated
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.
To the Style Guide I would add Activity Content - Uppercase A and C,
Verifiable Credentials - Uppercase V and C
Verifiable Credential Schema - Uppercase V, C and S
and
YES and no--When describing required items, use an all cap YES otherwise use a lowercase no.
### Verifiable Credential Link | ||
|
||
Attestation attachments must contain a link to a Verifiable Credential document. | ||
The link must contain a relationship identifier in the form of a DSNP Attribute Set Type (the `rel` field), to allow applications to determine the type of credential expected. |
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.
remove comma after (the 'rel' field)
@@ -18,6 +18,9 @@ Each Announcement has an enumerated type for use when separating out a stream of | |||
| 5 | ~~[Profile](Types/Profile.md)~~<sup>c</sup> | ~~a profile~~ | ~~YES~~ | ~~no~~ | | |||
| 6 | [Update](Types/Update.md) | an update to content| YES | no | | |||
| 7 | ~~[Public Key](Types/PublicKey.md)~~<sup>b</sup> | ~~a public key for secure communication~~ | ~~no~~ | ~~no~~ | | |||
| 8 | [User Attribute Set](Types/UserAttributeSet.md) | an attribute set for a DSNP User | YES | YES | | |||
| 9 | [DSNP Content Attribute Set](Types/DSNPContentAttributeSet.md) | an attribute set for a DSNP content item | YES | YES | | |||
| 10 | [External Content Attribute Set](Types/ExternalContentAttributeSet.md) | an attribute set for a non-DSNP content item | YES | YES | | |||
|
|||
<sup>a</sup> Since DSNP version 1.2, social graph changes use [User Data](UserData.md) operations as described in the [Graph](Graph.md) section. | |||
|
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.
Should we call out "b" as well as "a" down here?
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.
It's there, just didn't show in the diff section.
pages/DSNP/AttributeSets.md
Outdated
|
||
The conceptual model for Attribute Sets includes three types of data: | ||
|
||
1. A schema encoding rules for validating attribute set data. This MUST be in the form of a [DSNP Verifiable Credential Schema](../VerifiableCredentials/Types/VerifiableCredentialSchema.md) (a Verifiable Credential that contains a JSON Schema document). |
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.
- is confusing to me. Do you mean A schema that encodes rules for validating attribute set data?
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.
Yes. I'll use your wording.
pages/DSNP/AttributeSets.md
Outdated
Attribute Set Type canonical names are constructed as follows: | ||
|
||
* MUST be in the format _attributeSetTypeNamespace_ + "`$`" + _attributeSetTypeName_, where _attributeSetTypeNamespace_ MUST be either a multihash content hash (encoded as a multibase string), the DSNP DID of the schema author (beginning with "`did:dsnp:`"), or the empty string (for schemaless attribute set types). | ||
* _attributeSetTypeName_ MUST match a declared type value in the Verifiable Credential document |
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.
Needs a period at the end of this sentence
A DSNP DID document is a JSON-LD document representing key material associated with a DSNP User. | ||
|
||
| Property | Required | JSON Type | Description | Restrictions | | ||
| --- | --- | --- | --- | --- | |
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.
YES and NO should be YES and no to be consistent with other sections of the documentation.
| --- | --- | --- | --- | --- | | ||
| `type` | YES | String | Type of subject matter | MUST be `JsonSchema` | | ||
| `jsonSchema` | YES | Object | Embedded JSON Schema object | See [JSON Schema](#json-schema) | | ||
| `dsnp` | NO | Object | DSNP extension object | See [DSNP Extensions](#dsnp-extensions) | |
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.
Again YES/NO is YES/no in other parts of the document. And this link does not work.
| Property | Required | JSON Type | Description | Restrictions | | ||
| --- | --- | --- | --- | --- | | ||
| `type` | YES | Array of strings | Type of credential | MUST contain the strings `"VerifiableCredential"` and `"JsonSchemaCredential"` | | ||
|`credentialSubject` | YES | Object | Object containing JSON schema and DSNP extensions | See [Credential Subject](#credential-subject) | |
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.
This link does not work.
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.
Working for me
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 these are only an issue when looking at the github renderings; the published docs are OK.
| --- | --- | --- | --- | --- | | ||
| `type` | YES | Array of strings | Type of credential | MUST contain the strings `"VerifiableCredential"` and `"JsonSchemaCredential"` | | ||
|`credentialSubject` | YES | Object | Object containing JSON schema and DSNP extensions | See [Credential Subject](#credential-subject) | | ||
| `credentialSchema` | YES | Object | Metaschema defining JSON Schema types | See [Credential Schema](#credential-schema) | |
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.
This link does not work.
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.
Working for me
| Property | Required | JSON Type | Description | Restrictions | | ||
| --- | --- | --- | --- | --- | | ||
| `type` | YES | String | Type of subject matter | MUST be `JsonSchema` | | ||
| `jsonSchema` | YES | Object | Embedded JSON Schema object | See [JSON Schema](#json-schema) | |
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.
This link does not work.
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.
Working for me
Problem
Link to GitHub Issue(s): #257
Solution
Adds affordances for Attribute Sets via the Verifiable Credentials format.
Adds three new announcement types for Attribute Sets.
Specifies DSNP compliance criteria for use of VCs, and includes DSNP extensions for chains of trust and display of credentials.
Extends DSNP Activity Content usage to enable Attestations as attachments.