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

Fixed negative transactions #57

Merged
merged 7 commits into from
May 26, 2019
Merged

Fixed negative transactions #57

merged 7 commits into from
May 26, 2019

Conversation

JSKitty
Copy link
Contributor

@JSKitty JSKitty commented Dec 14, 2018

While messing around and testing the blockchain on my Naivecoin fork, I discovered it was possible to send "negative transactions", basically... you could 'steal' from any address, if you sent a minus transaction to them, even if that address didn't have those coins, or if the address didn't yet exist.

This means you could effectively create coins from thin air, just by adding a - onto the front of your transaction amount.

E.G: Sending -50 to bob --> bob loses 50 coins and you gain 50 coins. (Screenshot below will explain this visually, using the block-explorer)

image

This PR will fix this by looping through outputs of every regular transaction (Fee and Reward transactions are not checked), and if it encounters a negative output, the transaction is rejected. This is built into both Transaction.js and the miner index.

Hope this helps!! This is my first code commit to another project, take it easy on me! <3

@coveralls
Copy link

coveralls commented Dec 14, 2018

Coverage Status

Coverage decreased (-1.2%) to 85.043% when pulling bee4d10 on JSKitty:master into 9116c57 on conradoqg:master.

@Chovanec
Copy link

I think empty transaction should not be rejected

@Chovanec
Copy link

Chovanec commented May 21, 2019

And why you used decimal number in this condition:
if (this.data.outputs[i].amount < 0.00000001) {
this should work:
if (this.data.outputs[i].amount < 0) {
or this if you want reject even zero transaction:
if (this.data.outputs[i].amount < 1) {

@conradoqg
Copy link
Owner

Nice work @JSKitty! Thanks for this PR.

@Chovanec has a valid question. I think <0 is good enough.

Changing that, I will approve this PR.

@JSKitty
Copy link
Contributor Author

JSKitty commented May 25, 2019

Fixed

@conradoqg conradoqg merged commit 1aac1e6 into conradoqg:master May 26, 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.

4 participants