Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Smarter balance fetching #3605

Merged
merged 25 commits into from
Nov 25, 2016
Merged

Smarter balance fetching #3605

merged 25 commits into from
Nov 25, 2016

Conversation

ngotchac
Copy link
Contributor

Closes #3590

This might also (hopefully) close #3240

Fetching balances is done smarter, in several ways:

  • Only fetch balances of currently visible accounts
  • Fetch token balances with only 2 calls via eth Filters (2 because of FROM and TO) and the Transfer event

It is likely that every token used will implement the correct Transfer event. Some implementations might differ, or for example when buying some GavCoin (tokens created). Thus all the balances are fetched every 2 minutes, just to be sure.

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Nov 25, 2016
@@ -51,15 +51,30 @@ export function inHash (hash) {
return inHex(hash);
}

export function pad (input, length) {
export function padRight (input, length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move to /api/util/format.js? (pad{left,Right})

// Fetch all tokens every 2 minutes
this.throttledTokensFetch = throttle(
this.fetchTokens,
2 * 60 * 1000,
Copy link
Contributor

Choose a reason for hiding this comment

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

Start at 60s

format: format.toString(),
id: tokenId,
image: hashToImageUrl(image),

Copy link
Contributor

Choose a reason for hiding this comment

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

extra line to be removed?


const initialState = {
balances: {},
tokens: {}
tokens: {},

Copy link
Contributor

Choose a reason for hiding this comment

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

extra line to be removed?

}

const balance = Object.assign({}, balances[address]);

Copy link
Contributor

Choose a reason for hiding this comment

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

extra blank (and 2 down as well, doesn't add much in terms of readability)

}

if (this._manifests[manifestHash]) {
return this._manifests[manifestHash];
Copy link
Contributor

Choose a reason for hiding this comment

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

Promise.resolve?

@ngotchac
Copy link
Contributor Author

@jacogr Should be ok now :)

@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 25, 2016
@jacogr jacogr merged commit 08c507d into master Nov 25, 2016
@jacogr jacogr deleted the ng-balance-fetch branch November 25, 2016 15:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make balance fetching context specific UI causes Parity to consume too much CPU
2 participants