Merge bitcoin/bitcoin#26515: rpc: skip getpeerinfo for a peer without CNodeStateStats

6fefd49527 rpc: Require NodeStateStats object in getpeerinfo (Martin Zumsande)

Pull request description:

  The objects `CNode`, `CNodeState` and `Peer` store different info about a peer - `InitializeNode()` and `FinalizeNode()` make sure that for the duration of a connection, we should always have one of each for a peer.

  Therefore, there is no situation in which, as part of getpeerinfo RPC,  `GetNodeStateStats()` (which requires a `CNodeState` and a `Peer` entry for a `NodeId` to succeed)  could fail for a legitimate reason while the peer is connected - this can only happen if there is a race condition between peer disconnection and the `getpeerinfo` processing (see also a more detailed description of this in https://github.com/bitcoin/bitcoin/pull/26457#pullrequestreview-1181641835).

  But in this case I think it's better to just not include the newly disconnected peer in the response instead of returning just parts of its data.

  An earlier version of this PR also made the affected `CNodeStateStats` fields non-optional (see 5f900e27d0). Since this conflicts with #25923 and should be a separate discussion, I removed that commit from this PR.

ACKs for top commit:
  dergoegge:
    Approach ACK 6fefd49527
  MarcoFalke:
    review ACK 6fefd49527 👒

Tree-SHA512: 89c8f7318df4634c1630415de9c8350e6dc2d14d9d07e039e5b180c51bfd3ee2ce99eeac4f9f858af7de846f7a6b48fcae96ebac08495b30e431a5d2d4660532
This commit is contained in:
MarcoFalke 2022-12-19 13:59:14 +01:00
commit 3d974960d3
No known key found for this signature in database
GPG key ID: CE2B75697E69A548

View file

@ -185,6 +185,14 @@ static RPCHelpMan getpeerinfo()
UniValue obj(UniValue::VOBJ);
CNodeStateStats statestats;
bool fStateStats = peerman.GetNodeStateStats(stats.nodeid, statestats);
// GetNodeStateStats() requires the existence of a CNodeState and a Peer object
// to succeed for this peer. These are created at connection initialisation and
// exist for the duration of the connection - except if there is a race where the
// peer got disconnected in between the GetNodeStats() and the GetNodeStateStats()
// calls. In this case, the peer doesn't need to be reported here.
if (!fStateStats) {
continue;
}
obj.pushKV("id", stats.nodeid);
obj.pushKV("addr", stats.m_addr_name);
if (stats.addrBind.IsValid()) {