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

Critical fix: introduce back total-value check #1220

Merged
merged 3 commits into from
Jun 26, 2019
Merged

Conversation

protolambda
Copy link
Contributor

This was dropped in a376b66, as improvement in dust checking.
Now that dust-checking is done, we still need to check if the sender has the minimum value, as decrease balance just clips to 0.
See be86f96 for older dust-creation problem work around, which was dropped in the above.

The bug enabled you to transfer your full balance to someone else, and pay the same amount in fee, possibly to a puppet proposer to collect back funds.
Effectively enabling printing of money. Silly bug, good to fix and introduce tests for.

This was dropped in a376b66, as improvement in dust checking.
Now that dust-checking is done, we still need to check if the sender has the minimum value, as decrease balance just clips to 0.
See be86f96 for older dust-creation problem work around, which was dropped in the above.

The bug enabled you to transfer your full balance to someone else, and pay the same amount in fee, possibly to a puppet proposer to collect back funds.
Effectively enabling printing of money. Silly bug, good to fix and introduce tests for.
@protolambda protolambda added general:bug Something isn't working milestone:June 30 freeze 🥶 Phase 0 spec freeze for long-lived cross-client testnet labels Jun 26, 2019
Copy link
Contributor

@CarlBeek CarlBeek left a comment

Choose a reason for hiding this comment

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

Excellent catch! 👍

@protolambda
Copy link
Contributor Author

Added more tests, and tidied formatting a bit. This should cover the edge cases in the code from client implementations better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general:bug Something isn't working milestone:June 30 freeze 🥶 Phase 0 spec freeze for long-lived cross-client testnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants