Refactor eth_getEncryptionPublicKey handling #18319

bergarces · 2023-03-24T11:57:10Z

Explanation

Moves encryption-public-key-manager and related metamask-controller logic, responsible for eth_getEncryptionPublicKey, to a new EncryptionPublicKeyController that uses EncryptionPublicKeyManager from @metamask/message-managers.

See: https://github.com/MetaMask/MetaMask-planning/issues/394

Manual Testing Steps

Go to https://metamask.github.io/test-dapp/
Connect MM wallet
Go to the Encrypt / Decrypt section
Click on "Get Encryption Key"
If request is rejected, "Encryption key" field should display "Error: MetaMask EncryptionPublicKey: User denied message EncryptionPublicKey."
If request is approved, the encryption key should appear.

Pre-merge author checklist

I've clearly explained:
- What problem this PR is solving
- How this problem was solved
- How reviewers can test my changes
Sufficient automated test coverage has been added

Pre-merge reviewer checklist

Manual testing (e.g. pull and build branch, run in browser, test code being changed)
PR is linked to the appropriate GitHub issue
IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

github-actions · 2023-03-24T11:57:22Z

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.

bergarces · 2023-03-24T15:28:59Z

app/scripts/controllers/encryption-public-key.ts

+};
+
+export type StateMessage = Required<AbstractMessage> & {
+  msgParams: string;


The UI expects a string with the address, not a Required<AbstractMessageParams>.

app/scripts/controllers/encryption-public-key.ts

app/scripts/metamask-controller.js

package.json

.yarn/patches/@metamask-message-manager-npm-2.0.0-f23d924f4f.patch

app/scripts/controllers/sign.ts

metamaskbot · 2023-03-29T13:46:12Z

Builds ready [0206733]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1612 ± 42 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	89	129	112	10	5
		domContentLoaded	1472	1821	1604	90	43
		load	1473	1821	1612	88	42
		domInteractive	1472	1821	1604	90	43

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

background: 2743 bytes
ui: 0 bytes
common: 0 bytes

codecov · 2023-03-29T13:46:25Z

Codecov Report

Merging #18319 (6eda32a) into develop (b5184db) will increase coverage by 0.19%.
The diff coverage is 42.73%.

❗ Current head 6eda32a differs from pull request most recent head a24f983. Consider uploading reports for the commit a24f983 to get more accurate results

@@             Coverage Diff             @@
##           develop   #18319      +/-   ##
===========================================
+ Coverage    64.68%   64.87%   +0.19%     
===========================================
  Files          924      924              
  Lines        35493    35404      -89     
  Branches      9117     9098      -19     
===========================================
+ Hits         22956    22965       +9     
+ Misses       12537    12439      -98

Impacted Files	Coverage Δ
app/scripts/controllers/sign.ts	`92.77% <ø> (ø)`
app/scripts/controllers/encryption-public-key.ts	`38.38% <38.38%> (ø)`
app/scripts/metamask-controller.js	`63.87% <81.82%> (+1.90%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

metamaskbot · 2023-03-30T08:41:39Z

Builds ready [6eda32a]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1546 ± 42 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	91	212	116	25	12
		domContentLoaded	1414	1725	1536	85	41
		load	1414	1725	1546	87	42
		domInteractive	1414	1725	1536	85	41

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

background: 2743 bytes
ui: 0 bytes
common: 0 bytes

app/scripts/controllers/encryption-public-key.ts

socket-security · 2023-04-11T09:37:21Z

New dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No new dependency issues detected in pull request

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore [email protected] bar@* or ignore all packages with @SocketSecurity ignore-all

Pull request alert summary

Issue	Status
Install scripts	✅ 0 issues
Native code	✅ 0 issues
Bin script shell injection	✅ 0 issues
Unresolved require	✅ 0 issues
Invalid package.json	✅ 0 issues
HTTP dependency	✅ 0 issues
Git dependency	✅ 0 issues
Potential typo squat	✅ 0 issues
Known Malware	✅ 0 issues
Telemetry	✅ 0 issues
Protestware/Troll package	✅ 0 issues

📊 Modified Dependency Overview:

⬆️ Updated Package	Version Diff	Added Capability Access	`+/-` Transitive Count	Publisher
@metamask/[email protected]	2.1.0...3.0.0	None	`+0/-0`	metamaskbot

metamaskbot · 2023-04-11T13:11:35Z

Builds ready [f8f9a42]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1561 ± 37 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	86	134	105	11	5
		domContentLoaded	1404	1694	1538	79	38
		load	1467	1722	1561	76	37
		domInteractive	1404	1694	1538	79	38

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

background: 5217 bytes
ui: -44 bytes
common: 0 bytes

OGPoyraz

LGTM

metamaskbot · 2023-04-12T11:59:29Z

Builds ready [e60bfd4]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (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
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1568 ± 38 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	88	137	110	13	6
		domContentLoaded	1408	1715	1542	92	44
		load	1408	1729	1568	78	38
		domInteractive	1408	1715	1542	92	44

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

background: 5217 bytes
ui: -44 bytes
common: 0 bytes

bergarces requested a review from a team as a code owner March 24, 2023 11:57

bergarces requested a review from NiranjanaBinoy March 24, 2023 11:57

bergarces marked this pull request as draft March 24, 2023 12:47

bergarces force-pushed the encryption-public-key-controller branch 7 times, most recently from 9516d7b to d5c99cd Compare March 24, 2023 15:27

bergarces commented Mar 24, 2023

View reviewed changes

app/scripts/controllers/encryption-public-key.ts Show resolved Hide resolved

matthewwalsh0 reviewed Mar 28, 2023

View reviewed changes

app/scripts/controllers/encryption-public-key.ts Outdated Show resolved Hide resolved

matthewwalsh0 reviewed Mar 28, 2023

View reviewed changes

app/scripts/controllers/encryption-public-key.ts Outdated Show resolved Hide resolved

matthewwalsh0 reviewed Mar 28, 2023

View reviewed changes

app/scripts/controllers/encryption-public-key.ts Show resolved Hide resolved

matthewwalsh0 reviewed Mar 28, 2023

View reviewed changes

app/scripts/metamask-controller.js Show resolved Hide resolved

bergarces force-pushed the encryption-public-key-controller branch 3 times, most recently from 54f273d to 397ef22 Compare March 29, 2023 11:09

bergarces commented Mar 29, 2023

View reviewed changes

package.json Outdated Show resolved Hide resolved

bergarces commented Mar 29, 2023

View reviewed changes

.yarn/patches/@metamask-message-manager-npm-2.0.0-f23d924f4f.patch Outdated Show resolved Hide resolved

bergarces commented Mar 29, 2023

View reviewed changes

app/scripts/controllers/sign.ts Show resolved Hide resolved

bergarces force-pushed the encryption-public-key-controller branch from 62af169 to 4fb99cb Compare March 29, 2023 12:36

bergarces marked this pull request as ready for review March 29, 2023 12:36

bergarces added 3 commits March 30, 2023 09:13

add EncryptionPublicKeyController

4ea1774

review comments

6fedacc

review comments

5c9df56

bergarces added 2 commits March 30, 2023 09:13

adds tests

65006b8

patch to test ci

144553f

bergarces force-pushed the encryption-public-key-controller branch from 0206733 to 6eda32a Compare March 30, 2023 08:14

matthewwalsh0 reviewed Mar 30, 2023

View reviewed changes

app/scripts/controllers/encryption-public-key.ts Outdated Show resolved Hide resolved

bergarces force-pushed the encryption-public-key-controller branch 2 times, most recently from 985027e to a24f983 Compare April 3, 2023 10:10

coverage update

6045df0

bergarces force-pushed the encryption-public-key-controller branch from a24f983 to 6045df0 Compare April 3, 2023 10:19

bergarces added 3 commits April 3, 2023 12:43

Merge branch 'develop' into encryption-public-key-controller

5508393

remove duplicate message count from selector

ccc98a2

Merge branch 'develop' into encryption-public-key-controller

50725ae

bergarces force-pushed the encryption-public-key-controller branch from 8726d98 to f1f794f Compare April 11, 2023 12:11

change default origin

86d8ccd

bergarces force-pushed the encryption-public-key-controller branch from f1f794f to 86d8ccd Compare April 11, 2023 12:36

omit securityProviderResponse from type

f8f9a42

bergarces added 2 commits April 12, 2023 08:58

Merge branch 'develop' into encryption-public-key-controller

8eecac3

lint

e60bfd4

OGPoyraz approved these changes Apr 12, 2023

View reviewed changes

matthewwalsh0 approved these changes Apr 12, 2023

View reviewed changes

bergarces merged commit 6ed72d6 into develop Apr 13, 2023

bergarces deleted the encryption-public-key-controller branch April 13, 2023 08:25

github-actions bot locked and limited conversation to collaborators Apr 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor eth_getEncryptionPublicKey handling #18319

Refactor eth_getEncryptionPublicKey handling #18319

bergarces commented Mar 24, 2023 •

edited

Loading

github-actions bot commented Mar 24, 2023

bergarces Mar 24, 2023

metamaskbot commented Mar 29, 2023

codecov bot commented Mar 29, 2023 •

edited

Loading

metamaskbot commented Mar 30, 2023

socket-security bot commented Apr 11, 2023

metamaskbot commented Apr 11, 2023

OGPoyraz left a comment

metamaskbot commented Apr 12, 2023

Refactor eth_getEncryptionPublicKey handling #18319

Refactor eth_getEncryptionPublicKey handling #18319

Conversation

bergarces commented Mar 24, 2023 • edited Loading

Explanation

Manual Testing Steps

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Mar 24, 2023

bergarces Mar 24, 2023

Choose a reason for hiding this comment

metamaskbot commented Mar 29, 2023

codecov bot commented Mar 29, 2023 • edited Loading

Codecov Report

metamaskbot commented Mar 30, 2023

socket-security bot commented Apr 11, 2023

metamaskbot commented Apr 11, 2023

OGPoyraz left a comment

Choose a reason for hiding this comment

metamaskbot commented Apr 12, 2023

bergarces commented Mar 24, 2023 •

edited

Loading

codecov bot commented Mar 29, 2023 •

edited

Loading