-
Notifications
You must be signed in to change notification settings - Fork 985
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
[performance] wallet update #8231
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (18)
|
@hesterbruikman could you check out this PR it should improve your wallet experience problems |
a179bdc
to
a256b3a
Compare
a256b3a
to
531223e
Compare
531223e
to
e5bbc7b
Compare
:rinkeby #{:MOKSHA :KDO} | ||
:xdai #{} | ||
:poa #{}}}}) | ||
:wallet {:visible-tokens {}}}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you elaborate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@flexsurfer I don't understand why we used to force these default tokens on the user, when visible-tokens
is empty the wallet autoconfigures and adds the tokens for which the user has a balance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason to have re-frame events prepended with :TODO.remove
like :TODO.remove/update-estimated-gas
and TODO.remove/update-wallet
?
Not sure is it used somewhere or not.
UPD: nevermind, i see it's being removed in another PR https://github.com/status-im/status-react/pull/8232/files
@dmitryn it is used in some places that are doing things wrong, but that I don't want to touch yet as the scope of these PRs is already big enough. It's to make sure it is obvious that this is not the way to do things |
e5bbc7b
to
a62440e
Compare
`wallet-autoconfig-token` is a very expensive call on mainnet because it checks the balance of every known token. it is called: - when wallet is refreshed by pulling - when user goes on any wallet screen this PR changes that by: - calling it only when the wallet is initialized and there is no visible-token configuration it only calls update-wallet when a new transaction arrives
Summary
a few files are moved around because of circular dependencies, this continues migration of wallet and ethereum code into their own module (previous in
ui.screens.wallet
andutils.ethereums
respectively)wallet-autoconfig-token
is a very expensive call on mainnetbecause it checks the balance of every known token.
it is called:
this PR changes that by:
visible-token configuration
it only calls update-wallet when a new transaction arrives
Testing
I made the following experiment on Android:
satoshi document engage inflict goddess auction rule unfair bid next buddy shy
develop: 2.9mb
PR: 1.9 mb
The difference will be even bigger once persistence is added, the difference is already nice considering the PR pulls both transaction and token transfer history, and does it again everytime it misses a block. With transaction history this number will be very low.
status: ready