refactor: Refactor state classes to prepare for state corruption backup mitigation #29745

danjm · 2025-01-16T11:20:26Z

Description

This PR replaces #24016

This PR does the following:

Refactors local-store.js and network-store.js to ExtensionStore.ts and ReadOnlyNetworkStore.ts. These two classes are extensions of the new BaseStore class.

The new BaseStore class: "is an abstract class designed to be extended by other classes that implement the abstract methods set and get. This class provides the foundation for different storage implementations, enabling them to adhere to a consistent interface for retrieving and setting application state." In future PRs, new classes that use the indexeddb api, and the localStorage api in the offscreen document, will be added that also extend the BaseStore class.

The BaseStore class has a simple interface with just a get and set method. Accordingly, ExtensionSotre and ReadOnlyNetworkStore are significantly simplified compared to their predecessors. The functionality for managing data (e.g. handling error states, handling metadata, etc) that used to be in local-store.js and network-store.js is now in a new PersistanceManager class.

The new PersistanceManager class: "The PersistanceManager class serves as a high-level manager for handling storage-related operations using a local storage system. It provides methods to read and write state, manage metadata, and handle errors or corruption in the underlying storage system."

This is a step towards providing a fail safe for state corruption. The new classes will be utilized for this additional functionality.

Related issues

Fixes:

Manual testing steps

No functional changes should have occurred. Manual regression cases to test include:

make some changes such as a adding a network or changing settings. Close the browser. Re-open the browser and open metamask. The data should be unchanged.
Install an old version. Make some changes. Upgrade to the build generated from this branch. Verify that the state of the wallet, and its data, is as expected.

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 · 2025-01-16T11:20:37Z

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 · 2025-01-20T16:28:18Z

Builds ready [c5528f3]

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, 12, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 13, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1995 ± 141 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	265	2602	1794	542	260
		domContentLoaded	1646	2714	1966	281	135
		load	1662	2778	1995	293	141
		domInteractive	24	108	39	20	9
		backgroundConnect	10	69	30	21	10
		firstReactRender	15	41	22	7	3
		getState	93	172	136	18	9
		initialActions	0	1	0	0	0
		loadScripts	1261	2185	1532	239	115
		setupStore	6	15	9	3	1
		uiStartup	1873	3302	2310	363	174

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

background: 136 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 3.06 KiB (0.04%)

Capture sentry error if there is an empty/corrupt vault, and handle more possible empty/corrupt vault types Ensure log.debug call in case fetch in init in ReadOnlyNetworkStore has an error without a message Ensure that in ReadOnlyNetworkStore waits for initializing to complete Lint fix Ensure that first time state is returned if state has no data Update app/scripts/lib/Stores/ExtensionStore.ts Co-authored-by: Danica Shen <[email protected]> Update test description Use const for DEFAULT_INITIAL_STATE in ExtensionStore.test.ts Improve naming and usage of versionedData variable in background.js lint fix Capture exception with sentry in both error cases ExtensionStore.get Lint fix Add app/scripts/lib/Stores/ReadOnlyNetworkStore.test.ts Delete files that are no longer used Use async/await for get/set in ExtensionStore.ts

app/scripts/lib/Stores/BaseStore.ts

app/scripts/background.js

metamaskbot · 2025-01-20T21:38:46Z

Builds ready [bf23d04]

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

Page Load Metrics (1787 ± 79 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1546	2163	1791	171	82
		domContentLoaded	1530	2097	1764	160	77
		load	1547	2120	1787	164	79
		domInteractive	26	78	40	17	8
		backgroundConnect	6	60	21	15	7
		firstReactRender	16	74	32	20	10
		getState	4	52	19	18	9
		initialActions	0	0	0	0	0
		loadScripts	1109	1620	1302	131	63
		setupStore	6	57	21	20	10
		uiStartup	1758	2672	2059	222	106

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

background: 142 Bytes (0.00%)
ui: -462 Bytes (-0.01%)
common: 3.06 KiB (0.03%)

app/scripts/lib/Stores/BaseStore.ts

app/scripts/lib/Stores/PersistanceManager.ts

app/scripts/lib/Stores/ExtensionStore.ts

app/scripts/lib/Stores/ReadOnlyNetworkStore.ts

Co-authored-by: Mark Stacey <[email protected]>

metamaskbot · 2025-01-23T17:05:53Z

Builds ready [7296ee9]

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

Page Load Metrics (1820 ± 97 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1554	2298	1823	209	100
		domContentLoaded	1546	2205	1794	199	96
		load	1555	2229	1820	203	97
		domInteractive	22	146	49	28	14
		backgroundConnect	8	87	23	18	9
		firstReactRender	15	96	38	25	12
		getState	5	60	21	20	10
		initialActions	0	1	0	0	0
		loadScripts	1078	1735	1346	180	86
		setupStore	6	60	22	20	10
		uiStartup	1817	2525	2089	225	108

Bundle size diffs [🚀 Bundle size reduced!]

background: 44.81 KiB (0.76%)
ui: 1.5 KiB (0.02%)
common: -246.55 KiB (-2.72%)

app/scripts/lib/stores/extension-store.ts

app/scripts/lib/stores/persistence-manager.test.ts

DDDDDanica · 2025-01-27T19:33:53Z

app/scripts/background.js

+});
+
+const localStore = inTest ? new ReadOnlyNetworkStore() : new ExtensionStore();
+const persistanceManager = new PersistanceManager({ localStore });


Typo:

Suggested change

const persistanceManager = new PersistanceManager({ localStore });

const persistenceManager = new PersistenceManager({ localStore });

fixed in acd53a0

app/scripts/lib/stores/persistence-manager.ts

app/scripts/lib/stores/extension-store.test.ts

DDDDDanica · 2025-01-27T21:04:03Z

✅ Tested locally with

Switch network or changing some basic setting. Close the browser. Re-open the browser and open metamask. The data didn't change.

Co-authored-by: Mark Stacey <[email protected]>

metamaskbot · 2025-01-29T18:25:27Z

Builds ready [ee61d60]

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

Page Load Metrics (1707 ± 71 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	235	1974	1562	422	203
		domContentLoaded	1519	2007	1691	146	70
		load	1526	2019	1707	147	71
		domInteractive	25	106	45	23	11
		backgroundConnect	6	62	17	14	7
		firstReactRender	16	99	44	30	14
		getState	5	72	23	23	11
		initialActions	0	1	0	0	0
		loadScripts	1050	1483	1225	128	61
		setupStore	6	83	13	19	9
		uiStartup	1693	2511	2046	253	122

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

background: 177 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 2.04 KiB (0.02%)

metamaskbot · 2025-01-29T19:56:33Z

Builds ready [0b72887]

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

Page Load Metrics (1789 ± 93 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1502	2180	1788	193	93
		domContentLoaded	1460	2170	1753	192	92
		load	1488	2182	1789	194	93
		domInteractive	25	62	39	12	6
		backgroundConnect	10	73	33	18	9
		firstReactRender	15	84	35	21	10
		getState	5	59	19	18	8
		initialActions	0	1	0	0	0
		loadScripts	1085	1655	1309	158	76
		setupStore	5	60	15	17	8
		uiStartup	1771	2510	2057	217	104

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

background: 177 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 2.04 KiB (0.02%)

Gudahtt

LGTM!

metamaskbot · 2025-01-29T21:14:59Z

Builds ready [1aa223e]

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

Page Load Metrics (1898 ± 88 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1592	2242	1898	181	87
		domContentLoaded	1548	2221	1869	183	88
		load	1553	2246	1898	184	88
		domInteractive	26	107	49	26	12
		backgroundConnect	6	99	30	23	11
		firstReactRender	16	100	39	26	13
		getState	5	105	25	26	12
		initialActions	0	1	0	0	0
		loadScripts	1140	1648	1385	154	74
		setupStore	6	82	22	22	11
		uiStartup	1806	2771	2180	243	117

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

background: 177 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 2.04 KiB (0.02%)

DDDDDanica · 2025-01-30T11:31:05Z

LGTM !

metamaskbot added the team-extension-platform label Jan 16, 2025

danjm changed the title ~~Refactor state classes to prepare for state corruption backup mitigation~~ refactor: Refactor state classes to prepare for state corruption backup mitigation Jan 20, 2025

danjm marked this pull request as ready for review January 20, 2025 16:00

gauthierpetetin assigned danjm Jan 20, 2025

brad-decker and others added 4 commits January 20, 2025 16:45

Re-refactor state classes

4421f54

Lint and type error fixes

1ca62ea

Code comment update

3401019

danjm force-pushed the re-refactor-state-classes branch from c5528f3 to 3401019 Compare January 20, 2025 20:15

Gudahtt reviewed Jan 20, 2025

View reviewed changes

app/scripts/lib/Stores/BaseStore.ts Outdated Show resolved Hide resolved

app/scripts/background.js Outdated Show resolved Hide resolved

app/scripts/background.js Show resolved Hide resolved

app/scripts/background.js Outdated Show resolved Hide resolved

danjm added 2 commits January 20, 2025 17:28

Fix storeAlreadyExisted/firstTimeInstalled mistake in onInstall()

e4f03bc

Cleanup

bf23d04

Gudahtt reviewed Jan 21, 2025

View reviewed changes

app/scripts/lib/Stores/ExtensionStore.ts Outdated Show resolved Hide resolved

Gudahtt reviewed Jan 21, 2025

View reviewed changes

danjm and others added 11 commits January 23, 2025 12:56

Update app/scripts/lib/Stores/BaseStore.ts

731fbd2

Co-authored-by: Mark Stacey <[email protected]>

Update app/scripts/lib/Stores/BaseStore.ts

ab230de

Co-authored-by: Mark Stacey <[email protected]>

Update app/scripts/lib/Stores/PersistanceManager.ts

d7621df

Co-authored-by: Mark Stacey <[email protected]>

Update app/scripts/lib/Stores/ExtensionStore.ts

1ed21ce

Co-authored-by: Mark Stacey <[email protected]>

Update app/scripts/lib/Stores/ReadOnlyNetworkStore.ts

7addfa1

Co-authored-by: Mark Stacey <[email protected]>

Update app/scripts/lib/Stores/ExtensionStore.ts

43a36a5

Co-authored-by: Mark Stacey <[email protected]>

Update app/scripts/lib/Stores/ReadOnlyNetworkStore.ts

7296ee9

Co-authored-by: Mark Stacey <[email protected]>

Update names in app/scripts/lib/stores/ directory

9dd5642

Rename localStore -> persistanceManager in background.js

261351e

Stop exporting unused type

c9443ff

Update error handling in extension-store, and some lint fixes

46d664a

danjm added 2 commits January 27, 2025 12:46

Change misleading name and description of intermediary state type

775e5f5

Restore setMetadata method

fc418df

DDDDDanica reviewed Jan 27, 2025

View reviewed changes

app/scripts/lib/stores/extension-store.ts Outdated Show resolved Hide resolved

DDDDDanica reviewed Jan 27, 2025

View reviewed changes

app/scripts/lib/stores/persistence-manager.test.ts Outdated Show resolved Hide resolved

DDDDDanica reviewed Jan 27, 2025

View reviewed changes

app/scripts/lib/stores/persistence-manager.test.ts Outdated Show resolved Hide resolved

DDDDDanica reviewed Jan 27, 2025

View reviewed changes

app/scripts/lib/stores/persistence-manager.ts Outdated Show resolved Hide resolved

Gudahtt reviewed Jan 27, 2025

View reviewed changes

app/scripts/lib/stores/extension-store.test.ts Outdated Show resolved Hide resolved

danjm and others added 7 commits January 28, 2025 11:46

Update app/scripts/lib/stores/extension-store.test.ts

f91397b

Co-authored-by: Mark Stacey <[email protected]>

Correct type import

c5669c9

Improve test

1d4aa02

Move test data to const

b5a4a5d

Correct spelling persistance -> persistence

acd53a0

Lint fix

3236ed2

Fix error

ee61d60

Fix unit tests

0b72887

Lint fix

1aa223e

Gudahtt approved these changes Jan 29, 2025

View reviewed changes

DDDDDanica approved these changes Jan 30, 2025

View reviewed changes

danjm added this pull request to the merge queue Jan 30, 2025

Merged via the queue into main with commit 8534e11 Jan 30, 2025
74 checks passed

danjm deleted the re-refactor-state-classes branch January 30, 2025 15:28

github-actions bot locked and limited conversation to collaborators Jan 30, 2025

metamaskbot added the release-12.12.0 Issue or pull request that will be included in release 12.12.0 label Jan 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Refactor state classes to prepare for state corruption backup mitigation #29745

refactor: Refactor state classes to prepare for state corruption backup mitigation #29745

danjm commented Jan 16, 2025 •

edited

Loading

github-actions bot commented Jan 16, 2025

metamaskbot commented Jan 20, 2025

metamaskbot commented Jan 20, 2025

metamaskbot commented Jan 23, 2025

DDDDDanica Jan 27, 2025

danjm Jan 28, 2025

DDDDDanica commented Jan 27, 2025

metamaskbot commented Jan 29, 2025

metamaskbot commented Jan 29, 2025

Gudahtt left a comment

metamaskbot commented Jan 29, 2025

DDDDDanica commented Jan 30, 2025

	const persistanceManager = new PersistanceManager({ localStore });
	const persistenceManager = new PersistenceManager({ localStore });

refactor: Refactor state classes to prepare for state corruption backup mitigation #29745

refactor: Refactor state classes to prepare for state corruption backup mitigation #29745

Conversation

danjm commented Jan 16, 2025 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Jan 16, 2025

metamaskbot commented Jan 20, 2025

metamaskbot commented Jan 20, 2025

metamaskbot commented Jan 23, 2025

DDDDDanica Jan 27, 2025

Choose a reason for hiding this comment

danjm Jan 28, 2025

Choose a reason for hiding this comment

DDDDDanica commented Jan 27, 2025

metamaskbot commented Jan 29, 2025

metamaskbot commented Jan 29, 2025

Gudahtt left a comment

Choose a reason for hiding this comment

metamaskbot commented Jan 29, 2025

DDDDDanica commented Jan 30, 2025

danjm commented Jan 16, 2025 •

edited

Loading