Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make our background storage more size resilient #3076

Closed
4 of 5 tasks
danfinlay opened this issue Jan 23, 2018 · 11 comments
Closed
4 of 5 tasks

Make our background storage more size resilient #3076

danfinlay opened this issue Jan 23, 2018 · 11 comments

Comments

@danfinlay
Copy link
Contributor

danfinlay commented Jan 23, 2018

Related to #3071

Meta-issue to address #3071.

We now know that storage is overflowing for some users, and this may be causing vault loss.

We need to develop strategies to improve our storage size resilience, and implement them.

  • Study what happens when background localstorage cannot write anymore. Is the storage instantly lost? Is there an opportunity to backup/recover?
  • Study which portions of our persisted state take the most size (probably transactions).
  • Review extension storage limits.
  • Research other alternative storage strategies.
  • Move our localStorage use to the better solution (storage.local).
@danfinlay
Copy link
Contributor Author

Study what happens when background localstorage cannot write anymore. Is the storage instantly lost? Is there an opportunity to backup/recover?

While the write attempt fails, the storage seems to persist, suggesting this issue may not be causing any data loss.

@danfinlay
Copy link
Contributor Author

Study which portions of our persisted state take the most size

  • RecentBlocks (doesn't need to be persisted, should be removed from storage)
  • BlacklistController is getting pretty big, could use own key.
  • TransactionController

@danfinlay
Copy link
Contributor Author

Review extension storage limits.

Looks like the clear correct path is storage.local.

From the MDN documentation:

Although this API is similar to Window.localStorage it is recommended that you don't use Window.localStorage in the extension code to store extension-related data. Firefox will clear data stored by extensions using the localStorage API in various scenarios where users clear their browsing history and data for privacy reasons, while data saved using the storage.local API will be correctly persisted in these scenarios.

The storage.local API could allow for unlimited storage.

@danfinlay
Copy link
Contributor Author

storage.local API originally defined by chromium team in these docs.

@danfinlay
Copy link
Contributor Author

Previous example of setting up MetaMask to use an async storage api here.

@danfinlay
Copy link
Contributor Author

Turns out much of the work to use the extension.storage API properly was already done by @heyellieday in this PR, I've forked her work to leverage this for an easier fix. (Forget that other bounty, she gets a bounty for this).

@danfinlay
Copy link
Contributor Author

danfinlay commented Jan 24, 2018

Began the work converting to extension.storage in this branch.

It boots up fine, but doesn't seem to switch over to relying on this strategy yet (I tested by trashing the localStorage randomly and restarting after some persistance, and it reverted to first-time state, which it shouldn't do if it's falling back to a new strategy).

@kumavis
Copy link
Member

kumavis commented Jan 24, 2018

@danfinlay
Copy link
Contributor Author

@kumavis that particular sentry error refers to our send-form's persistence, not background, so it probably deserves its own issue.

@danfinlay
Copy link
Contributor Author

Added to that branch:

  • No longer writes to localStorage.
  • Fixed issue where quantity of write attempts overwhelmed our pump to storage.local stream.

@danfinlay
Copy link
Contributor Author

Fixes #3071

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants