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

Add support for Bls12381G2 keys in did-provider-key #895

Open
pcibraro opened this issue May 24, 2022 · 3 comments
Open

Add support for Bls12381G2 keys in did-provider-key #895

pcibraro opened this issue May 24, 2022 · 3 comments
Labels
enhancement New feature or request pinned don't close this just for being stale

Comments

@pcibraro
Copy link

The existing implementation for did-provider-key only allows creating identifiers for 'Ed25519'. It is not possible to extend it to support other key types such as 'Bls12381G2'.

The only supported key type is hardcoded here. https://github.com/uport-project/veramo/blob/next/packages/did-provider-key/src/key-did-provider.ts#L28

It would be ideal if we can pass the key type as an option for this implementation and also override how the specific portion for the key identifier is generated. For instance, Bls12381G2 uses a different prefix. See here, https://github.com/pcibraro/veramo/blob/next/packages/did-provider-key-bbs/src/key-did-provider.ts#L32

Also, the DIDResolver implementation in the resolver.ts file is hardcoded with a few supported prefixes (for key types) https://github.com/uport-project/veramo/blob/next/packages/did-provider-key/src/resolver.ts#L6. It would be great if that can be extended to support other prefixes like the one used by Bls12381G2.

@pcibraro pcibraro added the enhancement New feature or request label May 24, 2022
@mirceanis
Copy link
Member

Thank you for reporting this!

The BLS curves were removed from the implementations of did:key that we use here because of dependency issues with wasm.
We try to keep the Veramo packages available in this monorepo available on all our target platforms (nodejs, browsers, react-native).

But, Veramo is very modular and this means that platform-specific plugins can be used as well. So if you need to support this new algorithm and there is no pure javascript implementation available, then please fork thie kms-local implementation or add a new KMS implementation specifically for BLS.
Multiple KMS implementations can be used at the same time by @veramo/key-manager.

That being said, I think there are several modifications that have to be done in this monorepo as well, because there may be some assumptions made throughout the code about the types of keys that we work with.

I think you already started to flag some of them, but please feel free to post new issues if something else can be fixed.

@mirceanis
Copy link
Member

It would be ideal if we can pass the key type as an option for this implementation and also override how the specific portion for the key identifier is generated. For instance, Bls12381G2 uses a different prefix. See here, https://github.com/pcibraro/veramo/blob/next/packages/did-provider-key-bbs/src/key-did-provider.ts#L32

I agree, the KeyDIDProvider, should accept a parameter specifying the key type.

When it comes to DIDManager, a way to support multiple types of did:key providers is to name the providers including prefixes, similar to how we use multiple providers for did:ethr in out examples.

new DIDManager({
// ...
    providers: {
      'did:ethr': new EthrDIDProvider({
        defaultKms: 'local',
        network: 'mainnet',
        // ...
      }),
      'did:ethr:rinkeby': new EthrDIDProvider({
        defaultKms: 'local',
        network: 'rinkeby',
        // ...
      }),

// add prefixed did:key providers based on key type
      'did:key:z6Mk': new KeyDIDProvider({
        defaultKms: 'local',
        keyType: 'Ed25519'
      }),
      'did:key:zQ3s': new KeyDIDProvider({
        defaultKms: 'local',
        keyType: 'Secp256k1'
      }),
// And for BLS12381G2 it would be something like this:
      'did:key:zUC7': new KeyDIDProvider({
        defaultKms: 'kms-with-bls-support', // a custom KMS in case kms-local doesn't get the new bells and whistles for G2.
        keyType: 'Bls12381G2'
      }),
    },
  }),

@mirceanis mirceanis added the pinned don't close this just for being stale label Jun 3, 2022
@mirceanis
Copy link
Member

relates to #605

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pinned don't close this just for being stale
Projects
None yet
Development

No branches or pull requests

2 participants