mattrglobal / bls12381-key-pair

A library for using BLS 12-381 key pairs
Apache License 2.0
15 stars 10 forks source link

implement fromFingerprint #14

Closed OR13 closed 4 years ago

OR13 commented 4 years ago

needed for did key, here is an example:

https://github.com/transmute-industries/did-key.js/blob/master/packages/x25519/src/X25519KeyPair.ts#L64

OR13 commented 4 years ago

Feel free to copy and paste: https://github.com/transmute-industries/did-key.js/blob/master/packages/bls12381/src/Bls12381G2KeyPair.ts#L4

although I do like that your implementation uses constants.

tplooker commented 4 years ago

Thanks, good addition, should the function signature be fromFingerprint(fingerprint: string) or from({ fingerprint: string }: any) as currently it appears to be doubling up on semantics i.e the call would be const keyPair = Bls1238162.fromFingerprint ({ fingerprint : "z......." })?

OR13 commented 4 years ago

If should be like this: https://github.com/digitalbazaar/crypto-ld/blob/master/lib/LDKeyPair.js#L128

OR13 commented 4 years ago

this function is in turn relied on in did-key.... like so:

https://github.com/digitalbazaar/did-method-key-js/blob/master/lib/driver.js#L43

tplooker commented 4 years ago

Ok we will adhere to this interface for now but will perhaps revisit it down the track because of the semantic double up explained above.

OR13 commented 4 years ago

@tplooker We probably need an interface for LDKeyPair in TypeScript... open to collaboration on that....

I had similar challenges with this libraries use of buffers, and not leveraging base58btc... while I agree with what you are doing, I found the difference slightly frustrating.

a standard interface for these things would help us all :)

tplooker commented 4 years ago

@OR13 your implementation appears to not feed in the id and controller, instead it assumes they are did key, should the function signature be extended to be const keyPair = Bls1238162.fromFingerprint ({ id: "did:key:z", controller: "did:key:.....", fingerprint : "z......." }) should I open an issue on the did key implementation?

OR13 commented 4 years ago

AFAIK, did:key is the only consumer of that kind of fingerprint, and in my implementation it's assumed. I would not change the signature of that static method, because its basically meant to be the same as https://github.com/digitalbazaar/crypto-ld/blob/master/lib/LDKeyPair.js#L128

I agree its bit confusing though... since the way the fingerprint is calculated is basically what did:key is, but.... look, it shows up in these other classes that use base58 encoded keys but are not explicitly "did:key".... this is what lead me to say.... "why are they not?" and collapse them to a single class, and let the user override them if needed without breaking the LDKeyPair and Crypto-LD interfaces.

tplooker commented 4 years ago

Ok we will work with this interface for now and look to revisit it a little later on

tplooker commented 4 years ago

@OR13 this addition should be available in v0.3.1-unstable.046a371 we will roll a release in the next week or so once a few other changes have been made