-
Notifications
You must be signed in to change notification settings - Fork 2
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
chore: backport did config using DataIntegrity proofs #37
base: main
Are you sure you want to change the base?
Conversation
"@kiltprotocol/es256k-jcs-2023": "latest-rc", | ||
"@kiltprotocol/sr25519-jcs-2023": "latest-rc", | ||
"@kiltprotocol/eddsa-jcs-2022": "latest-rc", | ||
"@polkadot/keyring": "^12.3.2", |
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.
packages are ready to be released if no issues are found.
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.
Some questions to be answered
origin: string, | ||
did: DidUri, | ||
{ | ||
expirationDate = new Date(Date.now() + 1000 * 60 * 60 * 24 * 365 * 5), |
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 this a default of a year?
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.
Does this have any purpose or is it for show? E.g. Does the chain reflect this?
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 solution is completely off-chain. but credential verification checks this. And it's 5 years by default, I didn't change this.
credential: DomainLinkageCredential, | ||
expectedOrigin: string, | ||
expectedDid?: DidUri | ||
{ expectedDid, allowUnsafe = false }: { expectedDid?: DidUri; allowUnsafe?: boolean } = {} |
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.
allowUnsafe
is a flag that switches whether verification allows any data to be signed, supporting the credentials that are currently being used by socialKYC and the like, or whether it requires the JSON-stringified credential to be signed, securing the credential contents against manipulation.
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.
Sounds good. Do we need to document this? Considering the naming people may consider it, well frankly, unsafe.
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
fixes https://github.com/KILTprotocol/ticket/issues/3179
Backporting the use of DataIntegrity proofs for DID configuration resources from #31.
Cleans up a bit of duplication in the types files as well.
How to test:
Create credentials using the CLI.
Checklist: