Sometimes nodes send many duplicate blocks. This patch disconnects the p... #3765

pull rebroad wants to merge 1 commits into bitcoin:master from rebroad:RepeatedDupBlocksDisconnect changing 2 files +18 −4

rebroad commented at 10:14 PM on February 27, 2014: contributor

...eer that's running behind. This shouldn't happen, but it can and does sometimes, so this code caters for that.

This code also becomes more important as the block download algorithm is changed in future, such as concurrent block downloads from multiple peers, and retries are sent to other peers when peers seem unresponsive, only to become responsive again later, by which time they are sending duplicate blocks.

This was pull requested previously, but NACKed with the argument that we shouldn't punish peers for doing what was asked of them. However, although we may fix that in future, I still maintain this is for the overall health of the network, and may become redundant in future as the block download algorithm improves. Even if the algorithm is later improved, it would still be beneficial to have this code, although perhaps instead of simply disconnecting the nodes, it would be better to mark them as misbehaving (e.g. if they send blocks which were never requested).

Sometimes nodes send many duplicate blocks. This patch disconnects the peer that's running

behind. This shouldn't happen, but it can and does sometimes, so this code caters for that.

This code also becomes more important as the block download algorithm is changed in future,
such as concurrent block downloads from multiple peers, and retries are sent to other peers
when peers seem unresponsive, only to become responsive again later, by which time
they are sending duplicate blocks.

c1b98ca4e0

BitcoinPullTester commented at 10:56 PM on February 27, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/c1b98ca4e022ab263ce464753605b34da83e40be for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
gmaxwell commented at 10:57 PM on February 27, 2014: contributor

NACK again.
laanwj commented at 7:42 AM on February 28, 2014: member

NACK. This was rejected before because it is harmful to the network, it makes no sense to resubmit this. If nodes are sending you duplicate blocks that usually means that your node asked for them. So fix the request algorithm, don't punish nodes for replying.
laanwj closed this on Feb 28, 2014
rebroad commented at 6:41 PM on February 28, 2014: contributor

@laanwj so, to clarify, what should happen if a node requests a block from a node, but that node never responds with it? Should it wait forever, potentially never getting the block, or should it disconnect the node that isn't responding? Surely these are the only two options.
luke-jr commented at 6:48 PM on February 28, 2014: member

It should fetch it from another node.
laanwj commented at 9:12 AM on March 1, 2014: member

As for disconnecting nodes that don't answer, don't we already have a ping mechanism for that? I do suppose a malicious node could just ignore requests for blocks while answering pings, but that's not something that would be armored against with this pull.
DrahtBot locked this on Sep 8, 2021

Contributors