rpc: use peer_id, block_hash for FetchBlock

Author: Sjors

src/net_processing.cpp

@@ -312,7 +312,7 @@ class PeerManagerImpl final : public PeerManager
     /** Implement PeerManager */
     void StartScheduledTasks(CScheduler& scheduler) override;
     void CheckForStaleTipAndEvictPeers() override;
-    std::optional<std::string> FetchBlock(NodeId id, const CBlockIndex& block_index) override;
+    std::optional<std::string> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override;
     bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override;
     bool IgnoresIncomingTxs() override { return m_ignore_incoming_txs; }
     void SendPings() override;
@@ -1428,38 +1428,38 @@ bool PeerManagerImpl::BlockRequestAllowed(const CBlockIndex* pindex)
            (GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, m_chainparams.GetConsensus()) < STALE_RELAY_AGE_LIMIT);
 }
 
-std::optional<std::string> PeerManagerImpl::FetchBlock(NodeId id, const CBlockIndex& block_index)
+std::optional<std::string> PeerManagerImpl::FetchBlock(NodeId peer_id, const CBlockIndex& block_index)
 {
     if (fImporting) return "Importing...";
     if (fReindex) return "Reindexing...";
 
     LOCK(cs_main);
     // Ensure this peer exists and hasn't been disconnected
-    CNodeState* state = State(id);
+    CNodeState* state = State(peer_id);
     if (state == nullptr) return "Peer does not exist";
     // Ignore pre-segwit peers
     if (!state->fHaveWitness) return "Pre-SegWit peer";
 
     // Mark block as in-flight unless it already is (for this peer).
     // If a block was already in-flight for a different peer, its BLOCKTXN
     // response will be dropped.
-    if (!BlockRequested(id, block_index)) return "Already requested from this peer";
+    if (!BlockRequested(peer_id, block_index)) return "Already requested from this peer";
 
     // Construct message to request the block
     const uint256& hash{block_index.GetBlockHash()};
     std::vector<CInv> invs{CInv(MSG_BLOCK | MSG_WITNESS_FLAG, hash)};
 
     // Send block request message to the peer
-    bool success = m_connman.ForNode(id, [this, &invs](CNode* node) {
+    bool success = m_connman.ForNode(peer_id, [this, &invs](CNode* node) {
         const CNetMsgMaker msgMaker(node->GetCommonVersion());
         this->m_connman.PushMessage(node, msgMaker.Make(NetMsgType::GETDATA, invs));
         return true;
     });
 
-    if (!success) return "Node not fully connected";
+    if (!success) return "Peer not fully connected";
 
     LogPrint(BCLog::NET, "Requesting block %s from peer=%d\n",
-                 hash.ToString(), id);
+                 hash.ToString(), peer_id);
     return std::nullopt;
 }
 

src/net_processing.h

@@ -45,11 +45,11 @@ class PeerManager : public CValidationInterface, public NetEventsInterface
     /**
      * Attempt to manually fetch block from a given peer. We must already have the header.
      *
-     * @param[in]  id           The peer id
+     * @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
      */
-    virtual std::optional<std::string> FetchBlock(NodeId id, const CBlockIndex& block_index) = 0;
+    virtual std::optional<std::string> 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

@@ -784,8 +784,8 @@ static RPCHelpMan getblockfrompeer()
         "Subsequent calls for the same block and a new peer will cause the response from the previous peer to be ignored.\n"
         "\nReturns an empty JSON object if the request was successfully scheduled.",
         {
-            {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash"},
-            {"nodeid", RPCArg::Type::NUM, RPCArg::Optional::NO, "The node ID (see getpeerinfo for node IDs)"},
+            {"block_hash", 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_EMPTY, "", /*optional=*/ false, "", {}},
         RPCExamples{
@@ -798,10 +798,10 @@ static RPCHelpMan getblockfrompeer()
     ChainstateManager& chainman = EnsureChainman(node);
     PeerManager& peerman = EnsurePeerman(node);
 
-    const uint256& hash{ParseHashV(request.params[0], "hash")};
-    const NodeId nodeid{request.params[1].get_int64()};
+    const uint256& block_hash{ParseHashV(request.params[0], "block_hash")};
+    const NodeId peer_id{request.params[1].get_int64()};
 
-    const CBlockIndex* const index = WITH_LOCK(cs_main, return chainman.m_blockman.LookupBlockIndex(hash););
+    const CBlockIndex* const index = WITH_LOCK(cs_main, return chainman.m_blockman.LookupBlockIndex(block_hash););
 
     if (!index) {
         throw JSONRPCError(RPC_MISC_ERROR, "Block header missing");
@@ -811,7 +811,7 @@ static RPCHelpMan getblockfrompeer()
         throw JSONRPCError(RPC_MISC_ERROR, "Block already downloaded");
     }
 
-    if (const auto err{peerman.FetchBlock(nodeid, *index)}) {
+    if (const auto err{peerman.FetchBlock(peer_id, *index)}) {
         throw JSONRPCError(RPC_MISC_ERROR, err.value());
     }
     return UniValue::VOBJ;

src/rpc/client.cpp

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