-
Notifications
You must be signed in to change notification settings - Fork 971
feat(wallet): Apply persistent token/account balance for the rest of the wallet #24072
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
Conversation
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.
Do these stores need WalletUserAssetDataObserver.cachedBalanceRefreshed()
implemented?
9113e22
to
06d8e5d
Compare
ios/brave-ios/Sources/BraveWallet/Crypto/Stores/SelectAccountTokenStore.swift
Outdated
Show resolved
Hide resolved
89a02a2
to
1b31dc5
Compare
[puLL-Merge] - brave/brave-core@24072 DescriptionThis PR updates the logic for fetching and caching token balances across various wallet stores in the Brave iOS app. It introduces the use of ChangesChanges
Possible Issues
Overall, the changes seem reasonable to improve performance by better utilizing cached data. Just need to be diligent with testing and monitoring after the changes to watch out for any balance inconsistencies or caching bugs. |
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.
No need for assignment to transactionsSections (causes uneeded view update), otherwise lgtm.
ios/brave-ios/Sources/BraveWallet/Crypto/Stores/AccountActivityStore.swift
Outdated
Show resolved
Hide resolved
…oes not need balance values.
Resolves brave/brave-browser#36764
brave/brave-browser#36693
Wallet has now cached account/token balance since brave/brave-browser#35986
But only Portfolio/Assets is reading the cached balance.
This PR make the rest of the wallet first checking cached value and will fetch via core api if there is none cached balance.
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
Perform a sanity test to make sure balances are displayed correctly in wallet.
Most of the screens should have faster rendering since we are reading the cached value. But some screens have limited improvement since we are still asynchronously fetching asset prices and NFT metadata.