feat: Add anonymized signature metrics #27298

pnarayanaswamy · 2024-09-20T06:50:39Z

Description

Adds logic to send Signature Requested Anon, Signature Approved Anon and Signature Rejected Anon events when the signature is of type signTypedData_v4 with additional anonymous properties.
Updates the e2e to check the correct Anon events are sent with the additional properties
Added additional unit tests to increase sonar coverage
Exclude the test folder from sonar checks

Related issues

Fixes: 3009

Manual testing steps

Go to test dapp
Under the Permit section, click on Sign

This should trigger the Signature Anon events with:

eip712_verifyingContract == 0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC
eip712_domain_version == 1
eip712_domain_name == MyToken

Go to test dapp
Under the SignTypedData_v4 section, click on Sign

This should trigger the Signature Anon events with:

eip712_verifyingContract == 0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC
eip712_domain_version == 1
eip712_domain_name == Ether Mail

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). 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.

github-actions · 2024-09-20T06:50:53Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

metamaskbot · 2024-09-20T07:34:06Z

Builds ready [801a531]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1754 ± 45 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	369	2000	1687	314	151
		domContentLoaded	1595	1976	1726	91	44
		load	1598	1988	1754	94	45
		domInteractive	13	72	39	18	9

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 772 Bytes (0.02%)
ui: 0 Bytes (0.00%)
common: 152 Bytes (0.00%)

metamaskbot · 2024-09-20T07:57:25Z

Builds ready [b2139e1]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1765 ± 94 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	236	2330	1696	387	186
		domContentLoaded	1539	2187	1733	186	89
		load	1548	2277	1765	196	94
		domInteractive	22	82	35	16	8

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 772 Bytes (0.02%)
ui: 0 Bytes (0.00%)
common: 152 Bytes (0.00%)

metamaskbot · 2024-09-20T09:40:20Z

Builds ready [256f2a4]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1835 ± 172 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1559	3003	1835	355	170
		domContentLoaded	1524	2954	1808	342	164
		load	1556	3001	1835	357	172
		domInteractive	15	110	39	22	11

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 772 Bytes (0.02%)
ui: 0 Bytes (0.00%)
common: 152 Bytes (0.00%)

metamaskbot · 2024-09-20T10:49:12Z

Builds ready [ace801e]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1733 ± 74 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	230	2261	1664	361	173
		domContentLoaded	1533	2249	1709	152	73
		load	1544	2258	1733	154	74
		domInteractive	15	183	44	34	17

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 772 Bytes (0.02%)
ui: 0 Bytes (0.00%)
common: 152 Bytes (0.00%)

digiwand · 2024-09-20T13:18:47Z

app/scripts/controllers/metametrics.test.js

+        properties: { ...DEFAULT_EVENT_PROPERTIES },
+      });
+    });
+  });


[non-blocking] we could DRY these up by looping the tests together

digiwand · 2024-09-20T13:20:10Z

app/scripts/lib/createRPCMethodTrackingMiddleware.js

@@ -222,8 +224,7 @@ export default function createRPCMethodTrackingMiddleware({
  ) {
    const { origin, method, params } = req;

-    const rateLimitType =
-      RATE_LIMIT_MAP[method] ?? RATE_LIMIT_TYPES.RANDOM_SAMPLE;


why did we remove the RATE_LIMIT_TYPES.RANDOM_SAMPLE fallback?

the switch has a default condition that falls back to the RANDOM_SAMPLE anyhow, so this is redundant

digiwand · 2024-09-20T13:25:31Z

app/scripts/lib/createRPCMethodTrackingMiddleware.js

+            const parsedMessageData = parseTypedDataMessage(data);
+            sensitiveEventProperties = {};
+
+            eventProperties.eip712_primary_type = parsedMessageData.primaryType;
+            sensitiveEventProperties.eip712_verifyingContract =
+              parsedMessageData.domain.verifyingContract;
+            sensitiveEventProperties.eip712_domain_version =
+              parsedMessageData.domain.version;
+            sensitiveEventProperties.eip712_domain_name =
+              parsedMessageData.domain.name;


could do something like this. Would need to update primaryType again if this suggestion is accepted

Suggested change

const parsedMessageData = parseTypedDataMessage(data);

sensitiveEventProperties = {};

eventProperties.eip712_primary_type = parsedMessageData.primaryType;

sensitiveEventProperties.eip712_verifyingContract =

parsedMessageData.domain.verifyingContract;

sensitiveEventProperties.eip712_domain_version =

parsedMessageData.domain.version;

sensitiveEventProperties.eip712_domain_name =

parsedMessageData.domain.name;

const { domain, primaryType } = parseTypedDataMessage(data);

sensitiveEventProperties = {

eip712_verifyingContract: domain.verifyingContract,

eip712_domain_version: domain.version,

eip712_domain_name: domain.name,

};

eventProperties.eip712_primary_type = primaryType;

metamaskbot · 2024-09-20T15:33:43Z

Builds ready [cdec7d9]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1644 ± 47 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	307	1893	1532	404	194
		domContentLoaded	1470	1803	1626	90	43
		load	1474	1817	1644	97	47
		domInteractive	12	123	39	25	12

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 752 Bytes (0.02%)
ui: 0 Bytes (0.00%)
common: 152 Bytes (0.00%)

sonarcloud · 2024-09-23T06:55:48Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

metamaskbot · 2024-09-23T08:50:07Z

Builds ready [1a84558]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1703 ± 70 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1425	2203	1705	142	68
		domContentLoaded	1415	2079	1679	123	59
		load	1423	2223	1703	146	70
		domInteractive	15	79	36	17	8

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 752 Bytes (0.02%)
ui: 0 Bytes (0.00%)
common: 152 Bytes (0.00%)

digiwand · 2024-09-23T15:29:44Z

sonar-project.properties

@@ -3,7 +3,7 @@ sonar.organization=consensys

 # Source
 sonar.sources=app,development,offscreen,shared,test,types,ui
-sonar.exclusions=**/*.test.**,**/*.spec.**,app/images
+sonar.exclusions=**/*.test.**,**/*.spec.**,app/images,test/**


metamaskbot · 2024-09-29T17:09:52Z

Missing release label release-12.5.0 on PR. Adding release label release-12.5.0 on PR and removing other release labels(release-12.6.0), as PR was added to branch 12.5.0 when release was cut.

pnarayanaswamy added 5 commits September 20, 2024 08:49

feat: Add anonymized signature metrics

59be700

feat: Add anonymized signature metrics

c6b6add

chore: removed extra anon events

50c4551

chore: Added logic to send anon events and unit tests

5dac9ef

chore: Updated e2e to verify anon events are sent with the right data

801a531

github-actions bot added the team-confirmations Push issues to confirmations team label Sep 20, 2024

pnarayanaswamy changed the title ~~Feat sig anon metrics~~ feat: Add anonymized signature metrics Sep 20, 2024

pnarayanaswamy marked this pull request as ready for review September 20, 2024 06:57

pnarayanaswamy requested a review from a team as a code owner September 20, 2024 06:57

chore: fix helpers duplicates

b2139e1

chore: add unit tests for uncovered lines

256f2a4

chore: add unit tests for personal sign type

ace801e

pnarayanaswamy added 2 commits September 20, 2024 14:30

chore: change condition for rate limit for sonar coverage

0725009

chore: remove check for fragmentPayload and add missing unit tests

e9b92a7

digiwand reviewed Sep 20, 2024

View reviewed changes

chore: exclude test folder from sonar properties

6f4b995

digiwand reviewed Sep 20, 2024

View reviewed changes

pnarayanaswamy added 2 commits September 20, 2024 15:41

chore: refactor signature events test based on review

bce20f8

chore: revert jest config change

cdec7d9

digiwand approved these changes Sep 20, 2024

View reviewed changes

Merge branch 'develop' into feat-sig-anon-metrics

1a84558

jpuri approved these changes Sep 23, 2024

View reviewed changes

digiwand merged commit f17891a into develop Sep 23, 2024
77 checks passed

digiwand deleted the feat-sig-anon-metrics branch September 23, 2024 15:01

github-actions bot locked and limited conversation to collaborators Sep 23, 2024

metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 23, 2024

digiwand reviewed Sep 23, 2024

View reviewed changes

metamaskbot added release-12.5.0 Issue or pull request that will be included in release 12.5.0 and removed release-12.6.0 Issue or pull request that will be included in release 12.6.0 labels Sep 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add anonymized signature metrics #27298

feat: Add anonymized signature metrics #27298

pnarayanaswamy commented Sep 20, 2024 •

edited

Loading

github-actions bot commented Sep 20, 2024

metamaskbot commented Sep 20, 2024

metamaskbot commented Sep 20, 2024

metamaskbot commented Sep 20, 2024

metamaskbot commented Sep 20, 2024

digiwand Sep 20, 2024

pnarayanaswamy Sep 20, 2024

digiwand Sep 20, 2024

pnarayanaswamy Sep 20, 2024

digiwand Sep 20, 2024

metamaskbot commented Sep 20, 2024

sonarcloud bot commented Sep 23, 2024

metamaskbot commented Sep 23, 2024

digiwand Sep 23, 2024

metamaskbot commented Sep 29, 2024

feat: Add anonymized signature metrics #27298

feat: Add anonymized signature metrics #27298

Conversation

pnarayanaswamy commented Sep 20, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Sep 20, 2024

metamaskbot commented Sep 20, 2024

metamaskbot commented Sep 20, 2024

metamaskbot commented Sep 20, 2024

metamaskbot commented Sep 20, 2024

digiwand Sep 20, 2024

Choose a reason for hiding this comment

pnarayanaswamy Sep 20, 2024

Choose a reason for hiding this comment

digiwand Sep 20, 2024

Choose a reason for hiding this comment

pnarayanaswamy Sep 20, 2024

Choose a reason for hiding this comment

digiwand Sep 20, 2024

Choose a reason for hiding this comment

metamaskbot commented Sep 20, 2024

sonarcloud bot commented Sep 23, 2024

Quality Gate passed

metamaskbot commented Sep 23, 2024

digiwand Sep 23, 2024

Choose a reason for hiding this comment

metamaskbot commented Sep 29, 2024

pnarayanaswamy commented Sep 20, 2024 •

edited

Loading