bitcoinjs / bitcoinjs-lib

A javascript Bitcoin library for node.js and browsers.
MIT License
5.68k stars 2.1k forks source link

Works with React Native? #582

Closed ffxsam closed 7 years ago

ffxsam commented 8 years ago

Does anyone know if this library will work with React Native? Theoretically any JS could should, but I wanted to know if it would operate securely, or if there were any caveats.

dcousens commented 8 years ago

That is a good question. I don't have the answer for you, maybe @jprichardson does. However, the steps to testing would be:

If the results of those two three steps are satisfactory, then I think you'd be probably be OK to use this library :+1:

jprichardson commented 8 years ago

I don't have the answer for you, maybe @jprichardson does.

Unfortunately I don't either, I haven't been able to use React Native yet 😞 However @mvayngrib may know.

mvayngrib commented 8 years ago

bitcoinjs-lib works fine in react native. Currently you'll need rn-nodeify, at least until this issue is resolved.

re: random number generation, the async rng uses iOS's SecRandomCopyBytes, the sync one uses a sjcl's rng seed with SecRandomCopyBytes. You'll need to evaluate the security of that for yourself.

dcousens commented 8 years ago

@mvayngrib that is interesting that they are different.

mvayngrib commented 8 years ago

@dcousens yep, javascript only has async access to native functions

mvayngrib commented 8 years ago

you'll need a piece of native code to make that work, but react-native-randombytes basically does those five lines for you: js: https://github.com/mvayngrib/react-native-randombytes/blob/master/index.js#L39 objc: https://github.com/mvayngrib/react-native-randombytes/blob/master/RNRandomBytes.m

ffxsam commented 8 years ago

This thread is a gold mine. Thanks!

dcousens commented 7 years ago

ping @39otrebla, were you having issues with this?

https://github.com/39otrebla/react-native-bitcoinjs-lib implies you had issues, but I can't see any difference except the .gitignore...

39otrebla commented 7 years ago

@dcousens yeah had some issues, randomBytes does not work on ReactNative out of the box, as well as node-dependent packages.. I did need some workarounds, but now it works perfectly

EDIT: differences are in ecpair.js, where I include react native randombytes package instead of node randombytes .. also, things like Buffer and streams does not work on React Native out of the box, you do need to use rn-nodeify.. The README of https://github.com/39otrebla/react-native-bitcoinjs-lib explains all step by step

dcousens commented 7 years ago

@39otrebla can you think of a way we could incorporate support into this library without you needing to maintain/rebase a separate fork?

dcousens commented 7 years ago

@39otrebla any reason why you didn't just pass in your own rng parameter? Or did randombytes as a package fail to import entirely?

dcousens commented 7 years ago

@jprichardson @mvayngrib @calvinmetcalf @dominictarr @feross @fanatid

tl;dr: If we all used require('buffer') around the place we could claim this library, and many others, as react-native compatible. (and the ability to always pass in an RNG)

Thoughts?

It would require quite a few changes upstream to various dependencies.

I know this problem shouldn't be our responsibility, but at the same time it is just a nasty side effect of nodes implicit "non-standard" JS globals.

We took the approach in https://github.com/crypto-browserify to use require('create-hash') instead of require('crypto') as a way to reduce require bloat.

Maybe we can take the same initiative here (except in allowing for just a plug-and-play approach for new tools such as react-native).

edit: actually, maybe disregard, this is just totally not our problem?

mvayngrib commented 7 years ago

@dcousens i think the initiative mostly belongs on the react-native packager end, to enable shimming / resolve-aliasing a la browserify and webpack. I currently use bitcoinjs-lib and other libs with rn-nodeify's hackery, and a bunch of native shims i co-developed with other react native fans. @philikon has developed https://github.com/philikon/ReactNativify, which is a healthier approach, but one I haven't tested yet. I suggested to him recently to try it out on bitcoinjs-lib and he said it worked: https://github.com/mvayngrib/rn-nodeify/issues/25#issuecomment-277853745

also see: https://productpains.com/post/react-native/packager-support-resolvealias-ala-webpack

dominictarr commented 7 years ago

I'd be happy to merge a pr adding explicit require('buffer') to any of my modules, though I think there may be a lot of them, and a better way would be if

dcousens commented 7 years ago

@dominictarr

and a better way would be if

if? :smiley:

dominictarr commented 7 years ago

sorry ... if a pull request was made to react-native to support Buffer

dcousens commented 7 years ago

@dominictarr I'm thinking of re-visiting this one - maybe adding require('buffer') unanimously... Its a tiny change - no other impact - and allows us to get past this.

I know it is not great, but, maybe Buffer as a global was a dumb idea anyway?

dominictarr commented 7 years ago

@dcousens lets do it

dcousens commented 7 years ago

Closing in favour of https://github.com/bitcoinjs/bitcoinjs-lib/issues/797