[net] De-duplicate connection eviction logic
ClosedPublic
Actions

Authored by deadalnix on Oct 4 2018, 11:36.

Details

Reviewers

Group Reviewers

Restricted Project

Commits

Summary

This is a backport of core PR11524

Test Plan

make check

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

jasonbcox requested changes to this revision.Oct 4 2018, 20:51

jasonbcox added a subscriber: jasonbcox.

jasonbcox added inline comments.

src/net.cpp
1074 ↗	(On Diff #5267)	This is not entirely true. DoS attacks on these nodes could reduce their ping time as well. Not sure if it's worth updating the comment, but it should be noted.
1077 ↗	(On Diff #5267)	Can't an attacker send the node brand new transactions to manipulate this metric? Generating these transactions is essentially free.
1104 ↗	(On Diff #5267)	Seems odd to remove the intermediate value for group.size(). Please fix this.

This revision now requires changes to proceed.Oct 4 2018, 20:51

deadalnix marked 3 inline comments as done.Oct 4 2018, 21:10

deadalnix added inline comments.

src/net.cpp
1074 ↗	(On Diff #5267)	You'd have to know which nodes to DoS to begin with, or DoS the whole network. Both option seems highly unlikely.
1077 ↗	(On Diff #5267)	Yes. But it gets rid of all the peer that do not do any work and just leach connections. If these txns are going to be propagated anyways, this is useful for you to recieve them :)
1104 ↗	(On Diff #5267)	ok