fix: enhance metrics for Transaction/Signatures events (Shield) cp-13.10.0

package.json

@@ -352,7 +352,7 @@
     "@metamask/scure-bip39": "^2.0.3",
     "@metamask/seedless-onboarding-controller": "^5.0.0",
     "@metamask/selected-network-controller": "^25.0.0",
-    "@metamask/shield-controller": "^1.2.0",
+    "@metamask/shield-controller": "^2.1.0",
     "@metamask/signature-controller": "^35.0.0",
     "@metamask/smart-transactions-controller": "^20.0.0",
     "@metamask/snaps-controllers": "^16.1.1",

ui/pages/confirmations/hooks/alerts/useShieldCoverageAlert.test.ts

@@ -54,6 +54,7 @@ describe('useShieldCoverageAlert', () => {
     status?: string,
     reasonCode = 'E104',
     isTransaction: boolean = true,
+    latency: number | string = 'N/A',
   ): Record<string, unknown> => {
     const mockId = '123';
     const baseState = isTransaction
@@ -84,6 +85,9 @@ describe('useShieldCoverageAlert', () => {
               {
                 status,
                 reasonCode,
+                metrics: {
+                  latency,
+                },
               },
             ],
           },
@@ -157,7 +161,7 @@ describe('useShieldCoverageAlert', () => {
   });
 
   it('updates transaction event fragment with covered status', () => {
-    const state = getStateWithCoverage('covered', 'E104');
+    const state = getStateWithCoverage('covered', 'E104', true, 150);
     renderHookWithConfirmContextProvider(() => useShieldCoverageAlert(), state);
     expect(updateTransactionEventFragmentMock).toHaveBeenCalledWith(
       expect.objectContaining({
@@ -166,6 +170,8 @@ describe('useShieldCoverageAlert', () => {
           shield_result: 'covered',
           // eslint-disable-next-line @typescript-eslint/naming-convention
           shield_reason: 'shieldCoverageAlertCovered',
+          // eslint-disable-next-line @typescript-eslint/naming-convention
+          shield_result_response_latency_ms: 150,
         },
       }),
       expect.anything(),
@@ -182,6 +188,8 @@ describe('useShieldCoverageAlert', () => {
           shield_result: 'not_covered_malicious',
           // eslint-disable-next-line @typescript-eslint/naming-convention
           shield_reason: 'shieldCoverageAlertMessagePotentialRisks',
+          // eslint-disable-next-line @typescript-eslint/naming-convention
+          shield_result_response_latency_ms: 'N/A',
         },
       }),
       expect.anything(),
@@ -198,6 +206,8 @@ describe('useShieldCoverageAlert', () => {
           shield_result: 'not_covered',
           // eslint-disable-next-line @typescript-eslint/naming-convention
           shield_reason: 'shieldCoverageAlertMessagePotentialRisks',
+          // eslint-disable-next-line @typescript-eslint/naming-convention
+          shield_result_response_latency_ms: 'N/A',
         },
       }),
       expect.anything(),
@@ -214,14 +224,16 @@ describe('useShieldCoverageAlert', () => {
           shield_result: 'loading',
           // eslint-disable-next-line @typescript-eslint/naming-convention
           shield_reason: 'shieldCoverageAlertMessagePotentialRisks',
+          // eslint-disable-next-line @typescript-eslint/naming-convention
+          shield_result_response_latency_ms: 'N/A',
         },
       }),
       expect.anything(),
     );
   });
 
   it('updates signature event fragment with correct metrics', () => {
-    const state = getStateWithCoverage('covered', 'E104', false);
+    const state = getStateWithCoverage('covered', 'E104', false, 200);
     renderHookWithConfirmContextProvider(() => useShieldCoverageAlert(), state);
 
     expect(updateSignatureEventFragmentMock).toHaveBeenCalledWith(
@@ -231,6 +243,8 @@ describe('useShieldCoverageAlert', () => {
           shield_result: 'covered',
           // eslint-disable-next-line @typescript-eslint/naming-convention
           shield_reason: 'shieldCoverageAlertCovered',
+          // eslint-disable-next-line @typescript-eslint/naming-convention
+          shield_result_response_latency_ms: 200,
         },
       }),
     );

ui/pages/confirmations/hooks/alerts/useShieldCoverageAlert.ts

@@ -13,6 +13,7 @@ import {
 } from '../../../../helpers/constants/design-system';
 import { useI18nContext } from '../../../../hooks/useI18nContext';
 import {
+  getCoverageMetrics,
   getCoverageStatus,
   ShieldState,
 } from '../../../../selectors/shield/coverage';
@@ -25,6 +26,8 @@ import { useSignatureEventFragment } from '../useSignatureEventFragment';
 import { useTransactionEventFragment } from '../useTransactionEventFragment';
 import { ShieldCoverageAlertMessage } from './transactions/ShieldCoverageAlertMessage';
 
+const N_A = 'N/A';
+
 const getModalBodyStr = (reasonCode: string | undefined) => {
   // grouping codes with a fallthrough pattern is not allowed by the linter
   let modalBodyStr: string;
@@ -169,10 +172,11 @@ const getShieldResult = (
       return 'not_covered_malicious';
     case 'unknown':
       return 'not_covered';
-    case undefined:
-    case 'not_shown':
-      return 'loading';
     default:
+      // Returns 'loading' for:
+      // - undefined: coverage check not yet initiated or in progress
+      // - 'not_shown': coverage didn't load before user confirmed
+      // - any unexpected values: fail safe to loading state
       return 'loading';
   }
 };
@@ -190,6 +194,9 @@ export function useShieldCoverageAlert(): Alert[] {
   const { reasonCode, status } = useSelector((state) =>
     getCoverageStatus(state as ShieldState, id),
   );
+  const metrics = useSelector((state) =>
+    getCoverageMetrics(state as ShieldState, id),
+  );
 
   const { isEnabled, isPaused } = useEnableShieldCoverageChecks();
 
@@ -221,6 +228,8 @@ export function useShieldCoverageAlert(): Alert[] {
         shield_result: getShieldResult(status),
         // eslint-disable-next-line @typescript-eslint/naming-convention
         shield_reason: modalBodyStr,
+        // eslint-disable-next-line @typescript-eslint/naming-convention
+        shield_result_response_latency_ms: metrics?.latency ?? N_A,
       };
 
       if (isSignatureTransactionType(currentConfirmation)) {
@@ -237,12 +246,13 @@ export function useShieldCoverageAlert(): Alert[] {
       }
     }
   }, [
-    status,
-    modalBodyStr,
-    isEnabled,
-    isPaused,
     currentConfirmation,
     id,
+    isEnabled,
+    isPaused,
+    metrics?.latency,
+    modalBodyStr,
+    status,
     updateSignatureEventFragment,
     updateTransactionEventFragment,
   ]);

ui/selectors/shield/coverage.test.ts

@@ -1,54 +1,172 @@
 import type { CoverageStatus } from '@metamask/shield-controller';
-import { getCoverageStatus, ShieldState } from './coverage';
+import {
+  CoverageMetrics,
+  getCoverageMetrics,
+  getCoverageStatus,
+  ShieldState,
+} from './coverage';
 
 describe('shield coverage selectors', () => {
   const confirmationId = 'abc123';
 
-  it('returns undefined when there are no coverage results', () => {
-    const state = {
+  const createStateWithResult = (result: {
+    status?: CoverageStatus;
+    reasonCode?: string;
+    metrics?: { latency?: number };
+  }): ShieldState =>
+    ({
       metamask: {
-        coverageResults: {},
+        coverageResults: {
+          [confirmationId]: {
+            results: [result],
+          },
+        },
       },
-    } as unknown as ShieldState;
+    }) as unknown as ShieldState;
+
+  describe('getCoverageStatus', () => {
+    it('returns undefined when there are no coverage results', () => {
+      const state = {
+        metamask: {
+          coverageResults: {},
+        },
+      } as unknown as ShieldState;
+
+      const result = getCoverageStatus(state, confirmationId);
+      expect(result).toEqual({ status: undefined, reasonCode: undefined });
+    });
+
+    it('returns undefined when results array is empty', () => {
+      const state = {
+        metamask: {
+          coverageResults: {
+            [confirmationId]: { results: [] },
+          },
+        },
+      } as unknown as ShieldState;
+
+      const result = getCoverageStatus(state, confirmationId);
+      expect(result).toEqual({ status: undefined, reasonCode: undefined });
+    });
+
+    it('returns status and reasonCode from the first result', () => {
+      const status: CoverageStatus = 'covered';
+      const reasonCode = 'ok';
+      const state = {
+        metamask: {
+          coverageResults: {
+            [confirmationId]: {
+              results: [
+                {
+                  status,
+                  reasonCode,
+                },
+                { status: 'other', reasonCode: 'ignored' },
+              ],
+            },
+          },
+        },
+      } as unknown as ShieldState;
 
-    const result = getCoverageStatus(state, confirmationId);
-    expect(result).toEqual({ status: undefined, reasonCode: undefined });
+      const result = getCoverageStatus(state, confirmationId);
+      expect(result.status).toBe(status);
+      expect(result.reasonCode).toBe(reasonCode);
+    });
   });
 
-  it('returns undefined when results array is empty', () => {
-    const state = {
-      metamask: {
-        coverageResults: {
-          [confirmationId]: { results: [] },
+  describe('getCoverageMetrics', () => {
+    it('returns undefined when there are no coverage results', () => {
+      const state = {
+        metamask: {
+          coverageResults: {},
         },
+      } as unknown as ShieldState;
+
+      const result = getCoverageMetrics(state, confirmationId);
+      expect(result).toBeUndefined();
+    });
+
+    const metricsTestCases = [
+      {
+        description: 'metrics with latency',
+        metrics: { latency: 123 },
+        expectedLatency: 123,
+      },
+      {
+        description: 'metrics with latency value of 0',
+        metrics: { latency: 0 },
+        expectedLatency: 0,
+      },
+      {
+        description: 'empty metrics object',
+        metrics: {},
+        expectedLatency: undefined,
       },
-    } as unknown as ShieldState;
+      {
+        description: 'large latency values',
+        metrics: { latency: 999999 },
+        expectedLatency: 999999,
+      },
+    ];
+    // @ts-expect-error This function is missing from the Mocha type definitions
+    it.each(metricsTestCases)(
+      'returns $description',
+      ({
+        metrics,
+        expectedLatency,
+      }: {
+        metrics: CoverageMetrics;
+        expectedLatency?: number;
+      }) => {
+        const state = createStateWithResult({
+          status: 'covered',
+          metrics,
+        });
 
-    const result = getCoverageStatus(state, confirmationId);
-    expect(result).toEqual({ status: undefined, reasonCode: undefined });
-  });
+        const result = getCoverageMetrics(state, confirmationId);
 
-  it('returns status and reasonCode from the first result', () => {
-    const status: CoverageStatus = 'covered';
-    const reasonCode = 'ok';
-    const state = {
-      metamask: {
-        coverageResults: {
-          [confirmationId]: {
-            results: [
-              {
-                status,
-                reasonCode,
-              },
-              { status: 'other', reasonCode: 'ignored' },
-            ],
+        expect(result).toEqual(metrics);
+        expect(result?.latency).toBe(expectedLatency);
+      },
+    );
+
+    it('returns undefined when metrics property is missing', () => {
+      const state = createStateWithResult({
+        status: 'covered',
+        reasonCode: 'ok',
+      });
+
+      const result = getCoverageMetrics(state, confirmationId);
+
+      expect(result).toBeUndefined();
+    });
+
+    it('returns metrics from the first result only', () => {
+      const metrics = { latency: 123 };
+      const state = {
+        metamask: {
+          coverageResults: {
+            [confirmationId]: {
+              results: [
+                {
+                  status: 'covered',
+                  reasonCode: 'ok',
+                  metrics,
+                },
+                {
+                  status: 'unknown',
+                  metrics: { latency: 456 },
+                },
+              ],
+            },
           },
         },
-      },
-    } as unknown as ShieldState;
+      } as unknown as ShieldState;
+
+      const result = getCoverageMetrics(state, confirmationId);
 
-    const result = getCoverageStatus(state, confirmationId);
-    expect(result.status).toBe(status);
-    expect(result.reasonCode).toBe(reasonCode);
+      expect(result).toEqual(metrics);
+      expect(result?.latency).toBe(123);
+    });
   });
 });