diff options
| -rw-r--r-- | src/net_processing.cpp | 25 | ||||
| -rw-r--r-- | src/net_processing.h | 4 | ||||
| -rw-r--r-- | src/rpc/blockchain.cpp | 12 | ||||
| -rwxr-xr-x | test/functional/rpc_getblockfrompeer.py | 2 |
4 files changed, 17 insertions, 26 deletions
diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1319267944..b7b1c828d4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -312,7 +312,7 @@ public: /** Implement PeerManager */ void StartScheduledTasks(CScheduler& scheduler) override; void CheckForStaleTipAndEvictPeers() override; - bool FetchBlock(NodeId id, const CBlockIndex& block_index) override; + std::optional<std::string> FetchBlock(NodeId 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,21 +1428,22 @@ bool PeerManagerImpl::BlockRequestAllowed(const CBlockIndex* pindex) (GetBlockProofEquivalentTime(*pindexBestHeader, *pindex, *pindexBestHeader, m_chainparams.GetConsensus()) < STALE_RELAY_AGE_LIMIT); } -bool PeerManagerImpl::FetchBlock(NodeId id, const CBlockIndex& block_index) +std::optional<std::string> PeerManagerImpl::FetchBlock(NodeId id, const CBlockIndex& block_index) { - if (fImporting || fReindex) return false; + if (fImporting) return "Importing..."; + if (fReindex) return "Reindexing..."; LOCK(cs_main); // Ensure this peer exists and hasn't been disconnected CNodeState* state = State(id); - if (state == nullptr) return false; + if (state == nullptr) return "Peer does not exist"; // Ignore pre-segwit peers - if (!state->fHaveWitness) return false; + 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 false; + if (!BlockRequested(id, block_index)) return "Already requested from this peer"; // Construct message to request the block const uint256& hash{block_index.GetBlockHash()}; @@ -1455,15 +1456,11 @@ bool PeerManagerImpl::FetchBlock(NodeId id, const CBlockIndex& block_index) return true; }); - if (success) { - LogPrint(BCLog::NET, "Requesting block %s from peer=%d\n", - hash.ToString(), id); - } else { - RemoveBlockRequest(hash); - LogPrint(BCLog::NET, "Failed to request block %s from peer=%d\n", + if (!success) return "Node not fully connected"; + + LogPrint(BCLog::NET, "Requesting block %s from peer=%d\n", hash.ToString(), id); - } - return success; + return std::nullopt; } std::unique_ptr<PeerManager> PeerManager::make(const CChainParams& chainparams, CConnman& connman, AddrMan& addrman, diff --git a/src/net_processing.h b/src/net_processing.h index 2b22d8ce85..2c8ae05a26 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -47,9 +47,9 @@ public: * * @param[in] id The peer id * @param[in] block_index The blockindex - * @returns Whether a request was successfully made + * @returns std::nullopt if a request was successfully made, otherwise an error message */ - virtual bool FetchBlock(NodeId id, const CBlockIndex& block_index) = 0; + virtual std::optional<std::string> FetchBlock(NodeId id, const CBlockIndex& block_index) = 0; /** Begin running background tasks, should only be called once */ virtual void StartScheduledTasks(CScheduler& scheduler) = 0; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 27b04ed834..64c852f6a5 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -797,16 +797,10 @@ static RPCHelpMan getblockfrompeer() const NodeContext& node = EnsureAnyNodeContext(request.context); ChainstateManager& chainman = EnsureChainman(node); PeerManager& peerman = EnsurePeerman(node); - CConnman& connman = EnsureConnman(node); - const uint256 hash(ParseHashV(request.params[0], "hash")); + const uint256& hash{ParseHashV(request.params[0], "hash")}; const NodeId nodeid{request.params[1].get_int64()}; - // Check that the peer with nodeid exists - if (!connman.ForNode(nodeid, [](CNode* node) {return true;})) { - throw JSONRPCError(RPC_MISC_ERROR, strprintf("Peer nodeid %d does not exist", nodeid)); - } - const CBlockIndex* const index = WITH_LOCK(cs_main, return chainman.m_blockman.LookupBlockIndex(hash);); if (!index) { @@ -817,8 +811,8 @@ static RPCHelpMan getblockfrompeer() throw JSONRPCError(RPC_MISC_ERROR, "Block already downloaded"); } - if (!peerman.FetchBlock(nodeid, *index)) { - throw JSONRPCError(RPC_MISC_ERROR, "Failed to fetch block from peer"); + if (const auto err{peerman.FetchBlock(nodeid, *index)}) { + throw JSONRPCError(RPC_MISC_ERROR, err.value()); } return UniValue::VOBJ; }, diff --git a/test/functional/rpc_getblockfrompeer.py b/test/functional/rpc_getblockfrompeer.py index 19af197530..1ecf9c1057 100755 --- a/test/functional/rpc_getblockfrompeer.py +++ b/test/functional/rpc_getblockfrompeer.py @@ -56,7 +56,7 @@ class GetBlockFromPeerTest(BitcoinTestFramework): assert_raises_rpc_error(-1, "Block header missing", self.nodes[0].getblockfrompeer, "00" * 32, 0) self.log.info("Non-existent peer generates error") - assert_raises_rpc_error(-1, f"Peer nodeid {peer_0_peer_1_id + 1} does not exist", self.nodes[0].getblockfrompeer, short_tip, peer_0_peer_1_id + 1) + assert_raises_rpc_error(-1, "Peer does not exist", self.nodes[0].getblockfrompeer, short_tip, peer_0_peer_1_id + 1) self.log.info("Successful fetch") result = self.nodes[0].getblockfrompeer(short_tip, peer_0_peer_1_id) |