Adding metric events for Approved, Rejected, and Submitted to the TxController (#11358)

app/scripts/controllers/transactions/index.js

@@ -33,6 +33,14 @@ const hstInterface = new ethers.utils.Interface(abi);
 
 const MAX_MEMSTORE_TX_LIST_SIZE = 100; // Number of transactions (by unique nonces) to keep in memory
 
+export const TRANSACTION_EVENTS = {
+  ADDED: 'Transaction Added',
+  APPROVED: 'Transaction Approved',
+  FINALIZED: 'Transaction Finalized',
+  REJECTED: 'Transaction Rejected',
+  SUBMITTED: 'Transaction Submitted',
+};
+
 /**
   Transaction Controller is an aggregate of sub-controllers and trackers
   composing them in a way to be exposed to the metamask controller
@@ -151,32 +159,9 @@ export default class TransactionController extends EventEmitter {
   @emits ${txMeta.id}:unapproved
   */
   addTransaction(txMeta) {
-    const {
-      type,
-      status,
-      chainId,
-      origin: referrer,
-      txParams: { gasPrice },
-      metamaskNetworkId: network,
-    } = txMeta;
-    const source = referrer === 'metamask' ? 'user' : 'dapp';
-
     this.txStateManager.addTransaction(txMeta);
     this.emit(`${txMeta.id}:unapproved`, txMeta);
-
+    this._trackTransactionMetricsEvent(txMeta, TRANSACTION_EVENTS.ADDED);
-    this._trackMetaMetricsEvent({
-      event: 'Transaction Added',
-      category: 'Transactions',
-      sensitiveProperties: {
-        type,
-        status,
-        gasPrice,
-        referrer,
-        source,
-        network,
-        chain_id: chainId,
-      },
-    });
   }
 
   /**
@@ -556,12 +541,13 @@ export default class TransactionController extends EventEmitter {
       // sign transaction
       const rawTx = await this.signTransaction(txId);
       await this.publishTransaction(txId, rawTx);
+      this._trackTransactionMetricsEvent(txMeta, TRANSACTION_EVENTS.APPROVED);
       // must set transaction to submitted/failed before releasing lock
       nonceLock.releaseLock();
     } catch (err) {
       // this is try-catch wrapped so that we can guarantee that the nonceLock is released
       try {
-        this.txStateManager.setTxStatusFailed(txId, err);
+        this._failTransaction(txId, err);
       } catch (err2) {
         log.error(err2);
       }
@@ -639,6 +625,11 @@ export default class TransactionController extends EventEmitter {
     this.setTxHash(txId, txHash);
 
     this.txStateManager.setTxStatusSubmitted(txId);
+
+    const { gas } = txMeta.txParams;
+    this._trackTransactionMetricsEvent(txMeta, TRANSACTION_EVENTS.SUBMITTED, {
+      gas_limit: gas,
+    });
   }
 
   /**
@@ -671,6 +662,29 @@ export default class TransactionController extends EventEmitter {
       this.txStateManager.setTxStatusConfirmed(txId);
       this._markNonceDuplicatesDropped(txId);
 
+      const { submittedTime } = txMeta;
+      const { blockNumber } = txReceipt;
+      const metricsParams = { gas_used: gasUsed };
+      const completionTime = await this._getTransactionCompletionTime(
+        blockNumber,
+        submittedTime,
+      );
+
+      if (completionTime) {
+        metricsParams.completion_time = completionTime;
+      }
+
+      if (txReceipt.status === '0x0') {
+        metricsParams.status = 'failed on-chain';
+        // metricsParams.error = TODO: figure out a way to get the on-chain failure reason
+      }
+
+      this._trackTransactionMetricsEvent(
+        txMeta,
+        TRANSACTION_EVENTS.FINALIZED,
+        metricsParams,
+      );
+
       this.txStateManager.updateTransaction(
         txMeta,
         'transactions#confirmTransaction - add txReceipt',
@@ -704,7 +718,9 @@ export default class TransactionController extends EventEmitter {
     @returns {Promise<void>}
   */
   async cancelTransaction(txId) {
+    const txMeta = this.txStateManager.getTransaction(txId);
     this.txStateManager.setTxStatusRejected(txId);
+    this._trackTransactionMetricsEvent(txMeta, TRANSACTION_EVENTS.REJECTED);
   }
 
   /**
@@ -787,7 +803,7 @@ export default class TransactionController extends EventEmitter {
               txMeta,
               'failed to estimate gas during boot cleanup.',
             );
-            this.txStateManager.setTxStatusFailed(txMeta.id, error);
+            this._failTransaction(txMeta.id, error);
           });
       });
 
@@ -801,7 +817,7 @@ export default class TransactionController extends EventEmitter {
         const txSignError = new Error(
           'Transaction found as "approved" during boot - possibly stuck during signing',
         );
-        this.txStateManager.setTxStatusFailed(txMeta.id, txSignError);
+        this._failTransaction(txMeta.id, txSignError);
       });
   }
 
@@ -821,17 +837,15 @@ export default class TransactionController extends EventEmitter {
         'transactions/pending-tx-tracker#event: tx:warning',
       );
     });
-    this.pendingTxTracker.on(
+    this.pendingTxTracker.on('tx:failed', (txId, error) => {
-      'tx:failed',
+      this._failTransaction(txId, error);
-      this.txStateManager.setTxStatusFailed.bind(this.txStateManager),
+    });
-    );
     this.pendingTxTracker.on('tx:confirmed', (txId, transactionReceipt) =>
       this.confirmTransaction(txId, transactionReceipt),
     );
-    this.pendingTxTracker.on(
+    this.pendingTxTracker.on('tx:dropped', (txId) => {
-      'tx:dropped',
+      this._dropTransaction(txId);
-      this.txStateManager.setTxStatusDropped.bind(this.txStateManager),
+    });
-    );
     this.pendingTxTracker.on('tx:block-update', (txMeta, latestBlockNumber) => {
       if (!txMeta.firstRetryBlockNumber) {
         txMeta.firstRetryBlockNumber = latestBlockNumber;
@@ -941,7 +955,7 @@ export default class TransactionController extends EventEmitter {
         txMeta,
         'transactions/pending-tx-tracker#event: tx:confirmed reference to confirmed txHash with same nonce',
       );
-      this.txStateManager.setTxStatusDropped(otherTxMeta.id);
+      this._dropTransaction(otherTxMeta.id);
     });
   }
 
@@ -1034,4 +1048,71 @@ export default class TransactionController extends EventEmitter {
       }
     }
   }
+
+  /**
+   * Extracts relevant properties from a transaction meta
+   * object and uses them to create and send metrics for various transaction
+   * events.
+   * @param {Object} txMeta - the txMeta object
+   * @param {string} event - the name of the transaction event
+   * @param {Object} extraParams - optional props and values to include in sensitiveProperties
+   */
+  _trackTransactionMetricsEvent(txMeta, event, extraParams = {}) {
+    const {
+      type,
+      time,
+      status,
+      chainId,
+      origin: referrer,
+      txParams: { gasPrice },
+      metamaskNetworkId: network,
+    } = txMeta;
+    const source = referrer === 'metamask' ? 'user' : 'dapp';
+
+    this._trackMetaMetricsEvent({
+      event,
+      category: 'Transactions',
+      sensitiveProperties: {
+        type,
+        status,
+        referrer,
+        source,
+        network,
+        chain_id: chainId,
+        gas_price: gasPrice,
+        first_seen: time,
+        ...extraParams,
+      },
+    });
+  }
+
+  async _getTransactionCompletionTime(blockNumber, submittedTime) {
+    const transactionBlock = await this.query.getBlockByNumber(
+      blockNumber.toString(16),
+      false,
+    );
+
+    if (!transactionBlock) {
+      return '';
+    }
+
+    return new BigNumber(transactionBlock.timestamp, 10)
+      .minus(submittedTime / 1000)
+      .round()
+      .toString(10);
+  }
+
+  _failTransaction(txId, error) {
+    this.txStateManager.setTxStatusFailed(txId, error);
+    const txMeta = this.txStateManager.getTransaction(txId);
+    this._trackTransactionMetricsEvent(txMeta, TRANSACTION_EVENTS.FINALIZED, {
+      error: error.message,
+    });
+  }
+
+  _dropTransaction(txId) {
+    this.txStateManager.setTxStatusDropped(txId);
+    const txMeta = this.txStateManager.getTransaction(txId);
+    this._trackTransactionMetricsEvent(txMeta, TRANSACTION_EVENTS.FINALIZED);
+  }
 }

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

@@ -15,7 +15,7 @@ import {
 } from '../../../../shared/constants/transaction';
 import { SECOND } from '../../../../shared/constants/time';
 import { METAMASK_CONTROLLER_EVENTS } from '../../metamask-controller';
-import TransactionController from '.';
+import TransactionController, { TRANSACTION_EVENTS } from '.';
 
 const noop = () => true;
 const currentNetworkId = '42';
@@ -415,17 +415,17 @@ describe('Transaction Controller', function () {
   });
 
   describe('#addTransaction', function () {
-    let trackMetaMetricsEventSpy;
+    let trackTransactionMetricsEventSpy;
 
     beforeEach(function () {
-      trackMetaMetricsEventSpy = sinon.spy(
+      trackTransactionMetricsEventSpy = sinon.spy(
         txController,
-        '_trackMetaMetricsEvent',
+        '_trackTransactionMetricsEvent',
       );
     });
 
     afterEach(function () {
-      trackMetaMetricsEventSpy.restore();
+      trackTransactionMetricsEventSpy.restore();
     });
 
     it('should emit updates', function (done) {
@@ -466,7 +466,7 @@ describe('Transaction Controller', function () {
       txController.addTransaction(txMeta);
     });
 
-    it('should call _trackMetaMetricsEvent with the correct payload (one)', function () {
+    it('should call _trackTransactionMetricsEvent with the correct params', function () {
       const txMeta = {
         id: 1,
         status: TRANSACTION_STATUSES.UNAPPROVED,
@@ -480,65 +480,20 @@ describe('Transaction Controller', function () {
         type: 'sentEther',
         origin: 'metamask',
         chainId: currentChainId,
+        time: 1624408066355,
         metamaskNetworkId: currentNetworkId,
       };
-      const expectedPayload = {
-        event: 'Transaction Added',
-        category: 'Transactions',
-        sensitiveProperties: {
-          chain_id: '0x2a',
-          gasPrice: '0x77359400',
-          network: '42',
-          referrer: 'metamask',
-          source: 'user',
-          status: 'unapproved',
-          type: 'sentEther',
-        },
-      };
 
       txController.addTransaction(txMeta);
-      assert.equal(trackMetaMetricsEventSpy.callCount, 1);
+
+      assert.equal(trackTransactionMetricsEventSpy.callCount, 1);
       assert.deepEqual(
-        trackMetaMetricsEventSpy.getCall(0).args[0],
+        trackTransactionMetricsEventSpy.getCall(0).args[0],
-        expectedPayload,
+        txMeta,
       );
-    });
+      assert.equal(
-
+        trackTransactionMetricsEventSpy.getCall(0).args[1],
-    it('should call _trackMetaMetricsEvent with the correct payload (two)', function () {
+        TRANSACTION_EVENTS.ADDED,
-      const txMeta = {
-        id: 1,
-        status: TRANSACTION_STATUSES.UNAPPROVED,
-        txParams: {
-          from: fromAccount.address,
-          to: '0x1678a085c290ebd122dc42cba69373b5953b831d',
-          gasPrice: '0x77359400',
-          gas: '0x7b0d',
-          nonce: '0x4b',
-        },
-        type: 'sentEther',
-        origin: 'other',
-        chainId: '0x3',
-        metamaskNetworkId: '3',
-      };
-      const expectedPayload = {
-        event: 'Transaction Added',
-        category: 'Transactions',
-        sensitiveProperties: {
-          chain_id: '0x3',
-          gasPrice: '0x77359400',
-          network: '3',
-          referrer: 'other',
-          source: 'dapp',
-          status: 'unapproved',
-          type: 'sentEther',
-        },
-      };
-
-      txController.addTransaction(txMeta);
-      assert.equal(trackMetaMetricsEventSpy.callCount, 1);
-      assert.deepEqual(
-        trackMetaMetricsEventSpy.getCall(0).args[0],
-        expectedPayload,
       );
     });
   });
@@ -813,7 +768,8 @@ describe('Transaction Controller', function () {
   });
 
   describe('#publishTransaction', function () {
-    let hash, txMeta;
+    let hash, txMeta, trackTransactionMetricsEventSpy;
+
     beforeEach(function () {
       hash =
         '0x2a5523c6fa98b47b7d9b6c8320179785150b42a16bcff36b398c5062b65657e8';
@@ -821,12 +777,21 @@ describe('Transaction Controller', function () {
         id: 1,
         status: TRANSACTION_STATUSES.UNAPPROVED,
         txParams: {
+          gas: '0x7b0d',
           to: VALID_ADDRESS,
           from: VALID_ADDRESS_TWO,
         },
         metamaskNetworkId: currentNetworkId,
       };
       providerResultStub.eth_sendRawTransaction = hash;
+      trackTransactionMetricsEventSpy = sinon.spy(
+        txController,
+        '_trackTransactionMetricsEvent',
+      );
+    });
+
+    afterEach(function () {
+      trackTransactionMetricsEventSpy.restore();
     });
 
     it('should publish a tx, updates the rawTx when provided a one', async function () {
@@ -854,6 +819,25 @@ describe('Transaction Controller', function () {
       );
       assert.equal(publishedTx.status, TRANSACTION_STATUSES.SUBMITTED);
     });
+
+    it('should call _trackTransactionMetricsEvent with the correct params', async function () {
+      const rawTx =
+        '0x477b2e6553c917af0db0388ae3da62965ff1a184558f61b749d1266b2e6d024c';
+      txController.txStateManager.addTransaction(txMeta);
+      await txController.publishTransaction(txMeta.id, rawTx);
+      assert.equal(trackTransactionMetricsEventSpy.callCount, 1);
+      assert.deepEqual(
+        trackTransactionMetricsEventSpy.getCall(0).args[0],
+        txMeta,
+      );
+      assert.equal(
+        trackTransactionMetricsEventSpy.getCall(0).args[1],
+        TRANSACTION_EVENTS.SUBMITTED,
+      );
+      assert.deepEqual(trackTransactionMetricsEventSpy.getCall(0).args[2], {
+        gas_limit: txMeta.txParams.gas,
+      });
+    });
   });
 
   describe('#_markNonceDuplicatesDropped', function () {
@@ -1195,4 +1179,154 @@ describe('Transaction Controller', function () {
       );
     });
   });
+
+  describe('#_trackTransactionMetricsEvent', function () {
+    let trackMetaMetricsEventSpy;
+
+    beforeEach(function () {
+      trackMetaMetricsEventSpy = sinon.spy(
+        txController,
+        '_trackMetaMetricsEvent',
+      );
+    });
+
+    afterEach(function () {
+      trackMetaMetricsEventSpy.restore();
+    });
+
+    it('should call _trackMetaMetricsEvent with the correct payload (user source)', function () {
+      const txMeta = {
+        id: 1,
+        status: TRANSACTION_STATUSES.UNAPPROVED,
+        txParams: {
+          from: fromAccount.address,
+          to: '0x1678a085c290ebd122dc42cba69373b5953b831d',
+          gasPrice: '0x77359400',
+          gas: '0x7b0d',
+          nonce: '0x4b',
+        },
+        type: 'sentEther',
+        origin: 'metamask',
+        chainId: currentChainId,
+        time: 1624408066355,
+        metamaskNetworkId: currentNetworkId,
+      };
+      const expectedPayload = {
+        event: 'Transaction Added',
+        category: 'Transactions',
+        sensitiveProperties: {
+          chain_id: '0x2a',
+          gas_price: '0x77359400',
+          first_seen: 1624408066355,
+          network: '42',
+          referrer: 'metamask',
+          source: 'user',
+          status: 'unapproved',
+          type: 'sentEther',
+        },
+      };
+
+      txController._trackTransactionMetricsEvent(
+        txMeta,
+        TRANSACTION_EVENTS.ADDED,
+      );
+      assert.equal(trackMetaMetricsEventSpy.callCount, 1);
+      assert.deepEqual(
+        trackMetaMetricsEventSpy.getCall(0).args[0],
+        expectedPayload,
+      );
+    });
+
+    it('should call _trackMetaMetricsEvent with the correct payload (dapp source)', function () {
+      const txMeta = {
+        id: 1,
+        status: TRANSACTION_STATUSES.UNAPPROVED,
+        txParams: {
+          from: fromAccount.address,
+          to: '0x1678a085c290ebd122dc42cba69373b5953b831d',
+          gasPrice: '0x77359400',
+          gas: '0x7b0d',
+          nonce: '0x4b',
+        },
+        type: 'sentEther',
+        origin: 'other',
+        chainId: currentChainId,
+        time: 1624408066355,
+        metamaskNetworkId: currentNetworkId,
+      };
+      const expectedPayload = {
+        event: 'Transaction Added',
+        category: 'Transactions',
+        sensitiveProperties: {
+          chain_id: '0x2a',
+          gas_price: '0x77359400',
+          first_seen: 1624408066355,
+          network: '42',
+          referrer: 'other',
+          source: 'dapp',
+          status: 'unapproved',
+          type: 'sentEther',
+        },
+      };
+
+      txController._trackTransactionMetricsEvent(
+        txMeta,
+        TRANSACTION_EVENTS.ADDED,
+      );
+      assert.equal(trackMetaMetricsEventSpy.callCount, 1);
+      assert.deepEqual(
+        trackMetaMetricsEventSpy.getCall(0).args[0],
+        expectedPayload,
+      );
+    });
+
+    it('should call _trackMetaMetricsEvent with the correct payload (extra params)', function () {
+      const txMeta = {
+        id: 1,
+        status: TRANSACTION_STATUSES.UNAPPROVED,
+        txParams: {
+          from: fromAccount.address,
+          to: '0x1678a085c290ebd122dc42cba69373b5953b831d',
+          gasPrice: '0x77359400',
+          gas: '0x7b0d',
+          nonce: '0x4b',
+        },
+        type: 'sentEther',
+        origin: 'other',
+        chainId: currentChainId,
+        time: 1624408066355,
+        metamaskNetworkId: currentNetworkId,
+      };
+      const expectedPayload = {
+        event: 'Transaction Added',
+        category: 'Transactions',
+        sensitiveProperties: {
+          baz: 3.0,
+          foo: 'bar',
+          chain_id: '0x2a',
+          gas_price: '0x77359400',
+          first_seen: 1624408066355,
+          network: '42',
+          referrer: 'other',
+          source: 'dapp',
+          status: 'unapproved',
+          type: 'sentEther',
+        },
+      };
+
+      txController._trackTransactionMetricsEvent(
+        txMeta,
+        TRANSACTION_EVENTS.ADDED,
+        {
+          baz: 3.0,
+          foo: 'bar',
+        },
+      );
+      assert.equal(trackMetaMetricsEventSpy.callCount, 1);
+      assert.deepEqual(
+        trackMetaMetricsEventSpy.getCall(0).args[0],
+        expectedPayload,
+      );
+    });
+  });
 });