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 #3237 14 week powerdown #3238

Merged
merged 2 commits into from
May 6, 2019

Conversation

economicstudio
Copy link
Contributor

Fix #3237

  • Floor -> Ceil
  • Combine the zero new_vesting_withdraw_rate case.

Ceiling is fine (i.e., never withdraw more than requested in total) due to here:

int64_t next_withdrawal = (withdraw_per_period <= total_left) ? withdraw_per_period : total_left;

Actually, the above is what prevents more withdrawal than requested in 14 week.

Fix steemit#3237

- Floor -> Ceil
- Combine the zero new_vesting_withdraw_rate case.
@economicstudio economicstudio changed the title fix 14 week powerdown fix #3237 14 week powerdown Jan 23, 2019
Copy link
Contributor

@TimCliff TimCliff left a comment

Choose a reason for hiding this comment

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

This would need to be wrapped in a hardfork check.

@economicstudio
Copy link
Contributor Author

@TimCliff Thank you for your review. Unfortunately, STINC claims that it's intentional 👍 and closed the issue #3237.

While I agree that it's a tiny difference, I believe that money related problems (like #3184) should be avoided if possible. Is it good to show a user is still powering down for that tiny vest? If remaining withdrawal isn't calculated carefully, it's also prone to a bug to the front end (e.g., busyorg/busy#2159) since withdraw_rate is still the same (i.e., quite big for 14th week). Hope someone believes the same will fix this in the future. Many thanks!

ps. I'm absolutely fine if you or someone else close this.

add hardfork check
@economicstudio
Copy link
Contributor Author

In a case there might be a hardfork for SMT-lite or whatever, I've added hardfork check. @TimCliff Thanks.

@mvandeberg mvandeberg changed the base branch from master to 3238-PR May 6, 2019 20:04
@mvandeberg mvandeberg merged commit 6d940b1 into steemit:3238-PR May 6, 2019
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.

None yet

3 participants