From a3d62af6a0bc2679ce3f43cbc2055609689f6d5f Mon Sep 17 00:00:00 2001 From: Anirudha Bose Date: Fri, 21 Jan 2022 02:40:07 +0530 Subject: [PATCH 1/2] chore(wallet): refactor format-prices utils to use bignumber.js --- .../utils/format-prices.test.ts | 49 ++++---- .../brave_wallet_ui/utils/format-prices.ts | 51 ++++---- package-lock.json | 116 ++---------------- package.json | 2 +- third_party/npm_bignumber.js/README.chromium | 2 +- 5 files changed, 60 insertions(+), 160 deletions(-) diff --git a/components/brave_wallet_ui/utils/format-prices.test.ts b/components/brave_wallet_ui/utils/format-prices.test.ts index 5d571cc5ab55..f21ab5347237 100644 --- a/components/brave_wallet_ui/utils/format-prices.test.ts +++ b/components/brave_wallet_ui/utils/format-prices.test.ts @@ -4,50 +4,47 @@ import { formatTokenAmountWithCommasAndDecimals } from './format-prices' -describe('Check Formating with Commas and Decimals', () => { - test('Value was empty, should return an empty string', () => { +describe('Check Formatting with Commas and Decimals', () => { + it('should return an empty string if value is empty', () => { const value = '' expect(formatWithCommasAndDecimals(value)).toEqual('') }) - test('Value is 0 should return 0.00', () => { - const value = '0' - expect(formatWithCommasAndDecimals(value)).toEqual('0.00') - }) - - test('Value is Unlimited should return Unlimited', () => { - const value = 'Unlimited' - expect(formatWithCommasAndDecimals(value)).toEqual('Unlimited') + it.each([ + ['0.00', '0'], + ['Unlimited', 'Unlimited'] + ])('should return %s if value is %s', (expected, value) => { + expect(formatWithCommasAndDecimals(value)).toEqual(expected) }) }) -describe('Check Formating with Commas and Decimals for Fiat', () => { - test('Value was empty, should return an empty string', () => { +describe('Check Formatting with Commas and Decimals for Fiat', () => { + it('should return an empty string if value is empty', () => { const value = '' expect(formatFiatAmountWithCommasAndDecimals(value, 'USD')).toEqual('') }) - test('USD Value is 0 should return $0.00', () => { - const value = '0' - expect(formatFiatAmountWithCommasAndDecimals(value, 'USD')).toEqual('$0.00') - }) - - test('RUB Value is 0 should return $0.00', () => { - const value = '0' - expect(formatFiatAmountWithCommasAndDecimals(value, 'RUB')).toEqual('₽0.00') + it.each([ + ['$0.00', '0', 'USD'], + ['₽0.00', '0', 'RUB'], + ['₽0.10', '0.1', 'RUB'], + ['₽0.001', '0.001', 'RUB'] + ])('should return %s if value is %s %s', (expected, value, currency) => { + expect(formatFiatAmountWithCommasAndDecimals(value, currency)).toEqual(expected) }) }) -describe('Check Formating with Commas and Decimals for Tokens', () => { - test('Value was empty, should return an empty string', () => { +describe('Check Formatting with Commas and Decimals for Tokens', () => { + it('should return an empty string if value is empty', () => { const value = '' const symbol = '' expect(formatTokenAmountWithCommasAndDecimals(value, symbol)).toEqual('') }) - test('Value is 0 should return 0.00 ETH', () => { - const value = '0' - const symbol = 'ETH' - expect(formatTokenAmountWithCommasAndDecimals(value, symbol)).toEqual('0.00 ETH') + it.each([ + ['0.00 ETH', '0', 'ETH'], + ['0.000000000000000001 ETH', '0.000000000000000001', 'ETH'] + ])('should return %s if value is %s %s', (expected, value, symbol) => { + expect(formatTokenAmountWithCommasAndDecimals(value, symbol)).toEqual(expected) }) }) diff --git a/components/brave_wallet_ui/utils/format-prices.ts b/components/brave_wallet_ui/utils/format-prices.ts index 2d85a6f6f6b4..48b878156855 100644 --- a/components/brave_wallet_ui/utils/format-prices.ts +++ b/components/brave_wallet_ui/utils/format-prices.ts @@ -1,40 +1,43 @@ +import BigNumber from 'bignumber.js' + import { CurrencySymbols } from './currency-symbols' -const addCommas = (value: string) => { - const parts = value.split('.') - return ( - parts[0].replace(/\B(?=(\d{3})+(?!\d))/g, ',') + - (parts[1] ? '.' + parts[1] : '') - ) -} -export const formatWithCommasAndDecimals = (value: string) => { +export const formatWithCommasAndDecimals = (value: string, decimals?: number) => { // empty string indicates unknown balance if (value === '') { return '' } - // We some times return Unlimited as a value - if (isNaN(Number(value))) { + const valueBN = new BigNumber(value) + + // We sometimes return Unlimited as a value + if (valueBN.isNaN()) { return value } - const valueToNumber = Number(value) - - if (valueToNumber === 0) { + if (valueBN.isZero()) { return '0.00' } - if (valueToNumber >= 10) { - return addCommas(valueToNumber.toFixed(2)) + const fmt = { + decimalSeparator: '.', + groupSeparator: ',', + groupSize: 3 + } as BigNumber.Format + + if (decimals && valueBN.isGreaterThan(10 ** -decimals)) { + return valueBN.toFormat(decimals, BigNumber.ROUND_UP, fmt) + } + + if (valueBN.isGreaterThanOrEqualTo(10)) { + return valueBN.toFormat(2, BigNumber.ROUND_UP, fmt) } - if (valueToNumber >= 1) { - return addCommas(valueToNumber.toFixed(3)) + if (valueBN.isGreaterThanOrEqualTo(1)) { + return valueBN.toFormat(3, BigNumber.ROUND_UP, fmt) } - const calculatedDecimalPlace = -Math.floor(Math.log(valueToNumber) / Math.log(10) + 1) - const added = Number(calculatedDecimalPlace) + 3 - return addCommas(valueToNumber.toFixed(added)) + return valueBN.toFormat(fmt) } export const formatFiatAmountWithCommasAndDecimals = (value: string, defaultCurrency: string): string => { @@ -44,11 +47,11 @@ export const formatFiatAmountWithCommasAndDecimals = (value: string, defaultCurr const currencySymbol = CurrencySymbols[defaultCurrency] - // Check to make sure a formated value is returned before showing the fiat symbol - if (!formatWithCommasAndDecimals(value)) { + // Check to make sure a formatted value is returned before showing the fiat symbol + if (!formatWithCommasAndDecimals(value, 2)) { return '' } - return currencySymbol + formatWithCommasAndDecimals(value) + return currencySymbol + formatWithCommasAndDecimals(value, 2) } export const formatTokenAmountWithCommasAndDecimals = (value: string, symbol: string): string => { @@ -56,7 +59,7 @@ export const formatTokenAmountWithCommasAndDecimals = (value: string, symbol: st if (!value && !symbol) { return '' } - // Check to make sure a formated value is returned before showing the symbol + // Check to make sure a formatted value is returned before showing the symbol if (!formatWithCommasAndDecimals(value)) { return '' } diff --git a/package-lock.json b/package-lock.json index 6b8a1781bc22..2ff84a08c214 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6,7 +6,7 @@ "packages": { "": { "name": "brave-core", - "version": "1.36.55", + "version": "1.36.56", "license": "MPL-2.0", "dependencies": { "@brave/react-virtualized-auto-sizer": "^1.0.4", @@ -18,7 +18,7 @@ "@types/react-router-dom": "^5.1.9", "@types/webtorrent": "^0.109.0", "array-move": "^2.2.1", - "bignumber.js": "^7.2.1", + "bignumber.js": "^9.0.2", "bluebird": "^3.5.1", "core-js": "^3.9.1", "date-fns": "^2.15.0", @@ -3420,14 +3420,6 @@ "ethers": "^5.5.2" } }, - "node_modules/@ledgerhq/hw-app-eth/node_modules/bignumber.js": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", - "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==", - "engines": { - "node": "*" - } - }, "node_modules/@ledgerhq/hw-transport": { "version": "6.20.0", "resolved": "https://registry.npmjs.org/@ledgerhq/hw-transport/-/hw-transport-6.20.0.tgz", @@ -6092,15 +6084,6 @@ "ws": "^7.4.0" } }, - "node_modules/@trezor/blockchain-link/node_modules/bignumber.js": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", - "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==", - "dev": true, - "engines": { - "node": "*" - } - }, "node_modules/@trezor/connect-common": { "version": "0.0.2", "resolved": "https://registry.npmjs.org/@trezor/connect-common/-/connect-common-0.0.2.tgz", @@ -8675,9 +8658,9 @@ "dev": true }, "node_modules/bignumber.js": { - "version": "7.2.1", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-7.2.1.tgz", - "integrity": "sha512-S4XzBk5sMB+Rcb/LNcpzXr57VRTxgAvaAEDAl1AwRx27j00hT84O6OkteE7u8UB3NuaaygCRrEpqox4uDOrbdQ==", + "version": "9.0.2", + "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", + "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==", "engines": { "node": "*" } @@ -15315,15 +15298,6 @@ "socket.io-client": "^4.1.2" } }, - "node_modules/hd-wallet/node_modules/bignumber.js": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", - "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==", - "dev": true, - "engines": { - "node": "*" - } - }, "node_modules/he": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/he/-/he-1.2.0.tgz", @@ -22263,24 +22237,6 @@ "lodash": "^4.17.15" } }, - "node_modules/ripple-lib-transactionparser/node_modules/bignumber.js": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", - "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==", - "dev": true, - "engines": { - "node": "*" - } - }, - "node_modules/ripple-lib/node_modules/bignumber.js": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", - "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==", - "dev": true, - "engines": { - "node": "*" - } - }, "node_modules/rlp": { "version": "2.2.7", "resolved": "https://registry.npmjs.org/rlp/-/rlp-2.2.7.tgz", @@ -24961,15 +24917,6 @@ "whatwg-fetch": "^3.5.0" } }, - "node_modules/trezor-connect/node_modules/bignumber.js": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", - "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==", - "dev": true, - "engines": { - "node": "*" - } - }, "node_modules/trezor-link": { "version": "1.7.3", "resolved": "https://registry.npmjs.org/trezor-link/-/trezor-link-1.7.3.tgz", @@ -30426,13 +30373,6 @@ "axios": "^0.24.0", "bignumber.js": "^9.0.2", "ethers": "^5.5.2" - }, - "dependencies": { - "bignumber.js": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", - "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==" - } } }, "@ledgerhq/hw-transport": { @@ -32279,14 +32219,6 @@ "ripple-lib": "1.10.0", "tiny-worker": "^2.3.0", "ws": "^7.4.0" - }, - "dependencies": { - "bignumber.js": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", - "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==", - "dev": true - } } }, "@trezor/connect-common": { @@ -34436,9 +34368,9 @@ "dev": true }, "bignumber.js": { - "version": "7.2.1", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-7.2.1.tgz", - "integrity": "sha512-S4XzBk5sMB+Rcb/LNcpzXr57VRTxgAvaAEDAl1AwRx27j00hT84O6OkteE7u8UB3NuaaygCRrEpqox4uDOrbdQ==" + "version": "9.0.2", + "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", + "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==" }, "binary-extensions": { "version": "2.2.0", @@ -39601,14 +39533,6 @@ "bignumber.js": "^9.0.1", "queue": "^6.0.2", "socket.io-client": "^4.1.2" - }, - "dependencies": { - "bignumber.js": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", - "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==", - "dev": true - } } }, "he": { @@ -45017,14 +44941,6 @@ "ripple-keypairs": "^1.0.3", "ripple-lib-transactionparser": "0.8.2", "ws": "^7.2.0" - }, - "dependencies": { - "bignumber.js": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", - "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==", - "dev": true - } } }, "ripple-lib-transactionparser": { @@ -45035,14 +44951,6 @@ "requires": { "bignumber.js": "^9.0.0", "lodash": "^4.17.15" - }, - "dependencies": { - "bignumber.js": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", - "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==", - "dev": true - } } }, "rlp": { @@ -47101,14 +47009,6 @@ "tiny-worker": "^2.3.0", "trezor-link": "1.7.3", "whatwg-fetch": "^3.5.0" - }, - "dependencies": { - "bignumber.js": { - "version": "9.0.2", - "resolved": "https://registry.npmjs.org/bignumber.js/-/bignumber.js-9.0.2.tgz", - "integrity": "sha512-GAcQvbpsM0pUb0zw1EI0KhQEZ+lRwR5fYaAp3vPOYuP7aDvGy6cVN6XHLauvF8SOga2y0dcLcjt3iQDTSEliyw==", - "dev": true - } } }, "trezor-link": { diff --git a/package.json b/package.json index 6133aa220c20..1d1aa9c4edca 100644 --- a/package.json +++ b/package.json @@ -347,7 +347,7 @@ "@types/webtorrent": "^0.109.0", "@noble/bls12-381": "1.0.1", "array-move": "^2.2.1", - "bignumber.js": "^7.2.1", + "bignumber.js": "^9.0.2", "bluebird": "^3.5.1", "core-js": "^3.9.1", "date-fns": "^2.15.0", diff --git a/third_party/npm_bignumber.js/README.chromium b/third_party/npm_bignumber.js/README.chromium index 56a2f7b24d15..1517ed486529 100644 --- a/third_party/npm_bignumber.js/README.chromium +++ b/third_party/npm_bignumber.js/README.chromium @@ -1,4 +1,4 @@ Name: bignumber.js URL: https://github.com/MikeMcl/bignumber.js License: MIT -License File: /brave/node_modules/bignumber.js/LICENCE +License File: /brave/node_modules/bignumber.js/LICENCE.md From 1376418b45cb05580835e65bf6a5a1e33b89dfd0 Mon Sep 17 00:00:00 2001 From: Anirudha Bose Date: Fri, 21 Jan 2022 02:41:11 +0530 Subject: [PATCH 2/2] fix(wallet): use exact value in transaction confirmation screen --- .../brave_wallet_ui/common/hooks/transaction-parser.ts | 9 +++++++++ .../extension/confirm-transaction-panel/index.tsx | 8 ++++---- .../extension/confirm-transaction-panel/style.ts | 1 + components/brave_wallet_ui/utils/format-balances.ts | 10 ++++++++-- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/components/brave_wallet_ui/common/hooks/transaction-parser.ts b/components/brave_wallet_ui/common/hooks/transaction-parser.ts index 85afe205e42b..9216941cc166 100644 --- a/components/brave_wallet_ui/common/hooks/transaction-parser.ts +++ b/components/brave_wallet_ui/common/hooks/transaction-parser.ts @@ -56,6 +56,7 @@ interface ParsedTransaction extends ParsedTransactionFees { fiatTotal: string nativeCurrencyTotal: string value: string + valueExact: string symbol: string decimals: number insufficientFundsError: boolean @@ -193,6 +194,7 @@ export function useTransactionParser ( fiatTotal: totalAmountFiat, nativeCurrencyTotal: sendAmountFiat && formatGasFeeFromFiat(sendAmountFiat, networkSpotPrice), value: formatBalance(amount, token?.decimals ?? 18), + valueExact: formatBalance(amount, token?.decimals ?? 18, false), symbol: token?.symbol ?? '', decimals: token?.decimals ?? 18, insufficientFundsError: insufficientNativeFunds || insufficientTokenFunds, @@ -230,6 +232,7 @@ export function useTransactionParser ( fiatTotal: totalAmountFiat, nativeCurrencyTotal: totalAmountFiat && formatGasFeeFromFiat(totalAmountFiat, networkSpotPrice), value: '1', // Can only send 1 erc721 at a time + valueExact: '1', symbol: token?.symbol ?? '', decimals: 0, insufficientFundsError: insufficientNativeFunds, @@ -252,6 +255,10 @@ export function useTransactionParser ( ? getLocale('braveWalletTransactionApproveUnlimited') : formatBalance(amount, token?.decimals ?? 18) + const formattedAllowanceValueExact = isNumericValueGreaterThan(amount, accountTokenBalance) + ? getLocale('braveWalletTransactionApproveUnlimited') + : formatBalance(amount, token?.decimals ?? 18, false) + return { hash: transactionInfo.txHash, nonce, @@ -265,6 +272,7 @@ export function useTransactionParser ( fiatTotal: totalAmountFiat, nativeCurrencyTotal: (0).toFixed(2), value: formattedAllowanceValue, + valueExact: formattedAllowanceValueExact, symbol: token?.symbol ?? '', decimals: token?.decimals ?? 18, approvalTarget: address, @@ -299,6 +307,7 @@ export function useTransactionParser ( fiatTotal: totalAmountFiat, nativeCurrencyTotal: sendAmountFiat && formatGasFeeFromFiat(sendAmountFiat, networkSpotPrice), value: formatBalance(value, selectedNetwork.decimals), + valueExact: formatBalance(value, selectedNetwork.decimals, false), symbol: selectedNetwork.symbol, decimals: selectedNetwork?.decimals ?? 18, insufficientFundsError: isNumericValueGreaterThan(addNumericValues(gasFee, value), accountNativeBalance), diff --git a/components/brave_wallet_ui/components/extension/confirm-transaction-panel/index.tsx b/components/brave_wallet_ui/components/extension/confirm-transaction-panel/index.tsx index 44715592ce25..148382e84353 100644 --- a/components/brave_wallet_ui/components/extension/confirm-transaction-panel/index.tsx +++ b/components/brave_wallet_ui/components/extension/confirm-transaction-panel/index.tsx @@ -350,7 +350,7 @@ function ConfirmTransactionPanel (props: Props) { {transactionInfo.txType === BraveWallet.TransactionType.ERC721TransferFrom || transactionInfo.txType === BraveWallet.TransactionType.ERC721SafeTransferFrom ? transactionDetails.erc721BlockchainToken?.name + ' ' + transactionDetails.erc721TokenId - : formatTokenAmountWithCommasAndDecimals(transactionDetails.value, transactionDetails.symbol) + : formatTokenAmountWithCommasAndDecimals(transactionDetails.valueExact, transactionDetails.symbol) } {transactionInfo.txType !== BraveWallet.TransactionType.ERC721TransferFrom && @@ -405,7 +405,7 @@ function ConfirmTransactionPanel (props: Props) { {getLocale('braveWalletAllowSpendProposedAllowance')} - {formatTokenAmountWithCommasAndDecimals(transactionDetails.value, transactionDetails.symbol)} + {formatTokenAmountWithCommasAndDecimals(transactionDetails.valueExact, transactionDetails.symbol)} @@ -429,8 +429,8 @@ function ConfirmTransactionPanel (props: Props) { {(transactionInfo.txType !== BraveWallet.TransactionType.ERC721SafeTransferFrom && transactionInfo.txType !== BraveWallet.TransactionType.ERC721TransferFrom) - ? formatWithCommasAndDecimals(transactionDetails.value) - : transactionDetails.value + ? formatWithCommasAndDecimals(transactionDetails.valueExact) + : transactionDetails.valueExact } {transactionDetails.symbol} + {formatBalance(transactionDetails.gasFee, selectedNetwork.decimals)} {selectedNetwork.symbol} { +export const formatBalance = (balance: string, decimals: number, round: boolean = true) => { if (!balance) { return '' } @@ -36,7 +36,13 @@ export const formatBalance = (balance: string, decimals: number) => { } const result = new BigNumber(balance).dividedBy(10 ** decimals) - return (result.isNaN()) ? '0.0000' : result.toFixed(4, BigNumber.ROUND_UP) + if (result.isNaN()) { + return '0.0000' + } + + return round + ? result.toFixed(4, BigNumber.ROUND_UP) + : result.toFormat() } export const formatGasFee = (gasPrice: string, gasLimit: string, decimals: number) => {