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

Improve BigInts UX #280

Closed
AlicanC opened this issue May 17, 2022 · 2 comments · Fixed by #468
Closed

Improve BigInts UX #280

AlicanC opened this issue May 17, 2022 · 2 comments · Fixed by #468
Assignees

Comments

@AlicanC
Copy link
Contributor

AlicanC commented May 17, 2022

We moved from Ethers v5 BigNumbers to native JS BigInts in #258.

While the UX has been generally fine, the native JSON.stringify does not support BigInts so the users might get Do not know how to serialize a BigInt errors, both from their own usage of it (which they could fix) and also from the libraries they use (which they might not be able to).

One example issue is this, where Jest fails to render the actual error if the error involves a BigInt: jestjs/jest#11617 (comment)

(The solution is starting Jest with --maxWorkers 1 which probably disables the thing that can't handle BigInts.)

We should further investigate the UX of native BigInts. If we find them unworthy then we can stop using them in output positions so users never get BigInts while we can keep using them.

(Or maybe we could make the build system polyfill BigInts.)

@luizstacio
Copy link
Member

On SwaySwap bigints prove to be really hard to handle convert parse, etc. And it didn't remove the requirement of having some utilities and also some other big number libs to work: #165 and also https://github.com/FuelLabs/swayswap/blob/master/packages/app/src/systems/Core/utils/math.ts

@LuizAsFight
Copy link
Contributor

nice catch @AlicanC I didn't know it.

as mentioned in Serialize JSON - step 10.
Ref: [GoogleChromeLabs/jsbi/issues/30]

as number type has this max size limitation: Number.MAX_SAFE_INTEGER = 9007199254740991, we cannot use it safely.

bigint was created to address it natively, but still have its problems. I think we should never expose something that can break in a command often used like JSON.stringify (state libraries, save data to local storage, parse data to send to back end, and other use cases). And for internal use, it's not totally supported as some projects may compile for es5 for example, and would need to upgrade to es2020 minimum, which the user might not want.

Instead of going with native number or bigint, I think would be good to use some big number library, plenty of options in npm.

BN.js is a good one:

  • they're used by elliptic curve project.
  • BN.js is

Optimized for elliptic curves that work with 256-bit numbers. There is no limitation on the size of the numbers.

It supports sizes that we need and it's pretty complete with a lot of helpers.

I think we should create a lib that uses BN.js in the background and have some additional helpers that we will need, like some presents in SwaySwap swayswap/math.js.
The user should not need to write all these helpers and deal with ethers in the front end project,

  • parse to units functions (instead of user needing to deal with in the project)
  • basic math helpers (divide always valid ... so on)

When stringifyed it exposes a hex which will work as JSON common string and not break anything
image

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

Successfully merging a pull request may close this issue.

4 participants