RPC: getblockfrompeer, return id of peer from where block is being fe…

…tched

groundwork for introducing the fetch block from "any" peer functionality.

src/net_processing.cpp

@@ -514,7 +514,7 @@ class PeerManagerImpl final : public PeerManager
     /** Implement PeerManager */
     void StartScheduledTasks(CScheduler& scheduler) override;
     void CheckForStaleTipAndEvictPeers() override;
-    std::optional<std::string> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override
+    util::Result<NodeId> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override
         EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
     bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
     bool IgnoresIncomingTxs() override { return m_ignore_incoming_txs; }
@@ -1776,9 +1776,9 @@ 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(NodeId peer_id, const CBlockIndex& block_index)
+util::Result<NodeId> PeerManagerImpl::FetchBlock(NodeId peer_id, const CBlockIndex& block_index)
 {
-    if (m_chainman.m_blockman.LoadingBlocks()) return "Loading blocks ...";
+    if (m_chainman.m_blockman.LoadingBlocks()) return util::ErrorUntranslated("Loading blocks ...");
 
     // Only allow fetching from limited peers if the block height is within the allowed window
     bool allow_limited_peers = false;
@@ -1790,21 +1790,21 @@ std::optional<std::string> PeerManagerImpl::FetchBlock(NodeId peer_id, const CBl
 
     // Ensure this peer exists and hasn't been disconnected
     PeerRef peer = GetPeerRef(peer_id);
-    if (peer == nullptr) return "Peer does not exist";
+    if (peer == nullptr) return util::ErrorUntranslated("Peer does not exist");
 
     // Ignore pre-segwit peers
-    if (!CanServeWitnesses(*peer)) return "Pre-SegWit peer";
+    if (!CanServeWitnesses(*peer)) return util::ErrorUntranslated("Pre-SegWit peer");
 
     // Ignore limited peers if height isn't within the allowed window
-    if (IsLimitedPeer(*peer) && !allow_limited_peers) return "Cannot fetch old block from a limited peer";
+    if (IsLimitedPeer(*peer) && !allow_limited_peers) return util::ErrorUntranslated("Cannot fetch old block from a limited peer");
 
     LOCK(cs_main);
 
     // Forget about all prior requests
     RemoveBlockRequest(block_index.GetBlockHash(), std::nullopt);
 
     // Mark block as in-flight
-    if (!BlockRequested(peer_id, block_index)) return "Already requested from this peer";
+    if (!BlockRequested(peer_id, block_index)) return util::ErrorUntranslated("Already requested from this peer");
 
     // Construct message to request the block
     const uint256& hash{block_index.GetBlockHash()};
@@ -1817,11 +1817,11 @@ std::optional<std::string> PeerManagerImpl::FetchBlock(NodeId peer_id, const CBl
         return true;
     });
 
-    if (!success) return "Peer not fully connected";
+    if (!success) return util::ErrorUntranslated("Peer not fully connected");
 
     LogPrint(BCLog::NET, "Requesting block %s from peer=%d\n",
                  hash.ToString(), peer_id);
-    return std::nullopt;
+    return peer_id;
 }
 
 std::unique_ptr<PeerManager> PeerManager::make(CConnman& connman, AddrMan& addrman,

src/net_processing.h

@@ -7,6 +7,7 @@
 #define BITCOIN_NET_PROCESSING_H
 
 #include <net.h>
+#include <util/result.h>
 #include <validationinterface.h>
 
 class AddrMan;
@@ -53,9 +54,9 @@ class PeerManager : public CValidationInterface, public NetEventsInterface
      *
      * @param[in]  peer_id      The peer id
      * @param[in]  block_index  The blockindex
-     * @returns std::nullopt if a request was successfully made, otherwise an error message
+     * @returns the peer id if a request was successfully made, otherwise an error message
      */
-    virtual std::optional<std::string> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) = 0;
+    virtual util::Result<NodeId> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) = 0;
 
     /** Begin running background tasks, should only be called once */
     virtual void StartScheduledTasks(CScheduler& scheduler) = 0;

src/rpc/blockchain.cpp

@@ -432,12 +432,16 @@ static RPCHelpMan getblockfrompeer()
         "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"
         "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.",
+        "Returns the peer id we are requesting the block from if the request was successfully scheduled.",
         {
             {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash to try to fetch"},
             {"peer_id", RPCArg::Type::NUM, RPCArg::Optional::NO, "The peer to fetch it from (see getpeerinfo for peer IDs)"},
         },
-        RPCResult{RPCResult::Type::OBJ, "", /*optional=*/false, "", {}},
+        RPCResult{
+            RPCResult::Type::OBJ, "", "",
+            {
+                {RPCResult::Type::NUM, "peer_id", "The id of the peer from where the block is being fetched"},
+            }},
         RPCExamples{
             HelpExampleCli("getblockfrompeer", "\"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09\" 0")
             + HelpExampleRpc("getblockfrompeer", "\"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09\" 0")
@@ -468,10 +472,14 @@ static RPCHelpMan getblockfrompeer()
         throw JSONRPCError(RPC_MISC_ERROR, "Block already downloaded");
     }
 
-    if (const auto err{peerman.FetchBlock(peer_id, *index)}) {
-        throw JSONRPCError(RPC_MISC_ERROR, err.value());
+    if (const auto& op_res{peerman.FetchBlock(peer_id, *index)}) {
+        UniValue ret(UniValue::VOBJ);
+        ret.pushKV("peer_id", *op_res);
+        return ret;
+    } else {
+        // Error out
+        throw JSONRPCError(RPC_MISC_ERROR, util::ErrorString(op_res).original);
     }
-    return UniValue::VOBJ;
 },
     };
 }

src/util/result.h

@@ -16,6 +16,10 @@ struct Error {
     bilingual_str message;
 };
 
+static inline Error ErrorUntranslated(const std::string& err) {
+    return Error{Untranslated(err)};
+}
+
 //! The util::Result class provides a standard way for functions to return
 //! either error messages or result values.
 //!

test/functional/rpc_getblockfrompeer.py

@@ -88,7 +88,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(node=0, hash=short_tip), timeout=1)
-        assert_equal(result, {})
+        assert_equal(result["peer_id"], peer_0_peer_1_id)
 
         self.log.info("Don't fetch blocks we already have")
         assert_raises_rpc_error(-1, "Block already downloaded", self.nodes[0].getblockfrompeer, short_tip, peer_0_peer_1_id)
@@ -137,7 +137,7 @@ def run_test(self):
         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, {})
+        assert_equal(result["peer_id"], pruned_node_peer_0_id)
 
         self.log.info("Fetched block persists after next pruning event")
         self.generate(self.nodes[0], 250, sync_fun=self.no_op)