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

Fix/ Pinned Tokens Not Passed to Portfolio lib on failed RPC #1130

Open
wants to merge 9 commits into
base: v2
Choose a base branch
from

Conversation

gergana95
Copy link
Member

@gergana95 gergana95 commented Nov 27, 2024

Closes 3294

Update the structure of networksWithAssetsByAccounts with an object structure and boolean for each network.
Display rpc еrror banner on 1. network is with asset 2. network is not yet added in the networksWithAssetsByAccounts

@gergana95 gergana95 added the bug Something isn't working label Nov 27, 2024
@gergana95 gergana95 self-assigned this Nov 27, 2024
src/libs/banners/banners.ts Outdated Show resolved Hide resolved
const getAccountNetworksWithAssets = (
accountId: AccountId,
accountState: AccountState,
storageStateByAccount: {
[accountId: string]: NetworkId[]
[accountId: string]: { [networkId: NetworkId]: boolean } | NetworkId[]
Copy link
Member

Choose a reason for hiding this comment

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

Should we make { [networkId: NetworkId]: boolean } a type?

Comment on lines 53 to +56
if (nonZeroTokens.length || hasCollectibles) {
if (networksWithAssets.includes(networkId)) return

networksWithAssets.push(networkId)
return
networksWithAssets[networkId] = true
} else {
networksWithAssets[networkId] = false
Copy link
Member

Choose a reason for hiding this comment

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

networksWithAssets[networkId] = !!nonZeroTokens.length || hasCollectibles

Comment on lines +40 to +44
if (hasAssetsOnNetwork(storageStateByAccount[accountId], networkId)) {
networksWithAssets[networkId] = true
} else {
networksWithAssets[networkId] = false
}
Copy link
Member

Choose a reason for hiding this comment

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

If the RPC is down we shouldn't update the old value. To accomplish this we should initialize networksWithAssets with the old value:

const networksWithAssets: { [networkId: NetworkId]: boolean } = storageStateByAccount[accountId]

Then, in case of an RPC error we should simply return

if (!result || isRPCDown) return

because the result is not valid and thus the old value shouldn't be updated

Copy link
Member

Choose a reason for hiding this comment

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

Also, please check if storageStateByAccount[accountId] matches the new structure. Imo if the value doesn't match the new structure we can discard it as it takes one update to switch to the new structure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants