Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Works with React Native? #582

Closed
ffxsam opened this issue May 9, 2016 · 19 comments
Closed

Works with React Native? #582

ffxsam opened this issue May 9, 2016 · 19 comments

Comments

@ffxsam
Copy link

ffxsam commented May 9, 2016

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
Copy link
Contributor

dcousens commented May 10, 2016

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

  • Attempt to run the test suite in React Native
  • Investigate React Native's secure random number generation source
  • Run the Buffer test suite

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

@jprichardson
Copy link
Member

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
Copy link

mvayngrib commented May 10, 2016

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
Copy link
Contributor

@mvayngrib that is interesting that they are different.

@mvayngrib
Copy link

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

@mvayngrib
Copy link

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
Copy link
Author

ffxsam commented May 10, 2016

This thread is a gold mine. Thanks!

@dcousens
Copy link
Contributor

dcousens commented Oct 24, 2016

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
Copy link

39otrebla commented Oct 24, 2016

@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
Copy link
Contributor

dcousens commented Oct 24, 2016

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

@dcousens
Copy link
Contributor

@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
Copy link
Contributor

dcousens commented Feb 14, 2017

@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
Copy link

@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
Copy link

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
Copy link
Contributor

dcousens commented Mar 5, 2017

@dominictarr

and a better way would be if

if? 😃

@dominictarr
Copy link

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

@dcousens
Copy link
Contributor

dcousens commented May 16, 2017

@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
Copy link

@dcousens lets do it

@dcousens dcousens reopened this May 16, 2017
@ffxsam ffxsam closed this as completed May 16, 2017
@dcousens
Copy link
Contributor

Closing in favour of #797

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants