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!: VCs and proper DID documents as basic identity modules in SDK #807

Merged
merged 67 commits into from
Dec 14, 2023

Conversation

rflechtner
Copy link
Contributor

@rflechtner rflechtner commented Oct 31, 2023

fixes KILTProtocol/ticket#2156

This PR is the combined result of refactoring efforts over the last couple of months, making the following improvements:

  • The default representation of Kilt Credentials has been transitioned to one conforming to the VC Data Model 1.0.
    • Functions to support and interact with the previous representation is now available from the new package @kiltprotocol/legacy-credentials.
    • The new functions are available from the new package @kiltprotocol/credentials.
  • The Did resolver is now fully specification-conformant, implementing resolve, resolveRepresentation, and dereference as defined in https://www.w3.org/TR/did-core/#resolution.
  • Implements Verifiable Presentations and VC Data Integrity 1.0 for presentation signing
    • We implemented 3 cryptosuites for this purpose, compatible with Digital Bazaar’s implementation
  • Provides high-level implementations of common use cases, e.g.:
    • Resolving a DID
    • Issuing a VC
    • Creating a Presentation from VCs
    • Verifying a VC or a VP, checking the revocation status of a VC
  • Removal of low-level functions from sdk-js.
    • These are still available to be imported directly from the packages in which they are defined.

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 and others added 30 commits August 23, 2023 15:07
* Add base types for the compliant DIDs

* Add base types for the compliant DID resolution

* Add dereferencing

* Refactor of Did.utils and Did.chain

* Work on DID.signature

* Bit more mess here and there

* yarn check working fine

* did.resolve implemented

* DID resolver ok

* AccountLinks2.chain.ts

* Add resolveRepresentation to resolver

* Better DID resolver

* Resolver improvements

* Improve API surface to keep the same abstraction level

* First DID integration test passing!

* More DID integration tests uncommented

* More DID integration tests passing

* More DID integration tests

* All DID integration tests passing

* Refactoring

* Add utility function to add a verification method

* Minor refactoring

* Revert unwanted change

* Remove addKeypairAsVerificationMethod function

* Light DID unit tests passing

* Unit tests for light and full DID passing

* Minor changes to signing interfaces

* DIDSignature unit tests passing

* Halfway through the refactoring

* Better error messages

* Fix imports

* Refactoring

* First refactor

* Fix DID dereferencing and signature resolution

* Compiling!

* Compiling after applying stash

* Fix TestUtils

* verificationRelationship -> verificationRelationships

* Prepare ground for proper DID dereferencing

* Add support for query params in DID URLs

* Compiling again

* WIP dereferencing in tests

* Credential.spec.ts for legacy credential almost passing

* Did.signature.spec.ts working again

* Return minimal DID document when DID is deactivated or migrated

* Add test for legacy signature support

* Sr25519Signature2020.spec.ts test failing

* Key -> verification method renaming

* Key/service ID to URL

* More constants

* Fix yarn check

* Fix JSDocs

* Update tests

* Minor refinements

* Minor fixes

* Apply suggestion

* Push different yarn.lock

* Fix type import

* Fix integration test

* Fix integration test pt.2

* Update unit tests for DidResolver.spec.ts

* Half-way through new DidResolver unit tests

* Unit tests for DidResolver complete

* Replace buffer

* Apply suggestion

* Apply w3n suggestion

* export type

* Replace Buffer with Uint8Array

* Add multikey context

* Fix unit tests for linked signature suites

* Legacy support for document loader

* Make getStoreTxFromDidDocument only used in tests

* Use Object.fromEntries

* Refactor exportQueryParamsFromUri

* Use resolve instead of dereference

* Fixes

* Revert DidSignature renaming

* Rename URI to DID and keep DID URL
@rflechtner rflechtner self-assigned this Oct 31, 2023
ntn-x2 and others added 7 commits November 22, 2023 10:34
* chore: upgrade typedoc and fix links (amend me!)

* docs: update contribution guide

* feat: updaing to new link syntax in asset creds and utils and vc export

* feat: updating types links

* feat: updating the links in did

* feat: updating the links in core

* feat: catching missed links

* feat: checking whitespaces

* style: running through the yarn build docs and catching problems

* fix: updating linting

* Update packages/types/src/CryptoCallbacks.ts

Co-authored-by: Raphael Flechtner <[email protected]>

---------

Co-authored-by: Raphael Flechtner <[email protected]>
Co-authored-by: Raphael Flechtner <[email protected]>
* refactor!: move credential related functionality in one folder
* feat: allow signing of transactions with SignerInterface
* refactor!: use Date for timestamp
* refactor!: update issue submission options
* refactor!: separate presentation creation and signing
* feat: credential issuance interfaces
* feat: holder credential functions
* feat: verifier interfaces
* refactor!: move balance utils to chain-helpers
* refactor!: move connect/disconnect to chain-helpers
* refactor!: rename core to credentials
* chore: export sr25519Signer
* chore: uppercase Issuer, Holder, Verifier
* refactor: make signer selection synchronous
* fix: missed a few mentions of the old core package
* chore: remove unnecessary use of balance utils in bundle tests
* chore: do not use encryption when extracting the pk from KeyringPairs
* refactor!: remove all exports but the basics
* test: fix tests & bundle tests
* feat: export signerFromKeypair
* test: interim reduction of coverage goals
@rflechtner rflechtner marked this pull request as ready for review December 14, 2023 13:36
Copy link
Member

@Dudleyneedham Dudleyneedham left a comment

Choose a reason for hiding this comment

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

LGTM otherwise. There are consistencies to importing and declaring. You use Did for the type some places and KiltDid in others. Let us stick with DID and declare where we use them.

.github/workflows/tests.yml Outdated Show resolved Hide resolved
@@ -8,13 +8,13 @@
"license": "BSD-4-Clause",
"scripts": {
"check": "tsc -p tsconfig.json --noEmit",
"build": "yarn workspaces foreach -p -t --exclude '{root-workspace}' run build",
"build": "yarn workspaces foreach -ptAv --exclude '{root-workspace}' run build",
Copy link
Member

Choose a reason for hiding this comment

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

What is this doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

p: parallel
t: topological
A: all packages
v: verbose (more info)

AssetDidUri,
DidUri,
AssetDid,
Did as KiltDid,
Copy link
Member

Choose a reason for hiding this comment

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

Can you Declare this where it is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all these imports are internal... I don't think this is super relevant?

@@ -9,7 +9,7 @@ import type { AccountId } from '@polkadot/types/interfaces'
import type { PublicCredentialsCredentialsCredential } from '@kiltprotocol/augment-api'
import type {
HexString,
DidUri,
Did as KiltDid,
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.


import {
attestation,
blockHash,
credential,
credential as VC,
Copy link
Member

Choose a reason for hiding this comment

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

Please can we do this where we are using it?

import {
attestation,
credential,
credential as VC,
Copy link
Member

Choose a reason for hiding this comment

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

NAUGHTY


/**
* An [[Attestation]] certifies a [[Claim]], sent by a claimer in the form of a [[Credential]]. [[Attestation]]s are **written on the blockchain** and are **revocable**.
* An {@link IAttestation | Attestation} certifies a {@link IClaim | Claim}, sent by a claimer in the form of a {@link ICredential | Credential}. Attestations are **written on the blockchain** and are **revocable**.
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
* An {@link IAttestation | Attestation} certifies a {@link IClaim | Claim}, sent by a claimer in the form of a {@link ICredential | Credential}. Attestations are **written on the blockchain** and are **revocable**.
* An {@link IAttestation | Attestation} certifies a {@link IClaim | Claim}, sent by a claimer in the form of a {@link ICredential | Credential}.
Attestations are **written on the blockchain** and are **revocable**.

packages/did/src/Did.signature.ts Outdated Show resolved Hide resolved
Comment on lines +25 to +34
createSigner as es256kSigner,
cryptosuite as es256kSuite,
} from '@kiltprotocol/es256k-jcs-2023'
import {
createSigner as ed25519Signer,
cryptosuite as ed25519Suite,
} from '@kiltprotocol/eddsa-jcs-2022'
import {
cryptosuite as sr25519Suite,
createSigner as sr25519Signer,
Copy link
Member

Choose a reason for hiding this comment

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

Can we declare this where we use it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to make these available, but obviously they need to be identifiable by the algorithm they use. What are you suggesting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suggest if you have a better idea (like exporting both under cryptosuite and createSigner on an object, e.g., ed25519Suite) we make a ticket and implement it on a feature branch forked from develop

@rflechtner rflechtner merged commit 7de0d75 into develop Dec 14, 2023
13 checks passed
@rflechtner rflechtner deleted the vc-refactor branch December 14, 2023 16:03
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.

3 participants