m/metamask-extension
1
0
Fork 0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-12-22 17:33:23 +01:00
Code

Make metrics related actions idempotent (#15737)

Browse Source
This commit is contained in:
Jyoti Puri 2022-09-16 22:34:14 +05:30 committed by GitHub
parent f465089c2a
commit c2b7690119
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 170 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
app/scripts/controllers/metametrics.js
View File

@ -188,6 +188,15 @@ export default class MetaMetricsController {
}`,
);
}
const existingFragment = this.getExistingEventFragment(
options.actionId,
options.uniqueIdentifier,
);
if (existingFragment) {
return existingFragment;
}
const { fragments } = this.store.getState();
const id = options.uniqueIdentifier ?? uuidv4();
@ -236,6 +245,26 @@ export default class MetaMetricsController {
return fragment;
}
/**
* Returns the fragment stored in memory with provided id or undefined if it
* does not exist.
*
* @param {string} actionId - actionId passed from UI
* @param {string} uniqueIdentifier - uniqueIdentifier of the event
* @returns {[MetaMetricsEventFragment]}
*/
getExistingEventFragment(actionId, uniqueIdentifier) {
const { fragments } = this.store.getState();
const existingFragment = Object.values(fragments).find(
(fragment) =>
fragment.actionId === actionId &&
fragment.uniqueIdentifier === uniqueIdentifier,
);
return existingFragment;
}
/**
* Updates an event fragment in state
*

91
app/scripts/controllers/transactions/index.js
View File

@ -301,7 +301,11 @@ export default class TransactionController extends EventEmitter {
addTransaction(txMeta) {
this.txStateManager.addTransaction(txMeta);
this.emit(`${txMeta.id}:unapproved`, txMeta);
this._trackTransactionMetricsEvent(txMeta, TRANSACTION_EVENTS.ADDED);
this._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.ADDED,
txMeta.actionId,
);
}
/**
@ -1216,7 +1220,7 @@ export default class TransactionController extends EventEmitter {
}
this.addTransaction(newTxMeta);
await this.approveTransaction(newTxMeta.id);
await this.approveTransaction(newTxMeta.id, actionId);
return newTxMeta;
}
@ -1274,7 +1278,7 @@ export default class TransactionController extends EventEmitter {
}
this.addTransaction(newTxMeta);
await this.approveTransaction(newTxMeta.id);
await this.approveTransaction(newTxMeta.id, actionId);
return newTxMeta;
}
@ -1294,13 +1298,14 @@ export default class TransactionController extends EventEmitter {
* updates and approves the transaction
*
* @param {object} txMeta
* @param {string} actionId
*/
async updateAndApproveTransaction(txMeta) {
async updateAndApproveTransaction(txMeta, actionId) {
this.txStateManager.updateTransaction(
txMeta,
'confTx: user approved transaction',
);
await this.approveTransaction(txMeta.id);
await this.approveTransaction(txMeta.id, actionId);
}
/**
@ -1311,8 +1316,9 @@ export default class TransactionController extends EventEmitter {
* if any of these steps fails the tx status will be set to failed
*
* @param {number} txId - the tx's Id
* @param {string} actionId - actionId passed from UI
*/
async approveTransaction(txId) {
async approveTransaction(txId, actionId) {
// TODO: Move this safety out of this function.
// Since this transaction is async,
// we need to keep track of what is currently being signed,
@ -1354,14 +1360,18 @@ 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);
await this.publishTransaction(txId, rawTx, actionId);
this._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.APPROVED,
actionId,
);
// 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._failTransaction(txId, err);
this._failTransaction(txId, err, actionId);
} catch (err2) {
log.error(err2);
}
@ -1490,8 +1500,9 @@ export default class TransactionController extends EventEmitter {
* @param {number} txId - the tx's Id
* @param {string} rawTx - the hex string of the serialized signed transaction
* @returns {Promise<void>}
* @param {number} actionId - actionId passed from UI
*/
async publishTransaction(txId, rawTx) {
async publishTransaction(txId, rawTx, actionId) {
const txMeta = this.txStateManager.getTransaction(txId);
txMeta.rawTx = rawTx;
if (txMeta.type === TRANSACTION_TYPES.SWAP) {
@ -1517,7 +1528,11 @@ export default class TransactionController extends EventEmitter {
this.txStateManager.setTxStatusSubmitted(txId);
this._trackTransactionMetricsEvent(txMeta, TRANSACTION_EVENTS.SUBMITTED);
this._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.SUBMITTED,
actionId,
);
}
async updatePostTxBalance({ txMeta, txId, numberOfAttempts = 6 }) {
@ -1606,6 +1621,7 @@ export default class TransactionController extends EventEmitter {
this._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.FINALIZED,
undefined,
metricsParams,
);
@ -1666,6 +1682,7 @@ export default class TransactionController extends EventEmitter {
this._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.FINALIZED,
undefined,
metricsParams,
);
@ -1689,12 +1706,17 @@ export default class TransactionController extends EventEmitter {
* Convenience method for the ui thats sets the transaction to rejected
*
* @param {number} txId - the tx's Id
* @param {string} actionId - actionId passed from UI
* @returns {Promise<void>}
*/
async cancelTransaction(txId) {
async cancelTransaction(txId, actionId) {
const txMeta = this.txStateManager.getTransaction(txId);
this.txStateManager.setTxStatusRejected(txId);
this._trackTransactionMetricsEvent(txMeta, TRANSACTION_EVENTS.REJECTED);
this._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.REJECTED,
actionId,
);
}
/**
@ -1717,8 +1739,9 @@ export default class TransactionController extends EventEmitter {
* @param {number} transactionId - The transaction id to create the event
* fragment for
* @param {valueOf<TRANSACTION_EVENTS>} event - event type to create
* @param {string} actionId - actionId passed from UI
*/
async createTransactionEventFragment(transactionId, event) {
async createTransactionEventFragment(transactionId, event, actionId) {
const txMeta = this.txStateManager.getTransaction(transactionId);
const { properties, sensitiveProperties } =
await this._buildEventFragmentProperties(txMeta);
@ -1727,6 +1750,7 @@ export default class TransactionController extends EventEmitter {
event,
properties,
sensitiveProperties,
actionId,
);
}
@ -2329,6 +2353,7 @@ export default class TransactionController extends EventEmitter {
* triggered fragment creation
* @param {object} properties - properties to include in the fragment
* @param {object} [sensitiveProperties] - sensitive properties to include in
* @param {object} [actionId] - actionId passed from UI
* the fragment
*/
_createTransactionEventFragment(
@ -2336,6 +2361,7 @@ export default class TransactionController extends EventEmitter {
event,
properties,
sensitiveProperties,
actionId,
) {
const isSubmitted = [
TRANSACTION_EVENTS.FINALIZED,
@ -2370,6 +2396,7 @@ export default class TransactionController extends EventEmitter {
sensitiveProperties,
persist: true,
uniqueIdentifier,
actionId,
});
break;
// If for some reason an approval or rejection occurs without the added
@ -2390,6 +2417,7 @@ export default class TransactionController extends EventEmitter {
sensitiveProperties,
persist: true,
uniqueIdentifier,
actionId,
});
break;
// When a transaction is submitted it will always result in updating
@ -2411,6 +2439,7 @@ export default class TransactionController extends EventEmitter {
sensitiveProperties,
persist: true,
uniqueIdentifier,
actionId,
});
break;
// If for some reason a transaction is finalized without the submitted
@ -2429,6 +2458,7 @@ export default class TransactionController extends EventEmitter {
sensitiveProperties,
persist: true,
uniqueIdentifier,
actionId,
});
break;
default:
@ -2443,9 +2473,15 @@ export default class TransactionController extends EventEmitter {
*
* @param {object} txMeta - the txMeta object
* @param {TransactionMetaMetricsEventString} event - the name of the transaction event
* @param {string} actionId - actionId passed from UI
* @param {object} extraParams - optional props and values to include in sensitiveProperties
*/
async _trackTransactionMetricsEvent(txMeta, event, extraParams = {}) {
async _trackTransactionMetricsEvent(
txMeta,
event,
actionId,
extraParams = {},
) {
if (!txMeta) {
return;
}
@ -2459,6 +2495,7 @@ export default class TransactionController extends EventEmitter {
event,
properties,
sensitiveProperties,
actionId,
);
let id;
@ -2508,19 +2545,29 @@ export default class TransactionController extends EventEmitter {
return gasValuesInGwei;
}
_failTransaction(txId, error) {
_failTransaction(txId, error, actionId) {
this.txStateManager.setTxStatusFailed(txId, error);
const txMeta = this.txStateManager.getTransaction(txId);
this._trackTransactionMetricsEvent(txMeta, TRANSACTION_EVENTS.FINALIZED, {
error: error.message,
});
this._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.FINALIZED,
actionId,
{
error: error.message,
},
);
}
_dropTransaction(txId) {
this.txStateManager.setTxStatusDropped(txId);
const txMeta = this.txStateManager.getTransaction(txId);
this._trackTransactionMetricsEvent(txMeta, TRANSACTION_EVENTS.FINALIZED, {
dropped: true,
});
this._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.FINALIZED,
undefined,
{
dropped: true,
},
);
}
}

26
app/scripts/controllers/transactions/index.test.js
View File

@ -36,7 +36,7 @@ const currentChainId = '0x2a';
const providerConfig = {
type: 'kovan',
};
const actionId = 'DUMMY_ACTION_ID';
const VALID_ADDRESS = '0x0000000000000000000000000000000000000000';
const VALID_ADDRESS_TWO = '0x0000000000000000000000000000000000000001';
@ -551,8 +551,7 @@ describe('Transaction Controller', function () {
await txController.createCancelTransaction(
txMeta.id,
{},
undefined,
12345,
{ actionId: 12345 },
);
const transactionCount1 =
txController.txStateManager.getTransactions().length;
@ -1666,6 +1665,7 @@ describe('Transaction Controller', function () {
describe('On transaction created by the user', function () {
let txMeta;
before(function () {
txMeta = {
id: 1,
@ -1691,6 +1691,7 @@ describe('Transaction Controller', function () {
it('should create an event fragment when transaction added', async function () {
const expectedPayload = {
actionId,
initialEvent: 'Transaction Added',
successEvent: 'Transaction Approved',
failureEvent: 'Transaction Rejected',
@ -1728,6 +1729,7 @@ describe('Transaction Controller', function () {
await txController._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.ADDED,
actionId,
);
assert.equal(createEventFragmentSpy.callCount, 1);
assert.equal(finalizeEventFragmentSpy.callCount, 0);
@ -1742,6 +1744,7 @@ describe('Transaction Controller', function () {
await txController._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.REJECTED,
actionId,
);
assert.equal(createEventFragmentSpy.callCount, 0);
assert.equal(finalizeEventFragmentSpy.callCount, 1);
@ -1759,6 +1762,7 @@ describe('Transaction Controller', function () {
await txController._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.APPROVED,
actionId,
);
assert.equal(createEventFragmentSpy.callCount, 0);
assert.equal(finalizeEventFragmentSpy.callCount, 1);
@ -1774,6 +1778,7 @@ describe('Transaction Controller', function () {
it('should create an event fragment when transaction is submitted', async function () {
const expectedPayload = {
actionId,
initialEvent: 'Transaction Submitted',
successEvent: 'Transaction Finalized',
uniqueIdentifier: 'transaction-submitted-1',
@ -1810,6 +1815,7 @@ describe('Transaction Controller', function () {
await txController._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.SUBMITTED,
actionId,
);
assert.equal(createEventFragmentSpy.callCount, 1);
assert.equal(finalizeEventFragmentSpy.callCount, 0);
@ -1824,6 +1830,7 @@ describe('Transaction Controller', function () {
await txController._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.FINALIZED,
actionId,
);
assert.equal(createEventFragmentSpy.callCount, 0);
assert.equal(finalizeEventFragmentSpy.callCount, 1);
@ -1865,6 +1872,7 @@ describe('Transaction Controller', function () {
it('should create an event fragment when transaction added', async function () {
const expectedPayload = {
actionId,
initialEvent: 'Transaction Added',
successEvent: 'Transaction Approved',
failureEvent: 'Transaction Rejected',
@ -1902,6 +1910,7 @@ describe('Transaction Controller', function () {
await txController._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.ADDED,
actionId,
);
assert.equal(createEventFragmentSpy.callCount, 1);
assert.equal(finalizeEventFragmentSpy.callCount, 0);
@ -1917,6 +1926,7 @@ describe('Transaction Controller', function () {
await txController._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.REJECTED,
actionId,
);
assert.equal(createEventFragmentSpy.callCount, 0);
assert.equal(finalizeEventFragmentSpy.callCount, 1);
@ -1935,6 +1945,7 @@ describe('Transaction Controller', function () {
await txController._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.APPROVED,
actionId,
);
assert.equal(createEventFragmentSpy.callCount, 0);
assert.equal(finalizeEventFragmentSpy.callCount, 1);
@ -1950,6 +1961,7 @@ describe('Transaction Controller', function () {
it('should create an event fragment when transaction is submitted', async function () {
const expectedPayload = {
actionId,
initialEvent: 'Transaction Submitted',
successEvent: 'Transaction Finalized',
uniqueIdentifier: 'transaction-submitted-1',
@ -1986,6 +1998,7 @@ describe('Transaction Controller', function () {
await txController._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.SUBMITTED,
actionId,
);
assert.equal(createEventFragmentSpy.callCount, 1);
assert.equal(finalizeEventFragmentSpy.callCount, 0);
@ -2001,6 +2014,7 @@ describe('Transaction Controller', function () {
await txController._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.FINALIZED,
actionId,
);
assert.equal(createEventFragmentSpy.callCount, 0);
assert.equal(finalizeEventFragmentSpy.callCount, 1);
@ -2034,6 +2048,7 @@ describe('Transaction Controller', function () {
};
const expectedPayload = {
actionId,
successEvent: 'Transaction Approved',
failureEvent: 'Transaction Rejected',
uniqueIdentifier: 'transaction-added-1',
@ -2067,6 +2082,7 @@ describe('Transaction Controller', function () {
await txController._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.APPROVED,
actionId,
);
assert.equal(createEventFragmentSpy.callCount, 1);
assert.deepEqual(
@ -2099,6 +2115,7 @@ describe('Transaction Controller', function () {
metamaskNetworkId: currentNetworkId,
};
const expectedPayload = {
actionId,
initialEvent: 'Transaction Added',
successEvent: 'Transaction Approved',
failureEvent: 'Transaction Rejected',
@ -2136,6 +2153,7 @@ describe('Transaction Controller', function () {
await txController._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.ADDED,
actionId,
{
baz: 3.0,
foo: 'bar',
@ -2175,6 +2193,7 @@ describe('Transaction Controller', function () {
},
};
const expectedPayload = {
actionId,
initialEvent: 'Transaction Added',
successEvent: 'Transaction Approved',
failureEvent: 'Transaction Rejected',
@ -2218,6 +2237,7 @@ describe('Transaction Controller', function () {
await txController._trackTransactionMetricsEvent(
txMeta,
TRANSACTION_EVENTS.ADDED,
actionId,
{
baz: 3.0,
foo: 'bar',

6
ui/store/action-queue/index.js
View File

@ -29,6 +29,8 @@ let promisifiedBackground = null;
const actionRetryQueue = [];
export const generateActionId = () => Date.now() + Math.random();
function failQueue() {
actionRetryQueue.forEach(({ reject }) =>
reject(
@ -81,7 +83,7 @@ const executeActionOrAddToRetryQueue = (item) => {
export function submitRequestToBackground(
method,
args = [],
actionId = Date.now() + Math.random(), // current date is not guaranteed to be unique
actionId = generateActionId(), // current date is not guaranteed to be unique
) {
if (isManifestV3) {
return new Promise((resolve, reject) => {
@ -112,7 +114,7 @@ export const callBackgroundMethod = (
method,
args = [],
callback,
actionId = Date.now() + Math.random(), // current date is not guaranteed to be unique
actionId = generateActionId(), // current date is not guaranteed to be unique
) => {
if (isManifestV3) {
const resolve = (value) => callback(null, value);

70
ui/store/actions.js
View File

@ -48,6 +48,7 @@ import { NOTIFICATIONS_EXPIRATION_DELAY } from '../helpers/constants/notificatio
import { setNewCustomNetworkAdded } from '../ducks/app/app';
import * as actionConstants from './actionConstants';
import {
generateActionId,
callBackgroundMethod,
submitRequestToBackground,
} from './action-queue';
@ -894,7 +895,7 @@ export function addUnapprovedTransactionAndRouteToConfirmationPage(
sendFlowHistory,
) {
return async (dispatch) => {
const actionId = Date.now() + Math.random();
const actionId = generateActionId();
try {
log.debug('background.addUnapprovedTransaction');
const txMeta = await submitRequestToBackground(
@ -927,7 +928,7 @@ export function addUnapprovedTransactionAndRouteToConfirmationPage(
*/
export async function addUnapprovedTransaction(txParams, type) {
log.debug('background.addUnapprovedTransaction');
const actionId = Date.now() + Math.random();
const actionId = generateActionId();
const txMeta = await submitRequestToBackground(
'addUnapprovedTransaction',
[txParams, ORIGIN_METAMASK, type, undefined, actionId],
@ -940,20 +941,25 @@ export function updateAndApproveTx(txData, dontShowLoadingIndicator) {
return (dispatch) => {
!dontShowLoadingIndicator && dispatch(showLoadingIndication());
return new Promise((resolve, reject) => {
callBackgroundMethod('updateAndApproveTransaction', [txData], (err) => {
dispatch(updateTransactionParams(txData.id, txData.txParams));
dispatch(resetSendState());
const actionId = generateActionId();
callBackgroundMethod(
'updateAndApproveTransaction',
[txData, actionId],
(err) => {
dispatch(updateTransactionParams(txData.id, txData.txParams));
dispatch(resetSendState());
if (err) {
dispatch(txError(err));
dispatch(goHome());
log.error(err.message);
reject(err);
return;
}
if (err) {
dispatch(txError(err));
dispatch(goHome());
log.error(err.message);
reject(err);
return;
}
resolve(txData);
});
resolve(txData);
},
);
})
.then(() => updateMetamaskStateFromBackground())
.then((newState) => dispatch(updateMetamaskState(newState)))
@ -1287,14 +1293,19 @@ export function cancelTx(txData, _showLoadingIndication = true) {
return (dispatch) => {
_showLoadingIndication && dispatch(showLoadingIndication());
return new Promise((resolve, reject) => {
callBackgroundMethod('cancelTransaction', [txData.id], (error) => {
if (error) {
reject(error);
return;
}
const actionId = generateActionId();
callBackgroundMethod(
'cancelTransaction',
[txData.id, actionId],
(error) => {
if (error) {
reject(error);
return;
}
resolve();
});
resolve();
},
);
})
.then(() => updateMetamaskStateFromBackground())
.then((newState) => dispatch(updateMetamaskState(newState)))
@ -1328,7 +1339,8 @@ export function cancelTxs(txDataList) {
const cancellations = txIds.map(
(id) =>
new Promise((resolve, reject) => {
callBackgroundMethod('cancelTransaction', [id], (err) => {
const actionId = generateActionId();
callBackgroundMethod('cancelTransaction', [id, actionId], (err) => {
if (err) {
reject(err);
return;
@ -1979,7 +1991,7 @@ export function createCancelTransaction(txId, customGasSettings, options) {
let newTxId;
return (dispatch) => {
const actionId = Date.now() + Math.random();
const actionId = generateActionId();
return new Promise((resolve, reject) => {
callBackgroundMethod(
'createCancelTransaction',
@ -2009,7 +2021,7 @@ export function createSpeedUpTransaction(txId, customGasSettings, options) {
let newTx;
return (dispatch) => {
const actionId = Date.now() + Math.random();
const actionId = generateActionId();
return new Promise((resolve, reject) => {
callBackgroundMethod(
'createSpeedUpTransaction',
@ -2038,9 +2050,10 @@ export function createRetryTransaction(txId, customGasSettings) {
return (dispatch) => {
return new Promise((resolve, reject) => {
const actionId = generateActionId();
callBackgroundMethod(
'createSpeedUpTransaction',
[txId, customGasSettings],
[txId, customGasSettings, { actionId }],
(err, newState) => {
if (err) {
dispatch(displayWarning(err.message));
@ -3605,13 +3618,18 @@ export function trackMetaMetricsEvent(payload, options) {
}
export function createEventFragment(options) {
return submitRequestToBackground('createEventFragment', [options]);
const actionId = generateActionId();
return submitRequestToBackground('createEventFragment', [
{ ...options, actionId },
]);
}
export function createTransactionEventFragment(transactionId, event) {
const actionId = generateActionId();
return submitRequestToBackground('createTransactionEventFragment', [
transactionId,
event,
actionId,
]);
}

2
ui/store/actions.test.js
View File

@ -1664,7 +1664,7 @@ describe('Actions', () => {
const store = mockStore();
background.getApi.returns({
cancelTransaction: sinon.stub().callsFake((_, cb) => {
cancelTransaction: sinon.stub().callsFake((_1, _2, cb) => {
cb();
}),
getState: sinon.stub().callsFake((cb) =>