Merge branch 'develop' into flaky-swap-send-pending

test/e2e/tests/onboarding/onboarding.spec.js

@@ -372,7 +372,7 @@ describe('MetaMask onboarding @no-mmi', function () {
               json: {
                 jsonrpc: '2.0',
                 id: '1111111111111111',
-                result: '0x1',
+                result: '0x0',
               },
             };
           }),
@@ -508,7 +508,7 @@ describe('MetaMask onboarding @no-mmi', function () {
               json: {
                 jsonrpc: '2.0',
                 id: '1111111111111111',
-                result: '0x1',
+                result: '0x0',
               },
             };
           }),

ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.container.js

@@ -85,10 +85,7 @@ import {
   parseStandardTokenTransactionData,
   txParamsAreDappSuggested,
 } from '../../../../shared/modules/transaction.utils';
-import {
-  isEmptyHexString,
-  toChecksumHexAddress,
-} from '../../../../shared/modules/hexstring-utils';
+import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils';
 
 import { getGasLoadingAnimationIsShowing } from '../../../ducks/app/app';
 import { isLegacyTransaction } from '../../../helpers/utils/transactions.util';
@@ -108,7 +105,6 @@ import {
 } from '../../../selectors/institutional/selectors';
 import { showCustodyConfirmLink } from '../../../store/institutional/institution-actions';
 ///: END:ONLY_INCLUDE_IF
-import { getTokenAddressParam } from '../../../helpers/utils/token-util';
 import { calcGasTotal } from '../../../../shared/lib/transactions-controller-utils';
 import { subtractHexes } from '../../../../shared/modules/conversion.utils';
 import ConfirmTransactionBase from './confirm-transaction-base.component';
@@ -131,6 +127,24 @@ function addressIsNew(toAccounts, newAddress) {
   return !foundMatching;
 }
 
+function getTokenToAddress(data, type) {
+  if (
+    ![
+      TransactionType.tokenMethodTransferFrom,
+      TransactionType.tokenMethodSafeTransferFrom,
+      TransactionType.tokenMethodTransfer,
+    ].includes(type)
+  ) {
+    return undefined;
+  }
+
+  const transactionData = parseStandardTokenTransactionData(data);
+
+  const value = transactionData?.args?._to || transactionData?.args?.to;
+
+  return value?.toString().toLowerCase();
+}
+
 const mapStateToProps = (state, ownProps) => {
   const {
     toAddress: propsToAddress,
@@ -163,28 +177,20 @@ const mapStateToProps = (state, ownProps) => {
     to: txParamsToAddress,
     gasPrice,
     gas: gasLimit,
-    value: amount,
     data,
   } = (transaction && transaction.txParams) || txParams;
   const accounts = getMetaMaskAccounts(state);
   const smartTransactionsOptInStatus = getSmartTransactionsOptInStatus(state);
   const currentChainSupportsSmartTransactions =
     getCurrentChainSupportsSmartTransactions(state);
 
-  const transactionData = parseStandardTokenTransactionData(data);
-  const tokenToAddress = getTokenAddressParam(transactionData);
-
   const { balance } = accounts[fromAddress];
   const fromInternalAccount = getInternalAccountByAddress(state, fromAddress);
   const fromName = fromInternalAccount?.metadata.name;
   const keyring = findKeyringForAddress(state, fromAddress);
 
-  const isSendingAmount =
-    type === TransactionType.simpleSend || !isEmptyHexString(amount);
-
-  const toAddress = isSendingAmount
-    ? txParamsToAddress
-    : propsToAddress || tokenToAddress || txParamsToAddress;
+  const tokenToAddress = getTokenToAddress(data, type);
+  const toAddress = propsToAddress || tokenToAddress || txParamsToAddress;
 
   const toAccounts = getSendToAccounts(state);
 

ui/pages/confirmations/confirm-transaction-base/confirm-transaction-base.test.js

@@ -775,7 +775,7 @@ describe('Confirm Transaction Base', () => {
           TransactionType.contractInteraction;
       });
 
-      describe('when there is an amount being sent (it should be treated as a general contract intereaction rather than custom one)', () => {
+      describe('when there is an amount being sent (it should be treated as a general contract interaction rather than custom one)', () => {
         it('should use txParams.to address (contract address)', async () => {
           const state = mockedStoreWithConfirmTxParams(baseStore, {
             ...mockTxParams,
@@ -795,33 +795,69 @@ describe('Confirm Transaction Base', () => {
         });
       });
 
-      describe(`when there is no amount being sent`, () => {
-        it('should use propToAddress (toAddress passed as prop)', async () => {
+      describe('when determines the recipient from transaction data args', () => {
+        const testCases = [
+          {
+            type: TransactionType.tokenMethodTransfer,
+            data: `0xa9059cbb000000000000000000000000${mockParsedTxDataToAddressWithout0x}0000000000000000000000000000000000000000000000000000000000000001`,
+            description: 'tokenMethodTransfer',
+          },
+          {
+            type: TransactionType.tokenMethodSafeTransferFrom,
+            data: `0x42842e0e000000000000000000000000806627172af48bd5b0765d3449a7def80d6576ff000000000000000000000000${mockParsedTxDataToAddressWithout0x}000000000000000000000000000000000000000000000000000000000009a7cc`,
+            description: 'tokenMethodSafeTransferFrom',
+          },
+          {
+            type: TransactionType.tokenMethodTransferFrom,
+            data: `0x23b872dd000000000000000000000000ac9539a7d5c43940af498008a7c8f3abb35c3725000000000000000000000000${mockParsedTxDataToAddressWithout0x}000000000000000000000000000000000000000000000000000000000009a7b8`,
+            description: 'tokenMethodTransferFrom',
+          },
+        ];
+
+        it.each(testCases)(
+          'identifies correctly the recipient for $description transactions',
+          async ({ type, data }) => {
+            const state = mockedStoreWithConfirmTxParams(baseStore, {
+              ...mockTxParams,
+              data,
+            });
+            state.confirmTransaction.txData = {
+              ...state.confirmTransaction.txData,
+              type,
+            };
+
+            const { container } = await render({ state });
+
+            const recipientElem = container.querySelector(
+              sendToRecipientSelector,
+            );
+            expect(recipientElem).toHaveTextContent(mockParsedTxDataToAddress);
+          },
+        );
+      });
+
+      describe('when a non-transfer function matching the ABIs', () => {
+        it('does not determine the recipient from transaction data args', async () => {
           const state = mockedStoreWithConfirmTxParams(baseStore, {
             ...mockTxParams,
-            value: '0x0',
+            data: `0xa22cb465000000000000000000000000${mockParsedTxDataToAddressWithout0x}0000000000000000000000000000000000000000000000000000000000000001`,
           });
           state.confirmTransaction.txData = {
             ...state.confirmTransaction.txData,
             type: TransactionType.contractInteraction,
           };
 
-          const props = {
-            // we want to test toAddress provided by ownProps in mapStateToProps, but this
-            // currently overrides toAddress this should pan out fine when we refactor the
-            // component into a functional component and remove the container.js file
-            toAddress: mockPropsToAddress,
-          };
-
-          const { container } = await render({ props, state });
+          const { container } = await render({ state });
 
           const recipientElem = container.querySelector(
             sendToRecipientSelector,
           );
-          expect(recipientElem).toHaveTextContent(mockPropsToAddressConcat);
+          expect(recipientElem).toHaveTextContent(mockTxParamsToAddressConcat);
         });
+      });
 
-        it('should use address parsed from transaction data if propToAddress is not provided', async () => {
+      describe(`when there is no amount being sent`, () => {
+        it('should use propToAddress (toAddress passed as prop)', async () => {
           const state = mockedStoreWithConfirmTxParams(baseStore, {
             ...mockTxParams,
             value: '0x0',
@@ -831,14 +867,19 @@ describe('Confirm Transaction Base', () => {
             type: TransactionType.contractInteraction,
           };
 
-          const props = {};
+          const props = {
+            // we want to test toAddress provided by ownProps in mapStateToProps, but this
+            // currently overrides toAddress this should pan out fine when we refactor the
+            // component into a functional component and remove the container.js file
+            toAddress: mockPropsToAddress,
+          };
 
           const { container } = await render({ props, state });
 
           const recipientElem = container.querySelector(
             sendToRecipientSelector,
           );
-          expect(recipientElem).toHaveTextContent(mockParsedTxDataToAddress);
+          expect(recipientElem).toHaveTextContent(mockPropsToAddressConcat);
         });
 
         it('should use txParams.to if neither propToAddress is not provided nor the transaction data to address were provided', async () => {

ui/pages/confirmations/confirm-transaction/confirm-transaction.transaction.test.js

@@ -54,7 +54,7 @@ describe('Confirm Transaction', () => {
         )),
     );
 
-    expect(result.getByText('0xb19Ac...f0c5e')).toBeInTheDocument();
+    expect(result.getByText('Ledger Hardware 2')).toBeInTheDocument();
     expect(result.getByRole('button', { name: 'Details' })).toBeInTheDocument();
     expect(result.getByRole('button', { name: 'Hex' })).toBeInTheDocument();
   });