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

repairing the total balance calculation #110

Conversation

Viterbo
Copy link
Collaborator

@Viterbo Viterbo commented Jan 4, 2023

Fixes #94

Description

The calculation was fixed as described in issue #94. The display part was not implemented due to a previous discussion where we agreed to skip that part because it will be refactored anyway in the near future.

Test scenarios

Let's call myaccount any account you control with the following balances:

  • has some TLOS staking for NET and CPU
  • some refunding TLOS (when you unstake)
  • some TLOS staked in REX with unclaimed profits
  • some liquid deposits (funds) in REX,
  • some liquid in EVM,
  • and finally some liquid TLOS.

test scenarios 1

  • Log in using myaccount in web-wallet
  • explore the same account using OBE.
  • The total balance should only differ in Liquid EVM amount

test scenarios 2

  • Log in using myaccount in web-wallet
  • Move any TLOS from any of the balances without losing ownership (do not transfer)
  • Refresh the website
  • The total balance should be the same as before the balance movement.

Checklist:

  • I have performed a self-review of my own code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have cleaned up the code in the areas my change touches
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings
  • I have removed any unnecessary console messages

@netlify
Copy link

netlify bot commented Jan 4, 2023

Deploy Preview for wallet-staging ready!

Name Link
🔨 Latest commit 723fc65
🔍 Latest deploy log https://app.netlify.com/sites/wallet-staging/deploys/63c8bac20a91e3000968ae20
😎 Deploy Preview https://deploy-preview-110--wallet-staging.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Jan 4, 2023

Deploy Preview for wallet-testnet ready!

Name Link
🔨 Latest commit 723fc65
🔍 Latest deploy log https://app.netlify.com/sites/wallet-testnet/deploys/63c8bac20572c60008442d62
😎 Deploy Preview https://deploy-preview-110--wallet-testnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link

@Kersten-CR Kersten-CR left a comment

Choose a reason for hiding this comment

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

The balances seem to show correctly now. I have verified with EOS-Authority which is a match, and OBE (2 TLOS difference on my tested account though which equals staked resources), needs to be address separately for OBE @karynemayer @donnyquixotic

For the REX balance there is an issue with an edge case, where an account holds REX and REX Savings as the full amount of both is shown (no differentiation inside the balance), which can not be withdrawn with the web wallet and thus leads to an error when trying to remove all. This should be fixed with full code refactoring for Telos Station. @Viterbo

There is still an issue with "strange tokens" showing up though, both on Safari and Chrome, which should be fixed in the next sprint.

Bildschirm­foto 2023-01-05 um 11 19 13

@Kersten-CR Kersten-CR self-requested a review January 9, 2023 11:00
Copy link

@Kersten-CR Kersten-CR left a comment

Choose a reason for hiding this comment

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

works as expected for Balance part and change of balances.

@Viterbo
Copy link
Collaborator Author

Viterbo commented Jan 19, 2023

The balances seem to show correctly now. I have verified with EOS-Authority which is a match, and OBE (2 TLOS difference on my tested account though which equals staked resources), needs to be address separately for OBE @karynemayer @donnyquixotic

For the REX balance there is an issue with an edge case, where an account holds REX and REX Savings as the full amount of both is shown (no differentiation inside the balance), which can not be withdrawn with the web wallet and thus leads to an error when trying to remove all. This should be fixed with full code refactoring for Telos Station. @Viterbo

There is still an issue with "strange tokens" showing up though, both on Safari and Chrome, which should be fixed in the next sprint.

Bildschirm­foto 2023-01-05 um 11 19 13

@Kersten-CR,
What you say about REX and saving is correct. The total amount of REX displayed includes everything:

  • Matured (ready to withdraw)
  • Deposits (ready to withdraw)
  • Profits (partially, depends)
  • Maturing (not ready)
  • Savings (not ready)

In the near future spring, we can address the refactoring task which will be a definitive and clean solution.

Also, I created this issue (#126) to address the custom token list problems.

@donnyquixotic donnyquixotic merged commit 54cbdde into develop Jan 20, 2023
@donnyquixotic donnyquixotic deleted the 94-repair-the-balance-calculation-and-redesign-the-presentation branch January 20, 2023 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants