Skip to content

Commit

Permalink
feat: Metrics for when Blockaid banner is shown. (#21396)
Browse files Browse the repository at this point in the history
## **Description**

Adding analytics to Blockaid will help us understand and evaluate the
performance of this feature.

For users that have the Blockaid feature enabled, on performing a dapp
transaction, the following should happen

1. Add a security_alert_response property to Transactions and Signature
events, the value of this property should be the [result_type parameter
that is returned by
Blockaid](https://wobbly-nutmeg-8a5.notion.site/PPOM-Validation-Schema-Attack-Types-555e7f6a5b6c4b6caa9ced59561c8667)
(e.g. set_approval_for_all_famring).
2. Add a security_alert_reason property to Transactions and Signature
events, the value of this property should be the [reason parameter that
is returned by
Blockaid](https://wobbly-nutmeg-8a5.notion.site/PPOM-Validation-Schema-Attack-Types-555e7f6a5b6c4b6caa9ced59561c8667).
When there's no value for this property (because user has the feature
disabled or Blockaid response does not contain a reason value), we
should pass it the value not_applicable.
Create ui_customizations list property for transactions and signatures
events on mobile - this property should be similar to the one created
here on extension
[#17688](#17688).
This property should be a type list property (more info
[here](https://help.mixpanel.com/hc/en-us/articles/115004547063-Properties-Supported-Data-Types#list).
3. Whenever we display a warning to the user as a result of Blockaid
flagging a request as malicious, we should add the flagged_as_malicious
value to the ui_customizations property in the respective Transactions
or Signature events

## **Manual testing steps**

1. Start metamask in flask mode `yarn start:flask`
2. Go to testdapp and click on any of the PPOM buttons
3. See that blockaid banner is shown
4. Check that the above properties are added to the metrics event for
the particular page.


## **Related issues**

**Issue**

https://github.com/MetaMask/MetaMask-planning/issues/929

## **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:
  - [x] What problem this PR is solving.
  - [x] How this problem was solved.
  - [x] How reviewers can test my changes.
- [x] I’ve indicated what issue this PR is linked to: Fixes #???
- [x] I’ve included tests if applicable.
- [x] I’ve documented any added code.
- [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)).
- [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.

---------

Signed-off-by: Akintayo A. Olusegun <[email protected]>
  • Loading branch information
segun authored Oct 23, 2023
1 parent 416c8ff commit 8a96c2b
Show file tree
Hide file tree
Showing 9 changed files with 277 additions and 5 deletions.
5 changes: 4 additions & 1 deletion test/jest/rendering.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import PropTypes from 'prop-types';
import { I18nContext, LegacyI18nProvider } from '../../ui/contexts/i18n';
import { getMessage } from '../../ui/helpers/utils/i18n-helper';
import * as en from '../../app/_locales/en/messages.json';
import { LegacyMetaMetricsProvider } from '../../ui/contexts/metametrics';

export const I18nProvider = (props) => {
const { currentLocale, current, en: eng } = props;
Expand Down Expand Up @@ -38,7 +39,9 @@ export function renderWithProvider(component, store, initialEntries) {
const WithoutStore = () => (
<MemoryRouter initialEntries={initialEntries || ['/']} initialIndex={0}>
<I18nProvider currentLocale="en" current={en} en={en}>
<LegacyI18nProvider>{children}</LegacyI18nProvider>
<LegacyI18nProvider>
<LegacyMetaMetricsProvider>{children}</LegacyMetaMetricsProvider>
</LegacyI18nProvider>
</I18nProvider>
</MemoryRouter>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ import {
///: END:ONLY_INCLUDE_IN
} from '../../component-library';
///: BEGIN:ONLY_INCLUDE_IN(blockaid)
import {
MetaMetricsEventCategory,
MetaMetricsEventName,
} from '../../../../shared/constants/metametrics';
import { getBlockaidMetricsParams } from '../../../helpers/utils/metrics';
import BlockaidBannerAlert from '../security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert';
///: END:ONLY_INCLUDE_IN
import ConfirmPageContainerNavigation from '../confirm-page-container/confirm-page-container-navigation';
Expand All @@ -58,6 +63,7 @@ import SignatureRequestOriginalWarning from './signature-request-original-warnin
export default class SignatureRequestOriginal extends Component {
static contextTypes = {
t: PropTypes.func.isRequired,
trackEvent: PropTypes.func,
};

static propTypes = {
Expand Down Expand Up @@ -90,6 +96,26 @@ export default class SignatureRequestOriginal extends Component {
showSignatureRequestWarning: false,
};

///: BEGIN:ONLY_INCLUDE_IN(blockaid)
componentDidMount() {
const { txData } = this.props;
if (txData.securityAlertResponse) {
const blockaidMetricsParams = getBlockaidMetricsParams(
txData.securityAlertResponse,
);

this.context.trackEvent({
category: MetaMetricsEventCategory.Transactions,
event: MetaMetricsEventName.SignatureRequested,
properties: {
action: 'Sign Request',
...blockaidMetricsParams,
},
});
}
}
///: END:ONLY_INCLUDE_IN

msgHexToText = (hex) => {
try {
const stripped = stripHexPrefix(hex);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import React, { useCallback, useContext, useState } from 'react';
import React, {
useCallback,
useContext,
///: BEGIN:ONLY_INCLUDE_IN(blockaid)
useEffect,
///: END:ONLY_INCLUDE_IN
useState,
} from 'react';
import PropTypes from 'prop-types';
import { useSelector, useDispatch } from 'react-redux';
import { useHistory } from 'react-router-dom';
Expand Down Expand Up @@ -40,6 +47,12 @@ import ConfirmPageContainerNavigation from '../confirm-page-container/confirm-pa
import { getMostRecentOverviewPage } from '../../../ducks/history/history';
///: BEGIN:ONLY_INCLUDE_IN(blockaid)
import BlockaidBannerAlert from '../security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert';
import { getBlockaidMetricsParams } from '../../../helpers/utils/metrics';
import { MetaMetricsContext } from '../../../contexts/metametrics';
import {
MetaMetricsEventCategory,
MetaMetricsEventName,
} from '../../../../shared/constants/metametrics';
///: END:ONLY_INCLUDE_IN
import LedgerInstructionField from '../ledger-instruction-field';

Expand All @@ -56,6 +69,28 @@ export default function SignatureRequestSIWE({ txData }) {
const messagesCount = useSelector(getTotalUnapprovedMessagesCount);
const messagesList = useSelector(unconfirmedMessagesHashSelector);
const mostRecentOverviewPage = useSelector(getMostRecentOverviewPage);
///: BEGIN:ONLY_INCLUDE_IN(blockaid)
const trackEvent = useContext(MetaMetricsContext);
///: END:ONLY_INCLUDE_IN

///: BEGIN:ONLY_INCLUDE_IN(blockaid)
useEffect(() => {
if (txData.securityAlertResponse) {
const blockaidMetricsParams = getBlockaidMetricsParams(
txData.securityAlertResponse,
);

trackEvent({
category: MetaMetricsEventCategory.Transactions,
event: MetaMetricsEventName.SignatureRequested,
properties: {
action: 'Sign Request',
...blockaidMetricsParams,
},
});
}
}, [txData?.securityAlertResponse]);
///: END:ONLY_INCLUDE_IN

const {
msgParams: {
Expand Down
22 changes: 22 additions & 0 deletions ui/components/app/signature-request/signature-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ import { showCustodyConfirmLink } from '../../../store/institutional/institution
import { useMMICustodySignMessage } from '../../../hooks/useMMICustodySignMessage';
///: END:ONLY_INCLUDE_IN
///: BEGIN:ONLY_INCLUDE_IN(blockaid)
import { getBlockaidMetricsParams } from '../../../helpers/utils/metrics';
import BlockaidBannerAlert from '../security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert';
///: END:ONLY_INCLUDE_IN

Expand Down Expand Up @@ -255,6 +256,27 @@ const SignatureRequest = ({ txData }) => {
]);
///: END:ONLY_INCLUDE_IN

///: BEGIN:ONLY_INCLUDE_IN(blockaid)
useEffect(() => {
if (txData.securityAlertResponse) {
const blockaidMetricsParams = getBlockaidMetricsParams(
txData.securityAlertResponse,
);

trackEvent({
category: MetaMetricsEventCategory.Transactions,
event: MetaMetricsEventName.SignatureRequested,
properties: {
action: 'Sign Request',
type,
version,
...blockaidMetricsParams,
},
});
}
}, [txData?.securityAlertResponse]);
///: END:ONLY_INCLUDE_IN

return (
<div className="signature-request">
<ConfirmPageContainerNavigation />
Expand Down
25 changes: 23 additions & 2 deletions ui/components/app/transaction-alerts/transaction-alerts.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, {
///: BEGIN:ONLY_INCLUDE_IN(blockaid)
useCallback,
useContext,
useEffect,
///: END:ONLY_INCLUDE_IN
} from 'react';
import PropTypes from 'prop-types';
Expand All @@ -27,12 +28,13 @@ import { getNativeCurrency } from '../../../ducks/metamask/metamask';
import { TransactionType } from '../../../../shared/constants/transaction';
import { parseStandardTokenTransactionData } from '../../../../shared/modules/transaction.utils';
import { getTokenValueParam } from '../../../../shared/lib/metamask-controller-utils';
///: BEGIN:ONLY_INCLUDE_IN(blockaid)
import {
///: BEGIN:ONLY_INCLUDE_IN(blockaid)
MetaMetricsEventCategory,
MetaMetricsEventName,
///: END:ONLY_INCLUDE_IN
} from '../../../../shared/constants/metametrics';
import { getBlockaidMetricsParams } from '../../../helpers/utils/metrics';
///: END:ONLY_INCLUDE_IN

const TransactionAlerts = ({
userAcknowledgedGasMissing,
Expand Down Expand Up @@ -72,6 +74,25 @@ const TransactionAlerts = ({
const trackEvent = useContext(MetaMetricsContext);
///: END:ONLY_INCLUDE_IN

///: BEGIN:ONLY_INCLUDE_IN(blockaid)
useEffect(() => {
if (txData.securityAlertResponse) {
const blockaidMetricsParams = getBlockaidMetricsParams(
txData.securityAlertResponse,
);

trackEvent({
category: MetaMetricsEventCategory.Transactions,
event: MetaMetricsEventName.SignatureRequested,
properties: {
action: 'Sign Request',
...blockaidMetricsParams,
},
});
}
}, [txData?.securityAlertResponse]);
///: END:ONLY_INCLUDE_IN

///: BEGIN:ONLY_INCLUDE_IN(blockaid)
const onClickSupportLink = useCallback(() => {
trackEvent({
Expand Down
66 changes: 65 additions & 1 deletion ui/helpers/utils/metric.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { getMethodName } from './metrics';
import {
BlockaidReason,
BlockaidResultType,
} from '../../../shared/constants/security-provider';
import { getBlockaidMetricsParams, getMethodName } from './metrics';

describe('getMethodName', () => {
it('should get correct method names', () => {
Expand All @@ -11,3 +15,63 @@ describe('getMethodName', () => {
);
});
});

describe('getBlockaidMetricsParams', () => {
it('should return empty object when securityAlertResponse is not defined', () => {
const result = getBlockaidMetricsParams(undefined);
expect(result).toStrictEqual({});
});

it('should return additionalParams object when securityAlertResponse defined', () => {
const securityAlertResponse = {
result_type: BlockaidResultType.Malicious,
reason: BlockaidReason.notApplicable,
providerRequestsCount: {
eth_call: 5,
eth_getCode: 3,
},
features: [],
};

const result = getBlockaidMetricsParams(securityAlertResponse);
expect(result).toStrictEqual({
ui_customizations: ['flagged_as_malicious'],
security_alert_response: BlockaidResultType.Malicious,
security_alert_reason: BlockaidReason.notApplicable,
ppom_eth_call_count: 5,
ppom_eth_getCode_count: 3,
});
});

it('should not return eth call counts if providerRequestsCount is empty', () => {
const securityAlertResponse = {
result_type: BlockaidResultType.Malicious,
reason: BlockaidReason.notApplicable,
features: [],
providerRequestsCount: {},
};

const result = getBlockaidMetricsParams(securityAlertResponse);
expect(result).toStrictEqual({
ui_customizations: ['flagged_as_malicious'],
security_alert_response: BlockaidResultType.Malicious,
security_alert_reason: BlockaidReason.notApplicable,
});
});

it('should not return eth call counts if providerRequestsCount is undefined', () => {
const securityAlertResponse = {
result_type: BlockaidResultType.Malicious,
reason: BlockaidReason.notApplicable,
features: [],
providerRequestsCount: undefined,
};

const result = getBlockaidMetricsParams(securityAlertResponse);
expect(result).toStrictEqual({
ui_customizations: ['flagged_as_malicious'],
security_alert_response: BlockaidResultType.Malicious,
security_alert_reason: BlockaidReason.notApplicable,
});
});
});
44 changes: 44 additions & 0 deletions ui/helpers/utils/metrics.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
///: BEGIN:ONLY_INCLUDE_IN(blockaid)
import {
BlockaidReason,
BlockaidResultType,
} from '../../../shared/constants/security-provider';
///: END:ONLY_INCLUDE_IN

export function getMethodName(camelCase) {
if (!camelCase || typeof camelCase !== 'string') {
return '';
Expand All @@ -16,3 +23,40 @@ export function formatAccountType(accountType) {

return accountType;
}

///: BEGIN:ONLY_INCLUDE_IN(blockaid)
export const getBlockaidMetricsParams = (securityAlertResponse = null) => {
const additionalParams = {};

if (securityAlertResponse) {
const {
result_type: resultType,
reason,
providerRequestsCount,
} = securityAlertResponse;

if (resultType === BlockaidResultType.Malicious) {
additionalParams.ui_customizations = ['flagged_as_malicious'];
}

if (resultType !== BlockaidResultType.Benign) {
additionalParams.security_alert_reason = BlockaidReason.notApplicable;

if (reason) {
additionalParams.security_alert_response = resultType;
additionalParams.security_alert_reason = reason;
}
}

// add counts of each RPC call
if (providerRequestsCount) {
Object.keys(providerRequestsCount).forEach((key) => {
const metricKey = `ppom_${key}_count`;
additionalParams[metricKey] = providerRequestsCount[key];
});
}
}

return additionalParams;
};
///: END:ONLY_INCLUDE_IN
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ import { ConfirmPageContainerWarning } from '../../../components/app/confirm-pag
import LedgerInstructionField from '../../../components/app/ledger-instruction-field';
///: BEGIN:ONLY_INCLUDE_IN(blockaid)
import BlockaidBannerAlert from '../../../components/app/security-provider-banner-alert/blockaid-banner-alert/blockaid-banner-alert';
import { getBlockaidMetricsParams } from '../../../helpers/utils/metrics';
import {
MetaMetricsEventCategory,
MetaMetricsEventName,
} from '../../../../shared/constants/metametrics';
///: END:ONLY_INCLUDE_IN
import { isSuspiciousResponse } from '../../../../shared/modules/security-provider.utils';

Expand All @@ -46,6 +51,9 @@ import { COPY_OPTIONS } from '../../../../shared/constants/copy';
export default class ConfirmApproveContent extends Component {
static contextTypes = {
t: PropTypes.func,
///: BEGIN:ONLY_INCLUDE_IN(blockaid)
trackEvent: PropTypes.func,
///: END:ONLY_INCLUDE_IN
};

static propTypes = {
Expand Down Expand Up @@ -95,6 +103,26 @@ export default class ConfirmApproveContent extends Component {
setShowContractDetails: false,
};

///: BEGIN:ONLY_INCLUDE_IN(blockaid)
componentDidMount() {
const { txData } = this.props;
if (txData.securityAlertResponse) {
const blockaidMetricsParams = getBlockaidMetricsParams(
txData.securityAlertResponse,
);

this.context.trackEvent({
category: MetaMetricsEventCategory.Transactions,
event: MetaMetricsEventName.SignatureRequested,
properties: {
action: 'Sign Request',
...blockaidMetricsParams,
},
});
}
}
///: END:ONLY_INCLUDE_IN

renderApproveContentCard({
showHeader = true,
symbol,
Expand Down
Loading

0 comments on commit 8a96c2b

Please sign in to comment.