Fix Provider Tracking Metrics (#15082)

app/scripts/lib/createRPCMethodTrackingMiddleware.js

@@ -1,19 +1,91 @@
+import { MESSAGE_TYPE, ORIGIN_METAMASK } from '../../../shared/constants/app';
 import { EVENT, EVENT_NAMES } from '../../../shared/constants/metametrics';
 import { SECOND } from '../../../shared/constants/time';
 
-const USER_PROMPTED_EVENT_NAME_MAP = {
-  eth_signTypedData_v4: EVENT_NAMES.SIGNATURE_REQUESTED,
-  eth_signTypedData_v3: EVENT_NAMES.SIGNATURE_REQUESTED,
-  eth_signTypedData: EVENT_NAMES.SIGNATURE_REQUESTED,
-  eth_personal_sign: EVENT_NAMES.SIGNATURE_REQUESTED,
-  eth_sign: EVENT_NAMES.SIGNATURE_REQUESTED,
-  eth_getEncryptionPublicKey: EVENT_NAMES.ENCRYPTION_PUBLIC_KEY_REQUESTED,
-  eth_decrypt: EVENT_NAMES.DECRYPTION_REQUESTED,
-  wallet_requestPermissions: EVENT_NAMES.PERMISSIONS_REQUESTED,
-  eth_requestAccounts: EVENT_NAMES.PERMISSIONS_REQUESTED,
+/**
+ * These types determine how the method tracking middleware handles incoming
+ * requests based on the method name. There are three options right now but
+ * the types could be expanded to cover other options in the future.
+ */
+const RATE_LIMIT_TYPES = {
+  RATE_LIMITED: 'rate_limited',
+  BLOCKED: 'blocked',
+  NON_RATE_LIMITED: 'non_rate_limited',
 };
 
-const samplingTimeouts = {};
+/**
+ * This object maps a method name to a RATE_LIMIT_TYPE. If not in this map the
+ * default is 'RATE_LIMITED'
+ */
+const RATE_LIMIT_MAP = {
+  [MESSAGE_TYPE.ETH_SIGN]: RATE_LIMIT_TYPES.NON_RATE_LIMITED,
+  [MESSAGE_TYPE.ETH_SIGN_TYPED_DATA]: RATE_LIMIT_TYPES.NON_RATE_LIMITED,
+  [MESSAGE_TYPE.ETH_SIGN_TYPED_DATA_V3]: RATE_LIMIT_TYPES.NON_RATE_LIMITED,
+  [MESSAGE_TYPE.ETH_SIGN_TYPED_DATA_V4]: RATE_LIMIT_TYPES.NON_RATE_LIMITED,
+  [MESSAGE_TYPE.PERSONAL_SIGN]: RATE_LIMIT_TYPES.NON_RATE_LIMITED,
+  [MESSAGE_TYPE.ETH_DECRYPT]: RATE_LIMIT_TYPES.NON_RATE_LIMITED,
+  [MESSAGE_TYPE.ETH_GET_ENCRYPTION_PUBLIC_KEY]:
+    RATE_LIMIT_TYPES.NON_RATE_LIMITED,
+  [MESSAGE_TYPE.ETH_REQUEST_ACCOUNTS]: RATE_LIMIT_TYPES.RATE_LIMITED,
+  [MESSAGE_TYPE.WALLET_REQUEST_PERMISSIONS]: RATE_LIMIT_TYPES.RATE_LIMITED,
+  [MESSAGE_TYPE.SEND_METADATA]: RATE_LIMIT_TYPES.BLOCKED,
+  [MESSAGE_TYPE.GET_PROVIDER_STATE]: RATE_LIMIT_TYPES.BLOCKED,
+};
+
+/**
+ * For events with user interaction (approve / reject | cancel) this map will
+ * return an object with APPROVED, REJECTED and REQUESTED keys that map to the
+ * appropriate event names.
+ */
+const EVENT_NAME_MAP = {
+  [MESSAGE_TYPE.ETH_SIGN]: {
+    APPROVED: EVENT_NAMES.SIGNATURE_APPROVED,
+    REJECTED: EVENT_NAMES.SIGNATURE_REJECTED,
+    REQUESTED: EVENT_NAMES.SIGNATURE_REQUESTED,
+  },
+  [MESSAGE_TYPE.ETH_SIGN_TYPED_DATA]: {
+    APPROVED: EVENT_NAMES.SIGNATURE_APPROVED,
+    REJECTED: EVENT_NAMES.SIGNATURE_REJECTED,
+    REQUESTED: EVENT_NAMES.SIGNATURE_REQUESTED,
+  },
+  [MESSAGE_TYPE.ETH_SIGN_TYPED_DATA_V3]: {
+    APPROVED: EVENT_NAMES.SIGNATURE_APPROVED,
+    REJECTED: EVENT_NAMES.SIGNATURE_REJECTED,
+    REQUESTED: EVENT_NAMES.SIGNATURE_REQUESTED,
+  },
+  [MESSAGE_TYPE.ETH_SIGN_TYPED_DATA_V4]: {
+    APPROVED: EVENT_NAMES.SIGNATURE_APPROVED,
+    REJECTED: EVENT_NAMES.SIGNATURE_REJECTED,
+    REQUESTED: EVENT_NAMES.SIGNATURE_REQUESTED,
+  },
+  [MESSAGE_TYPE.PERSONAL_SIGN]: {
+    APPROVED: EVENT_NAMES.SIGNATURE_APPROVED,
+    REJECTED: EVENT_NAMES.SIGNATURE_REJECTED,
+    REQUESTED: EVENT_NAMES.SIGNATURE_REQUESTED,
+  },
+  [MESSAGE_TYPE.ETH_DECRYPT]: {
+    APPROVED: EVENT_NAMES.DECRYPTION_APPROVED,
+    REJECTED: EVENT_NAMES.DECRYPTION_REJECTED,
+    REQUESTED: EVENT_NAMES.DECRYPTION_REQUESTED,
+  },
+  [MESSAGE_TYPE.ETH_GET_ENCRYPTION_PUBLIC_KEY]: {
+    APPROVED: EVENT_NAMES.ENCRYPTION_PUBLIC_KEY_APPROVED,
+    REJECTED: EVENT_NAMES.ENCRYPTION_PUBLIC_KEY_REJECTED,
+    REQUESTED: EVENT_NAMES.ENCRYPTION_PUBLIC_KEY_REQUESTED,
+  },
+  [MESSAGE_TYPE.ETH_REQUEST_ACCOUNTS]: {
+    APPROVED: EVENT_NAMES.PERMISSIONS_APPROVED,
+    REJECTED: EVENT_NAMES.PERMISSIONS_REJECTED,
+    REQUESTED: EVENT_NAMES.PERMISSIONS_REQUESTED,
+  },
+  [MESSAGE_TYPE.WALLET_REQUEST_PERMISSIONS]: {
+    APPROVED: EVENT_NAMES.PERMISSIONS_APPROVED,
+    REJECTED: EVENT_NAMES.PERMISSIONS_REJECTED,
+    REQUESTED: EVENT_NAMES.PERMISSIONS_REQUESTED,
+  },
+};
+
+const rateLimitTimeouts = {};
 
 /**
  * Returns a middleware that tracks inpage_provider usage using sampling for
@@ -21,65 +93,113 @@ const samplingTimeouts = {};
  * signature requests
  *
  * @param {object} opts - options for the rpc method tracking middleware
- * @param {Function} opts.trackEvent - trackEvent method from MetaMetricsController
- * @param {Function} opts.getMetricsState - get the state of MetaMetricsController
+ * @param {Function} opts.trackEvent - trackEvent method from
+ *  MetaMetricsController
+ * @param {Function} opts.getMetricsState - get the state of
+ *  MetaMetricsController
+ * @param {number} [opts.rateLimitSeconds] - number of seconds to wait before
+ *  allowing another set of events to be tracked.
  * @returns {Function}
  */
 export default function createRPCMethodTrackingMiddleware({
   trackEvent,
   getMetricsState,
+  rateLimitSeconds = 60,
 }) {
   return function rpcMethodTrackingMiddleware(
     /** @type {any} */ req,
     /** @type {any} */ res,
     /** @type {Function} */ next,
   ) {
-    const startTime = Date.now();
-    const { origin } = req;
+    const { origin, method } = req;
+
+    // Determine what type of rate limit to apply based on method
+    const rateLimitType =
+      RATE_LIMIT_MAP[method] ?? RATE_LIMIT_TYPES.RATE_LIMITED;
+
+    // If the rateLimitType is RATE_LIMITED check the rateLimitTimeouts
+    const rateLimited =
+      rateLimitType === RATE_LIMIT_TYPES.RATE_LIMITED &&
+      typeof rateLimitTimeouts[method] !== 'undefined';
+
+    // Get the participateInMetaMetrics state to determine if we should track
+    // anything. This is extra redundancy because this value is checked in
+    // the metametrics controller's trackEvent method as well.
+    const userParticipatingInMetaMetrics =
+      getMetricsState().participateInMetaMetrics === true;
+
+    // Get the event type, each of which has APPROVED, REJECTED and REQUESTED
+    // keys for the various events in the flow.
+    const eventType = EVENT_NAME_MAP[method];
+
+    // Boolean variable that reduces code duplication and increases legibility
+    const shouldTrackEvent =
+      // Don't track if the request came from our own UI or background
+      origin !== ORIGIN_METAMASK &&
+      // Don't track if this is a blocked method
+      rateLimitType !== RATE_LIMIT_TYPES.BLOCKED &&
+      // Don't track if the rate limit has been hit
+      rateLimited === false &&
+      // Don't track if the user isn't participating in metametrics
+      userParticipatingInMetaMetrics === true;
+
+    if (shouldTrackEvent) {
+      // We track an initial "requested" event as soon as the dapp calls the
+      // provider method. For the events not special cased this is the only
+      // event that will be fired and the event name will be
+      // 'Provider Method Called'.
+      const event = eventType
+        ? eventType.REQUESTED
+        : EVENT_NAMES.PROVIDER_METHOD_CALLED;
+
+      const properties = {};
+
+      if (event === EVENT_NAMES.SIGNATURE_REQUESTED) {
+        properties.signature_type = method;
+      } else {
+        properties.method = method;
+      }
+
+      trackEvent({
+        event,
+        category: EVENT.CATEGORIES.INPAGE_PROVIDER,
+        referrer: {
+          url: origin,
+        },
+        properties,
+      });
+
+      rateLimitTimeouts[method] = setTimeout(() => {
+        delete rateLimitTimeouts[method];
+      }, SECOND * rateLimitSeconds);
+    }
 
     next((callback) => {
-      const endTime = Date.now();
-      if (!getMetricsState().participateInMetaMetrics) {
+      if (shouldTrackEvent === false || typeof eventType === 'undefined') {
         return callback();
       }
-      if (USER_PROMPTED_EVENT_NAME_MAP[req.method]) {
-        const userRejected = res.error?.code === 4001;
-        trackEvent({
-          event: USER_PROMPTED_EVENT_NAME_MAP[req.method],
-          category: EVENT.CATEGORIES.INPAGE_PROVIDER,
-          referrer: {
-            url: origin,
-          },
-          properties: {
-            method: req.method,
-            status: userRejected ? 'rejected' : 'approved',
-            error_code: res.error?.code,
-            error_message: res.error?.message,
-            has_result: typeof res.result !== 'undefined',
-            duration: endTime - startTime,
-          },
-        });
-      } else if (typeof samplingTimeouts[req.method] === 'undefined') {
-        trackEvent({
-          event: 'Provider Method Called',
-          category: EVENT.CATEGORIES.INPAGE_PROVIDER,
-          referrer: {
-            url: origin,
-          },
-          properties: {
-            method: req.method,
-            error_code: res.error?.code,
-            error_message: res.error?.message,
-            has_result: typeof res.result !== 'undefined',
-            duration: endTime - startTime,
-          },
-        });
-        // Only record one call to this method every ten seconds to avoid
-        // overloading network requests.
-        samplingTimeouts[req.method] = setTimeout(() => {
-          delete samplingTimeouts[req.method];
-        }, SECOND * 10);
+
+      // An error code of 4001 means the user rejected the request, which we
+      // can use here to determine which event to track.
+      const event =
+        res.error?.code === 4001 ? eventType.REJECTED : eventType.APPROVED;
+
+      const properties = {};
+
+      if (eventType.REQUESTED === EVENT_NAMES.SIGNATURE_REQUESTED) {
+        properties.signature_type = method;
+      } else {
+        properties.method = method;
       }
+
+      trackEvent({
+        event,
+        category: EVENT.CATEGORIES.INPAGE_PROVIDER,
+        referrer: {
+          url: origin,
+        },
+        properties,
+      });
       return callback();
     });
   };

app/scripts/lib/createRPCMethodTrackingMiddleware.test.js

@@ -0,0 +1,217 @@
+import { MESSAGE_TYPE } from '../../../shared/constants/app';
+import { EVENT_NAMES } from '../../../shared/constants/metametrics';
+import { SECOND } from '../../../shared/constants/time';
+import createRPCMethodTrackingMiddleware from './createRPCMethodTrackingMiddleware';
+
+const trackEvent = jest.fn();
+const metricsState = { participateInMetaMetrics: null };
+const getMetricsState = () => metricsState;
+
+const handler = createRPCMethodTrackingMiddleware({
+  trackEvent,
+  getMetricsState,
+  rateLimitSeconds: 1,
+});
+
+function getNext(timeout = 500) {
+  let deferred;
+  const promise = new Promise((resolve) => {
+    deferred = {
+      resolve,
+    };
+  });
+  const cb = () => deferred.resolve();
+  let triggerNext;
+  setTimeout(() => {
+    deferred.resolve();
+  }, timeout);
+  return {
+    executeMiddlewareStack: async () => {
+      if (triggerNext) {
+        triggerNext(() => cb());
+      }
+      return await deferred.resolve();
+    },
+    promise,
+    next: (postReqHandler) => {
+      triggerNext = postReqHandler;
+    },
+  };
+}
+
+const waitForSeconds = async (seconds) =>
+  await new Promise((resolve) => setTimeout(resolve, SECOND * seconds));
+
+describe('createRPCMethodTrackingMiddleware', () => {
+  afterEach(() => {
+    jest.resetAllMocks();
+    metricsState.participateInMetaMetrics = null;
+  });
+
+  describe('before participateInMetaMetrics is set', () => {
+    it('should not track an event for a signature request', async () => {
+      const req = {
+        method: MESSAGE_TYPE.ETH_SIGN,
+        origin: 'some.dapp',
+      };
+
+      const res = {
+        error: null,
+      };
+      const { executeMiddlewareStack, next } = getNext();
+      handler(req, res, next);
+      await executeMiddlewareStack();
+      expect(trackEvent).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('participateInMetaMetrics is set to false', () => {
+    beforeEach(() => {
+      metricsState.participateInMetaMetrics = false;
+    });
+
+    it('should not track an event for a signature request', async () => {
+      const req = {
+        method: MESSAGE_TYPE.ETH_SIGN,
+        origin: 'some.dapp',
+      };
+
+      const res = {
+        error: null,
+      };
+      const { executeMiddlewareStack, next } = getNext();
+      handler(req, res, next);
+      await executeMiddlewareStack();
+      expect(trackEvent).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('participateInMetaMetrics is set to true', () => {
+    beforeEach(() => {
+      metricsState.participateInMetaMetrics = true;
+    });
+
+    it(`should immediately track a ${EVENT_NAMES.SIGNATURE_REQUESTED} event`, () => {
+      const req = {
+        method: MESSAGE_TYPE.ETH_SIGN,
+        origin: 'some.dapp',
+      };
+
+      const res = {
+        error: null,
+      };
+      const { next } = getNext();
+      handler(req, res, next);
+      expect(trackEvent).toHaveBeenCalledTimes(1);
+      expect(trackEvent.mock.calls[0][0]).toMatchObject({
+        category: 'inpage_provider',
+        event: EVENT_NAMES.SIGNATURE_REQUESTED,
+        properties: { signature_type: MESSAGE_TYPE.ETH_SIGN },
+        referrer: { url: 'some.dapp' },
+      });
+    });
+
+    it(`should track a ${EVENT_NAMES.SIGNATURE_APPROVED} event if the user approves`, async () => {
+      const req = {
+        method: MESSAGE_TYPE.ETH_SIGN_TYPED_DATA_V4,
+        origin: 'some.dapp',
+      };
+
+      const res = {
+        error: null,
+      };
+      const { next, executeMiddlewareStack } = getNext();
+      handler(req, res, next);
+      await executeMiddlewareStack();
+      expect(trackEvent).toHaveBeenCalledTimes(2);
+      expect(trackEvent.mock.calls[1][0]).toMatchObject({
+        category: 'inpage_provider',
+        event: EVENT_NAMES.SIGNATURE_APPROVED,
+        properties: { signature_type: MESSAGE_TYPE.ETH_SIGN_TYPED_DATA_V4 },
+        referrer: { url: 'some.dapp' },
+      });
+    });
+
+    it(`should track a ${EVENT_NAMES.SIGNATURE_REJECTED} event if the user approves`, async () => {
+      const req = {
+        method: MESSAGE_TYPE.PERSONAL_SIGN,
+        origin: 'some.dapp',
+      };
+
+      const res = {
+        error: { code: 4001 },
+      };
+      const { next, executeMiddlewareStack } = getNext();
+      handler(req, res, next);
+      await executeMiddlewareStack();
+      expect(trackEvent).toHaveBeenCalledTimes(2);
+      expect(trackEvent.mock.calls[1][0]).toMatchObject({
+        category: 'inpage_provider',
+        event: EVENT_NAMES.SIGNATURE_REJECTED,
+        properties: { signature_type: MESSAGE_TYPE.PERSONAL_SIGN },
+        referrer: { url: 'some.dapp' },
+      });
+    });
+
+    it(`should track a ${EVENT_NAMES.PERMISSIONS_APPROVED} event if the user approves`, async () => {
+      const req = {
+        method: MESSAGE_TYPE.ETH_REQUEST_ACCOUNTS,
+        origin: 'some.dapp',
+      };
+
+      const res = {};
+      const { next, executeMiddlewareStack } = getNext();
+      handler(req, res, next);
+      await executeMiddlewareStack();
+      expect(trackEvent).toHaveBeenCalledTimes(2);
+      expect(trackEvent.mock.calls[1][0]).toMatchObject({
+        category: 'inpage_provider',
+        event: EVENT_NAMES.PERMISSIONS_APPROVED,
+        properties: { method: MESSAGE_TYPE.ETH_REQUEST_ACCOUNTS },
+        referrer: { url: 'some.dapp' },
+      });
+    });
+
+    it(`should never track blocked methods such as ${MESSAGE_TYPE.GET_PROVIDER_STATE}`, () => {
+      const req = {
+        method: MESSAGE_TYPE.GET_PROVIDER_STATE,
+        origin: 'www.notadapp.com',
+      };
+
+      const res = {
+        error: null,
+      };
+      const { next, executeMiddlewareStack } = getNext();
+      handler(req, res, next);
+      expect(trackEvent).not.toHaveBeenCalled();
+      executeMiddlewareStack();
+    });
+
+    it(`should only track events when not rate limited`, async () => {
+      const req = {
+        method: 'eth_chainId',
+        origin: 'some.dapp',
+      };
+
+      const res = {
+        error: null,
+      };
+
+      let callCount = 0;
+
+      while (callCount < 3) {
+        callCount += 1;
+        const { next, executeMiddlewareStack } = getNext();
+        handler(req, res, next);
+        await executeMiddlewareStack();
+        if (callCount !== 3) {
+          await waitForSeconds(0.6);
+        }
+      }
+
+      expect(trackEvent).toHaveBeenCalledTimes(2);
+      expect(trackEvent.mock.calls[0][0].properties.method).toBe('eth_chainId');
+      expect(trackEvent.mock.calls[1][0].properties.method).toBe('eth_chainId');
+    });
+  });
+});

jest.config.js

@@ -1,6 +1,7 @@
 module.exports = {
   collectCoverageFrom: [
     '<rootDir>/app/scripts/controllers/permissions/**/*.js',
+    '<rootDir>/app/scripts/lib/createRPCMethodTrackingMiddleware.js',
     '<rootDir>/shared/**/*.js',
     '<rootDir>/ui/**/*.js',
   ],
@@ -20,6 +21,12 @@ module.exports = {
       lines: 100,
       statements: 100,
     },
+    './app/scripts/lib/createRPCMethodTrackingMiddleware.js': {
+      branches: 95.65,
+      functions: 100,
+      lines: 100,
+      statements: 100,
+    },
   },
   // TODO: enable resetMocks
   // resetMocks: true,
@@ -34,6 +41,7 @@ module.exports = {
     '<rootDir>/app/scripts/platforms/*.test.js',
     '<rootDir>app/scripts/controllers/network/**/*.test.js',
     '<rootDir>/app/scripts/controllers/permissions/**/*.test.js',
+    '<rootDir>/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js',
   ],
   testTimeout: 2500,
   transform: {

shared/constants/app.js

@@ -39,11 +39,14 @@ export const MESSAGE_TYPE = {
   ETH_REQUEST_ACCOUNTS: 'eth_requestAccounts',
   ETH_SIGN: 'eth_sign',
   ETH_SIGN_TYPED_DATA: 'eth_signTypedData',
+  ETH_SIGN_TYPED_DATA_V3: 'eth_signTypedData_v3',
+  ETH_SIGN_TYPED_DATA_V4: 'eth_signTypedData_v4',
   GET_PROVIDER_STATE: 'metamask_getProviderState',
   LOG_WEB3_SHIM_USAGE: 'metamask_logWeb3ShimUsage',
   PERSONAL_SIGN: 'personal_sign',
   SEND_METADATA: 'metamask_sendDomainMetadata',
   SWITCH_ETHEREUM_CHAIN: 'wallet_switchEthereumChain',
+  WALLET_REQUEST_PERMISSIONS: 'wallet_requestPermissions',
   WATCH_ASSET: 'wallet_watchAsset',
   WATCH_ASSET_LEGACY: 'metamask_watchAsset',
   ///: BEGIN:ONLY_INCLUDE_IN(flask)

shared/constants/metametrics.js

@@ -276,9 +276,18 @@ export const REJECT_NOTFICIATION_CLOSE_SIG =
  */
 
 export const EVENT_NAMES = {
+  ENCRYPTION_PUBLIC_KEY_APPROVED: 'Encryption Public Key Approved',
+  ENCRYPTION_PUBLIC_KEY_REJECTED: 'Encryption Public Key Rejected',
   ENCRYPTION_PUBLIC_KEY_REQUESTED: 'Encryption Public Key Requested',
+  DECRYPTION_APPROVED: 'Decryption Approved',
+  DECRYPTION_REJECTED: 'Decryption Rejected',
   DECRYPTION_REQUESTED: 'Decryption Requested',
+  PERMISSIONS_APPROVED: 'Permissions Approved',
+  PERMISSIONS_REJECTED: 'Permissions Rejected',
   PERMISSIONS_REQUESTED: 'Permissions Requested',
+  PROVIDER_METHOD_CALLED: 'Provider Method Called',
+  SIGNATURE_APPROVED: 'Signature Approved',
+  SIGNATURE_REJECTED: 'Signature Rejected',
   SIGNATURE_REQUESTED: 'Signature Requested',
   TOKEN_ADDED: 'Token Added',
   TOKEN_DETECTED: 'Token Detected',