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

Filter tokens logs from current block, not genesis #6128

Merged
merged 2 commits into from Jul 25, 2017

Conversation

ngotchac
Copy link
Contributor

(note that filters are only used to trigger a balance update and notifications)

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Jul 24, 2017
.then(() => newFilters)
Promise
.all([
api.eth.blockNumber()
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to retrieve the blockNumber again when we are already triggering this because of a blockNumber update?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this doesn't get triggered on new block, only when the filters need to be updated, which is when you need to update the tokens value of the displayed addresses, ie. when switching views between your account, your address book, contracts, etc. We don't update the tokens balances for accounts we don't show, except those we own (so we can get a notification)

@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 24, 2017
@gavofyork gavofyork merged commit 58fec91 into master Jul 25, 2017
@gavofyork gavofyork deleted the ng-fix-tokens-logs branch July 25, 2017 06:39
arkpar pushed a commit that referenced this pull request Jul 25, 2017
* Filter tokens logs from current block, not genesis

* Fix linting
arkpar added a commit that referenced this pull request Jul 25, 2017
* Filter tokens logs from current block, not genesis

* Fix linting
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.

None yet

3 participants