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

[BUG] Incorrect addition on account transfer #2910

Closed
1 task done
pravissw opened this issue Feb 2, 2024 · 9 comments · Fixed by #3099
Closed
1 task done

[BUG] Incorrect addition on account transfer #2910

pravissw opened this issue Feb 2, 2024 · 9 comments · Fixed by #3099
Assignees
Labels
approved Approved by the Ivy Wallet team. Ready for dev bug Something isn't working keep Keep it from automatically getting closed by Stale P1 Important but can wait tech debt Legacy code that needs to be migrated

Comments

@pravissw
Copy link

pravissw commented Feb 2, 2024

Please confirm the following

Describe the bug

One of my accounts had a balance of 32.45€. I transferred 167.55€ from another account to make it 200€. Adding this on ivy wallet somehow results in ivy showing a balance of 199€ in the first account.

I modified the amount of the account transfer on ivy wallet to observe some funny results, pfa some screenshots. The same error is also there if I change the amount to 67,55 (the total should be 100€ but it shows 99€)

To Reproduce

Add an account transfer to make the balance a multiple of 100

Expected behavior

The new balance should be exact

Screenshots

Screenshot_2024-02-02-10-23-50-84_5e9f1c73c4ea0ad7b9937a61f977427e
Screenshot_2024-02-02-10-23-40-33_5e9f1c73c4ea0ad7b9937a61f977427e
Screenshot_2024-02-02-10-16-43-13_5e9f1c73c4ea0ad7b9937a61f977427e
Screenshot_2024-02-02-10-16-28-93_5e9f1c73c4ea0ad7b9937a61f977427e
Screenshot_2024-02-02-10-16-14-58_5e9f1c73c4ea0ad7b9937a61f977427e
Screenshot_2024-02-02-10-16-07-47_5e9f1c73c4ea0ad7b9937a61f977427e

Smartphone

OnePlus Nord CE 2

Additional context

No response

@pravissw pravissw added the bug Something isn't working label Feb 2, 2024
@ivywallet
Copy link
Collaborator

Thank you @pravissw for raising Issue #2910! 🚀
What's next? Read our Contribution Guidelines 📚.

Tagging @ILIYANGERMANOV for review & approval 👀

@ILIYANGERMANOV ILIYANGERMANOV added approved Approved by the Ivy Wallet team. Ready for dev P1 Important but can wait tech debt Legacy code that needs to be migrated keep Keep it from automatically getting closed by Stale labels Feb 2, 2024
@Vishwa-Raghavendra
Copy link
Contributor

Hey @ILIYANGERMANOV
I would like to work on this but this would result in re-write of calculation methods
As we have a lot of deprecated methods / Act for it

Acceptable for re-work?

@ILIYANGERMANOV
Copy link
Collaborator

Hey @ILIYANGERMANOV
I would like to work on this but this would result in re-write of calculation methods
As we have a lot of deprecated methods / Act for it

Acceptable for re-work?

Hey @Vishwa-Raghavendra, ideally, we should rewrite them in :shared:domain as use-cases + unit tests and having the new domain model.

You can also fix the old ones if you wish but at one point they'll be thrown away

@Vishwa-Raghavendra
Copy link
Contributor

Hey @ILIYANGERMANOV
I would like to work on this but this would result in re-write of calculation methods
As we have a lot of deprecated methods / Act for it
Acceptable for re-work?

Hey @Vishwa-Raghavendra, ideally, we should rewrite them in :shared:domain as use-cases + unit tests and having the new domain model.

You can also fix the old ones if you wish but at one point they'll be thrown away

Yes I was thinking the same,
re-write in shared:Domain as use cases with some tests

Can't fix the old ones now,
Too many modifications will result in unstable code

@ILIYANGERMANOV
Copy link
Collaborator

@Vishwa-Raghavendra that's perfect! The old ones are garbage, just make sure that the new are:

  • type-safe
  • do not throw exceptions, use Either
  • make them type-safe
  • simple
  • main-safe (computations happen on a background thread)

Also check #2990

@Vishwa-Raghavendra
Copy link
Contributor

Sure @ILIYANGERMANOV

@Vishwa-Raghavendra
Copy link
Contributor

I'm on it

@ivywallet
Copy link
Collaborator

Thank you for your interest @Vishwa-Raghavendra! 🎉
Issue #2910 is assigned to you. You can work on it! ✅

If you don't want to work on it now, please un-assign yourself so other contributors can take it.

Also, make sure to read our Contribution Guidelines.

@ILIYANGERMANOV
Copy link
Collaborator

Hey @Vishwa-Raghavendra did you find anything about this one? I'm considering if we can release the current GitHub version to Beta

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved by the Ivy Wallet team. Ready for dev bug Something isn't working keep Keep it from automatically getting closed by Stale P1 Important but can wait tech debt Legacy code that needs to be migrated
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants