fix(accounts): fix key selection and validation, and add use max to transfers (#5184)

libs/accounts/src/lib/transfer-container.tsx

@@ -1,5 +1,5 @@
 import * as Schema from '@vegaprotocol/types';
-import { addDecimal, truncateByChars } from '@vegaprotocol/utils';
+import { truncateByChars } from '@vegaprotocol/utils';
 import { t } from '@vegaprotocol/i18n';
 import {
   NetworkParams,
@@ -12,7 +12,6 @@ import { useVegaWallet } from '@vegaprotocol/wallet';
 import { useCallback } from 'react';
 import { accountsDataProvider } from './accounts-data-provider';
 import { TransferForm } from './transfer-form';
-import sortBy from 'lodash/sortBy';
 import { Lozenge } from '@vegaprotocol/ui-toolkit';
 
 export const ALLOWED_ACCOUNTS = [
@@ -42,18 +41,6 @@ export const TransferContainer = ({ assetId }: { assetId?: string }) => {
     ? data.filter((account) => ALLOWED_ACCOUNTS.includes(account.type))
     : [];
 
-  const assets = accounts
-    // Theres only one general account for each asset, this will give us a list
-    // of assets the user has accounts for
-    .filter((a) => a.type === Schema.AccountType.ACCOUNT_TYPE_GENERAL)
-    .map((account) => ({
-      id: account.asset.id,
-      symbol: account.asset.symbol,
-      name: account.asset.name,
-      decimals: account.asset.decimals,
-      balance: addDecimal(account.balance, account.asset.decimals),
-    }));
-
   return (
     <>
       <p className="mb-4 text-sm" data-testid="transfer-intro-text">
@@ -71,7 +58,6 @@ export const TransferContainer = ({ assetId }: { assetId?: string }) => {
       <TransferForm
         pubKey={pubKey}
         pubKeys={pubKeys ? pubKeys?.map((pk) => pk.publicKey) : null}
-        assets={sortBy(assets, 'name')}
         assetId={assetId}
         feeFactor={param}
         submitTransfer={transfer}

libs/accounts/src/lib/transfer-form.spec.tsx

@@ -3,7 +3,7 @@ import userEvent from '@testing-library/user-event';
 import BigNumber from 'bignumber.js';
 import { AddressField, TransferFee, TransferForm } from './transfer-form';
 import { AccountType } from '@vegaprotocol/types';
-import { addDecimal, formatNumber, removeDecimal } from '@vegaprotocol/utils';
+import { removeDecimal } from '@vegaprotocol/utils';
 
 describe('TransferForm', () => {
   const submit = async () => {
@@ -14,7 +14,6 @@ describe('TransferForm', () => {
 
   const selectAsset = async (asset: {
     id: string;
-    balance: string;
     name: string;
     decimals: number;
   }) => {
@@ -28,9 +27,6 @@ describe('TransferForm', () => {
     expect(await screen.findByTestId('select-asset')).toHaveTextContent(
       asset.name
     );
-    expect(await screen.findByTestId('asset-balance')).toHaveTextContent(
-      formatNumber(asset.balance, asset.decimals)
-    );
   };
 
   const amount = '100';
@@ -41,7 +37,6 @@ describe('TransferForm', () => {
     symbol: '€',
     name: 'EUR',
     decimals: 2,
-    balance: addDecimal(100000, 2), // 1000
   };
   const props = {
     pubKey,
@@ -49,19 +44,18 @@ describe('TransferForm', () => {
       pubKey,
       'a4b6e3de5d7ef4e31ae1b090be49d1a2ef7bcefff60cccf7658a0d4922651cce',
     ],
-    assets: [asset],
     feeFactor: '0.001',
     submitTransfer: jest.fn(),
     accounts: [
       {
         type: AccountType.ACCOUNT_TYPE_GENERAL,
         asset,
-        balance: '100',
+        balance: '100000',
       },
       {
         type: AccountType.ACCOUNT_TYPE_VESTED_REWARDS,
         asset,
-        balance: '100',
+        balance: '100000',
       },
     ],
   };
@@ -75,7 +69,7 @@ describe('TransferForm', () => {
     render(<TransferForm {...props} />);
     // Select a pubkey
     await userEvent.selectOptions(
-      screen.getByLabelText('Vega key'),
+      screen.getByLabelText('To Vega key'),
       props.pubKeys[1]
     );
 
@@ -121,13 +115,19 @@ describe('TransferForm', () => {
     // 1003-TRAN-004
     render(<TransferForm {...props} />);
     await submit();
-    expect(await screen.findAllByText('Required')).toHaveLength(4);
+    expect(await screen.findAllByText('Required')).toHaveLength(3); // pubkey is set as default value
     const toggle = screen.getByText('Enter manually');
     await userEvent.click(toggle);
     // has switched to input
     expect(toggle).toHaveTextContent('Select from wallet');
-    expect(screen.getByLabelText('Vega key')).toHaveAttribute('type', 'text');
+    expect(screen.getByLabelText('To Vega key')).toHaveAttribute(
-    await userEvent.type(screen.getByLabelText('Vega key'), 'invalid-address');
+      'type',
+      'text'
+    );
+    await userEvent.type(
+      screen.getByLabelText('To Vega key'),
+      'invalid-address'
+    );
     expect(screen.getAllByTestId('input-error-text')[0]).toHaveTextContent(
       'Invalid Vega key'
     );
@@ -142,7 +142,7 @@ describe('TransferForm', () => {
     render(<TransferForm {...props} />);
 
     // check current pubkey not shown
-    const keySelect = screen.getByLabelText<HTMLSelectElement>('Vega key');
+    const keySelect = screen.getByLabelText<HTMLSelectElement>('To Vega key');
     expect(keySelect.children).toHaveLength(3);
     expect(Array.from(keySelect.options).map((o) => o.value)).toEqual([
       '',
@@ -151,25 +151,17 @@ describe('TransferForm', () => {
     ]);
 
     await submit();
-    expect(await screen.findAllByText('Required')).toHaveLength(4);
+    expect(await screen.findAllByText('Required')).toHaveLength(3); // pubkey is set as default value
 
     // Select a pubkey
     await userEvent.selectOptions(
-      screen.getByLabelText('Vega key'),
+      screen.getByLabelText('To Vega key'),
       props.pubKeys[1]
     );
 
     // Select asset
     await selectAsset(asset);
 
-    // assert rich select as updated
-    expect(await screen.findByTestId('select-asset')).toHaveTextContent(
-      asset.name
-    );
-    expect(await screen.findByTestId('asset-balance')).toHaveTextContent(
-      formatNumber(asset.balance, asset.decimals)
-    );
-
     await userEvent.selectOptions(
       screen.getByLabelText('From account'),
       AccountType.ACCOUNT_TYPE_VESTED_REWARDS
@@ -217,7 +209,7 @@ describe('TransferForm', () => {
       render(<TransferForm {...props} submitTransfer={mockSubmit} />);
 
       // check current pubkey not shown
-      const keySelect = screen.getByLabelText<HTMLSelectElement>('Vega key');
+      const keySelect = screen.getByLabelText<HTMLSelectElement>('To Vega key');
       const pubKeyOptions = ['', pubKey, props.pubKeys[1]];
       expect(keySelect.children).toHaveLength(pubKeyOptions.length);
       expect(Array.from(keySelect.options).map((o) => o.value)).toEqual(
@@ -225,11 +217,11 @@ describe('TransferForm', () => {
       );
 
       await submit();
-      expect(await screen.findAllByText('Required')).toHaveLength(4);
+      expect(await screen.findAllByText('Required')).toHaveLength(3); // pubkey set as default value
 
       // Select a pubkey
       await userEvent.selectOptions(
-        screen.getByLabelText('Vega key'),
+        screen.getByLabelText('To Vega key'),
         props.pubKeys[1]
       );
 
@@ -286,7 +278,7 @@ describe('TransferForm', () => {
       render(<TransferForm {...props} />);
 
       // check current pubkey not shown
-      const keySelect: HTMLSelectElement = screen.getByLabelText('Vega key');
+      const keySelect: HTMLSelectElement = screen.getByLabelText('To Vega key');
       const pubKeyOptions = ['', pubKey, props.pubKeys[1]];
       expect(keySelect.children).toHaveLength(pubKeyOptions.length);
       expect(Array.from(keySelect.options).map((o) => o.value)).toEqual(
@@ -294,11 +286,11 @@ describe('TransferForm', () => {
       );
 
       await submit();
-      expect(await screen.findAllByText('Required')).toHaveLength(4);
+      expect(await screen.findAllByText('Required')).toHaveLength(3); // pubkey set as default value
 
       // Select a pubkey
       await userEvent.selectOptions(
-        screen.getByLabelText('Vega key'),
+        screen.getByLabelText('To Vega key'),
         props.pubKeys[1]
       );
 
@@ -324,7 +316,6 @@ describe('TransferForm', () => {
 
   describe('AddressField', () => {
     const props = {
-      pubKeys: ['pubkey-1', 'pubkey-2'],
       select: <div>select</div>,
       input: <div>input</div>,
       onChange: jest.fn(),
@@ -334,7 +325,7 @@ describe('TransferForm', () => {
       const mockOnChange = jest.fn();
       render(<AddressField {...props} onChange={mockOnChange} />);
 
-      // select should be shown as multiple pubkeys provided
+      // select should be shown by default
       expect(screen.getByText('select')).toBeInTheDocument();
       expect(screen.queryByText('input')).not.toBeInTheDocument();
       await userEvent.click(screen.getByText('Enter manually'));
@@ -346,12 +337,6 @@ describe('TransferForm', () => {
       expect(screen.queryByText('input')).not.toBeInTheDocument();
       expect(mockOnChange).toHaveBeenCalledTimes(2);
     });
-
-    it('Does not provide select option if there is only a single key', () => {
-      render(<AddressField {...props} pubKeys={['single-pubKey']} />);
-      expect(screen.getByText('input')).toBeInTheDocument();
-      expect(screen.queryByText('Select from wallet')).not.toBeInTheDocument();
-    });
   });
 
   describe('TransferFee', () => {

libs/accounts/src/lib/transfer-form.tsx

@@ -1,3 +1,4 @@
+import sortBy from 'lodash/sortBy';
 import {
   minSafe,
   maxSafe,
@@ -28,28 +29,19 @@ import { AssetOption, Balance } from '@vegaprotocol/assets';
 import { AccountType, AccountTypeMapping } from '@vegaprotocol/types';
 
 interface FormFields {
-  toAddress: string;
+  toVegaKey: string;
   asset: string;
   amount: string;
   fromAccount: AccountType;
 }
 
-interface Asset {
-  id: string;
-  symbol: string;
-  name: string;
-  decimals: number;
-  balance: string;
-}
-
 interface TransferFormProps {
   pubKey: string | null;
   pubKeys: string[] | null;
-  assets: Array<Asset>;
   accounts: Array<{
     type: AccountType;
     balance: string;
-    asset: { id: string; symbol: string; decimals: number };
+    asset: { id: string; symbol: string; name: string; decimals: number };
   }>;
   assetId?: string;
   feeFactor: string | null;
@@ -59,7 +51,6 @@ interface TransferFormProps {
 export const TransferForm = ({
   pubKey,
   pubKeys,
-  assets,
   assetId: initialAssetId,
   feeFactor,
   submitTransfer,
@@ -75,16 +66,50 @@ export const TransferForm = ({
   } = useForm<FormFields>({
     defaultValues: {
       asset: initialAssetId,
+      toVegaKey: pubKey || '',
     },
   });
 
-  const selectedPubKey = watch('toAddress');
+  const assets = sortBy(
+    accounts
+      .filter((a) => a.type === AccountType.ACCOUNT_TYPE_GENERAL)
+      .map((account) => ({
+        ...account.asset,
+        balance: addDecimal(account.balance, account.asset.decimals),
+      })),
+    'name'
+  );
+
+  const selectedPubKey = watch('toVegaKey');
   const amount = watch('amount');
+  const fromAccount = watch('fromAccount');
   const assetId = watch('asset');
+
   const asset = assets.find((a) => a.id === assetId);
 
+  const account = accounts.find(
+    (a) => a.asset.id === assetId && a.type === fromAccount
+  );
+  const accountBalance =
+    account && addDecimal(account.balance, account.asset.decimals);
+
+  // General account for the selected asset
+  const generalAccount = accounts.find((a) => {
+    return (
+      a.asset.id === assetId && a.type === AccountType.ACCOUNT_TYPE_GENERAL
+    );
+  });
+
   const [includeFee, setIncludeFee] = useState(false);
 
+  // Min viable amount given asset decimals EG for WEI 0.000000000000000001
+  const min = asset
+    ? new BigNumber(addDecimal('1', asset.decimals))
+    : new BigNumber(0);
+
+  // Max amount given selected asset and from account
+  const max = accountBalance ? new BigNumber(accountBalance) : new BigNumber(0);
+
   const transferAmount = useMemo(() => {
     if (!amount) return undefined;
     if (includeFee && feeFactor) {
@@ -112,7 +137,7 @@ export const TransferForm = ({
         throw new Error('Submitted transfer with no amount selected');
       }
       const transfer = normalizeTransfer(
-        fields.toAddress,
+        fields.toVegaKey,
         transferAmount,
         fields.fromAccount,
         AccountType.ACCOUNT_TYPE_GENERAL, // field is readonly in the form
@@ -126,19 +151,6 @@ export const TransferForm = ({
     [asset, submitTransfer, transferAmount]
   );
 
-  const min = useMemo(() => {
-    // Min viable amount given asset decimals EG for WEI 0.000000000000000001
-    const minViableAmount = asset
-      ? new BigNumber(addDecimal('1', asset.decimals))
-      : new BigNumber(0);
-    return minViableAmount;
-  }, [asset]);
-
-  const max = useMemo(() => {
-    const maxAmount = asset ? new BigNumber(asset.balance) : new BigNumber(0);
-    return maxAmount;
-  }, [asset]);
-
   // reset for placeholder workaround https://github.com/radix-ui/primitives/issues/1569
   useEffect(() => {
     if (!pubKey) {
@@ -146,34 +158,21 @@ export const TransferForm = ({
     }
   }, [setValue, pubKey]);
 
-  // General account for the selected asset
-  const generalAccount = accounts.find((a) => {
-    return (
-      a.asset.id === assetId && a.type === AccountType.ACCOUNT_TYPE_GENERAL
-    );
-  });
-
   return (
     <form
       onSubmit={handleSubmit(onSubmit)}
       className="text-sm"
       data-testid="transfer-form"
     >
-      <TradingFormGroup label="Vega key" labelFor="toAddress">
+      <TradingFormGroup label="To Vega key" labelFor="toVegaKey">
         <AddressField
-          pubKeys={pubKeys}
+          onChange={() => setValue('toVegaKey', '')}
-          onChange={() => setValue('toAddress', '')}
           select={
-            <TradingSelect
+            <TradingSelect {...register('toVegaKey')} id="toVegaKey">
-              {...register('toAddress')}
-              id="toAddress"
-              defaultValue=""
-            >
               <option value="" disabled={true}>
                 {t('Please select')}
               </option>
-              {pubKeys?.length &&
+              {pubKeys?.map((pk) => {
-                pubKeys.map((pk) => {
                 const text = pk === pubKey ? t('Current key: ') + pk : pk;
 
                 return (
@@ -188,9 +187,9 @@ export const TransferForm = ({
             <TradingInput
               // eslint-disable-next-line jsx-a11y/no-autofocus
               autoFocus={true} // focus input immediately after is shown
-              id="toAddress"
+              id="toVegaKey"
               type="text"
-              {...register('toAddress', {
+              {...register('toVegaKey', {
                 validate: {
                   required,
                   vegaPublicKey,
@@ -199,9 +198,9 @@ export const TransferForm = ({
             />
           }
         />
-        {errors.toAddress?.message && (
+        {errors.toVegaKey?.message && (
-          <TradingInputError forInput="toAddress">
+          <TradingInputError forInput="toVegaKey">
-            {errors.toAddress.message}
+            {errors.toVegaKey.message}
           </TradingInputError>
         )}
       </TradingFormGroup>
@@ -322,15 +321,24 @@ export const TransferForm = ({
               maxSafe: (v) => {
                 const value = new BigNumber(v);
                 if (value.isGreaterThan(max)) {
-                  return t(
+                  return t('You cannot transfer more than available');
-                    'You cannot transfer more than your available collateral'
-                  );
                 }
                 return maxSafe(max)(v);
               },
             },
           })}
         />
+        {accountBalance && (
+          <button
+            type="button"
+            className="absolute top-0 right-0 ml-auto text-xs underline"
+            onClick={() =>
+              setValue('amount', parseFloat(accountBalance).toString())
+            }
+          >
+            {t('Use max')}
+          </button>
+        )}
         {errors.amount?.message && (
           <TradingInputError forInput="amount">
             {errors.amount.message}
@@ -442,29 +450,21 @@ export const TransferFee = ({
 };
 
 interface AddressInputProps {
-  pubKeys: string[] | null;
   select: ReactNode;
   input: ReactNode;
   onChange: () => void;
 }
 
 export const AddressField = ({
-  pubKeys,
   select,
   input,
   onChange,
 }: AddressInputProps) => {
-  const [isInput, setIsInput] = useState(() => {
+  const [isInput, setIsInput] = useState(false);
-    if (pubKeys && pubKeys.length <= 1) {
-      return true;
-    }
-    return false;
-  });
 
   return (
     <>
       {isInput ? input : select}
-      {pubKeys && pubKeys.length > 1 && (
       <button
         type="button"
         onClick={() => {
@@ -475,7 +475,6 @@ export const AddressField = ({
       >
         {isInput ? t('Select from wallet') : t('Enter manually')}
       </button>
-      )}
     </>
   );
 };