paulmillr / noble-secp256k1

Fastest 4KB JS implementation of secp256k1 signatures and ECDH
https://paulmillr.com/noble
MIT License
757 stars 114 forks source link

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

Closed uidevkarthick closed 2 years ago

uidevkarthick commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

paulmillr commented 2 years ago

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 https://github.com/facebook/metro/issues/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 commented 2 years ago

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. :slightly_smiling_face: (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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

landabaso commented 2 years ago

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 commented 2 years ago

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

steveluscher commented 2 years ago

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 commented 2 years ago

@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: https://github.com/facebook/metro/issues/359#issuecomment-1237682140

steveluscher commented 2 years ago

Yep! I know. I already have a PR going on against https://github.com/facebook/react-native/pull/34589

I also updated the Solana Cookbook to this effect: https://github.com/solana-developers/solana-cookbook/pull/435

paulmillr commented 2 years ago

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

GabiDev45 commented 1 year ago

Any solution for this issue has been found yet ?

paulmillr commented 1 year ago

@GabiDev45 upgrade to latest react native

VGabriel45 commented 1 year ago

@GabiDev45 upgrade to latest react native

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

horatiubran commented 1 year ago

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

paulmillr commented 1 year ago

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

KholdStare commented 1 year ago

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