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

feat: move from BigNumber to BigInt #266

Merged
merged 2 commits into from
May 13, 2022
Merged

feat: move from BigNumber to BigInt #266

merged 2 commits into from
May 13, 2022

Conversation

AlicanC
Copy link
Contributor

@AlicanC AlicanC commented May 12, 2022

This PR:

  • Adds a new package called @fuel-ts/math which has some utilities for working with bigints. This is a copy from Ethers v6 and we can move to their package when that's out of beta.
  • Removes any dependency to @ethersproject/bignumber and uses native bigints instead.
  • Fixes the test for UtxoIdCoder, which was a copy of WitnessCoder that tested Witness :D
  • Upgrades to Jest 28. (27 had issues with bigints) Move to Jest 28 #267

Notes:

  • In the transactions package, instead of changing all BigNumbers to BigInts, I changed the ones lte u32 to just number. They are mostly lengths and indexes and those are natively represtented as numbers and not bigints in JS so this makes our APIs more convenient.

@AlicanC AlicanC self-assigned this May 12, 2022
@AlicanC AlicanC added the feat Issue is a feature label May 12, 2022
@AlicanC AlicanC linked an issue May 12, 2022 that may be closed by this pull request
@AlicanC AlicanC mentioned this pull request May 12, 2022
@AlicanC AlicanC marked this pull request as ready for review May 12, 2022 22:00
Copy link
Member

@luizstacio luizstacio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment

// This is to align the bits to the total bytes
// See https://github.com/FuelLabs/fuel-specs/blob/master/specs/protocol/abi.md#unsigned-integers
length: number;
baseType: string;
baseType: TType;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename it to BaseType?

Copy link
Contributor

@QuinnLee QuinnLee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@AlicanC AlicanC merged commit 7e615a3 into master May 13, 2022
@AlicanC AlicanC deleted the jc/bigint branch May 13, 2022 01:01
@camsjams
Copy link
Contributor

camsjams commented May 13, 2022

@AlicanC Should we add this to the typedoc list?

@AlicanC
Copy link
Contributor Author

AlicanC commented May 13, 2022

@AlicanC Should we add this to the typedoc list?

Yeah, nice catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Issue is a feature
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Move to BigInt
4 participants