cli -addrinfo: drop torv2; torv3 becomes onion per GetNetworkName()

[jonatack]

#22050 removed torv2 support from 22.0. For 23.0 and subsequent releases, we can probably remove torv2 from -addrinfo.

before

  "addresses_known": {
    "ipv4": 58305,
    "ipv6": 5138,
    "torv2": 0,
    "torv3": 5441,
    "i2p": 14,
    "total": 68898
  }

after

  "addresses_known": {
    "ipv4": 58305,
    "ipv6": 5138,
    "onion": 5441,
    "i2p": 14,
    "total": 68898
  }

Per the naming of netbase.{h, cpp}::GetNetworkName(), torv3 becomes onion, which is what is printed in the output of getpeerinfo, getnetworkinfo and getnodeaddresses.

[practicalswift]

Concept ACK

[sipa]

Do you think "onion" is clearer than "tor"?

[jonatack]

I don't have a strong opinion, but in the absence of a need to distinguish between tor/onion v2 and v3, using the default naming of GetNetworkName(), getnetworkinfo, getpeerinfo, and getnodeaddresses seemed to make sense and is the simplest to implement.

[sipa]

@jonatack Ah, being consistent with existing RPCs is a good reason.

[jonatack]

(Given that "onion" is used by our Lightning Network colleagues as a more generic term, it may make sense to globally revert our language back to "tor" at some point.)

[laanwj]

Do you think "onion" is clearer than "tor"?

As Tor allows connectivity to IPv4 and IPv6 as well, it's less specific. This was always the reasoning to use "onion" specifically when dealing with hidden services. I don't think lightning also using the term is a good reason to change this.

(So concept ACK anyway)

[jonatack]

As Tor allows connectivity to IPv4 and IPv6 as well, it's less specific. This was always the reasoning to use "onion" specifically when dealing with hidden services. I don't think lightning also using the term is a good reason to change this.

(So concept ACK anyway)

Ah, thanks for the reminder why we did this.

[Zero-1729]

Concept ACK

[jonatack]

(Note to self, this needs an update to doc/tor.md and the release notes.)

[tryphe]

Concept ACK

"tor" seems maybe slightly more descriptive than "onion" but "onion" exists in the code more 🤷‍♂️

[laanwj]

"tor" seems maybe slightly more descriptive than "onion" but "onion" exists in the code more man_shrugging

The addresses are literally .onion addresses I don't understand why people suddenly don't find that descriptive enough anymore and want to change this around. "Tor hidden service" would work as welll if you really want to avoid that word, but just "Tor" doesn't mean anything as Tor also has exit nodes for plainnet and that's the most common use of Tor.

I guess a compromise could be torhs. But I had the impression that Tor themselves was also moving away from the "hidden service" naming e.g. https://community.torproject.org/onion-services/ doesn't even mention it. tor onion could work, but that might be unnecessary specicifc unless we plan on supporting some other onions, I don't think any other make sense in this context.

[jonatack]

Yes, I agree. Sorry for the mini-kerfuffle, I think "onion" is fine.

[tryphe]

The addresses are literally .onion addresses I don't understand why people suddenly don't find that descriptive enough anymore and want to change this around.

I wasn't saying I preferred one or the other, but rather I was neutral to the conversation above and didn't really mind either one.

[jonatack]

(Note to self, this needs an update to doc/tor.md and the release notes.)

Done. This should be ready now.

[Zero-1729]

tACK 49d503a 🧉

LGTM, patch works as described.

Updated Tor docs and release notes are clear and straightforward.

$ bitcoin-cli -addrinfo
{
  "addresses_known": {
    "ipv4": 39192,
    "ipv6": 3219,
    "onion": 11,
    "i2p": 0,
    "total": 42422
  }
}

[klementtan]

Code review and tested ACK 49d503a

{
  "addresses_known": {
    "ipv4": 83,
    "ipv6": 37,
    "onion": 30,
    "i2p": 0,
    "total": 150
  }
}

[practicalswift]

cr ACK 49d503a