diff options
author | Ava Chow <github@achow101.com> | 2024-02-08 19:24:47 -0500 |
---|---|---|
committer | Ava Chow <github@achow101.com> | 2024-02-08 19:31:23 -0500 |
commit | b2b2b1e9e415c3b5f74d517eaebfc2073cef5175 (patch) | |
tree | 9c106b5b803017ce8971e66afa9fdd3b919057da /test/functional | |
parent | 0b3202d8ef49f9f77d94eee47ad730b12e398b82 (diff) | |
parent | b58f009d9585aab775998644de07e27e2a4a8045 (diff) | |
download | bitcoin-b2b2b1e9e415c3b5f74d517eaebfc2073cef5175.tar.xz |
Merge bitcoin/bitcoin#28996: test: maxuploadtarget: check for mempool msg disconnect if limit is reached, improve existing test coverage
b58f009d9585aab775998644de07e27e2a4a8045 test: check that mempool msgs lead to disconnect if uploadtarget is reached (Sebastian Falbesoner)
dd5cf38818d1e3f6cf583e2b242afd0da68b290a test: check for specific disconnect reasons in feature_maxuploadtarget.py (Sebastian Falbesoner)
73d737211536de5b834f21016c5549e52de11374 test: verify `-maxuploadtarget` limit state via `getnettotals` RPC result (Sebastian Falbesoner)
Pull request description:
This PR improves existing and adds new test coverage for the `-maxuploadtarget` mechanism (feature_maxuploadtarget.py) in the following ways, one commit each:
* verify the uploadtarget state via the `getnettotals` RPC (`uploadtarget` result field):
https://github.com/bitcoin/bitcoin/blob/160d23677ad799cf9b493eaa923b2ac080c3fb8e/src/rpc/net.cpp#L581-L582
Note that reaching the total limit (`target_reached` == True) always implies that the historical blocks serving limits is also reached (`serve_historical_blocks` == False), i.e. it's impossible that both flags are set to True.
* check for peer's specific disconnect reason (in this case, `"historical block serving limit reached, disconnect peer"`):
https://github.com/bitcoin/bitcoin/blob/160d23677ad799cf9b493eaa923b2ac080c3fb8e/src/net_processing.cpp#L2272-L2280
* add a test for a peer disconnect if the uploadtarget is reached and a `mempool` message is received (if bloom filters are enabled):
https://github.com/bitcoin/bitcoin/blob/160d23677ad799cf9b493eaa923b2ac080c3fb8e/src/net_processing.cpp#L4755-L4763
Note that another reason for disconnect after receiving a MEMPOOL msg of a peer is if bloom filters are disabled on the node. This case is already covered in the functional test `p2p_nobloomfilter_messages.py`.
ACKs for top commit:
maflcko:
lgtm ACK b58f009d9585aab775998644de07e27e2a4a8045
achow101:
ACK b58f009d9585aab775998644de07e27e2a4a8045
sr-gi:
tACK [b58f009](https://github.com/bitcoin/bitcoin/commit/b58f009d9585aab775998644de07e27e2a4a8045)
Tree-SHA512: 7439134129695c9c3a7ddc5e39f2ed700f91e7c91f0b7a9e0a783f275c6aa2f9918529cbfd38bb37f9139184e05e0f0354ef3c3df56da310177ec1d6b48b43d0
Diffstat (limited to 'test/functional')
-rwxr-xr-x | test/functional/feature_maxuploadtarget.py | 54 |
1 files changed, 45 insertions, 9 deletions
diff --git a/test/functional/feature_maxuploadtarget.py b/test/functional/feature_maxuploadtarget.py index c551c0b449..814eb21e6f 100755 --- a/test/functional/feature_maxuploadtarget.py +++ b/test/functional/feature_maxuploadtarget.py @@ -8,6 +8,7 @@ if uploadtarget has been reached. * Verify that getdata requests for recent blocks are respected even if uploadtarget has been reached. +* Verify that mempool requests lead to a disconnect if uploadtarget has been reached. * Verify that the upload counters are reset after 24 hours. """ from collections import defaultdict @@ -17,6 +18,7 @@ from test_framework.messages import ( CInv, MSG_BLOCK, msg_getdata, + msg_mempool, ) from test_framework.p2p import P2PInterface from test_framework.test_framework import BitcoinTestFramework @@ -27,6 +29,9 @@ from test_framework.util import ( from test_framework.wallet import MiniWallet +UPLOAD_TARGET_MB = 800 + + class TestP2PConn(P2PInterface): def __init__(self): super().__init__() @@ -45,12 +50,21 @@ class MaxUploadTest(BitcoinTestFramework): self.setup_clean_chain = True self.num_nodes = 1 self.extra_args = [[ - "-maxuploadtarget=800M", + f"-maxuploadtarget={UPLOAD_TARGET_MB}M", "-datacarriersize=100000", ]] self.supports_cli = False + def assert_uploadtarget_state(self, *, target_reached, serve_historical_blocks): + """Verify the node's current upload target state via the `getnettotals` RPC call.""" + uploadtarget = self.nodes[0].getnettotals()["uploadtarget"] + assert_equal(uploadtarget["target_reached"], target_reached) + assert_equal(uploadtarget["serve_historical_blocks"], serve_historical_blocks) + def run_test(self): + # Initially, neither historical blocks serving limit nor total limit are reached + self.assert_uploadtarget_state(target_reached=False, serve_historical_blocks=True) + # Before we connect anything, we first set the time on the node # to be in the past, otherwise things break because the CNode # time counters can't be reset backward after initialization @@ -93,7 +107,7 @@ class MaxUploadTest(BitcoinTestFramework): getdata_request = msg_getdata() getdata_request.inv.append(CInv(MSG_BLOCK, big_old_block)) - max_bytes_per_day = 800*1024*1024 + max_bytes_per_day = UPLOAD_TARGET_MB * 1024 *1024 daily_buffer = 144 * 4000000 max_bytes_available = max_bytes_per_day - daily_buffer success_count = max_bytes_available // old_block_size @@ -107,12 +121,16 @@ class MaxUploadTest(BitcoinTestFramework): assert_equal(len(self.nodes[0].getpeerinfo()), 3) # At most a couple more tries should succeed (depending on how long # the test has been running so far). - for _ in range(3): - p2p_conns[0].send_message(getdata_request) - p2p_conns[0].wait_for_disconnect() + with self.nodes[0].assert_debug_log(expected_msgs=["historical block serving limit reached, disconnect peer"]): + for _ in range(3): + p2p_conns[0].send_message(getdata_request) + p2p_conns[0].wait_for_disconnect() assert_equal(len(self.nodes[0].getpeerinfo()), 2) self.log.info("Peer 0 disconnected after downloading old block too many times") + # Historical blocks serving limit is reached by now, but total limit still isn't + self.assert_uploadtarget_state(target_reached=False, serve_historical_blocks=False) + # Requesting the current block on p2p_conns[1] should succeed indefinitely, # even when over the max upload target. # We'll try 800 times @@ -121,12 +139,16 @@ class MaxUploadTest(BitcoinTestFramework): p2p_conns[1].send_and_ping(getdata_request) assert_equal(p2p_conns[1].block_receive_map[big_new_block], i+1) + # Both historical blocks serving limit and total limit are reached + self.assert_uploadtarget_state(target_reached=True, serve_historical_blocks=False) + self.log.info("Peer 1 able to repeatedly download new block") # But if p2p_conns[1] tries for an old block, it gets disconnected too. getdata_request.inv = [CInv(MSG_BLOCK, big_old_block)] - p2p_conns[1].send_message(getdata_request) - p2p_conns[1].wait_for_disconnect() + with self.nodes[0].assert_debug_log(expected_msgs=["historical block serving limit reached, disconnect peer"]): + p2p_conns[1].send_message(getdata_request) + p2p_conns[1].wait_for_disconnect() assert_equal(len(self.nodes[0].getpeerinfo()), 1) self.log.info("Peer 1 disconnected after trying to download old block") @@ -139,23 +161,32 @@ class MaxUploadTest(BitcoinTestFramework): p2p_conns[2].sync_with_ping() p2p_conns[2].send_and_ping(getdata_request) assert_equal(p2p_conns[2].block_receive_map[big_old_block], 1) + self.assert_uploadtarget_state(target_reached=False, serve_historical_blocks=True) self.log.info("Peer 2 able to download old block") self.nodes[0].disconnect_p2ps() - self.log.info("Restarting node 0 with download permission and 1MB maxuploadtarget") - self.restart_node(0, ["-whitelist=download@127.0.0.1", "-maxuploadtarget=1"]) + self.log.info("Restarting node 0 with download permission, bloom filter support and 1MB maxuploadtarget") + self.restart_node(0, ["-whitelist=download@127.0.0.1", "-peerbloomfilters", "-maxuploadtarget=1"]) + # Total limit isn't reached after restart, but 1 MB is too small to serve historical blocks + self.assert_uploadtarget_state(target_reached=False, serve_historical_blocks=False) # Reconnect to self.nodes[0] peer = self.nodes[0].add_p2p_connection(TestP2PConn()) + # Sending mempool message shouldn't disconnect peer, as total limit isn't reached yet + peer.send_and_ping(msg_mempool()) + #retrieve 20 blocks which should be enough to break the 1MB limit getdata_request.inv = [CInv(MSG_BLOCK, big_new_block)] for i in range(20): peer.send_and_ping(getdata_request) assert_equal(peer.block_receive_map[big_new_block], i+1) + # Total limit is exceeded + self.assert_uploadtarget_state(target_reached=True, serve_historical_blocks=False) + getdata_request.inv = [CInv(MSG_BLOCK, big_old_block)] peer.send_and_ping(getdata_request) @@ -164,6 +195,11 @@ class MaxUploadTest(BitcoinTestFramework): assert_equal(len(peer_info), 1) # node is still connected assert_equal(peer_info[0]['permissions'], ['download']) + self.log.info("Peer gets disconnected for a mempool request after limit is reached") + with self.nodes[0].assert_debug_log(expected_msgs=["mempool request with bandwidth limit reached, disconnect peer"]): + peer.send_message(msg_mempool()) + peer.wait_for_disconnect() + self.log.info("Test passing an unparsable value to -maxuploadtarget throws an error") self.stop_node(0) self.nodes[0].assert_start_raises_init_error(extra_args=["-maxuploadtarget=abc"], expected_msg="Error: Unable to parse -maxuploadtarget: 'abc'") |