Skip to content
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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

vytick
Copy link
Contributor

@vytick vytick commented Nov 19, 2024

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 there BACKUP_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

Copy link

github-actions bot commented Nov 19, 2024

🚀 Expo preview is ready!

  • Project → trezor-suite-preview
  • Platforms → android, ios
  • Scheme → trezorsuitelite
  • Runtime Version → 11
  • More info

Learn more about 𝝠 Expo Github Action

Copy link
Contributor

@Nodonisko Nodonisko left a 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());
Copy link
Contributor

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.

Copy link
Contributor Author

@vytick vytick Nov 19, 2024

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Staking APY doesn't reflect reality
2 participants