1
0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-11-22 18:00:18 +01:00

Removing obsolete client-side transaction metrics events (#11329)

* Removing metametrics send count tracking

* Removing client side Transaction Completed and Canceled events
This commit is contained in:
ryanml 2021-06-24 15:37:44 -07:00 committed by GitHub
parent a396f55953
commit a69ed05141
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
23 changed files with 131 additions and 236 deletions

View File

@ -140,7 +140,6 @@ const state = {
} }
}, },
"participateInMetaMetrics": true, "participateInMetaMetrics": true,
"metaMetricsSendCount": 2,
"nextNonce": 71, "nextNonce": 71,
"connectedStatusPopoverHasBeenShown": true, "connectedStatusPopoverHasBeenShown": true,
"swapsWelcomeMessageHasBeenShown": true, "swapsWelcomeMessageHasBeenShown": true,

View File

@ -7,30 +7,6 @@ import {
METAMETRICS_BACKGROUND_PAGE_OBJECT, METAMETRICS_BACKGROUND_PAGE_OBJECT,
} from '../../../shared/constants/metametrics'; } 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').MetaMetricsContext} MetaMetricsContext
* @typedef {import('../../../shared/constants/metametrics').MetaMetricsEventPayload} MetaMetricsEventPayload * @typedef {import('../../../shared/constants/metametrics').MetaMetricsEventPayload} MetaMetricsEventPayload
@ -48,9 +24,6 @@ export function sendCountIsTrackable(sendCount) {
* @property {?boolean} participateInMetaMetrics - The user's preference for * @property {?boolean} participateInMetaMetrics - The user's preference for
* participating in the MetaMetrics analytics program. This setting controls * participating in the MetaMetrics analytics program. This setting controls
* whether or not events are tracked * 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 { export default class MetaMetricsController {
@ -89,7 +62,6 @@ export default class MetaMetricsController {
this.store = new ObservableStore({ this.store = new ObservableStore({
participateInMetaMetrics: null, participateInMetaMetrics: null,
metaMetricsId: null, metaMetricsId: null,
metaMetricsSendCount: 0,
...initState, ...initState,
}); });
@ -138,10 +110,6 @@ export default class MetaMetricsController {
return this.store.getState(); return this.store.getState();
} }
setMetaMetricsSendCount(val) {
this.store.updateState({ metaMetricsSendCount: val });
}
/** /**
* Build the context object to attach to page and track events. * Build the context object to attach to page and track events.
* @private * @private
@ -231,11 +199,7 @@ export default class MetaMetricsController {
// to be updated to work with the new tracking plan. I think we should use // 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 // a config setting for this instead of trying to match the event name
const isSendFlow = Boolean(payload.event.match(/^send|^confirm/iu)); const isSendFlow = Boolean(payload.event.match(/^send|^confirm/iu));
if ( if (isSendFlow) {
isSendFlow &&
this.state.metaMetricsSendCount &&
!sendCountIsTrackable(this.state.metaMetricsSendCount + 1)
) {
excludeMetaMetricsId = true; excludeMetaMetricsId = true;
} }
// If we are tracking sensitive data we will always use the anonymousId // If we are tracking sensitive data we will always use the anonymousId

View File

@ -84,7 +84,6 @@ function getMockPreferencesStore({ currentLocale = LOCALE } = {}) {
function getMetaMetricsController({ function getMetaMetricsController({
participateInMetaMetrics = true, participateInMetaMetrics = true,
metaMetricsId = TEST_META_METRICS_ID, metaMetricsId = TEST_META_METRICS_ID,
metaMetricsSendCount = 0,
preferencesStore = getMockPreferencesStore(), preferencesStore = getMockPreferencesStore(),
networkController = getMockNetworkController(), networkController = getMockNetworkController(),
} = {}) { } = {}) {
@ -106,7 +105,6 @@ function getMetaMetricsController({
initState: { initState: {
participateInMetaMetrics, participateInMetaMetrics,
metaMetricsId, 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 () { describe('trackEvent', function () {
it('should not track an event if user is not participating in metametrics', function () { it('should not track an event if user is not participating in metametrics', function () {
const mock = sinon.mock(segment); const mock = sinon.mock(segment);
@ -337,61 +327,6 @@ describe('MetaMetricsController', function () {
mock.verify(); 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 () { it('should immediately flush queue if flushImmediately set to true', async function () {
const metaMetricsController = getMetaMetricsController(); const metaMetricsController = getMetaMetricsController();
const flushStub = sinon.stub(segment, 'flush'); const flushStub = sinon.stub(segment, 'flush');

View File

@ -36,7 +36,6 @@ export const SENTRY_STATE = {
isInitialized: true, isInitialized: true,
isUnlocked: true, isUnlocked: true,
metaMetricsId: true, metaMetricsId: true,
metaMetricsSendCount: true,
nativeCurrency: true, nativeCurrency: true,
network: true, network: true,
nextNonce: true, nextNonce: true,

View File

@ -680,7 +680,6 @@ export default class MetamaskController extends EventEmitter {
setUsePhishDetect: this.setUsePhishDetect.bind(this), setUsePhishDetect: this.setUsePhishDetect.bind(this),
setIpfsGateway: this.setIpfsGateway.bind(this), setIpfsGateway: this.setIpfsGateway.bind(this),
setParticipateInMetaMetrics: this.setParticipateInMetaMetrics.bind(this), setParticipateInMetaMetrics: this.setParticipateInMetaMetrics.bind(this),
setMetaMetricsSendCount: this.setMetaMetricsSendCount.bind(this),
setFirstTimeFlowType: this.setFirstTimeFlowType.bind(this), setFirstTimeFlowType: this.setFirstTimeFlowType.bind(this),
setCurrentLocale: this.setCurrentLocale.bind(this), setCurrentLocale: this.setCurrentLocale.bind(this),
markPasswordForgotten: this.markPasswordForgotten.bind(this), markPasswordForgotten: this.markPasswordForgotten.bind(this),
@ -2768,18 +2767,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 * 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 * @param {string} type - Indicates the type of first time flow the user wishes to follow

View File

@ -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;
}

View File

@ -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',
});
});
});

View File

@ -66,6 +66,7 @@ const migrations = [
require('./059').default, require('./059').default,
require('./060').default, require('./060').default,
require('./061').default, require('./061').default,
require('./062').default,
]; ];
export default migrations; export default migrations;

View File

@ -128,7 +128,6 @@
"knownMethodData": {}, "knownMethodData": {},
"lostIdentities": {}, "lostIdentities": {},
"metaMetricsId": null, "metaMetricsId": null,
"metaMetricsSendCount": 0,
"participateInMetaMetrics": false, "participateInMetaMetrics": false,
"preferences": { "preferences": {
"useNativeCurrencyAsPrimaryCurrency": true "useNativeCurrencyAsPrimaryCurrency": true

View File

@ -138,7 +138,6 @@
}, },
"completedOnboarding": true, "completedOnboarding": true,
"metaMetricsId": null, "metaMetricsId": null,
"metaMetricsSendCount": 0,
"ipfsGateway": "dweb.link", "ipfsGateway": "dweb.link",
"selectedAddress": "0x5cfe73b6021e818b776b421b1c4db2474086a7e1" "selectedAddress": "0x5cfe73b6021e818b776b421b1c4db2474086a7e1"
}, },

View File

@ -129,7 +129,6 @@
"knownMethodData": {}, "knownMethodData": {},
"lostIdentities": {}, "lostIdentities": {},
"metaMetricsId": null, "metaMetricsId": null,
"metaMetricsSendCount": 0,
"participateInMetaMetrics": false, "participateInMetaMetrics": false,
"preferences": { "preferences": {
"useNativeCurrencyAsPrimaryCurrency": true "useNativeCurrencyAsPrimaryCurrency": true

View File

@ -121,7 +121,6 @@
"knownMethodData": {}, "knownMethodData": {},
"lostIdentities": {}, "lostIdentities": {},
"metaMetricsId": null, "metaMetricsId": null,
"metaMetricsSendCount": 0,
"participateInMetaMetrics": false, "participateInMetaMetrics": false,
"preferences": { "preferences": {
"useNativeCurrencyAsPrimaryCurrency": true "useNativeCurrencyAsPrimaryCurrency": true

View File

@ -125,8 +125,7 @@
}, },
"MetaMetricsController": { "MetaMetricsController": {
"participateInMetaMetrics": false, "participateInMetaMetrics": false,
"metaMetricsId": null, "metaMetricsId": null
"metaMetricsSendCount": 1
}, },
"PermissionsController": { "PermissionsController": {
"permissionsRequests": [], "permissionsRequests": [],

View File

@ -114,7 +114,6 @@
"knownMethodData": {}, "knownMethodData": {},
"lostIdentities": {}, "lostIdentities": {},
"metaMetricsId": null, "metaMetricsId": null,
"metaMetricsSendCount": 0,
"participateInMetaMetrics": false, "participateInMetaMetrics": false,
"preferences": { "preferences": {
"useNativeCurrencyAsPrimaryCurrency": true "useNativeCurrencyAsPrimaryCurrency": true

View File

@ -114,7 +114,6 @@
"knownMethodData": {}, "knownMethodData": {},
"lostIdentities": {}, "lostIdentities": {},
"metaMetricsId": null, "metaMetricsId": null,
"metaMetricsSendCount": 0,
"participateInMetaMetrics": false, "participateInMetaMetrics": false,
"preferences": { "preferences": {
"showFiatInTestnets": true, "showFiatInTestnets": true,

View File

@ -138,7 +138,6 @@
}, },
"completedOnboarding": true, "completedOnboarding": true,
"metaMetricsId": "fake-metrics-id", "metaMetricsId": "fake-metrics-id",
"metaMetricsSendCount": 0,
"ipfsGateway": "dweb.link", "ipfsGateway": "dweb.link",
"selectedAddress": "0x5cfe73b6021e818b776b421b1c4db2474086a7e1" "selectedAddress": "0x5cfe73b6021e818b776b421b1c4db2474086a7e1"
}, },

View File

@ -115,7 +115,6 @@
"knownMethodData": {}, "knownMethodData": {},
"lostIdentities": {}, "lostIdentities": {},
"metaMetricsId": null, "metaMetricsId": null,
"metaMetricsSendCount": 0,
"participateInMetaMetrics": false, "participateInMetaMetrics": false,
"preferences": { "preferences": {
"useNativeCurrencyAsPrimaryCurrency": true "useNativeCurrencyAsPrimaryCurrency": true

View File

@ -120,8 +120,7 @@
}, },
"MetaMetricsController": { "MetaMetricsController": {
"metaMetricsId": null, "metaMetricsId": null,
"participateInMetaMetrics": false, "participateInMetaMetrics": false
"metaMetricsSendCount": 0
}, },
"ThreeBoxController": { "ThreeBoxController": {
"threeBoxSyncingAllowed": true, "threeBoxSyncingAllowed": true,

View File

@ -33,7 +33,6 @@ export default function reduceMetamask(state = {}, action) {
completedOnboarding: false, completedOnboarding: false,
knownMethodData: {}, knownMethodData: {},
participateInMetaMetrics: null, participateInMetaMetrics: null,
metaMetricsSendCount: 0,
nextNonce: null, nextNonce: null,
conversionRate: null, conversionRate: null,
nativeCurrency: 'ETH', nativeCurrency: 'ETH',
@ -126,12 +125,6 @@ export default function reduceMetamask(state = {}, action) {
participateInMetaMetrics: action.value, participateInMetaMetrics: action.value,
}; };
case actionConstants.SET_METAMETRICS_SEND_COUNT:
return {
...metamaskState,
metaMetricsSendCount: action.value,
};
case actionConstants.SET_USE_BLOCKIE: case actionConstants.SET_USE_BLOCKIE:
return { return {
...metamaskState, ...metamaskState,

View File

@ -86,8 +86,6 @@ export default class ConfirmTransactionBase extends Component {
hideSubtitle: PropTypes.bool, hideSubtitle: PropTypes.bool,
identiconAddress: PropTypes.string, identiconAddress: PropTypes.string,
onEdit: PropTypes.func, onEdit: PropTypes.func,
setMetaMetricsSendCount: PropTypes.func,
metaMetricsSendCount: PropTypes.number,
subtitleComponent: PropTypes.node, subtitleComponent: PropTypes.node,
title: PropTypes.string, title: PropTypes.string,
advancedInlineGasShown: PropTypes.bool, advancedInlineGasShown: PropTypes.bool,
@ -474,35 +472,16 @@ export default class ConfirmTransactionBase extends Component {
} }
handleCancel() { handleCancel() {
const { metricsEvent } = this.context;
const { const {
txData, txData,
cancelTransaction, cancelTransaction,
history, history,
mostRecentOverviewPage, mostRecentOverviewPage,
clearConfirmTransaction, clearConfirmTransaction,
actionKey,
txData: { origin },
methodData = {},
updateCustomNonce, updateCustomNonce,
} = this.props; } = this.props;
this._removeBeforeUnload(); 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(''); updateCustomNonce('');
cancelTransaction(txData).then(() => { cancelTransaction(txData).then(() => {
clearConfirmTransaction(); clearConfirmTransaction();
@ -511,18 +490,12 @@ export default class ConfirmTransactionBase extends Component {
} }
handleSubmit() { handleSubmit() {
const { metricsEvent } = this.context;
const { const {
txData: { origin },
sendTransaction, sendTransaction,
clearConfirmTransaction, clearConfirmTransaction,
txData, txData,
history, history,
actionKey,
mostRecentOverviewPage, mostRecentOverviewPage,
metaMetricsSendCount = 0,
setMetaMetricsSendCount,
methodData = {},
updateCustomNonce, updateCustomNonce,
} = this.props; } = this.props;
const { submitting } = this.state; const { submitting } = this.state;
@ -538,23 +511,7 @@ export default class ConfirmTransactionBase extends Component {
}, },
() => { () => {
this._removeBeforeUnload(); 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) sendTransaction(txData)
.then(() => { .then(() => {
clearConfirmTransaction(); clearConfirmTransaction();
@ -575,7 +532,6 @@ export default class ConfirmTransactionBase extends Component {
}); });
updateCustomNonce(''); updateCustomNonce('');
}); });
});
}, },
); );
} }
@ -642,18 +598,7 @@ export default class ConfirmTransactionBase extends Component {
} }
_beforeUnload = () => { _beforeUnload = () => {
const { txData: { origin, id } = {}, cancelTransaction } = this.props; const { txData: { id } = {}, cancelTransaction } = this.props;
const { metricsEvent } = this.context;
metricsEvent({
eventOpts: {
category: 'Transactions',
action: 'Confirm Screen',
name: 'Cancel Tx Via Notification Close',
},
customVariables: {
origin,
},
});
cancelTransaction({ id }); cancelTransaction({ id });
}; };

View File

@ -10,7 +10,6 @@ import {
cancelTxs, cancelTxs,
updateAndApproveTx, updateAndApproveTx,
showModal, showModal,
setMetaMetricsSendCount,
updateTransaction, updateTransaction,
getNextNonce, getNextNonce,
tryReverseResolveAddress, tryReverseResolveAddress,
@ -75,7 +74,6 @@ const mapStateToProps = (state, ownProps) => {
assetImages, assetImages,
network, network,
unapprovedTxs, unapprovedTxs,
metaMetricsSendCount,
nextNonce, nextNonce,
provider: { chainId }, provider: { chainId },
} = metamask; } = metamask;
@ -185,7 +183,6 @@ const mapStateToProps = (state, ownProps) => {
insufficientBalance, insufficientBalance,
hideSubtitle: !getShouldShowFiat(state), hideSubtitle: !getShouldShowFiat(state),
hideFiatConversion: !getShouldShowFiat(state), hideFiatConversion: !getShouldShowFiat(state),
metaMetricsSendCount,
type, type,
nextNonce, nextNonce,
mostRecentOverviewPage: getMostRecentOverviewPage(state), mostRecentOverviewPage: getMostRecentOverviewPage(state),
@ -233,7 +230,6 @@ export const mapDispatchToProps = (dispatch) => {
cancelAllTransactions: (txList) => dispatch(cancelTxs(txList)), cancelAllTransactions: (txList) => dispatch(cancelTxs(txList)),
sendTransaction: (txData) => sendTransaction: (txData) =>
dispatch(updateAndApproveTx(customNonceMerge(txData))), dispatch(updateAndApproveTx(customNonceMerge(txData))),
setMetaMetricsSendCount: (val) => dispatch(setMetaMetricsSendCount(val)),
getNextNonce: () => dispatch(getNextNonce()), getNextNonce: () => dispatch(getNextNonce()),
setDefaultHomeActiveTabName: (tabName) => setDefaultHomeActiveTabName: (tabName) =>
dispatch(setDefaultHomeActiveTabName(tabName)), dispatch(setDefaultHomeActiveTabName(tabName)),

View File

@ -64,7 +64,6 @@ export const UPDATE_CUSTOM_NONCE = 'UPDATE_CUSTOM_NONCE';
export const SET_IPFS_GATEWAY = 'SET_IPFS_GATEWAY'; export const SET_IPFS_GATEWAY = 'SET_IPFS_GATEWAY';
export const SET_PARTICIPATE_IN_METAMETRICS = 'SET_PARTICIPATE_IN_METAMETRICS'; export const SET_PARTICIPATE_IN_METAMETRICS = 'SET_PARTICIPATE_IN_METAMETRICS';
export const SET_METAMETRICS_SEND_COUNT = 'SET_METAMETRICS_SEND_COUNT';
// locale // locale
export const SET_CURRENT_LOCALE = 'SET_CURRENT_LOCALE'; export const SET_CURRENT_LOCALE = 'SET_CURRENT_LOCALE';

View File

@ -1987,27 +1987,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) { export function setUseBlockie(val) {
return (dispatch) => { return (dispatch) => {
dispatch(showLoadingIndication()); dispatch(showLoadingIndication());