Failed to minify the code (during npm run build -- in a Electron/React app)

[bityogi]

We have an Electron app that is boot-strapped using create-react-app.

After referencing this module, the app will not compile anymore:

Failed to compile.

Failed to minify the code from this file: 

 	./node_modules/mymonero-core-js/monero_utils/monero_wallet_utils.js:41 

From react's documentation:

You may occasionally find a package you depend on needs compiled or ships code for a non-browser environment.
This is considered poor practice in the ecosystem and does not have an escape hatch in Create React App.

To resolve this:

Open an issue on the dependency's issue tracker and ask that the package be published pre-compiled (retaining ES6 Modules).

Is this something that you guys could help us out with?

[paulshapiro]

Hi bityogi, I'm not clear on what code from monero_wallet_utils could be considered inappropriate for a non-browser environment... it's working fine for me on both renderer and main processes. However, note that this file has been completely removed on the new branch called cpp. May I request you try it out and let me know what you think? Here is the PR. We'll end up merging it soon.

#41

[bityogi]

Hello @paulshapiro ,

I just tried the cpp branch, and this time the error was different, but nevertheless still complaining on minifying the code.

Failed to minify the code from this file: 

 	./node_modules/mymonero-core-js/monero_utils/monero_cryptonote_utils_instance.js:57 

Read more here: http://bit.ly/2tRViJ9

The link above gives more information from the react-team as to what they are expecting for packages to be built properly.

[paulshapiro]

bityogi, How about using webpack to compile the code to ES5? Here's an example webpack config file where we compile everything to a single module for embedding in another webview.

https://github.com/mymonero/mymonero-app-ios/tree/master/Modules/MyMoneroCore

Have a look at these

https://github.com/mymonero/mymonero-app-ios/blob/master/Modules/MyMoneroCore/bin/package_ios_js

https://github.com/mymonero/mymonero-app-ios/blob/master/Modules/MyMoneroCore/bin/package_ios_js

You could set this repo up as a submodule of your own.

[bityogi]

Hello @paulshapiro,

We bootstrapped our app with create-react-app and that internally handles all the work of webpack.

I can take control over webpack (by what is called ejecting the react-app), however I will lose a lot of ability and features coming from the upstream react-scripts module.

However, I was able to upgrade to a beta version of [email protected] which uses a newer version of Webpack under the covers.

I was able to successfully build the app with "mymonero-core-js" dependency from the "git+https://github.com/mymonero/mymonero-core-js.git#cpp" branch.

But with this code, I get the following error as soon as the application loads:

Uncaught Math.random calls are disallowed (cryptonote_utils) until emscripten has support to override fallback when (!crypto) - see randomFloat_unit() or remove this in fork

[paulshapiro]

I guess you're using Math.random somewhere.

Given that it's not just me who is affected by this, I've gone back to emscripten about it to try to get a resolution. emscripten-core/emscripten#542

[paulshapiro]

Thanks for your patience. By the way if you see a good way to improve the MyMonero core JS code so it doesn't complain under React, please let me know.

[paulshapiro]

For the moment, if you want to, you can get rid of that exception by removing the override of Math.random from cryptonote_utils/cryptonote_utils.js. The reason it's in there is to prevent emscripten from using Math.random as a source of entropy in case it's being run in an environment that doesn't have window.crypto/crypto nor require('crypto'). But it's not really a solution imo.

[paulshapiro]

If it comes down to maintaining a fork of emscripten, the alternative is to replace Math.random() calls in emscripten's generated JS with something like (function(){ throw "…" })(). Should have done that instead.

[bityogi]

Note, that if I reference the master branch, I get this error on start-up.

Not sure, if I get past this somehow that the error above from emscripten will not be there.

Regardless, I am able to package both the master and cpp branch now.

What baffles me is that when I used the older version of react-scripts (and webpack), I wasn't getting the Math.random calls are disallowed (cryptonote_utils) until emscripten has support to override fallback when (!crypto) - see randomFloat_unit() or remove this in fork error.

We were at-least able to use the library and actually create XMR wallets.

[bityogi]

For the moment, if you want to, you can get rid of that exception by removing the override of Math.random from cryptonote_utils/cryptonote_utils.js.

Where exactly is this code in cryptonode_utils.js?

[paulshapiro]

Top - https://github.com/mymonero/mymonero-core-js/blob/cpp/cryptonote_utils/cryptonote_utils.js#L37

[bityogi]

Sorry, I was on the wrong branch (master instead of cpp), so didn't see it.

I've gone ahead and changed the code, and it works!!!

FYI, here is the repository that I'm using this for now: https://github.com/bityogi/mymonero-core-js

Could we leave this issue open till it's either correct in this repo or upstream in emscripten ?

[paulshapiro]

OK, yes I'm still working on this issue.

But I just want to make it clear that you should be extremely careful to verify what environment the random_device function is getting called in for your app – if it's using Math.random, that could mean your users' funds could get stolen.

[bityogi]

Thanks for the heads up. I'll definitely bring it up to the rest of the team and ensure we're using the right environment for the random_device function.

[paulshapiro]

The clearest solution for the moment imo is just to edit MyMoneroCoreCpp.js and replace "Math.random()" with that scoped function I mentioned above. That would let you know right away if anything is accessing it in the wrong env. Talk soon!

[bityogi]

Hello @paulshapiro ,

Regarding Math.random(), we were using the function monero_utils.create_address and providing our own seed. (i.e, we generate the seed ourselves), so we never have a need to use Math.random().

Perhaps, this library does not need to be throwing an error if we are providing our own seed.

The same applies to what I mentioned in #55 ; which I will mention to you on that thread.

[paulshapiro]

@bityogi, no, nothing is ever actually calling Math.random, I am simply PRing to emscripten the removal of the potential to ever call that method. All functions to create addresses or random seeds etc will necessarily flow through emscripten's random_device handler for /dev/random, and will be passed safely into random number generation which we have vetted. If you are generated your seeds in another manner, I recommend that you disclose the method of seed generation to your users. Thank you for your patience as I get a moment to amend the Math.random issue.

[paulshapiro]

By the way, I pushed the Math.random thing recently, so closing this for now until further activity