Use `addNewAccount` from core `KeyringController` #19814

mikesposito · 2023-06-29T08:31:34Z

Explanation

This PR uses addNewAccount from core KeyringController instead of calling EthKeyringController directly. As addNewAccount from KeyringController does not automatically select the added account, it is now selected by the client after the account creation.

Manual Testing Steps

Adding a new HD account should work as before.
When adding a new account, the created account should be selected as before.

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-06-29T08:31: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 · 2023-06-29T08:57:51Z

Builds ready [cf0fb2a]

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

Page Load Metrics (1585 ± 39 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	100	139	118	11	5
		domContentLoaded	1452	1704	1558	75	36
		load	1453	1719	1585	81	39
		domInteractive	1452	1704	1557	75	36

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

background: 156569 bytes
ui: -6418 bytes
common: 6356 bytes

codecov · 2023-06-29T08:58:07Z

Codecov Report

Merging #19814 (614a366) into develop (6512cac) will decrease coverage by 0.04%.
The diff coverage is 80.00%.

❗ Current head 614a366 differs from pull request most recent head 275037f. Consider uploading reports for the commit 275037f to get more accurate results

@@             Coverage Diff             @@
##           develop   #19814      +/-   ##
===========================================
- Coverage    68.70%   68.66%   -0.04%     
===========================================
  Files          990      991       +1     
  Lines        38289    38273      -16     
  Branches     10262    10258       -4     
===========================================
- Hits         26304    26278      -26     
- Misses       11985    11995      +10

Files Changed	Coverage Δ
app/scripts/metamask-controller.js	`64.55% <66.67%> (-0.99%)`	⬇️
ui/store/actions.ts	`43.42% <100.00%> (-0.04%)`	⬇️

... and 9 files with indirect coverage changes

mikesposito · 2023-07-19T11:26:33Z

Depends on #19659

mikesposito · 2023-07-24T12:00:16Z

Fitness Functions CI is failing on this rule:

Checking rule "Don't use sinonorassert in unit tests"

It should be a false positive as we are not adding new test cases

metamaskbot · 2023-07-24T22:59:29Z

Builds ready [722c683]

builds: chrome, firefox
builds (beta): chrome
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, 5
- 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, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1502 ± 36 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	105	167	125	17	8
		domContentLoaded	1360	1648	1502	76	36
		load	1360	1648	1502	76	36
		domInteractive	1360	1648	1502	76	36

Bundle size diffs [🚀 Bundle size reduced!]

background: -498 Bytes (-0.01%)
ui: -62 Bytes (-0.00%)
common: 0 Bytes (0.00%)

metamaskbot · 2023-07-25T13:00:57Z

Builds ready [f271edb]

builds: chrome, firefox
builds (beta): chrome
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, 5
- 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, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1770 ± 100 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	107	288	157	41	20
		domContentLoaded	1484	2114	1770	207	99
		load	1484	2114	1770	207	100
		domInteractive	1484	2114	1770	207	99

Bundle size diffs [🚀 Bundle size reduced!]

background: -448 Bytes (-0.01%)
ui: -62 Bytes (-0.00%)
common: 0 Bytes (0.00%)

app/scripts/metamask-controller.js

metamaskbot · 2023-07-31T13:57:18Z

Builds ready [2abdb29]

builds: chrome, firefox
builds (beta): chrome
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, 5
- 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, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1735 ± 64 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	126	198	160	21	10
		domContentLoaded	1538	1977	1735	134	64
		load	1538	1978	1735	134	64
		domInteractive	1538	1977	1735	134	64

Bundle size diffs [🚀 Bundle size reduced!]

background: -383 Bytes (-0.01%)
ui: -62 Bytes (-0.00%)
common: 0 Bytes (0.00%)

metamaskbot · 2023-07-31T15:09:25Z

Builds ready [a95471b]

builds: chrome, firefox
builds (beta): chrome
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, 5
- 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, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1667 ± 50 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	122	196	145	20	10
		domContentLoaded	1531	1962	1666	105	50
		load	1531	1962	1667	104	50
		domInteractive	1531	1962	1666	105	50

mcmire

Looks good!

mcmire · 2023-07-31T22:23:31Z

Oh, looks like conflicts :(

Now that I've commented, I can keep an eye on this one better.

metamaskbot · 2023-08-03T08:59:23Z

Builds ready [81e752f]

builds: chrome, firefox
builds (beta): chrome
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, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1496 ± 43 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	102	173	125	18	9
		domContentLoaded	1411	1733	1496	90	43
		load	1411	1733	1496	90	43
		domInteractive	1411	1733	1496	90	43

Bundle size diffs [🚀 Bundle size reduced!]

background: -387 Bytes (-0.01%)
ui: -62 Bytes (-0.00%)
common: 0 Bytes (0.00%)

metamaskbot · 2023-08-14T08:48:46Z

Builds ready [0de0244]

builds: chrome, firefox
builds (beta): chrome
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, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1663 ± 65 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	107	214	148	24	12
		domContentLoaded	1440	2050	1662	136	65
		load	1458	2051	1663	134	65
		domInteractive	1440	2050	1662	136	65

Bundle size diffs [🚀 Bundle size reduced!]

background: -387 Bytes (-0.01%)
ui: -62 Bytes (-0.00%)
common: 0 Bytes (0.00%)

cryptodev-2s

LGTM!

metamaskbot · 2023-08-14T15:23:39Z

Builds ready [722b37f]

builds: chrome, firefox
builds (beta): chrome
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, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1640 ± 58 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	107	162	138	15	7
		domContentLoaded	1377	1914	1639	119	57
		load	1377	1915	1640	120	58
		domInteractive	1377	1914	1639	119	57

Bundle size diffs [🚀 Bundle size reduced!]

background: -387 Bytes (-0.01%)
ui: -62 Bytes (-0.00%)
common: 0 Bytes (0.00%)

Gudahtt

LGTM!

metamaskbot · 2023-08-15T20:25:45Z

Builds ready [614a366]

builds: chrome, firefox
builds (beta): chrome
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, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1557 ± 53 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	110	195	138	23	11
		domContentLoaded	1406	1788	1557	111	53
		load	1406	1788	1557	111	53
		domInteractive	1406	1788	1557	111	53

Bundle size diffs [🚀 Bundle size reduced!]

background: -419 Bytes (-0.01%)
ui: -62 Bytes (-0.00%)
common: 0 Bytes (0.00%)

metamaskbot · 2023-08-16T09:28:49Z

Builds ready [275037f]

builds: chrome, firefox
builds (beta): chrome
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, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1469 ± 43 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	106	176	127	18	9
		domContentLoaded	1330	1737	1469	90	43
		load	1330	1737	1469	90	43
		domInteractive	1330	1737	1469	90	43

Bundle size diffs [🚀 Bundle size reduced!]

background: -419 Bytes (-0.01%)
ui: -62 Bytes (-0.00%)
common: 0 Bytes (0.00%)

mikesposito force-pushed the refactor/use-core-keyring-controller branch 5 times, most recently from c65c6b9 to 2d584d7 Compare July 19, 2023 09:37

mikesposito force-pushed the refactor/use-core-keyring-controller branch 2 times, most recently from 95313e3 to 1567ae0 Compare July 24, 2023 10:32

mikesposito force-pushed the refactor/use-addnewaccount-from-core branch from cf0fb2a to fe58431 Compare July 24, 2023 11:54

Base automatically changed from refactor/use-core-keyring-controller to develop July 24, 2023 18:44

mikesposito force-pushed the refactor/use-addnewaccount-from-core branch from fe58431 to 722c683 Compare July 24, 2023 22:24

mikesposito marked this pull request as ready for review July 24, 2023 22:51

mikesposito requested a review from a team as a code owner July 24, 2023 22:51

Gudahtt reviewed Jul 26, 2023

View reviewed changes

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

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

mikesposito force-pushed the refactor/use-addnewaccount-from-core branch 3 times, most recently from 500075f to 58686ae Compare July 31, 2023 09:11

mikesposito force-pushed the refactor/use-addnewaccount-from-core branch from 2abdb29 to a95471b Compare July 31, 2023 14:30

mcmire previously approved these changes Jul 31, 2023

View reviewed changes

mikesposito dismissed mcmire’s stale review via 9529284 August 1, 2023 11:02

mikesposito force-pushed the refactor/use-addnewaccount-from-core branch from a95471b to 9529284 Compare August 1, 2023 11:02

mcmire previously approved these changes Aug 1, 2023

View reviewed changes

mikesposito force-pushed the refactor/use-addnewaccount-from-core branch from 9529284 to 28a260c Compare August 2, 2023 11:57

mikesposito added the team-wallet-framework label Aug 2, 2023

mikesposito force-pushed the refactor/use-addnewaccount-from-core branch from 28a260c to 81e752f Compare August 3, 2023 08:18

mikesposito requested a review from cryptodev-2s August 3, 2023 08:19

mikesposito force-pushed the refactor/use-addnewaccount-from-core branch from 81e752f to 0de0244 Compare August 14, 2023 08:08

cryptodev-2s previously approved these changes Aug 14, 2023

View reviewed changes

mikesposito force-pushed the refactor/use-addnewaccount-from-core branch from 0de0244 to 722b37f Compare August 14, 2023 14:44

mikesposito dismissed stale reviews from cryptodev-2s and mcmire via a93cdf7 August 14, 2023 15:28

mikesposito force-pushed the refactor/use-addnewaccount-from-core branch from 722b37f to a93cdf7 Compare August 14, 2023 15:28

mcmire approved these changes Aug 14, 2023

View reviewed changes

Gudahtt force-pushed the refactor/use-addnewaccount-from-core branch from a93cdf7 to 614a366 Compare August 15, 2023 19:31

Gudahtt approved these changes Aug 15, 2023

View reviewed changes

mikesposito added 4 commits August 16, 2023 10:15

refactor: use addNewAccount from core KeyringController

ea80db9

refactor: replace missed interaction

c0a3564

refactor: select account only when is new

6a11f2e

refactor: use getAccounts to check if account is new

275037f

mikesposito force-pushed the refactor/use-addnewaccount-from-core branch from 614a366 to 275037f Compare August 16, 2023 08:16

mikesposito merged commit 42d05ef into develop Aug 16, 2023

mikesposito deleted the refactor/use-addnewaccount-from-core branch August 16, 2023 09:19

github-actions bot locked and limited conversation to collaborators Aug 16, 2023

metamaskbot added the release-10.36.0 Issue or pull request that will be included in release 10.36.0 label Aug 16, 2023

Gudahtt added release-11.1.0 Issue or pull request that will be included in release 11.1.0 and removed release-10.36.0 Issue or pull request that will be included in release 10.36.0 labels Sep 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `addNewAccount` from core `KeyringController` #19814

Use `addNewAccount` from core `KeyringController` #19814

mikesposito commented Jun 29, 2023 •

edited

Loading

github-actions bot commented Jun 29, 2023

metamaskbot commented Jun 29, 2023

codecov bot commented Jun 29, 2023 •

edited

Loading

mikesposito commented Jul 19, 2023

mikesposito commented Jul 24, 2023

metamaskbot commented Jul 24, 2023

metamaskbot commented Jul 25, 2023

metamaskbot commented Jul 31, 2023

metamaskbot commented Jul 31, 2023

mcmire left a comment

mcmire commented Jul 31, 2023

metamaskbot commented Aug 3, 2023

metamaskbot commented Aug 14, 2023

cryptodev-2s left a comment

metamaskbot commented Aug 14, 2023

Gudahtt left a comment

metamaskbot commented Aug 15, 2023

metamaskbot commented Aug 16, 2023

Use addNewAccount from core KeyringController #19814

Use addNewAccount from core KeyringController #19814

Conversation

mikesposito commented Jun 29, 2023 • edited Loading

Explanation

Manual Testing Steps

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Jun 29, 2023

metamaskbot commented Jun 29, 2023

codecov bot commented Jun 29, 2023 • edited Loading

Codecov Report

mikesposito commented Jul 19, 2023

mikesposito commented Jul 24, 2023

metamaskbot commented Jul 24, 2023

metamaskbot commented Jul 25, 2023

metamaskbot commented Jul 31, 2023

metamaskbot commented Jul 31, 2023

mcmire left a comment

Choose a reason for hiding this comment

mcmire commented Jul 31, 2023

metamaskbot commented Aug 3, 2023

metamaskbot commented Aug 14, 2023

cryptodev-2s left a comment

Choose a reason for hiding this comment

metamaskbot commented Aug 14, 2023

Gudahtt left a comment

Choose a reason for hiding this comment

metamaskbot commented Aug 15, 2023

metamaskbot commented Aug 16, 2023

Use `addNewAccount` from core `KeyringController` #19814

Use `addNewAccount` from core `KeyringController` #19814

mikesposito commented Jun 29, 2023 •

edited

Loading

codecov bot commented Jun 29, 2023 •

edited

Loading