Skip to content

fix: (cherry-pick)(Version v12.1.0) Error: new BigNumber() more than 15 digits feat: apply ellipsis #25741 #25804

Merged
cryptotavares merged 1 commit intoVersion-v12.1.0from
Version-v12.1.0-feat-permit-simulation-ellipsis-format-2
Jul 17, 2024
Merged

fix: (cherry-pick)(Version v12.1.0) Error: new BigNumber() more than 15 digits feat: apply ellipsis #25741 #25804
cryptotavares merged 1 commit intoVersion-v12.1.0from
Version-v12.1.0-feat-permit-simulation-ellipsis-format-2

Conversation

@digiwand
Copy link
Copy Markdown
Contributor

Description

Cherry-pick #25741 for Version-v12.1.0

Open in GitHub Codespaces

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2730 (ellipsis)
Fixes: #25751 (Error)

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

…25741)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

Plan is to cherry-pick this PR for V12.1
- Adds ellipsis to Permit value
- Fixes error:
```
BigNumber Error: new BigNumber() number type has more than 15 significant digits
```

Note that the precision is lost through formatAmount. Fix will be added
#25755

Most file updates are snapshot updates

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25741?quickstart=1)

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2730
(ellipsis)
Fixes: #25751
(Error)

- `JSON.parse` coerces number values into scientific notation if they
are greater than Number.MAX_SAFE_INTEGER, which is the max safe integer
in JavaScript.
e.g. 30001231231212312138768 → 3.0001231231212312e+22
- `JSON.parse` reviver cannot help since the value will be coerced by
the time it
 reaches the reviver function.
- We preserve the value by extracting the value with a regex and
overwriting the `JSON.parse` value. The preserved value should never be
converted to a JavaScript number since it will lose its precision.
Instead, we can preserve the value as a string, BigNumber, or Numeric
type. BigInt also has rounding limitations.
- BigNumber errors out when constructed with numbers with 15+ digits. It
can accept strings to preserve digits.
- formatAmount in
`ui/pages/confirmations/components/simulation-details/formatAmount.ts`
utilizes Intl.NumberFormat, which requires a BigInt or number passed to
it causing a loss in precision when computing large numbers. It does not
accept strings. Currently, it will display 0s for the digits after 15
digits.
e.g. 30001231231212312138768 → 30,001,231,231,212,312,000,000
Issue ticket to address this
#25755

1. run `yarn storybook`
2. go to
http://localhost:6006/?path=/story/pages-confirmations-confirm-signatures-signedtypeddatav3orv4--permit-story&args=msgParams.data:3.0001231231212312e+22
3. copy and paste the following into data
```
"{\"types\":{\"EIP712Domain\":[{\"name\":\"name\",\"type\":\"string\"},{\"name\":\"version\",\"type\":\"string\"},{\"name\":\"chainId\",\"type\":\"uint256\"},{\"name\":\"verifyingContract\",\"type\":\"address\"}],\"Permit\":[{\"name\":\"owner\",\"type\":\"address\"},{\"name\":\"spender\",\"type\":\"address\"},{\"name\":\"value\",\"type\":\"uint256\"},{\"name\":\"nonce\",\"type\":\"uint256\"},{\"name\":\"deadline\",\"type\":\"uint256\"}]},\"primaryType\":\"Permit\",\"domain\":{\"name\":\"MyToken\",\"version\":\"1\",\"verifyingContract\":\"0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC\",\"chainId\":1},\"message\":{\"owner\":\"0x935e73edb9ff52e23bac7f7e043a1ecd06d05477\",\"spender\":\"0x5B38Da6a701c568545dCfcB03FcB875f56beddC4\",\"value\":30001231231212312138768,\"nonce\":0,\"deadline\":50000000000}}"
```

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

![CleanShot 2024-07-10 at 01 14
52](https://github.com/MetaMask/metamask-extension/assets/20778143/7a7ff9e3-07a9-4a3b-b8f4-7f672998cdab)

![CleanShot 2024-07-11 at 00 37
25@2x](https://github.com/MetaMask/metamask-extension/assets/20778143/df6b1f0e-fa12-4b6e-9ae1-0486feb7571a)
![CleanShot 2024-07-11 at 00 37
37@2x](https://github.com/MetaMask/metamask-extension/assets/20778143/faffb8e0-6eae-4ef9-8f3b-c399b7f9f7ae)

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
@digiwand digiwand added the team-confirmations Push issues to confirmations team label Jul 12, 2024
@digiwand digiwand requested review from a team as code owners July 12, 2024 14:25
@github-actions
Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [c9dbb7d]
Page Load Metrics (149 ± 168 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint641571002110
domContentLoaded95626136
load391667149349168
domInteractive95626136
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 3.5 KiB (0.05%)
  • common: 276 Bytes (0.00%)

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 12, 2024

Codecov Report

Attention: Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.

Project coverage is 69.58%. Comparing base (ce982e3) to head (c9dbb7d).

Files Patch % Lines
shared/modules/transaction.utils.ts 90.91% 1 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                 @@
##           Version-v12.1.0   #25804      +/-   ##
===================================================
+ Coverage            69.57%   69.58%   +0.01%     
===================================================
  Files                 1360     1360              
  Lines                48200    48215      +15     
  Branches             13306    13311       +5     
===================================================
+ Hits                 33535    33550      +15     
  Misses               14665    14665              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@digiwand digiwand added type-bug Something isn't working release-12.1.0 Issue or pull request that will be included in release 12.1.0 release-blocker This bug is blocking the next release and removed type-bug Something isn't working labels Jul 12, 2024
@gauthierpetetin gauthierpetetin added the type-bug Something isn't working label Jul 17, 2024
@cryptotavares cryptotavares merged commit 50f347c into Version-v12.1.0 Jul 17, 2024
@cryptotavares cryptotavares deleted the Version-v12.1.0-feat-permit-simulation-ellipsis-format-2 branch July 17, 2024 11:12
@github-actions github-actions bot locked and limited conversation to collaborators Jul 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

regression-RC-12.1.0 release-12.1.0 Issue or pull request that will be included in release 12.1.0 release-blocker This bug is blocking the next release team-confirmations Push issues to confirmations team type-bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants