Skip to content

Commit

Permalink
fix: Precision of BigNumber values of calcTokenAmount and Configure B…
Browse files Browse the repository at this point in the history
…igNumber to support 36 decimals (#13029)

## **Description**

Fix precision across mobile transaction values. JavaScript Number
coerces values into scientific notation which looses precision when
rendering full values

Update BigNumber.config to support 36 decimals similarly to
MetaMask/metamask-extension#21147

## **Related issues**

Fixes: #12991
Relates to: #12994

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

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

### **Before**

<img width="320"
src="https://github.com/user-attachments/assets/52177add-cb0c-4835-be5e-8ea4098e29fd">


### **After**

<img width="320"
src="https://github.com/user-attachments/assets/88e59987-a820-45d4-85a3-55f51997101e">

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.
  • Loading branch information
digiwand authored Jan 16, 2025
1 parent d1946d2 commit f62e266
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 10 deletions.
32 changes: 25 additions & 7 deletions app/components/UI/SimulationDetails/formatAmount.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { BigNumber } from 'bignumber.js';

const MIN_AMOUNT = new BigNumber('0.000001');
const PRECISION = 6;

// The default precision for displaying currency values.
// It set to the number of decimal places in the minimum amount.
export const DEFAULT_PRECISION = new BigNumber(MIN_AMOUNT).decimalPlaces();

// The number of significant decimals places to show for amounts less than 1.
const MAX_SIGNIFICANT_DECIMAL_PLACES = 3;
Expand All @@ -12,11 +15,21 @@ export function formatAmountMaxPrecision(
locale: string,
num: number | BigNumber,
): string {
return new Intl.NumberFormat(locale, {
minimumSignificantDigits: 1,
}).format(new BigNumber(num.toString()).toNumber());
const bigNumberValue = new BigNumber(num);
const numberOfDecimals = bigNumberValue.decimalPlaces();
const formattedValue = bigNumberValue.toFixed(numberOfDecimals ?? 0);

const [integerPart, fractionalPart] = formattedValue.split('.');
const formattedIntegerPart = new Intl.NumberFormat(locale).format(
integerPart as unknown as number,
);

return fractionalPart
? `${formattedIntegerPart}.${fractionalPart}`
: formattedIntegerPart;
}


/**
* Formats the a token amount with variable precision and significant
* digits.
Expand Down Expand Up @@ -65,21 +78,26 @@ export function formatAmount(locale: string, amount: BigNumber): string {
return new Intl.NumberFormat(locale, {
maximumSignificantDigits: MAX_SIGNIFICANT_DECIMAL_PLACES,
} as Intl.NumberFormatOptions).format(
amount.decimalPlaces(PRECISION).toNumber(),
amount.dp(DEFAULT_PRECISION ?? 0).toNumber(),
);
}

// Preserve all digits left of the decimal point.
// Cap the digits right of the decimal point: The more digits present
// on the left side of the decimal point, the less decimal places
// we show on the right side.
const digitsLeftOfDecimal = amount.abs().toFixed(0).length;
const digitsLeftOfDecimal = amount.abs().dp(0).toString().length;
const maximumFractionDigits = Math.max(
0,
MAX_SIGNIFICANT_DECIMAL_PLACES - digitsLeftOfDecimal + 1,
);

return new Intl.NumberFormat(locale, {
maximumFractionDigits,
} as Intl.NumberFormatOptions).format(amount.toNumber());
} as Intl.NumberFormatOptions).format(
// string is valid parameter for format function
// for some reason it gives TS issue
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/format#number
amount.toFixed(maximumFractionDigits) as unknown as number,
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ const SimulationValueDisplay: React.FC<
tokenDetails as TokenDetailsERC20,
);

const tokenAmount = isNumberValue(value) && !tokenId ? calcTokenAmount(value, tokenDecimals) : null;
const tokenAmount = isNumberValue(value) && !tokenId ? calcTokenAmount(value as number | string, tokenDecimals) : null;
const isValidTokenAmount = tokenAmount !== null && tokenAmount !== undefined && tokenAmount instanceof BigNumber;

const fiatValue = isValidTokenAmount && exchangeRate && !tokenId
Expand Down
3 changes: 3 additions & 0 deletions app/util/number/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import { isZero } from '../lodash';
import { regex } from '../regex';
export { BNToHex };

const MAX_DECIMALS_FOR_TOKENS = 36;
BigNumber.config({ DECIMAL_PLACES: MAX_DECIMALS_FOR_TOKENS });

// Big Number Constants
const BIG_NUMBER_WEI_MULTIPLIER = new BigNumber('1000000000000000000');
const BIG_NUMBER_GWEI_MULTIPLIER = new BigNumber('1000000000');
Expand Down
9 changes: 7 additions & 2 deletions app/util/transactions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1428,9 +1428,14 @@ export function validateTransactionActionBalance(transaction, rate, accounts) {
}
}

/**
* @param {number|string|BigNumber} value
* @param {number=} decimals
* @returns {BigNumber}
*/
export function calcTokenAmount(value, decimals) {
const multiplier = Math.pow(10, Number(decimals || 0));
return new BigNumber(String(value)).div(multiplier);
const divisor = new BigNumber(10).pow(decimals ?? 0);
return new BigNumber(String(value)).div(divisor);
}

export function calcTokenValue(value, decimals) {
Expand Down
48 changes: 48 additions & 0 deletions app/util/transactions/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { UINT256_BN_MAX_VALUE } from '../../constants/transaction';
import { NEGATIVE_TOKEN_DECIMALS } from '../../constants/error';
import {
generateTransferData,
calcTokenAmount,
decodeApproveData,
decodeTransferData,
getMethodData,
Expand Down Expand Up @@ -42,6 +43,7 @@ import Engine from '../../core/Engine';
import { strings } from '../../../locales/i18n';
import { TransactionType } from '@metamask/transaction-controller';
import { Provider } from '@metamask/network-controller';
import BigNumber from 'bignumber.js';

jest.mock('@metamask/controller-utils', () => ({
...jest.requireActual('@metamask/controller-utils'),
Expand Down Expand Up @@ -106,6 +108,52 @@ describe('Transactions utils :: generateTransferData', () => {
});
});

describe('Transactions utils :: calcTokenAmount', () => {
it.each([
// number values
[0, 5, '0'],
[123456, undefined, '123456'],
[123456, 5, '1.23456'],
[123456, 6, '0.123456'],
// Do not delete the following test. Testing decimal = 36 is important because it has broken
// BigNumber#div in the past when the value that was passed into it was not a BigNumber.
[123456, 36, '1.23456e-31'],
[3000123456789678, 6, '3000123456.789678'],
// eslint-disable-next-line @typescript-eslint/no-loss-of-precision
[3000123456789123456789123456789, 3, '3.0001234567891233e+27'], // expected precision lost
// eslint-disable-next-line @typescript-eslint/no-loss-of-precision
[3000123456789123456789123456789, 6, '3.0001234567891233e+24'], // expected precision lost
// string values
['0', 5, '0'],
['123456', undefined, '123456'],
['123456', 5, '1.23456'],
['123456', 6, '0.123456'],
['3000123456789678', 6, '3000123456.789678'],
[
'3000123456789123456789123456789',
3,
'3.000123456789123456789123456789e+27',
],
[
'3000123456789123456789123456789',
6,
'3.000123456789123456789123456789e+24',
],
// BigNumber values
[new BigNumber('3000123456789678'), 6, '3000123456.789678'],
[
new BigNumber('3000123456789123456789123456789'),
6,
'3.000123456789123456789123456789e+24',
],
])(
'returns the value %s divided by 10^%s = %s',
(value, decimals, expected) => {
expect(calcTokenAmount(value, decimals).toString()).toBe(expected);
},
);
});

describe('Transactions utils :: decodeTransferData', () => {
it('decodeTransferData transfer', () => {
const [address, amount] = decodeTransferData(
Expand Down

0 comments on commit f62e266

Please sign in to comment.