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

Save recent network balances in local storage #5843

Merged
merged 4 commits into from
Nov 30, 2018

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Nov 27, 2018

This PR prevents errors caused by null account balances. This problem is described in detail #5842

While we need to resolve the root cause of that as soon as possible, this PR provides a front end band-aid by saving the most non-null balance for each account in local storage. If the account balance is null when we select it from state, we will instead return the most recent cached value.

These null returns are very inconsistent and almost never happen two blocks in a row, so the account balance will (almost always) be at most one block out of date.

@danjm danjm force-pushed the cache-network-balances-local-storage branch 3 times, most recently from d82e5a8 to 929bd56 Compare November 28, 2018 15:12
@metamaskbot
Copy link
Collaborator

Builds ready [929bd56]: mascara, chrome, firefox, edge, opera

@danjm danjm force-pushed the cache-network-balances-local-storage branch from 929bd56 to 6b0500b Compare November 29, 2018 13:42
@danjm danjm force-pushed the cache-network-balances-local-storage branch from 6b0500b to cf2db0f Compare November 29, 2018 16:03
@danjm danjm force-pushed the cache-network-balances-local-storage branch from cf2db0f to f4cc41c Compare November 29, 2018 19:55
@metamaskbot
Copy link
Collaborator

Builds ready [f4cc41c]: mascara, chrome, firefox, edge, opera

@danjm danjm force-pushed the cache-network-balances-local-storage branch from f4cc41c to cd179e8 Compare November 30, 2018 02:33
@danjm danjm force-pushed the cache-network-balances-local-storage branch from cd179e8 to efa04db Compare November 30, 2018 03:16
@metamaskbot
Copy link
Collaborator

Builds ready [efa04db]: mascara, chrome, firefox, edge, opera

Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few quick suggestions/comments

app/scripts/controllers/cached-balances.js Outdated Show resolved Hide resolved
app/scripts/controllers/cached-balances.js Outdated Show resolved Hide resolved
app/scripts/controllers/cached-balances.js Outdated Show resolved Hide resolved
app/scripts/controllers/cached-balances.js Outdated Show resolved Hide resolved
app/scripts/controllers/cached-balances.js Outdated Show resolved Hide resolved
@metamaskbot
Copy link
Collaborator

Builds ready [c132f89]: mascara, chrome, firefox, edge, opera

@danfinlay danfinlay merged commit 4c24555 into develop Nov 30, 2018
@danjm danjm deleted the cache-network-balances-local-storage branch March 26, 2019 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants