digitalbazaar / did-method-key

A did-io driver for the DID "key" method
Other
25 stars 10 forks source link

Return the private key material in the generate API #19

Closed mattcollier closed 3 years ago

mattcollier commented 4 years ago

It appears the private key material is getting dropped on the floor in the generate API. I expect that I would be able to get the LDKeyPair instance returned along with the DID document. Is that an improper expectation?

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

dmitrizagidulin commented 4 years ago

::facepalms:: Er, merely a dumb oversight on my part.

dmitrizagidulin commented 4 years ago

This does change the return signature of .generate(), so it'll be a fairly major breaking change. However, since we just changed the multicodec encoding of X25519 keys, we already have a major change to propagate downstream, so this'll be good.

dmitrizagidulin commented 4 years ago

@mattcollier Would it be ok to return the generated key inside the DidDocument?

The way did-veres-one handles it is - generate() returns a DID Document instance, and all the generated public/private key pairs are available at didDoc.keys property.

We chould change the return signature of did:key driver's generate to be {keyPair, didDoc}, but would it be ok to match the did:v1 driver and just return didDoc, with the key pair being available on didDoc.keys?

mattcollier commented 4 years ago

@dmitrizagidulin having all the various methods have some consistent behavior would be great. Maybe we can discuss for a few moments during today's call.

dmitrizagidulin commented 4 years ago

@mattcollier sure thing. (Also, for a (non-binding) example of that would look like, see https://github.com/digitalbazaar/did-method-key-js/pull/22 )

dmitrizagidulin commented 3 years ago

Implemented in PR #22, closing.