Merge bitcoin/bitcoin#23813: Add test and docs for getblockfrompeer with pruning

fe329dc test: Add test for getblockfrompeer on pruned nodes (Fabian Jahr)
cd761e6 rpc: Add note on guarantees to getblockfrompeer (Fabian Jahr)

Pull request description:

  These are additions to `getblockfrompeer` that I already [suggested on the original PR](bitcoin/bitcoin#20295 (review)).

  The two commits do the following:
  1. Add a test for `getblockfrompeer` usage on pruned nodes. This is important because many use-cases for `getblockfrompeer` are in a context of a pruned node.
  2. Add some information on how long the users of pruned nodes can expect the block to be available after they have used the RPC. I think the behavior is not very intuitive for users and I would not be surprised if users expect the block to be available indefinitely.

ACKs for top commit:
  Sjors:
    re-utACK fe329dc
  MarcoFalke:
    review ACK fe329dc 🍉
  stratospher:
    ACK  fe329dc.
  brunoerg:
    re-ACK fe329dc

Tree-SHA512: a686bd8955d9c3baf365db384e497d6ee1aa9ce2fdb0733fe6150f7e3d94bae19d55bc1b347f1c9f619e749e18b41a52b9f8c0aa2042dd311a968a4b5d251fac

Author: fanquake

src/rpc/blockchain.cpp

@@ -432,7 +432,8 @@ static RPCHelpMan getblockfrompeer()
         "We must have the header for this block, e.g. using submitheader.\n"
         "Subsequent calls for the same block and a new peer will cause the response from the previous peer to be ignored.\n"
         "Peers generally ignore requests for a stale block that they never fully verified, or one that is more than a month old.\n"
-        "When a peer does not respond with a block, we will disconnect.\n\n"
+        "When a peer does not respond with a block, we will disconnect.\n"
+        "Note: The block could be re-pruned as soon as it is received.\n\n"
         "Returns an empty JSON object if the request was successfully scheduled.",
         {
             {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash to try to fetch"},

test/functional/rpc_getblockfrompeer.py

@@ -24,14 +24,19 @@
 
 class GetBlockFromPeerTest(BitcoinTestFramework):
     def set_test_params(self):
-        self.num_nodes = 2
+        self.num_nodes = 3
+        self.extra_args = [
+            [],
+            [],
+            ["-fastprune", "-prune=1"]
+        ]
 
     def setup_network(self):
         self.setup_nodes()
 
-    def check_for_block(self, hash):
+    def check_for_block(self, node, hash):
         try:
-            self.nodes[0].getblock(hash)
+            self.nodes[node].getblock(hash)
             return True
         except JSONRPCException:
             return False
@@ -48,7 +53,7 @@ def run_test(self):
 
         self.log.info("Connect nodes to sync headers")
         self.connect_nodes(0, 1)
-        self.sync_blocks()
+        self.sync_blocks(self.nodes[0:2])
 
         self.log.info("Node 0 should only have the header for node 1's block 3")
         x = next(filter(lambda x: x['hash'] == short_tip, self.nodes[0].getchaintips()))
@@ -81,7 +86,7 @@ def run_test(self):
 
         self.log.info("Successful fetch")
         result = self.nodes[0].getblockfrompeer(short_tip, peer_0_peer_1_id)
-        self.wait_until(lambda: self.check_for_block(short_tip), timeout=1)
+        self.wait_until(lambda: self.check_for_block(node=0, hash=short_tip), timeout=1)
         assert_equal(result, {})
 
         self.log.info("Don't fetch blocks we already have")
@@ -111,6 +116,40 @@ def run_test(self):
         error_msg = "In prune mode, only blocks that the node has already synced previously can be fetched from a peer"
         assert_raises_rpc_error(-1, error_msg, self.nodes[1].getblockfrompeer, blockhash, node1_interface_id)
 
+        self.log.info("Connect pruned node")
+        # We need to generate more blocks to be able to prune
+        self.connect_nodes(0, 2)
+        pruned_node = self.nodes[2]
+        self.generate(self.nodes[0], 400, sync_fun=self.no_op)
+        self.sync_blocks([self.nodes[0], pruned_node])
+        pruneheight = pruned_node.pruneblockchain(300)
+        assert_equal(pruneheight, 248)
+        # Ensure the block is actually pruned
+        pruned_block = self.nodes[0].getblockhash(2)
+        assert_raises_rpc_error(-1, "Block not available (pruned data)", pruned_node.getblock, pruned_block)
+
+        self.log.info("Fetch pruned block")
+        peers = pruned_node.getpeerinfo()
+        assert_equal(len(peers), 1)
+        pruned_node_peer_0_id = peers[0]["id"]
+        result = pruned_node.getblockfrompeer(pruned_block, pruned_node_peer_0_id)
+        self.wait_until(lambda: self.check_for_block(node=2, hash=pruned_block), timeout=1)
+        assert_equal(result, {})
+
+        self.log.info("Fetched block persists after next pruning event")
+        self.generate(self.nodes[0], 250, sync_fun=self.no_op)
+        self.sync_blocks([self.nodes[0], pruned_node])
+        pruneheight += 251
+        assert_equal(pruned_node.pruneblockchain(700), pruneheight)
+        assert_equal(pruned_node.getblock(pruned_block)["hash"], "36c56c5b5ebbaf90d76b0d1a074dcb32d42abab75b7ec6fa0ffd9b4fbce8f0f7")
+
+        self.log.info("Fetched block can be pruned again when prune height exceeds the height of the tip at the time when the block was fetched")
+        self.generate(self.nodes[0], 250, sync_fun=self.no_op)
+        self.sync_blocks([self.nodes[0], pruned_node])
+        pruneheight += 250
+        assert_equal(pruned_node.pruneblockchain(1000), pruneheight)
+        assert_raises_rpc_error(-1, "Block not available (pruned data)", pruned_node.getblock, pruned_block)
+
 
 if __name__ == '__main__':
     GetBlockFromPeerTest().main()