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.

Sign up for GitHub

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

Jump to bottom

feat: wallet balance rollup #101

Merged
merged 9 commits into from
Dec 6, 2024
Merged

feat: wallet balance rollup #101

merged 9 commits into from
Dec 6, 2024

Conversation

meeh0w
Copy link
Member

@meeh0w meeh0w commented Nov 27, 2024 •
edited
Loading

Description

Changes

  • Total balance for the wallet is added in the account switcher

Testing

  • Have underived accounts with balances (i.e. perform a clean onboarding)
  • In the account switcher, next to the wallet name, you should see the total balance for the wallet. It should be a total of:
    • Balances for all derived accounts on all favorited networks (C/P/X chains always included)
    • X/P-Chain balances for underived accounts (i.e. if you have only two accounts derived, but the 3rd also has some balance on the X- or P-Chain, this balance be included in the total).
      • Yes, this means that summing up the balances of all visible accounts for the wallet may not always result in the same value as the balance next to the wallet name
  • If there are accounts with X/P-Chain balances that are not visible in the account switcher (not derived yet), View P-Chain Details link should be displayed

Screenshots:

XP-Chain.balance.rollup.mov

Checklist for the author

  • I've covered new/modified business logic with Jest test cases.
  • I've tested the changes myself before sending it to code review and QA.

@meeh0w meeh0w changed the base branch from main to feat/new-account-switcher November 27, 2024 14:19
@meeh0w meeh0w marked this pull request as ready for review December 1, 2024 20:44
meeh0w
meeh0w commented Dec 1, 2024
View reviewed changes
src/contexts/BalancesProvider.tsx
const tokenTypes = Object.entries(subscribers)
.filter(([, subscriberCount]) => subscriberCount > 0)
.map(([tokenType]) => tokenType as TokenType);
const tokenTypes = Object.entries(subscribers)
Copy link
Member Author

Choose a reason for hiding this comment

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

This fixes a bug which was causing us to still actively poll balances on screens which don't necessarily need them.

meeh0w
meeh0w commented Dec 1, 2024
View reviewed changes
src/pages/Home/components/Portfolio/NetworkWidget/PchainActiveNetworkWidgetContent.tsx Outdated
Copy link
Member Author

@meeh0w meeh0w Dec 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Changes in this file fix a bug in the active network widget for P-Chain (we stopped showing the balance breakdown at some point):

image

meeh0w
meeh0w commented Dec 1, 2024
View reviewed changes
src/background/services/balances/utils/calculateTotalBalance.ts Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

This was no longer used

@meeh0w meeh0w force-pushed the feat/wallet-balance-rollup branch from 24f970b to 0284bac Compare December 6, 2024 11:04
@meeh0w meeh0w requested a review from gergelylovas December 6, 2024 11:45
gergelylovas
gergelylovas approved these changes Dec 6, 2024
View reviewed changes
Copy link
Contributor

@gergelylovas gergelylovas left a comment

Choose a reason for hiding this comment

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

Nice work!

@meeh0w meeh0w merged commit 530d0fa into feat/new-account-switcher Dec 6, 2024
2 checks passed
@meeh0w meeh0w deleted the feat/wallet-balance-rollup branch December 6, 2024 12:33
@meeh0w meeh0w mentioned this pull request Dec 6, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gergelylovas gergelylovas gergelylovas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@meeh0w @gergelylovas