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

Use @noble/hashes #1188

Merged
merged 40 commits into from
Nov 24, 2021
Merged

Use @noble/hashes #1188

merged 40 commits into from
Nov 24, 2021

Conversation

jacogr
Copy link
Member

@jacogr jacogr commented Nov 15, 2021

This requires BigInt ... so we are slightly held hostage by older versions of RN.

@Tbaut Do you know the state of BigInt support on RN? i.e. do these work in iOs/Android?

// test 1 (syntax)
const a = 1n + 2n;

// test 2 (does it have BigInt?)
const b = BigInt(1) + BigInt(2);

Trying to figure out what to do with this, i.e.

  • specific (old-code) overrides for react-native (like we do elsewhere, RN has an own export & tested path)
  • only allow react-native to use the WASM path (only caveat is crypto await before using)
  • revisit every now and again until support appears and then go ahead here
  • other?

I have been searching and it seem that iOs is probably less problematic than Android (which may be on a very old version of JSC). Then again, the info I could find has been very scarce.

@jacogr jacogr added the WIP Work in Progress label Nov 15, 2021
@jacogr jacogr mentioned this pull request Nov 15, 2021
@jacogr jacogr changed the title Use hashing functions from @noble/hashes Use @noble/hashes Nov 15, 2021
@Tbaut
Copy link
Contributor

Tbaut commented Nov 19, 2021

Sorry about the late reply, I'm afk. Will try it out within the next week and let you know.

@jacogr
Copy link
Member Author

jacogr commented Nov 19, 2021

The shims won’t do the trick at all. The issue is that it cannot polyfill stuff like 1n + 2n. In this case it is also a 3rd party that introduces it, so cannot use the Google polyfill.

But anyway, let‘s see. I think iOS has support, but android with JSC is always lagging.

So the current approach here is to detect if BigInt is available in the environment, i.e. typeof BigInt !== 'undefined' - if it is not available, it will just always use the WASM path.

@jacogr
Copy link
Member Author

jacogr commented Nov 22, 2021

@Tbaut https://blog.logrocket.com/whats-new-in-react-native-0-65/ - it lists BigInt.prototype.toLocaleString under the Android updates.

@jacogr jacogr removed the WIP Work in Progress label Nov 24, 2021
@jacogr
Copy link
Member Author

jacogr commented Nov 24, 2021

Following the approach with fallbacks when typeof BigInt === 'undefined'

@jacogr jacogr merged commit 6688ff0 into master Nov 24, 2021
@jacogr jacogr deleted the jg-noble-hashes branch November 24, 2021 11:42
@polkadot-js-bot
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Nov 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants