[Sentry] [Bug] Bug/Error: div() number type has more than 15 significant digits

[hilvmason]

• This is a previously solved bug: “MM_bug_2” but actually appears to have not gone away
• Affected users are reporting this happening upon opening the extension, and also upon executing transactions.
• Not super high volume of inquiries on this one.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.

[github-actions[bot]]

This issue was closed because there has been no follow up activity in the last 45 days. If you feel this was closed in error, please reopen and provide evidence on the latest release of the extension. Thank you for your contributions.

[sentry-io[bot]]

Sentry issue: METAMASK-X885

[gauthierpetetin]

Elevating this to a sev1 as it occurs in calcTokenAmount function and can potentially lead to wrong balances being displayed.

[gauthierpetetin]

Probably not a bug anymore, but we shall investigate why it was solved.

[vpintorico]

Hi team, any update on this one?

[vpintorico]

@MetaMask/swaps-engineers Hi team, can you please confirm this is still happening?

[klejeune]

@vpintorico happened twice in the last 3 months:

• June 20th on 11.16.11
• June 17th on 11.17.10

@infiniteflower @nikoferro I can reproduce with this:

BigNumber.DEBUG = true;
calcTokenAmount(1, 0.0000999999999999999) // "1" is not important, the error is triggered by the second parameter alone

// Error: [BigNumber Error] Number primitive has more than 15 significant digits: 1.0002302850208247

[digiwand]

hi there 👋🏼 I dug into a separate, related issue in the repo (See "Technical Details" here)

I see the issue in calcTokenAmount and am working on a solution

[digiwand]

Q: how may the app pass a fractional value as the decimals for calcTokenAmount?

cc: @klejeune

[digiwand]

cc: @klejeune

It turns out there was a way to cause the error without decimals being fractional. For example, when decimals = 36 it would cause the error reported in Sentry:

BigNumber Error:
div() number type has more than 15 significant digits: 9.999999999999999e+35

Created the fix for this here https://github.com/MetaMask/metamask-extension/pull/25799

[benjisclowder]

Collaborative Effort Required for Root Cause Analysis (RCA) on Critical Issues

We are quickly approaching the end of the quarter and we encourage you once again to take some moments and perform RCA on this critical issue. You may do so by answering the questions below:

1. What PR fixed the issue? 2. Can you pinpoint the commit from which the issue originated? 3. Write a short explanation of the technical cause of the bug 4. How could we have avoided merging this bug? What would have had to be different about our code, tests or processes? 4.1. Were there any missing unit, e2e or manual tests that could have preempted this issue? 4.2. Were there any other elements lacking, such as typed code, comprehensive documentation, well-architected APIs, etc., that might have prevented this issue? 4.3. If your answer to a and b is no, then is there anything at all that you can think of that, if it had been different before this bug was introduced, would have prevented it from being merged?

Please provide your answers as a reply to this comment and tag me as well.

You can read more about the initiative here. Thank you!

Tagging eng. who added the fix: @digiwand

[digiwand]

Hello again @benjisclowder, just sent a related RCA, which was focused on a related precision issue. Backlinking here: (Ref). Please reach out if any of this is unclear and I can update it. Thank you!

1. What PR fixed the issue? https://github.com/MetaMask/metamask-extension/pull/25799

2. Can you pinpoint the commit from which the issue originated? BigNumber was introduced into calcTokenAmount 6 years ago: https://github.com/MetaMask/metamask-extension/commit/79976b702685dd83d356e18fbb01e273e04f2fbf. I think it's possible similar 15 digit errors have occurred since then. Minor and related update to calcTokenAmount: https://github.com/MetaMask/metamask-extension/commit/4c87c05a02d5bf5634234a74910e5d3e559dd413#diff-38dd5378373982fd486d27410ed95f012dcfb238960362b3cb3612bbea1ff2baL110 (Linking this as the filepath for the related util has changed overtime making these commits harder to track.)

3. Write a short explanation of the technical cause of the bug https://github.com/MetaMask/metamask-extension/issues/13738#issuecomment-2229537527:

   It turns out there was a way to cause the error [... e.g.] when decimals = 36 it would cause the error reported in Sentry:

   BigNumber Error: div() number type has more than 15 significant digits: 9.999999999999999e+35

   Please see "Explanation:" in https://github.com/MetaMask/metamask-extension/pull/25799#issue-2405549012 and "Technical Details" in https://github.com/MetaMask/metamask-extension/pull/25741 for further technical details.

4. How could we have avoided merging this bug? What would have had to be different about our code, tests or processes? 4.1. Were there any missing unit, e2e or manual tests that could have preempted this issue? Yes, we could test larger integers and integers that may cause values greater than MAX_SAFE_INTEGER passed to BigNumber. 4.2. Were there any other elements lacking, such as typed code, comprehensive documentation, well-architected APIs, etc., that might have prevented this issue? updating the bignumber.js package from 4.1.0 → 9.1.2 could also fix this issue. bignumber.js has not been updated since the beginning of MM. 4.3. If your answer to a and b is no, then is there anything at all that you can think of that, if it had been different before this bug was introduced, would have prevented it from being merged?