Skip to content

Commit 4b6fb71

Browse files
authored
feat: Implement onAmountInput in send flow amount for non-EVM validation (#37345)
<!-- 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. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> This PR aims to implement `onAmountInput` RPC call into amount validations of send flow. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/37345?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** Fixes: #37188 ## **Manual testing steps** Case 1: 1. Go to send flow 2. Try send Sol 3. Don't type anything in amount input and click "Continue" 4. You should be able to see confirmation (not infinite spinner) Case 2: 1. Go to send flow 2. Try send BTC 3. Don't type anything in amount input and click "Continue" 4. You shouldn't be able to proceed confirmation because "0" is not a valid amount for BTC transaction ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [X] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [X] I've completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [X] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [X] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds non-EVM amount validation using snaps, integrates it into the send flow, refactors amount error handling, and updates tests. > > - **Send Flow UI**: > - `AmountRecipient`: integrates `useAmountValidation` and `useSendType`; on Continue, runs `validateNonEvmAmountAsync` for non-EVM before submit; removes local amount error state; passes `amountError` to `Amount`; updates button label/disabled logic to use `amountError`. > - `Amount`: switches prop from `setAmountValueError` to `amountError`; removes internal validation state/effects; displays error based on prop only. > - **Hooks/Validation**: > - New `useSnapAmountOnInput` hook to call `validateAmountMultichain` for non-EVM amount checks. > - Overhauls `useAmountValidation` to async pipeline (positive number, ERC1155 balance, token balance, non-EVM snap); exposes `validateNonEvmAmountAsync`; adds snap error mapping. > - Enhances validators: `validateERC1155Balance` (guards, standards), `validateTokenBalance` (decimals param), and new `validatePositiveNumericString`. > - `useSendActions`: for non-EVM, sends `amount` with `addLeadingZeroIfNeeded(value || '0')`. > - **Tests**: > - Adds tests for non-EVM validation flow in `amount-recipient.test.tsx` and new hook tests `useSnapAmountOnInput.test.ts`. > - Expands `useAmountValidation.test.ts` with edge cases and new validators. > - Updates `amount.test.tsx` to reflect new `Amount` API and behavior. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit c6eac31. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 5aaa253 commit 4b6fb71

File tree

9 files changed

+750
-122
lines changed

9 files changed

+750
-122
lines changed

ui/pages/confirmations/components/send/amount-recipient/amount-recipient.test.tsx

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import * as SendActions from '../../../hooks/send/useSendActions';
1414
import * as SendContext from '../../../context/send';
1515
import * as RecipientValidation from '../../../hooks/send/useRecipientValidation';
1616
import * as RecipientSelectionMetrics from '../../../hooks/send/metrics/useRecipientSelectionMetrics';
17+
import * as SendType from '../../../hooks/send/useSendType';
1718
import { AmountRecipient } from './amount-recipient';
1819

1920
const MOCK_ADDRESS = '0xdB055877e6c13b6A6B25aBcAA29B393777dD0a73';
@@ -211,4 +212,156 @@ describe('AmountRecipient', () => {
211212
fireEvent.click(getByRole('button', { name: 'Invalid hex data' }));
212213
expect(mockHandleSubmit).not.toHaveBeenCalled();
213214
});
215+
216+
it('should call validateNonEvmAmountAsync for non-EVM send type and submit if no error', async () => {
217+
const mockHandleSubmit = jest.fn();
218+
const mockValidateNonEvmAmountAsync = jest.fn().mockResolvedValue(null);
219+
const mockCaptureAmountSelected = jest.fn();
220+
const mockCaptureRecipientSelected = jest.fn();
221+
222+
jest.spyOn(SendActions, 'useSendActions').mockReturnValue({
223+
handleSubmit: mockHandleSubmit,
224+
} as unknown as ReturnType<typeof SendActions.useSendActions>);
225+
226+
jest
227+
.spyOn(AmountSelectionMetrics, 'useAmountSelectionMetrics')
228+
.mockReturnValue({
229+
captureAmountSelected: mockCaptureAmountSelected,
230+
} as unknown as ReturnType<
231+
typeof AmountSelectionMetrics.useAmountSelectionMetrics
232+
>);
233+
234+
jest
235+
.spyOn(RecipientSelectionMetrics, 'useRecipientSelectionMetrics')
236+
.mockReturnValue({
237+
captureRecipientSelected: mockCaptureRecipientSelected,
238+
setRecipientInputMethodManual: jest.fn(),
239+
} as unknown as ReturnType<
240+
typeof RecipientSelectionMetrics.useRecipientSelectionMetrics
241+
>);
242+
243+
jest.spyOn(SendContext, 'useSendContext').mockReturnValue({
244+
toResolved: MOCK_ADDRESS,
245+
asset: EVM_ASSET,
246+
chainId: '0x1',
247+
from: 'from-address',
248+
updateAsset: jest.fn(),
249+
updateCurrentPage: jest.fn(),
250+
updateTo: jest.fn(),
251+
updateToResolved: jest.fn(),
252+
updateValue: jest.fn(),
253+
value: '1',
254+
} as unknown as ReturnType<typeof SendContext.useSendContext>);
255+
256+
jest.spyOn(AmountValidation, 'useAmountValidation').mockReturnValue({
257+
amountError: undefined,
258+
validateNonEvmAmountAsync: mockValidateNonEvmAmountAsync,
259+
} as unknown as ReturnType<typeof AmountValidation.useAmountValidation>);
260+
261+
jest.spyOn(RecipientValidation, 'useRecipientValidation').mockReturnValue({
262+
recipientError: null,
263+
recipientWarning: null,
264+
recipientResolvedLookup: null,
265+
recipientConfusableCharacters: [],
266+
validateRecipient: jest.fn(),
267+
} as unknown as ReturnType<
268+
typeof RecipientValidation.useRecipientValidation
269+
>);
270+
271+
jest.spyOn(SendType, 'useSendType').mockReturnValue({
272+
isNonEvmSendType: true,
273+
} as unknown as ReturnType<typeof SendType.useSendType>);
274+
275+
const { getAllByRole, getByText } = render();
276+
277+
fireEvent.change(getAllByRole('textbox')[0], {
278+
target: { value: MOCK_ADDRESS },
279+
});
280+
281+
fireEvent.click(getByText('Continue'));
282+
283+
await new Promise(process.nextTick);
284+
285+
expect(mockValidateNonEvmAmountAsync).toHaveBeenCalled();
286+
expect(mockHandleSubmit).toHaveBeenCalled();
287+
expect(mockCaptureAmountSelected).toHaveBeenCalled();
288+
expect(mockCaptureRecipientSelected).toHaveBeenCalled();
289+
});
290+
291+
it('should call validateNonEvmAmountAsync for non-EVM send type and not submit if there is an error', async () => {
292+
const mockHandleSubmit = jest.fn();
293+
const mockValidateNonEvmAmountAsync = jest
294+
.fn()
295+
.mockResolvedValue('Amount required');
296+
const mockCaptureAmountSelected = jest.fn();
297+
const mockCaptureRecipientSelected = jest.fn();
298+
299+
jest.spyOn(SendActions, 'useSendActions').mockReturnValue({
300+
handleSubmit: mockHandleSubmit,
301+
} as unknown as ReturnType<typeof SendActions.useSendActions>);
302+
303+
jest
304+
.spyOn(AmountSelectionMetrics, 'useAmountSelectionMetrics')
305+
.mockReturnValue({
306+
captureAmountSelected: mockCaptureAmountSelected,
307+
} as unknown as ReturnType<
308+
typeof AmountSelectionMetrics.useAmountSelectionMetrics
309+
>);
310+
311+
jest
312+
.spyOn(RecipientSelectionMetrics, 'useRecipientSelectionMetrics')
313+
.mockReturnValue({
314+
captureRecipientSelected: mockCaptureRecipientSelected,
315+
setRecipientInputMethodManual: jest.fn(),
316+
} as unknown as ReturnType<
317+
typeof RecipientSelectionMetrics.useRecipientSelectionMetrics
318+
>);
319+
320+
jest.spyOn(SendContext, 'useSendContext').mockReturnValue({
321+
toResolved: MOCK_ADDRESS,
322+
asset: EVM_ASSET,
323+
chainId: '0x1',
324+
from: 'from-address',
325+
updateAsset: jest.fn(),
326+
updateCurrentPage: jest.fn(),
327+
updateTo: jest.fn(),
328+
updateToResolved: jest.fn(),
329+
updateValue: jest.fn(),
330+
value: '1',
331+
} as unknown as ReturnType<typeof SendContext.useSendContext>);
332+
333+
jest.spyOn(AmountValidation, 'useAmountValidation').mockReturnValue({
334+
amountError: undefined,
335+
validateNonEvmAmountAsync: mockValidateNonEvmAmountAsync,
336+
} as unknown as ReturnType<typeof AmountValidation.useAmountValidation>);
337+
338+
jest.spyOn(RecipientValidation, 'useRecipientValidation').mockReturnValue({
339+
recipientError: null,
340+
recipientWarning: null,
341+
recipientResolvedLookup: null,
342+
recipientConfusableCharacters: [],
343+
validateRecipient: jest.fn(),
344+
} as unknown as ReturnType<
345+
typeof RecipientValidation.useRecipientValidation
346+
>);
347+
348+
jest.spyOn(SendType, 'useSendType').mockReturnValue({
349+
isNonEvmSendType: true,
350+
} as unknown as ReturnType<typeof SendType.useSendType>);
351+
352+
const { getAllByRole, getByText } = render();
353+
354+
fireEvent.change(getAllByRole('textbox')[0], {
355+
target: { value: MOCK_ADDRESS },
356+
});
357+
358+
fireEvent.click(getByText('Continue'));
359+
360+
await new Promise(process.nextTick);
361+
362+
expect(mockValidateNonEvmAmountAsync).toHaveBeenCalled();
363+
expect(mockHandleSubmit).not.toHaveBeenCalled();
364+
expect(mockCaptureAmountSelected).not.toHaveBeenCalled();
365+
expect(mockCaptureRecipientSelected).not.toHaveBeenCalled();
366+
});
214367
});

ui/pages/confirmations/components/send/amount-recipient/amount-recipient.tsx

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,32 +19,49 @@ import { useSendActions } from '../../../hooks/send/useSendActions';
1919
import { useSendContext } from '../../../context/send';
2020
import { useRecipientValidation } from '../../../hooks/send/useRecipientValidation';
2121
import { useRecipientSelectionMetrics } from '../../../hooks/send/metrics/useRecipientSelectionMetrics';
22+
import { useAmountValidation } from '../../../hooks/send/useAmountValidation';
23+
import { useSendType } from '../../../hooks/send/useSendType';
2224
import { SendHero } from '../../UI/send-hero';
2325
import { Amount } from '../amount/amount';
2426
import { Recipient } from '../recipient';
2527
import { HexData } from '../hex-data';
2628

2729
export const AmountRecipient = () => {
2830
const t = useI18nContext();
29-
const [amountValueError, setAmountValueError] = useState<string>();
3031
const [hexDataError, setHexDataError] = useState<string>();
3132
const { asset, toResolved } = useSendContext();
33+
const { amountError, validateNonEvmAmountAsync } = useAmountValidation();
34+
const { isNonEvmSendType } = useSendType();
3235
const { handleSubmit } = useSendActions();
3336
const { captureAmountSelected } = useAmountSelectionMetrics();
3437
const { captureRecipientSelected } = useRecipientSelectionMetrics();
3538
const recipientValidationResult = useRecipientValidation();
3639

3740
const hasError =
38-
Boolean(amountValueError) ||
41+
Boolean(amountError) ||
3942
Boolean(recipientValidationResult.recipientError) ||
4043
Boolean(hexDataError);
4144
const isDisabled = hasError || !toResolved;
4245

43-
const onClick = useCallback(() => {
46+
const onClick = useCallback(async () => {
47+
if (isNonEvmSendType) {
48+
// Non EVM flows need an extra validation because "value" can be empty dependent on the blockchain (e.g it's fine for Solana but not for Bitcoin)
49+
// Hence we do a call for `validateNonEvmAmountAsync` here to raise UI validation errors if exists
50+
const nonEvmAmountError = await validateNonEvmAmountAsync();
51+
if (nonEvmAmountError) {
52+
return;
53+
}
54+
}
4455
handleSubmit();
4556
captureAmountSelected();
4657
captureRecipientSelected();
47-
}, [captureAmountSelected, captureRecipientSelected, handleSubmit]);
58+
}, [
59+
captureAmountSelected,
60+
captureRecipientSelected,
61+
handleSubmit,
62+
isNonEvmSendType,
63+
validateNonEvmAmountAsync,
64+
]);
4865

4966
if (!asset) {
5067
return <LoadingScreen />;
@@ -62,7 +79,7 @@ export const AmountRecipient = () => {
6279
<Box>
6380
<SendHero asset={asset as Asset} />
6481
<Recipient recipientValidationResult={recipientValidationResult} />
65-
<Amount setAmountValueError={setAmountValueError} />
82+
<Amount amountError={amountError} />
6683
<HexData setHexDataError={setHexDataError} />
6784
</Box>
6885
<Button
@@ -74,7 +91,7 @@ export const AmountRecipient = () => {
7491
}
7592
marginBottom={4}
7693
>
77-
{amountValueError ?? hexDataError ?? t('continue')}
94+
{amountError ?? hexDataError ?? t('continue')}
7895
</Button>
7996
</Box>
8097
);

ui/pages/confirmations/components/send/amount/amount.test.tsx

Lines changed: 2 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,11 @@ import { Amount } from './amount';
2121

2222
const render = (
2323
args?: Record<string, unknown>,
24-
mockSetAmountValueError = jest.fn(),
24+
mockAmountError = undefined,
2525
) => {
2626
const store = configureStore(args ?? mockState);
2727

28-
return renderWithProvider(
29-
<Amount setAmountValueError={mockSetAmountValueError} />,
30-
store,
31-
);
28+
return renderWithProvider(<Amount amountError={mockAmountError} />, store);
3229
};
3330

3431
describe('Amount', () => {
@@ -335,49 +332,4 @@ describe('Amount', () => {
335332
const { queryByText } = render();
336333
expect(queryByText('Max')).not.toBeInTheDocument();
337334
});
338-
339-
it('call arg mockSetAmountValueError if amount has error', () => {
340-
jest.spyOn(SendContext, 'useSendContext').mockReturnValue({
341-
asset: EVM_ASSET,
342-
updateValue: jest.fn(),
343-
} as unknown as SendContext.SendContextType);
344-
jest.spyOn(BalanceFunctions, 'useBalance').mockReturnValue({
345-
balance: '10.023',
346-
rawBalanceNumeric: new Numeric('10.023', 10),
347-
} as unknown as ReturnType<typeof BalanceFunctions.useBalance>);
348-
jest.spyOn(CurrencyConversions, 'useCurrencyConversions').mockReturnValue({
349-
conversionSupportedForAsset: true,
350-
fiatCurrencySymbol: '$',
351-
fiatCurrencyName: 'usd',
352-
getFiatValue: () => '20',
353-
getFiatDisplayValue: () => '$ 20.00',
354-
getNativeValue: () => '20',
355-
});
356-
const mockSetAmountInputTypeFiat = jest.fn();
357-
const mockSetAmountInputTypeToken = jest.fn();
358-
jest
359-
.spyOn(AmountSelectionMetrics, 'useAmountSelectionMetrics')
360-
.mockReturnValue({
361-
setAmountInputTypeFiat: mockSetAmountInputTypeFiat,
362-
setAmountInputTypeToken: mockSetAmountInputTypeToken,
363-
} as unknown as ReturnType<
364-
typeof AmountSelectionMetrics.useAmountSelectionMetrics
365-
>);
366-
const mockSetAmountValueError = jest.fn();
367-
368-
const { getByRole, getByTestId } = render(
369-
undefined,
370-
mockSetAmountValueError,
371-
);
372-
373-
expect(mockSetAmountValueError).toHaveBeenLastCalledWith(undefined);
374-
fireEvent.change(getByRole('textbox'), { target: { value: 'abc' } });
375-
expect(mockSetAmountValueError).toHaveBeenLastCalledWith('Invalid value');
376-
fireEvent.click(getByTestId('toggle-fiat-mode'));
377-
fireEvent.change(getByRole('textbox'), { target: { value: '' } });
378-
expect(mockSetAmountValueError).toHaveBeenLastCalledWith(undefined);
379-
fireEvent.change(getByRole('textbox'), { target: { value: 'abc' } });
380-
expect(mockSetAmountValueError).toHaveBeenLastCalledWith('Invalid value');
381-
expect(mockSetAmountValueError).toHaveBeenCalledTimes(4);
382-
});
383335
});

ui/pages/confirmations/components/send/amount/amount.tsx

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useCallback, useEffect, useMemo, useState } from 'react';
1+
import React, { useCallback, useMemo, useState } from 'react';
22
import { ERC721 } from '@metamask/controller-utils';
33

44
import {
@@ -20,7 +20,6 @@ import {
2020
} from '../../../../../helpers/constants/design-system';
2121
import { useI18nContext } from '../../../../../hooks/useI18nContext';
2222
import { useAmountSelectionMetrics } from '../../../hooks/send/metrics/useAmountSelectionMetrics';
23-
import { useAmountValidation } from '../../../hooks/send/useAmountValidation';
2423
import { useBalance } from '../../../hooks/send/useBalance';
2524
import { useCurrencyConversions } from '../../../hooks/send/useCurrencyConversions';
2625
import { useMaxAmount } from '../../../hooks/send/useMaxAmount';
@@ -33,14 +32,13 @@ import {
3332
} from '../../../utils/send';
3433

3534
export const Amount = ({
36-
setAmountValueError,
35+
amountError,
3736
}: {
38-
setAmountValueError: (str?: string) => void;
37+
amountError: string | undefined;
3938
}) => {
4039
const t = useI18nContext();
4140
const { asset, updateValue, value } = useSendContext();
4241
const [amount, setAmount] = useState(value ?? '');
43-
const [amountValueError, setAmountValueErrorLocal] = useState<string>();
4442
const { balance } = useBalance();
4543
const [fiatMode, setFiatMode] = useState(false);
4644
const {
@@ -66,20 +64,6 @@ export const Amount = ({
6664
: getFiatDisplayValue(amount),
6765
[amount, fiatMode, getFiatDisplayValue, value],
6866
);
69-
const { amountError } = useAmountValidation();
70-
71-
useEffect(() => {
72-
let amtError: string | undefined;
73-
if (amountError) {
74-
amtError = amountError;
75-
} else if (amount === undefined || amount === null || amount === '') {
76-
amtError = undefined;
77-
} else if (!isValidPositiveNumericString(amount)) {
78-
amtError = t('invalidValue');
79-
}
80-
setAmountValueError(amtError);
81-
setAmountValueErrorLocal(amtError);
82-
}, [amount, amountError, setAmountValueError, setAmountValueErrorLocal, t]);
8367

8468
const onChange = useCallback(
8569
(event: React.ChangeEvent<HTMLInputElement>) => {
@@ -147,7 +131,7 @@ export const Amount = ({
147131
{t('amount')}
148132
</Text>
149133
<TextField
150-
error={Boolean(amountValueError)}
134+
error={Boolean(amountError)}
151135
onChange={onChange}
152136
onPaste={setAmountInputMethodPasted}
153137
onInput={setAmountInputMethodManual}
@@ -180,13 +164,11 @@ export const Amount = ({
180164
>
181165
<Text
182166
color={
183-
amountValueError
184-
? TextColor.errorDefault
185-
: TextColor.textAlternative
167+
amountError ? TextColor.errorDefault : TextColor.textAlternative
186168
}
187169
variant={TextVariant.bodySm}
188170
>
189-
{amountValueError ||
171+
{amountError ||
190172
(conversionSupportedForAsset ? alternateDisplayValue : '')}
191173
</Text>
192174
<Box display={Display.Flex}>

0 commit comments

Comments
 (0)