Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(wallet): show missing gas limit error in txn confirmation screen #12044

Merged
merged 1 commit into from
Jan 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions components/brave_wallet/browser/brave_wallet_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,8 @@ constexpr webui::LocalizedString kLocalizedStrings[] = {
IDS_BRAVE_WALLET_ADDRESS_MISSING_CHECKSUM_INFO_WARNING},
{"braveWalletNotValidChecksumAddressError",
IDS_BRAVE_WALLET_NOT_VALID_CHECKSUM_ADDRESS_ERROR},
{"braveWalletMissingGasLimitError",
IDS_BRAVE_WALLET_MISSING_GAS_LIMIT_ERROR},
{"braveWalletQueueOf", IDS_BRAVE_WALLET_QUEUE_OF},
{"braveWalletQueueNext", IDS_BRAVE_WALLET_QUEUE_NEXT},
{"braveWalletQueueFirst", IDS_BRAVE_WALLET_QUEUE_FIRST},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -579,4 +579,72 @@ describe('useTransactionParser hook', () => {
})
})
})

describe('check for empty gas limit', () => {
describe.each([
['ERC20Approve', BraveWallet.TransactionType.ERC20Approve],
['ERC20Transfer', BraveWallet.TransactionType.ERC20Transfer],
['ERC721TransferFrom', BraveWallet.TransactionType.ERC721TransferFrom],
['ERC721SafeTransferFrom', BraveWallet.TransactionType.ERC721SafeTransferFrom],
['ETHSend', BraveWallet.TransactionType.ETHSend],
['Other', BraveWallet.TransactionType.Other]
])('%s', (_, txType) => {
it('should return missingGasLimitError if gas limit is zero or empty', () => {
const { result: { current: transactionParser } } = renderHook(() => useTransactionParser(
mockNetwork, [mockAccount], [], [], [mockERC20Token]
))

const baseMockTransactionInfo = {
...getMockedTransactionInfo(),
txType,
txArgs: [
'mockRecipient',
'mockAmount'
]
}

const mockTransactionInfo1 = {
...baseMockTransactionInfo,
txData: {
...baseMockTransactionInfo.txData,
baseData: {
...baseMockTransactionInfo.txData.baseData,
gasLimit: ''
}
}
}
const parsedTransaction1 = transactionParser(mockTransactionInfo1)
expect(parsedTransaction1.gasLimit).toEqual('')
expect(parsedTransaction1.missingGasLimitError).toBeTruthy()

const mockTransactionInfo2 = {
...baseMockTransactionInfo,
txData: {
...baseMockTransactionInfo.txData,
baseData: {
...baseMockTransactionInfo.txData.baseData,
gasLimit: '0x0'
}
}
}
const parsedTransaction2 = transactionParser(mockTransactionInfo2)
expect(parsedTransaction2.gasLimit).toEqual('0')
expect(parsedTransaction2.missingGasLimitError).toBeTruthy()

const mockTransactionInfo3 = {
...baseMockTransactionInfo,
txData: {
...baseMockTransactionInfo.txData,
baseData: {
...baseMockTransactionInfo.txData.baseData,
gasLimit: '0x1'
}
}
}
const parsedTransaction3 = transactionParser(mockTransactionInfo3)
expect(parsedTransaction3.gasLimit).toEqual('1')
expect(parsedTransaction3.missingGasLimitError).toBeUndefined()
})
})
})
})
22 changes: 21 additions & 1 deletion components/brave_wallet_ui/common/hooks/transaction-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ interface ParsedTransactionFees {
gasFee: string
gasFeeFiat: string
isEIP1559Transaction: boolean
missingGasLimitError?: string
}

interface ParsedTransaction extends ParsedTransactionFees {
Expand Down Expand Up @@ -72,6 +73,24 @@ interface ParsedTransaction extends ParsedTransactionFees {
}

export function useTransactionFeesParser (selectedNetwork: BraveWallet.EthereumChain, networkSpotPrice: string) {
/**
* Checks if a given gasLimit is empty or zero-value, and returns an
* appropriate localized error string.
*
* @remarks
*
* This function may only be used on ALL transaction types.
*
* @param gasLimit - The parsed gasLimit string.
* @returns Localized string describing the error, or undefined in case of
* no error.
*/
const checkForMissingGasLimitError = React.useCallback((gasLimit: string): string | undefined => {
return (gasLimit === '' || normalizeNumericValue(gasLimit) === '0')
? getLocale('braveWalletMissingGasLimitError')
: undefined
}, [])

return React.useCallback((transactionInfo: BraveWallet.TransactionInfo): ParsedTransactionFees => {
const { txData } = transactionInfo
const { baseData: { gasLimit, gasPrice }, maxFeePerGas, maxPriorityFeePerGas } = txData
Expand All @@ -88,7 +107,8 @@ export function useTransactionFeesParser (selectedNetwork: BraveWallet.EthereumC
maxPriorityFeePerGas: normalizeNumericValue(maxPriorityFeePerGas),
gasFee,
gasFeeFiat: formatFiatBalance(gasFee, selectedNetwork.decimals, networkSpotPrice),
isEIP1559Transaction
isEIP1559Transaction,
missingGasLimitError: checkForMissingGasLimitError(gasLimit)
}
}, [selectedNetwork, networkSpotPrice])
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@ function ConfirmTransactionPanel (props: Props) {
return (
!!transactionDetails.sameAddressError ||
!!transactionDetails.contractAddressError ||
transactionDetails.insufficientFundsError
transactionDetails.insufficientFundsError ||
!!transactionDetails.missingGasLimitError
)
}, [transactionDetails])

Expand Down Expand Up @@ -468,6 +469,12 @@ function ConfirmTransactionPanel (props: Props) {
</ErrorText>
}

{transactionDetails.missingGasLimitError &&
<ErrorText>
{transactionDetails.missingGasLimitError}
</ErrorText>
}

<ButtonRow>
<NavButton
buttonType='reject'
Expand Down
1 change: 1 addition & 0 deletions components/brave_wallet_ui/stories/locale.ts
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ provideStrings({
braveWalletContractAddressError: 'The receiving address is a tokens contract address',
braveWalletAddressMissingChecksumInfoWarning: 'Missing checksum information',
braveWalletNotValidChecksumAddressError: 'Invalid checksum information',
braveWalletMissingGasLimitError: 'Missing gas limit',

// Transaction Queue Strings
braveWalletQueueOf: 'of',
Expand Down
1 change: 1 addition & 0 deletions components/resources/wallet_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@
<message name="IDS_BRAVE_WALLET_CONTRACT_ADDRESS_ERROR" desc="Send input trying to send crypto to a tokens contract address">The receiving address is a token's contract address</message>
<message name="IDS_BRAVE_WALLET_ADDRESS_MISSING_CHECKSUM_INFO_WARNING" desc="Send input address missing checksum information">Missing checksum information</message>
<message name="IDS_BRAVE_WALLET_NOT_VALID_CHECKSUM_ADDRESS_ERROR" desc="Send input address has invalid checksum information">Invalid checksum information</message>
<message name="IDS_BRAVE_WALLET_MISSING_GAS_LIMIT_ERROR" desc="Transaction has missing gas limit information">Missing gas limit</message>
<message name="IDS_BRAVE_WALLET_QUEUE_OF" desc="Confirm Transaction Queue of">of</message>
<message name="IDS_BRAVE_WALLET_QUEUE_NEXT" desc="Confirm Transaction Queue next">next</message>
<message name="IDS_BRAVE_WALLET_QUEUE_FIRST" desc="Confirm Transaction Queue first">first</message>
Expand Down