Misleading/inaccurate `{Is,Set}Reachable` naming #16919

#15138 dropped IsLimited in favor of IsReachable, which is misleading.

Historically, it was correctly pointed out back in #7553 that "We do not know that a class of Network is reachable, only that it is not." That PR introduced vfLimited, and was a nice solution to #7098 (a problem that was partly due to this confusion).

In fact, prior to #15138 there was a clue in a comment that might have shed light on this situation... The previous comment for IsReachable read https://github.com/bitcoin/bitcoin/blob/0ed279cb4e4198e20a552c279c09134626bbb0bd/src/net.cpp#L295-L299

Which was removed and replaced with https://github.com/bitcoin/bitcoin/blob/d6b076c17bc7d513243711563b262524ef0ba74c/src/net.h#L529-L530

The missing clue is that IsReachable returning true doesn't mean "the network is reachable", it only means that "we can probably connect to" the network, or in other words, that "we're not limited by any user setting or missing any requirements (e.g. tor daemon) to connect to the network".

In other words, "limited" is more accurate than "reachable".

Let's look at two snippets to see why "reachable" can be error-prone and inaccurate.

https://github.com/bitcoin/bitcoin/blob/7d4bc60f1fee785d355fe4d376c0a369fc64dd68/src/init.cpp#L1362-L1377

In the above code snippet, we set NET_ONION to be "Reachable" if we have any proxy at all, which doesn't make sense: the proxy might be just a normal SOCKS proxy, and we wouldn't be able to reach the Tor network.

However, if we were to use the concept of "limited" it makes more sense: we are not limited from the Tor network if there is a proxy present, as there is a larger than 0 chance that it is a Tor proxy we can use to connect to the Tor network. Conversely, if there were no proxy, we are "limited" from the Tor network by the fact that Bitcoin Core can't connect to Tor without a proxy.

https://github.com/bitcoin/bitcoin/blob/7d4bc60f1fee785d355fe4d376c0a369fc64dd68/src/init.cpp#L1339-L1352

Here, we are setting all known networks that are not specified by -onlynet to be not "reachable", but what's more a more accurate description is that these networks are "limited," in this case by a user setting.

What makes things even more confusing is our getnetworkinfo RPC help: https://github.com/bitcoin/bitcoin/blob/7d4bc60f1fee785d355fe4d376c0a369fc64dd68/src/rpc/net.cpp#L465-L466

When this is the actual way we calculate these values: https://github.com/bitcoin/bitcoin/blob/7d4bc60f1fee785d355fe4d376c0a369fc64dd68/src/rpc/net.cpp#L434-L435

I propose we keep only the concept of a network being "limited", which seems to be more accurate and less error-prone. We should also document this clearly.

I don't really like flipping this around again, but I agree your argument makes sense that 'limited' is a better description. The issue, from the point of the code, isn't whether a network is reachable, but whether it's permitted to connect to it.

How about IsMaybeReachable?

Closing, might come back to this at some point in the future.

Changing this seems out of reach, for now.