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

fe329dc936d1e02da406345e4223e11d1fa6fb38 test: Add test for getblockfrompeer on pruned nodes (Fabian Jahr)
cd761e6b2cfade038b8018886c334bf25a0bcbcf 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](https://github.com/bitcoin/bitcoin/pull/20295#pullrequestreview-817157738).

  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 fe329dc936d1e02da406345e4223e11d1fa6fb38
  MarcoFalke:
    review ACK fe329dc936d1e02da406345e4223e11d1fa6fb38 🍉
  stratospher:
    ACK  fe329dc.
  brunoerg:
    re-ACK fe329dc936d1e02da406345e4223e11d1fa6fb38

Tree-SHA512: a686bd8955d9c3baf365db384e497d6ee1aa9ce2fdb0733fe6150f7e3d94bae19d55bc1b347f1c9f619e749e18b41a52b9f8c0aa2042dd311a968a4b5d251fac

2 files changed, 46 insertions, 6 deletions

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 9459801c2c..5afa460075 100644
--- a/src/rpc/blockchain.cpp
+++ b/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"},
diff --git a/test/functional/rpc_getblockfrompeer.py b/test/functional/rpc_getblockfrompeer.py
index dddc779763..2f093bebff 100755
--- a/test/functional/rpc_getblockfrompeer.py
+++ b/test/functional/rpc_getblockfrompeer.py
@@ -24,14 +24,19 @@ from test_framework.util import (
 
 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 @@ class GetBlockFromPeerTest(BitcoinTestFramework):
 
         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 @@ class GetBlockFromPeerTest(BitcoinTestFramework):
 
         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 @@ class GetBlockFromPeerTest(BitcoinTestFramework):
         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()