diff --git a/.storybook/test-data.js b/.storybook/test-data.js index 32c24690f..18f384647 100644 --- a/.storybook/test-data.js +++ b/.storybook/test-data.js @@ -140,7 +140,6 @@ const state = { } }, "participateInMetaMetrics": true, - "metaMetricsSendCount": 2, "nextNonce": 71, "connectedStatusPopoverHasBeenShown": true, "swapsWelcomeMessageHasBeenShown": true, diff --git a/app/scripts/controllers/metametrics.js b/app/scripts/controllers/metametrics.js index c55a97c85..1539cbcba 100644 --- a/app/scripts/controllers/metametrics.js +++ b/app/scripts/controllers/metametrics.js @@ -7,30 +7,6 @@ import { METAMETRICS_BACKGROUND_PAGE_OBJECT, } from '../../../shared/constants/metametrics'; -/** - * Used to determine whether or not to attach a user's metametrics id - * to events that include on-chain data. This helps to prevent identifying - * a user by being able to trace their activity on etherscan/block exploring - */ -const trackableSendCounts = { - 1: true, - 10: true, - 30: true, - 50: true, - 100: true, - 250: true, - 500: true, - 1000: true, - 2500: true, - 5000: true, - 10000: true, - 25000: true, -}; - -export function sendCountIsTrackable(sendCount) { - return Boolean(trackableSendCounts[sendCount]); -} - /** * @typedef {import('../../../shared/constants/metametrics').MetaMetricsContext} MetaMetricsContext * @typedef {import('../../../shared/constants/metametrics').MetaMetricsEventPayload} MetaMetricsEventPayload @@ -48,9 +24,6 @@ export function sendCountIsTrackable(sendCount) { * @property {?boolean} participateInMetaMetrics - The user's preference for * participating in the MetaMetrics analytics program. This setting controls * whether or not events are tracked - * @property {number} metaMetricsSendCount - How many send transactions have - * been tracked through this controller. Used to prevent attaching sensitive - * data that can be traced through on chain data. */ export default class MetaMetricsController { @@ -89,7 +62,6 @@ export default class MetaMetricsController { this.store = new ObservableStore({ participateInMetaMetrics: null, metaMetricsId: null, - metaMetricsSendCount: 0, ...initState, }); @@ -138,10 +110,6 @@ export default class MetaMetricsController { return this.store.getState(); } - setMetaMetricsSendCount(val) { - this.store.updateState({ metaMetricsSendCount: val }); - } - /** * Build the context object to attach to page and track events. * @private @@ -231,11 +199,7 @@ export default class MetaMetricsController { // to be updated to work with the new tracking plan. I think we should use // a config setting for this instead of trying to match the event name const isSendFlow = Boolean(payload.event.match(/^send|^confirm/iu)); - if ( - isSendFlow && - this.state.metaMetricsSendCount && - !sendCountIsTrackable(this.state.metaMetricsSendCount + 1) - ) { + if (isSendFlow) { excludeMetaMetricsId = true; } // If we are tracking sensitive data we will always use the anonymousId diff --git a/app/scripts/controllers/metametrics.test.js b/app/scripts/controllers/metametrics.test.js index e30b6ad5e..bb0d15bed 100644 --- a/app/scripts/controllers/metametrics.test.js +++ b/app/scripts/controllers/metametrics.test.js @@ -84,7 +84,6 @@ function getMockPreferencesStore({ currentLocale = LOCALE } = {}) { function getMetaMetricsController({ participateInMetaMetrics = true, metaMetricsId = TEST_META_METRICS_ID, - metaMetricsSendCount = 0, preferencesStore = getMockPreferencesStore(), networkController = getMockNetworkController(), } = {}) { @@ -106,7 +105,6 @@ function getMetaMetricsController({ initState: { participateInMetaMetrics, metaMetricsId, - metaMetricsSendCount, }, }); } @@ -198,14 +196,6 @@ describe('MetaMetricsController', function () { }); }); - describe('setMetaMetricsSendCount', function () { - it('should update the send count in state', function () { - const metaMetricsController = getMetaMetricsController(); - metaMetricsController.setMetaMetricsSendCount(1); - assert.equal(metaMetricsController.state.metaMetricsSendCount, 1); - }); - }); - describe('trackEvent', function () { it('should not track an event if user is not participating in metametrics', function () { const mock = sinon.mock(segment); @@ -337,61 +327,6 @@ describe('MetaMetricsController', function () { mock.verify(); }); - it('should use anonymousId when metametrics send count is not trackable in send flow', function () { - const mock = sinon.mock(segment); - const metaMetricsController = getMetaMetricsController({ - metaMetricsSendCount: 1, - }); - mock - .expects('track') - .once() - .withArgs({ - event: 'Send Fake Event', - anonymousId: METAMETRICS_ANONYMOUS_ID, - context: DEFAULT_TEST_CONTEXT, - properties: { - test: 1, - ...DEFAULT_EVENT_PROPERTIES, - }, - }); - metaMetricsController.trackEvent({ - event: 'Send Fake Event', - category: 'Unit Test', - properties: { - test: 1, - }, - }); - mock.verify(); - }); - - it('should use user metametrics id when metametrics send count is trackable in send flow', function () { - const mock = sinon.mock(segment); - const metaMetricsController = getMetaMetricsController(); - mock - .expects('track') - .once() - .withArgs({ - event: 'Send Fake Event', - userId: TEST_META_METRICS_ID, - context: DEFAULT_TEST_CONTEXT, - properties: { - test: 1, - ...DEFAULT_EVENT_PROPERTIES, - }, - }); - metaMetricsController.trackEvent( - { - event: 'Send Fake Event', - category: 'Unit Test', - properties: { - test: 1, - }, - }, - { metaMetricsSendCount: 0 }, - ); - mock.verify(); - }); - it('should immediately flush queue if flushImmediately set to true', async function () { const metaMetricsController = getMetaMetricsController(); const flushStub = sinon.stub(segment, 'flush'); diff --git a/app/scripts/lib/setupSentry.js b/app/scripts/lib/setupSentry.js index 25bbd7a46..56625b220 100644 --- a/app/scripts/lib/setupSentry.js +++ b/app/scripts/lib/setupSentry.js @@ -36,7 +36,6 @@ export const SENTRY_STATE = { isInitialized: true, isUnlocked: true, metaMetricsId: true, - metaMetricsSendCount: true, nativeCurrency: true, network: true, nextNonce: true, diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index e2546a91f..31186aaa3 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -676,7 +676,6 @@ export default class MetamaskController extends EventEmitter { setUsePhishDetect: this.setUsePhishDetect.bind(this), setIpfsGateway: this.setIpfsGateway.bind(this), setParticipateInMetaMetrics: this.setParticipateInMetaMetrics.bind(this), - setMetaMetricsSendCount: this.setMetaMetricsSendCount.bind(this), setFirstTimeFlowType: this.setFirstTimeFlowType.bind(this), setCurrentLocale: this.setCurrentLocale.bind(this), markPasswordForgotten: this.markPasswordForgotten.bind(this), @@ -2760,18 +2759,6 @@ export default class MetamaskController extends EventEmitter { } } - setMetaMetricsSendCount(val, cb) { - try { - this.metaMetricsController.setMetaMetricsSendCount(val); - cb(null); - return; - } catch (err) { - cb(err); - // eslint-disable-next-line no-useless-return - return; - } - } - /** * Sets the type of first time flow the user wishes to follow: create or import * @param {string} type - Indicates the type of first time flow the user wishes to follow diff --git a/app/scripts/migrations/062.js b/app/scripts/migrations/062.js new file mode 100644 index 000000000..54b52b967 --- /dev/null +++ b/app/scripts/migrations/062.js @@ -0,0 +1,28 @@ +import { cloneDeep } from 'lodash'; + +const version = 62; + +/** + * Removes metaMetricsSendCount from MetaMetrics controller + */ +export default { + version, + async migrate(originalVersionedData) { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + const state = versionedData.data; + const newState = transformState(state); + versionedData.data = newState; + return versionedData; + }, +}; + +function transformState(state) { + if (state.MetaMetricsController) { + const { metaMetricsSendCount } = state.MetaMetricsController; + if (metaMetricsSendCount !== undefined) { + delete state.MetaMetricsController.metaMetricsSendCount; + } + } + return state; +} diff --git a/app/scripts/migrations/062.test.js b/app/scripts/migrations/062.test.js new file mode 100644 index 000000000..44229ac1d --- /dev/null +++ b/app/scripts/migrations/062.test.js @@ -0,0 +1,80 @@ +import { strict as assert } from 'assert'; +import migration62 from './062'; + +describe('migration #62', function () { + it('should update the version metadata', async function () { + const oldStorage = { + meta: { + version: 61, + }, + data: {}, + }; + + const newStorage = await migration62.migrate(oldStorage); + assert.deepEqual(newStorage.meta, { + version: 62, + }); + }); + + it('should remove metaMetricsSendCount from MetaMetricsController', async function () { + const oldStorage = { + meta: {}, + data: { + MetaMetricsController: { + metaMetricsSendCount: 1, + bar: 'baz', + }, + foo: 'bar', + }, + }; + + const newStorage = await migration62.migrate(oldStorage); + assert.deepStrictEqual(newStorage.data, { + MetaMetricsController: { + bar: 'baz', + }, + foo: 'bar', + }); + }); + + it('should remove metaMetricsSendCount from MetaMetricsController (falsey but defined)', async function () { + const oldStorage = { + meta: {}, + data: { + MetaMetricsController: { + metaMetricsSendCount: 0, + bar: 'baz', + }, + foo: 'bar', + }, + }; + + const newStorage = await migration62.migrate(oldStorage); + assert.deepStrictEqual(newStorage.data, { + MetaMetricsController: { + bar: 'baz', + }, + foo: 'bar', + }); + }); + + it('should not modify MetaMetricsController when metaMetricsSendCount is undefined', async function () { + const oldStorage = { + meta: {}, + data: { + MetaMetricsController: { + bar: 'baz', + }, + foo: 'bar', + }, + }; + + const newStorage = await migration62.migrate(oldStorage); + assert.deepStrictEqual(newStorage.data, { + MetaMetricsController: { + bar: 'baz', + }, + foo: 'bar', + }); + }); +}); diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index 175ee0044..389b4ef4a 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -66,6 +66,7 @@ const migrations = [ require('./059').default, require('./060').default, require('./061').default, + require('./062').default, ]; export default migrations; diff --git a/test/e2e/fixtures/address-entry/state.json b/test/e2e/fixtures/address-entry/state.json index 0bc9f3b9d..c17fd7e0c 100644 --- a/test/e2e/fixtures/address-entry/state.json +++ b/test/e2e/fixtures/address-entry/state.json @@ -128,7 +128,6 @@ "knownMethodData": {}, "lostIdentities": {}, "metaMetricsId": null, - "metaMetricsSendCount": 0, "participateInMetaMetrics": false, "preferences": { "useNativeCurrencyAsPrimaryCurrency": true diff --git a/test/e2e/fixtures/connected-state/state.json b/test/e2e/fixtures/connected-state/state.json index baebea6b7..5d3d2f68e 100644 --- a/test/e2e/fixtures/connected-state/state.json +++ b/test/e2e/fixtures/connected-state/state.json @@ -138,7 +138,6 @@ }, "completedOnboarding": true, "metaMetricsId": null, - "metaMetricsSendCount": 0, "ipfsGateway": "dweb.link", "selectedAddress": "0x5cfe73b6021e818b776b421b1c4db2474086a7e1" }, diff --git a/test/e2e/fixtures/custom-rpc/state.json b/test/e2e/fixtures/custom-rpc/state.json index aa93a938c..54d6bb4a8 100644 --- a/test/e2e/fixtures/custom-rpc/state.json +++ b/test/e2e/fixtures/custom-rpc/state.json @@ -129,7 +129,6 @@ "knownMethodData": {}, "lostIdentities": {}, "metaMetricsId": null, - "metaMetricsSendCount": 0, "participateInMetaMetrics": false, "preferences": { "useNativeCurrencyAsPrimaryCurrency": true diff --git a/test/e2e/fixtures/custom-token/state.json b/test/e2e/fixtures/custom-token/state.json index 1d3a24437..bd85436ad 100644 --- a/test/e2e/fixtures/custom-token/state.json +++ b/test/e2e/fixtures/custom-token/state.json @@ -121,7 +121,6 @@ "knownMethodData": {}, "lostIdentities": {}, "metaMetricsId": null, - "metaMetricsSendCount": 0, "participateInMetaMetrics": false, "preferences": { "useNativeCurrencyAsPrimaryCurrency": true diff --git a/test/e2e/fixtures/import-ui/state.json b/test/e2e/fixtures/import-ui/state.json index 1e6572574..758a85d66 100644 --- a/test/e2e/fixtures/import-ui/state.json +++ b/test/e2e/fixtures/import-ui/state.json @@ -125,8 +125,7 @@ }, "MetaMetricsController": { "participateInMetaMetrics": false, - "metaMetricsId": null, - "metaMetricsSendCount": 1 + "metaMetricsId": null }, "PermissionsController": { "permissionsRequests": [], diff --git a/test/e2e/fixtures/imported-account/state.json b/test/e2e/fixtures/imported-account/state.json index 7c4a2108a..407bbbd83 100644 --- a/test/e2e/fixtures/imported-account/state.json +++ b/test/e2e/fixtures/imported-account/state.json @@ -114,7 +114,6 @@ "knownMethodData": {}, "lostIdentities": {}, "metaMetricsId": null, - "metaMetricsSendCount": 0, "participateInMetaMetrics": false, "preferences": { "useNativeCurrencyAsPrimaryCurrency": true diff --git a/test/e2e/fixtures/localization/state.json b/test/e2e/fixtures/localization/state.json index 009e8ac77..73a3b97b2 100644 --- a/test/e2e/fixtures/localization/state.json +++ b/test/e2e/fixtures/localization/state.json @@ -114,7 +114,6 @@ "knownMethodData": {}, "lostIdentities": {}, "metaMetricsId": null, - "metaMetricsSendCount": 0, "participateInMetaMetrics": false, "preferences": { "showFiatInTestnets": true, diff --git a/test/e2e/fixtures/metrics-enabled/state.json b/test/e2e/fixtures/metrics-enabled/state.json index 8e0f082d5..67cbf9977 100644 --- a/test/e2e/fixtures/metrics-enabled/state.json +++ b/test/e2e/fixtures/metrics-enabled/state.json @@ -138,7 +138,6 @@ }, "completedOnboarding": true, "metaMetricsId": "fake-metrics-id", - "metaMetricsSendCount": 0, "ipfsGateway": "dweb.link", "selectedAddress": "0x5cfe73b6021e818b776b421b1c4db2474086a7e1" }, diff --git a/test/e2e/fixtures/send-edit/state.json b/test/e2e/fixtures/send-edit/state.json index a5f3a8bab..6c9658c28 100644 --- a/test/e2e/fixtures/send-edit/state.json +++ b/test/e2e/fixtures/send-edit/state.json @@ -115,7 +115,6 @@ "knownMethodData": {}, "lostIdentities": {}, "metaMetricsId": null, - "metaMetricsSendCount": 0, "participateInMetaMetrics": false, "preferences": { "useNativeCurrencyAsPrimaryCurrency": true diff --git a/test/e2e/fixtures/threebox-enabled/state.json b/test/e2e/fixtures/threebox-enabled/state.json index f182c5d47..abad9782b 100644 --- a/test/e2e/fixtures/threebox-enabled/state.json +++ b/test/e2e/fixtures/threebox-enabled/state.json @@ -120,8 +120,7 @@ }, "MetaMetricsController": { "metaMetricsId": null, - "participateInMetaMetrics": false, - "metaMetricsSendCount": 0 + "participateInMetaMetrics": false }, "ThreeBoxController": { "threeBoxSyncingAllowed": true, diff --git a/ui/ducks/metamask/metamask.js b/ui/ducks/metamask/metamask.js index 21a0476ea..0047b6904 100644 --- a/ui/ducks/metamask/metamask.js +++ b/ui/ducks/metamask/metamask.js @@ -33,7 +33,6 @@ export default function reduceMetamask(state = {}, action) { completedOnboarding: false, knownMethodData: {}, participateInMetaMetrics: null, - metaMetricsSendCount: 0, nextNonce: null, conversionRate: null, nativeCurrency: 'ETH', @@ -126,12 +125,6 @@ export default function reduceMetamask(state = {}, action) { participateInMetaMetrics: action.value, }; - case actionConstants.SET_METAMETRICS_SEND_COUNT: - return { - ...metamaskState, - metaMetricsSendCount: action.value, - }; - case actionConstants.SET_USE_BLOCKIE: return { ...metamaskState, diff --git a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js index 8a8e9e47e..99544f587 100644 --- a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js +++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js @@ -86,8 +86,6 @@ export default class ConfirmTransactionBase extends Component { hideSubtitle: PropTypes.bool, identiconAddress: PropTypes.string, onEdit: PropTypes.func, - setMetaMetricsSendCount: PropTypes.func, - metaMetricsSendCount: PropTypes.number, subtitleComponent: PropTypes.node, title: PropTypes.string, advancedInlineGasShown: PropTypes.bool, @@ -475,35 +473,16 @@ export default class ConfirmTransactionBase extends Component { } handleCancel() { - const { metricsEvent } = this.context; const { txData, cancelTransaction, history, mostRecentOverviewPage, clearConfirmTransaction, - actionKey, - txData: { origin }, - methodData = {}, updateCustomNonce, } = this.props; this._removeBeforeUnload(); - metricsEvent({ - eventOpts: { - category: 'Transactions', - action: 'Confirm Screen', - name: 'Cancel', - }, - customVariables: { - recipientKnown: null, - functionType: - actionKey || - getMethodName(methodData.name) || - TRANSACTION_TYPES.CONTRACT_INTERACTION, - origin, - }, - }); updateCustomNonce(''); cancelTransaction(txData).then(() => { clearConfirmTransaction(); @@ -512,18 +491,12 @@ export default class ConfirmTransactionBase extends Component { } handleSubmit() { - const { metricsEvent } = this.context; const { - txData: { origin }, sendTransaction, clearConfirmTransaction, txData, history, - actionKey, mostRecentOverviewPage, - metaMetricsSendCount = 0, - setMetaMetricsSendCount, - methodData = {}, updateCustomNonce, } = this.props; const { submitting } = this.state; @@ -539,44 +512,27 @@ export default class ConfirmTransactionBase extends Component { }, () => { this._removeBeforeUnload(); - metricsEvent({ - eventOpts: { - category: 'Transactions', - action: 'Confirm Screen', - name: 'Transaction Completed', - }, - customVariables: { - recipientKnown: null, - functionType: - actionKey || - getMethodName(methodData.name) || - TRANSACTION_TYPES.CONTRACT_INTERACTION, - origin, - }, - }); - setMetaMetricsSendCount(metaMetricsSendCount + 1).then(() => { - sendTransaction(txData) - .then(() => { - clearConfirmTransaction(); - this.setState( - { - submitting: false, - }, - () => { - history.push(mostRecentOverviewPage); - updateCustomNonce(''); - }, - ); - }) - .catch((error) => { - this.setState({ + sendTransaction(txData) + .then(() => { + clearConfirmTransaction(); + this.setState( + { submitting: false, - submitError: error.message, - }); - updateCustomNonce(''); + }, + () => { + history.push(mostRecentOverviewPage); + updateCustomNonce(''); + }, + ); + }) + .catch((error) => { + this.setState({ + submitting: false, + submitError: error.message, }); - }); + updateCustomNonce(''); + }); }, ); } @@ -643,18 +599,7 @@ export default class ConfirmTransactionBase extends Component { } _beforeUnload = () => { - const { txData: { origin, id } = {}, cancelTransaction } = this.props; - const { metricsEvent } = this.context; - metricsEvent({ - eventOpts: { - category: 'Transactions', - action: 'Confirm Screen', - name: 'Cancel Tx Via Notification Close', - }, - customVariables: { - origin, - }, - }); + const { txData: { id } = {}, cancelTransaction } = this.props; cancelTransaction({ id }); }; diff --git a/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js b/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js index 6af7e49a9..711b12157 100644 --- a/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js +++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js @@ -10,7 +10,6 @@ import { cancelTxs, updateAndApproveTx, showModal, - setMetaMetricsSendCount, updateTransaction, getNextNonce, tryReverseResolveAddress, @@ -76,7 +75,6 @@ const mapStateToProps = (state, ownProps) => { assetImages, network, unapprovedTxs, - metaMetricsSendCount, nextNonce, provider: { chainId }, } = metamask; @@ -186,7 +184,6 @@ const mapStateToProps = (state, ownProps) => { insufficientBalance, hideSubtitle: !isMainnet && !showFiatInTestnets, hideFiatConversion: !isMainnet && !showFiatInTestnets, - metaMetricsSendCount, type, nextNonce, mostRecentOverviewPage: getMostRecentOverviewPage(state), @@ -234,7 +231,6 @@ export const mapDispatchToProps = (dispatch) => { cancelAllTransactions: (txList) => dispatch(cancelTxs(txList)), sendTransaction: (txData) => dispatch(updateAndApproveTx(customNonceMerge(txData))), - setMetaMetricsSendCount: (val) => dispatch(setMetaMetricsSendCount(val)), getNextNonce: () => dispatch(getNextNonce()), setDefaultHomeActiveTabName: (tabName) => dispatch(setDefaultHomeActiveTabName(tabName)), diff --git a/ui/store/actionConstants.js b/ui/store/actionConstants.js index d16caa61f..ed0bee9af 100644 --- a/ui/store/actionConstants.js +++ b/ui/store/actionConstants.js @@ -60,7 +60,6 @@ export const UPDATE_CUSTOM_NONCE = 'UPDATE_CUSTOM_NONCE'; export const SET_IPFS_GATEWAY = 'SET_IPFS_GATEWAY'; export const SET_PARTICIPATE_IN_METAMETRICS = 'SET_PARTICIPATE_IN_METAMETRICS'; -export const SET_METAMETRICS_SEND_COUNT = 'SET_METAMETRICS_SEND_COUNT'; // locale export const SET_CURRENT_LOCALE = 'SET_CURRENT_LOCALE'; diff --git a/ui/store/actions.js b/ui/store/actions.js index f2269cdad..8dc7e9ee9 100644 --- a/ui/store/actions.js +++ b/ui/store/actions.js @@ -1968,27 +1968,6 @@ export function setParticipateInMetaMetrics(val) { }; } -export function setMetaMetricsSendCount(val) { - return (dispatch) => { - log.debug(`background.setMetaMetricsSendCount`); - return new Promise((resolve, reject) => { - background.setMetaMetricsSendCount(val, (err) => { - if (err) { - dispatch(displayWarning(err.message)); - reject(err); - return; - } - - dispatch({ - type: actionConstants.SET_METAMETRICS_SEND_COUNT, - value: val, - }); - resolve(val); - }); - }); - }; -} - export function setUseBlockie(val) { return (dispatch) => { dispatch(showLoadingIndication());