net, test: CNetAddr scoped ipv6 test coverage, rename scopeId to m_scope_id #19951

pull jonatack wants to merge 2 commits into bitcoin:master from jonatack:CNetAddr-scoped-ipv6-unit-tests-docs-and-rename-scopeId-to-m_scope_id changing 3 files +27 −4
  1. jonatack commented at 10:54 AM on September 13, 2020: member

    Add some test coverage for the IPv6 scoped address feature in netaddress/netbase per https://tools.ietf.org/html/rfc4007, update the member name scopeId to m_scope_id per doc/developer-notes.md, and improve the code doc.

    Reviewers can manually verify the test with these steps:

    <details><summary><em>you should see passing test output like this</em></summary><p>

    Running 1 test case...
Entering test module "Bitcoin Core Test Suite"
test/net_tests.cpp(85): Entering test suite "net_tests"
test/net_tests.cpp(204): Entering test case "cnetaddr_basic"
test/net_tests.cpp(210): info: check LookupHost("0.0.0.0", addr, false) has passed
test/net_tests.cpp(211): info: check !addr.IsValid() has passed
test/net_tests.cpp(212): info: check addr.IsIPv4() has passed
test/net_tests.cpp(214): info: check addr.IsBindAny() has passed
test/net_tests.cpp(215): info: check addr.ToString() == "0.0.0.0" has passed
test/net_tests.cpp(218): info: check LookupHost("255.255.255.255", addr, false) has passed
test/net_tests.cpp(219): info: check !addr.IsValid() has passed
test/net_tests.cpp(220): info: check addr.IsIPv4() has passed
test/net_tests.cpp(222): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(223): info: check addr.ToString() == "255.255.255.255" has passed
test/net_tests.cpp(226): info: check LookupHost("12.34.56.78", addr, false) has passed
test/net_tests.cpp(227): info: check addr.IsValid() has passed
test/net_tests.cpp(228): info: check addr.IsIPv4() has passed
test/net_tests.cpp(230): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(231): info: check addr.ToString() == "12.34.56.78" has passed
test/net_tests.cpp(234): info: check LookupHost("::", addr, false) has passed
test/net_tests.cpp(235): info: check !addr.IsValid() has passed
test/net_tests.cpp(236): info: check addr.IsIPv6() has passed
test/net_tests.cpp(238): info: check addr.IsBindAny() has passed
test/net_tests.cpp(239): info: check addr.ToString() == "::" has passed
test/net_tests.cpp(242): info: check LookupHost(ipv6_addr, addr, false) has passed
test/net_tests.cpp(243): info: check addr.IsValid() has passed
test/net_tests.cpp(244): info: check addr.IsIPv6() has passed
test/net_tests.cpp(246): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(247): info: check addr.ToString() == ipv6_addr has passed
test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
test/net_tests.cpp(255): info: check addr.IsValid() has passed
test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(258): info: check addr.ToString() == scoped_addr has passed
test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
test/net_tests.cpp(255): info: check addr.IsValid() has passed
test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(258): info: check addr.ToString() == scoped_addr has passed
test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
test/net_tests.cpp(255): info: check addr.IsValid() has passed
test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(258): info: check addr.ToString() == scoped_addr has passed
test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
test/net_tests.cpp(255): info: check addr.IsValid() has passed
test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(258): info: check addr.ToString() == scoped_addr has passed
test/net_tests.cpp(263): info: check !LookupHost(ipv6_addr + "%" + zone_id, addr, false) has passed
test/net_tests.cpp(263): info: check !LookupHost(ipv6_addr + "%" + zone_id, addr, false) has passed
test/net_tests.cpp(263): info: check !LookupHost(ipv6_addr + "%" + zone_id, addr, false) has passed
test/net_tests.cpp(266): info: check LookupHost(ipv6_addr + "%0", addr, false) has passed
test/net_tests.cpp(267): info: check addr.IsValid() has passed
test/net_tests.cpp(268): info: check addr.IsIPv6() has passed
test/net_tests.cpp(269): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(270): info: check addr.ToString() == ipv6_addr has passed
test/net_tests.cpp(274): info: check addr.IsValid() has passed
test/net_tests.cpp(275): info: check addr.IsTor() has passed
test/net_tests.cpp(277): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(278): info: check addr.ToString() == "6hzph5hv6337r6p2.onion" has passed
test/net_tests.cpp(282): info: check !addr.IsValid() has passed
test/net_tests.cpp(283): info: check addr.IsInternal() has passed
test/net_tests.cpp(285): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(286): info: check addr.ToString() == "esffpvrt3wpeaygy.internal" has passed
test/net_tests.cpp(204): Leaving test case "cnetaddr_basic"; testing time: 30933us
test/net_tests.cpp(85): Leaving test suite "net_tests"; testing time: 30975us
Leaving test module "Bitcoin Core Test Suite"; testing time: 31169us

*** No errors detected

    </p>/</details>

    • change this line in the code to break the feature:
    --- a/src/netaddress.cpp
+++ b/src/netaddress.cpp
@@ -716,7 +716,7 @@ bool CService::GetSockAddr(struct sockaddr* paddr, socklen_t *addrlen) const
         memset(paddrin6, 0, *addrlen);
         if (!GetIn6Addr(&paddrin6->sin6_addr))
             return false;
-        paddrin6->sin6_scope_id = m_scope_id;
+        // paddrin6->sin6_scope_id = m_scope_id;

    • rebuild, e.g. make

    • run the test: test/test_bitcoin -t net_tests/cnetaddr_basic -l all, verify that the added tests break

    <details><summary><em>you should see test output with a few failed tests like this</em></summary><p>

    Running 1 test case...
Entering test module "Bitcoin Core Test Suite"
test/net_tests.cpp(85): Entering test suite "net_tests"
test/net_tests.cpp(204): Entering test case "cnetaddr_basic"
test/net_tests.cpp(210): info: check LookupHost("0.0.0.0", addr, false) has passed
test/net_tests.cpp(211): info: check !addr.IsValid() has passed
test/net_tests.cpp(212): info: check addr.IsIPv4() has passed
test/net_tests.cpp(214): info: check addr.IsBindAny() has passed
test/net_tests.cpp(215): info: check addr.ToString() == "0.0.0.0" has passed
test/net_tests.cpp(218): info: check LookupHost("255.255.255.255", addr, false) has passed
test/net_tests.cpp(219): info: check !addr.IsValid() has passed
test/net_tests.cpp(220): info: check addr.IsIPv4() has passed
test/net_tests.cpp(222): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(223): info: check addr.ToString() == "255.255.255.255" has passed
test/net_tests.cpp(226): info: check LookupHost("12.34.56.78", addr, false) has passed
test/net_tests.cpp(227): info: check addr.IsValid() has passed
test/net_tests.cpp(228): info: check addr.IsIPv4() has passed
test/net_tests.cpp(230): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(231): info: check addr.ToString() == "12.34.56.78" has passed
test/net_tests.cpp(234): info: check LookupHost("::", addr, false) has passed
test/net_tests.cpp(235): info: check !addr.IsValid() has passed
test/net_tests.cpp(236): info: check addr.IsIPv6() has passed
test/net_tests.cpp(238): info: check addr.IsBindAny() has passed
test/net_tests.cpp(239): info: check addr.ToString() == "::" has passed
test/net_tests.cpp(242): info: check LookupHost(ipv6_addr, addr, false) has passed
test/net_tests.cpp(243): info: check addr.IsValid() has passed
test/net_tests.cpp(244): info: check addr.IsIPv6() has passed
test/net_tests.cpp(246): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(247): info: check addr.ToString() == ipv6_addr has passed
test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
test/net_tests.cpp(255): info: check addr.IsValid() has passed
test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(258): error: in "net_tests/cnetaddr_basic": check addr.ToString() == scoped_addr has failed [1122:3344:5566:7788:9900:aabb:ccdd:eeff != 1122:3344:5566:7788:9900:aabb:ccdd:eeff%1]
test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
test/net_tests.cpp(255): info: check addr.IsValid() has passed
test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(258): error: in "net_tests/cnetaddr_basic": check addr.ToString() == scoped_addr has failed [1122:3344:5566:7788:9900:aabb:ccdd:eeff != 1122:3344:5566:7788:9900:aabb:ccdd:eeff%21]
test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
test/net_tests.cpp(255): info: check addr.IsValid() has passed
test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(258): error: in "net_tests/cnetaddr_basic": check addr.ToString() == scoped_addr has failed [1122:3344:5566:7788:9900:aabb:ccdd:eeff != 1122:3344:5566:7788:9900:aabb:ccdd:eeff%19]
test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
test/net_tests.cpp(255): info: check addr.IsValid() has passed
test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(258): error: in "net_tests/cnetaddr_basic": check addr.ToString() == scoped_addr has failed [1122:3344:5566:7788:9900:aabb:ccdd:eeff != 1122:3344:5566:7788:9900:aabb:ccdd:eeff%3]
test/net_tests.cpp(263): info: check !LookupHost(ipv6_addr + "%" + zone_id, addr, false) has passed
test/net_tests.cpp(263): info: check !LookupHost(ipv6_addr + "%" + zone_id, addr, false) has passed
test/net_tests.cpp(263): info: check !LookupHost(ipv6_addr + "%" + zone_id, addr, false) has passed
test/net_tests.cpp(266): info: check LookupHost(ipv6_addr + "%0", addr, false) has passed
test/net_tests.cpp(267): info: check addr.IsValid() has passed
test/net_tests.cpp(268): info: check addr.IsIPv6() has passed
test/net_tests.cpp(269): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(270): info: check addr.ToString() == ipv6_addr has passed
test/net_tests.cpp(274): info: check addr.IsValid() has passed
test/net_tests.cpp(275): info: check addr.IsTor() has passed
test/net_tests.cpp(277): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(278): info: check addr.ToString() == "6hzph5hv6337r6p2.onion" has passed
test/net_tests.cpp(282): info: check !addr.IsValid() has passed
test/net_tests.cpp(283): info: check addr.IsInternal() has passed
test/net_tests.cpp(285): info: check !addr.IsBindAny() has passed
test/net_tests.cpp(286): info: check addr.ToString() == "esffpvrt3wpeaygy.internal" has passed
test/net_tests.cpp(204): Leaving test case "cnetaddr_basic"; testing time: 32316us
test/net_tests.cpp(85): Leaving test suite "net_tests"; testing time: 32354us
Leaving test module "Bitcoin Core Test Suite"; testing time: 32522us

*** 4 failures are detected in the test module "Bitcoin Core Test Suite"

    </p></details>

    • leave your review here
  2. fanquake added the label P2P on Sep 13, 2020
  3. jonatack commented at 10:56 AM on September 13, 2020: member

    Thanks to @Saibato for setting me straight on this in #19946 and to @michaelfolkson for looking at that PR.

  4. jonatack force-pushed on Sep 13, 2020
  5. michaelfolkson commented at 11:22 AM on September 13, 2020: contributor

    Concept ACK. Will follow the above steps before ACKing the commit.

  6. jonatack force-pushed on Sep 13, 2020
  7. jonatack commented at 12:01 PM on September 13, 2020: member

    I don't know why the travis xenial and macOS GUI/no depends are failing. All the other CI jobs pass on travis, cirrus, appveyor ones and bitcoin_builds.org.

  8. jonatack force-pushed on Sep 13, 2020
  9. Saibato commented at 1:39 PM on September 13, 2020: contributor

    I don't know why the travis xenial and macOS GUI/no depends are failing. All the other CI jobs pass on travis, cirrus, appveyor ones and bitcoin_builds.org.

    BOOST_CHECK_EQUAL(addr.ToString(), ipv6_addr); ipv6 with % != ?

    test/net_tests.cpp:259: error: in "net_tests/cnetaddr_basic": check addr.ToString() == scoped_addr has failed [1122:3344:5566:7788:9900:aabb:ccdd:eeff != 1122:3344:5566:7788:9900:aabb:ccdd:eeff%1]

    i.e. test, does the stack support scopes at all?

  10. practicalswift commented at 1:55 PM on September 13, 2020: contributor

    Concept ACK

    Thanks for adding tests! ❤️

  11. sipa commented at 5:09 PM on September 13, 2020: member

    I don't think scope ids are intended to be portable.

    They're a way for a user, on their local system, to specify what interface to use if you have multiple and are using a link-local address.

    But in the end it's just a string that's passed to the OS for parsing. What interfaces exist, how they're named, and in what situation they're accepted is entirely up to it.

    One idea that may make it more portable is restricting these scopes in tests to link-local addresses (inside fe80::/10), as the OS may refuse them outside that range.

    If that still doesn't work... this is really no different than trying to unit test whether DNS resolving works, and then noticing the test fails on an offline system or so. It's dependent on local configuration, and there is no good cross-platform spec that all OSes adhere to.

  12. jonatack force-pushed on Sep 13, 2020
  13. jonatack force-pushed on Sep 13, 2020
  14. jonatack commented at 9:07 PM on September 13, 2020: member

    One idea that may make it more portable is restricting these scopes in tests to link-local addresses (inside fe80::/10), as the OS may refuse them outside that range.

    This suggestion solved the Travis Xenial job and stabilized the macOS no-depends one enough to enable a workaround. Thank you, @sipa!

    CI is now green; ready for review.

  15. in src/test/net_tests.cpp:261 in 431ebcefc6 outdated 
     256 | +        BOOST_REQUIRE(LookupHost(scoped_addr, addr, false));
 257 | +        BOOST_REQUIRE(addr.IsValid());
 258 | +        BOOST_REQUIRE(addr.IsIPv6());
 259 | +        BOOST_CHECK(!addr.IsBindAny());
 260 | +        const std::string addr_str{addr.ToString()};
 261 | +        BOOST_CHECK(addr_str == scoped_addr || addr_str == link_local_edge_case);
    eriknylund commented at 9:56 AM on September 14, 2020:

    I tested this locally on macOS Catalina 10.15.6 and I get an error on this line:

    test/net_tests.cpp:261: error: in "net_tests/cnetaddr_basic": check addr_str == scoped_addr || addr_str == link_local_edge_case has failed

    Update: It fails on scope 8 and 21, but not on 32.

    jonatack commented at 10:38 AM on September 14, 2020:

    Thanks @eriknylund! If you don't mind, can you give the output of rebuilding and running the test after adding these 2 lines?

             const std::string addr_str{addr.ToString()};
+        BOOST_TEST_MESSAGE(scoped_addr);
+        BOOST_TEST_MESSAGE(addr_str);
         BOOST_CHECK(addr_str == scoped_addr || addr_str == link_local_edge_case);
     }
    jonatack commented at 10:43 AM on September 14, 2020:

    (ISTM the lower scope values are often used by the local machine, so higher values may indeed make the test more agnostic.)

    eriknylund commented at 10:53 AM on September 14, 2020:

    (ISTM the lower scope values are often used by the local machine, so higher values may indeed make the test more agnostic.)

    That seems to be the issue, en2 and vmnet8 are two network interfaces on my local machine:

    fe80::1%8
fe80::1%en2
test/net_tests.cpp:264: error: in "net_tests/cnetaddr_basic": check addr_str == scoped_addr || addr_str == link_local_edge_case has failed
fe80::1%21
fe80::1%vmnet8
test/net_tests.cpp:264: error: in "net_tests/cnetaddr_basic": check addr_str == scoped_addr || addr_str == link_local_edge_case has failed
fe80::1%32
fe80:0:0:0:0:0:0:1
    jonatack commented at 11:16 AM on September 14, 2020:

    Perfect, thanks. Simplified the test to only the 32 and 0 values. No need for more than that.

  16. jonatack force-pushed on Sep 14, 2020
  17. jonatack force-pushed on Sep 14, 2020
  18. eriknylund approved
  19. eriknylund commented at 3:47 PM on September 14, 2020: contributor

    ACK 88b6f85419b65122d5a4469a8585b276960bbb1e I built the PR on macOS Catalina 10.15.6 and ran both tests and functional tests - all pass. I've reviewed the two commits and I think the changes look good.

    I also repeated the test instructions to verify that the tests break accordingly (a bit different now because of the modified test):

    test/net_tests.cpp:258: error: in "net_tests/cnetaddr_basic": check addr_str == scoped_addr || addr_str == "fe80:0:0:0:0:0:0:1" has failed
*** 1 failure is detected in the test module "Bitcoin Core Test Suite"
  20. Saibato commented at 4:51 PM on September 14, 2020: contributor

    btw: That macOS edge case results fromstd::string CNetAddr::ToStringIP() const in src/netaddress.cpp if we can not GetSockAddr(... and result there in fallback.

    If we convert the addr boost or https://tools.ietf.org/html/rfc4291 style we do not need the special addr representation string fe80:0:0:0:0:0:0:1"

    That patch would do the trick?

    diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
index e7156d1d3..108b1ace8 100644
--- a/src/test/net_tests.cpp
+++ b/src/test/net_tests.cpp
@@ -19,6 +19,7 @@
 #include <version.h>
 
 #include <boost/test/unit_test.hpp>
+#include <boost/asio/ip/address.hpp>
 
 #include <memory>
 #include <string>
@@ -251,12 +252,12 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic)
     const std::string link_local{"fe80::1"};
     const std::string scoped_addr{link_local + "%32"};
     BOOST_REQUIRE(LookupHost(scoped_addr, addr, false));
+
     BOOST_REQUIRE(addr.IsValid());
     BOOST_REQUIRE(addr.IsIPv6());
     BOOST_CHECK(!addr.IsBindAny());
-    const std::string addr_str{addr.ToString()};
-    BOOST_CHECK(addr_str == scoped_addr || addr_str == "fe80:0:0:0:0:0:0:1");
-    // The fallback case "fe80:0:0:0:0:0:0:1" is needed for macOS 10.14/10.15.
+    const std::string addr_str{(boost::asio::ip::address::from_string(addr.ToString())).to_string()};
+    BOOST_CHECK(addr_str == scoped_addr);
     // Test that the delimiter "%" and default zone id of 0 can be omitted for the default scope.
     BOOST_REQUIRE(LookupHost(link_local + "%0", addr, false));
     BOOST_REQUIRE(addr.IsValid());
  21. eriknylund commented at 5:28 AM on September 15, 2020: contributor

    btw: That macOS edge case results fromstd::string CNetAddr::ToStringIP() const in src/netaddress.cpp if we can not GetSockAddr(... and result there in fallback.

    If we convert the addr boost or https://tools.ietf.org/html/rfc4291 style we do not need the special addr representation string fe80:0:0:0:0:0:0:1"

    That patch would do the trick?

    diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
index e7156d1d3..108b1ace8 100644
--- a/src/test/net_tests.cpp
+++ b/src/test/net_tests.cpp
@@ -19,6 +19,7 @@
 #include <version.h>
 
 #include <boost/test/unit_test.hpp>
+#include <boost/asio/ip/address.hpp>
 
 #include <memory>
 #include <string>
@@ -251,12 +252,12 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic)
     const std::string link_local{"fe80::1"};
     const std::string scoped_addr{link_local + "%32"};
     BOOST_REQUIRE(LookupHost(scoped_addr, addr, false));
+
     BOOST_REQUIRE(addr.IsValid());
     BOOST_REQUIRE(addr.IsIPv6());
     BOOST_CHECK(!addr.IsBindAny());
-    const std::string addr_str{addr.ToString()};
-    BOOST_CHECK(addr_str == scoped_addr || addr_str == "fe80:0:0:0:0:0:0:1");
-    // The fallback case "fe80:0:0:0:0:0:0:1" is needed for macOS 10.14/10.15.
+    const std::string addr_str{(boost::asio::ip::address::from_string(addr.ToString())).to_string()};
+    BOOST_CHECK(addr_str == scoped_addr);
     // Test that the delimiter "%" and default zone id of 0 can be omitted for the default scope.
     BOOST_REQUIRE(LookupHost(link_local + "%0", addr, false));
     BOOST_REQUIRE(addr.IsValid());

    It's an interesting thought @Saibato, but I'm unsure if it will work here? I tried the patch by adding more verbose output with the following result:

        const std::string addr_str{(boost::asio::ip::address::from_string(addr.ToString())).to_string()};
    BOOST_TEST_MESSAGE(scoped_addr);
    BOOST_TEST_MESSAGE(addr_str);
    BOOST_CHECK(addr_str == scoped_addr);

    test/test_bitcoin -t net_tests/cnetaddr_basic -l all gives me:

    fe80::1%32
fe80::1
test/net_tests.cpp:267: error: in "net_tests/cnetaddr_basic": check addr_str == scoped_addr has failed
  22. jonatack commented at 5:47 AM on September 15, 2020: member

    Thanks @Saibato for the idea and @eriknylund for testing. MacOS 10.14+ seems to be a special case either way and I'd rather test what our code in ToStringIP() actually returns, so leaving this as-is for now.

  23. michaelfolkson commented at 10:26 AM on September 15, 2020: contributor

    Replicated steps @eriknylund followed here and got the same results on Mac OS Catalina 10.15.6. All tests passed and the single test failed with the broken feature.

    Code changes excluding the test changes are clearly fine.

    Anything else one could do before ACKing the commit? Running the old tests (tests before they were updated in this PR) on the broken feature to see that the old tests wouldn't have failed with this change?

  24. jonatack commented at 12:09 PM on October 2, 2020: member

    Up for grabs.

  25. jonatack closed this on Oct 2, 2020
  26. in src/test/net_tests.cpp:25 in 88b6f85419 outdated 
      21 | @@ -22,6 +22,7 @@
  22 |  
  23 |  #include <memory>
  24 |  #include <string>
  25 | +#include <vector>
    jonatack commented at 1:07 PM on October 2, 2020:

    This line is left over from an earlier push and should be removed.

    Other than that, I think this PR was finished, provided missing coverage+context, and just needed some review begging or sales effort.

  27. in src/test/net_tests.cpp:259 in 88b6f85419 outdated 
     254 | +    BOOST_REQUIRE(addr.IsValid());
 255 | +    BOOST_REQUIRE(addr.IsIPv6());
 256 | +    BOOST_CHECK(!addr.IsBindAny());
 257 | +    const std::string addr_str{addr.ToString()};
 258 | +    BOOST_CHECK(addr_str == scoped_addr || addr_str == "fe80:0:0:0:0:0:0:1");
 259 | +    // The fallback case "fe80:0:0:0:0:0:0:1" is needed for macOS 10.14/10.15.
    jonatack commented at 1:09 PM on October 2, 2020:

    Maybe "macOS 10.14/10.15" -> "macOS 10.14+"

  28. laanwj added the label Tests on Oct 2, 2020
  29. laanwj commented at 2:09 PM on October 2, 2020: member

    FWIW, I still think this is worth doing.

  30. jonatack reopened this on Oct 2, 2020
  31. jonatack force-pushed on Oct 2, 2020
  32. jonatack commented at 2:35 PM on October 2, 2020: member

    Rebased for the merged addrv2 changes, and updated to remove an unneeded include from an earlier push here and update a comment. Diff: git range-diff df2129a 88b6f85 f36887f

    /src/test/net_tests.cpp
 
 #include <string>
-#include <vector>
 
@@ -256,7 +255,7 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic)
     BOOST_CHECK(!addr.IsBindAny());
     const std::string addr_str{addr.ToString()};
     BOOST_CHECK(addr_str == scoped_addr || addr_str == "fe80:0:0:0:0:0:0:1");
-    // The fallback case "fe80:0:0:0:0:0:0:1" is needed for macOS 10.14/10.15.
+    // The fallback case "fe80:0:0:0:0:0:0:1" is needed for macOS 10.14/10.15 and (probably) later.
     // Test that the delimiter "%" and default zone id of 0 can be omitted for the default scope.
  33. test: add test coverage for CNetAddr ipv6 scoped addresses 5cb5fd3005
  34. net: rename CNetAddr scopeId to m_scope_id, improve code doc f36887fa47
  35. jonatack force-pushed on Oct 2, 2020
  36. laanwj commented at 3:02 PM on October 2, 2020: member

    ACK f36887fa47b42af60f8a06a3995baca7c73ca310

  37. laanwj merged this on Oct 2, 2020
  38. laanwj closed this on Oct 2, 2020
  39. jonatack deleted the branch on Oct 2, 2020
  40. in src/test/net_tests.cpp:261 in f36887fa47 
     256 | +    BOOST_REQUIRE(addr.IsIPv6());
 257 | +    BOOST_CHECK(!addr.IsBindAny());
 258 | +    const std::string addr_str{addr.ToString()};
 259 | +    BOOST_CHECK(addr_str == scoped_addr || addr_str == "fe80:0:0:0:0:0:0:1");
 260 | +    // The fallback case "fe80:0:0:0:0:0:0:1" is needed for macOS 10.14/10.15 and (probably) later.
 261 | +    // Test that the delimiter "%" and default zone id of 0 can be omitted for the default scope.
    ryanofsky commented at 10:57 PM on April 11, 2021:

    I saw this check fail in a recent cirrus run [previous releases, uses qt5 dev package and some depends packages] [unsigned char] [bionic]:

    https://cirrus-ci.com/task/5950587750580224?logs=ci#L4679

    I'm restarting the test, will see if happens again.

    jonatack commented at 6:22 PM on April 12, 2021:

    Thanks for reporting. It may be hitting a reserved id; have had it happen on occasion locally and bumping the 32 to a higher value resolved it. What happened after restarting?

        // Test with a fairly-high value, e.g. 32, to avoid locally reserved ids.
    const std::string link_local{"fe80::1"};
    const std::string scoped_addr{link_local + "%32"};
    ryanofsky commented at 6:45 PM on April 12, 2021:

    Thanks, interesting! (I don't know what a reserved id is and googling didn't seem to turn up anything helpful, so I'd be curious for any pointers.)

    But test passed on restart so it is an intermittent issue like you suggested. Maybe the test can be written so the check is less strict and just depends on parsing behavior not OS behavior. But it doesn't seem too urgent if this is a known infrequent issue.

    jonatack commented at 6:25 PM on April 14, 2021:

    My bad for ambiguous wording! s/reserved id/id in use/...will fix issue and wording.

  41. PastaPastaPasta referenced this in commit e0b0a312a7 on Jun 27, 2021
  42. PastaPastaPasta referenced this in commit dd7f6d697c on Jun 28, 2021
  43. PastaPastaPasta referenced this in commit 9cc2838154 on Jun 29, 2021
  44. Fabcien referenced this in commit 710e9aeb38 on Nov 3, 2021
  45. Fabcien referenced this in commit de3ebfa9af on Nov 3, 2021
  46. DeckerSU referenced this in commit 4cb98fc3f3 on Feb 8, 2022
  47. bitcoin locked this on Aug 18, 2022
  48. in src/test/net_tests.cpp:266 in f36887fa47 
     261 | +    // Test that the delimiter "%" and default zone id of 0 can be omitted for the default scope.
 262 | +    BOOST_REQUIRE(LookupHost(link_local + "%0", addr, false));
 263 | +    BOOST_REQUIRE(addr.IsValid());
 264 | +    BOOST_REQUIRE(addr.IsIPv6());
 265 | +    BOOST_CHECK(!addr.IsBindAny());
 266 | +    BOOST_CHECK_EQUAL(addr.ToString(), link_local);
    hebasto commented at 3:35 PM on December 30, 2024:

    Apparently, this test is not portable. See: #31580.

Contributors
jonatackmichaelfolksonSaibatopracticalswiftsipaeriknylundlaanwjryanofskyhebasto
Labels
TestsP2P
Linked (view graph)
#31580 test: Remove non-portable IPv6 test
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: 2026-04-22 03:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me