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 negative locked #1839

Merged
merged 8 commits into from
Dec 3, 2018
Merged

Fix negative locked #1839

merged 8 commits into from
Dec 3, 2018

Conversation

vctt94
Copy link
Member

@vctt94 vctt94 commented Nov 27, 2018

This PR closes #1823 .

It also clean up a bit the StatisticsActions.

I am starting the clean up by separating the methods from balancesStats, making them more independent, and separating the backwards logic from the straight forward one.

The objective behind that is to make it easier to understand and maintain the statistics logic.

This still a WIP because I still have to check if all the exports methods are right, and there are some more clean up to do, but I am opening this PR now so you guys can see it.

The immature and locked values on daily balance looks wrong on master and this PR also fixes it

@matheusd
Copy link
Member

I'm not sure moving the internal methods out of the balance calc stats is so important, but if you feel strongly enough about it, then sure.

You might wanna test/check with decred/dcrwallet#1330 given that it modifies how the lockedByTickets/total balances work (do notice that PR performs a database migration on the wallet, so you might wanna backup your wallet dirs first).

@vctt94
Copy link
Member Author

vctt94 commented Nov 28, 2018

I think this way was easier because I can use the same methods on the backwards and the forward logic. If you have suggestions on ways to make the code cleaner, they can be helpful, as well.

I will test with this PR, thanks for informing.

@vctt94 vctt94 force-pushed the fixNegativeLocked branch 5 times, most recently from 77d54db to 6d9eac9 Compare November 29, 2018 18:53
@vctt94 vctt94 changed the title [WIP] Fix negative locked Fix negative locked Nov 29, 2018
Copy link
Member

@matheusd matheusd left a comment

Choose a reason for hiding this comment

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

Working fine. All tests were with decred/dcrwallet#1330 applied.

@alexlyp alexlyp merged commit f87bc19 into decred:master Dec 3, 2018
@vctt94 vctt94 deleted the fixNegativeLocked branch December 3, 2018 17:36
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.

[overview - charts] Locked value shows as negative
3 participants