net_processing: drop Misbehavior for unconnecting headers

This misbehavior was originally intended to prevent bandwidth wastage due to
actually observed very broken (but likely non-malicious) nodes that respond
to GETHEADERS with a response unrelated to the request, triggering a request
cycle.

This has however largely been addressed by the previous commit, which causes
non-connecting HEADERS that are received while a GETHEADERS has not been
responded to, to be ignored, as long as they do not time out (2 minutes).
With that, the specific misbehavior is largely irrelevant (for inbound peers,
it is now harmless; for outbound peers, the eviction logic will eventually
kick them out if they're not keeping up with headers at all).
This commit is contained in:
Pieter Wuille
2024-03-07 11:22:09 -05:00
parent 9f66ac7cf1
commit 944c54290d
2 changed files with 8 additions and 62 deletions

View File

@@ -71,19 +71,13 @@ f. Announce 1 more header that builds on that fork.
Expect: no response.
Part 5: Test handling of headers that don't connect.
a. Repeat 10 times:
a. Repeat 100 times:
1. Announce a header that doesn't connect.
Expect: getheaders message
2. Send headers chain.
Expect: getdata for the missing blocks, tip update.
b. Then send 9 more headers that don't connect.
b. Then send 99 more headers that don't connect.
Expect: getheaders message each time.
c. Announce a header that does connect.
Expect: no response.
d. Announce 49 headers that don't connect.
Expect: getheaders message each time.
e. Announce one more that doesn't connect.
Expect: disconnect.
"""
from test_framework.blocktools import create_block, create_coinbase
from test_framework.messages import CInv
@@ -526,7 +520,8 @@ class SendHeadersTest(BitcoinTestFramework):
# First we test that receipt of an unconnecting header doesn't prevent
# chain sync.
expected_hash = tip
for i in range(10):
NUM_HEADERS = 100
for i in range(NUM_HEADERS):
self.log.debug("Part 5.{}: starting...".format(i))
test_node.last_message.pop("getdata", None)
blocks = []
@@ -550,15 +545,14 @@ class SendHeadersTest(BitcoinTestFramework):
blocks = []
# Now we test that if we repeatedly don't send connecting headers, we
# don't go into an infinite loop trying to get them to connect.
MAX_NUM_UNCONNECTING_HEADERS_MSGS = 10
for _ in range(MAX_NUM_UNCONNECTING_HEADERS_MSGS + 1):
for _ in range(NUM_HEADERS + 1):
blocks.append(create_block(tip, create_coinbase(height), block_time))
blocks[-1].solve()
tip = blocks[-1].sha256
block_time += 1
height += 1
for i in range(1, MAX_NUM_UNCONNECTING_HEADERS_MSGS):
for i in range(1, NUM_HEADERS):
with p2p_lock:
test_node.last_message.pop("getheaders", None)
# Send an empty header as a failed response to the received getheaders
@@ -569,29 +563,6 @@ class SendHeadersTest(BitcoinTestFramework):
test_node.send_header_for_blocks([blocks[i]])
test_node.wait_for_getheaders(block_hash=expected_hash)
# Next header will connect, should re-set our count:
test_node.send_header_for_blocks([blocks[0]])
expected_hash = blocks[0].sha256
# Remove the first two entries (blocks[1] would connect):
blocks = blocks[2:]
# Now try to see how many unconnecting headers we can send
# before we get disconnected. Should be 5*MAX_NUM_UNCONNECTING_HEADERS_MSGS
for i in range(5 * MAX_NUM_UNCONNECTING_HEADERS_MSGS - 1):
# Send a header that doesn't connect, check that we get a getheaders.
test_node.send_header_for_blocks([])
test_node.send_header_for_blocks([blocks[i % len(blocks)]])
test_node.wait_for_getheaders(block_hash=expected_hash)
# Eventually this stops working.
test_node.send_header_for_blocks([blocks[-1]])
# Should get disconnected
test_node.wait_for_disconnect()
self.log.info("Part 5: success!")
# Finally, check that the inv node never received a getdata request,
# throughout the test
assert "getdata" not in inv_node.last_message