ethers-io / ethers.js

Complete Ethereum library and wallet implementation in JavaScript.
https://ethers.org/
MIT License
7.96k stars 1.85k forks source link

React Native : Wallet.createRandom()._mnemonic(); function took 5 min for Android 6.0.1 #1503

Closed ashishonmobile closed 2 years ago

ashishonmobile commented 3 years ago

Describe the bug I am working latest application for using ethers.io for my react native application and this should work on major android and ios

Reproduction steps Just create a release build and use the function inside your code

Wallet.createRandom()._mnemonic();

Environment: Android - v6.0.1 Kernal 3.10.49

Regards, Ashish

zemse commented 3 years ago

I've seen many devs facing slow performance on React Native with crypto utils. You can see: https://github.com/ethers-io/ethers.js/issues/946, https://github.com/ethers-io/ethers.js/issues/390#issuecomment-491759810, #323, #287, #210, #49.

Can you try the solution mentioned in https://github.com/ethers-io/ethers.js/issues/612#issuecomment-535617874

mirceanis commented 3 years ago

I'm tracing the slowness to pbkdf2 which seems to be blazingly slow on android (in react-native). I imagine that in react-native it's rerouted to a JS implementation that is much slower than a native one. I haven't figured out yet how this rerouting happens to be able to suggest a workaround.

ashishonmobile commented 3 years ago

Best to use web3j and web3swift.

Regards, Ashish

On Tue, 8 Jun 2021 at 14:53, Mircea Nistor @.***> wrote:

I'm tracing the slowness to pbkdf2 which seems to be blazingly slow on android (in react-native). I imagine that in react-native it's rerouted to a JS implementation that is much slower than a native one. I haven't figured out yet how this rerouting happens to be able to suggest a workaround.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ethers-io/ethers.js/issues/1503#issuecomment-856609854, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6CBG77RZWP2IDXRDIGYTLTRXORBANCNFSM43O7PTLQ .

-- [image: photo] Ashish Kumar Patel Founder & CEO, APPXBUILD TECHNOLOGIES PVT LTD

+91-98606-77800 | @.***

http://www.appxbuild.com B401, Shree Ganesh Apartment, Kotwal Nagar , Khamla Sq. Nagpur http://facebook.com/ashish%5C.onmobile http://us.linkedin.com/in/ashishkumarpatel http://twitter.com/ashishonmobile Create your own email signature https://www.wisestamp.com/create-own-signature/?utm_source=promotion&utm_medium=signature&utm_campaign=create_your_own&srcid=

mirceanis commented 3 years ago

yeah.. sure.. but that's not the point.

Wallet.createRandom() will derive keys from a mnemonic that is based on a generated seed. Deriving keys from a mnemonic is the bit that takes super long because of pbkdf2. If you just need the mnemonic, then it should be easy to generate some randomness and then call 'entropyToMnemonic' from the hdnode package.

zemse commented 3 years ago

Are you sure pbkdf2 is used in Wallet.createRandom()? Can you point me to it if it's the case. AFAIK the kdfs are only used while keystore encrypting and ethers uses scrypt by default.

mirceanis commented 3 years ago

yeah, it calls Wallet.fromMnemonic() which calls HDNode.fromMnemonic() which at some point calls mnemonicToSeed() which uses pbkdf2

manoj398 commented 3 years ago

@ricmoo

There might be a way to create react-native wrappers in js around hardware backed native modules like https://developer.android.com/reference/android/security/keystore/KeyGenParameterSpec.html#example:-nist-p-256-ec-key-pair-for-signingverification-using-ecdsa (something similar should be available on iOS) which would make wallet generation blazing fast

Another possible solution would be using https://github.com/web3j/web3j/blob/f2d4d291f8f9fa1850df2bc90b8f468edef9f9bb/crypto/src/main/java/org/web3j/crypto/MnemonicUtils.java#L137 these lines as mnemonicToSeed() native module in react native wrapper hooked into ethers.js's HDNode.fromSeed(...).

kaxline commented 3 years ago

I'm getting 2-3 minutes of UI blocking processing on this as well. RN 0.65.1. It's pretty brutal form a UX perspective. Any help on this front would be greatly appreciated.

ashishonmobile commented 3 years ago

It is better to use native library for android and iOS.

On Sun, 19 Sep 2021 at 21:44, Keith Axline @.***> wrote:

I'm getting 2-3 minutes of UI blocking processing on this as well. RN 0.65.1. It's pretty brutal form a UX perspective. Any help on this front would be greatly appreciated.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ethers-io/ethers.js/issues/1503#issuecomment-922498464, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6CBG3WX2R5HCJ2SXZRUXDUCYD55ANCNFSM43O7PTLQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- [image: photo] Ashish Kumar Patel Founder & CEO, APPXBUILD TECHNOLOGIES PVT LTD

+91-98606-77800 | @.***

http://www.appxbuild.com B401, Shree Ganesh Apartment, Kotwal Nagar , Khamla Sq. Nagpur http://facebook.com/ashish%5C.onmobile http://us.linkedin.com/in/ashishkumarpatel http://twitter.com/ashishonmobile Create your own email signature https://www.wisestamp.com/create-own-signature/?utm_source=promotion&utm_medium=signature&utm_campaign=create_your_own&srcid=

ricmoo commented 3 years ago

I’ve heard similar complaints before. Is there any way to know what the time consuming operation is?

I do not have access to any android devices for testing (and on iOS it is fast, so I cannot reproduce it). The only operation off the top of my head is sha2-512, which shouldn’t be particularly slow on android.

I want to put a way to swap out some of these low-level operations, but it does pose a security threat, since many of these operations are passed sensitive data, so it someone shimmed them, it could leak sensitive data.

Does the UI lock up? Or do things like scrolling still function properly while it is creating a new wallet?

One other option is to forego a wallet based on a mnemonic, if you don’t need that functionality, using new Wallet(utils.random ytes(32)).

mirceanis commented 3 years ago

It's the pbkdf2 call that's causing issues. It is purposely slow, like any good password derivation function, but the implementation in JS makes it way slower than it should be for UI purposes. I guess the JS implementation could be optimized a bit, but that might still be very slow (and since it's JS, also blocking UI). If you're looking for something to be swapped, I would go directly for pbkdf2. I believe there are native implementations of that available on Android. I suppose shimming is a concern in any situation, not just this particular method.

kaxline commented 3 years ago

@ricmoo Yeah, the UI does lockup during wallet creation, unfortunately. I understand the thorniness of the issue, so I appreciate whatever can be done. Thanks for looking into it!

mrousavy commented 3 years ago

Is there a way to speed pbkdf2 up? I can create fast C++ / JSI bindings for a native C++ pbkdf2 implementation, assuming there's an API to swap the implementations in this library.

Waiting 5 Minutes seems very long to create a new Wallet...

mrousavy commented 3 years ago

@zemse / @klaop57 how does that solution at https://github.com/ethers-io/ethers.js/issues/612#issuecomment-535617874 look like without expo-random?

kaxline commented 3 years ago

I created a NativeModule that I can call into from React Native:

https://gist.github.com/kaxline/99918c139c9d53cbe8d59a288ae164e8

Seems to work pretty well but I'd love some crypto eyes on it to see if I'm making any subtle mistakes.

If you use that Module you'll need to add Web3j as a dependency in app/build.gradle. Happy to help or share the end to end solution.

sandys commented 2 years ago

@mrousavy @kaxline is ur patch open source ? if not, it will be super awesome if it was possible for u to check with https://github.com/tectiv3/react-native-aes and see if it works just as good.

that one is relatively audited and has a bunch of wallets using it in production

kaxline commented 2 years ago

@sandys Yes, it's open source! Feel free to use as you see fit.

ricmoo commented 2 years ago

@sandys I might be bugging you for some v6 help in the future. In v6, there is a sha256.register(sha256Func), for example that can replace the underlying functions. There is also a lock function, that once called, prevents any further calls to any of the crypto register functions.

So in a react shim, all the crypto functions (like scrypt, sha256, pbkdf2, etc) can be replaced and then call lock to prevent modifications. :)

ricmoo commented 2 years ago

This is a known issue, and I'm trying to burn down the issue backlog so I can focus on v6 (which will address this problem), so I'm going to close this now.

Thanks! :)

modenero commented 2 years ago

Greetings all,

I realize that everyone is in the same boat with the "glacier-like slowness" during Android wallet creation. However, based on the discussion here, I've just tested new ethers.Wallet(privateKey) and it's basically instant on a "physical" Android device. I suppose pbkdf2 is using JS as opposed to native as others have suggested.

I would like to ask if anyone knows how to prevent the UI from locking for 5+ minutes during that FIRST wallet creation using Wallet.fromMnemonic(mnemonic)? That seems to be the biggest issue for me at the moment.

As a last resort, I'll just open a Lottie-animated modal that says, "Wait for it..."

Cheers!

mirceanis commented 2 years ago

@modenero new ethers.Wallet(privateKey) does not use pbkdf2, which is why it is relatively instant. Wallet.fromMnemonic(mnemonic) DOES use pbkdf2 so it grinds the device to a halt using the JS implementation.

I'm not aware of a faster JS implementation for it (but that does not mean there isn't one), so the solution I see is for ethersjs to allow the dev to replace pbkdf2 with a different implementation during initialization. On mobile, this implementation would be a native one, which would be fast enough... see @ricmoo's answer above

Putting up a waiting animation for 5 minutes is a UX nightmare, so my suggestion would be to use a fully native implementation for key derivation from mnemonic and then to use the resulting private key with ethers.js... at least until replacing things gets resolved, likely after the ethers.js 6.0 release.

modenero commented 2 years ago

see @ricmoo's answer above

ok, I read that before but didn't really understand its implications. but now that I better understand the problem, it makes perfect sense.

my immediate purpose was to find a solution for this event (https://moralis.io/avalanche-hackathon/). for now, i'm ok with just using a private key, but for the final release, I definitely want bip-39 (seed words) support.

no idea when v6 will release, so for now I'm leaning towards forking the repo and replacing the slow pbkdf2 with a native solution. @mirceanis @ricmoo any suggestions as to what may serve as a good candidate for such a thing BEFORE then next release? 🤔

thanks!

edit: @mirceanis i'm sorry, i missed this link on my first pass. @ricmoo could you comment on this (albeit temporary) solution?

ricmoo commented 2 years ago

You won’t need to fork the library. You could just sub-class Wallet. :)

I have more time now, and have 2-3 things on my list, one of which is getting the v6 beta out. :)

modenero commented 2 years ago

You won’t need to fork the library.

yeah, i was more thinking that if could figure out a reasonable solution, it would be one less thing for you to do. i feel the need to give back to this library i've been using since v3 (i think).

You could just sub-class Wallet. :)

okay, thanks for the suggestion

I have more time now

so jealous, hahaha

one of which is getting the v6 beta out

definitely no rush from me, but do you have a "rough" eta when we can start testing v6. in my case, I'm working on an "at your own risk" mvp, so I'm not as concerned about things breaking (then fixed), just NO LOSS OF FUNDS.

really appreciate you @ricmoo

mertcankose commented 2 years ago

You won’t need to fork the library. You could just sub-class Wallet. :)

I have more time now, and have 2-3 things on my list, one of which is getting the v6 beta out. :)

We're counting on you <3