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

Network congestion attacks #353

Closed
bpetridis opened this issue Aug 24, 2018 · 7 comments · Fixed by #500
Closed

Network congestion attacks #353

bpetridis opened this issue Aug 24, 2018 · 7 comments · Fixed by #500
Labels
discussion Initial issue state - proposed but not yet accepted

Comments

@bpetridis
Copy link

bpetridis commented Aug 24, 2018

Raising as its happening again today. Network is under attack again, believe a long term solution needs to be investigated with a combination of larger block sizes, higher mandated fees and some method of identifying and de prioritising spam trasnactions. Without a long term solution the ecosystem wil continue to suffer. I raised this in the ama, I had a thread here as well.

Would like some ideas and feedback as to what is being done to combat this as it's really hurting the entire ecosystem right now.

@erikzhang erikzhang added the discussion Initial issue state - proposed but not yet accepted label Aug 24, 2018
@wy
Copy link
Contributor

wy commented Aug 24, 2018

@bpetridis - the network attacks were only successful because of three small bugs which made the code not behave as expected. With those three changes, the network attack would be rendered ineffective as long as people added a fee onto their transactions. These changes have now all been merged and will be live now/soon so we should be able to move forward successfully.

These three small bugs:

  1. Pick 499 best transactions - there's a limit of 500 (including the MinerTransaction). If there are too many candidate transactions (currently 10,000 plus due to spam), try and pick the best 499 by ranking it based on Fee per Bytes. This makes sense, however, a small bug in the way Fee per Bytes was being calculated meant that Fee per Bytes could equal 0 due to rounding if Fee was very small and Bytes was moderately large. This was why some transactions with fees were still not being included.
  2. Limit free transactions to at most 20 (+ Miner transaction) - an optimisation is in place which limits free transactions to at most 20, allowing the rest to be paid. There was a bug in how this was processed meaning that it would prematurely finish processing before reviewing all the transactions.
  3. If Unconfirmed Transaction count is too big (>50,000), discard the worst txs - in a similar idea to (1), if we have too many transactions, prefer the ones with the best Fee/Size ratio. Also suffers from the same rounding error.

These are fixed in:

  1. neo-project/neo-modules@095e89f
  2. remove break statement from FilterFree neo-modules#3
  3. Resolve rounding to prioritise fee tx over free tx #354

@hal0x2328 @belane @jseagrave21

@vncoelho
Copy link
Member

My appreciation to the time dedicated by you today, @hal0x2328 @belane @jseagrave2 and @wy. You all went directly to the point.
Furthermore, thanks to @erikzhang for quickly checking it out and merging with the necessary caution and attention that you always represent! Congratulations.

@bpetridis
Copy link
Author

Very much appreciate the quick responses guys, looking forward to the merging of the above code fixes so you and the community can all move on with life without these malicious attacks. The fact that parties are conducting these attacks is encouraging I guess from two aspects

  1. Gives you a chance to make the network more robust
  2. Means parties out there feel threatened by NEO

All positive things

Thanks again

Bill

@jsolman
Copy link
Contributor

jsolman commented Aug 25, 2018

This would help also to not starve RPC nodes with a large mem_pool around: #356
I'm using this with a couple tweaks to be able to have reasonable performance during the large mempool backlog situation.

I've finally been looking over #288 . Most of this won't really apply after merging #288. In various cases though the actor model can actually be worse performance than properly written read / write locking. I like the actor model approach though.

@bpetridis
Copy link
Author

Hi Guys,

When do you expect the fixes in the branches for the three tickets that were recently merged by Eric to be deployed to all nodes?

neo-project/neo-modules@095e89f
neo-project/neo-modules#3
#354

Cheers

Bill

@bpetridis
Copy link
Author

Guys this is now urgent, we are working with the community and there are a lot of impacted users. Switcheo is borderline unusable and its getting difficult to support people. We need an update on this urgently, if its being tested fine, we just need an update with a timeline so we can assist people. Need to know where this is at and we we can expect a deployment. The NEX sale is now looming so we need the network robust before that.

Please escalate this

Kind regards

Bill

@jsolman
Copy link
Contributor

jsolman commented Dec 5, 2018

There are some other issues open to help with potentially blocking addresses from spamming entirely, but as far as the mempool causing nodes to have performance issues during spam that will be fixed. So i'm going to set this issue to close by the PR that improves the mempool performance such that nodes can run normally with a full mempool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Initial issue state - proposed but not yet accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants