Trigger transaction popup using ApprovalController (#18400)

app/scripts/background.js

@@ -727,7 +727,6 @@ export function setupController(
   }
 
   function getUnapprovedTransactionCount() {
-    const unapprovedTxCount = controller.txController.getUnapprovedTxCount();
     const { unapprovedDecryptMsgCount } = controller.decryptMessageManager;
     const { unapprovedEncryptionPublicKeyMsgCount } =
       controller.encryptionPublicKeyManager;
@@ -736,7 +735,6 @@ export function setupController(
     const waitingForUnlockCount =
       controller.appStateController.waitingForUnlock.length;
     return (
-      unapprovedTxCount +
       unapprovedDecryptMsgCount +
       unapprovedEncryptionPublicKeyMsgCount +
       pendingApprovalCount +

app/scripts/controllers/transactions/index.js

@@ -52,7 +52,10 @@ import {
   determineTransactionType,
   isEIP1559Transaction,
 } from '../../../../shared/modules/transaction.utils';
-import { ORIGIN_METAMASK } from '../../../../shared/constants/app';
+import {
+  ORIGIN_METAMASK,
+  MESSAGE_TYPE,
+} from '../../../../shared/constants/app';
 import {
   calcGasTotal,
   getSwapsTokensReceivedFromTxMeta,
@@ -156,6 +159,7 @@ export default class TransactionController extends EventEmitter {
     this.getAccountType = opts.getAccountType;
     this.getTokenStandardAndDetails = opts.getTokenStandardAndDetails;
     this.securityProviderRequest = opts.securityProviderRequest;
+    this.messagingSystem = opts.messenger;
 
     this.memStore = new ObservableStore({});
 
@@ -798,6 +802,7 @@ export default class TransactionController extends EventEmitter {
         this.txStateManager.getTransactionWithActionId(actionId);
       if (existingTxMeta) {
         this.emit('newUnapprovedTx', existingTxMeta);
+        this._requestApproval(existingTxMeta);
         existingTxMeta = await this.addTransactionGasDefaults(existingTxMeta);
         return existingTxMeta;
       }
@@ -870,6 +875,7 @@ export default class TransactionController extends EventEmitter {
 
     this.addTransaction(txMeta);
     this.emit('newUnapprovedTx', txMeta);
+    this._requestApproval(txMeta);
 
     txMeta = await this.addTransactionGasDefaults(txMeta);
 
@@ -1355,6 +1361,7 @@ export default class TransactionController extends EventEmitter {
     try {
       // approve
       this.txStateManager.setTxStatusApproved(txId);
+      this._acceptApproval(txMeta);
       // get next nonce
       const fromAddress = txMeta.txParams.from;
       // wait for a nonce
@@ -1734,6 +1741,7 @@ export default class TransactionController extends EventEmitter {
   async cancelTransaction(txId, actionId) {
     const txMeta = this.txStateManager.getTransaction(txId);
     this.txStateManager.setTxStatusRejected(txId);
+    this._rejectApproval(txMeta);
     this._trackTransactionMetricsEvent(
       txMeta,
       TransactionMetaMetricsEvent.rejected,
@@ -2596,4 +2604,54 @@ export default class TransactionController extends EventEmitter {
       },
     );
   }
+
+  _requestApproval(txMeta) {
+    const id = this._getApprovalId(txMeta);
+    const { origin } = txMeta;
+    const type = MESSAGE_TYPE.TRANSACTION;
+    const requestData = { txId: txMeta.id };
+
+    this.messagingSystem
+      .call(
+        'ApprovalController:addRequest',
+        {
+          id,
+          origin,
+          type,
+          requestData,
+        },
+        true,
+      )
+      .catch(() => {
+        // Intentionally ignored as promise not currently used
+      });
+  }
+
+  _acceptApproval(txMeta) {
+    const id = this._getApprovalId(txMeta);
+
+    try {
+      this.messagingSystem.call('ApprovalController:acceptRequest', id);
+    } catch (error) {
+      log.error('Failed to accept transaction approval request', error);
+    }
+  }
+
+  _rejectApproval(txMeta) {
+    const id = this._getApprovalId(txMeta);
+
+    try {
+      this.messagingSystem.call(
+        'ApprovalController:rejectRequest',
+        id,
+        new Error('Rejected'),
+      );
+    } catch (error) {
+      log.error('Failed to reject transaction approval request', error);
+    }
+  }
+
+  _getApprovalId(txMeta) {
+    return String(txMeta.id);
+  }
 }

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

@@ -29,7 +29,10 @@ import {
   GasRecommendations,
 } from '../../../../shared/constants/gas';
 import { METAMASK_CONTROLLER_EVENTS } from '../../metamask-controller';
-import { ORIGIN_METAMASK } from '../../../../shared/constants/app';
+import {
+  MESSAGE_TYPE,
+  ORIGIN_METAMASK,
+} from '../../../../shared/constants/app';
 import { NetworkStatus } from '../../../../shared/constants/network';
 import { TRANSACTION_ENVELOPE_TYPE_NAMES } from '../../../../shared/lib/transactions-controller-utils';
 import TransactionController from '.';
@@ -52,7 +55,8 @@ describe('Transaction Controller', function () {
     fromAccount,
     fragmentExists,
     networkStatusStore,
-    getCurrentChainId;
+    getCurrentChainId,
+    messengerMock;
 
   beforeEach(function () {
     fragmentExists = false;
@@ -76,6 +80,7 @@ describe('Transaction Controller', function () {
     blockTrackerStub.getLatestBlock = noop;
 
     getCurrentChainId = sinon.stub().callsFake(() => currentChainId);
+    messengerMock = { call: sinon.stub().returns(Promise.resolve()) };
 
     txController = new TransactionController({
       provider,
@@ -108,6 +113,7 @@ describe('Transaction Controller', function () {
       getAccountType: () => 'MetaMask',
       getDeviceModel: () => 'N/A',
       securityProviderRequest: () => undefined,
+      messenger: messengerMock,
     });
     txController.nonceTracker.getNonceLock = () =>
       Promise.resolve({ nextNonce: 0, releaseLock: noop });
@@ -489,6 +495,67 @@ describe('Transaction Controller', function () {
         { message: 'MetaMask is having trouble connecting to the network' },
       );
     });
+
+    it('should create an approval request', async function () {
+      const txMeta = await txController.addUnapprovedTransaction(
+        undefined,
+        {
+          from: selectedAddress,
+          to: recipientAddress,
+        },
+        ORIGIN_METAMASK,
+      );
+
+      assert.equal(messengerMock.call.callCount, 1);
+      assert.deepEqual(messengerMock.call.getCall(0).args, [
+        'ApprovalController:addRequest',
+        {
+          id: String(txMeta.id),
+          origin: ORIGIN_METAMASK,
+          requestData: { txId: txMeta.id },
+          type: MESSAGE_TYPE.TRANSACTION,
+        },
+        true, // Show popup
+      ]);
+    });
+
+    it('should still create an approval request when called twice with same actionId', async function () {
+      await txController.addUnapprovedTransaction(
+        undefined,
+        {
+          from: selectedAddress,
+          to: recipientAddress,
+        },
+        ORIGIN_METAMASK,
+        undefined,
+        undefined,
+        '12345',
+      );
+
+      const secondTxMeta = await txController.addUnapprovedTransaction(
+        undefined,
+        {
+          from: selectedAddress,
+          to: recipientAddress,
+        },
+        undefined,
+        undefined,
+        undefined,
+        '12345',
+      );
+
+      assert.equal(messengerMock.call.callCount, 2);
+      assert.deepEqual(messengerMock.call.getCall(1).args, [
+        'ApprovalController:addRequest',
+        {
+          id: String(secondTxMeta.id),
+          origin: ORIGIN_METAMASK,
+          requestData: { txId: secondTxMeta.id },
+          type: MESSAGE_TYPE.TRANSACTION,
+        },
+        true, // Show popup
+      ]);
+    });
   });
 
   describe('#createCancelTransaction', function () {
@@ -997,9 +1064,11 @@ describe('Transaction Controller', function () {
   });
 
   describe('#approveTransaction', function () {
-    it('does not overwrite set values', async function () {
+    let originalValue, txMeta, signStub, pubStub;
-      const originalValue = '0x01';
+
-      const txMeta = {
+    beforeEach(function () {
+      originalValue = '0x01';
+      txMeta = {
         id: '1',
         status: TransactionStatus.unapproved,
         metamaskNetworkId: currentNetworkId,
@@ -1019,17 +1088,22 @@ describe('Transaction Controller', function () {
       providerResultStub.eth_gasPrice = wrongValue;
       providerResultStub.eth_estimateGas = '0x5209';
 
-      const signStub = sinon
+      signStub = sinon
         .stub(txController, 'signTransaction')
         .callsFake(() => Promise.resolve());
 
-      const pubStub = sinon
+      pubStub = sinon.stub(txController, 'publishTransaction').callsFake(() => {
-        .stub(txController, 'publishTransaction')
+        txController.setTxHash('1', originalValue);
-        .callsFake(() => {
+        txController.txStateManager.setTxStatusSubmitted('1');
-          txController.setTxHash('1', originalValue);
+      });
-          txController.txStateManager.setTxStatusSubmitted('1');
+    });
-        });
 
+    afterEach(function () {
+      signStub.restore();
+      pubStub.restore();
+    });
+
+    it('does not overwrite set values', async function () {
       await txController.approveTransaction(txMeta.id);
       const result = txController.txStateManager.getTransaction(txMeta.id);
       const params = result.txParams;
@@ -1042,8 +1116,21 @@ describe('Transaction Controller', function () {
         TransactionStatus.submitted,
         'should have reached the submitted status.',
       );
-      signStub.restore();
+    });
-      pubStub.restore();
+
+    it('should accept the approval request', async function () {
+      await txController.approveTransaction(txMeta.id);
+
+      assert.equal(messengerMock.call.callCount, 1);
+      assert.deepEqual(messengerMock.call.getCall(0).args, [
+        'ApprovalController:acceptRequest',
+        txMeta.id,
+      ]);
+    });
+
+    it('should not throw if accepting approval request throws', async function () {
+      messengerMock.call.throws();
+      await txController.approveTransaction(txMeta.id);
     });
   });
 
@@ -1108,7 +1195,7 @@ describe('Transaction Controller', function () {
   });
 
   describe('#cancelTransaction', function () {
-    it('should emit a status change to rejected', function (done) {
+    beforeEach(function () {
       txController.txStateManager._addTransactionsToState([
         {
           id: 0,
@@ -1181,7 +1268,9 @@ describe('Transaction Controller', function () {
           history: [{}],
         },
       ]);
+    });
 
+    it('should emit a status change to rejected', function (done) {
       txController.once('tx:status-update', (txId, status) => {
         try {
           assert.equal(
@@ -1198,6 +1287,22 @@ describe('Transaction Controller', function () {
 
       txController.cancelTransaction(0);
     });
+
+    it('should reject the approval request', function () {
+      txController.cancelTransaction(0);
+
+      assert.equal(messengerMock.call.callCount, 1);
+      assert.deepEqual(messengerMock.call.getCall(0).args, [
+        'ApprovalController:rejectRequest',
+        '0',
+        new Error('Rejected'),
+      ]);
+    });
+
+    it('should not throw if rejecting approval request throws', async function () {
+      messengerMock.call.throws();
+      txController.cancelTransaction(0);
+    });
   });
 
   describe('#createSpeedUpTransaction', function () {

app/scripts/metamask-controller.js

@@ -1007,8 +1007,15 @@ export default class MetamaskController extends EventEmitter {
       getDeviceModel: this.getDeviceModel.bind(this),
       getTokenStandardAndDetails: this.getTokenStandardAndDetails.bind(this),
       securityProviderRequest: this.securityProviderRequest.bind(this),
+      messenger: this.controllerMessenger.getRestricted({
+        name: 'TransactionController',
+        allowedActions: [
+          `${this.approvalController.name}:addRequest`,
+          `${this.approvalController.name}:acceptRequest`,
+          `${this.approvalController.name}:rejectRequest`,
+        ],
+      }),
     });
-    this.txController.on('newUnapprovedTx', () => opts.showUserConfirmation());
 
     this.txController.on(`tx:status-update`, async (txId, status) => {
       if (

shared/constants/app.ts

@@ -53,6 +53,7 @@ export const MESSAGE_TYPE = {
   PERSONAL_SIGN: 'personal_sign',
   SEND_METADATA: 'metamask_sendDomainMetadata',
   SWITCH_ETHEREUM_CHAIN: 'wallet_switchEthereumChain',
+  TRANSACTION: 'transaction',
   WALLET_REQUEST_PERMISSIONS: 'wallet_requestPermissions',
   WATCH_ASSET: 'wallet_watchAsset',
   WATCH_ASSET_LEGACY: 'metamask_watchAsset',

test/data/mock-state.json

@@ -1535,7 +1535,18 @@
         "origin": "tmashuang.github.io"
       }
     ],
-    "desktopEnabled": false
+    "desktopEnabled": false,
+    "pendingApprovals": {
+      "testApprovalId": {
+        "id": "testApprovalId",
+        "time": 1528133319641,
+        "origin": "metamask",
+        "type": "transaction",
+        "requestData": { "txId": "testTransactionId" },
+        "requestState": { "test": "value" }
+      }
+    },
+    "pendingApprovalCount": 1
   },
   "send": {
     "amountMode": "INPUT",

ui/selectors/selectors.js

@@ -484,21 +484,14 @@ export function getCurrentCurrency(state) {
 
 export function getTotalUnapprovedCount(state) {
   const {
-    unapprovedMsgCount = 0,
-    unapprovedPersonalMsgCount = 0,
     unapprovedDecryptMsgCount = 0,
     unapprovedEncryptionPublicKeyMsgCount = 0,
-    unapprovedTypedMessagesCount = 0,
     pendingApprovalCount = 0,
   } = state.metamask;
 
   return (
-    unapprovedMsgCount +
-    unapprovedPersonalMsgCount +
     unapprovedDecryptMsgCount +
     unapprovedEncryptionPublicKeyMsgCount +
-    unapprovedTypedMessagesCount +
-    getUnapprovedTxCount(state) +
     pendingApprovalCount +
     getSuggestedAssetCount(state)
   );