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

Don't use native numbers for storing arktoshi #2202

Closed
vasild opened this issue Mar 5, 2019 · 2 comments
Closed

Don't use native numbers for storing arktoshi #2202

vasild opened this issue Mar 5, 2019 · 2 comments

Comments

@vasild
Copy link
Contributor

vasild commented Mar 5, 2019

Even though the code uses Bignum for manipulating arktoshi we have Bignum | number | string in the definitions of the classes. This is because the unit tests use fixtures/jsons that use numbers, e.g. amount: 123 instead of amount: "123".

The problem is that the amount of arktoshi could exceed JavaScript's Number.MAX_SAFE_INTEGER, so any chance of the code to accidentally start using native numbers should be nullified. Thus:

Change all Bignum | number | string to Bignum | string and adjust the fixtures.

@faustbrian
Copy link
Contributor

number | string are only present because of tests that still use JSON fixtures so only Bignum should be used if you are going to work on this now.

@faustbrian
Copy link
Contributor

df9fd4d

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

2 participants