MMI-2119-env-name-as-key by bergarces · Pull Request #22546 · MetaMask/metamask-extension

bergarces · 2024-01-16T11:37:40Z

Description

Update MMI package dependencies and refactor code to handle the breaking changes.

The changes involve using environment name as the main key to identify a custodian, instead of the api url.

It should not change any existing feature

Related issues

Fixes: MMI-2119

Manual testing steps

Test MMI build with multiple custodians, including:

Updating an existing installation and verifying that existing custodian accounts have been migrated correctly
Adding accounts from a custodian
Refreshing tokens for custodian accounts

Screenshots/Recordings

Before

After

Pre-merge author checklist

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-01-16T11:38:08Z

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.

socket-security · 2024-01-16T16:45:55Z

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

👍 No dependency changes detected in pull request

codecov · 2024-01-18T16:16:01Z

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (391e9e7) 68.55% compared to head (44c5663) 68.54%.

Files	Patch %	Lines
app/scripts/controllers/mmi-controller.js	0.00%	5 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #22546      +/-   ##
===========================================
- Coverage    68.55%   68.54%   -0.00%     
===========================================
  Files         1088     1088              
  Lines        42907    42894      -13     
  Branches     11415    11414       -1     
===========================================
- Hits         29411    29400      -11     
+ Misses       13496    13494       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jacob121532 · 2024-01-18T17:46:39Z

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (2f0f141) 68.15% compared to head (5fbb8af) 68.15%.
Report is 16 commits behind head on develop.

Files Patch % Lines

app/scripts/controllers/mmi-controller.js 0.00% 7 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #22546      +/-   ##
===========================================
- Coverage    68.15%   68.15%   -0.00%     
===========================================
  Files         1083     1083              
  Lines        42495    42492       -3     
  Branches     11337    11336       -1     
===========================================
- Hits         28961    28958       -3     
  Misses       13534    13534              
☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

app/scripts/controllers/mmi-controller.js

metamaskbot · 2024-01-24T10:27:24Z

Builds ready [ce84a43]

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
- common: 0, 1, 2, 3, 4, 5, 6, 7
- 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 (820 ± 40 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	82	165	116	17	8
		domContentLoaded	9	48	22	11	5
		load	722	1027	820	84	40
		domInteractive	9	48	22	11	5

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

zone-live

Looks good code wise, I'll create some follow up tickets for the @todos you found, and run the extension with this branch to do some manual testing as well.
Awesome work! 🥇

zone-live · 2024-02-02T15:49:25Z

app/scripts/controllers/mmi-controller.js

-    const { token, apiUrl } = req.params;
+    const { token, envName } = req.params;
+    // TODO (Bernardo) - Check if this is the case
    const custodyType = 'Custody - JSONRPC'; // Only JSONRPC is supported for now


Should be yes, this will need to be updated and a ticket created for it. Ty! 👍🏼

zone-live · 2024-02-02T15:53:50Z

ui/helpers/utils/institutional/find-by-custodian-name.ts

  version: number;
 };

+// TODO (Bernardo) - There can be multiple custodian with the same name, envName should be used instead


True! Will create a ticket for it as well 👍🏼

zone-live · 2024-02-02T15:59:17Z

ui/pages/institutional/interactive-replacement-token-page/interactive-replacement-token-page.js

-  const connectRequests = useSelector(
-    (state) => state.metamask.institutionalFeatures?.connectRequests,
-  );
+  const connectRequests = useSelector(getInstitutionalConnectRequests);


Ty! 💪🏼

metamaskbot · 2024-02-06T09:40:51Z

Builds ready [b3a7935]

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
- common: 0, 1, 2, 3, 4, 5, 6, 7
- 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 (808 ± 41 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	97	149	111	13	6
		domContentLoaded	9	33	15	7	3
		load	715	1051	808	86	41
		domInteractive	9	33	15	7	3

Bundle size diffs [🚀 Bundle size reduced!]

background: 0 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: -37 Bytes (-0.00%)

metamaskbot · 2024-02-06T11:59:26Z

Builds ready [802f282]

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
- common: 0, 1, 2, 3, 4, 5, 6, 7
- 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 (741 ± 12 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	83	132	107	15	7
		domContentLoaded	9	19	14	4	2
		load	712	812	741	25	12
		domInteractive	9	19	14	3	2

Bundle size diffs [🚀 Bundle size reduced!]

background: 0 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: -37 Bytes (-0.00%)

zone-live

After some manual testing using Qredo, it seems we have a bug. This error happens when connecting MMI to Qredo and trying to import accounts.

Should be related with: https://github.com/consensys-vertical-apps/metamask-institutional/blob/main/packages/custodyKeyring/src/CustodyKeyring.ts#L526

…s custody type, example: Qredo

metamaskbot · 2024-02-08T15:31:41Z

Builds ready [796f42a]

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
- common: 0, 1, 2, 3, 4, 5, 6, 7
- 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 (799 ± 26 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	93	144	111	13	6
		domContentLoaded	9	21	15	4	2
		load	743	979	799	54	26
		domInteractive	9	21	15	4	2

Bundle size diffs [🚀 Bundle size reduced!]

background: 0 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: -37 Bytes (-0.00%)

metamaskbot · 2024-02-08T17:15:53Z

Builds ready [f5c77e9]

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
- common: 0, 1, 2, 3, 4, 5, 6, 7
- 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 (933 ± 43 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	96	196	131	25	12
		domContentLoaded	12	88	31	20	10
		load	787	1110	933	89	43
		domInteractive	12	88	31	20	10

Bundle size diffs [🚀 Bundle size reduced!]

background: 0 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: -37 Bytes (-0.00%)

…sk-extension into MMI-2119-env-name-as-key

metamaskbot · 2024-02-14T10:28:38Z

Builds ready [44c5663]

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

Page Load Metrics (1055 ± 55 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	118	271	197	36	17
		domContentLoaded	9	90	37	27	13
		load	906	1453	1055	115	55
		domInteractive	9	90	37	27	13

Bundle size diffs [🚀 Bundle size reduced!]

background: 0 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: -37 Bytes (-0.00%)

metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jan 16, 2024

bergarces added the mmi DEPRECATED: The product got sunset label Jan 16, 2024

metamaskbot removed the INVALID-PR-TEMPLATE PR's body doesn't match template label Jan 16, 2024

bergarces force-pushed the MMI-2119-env-name-as-key branch from 3892021 to a1b75d6 Compare January 16, 2024 16:39

bergarces added 14 commits January 18, 2024 20:56

refactor: use environment instead of apiUrl

b2b8afa

chore: patch to run tests

3c70d3c

feat: progress

824fe7e

fix: minor test changes

31dc71e

fix: lock file

9a8ede5

fix: change parameter name

af09368

missing package patch

e261983

fix: progress

7fd2a83

fix: patch updates

9776bb0

fix: add migration

98a9b92

fix: add migration

d2066c6

fix: patch

4e2b1e8

fix: more patch changes

cec0a96

fix: rebased changes

3ff297d

bergarces force-pushed the MMI-2119-env-name-as-key branch from fd8d835 to 3ff297d Compare January 18, 2024 20:58

bergarces added 2 commits January 18, 2024 21:33

fix: revert migration

18d0a34

fix: missing lock file

6546fc8

bergarces mentioned this pull request Jan 18, 2024

refactor: replace apiUrl by envName consensys-vertical-apps/metamask-institutional#600

Merged

8 tasks

Merge branch 'develop' into MMI-2119-env-name-as-key

6526753

bergarces commented Jan 19, 2024

View reviewed changes

app/scripts/controllers/mmi-controller.js Outdated Show resolved Hide resolved

shane-t self-requested a review January 31, 2024 12:21

shane-t previously approved these changes Jan 31, 2024

View reviewed changes

zone-live previously approved these changes Feb 2, 2024

View reviewed changes

update view to remove api-url input text, as we dont need it anymore

bc23ace

zone-live dismissed stale reviews from shane-t and themself via bc23ace February 5, 2024 16:54

zone-live added 2 commits February 5, 2024 17:09

removes unused string and unit test

1fdf5c5

removes unused string

b3a7935

Merge branch 'develop' into MMI-2119-env-name-as-key

802f282

zone-live approved these changes Feb 6, 2024

View reviewed changes

zone-live requested changes Feb 6, 2024

View reviewed changes

zone-live added 2 commits February 8, 2024 14:34

adds fallback for when custodian doesn't have a env name, will use it…

5988962

…s custody type, example: Qredo

lint fix

796f42a

Merge branch 'develop' into MMI-2119-env-name-as-key

f5c77e9

zone-live and others added 3 commits February 14, 2024 10:00

Merge branch 'develop' into MMI-2119-env-name-as-key

0bdc58b

packages update

7d55e07

Merge branch 'MMI-2119-env-name-as-key' of github.com:MetaMask/metama…

44c5663

…sk-extension into MMI-2119-env-name-as-key

zone-live approved these changes Feb 14, 2024

View reviewed changes

zone-live requested a review from shane-t February 14, 2024 10:50

shane-t approved these changes Feb 14, 2024

View reviewed changes

zone-live merged commit 38fcd39 into develop Feb 14, 2024

zone-live deleted the MMI-2119-env-name-as-key branch February 14, 2024 13:58

github-actions bot locked and limited conversation to collaborators Feb 14, 2024

metamaskbot added the release-11.12.0 Issue or pull request that will be included in release 11.12.0 label Feb 14, 2024

Conversation

bergarces commented Jan 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Jan 16, 2024

Uh oh!

socket-security bot commented Jan 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jacob121532 commented Jan 18, 2024

Codecov Report

Uh oh!

Uh oh!

metamaskbot commented Jan 24, 2024

Uh oh!

zone-live left a comment

Choose a reason for hiding this comment

Uh oh!

zone-live Feb 2, 2024

Choose a reason for hiding this comment

Uh oh!

zone-live Feb 2, 2024

Choose a reason for hiding this comment

Uh oh!

zone-live Feb 2, 2024

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Feb 6, 2024

Uh oh!

metamaskbot commented Feb 6, 2024

Uh oh!

zone-live left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Feb 8, 2024

Uh oh!

metamaskbot commented Feb 8, 2024

Uh oh!

metamaskbot commented Feb 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bergarces commented Jan 16, 2024 •

edited

Loading

socket-security bot commented Jan 16, 2024 •

edited

Loading

codecov bot commented Jan 18, 2024 •

edited

Loading

zone-live left a comment •

edited

Loading