Implement ADDRv2 support (part of BIP155) #19031

pull vasild wants to merge 2 commits into bitcoin:master from vasild:addrv2 changing 16 files +516 −41
  1. vasild commented at 4:16 pm on May 20, 2020: member

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

  2. in src/protocol.h:295 in 89d346a488 outdated 
    268@@ -263,6 +269,8 @@ enum ServiceFlags : uint64_t {
269     // NODE_WITNESS indicates that a node can be asked for blocks and transactions including
270     // witness data.
271     NODE_WITNESS = (1 << 3),
272+    // NODE_ADDRv2 indicates send and receive support for ADDRv2 messages (BIP155).
273+    NODE_ADDRv2 = (1 << 4),
    vasild commented at 4:18 pm on May 20, 2020:

    The signalling is done using a service flag because the service flags are gossiped and thus a node can choose to connect to ADDRv2-supporting nodes in the hopes of learning more e.g. Tor v3 addresses. For sure it is not going to learn any Tor v3, I2P or CJDNS addresses from nodes that do not support ADDRv2.

    However, BIP155 mentions support should be signalled by protocol version bump. And pull#16748 [WIP] Add support for addrv2 (BIP155) implemented this via a dedicated new type of messages sendaddrv2.

    I think that signalling via a service flag gives the most flexibility and thus I took the liberty to implement it that way, but I do not insist on it. I can change it to “protocol version bump”, “dedicated new message type” or something else.

  3. dongcarl commented at 4:55 pm on May 20, 2020: member

    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)

  4. laanwj commented at 5:03 pm on May 20, 2020: member
    Concept ACK!
  5. DrahtBot added the label GUI on May 20, 2020
  6. DrahtBot added the label P2P on May 20, 2020
  7. DrahtBot added the label RPC/REST/ZMQ on May 20, 2020
  8. DrahtBot added the label Tests on May 20, 2020
  9. vasild commented at 6:17 pm on May 20, 2020: member

    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.

  10. DrahtBot commented at 6:57 pm on May 20, 2020: member

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

    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.

  11. DrahtBot added the label Needs rebase on May 20, 2020
  12. vasild force-pushed on May 20, 2020
  13. vasild force-pushed on May 20, 2020
  14. DrahtBot removed the label Needs rebase on May 20, 2020
  15. naumenkogs commented at 10:13 pm on May 20, 2020: member
    Concept ACK. WIll do a good review later.
  16. vasild force-pushed on May 21, 2020
  17. vasild force-pushed on May 21, 2020
  18. vasild force-pushed on May 21, 2020
  19. vasild force-pushed on May 22, 2020
  20. vasild force-pushed on May 22, 2020
  21. MarcoFalke removed the label GUI on May 27, 2020
  22. MarcoFalke removed the label RPC/REST/ZMQ on May 27, 2020
  23. MarcoFalke removed the label Tests on May 27, 2020
  24. DrahtBot added the label Needs rebase on May 29, 2020
  25. vasild force-pushed on Jun 4, 2020
  26. vasild commented at 1:23 pm on June 4, 2020: member
    Rebased.
  27. DrahtBot removed the label Needs rebase on Jun 4, 2020
  28. vasild force-pushed on Jun 9, 2020
  29. vasild commented at 6:40 pm on June 9, 2020: member

    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:

    0 src/init.cpp                                |  2 +-
1 src/net.h                                   |  3 ++-
2 src/net_processing.cpp                      | 10 +++++++++-
3 src/protocol.cpp                            |  3 ++-
4 src/protocol.h                              |  8 ++++++--
5 test/functional/p2p_leak.py                 |  1 +
6 test/functional/p2p_node_network_limited.py |  4 ++--
7 test/functional/test_framework/messages.py  | 18 +++++++++++++++++-
8 test/functional/test_framework/mininode.py  |  3 +++
9 9 files changed, 43 insertions(+), 9 deletions(-)
  30. vasild force-pushed on Jun 9, 2020
  31. vasild commented at 6:54 pm on June 9, 2020: member
    Rebased.
  32. DrahtBot added the label Needs rebase on Jun 9, 2020
  33. vasild force-pushed on Jun 10, 2020
  34. vasild commented at 6:58 am on June 10, 2020: member
    Rebased.
  35. DrahtBot removed the label Needs rebase on Jun 10, 2020
  36. in src/netaddress.h:67 in 21e62e3245 outdated 
    37     NET_ONION,
38+
39+    /// A set of dummy addresses that map a name to an IPv6 address.
40     NET_INTERNAL,
41 
42+    /// Dummy value to indicate the number of NET_* constants.
    naumenkogs commented at 11:13 am on June 10, 2020:
    It has also something to do with NET_TEREDO below. I mean, the whole thing is somewhat confusing, but maybe there’s a chance to make it better or at least more clear?
    vasild commented at 12:03 pm on June 10, 2020:
    What about this? #19031 (review)
  37. in src/net.h:958 in 3deae0ce03 outdated 
    951@@ -951,11 +952,16 @@ class CNode
952 
953     void PushAddress(const CAddress& _addr, FastRandomContext &insecure_rand)
954     {
955+        // Whether the peer supports the address in `_addr`. For example,
956+        // nodes that do not implement BIP155 cannot receive Tor v3 addresses
957+        // because they require ADDRv2 (BIP155) encoding.
958+        const bool supported = m_supports_addrv2 || _addr.IsAddrV1Compatible();
    naumenkogs commented at 11:24 am on June 10, 2020:
    Can you rename the var to something more meaningful? At least addr_format_supported maybe. I see the comment, but well, it won’t hurt to have better war names too.
  38. naumenkogs commented at 11:34 am on June 10, 2020: member

    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.

  39. in src/netaddress.h:23 in 21e62e3245 outdated 
    15@@ -16,14 +16,30 @@
16 #include <string>
17 #include <vector>
18 
19+/**
20+ * A network type.
21+ * @note An address may belong to more than one network, for example `10.0.0.1`
22+ * belongs to both `NET_UNROUTABLE` and `NET_IPV4`.
23+ * Keep these sequential starting from 0 and `NET_MAX` as last.
    vasild commented at 11:54 am on June 10, 2020: 
    0 * Keep these sequential starting from 0 and `NET_MAX` as the last entry. 
1 * We have loops like `for (int i = 0; i < NET_MAX; i++)` that expect to iterate 
2 * over all enum values and also `GetExtNetwork()` "extends" this enum by 
3 * introducing standalone constants starting from `NET_MAX`.
  40. vasild commented at 12:40 pm on June 10, 2020: member

    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.

  41. vasild force-pushed on Jun 10, 2020
  42. DrahtBot added the label Needs rebase on Jun 19, 2020
  43. jonatack commented at 1:23 pm on June 19, 2020: member

    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.)

  44. vasild commented at 1:49 pm on June 22, 2020: member

    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

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

    Change CNetAddr::ip to have flexible size

    0net: change CNetAddr::ip to have flexible size

    Implement addrv2 (un)serializing

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

    0net: advertise support for ADDRv2 via new message
  45. vasild force-pushed on Jun 22, 2020
  46. vasild commented at 1:55 pm on June 22, 2020: member
    Rebased.
  47. jonatack commented at 1:59 pm on June 22, 2020: member
    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.
  48. DrahtBot removed the label Needs rebase on Jun 22, 2020
  49. laanwj referenced this in commit f591a1a184 on Jun 22, 2020
  50. vasild force-pushed on Jun 22, 2020
  51. vasild commented at 7:55 pm on June 22, 2020: member
    Rebased, removing commit test: add an edge case test for CSubNet which was merged via https://github.com/bitcoin/bitcoin/pull/19351
  52. vasild commented at 2:26 pm on June 23, 2020: member

    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!

  53. vasild force-pushed on Jul 6, 2020
  54. vasild commented at 2:04 pm on July 6, 2020: member
    Rebased to restart CI
  55. in src/net_processing.cpp:2604 in 6a5f357c9b outdated 
    2440@@ -2438,9 +2441,27 @@ void ProcessMessage(
2441         return;
2442     }
2443 
2444-    if (msg_type == NetMsgType::ADDR) {
2445+    if (msg_type == NetMsgType::ADDR || msg_type == NetMsgType::ADDRv2) {
2446+        const auto version_orig = vRecv.GetVersion();
2447+        if (msg_type == NetMsgType::ADDRv2) {
2448+            vRecv.SetVersion(version_orig | ADDRv2_FORMAT);
    naumenkogs commented at 3:12 pm on July 7, 2020:
    I have never seen this variable used like this before and I’ve just spent 10 minutes trying to figure out what’s going on, and I’m still a little confused. Maybe worth adding a comment? I think this code is counter-intuitive atm.
    vasild commented at 4:21 pm on July 9, 2020:

    Yes, that is a bit hackish but I do not think there is a cleaner way. It is somewhat similar to iostream::setf() which is used to configure the stream wrt how data should be parsed or printed, e.g.:

    0cout.setf(ios::boolalpha);
1cout << true; // prints (serializes as) "true" rather than "1"

    I added comments like:

    0// Sneak ADDRv2_FORMAT into the version so that the CNetAddr and CAddress
1// unserialize methods know that an address in v2 format is coming.

    to all places where we do that.

  56. in src/net.h:772 in 6a5f357c9b outdated 
    768@@ -769,6 +769,7 @@ class CNode
769     bool m_manual_connection{false};
770     bool fClient{false}; // set by version message
771     bool m_limited_node{false}; //after BIP159, set by version message
772+    std::atomic_bool m_supports_addrv2{false};
    dongcarl commented at 8:56 pm on July 7, 2020:
    I think perhaps m_wants_addrv2 would be a more appropriate name
    vasild commented at 4:22 pm on July 9, 2020:
    Renamed to m_wants_addrv2.
  57. in src/net_processing.cpp:2608 in 6a5f357c9b outdated 
    2459+            Misbehaving(pfrom.GetId(), 20,
2460+                strprintf(
2461+                    "Supplied us with a message that contains an unparsable address: %s",
2462+                    e.what()));
2463+            return;
2464+        }
    dongcarl commented at 9:00 pm on July 7, 2020:
    This part probably deserves some extra scrutiny, I know that it’s generally better to avoid exceptions in C++, but when I first wrote this I didn’t see another option here for bubbling up an error and handling it.
    vasild commented at 4:30 pm on July 9, 2020:

    Yes, I also couldn’t find a better way to cancel the parsing. I think that is a limitation of our ser/unser framework - its not possible to cancel the parsing of multiple entries in the middle (e.g. when parsing a std::vector of CNetAddr) without using an exception.

    Another way, but that’s not good, would be to not throw an exception from CNetAddr unserialize methods, consume the bogus data from the stream, mark the object as invalid, continue parsing the remaining CNetAddr objects and later remove the invalid objects. But that is not good because we do not want to continue parsing the data once we see that it is clearly malformed/malicious.

    dongcarl commented at 8:09 pm on July 9, 2020:
    @sipa Could you advise on whether there’s another way or if the current way is alright?
  58. in src/primitives/transaction.h:22 in 9b94ac3768 outdated 
    11@@ -12,6 +12,12 @@
12 #include <serialize.h>
13 #include <uint256.h>
14 
15+/**
16+ * A flag that is ORed into the protocol version to designate that a transaction
17+ * should be (un)serialized without witness data.
18+ * Make sure that this does not collide with any of the values in `version.h`
19+ * or with `ADDRv2_FORMAT`.
20+ */
    dongcarl commented at 9:02 pm on July 7, 2020:
    Potential followup: perhaps there is something cleaner we can do in the future for serialization/deserialization options other than abusing the protocol version.
    vasild commented at 7:47 pm on July 9, 2020:
    Right, there is room for improvement here. I opened #19477 to track this.
  59. dongcarl commented at 9:09 pm on July 7, 2020: member
    Could you take a look at the MSan/valgrind failures? :relaxed:
  60. vasild force-pushed on Jul 8, 2020
  61. vasild force-pushed on Jul 9, 2020
  62. DrahtBot added the label Needs rebase on Jul 9, 2020
  63. vasild force-pushed on Jul 9, 2020
  64. DrahtBot removed the label Needs rebase on Jul 9, 2020
  65. vasild force-pushed on Jul 10, 2020
  66. vasild commented at 1:59 pm on July 10, 2020: member

    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.

  67. Saibato commented at 6:21 pm on July 10, 2020: contributor

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

    02020-07-10T17:43:18Z sending feefilter (8 bytes) peer=0
12020-07-10T17:43:18Z trying connection [::]:0 lastseen=0.0hrs
22020-07-10T17:43:18Z SOCKS5 connecting [::]:0
32020-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

     0diff --git a/src/netaddress.cpp b/src/netaddress.cpp
 1index 1ac99ddd1..0171b1d80 100644
 2--- a/src/netaddress.cpp
 3+++ b/src/netaddress.cpp
 4@@ -21,6 +21,7 @@ CNetAddr::Bip155NetworkId CNetAddr::ToBIP155NetworkId() const
 5     case NET_ONION:
 6         switch (m_addr.size()) {
 7         case ADDR_TORv2_SIZE: return Bip155NetworkId::TORv2;
 8+        case ADDR_TORv3_SIZE: return Bip155NetworkId::TORv3;
 9         default: assert(!"Unexpected TOR address size");
10         }
11     case NET_UNROUTABLE:
12@@ -58,6 +59,11 @@ bool CNetAddr::FromBIP155NetworkId(Bip155NetworkId bip155_network_id,
13         }
14         return false;
15     case Bip155NetworkId::TORv3:
16+        if (address_size == ADDR_TORv3_SIZE) {
17+           net = NET_ONION;
18+           return true;
19+        }
20+        return false;
21     case Bip155NetworkId::I2P:
22     case Bip155NetworkId::CJDNS:
23         return false;
24@@ -137,7 +143,7 @@ bool CNetAddr::SetSpecial(const std::string &strName)
25 {
26     if (strName.size()>6 && strName.substr(strName.size() - 6, 6) == ".onion") {
27         std::vector<unsigned char> vchAddr = DecodeBase32(strName.substr(0, strName.size() - 6).c_str());
28-        if (vchAddr.size() != ADDR_TORv2_SIZE)
29+        if (vchAddr.size() != ADDR_TORv2_SIZE && vchAddr.size() != ADDR_TORv3_SIZE)
30             return false;
31         m_net = NET_ONION;
32         m_addr = vchAddr;
33diff --git a/src/netaddress.h b/src/netaddress.h
34index 0a94f0587..baf7d29a2 100644
35--- a/src/netaddress.h
36+++ b/src/netaddress.h
37@@ -81,6 +81,10 @@ static constexpr size_t ADDR_IPv6_SIZE = 16;
38 /// Size of TORv2 address (in bytes).
39 static constexpr size_t ADDR_TORv2_SIZE = 10;
40 
41+/// Size of TORv3 address (in bytes).
42+static constexpr size_t ADDR_TORv3_SIZE = 35;
43+
44+
45 /// Size of "internal" (NET_INTERNAL) address (in bytes).
46 static constexpr size_t ADDR_INTERNAL_SIZE = 10;
47 
48@@ -259,10 +263,12 @@ class CNetAddr
49                 memcpy(arr, m_addr.data(), m_addr.size());
50                 break;
51             case NET_ONION:
52+                if (ToBIP155NetworkId() == Bip155NetworkId::TORv2) {
53                 prefix_size = sizeof(TORv2_IN_IPv6_PREFIX);
54                 assert(m_addr.size() + prefix_size == sizeof(arr));
55                 memcpy(arr, TORv2_IN_IPv6_PREFIX, prefix_size);
56                 memcpy(arr + prefix_size, m_addr.data(), m_addr.size());
57+                }
58                 break;
59             case NET_INTERNAL:
60                 prefix_size = sizeof(INTERNAL_IN_IPv6_PREFIX);
  68. jonatack commented at 12:50 pm on July 11, 2020: member

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

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

  69. DrahtBot added the label Needs rebase on Jul 13, 2020
  70. vasild commented at 2:18 pm on July 13, 2020: member

    @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).

  71. vasild force-pushed on Jul 13, 2020
  72. DrahtBot removed the label Needs rebase on Jul 13, 2020
  73. dongcarl added the label Review club on Jul 13, 2020
  74. Saibato commented at 12:39 pm on July 14, 2020: contributor

    #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 after «. it might otherwise resolve in an hopefully unexpected unintended way. :woman_shrugging:

  75. vasild commented at 12:31 pm on July 15, 2020: member

    @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.

  76. Saibato commented at 1:54 pm on July 15, 2020: contributor

    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.

      0diff --git a/src/addrman.h b/src/addrman.h
  1index 8e82020df..4a810e53f 100644
  2--- a/src/addrman.h
  3+++ b/src/addrman.h
  4@@ -61,6 +61,7 @@ public:
  5     SERIALIZE_METHODS(CAddrInfo, obj)
  6     {
  7         READWRITEAS(CAddress, obj);
  8+        READWRITEAS(CNetAddr, obj);
  9         READWRITE(obj.source, obj.nLastSuccess, obj.nAttempts);
 10     }
 11 
 12diff --git a/src/net.cpp b/src/net.cpp
 13index 244b0094d..36af03d33 100644
 14--- a/src/net.cpp
 15+++ b/src/net.cpp
 16@@ -156,6 +156,7 @@ static std::vector<CAddress> convertSeed6(const std::vector<SeedSpec6> &vSeedsIn
 17         struct in6_addr ip;
 18         memcpy(&ip, seed_in.addr, sizeof(ip));
 19         CAddress addr(CService(ip, seed_in.port), GetDesirableServiceFlags(NODE_NONE));
 20+        LogPrintf("seeds %s\n", CNetAddr(addr).ToString());
 21         addr.nTime = GetTime() - rng.randrange(nOneWeek) - nOneWeek;
 22         vSeedsOut.push_back(addr);
 23     }
 24@@ -368,6 +369,9 @@ static CAddress GetBindAddress(SOCKET sock)
 25     return addr_bind;
 26 }
 27 
 28+static CAddress getadrr(std::string name) {
 29+       return CAddress(CService(name,18444), NODE_NONE);
 30+}
 31 CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, bool manual_connection, bool block_relay_only)
 32 {
 33     if (pszDest == nullptr) {
 34@@ -384,16 +388,23 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
 35     }
 36 
 37     /// debug print
 38-    LogPrint(BCLog::NET, "trying connection %s lastseen=%.1fhrs\n",
 39-        pszDest ? pszDest : addrConnect.ToString(),
 40+    LogPrint(BCLog::NET, "trying connection %s unresolved address %s lastseen=%.1fhrs\n",
 41+        pszDest ? pszDest:"", addrConnect.ToString(),
 42         pszDest ? 0.0 : (double)(GetAdjustedTime() - addrConnect.nTime)/3600.0);
 43 
 44+    std::string strName = std::string( pszDest ? pszDest:"");
 45     // Resolve
 46     const int default_port = Params().GetDefaultPort();
 47     if (pszDest) {
 48         std::vector<CService> resolved;
 49         if (Lookup(pszDest, resolved,  default_port, fNameLookup && !HaveNameProxy(), 256) && !resolved.empty()) {
 50+
 51+       // dont resolv local the onions
 52+       if (!strName.find(".onion")) {
 53+
 54             addrConnect = CAddress(resolved[GetRand(resolved.size())], NODE_NONE);
 55+
 56+
 57             if (!addrConnect.IsValid()) {
 58                 LogPrint(BCLog::NET, "Resolver returned invalid address %s for %s\n", addrConnect.ToString(), pszDest);
 59                 return nullptr;
 60@@ -410,6 +421,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
 61                 LogPrintf("Failed to open new connection, already connected\n");
 62                 return nullptr;
 63             }
 64+            }
 65         }
 66     }
 67 
 68@@ -417,7 +429,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
 69     bool connected = false;
 70     SOCKET hSocket = INVALID_SOCKET;
 71     proxyType proxy;
 72-    if (addrConnect.IsValid()) {
 73+    if (addrConnect.IsValid() && addrConnect.IsTor() && !pszDest) {
 74         bool proxyConnectionFailed = false;
 75 
 76         if (GetProxy(addrConnect.GetNetwork(), proxy)) {
 77@@ -425,14 +437,21 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
 78             if (hSocket == INVALID_SOCKET) {
 79                 return nullptr;
 80             }
 81-            connected = ConnectThroughProxy(proxy, addrConnect.ToStringIP(), addrConnect.GetPort(), hSocket, nConnectTimeout, proxyConnectionFailed);
 82+
 83+        if (addrConnect.ToStringIP() != "") {
 84+                connected = ConnectThroughProxy(proxy, addrConnect.ToStringIP(), addrConnect.GetPort(), hSocket, nConnectTimeout, proxyConnectionFailed);
 85+        } else return nullptr;
 86+
 87         } else {
 88-            // no proxy needed (none set for target network)
 89+            // no proxy needed (none set for tastrName.find(".onion")rget network)
 90             hSocket = CreateSocket(addrConnect);
 91             if (hSocket == INVALID_SOCKET) {
 92                 return nullptr;
 93             }
 94-            connected = ConnectSocketDirectly(addrConnect, hSocket, nConnectTimeout, manual_connection);
 95+
 96+            if (addrConnect.ToString() != "") {
 97+                connected = ConnectSocketDirectly(addrConnect, hSocket, nConnectTimeout, manual_connection);
 98+            } else return nullptr;
 99         }
100         if (!proxyConnectionFailed) {
101             // If a connection to the node was attempted, and failure (if any) is not caused by a problem connecting to
102@@ -444,14 +463,24 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
103         if (hSocket == INVALID_SOCKET) {
104             return nullptr;
105         }
106+
107         std::string host;
108         int port = default_port;
109         SplitHostPort(std::string(pszDest), port, host);
110         bool proxyConnectionFailed;
111-        connected = ConnectThroughProxy(proxy, host, port, hSocket, nConnectTimeout, proxyConnectionFailed);
112+        connected = false;
113+        //proxyConnectionFailed = true;
114+
115+
116+    if (strlen(pszDest) > 0) //strName.find(".onion"))
117+        {
118+        LogPrintf("Address to connect over proxz  %s %d\n", host, port);
119+            connected = ConnectThroughProxy(proxy, host, port, hSocket, nConnectTimeout, proxyConnectionFailed);
120+        } else return nullptr;
121+
122     }
123     if (!connected) {
124-        CloseSocket(hSocket);
125+        if (hSocket) CloseSocket(hSocket);
126         return nullptr;
127     }
128 
129@@ -1705,17 +1734,6 @@ void CConnman::ThreadDNSAddressSeed()
130     LogPrintf("%d addresses found from DNS seeds\n", found);
131 }
132 
133-
134-
135-
136-
137-
138-
139-
140-
141-
142-
143-
144 void CConnman::DumpAddresses()
145 {
146     int64_t nStart = GetTimeMillis();
147@@ -2057,6 +2075,8 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
148         }
149     } else if (FindNode(std::string(pszDest)))
150         return;
151+    // will "":port ever work?
152+    if(!pszDest && (addrConnect.ToStringIP() == "")) return;
153 
154     CNode* pnode = ConnectNode(addrConnect, pszDest, fCountFailure, manual_connection, block_relay_only);
155 
156@@ -2756,6 +2776,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
157     // peers (to prevent adversaries from inferring these links from addr
158     // traffic).
159     m_addr_known{block_relay_only ? nullptr : MakeUnique<CRollingBloomFilter>(5000, 0.001)},
160+
161     id(idIn),
162     nLocalHostNonce(nLocalHostNonceIn),
163     nLocalServices(nLocalServicesIn),
164diff --git a/src/net_processing.cpp b/src/net_processing.cpp
165index bfc60b18f..ecd83188e 100644
166--- a/src/net_processing.cpp
167+++ b/src/net_processing.cpp
168@@ -2454,14 +2454,47 @@ void ProcessMessage(
169         std::vector<CAddress> vAddrOk;
170         int64_t nNow = GetAdjustedTime();
171         int64_t nSince = nNow - 10 * 60;
172+        int i = 0;
173+        size_t len = 10;
174+        CAddress Tor = CAddress(vAddr[0]);
175+        char rawaddr[256] = {};
176+
177         for (CAddress& addr : vAddr)
178         {
179             if (interruptMsgProc)
180                 return;
181 
182+        if (addr.IsTorSequence(i)) {
183+            if (i == 0 ) {
184+                // LogPrintf("Adrress addr V3 raw  rec =");
185+                Tor = addr;
186+            }
187+            //V3sequence())
188+            addr.SetSpecial_v3(addr, 256); //fill hostdata from ip;
189+            memcpy(&rawaddr[i*10], &addr.fqdn.c_str()[0], 10);
190+            i++;
191+            if (i != 8) {
192+                continue;
193+            }
194+        } else addr.fqdn = {};
195+
196+        //LogPrintf("\nAdrress add raw? %s\n",  &rawaddr[0]);
197+
198+        if (i == 8) {
199+            Tor.SetSpecial_v3(addr, 255); // set clasic tor flag
200+            Tor.fqdn = rawaddr;
201+            if ( i == 8 ) Tor.fqdn = rawaddr;
202+            addr = Tor;
203+        }
204+
205+        if (addr.IsTor() && strlen(addr.fqdn.c_str()) < 32) addr.SetSpecial_v3(addr, 255); // v2 stzle
206+        LogPrint(BCLog::NET, "Adrress add? %s\n",  addr.ToString());
207+
208+
209             // We only bother storing full nodes, though this may include
210             // things which we would not make an outbound connection to, in
211             // part because we may make feeler connections to them.
212+
213             if (!MayHaveUsefulAddressDB(addr.nServices) && !HasAllDesirableServiceFlags(addr.nServices))
214                 continue;
215 
216@@ -3901,12 +3934,25 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
217             std::vector<CAddress> vAddr;
218             vAddr.reserve(pto->vAddrToSend.size());
219             assert(pto->m_addr_known);
220+            CAddress addr_add;
221             for (const CAddress& addr : pto->vAddrToSend)
222             {
223                 if (!pto->m_addr_known->contains(addr.GetKey()))
224                 {
225                     pto->m_addr_known->insert(addr.GetKey());
226-                    vAddr.push_back(addr);
227+                    if (!addr.IsTor()) vAddr.push_back(addr);
228+                    if (addr.IsTor()) LogPrint(BCLog::NET, "Try to send onion address = %s\n", addr.fqdn);
229+
230+                    if (addr.IsTor() && strlen(addr.fqdn.c_str()) > 32) {
231+                        for (int i=0;i<8;i++) {
232+                            addr_add = addr;
233+                            addr_add.SetSpecial_v3(addr, (i)*10);
234+
235+                            vAddr.push_back(addr_add);
236+
237+                        }
238+                    } else  vAddr.push_back(addr); //v2 tor
239+
240                     // receiver rejects addr messages larger than 1000
241                     if (vAddr.size() >= 1000)
242                     {
243diff --git a/src/netaddress.cpp b/src/netaddress.cpp
244index 674439161..8c8bad0f8 100644
245--- a/src/netaddress.cpp
246+++ b/src/netaddress.cpp
247@@ -5,13 +5,17 @@
248 
249 #include <cstdint>
250 #include <netaddress.h>
251+#include <netbase.h>
252+
253 #include <hash.h>
254 #include <util/strencodings.h>
255 #include <util/asmap.h>
256 #include <tinyformat.h>
257+#include <util/system.h>
258 
259 static const unsigned char pchIPv4[12] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff };
260 static const unsigned char pchOnionCat[] = {0xFD,0x87,0xD8,0x7E,0xEB,0x43};
261+static const unsigned char pchOnionSeq[] = {0xFD,'l','o','r','3',0x00};
262 
263 // 0xFD + sha256("bitcoin")[0:5]
264 static const unsigned char g_internal_prefix[] = { 0xFD, 0x6B, 0x88, 0xC0, 0x87, 0x24 };
265@@ -47,6 +51,11 @@ void CNetAddr::SetRaw(Network network, const uint8_t *ip_in)
266     }
267 }
268 
269+void CNetAddr::SetRawByte(char p , int pos)
270+{
271+    ip[6+pos] = p;
272+}
273+
274 /**
275  * Try to make this a dummy address that maps the specified name into IPv6 like
276  * so: (0xFD + %sha256("bitcoin")[0:5]) + %sha256(name)[0:10]. Such dummy
277@@ -70,6 +79,7 @@ bool CNetAddr::SetInternal(const std::string &name)
278     CSHA256().Write((const unsigned char*)name.data(), name.size()).Finalize(hash);
279     memcpy(ip, g_internal_prefix, sizeof(g_internal_prefix));
280     memcpy(ip + sizeof(g_internal_prefix), hash, sizeof(ip) - sizeof(g_internal_prefix));
281+    fqdn = name;
282     return true;
283 }
284 
285@@ -83,20 +93,51 @@ bool CNetAddr::SetInternal(const std::string &name)
286  *
287  * [@see](/bitcoin-bitcoin/contributor/see/) CNetAddr::IsTor(), CNetAddr::IsRFC4193()
288  */
289-bool CNetAddr::SetSpecial(const std::string &strName)
290+bool CNetAddr::SetSpecial(const std::string &strName, int flag)
291 {
292     if (strName.size()>6 && strName.substr(strName.size() - 6, 6) == ".onion") {
293-        std::vector<unsigned char> vchAddr = DecodeBase32(strName.substr(0, strName.size() - 6).c_str());
294-        if (vchAddr.size() != 16-sizeof(pchOnionCat))
295+        std::vector<unsigned char> vchAddr =  DecodeBase32(strName.substr(flag*10, strName.size() - 6).c_str());
296+        if (vchAddr.size() != 16-sizeof(pchOnionCat) && vchAddr.size() != 35)
297             return false;
298         memcpy(ip, pchOnionCat, sizeof(pchOnionCat));
299         for (unsigned int i=0; i<16-sizeof(pchOnionCat); i++)
300             ip[i + sizeof(pchOnionCat)] = vchAddr[i];
301+        fqdn = strName;
302+        LogPrint(BCLog::NET, "setspecail called with %s %d\n", strName, flag);
303         return true;
304     }
305     return false;
306 }
307 
308+bool CNetAddr::SetSpecial_v3(CNetAddr ref , int flag)
309+{
310+    //fqdn.resize(256);
311+    std::string cp = ref.fqdn;
312+    cp.resize(256);
313+    if( flag < 254 ) {  memcpy(ip, pchOnionSeq, sizeof(pchOnionSeq));
314+        for (unsigned int i=0; i<16-sizeof(pchOnionSeq); i++) {
315+            ip[i + sizeof(pchOnionSeq)] = cp.c_str()[flag+i];
316+        }
317+        fqdn = cp.substr(flag,10);
318+    }
319+    if (flag == 255) { // backward comp
320+        std::string name = EncodeBase32(&ip[6], 10) + ".onion";
321+        fqdn = name;
322+    }
323+    if (flag == 256) {
324+        for (unsigned int i=0; i<16-sizeof(pchOnionSeq); i++) {
325+            fqdn[i] = char(ip[i + sizeof(pchOnionSeq)]);
326+        }
327+    }
328+    return true;
329+}
330+
331+CNetAddr::CNetAddr(const std::string &strName)
332+{
333+    SetSpecial(strName);
334+    LogPrint(BCLog::NET,  "CNetAddr internal setspecail called from addr by name\n");
335+}
336+
337 CNetAddr::CNetAddr(const struct in_addr& ipv4Addr)
338 {
339     SetRaw(NET_IPV4, (const uint8_t*)&ipv4Addr);
340@@ -224,7 +265,15 @@ bool CNetAddr::IsHeNet() const
341  */
342 bool CNetAddr::IsTor() const
343 {
344-    return (memcmp(ip, pchOnionCat, sizeof(pchOnionCat)) == 0);
345+    return (memcmp(ip, pchOnionCat, sizeof(pchOnionCat)) == 0) || IsTorSequence(0x0);
346+}
347+
348+/**
349+ * [@see](/bitcoin-bitcoin/contributor/see/) CNetAddr::SetSpecial(const std::string &)
350+ */
351+bool CNetAddr::IsTorSequence(char seq) const
352+{
353+    return  memcmp(ip, pchOnionSeq, sizeof(pchOnionSeq)) == 0;
354 }
355 
356 bool CNetAddr::IsLocal() const
357@@ -333,8 +382,9 @@ enum Network CNetAddr::GetNetwork() const
358 
359 std::string CNetAddr::ToStringIP() const
360 {
361-    if (IsTor())
362-        return EncodeBase32(&ip[6], 10) + ".onion";
363+    if (IsTor()) {
364+        return fqdn;
365+    }
366     if (IsInternal())
367         return EncodeBase32(ip + sizeof(g_internal_prefix), sizeof(ip) - sizeof(g_internal_prefix)) + ".internal";
368     CService serv(*this, 0);
369@@ -640,6 +690,10 @@ CService::CService(const struct in6_addr& ipv6Addr, uint16_t portIn) : CNetAddr(
370 {
371 }
372 
373+CService::CService(const std::string fqdn, uint16_t portIn) : CNetAddr(fqdn), port(portIn)
374+{
375+}
376+
377 CService::CService(const struct sockaddr_in& addr) : CNetAddr(addr.sin_addr), port(ntohs(addr.sin_port))
378 {
379     assert(addr.sin_family == AF_INET);
380diff --git a/src/netaddress.h b/src/netaddress.h
381index c20101215..e18cc31a1 100644
382--- a/src/netaddress.h
383+++ b/src/netaddress.h
384@@ -38,16 +38,18 @@ class CNetAddr
385         CNetAddr();
386         explicit CNetAddr(const struct in_addr& ipv4Addr);
387         void SetIP(const CNetAddr& ip);
388+        std::string fqdn = {};
389 
390         /**
391          * Set raw IPv4 or IPv6 address (in network byte order)
392-         * [@note](/bitcoin-bitcoin/contributor/note/) Only NET_IPV4 and NET_IPV6 are allowed for network.
393+         * [@note](/bitcoin-bitcoin/contributor/note/) void SetRawBytes(const uint8_t *ip_in);Only NET_IPV4 and NET_IPV6 are allowed for network.
394          */
395         void SetRaw(Network network, const uint8_t *data);
396-
397+               void SetRawByte(char b, int pos);
398         bool SetInternal(const std::string& name);
399 
400-        bool SetSpecial(const std::string &strName); // for Tor addresses
401+        bool SetSpecial_v3(CNetAddr ref , int flag); // for Tor addresses
402+        bool SetSpecial(const std::string &strName, int flag = 0);
403         bool IsBindAny() const; // INADDR_ANY equivalent
404         bool IsIPv4() const;    // IPv4 mapped address (::FFFF:0:0/96, 0.0.0.0/0)
405         bool IsIPv6() const;    // IPv6 address (not mapped IPv4, not Tor)
406@@ -67,6 +69,7 @@ class CNetAddr
407         bool IsRFC6145() const; // IPv6 IPv4-translated address (::FFFF:0:0:0/96) (actually defined in RFC2765)
408         bool IsHeNet() const;   // IPv6 Hurricane Electric - https://he.net (2001:0470::/36)
409         bool IsTor() const;
410+        bool IsTorSequence(char pseq) const;
411         bool IsLocal() const;
412         bool IsRoutable() const;
413         bool IsInternal() const;
414@@ -94,13 +97,14 @@ class CNetAddr
415         int GetReachabilityFrom(const CNetAddr *paddrPartner = nullptr) const;
416 
417         explicit CNetAddr(const struct in6_addr& pipv6Addr, const uint32_t scope = 0);
418+        CNetAddr(const std::string &strName);
419         bool GetIn6Addr(struct in6_addr* pipv6Addr) const;
420 
421         friend bool operator==(const CNetAddr& a, const CNetAddr& b);
422         friend bool operator!=(const CNetAddr& a, const CNetAddr& b) { return !(a == b); }
423         friend bool operator<(const CNetAddr& a, const CNetAddr& b);
424 
425-        SERIALIZE_METHODS(CNetAddr, obj) { READWRITE(obj.ip); }
426+        SERIALIZE_METHODS(CNetAddr, obj) { READWRITE(obj.ip, obj.fqdn); }
427 
428         friend class CSubNet;
429 };
430@@ -144,7 +148,8 @@ class CService : public CNetAddr
431     public:
432         CService();
433         CService(const CNetAddr& ip, uint16_t port);
434-        CService(const struct in_addr& ipv4Addr, uint16_t port);
435+        CService(const struct in_addr& ipv4Addr, uint16_t port);;
436+        CService(const std::string, uint16_t port);
437         explicit CService(const struct sockaddr_in& addr);
438         uint16_t GetPort() const;
439         bool GetSockAddr(struct sockaddr* paddr, socklen_t *addrlen) const;
440diff --git a/src/netbase.cpp b/src/netbase.cpp
441index 3a3b5f3e6..816d66228 100644
442--- a/src/netbase.cpp
443+++ b/src/netbase.cpp
444@@ -4,6 +4,7 @@
445 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
446 
447 #include <netbase.h>
448+#include <net.h>
449 
450 #include <sync.h>
451 #include <tinyformat.h>
452@@ -69,17 +70,18 @@ bool static LookupIntern(const std::string& name, std::vector<CNetAddr>& vIP, un
453         return false;
454     }
455 
456-    {
457-        CNetAddr addr;
458-        // From our perspective, onion addresses are not hostnames but rather
459-        // direct encodings of CNetAddr much like IPv4 dotted-decimal notation
460-        // or IPv6 colon-separated hextet notation. Since we can't use
461-        // getaddrinfo to decode them and it wouldn't make sense to resolve
462-        // them, we return a network address representing it instead. See
463-        // CNetAddr::SetSpecial(const std::string&) for more details.
464-        if (addr.SetSpecial(name)) {
465+    CNetAddr addr;
466+    // From our perspective, onion addresses are not hostnames but rather
467+    // direct encodings of CNetAddr much like IPv4 dotted-decimal notation
468+    // or IPv6 colon-separated hextet notation. Since we can't use
469+    // getaddrinfo to decode them and it wouldn't make sense to resolve
470+    // them, we return a network address representing it instead. See
471+    // CNetAddr::SetSpecial(const std::string&) for more details.
472+
473+    if (name.find(".onion")) {
474+        if (addr.SetSpecial(name,0)) {
475             vIP.push_back(addr);
476-            return true;
477+            return false;
478         }
479     }
480 
481@@ -97,12 +99,19 @@ bool static LookupIntern(const std::string& name, std::vector<CNetAddr>& vIP, un
482     // If we don't allow lookups, then use the AI_NUMERICHOST flag for
483     // getaddrinfo to only decode numerical network addresses and suppress
484     // hostname lookups.
485-    aiHint.ai_flags = fAllowLookup ? AI_ADDRCONFIG : AI_NUMERICHOST;
486+    aiHint.ai_flags = fAllowLookup && !HaveNameProxy() ? AI_ADDRCONFIG : AI_NUMERICHOST;
487     struct addrinfo *aiRes = nullptr;
488     int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes);
489-    if (nErr)
490-        return false;
491 
492+    LogPrintf("Resolver :%s error = %d  local resolve allowed %s  %s\n", name.c_str() ,nErr,fAllowLookup, nErr?"fail":"local resolved");
493+    if (nErr ) {
494+        CNetAddr resolved;
495+        resolved.SetInternal(name);
496+        if (resolved.IsInternal()) {
497+            vIP.push_back(resolved);
498+        }
499+        return false;
500+    }
501     // Traverse the linked list starting with aiTrav, add all non-internal
502     // IPv4,v6 addresses to vIP while respecting nMaxSolutions.
503     struct addrinfo *aiTrav = aiRes;
504@@ -182,7 +191,7 @@ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup)
505     addr = vIP.front();
506     return true;
507 }
508-
509+;
510 /**
511  * Resolve a service string to its corresponding service.
512  *
513@@ -214,8 +223,17 @@ bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefau
514 
515     std::vector<CNetAddr> vIP;
516     bool fRet = LookupIntern(hostname, vIP, nMaxSolutions, fAllowLookup);
517-    if (!fRet)
518-        return false;
519+    if (vIP[0].IsTor()) {
520+        vAddr.resize(hostname.size());
521+        vAddr[0] = CService(hostname, port);
522+        return true;
523+    }
524+    if (!fRet) // mayby just a name  and only onion we do not resovel so use tor
525+    {
526+        vAddr.resize(hostname.size());
527+        vAddr[0] = CService(hostname, port);
528+        return true;
529+    }
530     vAddr.resize(vIP.size());
531     for (unsigned int i = 0; i < vIP.size(); i++)
532         vAddr[i] = CService(vIP[i], port);

    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.

  77. laanwj commented at 2:14 pm on July 15, 2020: member
    @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.
  78. Saibato commented at 2:26 pm on July 15, 2020: contributor

    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?

  79. jonatack commented at 2:55 pm on July 15, 2020: member
    @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
  80. in src/net_processing.cpp:2435 in 2533ce03c7 outdated 
    2404@@ -2405,6 +2405,9 @@ void ProcessMessage(
2405                       pfrom.m_tx_relay == nullptr ? "block-relay" : "full-relay");
2406         }
2407 
2408+        // Signal ADDRv2 support (BIP155).
2409+        connman->PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDADDRv2));
    jnewbery commented at 4:45 pm on July 15, 2020:

    There are a couple of things to note about this signalling method with our current version handshake:

    1. we currently send a getaddr before this, on receipt of the peer’s version message. If we don’t change that, then the peer will receive the getaddr before the sendaddrv2 message, and will therefore respond with an addr message. We only ever send one getaddr message per peer to get the large list of addresses.

    2. we also add our own address to vAddrToSend on receipt of the version message to advertise our address to the peer. That gets sent out in an addr message in the SendMessages() loop on the m_next_addr_send loop. I expect that will be on the first call time through SendMessages() since m_next_addr_send is initialized to 0. That means we’ll advertise our local address using an add message because we won’t have received our peer’s sendaddrv2 message yet.

    vasild commented at 3:51 pm on July 22, 2020:

    The first point is easy to fix:

     0diff --git i/src/net_processing.cpp w/src/net_processing.cpp
 1index 5af63acc4..296237075 100644
 2--- i/src/net_processing.cpp
 3+++ w/src/net_processing.cpp
 4@@ -2318,12 +2318,15 @@ void ProcessMessage(
 5         // Be shy and don't send version until we hear
 6         if (pfrom.fInbound)
 7             PushNodeVersion(pfrom, connman, GetAdjustedTime());
 8 
 9         connman.PushMessage(&pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERACK));
10 
11+        // Signal ADDRv2 support (BIP155).
12+        connman.PushMessage(&pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::SENDADDRv2));
13+
14         pfrom.nServices = nServices;
15         pfrom.SetAddrLocal(addrMe);
16         {
17             LOCK(pfrom.cs_SubVer);
18             pfrom.cleanSubVer = cleanSubVer;
19         }
20@@ -2428,15 +2431,12 @@ void ProcessMessage(
21             LogPrintf("New outbound peer connected: version: %d, blocks=%d, peer=%d%s (%s)\n",
22                       pfrom.nVersion.load(), pfrom.nStartingHeight,
23                       pfrom.GetId(), (fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToString()) : ""),
24                       pfrom.m_tx_relay == nullptr ? "block-relay" : "full-relay");
25         }
26 
27-        // Signal ADDRv2 support (BIP155).
28-        connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDADDRv2));
29-
30         if (pfrom.nVersion >= SENDHEADERS_VERSION) {
31             // Tell our peer we prefer to receive headers rather than inv's
32             // We send this to non-NODE NETWORK peers as well, because even
33             // non-NODE NETWORK peers can announce blocks (such as pruning
34             // nodes)
35             connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDHEADERS));

    would change the order of sent messages from verack, getaddr, sendaddrv2 to verack, sendaddrv2, getaddr.

    For the second point - the current code in this PR will assume no-addrv2 support when CNode::PushAddress() is called early when advertising our address. As a result if our address is e.g. TORv3, then it will not be appended to vAddrToSend[]. Hmm…

    vasild commented at 2:38 pm on July 24, 2020:

    On the second point:

    we also add our own address to vAddrToSend on receipt of the version message to advertise our address to the peer.

    Right, that is in AdvertiseLocal(), called from PeerLogicValidation::SendMessages()

    That gets sent out in an addr message in the SendMessages() loop on the m_next_addr_send loop.

    Not necessary as an addr message - if we have received sendaddrv2 and thus set CNode::m_wants_addrv2 to true, then we will send addrv2 message. But see below, you are right that on the first iteration we still have not received sendaddrv2.

    I expect that will be on the first call time through SendMessages() since m_next_addr_send is initialized to 0.

    Right.

    That means we’ll advertise our local address using an addr message because we won’t have received our peer’s sendaddrv2 message yet.

    Yes, for the first call to AdvertiseLocal() and only if our address is addr-compatible. Notice the following:

    • AdvertiseLocal() is being called periodically (AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL)
    • If our address is addr-compatible, then broadcasting it in addr message is fine, even if the peer has asked for addrv2 messages via sendaddrv2.
    • If our address is not addr-compatible (e.g. TORv3) then the first call to AdvertiseLocal() will end up being a noop because it will call CNode::PushAddress() which will see that the address being pushed is not supported by the peer and so it will not add it to vAddrToSend[].
      • On the next call to AdvertiseLocal() we should already have received sendaddrv2 and thus be aware that the peer supports addrv2 and so will properly advertise our address in addrv2 message.

    Verdict A “problem” exists only if our address is not addrv2-compatible (e.g. TORv3) and that “problem” manifests as the advertising of our address to the peer will not be done immediately when the connection is established, but some time later.

    Is that acceptable?

    jnewbery commented at 2:58 pm on July 24, 2020:

    I haven’t reviewed the implementation, but if it’s as you say:

    • If our address is not addr-compatible (e.g. TORv3) then the first call to AdvertiseLocal() will end up being a noop because it will call CNode::PushAddress() which will see that the address being pushed is not supported by the peer and so it will not add it to vAddrToSend[].
      • On the next call to AdvertiseLocal() we should already have received sendaddrv2 and thus be aware that the peer supports addrv2 and so will properly advertise our address in addrv2 message.

    then I think that’s fine.

    (My note was more something to be aware of than saying that this would be a serious issue)

    vasild commented at 3:19 pm on July 24, 2020:

    Yes, here are the changes to PushAddress():

    https://github.com/bitcoin/bitcoin/pull/19031/files#diff-1a8b9d1ad0a6fda5e751286c73102fc2R953

    It will only append the address to vAddrToSend[] if it is supported by the peer (which is - either the peer supports addrv2 or he does not but the address is addr-compatible).

    We can even avoid the delay if we detect that AdvertiseLocal() did nothing (due to addrv2 address and peer that does not support addrv2 (yet)) and if that is the case then don’t update m_next_local_addr_send so that the next iteration will also call AdvertiseLocal().

    vasild commented at 4:06 pm on July 30, 2020:
    I addressed the first concern and now the order of sent messages is verack, sendaddrv2, getaddr.
  81. laanwj referenced this in commit 3864219d40 on Jul 15, 2020
  82. sidhujag referenced this in commit effb941b99 on Jul 16, 2020
  83. vasild force-pushed on Jul 16, 2020
  84. vasild force-pushed on Jul 16, 2020
  85. vasild force-pushed on Jul 18, 2020
  86. DrahtBot added the label Needs rebase on Jul 22, 2020
  87. sipa commented at 5:11 pm on July 26, 2020: member

    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.

  88. laanwj referenced this in commit a76ccb01b9 on Jul 29, 2020
  89. vasild force-pushed on Jul 30, 2020
  90. vasild commented at 4:04 pm on July 30, 2020: member
    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!
  91. DrahtBot removed the label Needs rebase on Jul 30, 2020
  92. sidhujag referenced this in commit a040eaeadc on Jul 31, 2020
  93. DrahtBot added the label Needs rebase on Jul 31, 2020
  94. vasild force-pushed on Aug 10, 2020
  95. vasild commented at 8:17 pm on August 10, 2020: member
    Rebased to resolve conflicts.
  96. DrahtBot removed the label Needs rebase on Aug 10, 2020
  97. vasild force-pushed on Aug 11, 2020
  98. vasild commented at 12:07 pm on August 11, 2020: member
    Rebased just to restart Travis.
  99. DrahtBot added the label Needs rebase on Aug 12, 2020
  100. vasild force-pushed on Aug 12, 2020
  101. vasild commented at 7:45 am on August 12, 2020: member
    Rebased to resolve conflicts.
  102. DrahtBot removed the label Needs rebase on Aug 12, 2020
  103. DrahtBot added the label Needs rebase on Aug 24, 2020
  104. MarcoFalke referenced this in commit 8d6224fefe on Aug 25, 2020
  105. sidhujag referenced this in commit 348398bb82 on Aug 25, 2020
  106. vasild force-pushed on Aug 31, 2020
  107. DrahtBot removed the label Needs rebase on Aug 31, 2020
  108. vasild force-pushed on Aug 31, 2020
  109. vasild force-pushed on Sep 1, 2020
  110. vasild force-pushed on Sep 7, 2020
  111. DrahtBot added the label Needs rebase on Sep 10, 2020
  112. vasild force-pushed on Sep 10, 2020
  113. DrahtBot removed the label Needs rebase on Sep 10, 2020
  114. leto commented at 6:26 pm on September 11, 2020: contributor

    utACK

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

  115. practicalswift commented at 6:51 am on September 13, 2020: contributor
    Concept ACK
  116. vasild force-pushed on Sep 14, 2020
  117. vasild force-pushed on Sep 18, 2020
  118. vasild force-pushed on Sep 18, 2020
  119. vasild force-pushed on Sep 18, 2020
  120. vasild commented at 12:48 pm on September 18, 2020: member
    Rebased on latest #19845
  121. hebasto commented at 2:19 pm on September 26, 2020: member

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

    Compare RPC and GUI output:

     0$ src/bitcoin-cli -netinfo 2
 1Bitcoin Core v0.20.99.0-336ba7ca7 - 70016/Satoshi:0.20.99/
 2
 3Peer connections sorted by direction and min ping
 4<-> relay   net mping   ping send recv  txn  blk uptime id address                                                        
 5out  full onion   271    313    2    1    0           6  6 mwmfluek4au6mxxpw6fy7sjhkm65bdfc7izc7lpz3trewfdghyrzsbid.onion 
 6out  full onion   272    272    1    8    0           7  2 rp7k2go3s5lyj3fnj6zn62ktarlrsft2ohlsxkyd7v3e3idqyptvread.onion 
 7out  full onion   281    281    2    2    0           6 11 mwg3wapk3trgyg3e.onion:8333                                    
 8out  full onion   311    337    8    8    0           6 10 jzovs6u3fjpdrtow.onion:8333                                    
 9out  full onion   337    404    1    1    3           6  5 lw6hmfiyqkneb7biux5maaxztcizqnryjtrytwtsynj6szfwuo7dykyd.onion 
10out  full onion   346    410    1    7    1           7  1 h25e66x235hdrwgw.onion:8333                                    
11out  full onion   361    416    1    2    0           6 12 3ibzyvezkciodlfj.onion:8333                                    
12out  full onion   404    678    4    5    0    1      6  8 vuswbtydwykwd5faap72ayjrchxz3wzezsi5tobahjedhf2fbg3dj7qd.onion 
13out  full onion   408    542    2    2    0           6  4 esorot4j7z32pr7fm62pjgqhm2g5a26aaucesohjcy3csjewgmrvvmyd.onion 
14out block onion   473    473   67    9                5 15 uleur3cw27x53j64.onion:8333                                    
15out  full onion   501    634    6    6    0           6  7 wizbit5555bsslwv4ctronnsgk5vh2w2pdx7v7eyuivlyuoteejk7lid.onion 
16out  full onion   539    539    2    1    0           6  3 yp77p2jxvz5nzza5.onion:8333                                    
17out  full onion   546    859    2    2    0           6  9 e6pa22pv6k66lb7w.onion:8333                                    
18out  full onion   623    623    3    6    0           5 13 blbmcefvms57el3f.onion:8333                                    
19out  full onion   639    691    3    2    0    6      7  0 kpgvmscirrdqpekbqjsvw5teanhatztpp2gl6eee4zkowvwfxwenqaid.onion 
20out block onion   669   1196   59   59                5 16 4opclnv4bgcpyano.onion:8333                                    
21out  full onion  1595   2047    2    1                5 14 djhudr2ovodzeyv7.onion:8333                                    
22                   ms     ms  sec  sec  min  min    min
23
24        ipv4    ipv6   onion   total  block-relay
25in         0       0       0       0       0
26out        0       0      17      17       2
27total      0       0      17      17       2
28
29Local addresses
30i4ezhlel222uleirryjorb66b2wtn5ttbs6ixqciqebg2zqand6de4qd.onion  port  8333     score

    Screenshot from 2020-09-26 17-16-05

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

  122. vasild force-pushed on Sep 28, 2020
  123. vasild commented at 2:32 pm on September 28, 2020: member
    Added more tests.
  124. vasild force-pushed on Sep 28, 2020
  125. vasild commented at 2:34 pm on September 28, 2020: member
    Rebased on latest #19845
  126. sipa referenced this in commit 655937ebcb on Sep 28, 2020
  127. sidhujag referenced this in commit 06cbbae452 on Sep 29, 2020
  128. hebasto commented at 7:02 am on September 29, 2020: member
    Mind rebasing?
  129. DrahtBot added the label Needs rebase on Sep 29, 2020
  130. DrahtBot commented at 7:59 am on September 29, 2020: member

    🐙 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”.

  131. net: CAddress & CAddrMan: (un)serialize as ADDRv2 
    Change the serialization of `CAddrMan` to serialize its addresses
in ADDRv2/BIP155 format by default. Introduce a new `CAddrMan` format
version (3).

Add support for ADDRv2 format in `CAddress` (un)serialization.

Co-authored-by: Carl Dong <contact@carldong.me>
    e025121c1c
  132. net: advertise support for ADDRv2 via new message 
    Introduce a new message `sendaddrv2` to signal support for ADDRv2.
Send the new message immediately after sending the `VERACK` message.

Add support for receiving and parsing ADDRv2 messages.

Send ADDRv2 messages (instead of ADDR) to a peer if he has
advertised support for it.

Co-authored-by: Carl Dong <contact@carldong.me>
    fb90d274e3
  133. vasild force-pushed on Sep 29, 2020
  134. vasild commented at 8:49 am on September 29, 2020: member
    Rebased now that #19845 is merged.
  135. vasild commented at 8:53 am on September 29, 2020: member

    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.

  136. vasild closed this on Sep 29, 2020
  137. fanquake referenced this in commit 0b2abaa666 on Oct 11, 2020
  138. PastaPastaPasta referenced this in commit e8c6989e95 on Jan 16, 2021
  139. PastaPastaPasta referenced this in commit 1b237a541f on Jan 16, 2021
  140. PastaPastaPasta referenced this in commit 0e083669ff on Jan 16, 2021
  141. UdjinM6 referenced this in commit bce94b7837 on Jan 18, 2021
  142. UdjinM6 referenced this in commit f90a52ae45 on Jan 18, 2021
  143. UdjinM6 referenced this in commit 31ff054e1b on Jan 18, 2021
  144. PastaPastaPasta referenced this in commit 3af7c994db on Jan 18, 2021
  145. PastaPastaPasta referenced this in commit 888f36abe7 on Jan 18, 2021
  146. nunumichael referenced this in commit a1e6c55cbe on Feb 16, 2021
  147. nunumichael referenced this in commit 84f3be8ff6 on Feb 16, 2021
  148. nunumichael referenced this in commit faa2d23219 on Feb 16, 2021
  149. DrahtBot locked this on Feb 15, 2022


vasild dongcarl laanwj DrahtBot naumenkogs jonatack Saibato jnewbery sipa leto practicalswift hebasto

Labels
P2P Needs rebase Review club

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-23 06:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me