Fix and improve relay from whitelisted peers#7106
Conversation
|
utACK-we-should-do-this-over-nothing. I will test too. I still think that whitelist drastically changing our P2P behavior (instead of merely bypassing resource limits/banning/eviction) is highly surprising, and undermines the utility of whitelisting if we're unable to unlink them; so I'd rebase 7099 on top of this and be open to making it default to on. I think long term we should have another p2p message type which means "force relay this transaction" which we ignore the force part for non-whitelisted peers. ... we we don't manage to get initial transaction broadcast out of the basic p2p relaying mechanism first. |
|
Would you oppose adding a non-debug log to this every time it does a forced relay and any time it fails to do one (due to DOS)? Having it logged would help a lot with someone putting whitelisting things they shouldn't while this behavior exists (like mining software) and spamming the network. |
|
@gmaxwell Done. |
This makes sure that retransmits by a whitelisted peer also actually result in a retransmit. Further, this changes the logic to never relay in case we would assign a DoS score, as we expect to get DoS banned ourselves as a result.
|
ACK |
a9f3d3d Fix and improve relay from whitelisted peers (Pieter Wuille)
|
Just curious, if we're willing to relay rejected transactions from whitelisted peers, why not also relay their orphan transactions? The only reason I can think of is it would result in duplicate announcements, but only if the whitelisted peer is sending them out of order; and there might well be use cases where a transaction that is an orphan for you would not be an orphan for your peers (eg if you're running with a small mempool). |
This makes sure that retransmits by a whitelisted peer also actually result in a retransmit. Further, this changes the logic to never relay in case we would assign a DoS score, as we expect to get DoS banned ourselves as a result. Github-Pull: bitcoin#7106 Rebased-From: a9f3d3d
Upstream patch: Fix and improve relay from whitelisted peers bitcoin/bitcoin#7106 a9f3d3d An extra commit modifies the log message string, otherwise there are are a number of commits that need be to backported to add methods e.g. GetDebugMessage. These commits modify the interface in consensus/validation.h so there are conflicts to be resolved. e.g. 9003c7c a9ac95c 5f12263 fbf44e6
This makes sure that retransmits by a whitelisted peer also actually result in a retransmit. This was a bug introduced in 9524c4d (which is in 0.11.1).
Further, this changes the logic to never relay in case we would assign a DoS score, as we expect to get DoS banned ourselves as a result. This part may be acceptable as an alternative to #7099.