net: remove unused CNetAddr scopeId member #19946

1. 
   jonatack commented at 10:07 am on September 12, 2020: member

   I’m probably missing something here but CNetAddr::scopeId seems unused. It isn’t covered by tests. It was added in eda3d9248971a1c3df6e6c4b23ba89be30508b51 and potentially caused an uninitialized read fixed in b7b36decaf878a8c1dcfdb4a27196c730043474b. Am running master on mainnet with the following asserts as a sanity check.

    0--- a/src/netaddress.cpp
    1+++ b/src/netaddress.cpp
    2@@ -132,6 +132,7 @@ CNetAddr::CNetAddr(const struct in_addr& ipv4Addr)
    3 CNetAddr::CNetAddr(const struct in6_addr& ipv6Addr, const uint32_t scope)
    4 {
    5     SetLegacyIPv6(Span<const uint8_t>(reinterpret_cast<const uint8_t*>(&ipv6Addr), sizeof(ipv6Addr)));
    6+    assert(scope == 0);
    7     scopeId = scope;
    8 }
    9 
   10@@ -716,6 +717,7 @@ bool CService::GetSockAddr(struct sockaddr* paddr, socklen_t *addrlen) const
   11+        assert(scopeId == 0);
   12         paddrin6->sin6_scope_id = scopeId;
   13--- a/src/netbase.cpp
   14+++ b/src/netbase.cpp
   15@@ -120,6 +120,7 @@ bool static LookupIntern(const std::string& name, std::vector<CNetAddr>& vIP, un
   16             struct sockaddr_in6* s6 = (struct sockaddr_in6*) aiTrav->ai_addr;
   17+            assert(s6->sin6_scope_id == 0);
   18             resolved = CNetAddr(s6->sin6_addr, s6->sin6_scope_id);
   19         }
   

2. fanquake added the label P2P on Sep 12, 2020
3. net: remove unused CNetAddr scopeId member 9662fadf44
4. jonatack force-pushed on Sep 12, 2020
5. 
   michaelfolkson commented at 12:52 pm on September 12, 2020: contributor

   Concept ACK. Seems convincing to me. Thanks for the clear explanation above on your process.

   What are you seeking from reviewers? Replicating the above steps? Running the fuzz tests due to the minor change introduced to them? Or do you just need experienced reviewers to weigh in on whether removing this might create a problem?

6. 
   Saibato commented at 2:22 pm on September 12, 2020: contributor

   tNACK with that merged we would no longer be able to connect or bind to specific scoped ipv6 addresses e.g.[::1%21].

   There might be users who use scopes to tunnel traffic with same “ip” number to different adapters. with {ipv6%n]

   in general https://tools.ietf.org/html/rfc4007 rfc’s in the form of n-007 are relevant, imho :smile_cat:

7. 
   jonatack commented at 2:35 pm on September 12, 2020: member

   @Saibato thanks for the info. I suspected there was something I was missing, but when no tests fail it still surprises me. Looks like we could use some.
8. jonatack closed this on Sep 12, 2020

   ---

9. jonatack deleted the branch on Sep 13, 2020
10. DrahtBot locked this on Feb 15, 2022

Contributors
jonatack michaelfolkson Saibato

Labels
P2P