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

TypeError: Cannot convert a BigInt value to a number in react native #31

Closed
uidevkarthick opened this issue Nov 10, 2021 · 22 comments
Closed

Comments

@uidevkarthick
Copy link

if i use this npm in react-native, the error will be like TypeError: Cannot convert a BigInt value to a number occurs. even I have tried shim.js added and imported big-int npm but still not working. please clarify this

// my code

import * as secp from "noble-secp256k1";

const clientPrivatekey = Buffer.from("308193020100301306072a8648ce3d020106082a8648ce3d03010704793077020101042094a2fb3f1b5f49d8e523901392d331dbf1fffecda54204f1c8ed36fa753da1e2a00a06082a8648ce3d030107a144034200047c6a5ccaed72b3b274f176bf00d26c80ba4ab3e4c23119339688d73b4b2a0eb3b2dd904b3c13d6299bbd2034b0e44ec95f928876a8c1cd7e86b32a92220d8b23", "base64")
    const servPublicKey = Buffer.from(serverPublicKey, "base64")
const ecdhSecrtKey = secp.getSharedSecret(clientPrivatekey, servPublicKey);
@paulmillr
Copy link
Owner

There is no "shim" for native bigints. You need to use environments which support bigints natively. We don't provide support for transpiled code

@lf94
Copy link

lf94 commented Apr 26, 2022

I'm encountering the same problem. Unfortunately expo / metro can't be stopped easily in this regard.

This is literally the only package in our deps causing this issue. The exact issue is this:

** is being transpiled to Math.pow, which is incorrect, because Math.pow only takes regular number and not BigInt. So even if there is a BigInt polyfill, this does not fix the issue because now the code does Math.pow(BigInt(a), BigInt(b)). You can try this for yourself in nodejs or the browser console: 1n ** 1n is valid, Math.pow(1n, 1n) causes the error.

As a consequence we're going to have to fork the code and make adjustments... Which is not a big deal for us, but it will fragment the usage of this package.

Edit: I would like to point out that you have already done some work for supporting certain execution environments, i.e.

// Be friendly to bad ECMAScript parsers by not using bigint literals like 123n

This change would be along the same lines!

@paulmillr
Copy link
Owner

So, what do you want? Change constants to some shitty computed value? How would you audit them after that? One of the primary reasons for creating the lib was auditability.

This is a bug of bad parsers. Report it to appropriate repos. I've did everything I could to ease the burden. Don't want to do what's unnecessary.

@lf94
Copy link

lf94 commented Apr 26, 2022

I had to do worse actually: eval of a string of the equivalent to **, but it builds and all tests pass.

What I'm suggesting here is maybe keep this second file on the side and add a react-native property to your package.json. I'll open a PR so you can see what I've done. You'll see it's absolutely disgusting but there's no other option for us... I'm on your side that the parsers / transpilers need to step up but that's just the shitty ecosystem we are in. I'm with you that this change makes it much less auditable, if at all.

I would rather work with you on this than against; if this package can just provide a react-native variant, that's more than enough, even if it's shitty.

@lf94
Copy link

lf94 commented Apr 26, 2022

For those who will inevitably find themselves here: https://github.com/paulmillr/noble-secp256k1/pull/60/files

@paulmillr
Copy link
Owner

Thanks for the pull request @lf94. Folks who run into the same problem could totally fork secp and use the solution. As i've mentioned, it uses eval, so be careful with CSP (secure contexts).

Those who want to use this with React Native should report the issue to facebook/metro#359. As soon as they'll get enough exposure, they should do something. Maybe submit pull requests to Metro, and call FB developers via Twitter etc.

I'm just a one guy trying to maintain this library. FB has more than enough resources to adjust their stuff to modern syntax.

@lf94
Copy link

lf94 commented Apr 27, 2022

Yeah I totally understand. Thank you regardless for this excellent work you've done!

Unfortunately that issue is 3 years old (we found that issue early on in our attempts to fix this) and I don't expect anything to happen any time soon... Hence why I've come to you. 🙂 (Edit: I've commented on the issue so at least my vote is cast...)

To anyone else: you can use @alephium/noble-secp256k1 which is explicit about being a fork of this project, and that you should use this project over it if it's possible. It uses the precomputed solution so it's way way faster.

@landabaso
Copy link

landabaso commented Jul 12, 2022

I believe it is not a good idea to fork this library.

It's better to fix the source of the problem (not this lib).

Open this file:

node_modules/metro-react-native-babel-preset/src/configs/main.js

And comment these lines so that the buggy transformer is not used:

+  //if (!isHermes && (isNull || src.indexOf("**") !== -1)) {
+  //  extraPlugins.push([
+  //    require("@babel/plugin-transform-exponentiation-operator"),
+  //  ]);
+  //}

And then run this command to make the patch permanent. npx patch-package metro-react-native-babel-preset

Anyway @lf94 , this will not work on Android anyway, right? react-native on Android does not support BigInt (so far).
Any shim will not work because they won't do operator overloading unless I'm missing something.
From what I read, next react-native 0.7 version will support BigInt.

@LeeAlephium
Copy link

LeeAlephium commented Jul 12, 2022

It will work on Android through various means but in the end it all really sucks. Unfortunately avoiding BigInt is hard when it comes to cryptography. I believe it isn't a good idea either to fork the library but the choices are few and far between...

@paulmillr
Copy link
Owner

Report this to FB. They need to move -- not us.

@landabaso
Copy link

It will work on Android through various means but in the end it all really sucks.

@LeeAlephium I'd be very wary about using noble-secp256k1 with a BigInt polyfill for react-native & Android.

Note that polyfills will give wrong results for this kind of thing: BigInt("big number") + BigInt("another big number").
polyfills do not implement operator overloading. Worse than that, they will fail silently if I'm not mistaken.

Maybe you were thinking in another way to go around BigInt not being available in Android/RN that that I'm not aware?
Anyway I leave this note here in case someone else is tempted to use a polyfill with this library.

@LeeAlephium
Copy link

LeeAlephium commented Jul 13, 2022

I share the sentiment and warning to others. 👍 You must be very careful. I reiterate the whole situation sucks (and not because of noble-secp256k1 but because of FB)...

@steveluscher
Copy link

Help is finally on the way. The combination of React Native 0.70 and Hermes will bring support for bigint and the exponentiation operator. https://twitter.com/steveluscher/status/1563940234005745665

@elliotsayes
Copy link

@steveluscher Even with RN 7.0.0 and hermes, this does not yet work by default. See my comment here on how to activate it:
facebook/metro#359 (comment)

@steveluscher
Copy link

Yep! I know. I already have a PR going on against facebook/react-native#34589

I also updated the Solana Cookbook to this effect: solana-developers/solana-cookbook#435

@paulmillr
Copy link
Owner

75c244b makes sure bigint ** is never used. With this change, and new release, the library now works in React Native 0.70.

@GabiDev45
Copy link

Any solution for this issue has been found yet ?

@paulmillr
Copy link
Owner

@GabiDev45 upgrade to latest react native

@VGabriel45
Copy link

@GabiDev45 upgrade to latest react native

I already have it upgraded, tried everything discussed here, nothing seems to work.

@horatiubran
Copy link

ITS 2023... IS IT JUST ME HAVING THIS ISSUE ?

@paulmillr
Copy link
Owner

@horatiubran yes. 1. update react-native. 2. use noble-curves instead of noble-secp v2

@KholdStare
Copy link

May not be related, but some transpilers like babel will convert things like ** to Math.pow which doesn't work with BigInt. See hirosystems/stacks.js#1096 (comment) . Perhaps the problem is in something else in your setup that is transpiling the code wrong.

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

No branches or pull requests