-
-
Notifications
You must be signed in to change notification settings - Fork 252
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
feat(suite-common): fetch stake data based on device account symbols #15460
base: develop
Are you sure you want to change the base?
Conversation
🚀 Expo preview is ready!
|
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.
lgtm but would be nice if @tomasklim could take a look, because it could affect desktop
(_, { getState, dispatch, extra }) => { | ||
const enabledNetworks = extra.selectors.selectEnabledNetworks(getState()); | ||
(_, { getState, dispatch }) => { | ||
const networks = selectDeviceNetworkSymbolsOfVisibleAccounts(getState()); |
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.
I not sure if this is good solution. On desktop this relies on enabled networks instead of accounts, so if you will have blank desktop app, connect device, enable eth it will take another 2 minutes to fetch stake data because this thunk will not run again when new account is added.
On mobile it's not issue because mobile initiate this fetch when user enters staking detail screen.
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.
👍 You are right. I didnt realize that desktop does not refetch it when entering staking. I'll think about it once more. initStakeDataThunk
is run every minute, so if user enters staking within that 1 minute, it might have wrong apy.
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.
On mobile, using enabled networks might not work for imported accounts (portfolio tracker), because
ETH
might not be amongst enabled networks. On iOS, enabled networks are always empty array. This caused imported accounts with eth staking to show thereBACKUP_ETH_APY
instead of fetching actual apy.This is now resolved by fetching apy based on network symbols of actual device accounts.
Related Issue
Resolve #15362 for portfolio tracker