cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency #26988

stratospher · 2023-01-29T21:25:28Z

Rework of -addrinfo CLI is done using getaddrmaninfo RPC proposed in #27511. This would be useful for users who want to know the total number of addresses the node knows about and can make connections to.

Currently, -addrinfo returns total number of addresses the node knows about after filtering them for quality + recency using isTerrible. However isTerribleaddresses don't matter when selecting outbound peers to connect to. Total number of addresses the node knows about could be higher than what -addrinfo currently displays. See #24370.

DrahtBot · 2023-01-29T21:25:31Z

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/26988.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	pablomartin4btc
Concept ACK	mzumsande, brunoerg
Approach ACK	jonatack, sipa
Stale ACK	amitiuttarwar, vasild

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

Conflicts

No conflicts as of last run.

LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

[JSONRPCRequestObj("getaddrmaninfo", NullUniValue, 1)] in src/bitcoin-cli.cpp

^2025-12-13

mzumsande

Concept ACK, will review soon.

test/functional/rpc_net.py

amitiuttarwar · 2023-02-06T18:54:28Z

obvious concept ack considering I proposed #26907

mzumsande

Looks good to me, only minor nits.

ping @jonatack, could you have a look, especially at the changes to bitcoin-cli?

src/rpc/net.cpp

src/bitcoin-cli.cpp

src/rpc/net.cpp

brunoerg

Concept ACK

src/rpc/net.cpp

brunoerg · 2023-02-22T13:23:51Z

"network" : "str",    (string) The network (ipv4, ipv6, onion, i2p, cjdns)

Instead of being a string, wouldn't make sense it to be an array? E.g. I want to get new/tried table address count for ipv4 and ipv6 together.

stratospher · 2023-03-07T19:45:11Z

Instead of being a string, wouldn't make sense it to be an array? E.g. I want to get new/tried table address count for ipv5 and ipv6 together.

i'm confused about this since making it into RPCArg::Type::ARR would make the RPC harder to type and use?
$ bitcoin-cli getaddrmaninfo "[\"ipv4\", \"ipv6\"]"

and there are only few network types. would be interested in what others think.

amitiuttarwar · 2023-03-09T03:50:18Z

to minimize the complexity that we need to maintain over time, we try to reduce the amount of client-side niftiness that we offer. it seems to me that the string would offer all the information needed for a client to write a simple query if they wanted the sum of multiple. agreed that the syntax is another challenge of the array type

so I am +1 to leaving as is

brunoerg · 2023-03-09T14:11:04Z

that was just a question, thinking about complexity I agree on leaving it as is.

going to review in depth soon.

amitiuttarwar · 2023-03-09T19:01:12Z

light code review ACK 7c34c35b47. tested that the RPC and cli endpoints make sense & handle errors reasonably. these changes will require release notes, which can be done here or in a separate PR.

stratospher · 2025-11-20T15:32:43Z

thank you for the reviews! I've pushed an update changing the long key names.
also retained the original key name in -addrinfo's result (addresses_known) in case some other scripts/tools rely on it.

$ build/bin/bitcoin-cli -addrinfo
{
  "addresses_known": {
    "ipv4": 52037,
    "ipv6": 8855,
    "onion": 3465,
    "i2p": 0,
    "cjdns": 0,
    "total": 64357
  },
  "addresses_filtered": {
    "ipv4": 41811,
    "ipv6": 5808,
    "onion": 2465,
    "i2p": 0,
    "cjdns": 0,
    "total": 50084
  }
}

regarding #26988 (comment):

25.x, 26.x and 27.x are all end-of-life; so that seems out of scope for needing to test / accomodate code-wise?

Good point. Thinking of changing it in the next push.

According to bitnodes.io - among bitcoin core nodes, 95% are already on v26+ and therefore support getaddrmaninfo (introduced 2 years ago).

Maybe the concern regarding older node <-> newer CLI compatibility isn't there anymore considering how much the network/software has changed! (cc @jonatack, curious to hear your thoughts and anyone else's of course)

95% of bitcoin core nodes are on v26+ and have access to getaddrmaninfo (introduced 2 years ago)
the nodes we're concerned about breaking user space (v22 - v26 nodes) with have reached EOL

jonatack · 2025-11-20T18:29:56Z

CLI calls can be made to a node running a different, older version. If a researcher, data gatherer, developer, or node operator is calling this on a pre-getaddrmaninfo node, then the call should still work. As commented above, colleagues here requested that -netinfo continue to work in such cases, and we patched it. There doesn't seem to be any reason not to do the same here, rather than choosing to break it.

jonatack · 2025-11-20T18:39:55Z

src/bitcoin-cli.cpp

        uint64_t total{0}; // Total address count
        for (size_t i = 1; i < NETWORKS.size() - 1; ++i) {
            addresses.pushKV(NETWORKS[i], counts.at(i));
            total += counts.at(i);
        }
        addresses.pushKV("total", total);
-        result.pushKV("addresses_known", std::move(addresses));
+        result.pushKV("addresses_filtered", std::move(addresses));


I agree with preferably keeping the field names short, but the data corresponding to "addresses_known" should not change.

Perhaps use "addresses_unfiltered" or "addresses_not_filtered_for_quality_and_recency" for the new field.

As this is a CLI output, as opposed to an RPC one, its output presentation can be more optimized for human use (see -getinfo, for instance) while providing consistent historical data.

fanquake · 2025-11-20T18:41:13Z

CLI calls can be made to a node running a different, older version. If a researcher, data gatherer, developer, or node operator is calling this on a pre-getaddrmaninfo node,

Could you elaborate on why these people, particularly the developers, or node operators, are running EOL versions of Core (with known CVEs); and why they can't upgrade their nodes, but at the same time, they are downloading and using the latest version of bitcoin-cli?

There doesn't seem to be any reason not to do the same here, rather than choosing to break it.

The reason to not do it, is that it then implies we are still supporting versions of our software, which we have marked as EOL.

mzumsande

CLI calls can be made to a node running a different, older version. If a researcher, data gatherer, developer, or node operator is calling this on a pre-getaddrmaninfo node, then the call should still work. As commented above, colleagues here requested that -netinfo continue to work in such cases, and we patched it. There doesn't seem to be any reason not to do the same here, rather than choosing to break it.

I don't agree with that. I don't see bitcoin-cli and bitcoind as completely separate programs that should be supported in arbitrary combinations for eternity.

If that was the case, it would have been a bad mistake to ever have added non-trivial functionality like -addrinfo or -netinfo to bitcoin-cli instead of bitcoind in the first place because that would have created an infinite obligation to maintain compatibility of current bitcoind versions with ancient versions of bitcoin-cli long out of support.

While it makes some limited sense to try to keep up compatibility over version ranges supported at the same time (although even that is debatable in my opinion), I don't think that it is reasonable to make any efforts supporting compatibility beyond this.

currently -addrinfo returns addresses known to the node after filtering for quality and recency. However, the node considers all known addresses (even the filtered out addresses) when selecting peers to connect to. So update -addrinfo to also display the full set of known addresses for more useful node information.

stratospher · 2025-11-24T12:06:49Z

I pushed an update. I also don't think we should implement a fallback to support a rare usecase for few individuals. I've run into a few occasions where I used older bitcoind and newer cli but it's when I misconfigure something accidentally and wasn't the behaviour I intended. I guess this is what most normal users would run into.

This is why I think we shouldn't implement a fallback:

since -addrinfo is meant to be a human facing CLI dashboard (as implied in the documentation everywhere) and not an RPC endpoint (which other software/tools use programatically), the human facing CLI dashboard does not require a deprecation path (unlike RPCs). Humans can read and respond to it. It's not meant to used in automated scripts (those should use RPCs directly) and only meant to be read by humans.
if different versions of bitcoind and bitcoin-cli are used - it's the responsibility of the user to do proper package management. bitcoind and bitcoin-cli are versioned and released together.
This feels like an edge case for very few individuals. The transition period is over and 95% of bitcoin core nodes are already on v26+. If someone is capable of downloading the latest bitcoin-cli, they can upgrade their node/use an older bitcoin-cli.
getaddrmaninfo support is there for 95% of bitcoin core nodes that are already on v26+
v22-v27 are all EOL with known security vulnerabilities (from this comment)
it would be contradictory to state in the release notes that we are maintaining compatibility solely for EOL versions only for the human facing CLI dashboard, when we do not maintain EOL support anywhere else.

So my preferred solution is to give human node operators a clear + helpful message explaining the issue and asking them to use an older CLI or newer bitcoind to which they can choose. A clear message is more helpful than silently falling back.

@jonatack, thank you for your feedback and I'm sorry there's a difference in opinion + this is dragging out so long.
I hope the reasons stated here and the comments (1 and 2) above make sense to you. I appreciate your time spent discussing this, and I hope the reasoning above contributes to finding the best long-term solution!

doc/release-notes-26988.md

sipa · 2025-12-08T15:34:45Z

Concept/Approach ACK. I think it's fine to break compatibility with unmaintained node versions, given that there is a good error message for this case.

vasild

ACK 5b05a9959f1633bfee78d9edb180c672b0640ab5

pablomartin4btc

tACK 5b05a9959f1633bfee78d9edb180c672b0640ab5

with bitcoind < v26:

/build/bin/bitcoin-cli -signet -datadir=/tmp/btc -addrinfo
error: -addrinfo requires bitcoind v26.0 or later which supports getaddrmaninfo RPC. Please upgrade your node or use bitcoin-cli from the same version.

vasild

ACK 325f98bca07c3a0c48a46dc18f67376d44c5341d

doc/release-notes-26988.md

pablomartin4btc

ACK 325f98bca07c3a0c48a46dc18f67376d44c5341d

(Left a small suggestion since the older node may be operated by someone other than the CLI user, and upgrading it might not be possible or appropriate when this change is the only reason. I think this would also align with @jonatack’s earlier note.)

pablomartin4btc

re-ACK b3046cc

stratospher mentioned this pull request Jan 29, 2023

Expose addrman size info via RPC #26907

Closed

stratospher force-pushed the rpc_addrmaninfo branch 2 times, most recently from cac08f2 to caab213 Compare January 30, 2023 20:41

mzumsande reviewed Jan 30, 2023

View reviewed changes

test/functional/rpc_net.py Outdated Show resolved Hide resolved

DrahtBot mentioned this pull request Feb 2, 2023

cli: include local ("unroutable") peers in -netinfo table #26584

Merged

stratospher force-pushed the rpc_addrmaninfo branch from caab213 to c645fb6 Compare February 6, 2023 18:31

mzumsande reviewed Feb 6, 2023

View reviewed changes

src/rpc/net.cpp Outdated Show resolved Hide resolved

src/bitcoin-cli.cpp Outdated Show resolved Hide resolved

stratospher force-pushed the rpc_addrmaninfo branch from c645fb6 to 35bd55c Compare February 7, 2023 08:58

fanquake changed the title ~~[rpc]: Add test-only RPC addrmaninfo for new/tried table address count~~ rpc: Add test-only RPC addrmaninfo for new/tried table address count Feb 7, 2023

DrahtBot added the RPC/REST/ZMQ label Feb 7, 2023

amitiuttarwar reviewed Feb 8, 2023

View reviewed changes

src/rpc/net.cpp Outdated Show resolved Hide resolved

DrahtBot added the Needs rebase label Feb 15, 2023

stratospher force-pushed the rpc_addrmaninfo branch from 35bd55c to a04bfa3 Compare February 21, 2023 20:38

DrahtBot removed the Needs rebase label Feb 21, 2023

stratospher changed the title ~~rpc: Add test-only RPC addrmaninfo for new/tried table address count~~ rpc: Add test-only RPC getaddrmaninfo for new/tried table address count Feb 22, 2023

brunoerg reviewed Feb 22, 2023

View reviewed changes

src/rpc/net.cpp Outdated Show resolved Hide resolved

stratospher force-pushed the rpc_addrmaninfo branch from a04bfa3 to 7c34c35 Compare March 7, 2023 19:34

DrahtBot removed the CI failed label Nov 20, 2025

jonatack reviewed Nov 20, 2025

View reviewed changes

mzumsande reviewed Nov 20, 2025

View reviewed changes

stratospher force-pushed the rpc_addrmaninfo branch from 858f49b to 5b05a99 Compare November 24, 2025 11:54

sipa reviewed Dec 8, 2025

View reviewed changes

doc/release-notes-26988.md Outdated Show resolved Hide resolved

vasild approved these changes Dec 9, 2025

View reviewed changes

DrahtBot requested review from amitiuttarwar, jonatack, mzumsande, pablomartin4btc and sipa December 9, 2025 14:43

pablomartin4btc reviewed Dec 11, 2025

View reviewed changes

stratospher force-pushed the rpc_addrmaninfo branch from 5b05a99 to 325f98b Compare December 12, 2025 05:14

vasild approved these changes Dec 12, 2025

View reviewed changes

DrahtBot requested a review from pablomartin4btc December 12, 2025 06:35

pablomartin4btc reviewed Dec 12, 2025

View reviewed changes

doc/release-notes-26988.md Outdated Show resolved Hide resolved

DrahtBot requested a review from pablomartin4btc December 12, 2025 18:56

pablomartin4btc reviewed Dec 12, 2025

View reviewed changes

doc: add release notes for bitcoin#26988

b3046cc

stratospher force-pushed the rpc_addrmaninfo branch from 325f98b to b3046cc Compare December 13, 2025 03:21

pablomartin4btc approved these changes Dec 13, 2025

View reviewed changes

DrahtBot requested a review from vasild December 13, 2025 13:51

cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency #26988

Are you sure you want to change the base?

cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency #26988

Uh oh!

Conversation

stratospher commented Jan 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jan 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

Uh oh!

mzumsande left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amitiuttarwar commented Feb 6, 2023

Uh oh!

mzumsande left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brunoerg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brunoerg commented Feb 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stratospher commented Mar 7, 2023

Uh oh!

amitiuttarwar commented Mar 9, 2023

Uh oh!

brunoerg commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amitiuttarwar commented Mar 9, 2023

Uh oh!

stratospher commented Nov 20, 2025

Uh oh!

jonatack commented Nov 20, 2025

Uh oh!

jonatack Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

fanquake commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mzumsande left a comment

Choose a reason for hiding this comment

Uh oh!

stratospher commented Nov 24, 2025

Uh oh!

Uh oh!

sipa commented Dec 8, 2025

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

pablomartin4btc left a comment

Choose a reason for hiding this comment

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pablomartin4btc left a comment

Choose a reason for hiding this comment

Uh oh!

pablomartin4btc left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

stratospher commented Jan 29, 2023 •

edited

Loading

DrahtBot commented Jan 29, 2023 •

edited

Loading

brunoerg commented Feb 22, 2023 •

edited

Loading

brunoerg commented Mar 9, 2023 •

edited

Loading

fanquake commented Nov 20, 2025 •

edited

Loading