holepunchto / hypercore

Hypercore is a secure, distributed append-only log.
https://docs.holepunch.to
MIT License
2.59k stars 182 forks source link

[RFC] Add support for async crypto libraries #198

Closed alexindigo closed 2 years ago

alexindigo commented 5 years ago

@mafintosh We started conversation on Gitter, adding more details here

For the environments like React Native (or workers in the browser or Node) it's useful to run crypto ops on a separate thread, so it won't block UI. For that we need to be able to use sodium methods in async manner.

In this repo I made it work with async sodium and all tests pass without modifications: https://github.com/alexindigo/react-native-hypercore/commit/6f6b977c302d23c7a41bf5993d9d7250e1da55c0

I added hypercore-crypto and hypercore-protocol into the repo, as their API has changed, so it will be easier to reason about.

Although it's based of 6.22.4 and I need to catch up with the latest developments, it illustrates approach I taken, so we can start discussion.

Thank you.

martinheidegger commented 5 years ago

I don't see how supporting async crypto would make hypercore worse, as long as backwards compatibility can be guaranteed I think this is a good idea.

emilbayes commented 5 years ago

I also worked on this problem with hardware tokens in mind. Here is what I did so far: https://github.com/mafintosh/hypercore/compare/master...emilbayes:pluggable-signatures. You can ignore the stuff in lib/storage.js as that's a different use case than async crypto operations

yoshuawuyts commented 5 years ago

Even better would be if this could also support parallelization, although it might be out of scope for initial patches. I wrote about a parallelization scheme for merkle-tree-stream here.

fsteff commented 5 years ago

I can also imagine a scenario where Dat is running in the browser and crypto stuff runs on a web worker. As long as it doesn't introduce too much new complexity, I think it is a good idea.

alexindigo commented 5 years ago

Thanks for the feedback everybody.

@martinheidegger It does break backwards compatibility for hypercore-crypto and hypercore-protocol, not sure if that modules are used as standalone entities outside of hypercore. And to preserve hypercore API as much as possible, I had to utilize async/await in a lot of places, so I'd like that part to be evaluated too.

And in the long term, I think it would be good idea to embrace the async nature of this kind of operations rather than hide it.

Sounds like @yoshuawuyts, @emilbayes and @fsteff already have bunch of use cases in mind.

martinheidegger commented 5 years ago

@alexindigo I looked over the code. It would be good if you could fork both hypercore-crypto and hypercore-protocol to have a look at the changes necessary in those parts.

Aside from that it seems also not clear what I meant: I meant if the change influences the use of hypercore. It seems like it could be used just like before.

alexindigo commented 5 years ago

@martinheidegger I tried to keep hypercore's API the same, but both hypercore-crypto and hypercore-protocol had to changes their methods to be async. If hypercore is the main consumer it shouldn't be a problem, but if there are other consumers, I'm not sure what would be the best way forward.

I will fork and create PRs for both libraries.

mafintosh commented 2 years ago

10 uses a lot of new crypto abstractions, including async ones.