Improve confirm screen tests (#15163)

* Improve confirm screen tests

* Fix transaction controller unit test

* Fix unit test fixtures

* Fix e2e test

app/scripts/controllers/transactions/index.test.js

@@ -2274,6 +2274,7 @@ describe('Transaction Controller', function () {
     });
 
     it('updates editible params when type changes from simple send to token transfer', async function () {
+      providerResultStub.eth_getCode = '0xab';
       // test update gasFees
       await txController.updateEditableParams('1', {
         data:

shared/modules/transaction.utils.js

@@ -148,33 +148,35 @@ export async function determineTransactionType(txParams, query) {
     log.debug('Failed to parse transaction data.', error, data);
   }
 
-  const tokenMethodName = [
-    TRANSACTION_TYPES.TOKEN_METHOD_APPROVE,
-    TRANSACTION_TYPES.TOKEN_METHOD_SET_APPROVAL_FOR_ALL,
-    TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER,
-    TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER_FROM,
-    TRANSACTION_TYPES.TOKEN_METHOD_SAFE_TRANSFER_FROM,
-  ].find((methodName) => isEqualCaseInsensitive(methodName, name));
-
   let result;
-  if (data && tokenMethodName) {
-    result = tokenMethodName;
-  } else if (data && !to) {
-    result = TRANSACTION_TYPES.DEPLOY_CONTRACT;
-  }
-
   let contractCode;
 
-  if (!result) {
+  if (data && !to) {
+    result = TRANSACTION_TYPES.DEPLOY_CONTRACT;
+  } else {
     const {
       contractCode: resultCode,
       isContractAddress,
     } = await readAddressAsContract(query, to);
 
     contractCode = resultCode;
-    result = isContractAddress
-      ? TRANSACTION_TYPES.CONTRACT_INTERACTION
-      : TRANSACTION_TYPES.SIMPLE_SEND;
+
+    if (isContractAddress) {
+      const tokenMethodName = [
+        TRANSACTION_TYPES.TOKEN_METHOD_APPROVE,
+        TRANSACTION_TYPES.TOKEN_METHOD_SET_APPROVAL_FOR_ALL,
+        TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER,
+        TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER_FROM,
+        TRANSACTION_TYPES.TOKEN_METHOD_SAFE_TRANSFER_FROM,
+      ].find((methodName) => isEqualCaseInsensitive(methodName, name));
+
+      result =
+        data && tokenMethodName
+          ? tokenMethodName
+          : TRANSACTION_TYPES.CONTRACT_INTERACTION;
+    } else {
+      result = TRANSACTION_TYPES.SIMPLE_SEND;
+    }
   }
 
   return { type: result, getCodeResponse: contractCode };

shared/modules/transaction.utils.test.js

@@ -111,13 +111,23 @@ describe('Transaction.utils', function () {
     const genericProvider = createTestProviderTools().provider;
     const query = new EthQuery(genericProvider);
 
-    it('should return a simple send type when to is truthy but data is falsy', async function () {
+    it('should return a simple send type when to is truthy and is not a contract address', async function () {
+      const _providerResultStub = {
+        // 1 gwei
+        eth_gasPrice: '0x0de0b6b3a7640000',
+        // by default, all accounts are external accounts (not contracts)
+        eth_getCode: '0x',
+      };
+      const _provider = createTestProviderTools({
+        scaffold: _providerResultStub,
+      }).provider;
+
       const result = await determineTransactionType(
         {
           to: '0xabc',
           data: '',
         },
-        query,
+        new EthQuery(_provider),
       );
       expect(result).toMatchObject({
         type: TRANSACTION_TYPES.SIMPLE_SEND,
@@ -125,33 +135,78 @@ describe('Transaction.utils', function () {
       });
     });
 
-    it('should return a token transfer type when data is for the respective method call', async function () {
+    it('should return a token transfer type when the recipient is a contract and data is for the respective method call', async function () {
+      const _providerResultStub = {
+        // 1 gwei
+        eth_gasPrice: '0x0de0b6b3a7640000',
+        // by default, all accounts are external accounts (not contracts)
+        eth_getCode: '0xab',
+      };
+      const _provider = createTestProviderTools({
+        scaffold: _providerResultStub,
+      }).provider;
+
       const result = await determineTransactionType(
         {
-          to: '0xabc',
+          to: '0x9e673399f795D01116e9A8B2dD2F156705131ee9',
           data:
             '0xa9059cbb0000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C970000000000000000000000000000000000000000000000000000000000000000a',
         },
-        query,
+        new EthQuery(_provider),
       );
       expect(result).toMatchObject({
         type: TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER,
-        getCodeResponse: undefined,
+        getCodeResponse: '0xab',
       });
     });
 
-    it('should return a token approve type when data is for the respective method call', async function () {
+    it('should NOT return a token transfer type when the recipient is not a contract but the data matches the respective method call', async function () {
+      const _providerResultStub = {
+        // 1 gwei
+        eth_gasPrice: '0x0de0b6b3a7640000',
+        // by default, all accounts are external accounts (not contracts)
+        eth_getCode: '0x',
+      };
+      const _provider = createTestProviderTools({
+        scaffold: _providerResultStub,
+      }).provider;
+
       const result = await determineTransactionType(
         {
-          to: '0xabc',
+          to: '0x9e673399f795D01116e9A8B2dD2F156705131ee9',
+          data:
+            '0xa9059cbb0000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C970000000000000000000000000000000000000000000000000000000000000000a',
+        },
+        new EthQuery(_provider),
+      );
+      expect(result).toMatchObject({
+        type: TRANSACTION_TYPES.SIMPLE_SEND,
+        getCodeResponse: '0x',
+      });
+    });
+
+    it('should return a token approve type when when the recipient is a contract and data is for the respective method call', async function () {
+      const _providerResultStub = {
+        // 1 gwei
+        eth_gasPrice: '0x0de0b6b3a7640000',
+        // by default, all accounts are external accounts (not contracts)
+        eth_getCode: '0xab',
+      };
+      const _provider = createTestProviderTools({
+        scaffold: _providerResultStub,
+      }).provider;
+
+      const result = await determineTransactionType(
+        {
+          to: '0x9e673399f795D01116e9A8B2dD2F156705131ee9',
           data:
             '0x095ea7b30000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C9700000000000000000000000000000000000000000000000000000000000000005',
         },
-        query,
+        new EthQuery(_provider),
       );
       expect(result).toMatchObject({
         type: TRANSACTION_TYPES.TOKEN_METHOD_APPROVE,
-        getCodeResponse: undefined,
+        getCodeResponse: '0xab',
       });
     });
 
@@ -184,12 +239,22 @@ describe('Transaction.utils', function () {
     });
 
     it('should return a simple send type with a null getCodeResponse when to is truthy and there is data and but getCode returns an error', async function () {
+      const _providerResultStub = {
+        // 1 gwei
+        eth_gasPrice: '0x0de0b6b3a7640000',
+        // by default, all accounts are external accounts (not contracts)
+        eth_getCode: null,
+      };
+      const _provider = createTestProviderTools({
+        scaffold: _providerResultStub,
+      }).provider;
+
       const result = await determineTransactionType(
         {
-          to: '0xabc',
+          to: '0x9e673399f795D01116e9A8B2dD2F156705131ee9',
           data: '0xabd',
         },
-        query,
+        new EthQuery(_provider),
       );
       expect(result).toMatchObject({
         type: TRANSACTION_TYPES.SIMPLE_SEND,

test/e2e/fixtures/special-settings/state.json

@@ -0,0 +1,146 @@
+{
+  "data": {
+    "AppStateController": {
+      "mkrMigrationReminderTimestamp": null
+    },
+    "CachedBalancesController": {
+      "cachedBalances": {
+        "4": {}
+      }
+    },
+    "CurrencyController": {
+      "conversionDate": 1575697244.188,
+      "conversionRate": 149.61,
+      "currentCurrency": "usd",
+      "nativeCurrency": "ETH"
+    },
+    "IncomingTransactionsController": {
+      "incomingTransactions": {},
+      "incomingTxLastFetchedBlocksByNetwork": {
+        "goerli": null,
+        "kovan": null,
+        "mainnet": null,
+        "rinkeby": 5570536
+      }
+    },
+    "KeyringController": {
+      "vault": "{\"data\":\"s6TpYjlUNsn7ifhEFTkuDGBUM1GyOlPrim7JSjtfIxgTt8/6MiXgiR/CtFfR4dWW2xhq85/NGIBYEeWrZThGdKGarBzeIqBfLFhw9n509jprzJ0zc2Rf+9HVFGLw+xxC4xPxgCS0IIWeAJQ+XtGcHmn0UZXriXm8Ja4kdlow6SWinB7sr/WM3R0+frYs4WgllkwggDf2/Tv6VHygvLnhtzp6hIJFyTjh+l/KnyJTyZW1TkZhDaNDzX3SCOHT\",\"iv\":\"FbeHDAW5afeWNORfNJBR0Q==\",\"salt\":\"TxZ+WbCW6891C9LK/hbMAoUsSEW1E8pyGLVBU6x5KR8=\"}"
+    },
+    "NetworkController": {
+      "network": "1337",
+      "provider": {
+        "nickname": "Localhost 8545",
+        "rpcUrl": "http://localhost:8545",
+        "chainId": "0x539",
+        "ticker": "ETH",
+        "type": "rpc"
+      }
+    },
+    "NotificationController": {
+      "notifications": {
+        "1": {
+          "isShown": true
+        },
+        "3": {
+          "isShown": true
+        },
+        "5": {
+          "isShown": true
+        },
+        "6": {
+          "isShown": true
+        },
+        "8": {
+          "isShown": true
+        },
+        "12": {
+          "isShown": true
+        }
+      }
+    },
+    "OnboardingController": {
+      "onboardingTabs": {},
+      "seedPhraseBackedUp": false
+    },
+    "PermissionsMetadata": {
+      "domainMetadata": {
+        "metamask.github.io": {
+          "icon": null,
+          "name": "M E T A M A S K M E S H T E S T"
+        }
+      },
+      "permissionsHistory": {},
+      "permissionsLog": [
+        {
+          "id": 746677923,
+          "method": "eth_accounts",
+          "methodType": "restricted",
+          "origin": "metamask.github.io",
+          "request": {
+            "id": 746677923,
+            "jsonrpc": "2.0",
+            "method": "eth_accounts",
+            "origin": "metamask.github.io",
+            "params": []
+          },
+          "requestTime": 1575697241368,
+          "response": {
+            "id": 746677923,
+            "jsonrpc": "2.0",
+            "result": []
+          },
+          "responseTime": 1575697241370,
+          "success": true
+        }
+      ]
+    },
+    "PreferencesController": {
+      "accountTokens": {
+        "0x5cfe73b6021e818b776b421b1c4db2474086a7e1": {
+          "rinkeby": [],
+          "ropsten": []
+        }
+      },
+      "assetImages": {},
+      "completedOnboarding": true,
+      "dismissSeedBackUpReminder": true,
+      "currentLocale": "en",
+      "featureFlags": {
+        "showIncomingTransactions": true,
+        "transactionTime": false,
+        "sendHexData": true
+      },
+      "firstTimeFlowType": "create",
+      "forgottenPassword": false,
+      "frequentRpcListDetail": [],
+      "identities": {
+        "0x5cfe73b6021e818b776b421b1c4db2474086a7e1": {
+          "address": "0x5cfe73b6021e818b776b421b1c4db2474086a7e1",
+          "name": "Account 1"
+        }
+      },
+      "knownMethodData": {},
+      "lostIdentities": {},
+      "metaMetricsId": null,
+      "participateInMetaMetrics": false,
+      "preferences": {
+        "useNativeCurrencyAsPrimaryCurrency": true
+      },
+      "selectedAddress": "0x5cfe73b6021e818b776b421b1c4db2474086a7e1",
+      "suggestedTokens": {},
+      "tokens": [],
+      "useBlockie": false,
+      "useNonceField": false,
+      "usePhishDetect": true,
+      "useTokenDetection": true
+    },
+    "config": {},
+    "firstTimeInfo": {
+      "date": 1575697234195,
+      "version": "7.7.0"
+    }
+  },
+  "meta": {
+    "version": 40
+  }
+}

test/e2e/tests/send-eth.spec.js

@@ -92,6 +92,54 @@ describe('Send ETH from inside MetaMask using default gas', function () {
   });
 });
 
+describe('Send ETH non-contract address with data that matches ERC20 transfer data signature', function () {
+  const ganacheOptions = {
+    accounts: [
+      {
+        secretKey:
+          '0x7C9529A67102755B7E6102D6D950AC5D5863C98713805CEC576B945B15B71EAC',
+        balance: convertToHexValue(25000000000000000000),
+      },
+    ],
+  };
+  it('renders the correct recipient on the confirmation screen', async function () {
+    await withFixtures(
+      {
+        fixtures: 'special-settings',
+        ganacheOptions,
+        title: this.test.title,
+      },
+      async ({ driver }) => {
+        await driver.navigate();
+        await driver.fill('#password', 'correct horse battery staple');
+        await driver.press('#password', driver.Key.ENTER);
+
+        await driver.clickElement('[data-testid="eth-overview-send"]');
+
+        await driver.fill(
+          'input[placeholder="Search, public address (0x), or ENS"]',
+          '0xc427D562164062a23a5cFf596A4a3208e72Acd28',
+        );
+
+        await driver.fill(
+          'textarea[placeholder="Optional',
+          '0xa9059cbb0000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C970000000000000000000000000000000000000000000000000000000000000000a',
+        );
+
+        await driver.clickElement({ text: 'Next', tag: 'button' });
+
+        await driver.clickElement({ text: '0xc42...cd28' });
+
+        const recipientAddress = await driver.findElements({
+          text: '0xc427D562164062a23a5cFf596A4a3208e72Acd28',
+        });
+
+        assert.equal(recipientAddress.length, 1);
+      },
+    );
+  });
+});
+
 /* eslint-disable-next-line mocha/max-top-level-suites */
 describe('Send ETH from inside MetaMask using advanced gas modal', function () {
   const ganacheOptions = {

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

@@ -54,6 +54,7 @@ import { toChecksumHexAddress } from '../../../shared/modules/hexstring-utils';
 import { getGasLoadingAnimationIsShowing } from '../../ducks/app/app';
 import { isLegacyTransaction } from '../../helpers/utils/transactions.util';
 import { CUSTOM_GAS_ESTIMATE } from '../../../shared/constants/gas';
+import { TRANSACTION_TYPES } from '../../../shared/constants/transaction';
 import { isEqualCaseInsensitive } from '../../../shared/modules/string-utils';
 import { getTokenAddressParam } from '../../helpers/utils/token-util';
 import ConfirmTransactionBase from './confirm-transaction-base.component';
@@ -112,7 +113,10 @@ const mapStateToProps = (state, ownProps) => {
 
   const { balance } = accounts[fromAddress];
   const { name: fromName } = identities[fromAddress];
-  const toAddress = propsToAddress || tokenToAddress || txParamsToAddress;
+  let toAddress = txParamsToAddress;
+  if (type !== TRANSACTION_TYPES.SIMPLE_SEND) {
+    toAddress = propsToAddress || tokenToAddress || txParamsToAddress;
+  }
 
   const tokenList = getTokenList(state);
   const useTokenDetection = getUseTokenDetection(state);