indutny / bn.js

BigNum in pure javascript
MIT License
1.19k stars 150 forks source link

Easiest way to get percentage #275

Closed aress31 closed 3 years ago

aress31 commented 3 years ago

I am dealing with BN and am trying to get a percentage of a value as follows:

        const senderBalance = await this.token.balanceOf(sender);
        const burnTax = await this.token.getBurnTax();
        const expectedTokenBurnt = burnTax.div(senderBalance).mul(new BN(100));

Not sure where I am going wrong, but it seems that no decimal notation is supported in my above code therefore my test fails with:

  1) Contract: Transfers
       Contract: from an account not excluded from paying fees
         should burn x% of tokens on each transactions:

      AssertionError: expected '950000000000000000000000000000000' to equal '1000000000000000000000000000000000'
      + expected - actual

      -950000000000000000000000000000000
      +1000000000000000000000000000000000

      at Context.<anonymous> (test\ERC20Deflationary.test.js:157:50)
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (internal/process/task_queues.js:93:5)
shuckster commented 3 years ago

Unfortunately BN does not support decimals.

aress31 commented 3 years ago

@shuckster so what a work around could be then?

The numbers that I am retrieving using truffle are all BN and I need to calculate a percentage based on 2 BN.

shuckster commented 3 years ago

Since you seem to be forced into using two different big-number libraries, it may help you to think about separating your data-values from your data-processors.

Enforce a rule where your numbers must always be string, never BN or BigNumber:

const senderBalanceAsBN = await this.token.balanceOf(sender);
const burnTaxAsBN = await this.token.getBurnTax();

const senderBalance = senderBalanceAsBN.toString();
const burnTax = burnTaxAsBN.toString();

const expectedTokenBurnt = BigNumber(burnTax).dividedBy(senderBalance).multipliedBy(100).toString();
const expectedTokenBurntAsBN = new BN(expectedTokenBurnt);

This is what I've tried to show in the example above: The "numbers" are all string. If something is BN, I added *AsBN so I can see that. Both BN and BigNumber have a toString() method, so always use this to keep your data "pure".

You can then freely choose between BN and BigNumber to do your calculations.

aress31 commented 3 years ago

Kind of forced to use BigNumber since BN doesn't support decimal (and therefore percents) will try your solution later and post if it worked.

Thanks again for your help!

aress31 commented 3 years ago

Solved it this way:

        const burnTax = await this.token.getBurnTax();
        const senderBalance = await this.token.balanceOf(sender);

        // TODO: Track ticket https://github.com/indutny/bn.js/issues/275
        const expectedTokenBurnt = BigNumber(burnTax.toString())
          .dividedBy(100)
          .multipliedBy(senderBalance.toString())
          .toFixed();

        const receipt = await this.token.transfer(recipient, senderBalance, {
          from: sender,
        });

        await expectEvent(receipt, "Transfer", {
          from: sender,
          to: recipient,
          value: senderBalance.sub(new BN(expectedTokenBurnt)),
        });

        await expectEvent(receipt, "Burn", {
          from: this.token.address,
          amount: new BN(expectedTokenBurnt),
        });

Any plans in the roadmap to add support for decimals?

shuckster commented 3 years ago

Roadmap? I'm not the author of BN. :)

I'm just looking at this repo because I'm currently interested in big-number libraries. The reason is I'm working on an eslint-plugin that can convert normal JS to BigNumber:

Video of plugin-usage in VSCode

It's not perfect yet, but it's useful. I don't think it would work for your use-case very well, unfortunately.

Nice work fixing your issue. Good call on using toFixed() to avoid the e-syntax!

aress31 commented 3 years ago

Your plugin looks excellent! Dumb and out of the topic question but what are the added benefits or relying on BigNumbers rather than normal JS arithmetic? I had to look into BigNumber only because that's what truffle uses to test smart contract, other than that I don't really have an exposure to that.

shuckster commented 3 years ago

Thanks! The short-answer to why libraries like BN or BigNumber are needed is because of this:

const sum = 0.1 + 0.2
sum === 0.3
// false

sum
// 0.30000000000000004

Doesn't look right, eh? It's complicated to explain why this happens exactly, but it's to do with how floating-point numbers are stored. They have a binary-representation, not decimal. As a mental exercise, try and think of how 0.1 should be represented as a power-of-two, and how you'd go about removing the errors.

Well, since 1985 this has been the standard for FP in computers. It was fine at the time for performance and scientific reasons (where accuracy is not always important) but for finance it's a disaster.

The other reason to use big-number libraries is because default JS has a maximum number size. I notice that truffle is a block-chain library, so probably deals with numbers vastly bigger than what the native arithmetic can handle!

aress31 commented 3 years ago

Thanks for the explanation! Did not know that floating point numbers were subject to such limitations

On Sat, 8 May 2021 at 23:19, Conan Theobald @.***> wrote:

Thanks! The short-answer to why libraries like BN or BigNumber are needed is because of this:

const sum = 0.1 + 0.2sum === 0.3// false sum// 0.30000000000000004

Doesn't look right, eh? It's complicated to explain why this happens exactly, but it's to do with how floating-point numbers are stored. They have a binary-representation, not decimal. As a mental exercise, try and think of how 0.1 should be represented as a power-of-two, and how you'd go about removing the errors.

Well, since 1985 this has been the standard for FP in computers. It was fine at the time for performance and scientific reasons (where accuracy is not always important) but for finance it's a disaster.

The other reason to use big-number libraries is because default JS has a maximum number size. I notice that truffle is a block-chain library, so probably deals with numbers vastly bigger than what the native arithmetic can handle!

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/indutny/bn.js/issues/275#issuecomment-835543809, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACYQNVQRH5F2GZZGPM2XSRDTMW2HPANCNFSM44KU3LMA .

shuckster commented 3 years ago

You're welcome, and yeah, it's pretty surprising that things still work this way. The standard is IEEE-754 and is used in many languages, not just JavaScript, so it's good to check which kind of FP standard is being used to drive the number-types of your preferred languages.

Integers are immune to these problems, but of course they have a very low maximum-size, which might not work well for block-chain applications.