-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Fix and improve relay from whitelisted peers #7106
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
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.