Disconnect on mempool requests from peers when over the upload limit. #7166

Merged
merged 1 commit into from Dec 4, 2015

Conversation

Projects
None yet
8 participants
@gmaxwell
Member

gmaxwell commented Dec 3, 2015

Mempool requests use a fair amount of bandwidth when the mempool is large,
disconnecting peers using them follows the same logic as disconnecting
peers fetching historical blocks.

Disconnect on mempool requests from peers when over the upload limit.
Mempool requests use a fair amount of bandwidth when the mempool is large,
 disconnecting peers using them follows the same logic as disconnecting
 peers fetching historical blocks.

@gmaxwell gmaxwell added the P2P label Dec 3, 2015

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Dec 3, 2015

Member

utACK.

Member

jonasschnelli commented Dec 3, 2015

utACK.

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Dec 3, 2015

Member

Nice, Concept ACK.

Member

MarcoFalke commented Dec 3, 2015

Nice, Concept ACK.

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Dec 3, 2015

Contributor

ACK

Would be nice if #6589 was merged so we could quantify how much is actually used by this on average 👍

Contributor

dcousens commented Dec 3, 2015

ACK

Would be nice if #6589 was merged so we could quantify how much is actually used by this on average 👍

@GIJensen

This comment has been minimized.

Show comment
Hide comment

GIJensen commented Dec 4, 2015

utACK.

@petertodd

This comment has been minimized.

Show comment
Hide comment
@petertodd

petertodd Dec 4, 2015

Contributor

utACK

Contributor

petertodd commented Dec 4, 2015

utACK

@laanwj laanwj merged commit 6aadc75 into bitcoin:master Dec 4, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Dec 4, 2015

Merge pull request #7166
6aadc75 Disconnect on mempool requests from peers when over the upload limit. (Gregory Maxwell)

gmaxwell added a commit that referenced this pull request Dec 4, 2015

Disconnect on mempool requests from peers when over the upload limit.
Mempool requests use a fair amount of bandwidth when the mempool is large,
 disconnecting peers using them follows the same logic as disconnecting
 peers fetching historical blocks.

Rebased-From: 6aadc75
Github-Pull: #7166
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Dec 4, 2015

Member

Cherry-picked to 0.12 as 6ba25d2

Member

laanwj commented Dec 4, 2015

Cherry-picked to 0.12 as 6ba25d2

+ if (CNode::OutboundTargetReached(false) && !pfrom->fWhitelisted)
+ {
+ LogPrint("net", "mempool request with bandwidth limit reached, disconnect peer=%d\n", pfrom->GetId());
+ pfrom->fDisconnect = true;

This comment has been minimized.

@rebroad

rebroad Feb 1, 2016

Contributor

I'm not sure how much this achieves, just disconnecting. Is this considered misbehavior? Should it be considered misbehavior if the node reconnects and continues this behavior?

@rebroad

rebroad Feb 1, 2016

Contributor

I'm not sure how much this achieves, just disconnecting. Is this considered misbehavior? Should it be considered misbehavior if the node reconnects and continues this behavior?

This comment has been minimized.

@laanwj

laanwj Feb 1, 2016

Member

It's not misbehavior - the peer cannot know that you're close to exceeding your bandwidth limit.

If the node reconnects and keeps bombarding with mempool commands it is absolutely misbehavior (with or without this pull), but this change is not meant as mitigation for attacks.
For better or worse, at the moment we don't have any framework to ban peers based on P2P-level behavior.

@laanwj

laanwj Feb 1, 2016

Member

It's not misbehavior - the peer cannot know that you're close to exceeding your bandwidth limit.

If the node reconnects and keeps bombarding with mempool commands it is absolutely misbehavior (with or without this pull), but this change is not meant as mitigation for attacks.
For better or worse, at the moment we don't have any framework to ban peers based on P2P-level behavior.

@str4d str4d referenced this pull request in zcash/zcash Jul 14, 2017

Open

Bitcoin 0.12 P2P/Net PRs 1 #2534

codablock added a commit to codablock/dash that referenced this pull request Feb 7, 2018

Merge pull request #7166
6aadc75 Disconnect on mempool requests from peers when over the upload limit. (Gregory Maxwell)

codablock added a commit to codablock/dash that referenced this pull request Feb 7, 2018

Merge pull request #7166
6aadc75 Disconnect on mempool requests from peers when over the upload limit. (Gregory Maxwell)

codablock added a commit to codablock/dash that referenced this pull request Feb 7, 2018

Merge pull request #7166
6aadc75 Disconnect on mempool requests from peers when over the upload limit. (Gregory Maxwell)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment