diff options
| author | MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> | 2022-12-19 13:59:14 +0100 |
|---|---|---|
| committer | MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> | 2022-12-19 13:59:17 +0100 |
| commit | 3d974960d38b8effcf9c7f84965b8f9cc681d5a1 (patch) | |
| tree | 29224dc4a81fd43635f1e6ccaee00acf43b26863 /src/rpc | |
| parent | 65f5cfda656c150dd0df40369845c53fa387b25f (diff) | |
| parent | 6fefd49527fa0ed9535e54f2a3e76fe2599b2350 (diff) | |
Merge bitcoin/bitcoin#26515: rpc: skip getpeerinfo for a peer without CNodeStateStats
6fefd49527fa0ed9535e54f2a3e76fe2599b2350 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 https://github.com/mzumsande/bitcoin/commit/5f900e27d0e5ceaa3b800a2eb5a8e8ff6bccad3b). 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 6fefd49527fa0ed9535e54f2a3e76fe2599b2350
MarcoFalke:
review ACK 6fefd49527fa0ed9535e54f2a3e76fe2599b2350 👒
Tree-SHA512: 89c8f7318df4634c1630415de9c8350e6dc2d14d9d07e039e5b180c51bfd3ee2ce99eeac4f9f858af7de846f7a6b48fcae96ebac08495b30e431a5d2d4660532
Diffstat (limited to 'src/rpc')
| -rw-r--r-- | src/rpc/net.cpp | 8 |
1 files changed, 8 insertions, 0 deletions
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 1a4cd09284..ba4236aeef 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -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()) { |