Skip to content

Commit

Permalink
Merge pull request #11938 from brave/f/wallet/balance-formatting
Browse files Browse the repository at this point in the history
Fix inexact amounts in transction confirmation screen
  • Loading branch information
onyb authored Jan 21, 2022
2 parents de764c2 + 1376418 commit 42622b0
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 166 deletions.
9 changes: 9 additions & 0 deletions components/brave_wallet_ui/common/hooks/transaction-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ interface ParsedTransaction extends ParsedTransactionFees {
fiatTotal: string
nativeCurrencyTotal: string
value: string
valueExact: string
symbol: string
decimals: number
insufficientFundsError: boolean
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
</TransactionAmountBig>
{transactionInfo.txType !== BraveWallet.TransactionType.ERC721TransferFrom &&
Expand Down Expand Up @@ -405,7 +405,7 @@ function ConfirmTransactionPanel (props: Props) {
<SectionRow>
<TransactionTitle>{getLocale('braveWalletAllowSpendProposedAllowance')}</TransactionTitle>
<SectionRightColumn>
<TransactionTypeText>{formatTokenAmountWithCommasAndDecimals(transactionDetails.value, transactionDetails.symbol)}</TransactionTypeText>
<TransactionTypeText>{formatTokenAmountWithCommasAndDecimals(transactionDetails.valueExact, transactionDetails.symbol)}</TransactionTypeText>
<TransactionText />
</SectionRightColumn>
</SectionRow>
Expand All @@ -429,8 +429,8 @@ function ConfirmTransactionPanel (props: Props) {
<GrandTotalText>
{(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}</GrandTotalText>
</SingleRow>
<TransactionText
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ export const SingleRow = styled.div`
justify-content: space-between;
flex-direction: row;
width: 100%;
gap: 5px;
`

export const SectionRightColumn = styled.div`
Expand Down
10 changes: 8 additions & 2 deletions components/brave_wallet_ui/utils/format-balances.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const formatInputValue = (value: string, decimals: number, round = true)
return formattedValue.replace(/\.0*$|(\.\d*[1-9])0+$/, '$1')
}

export const formatBalance = (balance: string, decimals: number) => {
export const formatBalance = (balance: string, decimals: number, round: boolean = true) => {
if (!balance) {
return ''
}
Expand All @@ -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) => {
Expand Down
49 changes: 23 additions & 26 deletions components/brave_wallet_ui/utils/format-prices.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
51 changes: 27 additions & 24 deletions components/brave_wallet_ui/utils/format-prices.ts
Original file line number Diff line number Diff line change
@@ -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 => {
Expand All @@ -44,19 +47,19 @@ 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 => {
// Empty string indicates unknown balance
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 ''
}
Expand Down
Loading

0 comments on commit 42622b0

Please sign in to comment.