fix: src amount validation and quote fetching

MetaMask · Jan 30, 2025 · 39fbe7b · 39fbe7b
1 parent 180641b
commit 39fbe7b
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 12 deletions.
diff --git a/ui/pages/bridge/prepare/bridge-input-group.tsx b/ui/pages/bridge/prepare/bridge-input-group.tsx
@@ -139,19 +139,39 @@ export const BridgeInputGroup = ({
           placeholder={'0'}
           onKeyPress={(e?: React.KeyboardEvent<HTMLDivElement>) => {
             // Only allow numbers and at most one decimal point
-            if (
-              e &&
-              !/^[0-9]*\.{0,1}[0-9]*$/u.test(
-                `${amountFieldProps.value ?? ''}${e.key}`,
-              )
-            ) {
-              e.preventDefault();
+            if (e && token?.decimals) {
+              // Only allow numbers and at most one decimal point
+              if (
+                e.key === '.' &&
+                amountFieldProps.value?.toString().includes('.')
+              ) {
+                e.preventDefault();
+              } else if (!/^[\d.]{1}$/u.test(e.key)) {
+                e.preventDefault();
+              }
             }
           }}
+          onPaste={(e: React.ClipboardEvent<HTMLInputElement>) => {
+            e.preventDefault();
+            const cleanedValue = e.clipboardData
+              .getData('text')
+              // Remove characters that are not numbers or decimal points if rendering a controlled or pasted value
+              .replace(/[^\d.]+/gu, '')
+              // Only allow one decimal point, ignore digits after second decimal point
+              .split('.', 2)
+              .join('.');
+            onAmountChange?.(cleanedValue ?? '');
+          }}
           onChange={(e) => {
-            // Remove characters that are not numbers or decimal points if rendering a controlled or pasted value
-            const cleanedValue = e.target.value.replace(/[^0-9.]+/gu, '');
-            onAmountChange?.(cleanedValue);
+            e.preventDefault();
+            e.stopPropagation();
+            const cleanedValue = e.target.value
+              // Remove characters that are not numbers or decimal points if rendering a controlled or pasted value
+              .replace(/[^\d.]+/gu, '')
+              // Only allow one decimal point, ignore digits after second decimal point
+              .split('.', 2)
+              .join('.');
+            onAmountChange?.(cleanedValue ?? '');
           }}
           {...amountFieldProps}
         />

diff --git a/ui/pages/bridge/prepare/prepare-bridge-page.test.tsx b/ui/pages/bridge/prepare/prepare-bridge-page.test.tsx
@@ -2,6 +2,7 @@ import React from 'react';
 import { act } from '@testing-library/react';
 import * as reactRouterUtils from 'react-router-dom-v5-compat';
 import { zeroAddress } from 'ethereumjs-util';
+import userEvent from '@testing-library/user-event';
 import { fireEvent, renderWithProvider } from '../../../../test/jest';
 import configureStore from '../../../store/store';
 import { createBridgeMockStore } from '../../../../test/jest/mock-store';
@@ -208,4 +209,65 @@ describe('PrepareBridgePage', () => {
       renderWithProvider(<PrepareBridgePage />, configureStore(mockStore)),
     ).toThrow();
   });
+
+  it('should validate src amount on change', async () => {
+    jest
+      .spyOn(reactRouterUtils, 'useSearchParams')
+      .mockReturnValue([{ get: () => null }] as never);
+    const mockStore = createBridgeMockStore({
+      featureFlagOverrides: {
+        extensionConfig: {
+          chains: {
+            [CHAIN_IDS.MAINNET]: {
+              isActiveSrc: true,
+              isActiveDest: false,
+            },
+          },
+        },
+      },
+    });
+    const { getByTestId } = renderWithProvider(
+      <PrepareBridgePage />,
+      configureStore(mockStore),
+    );
+
+    expect(getByTestId('from-amount').closest('input')).not.toBeDisabled();
+
+    act(() => {
+      fireEvent.change(getByTestId('from-amount'), {
+        target: { value: '2abc.123456123456123456' },
+      });
+    });
+    expect(getByTestId('from-amount').closest('input')).toHaveValue(
+      '2.123456123456123456',
+    );
+
+    act(() => {
+      fireEvent.change(getByTestId('from-amount'), {
+        target: { value: '2abc,131.1212' },
+      });
+    });
+    expect(getByTestId('from-amount').closest('input')).toHaveValue(
+      '2131.1212',
+    );
+
+    act(() => {
+      fireEvent.change(getByTestId('from-amount'), {
+        target: { value: '2abc,131.123456123456123456123456' },
+      });
+    });
+    expect(getByTestId('from-amount').closest('input')).toHaveValue(
+      '2131.123456123456123456123456',
+    );
+
+    act(() => {
+      fireEvent.change(getByTestId('from-amount'), {
+        target: { value: '2abc.131.123456123456123456123456' },
+      });
+    });
+    expect(getByTestId('from-amount').closest('input')).toHaveValue('2.131');
+
+    userEvent.paste('2abc.131.123456123456123456123456');
+    expect(getByTestId('from-amount').closest('input')).toHaveValue('2.131');
+  });
 });
diff --git a/ui/pages/bridge/prepare/prepare-bridge-page.tsx b/ui/pages/bridge/prepare/prepare-bridge-page.tsx
@@ -266,7 +266,10 @@ const PrepareBridgePage = () => {
               // Treat empty or incomplete amount as 0 to reject NaN
               ['', '.'].includes(fromAmount) ? '0' : fromAmount,
               fromToken.decimals,
-            ).toFixed()
+            )
+              .toFixed()
+              // Length of decimal part cannot exceed token.decimals
+              .split('.')[0]
           : undefined,
       srcChainId: fromChain?.chainId
         ? Number(hexToDecimal(fromChain.chainId))
@@ -301,7 +304,7 @@ const PrepareBridgePage = () => {
 
   useEffect(() => {
     debouncedUpdateQuoteRequestInController(quoteParams);
-  }, Object.values(quoteParams));
+  }, [quoteParams]);
 
   const trackInputEvent = useCallback(
     (