Skip to content

Commit

Permalink
fix: Error: new BigNumber() more than 15 digits feat: apply ellipsis (#…
Browse files Browse the repository at this point in the history
…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: MetaMask/MetaMask-planning#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.
  • Loading branch information
digiwand committed Jul 12, 2024
1 parent ce982e3 commit c9dbb7d
Show file tree
Hide file tree
Showing 18 changed files with 178 additions and 86 deletions.
15 changes: 15 additions & 0 deletions shared/modules/transaction.utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,21 @@ describe('Transaction.utils', function () {
const result = parseTypedDataMessage('{"test": "dummy"}');
expect(result.test).toBe('dummy');
});

it('parses message.value as a string', () => {
const result = parseTypedDataMessage(
'{"test": "dummy", "message": { "value": 3000123} }',
);
expect(result.message.value).toBe('3000123');
});

it('parses message.value such that it does not lose precision', () => {
const result = parseTypedDataMessage(
'{"test": "dummy", "message": { "value": 30001231231212312138768} }',
);
expect(result.message.value).toBe('30001231231212312138768');
});

it('throw error for invalid typedDataMessage', () => {
expect(() => {
parseTypedDataMessage('');
Expand Down
39 changes: 37 additions & 2 deletions shared/modules/transaction.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,5 +282,40 @@ export async function determineTransactionAssetType(
return { assetType: AssetType.native, tokenStandard: TokenStandard.none };
}

export const parseTypedDataMessage = (dataToParse: string) =>
JSON.parse(dataToParse);
const REGEX_MESSAGE_VALUE_LARGE =
/"message"\s*:\s*\{[^}]*"value"\s*:\s*(\d{15,})/u;

function extractLargeMessageValue(dataToParse: string): string | undefined {
if (typeof dataToParse !== 'string') {
return undefined;
}
return dataToParse.match(REGEX_MESSAGE_VALUE_LARGE)?.[1];
}

/**
* JSON.parse has a limitation which coerces values to scientific notation if numbers are greator than
* Number.MAX_SAFE_INTEGER. This can cause a loss in precision.
*
* Aside from precision concerns, if the value returned was a large number greator than 15 digits,
* e.g. 3.000123123123121e+26, passing the value to BigNumber will throw the error:
* Error: new BigNumber() number type has more than 15 significant digits
*
* Note that using JSON.parse reviver cannot help since the value will be coerced by the time it
* reaches the reviver function.
*
* This function has a workaround to extract the large value from the message and replace
* the message value with the string value.
*
* @param dataToParse
* @returns
*/
export const parseTypedDataMessage = (dataToParse: string) => {
const result = JSON.parse(dataToParse);

const messageValue = extractLargeMessageValue(dataToParse);
if (result.message?.value) {
result.message.value = messageValue || String(result.message.value);
}

return result;
};
22 changes: 18 additions & 4 deletions ui/components/app/confirm/info/row/text.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useContext } from 'react';
import { I18nContext } from '../../../../../contexts/i18n';
import {
AlignItems,
BlockSize,
Display,
FlexWrap,
IconColor,
Expand All @@ -10,8 +11,18 @@ import {
import { Box, ButtonIcon, IconName, Text } from '../../../../component-library';
import Tooltip from '../../../../ui/tooltip';

const InfoText = ({ text }: { text: string }) => (
<Text color={TextColor.inherit} style={{ whiteSpace: 'pre-wrap' }}>
const InfoText = ({
isEllipsis,
text,
}: {
isEllipsis: boolean;
text: string;
}) => (
<Text
color={TextColor.inherit}
style={isEllipsis ? {} : { whiteSpace: 'pre-wrap' }}
ellipsis={isEllipsis}
>
{text}
</Text>
);
Expand All @@ -20,12 +31,14 @@ export type ConfirmInfoRowTextProps = {
text: string;
onEditClick?: () => void;
editIconClassName?: string;
isEllipsis?: boolean;
tooltip?: string;
};

export const ConfirmInfoRowText: React.FC<ConfirmInfoRowTextProps> = ({
text,
onEditClick,
isEllipsis = false,
editIconClassName,
tooltip,
}) => {
Expand All @@ -39,6 +52,7 @@ export const ConfirmInfoRowText: React.FC<ConfirmInfoRowTextProps> = ({
alignItems={AlignItems.center}
flexWrap={FlexWrap.Wrap}
gap={2}
minWidth={BlockSize.Zero}
>
{tooltip ? (
<Tooltip
Expand All @@ -47,10 +61,10 @@ export const ConfirmInfoRowText: React.FC<ConfirmInfoRowTextProps> = ({
wrapperStyle={{ minWidth: 0 }}
interactive
>
<InfoText text={text} />
<InfoText isEllipsis={isEllipsis} text={text} />
</Tooltip>
) : (
<InfoText text={text} />
<InfoText isEllipsis={isEllipsis} text={text} />
)}
{isEditable ? (
<ButtonIcon
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ exports[`Info renders info section for personal sign request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down Expand Up @@ -223,7 +223,7 @@ exports[`Info renders info section for typed sign request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down Expand Up @@ -270,7 +270,7 @@ exports[`Info renders info section for typed sign request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down Expand Up @@ -383,7 +383,7 @@ exports[`Info renders info section for typed sign request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down Expand Up @@ -480,7 +480,7 @@ exports[`Info renders info section for typed sign request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ exports[`PersonalSignInfo handle reverse string properly 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down Expand Up @@ -136,7 +136,7 @@ exports[`PersonalSignInfo renders correctly for personal sign request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand All @@ -40,7 +40,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand All @@ -64,7 +64,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down Expand Up @@ -159,7 +159,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand All @@ -183,7 +183,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand All @@ -207,7 +207,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down Expand Up @@ -255,7 +255,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down Expand Up @@ -284,7 +284,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request with resource
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand All @@ -308,7 +308,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request with resource
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand All @@ -332,7 +332,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request with resource
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down Expand Up @@ -427,7 +427,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request with resource
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand All @@ -451,7 +451,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request with resource
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand All @@ -475,7 +475,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request with resource
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down Expand Up @@ -523,7 +523,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request with resource
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand All @@ -547,7 +547,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request with resource
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand All @@ -557,7 +557,7 @@ exports[`SIWESignInfo renders correctly for SIWE signature request with resource
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ exports[`<AdvancedDetails /> does not render component for advanced transaction
</div>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down Expand Up @@ -82,7 +82,7 @@ exports[`<AdvancedDetails /> renders component for advanced transaction details
</div>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down Expand Up @@ -118,7 +118,7 @@ exports[`<AdvancedDetails /> renders component for advanced transaction details
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ exports[`TypedSignInfo correctly renders typed sign data request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand All @@ -108,7 +108,7 @@ exports[`TypedSignInfo correctly renders typed sign data request 1`] = `
</p>
</div>
<div
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center"
class="mm-box mm-box--display-flex mm-box--gap-2 mm-box--flex-wrap-wrap mm-box--align-items-center mm-box--min-width-0"
>
<p
class="mm-box mm-text mm-text--body-md mm-box--color-inherit"
Expand Down
Loading

0 comments on commit c9dbb7d

Please sign in to comment.