Skip to content

Conversation

@sipa
Copy link
Member

@sipa sipa commented Nov 26, 2015

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.

@gmaxwell
Copy link
Contributor

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.

@gmaxwell
Copy link
Contributor

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.

@sipa
Copy link
Member Author

sipa commented Nov 26, 2015

@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.
@gmaxwell gmaxwell modified the milestones: 0.11.0, 0.12.0 Nov 27, 2015
@gmaxwell
Copy link
Contributor

ACK

@gmaxwell gmaxwell merged commit a9f3d3d into bitcoin:master Nov 29, 2015
gmaxwell added a commit that referenced this pull request Nov 29, 2015
a9f3d3d Fix and improve relay from whitelisted peers (Pieter Wuille)
@sdaftuar
Copy link
Member

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).

luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Nov 30, 2015
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
zkbot pushed a commit to zcash/zcash that referenced this pull request Sep 20, 2016
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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants