ssbc / ssb-keys

keyfile operations for ssb
36 stars 26 forks source link

asynchronous methods #5

Closed ghost closed 5 years ago

ghost commented 9 years ago

I want to build a version of this module that will work with keyboot, but the underlying browser crypto methods are all asynchronous and the messages travel over an asynchronous postMessage rpc bridge too.

However, the current interface is synchronous.

pfrazee commented 9 years ago

Ok. I suggest we take the pattern used by create and load, where we make all of the functions async, and then implement *Sync versions for each.

pfrazee commented 9 years ago

@substack would you like me to make that change before you port to the browser methods?

dominictarr commented 9 years ago

there will be a bit we'll need to change here, but this is probably a good idea, because then web crypto will work, which will mean we can run efficiently in the browser.

jtremback commented 9 years ago

Was thinking of using this pattern:

exports.generate = function () {
  return keysToBase64(ecc.restore(curve, crypto.randomBytes(32)))
}

becomes

exports.generate = function (callback) {
  process.nextTick(function () {
    callback(null, keysToBase64(ecc.restore(curve, crypto.randomBytes(32))))
  })
}
dominictarr commented 9 years ago

@jtremback should leave a space for the error arg, even if your thing won't error. that is way better than breaking a very standard pattern.

jtremback commented 9 years ago

oops, typo

pfrazee commented 9 years ago

@jtremback That looks good to me! Would you mind also keeping the sync versions with Sync at the end?

dominictarr commented 9 years ago

it might be easiest to fallback to sync version if no cb is provided, during migration to async version... maybe just log the sync version... then we can upgrade everything while keeping it all working.

jtremback commented 9 years ago

Have not had much time recently! I hope to get to this on the weekend.

ghost commented 9 years ago

An sync version seems like a bad idea since only the node version will work with code written against sync methods. This will split the ecosystem and create extra work.

jtremback commented 9 years ago

Did you mean "a sync version" or "an async version"? Either way, https://github.com/jtremback/microstar-crypto may be of interest to you. It wraps tweetnacl, concatenating some keys to make them easier to handle and forces everything to be async. I'll probably add algorithm tagging at some point but it's not a priority right now.

dominictarr commented 9 years ago

yeah, this will require rewriting the validation in secure-scuttlebutt and a few other places - it's still last call on the webcrypto api so I asked them why not just make the api sync.

dominictarr commented 9 years ago

that said, it will be easier to support async than to wait for the webcrypto api to change.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.