rpc: Add optional peer_ids param to filter getpeerinfo #32741

waketraindev commented at 9:51 PM on June 12, 2025: contributor

Adds an optional peer_ids parameter to getpeerinfo to filter results by peer IDs.

This is useful when using bitcoin-cli or the console to inspect specific peers referenced in debug.log.

DrahtBot commented at 9:51 PM on June 12, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32741.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	danielabrozzoni, rkrux
Concept NACK	stickies-v

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

DrahtBot added the label RPC/REST/ZMQ on Jun 12, 2025

in src/rpc/net.cpp:126 in 6285d75ae9 outdated

 121 | @@ -122,7 +122,9 @@ static RPCHelpMan getpeerinfo()
 122 |      return RPCHelpMan{
 123 |          "getpeerinfo",
 124 |          "Returns data about each connected network peer as a json array of objects.",
 125 | -        {},
 126 | +        {
 127 | +            {"nodeid", RPCArg::Type::NUM, RPCArg::Default{-1}, "Filter results to the peer with this nodeid. Specify -1 to return all peers."}

maflcko commented at 6:57 AM on June 13, 2025:

this should be optional, not a magic default value

waketraindev commented at 12:29 PM on June 13, 2025:

Updated to use Optional::OMITTED to avoid relying on magic default value. Thanks!

in src/rpc/net.cpp:208 in 6285d75ae9 outdated

 203 |              + HelpExampleRpc("getpeerinfo", "")
 204 | +            + HelpExampleRpc("getpeerinfo", "7")
 205 |          },
 206 |          [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 207 |  {
 208 | +    const int filterPeerId = request.params[0].isNull() ? -1 : request.params[0].getInt<int>();

maflcko commented at 6:57 AM on June 13, 2025:

nit: snake_case for new code

maflcko commented at 6:58 AM on June 13, 2025:

Also, you can use the self.Arg to get the value.

waketraindev commented at 12:31 PM on June 13, 2025:

Squashed commits. Renamed to filter_id. Evaluating how to best use self.Arg with NodeId. Will follow up

waketraindev commented at 2:21 PM on June 13, 2025:

Also, you can use the self.Arg to get the value.

Would this approach be preferable to avoid using a sentinel value like -1?

const UniValue* nodeid_arg = self.MaybeArg<UniValue>("nodeid");
const std::optional<NodeId> filter_id = nodeid_arg ? std::make_optional(nodeid_arg->getInt<NodeId>()) : std::nullopt;

if (filter_id && stats.nodeid != *filter_id) continue;
...
if (filter_id && stats.nodeid == *filter_id) break;

Would it be acceptable to touch util.cpp for this to add a int64_t template for MaybeArg or stick to the sentinel approach?

TMPL_INST(nullptr, std::optional<int64_t>, maybe_arg ? std::optional{maybe_arg->getInt<int64_t>()} : std::nullopt;);

maflcko commented at 7:00 AM on June 13, 2025: member

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

Also, missing tests?

0xB10C commented at 9:17 AM on June 13, 2025: contributor

I'm frequently using something like bitcoin-cli getpeerinfo | jq '[.[] | select(.id == 24430)]' for this. Doesn't work in the GUI console though.

waketraindev force-pushed on Jun 13, 2025

waketraindev commented at 1:20 PM on June 13, 2025: contributor

Added test coverage with nodeid filter in test_getpeerinfo, including a check for nonexistent ids. Let me know if you'd prefer this in a separate test method or different cases

waketraindev force-pushed on Jun 13, 2025

DrahtBot added the label CI failed on Jun 13, 2025

DrahtBot commented at 1:22 PM on June 13, 2025: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/44049534572 LLM reason (✨ experimental): The CI failure is caused by lint errors, specifically a trailing whitespace issue detected during the lint check.

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

waketraindev force-pushed on Jun 13, 2025

waketraindev requested review from maflcko on Jun 13, 2025

DrahtBot removed the label CI failed on Jun 13, 2025

in test/functional/rpc_net.py:192 in 53c5ce915c outdated

 187 | +            assert_equal(filtered[0]["id"], peer["id"])
 188 | +            self.log.debug("Filtered peer id {}: OK".format(peer["id"]))
 189 | +
 190 | +        self.log.info("Check getpeerinfo with nonexistent nodeid filter")
 191 | +        nonexistent_id = max(p["id"] for p in all_peers) + 1000
 192 | +        assert_equal(self.nodes[0].getpeerinfo(nonexistent_id), [])

kevkevinpal commented at 2:08 PM on June 15, 2025:

Would it make sense to throw a JSONRPCError(RPC_MISC_ERROR,"Peer does not exist") if the peer does not exist, Similar to the getblockfrompeer instead of sending back an empty array?

where error is thrown in PeerManagerImpl::FetchBlock

waketraindev commented at 3:08 PM on June 15, 2025:

I'd lean toward keeping this behavior for consistency unless there's a broader plan to split these use cases into distinct RPCs (say: getpeerbyid). The name getpeerinfo is arguably a bit misleading since it returns a list of peers, but given that it's established, returning an empty array when filtering seems appropriate to me.

kevkevinpal commented at 1:28 PM on June 19, 2025:

If others think we should return an empty array then sounds good to me!

waketraindev force-pushed on Jun 15, 2025

waketraindev commented at 11:21 PM on June 15, 2025: contributor

Small touch up in code style, renamed parameter to peer_id for consistency

danielabrozzoni commented at 10:11 AM on June 24, 2025: member

Concept ACK, I think this is a good addition, as introducing a peer_id parameter to filter the getpeerinfo response is useful for debugging.

This needs a release note :)

in src/rpc/net.cpp:311 in af1c1df25a outdated

 307 | @@ -300,6 +308,9 @@ static RPCHelpMan getpeerinfo()
 308 |          obj.pushKV("session_id", stats.m_session_id);
 309 |  
 310 |          ret.push_back(std::move(obj));
 311 | +        if (opt_peer_id && stats.nodeid == *opt_peer_id) {

luke-jr commented at 5:44 PM on June 30, 2025:

Only need to check opt_peer_id here, since we'd never get here if it's set to something different

waketraindev commented at 3:22 PM on July 4, 2025:

First condition checks if the parameter is defined so it returns all results even if peer_id is 0

luke-jr commented at 3:31 AM on July 15, 2025:

?

in src/rpc/net.cpp:126 in af1c1df25a outdated

 121 | @@ -122,7 +122,9 @@ static RPCHelpMan getpeerinfo()
 122 |      return RPCHelpMan{
 123 |          "getpeerinfo",
 124 |          "Returns data about each connected network peer as a json array of objects.",
 125 | -        {},
 126 | +        {
 127 | +            {"peer_id", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "Only return info for the specified peer."}

luke-jr commented at 5:45 PM on June 30, 2025:

Since we have to iterate peers to find the right one, maybe this should be a list (unordered_set in C++)

waketraindev commented at 3:14 PM on July 15, 2025:

Since we have to iterate peers to find the right one, maybe this should be a list (unordered_set in C++)

I agree, the last push swapped to an unordered_set with an array argument

in src/rpc/net.cpp:124 in af1c1df25a outdated

 121 | @@ -122,7 +122,9 @@ static RPCHelpMan getpeerinfo()
 122 |      return RPCHelpMan{
 123 |          "getpeerinfo",
 124 |          "Returns data about each connected network peer as a json array of objects.",

rkrux commented at 2:55 PM on July 3, 2025:

Can mention in the RPC help.

- "Returns data about each connected network peer as a json array of objects.",
+ "Returns data about each connected network peer as a json array of objects.\n"
+ "Optionally filter by peer index.\n",

waketraindev commented at 3:21 PM on July 4, 2025:

Added line to help text, preferred using peer id naming instead of index, to avoid confusion with the array index

rkrux commented at 3:09 PM on July 3, 2025: contributor

Concept ACK

Useful addition, I can see myself using the filtering here.

As mentioned in #32741#pullrequestreview-2953107634, a release note would be helpful as per this: https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#release-notes

waketraindev force-pushed on Jul 4, 2025

DrahtBot added the label CI failed on Jul 4, 2025

DrahtBot commented at 12:42 PM on July 4, 2025: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/45364101888 LLM reason (✨ experimental): Lint check failed due to a coding style error in rpc_net.py caused by an unnecessary semicolon.

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

DrahtBot removed the label CI failed on Jul 4, 2025

waketraindev commented at 3:23 PM on July 4, 2025: contributor

Rebased on top of master, updated help text

waketraindev renamed this:
~~rpc: add optional nodeid param to filter getpeerinfo~~
rpc: add optional peer_id param to filter getpeerinfo
on Jul 15, 2025

waketraindev force-pushed on Jul 15, 2025

waketraindev renamed this:
~~rpc: add optional peer_id param to filter getpeerinfo~~
rpc: add optional peer_ids param to filter getpeerinfo
on Jul 15, 2025

waketraindev force-pushed on Jul 15, 2025

waketraindev commented at 3:34 PM on July 15, 2025: contributor

Updated to use peer_ids (array) instead of a single peer_id. This allows filtering by multiple peers in one call and aligns better with RPC design. Functional tests adjusted.

luke-jr commented at 8:16 PM on July 15, 2025: member

I would do peer_id|peer_ids and accept both a single number (most common use) or an array (efficient).

waketraindev force-pushed on Jul 15, 2025

waketraindev commented at 8:56 PM on July 15, 2025: contributor

I would do peer_id|peer_ids and accept both a single number (most common use) or an array (efficient).

I don't think RPCArg currently supports a single argument that can be either a num or an array (Maybe RPCArg::Type::ANY?). One option would be to add two separate optional arguments: one for a single ID and another for the list, then merge both into the filter set internally. That would allow getpeerinfo 7 for convenience and getpeerinfo -1 '[1,3,4,7]' for multiple.

Figured it out in the latest push, skipped type checking for peer_ids

waketraindev force-pushed on Jul 15, 2025

waketraindev commented at 11:07 PM on July 15, 2025: contributor

I would do peer_id|peer_ids and accept both a single number (most common use) or an array (efficient).

Skipped type checking and added simple cases so getpeerinfo 7 and getpeerinfo '[1,2,3]' should work

waketraindev force-pushed on Aug 5, 2025

DrahtBot added the label CI failed on Aug 5, 2025

DrahtBot commented at 10:58 AM on August 5, 2025: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/47411071680 LLM reason (✨ experimental): The CI failure caused by a lint check for a missing trailing newline in the release notes markdown file.

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

DrahtBot removed the label CI failed on Aug 5, 2025

in src/rpc/net.cpp:133 in 95eb2c01d1 outdated

 130 | +                "A JSON array of peer IDs, or a single peer ID as a number.\n"
 131 | +                "If omitted, returns all peers.\n",
 132 | +                {
 133 | +                    {"peer_id", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "Peer ID"},
 134 | +                },
 135 | +                {RPCArgOptions{.skip_type_check = true}}

luke-jr commented at 11:59 AM on August 22, 2025:

                {RPCArgOptions{.skip_type_check = true, .type_str = {"", "numeric or array"}}}

waketraindev commented at 6:18 AM on August 23, 2025:

Added to changeset

in src/rpc/net.cpp:127 in 95eb2c01d1 outdated

 120 | @@ -121,8 +121,18 @@ static RPCHelpMan getpeerinfo()
 121 |  {
 122 |      return RPCHelpMan{
 123 |          "getpeerinfo",
 124 | -        "Returns data about each connected network peer as a json array of objects.",
 125 | -        {},
 126 | +        "Returns data about each connected network peer as a json array of objects.\n"
 127 | +        "Optionally filter by peer ids.\n",
 128 | +        {
 129 | +            {"peer_ids", RPCArg::Type::ARR, RPCArg::Optional::OMITTED,

luke-jr commented at 12:00 PM on August 22, 2025:

            {"peer_id|peer_ids", RPCArg::Type::ARR, RPCArg::Optional::OMITTED,

waketraindev commented at 6:25 AM on August 23, 2025:

RPCArg doesn't support '|' in names and will throw a rpc error when it gets called. https://github.com/bitcoin/bitcoin/blob/73220fc0f958f9b65f66cf0cf042af220b312fc6/src/rpc/util.cpp#L923

luke-jr changes_requested

waketraindev force-pushed on Aug 23, 2025

waketraindev closed this on Sep 24, 2025

waketraindev reopened this on Oct 9, 2025

waketraindev force-pushed on Oct 14, 2025

waketraindev commented at 8:49 PM on October 14, 2025: contributor

Rebased on current master.

waketraindev requested review from rkrux on Oct 17, 2025

waketraindev requested review from kevkevinpal on Oct 17, 2025

waketraindev requested review from danielabrozzoni on Oct 17, 2025

waketraindev requested review from luke-jr on Oct 17, 2025

in test/functional/rpc_net.py:203 in 599628ab9c outdated

 198 | +
 199 | +        self.log.info("Check getpeerinfo with nonexistent peer_id")
 200 | +        nonexistent_id = max(p["id"] for p in all_peers) + 1000
 201 | +        assert_equal(node.getpeerinfo(nonexistent_id), [])
 202 | +        assert_raises_rpc_error(-8, "must be a number or an array of numbers", node.getpeerinfo, {})
 203 | +

rkrux commented at 7:58 AM on October 21, 2025:

Nit if retouched:

diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py
index 209e804021..47834c8375 100755
--- a/test/functional/rpc_net.py
+++ b/test/functional/rpc_net.py
@@ -200,6 +200,7 @@ class NetTest(BitcoinTestFramework):
         nonexistent_id = max(p["id"] for p in all_peers) + 1000
         assert_equal(node.getpeerinfo(nonexistent_id), [])
         assert_raises_rpc_error(-8, "must be a number or an array of numbers", node.getpeerinfo, {})
+        assert_equal(len(node.getpeerinfo([ids[0], nonexistent_id])), 1)
 
         no_version_peer.peer_disconnect()
         self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 2)

rkrux commented at 7:58 AM on October 21, 2025: contributor

crACK 599628ab9c23cd53a30fcbcb5d4ac370bb536547

waketraindev force-pushed on Oct 22, 2025

waketraindev requested review from rkrux on Oct 24, 2025

rkrux approved

rkrux commented at 7:58 AM on October 25, 2025: contributor

ACK 8471074

waketraindev renamed this:
~~rpc: add optional peer_ids param to filter getpeerinfo~~
rpc: Add optional peer_ids param to filter getpeerinfo
on Nov 3, 2025

DrahtBot added the label Needs rebase on Nov 18, 2025

waketraindev force-pushed on Nov 18, 2025

DrahtBot added the label CI failed on Nov 18, 2025

DrahtBot removed the label CI failed on Nov 19, 2025

DrahtBot removed the label Needs rebase on Nov 19, 2025

rpc: Add optional peer_ids param to filter getpeerinfo 9b6090d8d9

waketraindev force-pushed on Nov 19, 2025

waketraindev commented at 11:00 AM on November 19, 2025: contributor

the commit message is wrong?

Thanks. Must've replaced it by mistake when I rebased. Fixed now

in src/rpc/net.cpp:125 in 9b6090d8d9

 120 | @@ -121,8 +121,18 @@ static RPCHelpMan getpeerinfo()
 121 |  {
 122 |      return RPCHelpMan{
 123 |          "getpeerinfo",
 124 | -        "Returns data about each connected network peer as a json array of objects.",
 125 | -        {},
 126 | +        "Returns data about each connected network peer as a json array of objects.\n"
 127 | +        "Optionally filter by peer ids.\n",

danielabrozzoni commented at 6:06 PM on December 11, 2025:

nit if retouched: "peer IDs" (so it stays consistent with the rest of the help text)

danielabrozzoni commented at 6:13 PM on December 11, 2025: member

tACK 9b6090d8d985f3ce61eaa587511bbeeeedf28cb9

I left a micro-nit, but I'd say avoid fixing it unless you already need to update, so you don't invalidate the ack :)

DrahtBot requested review from rkrux on Dec 11, 2025

rkrux approved

rkrux commented at 2:41 PM on December 17, 2025: contributor

lgtm re-ACK 9b6090d8d985f3ce61eaa587511bbeeeedf28cb9

chriszeng1010 commented at 10:42 PM on March 8, 2026: none

Tested Ack

2026-03-08T22:42:05.369198Z TestFramework (INFO): Test getpeerinfo
2026-03-08T22:42:08.423239Z TestFramework (INFO): Check getpeerinfo output before a version message was sent
2026-03-08T22:42:08.476781Z TestFramework (INFO): Check getpeerinfo filtering: all, single, multiple
2026-03-08T22:42:08.479393Z TestFramework (INFO): Check getpeerinfo with nonexistent peer_id

sedited removed review request from kevkevinpal on Mar 19, 2026

sedited requested review from stickies-v on Mar 19, 2026

stickies-v commented at 1:52 PM on March 19, 2026: contributor

Concept NACK. This seems easy enough to address client-side such as withjq as already suggested earlier. If there are use cases I'm missing, please document them in OP.

waketraindev commented at 2:50 PM on March 19, 2026: contributor

Concept NACK. This seems easy enough to address client-side such as withjq as already suggested earlier. If there are use cases I'm missing, please document them in OP.

As mentioned jq and command line tools don't work in the qt console. And generally it's just bad to serialize and send the whole peer list when you're only interested in specific ones. I'm sure in a lot of cases it's easier to depend on command line tools but that doesn't mean you shouldn't have targeted fetches.

Anyway cheers thanks for the 'review'

stickies-v commented at 3:43 PM on March 19, 2026: contributor

And generally it's just bad to serialize and send the whole peer list when you're only interested in specific ones

If/when serialization overhead becomes meaningful, sure, it can make sense to add server-side filters. Until we're there, this is just unnecessary complexity.

As mentioned jq and command line tools don't work in the qt console.

That sounds like a gui / console issue more than an rpc issue? If the rpc is not the blocker, we shouldn't change the rpc.

Anyway cheers thanks for the 'review'

I'm not sure being dismissive of people spending time on your PR is going to be helpful for your making progress.

waketraindev commented at 3:56 PM on March 19, 2026: contributor

And generally it's just bad to serialize and send the whole peer list when you're only interested in specific ones

If/when serialization overhead becomes meaningful, sure, it can make sense to add server-side filters. Until we're there, this is just unnecessary complexity.

Serialization overhead is probably the only 'meaningful' bit out of all the get peer info functionality there is in the code. (Outside of being subjective) In both cpu time, memory and transmission if there is any.

As mentioned jq and command line tools don't work in the qt console.

That sounds like a gui / console issue more than an rpc issue? If the rpc is not the blocker, we shouldn't change the rpc.

Anyway cheers thanks for the 'review'

I'm not sure being dismissive of people spending time on your PR is going to be helpful for your making progress.

I wasn't being dismissive of you 'spending time' on 'my pr'. I was being thankful for your suggestion of using jq and depend on command line utilities instead of having it filtered server side. I'm not sure being passive aggressive in a PR review is going to be helpful to me, muh progress, or yours.

Again thanks for spending the time to write a review and a thoughtful suggestion!

maflcko commented at 4:20 PM on March 19, 2026: member

The GUI has a special peers tab, and the RPC console is only for expert users.

If the peers tab is lacking some feature, it may be better to submit the feature to the gui repo.

I don't mind the changes here, but I am ~0 on them.

waketraindev commented at 4:42 PM on March 19, 2026: contributor

The GUI has a special peers tab, and the RPC console is only for expert users.

If the peers tab is lacking some feature, it may be better to submit the feature to the gui repo.

I don't mind the changes here, but I am ~0 on them.

This is what I'm using and maintaining for my own builds. It covers my gui, cli, rpc, tracing uses. I don't intend to write specialized gui coverage or submit this to the gui repo as it's a rpc change.

If you don't use or need any of the features it's fine to be ~0 on them.

naiyoma commented at 12:13 PM on March 23, 2026: contributor

Reviewed the changes, and the filter works correctly, returning an empty list if the peer id is not found. My hesitation is around the precedent. I think this type of filtering makes more sense for larger, unbounded datasets. For getpeerinfo, which returns at most 125 peers, the cost seems small to me. That said, I am not strongly opposed. Though I’m not sure where the line is. If we add this, the same argument applies to other read RPCs, like listunspent, (filter with txid etc...)

sedited commented at 7:45 AM on April 20, 2026: contributor

This is useful when using bitcoin-cli or the console to inspect specific peers referenced in debug.log.

Next to the hesitations voiced already, I'm also not sure if this is a strong reason. If you are already combing through the debug log, I think the additional pain of using some kind of piped filter for your data is pretty minimal. I doubt there is a large overlap of users sifting through the logs, but not capable of using a command line tool or pasting the output from the debug console into an llm chat box.

waketraindev commented at 8:54 AM on April 20, 2026: contributor

This is useful when using bitcoin-cli or the console to inspect specific peers referenced in debug.log.

Next to the hesitations voiced already, I'm also not sure if this is a strong reason. If you are already combing through the debug log, I think the additional pain of using some kind of piped filter for your data is pretty minimal. I doubt there is a large overlap of users sifting through the logs, but not capable of using a command line tool or pasting the output from the debug console into an llm chat box.

Pretty useful for me, I don't even have jq built in my system. But thanks for the feedback.