Implement ADDRv2 support (part of BIP155) #19031

An implementation of BIP155 addrv2 messages. To ease review it is split in a few logical changes and submitted as separate, smaller, PRs.

The current one for review is: #19954

Preparation changes

Commits:

• test: add an edge case test for CSubNet (merged via #19351)
• net: improve encapsulation of CNetAddr (merged via #19360)
• net: document enum Network (merged via #19534)
• net: save the network type explicitly in CNetAddr (merged via #19534)

Change CNetAddr::ip to have flexible size

Commits:

• net: change CNetAddr::ip to have flexible size (merged via #19628)

Implement addrv2 (un)serializing

Add support to serialize CNetAddr and CAddress in ADDRv2 format. Invoke that from CAddrMan serialization methods. Commits:

• net: move HasPrefix() so it can be reused (merged via #19845)
• test: move HasReason() so it can be reused (merged via #19845)
• net: CNetAddr: add support to (un)serialize as ADDRv2 (merged via #19845)
• net: recognize TORv3/I2P/CJDNS networks (merged via #19845)
• net: CAddress & CAddrMan: (un)serialize as ADDRv2 (merged via #19954)

Advertise support

Advertise ADDRv2 support to peers, handle incoming ADDRv2 messages and send to peers in that format if they have advertised support. Commits:

• net: advertise support for ADDRv2 via new message (merged via #19954)

All the steps to get Tor v3 support are outlined in issue#18884 Tor v3 support, this PR is one of them.

Concept ACK :smile:

Am I correct in saying that you've replaced NetworkID in my implementation with Bip155NetworkId? (totally fine btw, just wanted to make sure)

Concept ACK!

Am I correct in saying that you've replaced NetworkID in my implementation with Bip155NetworkId? (totally fine btw, just wanted to make sure)

Yes, thanks for asking!

Ideally we would have just one enum for network types, not two. That would contain the network ids from BIP155 + the other ones we have now - unroutable, internal/name, etc. But the problem is that lots of code relies on the values being sequential - there are loops from 0 to NET_MAX. I did not wanted to plant e.g. "unroutable" as 0x07 just after CJDNS or to change those loops and assumptions because it would bloat this PR too much.

So I introduced a second enum but made it private inside CNetAddr and named it BIP155... to denote that it contains just the networks mentioned in BIP155 and nothing else.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #20028 (test: Check that invalid peer traffic is accounted for by MarcoFalke)
• #19889 (test: Add consistent formatting for messages.py repr methods by adaminsky)
• #19763 (net: don't try to relay to the address' originator by vasild)
• #19315 ([tests] Allow outbound & block-relay-only connections in functional tests. by amitiuttarwar)
• #19238 (refactor: Replace RecursiveMutex with Mutex in CAddrMan by hebasto)
• #18642 (Use std::chrono for the time to rotate destination of addr messages + tests by naumenkogs)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Concept ACK. WIll do a good review later.

Rebased.

Changed the implementation to use dedicated message sendaddrv2 for signaling support instead of a service bit. The code complexity is about the same, the dedicated message needs a few more lines, d85215213..9a526f22b:

 src/init.cpp                                |  2 +-
 src/net.h                                   |  3 ++-
 src/net_processing.cpp                      | 10 +++++++++-
 src/protocol.cpp                            |  3 ++-
 src/protocol.h                              |  8 ++++++--
 test/functional/p2p_leak.py                 |  1 +
 test/functional/p2p_node_network_limited.py |  4 ++--
 test/functional/test_framework/messages.py  | 18 +++++++++++++++++-
 test/functional/test_framework/mininode.py  |  3 +++
 9 files changed, 43 insertions(+), 9 deletions(-)

Rebased.

Rebased.

I reviewed the code and it looks sane.

I believe all the questionable parts about BIP155 are not included here? Making this PR more or less ready to review without high-level discussions.

We probably need much more eyes, there's just too much low level code changes.

I believe all the questionable parts about BIP155 are not included here? Making this PR more or less ready to review without high-level discussions.

This PR is a full implementation of ADDRv2. However:

• The maximum length of an address, mentioned in BIP155 is 32 bytes, whereas this PR has set it to 512 bytes.
• Signaling for ADDRv2 support is done by a protocol version bump in BIP155, whereas this PR does it by a new dedicated message sendaddrv2.

https://github.com/bitcoin/bips/pull/907 should align the code and the spec.

Concept ACK. At a high level the code looks well-done. Looked through the BIP and the previous PR.

I wonder if this would make faster progress if it was turned into a master roadmap PR and split into easier-to-review PRs similar to what we've seen with the serialization refactoring. (A possible key to consider in making that work, if you do it, may be to take the time to set a clear map from the start so as to not change it midway, have to backtrack, or invalidate in-progress review.)

I wonder if this would make faster progress if ... split into easier-to-review PRs ...

My thought exactly! Let's start with something small and see how it goes: #19351.

I updated the roadmap at: #18884, this PR can be split in:

Preparation cleanup

test: add an edge case test for CSubNet
net: improve encapsulation of CNetAddr
net: document `enum Network`
net: save the network type explicitly in CNetAddr

Change CNetAddr::ip to have flexible size

net: change CNetAddr::ip to have flexible size

Implement addrv2 (un)serializing

net: CAddress & CAddrMan: (un)serialize as ADDRv2
net: CNetAddr: add support to (un)serialize as ADDRv2
test: add CAddress serialization tests

Advertise support for addrv2 and start sending such messages to peers

net: advertise support for ADDRv2 via new message

Rebased.

Can you update the PR descriptions of this PR and 18884 with the roadmap and to indicate clearly which PRs correspond to which step and which PR of the 3 that are open should currently be reviewed? Review can be hard to obtain; best to make it easy and clear for your reviewers.

Rebased, removing commit test: add an edge case test for CSubNet which was merged via https://github.com/bitcoin/bitcoin/pull/19351

Can you update the PR descriptions...

Done. In summary - #18884 is a roadmap for Tor v3. It is just an issue, does not contain code and this PR is part of that roadmap. #19351 already got merged (Thanks!). I chopped off the next one at #19360. It is happening!

Rebased to restart CI

Could you take a look at the MSan/valgrind failures? :relaxed:

I simplified the code a little bit, now that CNetAddr::GetAddrBytes() has been added to master.

#19219 introduced a new method CNetAddr::GetAddrBytes() in master which does the same thing as CNetAddr::GetAddrKey() (introduced in this PR).

So, change this PR to use the existent CNetAddr::GetAddrBytes() and don't introduce a new method.

While testing https://github.com/bitcoin/bitcoin/commit/81780afb3b4575642f74b682327ff1c78b6efaa7 I saw that v3 are advertised as [::]:0 and ToString resulted also in [::]:0

2020-07-10T17:43:18Z sending feefilter (8 bytes) peer=0
2020-07-10T17:43:18Z trying connection [::]:0 lastseen=0.0hrs
2020-07-10T17:43:18Z SOCKS5 connecting [::]:0
2020-07-10T17:43:18Z Socks5() connect to [::]:0:18444 failed: general failure

With that Nit changes in the patch, The correct V3 long address is displayed and get an advertised reconnect at reboot.
With that minor changes I had some success and so far https://github.com/bitcoin/bitcoin/commit/81780afb3b4575642f74b682327ff1c78b6efaa7 LGTM

Hope that helps... Btw. if you git pull https://github.com/Saibato/bitcoin add-ed25519 on top you have v3 onion on the fly for testing. #19485

diff --git a/src/netaddress.cpp b/src/netaddress.cpp
index 1ac99ddd1..0171b1d80 100644
--- a/src/netaddress.cpp
+++ b/src/netaddress.cpp
@@ -21,6 +21,7 @@ CNetAddr::Bip155NetworkId CNetAddr::ToBIP155NetworkId() const
     case NET_ONION:
         switch (m_addr.size()) {
         case ADDR_TORv2_SIZE: return Bip155NetworkId::TORv2;
+        case ADDR_TORv3_SIZE: return Bip155NetworkId::TORv3;
         default: assert(!"Unexpected TOR address size");
         }
     case NET_UNROUTABLE:
@@ -58,6 +59,11 @@ bool CNetAddr::FromBIP155NetworkId(Bip155NetworkId bip155_network_id,
         }
         return false;
     case Bip155NetworkId::TORv3:
+        if (address_size == ADDR_TORv3_SIZE) {
+           net = NET_ONION;
+           return true;
+        }
+        return false;
     case Bip155NetworkId::I2P:
     case Bip155NetworkId::CJDNS:
         return false;
@@ -137,7 +143,7 @@ bool CNetAddr::SetSpecial(const std::string &strName)
 {
     if (strName.size()>6 && strName.substr(strName.size() - 6, 6) == ".onion") {
         std::vector<unsigned char> vchAddr = DecodeBase32(strName.substr(0, strName.size() - 6).c_str());
-        if (vchAddr.size() != ADDR_TORv2_SIZE)
+        if (vchAddr.size() != ADDR_TORv2_SIZE && vchAddr.size() != ADDR_TORv3_SIZE)
             return false;
         m_net = NET_ONION;
         m_addr = vchAddr;
diff --git a/src/netaddress.h b/src/netaddress.h
index 0a94f0587..baf7d29a2 100644
--- a/src/netaddress.h
+++ b/src/netaddress.h
@@ -81,6 +81,10 @@ static constexpr size_t ADDR_IPv6_SIZE = 16;
 /// Size of TORv2 address (in bytes).
 static constexpr size_t ADDR_TORv2_SIZE = 10;
 
+/// Size of TORv3 address (in bytes).
+static constexpr size_t ADDR_TORv3_SIZE = 35;
+
+
 /// Size of "internal" (NET_INTERNAL) address (in bytes).
 static constexpr size_t ADDR_INTERNAL_SIZE = 10;
 
@@ -259,10 +263,12 @@ class CNetAddr
                 memcpy(arr, m_addr.data(), m_addr.size());
                 break;
             case NET_ONION:
+                if (ToBIP155NetworkId() == Bip155NetworkId::TORv2) {
                 prefix_size = sizeof(TORv2_IN_IPv6_PREFIX);
                 assert(m_addr.size() + prefix_size == sizeof(arr));
                 memcpy(arr, TORv2_IN_IPv6_PREFIX, prefix_size);
                 memcpy(arr + prefix_size, m_addr.data(), m_addr.size());
+                }
                 break;
             case NET_INTERNAL:
                 prefix_size = sizeof(INTERNAL_IN_IPv6_PREFIX);

Can someone with label permissions add a Review club label to this PR?

Resource up soon at https://bitcoincore.reviews/19031.

@Saibato, thanks for testing this! To get this moving faster, maybe you can review #19360?

The changes you posted above and some of #19485 belong to a "Tor v3" implementation, which is out of the scope of this PR. It is the last item in the roadmap at #18884, the one that reads "Implement Tor v3 on top of that". I wonder if it would be too confusing to create a PR that is based on this PR (contains all of its commits) + more commits for Tor v3 implementation. I have been refraining from doing that as it could cause further confusion and more importantly, would be a distraction at this stage.

I think the focus now should be on reviewing and testing this PR - addrv2 support for existent networks (e.g. IPv4, IPv6, Torv2).

#19360

Last week i stumbled about this and i feel a bit sorry for you and the effort and work you put in this. that i looked not earlier in v3 support in bitcoin for the masses. I reviewed this as part of https://github.com/bitcoin/bitcoin/commit/2533ce03c71a865ba230b7ff770dd04cd8be5ea1 and although i stated LGTM and that your code works, the whole ADDRv2 thing is a full NACK and my view on c++ auto const and code tautologies and pointless erotic refactor you know already, i guess, when it comes to replace IP ( The address as of RFC 4291 https://tools.ietf.org/html/rfc4291 ) with something else non IP, and call it in the code ip, i am strong oppose that.

The way bitcoin piggy packs right now v2 onions in non routeable ipv6 or ipv4 in ipv6 seems to me the right way to do this for any extension. Since all those protocols are anyway socks5 kind of proxied FQDN i vote for add fqdn to CNetAddr for local storage and not to change CService and the network ADDR protocol

Any change at central points make me nervous, so please prove me wrong. And please don't tempt me with you cant do this or that because off impossible, i can and I will.

EDIT: nit when review, and dissam/ I like to mention here how critical it can be to have the correct byte order of ip.

Guess what friend of free uncensored internet registered 1.0.0.0/24 ? 0.0.1.in-addr.arpa. 3600 IN SOA jasmine.ns.cloudflare.com. dns.cloudflare.com

So if extensive use of library c++ voodoo, make absolutely sure to get i.e. the 127.0.0.1 assembled correctly in the ip <vector> after <<. it might otherwise resolve in an hopefully unexpected unintended way. :woman_shrugging:

@Saibato IMO sending IPv4, Tor or other addresses disguised as IPv6 addresses is a hack and it wastes space and bandwidth (it sends 16 bytes instead of e.g. 4). Furthermore this hack is limited because one cannot send addresses that are longer than 16 bytes.

ADDRv2 solves this in a clean way with the variable address length, allowing us to send longer addresses and also to avoid the 16-instead-of-4 bytes bloat.

Furthermore this hack is limited because one cannot send addresses that are longer than 16 bytes. @vasild at some point. yes and i dont say your work is void, in someway we got futher, but please lock at the diff, it can be done and i have done it.

Ok prob i should hash the seq and send a bit less bytes and do some checking but the patch works right away on top of all nodes out there AFAICS and can support any sort of address.

<details><summary>Code diff</summary>

diff --git a/src/addrman.h b/src/addrman.h
index 8e82020df..4a810e53f 100644
--- a/src/addrman.h
+++ b/src/addrman.h
@@ -61,6 +61,7 @@ public:
     SERIALIZE_METHODS(CAddrInfo, obj)
     {
         READWRITEAS(CAddress, obj);
+        READWRITEAS(CNetAddr, obj);
         READWRITE(obj.source, obj.nLastSuccess, obj.nAttempts);
     }
 
diff --git a/src/net.cpp b/src/net.cpp
index 244b0094d..36af03d33 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -156,6 +156,7 @@ static std::vector<CAddress> convertSeed6(const std::vector<SeedSpec6> &vSeedsIn
         struct in6_addr ip;
         memcpy(&ip, seed_in.addr, sizeof(ip));
         CAddress addr(CService(ip, seed_in.port), GetDesirableServiceFlags(NODE_NONE));
+        LogPrintf("seeds %s\n", CNetAddr(addr).ToString());
         addr.nTime = GetTime() - rng.randrange(nOneWeek) - nOneWeek;
         vSeedsOut.push_back(addr);
     }
@@ -368,6 +369,9 @@ static CAddress GetBindAddress(SOCKET sock)
     return addr_bind;
 }
 
+static CAddress getadrr(std::string name) {
+       return CAddress(CService(name,18444), NODE_NONE);
+}
 CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, bool manual_connection, bool block_relay_only)
 {
     if (pszDest == nullptr) {
@@ -384,16 +388,23 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
     }
 
     /// debug print
-    LogPrint(BCLog::NET, "trying connection %s lastseen=%.1fhrs\n",
-        pszDest ? pszDest : addrConnect.ToString(),
+    LogPrint(BCLog::NET, "trying connection %s unresolved address %s lastseen=%.1fhrs\n",
+        pszDest ? pszDest:"", addrConnect.ToString(),
         pszDest ? 0.0 : (double)(GetAdjustedTime() - addrConnect.nTime)/3600.0);
 
+    std::string strName = std::string( pszDest ? pszDest:"");
     // Resolve
     const int default_port = Params().GetDefaultPort();
     if (pszDest) {
         std::vector<CService> resolved;
         if (Lookup(pszDest, resolved,  default_port, fNameLookup && !HaveNameProxy(), 256) && !resolved.empty()) {
+
+       // dont resolv local the onions
+       if (!strName.find(".onion")) {
+
             addrConnect = CAddress(resolved[GetRand(resolved.size())], NODE_NONE);
+
+
             if (!addrConnect.IsValid()) {
                 LogPrint(BCLog::NET, "Resolver returned invalid address %s for %s\n", addrConnect.ToString(), pszDest);
                 return nullptr;
@@ -410,6 +421,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
                 LogPrintf("Failed to open new connection, already connected\n");
                 return nullptr;
             }
+            }
         }
     }
 
@@ -417,7 +429,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
     bool connected = false;
     SOCKET hSocket = INVALID_SOCKET;
     proxyType proxy;
-    if (addrConnect.IsValid()) {
+    if (addrConnect.IsValid() && addrConnect.IsTor() && !pszDest) {
         bool proxyConnectionFailed = false;
 
         if (GetProxy(addrConnect.GetNetwork(), proxy)) {
@@ -425,14 +437,21 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
             if (hSocket == INVALID_SOCKET) {
                 return nullptr;
             }
-            connected = ConnectThroughProxy(proxy, addrConnect.ToStringIP(), addrConnect.GetPort(), hSocket, nConnectTimeout, proxyConnectionFailed);
+
+        if (addrConnect.ToStringIP() != "") {
+                connected = ConnectThroughProxy(proxy, addrConnect.ToStringIP(), addrConnect.GetPort(), hSocket, nConnectTimeout, proxyConnectionFailed);
+        } else return nullptr;
+
         } else {
-            // no proxy needed (none set for target network)
+            // no proxy needed (none set for tastrName.find(".onion")rget network)
             hSocket = CreateSocket(addrConnect);
             if (hSocket == INVALID_SOCKET) {
                 return nullptr;
             }
-            connected = ConnectSocketDirectly(addrConnect, hSocket, nConnectTimeout, manual_connection);
+
+            if (addrConnect.ToString() != "") {
+                connected = ConnectSocketDirectly(addrConnect, hSocket, nConnectTimeout, manual_connection);
+            } else return nullptr;
         }
         if (!proxyConnectionFailed) {
             // If a connection to the node was attempted, and failure (if any) is not caused by a problem connecting to
@@ -444,14 +463,24 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
         if (hSocket == INVALID_SOCKET) {
             return nullptr;
         }
+
         std::string host;
         int port = default_port;
         SplitHostPort(std::string(pszDest), port, host);
         bool proxyConnectionFailed;
-        connected = ConnectThroughProxy(proxy, host, port, hSocket, nConnectTimeout, proxyConnectionFailed);
+        connected = false;
+        //proxyConnectionFailed = true;
+
+
+    if (strlen(pszDest) > 0) //strName.find(".onion"))
+        {
+        LogPrintf("Address to connect over proxz  %s %d\n", host, port);
+            connected = ConnectThroughProxy(proxy, host, port, hSocket, nConnectTimeout, proxyConnectionFailed);
+        } else return nullptr;
+
     }
     if (!connected) {
-        CloseSocket(hSocket);
+        if (hSocket) CloseSocket(hSocket);
         return nullptr;
     }
 
@@ -1705,17 +1734,6 @@ void CConnman::ThreadDNSAddressSeed()
     LogPrintf("%d addresses found from DNS seeds\n", found);
 }
 
-
-
-
-
-
-
-
-
-
-
-
 void CConnman::DumpAddresses()
 {
     int64_t nStart = GetTimeMillis();
@@ -2057,6 +2075,8 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
         }
     } else if (FindNode(std::string(pszDest)))
         return;
+    // will "":port ever work?
+    if(!pszDest && (addrConnect.ToStringIP() == "")) return;
 
     CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, manual_connection, block_relay_only);
 
@@ -2756,6 +2776,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
     // peers (to prevent adversaries from inferring these links from addr
     // traffic).
     m_addr_known{block_relay_only ? nullptr : MakeUnique<CRollingBloomFilter>(5000, 0.001)},
+
     id(idIn),
     nLocalHostNonce(nLocalHostNonceIn),
     nLocalServices(nLocalServicesIn),
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index bfc60b18f..ecd83188e 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2454,14 +2454,47 @@ void ProcessMessage(
         std::vector<CAddress> vAddrOk;
         int64_t nNow = GetAdjustedTime();
         int64_t nSince = nNow - 10 * 60;
+        int i = 0;
+        size_t len = 10;
+        CAddress Tor = CAddress(vAddr[0]);
+        char rawaddr[256] = {};
+
         for (CAddress& addr : vAddr)
         {
             if (interruptMsgProc)
                 return;
 
+        if (addr.IsTorSequence(i)) {
+            if (i == 0 ) {
+                // LogPrintf("Adrress addr V3 raw  rec =");
+                Tor = addr;
+            }
+            //V3sequence())
+            addr.SetSpecial_v3(addr, 256); //fill hostdata from ip;
+            memcpy(&rawaddr[i*10], &addr.fqdn.c_str()[0], 10);
+            i++;
+            if (i != 8) {
+                continue;
+            }
+        } else addr.fqdn = {};
+
+        //LogPrintf("\nAdrress add raw? %s\n",  &rawaddr[0]);
+
+        if (i == 8) {
+            Tor.SetSpecial_v3(addr, 255); // set clasic tor flag
+            Tor.fqdn = rawaddr;
+            if ( i == 8 ) Tor.fqdn = rawaddr;
+            addr = Tor;
+        }
+
+        if (addr.IsTor() && strlen(addr.fqdn.c_str()) < 32) addr.SetSpecial_v3(addr, 255); // v2 stzle
+        LogPrint(BCLog::NET, "Adrress add? %s\n",  addr.ToString());
+
+
             // We only bother storing full nodes, though this may include
             // things which we would not make an outbound connection to, in
             // part because we may make feeler connections to them.
+
             if (!MayHaveUsefulAddressDB(addr.nServices) && !HasAllDesirableServiceFlags(addr.nServices))
                 continue;
 
@@ -3901,12 +3934,25 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
             std::vector<CAddress> vAddr;
             vAddr.reserve(pto->vAddrToSend.size());
             assert(pto->m_addr_known);
+            CAddress addr_add;
             for (const CAddress& addr : pto->vAddrToSend)
             {
                 if (!pto->m_addr_known->contains(addr.GetKey()))
                 {
                     pto->m_addr_known->insert(addr.GetKey());
-                    vAddr.push_back(addr);
+                    if (!addr.IsTor()) vAddr.push_back(addr);
+                    if (addr.IsTor()) LogPrint(BCLog::NET, "Try to send onion address = %s\n", addr.fqdn);
+
+                    if (addr.IsTor() && strlen(addr.fqdn.c_str()) > 32) {
+                        for (int i=0;i<8;i++) {
+                            addr_add = addr;
+                            addr_add.SetSpecial_v3(addr, (i)*10);
+
+                            vAddr.push_back(addr_add);
+
+                        }
+                    } else  vAddr.push_back(addr); //v2 tor
+
                     // receiver rejects addr messages larger than 1000
                     if (vAddr.size() >= 1000)
                     {
diff --git a/src/netaddress.cpp b/src/netaddress.cpp
index 674439161..8c8bad0f8 100644
--- a/src/netaddress.cpp
+++ b/src/netaddress.cpp
@@ -5,13 +5,17 @@
 
 #include <cstdint>
 #include <netaddress.h>
+#include <netbase.h>
+
 #include <hash.h>
 #include <util/strencodings.h>
 #include <util/asmap.h>
 #include <tinyformat.h>
+#include <util/system.h>
 
 static const unsigned char pchIPv4[12] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff };
 static const unsigned char pchOnionCat[] = {0xFD,0x87,0xD8,0x7E,0xEB,0x43};
+static const unsigned char pchOnionSeq[] = {0xFD,'l','o','r','3',0x00};
 
 // 0xFD + sha256("bitcoin")[0:5]
 static const unsigned char g_internal_prefix[] = { 0xFD, 0x6B, 0x88, 0xC0, 0x87, 0x24 };
@@ -47,6 +51,11 @@ void CNetAddr::SetRaw(Network network, const uint8_t *ip_in)
     }
 }
 
+void CNetAddr::SetRawByte(char p , int pos)
+{
+    ip[6+pos] = p;
+}
+
 /**
  * Try to make this a dummy address that maps the specified name into IPv6 like
  * so: (0xFD + %sha256("bitcoin")[0:5]) + %sha256(name)[0:10]. Such dummy
@@ -70,6 +79,7 @@ bool CNetAddr::SetInternal(const std::string &name)
     CSHA256().Write((const unsigned char*)name.data(), name.size()).Finalize(hash);
     memcpy(ip, g_internal_prefix, sizeof(g_internal_prefix));
     memcpy(ip + sizeof(g_internal_prefix), hash, sizeof(ip) - sizeof(g_internal_prefix));
+    fqdn = name;
     return true;
 }
 
@@ -83,20 +93,51 @@ bool CNetAddr::SetInternal(const std::string &name)
  *
  * [@see](/github-metadata-backup-bitcoin-bitcoin/contributor/see/) CNetAddr::IsTor(), CNetAddr::IsRFC4193()
  */
-bool CNetAddr::SetSpecial(const std::string &strName)
+bool CNetAddr::SetSpecial(const std::string &strName, int flag)
 {
     if (strName.size()>6 && strName.substr(strName.size() - 6, 6) == ".onion") {
-        std::vector<unsigned char> vchAddr = DecodeBase32(strName.substr(0, strName.size() - 6).c_str());
-        if (vchAddr.size() != 16-sizeof(pchOnionCat))
+        std::vector<unsigned char> vchAddr =  DecodeBase32(strName.substr(flag*10, strName.size() - 6).c_str());
+        if (vchAddr.size() != 16-sizeof(pchOnionCat) && vchAddr.size() != 35)
             return false;
         memcpy(ip, pchOnionCat, sizeof(pchOnionCat));
         for (unsigned int i=0; i<16-sizeof(pchOnionCat); i++)
             ip[i + sizeof(pchOnionCat)] = vchAddr[i];
+        fqdn = strName;
+        LogPrint(BCLog::NET, "setspecail called with %s %d\n", strName, flag);
         return true;
     }
     return false;
 }
 
+bool CNetAddr::SetSpecial_v3(CNetAddr ref , int flag)
+{
+    //fqdn.resize(256);
+    std::string cp = ref.fqdn;
+    cp.resize(256);
+    if( flag < 254 ) {  memcpy(ip, pchOnionSeq, sizeof(pchOnionSeq));
+        for (unsigned int i=0; i<16-sizeof(pchOnionSeq); i++) {
+            ip[i + sizeof(pchOnionSeq)] = cp.c_str()[flag+i];
+        }
+        fqdn = cp.substr(flag,10);
+    }
+    if (flag == 255) { // backward comp
+        std::string name = EncodeBase32(&ip[6], 10) + ".onion";
+        fqdn = name;
+    }
+    if (flag == 256) {
+        for (unsigned int i=0; i<16-sizeof(pchOnionSeq); i++) {
+            fqdn[i] = char(ip[i + sizeof(pchOnionSeq)]);
+        }
+    }
+    return true;
+}
+
+CNetAddr::CNetAddr(const std::string &strName)
+{
+    SetSpecial(strName);
+    LogPrint(BCLog::NET,  "CNetAddr internal setspecail called from addr by name\n");
+}
+
 CNetAddr::CNetAddr(const struct in_addr& ipv4Addr)
 {
     SetRaw(NET_IPV4, (const uint8_t*)&ipv4Addr);
@@ -224,7 +265,15 @@ bool CNetAddr::IsHeNet() const
  */
 bool CNetAddr::IsTor() const
 {
-    return (memcmp(ip, pchOnionCat, sizeof(pchOnionCat)) == 0);
+    return (memcmp(ip, pchOnionCat, sizeof(pchOnionCat)) == 0) || IsTorSequence(0x0);
+}
+
+/**
+ * [@see](/github-metadata-backup-bitcoin-bitcoin/contributor/see/) CNetAddr::SetSpecial(const std::string &)
+ */
+bool CNetAddr::IsTorSequence(char seq) const
+{
+    return  memcmp(ip, pchOnionSeq, sizeof(pchOnionSeq)) == 0;
 }
 
 bool CNetAddr::IsLocal() const
@@ -333,8 +382,9 @@ enum Network CNetAddr::GetNetwork() const
 
 std::string CNetAddr::ToStringIP() const
 {
-    if (IsTor())
-        return EncodeBase32(&ip[6], 10) + ".onion";
+    if (IsTor()) {
+        return fqdn;
+    }
     if (IsInternal())
         return EncodeBase32(ip + sizeof(g_internal_prefix), sizeof(ip) - sizeof(g_internal_prefix)) + ".internal";
     CService serv(*this, 0);
@@ -640,6 +690,10 @@ CService::CService(const struct in6_addr& ipv6Addr, uint16_t portIn) : CNetAddr(
 {
 }
 
+CService::CService(const std::string fqdn, uint16_t portIn) : CNetAddr(fqdn), port(portIn)
+{
+}
+
 CService::CService(const struct sockaddr_in& addr) : CNetAddr(addr.sin_addr), port(ntohs(addr.sin_port))
 {
     assert(addr.sin_family == AF_INET);
diff --git a/src/netaddress.h b/src/netaddress.h
index c20101215..e18cc31a1 100644
--- a/src/netaddress.h
+++ b/src/netaddress.h
@@ -38,16 +38,18 @@ class CNetAddr
         CNetAddr();
         explicit CNetAddr(const struct in_addr& ipv4Addr);
         void SetIP(const CNetAddr& ip);
+        std::string fqdn = {};
 
         /**
          * Set raw IPv4 or IPv6 address (in network byte order)
-         * [@note](/github-metadata-backup-bitcoin-bitcoin/contributor/note/) Only NET_IPV4 and NET_IPV6 are allowed for network.
+         * [@note](/github-metadata-backup-bitcoin-bitcoin/contributor/note/) void SetRawBytes(const uint8_t *ip_in);Only NET_IPV4 and NET_IPV6 are allowed for network.
          */
         void SetRaw(Network network, const uint8_t *data);
-
+               void SetRawByte(char b, int pos);
         bool SetInternal(const std::string& name);
 
-        bool SetSpecial(const std::string &strName); // for Tor addresses
+        bool SetSpecial_v3(CNetAddr ref , int flag); // for Tor addresses
+        bool SetSpecial(const std::string &strName, int flag = 0);
         bool IsBindAny() const; // INADDR_ANY equivalent
         bool IsIPv4() const;    // IPv4 mapped address (::FFFF:0:0/96, 0.0.0.0/0)
         bool IsIPv6() const;    // IPv6 address (not mapped IPv4, not Tor)
@@ -67,6 +69,7 @@ class CNetAddr
         bool IsRFC6145() const; // IPv6 IPv4-translated address (::FFFF:0:0:0/96) (actually defined in RFC2765)
         bool IsHeNet() const;   // IPv6 Hurricane Electric - https://he.net (2001:0470::/36)
         bool IsTor() const;
+        bool IsTorSequence(char pseq) const;
         bool IsLocal() const;
         bool IsRoutable() const;
         bool IsInternal() const;
@@ -94,13 +97,14 @@ class CNetAddr
         int GetReachabilityFrom(const CNetAddr *paddrPartner = nullptr) const;
 
         explicit CNetAddr(const struct in6_addr& pipv6Addr, const uint32_t scope = 0);
+        CNetAddr(const std::string &strName);
         bool GetIn6Addr(struct in6_addr* pipv6Addr) const;
 
         friend bool operator==(const CNetAddr& a, const CNetAddr& b);
         friend bool operator!=(const CNetAddr& a, const CNetAddr& b) { return !(a == b); }
         friend bool operator<(const CNetAddr& a, const CNetAddr& b);
 
-        SERIALIZE_METHODS(CNetAddr, obj) { READWRITE(obj.ip); }
+        SERIALIZE_METHODS(CNetAddr, obj) { READWRITE(obj.ip, obj.fqdn); }
 
         friend class CSubNet;
 };
@@ -144,7 +148,8 @@ class CService : public CNetAddr
     public:
         CService();
         CService(const CNetAddr& ip, uint16_t port);
-        CService(const struct in_addr& ipv4Addr, uint16_t port);
+        CService(const struct in_addr& ipv4Addr, uint16_t port);;
+        CService(const std::string, uint16_t port);
         explicit CService(const struct sockaddr_in& addr);
         uint16_t GetPort() const;
         bool GetSockAddr(struct sockaddr* paddr, socklen_t *addrlen) const;
diff --git a/src/netbase.cpp b/src/netbase.cpp
index 3a3b5f3e6..816d66228 100644
--- a/src/netbase.cpp
+++ b/src/netbase.cpp
@@ -4,6 +4,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <netbase.h>
+#include <net.h>
 
 #include <sync.h>
 #include <tinyformat.h>
@@ -69,17 +70,18 @@ bool static LookupIntern(const std::string& name, std::vector<CNetAddr>& vIP, un
         return false;
     }
 
-    {
-        CNetAddr addr;
-        // From our perspective, onion addresses are not hostnames but rather
-        // direct encodings of CNetAddr much like IPv4 dotted-decimal notation
-        // or IPv6 colon-separated hextet notation. Since we can't use
-        // getaddrinfo to decode them and it wouldn't make sense to resolve
-        // them, we return a network address representing it instead. See
-        // CNetAddr::SetSpecial(const std::string&) for more details.
-        if (addr.SetSpecial(name)) {
+    CNetAddr addr;
+    // From our perspective, onion addresses are not hostnames but rather
+    // direct encodings of CNetAddr much like IPv4 dotted-decimal notation
+    // or IPv6 colon-separated hextet notation. Since we can't use
+    // getaddrinfo to decode them and it wouldn't make sense to resolve
+    // them, we return a network address representing it instead. See
+    // CNetAddr::SetSpecial(const std::string&) for more details.
+
+    if (name.find(".onion")) {
+        if (addr.SetSpecial(name,0)) {
             vIP.push_back(addr);
-            return true;
+            return false;
         }
     }
 
@@ -97,12 +99,19 @@ bool static LookupIntern(const std::string& name, std::vector<CNetAddr>& vIP, un
     // If we don't allow lookups, then use the AI_NUMERICHOST flag for
     // getaddrinfo to only decode numerical network addresses and suppress
     // hostname lookups.
-    aiHint.ai_flags = fAllowLookup ? AI_ADDRCONFIG : AI_NUMERICHOST;
+    aiHint.ai_flags = fAllowLookup && !HaveNameProxy() ? AI_ADDRCONFIG : AI_NUMERICHOST;
     struct addrinfo *aiRes = nullptr;
     int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes);
-    if (nErr)
-        return false;
 
+    LogPrintf("Resolver :%s error = %d  local resolve allowed %s  %s\n", name.c_str() ,nErr,fAllowLookup, nErr?"fail":"local resolved");
+    if (nErr ) {
+        CNetAddr resolved;
+        resolved.SetInternal(name);
+        if (resolved.IsInternal()) {
+            vIP.push_back(resolved);
+        }
+        return false;
+    }
     // Traverse the linked list starting with aiTrav, add all non-internal
     // IPv4,v6 addresses to vIP while respecting nMaxSolutions.
     struct addrinfo *aiTrav = aiRes;
@@ -182,7 +191,7 @@ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup)
     addr = vIP.front();
     return true;
 }
-
+;
 /**
  * Resolve a service string to its corresponding service.
  *
@@ -214,8 +223,17 @@ bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefau
 
     std::vector<CNetAddr> vIP;
     bool fRet = LookupIntern(hostname, vIP, nMaxSolutions, fAllowLookup);
-    if (!fRet)
-        return false;
+    if (vIP[0].IsTor()) {
+        vAddr.resize(hostname.size());
+        vAddr[0] = CService(hostname, port);
+        return true;
+    }
+    if (!fRet) // mayby just a name  and only onion we do not resovel so use tor
+    {
+        vAddr.resize(hostname.size());
+        vAddr[0] = CService(hostname, port);
+        return true;
+    }
     vAddr.resize(vIP.size());
     for (unsigned int i = 0; i < vIP.size(); i++)
         vAddr[i] = CService(vIP[i], port);

</details>

edit@laanwj: I've put the very long diff in a collapsible section in the interest of keeping the thread readable, edit@saibato thx, nice to know that ``` after the <p crush the format if one puts diff in a link.

@Saibato Let's keep the discussion focused on reviewing addrv2 and this specific code change. Although your proposal comes very late—we've been working on addrv2 for a long time—completely different suggestions are welcome, but please do so in a new PR. It would also need its own BIP.

Yup, i see.. For sure late raw and hacky and more a backup plan to get old node on also on board, to the path to other formats with ADDv2. or easy crypt of the packets.

So here we discuss ADDRv2 and my view that the address data should not be in ip and get a different space in the code stands firm. And I will try to come up with some suggested change. inspirit of review the PR edit> Since my autistic nature is incapable to take part in timed discussions ,like the RW club. I hope someone can reference the point of ip today for me?

@Saibato as it happens, there is an IRC review session on the ADDRv2 topic and this PR in 2 hours, if you'd like to join in. Info at https://bitcoincore.reviews

You may want to consider using a prevector instead of a vector for the IP data, to avoid needing a separate memory allocation for each IP address.

With a vector, an IPv4/IPv6 address would take 24 bytes directly + another 32 bytes for the allocation.

Rebased, added more tests and fixed some issues discovered by them and also addressed comments. @sipa changed to prevector and opted for a default size of 16, thanks for the suggestion!

Rebased to resolve conflicts.

Rebased just to restart Travis.

Rebased to resolve conflicts.

utACK

It's important to support new Tor service version 3 and adding support for i2p and cjdns is really awesome :+1:

Concept ACK

Rebased on latest #19845

Concept ACK 99047b0771f8dfc85640c095b8a331f4746e63e3, tested on Linux Mint 20 (x86_64).

Compare RPC and GUI output:

$ src/bitcoin-cli -netinfo 2
Bitcoin Core v0.20.99.0-336ba7ca7 - 70016/Satoshi:0.20.99/

Peer connections sorted by direction and min ping
<-> relay   net mping   ping send recv  txn  blk uptime id address                                                        
out  full onion   271    313    2    1    0           6  6 mwmfluek4au6mxxpw6fy7sjhkm65bdfc7izc7lpz3trewfdghyrzsbid.onion 
out  full onion   272    272    1    8    0           7  2 rp7k2go3s5lyj3fnj6zn62ktarlrsft2ohlsxkyd7v3e3idqyptvread.onion 
out  full onion   281    281    2    2    0           6 11 mwg3wapk3trgyg3e.onion:8333                                    
out  full onion   311    337    8    8    0           6 10 jzovs6u3fjpdrtow.onion:8333                                    
out  full onion   337    404    1    1    3           6  5 lw6hmfiyqkneb7biux5maaxztcizqnryjtrytwtsynj6szfwuo7dykyd.onion 
out  full onion   346    410    1    7    1           7  1 h25e66x235hdrwgw.onion:8333                                    
out  full onion   361    416    1    2    0           6 12 3ibzyvezkciodlfj.onion:8333                                    
out  full onion   404    678    4    5    0    1      6  8 vuswbtydwykwd5faap72ayjrchxz3wzezsi5tobahjedhf2fbg3dj7qd.onion 
out  full onion   408    542    2    2    0           6  4 esorot4j7z32pr7fm62pjgqhm2g5a26aaucesohjcy3csjewgmrvvmyd.onion 
out block onion   473    473   67    9                5 15 uleur3cw27x53j64.onion:8333                                    
out  full onion   501    634    6    6    0           6  7 wizbit5555bsslwv4ctronnsgk5vh2w2pdx7v7eyuivlyuoteejk7lid.onion 
out  full onion   539    539    2    1    0           6  3 yp77p2jxvz5nzza5.onion:8333                                    
out  full onion   546    859    2    2    0           6  9 e6pa22pv6k66lb7w.onion:8333                                    
out  full onion   623    623    3    6    0           5 13 blbmcefvms57el3f.onion:8333                                    
out  full onion   639    691    3    2    0    6      7  0 kpgvmscirrdqpekbqjsvw5teanhatztpp2gl6eee4zkowvwfxwenqaid.onion 
out block onion   669   1196   59   59                5 16 4opclnv4bgcpyano.onion:8333                                    
out  full onion  1595   2047    2    1                5 14 djhudr2ovodzeyv7.onion:8333                                    
                   ms     ms  sec  sec  min  min    min

        ipv4    ipv6   onion   total  block-relay
in         0       0       0       0       0
out        0       0      17      17       2
total      0       0      17      17       2

Local addresses
i4ezhlel222uleirryjorb66b2wtn5ttbs6ixqciqebg2zqand6de4qd.onion  port  8333     score      

UPDATE: the column width in the GUI just need to be adjusted :)

Added more tests.

Rebased on latest #19845

Mind rebasing?

<!--cf906140f33d8803c4a75a2196329ecb-->

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}

Rebased now that #19845 is merged.

Two more commits remain from this PR to be merged:

net: CAddress & CAddrMan: (un)serialize as ADDRv2 net: advertise support for ADDRv2 via new message

they are included in #19954.

Closing this as superseded by #19954.