From 7c362090662b530eb497e30aaa7fc244ab0e62bd Mon Sep 17 00:00:00 2001 From: jiexi Date: Wed, 22 May 2024 08:58:28 -0700 Subject: [PATCH] feat: Add RPC Method Middleware params transform and tracking (#23236) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Currently we track a sample of most RPC method calls, but we do not include params. This is for good reason of course since many param values are sensitive and may identify an otherwise anonymous MetaMask Metrics user. There are some param values that are not sensitive yet are not tracked as well. Our goal is to glean additional insight into the usage of our RPC methods by including non-sensitive params in the events that track them. This PR achieves that by including a map of method to transformation functions which accepts a request's params object (or array) and transforms it something that is acceptable for analytics. By default no params are included. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23236?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2167 ## **Manual testing steps** 1. Opt into MetaMetrics in the wallet settings 1. Open your the background console for extension 1. Add a breakpoint for trackEvent inside `createRPCMethodTrackingMiddleware.js` 1. On any page with a provider injected, use `window.ethereum.request` to make some rpc method calls 1. Check background console for breakpoint trigger * Currently only `wallet_watchAsset` should include the `type` param. All other methods should not include any params. * [the wallet_watchAsset in API playground](https://docs.metamask.io/wallet/reference/wallet_watchasset/) may be helpful to use NOTE: I recommend breakpoint and debugger over the network tab because I don't believe the tracking requests actually fire on dev. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [x] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [x] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [x] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Alex Donesky --- .../lib/createRPCMethodTrackingMiddleware.js | 16 ++++- .../createRPCMethodTrackingMiddleware.test.js | 71 +++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/app/scripts/lib/createRPCMethodTrackingMiddleware.js b/app/scripts/lib/createRPCMethodTrackingMiddleware.js index e5175326edea..8ca3af21fd1b 100644 --- a/app/scripts/lib/createRPCMethodTrackingMiddleware.js +++ b/app/scripts/lib/createRPCMethodTrackingMiddleware.js @@ -119,6 +119,15 @@ const EVENT_NAME_MAP = { }, }; +/** + * This object maps a method name to a function that accept the method params and + * returns a non-sensitive version that can be included in tracked events. + * The default is to return undefined. + */ +const TRANSFORM_PARAMS_MAP = { + [MESSAGE_TYPE.WATCH_ASSET]: ({ type }) => ({ type }), +}; + const rateLimitTimeoutsByMethod = {}; let globalRateLimitCount = 0; @@ -170,7 +179,7 @@ export default function createRPCMethodTrackingMiddleware({ /** @type {any} */ res, /** @type {Function} */ next, ) { - const { origin, method } = req; + const { origin, method, params } = req; const rateLimitType = RATE_LIMIT_MAP[method] ?? RATE_LIMIT_TYPES.RANDOM_SAMPLE; @@ -302,6 +311,11 @@ export default function createRPCMethodTrackingMiddleware({ eventProperties.method = method; } + const transformParams = TRANSFORM_PARAMS_MAP[method]; + if (transformParams) { + eventProperties.params = transformParams(params); + } + trackEvent({ event, category: MetaMetricsEventCategory.InpageProvider, diff --git a/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js b/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js index 347ef963b1b6..d02153079743 100644 --- a/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js +++ b/app/scripts/lib/createRPCMethodTrackingMiddleware.test.js @@ -561,5 +561,76 @@ describe('createRPCMethodTrackingMiddleware', () => { }); }); }); + + it.each([ + ['no params', 'method_without_transform', {}, undefined], + [ + 'no params', + 'eth_call', + [ + { + accessList: [], + blobVersionedHashes: [], + blobs: [], + to: null, + }, + null, + ], + undefined, + ], + ['no params', 'eth_sandRawTransaction', ['0xdeadbeef'], undefined], + [ + 'no params', + 'eth_sendTransaction', + { + to: '0x1', + from: '0x2', + gas: '0x3', + gasPrice: '0x4', + value: '0x5', + data: '0xdeadbeef', + maxPriorityFeePerGas: '0x6', + maxFeePerGas: '0x7', + }, + undefined, + ], + [ + 'only the type param', + 'wallet_watchAsset', + { + type: 'ERC20', + options: { + address: '0xb60e8dd61c5d32be8058bb8eb970870f07233155', + symbol: 'FOO', + decimals: 18, + image: 'https://foo.io/token-image.svg', + }, + }, + { type: 'ERC20' }, + ], + ])( + `should include %s in the '%s' tracked events params property`, + async (_, method, params, expected) => { + const req = { + method, + origin: 'some.dapp', + params, + }; + + const res = { + error: null, + }; + + const { next } = getNext(); + const handler = createHandler({ rateLimitSamplePercent: 1 }); + + await handler(req, res, next); + + expect(trackEvent).toHaveBeenCalledTimes(1); + expect(trackEvent.mock.calls[0][0].properties.params).toStrictEqual( + expected, + ); + }, + ); }); });