Move state management from storage.local to IndexedDB

danjm commented 3 months ago

What is this about?

Currently we use https://developer.chrome.com/docs/extensions/reference/api/storage#property-local for persisting state.

We believe that https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API is more reliable and performant.

We should switch.

We likely will need to implement a class to wrap the IndexedDB api, and then we should be able to swap the use of storage.local with the use of that class.

Scenario

No response

Design

No response

Technical Details

Technical refinement:

Context

There are 2 store in codebase based on build env: ReadOnlyNetworkStore for testing and LocalStore for normal build, which means we.

ReadOnlyNetworkStore: This store retrieves its state from a network source (in this case, a server serving a JSON file at http://localhost:12345/state.json). It does not persist the state in a traditional storage medium like local storage. It's a read-only store meant for loading data from the network.

ExtensionStore: This store uses the browser's local storage API (specifically browser.storage.local) to persist data across browser sessions. It handles reading and writing the state in a persistent manner, and state will be retained even after the extension or browser is closed.

Reference In blockaid to save / retrieve / delete filed and validate using checksum https://github.com/MetaMask/metamask-extension/blob/develop/app/scripts/lib/ppom/indexed-db-backend.ts

Ticket Breakdown

For ExtensionStore:

Starting from _init to initialize database and createObjectStore.
Then prepare _getObjectStore to conduct actions to opended indexedDB
Migrate set to indexDB _writeToDB way, which uses TransactionMode.READ_WRITE to access _getObjectStore
Migrate get to indexDB _readFromDB way, which uses TransactionMode.READ_ONLY to access _getObjectStore
Keep the rest of helper functions and setMetadata
update unit tests

For ReadOnlyNetworkStore:

Just refactor to typescript, it is not using localStorage

For e2e tests: Write e2e test to validate the state, which covers

log in and validate the state
disable and enable the extension to validate the state

Documentation Could use this script to index the data from console

    indexedDB.open('ExtensionStore').onsuccess = function (event) {
      const db = event.target.result;
      const transaction = db.transaction('ExtensionStore', 'readonly');
      const objectStore = transaction.objectStore('ExtensionStore');
      objectStore.getAll().onsuccess = function (e) {
        console.log(e.target.result);
      };
    };

Threat Modeling Framework

No response

Acceptance Criteria

We need to maintain the same behavior of localStorage API

Onboard, chrome.storage.local.get(console.log) can find storage
Clear cache and hard reload, state is back
Toggle off and on extension, state is back
Delete storage by chrome.storage.local.clear(function() { console.log('All items have been removed from chrome.storage.local.'}); and refresh, state is back;
Delete storage by chrome.storage.local.clear, toggle extension, it brings us back to to onboarding page

Stakeholder review needed before the work gets merged

[ ] Engineering (needed in most cases)
[ ] Design
[ ] Product
[ ] QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
[ ] Security
[ ] Legal
[ ] Marketing
[ ] Management (please specify)
[ ] Other (please specify)

References

No response

gauthierpetetin commented 2 months ago

Comment from @seaona during weekly review: we are already using IndexedDB in some parts of the codebase (Blockaid feature) and existing implementation could be used as a reference.

davidmurdoch commented 2 months ago

Wouldn't IndexedDB storage be cleared if a use clears their browser storage? Also need to be aware if works in Incognito?

gauthierpetetin commented 2 months ago

Wouldn't IndexedDB storage be cleared if a use clears their browser storage?

Yes, but that's also the case with storage.local , no?

Also need to be aware if works in Incognito?

IndexedDB is supposed to work in Incognito mode for most browsers, but there seems to be issues for Firefox (based on this article or this article)

gauthierpetetin commented 1 week ago

Closing this issue, as we'll end up including this work in the vault recovery flow work: https://github.com/MetaMask/metamask-extension/pull/23056

MetaMask / metamask-extension