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

Migrate code to use the native BigInt type wherever possible. #607

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Shaptic
Copy link
Contributor

@Shaptic Shaptic commented May 15, 2023

What

Minimizes usage of the external dependency BigNumber, preferring the native BigInt.

Why

Per @orbitlens' in stellar/js-stellar-sdk#792 (comment), this is an attempt to minimize usage of the bignumber.js external dependency.

Unfortunately, a full replacement of the library is not possible for a key reason: we need big rationals for price management. Anything to do with offers supports passing real values as strings (that may exceed number) and we need to convert those to fractions for the XDR. This necessitates big rationals, which BigInt does not support, obviously. Until there's also a native BigRat, we will need bignumber.js in these (not really) niche cases.

Known Limitations

Honestly, I'm not certain about the utility of this change given its potential bug surface. Since we still do need bignumber.js for big rational values, and since this is imported by operation.js, this will still need to be used by a giant portion of downstream consumers. (The number of people using the SDK but not using Operation is probably minuscule.)

@Shaptic Shaptic requested a review from paulbellamy May 15, 2023 21:08
@orbitlens
Copy link
Contributor

@Shaptic Please do not merge it so far. It's possible to calculate rational prices using bigints as well, so we'll be able to say farewell to the BigNumber.js dependency. Need to play around with this. We can express the rational price as string, right?
I'll get back to you with the solution for this.

@orbitlens
Copy link
Contributor

orbitlens commented Jun 14, 2023

@Shaptic Ok, here is the solution:

/**
* Divide two numbers with 20 digits precision (result is floored)
* @param {String|Number|BigInt} a
* @param {String|Number|BigInt} b
* @return {String}
**/
function div(a, b) {
    const val = (BigInt(a) * (10n**20n) / BigInt(b)).toString().padStart(20, '0')
    const int = val.substring(0, val.length - 20) || '0'
    const fract =  val.substring(val.length - 20)
    return int + '.' + fract
}

It behaves the same as BigNumber.js with the only difference that the result is always floored in contrast to BigNumber, where the result is rounded by default. But I guess this doesn't matter with 20-digits fractional part. The precision can be easily doubled if needed.

new BigNumber(2).div(3).toString()      // '0.66666666666666666667'
div(2, 3)                               // '0.66666666666666666666'
new BigNumber(2).div(300000).toString() // '0.00000666666666666667'
div(2, 300000)                          // '0.00000666666666666666'
new BigNumber(200000).div(3).toString() // '66666.66666666666666666667'
div(200000, 3)                          // '66666.66666666666666666666'

('200000'/'3').toString()               // '66666.66666666667' - regular Numbers division with limited precision

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

3 participants