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

refactor!: credentialSchema does not work for CTypes #774

Merged
merged 7 commits into from
Jul 14, 2023

Conversation

rflechtner
Copy link
Contributor

@rflechtner rflechtner commented Jun 15, 2023

fixes KILTProtocol/ticket#2747

We finally have a proper draft spec for JSON-schema descriptions of VCs - but this is not particularly compatible with our CTypes, and the latest CType version we released is even less likely to work with this.

So I wanted to explore what it would look like if we used this field for something else - namely for describing what a KiltCredentialV1 type VC has to look like. CType verification would then have to be moved elsewhere; I think it's best here to just do it as part of credential verification.

Done so far:

  • Update VC exporter to output a credentialSchema where the type is JsonSchema2023 and the id references the our credential schema for KiltCredentialV1 type credentials.
  • Require this credentialSchema on verification.
  • Perform CType validation on proof verification instead, with automatic fetching of the respective CType definition enabled by default (can be deactivated)
  • Add option to verification methods to specify the expected CType(s), which in combination with disabling the CType loader enforces that credentials follow these CType(s).
  • Define a new field on which the CType of a Kilt VC is listed and easily accessible (e.g. credential type field?).

How to test:

Please provide a brief step-by-step instruction.
If necessary provide information about dependencies (specific configuration, branches, database dumps, etc.)

  • Step 1
  • Step 2
  • etc.

Checklist:

  • I have verified that the code works
  • I have verified that the code is easy to understand
    • If not, I have left a well-balanced amount of inline comments
  • I have left the code in a better state
  • I have documented the changes (where applicable)
    • Either PR or Ticket to update the Docs
    • Link the PR/Ticket here

@rflechtner rflechtner force-pushed the rf-refactor-credentialSchema branch from 4b01737 to 1131d50 Compare June 15, 2023 15:24
@rflechtner rflechtner marked this pull request as ready for review June 27, 2023 11:13
@rflechtner rflechtner requested a review from ntn-x2 June 27, 2023 11:13
Copy link
Member

@ntn-x2 ntn-x2 left a comment

Choose a reason for hiding this comment

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

Your comments make sense.

@rflechtner
Copy link
Contributor Author

@ntn-x2 I did have a concern that CredentialSchema.validateSubject may be outdated as an accessor to a function verifying a credential's subject against it's (C)type due to the proposed change. This is because this check now has nothing to do with the VC's credentialSchema property. What's your opinion on this? Should we move this to KiltCredentialV1? Or do you have another suggestion for where this functionality should live?

@ntn-x2
Copy link
Member

ntn-x2 commented Jul 13, 2023

What's your opinion on this? Should we move this to KiltCredentialV1?

It would make sense, right? This is pretty much tied to what we do in this version of the spec.

@rflechtner rflechtner merged commit 93e8ac5 into develop Jul 14, 2023
@rflechtner rflechtner deleted the rf-refactor-credentialSchema branch July 14, 2023 09:33
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.

2 participants