From 8a96c2bc52579f6fbee22caa510102785d04fe50 Mon Sep 17 00:00:00 2001 From: Olusegun Akintayo Date: Mon, 23 Oct 2023 14:50:57 +0100 Subject: [PATCH] feat: Metrics for when Blockaid banner is shown. (#21396) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **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 [MetaMask/metamask-extension#17688](https://github.com/MetaMask/metamask-extension/pull/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 --- test/jest/rendering.js | 5 +- .../signature-request-original.component.js | 26 ++++++++ .../signature-request-siwe.js | 37 ++++++++++- .../signature-request/signature-request.js | 22 +++++++ .../transaction-alerts/transaction-alerts.js | 25 ++++++- ui/helpers/utils/metric.test.js | 66 ++++++++++++++++++- ui/helpers/utils/metrics.js | 44 +++++++++++++ .../confirm-approve-content.component.js | 28 ++++++++ ui/pages/token-allowance/token-allowance.js | 29 ++++++++ 9 files changed, 277 insertions(+), 5 deletions(-) diff --git a/test/jest/rendering.js b/test/jest/rendering.js index 2c46c306ad2a..091c2a629e2b 100644 --- a/test/jest/rendering.js +++ b/test/jest/rendering.js @@ -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; @@ -38,7 +39,9 @@ export function renderWithProvider(component, store, initialEntries) { const WithoutStore = () => ( - {children} + + {children} + ); diff --git a/ui/components/app/signature-request-original/signature-request-original.component.js b/ui/components/app/signature-request-original/signature-request-original.component.js index 073f6d5f61c1..176bd5c3bce5 100644 --- a/ui/components/app/signature-request-original/signature-request-original.component.js +++ b/ui/components/app/signature-request-original/signature-request-original.component.js @@ -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'; @@ -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 = { @@ -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); diff --git a/ui/components/app/signature-request-siwe/signature-request-siwe.js b/ui/components/app/signature-request-siwe/signature-request-siwe.js index 602aa2fe7141..7d85ea1cf159 100644 --- a/ui/components/app/signature-request-siwe/signature-request-siwe.js +++ b/ui/components/app/signature-request-siwe/signature-request-siwe.js @@ -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'; @@ -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'; @@ -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: { diff --git a/ui/components/app/signature-request/signature-request.js b/ui/components/app/signature-request/signature-request.js index e5de3f67cca0..f29601010ae3 100644 --- a/ui/components/app/signature-request/signature-request.js +++ b/ui/components/app/signature-request/signature-request.js @@ -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 @@ -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 (
diff --git a/ui/components/app/transaction-alerts/transaction-alerts.js b/ui/components/app/transaction-alerts/transaction-alerts.js index f2ef032eab69..00f0c6ab58ac 100644 --- a/ui/components/app/transaction-alerts/transaction-alerts.js +++ b/ui/components/app/transaction-alerts/transaction-alerts.js @@ -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'; @@ -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, @@ -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({ diff --git a/ui/helpers/utils/metric.test.js b/ui/helpers/utils/metric.test.js index b3a4981d5a4e..5dbfb8ced832 100644 --- a/ui/helpers/utils/metric.test.js +++ b/ui/helpers/utils/metric.test.js @@ -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', () => { @@ -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, + }); + }); +}); diff --git a/ui/helpers/utils/metrics.js b/ui/helpers/utils/metrics.js index 56cf958730e0..796377bcdee6 100644 --- a/ui/helpers/utils/metrics.js +++ b/ui/helpers/utils/metrics.js @@ -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 ''; @@ -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 diff --git a/ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js b/ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js index 4ce205308f32..5dd90c7e9814 100644 --- a/ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js +++ b/ui/pages/confirm-approve/confirm-approve-content/confirm-approve-content.component.js @@ -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'; @@ -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 = { @@ -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, diff --git a/ui/pages/token-allowance/token-allowance.js b/ui/pages/token-allowance/token-allowance.js index 2bae6f067205..78ac171323db 100644 --- a/ui/pages/token-allowance/token-allowance.js +++ b/ui/pages/token-allowance/token-allowance.js @@ -63,6 +63,12 @@ import { import { isSuspiciousResponse } from '../../../shared/modules/security-provider.utils'; ///: 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'; +import { MetaMetricsContext } from '../../contexts/metametrics'; ///: END:ONLY_INCLUDE_IN import { ConfirmPageContainerNavigation } from '../../components/app/confirm-page-container'; import { useSimulationFailureWarning } from '../../hooks/useSimulationFailureWarning'; @@ -137,6 +143,29 @@ export default function TokenAllowance({ const nextNonce = useSelector(getNextSuggestedNonce); const customNonceValue = useSelector(getCustomNonceValue); + ///: 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 + /** * We set the customSpendingCap to the dappProposedTokenAmount, if provided, rather than setting customTokenAmount * because customTokenAmount is reserved for custom user input. This is only set once when the component is mounted.