Skip to content

Commit

Permalink
fix: Send ppom metrics when transaction is cancelled. (#9215)
Browse files Browse the repository at this point in the history
## **Description**

We should send PPOM/Blockaid metrics parameters when a transaction from
wallet is rejected. Right now we only send the metrics when the
transaction is approved.

## **Related issues**

Fixes: [#8824](#8824)

## **Manual testing steps**

1. Enable Blockiad
2. Enable metrics
3. Initiate a malicious tx from inside the wallet
4. Cancel/Reject the transaction
5. See that PPOM Metrics is sent in the logs

## **Screenshots/Recordings**

### **Before**


https://github.com/MetaMask/metamask-mobile/assets/54408225/12153f52-baeb-4344-b6f6-0c00c2a1d1fc

### **After**


https://github.com/MetaMask/metamask-mobile/assets/44811/efea6aa9-96f7-4b57-915c-5fe946daabdf

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] 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-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.

---------

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
  • Loading branch information
segun authored Apr 16, 2024
1 parent 66821fd commit 594cf3f
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 41 deletions.
5 changes: 5 additions & 0 deletions app/components/UI/Navbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import { CommonSelectorsIDs } from '../../../../e2e/selectors/Common.selectors';
import { WalletViewSelectorsIDs } from '../../../../e2e/selectors/WalletView.selectors';
import { NetworksViewSelectorsIDs } from '../../../../e2e/selectors/Settings/NetworksView.selectors';
import { SendLinkViewSelectorsIDs } from '../../../../e2e/selectors/SendLinkView.selectors';
import { getBlockaidTransactionMetricsParams } from '../../../util/blockaid';

const trackEvent = (event, params = {}) => {
MetaMetrics.getInstance().trackEvent(event, params);
Expand Down Expand Up @@ -513,6 +514,7 @@ export function getSendFlowTitle(
route,
themeColors,
resetTransaction,
transaction,
) {
const innerStyles = StyleSheet.create({
headerButtonText: {
Expand All @@ -528,9 +530,12 @@ export function getSendFlowTitle(
});
const rightAction = () => {
const providerType = route?.params?.providerType ?? '';
const additionalTransactionMetricsParams =
getBlockaidTransactionMetricsParams(transaction);
trackEvent(MetaMetricsEvents.SEND_FLOW_CANCEL, {
view: title.split('.')[1],
network: providerType,
...additionalTransactionMetricsParams,
});
resetTransaction();
navigation.dangerouslyGetParent()?.pop();
Expand Down
26 changes: 6 additions & 20 deletions app/components/Views/confirmations/SendFlow/Confirm/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ import { ConfirmViewSelectorsIDs } from '../../../../../../e2e/selectors/SendFlo
import ExtendedKeyringTypes from '../../../../../constants/keyringTypes';
import { getLedgerKeyring } from '../../../../../core/Ledger/Ledger';
import {
getBlockaidMetricsParams,
getBlockaidTransactionMetricsParams,
isBlockaidFeatureEnabled,
} from '../../../../../util/blockaid';
import ppomUtil from '../../../../../lib/ppom/ppom-util';
Expand Down Expand Up @@ -302,23 +302,8 @@ class Confirm extends PureComponent {
}
};

withBlockaidMetricsParams = () => {
let blockaidParams = {};

const { transaction } = this.props;
if (
transaction.id === transaction.currentTransactionSecurityAlertResponse?.id
) {
blockaidParams = getBlockaidMetricsParams(
transaction.currentTransactionSecurityAlertResponse?.response,
);
}

return blockaidParams;
};

updateNavBar = () => {
const { navigation, route, resetTransaction } = this.props;
const { navigation, route, resetTransaction, transaction } = this.props;
const colors = this.context.colors || mockTheme.colors;
navigation.setOptions(
getSendFlowTitle(
Expand All @@ -327,6 +312,7 @@ class Confirm extends PureComponent {
route,
colors,
resetTransaction,
transaction,
),
);
};
Expand Down Expand Up @@ -858,7 +844,7 @@ class Confirm extends PureComponent {
assetType,
{
...this.getAnalyticsParams(),
...this.withBlockaidMetricsParams(),
...getBlockaidTransactionMetricsParams(transaction),
},
),
type: 'signTransaction',
Expand Down Expand Up @@ -887,7 +873,7 @@ class Confirm extends PureComponent {
MetaMetricsEvents.SEND_TRANSACTION_COMPLETED,
{
...this.getAnalyticsParams(),
...this.withBlockaidMetricsParams(),
...getBlockaidTransactionMetricsParams(transaction),
},
);
stopGasPolling();
Expand Down Expand Up @@ -1101,7 +1087,7 @@ class Confirm extends PureComponent {
const { transaction } = this.props;
const analyticsParams = {
...this.getAnalyticsParams(),
...this.withBlockaidMetricsParams(transaction),
...getBlockaidTransactionMetricsParams(transaction),
external_link_clicked: 'security_alert_support_link',
};
this.props.metrics.trackEvent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import TransactionReviewDetailsCard from '../TransactionReview/TransactionReview
import AppConstants from '../../../../../core/AppConstants';
import { UINT256_HEX_MAX_VALUE } from '../../../../../constants/transaction';
import { WALLET_CONNECT_ORIGIN } from '../../../../../util/walletconnect';
import { getBlockaidMetricsParams } from '../../../../../util/blockaid';
import { getBlockaidTransactionMetricsParams } from '../../../../../util/blockaid';
import { withNavigation } from '@react-navigation/compat';
import {
isTestNet,
Expand Down Expand Up @@ -510,21 +510,6 @@ class ApproveTransactionReview extends PureComponent {
clearInterval(intervalIdForEstimatedL1Fee);
};

withBlockaidMetricsParams = () => {
let blockaidParams = {};

const { transaction } = this.props;
if (
transaction.id === transaction.currentTransactionSecurityAlertResponse?.id
) {
blockaidParams = getBlockaidMetricsParams(
transaction.currentTransactionSecurityAlertResponse?.response,
);
}

return blockaidParams;
};

getAnalyticsParams = () => {
try {
const { chainId, transaction, onSetAnalyticsParams } = this.props;
Expand Down Expand Up @@ -697,9 +682,10 @@ class ApproveTransactionReview extends PureComponent {
};

onContactUsClicked = () => {
const { transaction } = this.props;
const analyticsParams = {
...this.getAnalyticsParams(),
...this.withBlockaidMetricsParams(),
...getBlockaidTransactionMetricsParams(transaction),
external_link_clicked: 'security_alert_support_link',
};
this.props.metrics.trackEvent(
Expand Down Expand Up @@ -1177,13 +1163,13 @@ class ApproveTransactionReview extends PureComponent {
};

onCancelPress = () => {
const { onCancel } = this.props;
const { onCancel, transaction } = this.props;
onCancel && onCancel();
this.props.metrics.trackEvent(
MetaMetricsEvents.APPROVAL_PERMISSION_UPDATED,
{
...this.getAnalyticsParams(),
...this.withBlockaidMetricsParams(),
...getBlockaidTransactionMetricsParams(transaction),
},
);
};
Expand All @@ -1200,7 +1186,7 @@ class ApproveTransactionReview extends PureComponent {
MetaMetricsEvents.APPROVAL_PERMISSION_UPDATED,
{
...this.getAnalyticsParams(),
...this.withBlockaidMetricsParams(),
...getBlockaidTransactionMetricsParams(this.props.transaction),
},
);
return this.setState({ isReadyToApprove: true });
Expand Down
65 changes: 64 additions & 1 deletion app/util/blockaid/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,72 @@ import {
import * as NetworkControllerMock from '../../selectors/networkController';
import { NETWORKS_CHAIN_ID } from '../../constants/network';

import { getBlockaidMetricsParams, isBlockaidSupportedOnCurrentChain } from '.';
import {
getBlockaidMetricsParams,
isBlockaidSupportedOnCurrentChain,
getBlockaidTransactionMetricsParams,
} from '.';

describe('Blockaid util', () => {
describe('getBlockaidTransactionMetricsParams', () => {
beforeEach(() => {
jest
.spyOn(NetworkControllerMock, 'selectChainId')
.mockReturnValue(NETWORKS_CHAIN_ID.MAINNET);
});

afterEach(() => {
jest.clearAllMocks();
});

it('returns empty object when transaction id does not match security response id', () => {
const transaction = {
id: 1,
currentTransactionSecurityAlertResponse: {
id: 2,
response: {
result_type: ResultType.Malicious,
reason: Reason.notApplicable,
providerRequestsCount: {
eth_call: 5,
eth_getCode: 3,
},
features: [],
},
},
};
const result = getBlockaidTransactionMetricsParams(transaction);
expect(result).toStrictEqual({});
});

it('returns metrics params object when transaction id matches security response id', () => {
const transaction = {
id: 1,
currentTransactionSecurityAlertResponse: {
id: 1,
response: {
result_type: ResultType.Malicious,
reason: Reason.notApplicable,
providerRequestsCount: {
eth_call: 5,
eth_getCode: 3,
},
features: [],
},
},
};

const result = getBlockaidTransactionMetricsParams(transaction);
expect(result).toEqual({
ui_customizations: ['flagged_as_malicious'],
security_alert_response: ResultType.Malicious,
security_alert_reason: Reason.notApplicable,
ppom_eth_call_count: 5,
ppom_eth_getCode_count: 3,
});
});
});

describe('getBlockaidMetricsParams', () => {
beforeEach(() => {
jest
Expand Down
31 changes: 31 additions & 0 deletions app/util/blockaid/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,17 @@ import {
import { BLOCKAID_SUPPORTED_CHAIN_IDS, getDecimalChainId } from '../networks';
import { store } from '../../store';
import { selectChainId } from '../../selectors/networkController';
import type { TransactionMeta } from '@metamask/transaction-controller';

interface TransactionSecurityAlertResponseType {
currentTransactionSecurityAlertResponse: {
id: string;
response: SecurityAlertResponse;
};
}

export type TransactionType = TransactionMeta &
TransactionSecurityAlertResponseType;

export const isSupportedChainId = (chainId: string) => {
/**
Expand Down Expand Up @@ -67,3 +78,23 @@ export const getBlockaidMetricsParams = (

return additionalParams;
};

export const getBlockaidTransactionMetricsParams = (
transaction: TransactionType,
) => {
let blockaidParams = {};

if (!transaction) {
return blockaidParams;
}

if (
transaction.id === transaction?.currentTransactionSecurityAlertResponse?.id
) {
blockaidParams = getBlockaidMetricsParams(
transaction.currentTransactionSecurityAlertResponse?.response,
);
}

return blockaidParams;
};

0 comments on commit 594cf3f

Please sign in to comment.