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

feat: add Multikey as supported vm type #1720

Merged
merged 4 commits into from
Jan 31, 2024

Conversation

dbluhm
Copy link
Contributor

@dbluhm dbluhm commented Jan 31, 2024

This PR adds support for the Multikey verification method type defined in vc-data-integrity. Similar to JsonWebKey2020, it can be used to represent multiple key types.

This is perhaps a naive implementation. This is my first time digging around inside of credo. Please let me know if there's anything I need to improve!

Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
@@ -40,6 +44,10 @@ export const keyDidEd25519: KeyDidMapping = {
return getKeyFromJsonWebKey2020(verificationMethod)
}

if (isMultikey(verificationMethod)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We also have this in keyDidMapping (a higher level), so i don't know when it'll reach this, but i see we have the same duplication for JsonWebKey2020 so it's okay for now.

We can refactor it a bit later on

)
}

return Key.fromFingerprint(verificationMethod.publicKeyMultibase)
Copy link
Contributor

Choose a reason for hiding this comment

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

fingerprint was supposed to give a fingerprint of the key, and it also is a multibase encoding. Might be good to add a toMultibase and fromMultibase options and also support e.g. base64 encoding as described in the di spec. But fine for now

@TimoGlastra TimoGlastra merged commit 5562cb1 into openwallet-foundation:main Jan 31, 2024
7 checks passed
@TimoGlastra
Copy link
Contributor

Thanks 🙏

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