call controller methods directly in send duck

app/scripts/controllers/transactions/index.js

@@ -50,6 +50,7 @@ import {
   determineTransactionType,
   isEIP1559Transaction,
 } from '../../../../shared/modules/transaction.utils';
+import { ORIGIN_METAMASK } from '../../../../shared/constants/app';
 import TransactionStateManager from './tx-state-manager';
 import TxGasUtil from './tx-gas-utils';
 import PendingTransactionTracker from './pending-tx-tracker';
@@ -62,6 +63,15 @@ const SWAP_TRANSACTION_TYPES = [
   TRANSACTION_TYPES.SWAP_APPROVAL,
 ];
 
+// Only certain types of transactions should be allowed to be specified when
+// adding a new unapproved transaction.
+const VALID_UNAPPROVED_TRANSACTION_TYPES = [
+  ...SWAP_TRANSACTION_TYPES,
+  TRANSACTION_TYPES.SIMPLE_SEND,
+  TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER,
+  TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER_FROM,
+];
+
 /**
  * @typedef {import('../../../../shared/constants/transaction').TransactionMeta} TransactionMeta
  * @typedef {import('../../../../shared/constants/transaction').TransactionMetaMetricsEventString} TransactionMetaMetricsEventString
@@ -650,7 +660,7 @@ export default class TransactionController extends EventEmitter {
   async addUnapprovedTransaction(txParams, origin, transactionType) {
     if (
       transactionType !== undefined &&
-      !SWAP_TRANSACTION_TYPES.includes(transactionType)
+      !VALID_UNAPPROVED_TRANSACTION_TYPES.includes(transactionType)
     ) {
       throw new Error(
         `TransactionController - invalid transactionType value: ${transactionType}`,
@@ -674,7 +684,7 @@ export default class TransactionController extends EventEmitter {
       origin,
     });
 
-    if (origin === 'metamask') {
+    if (origin === ORIGIN_METAMASK) {
       // Assert the from address is the selected address
       if (normalizedTxParams.from !== this.getSelectedAddress()) {
         throw ethErrors.rpc.internal({
@@ -783,7 +793,7 @@ export default class TransactionController extends EventEmitter {
         //  then we set maxFeePerGas and maxPriorityFeePerGas to the suggested gasPrice.
         txMeta.txParams.maxFeePerGas = txMeta.txParams.gasPrice;
         txMeta.txParams.maxPriorityFeePerGas = txMeta.txParams.gasPrice;
-        if (eip1559V2Enabled && txMeta.origin !== 'metamask') {
+        if (eip1559V2Enabled && txMeta.origin !== ORIGIN_METAMASK) {
           txMeta.userFeeLevel = PRIORITY_LEVELS.DAPP_SUGGESTED;
         } else {
           txMeta.userFeeLevel = CUSTOM_GAS_ESTIMATE;
@@ -794,7 +804,7 @@ export default class TransactionController extends EventEmitter {
             defaultMaxPriorityFeePerGas &&
             !txMeta.txParams.maxFeePerGas &&
             !txMeta.txParams.maxPriorityFeePerGas) ||
-          txMeta.origin === 'metamask'
+          txMeta.origin === ORIGIN_METAMASK
         ) {
           txMeta.userFeeLevel = GAS_RECOMMENDATIONS.MEDIUM;
         } else if (eip1559V2Enabled) {
@@ -1893,7 +1903,7 @@ export default class TransactionController extends EventEmitter {
       defaultGasEstimates,
       metamaskNetworkId: network,
     } = txMeta;
-    const source = referrer === 'metamask' ? 'user' : 'dapp';
+    const source = referrer === ORIGIN_METAMASK ? 'user' : 'dapp';
 
     const { assetType, tokenStandard } = await determineTransactionAssetType(
       txMeta,

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

@@ -26,6 +26,7 @@ import {
 import { TRANSACTION_ENVELOPE_TYPE_NAMES } from '../../../../ui/helpers/constants/transactions';
 import { METAMASK_CONTROLLER_EVENTS } from '../../metamask-controller';
 import { TOKEN_STANDARDS } from '../../../../ui/helpers/constants/common';
+import { ORIGIN_METAMASK } from '../../../../shared/constants/app';
 import TransactionController from '.';
 
 const noop = () => true;
@@ -791,7 +792,7 @@ describe('Transaction Controller', function () {
         },
         type: TRANSACTION_TYPES.SIMPLE_SEND,
         transaction_envelope_type: TRANSACTION_ENVELOPE_TYPE_NAMES.LEGACY,
-        origin: 'metamask',
+        origin: ORIGIN_METAMASK,
         chainId: currentChainId,
         time: 1624408066355,
         metamaskNetworkId: currentNetworkId,
@@ -1442,7 +1443,7 @@ describe('Transaction Controller', function () {
             nonce: '0x4b',
           },
           type: TRANSACTION_TYPES.SIMPLE_SEND,
-          origin: 'metamask',
+          origin: ORIGIN_METAMASK,
           chainId: currentChainId,
           time: 1624408066355,
           metamaskNetworkId: currentNetworkId,
@@ -1467,7 +1468,7 @@ describe('Transaction Controller', function () {
             gas_edit_attempted: 'none',
             gas_edit_type: 'none',
             network: '42',
-            referrer: 'metamask',
+            referrer: ORIGIN_METAMASK,
             source: 'user',
             type: TRANSACTION_TYPES.SIMPLE_SEND,
             account_type: 'MetaMask',
@@ -1546,7 +1547,7 @@ describe('Transaction Controller', function () {
             gas_edit_attempted: 'none',
             gas_edit_type: 'none',
             network: '42',
-            referrer: 'metamask',
+            referrer: ORIGIN_METAMASK,
             source: 'user',
             type: TRANSACTION_TYPES.SIMPLE_SEND,
             account_type: 'MetaMask',

app/scripts/controllers/transactions/tx-state-manager.js

@@ -6,6 +6,7 @@ import createId from '../../../../shared/modules/random-id';
 import { TRANSACTION_STATUSES } from '../../../../shared/constants/transaction';
 import { METAMASK_CONTROLLER_EVENTS } from '../../metamask-controller';
 import { transactionMatchesNetwork } from '../../../../shared/modules/transaction.utils';
+import { ORIGIN_METAMASK } from '../../../../shared/constants/app';
 import {
   generateHistoryEntry,
   replayHistory,
@@ -92,7 +93,7 @@ export default class TransactionStateManager extends EventEmitter {
     if (
       opts.txParams &&
       typeof opts.origin === 'string' &&
-      opts.origin !== 'metamask'
+      opts.origin !== ORIGIN_METAMASK
     ) {
       if (typeof opts.txParams.gasPrice !== 'undefined') {
         dappSuggestedGasFees = {

app/scripts/controllers/transactions/tx-state-manager.test.js

@@ -11,6 +11,7 @@ import {
   KOVAN_NETWORK_ID,
 } from '../../../../shared/constants/network';
 import { GAS_LIMITS } from '../../../../shared/constants/gas';
+import { ORIGIN_METAMASK } from '../../../../shared/constants/app';
 import TxStateManager from './tx-state-manager';
 import { snapshotFromTxMeta } from './lib/tx-state-history-helpers';
 
@@ -1188,7 +1189,7 @@ describe('TransactionStateManager', function () {
       };
       const generatedTransaction = txStateManager.generateTxMeta({
         txParams,
-        origin: 'metamask',
+        origin: ORIGIN_METAMASK,
       });
       assert.ok(generatedTransaction);
       assert.strictEqual(generatedTransaction.dappSuggestedGasFees, null);

app/scripts/metamask-controller.js

@@ -75,6 +75,7 @@ import { UI_NOTIFICATIONS } from '../../shared/notifications';
 import { toChecksumHexAddress } from '../../shared/modules/hexstring-utils';
 import { MILLISECOND } from '../../shared/constants/time';
 import {
+  ORIGIN_METAMASK,
   ///: BEGIN:ONLY_INCLUDE_IN(flask)
   MESSAGE_TYPE,
   ///: END:ONLY_INCLUDE_IN
@@ -1224,7 +1225,7 @@ export default class MetamaskController extends EventEmitter {
       version,
       // account mgmt
       getAccounts: async ({ origin }) => {
-        if (origin === 'metamask') {
+        if (origin === ORIGIN_METAMASK) {
           const selectedAddress = this.preferencesController.getSelectedAddress();
           return selectedAddress ? [selectedAddress] : [];
         } else if (this.isUnlocked()) {
@@ -3270,7 +3271,7 @@ export default class MetamaskController extends EventEmitter {
   setupProviderConnection(outStream, sender, subjectType) {
     let origin;
     if (subjectType === SUBJECT_TYPES.INTERNAL) {
-      origin = 'metamask';
+      origin = ORIGIN_METAMASK;
     }
     ///: BEGIN:ONLY_INCLUDE_IN(flask)
     else if (subjectType === SUBJECT_TYPES.SNAP) {
@@ -3572,7 +3573,7 @@ export default class MetamaskController extends EventEmitter {
    * @returns {string} The connection's id (so that it can be deleted later)
    */
   addConnection(origin, { engine }) {
-    if (origin === 'metamask') {
+    if (origin === ORIGIN_METAMASK) {
       return null;
     }
 

package.json

@@ -177,7 +177,6 @@
     "fast-safe-stringify": "^2.0.7",
     "fuse.js": "^3.2.0",
     "globalthis": "^1.0.1",
-    "human-standard-collectible-abi": "^1.0.2",
     "human-standard-token-abi": "^2.0.0",
     "immer": "^9.0.6",
     "json-rpc-engine": "^6.1.0",

shared/constants/app.js

@@ -70,3 +70,5 @@ export const POLLING_TOKEN_ENVIRONMENT_TYPES = {
   [ENVIRONMENT_TYPE_NOTIFICATION]: 'notificationGasPollTokens',
   [ENVIRONMENT_TYPE_FULLSCREEN]: 'fullScreenGasPollTokens',
 };
+
+export const ORIGIN_METAMASK = 'metamask';

ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-summary/confirm-page-container-summary.component.js

@@ -13,6 +13,7 @@ import InfoTooltip from '../../../../ui/info-tooltip';
 import NicknamePopovers from '../../../modals/nickname-popovers';
 import Typography from '../../../../ui/typography';
 import { TYPOGRAPHY } from '../../../../../helpers/constants/design-system';
+import { ORIGIN_METAMASK } from '../../../../../../shared/constants/app';
 
 const ConfirmPageContainerSummary = (props) => {
   const {
@@ -83,7 +84,7 @@ const ConfirmPageContainerSummary = (props) => {
 
   return (
     <div className={classnames('confirm-page-container-summary', className)}>
-      {origin === 'metamask' ? null : (
+      {origin === ORIGIN_METAMASK ? null : (
         <div className="confirm-page-container-summary__origin">{origin}</div>
       )}
       <div className="confirm-page-container-summary__action-row">

ui/ducks/send/send.js

@@ -1,6 +1,5 @@
 import { createAsyncThunk, createSlice } from '@reduxjs/toolkit';
 import abi from 'human-standard-token-abi';
-import abiERC721 from 'human-standard-collectible-abi';
 import BigNumber from 'bignumber.js';
 import { addHexPrefix } from 'ethereumjs-util';
 import { debounce } from 'lodash';
@@ -52,7 +51,6 @@ import {
   estimateGas,
   getGasFeeEstimatesAndStartPolling,
   hideLoadingIndication,
-  showConfTxPage,
   showLoadingIndication,
   updateEditableParams,
   updateTransactionGasFees,
@@ -61,6 +59,7 @@ import {
   isCollectibleOwner,
   getTokenStandardAndDetails,
   showModal,
+  addUnapprovedTransactionAndRouteToConfirmationPage,
 } from '../../store/actions';
 import { setCustomGasLimit } from '../gas/gas.duck';
 import {
@@ -106,6 +105,7 @@ import {
 import {
   ASSET_TYPES,
   TRANSACTION_ENVELOPE_TYPES,
+  TRANSACTION_TYPES,
 } from '../../../shared/constants/transaction';
 import { readAddressAsContract } from '../../../shared/modules/contract-utils';
 import { INVALID_ASSET_TYPE } from '../../helpers/constants/error-keys';
@@ -1718,9 +1718,6 @@ export function signTransaction() {
       asset,
       stage,
       draftTransaction: { id, txParams },
-      recipient: { address },
-      amount: { value },
-      account: { address: selectedAddress },
       eip1559support,
     } = state[name];
     if (stage === SEND_STAGES.EDIT) {
@@ -1751,63 +1748,21 @@ export function signTransaction() {
       };
       dispatch(updateEditableParams(id, editingTx.txParams));
       dispatch(updateTransactionGasFees(id, editingTx.txParams));
-    } else if (asset.type === ASSET_TYPES.TOKEN) {
-      // When sending a token transaction we have to the token.transfer method
-      // on the token contract to construct the transaction. This results in
-      // the proper transaction data and properties being set and a new
-      // transaction being added to background state. Once the new transaction
-      // is added to state a subsequent confirmation will be queued.
-      try {
-        const token = global.eth.contract(abi).at(asset.details.address);
-        token.transfer(address, value, {
-          ...txParams,
-          to: undefined,
-          data: undefined,
-        });
-        dispatch(showConfTxPage());
-        dispatch(hideLoadingIndication());
-      } catch (error) {
-        dispatch(hideLoadingIndication());
-        dispatch(displayWarning(error.message));
-      }
-    } else if (asset.type === ASSET_TYPES.COLLECTIBLE) {
-      // When sending a collectible transaction we have to use the collectible.transferFrom method
-      // on the collectible contract to construct the transaction. This results in
-      // the proper transaction data and properties being set and a new
-      // transaction being added to background state. Once the new transaction
-      // is added to state a subsequent confirmation will be queued.
-      try {
-        const collectibleContract = global.eth
-          .contract(abiERC721)
-          .at(asset.details.address);
-
-        collectibleContract.transferFrom(
-          selectedAddress,
-          address,
-          asset.details.tokenId,
-          {
-            ...txParams,
-            to: undefined,
-            data: undefined,
-          },
-        );
+    } else {
+      let transactionType = TRANSACTION_TYPES.SIMPLE_SEND;
 
-        dispatch(showConfTxPage());
-        dispatch(hideLoadingIndication());
-      } catch (error) {
-        dispatch(hideLoadingIndication());
-        dispatch(displayWarning(error.message));
+      if (asset.type !== ASSET_TYPES.NATIVE) {
+        transactionType =
+          asset.type === ASSET_TYPES.COLLECTIBLE
+            ? TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER_FROM
+            : TRANSACTION_TYPES.TOKEN_METHOD_TRANSFER;
       }
-    } else {
-      // When sending a native asset we use the ethQuery.sendTransaction method
-      // which will result in the transaction being added to background state
-      // and a subsequent confirmation will be queued.
-      global.ethQuery.sendTransaction(txParams, (err) => {
-        if (err) {
-          dispatch(displayWarning(err.message));
-        }
-      });
-      dispatch(showConfTxPage());
+      dispatch(
+        addUnapprovedTransactionAndRouteToConfirmationPage(
+          txParams,
+          transactionType,
+        ),
+      );
     }
   };
 }

ui/ducks/send/send.test.js

@@ -21,6 +21,7 @@ import {
   TRANSACTION_TYPES,
 } from '../../../shared/constants/transaction';
 import * as Actions from '../../store/actions';
+import { setBackgroundConnection } from '../../../test/jest';
 import sendReducer, {
   initialState,
   initializeSendState,
@@ -77,9 +78,17 @@ jest.mock('./send', () => {
   };
 });
 
+setBackgroundConnection({
+  addPollingTokenToAppState: jest.fn(),
+  addUnapprovedTransaction: jest.fn((_x, _y, _z, cb) => {
+    return cb(null, {});
+  }),
+});
+
 describe('Send Slice', () => {
   let getTokenStandardAndDetailsStub;
   beforeEach(() => {
+    jest.useFakeTimers();
     getTokenStandardAndDetailsStub = jest
       .spyOn(Actions, 'getTokenStandardAndDetails')
       .mockImplementation(() => Promise.resolve({ standard: 'ERC20' }));
@@ -2030,10 +2039,6 @@ describe('Send Slice', () => {
       };
 
       it('should show confirm tx page when no other conditions for signing have been met', async () => {
-        global.ethQuery = {
-          sendTransaction: sinon.stub(),
-        };
-
         const store = mockStore(signTransactionState);
 
         await store.dispatch(signTransaction());