rpc: getblockfrompeer, add one-shot block requests capability

Allowing what we had before, a single block request with no automatic
retry nor tracking mechanism.

src/net_processing.cpp

@@ -513,7 +513,7 @@ public:
     /** Implement PeerManager */
     void StartScheduledTasks(CScheduler& scheduler) override;
     void CheckForStaleTipAndEvictPeers() override;
-    std::optional<std::string> FetchBlock(std::optional<NodeId> op_peer_id, const CBlockIndex& block_index) override
+    std::optional<std::string> FetchBlock(std::optional<NodeId> op_peer_id, const CBlockIndex& block_index, bool retry) override
         EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
     bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
     std::vector<TxOrphanage::OrphanTxBase> GetOrphanTransactions() override EXCLUSIVE_LOCKS_REQUIRED(!m_tx_download_mutex);
@@ -2072,13 +2072,14 @@ bool PeerManagerImpl::BlockRequestAllowed(const CBlockIndex* pindex)
            (GetBlockProofEquivalentTime(*m_chainman.m_best_header, *pindex, *m_chainman.m_best_header, m_chainparams.GetConsensus()) < STALE_RELAY_AGE_LIMIT);
 }
 
-std::optional<std::string> PeerManagerImpl::FetchBlock(std::optional<NodeId> op_peer_id, const CBlockIndex& block_index)
+std::optional<std::string> PeerManagerImpl::FetchBlock(std::optional<NodeId> op_peer_id, const CBlockIndex& block_index, bool retry)
 {
     if (m_chainman.m_blockman.LoadingBlocks()) return "Loading blocks ...";
 
     // If no peer id was specified, track block. The internal block sync process will
     // be in charge of downloading the block.
     if (!op_peer_id) {
+        if (!retry) return "'retry' disabled, cannot perform single block request"; // future: enable one-try requests.
         if (!m_block_tracker.track(block_index.GetBlockHash())) return "Already tracked block";
         LogDebug(BCLog::NET, "Block added to the tracking list, hash %s\n", block_index.GetBlockHash().ToString());
         return std::nullopt;
@@ -2101,8 +2102,8 @@ std::optional<std::string> PeerManagerImpl::FetchBlock(std::optional<NodeId> op_
     // Mark block as in-flight
     if (!BlockRequested(peer_id, block_index)) return "Already requested from this peer";
 
-    // Track block request
-    m_block_tracker.track(block_index.GetBlockHash(), peer_id);
+    // Track block request (only if requested)
+    if (retry) m_block_tracker.track(block_index.GetBlockHash(), peer_id);
 
     // Construct message to request the block
     const uint256& hash{block_index.GetBlockHash()};

src/net_processing.h

@@ -92,7 +92,7 @@ public:
      * @param[in]  block_index  The blockindex
      * @returns std::nullopt if a request was successfully made, otherwise an error message
      */
-    virtual std::optional<std::string> FetchBlock(std::optional<NodeId> op_peer_id, const CBlockIndex& block_index) = 0;
+    virtual std::optional<std::string> FetchBlock(std::optional<NodeId> op_peer_id, const CBlockIndex& block_index, bool retry) = 0;
 
     /** Begin running background tasks, should only be called once */
     virtual void StartScheduledTasks(CScheduler& scheduler) = 0;

src/rpc/blockchain.cpp

@@ -461,6 +461,8 @@ static RPCHelpMan getblockfrompeer()
             {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash to try to fetch"},
             {"peer_id", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The peer to fetch it from (see getpeerinfo for peer IDs). "
                                                                       "If omitted, the node will fetch the block from any available peer."},
+            {"retry", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Whether to automatically retry to download the block from different peers if the initial request fails or not. "
+                                                                      "If omitted, the node will continuously attempt to download the block from various peers until it succeeds."},
         },
         RPCResult{RPCResult::Type::OBJ, "", /*optional=*/false, "", {}},
         RPCExamples{
@@ -475,6 +477,8 @@ static RPCHelpMan getblockfrompeer()
 
     const uint256& block_hash{ParseHashV(request.params[0], "blockhash")};
     const std::optional<NodeId> peer_id = request.params[1].isNull() ? std::nullopt : std::make_optional(request.params[1].getInt<int64_t>());
+    const bool retry = !request.params[2].isNull() ? request.params[2].get_bool() : true; // default true
+    if (!retry && !peer_id) throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot disable the 'retry' process without specifying a peer from which the block will be downloaded ('peer_id')");
 
     const CBlockIndex* const index = WITH_LOCK(cs_main, return chainman.m_blockman.LookupBlockIndex(block_hash););
 
@@ -493,7 +497,7 @@ static RPCHelpMan getblockfrompeer()
         throw JSONRPCError(RPC_MISC_ERROR, "Block already downloaded");
     }
 
-    if (const auto err{peerman.FetchBlock(peer_id, *index)}) {
+    if (const auto err{peerman.FetchBlock(peer_id, *index, retry)}) {
         throw JSONRPCError(RPC_MISC_ERROR, err.value());
     }
     return UniValue::VOBJ;

src/rpc/client.cpp

@@ -65,6 +65,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "getbalance", 2, "include_watchonly" },
     { "getbalance", 3, "avoid_reuse" },
     { "getblockfrompeer", 1, "peer_id" },
+    { "getblockfrompeer", 2, "retry" },
     { "getblockhash", 0, "height" },
     { "waitforblockheight", 0, "height" },
     { "waitforblockheight", 1, "timeout" },

test/functional/rpc_getblockfrompeer.py

@@ -195,9 +195,10 @@ class GetBlockFromPeerTest(BitcoinTestFramework):
         self.connect_nodes(1, 2)
 
         # Move clock above the block request timeout and assert the initial block fetching failed
+        current_time = current_time + 610
         with pruned_node.assert_debug_log([f"Timeout downloading block {pruned_block_15} from peer={not_responding_peer_id}"], timeout=5):
             for node in self.nodes:
-                node.setmocktime(current_time + 610)
+                node.setmocktime(current_time)
 
         # Now verify that the block was requested and received from another peer after the initial failure
         self.wait_until(lambda: self.check_for_block(node=2, hash=pruned_block_15), timeout=3)
@@ -219,6 +220,37 @@ class GetBlockFromPeerTest(BitcoinTestFramework):
         self.connect_nodes(0, 2)
         self.wait_until(lambda: self.check_for_block(node=2, hash=pruned_block_10), timeout=5)
 
+        ##############################################################################
+        # Try to fetch block from certain peer only once, no automatic retry process #
+        ##############################################################################
+
+        self.log.info("Try to fetch block from certain peer only once")
+
+        # Advance peers time so every peer stay responsive
+        current_time = current_time + 60
+        for node in self.nodes:
+            node.setmocktime(current_time)
+
+        # Connect peer that can serve blocks but will not answer to the getdata requests
+        pruned_node.add_p2p_connection(P2PInterface())
+        not_responding_peer_id = pruned_node.getpeerinfo()[-1]["id"]  # last connection
+        # Also connect full node that can serve the block
+        self.connect_nodes(1, 2)
+
+        # Request block with 'retry=false'
+        pruned_block_9 = self.nodes[0].getblockhash(9)
+        result = pruned_node.getblockfrompeer(pruned_block_9, not_responding_peer_id, retry=False)
+        assert_equal(result, {})
+
+        # Move clock above the block request timeout and assert the initial block fetching failed
+        with pruned_node.assert_debug_log([f"Timeout downloading block {pruned_block_9} from peer={not_responding_peer_id}"]):
+            for node in self.nodes:
+                node.setmocktime(current_time + 610)
+
+        # Sleep for a bit and verify that the block was not requested to any other peer
+        time.sleep(3)
+        self.wait_until(lambda: not self.check_for_block(node=2, hash=pruned_block_9), timeout=3)
+
 
 if __name__ == '__main__':
     GetBlockFromPeerTest(__file__).main()