net: improve encapsulation of CNetAddr #19360

1. 
   vasild commented at 9:27 am on June 23, 2020: member

   Do not access CNetAddr::ip directly from CService methods.

   This improvement will help later when we change the type of CNetAddr::ip (in the BIP155 implementation).

   (chopped off from #19031 to ease review)

2. fanquake added the label P2P on Jun 23, 2020
3. 
   vasild commented at 9:32 am on June 23, 2020: member

   All modified code is covered by existent tests. For example: CService::GetKey().
4. 
   DrahtBot commented at 9:50 am on June 23, 2020: member

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

   Conflicts

   No conflicts as of last run.

5. 
   naumenkogs commented at 12:32 pm on June 23, 2020: member

   utACK 567f98ea5d33d8cec699148be83b079dd5189101
6. net: improve encapsulation of CNetAddr

   Do not access `CNetAddr::ip` directly from `CService` methods.
   
   This improvement will help later when we change the type of
   `CNetAddr::ip` (in the BIP155 implementation).
   
   Co-authored-by: Carl Dong <contact@carldong.me>

   bc74a40a56
7. vasild force-pushed on Jul 10, 2020
8. 
   vasild commented at 2:02 pm on July 10, 2020: member

   Updated to use CNetAddr::GetAddrBytes(), see #19031 (comment) for longer explanation.

   Now this PR is even simpler.

9. 
   dongcarl commented at 4:27 pm on July 10, 2020: member

   ACK bc74a40a56128f81f11151d5966f53b82f19038c

   Restarted Travis job

10. in src/netaddress.cpp:729 in bc74a40a56

    730-     vKey.resize(18);
    731-     memcpy(vKey.data(), ip, 16);
    732-     vKey[16] = port / 0x100; // most significant byte of our port
    733-     vKey[17] = port & 0x0FF; // least significant byte of our port
    734-     return vKey;
    735+    auto key = GetAddrBytes();
    

    ---

    ---

    

    jonatack commented at 12:42 pm on July 11, 2020:

    I like auto, but if this is critical code, maybe it would be prudent to use the explicit std::vector<unsigned char> type.
11. 
    jonatack commented at 12:43 pm on July 11, 2020: member

    ACK bc74a40a56128f81f11151d5966f53b82f19038c
12. 
    naumenkogs commented at 7:19 am on July 14, 2020: member

    ACK bc74a40
13. 
    fjahr commented at 12:11 pm on July 15, 2020: member

    Code review ACK bc74a40
14. in src/netaddress.h:165 in bc74a40a56

    159@@ -160,7 +160,11 @@ class CService : public CNetAddr
    160         CService(const struct in6_addr& ipv6Addr, uint16_t port);
    161         explicit CService(const struct sockaddr_in6& addr);
    162 
    163-        SERIALIZE_METHODS(CService, obj) { READWRITE(obj.ip, Using<BigEndianFormatter<2>>(obj.port)); }
    164+        SERIALIZE_METHODS(CService, obj)
    165+        {
    166+            READWRITEAS(CNetAddr, obj);
    

    ---

    ---

    

    jnewbery commented at 4:55 pm on July 15, 2020:

    This should be updated to use the new AsBase<> syntax from #19503 if that PR gets merged first.
15. 
    jnewbery commented at 4:56 pm on July 15, 2020: member

    ACK bc74a40a5
16. 
    laanwj commented at 7:17 pm on July 15, 2020: member

    code review ACK bc74a40a56128f81f11151d5966f53b82f19038c
17. laanwj merged this on Jul 15, 2020
18. laanwj closed this on Jul 15, 2020

    ---

19. sidhujag referenced this in commit effb941b99 on Jul 16, 2020
20. vasild deleted the branch on Jul 16, 2020
21. 
    vasild commented at 10:16 am on July 16, 2020: member

    Thanks for reviewing!

    The reward for work well done is the opportunity to do more. – Jonas Salk

    So, the next one for review is at #19534 :-)

22. PastaPastaPasta referenced this in commit e8c6989e95 on Jan 16, 2021
23. UdjinM6 referenced this in commit bce94b7837 on Jan 18, 2021
24. deadalnix referenced this in commit 2184cc687d on Feb 5, 2021
25. UdjinM6 referenced this in commit 6e5210adeb on May 28, 2021
26. random-zebra referenced this in commit b4751e10ce on Aug 11, 2021
27. DrahtBot locked this on Feb 15, 2022

Contributors
vasild DrahtBot naumenkogs dongcarl jonatack fjahr jnewbery laanwj

Labels
P2P