Merge bitcoin/bitcoin#28996: test: maxuploadtarget: check for mempool msg disconnect if limit is reached, improve existing test coverage

b58f009d95 test: check that mempool msgs lead to disconnect if uploadtarget is reached (Sebastian Falbesoner)
dd5cf38818 test: check for specific disconnect reasons in feature_maxuploadtarget.py (Sebastian Falbesoner)
73d7372115 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):
  160d23677a/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"`):
  160d23677a/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):
  160d23677a/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 b58f009d95
  achow101:
    ACK b58f009d95
  sr-gi:
    tACK [b58f009](b58f009d95)

Tree-SHA512: 7439134129695c9c3a7ddc5e39f2ed700f91e7c91f0b7a9e0a783f275c6aa2f9918529cbfd38bb37f9139184e05e0f0354ef3c3df56da310177ec1d6b48b43d0

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):
+        with self.nodes[0].assert_debug_log(expected_msgs=["historical block serving limit reached, disconnect peer"]):
-            p2p_conns[0].send_message(getdata_request)
+            for _ in range(3):
-        p2p_conns[0].wait_for_disconnect()
+                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)
+        with self.nodes[0].assert_debug_log(expected_msgs=["historical block serving limit reached, disconnect peer"]):
-        p2p_conns[1].wait_for_disconnect()
+            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.log.info("Restarting node 0 with download permission, bloom filter support and 1MB maxuploadtarget")
-        self.restart_node(0, ["-whitelist=download@127.0.0.1", "-maxuploadtarget=1"])
+        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'")